auth: add ScopeFilter hook to AuthorizationCodeHandlerConfig (#1044)
## Summary The authorization-code handler requests every scope advertised in the protected resource's metadata `scopes_supported` (or named in the `WWW-Authenticate` challenge), and offers the caller no way to adjust that set. Some servers advertise a scope a client should *not* request: - Google's Gmail MCP server advertises `gmail.metadata` in `scopes_supported`. - The Gmail API refuses the search `q` parameter on any token that carries `gmail.metadata` — **even when `gmail.readonly` is also granted** (a long-standing documented Gmail restriction, see googleapis/google-api-python-client#582). Because the handler requests the full advertised set, `gmail.metadata` ends up in the grant and breaks search, with no client-side remedy. ## Change Add an optional `ScopeFilter func(discovered []string) []string` callback to `AuthorizationCodeHandlerConfig`. When non-nil, it receives the discovered scopes and returns the set to request, giving the client full control to narrow, extend, or reorder them. It runs **before** the `offline_access` (SEP-2207) and step-up union (SEP-2350) logic, so refresh-token support and scope accumulation are unaffected. (Started as a static `Scopes []string` allowlist; switched to a callback per review feedback for more flexibility.) ## Test `TestAuthorize_ScopeFilter` drives a full authorize flow and asserts the requested `scope` parameter for a nil filter (unchanged), a filter that drops a scope, and one that replaces the set entirely. `go test ./auth/...` (incl. `-race`), `go vet`, and `gofmt` are clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
This commit is contained in:
parent
7743b71b1c
commit
0c004ee48a
2 changed files with 116 additions and 0 deletions
|
|
@ -98,6 +98,15 @@ type AuthorizationCodeHandlerConfig struct {
|
|||
// See [AuthorizationCodeFetcher] for details.
|
||||
AuthorizationCodeFetcher AuthorizationCodeFetcher
|
||||
|
||||
// ScopeFilter, if non-nil, is called with the scopes discovered from the
|
||||
// protected resource metadata (or WWW-Authenticate challenge) and returns
|
||||
// the scopes to request during authorization. It gives the client full
|
||||
// control to narrow, extend, or reorder that set — e.g. dropping Gmail's
|
||||
// gmail.metadata, which the Gmail API refuses to combine with the search "q"
|
||||
// parameter even alongside gmail.readonly. It runs before offline_access
|
||||
// (see RequestRefreshToken) and the step-up union, so neither is affected.
|
||||
ScopeFilter func(discovered []string) []string
|
||||
|
||||
// RequestRefreshToken indicates that the client intends to use refresh
|
||||
// tokens and is capable of storing them securely.
|
||||
//
|
||||
|
|
@ -318,6 +327,12 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ
|
|||
requestedScopes = prm.ScopesSupported
|
||||
}
|
||||
|
||||
// Let the client adjust the discovered scopes before offline_access and the
|
||||
// step-up union below so neither is affected.
|
||||
if h.config.ScopeFilter != nil {
|
||||
requestedScopes = h.config.ScopeFilter(requestedScopes)
|
||||
}
|
||||
|
||||
// SEP-2207: when the client desires refresh tokens and the Authorization
|
||||
// Server advertises offline_access support, add it to the requested scopes.
|
||||
if h.config.RequestRefreshToken &&
|
||||
|
|
|
|||
|
|
@ -1222,6 +1222,107 @@ func TestAuthorize_OfflineAccessScope(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestAuthorize_ScopeFilter(t *testing.T) {
|
||||
// advertised is delivered via the WWW-Authenticate "scope" challenge, which
|
||||
// the handler treats as the discovered scopes passed to ScopeFilter.
|
||||
const advertised = "gmail.metadata gmail.readonly gmail.compose"
|
||||
tests := []struct {
|
||||
name string
|
||||
filter func(discovered []string) []string
|
||||
want []string // requested scopes, order-independent
|
||||
}{
|
||||
{
|
||||
name: "nil filter leaves scopes unchanged",
|
||||
filter: nil,
|
||||
want: []string{"gmail.metadata", "gmail.readonly", "gmail.compose"},
|
||||
},
|
||||
{
|
||||
name: "filter drops a scope",
|
||||
filter: func(d []string) []string {
|
||||
return slices.DeleteFunc(slices.Clone(d), func(s string) bool { return s == "gmail.metadata" })
|
||||
},
|
||||
want: []string{"gmail.readonly", "gmail.compose"},
|
||||
},
|
||||
{
|
||||
name: "filter replaces the set entirely",
|
||||
filter: func([]string) []string { return []string{"gmail.readonly"} },
|
||||
want: []string{"gmail.readonly"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
authServer := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{
|
||||
RegistrationConfig: &oauthtest.RegistrationConfig{
|
||||
PreregisteredClients: map[string]oauthtest.ClientInfo{
|
||||
"test_client_id": {
|
||||
Secret: "test_client_secret",
|
||||
RedirectURIs: []string{"http://localhost:12345/callback"},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
authServer.Start(t)
|
||||
|
||||
resourceMux := http.NewServeMux()
|
||||
resourceServer := httptest.NewServer(resourceMux)
|
||||
t.Cleanup(resourceServer.Close)
|
||||
resourceURL := resourceServer.URL + "/resource"
|
||||
resourceMux.Handle("/.well-known/oauth-protected-resource/resource", ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{
|
||||
Resource: resourceURL,
|
||||
AuthorizationServers: []string{authServer.URL()},
|
||||
}))
|
||||
|
||||
var capturedAuthURL string
|
||||
handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{
|
||||
RedirectURL: "http://localhost:12345/callback",
|
||||
PreregisteredClient: &oauthex.ClientCredentials{
|
||||
ClientID: "test_client_id",
|
||||
ClientSecretAuth: &oauthex.ClientSecretAuth{ClientSecret: "test_client_secret"},
|
||||
},
|
||||
ScopeFilter: tt.filter,
|
||||
AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) {
|
||||
capturedAuthURL = args.URL
|
||||
return nil, fmt.Errorf("stop after capturing URL")
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("NewAuthorizationCodeHandler failed: %v", err)
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, resourceURL, nil)
|
||||
resp := &http.Response{
|
||||
StatusCode: http.StatusUnauthorized,
|
||||
Header: make(http.Header),
|
||||
Body: http.NoBody,
|
||||
Request: req,
|
||||
}
|
||||
resp.Header.Set("WWW-Authenticate", fmt.Sprintf(
|
||||
"Bearer resource_metadata=%s/.well-known/oauth-protected-resource/resource, scope=%q",
|
||||
resourceServer.URL, advertised))
|
||||
|
||||
handler.Authorize(context.Background(), req, resp)
|
||||
|
||||
if capturedAuthURL == "" {
|
||||
t.Fatal("AuthorizationCodeFetcher was not called")
|
||||
}
|
||||
u, err := url.Parse(capturedAuthURL)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to parse captured auth URL: %v", err)
|
||||
}
|
||||
// Compare as a set: UnionScopes (applied downstream) returns map keys,
|
||||
// so the order of the requested scope parameter is not deterministic.
|
||||
got := strings.Fields(u.Query().Get("scope"))
|
||||
want := slices.Clone(tt.want)
|
||||
slices.Sort(got)
|
||||
slices.Sort(want)
|
||||
if !slices.Equal(got, want) {
|
||||
t.Errorf("requested scopes = %v, want %v (any order)", got, want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// validConfig for test to create an AuthorizationCodeHandler using its constructor.
|
||||
// Values that are relevant to the test should be set explicitly.
|
||||
func validConfig() *AuthorizationCodeHandlerConfig {
|
||||
|
|
|
|||
Loading…
Reference in a new issue