Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ jobs:
- name: Checkout repository
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: Initialize CodeQL
uses: github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2
uses: github/codeql-action/init@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
with:
languages: ${{ matrix.language }}
build-mode: ${{ matrix.build-mode }}
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2
uses: github/codeql-action/analyze@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
with:
category: "/language:${{matrix.language}}"
2 changes: 1 addition & 1 deletion .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,6 @@ jobs:
# Upload the results to GitHub's code scanning dashboard (optional).
# Commenting out will disable upload of results to your repo's Code Scanning dashboard
- name: "Upload to code-scanning"
uses: github/codeql-action/upload-sarif@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2
uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
with:
sarif_file: results.sarif
22 changes: 19 additions & 3 deletions auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,21 @@ type RequireBearerTokenOptions struct {
ResourceMetadataURL string
// The required scopes.
Scopes []string
// AllowMissingExpiration opts the middleware out of the
// `tokenInfo.Expiration.IsZero()` reject. Default false preserves the
// existing strict behaviour (every TokenInfo must carry an Expiration).
//
// Some IdPs emit session-bound bearer tokens that do not carry a standalone
// `exp` claim — the token's lifetime is bounded by an external session and
// is not advertised in-band. Resource servers integrating with such IdPs
// need to opt in to validating the rest of the token (scopes, signature
// via the verifier callback, etc.) without requiring the expiration field
// to be present.
//
// When enabled, the verifier is still responsible for any session-level
// validity check it can perform; this option only relaxes the middleware's
// own expiration enforcement.
AllowMissingExpiration bool
}

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

