From 80da706f91d57396a4c01ac5dc53db0c8535a202 Mon Sep 17 00:00:00 2001 From: fayzan Date: Tue, 15 Sep 2026 13:17:52 +0100 Subject: [PATCH 1/9] Add OAuth to CLI --- cmd/login.go | 58 +++++++ cmd/login_test.go | 95 ++++++++++++ cmd/requesty.go | 7 + internal/client/client.go | 6 +- internal/config/config.go | 15 ++ internal/config/config_test.go | 26 ++++ internal/oauth/browser.go | 31 ++++ internal/oauth/callback.go | 165 ++++++++++++++++++++ internal/oauth/callback_test.go | 165 ++++++++++++++++++++ internal/oauth/flow.go | 207 +++++++++++++++++++++++++ internal/oauth/flow_test.go | 261 ++++++++++++++++++++++++++++++++ internal/oauth/page.go | 161 ++++++++++++++++++++ internal/oauth/pkce.go | 37 +++++ internal/oauth/pkce_test.go | 44 ++++++ 14 files changed, 1273 insertions(+), 5 deletions(-) create mode 100644 cmd/login.go create mode 100644 cmd/login_test.go create mode 100644 internal/config/config_test.go create mode 100644 internal/oauth/browser.go create mode 100644 internal/oauth/callback.go create mode 100644 internal/oauth/callback_test.go create mode 100644 internal/oauth/flow.go create mode 100644 internal/oauth/flow_test.go create mode 100644 internal/oauth/page.go create mode 100644 internal/oauth/pkce.go create mode 100644 internal/oauth/pkce_test.go diff --git a/cmd/login.go b/cmd/login.go new file mode 100644 index 0000000..d35bd70 --- /dev/null +++ b/cmd/login.go @@ -0,0 +1,58 @@ +package cmd + +import ( + "fmt" + "strings" + + "github.com/requestyai/cli/internal/oauth" + "github.com/spf13/cobra" +) + +const ( + loginPrintTokenFlag = "print-token" +) + +func newLoginCommand(env environment) *cobra.Command { + cmd := &cobra.Command{ + Use: "login", + Short: "Sign in to Requesty in your browser", + Long: "Sign in to Requesty in your browser.\n\n" + + "The CLI opens the Requesty consent page and, once you approve, receives a\n" + + "short-lived token on this machine. Nothing is written to disk.", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + login := env.login + if login == nil { + login = oauth.Login + } + + token, err := login(cmd.Context(), oauth.Options{ + APIBaseURL: env.config.ResolveAPIBaseURL(), + Status: cmd.ErrOrStderr(), + }) + if err != nil { + return err + } + + out := cmd.OutOrStdout() + if _, err := fmt.Fprintln(out, "Signed in to Requesty."); err != nil { + return err + } + + fields := [][2]string{ + {"Scopes", strings.Join(token.Scopes(), " ")}, + {"Expires in", token.ExpiresIn.String()}, + } + if printToken, _ := cmd.Flags().GetBool(loginPrintTokenFlag); printToken { + fields = append(fields, [2]string{"Access token", token.AccessToken}) + } + + return writeFields(out, fields) + }, + } + + cmd.Flags().Bool(loginPrintTokenFlag, false, "print the access token for manual testing") + _ = cmd.Flags().MarkHidden(loginPrintTokenFlag) + + return cmd +} diff --git a/cmd/login_test.go b/cmd/login_test.go new file mode 100644 index 0000000..09aaed4 --- /dev/null +++ b/cmd/login_test.go @@ -0,0 +1,95 @@ +package cmd + +import ( + "bytes" + "context" + "errors" + "testing" + "time" + + "github.com/requestyai/cli/internal/config" + "github.com/requestyai/cli/internal/oauth" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoginPrintsScopesAndExpiry(t *testing.T) { + var gotOptions oauth.Options + var output bytes.Buffer + command := newRootCommand(environment{ + config: config.Config{APIBaseURL: "http://localhost:40003"}, + login: func(_ context.Context, opts oauth.Options) (*oauth.Token, error) { + gotOptions = opts + return &oauth.Token{ + AccessToken: "secret-token", + TokenType: "Bearer", + Scope: "manage:group:r manage:apikey:w", + ExpiresIn: 5 * time.Minute, + }, nil + }, + }) + command.SetOut(&output) + command.SetArgs([]string{"login"}) + + require.NoError(t, command.Execute()) + + assert.Equal(t, "http://localhost:40003", gotOptions.APIBaseURL) + assert.NotNil(t, gotOptions.Status) + assert.Equal(t, + "Signed in to Requesty.\n"+ + "Scopes manage:group:r manage:apikey:w\n"+ + "Expires in 5m0s\n", + output.String()) + assert.NotContains(t, output.String(), "secret-token") +} + +func TestLoginDefaultsToProductionAPI(t *testing.T) { + var gotOptions oauth.Options + command := newRootCommand(environment{ + login: func(_ context.Context, opts oauth.Options) (*oauth.Token, error) { + gotOptions = opts + return &oauth.Token{AccessToken: "t", ExpiresIn: time.Minute}, nil + }, + }) + command.SetOut(&bytes.Buffer{}) + command.SetArgs([]string{"login"}) + + require.NoError(t, command.Execute()) + assert.Equal(t, config.DefaultAPIBaseURL, gotOptions.APIBaseURL) +} + +func TestLoginPrintTokenFlagRevealsToken(t *testing.T) { + var output bytes.Buffer + command := newRootCommand(environment{ + login: func(context.Context, oauth.Options) (*oauth.Token, error) { + return &oauth.Token{AccessToken: "secret-token", Scope: "manage:group:r", ExpiresIn: time.Minute}, nil + }, + }) + command.SetOut(&output) + command.SetArgs([]string{"login", "--print-token"}) + + require.NoError(t, command.Execute()) + assert.Contains(t, output.String(), "Access token secret-token\n") +} + +func TestLoginPrintTokenFlagIsHidden(t *testing.T) { + command := newRootCommand(environment{}) + + login, _, err := command.Find([]string{"login"}) + require.NoError(t, err) + + flag := login.Flags().Lookup(loginPrintTokenFlag) + require.NotNil(t, flag) + assert.True(t, flag.Hidden) +} + +func TestLoginReportsFailure(t *testing.T) { + command := newRootCommand(environment{ + login: func(context.Context, oauth.Options) (*oauth.Token, error) { + return nil, errors.New("sign-in was not completed: access_denied") + }, + }) + command.SetArgs([]string{"login"}) + + require.EqualError(t, command.Execute(), "sign-in was not completed: access_denied") +} diff --git a/cmd/requesty.go b/cmd/requesty.go index 997dbb4..df8d524 100644 --- a/cmd/requesty.go +++ b/cmd/requesty.go @@ -1,11 +1,13 @@ package cmd import ( + "context" "fmt" tea "charm.land/bubbletea/v2" "github.com/requestyai/cli/internal/client" "github.com/requestyai/cli/internal/config" + "github.com/requestyai/cli/internal/oauth" "github.com/requestyai/cli/internal/tui" "github.com/spf13/cobra" ) @@ -25,6 +27,9 @@ func Run() error { type environment struct { config config.Config apiv2Client *client.Client + + // login runs the browser sign-in. nil means the real OAuth flow. + login func(context.Context, oauth.Options) (*oauth.Token, error) } func newEnvironment() (environment, error) { @@ -36,6 +41,7 @@ func newEnvironment() (environment, error) { return environment{ config: cfg, apiv2Client: client.New(cfg), + login: oauth.Login, }, nil } @@ -59,6 +65,7 @@ func newRootCommand(env environment) *cobra.Command { } root.AddCommand( + newLoginCommand(env), newAuthCommand(env), newAPIKeysCommand(env), newGroupsCommand(env), diff --git a/internal/client/client.go b/internal/client/client.go index 6bb8edb..ed35c38 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -31,11 +31,7 @@ func New(cfg config.Config) *Client { } func (c *Client) apiBaseURL() (string, error) { - if c.config.APIBaseURL != "" { - return c.config.APIBaseURL, nil - } - - return strings.Replace(c.config.RouterBaseURL, "router", "api-v2", 1), nil + return c.config.ResolveAPIBaseURL(), nil } func (c *Client) authorize(req *http.Request) { diff --git a/internal/config/config.go b/internal/config/config.go index bf4c04d..d186ce1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -7,6 +7,7 @@ import ( "io/fs" "os" "path/filepath" + "strings" ) const ( @@ -24,6 +25,20 @@ type Config struct { APIBaseURL string `json:"api_base_url,omitempty"` } +// ResolveAPIBaseURL returns the management API address: the configured one, +// else the one implied by the router address, else the production default. +// The fallback matters before onboarding, when the config file does not exist. +func (c Config) ResolveAPIBaseURL() string { + if c.APIBaseURL != "" { + return c.APIBaseURL + } + if c.RouterBaseURL != "" { + return strings.Replace(c.RouterBaseURL, "router", "api-v2", 1) + } + + return DefaultAPIBaseURL +} + // Load reads the settings. A missing file is not an error: it means the user // has not onboarded yet, so the zero Config comes back. func Load() (Config, error) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..0f8505d --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,26 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestResolveAPIBaseURL(t *testing.T) { + tests := []struct { + name string + config Config + want string + }{ + {name: "explicit api base url wins", config: Config{APIBaseURL: "http://localhost:40003", RouterBaseURL: "https://router.requesty.ai"}, want: "http://localhost:40003"}, + {name: "derived from router", config: Config{RouterBaseURL: "https://router.requesty.ai"}, want: "https://api-v2.requesty.ai"}, + {name: "derived from staging router", config: Config{RouterBaseURL: "https://router.staging.requesty.ai"}, want: "https://api-v2.staging.requesty.ai"}, + {name: "zero config uses production default", config: Config{}, want: DefaultAPIBaseURL}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, tc.config.ResolveAPIBaseURL()) + }) + } +} diff --git a/internal/oauth/browser.go b/internal/oauth/browser.go new file mode 100644 index 0000000..7e5e9a6 --- /dev/null +++ b/internal/oauth/browser.go @@ -0,0 +1,31 @@ +package oauth + +import ( + "fmt" + "os/exec" + "runtime" +) + +// openBrowser asks the desktop to open url in the default browser. It returns +// once the request has been handed off, without waiting for the browser to +// exit, because some launchers block until the browser window closes. +func openBrowser(url string) error { + var cmd *exec.Cmd + switch runtime.GOOS { + case "darwin": + cmd = exec.Command("open", url) + case "windows": + cmd = exec.Command("rundll32", "url.dll,FileProtocolHandler", url) + default: + cmd = exec.Command("xdg-open", url) + } + + if err := cmd.Start(); err != nil { + return fmt.Errorf("failed to start browser: %w", err) + } + + // Reap the launcher in the background so it does not linger as a zombie. + go func() { _ = cmd.Wait() }() + + return nil +} diff --git a/internal/oauth/callback.go b/internal/oauth/callback.go new file mode 100644 index 0000000..b42b878 --- /dev/null +++ b/internal/oauth/callback.go @@ -0,0 +1,165 @@ +package oauth + +import ( + "context" + "crypto/subtle" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "sync" + "time" +) + +const ( + callbackPath = "/callback" + + // loginTimeout bounds how long the CLI waits for the user to finish in the + // browser before giving up. + loginTimeout = 5 * time.Minute + + // shutdownTimeout bounds how long Close waits for the browser's connection + // to drain before dropping it. + shutdownTimeout = 2 * time.Second +) + +// Error is a failure the authorization server reported, either on the redirect +// back to the CLI or from the token endpoint. +type Error struct { + Code string + Description string +} + +func (e *Error) Error() string { + if e.Description == "" { + return e.Code + } + + return fmt.Sprintf("%s: %s", e.Code, e.Description) +} + +type callbackResult struct { + code string + err error +} + +// callbackServer is the loopback HTTP server the browser lands on once the +// user has approved or denied the request. It accepts exactly one outcome. +type callbackServer struct { + listener net.Listener + server *http.Server + state string + results chan callbackResult + once sync.Once +} + +// listenCallback starts serving on a free port of the IPv4 loopback interface. +// The redirect URI must use the 127.0.0.1 literal, not localhost, so that is +// what we bind to. +func listenCallback(state string) (*callbackServer, error) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return nil, fmt.Errorf("failed to listen for the browser callback: %w", err) + } + + server := &callbackServer{ + listener: listener, + state: state, + results: make(chan callbackResult, 1), + } + + mux := http.NewServeMux() + mux.HandleFunc("GET "+callbackPath, server.handle) + server.server = &http.Server{ + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, + } + + go func() { + // Serve always returns a non-nil error; after Shutdown it is + // ErrServerClosed, which is the expected way to stop. + _ = server.server.Serve(listener) + }() + + return server, nil +} + +// RedirectURI is the address the authorization server sends the browser back +// to. The exact same string must be presented at the token endpoint. +func (s *callbackServer) RedirectURI() string { + return "http://" + s.listener.Addr().String() + callbackPath +} + +// Wait blocks until the browser delivers an outcome, the context ends, or the +// login timeout passes. +func (s *callbackServer) Wait(ctx context.Context) (string, error) { + ctx, cancel := context.WithTimeout(ctx, loginTimeout) + defer cancel() + + select { + case result := <-s.results: + return result.code, result.err + case <-ctx.Done(): + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return "", fmt.Errorf("timed out waiting for the browser sign-in: %w", ctx.Err()) + } + + return "", ctx.Err() + } +} + +// Close stops the server, giving the browser a moment to read its response. +func (s *callbackServer) Close() { + ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) + defer cancel() + + if err := s.server.Shutdown(ctx); err != nil { + _ = s.server.Close() + } +} + +func (s *callbackServer) handle(w http.ResponseWriter, r *http.Request) { + result := s.resolve(r.URL.Query()) + + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") + + if result.err != nil { + w.WriteHeader(http.StatusBadRequest) + writePage(w, callbackPage{ + Title: "Sign-in failed", + Message: "Could not sign in to Requesty CLI: " + result.err.Error() + ".", + Hint: "You can close this tab and return to the terminal.", + }) + } else { + writePage(w, callbackPage{ + Success: true, + Title: "Signed in", + Message: "You're signed in to Requesty CLI. You can close this tab.", + Hint: "Return to the terminal to continue.", + }) + } + + s.once.Do(func() { s.results <- result }) +} + +// resolve turns the authorization response into a code or an error. The state +// check comes first so that a response meant for another attempt, or forged +// by a page that guessed the port, is never acted on. +func (s *callbackServer) resolve(query url.Values) callbackResult { + if subtle.ConstantTimeCompare([]byte(query.Get("state")), []byte(s.state)) != 1 { + return callbackResult{err: errors.New("state mismatch: the response did not belong to this sign-in attempt")} + } + + if code := query.Get("error"); code != "" { + return callbackResult{err: &Error{Code: code, Description: query.Get("error_description")}} + } + + code := query.Get("code") + if code == "" { + return callbackResult{err: errors.New("authorization response has no code")} + } + + return callbackResult{code: code} +} diff --git a/internal/oauth/callback_test.go b/internal/oauth/callback_test.go new file mode 100644 index 0000000..8ae3c61 --- /dev/null +++ b/internal/oauth/callback_test.go @@ -0,0 +1,165 @@ +package oauth + +import ( + "context" + "io" + "net/http" + "net/url" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// visit plays the browser: it follows the redirect the authorization server +// would have issued and returns what the callback page said. +func visit(t *testing.T, server *callbackServer, query url.Values) (int, string) { + t.Helper() + + resp, err := http.Get(server.RedirectURI() + "?" + query.Encode()) + require.NoError(t, err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + return resp.StatusCode, string(body) +} + +func TestCallbackRedirectURIUsesLoopbackIPLiteral(t *testing.T) { + server, err := listenCallback("state") + require.NoError(t, err) + defer server.Close() + + redirect, err := url.Parse(server.RedirectURI()) + require.NoError(t, err) + + assert.Equal(t, "http", redirect.Scheme) + assert.Equal(t, "127.0.0.1", redirect.Hostname()) + assert.NotEmpty(t, redirect.Port()) + assert.Equal(t, "/callback", redirect.Path) +} + +func TestCallbackDeliversCode(t *testing.T) { + server, err := listenCallback("expected-state") + require.NoError(t, err) + defer server.Close() + + status, body := visit(t, server, url.Values{ + "code": {"the-code"}, + "state": {"expected-state"}, + "iss": {"https://api-v2.requesty.ai"}, + }) + assert.Equal(t, http.StatusOK, status) + assert.Contains(t, body, "You're signed in to Requesty CLI. You can close this tab.") + + code, err := server.Wait(context.Background()) + require.NoError(t, err) + assert.Equal(t, "the-code", code) +} + +func TestCallbackRejectsStateMismatch(t *testing.T) { + server, err := listenCallback("expected-state") + require.NoError(t, err) + defer server.Close() + + status, body := visit(t, server, url.Values{ + "code": {"the-code"}, + "state": {"forged-state"}, + }) + assert.Equal(t, http.StatusBadRequest, status) + assert.Contains(t, body, "state mismatch") + + code, err := server.Wait(context.Background()) + require.ErrorContains(t, err, "state mismatch") + assert.Empty(t, code) +} + +func TestCallbackSurfacesErrorParameters(t *testing.T) { + server, err := listenCallback("expected-state") + require.NoError(t, err) + defer server.Close() + + status, body := visit(t, server, url.Values{ + "error": {"access_denied"}, + "error_description": {"the user denied the request"}, + "state": {"expected-state"}, + }) + assert.Equal(t, http.StatusBadRequest, status) + assert.Contains(t, body, "access_denied: the user denied the request") + + _, err = server.Wait(context.Background()) + var oauthErr *Error + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "access_denied", oauthErr.Code) + assert.Equal(t, "the user denied the request", oauthErr.Description) +} + +func TestCallbackRejectsResponseWithoutCode(t *testing.T) { + server, err := listenCallback("expected-state") + require.NoError(t, err) + defer server.Close() + + visit(t, server, url.Values{"state": {"expected-state"}}) + + _, err = server.Wait(context.Background()) + require.ErrorContains(t, err, "no code") +} + +func TestCallbackEscapesHTMLInErrors(t *testing.T) { + server, err := listenCallback("expected-state") + require.NoError(t, err) + defer server.Close() + + _, body := visit(t, server, url.Values{ + "error": {"access_denied"}, + "error_description": {""}, + "state": {"expected-state"}, + }) + + assert.False(t, strings.Contains(body, "