Skip to content

Commit bf69179

Browse files
mcp: Add client-side scope accumulation (#944)
## Description This PR implements modelcontextprotocol/modelcontextprotocol#2350 Fixes #943 --------- Co-authored-by: Maciej Kisiel <mkisiel@google.com>
1 parent 1253d17 commit bf69179

7 files changed

Lines changed: 413 additions & 23 deletions

File tree

auth/authorization_code.go

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"slices"
1616
"strings"
1717

18+
"github.com/modelcontextprotocol/go-sdk/internal/authutil"
1819
"github.com/modelcontextprotocol/go-sdk/internal/util"
1920
"github.com/modelcontextprotocol/go-sdk/oauthex"
2021
"golang.org/x/oauth2"
@@ -128,6 +129,9 @@ type AuthorizationCodeHandler struct {
128129

129130
// tokenSource is the token source to use for authorization.
130131
tokenSource oauth2.TokenSource
132+
133+
// grantedScopes maps authorization server issuer to the list of scopes granted by that issuer.
134+
grantedScopes map[string][]string
131135
}
132136

133137
var _ OAuthHandler = (*AuthorizationCodeHandler)(nil)
@@ -187,7 +191,10 @@ func NewAuthorizationCodeHandler(config *AuthorizationCodeHandlerConfig) (*Autho
187191
if config.Client == nil {
188192
config.Client = http.DefaultClient
189193
}
190-
return &AuthorizationCodeHandler{config: config}, nil
194+
return &AuthorizationCodeHandler{
195+
config: config,
196+
grantedScopes: make(map[string][]string),
197+
}, nil
191198
}
192199

193200
func isNonRootHTTPSURL(u string) bool {
@@ -275,19 +282,24 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ
275282
return err
276283
}
277284

278-
scps := scopesFromChallenges(wwwChallenges)
279-
if len(scps) == 0 && len(prm.ScopesSupported) > 0 {
280-
scps = prm.ScopesSupported
285+
requestedScopes := scopesFromChallenges(wwwChallenges)
286+
if len(requestedScopes) == 0 && len(prm.ScopesSupported) > 0 {
287+
requestedScopes = prm.ScopesSupported
281288
}
282289

283290
// SEP-2207: when the client desires refresh tokens and the Authorization
284291
// Server advertises offline_access support, add it to the requested scopes.
285292
if h.config.RequestRefreshToken &&
286293
slices.Contains(asm.ScopesSupported, "offline_access") &&
287-
!slices.Contains(scps, "offline_access") {
288-
scps = append(scps, "offline_access")
294+
!slices.Contains(requestedScopes, "offline_access") {
295+
requestedScopes = append(requestedScopes, "offline_access")
289296
}
290297

298+
// Accumulate scopes: union previously granted scopes with the newly
299+
// challenged scopes so that step-up authorization does not lose
300+
// permissions granted in earlier rounds (SEP-2350).
301+
requestedScopes = authutil.UnionScopes(h.grantedScopes[asm.Issuer], requestedScopes)
302+
291303
cfg := &oauth2.Config{
292304
ClientID: resolvedClientConfig.clientID,
293305
ClientSecret: resolvedClientConfig.clientSecret,
@@ -298,7 +310,7 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ
298310
AuthStyle: resolvedClientConfig.authStyle,
299311
},
300312
RedirectURL: h.config.RedirectURL,
301-
Scopes: scps,
313+
Scopes: requestedScopes,
302314
}
303315

304316
authRes, err := h.getAuthorizationCode(ctx, cfg, prm.Resource)
@@ -307,7 +319,12 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ
307319
return err
308320
}
309321

310-
return h.exchangeAuthorizationCode(ctx, cfg, authRes, prm.Resource)
322+
err = h.exchangeAuthorizationCode(ctx, cfg, authRes, prm.Resource)
323+
if err != nil {
324+
return err
325+
}
326+
327+
return h.updateGrantedScopes(asm.Issuer, requestedScopes)
311328
}
312329