// Check expiration.
if tokenInfo.Expiration.IsZero() {
return nil, "token missing expiration", http.StatusUnauthorized
}
if tokenInfo.Expiration.Before(time.Now()) {
if opts == nil || !opts.AllowMissingExpiration {
return nil, "token missing expiration", http.StatusUnauthorized
}
} else if tokenInfo.Expiration.Before(time.Now()) {
return nil, "token expired", http.StatusUnauthorized
}
return tokenInfo, "", 0
Expand Down
5 changes: 5 additions & 0 deletions auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ func TestVerify(t *testing.T) {
"no expiration", nil, "Bearer noexp",
"token missing expiration", 401,
},
{
"no expiration with AllowMissingExpiration accepts",
&RequireBearerTokenOptions{AllowMissingExpiration: true}, "Bearer noexp",
"", 0,
},
{
"expired", nil, "Bearer expired",
"token expired", 401,
Expand Down
33 changes: 33 additions & 0 deletions auth/authorization_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ type AuthorizationResult struct {
Code string
// State string returned by the authorization server.
State string
// Iss is the issuer identifier returned by the authorization server in the
// authorization response per [RFC 9207]. The AuthorizationCodeFetcher should
// populate this from the "iss" query parameter in the redirect URI if present.
//
// [RFC 9207]: https://www.rfc-editor.org/rfc/rfc9207
Iss string
}

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

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

// validateIssuerResponse validates the "iss" parameter in an authorization response
// per [RFC 9207].
//
// [RFC 9207]: https://www.rfc-editor.org/rfc/rfc9207
func validateIssuerResponse(iss, expectedIssuer string, issParameterSupported bool) error {
if issParameterSupported {
if iss == "" {
return fmt.Errorf("authorization server advertises RFC 9207 iss parameter support but none was received in the authorization response")
}
if iss != expectedIssuer {
return fmt.Errorf("authorization response issuer %q does not match expected issuer %q", iss, expectedIssuer)
}
} else {
if iss != "" {
return fmt.Errorf("authorization server does not advertise RFC 9207 iss parameter support but iss was received in the authorization response")
}
}

return nil
}

// exchangeAuthorizationCode exchanges the authorization code for a token
// and stores it in a token source.
func (h *AuthorizationCodeHandler) exchangeAuthorizationCode(ctx context.Context, cfg *oauth2.Config, authResult *authResult, resourceURL string) error {
Expand Down
137 changes: 137 additions & 0 deletions auth/authorization_code_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func TestAuthorize(t *testing.T) {
return &AuthorizationResult{
Code: location.Query().Get("code"),
State: location.Query().Get("state"),
Iss: location.Query().Get("iss"),
}, nil
},
})
Expand Down Expand Up @@ -176,6 +177,7 @@ func TestAuthorize_ScopeAccumulation(t *testing.T) {
return &AuthorizationResult{
Code: loc.Query().Get("code"),
State: loc.Query().Get("state"),
Iss: loc.Query().Get("iss"),
}, nil
},
})
Expand Down Expand Up @@ -607,6 +609,8 @@ func TestHandleRegistration(t *testing.T) {
asm *oauthex.AuthServerMeta
want *resolvedClientConfig
wantError bool
issuerMatch bool
issuerSuffix string
}{
{
name: "ClientIDMetadataDocument",
Expand Down Expand Up @@ -645,6 +649,79 @@ func TestHandleRegistration(t *testing.T) {
authStyle: oauth2.AuthStyleInParams,
},
},
{
name: "Preregistered_IssuerMatch",
serverConfig: &oauthtest.RegistrationConfig{
PreregisteredClients: map[string]oauthtest.ClientInfo{
"pre_client_id": {
Secret: "pre_client_secret",
},
},
},
handlerConfig: &AuthorizationCodeHandlerConfig{
PreregisteredClient: &oauthex.ClientCredentials{
ClientID: "pre_client_id",
ClientSecretAuth: &oauthex.ClientSecretAuth{
ClientSecret: "pre_client_secret",
},
Issuer: "", // set dynamically in the test
},
},
want: &resolvedClientConfig{
registrationType: registrationTypePreregistered,
clientID: "pre_client_id",
clientSecret: "pre_client_secret",
authStyle: oauth2.AuthStyleInParams,
},
issuerMatch: true,
},
{
name: "Preregistered_IssuerMismatch",
serverConfig: &oauthtest.RegistrationConfig{
PreregisteredClients: map[string]oauthtest.ClientInfo{
"pre_client_id": {
Secret: "pre_client_secret",
},
},
},
handlerConfig: &AuthorizationCodeHandlerConfig{
PreregisteredClient: &oauthex.ClientCredentials{
ClientID: "pre_client_id",
ClientSecretAuth: &oauthex.ClientSecretAuth{
ClientSecret: "pre_client_secret",
},
Issuer: "https://other-issuer.example.com",
},
},
wantError: true,
},
{
name: "Preregistered_IssuerMatchTrailingSlash",
serverConfig: &oauthtest.RegistrationConfig{
PreregisteredClients: map[string]oauthtest.ClientInfo{
"pre_client_id": {
Secret: "pre_client_secret",
},
},
},
handlerConfig: &AuthorizationCodeHandlerConfig{
PreregisteredClient: &oauthex.ClientCredentials{
ClientID: "pre_client_id",
ClientSecretAuth: &oauthex.ClientSecretAuth{
ClientSecret: "pre_client_secret",
},
Issuer: "", // set dynamically in the test (with trailing slash)
},
},
want: &resolvedClientConfig{
registrationType: registrationTypePreregistered,
clientID: "pre_client_id",
clientSecret: "pre_client_secret",
authStyle: oauth2.AuthStyleInParams,
},
issuerMatch: true,
issuerSuffix: "/",
},
{
name: "NoneSupported",
handlerConfig: &AuthorizationCodeHandlerConfig{
Expand All @@ -658,6 +735,10 @@ func TestHandleRegistration(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{RegistrationConfig: tt.serverConfig})
s.Start(t)
// Set the Issuer dynamically if requested by the test case.
if tt.issuerMatch {
tt.handlerConfig.PreregisteredClient.Issuer = s.URL() + tt.issuerSuffix
}
tt.handlerConfig.AuthorizationCodeFetcher = func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) {
return nil, nil
}
Expand All @@ -677,6 +758,9 @@ func TestHandleRegistration(t *testing.T) {
}
return
}
if tt.wantError {
t.Fatal("handleRegistration() expected error, got nil")
}
if got.registrationType != tt.want.registrationType {
t.Errorf("handleRegistration() registrationType = %v, want %v", got.registrationType, tt.want.registrationType)
}
Expand Down Expand Up @@ -736,6 +820,59 @@ func TestDynamicRegistration(t *testing.T) {
}
}

