better error messages on oauth errors

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 web.HTTPError(e.status_code, e.description)
-            raise
-            # return response_from_error(e)
 
         # 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,20 +257,22 @@ class OAuthAuthorizeHandler(OAuthHandler, BaseHandler):
                 uri, http_method, body, headers, scopes, credentials,
             )
         except oauth2.FatalClientError as e:
-            # TODO: human error page
+            raise web.HTTPError(e.status_code, e.description)
-            raise
         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 = {}
 
+        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)
 
 

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 = (
+        if redirect_uri == client.redirect_uri:
-            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:
             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
         """
+        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
-        raise NotImplementedError('Subclasses must implement this method.')
+        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.

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")

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")