From dda3762b485fefd7d2474fcebae1fd81cd6b19c7 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 7 Jun 2017 15:30:12 +0200 Subject: [PATCH] raise 403 on disallowed user, rather than redirect to login url raise UserNotAllowed exception in generic `check_hub_user` when a user or service is identified and not allowed. turn it into `HTTPError(403)` in tornado `get_current_user` wrapper, caching `None` so that subsequent calls don't re-trigger the same error. --- jupyterhub/log.py | 4 ++-- jupyterhub/services/auth.py | 31 +++++++++++++++++++++++------ jupyterhub/tests/test_singleuser.py | 7 +++++++ 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/jupyterhub/log.py b/jupyterhub/log.py index 0f2c2fb2..60b6288d 100644 --- a/jupyterhub/log.py +++ b/jupyterhub/log.py @@ -6,7 +6,7 @@ import json import traceback from tornado.log import LogFormatter, access_log -from tornado.web import StaticFileHandler +from tornado.web import StaticFileHandler, HTTPError def coroutine_traceback(typ, value, tb): @@ -88,7 +88,7 @@ def log_request(handler): try: user = handler.get_current_user() - except web.HTTPError: + except HTTPError: username = '' else: if user is None: diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index e85a8dd9..2be60ad6 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -7,7 +7,6 @@ HubAuth can be used in any application, even outside tornado. HubAuthenticated is a mixin class for tornado handlers that should authenticate with the Hub. """ -import json import os import re import socket @@ -492,7 +491,19 @@ class HubOAuth(HubAuth): def clear_cookie(self, handler): """Clear the OAuth cookie""" handler.clear_cookie(self.cookie_name, path=self.base_url) - + + +class UserNotAllowed(Exception): + """Exception raised when a user is identified and not allowed""" + def __init__(self, model): + self.model = model + + def __str__(self): + return '<{cls} {kind}={name}>'.format( + cls=self.__class__.__name__, + kind=self.model['kind'], + name=self.model['name'], + ) class HubAuthenticated(object): @@ -568,7 +579,7 @@ class HubAuthenticated(object): """ name = model['name'] - kind = model.get('kind', 'user') + kind = model.setdefault('kind', 'user') if self.allow_all: app_log.debug("Allowing Hub %s %s (all Hub users and services allowed)", kind, name) return model @@ -584,7 +595,7 @@ class HubAuthenticated(object): return model else: app_log.warning("Not allowing Hub service %s", name) - return None + raise UserNotAllowed(model) if self.hub_users and name in self.hub_users: # user in whitelist @@ -597,7 +608,7 @@ class HubAuthenticated(object): return model else: app_log.warning("Not allowing Hub user %s", name) - return None + raise UserNotAllowed(model) def get_current_user(self): """Tornado's authentication method @@ -611,7 +622,15 @@ class HubAuthenticated(object): if not user_model: self._hub_auth_user_cache = None return - self._hub_auth_user_cache = self.check_hub_user(user_model) + try: + self._hub_auth_user_cache = self.check_hub_user(user_model) + except UserNotAllowed as e: + # cache None, in case get_user is called again while processing the error + self._hub_auth_user_cache = None + raise HTTPError(403, "{kind} {name} is not allowed.".format(**e.model)) + except Exception: + self._hub_auth_user_cache = None + raise return self._hub_auth_user_cache diff --git a/jupyterhub/tests/test_singleuser.py b/jupyterhub/tests/test_singleuser.py index cede2e70..064ae94b 100644 --- a/jupyterhub/tests/test_singleuser.py +++ b/jupyterhub/tests/test_singleuser.py @@ -37,6 +37,13 @@ def test_singleuser_auth(app, io_loop): r = requests.get(url_path_join(url, 'logout'), cookies=cookies) assert len(r.cookies) == 0 + # another user accessing should get 403, not redirect to login + cookies = app.login_user('burgess') + r = requests.get(url, cookies=cookies) + assert r.status_code == 403 + print(r.text) + assert r.text == '' + def test_disable_user_config(app, io_loop): # use StubSingleUserSpawner to launch a single-user app in a thread