From 1f5d824aaf00c51b7935216c32be8b90deb710b1 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Mon, 28 Aug 2023 20:29:37 +0000 Subject: [PATCH 1/2] Fixed TestFetchAppUserIdentities and TestCreateUser under user_test.go --- pkg/models/user.go | 2 +- pkg/models/user_test.go | 11 +++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/pkg/models/user.go b/pkg/models/user.go index 0f5a1127dae..3c0d53b2871 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -202,7 +202,7 @@ func FetchAppUserIdentities(db *pop.Connection, appname auth.Application, limit sm.middle_name AS sm_middle FROM service_members as sm JOIN users on sm.user_id = users.id - WHERE users.okta_email != 'first.last@login.gov.test' + WHERE users.okta_email != 'first.last@okta.mil' ORDER BY users.created_at DESC LIMIT $1` } diff --git a/pkg/models/user_test.go b/pkg/models/user_test.go index 661ffb072fa..455161e838c 100644 --- a/pkg/models/user_test.go +++ b/pkg/models/user_test.go @@ -72,17 +72,12 @@ func (suite *ModelSuite) TestUserCreationDuplicateUUID() { func (suite *ModelSuite) TestCreateUser() { const testEmail = "Sally@GoVernment.gov" const expectedEmail = "sally@government.gov" - const goodUUID = "39b28c92-0506-4bef-8b57-e39519f42dc2" - const badUUID = "39xnfc92-0506-4bef-8b57-e39519f42dc2" + const oktaID = "00u3ckm7yEoUJuI1i0k6" - sally, err := CreateUser(suite.DB(), goodUUID, testEmail) + sally, err := CreateUser(suite.DB(), oktaID, testEmail) suite.Nil(err, "No error for good create") suite.Equal(expectedEmail, sally.OktaEmail, "should convert email to lower case") suite.NotEqual(sally.ID, uuid.Nil) - - fail, err := CreateUser(suite.DB(), expectedEmail, badUUID) - suite.NotNil(err, "should get and error from bad uuid") - suite.Nil(fail, "no user with bad uuid") } func (suite *ModelSuite) TestFetchUserIdentity() { @@ -261,7 +256,7 @@ func (suite *ModelSuite) TestFetchAppUserIdentities() { suite.Run("service member", func() { // Create a user email that won't be filtered out of the devlocal user query w/ a default value of - // first.last@login.gov.test + // first.last@okta.mil user := factory.BuildUser(suite.DB(), []factory.Customization{ { Model: User{ From 6c684656c61f72ec1fb8b4370b98b43b266c5c73 Mon Sep 17 00:00:00 2001 From: cameroncaci Date: Tue, 29 Aug 2023 15:01:45 +0000 Subject: [PATCH 2/2] Added new users constraint for unique okta id. Added migration. Made factory random string exportable. Adjusting unit tests --- migrations/app/migrations_manifest.txt | 1 + ...133429_add_unique_constaint_okta_id.up.sql | 1 + pkg/factory/admin_user_factory.go | 2 +- pkg/factory/duty_location_factory.go | 2 +- pkg/factory/office_user_factory.go | 2 +- pkg/factory/shared.go | 2 +- pkg/factory/user_factory.go | 2 +- pkg/models/user.go | 4 +-- pkg/models/user_test.go | 26 +++++++++---------- 9 files changed, 21 insertions(+), 21 deletions(-) create mode 100644 migrations/app/schema/20230829133429_add_unique_constaint_okta_id.up.sql diff --git a/migrations/app/migrations_manifest.txt b/migrations/app/migrations_manifest.txt index 3a1d7f8bee2..cb114d06709 100644 --- a/migrations/app/migrations_manifest.txt +++ b/migrations/app/migrations_manifest.txt @@ -844,3 +844,4 @@ 20230807180000_import_trdm_part_3.up.sql 20230807185455_alter_users_table.up.sql 20230823194524_copy_loginGov_oktaEmail.up.sql +20230829133429_add_unique_constaint_okta_id.up.sql diff --git a/migrations/app/schema/20230829133429_add_unique_constaint_okta_id.up.sql b/migrations/app/schema/20230829133429_add_unique_constaint_okta_id.up.sql new file mode 100644 index 00000000000..b860dc1842e --- /dev/null +++ b/migrations/app/schema/20230829133429_add_unique_constaint_okta_id.up.sql @@ -0,0 +1 @@ +ALTER TABLE users ADD CONSTRAINT users_okta_un UNIQUE (okta_id); diff --git a/pkg/factory/admin_user_factory.go b/pkg/factory/admin_user_factory.go index 91a78aee834..3cd1b1e752f 100644 --- a/pkg/factory/admin_user_factory.go +++ b/pkg/factory/admin_user_factory.go @@ -66,7 +66,7 @@ func BuildDefaultAdminUser(db *pop.Connection) models.AdminUser { // GetTraitAdminUserEmail helps comply with the uniqueness constraint on emails func GetTraitAdminUserEmail() []Customization { // There's a uniqueness constraint on admin user emails so add some randomness - email := strings.ToLower(fmt.Sprintf("leo_spaceman_admin_%s@example.com", makeRandomString(5))) + email := strings.ToLower(fmt.Sprintf("leo_spaceman_admin_%s@example.com", MakeRandomString(5))) return []Customization{ { Model: models.User{ diff --git a/pkg/factory/duty_location_factory.go b/pkg/factory/duty_location_factory.go index a7c6a361fd5..e55b41d785d 100644 --- a/pkg/factory/duty_location_factory.go +++ b/pkg/factory/duty_location_factory.go @@ -53,7 +53,7 @@ func buildDutyLocationWithBuildType(db *pop.Connection, customs []Customization, affiliation := internalmessages.AffiliationAIRFORCE location := models.DutyLocation{ - Name: makeRandomString(10), + Name: MakeRandomString(10), Affiliation: &affiliation, AddressID: dlAddress.ID, Address: dlAddress, diff --git a/pkg/factory/office_user_factory.go b/pkg/factory/office_user_factory.go index b13815484de..16dadd5657b 100644 --- a/pkg/factory/office_user_factory.go +++ b/pkg/factory/office_user_factory.go @@ -114,7 +114,7 @@ func BuildOfficeUserWithRoles(db *pop.Connection, customs []Customization, roleT // GetTraitOfficeUserEmail helps comply with the uniqueness constraint on emails func GetTraitOfficeUserEmail() []Customization { // There's a uniqueness constraint on office user emails so add some randomness - email := strings.ToLower(fmt.Sprintf("leo_spaceman_office_%s@example.com", makeRandomString(5))) + email := strings.ToLower(fmt.Sprintf("leo_spaceman_office_%s@example.com", MakeRandomString(5))) return []Customization{ { Model: models.User{ diff --git a/pkg/factory/shared.go b/pkg/factory/shared.go index d645408fc90..e616936aa52 100644 --- a/pkg/factory/shared.go +++ b/pkg/factory/shared.go @@ -620,7 +620,7 @@ func RandomEdipi() string { const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890" // Returns a random alphanumeric string of specified length -func makeRandomString(n int) string { +func MakeRandomString(n int) string { b := make([]byte, n) for i := range b { randInt, err := random.GetRandomInt(len(letterBytes)) diff --git a/pkg/factory/user_factory.go b/pkg/factory/user_factory.go index edbb2f98813..1ec4f40ed0c 100644 --- a/pkg/factory/user_factory.go +++ b/pkg/factory/user_factory.go @@ -30,7 +30,7 @@ func BuildUser(db *pop.Connection, customs []Customization, traits []Trait) mode } // create user - OktaID := makeRandomString(20) + OktaID := MakeRandomString(20) user := models.User{ OktaID: OktaID, OktaEmail: "first.last@okta.mil", diff --git a/pkg/models/user.go b/pkg/models/user.go index 3c0d53b2871..884c2071b6b 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -123,7 +123,7 @@ type UserIdentity struct { } // FetchUserIdentity queries the database for information about the logged in user -func FetchUserIdentity(db *pop.Connection, loginGovID string) (*UserIdentity, error) { +func FetchUserIdentity(db *pop.Connection, oktaID string) (*UserIdentity, error) { var identities []UserIdentity query := `SELECT users.id, users.okta_email AS email, @@ -147,7 +147,7 @@ func FetchUserIdentity(db *pop.Connection, loginGovID string) (*UserIdentity, er LEFT OUTER JOIN office_users AS ou on ou.user_id = users.id LEFT OUTER JOIN admin_users AS au on au.user_id = users.id WHERE users.okta_id = $1` - err := db.RawQuery(query, loginGovID).All(&identities) + err := db.RawQuery(query, oktaID).All(&identities) if err != nil { return nil, err } else if len(identities) == 0 { diff --git a/pkg/models/user_test.go b/pkg/models/user_test.go index 455161e838c..bbd61ab175a 100644 --- a/pkg/models/user_test.go +++ b/pkg/models/user_test.go @@ -14,11 +14,11 @@ import ( ) func (suite *ModelSuite) TestUserValidation() { - fakeUUID, _ := uuid.FromString("39b28c92-0506-4bef-8b57-e39519f42dc1") + oktaID := "abcdefghijklmnopqrst" userEmail := "sally@government.gov" newUser := User{ - OktaID: fakeUUID.String(), + OktaID: oktaID, OktaEmail: userEmail, } @@ -27,7 +27,7 @@ func (suite *ModelSuite) TestUserValidation() { suite.NoError(err) suite.False(verrs.HasAny(), "Error validating model") suite.Equal(userEmail, newUser.OktaEmail) - suite.Equal(fakeUUID, newUser.OktaID) + suite.Equal(oktaID, newUser.OktaID) } func (suite *ModelSuite) TestUserCreationWithoutValues() { @@ -40,17 +40,17 @@ func (suite *ModelSuite) TestUserCreationWithoutValues() { suite.verifyValidationErrors(newUser, expErrors) } -func (suite *ModelSuite) TestUserCreationDuplicateUUID() { - fakeUUID, _ := uuid.FromString("39b28c92-0506-4bef-8b57-e39519f42dc2") +func (suite *ModelSuite) TestUserCreationDuplicateOktaID() { + oktaID := "abcdefghijklmnopqrst" userEmail := "sally@government.gov" newUser := User{ - OktaID: fakeUUID.String(), + OktaID: oktaID, OktaEmail: userEmail, } sameUser := User{ - OktaID: fakeUUID.String(), + OktaID: oktaID, OktaEmail: userEmail, } @@ -65,8 +65,7 @@ func (suite *ModelSuite) TestUserCreationDuplicateUUID() { // nolint:errcheck suite.DB().Create(&newUser) err := suite.DB().Create(&sameUser) - - suite.True(dberr.IsDBErrorForConstraint(err, pgerrcode.UniqueViolation, "constraint_name"), "Db should have errored on unique constraint for UUID") + suite.True(dberr.IsDBErrorForConstraint(err, pgerrcode.UniqueViolation, "users_okta_un"), "Db should have errored on unique constraint for UUID") } func (suite *ModelSuite) TestCreateUser() { @@ -81,9 +80,9 @@ func (suite *ModelSuite) TestCreateUser() { } func (suite *ModelSuite) TestFetchUserIdentity() { - const goodUUID = "39b28c92-0506-4bef-8b57-e39519f42dc2" + oktaID := factory.MakeRandomString(20) // First check that it all works with no record - identity, err := FetchUserIdentity(suite.DB(), goodUUID) + identity, err := FetchUserIdentity(suite.DB(), oktaID) suite.Equal(ErrFetchNotFound, err, "Expected not to find missing Identity") suite.Nil(identity) @@ -134,7 +133,6 @@ func (suite *ModelSuite) TestFetchUserIdentity() { suite.Equal(systemAdmin.User.OktaEmail, identity.Email) suite.Nil(identity.ServiceMemberID) suite.Nil(identity.OfficeUserID) - rs := []roles.Role{{ ID: uuid.FromStringOrNil("ed2d2cd7-d427-412a-98bb-a9b391d98d32"), RoleType: roles.RoleTypeCustomer, @@ -145,11 +143,11 @@ func (suite *ModelSuite) TestFetchUserIdentity() { } suite.NoError(suite.DB().Create(&rs)) customerRole := rs[0] - patUUID := uuid.Must(uuid.NewV4()) + patOktaID := factory.MakeRandomString(20) pat := factory.BuildUser(suite.DB(), []factory.Customization{ { Model: User{ - OktaID: patUUID.String(), + OktaID: patOktaID, Active: true, Roles: []roles.Role{customerRole}, },