Skip to content

feat(cli): add browser-based OAuth2 login with PKCE #1085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jeremyeder wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(cli): add browser-based OAuth2 login with PKCE #1085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b1aee4
feat(cli): add browser-based OAuth2 login with PKCE
jeremyeder Mar 28, 2026
9754112
fix(oauth): address CodeRabbit security review feedback
jeremyeder Mar 28, 2026
6f1c25a
Merge branch 'main' into worktree-acpctl-browser-login
jeremyeder Mar 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
213 changes: 213 additions & 0 deletions components/ambient-cli/cmd/acpctl/login/browser/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
// Package browser implements browser-based OAuth2 login using Authorization Code + PKCE.
package browser

import (
"bufio"
"context"
"fmt"
"net/url"
"os"
"strings"
"time"

"github.com/ambient-code/platform/components/ambient-cli/pkg/config"
"github.com/ambient-code/platform/components/ambient-cli/pkg/oauth"
"github.com/spf13/cobra"
)

var args struct {
issuerURL string
clientID string
scopes string
}

var Cmd = &cobra.Command{
Use: "browser",
Short: "Log in via browser-based OAuth2 flow",
Long: `Open a browser to authenticate with the identity provider using OAuth2
Authorization Code + PKCE. The CLI starts a local callback server to receive the
authorization code, then exchanges it for access and refresh tokens.`,
Args: cobra.NoArgs,
RunE: run,
}

func init() {
flags := Cmd.Flags()
flags.StringVar(&args.issuerURL, "issuer-url", "", "OIDC issuer URL (e.g. https://keycloak.example.com/realms/myrealm)")
flags.StringVar(&args.clientID, "client-id", "", "OAuth2 client ID")
flags.StringVar(&args.scopes, "scopes", "openid email profile", "OAuth2 scopes to request")
}

func run(cmd *cobra.Command, _ []string) error {
cfg, err := config.Load()
if err != nil {
return fmt.Errorf("load config: %w", err)
}

issuerURL := args.issuerURL
if issuerURL == "" {
issuerURL = cfg.GetIssuerURL()
}
if issuerURL == "" {
return fmt.Errorf("--issuer-url is required (or set AMBIENT_ISSUER_URL / issuer_url in config)")
}

clientID := args.clientID
if clientID == "" {
clientID = cfg.GetClientID()
}
if clientID == "" {
return fmt.Errorf("--client-id is required (or set AMBIENT_CLIENT_ID / client_id in config)")
}

fmt.Fprintf(cmd.OutOrStdout(), "Authenticating with %s...\n", issuerURL)

oidcCfg, err := oauth.DiscoverEndpoints(issuerURL)
if err != nil {
return fmt.Errorf("OIDC discovery: %w", err)
}

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancel()

state, err := oauth.GenerateState()
if err != nil {
return err
}

pkce, err := oauth.GeneratePKCE()
if err != nil {
return err
}

addr, resultCh, cleanup, err := oauth.StartCallbackServer(ctx, state)
if err != nil {
return err
}
defer cleanup()

redirectURI := "http://" + addr + "/callback"
authorizeURL, err := oauth.BuildAuthorizeURL(
oidcCfg.AuthorizationEndpoint,
clientID,
redirectURI,
state,
pkce.Challenge,
args.scopes,
)
if err != nil {
return fmt.Errorf("build authorize URL: %w", err)
}

if err := oauth.OpenBrowser(authorizeURL); err != nil {
fmt.Fprintf(cmd.ErrOrStderr(), "Could not open browser: %v\n", err)
}

fmt.Fprintln(cmd.OutOrStdout(), "If the browser did not open, visit this URL:")
fmt.Fprintln(cmd.OutOrStdout(), authorizeURL)
fmt.Fprintln(cmd.OutOrStdout())
fmt.Fprintln(cmd.OutOrStdout(), "Or paste the redirect URL here:")

// Listen for both callback and manual URL paste.
// Use a pipe so we can close the reader to unblock the goroutine.
pr, pw, err := os.Pipe()
if err != nil {
return fmt.Errorf("create pipe: %w", err)
}
defer pr.Close()

// Copy stdin to pipe in background so we can close pr to stop the scanner.
go func() {
defer pw.Close()
buf := make([]byte, 4096)
for {
n, err := os.Stdin.Read(buf)
if n > 0 {
pw.Write(buf[:n]) //nolint:errcheck
}
if err != nil {
return
}
}
}()
Comment on lines +119 to +132
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Stdin copy goroutine will outlive the function if callback succeeds first.

The goroutine reading from os.Stdin will remain blocked indefinitely if the browser callback completes before any stdin input. While this is a minor leak, it's acceptable for a CLI tool that exits after login. No action required, but worth noting for future refactoring if this code is reused in a long-running context.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/ambient-cli/cmd/acpctl/login/browser/cmd.go` around lines 119 -
132, The stdin-copy goroutine reading from os.Stdin can block forever if the
browser callback finishes first; to fix, add a done channel and, when the
callback path completes, close done and if os.Stdin is a *os.File call its
Close() to unblock the reader; update the anonymous goroutine that writes to pw
to return when done is closed (check done in a non-blocking manner between
reads) and ensure pw.Close() is still deferred so the scanner stops cleanly —
reference the pw writer, the anonymous goroutine that reads os.Stdin, and the
callback/completion code path to wire up done and the conditional
os.Stdin.(*os.File).Close().


manualCh := make(chan oauth.CallbackResult, 1)
go func() {
scanner := bufio.NewScanner(pr)
if scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
if line == "" {
return
}
parsed, err := url.Parse(line)
if err != nil {
manualCh <- oauth.CallbackResult{Err: fmt.Errorf("invalid URL: %w", err)}
return
}
code := parsed.Query().Get("code")
pastedState := parsed.Query().Get("state")
if code == "" {
manualCh <- oauth.CallbackResult{Err: fmt.Errorf("URL missing 'code' parameter")}
return
}
if pastedState == "" {
manualCh <- oauth.CallbackResult{Err: fmt.Errorf("URL missing 'state' parameter")}
return
}
if pastedState != state {
manualCh <- oauth.CallbackResult{Err: fmt.Errorf("state mismatch in pasted URL")}
return
}
manualCh <- oauth.CallbackResult{Code: code}
}
}()

