From f04b5a43f966518a0bf2a9b7bcb3b8e4f15cfc65 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Fri, 19 Jun 2026 20:35:22 +0200
Subject: [PATCH] fix(auth): don't link OIDC login to an arbitrary local user
 when the email claim is empty

The email-only fallback built &user.User{Issuer: IssuerLocal, Email: cl.Email}.
GetUserWithEmail ANDs only non-zero fields, so an empty cl.Email degenerated the
lookup to issuer-only, matching an arbitrary local user and wrongly linking the
OIDC login to that account (account-takeover / wrong-link).

Guard the email-only branch to only run when cl.Email != "" so an empty email
falls through to user creation (nothing to safely match on). As defense in depth,
skip empty username candidates too, so no fallback search can ever degenerate to
issuer-only ({Issuer, Username:"", Email:""}).

This is a latent pre-existing issue (the pre-#2945 code had the same
searchUser.Email = cl.Email with no empty guard); the #2945 refactor only
surfaced it. Adds a regression test.
---
 pkg/modules/auth/openid/openid.go      | 11 +++++++--
 pkg/modules/auth/openid/openid_test.go | 31 ++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go
index 6361e4cf5..b5eb230b2 100644
--- a/pkg/modules/auth/openid/openid.go
+++ b/pkg/modules/auth/openid/openid.go
@@ -385,7 +385,12 @@ func fallbackSearchUsers(cl *claims, provider *Provider, idToken *oidc.IDToken)
 	// UUID, like PocketID) still link to an existing local account.
 	var searches []*user.User
 	if provider.UsernameFallback {
-		searches = append(searches, &user.User{Issuer: user.IssuerLocal, Username: idToken.Subject, Email: fallbackEmail})
+		// Skip empty username candidates: GetUserWithEmail ANDs only non-zero fields, so a
+		// {Issuer, Username:"", Email:""} would degenerate to an issuer-only lookup and link
+		// an arbitrary local user. idToken.Subject is non-empty per OIDC, but guard anyway.
+		if idToken.Subject != "" {
+			searches = append(searches, &user.User{Issuer: user.IssuerLocal, Username: idToken.Subject, Email: fallbackEmail})
+		}
 		preferred := strings.ReplaceAll(cl.PreferredUsername, " ", "-")
 		if preferred != "" && preferred != idToken.Subject {
 			searches = append(searches, &user.User{Issuer: user.IssuerLocal, Username: preferred, Email: fallbackEmail})
@@ -393,7 +398,9 @@ func fallbackSearchUsers(cl *claims, provider *Provider, idToken *oidc.IDToken)
 	}
 	// EmailFallback without UsernameFallback: a single email-only lookup (the caller only
 	// runs this when at least one fallback is enabled, so EmailFallback is guaranteed here).
-	if len(searches) == 0 {
+	// Only add it when there is a real email — an empty email would degenerate to an
+	// issuer-only lookup and link an arbitrary local user.
+	if len(searches) == 0 && cl.Email != "" {
 		searches = append(searches, &user.User{Issuer: user.IssuerLocal, Email: cl.Email})
 	}
 
diff --git a/pkg/modules/auth/openid/openid_test.go b/pkg/modules/auth/openid/openid_test.go
index df4d1204b..35bb27547 100644
--- a/pkg/modules/auth/openid/openid_test.go
+++ b/pkg/modules/auth/openid/openid_test.go
@@ -328,6 +328,37 @@ func TestGetOrCreateUser(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, usersBefore, usersAfter, "no new user should have been created")
 	})
+	t.Run("ProviderFallback: empty email claim does not link to an arbitrary local user", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		usersBefore, err := s.Count(&user.User{})
+		require.NoError(t, err)
+
+		// EmailFallback on, no username fallback, and the IdP sent no email claim. The
+		// email-only search must not degenerate to an issuer-only lookup matching an
+		// arbitrary local user. With no email there is nothing safe to match on, so the
+		// flow falls through to user creation (which then errors because an email is
+		// required) rather than silently linking an existing local account.
+		cl := &claims{
+			Email:             "",
+			PreferredUsername: "brandNewOidcUser",
+		}
+		provider := &Provider{
+			EmailFallback: true,
+		}
+		idToken := &oidc.IDToken{Issuer: "https://some.issuer", Subject: "opaque-subject-no-email"}
+
+		u, err := getOrCreateUser(s, cl, provider, idToken)
+		// Must not have linked an existing local user.
+		require.Error(t, err, "an empty email must not silently link an existing local user")
+		assert.Nil(t, u, "no existing local user should be returned for an empty email claim")
+
+		usersAfter, err := s.Count(&user.User{})
+		require.NoError(t, err)
+		assert.Equal(t, usersBefore, usersAfter, "no user should have been linked or created from an empty email claim")
+	})
 	t.Run("ProviderFallback: Match to existing local user  on username and email", func(t *testing.T) {
 
 		db.LoadAndAssertFixtures(t)