diff --git a/jupyterhub/apihandlers/auth.py b/jupyterhub/apihandlers/auth.py index 522b2baa..891dc092 100644 --- a/jupyterhub/apihandlers/auth.py +++ b/jupyterhub/apihandlers/auth.py @@ -104,7 +104,7 @@ class CookieAPIHandler(APIHandler): self.write(json.dumps(self.user_model(user))) -class OAuthHandler(BaseHandler): +class OAuthHandler: def extract_oauth_params(self): """extract oauthlib params from a request @@ -230,13 +230,11 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): # Errors that should be shown to the user on the provider website except oauth2.FatalClientError as e: - # TODO: human error page - raise - # return response_from_error(e) + raise web.HTTPError(e.status_code, e.description) # Errors embedded in the redirect URI back to the client except oauth2.OAuth2Error as e: - self.log.error("oauth error: %s" % e) + self.log.error("OAuth error: %s", e.description) self.redirect(e.in_uri(e.redirect_uri)) @web.authenticated @@ -259,21 +257,23 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler): uri, http_method, body, headers, scopes, credentials, ) except oauth2.FatalClientError as e: - # TODO: human error page - raise + raise web.HTTPError(e.status_code, e.description) else: self.send_oauth_response(headers, body, status) class OAuthTokenHandler(OAuthHandler, APIHandler): - def post(self): uri, http_method, body, headers = self.extract_oauth_params() credentials = {} - headers, body, status = self.oauth_provider.create_token_response( - uri, http_method, body, headers, credentials) - self.send_oauth_response(headers, body, status) + try: + headers, body, status = self.oauth_provider.create_token_response( + uri, http_method, body, headers, credentials) + except oauth2.FatalClientError as e: + raise web.HTTPError(e.status_code, e.description) + else: + self.send_oauth_response(headers, body, status) default_handlers = [ diff --git a/jupyterhub/oauth/provider.py b/jupyterhub/oauth/provider.py index ac2922d8..eafe1825 100644 --- a/jupyterhub/oauth/provider.py +++ b/jupyterhub/oauth/provider.py @@ -4,6 +4,7 @@ implements https://oauthlib.readthedocs.io/en/latest/oauth2/server.html """ from datetime import datetime +from urllib.parse import urlparse from oauthlib.oauth2 import RequestValidator, WebApplicationServer @@ -98,22 +99,15 @@ class JupyterHubRequestValidator(RequestValidator): Method is used by: - Authorization Code Grant (during token request) """ + # TODO: record redirect_uri used during oauth + # if we ever support multiple destinations app_log.debug("confirm_redirect_uri: client_id=%s, redirect_uri=%s", client_id, redirect_uri, ) - orm_client = ( - self.db - .query(orm.OAuthClient) - .filter_by(identifier=client_id) - .first() - ) - if orm_client is None: - app_log.warning("No such oauth client %s", client_id) - return False - if redirect_uri == orm_client.redirect_uri: + if redirect_uri == client.redirect_uri: return True else: - app_log.warning("Redirect uri %s != %s", redirect_uri, orm_client.redirect_uri) + app_log.warning("Redirect uri %s != %s", redirect_uri, client.redirect_uri) return False def get_default_redirect_uri(self, client_id, request, *args, **kwargs): @@ -465,7 +459,7 @@ class JupyterHubRequestValidator(RequestValidator): - Client Credentials Grant - Refresh Token Grant """ - return True + return grant_type == 'authorization_code' def validate_redirect_uri(self, client_id, redirect_uri, request, *args, **kwargs): """Ensure client is authorized to redirect to the redirect_uri requested. @@ -479,8 +473,34 @@ class JupyterHubRequestValidator(RequestValidator): - Authorization Code Grant - Implicit Grant """ - return True - raise NotImplementedError('Subclasses must implement this method.') + app_log.debug("validate_redirect_uri: client_id=%s, redirect_uri=%s", + client_id, redirect_uri, + ) + orm_client = ( + self.db + .query(orm.OAuthClient) + .filter_by(identifier=client_id) + .first() + ) + if orm_client is None: + app_log.warning("No such oauth client %s", client_id) + return False + if '://' in redirect_uri and '://' not in orm_client.redirect_uri: + # default internal "/path/only" redirect uri + # confirm it matches our Host header and protocol of Referer + expected = "{}://{}{}".format( + urlparse(request.headers.get('Referer', '')).scheme, + request.headers.get('Host', '[missing Host]'), + orm_client.redirect_uri, + ) + else: + expected = orm_client.redirect_uri + + if redirect_uri == expected: + return True + else: + app_log.warning("Redirect uri %s != %s", redirect_uri, orm_client.redirect_uri) + return False def validate_refresh_token(self, refresh_token, client, request, *args, **kwargs): """Ensure the Bearer token is valid and authorized access to scopes. diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 476ac936..3fa4f70d 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -302,7 +302,15 @@ class HubAuth(SingletonConfigurable): elif r.status_code >= 400: app_log.warning("Failed to check authorization: [%i] %s", r.status_code, r.reason) app_log.warning(r.text) - raise HTTPError(500, "Failed to check authorization") + msg = "Failed to check authorization" + # pass on error_description from oauth failure + try: + description = r.json().get("error_description") + except Exception: + pass + else: + msg += ": " + description + raise HTTPError(500, msg) else: data = r.json() @@ -847,6 +855,11 @@ class HubOAuthCallbackHandler(HubOAuthenticated, RequestHandler): @coroutine def get(self): + error = self.get_argument("error", False) + if error: + msg = self.get_argument("error_description", error) + raise HTTPError(400, "Error in oauth: %s" % msg) + code = self.get_argument("code", False) if not code: raise HTTPError(400, "oauth callback made without a token") diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py index 18c91e0d..d0f420bf 100644 --- a/jupyterhub/tests/test_pages.py +++ b/jupyterhub/tests/test_pages.py @@ -573,6 +573,21 @@ def test_announcements(app, announcements): assert_announcement("logout", r.text) +@pytest.mark.parametrize( + "params", + [ + "", + "redirect_uri=/noexist", + "redirect_uri=ok&client_id=nosuchthing", + ] +) +@pytest.mark.gen_test +def test_bad_oauth_get(app, params): + cookies = yield app.login_user("authorizer") + r = yield get_page("hub/api/oauth2/authorize?" + params, app, hub=False, cookies=cookies) + assert r.status_code == 400 + + @pytest.mark.gen_test def test_server_not_running_api_request(app): cookies = yield app.login_user("bees")