Skip to content

Commit 4a9aa1a

Browse files
Handle case where the frontend url already contains query parameters (#366)
* Handle case where the frontend url already contains query parameters * Fix #365 * Add test for new feature * Use JWT since tokens are only appended in the JWT scenario * Fix uv.lock * Parse query string and keep backward compatibility * Fix linter issues --------- Co-authored-by: Mostafa Moradian <[email protected]>
1 parent b30a236 commit 4a9aa1a

File tree

3 files changed

+610
-449
lines changed

3 files changed

+610
-449
lines changed

django_saml2_auth/tests/test_saml.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,3 +845,51 @@ def test_acs_view_use_jwt_set_inactive_user(
845845
result = acs(post_request)
846846
assert result.status_code == 500
847847
assert f"Error code: {INACTIVE_USER}" in result.content.decode()
848+
849+
@pytest.mark.django_db
850+
@responses.activate
851+
def test_acs_view_when_use_jwt_next_url_has_query_parameters(
852+
settings: SettingsWrapper,
853+
monkeypatch: "MonkeyPatch", # type: ignore # noqa: F821
854+
):
855+
"""Test Acs view when login_next_url has query parameters in the session"""
856+
responses.add(responses.GET, METADATA_URL1, body=METADATA1)
857+
settings.SAML2_AUTH = {
858+
"ASSERTION_URL": "https://api.example.com",
859+
"DEFAULT_NEXT_URL": "default_next_url",
860+
"USE_JWT": True,
861+
"JWT_SECRET": "JWT_SECRET",
862+
"JWT_ALGORITHM": "HS256",
863+
"TRIGGER": {
864+
"BEFORE_LOGIN": None,
865+
"AFTER_LOGIN": None,
866+
"GET_METADATA_AUTO_CONF_URLS": GET_METADATA_AUTO_CONF_URLS,
867+
},
868+
}
869+
post_request = RequestFactory().post(METADATA_URL1, {"SAMLResponse": "SAML RESPONSE"})
870+
871+
monkeypatch.setattr(
872+
Saml2Client, "parse_authn_request_response", mock_parse_authn_request_response
873+
)
874+
875+
created, mock_user = user.get_or_create_user(
876+
{"username": "[email protected]", "first_name": "John", "last_name": "Doe"}
877+
)
878+
879+
monkeypatch.setattr(
880+
user,
881+
"get_or_create_user",
882+
(
883+
created,
884+
mock_user,
885+
),
886+
)
887+
888+
middleware = SessionMiddleware(MagicMock())
889+
middleware.process_request(post_request)
890+
post_request.session["login_next_url"] = "/endpoint/?query=param&another=param"
891+
post_request.session.save()
892+
893+
result = acs(post_request)
894+
assert result["Location"].count("?") == 1
895+
assert result["Location"].count("&") == 2

django_saml2_auth/views.py

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,19 +177,41 @@ def acs(request: HttpRequest):
177177
# Create a new JWT token for IdP-initiated login (acs)
178178
jwt_token = create_custom_or_default_jwt(target_user)
179179
custom_token_query_trigger = dictor(saml2_auth_settings, "TRIGGER.CUSTOM_TOKEN_QUERY")
180+
query = "" # Initialize query variable
180181
if custom_token_query_trigger:
181-
query = run_hook(custom_token_query_trigger, jwt_token)
182-
else:
183-
query = f"?token={jwt_token}"
182+
query_result = run_hook(custom_token_query_trigger, jwt_token)
183+
query = query_result if query_result is not None else ""
184184

185185
# Use JWT auth to send token to frontend
186186
frontend_url = dictor(saml2_auth_settings, "FRONTEND_URL", next_url)
187187
custom_frontend_url_trigger = dictor(saml2_auth_settings, "TRIGGER.GET_CUSTOM_FRONTEND_URL")
188188
if custom_frontend_url_trigger:
189189
frontend_url = run_hook(custom_frontend_url_trigger, relay_state) # type: ignore
190190

191-
return HttpResponseRedirect(frontend_url + query)
192-
191+
# Parse the frontend URL to handle query parameters properly
192+
try:
193+
parsed_url = urlparse.urlparse(frontend_url)
194+
if not custom_token_query_trigger:
195+
# Default behavior: add JWT token to existing query parameters
196+
existing_query = urlparse.parse_qs(parsed_url.query)
197+
existing_query.setdefault("token", []).append(jwt_token)
198+
query_string = urlparse.urlencode(existing_query, doseq=True)
199+
new_parse = parsed_url._replace(query=query_string)
200+
destination_url = urlparse.urlunparse(new_parse)
201+
else:
202+
# Custom: merge custom query with existing query parameters
203+
existing_query = urlparse.parse_qs(parsed_url.query)
204+
custom_query = urlparse.parse_qs(query.lstrip("?"))
205+
existing_query.update(custom_query)
206+
query_string = urlparse.urlencode(existing_query, doseq=True)
207+
new_parse = parsed_url._replace(query=query_string)
208+
destination_url = urlparse.urlunparse(new_parse)
209+
except (ValueError, TypeError):
210+
# If URL parsing fails, fall back to simple string concatenation to
211+
# maintain backward compatibility with the old behavior
212+
destination_url = frontend_url + query
213+
214+
return HttpResponseRedirect(destination_url)
193215

194216
def redirect(redirect_url: Optional[str] = None) -> HttpResponseRedirect:
195217
"""Redirect to the redirect_url or the root page.

0 commit comments

Comments
 (0)