[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