Skip to content

Commit 8db10e7

Browse files
Merge branch 'main' into fix/reject-init-version-mismatch
2 parents 5135d89 + d54751a commit 8db10e7

41 files changed

Lines changed: 3438 additions & 124 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/codeql.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ jobs:
3838
- name: Checkout repository
3939
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
4040
- name: Initialize CodeQL
41-
uses: github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2
41+
uses: github/codeql-action/init@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
4242
with:
4343
languages: ${{ matrix.language }}
4444
build-mode: ${{ matrix.build-mode }}
4545
- name: Perform CodeQL Analysis
46-
uses: github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2
46+
uses: github/codeql-action/analyze@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
4747
with:
4848
category: "/language:${{matrix.language}}"

.github/workflows/scorecard.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,6 @@ jobs:
7373
# Upload the results to GitHub's code scanning dashboard (optional).
7474
# Commenting out will disable upload of results to your repo's Code Scanning dashboard
7575
- name: "Upload to code-scanning"
76-
uses: github/codeql-action/upload-sarif@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2
76+
uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
7777
with:
7878
sarif_file: results.sarif

auth/auth.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,21 @@ type RequireBearerTokenOptions struct {
4747
ResourceMetadataURL string
4848
// The required scopes.
4949
Scopes []string
50+
// AllowMissingExpiration opts the middleware out of the
51+
// `tokenInfo.Expiration.IsZero()` reject. Default false preserves the
52+
// existing strict behaviour (every TokenInfo must carry an Expiration).
53+
//
54+
// Some IdPs emit session-bound bearer tokens that do not carry a standalone
55+
// `exp` claim — the token's lifetime is bounded by an external session and
56+
// is not advertised in-band. Resource servers integrating with such IdPs
57+
// need to opt in to validating the rest of the token (scopes, signature
58+
// via the verifier callback, etc.) without requiring the expiration field
59+
// to be present.
60+
//
61+
// When enabled, the verifier is still responsible for any session-level
62+
// validity check it can perform; this option only relaxes the middleware's
63+
// own expiration enforcement.
64+
AllowMissingExpiration bool
5065
}
5166

