diff --git a/cmd/harbor/root/user/create.go b/cmd/harbor/root/user/create.go index bd705e3d9..279a22777 100644 --- a/cmd/harbor/root/user/create.go +++ b/cmd/harbor/root/user/create.go @@ -23,6 +23,38 @@ import ( "github.com/spf13/cobra" ) +type UserCreator interface { + FillUser(opts *create.CreateView) + UserCreate(opts create.CreateView) error +} +type DefaultUserCreator struct{} + +func (d *DefaultUserCreator) FillUser(opts *create.CreateView) { + create.CreateUserView(opts) +} + +func (d *DefaultUserCreator) UserCreate(opts create.CreateView) error { + return api.CreateUser(opts) +} +func CreateUser(opts *create.CreateView, userCreator UserCreator) { + var err error + + if opts.Email == "" || opts.Realname == "" || opts.Password == "" || opts.Username == "" { + userCreator.FillUser(opts) + } + + err = userCreator.UserCreate(*opts) + + if err != nil { + if isUnauthorizedError(err) { + log.WithFields(log.Fields{ + "action": "user create", + }).Error("Permission denied: The current account does not have the required permissions to create users.") + } else { + log.Errorf("failed to create user: %v", err) + } + } +} func UserCreateCmd() *cobra.Command { var opts create.CreateView @@ -31,30 +63,8 @@ func UserCreateCmd() *cobra.Command { Short: "create user", Args: cobra.ExactArgs(0), Run: func(cmd *cobra.Command, args []string) { - var err error - createView := &create.CreateView{ - Email: opts.Email, - Realname: opts.Realname, - Comment: opts.Comment, - Password: opts.Password, - Username: opts.Username, - } - - if opts.Email != "" && opts.Realname != "" && opts.Comment != "" && opts.Password != "" && opts.Username != "" { - err = api.CreateUser(opts) - } else { - err = createUserView(createView) - } - - // Check if the error is due to unauthorized access. - - if err != nil { - if isUnauthorizedError(err) { - log.Error("Permission denied: Admin privileges are required to execute this command.") - } else { - log.Errorf("failed to create user: %v", err) - } - } + d := &DefaultUserCreator{} + CreateUser(&opts, d) }, } @@ -67,12 +77,9 @@ func UserCreateCmd() *cobra.Command { return cmd } - -func createUserView(createView *create.CreateView) error { - create.CreateUserView(createView) - return api.CreateUser(*createView) -} - func isUnauthorizedError(err error) bool { + if err == nil { + return false + } return strings.Contains(err.Error(), "403") } diff --git a/cmd/harbor/root/user/create_test.go b/cmd/harbor/root/user/create_test.go new file mode 100644 index 000000000..3eec03613 --- /dev/null +++ b/cmd/harbor/root/user/create_test.go @@ -0,0 +1,340 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package user + +import ( + "bytes" + "cmp" + "fmt" + "reflect" + "slices" + "testing" + + log "github.com/sirupsen/logrus" + + "github.com/goharbor/harbor-cli/pkg/views/user/create" + "github.com/stretchr/testify/assert" +) + +type MockUserCreator struct { + usernames map[string]struct{} + emails map[string]struct{} + users []*create.CreateView + expectAuthError bool +} + +func initMockUserCreator(expectAuthError bool) *MockUserCreator { + return &MockUserCreator{ + usernames: make(map[string]struct{}), + emails: make(map[string]struct{}), + users: []*create.CreateView{}, + expectAuthError: expectAuthError, + } +} + +/* + FillUser simulates the interactive prompt that fills missing fields. + Note: Username and Email must be provided in test cases since they're + required fields that would be prompted interactively in real usage. + Only Realname and Password are filled here to test the FillUser path. +*/ + +func (m *MockUserCreator) FillUser(opts *create.CreateView) { + randomStr := "Random string 1234" + if opts.Realname == "" { + opts.Realname = randomStr + } + if opts.Password == "" { + opts.Password = randomStr + } +} + +func (m *MockUserCreator) UserCreate(opts create.CreateView) error { + if m.expectAuthError { + return fmt.Errorf("403") + } + if opts.Email == "" || opts.Realname == "" || opts.Password == "" || opts.Username == "" { + return fmt.Errorf("missing required fields") + } + _, foundUsername := m.usernames[opts.Username] + _, foundEmail := m.emails[opts.Email] + if foundUsername || foundEmail { + return fmt.Errorf("user %s or email %s already exists", opts.Username, opts.Email) + } + m.usernames[opts.Username] = struct{}{} + m.emails[opts.Email] = struct{}{} + m.users = append(m.users, &create.CreateView{ + Username: opts.Username, + Email: opts.Email, + Comment: opts.Comment, + Realname: opts.Realname, + Password: opts.Password, + }) + return nil +} +func TestCreateUsers(t *testing.T) { + usersAreEqual := func(u1, u2 []*create.CreateView) bool { + if len(u1) != len(u2) { + return false + } + slices.SortFunc(u1, func(a, b *create.CreateView) int { + return cmp.Compare(a.Username, b.Username) + }) + slices.SortFunc(u2, func(a, b *create.CreateView) int { + return cmp.Compare(a.Username, b.Username) + }) + for i := 0; i < len(u1); i++ { + if !reflect.DeepEqual(u1[i], u2[i]) { + return false + } + } + return true + } + tests := []struct { + name string + setup func() ([]*create.CreateView, *MockUserCreator) + expectedUsers []*create.CreateView + expectedErr string + }{ + { + name: "successfully create single user with all fields", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{ + { + Username: "testuser", + Email: "test@example.com", + Realname: "Test User", + Password: "TestPass123", + Comment: "Test comment", + }, + } + return views, initMockUserCreator(false) + }, + expectedUsers: []*create.CreateView{ + { + Username: "testuser", + Email: "test@example.com", + Realname: "Test User", + Password: "TestPass123", + Comment: "Test comment", + }, + }, + expectedErr: "", + }, + { + name: "successfully create multiple users", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{ + {Username: "user1", Email: "user1@example.com", Realname: "User One", Password: "Pass1", Comment: "First"}, + {Username: "user2", Email: "user2@example.com", Realname: "User Two", Password: "Pass2", Comment: "Second"}, + {Username: "user3", Email: "user3@example.com", Realname: "User Three", Password: "Pass3", Comment: "Third"}, + } + return views, initMockUserCreator(false) + }, + expectedUsers: []*create.CreateView{ + {Username: "user1", Email: "user1@example.com", Realname: "User One", Password: "Pass1", Comment: "First"}, + {Username: "user2", Email: "user2@example.com", Realname: "User Two", Password: "Pass2", Comment: "Second"}, + {Username: "user3", Email: "user3@example.com", Realname: "User Three", Password: "Pass3", Comment: "Third"}, + }, + expectedErr: "", + }, + { + name: "create user with missing fields triggers FillUser", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{ + { + Username: "testuser", + Email: "test@example.com", + }, + } + return views, initMockUserCreator(false) + }, + expectedUsers: []*create.CreateView{ + { + Username: "testuser", + Email: "test@example.com", + Realname: "Random string 1234", + Password: "Random string 1234", + }, + }, + expectedErr: "", + }, + { + name: "permission denied error (403)", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{ + { + Username: "testuser", + Email: "test@example.com", + Realname: "Test User", + Password: "TestPass123", + }, + } + return views, initMockUserCreator(true) + }, + expectedUsers: []*create.CreateView{}, + expectedErr: "Permission denied", + }, + { + name: "duplicate username fails second user", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{ + {Username: "sameuser", Email: "first@example.com", Realname: "First", Password: "Pass1"}, + {Username: "sameuser", Email: "second@example.com", Realname: "Second", Password: "Pass2"}, + } + return views, initMockUserCreator(false) + }, + expectedUsers: []*create.CreateView{ + {Username: "sameuser", Email: "first@example.com", Realname: "First", Password: "Pass1"}, + }, + expectedErr: "already exists", + }, + { + name: "duplicate email fails second user", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{ + {Username: "user1", Email: "same@example.com", Realname: "First", Password: "Pass1"}, + {Username: "user2", Email: "same@example.com", Realname: "Second", Password: "Pass2"}, + } + return views, initMockUserCreator(false) + }, + expectedUsers: []*create.CreateView{ + {Username: "user1", Email: "same@example.com", Realname: "First", Password: "Pass1"}, + }, + expectedErr: "already exists", + }, + { + name: "create user with empty comment succeeds", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{ + { + Username: "testuser", + Email: "test@example.com", + Realname: "Test User", + Password: "TestPass123", + Comment: "", + }, + } + return views, initMockUserCreator(false) + }, + expectedUsers: []*create.CreateView{ + { + Username: "testuser", + Email: "test@example.com", + Realname: "Test User", + Password: "TestPass123", + Comment: "", + }, + }, + expectedErr: "", + }, + { + name: "no users to create", + setup: func() ([]*create.CreateView, *MockUserCreator) { + views := []*create.CreateView{} + return views, initMockUserCreator(false) + }, + expectedUsers: []*create.CreateView{}, + expectedErr: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + originalLogOutput := log.StandardLogger().Out + log.SetOutput(&buf) + defer log.SetOutput(originalLogOutput) + + opts, m := tt.setup() + + for _, opt := range opts { + CreateUser(opt, m) + } + logs := buf.String() + if tt.expectedErr != "" { + assert.Contains(t, logs, tt.expectedErr, "Expected error logs to contain %s but got %s", tt.expectedErr, logs) + } else { + assert.Empty(t, logs, "Expected no error logs but got: %s", logs) + } + if !usersAreEqual(m.users, tt.expectedUsers) { + t.Errorf("Users mismatch.\nExpected: %+v\nGot: %+v", tt.expectedUsers, m.users) + } + }) + } +} + +func TestIsUnauthorizedError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "nil error returns false", + err: nil, + expected: false, + }, + { + name: "error containing 403 returns true", + err: fmt.Errorf("403 Forbidden"), + expected: true, + }, + { + name: "error without 403 returns false", + err: fmt.Errorf("404 Not Found"), + expected: false, + }, + { + name: "error with 500 returns false", + err: fmt.Errorf("500 Internal Server Error"), + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isUnauthorizedError(tt.err) + assert.Equal(t, tt.expected, result, "isUnauthorizedError(%v) = %v, want %v", tt.err, result, tt.expected) + }) + } +} + +func TestUserCreateCmd(t *testing.T) { + cmd := UserCreateCmd() + + assert.Equal(t, "create", cmd.Use) + assert.Equal(t, "create user", cmd.Short) + assert.NotNil(t, cmd.Args, "Args validator should be set") + + emailFlag := cmd.Flags().Lookup("email") + assert.NotNil(t, emailFlag) + assert.Equal(t, "", emailFlag.DefValue) + + realnameFlag := cmd.Flags().Lookup("realname") + assert.NotNil(t, realnameFlag) + assert.Equal(t, "", realnameFlag.DefValue) + + commentFlag := cmd.Flags().Lookup("comment") + assert.NotNil(t, commentFlag) + assert.Equal(t, "", commentFlag.DefValue) + + passwordFlag := cmd.Flags().Lookup("password") + assert.NotNil(t, passwordFlag) + assert.Equal(t, "", passwordFlag.DefValue) + + usernameFlag := cmd.Flags().Lookup("username") + assert.NotNil(t, usernameFlag) + assert.Equal(t, "", usernameFlag.DefValue) +} diff --git a/cmd/harbor/root/user/delete.go b/cmd/harbor/root/user/delete.go index eaa2b1f4c..da6651371 100644 --- a/cmd/harbor/root/user/delete.go +++ b/cmd/harbor/root/user/delete.go @@ -22,59 +22,81 @@ import ( "github.com/spf13/cobra" ) -// UserDeleteCmd defines the "delete" command for user deletion. -func UserDeleteCmd() *cobra.Command { - cmd := &cobra.Command{ - Use: "delete", - Short: "delete user by name or id", - Args: cobra.MinimumNArgs(0), - Run: func(cmd *cobra.Command, args []string) { - // If there are command line arguments, process them concurrently. - if len(args) > 0 { - var wg sync.WaitGroup - errChan := make(chan error, len(args)) // Channel to collect errors +type UserDeleter interface { + UserDelete(userID int64) error + GetUserIDByName(username string) (int64, error) + GetUserIDFromUser() int64 +} +type DefaultUserDeleter struct{} - for _, arg := range args { - // Retrieve user ID by name. - userID, err := api.GetUsersIdByName(arg) - if err != nil { - log.Errorf("failed to get user id for '%s': %v", arg, err) - continue - } - wg.Add(1) - go func(userID int64) { - defer wg.Done() - if err := api.DeleteUser(userID); err != nil { - errChan <- err - } - }(userID) - } +func (d *DefaultUserDeleter) UserDelete(userID int64) error { + return api.DeleteUser(userID) +} +func (d *DefaultUserDeleter) GetUserIDByName(username string) (int64, error) { + return api.GetUsersIdByName(username) +} +func (d *DefaultUserDeleter) GetUserIDFromUser() int64 { + return prompt.GetUserIdFromUser() +} - // Wait for all goroutines to finish and then close the error channel. - go func() { - wg.Wait() - close(errChan) - }() +func DeleteUser(args []string, userDeleter UserDeleter) { + // If there are command line arguments, process them concurrently. + if len(args) > 0 { + var wg sync.WaitGroup + errChan := make(chan error, len(args)) // Channel to collect errors - // Process errors from the goroutines. - for err := range errChan { - if isUnauthorizedError(err) { - log.Error("Permission denied: Admin privileges are required to execute this command.") - } else { - log.Errorf("failed to delete user: %v", err) - } + for _, arg := range args { + // Retrieve user ID by name. + userID, err := userDeleter.GetUserIDByName(arg) + if err != nil { + log.Errorf("failed to get user id for '%s': %v", arg, err) + continue + } + wg.Add(1) + go func(userID int64) { + defer wg.Done() + if err := userDeleter.UserDelete(userID); err != nil { + errChan <- err } + }(userID) + } + + // Wait for all goroutines to finish and then close the error channel. + go func() { + wg.Wait() + close(errChan) + }() + + // Process errors from the goroutines. + for err := range errChan { + if isUnauthorizedError(err) { + log.Error("Permission denied: Admin privileges are required to execute this command.") } else { - // Interactive mode: get the user ID from the prompt. - userID := prompt.GetUserIdFromUser() - if err := api.DeleteUser(userID); err != nil { - if isUnauthorizedError(err) { - log.Error("Permission denied: Admin privileges are required to execute this command.") - } else { - log.Errorf("failed to delete user: %v", err) - } - } + log.Errorf("failed to delete user: %v", err) + } + } + } else { + // Interactive mode: get the user ID from the prompt. + userID := userDeleter.GetUserIDFromUser() + if err := userDeleter.UserDelete(userID); err != nil { + if isUnauthorizedError(err) { + log.Error("Permission denied: Admin privileges are required to execute this command.") + } else { + log.Errorf("failed to delete user: %v", err) } + } + } +} + +// UserDeleteCmd defines the "delete" command for user deletion. +func UserDeleteCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "delete", + Short: "delete user by name or id", // nope it only deletes by name + Args: cobra.MinimumNArgs(0), + Run: func(cmd *cobra.Command, args []string) { + d := &DefaultUserDeleter{} + DeleteUser(args, d) }, } diff --git a/cmd/harbor/root/user/delete_test.go b/cmd/harbor/root/user/delete_test.go new file mode 100644 index 000000000..125e4622a --- /dev/null +++ b/cmd/harbor/root/user/delete_test.go @@ -0,0 +1,167 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package user + +import ( + "bytes" + "fmt" + "testing" + + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" +) + +type MockUserDeleter struct { + id map[string]int64 + user map[int64]string + userCnt int + expectAuthError bool +} + +func (m *MockUserDeleter) UserDelete(userID int64) error { + if m.expectAuthError { + return fmt.Errorf("403") + } + if v, ok := m.user[userID]; ok { + delete(m.id, v) + delete(m.user, userID) + return nil + } + return fmt.Errorf("user %d not found", userID) +} +func (m *MockUserDeleter) GetUserIDByName(username string) (int64, error) { + if v, ok := m.id[username]; ok { + return v, nil + } else { + return 0, fmt.Errorf(`Username %s not found`, username) + } +} +func (m *MockUserDeleter) GetUserIDFromUser() int64 { + return 999 +} + +func initMockUserDeleter(userCnt int, expectAuthError bool) *MockUserDeleter { + m := &MockUserDeleter{ + userCnt: userCnt, + expectAuthError: expectAuthError, + id: make(map[string]int64), + user: make(map[int64]string), + } + for i := 0; i < userCnt; i++ { + m.id[fmt.Sprintf("test%d", i+1)] = int64(i + 1) + m.user[int64(i+1)] = fmt.Sprintf("test%d", i+1) + } + return m +} +func TestDeleteUser(t *testing.T) { + tests := []struct { + name string + setup func() *MockUserDeleter + args []string + notExpectedID []int64 + expectedErr string + }{ + { + name: "successfully delete single user", + setup: func() *MockUserDeleter { + return initMockUserDeleter(5, false) + }, + args: []string{"test1"}, + notExpectedID: []int64{1}, + expectedErr: "", + }, + { + name: "successfully delete multiple users", + setup: func() *MockUserDeleter { + return initMockUserDeleter(5, false) + }, + args: []string{"test1", "test3", "test5"}, + notExpectedID: []int64{1, 3, 5}, + expectedErr: "", + }, + { + name: "delete non-existent user logs error", + setup: func() *MockUserDeleter { + return initMockUserDeleter(5, false) + }, + args: []string{"nonexistent"}, + notExpectedID: []int64{}, + expectedErr: "failed to get user id", + }, + { + name: "permission denied error", + setup: func() *MockUserDeleter { + return initMockUserDeleter(5, true) + }, + args: []string{"test1"}, + notExpectedID: []int64{}, + expectedErr: "Permission denied", + }, + { + name: "mixed existing and non-existing users", + setup: func() *MockUserDeleter { + return initMockUserDeleter(5, false) + }, + args: []string{"test1", "nonexistent", "test3"}, + notExpectedID: []int64{1, 3}, + expectedErr: "failed to get user id", + }, + { + name: "delete with empty args does not error", + setup: func() *MockUserDeleter { + m := initMockUserDeleter(5, false) + m.user[999] = "promptuser" + m.id["promptuser"] = 999 + return m + }, + args: []string{}, + notExpectedID: []int64{999}, + expectedErr: "", + }, + { + name: "delete all users", + setup: func() *MockUserDeleter { + return initMockUserDeleter(3, false) + }, + args: []string{"test1", "test2", "test3"}, + notExpectedID: []int64{1, 2, 3}, + expectedErr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + originalLogOutput := log.StandardLogger().Out + log.SetOutput(&buf) + defer log.SetOutput(originalLogOutput) + + m := tt.setup() + DeleteUser(tt.args, m) + + logs := buf.String() + + if tt.expectedErr != "" { + assert.Contains(t, logs, tt.expectedErr, "Expected error logs to contain %s but got %s", tt.expectedErr, logs) + } else { + assert.Empty(t, logs, "Expected no error logs but got: %s", logs) + } + + for _, id := range tt.notExpectedID { + _, ok := m.user[id] + assert.False(t, ok, "User with ID %d should have been deleted", id) + } + }) + } +} diff --git a/cmd/harbor/root/user/elevate.go b/cmd/harbor/root/user/elevate.go index 060f83805..53bdcfead 100644 --- a/cmd/harbor/root/user/elevate.go +++ b/cmd/harbor/root/user/elevate.go @@ -21,6 +21,69 @@ import ( "github.com/spf13/cobra" ) +type UserElevator interface { + GetUserIDByName(username string) (int64, error) + GetUserIDFromUser() int64 + ConfirmElevation() (bool, error) + ElevateUser(userID int64) error +} + +type DefaultUserElevator struct{} + +func (d *DefaultUserElevator) GetUserIDByName(username string) (int64, error) { + return api.GetUsersIdByName(username) +} + +func (d *DefaultUserElevator) GetUserIDFromUser() int64 { + return prompt.GetUserIdFromUser() +} + +func (d *DefaultUserElevator) ConfirmElevation() (bool, error) { + return views.ConfirmElevation() +} + +func (d *DefaultUserElevator) ElevateUser(userID int64) error { + return api.ElevateUser(userID) +} + +func ElevateUser(args []string, userElevator UserElevator) { + var err error + var userID int64 + + if len(args) > 0 { + userID, err = userElevator.GetUserIDByName(args[0]) + if err != nil { + log.Errorf("failed to get user id for '%s': %v", args[0], err) + return + } + if userID == 0 { + log.Errorf("User with name '%s' not found", args[0]) + return + } + } else { + userID = userElevator.GetUserIDFromUser() + } + + confirm, err := userElevator.ConfirmElevation() + if err != nil { + log.Errorf("failed to confirm elevation: %v", err) + return + } + if !confirm { + log.Error("User did not confirm elevation. Aborting command.") + return + } + + err = userElevator.ElevateUser(userID) + if err != nil { + if isUnauthorizedError(err) { + log.Error("Permission denied: Admin privileges are required to execute this command.") + } else { + log.Errorf("failed to elevate user: %v", err) + } + } +} + func ElevateUserCmd() *cobra.Command { cmd := &cobra.Command{ Use: "elevate", @@ -28,39 +91,8 @@ func ElevateUserCmd() *cobra.Command { Long: "elevate user to admin role", Args: cobra.MaximumNArgs(1), Run: func(cmd *cobra.Command, args []string) { - var err error - var userId int64 - if len(args) > 0 { - userId, err = api.GetUsersIdByName(args[0]) - if err != nil { - log.Errorf("failed to get user id for '%s': %v", args[0], err) - return - } - if userId == 0 { - log.Errorf("User with name '%s' not found", args[0]) - return - } - } else { - userId = prompt.GetUserIdFromUser() - } - confirm, err := views.ConfirmElevation() - if err != nil { - log.Errorf("failed to confirm elevation: %v", err) - return - } - if !confirm { - log.Error("User did not confirm elevation. Aborting command.") - return - } - - err = api.ElevateUser(userId) - if err != nil { - if isUnauthorizedError(err) { - log.Error("Permission denied: Admin privileges are required to execute this command.") - } else { - log.Errorf("failed to elevate user: %v", err) - } - } + d := &DefaultUserElevator{} + ElevateUser(args, d) }, } diff --git a/cmd/harbor/root/user/elevate_test.go b/cmd/harbor/root/user/elevate_test.go new file mode 100644 index 000000000..efcde6e57 --- /dev/null +++ b/cmd/harbor/root/user/elevate_test.go @@ -0,0 +1,185 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package user + +import ( + "bytes" + "fmt" + "testing" + + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" +) + +type MockUserElevator struct { + id map[string]int64 + admins map[int64]bool + userCnt int + expectAuthError bool + confirmElevation bool + confirmElevationErr error +} + +func (m *MockUserElevator) GetUserIDByName(username string) (int64, error) { + if v, ok := m.id[username]; ok { + return v, nil + } + return 0, fmt.Errorf("username %s not found", username) +} + +func (m *MockUserElevator) GetUserIDFromUser() int64 { + return 999 +} + +func (m *MockUserElevator) ConfirmElevation() (bool, error) { + return m.confirmElevation, m.confirmElevationErr +} + +func (m *MockUserElevator) ElevateUser(userID int64) error { + if m.expectAuthError { + return fmt.Errorf("403") + } + if _, ok := m.admins[userID]; !ok { + m.admins[userID] = true + return nil + } + return fmt.Errorf("user %d is already an admin", userID) +} + +func initMockUserElevator(userCnt int, expectAuthError bool, confirmElevation bool, confirmElevationErr error) *MockUserElevator { + m := &MockUserElevator{ + userCnt: userCnt, + expectAuthError: expectAuthError, + confirmElevation: confirmElevation, + confirmElevationErr: confirmElevationErr, + id: make(map[string]int64), + admins: make(map[int64]bool), + } + for i := 0; i < userCnt; i++ { + m.id[fmt.Sprintf("test%d", i+1)] = int64(i + 1) + } + return m +} + +func TestElevateUser(t *testing.T) { + tests := []struct { + name string + setup func() *MockUserElevator + args []string + expectedAdminID []int64 + expectedErr string + }{ + { + name: "successfully elevate user by username", + setup: func() *MockUserElevator { + return initMockUserElevator(5, false, true, nil) + }, + args: []string{"test1"}, + expectedAdminID: []int64{1}, + expectedErr: "", + }, + { + name: "elevate user via interactive prompt", + setup: func() *MockUserElevator { + m := initMockUserElevator(5, false, true, nil) + m.id["promptuser"] = 999 + return m + }, + args: []string{}, + expectedAdminID: []int64{999}, + expectedErr: "", + }, + { + name: "user not found logs error", + setup: func() *MockUserElevator { + return initMockUserElevator(5, false, true, nil) + }, + args: []string{"nonexistent"}, + expectedAdminID: []int64{}, + expectedErr: "failed to get user id", + }, + { + name: "permission denied error", + setup: func() *MockUserElevator { + return initMockUserElevator(5, true, true, nil) + }, + args: []string{"test1"}, + expectedAdminID: []int64{}, + expectedErr: "Permission denied", + }, + { + name: "user declines elevation confirmation", + setup: func() *MockUserElevator { + return initMockUserElevator(5, false, false, nil) + }, + args: []string{"test1"}, + expectedAdminID: []int64{}, + expectedErr: "User did not confirm elevation", + }, + { + name: "confirmation prompt returns error", + setup: func() *MockUserElevator { + return initMockUserElevator(5, false, false, fmt.Errorf("terminal error")) + }, + args: []string{"test1"}, + expectedAdminID: []int64{}, + expectedErr: "failed to confirm elevation", + }, + { + name: "elevate user that is already admin", + setup: func() *MockUserElevator { + m := initMockUserElevator(5, false, true, nil) + m.admins[1] = true + return m + }, + args: []string{"test1"}, + expectedAdminID: []int64{1}, + expectedErr: "already an admin", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + originalLogOutput := log.StandardLogger().Out + log.SetOutput(&buf) + defer log.SetOutput(originalLogOutput) + + m := tt.setup() + ElevateUser(tt.args, m) + + logs := buf.String() + + if tt.expectedErr != "" { + assert.Contains(t, logs, tt.expectedErr, "Expected error logs to contain %s but got %s", tt.expectedErr, logs) + } else { + assert.Empty(t, logs, "Expected no error logs but got: %s", logs) + } + + for _, id := range tt.expectedAdminID { + isAdmin, exists := m.admins[id] + assert.True(t, exists && isAdmin, "User with ID %d should be an admin", id) + } + }) + } +} + +func TestElevateUserCmd(t *testing.T) { + cmd := ElevateUserCmd() + + assert.Equal(t, "elevate", cmd.Use) + assert.Equal(t, "elevate user", cmd.Short) + assert.Equal(t, "elevate user to admin role", cmd.Long) + assert.NotNil(t, cmd.Args, "Args validator should be set") +} diff --git a/cmd/harbor/root/user/list.go b/cmd/harbor/root/user/list.go index 7bad0326d..b9be13370 100644 --- a/cmd/harbor/root/user/list.go +++ b/cmd/harbor/root/user/list.go @@ -16,6 +16,7 @@ package user import ( "fmt" + "github.com/goharbor/go-client/pkg/sdk/v2.0/client/user" "github.com/goharbor/go-client/pkg/sdk/v2.0/models" "github.com/goharbor/harbor-cli/pkg/api" "github.com/goharbor/harbor-cli/pkg/utils" @@ -25,64 +26,86 @@ import ( "github.com/spf13/viper" ) -func UserListCmd() *cobra.Command { - var opts api.ListFlags - var allUsers []*models.UserResp +type UserLister interface { + UserList(opts ...api.ListFlags) (*user.ListUsersOK, error) +} +type DefaultUserLister struct{} - cmd := &cobra.Command{ - Use: "list", - Short: "List users", - Args: cobra.ExactArgs(0), - Aliases: []string{"ls"}, - RunE: func(cmd *cobra.Command, args []string) error { - if opts.PageSize > 100 { - return fmt.Errorf("page size should be less than or equal to 100") - } +func (d *DefaultUserLister) UserList(opts ...api.ListFlags) (*user.ListUsersOK, error) { + return api.ListUsers(opts...) +} - if opts.PageSize == 0 { - opts.PageSize = 100 - opts.Page = 1 +func GetUsers(opts api.ListFlags, userLister UserLister) ([]*models.UserResp, error) { + var allUsers []*models.UserResp - for { - response, err := api.ListUsers(opts) - if err != nil { - if isUnauthorizedError(err) { - return fmt.Errorf("Permission denied: Admin privileges are required to execute this command.") - } - return fmt.Errorf("failed to list users: %v", err) - } + if opts.PageSize > 100 || opts.PageSize < 0 { + return nil, fmt.Errorf("page size should be greater than or equal to 0 and less than or equal to 100") + } - allUsers = append(allUsers, response.Payload...) + if opts.PageSize == 0 { + opts.PageSize = 100 + opts.Page = 1 - if len(response.Payload) < int(opts.PageSize) { - break - } - opts.Page++ + for { + response, err := userLister.UserList(opts) + if err != nil { + if isUnauthorizedError(err) { + return nil, fmt.Errorf("Permission denied: Admin privileges are required to execute this command.") } - } else { - response, err := api.ListUsers(opts) - if err != nil { - if isUnauthorizedError(err) { - return fmt.Errorf("Permission denied: Admin privileges are required to execute this command.") - } - return fmt.Errorf("failed to list users: %v", err) - } - allUsers = response.Payload + return nil, fmt.Errorf("failed to list users: %v", err) + } + + allUsers = append(allUsers, response.Payload...) + + if len(response.Payload) < int(opts.PageSize) { + break } - if len(allUsers) == 0 { - log.Info("No users found") - return nil + opts.Page++ + } + } else { + response, err := userLister.UserList(opts) + if err != nil { + if isUnauthorizedError(err) { + return nil, fmt.Errorf("Permission denied: Admin privileges are required to execute this command.") } - formatFlag := viper.GetString("output-format") - if formatFlag != "" { - err := utils.PrintFormat(allUsers, formatFlag) - if err != nil { - log.Error(err) - } - } else { - list.ListUsers(allUsers) + return nil, fmt.Errorf("failed to list users: %v", err) + } + allUsers = response.Payload + } + return allUsers, nil +} +func PrintUsers(allUsers []*models.UserResp) error { + if len(allUsers) == 0 { + log.Info("No users found") + return nil + } + formatFlag := viper.GetString("output-format") + if formatFlag != "" { + err := utils.PrintFormat(allUsers, formatFlag) + if err != nil { + log.Error(err) + } + } else { + if err := list.ListUsers(allUsers); err != nil { + return err + } + } + return nil +} +func UserListCmd() *cobra.Command { + var opts api.ListFlags + cmd := &cobra.Command{ + Use: "list", + Short: "List users", + Args: cobra.ExactArgs(0), + Aliases: []string{"ls"}, + RunE: func(cmd *cobra.Command, args []string) error { + defaultUserLister := &DefaultUserLister{} + allUsers, err := GetUsers(opts, defaultUserLister) + if err != nil { + return err } - return nil + return PrintUsers(allUsers) }, } diff --git a/cmd/harbor/root/user/list_test.go b/cmd/harbor/root/user/list_test.go new file mode 100644 index 000000000..bba848ea5 --- /dev/null +++ b/cmd/harbor/root/user/list_test.go @@ -0,0 +1,511 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package user + +import ( + "bytes" + "cmp" + "encoding/json" + "fmt" + "io" + "os" + "slices" + "strings" + "testing" + + "github.com/go-openapi/strfmt" + "github.com/goharbor/go-client/pkg/sdk/v2.0/client/user" + "github.com/goharbor/go-client/pkg/sdk/v2.0/models" + "github.com/goharbor/harbor-cli/pkg/api" + log "github.com/sirupsen/logrus" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "go.yaml.in/yaml/v4" +) + +func captureOutput(f func() error) (string, error) { + origStdout := os.Stdout + defer func() { os.Stdout = origStdout }() + + r, w, _ := os.Pipe() + os.Stdout = w + + if err := f(); err != nil { + return "", err + } + + w.Close() + + var buf bytes.Buffer + if _, err := io.Copy(&buf, r); err != nil { + return "", err + } + return buf.String(), nil +} +func TestPrintUsers(t *testing.T) { + testDate, _ := strfmt.ParseDateTime("2023-01-01T12:00:00Z") + testUsers := func() []*models.UserResp { + return []*models.UserResp{ + { + UserID: 1, + Username: "testUser1", + Email: "test1@domain.com", + SysadminFlag: true, + Realname: "Test1", + CreationTime: testDate, + }, + { + UserID: 2, + Username: "testUser2", + Email: "test2@domain.com", + SysadminFlag: false, + Realname: "Test2", + CreationTime: testDate, + }, + { + UserID: 3, + Username: "testUser3", + Email: "test3@domain.com", + SysadminFlag: false, + Realname: "Test3", + CreationTime: testDate, + }, + } + } + tests := []struct { + name string + setup func() []*models.UserResp + outputFormat string + }{ + { + name: "Number of users not zero and output format is json", + setup: func() []*models.UserResp { + users := testUsers() + return users + }, + outputFormat: "json", + }, + { + name: "Number of users not zero and output format yaml", + setup: func() []*models.UserResp { + users := testUsers() + return users + }, + outputFormat: "yaml", + }, + { + name: "Number of users not zero and output format default", + setup: func() []*models.UserResp { + users := testUsers() + return users + }, + outputFormat: "", + }, + { + name: "Number of users is zero", + setup: func() []*models.UserResp { + users := []*models.UserResp{} + return users + }, + outputFormat: "default", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + allUsers := tt.setup() + + var logBuf bytes.Buffer + originalLogOutput := log.StandardLogger().Out + log.SetOutput(&logBuf) + defer log.SetOutput(originalLogOutput) + + originalFormatFlag := viper.GetString("output-format") + viper.Set("output-format", tt.outputFormat) + defer viper.Set("output-format", originalFormatFlag) + + output, err := captureOutput(func() error { + return PrintUsers(allUsers) + }) + if err != nil { + t.Fatalf("PrintUsers() returned error: %v", err) + } + + logs := logBuf.String() + + switch { + case len(allUsers) == 0: + if !strings.Contains(logs, "No users found") { + t.Errorf(`Expected logs to contain "No users found" but got: %s`, logs) + } + case tt.outputFormat == "json": + if len(output) == 0 { + t.Fatal("Expected JSON output, but output was empty") + } + var decodedUsers []*models.UserResp + if err := json.Unmarshal([]byte(output), &decodedUsers); err != nil { + t.Fatalf("Output is not valid JSON: %v. Output:\n%s", err, output) + } + if len(decodedUsers) != len(allUsers) { + t.Errorf("Expected %d users in JSON, got %d", len(allUsers), len(decodedUsers)) + } + if len(decodedUsers) > 0 { + if decodedUsers[0].Username != allUsers[0].Username { + t.Errorf("Expected username '%s', got '%s'", allUsers[0].Username, decodedUsers[0].Username) + } + if decodedUsers[0].SysadminFlag != allUsers[0].SysadminFlag { + t.Errorf("Expected SysadminFlag to be %v, got %v", allUsers[0].SysadminFlag, decodedUsers[0].SysadminFlag) + } + } + case tt.outputFormat == "yaml": + if len(output) == 0 { + t.Fatal("Expected YAML output, but output was empty") + } + var decodedUsers []*models.UserResp + if err := yaml.Unmarshal([]byte(output), &decodedUsers); err != nil { + t.Fatalf("Output is not valid YAML: %v. Output:\n%s", err, output) + } + if len(decodedUsers) != len(allUsers) { + t.Errorf("Expected %d users in YAML, got %d", len(allUsers), len(decodedUsers)) + } + if len(decodedUsers) > 0 { + if decodedUsers[0].Username != allUsers[0].Username { + t.Errorf("Expected username '%s', got '%s'", allUsers[0].Username, decodedUsers[0].Username) + } + if decodedUsers[0].SysadminFlag != allUsers[0].SysadminFlag { + t.Errorf("Expected SysadminFlag to be %v, got %v", allUsers[0].SysadminFlag, decodedUsers[0].SysadminFlag) + } + } + default: + if len(output) == 0 { + t.Fatal("Expected TUI table output, but output was empty") + } + if !strings.Contains(output, "ID") || !strings.Contains(output, "Name") || !strings.Contains(output, "Administrator") { + t.Error("Expected table output to contain headers 'ID', 'Name' and 'Administrator among other headers") + } + if !strings.Contains(output, "testUser1") { + t.Errorf("Expected table to contain username 'testUser1'") + } + } + }) + } +} + +type MockUserLister struct { + numberOfUsersforTesting int64 + usersForTesting []*models.UserResp + expectAuthError bool +} + +func (m *MockUserLister) populateUsers() []*models.UserResp { + users := make([]*models.UserResp, 0, m.numberOfUsersforTesting) + for i := 0; i < int(m.numberOfUsersforTesting); i++ { + user := &models.UserResp{ + UserID: int64(i + 1), + } + users = append(users, user) + } + m.usersForTesting = users + return users +} +func (m *MockUserLister) UserList(opts ...api.ListFlags) (*user.ListUsersOK, error) { + if m.expectAuthError { + return nil, fmt.Errorf("403") + } + res := &user.ListUsersOK{} + if len(opts) == 0 { + return res, nil + } + listFlags := opts[0] + page, pageSize := listFlags.Page, listFlags.PageSize + users := m.populateUsers() + lo, hi := max(pageSize*(page-1), 0), min(pageSize*page, m.numberOfUsersforTesting) + if lo >= m.numberOfUsersforTesting { + return res, nil + } + res.Payload = users[lo:hi] + return res, nil +} +func TestGetUsers(t *testing.T) { + usersAreEqual := func(u1, u2 []*models.UserResp) bool { + if len(u1) != len(u2) { + return false + } + slices.SortFunc(u1, func(a, b *models.UserResp) int { + return cmp.Compare(a.UserID, b.UserID) + }) + slices.SortFunc(u2, func(a, b *models.UserResp) int { + return cmp.Compare(a.UserID, b.UserID) + }) + for i := 0; i < len(u1); i++ { + if u1[i].UserID != u2[i].UserID { + return false + } + } + return true + } + tests := []struct { + name string + setup func() (api.ListFlags, *MockUserLister) + wantError bool + errContains string + }{ + { + name: "fetch specific page with valid page size", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 2, + PageSize: 50, + } + m := &MockUserLister{ + numberOfUsersforTesting: 102, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "fetch all users with page size 0 (multiple pages)", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 0, + } + m := &MockUserLister{ + numberOfUsersforTesting: 250, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "fetch all users when total is exactly divisible by 100", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 0, + } + m := &MockUserLister{ + numberOfUsersforTesting: 200, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "fetch first page with page size 10", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 10, + } + m := &MockUserLister{ + numberOfUsersforTesting: 50, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "fetch last page with partial results", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 3, + PageSize: 10, + } + m := &MockUserLister{ + numberOfUsersforTesting: 25, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "fetch page beyond available data returns empty", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 10, + PageSize: 10, + } + m := &MockUserLister{ + numberOfUsersforTesting: 5, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "fetch with maximum allowed page size 100", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 100, + } + m := &MockUserLister{ + numberOfUsersforTesting: 150, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "fetch with zero users in database", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 10, + } + m := &MockUserLister{ + numberOfUsersforTesting: 0, + expectAuthError: false, + } + return opts, m + }, + wantError: false, + }, + { + name: "page size exceeds maximum (101)", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 101, + } + m := &MockUserLister{ + numberOfUsersforTesting: 50, + expectAuthError: false, + } + return opts, m + }, + wantError: true, + errContains: "page size should be greater than or equal to 0 and less than or equal to 100", + }, + { + name: "page size is negative", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: -1, + } + m := &MockUserLister{ + numberOfUsersforTesting: 50, + expectAuthError: false, + } + return opts, m + }, + wantError: true, + errContains: "page size should be greater than or equal to 0 and less than or equal to 100", + }, + { + name: "authentication error returns permission denied", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 10, + } + m := &MockUserLister{ + numberOfUsersforTesting: 50, + expectAuthError: true, + } + return opts, m + }, + wantError: true, + errContains: "Permission denied", + }, + { + name: "authentication error during fetch all", + setup: func() (api.ListFlags, *MockUserLister) { + opts := api.ListFlags{ + Page: 1, + PageSize: 0, + } + m := &MockUserLister{ + numberOfUsersforTesting: 50, + expectAuthError: true, + } + return opts, m + }, + wantError: true, + errContains: "Permission denied", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts, m := tt.setup() + allUsers, err := GetUsers(opts, m) + + // Check if we expected an error but did not get one (or vice-versa) + if (err != nil) != tt.wantError { + t.Fatalf("GetUsers() error presence mismatch: got error %v, wantError %v", err, tt.wantError) + } + if tt.wantError && !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("Expected error to contain '%s', got '%s'", tt.errContains, err.Error()) + } + if !tt.wantError { + // check if the returned allUsers are correct according to our mock database of users (which is m.usersForTesting) + if opts.PageSize == 0 { + if !usersAreEqual(allUsers, m.usersForTesting) { + t.Errorf("Expected all of the users to be returned") + } + } else { + requiredPage, requiredPageSize := opts.Page, opts.PageSize + start := max(requiredPageSize*(requiredPage-1), 0) + end := min(requiredPageSize*requiredPage, m.numberOfUsersforTesting) + + if start >= m.numberOfUsersforTesting { + if len(allUsers) != 0 { + t.Errorf("Expected empty result for page beyond data, got %d users", len(allUsers)) + } + } else { + if !usersAreEqual(allUsers, m.usersForTesting[start:end]) { + t.Errorf("Expected different set of users") + } + } + } + } + }) + } +} +func TestUserListCmd(t *testing.T) { + cmd := UserListCmd() + + assert.Equal(t, "list", cmd.Use) + assert.Equal(t, "List users", cmd.Short) + assert.Contains(t, cmd.Aliases, "ls") + + pageFlag := cmd.Flags().Lookup("page") + assert.NotNil(t, pageFlag) + assert.Equal(t, "1", pageFlag.DefValue) + + pageSizeFlag := cmd.Flags().Lookup("page-size") + assert.NotNil(t, pageSizeFlag) + assert.Equal(t, "0", pageSizeFlag.DefValue) + + queryFlag := cmd.Flags().Lookup("query") + assert.NotNil(t, queryFlag) + + sortFlag := cmd.Flags().Lookup("sort") + assert.NotNil(t, sortFlag) + + assert.Equal(t, "p", pageFlag.Shorthand) + assert.Equal(t, "n", pageSizeFlag.Shorthand) + assert.Equal(t, "q", queryFlag.Shorthand) + assert.Equal(t, "s", sortFlag.Shorthand) +} diff --git a/cmd/harbor/root/user/password.go b/cmd/harbor/root/user/password.go index 34a6ce98a..f6bf50b4a 100644 --- a/cmd/harbor/root/user/password.go +++ b/cmd/harbor/root/user/password.go @@ -22,46 +22,71 @@ import ( "github.com/spf13/cobra" ) -func UserPasswordChangeCmd() *cobra.Command { - var opts reset.PasswordChangeView +type UserPasswordChanger interface { + GetUserIDByName(username string) (int64, error) + GetUserIDFromUser() int64 + FillPasswordView(resetView *reset.PasswordChangeView) + ResetPassword(userID int64, resetView reset.PasswordChangeView) error +} + +type DefaultUserPasswordChanger struct{} + +func (d *DefaultUserPasswordChanger) GetUserIDByName(username string) (int64, error) { + return api.GetUsersIdByName(username) +} + +func (d *DefaultUserPasswordChanger) GetUserIDFromUser() int64 { + return prompt.GetUserIdFromUser() +} + +func (d *DefaultUserPasswordChanger) FillPasswordView(resetView *reset.PasswordChangeView) { + reset.ChangePasswordView(resetView) +} + +func (d *DefaultUserPasswordChanger) ResetPassword(userID int64, resetView reset.PasswordChangeView) error { + return api.ResetPassword(userID, resetView) +} +func ChangePassword(args []string, passwordChanger UserPasswordChanger) { + var userID int64 + var err error + resetView := &reset.PasswordChangeView{} + + if len(args) > 0 { + userID, err = passwordChanger.GetUserIDByName(args[0]) + if err != nil { + log.Errorf("failed to get user id for '%s': %v", args[0], err) + return + } + if userID == 0 { + log.Errorf("User with name '%s' not found", args[0]) + return + } + } else { + userID = passwordChanger.GetUserIDFromUser() + } + + passwordChanger.FillPasswordView(resetView) + + err = passwordChanger.ResetPassword(userID, *resetView) + if err != nil { + if isUnauthorizedError(err) { + log.Error("Permission denied: Admin privileges are required to execute this command.") + } else { + log.Errorf("failed to reset user password: %v", err) + } + } +} + +func UserPasswordChangeCmd() *cobra.Command { cmd := &cobra.Command{ Use: "password", Short: "Reset user password by name or id", Long: "Allows admin to reset the password for a specified user or select interactively if no username is provided.", Args: cobra.MinimumNArgs(0), Run: func(cmd *cobra.Command, args []string) { - var userId int64 - var err error - resetView := &reset.PasswordChangeView{ - NewPassword: opts.NewPassword, - ConfirmPassword: opts.ConfirmPassword, - } - - if len(args) > 0 { - userId, err = api.GetUsersIdByName(args[0]) - if err != nil { - log.Errorf("failed to get user id for '%s': %v", args[0], err) - return - } - if userId == 0 { - log.Errorf("User with name '%s' not found", args[0]) - return - } - } else { - userId = prompt.GetUserIdFromUser() - } - - reset.ChangePasswordView(resetView) - - err = api.ResetPassword(userId, opts) - if err != nil { - if isUnauthorizedError(err) { - log.Error("Permission denied: Admin privileges are required to execute this command.") - } else { - log.Errorf("failed to reset user password: %v", err) - } - } + d := &DefaultUserPasswordChanger{} + ChangePassword(args, d) }, } return cmd diff --git a/cmd/harbor/root/user/password_test.go b/cmd/harbor/root/user/password_test.go new file mode 100644 index 000000000..30af02017 --- /dev/null +++ b/cmd/harbor/root/user/password_test.go @@ -0,0 +1,153 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package user + +import ( + "bytes" + "fmt" + "testing" + + "github.com/goharbor/harbor-cli/pkg/views/password/reset" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" +) + +type MockUserPasswordChanger struct { + id map[string]int64 + passwords map[int64]string + userCnt int + expectAuthError bool +} + +func (m *MockUserPasswordChanger) GetUserIDByName(username string) (int64, error) { + if v, ok := m.id[username]; ok { + return v, nil + } + return 0, fmt.Errorf("username %s not found", username) +} + +func (m *MockUserPasswordChanger) GetUserIDFromUser() int64 { + return 999 +} + +func (m *MockUserPasswordChanger) FillPasswordView(resetView *reset.PasswordChangeView) { + resetView.NewPassword = "NewPass456" + resetView.ConfirmPassword = "NewPass456" +} + +func (m *MockUserPasswordChanger) ResetPassword(userID int64, resetView reset.PasswordChangeView) error { + if m.expectAuthError { + return fmt.Errorf("403") + } + if _, ok := m.passwords[userID]; !ok { + return fmt.Errorf("user %d not found", userID) + } + m.passwords[userID] = resetView.NewPassword + return nil +} + +func initMockUserPasswordChanger(userCnt int, expectAuthError bool) *MockUserPasswordChanger { + m := &MockUserPasswordChanger{ + userCnt: userCnt, + expectAuthError: expectAuthError, + id: make(map[string]int64), + passwords: make(map[int64]string), + } + for i := 0; i < userCnt; i++ { + m.id[fmt.Sprintf("test%d", i+1)] = int64(i + 1) + m.passwords[int64(i+1)] = "InitialPass123" + } + return m +} + +func TestChangePassword(t *testing.T) { + tests := []struct { + name string + setup func() *MockUserPasswordChanger + args []string + expectedPasswordID int64 + expectedNewPassword string + expectedErr string + }{ + { + name: "successfully change password by username", + setup: func() *MockUserPasswordChanger { + return initMockUserPasswordChanger(5, false) + }, + args: []string{"test1"}, + expectedPasswordID: 1, + expectedNewPassword: "NewPass456", + expectedErr: "", + }, + { + name: "change password via interactive prompt", + setup: func() *MockUserPasswordChanger { + m := initMockUserPasswordChanger(5, false) + m.id["promptuser"] = 999 + m.passwords[999] = "InitialPass123" + return m + }, + args: []string{}, + expectedPasswordID: 999, + expectedNewPassword: "NewPass456", + expectedErr: "", + }, + { + name: "user not found logs error", + setup: func() *MockUserPasswordChanger { + return initMockUserPasswordChanger(5, false) + }, + args: []string{"nonexistent"}, + expectedPasswordID: 0, + expectedNewPassword: "", + expectedErr: "failed to get user id", + }, + { + name: "permission denied error", + setup: func() *MockUserPasswordChanger { + return initMockUserPasswordChanger(5, true) + }, + args: []string{"test1"}, + expectedPasswordID: 0, + expectedNewPassword: "", + expectedErr: "Permission denied", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + originalLogOutput := log.StandardLogger().Out + log.SetOutput(&buf) + defer log.SetOutput(originalLogOutput) + + m := tt.setup() + ChangePassword(tt.args, m) + + logs := buf.String() + + if tt.expectedErr != "" { + assert.Contains(t, logs, tt.expectedErr, "Expected error logs to contain %s but got %s", tt.expectedErr, logs) + } else { + assert.Empty(t, logs, "Expected no error logs but got: %s", logs) + } + + if tt.expectedPasswordID != 0 { + password, exists := m.passwords[tt.expectedPasswordID] + assert.True(t, exists, "User with ID %d should exist", tt.expectedPasswordID) + assert.Equal(t, tt.expectedNewPassword, password, "Password for user %d should be changed to %s", tt.expectedPasswordID, tt.expectedNewPassword) + } + }) + } +} diff --git a/pkg/views/user/list/view.go b/pkg/views/user/list/view.go index 1314c28a8..2190cb8ab 100644 --- a/pkg/views/user/list/view.go +++ b/pkg/views/user/list/view.go @@ -33,7 +33,7 @@ var columns = []table.Column{ {Title: "Registration Time", Width: tablelist.WidthL}, } -func ListUsers(users []*models.UserResp) { +func MakeUserRows(users []*models.UserResp) []table.Row { var rows []table.Row for _, user := range users { isAdmin := "No" @@ -49,11 +49,15 @@ func ListUsers(users []*models.UserResp) { createdTime, }) } - + return rows +} +func ListUsers(users []*models.UserResp) error { + rows := MakeUserRows(users) m := tablelist.NewModel(columns, rows, len(rows)) + p := tea.NewProgram(m, tea.WithOutput(os.Stdout)) - if _, err := tea.NewProgram(m).Run(); err != nil { - fmt.Println("Error running program:", err) - os.Exit(1) + if _, err := p.Run(); err != nil { + return fmt.Errorf("failed to render user list: %w", err) } + return nil } diff --git a/pkg/views/user/list/view_test.go b/pkg/views/user/list/view_test.go new file mode 100644 index 000000000..6f7d7dae5 --- /dev/null +++ b/pkg/views/user/list/view_test.go @@ -0,0 +1,104 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package list + +import ( + "testing" + + "github.com/go-openapi/strfmt" + "github.com/goharbor/go-client/pkg/sdk/v2.0/models" + "github.com/goharbor/harbor-cli/pkg/utils" +) + +func TestMakeUserRows(t *testing.T) { + dateStr := "2023-01-01T12:00:00Z" + testDate, err := strfmt.ParseDateTime(dateStr) + if err != nil { + t.Fatalf("failed to parse date %q: %v", dateStr, err) + } + expectedTimeStr, err := utils.FormatCreatedTime(dateStr) + if err != nil { + t.Fatalf("failed to format created time %q: %v", dateStr, err) + } + tests := []struct { + name string + setup func() []*models.UserResp + expected [][]string + }{ + { + name: "Number of users non-zero", + setup: func() []*models.UserResp { + return []*models.UserResp{ + { + UserID: 1, + Username: "testUser1", + Email: "test1@domain.com", + SysadminFlag: true, + Realname: "Test1", + CreationTime: testDate, + }, + { + UserID: 2, + Username: "testUser2", + Email: "test2@domain.com", + SysadminFlag: false, + Realname: "Test2", + CreationTime: testDate, + }, + { + UserID: 3, + Username: "testUser3", + Email: "test3@domain.com", + SysadminFlag: false, + Realname: "Test3", + CreationTime: testDate, + }, + } + }, + expected: [][]string{ + {"1", "testUser1", "Yes", "test1@domain.com", expectedTimeStr}, + {"2", "testUser2", "No", "test2@domain.com", expectedTimeStr}, + {"3", "testUser3", "No", "test3@domain.com", expectedTimeStr}, + }, + }, + { + name: "No users", + setup: func() []*models.UserResp { + return []*models.UserResp{} + }, + expected: [][]string{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + users := tt.setup() + rows := MakeUserRows(users) + if len(tt.expected) != len(rows) { + t.Fatalf("MakeUserRows returned %d rows for %d users", len(rows), len(users)) + } + for i := 0; i < len(rows); i++ { + if len(rows[i]) != len(tt.expected[i]) { + t.Errorf("Row %d: expected %d columns, got %d", i, len(tt.expected[i]), len(rows[i])) + continue + } + for j := 0; j < len(rows[i]); j++ { + if rows[i][j] != tt.expected[i][j] { + t.Errorf("Row %d, Column %d: expected '%s', but got '%s'", + i, j, tt.expected[i][j], rows[i][j]) + } + } + } + }) + } +}