Merge pull request #393 from TeskaLabs/fix/parsing-request-query

Properly parse URL query before adding new parameters

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## v24.20
 
 ### Pre-releases
+- `v24.20-alpha11`
 - `v24.20-alpha10`
 - `v24.20-alpha9`
 - `v24.20-alpha8`
@@ -18,6 +19,7 @@
 - Default password criteria are more restrictive (#372, `v24.20-alpha1`, Compatible with Seacat Auth Webui v24.19-alpha and later, Seacat Account Webui v24.08-beta and later)
 
 ### Fix
+- Properly parse URL query before adding new parameters (#393, `v24.20-alpha11`)
 - Delete client cookie on introspection failure (#385, `v24.20-alpha6`)
 - Extend session expiration at cookie entrypoint (#383, `v24.20-alpha5`)
 - Do not log failed LDAP login as error (#381, `v24.20-alpha4`)

seacatauth/generic.py

@@ -264,9 +264,13 @@ def urlunparse(
 	return urllib.parse.urlunparse((scheme, netloc, path, params, query, fragment))
 
 
-def add_params_to_url_query(url, **params):
+def update_url_query_params(url: str, **params):
 	parsed = urlparse(url)
-	query = urllib.parse.parse_qs(parsed["query"])
+	query = {}
+	for k, v in urllib.parse.parse_qsl(parsed["query"]):
+		if k in query:
+			raise ValueError("Repeated query parameters ({!r}) are not supported.".format(k))
+		query[k] = v
 	query.update(params)
 	parsed["query"] = urllib.parse.urlencode(query)
 	return urlunparse(**parsed)

seacatauth/openidconnect/handler/authorize.py

@@ -889,21 +889,18 @@ def _build_login_uri(self, client_dict, login_query_params):
 		if login_uri is None:
 			login_uri = "{}{}".format(self.AuthWebuiBaseUrl, self.LoginPath)
 
-		parsed = generic.urlparse(login_uri)
-		if parsed["fragment"] != "":
+		if "#" in login_uri:
 			# If the Login URI contains fragment, add the login params into the fragment query
+			parsed = generic.urlparse(login_uri)
 			fragment_parsed = generic.urlparse(parsed["fragment"])
-			query = urllib.parse.parse_qs(fragment_parsed["query"])
+			query = dict(urllib.parse.parse_qsl(fragment_parsed["query"]))
 			query.update(login_query_params)
 			fragment_parsed["query"] = urllib.parse.urlencode(query)
 			parsed["fragment"] = generic.urlunparse(**fragment_parsed)
+			return generic.urlunparse(**parsed)
 		else:
 			# If the Login URI contains no fragment, add the login params into the regular URL query
-			query = urllib.parse.parse_qs(parsed["query"])
-			query.update(login_query_params)
-			parsed["query"] = urllib.parse.urlencode(query)
-
-		return generic.urlunparse(**parsed)
+			return generic.update_url_query_params(login_uri, **dict(login_query_params))
 
 
 	def _validate_request_parameters(self, request_parameters):

seacatauth/openidconnect/service.py

@@ -14,7 +14,7 @@
 import jwcrypto.jwk
 import jwcrypto.jws
 
-from ..generic import add_params_to_url_query
+from ..generic import update_url_query_params
 from ..session.adapter import SessionAdapter
 from .. import exceptions
 from . import pkce
@@ -407,7 +407,7 @@ def build_authorize_uri(self, client_dict: dict, **query_params):
 		authorize_uri = client_dict.get("authorize_uri")
 		if authorize_uri is None:
 			authorize_uri = "{}{}".format(self.PublicApiBaseUrl, self.AuthorizePath.lstrip("/"))
-		return add_params_to_url_query(authorize_uri, **{k: v for k, v in query_params.items() if v is not None})
+		return update_url_query_params(authorize_uri, **{k: v for k, v in query_params.items() if v is not None})
 
 
 	async def revoke_token(self, token, token_type_hint=None):