diff --git a/cli/commands/cluster_add.go b/cli/commands/cluster_add.go index 28b639bd..302d7395 100644 --- a/cli/commands/cluster_add.go +++ b/cli/commands/cluster_add.go @@ -103,7 +103,7 @@ func addCluster(ctx *Context, identityName, clusterName, address string, force b if clusterName == "" && address == "" { ctx.Info("Fetching available clusters from identity server...") - clusters, err := fetchAvailableClusters(ctx, mainConfig, identity) + clusters, err := fetchAvailableClusters(ctx, mainConfig, identityName, identity) if err != nil { return fmt.Errorf("failed to fetch available clusters: %w", err) } diff --git a/cli/commands/cluster_switch.go b/cli/commands/cluster_switch.go index b504edfe..bf71feea 100644 --- a/cli/commands/cluster_switch.go +++ b/cli/commands/cluster_switch.go @@ -10,7 +10,6 @@ import ( "miren.dev/runtime/appconfig" "miren.dev/runtime/clientconfig" - "miren.dev/runtime/pkg/cloudauth" "miren.dev/runtime/pkg/ui" ) @@ -133,7 +132,7 @@ func checkClusterAccess(ctx *Context, cfg *clientconfig.Config, clusterName stri if clusterXID == "" { // Fall back to fetching clusters and finding by name (for backwards compatibility // with clusters added before XID was stored) - clusters, err := fetchAvailableClusters(ctx, cfg, identity) + clusters, err := fetchAvailableClusters(ctx, cfg, cluster.Identity, identity) if err != nil { // If we can't reach the cloud API, we'll allow the switch // The user will get an access denied error when they try to perform actions @@ -156,7 +155,7 @@ func checkClusterAccess(ctx *Context, cfg *clientconfig.Config, clusterName stri } // Check RBAC permission via the check-access endpoint - hasAccess, reason, err := checkClusterAccessRBAC(ctx, cfg, identity, clusterXID) + hasAccess, reason, err := checkClusterAccessRBAC(ctx, cfg, cluster.Identity, identity, clusterXID) if err != nil { // If we can't reach the cloud API, we'll allow the switch ctx.Warn("Could not verify cluster access: %v", err) @@ -175,9 +174,9 @@ func checkClusterAccess(ctx *Context, cfg *clientconfig.Config, clusterName stri } // checkClusterAccessRBAC calls the cloud API to check if the user has RBAC permission to access a cluster -func checkClusterAccessRBAC(ctx *Context, config *clientconfig.Config, identity *clientconfig.IdentityConfig, clusterXID string) (bool, string, error) { - if identity.Type != "keypair" { - return false, "", fmt.Errorf("RBAC check is only supported for keypair identities") +func checkClusterAccessRBAC(ctx *Context, config *clientconfig.Config, identityName string, identity *clientconfig.IdentityConfig, clusterXID string) (bool, string, error) { + if identity.Type != clientconfig.IdentityKeypair && identity.Type != clientconfig.IdentityToken { + return false, "", fmt.Errorf("RBAC check is only supported for keypair and token identities") } // Get the issuer URL @@ -186,20 +185,8 @@ func checkClusterAccessRBAC(ctx *Context, config *clientconfig.Config, identity return false, "", fmt.Errorf("identity has no issuer configured") } - // Get the private key (handles both direct PrivateKey and KeyRef) - privateKeyPEM, err := config.GetPrivateKeyPEM(identity) - if err != nil { - return false, "", fmt.Errorf("failed to get private key: %w", err) - } - - // Load the private key - keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) - if err != nil { - return false, "", fmt.Errorf("failed to load private key: %w", err) - } - // Get JWT token - token, err := clientconfig.AuthenticateWithKey(ctx, issuerURL, keyPair) + token, err := config.TokenForIdentity(ctx, identityName, identity, issuerURL) if err != nil { return false, "", fmt.Errorf("failed to authenticate: %w", err) } diff --git a/cli/commands/config_bind_helpers.go b/cli/commands/config_bind_helpers.go index 16af79fc..0f40ceb7 100644 --- a/cli/commands/config_bind_helpers.go +++ b/cli/commands/config_bind_helpers.go @@ -15,7 +15,6 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" "miren.dev/runtime/clientconfig" - "miren.dev/runtime/pkg/cloudauth" "miren.dev/runtime/pkg/theme" "miren.dev/runtime/pkg/ui" ) @@ -195,10 +194,12 @@ func isPrivateAddress(host string) bool { return ip.IsPrivate() } -// fetchAvailableClusters queries the identity server for available clusters -func fetchAvailableClusters(ctx *Context, config *clientconfig.Config, identity *clientconfig.IdentityConfig) ([]ClusterResponse, error) { - if identity.Type != "keypair" { - return nil, fmt.Errorf("cluster listing is only supported for keypair identities") +// fetchAvailableClusters queries the identity server for available clusters. +// identityName may be "" for an anonymous in-memory identity (e.g. during login +// before it has been named), in which case token refreshes are not persisted. +func fetchAvailableClusters(ctx *Context, config *clientconfig.Config, identityName string, identity *clientconfig.IdentityConfig) ([]ClusterResponse, error) { + if identity.Type != clientconfig.IdentityKeypair && identity.Type != clientconfig.IdentityToken { + return nil, fmt.Errorf("cluster listing is only supported for keypair and token identities") } // Get the issuer URL @@ -207,20 +208,8 @@ func fetchAvailableClusters(ctx *Context, config *clientconfig.Config, identity return nil, fmt.Errorf("identity has no issuer configured") } - // Get the private key (handles both direct PrivateKey and KeyRef) - privateKeyPEM, err := config.GetPrivateKeyPEM(identity) - if err != nil { - return nil, fmt.Errorf("failed to get private key: %w", err) - } - - // Load the private key - keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) - if err != nil { - return nil, fmt.Errorf("failed to load private key: %w", err) - } - // Get JWT token - token, err := clientconfig.AuthenticateWithKey(ctx, issuerURL, keyPair) + token, err := config.TokenForIdentity(ctx, identityName, identity, issuerURL) if err != nil { return nil, fmt.Errorf("failed to authenticate: %w", err) } diff --git a/cli/commands/debug_connection.go b/cli/commands/debug_connection.go index d966e72a..cfc84eac 100644 --- a/cli/commands/debug_connection.go +++ b/cli/commands/debug_connection.go @@ -12,7 +12,6 @@ import ( "github.com/golang-jwt/jwt/v5" "github.com/quic-go/quic-go/http3" "miren.dev/runtime/clientconfig" - "miren.dev/runtime/pkg/cloudauth" ) // DebugAuthResponse represents the response from the debug-auth endpoint @@ -78,6 +77,7 @@ func DebugConnection(ctx *Context, opts struct { ctx.Info("Step 2: Preparing authentication...") var identity *clientconfig.IdentityConfig + var identityName string // Determine which identity to use if opts.Identity != "" { @@ -88,6 +88,7 @@ func DebugConnection(ctx *Context, opts struct { ctx.Warn("Failed to get identity '%s': %v", opts.Identity, err) return err } + identityName = opts.Identity ctx.Info("Using identity: %s", opts.Identity) } else { ctx.Warn("No configuration found. Please run 'miren login' first.") @@ -108,18 +109,19 @@ func DebugConnection(ctx *Context, opts struct { ctx.Warn("Failed to get identity '%s' for cluster '%s': %v", cluster.Identity, opts.Cluster, err) return err } + identityName = cluster.Identity ctx.Info("Using identity '%s' from cluster '%s'", cluster.Identity, opts.Cluster) } else if cluster.CloudAuth && cluster.ClientKey != "" { // Backward compatibility: create identity from cluster identity = &clientconfig.IdentityConfig{ - Type: "keypair", + Type: clientconfig.IdentityKeypair, PrivateKey: cluster.ClientKey, } ctx.Info("Using legacy CloudAuth from cluster '%s'", opts.Cluster) } else if cluster.ClientCert != "" && cluster.ClientKey != "" { // Backward compatibility: certificate auth identity = &clientconfig.IdentityConfig{ - Type: "certificate", + Type: clientconfig.IdentityCertificate, ClientCert: cluster.ClientCert, ClientKey: cluster.ClientKey, } @@ -142,30 +144,14 @@ func DebugConnection(ctx *Context, opts struct { if identity != nil { switch identity.Type { - case "keypair": - // For keypair auth, we need to get a JWT token - // The server we're testing should be the cluster, not the auth server - - // Get the private key (handles both direct PrivateKey and KeyRef) - var privateKeyPEM string - if config != nil { - privateKeyPEM, err = config.GetPrivateKeyPEM(identity) - if err != nil { - ctx.Warn("Failed to get private key: %v", err) - return err - } - } else if identity.PrivateKey != "" { - // Fallback for backward compatibility (when identity is created on-the-fly) - privateKeyPEM = identity.PrivateKey - } else { - ctx.Warn("Identity has no private key or key reference") - return fmt.Errorf("no private key available") - } - - keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) - if err != nil { - ctx.Warn("Failed to load private key: %v", err) - return err + case clientconfig.IdentityKeypair, clientconfig.IdentityToken: + // Both keypair and ephemeral token identities resolve to a JWT bearer + // token. The server we're testing is the cluster, not the auth server. + if config == nil { + // On-the-fly identities (legacy CloudAuth) require a config to + // resolve keys; without one there is nothing to authenticate with. + ctx.Warn("No configuration found. Please run 'miren login' first.") + return fmt.Errorf("no configuration found") } // Use the issuer from the identity @@ -176,17 +162,17 @@ func DebugConnection(ctx *Context, opts struct { ctx.Warn("Identity has no issuer configured, using test server as auth server") } - ctx.Info("Requesting JWT token using keypair from %s...", authServerURL) + ctx.Info("Requesting JWT token for %s identity from %s...", identity.Type, authServerURL) // Note: This will use the cached token if available - token, err := clientconfig.AuthenticateWithKey(ctx, authServerURL, keyPair) + token, err := config.TokenForIdentity(ctx, identityName, identity, authServerURL) if err != nil { - ctx.Warn("Failed to authenticate with keypair: %v", err) + ctx.Warn("Failed to authenticate: %v", err) ctx.Info("Note: Ensure the auth server is reachable at %s", authServerURL) return err } authHeader = "Bearer " + token - authMethod = "keypair/jwt" + authMethod = string(identity.Type) + "/jwt" ctx.Completed("JWT token obtained") // Decode and print JWT claims for debugging @@ -200,7 +186,7 @@ func DebugConnection(ctx *Context, opts struct { } } - case "certificate": + case clientconfig.IdentityCertificate: // Certificate auth is handled at TLS level authMethod = "certificate" ctx.Info("Using client certificate authentication") @@ -230,7 +216,7 @@ func DebugConnection(ctx *Context, opts struct { InsecureSkipVerify: opts.Insecure, } - if identity != nil && identity.Type == "certificate" { + if identity != nil && identity.Type == clientconfig.IdentityCertificate { // Add client certificate cert, err := tls.X509KeyPair([]byte(identity.ClientCert), []byte(identity.ClientKey)) if err != nil { diff --git a/cli/commands/doctor.go b/cli/commands/doctor.go index 6eaeafba..cc2a206d 100644 --- a/cli/commands/doctor.go +++ b/cli/commands/doctor.go @@ -13,7 +13,6 @@ import ( "github.com/charmbracelet/lipgloss" "miren.dev/runtime/clientconfig" "miren.dev/runtime/pkg/auth" - "miren.dev/runtime/pkg/cloudauth" "miren.dev/runtime/pkg/theme" ) @@ -98,35 +97,25 @@ func tryAuthenticate(ctx *Context, cfg *clientconfig.Config, cluster *clientconf result.IdentityName = cluster.Identity switch identity.Type { - case "keypair": - privateKeyPEM, err := cfg.GetPrivateKeyPEM(identity) - if err != nil { - return result - } - - keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) - if err != nil { - return result - } - + case clientconfig.IdentityKeypair, clientconfig.IdentityToken: authServer := identity.Issuer if authServer == "" { authServer = cluster.Hostname } authServer = normalizeAuthServerURL(authServer) - token, err := clientconfig.AuthenticateWithKey(ctx, authServer, keyPair) + token, err := cfg.TokenForIdentity(ctx, cluster.Identity, identity, authServer) if err != nil { return result } result.Claims, _ = auth.ParseUnverifiedClaims(token) - result.Method = "keypair" + result.Method = string(identity.Type) // Fetch user info from cloud result.UserInfo, _ = fetchCloudUserInfo(ctx, authServer, token) - case "certificate": + case clientconfig.IdentityCertificate: result.Method = "certificate" } diff --git a/cli/commands/global.go b/cli/commands/global.go index 65fb41bc..c9c101ba 100644 --- a/cli/commands/global.go +++ b/cli/commands/global.go @@ -446,7 +446,7 @@ func (c *Context) wrapRPCError(err error) error { clusterName = "the cluster" } - c.Warn("access denied: you don't have permission to access %s\nPlease check your credentials or request access from the cluster administrator", clusterName) + c.Warn("access denied: you don't have permission to access %s\nPlease check your credentials or request access from the cluster administrator.\nIf you were logged in previously, your session may have expired — run 'miren login'.", clusterName) return ErrAccessDenied } return err diff --git a/cli/commands/login.go b/cli/commands/login.go index cacbb359..e46b3ff2 100644 --- a/cli/commands/login.go +++ b/cli/commands/login.go @@ -11,7 +11,6 @@ import ( "net/http" "net/url" "os" - "strings" "time" "miren.dev/runtime/clientconfig" @@ -43,10 +42,20 @@ type DeviceFlowExchangeResponse struct { Error string `json:"error,omitempty"` ErrorDescription string `json:"error_description,omitempty"` AccessToken string `json:"access_token,omitempty"` + RefreshToken string `json:"refresh_token,omitempty"` TokenType string `json:"token_type,omitempty"` ExpiresIn int `json:"expires_in,omitempty"` } +// deviceFlowTokens carries the credentials returned by a successful device-flow +// exchange. The refresh token is present only when the cloud supports ephemeral +// login; older clouds return an empty RefreshToken. +type deviceFlowTokens struct { + AccessToken string + RefreshToken string + ExpiresIn int +} + // BeginKeyRegistrationRequest represents the request to begin key registration type BeginKeyRegistrationRequest struct { Name string `json:"name"` @@ -113,21 +122,22 @@ func getOrCreateKey(ctx *Context, keyName string) (*cloudauth.KeyPair, error) { // LoginWithDefaults runs the login flow with default settings func LoginWithDefaults(ctx *Context) error { - return login(ctx, "https://miren.cloud", "cloud", "miren-cli", false, false) + return login(ctx, "https://miren.cloud", "cloud", "miren-cli", false, false, false) } // Login authenticates with miren.cloud using device flow func Login(ctx *Context, opts struct { - CloudURL string `short:"u" long:"url" description:"Cloud URL" default:"https://miren.cloud"` - IdentityName string `short:"i" long:"identity" description:"Name for this identity in config" default:"cloud"` - KeyName string `short:"k" long:"key-name" description:"Name for the authentication key" default:"miren-cli"` - NoSave bool `long:"no-save" description:"Don't save credentials to config file"` - Force bool `short:"f" long:"force" description:"Overwrite existing identity without prompting"` + CloudURL string `short:"u" long:"url" description:"Cloud URL" default:"https://miren.cloud"` + IdentityName string `short:"i" long:"identity" description:"Name for this identity in config" default:"cloud"` + KeyName string `short:"k" long:"key-name" description:"Name for the authentication key" default:"miren-cli"` + NoSave bool `long:"no-save" description:"Don't save credentials to config file"` + Force bool `short:"f" long:"force" description:"Overwrite existing identity without prompting"` + PersistentKey bool `long:"persistent-key" description:"Register a persistent key instead of the default renewable token login"` }) error { - return login(ctx, opts.CloudURL, opts.IdentityName, opts.KeyName, opts.NoSave, opts.Force) + return login(ctx, opts.CloudURL, opts.IdentityName, opts.KeyName, opts.NoSave, opts.Force, opts.PersistentKey) } -func login(ctx *Context, cloudURL, identityName, keyName string, noSave, force bool) error { +func login(ctx *Context, cloudURL, identityName, keyName string, noSave, force, persistentKey bool) error { // Check for existing identity (unless forcing or not saving) if !force && !noSave { config, err := clientconfig.LoadConfig() @@ -185,6 +195,19 @@ func login(ctx *Context, cloudURL, identityName, keyName string, noSave, force b } } + // Decide whether to use the persistent-key flow. Ephemeral token login is the + // default; --persistent-key opts back in. An existing keypair identity keeps + // its key on re-login so we never silently orphan a registered server-side key. + usePersistentKey := persistentKey + if !usePersistentKey { + if existing, err := clientconfig.LoadConfig(); err == nil && existing != nil { + if id, err := existing.GetIdentity(identityName); err == nil && id != nil && id.Type == clientconfig.IdentityKeypair { + usePersistentKey = true + ctx.Info("Identity '%s' already uses a persistent key; keeping it.", identityName) + } + } + } + // Initialize device flow ctx.Info("Initiating device flow authentication...") initResp, err := initiateDeviceFlow(cloudURL) @@ -226,7 +249,7 @@ func login(ctx *Context, cloudURL, identityName, keyName string, noSave, force b pollInterval = time.Duration(initResp.PollingInterval) * time.Second } - token, err := pollForToken(ctx, cloudURL, initResp.DeviceCode, pollInterval, timeout, func(status string) { + tokens, err := pollForToken(ctx, cloudURL, initResp.DeviceCode, pollInterval, timeout, func(status string) { if status == "pending" { fmt.Print(".") } @@ -239,64 +262,128 @@ func login(ctx *Context, cloudURL, identityName, keyName string, noSave, force b ctx.Completed("Authentication successful!") - // Get or create a keypair for future authentication + // An ephemeral login needs a refresh token to renew without re-login. If the + // cloud is too old to return one, fall back to the persistent-key flow so + // login still produces durable credentials. + if !usePersistentKey && tokens.RefreshToken == "" { + ctx.Warn("Cloud did not return a refresh token; falling back to persistent key authentication.") + usePersistentKey = true + } + + if noSave { + return printUnsavedCredentials(ctx, cloudURL, tokens, usePersistentKey, keyName) + } + + if usePersistentKey { + return persistKeypairIdentity(ctx, identityName, cloudURL, keyName, tokens.AccessToken) + } + return persistTokenIdentity(ctx, identityName, cloudURL, tokens) +} + +// persistKeypairIdentity runs the persistent-key flow: register (or reuse) an +// ed25519 key with the cloud, store a keypair identity, and auto-configure a +// cluster. accessToken is the device-flow JWT used only to authorize key +// registration. +func persistKeypairIdentity(ctx *Context, identityName, cloudURL, keyName, accessToken string) error { keyPair, err := getOrCreateKey(ctx, keyName) if err != nil { - ctx.Warn("Failed to get or create keypair: %v", err) - ctx.Info("You can still use token authentication") - keyPair = nil - } else { - // Register the public key with the server (only if it's a new key) - ctx.Info("Registering public key with server...") - if err := registerPublicKey(cloudURL, token, keyPair, keyName); err != nil { - ctx.Warn("Failed to register public key: %v", err) - ctx.Info("You can still use token authentication") - keyPair = nil // Don't save the key if registration failed - } else { - ctx.Info("Public key registered successfully") + return fmt.Errorf("failed to get or create keypair: %w", err) + } + + ctx.Info("Registering public key with server...") + if err := registerPublicKey(cloudURL, accessToken, keyPair, keyName); err != nil { + return fmt.Errorf("failed to register public key: %w", err) + } + ctx.Info("Public key registered successfully") + + if err := saveKeyPairToConfig(identityName, cloudURL, keyPair, keyName); err != nil { + return fmt.Errorf("failed to save identity to config: %w", err) + } + ctx.Info("Identity '%s' saved to config", identityName) + ctx.Info("Future authentication will use the keypair (no login required)") + ctx.Info("") + + privateKeyPEM, err := keyPair.PrivateKeyPEM() + if err != nil { + return fmt.Errorf("failed to encode private key: %w", err) + } + identity := &clientconfig.IdentityConfig{ + Type: clientconfig.IdentityKeypair, + Issuer: clientconfig.NormalizeIssuerURL(cloudURL), + PrivateKey: privateKeyPEM, + } + maybeAutoConfigureCluster(ctx, identityName, cloudURL, identity) + return nil +} + +// persistTokenIdentity runs the ephemeral flow: store the device-flow access and +// refresh tokens as a "token" identity and auto-configure a cluster. No key is +// registered with the cloud, which is the whole point of ephemeral login. +func persistTokenIdentity(ctx *Context, identityName, cloudURL string, tokens *deviceFlowTokens) error { + issuer := clientconfig.NormalizeIssuerURL(cloudURL) + identity := &clientconfig.IdentityConfig{ + Type: clientconfig.IdentityToken, + Issuer: issuer, + Token: tokens.AccessToken, + RefreshToken: tokens.RefreshToken, + } + + if err := saveTokenIdentityToConfig(identityName, identity); err != nil { + return fmt.Errorf("failed to save identity to config: %w", err) + } + ctx.Info("Identity '%s' saved to config", identityName) + ctx.Info("Future authentication reuses the saved token (renewed automatically)") + ctx.Info("") + + maybeAutoConfigureCluster(ctx, identityName, cloudURL, identity) + return nil +} + +// maybeAutoConfigureCluster attempts cluster auto-configuration, logging only +// genuinely unexpected errors (not the expected "nothing to configure" cases). +func maybeAutoConfigureCluster(ctx *Context, identityName, cloudURL string, identity *clientconfig.IdentityConfig) { + if err := autoConfigureCluster(ctx, identityName, cloudURL, identity); err != nil { + if !errors.Is(err, ErrNoAutoConfigNeeded) && !errors.Is(err, ErrAutoConfigFailed) { + ctx.Info("Note: %v", err) } } +} - // Save to config unless --no-save is specified - if !noSave { - if keyPair != nil { - // Save identity with keypair for future authentication - if err := saveKeyPairToConfig(identityName, cloudURL, keyPair, keyName); err != nil { - ctx.Warn("Failed to save identity to config: %v", err) - } else { - ctx.Info("Identity '%s' saved to config", identityName) - ctx.Info("Future authentication will use the keypair (no login required)") - ctx.Info("") - - // Check if we should auto-configure a cluster - if err := autoConfigureCluster(ctx, identityName, cloudURL, keyPair); err != nil { - // Don't fail the login, just log if there's a real error - if !errors.Is(err, ErrNoAutoConfigNeeded) && !errors.Is(err, ErrAutoConfigFailed) { - // Only log unexpected errors, not expected ones - ctx.Info("Note: %v", err) - } - } - } - } else { - // No keypair was registered - ctx.Warn("Authentication successful but no persistent credentials were saved") - ctx.Info("You can still use the token with --token flag:") - ctx.Info(" export MIREN_TOKEN=%s", token) +// printUnsavedCredentials handles --no-save: it prints the obtained credentials +// instead of persisting them. For a persistent-key login it prints the freshly +// generated key; otherwise it prints the access token (and warns about the +// refresh token, a 7-day credential, landing in terminal scrollback). +func printUnsavedCredentials(ctx *Context, cloudURL string, tokens *deviceFlowTokens, usePersistentKey bool, keyName string) error { + if usePersistentKey { + keyPair, err := getOrCreateKey(ctx, keyName) + if err != nil { + return fmt.Errorf("failed to get or create keypair: %w", err) } - } else { - if keyPair != nil { - privateKeyPEM, _ := keyPair.PrivateKeyPEM() - ctx.Info("Private key (not saved):") - ctx.Info("%s", privateKeyPEM) - ctx.Info("") + + // Register even though we aren't saving: a private key the cloud has + // never seen cannot authenticate, so printing it unregistered would hand + // the user a credential that silently fails later. + ctx.Info("Registering public key with server...") + if err := registerPublicKey(cloudURL, tokens.AccessToken, keyPair, keyName); err != nil { + return fmt.Errorf("failed to register public key: %w", err) + } + + privateKeyPEM, err := keyPair.PrivateKeyPEM() + if err != nil { + return fmt.Errorf("failed to encode private key: %w", err) } - ctx.Info("Token (not saved):") - ctx.Info(" %s", token) + ctx.Info("Private key (not saved):") + ctx.Info("%s", privateKeyPEM) ctx.Info("") - ctx.Info("Export as environment variable:") - ctx.Info(" export MIREN_TOKEN=%s", token) } + ctx.Info("Access token (not saved):") + ctx.Info(" %s", tokens.AccessToken) + ctx.Info("") + if tokens.RefreshToken != "" { + ctx.Warn("A refresh token was issued but not printed; it is a long-lived credential.") + ctx.Info("Re-run 'miren login' without --no-save to store it securely.") + } return nil } @@ -347,10 +434,10 @@ func initiateDeviceFlow(cloudURL string) (*DeviceFlowInitResponse, error) { return &initResp, nil } -func pollForToken(ctx context.Context, cloudURL, deviceCode string, interval, maxDuration time.Duration, progress func(string)) (string, error) { +func pollForToken(ctx context.Context, cloudURL, deviceCode string, interval, maxDuration time.Duration, progress func(string)) (*deviceFlowTokens, error) { url, err := url.JoinPath(cloudURL, "/api/v1/device/token") if err != nil { - return "", fmt.Errorf("invalid cloud URL: %w", err) + return nil, fmt.Errorf("invalid cloud URL: %w", err) } reqBody := map[string]string{ @@ -361,7 +448,7 @@ func pollForToken(ctx context.Context, cloudURL, deviceCode string, interval, ma jsonData, err := json.Marshal(reqBody) if err != nil { - return "", err + return nil, err } client := &http.Client{Timeout: 30 * time.Second} @@ -376,13 +463,15 @@ func pollForToken(ctx context.Context, cloudURL, deviceCode string, interval, ma select { case <-timeoutCtx.Done(): if errors.Is(timeoutCtx.Err(), context.DeadlineExceeded) { - return "", fmt.Errorf("authentication timed out after %v: %w", maxDuration, timeoutCtx.Err()) + return nil, fmt.Errorf("authentication timed out after %v: %w", maxDuration, timeoutCtx.Err()) } - return "", timeoutCtx.Err() + return nil, timeoutCtx.Err() case <-ticker.C: - req, err := http.NewRequest("POST", url, bytes.NewBuffer(jsonData)) + // Bind each poll request to the timeout context so a hung server + // cannot outlive the overall deadline. + req, err := http.NewRequestWithContext(timeoutCtx, "POST", url, bytes.NewBuffer(jsonData)) if err != nil { - return "", err + return nil, err } req.Header.Set("Content-Type", "application/json") @@ -404,21 +493,25 @@ func pollForToken(ctx context.Context, cloudURL, deviceCode string, interval, ma // Server always returns 200 with status in JSON var exchangeResp DeviceFlowExchangeResponse if err := json.Unmarshal(body, &exchangeResp); err != nil { - return "", fmt.Errorf("failed to parse response: %w", err) + return nil, fmt.Errorf("failed to parse response: %w", err) } switch exchangeResp.Status { case "authorized": if exchangeResp.AccessToken == "" { - return "", fmt.Errorf("server returned authorized status but no token") + return nil, fmt.Errorf("server returned authorized status but no token") } - return exchangeResp.AccessToken, nil + return &deviceFlowTokens{ + AccessToken: exchangeResp.AccessToken, + RefreshToken: exchangeResp.RefreshToken, + ExpiresIn: exchangeResp.ExpiresIn, + }, nil case "denied": - return "", fmt.Errorf("authorization denied by user") + return nil, fmt.Errorf("authorization denied by user") case "expired": - return "", fmt.Errorf("device code expired") + return nil, fmt.Errorf("device code expired") case "pending": progress("pending") @@ -433,7 +526,7 @@ func pollForToken(ctx context.Context, cloudURL, deviceCode string, interval, ma case "authorization_pending": progress("pending") default: - return "", fmt.Errorf("server error: %s - %s", exchangeResp.Error, exchangeResp.ErrorDescription) + return nil, fmt.Errorf("server error: %s - %s", exchangeResp.Error, exchangeResp.ErrorDescription) } default: @@ -571,11 +664,10 @@ func saveKeyPairToConfig(identityName, cloudURL string, keyPair *cloudauth.KeyPa return fmt.Errorf("failed to encode private key: %w", err) } - // Parse cloud URL to get the issuer - issuer := strings.TrimSuffix(cloudURL, "/") - if !strings.HasPrefix(issuer, "http://") && !strings.HasPrefix(issuer, "https://") { - issuer = "https://" + issuer - } + // Parse cloud URL to get the issuer. Share one normalization rule with the + // token save path so a keypair and token identity for the same --url always + // store an identical issuer (notably http:// for loopback dev clouds). + issuer := clientconfig.NormalizeIssuerURL(cloudURL) // Load or create the main client config mainConfig, err := clientconfig.LoadConfig() @@ -622,7 +714,7 @@ func saveKeyPairToConfig(identityName, cloudURL string, keyPair *cloudauth.KeyPa leafConfigData := &clientconfig.ConfigData{ Identities: map[string]*clientconfig.IdentityConfig{ identityName: { - Type: "keypair", + Type: clientconfig.IdentityKeypair, Issuer: issuer, KeyRef: keyName, }, @@ -636,10 +728,36 @@ func saveKeyPairToConfig(identityName, cloudURL string, keyPair *cloudauth.KeyPa return mainConfig.Save() } +// saveTokenIdentityToConfig persists an ephemeral "token" identity (access + +// refresh token) to clientconfig.d/identity-{name}.yaml. Unlike the keypair +// flow there is no separate key file — the credentials live entirely in the +// identity leaf. +func saveTokenIdentityToConfig(identityName string, identity *clientconfig.IdentityConfig) error { + mainConfig, err := clientconfig.LoadConfig() + if err != nil { + if err == clientconfig.ErrNoConfig { + mainConfig = clientconfig.NewConfig() + } else { + return fmt.Errorf("failed to load config: %w", err) + } + } + + leafConfigData := &clientconfig.ConfigData{ + Identities: map[string]*clientconfig.IdentityConfig{ + identityName: identity, + }, + } + mainConfig.SetLeafConfig("identity-"+identityName, leafConfigData) + + return mainConfig.Save() +} + // autoConfigureCluster checks if there are any local clusters configured, // and if not, fetches available clusters from the server and automatically -// configures the client if there's only one cluster available -func autoConfigureCluster(ctx *Context, identityName, cloudURL string, keyPair *cloudauth.KeyPair) error { +// configures the client if there's only one cluster available. The identity is +// an in-memory (unnamed) credential used to authenticate the cluster lookup; it +// carries either a direct private key (keypair) or device-flow tokens (token). +func autoConfigureCluster(ctx *Context, identityName, cloudURL string, identity *clientconfig.IdentityConfig) error { // Load the main config to check if any clusters are configured mainConfig, err := clientconfig.LoadConfig() if err != nil && err != clientconfig.ErrNoConfig { @@ -654,30 +772,13 @@ func autoConfigureCluster(ctx *Context, identityName, cloudURL string, keyPair * ctx.Info("Checking for available clusters...") - // Create identity config for fetching clusters - issuer := strings.TrimSuffix(cloudURL, "/") - if !strings.HasPrefix(issuer, "http://") && !strings.HasPrefix(issuer, "https://") { - issuer = "https://" + issuer - } - - privateKeyPEM, err := keyPair.PrivateKeyPEM() - if err != nil { - return fmt.Errorf("failed to encode private key: %w", err) - } - - identity := &clientconfig.IdentityConfig{ - Type: "keypair", - Issuer: issuer, - PrivateKey: privateKeyPEM, - } - - // Fetch available clusters from the server - // Pass mainConfig even though this identity has a direct PrivateKey (not KeyRef) - // The helper will handle both cases + // Fetch available clusters from the server. The identity is anonymous + // (name ""), so any token refresh it triggers is used for this call only and + // not persisted — fine, since the tokens are fresh from the device flow. if mainConfig == nil { mainConfig = clientconfig.NewConfig() } - clusters, err := fetchAvailableClusters(ctx, mainConfig, identity) + clusters, err := fetchAvailableClusters(ctx, mainConfig, "", identity) if err != nil { return fmt.Errorf("failed to fetch available clusters: %w", err) } @@ -788,18 +889,7 @@ func getIdentityUserInfo(ctx *Context, config *clientconfig.Config, identityName return "" } - if identity.Type != "keypair" { - return "" - } - - // Get the private key - privateKeyPEM, err := config.GetPrivateKeyPEM(identity) - if err != nil { - return "" - } - - keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) - if err != nil { + if identity.Type != clientconfig.IdentityKeypair && identity.Type != clientconfig.IdentityToken { return "" } @@ -810,7 +900,7 @@ func getIdentityUserInfo(ctx *Context, config *clientconfig.Config, identityName } // Authenticate and get JWT token - token, err := clientconfig.AuthenticateWithKey(ctx, authServer, keyPair) + token, err := config.TokenForIdentity(ctx, identityName, identity, authServer) if err != nil { return "" } diff --git a/cli/commands/login_deviceflow_test.go b/cli/commands/login_deviceflow_test.go new file mode 100644 index 00000000..fbd3b63c --- /dev/null +++ b/cli/commands/login_deviceflow_test.go @@ -0,0 +1,128 @@ +package commands + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// deviceTokenServer serves a scripted sequence of /api/v1/device/token +// responses so pollForToken's state machine can be exercised deterministically. +func deviceTokenServer(t *testing.T, responses []map[string]any) *httptest.Server { + t.Helper() + var idx atomic.Int32 + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/api/v1/device/token", r.URL.Path) + i := int(idx.Add(1)) - 1 + if i >= len(responses) { + i = len(responses) - 1 // repeat the last response + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(responses[i]) + })) +} + +func TestPollForToken(t *testing.T) { + interval := 5 * time.Millisecond + timeout := 2 * time.Second + + t.Run("pending then authorized returns access and refresh tokens", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{ + {"status": "pending"}, + {"status": "pending"}, + { + "status": "authorized", + "access_token": "access-jwt", + "refresh_token": "refresh-jwt", + "token_type": "Bearer", + "expires_in": 3600, + }, + }) + defer srv.Close() + + var pendings atomic.Int32 + tokens, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, timeout, func(status string) { + if status == "pending" { + pendings.Add(1) + } + }) + require.NoError(t, err) + require.Equal(t, "access-jwt", tokens.AccessToken) + require.Equal(t, "refresh-jwt", tokens.RefreshToken) + require.Equal(t, 3600, tokens.ExpiresIn) + require.GreaterOrEqual(t, pendings.Load(), int32(2)) + }) + + t.Run("authorized without refresh token (older cloud)", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{ + {"status": "authorized", "access_token": "access-only", "token_type": "Bearer", "expires_in": 3600}, + }) + defer srv.Close() + + tokens, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, timeout, func(string) {}) + require.NoError(t, err) + require.Equal(t, "access-only", tokens.AccessToken) + require.Empty(t, tokens.RefreshToken) + }) + + t.Run("denied", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{{"status": "denied"}}) + defer srv.Close() + + _, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, timeout, func(string) {}) + require.ErrorContains(t, err, "denied") + }) + + t.Run("expired", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{{"status": "expired"}}) + defer srv.Close() + + _, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, timeout, func(string) {}) + require.ErrorContains(t, err, "expired") + }) + + t.Run("authorized with empty access token errors", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{{"status": "authorized", "access_token": ""}}) + defer srv.Close() + + _, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, timeout, func(string) {}) + require.Error(t, err) + }) + + t.Run("slow_down then authorized", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{ + {"status": "error", "error": "slow_down"}, + {"status": "authorized", "access_token": "access-jwt", "refresh_token": "refresh-jwt"}, + }) + defer srv.Close() + + tokens, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, timeout, func(string) {}) + require.NoError(t, err) + require.Equal(t, "access-jwt", tokens.AccessToken) + }) + + t.Run("unknown error status is fatal", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{ + {"status": "error", "error": "boom", "error_description": "kaboom"}, + }) + defer srv.Close() + + _, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, timeout, func(string) {}) + require.ErrorContains(t, err, "boom") + }) + + t.Run("times out while pending", func(t *testing.T) { + srv := deviceTokenServer(t, []map[string]any{{"status": "pending"}}) + defer srv.Close() + + _, err := pollForToken(context.Background(), srv.URL, "dev-code", interval, 40*time.Millisecond, func(string) {}) + require.ErrorContains(t, err, "timed out") + }) +} diff --git a/cli/commands/login_ephemeral_test.go b/cli/commands/login_ephemeral_test.go new file mode 100644 index 00000000..0b3305c7 --- /dev/null +++ b/cli/commands/login_ephemeral_test.go @@ -0,0 +1,228 @@ +package commands + +import ( + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" + + "miren.dev/runtime/clientconfig" +) + +// mockCloud is a fake miren.cloud implementing just enough of the device flow, +// key registration, and cluster listing for the login flow to run end-to-end. +type mockCloud struct { + keyBeginHits atomic.Int32 + keyCompleteHits atomic.Int32 + withRefresh bool +} + +func freshLoginJWT(t *testing.T) string { + t.Helper() + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ + "exp": float64(time.Now().Add(time.Hour).Unix()), + "sub": "user@example.com", + }) + s, err := tok.SignedString([]byte("secret")) + require.NoError(t, err) + return s +} + +func (m *mockCloud) server(t *testing.T) *httptest.Server { + mux := http.NewServeMux() + + mux.HandleFunc("/api/v1/device/code", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "device_code": "dev-code", + "user_code": "ABCD-1234", + "verification_uri": "https://example.com/verify", + "expires_in": 600, + "polling_interval": 1, + }) + }) + + mux.HandleFunc("/api/v1/device/token", func(w http.ResponseWriter, r *http.Request) { + resp := map[string]any{ + "status": "authorized", + "access_token": freshLoginJWT(t), + "token_type": "Bearer", + "expires_in": 3600, + } + if m.withRefresh { + resp["refresh_token"] = freshLoginJWT(t) + } + _ = json.NewEncoder(w).Encode(resp) + }) + + // Exercise the real begin -> sign -> complete round trip rather than the 409 + // "already registered" shortcut, so tests can prove a key was fully + // registered and not merely that begin was called. + mux.HandleFunc("/api/v1/users/keys/begin", func(w http.ResponseWriter, r *http.Request) { + m.keyBeginHits.Add(1) + _ = json.NewEncoder(w).Encode(map[string]any{ + "envelope": "test-envelope", + "challenge": base64.StdEncoding.EncodeToString([]byte("challenge-bytes")), + }) + }) + mux.HandleFunc("/api/v1/users/keys/complete", func(w http.ResponseWriter, r *http.Request) { + var req struct { + Envelope string `json:"envelope"` + Signature string `json:"signature"` + } + _ = json.NewDecoder(r.Body).Decode(&req) + require.Equal(t, "test-envelope", req.Envelope, "complete must echo the envelope from begin") + require.NotEmpty(t, req.Signature, "complete must carry a signature over the challenge") + m.keyCompleteHits.Add(1) + w.WriteHeader(http.StatusOK) + }) + + mux.HandleFunc("/api/v1/users/clusters", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"clusters": []any{}}) + }) + + return httptest.NewServer(mux) +} + +// TestLoginEphemeralDefault is the MIR-1385 acceptance test: a default login must +// store a token identity and never touch the key-registration endpoints. +func TestLoginEphemeralDefault(t *testing.T) { + mc := &mockCloud{withRefresh: true} + srv := mc.server(t) + defer srv.Close() + + dir := t.TempDir() + t.Setenv("MIREN_CONFIG", dir) + + ctx := newTestContext() + err := login(ctx, srv.URL, "cloud", "miren-cli", false, false, false) + require.NoError(t, err) + + // The whole point: no persistent key registered. + require.Zero(t, mc.keyBeginHits.Load(), "ephemeral login must not begin key registration") + require.Zero(t, mc.keyCompleteHits.Load(), "ephemeral login must not complete key registration") + + // Identity leaf is a token identity carrying both tokens, at mode 0600. + leaf := filepath.Join(dir, "clientconfig.d", "identity-cloud.yaml") + info, err := os.Stat(leaf) + require.NoError(t, err) + require.Equal(t, os.FileMode(0600), info.Mode().Perm()) + + cfg, err := clientconfig.LoadConfig() + require.NoError(t, err) + id, err := cfg.GetIdentity("cloud") + require.NoError(t, err) + require.Equal(t, clientconfig.IdentityToken, id.Type) + require.NotEmpty(t, id.Token) + require.NotEmpty(t, id.RefreshToken) + + // No key file should have been written. + _, err = os.Stat(filepath.Join(dir, "clientconfig.d", "key-miren-cli.yaml")) + require.True(t, os.IsNotExist(err), "ephemeral login must not write a key file") +} + +// TestLoginPersistentKeyFlag verifies --persistent-key still registers a key and +// stores a keypair identity. +func TestLoginPersistentKeyFlag(t *testing.T) { + mc := &mockCloud{withRefresh: true} + srv := mc.server(t) + defer srv.Close() + + dir := t.TempDir() + t.Setenv("MIREN_CONFIG", dir) + + ctx := newTestContext() + err := login(ctx, srv.URL, "cloud", "miren-cli", false, false, true /* persistentKey */) + require.NoError(t, err) + + require.Equal(t, int32(1), mc.keyBeginHits.Load(), "persistent-key login must register a key") + require.Equal(t, int32(1), mc.keyCompleteHits.Load(), "key registration must complete") + + cfg, err := clientconfig.LoadConfig() + require.NoError(t, err) + id, err := cfg.GetIdentity("cloud") + require.NoError(t, err) + require.Equal(t, clientconfig.IdentityKeypair, id.Type) + require.Empty(t, id.Token) + + _, err = os.Stat(filepath.Join(dir, "clientconfig.d", "key-miren-cli.yaml")) + require.NoError(t, err, "persistent-key login must write a key file") +} + +// TestLoginNoSavePersistentKeyRegisters guards a regression: --no-save with +// --persistent-key printed a freshly generated private key without ever +// registering the public half, so the key it handed the user could not +// authenticate anything. +func TestLoginNoSavePersistentKeyRegisters(t *testing.T) { + mc := &mockCloud{withRefresh: true} + srv := mc.server(t) + defer srv.Close() + + dir := t.TempDir() + t.Setenv("MIREN_CONFIG", dir) + + ctx := newTestContext() + err := login(ctx, srv.URL, "cloud", "miren-cli", true /* noSave */, false, true /* persistentKey */) + require.NoError(t, err) + + require.Equal(t, int32(1), mc.keyBeginHits.Load(), + "a printed key must be registered with the cloud or it cannot authenticate") + require.Equal(t, int32(1), mc.keyCompleteHits.Load(), + "registration must actually complete, not just begin") + + // --no-save must still persist nothing. + _, err = os.Stat(filepath.Join(dir, "clientconfig.d")) + require.True(t, os.IsNotExist(err), "--no-save must not write any config") +} + +// TestLoginNoSaveEphemeralSkipsKeyRegistration verifies the token path stays +// registration-free under --no-save. +func TestLoginNoSaveEphemeralSkipsKeyRegistration(t *testing.T) { + mc := &mockCloud{withRefresh: true} + srv := mc.server(t) + defer srv.Close() + + dir := t.TempDir() + t.Setenv("MIREN_CONFIG", dir) + + ctx := newTestContext() + err := login(ctx, srv.URL, "cloud", "miren-cli", true /* noSave */, false, false) + require.NoError(t, err) + + require.Zero(t, mc.keyBeginHits.Load(), "ephemeral --no-save must not register a key") + require.Zero(t, mc.keyCompleteHits.Load(), "ephemeral --no-save must not complete registration") + _, err = os.Stat(filepath.Join(dir, "clientconfig.d")) + require.True(t, os.IsNotExist(err), "--no-save must not write any config") +} + +// TestLoginFallsBackWhenNoRefreshToken verifies that if the cloud returns no +// refresh token, a default login falls back to the persistent-key flow rather +// than storing an unusable token identity. +func TestLoginFallsBackWhenNoRefreshToken(t *testing.T) { + mc := &mockCloud{withRefresh: false} + srv := mc.server(t) + defer srv.Close() + + dir := t.TempDir() + t.Setenv("MIREN_CONFIG", dir) + + ctx := newTestContext() + err := login(ctx, srv.URL, "cloud", "miren-cli", false, false, false) + require.NoError(t, err) + + require.Equal(t, int32(1), mc.keyBeginHits.Load(), "should fall back to key registration") + require.Equal(t, int32(1), mc.keyCompleteHits.Load(), "fallback key registration must complete") + + cfg, err := clientconfig.LoadConfig() + require.NoError(t, err) + id, err := cfg.GetIdentity("cloud") + require.NoError(t, err) + require.Equal(t, clientconfig.IdentityKeypair, id.Type) +} diff --git a/cli/commands/login_keys_test.go b/cli/commands/login_keys_test.go index 7807fc5f..d6213bf6 100644 --- a/cli/commands/login_keys_test.go +++ b/cli/commands/login_keys_test.go @@ -143,7 +143,7 @@ func TestSaveKeyPairToConfig(t *testing.T) { assert.True(t, config.HasIdentity(identityName), "Identity should be saved") identity, err := config.GetIdentity(identityName) require.NoError(t, err) - assert.Equal(t, "keypair", identity.Type) + assert.Equal(t, clientconfig.IdentityKeypair, identity.Type) assert.Equal(t, "https://test.miren.cloud", identity.Issuer) assert.Equal(t, keyName, identity.KeyRef, "Identity should reference the key") assert.Empty(t, identity.PrivateKey, "Identity should not have direct PrivateKey") diff --git a/cli/commands/logout.go b/cli/commands/logout.go index 50b9539d..577e1bdd 100644 --- a/cli/commands/logout.go +++ b/cli/commands/logout.go @@ -112,6 +112,13 @@ func Logout(ctx *Context, opts struct { } } + // For ephemeral token identities, best-effort revoke the refresh token so a + // leaked ~/.config copy can't keep renewing after logout. Never let a failed + // revoke block the local logout. + if identity.Type == clientconfig.IdentityToken && identity.RefreshToken != "" { + revokeIdentitySession(ctx, cfg, identityName, identity) + } + // Delete the identity file if err := os.Remove(identityFile); err != nil { if !os.IsNotExist(err) { @@ -138,6 +145,35 @@ func Logout(ctx *Context, opts struct { return nil } +// revokeIdentitySession best-effort revokes a token identity's refresh token so +// a copied config can't keep renewing after logout. +// +// The stored access token has a one hour life and is usually already expired by +// the time someone logs out, and the revoke endpoint needs a valid bearer. So we +// ask for a live token first, which may rotate the pair on disk; we then re-read +// the identity so we revoke the refresh token that is actually current rather +// than the one we just spent. +func revokeIdentitySession(ctx *Context, cfg *clientconfig.Config, identityName string, identity *clientconfig.IdentityConfig) { + accessToken, err := cfg.TokenForIdentity(ctx, identityName, identity, identity.Issuer) + if err != nil { + ctx.Warn("Could not obtain a valid token to revoke the session (removing local credentials anyway): %v", err) + return + } + + refreshToken := identity.RefreshToken + if reloaded, err := clientconfig.LoadConfig(); err == nil { + if current, err := reloaded.GetIdentity(identityName); err == nil && current.RefreshToken != "" { + refreshToken = current.RefreshToken + } + } + + if err := clientconfig.RevokeRefreshToken(ctx, identity.Issuer, accessToken, refreshToken); err != nil { + ctx.Warn("Could not revoke refresh token on the server (removing local credentials anyway): %v", err) + return + } + ctx.Info("Revoked refresh token on the server") +} + // getConfigDirPath returns the path to the clientconfig.d directory func getConfigDirPath() (string, error) { // Check environment variable first diff --git a/cli/commands/logout_revoke_test.go b/cli/commands/logout_revoke_test.go new file mode 100644 index 00000000..46dd08b0 --- /dev/null +++ b/cli/commands/logout_revoke_test.go @@ -0,0 +1,123 @@ +package commands + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" + + "miren.dev/runtime/clientconfig" +) + +// writeTokenIdentity writes a token identity leaf into a MIREN_CONFIG dir and +// returns the access token it stored. The access token is a genuinely fresh JWT +// so logout's revoke path can use it directly instead of trying to refresh. +func writeTokenIdentity(t *testing.T, dir, issuer string) string { + t.Helper() + accessToken := freshLoginJWT(t) + cd := clientconfig.ConfigData{ + Identities: map[string]*clientconfig.IdentityConfig{ + "cloud": { + Type: "token", + Issuer: issuer, + Token: accessToken, + RefreshToken: "refresh-jwt", + }, + }, + } + data, err := yaml.Marshal(&cd) + require.NoError(t, err) + leafDir := filepath.Join(dir, "clientconfig.d") + require.NoError(t, os.MkdirAll(leafDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(leafDir, "identity-cloud.yaml"), data, 0600)) + return accessToken +} + +func runLogout(t *testing.T) error { + t.Helper() + return Logout(newTestContext(), struct { + ConfigCentric + IdentityName string `short:"i" long:"identity" description:"Name of the identity to remove"` + }{IdentityName: "cloud"}) +} + +// revokeCapture records what the fake revoke endpoint saw. The handler runs on +// the server's goroutine while the test body reads, so it needs a mutex. +type revokeCapture struct { + mu sync.Mutex + auth string + refresh string + reason string +} + +func (c *revokeCapture) record(auth, refresh, reason string) { + c.mu.Lock() + defer c.mu.Unlock() + c.auth, c.refresh, c.reason = auth, refresh, reason +} + +func (c *revokeCapture) get() (auth, refresh, reason string) { + c.mu.Lock() + defer c.mu.Unlock() + return c.auth, c.refresh, c.reason +} + +func TestLogoutRevokesRefreshToken(t *testing.T) { + var revokeHits atomic.Int32 + var capture revokeCapture + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/auth/revoke/refresh" { + revokeHits.Add(1) + var body struct { + RefreshToken string `json:"refresh_token"` + Reason string `json:"reason"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + capture.record(r.Header.Get("Authorization"), body.RefreshToken, body.Reason) + w.WriteHeader(http.StatusOK) + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + dir := t.TempDir() + t.Setenv("MIREN_CONFIG", dir) + accessToken := writeTokenIdentity(t, dir, srv.URL) + + require.NoError(t, runLogout(t)) + + require.Equal(t, int32(1), revokeHits.Load(), "logout should revoke the refresh token") + auth, refresh, reason := capture.get() + require.Equal(t, "Bearer "+accessToken, auth, "revoke must authenticate with a live access token") + require.Equal(t, "refresh-jwt", refresh) + require.Equal(t, "logout", reason) + + // The identity file must be gone regardless. + _, err := os.Stat(filepath.Join(dir, "clientconfig.d", "identity-cloud.yaml")) + require.True(t, os.IsNotExist(err)) +} + +func TestLogoutSucceedsWhenRevokeFails(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) // revoke fails + })) + defer srv.Close() + + dir := t.TempDir() + t.Setenv("MIREN_CONFIG", dir) + writeTokenIdentity(t, dir, srv.URL) + + // A failed revoke must not block logout. + require.NoError(t, runLogout(t)) + _, err := os.Stat(filepath.Join(dir, "clientconfig.d", "identity-cloud.yaml")) + require.True(t, os.IsNotExist(err), "identity file should still be removed") +} diff --git a/cli/commands/whoami.go b/cli/commands/whoami.go index bff86027..86490a67 100644 --- a/cli/commands/whoami.go +++ b/cli/commands/whoami.go @@ -2,11 +2,9 @@ package commands import ( "fmt" - "strings" "miren.dev/runtime/clientconfig" "miren.dev/runtime/pkg/auth" - "miren.dev/runtime/pkg/cloudauth" ) // Whoami displays information about the current authenticated user @@ -33,43 +31,18 @@ func Whoami(ctx *Context, opts struct { if ctx.ClusterConfig.Identity != "" && ctx.ClientConfig != nil { var err error identity, err = ctx.ClientConfig.GetIdentity(ctx.ClusterConfig.Identity) - if err == nil && identity != nil && identity.Type == "keypair" { - // Get the private key (handles both direct PrivateKey and KeyRef) - privateKeyPEM, err := ctx.ClientConfig.GetPrivateKeyPEM(identity) - if err != nil { - ctx.Warn("Failed to get private key: %v", err) - } else { - keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) + if err == nil && identity != nil { + switch identity.Type { + case clientconfig.IdentityKeypair, clientconfig.IdentityToken: + token, err = ctx.ClientConfig.TokenForIdentity(ctx, ctx.ClusterConfig.Identity, identity, hostname) if err != nil { - ctx.Warn("Failed to load keypair: %v", err) - } else { - // Get the auth server URL - authServer := identity.Issuer - if authServer == "" { - authServer = hostname - } - - // For local development, use HTTP - if !strings.HasPrefix(authServer, "http://") && !strings.HasPrefix(authServer, "https://") { - if strings.Contains(authServer, "localhost") || strings.Contains(authServer, "127.0.0.1") { - authServer = "http://" + authServer - } else { - authServer = "https://" + authServer - } - } - - // Get JWT token - token, err = clientconfig.AuthenticateWithKey(ctx, authServer, keyPair) - if err != nil { - return fmt.Errorf("failed to authenticate with keypair: %w", err) - } - authMethod = "keypair" + return fmt.Errorf("failed to authenticate: %w", err) } + authMethod = string(identity.Type) + case clientconfig.IdentityCertificate: + authMethod = "certificate" } } - if identity != nil && identity.Type == "certificate" { - authMethod = "certificate" - } } // Try to parse JWT claims if we have a token diff --git a/clientconfig/clientconfig.go b/clientconfig/clientconfig.go index 16103927..074a15f2 100644 --- a/clientconfig/clientconfig.go +++ b/clientconfig/clientconfig.go @@ -20,14 +20,30 @@ const ( EnvConfigPath = "MIREN_CONFIG" ) +// IdentityType names how an identity authenticates. Naming the set (rather than +// using a bare string) lets the exhaustive linter flag any switch that forgets +// an arm when a new type is added. +type IdentityType string + +const ( + // IdentityKeypair authenticates with an ed25519 key via challenge-response. + IdentityKeypair IdentityType = "keypair" + // IdentityToken authenticates with a stored JWT that is refreshed as needed. + IdentityToken IdentityType = "token" + // IdentityCertificate authenticates with a client TLS certificate. + IdentityCertificate IdentityType = "certificate" +) + // IdentityConfig holds authentication credentials that can be used across clusters type IdentityConfig struct { - Type string `yaml:"type"` // Type of identity: "keypair", "certificate", etc. - Issuer string `yaml:"issuer,omitempty"` // The auth server that issued this identity (e.g., "https://miren.cloud") - KeyRef string `yaml:"key_ref,omitempty"` // Reference to a key in the Keys section (for keypair auth) - PrivateKey string `yaml:"private_key,omitempty"` // PEM encoded private key (for keypair auth, deprecated - use KeyRef) - ClientCert string `yaml:"client_cert,omitempty"` // PEM encoded client certificate (for cert auth) - ClientKey string `yaml:"client_key,omitempty"` // PEM encoded client key (for cert auth) + Type IdentityType `yaml:"type"` // How this identity authenticates + Issuer string `yaml:"issuer,omitempty"` // The auth server that issued this identity (e.g., "https://miren.cloud") + KeyRef string `yaml:"key_ref,omitempty"` // Reference to a key in the Keys section (for keypair auth) + PrivateKey string `yaml:"private_key,omitempty"` // PEM encoded private key (for keypair auth, deprecated - use KeyRef) + ClientCert string `yaml:"client_cert,omitempty"` // PEM encoded client certificate (for cert auth) + ClientKey string `yaml:"client_key,omitempty"` // PEM encoded client key (for cert auth) + Token string `yaml:"token,omitempty"` // JWT access token (for token auth) + RefreshToken string `yaml:"refresh_token,omitempty"` // Refresh token used to renew the access token (for token auth) } // KeyConfig holds a reusable cryptographic key @@ -257,6 +273,7 @@ func (c *Config) SetLeafConfig(name string, configData *ConfigData) { active: configData.Active, clusters: configData.Clusters, identities: configData.Identities, + keys: configData.Keys, sourcePath: name, // Store the name so we can identify this leaf config later } @@ -637,6 +654,34 @@ func isEmptyConfigData(d *ConfigData) bool { len(d.Keys) == 0) } +// atomicWriteFile writes data to path via a temp file in the same directory +// followed by a rename, so a concurrent reader never observes a partially +// written file. Leaf configs are now rewritten on a hot path (token refresh), +// so a torn read here would fail the whole LoadConfig. +func atomicWriteFile(path string, data []byte, perm os.FileMode) error { + dir := filepath.Dir(path) + tmp, err := os.CreateTemp(dir, "."+filepath.Base(path)+".tmp-*") + if err != nil { + return err + } + tmpPath := tmp.Name() + // Best-effort cleanup if we bail out before the rename. + defer func() { _ = os.Remove(tmpPath) }() + + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Chmod(perm); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + return os.Rename(tmpPath, path) +} + // saveLeafConfigs saves all unsaved leaf configs to clientconfig.d/ directory func (c *Config) saveLeafConfigs(mainConfigPath string) error { // Determine the config.d directory path based on the main config path @@ -666,7 +711,7 @@ func (c *Config) saveLeafConfigs(mainConfigPath string) error { return fmt.Errorf("failed to marshal leaf config %s: %w", name, err) } - if err := os.WriteFile(leafPath, data, 0600); err != nil { + if err := atomicWriteFile(leafPath, data, 0600); err != nil { return fmt.Errorf("failed to write leaf config %s: %w", name, err) } } @@ -751,6 +796,30 @@ func (c *Config) GetClusterSource(name string) string { return "" } +// GetIdentitySource returns the source file path for an identity. +// For identities in the main config, returns the main config path. +// For identities in leaf configs, returns the leaf config path. +// Returns "" if the identity is not found. +func (c *Config) GetIdentitySource(name string) string { + // Check leaf configs first (they override main config) + for _, leafConfig := range c.leafConfigs { + if _, exists := leafConfig.identities[name]; exists { + if c.sourcePath != "" { + configDir := filepath.Dir(c.sourcePath) + return filepath.Join(configDir, "clientconfig.d", leafConfig.sourcePath+".yaml") + } + return leafConfig.sourcePath + } + } + + // Check main config + if _, exists := c.identities[name]; exists { + return c.sourcePath + } + + return "" +} + // HasAnyClusters checks if any clusters are configured func (c *Config) HasAnyClusters() bool { // Check main config diff --git a/clientconfig/clientconfig_modification_test.go b/clientconfig/clientconfig_modification_test.go index 3ef4bda8..c9c47ae8 100644 --- a/clientconfig/clientconfig_modification_test.go +++ b/clientconfig/clientconfig_modification_test.go @@ -107,7 +107,7 @@ clusters: require.NoError(t, err) assert.Equal(t, "new-issuer", prodIdentitySaved.Issuer, "Should save modified issuer") assert.Equal(t, "new-key", prodIdentitySaved.PrivateKey, "Should save modified key") - assert.Equal(t, "keypair", prodIdentitySaved.Type, "Should preserve unmodified type") + assert.Equal(t, IdentityKeypair, prodIdentitySaved.Type, "Should preserve unmodified type") // Verify dev cluster (from config.d) was NOT saved _, err = savedConfig.GetCluster("dev") diff --git a/clientconfig/clientconfig_save_test.go b/clientconfig/clientconfig_save_test.go index 9067a477..a78a94ea 100644 --- a/clientconfig/clientconfig_save_test.go +++ b/clientconfig/clientconfig_save_test.go @@ -287,7 +287,7 @@ func TestSetLeafConfigAndSaving(t *testing.T) { devIdentity, err := config.GetIdentity("dev-identity") require.NoError(t, err) - assert.Equal(t, "keypair", devIdentity.Type) + assert.Equal(t, IdentityKeypair, devIdentity.Type) // Verify counts include both main and leaf configs assert.Equal(t, 2, config.GetClusterCount()) // main-cluster + dev-cluster @@ -336,7 +336,7 @@ func TestSetLeafConfigAndSaving(t *testing.T) { assert.Equal(t, 1, len(leafConfigFromFile.Identities)) devIdentityFromFile, exists := leafConfigFromFile.Identities["dev-identity"] require.True(t, exists) - assert.Equal(t, "keypair", devIdentityFromFile.Type) + assert.Equal(t, IdentityKeypair, devIdentityFromFile.Type) } func TestSetLeafConfigUpdatesExisting(t *testing.T) { diff --git a/clientconfig/leafwrite_test.go b/clientconfig/leafwrite_test.go new file mode 100644 index 00000000..b83341a7 --- /dev/null +++ b/clientconfig/leafwrite_test.go @@ -0,0 +1,114 @@ +package clientconfig + +import ( + "os" + "path/filepath" + "sync" + "testing" + + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +func TestGetIdentitySource(t *testing.T) { + dir := t.TempDir() + mainPath := filepath.Join(dir, "clientconfig.yaml") + + cfg := NewConfig() + cfg.sourcePath = mainPath + + // Identity that lives directly in the main config. + cfg.SetIdentity("main-id", &IdentityConfig{Type: "keypair"}) + + // Identity that lives in a leaf config file. + cfg.SetLeafConfig("identity-leaf-id", &ConfigData{ + Identities: map[string]*IdentityConfig{ + "leaf-id": {Type: "token"}, + }, + }) + + require.Equal(t, mainPath, cfg.GetIdentitySource("main-id")) + require.Equal(t, + filepath.Join(dir, "clientconfig.d", "identity-leaf-id.yaml"), + cfg.GetIdentitySource("leaf-id")) + require.Equal(t, "", cfg.GetIdentitySource("missing")) +} + +// TestAtomicWriteFileNoTornRead hammers atomicWriteFile with a concurrent +// reader that unmarshals the file; a non-atomic write would occasionally yield +// a truncated document and a yaml error. +func TestAtomicWriteFileNoTornRead(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "leaf.yaml") + + // Seed a valid file so the reader always has something to read. + small := []byte("type: token\n") + require.NoError(t, atomicWriteFile(path, small, 0600)) + + // A large payload makes a torn read far more likely under a plain WriteFile. + big, err := yaml.Marshal(&ConfigData{ + Identities: func() map[string]*IdentityConfig { + m := make(map[string]*IdentityConfig) + for i := 0; i < 200; i++ { + m[string(rune('a'+i%26))+string(rune('0'+i%10))+"-"+itoa(i)] = &IdentityConfig{ + Type: "token", + Token: "eyJhbGciOiJFZERTQSJ9." + itoa(i), + RefreshToken: "eyJhbGciOiJFZERTQSJ9.refresh." + itoa(i), + } + } + return m + }(), + }) + require.NoError(t, err) + + var wg sync.WaitGroup + stop := make(chan struct{}) + + // Reader goroutine. + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-stop: + return + default: + } + data, err := os.ReadFile(path) + if err != nil { + continue // File momentarily absent is fine; rename is what matters. + } + var cd ConfigData + require.NoError(t, yaml.Unmarshal(data, &cd), "reader observed a torn/partial write") + } + }() + + // Writer: alternate payloads so length changes each time. + for i := 0; i < 300; i++ { + payload := big + if i%2 == 0 { + payload = small + } + require.NoError(t, atomicWriteFile(path, payload, 0600)) + } + close(stop) + wg.Wait() + + // Final file must be valid and mode 0600. + info, err := os.Stat(path) + require.NoError(t, err) + require.Equal(t, os.FileMode(0600), info.Mode().Perm()) +} + +// itoa is a tiny helper to avoid pulling strconv into a hot loop's closure. +func itoa(i int) string { + if i == 0 { + return "0" + } + var b []byte + for i > 0 { + b = append([]byte{byte('0' + i%10)}, b...) + i /= 10 + } + return string(b) +} diff --git a/clientconfig/local.go b/clientconfig/local.go index 6cf931e2..610668b4 100644 --- a/clientconfig/local.go +++ b/clientconfig/local.go @@ -16,7 +16,6 @@ import ( "strings" "time" - "github.com/golang-jwt/jwt/v5" "github.com/quic-go/quic-go" "miren.dev/runtime/pkg/caauth" "miren.dev/runtime/pkg/cloudauth" @@ -110,27 +109,12 @@ foundAddress: // Handle different identity types switch identity.Type { - case "keypair": - // Get the private key (handles both direct PrivateKey and KeyRef) - privateKeyPEM, err := config.GetPrivateKeyPEM(identity) - if err != nil { - return nil, fmt.Errorf("failed to get private key: %w", err) - } - - // Load the private key - keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) - if err != nil { - return nil, fmt.Errorf("failed to load private key: %w", err) - } - - // Use the issuer from the identity, or fall back to cluster hostname - authServer := identity.Issuer - if authServer == "" { - authServer = hostname - } - - // Get JWT token using the challenge-response flow - token, err := AuthenticateWithKey(ctx, authServer, keyPair) + case IdentityKeypair, IdentityToken: + // Both keypair and ephemeral token identities resolve to a bearer + // token; TokenForIdentity dispatches on the identity type (re-minting + // via challenge-response for keypair, or refreshing the cached token + // for a token identity). + token, err := config.TokenForIdentity(ctx, c.Identity, identity, hostname) if err != nil { return nil, fmt.Errorf("failed to authenticate with cloud: %w", err) } @@ -148,7 +132,7 @@ foundAddress: return base, nil - case "certificate": + case IdentityCertificate: // Handle certificate-based authentication from identity return []rpc.StateOption{ rpc.WithCertPEMs( @@ -291,29 +275,8 @@ func getCachedToken(fingerprint string) (string, error) { tokenString := string(tokenData) - // Parse the JWT without verification to check expiry - parser := jwt.NewParser() - token, _, err := parser.ParseUnverified(tokenString, jwt.MapClaims{}) - if err != nil { - return "", nil // Invalid token, need new one - } - - claims, ok := token.Claims.(jwt.MapClaims) - if !ok { - return "", nil // Invalid claims, need new one - } - - // Check if token is expired - if exp, ok := claims["exp"].(float64); ok { - if time.Now().Unix() >= int64(exp) { - return "", nil // Token expired - } - // Add a buffer of 5 minutes to avoid edge cases - if time.Now().Unix() >= int64(exp)-300 { - return "", nil // Token expiring soon - } - } else { - return "", nil // No expiry claim, need new one + if !tokenFresh(tokenString, tokenExpiryBuffer) { + return "", nil // Missing, expired, or expiring soon — need a new one. } return tokenString, nil diff --git a/clientconfig/setleafconfig_keys_test.go b/clientconfig/setleafconfig_keys_test.go new file mode 100644 index 00000000..2c43ef32 --- /dev/null +++ b/clientconfig/setleafconfig_keys_test.go @@ -0,0 +1,32 @@ +package clientconfig + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestSetLeafConfigRetainsKeys guards against a regression where SetLeafConfig +// built its in-memory leaf Config without copying Keys, so a key registered via +// a leaf config was invisible to GetKey/HasKey until the config was reloaded +// from disk. +func TestSetLeafConfigRetainsKeys(t *testing.T) { + cfg := NewConfig() + + cfg.SetLeafConfig("key-miren-cli", &ConfigData{ + Keys: map[string]*KeyConfig{ + "miren-cli": { + Name: "miren-cli", + Type: "ed25519", + PrivateKey: "-----BEGIN PRIVATE KEY-----\nfake\n-----END PRIVATE KEY-----", + }, + }, + }) + + // The key must be visible immediately, before any Save/reload. + require.True(t, cfg.HasKey("miren-cli"), "HasKey should see a leaf-config key before reload") + + key, err := cfg.GetKey("miren-cli") + require.NoError(t, err, "GetKey should resolve a leaf-config key before reload") + require.Equal(t, "ed25519", key.Type) +} diff --git a/clientconfig/token.go b/clientconfig/token.go new file mode 100644 index 00000000..1bd21126 --- /dev/null +++ b/clientconfig/token.go @@ -0,0 +1,369 @@ +package clientconfig + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "os" + "path/filepath" + "strings" + "time" + + "github.com/gofrs/flock" + "github.com/golang-jwt/jwt/v5" + "gopkg.in/yaml.v3" + + "miren.dev/runtime/pkg/cloudauth" +) + +// ErrLoginRequired signals that a credential has expired or been revoked and +// the user must run `miren login` again. It is returned only for definitive +// server rejections (HTTP 401), never for transient failures (5xx, network +// errors) — treating a cloud blip as a logout would stampede every CLI user +// into re-authenticating at once. +var ErrLoginRequired = errors.New("login required: session expired or revoked") + +// ErrNoBearerToken signals that an identity does not authenticate via a bearer +// token (e.g. certificate identities). Callers should fall back to their own +// credential handling rather than treating this as a failure. +var ErrNoBearerToken = errors.New("identity does not use bearer-token authentication") + +// tokenPair is the access/refresh credential returned by the cloud's token +// endpoints (device flow and /auth/refresh). +type tokenPair struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + ExpiresIn int `json:"expires_in"` +} + +// NormalizeIssuerURL ensures an auth-server URL carries a scheme, defaulting to +// https:// but using http:// for loopback hosts so local-dev clouds work. A +// URL that already has a scheme is returned with any trailing slash trimmed. +func NormalizeIssuerURL(server string) string { + if strings.HasPrefix(server, "http://") || strings.HasPrefix(server, "https://") { + return strings.TrimSuffix(server, "/") + } + if strings.Contains(server, "localhost") || strings.Contains(server, "127.0.0.1") { + return "http://" + server + } + return "https://" + server +} + +// refreshTokenPair exchanges a refresh token for a fresh access/refresh pair via +// POST {issuer}/auth/refresh. The endpoint rotates: it consumes the presented +// refresh token and returns a new one, so the caller MUST persist the returned +// pair atomically before using it. +// +// It returns ErrLoginRequired for a 401 (expired/revoked/already-spent token), +// and a plain wrapped error for transient failures (network, 5xx, malformed +// response) so callers can distinguish "must re-login" from "try again later". +func refreshTokenPair(ctx context.Context, issuer, refreshToken string) (*tokenPair, error) { + if refreshToken == "" { + return nil, ErrLoginRequired + } + + refreshURL, err := url.JoinPath(NormalizeIssuerURL(issuer), "/auth/refresh") + if err != nil { + return nil, fmt.Errorf("invalid issuer URL: %w", err) + } + + reqBody, err := json.Marshal(map[string]string{"refresh_token": refreshToken}) + if err != nil { + return nil, fmt.Errorf("failed to marshal refresh request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, refreshURL, bytes.NewReader(reqBody)) + if err != nil { + return nil, fmt.Errorf("failed to create refresh request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to send refresh request: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read refresh response: %w", err) + } + + switch resp.StatusCode { + case http.StatusOK: + // fall through to parsing below + case http.StatusUnauthorized: + // Definitive rejection: token expired, revoked, or already spent by a + // concurrent refresh. The refresh endpoint returns plain-text bodies. + return nil, ErrLoginRequired + default: + // Transient (5xx, rate-limit, etc.): keep the existing tokens. + return nil, fmt.Errorf("token refresh failed: server returned status %d: %s", + resp.StatusCode, strings.TrimSpace(string(body))) + } + + var pair tokenPair + if err := json.Unmarshal(body, &pair); err != nil { + return nil, fmt.Errorf("failed to parse refresh response: %w", err) + } + if pair.AccessToken == "" || pair.RefreshToken == "" { + return nil, fmt.Errorf("token refresh response missing access or refresh token") + } + + return &pair, nil +} + +// tokenExpiryBuffer is how long before a JWT's actual expiry we treat it as +// stale. Refreshing slightly early avoids handing a token to an RPC that then +// expires mid-flight due to clock skew or request latency. +const tokenExpiryBuffer = 5 * time.Minute + +// tokenFresh reports whether a JWT access token is safe to use for at least +// buffer longer. It parses the token WITHOUT verifying the signature — this is +// only a local staleness check, not an authorization decision; the server +// validates the signature. A token is considered stale (not fresh) when it is +// unparseable, carries no exp claim, or expires within buffer. +func tokenFresh(tokenString string, buffer time.Duration) bool { + parser := jwt.NewParser() + token, _, err := parser.ParseUnverified(tokenString, jwt.MapClaims{}) + if err != nil { + return false + } + + claims, ok := token.Claims.(jwt.MapClaims) + if !ok { + return false + } + + exp, ok := claims["exp"].(float64) + if !ok { + return false // No expiry claim, treat as stale so we fetch a fresh one. + } + + return time.Now().Unix() < int64(exp)-int64(buffer.Seconds()) +} + +// TokenForIdentity returns a bearer token to authenticate as the given identity. +// +// - keypair: re-mints a JWT via the challenge-response flow (AuthenticateWithKey). +// - token: returns the cached access token if still fresh; otherwise refreshes +// it via /auth/refresh under a cross-process lock and persists the rotated pair. +// - certificate: returns ErrNoBearerToken (the caller supplies a client cert). +// +// name identifies which identity leaf to write rotated tokens back to. Pass "" +// for an anonymous/in-memory identity (e.g. during login before it is named): +// a refresh is then used for this call only and not persisted. +// +// fallbackHost is used as the issuer when the identity carries no Issuer. +func (c *Config) TokenForIdentity(ctx context.Context, name string, identity *IdentityConfig, fallbackHost string) (string, error) { + switch identity.Type { + case IdentityKeypair: + privateKeyPEM, err := c.GetPrivateKeyPEM(identity) + if err != nil { + return "", fmt.Errorf("failed to get private key: %w", err) + } + keyPair, err := cloudauth.LoadKeyPairFromPEM(privateKeyPEM) + if err != nil { + return "", fmt.Errorf("failed to load private key: %w", err) + } + authServer := identity.Issuer + if authServer == "" { + authServer = fallbackHost + } + return AuthenticateWithKey(ctx, authServer, keyPair) + + case IdentityToken: + return c.tokenForTokenIdentity(ctx, name, identity, fallbackHost) + + case IdentityCertificate: + return "", ErrNoBearerToken + + default: + return "", fmt.Errorf("unknown identity type: %s", identity.Type) + } +} + +// tokenForTokenIdentity implements the "token" arm of TokenForIdentity: a +// lock-free fast path for a still-fresh access token, and a locked slow path +// that refreshes and persists the rotated pair. +func (c *Config) tokenForTokenIdentity(ctx context.Context, name string, identity *IdentityConfig, fallbackHost string) (string, error) { + // Fast path: a fresh cached token needs no lock and no network. + if tokenFresh(identity.Token, tokenExpiryBuffer) { + return identity.Token, nil + } + + issuer := identity.Issuer + if issuer == "" { + issuer = fallbackHost + } + + // Anonymous identity: nothing to lock or persist, just refresh in memory. + if name == "" { + pair, err := refreshTokenPair(ctx, issuer, identity.RefreshToken) + if err != nil { + return "", err + } + return pair.AccessToken, nil + } + + // Refreshing rotates the pair, so we need a file to write the new one back to. + // An identity that was never loaded from disk (an in-memory config) has none. + source := c.GetIdentitySource(name) + if source == "" { + return "", fmt.Errorf("identity %q has no config file on disk to store refreshed tokens in; run 'miren login' to save it", name) + } + + // Serialize refreshes across processes. The refresh endpoint consumes the + // presented refresh token (single-use rotation), so two racers would leave + // one holding a spent token; the lock plus the in-lock re-read collapses the + // loser into a cheap no-op. + // + // The lock file is a hidden dotfile beside the config so it doesn't clutter + // the directory a user inspects. It is intentionally left in place rather than + // removed on unlock: unlinking a flock target lets a concurrent waiter and a + // new process end up locking two different inodes, defeating the mutex. + lock := flock.New(lockPathFor(source)) + // TryLockContext rather than Lock so a cancelled command or an expiring + // deadline stops waiting promptly instead of blocking on a peer's refresh. + locked, err := lock.TryLockContext(ctx, 50*time.Millisecond) + if err != nil { + return "", fmt.Errorf("failed to acquire token refresh lock: %w", err) + } + if !locked { + return "", fmt.Errorf("failed to acquire token refresh lock for identity %q", name) + } + defer func() { _ = lock.Unlock() }() + + // Re-read from disk inside the lock: a peer may have already rotated while we + // waited, in which case its fresh access token is right there. + current, err := readIdentityFromFile(source, name) + if err != nil { + return "", fmt.Errorf("failed to re-read identity %q: %w", name, err) + } + if tokenFresh(current.Token, tokenExpiryBuffer) { + return current.Token, nil + } + + pair, err := refreshTokenPair(ctx, issuer, current.RefreshToken) + if err != nil { + return "", err + } + + // Persist the rotated pair immediately — the old refresh token is now spent + // server-side, so losing the new one would force a re-login. + if err := persistIdentityTokens(source, name, pair.AccessToken, pair.RefreshToken); err != nil { + return "", fmt.Errorf("failed to persist refreshed tokens: %w", err) + } + + return pair.AccessToken, nil +} + +// lockPathFor returns the refresh-lock path for a config file: a hidden dotfile +// in the same directory (e.g. clientconfig.d/.identity-cloud.yaml.lock). Keeping +// it beside the file means every process derives the same path; the dot prefix +// keeps it out of the way of a user inspecting the directory. Being a non-.yaml +// name, loadConfigDir ignores it regardless. +func lockPathFor(source string) string { + return filepath.Join(filepath.Dir(source), "."+filepath.Base(source)+".lock") +} + +// readIdentityFromFile reads a single identity out of a config file (leaf or +// main). It reads only the named identity so it never depends on the rest of +// the file being complete. +func readIdentityFromFile(path, name string) (*IdentityConfig, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var cd ConfigData + if err := yaml.Unmarshal(data, &cd); err != nil { + return nil, err + } + id, ok := cd.Identities[name] + if !ok || id == nil { + return nil, fmt.Errorf("identity %q not found in %s", name, path) + } + return id, nil +} + +// persistIdentityTokens writes a rotated access/refresh pair into the identity's +// source file, preserving every other field, via an atomic replace. The caller +// must hold the identity's lock. +func persistIdentityTokens(path, name, accessToken, refreshToken string) error { + data, err := os.ReadFile(path) + if err != nil { + return err + } + var cd ConfigData + if err := yaml.Unmarshal(data, &cd); err != nil { + return err + } + id, ok := cd.Identities[name] + if !ok || id == nil { + return fmt.Errorf("identity %q not found in %s", name, path) + } + id.Token = accessToken + id.RefreshToken = refreshToken + + out, err := yaml.Marshal(&cd) + if err != nil { + return err + } + return atomicWriteFile(path, out, 0600) +} + +// RevokeRefreshToken best-effort revokes a refresh token via +// POST {issuer}/auth/revoke/refresh. The endpoint requires the access token as a +// bearer credential. Any error is returned to the caller, which should treat +// revocation as advisory — a failed revoke must never block logout. +func RevokeRefreshToken(ctx context.Context, issuer, accessToken, refreshToken string) error { + if refreshToken == "" { + return nil // Nothing to revoke. + } + if accessToken == "" { + // The endpoint authenticates with the access token, so without one we + // cannot revoke. Report it rather than returning nil, which would let the + // caller claim a revocation that never happened. + return errors.New("no access token available to authorize revocation") + } + + revokeURL, err := url.JoinPath(NormalizeIssuerURL(issuer), "/auth/revoke/refresh") + if err != nil { + return fmt.Errorf("invalid issuer URL: %w", err) + } + + reqBody, err := json.Marshal(map[string]string{ + "refresh_token": refreshToken, + "reason": "logout", + }) + if err != nil { + return err + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, revokeURL, bytes.NewReader(reqBody)) + if err != nil { + return err + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+accessToken) + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, resp.Body) + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent { + return fmt.Errorf("revoke returned status %d", resp.StatusCode) + } + return nil +} diff --git a/clientconfig/token_identity_test.go b/clientconfig/token_identity_test.go new file mode 100644 index 00000000..c56eee32 --- /dev/null +++ b/clientconfig/token_identity_test.go @@ -0,0 +1,44 @@ +package clientconfig + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestTokenIdentityRoundTrip verifies a "token" identity persists its access +// and refresh tokens through a save/reload cycle and lands at mode 0600. +func TestTokenIdentityRoundTrip(t *testing.T) { + dir := t.TempDir() + t.Setenv(EnvConfigPath, dir) + + cfg := NewConfig() + cfg.SetLeafConfig("identity-cloud", &ConfigData{ + Identities: map[string]*IdentityConfig{ + "cloud": { + Type: "token", + Issuer: "https://miren.cloud", + Token: "access.jwt.value", + RefreshToken: "refresh.jwt.value", + }, + }, + }) + require.NoError(t, cfg.SaveToHome()) + + leafPath := filepath.Join(dir, "clientconfig.d", "identity-cloud.yaml") + info, err := os.Stat(leafPath) + require.NoError(t, err) + require.Equal(t, os.FileMode(0600), info.Mode().Perm()) + + reloaded, err := LoadConfig() + require.NoError(t, err) + + id, err := reloaded.GetIdentity("cloud") + require.NoError(t, err) + require.Equal(t, IdentityToken, id.Type) + require.Equal(t, "https://miren.cloud", id.Issuer) + require.Equal(t, "access.jwt.value", id.Token) + require.Equal(t, "refresh.jwt.value", id.RefreshToken) +} diff --git a/clientconfig/token_refresh_concurrency_test.go b/clientconfig/token_refresh_concurrency_test.go new file mode 100644 index 00000000..7a9a6086 --- /dev/null +++ b/clientconfig/token_refresh_concurrency_test.go @@ -0,0 +1,207 @@ +package clientconfig + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// singleUseRefreshServer emulates the cloud's rotating, single-use /auth/refresh: +// the first presentation of a given refresh token mints a fresh pair; any reuse +// of a spent token returns 401. It counts successful (minting) refreshes. +type singleUseRefreshServer struct { + mu sync.Mutex + spent map[string]bool + minted int + rotation int +} + +func newSingleUseRefreshServer() *singleUseRefreshServer { + return &singleUseRefreshServer{spent: map[string]bool{}} +} + +func freshAccessJWT(t *testing.T) string { + t.Helper() + return makeToken(t, jwt.MapClaims{"exp": float64(time.Now().Add(time.Hour).Unix())}) +} + +func (s *singleUseRefreshServer) handler(t *testing.T) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + var req struct { + RefreshToken string `json:"refresh_token"` + } + _ = json.NewDecoder(r.Body).Decode(&req) + + s.mu.Lock() + defer s.mu.Unlock() + + if s.spent[req.RefreshToken] { + http.Error(w, "Refresh token has been revoked", http.StatusUnauthorized) + return + } + s.spent[req.RefreshToken] = true + s.minted++ + s.rotation++ + newRefresh := "refresh-rot-" + itoa(s.rotation) + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": freshAccessJWT(t), + "refresh_token": newRefresh, + "token_type": "Bearer", + "expires_in": 3600, + }) + } +} + +// writeTokenIdentityConfig writes a MIREN_CONFIG dir containing a "token" +// identity whose access token is already expired, forcing a refresh. +func writeTokenIdentityConfig(t *testing.T, dir, issuer string) { + t.Helper() + expired := makeToken(t, jwt.MapClaims{"exp": float64(time.Now().Add(-time.Hour).Unix())}) + cd := ConfigData{ + Identities: map[string]*IdentityConfig{ + "cloud": { + Type: "token", + Issuer: issuer, + Token: expired, + RefreshToken: "refresh-original", + }, + }, + } + data, err := yaml.Marshal(&cd) + require.NoError(t, err) + leafDir := filepath.Join(dir, "clientconfig.d") + require.NoError(t, os.MkdirAll(leafDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(leafDir, "identity-cloud.yaml"), data, 0600)) +} + +// TestTokenForIdentityConcurrentRefresh is the load-bearing test for MIR-1385: +// many concurrent callers (goroutines AND subprocesses) sharing one expired +// access token must trigger exactly ONE successful /auth/refresh, because the +// refresh token is single-use. Every caller must still receive a valid token. +func TestTokenForIdentityConcurrentRefresh(t *testing.T) { + srv := httptest.NewServer(nil) + server := newSingleUseRefreshServer() + srv.Config.Handler = server.handler(t) + defer srv.Close() + + dir := t.TempDir() + writeTokenIdentityConfig(t, dir, srv.URL) + // Set once, before any goroutine starts: t.Setenv mutates process-global + // state and must not be called concurrently. + t.Setenv(EnvConfigPath, dir) + + const goroutines = 12 + const subprocesses = 6 + + var wg sync.WaitGroup + var goroutineFailures atomic.Int32 + tokens := make(chan string, goroutines+subprocesses) + + // In-process goroutines, each with its own freshly-loaded Config (mirrors + // separate `m` invocations that share the on-disk config). + for i := 0; i < goroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + cfg, err := LoadConfig() + if err != nil { + goroutineFailures.Add(1) + return + } + id, err := cfg.GetIdentity("cloud") + if err != nil { + goroutineFailures.Add(1) + return + } + tok, err := cfg.TokenForIdentity(context.Background(), "cloud", id, srv.URL) + if err != nil { + goroutineFailures.Add(1) + return + } + tokens <- tok + }() + } + + // Subprocesses: prove the lock is cross-process, not just cross-goroutine. + for i := 0; i < subprocesses; i++ { + wg.Add(1) + go func() { + defer wg.Done() + cmd := exec.Command(os.Args[0], "-test.run=TestTokenForIdentityChildProcess") + cmd.Env = append(os.Environ(), + "MIREN_TEST_REFRESH_CHILD=1", + EnvConfigPath+"="+dir, + "MIREN_TEST_ISSUER="+srv.URL, + ) + out, err := cmd.CombinedOutput() + if err != nil { + goroutineFailures.Add(1) + t.Logf("child failed: %v\n%s", err, out) + return + } + // Child prints the token on a line prefixed with TOKEN=. + for _, line := range strings.Split(string(out), "\n") { + if strings.HasPrefix(line, "TOKEN=") { + tokens <- strings.TrimPrefix(line, "TOKEN=") + } + } + }() + } + + wg.Wait() + close(tokens) + + require.Zero(t, goroutineFailures.Load(), "no caller should fail to obtain a token") + + server.mu.Lock() + minted := server.minted + server.mu.Unlock() + require.Equal(t, 1, minted, "the single-use refresh token must be exchanged exactly once") + + // Every caller must have received a usable (fresh) access token. + count := 0 + for tok := range tokens { + require.True(t, tokenFresh(tok, tokenExpiryBuffer), "returned token must be fresh") + count++ + } + require.Equal(t, goroutines+subprocesses, count, "every caller must return a token") + + // On-disk pair must be the rotated one and internally consistent. + final, err := readIdentityFromFile(filepath.Join(dir, "clientconfig.d", "identity-cloud.yaml"), "cloud") + require.NoError(t, err) + require.True(t, tokenFresh(final.Token, tokenExpiryBuffer)) + require.True(t, strings.HasPrefix(final.RefreshToken, "refresh-rot-"), "refresh token must be rotated") +} + +// TestTokenForIdentityChildProcess is not a real test: it is the subprocess body +// re-executed by TestTokenForIdentityConcurrentRefresh. It is inert unless the +// MIREN_TEST_REFRESH_CHILD env var is set. +func TestTokenForIdentityChildProcess(t *testing.T) { + if os.Getenv("MIREN_TEST_REFRESH_CHILD") != "1" { + return + } + issuer := os.Getenv("MIREN_TEST_ISSUER") + cfg, err := LoadConfig() + require.NoError(t, err) + id, err := cfg.GetIdentity("cloud") + require.NoError(t, err) + tok, err := cfg.TokenForIdentity(context.Background(), "cloud", id, issuer) + require.NoError(t, err) + // Stdout is captured by the parent. + os.Stdout.WriteString("TOKEN=" + tok + "\n") +} diff --git a/clientconfig/token_test.go b/clientconfig/token_test.go new file mode 100644 index 00000000..74d04d85 --- /dev/null +++ b/clientconfig/token_test.go @@ -0,0 +1,214 @@ +package clientconfig + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" +) + +// makeToken builds an unsigned JWT with the given claims. tokenFresh parses +// tokens unverified, so the signature is irrelevant to these tests. +func makeToken(t *testing.T, claims jwt.MapClaims) string { + t.Helper() + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + s, err := tok.SignedString([]byte("test-secret")) + require.NoError(t, err) + return s +} + +func TestTokenFresh(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + token string + want bool + }{ + { + name: "valid well beyond buffer", + token: makeToken(t, jwt.MapClaims{"exp": float64(now.Add(time.Hour).Unix())}), + want: true, + }, + { + name: "expiring within buffer", + token: makeToken(t, jwt.MapClaims{"exp": float64(now.Add(4 * time.Minute).Unix())}), + want: false, + }, + { + name: "already expired", + token: makeToken(t, jwt.MapClaims{"exp": float64(now.Add(-time.Minute).Unix())}), + want: false, + }, + { + name: "no exp claim", + token: makeToken(t, jwt.MapClaims{"sub": "user-1"}), + want: false, + }, + { + name: "unparseable garbage", + token: "not-a-jwt", + want: false, + }, + { + name: "empty string", + token: "", + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, tokenFresh(tc.token, tokenExpiryBuffer)) + }) + } +} + +// TestTokenFreshBufferBoundary verifies the buffer is honored: a token that +// outlives exp-buffer is fresh, one that doesn't is stale. +func TestTokenFreshBufferBoundary(t *testing.T) { + now := time.Now() + + // Expires in 10 minutes; with a 5-minute buffer it is still fresh. + fresh := makeToken(t, jwt.MapClaims{"exp": float64(now.Add(10 * time.Minute).Unix())}) + require.True(t, tokenFresh(fresh, tokenExpiryBuffer)) + + // The same token is stale under a 15-minute buffer. + require.False(t, tokenFresh(fresh, 15*time.Minute)) +} + +func TestNormalizeIssuerURL(t *testing.T) { + tests := []struct { + in, want string + }{ + {"miren.cloud", "https://miren.cloud"}, + {"https://miren.cloud", "https://miren.cloud"}, + {"https://miren.cloud/", "https://miren.cloud"}, + {"http://miren.host:3001", "http://miren.host:3001"}, + {"localhost:3001", "http://localhost:3001"}, + {"127.0.0.1:3001", "http://127.0.0.1:3001"}, + } + for _, tc := range tests { + require.Equal(t, tc.want, NormalizeIssuerURL(tc.in)) + } +} + +func TestRefreshTokenPair(t *testing.T) { + t.Run("200 rotates and returns new pair", func(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"access_token":"new-access","refresh_token":"new-refresh","token_type":"Bearer","expires_in":3600}`)) + })) + defer srv.Close() + + pair, err := refreshTokenPair(context.Background(), srv.URL, "old-refresh") + require.NoError(t, err) + require.Equal(t, "/auth/refresh", gotPath) + require.Equal(t, "new-access", pair.AccessToken) + require.Equal(t, "new-refresh", pair.RefreshToken) + require.NotEqual(t, "old-refresh", pair.RefreshToken, "server must rotate the refresh token") + require.Equal(t, 3600, pair.ExpiresIn) + }) + + t.Run("401 means login required", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "Refresh token has been revoked", http.StatusUnauthorized) + })) + defer srv.Close() + + _, err := refreshTokenPair(context.Background(), srv.URL, "spent-refresh") + require.ErrorIs(t, err, ErrLoginRequired) + }) + + t.Run("500 is transient, NOT login required", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "Token validation error", http.StatusInternalServerError) + })) + defer srv.Close() + + _, err := refreshTokenPair(context.Background(), srv.URL, "some-refresh") + require.Error(t, err) + require.NotErrorIs(t, err, ErrLoginRequired, "a 5xx must not force a logout") + }) + + t.Run("200 with empty access token errors", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"access_token":"","refresh_token":"r","token_type":"Bearer","expires_in":3600}`)) + })) + defer srv.Close() + + _, err := refreshTokenPair(context.Background(), srv.URL, "some-refresh") + require.Error(t, err) + require.NotErrorIs(t, err, ErrLoginRequired) + }) + + t.Run("empty refresh token short-circuits to login required", func(t *testing.T) { + _, err := refreshTokenPair(context.Background(), "https://miren.cloud", "") + require.ErrorIs(t, err, ErrLoginRequired) + }) + + t.Run("network failure is transient", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + url := srv.URL + srv.Close() // nothing listening now + + _, err := refreshTokenPair(context.Background(), url, "some-refresh") + require.Error(t, err) + require.False(t, errors.Is(err, ErrLoginRequired), "a network failure must not force a logout") + }) +} + +func TestRevokeRefreshToken(t *testing.T) { + t.Run("posts bearer and refresh token", func(t *testing.T) { + var path, auth, gotRefresh, gotReason string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path = r.URL.Path + auth = r.Header.Get("Authorization") + var body struct { + RefreshToken string `json:"refresh_token"` + Reason string `json:"reason"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + gotRefresh, gotReason = body.RefreshToken, body.Reason + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + err := RevokeRefreshToken(context.Background(), srv.URL, "access-x", "refresh-y") + require.NoError(t, err) + require.Equal(t, "/auth/revoke/refresh", path) + require.Equal(t, "Bearer access-x", auth) + require.Equal(t, "refresh-y", gotRefresh) + require.Equal(t, "logout", gotReason) + }) + + t.Run("no refresh token is a no-op", func(t *testing.T) { + require.NoError(t, RevokeRefreshToken(context.Background(), "https://x", "access", "")) + }) + + t.Run("missing access token reports failure rather than silent success", func(t *testing.T) { + // Returning nil here would let the caller claim it revoked a token when + // no request was ever made. + err := RevokeRefreshToken(context.Background(), "https://x", "", "refresh-y") + require.Error(t, err) + require.Contains(t, err.Error(), "no access token") + }) + + t.Run("non-2xx is an error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + require.Error(t, RevokeRefreshToken(context.Background(), srv.URL, "a", "b")) + }) +} diff --git a/docs/docs/command/login.md b/docs/docs/command/login.md index 76af04ea..27729a71 100644 --- a/docs/docs/command/login.md +++ b/docs/docs/command/login.md @@ -20,6 +20,7 @@ miren login [flags] - `--identity, -i` — Name for this identity in config (default: `cloud`) - `--key-name, -k` — Name for the authentication key (default: `miren-cli`) - `--no-save` — Don't save credentials to config file +- `--persistent-key` — Register a persistent key instead of the default renewable token login - `--url, -u` — Cloud URL (default: `https://miren.cloud`) ## Global Options diff --git a/go.mod b/go.mod index 2820276f..65913674 100644 --- a/go.mod +++ b/go.mod @@ -245,7 +245,7 @@ require ( github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-yaml v1.18.0 // indirect github.com/godbus/dbus/v5 v5.1.0 // indirect - github.com/gofrs/flock v0.13.0 // indirect + github.com/gofrs/flock v0.13.0 github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect