Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/caib/authcmd/authcmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var (
func NewAuthCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "auth",
Short: "Manage authentication tokens",
Short: "Authentication and token management commands",
Long: `Commands for inspecting and refreshing OIDC authentication tokens.`,
}

Expand Down Expand Up @@ -129,7 +129,7 @@ func runRefresh(cmd *cobra.Command, _ []string) {
serverURL := config.DefaultServerWithDerive()
if serverURL == "" {
fmt.Println(color.RedString("No server configured."))
fmt.Println("Run 'caib login <server-url>' or 'jmp login <endpoint>' first.")
fmt.Println("Run 'caib login <server-url>' first.")
return
}

Expand Down
5 changes: 5 additions & 0 deletions cmd/caib/authcmd/authcmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,9 @@ var _ = Describe("NewAuthCmd", func() {
}
Fail("status subcommand not found")
})

It("should use description aligned with jmp auth", func() {
cmd := NewAuthCmd()
Expect(cmd.Short).To(Equal("Authentication and token management commands"))
})
})
84 changes: 70 additions & 14 deletions cmd/caib/common/api_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/centos-automotive-suite/automotive-dev-operator/cmd/caib/auth"
"github.com/centos-automotive-suite/automotive-dev-operator/cmd/caib/config"
buildapiclient "github.com/centos-automotive-suite/automotive-dev-operator/internal/buildapi/client"
"k8s.io/client-go/tools/clientcmd"
)
Expand All @@ -20,19 +21,39 @@ func CreateBuildAPIClient(serverURL string, authToken *string, insecureSkipTLS b

tokenValue := ""
if authToken != nil {
tokenValue = strings.TrimSpace(*authToken)
tokenValue = sanitizeToken(*authToken)
}
setToken := func(token string) {
tokenValue = strings.TrimSpace(token)
tokenValue = sanitizeToken(token)
if authToken != nil {
*authToken = tokenValue
}
}

envToken := strings.TrimSpace(os.Getenv("CAIB_TOKEN"))
explicitToken := tokenValue != "" || envToken != ""
// Token resolution order:
// 1. --token flag (tokenValue already set by caller)
// 2. saved_token from cli.json (set by caib login --token)
// 3. CAIB_TOKEN env var
// 4. OIDC cached / browser flow
// 5. kubeconfig / oc whoami -t fallback
envToken := sanitizeToken(os.Getenv("CAIB_TOKEN"))
// Many commands bind --token with a default value from CAIB_TOKEN.
// If the pointer value exactly matches the env var, treat it as implicit
// and allow saved_token to take precedence for "subsequent commands"
// after `caib login --token`.
if tokenValue != "" && envToken != "" && tokenValue == envToken {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This heuristic is clever but fragile: if a user explicitly passes --token X where X happens to equal CAIB_TOKEN, the saved token silently wins over their explicit flag value.

A more robust approach would be to thread a tokenExplicit bool parameter (or use cmd.Flags().Changed("token") at the call site) so CreateBuildAPIClient can distinguish "user typed it" from "cobra set the default." That said, the current approach works for the common case — just worth a // CAVEAT: note for future maintainers.

tokenValue = ""
}
if tokenValue == "" {
if saved := config.LoadSavedToken(); saved != "" {
setToken(saved)
}
}
if tokenValue == "" && envToken != "" {
setToken(envToken)
}

if !explicitToken {
if tokenValue == "" {
token, didAuth, err := auth.GetTokenWithReauth(ctx, serverURL, "", insecureSkipTLS)
if err != nil {
fmt.Fprintf(os.Stderr, "Error: OIDC authentication failed: %v\n", err)
Expand All @@ -50,12 +71,6 @@ func CreateBuildAPIClient(serverURL string, authToken *string, insecureSkipTLS b
} else if tok, loadErr := LoadTokenFromKubeconfig(); loadErr == nil && strings.TrimSpace(tok) != "" {
setToken(tok)
}
} else if tokenValue == "" {
if envToken != "" {
setToken(envToken)
} else if tok, loadErr := LoadTokenFromKubeconfig(); loadErr == nil && strings.TrimSpace(tok) != "" {
setToken(tok)
}
}

var opts []buildapiclient.Option
Expand Down Expand Up @@ -85,22 +100,28 @@ func ExecuteWithReauth(
ctx := context.Background()
currentToken := ""
if authToken != nil {
currentToken = strings.TrimSpace(*authToken)
currentToken = sanitizeToken(*authToken)
}
setToken := func(token string) {
currentToken = strings.TrimSpace(token)
currentToken = sanitizeToken(token)
if authToken != nil {
*authToken = currentToken
}
}

// Determine whether the token is explicitly user-provided before the first
// call resolves it. CreateBuildAPIClient can fill authToken from kubeconfig,
// so we must capture this upfront rather than checking currentToken after.
savedToken := config.LoadSavedToken()
isExplicitToken := currentToken != "" || savedToken != ""

runWithFreshClient := func() error {
client, err := CreateBuildAPIClient(serverURL, authToken, insecureSkipTLS)
if err != nil {
return err
}
if authToken != nil {
currentToken = strings.TrimSpace(*authToken)
currentToken = sanitizeToken(*authToken)
}
return fn(client)
}
Expand All @@ -113,6 +134,26 @@ func ExecuteWithReauth(
return err
}

// If the token was explicitly provided by the user (via caib login --token or
// the --token flag) and the server rejected it, give a clear actionable error
// rather than silently falling through to OIDC with a different identity.
if isExplicitToken {
if savedToken != "" {
return fmt.Errorf(
"saved token was rejected by the server (expired or invalid)\n"+
"Refresh it with:\n"+
" oc login # re-authenticate with your cluster\n"+
" caib login --token $(oc whoami -t) %s", serverURL,
)
}
return fmt.Errorf(
"provided token was rejected by the server (401)\n"+
"Verify the token is valid, or re-authenticate:\n"+
" oc login # re-authenticate with your cluster\n"+
" caib login --token $(oc whoami -t) %s", serverURL,
)
}

fmt.Fprintln(os.Stderr, "Authentication failed (401), re-authenticating...")
newToken, _, err := auth.GetTokenWithReauth(ctx, serverURL, currentToken, insecureSkipTLS)
if err != nil {
Expand Down Expand Up @@ -197,3 +238,18 @@ func LoadTokenFromKubeconfig() (string, error) {
}
return "", fmt.Errorf("no bearer token found in kubeconfig")
}

// sanitizeToken strips any "Bearer " prefix and surrounding whitespace from a
// token string. Mirrors config.normalizeToken (unexported) — kept here to avoid
// a circular import between the common and config packages.
func sanitizeToken(raw string) string {
trimmed := strings.TrimSpace(raw)
if trimmed == "" {
return ""
}
parts := strings.Fields(trimmed)
if len(parts) >= 2 && strings.EqualFold(parts[0], "bearer") {
Comment thread
bkhizgiy marked this conversation as resolved.
return strings.TrimSpace(strings.Join(parts[1:], " "))
}
return trimmed
}
141 changes: 141 additions & 0 deletions cmd/caib/common/api_client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package caibcommon

import (
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"

"github.com/centos-automotive-suite/automotive-dev-operator/cmd/caib/config"
buildapiclient "github.com/centos-automotive-suite/automotive-dev-operator/internal/buildapi/client"
)

// setupTempConfig redirects config reads/writes to a temp HOME directory.
// Returns a cleanup function.
func setupTempConfig(t *testing.T) func() {
t.Helper()
dir, err := os.MkdirTemp("", "caib-api-client-test-*")
if err != nil {
t.Fatalf("MkdirTemp: %v", err)
}
origHome := os.Getenv("HOME")
origXDG := os.Getenv("XDG_CONFIG_HOME")
_ = os.Setenv("HOME", dir)
_ = os.Unsetenv("XDG_CONFIG_HOME")
return func() {
_ = os.Setenv("HOME", origHome)
if origXDG != "" {
_ = os.Setenv("XDG_CONFIG_HOME", origXDG)
} else {
_ = os.Unsetenv("XDG_CONFIG_HOME")
}
_ = os.RemoveAll(dir)
}
}

// always401Handler is a handler that unconditionally returns 401.
var always401Handler = http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
})

// authErrorFn is an ExecuteWithReauth callback that always returns a 401 error.
func authErrorFn(_ *buildapiclient.Client) error {
return &fakeAuthError{}
}

// fakeAuthError satisfies the auth.IsAuthError check (its message contains "401").
type fakeAuthError struct{}

func (e *fakeAuthError) Error() string { return "401 Unauthorized" }

func TestExecuteWithReauth_SavedTokenRejected(t *testing.T) {
cleanup := setupTempConfig(t)
defer cleanup()

srv := httptest.NewServer(always401Handler)
defer srv.Close()

if err := config.SaveToken("sha256~fakesavedtoken"); err != nil {
t.Fatalf("SaveToken: %v", err)
}

// Empty --token flag (zero value) so CreateBuildAPIClient loads the saved token.
tok := ""
err := ExecuteWithReauth(srv.URL, &tok, false, authErrorFn)

if err == nil {
t.Fatal("expected error, got nil")
}
if !strings.Contains(err.Error(), "saved token was rejected") {
t.Errorf("expected 'saved token was rejected' in error, got: %v", err)
}
}

func TestExecuteWithReauth_ExplicitTokenRejected(t *testing.T) {
cleanup := setupTempConfig(t)
defer cleanup()

srv := httptest.NewServer(always401Handler)
defer srv.Close()

// No saved token — user passed an explicit --token flag value.
tok := "sha256~explicit-flag-token"
err := ExecuteWithReauth(srv.URL, &tok, false, authErrorFn)

if err == nil {
t.Fatal("expected error, got nil")
}
if !strings.Contains(err.Error(), "provided token was rejected") {
t.Errorf("expected 'provided token was rejected' in error, got: %v", err)
}
}

func TestExecuteWithReauth_NoTokenTriesOIDCFallback(t *testing.T) {
cleanup := setupTempConfig(t)
defer cleanup()

// Server returns 401 for API calls and 404 for OIDC config (no OIDC configured).
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "authconfig") {
w.WriteHeader(http.StatusNotFound)
return
}
w.WriteHeader(http.StatusUnauthorized)
}))
defer srv.Close()

