fix(auth): match OIDC username fallback on preferred_username as well as subject

When the username fallback is enabled, the local account lookup only matched
the local username against the OIDC `sub` claim. For providers that issue an
opaque, random `sub` (e.g. PocketID's UUID), this never matched a real local
username, so a duplicate user was created instead of linking the existing
local account.

The fallback now tries the `sub` first (preserving today's behaviour for IdPs
where sub == username) and, if no match is found, the `preferred_username`
claim (normalized the same way user creation normalizes it). When EmailFallback
is also enabled, the email continues to be ANDed with each username candidate.

Configuring an OIDC provider already delegates trust to it, and the username
fallback is an admin-enabled opt-in, so matching the admin-trusted
`preferred_username` is appropriate; `sub` matching is kept for backward
compatibility.

Fixes #2705

pkg/modules/auth/openid/openid.go

@@ -392,33 +392,52 @@ func getOrCreateUser(s *xorm.Session, cl *claims, provider *Provider, idToken *o
 
 	if !alreadyCreatedFromIssuer && (provider.EmailFallback || provider.UsernameFallback) {
 
-		// try finding the user on fallback mappingproperties
+		// try finding the user on fallback mapping properties
 
-		searchUser := &user.User{
-			Issuer: user.IssuerLocal,
-		}
-		if provider.UsernameFallback {
-			// Match oidc subject on username as each is unique identifier in its own referential
-			// Discouraged if multiple account providers are used.
-			searchUser.Username = idToken.Subject
-		}
+		// GetUserWithEmail ANDs all non-zero fields, so email is combined with each
+		// username candidate when EmailFallback is also enabled.
+		fallbackEmail := ""
 		if provider.EmailFallback {
 			// Used alone, allow for someone to connect from various provider to the same account
 			// Discouraged for untrusted provider where someone can set email without verification
 			// Note : mapping on email prevent from auto-updating user email
-			searchUser.Email = cl.Email
+			fallbackEmail = cl.Email
 		}
 
-		// Check if the user exists for the given fallback matching options
-		u, err = user.GetUserWithEmail(s, searchUser)
-		if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrUserStatusError(err) {
-			return nil, err
+		// Try the subject first (keeps working for IdPs where sub == username), then the
+		// preferred_username. The latter lets providers with an opaque sub (e.g. a random
+		// UUID, like PocketID) still link to an existing local account.
+		var usernameCandidates []string
+		if provider.UsernameFallback {
+			usernameCandidates = append(usernameCandidates, idToken.Subject)
+			preferred := strings.ReplaceAll(cl.PreferredUsername, " ", "-")
+			if preferred != "" && preferred != idToken.Subject {
+				usernameCandidates = append(usernameCandidates, preferred)
+			}
+		} else {
+			usernameCandidates = append(usernameCandidates, "")
 		}
-		fallbackMatchFound = err == nil || user.IsErrUserStatusError(err)
 
-		// Same as above: disabled/locked user found via fallback — return early.
-		if fallbackMatchFound && user.IsErrUserStatusError(err) {
-			return u, nil
+		for _, username := range usernameCandidates {
+			searchUser := &user.User{
+				Issuer:   user.IssuerLocal,
+				Username: username,
+				Email:    fallbackEmail,
+			}
+
+			u, err = user.GetUserWithEmail(s, searchUser)
+			if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrUserStatusError(err) {
+				return nil, err
+			}
+			fallbackMatchFound = err == nil || user.IsErrUserStatusError(err)
+
+			// Same as above: disabled/locked user found via fallback — return early.
+			if fallbackMatchFound && user.IsErrUserStatusError(err) {
+				return u, nil
+			}
+			if fallbackMatchFound {
+				break
+			}
 		}
 	}
 

pkg/modules/auth/openid/openid_test.go

@@ -254,6 +254,53 @@ func TestGetOrCreateUser(t *testing.T) {
 		assert.Equal(t, user.IssuerLocal, u.Issuer, "User should be a local one")
 		assert.Equal(t, 11, int(u.ID), "user id 11 expected")
 	})
+	t.Run("ProviderFallback: Match to existing local user on preferred_username when sub differs", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		cl := &claims{
+			PreferredUsername: "user11",
+		}
+		provider := &Provider{
+			UsernameFallback: true,
+		}
+		// PocketID-style: the subject is an opaque UUID that does not match any local username.
+		idToken := &oidc.IDToken{Issuer: "https://some.issuer", Subject: "c0ffee00-dead-beef-cafe-000000000011"}
+
+		u, err := getOrCreateUser(s, cl, provider, idToken)
+		require.NoError(t, err)
+		err = s.Commit()
+		require.NoError(t, err)
+
+		assert.Equal(t, "user11", u.Username, "should link to the local user matching preferred_username")
+		assert.Equal(t, user.IssuerLocal, u.Issuer, "User should be a local one")
+		assert.Equal(t, 11, int(u.ID), "user id 11 expected")
+
+		// No duplicate user must be created for the opaque subject.
+		db.AssertMissing(t, "users", map[string]interface{}{
+			"subject": idToken.Subject,
+		})
+	})
+	t.Run("ProviderFallback: Falls back to sub when preferred_username is empty", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		cl := &claims{
+			PreferredUsername: "",
+		}
+		provider := &Provider{
+			UsernameFallback: true,
+		}
+		idToken := &oidc.IDToken{Issuer: "https://some.issuer", Subject: "user11"}
+
+		u, err := getOrCreateUser(s, cl, provider, idToken)
+		require.NoError(t, err)
+		assert.Equal(t, idToken.Subject, u.Username, "subject should match username")
+		assert.Equal(t, user.IssuerLocal, u.Issuer, "User should be a local one")
+		assert.Equal(t, 11, int(u.ID), "user id 11 expected")
+	})
 	t.Run("ProviderFallback: Match to existing local user on email", func(t *testing.T) {
 		db.LoadAndAssertFixtures(t)
 		s := db.NewSession()