fix(oauth): upstream PKCE check off only when exemptions are configured (#884 follow-up) - #888
Open
keysersoft wants to merge 1 commit into
Open
keysersoft wants to merge 1 commit into
keysersoft wants to merge 1 commit into
Conversation
…nfigured @rekog/mcp-nest-auth enforces PKCE itself (requirePkce defaults to true) and has no per-client exception, so the OAUTH_PKCE_EXEMPT_CLIENT_IDS exemption from #884 passed AuthorizePkceMiddleware and was then refused upstream. When at least one exempt client is listed, upstream's check is switched off and AuthorizePkceMiddleware is the single enforcement point: S256 for every other client, exemption only for listed confidential clients. With no list, upstream keeps enforcing PKCE as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#884 is live but did not take effect:
@rekog/mcp-nest-authenforces PKCE on its own (requirePkcedefaults totrue, no per-client exception). Live check after the 755e30c deploy: the listed Copilot Studio client passed our middleware and was then redirected withinvalid_request: code_challenge is requiredby upstream.requirePkce: !pkceExemptionsConfigured()inMcpAuthModule.forRoot. With an emptyOAUTH_PKCE_EXEMPT_CLIENT_IDS(self-host default) nothing changes, and upstream still enforces PKCE.AuthorizePkceMiddlewareis the single gate on /authorize. It already requires S256 with a valid challenge shape from every client, and exempts only listed clients registered as confidential with a stored secret. Codes without a challenge can therefore only be issued to those clients. At /token, upstream still validates any challenge that was sent and still requires the client secret.pkceExemptClientIds()/pkceExemptionsConfigured()are shared by the module and the middleware, so both read the list the same way.requirePkceis false. That is expected on the cloud while the Copilot Studio client is listed.Tests: authorize-pkce spec 20/20; auth + mcp-server + connectors 1466 passed.