fix(auth): refine OIDC identity handling and claim-mapping logs (#4028)

Rename getOpenIDUsername to getOpenIDIdentity and thread "identity"
through bearer OIDC, Basic-auth parsing, OAuth2Callback, and log fields.

Only fall back (and warn) to the default email claim when the configured
username claim is non-default but missing or empty.

Stop emitting Info logs when groups are absent on only UserInfo or only ID
token claims; log once at Debug when no groups remain after merging both.

Update ClaimMapping docs to mention username and groups claims; fix mTLS
extractIdentity comment typo; clarify GetAuthUserFromRequestSession doc.

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

pkg/api/authn.go

@@ -602,7 +602,7 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc {
 			}
 
 			// Try OIDC authentication first if configured
-			var username string
+			var identity string
 
 			var groups []string
 
@@ -611,21 +611,21 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc {
 
 				var authenticated bool
 
-				username, groups, authenticated, err = oidcAuthorizer.AuthenticateRequest(request.Context(), header)
+				identity, groups, authenticated, err = oidcAuthorizer.AuthenticateRequest(request.Context(), header)
 				if err == nil && authenticated {
 					// OIDC authentication succeeded
-					ctlr.Log.Debug().Str("username", username).Msg("the OIDC bearer authentication was successful")
+					ctlr.Log.Debug().Str("identity", identity).Msg("the OIDC bearer authentication was successful")
 
 					// Set user context for authorization
 					userAc := reqCtx.NewUserAccessControl()
-					userAc.SetUsername(username)
+					userAc.SetUsername(identity)
 					userAc.AddGroups(groups)
 					userAc.SaveOnRequest(request)
 
 					// Update user groups in MetaDB if available
 					if ctlr.MetaDB != nil {
 						if err := ctlr.MetaDB.SetUserGroups(request.Context(), groups); err != nil {
-							ctlr.Log.Error().Err(err).Str("username", username).Msg("failed to update user profile")
+							ctlr.Log.Error().Err(err).Str("identity", identity).Msg("failed to update user profile")
 							response.WriteHeader(http.StatusInternalServerError)
 
 							return
@@ -1027,10 +1027,10 @@ func getUsernamePasswordBasicAuth(request *http.Request) (string, string, error)
 		return "", "", zerr.ErrParsingAuthHeader
 	}
 
-	username := pair[0]
+	identity := pair[0]
 	passphrase := pair[1]
 
-	return username, passphrase, nil
+	return identity, passphrase, nil
 }
 
 func GetGithubUserInfo(ctx context.Context, client *github.Client, log log.Logger) (string, []string, error) {
@@ -1111,36 +1111,37 @@ const (
 	defaultGroupsClaim   = "groups"
 )
 
-// getOpenIDClaimMapping resolves the username and groups claim names for a given provider.
-// The third return value reports whether the username claim was explicitly configured
-// (false means the default "email" claim is being used as a fallback).
+// getOpenIDClaimMapping resolves which OIDC claims supply identity (see ClaimMapping.Username)
+// and groups for a given provider.
+// The third return value reports whether the identity username claim was explicitly configured
+// (false means the default "email" claim is being used).
 func getOpenIDClaimMapping(authConfig *config.AuthConfig, providerName string) (string, string, bool) {
-	usernameClaim := defaultUsernameClaim
+	identityClaim := defaultUsernameClaim
 	groupsClaim := defaultGroupsClaim
-	usernameConfigured := false
+	identityConfigured := false
 
 	if authConfig == nil || authConfig.OpenID == nil || providerName == "" {
-		return usernameClaim, groupsClaim, usernameConfigured
+		return identityClaim, groupsClaim, identityConfigured
 	}
 
 	providerConfig, ok := authConfig.OpenID.Providers[providerName]
 	if !ok || providerConfig.ClaimMapping == nil {
-		return usernameClaim, groupsClaim, usernameConfigured
+		return identityClaim, groupsClaim, identityConfigured
 	}
 
 	if providerConfig.ClaimMapping.Username != "" {
-		usernameClaim = providerConfig.ClaimMapping.Username
-		usernameConfigured = true
+		identityClaim = providerConfig.ClaimMapping.Username
+		identityConfigured = true
 	}
 
 	if providerConfig.ClaimMapping.Groups != "" {
 		groupsClaim = providerConfig.ClaimMapping.Groups
 	}
 
-	return usernameClaim, groupsClaim, usernameConfigured
+	return identityClaim, groupsClaim, identityConfigured
 }
 
-func getOpenIDUsername(info *oidc.UserInfo, claimName string) string {
+func getOpenIDIdentity(info *oidc.UserInfo, claimName string) string {
 	if info == nil {
 		return ""
 	}
@@ -1196,73 +1197,67 @@ func appendOpenIDGroups(groups []string, claims map[string]any, claimName string
 	return groups, false
 }
 
-// extractOpenIDIdentity resolves the username and groups for an OIDC callback
+// extractOpenIDIdentity resolves identity and groups for an OIDC callback
 // based on the provider's configured claim mapping. It returns the resolved
-// username, the deduplicated/sorted groups, and a boolean reporting whether
-// the username could be resolved at all (false means callers should reject).
+// identity string, the deduplicated/sorted groups, and a boolean reporting whether
+// identity could be resolved at all (false means callers should reject).
 func extractOpenIDIdentity(logger log.Logger, authConfig *config.AuthConfig, providerName string,
 	info *oidc.UserInfo, idTokenClaims map[string]any,
 ) (string, []string, bool) {
-	usernameClaim, groupsClaim, usernameConfigured := getOpenIDClaimMapping(authConfig, providerName)
+	identityClaim, groupsClaim, identityConfigured := getOpenIDClaimMapping(authConfig, providerName)
 
-	username := getOpenIDUsername(info, usernameClaim)
+	identity := getOpenIDIdentity(info, identityClaim)
+	fellBackToDefaultClaim := false
 
-	if username == "" && usernameConfigured {
-		configuredClaim := usernameClaim
-		usernameClaim = defaultUsernameClaim
-		usernameConfigured = false
-		username = getOpenIDUsername(info, usernameClaim)
+	if identity == "" && identityConfigured && identityClaim != defaultUsernameClaim {
+		fellBackToDefaultClaim = true
+		configuredClaim := identityClaim
+		identityClaim = defaultUsernameClaim
+		identity = getOpenIDIdentity(info, identityClaim)
 
 		logger.Warn().
 			Str("provider", providerName).
 			Str("claim", configuredClaim).
-			Msg("configured username claim missing or empty, falling back to email")
+			Msgf("configured username claim missing or empty, falling back to %q claim", defaultUsernameClaim)
 	}
 
-	if username == "" {
+	if identity == "" {
 		return "", nil, false
 	}
 
-	if usernameConfigured {
-		logger.Debug().
-			Str("provider", providerName).
-			Str("claim", usernameClaim).
-			Str("username", username).
-			Msg("extracted username from configured claim")
-	} else {
-		logger.Debug().
-			Str("provider", providerName).
-			Str("username", username).
-			Msg("using email as username (fallback)")
-	}
+	logger.Debug().
+		Str("provider", providerName).
+		Str("claim", identityClaim).
+		Str("identity", identity).
+		Bool("fellBackToDefaultClaim", fellBackToDefaultClaim).
+		Msg("extracted identity")
 
-	var (
-		groups      []string
-		groupsFound bool
-	)
+	var groups []string
 
 	if info != nil {
-		groups, groupsFound = appendOpenIDGroups(groups, info.Claims, groupsClaim)
-		if !groupsFound {
-			logger.Info().Msgf("failed to find any %q claim for user %s in UserInfo", groupsClaim, username)
-		}
+		groups, _ = appendOpenIDGroups(groups, info.Claims, groupsClaim)
 	}
 
 	if idTokenClaims != nil {
-		groups, groupsFound = appendOpenIDGroups(groups, idTokenClaims, groupsClaim)
-		if !groupsFound {
-			logger.Info().Msgf("failed to find any %q claim for user %s in IDTokenClaimsToken", groupsClaim, username)
-		}
+		groups, _ = appendOpenIDGroups(groups, idTokenClaims, groupsClaim)
 	}
 
 	slices.Sort(groups)
 	groups = slices.Compact(groups)
 
-	return username, groups, true
+	if len(groups) == 0 {
+		logger.Debug().
+			Str("provider", providerName).
+			Str("groupsClaim", groupsClaim).
+			Str("identity", identity).
+			Msg("no groups claim values found in UserInfo or ID token claims")
+	}
+
+	return identity, groups, true
 }
 
 // OAuth2Callback is the callback logic where openid/oauth2 will redirect back to our app.
-func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, state, email, provider string,
+func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, state, identity, provider string,
 	groups []string,
 ) (string, error) {
 	stateCookie, _ := ctlr.CookieStore.Get(r, "statecookie")
@@ -1283,24 +1278,24 @@ func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, st
 	}
 
 	userAc := reqCtx.NewUserAccessControl()
-	userAc.SetUsername(email)
+	userAc.SetUsername(identity)
 	userAc.AddGroups(groups)
 	userAc.SaveOnRequest(r)
 
 	// if this line has been reached, then a new session should be created
 	// if the `session` key is already on the cookie, it's not a valid one
 	secure := ctlr.Config.UseSecureSession()
-	if err := saveUserLoggedSession(ctlr.CookieStore, w, r, email, provider, secure, ctlr.Log); err != nil {
+	if err := saveUserLoggedSession(ctlr.CookieStore, w, r, identity, provider, secure, ctlr.Log); err != nil {
 		return "", err
 	}
 
 	if err := ctlr.MetaDB.SetUserGroups(r.Context(), groups); err != nil {
-		ctlr.Log.Error().Err(err).Str("identity", email).Msg("failed to update the user profile")
+		ctlr.Log.Error().Err(err).Str("identity", identity).Msg("failed to update the user profile")
 
 		return "", err
 	}
 
-	ctlr.Log.Info().Msgf("user profile set successfully for email %s", email)
+	ctlr.Log.Info().Str("identity", identity).Msg("user profile set successfully")
 
 	// redirect to UI
 	callbackUI, _ := stateCookie.Values["callback"].(string)
@@ -1316,7 +1311,7 @@ func hashUUID(uuid string) string {
 }
 
 /*
-GetAuthUserFromRequestSession returns identity
+GetAuthUserFromRequestSession returns the authenticated user identifier
 and auth status if on the request's cookie session is a logged in user.
 */
 func GetAuthUserFromRequestSession(cookieStore sessions.Store, request *http.Request, log log.Logger,
@@ -1452,7 +1447,7 @@ func extractIdentityFromCertificate(cert *x509.Certificate, identityAttribute st
 	}
 }
 
-// extractMTLSIdentity extracts identity from certificate using configured soidentity attributes with fallback chain.
+// extractMTLSIdentity extracts identity from certificate using configured identity attributes with fallback chain.
 func extractMTLSIdentity(cert *x509.Certificate, mtlsConfig *config.MTLSConfig) (string, error) {
 	identityAttributes := []string{"CommonName"} // Default
 	if mtlsConfig != nil && len(mtlsConfig.IdentityAttibutes) > 0 {

pkg/api/authn_internal_test.go

@@ -1,29 +1,32 @@
 package api
 
 import (
+	"slices"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/zitadel/oidc/v3/pkg/oidc"
 
 	"zotregistry.dev/zot/v2/pkg/api/config"
+	"zotregistry.dev/zot/v2/pkg/log"
 )
 
 func TestGetOpenIDClaimMapping(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name               string
-		authConfig         *config.AuthConfig
-		providerName       string
-		expectedUsername   string
-		expectedGroups     string
-		expectedConfigured bool
+		name                       string
+		authConfig                 *config.AuthConfig
+		providerName               string
+		expectedIdentityClaim      string
+		expectedGroups             string
+		expectedIdentityConfigured bool
 	}{
 		{
-			name:               "nil auth config uses defaults",
-			expectedUsername:   defaultUsernameClaim,
-			expectedGroups:     defaultGroupsClaim,
-			expectedConfigured: false,
+			name:                       "nil auth config uses defaults",
+			expectedIdentityClaim:      defaultUsernameClaim,
+			expectedGroups:             defaultGroupsClaim,
+			expectedIdentityConfigured: false,
 		},
 		{
 			name: "empty provider uses defaults",
@@ -32,9 +35,9 @@ func TestGetOpenIDClaimMapping(t *testing.T) {
 					Providers: map[string]config.OpenIDProviderConfig{},
 				},
 			},
-			expectedUsername:   defaultUsernameClaim,
-			expectedGroups:     defaultGroupsClaim,
-			expectedConfigured: false,
+			expectedIdentityClaim:      defaultUsernameClaim,
+			expectedGroups:             defaultGroupsClaim,
+			expectedIdentityConfigured: false,
 		},
 		{
 			name: "missing provider uses defaults",
@@ -43,10 +46,10 @@ func TestGetOpenIDClaimMapping(t *testing.T) {
 					Providers: map[string]config.OpenIDProviderConfig{},
 				},
 			},
-			providerName:       "oidc",
-			expectedUsername:   defaultUsernameClaim,
-			expectedGroups:     defaultGroupsClaim,
-			expectedConfigured: false,
+			providerName:               "oidc",
+			expectedIdentityClaim:      defaultUsernameClaim,
+			expectedGroups:             defaultGroupsClaim,
+			expectedIdentityConfigured: false,
 		},
 		{
 			name: "provider without claim mapping uses defaults",
@@ -57,10 +60,10 @@ func TestGetOpenIDClaimMapping(t *testing.T) {
 					},
 				},
 			},
-			providerName:       "oidc",
-			expectedUsername:   defaultUsernameClaim,
-			expectedGroups:     defaultGroupsClaim,
-			expectedConfigured: false,
+			providerName:               "oidc",
+			expectedIdentityClaim:      defaultUsernameClaim,
+			expectedGroups:             defaultGroupsClaim,
+			expectedIdentityConfigured: false,
 		},
 		{
 			name: "custom username and groups claims",
@@ -76,10 +79,10 @@ func TestGetOpenIDClaimMapping(t *testing.T) {
 					},
 				},
 			},
-			providerName:       "oidc",
-			expectedUsername:   "preferred_username",
-			expectedGroups:     "roles",
-			expectedConfigured: true,
+			providerName:               "oidc",
+			expectedIdentityClaim:      "preferred_username",
+			expectedGroups:             "roles",
+			expectedIdentityConfigured: true,
 		},
 		{
 			name: "custom groups keeps default username",
@@ -94,10 +97,10 @@ func TestGetOpenIDClaimMapping(t *testing.T) {
 					},
 				},
 			},
-			providerName:       "oidc",
-			expectedUsername:   defaultUsernameClaim,
-			expectedGroups:     "roles",
-			expectedConfigured: false,
+			providerName:               "oidc",
+			expectedIdentityClaim:      defaultUsernameClaim,
+			expectedGroups:             "roles",
+			expectedIdentityConfigured: false,
 		},
 	}
 
@@ -107,23 +110,91 @@ func TestGetOpenIDClaimMapping(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
 
-			usernameClaim, groupsClaim, usernameConfigured := getOpenIDClaimMapping(test.authConfig, test.providerName)
-			if usernameClaim != test.expectedUsername {
-				t.Fatalf("expected username claim %q, got %q", test.expectedUsername, usernameClaim)
-			}
-
-			if groupsClaim != test.expectedGroups {
-				t.Fatalf("expected groups claim %q, got %q", test.expectedGroups, groupsClaim)
-			}
-
-			if usernameConfigured != test.expectedConfigured {
-				t.Fatalf("expected usernameConfigured %t, got %t", test.expectedConfigured, usernameConfigured)
-			}
+			identityClaim, groupsClaim, identityConfigured := getOpenIDClaimMapping(test.authConfig, test.providerName)
+			assert.Equal(t, test.expectedIdentityClaim, identityClaim)
+			assert.Equal(t, test.expectedGroups, groupsClaim)
+			assert.Equal(t, test.expectedIdentityConfigured, identityConfigured)
 		})
 	}
