From 11f7935155587f1f92632f2bbe07159ddc8c3945 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 18 May 2022 20:45:58 +0200 Subject: [PATCH] providers/oauth2: use regex to check redirect URI Signed-off-by: Jens Langhammer #2799 --- authentik/providers/oauth2/views/authorize.py | 14 +++----------- authentik/providers/oauth2/views/token.py | 12 ++++-------- website/docs/releases/v2022.5.md | 4 ++++ 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/authentik/providers/oauth2/views/authorize.py b/authentik/providers/oauth2/views/authorize.py index f8581c54f..e55349663 100644 --- a/authentik/providers/oauth2/views/authorize.py +++ b/authentik/providers/oauth2/views/authorize.py @@ -1,6 +1,7 @@ """authentik OAuth2 Authorization views""" from dataclasses import dataclass, field from datetime import timedelta +from re import fullmatch from typing import Optional from urllib.parse import parse_qs, urlencode, urlparse, urlsplit, urlunsplit from uuid import uuid4 @@ -173,10 +174,7 @@ class OAuthAuthorizationParams: def check_redirect_uri(self): """Redirect URI validation.""" allowed_redirect_urls = self.provider.redirect_uris.split() - # We don't want to actually lowercase the final URL we redirect to, - # we only lowercase it for comparison - redirect_uri = self.redirect_uri.lower() - if not redirect_uri: + if not self.redirect_uri: LOGGER.warning("Missing redirect uri.") raise RedirectUriError("", allowed_redirect_urls) @@ -186,13 +184,7 @@ class OAuthAuthorizationParams: self.provider.save() allowed_redirect_urls = self.provider.redirect_uris.split() - if self.provider.redirect_uris == "*": - LOGGER.warning( - "Provider has wildcard allowed redirect_uri set, allowing all.", - allow=self.redirect_uri, - ) - return - if redirect_uri not in [x.lower() for x in allowed_redirect_urls]: + if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): LOGGER.warning( "Invalid redirect uri", redirect_uri=self.redirect_uri, diff --git a/authentik/providers/oauth2/views/token.py b/authentik/providers/oauth2/views/token.py index 6b8e3b7b4..689f0e32d 100644 --- a/authentik/providers/oauth2/views/token.py +++ b/authentik/providers/oauth2/views/token.py @@ -2,6 +2,7 @@ from base64 import urlsafe_b64encode from dataclasses import InitVar, dataclass from hashlib import sha256 +from re import fullmatch from typing import Any, Optional from django.http import HttpRequest, HttpResponse @@ -146,18 +147,13 @@ class TokenParams: raise TokenError("invalid_grant") allowed_redirect_urls = self.provider.redirect_uris.split() - if self.provider.redirect_uris == "*": - LOGGER.warning( - "Provider has wildcard allowed redirect_uri set, allowing all.", - redirect=self.redirect_uri, - ) # At this point, no provider should have a blank redirect_uri, in case they do # this will check an empty array and raise an error - elif self.redirect_uri not in [x.lower() for x in allowed_redirect_urls]: + if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): LOGGER.warning( "Invalid redirect uri", - redirect=self.redirect_uri, - expected=self.provider.redirect_uris.split(), + redirect_uri=self.redirect_uri, + excepted=allowed_redirect_urls, ) raise TokenError("invalid_client") diff --git a/website/docs/releases/v2022.5.md b/website/docs/releases/v2022.5.md index ddc2f30eb..15a9f278a 100644 --- a/website/docs/releases/v2022.5.md +++ b/website/docs/releases/v2022.5.md @@ -9,6 +9,10 @@ slug: "2022.5" This requires some reconfiguration on both Twitter's and authentik's side. Check out the new Twitter integration docs [here](../../integrations/sources/twitter/) +- OAuth Provider: Redirect URIs are now checked using regular expressions + + Allowed Redirect URIs now accepts regular expressions to check redirect URIs to support wildcards. In most cases this will not change anything, however casing is also important now. Meaning if your redirect URI is "https://Foo.bar" and allowed is "https://foo.bar", authorization will not be allowed. Additionally, the special handling when _Redirect URIs/Origins_ is set to `*` has been removed. To get the same behaviour, set _Redirect URIs/Origins_ to `.+`. + ## New features - LDAP Outpost cached binding