diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 1dbf18a..4d22917 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -1,6 +1,7 @@ package commands import ( + "context" "encoding/json" stderrors "errors" "fmt" @@ -101,12 +102,15 @@ var authLoginCmd = &cobra.Command{ var authLogoutCmd = &cobra.Command{ Use: "logout", Short: "Remove saved credentials", - Long: "Removes saved credentials for the current profile (or all profiles with --all).", + Long: "Removes saved credentials for the current profile (or all profiles with --all).\n\n" + + "A token fizzy signup created is revoked at Fizzy first, best effort: if Fizzy can't confirm it, " + + "the credential is still removed and a warning says so. A token given to fizzy auth login is only " + + "removed, never revoked, since it may be in use elsewhere; revoke it in Fizzy's access tokens.", RunE: func(cmd *cobra.Command, args []string) error { all, _ := cmd.Flags().GetBool("all") if all { - return authLogoutAll() + return authLogoutAll(commandContext(cmd)) } profileName := currentProfileName() @@ -118,13 +122,20 @@ var authLogoutCmd = &cobra.Command{ // which identity remains active. wasDefault := false profileExists := false + var revocation mintedTokenRevocation if profiles != nil { allProfiles, defaultName, err := profiles.List() if err != nil { return &output.Error{Code: output.CodeAPI, Message: err.Error()} } - _, profileExists = allProfiles[profileName] + var p *profile.Profile + p, profileExists = allProfiles[profileName] wasDefault = defaultName == profileName + // Before anything is deleted: the token to revoke is read from + // where it is stored. + ctx, cancel := context.WithTimeout(commandContext(cmd), mintedTokenLogoutTimeout) + revocation = revokeProfileMintedToken(ctx, profileName, p) + cancel() } var cleanupErrors []error @@ -157,26 +168,41 @@ var authLogoutCmd = &cobra.Command{ breadcrumb("login", "fizzy auth login ", "Log in again"), } - printMutation(map[string]any{ + result := map[string]any{ "authenticated": false, "profile": profileName, "message": "Logged out successfully", - }, "", breadcrumbs) + } + reportMintedTokenRevocations(result, map[string]mintedTokenRevocation{profileName: revocation}) + printMutation(result, "", breadcrumbs) return nil }, } -func authLogoutAll() error { +// commandContext is cmd's context, or the background one for a command run +// without one (RunE called directly). +func commandContext(cmd *cobra.Command) context.Context { + if ctx := cmd.Context(); ctx != nil { + return ctx + } + return context.Background() +} + +func authLogoutAll(ctx context.Context) error { profileNames := map[string]bool{} credentialNames := map[string]bool{} var cleanupErrors []error + revocations := map[string]mintedTokenRevocation{} + ctx, cancel := context.WithTimeout(ctx, mintedTokenLogoutTimeout) + defer cancel() if profiles != nil { allProfiles, _, err := profiles.List() if err != nil { return &output.Error{Code: output.CodeAPI, Message: err.Error()} } for name, p := range allProfiles { + revocations[name] = revokeProfileMintedToken(ctx, name, p) profileNames[name] = true credentialNames[name] = true binding, err := resolveProfileAccountBinding(name, p) @@ -222,10 +248,12 @@ func authLogoutAll() error { breadcrumb("signup", "fizzy signup", "Sign up"), } - printMutation(map[string]any{ + result := map[string]any{ "authenticated": false, "message": "Logged out of all profiles", - }, "", breadcrumbs) + } + reportMintedTokenRevocations(result, revocations) + printMutation(result, "", breadcrumbs) return nil } diff --git a/internal/commands/auth_test.go b/internal/commands/auth_test.go index a2354ac..7c2ddbe 100644 --- a/internal/commands/auth_test.go +++ b/internal/commands/auth_test.go @@ -884,7 +884,7 @@ func TestAuthLogoutAllReportsCredentialDeletionFailure(t *testing.T) { } SetTestCreds(store) SetTestProfiles(profileStore) - err := authLogoutAll() + err := authLogoutAll(context.Background()) if removeErr := os.Remove(credDir); removeErr != nil { t.Fatalf("unblock credential directory: %v", removeErr) } @@ -928,7 +928,7 @@ func TestAuthLogoutAllCleansInvalidProfileMetadata(t *testing.T) { SetTestProfiles(profileStore) defer resetTest() - if err := authLogoutAll(); err != nil { + if err := authLogoutAll(context.Background()); err != nil { t.Fatalf("logout all: %v", err) } if _, err := store.Load("profile:agent"); err == nil { diff --git a/internal/commands/minted_token.go b/internal/commands/minted_token.go new file mode 100644 index 0000000..a3ac742 --- /dev/null +++ b/internal/commands/minted_token.go @@ -0,0 +1,213 @@ +package commands + +import ( + "context" + "crypto/sha256" + "crypto/subtle" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "os" + "sort" + "strings" + "time" + + "github.com/basecamp/cli/profile" + "github.com/basecamp/fizzy-cli/internal/config" +) + +// A token `fizzy signup` created is the CLI's own: nobody pasted it, so it +// lives nowhere else, and a logout that only forgot it would leave a live +// write token behind on the server. Logout revokes those. A token handed to +// `fizzy auth login` is not the CLI's to revoke — the same token may be in a +// password manager, CI or another tool — so logout only forgets it. +// +// The profile records which token the CLI created by its SHA-256, never the +// token itself. A mark that outlives its token (a later login on the same +// profile through a path that doesn't reset it) matches nothing, so it can +// never revoke a pasted token. +const mintedTokenExtraKey = "minted_token_sha256" //nolint:gosec // G101: a metadata key, not a credential + +// mintedTokenRevocationTimeout bounds each revocation request, and +// mintedTokenLogoutTimeout all of one logout's together (--all revokes one +// per profile): a logout must not hang on a slow server, and it forgets the +// tokens locally regardless. +var ( + mintedTokenRevocationTimeout = 10 * time.Second + mintedTokenLogoutTimeout = 20 * time.Second +) + +// mintedTokenRevocationClient sends the revocation. It follows no redirect: +// the token goes to the Fizzy that minted it and nowhere else, and a redirect +// is reported as a failed revocation. Replaceable in tests. +var mintedTokenRevocationClient = newMintedTokenRevocationClient + +func newMintedTokenRevocationClient() *http.Client { + return &http.Client{ + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } +} + +func mintedTokenDigest(token string) string { + sum := sha256.Sum256([]byte(token)) + return hex.EncodeToString(sum[:]) +} + +// setMintedTokenMark records token as CLI-created in a profile's extra +// metadata when minted, and drops any earlier mark when not. +func setMintedTokenMark(extra map[string]json.RawMessage, token string, minted bool) { + if !minted || token == "" { + delete(extra, mintedTokenExtraKey) + return + } + extra[mintedTokenExtraKey], _ = json.Marshal(mintedTokenDigest(token)) +} + +// mintedTokenMark is the digest a profile recorded for the token the CLI +// created, or "" when it recorded none. +func mintedTokenMark(p *profile.Profile) string { + if p == nil || p.Extra == nil { + return "" + } + var mark string + if raw, ok := p.Extra[mintedTokenExtraKey]; !ok || json.Unmarshal(raw, &mark) != nil { + return "" + } + return mark +} + +// mintedTokenFor returns the first candidate whose digest is mark, or "". +func mintedTokenFor(mark string, candidates ...string) string { + if mark == "" { + return "" + } + for _, token := range candidates { + if token != "" && subtle.ConstantTimeCompare([]byte(mintedTokenDigest(token)), []byte(mark)) == 1 { + return token + } + } + return "" +} + +// storedTokenCandidates are the places a profile's token can be stored: its +// credential-store entry, and the global config file signup falls back to +// when the credential store is unavailable. A credential store that can't be +// read is returned as an error beside the candidates still found: the token +// may be there, and live. +func storedTokenCandidates(profileName string) ([]string, error) { + var candidates []string + var loadErr error + if creds != nil { + token, err := credsLoadProfileToken(profileName) + switch { + case err == nil: + candidates = append(candidates, token) + case !isCredentialNotFound(err): + loadErr = fmt.Errorf("reading the stored token: %w", err) + } + } + return append(candidates, config.LoadGlobal().Token), loadErr +} + +// errMintedTokenNotStored is a profile marked with a signup token that holds +// none of the tokens stored for it. +var errMintedTokenNotStored = errors.New("the token is no longer stored here, so it can't be revoked from the CLI") + +// mintedTokenRevocation is what became of a profile's CLI-created token. +type mintedTokenRevocation struct { + // Attempted: the profile records a token the CLI created. + Attempted bool + // Err is why an attempted revocation did not go through. + Err error +} + +// revokeProfileMintedToken revokes the token the CLI created for p, if it +// still holds one, at the profile's Fizzy, and warns on stderr when that +// doesn't go through. Best effort: the caller forgets the token locally +// whatever happens here, so the warning comes before any cleanup can fail. +func revokeProfileMintedToken(ctx context.Context, name string, p *profile.Profile) mintedTokenRevocation { + mark := mintedTokenMark(p) + if mark == "" { + return mintedTokenRevocation{} + } + candidates, loadErr := storedTokenCandidates(name) + token := mintedTokenFor(mark, candidates...) + switch { + case token == "" && loadErr != nil: + return warnMintedTokenRevocation(name, mintedTokenRevocation{Attempted: true, Err: loadErr}) + case token == "": + // Marked, but the token isn't stored any more (a keyring reset, a + // downgrade, a save that didn't drop the mark): it may still be live, + // and this logout drops the last record of it. + return warnMintedTokenRevocation(name, mintedTokenRevocation{Attempted: true, Err: errMintedTokenNotStored}) + } + baseURL := config.DefaultAPIURL + if p.BaseURL != "" { + baseURL = p.BaseURL + } + return warnMintedTokenRevocation(name, mintedTokenRevocation{Attempted: true, Err: revokeToken(ctx, baseURL, token)}) +} + +func warnMintedTokenRevocation(name string, r mintedTokenRevocation) mintedTokenRevocation { + if r.Err != nil { + fmt.Fprintf(os.Stderr, "Warning: could not revoke the token fizzy signup created for %q (%v); it stays valid until you revoke it in Fizzy's access tokens\n", name, r.Err) + } + return r +} + +// reportMintedTokenRevocations adds to a logout's result whether the tokens +// the CLI created were revoked: revoked is true only when at least one was +// due and every one went through, and revoke_error names each that didn't. +func reportMintedTokenRevocations(result map[string]any, revocations map[string]mintedTokenRevocation) { + attempted := false + var failures []string + for name, r := range revocations { + if !r.Attempted { + continue + } + attempted = true + if r.Err != nil { + failures = append(failures, fmt.Sprintf("%s: %v", name, r.Err)) + } + } + if len(failures) > 0 { + sort.Strings(failures) + result["revoke_error"] = strings.Join(failures, "; ") + } + result["revoked"] = attempted && len(failures) == 0 +} + +// revokeToken asks Fizzy to revoke token (RFC 7009). Fizzy's revocation +// endpoint takes a personal access token on possession alone and answers 200 +// whatever the token's state; any other answer is reported, never the token. +func revokeToken(ctx context.Context, baseURL, token string) error { + endpoint := strings.TrimSuffix(baseURL, "/") + "/oauth/revocation" + if err := validateSignupURL(endpoint); err != nil { + return err + } + ctx, cancel := context.WithTimeout(ctx, mintedTokenRevocationTimeout) + defer cancel() + form := url.Values{"token": {token}, "token_type_hint": {"access_token"}} + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(form.Encode())) + if err != nil { + return fmt.Errorf("creating revocation request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", fmt.Sprintf("fizzy-cli/%s", cliVersion)) + + resp, err := mintedTokenRevocationClient().Do(req) + if err != nil { + return fmt.Errorf("revocation request failed: %w", err) + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, signupMaxResponseBytes)) + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("the revocation was answered with HTTP %d", resp.StatusCode) + } + return nil +} diff --git a/internal/commands/minted_token_test.go b/internal/commands/minted_token_test.go new file mode 100644 index 0000000..c87b487 --- /dev/null +++ b/internal/commands/minted_token_test.go @@ -0,0 +1,386 @@ +package commands + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/basecamp/cli/credstore" + "github.com/basecamp/cli/profile" + "github.com/basecamp/fizzy-cli/internal/config" +) + +// fakeFizzyRevocation is Fizzy's POST /oauth/revocation, answering status and +// recording each token it was asked to revoke. +type fakeFizzyRevocation struct { + t *testing.T + status int + mu sync.Mutex + tokens []string +} + +func (f *fakeFizzyRevocation) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/oauth/revocation" { + f.t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + return + } + raw, _ := io.ReadAll(r.Body) + form, _ := url.ParseQuery(string(raw)) + f.mu.Lock() + f.tokens = append(f.tokens, form.Get("token")) + f.mu.Unlock() + w.WriteHeader(f.status) +} + +func (f *fakeFizzyRevocation) revoked() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.tokens...) +} + +type mintedTokenRig struct { + server *httptest.Server + fizzy *fakeFizzyRevocation + store *credstore.Store + credDir string + profiles *profile.Store +} + +// newMintedTokenRig points the credential, profile and config stores at +// temporary directories and the revocation at a fake Fizzy answering status. +func newMintedTokenRig(t *testing.T, status int) *mintedTokenRig { + t.Helper() + fizzy := &fakeFizzyRevocation{t: t, status: status} + server := httptest.NewServer(fizzy) + t.Cleanup(server.Close) + + configDir := t.TempDir() + config.SetTestConfigDir(configDir) + t.Cleanup(config.ResetTestConfigDir) + t.Setenv("FIZZY_MINTED_TOKEN_NO_KR", "1") + credDir := filepath.Join(t.TempDir(), "credentials") + store := credstore.NewStore(credstore.StoreOptions{ + ServiceName: "fizzy-minted-token-test", + DisableEnvVar: "FIZZY_MINTED_TOKEN_NO_KR", + FallbackDir: credDir, + }) + profileStore := profile.NewStore(filepath.Join(configDir, "config.json")) + + SetTestModeWithSDK(NewMockClient()) + SetTestCreds(store) + SetTestProfiles(profileStore) + t.Cleanup(resetTest) + + original := mintedTokenRevocationClient + mintedTokenRevocationClient = func() *http.Client { return server.Client() } + t.Cleanup(func() { mintedTokenRevocationClient = original }) + + return &mintedTokenRig{server: server, fizzy: fizzy, store: store, credDir: credDir, profiles: profileStore} +} + +func (r *mintedTokenRig) logout(t *testing.T, all bool) map[string]any { + t.Helper() + flag := "false" + if all { + flag = "true" + } + authLogoutCmd.Flags().Set("all", flag) + t.Cleanup(func() { authLogoutCmd.Flags().Set("all", "false") }) + if err := authLogoutCmd.RunE(authLogoutCmd, nil); err != nil { + t.Fatalf("logout: %v", err) + } + if lastResult == nil || lastResult.Response == nil { + t.Fatal("logout printed no response") + } + data, _ := lastResult.Response.Data.(map[string]any) + return data +} + +// captureStderr returns what fn writes to os.Stderr, restoring it even when fn +// fails the test. +func captureStderr(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + oldStderr := os.Stderr + os.Stderr = w + t.Cleanup(func() { os.Stderr = oldStderr }) + done := make(chan []byte) + go func() { b, _ := io.ReadAll(r); done <- b }() + fn() + os.Stderr = oldStderr + w.Close() + return string(<-done) +} + +func TestLogoutRevokesTheTokenSignupCreated(t *testing.T) { + rig := newMintedTokenRig(t, http.StatusOK) + if err := saveSignupConfig("minted-token", "acct", rig.server.URL); err != nil { + t.Fatalf("save signup config: %v", err) + } + p, err := rig.profiles.Get("acct") + if err != nil { + t.Fatalf("get profile: %v", err) + } + if strings.Contains(string(p.Extra[mintedTokenExtraKey]), "minted-token") { + t.Fatal("the profile records the token itself, not its digest") + } + SetTestConfig("minted-token", "acct", rig.server.URL) + + data := rig.logout(t, false) + if got := rig.fizzy.revoked(); len(got) != 1 || got[0] != "minted-token" { + t.Errorf("revoked %d tokens, want the signup token once", len(got)) + } + if data["revoked"] != true { + t.Errorf("logout data = %v, want revoked", data) + } + if _, err := rig.store.Load("profile:acct"); err == nil { + t.Error("the token is still stored after logout") + } +} + +// A token handed to `fizzy auth login` may be in use elsewhere: logout only +// forgets it. So does a profile whose signup token was replaced by a login. +func TestLogoutOnlyForgetsATokenTheCLIDidNotCreate(t *testing.T) { + for name, setup := range map[string]func(*testing.T, *mintedTokenRig){ + "pasted token": func(t *testing.T, rig *mintedTokenRig) { + if _, err := saveProfileCredentialState(profileCredentialSaveOptions{ + ProfileName: "acct", Account: "acct", BaseURL: rig.server.URL, Token: "pasted-token", + }); err != nil { + t.Fatal(err) + } + }, + "signup token replaced by a login": func(t *testing.T, rig *mintedTokenRig) { + if err := saveSignupConfig("minted-token", "acct", rig.server.URL); err != nil { + t.Fatal(err) + } + if _, err := saveProfileCredentialState(profileCredentialSaveOptions{ + ProfileName: "acct", Account: "acct", BaseURL: rig.server.URL, Token: "pasted-token", + }); err != nil { + t.Fatal(err) + } + }, + "stale mark beside a pasted token": func(t *testing.T, rig *mintedTokenRig) { + // A mark left behind by a path that stores a token without + // resetting it matches nothing. + mark, _ := json.Marshal(mintedTokenDigest("minted-token")) + if err := rig.profiles.Create(&profile.Profile{ + Name: "acct", BaseURL: rig.server.URL, Extra: map[string]json.RawMessage{mintedTokenExtraKey: mark}, + }); err != nil { + t.Fatal(err) + } + if err := credsSaveProfileToken("acct", "pasted-token"); err != nil { + t.Fatal(err) + } + }, + } { + t.Run(name, func(t *testing.T) { + rig := newMintedTokenRig(t, http.StatusOK) + setup(t, rig) + SetTestConfig("pasted-token", "acct", rig.server.URL) + + var data map[string]any + stderr := captureStderr(t, func() { data = rig.logout(t, false) }) + stale := name == "stale mark beside a pasted token" + if warned := strings.Contains(stderr, "no longer stored here"); warned != stale { + t.Errorf("stderr = %q, want the no-longer-stored warning only for a stale mark", stderr) + } + if got := rig.fizzy.revoked(); len(got) != 0 { + t.Errorf("revoked %d tokens, want none", len(got)) + } + if data["revoked"] != false { + t.Errorf("logout data = %v, want revoked false", data) + } + if _, err := rig.store.Load("profile:acct"); err == nil { + t.Error("the token is still stored after logout") + } + }) + } +} + +// A Fizzy that can't confirm the revocation (an outage, a server without the +// endpoint) still gets the token forgotten here, and the failure reported. +func TestLogoutForgetsTheSignupTokenWhenRevocationFails(t *testing.T) { + for _, status := range []int{http.StatusInternalServerError, http.StatusNotFound} { + t.Run(http.StatusText(status), func(t *testing.T) { + rig := newMintedTokenRig(t, status) + if err := saveSignupConfig("minted-token", "acct", rig.server.URL); err != nil { + t.Fatal(err) + } + SetTestConfig("minted-token", "acct", rig.server.URL) + + var data map[string]any + stderr := captureStderr(t, func() { data = rig.logout(t, false) }) + + if data["revoked"] != false || data["revoke_error"] == nil { + t.Errorf("logout data = %v, want revoked false with the error", data) + } + if !strings.Contains(stderr, "could not revoke the token fizzy signup created") { + t.Errorf("stderr = %q, want the revocation warning", stderr) + } + if strings.Contains(stderr, "minted-token") || strings.Contains(fmt.Sprint(data["revoke_error"]), "minted-token") { + t.Error("the warning printed the token") + } + if _, err := rig.store.Load("profile:acct"); err == nil { + t.Error("the token is still stored after logout") + } + }) + } +} + +func TestLogoutAllRevokesEverySignupToken(t *testing.T) { + rig := newMintedTokenRig(t, http.StatusOK) + if err := saveSignupConfig("minted-one", "one", rig.server.URL); err != nil { + t.Fatal(err) + } + if err := saveSignupConfig("minted-two", "two", rig.server.URL); err != nil { + t.Fatal(err) + } + if _, err := saveProfileCredentialState(profileCredentialSaveOptions{ + ProfileName: "three", Account: "three", BaseURL: rig.server.URL, Token: "pasted-three", + }); err != nil { + t.Fatal(err) + } + SetTestConfig("pasted-three", "three", rig.server.URL) + + data := rig.logout(t, true) + got := strings.Join(rig.fizzy.revoked(), ",") + if len(rig.fizzy.revoked()) != 2 || !strings.Contains(got, "minted-one") || !strings.Contains(got, "minted-two") { + t.Errorf("revoked %d tokens, want both signup tokens and not the pasted one", len(rig.fizzy.revoked())) + } + if data["revoked"] != true { + t.Errorf("logout data = %v, want revoked", data) + } +} + +// One revocation through and another refused is not a revoked logout. +func TestLogoutAllReportsAPartialRevocationAsNotRevoked(t *testing.T) { + ok := newMintedTokenRig(t, http.StatusOK) + refusing := &fakeFizzyRevocation{t: t, status: http.StatusInternalServerError} + refusingServer := httptest.NewServer(refusing) + defer refusingServer.Close() + if err := saveSignupConfig("minted-one", "one", ok.server.URL); err != nil { + t.Fatal(err) + } + if err := saveSignupConfig("minted-two", "two", refusingServer.URL); err != nil { + t.Fatal(err) + } + mintedTokenRevocationClient = func() *http.Client { return &http.Client{} } + SetTestConfig("minted-two", "two", refusingServer.URL) + + var data map[string]any + stderr := captureStderr(t, func() { data = ok.logout(t, true) }) + if len(ok.fizzy.revoked()) != 1 || len(refusing.revoked()) != 1 { + t.Fatalf("revocations = %d and %d, want one at each Fizzy", len(ok.fizzy.revoked()), len(refusing.revoked())) + } + if data["revoked"] != false || !strings.Contains(fmt.Sprint(data["revoke_error"]), "two:") { + t.Errorf("logout data = %v, want revoked false naming profile two", data) + } + if !strings.Contains(stderr, `"two"`) { + t.Errorf("stderr = %q, want a warning for profile two", stderr) + } +} + +// The token goes to the Fizzy that minted it: a redirect is a failed +// revocation, and its target hears nothing. +func TestLogoutDoesNotFollowARevocationRedirect(t *testing.T) { + rig := newMintedTokenRig(t, http.StatusOK) + target := &fakeFizzyRevocation{t: t, status: http.StatusOK} + targetServer := httptest.NewServer(target) + defer targetServer.Close() + redirecting := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, targetServer.URL+"/oauth/revocation", http.StatusTemporaryRedirect) + })) + defer redirecting.Close() + // The production client, not the rig's: its redirect policy is under test. + mintedTokenRevocationClient = newMintedTokenRevocationClient + if err := saveSignupConfig("minted-token", "acct", redirecting.URL); err != nil { + t.Fatal(err) + } + SetTestConfig("minted-token", "acct", redirecting.URL) + + var data map[string]any + captureStderr(t, func() { data = rig.logout(t, false) }) + if data["revoked"] != false || data["revoke_error"] == nil { + t.Errorf("logout data = %v, want revoked false with the error", data) + } + if len(target.revoked()) != 0 { + t.Error("the redirect target received the token") + } +} + +// A credential store that can't be read may still hold a live signup token: +// that is a failed revocation, warned about, not a token that isn't there. +// The warning comes before the cleanup, which fails here too. +func TestLogoutWarnsWhenTheSignupTokenCannotBeRead(t *testing.T) { + rig := newMintedTokenRig(t, http.StatusOK) + if err := saveSignupConfig("minted-token", "acct", rig.server.URL); err != nil { + t.Fatal(err) + } + SetTestConfig("minted-token", "acct", rig.server.URL) + backup := rig.credDir + "-backup" + if err := os.Rename(rig.credDir, backup); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(rig.credDir, []byte("not a directory"), 0600); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Remove(rig.credDir); os.Rename(backup, rig.credDir) }) + + var err error + stderr := captureStderr(t, func() { + authLogoutCmd.Flags().Set("all", "false") + err = authLogoutCmd.RunE(authLogoutCmd, nil) + }) + if len(rig.fizzy.revoked()) != 0 { + t.Errorf("revoked %d tokens, want none", len(rig.fizzy.revoked())) + } + if !strings.Contains(stderr, "could not revoke the token fizzy signup created") { + t.Errorf("stderr = %q, want the revocation warning (logout error: %v)", stderr, err) + } +} + +// The config-file copy signup falls back to still counts when the +// credential store can't be read. +func TestLogoutRevokesTheConfigFileCopyWhenTheStoreCannotBeRead(t *testing.T) { + rig := newMintedTokenRig(t, http.StatusOK) + if err := saveSignupConfig("minted-token", "acct", rig.server.URL); err != nil { + t.Fatal(err) + } + global := config.LoadGlobal() + global.Token = "minted-token" + if err := global.Save(); err != nil { + t.Fatal(err) + } + SetTestConfig("minted-token", "acct", rig.server.URL) + backup := rig.credDir + "-backup" + if err := os.Rename(rig.credDir, backup); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(rig.credDir, []byte("not a directory"), 0600); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Remove(rig.credDir); os.Rename(backup, rig.credDir) }) + + stderr := captureStderr(t, func() { + authLogoutCmd.Flags().Set("all", "false") + _ = authLogoutCmd.RunE(authLogoutCmd, nil) + }) + if got := rig.fizzy.revoked(); len(got) != 1 || got[0] != "minted-token" { + t.Errorf("revoked %d tokens, want the config-file copy once", len(got)) + } + if strings.Contains(stderr, "could not revoke") { + t.Errorf("stderr = %q, want no revocation warning", stderr) + } +} diff --git a/internal/commands/profile_state.go b/internal/commands/profile_state.go index 867094b..1196695 100644 --- a/internal/commands/profile_state.go +++ b/internal/commands/profile_state.go @@ -207,6 +207,10 @@ type profileCredentialSaveOptions struct { Token string AllowYAMLTokenFallback bool UpdateGlobal func(*config.Config, bool) + + // MintedToken says the CLI created Token itself (fizzy signup), so a + // logout revokes it. Any other save replaces the token and drops the mark. + MintedToken bool } func saveProfileCredentialState(opts profileCredentialSaveOptions) (error, error) { @@ -246,7 +250,8 @@ func saveProfileCredentialState(opts profileCredentialSaveOptions) (error, error return stderrors.Join(rollbackErrors...) } - if err := ensureProfileForAccountWithBoard(opts.ProfileName, opts.Account, opts.BaseURL, opts.Board); err != nil { + markToken := func(extra map[string]json.RawMessage) { setMintedTokenMark(extra, opts.Token, opts.MintedToken) } + if err := ensureProfileForAccountWithBoard(opts.ProfileName, opts.Account, opts.BaseURL, opts.Board, markToken); err != nil { return nil, err } if profiles != nil { diff --git a/internal/commands/root.go b/internal/commands/root.go index 2852635..2e75e91 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -1385,10 +1385,13 @@ func ensureProfileForAccount(name, account, baseURL, board string) error { if board != "" { boardUpdate = &board } - return ensureProfileForAccountWithBoard(name, account, baseURL, boardUpdate) + return ensureProfileForAccountWithBoard(name, account, baseURL, boardUpdate, nil) } -func ensureProfileForAccountWithBoard(name, account, baseURL string, board *string) error { +// ensureProfileForAccountWithBoard is ensureProfileForAccount with the board +// update explicit (nil keeps it, "" clears it) and editExtra, when given, +// applied to the profile's extra metadata last. +func ensureProfileForAccountWithBoard(name, account, baseURL string, board *string, editExtra func(map[string]json.RawMessage)) error { if profiles == nil { if account != "" && account != name { return fmt.Errorf("profile store is unavailable for alias %q", name) @@ -1434,6 +1437,9 @@ func ensureProfileForAccountWithBoard(name, account, baseURL string, board *stri extra["account"] = func() json.RawMessage { b, _ := json.Marshal(account); return b }() } } + if editExtra != nil { + editExtra(extra) + } p := &profile.Profile{ Name: name, diff --git a/internal/commands/signup.go b/internal/commands/signup.go index f1f799e..3b7b6ba 100644 --- a/internal/commands/signup.go +++ b/internal/commands/signup.go @@ -814,6 +814,7 @@ func saveSignupConfig(token, account, apiURL string) error { Account: account, BaseURL: apiURL, Token: token, + MintedToken: true, AllowYAMLTokenFallback: true, UpdateGlobal: func(globalCfg *config.Config, credentialStored bool) { if credentialStored {