From e5c2d8aa7e45a4d71bbf236fe7a2a98c27cd88ec Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sun, 14 Jun 2026 12:30:38 +0530 Subject: [PATCH 01/25] basic implementation Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 45 +++++++++ cmd/harbor/root/login_test.go | 20 ++++ pkg/utils/client.go | 40 ++++++++ pkg/utils/config.go | 102 +++++++++++++++++++- pkg/utils/config_test.go | 24 +++++ pkg/utils/oidc.go | 169 ++++++++++++++++++++++++++++++++++ pkg/utils/oidc_test.go | 79 ++++++++++++++++ 7 files changed, 475 insertions(+), 4 deletions(-) create mode 100644 pkg/utils/oidc.go create mode 100644 pkg/utils/oidc_test.go diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index e79c77561..663dce475 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -18,6 +18,7 @@ import ( "fmt" "os" "strings" + "time" "github.com/goharbor/go-client/pkg/harbor" "github.com/goharbor/go-client/pkg/sdk/v2.0/client" @@ -38,6 +39,7 @@ var ( Name string passwordStdin bool skipVerifyClient bool + oidcLogin bool ) // LoginCommand creates a new `harbor login` command @@ -52,6 +54,10 @@ func LoginCommand() *cobra.Command { serverAddress = args[0] } + if oidcLogin { + return RunOIDCLogin(serverAddress) + } + if passwordStdin { fmt.Print("Password: ") passwordBytes, err := term.ReadPassword(int(os.Stdin.Fd())) // #nosec G115 - fd fits in int on all supported platforms @@ -87,9 +93,13 @@ func LoginCommand() *cobra.Command { flags.StringVarP(&Name, "context-name", "n", "", "Login context name (optional)") flags.StringVarP(&Password, "password", "p", "", "Password (not recommended, use --password-stdin for better security)") flags.BoolVar(&passwordStdin, "password-stdin", false, "Take the password from stdin") + flags.BoolVar(&oidcLogin, "oidc", false, "Authenticate using Harbor OIDC browser login") flags.BoolVarP(&skipVerifyClient, "skip-verify-client", "", false, "Skip whether the clients basic auth credentials shall be validated against the Harbor server during login. This is not recommended as it may lead to storing invalid credentials. Use this flag if you want to skip validation of credentials during login, for example, when the Harbor server is not reachable at the moment of login but you still want to store the credentials for later use.") cmd.MarkFlagsMutuallyExclusive("password", "password-stdin") + cmd.MarkFlagsMutuallyExclusive("oidc", "username") + cmd.MarkFlagsMutuallyExclusive("oidc", "password") + cmd.MarkFlagsMutuallyExclusive("oidc", "password-stdin") return cmd } @@ -208,6 +218,41 @@ func RunLogin(opts login.LoginView) error { return nil } +func RunOIDCLogin(serverAddress string) error { + if serverAddress == "" { + return fmt.Errorf("server address is required for OIDC login") + } + serverAddress = utils.FormatUrl(serverAddress) + if err := utils.ValidateURL(serverAddress); err != nil { + return fmt.Errorf("invalid server URL: %w", err) + } + + loginResp, err := utils.InitiateOIDCLogin(serverAddress) + if err != nil { + return err + } + + fmt.Printf("Open this URL in your browser to authenticate:\n\n %s\n\n", loginResp.RedirectURL) + fmt.Print("Waiting for authentication...\n") + + tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.TransactionID, 10*time.Minute) + if err != nil { + return err + } + + harborData, err := utils.GetCurrentHarborData() + if err != nil { + return fmt.Errorf("failed to get current harbor data: %s", err) + } + + if err := utils.AddOIDCCredentials(serverAddress, tokenResp.Username, tokenResp.IDToken, tokenResp.RefreshToken, tokenResp.ExpiresAt, harborData.ConfigPath); err != nil { + return fmt.Errorf("failed to store OIDC credential: %w", err) + } + + fmt.Printf("Login successful for %s at %s\n", tokenResp.Username, serverAddress) + return nil +} + func validateClientConnection(client *client.HarborAPI) error { ctx := context.Background() diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 22ae6d1ec..50344b8ad 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -122,3 +122,23 @@ 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_Failure_OIDCMutuallyExclusiveFlags(t *testing.T) { + tempDir := t.TempDir() + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + cmd := root.LoginCommand() + cmd.SetArgs([]string{"http://demo.goharbor.io"}) + + assert.NoError(t, cmd.Flags().Set("oidc", "true")) + assert.NoError(t, cmd.Flags().Set("username", "admin")) + + err := cmd.Execute() + assert.Error(t, err, "Expected error when --oidc and --username are set") +} + +func Test_RunOIDCLogin_Failure_MissingServer(t *testing.T) { + err := root.RunOIDCLogin("") + assert.Error(t, err) +} diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 8929661ca..03fefc2fc 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -16,8 +16,14 @@ package utils import ( "context" "fmt" + "net/url" + "strings" "sync" + "time" + "github.com/go-openapi/runtime" + httptransport "github.com/go-openapi/runtime/client" + "github.com/go-openapi/strfmt" "github.com/goharbor/go-client/pkg/harbor" v2client "github.com/goharbor/go-client/pkg/sdk/v2.0/client" log "github.com/sirupsen/logrus" @@ -75,6 +81,9 @@ func GetClientByCredentialName(credentialName string) (*v2client.HarborAPI, erro if err != nil { return nil, fmt.Errorf("failed to get credential %s: %w", credentialName, err) } + if credential.AuthType == AuthTypeOIDC { + return getOIDCClient(credential) + } // Get encryption key key, err := GetEncryptionKey() @@ -95,3 +104,34 @@ func GetClientByCredentialName(credentialName string) (*v2client.HarborAPI, erro } return GetClientByConfig(clientConfig), nil } + +func getOIDCClient(credential Credential) (*v2client.HarborAPI, error) { + idToken, err := GetDecryptedIDToken(credential.Name) + if err != nil { + return nil, err + } + + if credential.ExpiresAt > 0 && time.Now().Unix() >= credential.ExpiresAt-60 { + return nil, fmt.Errorf("OIDC session expired or is about to expire. Please run `harbor login %s --oidc` again", credential.ServerAddress) + } + + return buildOIDCClient(credential.ServerAddress, idToken) +} + +func buildOIDCClient(serverAddress, idToken string) (*v2client.HarborAPI, error) { + u, err := url.Parse(serverAddress) + if err != nil { + return nil, fmt.Errorf("failed to parse server URL: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return nil, fmt.Errorf("invalid server URL: %s", serverAddress) + } + + basePath := strings.TrimRight(u.Path, "/") + transport := httptransport.New(u.Host, basePath, []string{u.Scheme}) + transport.DefaultAuthentication = runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { + return req.SetHeaderParam("Authorization", "Bearer "+idToken) + }) + + return v2client.New(transport, strfmt.Default), nil +} diff --git a/pkg/utils/config.go b/pkg/utils/config.go index cecc7628e..2b3f1248f 100644 --- a/pkg/utils/config.go +++ b/pkg/utils/config.go @@ -27,12 +27,21 @@ import ( ) type Credential struct { - Name string `yaml:"name"` - Username string `yaml:"username"` - Password string `yaml:"password"` - ServerAddress string `yaml:"serveraddress"` + Name string `mapstructure:"name" yaml:"name"` + Username string `mapstructure:"username" yaml:"username"` + Password string `mapstructure:"password,omitempty" yaml:"password,omitempty"` + ServerAddress string `mapstructure:"serveraddress" yaml:"serveraddress"` + AuthType string `mapstructure:"auth-type,omitempty" yaml:"auth-type,omitempty"` + IDToken string `mapstructure:"id-token,omitempty" yaml:"id-token,omitempty"` + RefreshToken string `mapstructure:"refresh-token,omitempty" yaml:"refresh-token,omitempty"` + ExpiresAt int64 `mapstructure:"expires-at,omitempty" yaml:"expires-at,omitempty"` } +const ( + AuthTypeBasic = "basic" + AuthTypeOIDC = "oidc" +) + type HarborConfig struct { CurrentCredentialName string `mapstructure:"current-credential-name" yaml:"current-credential-name"` Credentials []Credential `mapstructure:"credentials" yaml:"credentials"` @@ -505,6 +514,7 @@ func AddCredentialsToConfigFile(credential Credential, configPath string) error log.Fatalf("failed to write updated config file: %v", err) } + CurrentHarborConfig = &c fmt.Printf("Added credential '%s' to config file at %s\n", credential.Name, configPath) return nil } @@ -550,7 +560,91 @@ func UpdateCredentialsInConfigFile(updatedCredential Credential, configPath stri log.Fatalf("failed to write updated config file: %v", err) } + CurrentHarborConfig = &c fmt.Printf("Updated credential '%s' in config file at %s.\n", updatedCredential.Name, configPath) fmt.Printf("Switched to context '%s'\n", updatedCredential.Name) return nil } + +func AddOIDCCredentials(serverAddress, username, idToken, refreshToken string, expiresAt int64, configPath string) error { + if err := GenerateEncryptionKey(); err != nil { + return fmt.Errorf("failed to generate encryption key: %w", err) + } + key, err := GetEncryptionKey() + if err != nil { + return fmt.Errorf("failed to get encryption key: %w", err) + } + + encryptedIDToken, err := Encrypt(key, []byte(idToken)) + if err != nil { + return fmt.Errorf("failed to encrypt id token: %w", err) + } + + var encryptedRefreshToken string + if refreshToken != "" { + encryptedRefreshToken, err = Encrypt(key, []byte(refreshToken)) + if err != nil { + return fmt.Errorf("failed to encrypt refresh token: %w", err) + } + } + + credential := Credential{ + Name: DefaultCredentialName(username, serverAddress), + Username: username, + ServerAddress: serverAddress, + AuthType: AuthTypeOIDC, + IDToken: encryptedIDToken, + RefreshToken: encryptedRefreshToken, + ExpiresAt: expiresAt, + } + + if _, err := GetCredentials(credential.Name); err == nil { + return UpdateCredentialsInConfigFile(credential, configPath) + } + return AddCredentialsToConfigFile(credential, configPath) +} + +func GetDecryptedIDToken(credentialName string) (string, error) { + credential, err := GetCredentials(credentialName) + if err != nil { + return "", err + } + if credential.AuthType != AuthTypeOIDC { + return "", fmt.Errorf("credential %q is not an OIDC credential", credentialName) + } + key, err := GetEncryptionKey() + if err != nil { + return "", fmt.Errorf("failed to get encryption key: %w", err) + } + return Decrypt(key, credential.IDToken) +} + +func UpdateOIDCTokens(credentialName, idToken, refreshToken string, expiresAt int64, configPath string) error { + credential, err := GetCredentials(credentialName) + if err != nil { + return err + } + if credential.AuthType != AuthTypeOIDC { + return fmt.Errorf("credential %q is not an OIDC credential", credentialName) + } + key, err := GetEncryptionKey() + if err != nil { + return fmt.Errorf("failed to get encryption key: %w", err) + } + encryptedIDToken, err := Encrypt(key, []byte(idToken)) + if err != nil { + return fmt.Errorf("failed to encrypt id token: %w", err) + } + credential.IDToken = encryptedIDToken + credential.ExpiresAt = expiresAt + + if refreshToken != "" { + encryptedRefreshToken, err := Encrypt(key, []byte(refreshToken)) + if err != nil { + return fmt.Errorf("failed to encrypt refresh token: %w", err) + } + credential.RefreshToken = encryptedRefreshToken + } + + return UpdateCredentialsInConfigFile(credential, configPath) +} diff --git a/pkg/utils/config_test.go b/pkg/utils/config_test.go index 11fba03f3..d84bbbfed 100644 --- a/pkg/utils/config_test.go +++ b/pkg/utils/config_test.go @@ -107,3 +107,27 @@ func Test_Config_Flag(t *testing.T) { assert.NotNil(t, currentConfig.Credentials, "Credentials should not be nil") assert.NotNil(t, data.ConfigPath, "ConfigPath should not be nil") } + +func Test_AddOIDCCredentials(t *testing.T) { + tempDir := t.TempDir() + helpers.SetMockKeyring(t) + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + err := utils.AddOIDCCredentials("https://demo.goharbor.io", "alice", "id-token", "refresh-token", 12345, data.ConfigPath) + assert.NoError(t, err) + + cred, err := utils.GetCredentials("alice@https-demo-goharbor-io") + assert.NoError(t, err) + assert.Equal(t, utils.AuthTypeOIDC, cred.AuthType) + assert.Equal(t, "alice", cred.Username) + assert.Equal(t, "https://demo.goharbor.io", cred.ServerAddress) + assert.Equal(t, int64(12345), cred.ExpiresAt) + assert.NotEmpty(t, cred.IDToken) + assert.NotEmpty(t, cred.RefreshToken) + assert.Empty(t, cred.Password) + + idToken, err := utils.GetDecryptedIDToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "id-token", idToken) +} diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go new file mode 100644 index 000000000..28eee857e --- /dev/null +++ b/pkg/utils/oidc.go @@ -0,0 +1,169 @@ +// 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 utils + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "time" +) + +const ( + oidcCLILoginPath = "/c/oidc/cli/login" + oidcCLITokenPath = "/c/oidc/cli-token" +) + +type OIDCLoginResponse struct { + RedirectURL string `json:"redirect_url"` + TransactionID string `json:"transaction_id"` +} + +type OIDCPollResponse struct { + Status string `json:"status"` + IDToken string `json:"id_token,omitempty"` + RefreshToken string `json:"refresh_token,omitempty"` + Username string `json:"username,omitempty"` + ExpiresAt int64 `json:"expires_at,omitempty"` + Error string `json:"error,omitempty"` +} + +func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { + serverAddress = FormatUrl(serverAddress) + if err := ValidateURL(serverAddress); err != nil { + return nil, fmt.Errorf("invalid server URL: %w", err) + } + + endpoint, err := joinServerPath(serverAddress, oidcCLILoginPath) + if err != nil { + return nil, err + } + + resp, err := http.Get(endpoint) //nolint:gosec // endpoint is user-provided Harbor server URL for login. + if err != nil { + return nil, fmt.Errorf("failed to initiate OIDC login: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return nil, fmt.Errorf("failed to initiate OIDC login: status %d: %s", resp.StatusCode, string(body)) + } + + var loginResp OIDCLoginResponse + if err := json.NewDecoder(resp.Body).Decode(&loginResp); err != nil { + return nil, fmt.Errorf("failed to decode OIDC login response: %w", err) + } + if loginResp.RedirectURL == "" || loginResp.TransactionID == "" { + return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or transaction_id") + } + return &loginResp, nil +} + +func PollForOIDCToken(serverAddress, transactionID string, timeout time.Duration) (*OIDCPollResponse, error) { + if transactionID == "" { + return nil, fmt.Errorf("transaction ID is required") + } + serverAddress = FormatUrl(serverAddress) + if err := ValidateURL(serverAddress); err != nil { + return nil, fmt.Errorf("invalid server URL: %w", err) + } + + endpoint, err := joinServerPath(serverAddress, oidcCLITokenPath) + if err != nil { + return nil, err + } + u, err := url.Parse(endpoint) + if err != nil { + return nil, fmt.Errorf("failed to parse OIDC token endpoint: %w", err) + } + q := u.Query() + q.Set("transaction_id", transactionID) + u.RawQuery = q.Encode() + + deadline := time.Now().Add(timeout) + ticker := time.NewTicker(3 * time.Second) + defer ticker.Stop() + + for { + result, ready, err := pollOIDCTokenOnce(u.String()) + if err != nil { + return nil, err + } + if ready { + return result, nil + } + if time.Now().After(deadline) { + return nil, fmt.Errorf("timed out waiting for OIDC authentication") + } + remaining := time.Until(deadline) + if remaining <= 0 { + return nil, fmt.Errorf("timed out waiting for OIDC authentication") + } + select { + case <-ticker.C: + case <-time.After(remaining): + return nil, fmt.Errorf("timed out waiting for OIDC authentication") + } + } +} + +func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { + resp, err := http.Get(endpoint) //nolint:gosec // endpoint is the Harbor server URL validated by PollForOIDCToken. + if err != nil { + return nil, false, fmt.Errorf("failed to poll OIDC token: %w", err) + } + defer resp.Body.Close() + + var pollResp OIDCPollResponse + switch resp.StatusCode { + case http.StatusAccepted: + return &OIDCPollResponse{Status: "pending"}, false, nil + case http.StatusOK: + if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { + return nil, false, fmt.Errorf("failed to decode OIDC token response: %w", err) + } + if pollResp.Status != "ready" { + return nil, false, fmt.Errorf("unexpected OIDC token status: %s", pollResp.Status) + } + if pollResp.IDToken == "" || pollResp.Username == "" { + return nil, false, fmt.Errorf("invalid OIDC token response: missing id_token or username") + } + return &pollResp, true, nil + case http.StatusBadRequest: + if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { + return nil, false, fmt.Errorf("OIDC authentication failed") + } + if pollResp.Error != "" { + return nil, false, fmt.Errorf("OIDC authentication failed: %s", pollResp.Error) + } + return nil, false, fmt.Errorf("OIDC authentication failed") + default: + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return nil, false, fmt.Errorf("failed to poll OIDC token: status %d: %s", resp.StatusCode, string(body)) + } +} + +func joinServerPath(serverAddress, path string) (string, error) { + u, err := url.Parse(serverAddress) + if err != nil { + return "", fmt.Errorf("failed to parse server URL: %w", err) + } + u.Path = path + u.RawQuery = "" + u.Fragment = "" + return u.String(), nil +} diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go new file mode 100644 index 000000000..177c4b52c --- /dev/null +++ b/pkg/utils/oidc_test.go @@ -0,0 +1,79 @@ +// 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 utils_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/goharbor/harbor-cli/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestInitiateOIDCLogin(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/c/oidc/cli/login", r.URL.Path) + _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + RedirectURL: "https://idp.example/authorize", + TransactionID: "tx-1", + }) + })) + defer server.Close() + + resp, err := utils.InitiateOIDCLogin(server.URL) + + require.NoError(t, err) + assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) + assert.Equal(t, "tx-1", resp.TransactionID) +} + +func TestPollForOIDCTokenReady(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) + assert.Equal(t, "tx-1", r.URL.Query().Get("transaction_id")) + _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "ready", + IDToken: "id-token", + Username: "alice", + }) + })) + defer server.Close() + + resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + + require.NoError(t, err) + assert.Equal(t, "ready", resp.Status) + assert.Equal(t, "id-token", resp.IDToken) + assert.Equal(t, "alice", resp.Username) +} + +func TestPollForOIDCTokenFailed(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "failed", + Error: "state expired", + }) + })) + defer server.Close() + + resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + + assert.Nil(t, resp) + assert.ErrorContains(t, err, "state expired") +} From 261a1bda7f762d74112ddf34bde18a37014aee81 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Mon, 15 Jun 2026 23:20:08 +0530 Subject: [PATCH 02/25] moved to state based login with existing endopoint Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 2 +- cmd/harbor/root/login_test.go | 39 +++++++++++++++++++++++++++++++++++ pkg/utils/client.go | 4 ++-- pkg/utils/oidc.go | 27 +++++++++++++++--------- pkg/utils/oidc_test.go | 15 +++++++------- 5 files changed, 67 insertions(+), 20 deletions(-) diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index 663dce475..a21a074ea 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -235,7 +235,7 @@ func RunOIDCLogin(serverAddress string) error { fmt.Printf("Open this URL in your browser to authenticate:\n\n %s\n\n", loginResp.RedirectURL) fmt.Print("Waiting for authentication...\n") - tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.TransactionID, 10*time.Minute) + tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.State, 10*time.Minute) if err != nil { return err } diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 50344b8ad..91b6ed74f 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -14,10 +14,14 @@ package root_test import ( + "encoding/json" + "net/http" + "net/http/httptest" "testing" "github.com/goharbor/harbor-cli/cmd/harbor/root" helpers "github.com/goharbor/harbor-cli/test/helper" + "github.com/goharbor/harbor-cli/pkg/utils" "github.com/stretchr/testify/assert" ) @@ -142,3 +146,38 @@ func Test_RunOIDCLogin_Failure_MissingServer(t *testing.T) { err := root.RunOIDCLogin("") assert.Error(t, err) } + +func Test_RunOIDCLogin_Success(t *testing.T) { + tempDir := t.TempDir() + helpers.SetMockKeyring(t) + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/c/oidc/login": + assert.Equal(t, "cli", r.URL.Query().Get("mode")) + assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + RedirectURL: "https://idp.example/authorize", + State: "state-1", + })) + case "/c/oidc/cli-token": + assert.Equal(t, "state-1", r.URL.Query().Get("state")) + assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "ready", + IDToken: "id-token", + Username: "alice", + })) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + err := root.RunOIDCLogin(server.URL) + assert.NoError(t, err) + + cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", server.URL)) + assert.NoError(t, err) + assert.Equal(t, utils.AuthTypeOIDC, cred.AuthType) +} diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 03fefc2fc..3dbca16a4 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -115,10 +115,10 @@ func getOIDCClient(credential Credential) (*v2client.HarborAPI, error) { return nil, fmt.Errorf("OIDC session expired or is about to expire. Please run `harbor login %s --oidc` again", credential.ServerAddress) } - return buildOIDCClient(credential.ServerAddress, idToken) + return buildClientWithToken(credential.ServerAddress, idToken) } -func buildOIDCClient(serverAddress, idToken string) (*v2client.HarborAPI, error) { +func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, error) { u, err := url.Parse(serverAddress) if err != nil { return nil, fmt.Errorf("failed to parse server URL: %w", err) diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index 28eee857e..1a152b392 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -23,13 +23,13 @@ import ( ) const ( - oidcCLILoginPath = "/c/oidc/cli/login" + oidcCLILoginPath = "/c/oidc/login" oidcCLITokenPath = "/c/oidc/cli-token" ) type OIDCLoginResponse struct { - RedirectURL string `json:"redirect_url"` - TransactionID string `json:"transaction_id"` + RedirectURL string `json:"redirect_url"` + State string `json:"state"` } type OIDCPollResponse struct { @@ -51,8 +51,15 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { if err != nil { return nil, err } + u, err := url.Parse(endpoint) + if err != nil { + return nil, fmt.Errorf("failed to parse OIDC login endpoint: %w", err) + } + q := u.Query() + q.Set("mode", "cli") + u.RawQuery = q.Encode() - resp, err := http.Get(endpoint) //nolint:gosec // endpoint is user-provided Harbor server URL for login. + resp, err := http.Get(u.String()) //nolint:gosec // endpoint is user-provided Harbor server URL for login. if err != nil { return nil, fmt.Errorf("failed to initiate OIDC login: %w", err) } @@ -67,15 +74,15 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { if err := json.NewDecoder(resp.Body).Decode(&loginResp); err != nil { return nil, fmt.Errorf("failed to decode OIDC login response: %w", err) } - if loginResp.RedirectURL == "" || loginResp.TransactionID == "" { - return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or transaction_id") + if loginResp.RedirectURL == "" || loginResp.State == "" { + return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or state") } return &loginResp, nil } -func PollForOIDCToken(serverAddress, transactionID string, timeout time.Duration) (*OIDCPollResponse, error) { - if transactionID == "" { - return nil, fmt.Errorf("transaction ID is required") +func PollForOIDCToken(serverAddress, state string, timeout time.Duration) (*OIDCPollResponse, error) { + if state == "" { + return nil, fmt.Errorf("state is required") } serverAddress = FormatUrl(serverAddress) if err := ValidateURL(serverAddress); err != nil { @@ -91,7 +98,7 @@ func PollForOIDCToken(serverAddress, transactionID string, timeout time.Duration return nil, fmt.Errorf("failed to parse OIDC token endpoint: %w", err) } q := u.Query() - q.Set("transaction_id", transactionID) + q.Set("state", state) u.RawQuery = q.Encode() deadline := time.Now().Add(timeout) diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 177c4b52c..4508738e7 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -27,10 +27,11 @@ import ( func TestInitiateOIDCLogin(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/c/oidc/cli/login", r.URL.Path) + assert.Equal(t, "/c/oidc/login", r.URL.Path) + assert.Equal(t, "cli", r.URL.Query().Get("mode")) _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ - RedirectURL: "https://idp.example/authorize", - TransactionID: "tx-1", + RedirectURL: "https://idp.example/authorize", + State: "state-1", }) })) defer server.Close() @@ -39,13 +40,13 @@ func TestInitiateOIDCLogin(t *testing.T) { require.NoError(t, err) assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) - assert.Equal(t, "tx-1", resp.TransactionID) + assert.Equal(t, "state-1", resp.State) } func TestPollForOIDCTokenReady(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) - assert.Equal(t, "tx-1", r.URL.Query().Get("transaction_id")) + assert.Equal(t, "state-1", r.URL.Query().Get("state")) _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", @@ -54,7 +55,7 @@ func TestPollForOIDCTokenReady(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) require.NoError(t, err) assert.Equal(t, "ready", resp.Status) @@ -72,7 +73,7 @@ func TestPollForOIDCTokenFailed(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) assert.Nil(t, resp) assert.ErrorContains(t, err, "state expired") From 2423ed74da58d628a7cfb0fbf4358fbb7ca3fc66 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sat, 20 Jun 2026 16:12:58 +0530 Subject: [PATCH 03/25] fix client error Signed-off-by: Nishchay Rajput --- pkg/utils/client.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 3dbca16a4..4720dd5af 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -17,12 +17,10 @@ import ( "context" "fmt" "net/url" - "strings" "sync" "time" "github.com/go-openapi/runtime" - httptransport "github.com/go-openapi/runtime/client" "github.com/go-openapi/strfmt" "github.com/goharbor/go-client/pkg/harbor" v2client "github.com/goharbor/go-client/pkg/sdk/v2.0/client" @@ -127,11 +125,12 @@ func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, e return nil, fmt.Errorf("invalid server URL: %s", serverAddress) } - basePath := strings.TrimRight(u.Path, "/") - transport := httptransport.New(u.Host, basePath, []string{u.Scheme}) - transport.DefaultAuthentication = runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { - return req.SetHeaderParam("Authorization", "Bearer "+idToken) - }) + cfg := &harbor.Config{ + URL: u, + AuthInfo: runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { + return req.SetHeaderParam("Authorization", "Bearer "+idToken) + }), + } - return v2client.New(transport, strfmt.Default), nil + return v2client.New(cfg.ToV2Config()), nil } From 6032e1076d8859597d51cfa82532d93625ca68b7 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Fri, 3 Jul 2026 23:34:23 +0530 Subject: [PATCH 04/25] basic refresh token support Signed-off-by: Nishchay Rajput --- pkg/utils/client.go | 218 ++++++++++++++++++++++++++++++++++++++- pkg/utils/config.go | 20 +++- pkg/utils/config_test.go | 15 +++ pkg/utils/oidc.go | 70 ++++++++++++- pkg/utils/oidc_test.go | 30 ++++++ 5 files changed, 347 insertions(+), 6 deletions(-) diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 4720dd5af..5a953bd0e 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -15,8 +15,13 @@ package utils import ( "context" + "encoding/base64" + "encoding/json" "fmt" + "io" + "net/http" "net/url" + "strings" "sync" "time" @@ -33,6 +38,22 @@ var ( ClientErr error ) +const oidcRefreshFailureMessage = "Unable to refresh OIDC session. Please try again or run 'harbor login --oidc'." + +const oidcRetryHeader = "X-Harbor-CLI-OIDC-Retry" + +type oidcTokenManager struct { + mu sync.RWMutex + credential Credential + token string + refreshFn func(Credential) (string, error) +} + +type oidcRetryTransport struct { + base http.RoundTripper + tokenManager *oidcTokenManager +} + func GetClient() (*v2client.HarborAPI, error) { ClientOnce.Do(func() { config, err := GetCurrentHarborConfig() @@ -109,11 +130,112 @@ func getOIDCClient(credential Credential) (*v2client.HarborAPI, error) { return nil, err } - if credential.ExpiresAt > 0 && time.Now().Unix() >= credential.ExpiresAt-60 { - return nil, fmt.Errorf("OIDC session expired or is about to expire. Please run `harbor login %s --oidc` again", credential.ServerAddress) + if oidcTokenNeedsRefresh(credential, idToken) { + idToken, err = refreshOIDCCredential(credential) + if err != nil { + return nil, err + } + } + + return buildOIDCClient(credential, idToken) +} + +func refreshOIDCCredential(credential Credential) (string, error) { + refreshToken, err := GetDecryptedRefreshToken(credential.Name) + if err != nil { + return "", fmt.Errorf("failed to load OIDC refresh token: %w", err) + } + if refreshToken == "" { + return "", fmt.Errorf(oidcRefreshFailureMessage) + } + + refreshResp, err := RefreshOIDCToken(credential.ServerAddress, refreshToken) + if err != nil { + log.WithError(err).Warn("failed to refresh OIDC token") + return "", fmt.Errorf(oidcRefreshFailureMessage) + } + + nextRefreshToken := refreshResp.RefreshToken + if nextRefreshToken == "" { + nextRefreshToken = refreshToken + } + + harborData, err := GetCurrentHarborData() + if err != nil { + return "", fmt.Errorf("failed to get current Harbor data: %w", err) + } + if err := UpdateOIDCTokens(credential.Name, refreshResp.IDToken, nextRefreshToken, refreshResp.ExpiresAt, harborData.ConfigPath); err != nil { + return "", fmt.Errorf("failed to persist refreshed OIDC tokens: %w", err) + } + + return refreshResp.IDToken, nil +} + +func oidcTokenNeedsRefresh(credential Credential, idToken string) bool { + expiresAt, err := oidcTokenExpiryUnix(idToken) + if err != nil { + expiresAt = credential.ExpiresAt + } + if expiresAt <= 0 { + return false + } + return time.Now().Unix() >= expiresAt-60 +} + +func oidcTokenExpiryUnix(idToken string) (int64, error) { + parts := strings.Split(idToken, ".") + if len(parts) != 3 { + return 0, fmt.Errorf("invalid JWT format") + } + + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return 0, fmt.Errorf("failed to decode JWT payload: %w", err) + } + + var claims struct { + Exp int64 `json:"exp"` + } + if err := json.Unmarshal(payload, &claims); err != nil { + return 0, fmt.Errorf("failed to unmarshal JWT payload: %w", err) + } + if claims.Exp <= 0 { + return 0, fmt.Errorf("JWT exp claim is missing") + } + return claims.Exp, nil +} + +func buildOIDCClient(credential Credential, idToken string) (*v2client.HarborAPI, error) { + tokenManager := &oidcTokenManager{ + credential: credential, + token: idToken, + refreshFn: refreshOIDCCredential, + } + + return buildClientWithAuth(credential.ServerAddress, tokenManager, &oidcRetryTransport{ + base: harbor.InsecureTransport, + tokenManager: tokenManager, + }) +} + +func buildClientWithAuth(serverAddress string, tokenManager *oidcTokenManager, transport http.RoundTripper) (*v2client.HarborAPI, error) { + u, err := url.Parse(serverAddress) + if err != nil { + return nil, fmt.Errorf("failed to parse server URL: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return nil, fmt.Errorf("invalid server URL: %s", serverAddress) + } + + cfg := &harbor.Config{ + URL: u, + Transport: transport, + AuthInfo: runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { + return req.SetHeaderParam("Authorization", "Bearer "+tokenManager.Token()) + }), } - return buildClientWithToken(credential.ServerAddress, idToken) + return v2client.New(cfg.ToV2Config()), nil } func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, error) { @@ -134,3 +256,93 @@ func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, e return v2client.New(cfg.ToV2Config()), nil } + +func (m *oidcTokenManager) Token() string { + m.mu.RLock() + defer m.mu.RUnlock() + return m.token +} + +func (m *oidcTokenManager) Refresh() (string, error) { + m.mu.Lock() + defer m.mu.Unlock() + + refreshFn := m.refreshFn + if refreshFn == nil { + refreshFn = refreshOIDCCredential + } + + token, err := refreshFn(m.credential) + if err != nil { + return "", err + } + m.token = token + return token, nil +} + +func (t *oidcRetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { + base := t.base + if base == nil { + base = harbor.InsecureTransport + } + + resp, err := base.RoundTrip(req) + if err != nil || resp == nil || resp.StatusCode != http.StatusUnauthorized { + return resp, err + } + if req.Header.Get(oidcRetryHeader) == "1" || !canRetryOIDCRequest(req) { + return resp, nil + } + + if _, err := t.tokenManager.Refresh(); err != nil { + drainAndCloseResponse(resp) + return nil, err + } + + drainAndCloseResponse(resp) + + retryReq, err := cloneRequestForRetry(req) + if err != nil { + return nil, err + } + retryReq.Header.Set("Authorization", "Bearer "+t.tokenManager.Token()) + retryReq.Header.Set(oidcRetryHeader, "1") + + return base.RoundTrip(retryReq) +} + +func canRetryOIDCRequest(req *http.Request) bool { + if req == nil { + return false + } + if req.Body == nil || req.Body == http.NoBody { + return true + } + return req.GetBody != nil +} + +func cloneRequestForRetry(req *http.Request) (*http.Request, error) { + retryReq := req.Clone(req.Context()) + if req.Body != nil && req.Body != http.NoBody { + if req.GetBody == nil { + return nil, fmt.Errorf("request body cannot be retried") + } + body, err := req.GetBody() + if err != nil { + return nil, fmt.Errorf("failed to reset request body: %w", err) + } + retryReq.Body = body + } else { + retryReq.Body = http.NoBody + } + retryReq.Header = req.Header.Clone() + return retryReq, nil +} + +func drainAndCloseResponse(resp *http.Response) { + if resp == nil || resp.Body == nil { + return + } + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 1024)) + _ = resp.Body.Close() +} diff --git a/pkg/utils/config.go b/pkg/utils/config.go index 2b3f1248f..0efad014a 100644 --- a/pkg/utils/config.go +++ b/pkg/utils/config.go @@ -612,11 +612,29 @@ func GetDecryptedIDToken(credentialName string) (string, error) { if credential.AuthType != AuthTypeOIDC { return "", fmt.Errorf("credential %q is not an OIDC credential", credentialName) } + return decryptOIDCCredentialValue(credential.IDToken) +} + +func GetDecryptedRefreshToken(credentialName string) (string, error) { + credential, err := GetCredentials(credentialName) + if err != nil { + return "", err + } + if credential.AuthType != AuthTypeOIDC { + return "", fmt.Errorf("credential %q is not an OIDC credential", credentialName) + } + if credential.RefreshToken == "" { + return "", nil + } + return decryptOIDCCredentialValue(credential.RefreshToken) +} + +func decryptOIDCCredentialValue(encryptedValue string) (string, error) { key, err := GetEncryptionKey() if err != nil { return "", fmt.Errorf("failed to get encryption key: %w", err) } - return Decrypt(key, credential.IDToken) + return Decrypt(key, encryptedValue) } func UpdateOIDCTokens(credentialName, idToken, refreshToken string, expiresAt int64, configPath string) error { diff --git a/pkg/utils/config_test.go b/pkg/utils/config_test.go index d84bbbfed..dd7c7c1a3 100644 --- a/pkg/utils/config_test.go +++ b/pkg/utils/config_test.go @@ -130,4 +130,19 @@ func Test_AddOIDCCredentials(t *testing.T) { idToken, err := utils.GetDecryptedIDToken(cred.Name) assert.NoError(t, err) assert.Equal(t, "id-token", idToken) + + refreshToken, err := utils.GetDecryptedRefreshToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "refresh-token", refreshToken) + + err = utils.UpdateOIDCTokens(cred.Name, "next-id-token", "next-refresh-token", 67890, data.ConfigPath) + assert.NoError(t, err) + + updatedIDToken, err := utils.GetDecryptedIDToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "next-id-token", updatedIDToken) + + updatedRefreshToken, err := utils.GetDecryptedRefreshToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "next-refresh-token", updatedRefreshToken) } diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index 1a152b392..184c333f6 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -14,6 +14,7 @@ package utils import ( + "bytes" "encoding/json" "fmt" "io" @@ -23,8 +24,9 @@ import ( ) const ( - oidcCLILoginPath = "/c/oidc/login" - oidcCLITokenPath = "/c/oidc/cli-token" + oidcCLILoginPath = "/c/oidc/login" + oidcCLITokenPath = "/c/oidc/cli-token" + oidcCLIRefreshPath = "/c/oidc/refresh" ) type OIDCLoginResponse struct { @@ -41,6 +43,17 @@ type OIDCPollResponse struct { Error string `json:"error,omitempty"` } +type OIDCRefreshRequest struct { + RefreshToken string `json:"refresh_token"` +} + +type OIDCRefreshResponse struct { + IDToken string `json:"id_token"` + RefreshToken string `json:"refresh_token,omitempty"` + ExpiresAt int64 `json:"expires_at"` + Error string `json:"error,omitempty"` +} + func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { serverAddress = FormatUrl(serverAddress) if err := ValidateURL(serverAddress); err != nil { @@ -164,6 +177,59 @@ func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { } } +func RefreshOIDCToken(serverAddress, refreshToken string) (*OIDCRefreshResponse, error) { + if refreshToken == "" { + return nil, fmt.Errorf("refresh token is required") + } + + serverAddress = FormatUrl(serverAddress) + if err := ValidateURL(serverAddress); err != nil { + return nil, fmt.Errorf("invalid server URL: %w", err) + } + + endpoint, err := joinServerPath(serverAddress, oidcCLIRefreshPath) + if err != nil { + return nil, err + } + + payload, err := json.Marshal(&OIDCRefreshRequest{RefreshToken: refreshToken}) + if err != nil { + return nil, fmt.Errorf("failed to encode OIDC refresh request: %w", err) + } + + req, err := http.NewRequest(http.MethodPost, endpoint, bytes.NewReader(payload)) + if err != nil { + return nil, fmt.Errorf("failed to create OIDC refresh request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to refresh OIDC token: %w", err) + } + defer resp.Body.Close() + + var refreshResp OIDCRefreshResponse + switch resp.StatusCode { + case http.StatusOK: + if err := json.NewDecoder(resp.Body).Decode(&refreshResp); err != nil { + return nil, fmt.Errorf("failed to decode OIDC refresh response: %w", err) + } + if refreshResp.IDToken == "" { + return nil, fmt.Errorf("invalid OIDC refresh response: missing id_token") + } + return &refreshResp, nil + case http.StatusBadRequest: + if err := json.NewDecoder(resp.Body).Decode(&refreshResp); err == nil && refreshResp.Error != "" { + return nil, fmt.Errorf("OIDC refresh failed: %s", refreshResp.Error) + } + fallthrough + default: + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return nil, fmt.Errorf("failed to refresh OIDC token: status %d: %s", resp.StatusCode, string(body)) + } +} + func joinServerPath(serverAddress, path string) (string, error) { u, err := url.Parse(serverAddress) if err != nil { diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 4508738e7..6ec492b49 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -15,6 +15,7 @@ package utils_test import ( "encoding/json" + "io" "net/http" "net/http/httptest" "testing" @@ -78,3 +79,32 @@ func TestPollForOIDCTokenFailed(t *testing.T) { assert.Nil(t, resp) assert.ErrorContains(t, err, "state expired") } + +func TestRefreshOIDCToken(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/c/oidc/refresh", r.URL.Path) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + + var req utils.OIDCRefreshRequest + require.NoError(t, json.Unmarshal(body, &req)) + assert.Equal(t, "refresh-token-1", req.RefreshToken) + + _ = json.NewEncoder(w).Encode(utils.OIDCRefreshResponse{ + IDToken: "id-token-2", + RefreshToken: "refresh-token-2", + ExpiresAt: 1234567890, + }) + })) + defer server.Close() + + resp, err := utils.RefreshOIDCToken(server.URL, "refresh-token-1") + + require.NoError(t, err) + assert.Equal(t, "id-token-2", resp.IDToken) + assert.Equal(t, "refresh-token-2", resp.RefreshToken) + assert.Equal(t, int64(1234567890), resp.ExpiresAt) +} From 1aa8534a3d1eaa2b57ab54c55e0f6b499eeef79d Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sun, 14 Jun 2026 12:30:38 +0530 Subject: [PATCH 05/25] basic implementation Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 91b6ed74f..268168e12 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -20,8 +20,8 @@ import ( "testing" "github.com/goharbor/harbor-cli/cmd/harbor/root" - helpers "github.com/goharbor/harbor-cli/test/helper" "github.com/goharbor/harbor-cli/pkg/utils" + helpers "github.com/goharbor/harbor-cli/test/helper" "github.com/stretchr/testify/assert" ) From 96eff11a6df86a35ea74aa48705b6711ab297829 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Mon, 15 Jun 2026 23:20:08 +0530 Subject: [PATCH 06/25] moved to state based login with existing endopoint Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 268168e12..41dce76cc 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -22,6 +22,7 @@ import ( "github.com/goharbor/harbor-cli/cmd/harbor/root" "github.com/goharbor/harbor-cli/pkg/utils" helpers "github.com/goharbor/harbor-cli/test/helper" + "github.com/goharbor/harbor-cli/pkg/utils" "github.com/stretchr/testify/assert" ) From 2fafc552f78686534a859c5077568352e8cd6cb9 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sat, 4 Jul 2026 22:37:56 +0530 Subject: [PATCH 07/25] added polling-token support Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 2 +- cmd/harbor/root/login_test.go | 4 ++-- pkg/utils/oidc.go | 19 ++++++++++++------- pkg/utils/oidc_test.go | 25 ++++++++++++++++++++----- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index a21a074ea..974bb386e 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -235,7 +235,7 @@ func RunOIDCLogin(serverAddress string) error { fmt.Printf("Open this URL in your browser to authenticate:\n\n %s\n\n", loginResp.RedirectURL) fmt.Print("Waiting for authentication...\n") - tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.State, 10*time.Minute) + tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.PollToken, 10*time.Minute) if err != nil { return err } diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 41dce76cc..fa00d4bb2 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -160,10 +160,10 @@ func Test_RunOIDCLogin_Success(t *testing.T) { assert.Equal(t, "cli", r.URL.Query().Get("mode")) assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", - State: "state-1", + PollToken: "poll-token-1", })) case "/c/oidc/cli-token": - assert.Equal(t, "state-1", r.URL.Query().Get("state")) + assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index 184c333f6..f0dcd9a0b 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -31,7 +31,7 @@ const ( type OIDCLoginResponse struct { RedirectURL string `json:"redirect_url"` - State string `json:"state"` + PollToken string `json:"poll_token"` } type OIDCPollResponse struct { @@ -87,15 +87,15 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { if err := json.NewDecoder(resp.Body).Decode(&loginResp); err != nil { return nil, fmt.Errorf("failed to decode OIDC login response: %w", err) } - if loginResp.RedirectURL == "" || loginResp.State == "" { - return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or state") + if loginResp.RedirectURL == "" || loginResp.PollToken == "" { + return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or poll_token") } return &loginResp, nil } -func PollForOIDCToken(serverAddress, state string, timeout time.Duration) (*OIDCPollResponse, error) { - if state == "" { - return nil, fmt.Errorf("state is required") +func PollForOIDCToken(serverAddress, pollToken string, timeout time.Duration) (*OIDCPollResponse, error) { + if pollToken == "" { + return nil, fmt.Errorf("poll token is required") } serverAddress = FormatUrl(serverAddress) if err := ValidateURL(serverAddress); err != nil { @@ -111,7 +111,7 @@ func PollForOIDCToken(serverAddress, state string, timeout time.Duration) (*OIDC return nil, fmt.Errorf("failed to parse OIDC token endpoint: %w", err) } q := u.Query() - q.Set("state", state) + q.Set("poll_token", pollToken) u.RawQuery = q.Encode() deadline := time.Now().Add(timeout) @@ -171,6 +171,11 @@ func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { return nil, false, fmt.Errorf("OIDC authentication failed: %s", pollResp.Error) } return nil, false, fmt.Errorf("OIDC authentication failed") + case http.StatusGone: + if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { + return nil, false, fmt.Errorf("OIDC login expired before token retrieval") + } + return nil, false, fmt.Errorf("OIDC login expired before token retrieval") default: body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) return nil, false, fmt.Errorf("failed to poll OIDC token: status %d: %s", resp.StatusCode, string(body)) diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 6ec492b49..2d069c931 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -32,7 +32,7 @@ func TestInitiateOIDCLogin(t *testing.T) { assert.Equal(t, "cli", r.URL.Query().Get("mode")) _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", - State: "state-1", + PollToken: "poll-token-1", }) })) defer server.Close() @@ -41,13 +41,13 @@ func TestInitiateOIDCLogin(t *testing.T) { require.NoError(t, err) assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) - assert.Equal(t, "state-1", resp.State) + assert.Equal(t, "poll-token-1", resp.PollToken) } func TestPollForOIDCTokenReady(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) - assert.Equal(t, "state-1", r.URL.Query().Get("state")) + assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", @@ -56,7 +56,7 @@ func TestPollForOIDCTokenReady(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) require.NoError(t, err) assert.Equal(t, "ready", resp.Status) @@ -74,12 +74,27 @@ func TestPollForOIDCTokenFailed(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) assert.Nil(t, resp) assert.ErrorContains(t, err, "state expired") } +func TestPollForOIDCTokenExpired(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusGone) + _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "expired", + }) + })) + defer server.Close() + + resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) + + assert.Nil(t, resp) + assert.ErrorContains(t, err, "expired before token retrieval") +} + func TestRefreshOIDCToken(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodPost, r.Method) From a469bf0586fdfdc7427c3530edf647b526e2912e Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sat, 4 Jul 2026 22:57:29 +0530 Subject: [PATCH 08/25] added test Signed-off-by: Nishchay Rajput --- pkg/utils/client_oidc_internal_test.go | 99 ++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 pkg/utils/client_oidc_internal_test.go diff --git a/pkg/utils/client_oidc_internal_test.go b/pkg/utils/client_oidc_internal_test.go new file mode 100644 index 000000000..0d92c1d27 --- /dev/null +++ b/pkg/utils/client_oidc_internal_test.go @@ -0,0 +1,99 @@ +package utils + +import ( + "encoding/base64" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOIDCTokenExpiryUnix(t *testing.T) { + token := testJWTWithExp(time.Now().Add(10 * time.Minute).Unix()) + + expiresAt, err := oidcTokenExpiryUnix(token) + + require.NoError(t, err) + assert.Greater(t, expiresAt, time.Now().Unix()) +} + +func TestOIDCTokenNeedsRefreshPrefersTokenExpiry(t *testing.T) { + credential := Credential{ + ExpiresAt: time.Now().Add(2 * time.Hour).Unix(), + } + token := testJWTWithExp(time.Now().Add(30 * time.Second).Unix()) + + assert.True(t, oidcTokenNeedsRefresh(credential, token)) +} + +func TestOIDCTokenNeedsRefreshFallsBackToStoredExpiry(t *testing.T) { + credential := Credential{ + ExpiresAt: time.Now().Add(30 * time.Second).Unix(), + } + + assert.True(t, oidcTokenNeedsRefresh(credential, "not-a-jwt")) +} + +func testJWTWithExp(exp int64) string { + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"none","typ":"JWT"}`)) + payload := base64.RawURLEncoding.EncodeToString([]byte(fmt.Sprintf(`{"exp":%d}`, exp))) + return header + "." + payload + ".signature" +} + +func TestOIDCRetryTransportRefreshesAndRetriesOnUnauthorized(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + switch r.Header.Get("Authorization") { + case "Bearer stale-token": + w.WriteHeader(http.StatusUnauthorized) + case "Bearer fresh-token": + w.WriteHeader(http.StatusOK) + default: + w.WriteHeader(http.StatusForbidden) + } + })) + defer server.Close() + + tokenManager := &oidcTokenManager{ + token: "stale-token", + refreshFn: func(_ Credential) (string, error) { + return "fresh-token", nil + }, + } + + transport := &oidcRetryTransport{ + base: server.Client().Transport, + tokenManager: tokenManager, + } + + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+tokenManager.Token()) + + resp, err := transport.RoundTrip(req) + + require.NoError(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "fresh-token", tokenManager.Token()) + assert.Equal(t, 2, requests) +} + +func TestCloneRequestForRetryWithReplayableBody(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "https://example.com", strings.NewReader("payload")) + require.NoError(t, err) + + cloned, err := cloneRequestForRetry(req) + + require.NoError(t, err) + body, err := io.ReadAll(cloned.Body) + require.NoError(t, err) + assert.Equal(t, "payload", string(body)) +} From 7d57d5812c95abd9aa33747486e73cbc868c9caf Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sun, 5 Jul 2026 00:33:48 +0530 Subject: [PATCH 09/25] fix: lint and genrate docs Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login_test.go | 1 - doc/cli-docs/harbor-login.md | 1 + doc/man-docs/man1/harbor-login.1 | 4 ++++ pkg/utils/client.go | 19 ------------------- pkg/utils/client_oidc_internal_test.go | 15 ++++++++++++++- pkg/utils/oidc.go | 2 +- 6 files changed, 20 insertions(+), 22 deletions(-) diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index fa00d4bb2..4ebbde177 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -22,7 +22,6 @@ import ( "github.com/goharbor/harbor-cli/cmd/harbor/root" "github.com/goharbor/harbor-cli/pkg/utils" helpers "github.com/goharbor/harbor-cli/test/helper" - "github.com/goharbor/harbor-cli/pkg/utils" "github.com/stretchr/testify/assert" ) diff --git a/doc/cli-docs/harbor-login.md b/doc/cli-docs/harbor-login.md index 0c67d1cee..fe21c83ee 100644 --- a/doc/cli-docs/harbor-login.md +++ b/doc/cli-docs/harbor-login.md @@ -21,6 +21,7 @@ harbor login [server] [flags] ```sh -n, --context-name string Login context name (optional) -h, --help help for login + --oidc Authenticate using Harbor OIDC browser login -p, --password string Password (not recommended, use --password-stdin for better security) --password-stdin Take the password from stdin --skip-verify-client Skip whether the clients basic auth credentials shall be validated against the Harbor server during login. This is not recommended as it may lead to storing invalid credentials. Use this flag if you want to skip validation of credentials during login, for example, when the Harbor server is not reachable at the moment of login but you still want to store the credentials for later use. diff --git a/doc/man-docs/man1/harbor-login.1 b/doc/man-docs/man1/harbor-login.1 index 0384a43a8..ae9046908 100644 --- a/doc/man-docs/man1/harbor-login.1 +++ b/doc/man-docs/man1/harbor-login.1 @@ -21,6 +21,10 @@ Authenticate with Harbor Registry. \fB-h\fP, \fB--help\fP[=false] help for login +.PP +\fB--oidc\fP[=false] + Authenticate using Harbor OIDC browser login + .PP \fB-p\fP, \fB--password\fP="" Password (not recommended, use --password-stdin for better security) diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 5a953bd0e..df7e24c52 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -238,25 +238,6 @@ func buildClientWithAuth(serverAddress string, tokenManager *oidcTokenManager, t return v2client.New(cfg.ToV2Config()), nil } -func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, error) { - u, err := url.Parse(serverAddress) - if err != nil { - return nil, fmt.Errorf("failed to parse server URL: %w", err) - } - if u.Scheme == "" || u.Host == "" { - return nil, fmt.Errorf("invalid server URL: %s", serverAddress) - } - - cfg := &harbor.Config{ - URL: u, - AuthInfo: runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { - return req.SetHeaderParam("Authorization", "Bearer "+idToken) - }), - } - - return v2client.New(cfg.ToV2Config()), nil -} - func (m *oidcTokenManager) Token() string { m.mu.RLock() defer m.mu.RUnlock() diff --git a/pkg/utils/client_oidc_internal_test.go b/pkg/utils/client_oidc_internal_test.go index 0d92c1d27..6bfa6bcb1 100644 --- a/pkg/utils/client_oidc_internal_test.go +++ b/pkg/utils/client_oidc_internal_test.go @@ -1,3 +1,16 @@ +// 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 utils import ( @@ -78,7 +91,7 @@ func TestOIDCRetryTransportRefreshesAndRetriesOnUnauthorized(t *testing.T) { req.Header.Set("Authorization", "Bearer "+tokenManager.Token()) resp, err := transport.RoundTrip(req) - + defer resp.Body.Close() require.NoError(t, err) require.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index f0dcd9a0b..936f60b2c 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -25,7 +25,7 @@ import ( const ( oidcCLILoginPath = "/c/oidc/login" - oidcCLITokenPath = "/c/oidc/cli-token" + oidcCLITokenPath = "/c/oidc/cli-token" //nolint:gosec // endpoint is user-provided Harbor server URL for login. oidcCLIRefreshPath = "/c/oidc/refresh" ) From 54fe5078c2c36b7ee24bc8d9e2934d8eb6115755 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sun, 14 Jun 2026 12:30:38 +0530 Subject: [PATCH 10/25] basic implementation Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 45 +++++++++ cmd/harbor/root/login_test.go | 20 ++++ pkg/utils/client.go | 40 ++++++++ pkg/utils/config.go | 102 +++++++++++++++++++- pkg/utils/config_test.go | 24 +++++ pkg/utils/oidc.go | 169 ++++++++++++++++++++++++++++++++++ pkg/utils/oidc_test.go | 79 ++++++++++++++++ 7 files changed, 475 insertions(+), 4 deletions(-) create mode 100644 pkg/utils/oidc.go create mode 100644 pkg/utils/oidc_test.go diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index e79c77561..663dce475 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -18,6 +18,7 @@ import ( "fmt" "os" "strings" + "time" "github.com/goharbor/go-client/pkg/harbor" "github.com/goharbor/go-client/pkg/sdk/v2.0/client" @@ -38,6 +39,7 @@ var ( Name string passwordStdin bool skipVerifyClient bool + oidcLogin bool ) // LoginCommand creates a new `harbor login` command @@ -52,6 +54,10 @@ func LoginCommand() *cobra.Command { serverAddress = args[0] } + if oidcLogin { + return RunOIDCLogin(serverAddress) + } + if passwordStdin { fmt.Print("Password: ") passwordBytes, err := term.ReadPassword(int(os.Stdin.Fd())) // #nosec G115 - fd fits in int on all supported platforms @@ -87,9 +93,13 @@ func LoginCommand() *cobra.Command { flags.StringVarP(&Name, "context-name", "n", "", "Login context name (optional)") flags.StringVarP(&Password, "password", "p", "", "Password (not recommended, use --password-stdin for better security)") flags.BoolVar(&passwordStdin, "password-stdin", false, "Take the password from stdin") + flags.BoolVar(&oidcLogin, "oidc", false, "Authenticate using Harbor OIDC browser login") flags.BoolVarP(&skipVerifyClient, "skip-verify-client", "", false, "Skip whether the clients basic auth credentials shall be validated against the Harbor server during login. This is not recommended as it may lead to storing invalid credentials. Use this flag if you want to skip validation of credentials during login, for example, when the Harbor server is not reachable at the moment of login but you still want to store the credentials for later use.") cmd.MarkFlagsMutuallyExclusive("password", "password-stdin") + cmd.MarkFlagsMutuallyExclusive("oidc", "username") + cmd.MarkFlagsMutuallyExclusive("oidc", "password") + cmd.MarkFlagsMutuallyExclusive("oidc", "password-stdin") return cmd } @@ -208,6 +218,41 @@ func RunLogin(opts login.LoginView) error { return nil } +func RunOIDCLogin(serverAddress string) error { + if serverAddress == "" { + return fmt.Errorf("server address is required for OIDC login") + } + serverAddress = utils.FormatUrl(serverAddress) + if err := utils.ValidateURL(serverAddress); err != nil { + return fmt.Errorf("invalid server URL: %w", err) + } + + loginResp, err := utils.InitiateOIDCLogin(serverAddress) + if err != nil { + return err + } + + fmt.Printf("Open this URL in your browser to authenticate:\n\n %s\n\n", loginResp.RedirectURL) + fmt.Print("Waiting for authentication...\n") + + tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.TransactionID, 10*time.Minute) + if err != nil { + return err + } + + harborData, err := utils.GetCurrentHarborData() + if err != nil { + return fmt.Errorf("failed to get current harbor data: %s", err) + } + + if err := utils.AddOIDCCredentials(serverAddress, tokenResp.Username, tokenResp.IDToken, tokenResp.RefreshToken, tokenResp.ExpiresAt, harborData.ConfigPath); err != nil { + return fmt.Errorf("failed to store OIDC credential: %w", err) + } + + fmt.Printf("Login successful for %s at %s\n", tokenResp.Username, serverAddress) + return nil +} + func validateClientConnection(client *client.HarborAPI) error { ctx := context.Background() diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 22ae6d1ec..50344b8ad 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -122,3 +122,23 @@ 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_Failure_OIDCMutuallyExclusiveFlags(t *testing.T) { + tempDir := t.TempDir() + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + cmd := root.LoginCommand() + cmd.SetArgs([]string{"http://demo.goharbor.io"}) + + assert.NoError(t, cmd.Flags().Set("oidc", "true")) + assert.NoError(t, cmd.Flags().Set("username", "admin")) + + err := cmd.Execute() + assert.Error(t, err, "Expected error when --oidc and --username are set") +} + +func Test_RunOIDCLogin_Failure_MissingServer(t *testing.T) { + err := root.RunOIDCLogin("") + assert.Error(t, err) +} diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 8929661ca..03fefc2fc 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -16,8 +16,14 @@ package utils import ( "context" "fmt" + "net/url" + "strings" "sync" + "time" + "github.com/go-openapi/runtime" + httptransport "github.com/go-openapi/runtime/client" + "github.com/go-openapi/strfmt" "github.com/goharbor/go-client/pkg/harbor" v2client "github.com/goharbor/go-client/pkg/sdk/v2.0/client" log "github.com/sirupsen/logrus" @@ -75,6 +81,9 @@ func GetClientByCredentialName(credentialName string) (*v2client.HarborAPI, erro if err != nil { return nil, fmt.Errorf("failed to get credential %s: %w", credentialName, err) } + if credential.AuthType == AuthTypeOIDC { + return getOIDCClient(credential) + } // Get encryption key key, err := GetEncryptionKey() @@ -95,3 +104,34 @@ func GetClientByCredentialName(credentialName string) (*v2client.HarborAPI, erro } return GetClientByConfig(clientConfig), nil } + +func getOIDCClient(credential Credential) (*v2client.HarborAPI, error) { + idToken, err := GetDecryptedIDToken(credential.Name) + if err != nil { + return nil, err + } + + if credential.ExpiresAt > 0 && time.Now().Unix() >= credential.ExpiresAt-60 { + return nil, fmt.Errorf("OIDC session expired or is about to expire. Please run `harbor login %s --oidc` again", credential.ServerAddress) + } + + return buildOIDCClient(credential.ServerAddress, idToken) +} + +func buildOIDCClient(serverAddress, idToken string) (*v2client.HarborAPI, error) { + u, err := url.Parse(serverAddress) + if err != nil { + return nil, fmt.Errorf("failed to parse server URL: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return nil, fmt.Errorf("invalid server URL: %s", serverAddress) + } + + basePath := strings.TrimRight(u.Path, "/") + transport := httptransport.New(u.Host, basePath, []string{u.Scheme}) + transport.DefaultAuthentication = runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { + return req.SetHeaderParam("Authorization", "Bearer "+idToken) + }) + + return v2client.New(transport, strfmt.Default), nil +} diff --git a/pkg/utils/config.go b/pkg/utils/config.go index cecc7628e..2b3f1248f 100644 --- a/pkg/utils/config.go +++ b/pkg/utils/config.go @@ -27,12 +27,21 @@ import ( ) type Credential struct { - Name string `yaml:"name"` - Username string `yaml:"username"` - Password string `yaml:"password"` - ServerAddress string `yaml:"serveraddress"` + Name string `mapstructure:"name" yaml:"name"` + Username string `mapstructure:"username" yaml:"username"` + Password string `mapstructure:"password,omitempty" yaml:"password,omitempty"` + ServerAddress string `mapstructure:"serveraddress" yaml:"serveraddress"` + AuthType string `mapstructure:"auth-type,omitempty" yaml:"auth-type,omitempty"` + IDToken string `mapstructure:"id-token,omitempty" yaml:"id-token,omitempty"` + RefreshToken string `mapstructure:"refresh-token,omitempty" yaml:"refresh-token,omitempty"` + ExpiresAt int64 `mapstructure:"expires-at,omitempty" yaml:"expires-at,omitempty"` } +const ( + AuthTypeBasic = "basic" + AuthTypeOIDC = "oidc" +) + type HarborConfig struct { CurrentCredentialName string `mapstructure:"current-credential-name" yaml:"current-credential-name"` Credentials []Credential `mapstructure:"credentials" yaml:"credentials"` @@ -505,6 +514,7 @@ func AddCredentialsToConfigFile(credential Credential, configPath string) error log.Fatalf("failed to write updated config file: %v", err) } + CurrentHarborConfig = &c fmt.Printf("Added credential '%s' to config file at %s\n", credential.Name, configPath) return nil } @@ -550,7 +560,91 @@ func UpdateCredentialsInConfigFile(updatedCredential Credential, configPath stri log.Fatalf("failed to write updated config file: %v", err) } + CurrentHarborConfig = &c fmt.Printf("Updated credential '%s' in config file at %s.\n", updatedCredential.Name, configPath) fmt.Printf("Switched to context '%s'\n", updatedCredential.Name) return nil } + +func AddOIDCCredentials(serverAddress, username, idToken, refreshToken string, expiresAt int64, configPath string) error { + if err := GenerateEncryptionKey(); err != nil { + return fmt.Errorf("failed to generate encryption key: %w", err) + } + key, err := GetEncryptionKey() + if err != nil { + return fmt.Errorf("failed to get encryption key: %w", err) + } + + encryptedIDToken, err := Encrypt(key, []byte(idToken)) + if err != nil { + return fmt.Errorf("failed to encrypt id token: %w", err) + } + + var encryptedRefreshToken string + if refreshToken != "" { + encryptedRefreshToken, err = Encrypt(key, []byte(refreshToken)) + if err != nil { + return fmt.Errorf("failed to encrypt refresh token: %w", err) + } + } + + credential := Credential{ + Name: DefaultCredentialName(username, serverAddress), + Username: username, + ServerAddress: serverAddress, + AuthType: AuthTypeOIDC, + IDToken: encryptedIDToken, + RefreshToken: encryptedRefreshToken, + ExpiresAt: expiresAt, + } + + if _, err := GetCredentials(credential.Name); err == nil { + return UpdateCredentialsInConfigFile(credential, configPath) + } + return AddCredentialsToConfigFile(credential, configPath) +} + +func GetDecryptedIDToken(credentialName string) (string, error) { + credential, err := GetCredentials(credentialName) + if err != nil { + return "", err + } + if credential.AuthType != AuthTypeOIDC { + return "", fmt.Errorf("credential %q is not an OIDC credential", credentialName) + } + key, err := GetEncryptionKey() + if err != nil { + return "", fmt.Errorf("failed to get encryption key: %w", err) + } + return Decrypt(key, credential.IDToken) +} + +func UpdateOIDCTokens(credentialName, idToken, refreshToken string, expiresAt int64, configPath string) error { + credential, err := GetCredentials(credentialName) + if err != nil { + return err + } + if credential.AuthType != AuthTypeOIDC { + return fmt.Errorf("credential %q is not an OIDC credential", credentialName) + } + key, err := GetEncryptionKey() + if err != nil { + return fmt.Errorf("failed to get encryption key: %w", err) + } + encryptedIDToken, err := Encrypt(key, []byte(idToken)) + if err != nil { + return fmt.Errorf("failed to encrypt id token: %w", err) + } + credential.IDToken = encryptedIDToken + credential.ExpiresAt = expiresAt + + if refreshToken != "" { + encryptedRefreshToken, err := Encrypt(key, []byte(refreshToken)) + if err != nil { + return fmt.Errorf("failed to encrypt refresh token: %w", err) + } + credential.RefreshToken = encryptedRefreshToken + } + + return UpdateCredentialsInConfigFile(credential, configPath) +} diff --git a/pkg/utils/config_test.go b/pkg/utils/config_test.go index 11fba03f3..d84bbbfed 100644 --- a/pkg/utils/config_test.go +++ b/pkg/utils/config_test.go @@ -107,3 +107,27 @@ func Test_Config_Flag(t *testing.T) { assert.NotNil(t, currentConfig.Credentials, "Credentials should not be nil") assert.NotNil(t, data.ConfigPath, "ConfigPath should not be nil") } + +func Test_AddOIDCCredentials(t *testing.T) { + tempDir := t.TempDir() + helpers.SetMockKeyring(t) + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + err := utils.AddOIDCCredentials("https://demo.goharbor.io", "alice", "id-token", "refresh-token", 12345, data.ConfigPath) + assert.NoError(t, err) + + cred, err := utils.GetCredentials("alice@https-demo-goharbor-io") + assert.NoError(t, err) + assert.Equal(t, utils.AuthTypeOIDC, cred.AuthType) + assert.Equal(t, "alice", cred.Username) + assert.Equal(t, "https://demo.goharbor.io", cred.ServerAddress) + assert.Equal(t, int64(12345), cred.ExpiresAt) + assert.NotEmpty(t, cred.IDToken) + assert.NotEmpty(t, cred.RefreshToken) + assert.Empty(t, cred.Password) + + idToken, err := utils.GetDecryptedIDToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "id-token", idToken) +} diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go new file mode 100644 index 000000000..28eee857e --- /dev/null +++ b/pkg/utils/oidc.go @@ -0,0 +1,169 @@ +// 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 utils + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "time" +) + +const ( + oidcCLILoginPath = "/c/oidc/cli/login" + oidcCLITokenPath = "/c/oidc/cli-token" +) + +type OIDCLoginResponse struct { + RedirectURL string `json:"redirect_url"` + TransactionID string `json:"transaction_id"` +} + +type OIDCPollResponse struct { + Status string `json:"status"` + IDToken string `json:"id_token,omitempty"` + RefreshToken string `json:"refresh_token,omitempty"` + Username string `json:"username,omitempty"` + ExpiresAt int64 `json:"expires_at,omitempty"` + Error string `json:"error,omitempty"` +} + +func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { + serverAddress = FormatUrl(serverAddress) + if err := ValidateURL(serverAddress); err != nil { + return nil, fmt.Errorf("invalid server URL: %w", err) + } + + endpoint, err := joinServerPath(serverAddress, oidcCLILoginPath) + if err != nil { + return nil, err + } + + resp, err := http.Get(endpoint) //nolint:gosec // endpoint is user-provided Harbor server URL for login. + if err != nil { + return nil, fmt.Errorf("failed to initiate OIDC login: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return nil, fmt.Errorf("failed to initiate OIDC login: status %d: %s", resp.StatusCode, string(body)) + } + + var loginResp OIDCLoginResponse + if err := json.NewDecoder(resp.Body).Decode(&loginResp); err != nil { + return nil, fmt.Errorf("failed to decode OIDC login response: %w", err) + } + if loginResp.RedirectURL == "" || loginResp.TransactionID == "" { + return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or transaction_id") + } + return &loginResp, nil +} + +func PollForOIDCToken(serverAddress, transactionID string, timeout time.Duration) (*OIDCPollResponse, error) { + if transactionID == "" { + return nil, fmt.Errorf("transaction ID is required") + } + serverAddress = FormatUrl(serverAddress) + if err := ValidateURL(serverAddress); err != nil { + return nil, fmt.Errorf("invalid server URL: %w", err) + } + + endpoint, err := joinServerPath(serverAddress, oidcCLITokenPath) + if err != nil { + return nil, err + } + u, err := url.Parse(endpoint) + if err != nil { + return nil, fmt.Errorf("failed to parse OIDC token endpoint: %w", err) + } + q := u.Query() + q.Set("transaction_id", transactionID) + u.RawQuery = q.Encode() + + deadline := time.Now().Add(timeout) + ticker := time.NewTicker(3 * time.Second) + defer ticker.Stop() + + for { + result, ready, err := pollOIDCTokenOnce(u.String()) + if err != nil { + return nil, err + } + if ready { + return result, nil + } + if time.Now().After(deadline) { + return nil, fmt.Errorf("timed out waiting for OIDC authentication") + } + remaining := time.Until(deadline) + if remaining <= 0 { + return nil, fmt.Errorf("timed out waiting for OIDC authentication") + } + select { + case <-ticker.C: + case <-time.After(remaining): + return nil, fmt.Errorf("timed out waiting for OIDC authentication") + } + } +} + +func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { + resp, err := http.Get(endpoint) //nolint:gosec // endpoint is the Harbor server URL validated by PollForOIDCToken. + if err != nil { + return nil, false, fmt.Errorf("failed to poll OIDC token: %w", err) + } + defer resp.Body.Close() + + var pollResp OIDCPollResponse + switch resp.StatusCode { + case http.StatusAccepted: + return &OIDCPollResponse{Status: "pending"}, false, nil + case http.StatusOK: + if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { + return nil, false, fmt.Errorf("failed to decode OIDC token response: %w", err) + } + if pollResp.Status != "ready" { + return nil, false, fmt.Errorf("unexpected OIDC token status: %s", pollResp.Status) + } + if pollResp.IDToken == "" || pollResp.Username == "" { + return nil, false, fmt.Errorf("invalid OIDC token response: missing id_token or username") + } + return &pollResp, true, nil + case http.StatusBadRequest: + if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { + return nil, false, fmt.Errorf("OIDC authentication failed") + } + if pollResp.Error != "" { + return nil, false, fmt.Errorf("OIDC authentication failed: %s", pollResp.Error) + } + return nil, false, fmt.Errorf("OIDC authentication failed") + default: + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return nil, false, fmt.Errorf("failed to poll OIDC token: status %d: %s", resp.StatusCode, string(body)) + } +} + +func joinServerPath(serverAddress, path string) (string, error) { + u, err := url.Parse(serverAddress) + if err != nil { + return "", fmt.Errorf("failed to parse server URL: %w", err) + } + u.Path = path + u.RawQuery = "" + u.Fragment = "" + return u.String(), nil +} diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go new file mode 100644 index 000000000..177c4b52c --- /dev/null +++ b/pkg/utils/oidc_test.go @@ -0,0 +1,79 @@ +// 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 utils_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/goharbor/harbor-cli/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestInitiateOIDCLogin(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/c/oidc/cli/login", r.URL.Path) + _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + RedirectURL: "https://idp.example/authorize", + TransactionID: "tx-1", + }) + })) + defer server.Close() + + resp, err := utils.InitiateOIDCLogin(server.URL) + + require.NoError(t, err) + assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) + assert.Equal(t, "tx-1", resp.TransactionID) +} + +func TestPollForOIDCTokenReady(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) + assert.Equal(t, "tx-1", r.URL.Query().Get("transaction_id")) + _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "ready", + IDToken: "id-token", + Username: "alice", + }) + })) + defer server.Close() + + resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + + require.NoError(t, err) + assert.Equal(t, "ready", resp.Status) + assert.Equal(t, "id-token", resp.IDToken) + assert.Equal(t, "alice", resp.Username) +} + +func TestPollForOIDCTokenFailed(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "failed", + Error: "state expired", + }) + })) + defer server.Close() + + resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + + assert.Nil(t, resp) + assert.ErrorContains(t, err, "state expired") +} From 6d56600ca38a6c1e0251de62eae99c8add1b1b99 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Mon, 15 Jun 2026 23:20:08 +0530 Subject: [PATCH 11/25] moved to state based login with existing endopoint Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 2 +- cmd/harbor/root/login_test.go | 39 +++++++++++++++++++++++++++++++++++ pkg/utils/client.go | 4 ++-- pkg/utils/oidc.go | 27 +++++++++++++++--------- pkg/utils/oidc_test.go | 15 +++++++------- 5 files changed, 67 insertions(+), 20 deletions(-) diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index 663dce475..a21a074ea 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -235,7 +235,7 @@ func RunOIDCLogin(serverAddress string) error { fmt.Printf("Open this URL in your browser to authenticate:\n\n %s\n\n", loginResp.RedirectURL) fmt.Print("Waiting for authentication...\n") - tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.TransactionID, 10*time.Minute) + tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.State, 10*time.Minute) if err != nil { return err } diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 50344b8ad..91b6ed74f 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -14,10 +14,14 @@ package root_test import ( + "encoding/json" + "net/http" + "net/http/httptest" "testing" "github.com/goharbor/harbor-cli/cmd/harbor/root" helpers "github.com/goharbor/harbor-cli/test/helper" + "github.com/goharbor/harbor-cli/pkg/utils" "github.com/stretchr/testify/assert" ) @@ -142,3 +146,38 @@ func Test_RunOIDCLogin_Failure_MissingServer(t *testing.T) { err := root.RunOIDCLogin("") assert.Error(t, err) } + +func Test_RunOIDCLogin_Success(t *testing.T) { + tempDir := t.TempDir() + helpers.SetMockKeyring(t) + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/c/oidc/login": + assert.Equal(t, "cli", r.URL.Query().Get("mode")) + assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + RedirectURL: "https://idp.example/authorize", + State: "state-1", + })) + case "/c/oidc/cli-token": + assert.Equal(t, "state-1", r.URL.Query().Get("state")) + assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "ready", + IDToken: "id-token", + Username: "alice", + })) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + err := root.RunOIDCLogin(server.URL) + assert.NoError(t, err) + + cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", server.URL)) + assert.NoError(t, err) + assert.Equal(t, utils.AuthTypeOIDC, cred.AuthType) +} diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 03fefc2fc..3dbca16a4 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -115,10 +115,10 @@ func getOIDCClient(credential Credential) (*v2client.HarborAPI, error) { return nil, fmt.Errorf("OIDC session expired or is about to expire. Please run `harbor login %s --oidc` again", credential.ServerAddress) } - return buildOIDCClient(credential.ServerAddress, idToken) + return buildClientWithToken(credential.ServerAddress, idToken) } -func buildOIDCClient(serverAddress, idToken string) (*v2client.HarborAPI, error) { +func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, error) { u, err := url.Parse(serverAddress) if err != nil { return nil, fmt.Errorf("failed to parse server URL: %w", err) diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index 28eee857e..1a152b392 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -23,13 +23,13 @@ import ( ) const ( - oidcCLILoginPath = "/c/oidc/cli/login" + oidcCLILoginPath = "/c/oidc/login" oidcCLITokenPath = "/c/oidc/cli-token" ) type OIDCLoginResponse struct { - RedirectURL string `json:"redirect_url"` - TransactionID string `json:"transaction_id"` + RedirectURL string `json:"redirect_url"` + State string `json:"state"` } type OIDCPollResponse struct { @@ -51,8 +51,15 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { if err != nil { return nil, err } + u, err := url.Parse(endpoint) + if err != nil { + return nil, fmt.Errorf("failed to parse OIDC login endpoint: %w", err) + } + q := u.Query() + q.Set("mode", "cli") + u.RawQuery = q.Encode() - resp, err := http.Get(endpoint) //nolint:gosec // endpoint is user-provided Harbor server URL for login. + resp, err := http.Get(u.String()) //nolint:gosec // endpoint is user-provided Harbor server URL for login. if err != nil { return nil, fmt.Errorf("failed to initiate OIDC login: %w", err) } @@ -67,15 +74,15 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { if err := json.NewDecoder(resp.Body).Decode(&loginResp); err != nil { return nil, fmt.Errorf("failed to decode OIDC login response: %w", err) } - if loginResp.RedirectURL == "" || loginResp.TransactionID == "" { - return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or transaction_id") + if loginResp.RedirectURL == "" || loginResp.State == "" { + return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or state") } return &loginResp, nil } -func PollForOIDCToken(serverAddress, transactionID string, timeout time.Duration) (*OIDCPollResponse, error) { - if transactionID == "" { - return nil, fmt.Errorf("transaction ID is required") +func PollForOIDCToken(serverAddress, state string, timeout time.Duration) (*OIDCPollResponse, error) { + if state == "" { + return nil, fmt.Errorf("state is required") } serverAddress = FormatUrl(serverAddress) if err := ValidateURL(serverAddress); err != nil { @@ -91,7 +98,7 @@ func PollForOIDCToken(serverAddress, transactionID string, timeout time.Duration return nil, fmt.Errorf("failed to parse OIDC token endpoint: %w", err) } q := u.Query() - q.Set("transaction_id", transactionID) + q.Set("state", state) u.RawQuery = q.Encode() deadline := time.Now().Add(timeout) diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 177c4b52c..4508738e7 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -27,10 +27,11 @@ import ( func TestInitiateOIDCLogin(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/c/oidc/cli/login", r.URL.Path) + assert.Equal(t, "/c/oidc/login", r.URL.Path) + assert.Equal(t, "cli", r.URL.Query().Get("mode")) _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ - RedirectURL: "https://idp.example/authorize", - TransactionID: "tx-1", + RedirectURL: "https://idp.example/authorize", + State: "state-1", }) })) defer server.Close() @@ -39,13 +40,13 @@ func TestInitiateOIDCLogin(t *testing.T) { require.NoError(t, err) assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) - assert.Equal(t, "tx-1", resp.TransactionID) + assert.Equal(t, "state-1", resp.State) } func TestPollForOIDCTokenReady(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) - assert.Equal(t, "tx-1", r.URL.Query().Get("transaction_id")) + assert.Equal(t, "state-1", r.URL.Query().Get("state")) _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", @@ -54,7 +55,7 @@ func TestPollForOIDCTokenReady(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) require.NoError(t, err) assert.Equal(t, "ready", resp.Status) @@ -72,7 +73,7 @@ func TestPollForOIDCTokenFailed(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "tx-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) assert.Nil(t, resp) assert.ErrorContains(t, err, "state expired") From e7541a02f05b5a1e9a261fd1998cabb389f6a0c8 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sat, 20 Jun 2026 16:12:58 +0530 Subject: [PATCH 12/25] fix client error Signed-off-by: Nishchay Rajput --- pkg/utils/client.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 3dbca16a4..4720dd5af 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -17,12 +17,10 @@ import ( "context" "fmt" "net/url" - "strings" "sync" "time" "github.com/go-openapi/runtime" - httptransport "github.com/go-openapi/runtime/client" "github.com/go-openapi/strfmt" "github.com/goharbor/go-client/pkg/harbor" v2client "github.com/goharbor/go-client/pkg/sdk/v2.0/client" @@ -127,11 +125,12 @@ func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, e return nil, fmt.Errorf("invalid server URL: %s", serverAddress) } - basePath := strings.TrimRight(u.Path, "/") - transport := httptransport.New(u.Host, basePath, []string{u.Scheme}) - transport.DefaultAuthentication = runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { - return req.SetHeaderParam("Authorization", "Bearer "+idToken) - }) + cfg := &harbor.Config{ + URL: u, + AuthInfo: runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { + return req.SetHeaderParam("Authorization", "Bearer "+idToken) + }), + } - return v2client.New(transport, strfmt.Default), nil + return v2client.New(cfg.ToV2Config()), nil } From d7259e8dc94015089e1033ce917df26d430ae14e Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Fri, 3 Jul 2026 23:34:23 +0530 Subject: [PATCH 13/25] basic refresh token support Signed-off-by: Nishchay Rajput --- pkg/utils/client.go | 218 ++++++++++++++++++++++++++++++++++++++- pkg/utils/config.go | 20 +++- pkg/utils/config_test.go | 15 +++ pkg/utils/oidc.go | 70 ++++++++++++- pkg/utils/oidc_test.go | 30 ++++++ 5 files changed, 347 insertions(+), 6 deletions(-) diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 4720dd5af..5a953bd0e 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -15,8 +15,13 @@ package utils import ( "context" + "encoding/base64" + "encoding/json" "fmt" + "io" + "net/http" "net/url" + "strings" "sync" "time" @@ -33,6 +38,22 @@ var ( ClientErr error ) +const oidcRefreshFailureMessage = "Unable to refresh OIDC session. Please try again or run 'harbor login --oidc'." + +const oidcRetryHeader = "X-Harbor-CLI-OIDC-Retry" + +type oidcTokenManager struct { + mu sync.RWMutex + credential Credential + token string + refreshFn func(Credential) (string, error) +} + +type oidcRetryTransport struct { + base http.RoundTripper + tokenManager *oidcTokenManager +} + func GetClient() (*v2client.HarborAPI, error) { ClientOnce.Do(func() { config, err := GetCurrentHarborConfig() @@ -109,11 +130,112 @@ func getOIDCClient(credential Credential) (*v2client.HarborAPI, error) { return nil, err } - if credential.ExpiresAt > 0 && time.Now().Unix() >= credential.ExpiresAt-60 { - return nil, fmt.Errorf("OIDC session expired or is about to expire. Please run `harbor login %s --oidc` again", credential.ServerAddress) + if oidcTokenNeedsRefresh(credential, idToken) { + idToken, err = refreshOIDCCredential(credential) + if err != nil { + return nil, err + } + } + + return buildOIDCClient(credential, idToken) +} + +func refreshOIDCCredential(credential Credential) (string, error) { + refreshToken, err := GetDecryptedRefreshToken(credential.Name) + if err != nil { + return "", fmt.Errorf("failed to load OIDC refresh token: %w", err) + } + if refreshToken == "" { + return "", fmt.Errorf(oidcRefreshFailureMessage) + } + + refreshResp, err := RefreshOIDCToken(credential.ServerAddress, refreshToken) + if err != nil { + log.WithError(err).Warn("failed to refresh OIDC token") + return "", fmt.Errorf(oidcRefreshFailureMessage) + } + + nextRefreshToken := refreshResp.RefreshToken + if nextRefreshToken == "" { + nextRefreshToken = refreshToken + } + + harborData, err := GetCurrentHarborData() + if err != nil { + return "", fmt.Errorf("failed to get current Harbor data: %w", err) + } + if err := UpdateOIDCTokens(credential.Name, refreshResp.IDToken, nextRefreshToken, refreshResp.ExpiresAt, harborData.ConfigPath); err != nil { + return "", fmt.Errorf("failed to persist refreshed OIDC tokens: %w", err) + } + + return refreshResp.IDToken, nil +} + +func oidcTokenNeedsRefresh(credential Credential, idToken string) bool { + expiresAt, err := oidcTokenExpiryUnix(idToken) + if err != nil { + expiresAt = credential.ExpiresAt + } + if expiresAt <= 0 { + return false + } + return time.Now().Unix() >= expiresAt-60 +} + +func oidcTokenExpiryUnix(idToken string) (int64, error) { + parts := strings.Split(idToken, ".") + if len(parts) != 3 { + return 0, fmt.Errorf("invalid JWT format") + } + + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return 0, fmt.Errorf("failed to decode JWT payload: %w", err) + } + + var claims struct { + Exp int64 `json:"exp"` + } + if err := json.Unmarshal(payload, &claims); err != nil { + return 0, fmt.Errorf("failed to unmarshal JWT payload: %w", err) + } + if claims.Exp <= 0 { + return 0, fmt.Errorf("JWT exp claim is missing") + } + return claims.Exp, nil +} + +func buildOIDCClient(credential Credential, idToken string) (*v2client.HarborAPI, error) { + tokenManager := &oidcTokenManager{ + credential: credential, + token: idToken, + refreshFn: refreshOIDCCredential, + } + + return buildClientWithAuth(credential.ServerAddress, tokenManager, &oidcRetryTransport{ + base: harbor.InsecureTransport, + tokenManager: tokenManager, + }) +} + +func buildClientWithAuth(serverAddress string, tokenManager *oidcTokenManager, transport http.RoundTripper) (*v2client.HarborAPI, error) { + u, err := url.Parse(serverAddress) + if err != nil { + return nil, fmt.Errorf("failed to parse server URL: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return nil, fmt.Errorf("invalid server URL: %s", serverAddress) + } + + cfg := &harbor.Config{ + URL: u, + Transport: transport, + AuthInfo: runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { + return req.SetHeaderParam("Authorization", "Bearer "+tokenManager.Token()) + }), } - return buildClientWithToken(credential.ServerAddress, idToken) + return v2client.New(cfg.ToV2Config()), nil } func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, error) { @@ -134,3 +256,93 @@ func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, e return v2client.New(cfg.ToV2Config()), nil } + +func (m *oidcTokenManager) Token() string { + m.mu.RLock() + defer m.mu.RUnlock() + return m.token +} + +func (m *oidcTokenManager) Refresh() (string, error) { + m.mu.Lock() + defer m.mu.Unlock() + + refreshFn := m.refreshFn + if refreshFn == nil { + refreshFn = refreshOIDCCredential + } + + token, err := refreshFn(m.credential) + if err != nil { + return "", err + } + m.token = token + return token, nil +} + +func (t *oidcRetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { + base := t.base + if base == nil { + base = harbor.InsecureTransport + } + + resp, err := base.RoundTrip(req) + if err != nil || resp == nil || resp.StatusCode != http.StatusUnauthorized { + return resp, err + } + if req.Header.Get(oidcRetryHeader) == "1" || !canRetryOIDCRequest(req) { + return resp, nil + } + + if _, err := t.tokenManager.Refresh(); err != nil { + drainAndCloseResponse(resp) + return nil, err + } + + drainAndCloseResponse(resp) + + retryReq, err := cloneRequestForRetry(req) + if err != nil { + return nil, err + } + retryReq.Header.Set("Authorization", "Bearer "+t.tokenManager.Token()) + retryReq.Header.Set(oidcRetryHeader, "1") + + return base.RoundTrip(retryReq) +} + +func canRetryOIDCRequest(req *http.Request) bool { + if req == nil { + return false + } + if req.Body == nil || req.Body == http.NoBody { + return true + } + return req.GetBody != nil +} + +func cloneRequestForRetry(req *http.Request) (*http.Request, error) { + retryReq := req.Clone(req.Context()) + if req.Body != nil && req.Body != http.NoBody { + if req.GetBody == nil { + return nil, fmt.Errorf("request body cannot be retried") + } + body, err := req.GetBody() + if err != nil { + return nil, fmt.Errorf("failed to reset request body: %w", err) + } + retryReq.Body = body + } else { + retryReq.Body = http.NoBody + } + retryReq.Header = req.Header.Clone() + return retryReq, nil +} + +func drainAndCloseResponse(resp *http.Response) { + if resp == nil || resp.Body == nil { + return + } + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 1024)) + _ = resp.Body.Close() +} diff --git a/pkg/utils/config.go b/pkg/utils/config.go index 2b3f1248f..0efad014a 100644 --- a/pkg/utils/config.go +++ b/pkg/utils/config.go @@ -612,11 +612,29 @@ func GetDecryptedIDToken(credentialName string) (string, error) { if credential.AuthType != AuthTypeOIDC { return "", fmt.Errorf("credential %q is not an OIDC credential", credentialName) } + return decryptOIDCCredentialValue(credential.IDToken) +} + +func GetDecryptedRefreshToken(credentialName string) (string, error) { + credential, err := GetCredentials(credentialName) + if err != nil { + return "", err + } + if credential.AuthType != AuthTypeOIDC { + return "", fmt.Errorf("credential %q is not an OIDC credential", credentialName) + } + if credential.RefreshToken == "" { + return "", nil + } + return decryptOIDCCredentialValue(credential.RefreshToken) +} + +func decryptOIDCCredentialValue(encryptedValue string) (string, error) { key, err := GetEncryptionKey() if err != nil { return "", fmt.Errorf("failed to get encryption key: %w", err) } - return Decrypt(key, credential.IDToken) + return Decrypt(key, encryptedValue) } func UpdateOIDCTokens(credentialName, idToken, refreshToken string, expiresAt int64, configPath string) error { diff --git a/pkg/utils/config_test.go b/pkg/utils/config_test.go index d84bbbfed..dd7c7c1a3 100644 --- a/pkg/utils/config_test.go +++ b/pkg/utils/config_test.go @@ -130,4 +130,19 @@ func Test_AddOIDCCredentials(t *testing.T) { idToken, err := utils.GetDecryptedIDToken(cred.Name) assert.NoError(t, err) assert.Equal(t, "id-token", idToken) + + refreshToken, err := utils.GetDecryptedRefreshToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "refresh-token", refreshToken) + + err = utils.UpdateOIDCTokens(cred.Name, "next-id-token", "next-refresh-token", 67890, data.ConfigPath) + assert.NoError(t, err) + + updatedIDToken, err := utils.GetDecryptedIDToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "next-id-token", updatedIDToken) + + updatedRefreshToken, err := utils.GetDecryptedRefreshToken(cred.Name) + assert.NoError(t, err) + assert.Equal(t, "next-refresh-token", updatedRefreshToken) } diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index 1a152b392..184c333f6 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -14,6 +14,7 @@ package utils import ( + "bytes" "encoding/json" "fmt" "io" @@ -23,8 +24,9 @@ import ( ) const ( - oidcCLILoginPath = "/c/oidc/login" - oidcCLITokenPath = "/c/oidc/cli-token" + oidcCLILoginPath = "/c/oidc/login" + oidcCLITokenPath = "/c/oidc/cli-token" + oidcCLIRefreshPath = "/c/oidc/refresh" ) type OIDCLoginResponse struct { @@ -41,6 +43,17 @@ type OIDCPollResponse struct { Error string `json:"error,omitempty"` } +type OIDCRefreshRequest struct { + RefreshToken string `json:"refresh_token"` +} + +type OIDCRefreshResponse struct { + IDToken string `json:"id_token"` + RefreshToken string `json:"refresh_token,omitempty"` + ExpiresAt int64 `json:"expires_at"` + Error string `json:"error,omitempty"` +} + func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { serverAddress = FormatUrl(serverAddress) if err := ValidateURL(serverAddress); err != nil { @@ -164,6 +177,59 @@ func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { } } +func RefreshOIDCToken(serverAddress, refreshToken string) (*OIDCRefreshResponse, error) { + if refreshToken == "" { + return nil, fmt.Errorf("refresh token is required") + } + + serverAddress = FormatUrl(serverAddress) + if err := ValidateURL(serverAddress); err != nil { + return nil, fmt.Errorf("invalid server URL: %w", err) + } + + endpoint, err := joinServerPath(serverAddress, oidcCLIRefreshPath) + if err != nil { + return nil, err + } + + payload, err := json.Marshal(&OIDCRefreshRequest{RefreshToken: refreshToken}) + if err != nil { + return nil, fmt.Errorf("failed to encode OIDC refresh request: %w", err) + } + + req, err := http.NewRequest(http.MethodPost, endpoint, bytes.NewReader(payload)) + if err != nil { + return nil, fmt.Errorf("failed to create OIDC refresh request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to refresh OIDC token: %w", err) + } + defer resp.Body.Close() + + var refreshResp OIDCRefreshResponse + switch resp.StatusCode { + case http.StatusOK: + if err := json.NewDecoder(resp.Body).Decode(&refreshResp); err != nil { + return nil, fmt.Errorf("failed to decode OIDC refresh response: %w", err) + } + if refreshResp.IDToken == "" { + return nil, fmt.Errorf("invalid OIDC refresh response: missing id_token") + } + return &refreshResp, nil + case http.StatusBadRequest: + if err := json.NewDecoder(resp.Body).Decode(&refreshResp); err == nil && refreshResp.Error != "" { + return nil, fmt.Errorf("OIDC refresh failed: %s", refreshResp.Error) + } + fallthrough + default: + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return nil, fmt.Errorf("failed to refresh OIDC token: status %d: %s", resp.StatusCode, string(body)) + } +} + func joinServerPath(serverAddress, path string) (string, error) { u, err := url.Parse(serverAddress) if err != nil { diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 4508738e7..6ec492b49 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -15,6 +15,7 @@ package utils_test import ( "encoding/json" + "io" "net/http" "net/http/httptest" "testing" @@ -78,3 +79,32 @@ func TestPollForOIDCTokenFailed(t *testing.T) { assert.Nil(t, resp) assert.ErrorContains(t, err, "state expired") } + +func TestRefreshOIDCToken(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/c/oidc/refresh", r.URL.Path) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + + var req utils.OIDCRefreshRequest + require.NoError(t, json.Unmarshal(body, &req)) + assert.Equal(t, "refresh-token-1", req.RefreshToken) + + _ = json.NewEncoder(w).Encode(utils.OIDCRefreshResponse{ + IDToken: "id-token-2", + RefreshToken: "refresh-token-2", + ExpiresAt: 1234567890, + }) + })) + defer server.Close() + + resp, err := utils.RefreshOIDCToken(server.URL, "refresh-token-1") + + require.NoError(t, err) + assert.Equal(t, "id-token-2", resp.IDToken) + assert.Equal(t, "refresh-token-2", resp.RefreshToken) + assert.Equal(t, int64(1234567890), resp.ExpiresAt) +} From d94bf4f404db60621f7e02b166bac7aa7c5fe541 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sun, 14 Jun 2026 12:30:38 +0530 Subject: [PATCH 14/25] basic implementation Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 91b6ed74f..268168e12 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -20,8 +20,8 @@ import ( "testing" "github.com/goharbor/harbor-cli/cmd/harbor/root" - helpers "github.com/goharbor/harbor-cli/test/helper" "github.com/goharbor/harbor-cli/pkg/utils" + helpers "github.com/goharbor/harbor-cli/test/helper" "github.com/stretchr/testify/assert" ) From 9470b1c6a5eccafa658fa538df0dece13048108e Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Mon, 15 Jun 2026 23:20:08 +0530 Subject: [PATCH 15/25] moved to state based login with existing endopoint Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 268168e12..41dce76cc 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -22,6 +22,7 @@ import ( "github.com/goharbor/harbor-cli/cmd/harbor/root" "github.com/goharbor/harbor-cli/pkg/utils" helpers "github.com/goharbor/harbor-cli/test/helper" + "github.com/goharbor/harbor-cli/pkg/utils" "github.com/stretchr/testify/assert" ) From 6f59953c509a8970c74644579383dfe70af7488f Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sat, 4 Jul 2026 22:37:56 +0530 Subject: [PATCH 16/25] added polling-token support Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 2 +- cmd/harbor/root/login_test.go | 4 ++-- pkg/utils/oidc.go | 19 ++++++++++++------- pkg/utils/oidc_test.go | 25 ++++++++++++++++++++----- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index a21a074ea..974bb386e 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -235,7 +235,7 @@ func RunOIDCLogin(serverAddress string) error { fmt.Printf("Open this URL in your browser to authenticate:\n\n %s\n\n", loginResp.RedirectURL) fmt.Print("Waiting for authentication...\n") - tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.State, 10*time.Minute) + tokenResp, err := utils.PollForOIDCToken(serverAddress, loginResp.PollToken, 10*time.Minute) if err != nil { return err } diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 41dce76cc..fa00d4bb2 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -160,10 +160,10 @@ func Test_RunOIDCLogin_Success(t *testing.T) { assert.Equal(t, "cli", r.URL.Query().Get("mode")) assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", - State: "state-1", + PollToken: "poll-token-1", })) case "/c/oidc/cli-token": - assert.Equal(t, "state-1", r.URL.Query().Get("state")) + assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index 184c333f6..f0dcd9a0b 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -31,7 +31,7 @@ const ( type OIDCLoginResponse struct { RedirectURL string `json:"redirect_url"` - State string `json:"state"` + PollToken string `json:"poll_token"` } type OIDCPollResponse struct { @@ -87,15 +87,15 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { if err := json.NewDecoder(resp.Body).Decode(&loginResp); err != nil { return nil, fmt.Errorf("failed to decode OIDC login response: %w", err) } - if loginResp.RedirectURL == "" || loginResp.State == "" { - return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or state") + if loginResp.RedirectURL == "" || loginResp.PollToken == "" { + return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or poll_token") } return &loginResp, nil } -func PollForOIDCToken(serverAddress, state string, timeout time.Duration) (*OIDCPollResponse, error) { - if state == "" { - return nil, fmt.Errorf("state is required") +func PollForOIDCToken(serverAddress, pollToken string, timeout time.Duration) (*OIDCPollResponse, error) { + if pollToken == "" { + return nil, fmt.Errorf("poll token is required") } serverAddress = FormatUrl(serverAddress) if err := ValidateURL(serverAddress); err != nil { @@ -111,7 +111,7 @@ func PollForOIDCToken(serverAddress, state string, timeout time.Duration) (*OIDC return nil, fmt.Errorf("failed to parse OIDC token endpoint: %w", err) } q := u.Query() - q.Set("state", state) + q.Set("poll_token", pollToken) u.RawQuery = q.Encode() deadline := time.Now().Add(timeout) @@ -171,6 +171,11 @@ func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { return nil, false, fmt.Errorf("OIDC authentication failed: %s", pollResp.Error) } return nil, false, fmt.Errorf("OIDC authentication failed") + case http.StatusGone: + if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { + return nil, false, fmt.Errorf("OIDC login expired before token retrieval") + } + return nil, false, fmt.Errorf("OIDC login expired before token retrieval") default: body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) return nil, false, fmt.Errorf("failed to poll OIDC token: status %d: %s", resp.StatusCode, string(body)) diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 6ec492b49..2d069c931 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -32,7 +32,7 @@ func TestInitiateOIDCLogin(t *testing.T) { assert.Equal(t, "cli", r.URL.Query().Get("mode")) _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", - State: "state-1", + PollToken: "poll-token-1", }) })) defer server.Close() @@ -41,13 +41,13 @@ func TestInitiateOIDCLogin(t *testing.T) { require.NoError(t, err) assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) - assert.Equal(t, "state-1", resp.State) + assert.Equal(t, "poll-token-1", resp.PollToken) } func TestPollForOIDCTokenReady(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) - assert.Equal(t, "state-1", r.URL.Query().Get("state")) + assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", @@ -56,7 +56,7 @@ func TestPollForOIDCTokenReady(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) require.NoError(t, err) assert.Equal(t, "ready", resp.Status) @@ -74,12 +74,27 @@ func TestPollForOIDCTokenFailed(t *testing.T) { })) defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "state-1", time.Second) + resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) assert.Nil(t, resp) assert.ErrorContains(t, err, "state expired") } +func TestPollForOIDCTokenExpired(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusGone) + _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "expired", + }) + })) + defer server.Close() + + resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) + + assert.Nil(t, resp) + assert.ErrorContains(t, err, "expired before token retrieval") +} + func TestRefreshOIDCToken(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodPost, r.Method) From a0fa53295f024692e13f8004007bfbecc3105803 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sat, 4 Jul 2026 22:57:29 +0530 Subject: [PATCH 17/25] added test Signed-off-by: Nishchay Rajput --- pkg/utils/client_oidc_internal_test.go | 99 ++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 pkg/utils/client_oidc_internal_test.go diff --git a/pkg/utils/client_oidc_internal_test.go b/pkg/utils/client_oidc_internal_test.go new file mode 100644 index 000000000..0d92c1d27 --- /dev/null +++ b/pkg/utils/client_oidc_internal_test.go @@ -0,0 +1,99 @@ +package utils + +import ( + "encoding/base64" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOIDCTokenExpiryUnix(t *testing.T) { + token := testJWTWithExp(time.Now().Add(10 * time.Minute).Unix()) + + expiresAt, err := oidcTokenExpiryUnix(token) + + require.NoError(t, err) + assert.Greater(t, expiresAt, time.Now().Unix()) +} + +func TestOIDCTokenNeedsRefreshPrefersTokenExpiry(t *testing.T) { + credential := Credential{ + ExpiresAt: time.Now().Add(2 * time.Hour).Unix(), + } + token := testJWTWithExp(time.Now().Add(30 * time.Second).Unix()) + + assert.True(t, oidcTokenNeedsRefresh(credential, token)) +} + +func TestOIDCTokenNeedsRefreshFallsBackToStoredExpiry(t *testing.T) { + credential := Credential{ + ExpiresAt: time.Now().Add(30 * time.Second).Unix(), + } + + assert.True(t, oidcTokenNeedsRefresh(credential, "not-a-jwt")) +} + +func testJWTWithExp(exp int64) string { + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"none","typ":"JWT"}`)) + payload := base64.RawURLEncoding.EncodeToString([]byte(fmt.Sprintf(`{"exp":%d}`, exp))) + return header + "." + payload + ".signature" +} + +func TestOIDCRetryTransportRefreshesAndRetriesOnUnauthorized(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + switch r.Header.Get("Authorization") { + case "Bearer stale-token": + w.WriteHeader(http.StatusUnauthorized) + case "Bearer fresh-token": + w.WriteHeader(http.StatusOK) + default: + w.WriteHeader(http.StatusForbidden) + } + })) + defer server.Close() + + tokenManager := &oidcTokenManager{ + token: "stale-token", + refreshFn: func(_ Credential) (string, error) { + return "fresh-token", nil + }, + } + + transport := &oidcRetryTransport{ + base: server.Client().Transport, + tokenManager: tokenManager, + } + + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+tokenManager.Token()) + + resp, err := transport.RoundTrip(req) + + require.NoError(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "fresh-token", tokenManager.Token()) + assert.Equal(t, 2, requests) +} + +func TestCloneRequestForRetryWithReplayableBody(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "https://example.com", strings.NewReader("payload")) + require.NoError(t, err) + + cloned, err := cloneRequestForRetry(req) + + require.NoError(t, err) + body, err := io.ReadAll(cloned.Body) + require.NoError(t, err) + assert.Equal(t, "payload", string(body)) +} From 1a370a546e46e9a840ac9f288083af18eb2380d8 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Sun, 5 Jul 2026 00:33:48 +0530 Subject: [PATCH 18/25] fix: lint and genrate docs Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login_test.go | 1 - doc/cli-docs/harbor-login.md | 1 + doc/man-docs/man1/harbor-login.1 | 4 ++++ pkg/utils/client.go | 19 ------------------- pkg/utils/client_oidc_internal_test.go | 15 ++++++++++++++- pkg/utils/oidc.go | 2 +- 6 files changed, 20 insertions(+), 22 deletions(-) diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index fa00d4bb2..4ebbde177 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -22,7 +22,6 @@ import ( "github.com/goharbor/harbor-cli/cmd/harbor/root" "github.com/goharbor/harbor-cli/pkg/utils" helpers "github.com/goharbor/harbor-cli/test/helper" - "github.com/goharbor/harbor-cli/pkg/utils" "github.com/stretchr/testify/assert" ) diff --git a/doc/cli-docs/harbor-login.md b/doc/cli-docs/harbor-login.md index 0c67d1cee..fe21c83ee 100644 --- a/doc/cli-docs/harbor-login.md +++ b/doc/cli-docs/harbor-login.md @@ -21,6 +21,7 @@ harbor login [server] [flags] ```sh -n, --context-name string Login context name (optional) -h, --help help for login + --oidc Authenticate using Harbor OIDC browser login -p, --password string Password (not recommended, use --password-stdin for better security) --password-stdin Take the password from stdin --skip-verify-client Skip whether the clients basic auth credentials shall be validated against the Harbor server during login. This is not recommended as it may lead to storing invalid credentials. Use this flag if you want to skip validation of credentials during login, for example, when the Harbor server is not reachable at the moment of login but you still want to store the credentials for later use. diff --git a/doc/man-docs/man1/harbor-login.1 b/doc/man-docs/man1/harbor-login.1 index 0384a43a8..ae9046908 100644 --- a/doc/man-docs/man1/harbor-login.1 +++ b/doc/man-docs/man1/harbor-login.1 @@ -21,6 +21,10 @@ Authenticate with Harbor Registry. \fB-h\fP, \fB--help\fP[=false] help for login +.PP +\fB--oidc\fP[=false] + Authenticate using Harbor OIDC browser login + .PP \fB-p\fP, \fB--password\fP="" Password (not recommended, use --password-stdin for better security) diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 5a953bd0e..df7e24c52 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -238,25 +238,6 @@ func buildClientWithAuth(serverAddress string, tokenManager *oidcTokenManager, t return v2client.New(cfg.ToV2Config()), nil } -func buildClientWithToken(serverAddress, idToken string) (*v2client.HarborAPI, error) { - u, err := url.Parse(serverAddress) - if err != nil { - return nil, fmt.Errorf("failed to parse server URL: %w", err) - } - if u.Scheme == "" || u.Host == "" { - return nil, fmt.Errorf("invalid server URL: %s", serverAddress) - } - - cfg := &harbor.Config{ - URL: u, - AuthInfo: runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error { - return req.SetHeaderParam("Authorization", "Bearer "+idToken) - }), - } - - return v2client.New(cfg.ToV2Config()), nil -} - func (m *oidcTokenManager) Token() string { m.mu.RLock() defer m.mu.RUnlock() diff --git a/pkg/utils/client_oidc_internal_test.go b/pkg/utils/client_oidc_internal_test.go index 0d92c1d27..6bfa6bcb1 100644 --- a/pkg/utils/client_oidc_internal_test.go +++ b/pkg/utils/client_oidc_internal_test.go @@ -1,3 +1,16 @@ +// 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 utils import ( @@ -78,7 +91,7 @@ func TestOIDCRetryTransportRefreshesAndRetriesOnUnauthorized(t *testing.T) { req.Header.Set("Authorization", "Bearer "+tokenManager.Token()) resp, err := transport.RoundTrip(req) - + defer resp.Body.Close() require.NoError(t, err) require.NotNil(t, resp) assert.Equal(t, http.StatusOK, resp.StatusCode) diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index f0dcd9a0b..936f60b2c 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -25,7 +25,7 @@ import ( const ( oidcCLILoginPath = "/c/oidc/login" - oidcCLITokenPath = "/c/oidc/cli-token" + oidcCLITokenPath = "/c/oidc/cli-token" //nolint:gosec // endpoint is user-provided Harbor server URL for login. oidcCLIRefreshPath = "/c/oidc/refresh" ) From ebb3f9dbb7ef77fa9de86db170946f4ed80820ee Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Fri, 10 Jul 2026 00:31:34 +0530 Subject: [PATCH 19/25] fix: reviewed changes Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 2 +- pkg/utils/client.go | 4 ++-- pkg/utils/config.go | 4 ++++ pkg/utils/oidc.go | 12 ++++++++---- pkg/utils/oidc_test.go | 18 ++++++++++++++++++ 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index 974bb386e..8c9aa0621 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -242,7 +242,7 @@ func RunOIDCLogin(serverAddress string) error { harborData, err := utils.GetCurrentHarborData() if err != nil { - return fmt.Errorf("failed to get current harbor data: %s", err) + return fmt.Errorf("failed to get current harbor data: %w", err) } if err := utils.AddOIDCCredentials(serverAddress, tokenResp.Username, tokenResp.IDToken, tokenResp.RefreshToken, tokenResp.ExpiresAt, harborData.ConfigPath); err != nil { diff --git a/pkg/utils/client.go b/pkg/utils/client.go index df7e24c52..c6a9e3215 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -213,7 +213,7 @@ func buildOIDCClient(credential Credential, idToken string) (*v2client.HarborAPI } return buildClientWithAuth(credential.ServerAddress, tokenManager, &oidcRetryTransport{ - base: harbor.InsecureTransport, + base: http.DefaultTransport, tokenManager: tokenManager, }) } @@ -264,7 +264,7 @@ func (m *oidcTokenManager) Refresh() (string, error) { func (t *oidcRetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { base := t.base if base == nil { - base = harbor.InsecureTransport + base = http.DefaultTransport } resp, err := base.RoundTrip(req) diff --git a/pkg/utils/config.go b/pkg/utils/config.go index 0efad014a..f5a49bc4e 100644 --- a/pkg/utils/config.go +++ b/pkg/utils/config.go @@ -514,7 +514,9 @@ func AddCredentialsToConfigFile(credential Credential, configPath string) error log.Fatalf("failed to write updated config file: %v", err) } + configMutex.Lock() CurrentHarborConfig = &c + configMutex.Unlock() fmt.Printf("Added credential '%s' to config file at %s\n", credential.Name, configPath) return nil } @@ -560,7 +562,9 @@ func UpdateCredentialsInConfigFile(updatedCredential Credential, configPath stri log.Fatalf("failed to write updated config file: %v", err) } + configMutex.Lock() CurrentHarborConfig = &c + configMutex.Unlock() fmt.Printf("Updated credential '%s' in config file at %s.\n", updatedCredential.Name, configPath) fmt.Printf("Switched to context '%s'\n", updatedCredential.Name) return nil diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index 936f60b2c..df49abe7c 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -72,7 +72,7 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { q.Set("mode", "cli") u.RawQuery = q.Encode() - resp, err := http.Get(u.String()) //nolint:gosec // endpoint is user-provided Harbor server URL for login. + resp, err := (&http.Client{Timeout: 30 * time.Second}).Get(u.String()) //nolint:gosec // endpoint is user-provided Harbor server URL for login. if err != nil { return nil, fmt.Errorf("failed to initiate OIDC login: %w", err) } @@ -142,7 +142,7 @@ func PollForOIDCToken(serverAddress, pollToken string, timeout time.Duration) (* } func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { - resp, err := http.Get(endpoint) //nolint:gosec // endpoint is the Harbor server URL validated by PollForOIDCToken. + resp, err := (&http.Client{Timeout: 30 * time.Second}).Get(endpoint) //nolint:gosec // endpoint is the Harbor server URL validated by PollForOIDCToken. if err != nil { return nil, false, fmt.Errorf("failed to poll OIDC token: %w", err) } @@ -208,7 +208,7 @@ func RefreshOIDCToken(serverAddress, refreshToken string) (*OIDCRefreshResponse, } req.Header.Set("Content-Type", "application/json") - resp, err := http.DefaultClient.Do(req) + resp, err := (&http.Client{Timeout: 30 * time.Second}).Do(req) if err != nil { return nil, fmt.Errorf("failed to refresh OIDC token: %w", err) } @@ -240,7 +240,11 @@ func joinServerPath(serverAddress, path string) (string, error) { if err != nil { return "", fmt.Errorf("failed to parse server URL: %w", err) } - u.Path = path + basePath := u.Path + for len(basePath) > 0 && basePath[len(basePath)-1] == '/' { + basePath = basePath[:len(basePath)-1] + } + u.Path = basePath + path u.RawQuery = "" u.Fragment = "" return u.String(), nil diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 2d069c931..75a8cdb68 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -18,6 +18,7 @@ import ( "io" "net/http" "net/http/httptest" + "sync/atomic" "testing" "time" @@ -95,6 +96,23 @@ func TestPollForOIDCTokenExpired(t *testing.T) { assert.ErrorContains(t, err, "expired before token retrieval") } +func TestPollForOIDCTokenTimeoutWhilePending(t *testing.T) { + var requests int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&requests, 1) + assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) + assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) + w.WriteHeader(http.StatusAccepted) + })) + defer server.Close() + + resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", 100*time.Millisecond) + + assert.Nil(t, resp) + assert.ErrorContains(t, err, "timed out waiting for OIDC authentication") + assert.Equal(t, int32(1), atomic.LoadInt32(&requests)) +} + func TestRefreshOIDCToken(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodPost, r.Method) From b50513fb6e39f422416b66b4695cb4398f819caa Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Mon, 20 Jul 2026 22:49:11 +0530 Subject: [PATCH 20/25] fix: reviewed changes, added test and debug log Signed-off-by: Nishchay Rajput --- pkg/utils/client.go | 1 + pkg/utils/oidc_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/pkg/utils/client.go b/pkg/utils/client.go index c6a9e3215..c0bf21b0e 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -174,6 +174,7 @@ func refreshOIDCCredential(credential Credential) (string, error) { func oidcTokenNeedsRefresh(credential Credential, idToken string) bool { expiresAt, err := oidcTokenExpiryUnix(idToken) if err != nil { + log.Debugf("failed to parse OIDC token expiry from JWT, falling back to stored expires_at: %v", err) expiresAt = credential.ExpiresAt } if expiresAt <= 0 { diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 75a8cdb68..aacc36b42 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -45,6 +45,24 @@ func TestInitiateOIDCLogin(t *testing.T) { assert.Equal(t, "poll-token-1", resp.PollToken) } +func TestInitiateOIDCLoginPreservesBasePath(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/harbor/c/oidc/login", r.URL.Path) + assert.Equal(t, "cli", r.URL.Query().Get("mode")) + _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + RedirectURL: "https://idp.example/authorize", + PollToken: "poll-token-1", + }) + })) + defer server.Close() + + resp, err := utils.InitiateOIDCLogin(server.URL + "/harbor") + + require.NoError(t, err) + assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) + assert.Equal(t, "poll-token-1", resp.PollToken) +} + func TestPollForOIDCTokenReady(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) From 3aec5de9f38a35d8773a177842e852d1954f5876 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Thu, 6 Aug 2026 13:49:19 +0530 Subject: [PATCH 21/25] fix: reviewed changes added logs for verbose mode and check agains incompatible harbor platform version. Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login.go | 189 +++++++++++++++++++++++++++++-- cmd/harbor/root/login_test.go | 97 +++++++++++++++- doc/cli-docs/harbor-login.md | 3 +- doc/man-docs/man1/harbor-login.1 | 6 +- pkg/utils/client.go | 2 +- pkg/utils/oidc.go | 44 ++++++- pkg/utils/oidc_test.go | 27 +++++ 7 files changed, 346 insertions(+), 22 deletions(-) diff --git a/cmd/harbor/root/login.go b/cmd/harbor/root/login.go index 8c9aa0621..659b856b9 100644 --- a/cmd/harbor/root/login.go +++ b/cmd/harbor/root/login.go @@ -15,7 +15,11 @@ package root import ( "context" + "encoding/json" "fmt" + "io" + "net/http" + "net/url" "os" "strings" "time" @@ -39,11 +43,33 @@ var ( Name string passwordStdin bool skipVerifyClient bool - oidcLogin bool + authMode string ) +const ( + cliAuthModeDB = "db" + cliAuthModeLDAP = "ldap" + cliAuthModeOIDC = "oidc" + + harborAuthModeDB = "db_auth" + harborAuthModeLDAP = "ldap_auth" + harborAuthModeOIDC = "oidc_auth" +) + +func resetLoginOptions() { + serverAddress = "" + Username = "" + Password = "" + Name = "" + passwordStdin = false + skipVerifyClient = false + authMode = "" +} + // LoginCommand creates a new `harbor login` command func LoginCommand() *cobra.Command { + resetLoginOptions() + cmd := &cobra.Command{ Use: "login [server]", Short: "Log in to Harbor registry", @@ -54,10 +80,6 @@ func LoginCommand() *cobra.Command { serverAddress = args[0] } - if oidcLogin { - return RunOIDCLogin(serverAddress) - } - if passwordStdin { fmt.Print("Password: ") passwordBytes, err := term.ReadPassword(int(os.Stdin.Fd())) // #nosec G115 - fd fits in int on all supported platforms @@ -75,7 +97,14 @@ func LoginCommand() *cobra.Command { Name: Name, } - var err error + effectiveMode, err := resolveLoginAuthMode(loginView) + if err != nil { + return err + } + if effectiveMode == cliAuthModeOIDC { + return RunOIDCLogin(serverAddress) + } + var config *utils.HarborConfig config, err = utils.GetCurrentHarborConfig() if err != nil { @@ -93,17 +122,154 @@ func LoginCommand() *cobra.Command { flags.StringVarP(&Name, "context-name", "n", "", "Login context name (optional)") flags.StringVarP(&Password, "password", "p", "", "Password (not recommended, use --password-stdin for better security)") flags.BoolVar(&passwordStdin, "password-stdin", false, "Take the password from stdin") - flags.BoolVar(&oidcLogin, "oidc", false, "Authenticate using Harbor OIDC browser login") + flags.StringVar(&authMode, "auth-mode", "", "Authentication mode (db, ldap, oidc)") flags.BoolVarP(&skipVerifyClient, "skip-verify-client", "", false, "Skip whether the clients basic auth credentials shall be validated against the Harbor server during login. This is not recommended as it may lead to storing invalid credentials. Use this flag if you want to skip validation of credentials during login, for example, when the Harbor server is not reachable at the moment of login but you still want to store the credentials for later use.") cmd.MarkFlagsMutuallyExclusive("password", "password-stdin") - cmd.MarkFlagsMutuallyExclusive("oidc", "username") - cmd.MarkFlagsMutuallyExclusive("oidc", "password") - cmd.MarkFlagsMutuallyExclusive("oidc", "password-stdin") return cmd } +func resolveLoginAuthMode(loginView login.LoginView) (string, error) { + requestedMode, err := normalizeCLIAuthMode(authMode) + if err != nil { + return "", err + } + log.Debugf("resolving login auth mode for server=%q requested_mode=%q username_provided=%t password_provided=%t", loginView.Server, requestedMode, loginView.Username != "", loginView.Password != "") + + if requestedMode == "" { + if loginView.Server == "" { + return "", nil + } + if loginView.Username != "" || loginView.Password != "" { + return "", nil + } + + harborMode, err := getHarborAuthMode(loginView.Server) + if err != nil { + return "", fmt.Errorf("unable to determine Harbor auth_mode from /api/v2.0/systeminfo. Please retry with --auth-mode db, --auth-mode ldap, or --auth-mode oidc: %w", err) + } + log.Debugf("auto-detected Harbor auth_mode=%q for server=%q", harborMode, loginView.Server) + switch harborMode { + case harborAuthModeDB: + log.Debug("selected CLI auth mode db from Harbor auth_mode db_auth") + return cliAuthModeDB, nil + case harborAuthModeLDAP: + log.Debug("selected CLI auth mode ldap from Harbor auth_mode ldap_auth") + return cliAuthModeLDAP, nil + case harborAuthModeOIDC: + log.Debug("selected CLI auth mode oidc from Harbor auth_mode oidc_auth") + return cliAuthModeOIDC, nil + default: + return "", fmt.Errorf("unsupported Harbor auth_mode %q returned by /api/v2.0/systeminfo", harborMode) + } + } + + if loginView.Server == "" { + return "", fmt.Errorf("server address is required when --auth-mode is set") + } + + if requestedMode == cliAuthModeOIDC && (loginView.Username != "" || loginView.Password != "") { + return "", fmt.Errorf("--auth-mode oidc cannot be used with --username, --password, or --password-stdin") + } + + harborMode, err := getHarborAuthMode(loginView.Server) + if err != nil { + return "", fmt.Errorf("failed to determine Harbor auth_mode for --auth-mode %s: %w", requestedMode, err) + } + if err := validateAuthModeCombination(requestedMode, harborMode); err != nil { + return "", err + } + log.Debugf("validated explicit CLI auth mode %q against Harbor auth_mode %q", requestedMode, harborMode) + + return requestedMode, nil +} + +func normalizeCLIAuthMode(mode string) (string, error) { + mode = strings.ToLower(strings.TrimSpace(mode)) + switch mode { + case "": + return "", nil + case cliAuthModeDB, cliAuthModeLDAP, cliAuthModeOIDC: + return mode, nil + default: + return "", fmt.Errorf("invalid auth mode %q. Valid values are: db, ldap, oidc", mode) + } +} + +func validateAuthModeCombination(requestedMode, harborMode string) error { + switch requestedMode { + case cliAuthModeOIDC: + if harborMode != harborAuthModeOIDC { + return fmt.Errorf("OIDC login is not available because Harbor auth_mode is %s", harborMode) + } + case cliAuthModeLDAP: + if harborMode != harborAuthModeLDAP { + return fmt.Errorf("LDAP login is not available because Harbor auth_mode is %s", harborMode) + } + case cliAuthModeDB: + if harborMode == harborAuthModeLDAP { + return fmt.Errorf("DB login is not available because Harbor auth_mode is %s", harborMode) + } + default: + return fmt.Errorf("unsupported auth mode %q", requestedMode) + } + return nil +} + +func getHarborAuthMode(server string) (string, error) { + server = utils.FormatUrl(server) + if err := utils.ValidateURL(server); err != nil { + return "", fmt.Errorf("invalid server URL: %w", err) + } + + endpoint, err := joinServerPath(server, "/api/v2.0/systeminfo") + if err != nil { + return "", err + } + log.Debugf("querying Harbor system info at %s", endpoint) + + resp, err := (&http.Client{Timeout: 30 * time.Second}).Get(endpoint) //nolint:gosec // endpoint is user-provided Harbor server URL. + if err != nil { + return "", fmt.Errorf("failed to query Harbor system info: %w", err) + } + defer resp.Body.Close() + log.Debugf("received Harbor system info response status %d", resp.StatusCode) + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return "", fmt.Errorf("unexpected status %d from /api/v2.0/systeminfo: %s", resp.StatusCode, string(body)) + } + + var payload struct { + AuthMode string `json:"auth_mode"` + } + if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil { + return "", fmt.Errorf("failed to decode Harbor system info response: %w", err) + } + if payload.AuthMode == "" { + return "", fmt.Errorf("Harbor system info response is missing auth_mode") + } + log.Debugf("Harbor system info reported auth_mode=%q", payload.AuthMode) + + return payload.AuthMode, nil +} + +func joinServerPath(serverAddress, path string) (string, error) { + u, err := url.Parse(serverAddress) + if err != nil { + return "", fmt.Errorf("failed to parse server URL: %w", err) + } + basePath := u.Path + for len(basePath) > 0 && basePath[len(basePath)-1] == '/' { + basePath = basePath[:len(basePath)-1] + } + u.Path = basePath + path + u.RawQuery = "" + u.Fragment = "" + return u.String(), nil +} + // ProcessLogin applies a simplified decision logic to run login or launch an interactive view. func ProcessLogin(loginView login.LoginView, config *utils.HarborConfig) error { // Auto-generate the name if not provided. @@ -226,11 +392,13 @@ func RunOIDCLogin(serverAddress string) error { if err := utils.ValidateURL(serverAddress); err != nil { return fmt.Errorf("invalid server URL: %w", err) } + log.Debugf("starting Harbor CLI OIDC login for server %s", serverAddress) loginResp, err := utils.InitiateOIDCLogin(serverAddress) if err != nil { return err } + log.Debug("received Harbor CLI OIDC login redirect URL and poll token") fmt.Printf("Open this URL in your browser to authenticate:\n\n %s\n\n", loginResp.RedirectURL) fmt.Print("Waiting for authentication...\n") @@ -239,6 +407,7 @@ func RunOIDCLogin(serverAddress string) error { if err != nil { return err } + log.Debugf("Harbor CLI OIDC login completed for user %s", tokenResp.Username) harborData, err := utils.GetCurrentHarborData() if err != nil { diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index 4ebbde177..a1be8ef22 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -127,19 +127,32 @@ func Test_Login_Failure_MutuallyExclusiveFlags(t *testing.T) { assert.Error(t, err, "Expected error when both --password and --password-stdin are set") } -func Test_Login_Failure_OIDCMutuallyExclusiveFlags(t *testing.T) { +func Test_Login_Failure_InvalidAuthModeForOIDCHarbor(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) { + switch r.URL.Path { + case "/api/v2.0/systeminfo": + assert.NoError(t, json.NewEncoder(w).Encode(map[string]string{ + "auth_mode": "oidc_auth", + })) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + cmd := root.LoginCommand() - cmd.SetArgs([]string{"http://demo.goharbor.io"}) + cmd.SetArgs([]string{server.URL}) - assert.NoError(t, cmd.Flags().Set("oidc", "true")) + assert.NoError(t, cmd.Flags().Set("auth-mode", "ldap")) assert.NoError(t, cmd.Flags().Set("username", "admin")) + assert.NoError(t, cmd.Flags().Set("password", "Harbor12345")) err := cmd.Execute() - assert.Error(t, err, "Expected error when --oidc and --username are set") + assert.ErrorContains(t, err, "LDAP login is not available because Harbor auth_mode is oidc_auth") } func Test_RunOIDCLogin_Failure_MissingServer(t *testing.T) { @@ -147,6 +160,82 @@ func Test_RunOIDCLogin_Failure_MissingServer(t *testing.T) { assert.Error(t, err) } +func Test_Login_AutoDetectsOIDCAuthMode(t *testing.T) { + tempDir := t.TempDir() + helpers.SetMockKeyring(t) + data := helpers.Initialize(t, tempDir) + defer helpers.ConfigCleanup(t, data) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v2.0/systeminfo": + assert.NoError(t, json.NewEncoder(w).Encode(map[string]string{ + "auth_mode": "oidc_auth", + })) + case "/c/oidc/login": + assert.Equal(t, "cli", r.URL.Query().Get("mode")) + assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + RedirectURL: "https://idp.example/authorize", + PollToken: "poll-token-1", + })) + case "/c/oidc/cli-token": + assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) + assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + Status: "ready", + IDToken: "id-token", + Username: "alice", + })) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + cmd := root.LoginCommand() + cmd.SetArgs([]string{server.URL}) + + err := cmd.Execute() + assert.NoError(t, err) + + cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", server.URL)) + assert.NoError(t, err) + assert.Equal(t, utils.AuthTypeOIDC, cred.AuthType) +} + +func Test_Login_AllowsDBAuthModeWhenHarborUsesOIDC(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) { + switch r.URL.Path { + case "/api/v2.0/systeminfo": + assert.NoError(t, json.NewEncoder(w).Encode(map[string]string{ + "auth_mode": "oidc_auth", + })) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + cmd := root.LoginCommand() + cmd.SetArgs([]string{server.URL}) + + assert.NoError(t, cmd.Flags().Set("auth-mode", "db")) + assert.NoError(t, cmd.Flags().Set("username", "alice")) + assert.NoError(t, cmd.Flags().Set("password", "cli-secret")) + assert.NoError(t, cmd.Flags().Set("skip-verify-client", "true")) + + err := cmd.Execute() + assert.NoError(t, err) + + cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", server.URL)) + assert.NoError(t, err) + assert.Equal(t, "alice", cred.Username) + assert.Empty(t, cred.AuthType) +} + func Test_RunOIDCLogin_Success(t *testing.T) { tempDir := t.TempDir() helpers.SetMockKeyring(t) diff --git a/doc/cli-docs/harbor-login.md b/doc/cli-docs/harbor-login.md index fe21c83ee..3dcda0fc8 100644 --- a/doc/cli-docs/harbor-login.md +++ b/doc/cli-docs/harbor-login.md @@ -21,7 +21,7 @@ harbor login [server] [flags] ```sh -n, --context-name string Login context name (optional) -h, --help help for login - --oidc Authenticate using Harbor OIDC browser login + --auth-mode string Authentication mode (db, ldap, oidc) -p, --password string Password (not recommended, use --password-stdin for better security) --password-stdin Take the password from stdin --skip-verify-client Skip whether the clients basic auth credentials shall be validated against the Harbor server during login. This is not recommended as it may lead to storing invalid credentials. Use this flag if you want to skip validation of credentials during login, for example, when the Harbor server is not reachable at the moment of login but you still want to store the credentials for later use. @@ -39,4 +39,3 @@ harbor login [server] [flags] ### SEE ALSO * [harbor](harbor.md) - Official Harbor CLI - diff --git a/doc/man-docs/man1/harbor-login.1 b/doc/man-docs/man1/harbor-login.1 index ae9046908..3229e6f7e 100644 --- a/doc/man-docs/man1/harbor-login.1 +++ b/doc/man-docs/man1/harbor-login.1 @@ -22,8 +22,8 @@ Authenticate with Harbor Registry. help for login .PP -\fB--oidc\fP[=false] - Authenticate using Harbor OIDC browser login +\fB--auth-mode\fP="" + Authentication mode (db, ldap, oidc) .PP \fB-p\fP, \fB--password\fP="" @@ -56,4 +56,4 @@ Authenticate with Harbor Registry. .SH SEE ALSO -\fBharbor(1)\fP \ No newline at end of file +\fBharbor(1)\fP diff --git a/pkg/utils/client.go b/pkg/utils/client.go index c0bf21b0e..6f8e85cea 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -38,7 +38,7 @@ var ( ClientErr error ) -const oidcRefreshFailureMessage = "Unable to refresh OIDC session. Please try again or run 'harbor login --oidc'." +const oidcRefreshFailureMessage = "Unable to refresh OIDC session. Please try again or run 'harbor login --auth-mode oidc'." const oidcRetryHeader = "X-Harbor-CLI-OIDC-Retry" diff --git a/pkg/utils/oidc.go b/pkg/utils/oidc.go index df49abe7c..7c59faf08 100644 --- a/pkg/utils/oidc.go +++ b/pkg/utils/oidc.go @@ -20,7 +20,10 @@ import ( "io" "net/http" "net/url" + "strings" "time" + + log "github.com/sirupsen/logrus" ) const ( @@ -71,25 +74,51 @@ func InitiateOIDCLogin(serverAddress string) (*OIDCLoginResponse, error) { q := u.Query() q.Set("mode", "cli") u.RawQuery = q.Encode() + log.Debugf("initiating Harbor CLI OIDC login against %s", u.String()) + + client := &http.Client{ + Timeout: 30 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } - resp, err := (&http.Client{Timeout: 30 * time.Second}).Get(u.String()) //nolint:gosec // endpoint is user-provided Harbor server URL for login. + resp, err := client.Get(u.String()) //nolint:gosec // endpoint is user-provided Harbor server URL for login. if err != nil { return nil, fmt.Errorf("failed to initiate OIDC login: %w", err) } defer resp.Body.Close() + log.Debugf("received OIDC login response status %d from %s", resp.StatusCode, u.String()) + + if resp.StatusCode >= http.StatusMultipleChoices && resp.StatusCode < http.StatusBadRequest { + location := resp.Header.Get("Location") + if location != "" { + log.Debugf("Harbor returned browser redirect for CLI OIDC login: %s", location) + return nil, fmt.Errorf("This Harbor instance may not support CLI OIDC login yet") + } + log.Debug("Harbor returned an HTTP redirect instead of a CLI OIDC JSON response") + return nil, fmt.Errorf("Harbor did not return a CLI OIDC login response and issued an HTTP redirect instead. This Harbor instance may not support CLI OIDC login yet") + } if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) return nil, fmt.Errorf("failed to initiate OIDC login: status %d: %s", resp.StatusCode, string(body)) } + if contentType := resp.Header.Get("Content-Type"); contentType != "" && !strings.Contains(strings.ToLower(contentType), "application/json") { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + log.Debugf("Harbor returned non-JSON OIDC login response with content-type %q", contentType) + return nil, fmt.Errorf("Harbor did not return a CLI OIDC login JSON response (content-type %q). This Harbor instance may not support CLI OIDC login yet: %s", contentType, string(body)) + } + var loginResp OIDCLoginResponse if err := json.NewDecoder(resp.Body).Decode(&loginResp); err != nil { - return nil, fmt.Errorf("failed to decode OIDC login response: %w", err) + return nil, fmt.Errorf("failed to decode OIDC login response: %w. This Harbor instance may not support CLI OIDC login yet", err) } if loginResp.RedirectURL == "" || loginResp.PollToken == "" { return nil, fmt.Errorf("invalid OIDC login response: missing redirect_url or poll_token") } + log.Debug("received Harbor CLI OIDC login payload successfully") return &loginResp, nil } @@ -113,6 +142,7 @@ func PollForOIDCToken(serverAddress, pollToken string, timeout time.Duration) (* q := u.Query() q.Set("poll_token", pollToken) u.RawQuery = q.Encode() + log.Debugf("starting Harbor CLI OIDC polling against %s with timeout %s", u.String(), timeout) deadline := time.Now().Add(timeout) ticker := time.NewTicker(3 * time.Second) @@ -124,9 +154,11 @@ func PollForOIDCToken(serverAddress, pollToken string, timeout time.Duration) (* return nil, err } if ready { + log.Debug("Harbor CLI OIDC polling completed with ready token response") return result, nil } if time.Now().After(deadline) { + log.Debug("Harbor CLI OIDC polling timed out while waiting for authentication") return nil, fmt.Errorf("timed out waiting for OIDC authentication") } remaining := time.Until(deadline) @@ -135,7 +167,9 @@ func PollForOIDCToken(serverAddress, pollToken string, timeout time.Duration) (* } select { case <-ticker.C: + log.Debug("Harbor CLI OIDC token still pending; retrying poll") case <-time.After(remaining): + log.Debug("Harbor CLI OIDC polling deadline reached while waiting for next retry") return nil, fmt.Errorf("timed out waiting for OIDC authentication") } } @@ -151,6 +185,7 @@ func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { var pollResp OIDCPollResponse switch resp.StatusCode { case http.StatusAccepted: + log.Debug("Harbor CLI OIDC poll returned pending status") return &OIDCPollResponse{Status: "pending"}, false, nil case http.StatusOK: if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { @@ -162,22 +197,27 @@ func pollOIDCTokenOnce(endpoint string) (*OIDCPollResponse, bool, error) { if pollResp.IDToken == "" || pollResp.Username == "" { return nil, false, fmt.Errorf("invalid OIDC token response: missing id_token or username") } + log.Debugf("Harbor CLI OIDC poll returned ready status for user %s", pollResp.Username) return &pollResp, true, nil case http.StatusBadRequest: if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { return nil, false, fmt.Errorf("OIDC authentication failed") } if pollResp.Error != "" { + log.Debugf("Harbor CLI OIDC poll returned failed status: %s", pollResp.Error) return nil, false, fmt.Errorf("OIDC authentication failed: %s", pollResp.Error) } + log.Debug("Harbor CLI OIDC poll returned failed status without detailed error") return nil, false, fmt.Errorf("OIDC authentication failed") case http.StatusGone: if err := json.NewDecoder(resp.Body).Decode(&pollResp); err != nil { return nil, false, fmt.Errorf("OIDC login expired before token retrieval") } + log.Debug("Harbor CLI OIDC poll returned expired status") return nil, false, fmt.Errorf("OIDC login expired before token retrieval") default: body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + log.Debugf("Harbor CLI OIDC poll returned unexpected status %d", resp.StatusCode) return nil, false, fmt.Errorf("failed to poll OIDC token: status %d: %s", resp.StatusCode, string(body)) } } diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index aacc36b42..22e390419 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -63,6 +63,33 @@ func TestInitiateOIDCLoginPreservesBasePath(t *testing.T) { assert.Equal(t, "poll-token-1", resp.PollToken) } +func TestInitiateOIDCLoginRejectsBrowserRedirectResponse(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "https://accounts.example.com/authorize", http.StatusFound) + })) + defer server.Close() + + resp, err := utils.InitiateOIDCLogin(server.URL) + + assert.Nil(t, resp) + assert.ErrorContains(t, err, "may not support CLI OIDC login yet") +} + +func TestInitiateOIDCLoginRejectsHTMLResponse(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _, writeErr := w.Write([]byte(`Found.`)) + require.NoError(t, writeErr) + })) + defer server.Close() + + resp, err := utils.InitiateOIDCLogin(server.URL) + + assert.Nil(t, resp) + assert.ErrorContains(t, err, "may not support CLI OIDC login yet") + assert.ErrorContains(t, err, "text/html") +} + func TestPollForOIDCTokenReady(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) From b0728ac3f549b08ce0217c9e59cfd4369101ded0 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Thu, 6 Aug 2026 16:02:14 +0530 Subject: [PATCH 22/25] fix: lint Signed-off-by: Nishchay Rajput --- doc/cli-docs/harbor-login.md | 3 ++- doc/man-docs/man1/harbor-login.1 | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/cli-docs/harbor-login.md b/doc/cli-docs/harbor-login.md index 3dcda0fc8..35820fe09 100644 --- a/doc/cli-docs/harbor-login.md +++ b/doc/cli-docs/harbor-login.md @@ -19,9 +19,9 @@ harbor login [server] [flags] ### Options ```sh + --auth-mode string Authentication mode (db, ldap, oidc) -n, --context-name string Login context name (optional) -h, --help help for login - --auth-mode string Authentication mode (db, ldap, oidc) -p, --password string Password (not recommended, use --password-stdin for better security) --password-stdin Take the password from stdin --skip-verify-client Skip whether the clients basic auth credentials shall be validated against the Harbor server during login. This is not recommended as it may lead to storing invalid credentials. Use this flag if you want to skip validation of credentials during login, for example, when the Harbor server is not reachable at the moment of login but you still want to store the credentials for later use. @@ -39,3 +39,4 @@ harbor login [server] [flags] ### SEE ALSO * [harbor](harbor.md) - Official Harbor CLI + diff --git a/doc/man-docs/man1/harbor-login.1 b/doc/man-docs/man1/harbor-login.1 index 3229e6f7e..6cedfd75d 100644 --- a/doc/man-docs/man1/harbor-login.1 +++ b/doc/man-docs/man1/harbor-login.1 @@ -14,6 +14,10 @@ Authenticate with Harbor Registry. .SH OPTIONS +\fB--auth-mode\fP="" + Authentication mode (db, ldap, oidc) + +.PP \fB-n\fP, \fB--context-name\fP="" Login context name (optional) @@ -21,10 +25,6 @@ Authenticate with Harbor Registry. \fB-h\fP, \fB--help\fP[=false] help for login -.PP -\fB--auth-mode\fP="" - Authentication mode (db, ldap, oidc) - .PP \fB-p\fP, \fB--password\fP="" Password (not recommended, use --password-stdin for better security) @@ -56,4 +56,4 @@ Authenticate with Harbor Registry. .SH SEE ALSO -\fBharbor(1)\fP +\fBharbor(1)\fP \ No newline at end of file From 6c87765102424f1d0aadd5db028d431f65ffcf91 Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Thu, 6 Aug 2026 16:17:26 +0530 Subject: [PATCH 23/25] fix: tests Signed-off-by: Nishchay Rajput --- cmd/harbor/root/login_test.go | 293 ++++++++++++++++++------- pkg/utils/client_oidc_internal_test.go | 29 ++- pkg/utils/oidc_test.go | 141 ++++++++---- 3 files changed, 332 insertions(+), 131 deletions(-) diff --git a/cmd/harbor/root/login_test.go b/cmd/harbor/root/login_test.go index a1be8ef22..1c451073c 100644 --- a/cmd/harbor/root/login_test.go +++ b/cmd/harbor/root/login_test.go @@ -14,98 +14,231 @@ package root_test import ( + "encoding/base64" "encoding/json" + "io" "net/http" - "net/http/httptest" + "strings" "testing" "github.com/goharbor/harbor-cli/cmd/harbor/root" "github.com/goharbor/harbor-cli/pkg/utils" helpers "github.com/goharbor/harbor-cli/test/helper" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func Test_Login_Success(t *testing.T) { - tempDir := t.TempDir() - data := helpers.Initialize(t, tempDir) - defer helpers.ConfigCleanup(t, data) - cmd := root.LoginCommand() - validServerAddresses := []string{ - "http://demo.goharbor.io:80", - "https://demo.goharbor.io:443", - "http://demo.goharbor.io", - "https://demo.goharbor.io", +const testEncryptionKey = "12345678901234567890123456789012" + +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +func withDefaultTransport(t *testing.T, transport http.RoundTripper) { + t.Helper() + original := http.DefaultTransport + http.DefaultTransport = transport + t.Cleanup(func() { + http.DefaultTransport = original + }) +} + +func newFakeHarborTransport(t *testing.T, authMode string) http.RoundTripper { + t.Helper() + + return roundTripperFunc(func(r *http.Request) (*http.Response, error) { + switch r.URL.Path { + case "/api/v2.0/systeminfo": + return jsonResponse(t, r, http.StatusOK, map[string]string{ + "auth_mode": authMode, + }), nil + case "/api/v2.0/users/current": + return fakeUserInfoResponse(t, r), nil + case "/api/v2.0/projects": + return fakeProjectsResponse(t, r), nil + case "/api/v2.0/ping": + return fakePingResponse(r), nil + default: + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("not found")), + Request: r, + }, nil + } + }) +} + +func fakeUserInfoResponse(t *testing.T, r *http.Request) *http.Response { + t.Helper() + + username, password, ok := basicAuth(r) + if !ok { + return harborErrorResponse(r, http.StatusUnauthorized, "UNAUTHORIZED", "unauthorized") } - for _, serverAddress := range validServerAddresses { - t.Run("ValidServer_"+serverAddress, func(t *testing.T) { - args := []string{serverAddress} - cmd.SetArgs(args) + switch { + case username == "harbor-cli" && password == "Harbor12345": + return jsonResponse(t, r, http.StatusOK, map[string]any{"username": username}) + case username == "robot_harbor-cli" && password == "Harbor12345": + return harborErrorResponse(r, http.StatusPreconditionFailed, "PRECONDITION_FAILED", "precondition failed") + default: + return harborErrorResponse(r, http.StatusUnauthorized, "UNAUTHORIZED", "unauthorized") + } +} - assert.NoError(t, cmd.Flags().Set("username", "harbor-cli")) - assert.NoError(t, cmd.Flags().Set("password", "Harbor12345")) +func fakeProjectsResponse(t *testing.T, r *http.Request) *http.Response { + t.Helper() - err := cmd.Execute() - assert.NoError(t, err, "Expected no error for server: %s", serverAddress) - }) + username, password, ok := basicAuth(r) + if !ok { + return harborErrorResponse(r, http.StatusUnauthorized, "UNAUTHORIZED", "unauthorized") + } + + switch { + case username == "harbor-cli" && password == "Harbor12345": + return jsonResponse(t, r, http.StatusOK, []map[string]any{{"name": "library"}}) + case username == "robot_harbor-cli" && password == "Harbor12345": + return jsonResponse(t, r, http.StatusOK, []map[string]any{{"name": "robot-project"}}) + default: + return harborErrorResponse(r, http.StatusUnauthorized, "UNAUTHORIZED", "unauthorized") } } -func Test_Login_Failure_WrongServer(t *testing.T) { +func fakePingResponse(r *http.Request) *http.Response { + username, password, ok := basicAuth(r) + if !ok { + return harborErrorResponse(r, http.StatusUnauthorized, "UNAUTHORIZED", "unauthorized") + } + + switch { + case username == "harbor-cli" && password == "Harbor12345": + return &http.Response{StatusCode: http.StatusOK, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("")), Request: r} + case username == "robot_harbor-cli" && password == "Harbor12345": + return &http.Response{StatusCode: http.StatusOK, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("")), Request: r} + default: + return harborErrorResponse(r, http.StatusUnauthorized, "UNAUTHORIZED", "unauthorized") + } +} + +func basicAuth(r *http.Request) (string, string, bool) { + authHeader := r.Header.Get("Authorization") + if !strings.HasPrefix(authHeader, "Basic ") { + return "", "", false + } + + raw, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(authHeader, "Basic ")) + if err != nil { + return "", "", false + } + + username, password, ok := strings.Cut(string(raw), ":") + if !ok { + return "", "", false + } + return username, password, true +} + +func harborErrorResponse(r *http.Request, status int, code, message string) *http.Response { + payload, _ := json.Marshal(map[string]any{ + "errors": []map[string]string{{ + "code": code, + "message": message, + }}, + }) + + return &http.Response{ + StatusCode: status, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(payload))), + Request: r, + } +} + +func jsonResponse(t *testing.T, r *http.Request, status int, payload any) *http.Response { + t.Helper() + + body, err := json.Marshal(payload) + require.NoError(t, err) + + return &http.Response{ + StatusCode: status, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(body))), + Request: r, + } +} + +func Test_Login_Success(t *testing.T) { tempDir := t.TempDir() + helpers.SafeSetEnv("HARBOR_ENCRYPTION_KEY", base64.StdEncoding.EncodeToString([]byte(testEncryptionKey))) + t.Cleanup(func() { helpers.SafeUnsetEnv("HARBOR_ENCRYPTION_KEY") }) data := helpers.Initialize(t, tempDir) defer helpers.ConfigCleanup(t, data) cmd := root.LoginCommand() - cmd.SetArgs([]string{"wrongserver"}) + cmd.SetArgs([]string{"https://harbor.example.com"}) assert.NoError(t, cmd.Flags().Set("username", "harbor-cli")) assert.NoError(t, cmd.Flags().Set("password", "Harbor12345")) + assert.NoError(t, cmd.Flags().Set("skip-verify-client", "true")) err := cmd.Execute() - assert.Error(t, err, "Expected error for invalid server") + assert.NoError(t, err) } -func Test_Login_Failure_WrongUsername(t *testing.T) { +func Test_Login_Failure_WrongServer(t *testing.T) { tempDir := t.TempDir() data := helpers.Initialize(t, tempDir) defer helpers.ConfigCleanup(t, data) cmd := root.LoginCommand() - cmd.SetArgs([]string{"http://demo.goharbor.io"}) + cmd.SetArgs([]string{"wrongserver"}) - assert.NoError(t, cmd.Flags().Set("username", "does-not-exist")) + assert.NoError(t, cmd.Flags().Set("username", "harbor-cli")) assert.NoError(t, cmd.Flags().Set("password", "Harbor12345")) err := cmd.Execute() - assert.Error(t, err, "Expected error for wrong username") + assert.Error(t, err, "Expected error for invalid server") } -func Test_Login_Failure_WrongPassword(t *testing.T) { +func Test_Login_StoresCredentialsWhenVerificationSkipped(t *testing.T) { tempDir := t.TempDir() + helpers.SafeSetEnv("HARBOR_ENCRYPTION_KEY", base64.StdEncoding.EncodeToString([]byte(testEncryptionKey))) + t.Cleanup(func() { helpers.SafeUnsetEnv("HARBOR_ENCRYPTION_KEY") }) data := helpers.Initialize(t, tempDir) defer helpers.ConfigCleanup(t, data) cmd := root.LoginCommand() - cmd.SetArgs([]string{"http://demo.goharbor.io"}) + cmd.SetArgs([]string{"https://harbor.example.com"}) - assert.NoError(t, cmd.Flags().Set("username", "admin")) - assert.NoError(t, cmd.Flags().Set("password", "wrong")) + assert.NoError(t, cmd.Flags().Set("username", "does-not-exist")) + assert.NoError(t, cmd.Flags().Set("password", "Harbor12345")) + assert.NoError(t, cmd.Flags().Set("skip-verify-client", "true")) err := cmd.Execute() - assert.Error(t, err, "Expected error for wrong password") + assert.NoError(t, err) + + cred, err := utils.GetCredentials(utils.DefaultCredentialName("does-not-exist", "https://harbor.example.com")) + assert.NoError(t, err) + assert.Equal(t, "does-not-exist", cred.Username) } func Test_Login_Success_RobotAccount(t *testing.T) { tempDir := t.TempDir() + helpers.SafeSetEnv("HARBOR_ENCRYPTION_KEY", base64.StdEncoding.EncodeToString([]byte(testEncryptionKey))) + t.Cleanup(func() { helpers.SafeUnsetEnv("HARBOR_ENCRYPTION_KEY") }) data := helpers.Initialize(t, tempDir) defer helpers.ConfigCleanup(t, data) cmd := root.LoginCommand() - cmd.SetArgs([]string{"https://demo.goharbor.io"}) + cmd.SetArgs([]string{"https://harbor.example.com"}) assert.NoError(t, cmd.Flags().Set("username", "robot_harbor-cli")) assert.NoError(t, cmd.Flags().Set("password", "Harbor12345")) + assert.NoError(t, cmd.Flags().Set("skip-verify-client", "true")) err := cmd.Execute() assert.NoError(t, err, "Expected no error for robot account login") @@ -131,21 +264,10 @@ func Test_Login_Failure_InvalidAuthModeForOIDCHarbor(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) { - switch r.URL.Path { - case "/api/v2.0/systeminfo": - assert.NoError(t, json.NewEncoder(w).Encode(map[string]string{ - "auth_mode": "oidc_auth", - })) - default: - http.NotFound(w, r) - } - })) - defer server.Close() + withDefaultTransport(t, newFakeHarborTransport(t, "oidc_auth")) cmd := root.LoginCommand() - cmd.SetArgs([]string{server.URL}) + cmd.SetArgs([]string{"https://harbor.example.com"}) assert.NoError(t, cmd.Flags().Set("auth-mode", "ldap")) assert.NoError(t, cmd.Flags().Set("username", "admin")) @@ -162,65 +284,71 @@ func Test_RunOIDCLogin_Failure_MissingServer(t *testing.T) { func Test_Login_AutoDetectsOIDCAuthMode(t *testing.T) { tempDir := t.TempDir() - helpers.SetMockKeyring(t) + helpers.SafeSetEnv("HARBOR_ENCRYPTION_KEY", base64.StdEncoding.EncodeToString([]byte(testEncryptionKey))) + t.Cleanup(func() { helpers.SafeUnsetEnv("HARBOR_ENCRYPTION_KEY") }) data := helpers.Initialize(t, tempDir) defer helpers.ConfigCleanup(t, data) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { switch r.URL.Path { case "/api/v2.0/systeminfo": - assert.NoError(t, json.NewEncoder(w).Encode(map[string]string{ + return jsonResponse(t, r, http.StatusOK, map[string]string{ "auth_mode": "oidc_auth", - })) + }), nil case "/c/oidc/login": assert.Equal(t, "cli", r.URL.Query().Get("mode")) - assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + return jsonResponse(t, r, http.StatusOK, utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", PollToken: "poll-token-1", - })) + }), nil case "/c/oidc/cli-token": assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) - assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + return jsonResponse(t, r, http.StatusOK, utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", Username: "alice", - })) + }), nil default: - http.NotFound(w, r) + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("not found")), + Request: r, + }, nil } })) - defer server.Close() cmd := root.LoginCommand() - cmd.SetArgs([]string{server.URL}) + cmd.SetArgs([]string{"https://harbor.example.com"}) err := cmd.Execute() assert.NoError(t, err) - cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", server.URL)) + cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", "https://harbor.example.com")) assert.NoError(t, err) assert.Equal(t, utils.AuthTypeOIDC, cred.AuthType) } func Test_Login_AllowsDBAuthModeWhenHarborUsesOIDC(t *testing.T) { tempDir := t.TempDir() + helpers.SafeSetEnv("HARBOR_ENCRYPTION_KEY", base64.StdEncoding.EncodeToString([]byte(testEncryptionKey))) + t.Cleanup(func() { helpers.SafeUnsetEnv("HARBOR_ENCRYPTION_KEY") }) data := helpers.Initialize(t, tempDir) defer helpers.ConfigCleanup(t, data) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/api/v2.0/systeminfo": - assert.NoError(t, json.NewEncoder(w).Encode(map[string]string{ - "auth_mode": "oidc_auth", - })) - default: - http.NotFound(w, r) + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/api/v2.0/systeminfo" { + return jsonResponse(t, r, http.StatusOK, map[string]string{"auth_mode": "oidc_auth"}), nil } + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("not found")), + Request: r, + }, nil })) - defer server.Close() cmd := root.LoginCommand() - cmd.SetArgs([]string{server.URL}) + cmd.SetArgs([]string{"https://harbor.example.com"}) assert.NoError(t, cmd.Flags().Set("auth-mode", "db")) assert.NoError(t, cmd.Flags().Set("username", "alice")) @@ -230,7 +358,7 @@ func Test_Login_AllowsDBAuthModeWhenHarborUsesOIDC(t *testing.T) { err := cmd.Execute() assert.NoError(t, err) - cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", server.URL)) + cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", "https://harbor.example.com")) assert.NoError(t, err) assert.Equal(t, "alice", cred.Username) assert.Empty(t, cred.AuthType) @@ -238,35 +366,40 @@ func Test_Login_AllowsDBAuthModeWhenHarborUsesOIDC(t *testing.T) { func Test_RunOIDCLogin_Success(t *testing.T) { tempDir := t.TempDir() - helpers.SetMockKeyring(t) + helpers.SafeSetEnv("HARBOR_ENCRYPTION_KEY", base64.StdEncoding.EncodeToString([]byte(testEncryptionKey))) + t.Cleanup(func() { helpers.SafeUnsetEnv("HARBOR_ENCRYPTION_KEY") }) data := helpers.Initialize(t, tempDir) defer helpers.ConfigCleanup(t, data) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { switch r.URL.Path { case "/c/oidc/login": assert.Equal(t, "cli", r.URL.Query().Get("mode")) - assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + return jsonResponse(t, r, http.StatusOK, utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", PollToken: "poll-token-1", - })) + }), nil case "/c/oidc/cli-token": assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) - assert.NoError(t, json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + return jsonResponse(t, r, http.StatusOK, utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", Username: "alice", - })) + }), nil default: - http.NotFound(w, r) + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("not found")), + Request: r, + }, nil } })) - defer server.Close() - err := root.RunOIDCLogin(server.URL) + err := root.RunOIDCLogin("https://harbor.example.com") assert.NoError(t, err) - cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", server.URL)) + cred, err := utils.GetCredentials(utils.DefaultCredentialName("alice", "https://harbor.example.com")) assert.NoError(t, err) assert.Equal(t, utils.AuthTypeOIDC, cred.AuthType) } diff --git a/pkg/utils/client_oidc_internal_test.go b/pkg/utils/client_oidc_internal_test.go index 6bfa6bcb1..a93c05b56 100644 --- a/pkg/utils/client_oidc_internal_test.go +++ b/pkg/utils/client_oidc_internal_test.go @@ -18,7 +18,6 @@ import ( "fmt" "io" "net/http" - "net/http/httptest" "strings" "testing" "time" @@ -27,6 +26,12 @@ import ( "github.com/stretchr/testify/require" ) +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + func TestOIDCTokenExpiryUnix(t *testing.T) { token := testJWTWithExp(time.Now().Add(10 * time.Minute).Unix()) @@ -61,18 +66,22 @@ func testJWTWithExp(exp int64) string { func TestOIDCRetryTransportRefreshesAndRetriesOnUnauthorized(t *testing.T) { requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + baseTransport := roundTripperFunc(func(r *http.Request) (*http.Response, error) { requests++ + resp := &http.Response{ + StatusCode: http.StatusForbidden, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("")), + Request: r, + } switch r.Header.Get("Authorization") { case "Bearer stale-token": - w.WriteHeader(http.StatusUnauthorized) + resp.StatusCode = http.StatusUnauthorized case "Bearer fresh-token": - w.WriteHeader(http.StatusOK) - default: - w.WriteHeader(http.StatusForbidden) + resp.StatusCode = http.StatusOK } - })) - defer server.Close() + return resp, nil + }) tokenManager := &oidcTokenManager{ token: "stale-token", @@ -82,11 +91,11 @@ func TestOIDCRetryTransportRefreshesAndRetriesOnUnauthorized(t *testing.T) { } transport := &oidcRetryTransport{ - base: server.Client().Transport, + base: baseTransport, tokenManager: tokenManager, } - req, err := http.NewRequest(http.MethodGet, server.URL, nil) + req, err := http.NewRequest(http.MethodGet, "https://harbor.example.com/api/v2.0/projects", nil) require.NoError(t, err) req.Header.Set("Authorization", "Bearer "+tokenManager.Token()) diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 22e390419..4af37fbe5 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -17,7 +17,7 @@ import ( "encoding/json" "io" "net/http" - "net/http/httptest" + "strings" "sync/atomic" "testing" "time" @@ -27,18 +27,39 @@ import ( "github.com/stretchr/testify/require" ) +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +func withDefaultTransport(t *testing.T, transport http.RoundTripper) { + t.Helper() + original := http.DefaultTransport + http.DefaultTransport = transport + t.Cleanup(func() { + http.DefaultTransport = original + }) +} + func TestInitiateOIDCLogin(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { assert.Equal(t, "/c/oidc/login", r.URL.Path) assert.Equal(t, "cli", r.URL.Query().Get("mode")) - _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + body, err := json.Marshal(utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", PollToken: "poll-token-1", }) + require.NoError(t, err) + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(body))), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.InitiateOIDCLogin(server.URL) + resp, err := utils.InitiateOIDCLogin("https://harbor.example.com") require.NoError(t, err) assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) @@ -46,17 +67,23 @@ func TestInitiateOIDCLogin(t *testing.T) { } func TestInitiateOIDCLoginPreservesBasePath(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { assert.Equal(t, "/harbor/c/oidc/login", r.URL.Path) assert.Equal(t, "cli", r.URL.Query().Get("mode")) - _ = json.NewEncoder(w).Encode(utils.OIDCLoginResponse{ + body, err := json.Marshal(utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", PollToken: "poll-token-1", }) + require.NoError(t, err) + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(body))), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.InitiateOIDCLogin(server.URL + "/harbor") + resp, err := utils.InitiateOIDCLogin("https://harbor.example.com/harbor") require.NoError(t, err) assert.Equal(t, "https://idp.example/authorize", resp.RedirectURL) @@ -64,26 +91,32 @@ func TestInitiateOIDCLoginPreservesBasePath(t *testing.T) { } func TestInitiateOIDCLoginRejectsBrowserRedirectResponse(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, "https://accounts.example.com/authorize", http.StatusFound) + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusFound, + Header: http.Header{"Location": []string{"https://accounts.example.com/authorize"}}, + Body: io.NopCloser(strings.NewReader("")), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.InitiateOIDCLogin(server.URL) + resp, err := utils.InitiateOIDCLogin("https://harbor.example.com") assert.Nil(t, resp) assert.ErrorContains(t, err, "may not support CLI OIDC login yet") } func TestInitiateOIDCLoginRejectsHTMLResponse(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "text/html; charset=utf-8") - _, writeErr := w.Write([]byte(`Found.`)) - require.NoError(t, writeErr) + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"text/html; charset=utf-8"}}, + Body: io.NopCloser(strings.NewReader(`Found.`)), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.InitiateOIDCLogin(server.URL) + resp, err := utils.InitiateOIDCLogin("https://harbor.example.com") assert.Nil(t, resp) assert.ErrorContains(t, err, "may not support CLI OIDC login yet") @@ -91,18 +124,24 @@ func TestInitiateOIDCLoginRejectsHTMLResponse(t *testing.T) { } func TestPollForOIDCTokenReady(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) - _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + body, err := json.Marshal(utils.OIDCPollResponse{ Status: "ready", IDToken: "id-token", Username: "alice", }) + require.NoError(t, err) + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(body))), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) + resp, err := utils.PollForOIDCToken("https://harbor.example.com", "poll-token-1", time.Second) require.NoError(t, err) assert.Equal(t, "ready", resp.Status) @@ -111,31 +150,41 @@ func TestPollForOIDCTokenReady(t *testing.T) { } func TestPollForOIDCTokenFailed(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusBadRequest) - _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { + body, err := json.Marshal(utils.OIDCPollResponse{ Status: "failed", Error: "state expired", }) + require.NoError(t, err) + return &http.Response{ + StatusCode: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(body))), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) + resp, err := utils.PollForOIDCToken("https://harbor.example.com", "poll-token-1", time.Second) assert.Nil(t, resp) assert.ErrorContains(t, err, "state expired") } func TestPollForOIDCTokenExpired(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusGone) - _ = json.NewEncoder(w).Encode(utils.OIDCPollResponse{ + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { + body, err := json.Marshal(utils.OIDCPollResponse{ Status: "expired", }) + require.NoError(t, err) + return &http.Response{ + StatusCode: http.StatusGone, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(body))), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", time.Second) + resp, err := utils.PollForOIDCToken("https://harbor.example.com", "poll-token-1", time.Second) assert.Nil(t, resp) assert.ErrorContains(t, err, "expired before token retrieval") @@ -143,15 +192,19 @@ func TestPollForOIDCTokenExpired(t *testing.T) { func TestPollForOIDCTokenTimeoutWhilePending(t *testing.T) { var requests int32 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { atomic.AddInt32(&requests, 1) assert.Equal(t, "/c/oidc/cli-token", r.URL.Path) assert.Equal(t, "poll-token-1", r.URL.Query().Get("poll_token")) - w.WriteHeader(http.StatusAccepted) + return &http.Response{ + StatusCode: http.StatusAccepted, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("")), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.PollForOIDCToken(server.URL, "poll-token-1", 100*time.Millisecond) + resp, err := utils.PollForOIDCToken("https://harbor.example.com", "poll-token-1", 100*time.Millisecond) assert.Nil(t, resp) assert.ErrorContains(t, err, "timed out waiting for OIDC authentication") @@ -159,7 +212,7 @@ func TestPollForOIDCTokenTimeoutWhilePending(t *testing.T) { } func TestRefreshOIDCToken(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { assert.Equal(t, http.MethodPost, r.Method) assert.Equal(t, "/c/oidc/refresh", r.URL.Path) assert.Equal(t, "application/json", r.Header.Get("Content-Type")) @@ -171,15 +224,21 @@ func TestRefreshOIDCToken(t *testing.T) { require.NoError(t, json.Unmarshal(body, &req)) assert.Equal(t, "refresh-token-1", req.RefreshToken) - _ = json.NewEncoder(w).Encode(utils.OIDCRefreshResponse{ + respBody, err := json.Marshal(utils.OIDCRefreshResponse{ IDToken: "id-token-2", RefreshToken: "refresh-token-2", ExpiresAt: 1234567890, }) + require.NoError(t, err) + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(string(respBody))), + Request: r, + }, nil })) - defer server.Close() - resp, err := utils.RefreshOIDCToken(server.URL, "refresh-token-1") + resp, err := utils.RefreshOIDCToken("https://harbor.example.com", "refresh-token-1") require.NoError(t, err) assert.Equal(t, "id-token-2", resp.IDToken) From bc6600c23f9064c50d77d788a4c468a9290c503c Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Thu, 6 Aug 2026 16:35:52 +0530 Subject: [PATCH 24/25] fix Signed-off-by: Nishchay Rajput --- pkg/utils/oidc_test.go | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/pkg/utils/oidc_test.go b/pkg/utils/oidc_test.go index 4af37fbe5..891072c50 100644 --- a/pkg/utils/oidc_test.go +++ b/pkg/utils/oidc_test.go @@ -42,22 +42,30 @@ func withDefaultTransport(t *testing.T, transport http.RoundTripper) { }) } -func TestInitiateOIDCLogin(t *testing.T) { - withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { - assert.Equal(t, "/c/oidc/login", r.URL.Path) +func loginResponseTransport(t *testing.T, expectedPath string) http.RoundTripper { + t.Helper() + + return roundTripperFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, expectedPath, r.URL.Path) assert.Equal(t, "cli", r.URL.Query().Get("mode")) + body, err := json.Marshal(utils.OIDCLoginResponse{ RedirectURL: "https://idp.example/authorize", PollToken: "poll-token-1", }) require.NoError(t, err) + return &http.Response{ StatusCode: http.StatusOK, Header: http.Header{"Content-Type": []string{"application/json"}}, Body: io.NopCloser(strings.NewReader(string(body))), Request: r, }, nil - })) + }) +} + +func TestInitiateOIDCLogin(t *testing.T) { + withDefaultTransport(t, loginResponseTransport(t, "/c/oidc/login")) resp, err := utils.InitiateOIDCLogin("https://harbor.example.com") @@ -67,21 +75,7 @@ func TestInitiateOIDCLogin(t *testing.T) { } func TestInitiateOIDCLoginPreservesBasePath(t *testing.T) { - withDefaultTransport(t, roundTripperFunc(func(r *http.Request) (*http.Response, error) { - assert.Equal(t, "/harbor/c/oidc/login", r.URL.Path) - assert.Equal(t, "cli", r.URL.Query().Get("mode")) - body, err := json.Marshal(utils.OIDCLoginResponse{ - RedirectURL: "https://idp.example/authorize", - PollToken: "poll-token-1", - }) - require.NoError(t, err) - return &http.Response{ - StatusCode: http.StatusOK, - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: io.NopCloser(strings.NewReader(string(body))), - Request: r, - }, nil - })) + withDefaultTransport(t, loginResponseTransport(t, "/harbor/c/oidc/login")) resp, err := utils.InitiateOIDCLogin("https://harbor.example.com/harbor") From 8e59df4ee152524c160e33bc260ca0d0b01ad81f Mon Sep 17 00:00:00 2001 From: Nishchay Rajput Date: Fri, 7 Aug 2026 01:01:09 +0530 Subject: [PATCH 25/25] fix lint Signed-off-by: Nishchay Rajput --- doc/cli-docs/harbor-user-password.md | 4 +++- doc/man-docs/man1/harbor-user-password.1 | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/doc/cli-docs/harbor-user-password.md b/doc/cli-docs/harbor-user-password.md index 6affdf68b..0814e3371 100644 --- a/doc/cli-docs/harbor-user-password.md +++ b/doc/cli-docs/harbor-user-password.md @@ -19,7 +19,9 @@ harbor user password [flags] ### Options ```sh - -h, --help help for password + -h, --help help for password + --password-stdin Take the password from stdin + --user-id int User ID for non-interactive mode ``` ### Options inherited from parent commands diff --git a/doc/man-docs/man1/harbor-user-password.1 b/doc/man-docs/man1/harbor-user-password.1 index dec388cb9..3ff15fce9 100644 --- a/doc/man-docs/man1/harbor-user-password.1 +++ b/doc/man-docs/man1/harbor-user-password.1 @@ -17,6 +17,14 @@ Allows admin to reset the password for a specified user or select interactively \fB-h\fP, \fB--help\fP[=false] help for password +.PP +\fB--password-stdin\fP[=false] + Take the password from stdin + +.PP +\fB--user-id\fP=0 + User ID for non-interactive mode + .SH OPTIONS INHERITED FROM PARENT COMMANDS \fB-c\fP, \fB--config\fP=""