[arvados] created: 2.1.0-2634-g36f730574

git repository hosting git at public.arvados.org
Mon Jul 4 14:32:20 UTC 2022


        at  36f730574cf6d5f720656de6a102963af5e15cab (commit)


commit 36f730574cf6d5f720656de6a102963af5e15cab
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Mon Jul 4 11:29:43 2022 -0300

    18858: Adds username-based test cases.
    
    Also, fixes user fixtures and improves railsAPI's debugging logs on database
    reset failures.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/services/api/app/controllers/database_controller.rb b/services/api/app/controllers/database_controller.rb
index fa1e1ca43..69453959d 100644
--- a/services/api/app/controllers/database_controller.rb
+++ b/services/api/app/controllers/database_controller.rb
@@ -25,7 +25,7 @@ class DatabaseController < ApplicationController
     unexpected_uuids = user_uuids - fixture_uuids
     if unexpected_uuids.any?
       logger.error("Running in test environment, but non-fixture users exist: " +
-                   "#{unexpected_uuids}")
+                   "#{unexpected_uuids}" + "\nMaybe test users without @example.com email addresses were created?")
       raise ArvadosModel::PermissionDeniedError
     end
 
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 1cf79b1a5..1d9bcbb04 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -12,6 +12,7 @@ system_user:
   modified_by_user_uuid: zzzzz-tpzed-000000000000000
   modified_at: 2014-11-27 06:38:21.208036000 Z
   email: root
+  username: root
   first_name: root
   last_name: ''
   identity_url:
@@ -193,6 +194,7 @@ inactive_uninvited:
   identity_url: https://inactive-uninvited-user.openid.local
   is_active: false
   is_admin: false
+  username: inactiveuninvited
   prefs: {}
 
 inactive:
@@ -216,6 +218,7 @@ inactive_but_signed_user_agreement:
   identity_url: https://inactive-but-agreeable-user.openid.local
   is_active: false
   is_admin: false
+  username: inactiveusersignedua
   prefs:
     profile:
       organization: example.com
@@ -230,6 +233,7 @@ anonymous:
   last_name: anonymouspublic
   is_active: false
   is_admin: false
+  username: anonymous
   prefs: {}
 
 job_reader:
@@ -273,6 +277,7 @@ active_no_prefs:
   identity_url: https://active_no_prefs.openid.local
   is_active: true
   is_admin: false
+  username: activenoprefs
   prefs: {}
 
 active_no_prefs_profile_no_getting_started_shown:
@@ -284,6 +289,7 @@ active_no_prefs_profile_no_getting_started_shown:
   identity_url: https://active_no_prefs_profile.openid.local
   is_active: true
   is_admin: false
+  username: activenoprefsprofilenogs
   prefs:
     test: abc
 
@@ -296,6 +302,7 @@ active_no_prefs_profile_with_getting_started_shown:
   identity_url: https://active_no_prefs_profile_seen_gs.openid.local
   is_active: true
   is_admin: false
+  username: activenoprefsprofile
   prefs:
     test: abc
     getting_started_shown: 2015-03-26 12:34:56.789000000 Z
@@ -308,6 +315,7 @@ active_with_prefs_profile_no_getting_started_shown:
   last_name: NoGettingStartedShown
   identity_url: https://active_nogettinstarted.openid.local
   is_active: true
+  username: activenogettinstarted
   prefs:
     profile:
       organization: example.com
diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 5cf341036..001014255 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -94,8 +94,8 @@ func ParseFlags(cfg *ConfigParams) error {
 
 	caseInsensitive := flags.Bool(
 		"case-insensitive",
-		true,
-		"Performs case insensitive matching on user IDs. Always ON whe using 'email' user IDs.")
+		false,
+		"Performs case insensitive matching on user IDs. Always ON when using 'email' user IDs.")
 	deactivateUnlisted := flags.Bool(
 		"deactivate-unlisted",
 		false,
@@ -107,7 +107,7 @@ func ParseFlags(cfg *ConfigParams) error {
 	verbose := flags.Bool(
 		"verbose",
 		false,
-		"Log informational messages. Off by default.")
+		"Log informational messages.")
 	getVersion := flags.Bool(
 		"version",
 		false,
@@ -366,7 +366,7 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, userIDToUUID map[string
 	}
 
 	wantedActiveStatus := strconv.FormatBool(record.Active)
-	wantedAdminStatus := strconv.FormatBool(record.Admin)
+	wantedAdminStatus := strconv.FormatBool(record.Active && record.Admin)
 	createRequired := false
 	updateRequired := false
 	// Check if user exists, set its active & admin status.
diff --git a/tools/sync-users/sync-users_test.go b/tools/sync-users/sync-users_test.go
index 8b93d32e4..5c272b0da 100644
--- a/tools/sync-users/sync-users_test.go
+++ b/tools/sync-users/sync-users_test.go
@@ -98,6 +98,14 @@ func (s *TestSuite) TestParseFlagsWithoutPositionalArgument(c *C) {
 	os.Args = []string{"cmd", "-verbose"}
 	err := ParseFlags(&ConfigParams{})
 	c.Assert(err, NotNil)
+	c.Assert(err, ErrorMatches, ".*please provide a path to an input file.*")
+}
+
+func (s *TestSuite) TestParseFlagsWrongUserID(c *C) {
+	os.Args = []string{"cmd", "-user-id=nickname", "/tmp/somefile.csv"}
+	err := ParseFlags(&ConfigParams{})
+	c.Assert(err, NotNil)
+	c.Assert(err, ErrorMatches, ".*user ID must be one of:.*")
 }
 
 func (s *TestSuite) TestParseFlagsWithPositionalArgument(c *C) {
@@ -108,16 +116,20 @@ func (s *TestSuite) TestParseFlagsWithPositionalArgument(c *C) {
 	c.Assert(cfg.Path, Equals, "/tmp/somefile.csv")
 	c.Assert(cfg.Verbose, Equals, false)
 	c.Assert(cfg.DeactivateUnlisted, Equals, false)
+	c.Assert(cfg.UserID, Equals, "email")
+	c.Assert(cfg.CaseInsensitive, Equals, true)
 }
 
 func (s *TestSuite) TestParseFlagsWithOptionalFlags(c *C) {
 	cfg := ConfigParams{}
-	os.Args = []string{"cmd", "-verbose", "-deactivate-unlisted", "/tmp/somefile.csv"}
+	os.Args = []string{"cmd", "-verbose", "-deactivate-unlisted", "-user-id=username", "/tmp/somefile.csv"}
 	err := ParseFlags(&cfg)
 	c.Assert(err, IsNil)
 	c.Assert(cfg.Path, Equals, "/tmp/somefile.csv")
 	c.Assert(cfg.Verbose, Equals, true)
 	c.Assert(cfg.DeactivateUnlisted, Equals, true)
+	c.Assert(cfg.UserID, Equals, "username")
+	c.Assert(cfg.CaseInsensitive, Equals, false)
 }
 
 func (s *TestSuite) TestGetConfig(c *C) {
@@ -218,85 +230,118 @@ func (s *TestSuite) TestWrongDataFields(c *C) {
 	}
 }
 
-// Activate and deactivate users
+// Create, activate and deactivate users
 func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
-	testCases := []userRecord{{
-		UserID:    "user1 at example.com",
-		FirstName: "Example",
-		LastName:  "User1",
-		Active:    true,
-		Admin:     false,
-	}, {
-		UserID:    "admin1 at example.com",
-		FirstName: "Example",
-		LastName:  "Admin1",
-		Active:    true,
-		Admin:     true,
-	}}
-	// Make sure users aren't already there from fixtures
-	for _, user := range s.users {
-		e := user.Email
-		found := false
-		for _, r := range testCases {
-			if e == r.UserID {
-				found = true
-				break
+	for _, tc := range []string{"email", "username"} {
+		uIDPrefix := tc
+		uIDSuffix := ""
+		if tc == "email" {
+			uIDSuffix = "@example.com"
+		}
+		s.cfg.UserID = tc
+		records := []userRecord{{
+			UserID:    fmt.Sprintf("%suser1%s", uIDPrefix, uIDSuffix),
+			FirstName: "Example",
+			LastName:  "User1",
+			Active:    true,
+			Admin:     false,
+		}, {
+			UserID:    fmt.Sprintf("%suser2%s", uIDPrefix, uIDSuffix),
+			FirstName: "Example",
+			LastName:  "User2",
+			Active:    false, // initially inactive
+			Admin:     false,
+		}, {
+			UserID:    fmt.Sprintf("%sadmin1%s", uIDPrefix, uIDSuffix),
+			FirstName: "Example",
+			LastName:  "Admin1",
+			Active:    true,
+			Admin:     true,
+		}, {
+			UserID:    fmt.Sprintf("%sadmin2%s", uIDPrefix, uIDSuffix),
+			FirstName: "Example",
+			LastName:  "Admin2",
+			Active:    false, // initially inactive
+			Admin:     true,
+		}}
+		// Make sure users aren't already there from fixtures
+		for _, user := range s.users {
+			uID, err := GetUserID(user, s.cfg.UserID)
+			c.Assert(err, IsNil)
+			found := false
+			for _, r := range records {
+				if uID == r.UserID {
+					found = true
+					break
+				}
 			}
+			c.Assert(found, Equals, false)
 		}
-		c.Assert(found, Equals, false)
-	}
-	// User creation
-	tmpfile, err := MakeTempCSVFile(RecordsToStrings(testCases))
-	c.Assert(err, IsNil)
-	defer os.Remove(tmpfile.Name())
-	s.cfg.Path = tmpfile.Name()
-	err = doMain(s.cfg)
-	c.Assert(err, IsNil)
+		// User creation
+		tmpfile, err := MakeTempCSVFile(RecordsToStrings(records))
+		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)
-	for _, tc := range testCases {
-		var foundUser arvados.User
-		for _, user := range users {
-			if user.Email == tc.UserID {
-				foundUser = user
-				break
+		users, err := ListUsers(s.cfg.Client)
+		c.Assert(err, IsNil)
+		for _, r := range records {
+			var foundUser arvados.User
+			for _, user := range users {
+				uID, err := GetUserID(user, s.cfg.UserID)
+				c.Assert(err, IsNil)
+				if uID == r.UserID {
+					// Add an @example.com email if missing
+					// (to avoid database reset errors)
+					if tc == "username" && user.Email == "" {
+						err := UpdateUser(s.cfg.Client, user.UUID, &user, map[string]string{
+							"email": fmt.Sprintf("%s at example.com", user.Username),
+						})
+						c.Assert(err, IsNil)
+					}
+					foundUser = user
+					break
+				}
 			}
+			c.Assert(foundUser, NotNil)
+			c.Logf("Checking creation for user %q", r.UserID)
+			c.Assert(foundUser.FirstName, Equals, r.FirstName)
+			c.Assert(foundUser.LastName, Equals, r.LastName)
+			c.Assert(foundUser.IsActive, Equals, r.Active)
+			c.Assert(foundUser.IsAdmin, Equals, (r.Active && r.Admin))
 		}
-		c.Assert(foundUser, NotNil)
-		c.Logf("Checking recently created user %q", foundUser.Email)
-		c.Assert(foundUser.FirstName, Equals, tc.FirstName)
-		c.Assert(foundUser.LastName, Equals, tc.LastName)
-		c.Assert(foundUser.IsActive, Equals, true)
-		c.Assert(foundUser.IsAdmin, Equals, tc.Admin)
-	}
-	// User deactivation
-	for idx := range testCases {
-		testCases[idx].Active = false
-	}
-	tmpfile, err = MakeTempCSVFile(RecordsToStrings(testCases))
-	c.Assert(err, IsNil)
-	defer os.Remove(tmpfile.Name())
-	s.cfg.Path = tmpfile.Name()
-	err = doMain(s.cfg)
-	c.Assert(err, IsNil)
+		// User active status switch
+		for idx := range records {
+			records[idx].Active = !records[idx].Active
+		}
+		tmpfile, err = MakeTempCSVFile(RecordsToStrings(records))
+		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)
-	for _, tc := range testCases {
-		var foundUser arvados.User
-		for _, user := range users {
-			if user.Email == tc.UserID {
-				foundUser = user
-				break
+		users, err = ListUsers(s.cfg.Client)
+		c.Assert(err, IsNil)
+		for _, r := range records {
+			var foundUser arvados.User
+			for _, user := range users {
+				uID, err := GetUserID(user, s.cfg.UserID)
+				c.Assert(err, IsNil)
+				if uID == r.UserID {
+					foundUser = user
+					break
+				}
 			}
+			c.Assert(foundUser, NotNil)
+			c.Logf("Checking update for user %q", r.UserID)
+			c.Assert(foundUser.FirstName, Equals, r.FirstName)
+			c.Assert(foundUser.LastName, Equals, r.LastName)
+			c.Assert(foundUser.IsActive, Equals, r.Active)
+			c.Assert(foundUser.IsAdmin, Equals, (r.Active && r.Admin))
 		}
-		c.Assert(foundUser, NotNil)
-		c.Logf("Checking recently deactivated user %q", foundUser.Email)
-		c.Assert(foundUser.FirstName, Equals, tc.FirstName)
-		c.Assert(foundUser.LastName, Equals, tc.LastName)
-		c.Assert(foundUser.IsActive, Equals, false)
-		c.Assert(foundUser.IsAdmin, Equals, false) // inactive users cannot be admins
 	}
 }
 

commit 100710416212448cf653a260f9a67933a30656cf
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Jul 1 15:16:30 2022 -0300

    18858: Tidies up testing code.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users_test.go b/tools/sync-users/sync-users_test.go
index 0b6ef6c74..8b93d32e4 100644
--- a/tools/sync-users/sync-users_test.go
+++ b/tools/sync-users/sync-users_test.go
@@ -71,6 +71,20 @@ func MakeTempCSVFile(data [][]string) (f *os.File, err error) {
 	return
 }
 
+// RecordsToStrings formats the input data suitable for MakeTempCSVFile
+func RecordsToStrings(records []userRecord) [][]string {
+	data := [][]string{}
+	for _, u := range records {
+		data = append(data, []string{
+			u.UserID,
+			u.FirstName,
+			u.LastName,
+			fmt.Sprintf("%t", u.Active),
+			fmt.Sprintf("%t", u.Admin)})
+	}
+	return data
+}
+
 func ListUsers(ac *arvados.Client) ([]arvados.User, error) {
 	var ul arvados.UserList
 	err := ac.RequestAndDecode(&ul, "GET", "/arvados/v1/users", nil, arvados.ResourceListParams{})
@@ -206,20 +220,14 @@ func (s *TestSuite) TestWrongDataFields(c *C) {
 
 // Activate and deactivate users
 func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
-	testCases := []struct {
-		Email     string
-		FirstName string
-		LastName  string
-		Active    bool
-		Admin     bool
-	}{{
-		Email:     "user1 at example.com",
+	testCases := []userRecord{{
+		UserID:    "user1 at example.com",
 		FirstName: "Example",
 		LastName:  "User1",
 		Active:    true,
 		Admin:     false,
 	}, {
-		Email:     "admin1 at example.com",
+		UserID:    "admin1 at example.com",
 		FirstName: "Example",
 		LastName:  "Admin1",
 		Active:    true,
@@ -228,15 +236,17 @@ func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
 	// Make sure users aren't already there from fixtures
 	for _, user := range s.users {
 		e := user.Email
-		found := e == testCases[0].Email || e == testCases[1].Email
+		found := false
+		for _, r := range testCases {
+			if e == r.UserID {
+				found = true
+				break
+			}
+		}
 		c.Assert(found, Equals, false)
 	}
 	// User creation
-	data := [][]string{
-		{testCases[0].Email, testCases[0].FirstName, testCases[0].LastName, fmt.Sprintf("%t", testCases[0].Active), fmt.Sprintf("%t", testCases[0].Admin)},
-		{testCases[1].Email, testCases[1].FirstName, testCases[1].LastName, fmt.Sprintf("%t", testCases[1].Active), fmt.Sprintf("%t", testCases[1].Admin)},
-	}
-	tmpfile, err := MakeTempCSVFile(data)
+	tmpfile, err := MakeTempCSVFile(RecordsToStrings(testCases))
 	c.Assert(err, IsNil)
 	defer os.Remove(tmpfile.Name())
 	s.cfg.Path = tmpfile.Name()
@@ -248,7 +258,7 @@ func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
 	for _, tc := range testCases {
 		var foundUser arvados.User
 		for _, user := range users {
-			if user.Email == tc.Email {
+			if user.Email == tc.UserID {
 				foundUser = user
 				break
 			}
@@ -261,13 +271,10 @@ func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
 		c.Assert(foundUser.IsAdmin, Equals, tc.Admin)
 	}
 	// User deactivation
-	testCases[0].Active = false
-	testCases[1].Active = false
-	data = [][]string{
-		{testCases[0].Email, testCases[0].FirstName, testCases[0].LastName, fmt.Sprintf("%t", testCases[0].Active), fmt.Sprintf("%t", testCases[0].Admin)},
-		{testCases[1].Email, testCases[1].FirstName, testCases[1].LastName, fmt.Sprintf("%t", testCases[1].Active), fmt.Sprintf("%t", testCases[1].Admin)},
+	for idx := range testCases {
+		testCases[idx].Active = false
 	}
-	tmpfile, err = MakeTempCSVFile(data)
+	tmpfile, err = MakeTempCSVFile(RecordsToStrings(testCases))
 	c.Assert(err, IsNil)
 	defer os.Remove(tmpfile.Name())
 	s.cfg.Path = tmpfile.Name()
@@ -279,7 +286,7 @@ func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
 	for _, tc := range testCases {
 		var foundUser arvados.User
 		for _, user := range users {
-			if user.Email == tc.Email {
+			if user.Email == tc.UserID {
 				foundUser = user
 				break
 			}

commit e7f9b76848ba874c777a950fac1b13a88ce485dd
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Jul 1 15:14:17 2022 -0300

    18858: Adds ability to identify users by their usernames.
    
    Also, adds case-insensitive matchig support for usernames, and forces it
    when using emails.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 561eb091d..5cf341036 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -59,21 +59,29 @@ func main() {
 }
 
 type ConfigParams struct {
+	CaseInsensitive    bool
 	Client             *arvados.Client
 	ClusterID          string
 	CurrentUser        arvados.User
 	DeactivateUnlisted bool
 	Path               string
+	UserID             string
 	SysUserUUID        string
 	AnonUserUUID       string
 	Verbose            bool
 }
 
 func ParseFlags(cfg *ConfigParams) error {
+	// Acceptable attributes to identify a user on the CSV file
+	userIDOpts := map[string]bool{
+		"email":    true, // default
+		"username": true,
+	}
+
 	flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
 	flags.Usage = func() {
 		usageStr := `Synchronize remote users into Arvados from a CSV format file with 5 columns:
-  * 1st: E-mail address
+  * 1st: User Identifier (email or username)
   * 2nd: First name
   * 3rd: Last name
   * 4th: Active status (0 or 1)
@@ -84,10 +92,18 @@ func ParseFlags(cfg *ConfigParams) error {
 		flags.PrintDefaults()
 	}
 
+	caseInsensitive := flags.Bool(
+		"case-insensitive",
+		true,
+		"Performs case insensitive matching on user IDs. Always ON whe using 'email' user IDs.")
 	deactivateUnlisted := flags.Bool(
 		"deactivate-unlisted",
 		false,
 		"Deactivate users that are not in the input file.")
+	userID := flags.String(
+		"user-id",
+		"email",
+		"Attribute by which every user is identified. Valid values are: email and username.")
 	verbose := flags.Bool(
 		"verbose",
 		false,
@@ -114,9 +130,22 @@ func ParseFlags(cfg *ConfigParams) error {
 	if *srcPath == "" {
 		return fmt.Errorf("input file path invalid")
 	}
+	if !userIDOpts[*userID] {
+		var options []string
+		for opt := range userIDOpts {
+			options = append(options, opt)
+		}
+		return fmt.Errorf("user ID must be one of: %s", strings.Join(options, ", "))
+	}
+	if *userID == "email" {
+		// Always do case-insensitive email addresses matching
+		*caseInsensitive = true
+	}
 
+	cfg.CaseInsensitive = *caseInsensitive
 	cfg.DeactivateUnlisted = *deactivateUnlisted
 	cfg.Path = *srcPath
+	cfg.UserID = *userID
 	cfg.Verbose = *verbose
 
 	return nil
@@ -164,6 +193,18 @@ func GetConfig() (cfg ConfigParams, err error) {
 	return cfg, nil
 }
 
+// GetUserID returns the correct user id value depending on the selector
+func GetUserID(u arvados.User, idSelector string) (string, error) {
+	switch idSelector {
+	case "email":
+		return u.Email, nil
+	case "username":
+		return u.Username, nil
+	default:
+		return "", fmt.Errorf("cannot identify user by %q selector", idSelector)
+	}
+}
+
 func doMain(cfg *ConfigParams) error {
 	// Try opening the input file early, just in case there's a problem.
 	f, err := os.Open(cfg.Path)
@@ -172,7 +213,14 @@ func doMain(cfg *ConfigParams) error {
 	}
 	defer f.Close()
 
+	iCaseLog := ""
+	if cfg.UserID == "username" && cfg.CaseInsensitive {
+		iCaseLog = " - username matching requested to be case-insensitive"
+	}
+	log.Printf("%s %s started. Using %q as users id%s", os.Args[0], version, cfg.UserID, iCaseLog)
+
 	allUsers := make(map[string]arvados.User)
+	userIDToUUID := make(map[string]string) // Index by email or username
 	dupedEmails := make(map[string][]arvados.User)
 	processedUsers := make(map[string]bool)
 	results, err := GetAll(cfg.Client, "users", arvados.ResourceListParams{}, &UserList{})
@@ -183,6 +231,7 @@ func doMain(cfg *ConfigParams) error {
 	localUserUuidRegex := regexp.MustCompile(fmt.Sprintf("^%s-tpzed-[0-9a-z]{15}$", cfg.ClusterID))
 	for _, item := range results {
 		u := item.(arvados.User)
+
 		// Remote user check
 		if !localUserUuidRegex.MatchString(u.UUID) {
 			if cfg.Verbose {
@@ -190,21 +239,38 @@ func doMain(cfg *ConfigParams) error {
 			}
 			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
+
+		// Duplicated user id check
+		uID, err := GetUserID(u, cfg.UserID)
+		if err != nil {
+			return err
 		}
-		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
+		if uID == "" {
+			return fmt.Errorf("%s is empty for user with uuid %q", cfg.UserID, u.UUID)
 		}
-		allUsers[email] = u
-		processedUsers[email] = false
+		if cfg.CaseInsensitive {
+			uID = strings.ToLower(uID)
+		}
+		if alreadySeenUUID, found := userIDToUUID[uID]; found {
+			if cfg.UserID == "username" && uID != "" {
+				return fmt.Errorf("case insensitive collision for username %q between %q and %q", uID, u.UUID, alreadySeenUUID)
+			} else if cfg.UserID == "email" && uID != "" {
+				log.Printf("Duplicated email %q found in user %s - ignoring", uID, u.UUID)
+				if len(dupedEmails[uID]) == 0 {
+					dupedEmails[uID] = []arvados.User{allUsers[alreadySeenUUID]}
+				}
+				dupedEmails[uID] = append(dupedEmails[uID], u)
+				delete(allUsers, alreadySeenUUID) // Skip even the first occurrence,
+				// for security purposes.
+				continue
+			}
+		}
+		if cfg.Verbose {
+			log.Printf("Seen user %q (%s)", uID, u.UUID)
+		}
+		userIDToUUID[uID] = u.UUID
+		allUsers[u.UUID] = u
+		processedUsers[u.UUID] = false
 	}
 
 	loadedRecords, err := LoadInputFile(f)
@@ -218,38 +284,42 @@ func doMain(cfg *ConfigParams) error {
 	updatesSkipped := map[string]bool{}
 
 	for _, record := range loadedRecords {
-		processedUsers[record.Email] = true
-		if record.Email == cfg.CurrentUser.Email {
-			updatesSkipped[record.Email] = true
-			log.Printf("Skipping current user %q (%s) from processing", record.Email, cfg.CurrentUser.UUID)
+		if cfg.CaseInsensitive {
+			record.UserID = strings.ToLower(record.UserID)
+		}
+		recordUUID := userIDToUUID[record.UserID]
+		processedUsers[recordUUID] = true
+		if cfg.UserID == "email" && record.UserID == cfg.CurrentUser.Email {
+			updatesSkipped[recordUUID] = true
+			log.Printf("Skipping current user %q (%s) from processing", record.UserID, cfg.CurrentUser.UUID)
 			continue
 		}
-		if updated, err := ProcessRecord(cfg, record, allUsers); err != nil {
-			log.Printf("error processing record %q: %s", record.Email, err)
-			updatesFailed[record.Email] = true
+		if updated, err := ProcessRecord(cfg, record, userIDToUUID, allUsers); err != nil {
+			log.Printf("error processing record %q: %s", record.UserID, err)
+			updatesFailed[recordUUID] = true
 		} else if updated {
-			updatesSucceeded[record.Email] = true
+			updatesSucceeded[recordUUID] = true
 		}
 	}
 
 	if cfg.DeactivateUnlisted {
-		for email, user := range allUsers {
+		for userUUID, user := range allUsers {
 			if shouldSkip(cfg, user) {
-				updatesSkipped[email] = true
+				updatesSkipped[userUUID] = true
 				log.Printf("Skipping unlisted user %q (%s) from deactivating", user.Email, user.UUID)
 				continue
 			}
-			if !processedUsers[email] && allUsers[email].IsActive {
+			if !processedUsers[userUUID] && allUsers[userUUID].IsActive {
 				if cfg.Verbose {
-					log.Printf("Deactivating unlisted user %q (%s)", user.Email, user.UUID)
+					log.Printf("Deactivating unlisted user %q (%s)", user.Username, user.UUID)
 				}
 				var updatedUser arvados.User
 				if err := UnsetupUser(cfg.Client, user.UUID, &updatedUser); err != nil {
 					log.Printf("error deactivating unlisted user %q: %s", user.UUID, err)
-					updatesFailed[email] = true
+					updatesFailed[userUUID] = true
 				} else {
-					allUsers[email] = updatedUser
-					updatesSucceeded[email] = true
+					allUsers[userUUID] = updatedUser
+					updatesSucceeded[userUUID] = true
 				}
 			}
 		}
@@ -282,7 +352,7 @@ func shouldSkip(cfg *ConfigParams, user arvados.User) bool {
 }
 
 type userRecord struct {
-	Email     string
+	UserID    string
 	FirstName string
 	LastName  string
 	Active    bool
@@ -290,9 +360,9 @@ type userRecord struct {
 }
 
 // ProcessRecord creates or updates a user based on the given record
-func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arvados.User) (bool, error) {
+func ProcessRecord(cfg *ConfigParams, record userRecord, userIDToUUID map[string]string, allUsers map[string]arvados.User) (bool, error) {
 	if cfg.Verbose {
-		log.Printf("Processing record for user %q", record.Email)
+		log.Printf("Processing record for user %q", record.UserID)
 	}
 
 	wantedActiveStatus := strconv.FormatBool(record.Active)
@@ -301,28 +371,29 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arv
 	updateRequired := false
 	// Check if user exists, set its active & admin status.
 	var user arvados.User
-	user, ok := allUsers[record.Email]
+	recordUUID := userIDToUUID[record.UserID]
+	user, ok := allUsers[recordUUID]
 	if !ok {
 		if cfg.Verbose {
-			log.Printf("User %q does not exist, creating", record.Email)
+			log.Printf("User %q does not exist, creating", record.UserID)
 		}
 		createRequired = true
 		err := CreateUser(cfg.Client, &user, map[string]string{
-			"email":      record.Email,
+			cfg.UserID:   record.UserID,
 			"first_name": record.FirstName,
 			"last_name":  record.LastName,
 			"is_active":  wantedActiveStatus,
 			"is_admin":   wantedAdminStatus,
 		})
 		if err != nil {
-			return false, fmt.Errorf("error creating user %q: %s", record.Email, err)
+			return false, fmt.Errorf("error creating user %q: %s", record.UserID, err)
 		}
 	}
 	if record.Active != user.IsActive {
 		updateRequired = true
 		if record.Active {
 			if cfg.Verbose {
-				log.Printf("User %q is inactive, activating", record.Email)
+				log.Printf("User %q is inactive, activating", record.UserID)
 			}
 			// Here we assume the 'setup' is done elsewhere if needed.
 			err := UpdateUser(cfg.Client, user.UUID, &user, map[string]string{
@@ -330,37 +401,37 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arv
 				"is_admin":  wantedAdminStatus, // Just in case it needs to be changed.
 			})
 			if err != nil {
-				return false, fmt.Errorf("error updating user %q: %s", record.Email, err)
+				return false, fmt.Errorf("error updating user %q: %s", record.UserID, err)
 			}
 		} else {
 			if cfg.Verbose {
-				log.Printf("User %q is active, deactivating", record.Email)
+				log.Printf("User %q is active, deactivating", record.UserID)
 			}
 			err := UnsetupUser(cfg.Client, user.UUID, &user)
 			if err != nil {
-				return false, fmt.Errorf("error deactivating user %q: %s", record.Email, err)
+				return false, fmt.Errorf("error deactivating user %q: %s", record.UserID, err)
 			}
 		}
 	}
 	// Inactive users cannot be admins.
 	if user.IsActive && record.Admin != user.IsAdmin {
 		if cfg.Verbose {
-			log.Printf("User %q is active, changing admin status to %v", record.Email, record.Admin)
+			log.Printf("User %q is active, changing admin status to %v", record.UserID, record.Admin)
 		}
 		updateRequired = true
 		err := UpdateUser(cfg.Client, user.UUID, &user, map[string]string{
 			"is_admin": wantedAdminStatus,
 		})
 		if err != nil {
-			return false, fmt.Errorf("error updating user %q: %s", record.Email, err)
+			return false, fmt.Errorf("error updating user %q: %s", record.UserID, err)
 		}
 	}
-	allUsers[record.Email] = user
+	allUsers[record.UserID] = user
 	if createRequired {
-		log.Printf("Created user %q", record.Email)
+		log.Printf("Created user %q", record.UserID)
 	}
 	if updateRequired {
-		log.Printf("Updated user %q", record.Email)
+		log.Printf("Updated user %q", record.UserID)
 	}
 
 	return createRequired || updateRequired, nil
@@ -386,12 +457,12 @@ func LoadInputFile(f *os.File) (loadedRecords []userRecord, err error) {
 			err = fmt.Errorf("parsing error at line %d: expected 5 fields, found %d", lineNo, len(record))
 			return
 		}
-		email := strings.ToLower(strings.TrimSpace(record[0]))
+		userID := strings.ToLower(strings.TrimSpace(record[0]))
 		firstName := strings.TrimSpace(record[1])
 		lastName := strings.TrimSpace(record[2])
 		active := strings.TrimSpace(record[3])
 		admin := strings.TrimSpace(record[4])
-		if email == "" || firstName == "" || lastName == "" || active == "" || admin == "" {
+		if userID == "" || firstName == "" || lastName == "" || active == "" || admin == "" {
 			err = fmt.Errorf("parsing error at line %d: fields cannot be empty", lineNo)
 			return
 		}
@@ -404,7 +475,7 @@ func LoadInputFile(f *os.File) (loadedRecords []userRecord, err error) {
 			return nil, fmt.Errorf("parsing error at line %d: admin status not recognized", lineNo)
 		}
 		loadedRecords = append(loadedRecords, userRecord{
-			Email:     email,
+			UserID:    userID,
 			FirstName: firstName,
 			LastName:  lastName,
 			Active:    activeBool,

commit 795a29f42c333ee369e852977e7018fe4c5c94ae
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Jun 24 14:29:05 2022 -0300

    18858: Adds duplicated email check & tests.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 25c759456..561eb091d 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 6d10595d4..0b6ef6c74 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 at 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 at 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 at 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.*")
+}

commit 6eb56e41997fe179d6aae140700c307a09d5b703
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Jun 24 11:02:25 2022 -0300

    18858: Fixes fixture to avoid having duplicated emails in user accounts.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 495149dc8..1cf79b1a5 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -171,7 +171,7 @@ spectator:
 container_runtime_token_user:
   owner_uuid: zzzzz-tpzed-000000000000000
   uuid: zzzzz-tpzed-l3skomkti0c4vg4
-  email: spectator at arvados.local
+  email: container_runtime_token_user at arvados.local
   first_name: Spect
   last_name: Ator
   identity_url: https://container_runtime_token_user.openid.local
@@ -278,7 +278,7 @@ active_no_prefs:
 active_no_prefs_profile_no_getting_started_shown:
   owner_uuid: zzzzz-tpzed-000000000000000
   uuid: zzzzz-tpzed-a46c98d1td4aoj4
-  email: active_no_prefs_profile at arvados.local
+  email: active_no_prefs_profile_no_gs at arvados.local
   first_name: HasPrefs
   last_name: NoProfile
   identity_url: https://active_no_prefs_profile.openid.local
@@ -372,7 +372,7 @@ fuse:
 permission_perftest:
   owner_uuid: zzzzz-tpzed-000000000000000
   uuid: zzzzz-tpzed-permissionptest
-  email: fuse at arvados.local
+  email: permission_perftest at arvados.local
   first_name: FUSE
   last_name: User
   identity_url: https://permission_perftest.openid.local

commit 4aaa7415151412b97b706a442d0487c15b39c495
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Jun 24 10:08:54 2022 -0300

    18858: Initial set of tests.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users_test.go b/tools/sync-users/sync-users_test.go
new file mode 100644
index 000000000..6d10595d4
--- /dev/null
+++ b/tools/sync-users/sync-users_test.go
@@ -0,0 +1,293 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package main
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"strings"
+	"testing"
+
+	"git.arvados.org/arvados.git/sdk/go/arvados"
+	. "gopkg.in/check.v1"
+)
+
+// Gocheck boilerplate
+func Test(t *testing.T) {
+	TestingT(t)
+}
+
+type TestSuite struct {
+	cfg   *ConfigParams
+	ac    *arvados.Client
+	users map[string]arvados.User
+}
+
+func (s *TestSuite) SetUpTest(c *C) {
+	s.ac = arvados.NewClientFromEnv()
+	u, err := s.ac.CurrentUser()
+	c.Assert(err, IsNil)
+	c.Assert(u.IsAdmin, Equals, true)
+
+	s.users = make(map[string]arvados.User)
+	ul := arvados.UserList{}
+	s.ac.RequestAndDecode(&ul, "GET", "/arvados/v1/users", nil, arvados.ResourceListParams{})
+	c.Assert(ul.ItemsAvailable, Not(Equals), 0)
+	s.users = make(map[string]arvados.User)
+	for _, u := range ul.Items {
+		s.users[u.UUID] = u
+	}
+
+	// Set up command config
+	os.Args = []string{"cmd", "somefile.csv"}
+	config, err := GetConfig()
+	c.Assert(err, IsNil)
+	s.cfg = &config
+}
+
+func (s *TestSuite) TearDownTest(c *C) {
+	var dst interface{}
+	// Reset database to fixture state after every test run.
+	err := s.cfg.Client.RequestAndDecode(&dst, "POST", "/database/reset", nil, nil)
+	c.Assert(err, IsNil)
+}
+
+var _ = Suite(&TestSuite{})
+
+// MakeTempCSVFile creates a temp file with data as comma separated values
+func MakeTempCSVFile(data [][]string) (f *os.File, err error) {
+	f, err = ioutil.TempFile("", "test_sync_users")
+	if err != nil {
+		return
+	}
+	for _, line := range data {
+		fmt.Fprintf(f, "%s\n", strings.Join(line, ","))
+	}
+	err = f.Close()
+	return
+}
+
+func ListUsers(ac *arvados.Client) ([]arvados.User, error) {
+	var ul arvados.UserList
+	err := ac.RequestAndDecode(&ul, "GET", "/arvados/v1/users", nil, arvados.ResourceListParams{})
+	if err != nil {
+		return nil, err
+	}
+	return ul.Items, nil
+}
+
+func (s *TestSuite) TestParseFlagsWithoutPositionalArgument(c *C) {
+	os.Args = []string{"cmd", "-verbose"}
+	err := ParseFlags(&ConfigParams{})
+	c.Assert(err, NotNil)
+}
+
+func (s *TestSuite) TestParseFlagsWithPositionalArgument(c *C) {
+	cfg := ConfigParams{}
+	os.Args = []string{"cmd", "/tmp/somefile.csv"}
+	err := ParseFlags(&cfg)
+	c.Assert(err, IsNil)
+	c.Assert(cfg.Path, Equals, "/tmp/somefile.csv")
+	c.Assert(cfg.Verbose, Equals, false)
+	c.Assert(cfg.DeactivateUnlisted, Equals, false)
+}
+
+func (s *TestSuite) TestParseFlagsWithOptionalFlags(c *C) {
+	cfg := ConfigParams{}
+	os.Args = []string{"cmd", "-verbose", "-deactivate-unlisted", "/tmp/somefile.csv"}
+	err := ParseFlags(&cfg)
+	c.Assert(err, IsNil)
+	c.Assert(cfg.Path, Equals, "/tmp/somefile.csv")
+	c.Assert(cfg.Verbose, Equals, true)
+	c.Assert(cfg.DeactivateUnlisted, Equals, true)
+}
+
+func (s *TestSuite) TestGetConfig(c *C) {
+	os.Args = []string{"cmd", "/tmp/somefile.csv"}
+	cfg, err := GetConfig()
+	c.Assert(err, IsNil)
+	c.Assert(cfg.AnonUserUUID, Not(Equals), "")
+	c.Assert(cfg.SysUserUUID, Not(Equals), "")
+	c.Assert(cfg.CurrentUser, Not(Equals), "")
+	c.Assert(cfg.ClusterID, Not(Equals), "")
+	c.Assert(cfg.Client, NotNil)
+}
+
+func (s *TestSuite) TestFailOnEmptyFields(c *C) {
+	records := [][]string{
+		{"", "first-name", "last-name", "1", "0"},
+		{"user at example", "", "last-name", "1", "0"},
+		{"user at example", "first-name", "", "1", "0"},
+		{"user at example", "first-name", "last-name", "", "0"},
+		{"user at example", "first-name", "last-name", "1", ""},
+	}
+	for _, record := range records {
+		data := [][]string{record}
+		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, ".*fields cannot be empty.*")
+	}
+}
+
+func (s *TestSuite) TestIgnoreSpaces(c *C) {
+	// Make sure users aren't already there from fixtures
+	for _, user := range s.users {
+		e := user.Email
+		found := e == "user1 at example.com" || e == "user2 at example.com" || e == "user3 at example.com"
+		c.Assert(found, Equals, false)
+	}
+	// Use CSV data with leading/trailing whitespaces, confirm that they get ignored
+	data := [][]string{
+		{" user1 at example.com", "  Example", "   User1", "1", "0"},
+		{"user2 at example.com ", "Example  ", "User2   ", "1", "0"},
+		{" user3 at example.com ", "  Example  ", "   User3   ", "1", "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)
+	for _, userNr := range []int{1, 2, 3} {
+		found := false
+		for _, user := range users {
+			if user.Email == fmt.Sprintf("user%d at example.com", userNr) &&
+				user.LastName == fmt.Sprintf("User%d", userNr) &&
+				user.FirstName == "Example" && user.IsActive == true {
+				found = true
+				break
+			}
+		}
+		c.Assert(found, Equals, true)
+	}
+}
+
+// Error out when records have != 5 records
+func (s *TestSuite) TestWrongNumberOfFields(c *C) {
+	for _, testCase := range [][][]string{
+		{{"user1 at example.com", "Example", "User1", "1"}},
+		{{"user1 at example.com", "Example", "User1", "1", "0", "extra data"}},
+	} {
+		tmpfile, err := MakeTempCSVFile(testCase)
+		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, ".*expected 5 fields, found.*")
+	}
+}
+
+// Error out when records have incorrect data types
+func (s *TestSuite) TestWrongDataFields(c *C) {
+	for _, testCase := range [][][]string{
+		{{"user1 at example.com", "Example", "User1", "yep", "0"}},
+		{{"user1 at example.com", "Example", "User1", "1", "nope"}},
+	} {
+		tmpfile, err := MakeTempCSVFile(testCase)
+		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, ".*parsing error at line.*[active|admin] status not recognized.*")
+	}
+}
+
+// Activate and deactivate users
+func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
+	testCases := []struct {
+		Email     string
+		FirstName string
+		LastName  string
+		Active    bool
+		Admin     bool
+	}{{
+		Email:     "user1 at example.com",
+		FirstName: "Example",
+		LastName:  "User1",
+		Active:    true,
+		Admin:     false,
+	}, {
+		Email:     "admin1 at example.com",
+		FirstName: "Example",
+		LastName:  "Admin1",
+		Active:    true,
+		Admin:     true,
+	}}
+	// Make sure users aren't already there from fixtures
+	for _, user := range s.users {
+		e := user.Email
+		found := e == testCases[0].Email || e == testCases[1].Email
+		c.Assert(found, Equals, false)
+	}
+	// User creation
+	data := [][]string{
+		{testCases[0].Email, testCases[0].FirstName, testCases[0].LastName, fmt.Sprintf("%t", testCases[0].Active), fmt.Sprintf("%t", testCases[0].Admin)},
+		{testCases[1].Email, testCases[1].FirstName, testCases[1].LastName, fmt.Sprintf("%t", testCases[1].Active), fmt.Sprintf("%t", testCases[1].Admin)},
+	}
+	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)
+	for _, tc := range testCases {
+		var foundUser arvados.User
+		for _, user := range users {
+			if user.Email == tc.Email {
+				foundUser = user
+				break
+			}
+		}
+		c.Assert(foundUser, NotNil)
+		c.Logf("Checking recently created user %q", foundUser.Email)
+		c.Assert(foundUser.FirstName, Equals, tc.FirstName)
+		c.Assert(foundUser.LastName, Equals, tc.LastName)
+		c.Assert(foundUser.IsActive, Equals, true)
+		c.Assert(foundUser.IsAdmin, Equals, tc.Admin)
+	}
+	// User deactivation
+	testCases[0].Active = false
+	testCases[1].Active = false
+	data = [][]string{
+		{testCases[0].Email, testCases[0].FirstName, testCases[0].LastName, fmt.Sprintf("%t", testCases[0].Active), fmt.Sprintf("%t", testCases[0].Admin)},
+		{testCases[1].Email, testCases[1].FirstName, testCases[1].LastName, fmt.Sprintf("%t", testCases[1].Active), fmt.Sprintf("%t", testCases[1].Admin)},
+	}
+	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)
+	for _, tc := range testCases {
+		var foundUser arvados.User
+		for _, user := range users {
+			if user.Email == tc.Email {
+				foundUser = user
+				break
+			}
+		}
+		c.Assert(foundUser, NotNil)
+		c.Logf("Checking recently deactivated user %q", foundUser.Email)
+		c.Assert(foundUser.FirstName, Equals, tc.FirstName)
+		c.Assert(foundUser.LastName, Equals, tc.LastName)
+		c.Assert(foundUser.IsActive, Equals, false)
+		c.Assert(foundUser.IsAdmin, Equals, false) // inactive users cannot be admins
+	}
+}

commit 8f69a16eb56dfea60b4abafb4e73a45d19c01476
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Thu Jun 23 10:03:57 2022 -0300

    18858: Enhances logging a bit.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 969e101fe..25c759456 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -198,7 +198,7 @@ func doMain(cfg *ConfigParams) error {
 		processedUsers[record.Email] = true
 		if record.Email == cfg.CurrentUser.Email {
 			updatesSkipped[record.Email] = true
-			log.Printf("Skipping current user %q from processing", record.Email)
+			log.Printf("Skipping current user %q (%s) from processing", record.Email, cfg.CurrentUser.UUID)
 			continue
 		}
 		if updated, err := ProcessRecord(cfg, record, allUsers); err != nil {
@@ -213,6 +213,7 @@ func doMain(cfg *ConfigParams) error {
 		for email, user := range allUsers {
 			if shouldSkip(cfg, user) {
 				updatesSkipped[email] = true
+				log.Printf("Skipping unlisted user %q (%s) from deactivating", user.Email, user.UUID)
 				continue
 			}
 			if !processedUsers[email] && allUsers[email].IsActive {

commit a229aa70a28a4f46fb9ba71bf038655634bcf2d4
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Thu Jun 23 09:22:53 2022 -0300

    18858: Fixes user fixture: usernames can only be alphanums.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 14630d9ef..495149dc8 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -431,4 +431,4 @@ has_can_login_permission:
   is_active: true
   is_admin: false
   modified_at: 2015-03-26 12:34:56.789000000 Z
-  username: can-login-user
+  username: canLoginUser

commit f699425ce9bd7b5055192e81de31cc10609ba41d
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Tue Jun 21 11:16:08 2022 -0300

    18858: Fixes unlisted account deactivation logic. Improves logging.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 23f111817..969e101fe 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -190,33 +190,29 @@ func doMain(cfg *ConfigParams) error {
 	}
 	log.Printf("Loaded %d records from input file", len(loadedRecords))
 
-	updatesSucceeded, updatesFailed := 0, 0
+	updatesSucceeded := map[string]bool{}
+	updatesFailed := map[string]bool{}
+	updatesSkipped := map[string]bool{}
+
 	for _, record := range loadedRecords {
+		processedUsers[record.Email] = true
 		if record.Email == cfg.CurrentUser.Email {
+			updatesSkipped[record.Email] = true
 			log.Printf("Skipping current user %q from processing", record.Email)
 			continue
 		}
 		if updated, err := ProcessRecord(cfg, record, allUsers); err != nil {
 			log.Printf("error processing record %q: %s", record.Email, err)
-			updatesFailed++
+			updatesFailed[record.Email] = true
 		} else if updated {
-			processedUsers[strings.ToLower(record.Email)] = true
-			updatesSucceeded++
+			updatesSucceeded[record.Email] = true
 		}
 	}
 
 	if cfg.DeactivateUnlisted {
 		for email, user := range allUsers {
-			switch user.UUID {
-			case cfg.SysUserUUID, cfg.AnonUserUUID:
-				if cfg.Verbose {
-					log.Printf("Skipping system user deactivation: %s", user.UUID)
-				}
-				continue
-			case cfg.CurrentUser.UUID:
-				if cfg.Verbose {
-					log.Printf("Skipping current user deactivation: %s (%s)", user.Email, user.UUID)
-				}
+			if shouldSkip(cfg, user) {
+				updatesSkipped[email] = true
 				continue
 			}
 			if !processedUsers[email] && allUsers[email].IsActive {
@@ -226,20 +222,30 @@ func doMain(cfg *ConfigParams) error {
 				var updatedUser arvados.User
 				if err := UnsetupUser(cfg.Client, user.UUID, &updatedUser); err != nil {
 					log.Printf("error deactivating unlisted user %q: %s", user.UUID, err)
-					updatesFailed++
+					updatesFailed[email] = true
 				} else {
 					allUsers[email] = updatedUser
-					updatesSucceeded++
+					updatesSucceeded[email] = true
 				}
 			}
 		}
 	}
 
-	log.Printf("Updated %d user(s), failed to update %d user(s)", updatesSucceeded, updatesFailed)
+	log.Printf("User update successes: %d, skips: %d, failures: %d", len(updatesSucceeded), len(updatesSkipped), len(updatesFailed))
 
 	return nil
 }
 
+func shouldSkip(cfg *ConfigParams, user arvados.User) bool {
+	switch user.UUID {
+	case cfg.SysUserUUID, cfg.AnonUserUUID:
+		return true
+	case cfg.CurrentUser.UUID:
+		return true
+	}
+	return false
+}
+
 type userRecord struct {
 	Email     string
 	FirstName string

commit 470641e02dc18aeba8506bd3479511db13492e25
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Jun 17 16:35:15 2022 -0300

    18858: Adds additional checks, improves logging.
    
    * Checks that if there's a LoginCluster federation, the current cluster
      isn't a satellite.
    * Skips system users when called with -deactivate-unlisted option.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 7cb55f74e..23f111817 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -59,9 +59,12 @@ func main() {
 
 type ConfigParams struct {
 	Client             *arvados.Client
+	ClusterID          string
 	CurrentUser        arvados.User
 	DeactivateUnlisted bool
 	Path               string
+	SysUserUUID        string
+	AnonUserUUID       string
 	Verbose            bool
 }
 
@@ -136,10 +139,27 @@ func GetConfig() (cfg ConfigParams, err error) {
 		return cfg, fmt.Errorf("current user %q is not an admin user", u.UUID)
 	}
 	if cfg.Verbose {
-		log.Printf("Running as admin user %q", u.UUID)
+		log.Printf("Running as admin user %q (%s)", u.Email, u.UUID)
 	}
 	cfg.CurrentUser = u
 
+	var ac struct {
+		ClusterID string
+		Login     struct {
+			LoginCluster string
+		}
+	}
+	err = cfg.Client.RequestAndDecode(&ac, "GET", "arvados/v1/config", nil, nil)
+	if err != nil {
+		return cfg, fmt.Errorf("error getting the exported config: %s", err)
+	}
+	if ac.Login.LoginCluster != "" && ac.Login.LoginCluster != ac.ClusterID {
+		return cfg, fmt.Errorf("cannot run on a cluster other than the login cluster")
+	}
+	cfg.SysUserUUID = ac.ClusterID + "-tpzed-000000000000000"
+	cfg.AnonUserUUID = ac.ClusterID + "-tpzed-anonymouspublic"
+	cfg.ClusterID = ac.ClusterID
+
 	return cfg, nil
 }
 
@@ -157,7 +177,7 @@ func doMain(cfg *ConfigParams) error {
 	if err != nil {
 		return fmt.Errorf("error getting all users: %s", err)
 	}
-	log.Printf("Found %d users", len(results))
+	log.Printf("Found %d users in cluster %q", len(results), cfg.ClusterID)
 	for _, item := range results {
 		u := item.(arvados.User)
 		allUsers[strings.ToLower(u.Email)] = u
@@ -187,13 +207,21 @@ func doMain(cfg *ConfigParams) error {
 
 	if cfg.DeactivateUnlisted {
 		for email, user := range allUsers {
-			if user.UUID == cfg.CurrentUser.UUID {
-				log.Printf("Skipping current user deactivation: %s", user.UUID)
+			switch user.UUID {
+			case cfg.SysUserUUID, cfg.AnonUserUUID:
+				if cfg.Verbose {
+					log.Printf("Skipping system user deactivation: %s", user.UUID)
+				}
+				continue
+			case cfg.CurrentUser.UUID:
+				if cfg.Verbose {
+					log.Printf("Skipping current user deactivation: %s (%s)", user.Email, user.UUID)
+				}
 				continue
 			}
-			if !processedUsers[email] {
+			if !processedUsers[email] && allUsers[email].IsActive {
 				if cfg.Verbose {
-					log.Printf("Deactivating unlisted user %q", user.UUID)
+					log.Printf("Deactivating unlisted user %q (%s)", user.Email, user.UUID)
 				}
 				var updatedUser arvados.User
 				if err := UnsetupUser(cfg.Client, user.UUID, &updatedUser); err != nil {
@@ -242,8 +270,8 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arv
 			"email":      record.Email,
 			"first_name": record.FirstName,
 			"last_name":  record.LastName,
-			"is_active":  strconv.FormatBool(record.Active),
-			"is_admin":   strconv.FormatBool(record.Admin),
+			"is_active":  wantedActiveStatus,
+			"is_admin":   wantedAdminStatus,
 		})
 		if err != nil {
 			return false, fmt.Errorf("error creating user %q: %s", record.Email, err)

commit 4e355db55f12be1944a1e21b9f386e6bc101dcde
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Jun 17 15:58:38 2022 -0300

    18858: Avoids updating the current user. Adds unlisted user disable option.
    
    Also, adds verbose logging.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/.gitignore b/tools/sync-users/.gitignore
new file mode 100644
index 000000000..cbbc17612
--- /dev/null
+++ b/tools/sync-users/.gitignore
@@ -0,0 +1 @@
+sync-users
\ No newline at end of file
diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 28d7e7373..7cb55f74e 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -58,9 +58,11 @@ func main() {
 }
 
 type ConfigParams struct {
-	Path    string
-	Verbose bool
-	Client  *arvados.Client
+	Client             *arvados.Client
+	CurrentUser        arvados.User
+	DeactivateUnlisted bool
+	Path               string
+	Verbose            bool
 }
 
 func ParseFlags(cfg *ConfigParams) error {
@@ -78,6 +80,10 @@ func ParseFlags(cfg *ConfigParams) error {
 		flags.PrintDefaults()
 	}
 
+	deactivateUnlisted := flags.Bool(
+		"deactivate-unlisted",
+		false,
+		"Deactivate users that are not in the input file.")
 	verbose := flags.Bool(
 		"verbose",
 		false,
@@ -105,6 +111,7 @@ func ParseFlags(cfg *ConfigParams) error {
 		return fmt.Errorf("input file path invalid")
 	}
 
+	cfg.DeactivateUnlisted = *deactivateUnlisted
 	cfg.Path = *srcPath
 	cfg.Verbose = *verbose
 
@@ -126,8 +133,12 @@ func GetConfig() (cfg ConfigParams, err error) {
 		return cfg, fmt.Errorf("error getting the current user: %s", err)
 	}
 	if !u.IsAdmin {
-		return cfg, fmt.Errorf("current user (%s) is not an admin user", u.UUID)
+		return cfg, fmt.Errorf("current user %q is not an admin user", u.UUID)
+	}
+	if cfg.Verbose {
+		log.Printf("Running as admin user %q", u.UUID)
 	}
+	cfg.CurrentUser = u
 
 	return cfg, nil
 }
@@ -141,6 +152,7 @@ func doMain(cfg *ConfigParams) error {
 	defer f.Close()
 
 	allUsers := 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)
@@ -149,6 +161,7 @@ func doMain(cfg *ConfigParams) error {
 	for _, item := range results {
 		u := item.(arvados.User)
 		allUsers[strings.ToLower(u.Email)] = u
+		processedUsers[strings.ToLower(u.Email)] = false
 	}
 
 	loadedRecords, err := LoadInputFile(f)
@@ -159,14 +172,42 @@ func doMain(cfg *ConfigParams) error {
 
 	updatesSucceeded, updatesFailed := 0, 0
 	for _, record := range loadedRecords {
+		if record.Email == cfg.CurrentUser.Email {
+			log.Printf("Skipping current user %q from processing", record.Email)
+			continue
+		}
 		if updated, err := ProcessRecord(cfg, record, allUsers); err != nil {
 			log.Printf("error processing record %q: %s", record.Email, err)
 			updatesFailed++
 		} else if updated {
+			processedUsers[strings.ToLower(record.Email)] = true
 			updatesSucceeded++
 		}
 	}
-	log.Printf("Updated %d account(s), failed to update %d account(s)", updatesSucceeded, updatesFailed)
+
+	if cfg.DeactivateUnlisted {
+		for email, user := range allUsers {
+			if user.UUID == cfg.CurrentUser.UUID {
+				log.Printf("Skipping current user deactivation: %s", user.UUID)
+				continue
+			}
+			if !processedUsers[email] {
+				if cfg.Verbose {
+					log.Printf("Deactivating unlisted user %q", user.UUID)
+				}
+				var updatedUser arvados.User
+				if err := UnsetupUser(cfg.Client, user.UUID, &updatedUser); err != nil {
+					log.Printf("error deactivating unlisted user %q: %s", user.UUID, err)
+					updatesFailed++
+				} else {
+					allUsers[email] = updatedUser
+					updatesSucceeded++
+				}
+			}
+		}
+	}
+
+	log.Printf("Updated %d user(s), failed to update %d user(s)", updatesSucceeded, updatesFailed)
 
 	return nil
 }
@@ -181,13 +222,22 @@ type userRecord struct {
 
 // ProcessRecord creates or updates a user based on the given record
 func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arvados.User) (bool, error) {
+	if cfg.Verbose {
+		log.Printf("Processing record for user %q", record.Email)
+	}
+
 	wantedActiveStatus := strconv.FormatBool(record.Active)
 	wantedAdminStatus := strconv.FormatBool(record.Admin)
+	createRequired := false
 	updateRequired := false
 	// Check if user exists, set its active & admin status.
 	var user arvados.User
 	user, ok := allUsers[record.Email]
 	if !ok {
+		if cfg.Verbose {
+			log.Printf("User %q does not exist, creating", record.Email)
+		}
+		createRequired = true
 		err := CreateUser(cfg.Client, &user, map[string]string{
 			"email":      record.Email,
 			"first_name": record.FirstName,
@@ -198,12 +248,13 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arv
 		if err != nil {
 			return false, fmt.Errorf("error creating user %q: %s", record.Email, err)
 		}
-		updateRequired = true
-		log.Printf("Created user %q", record.Email)
 	}
 	if record.Active != user.IsActive {
 		updateRequired = true
 		if record.Active {
+			if cfg.Verbose {
+				log.Printf("User %q is inactive, activating", record.Email)
+			}
 			// Here we assume the 'setup' is done elsewhere if needed.
 			err := UpdateUser(cfg.Client, user.UUID, &user, map[string]string{
 				"is_active": wantedActiveStatus,
@@ -213,6 +264,9 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arv
 				return false, fmt.Errorf("error updating user %q: %s", record.Email, err)
 			}
 		} else {
+			if cfg.Verbose {
+				log.Printf("User %q is active, deactivating", record.Email)
+			}
 			err := UnsetupUser(cfg.Client, user.UUID, &user)
 			if err != nil {
 				return false, fmt.Errorf("error deactivating user %q: %s", record.Email, err)
@@ -221,6 +275,9 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arv
 	}
 	// Inactive users cannot be admins.
 	if user.IsActive && record.Admin != user.IsAdmin {
+		if cfg.Verbose {
+			log.Printf("User %q is active, changing admin status to %v", record.Email, record.Admin)
+		}
 		updateRequired = true
 		err := UpdateUser(cfg.Client, user.UUID, &user, map[string]string{
 			"is_admin": wantedAdminStatus,
@@ -230,11 +287,14 @@ func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arv
 		}
 	}
 	allUsers[record.Email] = user
+	if createRequired {
+		log.Printf("Created user %q", record.Email)
+	}
 	if updateRequired {
 		log.Printf("Updated user %q", record.Email)
 	}
 
-	return updateRequired, nil
+	return createRequired || updateRequired, nil
 }
 
 // LoadInputFile reads the input file and returns a list of user records

commit e4cbfb51207a7b3430a9cad983b41527bcf4cf5e
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Thu Jun 16 16:07:40 2022 -0300

    18858: Implements basic sync-users tool.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
new file mode 100644
index 000000000..28d7e7373
--- /dev/null
+++ b/tools/sync-users/sync-users.go
@@ -0,0 +1,337 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package main
+
+import (
+	"bytes"
+	"encoding/csv"
+	"encoding/json"
+	"flag"
+	"fmt"
+	"io"
+	"log"
+	"net/url"
+	"os"
+	"strconv"
+	"strings"
+
+	"git.arvados.org/arvados.git/lib/cmd"
+	"git.arvados.org/arvados.git/sdk/go/arvados"
+)
+
+var version = "dev"
+
+type resourceList interface {
+	Len() int
+	GetItems() []interface{}
+}
+
+// UserList implements resourceList interface
+type UserList struct {
+	arvados.UserList
+}
+
+// Len returns the amount of items this list holds
+func (l UserList) Len() int {
+	return len(l.Items)
+}
+
+// GetItems returns the list of items
+func (l UserList) GetItems() (out []interface{}) {
+	for _, item := range l.Items {
+		out = append(out, item)
+	}
+	return
+}
+
+func main() {
+	cfg, err := GetConfig()
+	if err != nil {
+		log.Fatalf("%v", err)
+	}
+
+	if err := doMain(&cfg); err != nil {
+		log.Fatalf("%v", err)
+	}
+}
+
+type ConfigParams struct {
+	Path    string
+	Verbose bool
+	Client  *arvados.Client
+}
+
+func ParseFlags(cfg *ConfigParams) error {
+	flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+	flags.Usage = func() {
+		usageStr := `Synchronize remote users into Arvados from a CSV format file with 5 columns:
+  * 1st: E-mail address
+  * 2nd: First name
+  * 3rd: Last name
+  * 4th: Active status (0 or 1)
+  * 5th: Admin status (0 or 1)`
+		fmt.Fprintf(flags.Output(), "%s\n\n", usageStr)
+		fmt.Fprintf(flags.Output(), "Usage:\n%s [OPTIONS] <input-file.csv>\n\n", os.Args[0])
+		fmt.Fprintf(flags.Output(), "Options:\n")
+		flags.PrintDefaults()
+	}
+
+	verbose := flags.Bool(
+		"verbose",
+		false,
+		"Log informational messages. Off by default.")
+	getVersion := flags.Bool(
+		"version",
+		false,
+		"Print version information and exit.")
+
+	if ok, code := cmd.ParseFlags(flags, os.Args[0], os.Args[1:], "input-file.csv", os.Stderr); !ok {
+		os.Exit(code)
+	} else if *getVersion {
+		fmt.Printf("%s %s\n", os.Args[0], version)
+		os.Exit(0)
+	}
+
+	// Input file as a required positional argument
+	if flags.NArg() == 0 {
+		return fmt.Errorf("please provide a path to an input file")
+	}
+	srcPath := &os.Args[flags.NFlag()+1]
+
+	// Validations
+	if *srcPath == "" {
+		return fmt.Errorf("input file path invalid")
+	}
+
+	cfg.Path = *srcPath
+	cfg.Verbose = *verbose
+
+	return nil
+}
+
+// GetConfig sets up a ConfigParams struct
+func GetConfig() (cfg ConfigParams, err error) {
+	err = ParseFlags(&cfg)
+	if err != nil {
+		return
+	}
+
+	cfg.Client = arvados.NewClientFromEnv()
+
+	// Check current user permissions
+	u, err := cfg.Client.CurrentUser()
+	if err != nil {
+		return cfg, fmt.Errorf("error getting the current user: %s", err)
+	}
+	if !u.IsAdmin {
+		return cfg, fmt.Errorf("current user (%s) is not an admin user", u.UUID)
+	}
+
+	return cfg, nil
+}
+
+func doMain(cfg *ConfigParams) error {
+	// Try opening the input file early, just in case there's a problem.
+	f, err := os.Open(cfg.Path)
+	if err != nil {
+		return fmt.Errorf("error opening input file: %s", err)
+	}
+	defer f.Close()
+
+	allUsers := make(map[string]arvados.User)
+	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", len(results))
+	for _, item := range results {
+		u := item.(arvados.User)
+		allUsers[strings.ToLower(u.Email)] = u
+	}
+
+	loadedRecords, err := LoadInputFile(f)
+	if err != nil {
+		return fmt.Errorf("reading input file %q: %s", cfg.Path, err)
+	}
+	log.Printf("Loaded %d records from input file", len(loadedRecords))
+
+	updatesSucceeded, updatesFailed := 0, 0
+	for _, record := range loadedRecords {
+		if updated, err := ProcessRecord(cfg, record, allUsers); err != nil {
+			log.Printf("error processing record %q: %s", record.Email, err)
+			updatesFailed++
+		} else if updated {
+			updatesSucceeded++
+		}
+	}
+	log.Printf("Updated %d account(s), failed to update %d account(s)", updatesSucceeded, updatesFailed)
+
+	return nil
+}
+
+type userRecord struct {
+	Email     string
+	FirstName string
+	LastName  string
+	Active    bool
+	Admin     bool
+}
+
+// ProcessRecord creates or updates a user based on the given record
+func ProcessRecord(cfg *ConfigParams, record userRecord, allUsers map[string]arvados.User) (bool, error) {
+	wantedActiveStatus := strconv.FormatBool(record.Active)
+	wantedAdminStatus := strconv.FormatBool(record.Admin)
+	updateRequired := false
+	// Check if user exists, set its active & admin status.
+	var user arvados.User
+	user, ok := allUsers[record.Email]
+	if !ok {
+		err := CreateUser(cfg.Client, &user, map[string]string{
+			"email":      record.Email,
+			"first_name": record.FirstName,
+			"last_name":  record.LastName,
+			"is_active":  strconv.FormatBool(record.Active),
+			"is_admin":   strconv.FormatBool(record.Admin),
+		})
+		if err != nil {
+			return false, fmt.Errorf("error creating user %q: %s", record.Email, err)
+		}
+		updateRequired = true
+		log.Printf("Created user %q", record.Email)
+	}
+	if record.Active != user.IsActive {
+		updateRequired = true
+		if record.Active {
+			// Here we assume the 'setup' is done elsewhere if needed.
+			err := UpdateUser(cfg.Client, user.UUID, &user, map[string]string{
+				"is_active": wantedActiveStatus,
+				"is_admin":  wantedAdminStatus, // Just in case it needs to be changed.
+			})
+			if err != nil {
+				return false, fmt.Errorf("error updating user %q: %s", record.Email, err)
+			}
+		} else {
+			err := UnsetupUser(cfg.Client, user.UUID, &user)
+			if err != nil {
+				return false, fmt.Errorf("error deactivating user %q: %s", record.Email, err)
+			}
+		}
+	}
+	// Inactive users cannot be admins.
+	if user.IsActive && record.Admin != user.IsAdmin {
+		updateRequired = true
+		err := UpdateUser(cfg.Client, user.UUID, &user, map[string]string{
+			"is_admin": wantedAdminStatus,
+		})
+		if err != nil {
+			return false, fmt.Errorf("error updating user %q: %s", record.Email, err)
+		}
+	}
+	allUsers[record.Email] = user
+	if updateRequired {
+		log.Printf("Updated user %q", record.Email)
+	}
+
+	return updateRequired, nil
+}
+
+// LoadInputFile reads the input file and returns a list of user records
+func LoadInputFile(f *os.File) (loadedRecords []userRecord, err error) {
+	lineNo := 0
+	csvReader := csv.NewReader(f)
+	loadedRecords = make([]userRecord, 0)
+
+	for {
+		record, e := csvReader.Read()
+		if e == io.EOF {
+			break
+		}
+		lineNo++
+		if e != nil {
+			err = fmt.Errorf("parsing error at line %d: %s", lineNo, e)
+			return
+		}
+		if len(record) != 5 {
+			err = fmt.Errorf("parsing error at line %d: expected 5 fields, found %d", lineNo, len(record))
+			return
+		}
+		email := strings.ToLower(strings.TrimSpace(record[0]))
+		firstName := strings.TrimSpace(record[1])
+		lastName := strings.TrimSpace(record[2])
+		active := strings.TrimSpace(record[3])
+		admin := strings.TrimSpace(record[4])
+		if email == "" || firstName == "" || lastName == "" || active == "" || admin == "" {
+			err = fmt.Errorf("parsing error at line %d: fields cannot be empty", lineNo)
+			return
+		}
+		activeBool, err := strconv.ParseBool(active)
+		if err != nil {
+			return nil, fmt.Errorf("parsing error at line %d: active status not recognized", lineNo)
+		}
+		adminBool, err := strconv.ParseBool(admin)
+		if err != nil {
+			return nil, fmt.Errorf("parsing error at line %d: admin status not recognized", lineNo)
+		}
+		loadedRecords = append(loadedRecords, userRecord{
+			Email:     email,
+			FirstName: firstName,
+			LastName:  lastName,
+			Active:    activeBool,
+			Admin:     adminBool,
+		})
+	}
+	return loadedRecords, nil
+}
+
+// GetAll adds all objects of type 'resource' to the 'allItems' list
+func GetAll(c *arvados.Client, res string, params arvados.ResourceListParams, page resourceList) (allItems []interface{}, err error) {
+	// Use the maximum page size the server allows
+	limit := 1<<31 - 1
+	params.Limit = &limit
+	params.Offset = 0
+	params.Order = "uuid"
+	for {
+		if err = GetResourceList(c, &page, res, params); err != nil {
+			return allItems, err
+		}
+		// Have we finished paging?
+		if page.Len() == 0 {
+			break
+		}
+		allItems = append(allItems, page.GetItems()...)
+		params.Offset += page.Len()
+	}
+	return allItems, nil
+}
+
+func jsonReader(rscName string, ob interface{}) io.Reader {
+	j, err := json.Marshal(ob)
+	if err != nil {
+		panic(err)
+	}
+	v := url.Values{}
+	v[rscName] = []string{string(j)}
+	return bytes.NewBufferString(v.Encode())
+}
+
+// GetResourceList fetches res list using params
+func GetResourceList(c *arvados.Client, dst *resourceList, res string, params interface{}) error {
+	return c.RequestAndDecode(dst, "GET", "/arvados/v1/"+res, nil, params)
+}
+
+// CreateUser creates a user with userData parameters, assigns it to dst
+func CreateUser(c *arvados.Client, dst *arvados.User, userData map[string]string) error {
+	return c.RequestAndDecode(dst, "POST", "/arvados/v1/users", jsonReader("user", userData), nil)
+}
+
+// UpdateUser updates a user with userData parameters
+func UpdateUser(c *arvados.Client, userUUID string, dst *arvados.User, userData map[string]string) error {
+	return c.RequestAndDecode(&dst, "PUT", "/arvados/v1/users/"+userUUID, jsonReader("user", userData), nil)
+}
+
+// UnsetupUser deactivates a user
+func UnsetupUser(c *arvados.Client, userUUID string, dst *arvados.User) error {
+	return c.RequestAndDecode(&dst, "POST", "/arvados/v1/users/"+userUUID+"/unsetup", nil, nil)
+}

commit ffb6a2fbc2552b38dd27b216c9db6539e81cef56
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Wed Jun 15 10:26:54 2022 -0300

    18858: Doc site URL fix.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/apps/workbench/app/views/users/_show_admin.html.erb b/apps/workbench/app/views/users/_show_admin.html.erb
index 1da22d438..b151ceff0 100644
--- a/apps/workbench/app/views/users/_show_admin.html.erb
+++ b/apps/workbench/app/views/users/_show_admin.html.erb
@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
   <div class="col-md-6">
 
     <p>
-      This page enables you to <a href="https://doc.arvados.org/master/admin/user-management.html">manage users</a>.
+      This page enables you to <a href="https://doc.arvados.org/main/admin/user-management.html">manage users</a>.
     </p>
 
     <p>
@@ -22,7 +22,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
       As an admin, you can deactivate and reset this user. This will
       remove all repository/VM permissions for the user. If you
       "setup" the user again, the user will have to sign the user
-      agreement again.  You may also want to <a href="https://doc.arvados.org/master/admin/reassign-ownership.html">reassign data ownership</a>.
+      agreement again.  You may also want to <a href="https://doc.arvados.org/main/admin/reassign-ownership.html">reassign data ownership</a>.
     </p>
 
     <%= button_to "Deactivate #{@object.full_name}", unsetup_user_url(id: @object.uuid), class: 'btn btn-primary', data: {confirm: "Are you sure you want to deactivate #{@object.full_name}?"} %>

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list