// No saved token, no explicit token — should attempt OIDC (non-interactively
// since no OIDC config) and return some error rather than panicking or opening
// a browser.
tok := ""
err := ExecuteWithReauth(srv.URL, &tok, false, authErrorFn)
if err == nil {
t.Fatal("expected error when no token and no OIDC available, got nil")
}
}

func TestSanitizeToken(t *testing.T) {
cases := []struct {
input string
want string
}{
{"sha256~abc", "sha256~abc"},
{"Bearer sha256~abc", "sha256~abc"},
{"bearer sha256~abc", "sha256~abc"},
{"BEARER sha256~abc", "sha256~abc"},
{" Bearer sha256~abc ", "sha256~abc"},
{"eyJhbGciOiJSUzI1NiJ9.e.sig", "eyJhbGciOiJSUzI1NiJ9.e.sig"},
{"Bearer eyJhbGciOiJSUzI1NiJ9.e.sig", "eyJhbGciOiJSUzI1NiJ9.e.sig"},
{"", ""},
{" ", ""},
// Single word "Bearer" with no token — treated as an opaque token, not a prefix.
{"Bearer", "Bearer"},
}
for _, tc := range cases {
got := sanitizeToken(tc.input)
if got != tc.want {
t.Errorf("sanitizeToken(%q) = %q, want %q", tc.input, got, tc.want)
}
}
}
57 changes: 54 additions & 3 deletions cmd/caib/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ var healthHTTPClient *http.Client

