From 9471a91547382829428c95d327a45daca3088ea9 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 12:16:28 -0400 Subject: [PATCH 1/2] fix(auth): nself login works against production auth_server (P7-PROD-27) Default auth host is https://auth-server.nself.org (NSELF_AUTH_SERVER_URL still overrides, trailing slash trimmed). The device flow uses POST /auth/device-code and POST /auth/device-code/token with the server's fields (verification_uri, access_token, expires_in) and error codes: authorization_pending keeps waiting, slow_down adds 5 s, expired_token and access_denied stop with a remediation. The approved device token carries no identity, so PollDeviceCode reads GET /auth/session and fails the login if that lookup fails instead of storing a blank email or a defaulted tier. /account/devices and /account/team are served by no host: a 404 from either returns E225 (exit 2) instead of a raw 404. Contract tests run against fixtures taken from auth_server route shapes; nothing contacts a real host. E225 gets its owner row and its error-codes.md row. Fixes D-0083. (P7-PROD-27) --- .github/wiki/cmd-login.md | 13 + .github/wiki/error-codes.md | 1 + cmd/commands/account_test.go | 23 +- cmd/commands/login_test.go | 23 +- internal/auth/client.go | 63 +-- internal/auth/client_devices.go | 14 +- internal/auth/client_team.go | 5 +- internal/auth/client_test.go | 112 +---- internal/auth/device_code.go | 173 ++++++- internal/auth/device_code_test.go | 430 ++++++++++++++---- internal/auth/testdata/auth_server/README.md | 19 + .../auth_server/device_code_start.json | 7 + .../device_code_token_access_denied.json | 4 + ...vice_code_token_authorization_pending.json | 4 + .../device_code_token_expired_token.json | 4 + .../auth_server/device_code_token_ok.json | 5 + .../device_code_token_slow_down.json | 4 + .../auth_server/not_found_express.html | 10 + .../auth/testdata/auth_server/session_ok.json | 12 + .../auth_server/session_unauthorized.json | 4 + internal/errs/codes_blocks.go | 1 + internal/errs/codes_login.go | 19 + 22 files changed, 688 insertions(+), 262 deletions(-) create mode 100644 internal/auth/testdata/auth_server/README.md create mode 100644 internal/auth/testdata/auth_server/device_code_start.json create mode 100644 internal/auth/testdata/auth_server/device_code_token_access_denied.json create mode 100644 internal/auth/testdata/auth_server/device_code_token_authorization_pending.json create mode 100644 internal/auth/testdata/auth_server/device_code_token_expired_token.json create mode 100644 internal/auth/testdata/auth_server/device_code_token_ok.json create mode 100644 internal/auth/testdata/auth_server/device_code_token_slow_down.json create mode 100644 internal/auth/testdata/auth_server/not_found_express.html create mode 100644 internal/auth/testdata/auth_server/session_ok.json create mode 100644 internal/auth/testdata/auth_server/session_unauthorized.json create mode 100644 internal/errs/codes_login.go diff --git a/.github/wiki/cmd-login.md b/.github/wiki/cmd-login.md index 6c568ec3a..f40e0d8b1 100644 --- a/.github/wiki/cmd-login.md +++ b/.github/wiki/cmd-login.md @@ -21,6 +21,19 @@ Self-hosted users with no cloud account can skip this step, the CLI operates wit - Accessing `cloud.nself.org` to provision or manage hosted stacks - Syncing license keys from your account - Pushing plugin submissions via `nself plugin submit` + +### Auth host + +The CLI talks to the nSelf auth server at `https://auth-server.nself.org`. Set `NSELF_AUTH_SERVER_URL` to use another host (a local stack or a self-hosted auth server); a trailing slash is ignored. + +### Device flow + +1. The CLI calls `POST /auth/device-code` and prints the login code and the verification URL (`verification_uri`). +2. You open the URL, sign in and approve the code in the browser. +3. The CLI polls `POST /auth/device-code/token` every 5 seconds. `authorization_pending` keeps waiting; `slow_down` adds 5 seconds to the interval; `expired_token` and `access_denied` stop the login with a message (run `nself login` again). +4. On approval the server returns a device token (90 days). The CLI reads your account (`GET /auth/session`) for the email and tier, then stores both in `~/.nself/auth.json`. If that lookup fails, the login fails and nothing is stored. + +The account commands `nself account devices` and `nself account team` call `/account/devices` and `/account/team`, which the auth server does not provide. They fail with `E225` (exit 2) instead of a raw 404. ### Examples diff --git a/.github/wiki/error-codes.md b/.github/wiki/error-codes.md index da7b99b65..2930b5249 100644 --- a/.github/wiki/error-codes.md +++ b/.github/wiki/error-codes.md @@ -171,6 +171,7 @@ These errors occur when the CLI interacts with the PostgreSQL container, backups | E222 | 2 | Backup identity could not be created or read | nSelf could not create, secure or read the age identity under ~/.config/nself (missing age-keygen, unwritable directory, a symlink or a file that is not an age identity). An existing identity is never overwritten. | Install age, fix the directory permissions (0700), or pass --recipient and manage the key yourself. | | E223 | 2 | Backup identity missing for decrypt | Backups encrypted to an auto-created identity can only be decrypted with that identity file. Without it they are unrecoverable. | Restore the identity file from your off-host copy to ~/.config/nself/-age.key, or pass the command's key flag (restore --decrypt-key, restore-remote --key, drill --identity). | | E224 | 2 | No backup recipient and automatic key creation is disabled | NSELF_BACKUP_NO_AUTO_KEY=1 is set, so nSelf will not create an identity, and no recipient is configured. Backups are never written in the clear by default. | Pass --recipient , set BACKUP_AGE_RECIPIENTS, run nself backup init-key, or pass --no-encrypt to write in the clear. | +| E225 | 2 | Server does not provide this path | The server answered 404 for an account path this CLI version calls but the nSelf auth server does not serve. | Use the nself.org account pages for this action, or update the CLI to a version that matches the server. | --- diff --git a/cmd/commands/account_test.go b/cmd/commands/account_test.go index 378f72bb4..a8acb63b0 100644 --- a/cmd/commands/account_test.go +++ b/cmd/commands/account_test.go @@ -105,7 +105,7 @@ func TestAccountLoginCmd_DeviceCodeFlow(t *testing.T) { dcResp := auth.DeviceCodeResponse{ DeviceCode: "device-code-abc", UserCode: "TEST-1234", - VerificationURL: "https://nself.org/auth/cli?code=TEST-1234", + VerificationURL: "https://nself.org/device?code=TEST-1234", ExpiresInSec: 300, IntervalSec: 5, } @@ -120,10 +120,25 @@ func TestAccountLoginCmd_DeviceCodeFlow(t *testing.T) { setAuthServer(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch r.URL.Path { - case "/auth/device/authorize": + case "/auth/device-code": _ = json.NewEncoder(w).Encode(dcResp) - case "/auth/device/token": - _ = json.NewEncoder(w).Encode(tokenResp) + case "/auth/device-code/token": + // auth_server answers an approved poll with the device token only. + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": tokenResp.AccessToken, + "token_type": "Bearer", + "expires_in": 7776000, + }) + case "/auth/session": + // Identity comes from GET /auth/session, not from the poll reply. + _ = json.NewEncoder(w).Encode(map[string]any{ + "authenticated": true, + "account": map[string]any{ + "id": "acct-1", + "email": tokenResp.Email, + "tier": tokenResp.Tier, + }, + }) default: http.NotFound(w, r) } diff --git a/cmd/commands/login_test.go b/cmd/commands/login_test.go index 67bb1f3b5..a2b664935 100644 --- a/cmd/commands/login_test.go +++ b/cmd/commands/login_test.go @@ -96,7 +96,7 @@ func TestLoginCmd_DeviceCodeFlow(t *testing.T) { dcResp := auth.DeviceCodeResponse{ DeviceCode: "test-device-code", UserCode: "TEST-CODE", - VerificationURL: "https://nself.org/auth/cli?code=TEST-CODE", + VerificationURL: "https://nself.org/device?code=TEST-CODE", ExpiresInSec: 300, IntervalSec: 5, } @@ -111,10 +111,25 @@ func TestLoginCmd_DeviceCodeFlow(t *testing.T) { setAuthServer(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch r.URL.Path { - case "/auth/device/authorize": + case "/auth/device-code": _ = json.NewEncoder(w).Encode(dcResp) - case "/auth/device/token": - _ = json.NewEncoder(w).Encode(tokenResp) + case "/auth/device-code/token": + // auth_server answers an approved poll with the device token only. + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": tokenResp.AccessToken, + "token_type": "Bearer", + "expires_in": 7776000, + }) + case "/auth/session": + // Identity comes from GET /auth/session, not from the poll reply. + _ = json.NewEncoder(w).Encode(map[string]any{ + "authenticated": true, + "account": map[string]any{ + "id": "acct-1", + "email": tokenResp.Email, + "tier": tokenResp.Tier, + }, + }) default: http.NotFound(w, r) } diff --git a/internal/auth/client.go b/internal/auth/client.go index 61c285e2e..024ab67c7 100644 --- a/internal/auth/client.go +++ b/internal/auth/client.go @@ -7,16 +7,21 @@ import ( "encoding/json" "fmt" "net/http" + "strings" "time" ) -// AuthServerURL is the base URL of the auth server. -// Overridable via NSELF_AUTH_SERVER_URL for testing. +// DefaultAuthServerURL is the production auth_server host (D-0083). The old +// default, api.nself.org, is the Hasura host and never served the login routes. +const DefaultAuthServerURL = "https://auth-server.nself.org" + +// AuthServerURL is the base URL of the auth server, without a trailing slash. +// Overridable via NSELF_AUTH_SERVER_URL for testing and self-hosted setups. func AuthServerURL() string { if url := getEnv("NSELF_AUTH_SERVER_URL"); url != "" { - return url + return strings.TrimRight(url, "/") } - return "https://api.nself.org" + return DefaultAuthServerURL } // CLIAuthBaseURL is the web UI for the CLI device auth page. @@ -30,8 +35,8 @@ func CLIAuthBaseURL() string { // DeviceCodeResponse is returned by the device authorization endpoint. type DeviceCodeResponse struct { DeviceCode string `json:"device_code"` - UserCode string `json:"user_code"` // XXXX-YYYY format shown to user - VerificationURL string `json:"verification_url"` // nself.org/auth/cli?code=... + UserCode string `json:"user_code"` // 8-character code shown to the user + VerificationURL string `json:"verification_uri"` // page the user opens; auth_server sends it as verification_uri ExpiresInSec int `json:"expires_in"` IntervalSec int `json:"interval"` } @@ -90,12 +95,13 @@ var httpClient = &http.Client{ Timeout: 30 * time.Second, } -// DeviceAuthorize initiates the device code flow. -// Returns a DeviceCodeResponse with the code to display to the user. +// DeviceAuthorize initiates the device code flow with POST /auth/device-code. +// Returns a DeviceCodeResponse with the code to display to the user. A reply +// without a device_code or user_code is an error, never a half-filled value. func DeviceAuthorize(ctx context.Context) (*DeviceCodeResponse, error) { - url := fmt.Sprintf("%s/auth/device/authorize", AuthServerURL()) + url := fmt.Sprintf("%s/auth/device-code", AuthServerURL()) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBufferString(`{}`)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBufferString(`{"client_name":"nself-cli"}`)) if err != nil { return nil, err } @@ -114,42 +120,11 @@ func DeviceAuthorize(ctx context.Context) (*DeviceCodeResponse, error) { if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { return nil, fmt.Errorf("parsing device authorization response: %w", err) } - - return &result, nil -} - -// PollToken polls the auth server for the device code exchange result. -// Returns (nil, nil) if the user hasn't authorized yet (authorization_pending). -// Returns an error on timeout or other failure. -func PollToken(ctx context.Context, deviceCode string) (*TokenResponse, error) { - url := fmt.Sprintf("%s/auth/device/token", AuthServerURL()) - - body, _ := json.Marshal(map[string]string{"device_code": deviceCode}) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(body)) - if err != nil { - return nil, err + if result.DeviceCode == "" || result.UserCode == "" { + return nil, fmt.Errorf("auth server device authorization reply is missing device_code or user_code") } - req.Header.Set("Content-Type", "application/json") - resp, err := httpClient.Do(req) - if err != nil { - return nil, fmt.Errorf("polling token: %w", err) - } - defer func() { _ = resp.Body.Close() }() - // authorization_pending → user hasn't clicked "Authorize" yet - if resp.StatusCode == http.StatusAccepted { - return nil, nil - } - - if resp.StatusCode == http.StatusOK { - var result TokenResponse - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return nil, fmt.Errorf("parsing token response: %w", err) - } - return &result, nil - } - - return nil, parseAPIError(resp) + return &result, nil } // RefreshToken exchanges an existing session for a new access token. diff --git a/internal/auth/client_devices.go b/internal/auth/client_devices.go index a05b82c51..43fea23b3 100644 --- a/internal/auth/client_devices.go +++ b/internal/auth/client_devices.go @@ -5,7 +5,7 @@ package auth // Purpose: list and revoke authorized devices, transfer a license between devices, and parse auth server error responses, used by device-related commands, split out of client.go for file size. // Inputs: an account/session token and, for transfer/revoke, the target device id. // Outputs: DeviceEntry values, or an error from the auth server. -// Constraints: pure move from client.go (CLI-R12 Batch E); no behaviour change. +// Constraints: GetDevices maps a 404 to E225 (auth_server serves no /account/devices route); the rest is a pure move from client.go. import ( "bytes" @@ -15,6 +15,8 @@ import ( "io" "net/http" "os" + + "github.com/nself-org/cli/internal/errs" ) // DeviceEntry represents one registered device for an account. @@ -40,6 +42,9 @@ func GetDevices(ctx context.Context, accessToken string) ([]DeviceEntry, error) return nil, fmt.Errorf("getting devices: %w", err) } defer func() { _ = resp.Body.Close() }() + if resp.StatusCode == http.StatusNotFound { + return nil, errServerLacksPath("/account/devices") + } if resp.StatusCode != http.StatusOK { return nil, parseSessionError(resp) } @@ -112,6 +117,13 @@ func parseSessionError(resp *http.Response) error { return parseAPIError(resp) } +// errServerLacksPath is the E225 error: the auth server answered 404 for a path +// the CLI calls but the server does not provide (never a raw 404, never an +// empty list). The path is the route only, never a token or query. +func errServerLacksPath(path string) error { + return errs.Newf("E225", "this server does not provide %s", path) +} + func parseAPIError(resp *http.Response) error { body, _ := io.ReadAll(resp.Body) var apiErr AuthAPIError diff --git a/internal/auth/client_team.go b/internal/auth/client_team.go index 7c640baf7..74c6fd3e1 100644 --- a/internal/auth/client_team.go +++ b/internal/auth/client_team.go @@ -5,7 +5,7 @@ package auth // Purpose: list, invite, remove and change the role of team members, and activate a license for the account, used by team-related commands, split out of client.go for file size. // Inputs: an account/session token and the target team member's details. // Outputs: TeamMember values, or an error from the auth server. -// Constraints: pure move from client.go (CLI-R12 Batch E); no behaviour change. +// Constraints: GetTeamMembers maps a 404 to E225 (auth_server serves no /account/team route); the rest is a pure move from client.go. import ( "bytes" @@ -38,6 +38,9 @@ func GetTeamMembers(ctx context.Context, accessToken string) ([]TeamMember, erro } defer func() { _ = resp.Body.Close() }() + if resp.StatusCode == http.StatusNotFound { + return nil, errServerLacksPath("/account/team") + } if resp.StatusCode != http.StatusOK { return nil, parseSessionError(resp) } diff --git a/internal/auth/client_test.go b/internal/auth/client_test.go index 8c97ccbf0..d06de0bb5 100644 --- a/internal/auth/client_test.go +++ b/internal/auth/client_test.go @@ -34,35 +34,6 @@ func writeJSON(w http.ResponseWriter, status int, v interface{}) { // ─── DeviceAuthorize ────────────────────────────────────────────────────────── -func TestDeviceAuthorize_Success(t *testing.T) { - want := DeviceCodeResponse{ - DeviceCode: "dev-code-123", - UserCode: "ABCD-EFGH", - VerificationURL: "https://nself.org/auth/cli?code=ABCD-EFGH", - ExpiresInSec: 300, - IntervalSec: 5, - } - cleanup := newAuthServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost || r.URL.Path != "/auth/device/authorize" { - http.NotFound(w, r) - return - } - writeJSON(w, http.StatusOK, want) - }) - defer cleanup() - - got, err := DeviceAuthorize(context.Background()) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got.DeviceCode != want.DeviceCode { - t.Errorf("DeviceCode: got %q, want %q", got.DeviceCode, want.DeviceCode) - } - if got.UserCode != want.UserCode { - t.Errorf("UserCode: got %q, want %q", got.UserCode, want.UserCode) - } -} - func TestDeviceAuthorize_ServerError(t *testing.T) { cleanup := newAuthServer(t, func(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusServiceUnavailable, map[string]string{ @@ -91,87 +62,6 @@ func TestDeviceAuthorize_MalformedJSON(t *testing.T) { } } -// ─── PollToken ──────────────────────────────────────────────────────────────── - -func TestPollToken_Pending(t *testing.T) { - cleanup := newAuthServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/auth/device/token" { - w.WriteHeader(http.StatusAccepted) // authorization_pending - return - } - http.NotFound(w, r) - }) - defer cleanup() - - tok, err := PollToken(context.Background(), "test-device-code") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if tok != nil { - t.Errorf("expected nil token for pending state, got %+v", tok) - } -} - -func TestPollToken_Success(t *testing.T) { - want := TokenResponse{ - AccessToken: "jwt.access.token", - SessionToken: "session-opaque-token", - Email: "user@example.com", - Tier: "basic", - ExpiresAt: "2027-01-01T00:00:00Z", - } - cleanup := newAuthServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/auth/device/token" { - writeJSON(w, http.StatusOK, want) - return - } - http.NotFound(w, r) - }) - defer cleanup() - - tok, err := PollToken(context.Background(), "test-device-code") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if tok == nil { - t.Fatal("expected token, got nil") - } - if tok.Email != want.Email { - t.Errorf("Email: got %q, want %q", tok.Email, want.Email) - } - if tok.Tier != want.Tier { - t.Errorf("Tier: got %q, want %q", tok.Tier, want.Tier) - } -} - -func TestPollToken_Error(t *testing.T) { - cleanup := newAuthServer(t, func(w http.ResponseWriter, r *http.Request) { - writeJSON(w, http.StatusUnauthorized, map[string]string{ - "error": "expired_token", - "message": "device code expired", - }) - }) - defer cleanup() - - _, err := PollToken(context.Background(), "expired-code") - if err == nil { - t.Fatal("expected error, got nil") - } -} - -func TestPollToken_MalformedSuccess(t *testing.T) { - cleanup := newAuthServer(t, func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = fmt.Fprint(w, "{bad json}") - }) - defer cleanup() - - _, err := PollToken(context.Background(), "any") - if err == nil { - t.Fatal("expected parse error, got nil") - } -} - // ─── RefreshToken ───────────────────────────────────────────────────────────── func TestRefreshToken_Success(t *testing.T) { @@ -408,7 +298,7 @@ func TestAuthAPIError_Error(t *testing.T) { func TestAuthServerURL_Default(t *testing.T) { t.Setenv("NSELF_AUTH_SERVER_URL", "") got := AuthServerURL() - if got != "https://api.nself.org" { + if got != "https://auth-server.nself.org" { t.Errorf("default AuthServerURL: got %q", got) } } diff --git a/internal/auth/device_code.go b/internal/auth/device_code.go index c19d66732..2e20af50d 100644 --- a/internal/auth/device_code.go +++ b/internal/auth/device_code.go @@ -1,35 +1,151 @@ // Package auth — device code polling loop for the CLI login flow. +// +// Purpose: poll auth_server's POST /auth/device-code/token until the user +// approves the login in the browser, then look up who logged in. +// Contract source: web backend/services/auth_server/src/routes/device-code.ts +// (fixtures in testdata/auth_server/). package auth import ( + "bytes" "context" + "encoding/json" + "errors" "fmt" + "net/http" "time" ) const ( - // defaultPollInterval is the polling interval between token requests. + // defaultPollInterval is the polling interval between token requests + // (auth_server enforces a 5 s minimum and answers slow_down below it). defaultPollInterval = 5 * time.Second + // slowDownStep is added to the interval each time the server says slow_down. + slowDownStep = 5 * time.Second // defaultPollTimeout is the maximum time to wait for user authorization. defaultPollTimeout = 5 * time.Minute ) +// Test seams: the poll window and the wait between polls. +var ( + pollTimeout = defaultPollTimeout + sleepCtx = sleepContext +) + +// Device-code polling outcomes. PollToken maps the auth_server error codes +// authorization_pending (nil, nil), slow_down, expired_token and access_denied. +var ( + // ErrPollTimeout is returned when the device code polling window expires. + ErrPollTimeout = fmt.Errorf("authorization timed out — the login URL has expired. Run 'nself login' to try again") + // ErrDeviceCodeExpired means the server no longer knows the device code. + ErrDeviceCodeExpired = errors.New("the login code has expired. Run 'nself login' to get a new code") + // ErrAccessDenied means the user denied the login in the browser. + ErrAccessDenied = errors.New("the login was denied in the browser. Run 'nself login' to try again") + // ErrSlowDown means the server asked the client to poll less often. + ErrSlowDown = errors.New("auth server asked the CLI to poll more slowly") +) + // PollResult holds the outcome of a completed device code poll. type PollResult struct { Token *TokenResponse } +// PollOption adjusts PollDeviceCode. +type PollOption func(*pollConfig) + +type pollConfig struct{ interval time.Duration } + +// WithPollInterval sets the starting poll interval (the `interval` the server +// returned from the start call). Values of zero or less keep the default. +func WithPollInterval(d time.Duration) PollOption { + return func(c *pollConfig) { + if d > 0 { + c.interval = d + } + } +} + +// PollToken makes one POST /auth/device-code/token call. +// +// Returns (token, nil) when the user approved; the token carries only the +// access token and its expiry, identity comes from PollDeviceCode. +// Returns (nil, nil) for authorization_pending (and a bare 202). +// Returns ErrSlowDown, ErrDeviceCodeExpired or ErrAccessDenied for the matching +// server error codes, and the server error for anything else. +func PollToken(ctx context.Context, deviceCode string) (*TokenResponse, error) { + url := fmt.Sprintf("%s/auth/device-code/token", AuthServerURL()) + + body, _ := json.Marshal(map[string]string{"device_code": deviceCode}) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(body)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("polling token: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode == http.StatusAccepted { + return nil, nil + } + + if resp.StatusCode == http.StatusOK { + var reply struct { + AccessToken string `json:"access_token"` + ExpiresIn int `json:"expires_in"` + } + if err := json.NewDecoder(resp.Body).Decode(&reply); err != nil { + return nil, fmt.Errorf("parsing token response: %w", err) + } + if reply.AccessToken == "" { + return nil, errors.New("auth server approved the login but sent no access_token") + } + tok := &TokenResponse{AccessToken: reply.AccessToken} + if reply.ExpiresIn > 0 { + tok.ExpiresAt = time.Now().UTC().Add(time.Duration(reply.ExpiresIn) * time.Second).Format(time.RFC3339) + } + return tok, nil + } + + apiErr := parseAPIError(resp) + var ae *AuthAPIError + if errors.As(apiErr, &ae) { + switch ae.Code { + case "authorization_pending": + return nil, nil + case "slow_down": + return nil, ErrSlowDown + case "expired_token": + return nil, ErrDeviceCodeExpired + case "access_denied": + return nil, ErrAccessDenied + } + } + return nil, apiErr +} + // PollDeviceCode polls the auth server until the user authorizes the device code, -// the timeout expires, or the context is cancelled. +// the timeout expires, or the context is cancelled. On approval it looks up the +// account (GET /auth/session) and fills Email, Tier and DisplayName. // // onPoll is called before each poll attempt (use for progress indicators). // Pass nil to skip the callback. // -// Returns ErrPollTimeout if the poll window expires without user action. +// A slow_down reply raises the interval by 5 s for the rest of the flow. +// Returns ErrPollTimeout if the poll window expires without user action, the +// ErrDeviceCodeExpired / ErrAccessDenied outcomes (wrapped), and an error when +// the account lookup after approval fails: a login is never reported as +// successful with a blank identity or a defaulted tier. // Returns context.Canceled/context.DeadlineExceeded if ctx is cancelled. -func PollDeviceCode(ctx context.Context, deviceCode string, onPoll func(elapsed time.Duration)) (*PollResult, error) { - deadline := time.Now().Add(defaultPollTimeout) +func PollDeviceCode(ctx context.Context, deviceCode string, onPoll func(elapsed time.Duration), opts ...PollOption) (*PollResult, error) { + cfg := pollConfig{interval: defaultPollInterval} + for _, o := range opts { + o(&cfg) + } start := time.Now() + deadline := start.Add(pollTimeout) for { // Check context cancellation first. @@ -48,23 +164,48 @@ func PollDeviceCode(ctx context.Context, deviceCode string, onPoll func(elapsed } token, err := PollToken(ctx, deviceCode) - if err != nil { + switch { + case errors.Is(err, ErrSlowDown): + cfg.interval += slowDownStep + case err != nil: return nil, fmt.Errorf("polling for authorization: %w", err) - } - - // nil token means authorization_pending — user hasn't clicked Authorize yet. - if token != nil { + case token != nil: + if err := fillIdentity(ctx, token); err != nil { + return nil, err + } return &PollResult{Token: token}, nil } // Wait for the next poll interval, respecting context cancellation. - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-time.After(defaultPollInterval): + if err := sleepCtx(ctx, cfg.interval); err != nil { + return nil, err } } } -// ErrPollTimeout is returned when the device code polling window expires. -var ErrPollTimeout = fmt.Errorf("authorization timed out — the login URL has expired. Run 'nself login' to try again") +// fillIdentity completes tok with the account the device token belongs to. +func fillIdentity(ctx context.Context, tok *TokenResponse) error { + info, err := GetSession(ctx, tok.AccessToken) + if err != nil { + return fmt.Errorf("login was approved but looking up the account failed: %w", err) + } + if !info.Authenticated || info.Account.Email == "" || info.Account.Tier == "" { + return errors.New("login was approved but the auth server returned no account details; run 'nself login' again") + } + tok.Email = info.Account.Email + tok.Tier = info.Account.Tier + tok.DisplayName = info.Account.DisplayName + return nil +} + +// sleepContext waits d or until ctx is done. +func sleepContext(ctx context.Context, d time.Duration) error { + t := time.NewTimer(d) + defer t.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-t.C: + return nil + } +} diff --git a/internal/auth/device_code_test.go b/internal/auth/device_code_test.go index bd55ef6d1..a90d49812 100644 --- a/internal/auth/device_code_test.go +++ b/internal/auth/device_code_test.go @@ -1,140 +1,408 @@ -// Package auth — tests for device code polling loop. +// Package auth — contract tests for the device code login flow. +// +// The stub server answers with the recorded auth_server bodies in +// testdata/auth_server/ (copied from web routes/device-code.ts and +// session.ts). Nothing here contacts a real host: NSELF_AUTH_SERVER_URL always +// points at an httptest server and the wait between polls is faked. package auth import ( "context" "encoding/json" + "errors" + "io" "net/http" "net/http/httptest" - "sync/atomic" + "os" + "path/filepath" + "strings" + "sync" "testing" "time" + + "github.com/nself-org/cli/internal/errs" ) -// TestPollDeviceCode_Success verifies that a successful authorization returns the token. -func TestPollDeviceCode_Success(t *testing.T) { - var calls int32 - want := TokenResponse{ - AccessToken: "tok-access", - SessionToken: "tok-session", - Email: "user@example.com", - Tier: "pro", +// fixture returns the recorded auth_server body with the given file name. +func fixture(t *testing.T, name string) []byte { + t.Helper() + b, err := os.ReadFile(filepath.Join("testdata", "auth_server", name)) + if err != nil { + t.Fatalf("fixture %s: %v", name, err) } + return b +} - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - n := atomic.AddInt32(&calls, 1) - if r.URL.Path != "/auth/device/token" { - http.NotFound(w, r) +// loginStub is an auth_server stand-in: it serves the device-code start route, +// replays a script of token-poll replies, and serves GET /auth/session. +type loginStub struct { + t *testing.T + mu sync.Mutex + script []string // fixture names for successive POST /auth/device-code/token calls + session string // fixture name served by GET /auth/session + paths []string // "METHOD path" of every request, in order + bodies map[string]string + cookies []string +} + +// newLoginStub starts the stub and points NSELF_AUTH_SERVER_URL at it. +func newLoginStub(t *testing.T, script ...string) *loginStub { + t.Helper() + s := &loginStub{t: t, script: script, session: "session_ok.json", bodies: map[string]string{}} + srv := httptest.NewServer(http.HandlerFunc(s.serve)) + t.Cleanup(srv.Close) + t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + return s +} + +func (s *loginStub) serve(w http.ResponseWriter, r *http.Request) { + s.mu.Lock() + defer s.mu.Unlock() + key := r.Method + " " + r.URL.Path + s.paths = append(s.paths, key) + body, _ := io.ReadAll(r.Body) + s.bodies[key] = string(body) + w.Header().Set("Content-Type", "application/json") + switch key { + case "POST /auth/device-code": + _, _ = w.Write(fixture(s.t, "device_code_start.json")) + case "POST /auth/device-code/token": + if len(s.script) == 0 { + s.t.Errorf("unexpected extra token poll") + w.WriteHeader(http.StatusInternalServerError) return } - // First call: pending; second call: authorized. - if n == 1 { - w.WriteHeader(http.StatusAccepted) // authorization_pending - return + name := s.script[0] + s.script = s.script[1:] + if name != "device_code_token_ok.json" { + w.WriteHeader(http.StatusBadRequest) // auth_server answers every poll error with 400 } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(want) - })) - defer srv.Close() - t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + _, _ = w.Write(fixture(s.t, name)) + case "GET /auth/session": + s.cookies = append(s.cookies, r.Header.Get("Cookie")) + if s.session == "session_unauthorized.json" { + w.WriteHeader(http.StatusUnauthorized) + } + _, _ = w.Write(fixture(s.t, s.session)) + default: + http.NotFound(w, r) + } +} + +// fakeSleep replaces the wait between polls with a recorder and returns the +// durations it was asked to wait. +func fakeSleep(t *testing.T) *[]time.Duration { + t.Helper() + var waits []time.Duration + orig := sleepCtx + sleepCtx = func(ctx context.Context, d time.Duration) error { + waits = append(waits, d) + return ctx.Err() + } + t.Cleanup(func() { sleepCtx = orig }) + return &waits +} + +func TestAuthServerURL_DefaultAndOverride(t *testing.T) { + t.Setenv("NSELF_AUTH_SERVER_URL", "") + if got := AuthServerURL(); got != "https://auth-server.nself.org" { + t.Errorf("default: got %q, want https://auth-server.nself.org", got) + } + t.Setenv("NSELF_AUTH_SERVER_URL", "http://127.0.0.1:9999/") + if got := AuthServerURL(); got != "http://127.0.0.1:9999" { + t.Errorf("override (trailing slash trimmed): got %q", got) + } +} - // Use a very short poll interval override via monkey-patch on defaultPollInterval. - // We can't override the package-level const, so just verify timing is reasonable. - ctx := context.Background() - result, err := PollDeviceCode(ctx, "dev-code", nil) +func TestDeviceAuthorize_Contract(t *testing.T) { + stub := newLoginStub(t) + got, err := DeviceAuthorize(context.Background()) if err != nil { - t.Fatalf("PollDeviceCode returned error: %v", err) + t.Fatalf("DeviceAuthorize: %v", err) + } + var want struct { + DeviceCode string `json:"device_code"` + UserCode string `json:"user_code"` + URI string `json:"verification_uri"` + } + if err := json.Unmarshal(fixture(t, "device_code_start.json"), &want); err != nil { + t.Fatal(err) } - if result == nil || result.Token == nil { - t.Fatal("expected non-nil result") + if got.DeviceCode != want.DeviceCode || got.UserCode != want.UserCode || got.VerificationURL != want.URI { + t.Errorf("fields not decoded from verification_uri/user_code/device_code: %+v", got) } - if result.Token.Email != want.Email { - t.Errorf("email: got %q, want %q", result.Token.Email, want.Email) + if got.ExpiresInSec != 300 || got.IntervalSec != 5 { + t.Errorf("expires_in/interval: got %d/%d, want 300/5", got.ExpiresInSec, got.IntervalSec) } - if result.Token.Tier != want.Tier { - t.Errorf("tier: got %q, want %q", result.Token.Tier, want.Tier) + if len(stub.paths) != 1 || stub.paths[0] != "POST /auth/device-code" { + t.Errorf("paths = %v, want only POST /auth/device-code", stub.paths) + } + if !strings.Contains(stub.bodies["POST /auth/device-code"], `"client_name"`) { + t.Errorf("start body should name the client: %q", stub.bodies["POST /auth/device-code"]) } } -// TestPollDeviceCode_ContextCancelled verifies early exit on context cancellation. -func TestPollDeviceCode_ContextCancelled(t *testing.T) { +func TestDeviceAuthorize_MissingCodesIsError(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Always pending — should never complete. - w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"verification_uri":"https://nself.org/device"}`)) })) defer srv.Close() t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + if _, err := DeviceAuthorize(context.Background()); err == nil { + t.Fatal("a reply without device_code/user_code must be an error") + } +} - ctx, cancel := context.WithCancel(context.Background()) - // Cancel immediately after a tiny delay so PollDeviceCode enters the loop once. - go func() { - time.Sleep(100 * time.Millisecond) - cancel() - }() +func TestPollDeviceCode_PendingThenApproved(t *testing.T) { + stub := newLoginStub(t, "device_code_token_authorization_pending.json", + "device_code_token_authorization_pending.json", "device_code_token_ok.json") + waits := fakeSleep(t) + + var polls int + res, err := PollDeviceCode(context.Background(), "dc-1", func(time.Duration) { polls++ }) + if err != nil { + t.Fatalf("PollDeviceCode: %v", err) + } + var ok struct { + AccessToken string `json:"access_token"` + } + _ = json.Unmarshal(fixture(t, "device_code_token_ok.json"), &ok) + if res.Token.AccessToken != ok.AccessToken { + t.Errorf("AccessToken not stored from access_token") + } + if res.Token.Email != "owner@example.com" || res.Token.Tier != "plus" || res.Token.DisplayName != "Test Owner" { + t.Errorf("identity not filled from /auth/session: %+v", res.Token) + } + if exp, perr := time.Parse(time.RFC3339, res.Token.ExpiresAt); perr != nil || time.Until(exp) < 89*24*time.Hour { + t.Errorf("ExpiresAt %q should be about 90 days out (expires_in)", res.Token.ExpiresAt) + } + if polls != 3 || len(*waits) != 2 || (*waits)[0] != 5*time.Second || (*waits)[1] != 5*time.Second { + t.Errorf("polls=%d waits=%v, want 3 polls and two 5s waits", polls, *waits) + } + if strings.Contains(strings.Join(stub.paths, ","), "/auth/device/") { + t.Errorf("legacy device paths must not be used: %v", stub.paths) + } + if !strings.Contains(stub.bodies["POST /auth/device-code/token"], `"device_code":"dc-1"`) { + t.Errorf("poll body: %q", stub.bodies["POST /auth/device-code/token"]) + } + if len(stub.cookies) != 1 || stub.cookies[0] != "nself_auth_token="+ok.AccessToken { + t.Errorf("session lookup must send the device token as the auth cookie") + } +} - _, err := PollDeviceCode(ctx, "dev-code", nil) - if err == nil { - t.Fatal("expected error on context cancellation, got nil") +func TestPollDeviceCode_SlowDownAddsFiveSeconds(t *testing.T) { + newLoginStub(t, "device_code_token_slow_down.json", "device_code_token_authorization_pending.json", + "device_code_token_slow_down.json", "device_code_token_ok.json") + waits := fakeSleep(t) + if _, err := PollDeviceCode(context.Background(), "dc", nil); err != nil { + t.Fatalf("PollDeviceCode: %v", err) + } + want := []time.Duration{10 * time.Second, 10 * time.Second, 15 * time.Second} + if len(*waits) != len(want) { + t.Fatalf("waits = %v, want %v", *waits, want) + } + for i := range want { + if (*waits)[i] != want[i] { + t.Errorf("wait %d = %v, want %v (slow_down is +5s, kept for the rest of the flow)", i, (*waits)[i], want[i]) + } } } -// TestPollDeviceCode_Timeout verifies ErrPollTimeout is returned when deadline passes. -func TestPollDeviceCode_Timeout(t *testing.T) { - // Reduce timeout for the test by using a context that times out quickly. +func TestPollDeviceCode_StartsAtServerInterval(t *testing.T) { + newLoginStub(t, "device_code_token_authorization_pending.json", "device_code_token_ok.json") + waits := fakeSleep(t) + if _, err := PollDeviceCode(context.Background(), "dc", nil, WithPollInterval(7*time.Second), WithPollInterval(0)); err != nil { + t.Fatalf("PollDeviceCode: %v", err) + } + if len(*waits) != 1 || (*waits)[0] != 7*time.Second { + t.Errorf("waits = %v, want [7s] (zero interval keeps the previous value)", *waits) + } +} + +func TestPollDeviceCode_TerminalErrors(t *testing.T) { + cases := []struct { + fixture string + want error + hint string + }{ + {"device_code_token_expired_token.json", ErrDeviceCodeExpired, "nself login"}, + {"device_code_token_access_denied.json", ErrAccessDenied, "nself login"}, + } + for _, c := range cases { + t.Run(c.fixture, func(t *testing.T) { + newLoginStub(t, c.fixture) + fakeSleep(t) + res, err := PollDeviceCode(context.Background(), "secret-device-code", nil) + if res != nil || !errors.Is(err, c.want) { + t.Fatalf("got (%v, %v), want %v", res, err, c.want) + } + if !strings.Contains(err.Error(), c.hint) { + t.Errorf("error should carry a remediation: %v", err) + } + if strings.Contains(err.Error(), "secret-device-code") { + t.Errorf("error must not echo the device code: %v", err) + } + }) + } +} + +func TestPollDeviceCode_UnknownServerErrorIsReturned(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusAccepted) + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"invalid_request","message":"device_code required"}`)) })) defer srv.Close() t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + fakeSleep(t) + _, err := PollDeviceCode(context.Background(), "dc", nil) + var ae *AuthAPIError + if !errors.As(err, &ae) || ae.Code != "invalid_request" { + t.Fatalf("want the server's invalid_request error, got %v", err) + } +} - // Use a context deadline shorter than defaultPollTimeout (5 min). - ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) - defer cancel() +// A failed identity lookup after approval is an error: the login must not be +// reported with a blank email or a defaulted tier. +func TestPollDeviceCode_IdentityLookupFailureIsError(t *testing.T) { + cases := []struct { + name string + status int + body string + }{ + {"session 401", http.StatusUnauthorized, string(fixture(t, "session_unauthorized.json"))}, + {"session 500", http.StatusInternalServerError, `{"error":"internal_error","message":"boom"}`}, + {"tier missing", http.StatusOK, `{"authenticated":true,"account":{"email":"a@b.c","tier":""}}`}, + {"not authenticated", http.StatusOK, `{"authenticated":false}`}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/auth/session" { + w.WriteHeader(c.status) + _, _ = w.Write([]byte(c.body)) + return + } + _, _ = w.Write(fixture(t, "device_code_token_ok.json")) + })) + defer srv.Close() + t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + fakeSleep(t) + res, err := PollDeviceCode(context.Background(), "dc", nil) + if res != nil || err == nil { + t.Fatalf("want an error and no result, got (%v, %v)", res, err) + } + if strings.Contains(err.Error(), "eyJ") { + t.Errorf("error must not contain the token: %v", err) + } + }) + } +} - _, err := PollDeviceCode(ctx, "dev-code", nil) - if err == nil { - t.Fatal("expected timeout error, got nil") +func TestPollDeviceCode_Timeout(t *testing.T) { + newLoginStub(t) + orig := pollTimeout + pollTimeout = -time.Second + t.Cleanup(func() { pollTimeout = orig }) + if _, err := PollDeviceCode(context.Background(), "dc", nil); !errors.Is(err, ErrPollTimeout) { + t.Fatalf("want ErrPollTimeout, got %v", err) } } -// TestPollDeviceCode_OnPollCallback verifies the callback is invoked. -func TestPollDeviceCode_OnPollCallback(t *testing.T) { - var cbCalls int32 - want := TokenResponse{Email: "cb@example.com", Tier: "free", AccessToken: "tok"} +func TestPollDeviceCode_ContextCancelledWhileWaiting(t *testing.T) { + newLoginStub(t, "device_code_token_authorization_pending.json") + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(50 * time.Millisecond) + cancel() + }() + _, err := PollDeviceCode(ctx, "dc", nil, WithPollInterval(time.Hour)) + if !errors.Is(err, context.Canceled) { + t.Fatalf("want context.Canceled, got %v", err) + } +} +func TestPollToken_ApprovedWithoutAccessTokenIsError(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(want) + _, _ = w.Write([]byte(`{"token_type":"Bearer","expires_in":60}`)) })) defer srv.Close() t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + if tok, err := PollToken(context.Background(), "dc"); err == nil || tok != nil { + t.Fatalf("an approval with no access_token must be an error, got (%v, %v)", tok, err) + } +} - _, err := PollDeviceCode(context.Background(), "dev-code", func(elapsed time.Duration) { - atomic.AddInt32(&cbCalls, 1) - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) +// ─── E225: paths auth_server does not serve ─────────────────────────────────── + +// /account/devices and /account/team are served by no host today. A 404 (the +// Express default HTML page or a JSON body) is E225 with exit class 2, never a +// raw 404 and never an empty list. +func TestUnservedAccountPaths_E225(t *testing.T) { + calls := map[string]func(context.Context, string) error{ + "/account/devices": func(ctx context.Context, tok string) error { _, err := GetDevices(ctx, tok); return err }, + "/account/team": func(ctx context.Context, tok string) error { _, err := GetTeamMembers(ctx, tok); return err }, + } + bodies := map[string]string{ + "express html": string(fixture(t, "not_found_express.html")), + "json": `{"error":"not_found","message":"no such route"}`, } - if atomic.LoadInt32(&cbCalls) == 0 { - t.Error("onPoll callback was never called") + for path, call := range calls { + for bname, body := range bodies { + t.Run(path+" "+bname, func(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + + err := call(context.Background(), "tok-secret") + var ce *errs.CLIError + if !errors.As(err, &ce) || ce.Code != "E225" { + t.Fatalf("want an E225 CLIError, got %v", err) + } + if gotPath != path { + t.Errorf("requested %q, want %q", gotPath, path) + } + msg := err.Error() + if !strings.Contains(msg, "[E225]") || !strings.Contains(msg, "this server does not provide "+path) { + t.Errorf("message: %q", msg) + } + if strings.Contains(msg, "tok-secret") || strings.Contains(msg, "Cannot GET") { + t.Errorf("message leaks the token or the raw 404 body: %q", msg) + } + if got := errs.ExitCodeFor(err); got != 2 { + t.Errorf("exit class = %d, want 2", got) + } + }) + } } } -// TestPollDeviceCode_ServerError verifies propagation of auth server errors. -func TestPollDeviceCode_ServerError(t *testing.T) { +// Only a 404 maps to E225; other failures keep the server's own error, and +// /account/licenses (served by auth_server) is unchanged. +func TestUnservedAccountPaths_OtherStatusesUnchanged(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusUnauthorized) - _ = json.NewEncoder(w).Encode(map[string]string{ - "error": "expired_token", - "message": "device code has expired", - }) + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"not_found","message":"no licences"}`)) })) defer srv.Close() t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) + _, err := GetLicenses(context.Background(), "tok") + var ae *AuthAPIError + if !errors.As(err, &ae) || ae.Status != http.StatusNotFound { + t.Fatalf("GetLicenses 404 should stay an AuthAPIError, got %v", err) + } - _, err := PollDeviceCode(context.Background(), "bad-code", nil) - if err == nil { - t.Fatal("expected error from server, got nil") + srv500 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"internal_error","message":"boom"}`)) + })) + defer srv500.Close() + t.Setenv("NSELF_AUTH_SERVER_URL", srv500.URL) + if _, err := GetDevices(context.Background(), "tok"); errors.As(err, new(*errs.CLIError)) { + t.Errorf("a 500 must not become E225: %v", err) } } diff --git a/internal/auth/testdata/auth_server/README.md b/internal/auth/testdata/auth_server/README.md new file mode 100644 index 000000000..7ac03f970 --- /dev/null +++ b/internal/auth/testdata/auth_server/README.md @@ -0,0 +1,19 @@ +# auth_server contract fixtures + +Response bodies copied from `web:backend/services/auth_server/src/routes/` +(`device-code.ts`, `session.ts`) at web origin/main e72cd3de (P7-PROD-78 merged). +Values are made up; shapes and error codes are the server's. When auth_server +changes a shape, update the fixture from the route source and the CLI test fails +where the client no longer matches. + +| File | Route | Status | +|---|---|---| +| device_code_start.json | POST /auth/device-code | 200 | +| device_code_token_ok.json | POST /auth/device-code/token | 200 | +| device_code_token_authorization_pending.json | POST /auth/device-code/token | 400 | +| device_code_token_slow_down.json | POST /auth/device-code/token | 400 | +| device_code_token_expired_token.json | POST /auth/device-code/token | 400 | +| device_code_token_access_denied.json | POST /auth/device-code/token | 400 | +| session_ok.json | GET /auth/session | 200 | +| session_unauthorized.json | GET /auth/session | 401 | +| not_found_express.html | any unmatched route (`/account/devices`, `/account/team`) | 404 | diff --git a/internal/auth/testdata/auth_server/device_code_start.json b/internal/auth/testdata/auth_server/device_code_start.json new file mode 100644 index 000000000..d3cf02f4a --- /dev/null +++ b/internal/auth/testdata/auth_server/device_code_start.json @@ -0,0 +1,7 @@ +{ + "device_code": "3f9a1c0be2d84f6a9d0c5b7e1a2f4c6d8e0b3a5c7d9f1e2a4b6c8d0e2f4a6b8c", + "user_code": "K7MQ4XZP", + "verification_uri": "https://nself.org/device?code=K7MQ4XZP", + "expires_in": 300, + "interval": 5 +} diff --git a/internal/auth/testdata/auth_server/device_code_token_access_denied.json b/internal/auth/testdata/auth_server/device_code_token_access_denied.json new file mode 100644 index 000000000..834241cea --- /dev/null +++ b/internal/auth/testdata/auth_server/device_code_token_access_denied.json @@ -0,0 +1,4 @@ +{ + "error": "access_denied", + "message": "User denied the authorization request" +} diff --git a/internal/auth/testdata/auth_server/device_code_token_authorization_pending.json b/internal/auth/testdata/auth_server/device_code_token_authorization_pending.json new file mode 100644 index 000000000..ff58fb9c1 --- /dev/null +++ b/internal/auth/testdata/auth_server/device_code_token_authorization_pending.json @@ -0,0 +1,4 @@ +{ + "error": "authorization_pending", + "message": "Waiting for user approval" +} diff --git a/internal/auth/testdata/auth_server/device_code_token_expired_token.json b/internal/auth/testdata/auth_server/device_code_token_expired_token.json new file mode 100644 index 000000000..2fdbb5655 --- /dev/null +++ b/internal/auth/testdata/auth_server/device_code_token_expired_token.json @@ -0,0 +1,4 @@ +{ + "error": "expired_token", + "message": "Device code has expired" +} diff --git a/internal/auth/testdata/auth_server/device_code_token_ok.json b/internal/auth/testdata/auth_server/device_code_token_ok.json new file mode 100644 index 000000000..17ccb3a8b --- /dev/null +++ b/internal/auth/testdata/auth_server/device_code_token_ok.json @@ -0,0 +1,5 @@ +{ + "access_token": "eyJhbGciOiJFZERTQSIsImtpZCI6InRlc3Qta2lkIn0.eyJ0b2tlbl91c2UiOiJkZXZpY2UifQ.c2lnbmF0dXJl", + "token_type": "Bearer", + "expires_in": 7776000 +} diff --git a/internal/auth/testdata/auth_server/device_code_token_slow_down.json b/internal/auth/testdata/auth_server/device_code_token_slow_down.json new file mode 100644 index 000000000..bc9a16e3f --- /dev/null +++ b/internal/auth/testdata/auth_server/device_code_token_slow_down.json @@ -0,0 +1,4 @@ +{ + "error": "slow_down", + "message": "Polling too frequently — wait 5s" +} diff --git a/internal/auth/testdata/auth_server/not_found_express.html b/internal/auth/testdata/auth_server/not_found_express.html new file mode 100644 index 000000000..f950fc180 --- /dev/null +++ b/internal/auth/testdata/auth_server/not_found_express.html @@ -0,0 +1,10 @@ + + + + +Error + + +
Cannot GET /account/devices
+ + diff --git a/internal/auth/testdata/auth_server/session_ok.json b/internal/auth/testdata/auth_server/session_ok.json new file mode 100644 index 000000000..bd030328c --- /dev/null +++ b/internal/auth/testdata/auth_server/session_ok.json @@ -0,0 +1,12 @@ +{ + "authenticated": true, + "account": { + "id": "0b5f3c52-7d1e-4a52-9c63-2f6d1a7e9b10", + "email": "owner@example.com", + "display_name": "Test Owner", + "avatar_url": null, + "tier": "plus", + "email_verified": true, + "mfa_enabled": false + } +} diff --git a/internal/auth/testdata/auth_server/session_unauthorized.json b/internal/auth/testdata/auth_server/session_unauthorized.json new file mode 100644 index 000000000..04a747525 --- /dev/null +++ b/internal/auth/testdata/auth_server/session_unauthorized.json @@ -0,0 +1,4 @@ +{ + "authenticated": false, + "reason": "invalid_token" +} diff --git a/internal/errs/codes_blocks.go b/internal/errs/codes_blocks.go index 8a473f799..893ddf413 100644 --- a/internal/errs/codes_blocks.go +++ b/internal/errs/codes_blocks.go @@ -79,6 +79,7 @@ var Owners = []Owner{ {150, 151, "existing", "codes_ssl.go"}, {200, 202, "existing", "codes_database.go"}, {203, 216, "P7-REG-01", "codes_database.go"}, {217, 221, "P7-PROD-07", "codes_backup.go"}, {222, 224, "P7-PROD-08", "codes_backup_key.go"}, + {225, 225, "P7-PROD-27", "codes_login.go"}, {250, 252, "existing+P7-REG-01", "codes_health.go"}, {300, 301, "existing", "codes_init.go"}, {350, 351, "existing", "codes_domain.go"}, {400, 404, "P7-REG-01", "codes_cli.go"}, diff --git a/internal/errs/codes_login.go b/internal/errs/codes_login.go new file mode 100644 index 000000000..aec1fd2cb --- /dev/null +++ b/internal/errs/codes_login.go @@ -0,0 +1,19 @@ +// Registry fragment: login codes. E225 sits in the database block (E200-E249) +// because the P7-PROD Epic allocated it there (D20). Registered from init() +// through Register; see codes.go for the rules and codes_blocks.go for the +// allocation. +package errs + +func init() { + Register( + CodeEntry{ + Code: "E225", + Category: "database", + Summary: "Server does not provide this path", + DefaultWhy: "The server answered 404 for an account path this CLI version calls but the nSelf auth server does not serve.", + DefaultFix: "Use the nself.org account pages for this action, or update the CLI to a version that matches the server.", + DocsPath: "reference/error-codes#e225", + Exit: 2, + }, + ) +} From fcaf1266f3cf562ef45eba112310746e562181ca Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 13:26:44 -0400 Subject: [PATCH 2/2] fix(auth): keep internal/auth off internal/errs; map the unserved-path error to E225 in the command layer (P7-PROD-27) --- cmd/commands/account_test.go | 33 +++++++++++++++++++++++++++++++ cmd/commands/account_transfer.go | 5 +++++ internal/auth/client_devices.go | 18 +++++++++++------ internal/auth/device_code_test.go | 17 ++++++---------- 4 files changed, 56 insertions(+), 17 deletions(-) diff --git a/cmd/commands/account_test.go b/cmd/commands/account_test.go index a8acb63b0..a75f9c49a 100644 --- a/cmd/commands/account_test.go +++ b/cmd/commands/account_test.go @@ -1,7 +1,9 @@ package commands import ( + "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "os" @@ -10,6 +12,7 @@ import ( "testing" "github.com/nself-org/cli/internal/auth" + "github.com/nself-org/cli/internal/errs" ) // writeTestAuthFile writes a minimal auth.json to the isolated home temp dir. @@ -476,3 +479,33 @@ func TestRequireLogin_NotLoggedIn(t *testing.T) { t.Errorf("error should mention 'nself account login', got: %v", err) } } + +// TestHandleAuthError_UnservedAccountPathIsE225 verifies that a 404 from the +// account paths auth_server does not serve reaches the user as E225 with exit +// class 2, never a raw 404, and that the token never appears in the message. +func TestHandleAuthError_UnservedAccountPathIsE225(t *testing.T) { + setAuthServer(t, func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + }) + calls := map[string]func() error{ + "/account/devices": func() error { _, err := auth.GetDevices(context.Background(), "tok-secret"); return err }, + "/account/team": func() error { _, err := auth.GetTeamMembers(context.Background(), "tok-secret"); return err }, + } + for path, call := range calls { + err := handleAuthError(call()) + var ce *errs.CLIError + if !errors.As(err, &ce) || ce.Code != "E225" { + t.Fatalf("%s: want an E225 CLIError, got %v", path, err) + } + msg := err.Error() + if !strings.Contains(msg, "[E225]") || !strings.Contains(msg, "this server does not provide "+path) { + t.Errorf("%s: message %q", path, msg) + } + if strings.Contains(msg, "tok-secret") { + t.Errorf("%s: message leaks the token: %q", path, msg) + } + if got := errs.ExitCodeFor(err); got != 2 { + t.Errorf("%s: exit class = %d, want 2", path, got) + } + } +} diff --git a/cmd/commands/account_transfer.go b/cmd/commands/account_transfer.go index 6effe681a..89c57968c 100644 --- a/cmd/commands/account_transfer.go +++ b/cmd/commands/account_transfer.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/nself-org/cli/internal/auth" + "github.com/nself-org/cli/internal/errs" "github.com/nself-org/cli/internal/ui" "github.com/spf13/cobra" ) @@ -88,6 +89,10 @@ func handleAuthError(err error) error { if errors.Is(err, auth.ErrNotLoggedIn) { return errAccountRelogin() } + var lacks *auth.ServerLacksPathError + if errors.As(err, &lacks) { + return errs.Newf("E225", "this server does not provide %s", lacks.Path) + } if apiErr, ok := err.(*auth.AuthAPIError); ok { switch apiErr.Status { case 403: diff --git a/internal/auth/client_devices.go b/internal/auth/client_devices.go index 43fea23b3..f9e1d972f 100644 --- a/internal/auth/client_devices.go +++ b/internal/auth/client_devices.go @@ -15,8 +15,6 @@ import ( "io" "net/http" "os" - - "github.com/nself-org/cli/internal/errs" ) // DeviceEntry represents one registered device for an account. @@ -117,11 +115,19 @@ func parseSessionError(resp *http.Response) error { return parseAPIError(resp) } -// errServerLacksPath is the E225 error: the auth server answered 404 for a path -// the CLI calls but the server does not provide (never a raw 404, never an -// empty list). The path is the route only, never a token or query. +// ServerLacksPathError means the auth server answered 404 for a path the CLI +// calls but the server does not provide (never a raw 404, never an empty +// list). Path is the route only, never a token or query. The command layer maps +// it to E225 (internal/auth stays free of internal/errs, which keeps the +// package fan-in of errs flat). +type ServerLacksPathError struct{ Path string } + +func (e *ServerLacksPathError) Error() string { + return "this server does not provide " + e.Path +} + func errServerLacksPath(path string) error { - return errs.Newf("E225", "this server does not provide %s", path) + return &ServerLacksPathError{Path: path} } func parseAPIError(resp *http.Response) error { diff --git a/internal/auth/device_code_test.go b/internal/auth/device_code_test.go index a90d49812..13e40ee8e 100644 --- a/internal/auth/device_code_test.go +++ b/internal/auth/device_code_test.go @@ -19,8 +19,6 @@ import ( "sync" "testing" "time" - - "github.com/nself-org/cli/internal/errs" ) // fixture returns the recorded auth_server body with the given file name. @@ -336,7 +334,7 @@ func TestPollToken_ApprovedWithoutAccessTokenIsError(t *testing.T) { // /account/devices and /account/team are served by no host today. A 404 (the // Express default HTML page or a JSON body) is E225 with exit class 2, never a -// raw 404 and never an empty list. +// raw 404 and never an empty list. The command layer turns it into E225 (exit 2). func TestUnservedAccountPaths_E225(t *testing.T) { calls := map[string]func(context.Context, string) error{ "/account/devices": func(ctx context.Context, tok string) error { _, err := GetDevices(ctx, tok); return err }, @@ -359,23 +357,20 @@ func TestUnservedAccountPaths_E225(t *testing.T) { t.Setenv("NSELF_AUTH_SERVER_URL", srv.URL) err := call(context.Background(), "tok-secret") - var ce *errs.CLIError - if !errors.As(err, &ce) || ce.Code != "E225" { - t.Fatalf("want an E225 CLIError, got %v", err) + var lacks *ServerLacksPathError + if !errors.As(err, &lacks) || lacks.Path != path { + t.Fatalf("want a ServerLacksPathError for %s, got %v", path, err) } if gotPath != path { t.Errorf("requested %q, want %q", gotPath, path) } msg := err.Error() - if !strings.Contains(msg, "[E225]") || !strings.Contains(msg, "this server does not provide "+path) { + if !strings.Contains(msg, "this server does not provide "+path) { t.Errorf("message: %q", msg) } if strings.Contains(msg, "tok-secret") || strings.Contains(msg, "Cannot GET") { t.Errorf("message leaks the token or the raw 404 body: %q", msg) } - if got := errs.ExitCodeFor(err); got != 2 { - t.Errorf("exit class = %d, want 2", got) - } }) } } @@ -402,7 +397,7 @@ func TestUnservedAccountPaths_OtherStatusesUnchanged(t *testing.T) { })) defer srv500.Close() t.Setenv("NSELF_AUTH_SERVER_URL", srv500.URL) - if _, err := GetDevices(context.Background(), "tok"); errors.As(err, new(*errs.CLIError)) { + if _, err := GetDevices(context.Background(), "tok"); errors.As(err, new(*ServerLacksPathError)) { t.Errorf("a 500 must not become E225: %v", err) } }