diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index 5b9906187..a61bc1dd3 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -218,12 +218,12 @@ func validateClientConnection(client *client.HarborAPI) error { } errorCode := utils.ParseHarborErrorCode(err) - // 401/403 = definite auth failure - if errorCode == "401" || errorCode == "403" { + // 401 = definite auth failure + if errorCode == "401" { return fmt.Errorf("authentication failed, check your credentials: %v", utils.ParseHarborErrorMsg(err)) } - // For other errors (e.g. 412 for robot/OIDC accounts, 5xx), + // For other errors (e.g. 403 for robot accounts, 412 for OIDC accounts, 5xx, or deserialization errors), // fall back to secondary endpoints to verify creds and reachability. _, projectErr := client.Project.ListProjects(ctx, &project.ListProjectsParams{ Page: new(int64(1)), @@ -231,16 +231,18 @@ func validateClientConnection(client *client.HarborAPI) error { }) _, pingErr := client.Ping.GetPing(ctx, &ping.GetPingParams{}) - // If either secondary check returns 401/403, creds are bad. + // If either secondary check returns 401, creds are bad. if projectErr != nil { projCode := utils.ParseHarborErrorCode(projectErr) - if projCode == "401" || projCode == "403" { + if projCode == "401" { return fmt.Errorf("authentication failed, check your credentials: %v", utils.ParseHarborErrorMsg(projectErr)) } } - // Both passed → creds valid, server reachable - if projectErr == nil && pingErr == nil { + // If Ping succeeds, the server is reachable. + // If ListProjects succeeded (nil) or returned 403 (valid but restricted robot account), + // we consider the client connection validated. + if pingErr == nil && (projectErr == nil || utils.ParseHarborErrorCode(projectErr) == "403") { return nil } diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 22ae6d1ec..45bedb306 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -14,6 +14,8 @@ package root_test import ( + "net/http" + "net/http/httptest" "testing" "github.com/goharbor/harbor-cli/cmd/harbor/root" @@ -122,3 +124,115 @@ func Test_Login_Failure_MutuallyExclusiveFlags(t *testing.T) { err := cmd.Execute() assert.Error(t, err, "Expected error when both --password and --password-stdin are set") } + +func Test_Login_Validation_MockServer(t *testing.T) { + tests := []struct { + name string + currentUserStatus int + currentUserResponse string + projectsStatus int + projectsResponse string + pingStatus int + pingResponse string + expectError bool + expectedErrSubstr string + }{ + { + name: "Human Login Success", + currentUserStatus: http.StatusOK, + currentUserResponse: `{"username": "human-user", "sysadmin_flag": false}`, + expectError: false, + }, + { + name: "Invalid Credentials (401)", + currentUserStatus: http.StatusUnauthorized, + currentUserResponse: `{"errors": [{"code": "UNAUTHORIZED", "message": "unauthorized"}]}`, + expectError: true, + expectedErrSubstr: "authentication failed, check your credentials", + }, + { + name: "Robot Login Success (Standard fallback)", + currentUserStatus: http.StatusForbidden, + currentUserResponse: `{"errors": [{"code": "FORBIDDEN", "message": "forbidden"}]}`, + projectsStatus: http.StatusOK, + projectsResponse: `[]`, + pingStatus: http.StatusOK, + pingResponse: `"pong"`, + expectError: false, + }, + { + name: "Robot Login Success (Restricted project access 403 fallback)", + currentUserStatus: http.StatusForbidden, + currentUserResponse: `{"errors": [{"code": "FORBIDDEN", "message": "forbidden"}]}`, + projectsStatus: http.StatusForbidden, + projectsResponse: `{"errors": [{"code": "FORBIDDEN", "message": "forbidden"}]}`, + pingStatus: http.StatusOK, + pingResponse: `"pong"`, + expectError: false, + }, + { + name: "Robot Login Failure (Bad credentials 401 on fallback)", + currentUserStatus: http.StatusForbidden, + currentUserResponse: `{"errors": [{"code": "FORBIDDEN", "message": "forbidden"}]}`, + projectsStatus: http.StatusUnauthorized, + projectsResponse: `{"errors": [{"code": "UNAUTHORIZED", "message": "unauthorized"}]}`, + pingStatus: http.StatusOK, + pingResponse: `"pong"`, + expectError: true, + expectedErrSubstr: "authentication failed, check your credentials", + }, + { + name: "Server Error (500)", + currentUserStatus: http.StatusInternalServerError, + currentUserResponse: `Internal Server Error`, + projectsStatus: http.StatusInternalServerError, + projectsResponse: `Internal Server Error`, + pingStatus: http.StatusInternalServerError, + pingResponse: `Internal Server Error`, + expectError: true, + expectedErrSubstr: "server error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/v2.0/users/current": + w.WriteHeader(tt.currentUserStatus) + w.Write([]byte(tt.currentUserResponse)) + case "/api/v2.0/projects": + w.WriteHeader(tt.projectsStatus) + w.Write([]byte(tt.projectsResponse)) + case "/api/v2.0/ping": + w.WriteHeader(tt.pingStatus) + w.Write([]byte(tt.pingResponse)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + cmd := root.LoginCommand() + cmd.SetArgs([]string{server.URL}) + + assert.NoError(t, cmd.Flags().Set("username", "test-user")) + assert.NoError(t, cmd.Flags().Set("password", "test-password")) + + err := cmd.Execute() + if tt.expectError { + assert.Error(t, err) + if tt.expectedErrSubstr != "" { + assert.Contains(t, err.Error(), tt.expectedErrSubstr) + } + } else { + assert.NoError(t, err) + } + }) + } +}