From 8e22672b72ecc675aed3c69a161d4eeb02158bfb Mon Sep 17 00:00:00 2001 From: fadymak Date: Thu, 6 Aug 2026 17:06:57 +0200 Subject: [PATCH] fix: unlinking an identity should respect the autoconfirm behaviour --- internal/api/identity.go | 4 ++-- internal/api/identity_test.go | 16 ++++++++++++++ internal/models/user.go | 11 +++++----- internal/models/user_test.go | 40 +++++++++++++++++++++++++++-------- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/internal/api/identity.go b/internal/api/identity.go index b6301c751..bd1a4e240 100644 --- a/internal/api/identity.go +++ b/internal/api/identity.go @@ -74,7 +74,7 @@ func (a *API) DeleteIdentity(w http.ResponseWriter, r *http.Request) error { return apierrors.NewInternalServerError("Database error updating user phone").WithInternalError(terr) } default: - if terr := user.UpdateUserEmailFromIdentities(tx); terr != nil { + if terr := user.UpdateUserEmailFromIdentities(tx, config.Mailer.Autoconfirm); terr != nil { if models.IsUniqueConstraintViolatedError(terr) { return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailConflictIdentityNotDeletable, "Unable to unlink identity due to email conflict").WithInternalError(terr) } @@ -185,7 +185,7 @@ func (a *API) linkIdentityToUser(r *http.Request, ctx context.Context, tx *stora } if targetUser.GetEmail() == "" { - if terr := targetUser.UpdateUserEmailFromIdentities(tx); terr != nil { + if terr := targetUser.UpdateUserEmailFromIdentities(tx, a.config.Mailer.Autoconfirm); terr != nil { if models.IsUniqueConstraintViolatedError(terr) { return nil, apierrors.NewBadRequestError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg) } diff --git a/internal/api/identity_test.go b/internal/api/identity_test.go index 92151a58c..b869e3977 100644 --- a/internal/api/identity_test.go +++ b/internal/api/identity_test.go @@ -222,6 +222,8 @@ func (ts *IdentityTestSuite) TestUnlinkIdentity() { func (ts *IdentityTestSuite) TestUnlinkIdentityEmailVerification() { ts.Config.Security.ManualLinkingEnabled = true + originalAutoconfirm := ts.Config.Mailer.Autoconfirm + defer func() { ts.Config.Mailer.Autoconfirm = originalAutoconfirm }() boolPtr := func(b bool) *bool { return &b } cases := []struct { @@ -229,6 +231,7 @@ func (ts *IdentityTestSuite) TestUnlinkIdentityEmailVerification() { // value of email_verified on the remaining identity; nil means // the key is absent from identity_data emailVerified *bool + autoconfirm bool expectedConfirmed bool }{ { @@ -246,11 +249,24 @@ func (ts *IdentityTestSuite) TestUnlinkIdentityEmailVerification() { emailVerified: nil, expectedConfirmed: false, }, + { + desc: "Autoconfirm keeps an unverified identity email confirmed", + emailVerified: boolPtr(false), + autoconfirm: true, + expectedConfirmed: true, + }, + { + desc: "Autoconfirm keeps an identity without email_verified confirmed", + emailVerified: nil, + autoconfirm: true, + expectedConfirmed: true, + }, } for _, c := range cases { ts.Run(c.desc, func() { ts.SetupTest() + ts.Config.Mailer.Autoconfirm = c.autoconfirm u, err := models.NewUser("", "primary@example.com", "password", ts.Config.JWT.Aud, nil) require.NoError(ts.T(), err) require.NoError(ts.T(), ts.API.db.Create(u)) diff --git a/internal/models/user.go b/internal/models/user.go index f88a9729b..c469bf566 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -274,8 +274,9 @@ func (u *User) UpdateAppMetaDataProviders(tx *storage.Connection) error { } // UpdateUserEmail updates the user's email to one of the identity's email -// if the current email used doesn't match any of the identities email -func (u *User) UpdateUserEmailFromIdentities(tx *storage.Connection) error { +// if the current email used doesn't match any of the identities email. +// Unverified identity emails remain confirmed when mailer autoconfirm is enabled. +func (u *User) UpdateUserEmailFromIdentities(tx *storage.Connection, mailerAutoconfirm bool) error { identities, terr := FindIdentitiesByUserID(tx, u.ID) if terr != nil { return terr @@ -337,9 +338,9 @@ func (u *User) UpdateUserEmailFromIdentities(tx *storage.Connection) error { if terr := u.ClearAllPendingTokens(tx); terr != nil { return terr } - if primaryIdentity.GetEmail() == "" || !primaryIdentity.IsEmailVerified() { - // the promoted email was never verified by the IdP or ourselves, - // so the user's email can no longer be considered confirmed + if primaryIdentity.GetEmail() == "" || (!primaryIdentity.IsEmailVerified() && !mailerAutoconfirm) { + // the promoted email was neither verified by the IdP nor covered by + // the project's autoconfirm policy, so it can't remain confirmed u.EmailConfirmedAt = nil if terr := tx.UpdateOnly(u, "email_confirmed_at"); terr != nil { return terr diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 502392605..9e6e3cb43 100644 --- a/internal/models/user_test.go +++ b/internal/models/user_test.go @@ -487,7 +487,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailSuccess() { require.NoError(ts.T(), ts.db.Create(secondaryIdentity)) // UpdateUserEmail should not do anything and the user's email should still use the primaryIdentity - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), primaryIdentity.GetEmail(), userA.GetEmail()) require.NotNil(ts.T(), userA.EmailConfirmedAt) @@ -496,7 +496,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailSuccess() { // UpdateUserEmail should update the user to use the secondary identity's email // and clear the confirmation state since the promoted email was never verified - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), secondaryIdentity.GetEmail(), userA.GetEmail()) require.Nil(ts.T(), userA.EmailConfirmedAt) require.Equal(ts.T(), false, userA.UserMetaData["email_verified"]) @@ -518,7 +518,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailFromVerifiedIdentity() { // the promoted email was verified by the identity provider so the // user's confirmation state should be kept - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), secondaryIdentity.GetEmail(), userA.GetEmail()) require.NotNil(ts.T(), userA.EmailConfirmedAt) } @@ -539,12 +539,34 @@ func (ts *UserTestSuite) TestUpdateUserEmailFromUnverifiedIdentity() { // the promoted email was never verified so the user should no longer // be considered confirmed - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), secondaryIdentity.GetEmail(), userA.GetEmail()) require.Nil(ts.T(), userA.EmailConfirmedAt) require.Equal(ts.T(), false, userA.UserMetaData["email_verified"]) } +func (ts *UserTestSuite) TestUpdateUserEmailFromUnverifiedIdentityWithAutoconfirm() { + userA, err := NewUser("", "foo@example.com", "", "authenticated", nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(userA)) + require.NoError(ts.T(), userA.Confirm(ts.db)) + + secondaryIdentity, err := NewIdentity(userA, "google", map[string]any{ + "sub": userA.ID.String(), + "email": "bar@example.com", + "email_verified": false, + }) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(secondaryIdentity)) + + // autoconfirm keeps the promoted email confirmed even when the identity + // provider did not verify it + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, true)) + require.Equal(ts.T(), secondaryIdentity.GetEmail(), userA.GetEmail()) + require.NotNil(ts.T(), userA.EmailConfirmedAt) + require.Equal(ts.T(), true, userA.UserMetaData["email_verified"]) +} + func (ts *UserTestSuite) TestUpdateUserEmailPrefersVerifiedIdentity() { userA, err := NewUser("", "foo@example.com", "", "authenticated", nil) require.NoError(ts.T(), err) @@ -569,7 +591,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailPrefersVerifiedIdentity() { // the verified identity should be promoted even though the unverified // identity was created first - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), verifiedIdentity.GetEmail(), userA.GetEmail()) require.NotNil(ts.T(), userA.EmailConfirmedAt) } @@ -602,7 +624,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailVerifiedConflictFallsBack() { // the verified identity's email is taken by userB so the unverified // identity is promoted instead, which unconfirms the user - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), unverifiedIdentity.GetEmail(), userA.GetEmail()) require.Nil(ts.T(), userA.EmailConfirmedAt) require.Equal(ts.T(), false, userA.UserMetaData["email_verified"]) @@ -659,7 +681,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailClearsStaleTokens() { // promoting another identity's email must revoke every outstanding // token addressed to the previous email - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), secondaryIdentity.GetEmail(), userA.GetEmail()) userA, err = FindUserByID(ts.db, userA.ID) @@ -713,7 +735,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailFromEmptyClearsStaleTokens() { // promoting an identity's email over an empty one is still a primary // email transition, so outstanding tokens must be revoked - require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db)) + require.NoError(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false)) require.Equal(ts.T(), identity.GetEmail(), userA.GetEmail()) userA, err = FindUserByID(ts.db, userA.ID) @@ -755,7 +777,7 @@ func (ts *UserTestSuite) TestUpdateUserEmailFailure() { // UpdateUserEmail should fail with the email unique constraint violation error // since userB is using the secondary identity's email - require.ErrorIs(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db), UserEmailUniqueConflictError{}) + require.ErrorIs(ts.T(), userA.UpdateUserEmailFromIdentities(ts.db, false), UserEmailUniqueConflictError{}) require.Equal(ts.T(), primaryIdentity.GetEmail(), userA.GetEmail()) }