From e55a3bda8743603032ffcc499f2e319c6d06b595 Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Tue, 13 Oct 2026 15:13:48 +0200 Subject: [PATCH 1/3] [TW-7617] feat(cli): add device sign-in for remote login and signup --- CLAUDE.md | 1 + docs/COMMANDS.md | 16 ++ internal/adapters/oauthas/device.go | 67 ++++++++ internal/adapters/oauthas/device_test.go | 65 ++++++++ internal/app/oauthlogin/device.go | 134 ++++++++++++++++ internal/app/oauthlogin/device_test.go | 146 ++++++++++++++++++ internal/app/oauthlogin/service.go | 23 ++- internal/cli/integration/oauth_device_test.go | 63 ++++++++ internal/cli/oauth/device_test.go | 44 ++++++ internal/cli/oauth/login.go | 57 ++++++- internal/domain/oauth.go | 12 +- internal/domain/oauth_device.go | 55 +++++++ internal/domain/oauth_device_test.go | 28 ++++ internal/ports/oauthas.go | 6 + 14 files changed, 699 insertions(+), 18 deletions(-) create mode 100644 internal/adapters/oauthas/device.go create mode 100644 internal/adapters/oauthas/device_test.go create mode 100644 internal/app/oauthlogin/device.go create mode 100644 internal/app/oauthlogin/device_test.go create mode 100644 internal/cli/integration/oauth_device_test.go create mode 100644 internal/cli/oauth/device_test.go create mode 100644 internal/domain/oauth_device.go create mode 100644 internal/domain/oauth_device_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 76ffbc3..50bcb78 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,6 +94,7 @@ Go 1.24+ · Hexagonal architecture (CLI → Port → Adapter) · Cobra CLI · ** - AI: shared helpers in `adapters/ai/base_client.go` ### Framework Gotchas +- Go regexp counted repetitions have a maximum of 1,000; validate larger protocol length bounds explicitly and use a separate charset regex (verified by device authorization tests). - Go integration tests: `acquireRateLimit(t)` before API calls in parallel tests — omitting causes flaky 429s ### Environment Fixes diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 85acaec..d7e8e77 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -132,6 +132,8 @@ either it is created in the US. ```bash nylas oauth login # Log in via the browser (authorization code + PKCE) +nylas oauth login --device # Remote/headless sign-in or signup on another device +nylas oauth login --no-browser # Same device flow without a local browser nylas oauth login --scope openid,email nylas oauth login --for mcp # Scopes + resource for `nylas mcp serve --auth oauth` nylas oauth login --region eu # Sign up with a new organization in the EU @@ -149,6 +151,20 @@ a value too large for one keychain item (Windows allows 2560 bytes) is split across several. The ID token is not stored: the CLI neither verifies nor uses it. +Over SSH, or with `--device` / `--no-browser`, sign-in uses the server's +advertised device authorization endpoint instead of a local callback. Open +the printed verification URL on another device and enter the displayed user +code. Approve only a sign-in you started yourself. Signup uses the same page; +if the code expires during email verification, start `nylas oauth login +--device` again. Denial, expiry and cancellation stop polling without an +automatic restart. The CLI follows the advertised polling interval, adds +five seconds after every `slow_down`, and backs off on network failures. +The region hint becomes a regional resource in device mode, fixing the token +audience and the signup region. The device code and tokens are never printed +by this flow. A server that +has device sign-in disabled refuses this mode; local browser sign-in remains +the default outside SSH, including macOS and Windows without `DISPLAY`. + Every CLI process on the machine shares the one stored session. Refreshing is serialised by a lock file, `oauth-session.lock`: in `~/.config/nylas` of your account when the session is in the system keyring (whatever `XDG_CONFIG_HOME` diff --git a/internal/adapters/oauthas/device.go b/internal/adapters/oauthas/device.go new file mode 100644 index 0000000..90e32b0 --- /dev/null +++ b/internal/adapters/oauthas/device.go @@ -0,0 +1,67 @@ +package oauthas + +import ( + "context" + "errors" + "net/url" + "strings" + + "github.com/nylas/cli/internal/domain" +) + +// StartDeviceAuthorization requests a first-party device authorization. +func (c *Client) StartDeviceAuthorization(ctx context.Context, clientID string, scopes []string, resource string) (*domain.OAuthDeviceAuthorization, error) { + metadata, err := c.Metadata(ctx) + if err != nil { + return nil, err + } + if metadata.DeviceAuthorizationEndpoint == "" { + return nil, domain.ErrOAuthMetadata + } + form := url.Values{"client_id": {clientID}, "scope": {strings.Join(scopes, " ")}} + if resource != "" { + form.Set("resource", resource) + } + var result domain.OAuthDeviceAuthorization + if err := c.postForm(ctx, metadata.DeviceAuthorizationEndpoint, form, &result); err != nil { + return nil, safeDeviceError(err) + } + if err := result.ValidateFor(metadata.Issuer); err != nil { + return nil, err + } + return &result, nil +} + +// PollDeviceTokens performs one poll; the application service owns the cadence. +func (c *Client) PollDeviceTokens(ctx context.Context, clientID, deviceCode, resource string) (*domain.OAuthTokens, error) { + metadata, err := c.Metadata(ctx) + if err != nil { + return nil, err + } + form := url.Values{"grant_type": {domain.OAuthDeviceGrant}, "client_id": {clientID}, "device_code": {deviceCode}} + if resource != "" { + form.Set("resource", resource) + } + tokens, err := c.requestTokens(ctx, metadata.TokenEndpoint, form) + if err != nil { + return nil, safeDeviceError(err) + } + return tokens, nil +} + +// Device errors never echo a response body that could contain bearer codes. +func safeDeviceError(err error) error { + if errors.Is(err, domain.ErrNetworkError) { + return domain.ErrNetworkError + } + var oauthErr *domain.OAuthError + if errors.As(err, &oauthErr) { + code := "server_error" + switch oauthErr.Code { + case "authorization_pending", "slow_down", "access_denied", "expired_token", "invalid_grant", "invalid_client", "unauthorized_client", "unsupported_grant_type", "invalid_scope", "invalid_request", "server_error", "temporarily_unavailable": + code = oauthErr.Code + } + return &domain.OAuthError{Code: code, StatusCode: oauthErr.StatusCode} + } + return errors.New("device sign-in failed") +} diff --git a/internal/adapters/oauthas/device_test.go b/internal/adapters/oauthas/device_test.go new file mode 100644 index 0000000..e27999f --- /dev/null +++ b/internal/adapters/oauthas/device_test.go @@ -0,0 +1,65 @@ +//go:build !integration + +package oauthas + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDeviceClient_DiscoveryAndForms(t *testing.T) { + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + mux.HandleFunc(discoveryPath, func(w http.ResponseWriter, _ *http.Request) { + doc := discoveryDocument(server.URL) + doc["device_authorization_endpoint"] = server.URL + "/device-start" + _ = json.NewEncoder(w).Encode(doc) + }) + mux.HandleFunc("/device-start", func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, domain.DefaultOAuthClientID, r.Form.Get("client_id")) + assert.Equal(t, "openid dashboard.session", r.Form.Get("scope")) + assert.Equal(t, "https://mcp.us.nylas.com", r.Form.Get("resource")) + _ = json.NewEncoder(w).Encode(domain.OAuthDeviceAuthorization{DeviceCode: strings.Repeat("a", 32), UserCode: "BCDF-GHJK", VerificationURI: server.URL + "/verify", ExpiresIn: 600, Interval: 5}) + }) + mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, domain.OAuthDeviceGrant, r.Form.Get("grant_type")) + assert.Equal(t, strings.Repeat("a", 32), r.Form.Get("device_code")) + assert.Equal(t, "https://mcp.us.nylas.com", r.Form.Get("resource")) + _ = json.NewEncoder(w).Encode(domain.OAuthTokens{AccessToken: "token", ExpiresIn: 900}) + }) + c := NewClient(server.URL) + auth, err := c.StartDeviceAuthorization(context.Background(), domain.DefaultOAuthClientID, []string{"openid", "dashboard.session"}, "https://mcp.us.nylas.com") + require.NoError(t, err) + tokens, err := c.PollDeviceTokens(context.Background(), domain.DefaultOAuthClientID, auth.DeviceCode, "https://mcp.us.nylas.com") + require.NoError(t, err) + assert.Equal(t, "token", tokens.AccessToken) +} + +func TestDeviceClient_ErrorsNeverEchoResponseSecrets(t *testing.T) { + server := newTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"authorization_pending","error_description":"secret-device-code"}`)) + }) + _, err := NewClient(server.URL).PollDeviceTokens(context.Background(), domain.DefaultOAuthClientID, strings.Repeat("a", 32), "") + require.Error(t, err) + assert.NotContains(t, err.Error(), "secret-device-code") + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "authorization_pending", oauthErr.Code) +} + +func TestMetadata_RejectsCrossHostDeviceEndpoint(t *testing.T) { + metadata := domain.OAuthServerMetadata{Issuer: "https://auth.example.test", AuthorizationEndpoint: "https://auth.example.test/authorize", TokenEndpoint: "https://auth.example.test/token", DeviceAuthorizationEndpoint: "https://other.example.test/device"} + require.ErrorIs(t, metadata.ValidateFor(metadata.Issuer), domain.ErrOAuthMetadata) +} diff --git a/internal/app/oauthlogin/device.go b/internal/app/oauthlogin/device.go new file mode 100644 index 0000000..2640821 --- /dev/null +++ b/internal/app/oauthlogin/device.go @@ -0,0 +1,134 @@ +package oauthlogin + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" +) + +func waitDevicePoll(ctx context.Context, interval time.Duration) error { + timer := time.NewTimer(interval) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + +func (s *Service) loginDevice(ctx context.Context, metadata *domain.OAuthServerMetadata, opts LoginOptions, scopes, dropped []string) (*LoginResult, error) { + // Device requests carry a regional resource rather than a browser-only + // signup hint, so an explicitly configured signup region stays fixed. + if opts.Resource == "" && opts.Region != "" { + resource, err := domain.MCPResourceForRegion(opts.Region) + if err != nil { + return nil, err + } + opts.Resource = resource + } + if s.clientID != domain.DefaultOAuthClientID { + return nil, errors.New("device sign-in requires the built-in CLI client") + } + client, ok := s.client.(ports.OAuthDeviceAuthServerClient) + if !ok || metadata.DeviceAuthorizationEndpoint == "" { + return nil, errors.New("this server does not offer device sign-in") + } + if opts.OnDeviceAuthorization == nil { + return nil, errors.New("device sign-in output is not configured") + } + // Start the deadline before the request: response latency never extends + // the server's advertised device-code lifetime. + started := s.now() + authorization, err := client.StartDeviceAuthorization(ctx, s.clientID, scopes, opts.Resource) + if err != nil { + return nil, err + } + if authorization == nil { + return nil, domain.ErrOAuthMetadata + } + if err := authorization.ValidateFor(metadata.Issuer); err != nil { + return nil, err + } + deadline := started.Add(time.Duration(authorization.ExpiresIn) * time.Second) + pollCtx, cancel := context.WithTimeout(ctx, deadline.Sub(s.now())) + defer cancel() + if err := opts.OnDeviceAuthorization(domain.OAuthDevicePrompt{ + UserCode: authorization.UserCode, VerificationURI: authorization.VerificationURI, + VerificationURIComplete: authorization.VerificationURIComplete, + }); err != nil { + return nil, err + } + interval := time.Duration(authorization.Interval) * time.Second + if interval == 0 { + interval = 5 * time.Second + } + wait := s.waitDevice + if wait == nil { + wait = waitDevicePoll + } + for { + if err := wait(pollCtx, interval); err != nil { + if ctx.Err() != nil { + return nil, ctx.Err() + } + return nil, errors.New("device sign-in expired; run `nylas oauth login --device` again") + } + if !s.now().Before(deadline) { + return nil, errors.New("device sign-in expired; run `nylas oauth login --device` again") + } + tokens, err := client.PollDeviceTokens(pollCtx, s.clientID, authorization.DeviceCode, opts.Resource) + if err != nil { + if ctx.Err() != nil { + return nil, ctx.Err() + } + if pollCtx.Err() != nil { + return nil, errors.New("device sign-in expired; run `nylas oauth login --device` again") + } + var oauthErr *domain.OAuthError + if errors.As(err, &oauthErr) { + switch oauthErr.Code { + case "authorization_pending": + continue + case "slow_down": + interval += 5 * time.Second + continue + case "expired_token": + return nil, errors.New("device sign-in expired; run `nylas oauth login --device` again") + case "access_denied": + return nil, errors.New("device sign-in was denied") + } + } + if errors.Is(err, domain.ErrNetworkError) { + interval = max(interval, min(interval*2, 60*time.Second)) + continue + } + return nil, errors.New("device sign-in failed; start a new sign-in when ready") + } + if tokens == nil || tokens.AccessToken == "" { + return nil, errors.New("device sign-in returned no access token") + } + storeCtx, cancelStore := context.WithTimeout(context.WithoutCancel(ctx), refreshTimeout) + err = s.withSessionLock(storeCtx, func() error { + s.revokeReplacedSession(storeCtx) + if err := s.clearSession(); err != nil { + return err + } + if err := s.saveTokens(metadata.Issuer, opts.Resource, tokens); err != nil { + return errors.Join(err, s.clearSession()) + } + return nil + }) + cancelStore() + if err != nil { + return nil, fmt.Errorf("could not store device sign-in: %w", err) + } + return &LoginResult{Issuer: metadata.Issuer, ClientID: s.clientID, Scope: tokens.Scope, + ExpiresAt: tokens.ExpiresAt, HasRefresh: tokens.RefreshToken != "", Resource: opts.Resource, + DroppedScopes: dropped}, nil + } +} diff --git a/internal/app/oauthlogin/device_test.go b/internal/app/oauthlogin/device_test.go new file mode 100644 index 0000000..fbeeb0d --- /dev/null +++ b/internal/app/oauthlogin/device_test.go @@ -0,0 +1,146 @@ +//go:build !integration + +package oauthlogin + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/nylas/cli/internal/adapters/oauthas" + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type deviceTestClient struct { + *oauthas.MockClient + authorization *domain.OAuthDeviceAuthorization + errors []error + polls int + scopes []string + resource string +} + +func (c *deviceTestClient) StartDeviceAuthorization(_ context.Context, _ string, scopes []string, resource string) (*domain.OAuthDeviceAuthorization, error) { + c.scopes, c.resource = scopes, resource + return c.authorization, nil +} + +func (c *deviceTestClient) PollDeviceTokens(_ context.Context, _, _, _ string) (*domain.OAuthTokens, error) { + c.polls++ + if len(c.errors) > 0 { + err := c.errors[0] + c.errors = c.errors[1:] + return nil, err + } + return &domain.OAuthTokens{AccessToken: "device-access", RefreshToken: "device-refresh", Scope: "openid email offline_access dashboard.session", ExpiresAt: time.Now().Add(time.Minute)}, nil +} + +func deviceFixture(t *testing.T) (*fixture, *deviceTestClient, *LoginOptions, *[]time.Duration) { + t.Helper() + f := newFixture(t) + c := &deviceTestClient{MockClient: f.client, authorization: &domain.OAuthDeviceAuthorization{ + DeviceCode: "device-code-in-memory", UserCode: "BCDF-GHJK", + VerificationURI: oauthas.MockIssuer + "/oauth/device", ExpiresIn: 600, Interval: 5, + }} + c.MetadataFunc = func(context.Context) (*domain.OAuthServerMetadata, error) { + return &domain.OAuthServerMetadata{Issuer: oauthas.MockIssuer, DeviceAuthorizationEndpoint: oauthas.MockIssuer + "/oauth/device_authorization"}, nil + } + f.service.client = c + intervals := []time.Duration{} + f.service.waitDevice = func(ctx context.Context, d time.Duration) error { + intervals = append(intervals, d) + f.advance(d) + return ctx.Err() + } + opts := &LoginOptions{Device: true, OnDeviceAuthorization: func(domain.OAuthDevicePrompt) error { return nil }} + return f, c, opts, &intervals +} + +func TestDeviceLogin_CumulativeSlowDownAndPersistence(t *testing.T) { + f, c, opts, intervals := deviceFixture(t) + c.errors = []error{&domain.OAuthError{Code: "authorization_pending"}, &domain.OAuthError{Code: "slow_down"}, &domain.OAuthError{Code: "slow_down"}, domain.ErrNetworkError} + opts.Resource = "https://mcp.us.nylas.com" + var prompt domain.OAuthDevicePrompt + opts.OnDeviceAuthorization = func(p domain.OAuthDevicePrompt) error { prompt = p; return nil } + result, err := f.service.Login(context.Background(), *opts) + require.NoError(t, err) + assert.Equal(t, []time.Duration{5 * time.Second, 5 * time.Second, 10 * time.Second, 15 * time.Second, 30 * time.Second}, *intervals) + assert.True(t, result.HasRefresh) + assert.Equal(t, "BCDF-GHJK", prompt.UserCode) + assert.Equal(t, "device-access", f.secrets.GetAll()[ports.KeyOAuthAccessToken]) + assert.Equal(t, opts.Resource, f.secrets.GetAll()[ports.KeyOAuthResource]) + assert.Equal(t, domain.DefaultOAuthScopes(), c.scopes) + assert.Empty(t, f.browser.openedURL) + assert.Empty(t, f.client.AuthorizationCalls) +} + +func TestDeviceLogin_TerminalErrorsStopWithoutStoring(t *testing.T) { + for _, code := range []string{"access_denied", "expired_token", "invalid_grant", "unauthorized_client", "server_error"} { + t.Run(code, func(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + c.errors = []error{&domain.OAuthError{Code: code, Description: "must-not-appear"}} + _, err := f.service.Login(context.Background(), *opts) + require.Error(t, err) + assert.NotContains(t, err.Error(), "must-not-appear") + assert.Equal(t, 1, c.polls) + assert.Empty(t, f.secrets.GetAll()) + }) + } +} + +func TestDeviceLogin_ExpiryStopsBeforeNextPoll(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + c.authorization.ExpiresIn = 5 + _, err := f.service.Login(context.Background(), *opts) + require.ErrorContains(t, err, "run `nylas oauth login --device` again") + assert.Zero(t, c.polls) +} + +func TestDeviceLogin_CancellationStopsPolling(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + ctx, cancel := context.WithCancel(context.Background()) + f.service.waitDevice = func(context.Context, time.Duration) error { cancel(); return context.Canceled } + _, err := f.service.Login(ctx, *opts) + require.ErrorIs(t, err, context.Canceled) + assert.Zero(t, c.polls) +} + +func TestDeviceLogin_PromptFailureDoesNotPoll(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + opts.OnDeviceAuthorization = func(domain.OAuthDevicePrompt) error { return errors.New("output unavailable") } + _, err := f.service.Login(context.Background(), *opts) + require.ErrorContains(t, err, "output unavailable") + assert.Zero(t, c.polls) +} + +func TestDeviceLogin_RefusesUnadvertisedAndNonFirstPartyClients(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + f.service.clientID = "different-client" + _, err := f.service.Login(context.Background(), *opts) + require.ErrorContains(t, err, "built-in CLI") + f.service.clientID = domain.DefaultOAuthClientID + c.MetadataFunc = func(context.Context) (*domain.OAuthServerMetadata, error) { return &domain.OAuthServerMetadata{}, nil } + _, err = f.service.Login(context.Background(), *opts) + require.ErrorContains(t, err, "does not offer") + assert.Zero(t, c.polls) +} + +func TestWaitDevicePoll_Cancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + require.ErrorIs(t, waitDevicePoll(ctx, time.Hour), context.Canceled) +} + +func TestDeviceLogin_RegionConstrainsSignupAudience(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + opts.Region = "eu" + result, err := f.service.Login(context.Background(), *opts) + require.NoError(t, err) + assert.Equal(t, domain.MCPResourceEU, c.resource) + assert.Equal(t, domain.MCPResourceEU, result.Resource) + assert.Equal(t, domain.MCPResourceEU, f.secrets.GetAll()[ports.KeyOAuthResource]) +} diff --git a/internal/app/oauthlogin/service.go b/internal/app/oauthlogin/service.go index e403d65..d5fb7c0 100644 --- a/internal/app/oauthlogin/service.go +++ b/internal/app/oauthlogin/service.go @@ -20,13 +20,14 @@ const refreshTimeout = 30 * time.Second // Service performs authorization server logins and owns the stored session. type Service struct { - clientID string - client ports.OAuthAuthServerClient - server ports.OAuthServer - browser ports.Browser - secrets ports.SecretStore - lock ports.CrossProcessLock - now func() time.Time + clientID string + client ports.OAuthAuthServerClient + server ports.OAuthServer + browser ports.Browser + secrets ports.SecretStore + lock ports.CrossProcessLock + now func() time.Time + waitDevice func(context.Context, time.Duration) error } // NewService creates a login service for the given public client id — @@ -92,6 +93,10 @@ type LoginResult struct { // LoginOptions shape an authorization request. type LoginOptions struct { + // Device selects RFC 8628 instead of the loopback browser flow. + Device bool + // OnDeviceAuthorization receives no bearer credential. + OnDeviceAuthorization func(domain.OAuthDevicePrompt) error // Scopes to request; domain.DefaultOAuthScopes when empty. Scopes []string @@ -153,6 +158,10 @@ func (s *Service) Login(ctx context.Context, opts LoginOptions) (*LoginResult, e } } + if opts.Device { + return s.loginDevice(ctx, metadata, opts, scopes, dropped) + } + if err := s.server.Start(); err != nil { return nil, err } diff --git a/internal/cli/integration/oauth_device_test.go b/internal/cli/integration/oauth_device_test.go new file mode 100644 index 0000000..ca9e323 --- /dev/null +++ b/internal/cli/integration/oauth_device_test.go @@ -0,0 +1,63 @@ +//go:build integration + +package integration + +import ( + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The real CLI talks to a fake AS and stores its session in an isolated file +// store. No provider credentials or production services are involved. +func TestCLI_OAuthDevice_FakeServerLogin(t *testing.T) { + require.NotEmpty(t, testBinary, "build the CLI before this integration test") + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + deviceCode := "memory-only-device-code" + var polls atomic.Int32 + payload, err := json.Marshal(map[string]any{"sub": "user", "org": "org", "scope": "openid email", "exp": time.Now().Add(15 * time.Minute).Unix(), "aud": server.URL}) + require.NoError(t, err) + token := "e30." + base64.RawURLEncoding.EncodeToString(payload) + ".signature" + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(domain.OAuthServerMetadata{Issuer: server.URL, AuthorizationEndpoint: server.URL + "/authorize", TokenEndpoint: server.URL + "/token", DeviceAuthorizationEndpoint: server.URL + "/device", ScopesSupported: []string{"openid", "email"}}) + }) + mux.HandleFunc("/device", func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, domain.DefaultOAuthClientID, r.Form.Get("client_id")) + assert.Equal(t, "openid email", r.Form.Get("scope")) + _ = json.NewEncoder(w).Encode(domain.OAuthDeviceAuthorization{DeviceCode: deviceCode, UserCode: "BCDF-GHJK", VerificationURI: server.URL + "/verify", VerificationURIComplete: server.URL + "/verify?user_code=BCDF-GHJK", ExpiresIn: 600, Interval: 1}) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, domain.OAuthDeviceGrant, r.Form.Get("grant_type")) + assert.Equal(t, deviceCode, r.Form.Get("device_code")) + if polls.Add(1) == 1 { + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "authorization_pending"}) + return + } + _ = json.NewEncoder(w).Encode(domain.OAuthTokens{AccessToken: token, TokenType: "Bearer", Scope: "openid email", ExpiresIn: 900}) + }) + env := map[string]string{"NYLAS_DASHBOARD_ACCOUNT_URL": server.URL, "NYLAS_DISABLE_KEYRING": "true", "XDG_CONFIG_HOME": t.TempDir(), "SSH_CONNECTION": "", "SSH_CLIENT": "", "SSH_TTY": "", "NYLAS_OAUTH_CLIENT_ID": ""} + out, stderr, err := runCLIWithOverrides(30*time.Second, env, "oauth", "login", "--device", "--scope", "openid,email") + require.NoError(t, err, stderr) + assert.Contains(t, out, "BCDF-GHJK") + assert.Contains(t, out, "Logged in") + assert.NotContains(t, out+stderr, deviceCode) + assert.NotContains(t, out+stderr, token) + assert.EqualValues(t, 2, polls.Load()) + status, stderr, err := runCLIWithOverrides(10*time.Second, env, "oauth", "status") + require.NoError(t, err, stderr) + assert.Contains(t, status, server.URL) + assert.NotContains(t, status+stderr, token) +} diff --git a/internal/cli/oauth/device_test.go b/internal/cli/oauth/device_test.go new file mode 100644 index 0000000..9fe6e6b --- /dev/null +++ b/internal/cli/oauth/device_test.go @@ -0,0 +1,44 @@ +//go:build !integration + +package oauth + +import ( + "testing" + + "github.com/nylas/cli/internal/app/oauthlogin" + "github.com/nylas/cli/internal/cli/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoginCmd_DeviceSelection(t *testing.T) { + for _, tc := range []struct { + name string + flags []string + ssh string + want bool + }{ + {"device", []string{"--device"}, "", true}, + {"no-browser", []string{"--no-browser"}, "", true}, + {"SSH", nil, "remote", true}, + {"local-no-DISPLAY", nil, "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("SSH_CONNECTION", tc.ssh) + t.Setenv("SSH_CLIENT", "") + t.Setenv("SSH_TTY", "") + t.Setenv("DISPLAY", "") + svc := &fakeService{loginResult: &oauthlogin.LoginResult{Issuer: "https://auth.example.test", ClientID: "client"}} + withService(t, svc) + out, _, err := testutil.ExecuteSubCommand(newLoginCmd(), tc.flags...) + require.NoError(t, err) + require.NotNil(t, svc.loginOpts) + assert.Equal(t, tc.want, svc.loginOpts.Device) + if tc.want { + assert.Contains(t, out, "Using device sign-in") + } else { + assert.Contains(t, out, "Opening your browser") + } + }) + } +} diff --git a/internal/cli/oauth/login.go b/internal/cli/oauth/login.go index cc80544..1e662f1 100644 --- a/internal/cli/oauth/login.go +++ b/internal/cli/oauth/login.go @@ -1,9 +1,13 @@ package oauth import ( + "context" "errors" "fmt" + "os" + "os/signal" "strings" + "time" "github.com/spf13/cobra" @@ -18,15 +22,24 @@ const loginPresetMCP = "mcp" func newLoginCmd() *cobra.Command { var ( - scopes []string - preset string - region string + scopes []string + preset string + region string + device bool + noBrowser bool ) cmd := &cobra.Command{ Use: "login", - Short: "Log in to the Nylas authorization server via the browser", - Long: `Run an OAuth 2.1 authorization code flow with PKCE. + Short: "Log in to Nylas using a browser or device code", + Long: `Run an OAuth 2.1 authorization code flow with PKCE, or device sign-in. + +Use --device or --no-browser over SSH or on a headless machine. The CLI prints +an approval URL and user code, then waits while you sign in or sign up on +another device. SSH sessions select this flow automatically; local macOS and +Windows keep browser sign-in even without DISPLAY. If signup takes too long, +run the command again to get a new code. In device mode, the region also +fixes the regional token audience; signup must use that same region. The CLI is a static public client (no client secret; PKCE proves the exchange). It opens a browser for consent, receives the redirect on @@ -59,6 +72,9 @@ dev authorization server.`, # Log in for 'nylas mcp serve --auth oauth' nylas oauth login --for mcp + # Sign in over SSH without a local browser + nylas oauth login --device + # Sign up with a new organization in the EU nylas oauth login --region eu @@ -79,6 +95,18 @@ dev authorization server.`, return err } + opts.Device = device || noBrowser || isSSHSession() + opts.OnDeviceAuthorization = func(prompt domain.OAuthDevicePrompt) error { + out := cmd.OutOrStdout() + if _, err := fmt.Fprintf(out, "Open %s on a device with a browser.\nEnter code: %s\nOnly continue if you started this sign-in yourself.\n", prompt.VerificationURI, prompt.UserCode); err != nil { + return err + } + if prompt.VerificationURIComplete != "" { + _, err := fmt.Fprintln(out, prompt.VerificationURIComplete) + return err + } + return nil + } svc, err := createLoginServiceFn() if err != nil { return wrapOAuthError(err) @@ -87,9 +115,19 @@ dev authorization server.`, // The user has to read a consent screen, so this needs the long // interactive timeout rather than the API one. ctx, cancel := common.CreateLongContext() + if opts.Device { + cancel() + signalCtx, stopSignals := signal.NotifyContext(cmd.Context(), os.Interrupt) + defer stopSignals() + ctx, cancel = context.WithTimeout(signalCtx, 10*time.Minute) + } defer cancel() - _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Opening your browser to complete sign-in...") + if opts.Device { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Using device sign-in; waiting for approval...") + } else { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Opening your browser to complete sign-in...") + } result, err := svc.Login(ctx, opts) if err != nil { @@ -145,6 +183,8 @@ dev authorization server.`, cmd.Flags().StringVar(®ion, "region", "", "region for an organization created if you sign up: us or eu (default: the configured region)") + cmd.Flags().BoolVar(&device, "device", false, "sign in or sign up using a code on another device") + cmd.Flags().BoolVar(&noBrowser, "no-browser", false, "use device sign-in without opening a local browser") return cmd } @@ -197,3 +237,8 @@ func normalizeRegion(region string) (string, error) { ) } } + +// DISPLAY is deliberately irrelevant: local macOS/Windows normally omit it. +func isSSHSession() bool { + return os.Getenv("SSH_CONNECTION") != "" || os.Getenv("SSH_CLIENT") != "" || os.Getenv("SSH_TTY") != "" +} diff --git a/internal/domain/oauth.go b/internal/domain/oauth.go index 6d37018..06e6cf7 100644 --- a/internal/domain/oauth.go +++ b/internal/domain/oauth.go @@ -67,6 +67,7 @@ func WithDashboardSessionScope(scopes []string) []string { // OAuthServerMetadata is the subset of the RFC 8414 authorization server // metadata document the CLI uses. type OAuthServerMetadata struct { + DeviceAuthorizationEndpoint string `json:"device_authorization_endpoint"` Issuer string `json:"issuer"` AuthorizationEndpoint string `json:"authorization_endpoint"` TokenEndpoint string `json:"token_endpoint"` @@ -134,11 +135,12 @@ func (m *OAuthServerMetadata) ValidateFor(baseURL string) error { return fmt.Errorf("%w: issuer %q does not match %q", ErrOAuthMetadata, m.Issuer, baseURL) } for name, endpoint := range map[string]string{ - "authorization_endpoint": m.AuthorizationEndpoint, - "token_endpoint": m.TokenEndpoint, - "userinfo_endpoint": m.UserInfoEndpoint, - "revocation_endpoint": m.RevocationEndpoint, - "jwks_uri": m.JWKSURI, + "authorization_endpoint": m.AuthorizationEndpoint, + "token_endpoint": m.TokenEndpoint, + "userinfo_endpoint": m.UserInfoEndpoint, + "revocation_endpoint": m.RevocationEndpoint, + "jwks_uri": m.JWKSURI, + "device_authorization_endpoint": m.DeviceAuthorizationEndpoint, } { if endpoint == "" { continue diff --git a/internal/domain/oauth_device.go b/internal/domain/oauth_device.go new file mode 100644 index 0000000..20f7776 --- /dev/null +++ b/internal/domain/oauth_device.go @@ -0,0 +1,55 @@ +package domain + +import ( + "fmt" + "net/url" + "regexp" +) + +const OAuthDeviceGrant = "urn:ietf:params:oauth:grant-type:device_code" + +// OAuthDeviceAuthorization is held in memory only. DeviceCode is a bearer +// credential and must never reach terminal output, diagnostic logs or storage. +type OAuthDeviceAuthorization struct { + DeviceCode string `json:"device_code"` + UserCode string `json:"user_code"` + VerificationURI string `json:"verification_uri"` + VerificationURIComplete string `json:"verification_uri_complete"` + ExpiresIn int `json:"expires_in"` + Interval int `json:"interval"` +} + +// OAuthDevicePrompt contains only the values intended for the user's terminal. +type OAuthDevicePrompt struct { + UserCode string + VerificationURI string + VerificationURIComplete string +} + +var deviceCodePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + +var deviceUserCode = regexp.MustCompile(`^[A-Z0-9]{4}-?[A-Z0-9]{4}$`) + +// ValidateFor rejects malformed values and links outside the discovered issuer. +func (d *OAuthDeviceAuthorization) ValidateFor(issuer string) error { + if len(d.DeviceCode) < 16 || len(d.DeviceCode) > 2048 || !deviceCodePattern.MatchString(d.DeviceCode) || !deviceUserCode.MatchString(d.UserCode) || d.ExpiresIn < 1 || d.ExpiresIn > 600 || d.Interval < 0 || d.Interval > 600 { + return fmt.Errorf("%w: invalid device authorization", ErrOAuthMetadata) + } + base, err := url.Parse(issuer) + if err != nil || base.Host == "" { + return ErrOAuthMetadata + } + for _, raw := range []string{d.VerificationURI, d.VerificationURIComplete} { + if raw == "" { + continue + } + u, err := url.Parse(raw) + if err != nil || len(raw) > 4096 || u.Scheme != base.Scheme || u.Host != base.Host || u.User != nil || u.Fragment != "" { + return fmt.Errorf("%w: invalid device verification link", ErrOAuthMetadata) + } + } + if d.VerificationURI == "" { + return fmt.Errorf("%w: missing device verification link", ErrOAuthMetadata) + } + return nil +} diff --git a/internal/domain/oauth_device_test.go b/internal/domain/oauth_device_test.go new file mode 100644 index 0000000..5541944 --- /dev/null +++ b/internal/domain/oauth_device_test.go @@ -0,0 +1,28 @@ +package domain + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDeviceAuthorization_RejectsUnsafeValues(t *testing.T) { + valid := OAuthDeviceAuthorization{DeviceCode: strings.Repeat("a", 43), UserCode: "BCDF-GHJK", VerificationURI: "https://auth.example.test/device", ExpiresIn: 600, Interval: 5} + require.NoError(t, valid.ValidateFor("https://auth.example.test")) + for _, change := range []func(*OAuthDeviceAuthorization){ + func(d *OAuthDeviceAuthorization) { d.DeviceCode = strings.Repeat("a", 2049) }, + func(d *OAuthDeviceAuthorization) { d.DeviceCode = "invalid\n" }, + func(d *OAuthDeviceAuthorization) { d.UserCode = "code\x1b[31m" }, + func(d *OAuthDeviceAuthorization) { d.VerificationURI = "https://evil.example.test/device" }, + func(d *OAuthDeviceAuthorization) { + d.VerificationURIComplete = "https://user:secret@auth.example.test/device" + }, + func(d *OAuthDeviceAuthorization) { d.ExpiresIn = 601 }, + func(d *OAuthDeviceAuthorization) { d.Interval = -1 }, + } { + d := valid + change(&d) + require.ErrorIs(t, d.ValidateFor("https://auth.example.test"), ErrOAuthMetadata) + } +} diff --git a/internal/ports/oauthas.go b/internal/ports/oauthas.go index 84bd0e6..f8cfeb0 100644 --- a/internal/ports/oauthas.go +++ b/internal/ports/oauthas.go @@ -44,3 +44,9 @@ type OAuthAuthServerClient interface { // UserInfo returns the OIDC claims associated with an access token. UserInfo(ctx context.Context, accessToken string) (*domain.OAuthUserInfo, error) } + +// OAuthDeviceAuthServerClient is the optional RFC 8628 transport. +type OAuthDeviceAuthServerClient interface { + StartDeviceAuthorization(context.Context, string, []string, string) (*domain.OAuthDeviceAuthorization, error) + PollDeviceTokens(context.Context, string, string, string) (*domain.OAuthTokens, error) +} From ba5f3342d03faa26f809beca2ee976ad3ad18f52 Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Tue, 13 Oct 2026 15:52:01 +0200 Subject: [PATCH 2/3] [TW-7617] fix(cli): retry recoverable device token 503 responses --- internal/app/oauthlogin/device.go | 5 +++++ internal/app/oauthlogin/device_test.go | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/internal/app/oauthlogin/device.go b/internal/app/oauthlogin/device.go index 2640821..1629faf 100644 --- a/internal/app/oauthlogin/device.go +++ b/internal/app/oauthlogin/device.go @@ -94,6 +94,11 @@ func (s *Service) loginDevice(ctx context.Context, metadata *domain.OAuthServerM switch oauthErr.Code { case "authorization_pending": continue + case "temporarily_unavailable", "server_error": + if oauthErr.StatusCode == 503 { + interval = max(interval, min(interval*2, 60*time.Second)) + continue + } case "slow_down": interval += 5 * time.Second continue diff --git a/internal/app/oauthlogin/device_test.go b/internal/app/oauthlogin/device_test.go index fbeeb0d..04095af 100644 --- a/internal/app/oauthlogin/device_test.go +++ b/internal/app/oauthlogin/device_test.go @@ -144,3 +144,12 @@ func TestDeviceLogin_RegionConstrainsSignupAudience(t *testing.T) { assert.Equal(t, domain.MCPResourceEU, result.Resource) assert.Equal(t, domain.MCPResourceEU, f.secrets.GetAll()[ports.KeyOAuthResource]) } + +func TestDeviceLogin_RetriesTransient503WithoutRestart(t *testing.T) { + f, c, opts, intervals := deviceFixture(t) + c.errors = []error{&domain.OAuthError{Code: "temporarily_unavailable", StatusCode: 503}} + _, err := f.service.Login(context.Background(), *opts) + require.NoError(t, err) + assert.Equal(t, []time.Duration{5 * time.Second, 10 * time.Second}, *intervals) + assert.Equal(t, 2, c.polls) +} From d7e77822eb4b18b736c020356023d4300b8d8b6d Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Wed, 14 Oct 2026 15:50:43 +0200 Subject: [PATCH 3/3] [TW-7617] fix(cli): let device sign-in name a non-production MCP server, and keep invalid_target Device sign-in always sends the configured region's MCP server as the RFC 8707 resource, and those were hardcoded to production. Staging names mcp-staging.us.nylas.com, so every US device login against it was refused with invalid_target. NYLAS_MCP_US_URL / NYLAS_MCP_EU_URL now replace that region's server for the resource, the accepted audience, and so the host a token is sent to. They replace rather than add, so the CLI still trusts exactly one server per region. An override must be an https origin (http only for loopback); a bad one fails closed instead of falling back to production. safeDeviceError also collapsed invalid_target into server_error, so the reader saw `oauth error "server_error"` with no cause. invalid_target is kept, still without the server's description, and the device flow says which variable to set. Verified against staging: without the override the CLI explains the refusal; with NYLAS_MCP_US_URL=https://mcp-staging.us.nylas.com staging issues a device code for region us. Co-Authored-By: Claude Opus 5.5 --- docs/COMMANDS.md | 12 +++++ internal/adapters/oauthas/device.go | 2 +- internal/adapters/oauthas/device_test.go | 23 +++++++++ internal/app/oauthlogin/device.go | 16 ++++++- internal/app/oauthlogin/device_test.go | 39 ++++++++++++++++ internal/cli/oauth/login.go | 5 +- internal/domain/mcp_auth.go | 59 ++++++++++++++++++++++-- internal/domain/mcp_auth_test.go | 49 ++++++++++++++++++++ 8 files changed, 197 insertions(+), 8 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index d7e8e77..a1409ae 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -219,6 +219,18 @@ If that server registers the CLI under a different client id, override it with `NYLAS_OAUTH_CLIENT_ID` (it must still allow the `http://127.0.0.1/callback` redirect URI). +A non-production server also names its own MCP servers, and answers the +production ones with `invalid_target`. Point the regional resource at them with +`NYLAS_MCP_US_URL` / `NYLAS_MCP_EU_URL` (an `https://` origin; `http://` only +for loopback). Each replaces that region's server rather than adding one, so +an OAuth token is still only sent to the one server its audience names: + +```bash +NYLAS_DASHBOARD_ACCOUNT_URL=https://dashboard-account-stg.eu.nylas.com \ +NYLAS_MCP_US_URL=https://mcp-staging.us.nylas.com \ +nylas oauth login --device +``` + The dashboard session exchange only accepts tokens from dashboard-account's built-in first-party clients (the Nylas CLI and Nylas Mail), and only that client may request `dashboard.session`. With any other client id the CLI leaves diff --git a/internal/adapters/oauthas/device.go b/internal/adapters/oauthas/device.go index 90e32b0..ff9b295 100644 --- a/internal/adapters/oauthas/device.go +++ b/internal/adapters/oauthas/device.go @@ -58,7 +58,7 @@ func safeDeviceError(err error) error { if errors.As(err, &oauthErr) { code := "server_error" switch oauthErr.Code { - case "authorization_pending", "slow_down", "access_denied", "expired_token", "invalid_grant", "invalid_client", "unauthorized_client", "unsupported_grant_type", "invalid_scope", "invalid_request", "server_error", "temporarily_unavailable": + case "authorization_pending", "slow_down", "access_denied", "expired_token", "invalid_grant", "invalid_client", "unauthorized_client", "unsupported_grant_type", "invalid_scope", "invalid_target", "invalid_request", "server_error", "temporarily_unavailable": code = oauthErr.Code } return &domain.OAuthError{Code: code, StatusCode: oauthErr.StatusCode} diff --git a/internal/adapters/oauthas/device_test.go b/internal/adapters/oauthas/device_test.go index e27999f..cd2a0b4 100644 --- a/internal/adapters/oauthas/device_test.go +++ b/internal/adapters/oauthas/device_test.go @@ -63,3 +63,26 @@ func TestMetadata_RejectsCrossHostDeviceEndpoint(t *testing.T) { metadata := domain.OAuthServerMetadata{Issuer: "https://auth.example.test", AuthorizationEndpoint: "https://auth.example.test/authorize", TokenEndpoint: "https://auth.example.test/token", DeviceAuthorizationEndpoint: "https://other.example.test/device"} require.ErrorIs(t, metadata.ValidateFor(metadata.Issuer), domain.ErrOAuthMetadata) } + +// invalid_target is how a server refuses the MCP resource. It must survive +// as itself rather than collapse into server_error, still without the body. +func TestDeviceClient_KeepsInvalidTarget(t *testing.T) { + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + mux.HandleFunc(discoveryPath, func(w http.ResponseWriter, _ *http.Request) { + doc := discoveryDocument(server.URL) + doc["device_authorization_endpoint"] = server.URL + "/device-start" + _ = json.NewEncoder(w).Encode(doc) + }) + mux.HandleFunc("/device-start", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"invalid_target","error_description":"server-text"}`)) + }) + _, err := NewClient(server.URL).StartDeviceAuthorization(context.Background(), domain.DefaultOAuthClientID, []string{"openid"}, domain.MCPResourceUS) + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "invalid_target", oauthErr.Code) + assert.Equal(t, http.StatusBadRequest, oauthErr.StatusCode) + assert.NotContains(t, err.Error(), "server-text") +} diff --git a/internal/app/oauthlogin/device.go b/internal/app/oauthlogin/device.go index 1629faf..1f53fee 100644 --- a/internal/app/oauthlogin/device.go +++ b/internal/app/oauthlogin/device.go @@ -46,7 +46,7 @@ func (s *Service) loginDevice(ctx context.Context, metadata *domain.OAuthServerM started := s.now() authorization, err := client.StartDeviceAuthorization(ctx, s.clientID, scopes, opts.Resource) if err != nil { - return nil, err + return nil, explainDeviceTarget(err, opts.Resource) } if authorization == nil { return nil, domain.ErrOAuthMetadata @@ -137,3 +137,17 @@ func (s *Service) loginDevice(ctx context.Context, metadata *domain.OAuthServerM DroppedScopes: dropped}, nil } } + +// explainDeviceTarget says why the server refused the resource. Device errors +// never carry the server's description, so without this the reader sees a +// bare invalid_target and no way forward. The usual cause is a non-production +// server that names its own MCP servers. +func explainDeviceTarget(err error, resource string) error { + var oauthErr *domain.OAuthError + if resource == "" || !errors.As(err, &oauthErr) || oauthErr.Code != "invalid_target" { + return err + } + return fmt.Errorf("%w: this server does not accept %s as the MCP server; "+ + "for a non-production server set %s or %s to the MCP server it names", + err, resource, domain.EnvMCPURLUS, domain.EnvMCPURLEU) +} diff --git a/internal/app/oauthlogin/device_test.go b/internal/app/oauthlogin/device_test.go index 04095af..e54c6a1 100644 --- a/internal/app/oauthlogin/device_test.go +++ b/internal/app/oauthlogin/device_test.go @@ -22,10 +22,14 @@ type deviceTestClient struct { polls int scopes []string resource string + startErr error } func (c *deviceTestClient) StartDeviceAuthorization(_ context.Context, _ string, scopes []string, resource string) (*domain.OAuthDeviceAuthorization, error) { c.scopes, c.resource = scopes, resource + if c.startErr != nil { + return nil, c.startErr + } return c.authorization, nil } @@ -153,3 +157,38 @@ func TestDeviceLogin_RetriesTransient503WithoutRestart(t *testing.T) { assert.Equal(t, []time.Duration{5 * time.Second, 10 * time.Second}, *intervals) assert.Equal(t, 2, c.polls) } + +// Staging answers the production MCP server with invalid_target. Device +// errors drop the server's description, so the CLI must say what to do. +func TestDeviceLogin_InvalidTargetNamesTheOverride(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + c.startErr = &domain.OAuthError{Code: "invalid_target", StatusCode: 400} + opts.Resource = domain.MCPResourceUS + _, err := f.service.Login(context.Background(), *opts) + require.Error(t, err) + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "invalid_target", oauthErr.Code) + assert.Contains(t, err.Error(), domain.MCPResourceUS) + assert.Contains(t, err.Error(), domain.EnvMCPURLUS) + assert.Contains(t, err.Error(), domain.EnvMCPURLEU) +} + +func TestDeviceLogin_OtherStartErrorsAreUnchanged(t *testing.T) { + f, c, opts, _ := deviceFixture(t) + refused := &domain.OAuthError{Code: "invalid_client", StatusCode: 401} + c.startErr = refused + opts.Resource = domain.MCPResourceUS + _, err := f.service.Login(context.Background(), *opts) + assert.Same(t, refused, err) +} + +func TestDeviceLogin_RegionUsesTheMCPOverride(t *testing.T) { + t.Setenv(domain.EnvMCPURLUS, "https://mcp-staging.us.example.test") + f, c, opts, _ := deviceFixture(t) + opts.Region = "us" + _, err := f.service.Login(context.Background(), *opts) + require.NoError(t, err) + assert.Equal(t, "https://mcp-staging.us.example.test", c.resource) + assert.Equal(t, "https://mcp-staging.us.example.test", f.secrets.GetAll()[ports.KeyOAuthResource]) +} diff --git a/internal/cli/oauth/login.go b/internal/cli/oauth/login.go index 1e662f1..729668c 100644 --- a/internal/cli/oauth/login.go +++ b/internal/cli/oauth/login.go @@ -65,7 +65,10 @@ created; without either it is created in the US. It does not change an existing account. Set NYLAS_OAUTH_CLIENT_ID to use a different client id against a local or -dev authorization server.`, +dev authorization server. Such a server names its own MCP servers and refuses +the production ones (invalid_target): set NYLAS_MCP_US_URL and/or +NYLAS_MCP_EU_URL to the origins it names. Each replaces that region's server, +so a token is only ever sent to the one server its region names.`, Example: ` # Log in with the default scopes (openid, email, offline_access, dashboard.session) nylas oauth login diff --git a/internal/domain/mcp_auth.go b/internal/domain/mcp_auth.go index 080138d..dab7a1c 100644 --- a/internal/domain/mcp_auth.go +++ b/internal/domain/mcp_auth.go @@ -3,6 +3,9 @@ package domain import ( "errors" "fmt" + "net" + "net/url" + "os" "slices" "strings" ) @@ -15,6 +18,16 @@ const ( MCPResourceEU = "https://mcp.eu.nylas.com" ) +// A non-production authorization server (staging, a local stack) names its +// own MCP servers, and answers the production ones with invalid_target. These +// replace the regional server for the OAuth resource, the token audience the +// CLI accepts, and so the host an OAuth token is sent to. They replace rather +// than add: the CLI still trusts exactly one server per region. +const ( + EnvMCPURLUS = "NYLAS_MCP_US_URL" + EnvMCPURLEU = "NYLAS_MCP_EU_URL" +) + // ErrMCPResource is returned for a resource or audience that is not one of // the hosted Nylas MCP servers. var ErrMCPResource = errors.New("not a Nylas MCP server") @@ -24,9 +37,9 @@ var ErrMCPResource = errors.New("not a Nylas MCP server") func MCPResourceForRegion(region string) (string, error) { switch strings.ToLower(strings.TrimSpace(region)) { case "", "us": - return MCPResourceUS, nil + return mcpResource(EnvMCPURLUS, MCPResourceUS) case "eu": - return MCPResourceEU, nil + return mcpResource(EnvMCPURLEU, MCPResourceEU) default: return "", fmt.Errorf("%w: unknown region %q (want us or eu)", ErrMCPResource, region) } @@ -34,10 +47,46 @@ func MCPResourceForRegion(region string) (string, error) { // ValidateMCPResource reports whether resource is a hosted MCP server. func ValidateMCPResource(resource string) error { - if resource != MCPResourceUS && resource != MCPResourceEU { - return fmt.Errorf("%w: %q", ErrMCPResource, resource) + if resource != "" { + for _, region := range []string{"us", "eu"} { + if allowed, err := MCPResourceForRegion(region); err == nil && resource == allowed { + return nil + } + } + } + return fmt.Errorf("%w: %q", ErrMCPResource, resource) +} + +// mcpResource resolves one region's server: the override in env when set, +// otherwise the production server. An override is an origin, HTTPS unless it +// is loopback, so a typo cannot send a token over plain HTTP to another host. +// A single trailing slash is dropped because the authorization server binds +// the slash-less spelling as the audience; anything else is refused rather +// than silently replaced by production. +func mcpResource(env, production string) (string, error) { + raw := strings.TrimSpace(os.Getenv(env)) + if raw == "" { + return production, nil + } + u, err := url.Parse(raw) + if err != nil || u.Host == "" || u.User != nil || u.RawQuery != "" || u.ForceQuery || u.Fragment != "" || (u.Path != "" && u.Path != "/") { + return "", fmt.Errorf("%w: %s must be an origin such as https://mcp.example.com", ErrMCPResource, env) + } + switch { + case u.Scheme == "https": + case u.Scheme == "http" && isLoopbackHost(u.Hostname()): + default: + return "", fmt.Errorf("%w: %s must use https (http only for localhost)", ErrMCPResource, env) + } + return u.Scheme + "://" + u.Host, nil +} + +func isLoopbackHost(host string) bool { + if strings.EqualFold(host, "localhost") { + return true } - return nil + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() } // MCPEndpointFromAudience picks the MCP server a token was issued for. The diff --git a/internal/domain/mcp_auth_test.go b/internal/domain/mcp_auth_test.go index 13d61bf..c7b833e 100644 --- a/internal/domain/mcp_auth_test.go +++ b/internal/domain/mcp_auth_test.go @@ -30,6 +30,55 @@ func TestValidateMCPResource(t *testing.T) { } } +func TestMCPResource_Override(t *testing.T) { + t.Setenv(EnvMCPURLUS, "https://mcp-staging.us.example.test/") + got, err := MCPResourceForRegion("us") + require.NoError(t, err) + assert.Equal(t, "https://mcp-staging.us.example.test", got, "one trailing slash is dropped") + got, err = MCPResourceForRegion("eu") + require.NoError(t, err) + assert.Equal(t, MCPResourceEU, got, "the other region keeps production") + + // The override REPLACES the region's server: production US is no longer + // trusted, so a token can only go to the one server per region. + require.NoError(t, ValidateMCPResource("https://mcp-staging.us.example.test")) + require.NoError(t, ValidateMCPResource(MCPResourceEU)) + require.ErrorIs(t, ValidateMCPResource(MCPResourceUS), ErrMCPResource) + endpoint, err := MCPEndpointFromAudience([]string{"https://mcp-staging.us.example.test"}) + require.NoError(t, err) + assert.Equal(t, "https://mcp-staging.us.example.test", endpoint) +} + +func TestMCPResource_OverrideLoopbackHTTP(t *testing.T) { + for _, origin := range []string{"http://localhost:8080", "http://127.0.0.1:8080", "http://[::1]:8080"} { + t.Setenv(EnvMCPURLEU, origin) + got, err := MCPResourceForRegion("eu") + require.NoError(t, err, origin) + assert.Equal(t, origin, got) + } +} + +// A bad override fails closed instead of quietly using production, and never +// becomes trusted itself. +func TestMCPResource_OverrideRejected(t *testing.T) { + for _, bad := range []string{ + "http://mcp.example.test", + "ftp://mcp.example.test", + "https://user@mcp.example.test", + "https://mcp.example.test/path", + "https://mcp.example.test?x=1", + "https://mcp.example.test#f", + "mcp.example.test", + "https://", + } { + t.Setenv(EnvMCPURLUS, bad) + _, err := MCPResourceForRegion("us") + require.ErrorIs(t, err, ErrMCPResource, bad) + assert.ErrorIs(t, ValidateMCPResource(bad), ErrMCPResource, bad) + assert.ErrorIs(t, ValidateMCPResource(MCPResourceUS), ErrMCPResource, bad) + } +} + func TestMCPEndpointFromAudience(t *testing.T) { got, err := MCPEndpointFromAudience([]string{"https://mcp.eu.nylas.com"}) require.NoError(t, err)