From 66cb630b86a1d66fafcc2088d3008f2e67b6430c Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 19 Apr 2017 15:34:41 +0200 Subject: [PATCH 1/4] separate OAuth access tokens from API tokens OAuth access tokens can only be used to identify users, not perform actions on their behalf, which API tokens do. Implementing OAuth scopes would allow us to achieve this limitation without separating the two items, but that would be a much bigger change, including having an OAuth "Would you like to grant permissions..." confirmation page. --- jupyterhub/apihandlers/users.py | 5 ++ jupyterhub/handlers/base.py | 28 +++++++- jupyterhub/oauth/store.py | 21 +----- jupyterhub/orm.py | 110 ++++++++++++++++++++++---------- 4 files changed, 109 insertions(+), 55 deletions(-) diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index fb16f944..78af7c97 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -20,6 +20,11 @@ class SelfAPIHandler(APIHandler): @web.authenticated def get(self): user = self.get_current_user() + if user is None: + # whoami can be accessed via oauth token + user = self.get_current_user_oauth_token() + if user is None: + raise web.HTTPError(403) self.write(json.dumps(self.user_model(user))) diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 18755363..97c3255a 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -141,13 +141,35 @@ class BaseHandler(RequestHandler): def cookie_max_age_days(self): return self.settings.get('cookie_max_age_days', None) - def get_current_user_token(self): - """get_current_user from Authorization header token""" + def get_auth_token(self): + """Get the authorization token from Authorization header""" auth_header = self.request.headers.get('Authorization', '') match = auth_header_pat.match(auth_header) if not match: return None - token = match.group(1) + return match.group(1) + + def get_current_user_oauth_token(self): + """Get the current user identified by OAuth access token + + Separate from API token because OAuth access tokens + can only be used for identifying users, + not using the API. + """ + token = self.get_auth_token() + if token is None: + return None + orm_token = orm.OAuthAccessToken.find(self.db, token) + if orm_token is None: + return None + else: + return self._user_from_orm(orm_token.user) + + def get_current_user_token(self): + """get_current_user from Authorization header token""" + token = self.get_auth_token() + if token is None: + return None orm_token = orm.APIToken.find(self.db, token) if orm_token is None: return None diff --git a/jupyterhub/oauth/store.py b/jupyterhub/oauth/store.py index 941bd6de..e864320f 100644 --- a/jupyterhub/oauth/store.py +++ b/jupyterhub/oauth/store.py @@ -6,7 +6,7 @@ implements https://python-oauth2.readthedocs.io/en/latest/store.html import threading from oauth2.datatype import Client, AccessToken, AuthorizationCode -from oauth2.error import AccessTokenNotFound, AuthCodeNotFound, ClientNotFoundError, UserNotAuthenticated +from oauth2.error import AuthCodeNotFound, ClientNotFoundError, UserNotAuthenticated from oauth2.grant import AuthorizationCodeGrant from oauth2.web import AuthorizationCodeGrantSiteAdapter import oauth2.store @@ -17,8 +17,7 @@ from sqlalchemy.orm import scoped_session from tornado.escape import url_escape from .. import orm -from jupyterhub.orm import APIToken -from ..utils import url_path_join, hash_token, compare_token +from ..utils import url_path_join, hash_token, compare_token, new_token class JupyterHubSiteAdapter(AuthorizationCodeGrantSiteAdapter): @@ -66,17 +65,6 @@ class HubDBMixin(object): class AccessTokenStore(HubDBMixin, oauth2.store.AccessTokenStore): """OAuth2 AccessTokenStore, storing data in the Hub database""" - def _access_token_from_orm(self, orm_token): - """Transform an ORM AccessToken record into an oauth2 AccessToken instance""" - return AccessToken( - client_id=orm_token.client_id, - grant_type=orm_token.grant_type, - expires_at=orm_token.expires_at, - refresh_token=orm_token.refresh_token, - refresh_expires_at=orm_token.refresh_expires_at, - user_id=orm_token.user_id, - ) - def save_token(self, access_token): """ Stores an access token in the database. @@ -86,17 +74,14 @@ class AccessTokenStore(HubDBMixin, oauth2.store.AccessTokenStore): """ user = self.db.query(orm.User).filter(orm.User.id == access_token.user_id).first() - token = user.new_api_token(access_token.token) - orm_api_token = APIToken.find(self.db, token, kind='user') - orm_access_token = orm.OAuthAccessToken( client_id=access_token.client_id, grant_type=access_token.grant_type, expires_at=access_token.expires_at, refresh_token=access_token.refresh_token, refresh_expires_at=access_token.refresh_expires_at, + token=access_token.token, user=user, - api_token=orm_api_token, ) self.db.add(orm_access_token) self.db.commit() diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 2ddf1f57..d6a0ada4 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -506,8 +506,65 @@ class Service(Base): """ return db.query(cls).filter(cls.name == name).first() +class Hashed(object): + """Mixin for tables with hashed tokens""" + prefix_length = 4 + algorithm = "sha512" + rounds = 16384 + salt_bytes = 8 + min_length = 8 -class APIToken(Base): + @property + def token(self): + raise AttributeError("token is write-only") + + @token.setter + def token(self, token): + """Store the hashed value and prefix for a token""" + self.prefix = token[:self.prefix_length] + self.hashed = hash_token(token, rounds=self.rounds, salt=self.salt_bytes, algorithm=self.algorithm) + + def match(self, token): + """Is this my token?""" + return compare_token(self.hashed, token) + + @classmethod + def check_token(cls, db, token): + """Check if a token is acceptable""" + if len(token) < cls.min_length: + raise ValueError("Tokens must be at least %i characters, got %r" % ( + cls.min_length, token) + ) + found = cls.find(db, token) + if found: + raise ValueError("Collision on token: %s..." % token[:cls.prefix_length]) + + @classmethod + def find_prefix(cls, db, token): + """Start the query for matching token. + + Returns an SQLAlchemy query already filtered by prefix-matches. + """ + prefix = token[:cls.prefix_length] + # since we can't filter on hashed values, filter on prefix + # so we aren't comparing with all tokens + return db.query(cls).filter(bindparam('prefix', prefix).startswith(cls.prefix)) + + @classmethod + def find(cls, db, token): + """Find a token object by value. + + Returns None if not found. + + `kind='user'` only returns API tokens for users + `kind='service'` only returns API tokens for services + """ + prefix_match = cls.find_prefix(db, token) + for orm_token in prefix_match: + if orm_token.match(token): + return orm_token + +class APIToken(Hashed, Base): """An API token""" __tablename__ = 'api_tokens' @@ -521,21 +578,7 @@ class APIToken(Base): id = Column(Integer, primary_key=True) hashed = Column(Unicode(1023)) - prefix = Column(Unicode(1023)) - prefix_length = 4 - algorithm = "sha512" - rounds = 16384 - salt_bytes = 8 - - @property - def token(self): - raise AttributeError("token is write-only") - - @token.setter - def token(self, token): - """Store the hashed value and prefix for a token""" - self.prefix = token[:self.prefix_length] - self.hashed = hash_token(token, rounds=self.rounds, salt=self.salt_bytes, algorithm=self.algorithm) + prefix = Column(Unicode(16)) def __repr__(self): if self.user is not None: @@ -564,10 +607,7 @@ class APIToken(Base): `kind='user'` only returns API tokens for users `kind='service'` only returns API tokens for services """ - prefix = token[:cls.prefix_length] - # since we can't filter on hashed values, filter on prefix - # so we aren't comparing with all tokens - prefix_match = db.query(cls).filter(bindparam('prefix', prefix).startswith(cls.prefix)) + prefix_match = cls.find_prefix(db, token) if kind == 'user': prefix_match = prefix_match.filter(cls.user_id != None) elif kind == 'service': @@ -578,10 +618,6 @@ class APIToken(Base): if orm_token.match(token): return orm_token - def match(self, token): - """Is this my token?""" - return compare_token(self.hashed, token) - @classmethod def new(cls, token=None, user=None, service=None): """Generate a new API token for a user or service""" @@ -591,12 +627,8 @@ class APIToken(Base): if token is None: token = new_token() else: - if len(token) < 8: - raise ValueError("Tokens must be at least 8 characters, got %r" % token) - found = APIToken.find(db, token) - if found: - raise ValueError("Collision on token: %s..." % token[:4]) - orm_token = APIToken(token=token) + cls.check_token(db, token) + orm_token = cls(token=token) if user: assert user.id is not None orm_token.user_id = user.id @@ -622,19 +654,29 @@ class GrantType(enum.Enum): refresh_token = 'refresh_token' -class OAuthAccessToken(Base): +class OAuthAccessToken(Hashed, Base): __tablename__ = 'oauth_access_tokens' id = Column(Integer, primary_key=True, autoincrement=True) client_id = Column(Unicode(1023)) grant_type = Column(Enum(GrantType), nullable=False) expires_at = Column(Integer) - refresh_token = Column(Unicode(36)) + refresh_token = Column(Unicode(64)) refresh_expires_at = Column(Integer) user_id = Column(Integer, ForeignKey('users.id', ondelete='CASCADE')) user = relationship(User) - api_token_id = Column(Integer, ForeignKey('api_tokens.id', ondelete='CASCADE')) - api_token = relationship(APIToken, backref='oauth_token') + session = None # for API-equivalence with APIToken + + # from Hashed + hashed = Column(Unicode(64)) + prefix = Column(Unicode(16)) + + def __repr__(self): + return "<{cls}('{prefix}...', user='{user}'>".format( + cls=self.__class__.__name__, + user=self.user and self.user.name, + prefix=self.prefix, + ) class OAuthCode(Base): From c3a90e080456fe69f43621975b95a136062be010 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 21 Apr 2017 14:52:23 +0200 Subject: [PATCH 2/4] raise 500 on failure to identify a user in oauth callback --- jupyterhub/services/auth.py | 2 ++ jupyterhub/singleuser.py | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 0c940c12..e85a8dd9 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -638,6 +638,8 @@ class HubOAuthCallbackHandler(HubOAuthenticated, RequestHandler): # TODO: make async (in a Thread?) token = self.hub_auth.token_for_code(code) user_model = self.hub_auth.user_for_token(token) + if user_model is None: + raise HTTPError(500, "oauth callback failed to identify a user") app_log.info("Logged-in user %s", user_model) self.hub_auth.set_cookie(self, token) next_url = self.get_argument('next', '') or self.hub_auth.base_url diff --git a/jupyterhub/singleuser.py b/jupyterhub/singleuser.py index dd8ca9a5..14df66f2 100755 --- a/jupyterhub/singleuser.py +++ b/jupyterhub/singleuser.py @@ -5,12 +5,13 @@ # Distributed under the terms of the Modified BSD License. import os +from textwrap import dedent from urllib.parse import urlparse from jinja2 import ChoiceLoader, FunctionLoader from tornado import ioloop -from textwrap import dedent +from tornado.web import HTTPError try: import notebook @@ -119,6 +120,8 @@ class OAuthCallbackHandler(HubOAuthCallbackHandler, IPythonHandler): # TODO: make async (in a Thread?) token = self.hub_auth.token_for_code(code) user_model = self.hub_auth.user_for_token(token) + if user_model is None: + raise HTTPError(500, "oauth callback failed to identify a user") self.log.info("Logged-in user %s", user_model) self.hub_auth.set_cookie(self, token) next_url = self.get_argument('next', '') or self.base_url From 6117c0b5734e763ef2ada39b1147dc71e402f72a Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 21 Apr 2017 14:52:47 +0200 Subject: [PATCH 3/4] accept OAuthAccessTokens in /authorization/token --- jupyterhub/apihandlers/auth.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jupyterhub/apihandlers/auth.py b/jupyterhub/apihandlers/auth.py index 03d8ba17..234e9f4f 100644 --- a/jupyterhub/apihandlers/auth.py +++ b/jupyterhub/apihandlers/auth.py @@ -18,6 +18,8 @@ class TokenAPIHandler(APIHandler): @token_authenticated def get(self, token): orm_token = orm.APIToken.find(self.db, token) + if orm_token is None: + orm_token = orm.OAuthAccessToken.find(self.db, token) if orm_token is None: raise web.HTTPError(404) if orm_token.user: From 574d3ba1f42379abfd8351e0ea6826bd31d3ec24 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 2 May 2017 11:10:06 +0200 Subject: [PATCH 4/4] unused imports --- jupyterhub/oauth/store.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyterhub/oauth/store.py b/jupyterhub/oauth/store.py index e864320f..1832c871 100644 --- a/jupyterhub/oauth/store.py +++ b/jupyterhub/oauth/store.py @@ -5,7 +5,7 @@ implements https://python-oauth2.readthedocs.io/en/latest/store.html import threading -from oauth2.datatype import Client, AccessToken, AuthorizationCode +from oauth2.datatype import Client, AuthorizationCode from oauth2.error import AuthCodeNotFound, ClientNotFoundError, UserNotAuthenticated from oauth2.grant import AuthorizationCodeGrant from oauth2.web import AuthorizationCodeGrantSiteAdapter @@ -17,7 +17,7 @@ from sqlalchemy.orm import scoped_session from tornado.escape import url_escape from .. import orm -from ..utils import url_path_join, hash_token, compare_token, new_token +from ..utils import url_path_join, hash_token, compare_token class JupyterHubSiteAdapter(AuthorizationCodeGrantSiteAdapter):