Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support refreshing of temporary tokens #14578

Open
3 tasks
hexylena opened this issue Sep 7, 2022 · 2 comments
Open
3 tasks

Support refreshing of temporary tokens #14578

hexylena opened this issue Sep 7, 2022 · 2 comments
Labels
area/auth Authentication and authorization area/backend status/planning

Comments

@hexylena
Copy link
Member

hexylena commented Sep 7, 2022

We needed to support refreshing the temporary tokens that were associated with a user's OIDC login via Elixir AAI. We hacked this into place. This issue serves as documentation of the Proof of Concept we have, that we'd love to see translated to a proper implementation. Fyi for @nuwang and maybe also @poterlowicz-lab who I believe is also involved in some way.

Todos

  • Refreshing tokens should be built into galaxy via celery beat, not a cron script
  • ga4gh passports support is needed (i.e. changes to the auth library).
  • Do we need a better access method inside tools for accessing these tokens? That feels like it should be declared somewhere, like how ITs declare an API key requirement.

Refreshing

We setup a cron job that runs every 15 minutes, refreshing any user tokens:

*/15 * * * * cd /srv/galaxy/server && /srv/galaxy/venv/bin/python ~/refresh.py -c /srv/galaxy/config/galaxy.yml

The refresh.py script looks like follows

import datetime
import time
import requests
import sys

sys.path.insert(1, '/srv/galaxy/server/lib')

from galaxy.model import *  # noqa
from galaxy.model.mapping import init
from galaxy.model.orm.scripts import get_config
from jose import jwt
import jose

config = get_config(sys.argv)
db_url = config['db_url']
sa_session = init('/tmp/', db_url).context

# For all users
users = sa_session.query(User).enable_eagerloads(False).all()
for user in users:
    tokens = user.social_auth

    # For each of their tokens, refresh it IFF it's an elixir token, and we were provided a refresh token.
    for token in tokens:
        ed = token.extra_data
        if token.provider != 'elixir':
            continue
        if 'refresh_token' not in ed:
            continue

        rt = ed['refresh_token']
        now = time.mktime(datetime.utcnow().utctimetuple())

        # The verification here is not simple, but we should be doing this properly instead of accepting it. That said we can generally trust the IdP.
        try:
            at = jwt.decode(ed['access_token'], None, options={"verify_signature": False})
            exp = at['exp']
        except Exception:
            # Just assume expired
            exp = 0

        #print(f"{at['sub']} expires in {exp - now}")
        # If the token is expiring soon, we'll refresh it.
        if exp - now - 600 < 0:
            #print(exp - now - 3580)
            #print(f"Replacing {token}")
            data = {
                'grant_type': 'refresh_token',
                'client_id': 'redacted',
                'client_secret': 'redacted',
                'refresh_token': rt
            }
            r = requests.post('https://login.elixir-czech.org/oidc/token', data=data)
            token.extra_data = r.json()
            print(token)
            print(token.extra_data)
            sa_session.add(token)

sa_session.flush()

We additionally need the ga4gh passport fields that are being passed along, but that isn't included in this implementation, and, I believe may require updating social-core to support it.

Tools

Inside tools I've been doing something like:

#for token in $__user__.social_auth:
  #if $token.provider == "elixir":
    #set access_token = $token.extra_data.get('access_token', '')
    "token": "${access_token}",
  #end if
#end for

(assuming that there will only ever be a single elixir provider), and passing that along to the tool which uses it for authenticating.

I think I'd rather see something like

    <environment_variables>
        <environment_variable name="LS_TOKEN" provider="elixir" inject="access_token" />
    </environment_variables>

(elixir aai has since been renamed lifescience login or something)

@mvdbeek mvdbeek added area/auth Authentication and authorization area/backend labels Sep 7, 2022
@mvdbeek
Copy link
Member

mvdbeek commented Sep 7, 2022

Is there something particular about this being an external script that you dislike / that you foresee being a problem ? We could certainly ship the script and/or integrate it into celery beat for a more out of the box experience, but this doesn't seem like a bad way to do it.

(+1 for a nicer tool syntax)

@hexylena
Copy link
Member Author

hexylena commented Sep 7, 2022

Technically no, nothing I forsee being a problem, more that it feels like a point where we can do something more generic and less elixir specific.

It just feels a bit "off" to me. I would have expected that it should be part of our PSA authentication class somewhere, so that the celery beat task is more along the lines of "call psa refresh" which in turn processes all of the tokens it manages, and calls the social-core implemented refresh token action per token to ensure it's refreshed, allowing us to push the "ensure correct refreshing" off into social core where it's appropriate and relevant for other users of that library.

That said that's a bigger effort and a lot more coordinating than a rubbish cron job :) This works for us for now at least, for the internal proof of concept.

And agreed! Celery beat makes a lot of sense.

@hexylena hexylena mentioned this issue Jan 20, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/auth Authentication and authorization area/backend status/planning
Projects
None yet
Development

No branches or pull requests

2 participants