From e0bd9f656ee3a498a28ee23c5fdec016ebfe4c3c Mon Sep 17 00:00:00 2001 From: mushrafmim Date: Fri, 9 Oct 2026 13:54:04 +0530 Subject: [PATCH 1/3] refactor: remove ClientIDs from Config struct and related validations --- authn/README.md | 20 ++++++++--------- authn/config.go | 13 ++++------- authn/config_test.go | 23 +++++++------------- authn/extra_claims_test.go | 12 +++++------ authn/fixed_schema_test.go | 2 +- authn/manager.go | 2 +- authn/manager_test.go | 25 +++------------------- authn/test_helpers_test.go | 2 +- authn/token_parser.go | 29 +++++++++---------------- authn/token_parser_test.go | 44 ++++++++++++++++++++++++-------------- 10 files changed, 71 insertions(+), 101 deletions(-) diff --git a/authn/README.md b/authn/README.md index e1ed9d49..e9380ebf 100644 --- a/authn/README.md +++ b/authn/README.md @@ -8,10 +8,9 @@ JWT-based authentication. Validates tokens against a JWKS endpoint, extracts ide import "github.com/OpenNSW/core/authn" manager, err := authn.NewManager(userProfileSvc, authn.Config{ - JWKSURL: "https://idp.example.com/.well-known/jwks.json", - Issuer: "https://idp.example.com", - Audience: "my-api", - ClientIDs: []string{"my-m2m-client"}, + JWKSURL: "https://idp.example.com/.well-known/jwks.json", + Issuer: "https://idp.example.com", + Audience: "my-api", // Optional: declare any claims beyond the fixed schema you need. See // "Extra claims" below. @@ -28,14 +27,14 @@ authn: jwksURL: https://idp.example.com/.well-known/jwks.json issuer: https://idp.example.com audience: my-api - clientIDs: - - my-m2m-client userClaims: optional: [email, ouId, ouHandle] ``` > **Note:** every token must carry a `grant_type` claim — it decides user vs. client principal, and a token without it is rejected with `unsupported grant type`. This is not a standard access-token claim (RFC 9068 does not define it), so an IdP that omits it will not work with this package as-is. +> **Note:** `audience` is what binds a token to this service: any client the IdP issues a token for this audience is accepted. Which clients may call the service is authorization policy — restrict it in authz or the application using the principal's `ClientID`, not here. If your IdP sets `aud` to the requesting client rather than the API, configure an audience / API resource for the service in the IdP. + ## Middleware ```go @@ -82,10 +81,9 @@ email := authn.GetAuthContext(r.Context()).ExtraClaims().String("email") ```go manager, err := authn.NewManager(userProfileSvc, authn.Config{ - JWKSURL: "...", - Issuer: "...", - Audience: "...", - ClientIDs: []string{"my-m2m-client"}, + JWKSURL: "...", + Issuer: "...", + Audience: "...", UserClaims: authn.ClaimSpec{ Optional: []string{"email", "ouId", "ouHandle", "given_name"}, @@ -100,7 +98,7 @@ manager, err := authn.NewManager(userProfileSvc, authn.Config{ Or, constructing a `TokenExtractor` directly: ```go -extractor, err := authn.NewTokenExtractor(jwksURL, issuer, audience, clientIDs, +extractor, err := authn.NewTokenExtractor(jwksURL, issuer, audience, authn.WithUserClaims(authn.ClaimSpec{ Optional: []string{"email", "ouId", "ouHandle"}, Required: []string{"phone_number"}, diff --git a/authn/config.go b/authn/config.go index 5e75569d..3669c105 100644 --- a/authn/config.go +++ b/authn/config.go @@ -13,11 +13,10 @@ import ( // application config struct and populated generically (e.g. via // yaml.Unmarshal). type Config struct { - JWKSURL string `yaml:"jwksURL"` - Issuer string `yaml:"issuer"` - Audience string `yaml:"audience"` - ClientIDs []string `yaml:"clientIDs"` - InsecureSkipTLSVerify bool `yaml:"insecureSkipTLSVerify"` + JWKSURL string `yaml:"jwksURL"` + Issuer string `yaml:"issuer"` + Audience string `yaml:"audience"` + InsecureSkipTLSVerify bool `yaml:"insecureSkipTLSVerify"` // UserClaims declares extra JWT claims (beyond authn's fixed schema, e.g. // "email", "phone_number", "ouId", "ouHandle", "given_name") to extract @@ -50,10 +49,6 @@ func (c Config) Validate() error { return fmt.Errorf("audience is required") } - if len(c.ClientIDs) == 0 { - return fmt.Errorf("clientIDs is required") - } - if err := validateRolesClaimName(c.rolesClaim()); err != nil { return err } diff --git a/authn/config_test.go b/authn/config_test.go index d00ef98b..8ea7af01 100644 --- a/authn/config_test.go +++ b/authn/config_test.go @@ -12,9 +12,6 @@ func TestConfigValidate(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } // Each error names the setting by its yaml key, so a config file author @@ -25,12 +22,11 @@ func TestConfigValidate(t *testing.T) { wantErr string }{ {name: "valid config", config: valid}, - {name: "missing jwks url", config: Config{Issuer: valid.Issuer, Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "jwksURL is required"}, - {name: "invalid jwks url", config: Config{JWKSURL: "jwks", Issuer: valid.Issuer, Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "jwksURL must be a valid absolute URL"}, - {name: "missing issuer", config: Config{JWKSURL: valid.JWKSURL, Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "issuer is required"}, - {name: "invalid issuer", config: Config{JWKSURL: valid.JWKSURL, Issuer: "ftp://localhost", Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "issuer must use http or https"}, - {name: "missing audience", config: Config{JWKSURL: valid.JWKSURL, Issuer: valid.Issuer, ClientIDs: valid.ClientIDs}, wantErr: "audience is required"}, - {name: "missing client ids", config: Config{JWKSURL: valid.JWKSURL, Issuer: valid.Issuer, Audience: valid.Audience}, wantErr: "clientIDs is required"}, + {name: "missing jwks url", config: Config{Issuer: valid.Issuer, Audience: valid.Audience}, wantErr: "jwksURL is required"}, + {name: "invalid jwks url", config: Config{JWKSURL: "jwks", Issuer: valid.Issuer, Audience: valid.Audience}, wantErr: "jwksURL must be a valid absolute URL"}, + {name: "missing issuer", config: Config{JWKSURL: valid.JWKSURL, Audience: valid.Audience}, wantErr: "issuer is required"}, + {name: "invalid issuer", config: Config{JWKSURL: valid.JWKSURL, Issuer: "ftp://localhost", Audience: valid.Audience}, wantErr: "issuer must use http or https"}, + {name: "missing audience", config: Config{JWKSURL: valid.JWKSURL, Issuer: valid.Issuer}, wantErr: "audience is required"}, } for _, tt := range tests { @@ -51,10 +47,9 @@ func TestConfigValidate(t *testing.T) { func TestConfigValidate_RejectsFixedSchemaExtraClaim(t *testing.T) { valid := Config{ - JWKSURL: "https://localhost/jwks", - Issuer: "https://localhost/token", - Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, + JWKSURL: "https://localhost/jwks", + Issuer: "https://localhost/token", + Audience: "TRADER_PORTAL_APP", } tests := []struct { @@ -97,7 +92,6 @@ func TestConfigValidate_AllowsFormerlyFixedClaims(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, UserClaims: ClaimSpec{Optional: []string{"email", "phone_number", "ouId", "ouHandle", "given_name"}}, } if err := cfg.Validate(); err != nil { @@ -113,7 +107,6 @@ func TestConfigValidate_AllowsNamespacedClaimNames(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, RolesClaim: "https://app.example.com/roles", UserClaims: ClaimSpec{Optional: []string{"https://app.example.com/department"}}, } diff --git a/authn/extra_claims_test.go b/authn/extra_claims_test.go index a66e50a4..fbe924fa 100644 --- a/authn/extra_claims_test.go +++ b/authn/extra_claims_test.go @@ -452,19 +452,19 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { } for _, name := range names { t.Run(name+"/optional", func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Optional: []string{name}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Optional: []string{name}})); err == nil { t.Fatalf("expected error declaring fixed-schema claim %q", name) } }) t.Run(name+"/required", func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Required: []string{name}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Required: []string{name}})); err == nil { t.Fatalf("expected error declaring fixed-schema claim %q", name) } }) } t.Run("client variant is also rejected", func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithClientClaims(ClaimSpec{Required: []string{"scope"}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithClientClaims(ClaimSpec{Required: []string{"scope"}})); err == nil { t.Fatalf("expected error declaring fixed-schema claim via client option") } }) @@ -473,7 +473,7 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { // The natural result of strings.Split on an env var with a trailing // comma. Dropping it silently means an enforcement rule the caller // asked for never applies. - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Required: []string{"email", ""}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Required: []string{"email", ""}})); err == nil { t.Fatalf("expected error for a blank claim name") } }) @@ -481,7 +481,7 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { t.Run("collision error names every offender deterministically", func(t *testing.T) { var first string for i := range 8 { - _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, + _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Optional: []string{"scope", "roles", "sub"}})) if err == nil { t.Fatalf("expected error") @@ -508,7 +508,7 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { func TestNewTokenExtractor_AllowsFormerlyFixedClaimNames(t *testing.T) { for _, name := range []string{"email", "phone_number", "ouId", "ouHandle", "given_name"} { t.Run(name, func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Optional: []string{name}})); err != nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Optional: []string{name}})); err != nil { t.Fatalf("expected %q to be a legitimate extra claim, got error: %v", name, err) } }) diff --git a/authn/fixed_schema_test.go b/authn/fixed_schema_test.go index 4bd1ecd3..7503d4ad 100644 --- a/authn/fixed_schema_test.go +++ b/authn/fixed_schema_test.go @@ -195,7 +195,7 @@ func TestRolesClaim(t *testing.T) { func TestRolesClaim_NameValidation(t *testing.T) { construct := func(opts ...Option) error { - _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, opts...) + _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, opts...) return err } diff --git a/authn/manager.go b/authn/manager.go index b34aa573..481514e3 100644 --- a/authn/manager.go +++ b/authn/manager.go @@ -55,7 +55,7 @@ func NewManager(userProfileService UserProfileService, authConfig Config) (*Mana } tokenExtractor, err := NewTokenExtractorWithClient( - authConfig.JWKSURL, authConfig.Issuer, authConfig.Audience, authConfig.ClientIDs, httpClient, + authConfig.JWKSURL, authConfig.Issuer, authConfig.Audience, httpClient, buildClaimOptions(authConfig)..., ) if err != nil { diff --git a/authn/manager_test.go b/authn/manager_test.go index 392efd2a..fb1c42a1 100644 --- a/authn/manager_test.go +++ b/authn/manager_test.go @@ -14,9 +14,6 @@ func TestNewManager_AllowsNilUserProfileService(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -42,9 +39,6 @@ func TestNewManager_InvalidConfig(t *testing.T) { cfg := Config{ Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } if _, err := NewManager(nil, cfg); err == nil { @@ -64,7 +58,6 @@ func TestNewManager_InsecureSkipTLSVerify(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, InsecureSkipTLSVerify: true, } @@ -89,9 +82,6 @@ func TestManager_Health_Success(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -108,9 +98,6 @@ func TestManager_MiddlewareFunctions(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -140,9 +127,6 @@ func TestManager_RequireAuthMiddleware(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -177,10 +161,9 @@ func TestManager_Close(t *testing.T) { func TestNewManager_WiresExtraClaims(t *testing.T) { cfg := Config{ - JWKSURL: "https://localhost/jwks", - Issuer: "https://localhost/token", - Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, + JWKSURL: "https://localhost/jwks", + Issuer: "https://localhost/token", + Audience: "TRADER_PORTAL_APP", // "email" appears in both slices: required must win on the Config // path too, independently of the order buildClaimOptions emits. UserClaims: ClaimSpec{Optional: []string{"email", "given_name"}, Required: []string{"ouHandle", "email"}}, @@ -223,7 +206,6 @@ func TestNewManager_RejectsFixedSchemaExtraClaim(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, UserClaims: ClaimSpec{Optional: []string{"scope"}}, } if _, err := NewManager(nil, cfg); err == nil { @@ -240,7 +222,6 @@ func TestNewManager_RejectsRolesClaimCollision(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, RolesClaim: "groups", UserClaims: ClaimSpec{Optional: []string{"groups"}}, } diff --git a/authn/test_helpers_test.go b/authn/test_helpers_test.go index d24696b0..9c437638 100644 --- a/authn/test_helpers_test.go +++ b/authn/test_helpers_test.go @@ -58,7 +58,7 @@ func newTokenExtractorWithOptions(t *testing.T, opts ...Option) (*TokenExtractor }) })) - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}, opts...) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, opts...) if err != nil { jwksServer.Close() t.Fatalf("failed to create token extractor: %v", err) diff --git a/authn/token_parser.go b/authn/token_parser.go index 64ddb50b..f300a003 100644 --- a/authn/token_parser.go +++ b/authn/token_parser.go @@ -8,7 +8,6 @@ import ( "fmt" "log/slog" "net/http" - "slices" "strings" "sync" "time" @@ -112,11 +111,10 @@ const defaultJWKSCacheTTL = 5 * time.Minute // It validates JWT signatures using JWKS and resolves a user principal // or client principal based on grant type. type TokenExtractor struct { - jwksURL string - expIssuer string - expAudience string - expClientIDs []string - httpClient *http.Client + jwksURL string + expIssuer string + expAudience string + httpClient *http.Client // userClaims / clientClaims are the raw declarations accumulated from // WithUserClaims / WithClientClaims. They are validated and flattened into @@ -137,25 +135,24 @@ type TokenExtractor struct { jwksCacheTTL time.Duration } -func NewTokenExtractor(jwksURL, issuer, audience string, expectedClientIDs []string, opts ...Option) (*TokenExtractor, error) { - return newExtractor(jwksURL, issuer, audience, expectedClientIDs, &http.Client{Timeout: 10 * time.Second}, opts...) +func NewTokenExtractor(jwksURL, issuer, audience string, opts ...Option) (*TokenExtractor, error) { + return newExtractor(jwksURL, issuer, audience, &http.Client{Timeout: 10 * time.Second}, opts...) } -func NewTokenExtractorWithClient(jwksURL, issuer, audience string, expectedClientIDs []string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { +func NewTokenExtractorWithClient(jwksURL, issuer, audience string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { if httpClient == nil { - return NewTokenExtractor(jwksURL, issuer, audience, expectedClientIDs, opts...) + return NewTokenExtractor(jwksURL, issuer, audience, opts...) } - return newExtractor(jwksURL, issuer, audience, expectedClientIDs, httpClient, opts...) + return newExtractor(jwksURL, issuer, audience, httpClient, opts...) } // newExtractor is the single construction path, so a defaulted field // (rolesClaim) can only be forgotten in one place. -func newExtractor(jwksURL, issuer, audience string, expectedClientIDs []string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { +func newExtractor(jwksURL, issuer, audience string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { extractor := &TokenExtractor{ jwksURL: strings.TrimSpace(jwksURL), expIssuer: strings.TrimSpace(issuer), expAudience: strings.TrimSpace(audience), - expClientIDs: expectedClientIDs, jwksCacheTTL: defaultJWKSCacheTTL, httpClient: httpClient, rolesClaim: defaultRolesClaim, @@ -182,9 +179,6 @@ func (te *TokenExtractor) validateConfig() error { if te.expAudience == "" { return fmt.Errorf("audience is not configured") } - if len(te.expClientIDs) == 0 { - return fmt.Errorf("client ids are not configured") - } if te.httpClient == nil { return fmt.Errorf("http client is not configured") } @@ -255,9 +249,6 @@ func (te *TokenExtractor) ExtractPrincipalFromHeader(authHeader string) (*Princi if claims.ClientID == "" { return nil, fmt.Errorf("jwt missing client_id claim") } - if !slices.Contains(te.expClientIDs, claims.ClientID) { - return nil, fmt.Errorf("unexpected client_id claim: %q", claims.ClientID) - } switch claims.GrantType { case AuthorizationCodeGrant: diff --git a/authn/token_parser_test.go b/authn/token_parser_test.go index fc96e78e..6e3665c2 100644 --- a/authn/token_parser_test.go +++ b/authn/token_parser_test.go @@ -113,13 +113,6 @@ func TestTokenExtractor_ExtractPrincipalFromHeader_MissingClaims(t *testing.T) { }, errSubstr: "jwt missing client_id claim", }, - { - name: "unexpected client_id", - mutate: func(claims jwt.MapClaims) { - claims["client_id"] = "OTHER" - }, - errSubstr: "unexpected client_id claim", - }, { name: "unsupported grant_type", mutate: func(claims jwt.MapClaims) { @@ -207,6 +200,26 @@ func TestTokenExtractor_UserToken_NoLongerRequiresEmailOrOU(t *testing.T) { } } +// TestTokenExtractor_AcceptsAnyClientIDForAudience documents that there is no +// client allowlist: aud binds a token to this service, and which clients may +// obtain such a token is the IdP's decision. Restricting callers by client_id +// is authorization policy, not authentication. +func TestTokenExtractor_AcceptsAnyClientIDForAudience(t *testing.T) { + extractor, privateKey, cleanup := newTokenExtractor(t) + defer cleanup() + + claims := newBaseClaims(ClientCredentialsGrant) + claims["client_id"] = "SOME_OTHER_CLIENT" + + principal, err := extractor.ExtractPrincipalFromHeader("Bearer " + signToken(t, privateKey, claims)) + if err != nil { + t.Fatalf("expected success for a token issued for this audience, got %v", err) + } + if principal.ClientPrincipal == nil || principal.ClientPrincipal.ClientID != "SOME_OTHER_CLIENT" { + t.Fatalf("unexpected principal: %#v", principal) + } +} + func TestTokenExtractor_JWKSIsCached(t *testing.T) { privateKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { @@ -245,7 +258,7 @@ func TestTokenExtractor_JWKSIsCached(t *testing.T) { t.Fatalf("failed to sign token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -328,7 +341,7 @@ func TestTokenExtractor_RefreshesJWKSOnUnknownKid(t *testing.T) { t.Fatalf("failed to sign new token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -382,7 +395,7 @@ func TestTokenExtractor_UnknownKidAfterRefresh(t *testing.T) { t.Fatalf("failed to sign token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -418,7 +431,7 @@ func TestTokenExtractor_JWKSFetchError(t *testing.T) { t.Fatalf("failed to sign token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -430,11 +443,10 @@ func TestTokenExtractor_JWKSFetchError(t *testing.T) { func TestTokenExtractor_ValidateConfig_MissingHTTPClient(t *testing.T) { extractor := &TokenExtractor{ - jwksURL: "https://localhost/jwks", - expIssuer: testIssuer, - expAudience: testClientID, - expClientIDs: []string{testClientID}, - httpClient: nil, + jwksURL: "https://localhost/jwks", + expIssuer: testIssuer, + expAudience: testClientID, + httpClient: nil, } if err := extractor.validateConfig(); err == nil { From 93b205c5dde3f3b1ec48a516ec46276d0ab09136 Mon Sep 17 00:00:00 2001 From: mushrafmim Date: Fri, 9 Oct 2026 13:54:04 +0530 Subject: [PATCH 2/3] refactor: remove ClientIDs from Config struct and related validations --- authn/README.md | 20 ++++++++--------- authn/config.go | 13 ++++------- authn/config_test.go | 23 +++++++------------- authn/extra_claims_test.go | 12 +++++------ authn/fixed_schema_test.go | 2 +- authn/manager.go | 2 +- authn/manager_test.go | 25 +++------------------- authn/test_helpers_test.go | 2 +- authn/token_parser.go | 29 +++++++++---------------- authn/token_parser_test.go | 44 ++++++++++++++++++++++++-------------- 10 files changed, 71 insertions(+), 101 deletions(-) diff --git a/authn/README.md b/authn/README.md index e1ed9d49..e9380ebf 100644 --- a/authn/README.md +++ b/authn/README.md @@ -8,10 +8,9 @@ JWT-based authentication. Validates tokens against a JWKS endpoint, extracts ide import "github.com/OpenNSW/core/authn" manager, err := authn.NewManager(userProfileSvc, authn.Config{ - JWKSURL: "https://idp.example.com/.well-known/jwks.json", - Issuer: "https://idp.example.com", - Audience: "my-api", - ClientIDs: []string{"my-m2m-client"}, + JWKSURL: "https://idp.example.com/.well-known/jwks.json", + Issuer: "https://idp.example.com", + Audience: "my-api", // Optional: declare any claims beyond the fixed schema you need. See // "Extra claims" below. @@ -28,14 +27,14 @@ authn: jwksURL: https://idp.example.com/.well-known/jwks.json issuer: https://idp.example.com audience: my-api - clientIDs: - - my-m2m-client userClaims: optional: [email, ouId, ouHandle] ``` > **Note:** every token must carry a `grant_type` claim — it decides user vs. client principal, and a token without it is rejected with `unsupported grant type`. This is not a standard access-token claim (RFC 9068 does not define it), so an IdP that omits it will not work with this package as-is. +> **Note:** `audience` is what binds a token to this service: any client the IdP issues a token for this audience is accepted. Which clients may call the service is authorization policy — restrict it in authz or the application using the principal's `ClientID`, not here. If your IdP sets `aud` to the requesting client rather than the API, configure an audience / API resource for the service in the IdP. + ## Middleware ```go @@ -82,10 +81,9 @@ email := authn.GetAuthContext(r.Context()).ExtraClaims().String("email") ```go manager, err := authn.NewManager(userProfileSvc, authn.Config{ - JWKSURL: "...", - Issuer: "...", - Audience: "...", - ClientIDs: []string{"my-m2m-client"}, + JWKSURL: "...", + Issuer: "...", + Audience: "...", UserClaims: authn.ClaimSpec{ Optional: []string{"email", "ouId", "ouHandle", "given_name"}, @@ -100,7 +98,7 @@ manager, err := authn.NewManager(userProfileSvc, authn.Config{ Or, constructing a `TokenExtractor` directly: ```go -extractor, err := authn.NewTokenExtractor(jwksURL, issuer, audience, clientIDs, +extractor, err := authn.NewTokenExtractor(jwksURL, issuer, audience, authn.WithUserClaims(authn.ClaimSpec{ Optional: []string{"email", "ouId", "ouHandle"}, Required: []string{"phone_number"}, diff --git a/authn/config.go b/authn/config.go index 5e75569d..3669c105 100644 --- a/authn/config.go +++ b/authn/config.go @@ -13,11 +13,10 @@ import ( // application config struct and populated generically (e.g. via // yaml.Unmarshal). type Config struct { - JWKSURL string `yaml:"jwksURL"` - Issuer string `yaml:"issuer"` - Audience string `yaml:"audience"` - ClientIDs []string `yaml:"clientIDs"` - InsecureSkipTLSVerify bool `yaml:"insecureSkipTLSVerify"` + JWKSURL string `yaml:"jwksURL"` + Issuer string `yaml:"issuer"` + Audience string `yaml:"audience"` + InsecureSkipTLSVerify bool `yaml:"insecureSkipTLSVerify"` // UserClaims declares extra JWT claims (beyond authn's fixed schema, e.g. // "email", "phone_number", "ouId", "ouHandle", "given_name") to extract @@ -50,10 +49,6 @@ func (c Config) Validate() error { return fmt.Errorf("audience is required") } - if len(c.ClientIDs) == 0 { - return fmt.Errorf("clientIDs is required") - } - if err := validateRolesClaimName(c.rolesClaim()); err != nil { return err } diff --git a/authn/config_test.go b/authn/config_test.go index d00ef98b..8ea7af01 100644 --- a/authn/config_test.go +++ b/authn/config_test.go @@ -12,9 +12,6 @@ func TestConfigValidate(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } // Each error names the setting by its yaml key, so a config file author @@ -25,12 +22,11 @@ func TestConfigValidate(t *testing.T) { wantErr string }{ {name: "valid config", config: valid}, - {name: "missing jwks url", config: Config{Issuer: valid.Issuer, Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "jwksURL is required"}, - {name: "invalid jwks url", config: Config{JWKSURL: "jwks", Issuer: valid.Issuer, Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "jwksURL must be a valid absolute URL"}, - {name: "missing issuer", config: Config{JWKSURL: valid.JWKSURL, Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "issuer is required"}, - {name: "invalid issuer", config: Config{JWKSURL: valid.JWKSURL, Issuer: "ftp://localhost", Audience: valid.Audience, ClientIDs: valid.ClientIDs}, wantErr: "issuer must use http or https"}, - {name: "missing audience", config: Config{JWKSURL: valid.JWKSURL, Issuer: valid.Issuer, ClientIDs: valid.ClientIDs}, wantErr: "audience is required"}, - {name: "missing client ids", config: Config{JWKSURL: valid.JWKSURL, Issuer: valid.Issuer, Audience: valid.Audience}, wantErr: "clientIDs is required"}, + {name: "missing jwks url", config: Config{Issuer: valid.Issuer, Audience: valid.Audience}, wantErr: "jwksURL is required"}, + {name: "invalid jwks url", config: Config{JWKSURL: "jwks", Issuer: valid.Issuer, Audience: valid.Audience}, wantErr: "jwksURL must be a valid absolute URL"}, + {name: "missing issuer", config: Config{JWKSURL: valid.JWKSURL, Audience: valid.Audience}, wantErr: "issuer is required"}, + {name: "invalid issuer", config: Config{JWKSURL: valid.JWKSURL, Issuer: "ftp://localhost", Audience: valid.Audience}, wantErr: "issuer must use http or https"}, + {name: "missing audience", config: Config{JWKSURL: valid.JWKSURL, Issuer: valid.Issuer}, wantErr: "audience is required"}, } for _, tt := range tests { @@ -51,10 +47,9 @@ func TestConfigValidate(t *testing.T) { func TestConfigValidate_RejectsFixedSchemaExtraClaim(t *testing.T) { valid := Config{ - JWKSURL: "https://localhost/jwks", - Issuer: "https://localhost/token", - Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, + JWKSURL: "https://localhost/jwks", + Issuer: "https://localhost/token", + Audience: "TRADER_PORTAL_APP", } tests := []struct { @@ -97,7 +92,6 @@ func TestConfigValidate_AllowsFormerlyFixedClaims(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, UserClaims: ClaimSpec{Optional: []string{"email", "phone_number", "ouId", "ouHandle", "given_name"}}, } if err := cfg.Validate(); err != nil { @@ -113,7 +107,6 @@ func TestConfigValidate_AllowsNamespacedClaimNames(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, RolesClaim: "https://app.example.com/roles", UserClaims: ClaimSpec{Optional: []string{"https://app.example.com/department"}}, } diff --git a/authn/extra_claims_test.go b/authn/extra_claims_test.go index a66e50a4..fbe924fa 100644 --- a/authn/extra_claims_test.go +++ b/authn/extra_claims_test.go @@ -452,19 +452,19 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { } for _, name := range names { t.Run(name+"/optional", func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Optional: []string{name}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Optional: []string{name}})); err == nil { t.Fatalf("expected error declaring fixed-schema claim %q", name) } }) t.Run(name+"/required", func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Required: []string{name}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Required: []string{name}})); err == nil { t.Fatalf("expected error declaring fixed-schema claim %q", name) } }) } t.Run("client variant is also rejected", func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithClientClaims(ClaimSpec{Required: []string{"scope"}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithClientClaims(ClaimSpec{Required: []string{"scope"}})); err == nil { t.Fatalf("expected error declaring fixed-schema claim via client option") } }) @@ -473,7 +473,7 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { // The natural result of strings.Split on an env var with a trailing // comma. Dropping it silently means an enforcement rule the caller // asked for never applies. - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Required: []string{"email", ""}})); err == nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Required: []string{"email", ""}})); err == nil { t.Fatalf("expected error for a blank claim name") } }) @@ -481,7 +481,7 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { t.Run("collision error names every offender deterministically", func(t *testing.T) { var first string for i := range 8 { - _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, + _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Optional: []string{"scope", "roles", "sub"}})) if err == nil { t.Fatalf("expected error") @@ -508,7 +508,7 @@ func TestNewTokenExtractor_RejectsFixedSchemaClaimNames(t *testing.T) { func TestNewTokenExtractor_AllowsFormerlyFixedClaimNames(t *testing.T) { for _, name := range []string{"email", "phone_number", "ouId", "ouHandle", "given_name"} { t.Run(name, func(t *testing.T) { - if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, WithUserClaims(ClaimSpec{Optional: []string{name}})); err != nil { + if _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, WithUserClaims(ClaimSpec{Optional: []string{name}})); err != nil { t.Fatalf("expected %q to be a legitimate extra claim, got error: %v", name, err) } }) diff --git a/authn/fixed_schema_test.go b/authn/fixed_schema_test.go index 4bd1ecd3..7503d4ad 100644 --- a/authn/fixed_schema_test.go +++ b/authn/fixed_schema_test.go @@ -195,7 +195,7 @@ func TestRolesClaim(t *testing.T) { func TestRolesClaim_NameValidation(t *testing.T) { construct := func(opts ...Option) error { - _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, []string{testClientID}, opts...) + _, err := NewTokenExtractor("https://localhost/jwks", testIssuer, testClientID, opts...) return err } diff --git a/authn/manager.go b/authn/manager.go index b34aa573..481514e3 100644 --- a/authn/manager.go +++ b/authn/manager.go @@ -55,7 +55,7 @@ func NewManager(userProfileService UserProfileService, authConfig Config) (*Mana } tokenExtractor, err := NewTokenExtractorWithClient( - authConfig.JWKSURL, authConfig.Issuer, authConfig.Audience, authConfig.ClientIDs, httpClient, + authConfig.JWKSURL, authConfig.Issuer, authConfig.Audience, httpClient, buildClaimOptions(authConfig)..., ) if err != nil { diff --git a/authn/manager_test.go b/authn/manager_test.go index 392efd2a..fb1c42a1 100644 --- a/authn/manager_test.go +++ b/authn/manager_test.go @@ -14,9 +14,6 @@ func TestNewManager_AllowsNilUserProfileService(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -42,9 +39,6 @@ func TestNewManager_InvalidConfig(t *testing.T) { cfg := Config{ Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } if _, err := NewManager(nil, cfg); err == nil { @@ -64,7 +58,6 @@ func TestNewManager_InsecureSkipTLSVerify(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, InsecureSkipTLSVerify: true, } @@ -89,9 +82,6 @@ func TestManager_Health_Success(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -108,9 +98,6 @@ func TestManager_MiddlewareFunctions(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -140,9 +127,6 @@ func TestManager_RequireAuthMiddleware(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{ - "TRADER_PORTAL_APP", - }, } manager, err := NewManager(nil, cfg) @@ -177,10 +161,9 @@ func TestManager_Close(t *testing.T) { func TestNewManager_WiresExtraClaims(t *testing.T) { cfg := Config{ - JWKSURL: "https://localhost/jwks", - Issuer: "https://localhost/token", - Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, + JWKSURL: "https://localhost/jwks", + Issuer: "https://localhost/token", + Audience: "TRADER_PORTAL_APP", // "email" appears in both slices: required must win on the Config // path too, independently of the order buildClaimOptions emits. UserClaims: ClaimSpec{Optional: []string{"email", "given_name"}, Required: []string{"ouHandle", "email"}}, @@ -223,7 +206,6 @@ func TestNewManager_RejectsFixedSchemaExtraClaim(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, UserClaims: ClaimSpec{Optional: []string{"scope"}}, } if _, err := NewManager(nil, cfg); err == nil { @@ -240,7 +222,6 @@ func TestNewManager_RejectsRolesClaimCollision(t *testing.T) { JWKSURL: "https://localhost/jwks", Issuer: "https://localhost/token", Audience: "TRADER_PORTAL_APP", - ClientIDs: []string{"TRADER_PORTAL_APP"}, RolesClaim: "groups", UserClaims: ClaimSpec{Optional: []string{"groups"}}, } diff --git a/authn/test_helpers_test.go b/authn/test_helpers_test.go index d24696b0..9c437638 100644 --- a/authn/test_helpers_test.go +++ b/authn/test_helpers_test.go @@ -58,7 +58,7 @@ func newTokenExtractorWithOptions(t *testing.T, opts ...Option) (*TokenExtractor }) })) - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}, opts...) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, opts...) if err != nil { jwksServer.Close() t.Fatalf("failed to create token extractor: %v", err) diff --git a/authn/token_parser.go b/authn/token_parser.go index 64ddb50b..f300a003 100644 --- a/authn/token_parser.go +++ b/authn/token_parser.go @@ -8,7 +8,6 @@ import ( "fmt" "log/slog" "net/http" - "slices" "strings" "sync" "time" @@ -112,11 +111,10 @@ const defaultJWKSCacheTTL = 5 * time.Minute // It validates JWT signatures using JWKS and resolves a user principal // or client principal based on grant type. type TokenExtractor struct { - jwksURL string - expIssuer string - expAudience string - expClientIDs []string - httpClient *http.Client + jwksURL string + expIssuer string + expAudience string + httpClient *http.Client // userClaims / clientClaims are the raw declarations accumulated from // WithUserClaims / WithClientClaims. They are validated and flattened into @@ -137,25 +135,24 @@ type TokenExtractor struct { jwksCacheTTL time.Duration } -func NewTokenExtractor(jwksURL, issuer, audience string, expectedClientIDs []string, opts ...Option) (*TokenExtractor, error) { - return newExtractor(jwksURL, issuer, audience, expectedClientIDs, &http.Client{Timeout: 10 * time.Second}, opts...) +func NewTokenExtractor(jwksURL, issuer, audience string, opts ...Option) (*TokenExtractor, error) { + return newExtractor(jwksURL, issuer, audience, &http.Client{Timeout: 10 * time.Second}, opts...) } -func NewTokenExtractorWithClient(jwksURL, issuer, audience string, expectedClientIDs []string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { +func NewTokenExtractorWithClient(jwksURL, issuer, audience string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { if httpClient == nil { - return NewTokenExtractor(jwksURL, issuer, audience, expectedClientIDs, opts...) + return NewTokenExtractor(jwksURL, issuer, audience, opts...) } - return newExtractor(jwksURL, issuer, audience, expectedClientIDs, httpClient, opts...) + return newExtractor(jwksURL, issuer, audience, httpClient, opts...) } // newExtractor is the single construction path, so a defaulted field // (rolesClaim) can only be forgotten in one place. -func newExtractor(jwksURL, issuer, audience string, expectedClientIDs []string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { +func newExtractor(jwksURL, issuer, audience string, httpClient *http.Client, opts ...Option) (*TokenExtractor, error) { extractor := &TokenExtractor{ jwksURL: strings.TrimSpace(jwksURL), expIssuer: strings.TrimSpace(issuer), expAudience: strings.TrimSpace(audience), - expClientIDs: expectedClientIDs, jwksCacheTTL: defaultJWKSCacheTTL, httpClient: httpClient, rolesClaim: defaultRolesClaim, @@ -182,9 +179,6 @@ func (te *TokenExtractor) validateConfig() error { if te.expAudience == "" { return fmt.Errorf("audience is not configured") } - if len(te.expClientIDs) == 0 { - return fmt.Errorf("client ids are not configured") - } if te.httpClient == nil { return fmt.Errorf("http client is not configured") } @@ -255,9 +249,6 @@ func (te *TokenExtractor) ExtractPrincipalFromHeader(authHeader string) (*Princi if claims.ClientID == "" { return nil, fmt.Errorf("jwt missing client_id claim") } - if !slices.Contains(te.expClientIDs, claims.ClientID) { - return nil, fmt.Errorf("unexpected client_id claim: %q", claims.ClientID) - } switch claims.GrantType { case AuthorizationCodeGrant: diff --git a/authn/token_parser_test.go b/authn/token_parser_test.go index fc96e78e..6e3665c2 100644 --- a/authn/token_parser_test.go +++ b/authn/token_parser_test.go @@ -113,13 +113,6 @@ func TestTokenExtractor_ExtractPrincipalFromHeader_MissingClaims(t *testing.T) { }, errSubstr: "jwt missing client_id claim", }, - { - name: "unexpected client_id", - mutate: func(claims jwt.MapClaims) { - claims["client_id"] = "OTHER" - }, - errSubstr: "unexpected client_id claim", - }, { name: "unsupported grant_type", mutate: func(claims jwt.MapClaims) { @@ -207,6 +200,26 @@ func TestTokenExtractor_UserToken_NoLongerRequiresEmailOrOU(t *testing.T) { } } +// TestTokenExtractor_AcceptsAnyClientIDForAudience documents that there is no +// client allowlist: aud binds a token to this service, and which clients may +// obtain such a token is the IdP's decision. Restricting callers by client_id +// is authorization policy, not authentication. +func TestTokenExtractor_AcceptsAnyClientIDForAudience(t *testing.T) { + extractor, privateKey, cleanup := newTokenExtractor(t) + defer cleanup() + + claims := newBaseClaims(ClientCredentialsGrant) + claims["client_id"] = "SOME_OTHER_CLIENT" + + principal, err := extractor.ExtractPrincipalFromHeader("Bearer " + signToken(t, privateKey, claims)) + if err != nil { + t.Fatalf("expected success for a token issued for this audience, got %v", err) + } + if principal.ClientPrincipal == nil || principal.ClientPrincipal.ClientID != "SOME_OTHER_CLIENT" { + t.Fatalf("unexpected principal: %#v", principal) + } +} + func TestTokenExtractor_JWKSIsCached(t *testing.T) { privateKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { @@ -245,7 +258,7 @@ func TestTokenExtractor_JWKSIsCached(t *testing.T) { t.Fatalf("failed to sign token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -328,7 +341,7 @@ func TestTokenExtractor_RefreshesJWKSOnUnknownKid(t *testing.T) { t.Fatalf("failed to sign new token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -382,7 +395,7 @@ func TestTokenExtractor_UnknownKidAfterRefresh(t *testing.T) { t.Fatalf("failed to sign token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -418,7 +431,7 @@ func TestTokenExtractor_JWKSFetchError(t *testing.T) { t.Fatalf("failed to sign token: %v", err) } - extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID, []string{testClientID}) + extractor, err := NewTokenExtractor(jwksServer.URL, testIssuer, testClientID) if err != nil { t.Fatalf("failed to create token extractor: %v", err) } @@ -430,11 +443,10 @@ func TestTokenExtractor_JWKSFetchError(t *testing.T) { func TestTokenExtractor_ValidateConfig_MissingHTTPClient(t *testing.T) { extractor := &TokenExtractor{ - jwksURL: "https://localhost/jwks", - expIssuer: testIssuer, - expAudience: testClientID, - expClientIDs: []string{testClientID}, - httpClient: nil, + jwksURL: "https://localhost/jwks", + expIssuer: testIssuer, + expAudience: testClientID, + httpClient: nil, } if err := extractor.validateConfig(); err == nil { From 7d2e7ffc8d6707077e82de0b663ea8930d355cb9 Mon Sep 17 00:00:00 2001 From: mushrafmim Date: Mon, 12 Oct 2026 13:33:14 +0530 Subject: [PATCH 3/3] docs: clarify audience token binding and client authorization in README --- authn/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authn/README.md b/authn/README.md index e9380ebf..7a614071 100644 --- a/authn/README.md +++ b/authn/README.md @@ -33,7 +33,7 @@ authn: > **Note:** every token must carry a `grant_type` claim — it decides user vs. client principal, and a token without it is rejected with `unsupported grant type`. This is not a standard access-token claim (RFC 9068 does not define it), so an IdP that omits it will not work with this package as-is. -> **Note:** `audience` is what binds a token to this service: any client the IdP issues a token for this audience is accepted. Which clients may call the service is authorization policy — restrict it in authz or the application using the principal's `ClientID`, not here. If your IdP sets `aud` to the requesting client rather than the API, configure an audience / API resource for the service in the IdP. +> **Note:** `audience` is what binds a token to this service: any client the IdP issues a token for this audience is accepted. Which clients may call the service is authorization policy, not this package's concern. For client-credentials tokens, restrict it in authz or the application using `ClientPrincipal.ClientID`. User (`authorization_code`) tokens produce a `UserPrincipal` that does not carry the originating client ID, so restricting those by client must be done another way (e.g. IdP-side policy or a dedicated audience per client group). If your IdP sets `aud` to the requesting client rather than the API, configure an audience / API resource for the service in the IdP. ## Middleware