From 9dcc928f21c608ba3599e8683fea34fcb5ec9f92 Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Mon, 21 Sep 2026 14:34:08 +0200 Subject: [PATCH 1/5] TW-6922: add OAuth authorization server client and RFC 7636 PKCE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 1a of integrating the dashboard-account OAuth 2.1 authorization server into the CLI: the domain types, the port, and the HTTP adapter. The adapter does not reuse dashboard.AccountClient because these endpoints are plain RFC 6749/7009/7591 — no house {"data":...} envelope and no DPoP proof. It resolves every endpoint from the RFC 8414 discovery document rather than assuming a path, which matters because dashboard-account builds them all from OAUTH_ISSUER and that is a different host from the local port in a tunnelled dev setup. PKCE is generated fresh rather than reusing auth.generatePKCEPair: that helper computes base64std(hex(sha256(v))) for Nylas hosted auth, and the authorization server enforces /^[A-Za-z0-9\-_]{43}$/, which only the RFC form satisfies. A test pins the divergence so the two cannot be merged. Co-Authored-By: Claude Opus 5 --- internal/adapters/oauthas/client.go | 156 ++++++++++ internal/adapters/oauthas/client_test.go | 377 +++++++++++++++++++++++ internal/adapters/oauthas/flows.go | 142 +++++++++ internal/domain/oauth.go | 211 +++++++++++++ internal/domain/oauth_test.go | 161 ++++++++++ internal/ports/oauthas.go | 41 +++ 6 files changed, 1088 insertions(+) create mode 100644 internal/adapters/oauthas/client.go create mode 100644 internal/adapters/oauthas/client_test.go create mode 100644 internal/adapters/oauthas/flows.go create mode 100644 internal/domain/oauth.go create mode 100644 internal/domain/oauth_test.go create mode 100644 internal/ports/oauthas.go diff --git a/internal/adapters/oauthas/client.go b/internal/adapters/oauthas/client.go new file mode 100644 index 0000000..f2f88ec --- /dev/null +++ b/internal/adapters/oauthas/client.go @@ -0,0 +1,156 @@ +// Package oauthas implements a client for the Nylas OAuth 2.1 authorization +// server hosted by dashboard-account. +// +// These endpoints speak plain RFC 6749/7009/7591: no house {"data":...} +// envelope and no DPoP proof. That is why this does not reuse the +// dashboard.AccountClient transport, which adds both. +package oauthas + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "strings" + "sync" + "time" + + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/version" +) + +const ( + maxResponseBody = 1 << 20 // 1 MB + discoveryPath = "/.well-known/oauth-authorization-server" + defaultHTTPTimout = 30 * time.Second +) + +// Client is an HTTP client for the authorization server. +type Client struct { + baseURL string + httpClient *http.Client + now func() time.Time + + mu sync.Mutex + metadata *domain.OAuthServerMetadata +} + +// NewClient creates a client rooted at the authorization server's base URL. +func NewClient(baseURL string) *Client { + return &Client{ + baseURL: strings.TrimRight(baseURL, "/"), + httpClient: &http.Client{Timeout: defaultHTTPTimout}, + now: time.Now, + } +} + +// Metadata fetches and caches the RFC 8414 metadata document. +func (c *Client) Metadata(ctx context.Context) (*domain.OAuthServerMetadata, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.metadata != nil { + return c.metadata, nil + } + + var metadata domain.OAuthServerMetadata + if err := c.getJSON(ctx, c.baseURL+discoveryPath, "", &metadata); err != nil { + return nil, fmt.Errorf("failed to discover authorization server at %s: %w", c.baseURL, err) + } + if err := metadata.Validate(); err != nil { + return nil, err + } + + c.metadata = &metadata + return c.metadata, nil +} + +func (c *Client) getJSON(ctx context.Context, endpoint, accessToken string, result any) error { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return fmt.Errorf("failed to create request: %w", err) + } + if accessToken != "" { + req.Header.Set("Authorization", "Bearer "+accessToken) + } + return c.do(req, result) +} + +func (c *Client) postForm(ctx context.Context, endpoint string, form url.Values, result any) error { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(form.Encode())) + if err != nil { + return fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return c.do(req, result) +} + +func (c *Client) postJSON(ctx context.Context, endpoint string, body, result any) error { + payload, err := json.Marshal(body) + if err != nil { + return fmt.Errorf("failed to encode request: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(string(payload))) + if err != nil { + return fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + return c.do(req, result) +} + +func (c *Client) do(req *http.Request, result any) error { + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", version.UserAgent()) + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("%w: %v", domain.ErrNetworkError, err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBody)) + if err != nil { + return fmt.Errorf("failed to read response: %w", err) + } + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return parseOAuthError(resp.StatusCode, body) + } + + if result == nil { + return nil + } + if err := json.Unmarshal(body, result); err != nil { + return fmt.Errorf("failed to decode response: %w", err) + } + return nil +} + +// parseOAuthError decodes an RFC 6749 section 5.2 error body. A response that +// is not in that shape (an HTML error page, or the house envelope, whose +// "error" is an object) falls back to the status code and a body snippet. +func parseOAuthError(statusCode int, body []byte) error { + var payload struct { + Error string `json:"error"` + Description string `json:"error_description"` + } + if err := json.Unmarshal(body, &payload); err == nil && payload.Error != "" { + return &domain.OAuthError{ + Code: payload.Error, + Description: payload.Description, + StatusCode: statusCode, + } + } + + snippet := strings.TrimSpace(string(body)) + if len(snippet) > 200 { + snippet = snippet[:200] + } + return &domain.OAuthError{ + Code: "http_" + strconv.Itoa(statusCode), + Description: snippet, + StatusCode: statusCode, + } +} diff --git a/internal/adapters/oauthas/client_test.go b/internal/adapters/oauthas/client_test.go new file mode 100644 index 0000000..a973175 --- /dev/null +++ b/internal/adapters/oauthas/client_test.go @@ -0,0 +1,377 @@ +//go:build !integration + +package oauthas + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// discoveryDocument mirrors what dashboard-account serves, including the fact +// that every endpoint is an absolute URL derived from OAUTH_ISSUER rather +// than a path relative to where discovery was fetched. +func discoveryDocument(issuer string) map[string]any { + return map[string]any{ + "issuer": issuer, + "authorization_endpoint": issuer + "/oauth/authorize", + "token_endpoint": issuer + "/oauth/token", + "userinfo_endpoint": issuer + "/oauth/userinfo", + "revocation_endpoint": issuer + "/oauth/revoke", + "registration_endpoint": issuer + "/oauth/register", + "jwks_uri": issuer + "/.well-known/jwks.json", + "scopes_supported": []string{"openid", "email", "offline_access"}, + "grant_types_supported": []string{"authorization_code", "refresh_token"}, + "response_types_supported": []string{"code"}, + "code_challenge_methods_supported": []string{"S256"}, + "token_endpoint_auth_methods_supported": []string{"client_secret_post", "client_secret_basic", "none"}, + } +} + +// newTestServer starts a stub authorization server. handler receives every +// request other than discovery. +func newTestServer(t *testing.T, handler http.HandlerFunc) *httptest.Server { + t.Helper() + + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(discoveryDocument(server.URL)) + }) + if handler != nil { + mux.HandleFunc("/", handler) + } + + return server +} + +func TestClient_Metadata(t *testing.T) { + server := newTestServer(t, nil) + client := NewClient(server.URL) + + metadata, err := client.Metadata(context.Background()) + + require.NoError(t, err) + assert.Equal(t, server.URL, metadata.Issuer) + assert.Equal(t, server.URL+"/oauth/token", metadata.TokenEndpoint) + assert.Equal(t, []string{"S256"}, metadata.CodeChallengeMethodsSupported) +} + +func TestClient_Metadata_IsFetchedOnce(t *testing.T) { + calls := 0 + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + calls++ + _ = json.NewEncoder(w).Encode(discoveryDocument(server.URL)) + }) + + client := NewClient(server.URL) + _, err := client.Metadata(context.Background()) + require.NoError(t, err) + _, err = client.Metadata(context.Background()) + require.NoError(t, err) + + assert.Equal(t, 1, calls, "metadata should be cached for the client's lifetime") +} + +func TestClient_Metadata_RejectsUnrelatedJSON(t *testing.T) { + // Pointing the CLI at the wrong port is the most common local-setup + // mistake; it must fail naming the missing fields, not much later with + // an empty-URL request. + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"status":"ok"}`)) + }) + + _, err := NewClient(server.URL).Metadata(context.Background()) + + require.ErrorIs(t, err, domain.ErrOAuthMetadata) +} + +func TestClient_AuthorizationURL(t *testing.T) { + server := newTestServer(t, nil) + client := NewClient(server.URL) + + raw, err := client.AuthorizationURL(context.Background(), domain.OAuthAuthorizationParams{ + ClientID: "client-123", + RedirectURI: "http://localhost:9007/callback", + Scopes: domain.DefaultOAuthScopes(), + State: "state-abc", + CodeChallenge: "challenge-xyz", + Nonce: "nonce-1", + }) + require.NoError(t, err) + + parsed, err := url.Parse(raw) + require.NoError(t, err) + query := parsed.Query() + + assert.Equal(t, server.URL+"/oauth/authorize", parsed.Scheme+"://"+parsed.Host+parsed.Path) + assert.Equal(t, "code", query.Get("response_type")) + assert.Equal(t, "client-123", query.Get("client_id")) + assert.Equal(t, "http://localhost:9007/callback", query.Get("redirect_uri")) + assert.Equal(t, "openid email offline_access", query.Get("scope")) + assert.Equal(t, "state-abc", query.Get("state")) + assert.Equal(t, "challenge-xyz", query.Get("code_challenge")) + assert.Equal(t, "S256", query.Get("code_challenge_method")) + assert.Equal(t, "nonce-1", query.Get("nonce")) +} + +func TestClient_AuthorizationURL_OmitsEmptyNonce(t *testing.T) { + server := newTestServer(t, nil) + + raw, err := NewClient(server.URL).AuthorizationURL(context.Background(), domain.OAuthAuthorizationParams{ + ClientID: "client-123", + }) + require.NoError(t, err) + + parsed, err := url.Parse(raw) + require.NoError(t, err) + assert.False(t, parsed.Query().Has("nonce")) +} + +func TestClient_ExchangeCode(t *testing.T) { + var got url.Values + var contentType string + + server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/oauth/token", r.URL.Path) + require.NoError(t, r.ParseForm()) + got = r.PostForm + contentType = r.Header.Get("Content-Type") + + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "access_token": "at-1", + "token_type": "Bearer", + "expires_in": 3600, + "refresh_token": "rt-1", + "id_token": "idt-1", + "scope": "openid email offline_access" + }`)) + }) + + client := NewClient(server.URL) + client.now = func() time.Time { return time.Date(2026, 9, 21, 12, 0, 0, 0, time.UTC) } + + tokens, err := client.ExchangeCode(context.Background(), domain.OAuthCodeExchange{ + ClientID: "client-123", + Code: "auth-code", + RedirectURI: "http://localhost:9007/callback", + CodeVerifier: "verifier-abc", + }) + require.NoError(t, err) + + assert.Equal(t, "application/x-www-form-urlencoded", contentType) + assert.Equal(t, "authorization_code", got.Get("grant_type")) + assert.Equal(t, "auth-code", got.Get("code")) + assert.Equal(t, "verifier-abc", got.Get("code_verifier")) + assert.Equal(t, "client-123", got.Get("client_id")) + assert.False(t, got.Has("client_secret"), "a public client must not send a secret") + + assert.Equal(t, "at-1", tokens.AccessToken) + assert.Equal(t, "rt-1", tokens.RefreshToken) + assert.Equal(t, "idt-1", tokens.IDToken) + assert.Equal(t, time.Date(2026, 9, 21, 13, 0, 0, 0, time.UTC), tokens.ExpiresAt) +} + +func TestClient_ExchangeCode_SendsSecretForConfidentialClient(t *testing.T) { + var got url.Values + server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + got = r.PostForm + _, _ = w.Write([]byte(`{"access_token":"at-1","token_type":"Bearer","expires_in":3600}`)) + }) + + _, err := NewClient(server.URL).ExchangeCode(context.Background(), domain.OAuthCodeExchange{ + ClientID: "client-123", + ClientSecret: "shh", + Code: "auth-code", + }) + require.NoError(t, err) + + assert.Equal(t, "shh", got.Get("client_secret")) +} + +func TestClient_ExchangeCode_SurfacesOAuthError(t *testing.T) { + server := newTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"invalid_grant","error_description":"code already redeemed"}`)) + }) + + _, err := NewClient(server.URL).ExchangeCode(context.Background(), domain.OAuthCodeExchange{ClientID: "c"}) + + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "invalid_grant", oauthErr.Code) + assert.Equal(t, "code already redeemed", oauthErr.Description) + assert.Equal(t, http.StatusBadRequest, oauthErr.StatusCode) +} + +func TestClient_ExchangeCode_FallsBackWhenErrorIsNotRFCShaped(t *testing.T) { + // The house error envelope wraps "error" as an object, which no OAuth + // client can read; the status code must still reach the user. + server := newTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":{"code":"INTERNAL","message":"boom"}}`)) + }) + + _, err := NewClient(server.URL).ExchangeCode(context.Background(), domain.OAuthCodeExchange{ClientID: "c"}) + + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, http.StatusInternalServerError, oauthErr.StatusCode) + assert.Contains(t, oauthErr.Description, "boom") +} + +func TestClient_ExchangeCode_RejectsResponseWithoutAccessToken(t *testing.T) { + server := newTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"token_type":"Bearer"}`)) + }) + + _, err := NewClient(server.URL).ExchangeCode(context.Background(), domain.OAuthCodeExchange{ClientID: "c"}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "access_token") +} + +func TestClient_Refresh_ReturnsRotatedToken(t *testing.T) { + var got url.Values + server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + got = r.PostForm + // The server rotates on every use and burns the family on replay, + // so the caller must persist this new value. + _, _ = w.Write([]byte(`{"access_token":"at-2","token_type":"Bearer","expires_in":3600,"refresh_token":"rt-2"}`)) + }) + + tokens, err := NewClient(server.URL).Refresh(context.Background(), "client-123", "rt-1") + require.NoError(t, err) + + assert.Equal(t, "refresh_token", got.Get("grant_type")) + assert.Equal(t, "rt-1", got.Get("refresh_token")) + assert.Equal(t, "client-123", got.Get("client_id")) + assert.Equal(t, "rt-2", tokens.RefreshToken) +} + +func TestClient_Register(t *testing.T) { + var body domain.OAuthClientRegistrationRequest + server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/oauth/register", r.URL.Path) + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "dcr-client-1", + "client_id_issued_at": 1758456000, + "client_name": "Nylas CLI", + "redirect_uris": ["http://localhost/callback"], + "token_endpoint_auth_method": "none", + "grant_types": ["authorization_code","refresh_token"], + "response_types": ["code"] + }`)) + }) + + registration, err := NewClient(server.URL).Register(context.Background(), domain.OAuthClientRegistrationRequest{ + ClientName: "Nylas CLI", + RedirectURIs: []string{"http://localhost/callback"}, + TokenEndpointAuthMethod: "none", + }) + require.NoError(t, err) + + assert.Equal(t, "none", body.TokenEndpointAuthMethod, + "the CLI must register as a public client explicitly") + assert.Equal(t, "dcr-client-1", registration.ClientID) + assert.Empty(t, registration.ClientSecret) +} + +func TestClient_Register_RejectsResponseWithoutClientID(t *testing.T) { + server := newTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"client_name":"Nylas CLI"}`)) + }) + + _, err := NewClient(server.URL).Register(context.Background(), domain.OAuthClientRegistrationRequest{}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "client_id") +} + +func TestClient_Revoke(t *testing.T) { + var got url.Values + server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/oauth/revoke", r.URL.Path) + require.NoError(t, r.ParseForm()) + got = r.PostForm + w.WriteHeader(http.StatusOK) + }) + + err := NewClient(server.URL).Revoke(context.Background(), "client-123", "rt-1") + require.NoError(t, err) + + assert.Equal(t, "rt-1", got.Get("token")) + assert.Equal(t, "client-123", got.Get("client_id")) +} + +func TestClient_UserInfo(t *testing.T) { + var authorization string + server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/oauth/userinfo", r.URL.Path) + authorization = r.Header.Get("Authorization") + + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"sub":"user-1","email":"dev@example.test","email_verified":true,"org":"org-1"}`)) + }) + + info, err := NewClient(server.URL).UserInfo(context.Background(), "at-1") + require.NoError(t, err) + + assert.Equal(t, "Bearer at-1", authorization) + assert.Equal(t, "user-1", info.Subject) + assert.Equal(t, "dev@example.test", info.Email) + assert.True(t, info.EmailVerified) + assert.Equal(t, "org-1", info.Org) +} + +func TestClient_FollowsDiscoveredEndpointHost(t *testing.T) { + // dashboard-account builds every endpoint from OAUTH_ISSUER, which in a + // tunnelled dev setup is a different host from where discovery was + // fetched. The client must use the advertised URL, not its own base. + tokenCalls := 0 + issuerMux := http.NewServeMux() + issuer := httptest.NewServer(issuerMux) + t.Cleanup(issuer.Close) + issuerMux.HandleFunc("/oauth/token", func(w http.ResponseWriter, _ *http.Request) { + tokenCalls++ + _, _ = w.Write([]byte(`{"access_token":"at-1","token_type":"Bearer","expires_in":3600}`)) + }) + + discoveryMux := http.NewServeMux() + discovery := httptest.NewServer(discoveryMux) + t.Cleanup(discovery.Close) + discoveryMux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(discoveryDocument(issuer.URL)) + }) + + _, err := NewClient(discovery.URL).ExchangeCode(context.Background(), domain.OAuthCodeExchange{ClientID: "c"}) + require.NoError(t, err) + + assert.Equal(t, 1, tokenCalls) +} diff --git a/internal/adapters/oauthas/flows.go b/internal/adapters/oauthas/flows.go new file mode 100644 index 0000000..5402ebd --- /dev/null +++ b/internal/adapters/oauthas/flows.go @@ -0,0 +1,142 @@ +package oauthas + +import ( + "context" + "fmt" + "net/url" + "strings" + "time" + + "github.com/nylas/cli/internal/domain" +) + +// Register performs RFC 7591 dynamic client registration. The server defaults +// an omitted token_endpoint_auth_method to "none", but the CLI states it so +// the registration cannot silently become confidential. +func (c *Client) Register(ctx context.Context, req domain.OAuthClientRegistrationRequest) (*domain.OAuthClientRegistration, error) { + metadata, err := c.Metadata(ctx) + if err != nil { + return nil, err + } + if metadata.RegistrationEndpoint == "" { + return nil, fmt.Errorf("%w: server does not advertise a registration endpoint", domain.ErrOAuthMetadata) + } + + var registration domain.OAuthClientRegistration + if err := c.postJSON(ctx, metadata.RegistrationEndpoint, req, ®istration); err != nil { + return nil, fmt.Errorf("client registration failed: %w", err) + } + if registration.ClientID == "" { + return nil, fmt.Errorf("client registration failed: server returned no client_id") + } + return ®istration, nil +} + +// AuthorizationURL builds the authorization request URL. +func (c *Client) AuthorizationURL(ctx context.Context, params domain.OAuthAuthorizationParams) (string, error) { + metadata, err := c.Metadata(ctx) + if err != nil { + return "", err + } + + query := url.Values{} + query.Set("response_type", "code") + query.Set("client_id", params.ClientID) + query.Set("redirect_uri", params.RedirectURI) + query.Set("scope", strings.Join(params.Scopes, " ")) + query.Set("state", params.State) + query.Set("code_challenge", params.CodeChallenge) + query.Set("code_challenge_method", "S256") + if params.Nonce != "" { + query.Set("nonce", params.Nonce) + } + + separator := "?" + if strings.Contains(metadata.AuthorizationEndpoint, "?") { + separator = "&" + } + return metadata.AuthorizationEndpoint + separator + query.Encode(), nil +} + +// ExchangeCode redeems an authorization code for tokens. +func (c *Client) ExchangeCode(ctx context.Context, params domain.OAuthCodeExchange) (*domain.OAuthTokens, error) { + metadata, err := c.Metadata(ctx) + if err != nil { + return nil, err + } + + form := url.Values{} + form.Set("grant_type", "authorization_code") + form.Set("code", params.Code) + form.Set("redirect_uri", params.RedirectURI) + form.Set("code_verifier", params.CodeVerifier) + form.Set("client_id", params.ClientID) + if params.ClientSecret != "" { + form.Set("client_secret", params.ClientSecret) + } + + return c.requestTokens(ctx, metadata.TokenEndpoint, form) +} + +// Refresh exchanges a refresh token for a new token set. +func (c *Client) Refresh(ctx context.Context, clientID, refreshToken string) (*domain.OAuthTokens, error) { + metadata, err := c.Metadata(ctx) + if err != nil { + return nil, err + } + + form := url.Values{} + form.Set("grant_type", "refresh_token") + form.Set("refresh_token", refreshToken) + form.Set("client_id", clientID) + + return c.requestTokens(ctx, metadata.TokenEndpoint, form) +} + +func (c *Client) requestTokens(ctx context.Context, endpoint string, form url.Values) (*domain.OAuthTokens, error) { + var tokens domain.OAuthTokens + if err := c.postForm(ctx, endpoint, form, &tokens); err != nil { + return nil, err + } + if tokens.AccessToken == "" { + return nil, fmt.Errorf("token endpoint returned no access_token") + } + if tokens.ExpiresIn > 0 { + tokens.ExpiresAt = c.now().Add(time.Duration(tokens.ExpiresIn) * time.Second) + } + return &tokens, nil +} + +// Revoke revokes an access or refresh token. +func (c *Client) Revoke(ctx context.Context, clientID, token string) error { + metadata, err := c.Metadata(ctx) + if err != nil { + return err + } + if metadata.RevocationEndpoint == "" { + return fmt.Errorf("%w: server does not advertise a revocation endpoint", domain.ErrOAuthMetadata) + } + + form := url.Values{} + form.Set("token", token) + form.Set("client_id", clientID) + + return c.postForm(ctx, metadata.RevocationEndpoint, form, nil) +} + +// UserInfo returns the OIDC claims for an access token. +func (c *Client) UserInfo(ctx context.Context, accessToken string) (*domain.OAuthUserInfo, error) { + metadata, err := c.Metadata(ctx) + if err != nil { + return nil, err + } + if metadata.UserInfoEndpoint == "" { + return nil, fmt.Errorf("%w: server does not advertise a userinfo endpoint", domain.ErrOAuthMetadata) + } + + var info domain.OAuthUserInfo + if err := c.getJSON(ctx, metadata.UserInfoEndpoint, accessToken, &info); err != nil { + return nil, err + } + return &info, nil +} diff --git a/internal/domain/oauth.go b/internal/domain/oauth.go new file mode 100644 index 0000000..162b455 --- /dev/null +++ b/internal/domain/oauth.go @@ -0,0 +1,211 @@ +package domain + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "errors" + "fmt" + "strings" + "time" +) + +// OAuth authorization server errors. +var ( + ErrOAuthNotLoggedIn = errors.New("not logged in to the Nylas authorization server") + ErrOAuthNoRefreshToken = errors.New("no refresh token stored") + ErrOAuthMetadata = errors.New("invalid authorization server metadata") +) + +// Scopes the Nylas authorization server currently grants. Resource scopes +// (email.read, grants.read, ...) are defined server-side but withheld until a +// resource server exists, so requesting one fails the authorization request. +const ( + OAuthScopeOpenID = "openid" + OAuthScopeEmail = "email" + OAuthScopeOfflineAccess = "offline_access" +) + +// DefaultOAuthScopes is what `nylas oauth login` requests: identity plus a +// refresh token. offline_access is the only way the server issues one. +func DefaultOAuthScopes() []string { + return []string{OAuthScopeOpenID, OAuthScopeEmail, OAuthScopeOfflineAccess} +} + +// OAuthServerMetadata is the subset of the RFC 8414 authorization server +// metadata document the CLI uses. +type OAuthServerMetadata struct { + Issuer string `json:"issuer"` + AuthorizationEndpoint string `json:"authorization_endpoint"` + TokenEndpoint string `json:"token_endpoint"` + UserInfoEndpoint string `json:"userinfo_endpoint"` + RevocationEndpoint string `json:"revocation_endpoint"` + RegistrationEndpoint string `json:"registration_endpoint"` + JWKSURI string `json:"jwks_uri"` + ScopesSupported []string `json:"scopes_supported"` + GrantTypesSupported []string `json:"grant_types_supported"` + ResponseTypesSupported []string `json:"response_types_supported"` + CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"` + TokenEndpointAuthMethods []string `json:"token_endpoint_auth_methods_supported"` +} + +// Validate reports whether the document carries what an authorization code +// flow needs. Discovery pointed at the wrong host usually returns valid JSON +// with none of these fields, and failing here names the problem. +func (m *OAuthServerMetadata) Validate() error { + missing := []string{} + for name, value := range map[string]string{ + "issuer": m.Issuer, + "authorization_endpoint": m.AuthorizationEndpoint, + "token_endpoint": m.TokenEndpoint, + } { + if value == "" { + missing = append(missing, name) + } + } + if len(missing) > 0 { + return fmt.Errorf("%w: missing %s", ErrOAuthMetadata, strings.Join(missing, ", ")) + } + if len(m.CodeChallengeMethodsSupported) > 0 && !contains(m.CodeChallengeMethodsSupported, "S256") { + return fmt.Errorf("%w: server does not support the S256 code challenge method", ErrOAuthMetadata) + } + return nil +} + +func contains(values []string, want string) bool { + for _, value := range values { + if value == want { + return true + } + } + return false +} + +// OAuthError is an RFC 6749 section 5.2 error response. +type OAuthError struct { + Code string `json:"error"` + Description string `json:"error_description"` + StatusCode int `json:"-"` +} + +func (e *OAuthError) Error() string { + if e.Description == "" { + return fmt.Sprintf("oauth error %q (HTTP %d)", e.Code, e.StatusCode) + } + return fmt.Sprintf("oauth error %q: %s (HTTP %d)", e.Code, e.Description, e.StatusCode) +} + +// OAuthAuthorizationParams are the query parameters of an authorization request. +type OAuthAuthorizationParams struct { + ClientID string + RedirectURI string + Scopes []string + State string + CodeChallenge string + Nonce string +} + +// OAuthCodeExchange carries an authorization code back to the token endpoint. +// ClientSecret stays empty for a public client, which is what the CLI registers as. +type OAuthCodeExchange struct { + ClientID string + ClientSecret string + Code string + RedirectURI string + CodeVerifier string +} + +// OAuthClientRegistrationRequest is an RFC 7591 dynamic registration request. +type OAuthClientRegistrationRequest struct { + ClientName string `json:"client_name,omitempty"` + RedirectURIs []string `json:"redirect_uris"` + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method,omitempty"` + GrantTypes []string `json:"grant_types,omitempty"` + ResponseTypes []string `json:"response_types,omitempty"` + Scope string `json:"scope,omitempty"` +} + +// OAuthClientRegistration is the registered client the server returns. +type OAuthClientRegistration struct { + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret,omitempty"` + ClientIDIssuedAt int64 `json:"client_id_issued_at"` + ClientName string `json:"client_name"` + RedirectURIs []string `json:"redirect_uris"` + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"` + GrantTypes []string `json:"grant_types"` + ResponseTypes []string `json:"response_types"` +} + +// OAuthTokens is a token endpoint response. +type OAuthTokens struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int `json:"expires_in"` + RefreshToken string `json:"refresh_token,omitempty"` + IDToken string `json:"id_token,omitempty"` + Scope string `json:"scope,omitempty"` + ExpiresAt time.Time `json:"-"` +} + +// oauthExpiryLeeway refreshes slightly early so a token cannot expire in +// flight between the check and the request that uses it. +const oauthExpiryLeeway = 30 * time.Second + +// IsExpired reports whether the access token is expired at now. A token set +// with no known expiry is treated as expired so the caller refreshes rather +// than sending a credential the server will reject. +func (t *OAuthTokens) IsExpired(now time.Time) bool { + if t.ExpiresAt.IsZero() { + return true + } + return !now.Add(oauthExpiryLeeway).Before(t.ExpiresAt) +} + +// OAuthUserInfo holds the OIDC claims returned by the userinfo endpoint. +type OAuthUserInfo struct { + Subject string `json:"sub"` + Email string `json:"email,omitempty"` + EmailVerified bool `json:"email_verified,omitempty"` + Org string `json:"org,omitempty"` +} + +// PKCE is an RFC 7636 verifier and its S256 challenge. +type PKCE struct { + Verifier string + Challenge string +} + +// NewPKCE generates an RFC 7636 S256 pair. +// +// Deliberately not the same as auth.generatePKCEPair, which computes +// base64std(hex(sha256(v))) for Nylas hosted auth. The authorization server +// enforces /^[A-Za-z0-9\-_]{43}$/ on the challenge, so only the RFC form passes. +func NewPKCE() (*PKCE, error) { + verifier, err := randomURLSafe(32) + if err != nil { + return nil, fmt.Errorf("failed to generate PKCE verifier: %w", err) + } + sum := sha256.Sum256([]byte(verifier)) + return &PKCE{ + Verifier: verifier, + Challenge: base64.RawURLEncoding.EncodeToString(sum[:]), + }, nil +} + +// NewOAuthState generates an opaque CSRF state value for an authorization request. +func NewOAuthState() (string, error) { + state, err := randomURLSafe(32) + if err != nil { + return "", fmt.Errorf("failed to generate OAuth state: %w", err) + } + return state, nil +} + +func randomURLSafe(size int) (string, error) { + buf := make([]byte, size) + if _, err := rand.Read(buf); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(buf), nil +} diff --git a/internal/domain/oauth_test.go b/internal/domain/oauth_test.go new file mode 100644 index 0000000..26c37ab --- /dev/null +++ b/internal/domain/oauth_test.go @@ -0,0 +1,161 @@ +//go:build !integration + +package domain + +import ( + "crypto/sha256" + "encoding/base64" + "regexp" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// serverChallengePattern is the regex dashboard-account enforces on +// code_challenge. A challenge that fails it is rejected before the +// authorization request is even considered, so this is the contract. +var serverChallengePattern = regexp.MustCompile(`^[A-Za-z0-9\-_]{43}$`) + +func TestNewPKCE_ChallengeSatisfiesServerPattern(t *testing.T) { + pkce, err := NewPKCE() + require.NoError(t, err) + + assert.Regexp(t, serverChallengePattern, pkce.Challenge, + "authorization server rejects any challenge outside this shape") +} + +func TestNewPKCE_ChallengeIsRFC7636S256(t *testing.T) { + pkce, err := NewPKCE() + require.NoError(t, err) + + // RFC 7636 4.2: challenge = base64url(sha256(ASCII(verifier))), unpadded. + sum := sha256.Sum256([]byte(pkce.Verifier)) + want := base64.RawURLEncoding.EncodeToString(sum[:]) + + assert.Equal(t, want, pkce.Challenge) +} + +func TestNewPKCE_VerifierLengthWithinRFCRange(t *testing.T) { + pkce, err := NewPKCE() + require.NoError(t, err) + + // RFC 7636 4.1 allows 43-128 characters. + assert.GreaterOrEqual(t, len(pkce.Verifier), 43) + assert.LessOrEqual(t, len(pkce.Verifier), 128) +} + +func TestNewPKCE_IsNotTheNylasHostedAuthEncoding(t *testing.T) { + pkce, err := NewPKCE() + require.NoError(t, err) + + // app/auth uses base64std(hex(sha256(v))) for Nylas hosted auth. That + // form is 88 characters and would be refused here; guard the divergence + // so the two flows cannot be collapsed by a well-meaning refactor. + sum := sha256.Sum256([]byte(pkce.Verifier)) + hosted := base64.RawStdEncoding.EncodeToString([]byte(hexString(sum[:]))) + + assert.NotEqual(t, hosted, pkce.Challenge) +} + +func hexString(b []byte) string { + const digits = "0123456789abcdef" + out := make([]byte, 0, len(b)*2) + for _, c := range b { + out = append(out, digits[c>>4], digits[c&0x0f]) + } + return string(out) +} + +func TestNewPKCE_IsRandomPerCall(t *testing.T) { + first, err := NewPKCE() + require.NoError(t, err) + second, err := NewPKCE() + require.NoError(t, err) + + assert.NotEqual(t, first.Verifier, second.Verifier) + assert.NotEqual(t, first.Challenge, second.Challenge) +} + +func TestNewOAuthState_IsRandomAndURLSafe(t *testing.T) { + first, err := NewOAuthState() + require.NoError(t, err) + second, err := NewOAuthState() + require.NoError(t, err) + + assert.NotEqual(t, first, second) + assert.Regexp(t, `^[A-Za-z0-9\-_]+$`, first) +} + +func TestOAuthTokens_IsExpired(t *testing.T) { + now := time.Date(2026, 9, 21, 12, 0, 0, 0, time.UTC) + + tests := []struct { + name string + expiresAt time.Time + want bool + }{ + {"unknown expiry is treated as expired", time.Time{}, true}, + {"already past", now.Add(-time.Second), true}, + {"inside the leeway window", now.Add(10 * time.Second), true}, + {"comfortably valid", now.Add(time.Hour), false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tokens := &OAuthTokens{ExpiresAt: tt.expiresAt} + assert.Equal(t, tt.want, tokens.IsExpired(now)) + }) + } +} + +func TestOAuthServerMetadata_Validate(t *testing.T) { + valid := func() *OAuthServerMetadata { + return &OAuthServerMetadata{ + Issuer: "https://example.test", + AuthorizationEndpoint: "https://example.test/oauth/authorize", + TokenEndpoint: "https://example.test/oauth/token", + CodeChallengeMethodsSupported: []string{"S256"}, + } + } + + t.Run("accepts a complete document", func(t *testing.T) { + require.NoError(t, valid().Validate()) + }) + + t.Run("rejects a document with no token endpoint", func(t *testing.T) { + metadata := valid() + metadata.TokenEndpoint = "" + + err := metadata.Validate() + + require.ErrorIs(t, err, ErrOAuthMetadata) + assert.Contains(t, err.Error(), "token_endpoint") + }) + + t.Run("rejects a server without S256", func(t *testing.T) { + metadata := valid() + metadata.CodeChallengeMethodsSupported = []string{"plain"} + + err := metadata.Validate() + + require.ErrorIs(t, err, ErrOAuthMetadata) + assert.Contains(t, err.Error(), "S256") + }) +} + +func TestOAuthError_Error(t *testing.T) { + withDescription := &OAuthError{Code: "invalid_grant", Description: "code expired", StatusCode: 400} + assert.Contains(t, withDescription.Error(), "invalid_grant") + assert.Contains(t, withDescription.Error(), "code expired") + + bare := &OAuthError{Code: "invalid_client", StatusCode: 401} + assert.Contains(t, bare.Error(), "invalid_client") +} + +func TestDefaultOAuthScopes_RequestsOfflineAccess(t *testing.T) { + // offline_access is the only way the server issues a refresh token; + // without it every login would need the browser again in an hour. + assert.Contains(t, DefaultOAuthScopes(), OAuthScopeOfflineAccess) +} diff --git a/internal/ports/oauthas.go b/internal/ports/oauthas.go new file mode 100644 index 0000000..dc81018 --- /dev/null +++ b/internal/ports/oauthas.go @@ -0,0 +1,41 @@ +package ports + +import ( + "context" + + "github.com/nylas/cli/internal/domain" +) + +// OAuthAuthServerClient talks to the Nylas OAuth 2.1 authorization server +// hosted by dashboard-account. +// +// Distinct from AuthClient, which drives Nylas hosted auth to create a +// provider grant. This one authenticates the person operating the CLI and +// yields an OIDC identity plus an access token. +type OAuthAuthServerClient interface { + // Metadata fetches the RFC 8414 authorization server metadata, caching it + // for the lifetime of the client. Every other method resolves its + // endpoint from this document rather than assuming a path. + Metadata(ctx context.Context) (*domain.OAuthServerMetadata, error) + + // Register performs RFC 7591 dynamic client registration. + Register(ctx context.Context, req domain.OAuthClientRegistrationRequest) (*domain.OAuthClientRegistration, error) + + // AuthorizationURL builds the URL to open in the browser to start a + // PKCE authorization code flow. + AuthorizationURL(ctx context.Context, params domain.OAuthAuthorizationParams) (string, error) + + // ExchangeCode redeems an authorization code for a token set. + ExchangeCode(ctx context.Context, params domain.OAuthCodeExchange) (*domain.OAuthTokens, error) + + // Refresh exchanges a refresh token for a new token set. The server + // rotates refresh tokens and detects replay, so the returned + // RefreshToken must replace the one passed in. + Refresh(ctx context.Context, clientID, refreshToken string) (*domain.OAuthTokens, error) + + // Revoke revokes an access or refresh token (RFC 7009). + Revoke(ctx context.Context, clientID, token string) error + + // UserInfo returns the OIDC claims associated with an access token. + UserInfo(ctx context.Context, accessToken string) (*domain.OAuthUserInfo, error) +} From e076b25c1a57874dd3d725a52923f9e339eb812f Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Mon, 21 Sep 2026 14:39:14 +0200 Subject: [PATCH 2/5] TW-6922: add OAuth login service with PKCE flow and token rotation Stage 1b: the app-layer service that runs the browser authorization code flow and owns the stored session. Reuses the existing loopback callback server and browser adapters unchanged. The client is registered dynamically as a public client the first time it is needed, against the redirect URI http://localhost/callback with no port: RFC 8252 lets the server free the port of a loopback URI at request time, so the ephemeral port the callback server picks still matches. The registration is pinned to the issuer that produced it, because a dev tunnel URL changes between sessions and a client_id does not survive it. Refresh handling is the subtle part. The server rotates the refresh token on every use and burns the whole family if a consumed one reappears, so the service persists exactly what came back and never carries the previous refresh token forward to fill an empty field. A test covers that specific mistake. Co-Authored-By: Claude Opus 5 --- internal/adapters/oauthas/mock.go | 126 +++++++++ internal/app/oauthlogin/service.go | 252 +++++++++++++++++ internal/app/oauthlogin/service_test.go | 346 ++++++++++++++++++++++++ internal/app/oauthlogin/store.go | 146 ++++++++++ internal/ports/oauthas.go | 4 +- internal/ports/secrets.go | 11 + 6 files changed, 884 insertions(+), 1 deletion(-) create mode 100644 internal/adapters/oauthas/mock.go create mode 100644 internal/app/oauthlogin/service.go create mode 100644 internal/app/oauthlogin/service_test.go create mode 100644 internal/app/oauthlogin/store.go diff --git a/internal/adapters/oauthas/mock.go b/internal/adapters/oauthas/mock.go new file mode 100644 index 0000000..67e7cd7 --- /dev/null +++ b/internal/adapters/oauthas/mock.go @@ -0,0 +1,126 @@ +package oauthas + +import ( + "context" + "time" + + "github.com/nylas/cli/internal/domain" +) + +// MockClient is a configurable ports.OAuthAuthServerClient for tests. +// Unset function fields fall back to a working default so a test only has to +// state the behaviour it cares about. +type MockClient struct { + MetadataFunc func(ctx context.Context) (*domain.OAuthServerMetadata, error) + RegisterFunc func(ctx context.Context, req domain.OAuthClientRegistrationRequest) (*domain.OAuthClientRegistration, error) + AuthorizationURLFunc func(ctx context.Context, params domain.OAuthAuthorizationParams) (string, error) + ExchangeCodeFunc func(ctx context.Context, params domain.OAuthCodeExchange) (*domain.OAuthTokens, error) + RefreshFunc func(ctx context.Context, clientID, refreshToken string) (*domain.OAuthTokens, error) + RevokeFunc func(ctx context.Context, clientID, token string) error + UserInfoFunc func(ctx context.Context, accessToken string) (*domain.OAuthUserInfo, error) + + // Now backs the ExpiresAt the default token responses carry, mirroring + // what the real client derives from expires_in. Tests with a fixed clock + // set this to the same clock as the service under test. + Now func() time.Time + + // Recorded calls. + RegisterRequests []domain.OAuthClientRegistrationRequest + AuthorizationCalls []domain.OAuthAuthorizationParams + ExchangeCalls []domain.OAuthCodeExchange + RefreshCalls []string + RevokeCalls []string +} + +// MockIssuer is the issuer MockClient advertises by default. +const MockIssuer = "https://auth.example.test" + +func (m *MockClient) Metadata(ctx context.Context) (*domain.OAuthServerMetadata, error) { + if m.MetadataFunc != nil { + return m.MetadataFunc(ctx) + } + return &domain.OAuthServerMetadata{ + Issuer: MockIssuer, + AuthorizationEndpoint: MockIssuer + "/oauth/authorize", + TokenEndpoint: MockIssuer + "/oauth/token", + UserInfoEndpoint: MockIssuer + "/oauth/userinfo", + RevocationEndpoint: MockIssuer + "/oauth/revoke", + RegistrationEndpoint: MockIssuer + "/oauth/register", + CodeChallengeMethodsSupported: []string{"S256"}, + }, nil +} + +func (m *MockClient) Register(ctx context.Context, req domain.OAuthClientRegistrationRequest) (*domain.OAuthClientRegistration, error) { + m.RegisterRequests = append(m.RegisterRequests, req) + if m.RegisterFunc != nil { + return m.RegisterFunc(ctx, req) + } + return &domain.OAuthClientRegistration{ + ClientID: "mock-client-id", + ClientName: req.ClientName, + RedirectURIs: req.RedirectURIs, + TokenEndpointAuthMethod: "none", + }, nil +} + +func (m *MockClient) AuthorizationURL(ctx context.Context, params domain.OAuthAuthorizationParams) (string, error) { + m.AuthorizationCalls = append(m.AuthorizationCalls, params) + if m.AuthorizationURLFunc != nil { + return m.AuthorizationURLFunc(ctx, params) + } + return MockIssuer + "/oauth/authorize?state=" + params.State, nil +} + +func (m *MockClient) ExchangeCode(ctx context.Context, params domain.OAuthCodeExchange) (*domain.OAuthTokens, error) { + m.ExchangeCalls = append(m.ExchangeCalls, params) + if m.ExchangeCodeFunc != nil { + return m.ExchangeCodeFunc(ctx, params) + } + return &domain.OAuthTokens{ + AccessToken: "mock-access-token", + TokenType: "Bearer", + ExpiresIn: 3600, + ExpiresAt: m.expiry(3600), + RefreshToken: "mock-refresh-token", + IDToken: "mock-id-token", + Scope: "openid email offline_access", + }, nil +} + +func (m *MockClient) Refresh(ctx context.Context, clientID, refreshToken string) (*domain.OAuthTokens, error) { + m.RefreshCalls = append(m.RefreshCalls, refreshToken) + if m.RefreshFunc != nil { + return m.RefreshFunc(ctx, clientID, refreshToken) + } + return &domain.OAuthTokens{ + AccessToken: "mock-access-token-2", + TokenType: "Bearer", + ExpiresIn: 3600, + ExpiresAt: m.expiry(3600), + RefreshToken: "mock-refresh-token-2", + Scope: "openid email offline_access", + }, nil +} + +func (m *MockClient) expiry(seconds int) time.Time { + now := time.Now + if m.Now != nil { + now = m.Now + } + return now().Add(time.Duration(seconds) * time.Second) +} + +func (m *MockClient) Revoke(ctx context.Context, clientID, token string) error { + m.RevokeCalls = append(m.RevokeCalls, token) + if m.RevokeFunc != nil { + return m.RevokeFunc(ctx, clientID, token) + } + return nil +} + +func (m *MockClient) UserInfo(ctx context.Context, accessToken string) (*domain.OAuthUserInfo, error) { + if m.UserInfoFunc != nil { + return m.UserInfoFunc(ctx, accessToken) + } + return &domain.OAuthUserInfo{Subject: "user-1", Email: "dev@example.test", EmailVerified: true}, nil +} diff --git a/internal/app/oauthlogin/service.go b/internal/app/oauthlogin/service.go new file mode 100644 index 0000000..79e5784 --- /dev/null +++ b/internal/app/oauthlogin/service.go @@ -0,0 +1,252 @@ +// Package oauthlogin drives the OAuth 2.1 authorization code flow against +// the Nylas authorization server hosted by dashboard-account. +package oauthlogin + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" +) + +// clientName identifies this client on the server's consent screen. +const clientName = "Nylas CLI" + +// loopbackRegistrationURI is the redirect URI the CLI registers. +// +// RFC 8252 section 7.3 lets the server free the PORT of a loopback redirect +// URI at request time, so the ephemeral port the callback server picks still +// matches this registration. Nothing else is relaxed — the host must match +// exactly, and the server never treats "localhost" and "127.0.0.1" as the +// same, so this has to spell the host the callback server advertises. +const loopbackRegistrationURI = "http://localhost/callback" + +// Service performs authorization server logins and owns the stored session. +type Service struct { + client ports.OAuthAuthServerClient + server ports.OAuthServer + browser ports.Browser + secrets ports.SecretStore + now func() time.Time +} + +// NewService creates a login service. +func NewService( + client ports.OAuthAuthServerClient, + server ports.OAuthServer, + browser ports.Browser, + secrets ports.SecretStore, +) *Service { + return &Service{ + client: client, + server: server, + browser: browser, + secrets: secrets, + now: time.Now, + } +} + +// LoginResult summarises a completed login. +type LoginResult struct { + Issuer string + ClientID string + Scope string + ExpiresAt time.Time + HasRefresh bool +} + +// Login runs the browser authorization code flow and stores the tokens. +func (s *Service) Login(ctx context.Context, scopes []string) (*LoginResult, error) { + if len(scopes) == 0 { + scopes = domain.DefaultOAuthScopes() + } + + metadata, err := s.client.Metadata(ctx) + if err != nil { + return nil, err + } + + clientID, err := s.resolveClientID(ctx, metadata.Issuer, scopes) + if err != nil { + return nil, err + } + + if err := s.server.Start(); err != nil { + return nil, err + } + defer func() { _ = s.server.Stop() }() + + state, err := domain.NewOAuthState() + if err != nil { + return nil, err + } + nonce, err := domain.NewOAuthState() + if err != nil { + return nil, err + } + pkce, err := domain.NewPKCE() + if err != nil { + return nil, err + } + + redirectURI := s.server.GetRedirectURI() + authURL, err := s.client.AuthorizationURL(ctx, domain.OAuthAuthorizationParams{ + ClientID: clientID, + RedirectURI: redirectURI, + Scopes: scopes, + State: state, + CodeChallenge: pkce.Challenge, + Nonce: nonce, + }) + if err != nil { + return nil, err + } + + // Listen before the browser is opened: a fast redirect would otherwise + // arrive before anything is waiting for it. + type callbackResult struct { + code string + err error + } + callbackCh := make(chan callbackResult, 1) + waitCtx, cancelWait := context.WithCancel(ctx) + defer cancelWait() + go func() { + code, waitErr := s.server.WaitForCallback(waitCtx, state) + callbackCh <- callbackResult{code: code, err: waitErr} + }() + + if err := s.browser.Open(authURL); err != nil { + return nil, fmt.Errorf("failed to open browser: %w", err) + } + + callback := <-callbackCh + if callback.err != nil { + return nil, callback.err + } + + tokens, err := s.client.ExchangeCode(ctx, domain.OAuthCodeExchange{ + ClientID: clientID, + Code: callback.code, + RedirectURI: redirectURI, + CodeVerifier: pkce.Verifier, + }) + if err != nil { + return nil, err + } + + if err := s.saveTokens(metadata.Issuer, clientID, tokens); err != nil { + return nil, err + } + + return &LoginResult{ + Issuer: metadata.Issuer, + ClientID: clientID, + Scope: tokens.Scope, + ExpiresAt: tokens.ExpiresAt, + HasRefresh: tokens.RefreshToken != "", + }, nil +} + +// resolveClientID reuses the stored registration when it belongs to this +// issuer, and registers a new public client otherwise. +func (s *Service) resolveClientID(ctx context.Context, issuer string, scopes []string) (string, error) { + storedIssuer, err := s.getSecret(ports.KeyOAuthIssuer) + if err != nil { + return "", err + } + storedClientID, err := s.getSecret(ports.KeyOAuthClientID) + if err != nil { + return "", err + } + if storedClientID != "" && storedIssuer == issuer { + return storedClientID, nil + } + + registration, err := s.client.Register(ctx, domain.OAuthClientRegistrationRequest{ + ClientName: clientName, + RedirectURIs: []string{loopbackRegistrationURI}, + TokenEndpointAuthMethod: "none", + GrantTypes: []string{"authorization_code", "refresh_token"}, + ResponseTypes: []string{"code"}, + Scope: strings.Join(scopes, " "), + }) + if err != nil { + return "", err + } + + if err := s.saveClientRegistration(issuer, registration.ClientID); err != nil { + return "", err + } + return registration.ClientID, nil +} + +// AccessToken returns a usable access token, refreshing it when it has expired. +func (s *Service) AccessToken(ctx context.Context) (string, error) { + session, err := s.loadSession() + if err != nil { + return "", err + } + if !session.Tokens.IsExpired(s.now()) { + return session.Tokens.AccessToken, nil + } + if session.Tokens.RefreshToken == "" { + return "", fmt.Errorf("%w: run `nylas oauth login` again", domain.ErrOAuthNoRefreshToken) + } + + tokens, err := s.client.Refresh(ctx, session.ClientID, session.Tokens.RefreshToken) + if err != nil { + return "", err + } + + // The server rotates the refresh token on every use and burns the whole + // family if an old one reappears. Persist exactly what came back — never + // carry the previous refresh token forward to fill an empty field. + if err := s.saveTokens(session.Issuer, session.ClientID, tokens); err != nil { + return "", err + } + return tokens.AccessToken, nil +} + +// Status returns the stored session without contacting the server. +func (s *Service) Status() (*Session, error) { + return s.loadSession() +} + +// UserInfo returns the OIDC claims for the current session. +func (s *Service) UserInfo(ctx context.Context) (*domain.OAuthUserInfo, error) { + accessToken, err := s.AccessToken(ctx) + if err != nil { + return nil, err + } + return s.client.UserInfo(ctx, accessToken) +} + +// Logout revokes the session's refresh token and clears local state. +// +// Revocation is best effort: an unreachable server must not leave tokens on +// disk, since the local copy is the thing the user asked to be rid of. +func (s *Service) Logout(ctx context.Context) error { + session, err := s.loadSession() + if err != nil { + if errors.Is(err, domain.ErrOAuthNotLoggedIn) { + return s.clearSession() + } + return err + } + + token := session.Tokens.RefreshToken + if token == "" { + token = session.Tokens.AccessToken + } + revokeErr := s.client.Revoke(ctx, session.ClientID, token) + + if err := s.clearSession(); err != nil { + return err + } + return revokeErr +} diff --git a/internal/app/oauthlogin/service_test.go b/internal/app/oauthlogin/service_test.go new file mode 100644 index 0000000..1d07e6c --- /dev/null +++ b/internal/app/oauthlogin/service_test.go @@ -0,0 +1,346 @@ +//go:build !integration + +package oauthlogin + +import ( + "context" + "crypto/sha256" + "encoding/base64" + "errors" + "testing" + "time" + + "github.com/nylas/cli/internal/adapters/keyring" + "github.com/nylas/cli/internal/adapters/oauth" + "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 mockBrowser struct { + openedURL string + err error +} + +func (m *mockBrowser) Open(url string) error { + m.openedURL = url + return m.err +} + +type fixture struct { + service *Service + client *oauthas.MockClient + browser *mockBrowser + secrets *keyring.MockSecretStore + server *oauth.MockServer + clock time.Time +} + +// advance moves the shared clock the service and the fake server both read, +// so a token minted before the jump is genuinely stale afterwards. +func (f *fixture) advance(d time.Duration) { + f.clock = f.clock.Add(d) +} + +func newFixture(t *testing.T) *fixture { + t.Helper() + + f := &fixture{ + client: &oauthas.MockClient{}, + browser: &mockBrowser{}, + secrets: keyring.NewMockSecretStore(), + server: oauth.NewMockServer("auth-code-1"), + clock: time.Date(2026, 9, 21, 12, 0, 0, 0, time.UTC), + } + + now := func() time.Time { return f.clock } + f.client.Now = now + f.service = NewService(f.client, f.server, f.browser, f.secrets) + f.service.now = now + + return f +} + +func TestLogin_StoresTokensAndReportsSession(t *testing.T) { + f := newFixture(t) + + result, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + assert.Equal(t, oauthas.MockIssuer, result.Issuer) + assert.Equal(t, "mock-client-id", result.ClientID) + assert.True(t, result.HasRefresh) + + stored := f.secrets.GetAll() + assert.Equal(t, "mock-access-token", stored[ports.KeyOAuthAccessToken]) + assert.Equal(t, "mock-refresh-token", stored[ports.KeyOAuthRefreshToken]) + assert.Equal(t, "mock-id-token", stored[ports.KeyOAuthIDToken]) + assert.Equal(t, oauthas.MockIssuer, stored[ports.KeyOAuthIssuer]) +} + +func TestLogin_RegistersAsPublicLoopbackClient(t *testing.T) { + f := newFixture(t) + + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + require.Len(t, f.client.RegisterRequests, 1) + req := f.client.RegisterRequests[0] + + // "none" makes it a public client: the CLI cannot keep a secret, and the + // server only admits a secretless token request from a client registered + // this way. + assert.Equal(t, "none", req.TokenEndpointAuthMethod) + // Registered without a port because the callback port is assigned at run + // time; RFC 8252 lets the server free the port for loopback URIs. + assert.Equal(t, []string{"http://localhost/callback"}, req.RedirectURIs) +} + +func TestLogin_SendsRFCCompliantPKCEChallenge(t *testing.T) { + f := newFixture(t) + + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + require.Len(t, f.client.AuthorizationCalls, 1) + require.Len(t, f.client.ExchangeCalls, 1) + + challenge := f.client.AuthorizationCalls[0].CodeChallenge + verifier := f.client.ExchangeCalls[0].CodeVerifier + + // The verifier sent to the token endpoint must be the pre-image of the + // challenge sent to the authorization endpoint, or the exchange fails. + sum := sha256.Sum256([]byte(verifier)) + assert.Equal(t, base64.RawURLEncoding.EncodeToString(sum[:]), challenge) + assert.Regexp(t, `^[A-Za-z0-9\-_]{43}$`, challenge) +} + +func TestLogin_BindsCallbackStateToAuthorizationRequest(t *testing.T) { + f := newFixture(t) + + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + sentState := f.client.AuthorizationCalls[0].State + assert.NotEmpty(t, sentState) + assert.Equal(t, sentState, f.server.ExpectedState, + "the callback server must reject any state but the one we sent") +} + +func TestLogin_ExchangesAgainstTheSameRedirectURI(t *testing.T) { + f := newFixture(t) + + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + // The server compares the redirect_uri at the token endpoint against the + // one in the authorization request; a mismatch is invalid_grant. + assert.Equal(t, f.client.AuthorizationCalls[0].RedirectURI, f.client.ExchangeCalls[0].RedirectURI) + assert.Equal(t, f.server.GetRedirectURI(), f.client.ExchangeCalls[0].RedirectURI) +} + +func TestLogin_OpensBrowserAndStopsServer(t *testing.T) { + f := newFixture(t) + + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + assert.NotEmpty(t, f.browser.openedURL) + assert.True(t, f.server.StartCalled) + assert.True(t, f.server.StopCalled) +} + +func TestLogin_RequestsOfflineAccessByDefault(t *testing.T) { + f := newFixture(t) + + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + assert.Contains(t, f.client.AuthorizationCalls[0].Scopes, domain.OAuthScopeOfflineAccess) +} + +func TestLogin_ReusesStoredClientForSameIssuer(t *testing.T) { + f := newFixture(t) + require.NoError(t, f.secrets.Set(ports.KeyOAuthIssuer, oauthas.MockIssuer)) + require.NoError(t, f.secrets.Set(ports.KeyOAuthClientID, "existing-client")) + + result, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + assert.Empty(t, f.client.RegisterRequests, "an existing registration must not be duplicated") + assert.Equal(t, "existing-client", result.ClientID) +} + +func TestLogin_ReregistersWhenIssuerChanged(t *testing.T) { + // A dev tunnel URL changes between sessions, and a client_id registered + // against the old issuer does not exist on the new one. + f := newFixture(t) + require.NoError(t, f.secrets.Set(ports.KeyOAuthIssuer, "https://old-tunnel.example.test")) + require.NoError(t, f.secrets.Set(ports.KeyOAuthClientID, "stale-client")) + + result, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + assert.Len(t, f.client.RegisterRequests, 1) + assert.Equal(t, "mock-client-id", result.ClientID) +} + +func TestLogin_FailsWhenCallbackFails(t *testing.T) { + f := newFixture(t) + f.server.AuthCode = "" + + _, err := f.service.Login(context.Background(), nil) + + require.ErrorIs(t, err, domain.ErrAuthFailed) + assert.Empty(t, f.secrets.GetAll()[ports.KeyOAuthAccessToken]) +} + +func TestAccessToken_ReturnsStoredTokenWhileValid(t *testing.T) { + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + token, err := f.service.AccessToken(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "mock-access-token", token) + assert.Empty(t, f.client.RefreshCalls) +} + +func TestAccessToken_RefreshesWhenExpired(t *testing.T) { + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + f.advance(2 * time.Hour) + + token, err := f.service.AccessToken(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "mock-access-token-2", token) + assert.Equal(t, []string{"mock-refresh-token"}, f.client.RefreshCalls) +} + +func TestAccessToken_PersistsRotatedRefreshToken(t *testing.T) { + // The server rotates on every use and burns the family if an old token + // reappears, so the new one must land in the store before it is needed. + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + f.advance(2 * time.Hour) + + _, err = f.service.AccessToken(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "mock-refresh-token-2", f.secrets.GetAll()[ports.KeyOAuthRefreshToken]) +} + +func TestAccessToken_DoesNotResurrectOldRefreshTokenWhenServerOmitsOne(t *testing.T) { + // Carrying the previous refresh token forward would replay a token the + // server has already consumed, which revokes the entire family. + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + f.advance(2 * time.Hour) + f.client.RefreshFunc = func(context.Context, string, string) (*domain.OAuthTokens, error) { + return &domain.OAuthTokens{AccessToken: "at-only", TokenType: "Bearer", ExpiresIn: 3600}, nil + } + + _, err = f.service.AccessToken(context.Background()) + require.NoError(t, err) + + assert.Empty(t, f.secrets.GetAll()[ports.KeyOAuthRefreshToken]) +} + +func TestAccessToken_WithoutSession(t *testing.T) { + f := newFixture(t) + + _, err := f.service.AccessToken(context.Background()) + + require.ErrorIs(t, err, domain.ErrOAuthNotLoggedIn) +} + +func TestAccessToken_ExpiredWithNoRefreshToken(t *testing.T) { + f := newFixture(t) + f.client.ExchangeCodeFunc = func(context.Context, domain.OAuthCodeExchange) (*domain.OAuthTokens, error) { + return &domain.OAuthTokens{AccessToken: "at-1", TokenType: "Bearer", ExpiresIn: 3600}, nil + } + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + f.advance(2 * time.Hour) + + _, err = f.service.AccessToken(context.Background()) + + require.ErrorIs(t, err, domain.ErrOAuthNoRefreshToken) +} + +func TestStatus_ReportsStoredSession(t *testing.T) { + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + session, err := f.service.Status() + require.NoError(t, err) + + assert.Equal(t, oauthas.MockIssuer, session.Issuer) + assert.Equal(t, "mock-client-id", session.ClientID) + assert.Equal(t, "openid email offline_access", session.Tokens.Scope) + assert.False(t, session.Tokens.IsExpired(f.clock)) +} + +func TestLogout_RevokesRefreshTokenAndClearsEverything(t *testing.T) { + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + require.NoError(t, f.service.Logout(context.Background())) + + // Revoking the refresh token takes the whole family with it; revoking + // only the access token would leave the grant alive. + assert.Equal(t, []string{"mock-refresh-token"}, f.client.RevokeCalls) + for _, key := range sessionKeys { + assert.NotContains(t, f.secrets.GetAll(), key) + } +} + +func TestLogout_ClearsLocalStateWhenRevocationFails(t *testing.T) { + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + f.client.RevokeFunc = func(context.Context, string, string) error { + return errors.New("server unreachable") + } + + err = f.service.Logout(context.Background()) + + require.Error(t, err, "the revocation failure is still reported") + assert.NotContains(t, f.secrets.GetAll(), ports.KeyOAuthAccessToken, + "local tokens must be gone regardless") +} + +func TestLogout_WithoutSessionSucceeds(t *testing.T) { + f := newFixture(t) + + require.NoError(t, f.service.Logout(context.Background())) + assert.Empty(t, f.client.RevokeCalls) +} + +func TestUserInfo_UsesCurrentAccessToken(t *testing.T) { + f := newFixture(t) + _, err := f.service.Login(context.Background(), nil) + require.NoError(t, err) + + var seen string + f.client.UserInfoFunc = func(_ context.Context, accessToken string) (*domain.OAuthUserInfo, error) { + seen = accessToken + return &domain.OAuthUserInfo{Subject: "user-1", Email: "dev@example.test"}, nil + } + + info, err := f.service.UserInfo(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "mock-access-token", seen) + assert.Equal(t, "dev@example.test", info.Email) +} diff --git a/internal/app/oauthlogin/store.go b/internal/app/oauthlogin/store.go new file mode 100644 index 0000000..60be78d --- /dev/null +++ b/internal/app/oauthlogin/store.go @@ -0,0 +1,146 @@ +package oauthlogin + +import ( + "errors" + "fmt" + "time" + + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" +) + +// sessionKeys is every secret this package owns. Clearing the set is what +// logout means, so a new key must be added here or it outlives the session. +var sessionKeys = []string{ + ports.KeyOAuthIssuer, + ports.KeyOAuthClientID, + ports.KeyOAuthAccessToken, + ports.KeyOAuthRefreshToken, + ports.KeyOAuthIDToken, + ports.KeyOAuthExpiresAt, + ports.KeyOAuthScope, +} + +// Session is a stored authorization server login. +type Session struct { + Issuer string + ClientID string + Tokens domain.OAuthTokens +} + +func (s *Service) loadSession() (*Session, error) { + accessToken, err := s.getSecret(ports.KeyOAuthAccessToken) + if err != nil { + return nil, err + } + if accessToken == "" { + return nil, domain.ErrOAuthNotLoggedIn + } + + session := &Session{Tokens: domain.OAuthTokens{AccessToken: accessToken, TokenType: "Bearer"}} + for key, target := range map[string]*string{ + ports.KeyOAuthIssuer: &session.Issuer, + ports.KeyOAuthClientID: &session.ClientID, + ports.KeyOAuthRefreshToken: &session.Tokens.RefreshToken, + ports.KeyOAuthIDToken: &session.Tokens.IDToken, + ports.KeyOAuthScope: &session.Tokens.Scope, + } { + value, err := s.getSecret(key) + if err != nil { + return nil, err + } + *target = value + } + + expiresAt, err := s.getSecret(ports.KeyOAuthExpiresAt) + if err != nil { + return nil, err + } + if expiresAt != "" { + parsed, err := time.Parse(time.RFC3339, expiresAt) + if err != nil { + // A zero ExpiresAt reads as expired, so an unparseable value + // triggers a refresh rather than failing the command. + parsed = time.Time{} + } + session.Tokens.ExpiresAt = parsed + } + + return session, nil +} + +// saveTokens persists a token set. The access token is written last so a +// partial write cannot leave a session that looks complete but carries a +// refresh token belonging to a different exchange. +func (s *Service) saveTokens(issuer, clientID string, tokens *domain.OAuthTokens) error { + expiresAt := "" + if !tokens.ExpiresAt.IsZero() { + expiresAt = tokens.ExpiresAt.UTC().Format(time.RFC3339) + } + + ordered := []struct { + key string + value string + }{ + {ports.KeyOAuthIssuer, issuer}, + {ports.KeyOAuthClientID, clientID}, + {ports.KeyOAuthRefreshToken, tokens.RefreshToken}, + {ports.KeyOAuthIDToken, tokens.IDToken}, + {ports.KeyOAuthScope, tokens.Scope}, + {ports.KeyOAuthExpiresAt, expiresAt}, + {ports.KeyOAuthAccessToken, tokens.AccessToken}, + } + + for _, entry := range ordered { + if entry.value == "" { + if err := s.deleteSecret(entry.key); err != nil { + return err + } + continue + } + if err := s.secrets.Set(entry.key, entry.value); err != nil { + return fmt.Errorf("failed to store %s: %w", entry.key, err) + } + } + return nil +} + +// saveClientRegistration records the dynamically registered client against +// the issuer it belongs to, so a later login can reuse it. +func (s *Service) saveClientRegistration(issuer, clientID string) error { + if err := s.secrets.Set(ports.KeyOAuthIssuer, issuer); err != nil { + return fmt.Errorf("failed to store %s: %w", ports.KeyOAuthIssuer, err) + } + if err := s.secrets.Set(ports.KeyOAuthClientID, clientID); err != nil { + return fmt.Errorf("failed to store %s: %w", ports.KeyOAuthClientID, err) + } + return nil +} + +func (s *Service) clearSession() error { + var errs []error + for _, key := range sessionKeys { + if err := s.deleteSecret(key); err != nil { + errs = append(errs, err) + } + } + return errors.Join(errs...) +} + +func (s *Service) getSecret(key string) (string, error) { + value, err := s.secrets.Get(key) + if err != nil { + if errors.Is(err, domain.ErrSecretNotFound) { + return "", nil + } + return "", fmt.Errorf("failed to read %s: %w", key, err) + } + return value, nil +} + +func (s *Service) deleteSecret(key string) error { + if err := s.secrets.Delete(key); err != nil && !errors.Is(err, domain.ErrSecretNotFound) { + return fmt.Errorf("failed to clear %s: %w", key, err) + } + return nil +} diff --git a/internal/ports/oauthas.go b/internal/ports/oauthas.go index dc81018..127d800 100644 --- a/internal/ports/oauthas.go +++ b/internal/ports/oauthas.go @@ -25,7 +25,9 @@ type OAuthAuthServerClient interface { // PKCE authorization code flow. AuthorizationURL(ctx context.Context, params domain.OAuthAuthorizationParams) (string, error) - // ExchangeCode redeems an authorization code for a token set. + // ExchangeCode redeems an authorization code for a token set. The + // returned tokens carry an absolute ExpiresAt derived from expires_in, + // which is what callers check before reusing an access token. ExchangeCode(ctx context.Context, params domain.OAuthCodeExchange) (*domain.OAuthTokens, error) // Refresh exchanges a refresh token for a new token set. The server diff --git a/internal/ports/secrets.go b/internal/ports/secrets.go index 33ba6d6..f461adc 100644 --- a/internal/ports/secrets.go +++ b/internal/ports/secrets.go @@ -34,4 +34,15 @@ const ( KeyDashboardDPoPKey = "dashboard_dpop_key" KeyDashboardAppID = "dashboard_app_id" KeyDashboardAppRegion = "dashboard_app_region" + + // OAuth authorization server keys. KeyOAuthIssuer pins the other values + // to the server that produced them: a dynamically registered client_id is + // meaningless against a different issuer, and a dev tunnel URL changes often. + KeyOAuthIssuer = "oauth_issuer" + KeyOAuthClientID = "oauth_client_id" + KeyOAuthAccessToken = "oauth_access_token" + KeyOAuthRefreshToken = "oauth_refresh_token" + KeyOAuthIDToken = "oauth_id_token" + KeyOAuthExpiresAt = "oauth_expires_at" + KeyOAuthScope = "oauth_scope" ) From f55fcc2ea3ec858ae5050a8fe2917b4e46208cfd Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Mon, 21 Sep 2026 14:45:49 +0200 Subject: [PATCH 3/5] TW-6922: add nylas oauth login, status, token and logout commands Stage 1c: the user-facing commands, wired into the root command. Named as its own subtree rather than folded into `nylas auth`, which in this CLI means connecting an end user's mailbox as a provider grant, or `nylas dashboard login`, which opens a dashboard management session. This authenticates the person running the CLI. The authorization server is hosted by dashboard-account, so the base URL resolution is shared rather than duplicated: getDashboardAccountBaseURL is now exported as dashboard.AccountBaseURL and loses a parameter it never read. NYLAS_DASHBOARD_ACCOUNT_URL therefore points both command groups at a local server. `oauth token` prints the bare token so it can be substituted into a curl header, and `oauth status` deliberately never prints the token itself. That file needs `git add -f`: .gitignore has a broad `*token*` rule, and the two token.go files already tracked were added the same way. Co-Authored-By: Claude Opus 5 --- cmd/nylas/main.go | 2 + docs/COMMANDS.md | 47 +++++ internal/cli/dashboard/dashboard_test.go | 2 +- internal/cli/dashboard/helpers.go | 11 +- internal/cli/oauth/helpers.go | 68 +++++++ internal/cli/oauth/login.go | 68 +++++++ internal/cli/oauth/logout.go | 34 ++++ internal/cli/oauth/oauth.go | 40 ++++ internal/cli/oauth/oauth_test.go | 239 +++++++++++++++++++++++ internal/cli/oauth/status.go | 84 ++++++++ internal/cli/oauth/token.go | 39 ++++ 11 files changed, 629 insertions(+), 5 deletions(-) create mode 100644 internal/cli/oauth/helpers.go create mode 100644 internal/cli/oauth/login.go create mode 100644 internal/cli/oauth/logout.go create mode 100644 internal/cli/oauth/oauth.go create mode 100644 internal/cli/oauth/oauth_test.go create mode 100644 internal/cli/oauth/status.go create mode 100644 internal/cli/oauth/token.go diff --git a/cmd/nylas/main.go b/cmd/nylas/main.go index de16fd9..3e5b4dd 100644 --- a/cmd/nylas/main.go +++ b/cmd/nylas/main.go @@ -20,6 +20,7 @@ import ( "github.com/nylas/cli/internal/cli/email" "github.com/nylas/cli/internal/cli/mcp" "github.com/nylas/cli/internal/cli/notetaker" + oauthcmd "github.com/nylas/cli/internal/cli/oauth" "github.com/nylas/cli/internal/cli/otp" "github.com/nylas/cli/internal/cli/rpc" "github.com/nylas/cli/internal/cli/scheduler" @@ -48,6 +49,7 @@ func main() { rootCmd.AddCommand(calendar.NewCalendarCmd()) rootCmd.AddCommand(contacts.NewContactsCmd()) rootCmd.AddCommand(dashboard.NewDashboardCmd()) + rootCmd.AddCommand(oauthcmd.NewOAuthCmd()) rootCmd.AddCommand(setup.NewSetupCmd()) rootCmd.AddCommand(scheduler.NewSchedulerCmd()) rootCmd.AddCommand(admin.NewAdminCmd()) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 4874536..3b8d7e7 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -113,6 +113,53 @@ nylas auth migrate # Migrate from v2 to v3 --- +## OAuth (Authorization Server) + +Log in to the Nylas OAuth 2.1 / OIDC authorization server. This authenticates +**you**, the person running the CLI, and is distinct from `nylas auth` (which +connects an end user's mailbox as a provider grant) and from +`nylas dashboard login` (which opens a dashboard management session). + +```bash +nylas oauth login # Log in via the browser (authorization code + PKCE) +nylas oauth login --scope openid,email +nylas oauth status # Show the stored session +nylas oauth status --verify # Also confirm the token against /oauth/userinfo +nylas oauth token # Print a valid access token, refreshing if needed +nylas oauth logout # Revoke the session and clear stored tokens +``` + +The CLI registers itself as a public client via RFC 7591 dynamic registration +the first time it runs, and stores the tokens in the system keyring. + +Default scopes are `openid`, `email` and `offline_access`. `offline_access` is +what makes the server issue a refresh token; without it the session ends when +the access token expires (one hour). + +Use the access token with any OAuth-protected endpoint: + +```bash +curl -H "Authorization: Bearer $(nylas oauth token)" https://example/resource +``` + +### Pointing at a local authorization server + +The authorization server is hosted by `dashboard-account`, so it uses the same +base URL as the `nylas dashboard` commands: + +```bash +NYLAS_DASHBOARD_ACCOUNT_URL=http://localhost:3001 nylas oauth login +``` + +The CLI resolves every endpoint from the server's +`/.well-known/oauth-authorization-server` document, and that document is built +from the server's `OAUTH_ISSUER`. If `OAUTH_ISSUER` names a host the CLI cannot +reach (for example a Cloudflare tunnel that is no longer running), login fails +even though the local port responds — set `OAUTH_ISSUER` to the address you +actually browse to. + +--- + ## Dashboard Manage your Nylas Dashboard account, applications, domains, and API keys directly from the CLI. diff --git a/internal/cli/dashboard/dashboard_test.go b/internal/cli/dashboard/dashboard_test.go index 61cf133..14d3c91 100644 --- a/internal/cli/dashboard/dashboard_test.go +++ b/internal/cli/dashboard/dashboard_test.go @@ -251,7 +251,7 @@ func TestGetDashboardAccountBaseURL(t *testing.T) { }() require.NoError(t, os.Setenv("NYLAS_DASHBOARD_ACCOUNT_URL", "https://dashboard.example.com")) - assert.Equal(t, "https://dashboard.example.com", getDashboardAccountBaseURL(nil)) + assert.Equal(t, "https://dashboard.example.com", AccountBaseURL()) } func TestMapProvider(t *testing.T) { diff --git a/internal/cli/dashboard/helpers.go b/internal/cli/dashboard/helpers.go index 31ebf84..ea339b6 100644 --- a/internal/cli/dashboard/helpers.go +++ b/internal/cli/dashboard/helpers.go @@ -52,7 +52,7 @@ func createAuthService() (*dashboardapp.AuthService, ports.SecretStore, error) { return nil, nil, err } - baseURL := getDashboardAccountBaseURL(secretStore) + baseURL := AccountBaseURL() accountClient := dashboard.NewAccountClient(baseURL, dpopSvc) return dashboardapp.NewAuthService(accountClient, secretStore), secretStore, nil @@ -83,14 +83,17 @@ func newDomainService() (*dashboardapp.DomainService, error) { return nil, err } - baseURL := getDashboardAccountBaseURL(secretStore) + baseURL := AccountBaseURL() accountClient := dashboard.NewAccountClient(baseURL, dpopSvc) return dashboardapp.NewDomainService(accountClient, secretStore), nil } -// getDashboardAccountBaseURL returns the dashboard-account base URL. +// AccountBaseURL returns the dashboard-account base URL. // Priority: NYLAS_DASHBOARD_ACCOUNT_URL env var > config file > default. -func getDashboardAccountBaseURL(secrets ports.SecretStore) string { +// +// Exported because the OAuth authorization server is hosted by the same +// service, so `nylas oauth` must resolve the same address. +func AccountBaseURL() string { if envURL := os.Getenv("NYLAS_DASHBOARD_ACCOUNT_URL"); envURL != "" { return envURL } diff --git a/internal/cli/oauth/helpers.go b/internal/cli/oauth/helpers.go new file mode 100644 index 0000000..444a156 --- /dev/null +++ b/internal/cli/oauth/helpers.go @@ -0,0 +1,68 @@ +package oauth + +import ( + "context" + "errors" + + "github.com/nylas/cli/internal/adapters/browser" + "github.com/nylas/cli/internal/adapters/config" + "github.com/nylas/cli/internal/adapters/keyring" + oauthadapter "github.com/nylas/cli/internal/adapters/oauth" + "github.com/nylas/cli/internal/adapters/oauthas" + "github.com/nylas/cli/internal/app/oauthlogin" + "github.com/nylas/cli/internal/cli/common" + "github.com/nylas/cli/internal/cli/dashboard" + "github.com/nylas/cli/internal/domain" +) + +// loginService is the slice of oauthlogin.Service the commands use, named so +// tests can substitute a fake via createLoginServiceFn. +type loginService interface { + Login(ctx context.Context, scopes []string) (*oauthlogin.LoginResult, error) + Status() (*oauthlogin.Session, error) + AccessToken(ctx context.Context) (string, error) + UserInfo(ctx context.Context) (*domain.OAuthUserInfo, error) + Logout(ctx context.Context) error +} + +var createLoginServiceFn = func() (loginService, error) { return createLoginService() } + +// createLoginService wires the OAuth login service. The authorization server +// is hosted by dashboard-account, so it resolves to the same base URL as the +// `nylas dashboard` commands (NYLAS_DASHBOARD_ACCOUNT_URL overrides it). +func createLoginService() (*oauthlogin.Service, error) { + secrets, err := keyring.NewSecretStore(config.DefaultConfigDir()) + if err != nil { + return nil, err + } + + cfg, _ := config.NewDefaultFileStore().Load() + callbackPort := 0 + if cfg != nil { + callbackPort = cfg.CallbackPort + } + + client := oauthas.NewClient(dashboard.AccountBaseURL()) + callbackServer := oauthadapter.NewCallbackServer(callbackPort) + + return oauthlogin.NewService(client, callbackServer, browser.NewDefaultBrowser(), secrets), nil +} + +// wrapOAuthError turns the not-logged-in sentinel into a CLI error that +// names the command to run, and passes everything else through. +func wrapOAuthError(err error) error { + if err == nil { + return nil + } + if errors.Is(err, domain.ErrOAuthNotLoggedIn) { + return &common.CLIError{ + Err: err, + Message: "Not logged in\n Hint: run `nylas oauth login`", + } + } + var cliErr *common.CLIError + if errors.As(err, &cliErr) { + return cliErr + } + return &common.CLIError{Err: err, Message: err.Error()} +} diff --git a/internal/cli/oauth/login.go b/internal/cli/oauth/login.go new file mode 100644 index 0000000..feaabb6 --- /dev/null +++ b/internal/cli/oauth/login.go @@ -0,0 +1,68 @@ +package oauth + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/nylas/cli/internal/cli/common" + "github.com/nylas/cli/internal/domain" +) + +func newLoginCmd() *cobra.Command { + var scopes []string + + 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. + +The CLI registers itself as a public client the first time it runs, opens +a browser for consent, and stores the tokens in the system keyring.`, + Example: ` # Log in with the default scopes (openid, email, offline_access) + nylas oauth login + + # Log in against a local authorization server + NYLAS_DASHBOARD_ACCOUNT_URL=http://localhost:3001 nylas oauth login`, + RunE: func(cmd *cobra.Command, _ []string) error { + svc, err := createLoginServiceFn() + if err != nil { + return wrapOAuthError(err) + } + + // The user has to read a consent screen, so this needs the long + // interactive timeout rather than the API one. + ctx, cancel := common.CreateLongContext() + defer cancel() + + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Opening your browser to complete sign-in...") + + result, err := svc.Login(ctx, scopes) + if err != nil { + return wrapOAuthError(err) + } + + out := cmd.OutOrStdout() + _, _ = common.Green.Fprintln(out, "✓ Logged in") + _, _ = fmt.Fprintf(out, " Issuer: %s\n", result.Issuer) + _, _ = fmt.Fprintf(out, " Client ID: %s\n", result.ClientID) + if result.Scope != "" { + _, _ = fmt.Fprintf(out, " Scopes: %s\n", result.Scope) + } + if !result.ExpiresAt.IsZero() { + _, _ = fmt.Fprintf(out, " Expires: %s\n", result.ExpiresAt.Local().Format("2006-01-02 15:04:05 MST")) + } + if !result.HasRefresh { + _, _ = common.Yellow.Fprintln(out, + " No refresh token issued — request the offline_access scope to stay signed in.") + } + + return nil + }, + } + + cmd.Flags().StringSliceVar(&scopes, "scope", nil, + fmt.Sprintf("OAuth scopes to request (default %v)", domain.DefaultOAuthScopes())) + + return cmd +} diff --git a/internal/cli/oauth/logout.go b/internal/cli/oauth/logout.go new file mode 100644 index 0000000..efecd87 --- /dev/null +++ b/internal/cli/oauth/logout.go @@ -0,0 +1,34 @@ +package oauth + +import ( + "github.com/spf13/cobra" + + "github.com/nylas/cli/internal/cli/common" +) + +func newLogoutCmd() *cobra.Command { + return &cobra.Command{ + Use: "logout", + Short: "Revoke the OAuth session and clear stored tokens", + Long: `Revoke the refresh token and remove the stored session. + +Revoking the refresh token takes the whole token family with it. Local +tokens are cleared even when the server cannot be reached.`, + RunE: func(cmd *cobra.Command, _ []string) error { + svc, err := createLoginServiceFn() + if err != nil { + return wrapOAuthError(err) + } + + ctx, cancel := common.CreateContext() + defer cancel() + + if err := svc.Logout(ctx); err != nil { + return wrapOAuthError(err) + } + + _, _ = common.Green.Fprintln(cmd.OutOrStdout(), "✓ Logged out") + return nil + }, + } +} diff --git a/internal/cli/oauth/oauth.go b/internal/cli/oauth/oauth.go new file mode 100644 index 0000000..0236540 --- /dev/null +++ b/internal/cli/oauth/oauth.go @@ -0,0 +1,40 @@ +// Package oauth provides CLI commands for logging in to the Nylas OAuth 2.1 +// authorization server. +// +// Distinct from `nylas auth`, which connects an end user's mailbox or +// calendar as a provider grant, and from `nylas dashboard login`, which opens +// a dashboard management session. This authenticates the person operating the +// CLI and yields an OIDC identity plus an access token. +package oauth + +import ( + "github.com/spf13/cobra" +) + +// NewOAuthCmd creates the oauth command group. +func NewOAuthCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "oauth", + Short: "Log in to the Nylas authorization server (OAuth 2.1 / OIDC)", + Long: `Authenticate with the Nylas OAuth 2.1 authorization server. + +Opens a browser to complete an authorization code flow with PKCE and +stores the resulting tokens in the system keyring. + +Commands: + login Log in via the browser + status Show the current OAuth session + token Print a valid access token, refreshing it if needed + logout Revoke the session and clear stored tokens + +The authorization server is hosted by dashboard-account; point the CLI at +a local one with NYLAS_DASHBOARD_ACCOUNT_URL.`, + } + + cmd.AddCommand(newLoginCmd()) + cmd.AddCommand(newStatusCmd()) + cmd.AddCommand(newTokenCmd()) + cmd.AddCommand(newLogoutCmd()) + + return cmd +} diff --git a/internal/cli/oauth/oauth_test.go b/internal/cli/oauth/oauth_test.go new file mode 100644 index 0000000..a508ea9 --- /dev/null +++ b/internal/cli/oauth/oauth_test.go @@ -0,0 +1,239 @@ +//go:build !integration + +package oauth + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/nylas/cli/internal/app/oauthlogin" + "github.com/nylas/cli/internal/cli/common" + "github.com/nylas/cli/internal/cli/testutil" + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakeService struct { + loginResult *oauthlogin.LoginResult + loginErr error + loginScopes []string + + session *oauthlogin.Session + statusErr error + + accessToken string + accessErr error + + userInfo *domain.OAuthUserInfo + userErr error + + logoutCalled bool + logoutErr error +} + +func (f *fakeService) Login(_ context.Context, scopes []string) (*oauthlogin.LoginResult, error) { + f.loginScopes = scopes + return f.loginResult, f.loginErr +} + +func (f *fakeService) Status() (*oauthlogin.Session, error) { return f.session, f.statusErr } + +func (f *fakeService) AccessToken(context.Context) (string, error) { + return f.accessToken, f.accessErr +} + +func (f *fakeService) UserInfo(context.Context) (*domain.OAuthUserInfo, error) { + return f.userInfo, f.userErr +} + +func (f *fakeService) Logout(context.Context) error { + f.logoutCalled = true + return f.logoutErr +} + +// withService swaps the command factory for the duration of one test. +func withService(t *testing.T, svc *fakeService) { + t.Helper() + original := createLoginServiceFn + createLoginServiceFn = func() (loginService, error) { return svc, nil } + t.Cleanup(func() { createLoginServiceFn = original }) +} + +func loggedInSession() *oauthlogin.Session { + return &oauthlogin.Session{ + Issuer: "https://auth.example.test", + ClientID: "client-1", + Tokens: domain.OAuthTokens{ + AccessToken: "at-1", + RefreshToken: "rt-1", + IDToken: "idt-1", + Scope: "openid email offline_access", + ExpiresAt: time.Now().Add(time.Hour), + }, + } +} + +func TestOAuthCmd_HasExpectedSubcommands(t *testing.T) { + cmd := NewOAuthCmd() + + names := []string{} + for _, sub := range cmd.Commands() { + names = append(names, sub.Name()) + } + + assert.ElementsMatch(t, []string{"login", "status", "token", "logout"}, names) +} + +func TestLoginCmd_ReportsSession(t *testing.T) { + svc := &fakeService{loginResult: &oauthlogin.LoginResult{ + Issuer: "https://auth.example.test", + ClientID: "client-1", + Scope: "openid email offline_access", + ExpiresAt: time.Now().Add(time.Hour), + HasRefresh: true, + }} + withService(t, svc) + + stdout, _, err := testutil.ExecuteSubCommand(newLoginCmd()) + require.NoError(t, err) + + assert.Contains(t, stdout, "Logged in") + assert.Contains(t, stdout, "https://auth.example.test") + assert.Contains(t, stdout, "client-1") +} + +func TestLoginCmd_WarnsWhenNoRefreshTokenIssued(t *testing.T) { + // Without a refresh token the session dies in an hour, and the user + // should know why before they hit it. + svc := &fakeService{loginResult: &oauthlogin.LoginResult{ + Issuer: "https://auth.example.test", + ClientID: "client-1", + Scope: "openid email", + HasRefresh: false, + }} + withService(t, svc) + + stdout, _, err := testutil.ExecuteSubCommand(newLoginCmd()) + require.NoError(t, err) + + assert.Contains(t, stdout, "offline_access") +} + +func TestLoginCmd_PassesScopeFlag(t *testing.T) { + svc := &fakeService{loginResult: &oauthlogin.LoginResult{Issuer: "i", ClientID: "c"}} + withService(t, svc) + + _, _, err := testutil.ExecuteSubCommand(newLoginCmd(), "--scope", "openid,email") + require.NoError(t, err) + + assert.Equal(t, []string{"openid", "email"}, svc.loginScopes) +} + +func TestLoginCmd_SurfacesFailure(t *testing.T) { + withService(t, &fakeService{loginErr: domain.ErrAuthTimeout}) + + _, _, err := testutil.ExecuteSubCommand(newLoginCmd()) + + require.Error(t, err) + assert.ErrorIs(t, err, domain.ErrAuthTimeout) +} + +func TestStatusCmd_ShowsStoredSession(t *testing.T) { + withService(t, &fakeService{session: loggedInSession()}) + + stdout, _, err := testutil.ExecuteSubCommand(newStatusCmd()) + require.NoError(t, err) + + assert.Contains(t, stdout, "Logged in") + assert.Contains(t, stdout, "https://auth.example.test") + assert.Contains(t, stdout, "Refresh: present") + assert.NotContains(t, stdout, "at-1", "the access token itself must not be printed") +} + +func TestStatusCmd_FlagsExpiredToken(t *testing.T) { + session := loggedInSession() + session.Tokens.ExpiresAt = time.Now().Add(-time.Hour) + withService(t, &fakeService{session: session}) + + stdout, _, err := testutil.ExecuteSubCommand(newStatusCmd()) + require.NoError(t, err) + + assert.Contains(t, stdout, "expired") +} + +func TestStatusCmd_NotLoggedIn(t *testing.T) { + withService(t, &fakeService{statusErr: domain.ErrOAuthNotLoggedIn}) + + _, _, err := testutil.ExecuteSubCommand(newStatusCmd()) + + require.Error(t, err) + var cliErr *common.CLIError + require.ErrorAs(t, err, &cliErr) + assert.Contains(t, cliErr.Message, "nylas oauth login") +} + +func TestStatusCmd_VerifyCallsUserInfo(t *testing.T) { + svc := &fakeService{ + session: loggedInSession(), + userInfo: &domain.OAuthUserInfo{Subject: "user-1", Email: "dev@example.test", EmailVerified: true}, + } + withService(t, svc) + + stdout, _, err := testutil.ExecuteSubCommand(newStatusCmd(), "--verify") + require.NoError(t, err) + + assert.Contains(t, stdout, "user-1") + assert.Contains(t, stdout, "dev@example.test") +} + +func TestStatusCmd_WithoutVerifyDoesNotCallUserInfo(t *testing.T) { + svc := &fakeService{session: loggedInSession(), userErr: errors.New("should not be called")} + withService(t, svc) + + _, _, err := testutil.ExecuteSubCommand(newStatusCmd()) + + require.NoError(t, err) +} + +func TestTokenCmd_PrintsBareToken(t *testing.T) { + // The documented use is command substitution into a curl header, so the + // output has to be the token and nothing else. + withService(t, &fakeService{accessToken: "at-42"}) + + stdout, _, err := testutil.ExecuteSubCommand(newTokenCmd()) + require.NoError(t, err) + + assert.Equal(t, "at-42\n", stdout) +} + +func TestTokenCmd_NotLoggedIn(t *testing.T) { + withService(t, &fakeService{accessErr: domain.ErrOAuthNotLoggedIn}) + + _, _, err := testutil.ExecuteSubCommand(newTokenCmd()) + + require.Error(t, err) + assert.ErrorIs(t, err, domain.ErrOAuthNotLoggedIn) +} + +func TestLogoutCmd(t *testing.T) { + svc := &fakeService{} + withService(t, svc) + + stdout, _, err := testutil.ExecuteSubCommand(newLogoutCmd()) + require.NoError(t, err) + + assert.True(t, svc.logoutCalled) + assert.Contains(t, stdout, "Logged out") +} + +func TestLogoutCmd_SurfacesRevocationFailure(t *testing.T) { + withService(t, &fakeService{logoutErr: errors.New("server unreachable")}) + + _, _, err := testutil.ExecuteSubCommand(newLogoutCmd()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "server unreachable") +} diff --git a/internal/cli/oauth/status.go b/internal/cli/oauth/status.go new file mode 100644 index 0000000..4cf56ca --- /dev/null +++ b/internal/cli/oauth/status.go @@ -0,0 +1,84 @@ +package oauth + +import ( + "fmt" + "time" + + "github.com/spf13/cobra" + + "github.com/nylas/cli/internal/cli/common" +) + +func newStatusCmd() *cobra.Command { + var remote bool + + cmd := &cobra.Command{ + Use: "status", + Short: "Show the current OAuth session", + RunE: func(cmd *cobra.Command, _ []string) error { + svc, err := createLoginServiceFn() + if err != nil { + return wrapOAuthError(err) + } + + session, err := svc.Status() + if err != nil { + return wrapOAuthError(err) + } + + ctx, cancel := common.CreateContext() + defer cancel() + + out := cmd.OutOrStdout() + expired := session.Tokens.IsExpired(time.Now()) + if expired { + _, _ = common.Yellow.Fprintln(out, "● Logged in (access token expired)") + } else { + _, _ = common.Green.Fprintln(out, "✓ Logged in") + } + + _, _ = fmt.Fprintf(out, " Issuer: %s\n", session.Issuer) + _, _ = fmt.Fprintf(out, " Client ID: %s\n", session.ClientID) + if session.Tokens.Scope != "" { + _, _ = fmt.Fprintf(out, " Scopes: %s\n", session.Tokens.Scope) + } + if !session.Tokens.ExpiresAt.IsZero() { + _, _ = fmt.Fprintf(out, " Expires: %s\n", session.Tokens.ExpiresAt.Local().Format("2006-01-02 15:04:05 MST")) + } + _, _ = fmt.Fprintf(out, " Refresh: %s\n", presentAbsent(session.Tokens.RefreshToken != "")) + _, _ = fmt.Fprintf(out, " ID token: %s\n", presentAbsent(session.Tokens.IDToken != "")) + + if !remote { + return nil + } + + // --verify proves the token against the live server rather than + // trusting what is on disk. + info, err := svc.UserInfo(ctx) + if err != nil { + return wrapOAuthError(err) + } + _, _ = fmt.Fprintf(out, " Subject: %s\n", info.Subject) + if info.Email != "" { + _, _ = fmt.Fprintf(out, " Email: %s (verified: %t)\n", info.Email, info.EmailVerified) + } + if info.Org != "" { + _, _ = fmt.Fprintf(out, " Org: %s\n", info.Org) + } + + return nil + }, + } + + cmd.Flags().BoolVar(&remote, "verify", false, + "call the userinfo endpoint to confirm the token is accepted") + + return cmd +} + +func presentAbsent(present bool) string { + if present { + return "present" + } + return "absent" +} diff --git a/internal/cli/oauth/token.go b/internal/cli/oauth/token.go new file mode 100644 index 0000000..71d8c29 --- /dev/null +++ b/internal/cli/oauth/token.go @@ -0,0 +1,39 @@ +package oauth + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/nylas/cli/internal/cli/common" +) + +func newTokenCmd() *cobra.Command { + return &cobra.Command{ + Use: "token", + Short: "Print a valid access token, refreshing it if needed", + Long: `Print the current access token. + +If the stored token has expired it is refreshed first, and the rotated +refresh token replaces the old one in the keyring.`, + Example: ` # Call an OAuth-protected endpoint + curl -H "Authorization: Bearer $(nylas oauth token)" https://example/resource`, + RunE: func(cmd *cobra.Command, _ []string) error { + svc, err := createLoginServiceFn() + if err != nil { + return wrapOAuthError(err) + } + + ctx, cancel := common.CreateContext() + defer cancel() + + token, err := svc.AccessToken(ctx) + if err != nil { + return wrapOAuthError(err) + } + + _, _ = fmt.Fprintln(cmd.OutOrStdout(), token) + return nil + }, + } +} From fda95a094f7753d63b6dadc4cfb3f18808e6f2f1 Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Mon, 21 Sep 2026 15:08:16 +0200 Subject: [PATCH 4/5] TW-6922: verify the OAuth client against a live authorization server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 1d: integration tests that run the real adapter against a running dashboard-account, covering discovery, dynamic registration, the full PKCE code exchange, userinfo, refresh rotation, revocation, and the two rejections that matter (replayed code, mismatched verifier). They seed their own user, consent grant and authorization code through the /dev routes. That is what removes the browser from the loop: the consent screen needs a UAS-connected mailbox, which a local stack does not have. The tests front the server with a proxy that rewrites the issuer origin in the discovery document. dashboard-account builds every advertised endpoint from OAUTH_ISSUER, and locally that is often a tunnel hostname that is stale or unreachable, while the client is spec-correct and follows whatever the document says. The proxy is confined to the test — no workaround leaks into the client. Confirmed live, and worth recording: the server really does burn the whole refresh family when a consumed token is replayed, which is the behaviour the storage rules in oauthlogin were written against. Skips unless NYLAS_OAUTH_AS_URL is set. Co-Authored-By: Claude Opus 5 --- docs/DEVELOPMENT.md | 26 ++ internal/cli/integration/oauth_test.go | 452 +++++++++++++++++++++++++ 2 files changed, 478 insertions(+) create mode 100644 internal/cli/integration/oauth_test.go diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 63a0c88..6f77dd1 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -68,6 +68,32 @@ make test-integration **CRITICAL:** Integration tests create real resources. Always use `make ci-full` for automatic cleanup. +### OAuth authorization server tests + +`internal/cli/integration/oauth_test.go` drives a real dashboard-account +authorization server instead of the Nylas API, so it needs its own variable and +skips without it: + +```bash +NYLAS_OAUTH_AS_URL=http://localhost:3001 \ + go test -tags integration -run TestOAuthAS ./internal/cli/integration/ +``` + +Requirements on the server side: + +- dashboard-account running (in a Tilt stack it is on port 3001) +- `/dev` routes enabled — `ENABLE_DEV_ROUTES=true` or `IS_E2E=true`. The tests + seed their own user, consent grant and authorization code through them, which + is what lets the token exchange run without a browser. + +The tests front the server with a small proxy that rewrites the issuer origin in +the discovery document. dashboard-account builds every advertised endpoint from +`OAUTH_ISSUER`, and in a local stack that is frequently a tunnel hostname that is +stale or unreachable; the client under test is spec-correct and follows whatever +the document says. If you would rather fix it at the source, set +`OAUTH_ISSUER=http://localhost:3001` in `infra/.env.local` and restart the +service — the proxy then rewrites nothing. + --- ## Project Structure diff --git a/internal/cli/integration/oauth_test.go b/internal/cli/integration/oauth_test.go new file mode 100644 index 0000000..15e1701 --- /dev/null +++ b/internal/cli/integration/oauth_test.go @@ -0,0 +1,452 @@ +//go:build integration + +package integration + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "testing" + "time" + + "github.com/nylas/cli/internal/adapters/oauthas" + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// These tests drive the real dashboard-account authorization server. Point +// NYLAS_OAUTH_AS_URL at it (for a Tilt stack, http://localhost:3001) with its +// /dev routes enabled; without the variable they skip. +const oauthASEnv = "NYLAS_OAUTH_AS_URL" + +// oauthTestRedirectURI carries a port while the client registers the same URI +// without one, so every exchange here also exercises the RFC 8252 rule that +// only the port of a loopback redirect URI is free. +const oauthTestRedirectURI = "http://localhost:9007/callback" + +func oauthUpstream(t *testing.T) string { + t.Helper() + upstream := strings.TrimRight(os.Getenv(oauthASEnv), "/") + if upstream == "" { + t.Skipf("set %s to run the OAuth authorization server integration tests", oauthASEnv) + } + return upstream +} + +// newNormalizedASProxy fronts the authorization server so its discovery +// document advertises endpoints the test can actually reach. +// +// dashboard-account builds every endpoint from OAUTH_ISSUER, which in a local +// stack is often a tunnel hostname that is stale or unreachable. The client +// under test is spec-correct and follows whatever the document says, so +// without this it would chase a dead host. Only the origin is rewritten; +// every request is forwarded to the real server untouched. +func newNormalizedASProxy(t *testing.T, upstream string) *httptest.Server { + t.Helper() + + // The proxy must not follow redirects itself: /oauth/authorize answers an + // anonymous caller with a 302 to login, and swallowing it here would hide + // whether the server accepted the authorization request at all. + forwarder := &http.Client{ + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } + + var proxy *httptest.Server + proxy = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + + outbound, err := http.NewRequestWithContext(r.Context(), r.Method, upstream+r.URL.RequestURI(), bytes.NewReader(body)) + if err != nil { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + for name, values := range r.Header { + for _, value := range values { + outbound.Header.Add(name, value) + } + } + outbound.Header.Del("Accept-Encoding") + + resp, err := forwarder.Do(outbound) + if err != nil { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + defer func() { _ = resp.Body.Close() }() + + payload, err := io.ReadAll(resp.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + + if strings.HasPrefix(r.URL.Path, "/.well-known/") { + payload = rewriteIssuerOrigin(t, payload, proxy.URL) + } + + for name, values := range resp.Header { + if strings.EqualFold(name, "Content-Length") { + continue + } + for _, value := range values { + w.Header().Add(name, value) + } + } + w.WriteHeader(resp.StatusCode) + _, _ = w.Write(payload) + })) + t.Cleanup(proxy.Close) + + return proxy +} + +// rewriteIssuerOrigin replaces the advertised issuer origin with the proxy's. +func rewriteIssuerOrigin(t *testing.T, document []byte, proxyURL string) []byte { + t.Helper() + + var parsed map[string]any + if err := json.Unmarshal(document, &parsed); err != nil { + return document + } + issuer, _ := parsed["issuer"].(string) + if issuer == "" || issuer == proxyURL { + return document + } + return []byte(strings.ReplaceAll(string(document), issuer, proxyURL)) +} + +type oauthTestSubject struct { + userPublicID string + orgPublicID string + email string +} + +func postDevJSON(t *testing.T, baseURL, path string, body, result any) { + t.Helper() + + payload, err := json.Marshal(body) + require.NoError(t, err) + + resp, err := http.Post(baseURL+path, "application/json", bytes.NewReader(payload)) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + + raw, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Less(t, resp.StatusCode, 300, + "%s failed (%d) — are the /dev routes enabled on the authorization server? %s", path, resp.StatusCode, raw) + + if result == nil { + return + } + // Dev routes answer in the house {"data": ...} envelope. + var envelope struct { + Data json.RawMessage `json:"data"` + } + require.NoError(t, json.Unmarshal(raw, &envelope)) + require.NoError(t, json.Unmarshal(envelope.Data, result)) +} + +// seedOAuthSubject creates a fresh user and organization to authorize as. +func seedOAuthSubject(t *testing.T, upstream string) oauthTestSubject { + t.Helper() + + email := fmt.Sprintf("cli-oauth-it-%d@nylas.com", time.Now().UnixNano()) + var created struct { + User struct { + PublicID string `json:"publicId"` + Email string `json:"email"` + } `json:"user"` + Organization struct { + PublicID string `json:"publicId"` + } `json:"organization"` + } + postDevJSON(t, upstream, "/dev/create-seed-user", map[string]any{ + "email": email, + "firstName": "CLI", + "lastName": "Integration", + "emailVerified": true, + "createOrganization": true, + "organizationName": "CLI Integration Org", + "organizationRegion": "us", + }, &created) + + require.NotEmpty(t, created.User.PublicID) + require.NotEmpty(t, created.Organization.PublicID) + + return oauthTestSubject{ + userPublicID: created.User.PublicID, + orgPublicID: created.Organization.PublicID, + email: created.User.Email, + } +} + +// mintAuthorizationCode bypasses the browser consent screen. The consent +// screen needs a UAS-connected mailbox, which a local stack does not have; +// the dev route exists precisely so the token exchange stays testable. +func mintAuthorizationCode(t *testing.T, upstream, clientID string, subject oauthTestSubject, challenge string) string { + t.Helper() + + // A refresh token is only issued against an active consent grant. + postDevJSON(t, upstream, "/dev/oauth/consent-grant", map[string]any{ + "clientId": clientID, + "userPublicId": subject.userPublicID, + "orgPublicId": subject.orgPublicID, + "scopes": domain.DefaultOAuthScopes(), + }, nil) + + var minted struct { + Code string `json:"code"` + } + postDevJSON(t, upstream, "/dev/oauth/authorization-code", map[string]any{ + "clientId": clientID, + "userPublicId": subject.userPublicID, + "orgPublicId": subject.orgPublicID, + "redirectUri": oauthTestRedirectURI, + "scopes": domain.DefaultOAuthScopes(), + "codeChallenge": challenge, + }, &minted) + + require.NotEmpty(t, minted.Code) + return minted.Code +} + +func registerCLIClient(t *testing.T, client *oauthas.Client) *domain.OAuthClientRegistration { + t.Helper() + + registration, err := client.Register(context.Background(), domain.OAuthClientRegistrationRequest{ + ClientName: "Nylas CLI integration test", + RedirectURIs: []string{"http://localhost/callback"}, + TokenEndpointAuthMethod: "none", + GrantTypes: []string{"authorization_code", "refresh_token"}, + ResponseTypes: []string{"code"}, + }) + require.NoError(t, err) + return registration +} + +func TestOAuthAS_DiscoveryMatchesWhatTheClientNeeds(t *testing.T) { + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + + metadata, err := client.Metadata(context.Background()) + require.NoError(t, err) + + assert.Contains(t, metadata.CodeChallengeMethodsSupported, "S256") + assert.Contains(t, metadata.TokenEndpointAuthMethods, "none", + "the CLI registers as a public client and cannot authenticate otherwise") + assert.Contains(t, metadata.ScopesSupported, domain.OAuthScopeOfflineAccess, + "without offline_access the server issues no refresh token") + assert.NotEmpty(t, metadata.RegistrationEndpoint) + assert.NotEmpty(t, metadata.RevocationEndpoint) + assert.NotEmpty(t, metadata.UserInfoEndpoint) +} + +func TestOAuthAS_RegistersCLIAsPublicClient(t *testing.T) { + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + + registration := registerCLIClient(t, client) + + assert.NotEmpty(t, registration.ClientID) + assert.Equal(t, "none", registration.TokenEndpointAuthMethod) + assert.Empty(t, registration.ClientSecret, + "a public client must not be handed a secret it cannot protect") +} + +func TestOAuthAS_FullAuthorizationCodeExchange(t *testing.T) { + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + ctx := context.Background() + + subject := seedOAuthSubject(t, upstream) + registration := registerCLIClient(t, client) + + pkce, err := domain.NewPKCE() + require.NoError(t, err) + code := mintAuthorizationCode(t, upstream, registration.ClientID, subject, pkce.Challenge) + + tokens, err := client.ExchangeCode(ctx, domain.OAuthCodeExchange{ + ClientID: registration.ClientID, + Code: code, + RedirectURI: oauthTestRedirectURI, + CodeVerifier: pkce.Verifier, + }) + require.NoError(t, err) + + assert.NotEmpty(t, tokens.AccessToken) + assert.Equal(t, "Bearer", tokens.TokenType) + assert.NotEmpty(t, tokens.RefreshToken, "offline_access was consented") + assert.NotEmpty(t, tokens.IDToken, "openid was consented") + assert.False(t, tokens.ExpiresAt.IsZero(), "ExpiresAt must be derived from expires_in") + assert.False(t, tokens.IsExpired(time.Now())) + + info, err := client.UserInfo(ctx, tokens.AccessToken) + require.NoError(t, err) + assert.Equal(t, subject.userPublicID, info.Subject) + assert.Equal(t, subject.email, info.Email) + assert.Equal(t, subject.orgPublicID, info.Org) +} + +func TestOAuthAS_RejectsReplayedAuthorizationCode(t *testing.T) { + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + ctx := context.Background() + + subject := seedOAuthSubject(t, upstream) + registration := registerCLIClient(t, client) + pkce, err := domain.NewPKCE() + require.NoError(t, err) + code := mintAuthorizationCode(t, upstream, registration.ClientID, subject, pkce.Challenge) + + exchange := domain.OAuthCodeExchange{ + ClientID: registration.ClientID, + Code: code, + RedirectURI: oauthTestRedirectURI, + CodeVerifier: pkce.Verifier, + } + _, err = client.ExchangeCode(ctx, exchange) + require.NoError(t, err) + + _, err = client.ExchangeCode(ctx, exchange) + + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "invalid_grant", oauthErr.Code) +} + +func TestOAuthAS_RejectsMismatchedCodeVerifier(t *testing.T) { + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + ctx := context.Background() + + subject := seedOAuthSubject(t, upstream) + registration := registerCLIClient(t, client) + pkce, err := domain.NewPKCE() + require.NoError(t, err) + other, err := domain.NewPKCE() + require.NoError(t, err) + code := mintAuthorizationCode(t, upstream, registration.ClientID, subject, pkce.Challenge) + + _, err = client.ExchangeCode(ctx, domain.OAuthCodeExchange{ + ClientID: registration.ClientID, + Code: code, + RedirectURI: oauthTestRedirectURI, + CodeVerifier: other.Verifier, + }) + + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "invalid_grant", oauthErr.Code) +} + +func TestOAuthAS_RefreshRotatesAndDetectsReuse(t *testing.T) { + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + ctx := context.Background() + + subject := seedOAuthSubject(t, upstream) + registration := registerCLIClient(t, client) + pkce, err := domain.NewPKCE() + require.NoError(t, err) + code := mintAuthorizationCode(t, upstream, registration.ClientID, subject, pkce.Challenge) + + tokens, err := client.ExchangeCode(ctx, domain.OAuthCodeExchange{ + ClientID: registration.ClientID, + Code: code, + RedirectURI: oauthTestRedirectURI, + CodeVerifier: pkce.Verifier, + }) + require.NoError(t, err) + require.NotEmpty(t, tokens.RefreshToken) + + refreshed, err := client.Refresh(ctx, registration.ClientID, tokens.RefreshToken) + require.NoError(t, err) + + assert.NotEmpty(t, refreshed.AccessToken) + assert.NotEmpty(t, refreshed.RefreshToken) + assert.NotEqual(t, tokens.RefreshToken, refreshed.RefreshToken, + "the server rotates the refresh token on every use") + + // Replaying the consumed token is what burns the whole family, which is + // why oauthlogin never carries an old refresh token forward. + _, err = client.Refresh(ctx, registration.ClientID, tokens.RefreshToken) + var oauthErr *domain.OAuthError + require.ErrorAs(t, err, &oauthErr) + assert.Equal(t, "invalid_grant", oauthErr.Code) +} + +func TestOAuthAS_RevokeEndsTheSession(t *testing.T) { + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + ctx := context.Background() + + subject := seedOAuthSubject(t, upstream) + registration := registerCLIClient(t, client) + pkce, err := domain.NewPKCE() + require.NoError(t, err) + code := mintAuthorizationCode(t, upstream, registration.ClientID, subject, pkce.Challenge) + + tokens, err := client.ExchangeCode(ctx, domain.OAuthCodeExchange{ + ClientID: registration.ClientID, + Code: code, + RedirectURI: oauthTestRedirectURI, + CodeVerifier: pkce.Verifier, + }) + require.NoError(t, err) + + require.NoError(t, client.Revoke(ctx, registration.ClientID, tokens.RefreshToken)) + + _, err = client.Refresh(ctx, registration.ClientID, tokens.RefreshToken) + require.Error(t, err, "a revoked refresh token must not mint new tokens") +} + +func TestOAuthAS_AuthorizationURLIsAcceptedByTheServer(t *testing.T) { + // The browser step cannot run headless, but the server still validates + // the request shape before it redirects to login — a malformed + // code_challenge or scope is rejected here rather than at that redirect. + upstream := oauthUpstream(t) + client := oauthas.NewClient(newNormalizedASProxy(t, upstream).URL) + ctx := context.Background() + + registration := registerCLIClient(t, client) + pkce, err := domain.NewPKCE() + require.NoError(t, err) + state, err := domain.NewOAuthState() + require.NoError(t, err) + + authURL, err := client.AuthorizationURL(ctx, domain.OAuthAuthorizationParams{ + ClientID: registration.ClientID, + RedirectURI: oauthTestRedirectURI, + Scopes: domain.DefaultOAuthScopes(), + State: state, + CodeChallenge: pkce.Challenge, + }) + require.NoError(t, err) + + httpClient := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }} + resp, err := httpClient.Get(authURL) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + + // An anonymous caller is sent to login; a rejected request would instead + // come back 400, or redirect to the callback carrying ?error=. + require.Equal(t, http.StatusFound, resp.StatusCode, "expected a redirect to login") + location, err := url.Parse(resp.Header.Get("Location")) + require.NoError(t, err) + assert.Empty(t, location.Query().Get("error"), "the server rejected the authorization request") +} From 00f81ea47d7082570eb23de3ff7b2a43aad9ec03 Mon Sep 17 00:00:00 2001 From: Dan Radenkovic Date: Tue, 22 Sep 2026 15:02:59 +0200 Subject: [PATCH 5/5] TW-6922: use plain background on OAuth success page Co-Authored-By: Claude Sonnet 5 --- internal/adapters/oauth/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/adapters/oauth/server.go b/internal/adapters/oauth/server.go index 23a6887..b1033b5 100644 --- a/internal/adapters/oauth/server.go +++ b/internal/adapters/oauth/server.go @@ -184,7 +184,7 @@ func (s *CallbackServer) handleCallback(w http.ResponseWriter, r *http.Request)