From 547a6a8d494b37cde641eb42fe3579e2a4caedb8 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Fri, 9 Oct 2026 17:49:00 +1300 Subject: [PATCH 1/2] CLI login: RFC 8252 loopback flow bound to the approving browser The CLI login used to be a device flow without a user code: whoever held the gateway-issued `state` got the 90-day session. /auth/cli/complete never checked the code_verifier, states never expired, `state` and the OAuth code were written to the request log, and nothing tied the browser that approved the login to the CLI that polled. A log reader or anyone who got a user to open their login link could take over the session. Now: - The CLI generates the PKCE verifier and its own state, listens on 127.0.0.1 with a random port, and sends only the S256 challenge, state and loopback redirect URI to /auth/cli/start. - The browser that completes Google OAuth is bound to the login with an HttpOnly cookie. The MFA form, MFA submit and return steps all require it, so a third party holding the state can't use it or burn the user's MFA attempts. - After MFA, the browser is redirected to the CLI's loopback listener with a single-use login code (stored hashed, 10-minute TTL, consumed atomically). /auth/cli/complete needs the code AND the verifier. The polling endpoint and the old state-as-credential path are gone. - The MFA page shows the initiating IP and device name. A completed CLI login sends a notification. - state, code and login_code query params are redacted from request and audit logs. - The CLI only opens https auth URLs, and on Windows uses rundll32 rather than `cmd /c start` (which split URLs on '&'). E2E harness: the simulated browser stays on the gateway's hostname (the binding cookie is host-scoped, as in a real browser), and commands run with stdin from /dev/null. `env set` reads extra input from a non-terminal stdin until EOF, so an inherited open pipe hung the suite. --- internal/cli/auth.go | 57 +-- internal/cli/cli_login.go | 123 ++++-- internal/cli/login_loopback.go | 169 ++++++++ internal/cli/login_loopback_test.go | 151 +++++++ internal/cli/types.go | 24 +- internal/cli/utils.go | 5 +- internal/gateway/audit/logger.go | 5 +- .../gateway/audit/query_redaction_test.go | 36 ++ internal/gateway/audit/redaction.go | 34 +- internal/gateway/db/cli_login.go | 218 +++++++--- .../20261009000000_cli_login_loopback.sql | 26 ++ internal/gateway/handlers/auth_cli.go | 307 +++++++------- .../gateway/handlers/auth_cli_complete.go | 284 ++++--------- .../gateway/handlers/auth_cli_loopback.go | 257 ++++++++++++ .../handlers/auth_cli_loopback_test.go | 334 +++++++++++++++ internal/gateway/handlers/auth_helpers.go | 9 +- internal/gateway/handlers/dto.go | 17 +- internal/gateway/jobs/client.go | 1 + internal/gateway/jobs/email/cli_login.go | 81 ++++ internal/gateway/middleware/debug_logging.go | 6 +- .../gateway/openapi/generated/swagger.json | 130 ++++-- internal/gateway/routes/route_registration.go | 1 + internal/gateway/security/notifier.go | 12 + internal/integration/integration_test.go | 17 +- scripts/lib/cli-e2e/cli_helpers.sh | 27 +- scripts/lib/cli-e2e/suite.sh | 26 +- web/e2e/cli-login-webui.spec.ts | 393 ++++++++---------- web/e2e/cli-loopback.ts | 81 ++++ web/src/app.tsx | 7 - .../use-account-security-mutations.ts | 5 +- web/src/pages/cli-auth-success-page.tsx | 16 - web/src/pages/mfa-challenge-page.test.tsx | 86 ++++ web/src/pages/mfa-challenge-page.tsx | 50 ++- 33 files changed, 2151 insertions(+), 844 deletions(-) create mode 100644 internal/cli/login_loopback.go create mode 100644 internal/cli/login_loopback_test.go create mode 100644 internal/gateway/audit/query_redaction_test.go create mode 100644 internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql create mode 100644 internal/gateway/handlers/auth_cli_loopback.go create mode 100644 internal/gateway/handlers/auth_cli_loopback_test.go create mode 100644 internal/gateway/jobs/email/cli_login.go create mode 100644 web/e2e/cli-loopback.ts delete mode 100644 web/src/pages/cli-auth-success-page.tsx create mode 100644 web/src/pages/mfa-challenge-page.test.tsx diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 23521847..56d74a7b 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -13,67 +13,52 @@ import ( "github.com/google/uuid" ) -// StartLogin initiates the OAuth login flow -func StartLogin(gatewayURL string) (*LoginStartResponse, error) { - url := buildGatewayAPIURL(gatewayURL, "/api/v1/auth/cli/start") - - resp, err := sendGatewayRequest(http.MethodPost, url, nil) - if err != nil { - return nil, err - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("login start failed: %s", string(body)) - } - +// StartLogin starts a loopback login and returns the identity provider URL to open. +func StartLogin(gatewayURL string, req LoginStartRequest) (*LoginStartResponse, error) { var result LoginStartResponse - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + url := buildGatewayAPIURL(gatewayURL, "/api/v1/auth/cli/start") + if err := postLoginJSON(url, req, "login start failed: ", &result); err != nil { return nil, err } - return &result, nil } -// CompleteLogin polls the server to complete the OAuth login flow -func CompleteLogin(gatewayURL, state, codeVerifier string, device DeviceInfo) (*LoginResponse, error) { - url := buildGatewayAPIURL(gatewayURL, "/api/v1/auth/cli/complete") - +// CompleteLogin redeems the single-use login code with the PKCE code verifier for a session token. +func CompleteLogin(gatewayURL, loginCode, codeVerifier string, device DeviceInfo) (*LoginResponse, error) { payload := map[string]string{ - "state": state, + "login_code": loginCode, "code_verifier": codeVerifier, "device_id": device.ID, "device_name": device.Name, "device_os": device.OS, "client_version": device.ClientVersion, } + var result LoginResponse + url := buildGatewayAPIURL(gatewayURL, "/api/v1/auth/cli/complete") + if err := postLoginJSON(url, payload, "", &result); err != nil { + return nil, err + } + return &result, nil +} +// postLoginJSON POSTs payload as JSON to an unauthenticated login endpoint and decodes the response +// into out. Non-200 responses become errors carrying errPrefix and the gateway's error message. +func postLoginJSON(url string, payload interface{}, errPrefix string, out interface{}) error { data, err := json.Marshal(payload) if err != nil { - return nil, err + return err } - resp, err := sendGatewayRequest(http.MethodPost, url, bytes.NewReader(data)) if err != nil { - return nil, err + return err } defer func() { _ = resp.Body.Close() }() - if resp.StatusCode == http.StatusAccepted { - return nil, ErrLoginPending - } if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("%s", RenderGatewayError(body)) - } - - var result LoginResponse - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return nil, err + return fmt.Errorf("%s%s", errPrefix, RenderGatewayError(body)) } - - return &result, nil + return json.NewDecoder(resp.Body).Decode(out) } // DetermineDeviceInfo gathers information about the current CLI client device diff --git a/internal/cli/cli_login.go b/internal/cli/cli_login.go index 460bf719..456f2d02 100644 --- a/internal/cli/cli_login.go +++ b/internal/cli/cli_login.go @@ -1,11 +1,11 @@ package cli import ( - "errors" "fmt" + "net" + "net/url" "os" "strings" - "time" "github.com/spf13/cobra" ) @@ -30,7 +30,7 @@ Provide both rack name and gateway URL to login to a new rack.`, } cmd.Flags().BoolVar(&noOpen, "no-open", false, "Don't open browser automatically") - cmd.Flags().StringVar(&authFile, "auth-file", "", "Write auth details to file for automation") + cmd.Flags().StringVar(&authFile, "auth-file", "", "Write the login URL to file for automation") return cmd } @@ -43,30 +43,68 @@ func loginCommandWithFlags(args []string, noOpen bool, authFile string) error { fmt.Printf("Starting login for rack: %s via gateway: %s\n", rack, gatewayURL) - startResp, err := StartLogin(gatewayURL) + loginResp, err := runLoopbackLogin(gatewayURL, noOpen, authFile) if err != nil { - return fmt.Errorf("failed to start login: %w", err) + return err } - fmt.Printf("Auth URL: %s\n", startResp.AuthURL) - if err := writeAuthFile(authFile, startResp); err != nil { + if err := finalizeLogin(rack, loginResp); err != nil { return err } - notifyBrowser(startResp.AuthURL, noOpen) + fmt.Printf("✓ Successfully logged in to %s as %s\n", rack, loginResp.Email) + return nil +} + +// runLoopbackLogin performs an RFC 8252 loopback login: the browser hands a single-use login code +// back to this process on 127.0.0.1, and only this process holds the PKCE verifier to redeem it. +func runLoopbackLogin(gatewayURL string, noOpen bool, authFile string) (*LoginResponse, error) { + challenge, err := newPKCE() + if err != nil { + return nil, err + } + state, err := randomURLSafe(32) + if err != nil { + return nil, err + } + + loopback, err := startLoopbackServer(state, gatewayURL) + if err != nil { + return nil, err + } + defer loopback.close() deviceInfo := DetermineDeviceInfo() - loginResp, err := pollLoginCompletion(gatewayURL, startResp, deviceInfo) + startResp, err := StartLogin(gatewayURL, LoginStartRequest{ + CodeChallenge: challenge.challenge, + CodeChallengeMethod: "S256", + RedirectURI: loopback.redirectURI, + State: state, + DeviceName: deviceInfo.Name, + }) if err != nil { - return err + return nil, fmt.Errorf("failed to start login: %w", err) + } + if err := validateAuthURL(startResp.AuthURL, gatewayURL); err != nil { + return nil, err + } + if err := writeAuthFile(authFile, startResp); err != nil { + return nil, err } - if err := finalizeLogin(rack, loginResp); err != nil { - return err + notifyBrowser(startResp.AuthURL, noOpen) + fmt.Println("Waiting for you to finish logging in in your browser...") + + loginCode, err := loopback.wait(loginTimeout) + if err != nil { + return nil, fmt.Errorf("login failed: %w", err) } - fmt.Printf("✓ Successfully logged in to %s as %s\n", rack, loginResp.Email) - return nil + loginResp, err := CompleteLogin(gatewayURL, loginCode, challenge.verifier, deviceInfo) + if err != nil { + return nil, fmt.Errorf("login failed: %w", err) + } + return loginResp, nil } func resolveLoginTarget(args []string) (string, string, error) { @@ -138,16 +176,13 @@ func resolveLoginGatewayURL(rack string) (string, error) { return LoadGatewayURL(rack) } +// writeAuthFile records the login URL for automation (e.g. E2E tests drive the browser step). +// It never contains the PKCE verifier or the CLI's state. func writeAuthFile(path string, startResp *LoginStartResponse) error { if path == "" { return nil } - content := fmt.Sprintf( - "AUTH_URL=%s\nSTATE=%s\nCODE_VERIFIER=%s\n", - startResp.AuthURL, - startResp.State, - startResp.CodeVerifier, - ) + content := fmt.Sprintf("AUTH_URL=%s\n", startResp.AuthURL) if err := os.WriteFile(path, []byte(content), 0o600); err != nil { return fmt.Errorf("failed to write auth file: %w", err) } @@ -156,6 +191,7 @@ func writeAuthFile(path string, startResp *LoginStartResponse) error { func notifyBrowser(authURL string, noOpen bool) { if noOpen { + fmt.Printf("Open this URL in your browser to log in:\n%s\n", authURL) return } fmt.Printf("Opening browser for authentication...\n") @@ -164,33 +200,30 @@ func notifyBrowser(authURL string, noOpen bool) { } } -func pollLoginCompletion( - gatewayURL string, - startResp *LoginStartResponse, - deviceInfo DeviceInfo, -) (*LoginResponse, error) { - deadline := time.Now().Add(2 * time.Minute) - pendingNotified := false - for { - resp, err := CompleteLogin(gatewayURL, startResp.State, startResp.CodeVerifier, deviceInfo) - if err == nil { - return resp, nil - } - - if !errors.Is(err, ErrLoginPending) { - return nil, fmt.Errorf("login failed: %w", err) - } - - if !pendingNotified { - fmt.Println("Waiting for multi-factor authentication to complete in your browser...") - pendingNotified = true - } +// validateAuthURL only lets the CLI open Google's sign-in page, or a loopback identity provider +// when the gateway itself is on a loopback address (local development and tests). +func validateAuthURL(authURL, gatewayURL string) error { + parsed, err := url.Parse(authURL) + if err != nil { + return fmt.Errorf("gateway returned an invalid login URL: %w", err) + } + if parsed.Scheme == "https" && parsed.Hostname() == "accounts.google.com" { + return nil + } + gateway, err := url.Parse(buildGatewayAPIURL(gatewayURL, "")) + if err == nil && isLoopbackHost(gateway.Hostname()) && isLoopbackHost(parsed.Hostname()) && + (parsed.Scheme == "http" || parsed.Scheme == "https") { + return nil + } + return fmt.Errorf("gateway returned an unexpected login URL host %q", parsed.Host) +} - if time.Now().After(deadline) { - return nil, fmt.Errorf("login timed out waiting for browser authentication") - } - time.Sleep(1 * time.Second) +func isLoopbackHost(host string) bool { + if host == "localhost" { + return true } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() } func finalizeLogin(rack string, loginResp *LoginResponse) error { diff --git a/internal/cli/login_loopback.go b/internal/cli/login_loopback.go new file mode 100644 index 00000000..f36e27ba --- /dev/null +++ b/internal/cli/login_loopback.go @@ -0,0 +1,169 @@ +package cli + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "errors" + "fmt" + "html/template" + "net" + "net/http" + "strings" + "time" +) + +// loginTimeout matches the gateway's 10-minute limit for a CLI login. +const loginTimeout = 10 * time.Minute + +var errLoginTimedOut = errors.New("login timed out waiting for browser authentication") + +// pkce holds an RFC 7636 code verifier and its S256 challenge. +type pkce struct { + verifier string + challenge string +} + +func newPKCE() (pkce, error) { + verifier, err := randomURLSafe(64) + if err != nil { + return pkce{}, err + } + sum := sha256.Sum256([]byte(verifier)) + return pkce{verifier: verifier, challenge: base64.RawURLEncoding.EncodeToString(sum[:])}, nil +} + +func randomURLSafe(n int) (string, error) { + buf := make([]byte, n) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("failed to generate random value: %w", err) + } + return base64.RawURLEncoding.EncodeToString(buf), nil +} + +type loopbackResult struct { + code string + err error +} + +// loopbackServer receives the browser redirect that carries the single-use login code (RFC 8252). +// It only listens on 127.0.0.1 and only accepts the redirect carrying this login's state. +type loopbackServer struct { + redirectURI string + state string + gatewayURL string + server *http.Server + results chan loopbackResult +} + +func startLoopbackServer(state, gatewayURL string) (*loopbackServer, error) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return nil, fmt.Errorf("failed to listen for the login redirect on 127.0.0.1: %w", err) + } + addr, ok := listener.Addr().(*net.TCPAddr) + if !ok { + _ = listener.Close() + return nil, fmt.Errorf("unexpected listener address %s", listener.Addr()) + } + + s := &loopbackServer{ + redirectURI: fmt.Sprintf("http://127.0.0.1:%d/callback", addr.Port), + state: state, + gatewayURL: gatewayURL, + results: make(chan loopbackResult, 1), + } + mux := http.NewServeMux() + mux.HandleFunc("/callback", s.handleCallback) + s.server = &http.Server{Handler: mux, ReadHeaderTimeout: 5 * time.Second} + go func() { _ = s.server.Serve(listener) }() + return s, nil +} + +func (s *loopbackServer) handleCallback(w http.ResponseWriter, r *http.Request) { + query := r.URL.Query() + if subtle.ConstantTimeCompare([]byte(query.Get("state")), []byte(s.state)) != 1 { + // Not our login: ignore it and keep waiting for the real redirect. + s.writePage(w, http.StatusBadRequest, "Login link mismatch", + "This login does not match the rack-gateway login waiting in your terminal.") + return + } + if errCode := strings.TrimSpace(query.Get("error")); errCode != "" { + message := loginErrorMessage(errCode) + s.writePage(w, http.StatusOK, "Login failed", message+". Return to your terminal.") + s.deliver(loopbackResult{err: errors.New(message)}) + return + } + code := strings.TrimSpace(query.Get("code")) + if code == "" { + s.writePage(w, http.StatusBadRequest, "Login failed", "The login code is missing. Return to your terminal.") + s.deliver(loopbackResult{err: errors.New("the gateway did not return a login code")}) + return + } + s.writePage(w, http.StatusOK, "Login approved", "You can close this tab and return to your terminal.") + s.deliver(loopbackResult{code: code}) +} + +func (s *loopbackServer) deliver(result loopbackResult) { + select { + case s.results <- result: + default: + } +} + +// wait blocks until the browser delivers the login code, the login fails, or the timeout passes. +func (s *loopbackServer) wait(timeout time.Duration) (string, error) { + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case result := <-s.results: + return result.code, result.err + case <-timer.C: + return "", errLoginTimedOut + } +} + +func (s *loopbackServer) close() { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _ = s.server.Shutdown(ctx) +} + +var loopbackPage = template.Must(template.New("loopback").Parse(` + +{{.Title}} + +

{{.Title}}

+

{{.Message}}

+

Open the Rack Gateway web UI