var result oauth.CallbackResult
select {
case result = <-resultCh:
case result = <-manualCh:
case <-ctx.Done():
return fmt.Errorf("login timed out after 5 minutes")
}

if result.Err != nil {
return fmt.Errorf("authorization failed: %w", result.Err)
}

fmt.Fprintln(cmd.OutOrStdout(), "Authorization code received, exchanging for tokens...")

tokenResp, err := oauth.ExchangeCode(
oidcCfg.TokenEndpoint,
clientID,
result.Code,
redirectURI,
pkce.Verifier,
)
if err != nil {
return fmt.Errorf("token exchange: %w", err)
}

cfg.AccessToken = tokenResp.AccessToken
cfg.RefreshToken = ""
cfg.IssuerURL = issuerURL
cfg.ClientID = clientID

if err := config.Save(cfg); err != nil {
return fmt.Errorf("save config: %w", err)
}

location, err := config.Location()
if err != nil {
fmt.Fprintln(cmd.OutOrStdout(), "Login successful. Configuration saved.")
} else {
fmt.Fprintf(cmd.OutOrStdout(), "Login successful. Configuration saved to %s\n", location)
}

if exp, err := config.TokenExpiry(tokenResp.AccessToken); err == nil && !exp.IsZero() {
if time.Until(exp) < 24*time.Hour {
fmt.Fprintf(cmd.ErrOrStderr(), "Note: token expires at %s\n", exp.Format(time.RFC3339))
}
}

return nil
}
7 changes: 5 additions & 2 deletions components/ambient-cli/cmd/acpctl/login/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/url"
"time"