5267
type tokenInfoKey struct{}
@@ -131,9 +146,10 @@ func verify(req *http.Request, verifier TokenVerifier, opts *RequireBearerTokenO
131146

132147
// Check expiration.
133148
if tokenInfo.Expiration.IsZero() {
134-
return nil, "token missing expiration", http.StatusUnauthorized
135-
}
136-
if tokenInfo.Expiration.Before(time.Now()) {
149+
if opts == nil || !opts.AllowMissingExpiration {
150+
return nil, "token missing expiration", http.StatusUnauthorized
151+
}
152+
} else if tokenInfo.Expiration.Before(time.Now()) {
137153
return nil, "token expired", http.StatusUnauthorized
138154
}
139155
return tokenInfo, "", 0

auth/auth_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ func TestVerify(t *testing.T) {
6262
"no expiration", nil, "Bearer noexp",
6363
"token missing expiration", 401,
6464
},
65+
{
66+
"no expiration with AllowMissingExpiration accepts",
67+
&RequireBearerTokenOptions{AllowMissingExpiration: true}, "Bearer noexp",
68+
"", 0,
69+
},
6570
{
6671
"expired", nil, "Bearer expired",
6772
"token expired", 401,

auth/authorization_code.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ type AuthorizationResult struct {
5050
Code string
5151
// State string returned by the authorization server.
5252
State string
53+
// Iss is the issuer identifier returned by the authorization server in the
54+
// authorization response per [RFC 9207]. The AuthorizationCodeFetcher should
55+
// populate this from the "iss" query parameter in the redirect URI if present.
56+
//
57+
// [RFC 9207]: https://www.rfc-editor.org/rfc/rfc9207
58+
Iss string
5359
}
5460

5561
// AuthorizationArgs is the input to [AuthorizationCodeFetcher].
@@ -318,6 +324,9 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ
318324
// Purposefully leaving the error unwrappable so it can be handled by the caller.
319325
return err
320326
}
327+
if err := validateIssuerResponse(authRes.Iss, asm.Issuer, asm.AuthorizationResponseIssParameterSupported); err != nil {
328+
return err
329+
}
321330

322331
err = h.exchangeAuthorizationCode(ctx, cfg, authRes, prm.Resource)
323332
if err != nil {
@@ -498,6 +507,9 @@ func (h *AuthorizationCodeHandler) handleRegistration(ctx context.Context, asm *
498507
// 2. Attempt to use pre-registered client configuration.
499508
preCfg := h.config.PreregisteredClient
500509
if preCfg != nil {
510+
if preCfg.Issuer != "" && !authutil.IssuersEqual(preCfg.Issuer, asm.Issuer) {
511+
return nil, fmt.Errorf("authorization server issuer %q does not match pre-registered credentials issuer %q", asm.Issuer, preCfg.Issuer)
512+
}
501513
authStyle := selectTokenAuthMethod(asm.TokenEndpointAuthMethodsSupported)
502514
clientSecret := ""
503515
if preCfg.ClientSecretAuth != nil {
@@ -560,6 +572,27 @@ func (h *AuthorizationCodeHandler) getAuthorizationCode(ctx context.Context, cfg
560572
}, nil
561573
}
562574

575+
// validateIssuerResponse validates the "iss" parameter in an authorization response
576+
// per [RFC 9207].
577+
//
578+
// [RFC 9207]: https://www.rfc-editor.org/rfc/rfc9207
579+
func validateIssuerResponse(iss, expectedIssuer string, issParameterSupported bool) error {
580+
if issParameterSupported {
581+
if iss == "" {
582+
return fmt.Errorf("authorization server advertises RFC 9207 iss parameter support but none was received in the authorization response")
583+
}
584+
if iss != expectedIssuer {
585+
return fmt.Errorf("authorization response issuer %q does not match expected issuer %q", iss, expectedIssuer)
586+
}
587+
} else {
588+
if iss != "" {
589+
return fmt.Errorf("authorization server does not advertise RFC 9207 iss parameter support but iss was received in the authorization response")
590+
}
591+
}
592+
593+
return nil
594+
}
595+
563596
// exchangeAuthorizationCode exchanges the authorization code for a token
564597
// and stores it in a token source.
565598
func (h *AuthorizationCodeHandler) exchangeAuthorizationCode(ctx context.Context, cfg *oauth2.Config, authResult *authResult, resourceURL string) error {

auth/authorization_code_test.go

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ func TestAuthorize(t *testing.T) {
7878
return &AuthorizationResult{
7979
Code: location.Query().Get("code"),
8080
State: location.Query().Get("state"),
81+
Iss: location.Query().Get("iss"),
8182
}, nil
8283
},
8384
})
@@ -176,6 +177,7 @@ func TestAuthorize_ScopeAccumulation(t *testing.T) {
176177
return &AuthorizationResult{
177178
Code: loc.Query().Get("code"),
178179
State: loc.Query().Get("state"),
180+
Iss: loc.Query().Get("iss"),
179181
}, nil
180182
},
181183
})
@@ -607,6 +609,8 @@ func TestHandleRegistration(t *testing.T) {
607609
asm *oauthex.AuthServerMeta
608610
want *resolvedClientConfig
609611
wantError bool
612+
issuerMatch bool
613+
issuerSuffix string
610614
}{
611615
{
612616
name: "ClientIDMetadataDocument",
@@ -645,6 +649,79 @@ func TestHandleRegistration(t *testing.T) {
645649
authStyle: oauth2.AuthStyleInParams,
646650
},
647651
},
652+
{
653+
name: "Preregistered_IssuerMatch",
654+
serverConfig: &oauthtest.RegistrationConfig{
655+
PreregisteredClients: map[string]oauthtest.ClientInfo{
656+
"pre_client_id": {
657+
Secret: "pre_client_secret",
658+
},
659+
},
660+
},
661+
handlerConfig: &AuthorizationCodeHandlerConfig{
662+
PreregisteredClient: &oauthex.ClientCredentials{
663+
ClientID: "pre_client_id",
664+
ClientSecretAuth: &oauthex.ClientSecretAuth{
665+
ClientSecret: "pre_client_secret",
666+
},
667+
Issuer: "", // set dynamically in the test
668+
},
669+
},
670+
want: &resolvedClientConfig{
671+
registrationType: registrationTypePreregistered,
672+
clientID: "pre_client_id",
673+
clientSecret: "pre_client_secret",
674+
authStyle: oauth2.AuthStyleInParams,
675+
},
676+
issuerMatch: true,
677+
},
678+
{
679+
name: "Preregistered_IssuerMismatch",
680+
serverConfig: &oauthtest.RegistrationConfig{
681+
PreregisteredClients: map[string]oauthtest.ClientInfo{
682+
"pre_client_id": {
683+
Secret: "pre_client_secret",
684+
},
685+
},
686+
},
687+
handlerConfig: &AuthorizationCodeHandlerConfig{
688+
PreregisteredClient: &oauthex.ClientCredentials{
689+
ClientID: "pre_client_id",
690+
ClientSecretAuth: &oauthex.ClientSecretAuth{
691+
ClientSecret: "pre_client_secret",
692+
},
693+
Issuer: "https://other-issuer.example.com",
694+
},
695+
},
696+
wantError: true,
697+
},
698+
{
699+
name: "Preregistered_IssuerMatchTrailingSlash",
700+
serverConfig: &oauthtest.RegistrationConfig{
701+
PreregisteredClients: map[string]oauthtest.ClientInfo{
702+
"pre_client_id": {
703+
Secret: "pre_client_secret",
704+
},
705+
},
706+
},
707+
handlerConfig: &AuthorizationCodeHandlerConfig{
708+
PreregisteredClient: &oauthex.ClientCredentials{
709+
ClientID: "pre_client_id",
710+
ClientSecretAuth: &oauthex.ClientSecretAuth{
711+
ClientSecret: "pre_client_secret",
712+
},
713+
Issuer: "", // set dynamically in the test (with trailing slash)
714+
},
715+
},
716+
want: &resolvedClientConfig{
717+
registrationType: registrationTypePreregistered,
718+
clientID: "pre_client_id",
719+
clientSecret: "pre_client_secret",
720+
authStyle: oauth2.AuthStyleInParams,
721+
},
722+
issuerMatch: true,
723+
issuerSuffix: "/",
724+
},
648725
{
649726
name: "NoneSupported",
650727
handlerConfig: &AuthorizationCodeHandlerConfig{
@@ -658,6 +735,10 @@ func TestHandleRegistration(t *testing.T) {
658735
t.Run(tt.name, func(t *testing.T) {
659736
s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{RegistrationConfig: tt.serverConfig})
660737
s.Start(t)
738+
// Set the Issuer dynamically if requested by the test case.
739+
if tt.issuerMatch {
740+
tt.handlerConfig.PreregisteredClient.Issuer = s.URL() + tt.issuerSuffix
741+
}
661742
tt.handlerConfig.AuthorizationCodeFetcher = func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) {
662743
return nil, nil
663744
}
@@ -677,6 +758,9 @@ func TestHandleRegistration(t *testing.T) {
677758
}
678759
return
679760
}
761+
if tt.wantError {
762+
t.Fatal("handleRegistration() expected error, got nil")
763+
}
680764
if got.registrationType != tt.want.registrationType {
681765
t.Errorf("handleRegistration() registrationType = %v, want %v", got.registrationType, tt.want.registrationType)
682766
}
@@ -736,6 +820,59 @@ func TestDynamicRegistration(t *testing.T) {
736820
}
737821
}
738822

823+
func TestValidateIssuerResponse(t *testing.T) {
824+
const expectedIssuer = "https://auth.example.com"
825+
826+
tests := []struct {
827+
name string
828+
iss string
829+
issSupported bool
830+
wantErr bool
831+
wantErrContains string
832+
}{
833+
{
834+
name: "ValidIss",
835+
iss: expectedIssuer,
836+
issSupported: true,
837+
},
838+
{
839+
name: "WrongIss",
840+
iss: "https://attacker.example.com",
841+
issSupported: true,
842+
wantErr: true,
843+
wantErrContains: "does not match expected issuer",
844+
},
845+
{
846+
name: "MissingIssWhenRequired",
847+
iss: "",
848+
issSupported: true,
849+
wantErr: true,
850+
wantErrContains: "RFC 9207",
851+
},
852+
{
853+
name: "MissingIssWhenNotRequired",
854+
iss: "",
855+
issSupported: false,
856+
},
857+
}
858+
859+
for _, tt := range tests {
860+
t.Run(tt.name, func(t *testing.T) {
861+
err := validateIssuerResponse(tt.iss, expectedIssuer, tt.issSupported)
862+
if tt.wantErr {
863+
if err == nil {
864+
t.Fatalf("validateIssuerResponse() = nil, want error containing %q", tt.wantErrContains)
865+
}
866+
if !strings.Contains(err.Error(), tt.wantErrContains) {
867+
t.Errorf("validateIssuerResponse() error = %q, want it to contain %q", err.Error(), tt.wantErrContains)
868+
}
869+
} else if err != nil {
870+
t.Fatalf("validateIssuerResponse() unexpected error = %v", err)
871+
}
872+
})
873+
}
874+
}
875+
739876
func TestInferApplicationType(t *testing.T) {
740877
tests := []struct {
741878
name string

auth/extauth/client_credentials.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ
128128
}
129129
}
130130

131+
creds := h.config.Credentials
132+
if creds.Issuer != "" && !authutil.IssuersEqual(creds.Issuer, asm.Issuer) {
133+
return fmt.Errorf("authorization server issuer %q does not match pre-registered credentials issuer %q", asm.Issuer, creds.Issuer)
134+
}
135+
131136
// Determine requestedScopes: use PRM's scopes_supported if available.
132137
requestedScopes := scopesFromChallenges(wwwChallenges)
133138
if len(requestedScopes) == 0 && len(prm.ScopesSupported) > 0 {
@@ -140,7 +145,6 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ
140145
requestedScopes = authutil.UnionScopes(h.grantedScopes[asm.Issuer], requestedScopes)
141146

142147
// Step 3: Exchange client credentials for an access token.
143-
creds := h.config.Credentials
144148
cfg := &clientcredentials.Config{
145149
ClientID: creds.ClientID,
146150
ClientSecret: creds.ClientSecretAuth.ClientSecret,

auth/extauth/client_credentials_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,51 @@ func TestClientCredentialsHandler_Authorize(t *testing.T) {
221221
}
222222
})
223223

224+
t.Run("issuer mismatch", func(t *testing.T) {
225+
config := validClientCredentialsConfig()
226+
config.Credentials.Issuer = "https://other-issuer.example.com"
227+
handler, err := NewClientCredentialsHandler(config)
228+
if err != nil {
229+
t.Fatal(err)
230+
}
231+
232+
resp := &http.Response{
233+
StatusCode: http.StatusUnauthorized,
234+
Header: http.Header{},
235+
Body: http.NoBody,
236+
}
237+
req := httptest.NewRequest("GET", resourceURL, nil)
238+
err = handler.Authorize(t.Context(), req, resp)
239+
if err == nil {
240+
t.Fatal("expected Authorize to fail with issuer mismatch")
241+
}
242+
if !strings.Contains(err.Error(), "does not match") {
243+
t.Errorf("error %q does not mention issuer mismatch", err.Error())
244+
}
245+
})
246+
247+
t.Run("issuer match ignoring trailing slash", func(t *testing.T) {
248+
config := validClientCredentialsConfig()
249+
// authServer.URL() has no trailing slash; configure with one to
250+
// verify the comparison tolerates the difference (per RFC 8414 §3.3
251+
// normalization applied in oauthex.IssuersEqual).
252+
config.Credentials.Issuer = authServer.URL() + "/"
253+
handler, err := NewClientCredentialsHandler(config)
254+
if err != nil {
255+
t.Fatal(err)
256+
}
257+
258+
resp := &http.Response{
259+
StatusCode: http.StatusUnauthorized,
260+
Header: http.Header{},
261+
Body: http.NoBody,
262+
}
263+
req := httptest.NewRequest("GET", resourceURL, nil)
264+
if err := handler.Authorize(t.Context(), req, resp); err != nil {
265+
t.Fatalf("Authorize() unexpected error = %v", err)
266+
}
267+
})
268+
224269
t.Run("PRM via resource_metadata in challenge", func(t *testing.T) {
225270
prmMux := http.NewServeMux()
226271
prmMux.Handle("/custom-prm", auth.ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{

0 commit comments

Comments
 (0)