From 3f140e06a97036ae836508d69fdd6f9f50d4d992 Mon Sep 17 00:00:00 2001 From: Pragati Date: Tue, 30 May 2023 21:18:23 -0700 Subject: [PATCH 01/20] Adding TotpInfo to userRecord --- auth/user_mgt.go | 76 +++++++++++++++++--------- auth/user_mgt_test.go | 89 ++++++++++++++++++++++--------- integration/auth/user_mgt_test.go | 14 +++-- testdata/get_user.json | 10 +++- testdata/list_users.json | 38 ++++++++----- 5 files changed, 158 insertions(+), 69 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index fe6ac986..3b0db181 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -42,6 +42,7 @@ const ( createUserMethod = "createUser" updateUserMethod = "updateUser" phoneMultiFactorID = "phone" + totpMultiFactorID = "totp" ) // 'REDACTED', encoded as a base64 string. @@ -62,20 +63,29 @@ type UserInfo struct { // multiFactorInfoResponse describes the `mfaInfo` of the user record API response type multiFactorInfoResponse struct { - MFAEnrollmentID string `json:"mfaEnrollmentId,omitempty"` - DisplayName string `json:"displayName,omitempty"` - PhoneInfo string `json:"phoneInfo,omitempty"` - EnrolledAt string `json:"enrolledAt,omitempty"` + MFAEnrollmentID string `json:"mfaEnrollmentId,omitempty"` + DisplayName string `json:"displayName,omitempty"` + PhoneInfo string `json:"phoneInfo,omitempty"` + TOTPInfo *TOTPInfo `json:"totpInfo,omitempty"` + EnrolledAt string `json:"enrolledAt,omitempty"` +} + +// TOTPInfo describes a user enrolled second totp factor. +type TOTPInfo struct{} + +// PhoneMultiFactorInfo describes a user enrolled second phone factor. +type PhoneMultiFactorInfo struct { + PhoneNumber string } // MultiFactorInfo describes a user enrolled second phone factor. -// TODO : convert PhoneNumber to PhoneMultiFactorInfo struct type MultiFactorInfo struct { - UID string - DisplayName string - EnrollmentTimestamp int64 - FactorID string - PhoneNumber string + UID string + DisplayName string + EnrollmentTimestamp int64 + FactorID string + PhoneMultiFactorInfo *PhoneMultiFactorInfo + TOTPMultiFactorInfo *TOTPInfo } // MultiFactorSettings describes the multi-factor related user settings. @@ -170,12 +180,18 @@ func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorI if mfaInfo.EnrollmentTimestamp != 0 { authFactorInfo.EnrolledAt = time.Unix(mfaInfo.EnrollmentTimestamp, 0).Format("2006-01-02T15:04:05Z07:00Z") } - if mfaInfo.FactorID == phoneMultiFactorID { - authFactorInfo.PhoneInfo = mfaInfo.PhoneNumber - authFactorInfo.DisplayName = mfaInfo.DisplayName - authFactorInfo.MFAEnrollmentID = mfaInfo.UID + authFactorInfo.DisplayName = mfaInfo.DisplayName + authFactorInfo.MFAEnrollmentID = mfaInfo.UID + + switch mfaInfo.FactorID { + case phoneMultiFactorID: + authFactorInfo.PhoneInfo = mfaInfo.PhoneMultiFactorInfo.PhoneNumber + return authFactorInfo, nil + case totpMultiFactorID: + authFactorInfo.TOTPInfo = (*TOTPInfo)(mfaInfo.TOTPMultiFactorInfo) return authFactorInfo, nil } + out, _ := json.Marshal(mfaInfo) return multiFactorInfoResponse{}, fmt.Errorf("Unsupported second factor %s provided", string(out)) } @@ -675,8 +691,8 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("the second factor \"displayName\" for \"%s\" must be a valid non-empty string", multiFactorInfo.DisplayName) } if multiFactorInfo.FactorID == phoneMultiFactorID { - if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo) @@ -1075,17 +1091,27 @@ func (r *userQueryResponse) makeExportedUserRecord() (*ExportedUserRecord, error enrollmentTimestamp = t.Unix() * 1000 } - if factor.PhoneInfo == "" { + if factor.PhoneInfo != "" { + enrolledFactors = append(enrolledFactors, &MultiFactorInfo{ + UID: factor.MFAEnrollmentID, + DisplayName: factor.DisplayName, + EnrollmentTimestamp: enrollmentTimestamp, + FactorID: phoneMultiFactorID, + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: factor.PhoneInfo, + }, + }) + } else if factor.TOTPInfo != nil { + enrolledFactors = append(enrolledFactors, &MultiFactorInfo{ + UID: factor.MFAEnrollmentID, + DisplayName: factor.DisplayName, + EnrollmentTimestamp: enrollmentTimestamp, + FactorID: totpMultiFactorID, + TOTPMultiFactorInfo: &TOTPInfo{}, + }) + } else { return nil, fmt.Errorf("unsupported multi-factor auth response: %#v", factor) } - - enrolledFactors = append(enrolledFactors, &MultiFactorInfo{ - UID: factor.MFAEnrollmentID, - DisplayName: factor.DisplayName, - EnrollmentTimestamp: enrollmentTimestamp, - FactorID: phoneMultiFactorID, - PhoneNumber: factor.PhoneInfo, - }) } return &ExportedUserRecord{ diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index 943810cd..94d2bfd6 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -69,11 +69,20 @@ var testUser = &UserRecord{ MultiFactor: &MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "0aaded3f-5e73-461d-aef9-37b48e3769be", + UID: "enrolledFactor1", FactorID: "phone", EnrollmentTimestamp: 1614776780000, - PhoneNumber: "+1234567890", - DisplayName: "My MFA Phone", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+1234567890", + }, + DisplayName: "My MFA Phone", + }, + { + UID: "enrolledFactor2", + FactorID: "totp", + EnrollmentTimestamp: 1614776780000, + TOTPMultiFactorInfo: &TOTPInfo{}, + DisplayName: "My MFA TOTP", }, }, }, @@ -646,8 +655,10 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "EnrollmentID", - PhoneNumber: "+11234567890", + UID: "EnrollmentID", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -658,7 +669,9 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "invalid", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "invalid", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -669,7 +682,9 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", EnrollmentTimestamp: time.Now().UTC().Unix(), @@ -681,7 +696,9 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "", }, @@ -692,8 +709,10 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", - FactorID: "phone", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, + FactorID: "phone", }, }, }), @@ -773,7 +792,9 @@ var createUserCases = []struct { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -790,12 +811,16 @@ var createUserCases = []struct { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "number1", FactorID: "phone", }, { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "number2", FactorID: "phone", }, @@ -875,9 +900,11 @@ func TestInvalidUpdateUser(t *testing.T) { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledSecondFactor1", - PhoneNumber: "+11234567890", - FactorID: "phone", + UID: "enrolledSecondFactor1", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, + FactorID: "phone", }, }, }), @@ -886,8 +913,10 @@ func TestInvalidUpdateUser(t *testing.T) { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledSecondFactor1", - PhoneNumber: "invalid", + UID: "enrolledSecondFactor1", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "invalid", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -898,7 +927,9 @@ func TestInvalidUpdateUser(t *testing.T) { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, FactorID: "phone", DisplayName: "Spouse's phone number", }, @@ -1049,14 +1080,18 @@ var updateUserCases = []struct { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledSecondFactor1", - PhoneNumber: "+11234567890", + UID: "enrolledSecondFactor1", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", EnrollmentTimestamp: time.Now().Unix(), }, { - UID: "enrolledSecondFactor2", - PhoneNumber: "+11234567890", + UID: "enrolledSecondFactor2", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -1886,10 +1921,16 @@ func TestMakeExportedUser(t *testing.T) { MFAInfo: []*multiFactorInfoResponse{ { PhoneInfo: "+1234567890", - MFAEnrollmentID: "0aaded3f-5e73-461d-aef9-37b48e3769be", + MFAEnrollmentID: "enrolledFactor1", DisplayName: "My MFA Phone", EnrolledAt: "2021-03-03T13:06:20.542896Z", }, + { + TOTPInfo: &TOTPInfo{}, + MFAEnrollmentID: "enrolledFactor2", + DisplayName: "My MFA TOTP", + EnrolledAt: "2021-03-03T13:06:20.542896Z", + }, }, } diff --git a/integration/auth/user_mgt_test.go b/integration/auth/user_mgt_test.go index c1301f6d..6f43224c 100644 --- a/integration/auth/user_mgt_test.go +++ b/integration/auth/user_mgt_test.go @@ -434,7 +434,9 @@ func TestCreateUserMFA(t *testing.T) { tc.MFASettings(auth.MultiFactorSettings{ EnrolledFactors: []*auth.MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &auth.PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -447,10 +449,12 @@ func TestCreateUserMFA(t *testing.T) { defer deleteUser(user.UID) var factor []*auth.MultiFactorInfo = []*auth.MultiFactorInfo{ { - UID: user.MultiFactor.EnrolledFactors[0].UID, - DisplayName: "Spouse's phone number", - FactorID: "phone", - PhoneNumber: "+11234567890", + UID: user.MultiFactor.EnrolledFactors[0].UID, + DisplayName: "Spouse's phone number", + FactorID: "phone", + PhoneMultiFactorInfo: &auth.PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, EnrollmentTimestamp: user.MultiFactor.EnrolledFactors[0].EnrollmentTimestamp, }, } diff --git a/testdata/get_user.json b/testdata/get_user.json index a2cac48b..95802ab3 100644 --- a/testdata/get_user.json +++ b/testdata/get_user.json @@ -35,11 +35,17 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "0aaded3f-5e73-461d-aef9-37b48e3769be", + "mfaEnrollmentId": "enrolledFactor1", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" + }, + { + "totpInfo": {}, + "mfaEnrollmentId": "enrolledFactor2", + "displayName": "My MFA TOTP", + "enrolledAt": "2021-03-03T13:06:20.542896Z" } ] } ] -} +} \ No newline at end of file diff --git a/testdata/list_users.json b/testdata/list_users.json index bf94ff49..8fb96058 100644 --- a/testdata/list_users.json +++ b/testdata/list_users.json @@ -33,12 +33,18 @@ "customAttributes": "{\"admin\": true, \"package\": \"gold\"}", "tenantId": "testTenant", "mfaInfo": [ - { - "phoneInfo": "+1234567890", - "mfaEnrollmentId": "0aaded3f-5e73-461d-aef9-37b48e3769be", - "displayName": "My MFA Phone", - "enrolledAt": "2021-03-03T13:06:20.542896Z" - } + { + "phoneInfo": "+1234567890", + "mfaEnrollmentId": "enrolledFactor1", + "displayName": "My MFA Phone", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + }, + { + "totpInfo": {}, + "mfaEnrollmentId": "enrolledFactor2", + "displayName": "My MFA TOTP", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + } ] }, { @@ -73,12 +79,18 @@ "customAttributes": "{\"admin\": true, \"package\": \"gold\"}", "tenantId": "testTenant", "mfaInfo": [ - { - "phoneInfo": "+1234567890", - "mfaEnrollmentId": "0aaded3f-5e73-461d-aef9-37b48e3769be", - "displayName": "My MFA Phone", - "enrolledAt": "2021-03-03T13:06:20.542896Z" - } + { + "phoneInfo": "+1234567890", + "mfaEnrollmentId": "enrolledFactor1", + "displayName": "My MFA Phone", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + }, + { + "totpInfo": {}, + "mfaEnrollmentId": "enrolledFactor2", + "displayName": "My MFA TOTP", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + } ] }, { @@ -115,4 +127,4 @@ } ], "nextPageToken": "" -} +} \ No newline at end of file From e178eb3eed429f8adef72eec0c383a0cba70415b Mon Sep 17 00:00:00 2001 From: Pragati Date: Wed, 31 May 2023 18:44:51 -0700 Subject: [PATCH 02/20] Add MFA fixes --- auth/user_mgt.go | 17 +++++++++++------ auth/user_mgt_test.go | 29 +++++++++++++---------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 3b0db181..de861dc0 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -78,6 +78,10 @@ type PhoneMultiFactorInfo struct { PhoneNumber string } +type multiFactorEnrollments struct { + Enrollments []*multiFactorInfoResponse `json:"enrollments"` +} + // MultiFactorInfo describes a user enrolled second phone factor. type MultiFactorInfo struct { UID string @@ -176,10 +180,13 @@ func (u *UserToCreate) set(key string, value interface{}) *UserToCreate { // Converts a client format second factor object to server format. func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorInfoResponse, error) { - var authFactorInfo multiFactorInfoResponse + authFactorInfo := multiFactorInfoResponse{DisplayName: mfaInfo.DisplayName} if mfaInfo.EnrollmentTimestamp != 0 { authFactorInfo.EnrolledAt = time.Unix(mfaInfo.EnrollmentTimestamp, 0).Format("2006-01-02T15:04:05Z07:00Z") } + if mfaInfo.UID != "" { + authFactorInfo.MFAEnrollmentID = mfaInfo.UID + } authFactorInfo.DisplayName = mfaInfo.DisplayName authFactorInfo.MFAEnrollmentID = mfaInfo.UID @@ -193,7 +200,7 @@ func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorI } out, _ := json.Marshal(mfaInfo) - return multiFactorInfoResponse{}, fmt.Errorf("Unsupported second factor %s provided", string(out)) + return multiFactorInfoResponse{}, fmt.Errorf("unsupported second factor %s provided", string(out)) } func (u *UserToCreate) validatedRequest() (map[string]interface{}, error) { @@ -349,7 +356,8 @@ func (u *UserToUpdate) validatedRequest() (map[string]interface{}, error) { if err != nil { return nil, err } - req["mfaInfo"] = mfaInfo + // https://cloud.google.com/identity-platform/docs/reference/rest/v1/accounts/update + req["mfa"] = multiFactorEnrollments{mfaInfo} } else { req[k] = v } @@ -681,9 +689,6 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("\"uid\" is not supported when adding second factors via \"createUser()\"") } case updateUserMethod: - if multiFactorInfo.UID == "" { - return nil, fmt.Errorf("the second factor \"uid\" must be a valid non-empty string when adding second factors via \"updateUser()\"") - } default: return nil, fmt.Errorf("unsupported methodType: %s", methodType) } diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index 94d2bfd6..1279d5f5 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -923,19 +923,6 @@ func TestInvalidUpdateUser(t *testing.T) { }, }), `the second factor "phoneNumber" for "invalid" must be a non-empty E.164 standard compliant identifier string`, - }, { - (&UserToUpdate{}).MFASettings(MultiFactorSettings{ - EnrolledFactors: []*MultiFactorInfo{ - { - PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ - PhoneNumber: "+11234567890", - }, - FactorID: "phone", - DisplayName: "Spouse's phone number", - }, - }, - }), - `the second factor "uid" must be a valid non-empty string when adding second factors via "updateUser()"`, }, { (&UserToUpdate{}).ProviderToLink(&UserProvider{UID: "google_uid"}), "user provider must specify a provider ID", @@ -1094,10 +1081,16 @@ var updateUserCases = []struct { }, DisplayName: "Spouse's phone number", FactorID: "phone", + }, { + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, + DisplayName: "Spouse's phone number", + FactorID: "phone", }, }, }), - map[string]interface{}{"mfaInfo": []*multiFactorInfoResponse{ + map[string]interface{}{"mfa": multiFactorEnrollments{Enrollments: []*multiFactorInfoResponse{ { MFAEnrollmentID: "enrolledSecondFactor1", PhoneInfo: "+11234567890", @@ -1109,12 +1102,16 @@ var updateUserCases = []struct { DisplayName: "Spouse's phone number", PhoneInfo: "+11234567890", }, - }, + { + DisplayName: "Spouse's phone number", + PhoneInfo: "+11234567890", + }, + }}, }, }, { (&UserToUpdate{}).MFASettings(MultiFactorSettings{}), - map[string]interface{}{"mfaInfo": nil}, + map[string]interface{}{"mfa": multiFactorEnrollments{Enrollments: nil}}, }, { (&UserToUpdate{}).ProviderToLink(&UserProvider{ From 8c41cc7879ad758e2f8661eda55da0dc8dd8c74d Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Mon, 5 Jun 2023 23:46:35 +0000 Subject: [PATCH 03/20] Addressing feedback --- auth/user_mgt.go | 2 +- auth/user_mgt_test.go | 8 ++++---- testdata/get_user.json | 6 +++--- testdata/list_users.json | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index de861dc0..459c1435 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -356,7 +356,7 @@ func (u *UserToUpdate) validatedRequest() (map[string]interface{}, error) { if err != nil { return nil, err } - // https://cloud.google.com/identity-platform/docs/reference/rest/v1/accounts/update + // Request body ref: https://cloud.google.com/identity-platform/docs/reference/rest/v1/accounts/update req["mfa"] = multiFactorEnrollments{mfaInfo} } else { req[k] = v diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index 1279d5f5..d6fbab71 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -69,7 +69,7 @@ var testUser = &UserRecord{ MultiFactor: &MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledFactor1", + UID: "enrolledPhoneFactor", FactorID: "phone", EnrollmentTimestamp: 1614776780000, PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ @@ -78,7 +78,7 @@ var testUser = &UserRecord{ DisplayName: "My MFA Phone", }, { - UID: "enrolledFactor2", + UID: "enrolledTOTPFactor", FactorID: "totp", EnrollmentTimestamp: 1614776780000, TOTPMultiFactorInfo: &TOTPInfo{}, @@ -1918,13 +1918,13 @@ func TestMakeExportedUser(t *testing.T) { MFAInfo: []*multiFactorInfoResponse{ { PhoneInfo: "+1234567890", - MFAEnrollmentID: "enrolledFactor1", + MFAEnrollmentID: "enrolledPhoneFactor", DisplayName: "My MFA Phone", EnrolledAt: "2021-03-03T13:06:20.542896Z", }, { TOTPInfo: &TOTPInfo{}, - MFAEnrollmentID: "enrolledFactor2", + MFAEnrollmentID: "enrolledTOTPFactor", DisplayName: "My MFA TOTP", EnrolledAt: "2021-03-03T13:06:20.542896Z", }, diff --git a/testdata/get_user.json b/testdata/get_user.json index 95802ab3..0bf86f95 100644 --- a/testdata/get_user.json +++ b/testdata/get_user.json @@ -35,17 +35,17 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "enrolledFactor1", + "mfaEnrollmentId": "enrolledPhoneFactor", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" }, { "totpInfo": {}, - "mfaEnrollmentId": "enrolledFactor2", + "mfaEnrollmentId": "enrolledTOTPFactor", "displayName": "My MFA TOTP", "enrolledAt": "2021-03-03T13:06:20.542896Z" } ] } ] -} \ No newline at end of file +} diff --git a/testdata/list_users.json b/testdata/list_users.json index 8fb96058..2b630686 100644 --- a/testdata/list_users.json +++ b/testdata/list_users.json @@ -35,13 +35,13 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "enrolledFactor1", + "mfaEnrollmentId": "enrolledPhoneFactor", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" }, { "totpInfo": {}, - "mfaEnrollmentId": "enrolledFactor2", + "mfaEnrollmentId": "enrolledTOTPFactor", "displayName": "My MFA TOTP", "enrolledAt": "2021-03-03T13:06:20.542896Z" } @@ -81,13 +81,13 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "enrolledFactor1", + "mfaEnrollmentId": "enrolledPhoneFactor", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" }, { "totpInfo": {}, - "mfaEnrollmentId": "enrolledFactor2", + "mfaEnrollmentId": "enrolledTOTPFactor", "displayName": "My MFA TOTP", "enrolledAt": "2021-03-03T13:06:20.542896Z" } @@ -127,4 +127,4 @@ } ], "nextPageToken": "" -} \ No newline at end of file +} From 4c5e08fc1d2368222a56882cca26391aa8bf1444 Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Tue, 6 Jun 2023 00:11:06 +0000 Subject: [PATCH 04/20] Adding struct "`TOTPMultiFactorInfo`" as per review discussion. --- auth/user_mgt.go | 9 ++++++--- auth/user_mgt_test.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 459c1435..eee15523 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -70,7 +70,7 @@ type multiFactorInfoResponse struct { EnrolledAt string `json:"enrolledAt,omitempty"` } -// TOTPInfo describes a user enrolled second totp factor. +// TOTPInfo describes a server side user enrolled second totp factor. type TOTPInfo struct{} // PhoneMultiFactorInfo describes a user enrolled second phone factor. @@ -78,6 +78,9 @@ type PhoneMultiFactorInfo struct { PhoneNumber string } +// TOTPMultiFactorInfo describes a user enrolled second totp factor. +type TOTPMultiFactorInfo struct{} + type multiFactorEnrollments struct { Enrollments []*multiFactorInfoResponse `json:"enrollments"` } @@ -89,7 +92,7 @@ type MultiFactorInfo struct { EnrollmentTimestamp int64 FactorID string PhoneMultiFactorInfo *PhoneMultiFactorInfo - TOTPMultiFactorInfo *TOTPInfo + TOTPMultiFactorInfo *TOTPMultiFactorInfo } // MultiFactorSettings describes the multi-factor related user settings. @@ -1112,7 +1115,7 @@ func (r *userQueryResponse) makeExportedUserRecord() (*ExportedUserRecord, error DisplayName: factor.DisplayName, EnrollmentTimestamp: enrollmentTimestamp, FactorID: totpMultiFactorID, - TOTPMultiFactorInfo: &TOTPInfo{}, + TOTPMultiFactorInfo: &TOTPMultiFactorInfo{}, }) } else { return nil, fmt.Errorf("unsupported multi-factor auth response: %#v", factor) diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index d6fbab71..9ad64604 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -81,7 +81,7 @@ var testUser = &UserRecord{ UID: "enrolledTOTPFactor", FactorID: "totp", EnrollmentTimestamp: 1614776780000, - TOTPMultiFactorInfo: &TOTPInfo{}, + TOTPMultiFactorInfo: &TOTPMultiFactorInfo{}, DisplayName: "My MFA TOTP", }, }, From d29e057206019f8788bec0e88b0697434025e6d5 Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Thu, 22 Jun 2023 01:13:07 +0000 Subject: [PATCH 05/20] Addressing PR feedback --- auth/user_mgt.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index eee15523..6cc11bcb 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -190,20 +190,18 @@ func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorI if mfaInfo.UID != "" { authFactorInfo.MFAEnrollmentID = mfaInfo.UID } - authFactorInfo.DisplayName = mfaInfo.DisplayName authFactorInfo.MFAEnrollmentID = mfaInfo.UID switch mfaInfo.FactorID { case phoneMultiFactorID: authFactorInfo.PhoneInfo = mfaInfo.PhoneMultiFactorInfo.PhoneNumber - return authFactorInfo, nil case totpMultiFactorID: authFactorInfo.TOTPInfo = (*TOTPInfo)(mfaInfo.TOTPMultiFactorInfo) - return authFactorInfo, nil + default: + out, _ := json.Marshal(mfaInfo) + return multiFactorInfoResponse{}, fmt.Errorf("unsupported second factor %s provided", string(out)) } - - out, _ := json.Marshal(mfaInfo) - return multiFactorInfoResponse{}, fmt.Errorf("unsupported second factor %s provided", string(out)) + return authFactorInfo, nil } func (u *UserToCreate) validatedRequest() (map[string]interface{}, error) { @@ -692,6 +690,9 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("\"uid\" is not supported when adding second factors via \"createUser()\"") } case updateUserMethod: + if multiFactorInfo.UID == "" { + return nil, fmt.Errorf("the second factor \"uid\" must be a valid non-empty string when adding second factors via \"updateUser()\"") + } default: return nil, fmt.Errorf("unsupported methodType: %s", methodType) } @@ -699,8 +700,12 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("the second factor \"displayName\" for \"%s\" must be a valid non-empty string", multiFactorInfo.DisplayName) } if multiFactorInfo.FactorID == phoneMultiFactorID { - if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) + if multiFactorInfo.PhoneMultiFactorInfo != nil { + if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) + } + } else { + return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo) From 6921afb4b2fcc86e93b10c0901511867db2a9279 Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Thu, 22 Jun 2023 01:27:16 +0000 Subject: [PATCH 06/20] Addressing PR feedback --- auth/user_mgt.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 6cc11bcb..13e62a10 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -690,9 +690,6 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("\"uid\" is not supported when adding second factors via \"createUser()\"") } case updateUserMethod: - if multiFactorInfo.UID == "" { - return nil, fmt.Errorf("the second factor \"uid\" must be a valid non-empty string when adding second factors via \"updateUser()\"") - } default: return nil, fmt.Errorf("unsupported methodType: %s", methodType) } From ce0bac0beb1caa1a2b9dfa93012ae0319351bdf8 Mon Sep 17 00:00:00 2001 From: Pragati Date: Fri, 21 Jul 2023 12:48:20 -0700 Subject: [PATCH 07/20] adding `deprecated` tag to `PhoneNumber` field --- auth/user_mgt.go | 1 + 1 file changed, 1 insertion(+) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 6cc11bcb..15a61765 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -91,6 +91,7 @@ type MultiFactorInfo struct { DisplayName string EnrollmentTimestamp int64 FactorID string + PhoneNumber string `deprecated:"Use PhoneMultiFactorInfo instead"` PhoneMultiFactorInfo *PhoneMultiFactorInfo TOTPMultiFactorInfo *TOTPMultiFactorInfo } From 939d6c008977f42773d1b08fdf7a6db62d4c1705 Mon Sep 17 00:00:00 2001 From: Pragati Date: Mon, 24 Jul 2023 13:10:32 -0700 Subject: [PATCH 08/20] Keep `PhoneNumber` field in `MultiFactorInfo` for compatibility --- auth/user_mgt.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index bdfd4289..4bc7ca2f 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -91,7 +91,7 @@ type MultiFactorInfo struct { DisplayName string EnrollmentTimestamp int64 FactorID string - PhoneNumber string `deprecated:"Use PhoneMultiFactorInfo instead"` + PhoneNumber string `Deprecated:"Use PhoneMultiFactorInfo instead"` PhoneMultiFactorInfo *PhoneMultiFactorInfo TOTPMultiFactorInfo *TOTPMultiFactorInfo } @@ -702,6 +702,13 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } + } else if multiFactorInfo.PhoneNumber != "" { + if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + } else { + multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber = multiFactorInfo.PhoneNumber + fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + } } else { return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } From f4487824f495dbd6894c81f26ddaa0467043d1cb Mon Sep 17 00:00:00 2001 From: Pragati Date: Mon, 24 Jul 2023 13:35:47 -0700 Subject: [PATCH 09/20] Validation logic fix --- auth/user_mgt.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 4bc7ca2f..1b01ddb6 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -699,18 +699,26 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st } if multiFactorInfo.FactorID == phoneMultiFactorID { if multiFactorInfo.PhoneMultiFactorInfo != nil { + // If PhoneMultiFactorInfo is provided, validate its PhoneNumber field if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } - } else if multiFactorInfo.PhoneNumber != "" { - if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + } else { + // PhoneMultiFactorInfo is nil, check the deprecated PhoneNumber field + if multiFactorInfo.PhoneNumber != "" { + if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + } else { + // The PhoneNumber field is deprecated, set it in PhoneMultiFactorInfo and inform about the deprecation. + multiFactorInfo.PhoneMultiFactorInfo = &PhoneMultiFactorInfo{ + PhoneNumber: multiFactorInfo.PhoneNumber, + } + fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + } } else { - multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber = multiFactorInfo.PhoneNumber - fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + // Both PhoneMultiFactorInfo and deprecated PhoneNumber are missing. + return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } - } else { - return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo) From 336ca6dcb7b16597ca8214bebb132827dc828191 Mon Sep 17 00:00:00 2001 From: Pragati Date: Mon, 24 Jul 2023 13:38:57 -0700 Subject: [PATCH 10/20] lint fix --- auth/user_mgt.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 1b01ddb6..96efd545 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -703,22 +703,20 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } - } else { + // No need for the else here since we are returning from the function + } else if multiFactorInfo.PhoneNumber != "" { // PhoneMultiFactorInfo is nil, check the deprecated PhoneNumber field - if multiFactorInfo.PhoneNumber != "" { - if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) - } else { - // The PhoneNumber field is deprecated, set it in PhoneMultiFactorInfo and inform about the deprecation. - multiFactorInfo.PhoneMultiFactorInfo = &PhoneMultiFactorInfo{ - PhoneNumber: multiFactorInfo.PhoneNumber, - } - fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") - } - } else { - // Both PhoneMultiFactorInfo and deprecated PhoneNumber are missing. - return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") + if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) } + // The PhoneNumber field is deprecated, set it in PhoneMultiFactorInfo and inform about the deprecation. + multiFactorInfo.PhoneMultiFactorInfo = &PhoneMultiFactorInfo{ + PhoneNumber: multiFactorInfo.PhoneNumber, + } + fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + } else { + // Both PhoneMultiFactorInfo and deprecated PhoneNumber are missing. + return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo) From 587f0dfa07cf3dd92073e5aef39ead1a0119e1b3 Mon Sep 17 00:00:00 2001 From: Pragati Date: Tue, 30 May 2023 21:18:23 -0700 Subject: [PATCH 11/20] Adding TotpInfo to userRecord --- auth/user_mgt.go | 76 +++++++++++++++++--------- auth/user_mgt_test.go | 89 ++++++++++++++++++++++--------- integration/auth/user_mgt_test.go | 14 +++-- testdata/get_user.json | 10 +++- testdata/list_users.json | 38 ++++++++----- 5 files changed, 158 insertions(+), 69 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index fe6ac986..3b0db181 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -42,6 +42,7 @@ const ( createUserMethod = "createUser" updateUserMethod = "updateUser" phoneMultiFactorID = "phone" + totpMultiFactorID = "totp" ) // 'REDACTED', encoded as a base64 string. @@ -62,20 +63,29 @@ type UserInfo struct { // multiFactorInfoResponse describes the `mfaInfo` of the user record API response type multiFactorInfoResponse struct { - MFAEnrollmentID string `json:"mfaEnrollmentId,omitempty"` - DisplayName string `json:"displayName,omitempty"` - PhoneInfo string `json:"phoneInfo,omitempty"` - EnrolledAt string `json:"enrolledAt,omitempty"` + MFAEnrollmentID string `json:"mfaEnrollmentId,omitempty"` + DisplayName string `json:"displayName,omitempty"` + PhoneInfo string `json:"phoneInfo,omitempty"` + TOTPInfo *TOTPInfo `json:"totpInfo,omitempty"` + EnrolledAt string `json:"enrolledAt,omitempty"` +} + +// TOTPInfo describes a user enrolled second totp factor. +type TOTPInfo struct{} + +// PhoneMultiFactorInfo describes a user enrolled second phone factor. +type PhoneMultiFactorInfo struct { + PhoneNumber string } // MultiFactorInfo describes a user enrolled second phone factor. -// TODO : convert PhoneNumber to PhoneMultiFactorInfo struct type MultiFactorInfo struct { - UID string - DisplayName string - EnrollmentTimestamp int64 - FactorID string - PhoneNumber string + UID string + DisplayName string + EnrollmentTimestamp int64 + FactorID string + PhoneMultiFactorInfo *PhoneMultiFactorInfo + TOTPMultiFactorInfo *TOTPInfo } // MultiFactorSettings describes the multi-factor related user settings. @@ -170,12 +180,18 @@ func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorI if mfaInfo.EnrollmentTimestamp != 0 { authFactorInfo.EnrolledAt = time.Unix(mfaInfo.EnrollmentTimestamp, 0).Format("2006-01-02T15:04:05Z07:00Z") } - if mfaInfo.FactorID == phoneMultiFactorID { - authFactorInfo.PhoneInfo = mfaInfo.PhoneNumber - authFactorInfo.DisplayName = mfaInfo.DisplayName - authFactorInfo.MFAEnrollmentID = mfaInfo.UID + authFactorInfo.DisplayName = mfaInfo.DisplayName + authFactorInfo.MFAEnrollmentID = mfaInfo.UID + + switch mfaInfo.FactorID { + case phoneMultiFactorID: + authFactorInfo.PhoneInfo = mfaInfo.PhoneMultiFactorInfo.PhoneNumber + return authFactorInfo, nil + case totpMultiFactorID: + authFactorInfo.TOTPInfo = (*TOTPInfo)(mfaInfo.TOTPMultiFactorInfo) return authFactorInfo, nil } + out, _ := json.Marshal(mfaInfo) return multiFactorInfoResponse{}, fmt.Errorf("Unsupported second factor %s provided", string(out)) } @@ -675,8 +691,8 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("the second factor \"displayName\" for \"%s\" must be a valid non-empty string", multiFactorInfo.DisplayName) } if multiFactorInfo.FactorID == phoneMultiFactorID { - if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo) @@ -1075,17 +1091,27 @@ func (r *userQueryResponse) makeExportedUserRecord() (*ExportedUserRecord, error enrollmentTimestamp = t.Unix() * 1000 } - if factor.PhoneInfo == "" { + if factor.PhoneInfo != "" { + enrolledFactors = append(enrolledFactors, &MultiFactorInfo{ + UID: factor.MFAEnrollmentID, + DisplayName: factor.DisplayName, + EnrollmentTimestamp: enrollmentTimestamp, + FactorID: phoneMultiFactorID, + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: factor.PhoneInfo, + }, + }) + } else if factor.TOTPInfo != nil { + enrolledFactors = append(enrolledFactors, &MultiFactorInfo{ + UID: factor.MFAEnrollmentID, + DisplayName: factor.DisplayName, + EnrollmentTimestamp: enrollmentTimestamp, + FactorID: totpMultiFactorID, + TOTPMultiFactorInfo: &TOTPInfo{}, + }) + } else { return nil, fmt.Errorf("unsupported multi-factor auth response: %#v", factor) } - - enrolledFactors = append(enrolledFactors, &MultiFactorInfo{ - UID: factor.MFAEnrollmentID, - DisplayName: factor.DisplayName, - EnrollmentTimestamp: enrollmentTimestamp, - FactorID: phoneMultiFactorID, - PhoneNumber: factor.PhoneInfo, - }) } return &ExportedUserRecord{ diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index 943810cd..94d2bfd6 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -69,11 +69,20 @@ var testUser = &UserRecord{ MultiFactor: &MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "0aaded3f-5e73-461d-aef9-37b48e3769be", + UID: "enrolledFactor1", FactorID: "phone", EnrollmentTimestamp: 1614776780000, - PhoneNumber: "+1234567890", - DisplayName: "My MFA Phone", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+1234567890", + }, + DisplayName: "My MFA Phone", + }, + { + UID: "enrolledFactor2", + FactorID: "totp", + EnrollmentTimestamp: 1614776780000, + TOTPMultiFactorInfo: &TOTPInfo{}, + DisplayName: "My MFA TOTP", }, }, }, @@ -646,8 +655,10 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "EnrollmentID", - PhoneNumber: "+11234567890", + UID: "EnrollmentID", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -658,7 +669,9 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "invalid", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "invalid", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -669,7 +682,9 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", EnrollmentTimestamp: time.Now().UTC().Unix(), @@ -681,7 +696,9 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "", }, @@ -692,8 +709,10 @@ func TestInvalidCreateUser(t *testing.T) { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", - FactorID: "phone", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, + FactorID: "phone", }, }, }), @@ -773,7 +792,9 @@ var createUserCases = []struct { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -790,12 +811,16 @@ var createUserCases = []struct { (&UserToCreate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "number1", FactorID: "phone", }, { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "number2", FactorID: "phone", }, @@ -875,9 +900,11 @@ func TestInvalidUpdateUser(t *testing.T) { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledSecondFactor1", - PhoneNumber: "+11234567890", - FactorID: "phone", + UID: "enrolledSecondFactor1", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, + FactorID: "phone", }, }, }), @@ -886,8 +913,10 @@ func TestInvalidUpdateUser(t *testing.T) { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledSecondFactor1", - PhoneNumber: "invalid", + UID: "enrolledSecondFactor1", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "invalid", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -898,7 +927,9 @@ func TestInvalidUpdateUser(t *testing.T) { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, FactorID: "phone", DisplayName: "Spouse's phone number", }, @@ -1049,14 +1080,18 @@ var updateUserCases = []struct { (&UserToUpdate{}).MFASettings(MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledSecondFactor1", - PhoneNumber: "+11234567890", + UID: "enrolledSecondFactor1", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", EnrollmentTimestamp: time.Now().Unix(), }, { - UID: "enrolledSecondFactor2", - PhoneNumber: "+11234567890", + UID: "enrolledSecondFactor2", + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -1886,10 +1921,16 @@ func TestMakeExportedUser(t *testing.T) { MFAInfo: []*multiFactorInfoResponse{ { PhoneInfo: "+1234567890", - MFAEnrollmentID: "0aaded3f-5e73-461d-aef9-37b48e3769be", + MFAEnrollmentID: "enrolledFactor1", DisplayName: "My MFA Phone", EnrolledAt: "2021-03-03T13:06:20.542896Z", }, + { + TOTPInfo: &TOTPInfo{}, + MFAEnrollmentID: "enrolledFactor2", + DisplayName: "My MFA TOTP", + EnrolledAt: "2021-03-03T13:06:20.542896Z", + }, }, } diff --git a/integration/auth/user_mgt_test.go b/integration/auth/user_mgt_test.go index c1301f6d..6f43224c 100644 --- a/integration/auth/user_mgt_test.go +++ b/integration/auth/user_mgt_test.go @@ -434,7 +434,9 @@ func TestCreateUserMFA(t *testing.T) { tc.MFASettings(auth.MultiFactorSettings{ EnrolledFactors: []*auth.MultiFactorInfo{ { - PhoneNumber: "+11234567890", + PhoneMultiFactorInfo: &auth.PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, DisplayName: "Spouse's phone number", FactorID: "phone", }, @@ -447,10 +449,12 @@ func TestCreateUserMFA(t *testing.T) { defer deleteUser(user.UID) var factor []*auth.MultiFactorInfo = []*auth.MultiFactorInfo{ { - UID: user.MultiFactor.EnrolledFactors[0].UID, - DisplayName: "Spouse's phone number", - FactorID: "phone", - PhoneNumber: "+11234567890", + UID: user.MultiFactor.EnrolledFactors[0].UID, + DisplayName: "Spouse's phone number", + FactorID: "phone", + PhoneMultiFactorInfo: &auth.PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, EnrollmentTimestamp: user.MultiFactor.EnrolledFactors[0].EnrollmentTimestamp, }, } diff --git a/testdata/get_user.json b/testdata/get_user.json index a2cac48b..95802ab3 100644 --- a/testdata/get_user.json +++ b/testdata/get_user.json @@ -35,11 +35,17 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "0aaded3f-5e73-461d-aef9-37b48e3769be", + "mfaEnrollmentId": "enrolledFactor1", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" + }, + { + "totpInfo": {}, + "mfaEnrollmentId": "enrolledFactor2", + "displayName": "My MFA TOTP", + "enrolledAt": "2021-03-03T13:06:20.542896Z" } ] } ] -} +} \ No newline at end of file diff --git a/testdata/list_users.json b/testdata/list_users.json index bf94ff49..8fb96058 100644 --- a/testdata/list_users.json +++ b/testdata/list_users.json @@ -33,12 +33,18 @@ "customAttributes": "{\"admin\": true, \"package\": \"gold\"}", "tenantId": "testTenant", "mfaInfo": [ - { - "phoneInfo": "+1234567890", - "mfaEnrollmentId": "0aaded3f-5e73-461d-aef9-37b48e3769be", - "displayName": "My MFA Phone", - "enrolledAt": "2021-03-03T13:06:20.542896Z" - } + { + "phoneInfo": "+1234567890", + "mfaEnrollmentId": "enrolledFactor1", + "displayName": "My MFA Phone", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + }, + { + "totpInfo": {}, + "mfaEnrollmentId": "enrolledFactor2", + "displayName": "My MFA TOTP", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + } ] }, { @@ -73,12 +79,18 @@ "customAttributes": "{\"admin\": true, \"package\": \"gold\"}", "tenantId": "testTenant", "mfaInfo": [ - { - "phoneInfo": "+1234567890", - "mfaEnrollmentId": "0aaded3f-5e73-461d-aef9-37b48e3769be", - "displayName": "My MFA Phone", - "enrolledAt": "2021-03-03T13:06:20.542896Z" - } + { + "phoneInfo": "+1234567890", + "mfaEnrollmentId": "enrolledFactor1", + "displayName": "My MFA Phone", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + }, + { + "totpInfo": {}, + "mfaEnrollmentId": "enrolledFactor2", + "displayName": "My MFA TOTP", + "enrolledAt": "2021-03-03T13:06:20.542896Z" + } ] }, { @@ -115,4 +127,4 @@ } ], "nextPageToken": "" -} +} \ No newline at end of file From 34b2f98e807b49d385e8c32abf4f5a6018847c00 Mon Sep 17 00:00:00 2001 From: Pragati Date: Wed, 31 May 2023 18:44:51 -0700 Subject: [PATCH 12/20] Add MFA fixes --- auth/user_mgt.go | 17 +++++++++++------ auth/user_mgt_test.go | 29 +++++++++++++---------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 3b0db181..de861dc0 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -78,6 +78,10 @@ type PhoneMultiFactorInfo struct { PhoneNumber string } +type multiFactorEnrollments struct { + Enrollments []*multiFactorInfoResponse `json:"enrollments"` +} + // MultiFactorInfo describes a user enrolled second phone factor. type MultiFactorInfo struct { UID string @@ -176,10 +180,13 @@ func (u *UserToCreate) set(key string, value interface{}) *UserToCreate { // Converts a client format second factor object to server format. func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorInfoResponse, error) { - var authFactorInfo multiFactorInfoResponse + authFactorInfo := multiFactorInfoResponse{DisplayName: mfaInfo.DisplayName} if mfaInfo.EnrollmentTimestamp != 0 { authFactorInfo.EnrolledAt = time.Unix(mfaInfo.EnrollmentTimestamp, 0).Format("2006-01-02T15:04:05Z07:00Z") } + if mfaInfo.UID != "" { + authFactorInfo.MFAEnrollmentID = mfaInfo.UID + } authFactorInfo.DisplayName = mfaInfo.DisplayName authFactorInfo.MFAEnrollmentID = mfaInfo.UID @@ -193,7 +200,7 @@ func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorI } out, _ := json.Marshal(mfaInfo) - return multiFactorInfoResponse{}, fmt.Errorf("Unsupported second factor %s provided", string(out)) + return multiFactorInfoResponse{}, fmt.Errorf("unsupported second factor %s provided", string(out)) } func (u *UserToCreate) validatedRequest() (map[string]interface{}, error) { @@ -349,7 +356,8 @@ func (u *UserToUpdate) validatedRequest() (map[string]interface{}, error) { if err != nil { return nil, err } - req["mfaInfo"] = mfaInfo + // https://cloud.google.com/identity-platform/docs/reference/rest/v1/accounts/update + req["mfa"] = multiFactorEnrollments{mfaInfo} } else { req[k] = v } @@ -681,9 +689,6 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("\"uid\" is not supported when adding second factors via \"createUser()\"") } case updateUserMethod: - if multiFactorInfo.UID == "" { - return nil, fmt.Errorf("the second factor \"uid\" must be a valid non-empty string when adding second factors via \"updateUser()\"") - } default: return nil, fmt.Errorf("unsupported methodType: %s", methodType) } diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index 94d2bfd6..1279d5f5 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -923,19 +923,6 @@ func TestInvalidUpdateUser(t *testing.T) { }, }), `the second factor "phoneNumber" for "invalid" must be a non-empty E.164 standard compliant identifier string`, - }, { - (&UserToUpdate{}).MFASettings(MultiFactorSettings{ - EnrolledFactors: []*MultiFactorInfo{ - { - PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ - PhoneNumber: "+11234567890", - }, - FactorID: "phone", - DisplayName: "Spouse's phone number", - }, - }, - }), - `the second factor "uid" must be a valid non-empty string when adding second factors via "updateUser()"`, }, { (&UserToUpdate{}).ProviderToLink(&UserProvider{UID: "google_uid"}), "user provider must specify a provider ID", @@ -1094,10 +1081,16 @@ var updateUserCases = []struct { }, DisplayName: "Spouse's phone number", FactorID: "phone", + }, { + PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ + PhoneNumber: "+11234567890", + }, + DisplayName: "Spouse's phone number", + FactorID: "phone", }, }, }), - map[string]interface{}{"mfaInfo": []*multiFactorInfoResponse{ + map[string]interface{}{"mfa": multiFactorEnrollments{Enrollments: []*multiFactorInfoResponse{ { MFAEnrollmentID: "enrolledSecondFactor1", PhoneInfo: "+11234567890", @@ -1109,12 +1102,16 @@ var updateUserCases = []struct { DisplayName: "Spouse's phone number", PhoneInfo: "+11234567890", }, - }, + { + DisplayName: "Spouse's phone number", + PhoneInfo: "+11234567890", + }, + }}, }, }, { (&UserToUpdate{}).MFASettings(MultiFactorSettings{}), - map[string]interface{}{"mfaInfo": nil}, + map[string]interface{}{"mfa": multiFactorEnrollments{Enrollments: nil}}, }, { (&UserToUpdate{}).ProviderToLink(&UserProvider{ From 3535bbddfa93f0ac15af5477ec3b63b7e7ec268a Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Mon, 5 Jun 2023 23:46:35 +0000 Subject: [PATCH 13/20] Addressing feedback --- auth/user_mgt.go | 2 +- auth/user_mgt_test.go | 8 ++++---- testdata/get_user.json | 6 +++--- testdata/list_users.json | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index de861dc0..459c1435 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -356,7 +356,7 @@ func (u *UserToUpdate) validatedRequest() (map[string]interface{}, error) { if err != nil { return nil, err } - // https://cloud.google.com/identity-platform/docs/reference/rest/v1/accounts/update + // Request body ref: https://cloud.google.com/identity-platform/docs/reference/rest/v1/accounts/update req["mfa"] = multiFactorEnrollments{mfaInfo} } else { req[k] = v diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index 1279d5f5..d6fbab71 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -69,7 +69,7 @@ var testUser = &UserRecord{ MultiFactor: &MultiFactorSettings{ EnrolledFactors: []*MultiFactorInfo{ { - UID: "enrolledFactor1", + UID: "enrolledPhoneFactor", FactorID: "phone", EnrollmentTimestamp: 1614776780000, PhoneMultiFactorInfo: &PhoneMultiFactorInfo{ @@ -78,7 +78,7 @@ var testUser = &UserRecord{ DisplayName: "My MFA Phone", }, { - UID: "enrolledFactor2", + UID: "enrolledTOTPFactor", FactorID: "totp", EnrollmentTimestamp: 1614776780000, TOTPMultiFactorInfo: &TOTPInfo{}, @@ -1918,13 +1918,13 @@ func TestMakeExportedUser(t *testing.T) { MFAInfo: []*multiFactorInfoResponse{ { PhoneInfo: "+1234567890", - MFAEnrollmentID: "enrolledFactor1", + MFAEnrollmentID: "enrolledPhoneFactor", DisplayName: "My MFA Phone", EnrolledAt: "2021-03-03T13:06:20.542896Z", }, { TOTPInfo: &TOTPInfo{}, - MFAEnrollmentID: "enrolledFactor2", + MFAEnrollmentID: "enrolledTOTPFactor", DisplayName: "My MFA TOTP", EnrolledAt: "2021-03-03T13:06:20.542896Z", }, diff --git a/testdata/get_user.json b/testdata/get_user.json index 95802ab3..0bf86f95 100644 --- a/testdata/get_user.json +++ b/testdata/get_user.json @@ -35,17 +35,17 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "enrolledFactor1", + "mfaEnrollmentId": "enrolledPhoneFactor", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" }, { "totpInfo": {}, - "mfaEnrollmentId": "enrolledFactor2", + "mfaEnrollmentId": "enrolledTOTPFactor", "displayName": "My MFA TOTP", "enrolledAt": "2021-03-03T13:06:20.542896Z" } ] } ] -} \ No newline at end of file +} diff --git a/testdata/list_users.json b/testdata/list_users.json index 8fb96058..2b630686 100644 --- a/testdata/list_users.json +++ b/testdata/list_users.json @@ -35,13 +35,13 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "enrolledFactor1", + "mfaEnrollmentId": "enrolledPhoneFactor", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" }, { "totpInfo": {}, - "mfaEnrollmentId": "enrolledFactor2", + "mfaEnrollmentId": "enrolledTOTPFactor", "displayName": "My MFA TOTP", "enrolledAt": "2021-03-03T13:06:20.542896Z" } @@ -81,13 +81,13 @@ "mfaInfo": [ { "phoneInfo": "+1234567890", - "mfaEnrollmentId": "enrolledFactor1", + "mfaEnrollmentId": "enrolledPhoneFactor", "displayName": "My MFA Phone", "enrolledAt": "2021-03-03T13:06:20.542896Z" }, { "totpInfo": {}, - "mfaEnrollmentId": "enrolledFactor2", + "mfaEnrollmentId": "enrolledTOTPFactor", "displayName": "My MFA TOTP", "enrolledAt": "2021-03-03T13:06:20.542896Z" } @@ -127,4 +127,4 @@ } ], "nextPageToken": "" -} \ No newline at end of file +} From e13f10cba6282dfa1c3db7f4be086882f7a8b52f Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Tue, 6 Jun 2023 00:11:06 +0000 Subject: [PATCH 14/20] Adding struct "`TOTPMultiFactorInfo`" as per review discussion. --- auth/user_mgt.go | 9 ++++++--- auth/user_mgt_test.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 459c1435..eee15523 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -70,7 +70,7 @@ type multiFactorInfoResponse struct { EnrolledAt string `json:"enrolledAt,omitempty"` } -// TOTPInfo describes a user enrolled second totp factor. +// TOTPInfo describes a server side user enrolled second totp factor. type TOTPInfo struct{} // PhoneMultiFactorInfo describes a user enrolled second phone factor. @@ -78,6 +78,9 @@ type PhoneMultiFactorInfo struct { PhoneNumber string } +// TOTPMultiFactorInfo describes a user enrolled second totp factor. +type TOTPMultiFactorInfo struct{} + type multiFactorEnrollments struct { Enrollments []*multiFactorInfoResponse `json:"enrollments"` } @@ -89,7 +92,7 @@ type MultiFactorInfo struct { EnrollmentTimestamp int64 FactorID string PhoneMultiFactorInfo *PhoneMultiFactorInfo - TOTPMultiFactorInfo *TOTPInfo + TOTPMultiFactorInfo *TOTPMultiFactorInfo } // MultiFactorSettings describes the multi-factor related user settings. @@ -1112,7 +1115,7 @@ func (r *userQueryResponse) makeExportedUserRecord() (*ExportedUserRecord, error DisplayName: factor.DisplayName, EnrollmentTimestamp: enrollmentTimestamp, FactorID: totpMultiFactorID, - TOTPMultiFactorInfo: &TOTPInfo{}, + TOTPMultiFactorInfo: &TOTPMultiFactorInfo{}, }) } else { return nil, fmt.Errorf("unsupported multi-factor auth response: %#v", factor) diff --git a/auth/user_mgt_test.go b/auth/user_mgt_test.go index d6fbab71..9ad64604 100644 --- a/auth/user_mgt_test.go +++ b/auth/user_mgt_test.go @@ -81,7 +81,7 @@ var testUser = &UserRecord{ UID: "enrolledTOTPFactor", FactorID: "totp", EnrollmentTimestamp: 1614776780000, - TOTPMultiFactorInfo: &TOTPInfo{}, + TOTPMultiFactorInfo: &TOTPMultiFactorInfo{}, DisplayName: "My MFA TOTP", }, }, From b1a5ac8608c261d358a212ade192258710c322b8 Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Thu, 22 Jun 2023 01:13:07 +0000 Subject: [PATCH 15/20] Addressing PR feedback --- auth/user_mgt.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index eee15523..6cc11bcb 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -190,20 +190,18 @@ func convertMultiFactorInfoToServerFormat(mfaInfo MultiFactorInfo) (multiFactorI if mfaInfo.UID != "" { authFactorInfo.MFAEnrollmentID = mfaInfo.UID } - authFactorInfo.DisplayName = mfaInfo.DisplayName authFactorInfo.MFAEnrollmentID = mfaInfo.UID switch mfaInfo.FactorID { case phoneMultiFactorID: authFactorInfo.PhoneInfo = mfaInfo.PhoneMultiFactorInfo.PhoneNumber - return authFactorInfo, nil case totpMultiFactorID: authFactorInfo.TOTPInfo = (*TOTPInfo)(mfaInfo.TOTPMultiFactorInfo) - return authFactorInfo, nil + default: + out, _ := json.Marshal(mfaInfo) + return multiFactorInfoResponse{}, fmt.Errorf("unsupported second factor %s provided", string(out)) } - - out, _ := json.Marshal(mfaInfo) - return multiFactorInfoResponse{}, fmt.Errorf("unsupported second factor %s provided", string(out)) + return authFactorInfo, nil } func (u *UserToCreate) validatedRequest() (map[string]interface{}, error) { @@ -692,6 +690,9 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("\"uid\" is not supported when adding second factors via \"createUser()\"") } case updateUserMethod: + if multiFactorInfo.UID == "" { + return nil, fmt.Errorf("the second factor \"uid\" must be a valid non-empty string when adding second factors via \"updateUser()\"") + } default: return nil, fmt.Errorf("unsupported methodType: %s", methodType) } @@ -699,8 +700,12 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("the second factor \"displayName\" for \"%s\" must be a valid non-empty string", multiFactorInfo.DisplayName) } if multiFactorInfo.FactorID == phoneMultiFactorID { - if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) + if multiFactorInfo.PhoneMultiFactorInfo != nil { + if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) + } + } else { + return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo) From 7495d4fbdecb22902ee19a99d1cc9c47453824e5 Mon Sep 17 00:00:00 2001 From: Pragati Date: Fri, 21 Jul 2023 12:48:20 -0700 Subject: [PATCH 16/20] adding `deprecated` tag to `PhoneNumber` field --- auth/user_mgt.go | 1 + 1 file changed, 1 insertion(+) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 6cc11bcb..15a61765 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -91,6 +91,7 @@ type MultiFactorInfo struct { DisplayName string EnrollmentTimestamp int64 FactorID string + PhoneNumber string `deprecated:"Use PhoneMultiFactorInfo instead"` PhoneMultiFactorInfo *PhoneMultiFactorInfo TOTPMultiFactorInfo *TOTPMultiFactorInfo } From b4a93cf4a89bb340210d79429fe08dc0fc79036b Mon Sep 17 00:00:00 2001 From: pragatimodi <110490169+pragatimodi@users.noreply.github.com> Date: Thu, 22 Jun 2023 01:27:16 +0000 Subject: [PATCH 17/20] Addressing PR feedback --- auth/user_mgt.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 15a61765..bdfd4289 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -691,9 +691,6 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st return nil, fmt.Errorf("\"uid\" is not supported when adding second factors via \"createUser()\"") } case updateUserMethod: - if multiFactorInfo.UID == "" { - return nil, fmt.Errorf("the second factor \"uid\" must be a valid non-empty string when adding second factors via \"updateUser()\"") - } default: return nil, fmt.Errorf("unsupported methodType: %s", methodType) } From afce475a47be27b7526deaf95eadf385f5b9a85c Mon Sep 17 00:00:00 2001 From: Pragati Date: Mon, 24 Jul 2023 13:10:32 -0700 Subject: [PATCH 18/20] Keep `PhoneNumber` field in `MultiFactorInfo` for compatibility --- auth/user_mgt.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index bdfd4289..4bc7ca2f 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -91,7 +91,7 @@ type MultiFactorInfo struct { DisplayName string EnrollmentTimestamp int64 FactorID string - PhoneNumber string `deprecated:"Use PhoneMultiFactorInfo instead"` + PhoneNumber string `Deprecated:"Use PhoneMultiFactorInfo instead"` PhoneMultiFactorInfo *PhoneMultiFactorInfo TOTPMultiFactorInfo *TOTPMultiFactorInfo } @@ -702,6 +702,13 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } + } else if multiFactorInfo.PhoneNumber != "" { + if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + } else { + multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber = multiFactorInfo.PhoneNumber + fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + } } else { return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } From afdd0576de2554f1e6261a2847f7be9907a583d9 Mon Sep 17 00:00:00 2001 From: Pragati Date: Mon, 24 Jul 2023 13:35:47 -0700 Subject: [PATCH 19/20] Validation logic fix --- auth/user_mgt.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 4bc7ca2f..1b01ddb6 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -699,18 +699,26 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st } if multiFactorInfo.FactorID == phoneMultiFactorID { if multiFactorInfo.PhoneMultiFactorInfo != nil { + // If PhoneMultiFactorInfo is provided, validate its PhoneNumber field if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } - } else if multiFactorInfo.PhoneNumber != "" { - if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + } else { + // PhoneMultiFactorInfo is nil, check the deprecated PhoneNumber field + if multiFactorInfo.PhoneNumber != "" { + if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) + } else { + // The PhoneNumber field is deprecated, set it in PhoneMultiFactorInfo and inform about the deprecation. + multiFactorInfo.PhoneMultiFactorInfo = &PhoneMultiFactorInfo{ + PhoneNumber: multiFactorInfo.PhoneNumber, + } + fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + } } else { - multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber = multiFactorInfo.PhoneNumber - fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + // Both PhoneMultiFactorInfo and deprecated PhoneNumber are missing. + return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } - } else { - return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo) From b9aa6d6391a027573d2495ce32cd73cd4e9a35db Mon Sep 17 00:00:00 2001 From: Pragati Date: Mon, 24 Jul 2023 13:38:57 -0700 Subject: [PATCH 20/20] lint fix --- auth/user_mgt.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/auth/user_mgt.go b/auth/user_mgt.go index 1b01ddb6..96efd545 100644 --- a/auth/user_mgt.go +++ b/auth/user_mgt.go @@ -703,22 +703,20 @@ func validateAndFormatMfaSettings(mfaSettings MultiFactorSettings, methodType st if err := validatePhone(multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber); err != nil { return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneMultiFactorInfo.PhoneNumber) } - } else { + // No need for the else here since we are returning from the function + } else if multiFactorInfo.PhoneNumber != "" { // PhoneMultiFactorInfo is nil, check the deprecated PhoneNumber field - if multiFactorInfo.PhoneNumber != "" { - if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { - return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) - } else { - // The PhoneNumber field is deprecated, set it in PhoneMultiFactorInfo and inform about the deprecation. - multiFactorInfo.PhoneMultiFactorInfo = &PhoneMultiFactorInfo{ - PhoneNumber: multiFactorInfo.PhoneNumber, - } - fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") - } - } else { - // Both PhoneMultiFactorInfo and deprecated PhoneNumber are missing. - return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") + if err := validatePhone(multiFactorInfo.PhoneNumber); err != nil { + return nil, fmt.Errorf("the second factor \"phoneNumber\" for \"%s\" must be a non-empty E.164 standard compliant identifier string", multiFactorInfo.PhoneNumber) } + // The PhoneNumber field is deprecated, set it in PhoneMultiFactorInfo and inform about the deprecation. + multiFactorInfo.PhoneMultiFactorInfo = &PhoneMultiFactorInfo{ + PhoneNumber: multiFactorInfo.PhoneNumber, + } + fmt.Println("`PhoneNumber` is deprecated, use `PhoneMultiFactorInfo` instead") + } else { + // Both PhoneMultiFactorInfo and deprecated PhoneNumber are missing. + return nil, fmt.Errorf("\"PhoneMultiFactorInfo\" must be defined") } } obj, err := convertMultiFactorInfoToServerFormat(*multiFactorInfo)