[snyaggarwal]

Server-side part of OpenConceptLab/ocl_online#535.

Why

OpenIDAuthService built the Keycloak URLs with f-strings. The values come from request.query_params, which Django has already decoded, and went into the URL raw. A post_logout_redirect_uri or redirect_uri containing ?, & or # therefore split into extra parameters or, with #, lost everything after it on the redirect to Keycloak. That second hop contributed to the "Invalid redirect uri" page users hit after an expired session in the Mapper.

What changes

• A new OpenIDAuthService.build_url(endpoint, params, code_challenge, code_challenge_method) encodes every value with urlencode(..., quote_via=quote).
• get_login_redirect_url, get_registration_redirect_url, get_reset_password_redirect_url and get_logout_redirect_url use it. Parameter names and order are unchanged.
• On the wire, values are now percent-encoded, for example redirect_uri=http%3A%2F%2Flocalhost%3A4000 and scope=openid%20profile%20email. Keycloak decodes them to the same strings.

No migrations, indexing or env var changes.

Verification

• docker exec -e OIDC_SERVER_URL= -e OIDC_SERVER_INTERNAL_URL= oclapi2-api-1 python manage.py test core.services.auth core.users.tests --keepdb: 97 tests, OK.
  • The two exact-string tests now expect the encoded form, and the login test also covers the PKCE parameters.
  • The new test_redirect_urls_keep_query_values_whole passes a redirect_uri containing #, ? and &, plus & and # in state and nonce, through all four builders. It checks that each value reads back intact via parse_qs with no fragment.
  • The OIDC URLs are unset in that command because the auth tests expect empty endpoints. A container with OIDC_SERVER_URL set fails them before and after this change.
• docker exec oclapi2-api-1 pylint core/services/auth/openid.py core/services/auth/tests.py: 10.00/10.
• Not tested against a real Keycloak.

Companion PRs: OpenConceptLab/oclmap#109 and OpenConceptLab/oclweb3#67. Their values are already safe either way, so the three can be deployed in any order.

🤖 Generated with Claude Code

[snyaggarwal]

Verdict: Approve

Issue complete: yes. Centralizing Keycloak URL construction with urlencode preserves redirect_uri, post_logout_redirect_uri, state, nonce, and PKCE values across the API-to-Keycloak hop, including embedded #, ? and & characters. Existing parameter order and endpoint selection remain intact.

Security blockers: none. Encoding the decoded query values removes parameter-splitting and fragment-loss behavior without weakening redirect validation.

Verification: the focused auth suite passed locally (7 tests), including the new round-trip encoding coverage; Snyk passes. GitHub Pylint was still pending at review time, so merge should remain gated on that check.

(snyaggarwal • ReviewAutomation/GPT-5)