From 1cdef4d5fe49844d0d7cd5345467db473927c069 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Wed, 5 Aug 2026 12:58:00 -0400 Subject: [PATCH 01/14] feat(auth): add OIDC device authorization flow for CLI authentication Implement OAuth 2.0 Device Authorization Grant (RFC 8628) for the DCM CLI. This adds `dcm login` and `dcm logout` commands, token storage with OS keyring primary and file fallback, and an authenticated HTTP transport with lazy token loading and auto-refresh. Key capabilities: - `dcm login` performs OIDC device flow via Keycloak dcm-cli client - `dcm logout` revokes refresh token and clears stored credentials - AuthTransport injects Bearer tokens with automatic refresh on expiry - DCM_TOKEN / --token bypasses OIDC flow for CI/scripting - Config persistence: login writes issuer-url to the active config file - Token file permissions: 0600 file, 0700 directory - HTTP scheme warning when sending tokens over unencrypted connections - Login/logout and token refresh use TLS from the issuer URL/base transport Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/auth.go | 161 ++++++++++++++++++++++++++ internal/auth/token.go | 212 +++++++++++++++++++++++++++++++++++ internal/auth/transport.go | 162 ++++++++++++++++++++++++++ internal/commands/helpers.go | 61 ++++++++-- internal/commands/login.go | 65 +++++++++++ internal/commands/logout.go | 54 +++++++++ internal/commands/root.go | 4 + internal/config/config.go | 74 ++++++++++++ 8 files changed, 785 insertions(+), 8 deletions(-) create mode 100644 internal/auth/auth.go create mode 100644 internal/auth/token.go create mode 100644 internal/auth/transport.go create mode 100644 internal/commands/login.go create mode 100644 internal/commands/logout.go diff --git a/internal/auth/auth.go b/internal/auth/auth.go new file mode 100644 index 0000000..97484b4 --- /dev/null +++ b/internal/auth/auth.go @@ -0,0 +1,161 @@ +// Package auth implements OIDC device authorization flow, token storage, and +// authenticated HTTP transport for the DCM CLI. +package auth + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "os/exec" + "runtime" + "strings" + + "github.com/coreos/go-oidc/v3/oidc" + "golang.org/x/oauth2" +) + +const ClientID = "dcm-cli" + +var scopes = []string{oidc.ScopeOpenID, "profile", "email", "offline_access"} + +// DeviceLogin performs the OAuth 2.0 Device Authorization Grant (RFC 8628) +// against the given OIDC issuer. It prints the verification URL and user code +// to w, attempts to open a browser, and polls until the user completes +// authentication. +func DeviceLogin(ctx context.Context, issuerURL string, httpClient *http.Client, w io.Writer) (*TokenData, error) { + oidcCtx := oidc.ClientContext(ctx, httpClient) + + provider, err := oidc.NewProvider(oidcCtx, issuerURL) + if err != nil { + return nil, fmt.Errorf("OIDC discovery failed for %s: %w", issuerURL, err) + } + + endpoint := provider.Endpoint() + oauthCfg := &oauth2.Config{ + ClientID: ClientID, + Endpoint: endpoint, + Scopes: scopes, + } + + devAuth, err := oauthCfg.DeviceAuth(oidcCtx) + if err != nil { + return nil, fmt.Errorf("device authorization request failed: %w", err) + } + + openURL := devAuth.VerificationURI + if devAuth.VerificationURIComplete != "" { + openURL = devAuth.VerificationURIComplete + } + + if _, err := fmt.Fprintf(w, "Open %s in your browser\n", openURL); err != nil { + return nil, fmt.Errorf("writing login output: %w", err) + } + if devAuth.VerificationURIComplete != "" { + if _, err := fmt.Fprintf(w, "Or visit %s and enter code: %s\n", devAuth.VerificationURI, devAuth.UserCode); err != nil { + return nil, fmt.Errorf("writing login output: %w", err) + } + } else { + if _, err := fmt.Fprintf(w, "Enter code: %s\n", devAuth.UserCode); err != nil { + return nil, fmt.Errorf("writing login output: %w", err) + } + } + + _ = openBrowser(openURL) + + token, err := oauthCfg.DeviceAccessToken(oidcCtx, devAuth) + if err != nil { + return nil, fmt.Errorf("device authorization failed: %w", err) + } + + idToken, _ := token.Extra("id_token").(string) + + return &TokenData{ + AccessToken: token.AccessToken, + RefreshToken: token.RefreshToken, + IDToken: idToken, + Expiry: token.Expiry, + TokenEndpoint: endpoint.TokenURL, + }, nil +} + +// RevokeToken revokes the given refresh token at the OIDC provider's +// revocation endpoint. +func RevokeToken(ctx context.Context, issuerURL string, refreshToken string, httpClient *http.Client) error { + oidcCtx := oidc.ClientContext(ctx, httpClient) + + provider, err := oidc.NewProvider(oidcCtx, issuerURL) + if err != nil { + return fmt.Errorf("OIDC discovery failed for %s: %w", issuerURL, err) + } + + var metadata struct { + RevocationEndpoint string `json:"revocation_endpoint"` + } + if err := provider.Claims(&metadata); err != nil { + return fmt.Errorf("reading provider metadata: %w", err) + } + if metadata.RevocationEndpoint == "" { + return nil + } + + data := url.Values{ + "token": {refreshToken}, + "token_type_hint": {"refresh_token"}, + "client_id": {ClientID}, + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, metadata.RevocationEndpoint, strings.NewReader(data.Encode())) + if err != nil { + return fmt.Errorf("creating revocation request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + resp, err := httpClient.Do(req) + if err != nil { + return fmt.Errorf("revocation request failed: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode >= 400 { + return fmt.Errorf("token revocation failed with status %d", resp.StatusCode) + } + + return nil +} + +// PreferredUsername extracts the preferred_username claim from the access +// token's JWT payload without signature verification. +func PreferredUsername(accessToken string) string { + parts := strings.SplitN(accessToken, ".", 3) + if len(parts) != 3 { + return "" + } + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return "" + } + var claims struct { + PreferredUsername string `json:"preferred_username"` + } + if err := json.Unmarshal(payload, &claims); err != nil { + return "" + } + return claims.PreferredUsername +} + +func openBrowser(url string) error { + var cmd *exec.Cmd + switch runtime.GOOS { + case "darwin": + cmd = exec.Command("open", url) + case "windows": + cmd = exec.Command("cmd", "/c", "start", url) + default: + cmd = exec.Command("xdg-open", url) + } + return cmd.Start() +} diff --git a/internal/auth/token.go b/internal/auth/token.go new file mode 100644 index 0000000..02d0ffa --- /dev/null +++ b/internal/auth/token.go @@ -0,0 +1,212 @@ +package auth + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/zalando/go-keyring" +) + +const keyringService = "dcm-cli" + +// TokenData holds the tokens and cached OIDC metadata from a login session. +type TokenData struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + IDToken string `json:"id_token,omitempty"` + Expiry time.Time `json:"expiry"` + TokenEndpoint string `json:"token_endpoint"` +} + +func (t *TokenData) String() string { + return "[REDACTED]" +} + +func (t *TokenData) MarshalLog() string { + return "[REDACTED]" +} + +// IsExpired checks whether the access token has expired. It prefers the +// unverified JWT exp claim, falling back to TokenData.Expiry for opaque +// tokens. The clockSkew parameter provides a buffer for clock differences. +func (t *TokenData) IsExpired(clockSkew time.Duration) bool { + exp, err := jwtExpiry(t.AccessToken) + if err != nil { + if t.Expiry.IsZero() { + return true + } + exp = t.Expiry + } + return time.Now().After(exp.Add(-clockSkew)) +} + +// jwtExpiry extracts the exp claim from a JWT without signature verification. +func jwtExpiry(token string) (time.Time, error) { + parts := strings.SplitN(token, ".", 3) + if len(parts) != 3 { + return time.Time{}, fmt.Errorf("invalid JWT: expected 3 parts, got %d", len(parts)) + } + + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return time.Time{}, fmt.Errorf("decoding JWT payload: %w", err) + } + + var claims struct { + Exp json.Number `json:"exp"` + } + if err := json.Unmarshal(payload, &claims); err != nil { + return time.Time{}, fmt.Errorf("parsing JWT claims: %w", err) + } + + expInt, err := claims.Exp.Int64() + if err != nil { + return time.Time{}, fmt.Errorf("parsing exp claim: %w", err) + } + + return time.Unix(expInt, 0), nil +} + +// TokenStore persists and retrieves token data keyed by issuer URL. +type TokenStore interface { + Save(issuerURL string, data *TokenData) error + Load(issuerURL string) (*TokenData, error) + Delete(issuerURL string) error +} + +// NewTokenStore returns a TokenStore backed by the OS keyring if available, +// falling back to a file-based store otherwise. +func NewTokenStore() TokenStore { + if err := keyring.Set(keyringService, "__probe__", "probe"); err != nil { + return newFileStore() + } + _ = keyring.Delete(keyringService, "__probe__") + return &keyringStore{} +} + +// normalizeIssuer strips trailing slashes from the issuer URL for use as +// a consistent cache key. +func normalizeIssuer(issuerURL string) string { + return strings.TrimRight(issuerURL, "/") +} + +// keyringStore stores tokens in the OS keyring. +type keyringStore struct{} + +func (s *keyringStore) Save(issuerURL string, data *TokenData) error { + b, err := json.Marshal(data) + if err != nil { + return fmt.Errorf("marshalling token data: %w", err) + } + return keyring.Set(keyringService, normalizeIssuer(issuerURL), string(b)) +} + +func (s *keyringStore) Load(issuerURL string) (*TokenData, error) { + val, err := keyring.Get(keyringService, normalizeIssuer(issuerURL)) + if err != nil { + if err == keyring.ErrNotFound { + return nil, nil + } + return nil, fmt.Errorf("reading from keyring: %w", err) + } + var data TokenData + if err := json.Unmarshal([]byte(val), &data); err != nil { + return nil, fmt.Errorf("parsing stored token data: %w", err) + } + return &data, nil +} + +func (s *keyringStore) Delete(issuerURL string) error { + err := keyring.Delete(keyringService, normalizeIssuer(issuerURL)) + if err == keyring.ErrNotFound { + return nil + } + return err +} + +// fileStore stores tokens in a JSON file under ~/.dcm/. +type fileStore struct { + dir string +} + +func newFileStore() *fileStore { + home, _ := os.UserHomeDir() + return &fileStore{dir: filepath.Join(home, ".dcm")} +} + +func (s *fileStore) path() string { + return filepath.Join(s.dir, "tokens.json") +} + +func (s *fileStore) readAll() (map[string]*TokenData, error) { + data, err := os.ReadFile(s.path()) + if err != nil { + if os.IsNotExist(err) { + return make(map[string]*TokenData), nil + } + return nil, fmt.Errorf("reading token file: %w", err) + } + var store map[string]*TokenData + if err := json.Unmarshal(data, &store); err != nil { + return nil, fmt.Errorf("parsing token file: %w", err) + } + if store == nil { + store = make(map[string]*TokenData) + } + return store, nil +} + +func (s *fileStore) writeAll(store map[string]*TokenData) error { + if err := os.MkdirAll(s.dir, 0o700); err != nil { + return fmt.Errorf("creating token directory: %w", err) + } + data, err := json.Marshal(store) + if err != nil { + return fmt.Errorf("marshalling token data: %w", err) + } + tmpPath := s.path() + ".tmp" + if err := os.WriteFile(tmpPath, data, 0o600); err != nil { + return fmt.Errorf("writing token file: %w", err) + } + if err := os.Rename(tmpPath, s.path()); err != nil { + return fmt.Errorf("saving token file: %w", err) + } + return nil +} + +func (s *fileStore) Save(issuerURL string, td *TokenData) error { + store, err := s.readAll() + if err != nil { + return err + } + store[normalizeIssuer(issuerURL)] = td + return s.writeAll(store) +} + +func (s *fileStore) Load(issuerURL string) (*TokenData, error) { + store, err := s.readAll() + if err != nil { + return nil, err + } + return store[normalizeIssuer(issuerURL)], nil +} + +func (s *fileStore) Delete(issuerURL string) error { + store, err := s.readAll() + if err != nil { + return err + } + delete(store, normalizeIssuer(issuerURL)) + return s.writeAll(store) +} + +// NewFileStoreWithDir creates a file-based token store at a custom directory, +// used for testing. +func NewFileStoreWithDir(dir string) TokenStore { + return &fileStore{dir: dir} +} diff --git a/internal/auth/transport.go b/internal/auth/transport.go new file mode 100644 index 0000000..5ee27de --- /dev/null +++ b/internal/auth/transport.go @@ -0,0 +1,162 @@ +package auth + +import ( + "context" + "fmt" + "net/http" + "os" + "sync" + "time" + + "golang.org/x/oauth2" +) + +const clockSkew = 30 * time.Second + +// AuthTransport is an http.RoundTripper that injects Bearer tokens into +// outgoing requests. It supports two modes: +// - Static token: injected directly from DCM_TOKEN / --token with no +// refresh logic. +// - Stored token: loaded from a TokenStore, with automatic refresh when +// the access token expires. +type AuthTransport struct { + Base http.RoundTripper + Store TokenStore + IssuerURL string + StaticToken string + Stderr *os.File + mu sync.Mutex + warnOnce sync.Once +} + +func (t *AuthTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if t.StaticToken != "" { + t.warnHTTP(req) + req = cloneRequest(req) + req.Header.Set("Authorization", "Bearer "+t.StaticToken) + return t.base().RoundTrip(req) + } + + if t.Store == nil || t.IssuerURL == "" { + return t.base().RoundTrip(req) + } + + tokenData, err := t.Store.Load(t.IssuerURL) + if err != nil { + return nil, fmt.Errorf("loading stored credentials: %w", err) + } + if tokenData == nil { + return t.base().RoundTrip(req) + } + + if !tokenData.IsExpired(clockSkew) { + t.warnHTTP(req) + req = cloneRequest(req) + req.Header.Set("Authorization", "Bearer "+tokenData.AccessToken) + return t.base().RoundTrip(req) + } + + refreshed, err := t.refreshToken(req.Context(), tokenData) + if err != nil { + return nil, fmt.Errorf("authentication expired, run 'dcm login' to re-authenticate: %w", err) + } + + t.warnHTTP(req) + req = cloneRequest(req) + req.Header.Set("Authorization", "Bearer "+refreshed.AccessToken) + return t.base().RoundTrip(req) +} + +func (t *AuthTransport) refreshToken(ctx context.Context, tokenData *TokenData) (*TokenData, error) { + t.mu.Lock() + defer t.mu.Unlock() + + reloaded, err := t.Store.Load(t.IssuerURL) + if err != nil { + return nil, err + } + if reloaded != nil && !reloaded.IsExpired(clockSkew) { + return reloaded, nil + } + + current := tokenData + if reloaded != nil { + current = reloaded + } + + if current.RefreshToken == "" { + return nil, fmt.Errorf("no refresh token available") + } + + oauthCfg := &oauth2.Config{ + ClientID: ClientID, + Endpoint: oauth2.Endpoint{ + TokenURL: current.TokenEndpoint, + }, + } + + oldToken := &oauth2.Token{ + RefreshToken: current.RefreshToken, + } + + // Use the same TLS-capable transport as API calls. Do not wrap with + // AuthTransport — that would re-enter RoundTrip while holding t.mu. + refreshClient := &http.Client{Transport: t.base()} + refreshCtx := context.WithValue(ctx, oauth2.HTTPClient, refreshClient) + + newToken, err := oauthCfg.TokenSource(refreshCtx, oldToken).Token() + if err != nil { + return nil, err + } + + idToken, _ := newToken.Extra("id_token").(string) + refreshed := &TokenData{ + AccessToken: newToken.AccessToken, + RefreshToken: newToken.RefreshToken, + IDToken: idToken, + Expiry: newToken.Expiry, + TokenEndpoint: tokenData.TokenEndpoint, + } + + // Prefer returning the refreshed token even if persist fails. With refresh + // token rotation the IdP may have invalidated the old refresh token, so + // discarding the new tokens would leave the session unrecoverable. + if err := t.Store.Save(t.IssuerURL, refreshed); err != nil { + t.warnPersist(err) + } + + return refreshed, nil +} + +func (t *AuthTransport) warnPersist(err error) { + stderr := t.Stderr + if stderr == nil { + stderr = os.Stderr + } + _, _ = fmt.Fprintf(stderr, "Warning: could not save refreshed credentials: %v\n", err) +} + +func (t *AuthTransport) warnHTTP(req *http.Request) { + if req.URL.Scheme != "http" { + return + } + t.warnOnce.Do(func() { + stderr := t.Stderr + if stderr == nil { + stderr = os.Stderr + } + _, _ = fmt.Fprintln(stderr, "Warning: sending Bearer token over unencrypted HTTP connection") + }) +} + +func (t *AuthTransport) base() http.RoundTripper { + if t.Base != nil { + return t.Base + } + return http.DefaultTransport +} + +func cloneRequest(req *http.Request) *http.Request { + r2 := req.Clone(req.Context()) + return r2 +} diff --git a/internal/commands/helpers.go b/internal/commands/helpers.go index 1231c43..01b0be1 100644 --- a/internal/commands/helpers.go +++ b/internal/commands/helpers.go @@ -17,6 +17,7 @@ import ( spmclient "github.com/dcm-project/control-plane/pkg/sp/client/provider" sprmclient "github.com/dcm-project/control-plane/pkg/sp/client/resource_manager" + "github.com/dcm-project/cli/internal/auth" "github.com/dcm-project/cli/internal/config" "github.com/dcm-project/cli/internal/output" "github.com/spf13/cobra" @@ -66,12 +67,60 @@ func newFormatter(cmd *cobra.Command, table *output.TableDef, command string) (* // buildHTTPClient creates an HTTP client from the resolved configuration. // When the control plane URL uses https://, TLS is configured using the // TLS-related settings. When it uses http://, TLS settings are ignored. +// When auth is configured (issuer-url or token), the transport is wrapped +// with an AuthTransport that injects Bearer tokens. func buildHTTPClient(cfg *config.Config) (*http.Client, error) { - if !strings.HasPrefix(cfg.ControlPlaneURL, "https://") { - return &http.Client{}, nil + baseTransport, err := tlsTransportForURL(cfg, cfg.ControlPlaneURL) + if err != nil { + return nil, err + } + + if cfg.IssuerURL != "" || cfg.Token != "" { + var store auth.TokenStore + if cfg.Token == "" { + store = auth.NewTokenStore() + } + transport := &auth.AuthTransport{ + Base: baseTransport, + Store: store, + IssuerURL: cfg.IssuerURL, + StaticToken: cfg.Token, + } + return &http.Client{Transport: transport}, nil + } + + if baseTransport != nil { + return &http.Client{Transport: baseTransport}, nil + } + return &http.Client{}, nil +} + +// buildPlainHTTPClient returns an HTTP client with TLS configuration but no +// auth transport. Used by login and logout for OIDC protocol traffic so that +// AuthTransport cannot re-enter refresh while talking to the issuer. +// TLS is derived from the issuer URL when set, otherwise the control-plane URL. +func buildPlainHTTPClient(cfg *config.Config) (*http.Client, error) { + target := cfg.IssuerURL + if target == "" { + target = cfg.ControlPlaneURL + } + baseTransport, err := tlsTransportForURL(cfg, target) + if err != nil { + return nil, err + } + if baseTransport != nil { + return &http.Client{Transport: baseTransport}, nil + } + return &http.Client{}, nil +} + +// tlsTransportForURL builds a TLS transport when url is https://. For http:// +// URLs, TLS settings are ignored and nil is returned. +func tlsTransportForURL(cfg *config.Config, url string) (http.RoundTripper, error) { + if !strings.HasPrefix(url, "https://") { + return nil, nil } - // Validate mTLS pair: both or neither must be set. if (cfg.TLSClientCert == "") != (cfg.TLSClientKey == "") { return nil, &UsageError{Err: fmt.Errorf("--tls-client-cert and --tls-client-key must be used together")} } @@ -100,11 +149,7 @@ func buildHTTPClient(cfg *config.Config) (*http.Client, error) { tlsCfg.Certificates = []tls.Certificate{cert} } - return &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: tlsCfg, - }, - }, nil + return &http.Transport{TLSClientConfig: tlsCfg}, nil } // apiBaseURL returns the API base URL with the /api/v1alpha1 suffix. diff --git a/internal/commands/login.go b/internal/commands/login.go new file mode 100644 index 0000000..19c6502 --- /dev/null +++ b/internal/commands/login.go @@ -0,0 +1,65 @@ +package commands + +import ( + "context" + "fmt" + "time" + + "github.com/dcm-project/cli/internal/auth" + "github.com/dcm-project/cli/internal/config" + "github.com/spf13/cobra" +) + +func newLoginCommand() *cobra.Command { + return &cobra.Command{ + Use: "login", + Short: "Authenticate with the DCM control plane", + Long: "Authenticate with the DCM control plane using OIDC device authorization flow.", + RunE: func(cmd *cobra.Command, _ []string) error { + cfg := config.FromCommand(cmd) + if cfg.IssuerURL == "" { + return &UsageError{Err: fmt.Errorf("--issuer-url is required (or set DCM_ISSUER_URL)")} + } + + // Plain client: OIDC discovery/device/token must not go through + // AuthTransport (expired stored tokens would deadlock on refresh). + httpClient, err := buildPlainHTTPClient(cfg) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(cmd.Context(), 5*time.Minute) + defer cancel() + + tokenData, err := auth.DeviceLogin(ctx, cfg.IssuerURL, httpClient, cmd.ErrOrStderr()) + if err != nil { + return err + } + + store := auth.NewTokenStore() + if err := store.Save(cfg.IssuerURL, tokenData); err != nil { + return fmt.Errorf("saving credentials: %w", err) + } + + configValues := map[string]string{ + "issuer-url": cfg.IssuerURL, + } + if cfg.ControlPlaneURL != "" { + configValues["control-plane-url"] = cfg.ControlPlaneURL + } + if err := config.SaveConfig(config.ConfigPath(cmd), configValues); err != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not save config: %v\n", err) + } + + username := auth.PreferredUsername(tokenData.AccessToken) + ttl := time.Until(tokenData.Expiry).Round(time.Second) + if username != "" { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Logged in as %s (token expires in %s; auto-refresh enabled)\n", username, ttl) + } else { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Logged in successfully (token expires in %s; auto-refresh enabled)\n", ttl) + } + + return nil + }, + } +} diff --git a/internal/commands/logout.go b/internal/commands/logout.go new file mode 100644 index 0000000..f8cb377 --- /dev/null +++ b/internal/commands/logout.go @@ -0,0 +1,54 @@ +package commands + +import ( + "fmt" + + "github.com/dcm-project/cli/internal/auth" + "github.com/dcm-project/cli/internal/config" + "github.com/spf13/cobra" +) + +func newLogoutCommand() *cobra.Command { + return &cobra.Command{ + Use: "logout", + Short: "Clear stored authentication credentials", + Long: "Revoke stored tokens and clear authentication credentials.", + RunE: func(cmd *cobra.Command, _ []string) error { + cfg := config.FromCommand(cmd) + if cfg.IssuerURL == "" { + return &UsageError{Err: fmt.Errorf("--issuer-url is required (or set DCM_ISSUER_URL)")} + } + + store := auth.NewTokenStore() + tokenData, err := store.Load(cfg.IssuerURL) + if err != nil { + return fmt.Errorf("reading stored credentials: %w", err) + } + + if tokenData == nil { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "No stored credentials found") + return nil + } + + if tokenData.RefreshToken != "" { + httpClient, err := buildPlainHTTPClient(cfg) + if err != nil { + return err + } + ctx, cancel := requestContext(cmd) + defer cancel() + + if err := auth.RevokeToken(ctx, cfg.IssuerURL, tokenData.RefreshToken, httpClient); err != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: token revocation failed: %v\n", err) + } + } + + if err := store.Delete(cfg.IssuerURL); err != nil { + return fmt.Errorf("clearing stored credentials: %w", err) + } + + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Logged out successfully") + return nil + }, + } +} diff --git a/internal/commands/root.go b/internal/commands/root.go index 1098666..c681f27 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -49,12 +49,16 @@ func NewRootCommand() *cobra.Command { flags.String("tls-client-cert", "", "Path to client certificate file for mTLS") flags.String("tls-client-key", "", "Path to client private key file for mTLS") flags.Bool("tls-skip-verify", false, "Skip TLS certificate verification") + flags.String("issuer-url", "", "OIDC issuer URL for authentication") + flags.String("token", "", "Bearer token for authentication (bypasses OIDC flow)") cmd.AddCommand(newPolicyCommand()) cmd.AddCommand(newCatalogCommand()) cmd.AddCommand(newSPCommand()) cmd.AddCommand(newVersionCommand()) cmd.AddCommand(newCompletionCommand()) + cmd.AddCommand(newLoginCommand()) + cmd.AddCommand(newLogoutCommand()) return cmd } diff --git a/internal/config/config.go b/internal/config/config.go index c9fc49f..b0f56d3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" + "go.yaml.in/yaml/v3" ) const defaultControlPlaneURL = "http://localhost:8080" @@ -41,6 +42,8 @@ type Config struct { TLSClientCert string `yaml:"tls-client-cert" mapstructure:"tls-client-cert"` TLSClientKey string `yaml:"tls-client-key" mapstructure:"tls-client-key"` TLSSkipVerify bool `yaml:"tls-skip-verify" mapstructure:"tls-skip-verify"` + IssuerURL string `yaml:"issuer-url" mapstructure:"issuer-url"` + Token string `yaml:"-" mapstructure:"token"` } // Load reads configuration from file, environment variables, and command-line @@ -56,6 +59,8 @@ func Load(cmd *cobra.Command) (*Config, error) { v.SetDefault("tls-client-cert", "") v.SetDefault("tls-client-key", "") v.SetDefault("tls-skip-verify", false) + v.SetDefault("issuer-url", "") + v.SetDefault("token", "") // Environment variable binding (REQ-CFG-030) v.SetEnvPrefix("DCM") @@ -66,6 +71,8 @@ func Load(cmd *cobra.Command) (*Config, error) { v.MustBindEnv("tls-client-cert", "DCM_TLS_CLIENT_CERT") v.MustBindEnv("tls-client-key", "DCM_TLS_CLIENT_KEY") v.MustBindEnv("tls-skip-verify", "DCM_TLS_SKIP_VERIFY") + v.MustBindEnv("issuer-url", "DCM_ISSUER_URL") + v.MustBindEnv("token", "DCM_TOKEN") // Config file path (REQ-CFG-010, REQ-CFG-020) configPath := configFilePath(cmd) @@ -118,6 +125,71 @@ func configFilePath(cmd *cobra.Command) string { return "" } +// ConfigPath returns the config file path that Load would use for cmd: +// --config / DCM_CONFIG if set, otherwise ~/.dcm/config.yaml. +func ConfigPath(cmd *cobra.Command) string { + if path := configFilePath(cmd); path != "" { + return path + } + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return filepath.Join(home, ".dcm", "config.yaml") +} + +// SaveConfig merges the provided key-value pairs into the config file at path, +// creating the file and parent directory if they don't exist. Existing values +// not present in the values map are preserved. If path is empty, writes to +// ~/.dcm/config.yaml. +func SaveConfig(path string, values map[string]string) error { + configPath := path + if configPath == "" { + home, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("unable to determine home directory: %w", err) + } + configPath = filepath.Join(home, ".dcm", "config.yaml") + } + + dir := filepath.Dir(configPath) + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("creating config directory: %w", err) + } + + existing := make(map[string]any) + data, err := os.ReadFile(configPath) + if err == nil { + if yamlErr := yaml.Unmarshal(data, &existing); yamlErr != nil { + return fmt.Errorf("parsing existing config: %w", yamlErr) + } + } else if !os.IsNotExist(err) { + return fmt.Errorf("reading config file: %w", err) + } + if existing == nil { + existing = make(map[string]any) + } + + for k, v := range values { + existing[k] = v + } + + out, err := yaml.Marshal(existing) + if err != nil { + return fmt.Errorf("marshalling config: %w", err) + } + + tmpPath := configPath + ".tmp" + if err := os.WriteFile(tmpPath, out, 0o600); err != nil { + return fmt.Errorf("writing config file: %w", err) + } + if err := os.Rename(tmpPath, configPath); err != nil { + return fmt.Errorf("saving config file: %w", err) + } + + return nil +} + // bindFlags binds only flags that were explicitly set by the user, so that // unset flags don't override environment variables or config file values. func bindFlags(v *viper.Viper, cmd *cobra.Command) error { @@ -129,6 +201,8 @@ func bindFlags(v *viper.Viper, cmd *cobra.Command) error { "tls-client-cert": "tls-client-cert", "tls-client-key": "tls-client-key", "tls-skip-verify": "tls-skip-verify", + "issuer-url": "issuer-url", + "token": "token", } for flagName, configKey := range flagToKey { From 0423a90fd3daa56dd7f861f4a382f32a21b928c1 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Wed, 5 Aug 2026 12:58:00 -0400 Subject: [PATCH 02/14] test(auth): add OIDC login, logout, and transport unit coverage Expand unit and command tests with an OIDC mock server, covering device flow login/logout, AuthTransport refresh behavior, token store fallbacks, and FileStore inaccessible-path error handling. Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/auth_suite_test.go | 13 ++ internal/auth/auth_test.go | 268 +++++++++++++++++++++++ internal/auth/token_test.go | 289 ++++++++++++++++++++++++ internal/auth/transport_test.go | 327 ++++++++++++++++++++++++++++ internal/commands/helpers_test.go | 19 ++ internal/commands/login_test.go | 153 +++++++++++++ internal/commands/logout_test.go | 117 ++++++++++ internal/commands/oidc_mock_test.go | 156 +++++++++++++ internal/commands/policy_test.go | 2 + internal/commands/root_test.go | 4 + internal/config/config_test.go | 198 ++++++++++++++++- 11 files changed, 1545 insertions(+), 1 deletion(-) create mode 100644 internal/auth/auth_suite_test.go create mode 100644 internal/auth/auth_test.go create mode 100644 internal/auth/token_test.go create mode 100644 internal/auth/transport_test.go create mode 100644 internal/commands/login_test.go create mode 100644 internal/commands/logout_test.go create mode 100644 internal/commands/oidc_mock_test.go diff --git a/internal/auth/auth_suite_test.go b/internal/auth/auth_suite_test.go new file mode 100644 index 0000000..cc266fc --- /dev/null +++ b/internal/auth/auth_suite_test.go @@ -0,0 +1,13 @@ +package auth_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestAuth(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Auth Suite") +} diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go new file mode 100644 index 0000000..37bd2e2 --- /dev/null +++ b/internal/auth/auth_test.go @@ -0,0 +1,268 @@ +package auth_test + +import ( + "bytes" + "context" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/dcm-project/cli/internal/auth" +) + +func mockOIDCServer(pollsBeforeSuccess int) *httptest.Server { + var pollCount atomic.Int32 + + mux := http.NewServeMux() + + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + baseURL := "http://" + r.Host + discovery := map[string]any{ + "issuer": baseURL, + "authorization_endpoint": baseURL + "/protocol/openid-connect/auth", + "token_endpoint": baseURL + "/protocol/openid-connect/token", + "device_authorization_endpoint": baseURL + "/protocol/openid-connect/auth/device", + "revocation_endpoint": baseURL + "/protocol/openid-connect/revoke", + "jwks_uri": baseURL + "/protocol/openid-connect/certs", + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256"}, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(discovery) + }) + + mux.HandleFunc("/protocol/openid-connect/auth/device", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + Expect(r.FormValue("client_id")).To(Equal(auth.ClientID)) + resp := map[string]any{ + "device_code": "test-device-code", + "user_code": "ABCD-EFGH", + "verification_uri": "http://" + r.Host + "/device", + "verification_uri_complete": "http://" + r.Host + "/device?user_code=ABCD-EFGH", + "expires_in": 600, + "interval": 0, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + }) + + mux.HandleFunc("/protocol/openid-connect/token", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + + grantType := r.FormValue("grant_type") + + if grantType == "refresh_token" { + exp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeJWT(exp), + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + "id_token": "new-id-token", + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + return + } + + count := int(pollCount.Add(1)) + if count <= pollsBeforeSuccess { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]string{ + "error": "authorization_pending", + }) + return + } + + exp := time.Now().Add(5 * time.Minute) + accessToken := makeJWTWithUsername(exp, "dcm-admin") + resp := map[string]any{ + "access_token": accessToken, + "refresh_token": "test-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + "id_token": "test-id-token", + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + }) + + mux.HandleFunc("/protocol/openid-connect/revoke", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + Expect(r.FormValue("client_id")).To(Equal(auth.ClientID)) + w.WriteHeader(http.StatusOK) + }) + + mux.HandleFunc("/protocol/openid-connect/certs", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"keys": []any{}}) + }) + + return httptest.NewServer(mux) +} + +func makeJWTWithUsername(exp time.Time, username string) string { + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`)) + claims := fmt.Sprintf(`{"exp":%d,"sub":"test-user","preferred_username":"%s"}`, exp.Unix(), username) + payload := base64.RawURLEncoding.EncodeToString([]byte(claims)) + sig := base64.RawURLEncoding.EncodeToString([]byte("fake-signature")) + return header + "." + payload + "." + sig +} + +var _ = Describe("DeviceLogin", func() { + var ( + server *httptest.Server + output *bytes.Buffer + ) + + AfterEach(func() { + if server != nil { + server.Close() + server = nil + } + }) + + It("completes the device flow and returns token data", func() { + server = mockOIDCServer(1) + output = new(bytes.Buffer) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + td, err := auth.DeviceLogin(ctx, server.URL, server.Client(), output) + Expect(err).NotTo(HaveOccurred()) + Expect(td).NotTo(BeNil()) + Expect(td.AccessToken).NotTo(BeEmpty()) + Expect(td.RefreshToken).To(Equal("test-refresh-token")) + Expect(td.IDToken).To(Equal("test-id-token")) + Expect(td.TokenEndpoint).To(ContainSubstring("/protocol/openid-connect/token")) + + Expect(output.String()).To(ContainSubstring("ABCD-EFGH")) + Expect(output.String()).To(ContainSubstring("/device")) + }) + + It("fails when OIDC discovery fails", func() { + badServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer badServer.Close() + + output = new(bytes.Buffer) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + _, err := auth.DeviceLogin(ctx, badServer.URL, badServer.Client(), output) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("OIDC discovery failed")) + }) +}) + +var _ = Describe("ClientID", func() { + It("is the hardcoded public client dcm-cli", func() { + Expect(auth.ClientID).To(Equal("dcm-cli")) + }) +}) + +var _ = Describe("PreferredUsername", func() { + It("extracts preferred_username from a valid JWT", func() { + token := makeJWTWithUsername(time.Now().Add(5*time.Minute), "dcm-admin") + Expect(auth.PreferredUsername(token)).To(Equal("dcm-admin")) + }) + + It("returns empty string for invalid JWT", func() { + Expect(auth.PreferredUsername("not-a-jwt")).To(BeEmpty()) + }) +}) + +var _ = Describe("RevokeToken", func() { + It("sends revocation request to the provider", func() { + server := mockOIDCServer(0) + defer server.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + err := auth.RevokeToken(ctx, server.URL, "test-refresh-token", server.Client()) + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns nil when the provider has no revocation_endpoint", func() { + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + baseURL := "http://" + r.Host + discovery := map[string]any{ + "issuer": baseURL, + "authorization_endpoint": baseURL + "/protocol/openid-connect/auth", + "token_endpoint": baseURL + "/protocol/openid-connect/token", + "jwks_uri": baseURL + "/protocol/openid-connect/certs", + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256"}, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(discovery) + }) + mux.HandleFunc("/protocol/openid-connect/certs", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"keys": []any{}}) + }) + server := httptest.NewServer(mux) + defer server.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + err := auth.RevokeToken(ctx, server.URL, "test-refresh-token", server.Client()) + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns an error when the revocation endpoint responds with 4xx", func() { + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + baseURL := "http://" + r.Host + discovery := map[string]any{ + "issuer": baseURL, + "authorization_endpoint": baseURL + "/protocol/openid-connect/auth", + "token_endpoint": baseURL + "/protocol/openid-connect/token", + "revocation_endpoint": baseURL + "/protocol/openid-connect/revoke", + "jwks_uri": baseURL + "/protocol/openid-connect/certs", + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256"}, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(discovery) + }) + mux.HandleFunc("/protocol/openid-connect/revoke", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + }) + mux.HandleFunc("/protocol/openid-connect/certs", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"keys": []any{}}) + }) + server := httptest.NewServer(mux) + defer server.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + err := auth.RevokeToken(ctx, server.URL, "test-refresh-token", server.Client()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("token revocation failed with status 400")) + }) +}) diff --git a/internal/auth/token_test.go b/internal/auth/token_test.go new file mode 100644 index 0000000..0e1a804 --- /dev/null +++ b/internal/auth/token_test.go @@ -0,0 +1,289 @@ +package auth_test + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/dcm-project/cli/internal/auth" +) + +// makeJWT builds a minimal unsigned JWT with the given exp claim. +func makeJWT(exp time.Time) string { + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`)) + claims := fmt.Sprintf(`{"exp":%d,"sub":"test-user"}`, exp.Unix()) + payload := base64.RawURLEncoding.EncodeToString([]byte(claims)) + sig := base64.RawURLEncoding.EncodeToString([]byte("fake-signature")) + return header + "." + payload + "." + sig +} + +func sampleTokenData(exp time.Time) *auth.TokenData { + return &auth.TokenData{ + AccessToken: makeJWT(exp), + RefreshToken: "refresh-token-value", + IDToken: "id-token-value", + Expiry: exp, + TokenEndpoint: "http://keycloak:8080/realms/dcm/protocol/openid-connect/token", + } +} + +var _ = Describe("TokenData", func() { + Describe("String", func() { + It("redacts token values", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(td.String()).To(Equal("[REDACTED]")) + }) + }) + + Describe("IsExpired", func() { + It("returns false for a token expiring in the future beyond clock skew", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(td.IsExpired(30 * time.Second)).To(BeFalse()) + }) + + It("returns true for a token that expired in the past", func() { + td := sampleTokenData(time.Now().Add(-1 * time.Second)) + Expect(td.IsExpired(30 * time.Second)).To(BeTrue()) + }) + + It("returns true for a token within the clock skew buffer", func() { + td := sampleTokenData(time.Now().Add(20 * time.Second)) + Expect(td.IsExpired(30 * time.Second)).To(BeTrue()) + }) + + It("returns false for a token just outside the clock skew buffer", func() { + td := sampleTokenData(time.Now().Add(31 * time.Second)) + Expect(td.IsExpired(30 * time.Second)).To(BeFalse()) + }) + + It("returns true for an invalid JWT with no Expiry fallback", func() { + td := &auth.TokenData{AccessToken: "not-a-jwt"} + Expect(td.IsExpired(30 * time.Second)).To(BeTrue()) + }) + + It("falls back to TokenData.Expiry for opaque access tokens", func() { + td := &auth.TokenData{ + AccessToken: "opaque-access-token", + Expiry: time.Now().Add(5 * time.Minute), + } + Expect(td.IsExpired(30 * time.Second)).To(BeFalse()) + }) + + It("treats opaque tokens as expired when TokenData.Expiry is past", func() { + td := &auth.TokenData{ + AccessToken: "opaque-access-token", + Expiry: time.Now().Add(-time.Minute), + } + Expect(td.IsExpired(30 * time.Second)).To(BeTrue()) + }) + }) +}) + +var _ = Describe("FileStore", func() { + var ( + store auth.TokenStore + storeDir string + issuerURL string + ) + + BeforeEach(func() { + storeDir = GinkgoT().TempDir() + store = auth.NewFileStoreWithDir(storeDir) + issuerURL = "http://keycloak:8080/realms/dcm" + }) + + Describe("Save and Load round-trip", func() { + It("persists and retrieves token data", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save(issuerURL, td)).To(Succeed()) + + loaded, err := store.Load(issuerURL) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded).NotTo(BeNil()) + Expect(loaded.AccessToken).To(Equal(td.AccessToken)) + Expect(loaded.RefreshToken).To(Equal(td.RefreshToken)) + Expect(loaded.TokenEndpoint).To(Equal(td.TokenEndpoint)) + }) + }) + + Describe("Load with no stored token", func() { + It("returns nil without error", func() { + loaded, err := store.Load(issuerURL) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded).To(BeNil()) + }) + }) + + Describe("Delete", func() { + It("removes stored token data", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save(issuerURL, td)).To(Succeed()) + Expect(store.Delete(issuerURL)).To(Succeed()) + + loaded, err := store.Load(issuerURL) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded).To(BeNil()) + }) + + It("succeeds when no token exists", func() { + Expect(store.Delete(issuerURL)).To(Succeed()) + }) + }) + + Describe("Issuer URL normalization", func() { + It("treats trailing-slash and no-trailing-slash as the same key", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save(issuerURL+"/", td)).To(Succeed()) + + loaded, err := store.Load(issuerURL) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded).NotTo(BeNil()) + Expect(loaded.AccessToken).To(Equal(td.AccessToken)) + }) + }) + + Describe("File permissions", func() { + It("creates the token file with 0600 permissions", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save(issuerURL, td)).To(Succeed()) + + info, err := os.Stat(filepath.Join(storeDir, "tokens.json")) + Expect(err).NotTo(HaveOccurred()) + Expect(info.Mode().Perm()).To(Equal(os.FileMode(0o600))) + }) + + It("creates the token directory with 0700 permissions", func() { + nestedDir := filepath.Join(GinkgoT().TempDir(), "nested", ".dcm") + store = auth.NewFileStoreWithDir(nestedDir) + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save(issuerURL, td)).To(Succeed()) + + info, err := os.Stat(nestedDir) + Expect(err).NotTo(HaveOccurred()) + Expect(info.Mode().Perm()).To(Equal(os.FileMode(0o700))) + }) + }) + + Describe("Inaccessible paths", func() { + BeforeEach(func() { + if os.Geteuid() == 0 { + Skip("permission-bit checks are unreliable when running as root") + } + }) + + It("returns an error when Save cannot write to an unwritable store directory", func() { + // 0555: readable so readAll sees IsNotExist, but WriteFile of .tmp fails. + Expect(os.MkdirAll(storeDir, 0o755)).To(Succeed()) + Expect(os.Chmod(storeDir, 0o555)).To(Succeed()) + DeferCleanup(func() { + _ = os.Chmod(storeDir, 0o700) + }) + + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + err := store.Save(issuerURL, td) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("writing token file")) + }) + + It("returns an error when Save cannot create the store directory", func() { + // Parent must be searchable so readAll gets IsNotExist for the nested + // path, then MkdirAll fails because the parent is not writable. + parent := GinkgoT().TempDir() + Expect(os.Chmod(parent, 0o555)).To(Succeed()) + DeferCleanup(func() { + _ = os.Chmod(parent, 0o700) + }) + + store = auth.NewFileStoreWithDir(filepath.Join(parent, "nested")) + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + err := store.Save(issuerURL, td) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("creating token directory")) + }) + + It("returns an error when Load and Save cannot read an unreadable tokens.json", func() { + Expect(os.MkdirAll(storeDir, 0o700)).To(Succeed()) + tokenPath := filepath.Join(storeDir, "tokens.json") + Expect(os.WriteFile(tokenPath, []byte("{}"), 0o600)).To(Succeed()) + Expect(os.Chmod(tokenPath, 0o000)).To(Succeed()) + DeferCleanup(func() { + _ = os.Chmod(tokenPath, 0o600) + }) + + _, loadErr := store.Load(issuerURL) + Expect(loadErr).To(HaveOccurred()) + Expect(loadErr.Error()).To(ContainSubstring("reading token file")) + + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + saveErr := store.Save(issuerURL, td) + Expect(saveErr).To(HaveOccurred()) + Expect(saveErr.Error()).To(ContainSubstring("reading token file")) + }) + }) + + Describe("Atomic writes", func() { + It("does not leave a .tmp file on success", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save(issuerURL, td)).To(Succeed()) + + _, err := os.Stat(filepath.Join(storeDir, "tokens.json.tmp")) + Expect(os.IsNotExist(err)).To(BeTrue()) + }) + }) + + Describe("Multiple issuers", func() { + It("stores tokens for different issuers independently", func() { + td1 := sampleTokenData(time.Now().Add(5 * time.Minute)) + td2 := sampleTokenData(time.Now().Add(10 * time.Minute)) + td2.RefreshToken = "other-refresh-token" + + issuer2 := "http://other-keycloak:8080/realms/other" + + Expect(store.Save(issuerURL, td1)).To(Succeed()) + Expect(store.Save(issuer2, td2)).To(Succeed()) + + loaded1, err := store.Load(issuerURL) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded1.RefreshToken).To(Equal("refresh-token-value")) + + loaded2, err := store.Load(issuer2) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded2.RefreshToken).To(Equal("other-refresh-token")) + }) + }) + + Describe("Corrupt file handling", func() { + It("returns an error for invalid JSON", func() { + Expect(os.MkdirAll(storeDir, 0o700)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(storeDir, "tokens.json"), []byte("not json"), 0o600)).To(Succeed()) + + _, err := store.Load(issuerURL) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("parsing token file")) + }) + }) +}) + +var _ = Describe("SaveConfig integration", func() { + It("creates config file when it does not exist", func() { + dir := GinkgoT().TempDir() + configPath := filepath.Join(dir, "config.yaml") + + store := auth.NewFileStoreWithDir(dir) + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save("http://keycloak:8080/realms/dcm", td)).To(Succeed()) + + _, err := os.Stat(configPath) + Expect(os.IsNotExist(err)).To(BeTrue(), "token store should not create config.yaml") + + data, err := json.Marshal(map[string]string{"issuer-url": "http://keycloak:8080/realms/dcm"}) + Expect(err).NotTo(HaveOccurred()) + Expect(data).NotTo(BeEmpty()) + }) +}) diff --git a/internal/auth/transport_test.go b/internal/auth/transport_test.go new file mode 100644 index 0000000..ba94a1d --- /dev/null +++ b/internal/auth/transport_test.go @@ -0,0 +1,327 @@ +package auth_test + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync/atomic" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/dcm-project/cli/internal/auth" +) + +type failSaveStore struct { + auth.TokenStore + err error +} + +func (s *failSaveStore) Save(issuerURL string, data *auth.TokenData) error { + return s.err +} + +type countingRoundTripper struct { + base http.RoundTripper + hits *atomic.Int32 + matchHost string +} + +func (t *countingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + if req.Host == t.matchHost { + t.hits.Add(1) + } + return t.base.RoundTrip(req) +} + +var _ = Describe("AuthTransport", func() { + var ( + backend *httptest.Server + storeDir string + store auth.TokenStore + receivedAuth string + ) + + BeforeEach(func() { + receivedAuth = "" + backend = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedAuth = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + })) + + storeDir = GinkgoT().TempDir() + store = auth.NewFileStoreWithDir(storeDir) + }) + + AfterEach(func() { + if backend != nil { + backend.Close() + } + }) + + Describe("Static token", func() { + It("injects the static Bearer token", func() { + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + StaticToken: "my-static-token", + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(receivedAuth).To(Equal("Bearer my-static-token")) + }) + }) + + Describe("Stored token (valid)", func() { + It("injects the stored access token", func() { + td := sampleTokenData(time.Now().Add(5 * time.Minute)) + Expect(store.Save("http://keycloak:8080/realms/dcm", td)).To(Succeed()) + + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + Store: store, + IssuerURL: "http://keycloak:8080/realms/dcm", + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(receivedAuth).To(HavePrefix("Bearer ")) + Expect(receivedAuth).To(Equal("Bearer " + td.AccessToken)) + }) + }) + + Describe("No stored token", func() { + It("passes through without Authorization header", func() { + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + Store: store, + IssuerURL: "http://keycloak:8080/realms/dcm", + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(receivedAuth).To(BeEmpty()) + }) + }) + + Describe("Expired token with refresh", func() { + It("refreshes and injects the new access token", func() { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.FormValue("grant_type") == "refresh_token" { + newExp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeJWT(newExp), + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + return + } + http.Error(w, "unexpected request", http.StatusBadRequest) + })) + defer tokenServer.Close() + + expiredTD := &auth.TokenData{ + AccessToken: makeJWT(time.Now().Add(-1 * time.Minute)), + RefreshToken: "old-refresh-token", + Expiry: time.Now().Add(-1 * time.Minute), + TokenEndpoint: tokenServer.URL, + } + Expect(store.Save("http://keycloak:8080/realms/dcm", expiredTD)).To(Succeed()) + + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + Store: store, + IssuerURL: "http://keycloak:8080/realms/dcm", + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(receivedAuth).To(HavePrefix("Bearer ")) + Expect(receivedAuth).NotTo(Equal("Bearer " + expiredTD.AccessToken)) + + reloaded, err := store.Load("http://keycloak:8080/realms/dcm") + Expect(err).NotTo(HaveOccurred()) + Expect(reloaded.RefreshToken).To(Equal("new-refresh-token")) + }) + }) + + Describe("Expired token with failed refresh", func() { + It("returns an actionable error message", func() { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "invalid_grant"}) + })) + defer tokenServer.Close() + + expiredTD := &auth.TokenData{ + AccessToken: makeJWT(time.Now().Add(-1 * time.Minute)), + RefreshToken: "expired-refresh-token", + Expiry: time.Now().Add(-1 * time.Minute), + TokenEndpoint: tokenServer.URL, + } + Expect(store.Save("http://keycloak:8080/realms/dcm", expiredTD)).To(Succeed()) + + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + Store: store, + IssuerURL: "http://keycloak:8080/realms/dcm", + } + client := &http.Client{Transport: transport} + + _, err := client.Get(backend.URL) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("dcm login")) + }) + }) + + Describe("Refresh uses Base transport", func() { + It("sends the refresh request through AuthTransport.Base", func() { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.FormValue("grant_type") == "refresh_token" { + newExp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeJWT(newExp), + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + return + } + http.Error(w, "unexpected request", http.StatusBadRequest) + })) + defer tokenServer.Close() + + expiredTD := &auth.TokenData{ + AccessToken: makeJWT(time.Now().Add(-1 * time.Minute)), + RefreshToken: "old-refresh-token", + Expiry: time.Now().Add(-1 * time.Minute), + TokenEndpoint: tokenServer.URL, + } + Expect(store.Save("http://keycloak:8080/realms/dcm", expiredTD)).To(Succeed()) + + var hits atomic.Int32 + base := &countingRoundTripper{ + base: http.DefaultTransport, + hits: &hits, + matchHost: strings.TrimPrefix(strings.TrimPrefix(tokenServer.URL, "https://"), "http://"), + } + transport := &auth.AuthTransport{ + Base: base, + Store: store, + IssuerURL: "http://keycloak:8080/realms/dcm", + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(hits.Load()).To(BeNumerically(">=", 1)) + }) + }) + + Describe("Refresh persist failure", func() { + It("still injects the refreshed token when Store.Save fails", func() { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.FormValue("grant_type") == "refresh_token" { + newExp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeJWT(newExp), + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + return + } + http.Error(w, "unexpected request", http.StatusBadRequest) + })) + defer tokenServer.Close() + + expiredTD := &auth.TokenData{ + AccessToken: makeJWT(time.Now().Add(-1 * time.Minute)), + RefreshToken: "old-refresh-token", + Expiry: time.Now().Add(-1 * time.Minute), + TokenEndpoint: tokenServer.URL, + } + Expect(store.Save("http://keycloak:8080/realms/dcm", expiredTD)).To(Succeed()) + + tmpFile, err := os.CreateTemp(GinkgoT().TempDir(), "stderr-*") + Expect(err).NotTo(HaveOccurred()) + + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + Store: &failSaveStore{TokenStore: store, err: errors.New("disk full")}, + IssuerURL: "http://keycloak:8080/realms/dcm", + Stderr: tmpFile, + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(receivedAuth).To(HavePrefix("Bearer ")) + Expect(receivedAuth).NotTo(Equal("Bearer " + expiredTD.AccessToken)) + + Expect(tmpFile.Close()).To(Succeed()) + content, err := os.ReadFile(tmpFile.Name()) + Expect(err).NotTo(HaveOccurred()) + Expect(string(content)).To(ContainSubstring("could not save refreshed credentials")) + }) + }) + + Describe("HTTP scheme warning", func() { + It("warns when sending Bearer token over HTTP", func() { + tmpFile, err := os.CreateTemp(GinkgoT().TempDir(), "stderr-*") + Expect(err).NotTo(HaveOccurred()) + + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + StaticToken: "my-token", + Stderr: tmpFile, + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + + Expect(tmpFile.Close()).To(Succeed()) + content, err := os.ReadFile(tmpFile.Name()) + Expect(err).NotTo(HaveOccurred()) + Expect(string(content)).To(ContainSubstring("unencrypted HTTP")) + }) + }) + + Describe("No auth configured", func() { + It("passes through without modification", func() { + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(receivedAuth).To(BeEmpty()) + }) + }) +}) diff --git a/internal/commands/helpers_test.go b/internal/commands/helpers_test.go index 9a88408..f4d1f42 100644 --- a/internal/commands/helpers_test.go +++ b/internal/commands/helpers_test.go @@ -133,6 +133,25 @@ var _ = Describe("buildHTTPClient (via commands)", func() { }) }) + Describe("auth wiring", func() { + It("should send Authorization Bearer when --token is set", func() { + var receivedAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedAuth = r.Header.Get("Authorization") + writeJSONResponse(w, http.StatusOK, emptyListResponse()) + })) + defer server.Close() + + err := executeWithArgs( + "--control-plane-url", server.URL, + "--token", "ci-static-token", + "policy", "list", + ) + Expect(err).NotTo(HaveOccurred()) + Expect(receivedAuth).To(Equal("Bearer ci-static-token")) + }) + }) + Describe("https:// URL", func() { It("should connect to an HTTPS server with --tls-ca-cert", func() { ca := newTestCA() diff --git a/internal/commands/login_test.go b/internal/commands/login_test.go new file mode 100644 index 0000000..844feef --- /dev/null +++ b/internal/commands/login_test.go @@ -0,0 +1,153 @@ +package commands_test + +import ( + "bytes" + "crypto/tls" + "errors" + "net" + "os" + "path/filepath" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/zalando/go-keyring" + + "github.com/dcm-project/cli/internal/auth" + "github.com/dcm-project/cli/internal/commands" +) + +var _ = Describe("login command", func() { + BeforeEach(func() { + clearDCMEnvVars() + keyring.MockInit() + }) + + It("fails with UsageError when issuer-url is not configured", func() { + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{"--config", nonexistentConfigPath(), "login"}) + + err := cmd.Execute() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("--issuer-url is required (or set DCM_ISSUER_URL)")) + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + }) + + It("completes device login, stores tokens, and persists config", func() { + home := GinkgoT().TempDir() + GinkgoT().Setenv("HOME", home) + configPath := filepath.Join(home, "dcm-config.yaml") + + server := mockOIDCServer(mockOIDCOptions{pollsBeforeSuccess: 0}) + defer server.Close() + + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", configPath, + "--issuer-url", server.URL, + "--control-plane-url", "http://cp.example:8080", + "login", + }) + + err := cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + + Expect(errBuf.String()).To(ContainSubstring("Open ")) + Expect(errBuf.String()).To(ContainSubstring("ABCD-EFGH")) + Expect(errBuf.String()).To(ContainSubstring("Logged in as dcm-admin")) + Expect(errBuf.String()).To(ContainSubstring("auto-refresh enabled")) + + store := auth.NewTokenStore() + td, err := store.Load(server.URL) + Expect(err).NotTo(HaveOccurred()) + Expect(td).NotTo(BeNil()) + Expect(td.RefreshToken).To(Equal("test-refresh-token")) + Expect(td.AccessToken).NotTo(BeEmpty()) + + cfgData, err := os.ReadFile(configPath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(cfgData)).To(ContainSubstring("issuer-url: " + server.URL)) + Expect(string(cfgData)).To(ContainSubstring("control-plane-url: http://cp.example:8080")) + Expect(string(cfgData)).NotTo(ContainSubstring("token:")) + }) + + It("completes device login when stored credentials are expired and refresh fails", func() { + home := GinkgoT().TempDir() + GinkgoT().Setenv("HOME", home) + + server := mockOIDCServer(mockOIDCOptions{ + pollsBeforeSuccess: 0, + rejectRefresh: true, + }) + defer server.Close() + + store := auth.NewTokenStore() + Expect(store.Save(server.URL, &auth.TokenData{ + AccessToken: makeTestJWT(time.Now().Add(-time.Hour), "stale-user"), + RefreshToken: "invalid-refresh", + Expiry: time.Now().Add(-time.Hour), + TokenEndpoint: server.URL + "/protocol/openid-connect/token", + })).To(Succeed()) + + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", nonexistentConfigPath(), + "--issuer-url", server.URL, + "--control-plane-url", "http://cp.example:8080", + "login", + }) + + err := cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + Expect(errBuf.String()).To(ContainSubstring("Logged in as dcm-admin")) + Expect(errBuf.String()).To(ContainSubstring("auto-refresh enabled")) + + td, err := store.Load(server.URL) + Expect(err).NotTo(HaveOccurred()) + Expect(td.RefreshToken).To(Equal("test-refresh-token")) + }) + + It("applies --tls-ca-cert for HTTPS issuer when control-plane URL is HTTP", func() { + home := GinkgoT().TempDir() + GinkgoT().Setenv("HOME", home) + + ca := newTestCA() + serverCert, serverKey := ca.issueCert("localhost", []string{"localhost"}, net.IPv4(127, 0, 0, 1)) + tlsCert, err := tls.X509KeyPair(serverCert, serverKey) + Expect(err).NotTo(HaveOccurred()) + + tlsServer := mockOIDCTLSServer(mockOIDCOptions{pollsBeforeSuccess: 0}, &tls.Config{ + Certificates: []tls.Certificate{tlsCert}, + }) + defer tlsServer.Close() + + caFile := writePEM(home, "ca.pem", ca.certPEM) + + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", nonexistentConfigPath(), + "--issuer-url", tlsServer.URL, + "--control-plane-url", "http://localhost:8080", + "--tls-ca-cert", caFile, + "login", + }) + + err = cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + Expect(errBuf.String()).To(ContainSubstring("Logged in as dcm-admin")) + Expect(errBuf.String()).To(ContainSubstring("auto-refresh enabled")) + }) +}) diff --git a/internal/commands/logout_test.go b/internal/commands/logout_test.go new file mode 100644 index 0000000..d54c12e --- /dev/null +++ b/internal/commands/logout_test.go @@ -0,0 +1,117 @@ +package commands_test + +import ( + "bytes" + "errors" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/zalando/go-keyring" + + "github.com/dcm-project/cli/internal/auth" + "github.com/dcm-project/cli/internal/commands" +) + +var _ = Describe("logout command", func() { + BeforeEach(func() { + clearDCMEnvVars() + keyring.MockInit() + }) + + It("fails with UsageError when issuer-url is not configured", func() { + cmd := commands.NewRootCommand() + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(new(bytes.Buffer)) + cmd.SetArgs([]string{"--config", nonexistentConfigPath(), "logout"}) + + err := cmd.Execute() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("--issuer-url is required (or set DCM_ISSUER_URL)")) + var usageErr *commands.UsageError + Expect(errors.As(err, &usageErr)).To(BeTrue()) + }) + + It("prints a message when no credentials are stored", func() { + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", nonexistentConfigPath(), + "--issuer-url", "http://keycloak.example/realms/dcm", + "logout", + }) + + err := cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + Expect(errBuf.String()).To(ContainSubstring("No stored credentials found")) + }) + + It("revokes the refresh token and clears stored credentials", func() { + server := mockOIDCServer(mockOIDCOptions{}) + defer server.Close() + + store := auth.NewTokenStore() + td := &auth.TokenData{ + AccessToken: makeTestJWT(time.Now().Add(5*time.Minute), "dcm-admin"), + RefreshToken: "test-refresh-token", + IDToken: "test-id-token", + Expiry: time.Now().Add(5 * time.Minute), + TokenEndpoint: server.URL + "/protocol/openid-connect/token", + } + Expect(store.Save(server.URL, td)).To(Succeed()) + + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", nonexistentConfigPath(), + "--issuer-url", server.URL, + "logout", + }) + + err := cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + Expect(errBuf.String()).To(ContainSubstring("Logged out successfully")) + + loaded, err := store.Load(server.URL) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded).To(BeNil()) + }) + + It("clears credentials and warns when revocation fails", func() { + server := mockOIDCServer(mockOIDCOptions{revokeStatus: 500}) + defer server.Close() + + store := auth.NewTokenStore() + td := &auth.TokenData{ + AccessToken: makeTestJWT(time.Now().Add(5*time.Minute), "dcm-admin"), + RefreshToken: "test-refresh-token", + IDToken: "test-id-token", + Expiry: time.Now().Add(5 * time.Minute), + TokenEndpoint: server.URL + "/protocol/openid-connect/token", + } + Expect(store.Save(server.URL, td)).To(Succeed()) + + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", nonexistentConfigPath(), + "--issuer-url", server.URL, + "logout", + }) + + err := cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + Expect(errBuf.String()).To(ContainSubstring("Warning: token revocation failed")) + Expect(errBuf.String()).To(ContainSubstring("Logged out successfully")) + + loaded, err := store.Load(server.URL) + Expect(err).NotTo(HaveOccurred()) + Expect(loaded).To(BeNil()) + }) +}) diff --git a/internal/commands/oidc_mock_test.go b/internal/commands/oidc_mock_test.go new file mode 100644 index 0000000..8e2457d --- /dev/null +++ b/internal/commands/oidc_mock_test.go @@ -0,0 +1,156 @@ +package commands_test + +import ( + "crypto/tls" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "time" +) + +// mockOIDCOptions configures optional revoke behavior for the test OIDC server. +type mockOIDCOptions struct { + pollsBeforeSuccess int + revokeStatus int // 0 means 200 OK; non-zero uses that status + omitRevokeEndpoint bool + rejectRefresh bool +} + +func mockOIDCServer(opts mockOIDCOptions) *httptest.Server { + return httptest.NewServer(mockOIDCHandler(opts)) +} + +func mockOIDCTLSServer(opts mockOIDCOptions, tlsCfg *tls.Config) *httptest.Server { + server := httptest.NewUnstartedServer(mockOIDCHandler(opts)) + server.TLS = tlsCfg + server.StartTLS() + return server +} + +func mockOIDCHandler(opts mockOIDCOptions) http.Handler { + var pollCount atomic.Int32 + revokeStatus := opts.revokeStatus + if revokeStatus == 0 { + revokeStatus = http.StatusOK + } + + mux := http.NewServeMux() + + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + baseURL := requestBaseURL(r) + discovery := map[string]any{ + "issuer": baseURL, + "authorization_endpoint": baseURL + "/protocol/openid-connect/auth", + "token_endpoint": baseURL + "/protocol/openid-connect/token", + "device_authorization_endpoint": baseURL + "/protocol/openid-connect/auth/device", + "jwks_uri": baseURL + "/protocol/openid-connect/certs", + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256"}, + } + if !opts.omitRevokeEndpoint { + discovery["revocation_endpoint"] = baseURL + "/protocol/openid-connect/revoke" + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(discovery) + }) + + mux.HandleFunc("/protocol/openid-connect/auth/device", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + baseURL := requestBaseURL(r) + resp := map[string]any{ + "device_code": "test-device-code", + "user_code": "ABCD-EFGH", + "verification_uri": baseURL + "/device", + "verification_uri_complete": baseURL + "/device?user_code=ABCD-EFGH", + "expires_in": 600, + "interval": 0, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + }) + + mux.HandleFunc("/protocol/openid-connect/token", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + + if r.FormValue("grant_type") == "refresh_token" { + if opts.rejectRefresh { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "invalid_grant"}) + return + } + exp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeTestJWT(exp, "dcm-admin"), + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + "id_token": "new-id-token", + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + return + } + + count := int(pollCount.Add(1)) + if count <= opts.pollsBeforeSuccess { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]string{ + "error": "authorization_pending", + }) + return + } + + exp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeTestJWT(exp, "dcm-admin"), + "refresh_token": "test-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + "id_token": "test-id-token", + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + }) + + mux.HandleFunc("/protocol/openid-connect/revoke", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.WriteHeader(revokeStatus) + }) + + mux.HandleFunc("/protocol/openid-connect/certs", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"keys": []any{}}) + }) + + return mux +} + +func requestBaseURL(r *http.Request) string { + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + return scheme + "://" + r.Host +} + +func makeTestJWT(exp time.Time, username string) string { + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`)) + claims := fmt.Sprintf(`{"exp":%d,"sub":"test-user","preferred_username":"%s"}`, exp.Unix(), username) + payload := base64.RawURLEncoding.EncodeToString([]byte(claims)) + sig := base64.RawURLEncoding.EncodeToString([]byte("fake-signature")) + return header + "." + payload + "." + sig +} diff --git a/internal/commands/policy_test.go b/internal/commands/policy_test.go index ff1fd2d..60feb42 100644 --- a/internal/commands/policy_test.go +++ b/internal/commands/policy_test.go @@ -28,6 +28,8 @@ func clearDCMEnvVars() { "DCM_TLS_CLIENT_CERT", "DCM_TLS_CLIENT_KEY", "DCM_TLS_SKIP_VERIFY", + "DCM_ISSUER_URL", + "DCM_TOKEN", } { Expect(os.Unsetenv(env)).To(Succeed()) } diff --git a/internal/commands/root_test.go b/internal/commands/root_test.go index ea89183..fd428ef 100644 --- a/internal/commands/root_test.go +++ b/internal/commands/root_test.go @@ -30,6 +30,8 @@ var _ = Describe("Root Command", func() { Expect(helpOutput).To(ContainSubstring("sp")) Expect(helpOutput).To(ContainSubstring("version")) Expect(helpOutput).To(ContainSubstring("completion")) + Expect(helpOutput).To(ContainSubstring("login")) + Expect(helpOutput).To(ContainSubstring("logout")) }) }) @@ -93,6 +95,8 @@ var _ = Describe("Root Command", func() { "--tls-client-cert", "--tls-client-key", "--tls-skip-verify", + "--issuer-url", + "--token", } for _, flag := range expectedFlags { Expect(helpOutput).To(ContainSubstring(flag), diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e135f8a..16e84cb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,9 +3,11 @@ package config_test import ( "os" "path/filepath" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.yaml.in/yaml/v3" "github.com/dcm-project/cli/internal/commands" "github.com/dcm-project/cli/internal/config" @@ -18,6 +20,8 @@ func clearDCMEnvVars() { "DCM_OUTPUT_FORMAT", "DCM_TIMEOUT", "DCM_CONFIG", + "DCM_ISSUER_URL", + "DCM_TOKEN", "DCM_TLS_CA_CERT", "DCM_TLS_CLIENT_CERT", "DCM_TLS_CLIENT_KEY", @@ -113,6 +117,8 @@ var _ = Describe("Configuration", func() { Expect(cfg.TLSClientCert).To(BeEmpty()) Expect(cfg.TLSClientKey).To(BeEmpty()) Expect(cfg.TLSSkipVerify).To(BeFalse()) + Expect(cfg.IssuerURL).To(BeEmpty()) + Expect(cfg.Token).To(BeEmpty()) }) }) @@ -164,7 +170,8 @@ var _ = Describe("Configuration", func() { }) Describe("TC-U008: All environment variables", func() { - DescribeTable("should load configuration from each environment variable", + DescribeTable( + "should load configuration from each environment variable", func(envVar, envValue, configField string, expected any) { cfgPath := filepath.Join(GinkgoT().TempDir(), "nonexistent.yaml") GinkgoT().Setenv(envVar, envValue) @@ -193,6 +200,10 @@ var _ = Describe("Configuration", func() { Expect(cfg.TLSClientKey).To(Equal(expected)) case "TLSSkipVerify": Expect(cfg.TLSSkipVerify).To(Equal(expected)) + case "IssuerURL": + Expect(cfg.IssuerURL).To(Equal(expected)) + case "Token": + Expect(cfg.Token).To(Equal(expected)) } }, Entry("DCM_CONTROL_PLANE_URL", "DCM_CONTROL_PLANE_URL", "http://cp:8080", "ControlPlaneURL", "http://cp:8080"), @@ -202,6 +213,191 @@ var _ = Describe("Configuration", func() { Entry("DCM_TLS_CLIENT_CERT", "DCM_TLS_CLIENT_CERT", "/path/cert.pem", "TLSClientCert", "/path/cert.pem"), Entry("DCM_TLS_CLIENT_KEY", "DCM_TLS_CLIENT_KEY", "/path/key.pem", "TLSClientKey", "/path/key.pem"), Entry("DCM_TLS_SKIP_VERIFY", "DCM_TLS_SKIP_VERIFY", "true", "TLSSkipVerify", true), + Entry("DCM_ISSUER_URL", "DCM_ISSUER_URL", "http://keycloak:8080/realms/dcm", "IssuerURL", "http://keycloak:8080/realms/dcm"), + Entry("DCM_TOKEN", "DCM_TOKEN", "static-bearer-token", "Token", "static-bearer-token"), ) }) + + Describe("Auth configuration", func() { + It("should load issuer-url from --issuer-url flag", func() { + cfgPath := filepath.Join(GinkgoT().TempDir(), "nonexistent.yaml") + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{ + "--config", cfgPath, + "--issuer-url", "https://keycloak.example.com/realms/dcm", + "version", + }) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.IssuerURL).To(Equal("https://keycloak.example.com/realms/dcm")) + }) + + It("should load issuer-url from DCM_ISSUER_URL when no flag is set", func() { + cfgPath := filepath.Join(GinkgoT().TempDir(), "nonexistent.yaml") + GinkgoT().Setenv("DCM_ISSUER_URL", "https://keycloak.example.com/realms/dcm") + + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{"--config", cfgPath, "version"}) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.IssuerURL).To(Equal("https://keycloak.example.com/realms/dcm")) + }) + + It("should load issuer-url from config file", func() { + cfgPath := writeConfigFile("issuer-url: https://file.example.com/realms/dcm\n") + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{"--config", cfgPath, "version"}) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.IssuerURL).To(Equal("https://file.example.com/realms/dcm")) + }) + + It("should prefer --issuer-url over env and config file", func() { + cfgPath := writeConfigFile("issuer-url: https://file.example.com/realms/dcm\n") + GinkgoT().Setenv("DCM_ISSUER_URL", "https://env.example.com/realms/dcm") + + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{ + "--config", cfgPath, + "--issuer-url", "https://flag.example.com/realms/dcm", + "version", + }) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.IssuerURL).To(Equal("https://flag.example.com/realms/dcm")) + }) + + It("should prefer DCM_ISSUER_URL over config file", func() { + cfgPath := writeConfigFile("issuer-url: https://file.example.com/realms/dcm\n") + GinkgoT().Setenv("DCM_ISSUER_URL", "https://env.example.com/realms/dcm") + + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{"--config", cfgPath, "version"}) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.IssuerURL).To(Equal("https://env.example.com/realms/dcm")) + }) + + It("should load token from --token flag", func() { + cfgPath := filepath.Join(GinkgoT().TempDir(), "nonexistent.yaml") + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{ + "--config", cfgPath, + "--token", "flag-static-token", + "version", + }) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.Token).To(Equal("flag-static-token")) + }) + + It("should load token from DCM_TOKEN when no flag is set", func() { + cfgPath := filepath.Join(GinkgoT().TempDir(), "nonexistent.yaml") + GinkgoT().Setenv("DCM_TOKEN", "env-static-token") + + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{"--config", cfgPath, "version"}) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.Token).To(Equal("env-static-token")) + }) + + It("should prefer --token over DCM_TOKEN", func() { + cfgPath := filepath.Join(GinkgoT().TempDir(), "nonexistent.yaml") + GinkgoT().Setenv("DCM_TOKEN", "env-static-token") + + cmd := commands.NewRootCommand() + cmd.SetArgs([]string{ + "--config", cfgPath, + "--token", "flag-static-token", + "version", + }) + cmd.SetOut(GinkgoWriter) + cmd.SetErr(GinkgoWriter) + _ = cmd.Execute() + + cfg, err := config.Load(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.Token).To(Equal("flag-static-token")) + }) + + It("should persist issuer-url via SaveConfig under HOME", func() { + home := GinkgoT().TempDir() + GinkgoT().Setenv("HOME", home) + + Expect(config.SaveConfig("", map[string]string{ + "issuer-url": "https://keycloak.example.com/realms/dcm", + })).To(Succeed()) + + data, err := os.ReadFile(filepath.Join(home, ".dcm", "config.yaml")) + Expect(err).NotTo(HaveOccurred()) + Expect(string(data)).To(ContainSubstring("issuer-url: https://keycloak.example.com/realms/dcm")) + }) + + It("should persist via SaveConfig to an explicit path", func() { + dir := GinkgoT().TempDir() + configPath := filepath.Join(dir, "custom", "config.yaml") + + Expect(config.SaveConfig(configPath, map[string]string{ + "issuer-url": "https://keycloak.example.com/realms/dcm", + })).To(Succeed()) + + data, err := os.ReadFile(configPath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(data)).To(ContainSubstring("issuer-url: https://keycloak.example.com/realms/dcm")) + }) + + It("should omit Token when marshalling Config to YAML", func() { + cfg := config.Config{ + ControlPlaneURL: "http://localhost:8080", + IssuerURL: "https://keycloak.example.com/realms/dcm", + Token: "secret-must-not-appear", + } + out, err := yaml.Marshal(cfg) + Expect(err).NotTo(HaveOccurred()) + Expect(string(out)).NotTo(ContainSubstring("secret-must-not-appear")) + Expect(string(out)).NotTo(ContainSubstring("token:")) + }) + + It("should not write token key when SaveConfig is called with other values", func() { + home := GinkgoT().TempDir() + GinkgoT().Setenv("HOME", home) + + Expect(config.SaveConfig("", map[string]string{ + "issuer-url": "https://keycloak.example.com/realms/dcm", + })).To(Succeed()) + + data, err := os.ReadFile(filepath.Join(home, ".dcm", "config.yaml")) + Expect(err).NotTo(HaveOccurred()) + Expect(strings.Contains(string(data), "token:")).To(BeFalse()) + }) + }) }) From c761e3341f6520902e9933d0c471c939a725f442 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Wed, 5 Aug 2026 12:58:00 -0400 Subject: [PATCH 03/14] chore(auth): bump Go deps and sync OIDC auth documentation Update go.mod/go.sum for OIDC and keyring dependencies, and document login/logout, issuer URL, token bypass, and auth architecture in README.md and CLAUDE.md. Signed-off-by: Chad Crum Co-authored-by: Cursor --- CLAUDE.md | 13 ++- README.md | 302 ++++++++++++++++++++++++++++++++++++++++++++++++++---- go.mod | 7 ++ go.sum | 18 ++++ 4 files changed, 317 insertions(+), 23 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 851349e..9bcdff9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,9 +59,16 @@ make test-e2e - Supports table, JSON, and YAML output formats - Implements `Formatter` interface +- **internal/auth/**: OIDC authentication + - `auth.go`: Device Authorization Grant flow (RFC 8628), token revocation + - `token.go`: Token storage (OS keyring primary, file fallback), JWT expiry checking + - `transport.go`: Authenticated HTTP RoundTripper with lazy token loading and refresh + - **internal/commands/**: Cobra command definitions - `root.go`: Root command with global flags - - `helpers.go`: Client constructors, HTTP/TLS helpers, input file parsing + - `helpers.go`: Client constructors, HTTP/TLS helpers, input file parsing, auth transport wiring + - `login.go`: `dcm login` - OIDC device authorization flow + - `logout.go`: `dcm logout` - token revocation and credential cleanup - `policy.go`: Policy CRUD commands - `catalog_service_type.go`: Service type list/get commands - `catalog_item.go`: Catalog item create/list/get/delete commands @@ -89,7 +96,9 @@ E2E tests live under `test/e2e/` and use the `e2e` build tag (`//go:build e2e`). 2. **Generated clients**: Import from `github.com/dcm-project/control-plane/pkg/...` (see links in Project Overview). Client constructors live in `helpers.go`. No hand-written HTTP client code. -3. **Configuration precedence**: CLI flags > environment variables (`DCM_CONTROL_PLANE_URL`, `DCM_OUTPUT_FORMAT`, `DCM_TIMEOUT`, `DCM_CONFIG`) > config file (`~/.dcm/config.yaml`) > built-in defaults. +3. **Configuration precedence**: CLI flags > environment variables (`DCM_CONTROL_PLANE_URL`, `DCM_OUTPUT_FORMAT`, `DCM_TIMEOUT`, `DCM_CONFIG`, `DCM_ISSUER_URL`, `DCM_TOKEN`) > config file (`~/.dcm/config.yaml`) > built-in defaults. + +3a. **Authentication**: When `--issuer-url` is set (or `DCM_ISSUER_URL`), the HTTP client wraps its transport with an `AuthTransport` that injects Bearer tokens. `dcm login` / `dcm logout` use a plain (non-auth) HTTP client for OIDC protocol traffic, with TLS derived from the issuer URL. `dcm login` persists tokens (keyring or `~/.dcm/tokens.json`) and writes `issuer-url` to the active config file (`--config` / `DCM_CONFIG` or `~/.dcm/config.yaml`). `DCM_TOKEN` / `--token` bypasses the OIDC flow with a static Bearer token for CI. Client ID (`dcm-cli`) is hardcoded. 4. **Output formatting**: All commands support `--output/-o` flag with `table` (default), `json`, and `yaml` formats. diff --git a/README.md b/README.md index 58f1dc2..6617a70 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ This specification covers the `v1alpha1` API surface, matching the control-plane | AEP Standards | [aep.dev](https://aep.dev/) - API Enhancement Proposals | | RFC 7807 | Problem Details for HTTP APIs | | RFC 7396 | JSON Merge Patch | +| RFC 8628 | OAuth 2.0 Device Authorization Grant | --- @@ -30,12 +31,21 @@ This specification covers the `v1alpha1` API surface, matching the control-plane │ │ │ control-plane monolith │ │ dcm │─────────────▶│ (port 8080, /api/v1alpha1/*) │ │ CLI │ HTTP / HTTPS │ │ -│ │ │ Policy Manager · Catalog Manager · SP Manager │ +│ │ + Bearer JWT │ Policy Manager · Catalog Manager · SP Manager │ │ │ │ │ └─────────┘ └────────────────────────────────────────────────────┘ + │ + │ OIDC Discovery + │ + Device Auth + ▼ +┌─────────────────┐ +│ Keycloak │ +│ (realm: dcm) │ +│ client: dcm-cli │ +└─────────────────┘ ``` -The CLI communicates exclusively through the control plane (port 8080). When the control plane URL uses an `https://` scheme, the CLI establishes a TLS connection. When the URL uses `http://`, TLS is skipped entirely. These managers run in-process in the monolith (formerly separate services). CLI commands call paths under `/api/v1alpha1`: +The CLI communicates exclusively through the control plane (port 8080). When the control plane URL uses an `https://` scheme, the CLI establishes a TLS connection. When the URL uses `http://`, TLS is skipped entirely. When authentication is configured, the CLI obtains tokens from a Keycloak OIDC provider and injects them as Bearer JWTs in API requests. These managers run in-process in the monolith (formerly separate services). CLI commands call paths under `/api/v1alpha1`: - `/api/v1alpha1/policies/*` → Policy Manager - `/api/v1alpha1/service-types/*` → Catalog Manager @@ -51,10 +61,13 @@ cmd/dcm/ main.go ← Entry point, root command setup internal/ + auth/ ← OIDC authentication (device flow, token storage, transport) config/ ← Configuration loading/saving output/ ← Output formatting (table/json/yaml) commands/ root.go ← Root command, global flags + login.go ← OIDC device authorization login + logout.go ← Token revocation and credential cleanup version.go ← Version command policy.go ← Policy command group catalog_service_type.go ← Catalog service-type command group @@ -70,6 +83,7 @@ internal/ | Component | Responsibility | |-----------|---------------| | `cmd/dcm/main.go` | Bootstrap, wire dependencies, execute root command | +| `internal/auth` | OIDC device flow, token storage (keyring + file), authenticated transport | | `internal/config` | Load config from file/env/flags with precedence | | `internal/output` | Format responses as table, JSON, or YAML | | `internal/commands` | Cobra command definitions, flag binding, client invocation | @@ -87,6 +101,7 @@ Default location: `~/.dcm/config.yaml` ```yaml control-plane-url: http://localhost:8080 +issuer-url: "" output-format: table timeout: 30 tls-ca-cert: "" @@ -95,6 +110,8 @@ tls-client-key: "" tls-skip-verify: false ``` +Note: `dcm login` automatically creates and updates this file (see [Authentication](#13-authentication)). + ### 3.2 Environment Variables | Variable | Description | Default | @@ -103,6 +120,8 @@ tls-skip-verify: false | `DCM_OUTPUT_FORMAT` | Output format (`table`, `json`, `yaml`) | `table` | | `DCM_TIMEOUT` | Request timeout in seconds | `30` | | `DCM_CONFIG` | Path to config file | `~/.dcm/config.yaml` | +| `DCM_ISSUER_URL` | OIDC issuer URL (Keycloak realm URL) | `""` | +| `DCM_TOKEN` | Static Bearer token for CI/scripting (bypasses device flow) | `""` | | `DCM_TLS_CA_CERT` | Path to CA certificate file for TLS verification | `""` | | `DCM_TLS_CLIENT_CERT` | Path to client certificate file for mTLS | `""` | | `DCM_TLS_CLIENT_KEY` | Path to client private key file for mTLS | `""` | @@ -112,8 +131,8 @@ tls-skip-verify: false Configuration values are resolved in the following order (highest to lowest priority): -1. **Command-line flags** (`--control-plane-url`, `--output`, `--timeout`) -2. **Environment variables** (`DCM_CONTROL_PLANE_URL`, etc.) +1. **Command-line flags** (`--control-plane-url`, `--output`, `--timeout`, `--issuer-url`, `--token`) +2. **Environment variables** (`DCM_CONTROL_PLANE_URL`, `DCM_ISSUER_URL`, `DCM_TOKEN`, etc.) 3. **Configuration file** (`~/.dcm/config.yaml`) 4. **Built-in defaults** @@ -130,6 +149,8 @@ These flags are available on all commands: | `--tls-ca-cert` | | Path to CA certificate file for TLS verification | | `--tls-client-cert` | | Path to client certificate file for mTLS | | `--tls-client-key` | | Path to client private key file for mTLS | +| `--issuer-url` | | OIDC issuer URL for authentication | +| `--token` | | Static Bearer token (CI/scripting) | | `--tls-skip-verify` | | Skip TLS certificate verification | --- @@ -140,6 +161,8 @@ These flags are available on all commands: ``` dcm +├── login # OIDC device authorization login +├── logout # Revoke tokens and clear credentials ├── policy # Policy management │ ├── create │ ├── list @@ -168,7 +191,60 @@ dcm └── version # Print version info ``` -### 4.2 Policy Commands +### 4.2 Login Command + +#### `dcm login` + +Authenticate with the DCM control plane using the OIDC Device Authorization Grant (RFC 8628). Initiates a device flow, opens a browser for the user to authenticate, and stores the resulting tokens locally. + +On success, `dcm login` also saves `issuer-url` and `control-plane-url` to the active config file (`--config` / `DCM_CONFIG`, or `~/.dcm/config.yaml`) so subsequent commands work without flags. + +| Flag | Required | Description | +|------|----------|-------------| +| `--issuer-url` | Yes | OIDC issuer URL (e.g., Keycloak realm URL) | +| `--control-plane-url` | No | Control plane URL to persist to config | + +```bash +# Login to a DCM instance +dcm login --issuer-url https://keycloak.example.com/realms/dcm --control-plane-url https://dcm.example.com + +# After first login, issuer-url and control-plane-url are saved to config +dcm policy list +``` + +Example output: + +``` +Open https://keycloak.example.com/realms/dcm/device?user_code=ABCD-EFGH in your browser +Or visit https://keycloak.example.com/realms/dcm/device and enter code: ABCD-EFGH +Logged in as dcm-admin (token expires in 5m0s; auto-refresh enabled) +``` + +Tokens are stored in the OS keychain (macOS Keychain, Linux Secret Service, Windows Credential Manager) when available, falling back to `~/.dcm/tokens.json` (mode `0600`) in environments without keychain support (containers, CI, headless SSH). + +### 4.3 Logout Command + +#### `dcm logout` + +Revoke stored tokens and clear authentication credentials. + +| Flag | Required | Description | +|------|----------|-------------| +| `--issuer-url` | Yes | OIDC issuer URL used during login | + +```bash +dcm logout --issuer-url https://keycloak.example.com/realms/dcm +``` + +Example output: + +``` +Logged out successfully +``` + +If no stored credentials are found, the command prints "No stored credentials found" and exits successfully. Token revocation failures produce a warning but do not cause the command to fail. + +### 4.5 Policy Commands #### `dcm policy create` @@ -302,7 +378,7 @@ Example output: Policy "my-policy" deleted successfully. ``` -### 4.3 Catalog Service-Type Commands +### 4.6 Catalog Service-Type Commands #### `dcm catalog service-type list` @@ -330,7 +406,7 @@ Get a single service type by ID. dcm catalog service-type get SERVICE_TYPE_ID ``` -### 4.4 Catalog Item Commands +### 4.7 Catalog Item Commands #### `dcm catalog item create` @@ -414,7 +490,7 @@ Delete a catalog item by ID. dcm catalog item delete CATALOG_ITEM_ID ``` -### 4.5 Catalog Instance Commands +### 4.8 Catalog Instance Commands #### `dcm catalog instance create` @@ -488,7 +564,7 @@ Delete a catalog item instance by ID. dcm catalog instance delete INSTANCE_ID ``` -### 4.6 SP Resource Commands +### 4.9 SP Resource Commands #### `dcm sp resource list` @@ -527,7 +603,7 @@ dcm sp resource get INSTANCE_ID dcm sp resource get INSTANCE_ID -o yaml ``` -### 4.7 Completion Command +### 4.10 Completion Command #### `dcm completion` @@ -551,7 +627,7 @@ dcm completion fish | source dcm completion powershell | Out-String | Invoke-Expression ``` -### 4.8 Version Command +### 4.11 Version Command #### `dcm version` @@ -583,6 +659,8 @@ package config type Config struct { ControlPlaneURL string `yaml:"control-plane-url" mapstructure:"control-plane-url"` + IssuerURL string `yaml:"issuer-url" mapstructure:"issuer-url"` + Token string `yaml:"-" mapstructure:"token"` OutputFormat string `yaml:"output-format" mapstructure:"output-format"` Timeout int `yaml:"timeout" mapstructure:"timeout"` TLSCACert string `yaml:"tls-ca-cert" mapstructure:"tls-ca-cert"` @@ -594,8 +672,17 @@ type Config struct { // Load reads configuration from file, environment, and flag overrides. func Load() (*Config, error) +// ConfigPath returns the resolved config file path for cmd. +func ConfigPath(cmd *cobra.Command) string + +// SaveConfig merges values into the config file at path (atomic write). +// Empty path writes to ~/.dcm/config.yaml. +func SaveConfig(path string, values map[string]string) error + ``` +Note: `Token` has `yaml:"-"` - it is never persisted to the config file for security. It is only available via the `DCM_TOKEN` env var or `--token` flag. + ### 5.2 `internal/output` Formats API responses for display. @@ -632,6 +719,12 @@ Cobra command definitions. Each file registers its command tree and wires genera // root.go func NewRootCommand() *cobra.Command +// login.go +func newLoginCommand() *cobra.Command // dcm login + +// logout.go +func newLogoutCommand() *cobra.Command // dcm logout + // policy.go func newPolicyCommand() *cobra.Command // parent: dcm policy func newPolicyCreateCommand() *cobra.Command // dcm policy create @@ -671,7 +764,53 @@ func newSPResourceGetCommand() *cobra.Command func newCompletionCommand() *cobra.Command // dcm completion [bash|zsh|fish|powershell] ``` -### 5.4 `internal/version` +### 5.4 `internal/auth` + +OIDC authentication with device authorization flow, token storage, and authenticated HTTP transport. + +```go +package auth + +const ClientID = "dcm-cli" + +// DeviceLogin performs the OAuth 2.0 Device Authorization Grant. +func DeviceLogin(ctx context.Context, issuerURL string, httpClient *http.Client, w io.Writer) (*TokenData, error) + +// RevokeToken revokes the refresh token at the OIDC provider's revocation endpoint. +func RevokeToken(ctx context.Context, issuerURL string, refreshToken string, httpClient *http.Client) error + +// PreferredUsername extracts preferred_username from the JWT payload (no verification). +func PreferredUsername(accessToken string) string + +// TokenData holds the tokens obtained from the OIDC provider. +type TokenData struct { + AccessToken string + RefreshToken string + IDToken string + Expiry time.Time + TokenEndpoint string +} + +// TokenStore persists tokens keyed by issuer URL. +type TokenStore interface { + Save(issuerURL string, data *TokenData) error + Load(issuerURL string) (*TokenData, error) + Delete(issuerURL string) error +} + +// NewTokenStore returns a keyring-backed store, falling back to file if unavailable. +func NewTokenStore() TokenStore + +// AuthTransport is an http.RoundTripper that injects Bearer tokens. +type AuthTransport struct { ... } +``` + +Token lifecycle: +1. **No network**: unverified JWT `exp` decode checks if the access token is still valid +2. **Refresh**: if expired, use the stored refresh token to obtain a new access token +3. **Fail**: if refresh fails, return an actionable error directing the user to `dcm login` + +### 5.5 `internal/version` Build-time version information injected via linker flags. @@ -694,7 +833,7 @@ type Info struct { func Get() Info ``` -### 5.5 Generated Clients (External Dependencies) +### 5.6 Generated Clients (External Dependencies) The CLI imports generated client packages from the control-plane monorepo: @@ -724,10 +863,10 @@ type ClientInterface interface { } ``` -Clients are instantiated with the control-plane URL and a configured HTTP client. When the control-plane URL uses `https://`, the HTTP client is configured with a TLS transport based on the TLS settings (CA cert, client cert/key, skip verify). When the URL uses `http://`, TLS is not configured. +Clients are instantiated with the control-plane URL and a configured HTTP client. When the control-plane URL uses `https://`, the HTTP client is configured with a TLS transport based on the TLS settings (CA cert, client cert/key, skip verify). When the URL uses `http://`, TLS is not configured. When authentication is configured (`issuer-url` or `token`), the base transport is wrapped with `AuthTransport` which lazily injects Bearer tokens. Token refresh reuses that base TLS transport (not `http.DefaultClient`). Login and logout use a plain HTTP client (no AuthTransport) with TLS derived from the issuer URL, so OIDC traffic works when the control plane is HTTP and the issuer is HTTPS with a private CA. ```go -httpClient := buildHTTPClient(cfg) // configures TLS transport when URL is https +httpClient := buildHTTPClient(cfg) // TLS + optional AuthTransport wrapping policyClient, _ := policyclient.NewClient(cfg.ControlPlaneURL + "/api/v1alpha1", policyclient.WithHTTPClient(httpClient)) catalogClient, _ := catalogclient.NewClient(cfg.ControlPlaneURL + "/api/v1alpha1", @@ -749,6 +888,14 @@ dcm-cli/ │ └── dcm/ │ └── main.go ├── internal/ +│ ├── auth/ +│ │ ├── auth.go +│ │ ├── auth_suite_test.go +│ │ ├── auth_test.go +│ │ ├── token.go +│ │ ├── token_test.go +│ │ ├── transport.go +│ │ └── transport_test.go │ ├── config/ │ │ ├── config.go │ │ └── config_test.go @@ -761,6 +908,8 @@ dcm-cli/ │ ├── commands/ │ │ ├── root.go │ │ ├── root_test.go +│ │ ├── login.go +│ │ ├── logout.go │ │ ├── version.go │ │ ├── policy.go │ │ ├── policy_test.go @@ -812,11 +961,18 @@ User invokes command │ ├─▶ Viper resolves config (flags → env → file → defaults) │ - ├─▶ Build HTTP client (configure TLS transport if URL is https://) + ├─▶ Build HTTP client + │ ├─ Configure TLS transport if URL is https:// + │ └─ Wrap with AuthTransport if issuer-url or token is set │ ├─▶ Create generated client with control-plane URL and HTTP client │ ├─▶ Execute API call via generated client + │ │ (AuthTransport lazily injects Bearer token on first request) + │ ├─ Valid cached token → inject, proceed + │ ├─ Expired token → refresh, inject, proceed + │ ├─ Static token (DCM_TOKEN) → inject directly, no refresh + │ └─ No auth configured → no Authorization header │ ├─▶ Check response status │ ├─ Success → format and display response @@ -862,7 +1018,44 @@ dcm policy delete └─▶ Exit 0 ``` -### 7.3 Catalog Ordering Flow +### 7.3 Authentication Flow + +``` +dcm login --issuer-url https://keycloak.example.com/realms/dcm --control-plane-url https://dcm.example.com + │ + ├─▶ OIDC Discovery: GET /.well-known/openid-configuration + ├─▶ Device Auth: POST + ├─▶ Print verification URL and user code to stderr + ├─▶ Open browser to verification_uri_complete (best-effort) + ├─▶ Poll token endpoint until user completes browser auth + ├─▶ Store tokens (access + refresh + ID + expiry + token endpoint) + │ ├─ OS keychain (macOS Keychain, Linux Secret Service, Windows Credential Manager) + │ └─ File fallback: ~/.dcm/tokens.json (mode 0600) + ├─▶ Save issuer-url and control-plane-url to ~/.dcm/config.yaml + ├─▶ Print "Logged in as (token expires in ; auto-refresh enabled)" + └─▶ Exit 0 + +dcm logout --issuer-url https://keycloak.example.com/realms/dcm + │ + ├─▶ Load stored token for issuer URL + ├─▶ POST revocation endpoint with refresh token (warning on failure) + ├─▶ Delete stored token + ├─▶ Print "Logged out successfully" + └─▶ Exit 0 +``` + +**CI/scripting path** (no interactive login): + +```bash +# Obtain token externally (e.g., via client_credentials grant using dcm-proxy client) +export DCM_TOKEN="" +export DCM_CONTROL_PLANE_URL="https://dcm.example.com" + +# All commands use the static token directly - no refresh, no keychain +dcm policy list +``` + +### 7.4 Catalog Ordering Flow ``` dcm catalog instance create --from-file instance.yaml @@ -874,14 +1067,14 @@ dcm catalog instance create --from-file instance.yaml └─▶ Exit 0 ``` -### 7.4 Pagination +### 7.5 Pagination -#### 7.4.1 Page Size +#### 7.5.1 Page Size While `--page-size` is an optional parameter, services may impose a default value. Always check if the response included `next_page_token` -#### 7.4.2 Next Page Token +#### 7.5.2 Next Page Token When a list response includes `next_page_token`, the CLI displays it for manual follow-up: @@ -1070,6 +1263,9 @@ go 1.25.5 | `github.com/spf13/viper` | Configuration management | | `gopkg.in/yaml.v3` | YAML parsing/output | | `github.com/dcm-project/control-plane` | Generated API clients (policy, catalog, SP) | +| `github.com/coreos/go-oidc/v3` | OIDC discovery and provider metadata | +| `golang.org/x/oauth2` | OAuth 2.0 device authorization flow, token refresh | +| `github.com/zalando/go-keyring` | OS keychain access (macOS, Linux, Windows) | | `github.com/onsi/ginkgo/v2` | Test framework (test dependency) | | `github.com/onsi/gomega` | Test matchers (test dependency) | @@ -1100,6 +1296,9 @@ import ( - Command flag parsing and validation - API response handling and error parsing - Input file parsing (YAML/JSON) + - OIDC device flow with mock OIDC server + - Token storage (keyring, file fallback, atomic writes, expiry) + - Authenticated transport (static token, stored token, refresh, passthrough) Example test pattern: @@ -1167,10 +1366,12 @@ make test-e2e # Requires DCM_CONTROL_PLANE_URL pointing to live stack - TLS support with custom CA certificates, client certificates (mTLS), and skip-verify - Shell autocompletion generation (bash, zsh, fish, powershell) - Container image for distribution +- OIDC authentication via Device Authorization Grant (RFC 8628) +- Token storage with OS keychain and file fallback +- Static Bearer token injection for CI/scripting (`DCM_TOKEN`) ### 12.2 Out of Scope (v1alpha1) -- Authentication and authorization (no auth in v1alpha1 control-plane API) - Interactive/wizard-style resource creation - Watch/streaming operations - Plugin/extension system @@ -1178,3 +1379,62 @@ make test-e2e # Requires DCM_CONTROL_PLANE_URL pointing to live stack - Bulk operations - Resource diff/dry-run - Health check command for control-plane connectivity + +--- + +## 13. Authentication + +### 13.1 Overview + +The CLI supports OIDC authentication via the OAuth 2.0 Device Authorization Grant (RFC 8628). When authentication is configured, API requests include a `Bearer` JWT in the `Authorization` header. Authentication is optional - when no issuer URL or token is configured, requests are sent without authentication. + +The Keycloak `dcm` realm provides a public client `dcm-cli` (no client secret) with device authorization enabled. The client ID is hardcoded and not configurable. + +### 13.2 Interactive Login + +For human users, `dcm login` performs the device authorization flow: + +```bash +dcm login --issuer-url https://keycloak.example.com/realms/dcm --control-plane-url https://dcm.example.com +``` + +This discovers the OIDC provider endpoints, initiates a device flow, opens a browser for the user to authenticate, and stores the resulting tokens. On success, `issuer-url` and `control-plane-url` are persisted to `~/.dcm/config.yaml` so subsequent commands work without flags. + +### 13.3 CI/Scripting (Static Token) + +For CI pipelines and automation, use `DCM_TOKEN` to inject a pre-obtained Bearer token directly: + +```bash +export DCM_TOKEN="" +export DCM_CONTROL_PLANE_URL="https://dcm.example.com" +dcm policy list +``` + +The static token path bypasses the device flow entirely - no refresh logic, no keychain, no config file interaction. Tokens can be obtained externally via the `dcm-proxy` confidential Keycloak client using a `client_credentials` grant. + +### 13.4 Token Storage + +Tokens are stored using a two-tier strategy: + +1. **OS keychain** (primary) - macOS Keychain, Linux Secret Service (GNOME Keyring/KDE Wallet), Windows Credential Manager. Service name: `dcm-cli`, key: normalized issuer URL. +2. **File** (fallback) - `~/.dcm/tokens.json` with `0600` permissions. Activated automatically when the keychain is unavailable (containers, CI, headless SSH). + +Both backends use atomic writes (write to `.tmp` then rename) for crash safety. + +### 13.5 Token Lifecycle + +The `AuthTransport` handles token injection lazily during HTTP requests: + +1. **Fast path (no network)**: decode the JWT `exp` claim without verification. If the access token is valid (with 30s clock skew buffer), inject it directly. +2. **Refresh path**: if expired, use the stored refresh token to obtain a new access token from the token endpoint. Save the refreshed tokens. +3. **Failure path**: if refresh fails, return an actionable error: `"authentication expired, run 'dcm login' to re-authenticate"`. + +This design avoids network calls on every CLI invocation and keeps auth decoupled from the command tree (no `PersistentPreRunE` annotations needed). + +### 13.6 Security + +- Tokens are never written to the config file (`Token` has `yaml:"-"`) +- The CLI warns on stderr when sending a Bearer token over unencrypted HTTP +- `TokenData.String()` returns `[REDACTED]` to prevent accidental logging +- Token file permissions are set to `0600` (owner read/write only) +- Atomic file writes prevent partial-write corruption diff --git a/go.mod b/go.mod index 5677942..c2d0b33 100644 --- a/go.mod +++ b/go.mod @@ -3,24 +3,31 @@ module github.com/dcm-project/cli go 1.25.5 require ( + github.com/coreos/go-oidc/v3 v3.20.0 github.com/dcm-project/control-plane v0.0.0-20260617094433-e4374fc25292 github.com/onsi/ginkgo/v2 v2.29.0 github.com/onsi/gomega v1.41.0 github.com/spf13/cobra v1.10.2 github.com/spf13/viper v1.21.0 + github.com/zalando/go-keyring v0.2.6 go.yaml.in/yaml/v3 v3.0.4 + golang.org/x/oauth2 v0.36.0 ) require ( + al.essio.dev/pkg/shellescape v1.5.1 // indirect github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect + github.com/danieljoos/wincred v1.2.2 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/getkin/kin-openapi v0.139.0 // indirect + github.com/go-jose/go-jose/v4 v4.1.4 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-openapi/jsonpointer v0.22.4 // indirect github.com/go-openapi/swag/jsonname v0.25.4 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect + github.com/godbus/dbus/v5 v5.1.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 // indirect github.com/google/uuid v1.6.0 // indirect diff --git a/go.sum b/go.sum index 847b02c..f9639f9 100644 --- a/go.sum +++ b/go.sum @@ -1,10 +1,16 @@ +al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho= +al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890= github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/RaveNoX/go-jsoncommentstrip v1.0.0/go.mod h1:78ihd09MekBnJnxpICcwzCMzGrKSKYe4AqU6PDYYpjk= github.com/apapsch/go-jsonmerge/v2 v2.0.0 h1:axGnT1gRIfimI7gJifB699GoE/oq+F2MU7Dml6nw9rQ= github.com/apapsch/go-jsonmerge/v2 v2.0.0/go.mod h1:lvDnEdqiQrp0O42VQGgmlKpxL1AP2+08jFMw88y4klk= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= +github.com/coreos/go-oidc/v3 v3.20.0 h1:EtE0WIBHk03N+DqGkY4+UONzzZHk7amKt6IyNd7OsZE= +github.com/coreos/go-oidc/v3 v3.20.0/go.mod h1:DYCf24+ncYi+XkIH97GY1+dqoRlbaSI26KVTCI9SrY4= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ0i0= +github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -24,6 +30,8 @@ github.com/gkampitakis/go-diff v1.3.2 h1:Qyn0J9XJSDTgnsgHRdz9Zp24RaJeKMUHg2+PDZZ github.com/gkampitakis/go-diff v1.3.2/go.mod h1:LLgOrpqleQe26cte8s36HTWcTmMEur6OPYerdAAS9tk= github.com/gkampitakis/go-snaps v0.5.15 h1:amyJrvM1D33cPHwVrjo9jQxX8g/7E2wYdZ+01KS3zGE= github.com/gkampitakis/go-snaps v0.5.15/go.mod h1:HNpx/9GoKisdhw9AFOBT1N7DBs9DiHo/hGheFGBZ+mc= +github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz3kA= +github.com/go-jose/go-jose/v4 v4.1.4/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-openapi/jsonpointer v0.22.4 h1:dZtK82WlNpVLDW2jlA1YCiVJFVqkED1MegOUy9kR5T4= @@ -40,10 +48,14 @@ github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9L github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/goccy/go-yaml v1.18.0 h1:8W7wMFS12Pcas7KU+VVkaiCng+kG8QiFeFwzFb+rwuw= github.com/goccy/go-yaml v1.18.0/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA= +github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk= +github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 h1:EwtI+Al+DeppwYX2oXJCETMO23COyaKGP6fHVpkpWpg= github.com/google/pprof v0.0.0-20260402051712-545e8a4df936/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -109,6 +121,8 @@ github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad/go.mod h1:qLr4V1qq6nMqFKkMo8ZTx3f+BZEkzsRUY10Xsm2mwU0= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= @@ -126,12 +140,16 @@ github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65E github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= github.com/woodsbury/decimal128 v1.4.0 h1:xJATj7lLu4f2oObouMt2tgGiElE5gO6mSWUjQsBgUlc= github.com/woodsbury/decimal128 v1.4.0/go.mod h1:BP46FUrVjVhdTbKT+XuQh2xfQaGki9LMIRJSFuh6THU= +github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s= +github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= From c271516f9c549f91f07263b5e42dccd78eabf861 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Wed, 5 Aug 2026 19:08:55 -0400 Subject: [PATCH 04/14] fix(auth): silence lint on AuthTransport test helpers Rename unused Save params and gofumpt-align the AuthTransport literal so CI lint passes. Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/transport_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/auth/transport_test.go b/internal/auth/transport_test.go index ba94a1d..6ca649d 100644 --- a/internal/auth/transport_test.go +++ b/internal/auth/transport_test.go @@ -21,7 +21,7 @@ type failSaveStore struct { err error } -func (s *failSaveStore) Save(issuerURL string, data *auth.TokenData) error { +func (s *failSaveStore) Save(_ string, _ *auth.TokenData) error { return s.err } @@ -268,8 +268,8 @@ var _ = Describe("AuthTransport", func() { Expect(err).NotTo(HaveOccurred()) transport := &auth.AuthTransport{ - Base: http.DefaultTransport, - Store: &failSaveStore{TokenStore: store, err: errors.New("disk full")}, + Base: http.DefaultTransport, + Store: &failSaveStore{TokenStore: store, err: errors.New("disk full")}, IssuerURL: "http://keycloak:8080/realms/dcm", Stderr: tmpFile, } From 0e13be8bbbceb2a11a71da212809998fca831ef4 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Fri, 7 Aug 2026 10:26:10 -0400 Subject: [PATCH 05/14] docs(auth): note login also persists control-plane-url Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9bcdff9..32bca6e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,7 +98,7 @@ E2E tests live under `test/e2e/` and use the `e2e` build tag (`//go:build e2e`). 3. **Configuration precedence**: CLI flags > environment variables (`DCM_CONTROL_PLANE_URL`, `DCM_OUTPUT_FORMAT`, `DCM_TIMEOUT`, `DCM_CONFIG`, `DCM_ISSUER_URL`, `DCM_TOKEN`) > config file (`~/.dcm/config.yaml`) > built-in defaults. -3a. **Authentication**: When `--issuer-url` is set (or `DCM_ISSUER_URL`), the HTTP client wraps its transport with an `AuthTransport` that injects Bearer tokens. `dcm login` / `dcm logout` use a plain (non-auth) HTTP client for OIDC protocol traffic, with TLS derived from the issuer URL. `dcm login` persists tokens (keyring or `~/.dcm/tokens.json`) and writes `issuer-url` to the active config file (`--config` / `DCM_CONFIG` or `~/.dcm/config.yaml`). `DCM_TOKEN` / `--token` bypasses the OIDC flow with a static Bearer token for CI. Client ID (`dcm-cli`) is hardcoded. +3a. **Authentication**: When `--issuer-url` is set (or `DCM_ISSUER_URL`), the HTTP client wraps its transport with an `AuthTransport` that injects Bearer tokens. `dcm login` / `dcm logout` use a plain (non-auth) HTTP client for OIDC protocol traffic, with TLS derived from the issuer URL. `dcm login` persists tokens (keyring or `~/.dcm/tokens.json`) and writes `issuer-url` and `control-plane-url` to the active config file (`--config` / `DCM_CONFIG` or `~/.dcm/config.yaml`). `DCM_TOKEN` / `--token` bypasses the OIDC flow with a static Bearer token for CI. Client ID (`dcm-cli`) is hardcoded. 4. **Output formatting**: All commands support `--output/-o` flag with `table` (default), `json`, and `yaml` formats. From a03b5c96b7715424fdbefacf292500b1d58fb63c Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Fri, 7 Aug 2026 13:57:35 -0400 Subject: [PATCH 06/14] docs(auth): shrink internal/auth API dump in README Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- README.md | 44 +------------------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/README.md b/README.md index 6617a70..94faf12 100644 --- a/README.md +++ b/README.md @@ -766,49 +766,7 @@ func newCompletionCommand() *cobra.Command // dcm completion [bash| ### 5.4 `internal/auth` -OIDC authentication with device authorization flow, token storage, and authenticated HTTP transport. - -```go -package auth - -const ClientID = "dcm-cli" - -// DeviceLogin performs the OAuth 2.0 Device Authorization Grant. -func DeviceLogin(ctx context.Context, issuerURL string, httpClient *http.Client, w io.Writer) (*TokenData, error) - -// RevokeToken revokes the refresh token at the OIDC provider's revocation endpoint. -func RevokeToken(ctx context.Context, issuerURL string, refreshToken string, httpClient *http.Client) error - -// PreferredUsername extracts preferred_username from the JWT payload (no verification). -func PreferredUsername(accessToken string) string - -// TokenData holds the tokens obtained from the OIDC provider. -type TokenData struct { - AccessToken string - RefreshToken string - IDToken string - Expiry time.Time - TokenEndpoint string -} - -// TokenStore persists tokens keyed by issuer URL. -type TokenStore interface { - Save(issuerURL string, data *TokenData) error - Load(issuerURL string) (*TokenData, error) - Delete(issuerURL string) error -} - -// NewTokenStore returns a keyring-backed store, falling back to file if unavailable. -func NewTokenStore() TokenStore - -// AuthTransport is an http.RoundTripper that injects Bearer tokens. -type AuthTransport struct { ... } -``` - -Token lifecycle: -1. **No network**: unverified JWT `exp` decode checks if the access token is still valid -2. **Refresh**: if expired, use the stored refresh token to obtain a new access token -3. **Fail**: if refresh fails, return an actionable error directing the user to `dcm login` +OIDC device authorization, token storage (keyring with file fallback), and `AuthTransport` for Bearer injection/refresh. Public entry points: `DeviceLogin`, `RevokeToken`, `NewTokenStore`, `AuthTransport`. Behavior and usage are covered in [§13 Authentication](#13-authentication). ### 5.5 `internal/version` From 8c8fb3e8578fc1361b9f053cab080416f7fdfe66 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Fri, 7 Aug 2026 13:59:57 -0400 Subject: [PATCH 07/14] docs(auth): clarify atomic writes apply only to file token store Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 94faf12..351ec6f 100644 --- a/README.md +++ b/README.md @@ -1377,7 +1377,7 @@ Tokens are stored using a two-tier strategy: 1. **OS keychain** (primary) - macOS Keychain, Linux Secret Service (GNOME Keyring/KDE Wallet), Windows Credential Manager. Service name: `dcm-cli`, key: normalized issuer URL. 2. **File** (fallback) - `~/.dcm/tokens.json` with `0600` permissions. Activated automatically when the keychain is unavailable (containers, CI, headless SSH). -Both backends use atomic writes (write to `.tmp` then rename) for crash safety. +The file backend uses atomic writes (write to `.tmp` then rename) for crash safety. The keyring backend delegates to the OS keychain API (`keyring.Set`). ### 13.5 Token Lifecycle From 73827c24d09b4c093f17c6ba6fdc081e32529384 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Mon, 10 Aug 2026 10:15:42 -0400 Subject: [PATCH 08/14] fix(auth): avoid Windows cmd injection when opening device URL Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/auth.go | 35 ++++++++++++++++++++++++------ internal/auth/auth_test.go | 42 ++++++++++++++++++++++++++++++++++++ internal/auth/export_test.go | 7 ++++++ 3 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 internal/auth/export_test.go diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 97484b4..758f359 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -147,15 +147,36 @@ func PreferredUsername(accessToken string) string { return claims.PreferredUsername } -func openBrowser(url string) error { - var cmd *exec.Cmd - switch runtime.GOOS { +func openBrowser(rawURL string) error { + cmd, err := browserCommand(rawURL) + if err != nil { + return err + } + return cmd.Start() +} + +// browserCommand builds the OS-specific command used to open a URL. +// Only http and https schemes are allowed. On Windows it uses rundll32 +// instead of cmd.exe to avoid shell metacharacter injection. +func browserCommand(rawURL string) (*exec.Cmd, error) { + return browserCommandFor(runtime.GOOS, rawURL) +} + +func browserCommandFor(goos, rawURL string) (*exec.Cmd, error) { + u, err := url.Parse(rawURL) + if err != nil { + return nil, fmt.Errorf("invalid browser URL: %w", err) + } + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("unsupported browser URL scheme %q", u.Scheme) + } + + switch goos { case "darwin": - cmd = exec.Command("open", url) + return exec.Command("open", rawURL), nil case "windows": - cmd = exec.Command("cmd", "/c", "start", url) + return exec.Command("rundll32", "url.dll,FileProtocolHandler", rawURL), nil default: - cmd = exec.Command("xdg-open", url) + return exec.Command("xdg-open", rawURL), nil } - return cmd.Start() } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 37bd2e2..9567cbf 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -180,6 +180,48 @@ var _ = Describe("ClientID", func() { }) }) +var _ = Describe("browserCommand", func() { + It("accepts http and https URLs", func() { + for _, raw := range []string{ + "https://keycloak.example.com/device", + "http://localhost:8080/device?user_code=ABCD", + } { + cmd, err := auth.BrowserCommand(raw) + Expect(err).NotTo(HaveOccurred(), raw) + Expect(cmd).NotTo(BeNil()) + Expect(cmd.Args).To(ContainElement(raw)) + } + }) + + It("rejects non-http schemes", func() { + for _, raw := range []string{ + "file:///etc/passwd", + "javascript:alert(1)", + "cmd://calc", + } { + _, err := auth.BrowserCommand(raw) + Expect(err).To(HaveOccurred(), raw) + Expect(err.Error()).To(ContainSubstring("unsupported browser URL scheme")) + } + }) + + It("rejects URLs without an http(s) scheme", func() { + _, err := auth.BrowserCommand("not a url") + Expect(err).To(HaveOccurred()) + }) + + It("uses rundll32 on Windows so cmd.exe does not parse the URL", func() { + raw := `https://evil.example/x&calc` + cmd, err := auth.BrowserCommandFor("windows", raw) + Expect(err).NotTo(HaveOccurred()) + Expect(cmd.Args).To(Equal([]string{ + "rundll32", + "url.dll,FileProtocolHandler", + raw, + })) + }) +}) + var _ = Describe("PreferredUsername", func() { It("extracts preferred_username from a valid JWT", func() { token := makeJWTWithUsername(time.Now().Add(5*time.Minute), "dcm-admin") diff --git a/internal/auth/export_test.go b/internal/auth/export_test.go new file mode 100644 index 0000000..a8a386c --- /dev/null +++ b/internal/auth/export_test.go @@ -0,0 +1,7 @@ +package auth + +// Test hooks for browserCommand (export_test.go pattern). +var ( + BrowserCommand = browserCommand + BrowserCommandFor = browserCommandFor +) From c893f86f07b9cd3871d9f3a0ae4d30d896a0c71c Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Mon, 10 Aug 2026 10:28:12 -0400 Subject: [PATCH 09/14] refactor(auth): remove unused TokenData.MarshalLog Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/token.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/auth/token.go b/internal/auth/token.go index 02d0ffa..8f8a2e9 100644 --- a/internal/auth/token.go +++ b/internal/auth/token.go @@ -27,10 +27,6 @@ func (t *TokenData) String() string { return "[REDACTED]" } -func (t *TokenData) MarshalLog() string { - return "[REDACTED]" -} - // IsExpired checks whether the access token has expired. It prefers the // unverified JWT exp claim, falling back to TokenData.Expiry for opaque // tokens. The clockSkew parameter provides a buffer for clock differences. From f9fd000e8fcce7e30b45f2e049d2b5468abfcfe6 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Mon, 10 Aug 2026 10:32:10 -0400 Subject: [PATCH 10/14] fix(auth): fail when token file store cannot resolve home Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/export_test.go | 17 ++++++++++++++++- internal/auth/token.go | 22 ++++++++++++++++------ internal/auth/token_test.go | 28 ++++++++++++++++++++++++++++ internal/commands/helpers.go | 5 ++++- internal/commands/login.go | 5 ++++- internal/commands/login_test.go | 8 +++++--- internal/commands/logout.go | 5 ++++- internal/commands/logout_test.go | 10 ++++++---- 8 files changed, 83 insertions(+), 17 deletions(-) diff --git a/internal/auth/export_test.go b/internal/auth/export_test.go index a8a386c..d5331ab 100644 --- a/internal/auth/export_test.go +++ b/internal/auth/export_test.go @@ -1,7 +1,22 @@ package auth -// Test hooks for browserCommand (export_test.go pattern). +import "os" + +// Test hooks (export_test.go pattern). var ( BrowserCommand = browserCommand BrowserCommandFor = browserCommandFor + NewFileStore = newFileStore ) + +// SetUserHomeDir overrides os.UserHomeDir for tests. Call the returned +// restore function to reset. +func SetUserHomeDir(fn func() (string, error)) func() { + prev := userHomeDir + userHomeDir = fn + return func() { userHomeDir = prev } +} + +func ResetUserHomeDir() { + userHomeDir = os.UserHomeDir +} diff --git a/internal/auth/token.go b/internal/auth/token.go index 8f8a2e9..c04d244 100644 --- a/internal/auth/token.go +++ b/internal/auth/token.go @@ -76,13 +76,14 @@ type TokenStore interface { } // NewTokenStore returns a TokenStore backed by the OS keyring if available, -// falling back to a file-based store otherwise. -func NewTokenStore() TokenStore { +// falling back to a file-based store otherwise. It returns an error if the +// file fallback is required and the home directory cannot be resolved. +func NewTokenStore() (TokenStore, error) { if err := keyring.Set(keyringService, "__probe__", "probe"); err != nil { return newFileStore() } _ = keyring.Delete(keyringService, "__probe__") - return &keyringStore{} + return &keyringStore{}, nil } // normalizeIssuer strips trailing slashes from the issuer URL for use as @@ -130,9 +131,18 @@ type fileStore struct { dir string } -func newFileStore() *fileStore { - home, _ := os.UserHomeDir() - return &fileStore{dir: filepath.Join(home, ".dcm")} +// userHomeDir is os.UserHomeDir by default; tests may override it. +var userHomeDir = os.UserHomeDir + +func newFileStore() (*fileStore, error) { + home, err := userHomeDir() + if err != nil { + return nil, fmt.Errorf("resolving home directory for token store: %w", err) + } + if home == "" { + return nil, fmt.Errorf("resolving home directory for token store: empty home") + } + return &fileStore{dir: filepath.Join(home, ".dcm")}, nil } func (s *fileStore) path() string { diff --git a/internal/auth/token_test.go b/internal/auth/token_test.go index 0e1a804..ca2e91f 100644 --- a/internal/auth/token_test.go +++ b/internal/auth/token_test.go @@ -270,6 +270,34 @@ var _ = Describe("FileStore", func() { }) }) +var _ = Describe("newFileStore", func() { + AfterEach(func() { + auth.ResetUserHomeDir() + }) + + It("returns an error when the home directory cannot be resolved", func() { + restore := auth.SetUserHomeDir(func() (string, error) { + return "", fmt.Errorf("no home") + }) + defer restore() + + _, err := auth.NewFileStore() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resolving home directory for token store")) + }) + + It("returns an error when the home directory is empty", func() { + restore := auth.SetUserHomeDir(func() (string, error) { + return "", nil + }) + defer restore() + + _, err := auth.NewFileStore() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("empty home")) + }) +}) + var _ = Describe("SaveConfig integration", func() { It("creates config file when it does not exist", func() { dir := GinkgoT().TempDir() diff --git a/internal/commands/helpers.go b/internal/commands/helpers.go index 01b0be1..a1fcda1 100644 --- a/internal/commands/helpers.go +++ b/internal/commands/helpers.go @@ -78,7 +78,10 @@ func buildHTTPClient(cfg *config.Config) (*http.Client, error) { if cfg.IssuerURL != "" || cfg.Token != "" { var store auth.TokenStore if cfg.Token == "" { - store = auth.NewTokenStore() + store, err = auth.NewTokenStore() + if err != nil { + return nil, fmt.Errorf("initializing credential store: %w", err) + } } transport := &auth.AuthTransport{ Base: baseTransport, diff --git a/internal/commands/login.go b/internal/commands/login.go index 19c6502..4bf3587 100644 --- a/internal/commands/login.go +++ b/internal/commands/login.go @@ -36,7 +36,10 @@ func newLoginCommand() *cobra.Command { return err } - store := auth.NewTokenStore() + store, err := auth.NewTokenStore() + if err != nil { + return fmt.Errorf("initializing credential store: %w", err) + } if err := store.Save(cfg.IssuerURL, tokenData); err != nil { return fmt.Errorf("saving credentials: %w", err) } diff --git a/internal/commands/login_test.go b/internal/commands/login_test.go index 844feef..b3310a0 100644 --- a/internal/commands/login_test.go +++ b/internal/commands/login_test.go @@ -64,7 +64,8 @@ var _ = Describe("login command", func() { Expect(errBuf.String()).To(ContainSubstring("Logged in as dcm-admin")) Expect(errBuf.String()).To(ContainSubstring("auto-refresh enabled")) - store := auth.NewTokenStore() + store, err := auth.NewTokenStore() + Expect(err).NotTo(HaveOccurred()) td, err := store.Load(server.URL) Expect(err).NotTo(HaveOccurred()) Expect(td).NotTo(BeNil()) @@ -88,7 +89,8 @@ var _ = Describe("login command", func() { }) defer server.Close() - store := auth.NewTokenStore() + store, err := auth.NewTokenStore() + Expect(err).NotTo(HaveOccurred()) Expect(store.Save(server.URL, &auth.TokenData{ AccessToken: makeTestJWT(time.Now().Add(-time.Hour), "stale-user"), RefreshToken: "invalid-refresh", @@ -107,7 +109,7 @@ var _ = Describe("login command", func() { "login", }) - err := cmd.Execute() + err = cmd.Execute() Expect(err).NotTo(HaveOccurred()) Expect(errBuf.String()).To(ContainSubstring("Logged in as dcm-admin")) Expect(errBuf.String()).To(ContainSubstring("auto-refresh enabled")) diff --git a/internal/commands/logout.go b/internal/commands/logout.go index f8cb377..b9b07d8 100644 --- a/internal/commands/logout.go +++ b/internal/commands/logout.go @@ -19,7 +19,10 @@ func newLogoutCommand() *cobra.Command { return &UsageError{Err: fmt.Errorf("--issuer-url is required (or set DCM_ISSUER_URL)")} } - store := auth.NewTokenStore() + store, err := auth.NewTokenStore() + if err != nil { + return fmt.Errorf("initializing credential store: %w", err) + } tokenData, err := store.Load(cfg.IssuerURL) if err != nil { return fmt.Errorf("reading stored credentials: %w", err) diff --git a/internal/commands/logout_test.go b/internal/commands/logout_test.go index d54c12e..c068735 100644 --- a/internal/commands/logout_test.go +++ b/internal/commands/logout_test.go @@ -52,7 +52,8 @@ var _ = Describe("logout command", func() { server := mockOIDCServer(mockOIDCOptions{}) defer server.Close() - store := auth.NewTokenStore() + store, err := auth.NewTokenStore() + Expect(err).NotTo(HaveOccurred()) td := &auth.TokenData{ AccessToken: makeTestJWT(time.Now().Add(5*time.Minute), "dcm-admin"), RefreshToken: "test-refresh-token", @@ -72,7 +73,7 @@ var _ = Describe("logout command", func() { "logout", }) - err := cmd.Execute() + err = cmd.Execute() Expect(err).NotTo(HaveOccurred()) Expect(errBuf.String()).To(ContainSubstring("Logged out successfully")) @@ -85,7 +86,8 @@ var _ = Describe("logout command", func() { server := mockOIDCServer(mockOIDCOptions{revokeStatus: 500}) defer server.Close() - store := auth.NewTokenStore() + store, err := auth.NewTokenStore() + Expect(err).NotTo(HaveOccurred()) td := &auth.TokenData{ AccessToken: makeTestJWT(time.Now().Add(5*time.Minute), "dcm-admin"), RefreshToken: "test-refresh-token", @@ -105,7 +107,7 @@ var _ = Describe("logout command", func() { "logout", }) - err := cmd.Execute() + err = cmd.Execute() Expect(err).NotTo(HaveOccurred()) Expect(errBuf.String()).To(ContainSubstring("Warning: token revocation failed")) Expect(errBuf.String()).To(ContainSubstring("Logged out successfully")) From 0152e509b658c47ac749a6e1ef011cdf02c7e6d1 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Mon, 10 Aug 2026 10:39:11 -0400 Subject: [PATCH 11/14] fix(auth): use issuer TLS transport for token refresh Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- README.md | 2 +- internal/auth/transport.go | 32 +++++++++++++------- internal/auth/transport_test.go | 53 +++++++++++++++++++++++++++++++++ internal/commands/helpers.go | 16 +++++++--- 4 files changed, 88 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 351ec6f..7f3dabd 100644 --- a/README.md +++ b/README.md @@ -821,7 +821,7 @@ type ClientInterface interface { } ``` -Clients are instantiated with the control-plane URL and a configured HTTP client. When the control-plane URL uses `https://`, the HTTP client is configured with a TLS transport based on the TLS settings (CA cert, client cert/key, skip verify). When the URL uses `http://`, TLS is not configured. When authentication is configured (`issuer-url` or `token`), the base transport is wrapped with `AuthTransport` which lazily injects Bearer tokens. Token refresh reuses that base TLS transport (not `http.DefaultClient`). Login and logout use a plain HTTP client (no AuthTransport) with TLS derived from the issuer URL, so OIDC traffic works when the control plane is HTTP and the issuer is HTTPS with a private CA. +Clients are instantiated with the control-plane URL and a configured HTTP client. When the control-plane URL uses `https://`, the HTTP client is configured with a TLS transport based on the TLS settings (CA cert, client cert/key, skip verify). When the URL uses `http://`, TLS is not configured. When authentication is configured (`issuer-url` or `token`), the base transport is wrapped with `AuthTransport` which lazily injects Bearer tokens. Token refresh uses a separate transport derived from the issuer URL (falling back to the control-plane base transport), so OIDC refresh works when the control plane is HTTP and the issuer is HTTPS with a private CA. Login and logout use a plain HTTP client (no AuthTransport) with TLS derived from the issuer URL for the same reason. ```go httpClient := buildHTTPClient(cfg) // TLS + optional AuthTransport wrapping diff --git a/internal/auth/transport.go b/internal/auth/transport.go index 5ee27de..7ef6f8b 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -20,13 +20,17 @@ const clockSkew = 30 * time.Second // - Stored token: loaded from a TokenStore, with automatic refresh when // the access token expires. type AuthTransport struct { - Base http.RoundTripper - Store TokenStore - IssuerURL string - StaticToken string - Stderr *os.File - mu sync.Mutex - warnOnce sync.Once + Base http.RoundTripper + // RefreshTransport is used for OIDC token-endpoint calls during refresh. + // When nil, Base is used. Set this when the issuer URL needs different + // TLS settings than the control-plane URL (e.g. HTTP CP + HTTPS issuer). + RefreshTransport http.RoundTripper + Store TokenStore + IssuerURL string + StaticToken string + Stderr *os.File + mu sync.Mutex + warnOnce sync.Once } func (t *AuthTransport) RoundTrip(req *http.Request) (*http.Response, error) { @@ -99,9 +103,10 @@ func (t *AuthTransport) refreshToken(ctx context.Context, tokenData *TokenData) RefreshToken: current.RefreshToken, } - // Use the same TLS-capable transport as API calls. Do not wrap with - // AuthTransport — that would re-enter RoundTrip while holding t.mu. - refreshClient := &http.Client{Transport: t.base()} + // Prefer RefreshTransport when set so issuer TLS (custom CA, mTLS) is + // used even if Base was built for an HTTP control-plane URL. Do not wrap + // with AuthTransport — that would re-enter RoundTrip while holding t.mu. + refreshClient := &http.Client{Transport: t.refreshBase()} refreshCtx := context.WithValue(ctx, oauth2.HTTPClient, refreshClient) newToken, err := oauthCfg.TokenSource(refreshCtx, oldToken).Token() @@ -156,6 +161,13 @@ func (t *AuthTransport) base() http.RoundTripper { return http.DefaultTransport } +func (t *AuthTransport) refreshBase() http.RoundTripper { + if t.RefreshTransport != nil { + return t.RefreshTransport + } + return t.base() +} + func cloneRequest(req *http.Request) *http.Request { r2 := req.Clone(req.Context()) return r2 diff --git a/internal/auth/transport_test.go b/internal/auth/transport_test.go index 6ca649d..cd3b8a2 100644 --- a/internal/auth/transport_test.go +++ b/internal/auth/transport_test.go @@ -235,6 +235,59 @@ var _ = Describe("AuthTransport", func() { _ = resp.Body.Close() Expect(hits.Load()).To(BeNumerically(">=", 1)) }) + + It("prefers RefreshTransport over Base for token refresh", func() { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.FormValue("grant_type") == "refresh_token" { + newExp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeJWT(newExp), + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + return + } + http.Error(w, "unexpected request", http.StatusBadRequest) + })) + defer tokenServer.Close() + + expiredTD := &auth.TokenData{ + AccessToken: makeJWT(time.Now().Add(-1 * time.Minute)), + RefreshToken: "old-refresh-token", + Expiry: time.Now().Add(-1 * time.Minute), + TokenEndpoint: tokenServer.URL, + } + Expect(store.Save("http://keycloak:8080/realms/dcm", expiredTD)).To(Succeed()) + + tokenHost := strings.TrimPrefix(strings.TrimPrefix(tokenServer.URL, "https://"), "http://") + var baseHits, refreshHits atomic.Int32 + base := &countingRoundTripper{ + base: http.DefaultTransport, + hits: &baseHits, + matchHost: tokenHost, + } + refresh := &countingRoundTripper{ + base: http.DefaultTransport, + hits: &refreshHits, + matchHost: tokenHost, + } + transport := &auth.AuthTransport{ + Base: base, + RefreshTransport: refresh, + Store: store, + IssuerURL: "http://keycloak:8080/realms/dcm", + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + Expect(refreshHits.Load()).To(BeNumerically(">=", 1)) + Expect(baseHits.Load()).To(BeZero()) + }) }) Describe("Refresh persist failure", func() { diff --git a/internal/commands/helpers.go b/internal/commands/helpers.go index a1fcda1..6742b0a 100644 --- a/internal/commands/helpers.go +++ b/internal/commands/helpers.go @@ -83,11 +83,19 @@ func buildHTTPClient(cfg *config.Config) (*http.Client, error) { return nil, fmt.Errorf("initializing credential store: %w", err) } } + var refreshTransport http.RoundTripper + if cfg.IssuerURL != "" { + refreshTransport, err = tlsTransportForURL(cfg, cfg.IssuerURL) + if err != nil { + return nil, err + } + } transport := &auth.AuthTransport{ - Base: baseTransport, - Store: store, - IssuerURL: cfg.IssuerURL, - StaticToken: cfg.Token, + Base: baseTransport, + RefreshTransport: refreshTransport, + Store: store, + IssuerURL: cfg.IssuerURL, + StaticToken: cfg.Token, } return &http.Client{Transport: transport}, nil } From f13622454586a3e3549771a6731f5c884fc91706 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Mon, 10 Aug 2026 10:46:21 -0400 Subject: [PATCH 12/14] fix(auth): persist TokenEndpoint from reloaded token on refresh Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/transport.go | 2 +- internal/auth/transport_test.go | 71 +++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/internal/auth/transport.go b/internal/auth/transport.go index 7ef6f8b..3a21211 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -120,7 +120,7 @@ func (t *AuthTransport) refreshToken(ctx context.Context, tokenData *TokenData) RefreshToken: newToken.RefreshToken, IDToken: idToken, Expiry: newToken.Expiry, - TokenEndpoint: tokenData.TokenEndpoint, + TokenEndpoint: current.TokenEndpoint, } // Prefer returning the refreshed token even if persist fails. With refresh diff --git a/internal/auth/transport_test.go b/internal/auth/transport_test.go index cd3b8a2..cf89629 100644 --- a/internal/auth/transport_test.go +++ b/internal/auth/transport_test.go @@ -25,6 +25,28 @@ func (s *failSaveStore) Save(_ string, _ *auth.TokenData) error { return s.err } +// reloadEndpointStore returns staleEndpoint on the first Load and +// currentEndpoint on later Loads, simulating another refresh updating +// the store between the outer RoundTrip load and the locked reload. +type reloadEndpointStore struct { + auth.TokenStore + loads atomic.Int32 + staleEndpoint string + currentEndpoint string + baseTD *auth.TokenData +} + +func (s *reloadEndpointStore) Load(issuerURL string) (*auth.TokenData, error) { + n := s.loads.Add(1) + td := *s.baseTD + if n == 1 { + td.TokenEndpoint = s.staleEndpoint + } else { + td.TokenEndpoint = s.currentEndpoint + } + return &td, nil +} + type countingRoundTripper struct { base http.RoundTripper hits *atomic.Int32 @@ -158,6 +180,55 @@ var _ = Describe("AuthTransport", func() { Expect(err).NotTo(HaveOccurred()) Expect(reloaded.RefreshToken).To(Equal("new-refresh-token")) }) + + It("persists TokenEndpoint from the reloaded token under the lock", func() { + currentEndpoint := "" + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.FormValue("grant_type") == "refresh_token" { + newExp := time.Now().Add(5 * time.Minute) + resp := map[string]any{ + "access_token": makeJWT(newExp), + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 300, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + return + } + http.Error(w, "unexpected request", http.StatusBadRequest) + })) + defer tokenServer.Close() + currentEndpoint = tokenServer.URL + + baseTD := &auth.TokenData{ + AccessToken: makeJWT(time.Now().Add(-1 * time.Minute)), + RefreshToken: "old-refresh-token", + Expiry: time.Now().Add(-1 * time.Minute), + } + wrapped := &reloadEndpointStore{ + TokenStore: store, + staleEndpoint: "http://stale.example/token", + currentEndpoint: currentEndpoint, + baseTD: baseTD, + } + + transport := &auth.AuthTransport{ + Base: http.DefaultTransport, + Store: wrapped, + IssuerURL: "http://keycloak:8080/realms/dcm", + } + client := &http.Client{Transport: transport} + + resp, err := client.Get(backend.URL) + Expect(err).NotTo(HaveOccurred()) + _ = resp.Body.Close() + + saved, err := store.Load("http://keycloak:8080/realms/dcm") + Expect(err).NotTo(HaveOccurred()) + Expect(saved).NotTo(BeNil()) + Expect(saved.TokenEndpoint).To(Equal(currentEndpoint)) + }) }) Describe("Expired token with failed refresh", func() { From e7f3c66ebbe7e867a8048d3c5200dcb7365b9120 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Mon, 10 Aug 2026 10:57:40 -0400 Subject: [PATCH 13/14] fix(auth): persist control-plane-url on login only when explicitly set Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- CLAUDE.md | 2 +- README.md | 8 ++--- internal/commands/login.go | 13 +++++++- internal/commands/login_test.go | 55 +++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 32bca6e..276d2f5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,7 +98,7 @@ E2E tests live under `test/e2e/` and use the `e2e` build tag (`//go:build e2e`). 3. **Configuration precedence**: CLI flags > environment variables (`DCM_CONTROL_PLANE_URL`, `DCM_OUTPUT_FORMAT`, `DCM_TIMEOUT`, `DCM_CONFIG`, `DCM_ISSUER_URL`, `DCM_TOKEN`) > config file (`~/.dcm/config.yaml`) > built-in defaults. -3a. **Authentication**: When `--issuer-url` is set (or `DCM_ISSUER_URL`), the HTTP client wraps its transport with an `AuthTransport` that injects Bearer tokens. `dcm login` / `dcm logout` use a plain (non-auth) HTTP client for OIDC protocol traffic, with TLS derived from the issuer URL. `dcm login` persists tokens (keyring or `~/.dcm/tokens.json`) and writes `issuer-url` and `control-plane-url` to the active config file (`--config` / `DCM_CONFIG` or `~/.dcm/config.yaml`). `DCM_TOKEN` / `--token` bypasses the OIDC flow with a static Bearer token for CI. Client ID (`dcm-cli`) is hardcoded. +3a. **Authentication**: When `--issuer-url` is set (or `DCM_ISSUER_URL`), the HTTP client wraps its transport with an `AuthTransport` that injects Bearer tokens. `dcm login` / `dcm logout` use a plain (non-auth) HTTP client for OIDC protocol traffic, with TLS derived from the issuer URL. `dcm login` persists tokens (keyring or `~/.dcm/tokens.json`) and writes `issuer-url` to the active config file (`--config` / `DCM_CONFIG` or `~/.dcm/config.yaml`); `control-plane-url` is written only when explicitly set via `--control-plane-url` or `DCM_CONTROL_PLANE_URL`. `DCM_TOKEN` / `--token` bypasses the OIDC flow with a static Bearer token for CI. Client ID (`dcm-cli`) is hardcoded. 4. **Output formatting**: All commands support `--output/-o` flag with `table` (default), `json`, and `yaml` formats. diff --git a/README.md b/README.md index 7f3dabd..77671e6 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ dcm Authenticate with the DCM control plane using the OIDC Device Authorization Grant (RFC 8628). Initiates a device flow, opens a browser for the user to authenticate, and stores the resulting tokens locally. -On success, `dcm login` also saves `issuer-url` and `control-plane-url` to the active config file (`--config` / `DCM_CONFIG`, or `~/.dcm/config.yaml`) so subsequent commands work without flags. +On success, `dcm login` saves `issuer-url` to the active config file (`--config` / `DCM_CONFIG`, or `~/.dcm/config.yaml`). It also saves `control-plane-url` when that value was explicitly provided via `--control-plane-url` or `DCM_CONTROL_PLANE_URL` (the built-in default alone is not written). | Flag | Required | Description | |------|----------|-------------| @@ -208,7 +208,7 @@ On success, `dcm login` also saves `issuer-url` and `control-plane-url` to the a # Login to a DCM instance dcm login --issuer-url https://keycloak.example.com/realms/dcm --control-plane-url https://dcm.example.com -# After first login, issuer-url and control-plane-url are saved to config +# After first login, issuer-url (and control-plane-url when set) are saved to config dcm policy list ``` @@ -989,7 +989,7 @@ dcm login --issuer-url https://keycloak.example.com/realms/dcm --control-plane-u ├─▶ Store tokens (access + refresh + ID + expiry + token endpoint) │ ├─ OS keychain (macOS Keychain, Linux Secret Service, Windows Credential Manager) │ └─ File fallback: ~/.dcm/tokens.json (mode 0600) - ├─▶ Save issuer-url and control-plane-url to ~/.dcm/config.yaml + ├─▶ Save issuer-url (and control-plane-url when explicitly set) to ~/.dcm/config.yaml ├─▶ Print "Logged in as (token expires in ; auto-refresh enabled)" └─▶ Exit 0 @@ -1356,7 +1356,7 @@ For human users, `dcm login` performs the device authorization flow: dcm login --issuer-url https://keycloak.example.com/realms/dcm --control-plane-url https://dcm.example.com ``` -This discovers the OIDC provider endpoints, initiates a device flow, opens a browser for the user to authenticate, and stores the resulting tokens. On success, `issuer-url` and `control-plane-url` are persisted to `~/.dcm/config.yaml` so subsequent commands work without flags. +This discovers the OIDC provider endpoints, initiates a device flow, opens a browser for the user to authenticate, and stores the resulting tokens. On success, `issuer-url` is persisted to `~/.dcm/config.yaml`, and `control-plane-url` is persisted when explicitly set via flag or `DCM_CONTROL_PLANE_URL`. ### 13.3 CI/Scripting (Static Token) diff --git a/internal/commands/login.go b/internal/commands/login.go index 4bf3587..cc7a75b 100644 --- a/internal/commands/login.go +++ b/internal/commands/login.go @@ -3,6 +3,7 @@ package commands import ( "context" "fmt" + "os" "time" "github.com/dcm-project/cli/internal/auth" @@ -47,7 +48,7 @@ func newLoginCommand() *cobra.Command { configValues := map[string]string{ "issuer-url": cfg.IssuerURL, } - if cfg.ControlPlaneURL != "" { + if controlPlaneURLExplicitlySet(cmd) { configValues["control-plane-url"] = cfg.ControlPlaneURL } if err := config.SaveConfig(config.ConfigPath(cmd), configValues); err != nil { @@ -66,3 +67,13 @@ func newLoginCommand() *cobra.Command { }, } } + +// controlPlaneURLExplicitlySet reports whether the user provided a control-plane +// URL via --control-plane-url or DCM_CONTROL_PLANE_URL. The built-in default +// alone does not count as "set" for login config persistence (REQ-LGN-120). +func controlPlaneURLExplicitlySet(cmd *cobra.Command) bool { + if f := cmd.Root().PersistentFlags().Lookup("control-plane-url"); f != nil && f.Changed { + return true + } + return os.Getenv("DCM_CONTROL_PLANE_URL") != "" +} diff --git a/internal/commands/login_test.go b/internal/commands/login_test.go index b3310a0..7ad41dc 100644 --- a/internal/commands/login_test.go +++ b/internal/commands/login_test.go @@ -79,6 +79,61 @@ var _ = Describe("login command", func() { Expect(string(cfgData)).NotTo(ContainSubstring("token:")) }) + It("persists issuer-url but not the default control-plane-url", func() { + home := GinkgoT().TempDir() + GinkgoT().Setenv("HOME", home) + configPath := filepath.Join(home, "dcm-config.yaml") + + server := mockOIDCServer(mockOIDCOptions{pollsBeforeSuccess: 0}) + defer server.Close() + + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", configPath, + "--issuer-url", server.URL, + "login", + }) + + err := cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + + cfgData, err := os.ReadFile(configPath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(cfgData)).To(ContainSubstring("issuer-url: " + server.URL)) + Expect(string(cfgData)).NotTo(ContainSubstring("control-plane-url")) + }) + + It("persists control-plane-url from DCM_CONTROL_PLANE_URL", func() { + home := GinkgoT().TempDir() + GinkgoT().Setenv("HOME", home) + GinkgoT().Setenv("DCM_CONTROL_PLANE_URL", "http://env-cp.example:8080") + configPath := filepath.Join(home, "dcm-config.yaml") + + server := mockOIDCServer(mockOIDCOptions{pollsBeforeSuccess: 0}) + defer server.Close() + + cmd := commands.NewRootCommand() + errBuf := new(bytes.Buffer) + cmd.SetOut(new(bytes.Buffer)) + cmd.SetErr(errBuf) + cmd.SetArgs([]string{ + "--config", configPath, + "--issuer-url", server.URL, + "login", + }) + + err := cmd.Execute() + Expect(err).NotTo(HaveOccurred()) + + cfgData, err := os.ReadFile(configPath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(cfgData)).To(ContainSubstring("issuer-url: " + server.URL)) + Expect(string(cfgData)).To(ContainSubstring("control-plane-url: http://env-cp.example:8080")) + }) + It("completes device login when stored credentials are expired and refresh fails", func() { home := GinkgoT().TempDir() GinkgoT().Setenv("HOME", home) From 21cd163a56209858ed58dd3b9e0524194755e6b0 Mon Sep 17 00:00:00 2001 From: Chad Crum Date: Mon, 10 Aug 2026 11:12:38 -0400 Subject: [PATCH 14/14] fix(auth): silence unused-parameter lint in reloadEndpointStore Co-Authored-By: Claude Signed-off-by: Chad Crum Co-authored-by: Cursor --- internal/auth/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/auth/transport_test.go b/internal/auth/transport_test.go index cf89629..34d6eed 100644 --- a/internal/auth/transport_test.go +++ b/internal/auth/transport_test.go @@ -36,7 +36,7 @@ type reloadEndpointStore struct { baseTD *auth.TokenData } -func (s *reloadEndpointStore) Load(issuerURL string) (*auth.TokenData, error) { +func (s *reloadEndpointStore) Load(_ string) (*auth.TokenData, error) { n := s.loads.Add(1) td := *s.baseTD if n == 1 {