From dd85573c8f1091046ef499b4ed3ad5474961d094 Mon Sep 17 00:00:00 2001 From: Ben Teichman Date: Fri, 18 Sep 2026 15:03:43 -0400 Subject: [PATCH 1/2] feat: use oauth to authenticate (COR-14002) --- README.md | 53 ++- docs/vf_auth.md | 2 +- docs/vf_auth_login.md | 21 +- internal/cli/auth.go | 37 +- internal/cli/whoami.go | 3 + internal/client/client.go | 13 +- internal/oauth/browser.go | 54 +++ internal/oauth/callback.go | 177 ++++++++++ internal/oauth/callback_test.go | 122 +++++++ internal/oauth/command.go | 129 +++++++ internal/oauth/command_test.go | 116 ++++++ internal/oauth/login_test.go | 548 +++++++++++++++++++++++++++++ internal/oauth/metadata.go | 122 +++++++ internal/oauth/metadata_test.go | 171 +++++++++ internal/oauth/oauth.go | 476 +++++++++++++++++++++++++ internal/oauth/pkce.go | 39 ++ internal/oauth/pkce_test.go | 38 ++ internal/oauth/register.go | 101 ++++++ internal/oauth/store.go | 264 ++++++++++++++ internal/oauth/store_test.go | 183 ++++++++++ internal/oauth/testsupport_test.go | 78 ++++ internal/oauth/token.go | 133 +++++++ internal/oauth/token_test.go | 109 ++++++ 23 files changed, 2971 insertions(+), 18 deletions(-) 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/command.go create mode 100644 internal/oauth/command_test.go create mode 100644 internal/oauth/login_test.go create mode 100644 internal/oauth/metadata.go create mode 100644 internal/oauth/metadata_test.go create mode 100644 internal/oauth/oauth.go create mode 100644 internal/oauth/pkce.go create mode 100644 internal/oauth/pkce_test.go create mode 100644 internal/oauth/register.go create mode 100644 internal/oauth/store.go create mode 100644 internal/oauth/store_test.go create mode 100644 internal/oauth/testsupport_test.go create mode 100644 internal/oauth/token.go create mode 100644 internal/oauth/token_test.go diff --git a/README.md b/README.md index c32aac1f..3cd3609c 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ Realtime: Realtime gateway API service * [Shell Completion](#shell-completion) * [CLI Example Usage](#cli-example-usage) * [Authentication](#authentication) + * [Browser sign-in (OAuth2)](#browser-sign-in-oauth2) * [Available Commands](#available-commands) * [Request Body Input](#request-body-input) * [Server Selection](#server-selection) @@ -75,7 +76,7 @@ Download pre-built binaries for your platform from the [releases page](https://g ## Quickstart: zero to a talking agent -Runnable end to end by a person or by a coding agent. You need an access token and [`jq`](https://jqlang.org) (used only to pull IDs out of the JSON responses). Create a **personal access token** in Voiceflow under **Settings → Access tokens** (tokens start with `vfp_`; they expire, default 30 days), then: +Runnable end to end by a person or by a coding agent. You need credentials and [`jq`](https://jqlang.org) (used only to pull IDs out of the JSON responses). At a terminal, `vf auth login` signs you in through the browser and stores the session for you (see [Browser sign-in](#browser-sign-in-oauth2)). For scripts, CI, and coding agents, create a **personal access token** in Voiceflow under **Settings → Access tokens** (tokens start with `vfp_`; they expire, default 30 days), then: ```bash export VF_TOKEN=vfp_... # every command also accepts --token @@ -210,6 +211,56 @@ vf configure Configuration is stored in `~/.config/vf/config.yaml`. + +## Browser sign-in (OAuth2) + +Instead of pasting a personal access token, sign in through the browser: + +```bash +vf auth login +``` + +This runs an OAuth2 authorization code flow with PKCE against +`https://auth-api.voiceflow.com`: the CLI opens your browser, receives the +authorization code on a loopback listener (`http://127.0.0.1:5133x/oauth/callback`), +and exchanges it for an access and refresh token. Later commands refresh the +access token automatically as it nears expiry, so the session keeps working +without another sign-in. + +| Command | What it does | +|---------|--------------| +| `vf auth login` | Sign in through the browser | +| `vf auth login --no-browser` | Print the sign-in URL instead of opening a browser (SSH, headless) | +| `vf auth login --token ` | Store a bearer token non-interactively, no browser | +| `vf auth login --manual` | Prompt for a bearer token instead of signing in | +| `vf auth whoami` | Show the configured credential and the current session | +| `vf auth logout` | Clear the session and every stored credential | + +Other flags: `--scope` (repeatable) requests specific scopes instead of the +ones the authorization server advertises, and `--login-timeout` bounds the wait +for the browser (default 5m). + +**Where the tokens live.** Access and refresh tokens go to the OS keychain — +macOS Keychain, Windows Credential Manager, Linux Secret Service — under +separate entries. On a machine with no keychain (headless Linux, containers) +they fall back to `~/.config/vf/oauth.json`, written with owner-only +permissions. Session metadata that is not secret (issuer, scopes, expiry, and +the cached public client registration) is always kept in +`~/.config/vf/oauth.json` and `~/.config/vf/oauth-client.json`. + +**Precedence.** An active browser session outranks a token stored by +`vf configure` or `vf auth login --token`, because it is refreshed on demand. +A `--token` flag or a `VF_TOKEN` environment variable still wins over both, so +CI keeps behaving exactly as before and agent mode is unaffected — +`vf auth login` is interactive and stays blocked there. + +**Pointing at another authorization server.** `VF_OAUTH_ISSUER`, +`VF_OAUTH_CLIENT_ID`, `VF_OAUTH_RESOURCE`, `VF_OAUTH_SCOPES`, and +`VF_OAUTH_REDIRECT_URI` override the defaults. With no `VF_OAUTH_CLIENT_ID` +set, the CLI registers itself as a public client through the authorization +server's dynamic client registration endpoint and caches the resulting +`client_id` for later logins. + ## Available Commands diff --git a/docs/vf_auth.md b/docs/vf_auth.md index 892cf2e1..e1865091 100644 --- a/docs/vf_auth.md +++ b/docs/vf_auth.md @@ -38,6 +38,6 @@ Subcommands: ### SEE ALSO * [vf](vf.md) - Realtime: Realtime gateway API service -* [vf auth login](vf_auth_login.md) - Interactively configure authentication credentials +* [vf auth login](vf_auth_login.md) - Sign in through the browser or configure credentials * [vf auth logout](vf_auth_logout.md) - Clear all stored authentication credentials * [vf auth whoami](vf_auth_whoami.md) - Display current authentication configuration diff --git a/docs/vf_auth_login.md b/docs/vf_auth_login.md index 9ea7c310..10d43a95 100644 --- a/docs/vf_auth_login.md +++ b/docs/vf_auth_login.md @@ -1,15 +1,18 @@ ## vf auth login -Interactively configure authentication credentials +Sign in through the browser or configure credentials ### Synopsis -Interactively configure authentication credentials for vf. -Secret credentials are stored in the OS keychain when available, -with a config file fallback. +Sign in to Voiceflow. -All fields are optional — press Enter to skip any field you don't need. -Use the configure command for both authentication and global parameters. +By default this opens your browser to complete an OAuth2 authorization code +flow, then stores the resulting access and refresh tokens in the OS keychain +when available, with an owner-only file fallback. + +Use --token to store a bearer token non-interactively, or --manual to be +prompted for one. Use the configure command for both authentication and global +parameters. ``` vf auth login [flags] @@ -18,7 +21,11 @@ vf auth login [flags] ### Options ``` - -h, --help help for login + -h, --help help for login + --login-timeout duration How long to wait for the browser to complete sign-in (default 5m0s) + --manual Prompt for a bearer token instead of signing in through the browser + --no-browser Print the sign-in URL instead of opening a browser + --scope stringArray OAuth scope to request (repeatable). Defaults to the scopes the authorization server advertises. ``` ### Options inherited from parent commands diff --git a/internal/cli/auth.go b/internal/cli/auth.go index a807a2c6..2a26cb02 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -9,6 +9,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/spf13/cobra" "github.com/voiceflow/cli/internal/config" + "github.com/voiceflow/cli/internal/oauth" "github.com/voiceflow/cli/internal/output" "golang.org/x/term" "os" @@ -28,17 +29,22 @@ Subcommands: } parent.AddCommand(authCmd) - authCmd.AddCommand(&cobra.Command{ + loginCmd := &cobra.Command{ Use: "login", - Short: "Interactively configure authentication credentials", - Long: `Interactively configure authentication credentials for vf. -Secret credentials are stored in the OS keychain when available, -with a config file fallback. + Short: "Sign in through the browser or configure credentials", + Long: `Sign in to Voiceflow. + +By default this opens your browser to complete an OAuth2 authorization code +flow, then stores the resulting access and refresh tokens in the OS keychain +when available, with an owner-only file fallback. -All fields are optional — press Enter to skip any field you don't need. -Use the configure command for both authentication and global parameters.`, +Use --token to store a bearer token non-interactively, or --manual to be +prompted for one. Use the configure command for both authentication and global +parameters.`, RunE: runAuthLoginCmd, - }) + } + oauth.AddLoginFlags(loginCmd) // browser login flags; see internal/oauth + authCmd.AddCommand(loginCmd) authCmd.AddCommand(&cobra.Command{ Use: "whoami", @@ -83,6 +89,12 @@ func runAuthLoginCmd(cmd *cobra.Command, args []string) error { ) } + // Browser-based OAuth2 login is the default. --token, --manual, and + // --no-interactive fall through to the token-entry paths below. + if oauth.ShouldUseBrowserLogin(cmd) { + return oauth.RunLogin(cmd) + } + cfg := config.GetConfig() if cfg == nil { cfg = &config.Config{} @@ -161,11 +173,20 @@ func runAuthLogoutCmd(cmd *cobra.Command, args []string) error { } cfg.Security.Token = "" + // Clear the stored OAuth session as well; see internal/oauth. + hadSession, err := oauth.RunLogout() + if err != nil { + return fmt.Errorf("failed to clear OAuth session: %w", err) + } + if err := config.SaveConfig(cfg); err != nil { return fmt.Errorf("failed to save configuration: %w", err) } out := cmd.OutOrStdout() + if hadSession { + fmt.Fprintln(out, "Signed out of the OAuth session.") + } fmt.Fprintln(out, "All authentication credentials have been cleared.") fmt.Fprintf(out, "Configuration saved to %s\n", config.GetConfigPath()) return nil diff --git a/internal/cli/whoami.go b/internal/cli/whoami.go index 05e18811..75fd0428 100644 --- a/internal/cli/whoami.go +++ b/internal/cli/whoami.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/spf13/cobra" "github.com/voiceflow/cli/internal/config" + "github.com/voiceflow/cli/internal/oauth" "github.com/voiceflow/cli/internal/usage" ) @@ -51,5 +52,7 @@ func runWhoamiCmd(cmd *cobra.Command, args []string) error { fmt.Fprintf(out, " --%-25s [%-7s] %s\n", "token", source, maskSecret(value)) } + oauth.WriteStatus(out) // browser login session; see internal/oauth + return nil } diff --git a/internal/client/client.go b/internal/client/client.go index fdeb8eb0..ebdc5a56 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" "github.com/voiceflow/cli/internal/config" "github.com/voiceflow/cli/internal/flagutil" + "github.com/voiceflow/cli/internal/oauth" "github.com/voiceflow/cli/internal/sdk" "github.com/voiceflow/cli/internal/sdk/models/components" "github.com/voiceflow/cli/internal/testclient" @@ -64,7 +65,17 @@ func resolveStringFlag(cmd *cobra.Command, name string) string { // Priority: flag > env var > config file. func buildGlobalSecurity(cmd *cobra.Command) components.Security { // Resolve security credentials: flag > env var > keyring > config file - token, _ := config.ResolveSecurityCredential(cmd, "token") + token, source := config.ResolveSecurityCredential(cmd, "token") + + // An OAuth session from 'vf auth login' outranks a stored static token — + // it is refreshed on demand — while an explicit flag or environment + // variable still wins, so CI can override it. See internal/oauth. + if source != "flag" && source != "env" { + if sessionToken, err := oauth.AccessToken(cmd.Context()); err == nil && sessionToken != "" { + token = sessionToken + } + } + globalSecurity := components.Security{} if token != "" { globalSecurity.Token = token diff --git a/internal/oauth/browser.go b/internal/oauth/browser.go new file mode 100644 index 00000000..a8ea2071 --- /dev/null +++ b/internal/oauth/browser.go @@ -0,0 +1,54 @@ +package oauth + +import ( + "fmt" + "os/exec" + "runtime" + "strings" +) + +// browserOpener is swapped out in tests. +var browserOpener = openBrowser + +// openBrowser launches the platform's default browser on url. The URL is +// always passed as a separate argument, never interpolated into a shell +// command, so its query string cannot be reinterpreted by a shell. +func openBrowser(url string) error { + var name string + var args []string + + switch runtime.GOOS { + case "darwin": + name, args = "open", []string{url} + case "windows": + // rundll32 hands the URL to the registered protocol handler without + // going through cmd.exe, where '&' would split the command. + name, args = "rundll32", []string{"url.dll,FileProtocolHandler", url} + default: + // xdg-open is the freedesktop standard; the others cover minimal + // installs and WSL. + for _, candidate := range []string{"xdg-open", "wslview", "gio", "sensible-browser", "x-www-browser"} { + if path, err := exec.LookPath(candidate); err == nil { + name = path + if candidate == "gio" { + args = []string{"open", url} + } else { + args = []string{url} + } + break + } + } + if name == "" { + return fmt.Errorf("no browser launcher found (tried xdg-open, wslview, gio)") + } + } + + cmd := exec.Command(name, args...) + if err := cmd.Start(); err != nil { + return fmt.Errorf("launch browser via %s: %w", strings.TrimSpace(name), err) + } + // The launcher exits immediately after handing the URL to the browser; + // reap it so it does not linger as a zombie for the CLI's lifetime. + go func() { _ = cmd.Wait() }() + return nil +} diff --git a/internal/oauth/callback.go b/internal/oauth/callback.go new file mode 100644 index 00000000..d6baa4c7 --- /dev/null +++ b/internal/oauth/callback.go @@ -0,0 +1,177 @@ +package oauth + +import ( + "context" + "crypto/subtle" + "fmt" + "html" + "net" + "net/http" + "sync" + "time" +) + +// callbackResult carries the outcome of the authorization redirect back to the +// login flow. +type callbackResult struct { + code string + err error +} + +// callbackServer is the loopback redirect receiver required by RFC 8252 for +// native apps: a short-lived HTTP server on 127.0.0.1 that the browser is +// redirected to with the authorization code. +type callbackServer struct { + listener net.Listener + server *http.Server + results chan callbackResult + redirectURI string + state string + once sync.Once +} + +// startCallbackServer binds the first available address in candidates (each a +// "host:port" the client is registered to redirect to) and serves path until +// the authorization code arrives. +func startCallbackServer(candidates []redirectTarget, state string) (*callbackServer, error) { + var listener net.Listener + var target redirectTarget + var lastErr error + + for _, candidate := range candidates { + ln, err := net.Listen("tcp", candidate.address()) + if err != nil { + lastErr = err + continue + } + listener, target = ln, candidate + if addr, ok := ln.Addr().(*net.TCPAddr); ok { + // Port 0 means "any free port"; record the one actually bound so + // the redirect URI matches where the server is listening. + target.Port = addr.Port + } + break + } + if listener == nil { + return nil, fmt.Errorf("no registered callback port is available: %w", lastErr) + } + + cb := &callbackServer{ + listener: listener, + results: make(chan callbackResult, 1), + redirectURI: target.uri(), + state: state, + } + + mux := http.NewServeMux() + mux.HandleFunc(target.Path, cb.handle) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + }) + + cb.server = &http.Server{ + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, + } + go func() { + // Serve returns ErrServerClosed on shutdown; any other error surfaces + // through Wait as a closed connection to the browser. + _ = cb.server.Serve(listener) + }() + + return cb, nil +} + +// RedirectURI returns the URI the browser will be redirected to. +func (c *callbackServer) RedirectURI() string { return c.redirectURI } + +func (c *callbackServer) handle(w http.ResponseWriter, r *http.Request) { + query := r.URL.Query() + + if errCode := query.Get("error"); errCode != "" { + description := query.Get("error_description") + message := errCode + if description != "" { + message = fmt.Sprintf("%s: %s", errCode, description) + } + c.writePage(w, http.StatusOK, "Login failed", message) + c.deliver(callbackResult{err: fmt.Errorf("authorization denied (%s)", message)}) + return + } + + // Constant-time comparison keeps the state value from leaking through + // response timing; a mismatch means the redirect was not ours (CSRF). + gotState := query.Get("state") + if subtle.ConstantTimeCompare([]byte(gotState), []byte(c.state)) != 1 { + c.writePage(w, http.StatusBadRequest, "Login failed", "The login response did not match this session. Start over with 'vf auth login'.") + c.deliver(callbackResult{err: fmt.Errorf("state mismatch in authorization response")}) + return + } + + code := query.Get("code") + if code == "" { + c.writePage(w, http.StatusBadRequest, "Login failed", "The login response contained no authorization code.") + c.deliver(callbackResult{err: fmt.Errorf("authorization response contained no code")}) + return + } + + c.writePage(w, http.StatusOK, "Signed in", "You can close this tab and return to your terminal.") + c.deliver(callbackResult{code: code}) +} + +// deliver sends the first result and ignores any later ones, so a reloaded +// browser tab cannot overwrite a successful outcome. +func (c *callbackServer) deliver(result callbackResult) { + c.once.Do(func() { c.results <- result }) +} + +// Wait blocks until the browser completes the redirect, the context is +// cancelled, or the deadline passes. +func (c *callbackServer) Wait(ctx context.Context) (string, error) { + select { + case result := <-c.results: + return result.code, result.err + case <-ctx.Done(): + return "", ctx.Err() + } +} + +// Close shuts the server down, allowing in-flight responses a moment to reach +// the browser. +func (c *callbackServer) Close() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = c.server.Shutdown(ctx) +} + +func (c *callbackServer) writePage(w http.ResponseWriter, status int, heading, message string) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") + w.WriteHeader(status) + // The message can include an error_description from the authorization + // server, so escape everything before it reaches the page. + fmt.Fprintf(w, callbackPageTemplate, html.EscapeString(heading), html.EscapeString(heading), html.EscapeString(message)) +} + +// callbackPageTemplate takes the page title, heading, and message, each of +// which must be HTML-escaped by the caller. +const callbackPageTemplate = ` + + + +%s · Voiceflow CLI + + +

%s

%s

+ +` diff --git a/internal/oauth/callback_test.go b/internal/oauth/callback_test.go new file mode 100644 index 00000000..84beae09 --- /dev/null +++ b/internal/oauth/callback_test.go @@ -0,0 +1,122 @@ +package oauth + +import ( + "context" + "io" + "net/http" + "strings" + "testing" + "time" +) + +func startTestCallback(t *testing.T, state string) *callbackServer { + t.Helper() + // Port 0 lets the OS pick a free port, which keeps concurrent tests apart. + cb, err := startCallbackServer([]redirectTarget{{Host: "127.0.0.1", Port: 0, Path: callbackPath}}, state) + if err != nil { + t.Fatalf("startCallbackServer: %v", err) + } + t.Cleanup(cb.Close) + return cb +} + +func TestCallbackServerReturnsCode(t *testing.T) { + cb := startTestCallback(t, "the-state") + + if !strings.HasSuffix(cb.RedirectURI(), callbackPath) { + t.Errorf("redirect URI = %q, want it to end in %q", cb.RedirectURI(), callbackPath) + } + if strings.Contains(cb.RedirectURI(), ":0/") { + t.Errorf("redirect URI = %q, want the port actually bound", cb.RedirectURI()) + } + + go func() { + resp, err := http.Get(cb.RedirectURI() + "?code=the-code&state=the-state") + if err == nil { + io.Copy(io.Discard, resp.Body) + resp.Body.Close() + } + }() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + code, err := cb.Wait(ctx) + if err != nil { + t.Fatalf("Wait: %v", err) + } + if code != "the-code" { + t.Errorf("code = %q, want the-code", code) + } +} + +func TestCallbackServerRejectsStateMismatch(t *testing.T) { + cb := startTestCallback(t, "the-state") + + go http.Get(cb.RedirectURI() + "?code=the-code&state=forged") + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + if _, err := cb.Wait(ctx); err == nil || !strings.Contains(err.Error(), "state mismatch") { + t.Fatalf("Wait error = %v, want a state mismatch", err) + } +} + +func TestCallbackServerSurfacesAuthorizationError(t *testing.T) { + cb := startTestCallback(t, "the-state") + + go http.Get(cb.RedirectURI() + "?error=access_denied&error_description=user+said+no") + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + _, err := cb.Wait(ctx) + if err == nil || !strings.Contains(err.Error(), "access_denied") || !strings.Contains(err.Error(), "user said no") { + t.Fatalf("Wait error = %v, want the server's denial reason", err) + } +} + +func TestCallbackServerEscapesServerSuppliedText(t *testing.T) { + cb := startTestCallback(t, "the-state") + + resp, err := http.Get(cb.RedirectURI() + "?error=access_denied&error_description=" + "%3Cscript%3Ealert(1)%3C/script%3E") + if err != nil { + t.Fatalf("GET callback: %v", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + if strings.Contains(string(body), "