Skip to content

Commit

Permalink
Don't delete all oauth clients on startup
Browse files Browse the repository at this point in the history
When an oauth client changes, we delete all the tokens
associated with that client. This invalidates all user sessions
for that oauth client, and the oauth client's users will need to
go through the OAuth workflow again after the cache period (specified
by cache_max_age in HubAuth, 5min by default). This is fine in theory,
since oauth client information doesn't change frequently.

However, we were deleting and re-adding all oauth clients each time
the hub started! This was unnecessary, since the data was going to
be the same 99% of the time. Rest of the time, we should just update,
preventing unnecessary churn.

This PR does that.

Ref yuvipanda/jupyterhub-configurator#2
Ref berkeley-dsep-infra/datahub#2284
  • Loading branch information
yuvipanda committed Apr 8, 2021
1 parent 9eeb841 commit 054c7f2
Showing 1 changed file with 16 additions and 12 deletions.
28 changes: 16 additions & 12 deletions jupyterhub/oauth/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,19 +558,23 @@ def add_client(self, client_id, client_secret, redirect_uri, description=''):
hash its client_secret before putting it in the database.
"""
# clear existing clients with same ID
for orm_client in self.db.query(orm.OAuthClient).filter_by(
identifier=client_id
):
self.db.delete(orm_client)
self.db.commit()

orm_client = orm.OAuthClient(
identifier=client_id,
secret=hash_token(client_secret),
redirect_uri=redirect_uri,
description=description,
# Update client if it already exists, else create it
# Sqlalchemy doesn't have a good db agnostic UPSERT,
# so we do this manually. It's protected inside a
# transaction, so should fail if there are multiple
# rows with the same identifier.
# TODO: Is this safe?
orm_client = (
self.db.query(orm.OAuthClient).filter_by(identifier=client_id).one_or_none()
)
if orm_client is None:
orm_client = orm.OAuthClient(
identifier=client_id,
)

orm_client.secret = hash_token(client_secret)
orm_client.redirect_uri = redirect_uri
orm_client.description = description
self.db.add(orm_client)
self.db.commit()

Expand Down

0 comments on commit 054c7f2

Please sign in to comment.