// CLIConfig holds saved CLI settings.
type CLIConfig struct {
ServerURL string `json:"server_url"`
ServerURL string `json:"server_url"`
SavedToken string `json:"saved_token,omitempty"`
}

// DefaultServer returns the effective default server URL: CAIB_SERVER env, then saved config.
Expand Down Expand Up @@ -195,16 +196,66 @@ func Read() (*CLIConfig, error) {
return &cfg, nil
}

// SaveServerURL writes the given server URL to the local config file.
// SaveServerURL writes the given server URL to the local config file,
// preserving any other existing fields (e.g. SavedToken).
func SaveServerURL(serverURL string) error {
return updateConfig(func(cfg *CLIConfig) {
cfg.ServerURL = strings.TrimSpace(serverURL)
})
}

// SaveToken saves an explicit bearer token to the local config file.
// The token is used as the primary auth credential for all API calls,
// bypassing OIDC. Accepts both opaque tokens (e.g. oc whoami -t) and JWTs.
// Pass an empty string to clear the saved token.
func SaveToken(token string) error {
return updateConfig(func(cfg *CLIConfig) {
cfg.SavedToken = normalizeToken(token)
})
}

// LoadSavedToken returns the token saved via SaveToken, or "" if none is set.
func LoadSavedToken() string {
cfg, err := Read()
if err != nil || cfg == nil {
return ""
}
return normalizeToken(cfg.SavedToken)
}

func normalizeToken(token string) string {
trimmed := strings.TrimSpace(token)
if trimmed == "" {
return ""
}

parts := strings.Fields(trimmed)
if len(parts) >= 2 && strings.EqualFold(parts[0], "bearer") {
return strings.TrimSpace(strings.Join(parts[1:], " "))
}

return trimmed
}

// updateConfig reads the current config (if any), applies fn, and writes it back.
func updateConfig(fn func(*CLIConfig)) error {
path, err := configFilePath()
if err != nil {
return err
}
if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
return err
}
cfg := &CLIConfig{ServerURL: strings.TrimSpace(serverURL)}

cfg := &CLIConfig{}
if data, readErr := os.ReadFile(path); readErr == nil {
if jsonErr := json.Unmarshal(data, cfg); jsonErr != nil {
fmt.Fprintf(os.Stderr, "Warning: caib config file is corrupted and will be reset (%s): %v\n", path, jsonErr)
}
}

fn(cfg)

data, err := json.MarshalIndent(cfg, "", " ")
if err != nil {
return err
Expand Down
Loading