oidc: keep email_verified with the address it came with

Fixes ANT-2026-MX8FKG4K
This commit is contained in:
Kristoffer Dalby committed 2026-10-29 14:00:02 +01:00
1 parent b5bebf9b32
commit 208043c88d
3 files changed
+91 -2

No files matched your search

+1
View File
@@ -167,6 +167,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`,
- Only the newest follow-up waits on a pending registration or SSH check; older ones are answered at once - Only the newest follow-up waits on a pending registration or SSH check; older ones are answered at once
- Registration locks are freed once no registration uses them - Registration locks are freed once no registration uses them
- Concurrent deletes of one node free its addresses and announce its removal once - Concurrent deletes of one node free its addresses and announce its removal once
- OIDC verifies an email against the document supplying it; an `email_verified` flag without an address no longer verifies another document's email
## 0.29.7 (202x-xx-xx) ## 0.29.7 (202x-xx-xx)
+17 -2
View File
@@ -314,8 +314,7 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
if userinfo != nil && userinfo.Claims(&userinfo2) == nil && userinfo2.Sub == claims.Sub { if userinfo != nil && userinfo.Claims(&userinfo2) == nil && userinfo2.Sub == claims.Sub {
// Update the user with the userinfo claims (with id token claims as fallback). // 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. // TODO(kradalby): there might be more interesting fields here that we have not found yet.
claims.Email = cmp.Or(userinfo2.Email, claims.Email) claims.Email, claims.EmailVerified = mergeEmailClaims(&claims, &userinfo2)
claims.EmailVerified = cmp.Or(userinfo2.EmailVerified, claims.EmailVerified)
claims.Username = cmp.Or(userinfo2.PreferredUsername, claims.Username) claims.Username = cmp.Or(userinfo2.PreferredUsername, claims.Username)
claims.Name = cmp.Or(userinfo2.Name, claims.Name) claims.Name = cmp.Or(userinfo2.Name, claims.Name)
@@ -445,6 +444,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 { func (a *AuthProviderOIDC) determineNodeExpiry(idTokenExpiration time.Time) *time.Time {
if a.cfg.UseExpiryFromToken { if a.cfg.UseExpiryFromToken {
return &idTokenExpiration return &idTokenExpiration
+73
View File
@@ -621,6 +621,79 @@ func TestRouterMethodNotAllowedIncludesAllow(t *testing.T) {
assert.Equal(t, []string{http.MethodGet}, resp.Header.Values("Allow")) assert.Equal(t, []string{http.MethodGet}, resp.Header.Values("Allow"))
} }
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)
})
}
}
func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) { func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) {
t.Parallel() t.Parallel()