+
+	logger := log.NewTestLogger()
+
+	t.Run("extractOpenIDIdentity_fallbackToEmailWhenConfiguredNonDefaultClaimMissing", func(t *testing.T) {
+		t.Parallel()
+
+		authConfig := &config.AuthConfig{
+			OpenID: &config.OpenIDConfig{
+				Providers: map[string]config.OpenIDProviderConfig{
+					"oidc": {
+						ClaimMapping: &config.ClaimMapping{
+							Username: "preferred_username",
+						},
+					},
+				},
+			},
+		}
+
+		info := &oidc.UserInfo{
+			UserInfoProfile: oidc.UserInfoProfile{
+				PreferredUsername: "",
+			},
+			UserInfoEmail: oidc.UserInfoEmail{Email: "user@example.com"},
+		}
+
+		identity, groups, ok := extractOpenIDIdentity(logger, authConfig, "oidc", info, nil)
+		assert.True(t, ok)
+		assert.Equal(t, "user@example.com", identity)
+		assert.Empty(t, groups)
+	})
+
+	t.Run("extractOpenIDIdentity_explicitDefaultClaimMissingRejects", func(t *testing.T) {
+		t.Parallel()
+
+		authConfig := &config.AuthConfig{
+			OpenID: &config.OpenIDConfig{
+				Providers: map[string]config.OpenIDProviderConfig{
+					"oidc": {
+						ClaimMapping: &config.ClaimMapping{
+							Username: "email",
+						},
+					},
+				},
+			},
+		}
+
+		info := &oidc.UserInfo{
+			UserInfoEmail: oidc.UserInfoEmail{Email: ""},
+		}
+
+		identity, groups, ok := extractOpenIDIdentity(logger, authConfig, "oidc", info, nil)
+		assert.False(t, ok)
+		assert.Empty(t, identity)
+		assert.Nil(t, groups)
+	})
+
+	t.Run("extractOpenIDIdentity_mergesSortsDedupesGroupsFromUserInfoAndIDToken", func(t *testing.T) {
+		t.Parallel()
+
+		info := &oidc.UserInfo{
+			UserInfoEmail: oidc.UserInfoEmail{Email: "user@example.com"},
+			Claims: map[string]any{
+				"groups": []any{"b", "a", "", nil, "a"},
+			},
+		}
+
+		idTokenClaims := map[string]any{
+			"groups": []string{"c", "b"},
+		}
+
+		identity, groups, ok := extractOpenIDIdentity(logger, nil, "", info, idTokenClaims)
+		assert.True(t, ok)
+		assert.Equal(t, "user@example.com", identity)
+		expected := []string{"a", "b", "c"}
+		assert.True(t, slices.Equal(expected, groups))
+	})
 }
 
