From e1c6cb9037ddfb1e0a6592519b233010b42fff6f Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 28 Oct 2026 13:39:52 +0000 Subject: [PATCH] oidc: keep email_verified with the address it came with Fixes ANT-2026-MX8FKG4K (cherry picked from commit 208043c88d2205c45f34b5b27439c04cb41b0f6b, adapted: only the new test appended) --- hscontrol/oidc.go | 19 +++++++++-- hscontrol/oidc_test.go | 73 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index 0d32203b8..d17787aef 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -313,8 +313,7 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( if userinfo != nil && userinfo.Claims(&userinfo2) == nil && userinfo2.Sub == claims.Sub { // Update the user with the userinfo claims (with id token claims as fallback). // TODO(kradalby): there might be more interesting fields here that we have not found yet. - claims.Email = cmp.Or(userinfo2.Email, claims.Email) - claims.EmailVerified = cmp.Or(userinfo2.EmailVerified, claims.EmailVerified) + claims.Email, claims.EmailVerified = mergeEmailClaims(&claims, &userinfo2) claims.Username = cmp.Or(userinfo2.PreferredUsername, claims.Username) claims.Name = cmp.Or(userinfo2.Name, claims.Name) @@ -446,6 +445,22 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( } } +// mergeEmailClaims keeps each verification flag with the address it describes. +func mergeEmailClaims( + claims *types.OIDCClaims, + userinfo *types.OIDCUserInfo, +) (string, types.FlexibleBoolean) { + if userinfo.Email == "" { + return claims.Email, claims.EmailVerified + } + + if userinfo.Email == claims.Email { + return claims.Email, claims.EmailVerified || userinfo.EmailVerified + } + + return userinfo.Email, userinfo.EmailVerified +} + func (a *AuthProviderOIDC) determineNodeExpiry(idTokenExpiration time.Time) *time.Time { if a.cfg.UseExpiryFromToken { return &idTokenExpiration diff --git a/hscontrol/oidc_test.go b/hscontrol/oidc_test.go index e1a9f8baa..73e00190c 100644 --- a/hscontrol/oidc_test.go +++ b/hscontrol/oidc_test.go @@ -659,3 +659,76 @@ func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) { assert.Equal(t, before.Hostname(), after.Hostname()) assert.True(t, before.Hostinfo().AsStruct().Equal(after.Hostinfo().AsStruct())) } + +type oidcUserWithInfo struct { + mockoidc.MockUser + + info types.OIDCUserInfo +} + +func (u *oidcUserWithInfo) Userinfo(_ []string) ([]byte, error) { + return json.Marshal(u.info) +} + +func TestOIDCEmailClaims(t *testing.T) { + for _, tt := range []struct { + name string + tokenEmail string + tokenVerified bool + infoEmail string + infoVerified bool + wantStatus int + }{ + {"different address", "guest@other.example", true, "boss@corp.example", false, http.StatusUnauthorized}, + {"flag without address", "boss@corp.example", false, "", true, http.StatusUnauthorized}, + {"same address token flag", "boss@corp.example", true, "boss@corp.example", false, http.StatusOK}, + {"same address userinfo flag", "boss@corp.example", false, "boss@corp.example", true, http.StatusOK}, + {"no userinfo email", "boss@corp.example", true, "", false, http.StatusOK}, + {"different verified address", "guest@other.example", false, "boss@corp.example", true, http.StatusOK}, + } { + t.Run(tt.name, func(t *testing.T) { + b := newOIDCBrowser(t) + provider, ok := b.app.authProvider.(*AuthProviderOIDC) + require.True(t, ok) + + provider.cfg.EmailVerifiedRequired = true + provider.cfg.AllowedDomains = []string{"corp.example"} + + b.idp.QueueUser(&oidcUserWithInfo{ + Subject: "email-pair", + Email: tt.tokenEmail, + EmailVerified: tt.tokenVerified, + info: types.OIDCUserInfo{ + Sub: "email-pair", + Email: tt.infoEmail, + EmailVerified: types.FlexibleBoolean(tt.infoVerified), + }, + }) + _, registerURL := b.pendingNode(t) + status, _, _ := b.get(t, registerURL) + assert.Equal(t, tt.wantStatus, status) + }) + } +} + +func TestMergeEmailClaims(t *testing.T) { + for _, tt := range []struct { + name string + token types.OIDCClaims + info types.OIDCUserInfo + email string + verified types.FlexibleBoolean + }{ + {"different address", types.OIDCClaims{Email: "old@example.com", EmailVerified: true}, types.OIDCUserInfo{Email: "new@example.com"}, "new@example.com", false}, + {"flag without address", types.OIDCClaims{Email: "old@example.com"}, types.OIDCUserInfo{EmailVerified: true}, "old@example.com", false}, + {"same address token flag", types.OIDCClaims{Email: "same@example.com", EmailVerified: true}, types.OIDCUserInfo{Email: "same@example.com"}, "same@example.com", true}, + {"same address userinfo flag", types.OIDCClaims{Email: "same@example.com"}, types.OIDCUserInfo{Email: "same@example.com", EmailVerified: true}, "same@example.com", true}, + {"no userinfo email", types.OIDCClaims{Email: "old@example.com", EmailVerified: true}, types.OIDCUserInfo{}, "old@example.com", true}, + } { + t.Run(tt.name, func(t *testing.T) { + email, verified := mergeEmailClaims(&tt.token, &tt.info) + assert.Equal(t, tt.email, email) + assert.Equal(t, tt.verified, verified) + }) + } +}