diff --git a/pkg/api/authn.go b/pkg/api/authn.go index e3a639d1..371c2927 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -487,6 +487,7 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { // Initialize authorizers based on configuration var traditionalAuthorizer *BearerAuthorizer + var oidcAuthorizer *OIDCBearerAuthorizer // Traditional bearer auth with public key/certificate @@ -565,12 +566,15 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { } // Try OIDC authentication first if configured - authenticated := false var username string + var groups []string if oidcAuthorizer != nil { var err error + + var authenticated bool + username, groups, authenticated, err = oidcAuthorizer.AuthenticateRequest(request.Context(), header) if err == nil && authenticated { // OIDC authentication succeeded @@ -587,12 +591,14 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { if err := ctlr.MetaDB.SetUserGroups(request.Context(), groups); err != nil { ctlr.Log.Error().Err(err).Str("username", username).Msg("failed to update user profile") response.WriteHeader(http.StatusInternalServerError) + return } } amCtx := acCtrlr.getAuthnMiddlewareContext(BEARER, request) next.ServeHTTP(response, request.WithContext(amCtx)) //nolint:contextcheck + return } } @@ -620,6 +626,7 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { amCtx := acCtrlr.getAuthnMiddlewareContext(BEARER, request) next.ServeHTTP(response, request.WithContext(amCtx)) //nolint:contextcheck + return } diff --git a/pkg/api/bearer_oidc.go b/pkg/api/bearer_oidc.go index 500ab8a5..ea3db08a 100644 --- a/pkg/api/bearer_oidc.go +++ b/pkg/api/bearer_oidc.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "regexp" + "slices" "time" "github.com/coreos/go-oidc/v3/oidc" @@ -26,7 +27,9 @@ type OIDCBearerAuthorizer struct { } // NewOIDCBearerAuthorizer creates a new OIDC bearer token authorizer. -func NewOIDCBearerAuthorizer(ctx context.Context, oidcConfig *config.BearerOIDCConfig, log log.Logger) (*OIDCBearerAuthorizer, error) { +func NewOIDCBearerAuthorizer(ctx context.Context, oidcConfig *config.BearerOIDCConfig, + log log.Logger, +) (*OIDCBearerAuthorizer, error) { if oidcConfig == nil { return nil, fmt.Errorf("%w: OIDC config is nil", zerr.ErrBadConfig) } @@ -47,9 +50,9 @@ func NewOIDCBearerAuthorizer(ctx context.Context, oidcConfig *config.BearerOIDCC // Configure verifier verifierConfig := &oidc.Config{ - ClientID: "", // We'll check audiences manually + ClientID: "", // We'll check audiences manually SkipIssuerCheck: oidcConfig.SkipIssuerVerification, - SkipClientIDCheck: true, // Check audiences manually to support multiple + SkipClientIDCheck: true, // Check audiences manually to support multiple SkipExpiryCheck: false, Now: time.Now, } @@ -86,6 +89,7 @@ func (a *OIDCBearerAuthorizer) Authenticate(ctx context.Context, header string) idToken, err := a.verifier.Verify(ctx, tokenString) if err != nil { a.log.Debug().Err(err).Msg("OIDC token verification failed") + return "", nil, fmt.Errorf("%w: %w", zerr.ErrInvalidBearerToken, err) } @@ -94,11 +98,12 @@ func (a *OIDCBearerAuthorizer) Authenticate(ctx context.Context, header string) a.log.Debug().Str("token_aud", fmt.Sprintf("%v", idToken.Audience)). Strs("accepted_aud", a.audiences). Msg("token audience not accepted") + return "", nil, fmt.Errorf("%w: audience not accepted", zerr.ErrInvalidBearerToken) } // Extract claims - var claims map[string]interface{} + var claims map[string]any if err := idToken.Claims(&claims); err != nil { return "", nil, fmt.Errorf("%w: failed to extract claims: %w", zerr.ErrInvalidBearerToken, err) } @@ -107,6 +112,7 @@ func (a *OIDCBearerAuthorizer) Authenticate(ctx context.Context, header string) username := a.extractUsername(claims) if username == "" { a.log.Debug().Interface("claims", claims).Msg("failed to extract username from token") + return "", nil, fmt.Errorf("%w: no username found in token", zerr.ErrInvalidBearerToken) } @@ -121,18 +127,18 @@ func (a *OIDCBearerAuthorizer) Authenticate(ctx context.Context, header string) // verifyAudience checks if the token's audience matches any of the accepted audiences. func (a *OIDCBearerAuthorizer) verifyAudience(token *oidc.IDToken) bool { tokenAudiences := token.Audience + for _, tokenAud := range tokenAudiences { - for _, acceptedAud := range a.audiences { - if tokenAud == acceptedAud { - return true - } + if slices.Contains(a.audiences, tokenAud) { + return true } } + return false } // extractUsername extracts the username from token claims based on claim mapping configuration. -func (a *OIDCBearerAuthorizer) extractUsername(claims map[string]interface{}) string { +func (a *OIDCBearerAuthorizer) extractUsername(claims map[string]any) string { // Default claim to use for username claimName := "sub" @@ -162,23 +168,23 @@ func (a *OIDCBearerAuthorizer) extractUsername(claims map[string]interface{}) st // extractGroups extracts groups from token claims. // It looks for "groups" claim as an array of strings. -func (a *OIDCBearerAuthorizer) extractGroups(claims map[string]interface{}) []string { +func (a *OIDCBearerAuthorizer) extractGroups(claims map[string]any) []string { groups := []string{} // Try to extract groups from "groups" claim if groupsClaim, ok := claims["groups"]; ok { - switch v := groupsClaim.(type) { - case []interface{}: - for _, g := range v { + switch groupsValue := groupsClaim.(type) { + case []any: + for _, g := range groupsValue { if str, ok := g.(string); ok { groups = append(groups, str) } } case []string: - groups = v + groups = groupsValue case string: // Single group as string - groups = append(groups, v) + groups = append(groups, groupsValue) } } @@ -187,7 +193,9 @@ func (a *OIDCBearerAuthorizer) extractGroups(claims map[string]interface{}) []st // AuthenticateRequest is a convenience method that handles the full authentication flow // and returns whether authentication succeeded and any error. -func (a *OIDCBearerAuthorizer) AuthenticateRequest(ctx context.Context, authHeader string) (string, []string, bool, error) { +func (a *OIDCBearerAuthorizer) AuthenticateRequest(ctx context.Context, + authHeader string, +) (string, []string, bool, error) { username, groups, err := a.Authenticate(ctx, authHeader) if err != nil { return "", nil, false, err diff --git a/pkg/api/bearer_oidc_test.go b/pkg/api/bearer_oidc_test.go index 37ee300b..50605b7c 100644 --- a/pkg/api/bearer_oidc_test.go +++ b/pkg/api/bearer_oidc_test.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "crypto/rsa" "encoding/json" + "maps" "net/http" "net/http/httptest" "testing" @@ -28,10 +29,12 @@ func mockOIDCServer(t *testing.T, pubKey *rsa.PublicKey) *httptest.Server { // OpenID configuration endpoint mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - config := map[string]interface{}{ + + config := map[string]any{ "issuer": "http://" + r.Host, "jwks_uri": "http://" + r.Host + "/jwks", } + _ = json.NewEncoder(w).Encode(config) }) @@ -47,7 +50,7 @@ func mockOIDCServer(t *testing.T, pubKey *rsa.PublicKey) *httptest.Server { Use: "sig", } - jwks := map[string]interface{}{ + jwks := map[string]any{ "keys": []jose.JSONWebKey{jwk}, } @@ -58,7 +61,9 @@ func mockOIDCServer(t *testing.T, pubKey *rsa.PublicKey) *httptest.Server { } // createTestOIDCToken creates a test OIDC ID token. -func createTestOIDCToken(privKey *rsa.PrivateKey, issuer, audience, subject string, claims map[string]interface{}) (string, error) { +func createTestOIDCToken(privKey *rsa.PrivateKey, issuer, audience, subject string, + claims map[string]any, +) (string, error) { now := time.Now() tokenClaims := jwt.MapClaims{ @@ -70,9 +75,7 @@ func createTestOIDCToken(privKey *rsa.PrivateKey, issuer, audience, subject stri } // Add additional claims - for k, v := range claims { - tokenClaims[k] = v - } + maps.Copy(tokenClaims, claims) token := jwt.NewWithClaims(jwt.SigningMethodRS256, tokenClaims) token.Header["kid"] = "test-key-id" @@ -172,9 +175,10 @@ func TestOIDCBearerAuthorizer(t *testing.T) { Convey("Valid token with groups", func() { subject := "test-user" - groups := []string{"group1", "group2"} - token, err := createTestOIDCToken(privKey, issuer, audience, subject, map[string]interface{}{ - "groups": groups, + testGroups := []string{"group1", "group2"} + + token, err := createTestOIDCToken(privKey, issuer, audience, subject, map[string]any{ + "groups": testGroups, }) So(err, ShouldBeNil) @@ -183,7 +187,7 @@ func TestOIDCBearerAuthorizer(t *testing.T) { username, extractedGroups, err := authorizer.Authenticate(ctx, authHeader) So(err, ShouldBeNil) So(username, ShouldEqual, subject) - So(extractedGroups, ShouldResemble, groups) + So(extractedGroups, ShouldResemble, testGroups) }) Convey("Token with wrong audience should fail", func() { @@ -243,7 +247,8 @@ func TestOIDCBearerAuthorizer(t *testing.T) { Convey("Extract username from custom claim", func() { subject := "original-sub" - token, err := createTestOIDCToken(privKey, issuer, audience, subject, map[string]interface{}{ + + token, err := createTestOIDCToken(privKey, issuer, audience, subject, map[string]any{ customClaimName: customUsername, }) So(err, ShouldBeNil)