Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions authlib/integrations/django_oauth2/requests.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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):
Expand Down
10 changes: 10 additions & 0 deletions authlib/integrations/flask_oauth2/requests.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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):
Expand Down
1 change: 1 addition & 0 deletions authlib/oauth2/rfc6749/authorization_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 14 additions & 0 deletions authlib/oauth2/rfc6749/grants/base.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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)
Expand Down
23 changes: 21 additions & 2 deletions authlib/oauth2/rfc6749/requests.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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):
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions authlib/oauth2/rfc7636/challenge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
8 changes: 8 additions & 0 deletions tests/django/test_oauth2/test_authorization_code_grant.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 7 additions & 0 deletions tests/flask/test_oauth2/test_authorization_code_grant.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading