From b29f19e20692d0da088988b7615cd3ab1097b898 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 7 Dec 2017 15:09:00 +0100 Subject: [PATCH] add session_id for OAuth tokens allows tracking and revoking tokens for a login session --- .../versions/1cebaf56856c_session_id.py | 28 +++++++ jupyterhub/handlers/base.py | 76 ++++++++++++++++++- jupyterhub/oauth/store.py | 13 +++- jupyterhub/orm.py | 6 +- 4 files changed, 115 insertions(+), 8 deletions(-) create mode 100644 jupyterhub/alembic/versions/1cebaf56856c_session_id.py diff --git a/jupyterhub/alembic/versions/1cebaf56856c_session_id.py b/jupyterhub/alembic/versions/1cebaf56856c_session_id.py new file mode 100644 index 00000000..48f25f8e --- /dev/null +++ b/jupyterhub/alembic/versions/1cebaf56856c_session_id.py @@ -0,0 +1,28 @@ +"""Add session_id to auth tokens + +Revision ID: 1cebaf56856c +Revises: 3ec6993fe20c +Create Date: 2017-12-07 14:43:51.500740 + +""" + +# revision identifiers, used by Alembic. +revision = '1cebaf56856c' +down_revision = '3ec6993fe20c' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + +tables = ('oauth_access_tokens', 'oauth_codes') + +def upgrade(): + for table in tables: + op.add_column(table, sa.Column('session_id', sa.Unicode(255))) + + +def downgrade(): + # sqlite cannot downgrade because of limited ALTER TABLE support (no DROP COLUMN) + for table in tables: + op.drop_column(table, 'session_id') diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index aefea6fb..8be79748 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -8,6 +8,7 @@ import re from datetime import timedelta from http.client import responses from urllib.parse import urlparse, urlunparse, parse_qs, urlencode +import uuid from jinja2 import TemplateNotFound @@ -34,6 +35,9 @@ reasons = { 'error': "Failed to start your server. Please contact admin.", } +# constant, not configurable +SESSION_COOKIE_NAME = 'jupyterhub-session-id' + class BaseHandler(RequestHandler): """Base Handler class with access to common methods and properties.""" @@ -77,6 +81,7 @@ class BaseHandler(RequestHandler): @property def services(self): return self.settings.setdefault('services', {}) + @property def hub(self): return self.settings['hub'] @@ -255,10 +260,39 @@ class BaseHandler(RequestHandler): kwargs = {} if self.subdomain_host: kwargs['domain'] = self.domain + user = self.get_current_user_cookie() + session_id = self.get_session_cookie() + if session_id: + # clear session id + self.clear_cookie(SESSION_COOKIE_NAME) + + if user: + # user is logged in, clear any tokens associated with the current session + # don't clear session tokens if not logged in, + # because that could be a malicious logout request! + count = 0 + for access_token in ( + self.db.query(orm.OAuthAccessToken) + .filter(orm.OAuthAccessToken.user_id==user.id) + .filter(orm.OAuthAccessToken.session_id==session_id) + ): + self.db.delete(access_token) + count += 1 + if count: + self.log.debug("Deleted %s access tokens for %s", count, user.name) + self.db.commit() + + + # clear hub cookie self.clear_cookie(self.hub.cookie_name, path=self.hub.base_url, **kwargs) self.clear_cookie('jupyterhub-services', path=url_path_join(self.base_url, 'services')) - def _set_user_cookie(self, user, server): + def _set_cookie(self, key, value, encrypted=True, **overrides): + """Setting any cookie should go through here + + if encrypted use tornado's set_secure_cookie, + otherwise set plaintext cookies. + """ # tornado <4.2 have a bug that consider secure==True as soon as # 'secure' kwarg is passed to set_secure_cookie kwargs = { @@ -270,14 +304,45 @@ class BaseHandler(RequestHandler): kwargs['domain'] = self.domain kwargs.update(self.settings.get('cookie_options', {})) - self.log.debug("Setting cookie for %s: %s, %s", user.name, server.cookie_name, kwargs) - self.set_secure_cookie( + kwargs.update(overrides) + + if encrypted: + set_cookie = self.set_secure_cookie + else: + set_cookie = self.set_cookie + + self.log.debug("Setting cookie %s: %s", key, kwargs) + set_cookie(key, value, **kwargs) + + + def _set_user_cookie(self, user, server): + self.log.debug("Setting cookie for %s: %s", user.name, server.cookie_name) + self._set_cookie( server.cookie_name, user.cookie_id, + encrypted=True, path=server.base_url, - **kwargs ) + def get_session_cookie(self): + """Get the session id from a cookie + + Returns None if no session id is stored + """ + return self.get_cookie(SESSION_COOKIE_NAME, None) + + def set_session_cookie(self): + """Set a new session id cookie + + new session id is returned + + Session id cookie is *not* encrypted, + so other services on this domain can read it. + """ + session_id = uuid.uuid4().hex + self._set_cookie(SESSION_COOKIE_NAME, session_id, encrypted=False) + return session_id + def set_service_cookie(self, user): """set the login cookie for services""" self._set_user_cookie(user, orm.Server( @@ -300,6 +365,9 @@ class BaseHandler(RequestHandler): if self.db.query(orm.Service).filter(orm.Service.server != None).first(): self.set_service_cookie(user) + if not self.get_session_cookie(): + self.set_session_cookie() + # create and set a new cookie token for the hub if not self.get_current_user_cookie(): self.set_hub_cookie(user) diff --git a/jupyterhub/oauth/store.py b/jupyterhub/oauth/store.py index 9d23bb9b..41a2a7a8 100644 --- a/jupyterhub/oauth/store.py +++ b/jupyterhub/oauth/store.py @@ -39,15 +39,19 @@ class JupyterHubSiteAdapter(AuthorizationCodeGrantSiteAdapter): def authenticate(self, request, environ, scopes, client): handler = request.handler user = handler.get_current_user() + # ensure session_id is set + session_id = handler.get_session_cookie() + if session_id is None: + session_id = handler.set_session_cookie() if user: - return {}, user.id + return {'session_id': session_id}, user.id else: raise UserNotAuthenticated() def user_has_denied_access(self, request): # user can't deny access return False - + class HubDBMixin(object): """Mixin for connecting to the hub database""" @@ -65,7 +69,7 @@ class AccessTokenStore(HubDBMixin, oauth2.store.AccessTokenStore): :param access_token: An instance of :class:`oauth2.datatype.AccessToken`. """ - + user = self.db.query(orm.User).filter(orm.User.id == access_token.user_id).first() if user is None: raise ValueError("No user for access token: %s" % access_token.user_id) @@ -76,6 +80,7 @@ class AccessTokenStore(HubDBMixin, oauth2.store.AccessTokenStore): refresh_token=access_token.refresh_token, refresh_expires_at=access_token.refresh_expires_at, token=access_token.token, + session_id=access_token.data['session_id'], user=user, ) self.db.add(orm_access_token) @@ -110,6 +115,7 @@ class AuthCodeStore(HubDBMixin, oauth2.store.AuthCodeStore): redirect_uri=orm_code.redirect_uri, scopes=[], user_id=orm_code.user_id, + data={'session_id': orm_code.session_id}, ) @@ -126,6 +132,7 @@ class AuthCodeStore(HubDBMixin, oauth2.store.AuthCodeStore): expires_at=authorization_code.expires_at, user_id=authorization_code.user_id, redirect_uri=authorization_code.redirect_uri, + session_id=authorization_code.data.get('session_id', ''), ) self.db.add(orm_code) self.db.commit() diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 8dcf1eae..73fcdc2f 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -412,10 +412,13 @@ class OAuthAccessToken(Hashed, Base): user = relationship(User) service = None # for API-equivalence with APIToken + # the browser session id associated with a given token + session_id = Column(Unicode(255)) + # from Hashed hashed = Column(Unicode(255), unique=True) prefix = Column(Unicode(16), index=True) - + def __repr__(self): return "<{cls}('{prefix}...', user='{user}'>".format( cls=self.__class__.__name__, @@ -431,6 +434,7 @@ class OAuthCode(Base): code = Column(Unicode(36)) expires_at = Column(Integer) redirect_uri = Column(Unicode(1023)) + session_id = Column(Unicode(255)) user_id = Column(Integer, ForeignKey('users.id', ondelete='CASCADE'))