From 94a21ac397809573f70f1bc0d7eb966c81eaf3db Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 2 Oct 2026 09:35:19 +0000 Subject: [PATCH 1/2] fix(oidc): parse JSON array strings in mapstructure decode hook Co-Authored-By: Claude Opus 4.6 --- authentication/oidc.go | 7 ++ authentication/oidc_test.go | 211 ++++++++++++++++++++++++++++++++++++ 2 files changed, 218 insertions(+) diff --git a/authentication/oidc.go b/authentication/oidc.go index 3db182605..5e33c7156 100644 --- a/authentication/oidc.go +++ b/authentication/oidc.go @@ -74,6 +74,13 @@ func stringOrSliceDecodeHook() mapstructure.DecodeHookFunc { switch v := data.(type) { case string: + // Check if the string is a JSON-encoded array (e.g. from config generators + // that store arrays in string-typed fields). + var arr []string + if err := json.Unmarshal([]byte(v), &arr); err == nil { + return StringOrSlice(arr), nil + } + return StringOrSlice{v}, nil case []interface{}: result := make([]string, len(v)) diff --git a/authentication/oidc_test.go b/authentication/oidc_test.go index ad0b5e8dc..0b1d39fa1 100644 --- a/authentication/oidc_test.go +++ b/authentication/oidc_test.go @@ -7,12 +7,14 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "reflect" "testing" "time" "github.com/coreos/go-oidc/v3/oidc" "github.com/go-jose/go-jose/v4" "github.com/go-kit/log" + "github.com/mitchellh/mapstructure" ) // testOIDCEnv holds a test OIDC server, signing key, and helpers for @@ -479,6 +481,215 @@ func TestCheckAuthClaimLists(t *testing.T) { } } +// newAuthenticatorViaMapstructure creates an oidcAuthenticator by running +// the raw config map through mapstructure.Decode with stringOrSliceDecodeHook, +// exactly as the production code path does. This exercises the decode hook +// for StringOrSlice fields (UsernameClaim, GroupClaim) where values may arrive +// as plain strings — including JSON-encoded arrays that config generators +// sometimes produce. +func (env *testOIDCEnv) newAuthenticatorViaMapstructure(t *testing.T, rawConfig map[string]interface{}) *oidcAuthenticator { + t.Helper() + + var config oidcConfig + + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: stringOrSliceDecodeHook(), + Result: &config, + }) + if err != nil { + t.Fatalf("create mapstructure decoder: %v", err) + } + + if err := decoder.Decode(rawConfig); err != nil { + t.Fatalf("mapstructure decode: %v", err) + } + + ctx := oidc.ClientContext(context.Background(), env.server.Client()) + + provider, err := oidc.NewProvider(ctx, env.issuerURL) + if err != nil { + t.Fatalf("create OIDC provider: %v", err) + } + + verifier := provider.Verifier(&oidc.Config{ + ClientID: "test-client", + SkipClientIDCheck: true, + }) + + return &oidcAuthenticator{ + tenant: "test-tenant", + logger: log.NewNopLogger(), + config: config, + provider: provider, + verifier: verifier, + client: env.server.Client(), + } +} + +func TestCheckAuthMapstructureJSONArrayString(t *testing.T) { + env := newTestOIDCEnv(t) + defer env.server.Close() + + tests := []struct { + name string + rawConfig map[string]interface{} + extraClaims map[string]interface{} + wantCode int + wantSubject string + wantGroups []string + }{ + { + name: "groupClaim JSON array string JWT has second claim", + rawConfig: map[string]interface{}{ + "clientID": "test-client", + "issuerURL": "PLACEHOLDER", + "usernameClaim": "preferred_username", + "groupClaim": `["org_id", "rh-org-id"]`, + }, + extraClaims: map[string]interface{}{ + "preferred_username": "testuser", + "rh-org-id": "12541229", + }, + wantCode: http.StatusOK, + wantSubject: "testuser", + wantGroups: []string{"12541229"}, + }, + { + name: "groupClaim JSON array string JWT has first claim", + rawConfig: map[string]interface{}{ + "clientID": "test-client", + "issuerURL": "PLACEHOLDER", + "usernameClaim": "preferred_username", + "groupClaim": `["org_id", "rh-org-id"]`, + }, + extraClaims: map[string]interface{}{ + "preferred_username": "testuser", + "org_id": "6340056", + }, + wantCode: http.StatusOK, + wantSubject: "testuser", + wantGroups: []string{"6340056"}, + }, + { + name: "usernameClaim JSON array string JWT has fallback claim", + rawConfig: map[string]interface{}{ + "clientID": "test-client", + "issuerURL": "PLACEHOLDER", + "usernameClaim": `["email", "preferred_username"]`, + "groupClaim": "groups", + }, + extraClaims: map[string]interface{}{ + "preferred_username": "user@example.com", + "groups": []interface{}{"team-a"}, + }, + wantCode: http.StatusOK, + wantSubject: "user@example.com", + wantGroups: []string{"team-a"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.rawConfig["issuerURL"] = env.issuerURL + + auth := env.newAuthenticatorViaMapstructure(t, tt.rawConfig) + token := env.signToken(t, tt.extraClaims) + + ctx, _, httpCode, _ := auth.checkAuth(context.Background(), token) + + if httpCode != tt.wantCode { + t.Fatalf("HTTP status = %d, want %d", httpCode, tt.wantCode) + } + + if tt.wantCode != http.StatusOK { + return + } + + if tt.wantSubject != "" { + gotSubject, ok := GetSubject(ctx) + if !ok { + t.Fatal("expected subject in context, got none") + } + + if gotSubject != tt.wantSubject { + t.Errorf("subject = %q, want %q", gotSubject, tt.wantSubject) + } + } + + if tt.wantGroups != nil { + gotGroups, ok := GetGroups(ctx) + if !ok { + t.Fatal("expected groups in context, got none") + } + + if len(gotGroups) != len(tt.wantGroups) { + t.Fatalf("got %d groups %v, want %d groups %v", + len(gotGroups), gotGroups, len(tt.wantGroups), tt.wantGroups) + } + + for i, g := range gotGroups { + if g != tt.wantGroups[i] { + t.Errorf("groups[%d] = %q, want %q", i, g, tt.wantGroups[i]) + } + } + } + }) + } +} + +func TestStringOrSliceDecodeHookJSONArrayString(t *testing.T) { + hook := stringOrSliceDecodeHook() + fn := hook.(func(reflect.Type, reflect.Type, interface{}) (interface{}, error)) + + targetType := reflect.TypeOf(StringOrSlice{}) + + tests := []struct { + name string + input interface{} + want StringOrSlice + }{ + { + name: "JSON array string is parsed into slice", + input: `["org_id", "rh-org-id"]`, + want: StringOrSlice{"org_id", "rh-org-id"}, + }, + { + name: "plain string stays as single-element slice", + input: "preferred_username", + want: StringOrSlice{"preferred_username"}, + }, + { + name: "string that is not valid JSON array stays as single-element", + input: "not-json", + want: StringOrSlice{"not-json"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := fn(reflect.TypeOf(tt.input), targetType, tt.input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + got, ok := result.(StringOrSlice) + if !ok { + t.Fatalf("expected StringOrSlice, got %T", result) + } + + if len(got) != len(tt.want) { + t.Fatalf("got %v (len %d), want %v (len %d)", got, len(got), tt.want, len(tt.want)) + } + + for i, v := range got { + if v != tt.want[i] { + t.Errorf("element %d = %q, want %q", i, v, tt.want[i]) + } + } + }) + } +} + func TestStringOrSliceUnmarshalJSON(t *testing.T) { tests := []struct { name string From e91b0227873173180b9f36c6d3183fa174e85e28 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 2 Oct 2026 09:37:58 +0000 Subject: [PATCH 2/2] fix(oidc): parse JSON array strings in mapstructure decode hook Co-Authored-By: Claude Opus 4.6 --- authentication/oidc_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/authentication/oidc_test.go b/authentication/oidc_test.go index 0b1d39fa1..537e6785c 100644 --- a/authentication/oidc_test.go +++ b/authentication/oidc_test.go @@ -517,9 +517,9 @@ func (env *testOIDCEnv) newAuthenticatorViaMapstructure(t *testing.T, rawConfig }) return &oidcAuthenticator{ - tenant: "test-tenant", - logger: log.NewNopLogger(), - config: config, + tenant: "test-tenant", + logger: log.NewNopLogger(), + config: config, provider: provider, verifier: verifier, client: env.server.Client(),