fix(login): avoid false login failures for robot accounts

cmd/harbor/root/login.go

@@ -218,29 +218,31 @@ 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)),
 		PageSize: new(int64(1)),
 	})
 	_, 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
 	}
 

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)
+			}
+		})
+	}
+}