313330
// resourceMetadataURLFromChallenges returns a resource metadata URL from the given "WWW-Authenticate" header challenges,
@@ -558,3 +575,20 @@ func (h *AuthorizationCodeHandler) exchangeAuthorizationCode(ctx context.Context
558575
h.tokenSource = cfg.TokenSource(clientCtx, token)
559576
return nil
560577
}
578+
579+
// updateGrantedScopes updates the granted scopes based on the token source and requested scopes.
580+
func (h *AuthorizationCodeHandler) updateGrantedScopes(issuer string, requestedScopes []string) error {
581+
if h.tokenSource == nil {
582+
return nil
583+
}
584+
tok, err := h.tokenSource.Token()
585+
if err != nil {
586+
return err
587+
}
588+
if tokenScopes := authutil.ScopesFromToken(tok); tokenScopes == nil {
589+
h.grantedScopes[issuer] = requestedScopes
590+
} else {
591+
h.grantedScopes[issuer] = tokenScopes
592+
}
593+
return nil
594+
}

auth/authorization_code_test.go

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"testing"
1717

1818
"github.com/google/go-cmp/cmp"
19+
"github.com/google/go-cmp/cmp/cmpopts"
1920
"github.com/modelcontextprotocol/go-sdk/internal/oauthtest"
2021
"github.com/modelcontextprotocol/go-sdk/oauthex"
2122
"golang.org/x/oauth2"
@@ -113,6 +114,131 @@ func TestAuthorize(t *testing.T) {
113114
}
114115
}
115116

