feat(auth): Allow ForceIntrospection, pass optional Client ID/Secret on Introspect - #3831
feat(auth): Allow ForceIntrospection, pass optional Client ID/Secret on Introspect#3831danielmustafa wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a forceIntrospection configuration option to the generic authentication service, allowing tokens to be validated via the introspection endpoint even if they are JWT-shaped. It also adds support for HTTP Basic Authentication on the introspection endpoint using environment variables. Feedback on these changes suggests refining the environment variable validation logic to prevent redundant warning logs when both variables are unset, and correcting a typo in a test function name.
| if introspectionClientID == "" || introspectionClientSecret == "" { | ||
| log.Printf("WARNING: Introspection client ID or secret not set in environment variables %s and %s. Will not be used until both are set.", IntrospectionClientIDEnvVar, IntrospectionClientSecretEnvVar) | ||
| introspectionClientID = "" | ||
| introspectionClientSecret = "" | ||
| } |
There was a problem hiding this comment.
The current condition logs a warning even when both environment variables are empty (the default case). This will cause unnecessary warning logs on every startup for users who do not use introspection client authentication. We should only log a warning if exactly one of the environment variables is set.
if introspectionClientID == "" || introspectionClientSecret == "" {
if (introspectionClientID == "") != (introspectionClientSecret == "") {
log.Printf("WARNING: Only one of %s or %s is set. Both must be set to enable introspection client authentication.", IntrospectionClientIDEnvVar, IntrospectionClientSecretEnvVar)
}
introspectionClientID = ""
introspectionClientSecret = ""
}Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Description
forceIntrospection. Some identity providers require introspection on access_token validation (instead of the normal JWKS validation approach). If the value istrue, will treat the incoming token as opaque and perform introspection validation. This is only enabled whenmcpEnabledistrue.INTROSPECTION_CLIENT_IDandINTROSPECTION_CLIENT_SECRET. Some identity providers require a client id/secret to be set when performing introspection on an access token. If both environment variables are set, the logic will send Basic<client_id:client_secret> as the user/pass of the introspection request. If only one environment variable is populated, a warning log will be written.PR Checklist
CONTRIBUTING.md
bug/issue
before writing your code! That way we can discuss the change, evaluate
designs, and agree on the general idea
review
!if this involve a breaking change🛠️ Fixes #<issue_number_goes_here>