diff --git a/cmd/root.go b/cmd/root.go index 862b247b..79f97720 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -133,6 +133,7 @@ func init() { f.StringVar(&rootArgs.config.ClientSecret, "client-secret", "", "Client secret for the OAuth") f.StringVar(&rootArgs.config.DefaultAuthorizeResponseMode, "default-authorize-response-mode", constants.ResponseModeQuery, "Default response mode for the authorize endpoint") f.StringVar(&rootArgs.config.DefaultAuthorizeResponseType, "default-authorize-response-type", constants.ResponseTypeToken, "Default response type for the authorize endpoint") + f.BoolVar(&rootArgs.config.OAuth21Strict, "oauth2-1-strict", false, "Enforce OAuth 2.1 restrictions: reject the implicit/hybrid-with-token response types (token, id_token token) and PKCE plain (require S256). Breaking; opt-in") // Admin flags f.StringVar(&rootArgs.config.AdminSecret, "admin-secret", "", "Secret for the admin (REQUIRED, must not be empty)") diff --git a/internal/config/config.go b/internal/config/config.go index f018ab57..439b0dd5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -249,6 +249,12 @@ type Config struct { DefaultAuthorizeResponseMode string // Default Authorize response type DefaultAuthorizeResponseType string + // OAuth21Strict opts into OAuth 2.1 breaking restrictions (default false). + // When true the authorization endpoint rejects the implicit/hybrid-with-token + // response types (response_type=token and id_token token) and the PKCE + // "plain" code_challenge_method (S256 required). Off keeps all current + // behavior unchanged. + OAuth21Strict bool // Twilio Configurations // TwilioAPISecret is the API secret for Twilio diff --git a/internal/http_handlers/authorize.go b/internal/http_handlers/authorize.go index d81bf825..603c9d50 100644 --- a/internal/http_handlers/authorize.go +++ b/internal/http_handlers/authorize.go @@ -95,6 +95,14 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc { nonce := param("nonce") screenHint := param("screen_hint") + // RFC 8707 resource indicator: optional. When present it binds the + // issued authorization code (and, at exchange time, the access token's + // `aud`) to the target resource server. RFC 8707 §2 permits multiple + // resource values, but this flow issues a single-audience access token + // (matching token_exchange.go), so exactly one is allowed — reject a + // repeated parameter rather than silently picking one. + resource := param("resource") + // OIDC Core §3.1.2.1 standard authorization request parameters. loginHint := param("login_hint") uiLocales := param("ui_locales") @@ -175,6 +183,21 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc { } } + // RFC 8707: reject a repeated resource parameter (single audience only). + // redirect_uri is validated above, so redirectErrorToRP is safe here. + if vals := gc.Request.Form["resource"]; len(vals) > 1 { + redirectErrorToRP(gc, responseMode, redirectURI, state, "invalid_request", "only one resource parameter is supported") + return + } + + // RFC 8707 §2: the resource indicator MUST be an absolute URI and MUST + // NOT include a fragment component. The RFC-conventional error code for + // a rejected resource is invalid_target. + if resource != "" && !isValidResourceIndicator(resource) { + redirectErrorToRP(gc, responseMode, redirectURI, state, "invalid_target", "resource must be an absolute URI without a fragment") + return + } + // OIDCC-3.1.2.6 (JAR, RFC 9101): the server must either process a // `request`/`request_uri` object or reject it with // request_not_supported / request_uri_not_supported. We don't parse @@ -221,6 +244,14 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc { }) return } + // OAuth 2.1 strict mode: PKCE "plain" is removed — S256 only. + if h.Config.OAuth21Strict && codeChallengeMethod == "plain" && codeChallenge != "" { + gc.JSON(http.StatusBadRequest, gin.H{ + "error": "invalid_request", + "error_description": "code_challenge_method=plain is not allowed; use S256", + }) + return + } canonical, ok := supportedResponseTypeSet(responseType) if !ok { @@ -260,6 +291,21 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc { } } + // OAuth 2.1 strict mode: the implicit grant is removed. Reject EVERY + // response type that delivers a bearer access token into the URL + // fragment — not just "token" and "id_token token" but also the + // front-channel-token hybrids "code token" and "code id_token token", + // which equally return an access_token in the fragment (see the + // hasAccessToken dispatch below). responseType is already canonical + // (sorted/deduped/lowercased), so a bearer access token is present iff + // the space-separated components include the exact component "token". + // The check is component-exact (not a substring) so "id_token" — which + // contains "token" only as a substring — is NOT matched. + if h.Config.OAuth21Strict && responseTypeHasBareToken(responseType) { + redirectErrorToRP(gc, responseMode, redirectURI, state, "unsupported_response_type", "response_type="+responseType+" is not supported") + return + } + if errCode, errDesc := h.validateAuthorizeRequest(responseType, responseMode, state); errCode != "" { log.Debug().Str("error", errCode).Str("error_description", errDesc).Msg("Invalid request") gc.JSON(http.StatusBadRequest, gin.H{ @@ -332,7 +378,10 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc { if codeChallenge != "" { challengeData = codeChallenge + "::" + codeChallengeMethod } - if err := h.MemoryStoreProvider.SetState(state, code+"@@"+challengeData+"@@"+nonce+"@@"+url.QueryEscape(redirectURI)); err != nil { + // [4] carries the RFC 8707 resource (url-escaped, empty when absent) + // so the login/signup/session/auth_response services can rebind it + // to the code state they persist after a fresh login. + if err := h.MemoryStoreProvider.SetState(state, code+"@@"+challengeData+"@@"+nonce+"@@"+url.QueryEscape(redirectURI)+"@@"+url.QueryEscape(resource)); err != nil { log.Debug().Err(err).Msg("Error setting temp code") gc.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) return @@ -715,7 +764,10 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc { if codeChallenge != "" { codeChallengeData = codeChallenge + "::" + codeChallengeMethod } - if err := h.MemoryStoreProvider.SetState(code, codeChallengeData+"@@"+newSessionToken+"@@"+nonce+"@@"+url.QueryEscape(redirectURI)); err != nil { + // [4] binds the RFC 8707 resource to the code (url-escaped, empty + // when absent); the token endpoint enforces the echoed resource + // matches this and sets the access token `aud` to it. + if err := h.MemoryStoreProvider.SetState(code, codeChallengeData+"@@"+newSessionToken+"@@"+nonce+"@@"+url.QueryEscape(redirectURI)+"@@"+url.QueryEscape(resource)); err != nil { log.Debug().Err(err).Msg("Error setting temp code") handleResponse(gc, responseMode, authURL, redirectURI, loginError, http.StatusOK) return @@ -936,6 +988,47 @@ func supportedResponseTypeSet(raw string) (string, bool) { return "", false } +// isValidResourceIndicator enforces RFC 8707 §2 on a resource indicator: it +// MUST be an absolute URI (scheme + hierarchical part) and MUST NOT contain a +// fragment component. A relative reference, an opaque non-URI string, or any +// value carrying a "#" is rejected. Callers must pass a non-empty value. +func isValidResourceIndicator(resource string) bool { + // A fragment delimiter is forbidden outright — check the raw string so a + // trailing "#" (empty fragment) is also rejected, not just a populated one. + if strings.Contains(resource, "#") { + return false + } + u, err := url.Parse(resource) + if err != nil { + return false + } + // Absolute URI: has a scheme and is not just an opaque scheme:string. Require + // a scheme and either a host or a rooted path so bare "mailto:" style opaque + // forms and relative references are rejected. + if !u.IsAbs() { + return false + } + if u.Host == "" && !strings.HasPrefix(u.Path, "/") { + return false + } + return true +} + +// responseTypeHasBareToken reports whether the (already canonicalized, +// space-separated) response_type includes the exact component "token" — i.e. +// the flow delivers a bearer access token into the front channel (URL +// fragment). It splits on whitespace and compares each component exactly, so +// "id_token" (which contains "token" only as a substring) is NOT matched. +// Matches: "token", "code token", "id_token token", "code id_token token". +func responseTypeHasBareToken(responseType string) bool { + for _, c := range strings.Fields(responseType) { + if c == constants.ResponseTypeToken { + return true + } + } + return false +} + // validateAuthorizeRequest validates the authorize request parameters and // returns RFC 6749 §4.1.2.1 compliant error code and description on failure. // Returns empty strings when validation passes. diff --git a/internal/http_handlers/openid_config.go b/internal/http_handlers/openid_config.go index 29263d34..c0328903 100644 --- a/internal/http_handlers/openid_config.go +++ b/internal/http_handlers/openid_config.go @@ -36,6 +36,13 @@ func (h *httpProvider) OpenIDConfigurationHandler() gin.HandlerFunc { // configured a backchannel_logout_uri — avoids lying to RPs. backchannelSupported := h.Config.BackchannelLogoutURI != "" + // OAuth 2.1 strict mode drops PKCE "plain" (S256 only). RFC 7636 §4.2 + // otherwise requires advertising both. + codeChallengeMethods := []string{"S256", "plain"} + if h.Config.OAuth21Strict { + codeChallengeMethods = []string{"S256"} + } + resp := gin.H{ // REQUIRED fields (OIDC Discovery §3) "issuer": issuer, @@ -62,8 +69,11 @@ func (h *httpProvider) OpenIDConfigurationHandler() gin.HandlerFunc { // code_verifier (PKCE) or client_secret — "none" without PKCE is // rejected with invalid_request. "private_key_jwt" advertises the // RFC 7523 client_assertion (JWT-bearer) workload-identity path. - "token_endpoint_auth_methods_supported": []string{"client_secret_basic", "client_secret_post", "none", "private_key_jwt"}, - "code_challenge_methods_supported": []string{"S256", "plain"}, + "token_endpoint_auth_methods_supported": []string{"client_secret_basic", "client_secret_post", "none", "private_key_jwt"}, + "code_challenge_methods_supported": codeChallengeMethods, + // RFC 8707 resource indicators are honored on the authorization_code + // flow (resource query param → access token aud) and token-exchange. + "resource_indicators_supported": true, "revocation_endpoint": issuer + "/oauth/revoke", "revocation_endpoint_auth_methods_supported": []string{"client_secret_basic", "client_secret_post"}, "introspection_endpoint": issuer + "/oauth/introspect", diff --git a/internal/http_handlers/token.go b/internal/http_handlers/token.go index 6fb793b4..c36198fd 100644 --- a/internal/http_handlers/token.go +++ b/internal/http_handlers/token.go @@ -4,6 +4,7 @@ import ( "crypto/sha256" "crypto/subtle" "encoding/base64" + "encoding/json" "errors" "net/http" "net/url" @@ -64,6 +65,128 @@ func formURLDecodeOrKeep(s string) string { return s } +// refreshFamilyKeyPrefix namespaces the per-lineage reuse-detection index in +// the session store. The full sub-key is refreshFamilyKeyPrefix+, +// stored under the same session key (":") as the token +// entries. It can never collide with a token entry (those are +// "_", e.g. "refresh_token_") because the literal prefix +// differs ("refresh_family_" vs "refresh_token_"). DeleteAllUserSessions still +// sweeps it (substring/glob match on the user id), so logout/reset clear it. +const refreshFamilyKeyPrefix = "refresh_family_" + +// refreshReuseGraceWindow is how long after a rotation a replay of the +// just-rotated-away (immediate-predecessor) token is treated as a benign +// double-submit (multi-tab SPA, network retry) rather than a breach. Kept +// short: long enough to absorb a client retrying with the token it already +// swapped, short enough that a stolen predecessor replayed later still trips +// revocation. +// ponytail: fixed 10s window; make it a CLI flag only if a real client needs longer. +const refreshReuseGraceWindow = 10 * time.Second + +// refreshFamilyRecord is the reuse-detection index for one refresh-token +// lineage. LiveNonce is the current live token's nonce (the one revoked on a +// genuine reuse); PrevNonce is the nonce it was just rotated from (the benign +// double-submit candidate); RotatedAt bounds the grace window. +type refreshFamilyRecord struct { + LiveNonce string `json:"live"` + PrevNonce string `json:"prev"` + RotatedAt int64 `json:"rotated_at"` +} + +// setRefreshFamilyRecord persists (overwrites) the family index for a lineage. +// Best-effort: a failure only degrades reuse detection for this family, it must +// not fail the token issuance the caller just completed. +func (h *httpProvider) setRefreshFamilyRecord(sessionKey, familyID string, rec refreshFamilyRecord, expiresAt int64) { + b, err := json.Marshal(rec) + if err != nil { + return + } + if err := h.MemoryStoreProvider.SetUserSession(sessionKey, refreshFamilyKeyPrefix+familyID, string(b), expiresAt); err != nil { + h.Log.Debug().Err(err).Str("session_key", sessionKey).Msg("Failed to persist refresh family record") + } +} + +// handleRefreshTokenReuse applies OAuth 2.1 §6.1 / RFC 9700 §4.14.2 reuse +// revocation, scoped to the compromised refresh token's own lineage (family) +// rather than the whole user. This closes the unauthenticated forced-logout +// DoS: replaying any single retired token can no longer wipe a user's other +// sessions/login-methods. It also applies the benign double-submit grace +// window so a legitimate multi-tab / retry race does not self-revoke. +func (h *httpProvider) handleRefreshTokenReuse(gc *gin.Context, claims map[string]interface{}) { + log := h.Log.With().Str("func", "handleRefreshTokenReuse").Logger() + + sub, _ := claims["sub"].(string) + if sub == "" { + return + } + sessionKey := sub + if lm, _ := claims["login_method"].(string); lm != "" { + sessionKey = lm + ":" + sub + } + familyID, _ := claims["family_id"].(string) + replayedNonce, _ := claims["nonce"].(string) + + // Legacy tokens (issued before family tracking) carry no family_id, so there + // is no lineage to scope to. Do NOT fall back to nuking the whole user — + // that is exactly the DoS this fix removes. A family-less replay revokes + // nothing; the token is still rejected (invalid_grant) by the caller. + if familyID == "" { + metrics.RecordSecurityEvent("refresh_token_reuse_no_family", "token_endpoint") + log.Warn().Msg("refresh token reuse detected on a family-less (legacy) token — no lineage to revoke") + return + } + + recRaw, err := h.MemoryStoreProvider.GetUserSession(sessionKey, refreshFamilyKeyPrefix+familyID) + if err != nil || recRaw == "" { + // No live lineage backing this family: it was never rotated, or was + // logged out / expired. Nothing live to revoke — and crucially, no other + // session or login-method for this user is touched. + metrics.RecordSecurityEvent("refresh_token_reuse_no_live_family", "token_endpoint") + return + } + var rec refreshFamilyRecord + if err := json.Unmarshal([]byte(recRaw), &rec); err != nil { + log.Debug().Err(err).Msg("Failed to decode refresh family record") + return + } + + // The presented nonce IS the family's current live token: this is not a + // rotated-away token being replayed at all — ValidateRefreshToken's own + // session lookup for this exact live nonce must have hit a transient store + // error (a timeout, a momentary blip) rather than a genuine "key absent" + // (the store layer can't distinguish the two, see GetUserSession). Treat it + // as an ordinary failed validation, not a breach: revoking here would nuke + // the user's own still-legitimate session over a retryable fault, which is + // strictly worse than the plain invalid_grant the caller already returns. + if replayedNonce == rec.LiveNonce { + metrics.RecordSecurityEvent("refresh_token_reuse_transient", "token_endpoint") + log.Debug().Msg("presented nonce matches the family's live token — likely a transient session-store error, not reuse; not revoking") + return + } + + // Benign double-submit grace window: only when the replayed token is the + // IMMEDIATE predecessor of the current live token (rec.PrevNonce) AND the + // rotation was within the window. An immediate-predecessor replay means the + // live token has not itself been used since (had it been used, the chain + // would have advanced and rec.PrevNonce would no longer equal the replayed + // nonce) — that is a multi-tab/retry race, not theft. Any older nonce, or a + // predecessor replayed after the window, is treated as genuine reuse. + if replayedNonce == rec.PrevNonce && time.Now().Unix()-rec.RotatedAt <= int64(refreshReuseGraceWindow.Seconds()) { + metrics.RecordSecurityEvent("refresh_token_double_submit", "token_endpoint") + log.Debug().Msg("refresh token double-submit within grace window — treated as race, family not revoked") + return + } + + // Genuine reuse: revoke only this family's live session (session + access + + // refresh entries for the current live nonce). DeleteUserSession is scoped to + // this exact (sessionKey, nonce), so other lineages/login-methods survive. + if err := h.MemoryStoreProvider.DeleteUserSession(sessionKey, rec.LiveNonce); err != nil { + log.Debug().Err(err).Str("session_key", sessionKey).Msg("Failed to revoke live session on refresh token reuse") + } + metrics.RecordSecurityEvent("refresh_token_reuse", "token_endpoint") + log.Warn().Msg("refresh token reuse detected — revoked the compromised token family only") +} + // TokenHandler to handle /oauth/token requests // grant type required func (h *httpProvider) TokenHandler() gin.HandlerFunc { @@ -177,8 +300,15 @@ func (h *httpProvider) TokenHandler() gin.HandlerFunc { var roles, scope []string loginMethod := "" sessionKey := "" + resource := "" // RFC 8707 resource bound to the auth code; becomes the access token aud oidcNonce := "" // OIDC nonce from the original /authorize request authTime := int64(0) // End-User's actual last authentication (OIDC Core §2 auth_time); 0 = unknown, CreateIDToken falls back to time.Now() + // Refresh-token family tracking for reuse detection (OAuth 2.1 §6.1). + // Populated only on the refresh_token grant: refreshFamilyID is the + // lineage id carried forward into the rotated token, oldRefreshNonce is + // the nonce being rotated away (recorded as the family's prev_nonce). + refreshFamilyID := "" + oldRefreshNonce := "" if isAuthorizationCodeGrant { if code == "" { @@ -233,6 +363,28 @@ func (h *httpProvider) TokenHandler() gin.HandlerFunc { } } + // RFC 8707 resource binding: sessionDataSplit[4] holds the resource + // (url-escaped) the client bound at /authorize, empty when none was + // supplied. When bound, the token request MUST echo exactly one + // matching resource (same enforcement shape as the PKCE + // code_verifier and redirect_uri checks). When present it becomes + // the access token's `aud` (see CreateAuthToken / AuthTokenConfig). + // When unbound the request `resource` is ignored — clients that + // don't use resource indicators are unaffected. + if len(sessionDataSplit) > 4 { + resource, _ = url.QueryUnescape(sessionDataSplit[4]) + } + if resource != "" { + requestResources := gc.PostFormArray("resource") + if len(requestResources) != 1 || strings.TrimSpace(requestResources[0]) != resource { + gc.JSON(http.StatusBadRequest, gin.H{ + "error": "invalid_grant", + "error_description": "The resource parameter does not match the authorization request", + }) + return + } + } + // Parse code_challenge and method from stored state. // Format: "challenge::method" or just "challenge" (legacy, defaults to plain per RFC 7636 §4.2) // or empty string (no PKCE — confidential client). @@ -243,6 +395,18 @@ func (h *httpProvider) TokenHandler() gin.HandlerFunc { storedChallenge = storedChallenge[:idx] } + // OAuth 2.1 strict mode: reject a "plain" PKCE code. /authorize + // already refuses to mint one under strict mode; this closes the + // window where the flag was flipped on between authorize and + // exchange for a code already in flight. + if h.Config.OAuth21Strict && storedChallenge != "" && storedMethod == "plain" { + gc.JSON(http.StatusBadRequest, gin.H{ + "error": "invalid_request", + "error_description": "code_challenge_method=plain is not allowed; use S256", + }) + return + } + // RFC 7636 §4.5: If PKCE was used at /authorize, the token request // MUST include code_verifier. This is orthogonal to client authentication. if storedChallenge != "" && codeVerifier != "" { @@ -365,7 +529,20 @@ func (h *httpProvider) TokenHandler() gin.HandlerFunc { // HTTP 400, not 401. claims, err := h.TokenProvider.ValidateRefreshToken(gc, refreshToken, resolvedClient.ClientID) if err != nil { - log.Debug().Err(err).Msg("Error validating refresh token") + // OAuth 2.1 §6.1 / RFC 9700 §4.14.2 refresh-token reuse + // response: a genuine, already-rotated refresh token was + // replayed. Revoke ONLY the compromised token's lineage + // (family) — not the whole user — so replaying one retired + // token cannot force-log-out the user's other sessions and + // login-methods (an unauthenticated DoS). handleRefreshTokenReuse + // also applies the benign double-submit grace window. + if errors.Is(err, token.ErrRefreshTokenReuse) { + h.handleRefreshTokenReuse(gc, claims) + } else { + log.Debug().Err(err).Msg("Error validating refresh token") + } + // Same opaque response either way — never reveal to the caller + // whether reuse was detected (no oracle). gc.JSON(http.StatusBadRequest, gin.H{ "error": "invalid_grant", "error_description": "The refresh token is invalid or has expired", @@ -452,6 +629,17 @@ func (h *httpProvider) TokenHandler() gin.HandlerFunc { return } + // Carry the refresh-token family forward across this rotation. A + // legacy token issued before family tracking has no family_id claim — + // mint a fresh one so the rotated token (and its family record) are + // consistent from here on. oldRefreshNonce becomes the family's + // prev_nonce, used by the reuse grace window. + refreshFamilyID, _ = claims["family_id"].(string) + if refreshFamilyID == "" { + refreshFamilyID = uuid.New().String() + } + oldRefreshNonce = nonce + // RFC 6749 §6: "the authorization server MUST verify the binding // between the refresh token and client identity whenever // possible." Enforced above via ValidateRefreshToken's audience @@ -501,15 +689,17 @@ func (h *httpProvider) TokenHandler() gin.HandlerFunc { hostname := parsers.GetHost(gc) nonce := uuid.New().String() authToken, err := h.TokenProvider.CreateAuthToken(gc, &token.AuthTokenConfig{ - User: user, - Roles: roles, - Scope: scope, - LoginMethod: loginMethod, - Nonce: nonce, - OIDCNonce: oidcNonce, - HostName: hostname, - AuthTime: authTime, - ClientID: resolvedClient.ClientID, + User: user, + Roles: roles, + Scope: scope, + LoginMethod: loginMethod, + Nonce: nonce, + OIDCNonce: oidcNonce, + HostName: hostname, + AuthTime: authTime, + ClientID: resolvedClient.ClientID, + Resource: resource, + RefreshTokenFamilyID: refreshFamilyID, }) if err != nil { log.Debug().Err(err).Msg("Error creating auth token") @@ -572,6 +762,20 @@ func (h *httpProvider) TokenHandler() gin.HandlerFunc { }) return } + // Record/refresh the family index on rotation so reuse detection can + // (a) find and revoke only this lineage's live session and (b) tell a + // benign double-submit of the just-rotated token from genuine theft. + // Only written on the refresh grant — first issuance needs no record + // (reuse can only be detected after a rotation or expiry, and a + // replay of a never-rotated / logged-out token has no live lineage to + // protect, which is exactly when the old whole-user nuke did harm). + if isRefreshTokenGrant && refreshFamilyID != "" { + h.setRefreshFamilyRecord(sessionKey, refreshFamilyID, refreshFamilyRecord{ + LiveNonce: authToken.FingerPrint, + PrevNonce: oldRefreshNonce, + RotatedAt: time.Now().Unix(), + }, authToken.RefreshToken.ExpiresAt) + } } if isRefreshTokenGrant { metrics.RecordAuthEvent(metrics.EventTokenRefresh, metrics.StatusSuccess) diff --git a/internal/integration_tests/oauth21_hardening_test.go b/internal/integration_tests/oauth21_hardening_test.go new file mode 100644 index 00000000..fcfac26b --- /dev/null +++ b/internal/integration_tests/oauth21_hardening_test.go @@ -0,0 +1,467 @@ +package integration_tests + +import ( + "context" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authorizerdev/authorizer/internal/constants" + "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/token" +) + +// refreshFamilyKeyPrefix mirrors the (unexported) constant in +// internal/http_handlers/token.go. The reuse-detection family record for a +// lineage is stored under ":" at this sub-key + +// . Tests reach into it to simulate elapsed time for the grace +// window without sleeping. +const refreshFamilyKeyPrefix = "refresh_family_" + +// offlineAccessSessionForEmail logs an ALREADY-signed-up user in for a fresh +// offline_access session and returns a router plus an authorization code + +// PKCE verifier ready to exchange. Calling it twice for the same email yields +// two independent refresh-token lineages (families) under the same session key +// — the setup needed to prove reuse revocation is scoped to one family. +func offlineAccessSessionForEmail(t *testing.T, ts *testSetup, clientID, email string) (http.Handler, string, string) { + t.Helper() + _, ctx := createContext(ts) + + user, err := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, err) + + nonce := uuid.New().String() + scope := []string{"openid", "profile", "email", "offline_access"} + sessionData, sessionToken, sessionExpiresAt, err := ts.TokenProvider.CreateSessionToken(&token.AuthTokenConfig{ + User: user, + Nonce: nonce, + Roles: ts.Config.DefaultRoles, + Scope: scope, + LoginMethod: constants.AuthRecipeMethodBasicAuth, + }) + require.NoError(t, err) + + sessionKey := constants.AuthRecipeMethodBasicAuth + ":" + user.ID + require.NoError(t, ts.MemoryStoreProvider.SetUserSession( + sessionKey, constants.TokenTypeSessionToken+"_"+sessionData.Nonce, sessionToken, sessionExpiresAt)) + + verifier := uuid.New().String() + uuid.New().String() + sum := sha256.Sum256([]byte(verifier)) + challenge := base64.RawURLEncoding.EncodeToString(sum[:]) + + router := gin.New() + router.GET("/authorize", ts.HttpProvider.AuthorizeHandler()) + router.POST("/oauth/token", ts.HttpProvider.TokenHandler()) + + qs := url.Values{} + qs.Set("client_id", clientID) + qs.Set("response_type", "code") + qs.Set("redirect_uri", "http://localhost:3000/callback") + qs.Set("code_challenge", challenge) + qs.Set("code_challenge_method", "S256") + qs.Set("state", uuid.New().String()) + qs.Set("response_mode", "query") + qs.Set("scope", strings.Join(scope, " ")) + + code := codeFromRedirect(t, doAuthorizeGET(router, qs, sessionToken)) + return router, code, verifier +} + +func signupUser(t *testing.T, ts *testSetup) string { + t.Helper() + _, ctx := createContext(ts) + email := "hardening_" + uuid.New().String() + "@authorizer.dev" + password := "Password@123" + _, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + Email: &email, + Password: password, + ConfirmPassword: password, + }) + require.NoError(t, err) + return email +} + +func exchangeForRefresh(t *testing.T, router http.Handler, code, verifier string, basic []string) string { + t.Helper() + form := url.Values{} + form.Set("grant_type", "authorization_code") + form.Set("code", code) + form.Set("code_verifier", verifier) + form.Set("redirect_uri", "http://localhost:3000/callback") + w := exchangeCode(router, form, basic) + require.Equal(t, http.StatusOK, w.Code, "code exchange body: %s", w.Body.String()) + var body map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) + rt, _ := body["refresh_token"].(string) + require.NotEmpty(t, rt, "offline_access must yield a refresh_token") + return rt +} + +func doRefresh(router http.Handler, rt string, basic []string) *httptest.ResponseRecorder { + f := url.Values{} + f.Set("grant_type", "refresh_token") + f.Set("refresh_token", rt) + return exchangeCode(router, f, basic) +} + +func refreshOK(t *testing.T, router http.Handler, rt string, basic []string) string { + t.Helper() + w := doRefresh(router, rt, basic) + require.Equal(t, http.StatusOK, w.Code, "refresh body: %s", w.Body.String()) + var body map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) + rt2, _ := body["refresh_token"].(string) + require.NotEmpty(t, rt2) + return rt2 +} + +// Item 1 (the HIGH fix): refresh-token reuse revocation MUST be scoped to the +// compromised token's own family — replaying one retired token must NOT log the +// user out of their OTHER live sessions. Before the fix, the token endpoint +// called DeleteAllUserSessions(userID), so any single leaked/retired refresh +// token was an unauthenticated forced-logout DoS across every session of that +// user. This test builds TWO independent refresh lineages for the SAME user +// (same session key), triggers genuine reuse on one, and asserts the other +// survives. +func TestOAuth21_RefreshTokenReuse_ScopedToFamily(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "family-client", "family-secret") + basic := []string{"family-client", "family-secret"} + + email := signupUser(t, ts) + + // Family A and family B: two separate logins of the same user. + routerA, codeA, verifierA := offlineAccessSessionForEmail(t, ts, "family-client", email) + familyARefresh := exchangeForRefresh(t, routerA, codeA, verifierA, basic) + + routerB, codeB, verifierB := offlineAccessSessionForEmail(t, ts, "family-client", email) + familyBRefresh := exchangeForRefresh(t, routerB, codeB, verifierB, basic) + + // Advance family A twice so its original token is genuinely stale (not the + // immediate predecessor covered by the grace window), then replay it. + aRotated := refreshOK(t, routerA, familyARefresh, basic) + aLive := refreshOK(t, routerA, aRotated, basic) + + reuse := doRefresh(routerA, familyARefresh, basic) + assert.Equal(t, http.StatusBadRequest, reuse.Code, "reuse body: %s", reuse.Body.String()) + + // Family A's live token is revoked (breach response, scoped to family A). + deadA := doRefresh(routerA, aLive, basic) + assert.Equal(t, http.StatusBadRequest, deadA.Code, + "family A live token must be revoked after reuse in family A: %s", deadA.Body.String()) + + // The DoS fix: family B — an unrelated session of the SAME user — is + // untouched and still refreshes. Under the old whole-user revocation this + // would have been wiped too. + survivor := doRefresh(routerB, familyBRefresh, basic) + assert.Equal(t, http.StatusOK, survivor.Code, + "an unrelated session of the same user must survive reuse in another family: %s", survivor.Body.String()) +} + +// Item 1 grace window: a benign double-submit (multi-tab SPA / network retry) +// replays the just-rotated-away token against its own still-live successor. +// Within the short grace window that is treated as an ordinary race — the +// replay fails invalid_grant but the live session is NOT revoked. After the +// window the SAME replay is treated as genuine reuse and revokes the family. +func TestOAuth21_RefreshTokenReuse_GraceWindow(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "grace-client", "grace-secret") + basic := []string{"grace-client", "grace-secret"} + + t.Run("within window: benign double-submit does not revoke", func(t *testing.T) { + email := signupUser(t, ts) + router, code, verifier := offlineAccessSessionForEmail(t, ts, "grace-client", email) + original := exchangeForRefresh(t, router, code, verifier, basic) + + live := refreshOK(t, router, original, basic) // rotate once: original -> live + + // Immediately replay the just-rotated-away token (the immediate + // predecessor) — a double-submit race. + replay := doRefresh(router, original, basic) + assert.Equal(t, http.StatusBadRequest, replay.Code, "the replayed token itself is invalid: %s", replay.Body.String()) + + // The live session must NOT have been revoked — it still refreshes. + stillAlive := doRefresh(router, live, basic) + assert.Equal(t, http.StatusOK, stillAlive.Code, + "a benign double-submit within the grace window must not revoke the live session: %s", stillAlive.Body.String()) + }) + + t.Run("after window: same replay is genuine reuse and revokes", func(t *testing.T) { + email := signupUser(t, ts) + router, code, verifier := offlineAccessSessionForEmail(t, ts, "grace-client", email) + original := exchangeForRefresh(t, router, code, verifier, basic) + + live := refreshOK(t, router, original, basic) // rotate once: original -> live + + // Simulate the grace window having elapsed by ageing the family record's + // rotated_at far into the past (no sleep). familyID + owner come from the + // live refresh token's claims. + ageFamilyRecordPast(t, ts, live) + + replay := doRefresh(router, original, basic) + assert.Equal(t, http.StatusBadRequest, replay.Code, "reuse body: %s", replay.Body.String()) + + // Now the live session IS revoked — reuse after the window is a breach. + revoked := doRefresh(router, live, basic) + assert.Equal(t, http.StatusBadRequest, revoked.Code, + "reuse of the immediate predecessor after the grace window must revoke the family: %s", revoked.Body.String()) + }) +} + +// ageFamilyRecordPast rewrites the reuse-detection family record for the given +// live refresh token so its rotated_at is far in the past, simulating an +// elapsed grace window without sleeping. +func ageFamilyRecordPast(t *testing.T, ts *testSetup, liveRefresh string) { + t.Helper() + claims := decodeJWTPayloadUnverified(t, liveRefresh) + familyID, _ := claims["family_id"].(string) + require.NotEmpty(t, familyID, "live refresh token must carry a family_id") + sub, _ := claims["sub"].(string) + loginMethod, _ := claims["login_method"].(string) + sessionKey := sub + if loginMethod != "" { + sessionKey = loginMethod + ":" + sub + } + + raw, err := ts.MemoryStoreProvider.GetUserSession(sessionKey, refreshFamilyKeyPrefix+familyID) + require.NoError(t, err) + require.NotEmpty(t, raw) + var rec map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(raw), &rec)) + rec["rotated_at"] = int64(1) // far in the past + out, err := json.Marshal(rec) + require.NoError(t, err) + // Keep it live in the store (far-future expiry) so the reuse path finds it. + require.NoError(t, ts.MemoryStoreProvider.SetUserSession( + sessionKey, refreshFamilyKeyPrefix+familyID, string(out), 1<<62)) +} + +// A transient failure looking up the CURRENT live refresh token's own session +// record (a store timeout/blip — GetUserSession cannot distinguish that from +// "key genuinely absent") must NOT be treated as reuse. Simulated here by +// deleting the live token's session entry directly (without a second +// rotation), so the presented nonce still equals the family record's +// LiveNonce — the exact signal handleRefreshTokenReuse uses to recognize +// "this is not a rotated-away token, something else went wrong" and skip +// revocation. +// +// One real rotation happens first and is load-bearing for the test, not +// incidental: a family record only exists once a refresh has actually +// happened (the initial authorization_code exchange writes none), so the +// LiveNonce-guard path can't be reached at all without it. +func TestOAuth21_RefreshTokenReuse_TransientLookupFailureNotRevoked(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "transient-client", "transient-secret") + basic := []string{"transient-client", "transient-secret"} + + email := signupUser(t, ts) + router, code, verifier := offlineAccessSessionForEmail(t, ts, "transient-client", email) + original := exchangeForRefresh(t, router, code, verifier, basic) + live := refreshOK(t, router, original, basic) // rotate once: establishes the family record + + claims := decodeJWTPayloadUnverified(t, live) + nonce, _ := claims["nonce"].(string) + sub, _ := claims["sub"].(string) + loginMethod, _ := claims["login_method"].(string) + require.NotEmpty(t, nonce) + sessionKey := sub + if loginMethod != "" { + sessionKey = loginMethod + ":" + sub + } + + // Simulate the transient blip: ONLY the live token's own refresh_token_ + // session entry vanishes out from under it (a store timeout mid-lookup) — + // nothing rotated, so this is still the family's LiveNonce. Overwrite just + // that one sub-key's expiry to the past (the db-backed store's + // GetUserSession treats an expired row as "not found" and deletes it on + // read) rather than DeleteUserSession, which cascades to session_token_/ + // access_token_/refresh_token_ TOGETHER for the nonce and would erase the + // very sibling entry this test needs intact to prove the point below. + refreshTokenKey := "refresh_token_" + nonce + accessTokenKey := "access_token_" + nonce + require.NoError(t, ts.MemoryStoreProvider.SetUserSession(sessionKey, refreshTokenKey, live, 1)) + preAccess, err := ts.MemoryStoreProvider.GetUserSession(sessionKey, accessTokenKey) + require.NoError(t, err) + require.NotEmpty(t, preAccess, "test setup: the access token entry must still be present before the replay") + + // Presenting the (now-orphaned-but-still-live-per-the-family-record) token + // fails validation as an ordinary invalid_grant... + replay := doRefresh(router, live, basic) + assert.Equal(t, http.StatusBadRequest, replay.Code, "replay body: %s", replay.Body.String()) + + // ...but must NOT have been treated as a breach. If handleRefreshTokenReuse + // incorrectly ran the genuine-reuse branch, DeleteUserSession(sessionKey, + // rec.LiveNonce) would have wiped this SAME nonce's access token too — even + // though nothing about it actually rotated or was stolen. + postAccess, err := ts.MemoryStoreProvider.GetUserSession(sessionKey, accessTokenKey) + require.NoError(t, err) + assert.NotEmpty(t, postAccess, + "a transient lookup failure on the live token must not revoke the live session (access token entry was wiped)") +} + +// Item 2 (MEDIUM): --oauth2-1-strict must reject EVERY response_type that +// delivers a bearer access token into the URL fragment, including the +// front-channel-token hybrids "code token" and "code id_token token" — not +// only "token" / "id_token token". The check is component-exact so pure +// "id_token" (which contains "token" only as a substring) is unaffected. +func TestOAuth21_StrictMode_RejectsFrontChannelTokenHybrids(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "hybrid-client", "hybrid-secret") + + authorize := func(responseType string) *httptest.ResponseRecorder { + router, cookie := mcpSession(t, ts, []string{"openid", "profile", "email"}) + qs := url.Values{} + qs.Set("client_id", "hybrid-client") + qs.Set("response_type", responseType) + qs.Set("redirect_uri", "http://localhost:3000/callback") + qs.Set("state", uuid.New().String()) + qs.Set("response_mode", "fragment") + qs.Set("nonce", uuid.New().String()) // required whenever id_token is present + qs.Set("scope", "openid profile email") + return doAuthorizeGET(router, qs, cookie) + } + + rejected := []string{"code token", "code id_token token", "token", "id_token token"} + for _, rt := range rejected { + t.Run("strict rejects "+rt, func(t *testing.T) { + ts.Config.OAuth21Strict = true + w := authorize(rt) + require.Equal(t, http.StatusFound, w.Code, "body: %s", w.Body.String()) + loc := w.Header().Get("Location") + assert.Contains(t, loc, "unsupported_response_type", "loc: %s", loc) + assert.NotContains(t, loc, "access_token=", "no access token may leak into the fragment: %s", loc) + }) + } + + // Pure id_token (contains "token" only as a substring) is NOT a + // front-channel access-token flow — strict mode must still allow it. + t.Run("strict allows pure id_token", func(t *testing.T) { + ts.Config.OAuth21Strict = true + w := authorize("id_token") + require.Equal(t, http.StatusFound, w.Code, "body: %s", w.Body.String()) + loc := w.Header().Get("Location") + assert.NotContains(t, loc, "unsupported_response_type", "pure id_token must not be rejected by strict mode: %s", loc) + }) + + ts.Config.OAuth21Strict = false +} + +// Item 3 (LOW): RFC 8707 §2 requires the resource indicator to be an absolute +// URI with no fragment. An invalid resource is rejected at /authorize with the +// RFC-conventional invalid_target error code. +func TestOAuth21_ResourceIndicator_Validation(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "res-client", "res-secret") + + authorizeWithResource := func(resource string) *httptest.ResponseRecorder { + router, cookie := mcpSession(t, ts, []string{"openid", "profile", "email"}) + verifier := uuid.New().String() + uuid.New().String() + sum := sha256.Sum256([]byte(verifier)) + challenge := base64.RawURLEncoding.EncodeToString(sum[:]) + qs := url.Values{} + qs.Set("client_id", "res-client") + qs.Set("response_type", "code") + qs.Set("redirect_uri", "http://localhost:3000/callback") + qs.Set("code_challenge", challenge) + qs.Set("code_challenge_method", "S256") + qs.Set("state", uuid.New().String()) + qs.Set("response_mode", "query") + qs.Set("scope", "openid profile email") + qs.Set("resource", resource) + return doAuthorizeGET(router, qs, cookie) + } + + invalidTargetRejected := func(t *testing.T, w *httptest.ResponseRecorder) { + t.Helper() + require.Equal(t, http.StatusFound, w.Code, "body: %s", w.Body.String()) + loc, err := url.Parse(w.Header().Get("Location")) + require.NoError(t, err) + assert.Equal(t, "invalid_target", loc.Query().Get("error"), + "invalid resource must be rejected with invalid_target: %s", w.Header().Get("Location")) + } + + t.Run("relative reference rejected", func(t *testing.T) { + invalidTargetRejected(t, authorizeWithResource("/api/resource")) + }) + t.Run("non-URI string rejected", func(t *testing.T) { + invalidTargetRejected(t, authorizeWithResource("not-a-uri")) + }) + t.Run("fragment rejected", func(t *testing.T) { + invalidTargetRejected(t, authorizeWithResource("https://mcp.example.com/api#frag")) + }) + + t.Run("valid absolute URI accepted", func(t *testing.T) { + w := authorizeWithResource("https://mcp.example.com/api") + code := codeFromRedirect(t, w) // redirects with a code, not an error + require.NotEmpty(t, code) + }) +} + +// Item 4 (LOW/MEDIUM): RFC 8707 audience restriction must be enforced at +// Authorizer's OWN protected endpoints. A token minted for an external resource +// (aud = resource URI) must be rejected at /userinfo, while a normal +// client-bound token still works. Introspection keeps its own path and is +// unaffected (see /oauth/introspect — it uses ParseJWTToken directly). +func TestOAuth21_ResourceBoundToken_RejectedAtUserInfo(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) + + email := signupUser(t, ts) + + // A resource-bound access token (aud = external resource URI). + resourceToken := issueAccessTokenWithResource(t, ts, ctx, email, "https://mcp.example.com") + assert.Equal(t, "https://mcp.example.com", decodeJWTPayloadUnverified(t, resourceToken)["aud"], + "sanity: token aud must be the resource") + code, _ := callUserInfo(t, ts, resourceToken) + assert.Equal(t, http.StatusUnauthorized, code, + "a resource-bound token must not authenticate at authorizer's own /userinfo") + + // A normal client-bound token (aud = default audience) still works. + clientToken := issueAccessTokenWithScopes(t, ts, ctx, email, []string{"openid", "profile", "email"}) + code2, body := callUserInfo(t, ts, clientToken) + assert.Equal(t, http.StatusOK, code2, "a client-bound token must still work at /userinfo: %v", body) +} + +// issueAccessTokenWithResource mints and persists an access token whose `aud` +// is the given RFC 8707 resource indicator, mirroring what /oauth/token does +// for a resource-bound exchange. +func issueAccessTokenWithResource(t *testing.T, ts *testSetup, ctx context.Context, email, resource string) string { + t.Helper() + user, err := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, err) + nonce := "nonce-" + uuid.New().String() + authToken, err := ts.TokenProvider.CreateAuthToken(nil, &token.AuthTokenConfig{ + User: user, + Roles: []string{"user"}, + Scope: []string{"openid", "profile", "email"}, + LoginMethod: constants.AuthRecipeMethodBasicAuth, + Nonce: nonce, + HostName: "http://localhost", + Resource: resource, + }) + require.NoError(t, err) + require.NotNil(t, authToken.AccessToken) + + sessionStoreKey := constants.AuthRecipeMethodBasicAuth + ":" + user.ID + require.NoError(t, ts.MemoryStoreProvider.SetUserSession( + sessionStoreKey, constants.TokenTypeSessionToken+"_"+authToken.FingerPrint, authToken.FingerPrintHash, authToken.SessionTokenExpiresAt)) + require.NoError(t, ts.MemoryStoreProvider.SetUserSession( + sessionStoreKey, constants.TokenTypeAccessToken+"_"+authToken.FingerPrint, authToken.AccessToken.Token, authToken.AccessToken.ExpiresAt)) + return authToken.AccessToken.Token +} diff --git a/internal/integration_tests/oauth21_mcp_test.go b/internal/integration_tests/oauth21_mcp_test.go new file mode 100644 index 00000000..9a5d8c96 --- /dev/null +++ b/internal/integration_tests/oauth21_mcp_test.go @@ -0,0 +1,383 @@ +package integration_tests + +import ( + "crypto/sha256" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/gin-gonic/gin" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authorizerdev/authorizer/internal/constants" + "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/token" +) + +// mcpSession signs up a fresh user, registers a browser session, and returns a +// router with /authorize + /oauth/token mounted plus the session cookie value. +// Tests build their own /authorize query (response_type, resource, PKCE method) +// on top of it. +func mcpSession(t *testing.T, ts *testSetup, scope []string) (http.Handler, string) { + t.Helper() + _, ctx := createContext(ts) + + email := "mcp_" + uuid.New().String() + "@authorizer.dev" + password := "Password@123" + _, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + Email: &email, + Password: password, + ConfirmPassword: password, + }) + require.NoError(t, err) + + user, err := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, err) + + nonce := uuid.New().String() + sessionData, sessionToken, sessionExpiresAt, err := ts.TokenProvider.CreateSessionToken(&token.AuthTokenConfig{ + User: user, + Nonce: nonce, + Roles: ts.Config.DefaultRoles, + Scope: scope, + LoginMethod: constants.AuthRecipeMethodBasicAuth, + }) + require.NoError(t, err) + + sessionKey := constants.AuthRecipeMethodBasicAuth + ":" + user.ID + require.NoError(t, ts.MemoryStoreProvider.SetUserSession( + sessionKey, constants.TokenTypeSessionToken+"_"+sessionData.Nonce, sessionToken, sessionExpiresAt)) + + router := gin.New() + router.GET("/authorize", ts.HttpProvider.AuthorizeHandler()) + router.POST("/oauth/token", ts.HttpProvider.TokenHandler()) + return router, sessionToken +} + +func doAuthorizeGET(router http.Handler, qs url.Values, sessionToken string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/authorize?"+qs.Encode(), nil) + req.AddCookie(&http.Cookie{Name: constants.AppCookieName + "_session", Value: sessionToken}) + router.ServeHTTP(w, req) + return w +} + +func codeFromRedirect(t *testing.T, w *httptest.ResponseRecorder) string { + t.Helper() + require.Equal(t, http.StatusFound, w.Code, "authorize should redirect: %s", w.Body.String()) + loc, err := url.Parse(w.Header().Get("Location")) + require.NoError(t, err) + code := loc.Query().Get("code") + require.NotEmpty(t, code, "authorization code must be present") + return code +} + +// Item 2: RFC 8707 resource indicator on the primary authorization_code flow. +// A resource bound at /authorize must become the access token's `aud`, while +// the id_token audience stays the requesting client. +func TestOAuth21_ResourceIndicator_BindsAccessTokenAudience(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "mcp-client", "mcp-secret") + + const resource = "https://mcp.example.com" + scope := []string{"openid", "profile", "email"} + + verifier := uuid.New().String() + uuid.New().String() + sum := sha256.Sum256([]byte(verifier)) + challenge := base64.RawURLEncoding.EncodeToString(sum[:]) + + router, cookie := mcpSession(t, ts, scope) + + qs := url.Values{} + qs.Set("client_id", "mcp-client") + qs.Set("response_type", "code") + qs.Set("redirect_uri", "http://localhost:3000/callback") + qs.Set("code_challenge", challenge) + qs.Set("code_challenge_method", "S256") + qs.Set("state", uuid.New().String()) + qs.Set("response_mode", "query") + qs.Set("scope", "openid profile email") + qs.Set("resource", resource) + + code := codeFromRedirect(t, doAuthorizeGET(router, qs, cookie)) + + form := url.Values{} + form.Set("grant_type", "authorization_code") + form.Set("code", code) + form.Set("code_verifier", verifier) + form.Set("redirect_uri", "http://localhost:3000/callback") + form.Set("resource", resource) + + w := exchangeCode(router, form, []string{"mcp-client", "mcp-secret"}) + require.Equal(t, http.StatusOK, w.Code, "code exchange body: %s", w.Body.String()) + + var body map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) + + accessToken, _ := body["access_token"].(string) + require.NotEmpty(t, accessToken) + assert.Equal(t, resource, decodeJWTPayloadUnverified(t, accessToken)["aud"], + "access_token aud must be the bound resource (RFC 8707)") + + // The id_token audience must remain the OAuth client (OIDC), not the resource. + idToken, _ := body["id_token"].(string) + require.NotEmpty(t, idToken) + assert.Equal(t, "mcp-client", decodeJWTPayloadUnverified(t, idToken)["aud"], + "id_token aud must stay the requesting client, not the resource") +} + +// Item 2: a resource bound at /authorize must be echoed and matched at +// /oauth/token — a mismatch (or an omitted resource) is rejected invalid_grant, +// exactly like a PKCE code_verifier mismatch. +func TestOAuth21_ResourceIndicator_MismatchRejected(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "mcp-client", "mcp-secret") + + scope := []string{"openid", "profile", "email"} + + authorizeForCode := func() (http.Handler, string, string) { + verifier := uuid.New().String() + uuid.New().String() + sum := sha256.Sum256([]byte(verifier)) + challenge := base64.RawURLEncoding.EncodeToString(sum[:]) + router, cookie := mcpSession(t, ts, scope) + qs := url.Values{} + qs.Set("client_id", "mcp-client") + qs.Set("response_type", "code") + qs.Set("redirect_uri", "http://localhost:3000/callback") + qs.Set("code_challenge", challenge) + qs.Set("code_challenge_method", "S256") + qs.Set("state", uuid.New().String()) + qs.Set("response_mode", "query") + qs.Set("scope", "openid profile email") + qs.Set("resource", "https://mcp.example.com") + return router, codeFromRedirect(t, doAuthorizeGET(router, qs, cookie)), verifier + } + + baseForm := func(code, verifier string) url.Values { + form := url.Values{} + form.Set("grant_type", "authorization_code") + form.Set("code", code) + form.Set("code_verifier", verifier) + form.Set("redirect_uri", "http://localhost:3000/callback") + return form + } + + t.Run("wrong resource rejected", func(t *testing.T) { + router, code, verifier := authorizeForCode() + form := baseForm(code, verifier) + form.Set("resource", "https://evil.example.com") + w := exchangeCode(router, form, []string{"mcp-client", "mcp-secret"}) + assert.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String()) + var errBody map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &errBody)) + assert.Equal(t, "invalid_grant", errBody["error"]) + }) + + t.Run("omitted resource rejected", func(t *testing.T) { + router, code, verifier := authorizeForCode() + form := baseForm(code, verifier) // no resource echoed + w := exchangeCode(router, form, []string{"mcp-client", "mcp-secret"}) + assert.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String()) + var errBody map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &errBody)) + assert.Equal(t, "invalid_grant", errBody["error"]) + }) +} + +// Item 1: OAuth 2.1 refresh-token rotation with reuse invalidation, scoped to +// the compromised token's own lineage (family). Genuine reuse — replaying a +// stale token AFTER the chain has advanced past it, so it is not the immediate +// predecessor caught by the double-submit grace window — must be rejected +// invalid_grant AND revoke the family's live token. (That reuse revokes ONLY +// this family and not the user's other sessions is proved by +// TestOAuth21_RefreshTokenReuse_ScopedToFamily; the benign double-submit grace +// window by TestOAuth21_RefreshTokenReuse_GraceWindow.) +func TestOAuth21_RefreshTokenReuse_RevokesFamily(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "reuse-client", "reuse-secret") + + router, code, verifier := loginForOfflineAccess(t, ts, "reuse-client") + + exchangeForm := url.Values{} + exchangeForm.Set("grant_type", "authorization_code") + exchangeForm.Set("code", code) + exchangeForm.Set("code_verifier", verifier) + exchangeForm.Set("redirect_uri", "http://localhost:3000/callback") + + w := exchangeCode(router, exchangeForm, []string{"reuse-client", "reuse-secret"}) + require.Equal(t, http.StatusOK, w.Code, "code exchange body: %s", w.Body.String()) + var body map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) + oldRefresh, _ := body["refresh_token"].(string) + require.NotEmpty(t, oldRefresh, "offline_access scope must yield a refresh_token") + + refresh := func(rt string) *httptest.ResponseRecorder { + f := url.Values{} + f.Set("grant_type", "refresh_token") + f.Set("refresh_token", rt) + return exchangeCode(router, f, []string{"reuse-client", "reuse-secret"}) + } + + // Rotate TWICE so oldRefresh is no longer the immediate predecessor of the + // live token — its replay is unambiguous theft-after-advance, not a race. + w1 := refresh(oldRefresh) + require.Equal(t, http.StatusOK, w1.Code, "first refresh body: %s", w1.Body.String()) + var body1 map[string]interface{} + require.NoError(t, json.Unmarshal(w1.Body.Bytes(), &body1)) + refresh2, _ := body1["refresh_token"].(string) + require.NotEmpty(t, refresh2) + require.NotEqual(t, oldRefresh, refresh2, "rotation must mint a fresh refresh token") + + w2 := refresh(refresh2) + require.Equal(t, http.StatusOK, w2.Code, "second refresh body: %s", w2.Body.String()) + var body2 map[string]interface{} + require.NoError(t, json.Unmarshal(w2.Body.Bytes(), &body2)) + liveRefresh, _ := body2["refresh_token"].(string) + require.NotEmpty(t, liveRefresh) + + // Replay the ORIGINAL (now two rotations stale) refresh token: genuine reuse. + w3 := refresh(oldRefresh) + assert.Equal(t, http.StatusBadRequest, w3.Code, "reuse body: %s", w3.Body.String()) + var errBody map[string]interface{} + require.NoError(t, json.Unmarshal(w3.Body.Bytes(), &errBody)) + assert.Equal(t, "invalid_grant", errBody["error"]) + + // Breach response: the reuse revoked this family's live session, so the + // token that was legitimately rotated in is now dead too. + w4 := refresh(liveRefresh) + assert.Equal(t, http.StatusBadRequest, w4.Code, + "the live refresh token must be revoked after genuine reuse in its family: %s", w4.Body.String()) + var errBody4 map[string]interface{} + require.NoError(t, json.Unmarshal(w4.Body.Bytes(), &errBody4)) + assert.Equal(t, "invalid_grant", errBody4["error"]) +} + +// Item 4: the --oauth2-1-strict flag. Default (false) leaves the implicit grant +// and PKCE "plain" working exactly as today; true rejects both. +func TestOAuth21_StrictMode_Gating(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + registerTestClient(t, ts, "strict-client", "strict-secret") + + implicitQS := func() url.Values { + qs := url.Values{} + qs.Set("client_id", "strict-client") + qs.Set("response_type", "token") + qs.Set("redirect_uri", "http://localhost:3000/callback") + qs.Set("state", uuid.New().String()) + qs.Set("response_mode", "fragment") + qs.Set("scope", "openid profile email") + return qs + } + + plainPKCEQS := func(verifier string) url.Values { + qs := url.Values{} + qs.Set("client_id", "strict-client") + qs.Set("response_type", "code") + qs.Set("redirect_uri", "http://localhost:3000/callback") + // plain: code_challenge == code_verifier (RFC 7636 §4.2). + qs.Set("code_challenge", verifier) + qs.Set("code_challenge_method", "plain") + qs.Set("state", uuid.New().String()) + qs.Set("response_mode", "query") + qs.Set("scope", "openid profile email") + return qs + } + + t.Run("default false: implicit response_type=token still works", func(t *testing.T) { + ts.Config.OAuth21Strict = false + router, cookie := mcpSession(t, ts, []string{"openid", "profile", "email"}) + w := doAuthorizeGET(router, implicitQS(), cookie) + require.Equal(t, http.StatusFound, w.Code, "body: %s", w.Body.String()) + assert.Contains(t, w.Header().Get("Location"), "access_token=", + "implicit flow must return an access token in the fragment when strict is off") + }) + + t.Run("strict true: implicit response_type=token rejected", func(t *testing.T) { + ts.Config.OAuth21Strict = true + router, cookie := mcpSession(t, ts, []string{"openid", "profile", "email"}) + w := doAuthorizeGET(router, implicitQS(), cookie) + require.Equal(t, http.StatusFound, w.Code, "body: %s", w.Body.String()) + loc := w.Header().Get("Location") + assert.Contains(t, loc, "unsupported_response_type") + assert.NotContains(t, loc, "access_token=") + }) + + t.Run("default false: PKCE plain still works", func(t *testing.T) { + ts.Config.OAuth21Strict = false + verifier := uuid.New().String() + uuid.New().String() + router, cookie := mcpSession(t, ts, []string{"openid", "profile", "email"}) + code := codeFromRedirect(t, doAuthorizeGET(router, plainPKCEQS(verifier), cookie)) + + form := url.Values{} + form.Set("grant_type", "authorization_code") + form.Set("code", code) + form.Set("code_verifier", verifier) + form.Set("redirect_uri", "http://localhost:3000/callback") + w := exchangeCode(router, form, []string{"strict-client", "strict-secret"}) + require.Equal(t, http.StatusOK, w.Code, "plain PKCE exchange body: %s", w.Body.String()) + }) + + t.Run("strict true: PKCE plain rejected at authorize", func(t *testing.T) { + ts.Config.OAuth21Strict = true + verifier := uuid.New().String() + uuid.New().String() + router, cookie := mcpSession(t, ts, []string{"openid", "profile", "email"}) + w := doAuthorizeGET(router, plainPKCEQS(verifier), cookie) + assert.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String()) + assert.Contains(t, w.Body.String(), "invalid_request") + assert.Contains(t, w.Body.String(), "plain") + }) + + // Reset so a shared cfg pointer can't leak strict mode into later tests. + ts.Config.OAuth21Strict = false +} + +// Item 3 / Item 4: the RFC 8414 metadata handler advertises resource indicators +// and reflects strict-mode PKCE. The route alias itself is a one-line wrapper of +// this same handler (see internal/server/http_routes.go). +func TestOAuth21_AuthServerMetadata(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + + router := gin.New() + router.GET("/.well-known/oauth-authorization-server", ts.HttpProvider.OpenIDConfigurationHandler()) + + fetch := func() map[string]interface{} { + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/.well-known/oauth-authorization-server", nil) + router.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + var doc map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &doc)) + return doc + } + + methodsOf := func(doc map[string]interface{}) []string { + raw, _ := doc["code_challenge_methods_supported"].([]interface{}) + out := make([]string, 0, len(raw)) + for _, m := range raw { + out = append(out, m.(string)) + } + return out + } + + ts.Config.OAuth21Strict = false + doc := fetch() + assert.Equal(t, true, doc["resource_indicators_supported"], + "metadata must advertise resource_indicators_supported") + assert.ElementsMatch(t, []string{"S256", "plain"}, methodsOf(doc)) + + ts.Config.OAuth21Strict = true + assert.ElementsMatch(t, []string{"S256"}, methodsOf(fetch()), + "strict mode must advertise S256 only") + + ts.Config.OAuth21Strict = false +} diff --git a/internal/server/http_routes.go b/internal/server/http_routes.go index 62b0c0d6..394ee112 100644 --- a/internal/server/http_routes.go +++ b/internal/server/http_routes.go @@ -94,6 +94,10 @@ func (s *server) NewRouter() *gin.Engine { router.GET("/api/v1/org-discovery", s.Dependencies.HTTPProvider.OrgDiscoveryHandler()) // OPEN ID routes router.GET("/.well-known/openid-configuration", s.Dependencies.HTTPProvider.OpenIDConfigurationHandler()) + // RFC 8414 OAuth 2.0 Authorization Server Metadata. MCP clients probe this + // path first; the OIDC discovery document already satisfies RFC 8414's MUSTs, + // so serve the same handler as a thin alias (cheap interop win). + router.GET("/.well-known/oauth-authorization-server", s.Dependencies.HTTPProvider.OpenIDConfigurationHandler()) router.GET("/.well-known/jwks.json", s.Dependencies.HTTPProvider.JWKsHandler()) // RFC 6749 §3.1 / OIDC Core §3.1.2.1: the authorization endpoint MUST // support GET and MAY support POST. diff --git a/internal/service/auth_response.go b/internal/service/auth_response.go index c0074a90..8f36dfa9 100644 --- a/internal/service/auth_response.go +++ b/internal/service/auth_response.go @@ -36,6 +36,7 @@ func (p *provider) issueAuthResponse(ctx context.Context, meta RequestMetadata, nonce := "" oidcNonce := "" authorizeRedirectURI := "" + authorizeResource := "" if state != nil { authorizeState, _ := p.MemoryStoreProvider.GetState(refs.StringValue(state)) if authorizeState != "" { @@ -49,6 +50,10 @@ func (p *provider) issueAuthResponse(ctx context.Context, meta RequestMetadata, if len(authorizeStateSplit) > 3 { authorizeRedirectURI = authorizeStateSplit[3] } + // RFC 8707 resource (url-escaped) bound at /authorize, rebound to the code below. + if len(authorizeStateSplit) > 4 { + authorizeResource = authorizeStateSplit[4] + } } else { nonce = authorizeState } @@ -76,7 +81,7 @@ func (p *provider) issueAuthResponse(ctx context.Context, meta RequestMetadata, // Code challenge could be optional if PKCE flow is not used if code != "" { - if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI); err != nil { + if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI+"@@"+authorizeResource); err != nil { log.Debug().Err(err).Msg("Failed to set state") return nil, err } diff --git a/internal/service/login.go b/internal/service/login.go index 5c5864b1..10fb0023 100644 --- a/internal/service/login.go +++ b/internal/service/login.go @@ -491,6 +491,7 @@ func (p *provider) Login(ctx context.Context, meta RequestMetadata, params *mode nonce := "" oidcNonce := "" authorizeRedirectURI := "" + authorizeResource := "" if params.State != nil { // Get state from store authorizeState, _ := p.MemoryStoreProvider.GetState(refs.StringValue(params.State)) @@ -506,6 +507,10 @@ func (p *provider) Login(ctx context.Context, meta RequestMetadata, params *mode if len(authorizeStateSplit) > 3 { authorizeRedirectURI = authorizeStateSplit[3] } + // RFC 8707 resource (url-escaped) bound at /authorize, rebound to the code below. + if len(authorizeStateSplit) > 4 { + authorizeResource = authorizeStateSplit[4] + } } else { nonce = authorizeState } @@ -540,7 +545,7 @@ func (p *provider) Login(ctx context.Context, meta RequestMetadata, params *mode // Code challenge could be optional if PKCE flow is not used if code != "" { - if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI); err != nil { + if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI+"@@"+authorizeResource); err != nil { log.Debug().Msg("Failed to set state") return nil, nil, err } diff --git a/internal/service/session.go b/internal/service/session.go index 32f51cb1..9670f19f 100644 --- a/internal/service/session.go +++ b/internal/service/session.go @@ -78,6 +78,7 @@ func (p *provider) Session(ctx context.Context, meta RequestMetadata, params *mo codeChallenge := "" oidcNonce := "" authorizeRedirectURI := "" + authorizeResource := "" if params != nil && params.State != nil { authorizeState, _ := p.MemoryStoreProvider.GetState(refs.StringValue(params.State)) if authorizeState != "" { @@ -91,6 +92,10 @@ func (p *provider) Session(ctx context.Context, meta RequestMetadata, params *mo if len(parts) > 3 { authorizeRedirectURI = parts[3] } + // RFC 8707 resource (url-escaped) bound at /authorize, rebound to the code below. + if len(parts) > 4 { + authorizeResource = parts[4] + } } _ = p.MemoryStoreProvider.RemoveState(refs.StringValue(params.State)) } @@ -115,7 +120,7 @@ func (p *provider) Session(ctx context.Context, meta RequestMetadata, params *mo } if code != "" { - if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI); err != nil { + if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI+"@@"+authorizeResource); err != nil { log.Debug().Err(err).Msg("Failed to set code state") return nil, nil, err } diff --git a/internal/service/signup.go b/internal/service/signup.go index af76527b..62ce7326 100644 --- a/internal/service/signup.go +++ b/internal/service/signup.go @@ -299,6 +299,7 @@ func (p *provider) SignUp(ctx context.Context, meta RequestMetadata, params *mod nonce := "" oidcNonce := "" authorizeRedirectURI := "" + authorizeResource := "" if params.State != nil { // Get state from store authorizeState, _ := p.MemoryStoreProvider.GetState(refs.StringValue(params.State)) @@ -313,6 +314,10 @@ func (p *provider) SignUp(ctx context.Context, meta RequestMetadata, params *mod if len(authorizeStateSplit) > 3 { authorizeRedirectURI = authorizeStateSplit[3] } + // RFC 8707 resource (url-escaped) bound at /authorize, rebound to the code below. + if len(authorizeStateSplit) > 4 { + authorizeResource = authorizeStateSplit[4] + } } else { nonce = authorizeState } @@ -391,7 +396,7 @@ func (p *provider) SignUp(ctx context.Context, meta RequestMetadata, params *mod // Code challenge could be optional if PKCE flow is not used if code != "" { - if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI); err != nil { + if err := p.MemoryStoreProvider.SetState(code, codeChallenge+"@@"+authToken.FingerPrintHash+"@@"+oidcNonce+"@@"+authorizeRedirectURI+"@@"+authorizeResource); err != nil { log.Debug().Err(err).Msg("SetState failed") return nil, nil, err } diff --git a/internal/token/auth_token.go b/internal/token/auth_token.go index 2494851d..21cc6ea3 100644 --- a/internal/token/auth_token.go +++ b/internal/token/auth_token.go @@ -5,12 +5,15 @@ import ( "crypto/subtle" "encoding/base64" "encoding/json" + "errors" "fmt" + "net/url" "strings" "time" "github.com/gin-gonic/gin" "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" "github.com/robertkrimen/otto" "github.com/authorizerdev/authorizer/internal/constants" @@ -21,6 +24,16 @@ import ( "github.com/authorizerdev/authorizer/internal/utils" ) +// ErrRefreshTokenReuse signals that a genuine, correctly-signed and +// client-bound refresh token was presented but no live session record backs +// it — i.e. it was already rotated away (or the session was logged out / +// expired) and is now being replayed. OAuth 2.1 §6.1 / RFC 9700 §4.14.2 treat +// this as a compromise signal: the caller MUST revoke the token family. It is +// returned instead of a plain unauthorized so the token endpoint can trigger +// that revocation. A malformed / foreign / wrong-type token yields a plain +// unauthorized (no side effect) — only a genuine reuse trips this. +var ErrRefreshTokenReuse = errors.New("refresh token reuse detected") + // reservedClaims are security-critical JWT claims that custom scripts must not override. var reservedClaims = map[string]bool{ "sub": true, @@ -45,6 +58,10 @@ var reservedClaims = map[string]bool{ // (RFC 9068) identifies the actor and is likewise reserved. "act": true, "client_id": true, + // family_id binds a refresh token to its rotation lineage for reuse + // detection (OAuth 2.1 §6.1). A script that could forge it would let a + // stolen token masquerade as a different family and dodge revocation. + "family_id": true, } // AuthTokenConfig is the configuration for auth token @@ -84,6 +101,25 @@ type AuthTokenConfig struct { // to the same client (RFC 6749 §6). Empty is valid (client_credentials // / machine paths that never mint a refresh token). ClientID string + // Resource is the RFC 8707 resource indicator the client bound to the + // authorization request (the target MCP/API server). When non-empty it + // becomes the ACCESS token's `aud` claim so the token cannot be replayed + // at a different resource server (audience restriction). It intentionally + // does NOT change the id_token or refresh_token audience: the id_token + // audience is the client (OIDC), and the refresh_token audience is the + // client too (RFC 6749 §6 client binding). Empty preserves existing + // behavior — aud defaults to the requesting client / bootstrap client_id. + Resource string + // RefreshTokenFamilyID is the stable identifier of the refresh-token + // lineage (family). It is established once at first issuance (a fresh UUID + // when empty) and carried forward UNCHANGED across every rotation of the + // same lineage — each rotation still gets a fresh Nonce, but the family_id + // stays constant. It scopes refresh-token-reuse revocation (OAuth 2.1 §6.1 / + // RFC 9700 §4.14.2) to the compromised lineage instead of the whole user, + // so replaying one retired token cannot force-log-out a user's other + // sessions/login-methods. CreateRefreshToken embeds it as the "family_id" + // claim. Empty on non-refresh paths (client_credentials / machine tokens). + RefreshTokenFamilyID string } // loginMethodToAMR maps an internal LoginMethod value to the OIDC Core §2 @@ -246,6 +282,19 @@ func (p *provider) audience(cfg *AuthTokenConfig) string { return p.config.ClientID } +// accessTokenAudience returns the "aud" claim for an ACCESS token. When the +// client supplied an RFC 8707 resource indicator it is the audience — the +// token is bound to that resource server and is not replayable elsewhere. +// Otherwise it falls back to audience() (the requesting client), so callers +// that don't use resource indicators are unaffected. Only the access token +// uses this; id_token / refresh_token audiences remain the client. +func (p *provider) accessTokenAudience(cfg *AuthTokenConfig) string { + if cfg.Resource != "" { + return cfg.Resource + } + return p.audience(cfg) +} + // CreateRefreshToken util to create JWT token func (p *provider) CreateRefreshToken(cfg *AuthTokenConfig) (string, int64, error) { // Lifetime is configurable via --refresh-token-expires-in (seconds). @@ -260,6 +309,14 @@ func (p *provider) CreateRefreshToken(cfg *AuthTokenConfig) (string, int64, erro if authTime == 0 { authTime = time.Now().Unix() } + // Refresh-token family (OAuth 2.1 §6.1 / RFC 9700 §4.14.2). Established once + // at first issuance (fresh UUID when the caller supplies none) and carried + // forward unchanged by the refresh grant across every rotation, so reuse + // detection can revoke exactly this lineage instead of the whole user. + familyID := cfg.RefreshTokenFamilyID + if familyID == "" { + familyID = uuid.NewString() + } customClaims := jwt.MapClaims{ "iss": cfg.HostName, "aud": p.audience(cfg), @@ -274,6 +331,7 @@ func (p *provider) CreateRefreshToken(cfg *AuthTokenConfig) (string, int64, erro "login_method": cfg.LoginMethod, "allowed_roles": strings.Split(cfg.User.Roles, ","), "client_id": cfg.ClientID, + "family_id": familyID, } token, err := p.SignJWTToken(customClaims) @@ -301,7 +359,7 @@ func (p *provider) CreateAccessToken(cfg *AuthTokenConfig) (string, int64, error expiresAt := time.Now().Add(expiryBound).Unix() customClaims := jwt.MapClaims{ "iss": cfg.HostName, - "aud": p.audience(cfg), + "aud": p.accessTokenAudience(cfg), "nonce": cfg.Nonce, "sub": cfg.User.ID, "exp": expiresAt, @@ -444,6 +502,23 @@ func (p *provider) ValidateAccessToken(gc *gin.Context, accessToken string) (map // the JWT signature) as the expected value; the checks above already // establish the token is a genuine, unexpired, unrevoked Authorizer token. aud, _ := res["aud"].(string) + + // RFC 8707 audience restriction. A token minted with a resource indicator + // carries that resource (an absolute URI, e.g. "https://mcp.example.com") as + // its `aud` so it is usable ONLY at that external resource server — which + // validates it via /oauth/introspect or JWKS, NOT here. This path guards + // Authorizer's OWN protected resources (/userinfo, GraphQL, gRPC). Accepting + // a resource-bound token here would defeat the audience restriction the token + // was issued with, so reject any `aud` that is an absolute URI (resource + // indicator form) other than the configured default audience. Legitimate + // client-bound tokens carry an opaque client_id `aud` (not a URI) and are + // unaffected. Introspection uses ParseJWTToken directly and is untouched. + if aud != "" && aud != p.config.ClientID { + if u, err := url.Parse(aud); err == nil && u.IsAbs() { + p.dependencies.Log.Debug().Str("aud", aud).Msg("access token rejected: resource-bound audience not valid at authorizer's own endpoints") + return res, fmt.Errorf(`unauthorized: token audience is a resource indicator`) + } + } hostname := parsers.GetHost(gc) if ok, err := p.ValidateJWTClaims(res, &AuthTokenConfig{ HostName: hostname, @@ -490,6 +565,22 @@ func (p *provider) ValidateRefreshToken(gc *gin.Context, refreshToken string, ex token, err := p.dependencies.MemoryStoreProvider.GetUserSession(sessionKey, constants.TokenTypeRefreshToken+"_"+nonce) if nonce == "" || err != nil { p.dependencies.Log.Debug().Err(err).Msgf("invalid refresh token: %v, key: %s", err, sessionKey+":"+constants.TokenTypeRefreshToken+"_"+nonce) + // Reuse detection (OAuth 2.1 §6.1 / RFC 9700 §4.14.2): ParseJWTToken + // above already verified this token's signature, so it is genuinely + // one we issued. If it carries a nonce and is a refresh token bound to + // the presenting client, yet no live session record backs the nonce, + // it was already rotated away and is being replayed — a compromise + // signal. Surface it distinctly so the token endpoint revokes the + // family. Anything else (missing nonce, wrong token_type, or a token + // bound to a different client) is an ordinary unauthorized with no + // side effect, so a forged/foreign token cannot force a revocation. + if nonce != "" { + tokenType, _ := res["token_type"].(string) + aud, _ := res["aud"].(string) + if tokenType == constants.TokenTypeRefreshToken && aud == expectedClientID { + return res, ErrRefreshTokenReuse + } + } return res, fmt.Errorf(`unauthorized`) }