From 0e9ebed8d0a0373bd82712de288df9912251d1e7 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Tue, 14 May 2024 09:20:31 +0200 Subject: [PATCH] fix: import totp in add human user with secret (#7936) * fix: import totp in add human user with secret * fix: import totp in add human user with secret * fix: import totp in add human user with secret * fix: review comment changes --- .../session/v2/session_integration_test.go | 217 ++++++++++++++---- internal/api/grpc/user/v2/user.go | 1 + .../api/grpc/user/v2/user_integration_test.go | 46 ++++ internal/command/session_test.go | 4 +- internal/command/user_human.go | 3 + internal/command/user_human_otp.go | 8 +- internal/command/user_human_otp_test.go | 4 +- internal/command/user_v2_human.go | 11 + internal/command/user_v2_human_test.go | 95 ++++++++ internal/command/user_v2_totp_test.go | 5 +- internal/domain/human_otp.go | 10 +- internal/integration/client.go | 31 +++ proto/zitadel/user/v2beta/user_service.proto | 11 + 13 files changed, 395 insertions(+), 51 deletions(-) diff --git a/internal/api/grpc/session/v2/session_integration_test.go b/internal/api/grpc/session/v2/session_integration_test.go index 6818b627e80..645db76733f 100644 --- a/internal/api/grpc/session/v2/session_integration_test.go +++ b/internal/api/grpc/session/v2/session_integration_test.go @@ -13,6 +13,7 @@ import ( "github.com/pquerna/otp/totp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/zitadel/logging" "google.golang.org/grpc/metadata" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/durationpb" @@ -43,26 +44,43 @@ func TestMain(m *testing.M) { Client = Tester.Client.SessionV2 CTX, _ = Tester.WithAuthorization(ctx, integration.OrgOwner), errCtx - User = Tester.CreateHumanUser(CTX) - Tester.Client.UserV2.VerifyEmail(CTX, &user.VerifyEmailRequest{ - UserId: User.GetUserId(), - VerificationCode: User.GetEmailCode(), - }) - Tester.Client.UserV2.VerifyPhone(CTX, &user.VerifyPhoneRequest{ - UserId: User.GetUserId(), - VerificationCode: User.GetPhoneCode(), - }) - Tester.SetUserPassword(CTX, User.GetUserId(), integration.UserPassword, false) - Tester.RegisterUserPasskey(CTX, User.GetUserId()) - DeactivatedUser = Tester.CreateHumanUser(CTX) - Tester.Client.UserV2.DeactivateUser(CTX, &user.DeactivateUserRequest{UserId: DeactivatedUser.GetUserId()}) - LockedUser = Tester.CreateHumanUser(CTX) - Tester.Client.UserV2.LockUser(CTX, &user.LockUserRequest{UserId: LockedUser.GetUserId()}) + User = createFullUser(CTX) + DeactivatedUser = createDeactivatedUser(CTX) + LockedUser = createLockedUser(CTX) return m.Run() }()) } -func verifyCurrentSession(t testing.TB, id, token string, sequence uint64, window time.Duration, metadata map[string][]byte, userAgent *session.UserAgent, expirationWindow time.Duration, factors ...wantFactor) *session.Session { +func createFullUser(ctx context.Context) *user.AddHumanUserResponse { + userResp := Tester.CreateHumanUser(ctx) + Tester.Client.UserV2.VerifyEmail(ctx, &user.VerifyEmailRequest{ + UserId: userResp.GetUserId(), + VerificationCode: userResp.GetEmailCode(), + }) + Tester.Client.UserV2.VerifyPhone(ctx, &user.VerifyPhoneRequest{ + UserId: userResp.GetUserId(), + VerificationCode: userResp.GetPhoneCode(), + }) + Tester.SetUserPassword(ctx, userResp.GetUserId(), integration.UserPassword, false) + Tester.RegisterUserPasskey(ctx, userResp.GetUserId()) + return userResp +} + +func createDeactivatedUser(ctx context.Context) *user.AddHumanUserResponse { + userResp := Tester.CreateHumanUser(ctx) + _, err := Tester.Client.UserV2.DeactivateUser(ctx, &user.DeactivateUserRequest{UserId: userResp.GetUserId()}) + logging.OnError(err).Fatal("deactivate human user") + return userResp +} + +func createLockedUser(ctx context.Context) *user.AddHumanUserResponse { + userResp := Tester.CreateHumanUser(ctx) + _, err := Tester.Client.UserV2.LockUser(ctx, &user.LockUserRequest{UserId: userResp.GetUserId()}) + logging.OnError(err).Fatal("lock human user") + return userResp +} + +func verifyCurrentSession(t testing.TB, id, token string, sequence uint64, window time.Duration, metadata map[string][]byte, userAgent *session.UserAgent, expirationWindow time.Duration, userID string, factors ...wantFactor) *session.Session { t.Helper() require.NotEmpty(t, id) require.NotEmpty(t, token) @@ -89,7 +107,7 @@ func verifyCurrentSession(t testing.TB, id, token string, sequence uint64, windo assert.WithinRange(t, s.GetExpirationDate().AsTime(), time.Now().Add(-expirationWindow), time.Now().Add(expirationWindow)) } - verifyFactors(t, s.GetFactors(), window, factors) + verifyFactors(t, s.GetFactors(), window, userID, factors) return s } @@ -106,14 +124,14 @@ const ( wantOTPEmailFactor ) -func verifyFactors(t testing.TB, factors *session.Factors, window time.Duration, want []wantFactor) { +func verifyFactors(t testing.TB, factors *session.Factors, window time.Duration, userID string, want []wantFactor) { for _, w := range want { switch w { case wantUserFactor: uf := factors.GetUser() assert.NotNil(t, uf) assert.WithinRange(t, uf.GetVerifiedAt().AsTime(), time.Now().Add(-window), time.Now().Add(window)) - assert.Equal(t, User.GetUserId(), uf.GetId()) + assert.Equal(t, userID, uf.GetId()) case wantPasswordFactor: pf := factors.GetPassword() assert.NotNil(t, pf) @@ -318,7 +336,7 @@ func TestServer_CreateSession(t *testing.T) { require.NoError(t, err) integration.AssertDetails(t, tt.want, got) - verifyCurrentSession(t, got.GetSessionId(), got.GetSessionToken(), got.GetDetails().GetSequence(), time.Minute, tt.req.GetMetadata(), tt.wantUserAgent, tt.wantExpirationWindow, tt.wantFactors...) + verifyCurrentSession(t, got.GetSessionId(), got.GetSessionToken(), got.GetDetails().GetSequence(), time.Minute, tt.req.GetMetadata(), tt.wantUserAgent, tt.wantExpirationWindow, User.GetUserId(), tt.wantFactors...) }) } } @@ -341,7 +359,7 @@ func TestServer_CreateSession_webauthn(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) assertionData, err := Tester.WebAuthN.CreateAssertionResponse(createResp.GetChallenges().GetWebAuthN().GetPublicKeyCredentialRequestOptions(), true) require.NoError(t, err) @@ -357,7 +375,7 @@ func TestServer_CreateSession_webauthn(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), updateResp.GetSessionToken(), updateResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantWebAuthNFactorUserVerified) + verifyCurrentSession(t, createResp.GetSessionId(), updateResp.GetSessionToken(), updateResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantWebAuthNFactorUserVerified) } func TestServer_CreateSession_successfulIntent(t *testing.T) { @@ -372,7 +390,7 @@ func TestServer_CreateSession_successfulIntent(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) intentID, token, _, _ := Tester.CreateSuccessfulOAuthIntent(t, CTX, idpID, User.GetUserId(), "id") updateResp, err := Client.SetSession(CTX, &session.SetSessionRequest{ @@ -386,7 +404,7 @@ func TestServer_CreateSession_successfulIntent(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), updateResp.GetSessionToken(), updateResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantIntentFactor) + verifyCurrentSession(t, createResp.GetSessionId(), updateResp.GetSessionToken(), updateResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantIntentFactor) } func TestServer_CreateSession_successfulIntent_instant(t *testing.T) { @@ -407,7 +425,7 @@ func TestServer_CreateSession_successfulIntent_instant(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantIntentFactor) + verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantIntentFactor) } func TestServer_CreateSession_successfulIntentUnknownUserID(t *testing.T) { @@ -423,7 +441,7 @@ func TestServer_CreateSession_successfulIntentUnknownUserID(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) idpUserID := "id" intentID, token, _, _ := Tester.CreateSuccessfulOAuthIntent(t, CTX, idpID, "", idpUserID) @@ -451,7 +469,7 @@ func TestServer_CreateSession_successfulIntentUnknownUserID(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), updateResp.GetSessionToken(), updateResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantIntentFactor) + verifyCurrentSession(t, createResp.GetSessionId(), updateResp.GetSessionToken(), updateResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantIntentFactor) } func TestServer_CreateSession_startedIntentFalseToken(t *testing.T) { @@ -467,7 +485,7 @@ func TestServer_CreateSession_startedIntentFalseToken(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), createResp.GetSessionToken(), createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) intentID := Tester.CreateIntent(t, CTX, idpID) _, err = Client.SetSession(CTX, &session.SetSessionRequest{ @@ -514,12 +532,133 @@ func registerOTPEmail(ctx context.Context, t *testing.T, userID string) { require.NoError(t, err) } +func TestServer_SetSession_flow_totp(t *testing.T) { + userExisting := createFullUser(CTX) + + // create new, empty session + createResp, err := Client.CreateSession(CTX, &session.CreateSessionRequest{}) + require.NoError(t, err) + sessionToken := createResp.GetSessionToken() + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, "") + + t.Run("check user", func(t *testing.T) { + resp, err := Client.SetSession(CTX, &session.SetSessionRequest{ + SessionId: createResp.GetSessionId(), + SessionToken: sessionToken, + Checks: &session.Checks{ + User: &session.CheckUser{ + Search: &session.CheckUser_UserId{ + UserId: userExisting.GetUserId(), + }, + }, + }, + }) + require.NoError(t, err) + sessionToken = resp.GetSessionToken() + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, userExisting.GetUserId(), wantUserFactor) + }) + + t.Run("check webauthn, user verified (passkey)", func(t *testing.T) { + resp, err := Client.SetSession(CTX, &session.SetSessionRequest{ + SessionId: createResp.GetSessionId(), + SessionToken: sessionToken, + Challenges: &session.RequestChallenges{ + WebAuthN: &session.RequestChallenges_WebAuthN{ + Domain: Tester.Config.ExternalDomain, + UserVerificationRequirement: session.UserVerificationRequirement_USER_VERIFICATION_REQUIREMENT_REQUIRED, + }, + }, + }) + require.NoError(t, err) + verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, userExisting.GetUserId()) + sessionToken = resp.GetSessionToken() + + assertionData, err := Tester.WebAuthN.CreateAssertionResponse(resp.GetChallenges().GetWebAuthN().GetPublicKeyCredentialRequestOptions(), true) + require.NoError(t, err) + + resp, err = Client.SetSession(CTX, &session.SetSessionRequest{ + SessionId: createResp.GetSessionId(), + SessionToken: sessionToken, + Checks: &session.Checks{ + WebAuthN: &session.CheckWebAuthN{ + CredentialAssertionData: assertionData, + }, + }, + }) + require.NoError(t, err) + sessionToken = resp.GetSessionToken() + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, userExisting.GetUserId(), wantUserFactor, wantWebAuthNFactorUserVerified) + }) + + userAuthCtx := Tester.WithAuthorizationToken(CTX, sessionToken) + Tester.RegisterUserU2F(userAuthCtx, userExisting.GetUserId()) + totpSecret := registerTOTP(userAuthCtx, t, userExisting.GetUserId()) + registerOTPSMS(userAuthCtx, t, userExisting.GetUserId()) + registerOTPEmail(userAuthCtx, t, userExisting.GetUserId()) + + t.Run("check TOTP", func(t *testing.T) { + code, err := totp.GenerateCode(totpSecret, time.Now()) + require.NoError(t, err) + resp, err := Client.SetSession(CTX, &session.SetSessionRequest{ + SessionId: createResp.GetSessionId(), + SessionToken: sessionToken, + Checks: &session.Checks{ + Totp: &session.CheckTOTP{ + Code: code, + }, + }, + }) + require.NoError(t, err) + sessionToken = resp.GetSessionToken() + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, userExisting.GetUserId(), wantUserFactor, wantTOTPFactor) + }) + + userImport := Tester.CreateHumanUserWithTOTP(CTX, totpSecret) + createRespImport, err := Client.CreateSession(CTX, &session.CreateSessionRequest{}) + require.NoError(t, err) + sessionTokenImport := createRespImport.GetSessionToken() + verifyCurrentSession(t, createRespImport.GetSessionId(), sessionTokenImport, createRespImport.GetDetails().GetSequence(), time.Minute, nil, nil, 0, "") + + t.Run("check user", func(t *testing.T) { + resp, err := Client.SetSession(CTX, &session.SetSessionRequest{ + SessionId: createRespImport.GetSessionId(), + SessionToken: sessionTokenImport, + Checks: &session.Checks{ + User: &session.CheckUser{ + Search: &session.CheckUser_UserId{ + UserId: userImport.GetUserId(), + }, + }, + }, + }) + require.NoError(t, err) + sessionTokenImport = resp.GetSessionToken() + verifyCurrentSession(t, createRespImport.GetSessionId(), sessionTokenImport, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, userImport.GetUserId(), wantUserFactor) + }) + t.Run("check TOTP", func(t *testing.T) { + code, err := totp.GenerateCode(totpSecret, time.Now()) + require.NoError(t, err) + resp, err := Client.SetSession(CTX, &session.SetSessionRequest{ + SessionId: createRespImport.GetSessionId(), + SessionToken: sessionTokenImport, + Checks: &session.Checks{ + Totp: &session.CheckTOTP{ + Code: code, + }, + }, + }) + require.NoError(t, err) + sessionTokenImport = resp.GetSessionToken() + verifyCurrentSession(t, createRespImport.GetSessionId(), sessionTokenImport, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, userImport.GetUserId(), wantUserFactor, wantTOTPFactor) + }) +} + func TestServer_SetSession_flow(t *testing.T) { // create new, empty session createResp, err := Client.CreateSession(CTX, &session.CreateSessionRequest{}) require.NoError(t, err) sessionToken := createResp.GetSessionToken() - verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, createResp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) t.Run("check user", func(t *testing.T) { resp, err := Client.SetSession(CTX, &session.SetSessionRequest{ @@ -535,7 +674,7 @@ func TestServer_SetSession_flow(t *testing.T) { }) require.NoError(t, err) sessionToken = resp.GetSessionToken() - verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor) + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor) }) t.Run("check webauthn, user verified (passkey)", func(t *testing.T) { @@ -550,7 +689,7 @@ func TestServer_SetSession_flow(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) sessionToken = resp.GetSessionToken() assertionData, err := Tester.WebAuthN.CreateAssertionResponse(resp.GetChallenges().GetWebAuthN().GetPublicKeyCredentialRequestOptions(), true) @@ -567,7 +706,7 @@ func TestServer_SetSession_flow(t *testing.T) { }) require.NoError(t, err) sessionToken = resp.GetSessionToken() - verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantWebAuthNFactorUserVerified) + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantWebAuthNFactorUserVerified) }) userAuthCtx := Tester.WithAuthorizationToken(CTX, sessionToken) @@ -594,7 +733,7 @@ func TestServer_SetSession_flow(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) sessionToken = resp.GetSessionToken() assertionData, err := Tester.WebAuthN.CreateAssertionResponse(resp.GetChallenges().GetWebAuthN().GetPublicKeyCredentialRequestOptions(), false) @@ -611,7 +750,7 @@ func TestServer_SetSession_flow(t *testing.T) { }) require.NoError(t, err) sessionToken = resp.GetSessionToken() - verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantWebAuthNFactor) + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantWebAuthNFactor) }) } }) @@ -630,7 +769,7 @@ func TestServer_SetSession_flow(t *testing.T) { }) require.NoError(t, err) sessionToken = resp.GetSessionToken() - verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantWebAuthNFactor, wantTOTPFactor) + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantWebAuthNFactor, wantTOTPFactor) }) t.Run("check OTP SMS", func(t *testing.T) { @@ -642,7 +781,7 @@ func TestServer_SetSession_flow(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) sessionToken = resp.GetSessionToken() otp := resp.GetChallenges().GetOtpSms() @@ -659,7 +798,7 @@ func TestServer_SetSession_flow(t *testing.T) { }) require.NoError(t, err) sessionToken = resp.GetSessionToken() - verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantWebAuthNFactor, wantOTPSMSFactor) + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantWebAuthNFactor, wantOTPSMSFactor) }) t.Run("check OTP Email", func(t *testing.T) { @@ -673,7 +812,7 @@ func TestServer_SetSession_flow(t *testing.T) { }, }) require.NoError(t, err) - verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0) + verifyCurrentSession(t, createResp.GetSessionId(), resp.GetSessionToken(), resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId()) sessionToken = resp.GetSessionToken() otp := resp.GetChallenges().GetOtpEmail() @@ -690,7 +829,7 @@ func TestServer_SetSession_flow(t *testing.T) { }) require.NoError(t, err) sessionToken = resp.GetSessionToken() - verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, wantUserFactor, wantWebAuthNFactor, wantOTPEmailFactor) + verifyCurrentSession(t, createResp.GetSessionId(), sessionToken, resp.GetDetails().GetSequence(), time.Minute, nil, nil, 0, User.GetUserId(), wantUserFactor, wantWebAuthNFactor, wantOTPEmailFactor) }) } diff --git a/internal/api/grpc/user/v2/user.go b/internal/api/grpc/user/v2/user.go index 6ff420a44b6..dc91fe3eff5 100644 --- a/internal/api/grpc/user/v2/user.go +++ b/internal/api/grpc/user/v2/user.go @@ -95,6 +95,7 @@ func AddUserRequestToAddHuman(req *user.AddHumanUserRequest) (*command.AddHuman, Register: false, Metadata: metadata, Links: links, + TOTPSecret: req.GetTotpSecret(), }, nil } diff --git a/internal/api/grpc/user/v2/user_integration_test.go b/internal/api/grpc/user/v2/user_integration_test.go index 132ed0b30fc..19ce0aecc68 100644 --- a/internal/api/grpc/user/v2/user_integration_test.go +++ b/internal/api/grpc/user/v2/user_integration_test.go @@ -449,6 +449,52 @@ func TestServer_AddHumanUser(t *testing.T) { }, }, }, + { + name: "with totp", + args: args{ + CTX, + &user.AddHumanUserRequest{ + Organization: &object.Organization{ + Org: &object.Organization_OrgId{ + OrgId: Tester.Organisation.ID, + }, + }, + Profile: &user.SetHumanProfile{ + GivenName: "Donald", + FamilyName: "Duck", + NickName: gu.Ptr("Dukkie"), + DisplayName: gu.Ptr("Donald Duck"), + PreferredLanguage: gu.Ptr("en"), + Gender: user.Gender_GENDER_DIVERSE.Enum(), + }, + Email: &user.SetHumanEmail{ + Email: "livio@zitadel.com", + Verification: &user.SetHumanEmail_IsVerified{ + IsVerified: true, + }, + }, + Metadata: []*user.SetMetadataEntry{ + { + Key: "somekey", + Value: []byte("somevalue"), + }, + }, + PasswordType: &user.AddHumanUserRequest_Password{ + Password: &user.Password{ + Password: "DifficultPW666!", + ChangeRequired: false, + }, + }, + TotpSecret: gu.Ptr("secret"), + }, + }, + want: &user.AddHumanUserResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Tester.Organisation.ID, + }, + }, + }, { name: "password not complexity conform", args: args{ diff --git a/internal/command/session_test.go b/internal/command/session_test.go index 58f5f73706c..09d5a8a4e54 100644 --- a/internal/command/session_test.go +++ b/internal/command/session_test.go @@ -829,7 +829,9 @@ func TestCheckTOTP(t *testing.T) { ctx := authz.NewMockContext("instance1", "org1", "user1") cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) - key, secret, err := domain.NewTOTPKey("example.com", "user1", cryptoAlg) + key, err := domain.NewTOTPKey("example.com", "user1") + require.NoError(t, err) + secret, err := crypto.Encrypt([]byte(key.Secret()), cryptoAlg) require.NoError(t, err) sessAgg := &session.NewAggregate("session1", "instance1").Aggregate diff --git a/internal/command/user_human.go b/internal/command/user_human.go index 41ce13f9504..7f117751b73 100644 --- a/internal/command/user_human.go +++ b/internal/command/user_human.go @@ -68,6 +68,9 @@ type AddHuman struct { // Links are optional Links []*AddLink + // TOTPSecret is optional + TOTPSecret string + // Details are set after a successful execution of the command Details *domain.ObjectDetails diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index a24751f5707..c368167b3db 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -106,7 +106,11 @@ func (c *Commands) createHumanTOTP(ctx context.Context, userID, resourceOwner st if issuer == "" { issuer = authz.GetInstance(ctx).RequestedDomain() } - key, secret, err := domain.NewTOTPKey(issuer, accountName, c.multifactors.OTP.CryptoMFA) + key, err := domain.NewTOTPKey(issuer, accountName) + if err != nil { + return nil, err + } + encryptedSecret, err := crypto.Encrypt([]byte(key.Secret()), c.multifactors.OTP.CryptoMFA) if err != nil { return nil, err } @@ -115,7 +119,7 @@ func (c *Commands) createHumanTOTP(ctx context.Context, userID, resourceOwner st userAgg: userAgg, key: key, cmds: []eventstore.Command{ - user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + user.NewHumanOTPAddedEvent(ctx, userAgg, encryptedSecret), }, }, nil } diff --git a/internal/command/user_human_otp_test.go b/internal/command/user_human_otp_test.go index 0cfba917c79..81615a4f6e2 100644 --- a/internal/command/user_human_otp_test.go +++ b/internal/command/user_human_otp_test.go @@ -620,7 +620,9 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { ctx := authz.NewMockContext("", "org1", "user1") cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) - key, secret, err := domain.NewTOTPKey("example.com", "user1", cryptoAlg) + key, err := domain.NewTOTPKey("example.com", "user1") + require.NoError(t, err) + secret, err := crypto.Encrypt([]byte(key.Secret()), cryptoAlg) require.NoError(t, err) userAgg := &user.NewAggregate("user1", "org1").Aggregate userAgg2 := &user.NewAggregate("user2", "org1").Aggregate diff --git a/internal/command/user_v2_human.go b/internal/command/user_v2_human.go index 68955b4a075..c89629ec446 100644 --- a/internal/command/user_v2_human.go +++ b/internal/command/user_v2_human.go @@ -218,6 +218,17 @@ func (c *Commands) AddUserHuman(ctx context.Context, resourceOwner string, human cmds = append(cmds, cmd) } + if human.TOTPSecret != "" { + encryptedSecret, err := crypto.Encrypt([]byte(human.TOTPSecret), c.multifactors.OTP.CryptoMFA) + if err != nil { + return err + } + cmds = append(cmds, + user.NewHumanOTPAddedEvent(ctx, &existingHuman.Aggregate().Aggregate, encryptedSecret), + user.NewHumanOTPVerifiedEvent(ctx, &existingHuman.Aggregate().Aggregate, ""), + ) + } + if len(cmds) == 0 { human.Details = writeModelToObjectDetails(&existingHuman.WriteModel) return nil diff --git a/internal/command/user_v2_human_test.go b/internal/command/user_v2_human_test.go index 59f0935968d..e3dccde360a 100644 --- a/internal/command/user_v2_human_test.go +++ b/internal/command/user_v2_human_test.go @@ -8,6 +8,7 @@ import ( "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "golang.org/x/text/language" @@ -47,6 +48,11 @@ func TestCommandSide_AddUserHuman(t *testing.T) { userAgg := user.NewAggregate("user1", "org1") + cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) + totpSecret := "TOTPSecret" + totpSecretEnc, err := crypto.Encrypt([]byte(totpSecret), cryptoAlg) + require.NoError(t, err) + tests := []struct { name string fields fields @@ -1394,6 +1400,89 @@ func TestCommandSide_AddUserHuman(t *testing.T) { wantID: "user1", }, }, + { + name: "register human with TOTPSecret, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &userAgg.Aggregate, + true, + true, + true, + ), + ), + ), + expectPush( + user.NewHumanRegisteredEvent(context.Background(), + &userAgg.Aggregate, + "username", + "firstname", + "lastname", + "", + "firstname lastname", + language.English, + domain.GenderUnspecified, + "email@test.ch", + true, + "userAgentID", + ), + user.NewHumanInitialCodeAddedEvent(context.Background(), + &userAgg.Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("userinit"), + }, + time.Hour*1, + "authRequestID", + ), + user.NewHumanOTPAddedEvent(context.Background(), + &userAgg.Aggregate, + totpSecretEnc, + ), + user.NewHumanOTPVerifiedEvent(context.Background(), + &userAgg.Aggregate, + "", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"), + newCode: mockEncryptedCode("userinit", time.Hour), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &AddHuman{ + Username: "username", + FirstName: "firstname", + LastName: "lastname", + Email: Email{ + Address: "email@test.ch", + }, + PreferredLanguage: language.English, + Register: true, + UserAgentID: "userAgentID", + AuthRequestID: "authRequestID", + TOTPSecret: totpSecret, + }, + secretGenerator: GetMockSecretGenerator(t), + allowInitMail: true, + codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + res: res{ + want: &domain.ObjectDetails{ + Sequence: 0, + EventDate: time.Time{}, + ResourceOwner: "org1", + }, + wantID: "user1", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1403,6 +1492,12 @@ func TestCommandSide_AddUserHuman(t *testing.T) { idGenerator: tt.fields.idGenerator, newEncryptedCode: tt.fields.newCode, checkPermission: tt.fields.checkPermission, + multifactors: domain.MultifactorConfigs{ + OTP: domain.OTPConfig{ + Issuer: "zitadel.com", + CryptoMFA: cryptoAlg, + }, + }, } err := r.AddUserHuman(tt.args.ctx, tt.args.orgID, tt.args.human, tt.args.allowInitMail, tt.args.codeAlg) if tt.res.err == nil { diff --git a/internal/command/user_v2_totp_test.go b/internal/command/user_v2_totp_test.go index d05bdc9647e..f1a4968275b 100644 --- a/internal/command/user_v2_totp_test.go +++ b/internal/command/user_v2_totp_test.go @@ -262,8 +262,11 @@ func TestCommands_CheckUserTOTP(t *testing.T) { ctx := authz.NewMockContext("", "org1", "user1") cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) - key, secret, err := domain.NewTOTPKey("example.com", "user1", cryptoAlg) + key, err := domain.NewTOTPKey("example.com", "user1") require.NoError(t, err) + secret, err := crypto.Encrypt([]byte(key.Secret()), cryptoAlg) + require.NoError(t, err) + userAgg := &user.NewAggregate("user1", "org1").Aggregate code, err := totp.GenerateCode(key.Secret(), time.Now()) diff --git a/internal/domain/human_otp.go b/internal/domain/human_otp.go index 68b4b4ca0d8..fcadb32a726 100644 --- a/internal/domain/human_otp.go +++ b/internal/domain/human_otp.go @@ -15,16 +15,12 @@ type TOTP struct { URI string } -func NewTOTPKey(issuer, accountName string, cryptoAlg crypto.EncryptionAlgorithm) (*otp.Key, *crypto.CryptoValue, error) { +func NewTOTPKey(issuer, accountName string) (*otp.Key, error) { key, err := totp.Generate(totp.GenerateOpts{Issuer: issuer, AccountName: accountName}) if err != nil { - return nil, nil, zerrors.ThrowInternal(err, "TOTP-ieY3o", "Errors.Internal") + return nil, zerrors.ThrowInternal(err, "TOTP-ieY3o", "Errors.Internal") } - encryptedSecret, err := crypto.Encrypt([]byte(key.Secret()), cryptoAlg) - if err != nil { - return nil, nil, err - } - return key, encryptedSecret, nil + return key, nil } func VerifyTOTP(code string, secret *crypto.CryptoValue, cryptoAlg crypto.EncryptionAlgorithm) error { diff --git a/internal/integration/client.go b/internal/integration/client.go index 3ebd4fea515..4b843f68ebf 100644 --- a/internal/integration/client.go +++ b/internal/integration/client.go @@ -150,6 +150,37 @@ func (s *Tester) CreateHumanUser(ctx context.Context) *user.AddHumanUserResponse return resp } +func (s *Tester) CreateHumanUserWithTOTP(ctx context.Context, secret string) *user.AddHumanUserResponse { + resp, err := s.Client.UserV2.AddHumanUser(ctx, &user.AddHumanUserRequest{ + Organization: &object.Organization{ + Org: &object.Organization_OrgId{ + OrgId: s.Organisation.ID, + }, + }, + Profile: &user.SetHumanProfile{ + GivenName: "Mickey", + FamilyName: "Mouse", + PreferredLanguage: gu.Ptr("nl"), + Gender: gu.Ptr(user.Gender_GENDER_MALE), + }, + Email: &user.SetHumanEmail{ + Email: fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + Verification: &user.SetHumanEmail_ReturnCode{ + ReturnCode: &user.ReturnEmailVerificationCode{}, + }, + }, + Phone: &user.SetHumanPhone{ + Phone: "+41791234567", + Verification: &user.SetHumanPhone_ReturnCode{ + ReturnCode: &user.ReturnPhoneVerificationCode{}, + }, + }, + TotpSecret: gu.Ptr(secret), + }) + logging.OnError(err).Fatal("create human user") + return resp +} + func (s *Tester) CreateOrganization(ctx context.Context, name, adminEmail string) *org.AddOrganizationResponse { resp, err := s.Client.OrgV2.AddOrganization(ctx, &org.AddOrganizationRequest{ Name: name, diff --git a/proto/zitadel/user/v2beta/user_service.proto b/proto/zitadel/user/v2beta/user_service.proto index cbf20d6790d..f3d8de747c8 100644 --- a/proto/zitadel/user/v2beta/user_service.proto +++ b/proto/zitadel/user/v2beta/user_service.proto @@ -931,6 +931,17 @@ message AddHumanUserRequest{ HashedPassword hashed_password = 8; } repeated IDPLink idp_links = 9; + + // An Implementation of RFC 6238 is used, with HMAC-SHA-1 and time-step of 30 seconds. + // Currently no other options are supported, and if anything different is used the validation will fail. + optional string totp_secret = 12 [ + (validate.rules).string = {min_len: 1, max_len: 200}, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + min_length: 1; + max_length: 200; + example: "\"TJOPWSDYILLHXFV4MLKNNJOWFG7VSDCK\""; + } + ]; } message AddHumanUserResponse {