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
zitadel · May 14, 2024 · 0e9ebed · 0e9ebed
1 parent 15d5338
commit 0e9ebed
Show file tree

Hide file tree

Showing 13 changed files with 395 additions and 51 deletions.
diff --git a/internal/api/grpc/session/v2/session_integration_test.go b/internal/api/grpc/session/v2/session_integration_test.go
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {