diff --git a/docs/migration-1.0.md b/docs/migration-1.0.md index 04232232..92a994cc 100644 --- a/docs/migration-1.0.md +++ b/docs/migration-1.0.md @@ -400,7 +400,8 @@ archive, or run `make docs`. - `config get KEY` prints only the value; `--show-source` adds where it comes from. - `auth status` exits with 4 when there is no account or a credential is - missing or rejected. `auth token` prints the active account's token. + missing or rejected; with `--json` it exits 0 and reports the problem in the + output (`credential_available`, `verified`, `error`), like `gh auth status --json`. `auth token` prints the active account's token. - `webhook edit` must be told what to do with the delivery authorization header on every call, because Gitea replaces it: pass `--authorization-header`, `--authorization-header-file` or `--clear-authorization`. The signing secret diff --git a/internal/app/auth.go b/internal/app/auth.go index 654cd1f2..5d150a4f 100644 --- a/internal/app/auth.go +++ b/internal/app/auth.go @@ -107,7 +107,8 @@ func (a *application) authCommand() *cobra.Command { tea keeps the accounts of each instance in hosts.yml and their tokens in the system keyring, or in a private file when you choose so. One account per instance is active; --user or TEA_USER selects another for a single command. GITEA_TOKEN, together with -GITEA_INSTANCE_URL or TEA_HOST, replaces the stored credentials of its instance. See +GITEA_INSTANCE_URL or TEA_HOST, replaces the stored credentials of its instance for the +active account too; it must belong to the account that --user or TEA_USER names. See tea help environment for how commands select the instance.`} group.AddCommand(a.loginCommand(), a.logoutCommand(), a.authStatusCommand(), a.authSwitchCommand(), a.authRefreshCommand(), a.authTokenCommand(), a.setupGitCommand(), a.gitCredentialCommand()) return group @@ -424,7 +425,7 @@ func (a *application) authStatusCommand() *cobra.Command { Use: "status", Short: "Show the accounts tea can use", Args: argsExact(0), Long: `Show the accounts tea can use on each instance, the active one, and how they authenticate. Fails with exit code 4 when there is no account, or when a credential is -missing or rejected.`, +missing or rejected. With --json it exits 0 either way; read credential_available, verified and error.`, Example: `$ tea auth status # Check the credentials against their instances $ tea auth status --verify @@ -836,8 +837,8 @@ $ tea auth token --host gitea.com --user alice`, if instance, err := settings.ResolveHost(a.tokenInstance(), s.Hosts.Hosts); err != nil || instance != t.Host { token = "" } - if token != "" && t.User != "" { - // Like connect, print the environment token only for the account it authenticates. + if token != "" && t.explicitUser() { + // Like connect, print the environment token only for the account --user or TEA_USER names. actual, err := services.CurrentAccount(cmd.Context(), a.deps.Client(t.Host, token, t.host.Insecure)) if err != nil { return err diff --git a/internal/app/auth_test.go b/internal/app/auth_test.go index 339aa6c2..fbdbbca8 100644 --- a/internal/app/auth_test.go +++ b/internal/app/auth_test.go @@ -841,3 +841,22 @@ func TestAuthStatusVerifyHonorsInstanceInsecureForEnvironmentToken(t *testing.T) require.NoError(t, Execute(context.Background(), deps, []string{"auth", "status", "--verify", "--json=user,verified"})) require.JSONEq(t, `[{"user":"alice","verified":true}]`, out.String()) } + +// An instance with REQUIRE_SIGNIN_VIEW answers every unsigned request, /api/v1/version included, +// with 403; login must only make the signed requests. +func TestAuthLoginSSHKeyNeverAsksTheVersionOfAPrivateInstance(t *testing.T) { + keyPath := writeAppTempSSHKey(t, "") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Signature") == "" { + http.Error(w, `{"message":"Only signed in user is allowed to call APIs."}`, http.StatusForbidden) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"login":"alice"}`)) + })) + defer server.Close() + deps, _, _ := testDeps(t) + require.NoError(t, Execute(context.Background(), deps, []string{ + "auth", "login", "--host", server.URL, "--ssh-key", keyPath, "--no-input", "--yes", + })) +} diff --git a/internal/app/target.go b/internal/app/target.go index e9c3d437..7d01a613 100644 --- a/internal/app/target.go +++ b/internal/app/target.go @@ -186,8 +186,10 @@ func (a *application) connect(ctx context.Context, req targetRequest) (target, e if err != nil { return t, err } - // The message must not reveal which account the token belongs to. - if !services.MatchAccount(t.User, actual) { + // Like GH_TOKEN, the token overrides the stored credentials whichever account is active; + // only an account that --user or TEA_USER names must match. The message must not reveal + // which account the token belongs to. + if t.explicitUser() && !services.MatchAccount(t.User, actual) { return t, clierr.Wrap(clierr.Authentication, errEnvironmentAccount) } if t.User == "" { @@ -425,6 +427,11 @@ func (a *application) selectAccount(ctx context.Context, t *target, s settings.S return nil } +// explicitUser reports whether the user named the account with --user or TEA_USER. +func (t target) explicitUser() bool { + return t.User != "" && (t.UserSource == "--user" || t.UserSource == "TEA_USER") +} + // accountError explains why user has no stored credential on the target's instance. func (t target) accountError(user string) error { users := t.host.Users diff --git a/internal/app/target_test.go b/internal/app/target_test.go index 3c70db4d..fbd53bc0 100644 --- a/internal/app/target_test.go +++ b/internal/app/target_test.go @@ -88,7 +88,7 @@ func TestAccountNamesMatchCaseInsensitively(t *testing.T) { } func TestEnvironmentCredentialVerifiesSelectedAccountBeforeMutation(t *testing.T) { - for _, selection := range []string{"--user", "TEA_USER", "active instance account"} { + for _, selection := range []string{"--user", "TEA_USER"} { t.Run(selection, func(t *testing.T) { reads, mutations := 0, 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -113,9 +113,6 @@ func TestEnvironmentCredentialVerifiesSelectedAccountBeforeMutation(t *testing.T if selection == "--user" { args = append(args, "--user", "bob") } - if selection == "active instance account" { - configureHost(t, deps, host, settings.Host{User: "bob", Users: tokenAccounts(settings.StoreKeyring, "bob")}) - } err := Execute(context.Background(), deps, args) require.ErrorContains(t, err, "does not belong to the selected account") require.Equal(t, clierr.Authentication, clierr.ExitCode(err)) @@ -358,3 +355,24 @@ func TestRepositoryDiscoveryOutsideCheckoutWritesNoGitDiagnostics(t *testing.T) require.Empty(t, out.String()) require.Empty(t, diagnostics.String()) } + +func TestEnvironmentCredentialOverridesTheActiveAccount(t *testing.T) { + created := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "token private-test-token", r.Header.Get("Authorization")) + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodGet && r.URL.Path == "/base/api/v1/user" { + _, _ = w.Write([]byte(`{"login":"alice"}`)) + return + } + created = true + _, _ = w.Write([]byte(`{"id":9,"number":3,"title":"Issue","state":"open"}`)) + })) + defer server.Close() + deps, _, _ := testDeps(t) + host := server.URL + "/base" + deps.Getenv = envFunc(map[string]string{"TEA_HOST": host, "GITEA_TOKEN": "private-test-token"}) + configureHost(t, deps, host, settings.Host{User: "bob", Users: tokenAccounts(settings.StoreKeyring, "bob")}) + require.NoError(t, Execute(context.Background(), deps, []string{"issue", "create", "-R", "alice/project", "--title", "Issue", "--body", "Body", "--no-input", "--json"})) + require.True(t, created) +}