oauth: Add the code param to the oauthrize.html template (PROJQUAY-3648…

…) (#1362)

Fixes an issue where the code param is not passed to the app redirect
URI if the user has not authorized the app before

endpoints/web.py

@@ -642,11 +642,27 @@ def authorize_application():
 
     provider = FlaskAuthorizationProvider()
     redirect_uri = request.form.get("redirect_uri", None)
+    response_type = request.form.get("response_type", "code")
     scope = request.form.get("scope", None)
     state = request.form.get("state", None)
 
     # Add the access token.
-    return provider.get_token_response("token", client_id, redirect_uri, scope=scope, state=state)
+    if response_type == "token":
+        return provider.get_token_response(
+            response_type,
+            client_id,
+            redirect_uri,
+            scope=scope,
+            state=state,
+        )
+    else:
+        return provider.get_authorization_code(
+            response_type,
+            client_id,
+            redirect_uri,
+            scope=scope,
+            state=state,
+        )
 
 
 @web.route(app.config["LOCAL_OAUTH_HANDLER"], methods=["GET"])
@@ -732,6 +748,7 @@ def request_authorization_code():
             has_dangerous_scopes=has_dangerous_scopes,
             application=oauth_app_view,
             enumerate=enumerate,
+            response_type=response_type,
             client_id=client_id,
             redirect_uri=redirect_uri,
             scope=scope,
@@ -741,11 +758,19 @@ def request_authorization_code():
 
     if response_type == "token":
         return provider.get_token_response(
-            response_type, client_id, redirect_uri, scope=scope, state=state
+            response_type,
+            client_id,
+            redirect_uri,
+            scope=scope,
+            state=state,
         )
     else:
         return provider.get_authorization_code(
-            response_type, client_id, redirect_uri, scope=scope, state=state
+            response_type,
+            client_id,
+            redirect_uri,
+            scope=scope,
+            state=state,
         )
 
 

oauth/provider.py

@@ -216,22 +216,21 @@ def get_authorization_code(self, response_type, client_id, redirect_uri, **param
             err = "unsupported_response_type"
             return self._make_redirect_error_response(redirect_uri, err)
 
+        # Check conditions
+        is_valid_client_id = self.validate_client_id(client_id)
+        if not is_valid_client_id:
+            err = "unauthorized_client"
+            return self._make_redirect_error_response(redirect_uri, err)
+
         # Check redirect URI
         is_valid_redirect_uri = self.validate_redirect_uri(client_id, redirect_uri)
         if not is_valid_redirect_uri:
             return self._invalid_redirect_uri_response()
 
-        # Check conditions
-        is_valid_client_id = self.validate_client_id(client_id)
         is_valid_access = self.validate_access()
         scope = params.get("scope", "")
         is_valid_scope = self.validate_scope(client_id, scope)
-
         # Return proper error responses on invalid conditions
-        if not is_valid_client_id:
-            err = "unauthorized_client"
-            return self._make_redirect_error_response(redirect_uri, err)
-
         if not is_valid_access:
             err = "access_denied"
             return self._make_redirect_error_response(redirect_uri, err)

requirements-dev.txt

@@ -13,6 +13,7 @@ ipython
 mock==3.0.5
 mypy==0.910
 moto==2.0.1
+parameterized==0.8.1
 pytest
 pytest-cov
 pytest-flask

templates/oauthorize.html

@@ -66,6 +66,7 @@ <h4 class="panel-title">
       <button type="button" class="btn btn-warning" onclick="$('#confirmAuthorizeModal').modal()">Authorize Application</button>
       {% else %}
       <form method="post" action="/oauth/authorizeapp">
+        <input type="hidden" name="response_type" value="{{ response_type }}">
         <input type="hidden" name="client_id" value="{{ client_id }}">
         <input type="hidden" name="redirect_uri" value="{{ redirect_uri }}">
         <input type="hidden" name="scope" value="{{ scope }}">

test/test_endpoints.py

@@ -6,6 +6,8 @@
 import base64
 import zlib
 
+from parameterized import parameterized, parameterized_class
+
 from mock import patch
 from io import BytesIO
 from urllib.parse import urlencode
@@ -59,7 +61,6 @@
     # This blueprint was already registered
     pass
 
-
 CSRF_TOKEN_KEY = "_csrf_token"
 CSRF_TOKEN = "123csrfforme"
 
@@ -396,22 +397,26 @@ def test_redirect_to_namespace(self):
 
 
 class OAuthTestCase(EndpointTestCase):
-    def test_authorize_nologin(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_nologin(self, response_type):
         form = {
             "client_id": "someclient",
             "redirect_uri": "http://localhost:5000/foobar",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         self.postResponse("web.authorize_application", form=form, with_csrf=True, expected_code=401)
 
-    def test_authorize_invalidclient(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_invalidclient(self, response_type):
         self.login("devtable", "password")
 
         form = {
             "client_id": "someclient",
             "redirect_uri": "http://localhost:5000/foobar",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         resp = self.postResponse(
@@ -421,13 +426,15 @@ def test_authorize_invalidclient(self):
             "http://localhost:5000/foobar?error=unauthorized_client", resp.headers["Location"]
         )
 
-    def test_authorize_invalidscope(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_invalidscope(self, response_type):
         self.login("devtable", "password")
 
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://localhost:8000/o2c.html",
             "scope": "invalid:scope",
+            "response_type": response_type,
         }
 
         resp = self.postResponse(
@@ -437,53 +444,62 @@ def test_authorize_invalidscope(self):
             "http://localhost:8000/o2c.html?error=invalid_scope", resp.headers["Location"]
         )
 
-    def test_authorize_invalidredirecturi(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_invalidredirecturi(self, response_type):
         self.login("devtable", "password")
 
         # Note: Defined in initdb.py
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://some/invalid/uri",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         self.postResponse("web.authorize_application", form=form, with_csrf=True, expected_code=400)
 
-    def test_authorize_success(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_success(self, response_type):
         self.login("devtable", "password")
 
         # Note: Defined in initdb.py
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://localhost:8000/o2c.html",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         resp = self.postResponse(
             "web.authorize_application", form=form, with_csrf=True, expected_code=302
         )
-        self.assertTrue("access_token=" in resp.headers["Location"])
+        expected_value = "access_token=" if response_type == "token" else "code="
+        self.assertTrue(expected_value in resp.headers["Location"])
 
-    def test_authorize_nocsrf(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_nocsrf(self, response_type):
         self.login("devtable", "password")
 
         # Note: Defined in initdb.py
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://localhost:8000/o2c.html",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         self.postResponse(
             "web.authorize_application", form=form, with_csrf=False, expected_code=403
         )
 
-    def test_authorize_nocsrf_withinvalidheader(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_nocsrf_withinvalidheader(self, response_type):
         # Note: Defined in initdb.py
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://localhost:8000/o2c.html",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         headers = dict(authorization="Some random header")
@@ -495,12 +511,14 @@ def test_authorize_nocsrf_withinvalidheader(self):
             expected_code=401,
         )
 
-    def test_authorize_nocsrf_withbadheader(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_nocsrf_withbadheader(self, response_type):
         # Note: Defined in initdb.py
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://localhost:8000/o2c.html",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         headers = dict(authorization=gen_basic_auth("devtable", "invalidpassword"))
@@ -512,12 +530,14 @@ def test_authorize_nocsrf_withbadheader(self):
             expected_code=401,
         )
 
-    def test_authorize_nocsrf_correctheader(self):
+    @parameterized.expand(["token", "code"])
+    def test_authorize_nocsrf_correctheader(self, response_type):
         # Note: Defined in initdb.py
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://localhost:8000/o2c.html",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         # Try without the client id being in the whitelist.
@@ -541,14 +561,21 @@ def test_authorize_nocsrf_correctheader(self):
             with_csrf=False,
             expected_code=302,
         )
-        self.assertTrue("access_token=" in resp.headers["Location"])
 
-    def test_authorize_nocsrf_ratelimiting(self):
+        # Reset app config
+        app.config["DIRECT_OAUTH_CLIENTID_WHITELIST"] = []
+
+        expected_value = "access_token=" if response_type == "token" else "code="
+        self.assertTrue(expected_value in resp.headers["Location"])
+
+    @parameterized.expand(["token", "code"])
+    def test_authorize_nocsrf_ratelimiting(self, response_type):
         # Note: Defined in initdb.py
         form = {
             "client_id": "deadbeef",
             "redirect_uri": "http://localhost:8000/o2c.html",
             "scope": "user:admin",
+            "response_type": response_type,
         }
 
         # Try without the client id being in the whitelist a few times, making sure we eventually get rate limited.