From: Lucas Di Pentima Date: Fri, 24 Jun 2022 17:29:05 +0000 (-0300) Subject: 18858: Adds duplicated email check & tests. X-Git-Tag: 2.5.0~124^2~6 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/795a29f42c333ee369e852977e7018fe4c5c94ae 18858: Adds duplicated email check & tests. Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima --- diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go index 25c7594560..561eb091da 100644 --- a/tools/sync-users/sync-users.go +++ b/tools/sync-users/sync-users.go @@ -14,6 +14,7 @@ import ( "log" "net/url" "os" + "regexp" "strconv" "strings" @@ -172,16 +173,38 @@ func doMain(cfg *ConfigParams) error { defer f.Close() allUsers := make(map[string]arvados.User) + dupedEmails := make(map[string][]arvados.User) processedUsers := make(map[string]bool) results, err := GetAll(cfg.Client, "users", arvados.ResourceListParams{}, &UserList{}) if err != nil { return fmt.Errorf("error getting all users: %s", err) } log.Printf("Found %d users in cluster %q", len(results), cfg.ClusterID) + localUserUuidRegex := regexp.MustCompile(fmt.Sprintf("^%s-tpzed-[0-9a-z]{15}$", cfg.ClusterID)) for _, item := range results { u := item.(arvados.User) - allUsers[strings.ToLower(u.Email)] = u - processedUsers[strings.ToLower(u.Email)] = false + // Remote user check + if !localUserUuidRegex.MatchString(u.UUID) { + if cfg.Verbose { + log.Printf("Remote user %q (%s) won't be considered for processing", u.Email, u.UUID) + } + continue + } + // Duplicated user's email check + email := strings.ToLower(u.Email) + if ul, ok := dupedEmails[email]; ok { + log.Printf("Duplicated email %q found in user %s", email, u.UUID) + dupedEmails[email] = append(ul, u) + continue + } + if eu, ok := allUsers[email]; ok { + log.Printf("Duplicated email %q found in users %s and %s", email, eu.UUID, u.UUID) + dupedEmails[email] = []arvados.User{eu, u} + delete(allUsers, email) + continue + } + allUsers[email] = u + processedUsers[email] = false } loadedRecords, err := LoadInputFile(f) @@ -234,6 +257,17 @@ func doMain(cfg *ConfigParams) error { log.Printf("User update successes: %d, skips: %d, failures: %d", len(updatesSucceeded), len(updatesSkipped), len(updatesFailed)) + // Report duplicated emails detection + if len(dupedEmails) > 0 { + emails := make([]string, len(dupedEmails)) + i := 0 + for e := range dupedEmails { + emails[i] = e + i++ + } + return fmt.Errorf("skipped %d duplicated email address(es) in the cluster's local user list: %v", len(dupedEmails), emails) + } + return nil } diff --git a/tools/sync-users/sync-users_test.go b/tools/sync-users/sync-users_test.go index 6d10595d46..0b6ef6c744 100644 --- a/tools/sync-users/sync-users_test.go +++ b/tools/sync-users/sync-users_test.go @@ -8,6 +8,7 @@ import ( "fmt" "io/ioutil" "os" + "regexp" "strings" "testing" @@ -291,3 +292,91 @@ func (s *TestSuite) TestUserCreationAndUpdate(c *C) { c.Assert(foundUser.IsAdmin, Equals, false) // inactive users cannot be admins } } + +func (s *TestSuite) TestDeactivateUnlisted(c *C) { + localUserUuidRegex := regexp.MustCompile(fmt.Sprintf("^%s-tpzed-[0-9a-z]{15}$", s.cfg.ClusterID)) + users, err := ListUsers(s.cfg.Client) + c.Assert(err, IsNil) + previouslyActiveUsers := 0 + for _, u := range users { + if u.UUID == fmt.Sprintf("%s-tpzed-anonymouspublic", s.cfg.ClusterID) && !u.IsActive { + // Make sure the anonymous user is active for this test + var au arvados.User + err := UpdateUser(s.cfg.Client, u.UUID, &au, map[string]string{"is_active": "true"}) + c.Assert(err, IsNil) + c.Assert(au.IsActive, Equals, true) + } + if localUserUuidRegex.MatchString(u.UUID) && u.IsActive { + previouslyActiveUsers++ + } + } + // At least 3 active users: System root, Anonymous and the current user. + // Other active users should exist from fixture. + c.Logf("Initial active users count: %d", previouslyActiveUsers) + c.Assert(previouslyActiveUsers > 3, Equals, true) + + s.cfg.DeactivateUnlisted = true + s.cfg.Verbose = true + data := [][]string{ + {"user1@example.com", "Example", "User1", "0", "0"}, + } + tmpfile, err := MakeTempCSVFile(data) + c.Assert(err, IsNil) + defer os.Remove(tmpfile.Name()) + s.cfg.Path = tmpfile.Name() + err = doMain(s.cfg) + c.Assert(err, IsNil) + + users, err = ListUsers(s.cfg.Client) + c.Assert(err, IsNil) + currentlyActiveUsers := 0 + acceptableActiveUUIDs := map[string]bool{ + fmt.Sprintf("%s-tpzed-000000000000000", s.cfg.ClusterID): true, + fmt.Sprintf("%s-tpzed-anonymouspublic", s.cfg.ClusterID): true, + s.cfg.CurrentUser.UUID: true, + } + remainingActiveUUIDs := map[string]bool{} + seenUserEmails := map[string]bool{} + for _, u := range users { + if _, ok := seenUserEmails[u.Email]; ok { + c.Errorf("Duplicated email address %q in user list (probably from fixtures). This test requires unique email addresses.", u.Email) + } + seenUserEmails[u.Email] = true + if localUserUuidRegex.MatchString(u.UUID) && u.IsActive { + c.Logf("Found remaining active user %q (%s)", u.Email, u.UUID) + _, ok := acceptableActiveUUIDs[u.UUID] + c.Assert(ok, Equals, true) + remainingActiveUUIDs[u.UUID] = true + currentlyActiveUsers++ + } + } + // 3 active users remaining: System root, Anonymous and the current user. + c.Logf("Active local users remaining: %v", remainingActiveUUIDs) + c.Assert(currentlyActiveUsers, Equals, 3) +} + +func (s *TestSuite) TestFailOnDuplicatedEmails(c *C) { + for i := range []int{1, 2} { + isAdmin := i == 2 + err := CreateUser(s.cfg.Client, &arvados.User{}, map[string]string{ + "email": "somedupedemail@example.com", + "first_name": fmt.Sprintf("Duped %d", i), + "username": fmt.Sprintf("dupedemail%d", i), + "last_name": "User", + "is_active": "true", + "is_admin": fmt.Sprintf("%t", isAdmin), + }) + c.Assert(err, IsNil) + } + s.cfg.Verbose = true + data := [][]string{ + {"user1@example.com", "Example", "User1", "0", "0"}, + } + tmpfile, err := MakeTempCSVFile(data) + c.Assert(err, IsNil) + defer os.Remove(tmpfile.Name()) + s.cfg.Path = tmpfile.Name() + err = doMain(s.cfg) + c.Assert(err, NotNil) + c.Assert(err, ErrorMatches, "skipped.*duplicated email address.*") +}