From e024348c1f7e9fff21fa44ffacfec1b48b185402 Mon Sep 17 00:00:00 2001 From: LinYushen Date: Fri, 29 May 2026 15:55:09 +0800 Subject: [PATCH] fix(cli/login): accept mcn_ Cloud Node PATs alongside mul_ (MUL-2815) (#3518) * fix(cli/login): accept mcn_ Cloud Node PATs alongside mul_ (MUL-2815) multica login --token rejected anything not starting with mul_, so users with a Multica Cloud Node PAT (mcn_ prefix) hit "invalid token format: must start with mul_" even though the server middleware verifies both kinds. Replace the inline literal check with validateLoginTokenPrefix(), backed by a small loginTokenPrefixes list ({mul_, auth.CloudPATPrefix}) so the accepted set has one source of truth. Add unit-test coverage so adding a new prefix in future is an obvious one-line edit. Co-authored-by: multica-agent * fix(cli/login): mention mcn_ Cloud Node PATs in --token help and comments Follow-up to 47e423c4: the login command now accepts mcn_ tokens but the help string and surrounding comments still only documented mul_, so a user running 'multica login --help' couldn't tell that mcn_ was supported. Update the --token help string and the cobra Args / NoOptDefVal comments to list both mul_... and mcn_... prefixes. Co-authored-by: multica-agent --------- Co-authored-by: multica-agent --- server/cmd/multica/cmd_auth.go | 24 ++++++++++++-- server/cmd/multica/cmd_auth_test.go | 50 +++++++++++++++++++++++++++++ server/cmd/multica/cmd_login.go | 13 +++++--- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/server/cmd/multica/cmd_auth.go b/server/cmd/multica/cmd_auth.go index 55cedfad3..d0cf1390a 100644 --- a/server/cmd/multica/cmd_auth.go +++ b/server/cmd/multica/cmd_auth.go @@ -17,9 +17,29 @@ import ( "github.com/spf13/cobra" + "github.com/multica-ai/multica/server/internal/auth" "github.com/multica-ai/multica/server/internal/cli" ) +// loginTokenPrefixes are the token prefixes `multica login --token` accepts. +// The CLI used to hardcode `mul_` only, which made it impossible to log in +// with a Multica Cloud Node PAT (`mcn_`) even though the server happily +// authenticates both kinds. Keep this list in sync with the prefix branches +// in server/internal/middleware/auth.go. +var loginTokenPrefixes = []string{"mul_", auth.CloudPATPrefix} + +// validateLoginTokenPrefix returns nil if token starts with one of the +// CLI-recognised PAT prefixes, or an error describing the accepted set. +// Extracted so the prefix list has one obvious test surface. +func validateLoginTokenPrefix(token string) error { + for _, p := range loginTokenPrefixes { + if strings.HasPrefix(token, p) { + return nil + } + } + return fmt.Errorf("invalid token format: must start with %s", strings.Join(loginTokenPrefixes, " or ")) +} + var authCmd = &cobra.Command{ Use: "auth", Short: "Authenticate multica with Multica", @@ -346,8 +366,8 @@ func runAuthLoginToken(cmd *cobra.Command, providedToken string) error { if token == "" { return fmt.Errorf("token is required") } - if !strings.HasPrefix(token, "mul_") { - return fmt.Errorf("invalid token format: must start with mul_") + if err := validateLoginTokenPrefix(token); err != nil { + return err } serverURL := resolveServerURL(cmd) diff --git a/server/cmd/multica/cmd_auth_test.go b/server/cmd/multica/cmd_auth_test.go index 430fbb3fb..75d4b1b5e 100644 --- a/server/cmd/multica/cmd_auth_test.go +++ b/server/cmd/multica/cmd_auth_test.go @@ -2,6 +2,7 @@ package main import ( "net" + "strings" "testing" "github.com/spf13/cobra" @@ -244,3 +245,52 @@ func TestNormalizeAPIBaseURL(t *testing.T) { } }) } + +// TestValidateLoginTokenPrefix pins the accepted PAT prefix set for +// `multica login --token`. The original implementation hardcoded `mul_` +// only, which rejected legitimate Multica Cloud Node PATs (`mcn_`) at +// the CLI even though the server's middleware would have accepted them. +// If a future change drops `mcn_` from the list (or accidentally +// broadens the set to anything-goes), this test fails. +func TestValidateLoginTokenPrefix(t *testing.T) { + cases := []struct { + name string + token string + wantErr bool + }{ + {name: "mul_ PAT", token: "mul_abc123", wantErr: false}, + {name: "mcn_ Cloud Node PAT", token: "mcn_abc123", wantErr: false}, + {name: "empty token", token: "", wantErr: true}, + {name: "no prefix", token: "abc123", wantErr: true}, + {name: "wrong prefix mdt_", token: "mdt_abc123", wantErr: true}, + {name: "wrong prefix mat_", token: "mat_abc123", wantErr: true}, + {name: "case-sensitive: MUL_ rejected", token: "MUL_abc123", wantErr: true}, + {name: "leading whitespace not allowed (callers TrimSpace first)", token: " mul_abc", wantErr: true}, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + err := validateLoginTokenPrefix(tc.token) + if tc.wantErr && err == nil { + t.Fatalf("validateLoginTokenPrefix(%q) = nil, want error", tc.token) + } + if !tc.wantErr && err != nil { + t.Fatalf("validateLoginTokenPrefix(%q) = %v, want nil", tc.token, err) + } + }) + } + + // The error string is user-facing; make sure it lists every accepted + // prefix so users hitting it can self-serve. Hardcoding the exact + // prefixes here is deliberate — if someone adds a new prefix to + // loginTokenPrefixes they should also update the docs / this test. + err := validateLoginTokenPrefix("nope_xxx") + if err == nil { + t.Fatal("expected error for unknown prefix") + } + for _, p := range []string{"mul_", "mcn_"} { + if !strings.Contains(err.Error(), p) { + t.Errorf("error %q does not mention prefix %q", err.Error(), p) + } + } +} diff --git a/server/cmd/multica/cmd_login.go b/server/cmd/multica/cmd_login.go index 272743d9c..db26c46f5 100644 --- a/server/cmd/multica/cmd_login.go +++ b/server/cmd/multica/cmd_login.go @@ -32,8 +32,9 @@ var loginCmd = &cobra.Command{ Use: "login", Short: "Authenticate and set up workspaces", Long: "Log in to Multica, then automatically discover and watch all your workspaces.", - // Up to one positional is accepted so `--token mul_...` (space form) can - // recover the token in runAuthLogin even though pflag won't bind it. + // Up to one positional is accepted so `--token mul_...` / `--token mcn_...` + // (space form) can recover the token in runAuthLogin even though pflag + // won't bind it. Args: cobra.MaximumNArgs(1), RunE: runLogin, } @@ -41,13 +42,15 @@ var loginCmd = &cobra.Command{ // tokenPromptSentinel is the value pflag assigns to `--token` when the flag // is supplied without an explicit value. runAuthLoginToken treats it as // "prompt me interactively", preserving the legacy `multica login --token` -// no-value form alongside the documented `--token mul_...` value form. +// no-value form alongside the documented `--token mul_...` / `--token mcn_...` +// value form. const tokenPromptSentinel = "\x00prompt" func init() { - loginCmd.Flags().String("token", "", "Authenticate using a personal access token. Pass `--token mul_...` to supply it inline, or `--token` alone to be prompted interactively.") + loginCmd.Flags().String("token", "", "Authenticate using a personal access token (`mul_...` user PAT or `mcn_...` Cloud Node PAT). Pass `--token mul_...` / `--token mcn_...` to supply it inline, or `--token` alone to be prompted interactively.") // NoOptDefVal lets `--token` (no value) keep its old prompt-mode behavior - // while `--token mul_...` and `--token=mul_...` consume the value normally. + // while `--token mul_...` / `--token mcn_...` and the `=value` form + // consume the value normally. loginCmd.Flags().Lookup("token").NoOptDefVal = tokenPromptSentinel loginCmd.Flags().String(callbackHostFlag, "", "Host the OAuth callback URL points at (auto-detected from the server's route when empty). Use this for reverse-proxy / FQDN setups where auto-detection picks the wrong interface.") }