Skip to content

Commit 42e3b95

Browse files
authored
Merge pull request #107 from lionick/add_native_option_upstream
Redirect URI comparison for native OAuth clients (RFC 8252)
2 parents 074ea67 + 237bbea commit 42e3b95

21 files changed

+307
-111
lines changed

private/xmetadata/oidc.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from idpyoidc import metadata
66
from idpyoidc.client import metadata as client_metadata
7+
from idpyoidc.message.oidc import APPLICATION_TYPE_WEB
78
from idpyoidc.message.oidc import RegistrationRequest
89
from idpyoidc.message.oidc import RegistrationResponse
910

@@ -64,7 +65,7 @@ class Metadata(client_metadata.Metadata):
6465

6566
_supports = {
6667
"acr_values_supported": None,
67-
"application_type": "web",
68+
"application_type": APPLICATION_TYPE_WEB,
6869
"callback_uris": None,
6970
# "client_authn_methods": get_client_authn_methods,
7071
"client_id": None,

src/idpyoidc/client/claims/oidc.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from idpyoidc import metadata
66
from idpyoidc.client import claims as client_claims
77
from idpyoidc.client.claims.transform import create_registration_request
8+
from idpyoidc.message.oidc import APPLICATION_TYPE_WEB
89
from idpyoidc.message.oidc import RegistrationRequest
910
from idpyoidc.message.oidc import RegistrationResponse
1011

@@ -63,7 +64,7 @@ class Claims(client_claims.Claims):
6364

6465
_supports = {
6566
"acr_values_supported": None,
66-
"application_type": "web",
67+
"application_type": APPLICATION_TYPE_WEB,
6768
"callback_uris": None,
6869
# "client_authn_methods": get_client_authn_methods,
6970
"client_id": None,

src/idpyoidc/client/defaults.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import hashlib
22
import string
33

4+
from idpyoidc.message.oidc import APPLICATION_TYPE_WEB
5+
46
SUCCESSFUL = [200, 201, 202, 203, 204, 205, 206]
57

68
SERVICE_NAME = "OIC"
@@ -27,7 +29,7 @@
2729
}
2830

2931
DEFAULT_CLIENT_PREFERENCES = {
30-
"application_type": "web",
32+
"application_type": APPLICATION_TYPE_WEB,
3133
"response_types": [
3234
"code",
3335
"id_token",

src/idpyoidc/message/oidc/__init__.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@
4747

4848
NONCE_STORAGE_TIME = 4 * 3600
4949

50+
APPLICATION_TYPE_NATIVE = "native"
51+
APPLICATION_TYPE_WEB = "web"
52+
5053

5154
class AtHashError(VerificationError):
5255
pass
@@ -638,9 +641,9 @@ class RegistrationRequest(Message):
638641
# "organization_name": SINGLE_OPTIONAL_STRING,
639642
"response_modes": OPTIONAL_LIST_OF_STRINGS,
640643
}
641-
c_default = {"application_type": "web", "response_types": ["code"]}
644+
c_default = {"application_type": APPLICATION_TYPE_WEB, "response_types": ["code"]}
642645
c_allowed_values = {
643-
"application_type": ["native", "web"],
646+
"application_type": [APPLICATION_TYPE_NATIVE, APPLICATION_TYPE_WEB],
644647
"subject_type": ["public", "pairwise"],
645648
}
646649

src/idpyoidc/server/exception.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class UnknownAssertionType(OidcEndpointError):
6666
pass
6767

6868

69-
class RedirectURIError(OidcEndpointError):
69+
class RedirectURIError(OidcEndpointError, ValueError):
7070
pass
7171

7272

src/idpyoidc/server/oauth2/authorization.py

Lines changed: 107 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
import logging
33
from typing import List
44
from typing import Optional
5+
from typing import TypeVar
56
from typing import Union
7+
from urllib.parse import ParseResult
8+
from urllib.parse import SplitResult
9+
from urllib.parse import parse_qs
610
from urllib.parse import unquote
711
from urllib.parse import urlencode
812
from urllib.parse import urlparse
@@ -21,6 +25,8 @@
2125
from idpyoidc.message import Message
2226
from idpyoidc.message import oauth2
2327
from idpyoidc.message.oauth2 import AuthorizationRequest
28+
from idpyoidc.message.oidc import APPLICATION_TYPE_NATIVE
29+
from idpyoidc.message.oidc import APPLICATION_TYPE_WEB
2430
from idpyoidc.message.oidc import AuthorizationResponse
2531
from idpyoidc.message.oidc import verified_claim_name
2632
from idpyoidc.server.authn_event import create_authn_event
@@ -41,7 +47,9 @@
4147
from idpyoidc.time_util import utc_time_sans_frac
4248
from idpyoidc.util import importer
4349
from idpyoidc.util import rndstr
44-
from idpyoidc.util import split_uri
50+
51+
52+
ParsedURI = TypeVar('ParsedURI', ParseResult, SplitResult)
4553

4654
logger = logging.getLogger(__name__)
4755

@@ -106,80 +114,115 @@ def verify_uri(
106114
:param context: An EndpointContext instance
107115
:param request: The authorization request
108116
:param uri_type: redirect_uri or post_logout_redirect_uri
109-
:return: An error response if the redirect URI is faulty otherwise
110-
None
117+
:return: Raise an exception response if the redirect URI is faulty otherwise None
111118
"""
112-
_cid = request.get("client_id", client_id)
113119

114-
if not _cid:
115-
logger.error("No client id found")
120+
client_id = request.get("client_id") or client_id
121+
if not client_id:
122+
logger.error("No client_id provided")
116123
raise UnknownClient("No client_id provided")
117124

118-
_uri = request.get(uri_type)
119-
if _uri is None:
120-
raise ValueError(f"Wrong uri_type: {uri_type}")
125+
client_info = context.cdb.get(client_id)
126+
if not client_info:
127+
logger.error("No client info found")
128+
raise KeyError("No client info found")
121129

122-
_redirect_uri = unquote(_uri)
130+
req_redirect_uri_quoted = request.get(uri_type)
131+
if req_redirect_uri_quoted is None:
132+
raise ValueError(f"Wrong uri_type: {uri_type}")
123133

124-
part = urlparse(_redirect_uri)
125-
if part.fragment:
134+
req_redirect_uri = unquote(req_redirect_uri_quoted)
135+
req_redirect_uri_obj = urlparse(req_redirect_uri)
136+
if req_redirect_uri_obj.fragment:
126137
raise URIError("Contains fragment")
127138

128-
(_base, _query) = split_uri(_redirect_uri)
139+
# basic URL validation
140+
if not req_redirect_uri_obj.hostname:
141+
raise URIError("Invalid redirect_uri hostname")
142+
if req_redirect_uri_obj.path and not req_redirect_uri_obj.path.startswith("/"):
143+
raise URIError("Invalid redirect_uri path")
144+
try:
145+
req_redirect_uri_obj.port
146+
except ValueError as e:
147+
raise URIError(f"Invalid redirect_uri port: {str(e)}") from e
148+
149+
uri_type_property = f"{uri_type}s" if uri_type == "redirect_uri" else uri_type
150+
client_redirect_uris: list[Union[str, tuple[str, dict]]] = client_info.get(uri_type_property)
151+
if not client_redirect_uris:
152+
# an OIDC client must have registered with redirect URIs
153+
if endpoint_type == "oidc":
154+
raise RedirectURIError(f"No registered {uri_type} for {client_id}")
155+
else:
156+
return
157+
158+
# TODO move: this processing should be done during client registration/loading
159+
# TODO optimize: keep unique URIs (mayby use a set)
160+
# Pre-processing to homogenize the types of each item,
161+
# and normalize (lower-case, remove params, etc) the rediret URIs.
162+
# Each item is a tuple composed of:
163+
# - a ParseResult item, representing a URI without the query part, and
164+
# - a dict, representing a query string
165+
client_redirect_uris_obj: list[tuple[ParseResult, dict[str, list[str]]]] = [
166+
(
167+
urlparse(uri_base)._replace(query=None),
168+
(uri_qs_obj or {}),
169+
)
170+
for uri in client_redirect_uris
171+
for uri_base, uri_qs_obj in [(uri, {}) if isinstance(uri, str) else uri]
172+
]
173+
174+
# Handle redirect URIs for native clients:
175+
# When the URI is an http localhost (IPv4 or IPv6) literal, then
176+
# the port should not be taken into account when matching redirect URIs.
177+
client_type = client_info.get("application_type") or APPLICATION_TYPE_WEB
178+
if client_type == APPLICATION_TYPE_NATIVE:
179+
if is_http_uri(req_redirect_uri_obj) and is_localhost_uri(req_redirect_uri_obj):
180+
req_redirect_uri_obj = remove_port_from_uri(req_redirect_uri_obj)
181+
182+
# TODO move: this processing should be done during client registration/loading
183+
# When the URI is an http localhost (IPv4 or IPv6) literal, then
184+
# the port should not be taken into account when matching redirect URIs.
185+
_client_redirect_uris_without_port_obj = []
186+
for uri_obj, url_qs_obj in client_redirect_uris_obj:
187+
if is_http_uri(uri_obj) and is_localhost_uri(uri_obj):
188+
uri_obj = remove_port_from_uri(uri_obj)
189+
_client_redirect_uris_without_port_obj.append((uri_obj, url_qs_obj))
190+
client_redirect_uris_obj = _client_redirect_uris_without_port_obj
191+
192+
# Separate the URL from the query string object for the requested redirect URI.
193+
req_redirect_uri_query_obj = parse_qs(req_redirect_uri_obj.query)
194+
req_redirect_uri_without_query_obj = req_redirect_uri_obj._replace(query=None)
195+
196+
match = any(
197+
req_redirect_uri_without_query_obj == uri_obj
198+
and req_redirect_uri_query_obj == uri_query_obj
199+
for uri_obj, uri_query_obj in client_redirect_uris_obj
200+
)
201+
if not match:
202+
raise RedirectURIError("Doesn't match any registered uris")
203+
129204

130-
# Get the clients registered redirect uris
131-
client_info = context.cdb.get(_cid)
132-
if client_info is None:
133-
raise KeyError("No such client")
205+
def is_http_uri(uri_obj: Union[ParseResult, SplitResult]) -> bool:
206+
value = uri_obj.scheme == "http"
207+
return value
134208

135-
if uri_type == "redirect_uri":
136-
redirect_uris = client_info.get(f"{uri_type}s")
137-
else:
138-
redirect_uris = client_info.get(f"{uri_type}")
139209

140-
if redirect_uris is None:
141-
if endpoint_type == "oidc":
142-
raise RedirectURIError(f"No registered {uri_type} for {_cid}")
143-
else:
144-
match = False
145-
for _item in redirect_uris:
146-
if isinstance(_item, str):
147-
regbase = _item
148-
rquery = {}
149-
else:
150-
regbase, rquery = _item
151-
152-
# The URI MUST exactly match one of the Redirection URI
153-
if _base == regbase:
154-
# every registered query component must exist in the uri
155-
if rquery:
156-
if not _query:
157-
raise ValueError("Missing query part")
158-
159-
for key, vals in rquery.items():
160-
if key not in _query:
161-
raise ValueError('"{}" not in query part'.format(key))
162-
163-
for val in vals:
164-
if val not in _query[key]:
165-
raise ValueError("{}={} value not in query part".format(key, val))
166-
167-
# and vice versa, every query component in the uri
168-
# must be registered
169-
if _query:
170-
if not rquery:
171-
raise ValueError("No registered query part")
172-
173-
for key, vals in _query.items():
174-
if key not in rquery:
175-
raise ValueError('"{}" extra in query part'.format(key))
176-
for val in vals:
177-
if val not in rquery[key]:
178-
raise ValueError("Extra {}={} value in query part".format(key, val))
179-
match = True
180-
break
181-
if not match:
182-
raise RedirectURIError("Doesn't match any registered uris")
210+
def is_localhost_uri(uri_obj: Union[ParseResult, SplitResult]) -> bool:
211+
value = uri_obj.hostname in [
212+
"127.0.0.1",
213+
"::1",
214+
"0000:0000:0000:0000:0000:0000:0000:0001",
215+
]
216+
return value
217+
218+
219+
def remove_port_from_uri(uri_obj: ParsedURI) -> ParsedURI:
220+
if not uri_obj.port or not uri_obj.netloc:
221+
return uri_obj
222+
223+
netloc_without_port = uri_obj.netloc.rsplit(":", 1)[0]
224+
uri_without_port_obj = uri_obj._replace(netloc=netloc_without_port)
225+
return uri_without_port_obj
183226

184227

185228
def join_query(base, query):

src/idpyoidc/server/oidc/registration.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
from idpyoidc.exception import MessageException
1515
from idpyoidc.message.oauth2 import ResponseMessage
16+
from idpyoidc.message.oidc import APPLICATION_TYPE_NATIVE
17+
from idpyoidc.message.oidc import APPLICATION_TYPE_WEB
1618
from idpyoidc.message.oidc import ClientRegistrationErrorResponse
1719
from idpyoidc.message.oidc import RegistrationRequest
1820
from idpyoidc.message.oidc import RegistrationResponse
@@ -291,18 +293,18 @@ def do_client_registration(self, request, client_id, ignore=None):
291293
@staticmethod
292294
def verify_redirect_uris(registration_request):
293295
verified_redirect_uris = []
294-
client_type = registration_request.get("application_type", "web")
296+
client_type = registration_request.get("application_type") or APPLICATION_TYPE_WEB
295297

296298
must_https = False
297-
if client_type == "web":
299+
if client_type == APPLICATION_TYPE_WEB:
298300
must_https = True
299301
if registration_request.get("response_types") == ["code"]:
300302
must_https = False
301303

302304
for uri in registration_request["redirect_uris"]:
303305
_custom = False
304306
p = urlparse(uri)
305-
if client_type == "native":
307+
if client_type == APPLICATION_TYPE_NATIVE:
306308
if p.scheme not in ["http", "https"]: # Custom scheme
307309
_custom = True
308310
elif p.scheme == "http" and p.hostname in ["localhost", "127.0.0.1"]:

tests/test_06_oidc.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from idpyoidc.message.oidc import AccessTokenRequest
2828
from idpyoidc.message.oidc import AccessTokenResponse
2929
from idpyoidc.message.oidc import AddressClaim
30+
from idpyoidc.message.oidc import APPLICATION_TYPE_WEB
3031
from idpyoidc.message.oidc import AtHashError
3132
from idpyoidc.message.oidc import AuthnToken
3233
from idpyoidc.message.oidc import AuthorizationErrorResponse
@@ -549,7 +550,7 @@ def test_required_parameters_only_none_signing_alg(self):
549550
class TestRegistrationRequest(object):
550551
def test_deserialize(self):
551552
msg = {
552-
"application_type": "web",
553+
"application_type": APPLICATION_TYPE_WEB,
553554
"redirect_uris": [
554555
"https://client.example.org/callback",
555556
"https://client.example.org/callback2",
@@ -579,15 +580,15 @@ def test_registration_request(self):
579580
default_max_age=10,
580581
require_auth_time=True,
581582
default_acr="foo",
582-
application_type="web",
583+
application_type=APPLICATION_TYPE_WEB,
583584
redirect_uris=["https://example.com/authz_cb"],
584585
)
585586
assert req.verify()
586587
js = req.to_json()
587588
js_obj = json.loads(js)
588589
expected_js_obj = {
589590
"redirect_uris": ["https://example.com/authz_cb"],
590-
"application_type": "web",
591+
"application_type": APPLICATION_TYPE_WEB,
591592
"default_acr": "foo",
592593
"require_auth_time": True,
593594
"operation": "register",
@@ -624,7 +625,7 @@ def test_deser(self):
624625
default_max_age=10,
625626
require_auth_time=True,
626627
default_acr="foo",
627-
application_type="web",
628+
application_type=APPLICATION_TYPE_WEB,
628629
redirect_uris=["https://example.com/authz_cb"],
629630
)
630631
ser_req = req.serialize("urlencoded")
@@ -645,7 +646,7 @@ def test_deser_dict(self):
645646
"default_max_age": 10,
646647
"require_auth_time": True,
647648
"default_acr": "foo",
648-
"application_type": "web",
649+
"application_type": APPLICATION_TYPE_WEB,
649650
"redirect_uris": ["https://example.com/authz_cb"],
650651
}
651652

@@ -666,7 +667,7 @@ def test_deser_dict_json(self):
666667
"default_max_age": 10,
667668
"require_auth_time": True,
668669
"default_acr": "foo",
669-
"application_type": "web",
670+
"application_type": APPLICATION_TYPE_WEB,
670671
"redirect_uris": ["https://example.com/authz_cb"],
671672
}
672673

@@ -692,7 +693,7 @@ def test_deserialize(self):
692693
"registration_client_uri": "https://server.example.com/connect/register?client_id"
693694
"=s6BhdRkqt3",
694695
"token_endpoint_auth_method": "client_secret_basic",
695-
"application_type": "web",
696+
"application_type": APPLICATION_TYPE_WEB,
696697
"redirect_uris": [
697698
"https://client.example.org/callback",
698699
"https://client.example.org/callback2",

0 commit comments

Comments
 (0)