+ +`)) + +func (s *loopbackServer) writePage(w http.ResponseWriter, status int, title, message string) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Content-Security-Policy", "default-src 'none'") + w.Header().Set("Referrer-Policy", "no-referrer") + w.Header().Set("Cache-Control", "no-store") + w.WriteHeader(status) + _ = loopbackPage.Execute(w, map[string]string{ + "Title": title, + "Message": message, + "WebURL": buildGatewayAPIURL(s.gatewayURL, "/app/"), + }) +} + +// loginErrorMessage maps the gateway's login error codes to messages for the terminal. +func loginErrorMessage(code string) string { + switch code { + case "unauthorized": + return "your account is not authorized for this gateway" + case "exchange_failed": + return "the identity provider login could not be completed" + case "session_incomplete": + return "the login session was incomplete; run rack-gateway login again" + default: + return fmt.Sprintf("the gateway could not complete the login (%s)", code) + } +} diff --git a/internal/cli/login_loopback_test.go b/internal/cli/login_loopback_test.go new file mode 100644 index 00000000..101c26e9 --- /dev/null +++ b/internal/cli/login_loopback_test.go @@ -0,0 +1,151 @@ +package cli + +import ( + "context" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func fetch(target string) (*http.Response, error) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, target, http.NoBody) + if err != nil { + return nil, err + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + _ = resp.Body.Close() + return resp, nil +} + +func getURL(t *testing.T, target string) *http.Response { + t.Helper() + resp, err := fetch(target) + require.NoError(t, err) + return resp +} + +func TestLoopbackServerListensOnLoopbackAndChecksState(t *testing.T) { + server, err := startLoopbackServer("expected-state", "http://127.0.0.1:9447") + require.NoError(t, err) + defer server.close() + + redirect, err := url.Parse(server.redirectURI) + require.NoError(t, err) + require.Equal(t, "http", redirect.Scheme) + require.Equal(t, "127.0.0.1", redirect.Hostname()) + require.Equal(t, "/callback", redirect.Path) + + // A redirect for some other login is rejected and does not end the wait. + resp := getURL(t, server.redirectURI+"?code=attacker&state=wrong-state") + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + _, err = server.wait(50 * time.Millisecond) + require.ErrorIs(t, err, errLoginTimedOut) + + resp = getURL(t, server.redirectURI+"?code=the-login-code&state=expected-state") + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, "default-src 'none'", resp.Header.Get("Content-Security-Policy")) + code, err := server.wait(time.Second) + require.NoError(t, err) + require.Equal(t, "the-login-code", code) +} + +func TestLoopbackServerReportsGatewayError(t *testing.T) { + server, err := startLoopbackServer("expected-state", "http://127.0.0.1:9447") + require.NoError(t, err) + defer server.close() + + getURL(t, server.redirectURI+"?error=unauthorized&state=expected-state") + _, err = server.wait(time.Second) + require.ErrorContains(t, err, "not authorized") +} + +func TestLoopbackServerTimesOut(t *testing.T) { + server, err := startLoopbackServer("expected-state", "http://127.0.0.1:9447") + require.NoError(t, err) + defer server.close() + + _, err = server.wait(20 * time.Millisecond) + require.ErrorIs(t, err, errLoginTimedOut) +} + +func TestValidateAuthURL(t *testing.T) { + cases := []struct { + authURL string + gatewayURL string + ok bool + }{ + {"https://accounts.google.com/o/oauth2/auth?x=1", "https://gateway.example.ts.net", true}, + {"http://accounts.google.com/o/oauth2/auth", "https://gateway.example.ts.net", false}, + {"https://evil.example/login", "https://gateway.example.ts.net", false}, + {"file:///etc/passwd", "https://gateway.example.ts.net", false}, + {"http://localhost:9345/authorize", "http://127.0.0.1:9447", true}, + {"http://localhost:9345/authorize", "https://gateway.example.ts.net", false}, + {"javascript:alert(1)", "http://127.0.0.1:9447", false}, + } + for _, tc := range cases { + err := validateAuthURL(tc.authURL, tc.gatewayURL) + if tc.ok { + require.NoErrorf(t, err, "%s via %s", tc.authURL, tc.gatewayURL) + } else { + require.Errorf(t, err, "%s via %s", tc.authURL, tc.gatewayURL) + } + } +} + +// fakeLoopbackGateway plays the gateway: it records the start request and only completes a login +// for the issued login code together with a verifier matching the challenge. +type fakeLoopbackGateway struct { + t *testing.T + start LoginStartRequest +} + +func (g *fakeLoopbackGateway) ServeHTTP(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/auth/cli/start": + require.NoError(g.t, json.NewDecoder(r.Body).Decode(&g.start)) + // Play the browser: deliver the login code to the CLI's loopback listener. + go func() { + _, _ = fetch(g.start.RedirectURI + "?code=issued-code&state=" + url.QueryEscape(g.start.State)) + }() + _ = json.NewEncoder(w).Encode(LoginStartResponse{AuthURL: "https://accounts.google.com/o/oauth2/auth"}) + case "/api/v1/auth/cli/complete": + var body map[string]string + require.NoError(g.t, json.NewDecoder(r.Body).Decode(&body)) + sum := sha256.Sum256([]byte(body["code_verifier"])) + if body["login_code"] != "issued-code" || + base64.RawURLEncoding.EncodeToString(sum[:]) != g.start.CodeChallenge { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"invalid or expired login code"}`)) + return + } + _ = json.NewEncoder(w).Encode(LoginResponse{Token: "session-token", Email: "user@example.com"}) + default: + w.WriteHeader(http.StatusNotFound) + } +} + +func TestRunLoopbackLogin(t *testing.T) { + ConfigPath = t.TempDir() + gateway := &fakeLoopbackGateway{t: t} + server := httptest.NewServer(gateway) + defer server.Close() + + resp, err := runLoopbackLogin(server.URL, true, "") + require.NoError(t, err) + require.Equal(t, "session-token", resp.Token) + + require.Equal(t, "S256", gateway.start.CodeChallengeMethod) + require.Len(t, gateway.start.CodeChallenge, 43) + require.GreaterOrEqual(t, len(gateway.start.State), 32) + require.Regexp(t, `^http://127\.0\.0\.1:\d+/callback$`, gateway.start.RedirectURI) +} diff --git a/internal/cli/types.go b/internal/cli/types.go index 623942af..56e72918 100644 --- a/internal/cli/types.go +++ b/internal/cli/types.go @@ -46,18 +46,19 @@ type RackStatus struct { StatusLines []string } -// LoginStartResponse is the response from /api/v1/auth/cli/start -type LoginStartResponse struct { - AuthURL string `json:"auth_url"` - State string `json:"state"` - CodeVerifier string `json:"code_verifier"` +// LoginStartRequest is the request to /api/v1/auth/cli/start. The CLI keeps the PKCE code +// verifier; the gateway only sees its S256 challenge. +type LoginStartRequest struct { + CodeChallenge string `json:"code_challenge"` + CodeChallengeMethod string `json:"code_challenge_method"` + RedirectURI string `json:"redirect_uri"` + State string `json:"state"` + DeviceName string `json:"device_name"` } -// LoginCallbackRequest is the request to /api/v1/auth/cli/complete -type LoginCallbackRequest struct { - Code string `json:"code"` - State string `json:"state"` - CodeVerifier string `json:"code_verifier"` +// LoginStartResponse is the response from /api/v1/auth/cli/start +type LoginStartResponse struct { + AuthURL string `json:"auth_url"` } // LoginResponse is the response from /api/v1/auth/cli/complete @@ -102,9 +103,6 @@ type MFAMethodResponse struct { IsEnrolling bool `json:"is_enrolling"` } -// ErrLoginPending is returned when login is still pending browser completion -var ErrLoginPending = errors.New("login pending") - // ErrTokenExpired is returned when a stored or gateway-authenticated token has expired. var ErrTokenExpired = errors.New("token expired") diff --git a/internal/cli/utils.go b/internal/cli/utils.go index 00b75b8f..06d09c81 100644 --- a/internal/cli/utils.go +++ b/internal/cli/utils.go @@ -55,8 +55,9 @@ func OpenBrowser(url string) error { cmd = "xdg-open" args = []string{url} case "windows": - cmd = "cmd" - args = []string{"/c", "start", url} + // rundll32 receives the URL as one argument; `cmd /c start` would let '&' split the command. + cmd = "rundll32" + args = []string{"url.dll,FileProtocolHandler", url} default: return fmt.Errorf("unsupported platform") } diff --git a/internal/gateway/audit/logger.go b/internal/gateway/audit/logger.go index ddac6a21..468f7ab2 100644 --- a/internal/gateway/audit/logger.go +++ b/internal/gateway/audit/logger.go @@ -233,7 +233,7 @@ func (l *Logger) LogRequest( UserEmail: userEmail, Method: r.Method, Path: path, - QueryParams: r.URL.RawQuery, + QueryParams: RedactQuery(r.URL.RawQuery), Status: status, LatencyMs: latency.Milliseconds(), RBACDecision: rbacDecision, @@ -368,9 +368,8 @@ func (_ *Logger) BuildDetailsJSON(r *http.Request) string { "path": r.URL.Path, } - // Add query parameters as-is (only app IDs and pagination params) if r.URL.RawQuery != "" { - details["query"] = r.URL.RawQuery + details["query"] = RedactQuery(r.URL.RawQuery) } // For exec, include command and process id if available diff --git a/internal/gateway/audit/query_redaction_test.go b/internal/gateway/audit/query_redaction_test.go new file mode 100644 index 00000000..cdbddfeb --- /dev/null +++ b/internal/gateway/audit/query_redaction_test.go @@ -0,0 +1,36 @@ +package audit + +import ( + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRedactQueryHidesLoginSecrets(t *testing.T) { + redacted := RedactQuery("code=4%2F0AQSTgQ-googlecode&state=SECRET-STATE&login_code=abc&app=web&limit=10") + values, err := url.ParseQuery(redacted) + require.NoError(t, err) + require.Equal(t, "[REDACTED]", values.Get("code")) + require.Equal(t, "[REDACTED]", values.Get("state")) + require.Equal(t, "[REDACTED]", values.Get("login_code")) + require.Equal(t, "web", values.Get("app")) + require.Equal(t, "10", values.Get("limit")) + require.NotContains(t, redacted, "SECRET-STATE") + require.NotContains(t, redacted, "googlecode") +} + +func TestRedactQueryEdgeCases(t *testing.T) { + require.Empty(t, RedactQuery("")) + require.Equal(t, "[UNPARSEABLE]", RedactQuery("state=%zz")) + require.Equal(t, "STATE=%5BREDACTED%5D", RedactQuery("STATE=x")) +} + +func TestBuildDetailsJSONRedactsQuery(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/api/v1/auth/cli/callback?code=c0de&state=s3cr3t", http.NoBody) + require.NoError(t, err) + details := (&Logger{}).BuildDetailsJSON(req) + require.NotContains(t, details, "c0de") + require.NotContains(t, details, "s3cr3t") +} diff --git a/internal/gateway/audit/redaction.go b/internal/gateway/audit/redaction.go index c634a3f7..a88c47ab 100644 --- a/internal/gateway/audit/redaction.go +++ b/internal/gateway/audit/redaction.go @@ -1,6 +1,38 @@ package audit -import "regexp" +import ( + "net/url" + "regexp" + "strings" +) + +// sensitiveQueryParams carry one-time login credentials or OAuth values and must never be logged. +var sensitiveQueryParams = map[string]struct{}{ + "code": {}, + "state": {}, + "login_code": {}, + "code_verifier": {}, + "token": {}, + "access_token": {}, + "id_token": {}, +} + +// RedactQuery returns rawQuery with the values of sensitive parameters replaced by [REDACTED]. +func RedactQuery(rawQuery string) string { + if rawQuery == "" { + return "" + } + values, err := url.ParseQuery(rawQuery) + if err != nil { + return "[UNPARSEABLE]" + } + for key := range values { + if _, sensitive := sensitiveQueryParams[strings.ToLower(key)]; sensitive { + values[key] = []string{"[REDACTED]"} + } + } + return values.Encode() +} func (l *Logger) redactMap(data map[string]interface{}) map[string]interface{} { redacted := make(map[string]interface{}) diff --git a/internal/gateway/db/cli_login.go b/internal/gateway/db/cli_login.go index b7b57638..216d682b 100644 --- a/internal/gateway/db/cli_login.go +++ b/internal/gateway/db/cli_login.go @@ -6,69 +6,97 @@ import ( "time" ) -// CLILoginState captures the persisted state for CLI OAuth flows. +// cliLoginLive restricts queries to CLI logins started within the last 10 minutes. +const cliLoginLive = "created_at > NOW() - INTERVAL '10 minutes'" + +// CLILoginState is a CLI login in progress. The state column is the gateway's own OAuth +// state for the Google leg; the CLI never needs it. The CLI is identified by its PKCE code +// challenge, its loopback redirect URI and its own state value. type CLILoginState struct { - State string - Code sql.NullString - CodeVerifier sql.NullString - LoginToken sql.NullString - LoginEmail sql.NullString - LoginName sql.NullString - LoginExpiresAt sql.NullTime - MFAVerifiedAt sql.NullTime - MFAMethodID sql.NullInt64 - LoginError sql.NullString - CreatedAt time.Time - UpdatedAt time.Time + State string + OAuthCode sql.NullString + OAuthCodeVerifier sql.NullString + CLICodeChallenge string + CLIRedirectURI string + CLIState string + InitiatorIP sql.NullString + InitiatorDevice sql.NullString + BrowserBindingHash sql.NullString + EnrollmentRequired bool + LoginEmail sql.NullString + LoginName sql.NullString + MFAVerifiedAt sql.NullTime + MFAMethodID sql.NullInt64 + LoginError sql.NullString + CreatedAt time.Time + UpdatedAt time.Time +} + +// NewCLILogin holds the values recorded when a CLI starts a login. +type NewCLILogin struct { + State string + OAuthCodeVerifier string + CLICodeChallenge string + CLIRedirectURI string + CLIState string + InitiatorIP string + InitiatorDevice string } -// StoreCLILoginState upserts the state with the generated PKCE verifier. -func (d *Database) StoreCLILoginState(state, codeVerifier string) error { +const cliLoginColumns = `state, oauth_code, oauth_code_verifier, cli_code_challenge, cli_redirect_uri, cli_state, + initiator_ip, initiator_device, browser_binding_hash, enrollment_required, login_email, login_name, + mfa_verified_at, mfa_method_id, login_error, created_at, updated_at` + +// CreateCLILoginState records a new CLI login and removes expired ones. +func (d *Database) CreateCLILoginState(login NewCLILogin) error { + if err := d.DeleteExpiredCLILoginStates(); err != nil { + return err + } _, err := d.exec(` - INSERT INTO cli_login_states (state, code_verifier, created_at, updated_at) - VALUES (?, ?, NOW(), NOW()) - ON CONFLICT (state) - DO UPDATE SET code_verifier = EXCLUDED.code_verifier, - code = NULL, - login_token = NULL, - login_email = NULL, - login_name = NULL, - login_expires_at = NULL, - mfa_verified_at = NULL, - mfa_method_id = NULL, - login_error = NULL, - updated_at = NOW() - `, state, codeVerifier) + INSERT INTO cli_login_states ( + state, oauth_code_verifier, cli_code_challenge, cli_redirect_uri, cli_state, + initiator_ip, initiator_device, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, NOW(), NOW()) + `, + login.State, + login.OAuthCodeVerifier, + login.CLICodeChallenge, + login.CLIRedirectURI, + login.CLIState, + nullableString(login.InitiatorIP, 64), + nullableString(login.InitiatorDevice, 64), + ) if err != nil { return fmt.Errorf("failed to store CLI login state: %w", err) } return nil } -// UpdateCLILoginCode records the authorization code returned by the IdP. -func (d *Database) UpdateCLILoginCode(state, code string) error { - _, err := d.exec(`UPDATE cli_login_states SET code = ?, updated_at = NOW() WHERE state = ?`, code, state) +// BindCLILoginBrowser stores the identity provider's authorization code and binds the login to +// the browser that delivered it. It only succeeds once per login, so a replayed callback URL +// cannot bind a second browser. Returns false when the login is unknown, expired or already bound. +func (d *Database) BindCLILoginBrowser(state, oauthCode, bindingHash string) (bool, error) { + res, err := d.exec(` + UPDATE cli_login_states + SET oauth_code = ?, browser_binding_hash = ?, updated_at = NOW() + WHERE state = ? AND browser_binding_hash IS NULL AND `+cliLoginLive, + oauthCode, bindingHash, state) if err != nil { - return fmt.Errorf("failed to update CLI login code: %w", err) + return false, fmt.Errorf("failed to bind CLI login browser: %w", err) } - return nil -} - -// SaveCLILoginResult persists the successful login response after MFA verification. -func (d *Database) SaveCLILoginResult(state, email, name string, methodID *int64) error { - if err := d.SetCLILoginProfile(state, email, name); err != nil { - return err + rows, err := res.RowsAffected() + if err != nil { + return false, fmt.Errorf("failed to bind CLI login browser: %w", err) } - return d.MarkCLILoginVerified(state, methodID) + return rows == 1, nil } -// SetCLILoginProfile stores the OAuth exchange result so the CLI can poll for completion while -// additional MFA checks (or enrollment) are performed. +// SetCLILoginProfile stores the identity provider result and discards the authorization code. func (d *Database) SetCLILoginProfile(state, email, name string) error { _, err := d.exec(` UPDATE cli_login_states - SET code = NULL, - code_verifier = NULL, + SET oauth_code = NULL, + oauth_code_verifier = NULL, login_email = ?, login_name = ?, login_error = NULL, @@ -97,8 +125,20 @@ func (d *Database) MarkCLILoginVerified(state string, methodID *int64) error { return nil } -// FailCLILoginState stores a terminal error for a CLI login attempt so polling clients can -// stop waiting and surface the failure message. +// MarkCLILoginEnrollmentRequired records that the user had no MFA factor when the login reached MFA, +// so a factor enrolled in the bound browser during this login may satisfy the login's MFA requirement. +func (d *Database) MarkCLILoginEnrollmentRequired(state string) error { + _, err := d.exec( + `UPDATE cli_login_states SET enrollment_required = TRUE, updated_at = NOW() WHERE state = ?`, + state, + ) + if err != nil { + return fmt.Errorf("failed to mark CLI login enrollment required: %w", err) + } + return nil +} + +// FailCLILoginState stores a terminal error for a CLI login attempt. func (d *Database) FailCLILoginState(state, reason string) error { _, err := d.exec(` UPDATE cli_login_states @@ -112,24 +152,73 @@ func (d *Database) FailCLILoginState(state, reason string) error { return nil } -// GetCLILoginState retrieves the persisted CLI login state for the given key. +// GetCLILoginState retrieves a live (unexpired) CLI login. func (d *Database) GetCLILoginState(state string) (*CLILoginState, error) { - query := ` - SELECT state, code, code_verifier, login_token, login_email, login_name, - login_expires_at, mfa_verified_at, mfa_method_id, login_error, - created_at, updated_at - FROM cli_login_states WHERE state = ? - ` + row := d.queryRow(`SELECT `+cliLoginColumns+` FROM cli_login_states WHERE state = ? AND `+cliLoginLive, state) + record, err := scanCLILoginState(row) + if err != nil { + return nil, fmt.Errorf("failed to get CLI login state: %w", err) + } + return record, nil +} +// SetCLILoginCode stores the hash of a single-use login code for a verified CLI login. +// The code is valid for two minutes. Returns false when the login is not live or not verified. +func (d *Database) SetCLILoginCode(state, codeHash string) (bool, error) { + res, err := d.exec(` + UPDATE cli_login_states + SET login_code_hash = ?, + login_code_expires_at = NOW() + INTERVAL '2 minutes', + updated_at = NOW() + WHERE state = ? AND mfa_verified_at IS NOT NULL AND login_error IS NULL AND `+cliLoginLive, + codeHash, state) + if err != nil { + return false, fmt.Errorf("failed to store CLI login code: %w", err) + } + rows, err := res.RowsAffected() + if err != nil { + return false, fmt.Errorf("failed to store CLI login code: %w", err) + } + return rows == 1, nil +} + +// ConsumeCLILoginCode atomically deletes and returns the live CLI login holding the login code. +// Each code can be redeemed at most once. Returns nil when no live login holds the code. +func (d *Database) ConsumeCLILoginCode(codeHash string) (*CLILoginState, error) { + row := d.queryRow(` + DELETE FROM cli_login_states + WHERE login_code_hash = ? AND login_code_expires_at > NOW() AND `+cliLoginLive+` + RETURNING `+cliLoginColumns, codeHash) + record, err := scanCLILoginState(row) + if err != nil { + return nil, fmt.Errorf("failed to consume CLI login code: %w", err) + } + return record, nil +} + +// DeleteExpiredCLILoginStates removes CLI logins older than the login time limit. +func (d *Database) DeleteExpiredCLILoginStates() error { + if _, err := d.exec(`DELETE FROM cli_login_states WHERE NOT (` + cliLoginLive + `)`); err != nil { + return fmt.Errorf("failed to delete expired CLI login states: %w", err) + } + return nil +} + +func scanCLILoginState(row *sql.Row) (*CLILoginState, error) { var record CLILoginState - err := d.queryRow(query, state).Scan( + err := row.Scan( &record.State, - &record.Code, - &record.CodeVerifier, - &record.LoginToken, + &record.OAuthCode, + &record.OAuthCodeVerifier, + &record.CLICodeChallenge, + &record.CLIRedirectURI, + &record.CLIState, + &record.InitiatorIP, + &record.InitiatorDevice, + &record.BrowserBindingHash, + &record.EnrollmentRequired, &record.LoginEmail, &record.LoginName, - &record.LoginExpiresAt, &record.MFAVerifiedAt, &record.MFAMethodID, &record.LoginError, @@ -140,20 +229,11 @@ func (d *Database) GetCLILoginState(state string) (*CLILoginState, error) { return nil, nil } if err != nil { - return nil, fmt.Errorf("failed to get CLI login state: %w", err) + return nil, err } return &record, nil } -// DeleteCLILoginState removes the stored CLI login state. -func (d *Database) DeleteCLILoginState(state string) error { - _, err := d.exec(`DELETE FROM cli_login_states WHERE state = ?`, state) - if err != nil { - return fmt.Errorf("failed to delete CLI login state: %w", err) - } - return nil -} - func nullableInt64(v *int64) interface{} { if v == nil { return nil diff --git a/internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql b/internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql new file mode 100644 index 00000000..75574084 --- /dev/null +++ b/internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql @@ -0,0 +1,26 @@ +-- CLI login now uses an RFC 8252 loopback redirect: +-- * the CLI sends an S256 code challenge, its own state and a 127.0.0.1 redirect URI to /auth/cli/start +-- * the browser that completes Google OAuth is bound to the login with an HttpOnly cookie +-- * after MFA the browser is redirected to the CLI's loopback listener with a single-use login code +-- * /auth/cli/complete requires that login code AND the CLI's code verifier +-- Login states only live for minutes, so existing rows are discarded. +DELETE FROM cli_login_states; + +ALTER TABLE cli_login_states RENAME COLUMN code TO oauth_code; +ALTER TABLE cli_login_states RENAME COLUMN code_verifier TO oauth_code_verifier; + +ALTER TABLE cli_login_states + DROP COLUMN login_token, + DROP COLUMN login_expires_at, + ALTER COLUMN oauth_code TYPE TEXT, + ADD COLUMN cli_code_challenge VARCHAR(64) NOT NULL, + ADD COLUMN cli_redirect_uri VARCHAR(64) NOT NULL, + ADD COLUMN cli_state VARCHAR(128) NOT NULL, + ADD COLUMN initiator_ip VARCHAR(64), + ADD COLUMN initiator_device VARCHAR(64), + ADD COLUMN browser_binding_hash VARCHAR(64), + ADD COLUMN enrollment_required BOOLEAN NOT NULL DEFAULT FALSE, + ADD COLUMN login_code_hash VARCHAR(64) UNIQUE, + ADD COLUMN login_code_expires_at TIMESTAMPTZ; + +CREATE INDEX IF NOT EXISTS cli_login_states_created_at_idx ON cli_login_states (created_at); diff --git a/internal/gateway/handlers/auth_cli.go b/internal/gateway/handlers/auth_cli.go index aedab020..8468b117 100644 --- a/internal/gateway/handlers/auth_cli.go +++ b/internal/gateway/handlers/auth_cli.go @@ -1,6 +1,7 @@ package handlers import ( + "errors" "fmt" "log" "net/http" @@ -9,6 +10,7 @@ import ( "github.com/gin-gonic/gin" + "github.com/DocSpring/rack-gateway/internal/gateway/auth" "github.com/DocSpring/rack-gateway/internal/gateway/auth/mfa" "github.com/DocSpring/rack-gateway/internal/gateway/db" ) @@ -22,14 +24,32 @@ type cliMFASubmit struct { } // CLILoginStart godoc -// @Summary Start CLI OAuth login -// @Description Initiates the CLI OAuth flow and returns PKCE parameters. +// @Summary Start CLI login +// @Description Starts a loopback CLI login and returns the identity provider URL to open in the browser. // @Tags Auth +// @Accept json // @Produce json -// @Success 200 {object} auth.LoginStartResponse +// @Param request body CLILoginStartRequest true "CLI login parameters" +// @Success 200 {object} CLILoginStartResponse +// @Failure 400 {object} ErrorResponse // @Failure 500 {object} ErrorResponse // @Router /auth/cli/start [post] func (h *AuthHandler) CLILoginStart(c *gin.Context) { + var req CLILoginStartRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) + return + } + deviceName, err := validateCLIStart(req) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + if h.database == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "service_unavailable"}) + return + } + resp, err := h.oauth.StartLogin() if err != nil { h.auditLogin(c, "cli", "error") @@ -37,58 +57,35 @@ func (h *AuthHandler) CLILoginStart(c *gin.Context) { return } - if h.database != nil { - if err := h.database.StoreCLILoginState(resp.State, resp.CodeVerifier); err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to initialize login state"}) - return - } + if err := h.database.CreateCLILoginState(db.NewCLILogin{ + State: resp.State, + OAuthCodeVerifier: resp.CodeVerifier, + CLICodeChallenge: req.CodeChallenge, + CLIRedirectURI: req.RedirectURI, + CLIState: req.State, + InitiatorIP: c.ClientIP(), + InitiatorDevice: deviceName, + }); err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to initialize login state"}) + return } h.auditLogin(c, "cli", "success") - c.JSON(http.StatusOK, resp) + c.JSON(http.StatusOK, CLILoginStartResponse{AuthURL: resp.AuthURL}) } // CLILoginCallback godoc -// @Summary Complete CLI OAuth redirect -// @Description Stores the OAuth authorization code for the CLI to finish login. +// @Summary Identity provider redirect for CLI login +// @Description Stores the authorization code and binds the login to this browser. // @Tags Auth // @Param code query string true "Authorization code" // @Param state query string true "State" // @Success 307 {string} string "Temporary Redirect" -// @Failure 400 {string} string "Missing parameters" // @Router /auth/cli/callback [get] func (h *AuthHandler) CLILoginCallback(c *gin.Context) { code := strings.TrimSpace(c.Query("code")) state := strings.TrimSpace(c.Query("state")) - if code == "" || state == "" { - c.String(http.StatusBadRequest, "Missing code or state") - return - } - - // Store the auth code in database for CLI polling - if h.database != nil { - if err := h.database.UpdateCLILoginCode(state, code); err != nil { - c.String(http.StatusInternalServerError, "Failed to persist login state") - return - } - } - - redirect := fmt.Sprintf("%s?state=%s", APIRoute("auth/cli/mfa"), url.QueryEscape(state)) - c.Redirect(http.StatusTemporaryRedirect, redirect) -} - -// CLILoginMFAForm godoc -// @Summary Display MFA challenge form -// @Description Displays the MFA challenge form for CLI login. -// @Tags Auth -// @Param state query string true "State" -// @Success 307 {string} string "Temporary Redirect" -// @Failure 400 {string} string "Missing parameters" -// @Router /auth/cli/mfa [get] -func (h *AuthHandler) CLILoginMFAForm(c *gin.Context) { - state := strings.TrimSpace(c.Query("state")) - if state == "" { cliRedirectWithError(c, "missing_state") return } @@ -97,41 +94,54 @@ func (h *AuthHandler) CLILoginMFAForm(c *gin.Context) { return } - record, err := h.database.GetCLILoginState(state) + binding, err := newCLISecret() + if err != nil { + cliRedirectWithError(c, "persist_failure") + return + } + bound, err := h.database.BindCLILoginBrowser(state, code, hashCLISecret(binding)) if err != nil { cliRedirectWithError(c, "load_failure") return } - if record == nil { + if !bound { cliRedirectWithError(c, "expired") return } - if record.LoginError.Valid { - challengeRoute := WebRoute("auth/mfa/challenge") - params := url.Values{} - params.Set("error", strings.TrimSpace(record.LoginError.String)) - c.Redirect(http.StatusTemporaryRedirect, buildChallengeURL(challengeRoute, params)) - return - } + h.setCLILoginCookie(c, binding, cliLoginCookieMaxAge) + c.Redirect(http.StatusTemporaryRedirect, cliMFARoute(state)) +} - if record.MFAVerifiedAt.Valid { - c.Redirect(http.StatusTemporaryRedirect, WebRoute("cli/auth/success")) +// CLILoginMFAForm godoc +// @Summary Continue CLI login in the browser +// @Description Completes the identity provider exchange and sends the bound browser to MFA or back to the CLI. +// @Tags Auth +// @Param state query string true "State" +// @Success 307 {string} string "Temporary Redirect" +// @Router /auth/cli/mfa [get] +func (h *AuthHandler) CLILoginMFAForm(c *gin.Context) { + state := strings.TrimSpace(c.Query("state")) + record, ok := h.liveBoundCLILogin(c, state) + if !ok { return } - - loginEmail := h.resolveCLILoginEmail(c, record, state) - if loginEmail == "" { + if record.MFAVerifiedAt.Valid { + c.Redirect(http.StatusTemporaryRedirect, cliReturnRoute(state)) return } - userRecord, ok := h.cliLoadAndValidateUser(c, state, loginEmail) + userRecord, ok := h.cliResolveUser(c, record) if !ok { return } if !shouldEnforceMFA(h.mfaSettings, userRecord) { - h.cliHandleNoMFARequired(c, state) + if err := h.database.MarkCLILoginVerified(state, nil); err != nil { + cliRedirectWithError(c, "persist_failure") + return + } + c.Redirect(http.StatusTemporaryRedirect, cliReturnRoute(state)) return } @@ -142,42 +152,93 @@ func (h *AuthHandler) CLILoginMFAForm(c *gin.Context) { } if !userRecord.MFAEnrolled { - h.cliHandleEnrollmentRequired(c, state, userRecord.Email) + h.cliRedirectToEnrollment(c, state) return } - challengeRoute := WebRoute("auth/mfa/challenge") - params := url.Values{} - params.Set("state", state) - challengeURL := buildChallengeURL(challengeRoute, params) - log.Printf( - "CLI callback: redirecting to MFA challenge: url=%s user=%s state=%s", - challengeURL, - userRecord.Email, - state, - ) - c.Redirect(http.StatusTemporaryRedirect, challengeURL) + c.Redirect(http.StatusTemporaryRedirect, cliChallengeURL(record)) } -func (h *AuthHandler) resolveCLILoginEmail( - c *gin.Context, - record *db.CLILoginState, - state string, -) string { - var loginEmail string +// cliResolveUser exchanges the identity provider code (once) and loads the gateway user. +// On failure the login is ended and the browser is sent back to the CLI with the reason. +func (h *AuthHandler) cliResolveUser(c *gin.Context, record *db.CLILoginState) (*db.User, bool) { + email := "" if record.LoginEmail.Valid { - loginEmail = strings.TrimSpace(record.LoginEmail.String) + email = strings.TrimSpace(record.LoginEmail.String) + } + if email == "" { + exchanged, errCode := h.cliExchangeOAuthCode(record) + if errCode != "" { + h.failCLILogin(c, record, errCode) + return nil, false + } + email = exchanged + record.LoginEmail.String, record.LoginEmail.Valid = exchanged, true + } + + userRecord, err := h.database.GetUser(email) + if err != nil { + cliRedirectWithError(c, "load_failure") + return nil, false + } + if userRecord == nil { + h.notifyUnauthorizedCLILogin(c, record) + h.failCLILogin(c, record, "unauthorized") + return nil, false + } + return userRecord, true +} + +// cliExchangeOAuthCode exchanges the stored identity provider code and records the profile. +// Returns the login email, or an error code for the CLI. +func (h *AuthHandler) cliExchangeOAuthCode(record *db.CLILoginState) (string, string) { + if !record.OAuthCode.Valid || !record.OAuthCodeVerifier.Valid { + return "", "session_incomplete" + } + loginResp, err := h.oauth.CompleteLogin(record.OAuthCode.String, record.State, record.OAuthCodeVerifier.String) + if err != nil { + var domainErr *auth.DomainNotAllowedError + if errors.As(err, &domainErr) { + return "", "unauthorized" + } + return "", "exchange_failed" + } + if err := h.database.SetCLILoginProfile(record.State, loginResp.Email, loginResp.Name); err != nil { + return "", "persist_failure" } + record.LoginName.String, record.LoginName.Valid = loginResp.Name, true + return strings.TrimSpace(loginResp.Email), "" +} - if loginEmail != "" && record.LoginToken.Valid && record.LoginExpiresAt.Valid { - return loginEmail +func (h *AuthHandler) cliRedirectToEnrollment(c *gin.Context, state string) { + if err := h.database.MarkCLILoginEnrollmentRequired(state); err != nil { + cliRedirectWithError(c, "persist_failure") + return } + params := url.Values{} + params.Set("enrollment", "required") + params.Set("channel", "cli") + params.Set("state", state) + c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s?%s", WebRoute("account/security"), params.Encode())) +} - email, _ := h.cliExchangeOAuthCode(c, record, state, false) - return email +// cliChallengeURL builds the MFA challenge page URL, including where the login was started so the +// user can spot a login they did not start. +func cliChallengeURL(record *db.CLILoginState) string { + params := url.Values{} + params.Set("state", record.State) + if record.InitiatorDevice.Valid && record.InitiatorDevice.String != "" { + params.Set("device", record.InitiatorDevice.String) + } + if record.InitiatorIP.Valid && record.InitiatorIP.String != "" { + params.Set("ip", record.InitiatorIP.String) + } + return buildChallengeURL(WebRoute("auth/mfa/challenge"), params) } -// CLILoginMFASubmit handles both TOTP and WebAuthn verification for CLI login +// CLILoginMFASubmit handles both TOTP and WebAuthn verification for CLI login. +// Only the browser bound to the login may submit codes, so a third party who learns the +// login state cannot burn the user's MFA attempts. func (h *AuthHandler) CLILoginMFASubmit(c *gin.Context) { if h.database == nil || h.mfaService == nil { c.JSON(http.StatusServiceUnavailable, gin.H{"error": "service_unavailable"}) @@ -189,16 +250,17 @@ func (h *AuthHandler) CLILoginMFASubmit(c *gin.Context) { return } - record, ok := h.loadMFALoginState(c, parsed.state) - if !ok { + record, errCode := h.loadBoundCLILogin(c, parsed.state) + if errCode != "" { + c.JSON(http.StatusBadRequest, gin.H{"error": errCode}) return } - if h.shortCircuitIfAlreadyVerified(c, record) { + if record.MFAVerifiedAt.Valid { + c.JSON(http.StatusOK, gin.H{"redirect": cliReturnRoute(parsed.state)}) return } - - record, ok = h.cliExchangeIfNeeded(c, record, parsed.state, true) - if !ok { + if !record.LoginEmail.Valid { + c.JSON(http.StatusBadRequest, gin.H{"error": "session_incomplete"}) return } @@ -223,7 +285,7 @@ func (h *AuthHandler) CLILoginMFASubmit(c *gin.Context) { return } - c.JSON(http.StatusOK, gin.H{"redirect": WebRoute("cli/auth/success")}) + c.JSON(http.StatusOK, gin.H{"redirect": cliReturnRoute(parsed.state)}) } // parseMFASubmitRequest parses and validates the MFA submit request payload. @@ -261,29 +323,6 @@ func (_ *AuthHandler) parseMFASubmitRequest(c *gin.Context) (cliMFASubmit, bool) }, true } -// loadMFALoginState loads the CLI login state or writes an error response. -func (h *AuthHandler) loadMFALoginState(c *gin.Context, state string) (*db.CLILoginState, bool) { - record, err := h.database.GetCLILoginState(state) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "load_failure"}) - return nil, false - } - if record == nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "session_expired"}) - return nil, false - } - return record, true -} - -// shortCircuitIfAlreadyVerified responds immediately if MFA is already verified. -func (_ *AuthHandler) shortCircuitIfAlreadyVerified(c *gin.Context, record *db.CLILoginState) bool { - if record.MFAVerifiedAt.Valid { - c.JSON(http.StatusOK, gin.H{"redirect": WebRoute("cli/auth/success")}) - return true - } - return false -} - // markCLILoginVerified persists MFA verification and handles errors. func (h *AuthHandler) markCLILoginVerified(c *gin.Context, state string, verification *mfa.VerificationResult) bool { var methodID *int64 @@ -337,51 +376,3 @@ func (h *AuthHandler) performMFAVerification( return verification, nil } - -// CLILoginComplete godoc -// @Summary Finalize CLI OAuth login -// @Description Exchanges the stored authorization code and PKCE verifier for a session token. -// @Tags Auth -// @Accept json -// @Produce json -// @Param request body CLILoginCompleteRequest true "CLI login payload" -// @Success 200 {object} CLILoginResponse -// @Failure 400 {object} ErrorResponse -// @Failure 500 {object} ErrorResponse -// @Router /auth/cli/complete [post] -func (h *AuthHandler) CLILoginComplete(c *gin.Context) { - req, ok := h.bindCLILoginComplete(c) - if !ok { - return - } - - record, ok := h.loadCLILoginState(c, req.State) - if !ok { - return - } - - if handled := h.handleCLIRecordErrors(c, record); handled { - return - } - - if h.respondIfPending(c, record) { - return - } - - userRecord, ok := h.loadUserForCLI(c, record) - if !ok { - return - } - - sessionToken, session, ok := h.createCLISessionOrRespond(c, userRecord, req) - if !ok { - return - } - - h.applyMFATimestamps(record, session) - - response := h.buildCLILoginResponse(userRecord, record, sessionToken, session) - _ = h.database.DeleteCLILoginState(req.State) - h.notifyCLILoginComplete(c, userRecord) - c.JSON(http.StatusOK, response) -} diff --git a/internal/gateway/handlers/auth_cli_complete.go b/internal/gateway/handlers/auth_cli_complete.go index 77ed869b..cc7831ff 100644 --- a/internal/gateway/handlers/auth_cli_complete.go +++ b/internal/gateway/handlers/auth_cli_complete.go @@ -15,119 +15,69 @@ import ( "github.com/DocSpring/rack-gateway/internal/gateway/db" ) +const cliSessionTTL = 90 * 24 * time.Hour + func cliRedirectWithError(c *gin.Context, errorCode string) { - challengeRoute := WebRoute("auth/mfa/challenge") params := url.Values{} params.Set("error", errorCode) - c.Redirect(http.StatusTemporaryRedirect, buildChallengeURL(challengeRoute, params)) + c.Redirect(http.StatusTemporaryRedirect, buildChallengeURL(WebRoute("auth/mfa/challenge"), params)) } func buildChallengeURL(base string, params url.Values) string { return fmt.Sprintf("%s?%s", base, params.Encode()) } -func (h *AuthHandler) cliLoadAndValidateUser(c *gin.Context, state, loginEmail string) (*db.User, bool) { - userRecord, err := h.database.GetUser(loginEmail) - if err != nil { - cliRedirectWithError(c, "load_failure") - return nil, false - } - if userRecord == nil { - if err := h.database.FailCLILoginState(state, "User not authorized for this gateway."); err != nil { - log.Printf("cli login fail (unknown user): state=%s err=%v", state, err) - } - cliRedirectWithError(c, "unauthorized") - return nil, false - } - return userRecord, true -} - -func (h *AuthHandler) cliHandleNoMFARequired(c *gin.Context, state string) { - if err := h.database.MarkCLILoginVerified(state, nil); err != nil { - log.Printf("cli login mark verified failed: state=%s err=%v", state, err) - cliRedirectWithError(c, "persist_failure") +// CLILoginComplete godoc +// @Summary Finalize CLI login +// @Description Redeems the single-use login code delivered to the CLI's loopback listener, proving +// @Description possession of the PKCE code verifier, and returns a CLI session token. +// @Tags Auth +// @Accept json +// @Produce json +// @Param request body CLILoginCompleteRequest true "CLI login payload" +// @Success 200 {object} CLILoginResponse +// @Failure 400 {object} ErrorResponse +// @Failure 500 {object} ErrorResponse +// @Router /auth/cli/complete [post] +func (h *AuthHandler) CLILoginComplete(c *gin.Context) { + var req CLILoginCompleteRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) return } - c.Redirect(http.StatusTemporaryRedirect, WebRoute("cli/auth/success")) -} - -func (h *AuthHandler) cliHandleEnrollmentRequired(c *gin.Context, state, _ string) { - if err := h.database.FailCLILoginState(state, cliEnrollmentErrorMessage); err != nil { - log.Printf("cli login fail (enrollment required): state=%s err=%v", state, err) - } - - enrollParams := url.Values{} - enrollParams.Set("enrollment", "required") - enrollParams.Set("channel", "cli") - enrollParams.Set("state", state) - c.Redirect( - http.StatusTemporaryRedirect, - fmt.Sprintf("%s?%s", WebRoute("account/security"), enrollParams.Encode()), - ) -} - -func (h *AuthHandler) cliExchangeOAuthCode( - c *gin.Context, - record *db.CLILoginState, - state string, - useJSON bool, -) (string, bool) { - if !record.Code.Valid || !record.CodeVerifier.Valid { - if useJSON { - c.JSON(http.StatusBadRequest, gin.H{"error": "session_incomplete"}) - } else { - cliRedirectWithError(c, "session_incomplete") - } - return "", false + if h.database == nil || h.sessions == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "service_unavailable"}) + return } - loginResp, err := h.oauth.CompleteLogin(record.Code.String, state, record.CodeVerifier.String) + // Consuming deletes the login, so each login code can be tried exactly once. + record, err := h.database.ConsumeCLILoginCode(hashCLISecret(strings.TrimSpace(req.LoginCode))) if err != nil { - if useJSON { - c.JSON(http.StatusBadRequest, gin.H{"error": "exchange_failed"}) - } else { - cliRedirectWithError(c, "exchange_failed") - } - return "", false - } - - if err := h.database.SetCLILoginProfile(state, loginResp.Email, loginResp.Name); err != nil { - if useJSON { - c.JSON(http.StatusInternalServerError, gin.H{"error": "persist_failure"}) - } else { - cliRedirectWithError(c, "persist_failure") - } - return "", false + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to load login"}) + return } - - return strings.TrimSpace(loginResp.Email), true -} - -func (h *AuthHandler) cliExchangeIfNeeded( - c *gin.Context, - record *db.CLILoginState, - state string, - useJSON bool, -) (*db.CLILoginState, bool) { - // If login email is already set, OAuth exchange has completed - // LoginToken and LoginExpiresAt are only set for non-MFA flows - if record.LoginEmail.Valid { - return record, true + if record == nil || !record.MFAVerifiedAt.Valid || !record.LoginEmail.Valid || + !pkceS256Matches(strings.TrimSpace(req.CodeVerifier), record.CLICodeChallenge) { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid or expired login code"}) + return } - _, ok := h.cliExchangeOAuthCode(c, record, state, useJSON) - if !ok { - return nil, false + userRecord, err := h.database.GetUser(record.LoginEmail.String) + if err != nil || userRecord == nil { + h.notifyUnauthorizedCLILogin(c, record) + c.JSON(http.StatusUnauthorized, gin.H{"error": "user not authorized"}) + return } - // Reload record after exchange - refreshed, err := h.database.GetCLILoginState(state) - if err != nil || refreshed == nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "reload_failure"}) - return nil, false + sessionToken, session, err := h.createCLISession(c, userRecord, req) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create session"}) + return } + h.cliStampMFAVerification(session, record.MFAVerifiedAt.Time) - return refreshed, true + h.notifyCLILoginComplete(c, userRecord, session) + c.JSON(http.StatusOK, h.buildCLILoginResponse(userRecord, record, sessionToken, session)) } func (h *AuthHandler) cliGetUserRecord(c *gin.Context, loginEmail string) (*db.User, bool) { @@ -143,127 +93,55 @@ func (h *AuthHandler) cliGetUserRecord(c *gin.Context, loginEmail string) (*db.U return userRecord, true } -func (h *AuthHandler) cliStampMFAVerification(sessionID int64, verifiedAt time.Time) (time.Time, bool) { - if h.sessions == nil { - return time.Time{}, false - } - - if err := h.sessions.UpdateSessionMFAVerified(sessionID, verifiedAt, nil); err != nil { +// cliStampMFAVerification carries the browser-side MFA verification over to the new CLI session. +func (h *AuthHandler) cliStampMFAVerification(session *db.UserSession, verifiedAt time.Time) { + if err := h.sessions.UpdateSessionMFAVerified(session.ID, verifiedAt, nil); err != nil { log.Printf("failed to update session mfa timestamp: %v", err) - return time.Time{}, false - } - - return verifiedAt, true -} - -func (_ *AuthHandler) bindCLILoginComplete(c *gin.Context) (CLILoginCompleteRequest, bool) { - var req CLILoginCompleteRequest - if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) - return CLILoginCompleteRequest{}, false - } - return req, true -} - -func (h *AuthHandler) loadCLILoginState(c *gin.Context, state string) (*db.CLILoginState, bool) { - record, err := h.database.GetCLILoginState(state) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to load login state"}) - return nil, false - } - if record == nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid or expired state"}) - return nil, false - } - return record, true -} - -func (_ *AuthHandler) handleCLIRecordErrors(c *gin.Context, record *db.CLILoginState) bool { - if record.LoginError.Valid { - reason := strings.TrimSpace(record.LoginError.String) - if reason == "" { - reason = "login_failed" - } - c.JSON(http.StatusBadRequest, gin.H{"error": reason}) - return true - } - return false -} - -func (_ *AuthHandler) respondIfPending(c *gin.Context, record *db.CLILoginState) bool { - if !record.LoginEmail.Valid || !record.MFAVerifiedAt.Valid { - c.JSON(http.StatusAccepted, gin.H{"status": "pending"}) - return true - } - return false -} - -func (h *AuthHandler) loadUserForCLI(c *gin.Context, record *db.CLILoginState) (*db.User, bool) { - userRecord, err := h.database.GetUser(record.LoginEmail.String) - if err != nil || userRecord == nil { - h.notifyUnauthorizedCLILogin(c, record) - c.JSON(http.StatusUnauthorized, gin.H{"error": "user not authorized"}) - return nil, false - } - if h.sessions == nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "session manager not available"}) - return nil, false - } - return userRecord, true -} - -func (h *AuthHandler) createCLISessionOrRespond( - c *gin.Context, - userRecord *db.User, - req CLILoginCompleteRequest, -) (string, *db.UserSession, bool) { - sessionToken, session, err := h.createCLISession(c, userRecord, req) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create session"}) - return "", nil, false - } - return sessionToken, session, true -} - -func (h *AuthHandler) applyMFATimestamps(record *db.CLILoginState, session *db.UserSession) { - if record.MFAVerifiedAt.Valid { - if mfaTime, ok := h.cliStampMFAVerification(session.ID, record.MFAVerifiedAt.Time); ok { - session.MFAVerifiedAt = &mfaTime - session.RecentStepUpAt = &mfaTime - } + return } + session.MFAVerifiedAt = &verifiedAt + session.RecentStepUpAt = &verifiedAt } -func (h *AuthHandler) notifyCLILoginComplete(c *gin.Context, userRecord *db.User) { - if h.securityNotifier != nil { - h.securityNotifier.LoginAttempt( - userRecord.Email, - userRecord.Name, - "cli", - "complete", - c.ClientIP(), - c.GetHeader("User-Agent"), - true, - ) +func (h *AuthHandler) notifyCLILoginComplete(c *gin.Context, userRecord *db.User, session *db.UserSession) { + if h.securityNotifier == nil { + return } + h.securityNotifier.LoginAttempt( + userRecord.Email, + userRecord.Name, + "cli", + "complete", + c.ClientIP(), + c.GetHeader("User-Agent"), + true, + ) + h.securityNotifier.NewCLISession( + userRecord.Email, + userRecord.Name, + session.DeviceName, + c.ClientIP(), + c.GetHeader("User-Agent"), + ) } func (h *AuthHandler) notifyUnauthorizedCLILogin(c *gin.Context, record *db.CLILoginState) { + if h.securityNotifier == nil { + return + } userName := "" if record.LoginName.Valid { userName = record.LoginName.String } - if h.securityNotifier != nil { - h.securityNotifier.LoginAttempt( - record.LoginEmail.String, - userName, - "cli", - "user_not_authorized", - c.ClientIP(), - c.GetHeader("User-Agent"), - false, - ) - } + h.securityNotifier.LoginAttempt( + record.LoginEmail.String, + userName, + "cli", + "user_not_authorized", + c.ClientIP(), + c.GetHeader("User-Agent"), + false, + ) } func (h *AuthHandler) createCLISession( @@ -287,12 +165,12 @@ func (h *AuthHandler) createCLISession( return h.sessions.CreateSession(user, auth.SessionMetadata{ Channel: "cli", DeviceID: deviceID, - DeviceName: strings.TrimSpace(req.DeviceName), + DeviceName: sanitizeDeviceName(req.DeviceName), DeviceMetadata: deviceMeta, IPAddress: c.ClientIP(), UserAgent: c.GetHeader("User-Agent"), Extra: map[string]interface{}{"login_flow": "cli"}, - TTLOverride: 90 * 24 * time.Hour, + TTLOverride: cliSessionTTL, }) } diff --git a/internal/gateway/handlers/auth_cli_loopback.go b/internal/gateway/handlers/auth_cli_loopback.go new file mode 100644 index 00000000..2909abe0 --- /dev/null +++ b/internal/gateway/handlers/auth_cli_loopback.go @@ -0,0 +1,257 @@ +package handlers + +import ( + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "encoding/hex" + "errors" + "net/http" + "net/url" + "regexp" + "strconv" + "strings" + + "github.com/gin-gonic/gin" + + "github.com/DocSpring/rack-gateway/internal/gateway/db" +) + +// The CLI login is an RFC 8252 loopback flow: +// +// 1. The CLI listens on 127.0.0.1, then calls /auth/cli/start with an S256 code challenge, its own +// state and its loopback redirect URI. It receives only the identity provider URL. +// 2. The browser that completes the identity provider login is bound to the login with an HttpOnly +// cookie. Every later browser step (MFA form, MFA submit, return) requires that cookie. +// 3. Once MFA is satisfied, /auth/cli/return redirects the browser to the CLI's loopback listener with +// a single-use login code and the CLI's state. +// 4. The CLI redeems the login code together with its code verifier at /auth/cli/complete. +// +// A login link sent to someone else delivers its login code to their own machine, never to the +// person who started it, and the gateway's own OAuth state is never a credential. +const ( + cliLoginCookie = "rgw_cli_login" + cliLoginCookiePath = "/api/v1/auth/cli" + cliLoginCookieMaxAge = 10 * 60 + cliChallengeMethod = "S256" + cliMinLoopbackPort = 1024 + cliMaxLoopbackPort = 65535 + cliMaxDeviceNameLen = 64 +) + +var ( + cliChallengePattern = regexp.MustCompile(`^[A-Za-z0-9_-]{43}$`) + cliStatePattern = regexp.MustCompile(`^[A-Za-z0-9_-]{16,128}$`) + cliDeviceUnsafeChars = regexp.MustCompile(`[^A-Za-z0-9._ -]`) + errInvalidRedirectURI = errors.New("redirect_uri must be http://127.0.0.1:/callback") +) + +// validateCLIStart checks the CLI's loopback login parameters and returns a display-safe device name. +func validateCLIStart(req CLILoginStartRequest) (string, error) { + if req.CodeChallengeMethod != cliChallengeMethod { + return "", errors.New("code_challenge_method must be S256") + } + if !cliChallengePattern.MatchString(req.CodeChallenge) { + return "", errors.New("code_challenge must be a base64url SHA-256 digest") + } + if !cliStatePattern.MatchString(req.State) { + return "", errors.New("state must be 16-128 base64url characters") + } + if err := validateLoopbackRedirectURI(req.RedirectURI); err != nil { + return "", err + } + return sanitizeDeviceName(req.DeviceName), nil +} + +// validateLoopbackRedirectURI accepts only http://127.0.0.1:/callback or http://[::1]:/callback. +func validateLoopbackRedirectURI(raw string) error { + u, err := url.Parse(raw) + if err != nil || u.String() != raw { + return errInvalidRedirectURI + } + if u.Scheme != "http" || u.User != nil || u.RawQuery != "" || u.Fragment != "" || u.Path != "/callback" { + return errInvalidRedirectURI + } + if host := u.Hostname(); host != "127.0.0.1" && host != "::1" { + return errInvalidRedirectURI + } + port, err := strconv.Atoi(u.Port()) + if err != nil || port < cliMinLoopbackPort || port > cliMaxLoopbackPort { + return errInvalidRedirectURI + } + return nil +} + +func sanitizeDeviceName(name string) string { + cleaned := strings.TrimSpace(cliDeviceUnsafeChars.ReplaceAllString(name, "")) + if len(cleaned) > cliMaxDeviceNameLen { + cleaned = cleaned[:cliMaxDeviceNameLen] + } + return cleaned +} + +// pkceS256Matches reports whether S256(verifier) equals the stored challenge, in constant time. +func pkceS256Matches(verifier, challenge string) bool { + if len(verifier) < 43 || len(verifier) > 128 { + return false + } + sum := sha256.Sum256([]byte(verifier)) + computed := base64.RawURLEncoding.EncodeToString(sum[:]) + return subtle.ConstantTimeCompare([]byte(computed), []byte(challenge)) == 1 +} + +// newCLISecret returns a random 256-bit base64url value. +func newCLISecret() (string, error) { + buf := make([]byte, 32) + if _, err := rand.Read(buf); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(buf), nil +} + +// hashCLISecret returns the hex SHA-256 of a secret; only hashes are stored. +func hashCLISecret(secret string) string { + sum := sha256.Sum256([]byte(secret)) + return hex.EncodeToString(sum[:]) +} + +func (h *AuthHandler) setCLILoginCookie(c *gin.Context, value string, maxAge int) { + c.SetSameSite(http.SameSiteLaxMode) + c.SetCookie(cliLoginCookie, value, maxAge, cliLoginCookiePath, "", h.cookieSecure(), true) + c.SetSameSite(http.SameSiteDefaultMode) +} + +// browserBoundTo reports whether this request comes from the browser bound to the login. +func browserBoundTo(c *gin.Context, record *db.CLILoginState) bool { + cookie, err := c.Cookie(cliLoginCookie) + if err != nil || strings.TrimSpace(cookie) == "" || !record.BrowserBindingHash.Valid { + return false + } + computed := hashCLISecret(strings.TrimSpace(cookie)) + return subtle.ConstantTimeCompare([]byte(computed), []byte(record.BrowserBindingHash.String)) == 1 +} + +// loadBoundCLILogin loads a live CLI login for the bound browser. It returns an error code +// suitable for the MFA challenge page when the login is missing, expired or not bound to this browser. +func (h *AuthHandler) loadBoundCLILogin(c *gin.Context, state string) (*db.CLILoginState, string) { + if state == "" { + return nil, "missing_state" + } + if h.database == nil { + return nil, "service_unavailable" + } + record, err := h.database.GetCLILoginState(state) + if err != nil { + return nil, "load_failure" + } + if record == nil { + return nil, "expired" + } + if !browserBoundTo(c, record) { + return nil, "browser_mismatch" + } + return record, "" +} + +// liveBoundCLILogin loads the CLI login bound to this browser for a browser step. When the login is +// missing, unbound or already failed, it ends the request (redirecting with the error) and returns false. +func (h *AuthHandler) liveBoundCLILogin(c *gin.Context, state string) (*db.CLILoginState, bool) { + record, errCode := h.loadBoundCLILogin(c, state) + if errCode != "" { + cliRedirectWithError(c, errCode) + return nil, false + } + if record.LoginError.Valid { + h.failCLILogin(c, record, strings.TrimSpace(record.LoginError.String)) + return nil, false + } + return record, true +} + +func cliReturnRoute(state string) string { + return APIRoute("auth/cli/return") + "?state=" + url.QueryEscape(state) +} + +func cliMFARoute(state string) string { + return APIRoute("auth/cli/mfa") + "?state=" + url.QueryEscape(state) +} + +// redirectToCLI sends the browser to the CLI's loopback listener with the CLI's state. +func redirectToCLI(c *gin.Context, record *db.CLILoginState, params url.Values) { + params.Set("state", record.CLIState) + c.Redirect(http.StatusFound, record.CLIRedirectURI+"?"+params.Encode()) +} + +// failCLILogin ends the login and tells the waiting CLI why. +func (h *AuthHandler) failCLILogin(c *gin.Context, record *db.CLILoginState, errorCode string) { + if h.database != nil { + _ = h.database.FailCLILoginState(record.State, errorCode) + } + h.setCLILoginCookie(c, "", -1) + redirectToCLI(c, record, url.Values{"error": {errorCode}}) +} + +// CLILoginReturn godoc +// @Summary Return the browser to the CLI +// @Description Issues a single-use login code and redirects the bound browser to the CLI's loopback listener. +// @Tags Auth +// @Param state query string true "Login state" +// @Success 302 {string} string "Found" +// @Router /auth/cli/return [get] +func (h *AuthHandler) CLILoginReturn(c *gin.Context) { + state := strings.TrimSpace(c.Query("state")) + record, ok := h.liveBoundCLILogin(c, state) + if !ok { + return + } + if !record.LoginEmail.Valid || !h.cliMFASatisfied(record) { + c.Redirect(http.StatusFound, cliMFARoute(state)) + return + } + + loginCode, err := newCLISecret() + if err != nil { + cliRedirectWithError(c, "persist_failure") + return + } + stored, err := h.database.SetCLILoginCode(state, hashCLISecret(loginCode)) + if err != nil || !stored { + cliRedirectWithError(c, "expired") + return + } + h.setCLILoginCookie(c, "", -1) + redirectToCLI(c, record, url.Values{"code": {loginCode}}) +} + +// cliMFASatisfied reports whether the login's MFA requirement is met. A user who had no factor when +// the login reached MFA satisfies it by enrolling one during this login: confirming a new factor +// requires a valid code from it. +func (h *AuthHandler) cliMFASatisfied(record *db.CLILoginState) bool { + if record.MFAVerifiedAt.Valid { + return true + } + if !record.EnrollmentRequired { + return false + } + user, err := h.database.GetUser(record.LoginEmail.String) + if err != nil || user == nil || !h.enrolledDuringLogin(user, record) { + return false + } + return h.database.MarkCLILoginVerified(record.State, nil) == nil +} + +// enrolledDuringLogin reports whether every confirmed factor the user has was confirmed after the +// login started (and there is at least one), i.e. the user had none before this login. +func (h *AuthHandler) enrolledDuringLogin(user *db.User, record *db.CLILoginState) bool { + methods, err := h.database.ListMFAMethods(user.ID) + if err != nil || len(methods) == 0 { + return false + } + for _, method := range methods { + if method.ConfirmedAt == nil || method.ConfirmedAt.Before(record.CreatedAt) { + return false + } + } + return true +} diff --git a/internal/gateway/handlers/auth_cli_loopback_test.go b/internal/gateway/handlers/auth_cli_loopback_test.go new file mode 100644 index 00000000..3584016a --- /dev/null +++ b/internal/gateway/handlers/auth_cli_loopback_test.go @@ -0,0 +1,334 @@ +package handlers + +import ( + "bytes" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/auth/mfa" + "github.com/DocSpring/rack-gateway/internal/gateway/config" + "github.com/DocSpring/rack-gateway/internal/gateway/db" + "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" +) + +const ( + loopbackTestRedirect = "http://127.0.0.1:54321/callback" + loopbackTestVerifier = "test-verifier-0123456789-abcdefghijklmnopqrstuvwxyz-ABCDEFGHIJK" +) + +// loopbackOAuth is a fake identity provider that issues a fresh gateway state per login. +type loopbackOAuth struct { + count int + lastState string + email string +} + +func (o *loopbackOAuth) StartLogin() (*auth.LoginStartResponse, error) { + o.count++ + o.lastState = fmt.Sprintf("gateway-state-%d", o.count) + return &auth.LoginStartResponse{ + AuthURL: "https://accounts.google.com/o/oauth2/auth", State: o.lastState, + CodeVerifier: "google-verifier", + }, nil +} + +func (_ *loopbackOAuth) StartWebLogin() (string, string) { return "", "" } + +func (o *loopbackOAuth) CompleteLogin(_, _, _ string) (*auth.LoginResponse, error) { + return &auth.LoginResponse{Email: o.email, Name: "Loopback User"}, nil +} + +type loopbackEnv struct { + router *gin.Engine + database *db.Database + oauth *loopbackOAuth + sessions *auth.SessionManager +} + +func newLoopbackEnv(t *testing.T, requireMFA bool) *loopbackEnv { + t.Helper() + gin.SetMode(gin.TestMode) + database := dbtest.NewDatabase(t) + _, err := database.CreateUser("user@example.com", "Loopback User", []string{"admin"}) + require.NoError(t, err) + + sessions := auth.NewSessionManager(database, "test-secret", &auth.StaticTTLProvider{TTL: time.Hour}) + mfaService, err := mfa.NewService(database, "Rack Gateway", 30*time.Minute, 10*time.Minute, + []byte("pepper"), "", "", "", "", nil) + require.NoError(t, err) + oauth := &loopbackOAuth{email: "user@example.com"} + handler := NewAuthHandler(oauth, database, &config.Config{DevMode: true}, sessions, mfaService, + &db.MFASettings{RequireAllUsers: requireMFA}, nil, audit.NewLogger(database)) + + router := gin.New() + router.POST("/api/v1/auth/cli/start", handler.CLILoginStart) + router.GET("/api/v1/auth/cli/callback", handler.CLILoginCallback) + router.GET("/api/v1/auth/cli/mfa", handler.CLILoginMFAForm) + router.POST("/api/v1/auth/cli/mfa", handler.CLILoginMFASubmit) + router.GET("/api/v1/auth/cli/return", handler.CLILoginReturn) + router.POST("/api/v1/auth/cli/complete", handler.CLILoginComplete) + return &loopbackEnv{router: router, database: database, oauth: oauth, sessions: sessions} +} + +func s256(verifier string) string { + sum := sha256.Sum256([]byte(verifier)) + return base64.RawURLEncoding.EncodeToString(sum[:]) +} + +func (e *loopbackEnv) send( + t *testing.T, + method, target string, + body interface{}, + cookies ...*http.Cookie, +) *http.Response { + t.Helper() + var buf bytes.Buffer + if body != nil { + require.NoError(t, json.NewEncoder(&buf).Encode(body)) + } + req := httptest.NewRequest(method, target, &buf) + req.Header.Set("Content-Type", "application/json") + for _, cookie := range cookies { + req.AddCookie(cookie) + } + w := httptest.NewRecorder() + e.router.ServeHTTP(w, req) + return w.Result() +} + +func validStartRequest() CLILoginStartRequest { + return CLILoginStartRequest{ + CodeChallenge: s256(loopbackTestVerifier), + CodeChallengeMethod: "S256", + RedirectURI: loopbackTestRedirect, + State: "cli-state-abcdefghijklmnop", + DeviceName: "laptop.local", + } +} + +// startAndBind starts a login and completes the identity provider callback in a browser. +// It returns the gateway state and the browser's binding cookie. +func (e *loopbackEnv) startAndBind(t *testing.T) (string, *http.Cookie) { + t.Helper() + res := e.send(t, http.MethodPost, "/api/v1/auth/cli/start", validStartRequest()) + require.Equal(t, http.StatusOK, res.StatusCode) + var started map[string]interface{} + require.NoError(t, json.NewDecoder(res.Body).Decode(&started)) + require.Equal(t, []string{"auth_url"}, keysOf(started), "start must not return the state or any verifier") + + state := e.oauth.lastState + res = e.send(t, http.MethodGet, "/api/v1/auth/cli/callback?code=google-code&state="+state, nil) + require.Equal(t, http.StatusTemporaryRedirect, res.StatusCode) + binding := findCookie(res, cliLoginCookie) + require.NotNil(t, binding) + require.True(t, binding.HttpOnly) + return state, binding +} + +func keysOf(m map[string]interface{}) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + return keys +} + +// loginCodeFromBrowser follows the bound browser through MFA (none required) to the loopback redirect. +func (e *loopbackEnv) loginCodeFromBrowser(t *testing.T, state string, binding *http.Cookie) string { + t.Helper() + res := e.send(t, http.MethodGet, "/api/v1/auth/cli/mfa?state="+state, nil, binding) + require.Equal(t, http.StatusTemporaryRedirect, res.StatusCode) + require.Equal(t, cliReturnRoute(state), res.Header.Get("Location")) + + res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding) + require.Equal(t, http.StatusFound, res.StatusCode) + return loopbackCode(t, res) +} + +func loopbackCode(t *testing.T, res *http.Response) string { + t.Helper() + location, err := url.Parse(res.Header.Get("Location")) + require.NoError(t, err) + require.Equal(t, loopbackTestRedirect, location.Scheme+"://"+location.Host+location.Path) + require.Equal(t, validStartRequest().State, location.Query().Get("state")) + code := location.Query().Get("code") + require.NotEmpty(t, code) + return code +} + +func (e *loopbackEnv) complete(t *testing.T, code, verifier string) *http.Response { + t.Helper() + return e.send(t, http.MethodPost, "/api/v1/auth/cli/complete", CLILoginCompleteRequest{ + LoginCode: code, CodeVerifier: verifier, DeviceName: "laptop.local", + }) +} + +func TestCLILoginLoopbackFlowIssuesSingleUseCode(t *testing.T) { + e := newLoopbackEnv(t, false) + state, binding := e.startAndBind(t) + code := e.loginCodeFromBrowser(t, state, binding) + + res := e.complete(t, code, loopbackTestVerifier) + require.Equal(t, http.StatusOK, res.StatusCode) + var login CLILoginResponse + require.NoError(t, json.NewDecoder(res.Body).Decode(&login)) + require.NotEmpty(t, login.Token) + require.Equal(t, "user@example.com", login.Email) + require.True(t, login.MFAVerified) + + res = e.complete(t, code, loopbackTestVerifier) + require.Equal(t, http.StatusBadRequest, res.StatusCode, "login code must be single use") +} + +func TestCLILoginCompleteRejectsWrongVerifierAndBurnsCode(t *testing.T) { + e := newLoopbackEnv(t, false) + state, binding := e.startAndBind(t) + code := e.loginCodeFromBrowser(t, state, binding) + + res := e.complete(t, code, "attacker-verifier-0123456789-abcdefghijklmnopqrstuvwxyz-ABCDEF") + require.Equal(t, http.StatusBadRequest, res.StatusCode) + + res = e.complete(t, code, loopbackTestVerifier) + require.Equal(t, http.StatusBadRequest, res.StatusCode, "a failed redemption must consume the code") +} + +func TestCLILoginBrowserStepsRequireBindingCookie(t *testing.T) { + e := newLoopbackEnv(t, false) + state, binding := e.startAndBind(t) + + res := e.send(t, http.MethodGet, "/api/v1/auth/cli/mfa?state="+state, nil) + require.Equal(t, http.StatusTemporaryRedirect, res.StatusCode) + require.Contains(t, res.Header.Get("Location"), "error=browser_mismatch") + + res = e.send(t, http.MethodPost, "/api/v1/auth/cli/mfa", map[string]string{"state": state, "code": "123456"}) + require.Equal(t, http.StatusBadRequest, res.StatusCode) + var body map[string]string + require.NoError(t, json.NewDecoder(res.Body).Decode(&body)) + require.Equal(t, "browser_mismatch", body["error"]) + + wrong := &http.Cookie{Name: cliLoginCookie, Value: "not-the-binding"} + res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, wrong) + require.Contains(t, res.Header.Get("Location"), "error=browser_mismatch") + + // A replayed identity provider callback cannot bind a second browser. + res = e.send(t, http.MethodGet, "/api/v1/auth/cli/callback?code=google-code&state="+state, nil) + require.Contains(t, res.Header.Get("Location"), "error=expired") + require.Nil(t, findCookie(res, cliLoginCookie)) + + // The original browser still works. + require.NotEmpty(t, e.loginCodeFromBrowser(t, state, binding)) +} + +func TestCLILoginExpiresAfterTenMinutes(t *testing.T) { + e := newLoopbackEnv(t, false) + state, binding := e.startAndBind(t) + code := e.loginCodeFromBrowser(t, state, binding) + + _, err := e.database.DB().Exec( + `UPDATE cli_login_states SET created_at = NOW() - INTERVAL '11 minutes' WHERE state = $1`, state) + require.NoError(t, err) + res := e.complete(t, code, loopbackTestVerifier) + require.Equal(t, http.StatusBadRequest, res.StatusCode) +} + +func TestCLILoginCodeExpires(t *testing.T) { + e := newLoopbackEnv(t, false) + state, binding := e.startAndBind(t) + code := e.loginCodeFromBrowser(t, state, binding) + + _, err := e.database.DB().Exec( + `UPDATE cli_login_states SET login_code_expires_at = NOW() - INTERVAL '1 second' WHERE state = $1`, state) + require.NoError(t, err) + res := e.complete(t, code, loopbackTestVerifier) + require.Equal(t, http.StatusBadRequest, res.StatusCode) +} + +func TestCLILoginUnauthorizedUserIsReturnedToCLIWithError(t *testing.T) { + e := newLoopbackEnv(t, false) + e.oauth.email = "stranger@example.com" + state, binding := e.startAndBind(t) + + res := e.send(t, http.MethodGet, "/api/v1/auth/cli/mfa?state="+state, nil, binding) + require.Equal(t, http.StatusFound, res.StatusCode) + location, err := url.Parse(res.Header.Get("Location")) + require.NoError(t, err) + require.Equal(t, "127.0.0.1:54321", location.Host) + require.Equal(t, "unauthorized", location.Query().Get("error")) + require.Empty(t, location.Query().Get("code")) +} + +func TestCLILoginEnrollmentDuringLoginCompletesCLILogin(t *testing.T) { + e := newLoopbackEnv(t, true) + state, binding := e.startAndBind(t) + + res := e.send(t, http.MethodGet, "/api/v1/auth/cli/mfa?state="+state, nil, binding) + require.Equal(t, http.StatusTemporaryRedirect, res.StatusCode) + require.Contains(t, res.Header.Get("Location"), "enrollment=required") + sessionCookie := findCookie(res, "session_token") + require.NotNil(t, sessionCookie) + + // Before enrolling, the return step sends the browser back to MFA, even though the web session + // of a not-yet-enrolled user counts as verified. + res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding, sessionCookie) + require.Equal(t, cliMFARoute(state), res.Header.Get("Location")) + + // The user confirms a first factor during this login. + user, err := e.database.GetUser("user@example.com") + require.NoError(t, err) + method, err := e.database.CreateMFAMethod(user.ID, "totp", "Authenticator App", "SECRET", nil, nil, nil, nil) + require.NoError(t, err) + require.NoError(t, e.database.ConfirmMFAMethod(method.ID, time.Now())) + + res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding, sessionCookie) + require.Equal(t, http.StatusFound, res.StatusCode) + code := loopbackCode(t, res) + require.Equal(t, http.StatusOK, e.complete(t, code, loopbackTestVerifier).StatusCode) +} + +func TestCLILoginStartValidation(t *testing.T) { + e := newLoopbackEnv(t, false) + cases := map[string]func(*CLILoginStartRequest){ + "plain challenge method": func(r *CLILoginStartRequest) { r.CodeChallengeMethod = "plain" }, + "short challenge": func(r *CLILoginStartRequest) { r.CodeChallenge = "abc" }, + "short state": func(r *CLILoginStartRequest) { r.State = "short" }, + "remote host": func(r *CLILoginStartRequest) { r.RedirectURI = "http://evil.example:5000/callback" }, + "localhost name": func(r *CLILoginStartRequest) { r.RedirectURI = "http://localhost:5000/callback" }, + "https scheme": func(r *CLILoginStartRequest) { + r.RedirectURI = "https://127.0.0.1:5000/callback" + }, + "other path": func(r *CLILoginStartRequest) { r.RedirectURI = "http://127.0.0.1:5000/other" }, + "query string": func(r *CLILoginStartRequest) { r.RedirectURI = "http://127.0.0.1:5000/callback?x=1" }, + "userinfo": func(r *CLILoginStartRequest) { r.RedirectURI = "http://a@127.0.0.1:5000/callback" }, + "privileged port": func(r *CLILoginStartRequest) { r.RedirectURI = "http://127.0.0.1:80/callback" }, + "no port": func(r *CLILoginStartRequest) { r.RedirectURI = "http://127.0.0.1/callback" }, + } + for name, mutate := range cases { + req := validStartRequest() + mutate(&req) + res := e.send(t, http.MethodPost, "/api/v1/auth/cli/start", req) + require.Equalf(t, http.StatusBadRequest, res.StatusCode, name) + } + + ipv6 := validStartRequest() + ipv6.RedirectURI = "http://[::1]:5000/callback" + require.Equal(t, http.StatusOK, e.send(t, http.MethodPost, "/api/v1/auth/cli/start", ipv6).StatusCode) +} + +func TestSanitizeDeviceName(t *testing.T) { + require.Equal(t, "laptop.local", sanitizeDeviceName(" laptop.local\n")) + require.Equal(t, "Approve bITb login", sanitizeDeviceName("Approve IT login")) + require.Len(t, sanitizeDeviceName(strings.Repeat("a", 200)), cliMaxDeviceNameLen) +} diff --git a/internal/gateway/handlers/auth_helpers.go b/internal/gateway/handlers/auth_helpers.go index 476da4ee..f48592bc 100644 --- a/internal/gateway/handlers/auth_helpers.go +++ b/internal/gateway/handlers/auth_helpers.go @@ -20,11 +20,10 @@ import ( ) const ( - webOAuthStateCookie = "rgw_oauth_state" - webOAuthReturnToCookie = "rgw_return_to" - webOAuthStateTTL = 5 * time.Minute - trustedDeviceCookie = "rgw_trusted_device" - cliEnrollmentErrorMessage = "You must set up multi-factor authentication before you can continue using the CLI." + webOAuthStateCookie = "rgw_oauth_state" + webOAuthReturnToCookie = "rgw_return_to" + webOAuthStateTTL = 5 * time.Minute + trustedDeviceCookie = "rgw_trusted_device" ) func extractSessionToken(c *gin.Context) string { diff --git a/internal/gateway/handlers/dto.go b/internal/gateway/handlers/dto.go index 994b0dfd..ebfa209a 100644 --- a/internal/gateway/handlers/dto.go +++ b/internal/gateway/handlers/dto.go @@ -209,9 +209,24 @@ type UpdateEnvValuesResponse struct { ReleaseID string `json:"release_id,omitempty"` } +// CLILoginStartRequest starts a loopback CLI login. The CLI keeps the PKCE code verifier and +// receives a single-use login code on its loopback redirect URI. +type CLILoginStartRequest struct { + CodeChallenge string `json:"code_challenge" binding:"required"` + CodeChallengeMethod string `json:"code_challenge_method" binding:"required"` + RedirectURI string `json:"redirect_uri" binding:"required"` + State string `json:"state" binding:"required"` + DeviceName string `json:"device_name"` +} + +// CLILoginStartResponse returns the identity provider URL for the CLI to open in the browser. +type CLILoginStartResponse struct { + AuthURL string `json:"auth_url" validate:"required"` +} + // CLILoginCompleteRequest represents the payload used to finish the CLI OAuth flow. type CLILoginCompleteRequest struct { - State string `json:"state" binding:"required"` + LoginCode string `json:"login_code" binding:"required"` CodeVerifier string `json:"code_verifier" binding:"required"` DeviceID string `json:"device_id"` DeviceName string `json:"device_name"` diff --git a/internal/gateway/jobs/client.go b/internal/gateway/jobs/client.go index 336cc51e..de979d87 100644 --- a/internal/gateway/jobs/client.go +++ b/internal/gateway/jobs/client.go @@ -55,6 +55,7 @@ func NewClient(pool *pgxpool.Pool, deps *Dependencies, auditAnchorConfig *AuditA // Email workers - security notifications river.AddWorker(workers, jobemail.NewFailedMFAWorker(deps.EmailSender)) river.AddWorker(workers, jobemail.NewFailedLoginWorker(deps.EmailSender)) + river.AddWorker(workers, jobemail.NewNewCLISessionWorker(deps.EmailSender)) river.AddWorker(workers, jobemail.NewRateLimitUserWorker(deps.EmailSender)) river.AddWorker(workers, jobemail.NewRateLimitAdminWorker(deps.EmailSender)) river.AddWorker(workers, jobemail.NewSuspiciousActivityUserWorker(deps.EmailSender)) diff --git a/internal/gateway/jobs/email/cli_login.go b/internal/gateway/jobs/email/cli_login.go new file mode 100644 index 00000000..ba592deb --- /dev/null +++ b/internal/gateway/jobs/email/cli_login.go @@ -0,0 +1,81 @@ +package email + +import ( + "context" + "fmt" + "html" + + "github.com/riverqueue/river" + + "github.com/DocSpring/rack-gateway/internal/gateway/email" +) + +// NewCLISessionArgs contains parameters for the new CLI session email notification. +type NewCLISessionArgs struct { + UserEmail string `json:"user_email"` + UserName string `json:"user_name"` + DeviceName string `json:"device_name"` + IPAddress string `json:"ip_address"` + UserAgent string `json:"user_agent"` +} + +// Kind returns the unique identifier for this job type +func (NewCLISessionArgs) Kind() string { return "email:security:new_cli_session" } + +// NewCLISessionWorker emails a user whenever a CLI session is created for their account. +type NewCLISessionWorker struct { + river.WorkerDefaults[NewCLISessionArgs] + emailSender email.Sender +} + +// NewNewCLISessionWorker creates a new CLI session email worker +func NewNewCLISessionWorker(emailSender email.Sender) *NewCLISessionWorker { + return &NewCLISessionWorker{emailSender: emailSender} +} + +// Work sends the new CLI session email +func (w *NewCLISessionWorker) Work(_ context.Context, job *river.Job[NewCLISessionArgs]) error { + args := job.Args + when := job.CreatedAt.Format("2006-01-02 15:04:05 MST") + + subject := "New CLI Login" + text := fmt.Sprintf(`Hello %s, + +A new rack-gateway CLI session was created for your account. + +Details: +- Time: %s +- Device: %s +- IP Address: %s +- User Agent: %s + +If you did not just run "rack-gateway login", revoke the session in the web UI and contact your administrator. + +This is an automated security notification from Rack Gateway.`, + args.UserName, when, args.DeviceName, args.IPAddress, args.UserAgent, + ) + + body := fmt.Sprintf(`

Hello %s,

+

A new rack-gateway CLI session was created for your account.

+

Details:

+
    +
  • Time: %s
    • +
  • Device: %s
    • +
  • IP Address: %s
    • +
  • User Agent: %s
    • +
+

If you did not just run rack-gateway login, revoke the session in the web UI and contact +your administrator.

+

This is an automated security notification from Rack Gateway.

`, + html.EscapeString(args.UserName), + html.EscapeString(when), + html.EscapeString(args.DeviceName), + html.EscapeString(args.IPAddress), + html.EscapeString(args.UserAgent), + ) + + if err := w.emailSender.Send(args.UserEmail, subject, text, body); err != nil { + return fmt.Errorf("failed to send new CLI session email: %w", err) + } + return nil +} diff --git a/internal/gateway/middleware/debug_logging.go b/internal/gateway/middleware/debug_logging.go index 2f18949b..02c21242 100644 --- a/internal/gateway/middleware/debug_logging.go +++ b/internal/gateway/middleware/debug_logging.go @@ -5,6 +5,7 @@ import ( "github.com/gin-gonic/gin" + "github.com/DocSpring/rack-gateway/internal/gateway/audit" "github.com/DocSpring/rack-gateway/internal/gateway/config" ) @@ -38,7 +39,10 @@ var staticAssetExtensions = map[string]struct{}{ // This middleware should be added early in the chain to capture all requests. func DebugLogging(_ *config.Config) gin.HandlerFunc { return func(c *gin.Context) { - path := c.Request.URL.RequestURI() + path := c.Request.URL.EscapedPath() + if c.Request.URL.RawQuery != "" { + path += "?" + audit.RedactQuery(c.Request.URL.RawQuery) + } logReqInfo, logReqHeaders, logReqBody, logRespHeaders, logRespBody := shouldLogDebugTopics(path) if !logReqInfo && !logReqHeaders && !logReqBody && !logRespHeaders && !logRespBody { diff --git a/internal/gateway/openapi/generated/swagger.json b/internal/gateway/openapi/generated/swagger.json index f05f76d5..0aff174b 100644 --- a/internal/gateway/openapi/generated/swagger.json +++ b/internal/gateway/openapi/generated/swagger.json @@ -1272,11 +1272,11 @@ }, "/auth/cli/callback": { "get": { - "description": "Stores the OAuth authorization code for the CLI to finish login.", + "description": "Stores the authorization code and binds the login to this browser.", "tags": [ "Auth" ], - "summary": "Complete CLI OAuth redirect", + "summary": "Identity provider redirect for CLI login", "parameters": [ { "type": "string", @@ -1299,19 +1299,13 @@ "schema": { "type": "string" } - }, - "400": { - "description": "Missing parameters", - "schema": { - "type": "string" - } } } } }, "/auth/cli/complete": { "post": { - "description": "Exchanges the stored authorization code and PKCE verifier for a session token.", + "description": "Redeems the single-use login code delivered to the CLI's loopback listener, proving\npossession of the PKCE code verifier, and returns a CLI session token.", "consumes": [ "application/json" ], @@ -1321,7 +1315,7 @@ "tags": [ "Auth" ], - "summary": "Finalize CLI OAuth login", + "summary": "Finalize CLI login", "parameters": [ { "description": "CLI login payload", @@ -1357,11 +1351,11 @@ }, "/auth/cli/mfa": { "get": { - "description": "Displays the MFA challenge form for CLI login.", + "description": "Completes the identity provider exchange and sends the bound browser to MFA or back to the CLI.", "tags": [ "Auth" ], - "summary": "Display MFA challenge form", + "summary": "Continue CLI login in the browser", "parameters": [ { "type": "string", @@ -1377,9 +1371,29 @@ "schema": { "type": "string" } - }, - "400": { - "description": "Missing parameters", + } + } + } + }, + "/auth/cli/return": { + "get": { + "description": "Issues a single-use login code and redirects the bound browser to the CLI's loopback listener.", + "tags": [ + "Auth" + ], + "summary": "Return the browser to the CLI", + "parameters": [ + { + "type": "string", + "description": "Login state", + "name": "state", + "in": "query", + "required": true + } + ], + "responses": { + "302": { + "description": "Found", "schema": { "type": "string" } @@ -1389,19 +1403,39 @@ }, "/auth/cli/start": { "post": { - "description": "Initiates the CLI OAuth flow and returns PKCE parameters.", + "description": "Starts a loopback CLI login and returns the identity provider URL to open in the browser.", + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], "tags": [ "Auth" ], - "summary": "Start CLI OAuth login", + "summary": "Start CLI login", + "parameters": [ + { + "description": "CLI login parameters", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/handlers.CLILoginStartRequest" + } + } + ], "responses": { "200": { "description": "OK", "schema": { - "$ref": "#/definitions/auth.LoginStartResponse" + "$ref": "#/definitions/handlers.CLILoginStartResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" } }, "500": { @@ -3878,25 +3912,6 @@ } }, "definitions": { - "auth.LoginStartResponse": { - "type": "object", - "required": [ - "auth_url", - "code_verifier", - "state" - ], - "properties": { - "auth_url": { - "type": "string" - }, - "code_verifier": { - "type": "string" - }, - "state": { - "type": "string" - } - } - }, "db.APIToken": { "type": "object", "properties": { @@ -4306,7 +4321,7 @@ "type": "object", "required": [ "code_verifier", - "state" + "login_code" ], "properties": { "client_version": { @@ -4324,7 +4339,7 @@ "device_os": { "type": "string" }, - "state": { + "login_code": { "type": "string" } } @@ -4367,6 +4382,43 @@ } } }, + "handlers.CLILoginStartRequest": { + "type": "object", + "required": [ + "code_challenge", + "code_challenge_method", + "redirect_uri", + "state" + ], + "properties": { + "code_challenge": { + "type": "string" + }, + "code_challenge_method": { + "type": "string" + }, + "device_name": { + "type": "string" + }, + "redirect_uri": { + "type": "string" + }, + "state": { + "type": "string" + } + } + }, + "handlers.CLILoginStartResponse": { + "type": "object", + "required": [ + "auth_url" + ], + "properties": { + "auth_url": { + "type": "string" + } + } + }, "handlers.ConfirmTOTPEnrollmentRequest": { "type": "object", "required": [ diff --git a/internal/gateway/routes/route_registration.go b/internal/gateway/routes/route_registration.go index dc733010..a5be545b 100644 --- a/internal/gateway/routes/route_registration.go +++ b/internal/gateway/routes/route_registration.go @@ -25,6 +25,7 @@ func registerAuthRoutes(api *gin.RouterGroup, cfg *Config, h *handlerSet) { authGroup.POST("/auth/cli/complete", h.auth.CLILoginComplete) authGroup.GET("/auth/cli/mfa", h.auth.CLILoginMFAForm) authGroup.POST("/auth/cli/mfa", h.auth.CLILoginMFASubmit) + authGroup.GET("/auth/cli/return", h.auth.CLILoginReturn) authGroup.GET("/auth/web/login", h.auth.WebLoginStart) authGroup.HEAD("/auth/web/login", h.auth.WebLoginStart) authGroup.GET("/auth/web/callback", h.auth.WebLoginCallback) diff --git a/internal/gateway/security/notifier.go b/internal/gateway/security/notifier.go index c603b0e8..2a18a6e2 100644 --- a/internal/gateway/security/notifier.go +++ b/internal/gateway/security/notifier.go @@ -150,6 +150,18 @@ func (n *Notifier) shouldSendEmail(recipient, subject, eventType string) bool { return true } +// NewCLISession emails the user when a CLI session is created for their account, so a login they +// did not start is noticed. +func (n *Notifier) NewCLISession(userEmail, userName, deviceName, ipAddress, userAgent string) { + n.enqueueSecurityNotification(userEmail, "New CLI Login", "cli_session:"+deviceName, jobemail.NewCLISessionArgs{ + UserEmail: userEmail, + UserName: userName, + DeviceName: deviceName, + IPAddress: ipAddress, + UserAgent: userAgent, + }) +} + // FailedMFAAttempt logs and notifies about failed MFA verification func (n *Notifier) FailedMFAAttempt(userEmail, userName, ipAddress, userAgent string) { // Audit log diff --git a/internal/integration/integration_test.go b/internal/integration/integration_test.go index b6d4214c..1485e15e 100644 --- a/internal/integration/integration_test.go +++ b/internal/integration/integration_test.go @@ -355,8 +355,17 @@ func testProxyWithInvalidToken(t *testing.T, s *TestServers) { } func testOAuthLoginFlow(t *testing.T, s *TestServers) { - // Test login start endpoint - resp, err := s.client.Post("http://localhost:"+gatewayPort+"/api/v1/auth/cli/start", "application/json", nil) + startURL := "http://localhost:" + gatewayPort + "/api/v1/auth/cli/start" + + // The CLI must supply its own PKCE challenge, state and loopback redirect. + resp, err := s.client.Post(startURL, "application/json", nil) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + + body := `{"code_challenge":"E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM","code_challenge_method":"S256",` + + `"redirect_uri":"http://127.0.0.1:54321/callback","state":"integration-state-0123456789"}` + resp, err = s.client.Post(startURL, "application/json", strings.NewReader(body)) require.NoError(t, err) defer resp.Body.Close() @@ -366,8 +375,8 @@ func testOAuthLoginFlow(t *testing.T, s *TestServers) { require.NoError(t, json.NewDecoder(resp.Body).Decode(&loginStart)) assert.NotEmpty(t, loginStart["auth_url"]) - assert.NotEmpty(t, loginStart["state"]) - assert.NotEmpty(t, loginStart["code_verifier"]) + assert.NotContains(t, loginStart, "state", "the gateway state must not be returned to the CLI") + assert.NotContains(t, loginStart, "code_verifier", "the verifier must never leave the CLI") } func testAdminEndpointProtection(t *testing.T, s *TestServers) { diff --git a/scripts/lib/cli-e2e/cli_helpers.sh b/scripts/lib/cli-e2e/cli_helpers.sh index 7fdec714..c05a5f68 100644 --- a/scripts/lib/cli-e2e/cli_helpers.sh +++ b/scripts/lib/cli-e2e/cli_helpers.sh @@ -33,26 +33,39 @@ login_cli_as() { local AUTH_URL STATE AUTH_URL=$(sed -n 's/^AUTH_URL=//p' "$AUTH_FILE") - STATE=$(sed -n 's/^STATE=//p' "$AUTH_FILE") + # The gateway's own OAuth state is a parameter of the identity provider URL. The browser + # (curl, holding the login's binding cookie) uses it to submit MFA for this login. + STATE=$(printf '%s' "$AUTH_URL" | sed -n 's/.*[?&]state=\([^&]*\).*/\1/p') if [[ -z "$AUTH_URL" || -z "$STATE" ]]; then echo -e "${RED}Auth URL or state not produced" >&2 kill $CLI_PID || true exit 1 fi + # The browser (curl) follows the redirects; without MFA they end at the CLI's loopback listener. + # It must stay on the gateway's own hostname (DOMAIN=localhost in the test stack): the login's + # binding cookie is scoped to that host, as it would be in a real browser. echo " - Driving OAuth authorization for ${user_email} (headless)..." curl -s -L -c "$COOKIE_FILE" -b "$COOKIE_FILE" -o /dev/null "${AUTH_URL}&selected_user=${user_email}" || true if [[ -n "$secret" ]]; then local totp_code totp_code=$(generate_totp_code "$secret") - echo " Sending MFA code for state: ${STATE}" - local mfa_response + echo " Sending MFA code for the login" + local mfa_response return_path mfa_response=$(curl -s -c "$COOKIE_FILE" -b "$COOKIE_FILE" \ -H "Content-Type: application/json" \ --data "{\"state\":\"${STATE}\",\"code\":\"${totp_code}\"}" \ - "http://127.0.0.1:${GATEWAY_PORT}/api/v1/auth/cli/mfa") - echo " MFA response: $mfa_response" + "http://localhost:${GATEWAY_PORT}/api/v1/auth/cli/mfa") + return_path=$(printf '%s' "$mfa_response" | sed -n 's/.*"redirect":"\([^"]*\)".*/\1/p') + if [[ -z "$return_path" ]]; then + echo -e "${RED}MFA submission failed: ${mfa_response}${NC}" >&2 + kill $CLI_PID 2>/dev/null || true + exit 1 + fi + echo " - Returning the browser to the CLI..." + curl -s -L -c "$COOKIE_FILE" -b "$COOKIE_FILE" -o /dev/null \ + "http://localhost:${GATEWAY_PORT}${return_path}" || true fi echo " - Waiting for CLI to complete..." @@ -98,7 +111,9 @@ verify_command_status_and_output() { echo -e "${BLUE}Running: $shell_cmd...${NC}" set +e local output - output=$(eval "$shell_cmd" 2>&1) + # Never inherit the harness's stdin: `convox env set` (and similar) read extra input from a + # non-terminal stdin until EOF, which hangs forever if stdin is an open pipe. + output=$(eval "$shell_cmd" 2>&1 &2 + if [[ -z "$auth_url" ]]; then + echo -e "${RED}CLI login did not produce AUTH_URL${NC}" >&2 kill $cli_pid || true exit 1 fi - curl -s -L -c "$cookie_file" -b "$cookie_file" "${auth_url}&selected_user=admin@example.com" -o /dev/null || true + # Without an MFA factor the browser is sent to enrollment instead of back to the CLI. + local final_url + final_url=$(curl -s -L -c "$cookie_file" -b "$cookie_file" -o /dev/null -w '%{url_effective}' \ + "${auth_url}&selected_user=admin@example.com" || true) + local cli_running=true + kill -0 $cli_pid 2>/dev/null || cli_running=false + kill $cli_pid 2>/dev/null || true set +e - wait $cli_pid - local cli_status=$? + wait $cli_pid 2>/dev/null set -e set +m local cli_output cli_output=$(cat "$output_file") rm -f "$auth_file" "$output_file" "$cookie_file" - if [[ $cli_status -eq 0 ]]; then - echo -e "${RED}CLI login succeeded unexpectedly when MFA enrollment is required.${NC}" >&2 + if [[ "$final_url" != *"/app/account/security"*"enrollment=required"* ]]; then + echo -e "${RED}Browser was not sent to MFA enrollment (ended at: ${final_url}).${NC}" >&2 echo "$cli_output" >&2 exit 1 fi - if ! echo "$cli_output" | grep -Fq "Error: login failed: You must set up multi-factor authentication before you can continue using the CLI."; then - echo -e "${RED}CLI did not report MFA enrollment error as expected.${NC}" >&2 + if [[ "$cli_running" != true ]] || echo "$cli_output" | grep -Fq "Successfully logged in"; then + echo -e "${RED}CLI login finished before MFA enrollment.${NC}" >&2 echo "$cli_output" >&2 exit 1 fi diff --git a/web/e2e/cli-login-webui.spec.ts b/web/e2e/cli-login-webui.spec.ts index 6c5d08f9..a115eb20 100644 --- a/web/e2e/cli-login-webui.spec.ts +++ b/web/e2e/cli-login-webui.spec.ts @@ -1,11 +1,12 @@ /** * Test: CLI login flow followed by WebUI access * - * This test verifies that users who authenticate via CLI can use the WebUI: - * 1. User logs in via CLI (`rack-gateway login`) - * 2. User completes OAuth and MFA in the browser - * 3. User clicks "Open Web UI" button on the success page - * 4. User can perform authenticated actions in the WebUI + * This test verifies the RFC 8252 loopback CLI login and that the browser used for it can use the WebUI: + * 1. The CLI (played by the test) listens on 127.0.0.1 and starts a login with a PKCE challenge + * 2. User completes OAuth and MFA enrollment in the browser + * 3. The browser is handed back to the CLI's loopback listener with a single-use login code + * 4. The CLI redeems the code with its PKCE verifier + * 5. User can perform authenticated actions in the WebUI * * NOTE: Deploy approval "approve" action requires MFAAlways (inline MFA with each request). * The WebUI handles this by showing an MFA dialog and sending X-MFA-TOTP header. @@ -13,8 +14,10 @@ * * See: https://github.com/DocSpring/rack-gateway/issues/12 */ +import type { APIRequestContext, Page } from '@playwright/test' import { authenticator } from 'otplib' import { APIRoute, WebRoute } from '@/lib/routes' +import { type CliLoopbackLogin, startCliLoopbackLogin } from './cli-loopback' import { clearMfaAttempts, createPendingDeployApprovalRequest, @@ -24,6 +27,60 @@ import { expect, test } from './fixtures' import { ensureMfaEnrollment, resetMfaFor } from './helpers' const ADMIN_EMAIL = 'admin@example.com' +const LOOPBACK_URL = /^http:\/\/127\.0\.0\.1:\d+\/callback/ + +/** Runs the browser half of a CLI login for an unenrolled admin and returns the new TOTP secret. */ +async function approveCliLoginWithEnrollment(page: Page, cli: CliLoopbackLogin): Promise { + await page.goto(cli.authUrl) + + const userCard = page.locator('text=Admin User').first() + await expect(userCard).toBeVisible({ timeout: 5000 }) + await userCard.click() + + // The user has no MFA factor yet, so the browser goes to enrollment with the CLI login preserved + await page.waitForURL(/\/app\/account\/security.*enrollment=required/, { timeout: 10_000 }) + expect(page.url()).toContain('channel=cli') + + // After enrolling, the browser is handed back to the CLI's loopback listener + const secret = await ensureMfaEnrollment(page, { email: ADMIN_EMAIL, useUi: true }) + await expect(page).toHaveURL(LOOPBACK_URL, { timeout: 15_000 }) + await expect(page.getByText(/Login approved/i)).toBeVisible() + return secret +} + +async function withCliLogin( + request: APIRequestContext, + run: (cli: CliLoopbackLogin) => Promise +): Promise { + const cli = await startCliLoopbackLogin(request) + try { + await run(cli) + } finally { + await cli.close() + } +} + +async function readCsrfToken(page: Page): Promise { + const csrfToken = await page.evaluate(() => { + const meta = document.querySelector('meta[name="rgw-csrf-token"]') + return meta?.content ?? null + }) + expect(csrfToken).toBeTruthy() + return csrfToken as string +} + +async function fetchJson(page: Page, endpoint: string) { + return await page.evaluate(async (url) => { + const response = await fetch(url, { credentials: 'include' }) + let data: unknown = null + try { + data = await response.json() + } catch { + // Ignore JSON parse errors + } + return { ok: response.ok, status: response.status, data } + }, endpoint) +} test.describe('CLI login to WebUI flow', () => { test.beforeEach(async () => { @@ -34,227 +91,79 @@ test.describe('CLI login to WebUI flow', () => { page, request, }) => { - // Step 1: Start CLI login flow - const startResponse = await request.post(APIRoute('auth/cli/start')) - expect(startResponse.ok()).toBeTruthy() - const startData = await startResponse.json() - expect(startData.auth_url).toBeTruthy() - expect(startData.state).toBeTruthy() - - // Step 2: Navigate to the OAuth URL (mock OAuth will redirect to callback) - await page.goto(startData.auth_url) - - // Select the admin user card in mock OAuth - const userCard = page.locator('text=Admin User').first() - await expect(userCard).toBeVisible({ timeout: 5000 }) - await userCard.click() - - // Step 3: Since user doesn't have MFA enrolled, they're redirected directly to enrollment - // The CLI state is preserved in the URL - await page.waitForURL(/\/app\/account\/security.*enrollment=required/, { timeout: 10_000 }) - expect(page.url()).toContain('channel=cli') - - // Complete MFA enrollment - const secret = await ensureMfaEnrollment(page, { email: ADMIN_EMAIL, useUi: true }) - - // After enrollment, should redirect back to CLI success page - await expect(page).toHaveURL(/\/app\/cli\/auth\/success/, { timeout: 15_000 }) + await withCliLogin(request, async (cli) => { + const secret = await approveCliLoginWithEnrollment(page, cli) - // Verify we're on the CLI success page - await expect(page.getByText(/Authentication Complete/i)).toBeVisible() - await expect(page.getByText(/Your CLI login is approved/i)).toBeVisible() + // The CLI redeems the login code with its PKCE verifier + const completion = await cli.complete(request) + expect(completion.status).toBe(200) + expect(completion.token).toBeTruthy() - // Step 5: Click "Open Web UI" button - const openWebUIButton = page.getByRole('link', { name: /Open Web UI/i }) - await expect(openWebUIButton).toBeVisible() - await openWebUIButton.click() - - // Should navigate to /app/ (or /app/rack as the default landing page) - await page.waitForURL(/\/app\/(rack)?$/, { timeout: 10_000 }) - - // Step 6: Verify we're authenticated by checking the info endpoint - const infoResponse = await page.evaluate(async (endpoint) => { - const response = await fetch(endpoint, { credentials: 'include' }) - let data: unknown = null - try { - data = await response.json() - } catch { - // Ignore JSON parse errors - } - return { ok: response.ok, status: response.status, data } - }, APIRoute('info')) - - // Get CSRF token from the page (needed for POST requests) - const csrfToken = await page.evaluate(() => { - const meta = document.querySelector('meta[name="rgw-csrf-token"]') - return meta?.content ?? null - }) - expect(csrfToken).toBeTruthy() + // The browser that approved the CLI login also holds a web session + await page.goto(WebRoute('rack')) + await page.waitForURL(/\/app\/rack/, { timeout: 10_000 }) + const csrfToken = await readCsrfToken(page) - expect(infoResponse.ok).toBeTruthy() - expect(infoResponse.status).toBe(200) - expect((infoResponse.data as { user?: { email?: string } })?.user?.email).toBe(ADMIN_EMAIL) + const infoResponse = await fetchJson(page, APIRoute('info')) + expect(infoResponse.status).toBe(200) + expect((infoResponse.data as { user?: { email?: string } })?.user?.email).toBe(ADMIN_EMAIL) - // Step 6b: Check MFA status to understand session state - const mfaStatusResponse = await page.evaluate(async (endpoint) => { - const response = await fetch(endpoint, { credentials: 'include' }) - return response.json() - }, APIRoute('auth/mfa/status')) - - // Verify step-up is valid (MFA was verified during enrollment) - expect(mfaStatusResponse.recent_step_up_expires_at).toBeTruthy() - - // Step 7: Create a deploy approval request to test approving it - const approvalPublicId = await createPendingDeployApprovalRequest() - - try { - // Step 8: Test the approve API with inline MFA (MFAAlways requirement) - // The approve endpoint requires inline MFA with every request. - // The WebUI's MFA dialog handles this by prompting for TOTP and sending X-MFA-TOTP header. - // Here we simulate that by generating a TOTP code and sending it with the request. - // Other tests on this shard may have used admin's current TOTP time step; clear replay state - // so this code isn't rejected as a replay. - await clearMfaAttempts() - const totpCode = authenticator.generate(secret) - - const approveResponse = await page.evaluate( - async ({ endpoint, publicId, code, csrf }) => { - const response = await fetch(`${endpoint}/${publicId}/approve`, { - method: 'POST', - credentials: 'include', - headers: { - 'Content-Type': 'application/json', - 'X-MFA-TOTP': code, - 'X-CSRF-Token': csrf, - }, - body: JSON.stringify({}), - }) - let body: unknown = null - try { - body = await response.json() - } catch { - // Ignore parse errors - } - return { ok: response.ok, status: response.status, body } - }, - { - endpoint: APIRoute('deploy-approval-requests'), - publicId: approvalPublicId, - code: totpCode, - csrf: csrfToken as string, - } - ) - - // The approve request should succeed when MFA code is provided + // Step-up is valid (MFA was verified during enrollment) + const mfaStatus = await fetchJson(page, APIRoute('auth/mfa/status')) expect( - approveResponse.ok, - `POST to approve endpoint failed with status ${approveResponse.status}. ` + - `Body: ${JSON.stringify(approveResponse.body)}. ` + - 'CLI login sessions should properly authenticate POST requests with MFA.' + (mfaStatus.data as { recent_step_up_expires_at?: string })?.recent_step_up_expires_at ).toBeTruthy() - // Navigate to deploy approvals to verify the status changed - await page.goto(WebRoute('deploy-approval-requests')) - await expect(page.getByRole('heading', { name: /Deploy Approvals/i })).toBeVisible({ - timeout: 10_000, - }) - - // Verify the approval was successful by checking the status changed - // Look for a table cell containing the "approved" status text - await expect(page.locator('table td:has-text("approved")').first()).toBeVisible({ - timeout: 5000, - }) - } finally { - // Cleanup - await deleteDeployApprovalRequest(approvalPublicId) - } + await approveDeployRequestWithInlineMfa(page, secret, csrfToken) + }) }) test('API calls after CLI login work correctly', async ({ page, request }) => { - // This is a more focused test to verify session cookies are working - - // Step 1: Start CLI login flow - const startResponse = await request.post(APIRoute('auth/cli/start')) - expect(startResponse.ok()).toBeTruthy() - const startData = await startResponse.json() - - // Step 2: Navigate to the OAuth URL - await page.goto(startData.auth_url) - - // Select the admin user card - const userCard = page.locator('text=Admin User').first() - await expect(userCard).toBeVisible({ timeout: 5000 }) - await userCard.click() - - // Step 3: Since user doesn't have MFA enrolled, they're redirected directly to enrollment - await page.waitForURL(/\/app\/account\/security.*enrollment=required/, { timeout: 10_000 }) - - // Complete MFA enrollment - await ensureMfaEnrollment(page, { email: ADMIN_EMAIL, useUi: true }) - - // After enrollment, should be on CLI success page - await expect(page).toHaveURL(/\/app\/cli\/auth\/success/, { timeout: 15_000 }) - - // Step 5: Verify session cookie was set - const cookies = await page.context().cookies() - const sessionCookie = cookies.find((c) => c.name === 'session_token') - expect(sessionCookie).toBeTruthy() - - // Navigate to the app to get CSRF token (we're on CLI success page, need to go to app) - await page.goto(WebRoute('rack')) - await page.waitForURL(/\/app\/rack/, { timeout: 10_000 }) + await withCliLogin(request, async (cli) => { + await approveCliLoginWithEnrollment(page, cli) + + // The login code is single use + expect((await cli.complete(request)).status).toBe(200) + expect((await cli.complete(request)).status).toBe(400) + + const cookies = await page.context().cookies() + expect(cookies.find((c) => c.name === 'session_token')).toBeTruthy() + + await page.goto(WebRoute('rack')) + await page.waitForURL(/\/app\/rack/, { timeout: 10_000 }) + const csrfToken = await readCsrfToken(page) + + for (const endpoint of ['info', 'auth/mfa/status', 'deploy-approval-requests']) { + const result = await fetchJson(page, APIRoute(endpoint)) + expect( + result.ok, + `Expected ${endpoint} to return ok, got status ${result.status}` + ).toBeTruthy() + } - // Get CSRF token from the page (needed for POST requests) - const csrfToken = await page.evaluate(() => { - const meta = document.querySelector('meta[name="rgw-csrf-token"]') - return meta?.content ?? null + await assertDeployRequestPostAuthenticates(page, csrfToken) }) - expect(csrfToken).toBeTruthy() - - // Step 6: Test GET API calls work with the session - // Use page.evaluate to make fetch calls with credentials - const getTestCases = [ - { endpoint: 'info', method: 'GET' }, - { endpoint: 'auth/mfa/status', method: 'GET' }, - { endpoint: 'deploy-approval-requests', method: 'GET' }, - ] - - for (const { endpoint, method } of getTestCases) { - const result = await page.evaluate( - async ({ url, httpMethod }) => { - const response = await fetch(url, { - method: httpMethod, - credentials: 'include', - headers: { 'Content-Type': 'application/json' }, - }) - return { ok: response.ok, status: response.status } - }, - { url: APIRoute(endpoint), httpMethod: method } - ) - - expect( - result.ok, - `Expected ${endpoint} to return ok, got status ${result.status}` - ).toBeTruthy() - } + }) +}) - // Step 7: Test POST API call works (MFANone route - creating deploy approval requests) - // This verifies that POST requests work after CLI login for routes that don't require MFAAlways - const postResult = await page.evaluate( - async ({ url, csrf }) => { - const response = await fetch(url, { +async function approveDeployRequestWithInlineMfa(page: Page, secret: string, csrfToken: string) { + const approvalPublicId = await createPendingDeployApprovalRequest() + try { + // The approve endpoint requires inline MFA with every request (MFAAlways). Other tests on this + // shard may have used admin's current TOTP time step, so clear replay state first. + await clearMfaAttempts() + const totpCode = authenticator.generate(secret) + const approveResponse = await page.evaluate( + async ({ endpoint, publicId, code, csrf }) => { + const response = await fetch(`${endpoint}/${publicId}/approve`, { method: 'POST', credentials: 'include', headers: { 'Content-Type': 'application/json', + 'X-MFA-TOTP': code, 'X-CSRF-Token': csrf, }, - body: JSON.stringify({ - app: 'test-app', - message: 'E2E test deploy approval request', - git_commit_hash: 'abc123def456', - git_branch: 'test-branch', - }), + body: JSON.stringify({}), }) let body: unknown = null try { @@ -264,17 +173,67 @@ test.describe('CLI login to WebUI flow', () => { } return { ok: response.ok, status: response.status, body } }, - { url: APIRoute('deploy-approval-requests'), csrf: csrfToken as string } + { + endpoint: APIRoute('deploy-approval-requests'), + publicId: approvalPublicId, + code: totpCode, + csrf: csrfToken, + } ) - // The request should be authenticated (not 401/403) - // A 400 validation error is acceptable because it means auth worked but the payload was invalid - // We're specifically testing that the session cookie and CSRF token work for POST requests expect( - postResult.status !== 401 && postResult.status !== 403, - `POST deploy-approval-requests failed with auth error status ${postResult.status}. ` + - `Body: ${JSON.stringify(postResult.body)}. ` + - 'This indicates CLI login sessions are not properly authenticating POST requests.' + approveResponse.ok, + `POST to approve endpoint failed with status ${approveResponse.status}. ` + + `Body: ${JSON.stringify(approveResponse.body)}. ` + + 'CLI login sessions should properly authenticate POST requests with MFA.' ).toBeTruthy() - }) -}) + + await page.goto(WebRoute('deploy-approval-requests')) + await expect(page.getByRole('heading', { name: /Deploy Approvals/i })).toBeVisible({ + timeout: 10_000, + }) + await expect(page.locator('table td:has-text("approved")').first()).toBeVisible({ + timeout: 5000, + }) + } finally { + await deleteDeployApprovalRequest(approvalPublicId) + } +} + +async function assertDeployRequestPostAuthenticates(page: Page, csrfToken: string) { + // POST to an MFANone route works after CLI login (session cookie + CSRF token) + const postResult = await page.evaluate( + async ({ url, csrf }) => { + const response = await fetch(url, { + method: 'POST', + credentials: 'include', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': csrf, + }, + body: JSON.stringify({ + app: 'test-app', + message: 'E2E test deploy approval request', + git_commit_hash: 'abc123def456', + git_branch: 'test-branch', + }), + }) + let body: unknown = null + try { + body = await response.json() + } catch { + // Ignore parse errors + } + return { ok: response.ok, status: response.status, body } + }, + { url: APIRoute('deploy-approval-requests'), csrf: csrfToken } + ) + + // A 400 validation error is acceptable: auth worked but the payload was invalid + expect( + postResult.status !== 401 && postResult.status !== 403, + `POST deploy-approval-requests failed with auth error status ${postResult.status}. ` + + `Body: ${JSON.stringify(postResult.body)}. ` + + 'This indicates CLI login sessions are not properly authenticating POST requests.' + ).toBeTruthy() +} diff --git a/web/e2e/cli-loopback.ts b/web/e2e/cli-loopback.ts new file mode 100644 index 00000000..87abb41f --- /dev/null +++ b/web/e2e/cli-loopback.ts @@ -0,0 +1,81 @@ +import { createHash, randomBytes } from 'node:crypto' +import { createServer } from 'node:http' +import type { AddressInfo } from 'node:net' +import type { APIRequestContext } from '@playwright/test' +import { APIRoute } from '@/lib/routes' + +export type CliLoopbackLogin = { + /** Identity provider URL the CLI would open in the browser. */ + authUrl: string + /** Resolves with the single-use login code the gateway delivers to the loopback listener. */ + waitForLoginCode: () => Promise + /** Redeems the login code with the PKCE verifier, as the CLI does. */ + complete: (request: APIRequestContext) => Promise<{ status: number; token?: string }> + close: () => Promise +} + +const base64url = (buf: Buffer): string => buf.toString('base64url') + +/** + * Plays the rack-gateway CLI in an RFC 8252 loopback login: listens on 127.0.0.1, starts the login + * with a PKCE challenge, and captures the login code the browser is redirected back with. + */ +export async function startCliLoopbackLogin(request: APIRequestContext): Promise { + const verifier = base64url(randomBytes(64)) + const challenge = base64url(createHash('sha256').update(verifier).digest()) + const state = base64url(randomBytes(32)) + + let deliverCode: ((code: string) => void) | null = null + const codePromise = new Promise((resolve) => { + deliverCode = resolve + }) + + const server = createServer((req, res) => { + const url = new URL(req.url ?? '/', 'http://127.0.0.1') + const code = url.searchParams.get('code') + if (url.pathname === '/callback' && url.searchParams.get('state') === state && code) { + deliverCode?.(code) + res.writeHead(200, { 'Content-Type': 'text/html' }) + res.end('

Login approved

') + return + } + res.writeHead(400) + res.end('unexpected login redirect') + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + const { port } = server.address() as AddressInfo + + const response = await request.post(APIRoute('auth/cli/start'), { + data: { + code_challenge: challenge, + code_challenge_method: 'S256', + redirect_uri: `http://127.0.0.1:${port}/callback`, + state, + device_name: 'e2e-cli', + }, + }) + if (!response.ok()) { + server.close() + throw new Error(`auth/cli/start failed: ${response.status()} ${await response.text()}`) + } + const { auth_url: authUrl } = (await response.json()) as { auth_url: string } + + return { + authUrl, + waitForLoginCode: () => codePromise, + complete: async (api) => { + const loginCode = await codePromise + const completion = await api.post(APIRoute('auth/cli/complete'), { + data: { login_code: loginCode, code_verifier: verifier, device_name: 'e2e-cli' }, + }) + const body = (await completion.json().catch(() => ({}))) as { token?: string } + return { status: completion.status(), token: body.token } + }, + close: () => + new Promise((resolve) => { + server.close(() => resolve()) + }), + } +} diff --git a/web/src/app.tsx b/web/src/app.tsx index 4a3881cb..c5e19a7d 100644 --- a/web/src/app.tsx +++ b/web/src/app.tsx @@ -26,7 +26,6 @@ import { AppSettingsPage } from './pages/app-settings-page' import { AppsListPage } from './pages/apps-list-page' import { AuditPage } from './pages/audit-page' import { CallbackPage } from './pages/callback-page' -import { CLIAuthSuccessPage } from './pages/cli-auth-success-page' import { DeployApprovalRequestDetailPage } from './pages/deploy-approval-request-detail-page' import { DeployApprovalRequestsPage } from './pages/deploy-approval-requests-page' import { InstancesPage } from './pages/instances-page' @@ -78,11 +77,6 @@ function buildRouteTree() { path: 'auth/mfa/challenge', component: MFAChallengePage, }) - const cliAuthSuccessRoute = createRoute({ - getParentRoute: () => rootRoute, - path: 'cli/auth/success', - component: CLIAuthSuccessPage, - }) // App layout route with nested pages const layoutRoute = createRoute({ @@ -259,7 +253,6 @@ function buildRouteTree() { callbackRoute, loginErrorRoute, mfaChallengeRoute, - cliAuthSuccessRoute, layoutRoute.addChildren(layoutChildren), ]) } diff --git a/web/src/pages/account-security/use-account-security-mutations.ts b/web/src/pages/account-security/use-account-security-mutations.ts index 355910da..786ca966 100644 --- a/web/src/pages/account-security/use-account-security-mutations.ts +++ b/web/src/pages/account-security/use-account-security-mutations.ts @@ -20,7 +20,7 @@ import { updatePreferredMFAMethod, } from '@/lib/api' import { getErrorMessage } from '@/lib/error-utils' -import { resolveWebRedirect, WebRoute } from '@/lib/routes' +import { APIRoute, resolveWebRedirect } from '@/lib/routes' import { createCredential, prepareCreationOptions, @@ -39,7 +39,8 @@ function handleEnrollmentRedirect( if (typeof window === 'undefined') return if (enrollmentChannel === 'cli' && cliState) { - window.location.assign(`${WebRoute('cli/auth/success')}?state=${encodeURIComponent(cliState)}`) + // The gateway finishes the CLI login and hands the browser back to the waiting CLI. + window.location.assign(`${APIRoute('auth/cli/return')}?state=${encodeURIComponent(cliState)}`) return } diff --git a/web/src/pages/cli-auth-success-page.tsx b/web/src/pages/cli-auth-success-page.tsx deleted file mode 100644 index be172d6e..00000000 --- a/web/src/pages/cli-auth-success-page.tsx +++ /dev/null @@ -1,16 +0,0 @@ -import { AuthResultCard } from '@/components/auth-result-card' -import { Button } from '@/components/ui/button' - -export function CLIAuthSuccessPage() { - return ( - - - - ) -} diff --git a/web/src/pages/mfa-challenge-page.test.tsx b/web/src/pages/mfa-challenge-page.test.tsx new file mode 100644 index 00000000..4c169f6a --- /dev/null +++ b/web/src/pages/mfa-challenge-page.test.tsx @@ -0,0 +1,86 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import type { ReactNode } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { MFAChallengePage } from './mfa-challenge-page' + +const { verifyCliMfa } = vi.hoisted(() => ({ verifyCliMfa: vi.fn() })) + +vi.mock('../lib/api', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, verifyCliMfa } +}) + +vi.mock('../components/mfa-verification-form', () => ({ + MFAVerificationForm: ({ + onVerify, + }: { + onVerify: (params: { method: 'totp'; code: string; trust_device: boolean }) => Promise + }) => ( + + ), +})) + +const assign = vi.fn() + +function renderPage(search: string, children?: ReactNode) { + vi.stubGlobal('location', { ...window.location, assign, search }) + const client = new QueryClient({ defaultOptions: { mutations: { retry: false } } }) + return render( + + + {children} + + ) +} + +describe('MFAChallengePage CLI login', () => { + beforeEach(() => { + assign.mockReset() + }) + + afterEach(() => { + vi.unstubAllGlobals() + vi.clearAllMocks() + }) + + it('shows where the CLI login was started', () => { + renderPage('?state=gateway-state&device=laptop.local&ip=203.0.113.7') + expect(screen.getByTestId('cli-login-initiator')).toHaveTextContent( + 'Login started from laptop.local, IP 203.0.113.7' + ) + }) + + it('hands the browser back to the CLI after MFA', async () => { + verifyCliMfa.mockResolvedValue({ redirect: '/api/v1/auth/cli/return?state=gateway-state' }) + renderPage('?state=gateway-state') + + await userEvent.click(screen.getByRole('button', { name: 'Verify' })) + + await waitFor(() => + expect(assign).toHaveBeenCalledWith('/api/v1/auth/cli/return?state=gateway-state') + ) + expect(verifyCliMfa).toHaveBeenCalledWith({ state: 'gateway-state', code: '123456' }) + }) + + it('shows an error instead of guessing the next step', async () => { + verifyCliMfa.mockResolvedValue({ redirect: '' }) + renderPage('?state=gateway-state') + + await userEvent.click(screen.getByRole('button', { name: 'Verify' })) + + expect(await screen.findByText(/did not return the next login step/i)).toBeInTheDocument() + expect(assign).not.toHaveBeenCalled() + }) +}) diff --git a/web/src/pages/mfa-challenge-page.tsx b/web/src/pages/mfa-challenge-page.tsx index ec6ed7b2..14151bb0 100644 --- a/web/src/pages/mfa-challenge-page.tsx +++ b/web/src/pages/mfa-challenge-page.tsx @@ -17,6 +17,13 @@ type CLICompletion = { redirect: string } +const EXPIRED_MESSAGE = + 'This login session has expired. Return to your terminal and start the login again.' +const BROWSER_MISMATCH_MESSAGE = + 'This login was started in a different browser. Finish it in the browser that opened from your terminal, or start the login again.' +const MISSING_NEXT_STEP_MESSAGE = + 'The gateway did not return the next login step. Return to your terminal and start the login again.' + const CLI_ERROR_MESSAGES: Record = { session_expired: 'This login session has expired. Return to your terminal and start the login again.', @@ -30,6 +37,8 @@ const CLI_ERROR_MESSAGES: Record = { unauthorized: 'You do not have access to this gateway.', service_unavailable: 'Login approval is temporarily unavailable. Try again shortly.', persist_failure: 'Failed to finalise the login approval. Please try again.', + browser_mismatch: BROWSER_MISMATCH_MESSAGE, + expired: EXPIRED_MESSAGE, } const WEB_ERROR_MESSAGES: Record = { @@ -58,7 +67,9 @@ function mapQueryError(code: string | null): string | null { case 'load_failure': return 'We could not load the login session. Try again from your terminal.' case 'expired': - return 'This login session has expired. Return to your terminal and start the login again.' + return EXPIRED_MESSAGE + case 'browser_mismatch': + return BROWSER_MISMATCH_MESSAGE default: return code } @@ -109,8 +120,16 @@ async function handleWebAuthnCLI( session_data: sessionData, assertion_response: assertionResponse, }) + return cliNextStep(result) +} + +// cliNextStep returns the gateway URL that hands the browser back to the waiting CLI. +function cliNextStep(result: CLICompletion | null | undefined): string { const target = result?.redirect?.trim() - return target && target !== '' ? target : WebRoute('cli/auth/success') + if (!target) { + throw new Error(MISSING_NEXT_STEP_MESSAGE) + } + return target } async function handleWebAuthnWeb( @@ -127,12 +146,28 @@ async function handleWebAuthnWeb( return resolveWebRedirect(redirectTarget) } +// CLIInitiator shows where the CLI login was started so the user can spot a login they did not start. +function CLIInitiator({ device, ipAddress }: { device: string | null; ipAddress: string | null }) { + if (!(device || ipAddress)) { + return null + } + const parts = [device, ipAddress ? `IP ${ipAddress}` : null].filter(Boolean).join(', ') + return ( +

+ Login started from {parts}. If you did not just run rack-gateway login, cancel + this login. +

+ ) +} + export function MFAChallengePage() { const search = useMemo(() => new URLSearchParams(window.location.search), []) const state = extractParam(search, 'state') const channel = extractParam(search, 'channel') ?? extractParam(search, 'flow') const redirectParam = extractParam(search, 'redirect') const presetError = mapQueryError(extractParam(search, 'error')) + const initiatorDevice = extractParam(search, 'device') + const initiatorIP = extractParam(search, 'ip') const mode = resolveMode(channel, state) const redirectTarget = useMemo(() => normalizeRedirectPath(redirectParam), [redirectParam]) @@ -161,9 +196,11 @@ export function MFAChallengePage() { }, onSuccess: (result) => { if (mode === 'cli') { - const target = result?.redirect?.trim() - const destination = target && target !== '' ? target : WebRoute('cli/auth/success') - window.location.assign(destination) + try { + window.location.assign(cliNextStep(result)) + } catch (err) { + setError(mapServerError(mode, err)) + } return } @@ -194,6 +231,9 @@ export function MFAChallengePage() { {title} + {mode === 'cli' ? ( + + ) : null} {error ? ( From 084c03e03855b1fb4d8fb979b0c58f2bd19883a3 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Sat, 10 Oct 2026 12:06:54 +1300 Subject: [PATCH 2/2] Review fixes for the loopback CLI login Findings from the PR #22 review: - A first factor enrolled during an enrollment-required CLI login only completes the login if the browser that approved it verified its own session after the enrollment. - The Google code is exchanged in the callback and the browser is bound only after it succeeds, so a junk-code callback can't claim the login, and the Google code is no longer stored. - Failures after binding, provider errors and exchange failures go back to the CLI's loopback listener, so the CLI exits instead of waiting out its timeout. "Cancel Login" now cancels on the server (POST /auth/cli/cancel) and tells the CLI. - The CLI always prints the login URL, accepts any https identity provider, and reports version skew clearly in both directions. - Login codes are issued once; the CLI-login email dedup uses the client IP instead of the client-supplied device name. - Docs describe the loopback flow, the same-machine requirement (ssh -L for remote hosts) and the CLI upgrade needed after a gateway upgrade. --- .../docs/development/api-reference.mdx | 14 +- .../docs/operations/troubleshooting.mdx | 13 +- .../docs/security/authentication/index.mdx | 15 +- .../security/authentication/oauth-flow.mdx | 80 ++++--- .../docs/user-guide/cli/authentication.mdx | 50 +++-- .../src/content/docs/user-guide/cli/index.mdx | 3 +- internal/cli/CLAUDE.md | 20 +- internal/cli/auth.go | 19 +- internal/cli/cli_login.go | 26 ++- internal/cli/login_loopback.go | 26 ++- internal/cli/login_loopback_test.go | 127 +++++++++--- internal/gateway/db/cli_login.go | 45 ++-- .../20261009000000_cli_login_loopback.sql | 6 +- internal/gateway/handlers/auth_cli.go | 139 ++++++++----- .../handlers/auth_cli_browser_session.go | 26 ++- .../gateway/handlers/auth_cli_loopback.go | 95 +++++++-- .../handlers/auth_cli_loopback_test.go | 142 ++++++++++++- internal/gateway/handlers/dto.go | 10 + .../gateway/openapi/generated/swagger.json | 75 ++++++- internal/gateway/routes/mfa_recovery_test.go | 9 +- internal/gateway/routes/route_registration.go | 1 + internal/gateway/security/notifier.go | 5 +- web/src/api/generated.ts | 78 +++++-- web/src/api/openapi.json | 196 ++++++++++++++---- .../api/schemas/getAuthCliCallbackParams.ts | 6 +- web/src/api/schemas/getAuthCliReturnParams.ts | 14 ++ .../schemas/handlersCLILoginCancelRequest.ts | 11 + .../handlersCLILoginCompleteRequest.ts | 2 +- .../handlersCLILoginRedirectResponse.ts | 11 + .../schemas/handlersCLILoginStartRequest.ts | 15 ++ .../schemas/handlersCLILoginStartResponse.ts | 11 + web/src/api/schemas/index.ts | 5 + web/src/api/types.generated.ts | 151 ++++++++++++-- web/src/lib/api.ts | 9 + .../use-account-security-mutations.ts | 3 +- web/src/pages/mfa-challenge-page.test.tsx | 18 +- web/src/pages/mfa-challenge-page.tsx | 15 +- 37 files changed, 1182 insertions(+), 309 deletions(-) create mode 100644 web/src/api/schemas/getAuthCliReturnParams.ts create mode 100644 web/src/api/schemas/handlersCLILoginCancelRequest.ts create mode 100644 web/src/api/schemas/handlersCLILoginRedirectResponse.ts create mode 100644 web/src/api/schemas/handlersCLILoginStartRequest.ts create mode 100644 web/src/api/schemas/handlersCLILoginStartResponse.ts diff --git a/docs/src/content/docs/development/api-reference.mdx b/docs/src/content/docs/development/api-reference.mdx index 3b9c6a1e..82084094 100644 --- a/docs/src/content/docs/development/api-reference.mdx +++ b/docs/src/content/docs/development/api-reference.mdx @@ -48,11 +48,15 @@ For the complete, generated schema, use `GET /openapi.json`. ### OAuth + CLI Login -- `POST /auth/cli/start` -- `GET /auth/cli/callback` -- `POST /auth/cli/complete` -- `GET /auth/cli/mfa` -- `POST /auth/cli/mfa` +CLI login uses the RFC 8252 loopback flow (see [OAuth Flow](/security/authentication/oauth-flow/)): + +- `POST /auth/cli/start` - S256 code challenge, CLI state and loopback `redirect_uri`; returns `auth_url` +- `GET /auth/cli/callback` - identity provider redirect; exchanges the code, then binds the browser +- `GET /auth/cli/mfa` - continues the bound browser to MFA (or enrollment) +- `POST /auth/cli/mfa` - submits an MFA code from the bound browser +- `GET /auth/cli/return` - issues the single-use login code and redirects the browser to the CLI's loopback listener +- `POST /auth/cli/cancel` - cancels the login from the bound browser; returns the loopback URL that tells the CLI +- `POST /auth/cli/complete` - redeems the login code with the CLI's code verifier for a session token - `GET /auth/web/login` (also supports `HEAD`) - `GET /auth/web/callback` - `GET /auth/web/logout` diff --git a/docs/src/content/docs/operations/troubleshooting.mdx b/docs/src/content/docs/operations/troubleshooting.mdx index cc558899..696197d9 100644 --- a/docs/src/content/docs/operations/troubleshooting.mdx +++ b/docs/src/content/docs/operations/troubleshooting.mdx @@ -48,7 +48,8 @@ convox logs -a rack-gateway --since 10m ### CLI Login Hangs -The CLI does not run a local callback server. It opens the browser and polls the gateway. +The CLI listens on `http://127.0.0.1:/callback` and waits (up to 10 minutes) for the browser +to be redirected there with a single-use login code. @@ -58,13 +59,19 @@ The CLI does not run a local callback server. It opens the browser and polls the curl https://gateway.example.com/api/v1/health ``` -2. **Use `--no-open` to capture the auth URL** +2. **Use a browser on the same machine as the CLI** + + The login code is delivered to `127.0.0.1`. For a remote host, forward the port from the CLI's + "Waiting for the browser to return to `http://127.0.0.1:/callback`" line with + `ssh -L :127.0.0.1: `. Local firewalls must allow connections to `127.0.0.1`. + +3. **Use `--no-open` to capture the auth URL** ```bash rack-gateway login production https://gateway.example.com --no-open ``` -3. **Complete the browser flow** +4. **Complete the browser flow** Ensure the browser can reach the gateway and Google OAuth endpoints. diff --git a/docs/src/content/docs/security/authentication/index.mdx b/docs/src/content/docs/security/authentication/index.mdx index 8fc1ce52..82a73420 100644 --- a/docs/src/content/docs/security/authentication/index.mdx +++ b/docs/src/content/docs/security/authentication/index.mdx @@ -77,14 +77,17 @@ The gateway supports two authentication channels: ### CLI Authentication -1. User runs `rack-gateway login` -2. CLI generates PKCE code verifier and challenge -3. Browser opens to Google OAuth with challenge -4. After approval, Google redirects back to the gateway callback -5. CLI polls the gateway to complete login -6. Gateway exchanges code + verifier for ID token and issues a session +1. User runs `rack-gateway login`; the CLI listens on `http://127.0.0.1:/callback` +2. CLI generates its PKCE code verifier and sends only the S256 challenge, its state and the loopback + redirect URI to the gateway +3. Browser opens to Google OAuth (the URL is also printed in the terminal) +4. The gateway exchanges Google's code at the callback, then binds the login to that browser +5. After MFA, the browser is redirected to the CLI's loopback listener with a single-use login code +6. The CLI redeems the login code with its code verifier and receives a session token 7. Session token stored in `~/.config/rack-gateway/config.json` +See [OAuth Flow](/security/authentication/oauth-flow/) for details. + ## Session Management Sessions are the primary authentication mechanism after initial OAuth: diff --git a/docs/src/content/docs/security/authentication/oauth-flow.mdx b/docs/src/content/docs/security/authentication/oauth-flow.mdx index 814db2a2..0691edcf 100644 --- a/docs/src/content/docs/security/authentication/oauth-flow.mdx +++ b/docs/src/content/docs/security/authentication/oauth-flow.mdx @@ -83,9 +83,11 @@ sequenceDiagram | `prompt` | `select_account` | Always show account picker | | `hd` | Allowed domain | Filter to organization accounts | -## CLI OAuth Flow (with PKCE) +## CLI Login Flow (RFC 8252 loopback, with PKCE) -The CLI uses OAuth 2.0 with PKCE (Proof Key for Code Exchange) for enhanced security: +The CLI logs in with the [RFC 8252](https://www.rfc-editor.org/rfc/rfc8252) loopback flow. Two PKCE pairs +are involved: the gateway's own pair for the Google leg, and the CLI's pair, which proves that only the +CLI that started the login can finish it. ```mermaid sequenceDiagram @@ -94,43 +96,67 @@ sequenceDiagram participant Browser participant Google - CLI->>Gateway: POST /api/v1/auth/cli/start - Gateway->>Gateway: Generate state + PKCE - Gateway-->>CLI: auth_url, state, code_verifier + CLI->>CLI: Listen on http://127.0.0.1:/callback + CLI->>Gateway: POST /api/v1/auth/cli/start (S256 challenge, CLI state, loopback redirect_uri) + Gateway-->>CLI: auth_url (only) - CLI->>Browser: Open auth_url - Browser->>Google: Authorization request + code_challenge - Note over Browser,Google: User authenticates + CLI->>Browser: Open auth_url (also printed in the terminal) + Browser->>Google: Authorization request (gateway PKCE) + Note over Browser,Google: User signs in Google-->>Browser: Redirect to callback Browser->>Gateway: GET /api/v1/auth/cli/callback?code=...&state=... - Gateway-->>Browser: Redirect to MFA/success page - - CLI->>Gateway: POST /api/v1/auth/cli/complete - Note right of CLI: Includes state + code_verifier (polled) - Gateway->>Gateway: Validate state + MFA - Gateway->>Google: Exchange code + verifier + Gateway->>Google: Exchange code + gateway verifier Google-->>Gateway: ID token - Gateway->>Gateway: Verify + create session + Gateway-->>Browser: Bind this browser (HttpOnly cookie), continue to MFA + + Browser->>Gateway: MFA challenge (or first-factor enrollment) + Browser->>Gateway: GET /api/v1/auth/cli/return + Gateway-->>Browser: Redirect to 127.0.0.1 loopback with single-use login code + CLI state + Browser->>CLI: GET /callback?code=...&state=... + + CLI->>Gateway: POST /api/v1/auth/cli/complete (login_code + CLI code_verifier) Gateway-->>CLI: Session token ``` -### Why PKCE? +### What protects the login + +- **Loopback redirect only.** `/auth/cli/start` only accepts a redirect URI of the form + `http://127.0.0.1:/callback` (or `http://[::1]:/callback`). The login code is only ever sent to + that address, so a login link someone else started delivers its code to *your* machine, never theirs. +- **Exchange before binding.** The gateway exchanges Google's authorization code at the callback, and only + when that succeeds binds the login to the browser with an HttpOnly, SameSite=Lax cookie (path + `/api/v1/auth/cli`). Every later browser step (MFA form, MFA submit, return, cancel) requires that cookie. + The Google code is never stored. +- **Single-use login code.** The login code is 256 bits of randomness, stored only as a SHA-256 hash, valid + for 2 minutes, issued once per login, and deleted when redeemed (a failed redemption also burns it). +- **CLI PKCE.** Redeeming the login code also requires the CLI's code verifier, which never leaves the CLI. +- **Time limit.** The whole login must finish within 10 minutes. +- **First-factor enrollment.** A user with no MFA factor can enroll one during the login, but only in the + browser bound to the login. +- **Visible initiator.** The MFA page shows the device name and IP that started the login, and the user gets + a "New CLI Login" email when a CLI session is created. **Cancel Login** ends the login on the gateway and + tells the waiting CLI. + +### PKCE parameters (CLI pair) -PKCE prevents authorization code interception attacks: +| Parameter | Description | +|-----------|-------------| +| `code_verifier` | 64 random bytes, base64url encoded (86 characters); kept by the CLI | +| `code_challenge` | SHA-256(code_verifier), base64url encoded (43 characters) | +| `code_challenge_method` | Always `S256` | -1. **Code verifier**: High-entropy random string (128 bytes) -2. **Code challenge**: SHA-256 hash of verifier -3. **Verification**: Google verifies the verifier matches the challenge +### Same-machine requirement -Even if an attacker intercepts the authorization code, they cannot exchange it without the code verifier. +The browser must run on the same machine as the CLI, because the login code is delivered to +`127.0.0.1`. To log in from a remote host (for example over SSH), forward the CLI's port: start +`rack-gateway login --no-open`, note the port in the printed "Waiting for the browser to return to +http://127.0.0.1:/callback" line, and run `ssh -L :127.0.0.1: ` from the machine with +the browser before opening the printed login URL there. -### PKCE Parameters +### Upgrading -| Parameter | Description | -|-----------|-------------| -| `code_verifier` | 128-byte random string (base64url encoded) | -| `code_challenge` | SHA-256(code_verifier), base64url encoded | -| `code_challenge_method` | Always `S256` | +The gateway and CLI must both support the loopback flow. After upgrading the gateway, install the matching +rack-gateway CLI: an older CLI is told to upgrade, and a newer CLI refuses to log in to an older gateway. ## Token Verification diff --git a/docs/src/content/docs/user-guide/cli/authentication.mdx b/docs/src/content/docs/user-guide/cli/authentication.mdx index b25cc987..e4faa2de 100644 --- a/docs/src/content/docs/user-guide/cli/authentication.mdx +++ b/docs/src/content/docs/user-guide/cli/authentication.mdx @@ -46,20 +46,27 @@ sequenceDiagram participant Gateway participant Google - CLI->>Gateway: 1. Start login + CLI->>Gateway: 1. Start login (loopback redirect URI + PKCE challenge) Gateway-->>CLI: 2. OAuth URL CLI->>Google: 3. Open browser - Google-->>Gateway: 4. Auth callback - Gateway-->>CLI: 5. Session token + Google-->>Gateway: 4. Auth callback (gateway exchanges the code) + Gateway-->>CLI: 5. Browser redirected to 127.0.0.1 with a single-use login code + CLI->>Gateway: 6. Redeem login code + PKCE verifier + Gateway-->>CLI: 7. Session token ``` The CLI: -1. Requests a login URL from the gateway -2. Opens your browser to Google's OAuth consent page -3. The gateway receives the OAuth callback and stores the auth code -4. The CLI polls the gateway to complete login +1. Listens on `http://127.0.0.1:/callback` and starts the login with the gateway +2. Prints the login URL and opens your browser to Google's sign-in page +3. After you sign in (and complete MFA, or enroll a first factor), the browser is sent back to the CLI's + local listener with a single-use login code +4. Redeems the code together with its PKCE verifier for a session token 5. Stores the session token in your config file +The whole login must finish within 10 minutes. The browser must be on the **same machine** as the CLI, +because the login code is delivered to `127.0.0.1` (see [Logging in from a remote host](#logging-in-from-a-remote-host)). +**Cancel Login** on the approval page ends the login and the CLI exits. + ## Session Storage Session tokens are stored in `~/.config/rack-gateway/config.json`: @@ -190,17 +197,36 @@ See [MFA Verification](/user-guide/cli/mfa-verification/) for details. The CLI tries to open your default browser. If it fails: ```bash -# Copy the printed URL and open it manually +# The CLI always prints the login URL; open it manually in a browser on the same machine rack-gateway login production https://gateway.example.com -# Output: Open this URL in your browser: https://gateway.example.com/api/v1/auth/cli/start?... +# Output: Open this URL in your browser to log in (on this machine): https://accounts.google.com/... ``` ### "OAuth callback failed" -The gateway receives the OAuth callback, then the CLI polls until completion. Issues can include: +After Google, the gateway sends the browser to the CLI's local listener on `127.0.0.1`. Issues can include: - **Gateway not reachable**: DNS/VPN/Tailscale issues -- **Browser blocked the redirect**: allow the callback URL -- **Stale login state**: retry `rack-gateway login` +- **Browser on another machine**: the redirect to `127.0.0.1` can't reach the CLI (see below) +- **Local firewall or security software** blocking connections to `127.0.0.1` +- **Login took longer than 10 minutes**: retry `rack-gateway login` + +### Logging in from a remote host + +When the CLI runs on a remote machine (for example over SSH), forward its listener port to the machine with +the browser. Run `rack-gateway login --no-open` on the remote host, note the port in the printed +"Waiting for the browser to return to `http://127.0.0.1:/callback`" line, then from the machine with +the browser run: + +```bash +ssh -L :127.0.0.1: +``` + +and open the printed URL in the browser there. + +### "Gateway is older than this CLI" / "CLI is too old" + +The gateway and CLI must both support the loopback login. Upgrade whichever side is older; after upgrading +the gateway, install the matching rack-gateway CLI. ### "Domain not allowed" diff --git a/docs/src/content/docs/user-guide/cli/index.mdx b/docs/src/content/docs/user-guide/cli/index.mdx index 97043495..c8d7630b 100644 --- a/docs/src/content/docs/user-guide/cli/index.mdx +++ b/docs/src/content/docs/user-guide/cli/index.mdx @@ -17,7 +17,8 @@ With Gateway: rack-gateway → Gateway (OAuth session) → Rack API ``` The `rack-gateway` CLI: -- Handles OAuth authentication flow (opens browser, gateway receives callback, CLI polls for completion) +- Handles OAuth authentication flow (opens the browser, receives a single-use login code on a local + `127.0.0.1` listener, and redeems it with its PKCE verifier) - Stores session tokens securely per-rack - Wraps Convox commands - Manages MFA verification when required diff --git a/internal/cli/CLAUDE.md b/internal/cli/CLAUDE.md index 7e08b907..7e270939 100644 --- a/internal/cli/CLAUDE.md +++ b/internal/cli/CLAUDE.md @@ -171,15 +171,21 @@ The integration tests create backups of the real Convox CLI configuration to pre ### OAuth Flow -The CLI uses PKCE (Proof Key for Code Exchange) for secure OAuth without client secrets: - -1. Generate code verifier and challenge -2. Open browser to gateway OAuth endpoint -3. User authenticates with Google -4. Gateway validates and returns authorization code -5. CLI exchanges code for session token +`rack-gateway login` uses the RFC 8252 loopback flow (`cli_login.go`, `login_loopback.go`): + +1. Generate a PKCE verifier/challenge and a random state; listen on `http://127.0.0.1:/callback` +2. `POST /api/v1/auth/cli/start` with the S256 challenge, state and loopback redirect URI; the gateway + returns only `auth_url` (a response with `state`/`code_verifier` means an older gateway → refuse) +3. Always print the login URL, then try to open the browser (only https URLs, or http to a loopback + identity provider when the gateway is loopback) +4. The gateway exchanges Google's code, binds the browser, runs MFA, and redirects the browser to the + loopback listener with a single-use login code (or `error=`, e.g. `cancelled`) +5. Redeem the login code with the verifier at `POST /api/v1/auth/cli/complete` for a session token 6. Token stored in config file +The browser must be on the same machine as the CLI (remote hosts: `ssh -L :127.0.0.1:`). +The login times out after 10 minutes. A gateway upgrade to this flow needs a matching CLI build. + ### Error Handling - Network errors: Suggest checking gateway URL diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 56d74a7b..71a8ad7c 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -13,14 +14,28 @@ import ( "github.com/google/uuid" ) +// errGatewayTooOld means the gateway predates the loopback login this CLI uses. +var errGatewayTooOld = errors.New( + "the gateway is older than this CLI; upgrade the gateway or use an older rack-gateway CLI", +) + // StartLogin starts a loopback login and returns the identity provider URL to open. func StartLogin(gatewayURL string, req LoginStartRequest) (*LoginStartResponse, error) { - var result LoginStartResponse + // Gateways from before the loopback login answer with their own state and code verifier, and + // would never redirect the browser to this CLI. + var result struct { + AuthURL string `json:"auth_url"` + State string `json:"state"` + CodeVerifier string `json:"code_verifier"` + } url := buildGatewayAPIURL(gatewayURL, "/api/v1/auth/cli/start") if err := postLoginJSON(url, req, "login start failed: ", &result); err != nil { return nil, err } - return &result, nil + if result.State != "" || result.CodeVerifier != "" { + return nil, errGatewayTooOld + } + return &LoginStartResponse{AuthURL: result.AuthURL}, nil } // CompleteLogin redeems the single-use login code with the PKCE code verifier for a session token. diff --git a/internal/cli/cli_login.go b/internal/cli/cli_login.go index 456f2d02..97d45cd2 100644 --- a/internal/cli/cli_login.go +++ b/internal/cli/cli_login.go @@ -93,7 +93,8 @@ func runLoopbackLogin(gatewayURL string, noOpen bool, authFile string) (*LoginRe } notifyBrowser(startResp.AuthURL, noOpen) - fmt.Println("Waiting for you to finish logging in in your browser...") + // The listener address matters when the browser is on another machine (forward it with ssh -L). + fmt.Printf("Waiting for the browser to return to %s ...\n", loopback.redirectURI) loginCode, err := loopback.wait(loginTimeout) if err != nil { @@ -189,33 +190,38 @@ func writeAuthFile(path string, startResp *LoginStartResponse) error { return nil } +// notifyBrowser always prints the login URL (the browser may open somewhere you can't see it, or not +// at all, e.g. over SSH or from an agent), then tries to open it unless noOpen is set. func notifyBrowser(authURL string, noOpen bool) { + fmt.Printf("Open this URL in your browser to log in (on this machine):\n%s\n", authURL) if noOpen { - fmt.Printf("Open this URL in your browser to log in:\n%s\n", authURL) return } - fmt.Printf("Opening browser for authentication...\n") if err := OpenBrowser(authURL); err != nil { - fmt.Printf("Please open this URL in your browser:\n%s\n", authURL) + fmt.Printf("Could not open a browser automatically: %v\n", err) } } -// validateAuthURL only lets the CLI open Google's sign-in page, or a loopback identity provider -// when the gateway itself is on a loopback address (local development and tests). +// validateAuthURL lets the CLI open the identity provider's https sign-in page (Google, or the +// gateway's configured OIDC issuer), or a loopback identity provider when the gateway itself is on a +// loopback address (local development and tests). Other schemes and URLs with credentials are refused. func validateAuthURL(authURL, gatewayURL string) error { parsed, err := url.Parse(authURL) if err != nil { return fmt.Errorf("gateway returned an invalid login URL: %w", err) } - if parsed.Scheme == "https" && parsed.Hostname() == "accounts.google.com" { + if parsed.User != nil || parsed.Hostname() == "" { + return fmt.Errorf("gateway returned an unexpected login URL %q", parsed.Redacted()) + } + if parsed.Scheme == "https" { return nil } gateway, err := url.Parse(buildGatewayAPIURL(gatewayURL, "")) - if err == nil && isLoopbackHost(gateway.Hostname()) && isLoopbackHost(parsed.Hostname()) && - (parsed.Scheme == "http" || parsed.Scheme == "https") { + if err == nil && parsed.Scheme == "http" && isLoopbackHost(gateway.Hostname()) && + isLoopbackHost(parsed.Hostname()) { return nil } - return fmt.Errorf("gateway returned an unexpected login URL host %q", parsed.Host) + return fmt.Errorf("gateway returned a login URL that is not https: %q", parsed.Redacted()) } func isLoopbackHost(host string) bool { diff --git a/internal/cli/login_loopback.go b/internal/cli/login_loopback.go index f36e27ba..92a2004d 100644 --- a/internal/cli/login_loopback.go +++ b/internal/cli/login_loopback.go @@ -154,16 +154,22 @@ func (s *loopbackServer) writePage(w http.ResponseWriter, status int, title, mes }) } -// loginErrorMessage maps the gateway's login error codes to messages for the terminal. +// loginErrorMessages maps the gateway's login error codes to messages for the terminal. +var loginErrorMessages = map[string]string{ + "unauthorized": "your account is not authorized for this gateway", + "exchange_failed": "the identity provider login could not be completed", + "session_incomplete": "the login session was incomplete; run rack-gateway login again", + "canceled": "the login was canceled in the browser", + "access_denied": "the identity provider login was canceled or denied", + "identity_provider_error": "the identity provider reported an error", + "session_failed": "the gateway could not start a browser session; run rack-gateway login again", + "persist_failure": "the gateway could not save the login; run rack-gateway login again", + "load_failure": "the gateway could not load the login; run rack-gateway login again", +} + func loginErrorMessage(code string) string { - switch code { - case "unauthorized": - return "your account is not authorized for this gateway" - case "exchange_failed": - return "the identity provider login could not be completed" - case "session_incomplete": - return "the login session was incomplete; run rack-gateway login again" - default: - return fmt.Sprintf("the gateway could not complete the login (%s)", code) + if message, ok := loginErrorMessages[code]; ok { + return message } + return fmt.Sprintf("the gateway could not complete the login (%s)", code) } diff --git a/internal/cli/login_loopback_test.go b/internal/cli/login_loopback_test.go index 101c26e9..a119b02f 100644 --- a/internal/cli/login_loopback_test.go +++ b/internal/cli/login_loopback_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "sync" "testing" "time" @@ -78,6 +79,16 @@ func TestLoopbackServerTimesOut(t *testing.T) { require.ErrorIs(t, err, errLoginTimedOut) } +func TestLoopbackServerReportsCancelledLogin(t *testing.T) { + server, err := startLoopbackServer("expected-state", "http://127.0.0.1:9447") + require.NoError(t, err) + defer server.close() + + getURL(t, server.redirectURI+"?error=canceled&state=expected-state") + _, err = server.wait(time.Second) + require.ErrorContains(t, err, "canceled in the browser") +} + func TestValidateAuthURL(t *testing.T) { cases := []struct { authURL string @@ -85,8 +96,11 @@ func TestValidateAuthURL(t *testing.T) { ok bool }{ {"https://accounts.google.com/o/oauth2/auth?x=1", "https://gateway.example.ts.net", true}, + // The gateway's identity provider can be any https OIDC issuer. + {"https://idp.example.com/authorize?x=1", "https://gateway.example.ts.net", true}, {"http://accounts.google.com/o/oauth2/auth", "https://gateway.example.ts.net", false}, - {"https://evil.example/login", "https://gateway.example.ts.net", false}, + {"https://user:pass@idp.example.com/authorize", "https://gateway.example.ts.net", false}, + {"https:///no-host", "https://gateway.example.ts.net", false}, {"file:///etc/passwd", "https://gateway.example.ts.net", false}, {"http://localhost:9345/authorize", "http://127.0.0.1:9447", true}, {"http://localhost:9345/authorize", "https://gateway.example.ts.net", false}, @@ -103,40 +117,90 @@ func TestValidateAuthURL(t *testing.T) { } // fakeLoopbackGateway plays the gateway: it records the start request and only completes a login -// for the issued login code together with a verifier matching the challenge. +// for the issued login code together with a verifier matching the challenge. Handler errors are +// recorded and checked by the test (require can't be called from the server goroutine). type fakeLoopbackGateway struct { - t *testing.T - start LoginStartRequest + mu sync.Mutex + start LoginStartRequest + errs []error + oldResponse bool +} + +func (g *fakeLoopbackGateway) fail(w http.ResponseWriter, err error) { + g.mu.Lock() + g.errs = append(g.errs, err) + g.mu.Unlock() + w.WriteHeader(http.StatusBadRequest) } func (g *fakeLoopbackGateway) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/api/v1/auth/cli/start": - require.NoError(g.t, json.NewDecoder(r.Body).Decode(&g.start)) - // Play the browser: deliver the login code to the CLI's loopback listener. - go func() { - _, _ = fetch(g.start.RedirectURI + "?code=issued-code&state=" + url.QueryEscape(g.start.State)) - }() - _ = json.NewEncoder(w).Encode(LoginStartResponse{AuthURL: "https://accounts.google.com/o/oauth2/auth"}) + g.serveStart(w, r) case "/api/v1/auth/cli/complete": - var body map[string]string - require.NoError(g.t, json.NewDecoder(r.Body).Decode(&body)) - sum := sha256.Sum256([]byte(body["code_verifier"])) - if body["login_code"] != "issued-code" || - base64.RawURLEncoding.EncodeToString(sum[:]) != g.start.CodeChallenge { - w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"error":"invalid or expired login code"}`)) - return - } - _ = json.NewEncoder(w).Encode(LoginResponse{Token: "session-token", Email: "user@example.com"}) + g.serveComplete(w, r) default: w.WriteHeader(http.StatusNotFound) } } -func TestRunLoopbackLogin(t *testing.T) { +func (g *fakeLoopbackGateway) serveStart(w http.ResponseWriter, r *http.Request) { + var start LoginStartRequest + if err := json.NewDecoder(r.Body).Decode(&start); err != nil { + g.fail(w, err) + return + } + g.mu.Lock() + g.start = start + g.mu.Unlock() + if g.oldResponse { + // A gateway from before the loopback login returns its own state and verifier. + _ = json.NewEncoder(w).Encode(map[string]string{ + "auth_url": "https://accounts.google.com/o/oauth2/auth", "state": "s", "code_verifier": "v", + }) + return + } + // Play the browser: deliver the login code to the CLI's loopback listener. + go func() { + _, _ = fetch(start.RedirectURI + "?code=issued-code&state=" + url.QueryEscape(start.State)) + }() + _ = json.NewEncoder(w).Encode(LoginStartResponse{AuthURL: "https://accounts.google.com/o/oauth2/auth"}) +} + +func (g *fakeLoopbackGateway) serveComplete(w http.ResponseWriter, r *http.Request) { + var body map[string]string + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + g.fail(w, err) + return + } + g.mu.Lock() + challenge := g.start.CodeChallenge + g.mu.Unlock() + sum := sha256.Sum256([]byte(body["code_verifier"])) + if body["login_code"] != "issued-code" || base64.RawURLEncoding.EncodeToString(sum[:]) != challenge { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"invalid or expired login code"}`)) + return + } + _ = json.NewEncoder(w).Encode(LoginResponse{Token: "session-token", Email: "user@example.com"}) +} + +func (g *fakeLoopbackGateway) snapshot() (LoginStartRequest, []error) { + g.mu.Lock() + defer g.mu.Unlock() + return g.start, append([]error(nil), g.errs...) +} + +func useTempConfig(t *testing.T) { + t.Helper() + previous := ConfigPath ConfigPath = t.TempDir() - gateway := &fakeLoopbackGateway{t: t} + t.Cleanup(func() { ConfigPath = previous }) +} + +func TestRunLoopbackLogin(t *testing.T) { + useTempConfig(t) + gateway := &fakeLoopbackGateway{} server := httptest.NewServer(gateway) defer server.Close() @@ -144,8 +208,19 @@ func TestRunLoopbackLogin(t *testing.T) { require.NoError(t, err) require.Equal(t, "session-token", resp.Token) - require.Equal(t, "S256", gateway.start.CodeChallengeMethod) - require.Len(t, gateway.start.CodeChallenge, 43) - require.GreaterOrEqual(t, len(gateway.start.State), 32) - require.Regexp(t, `^http://127\.0\.0\.1:\d+/callback$`, gateway.start.RedirectURI) + start, errs := gateway.snapshot() + require.Empty(t, errs) + require.Equal(t, "S256", start.CodeChallengeMethod) + require.Len(t, start.CodeChallenge, 43) + require.GreaterOrEqual(t, len(start.State), 32) + require.Regexp(t, `^http://127\.0\.0\.1:\d+/callback$`, start.RedirectURI) +} + +func TestRunLoopbackLoginRefusesOldGateway(t *testing.T) { + useTempConfig(t) + server := httptest.NewServer(&fakeLoopbackGateway{oldResponse: true}) + defer server.Close() + + _, err := runLoopbackLogin(server.URL, true, "") + require.ErrorIs(t, err, errGatewayTooOld) } diff --git a/internal/gateway/db/cli_login.go b/internal/gateway/db/cli_login.go index 216d682b..db909483 100644 --- a/internal/gateway/db/cli_login.go +++ b/internal/gateway/db/cli_login.go @@ -14,7 +14,6 @@ const cliLoginLive = "created_at > NOW() - INTERVAL '10 minutes'" // challenge, its loopback redirect URI and its own state value. type CLILoginState struct { State string - OAuthCode sql.NullString OAuthCodeVerifier sql.NullString CLICodeChallenge string CLIRedirectURI string @@ -43,7 +42,7 @@ type NewCLILogin struct { InitiatorDevice string } -const cliLoginColumns = `state, oauth_code, oauth_code_verifier, cli_code_challenge, cli_redirect_uri, cli_state, +const cliLoginColumns = `state, oauth_code_verifier, cli_code_challenge, cli_redirect_uri, cli_state, initiator_ip, initiator_device, browser_binding_hash, enrollment_required, login_email, login_name, mfa_verified_at, mfa_method_id, login_error, created_at, updated_at` @@ -72,15 +71,20 @@ func (d *Database) CreateCLILoginState(login NewCLILogin) error { return nil } -// BindCLILoginBrowser stores the identity provider's authorization code and binds the login to -// the browser that delivered it. It only succeeds once per login, so a replayed callback URL -// cannot bind a second browser. Returns false when the login is unknown, expired or already bound. -func (d *Database) BindCLILoginBrowser(state, oauthCode, bindingHash string) (bool, error) { +// BindCLILoginBrowser binds the login to the browser that completed the identity provider login and +// records who signed in. The gateway's own PKCE verifier is discarded once the exchange has succeeded. +// It only succeeds once per login, so a replayed callback cannot bind a second browser. Returns false +// when the login is unknown, expired or already bound. +func (d *Database) BindCLILoginBrowser(state, bindingHash, email, name string) (bool, error) { res, err := d.exec(` UPDATE cli_login_states - SET oauth_code = ?, browser_binding_hash = ?, updated_at = NOW() + SET browser_binding_hash = ?, + login_email = ?, + login_name = ?, + oauth_code_verifier = NULL, + updated_at = NOW() WHERE state = ? AND browser_binding_hash IS NULL AND `+cliLoginLive, - oauthCode, bindingHash, state) + bindingHash, email, name, state) if err != nil { return false, fmt.Errorf("failed to bind CLI login browser: %w", err) } @@ -91,24 +95,6 @@ func (d *Database) BindCLILoginBrowser(state, oauthCode, bindingHash string) (bo return rows == 1, nil } -// SetCLILoginProfile stores the identity provider result and discards the authorization code. -func (d *Database) SetCLILoginProfile(state, email, name string) error { - _, err := d.exec(` - UPDATE cli_login_states - SET oauth_code = NULL, - oauth_code_verifier = NULL, - login_email = ?, - login_name = ?, - login_error = NULL, - updated_at = NOW() - WHERE state = ? - `, email, name, state) - if err != nil { - return fmt.Errorf("failed to store CLI login profile: %w", err) - } - return nil -} - // MarkCLILoginVerified records that the CLI login has satisfied MFA requirements. func (d *Database) MarkCLILoginVerified(state string, methodID *int64) error { _, err := d.exec(` @@ -163,14 +149,16 @@ func (d *Database) GetCLILoginState(state string) (*CLILoginState, error) { } // SetCLILoginCode stores the hash of a single-use login code for a verified CLI login. -// The code is valid for two minutes. Returns false when the login is not live or not verified. +// The code is valid for two minutes and is issued at most once per login. Returns false when the +// login is not live, not verified, or already has a code. func (d *Database) SetCLILoginCode(state, codeHash string) (bool, error) { res, err := d.exec(` UPDATE cli_login_states SET login_code_hash = ?, login_code_expires_at = NOW() + INTERVAL '2 minutes', updated_at = NOW() - WHERE state = ? AND mfa_verified_at IS NOT NULL AND login_error IS NULL AND `+cliLoginLive, + WHERE state = ? AND mfa_verified_at IS NOT NULL AND login_error IS NULL AND login_code_hash IS NULL + AND `+cliLoginLive, codeHash, state) if err != nil { return false, fmt.Errorf("failed to store CLI login code: %w", err) @@ -208,7 +196,6 @@ func scanCLILoginState(row *sql.Row) (*CLILoginState, error) { var record CLILoginState err := row.Scan( &record.State, - &record.OAuthCode, &record.OAuthCodeVerifier, &record.CLICodeChallenge, &record.CLIRedirectURI, diff --git a/internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql b/internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql index 75574084..25a881fc 100644 --- a/internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql +++ b/internal/gateway/db/migrations/20261009000000_cli_login_loopback.sql @@ -1,18 +1,18 @@ -- CLI login now uses an RFC 8252 loopback redirect: -- * the CLI sends an S256 code challenge, its own state and a 127.0.0.1 redirect URI to /auth/cli/start --- * the browser that completes Google OAuth is bound to the login with an HttpOnly cookie +-- * the identity provider code is exchanged at the callback (never stored), and only then is the +-- browser that completed the login bound to it with an HttpOnly cookie -- * after MFA the browser is redirected to the CLI's loopback listener with a single-use login code -- * /auth/cli/complete requires that login code AND the CLI's code verifier -- Login states only live for minutes, so existing rows are discarded. DELETE FROM cli_login_states; -ALTER TABLE cli_login_states RENAME COLUMN code TO oauth_code; ALTER TABLE cli_login_states RENAME COLUMN code_verifier TO oauth_code_verifier; ALTER TABLE cli_login_states + DROP COLUMN code, DROP COLUMN login_token, DROP COLUMN login_expires_at, - ALTER COLUMN oauth_code TYPE TEXT, ADD COLUMN cli_code_challenge VARCHAR(64) NOT NULL, ADD COLUMN cli_redirect_uri VARCHAR(64) NOT NULL, ADD COLUMN cli_state VARCHAR(128) NOT NULL, diff --git a/internal/gateway/handlers/auth_cli.go b/internal/gateway/handlers/auth_cli.go index f24f44c6..36397d52 100644 --- a/internal/gateway/handlers/auth_cli.go +++ b/internal/gateway/handlers/auth_cli.go @@ -3,6 +3,7 @@ package handlers import ( "errors" "fmt" + "io" "log" "net/http" "net/url" @@ -37,7 +38,7 @@ type cliMFASubmit struct { func (h *AuthHandler) CLILoginStart(c *gin.Context) { var req CLILoginStartRequest if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) + c.JSON(http.StatusBadRequest, gin.H{"error": cliStartBindError(err)}) return } deviceName, err := validateCLIStart(req) @@ -74,18 +75,30 @@ func (h *AuthHandler) CLILoginStart(c *gin.Context) { c.JSON(http.StatusOK, CLILoginStartResponse{AuthURL: resp.AuthURL}) } +// cliTooOldMessage answers CLIs that predate the loopback login: they start a login without sending +// any login parameters. +const cliTooOldMessage = "this rack-gateway CLI is too old for this gateway; " + + "upgrade the rack-gateway CLI and run login again" + +func cliStartBindError(err error) string { + if errors.Is(err, io.EOF) { + return cliTooOldMessage + } + return "invalid request" +} + // CLILoginCallback godoc // @Summary Identity provider redirect for CLI login -// @Description Stores the authorization code and binds the login to this browser. +// @Description Exchanges the authorization code and, only once that succeeds, binds the login to this browser. // @Tags Auth -// @Param code query string true "Authorization code" +// @Param code query string false "Authorization code" // @Param state query string true "State" +// @Param error query string false "Identity provider error" // @Success 307 {string} string "Temporary Redirect" // @Router /auth/cli/callback [get] func (h *AuthHandler) CLILoginCallback(c *gin.Context) { - code := strings.TrimSpace(c.Query("code")) state := strings.TrimSpace(c.Query("state")) - if code == "" || state == "" { + if state == "" { cliRedirectWithError(c, "missing_state") return } @@ -93,13 +106,71 @@ func (h *AuthHandler) CLILoginCallback(c *gin.Context) { cliRedirectWithError(c, "service_unavailable") return } + record, err := h.database.GetCLILoginState(state) + if err != nil { + cliRedirectWithError(c, "load_failure") + return + } + if record == nil || record.BrowserBindingHash.Valid { + // Unknown, expired, or already being completed in another browser. + cliRedirectWithError(c, "expired") + return + } + // Nothing below proves this is the browser the CLI's user is using, so failures are not recorded + // on the login (that would let anyone holding the state end it). The browser is sent to the + // login's own loopback address, which only reaches a CLI on the same machine as the browser. + if providerError := strings.TrimSpace(c.Query("error")); providerError != "" { + returnErrorToCLI(c, record, cliProviderErrorCode(providerError)) + return + } + code := strings.TrimSpace(c.Query("code")) + if code == "" { + cliRedirectWithError(c, "missing_state") + return + } + email, name, errCode := h.cliExchangeOAuthCode(code, record) + if errCode != "" { + returnErrorToCLI(c, record, errCode) + return + } + h.bindCLILoginBrowser(c, record, email, name) +} + +// cliProviderErrorCode maps an identity provider error (e.g. the user declined consent) to a CLI error code. +func cliProviderErrorCode(providerError string) string { + if providerError == "access_denied" { + return "access_denied" + } + return "identity_provider_error" +} + +// cliExchangeOAuthCode exchanges the identity provider code for the signed-in identity. +// Returns the email and name, or an error code for the CLI. +func (h *AuthHandler) cliExchangeOAuthCode(code string, record *db.CLILoginState) (string, string, string) { + if !record.OAuthCodeVerifier.Valid { + return "", "", "session_incomplete" + } + loginResp, err := h.oauth.CompleteLogin(code, record.State, record.OAuthCodeVerifier.String) + if err != nil { + var domainErr *auth.DomainNotAllowedError + if errors.As(err, &domainErr) { + return "", "", "unauthorized" + } + return "", "", "exchange_failed" + } + return strings.TrimSpace(loginResp.Email), loginResp.Name, "" +} + +// bindCLILoginBrowser binds the login to this browser, which has just completed the identity provider +// login for it, and continues to MFA. +func (h *AuthHandler) bindCLILoginBrowser(c *gin.Context, record *db.CLILoginState, email, name string) { binding, err := newCLISecret() if err != nil { cliRedirectWithError(c, "persist_failure") return } - bound, err := h.database.BindCLILoginBrowser(state, code, hashCLISecret(binding)) + bound, err := h.database.BindCLILoginBrowser(record.State, hashCLISecret(binding), email, name) if err != nil { cliRedirectWithError(c, "load_failure") return @@ -108,14 +179,13 @@ func (h *AuthHandler) CLILoginCallback(c *gin.Context) { cliRedirectWithError(c, "expired") return } - h.setCLILoginCookie(c, binding, cliLoginCookieMaxAge) - c.Redirect(http.StatusTemporaryRedirect, cliMFARoute(state)) + c.Redirect(http.StatusTemporaryRedirect, cliMFARoute(record.State)) } // CLILoginMFAForm godoc // @Summary Continue CLI login in the browser -// @Description Completes the identity provider exchange and sends the bound browser to MFA or back to the CLI. +// @Description Sends the bound browser to MFA (or MFA enrollment), or back to the CLI when MFA is satisfied. // @Tags Auth // @Param state query string true "State" // @Success 307 {string} string "Temporary Redirect" @@ -138,7 +208,7 @@ func (h *AuthHandler) CLILoginMFAForm(c *gin.Context) { if !shouldEnforceMFA(h.mfaSettings, userRecord) { if err := h.database.MarkCLILoginVerified(state, nil); err != nil { - cliRedirectWithError(c, "persist_failure") + h.failCLILogin(c, record, "persist_failure") return } c.Redirect(http.StatusTemporaryRedirect, cliReturnRoute(state)) @@ -147,38 +217,32 @@ func (h *AuthHandler) CLILoginMFAForm(c *gin.Context) { if _, err := h.createLoginSession(c, userRecord, "cli-mfa"); err != nil { log.Printf("cli mfa session create failed: user=%s err=%v", userRecord.Email, err) - cliRedirectWithError(c, "session_failed") + h.failCLILogin(c, record, "session_failed") return } if !userRecord.MFAEnrolled { - h.cliRedirectToEnrollment(c, state) + h.cliRedirectToEnrollment(c, record) return } c.Redirect(http.StatusTemporaryRedirect, cliChallengeURL(record)) } -// cliResolveUser exchanges the identity provider code (once) and loads the gateway user. -// On failure the login is ended and the browser is sent back to the CLI with the reason. +// cliResolveUser loads the gateway user who signed in for the bound browser. On failure the login is +// ended and the browser is sent back to the CLI with the reason. func (h *AuthHandler) cliResolveUser(c *gin.Context, record *db.CLILoginState) (*db.User, bool) { email := "" if record.LoginEmail.Valid { email = strings.TrimSpace(record.LoginEmail.String) } if email == "" { - exchanged, errCode := h.cliExchangeOAuthCode(record) - if errCode != "" { - h.failCLILogin(c, record, errCode) - return nil, false - } - email = exchanged - record.LoginEmail.String, record.LoginEmail.Valid = exchanged, true + h.failCLILogin(c, record, "session_incomplete") + return nil, false } - userRecord, err := h.database.GetUser(email) if err != nil { - cliRedirectWithError(c, "load_failure") + h.failCLILogin(c, record, "load_failure") return nil, false } if userRecord == nil { @@ -189,36 +253,15 @@ func (h *AuthHandler) cliResolveUser(c *gin.Context, record *db.CLILoginState) ( return userRecord, true } -// cliExchangeOAuthCode exchanges the stored identity provider code and records the profile. -// Returns the login email, or an error code for the CLI. -func (h *AuthHandler) cliExchangeOAuthCode(record *db.CLILoginState) (string, string) { - if !record.OAuthCode.Valid || !record.OAuthCodeVerifier.Valid { - return "", "session_incomplete" - } - loginResp, err := h.oauth.CompleteLogin(record.OAuthCode.String, record.State, record.OAuthCodeVerifier.String) - if err != nil { - var domainErr *auth.DomainNotAllowedError - if errors.As(err, &domainErr) { - return "", "unauthorized" - } - return "", "exchange_failed" - } - if err := h.database.SetCLILoginProfile(record.State, loginResp.Email, loginResp.Name); err != nil { - return "", "persist_failure" - } - record.LoginName.String, record.LoginName.Valid = loginResp.Name, true - return strings.TrimSpace(loginResp.Email), "" -} - -func (h *AuthHandler) cliRedirectToEnrollment(c *gin.Context, state string) { - if err := h.database.MarkCLILoginEnrollmentRequired(state); err != nil { - cliRedirectWithError(c, "persist_failure") +func (h *AuthHandler) cliRedirectToEnrollment(c *gin.Context, record *db.CLILoginState) { + if err := h.database.MarkCLILoginEnrollmentRequired(record.State); err != nil { + h.failCLILogin(c, record, "persist_failure") return } params := url.Values{} params.Set("enrollment", "required") params.Set("channel", "cli") - params.Set("state", state) + params.Set("state", record.State) c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s?%s", WebRoute("account/security"), params.Encode())) } diff --git a/internal/gateway/handlers/auth_cli_browser_session.go b/internal/gateway/handlers/auth_cli_browser_session.go index 5b7a1d91..5c20be94 100644 --- a/internal/gateway/handlers/auth_cli_browser_session.go +++ b/internal/gateway/handlers/auth_cli_browser_session.go @@ -19,22 +19,30 @@ import ( // guarantee the web challenge gives. Only a web session belonging to the verified user is touched. // Failures are logged rather than returned because the CLI login itself is already approved. func (h *AuthHandler) completeBrowserSessionMFA(c *gin.Context, user *db.User) { - if h.sessions == nil || user == nil { + session := h.cliBrowserSession(c, user) + if session == nil || session.MFAVerifiedAt != nil { return } + if err := h.sessions.UpdateSessionMFAVerified(session.ID, time.Now(), nil); err != nil { + log.Printf("cli mfa: failed to verify browser session=%d user=%s: %v", session.ID, user.Email, err) + } +} + +// cliBrowserSession returns this browser's web session when it belongs to user, or nil. +func (h *AuthHandler) cliBrowserSession(c *gin.Context, user *db.User) *db.UserSession { + if h.sessions == nil || user == nil { + return nil + } token, err := c.Cookie("session_token") if err != nil || strings.TrimSpace(token) == "" { - return + return nil } result, err := h.sessions.ValidateSession(token, c.ClientIP(), c.GetHeader("User-Agent")) if err != nil || result == nil || result.Session == nil || result.User == nil { - return + return nil } - session := result.Session - if result.User.ID != user.ID || session.Channel != "web" || session.MFAVerifiedAt != nil { - return - } - if err := h.sessions.UpdateSessionMFAVerified(session.ID, time.Now(), nil); err != nil { - log.Printf("cli mfa: failed to verify browser session=%d user=%s: %v", session.ID, user.Email, err) + if result.User.ID != user.ID || result.Session.Channel != "web" { + return nil } + return result.Session } diff --git a/internal/gateway/handlers/auth_cli_loopback.go b/internal/gateway/handlers/auth_cli_loopback.go index 2909abe0..5bf9c266 100644 --- a/internal/gateway/handlers/auth_cli_loopback.go +++ b/internal/gateway/handlers/auth_cli_loopback.go @@ -12,6 +12,7 @@ import ( "regexp" "strconv" "strings" + "time" "github.com/gin-gonic/gin" @@ -177,19 +178,62 @@ func cliMFARoute(state string) string { return APIRoute("auth/cli/mfa") + "?state=" + url.QueryEscape(state) } +// cliLoopbackURL is the CLI's loopback listener URL (validated at /auth/cli/start) with the CLI's state. +func cliLoopbackURL(record *db.CLILoginState, params url.Values) string { + params.Set("state", record.CLIState) + return record.CLIRedirectURI + "?" + params.Encode() +} + // redirectToCLI sends the browser to the CLI's loopback listener with the CLI's state. func redirectToCLI(c *gin.Context, record *db.CLILoginState, params url.Values) { - params.Set("state", record.CLIState) - c.Redirect(http.StatusFound, record.CLIRedirectURI+"?"+params.Encode()) + c.Redirect(http.StatusFound, cliLoopbackURL(record, params)) +} + +// returnErrorToCLI tells a CLI on the same machine as this browser that the login failed, without +// recording the failure: the request isn't proven to come from the browser bound to the login. +func returnErrorToCLI(c *gin.Context, record *db.CLILoginState, errorCode string) { + redirectToCLI(c, record, url.Values{"error": {errorCode}}) } -// failCLILogin ends the login and tells the waiting CLI why. +// failCLILogin ends the login and tells the waiting CLI why. Only call it for the browser bound to the +// login (or one that has just been bound). func (h *AuthHandler) failCLILogin(c *gin.Context, record *db.CLILoginState, errorCode string) { + h.endCLILogin(record, errorCode, c) + returnErrorToCLI(c, record, errorCode) +} + +// endCLILogin records a terminal error on the login and clears the browser's binding cookie. +func (h *AuthHandler) endCLILogin(record *db.CLILoginState, errorCode string, c *gin.Context) { if h.database != nil { _ = h.database.FailCLILoginState(record.State, errorCode) } h.setCLILoginCookie(c, "", -1) - redirectToCLI(c, record, url.Values{"error": {errorCode}}) +} + +// CLILoginCancel godoc +// @Summary Cancel a CLI login +// @Description Ends the CLI login from the browser bound to it and returns the URL that tells the waiting CLI. +// @Tags Auth +// @Accept json +// @Produce json +// @Param request body CLILoginCancelRequest true "Login state" +// @Success 200 {object} CLILoginRedirectResponse +// @Failure 400 {object} ErrorResponse +// @Router /auth/cli/cancel [post] +func (h *AuthHandler) CLILoginCancel(c *gin.Context) { + var req CLILoginCancelRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid_request"}) + return + } + record, errCode := h.loadBoundCLILogin(c, strings.TrimSpace(req.State)) + if errCode != "" { + c.JSON(http.StatusBadRequest, gin.H{"error": errCode}) + return + } + h.endCLILogin(record, "canceled", c) + redirect := cliLoopbackURL(record, url.Values{"error": {"canceled"}}) + c.JSON(http.StatusOK, CLILoginRedirectResponse{Redirect: redirect}) } // CLILoginReturn godoc @@ -205,18 +249,23 @@ func (h *AuthHandler) CLILoginReturn(c *gin.Context) { if !ok { return } - if !record.LoginEmail.Valid || !h.cliMFASatisfied(record) { + if !record.LoginEmail.Valid || !h.cliMFASatisfied(c, record) { c.Redirect(http.StatusFound, cliMFARoute(state)) return } loginCode, err := newCLISecret() if err != nil { - cliRedirectWithError(c, "persist_failure") + h.failCLILogin(c, record, "persist_failure") return } stored, err := h.database.SetCLILoginCode(state, hashCLISecret(loginCode)) - if err != nil || !stored { + if err != nil { + h.failCLILogin(c, record, "persist_failure") + return + } + if !stored { + // A login code was already issued for this login (codes are issued once). cliRedirectWithError(c, "expired") return } @@ -225,9 +274,10 @@ func (h *AuthHandler) CLILoginReturn(c *gin.Context) { } // cliMFASatisfied reports whether the login's MFA requirement is met. A user who had no factor when -// the login reached MFA satisfies it by enrolling one during this login: confirming a new factor -// requires a valid code from it. -func (h *AuthHandler) cliMFASatisfied(record *db.CLILoginState) bool { +// the login reached MFA satisfies it by enrolling one during this login, in this browser: confirming a +// new factor requires a valid code from it, and re-verifies the session that confirmed it (enrolling a +// first factor clears every other session's MFA state). +func (h *AuthHandler) cliMFASatisfied(c *gin.Context, record *db.CLILoginState) bool { if record.MFAVerifiedAt.Valid { return true } @@ -235,23 +285,36 @@ func (h *AuthHandler) cliMFASatisfied(record *db.CLILoginState) bool { return false } user, err := h.database.GetUser(record.LoginEmail.String) - if err != nil || user == nil || !h.enrolledDuringLogin(user, record) { + if err != nil || user == nil { + return false + } + enrolledAt, ok := h.enrolledDuringLogin(user, record) + if !ok { + return false + } + session := h.cliBrowserSession(c, user) + if session == nil || session.MFAVerifiedAt == nil || session.MFAVerifiedAt.Before(enrolledAt) { return false } return h.database.MarkCLILoginVerified(record.State, nil) == nil } // enrolledDuringLogin reports whether every confirmed factor the user has was confirmed after the -// login started (and there is at least one), i.e. the user had none before this login. -func (h *AuthHandler) enrolledDuringLogin(user *db.User, record *db.CLILoginState) bool { +// login started (and there is at least one), i.e. the user had none before this login. It returns +// when the most recent factor was confirmed. +func (h *AuthHandler) enrolledDuringLogin(user *db.User, record *db.CLILoginState) (time.Time, bool) { methods, err := h.database.ListMFAMethods(user.ID) if err != nil || len(methods) == 0 { - return false + return time.Time{}, false } + var latest time.Time for _, method := range methods { if method.ConfirmedAt == nil || method.ConfirmedAt.Before(record.CreatedAt) { - return false + return time.Time{}, false + } + if method.ConfirmedAt.After(latest) { + latest = *method.ConfirmedAt } } - return true + return latest, true } diff --git a/internal/gateway/handlers/auth_cli_loopback_test.go b/internal/gateway/handlers/auth_cli_loopback_test.go index 3584016a..d6a7b493 100644 --- a/internal/gateway/handlers/auth_cli_loopback_test.go +++ b/internal/gateway/handlers/auth_cli_loopback_test.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/base64" "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -47,7 +48,10 @@ func (o *loopbackOAuth) StartLogin() (*auth.LoginStartResponse, error) { func (_ *loopbackOAuth) StartWebLogin() (string, string) { return "", "" } -func (o *loopbackOAuth) CompleteLogin(_, _, _ string) (*auth.LoginResponse, error) { +func (o *loopbackOAuth) CompleteLogin(code, _, _ string) (*auth.LoginResponse, error) { + if code != "google-code" { + return nil, errors.New("invalid authorization code") + } return &auth.LoginResponse{Email: o.email, Name: "Loopback User"}, nil } @@ -79,6 +83,7 @@ func newLoopbackEnv(t *testing.T, requireMFA bool) *loopbackEnv { router.GET("/api/v1/auth/cli/mfa", handler.CLILoginMFAForm) router.POST("/api/v1/auth/cli/mfa", handler.CLILoginMFASubmit) router.GET("/api/v1/auth/cli/return", handler.CLILoginReturn) + router.POST("/api/v1/auth/cli/cancel", handler.CLILoginCancel) router.POST("/api/v1/auth/cli/complete", handler.CLILoginComplete) return &loopbackEnv{router: router, database: database, oauth: oauth, sessions: sessions} } @@ -285,19 +290,148 @@ func TestCLILoginEnrollmentDuringLoginCompletesCLILogin(t *testing.T) { res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding, sessionCookie) require.Equal(t, cliMFARoute(state), res.Header.Get("Location")) - // The user confirms a first factor during this login. + // The user confirms a first factor in this browser during this login: confirming re-verifies the + // session that did it. + e.confirmFirstFactor(t) + e.markSessionVerified(t, sessionCookie) + + res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding, sessionCookie) + require.Equal(t, http.StatusFound, res.StatusCode) + code := loopbackCode(t, res) + require.Equal(t, http.StatusOK, e.complete(t, code, loopbackTestVerifier).StatusCode) +} + +// A first factor enrolled somewhere else (another session) doesn't satisfy the CLI login: someone +// holding the user's Google login can't just wait for the user to enroll. +func TestCLILoginEnrollmentInAnotherSessionDoesNotCompleteCLILogin(t *testing.T) { + e := newLoopbackEnv(t, true) + state, binding := e.startAndBind(t) + res := e.send(t, http.MethodGet, "/api/v1/auth/cli/mfa?state="+state, nil, binding) + sessionCookie := findCookie(res, "session_token") + require.NotNil(t, sessionCookie) + + user, err := e.database.GetUser("user@example.com") + require.NoError(t, err) + otherToken, _, err := e.sessions.CreateSession(user, auth.SessionMetadata{Channel: "web"}) + require.NoError(t, err) + e.confirmFirstFactor(t) + e.markSessionVerified(t, &http.Cookie{Name: "session_token", Value: otherToken}) + + res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding, sessionCookie) + require.Equal(t, cliMFARoute(state), res.Header.Get("Location"), "no login code without enrolling here") +} + +func (e *loopbackEnv) confirmFirstFactor(t *testing.T) { + t.Helper() user, err := e.database.GetUser("user@example.com") require.NoError(t, err) method, err := e.database.CreateMFAMethod(user.ID, "totp", "Authenticator App", "SECRET", nil, nil, nil, nil) require.NoError(t, err) require.NoError(t, e.database.ConfirmMFAMethod(method.ID, time.Now())) +} - res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding, sessionCookie) +func (e *loopbackEnv) markSessionVerified(t *testing.T, sessionCookie *http.Cookie) { + t.Helper() + result, err := e.sessions.ValidateSession(sessionCookie.Value, "", "") + require.NoError(t, err) + require.NoError(t, e.database.UpdateSessionMFAVerified(result.Session.ID, time.Now(), nil)) +} + +// The browser is only bound once the identity provider code exchange has succeeded, so a junk code +// from someone who learned the state can neither bind nor end the login. +func TestCLILoginJunkCodeCannotBindOrBlockLogin(t *testing.T) { + e := newLoopbackEnv(t, false) + res := e.send(t, http.MethodPost, "/api/v1/auth/cli/start", validStartRequest()) + require.Equal(t, http.StatusOK, res.StatusCode) + state := e.oauth.lastState + + res = e.send(t, http.MethodGet, "/api/v1/auth/cli/callback?code=junk&state="+state, nil) + require.Equal(t, "exchange_failed", loopbackError(t, res)) + require.Nil(t, findCookie(res, cliLoginCookie), "a failed exchange must not bind the browser") + + res = e.send(t, http.MethodGet, "/api/v1/auth/cli/callback?code=google-code&state="+state, nil) + require.Equal(t, http.StatusTemporaryRedirect, res.StatusCode) + binding := findCookie(res, cliLoginCookie) + require.NotNil(t, binding) + require.NotEmpty(t, e.loginCodeFromBrowser(t, state, binding)) +} + +// An identity provider error (e.g. the user declined consent) is sent back to the CLI. +func TestCLILoginProviderErrorReturnsToCLI(t *testing.T) { + e := newLoopbackEnv(t, false) + e.send(t, http.MethodPost, "/api/v1/auth/cli/start", validStartRequest()) + state := e.oauth.lastState + + res := e.send(t, http.MethodGet, "/api/v1/auth/cli/callback?error=access_denied&state="+state, nil) + require.Equal(t, "access_denied", loopbackError(t, res)) + record, err := e.database.GetCLILoginState(state) + require.NoError(t, err) + require.False(t, record.LoginError.Valid, "an unauthenticated callback must not end the login") +} + +func TestCLILoginCancelTellsTheCLI(t *testing.T) { + e := newLoopbackEnv(t, false) + state, binding := e.startAndBind(t) + + res := e.send(t, http.MethodPost, "/api/v1/auth/cli/cancel", map[string]string{"state": state}) + require.Equal(t, http.StatusBadRequest, res.StatusCode, "only the bound browser can cancel") + + res = e.send(t, http.MethodPost, "/api/v1/auth/cli/cancel", map[string]string{"state": state}, binding) + require.Equal(t, http.StatusOK, res.StatusCode) + var body CLILoginRedirectResponse + require.NoError(t, json.NewDecoder(res.Body).Decode(&body)) + location, err := url.Parse(body.Redirect) + require.NoError(t, err) + require.Equal(t, "127.0.0.1:54321", location.Host) + require.Equal(t, "canceled", location.Query().Get("error")) + require.Equal(t, validStartRequest().State, location.Query().Get("state")) + + res = e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding) + require.Equal(t, "canceled", loopbackError(t, res), "a canceled login can't be completed") +} + +func loopbackError(t *testing.T, res *http.Response) string { + t.Helper() require.Equal(t, http.StatusFound, res.StatusCode) - code := loopbackCode(t, res) + location, err := url.Parse(res.Header.Get("Location")) + require.NoError(t, err) + require.Equal(t, loopbackTestRedirect, location.Scheme+"://"+location.Host+location.Path) + require.Equal(t, validStartRequest().State, location.Query().Get("state")) + require.Empty(t, location.Query().Get("code")) + return location.Query().Get("error") +} + +// A login code is issued once; returning to /auth/cli/return again doesn't mint another. +func TestCLILoginCodeIsIssuedOnce(t *testing.T) { + e := newLoopbackEnv(t, false) + state, binding := e.startAndBind(t) + code := e.loginCodeFromBrowser(t, state, binding) + + res := e.send(t, http.MethodGet, cliReturnRoute(state), nil, binding) + require.Contains(t, res.Header.Get("Location"), "error=expired") require.Equal(t, http.StatusOK, e.complete(t, code, loopbackTestVerifier).StatusCode) } +func TestCLILoginBindingCookieAttributes(t *testing.T) { + t.Setenv("COOKIE_SECURE", "true") + e := newLoopbackEnv(t, false) + _, binding := e.startAndBind(t) + require.True(t, binding.Secure) + require.True(t, binding.HttpOnly) + require.Equal(t, http.SameSiteLaxMode, binding.SameSite) + require.Equal(t, "/api/v1/auth/cli", binding.Path) +} + +// CLIs from before the loopback login post no login parameters; they're told to upgrade. +func TestCLILoginStartTellsOldCLIsToUpgrade(t *testing.T) { + e := newLoopbackEnv(t, false) + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/cli/start", http.NoBody) + w := httptest.NewRecorder() + e.router.ServeHTTP(w, req) + require.Equal(t, http.StatusBadRequest, w.Code) + require.Contains(t, w.Body.String(), "upgrade the rack-gateway CLI") +} + func TestCLILoginStartValidation(t *testing.T) { e := newLoopbackEnv(t, false) cases := map[string]func(*CLILoginStartRequest){ diff --git a/internal/gateway/handlers/dto.go b/internal/gateway/handlers/dto.go index 4aae1de3..40f9ea54 100644 --- a/internal/gateway/handlers/dto.go +++ b/internal/gateway/handlers/dto.go @@ -228,6 +228,16 @@ type CLILoginStartResponse struct { AuthURL string `json:"auth_url" validate:"required"` } +// CLILoginCancelRequest cancels a CLI login from the browser bound to it. +type CLILoginCancelRequest struct { + State string `json:"state" binding:"required"` +} + +// CLILoginRedirectResponse tells the browser where to go next in a CLI login. +type CLILoginRedirectResponse struct { + Redirect string `json:"redirect" validate:"required"` +} + // CLILoginCompleteRequest represents the payload used to finish the CLI OAuth flow. type CLILoginCompleteRequest struct { LoginCode string `json:"login_code" binding:"required"` diff --git a/internal/gateway/openapi/generated/swagger.json b/internal/gateway/openapi/generated/swagger.json index be6a0be9..ffb6df24 100644 --- a/internal/gateway/openapi/generated/swagger.json +++ b/internal/gateway/openapi/generated/swagger.json @@ -1284,7 +1284,7 @@ }, "/auth/cli/callback": { "get": { - "description": "Stores the authorization code and binds the login to this browser.", + "description": "Exchanges the authorization code and, only once that succeeds, binds the login to this browser.", "tags": [ "Auth" ], @@ -1294,8 +1294,7 @@ "type": "string", "description": "Authorization code", "name": "code", - "in": "query", - "required": true + "in": "query" }, { "type": "string", @@ -1303,6 +1302,12 @@ "name": "state", "in": "query", "required": true + }, + { + "type": "string", + "description": "Identity provider error", + "name": "error", + "in": "query" } ], "responses": { @@ -1315,6 +1320,46 @@ } } }, + "/auth/cli/cancel": { + "post": { + "description": "Ends the CLI login from the browser bound to it and returns the URL that tells the waiting CLI.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Auth" + ], + "summary": "Cancel a CLI login", + "parameters": [ + { + "description": "Login state", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/handlers.CLILoginCancelRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handlers.CLILoginRedirectResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } + } + } + } + }, "/auth/cli/complete": { "post": { "description": "Redeems the single-use login code delivered to the CLI's loopback listener, proving\npossession of the PKCE code verifier, and returns a CLI session token.", @@ -1363,7 +1408,7 @@ }, "/auth/cli/mfa": { "get": { - "description": "Completes the identity provider exchange and sends the bound browser to MFA or back to the CLI.", + "description": "Sends the bound browser to MFA (or MFA enrollment), or back to the CLI when MFA is satisfied.", "tags": [ "Auth" ], @@ -4429,6 +4474,17 @@ } } }, + "handlers.CLILoginCancelRequest": { + "type": "object", + "required": [ + "state" + ], + "properties": { + "state": { + "type": "string" + } + } + }, "handlers.CLILoginCompleteRequest": { "type": "object", "required": [ @@ -4456,6 +4512,17 @@ } } }, + "handlers.CLILoginRedirectResponse": { + "type": "object", + "required": [ + "redirect" + ], + "properties": { + "redirect": { + "type": "string" + } + } + }, "handlers.CLILoginResponse": { "type": "object", "properties": { diff --git a/internal/gateway/routes/mfa_recovery_test.go b/internal/gateway/routes/mfa_recovery_test.go index ae51c2b7..f2cdba7a 100644 --- a/internal/gateway/routes/mfa_recovery_test.go +++ b/internal/gateway/routes/mfa_recovery_test.go @@ -12,8 +12,8 @@ import ( "github.com/DocSpring/rack-gateway/internal/gateway/db" ) -// startCLILogin creates a CLI login whose browser has been bound and whose OAuth exchange has -// completed for the user, as the Google callback leaves it before the MFA challenge. It returns the +// startCLILogin creates a CLI login whose browser has been bound after a successful identity provider +// exchange for the user, as the callback leaves it before the MFA challenge. It returns the // browser-binding cookie value. func (e *mfaRouteEnv) startCLILogin(t *testing.T, state string, user *db.User) string { t.Helper() @@ -29,13 +29,10 @@ func (e *mfaRouteEnv) startCLILogin(t *testing.T, state string, user *db.User) s } binding := "binding-" + state sum := sha256.Sum256([]byte(binding)) - bound, err := e.database.BindCLILoginBrowser(state, "oauth-code", hex.EncodeToString(sum[:])) + bound, err := e.database.BindCLILoginBrowser(state, hex.EncodeToString(sum[:]), user.Email, user.Name) if err != nil || !bound { t.Fatalf("bind CLI login browser: bound=%v err=%v", bound, err) } - if err := e.database.SetCLILoginProfile(state, user.Email, user.Name); err != nil { - t.Fatal(err) - } return binding } diff --git a/internal/gateway/routes/route_registration.go b/internal/gateway/routes/route_registration.go index 5aa96b12..fcc38c6b 100644 --- a/internal/gateway/routes/route_registration.go +++ b/internal/gateway/routes/route_registration.go @@ -26,6 +26,7 @@ func registerAuthRoutes(api *gin.RouterGroup, cfg *Config, h *handlerSet) { authGroup.GET("/auth/cli/mfa", h.auth.CLILoginMFAForm) authGroup.POST("/auth/cli/mfa", h.auth.CLILoginMFASubmit) authGroup.GET("/auth/cli/return", h.auth.CLILoginReturn) + authGroup.POST("/auth/cli/cancel", h.auth.CLILoginCancel) authGroup.GET("/auth/web/login", h.auth.WebLoginStart) authGroup.HEAD("/auth/web/login", h.auth.WebLoginStart) authGroup.GET("/auth/web/callback", h.auth.WebLoginCallback) diff --git a/internal/gateway/security/notifier.go b/internal/gateway/security/notifier.go index 2a18a6e2..792bf4f3 100644 --- a/internal/gateway/security/notifier.go +++ b/internal/gateway/security/notifier.go @@ -151,9 +151,10 @@ func (n *Notifier) shouldSendEmail(recipient, subject, eventType string) bool { } // NewCLISession emails the user when a CLI session is created for their account, so a login they -// did not start is noticed. +// did not start is noticed. Repeats are suppressed per user and client IP (the device name is chosen +// by the client, so it can't decide whether the email is sent). func (n *Notifier) NewCLISession(userEmail, userName, deviceName, ipAddress, userAgent string) { - n.enqueueSecurityNotification(userEmail, "New CLI Login", "cli_session:"+deviceName, jobemail.NewCLISessionArgs{ + n.enqueueSecurityNotification(userEmail, "New CLI Login", "cli_session:"+ipAddress, jobemail.NewCLISessionArgs{ UserEmail: userEmail, UserName: userName, DeviceName: deviceName, diff --git a/web/src/api/generated.ts b/web/src/api/generated.ts index 5d1d7f89..bccccbad 100644 --- a/web/src/api/generated.ts +++ b/web/src/api/generated.ts @@ -6,7 +6,6 @@ * OpenAPI spec version: 1.0 */ import type { - AuthLoginStartResponse, DbAPIToken, DbRackTLSCert, DbSlackIntegration, @@ -25,6 +24,7 @@ import type { GetAuditLogsParams, GetAuthCliCallbackParams, GetAuthCliMfaParams, + GetAuthCliReturnParams, GetAuthWebCallbackParams, GetAuthWebLoginParams, GetCreatedBy200, @@ -38,8 +38,12 @@ import type { GetUsersEmailAuditLogsParams, HandlersAuditLogsResponse, HandlersBackupCodesResponse, + HandlersCLILoginCancelRequest, HandlersCLILoginCompleteRequest, + HandlersCLILoginRedirectResponse, HandlersCLILoginResponse, + HandlersCLILoginStartRequest, + HandlersCLILoginStartResponse, HandlersConfirmTOTPEnrollmentRequest, HandlersConfirmWebAuthnEnrollmentRequest, HandlersCreateAPITokenRequest, @@ -479,8 +483,8 @@ Secrets remain masked unless the user has secrets permissions. }; /** - * Stores the OAuth authorization code for the CLI to finish login. - * @summary Complete CLI OAuth redirect + * Exchanges the authorization code and, only once that succeeds, binds the login to this browser. + * @summary Identity provider redirect for CLI login */ const getAuthCliCallback = ( params: GetAuthCliCallbackParams, @@ -493,9 +497,31 @@ Secrets remain masked unless the user has secrets permissions. }; /** - * Exchanges the stored authorization code and PKCE verifier for a session token. - * @summary Finalize CLI OAuth login + * Ends the CLI login from the browser bound to it and returns the URL that tells the waiting CLI. + * @summary Cancel a CLI login */ + const postAuthCliCancel = ( + handlersCLILoginCancelRequest: HandlersCLILoginCancelRequest, + options?: SecondParameter< + typeof createGatewayClient + >, + ) => { + return createGatewayClient( + { + url: `/auth/cli/cancel`, + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + data: handlersCLILoginCancelRequest, + }, + options, + ); + }; + + /** + * Redeems the single-use login code delivered to the CLI's loopback listener, proving +possession of the PKCE code verifier, and returns a CLI session token. + * @summary Finalize CLI login + */ const postAuthCliComplete = ( handlersCLILoginCompleteRequest: HandlersCLILoginCompleteRequest, options?: SecondParameter< @@ -514,8 +540,8 @@ Secrets remain masked unless the user has secrets permissions. }; /** - * Displays the MFA challenge form for CLI login. - * @summary Display MFA challenge form + * Sends the bound browser to MFA (or MFA enrollment), or back to the CLI when MFA is satisfied. + * @summary Continue CLI login in the browser */ const getAuthCliMfa = ( params: GetAuthCliMfaParams, @@ -528,16 +554,36 @@ Secrets remain masked unless the user has secrets permissions. }; /** - * Initiates the CLI OAuth flow and returns PKCE parameters. - * @summary Start CLI OAuth login + * Issues a single-use login code and redirects the bound browser to the CLI's loopback listener. + * @summary Return the browser to the CLI + */ + const getAuthCliReturn = ( + params: GetAuthCliReturnParams, + options?: SecondParameter>, + ) => { + return createGatewayClient( + { url: `/auth/cli/return`, method: 'GET', params }, + options, + ); + }; + + /** + * Starts a loopback CLI login and returns the identity provider URL to open in the browser. + * @summary Start CLI login */ const postAuthCliStart = ( + handlersCLILoginStartRequest: HandlersCLILoginStartRequest, options?: SecondParameter< - typeof createGatewayClient + typeof createGatewayClient >, ) => { - return createGatewayClient( - { url: `/auth/cli/start`, method: 'POST' }, + return createGatewayClient( + { + url: `/auth/cli/start`, + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + data: handlersCLILoginStartRequest, + }, options, ); }; @@ -1397,8 +1443,10 @@ their own activity; reading another user's activity requires gateway:audit_log:r getAuditLogs, getAuditLogsExport, getAuthCliCallback, + postAuthCliCancel, postAuthCliComplete, getAuthCliMfa, + getAuthCliReturn, postAuthCliStart, postAuthMfaBackupCodesRegenerate, postAuthMfaEnrollTotpConfirm, @@ -1578,6 +1626,9 @@ export type GetAuthCliCallbackResult = NonNullable< ReturnType['getAuthCliCallback']> > >; +export type PostAuthCliCancelResult = NonNullable< + Awaited['postAuthCliCancel']>> +>; export type PostAuthCliCompleteResult = NonNullable< Awaited< ReturnType['postAuthCliComplete']> @@ -1586,6 +1637,9 @@ export type PostAuthCliCompleteResult = NonNullable< export type GetAuthCliMfaResult = NonNullable< Awaited['getAuthCliMfa']>> >; +export type GetAuthCliReturnResult = NonNullable< + Awaited['getAuthCliReturn']>> +>; export type PostAuthCliStartResult = NonNullable< Awaited['postAuthCliStart']>> >; diff --git a/web/src/api/openapi.json b/web/src/api/openapi.json index aaa08e8b..9d1bf403 100644 --- a/web/src/api/openapi.json +++ b/web/src/api/openapi.json @@ -1562,17 +1562,16 @@ }, "/auth/cli/callback": { "get": { - "description": "Stores the OAuth authorization code for the CLI to finish login.", + "description": "Exchanges the authorization code and, only once that succeeds, binds the login to this browser.", "tags": [ "Auth" ], - "summary": "Complete CLI OAuth redirect", + "summary": "Identity provider redirect for CLI login", "parameters": [ { "description": "Authorization code", "name": "code", "in": "query", - "required": true, "schema": { "type": "string" } @@ -1585,6 +1584,14 @@ "schema": { "type": "string" } + }, + { + "description": "Identity provider error", + "name": "error", + "in": "query", + "schema": { + "type": "string" + } } ], "responses": { @@ -1597,13 +1604,45 @@ } } } + } + } + } + }, + "/auth/cli/cancel": { + "post": { + "description": "Ends the CLI login from the browser bound to it and returns the URL that tells the waiting CLI.", + "tags": [ + "Auth" + ], + "summary": "Cancel a CLI login", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.CLILoginCancelRequest" + } + } + }, + "description": "Login state", + "required": true + }, + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.CLILoginRedirectResponse" + } + } + } }, "400": { - "description": "Missing parameters", + "description": "Bad Request", "content": { - "*/*": { + "application/json": { "schema": { - "type": "string" + "$ref": "#/components/schemas/handlers.ErrorResponse" } } } @@ -1613,11 +1652,11 @@ }, "/auth/cli/complete": { "post": { - "description": "Exchanges the stored authorization code and PKCE verifier for a session token.", + "description": "Redeems the single-use login code delivered to the CLI's loopback listener, proving\npossession of the PKCE code verifier, and returns a CLI session token.", "tags": [ "Auth" ], - "summary": "Finalize CLI OAuth login", + "summary": "Finalize CLI login", "requestBody": { "content": { "application/json": { @@ -1665,11 +1704,11 @@ }, "/auth/cli/mfa": { "get": { - "description": "Displays the MFA challenge form for CLI login.", + "description": "Sends the bound browser to MFA (or MFA enrollment), or back to the CLI when MFA is satisfied.", "tags": [ "Auth" ], - "summary": "Display MFA challenge form", + "summary": "Continue CLI login in the browser", "parameters": [ { "description": "State", @@ -1691,9 +1730,31 @@ } } } - }, - "400": { - "description": "Missing parameters", + } + } + } + }, + "/auth/cli/return": { + "get": { + "description": "Issues a single-use login code and redirects the bound browser to the CLI's loopback listener.", + "tags": [ + "Auth" + ], + "summary": "Return the browser to the CLI", + "parameters": [ + { + "description": "Login state", + "name": "state", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "302": { + "description": "Found", "content": { "*/*": { "schema": { @@ -1707,18 +1768,39 @@ }, "/auth/cli/start": { "post": { - "description": "Initiates the CLI OAuth flow and returns PKCE parameters.", + "description": "Starts a loopback CLI login and returns the identity provider URL to open in the browser.", "tags": [ "Auth" ], - "summary": "Start CLI OAuth login", + "summary": "Start CLI login", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.CLILoginStartRequest" + } + } + }, + "description": "CLI login parameters", + "required": true + }, "responses": { "200": { "description": "OK", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/auth.LoginStartResponse" + "$ref": "#/components/schemas/handlers.CLILoginStartResponse" + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.ErrorResponse" } } } @@ -4930,25 +5012,6 @@ } }, "schemas": { - "auth.LoginStartResponse": { - "type": "object", - "required": [ - "auth_url", - "code_verifier", - "state" - ], - "properties": { - "auth_url": { - "type": "string" - }, - "code_verifier": { - "type": "string" - }, - "state": { - "type": "string" - } - } - }, "db.APIToken": { "type": "object", "properties": { @@ -5354,11 +5417,22 @@ } } }, + "handlers.CLILoginCancelRequest": { + "type": "object", + "required": [ + "state" + ], + "properties": { + "state": { + "type": "string" + } + } + }, "handlers.CLILoginCompleteRequest": { "type": "object", "required": [ "code_verifier", - "state" + "login_code" ], "properties": { "client_version": { @@ -5376,7 +5450,18 @@ "device_os": { "type": "string" }, - "state": { + "login_code": { + "type": "string" + } + } + }, + "handlers.CLILoginRedirectResponse": { + "type": "object", + "required": [ + "redirect" + ], + "properties": { + "redirect": { "type": "string" } } @@ -5419,6 +5504,43 @@ } } }, + "handlers.CLILoginStartRequest": { + "type": "object", + "required": [ + "code_challenge", + "code_challenge_method", + "redirect_uri", + "state" + ], + "properties": { + "code_challenge": { + "type": "string" + }, + "code_challenge_method": { + "type": "string" + }, + "device_name": { + "type": "string" + }, + "redirect_uri": { + "type": "string" + }, + "state": { + "type": "string" + } + } + }, + "handlers.CLILoginStartResponse": { + "type": "object", + "required": [ + "auth_url" + ], + "properties": { + "auth_url": { + "type": "string" + } + } + }, "handlers.ConfirmTOTPEnrollmentRequest": { "type": "object", "required": [ diff --git a/web/src/api/schemas/getAuthCliCallbackParams.ts b/web/src/api/schemas/getAuthCliCallbackParams.ts index 34289354..e299a31f 100644 --- a/web/src/api/schemas/getAuthCliCallbackParams.ts +++ b/web/src/api/schemas/getAuthCliCallbackParams.ts @@ -10,9 +10,13 @@ export type GetAuthCliCallbackParams = { /** * Authorization code */ - code: string; + code?: string; /** * State */ state: string; + /** + * Identity provider error + */ + error?: string; }; diff --git a/web/src/api/schemas/getAuthCliReturnParams.ts b/web/src/api/schemas/getAuthCliReturnParams.ts new file mode 100644 index 00000000..469326ed --- /dev/null +++ b/web/src/api/schemas/getAuthCliReturnParams.ts @@ -0,0 +1,14 @@ +/** + * Generated by orval v7.15.0 🍺 + * Do not edit manually. + * Rack Gateway API + * API for the Rack Gateway administration and proxy services. + * OpenAPI spec version: 1.0 + */ + +export type GetAuthCliReturnParams = { + /** + * Login state + */ + state: string; +}; diff --git a/web/src/api/schemas/handlersCLILoginCancelRequest.ts b/web/src/api/schemas/handlersCLILoginCancelRequest.ts new file mode 100644 index 00000000..13cf22fb --- /dev/null +++ b/web/src/api/schemas/handlersCLILoginCancelRequest.ts @@ -0,0 +1,11 @@ +/** + * Generated by orval v7.15.0 🍺 + * Do not edit manually. + * Rack Gateway API + * API for the Rack Gateway administration and proxy services. + * OpenAPI spec version: 1.0 + */ + +export interface HandlersCLILoginCancelRequest { + state: string; +} diff --git a/web/src/api/schemas/handlersCLILoginCompleteRequest.ts b/web/src/api/schemas/handlersCLILoginCompleteRequest.ts index 705b0769..c58b0693 100644 --- a/web/src/api/schemas/handlersCLILoginCompleteRequest.ts +++ b/web/src/api/schemas/handlersCLILoginCompleteRequest.ts @@ -12,5 +12,5 @@ export interface HandlersCLILoginCompleteRequest { device_id?: string; device_name?: string; device_os?: string; - state: string; + login_code: string; } diff --git a/web/src/api/schemas/handlersCLILoginRedirectResponse.ts b/web/src/api/schemas/handlersCLILoginRedirectResponse.ts new file mode 100644 index 00000000..08b8aed4 --- /dev/null +++ b/web/src/api/schemas/handlersCLILoginRedirectResponse.ts @@ -0,0 +1,11 @@ +/** + * Generated by orval v7.15.0 🍺 + * Do not edit manually. + * Rack Gateway API + * API for the Rack Gateway administration and proxy services. + * OpenAPI spec version: 1.0 + */ + +export interface HandlersCLILoginRedirectResponse { + redirect: string; +} diff --git a/web/src/api/schemas/handlersCLILoginStartRequest.ts b/web/src/api/schemas/handlersCLILoginStartRequest.ts new file mode 100644 index 00000000..a9891be0 --- /dev/null +++ b/web/src/api/schemas/handlersCLILoginStartRequest.ts @@ -0,0 +1,15 @@ +/** + * Generated by orval v7.15.0 🍺 + * Do not edit manually. + * Rack Gateway API + * API for the Rack Gateway administration and proxy services. + * OpenAPI spec version: 1.0 + */ + +export interface HandlersCLILoginStartRequest { + code_challenge: string; + code_challenge_method: string; + device_name?: string; + redirect_uri: string; + state: string; +} diff --git a/web/src/api/schemas/handlersCLILoginStartResponse.ts b/web/src/api/schemas/handlersCLILoginStartResponse.ts new file mode 100644 index 00000000..6fbf0212 --- /dev/null +++ b/web/src/api/schemas/handlersCLILoginStartResponse.ts @@ -0,0 +1,11 @@ +/** + * Generated by orval v7.15.0 🍺 + * Do not edit manually. + * Rack Gateway API + * API for the Rack Gateway administration and proxy services. + * OpenAPI spec version: 1.0 + */ + +export interface HandlersCLILoginStartResponse { + auth_url: string; +} diff --git a/web/src/api/schemas/index.ts b/web/src/api/schemas/index.ts index 921b48fa..b2131561 100644 --- a/web/src/api/schemas/index.ts +++ b/web/src/api/schemas/index.ts @@ -37,6 +37,7 @@ export * from './getAuditLogsExportParams'; export * from './getAuditLogsParams'; export * from './getAuthCliCallbackParams'; export * from './getAuthCliMfaParams'; +export * from './getAuthCliReturnParams'; export * from './getAuthWebCallbackParams'; export * from './getAuthWebLoginParams'; export * from './getCreatedBy200'; @@ -52,8 +53,12 @@ export * from './getSettings200'; export * from './getUsersEmailAuditLogsParams'; export * from './handlersAuditLogsResponse'; export * from './handlersBackupCodesResponse'; +export * from './handlersCLILoginCancelRequest'; export * from './handlersCLILoginCompleteRequest'; +export * from './handlersCLILoginRedirectResponse'; export * from './handlersCLILoginResponse'; +export * from './handlersCLILoginStartRequest'; +export * from './handlersCLILoginStartResponse'; export * from './handlersConfirmTOTPEnrollmentRequest'; export * from './handlersConfirmWebAuthnEnrollmentRequest'; export * from './handlersCreateAPITokenRequest'; diff --git a/web/src/api/types.generated.ts b/web/src/api/types.generated.ts index 11a8003b..4c500fa7 100644 --- a/web/src/api/types.generated.ts +++ b/web/src/api/types.generated.ts @@ -1285,16 +1285,18 @@ export interface paths { cookie?: never; }; /** - * Complete CLI OAuth redirect - * @description Stores the OAuth authorization code for the CLI to finish login. + * Identity provider redirect for CLI login + * @description Exchanges the authorization code and, only once that succeeds, binds the login to this browser. */ get: { parameters: { query: { /** @description Authorization code */ - code: string; + code?: string; /** @description State */ state: string; + /** @description Identity provider error */ + error?: string; }; header?: never; path?: never; @@ -1311,19 +1313,63 @@ export interface paths { "*/*": string; }; }; - /** @description Missing parameters */ + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/auth/cli/cancel": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** + * Cancel a CLI login + * @description Ends the CLI login from the browser bound to it and returns the URL that tells the waiting CLI. + */ + post: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** @description Login state */ + requestBody: { + content: { + "application/json": components["schemas"]["handlers.CLILoginCancelRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.CLILoginRedirectResponse"]; + }; + }; + /** @description Bad Request */ 400: { headers: { [name: string]: unknown; }; content: { - "*/*": string; + "application/json": components["schemas"]["handlers.ErrorResponse"]; }; }; }; }; - put?: never; - post?: never; delete?: never; options?: never; head?: never; @@ -1340,8 +1386,9 @@ export interface paths { get?: never; put?: never; /** - * Finalize CLI OAuth login - * @description Exchanges the stored authorization code and PKCE verifier for a session token. + * Finalize CLI login + * @description Redeems the single-use login code delivered to the CLI's loopback listener, proving + * possession of the PKCE code verifier, and returns a CLI session token. */ post: { parameters: { @@ -1400,8 +1447,8 @@ export interface paths { cookie?: never; }; /** - * Display MFA challenge form - * @description Displays the MFA challenge form for CLI login. + * Continue CLI login in the browser + * @description Sends the bound browser to MFA (or MFA enrollment), or back to the CLI when MFA is satisfied. */ get: { parameters: { @@ -1424,8 +1471,41 @@ export interface paths { "*/*": string; }; }; - /** @description Missing parameters */ - 400: { + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/auth/cli/return": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Return the browser to the CLI + * @description Issues a single-use login code and redirects the bound browser to the CLI's loopback listener. + */ + get: { + parameters: { + query: { + /** @description Login state */ + state: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Found */ + 302: { headers: { [name: string]: unknown; }; @@ -1453,8 +1533,8 @@ export interface paths { get?: never; put?: never; /** - * Start CLI OAuth login - * @description Initiates the CLI OAuth flow and returns PKCE parameters. + * Start CLI login + * @description Starts a loopback CLI login and returns the identity provider URL to open in the browser. */ post: { parameters: { @@ -1463,7 +1543,12 @@ export interface paths { path?: never; cookie?: never; }; - requestBody?: never; + /** @description CLI login parameters */ + requestBody: { + content: { + "application/json": components["schemas"]["handlers.CLILoginStartRequest"]; + }; + }; responses: { /** @description OK */ 200: { @@ -1471,7 +1556,16 @@ export interface paths { [name: string]: unknown; }; content: { - "application/json": components["schemas"]["auth.LoginStartResponse"]; + "application/json": components["schemas"]["handlers.CLILoginStartResponse"]; + }; + }; + /** @description Bad Request */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.ErrorResponse"]; }; }; /** @description Internal Server Error */ @@ -4623,11 +4717,6 @@ export interface paths { export type webhooks = Record; export interface components { schemas: { - "auth.LoginStartResponse": { - auth_url: string; - code_verifier: string; - state: string; - }; "db.APIToken": { created_at?: string; created_by_email?: string; @@ -4761,13 +4850,19 @@ export interface components { "handlers.BackupCodesResponse": { backup_codes: string[]; }; + "handlers.CLILoginCancelRequest": { + state: string; + }; "handlers.CLILoginCompleteRequest": { client_version?: string; code_verifier: string; device_id?: string; device_name?: string; device_os?: string; - state: string; + login_code: string; + }; + "handlers.CLILoginRedirectResponse": { + redirect: string; }; "handlers.CLILoginResponse": { channel?: string; @@ -4782,6 +4877,16 @@ export interface components { session_id?: number; token?: string; }; + "handlers.CLILoginStartRequest": { + code_challenge: string; + code_challenge_method: string; + device_name?: string; + redirect_uri: string; + state: string; + }; + "handlers.CLILoginStartResponse": { + auth_url: string; + }; "handlers.ConfirmTOTPEnrollmentRequest": { code: string; label?: string; diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 2b0867a3..a4fa710b 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -307,6 +307,15 @@ export const confirmWebAuthnEnrollment = ( ): Promise => post('/auth/mfa/enroll/webauthn/confirm', payload) +// Cancels a CLI login from the browser bound to it; the response says where to send the browser so the +// waiting CLI hears about it. +export const cancelCliLogin = (payload: { state: string }): Promise<{ redirect: string }> => + getHttpClientInstance() + .post<{ redirect: string }>('/auth/cli/cancel', payload, { + headers: { 'Content-Type': 'application/json' }, + }) + .then((res) => res.data) + export const verifyCliMfa = (payload: { state: string method?: string diff --git a/web/src/pages/account-security/use-account-security-mutations.ts b/web/src/pages/account-security/use-account-security-mutations.ts index 786ca966..5f1ce926 100644 --- a/web/src/pages/account-security/use-account-security-mutations.ts +++ b/web/src/pages/account-security/use-account-security-mutations.ts @@ -29,7 +29,8 @@ import { /** * Handles post-enrollment redirect based on the enrollment channel. - * CLI mode redirects to the CLI success page, web mode redirects to the original destination. + * CLI mode sends the browser to the gateway's CLI return step (which hands it to the waiting CLI), + * web mode redirects to the original destination. */ function handleEnrollmentRedirect( enrollmentChannel: string | undefined, diff --git a/web/src/pages/mfa-challenge-page.test.tsx b/web/src/pages/mfa-challenge-page.test.tsx index 4c169f6a..e2624409 100644 --- a/web/src/pages/mfa-challenge-page.test.tsx +++ b/web/src/pages/mfa-challenge-page.test.tsx @@ -5,11 +5,14 @@ import type { ReactNode } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { MFAChallengePage } from './mfa-challenge-page' -const { verifyCliMfa } = vi.hoisted(() => ({ verifyCliMfa: vi.fn() })) +const { cancelCliLogin, verifyCliMfa } = vi.hoisted(() => ({ + cancelCliLogin: vi.fn(), + verifyCliMfa: vi.fn(), +})) vi.mock('../lib/api', async (importOriginal) => { const actual = await importOriginal() - return { ...actual, verifyCliMfa } + return { ...actual, cancelCliLogin, verifyCliMfa } }) vi.mock('../components/mfa-verification-form', () => ({ @@ -74,6 +77,17 @@ describe('MFAChallengePage CLI login', () => { expect(verifyCliMfa).toHaveBeenCalledWith({ state: 'gateway-state', code: '123456' }) }) + it('cancels the login on the gateway and hands the browser back to the CLI', async () => { + const loopback = 'http://127.0.0.1:54321/callback?error=cancelled&state=cli-state' + cancelCliLogin.mockResolvedValue({ redirect: loopback }) + renderPage('?state=gateway-state') + + await userEvent.click(screen.getByRole('button', { name: 'Cancel Login' })) + + await waitFor(() => expect(assign).toHaveBeenCalledWith(loopback)) + expect(cancelCliLogin).toHaveBeenCalledWith({ state: 'gateway-state' }) + }) + it('shows an error instead of guessing the next step', async () => { verifyCliMfa.mockResolvedValue({ redirect: '' }) renderPage('?state=gateway-state') diff --git a/web/src/pages/mfa-challenge-page.tsx b/web/src/pages/mfa-challenge-page.tsx index 14151bb0..c6d10096 100644 --- a/web/src/pages/mfa-challenge-page.tsx +++ b/web/src/pages/mfa-challenge-page.tsx @@ -6,7 +6,7 @@ import { Alert, AlertDescription } from '@/components/ui/alert' import { Button } from '@/components/ui/button' import { Card, CardContent, CardFooter, CardHeader, CardTitle } from '@/components/ui/card' import { useMutation } from '@/hooks/use-mutation' -import { verifyCliMfa, verifyMFA, verifyWebAuthnAssertion } from '@/lib/api' +import { cancelCliLogin, verifyCliMfa, verifyMFA, verifyWebAuthnAssertion } from '@/lib/api' import { authService } from '@/lib/auth' import { normalizeRedirectPath } from '@/lib/navigation' import { resolveWebRedirect, WebRoute } from '@/lib/routes' @@ -216,7 +216,18 @@ export function MFAChallengePage() { authService.logout() } - const handleCancelCli = () => { + // Ends the login on the gateway and hands the browser to the waiting CLI, which then exits. + const handleCancelCli = async () => { + if (state) { + try { + const result = await cancelCliLogin({ state }) + window.location.assign(cliNextStep(result)) + return + } catch (err) { + setError(mapServerError(mode, err)) + return + } + } if (window.opener) { window.close() return