-func TestGetOpenIDUsername(t *testing.T) {
+func TestGetOpenIDIdentity(t *testing.T) {
 	t.Parallel()
 
 	info := &oidc.UserInfo{
@@ -161,10 +232,8 @@ func TestGetOpenIDUsername(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
 
-			username := getOpenIDUsername(test.info, test.claim)
-			if username != test.expected {
-				t.Fatalf("expected username %q, got %q", test.expected, username)
-			}
+			identity := getOpenIDIdentity(test.info, test.claim)
+			assert.Equal(t, test.expected, identity)
 		})
 	}
 }
@@ -260,19 +329,8 @@ func TestAppendOpenIDGroups(t *testing.T) {
 			t.Parallel()
 
 			groups, found := appendOpenIDGroups(test.groups, test.claims, test.claim)
-			if found != test.expectedFound {
-				t.Fatalf("expected found %t, got %t", test.expectedFound, found)
-			}
-
-			if len(groups) != len(test.expected) {
-				t.Fatalf("expected groups %v, got %v", test.expected, groups)
-			}
-
-			for i := range test.expected {
-				if groups[i] != test.expected[i] {
-					t.Fatalf("expected groups %v, got %v", test.expected, groups)
-				}
-			}
+			assert.Equal(t, test.expectedFound, found)
+			assert.Equal(t, test.expected, groups)
 		})
 	}
 }

pkg/api/config/config.go

@@ -315,8 +315,8 @@ type OpenIDProviderConfig struct {
 	ClaimMapping    *ClaimMapping `mapstructure:",omitempty"`
 }
 
-// ClaimMapping specifies how OpenID claims are mapped to application fields.
-// It allows customization of which claim is used as the username when authenticating users.
+// ClaimMapping specifies how OpenID claims are mapped to Zot identities:
+// which claim supplies the username and which claim supplies group membership.
 type ClaimMapping struct {
 	// Username specifies which OpenID claim to use as the username for the authenticated user.
 	// Acceptable values include "preferred_username", "email", "sub", "name", or any custom claim name.