117+
func TestAuthorize_ScopeAccumulation(t *testing.T) {
118+
authServer := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{
119+
RegistrationConfig: &oauthtest.RegistrationConfig{
120+
PreregisteredClients: map[string]oauthtest.ClientInfo{
121+
"test_client_id": {
122+
Secret: "test_client_secret",
123+
RedirectURIs: []string{"http://localhost:12345/callback"},
124+
},
125+
},
126+
},
127+
TokenScopeFunc: func(requestedScope string) string {
128+
// Simulate a server that never grants "write".
129+
var granted []string
130+
for _, s := range strings.Fields(requestedScope) {
131+
if s != "write" {
132+
granted = append(granted, s)
133+
}
134+
}
135+
return strings.Join(granted, " ")
136+
},
137+
})
138+
authServer.Start(t)
139+
140+
resourceMux := http.NewServeMux()
141+
resourceServer := httptest.NewServer(resourceMux)
142+
t.Cleanup(resourceServer.Close)
143+
resourceURL := resourceServer.URL + "/resource"
144+
145+
resourceMux.Handle("/.well-known/oauth-protected-resource/resource", ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{
146+
Resource: resourceURL,
147+
AuthorizationServers: []string{authServer.URL()},
148+
}))
149+
150+
var capturedAuthURLs []string
151+
noRedirectClient := &http.Client{
152+
CheckRedirect: func(req *http.Request, via []*http.Request) error {
153+
return http.ErrUseLastResponse
154+
},
155+
}
156+
157+
handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{
158+
RedirectURL: "http://localhost:12345/callback",
159+
PreregisteredClient: &oauthex.ClientCredentials{
160+
ClientID: "test_client_id",
161+
ClientSecretAuth: &oauthex.ClientSecretAuth{
162+
ClientSecret: "test_client_secret",
163+
},
164+
},
165+
AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) {
166+
capturedAuthURLs = append(capturedAuthURLs, args.URL)
167+
resp, err := noRedirectClient.Get(args.URL)
168+
if err != nil {
169+
return nil, err
170+
}
171+
defer resp.Body.Close()
172+
loc, err := resp.Location()
173+
if err != nil {
174+
return nil, err
175+
}
176+
return &AuthorizationResult{
177+
Code: loc.Query().Get("code"),
178+
State: loc.Query().Get("state"),
179+
}, nil
180+
},
181+
})
182+
if err != nil {
183+
t.Fatalf("NewAuthorizationCodeHandler failed: %v", err)
184+
}
185+
186+
// First authorization: 401 with scope="read write".
187+
// The token response will only grant "read" (TokenScopeFunc strips "write").
188+
req := httptest.NewRequest(http.MethodGet, resourceURL, nil)
189+
resp := &http.Response{
190+
StatusCode: http.StatusUnauthorized,
191+
Header: make(http.Header),
192+
Body: http.NoBody,
193+
}
194+
resp.Header.Set("WWW-Authenticate",
195+
fmt.Sprintf(`Bearer scope="read write", resource_metadata="%s/.well-known/oauth-protected-resource/resource"`, resourceServer.URL))
196+
if err := handler.Authorize(context.Background(), req, resp); err != nil {
197+
t.Fatalf("First Authorize failed: %v", err)
198+
}
199+
200+
// Verify first auth URL requested "read" and "write".
201+
firstURL, err := url.Parse(capturedAuthURLs[0])
202+
if err != nil {
203+
t.Fatalf("Failed to parse first auth URL: %v", err)
204+
}
205+
firstScopes := strings.Fields(firstURL.Query().Get("scope"))
206+
if diff := cmp.Diff([]string{"read", "write"}, firstScopes, cmpopts.SortSlices(func(a, b string) bool { return a < b })); diff != "" {
207+
t.Errorf("First auth scopes mismatch (-want +got):\n%s", diff)
208+
}
209+
210+
// Verify only "read" was granted (the token omitted "write").
211+
issuer := authServer.URL()
212+
if diff := cmp.Diff([]string{"read"}, handler.grantedScopes[issuer], cmpopts.SortSlices(func(a, b string) bool { return a < b })); diff != "" {
213+
t.Errorf("After first Authorize, grantedScopes mismatch (-want +got):\n%s", diff)
214+
}
215+
216+
// Second authorization: 403 insufficient_scope with scope="admin".
217+
// Accumulated scopes should be "read" (previously granted) + "admin" (new).
218+
req2 := httptest.NewRequest(http.MethodGet, resourceURL, nil)
219+
resp2 := &http.Response{
220+
StatusCode: http.StatusForbidden,
221+
Header: make(http.Header),
222+
Body: http.NoBody,
223+
}
224+
resp2.Header.Set("WWW-Authenticate",
225+
fmt.Sprintf(`Bearer error="insufficient_scope", scope="admin", resource_metadata="%s/.well-known/oauth-protected-resource/resource"`, resourceServer.URL))
226+
if err := handler.Authorize(context.Background(), req2, resp2); err != nil {
227+
t.Fatalf("Second Authorize failed: %v", err)
228+
}
229+
230+
// Verify second auth URL accumulated "read" (granted) + "admin" (challenged),
231+
// but NOT "write" (requested but never granted).
232+
secondURL, err := url.Parse(capturedAuthURLs[1])
233+
if err != nil {
234+
t.Fatalf("Failed to parse second auth URL: %v", err)
235+
}
236+
secondScopes := strings.Fields(secondURL.Query().Get("scope"))
237+
if diff := cmp.Diff([]string{"admin", "read"}, secondScopes, cmpopts.SortSlices(func(a, b string) bool { return a < b })); diff != "" {
238+
t.Errorf("Second auth scopes mismatch (-want +got):\n%s", diff)
239+
}
240+
}
241+
116242
func TestAuthorize_ForbiddenUnhandledError(t *testing.T) {
117243
req := httptest.NewRequest(http.MethodGet, "http://example.com/resource", nil)
118244
resp := &http.Response{

auth/extauth/client_credentials.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"strings"
1515

1616
"github.com/modelcontextprotocol/go-sdk/auth"
17+
"github.com/modelcontextprotocol/go-sdk/internal/authutil"
1718
"github.com/modelcontextprotocol/go-sdk/oauthex"
1819
"golang.org/x/oauth2"
1920
"golang.org/x/oauth2/clientcredentials"
@@ -47,6 +48,9 @@ type ClientCredentialsHandlerConfig struct {
4748
type ClientCredentialsHandler struct {
4849
config *ClientCredentialsHandlerConfig
4950
tokenSource oauth2.TokenSource
51+
52+
// grantedScopes maps authorization server issuer to the list of scopes granted by that issuer.
53+
grantedScopes map[string][]string
5054
}
5155

5256
// Compile-time check that ClientCredentialsHandler implements auth.OAuthHandler.
@@ -67,7 +71,10 @@ func NewClientCredentialsHandler(config *ClientCredentialsHandlerConfig) (*Clien
6771
if config.Credentials.ClientSecretAuth == nil {
6872
return nil, fmt.Errorf("clientSecretAuth is required for client credentials grant")
6973
}
70-
return &ClientCredentialsHandler{config: config}, nil
74+
return &ClientCredentialsHandler{
75+
config: config,
76+
grantedScopes: make(map[string][]string),
77+
}, nil
7178
}
7279

7380
// TokenSource returns the token source for outgoing requests.
@@ -121,30 +128,47 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ
121128
}
122129
}
123130

124-
// Determine scopes: use PRM's scopes_supported if available.
125-
scopes := scopesFromChallenges(wwwChallenges)
126-
if len(scopes) == 0 && len(prm.ScopesSupported) > 0 {
127-
scopes = prm.ScopesSupported
131+
// Determine requestedScopes: use PRM's scopes_supported if available.
132+
requestedScopes := scopesFromChallenges(wwwChallenges)
133+
if len(requestedScopes) == 0 && len(prm.ScopesSupported) > 0 {
134+
requestedScopes = prm.ScopesSupported
128135
}
129136

137+
// Accumulate scopes: union previously granted scopes with the newly
138+
// challenged scopes so that step-up authorization does not lose
139+
// permissions granted in earlier rounds (SEP-2350).
140+
requestedScopes = authutil.UnionScopes(h.grantedScopes[asm.Issuer], requestedScopes)
141+
130142
// Step 3: Exchange client credentials for an access token.
131143
creds := h.config.Credentials
132144
cfg := &clientcredentials.Config{
133145
ClientID: creds.ClientID,
134146
ClientSecret: creds.ClientSecretAuth.ClientSecret,
135147
TokenURL: asm.TokenEndpoint,
136-
Scopes: scopes,
148+
Scopes: requestedScopes,
137149
AuthStyle: selectTokenAuthMethod(asm.TokenEndpointAuthMethodsSupported),
138150
}
139151

140152
ctxWithClient := context.WithValue(ctx, oauth2.HTTPClient, httpClient)
141153
h.tokenSource = cfg.TokenSource(ctxWithClient)
142154

143-
// Eagerly fetch a token to surface errors immediately.
144-
if _, err := h.tokenSource.Token(); err != nil {
155+
return h.updateGrantedScopes(asm.Issuer, requestedScopes)
156+
}
157+
158+
func (h *ClientCredentialsHandler) updateGrantedScopes(issuer string, requestedScopes []string) error {
159+
if h.tokenSource == nil {
160+
return nil
161+
}
162+
tok, err := h.tokenSource.Token()
163+
if err != nil {
145164
h.tokenSource = nil
146165
return fmt.Errorf("client credentials token request failed: %w", err)
147166
}
167+
if tokenScopes := authutil.ScopesFromToken(tok); tokenScopes == nil {
168+
h.grantedScopes[issuer] = requestedScopes
169+
} else {
170+
h.grantedScopes[issuer] = tokenScopes
171+
}
148172
return nil
149173
}
150174

0 commit comments

Comments
 (0)