func TestValidateIssuerResponse(t *testing.T) {
const expectedIssuer = "https://auth.example.com"

tests := []struct {
name string
iss string
issSupported bool
wantErr bool
wantErrContains string
}{
{
name: "ValidIss",
iss: expectedIssuer,
issSupported: true,
},
{
name: "WrongIss",
iss: "https://attacker.example.com",
issSupported: true,
wantErr: true,
wantErrContains: "does not match expected issuer",
},
{
name: "MissingIssWhenRequired",
iss: "",
issSupported: true,
wantErr: true,
wantErrContains: "RFC 9207",
},
{
name: "MissingIssWhenNotRequired",
iss: "",
issSupported: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateIssuerResponse(tt.iss, expectedIssuer, tt.issSupported)
if tt.wantErr {
if err == nil {
t.Fatalf("validateIssuerResponse() = nil, want error containing %q", tt.wantErrContains)
}
if !strings.Contains(err.Error(), tt.wantErrContains) {
t.Errorf("validateIssuerResponse() error = %q, want it to contain %q", err.Error(), tt.wantErrContains)
}
} else if err != nil {
t.Fatalf("validateIssuerResponse() unexpected error = %v", err)
}
})
}
}

func TestInferApplicationType(t *testing.T) {
tests := []struct {
name string
Expand Down
6 changes: 5 additions & 1 deletion auth/extauth/client_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ
}
}

creds := h.config.Credentials
if creds.Issuer != "" && !authutil.IssuersEqual(creds.Issuer, asm.Issuer) {
return fmt.Errorf("authorization server issuer %q does not match pre-registered credentials issuer %q", asm.Issuer, creds.Issuer)
}

// Determine requestedScopes: use PRM's scopes_supported if available.
requestedScopes := scopesFromChallenges(wwwChallenges)
if len(requestedScopes) == 0 && len(prm.ScopesSupported) > 0 {
Expand All @@ -140,7 +145,6 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ
requestedScopes = authutil.UnionScopes(h.grantedScopes[asm.Issuer], requestedScopes)

// Step 3: Exchange client credentials for an access token.
creds := h.config.Credentials
cfg := &clientcredentials.Config{
ClientID: creds.ClientID,
ClientSecret: creds.ClientSecretAuth.ClientSecret,
Expand Down
45 changes: 45 additions & 0 deletions auth/extauth/client_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,51 @@ func TestClientCredentialsHandler_Authorize(t *testing.T) {
}
})

t.Run("issuer mismatch", func(t *testing.T) {
config := validClientCredentialsConfig()
config.Credentials.Issuer = "https://other-issuer.example.com"
handler, err := NewClientCredentialsHandler(config)
if err != nil {
t.Fatal(err)
}

resp := &http.Response{
StatusCode: http.StatusUnauthorized,
Header: http.Header{},
Body: http.NoBody,
}
req := httptest.NewRequest("GET", resourceURL, nil)
err = handler.Authorize(t.Context(), req, resp)
if err == nil {
t.Fatal("expected Authorize to fail with issuer mismatch")
}
if !strings.Contains(err.Error(), "does not match") {
t.Errorf("error %q does not mention issuer mismatch", err.Error())
}
})

t.Run("issuer match ignoring trailing slash", func(t *testing.T) {
config := validClientCredentialsConfig()
// authServer.URL() has no trailing slash; configure with one to
// verify the comparison tolerates the difference (per RFC 8414 §3.3
// normalization applied in oauthex.IssuersEqual).
config.Credentials.Issuer = authServer.URL() + "/"
handler, err := NewClientCredentialsHandler(config)
if err != nil {
t.Fatal(err)
}

resp := &http.Response{
StatusCode: http.StatusUnauthorized,
Header: http.Header{},
Body: http.NoBody,
}
req := httptest.NewRequest("GET", resourceURL, nil)
if err := handler.Authorize(t.Context(), req, resp); err != nil {
t.Fatalf("Authorize() unexpected error = %v", err)
}
})

t.Run("PRM via resource_metadata in challenge", func(t *testing.T) {
prmMux := http.NewServeMux()
prmMux.Handle("/custom-prm", auth.ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{
Expand Down
Loading