"github.com/ambient-code/platform/components/ambient-cli/cmd/acpctl/login/browser"
"github.com/ambient-code/platform/components/ambient-cli/pkg/config"
"github.com/spf13/cobra"
)
Expand All @@ -27,15 +28,17 @@ var Cmd = &cobra.Command{

func init() {
flags := Cmd.Flags()
flags.StringVar(&args.token, "token", "", "Access token (required)")
flags.StringVar(&args.token, "token", "", "Access token (required when not using 'browser' subcommand)")
flags.StringVar(&args.url, "url", "", "API server URL (default: http://localhost:8000)")
flags.StringVar(&args.project, "project", "", "Default project name")
flags.BoolVar(&args.insecureSkipVerify, "insecure-skip-tls-verify", false, "Skip TLS certificate verification (insecure)")

Cmd.AddCommand(browser.Cmd)
}

func run(cmd *cobra.Command, positional []string) error {
if args.token == "" {
return fmt.Errorf("--token is required")
return fmt.Errorf("--token is required (or use 'acpctl login browser' for browser-based OAuth login)")
}

cfg, err := config.Load()
Expand Down
18 changes: 18 additions & 0 deletions components/ambient-cli/pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import (
type Config struct {
APIUrl string `json:"api_url,omitempty"`
AccessToken string `json:"access_token,omitempty"`
RefreshToken string `json:"refresh_token,omitempty"`
IssuerURL string `json:"issuer_url,omitempty"`
ClientID string `json:"client_id,omitempty"`
Project string `json:"project,omitempty"`
Pager string `json:"pager,omitempty"` // TODO: Wire pager support into output commands (e.g. pipe through less)
RequestTimeout int `json:"request_timeout,omitempty"` // Request timeout in seconds
Expand Down Expand Up @@ -80,6 +83,21 @@ func Save(cfg *Config) error {

func (c *Config) ClearToken() {
c.AccessToken = ""
c.RefreshToken = ""
}

func (c *Config) GetIssuerURL() string {
if env := os.Getenv("AMBIENT_ISSUER_URL"); env != "" {
return env
}
return c.IssuerURL
}

func (c *Config) GetClientID() string {
if env := os.Getenv("AMBIENT_CLIENT_ID"); env != "" {
return env
}
return c.ClientID
}

func (c *Config) GetAPIUrl() string {
Expand Down
23 changes: 23 additions & 0 deletions components/ambient-cli/pkg/oauth/browser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package oauth

import (
"fmt"
"os/exec"
"runtime"
)

// OpenBrowser opens the specified URL in the user's default browser.
func OpenBrowser(url string) error {
var cmd *exec.Cmd
switch runtime.GOOS {
case "darwin":
cmd = exec.Command("open", url)
case "linux":
cmd = exec.Command("xdg-open", url)
case "windows":
cmd = exec.Command("rundll32", "url.dll,FileProtocolHandler", url)
default:
return fmt.Errorf("unsupported platform %q", runtime.GOOS)
}
return cmd.Start()
}
86 changes: 86 additions & 0 deletions components/ambient-cli/pkg/oauth/callback.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package oauth

import (
"context"
"fmt"
"net"
"net/http"
"time"
)

// CallbackResult holds the authorization code received from the callback.
type CallbackResult struct {
Code string
Err error
}

// StartCallbackServer starts a local HTTP server on a random port to receive
// the OAuth callback. It returns the server's address and a channel that will
// receive the authorization code.
func StartCallbackServer(ctx context.Context, expectedState string) (addr string, resultCh <-chan CallbackResult, cleanup func(), err error) {
listener, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
return "", nil, nil, fmt.Errorf("listen on localhost: %w", err)
}

ch := make(chan CallbackResult, 1)
mux := http.NewServeMux()
server := &http.Server{Handler: mux}

mux.HandleFunc("/callback", func(w http.ResponseWriter, r *http.Request) {
state := r.URL.Query().Get("state")
if state != expectedState {
http.Error(w, "Invalid state parameter", http.StatusBadRequest)
ch <- CallbackResult{Err: fmt.Errorf("state mismatch: expected %q, got %q", expectedState, state)}
return
}

errParam := r.URL.Query().Get("error")
if errParam != "" {
desc := r.URL.Query().Get("error_description")
http.Error(w, "Authorization failed: "+errParam, http.StatusBadRequest)
ch <- CallbackResult{Err: fmt.Errorf("authorization error: %s: %s", errParam, desc)}
return
}

code := r.URL.Query().Get("code")
if code == "" {
http.Error(w, "Missing authorization code", http.StatusBadRequest)
ch <- CallbackResult{Err: fmt.Errorf("callback missing authorization code")}
return
}

w.Header().Set("Content-Type", "text/html")
fmt.Fprint(w, successHTML)
ch <- CallbackResult{Code: code}
})

go func() {
if err := server.Serve(listener); err != nil && err != http.ErrServerClosed {
ch <- CallbackResult{Err: fmt.Errorf("callback server: %w", err)}
}
}()

cleanupFn := func() {
shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second)
defer shutdownCancel()
server.Shutdown(shutdownCtx) //nolint:errcheck
}

return listener.Addr().String(), ch, cleanupFn, nil
}

const successHTML = `<!DOCTYPE html>
<html><head><title>Login Successful</title>
<style>
body { font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
display: flex; justify-content: center; align-items: center; height: 100vh;
margin: 0; background: #1a1a2e; color: #e0e0e0; }
.container { text-align: center; padding: 2rem; }
h1 { color: #4ecca3; }
p { color: #a0a0a0; }
</style></head>
<body><div class="container">
<h1>Login Successful</h1>
<p>You can close this window and return to the terminal.</p>
</div></body></html>`
Loading