18858: Adds duplicated email check & tests.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 25c7594560a9800f308c10e4b95da05e99306839..561eb091daeb48a71c44620a615a619f9c660e01 100644 (file)
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -14,6 +14,7 @@ import (
        "log"
        "net/url"
        "os"
        "log"
        "net/url"
        "os"

+       "regexp"

        "strconv"
        "strings"
 
        "strconv"
        "strings"
 


@@ -172,16 +173,38 @@ func doMain(cfg *ConfigParams) error {
        defer f.Close()
 
        allUsers := make(map[string]arvados.User)
        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)
        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)
        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)
        }
 
        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))
 
 
        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
 }
 
        return nil
 }
 

diff --git a/tools/sync-users/sync-users_test.go b/tools/sync-users/sync-users_test.go
index 6d10595d4655a7ad96c339cb2ead154fd3c198d4..0b6ef6c744c31e5f976c51b9abd7b20ce3db141f 100644 (file)
--- 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"
        "fmt"
        "io/ioutil"
        "os"

+       "regexp"

        "strings"
        "testing"
 
        "strings"
        "testing"
 


@@ -291,3 +292,91 @@ func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
                c.Assert(foundUser.IsAdmin, Equals, false) // inactive users cannot be admins
        }
 }
                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.*")
+}