Skip to content

mbp-1043: gate AzureClientIdRequestFilter on config property - #9

Closed
p-rog wants to merge 1 commit into
validatedpatterns-demos:mainfrom
p-rog:mbp-1043
Closed

mbp-1043: gate AzureClientIdRequestFilter on config property#9
p-rog wants to merge 1 commit into
validatedpatterns-demos:mainfrom
p-rog:mbp-1043

Conversation

@p-rog

@p-rog p-rog commented Jul 17, 2026

Copy link
Copy Markdown

Problem:
The AzureClientIdRequestFilter (added in PR #8 for Entra ID support) unconditionally injects client_id into every OIDC token exchange request that contains a client_assertion. This breaks Keycloak deployments using federated-jwt client authentication — Keycloak requires client_id to match the JWT sub claim, which is the SPIFFE URI (spiffe://...), not the OIDC client ID (qtodo-app).
Result: every login attempt fails with "client_id parameter does not match sub claim" (HTTP 400).

Fix: Add a qtodo.oidc.inject-client-id config property (default false) to the filter. When false, the filter skips injection entirely. The Helm chart companion change (in layered-zero-trust) sets QTODO_OIDC_INJECT_CLIENT_ID=true only when provider == "entraid".

The filter unconditionally injects client_id into OIDC token exchange
requests containing a client_assertion. This breaks Keycloak's
federated-jwt authenticator, which requires client_id to match the
JWT sub claim (the SPIFFE URI), not the OIDC client ID.

Add a qtodo.oidc.inject-client-id config property (default false) so
the filter only activates when explicitly enabled. The Helm chart
companion change sets QTODO_OIDC_INJECT_CLIENT_ID=true only when the
OIDC provider is entraid.

Co-authored-by: Cursor <cursoragent@cursor.com>
@sabre1041

Copy link
Copy Markdown
Contributor

@p-rog with the incorporation of validatedpatterns/layered-zero-trust#160, I believe this PR can be closed

@p-rog

p-rog commented Jul 21, 2026

Copy link
Copy Markdown
Author

Closing this PR in favor of validatedpatterns/layered-zero-trust#160 patch.

@p-rog p-rog closed this Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants