Skip to content

Commit 438cd43

Browse files
mcp: OIDC-Flavored Refresh Token Guidance (#939)
## Description Implemets SEP-2207 (https://modelcontextprotocol.io/seps/2207-oidc-refresh-token-guidance) Fixes #940
1 parent 9604499 commit 438cd43

3 files changed

Lines changed: 155 additions & 0 deletions

File tree

auth/authorization_code.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,23 @@ type AuthorizationCodeHandlerConfig struct {
9090
// See [AuthorizationCodeFetcher] for details.
9191
AuthorizationCodeFetcher AuthorizationCodeFetcher
9292

93+
// RequestRefreshToken indicates that the client intends to use refresh
94+
// tokens and is capable of storing them securely.
95+
//
96+
// When true and the Authorization Server metadata contains "offline_access"
97+
// in its scopes_supported, the client adds "offline_access" to the
98+
// requested scopes.
99+
//
100+
// When using Dynamic Client Registration, callers should include
101+
// "refresh_token" in [DynamicClientRegistrationConfig].Metadata.GrantTypes
102+
// directly to advertise refresh token support to the Authorization Server.
103+
//
104+
// When using Client ID Metadata Document, the document hosted at the
105+
// Client ID URL should include "refresh_token" in its grant_types.
106+
//
107+
// See https://modelcontextprotocol.io/seps/2207-oidc-refresh-token-guidance.
108+
RequestRefreshToken bool
109+
93110
// Client is an optional HTTP client to use for HTTP requests.
94111
// It is used for the following requests:
95112
// - Fetching Protected Resource Metadata
@@ -263,6 +280,14 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ
263280
scps = prm.ScopesSupported
264281
}
265282

283+
// SEP-2207: when the client desires refresh tokens and the Authorization
284+
// Server advertises offline_access support, add it to the requested scopes.
285+
if h.config.RequestRefreshToken &&
286+
slices.Contains(asm.ScopesSupported, "offline_access") &&
287+
!slices.Contains(scps, "offline_access") {
288+
scps = append(scps, "offline_access")
289+
}
290+
266291
cfg := &oauth2.Config{
267292
ClientID: resolvedClientConfig.clientID,
268293
ClientSecret: resolvedClientConfig.clientSecret,

auth/authorization_code_test.go

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"net/http"
1111
"net/http/httptest"
1212
"net/http/httputil"
13+
"net/url"
14+
"slices"
1315
"strings"
1416
"testing"
1517

@@ -738,6 +740,130 @@ func TestApplicationTypeInference(t *testing.T) {
738740
}
739741
}
740742

743+
func TestAuthorize_OfflineAccessScope(t *testing.T) {
744+
tests := []struct {
745+
name string
746+
requestRefreshToken bool
747+
asScopesSupported []string
748+
challengeScopes string
749+
wantOfflineAccess bool
750+
}{
751+
{
752+
name: "AddedWhenASSupportsAndClientRequests",
753+
requestRefreshToken: true,
754+
asScopesSupported: []string{"openid", "offline_access"},
755+
wantOfflineAccess: true,
756+
},
757+
{
758+
name: "NotAddedWhenClientDoesNotRequest",
759+
requestRefreshToken: false,
760+
asScopesSupported: []string{"openid", "offline_access"},
761+
wantOfflineAccess: false,
762+
},
763+
{
764+
name: "NotAddedWhenASDoesNotSupport",
765+
requestRefreshToken: true,
766+
asScopesSupported: []string{"openid"},
767+
wantOfflineAccess: false,
768+
},
769+
{
770+
name: "NotDuplicatedWhenAlreadyInScopes",
771+
requestRefreshToken: true,
772+
asScopesSupported: []string{"openid", "offline_access"},
773+
challengeScopes: "read offline_access",
774+
wantOfflineAccess: true,
775+
},
776+
}
777+
778+
for _, tt := range tests {
779+
t.Run(tt.name, func(t *testing.T) {
780+
authServer := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{
781+
ScopesSupported: tt.asScopesSupported,
782+
RegistrationConfig: &oauthtest.RegistrationConfig{
783+
PreregisteredClients: map[string]oauthtest.ClientInfo{
784+
"test_client_id": {
785+
Secret: "test_client_secret",
786+
RedirectURIs: []string{"http://localhost:12345/callback"},
787+
},
788+
},
789+
},
790+
})
791+
authServer.Start(t)
792+
793+
resourceMux := http.NewServeMux()
794+
resourceServer := httptest.NewServer(resourceMux)
795+
t.Cleanup(resourceServer.Close)
796+
resourceURL := resourceServer.URL + "/resource"
797+
resourceMux.Handle("/.well-known/oauth-protected-resource/resource", ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{
798+
Resource: resourceURL,
799+
AuthorizationServers: []string{authServer.URL()},
800+
}))
801+
802+
var capturedAuthURL string
803+
handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{
804+
RedirectURL: "http://localhost:12345/callback",
805+
PreregisteredClient: &oauthex.ClientCredentials{
806+
ClientID: "test_client_id",
807+
ClientSecretAuth: &oauthex.ClientSecretAuth{
808+
ClientSecret: "test_client_secret",
809+
},
810+
},
811+
RequestRefreshToken: tt.requestRefreshToken,
812+
AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) {
813+
capturedAuthURL = args.URL
814+
return nil, fmt.Errorf("stop after capturing URL")
815+
},
816+
})
817+
if err != nil {
818+
t.Fatalf("NewAuthorizationCodeHandler failed: %v", err)
819+
}
820+
821+
req := httptest.NewRequest(http.MethodGet, resourceURL, nil)
822+
resp := &http.Response{
823+
StatusCode: http.StatusUnauthorized,
824+
Header: make(http.Header),
825+
Body: http.NoBody,
826+
Request: req,
827+
}
828+
wwwAuth := "Bearer resource_metadata=" + resourceServer.URL + "/.well-known/oauth-protected-resource/resource"
829+
if tt.challengeScopes != "" {
830+
wwwAuth += fmt.Sprintf(", scope=%q", tt.challengeScopes)
831+
}
832+
resp.Header.Set("WWW-Authenticate", wwwAuth)
833+
834+
// Authorize will fail at the fetcher, but we only care about the URL.
835+
handler.Authorize(context.Background(), req, resp)
836+
837+
if capturedAuthURL == "" {
838+
t.Fatal("AuthorizationCodeFetcher was not called")
839+
}
840+
u, err := url.Parse(capturedAuthURL)
841+
if err != nil {
842+
t.Fatalf("failed to parse captured auth URL: %v", err)
843+
}
844+
scopes := strings.Fields(u.Query().Get("scope"))
845+
hasOfflineAccess := slices.Contains(scopes, "offline_access")
846+
if hasOfflineAccess != tt.wantOfflineAccess {
847+
t.Errorf("offline_access in scopes = %v, want %v (scopes: %v)", hasOfflineAccess, tt.wantOfflineAccess, scopes)
848+
}
849+
850+
// When offline_access was already present in challenge scopes,
851+
// verify it appears exactly once.
852+
if tt.wantOfflineAccess {
853+
count := 0
854+
for _, s := range scopes {
855+
if s == "offline_access" {
856+
count++
857+
}
858+
}
859+
if count != 1 {
860+
t.Errorf("offline_access appears %d times in scopes, want 1", count)
861+
}
862+
}
863+
})
864+
}
865+
}
866+
741867
// validConfig for test to create an AuthorizationCodeHandler using its constructor.
742868
// Values that are relevant to the test should be set explicitly.
743869
func validConfig() *AuthorizationCodeHandlerConfig {

internal/oauthtest/fake_authorization_server.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ type Config struct {
8282
// ClientCredentialsConfig enables RFC 6749 Section 4.4 client credentials
8383
// grant at the /token endpoint.
8484
ClientCredentialsConfig *ClientCredentialsConfig
85+
// ScopesSupported is an optional list of scopes to advertise in the
86+
// authorization server metadata.
87+
ScopesSupported []string
8588
}
8689

8790
// FakeAuthorizationServer is a fake OAuth 2.0 Authorization Server for testing.
@@ -166,6 +169,7 @@ func (s *FakeAuthorizationServer) handleMetadata(w http.ResponseWriter, r *http.
166169
AuthorizationEndpoint: s.URL() + s.config.IssuerPath + "/authorize",
167170
TokenEndpoint: s.URL() + s.config.IssuerPath + "/token",
168171
RegistrationEndpoint: registrationEndpoint,
172+
ScopesSupported: s.config.ScopesSupported,
169173
ResponseTypesSupported: []string{"code"},
170174
CodeChallengeMethodsSupported: []string{"S256"},
171175
ClientIDMetadataDocumentSupported: cimdSupported,

0 commit comments

Comments
 (0)