From 900894161f34d27aa1481286eadf9624a3646d82 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Fri, 3 May 2024 09:23:40 +0200 Subject: [PATCH] fix(login): prevent init mail on idp registration (#7895) --- .../eventsourcing/eventstore/auth_request.go | 2 +- internal/command/user_human.go | 9 +- internal/command/user_human_test.go | 2 + internal/command/user_v2_email.go | 2 +- internal/command/user_v2_email_test.go | 32 ++-- internal/command/user_v2_human.go | 2 +- internal/command/user_v2_human_test.go | 172 ++++++++++++++++++ internal/command/user_v2_model_test.go | 4 + internal/repository/user/human_email.go | 14 +- 9 files changed, 204 insertions(+), 35 deletions(-) diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 1d0446249d..efdb20505a 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -551,7 +551,7 @@ func (repo *AuthRequestRepo) AutoRegisterExternalUser(ctx context.Context, regis } } human := command.AddHumanFromDomain(registerUser, metadatas, request, externalIDP) - err = repo.Command.AddUserHuman(ctx, resourceOwner, human, true, repo.UserCodeAlg) + err = repo.Command.AddUserHuman(ctx, resourceOwner, human, false, repo.UserCodeAlg) if err != nil { return err } diff --git a/internal/command/user_human.go b/internal/command/user_human.go index cbe2464f8a..4a7ef97add 100644 --- a/internal/command/user_human.go +++ b/internal/command/user_human.go @@ -288,7 +288,7 @@ func (c *Commands) addHumanCommandEmail(ctx context.Context, filter preparation. if human.Email.ReturnCode { human.EmailCode = &emailCode.Plain } - return append(cmds, user.NewHumanEmailCodeAddedEventV2(ctx, &a.Aggregate, emailCode.Crypted, emailCode.Expiry, human.Email.URLTemplate, human.Email.ReturnCode)), nil + return append(cmds, user.NewHumanEmailCodeAddedEventV2(ctx, &a.Aggregate, emailCode.Crypted, emailCode.Expiry, human.Email.URLTemplate, human.Email.ReturnCode, human.AuthRequestID)), nil } return cmds, nil } @@ -411,10 +411,9 @@ func (h *AddHuman) ensureDisplayName() { // and / or // - have no authentication method (password / passwordless) func (h *AddHuman) shouldAddInitCode() bool { - return !h.ExternalIDP && - !h.Email.Verified || - !h.Passwordless && - h.Password == "" + return len(h.Links) == 0 && + (!h.Email.Verified || + (!h.Passwordless && h.Password == "")) } // Deprecated: use commands.AddUserHuman diff --git a/internal/command/user_human_test.go b/internal/command/user_human_test.go index 7010d41756..daf037c951 100644 --- a/internal/command/user_human_test.go +++ b/internal/command/user_human_test.go @@ -519,6 +519,7 @@ func TestCommandSide_AddHuman(t *testing.T) { 1*time.Hour, "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}", false, + "", ), ), ), @@ -591,6 +592,7 @@ func TestCommandSide_AddHuman(t *testing.T) { 1*time.Hour, "", true, + "", ), ), ), diff --git a/internal/command/user_v2_email.go b/internal/command/user_v2_email.go index 9053b577dd..cc81f7399c 100644 --- a/internal/command/user_v2_email.go +++ b/internal/command/user_v2_email.go @@ -250,7 +250,7 @@ func generateCodeCommand(ctx context.Context, agg *eventstore.Aggregate, gen cry return nil, "", err } - cmd := user.NewHumanEmailCodeAddedEventV2(ctx, agg, value, gen.Expiry(), urlTmpl, returnCode) + cmd := user.NewHumanEmailCodeAddedEventV2(ctx, agg, value, gen.Expiry(), urlTmpl, returnCode, "") if returnCode { return cmd, plain, nil } diff --git a/internal/command/user_v2_email_test.go b/internal/command/user_v2_email_test.go index fc8fc1c703..5a7b4fb2ac 100644 --- a/internal/command/user_v2_email_test.go +++ b/internal/command/user_v2_email_test.go @@ -448,7 +448,7 @@ func TestCommands_ResendUserEmailCode(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -577,7 +577,7 @@ func TestCommands_ResendUserEmailCodeURLTemplate(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -696,7 +696,7 @@ func TestCommands_ResendUserEmailReturnCode(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1070,7 +1070,7 @@ func TestCommands_changeUserEmailWithGenerator(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1126,7 +1126,7 @@ func TestCommands_changeUserEmailWithGenerator(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", true, + "", true, "", ), ), ), @@ -1183,7 +1183,7 @@ func TestCommands_changeUserEmailWithGenerator(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}", false, + "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}", false, "", ), ), ), @@ -1308,7 +1308,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1352,7 +1352,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1366,7 +1366,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1416,7 +1416,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1430,7 +1430,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", true, + "", true, "", ), ), ), @@ -1481,7 +1481,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1495,7 +1495,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}", false, + "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}", false, "", ), ), ), @@ -1642,7 +1642,7 @@ func TestCommands_VerifyUserEmail(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1757,7 +1757,7 @@ func TestCommands_verifyUserEmailWithGenerator(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), @@ -1804,7 +1804,7 @@ func TestCommands_verifyUserEmailWithGenerator(t *testing.T) { Crypted: []byte("a"), }, time.Hour*1, - "", false, + "", false, "", ), ), ), diff --git a/internal/command/user_v2_human.go b/internal/command/user_v2_human.go index 3cd3269e4b..68955b4a07 100644 --- a/internal/command/user_v2_human.go +++ b/internal/command/user_v2_human.go @@ -317,7 +317,7 @@ func (c *Commands) changeUserEmail(ctx context.Context, cmds []eventstore.Comman if err != nil { return cmds, code, err } - cmds = append(cmds, user.NewHumanEmailCodeAddedEventV2(ctx, &wm.Aggregate().Aggregate, cryptoCode.Crypted, cryptoCode.Expiry, email.URLTemplate, email.ReturnCode)) + cmds = append(cmds, user.NewHumanEmailCodeAddedEventV2(ctx, &wm.Aggregate().Aggregate, cryptoCode.Crypted, cryptoCode.Expiry, email.URLTemplate, email.ReturnCode, "")) if email.ReturnCode { code = &cryptoCode.Plain } diff --git a/internal/command/user_v2_human_test.go b/internal/command/user_v2_human_test.go index 4a11ba0ea1..59f0935968 100644 --- a/internal/command/user_v2_human_test.go +++ b/internal/command/user_v2_human_test.go @@ -16,6 +16,7 @@ import ( "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/id" id_mock "github.com/zitadel/zitadel/internal/id/mock" + "github.com/zitadel/zitadel/internal/repository/idp" "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/zerrors" @@ -492,6 +493,7 @@ func TestCommandSide_AddUserHuman(t *testing.T) { 1*time.Hour, "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}", false, + "", ), ), ), @@ -565,6 +567,7 @@ func TestCommandSide_AddUserHuman(t *testing.T) { 1*time.Hour, "", true, + "", ), ), ), @@ -1224,6 +1227,173 @@ func TestCommandSide_AddUserHuman(t *testing.T) { wantID: "user1", }, }, + { + name: "register human with idp, unverified email, allow init mail, ok (verify mail)", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &userAgg.Aggregate, + true, + true, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewGoogleIDPAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "idpID", + "google", + "clientID", + nil, + []string{"openid"}, + idp.Options{}, + ), + ), + ), + expectPush( + newRegisterHumanEvent("email@test.ch", "", false, true, "", language.English), + user.NewHumanEmailCodeAddedEvent( + context.Background(), + &userAgg.Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("mailVerify"), + }, + time.Hour, + "authRequestID", + ), + user.NewUserIDPLinkAddedEvent( + context.Background(), + &userAgg.Aggregate, + "idpID", + "displayName", + "externalID", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"), + newCode: mockEncryptedCode("mailVerify", time.Hour), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &AddHuman{ + Username: "email@test.ch", + FirstName: "firstname", + LastName: "lastname", + Email: Email{ + Address: "email@test.ch", + }, + PreferredLanguage: language.English, + Register: true, + Links: []*AddLink{ + { + IDPID: "idpID", + DisplayName: "displayName", + IDPExternalID: "externalID", + }, + }, + AuthRequestID: "authRequestID", + }, + secretGenerator: GetMockSecretGenerator(t), + allowInitMail: true, + codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + wantID: "user1", + }, + }, + { + name: "register human with idp, verified email, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &userAgg.Aggregate, + true, + true, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewGoogleIDPAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "idpID", + "google", + "clientID", + nil, + []string{"openid"}, + idp.Options{}, + ), + ), + ), + expectPush( + newRegisterHumanEvent("email@test.ch", "", false, true, "", language.English), + user.NewHumanEmailVerifiedEvent( + context.Background(), + &userAgg.Aggregate, + ), + user.NewUserIDPLinkAddedEvent( + context.Background(), + &userAgg.Aggregate, + "idpID", + "displayName", + "externalID", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"), + newCode: mockEncryptedCode("mailVerify", time.Hour), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &AddHuman{ + Username: "email@test.ch", + FirstName: "firstname", + LastName: "lastname", + Email: Email{ + Address: "email@test.ch", + Verified: true, + }, + PreferredLanguage: language.English, + Register: true, + Links: []*AddLink{ + { + IDPID: "idpID", + DisplayName: "displayName", + IDPExternalID: "externalID", + }, + }, + AuthRequestID: "authRequestID", + }, + secretGenerator: GetMockSecretGenerator(t), + allowInitMail: false, + codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + wantID: "user1", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1566,6 +1736,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { time.Hour, "", false, + "", ), ), ), @@ -1745,6 +1916,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { time.Hour, "", true, + "", ), ), ), diff --git a/internal/command/user_v2_model_test.go b/internal/command/user_v2_model_test.go index 73513a86a6..a62bf4546a 100644 --- a/internal/command/user_v2_model_test.go +++ b/internal/command/user_v2_model_test.go @@ -672,6 +672,7 @@ func TestCommandSide_userHumanWriteModel_email(t *testing.T) { time.Hour*1, "", false, + "", ), ), ), @@ -733,6 +734,7 @@ func TestCommandSide_userHumanWriteModel_email(t *testing.T) { time.Hour*1, "", false, + "", ), ), eventFromEventPusher( @@ -791,6 +793,7 @@ func TestCommandSide_userHumanWriteModel_email(t *testing.T) { time.Hour*1, "", false, + "", ), ), eventFromEventPusher( @@ -858,6 +861,7 @@ func TestCommandSide_userHumanWriteModel_email(t *testing.T) { time.Hour*1, "", false, + "", ), ), eventFromEventPusher( diff --git a/internal/repository/user/human_email.go b/internal/repository/user/human_email.go index aeb2011c7a..942f101912 100644 --- a/internal/repository/user/human_email.go +++ b/internal/repository/user/human_email.go @@ -149,17 +149,7 @@ func NewHumanEmailCodeAddedEvent( expiry time.Duration, authRequestID string, ) *HumanEmailCodeAddedEvent { - return &HumanEmailCodeAddedEvent{ - BaseEvent: *eventstore.NewBaseEventForPush( - ctx, - aggregate, - HumanEmailCodeAddedType, - ), - Code: code, - Expiry: expiry, - TriggeredAtOrigin: http.ComposedOrigin(ctx), - AuthRequestID: authRequestID, - } + return NewHumanEmailCodeAddedEventV2(ctx, aggregate, code, expiry, "", false, authRequestID) } func NewHumanEmailCodeAddedEventV2( @@ -169,6 +159,7 @@ func NewHumanEmailCodeAddedEventV2( expiry time.Duration, urlTemplate string, codeReturned bool, + authRequestID string, ) *HumanEmailCodeAddedEvent { return &HumanEmailCodeAddedEvent{ BaseEvent: *eventstore.NewBaseEventForPush( @@ -181,6 +172,7 @@ func NewHumanEmailCodeAddedEventV2( URLTemplate: urlTemplate, CodeReturned: codeReturned, TriggeredAtOrigin: http.ComposedOrigin(ctx), + AuthRequestID: authRequestID, } }