Merge pull request #645 from amCap1712/parameter-repeat

rfc6749: ensure request parameters are not included more than once in authorization endpoint

authlib/integrations/django_oauth2/requests.py

@@ -1,3 +1,5 @@
+from collections import defaultdict
+
 from django.http import HttpRequest
 from django.utils.functional import cached_property
 from authlib.common.encoding import json_loads
@@ -24,6 +26,15 @@ def data(self):
         data.update(self._request.POST.dict())
         return data
 
+    @cached_property
+    def datalist(self):
+        values = defaultdict(list)
+        for k in self.args:
+            values[k].extend(self.args.getlist(k))
+        for k in self.form:
+            values[k].extend(self.form.getlist(k))
+        return values
+
 
 class DjangoJsonRequest(JsonRequest):
     def __init__(self, request: HttpRequest):

authlib/integrations/flask_oauth2/requests.py

@@ -1,3 +1,6 @@
+from collections import defaultdict
+from functools import cached_property
+
 from flask.wrappers import Request
 from authlib.oauth2.rfc6749 import OAuth2Request, JsonRequest
 
@@ -19,6 +22,13 @@ def form(self):
     def data(self):
         return self._request.values
 
+    @cached_property
+    def datalist(self):
+        values = defaultdict(list)
+        for k in self.data:
+            values[k].extend(self.data.getlist(k))
+        return values
+
 
 class FlaskJsonRequest(JsonRequest):
     def __init__(self, request: Request):

authlib/oauth2/rfc6749/authorization_server.py

@@ -214,6 +214,7 @@ def get_consent_grant(self, request=None, end_user=None):
         request.user = end_user
 
         grant = self.get_authorization_grant(request)
+        grant.validate_no_multiple_request_parameter(request)
         grant.validate_consent_request()
         return grant
 

authlib/oauth2/rfc6749/grants/base.py

@@ -1,4 +1,5 @@
 from authlib.consts import default_json_headers
+from authlib.common.urls import urlparse
 from ..requests import OAuth2Request
 from ..errors import InvalidRequestError
 
@@ -136,6 +137,19 @@ def validate_authorization_redirect_uri(request: OAuth2Request, client):
                     state=request.state)
             return redirect_uri
 
+    @staticmethod
+    def validate_no_multiple_request_parameter(request: OAuth2Request):
+        """For the Authorization Endpoint, request and response parameters MUST NOT be included
+        more than once. Per `Section 3.1`_.
+
+        .. _`Section 3.1`: https://tools.ietf.org/html/rfc6749#section-3.1
+        """
+        datalist = request.datalist
+        parameters = ["response_type", "client_id", "redirect_uri", "scope", "state"]
+        for param in parameters:
+            if len(datalist.get(param, [])) > 1:
+                raise InvalidRequestError(f'Multiple "{param}" in request.', state=request.state)
+
     def validate_consent_request(self):
         redirect_uri = self.validate_authorization_request()
         self.execute_hook('after_validate_consent_request', redirect_uri)

authlib/oauth2/rfc6749/requests.py

@@ -1,3 +1,6 @@
+from collections import defaultdict
+from typing import DefaultDict
+
 from authlib.common.encoding import json_loads
 from authlib.common.urls import urlparse, url_decode
 from .errors import InsecureTransportError
@@ -20,10 +23,13 @@ def __init__(self, method: str, uri: str, body=None, headers=None):
         self.refresh_token = None
         self.credential = None
 
+        self._parsed_query = None
+
     @property
     def args(self):
-        query = urlparse.urlparse(self.uri).query
-        return dict(url_decode(query))
+        if self._parsed_query is None:
+            self._parsed_query = url_decode(urlparse.urlparse(self.uri).query)
+        return dict(self._parsed_query)
 
     @property
     def form(self):
@@ -36,6 +42,19 @@ def data(self):
         data.update(self.form)
         return data
 
+    @property
+    def datalist(self) -> DefaultDict[str, list]:
+        """ Return all the data in query parameters and the body of the request as a dictionary with all the values
+        in lists. """
+        if self._parsed_query is None:
+            self._parsed_query = url_decode(urlparse.urlparse(self.uri).query)
+        values = defaultdict(list)
+        for k, v in self._parsed_query:
+            values[k].append(v)
+        for k, v in self.form.items():
+            values[k].append(v)
+        return values
+
     @property
     def client_id(self) -> str:
         """The authorization server issues the registered client a client

authlib/oauth2/rfc7636/challenge.py

@@ -77,12 +77,18 @@ def validate_code_challenge(self, grant):
         if not challenge:
             raise InvalidRequestError('Missing "code_challenge"')
 
+        if len(request.datalist.get('code_challenge', [])) > 1:
+            raise InvalidRequestError('Multiple "code_challenge" in request.')
+
         if not CODE_CHALLENGE_PATTERN.match(challenge):
             raise InvalidRequestError('Invalid "code_challenge"')
 
         if method and method not in self.SUPPORTED_CODE_CHALLENGE_METHOD:
             raise InvalidRequestError('Unsupported "code_challenge_method"')
 
+        if len(request.datalist.get('code_challenge_method', [])) > 1:
+            raise InvalidRequestError('Multiple "code_challenge_method" in request.')
+
     def validate_code_verifier(self, grant):
         request: OAuth2Request = grant.request
         verifier = request.form.get('code_verifier')

tests/django/test_oauth2/test_authorization_code_grant.py

@@ -68,6 +68,14 @@ def test_get_consent_grant_client(self):
             request
         )
 
+        url = '/authorize?response_type=code&client_id=client&scope=profile&state=bar&redirect_uri=https%3A%2F%2Fa.b&response_type=code'
+        request = self.factory.get(url)
+        self.assertRaises(
+            errors.InvalidRequestError,
+            server.get_consent_grant,
+            request
+        )
+
     def test_get_consent_grant_redirect_uri(self):
         server = self.create_server()
         self.prepare_data()

tests/flask/test_oauth2/test_authorization_code_grant.py

@@ -210,6 +210,13 @@ def test_authorize_token_has_refresh_token(self):
         self.assertIn('access_token', resp)
         self.assertIn('refresh_token', resp)
 
+    def test_invalid_multiple_request_parameters(self):
+        self.prepare_data()
+        url = self.authorize_url + '&scope=profile&state=bar&redirect_uri=https%3A%2F%2Fa.b&response_type=code'
+        rv = self.client.get(url)
+        self.assertIn(b'invalid_request', rv.data)
+        self.assertIn(b'Multiple+%22response_type%22+in+request.', rv.data)
+
     def test_client_secret_post(self):
         self.app.config.update({'OAUTH2_REFRESH_TOKEN_GENERATOR': True})
         self.prepare_data(