fix(openid-connect): token introspection with client_secret_basic sends credentials in both header and body#13524
Draft
nic-6443 wants to merge 1 commit into
Draft
Conversation
…d body during introspection
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
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.
Description
When
introspection_endpoint_auth_methodisclient_secret_basic(the default), the token introspection request sends the client credentials twice: once in theAuthorization: Basicheader and once more asclient_id/client_secretin the POST body. RFC 6749 Section 2.3.1 says the client must not use more than one authentication mechanism per request, so strict authorization servers reject the introspection call withinvalid_clientand every bearer-token request through the route fails with 401 — with the default plugin config.The root cause is in lua-resty-openidc 1.8.0:
introspect()unconditionally copiesclient_id/client_secretinto the request body, andcall_token_endpoint()then adds the Basic header forclient_secret_basicwithout removing the body copies. There is an upstream issue (zmartzone/lua-resty-openidc#556), but it has been open for a while with no release containing a fix, so this patches it on the APISIX side without forking the library.The fix extends the existing
http_request_decoratorhook usage in the plugin's introspection path: when the auth method isclient_secret_basic, the decorator stripsclient_id/client_secretfrom the urlencoded request body before the request is sent. Scoping notes:openidc.introspect()and cleared right after it returns (existing behavior), so it only affects the introspection call, not the token-endpoint calls of the authorization code flow.openidc.introspect(), the decorator can also see a body-less discovery GET (whenintrospection_endpointis not set explicitly), hence thereq.bodyguard.client_secret_post,private_key_jwtandclient_secret_jwtare untouched: stripping only happens forclient_secret_basic, where the credentials are already carried by the Basic header.The new test file
t/plugin/openid-connect11.tuses a mock introspection endpoint that behaves like a strict authorization server (rejects requests carrying credentials in both the header and the body) and coversclient_secret_basic(default and explicit, with and withoutintrospection_addon_headers) as well asclient_secret_post, which must keep sending body credentials.Which issue(s) this PR fixes:
Fixes #13085
Checklist