diff --git a/cmd/nylas/main.go b/cmd/nylas/main.go index 3e5b4dd..246b74c 100644 --- a/cmd/nylas/main.go +++ b/cmd/nylas/main.go @@ -50,6 +50,8 @@ func main() { rootCmd.AddCommand(contacts.NewContactsCmd()) rootCmd.AddCommand(dashboard.NewDashboardCmd()) rootCmd.AddCommand(oauthcmd.NewOAuthCmd()) + // The dashboard commands renew OAuth-backed sessions through `nylas oauth`. + oauthcmd.WireDashboard() rootCmd.AddCommand(setup.NewSetupCmd()) rootCmd.AddCommand(scheduler.NewSchedulerCmd()) rootCmd.AddCommand(admin.NewAdminCmd()) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ba5234f..7768d6a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -179,6 +179,9 @@ url := qb.BuildURL(baseURL) - `llm.go` - LLM interface (AI providers) - `config.go` - Config interface - `webhook_server.go` - Webhook server interface + - `oauthas.go` - OAuthAuthServerClient (Nylas OAuth authorization server: discovery, PKCE code exchange, refresh, revoke) + - `lock.go` - CrossProcessLock (session locks shared by every CLI process on a machine) + - `mcp.go` - MCPCredentialSource (the API key or OAuth token `nylas mcp serve` sends) 3. **Adapters** (`internal/adapters/`) @@ -192,7 +195,8 @@ url := qb.BuildURL(baseURL) | `mcp/` | MCP proxy server for AI assistants | | `config/` | Configuration validation | | `oauth/` | OAuth callback server | - | `filelock/` | Cross-process advisory file lock (flock / LockFileEx) serialising OAuth token refresh | + | `oauthas/` | Client for the Nylas OAuth authorization server (PKCE, refresh, revoke) | + | `filelock/` | Cross-process advisory file lock (flock / LockFileEx): the OAuth and dashboard session locks and the encrypted file store's `.secrets.lock` | | `utilities/` | Services (contacts, email, scheduling, timezone, webhook) | | `browser/` | Browser automation | | `tunnel/` | Cloudflare tunnel | diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 85acaec..049b248 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -137,6 +137,7 @@ nylas oauth login --for mcp # Scopes + resource for `nylas mcp serve --auth nylas oauth login --region eu # Sign up with a new organization in the EU nylas oauth status # Show the stored session and decoded token claims nylas oauth status --verify # Also confirm the token against /oauth/userinfo +nylas oauth status --json # Same facts as JSON (never includes a token) nylas oauth token # Print a valid access token, refreshing if needed nylas oauth logout # Revoke the session and clear stored tokens ``` @@ -165,6 +166,14 @@ server has already rotated the token it was sent. scopes and expiry. The claims are **decoded, not verified** — the CLI does not check the signature; only the resource server's answer is authoritative. +`nylas oauth status`, `login` and `logout` honour the global `--json`, +`--format yaml` and `--quiet` flags. `oauth status --json` prints `logged_in`, +`expired`, `server`, `issuer`, `client_id`, `resource`, `scopes`, `expires_at`, +`has_refresh_token` and the decoded `token_claims` (plus `userinfo` with +`--verify`); no token is ever included. `--quiet` prints nothing, so the exit +code alone says whether a session is stored. `nylas oauth token` always prints +the bare token. + Default scopes are `openid`, `email`, `offline_access` and `dashboard.session`. `offline_access` is what makes the server issue a refresh token; without it the session ends when the access token expires (15 minutes by default). diff --git a/internal/adapters/dashboard/account_client_domain_test.go b/internal/adapters/dashboard/account_client_domain_test.go new file mode 100644 index 0000000..de439e8 --- /dev/null +++ b/internal/adapters/dashboard/account_client_domain_test.go @@ -0,0 +1,285 @@ +package dashboard + +import ( + "context" + "net/http" + "testing" + + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAccountClientDomainOperations(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + handler func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) + run func(t *testing.T, client *AccountClient) + }{ + { + name: "list domains", + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/orgs/inbox/domains", r.URL.Path) + assert.Equal(t, "25", r.URL.Query().Get("limit")) + assert.Equal(t, "cursor-1", r.URL.Query().Get("pageToken")) + assert.Equal(t, "Bearer user-token", r.Header.Get("Authorization")) + assert.Equal(t, "org-token", r.Header.Get("X-Nylas-Org")) + assert.NotEmpty(t, r.Header.Get("DPoP")) + + writeDashboardEnvelopeWithCursor(t, w, []map[string]any{ + { + "id": "dom_1", + "name": "Example", + "domainAddress": "example.com", + "organizationId": "org_1", + "region": "us", + "branded": true, + "verifiedOwnership": true, + }, + }, "cursor-2") + }, + run: func(t *testing.T, client *AccountClient) { + page, err := client.ListDomains(context.Background(), 25, "cursor-1", "user-token", "org-token") + require.NoError(t, err) + require.Len(t, page.Domains, 1) + assert.Equal(t, "dom_1", page.Domains[0].ID) + assert.Equal(t, "example.com", page.Domains[0].DomainAddress) + assert.True(t, page.Domains[0].VerifiedOwnership) + assert.Equal(t, "cursor-2", page.NextCursor) + }, + }, + { + name: "check availability", + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/orgs/inbox/domains/availability", r.URL.Path) + assert.Equal(t, "example.com", r.URL.Query().Get("domainAddress")) + + writeDashboardEnvelope(t, w, map[string]any{ + "domainAddress": "example.com", + "available": true, + "conflictsWith": nil, + }) + }, + run: func(t *testing.T, client *AccountClient) { + result, err := client.CheckDomainAvailability(context.Background(), "example.com", "user-token", "org-token") + require.NoError(t, err) + assert.True(t, result.Available) + assert.Nil(t, result.ConflictsWith) + }, + }, + { + name: "create domain", + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/orgs/inbox/domains", r.URL.Path) + assert.Equal(t, "Example", body["name"]) + assert.Equal(t, "example.com", body["domainAddress"]) + assert.Equal(t, "eu", body["region"]) + + writeDashboardEnvelope(t, w, map[string]any{ + "id": "dom_new", + "name": "Example", + "domainAddress": "example.com", + "region": "eu", + }) + }, + run: func(t *testing.T, client *AccountClient) { + created, err := client.CreateDomain(context.Background(), domain.DashboardCreateInboxDomainInput{ + Name: "Example", + DomainAddress: "example.com", + Region: "eu", + }, "user-token", "org-token") + require.NoError(t, err) + assert.Equal(t, "dom_new", created.ID) + }, + }, + { + name: "update domain", + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) { + assert.Equal(t, http.MethodPatch, r.Method) + assert.Equal(t, "/orgs/inbox/domains/dom_1", r.URL.Path) + assert.Equal(t, "us", r.URL.Query().Get("region")) + assert.Equal(t, "Renamed", body["name"]) + + writeDashboardEnvelope(t, w, map[string]any{ + "id": "dom_1", + "name": "Renamed", + "domainAddress": "example.com", + "region": "us", + }) + }, + run: func(t *testing.T, client *AccountClient) { + updated, err := client.UpdateDomain(context.Background(), "dom_1", "us", domain.DashboardUpdateInboxDomainInput{Name: "Renamed"}, "user-token", "org-token") + require.NoError(t, err) + assert.Equal(t, "Renamed", updated.Name) + }, + }, + { + name: "get domain info", + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/orgs/inbox/domains/dom_1/info", r.URL.Path) + assert.Equal(t, "us", r.URL.Query().Get("region")) + assert.Equal(t, "mx", r.URL.Query().Get("type")) + + writeDashboardEnvelope(t, w, map[string]any{ + "domainId": "dom_1", + "status": "pending", + "message": "Configure MX", + "attempt": map[string]any{ + "type": "mx", + "options": map[string]any{ + "host": "example.com", + "type": "MX", + "value": "10 inbound.nylas.com", + }, + }, + }) + }, + run: func(t *testing.T, client *AccountClient) { + info, err := client.GetDomainInfo(context.Background(), "dom_1", "us", "mx", "user-token", "org-token") + require.NoError(t, err) + assert.Equal(t, "pending", info.Status) + require.NotNil(t, info.Attempt) + assert.Equal(t, "MX", info.Attempt.Options.Type) + }, + }, + { + name: "verify domain", + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/orgs/inbox/domains/dom_1/verify", r.URL.Path) + assert.Equal(t, "eu", r.URL.Query().Get("region")) + assert.Equal(t, "ownership", body["type"]) + + writeDashboardEnvelope(t, w, map[string]any{ + "domainId": "dom_1", + "status": "done", + "message": "Verified", + }) + }, + run: func(t *testing.T, client *AccountClient) { + result, err := client.VerifyDomain(context.Background(), "dom_1", "eu", domain.DashboardVerifyInboxDomainInput{Type: "ownership"}, "user-token", "org-token") + require.NoError(t, err) + assert.Equal(t, "done", result.Status) + }, + }, + { + name: "delete domain", + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { + assert.Equal(t, http.MethodDelete, r.Method) + assert.Equal(t, "/orgs/inbox/domains/dom_1", r.URL.Path) + assert.Equal(t, "us", r.URL.Query().Get("region")) + + writeDashboardEnvelope(t, w, map[string]any{"success": true}) + }, + run: func(t *testing.T, client *AccountClient) { + deleted, err := client.DeleteDomain(context.Background(), "dom_1", "us", "user-token", "org-token") + require.NoError(t, err) + assert.True(t, deleted) + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + server := newAccountClientTestServer(t, tt.handler) + defer server.Close() + + client := &AccountClient{ + baseURL: server.URL, + httpClient: server.Client(), + dpop: &mockDPoP{proof: "test-proof"}, + } + + tt.run(t, client) + }) + } +} + +func TestDecodeDomainPageFallbackShapes(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw rawResponse + wantIDs []string + wantCursor string + wantErr string + }{ + { + name: "object fallback with snake cursor", + raw: rawResponse{ + Data: []byte(`{"domains":[{"id":"dom_1","domainAddress":"example.com"}],"next_cursor":"cursor-2"}`), + }, + wantIDs: []string{"dom_1"}, + wantCursor: "cursor-2", + }, + { + name: "raw response cursor wins over object cursor", + raw: rawResponse{ + Data: []byte(`{"domains":[{"id":"dom_1","domainAddress":"example.com"}],"nextCursor":"payload-cursor"}`), + NextCursor: "envelope-cursor", + }, + wantIDs: []string{"dom_1"}, + wantCursor: "envelope-cursor", + }, + { + name: "empty object page with cursor", + raw: rawResponse{ + Data: []byte(`{"domains":[],"pageToken":"cursor-3"}`), + }, + wantCursor: "cursor-3", + }, + { + name: "object fallback requires domains field", + raw: rawResponse{ + Data: []byte(`{"nextCursor":"cursor-4"}`), + }, + wantErr: "missing domains", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + page, err := decodeDomainPage(tt.raw) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantCursor, page.NextCursor) + require.Len(t, page.Domains, len(tt.wantIDs)) + for i, id := range tt.wantIDs { + assert.Equal(t, id, page.Domains[i].ID) + } + }) + } +} + +func TestUnwrapRawResponseExtractsNestedCursor(t *testing.T) { + t.Parallel() + + raw, err := unwrapRawResponse([]byte(`{ + "request_id":"req", + "success":true, + "data":[], + "pagination":{"next_cursor":"cursor-2"} + }`)) + + require.NoError(t, err) + assert.JSONEq(t, `[]`, string(raw.Data)) + assert.Equal(t, "cursor-2", raw.NextCursor) +} diff --git a/internal/adapters/dashboard/account_client_test.go b/internal/adapters/dashboard/account_client_test.go index fe62831..d998340 100644 --- a/internal/adapters/dashboard/account_client_test.go +++ b/internal/adapters/dashboard/account_client_test.go @@ -574,277 +574,3 @@ func TestAccountClientRefreshPropagatesUnderlyingError(t *testing.T) { assert.True(t, errors.Is(err, domain.ErrDashboardSessionExpired)) assert.Contains(t, err.Error(), "failed to refresh session") } - -func TestAccountClientDomainOperations(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - handler func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) - run func(t *testing.T, client *AccountClient) - }{ - { - name: "list domains", - handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { - assert.Equal(t, http.MethodGet, r.Method) - assert.Equal(t, "/orgs/inbox/domains", r.URL.Path) - assert.Equal(t, "25", r.URL.Query().Get("limit")) - assert.Equal(t, "cursor-1", r.URL.Query().Get("pageToken")) - assert.Equal(t, "Bearer user-token", r.Header.Get("Authorization")) - assert.Equal(t, "org-token", r.Header.Get("X-Nylas-Org")) - assert.NotEmpty(t, r.Header.Get("DPoP")) - - writeDashboardEnvelopeWithCursor(t, w, []map[string]any{ - { - "id": "dom_1", - "name": "Example", - "domainAddress": "example.com", - "organizationId": "org_1", - "region": "us", - "branded": true, - "verifiedOwnership": true, - }, - }, "cursor-2") - }, - run: func(t *testing.T, client *AccountClient) { - page, err := client.ListDomains(context.Background(), 25, "cursor-1", "user-token", "org-token") - require.NoError(t, err) - require.Len(t, page.Domains, 1) - assert.Equal(t, "dom_1", page.Domains[0].ID) - assert.Equal(t, "example.com", page.Domains[0].DomainAddress) - assert.True(t, page.Domains[0].VerifiedOwnership) - assert.Equal(t, "cursor-2", page.NextCursor) - }, - }, - { - name: "check availability", - handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { - assert.Equal(t, http.MethodGet, r.Method) - assert.Equal(t, "/orgs/inbox/domains/availability", r.URL.Path) - assert.Equal(t, "example.com", r.URL.Query().Get("domainAddress")) - - writeDashboardEnvelope(t, w, map[string]any{ - "domainAddress": "example.com", - "available": true, - "conflictsWith": nil, - }) - }, - run: func(t *testing.T, client *AccountClient) { - result, err := client.CheckDomainAvailability(context.Background(), "example.com", "user-token", "org-token") - require.NoError(t, err) - assert.True(t, result.Available) - assert.Nil(t, result.ConflictsWith) - }, - }, - { - name: "create domain", - handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) { - assert.Equal(t, http.MethodPost, r.Method) - assert.Equal(t, "/orgs/inbox/domains", r.URL.Path) - assert.Equal(t, "Example", body["name"]) - assert.Equal(t, "example.com", body["domainAddress"]) - assert.Equal(t, "eu", body["region"]) - - writeDashboardEnvelope(t, w, map[string]any{ - "id": "dom_new", - "name": "Example", - "domainAddress": "example.com", - "region": "eu", - }) - }, - run: func(t *testing.T, client *AccountClient) { - created, err := client.CreateDomain(context.Background(), domain.DashboardCreateInboxDomainInput{ - Name: "Example", - DomainAddress: "example.com", - Region: "eu", - }, "user-token", "org-token") - require.NoError(t, err) - assert.Equal(t, "dom_new", created.ID) - }, - }, - { - name: "update domain", - handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) { - assert.Equal(t, http.MethodPatch, r.Method) - assert.Equal(t, "/orgs/inbox/domains/dom_1", r.URL.Path) - assert.Equal(t, "us", r.URL.Query().Get("region")) - assert.Equal(t, "Renamed", body["name"]) - - writeDashboardEnvelope(t, w, map[string]any{ - "id": "dom_1", - "name": "Renamed", - "domainAddress": "example.com", - "region": "us", - }) - }, - run: func(t *testing.T, client *AccountClient) { - updated, err := client.UpdateDomain(context.Background(), "dom_1", "us", domain.DashboardUpdateInboxDomainInput{Name: "Renamed"}, "user-token", "org-token") - require.NoError(t, err) - assert.Equal(t, "Renamed", updated.Name) - }, - }, - { - name: "get domain info", - handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { - assert.Equal(t, http.MethodGet, r.Method) - assert.Equal(t, "/orgs/inbox/domains/dom_1/info", r.URL.Path) - assert.Equal(t, "us", r.URL.Query().Get("region")) - assert.Equal(t, "mx", r.URL.Query().Get("type")) - - writeDashboardEnvelope(t, w, map[string]any{ - "domainId": "dom_1", - "status": "pending", - "message": "Configure MX", - "attempt": map[string]any{ - "type": "mx", - "options": map[string]any{ - "host": "example.com", - "type": "MX", - "value": "10 inbound.nylas.com", - }, - }, - }) - }, - run: func(t *testing.T, client *AccountClient) { - info, err := client.GetDomainInfo(context.Background(), "dom_1", "us", "mx", "user-token", "org-token") - require.NoError(t, err) - assert.Equal(t, "pending", info.Status) - require.NotNil(t, info.Attempt) - assert.Equal(t, "MX", info.Attempt.Options.Type) - }, - }, - { - name: "verify domain", - handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, body map[string]any) { - assert.Equal(t, http.MethodPost, r.Method) - assert.Equal(t, "/orgs/inbox/domains/dom_1/verify", r.URL.Path) - assert.Equal(t, "eu", r.URL.Query().Get("region")) - assert.Equal(t, "ownership", body["type"]) - - writeDashboardEnvelope(t, w, map[string]any{ - "domainId": "dom_1", - "status": "done", - "message": "Verified", - }) - }, - run: func(t *testing.T, client *AccountClient) { - result, err := client.VerifyDomain(context.Background(), "dom_1", "eu", domain.DashboardVerifyInboxDomainInput{Type: "ownership"}, "user-token", "org-token") - require.NoError(t, err) - assert.Equal(t, "done", result.Status) - }, - }, - { - name: "delete domain", - handler: func(t *testing.T, w http.ResponseWriter, r *http.Request, _ []byte, _ map[string]any) { - assert.Equal(t, http.MethodDelete, r.Method) - assert.Equal(t, "/orgs/inbox/domains/dom_1", r.URL.Path) - assert.Equal(t, "us", r.URL.Query().Get("region")) - - writeDashboardEnvelope(t, w, map[string]any{"success": true}) - }, - run: func(t *testing.T, client *AccountClient) { - deleted, err := client.DeleteDomain(context.Background(), "dom_1", "us", "user-token", "org-token") - require.NoError(t, err) - assert.True(t, deleted) - }, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - server := newAccountClientTestServer(t, tt.handler) - defer server.Close() - - client := &AccountClient{ - baseURL: server.URL, - httpClient: server.Client(), - dpop: &mockDPoP{proof: "test-proof"}, - } - - tt.run(t, client) - }) - } -} - -func TestDecodeDomainPageFallbackShapes(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - raw rawResponse - wantIDs []string - wantCursor string - wantErr string - }{ - { - name: "object fallback with snake cursor", - raw: rawResponse{ - Data: []byte(`{"domains":[{"id":"dom_1","domainAddress":"example.com"}],"next_cursor":"cursor-2"}`), - }, - wantIDs: []string{"dom_1"}, - wantCursor: "cursor-2", - }, - { - name: "raw response cursor wins over object cursor", - raw: rawResponse{ - Data: []byte(`{"domains":[{"id":"dom_1","domainAddress":"example.com"}],"nextCursor":"payload-cursor"}`), - NextCursor: "envelope-cursor", - }, - wantIDs: []string{"dom_1"}, - wantCursor: "envelope-cursor", - }, - { - name: "empty object page with cursor", - raw: rawResponse{ - Data: []byte(`{"domains":[],"pageToken":"cursor-3"}`), - }, - wantCursor: "cursor-3", - }, - { - name: "object fallback requires domains field", - raw: rawResponse{ - Data: []byte(`{"nextCursor":"cursor-4"}`), - }, - wantErr: "missing domains", - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - page, err := decodeDomainPage(tt.raw) - - if tt.wantErr != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) - return - } - require.NoError(t, err) - assert.Equal(t, tt.wantCursor, page.NextCursor) - require.Len(t, page.Domains, len(tt.wantIDs)) - for i, id := range tt.wantIDs { - assert.Equal(t, id, page.Domains[i].ID) - } - }) - } -} - -func TestUnwrapRawResponseExtractsNestedCursor(t *testing.T) { - t.Parallel() - - raw, err := unwrapRawResponse([]byte(`{ - "request_id":"req", - "success":true, - "data":[], - "pagination":{"next_cursor":"cursor-2"} - }`)) - - require.NoError(t, err) - assert.JSONEq(t, `[]`, string(raw.Data)) - assert.Equal(t, "cursor-2", raw.NextCursor) -} diff --git a/internal/adapters/dpop/dpop.go b/internal/adapters/dpop/dpop.go index 0fb899e..1bdf55c 100644 --- a/internal/adapters/dpop/dpop.go +++ b/internal/adapters/dpop/dpop.go @@ -40,13 +40,20 @@ func New(secrets ports.SecretStore) (*Service, error) { return nil, fmt.Errorf("%w: failed to read the stored key: %w", domain.ErrDashboardDPoP, err) } if seedB64 != "" { + // A stored key that does not decode is reported, not replaced: it may + // be a damaged read rather than a damaged key, and overwriting it would + // orphan every session bound to the real one. seed, decErr := base64.StdEncoding.DecodeString(seedB64) - if decErr == nil && len(seed) == ed25519.SeedSize { - s.privateKey = ed25519.NewKeyFromSeed(seed) - s.publicKey = s.privateKey.Public().(ed25519.PublicKey) - s.thumbprint = s.computeThumbprint() - return s, nil + if decErr != nil { + return nil, fmt.Errorf("%w: the stored key is not valid base64: %w", domain.ErrDashboardDPoP, decErr) } + if len(seed) != ed25519.SeedSize { + return nil, fmt.Errorf("%w: the stored key is %d bytes, want %d", domain.ErrDashboardDPoP, len(seed), ed25519.SeedSize) + } + s.privateKey = ed25519.NewKeyFromSeed(seed) + s.publicKey = s.privateKey.Public().(ed25519.PublicKey) + s.thumbprint = s.computeThumbprint() + return s, nil } // Generate new keypair diff --git a/internal/adapters/dpop/dpop_seed_test.go b/internal/adapters/dpop/dpop_seed_test.go new file mode 100644 index 0000000..b33a72c --- /dev/null +++ b/internal/adapters/dpop/dpop_seed_test.go @@ -0,0 +1,38 @@ +package dpop + +import ( + "encoding/base64" + "testing" + + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNew_MalformedStoredKeyIsReportedNotReplaced(t *testing.T) { + t.Parallel() + // The server binds sessions to the stored key. One that does not decode + // may be a damaged read; replacing it would orphan every bound session, + // so the user is told instead. + tests := []struct { + name string + stored string + }{ + {name: "not base64", stored: "%%% not base64 %%%"}, + {name: "wrong length", stored: base64.StdEncoding.EncodeToString([]byte("too short"))}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + store := newMockSecretStore() + store.data[ports.KeyDashboardDPoPKey] = tt.stored + + svc, err := New(store) + + require.ErrorIs(t, err, domain.ErrDashboardDPoP) + assert.Nil(t, svc) + assert.Equal(t, tt.stored, store.data[ports.KeyDashboardDPoPKey]) + }) + } +} diff --git a/internal/adapters/mcp/proxy_e2e_normalize_test.go b/internal/adapters/mcp/proxy_e2e_normalize_test.go new file mode 100644 index 0000000..de3c343 --- /dev/null +++ b/internal/adapters/mcp/proxy_e2e_normalize_test.go @@ -0,0 +1,422 @@ +package mcp + +import ( + "net/http/httptest" + "testing" +) + +// TestE2E_NormalizeListEventsArgs verifies the proxy coerces integer start/end +// to strings before forwarding list_events to the upstream server. +func TestE2E_NormalizeListEventsArgs(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + proxy.SetDefaultGrant("grant-norm") + + ctx := t.Context() + + t.Run("integer_start_end_coerced_to_strings", func(t *testing.T) { + req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000,"limit":5}}}}`) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + + params, ok := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if !ok { + t.Fatal("expected get_all_query_parameters in forwarded args") + } + + start, ok := params["start"].(string) + if !ok { + t.Errorf("expected start to be string, got %T (%v)", params["start"], params["start"]) + } else if start != "1747065600" { + t.Errorf("start = %q, want %q", start, "1747065600") + } + + end, ok := params["end"].(string) + if !ok { + t.Errorf("expected end to be string, got %T (%v)", params["end"], params["end"]) + } else if end != "1747152000" { + t.Errorf("end = %q, want %q", end, "1747152000") + } + }) + + t.Run("string_start_end_preserved", func(t *testing.T) { + req := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":"1747065600","end":"1747152000","limit":5}}}}`) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + + params, ok := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if !ok { + t.Fatal("expected get_all_query_parameters in forwarded args") + } + + if start, ok := params["start"].(string); !ok || start != "1747065600" { + t.Errorf("expected start string preserved, got %T %v", params["start"], params["start"]) + } + }) +} + +// TestE2E_NormalizeAvailabilityArgs verifies the proxy rounds availability +// timestamps to 5-minute boundaries before forwarding. +func TestE2E_NormalizeAvailabilityArgs(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + + ctx := t.Context() + + t.Run("unaligned_timestamps_rounded", func(t *testing.T) { + req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":1747065601,"end_time":1747065899,"duration_minutes":30,"interval_minutes":30,"participants":[{"email":"user@example.com"}]}}}}`) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + + avail, ok := mock.lastArgs["availability_request"].(map[string]any) + if !ok { + t.Fatal("expected availability_request in forwarded args") + } + + startTime, ok := avail["start_time"].(float64) + if !ok { + t.Fatalf("expected start_time to be float64, got %T", avail["start_time"]) + } + if startTime != 1747065600 { + t.Errorf("start_time = %v, want %v (rounded down)", startTime, 1747065600) + } + + endTime, ok := avail["end_time"].(float64) + if !ok { + t.Fatalf("expected end_time to be float64, got %T", avail["end_time"]) + } + if endTime != 1747065900 { + t.Errorf("end_time = %v, want %v (rounded up)", endTime, 1747065900) + } + }) + + t.Run("aligned_timestamps_unchanged", func(t *testing.T) { + req := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":1747065600,"end_time":1747066200,"duration_minutes":30,"interval_minutes":30,"participants":[{"email":"user@example.com"}]}}}}`) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + + avail, ok := mock.lastArgs["availability_request"].(map[string]any) + if !ok { + t.Fatal("expected availability_request in forwarded args") + } + + if startTime, ok := avail["start_time"].(float64); !ok || startTime != 1747065600 { + t.Errorf("start_time = %v, want %v (should be unchanged)", avail["start_time"], 1747065600) + } + if endTime, ok := avail["end_time"].(float64); !ok || endTime != 1747066200 { + t.Errorf("end_time = %v, want %v (should be unchanged)", avail["end_time"], 1747066200) + } + }) +} + +// TestE2E_NormalizeListEvents_GrantInjectionCombined verifies grant injection and +// list_events normalization work together through the full forward path. +func TestE2E_NormalizeListEvents_GrantInjectionCombined(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + proxy.SetDefaultGrant("combined-grant-xyz") + + ctx := t.Context() + + // No grant_id in request + integer start/end — both should be fixed + req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000,"limit":3}}}}`) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + + // Verify grant was injected + if mock.receivedGrant != "combined-grant-xyz" { + t.Errorf("grant_id = %q, want 'combined-grant-xyz'", mock.receivedGrant) + } + + // Verify start/end were coerced to strings + params, ok := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if !ok { + t.Fatal("expected get_all_query_parameters") + } + if start, ok := params["start"].(string); !ok || start != "1747065600" { + t.Errorf("start = %v (%T), want string '1747065600'", params["start"], params["start"]) + } + if end, ok := params["end"].(string); !ok || end != "1747152000" { + t.Errorf("end = %v (%T), want string '1747152000'", params["end"], params["end"]) + } + + // Verify limit was NOT converted to string + if limit, ok := params["limit"].(float64); !ok || limit != 3 { + t.Errorf("limit = %v (%T), want float64 3", params["limit"], params["limit"]) + } +} + +// TestE2E_NormalizeListEvents_ExplicitGrantPreserved verifies that explicit +// grant_id is preserved while start/end are still normalized. +func TestE2E_NormalizeListEvents_ExplicitGrantPreserved(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + proxy.SetDefaultGrant("default-grant") + + ctx := t.Context() + + req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"grant_id":"user-explicit-grant","get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000}}}}`) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + + // User's explicit grant should be preserved, not overridden + if mock.receivedGrant != "user-explicit-grant" { + t.Errorf("grant_id = %q, want 'user-explicit-grant'", mock.receivedGrant) + } + + // start/end should still be normalized + params := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if _, ok := params["start"].(string); !ok { + t.Errorf("start should be string even with explicit grant, got %T", params["start"]) + } +} + +// TestE2E_NormalizeAvailability_WithParticipants verifies availability rounding +// doesn't affect participant data. +func TestE2E_NormalizeAvailability_WithParticipants(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + + ctx := t.Context() + + req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":1747065601,"end_time":1747065899,"duration_minutes":30,"interval_minutes":15,"participants":[{"email":"alice@example.com"},{"email":"bob@example.com","open_hours":[{"days":[1,2,3,4,5],"start":"09:00","end":"17:00","timezone":"America/New_York","exdates":[]}]}]}}}}`) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + + avail := mock.lastArgs["availability_request"].(map[string]any) + + // Timestamps should be rounded + if st, ok := avail["start_time"].(float64); !ok || st != 1747065600 { + t.Errorf("start_time = %v, want 1747065600", avail["start_time"]) + } + if et, ok := avail["end_time"].(float64); !ok || et != 1747065900 { + t.Errorf("end_time = %v, want 1747065900", avail["end_time"]) + } + + // Participants should be fully preserved + participants, ok := avail["participants"].([]any) + if !ok || len(participants) != 2 { + t.Fatalf("expected 2 participants, got %v", avail["participants"]) + } + + p1 := participants[0].(map[string]any) + if p1["email"] != "alice@example.com" { + t.Errorf("first participant email = %v", p1["email"]) + } + + p2 := participants[1].(map[string]any) + if p2["email"] != "bob@example.com" { + t.Errorf("second participant email = %v", p2["email"]) + } + + // open_hours on participant 2 should be preserved + openHours, ok := p2["open_hours"].([]any) + if !ok || len(openHours) != 1 { + t.Fatalf("expected 1 open_hours entry on p2, got %v", p2["open_hours"]) + } + oh := openHours[0].(map[string]any) + if oh["timezone"] != "America/New_York" { + t.Errorf("open_hours timezone = %v, want America/New_York", oh["timezone"]) + } + + // duration and interval should be preserved + if dur, ok := avail["duration_minutes"].(float64); !ok || dur != 30 { + t.Errorf("duration_minutes = %v, want 30", avail["duration_minutes"]) + } + if iv, ok := avail["interval_minutes"].(float64); !ok || iv != 15 { + t.Errorf("interval_minutes = %v, want 15", avail["interval_minutes"]) + } +} + +// TestE2E_NormalizationAfterDiscovery verifies that normalization works correctly +// after tools/list dynamic discovery has populated grantTools. +func TestE2E_NormalizationAfterDiscovery(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + proxy.SetDefaultGrant("disc-grant") + + ctx := t.Context() + + // Step 1: trigger discovery + toolsReq := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}`) + _, err := proxy.forward(ctx, toolsReq.raw, toolsReq.parsed) + if err != nil { + t.Fatalf("tools/list failed: %v", err) + } + + // Step 2: list_events with integer start/end — should normalize + inject grant + eventsReq := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000}}}}`) + _, err = proxy.forward(ctx, eventsReq.raw, eventsReq.parsed) + if err != nil { + t.Fatalf("list_events failed: %v", err) + } + + if mock.receivedGrant != "disc-grant" { + t.Errorf("post-discovery grant = %q, want 'disc-grant'", mock.receivedGrant) + } + params := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if _, ok := params["start"].(string); !ok { + t.Errorf("post-discovery start not coerced, got %T", params["start"]) + } +} + +// TestE2E_NormalizationDoesNotAffectOtherTools verifies that tools like +// list_messages, list_threads, etc. are forwarded without modification even +// when they contain get_all_query_parameters. +func TestE2E_NormalizationDoesNotAffectOtherTools(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + proxy.SetDefaultGrant("other-grant") + + ctx := t.Context() + + tests := []struct { + name string + input string + check func(t *testing.T) + }{ + { + name: "list_messages numeric fields unchanged", + input: `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_messages","arguments":{"get_all_query_parameters":{"limit":10,"received_after":1747065600}}}}`, + check: func(t *testing.T) { + params := mock.lastArgs["get_all_query_parameters"].(map[string]any) + // received_after should stay numeric, not converted to string + if _, ok := params["received_after"].(float64); !ok { + t.Errorf("received_after should remain numeric, got %T", params["received_after"]) + } + }, + }, + { + name: "list_threads numeric fields unchanged", + input: `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"list_threads","arguments":{"get_all_query_parameters":{"limit":5}}}}`, + check: func(t *testing.T) { + params := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if limit, ok := params["limit"].(float64); !ok || limit != 5 { + t.Errorf("limit = %v, want 5", params["limit"]) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := parseRPC(tt.input) + _, err := proxy.forward(ctx, req.raw, req.parsed) + if err != nil { + t.Fatalf("forward failed: %v", err) + } + tt.check(t) + }) + } +} + +// TestE2E_MultipleSequentialNormalizations verifies normalization works correctly +// across multiple sequential calls on the same proxy instance. +func TestE2E_MultipleSequentialNormalizations(t *testing.T) { + t.Parallel() + + mock := &mockMCPServer{t: t} + server := httptest.NewServer(mock) + defer server.Close() + + proxy := NewProxy("test-api-key", "us") + proxy.endpoint = server.URL + proxy.SetDefaultGrant("seq-grant") + + ctx := t.Context() + + // Call 1: list_events + req1 := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":100,"end":200}}}}`) + _, err := proxy.forward(ctx, req1.raw, req1.parsed) + if err != nil { + t.Fatalf("call 1 failed: %v", err) + } + params1 := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if params1["start"] != "100" { + t.Errorf("call 1 start = %v, want '100'", params1["start"]) + } + + // Call 2: availability + req2 := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":301,"end_time":599,"duration_minutes":30,"participants":[]}}}}`) + _, err = proxy.forward(ctx, req2.raw, req2.parsed) + if err != nil { + t.Fatalf("call 2 failed: %v", err) + } + avail := mock.lastArgs["availability_request"].(map[string]any) + if st, ok := avail["start_time"].(float64); !ok || st != 300 { + t.Errorf("call 2 start_time = %v, want 300", avail["start_time"]) + } + if et, ok := avail["end_time"].(float64); !ok || et != 600 { + t.Errorf("call 2 end_time = %v, want 600", avail["end_time"]) + } + + // Call 3: list_events again with different values + req3 := parseRPC(`{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":999,"end":1000}}}}`) + _, err = proxy.forward(ctx, req3.raw, req3.parsed) + if err != nil { + t.Fatalf("call 3 failed: %v", err) + } + params3 := mock.lastArgs["get_all_query_parameters"].(map[string]any) + if params3["start"] != "999" { + t.Errorf("call 3 start = %v, want '999'", params3["start"]) + } + if params3["end"] != "1000" { + t.Errorf("call 3 end = %v, want '1000'", params3["end"]) + } +} diff --git a/internal/adapters/mcp/proxy_e2e_test.go b/internal/adapters/mcp/proxy_e2e_test.go index f75fdc8..14c468e 100644 --- a/internal/adapters/mcp/proxy_e2e_test.go +++ b/internal/adapters/mcp/proxy_e2e_test.go @@ -419,422 +419,6 @@ func TestE2E_ServerError(t *testing.T) { } } -// TestE2E_NormalizeListEventsArgs verifies the proxy coerces integer start/end -// to strings before forwarding list_events to the upstream server. -func TestE2E_NormalizeListEventsArgs(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - proxy.SetDefaultGrant("grant-norm") - - ctx := t.Context() - - t.Run("integer_start_end_coerced_to_strings", func(t *testing.T) { - req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000,"limit":5}}}}`) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - - params, ok := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if !ok { - t.Fatal("expected get_all_query_parameters in forwarded args") - } - - start, ok := params["start"].(string) - if !ok { - t.Errorf("expected start to be string, got %T (%v)", params["start"], params["start"]) - } else if start != "1747065600" { - t.Errorf("start = %q, want %q", start, "1747065600") - } - - end, ok := params["end"].(string) - if !ok { - t.Errorf("expected end to be string, got %T (%v)", params["end"], params["end"]) - } else if end != "1747152000" { - t.Errorf("end = %q, want %q", end, "1747152000") - } - }) - - t.Run("string_start_end_preserved", func(t *testing.T) { - req := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":"1747065600","end":"1747152000","limit":5}}}}`) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - - params, ok := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if !ok { - t.Fatal("expected get_all_query_parameters in forwarded args") - } - - if start, ok := params["start"].(string); !ok || start != "1747065600" { - t.Errorf("expected start string preserved, got %T %v", params["start"], params["start"]) - } - }) -} - -// TestE2E_NormalizeAvailabilityArgs verifies the proxy rounds availability -// timestamps to 5-minute boundaries before forwarding. -func TestE2E_NormalizeAvailabilityArgs(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - - ctx := t.Context() - - t.Run("unaligned_timestamps_rounded", func(t *testing.T) { - req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":1747065601,"end_time":1747065899,"duration_minutes":30,"interval_minutes":30,"participants":[{"email":"user@example.com"}]}}}}`) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - - avail, ok := mock.lastArgs["availability_request"].(map[string]any) - if !ok { - t.Fatal("expected availability_request in forwarded args") - } - - startTime, ok := avail["start_time"].(float64) - if !ok { - t.Fatalf("expected start_time to be float64, got %T", avail["start_time"]) - } - if startTime != 1747065600 { - t.Errorf("start_time = %v, want %v (rounded down)", startTime, 1747065600) - } - - endTime, ok := avail["end_time"].(float64) - if !ok { - t.Fatalf("expected end_time to be float64, got %T", avail["end_time"]) - } - if endTime != 1747065900 { - t.Errorf("end_time = %v, want %v (rounded up)", endTime, 1747065900) - } - }) - - t.Run("aligned_timestamps_unchanged", func(t *testing.T) { - req := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":1747065600,"end_time":1747066200,"duration_minutes":30,"interval_minutes":30,"participants":[{"email":"user@example.com"}]}}}}`) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - - avail, ok := mock.lastArgs["availability_request"].(map[string]any) - if !ok { - t.Fatal("expected availability_request in forwarded args") - } - - if startTime, ok := avail["start_time"].(float64); !ok || startTime != 1747065600 { - t.Errorf("start_time = %v, want %v (should be unchanged)", avail["start_time"], 1747065600) - } - if endTime, ok := avail["end_time"].(float64); !ok || endTime != 1747066200 { - t.Errorf("end_time = %v, want %v (should be unchanged)", avail["end_time"], 1747066200) - } - }) -} - -// TestE2E_NormalizeListEvents_GrantInjectionCombined verifies grant injection and -// list_events normalization work together through the full forward path. -func TestE2E_NormalizeListEvents_GrantInjectionCombined(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - proxy.SetDefaultGrant("combined-grant-xyz") - - ctx := t.Context() - - // No grant_id in request + integer start/end — both should be fixed - req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000,"limit":3}}}}`) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - - // Verify grant was injected - if mock.receivedGrant != "combined-grant-xyz" { - t.Errorf("grant_id = %q, want 'combined-grant-xyz'", mock.receivedGrant) - } - - // Verify start/end were coerced to strings - params, ok := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if !ok { - t.Fatal("expected get_all_query_parameters") - } - if start, ok := params["start"].(string); !ok || start != "1747065600" { - t.Errorf("start = %v (%T), want string '1747065600'", params["start"], params["start"]) - } - if end, ok := params["end"].(string); !ok || end != "1747152000" { - t.Errorf("end = %v (%T), want string '1747152000'", params["end"], params["end"]) - } - - // Verify limit was NOT converted to string - if limit, ok := params["limit"].(float64); !ok || limit != 3 { - t.Errorf("limit = %v (%T), want float64 3", params["limit"], params["limit"]) - } -} - -// TestE2E_NormalizeListEvents_ExplicitGrantPreserved verifies that explicit -// grant_id is preserved while start/end are still normalized. -func TestE2E_NormalizeListEvents_ExplicitGrantPreserved(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - proxy.SetDefaultGrant("default-grant") - - ctx := t.Context() - - req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"grant_id":"user-explicit-grant","get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000}}}}`) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - - // User's explicit grant should be preserved, not overridden - if mock.receivedGrant != "user-explicit-grant" { - t.Errorf("grant_id = %q, want 'user-explicit-grant'", mock.receivedGrant) - } - - // start/end should still be normalized - params := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if _, ok := params["start"].(string); !ok { - t.Errorf("start should be string even with explicit grant, got %T", params["start"]) - } -} - -// TestE2E_NormalizeAvailability_WithParticipants verifies availability rounding -// doesn't affect participant data. -func TestE2E_NormalizeAvailability_WithParticipants(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - - ctx := t.Context() - - req := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":1747065601,"end_time":1747065899,"duration_minutes":30,"interval_minutes":15,"participants":[{"email":"alice@example.com"},{"email":"bob@example.com","open_hours":[{"days":[1,2,3,4,5],"start":"09:00","end":"17:00","timezone":"America/New_York","exdates":[]}]}]}}}}`) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - - avail := mock.lastArgs["availability_request"].(map[string]any) - - // Timestamps should be rounded - if st, ok := avail["start_time"].(float64); !ok || st != 1747065600 { - t.Errorf("start_time = %v, want 1747065600", avail["start_time"]) - } - if et, ok := avail["end_time"].(float64); !ok || et != 1747065900 { - t.Errorf("end_time = %v, want 1747065900", avail["end_time"]) - } - - // Participants should be fully preserved - participants, ok := avail["participants"].([]any) - if !ok || len(participants) != 2 { - t.Fatalf("expected 2 participants, got %v", avail["participants"]) - } - - p1 := participants[0].(map[string]any) - if p1["email"] != "alice@example.com" { - t.Errorf("first participant email = %v", p1["email"]) - } - - p2 := participants[1].(map[string]any) - if p2["email"] != "bob@example.com" { - t.Errorf("second participant email = %v", p2["email"]) - } - - // open_hours on participant 2 should be preserved - openHours, ok := p2["open_hours"].([]any) - if !ok || len(openHours) != 1 { - t.Fatalf("expected 1 open_hours entry on p2, got %v", p2["open_hours"]) - } - oh := openHours[0].(map[string]any) - if oh["timezone"] != "America/New_York" { - t.Errorf("open_hours timezone = %v, want America/New_York", oh["timezone"]) - } - - // duration and interval should be preserved - if dur, ok := avail["duration_minutes"].(float64); !ok || dur != 30 { - t.Errorf("duration_minutes = %v, want 30", avail["duration_minutes"]) - } - if iv, ok := avail["interval_minutes"].(float64); !ok || iv != 15 { - t.Errorf("interval_minutes = %v, want 15", avail["interval_minutes"]) - } -} - -// TestE2E_NormalizationAfterDiscovery verifies that normalization works correctly -// after tools/list dynamic discovery has populated grantTools. -func TestE2E_NormalizationAfterDiscovery(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - proxy.SetDefaultGrant("disc-grant") - - ctx := t.Context() - - // Step 1: trigger discovery - toolsReq := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}`) - _, err := proxy.forward(ctx, toolsReq.raw, toolsReq.parsed) - if err != nil { - t.Fatalf("tools/list failed: %v", err) - } - - // Step 2: list_events with integer start/end — should normalize + inject grant - eventsReq := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":1747065600,"end":1747152000}}}}`) - _, err = proxy.forward(ctx, eventsReq.raw, eventsReq.parsed) - if err != nil { - t.Fatalf("list_events failed: %v", err) - } - - if mock.receivedGrant != "disc-grant" { - t.Errorf("post-discovery grant = %q, want 'disc-grant'", mock.receivedGrant) - } - params := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if _, ok := params["start"].(string); !ok { - t.Errorf("post-discovery start not coerced, got %T", params["start"]) - } -} - -// TestE2E_NormalizationDoesNotAffectOtherTools verifies that tools like -// list_messages, list_threads, etc. are forwarded without modification even -// when they contain get_all_query_parameters. -func TestE2E_NormalizationDoesNotAffectOtherTools(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - proxy.SetDefaultGrant("other-grant") - - ctx := t.Context() - - tests := []struct { - name string - input string - check func(t *testing.T) - }{ - { - name: "list_messages numeric fields unchanged", - input: `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_messages","arguments":{"get_all_query_parameters":{"limit":10,"received_after":1747065600}}}}`, - check: func(t *testing.T) { - params := mock.lastArgs["get_all_query_parameters"].(map[string]any) - // received_after should stay numeric, not converted to string - if _, ok := params["received_after"].(float64); !ok { - t.Errorf("received_after should remain numeric, got %T", params["received_after"]) - } - }, - }, - { - name: "list_threads numeric fields unchanged", - input: `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"list_threads","arguments":{"get_all_query_parameters":{"limit":5}}}}`, - check: func(t *testing.T) { - params := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if limit, ok := params["limit"].(float64); !ok || limit != 5 { - t.Errorf("limit = %v, want 5", params["limit"]) - } - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - req := parseRPC(tt.input) - _, err := proxy.forward(ctx, req.raw, req.parsed) - if err != nil { - t.Fatalf("forward failed: %v", err) - } - tt.check(t) - }) - } -} - -// TestE2E_MultipleSequentialNormalizations verifies normalization works correctly -// across multiple sequential calls on the same proxy instance. -func TestE2E_MultipleSequentialNormalizations(t *testing.T) { - t.Parallel() - - mock := &mockMCPServer{t: t} - server := httptest.NewServer(mock) - defer server.Close() - - proxy := NewProxy("test-api-key", "us") - proxy.endpoint = server.URL - proxy.SetDefaultGrant("seq-grant") - - ctx := t.Context() - - // Call 1: list_events - req1 := parseRPC(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":100,"end":200}}}}`) - _, err := proxy.forward(ctx, req1.raw, req1.parsed) - if err != nil { - t.Fatalf("call 1 failed: %v", err) - } - params1 := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if params1["start"] != "100" { - t.Errorf("call 1 start = %v, want '100'", params1["start"]) - } - - // Call 2: availability - req2 := parseRPC(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"availability","arguments":{"availability_request":{"start_time":301,"end_time":599,"duration_minutes":30,"participants":[]}}}}`) - _, err = proxy.forward(ctx, req2.raw, req2.parsed) - if err != nil { - t.Fatalf("call 2 failed: %v", err) - } - avail := mock.lastArgs["availability_request"].(map[string]any) - if st, ok := avail["start_time"].(float64); !ok || st != 300 { - t.Errorf("call 2 start_time = %v, want 300", avail["start_time"]) - } - if et, ok := avail["end_time"].(float64); !ok || et != 600 { - t.Errorf("call 2 end_time = %v, want 600", avail["end_time"]) - } - - // Call 3: list_events again with different values - req3 := parseRPC(`{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"list_events","arguments":{"get_all_query_parameters":{"calendar_id":"primary","start":999,"end":1000}}}}`) - _, err = proxy.forward(ctx, req3.raw, req3.parsed) - if err != nil { - t.Fatalf("call 3 failed: %v", err) - } - params3 := mock.lastArgs["get_all_query_parameters"].(map[string]any) - if params3["start"] != "999" { - t.Errorf("call 3 start = %v, want '999'", params3["start"]) - } - if params3["end"] != "1000" { - t.Errorf("call 3 end = %v, want '1000'", params3["end"]) - } -} - // --- helpers --- type parsedRPC struct { diff --git a/internal/adapters/mcp/proxy_response.go b/internal/adapters/mcp/proxy_response.go index 382a7d6..bbc015b 100644 --- a/internal/adapters/mcp/proxy_response.go +++ b/internal/adapters/mcp/proxy_response.go @@ -211,7 +211,7 @@ func (p *Proxy) modifyToolsListResponse(response []byte) []byte { } } - // Cache the discovered grant tools for use in injectDefaultGrant + // Cache the discovered grant tools for use in injectGrant if len(discovered) > 0 { p.mu.Lock() p.grantTools = discovered diff --git a/internal/app/auth/service_login_test.go b/internal/app/auth/service_login_test.go new file mode 100644 index 0000000..61a67a9 --- /dev/null +++ b/internal/app/auth/service_login_test.go @@ -0,0 +1,306 @@ +package auth + +import ( + "context" + "crypto/sha256" + "encoding/base64" + "errors" + "fmt" + "testing" + + "github.com/nylas/cli/internal/adapters/nylas" + "github.com/nylas/cli/internal/domain" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestService_Login(t *testing.T) { + t.Run("successful login sets grant as default", func(t *testing.T) { + client := nylas.NewMockClient() + var capturedState string + var capturedChallenge string + client.BuildAuthURLFunc = func(provider domain.Provider, redirectURI, state, codeChallenge string) string { + capturedState = state + capturedChallenge = codeChallenge + return "https://mock.nylas.com/auth?state=" + state + } + client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { + assert.Equal(t, "auth-code-123", code) + assert.Equal(t, capturedChallenge, pkceChallenge(codeVerifier)) + return &domain.Grant{ + ID: "grant-123", + Email: "user@example.com", + Provider: domain.ProviderGoogle, + }, nil + } + + grantStore := newMockGrantStore() + configStore := newMockConfigStore() + server := &mockOAuthServer{ + redirectURI: "http://localhost:8080/callback", + code: "auth-code-123", + } + var stateWhenBrowserOpened string + browser := &mockBrowser{onOpen: func() { stateWhenBrowserOpened = server.setState }} + + svc := NewService(client, grantStore, configStore, server, browser) + + grant, err := svc.Login(context.Background(), domain.ProviderGoogle) + + require.NoError(t, err) + assert.NotNil(t, grant) + assert.Equal(t, "grant-123", grant.ID) + assert.Equal(t, "user@example.com", grant.Email) + + // Verify server was started and stopped + assert.True(t, server.startCalled) + assert.True(t, server.stopCalled) + + assert.Equal(t, capturedState, stateWhenBrowserOpened, + "a fast redirect must be checked against this login's state, not an unset one") + + // Verify browser and callback state/PKCE values were wired through. + assert.Equal(t, "https://mock.nylas.com/auth?state="+capturedState, browser.openedURL) + assert.Equal(t, capturedState, server.expectedState) + assert.NotEmpty(t, capturedState) + assert.NotEmpty(t, capturedChallenge) + + // Verify grant was saved + savedGrant, err := grantStore.GetGrant("grant-123") + require.NoError(t, err) + assert.Equal(t, "grant-123", savedGrant.ID) + + // Verify grant was set as default + defaultID, err := grantStore.GetDefaultGrant() + require.NoError(t, err) + assert.Equal(t, "grant-123", defaultID) + assert.Equal(t, "grant-123", configStore.config.DefaultGrant) + assert.Empty(t, configStore.config.Grants) + }) + + t.Run("default persistence failure returns error", func(t *testing.T) { + client := nylas.NewMockClient() + client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { + return &domain.Grant{ + ID: "grant-123", + Email: "user@example.com", + Provider: domain.ProviderGoogle, + }, nil + } + + grantStore := newMockGrantStore() + configStore := &failingSaveConfigStore{config: &domain.Config{Region: "us"}} + server := &mockOAuthServer{ + redirectURI: "http://localhost:8080/callback", + code: "auth-code-123", + } + browser := &mockBrowser{} + + svc := NewService(client, grantStore, configStore, server, browser) + + grant, err := svc.Login(context.Background(), domain.ProviderGoogle) + + require.Error(t, err) + assert.Nil(t, grant) + assert.ErrorIs(t, err, domain.ErrInvalidInput) + _, defaultErr := grantStore.GetDefaultGrant() + assert.ErrorIs(t, defaultErr, domain.ErrNoDefaultGrant) + }) + + t.Run("existing default is mirrored to config after login", func(t *testing.T) { + client := nylas.NewMockClient() + client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { + return &domain.Grant{ + ID: "grant-2", + Email: "second@example.com", + Provider: domain.ProviderMicrosoft, + }, nil + } + + grantStore := newMockGrantStore() + require.NoError(t, grantStore.SaveGrant(domain.GrantInfo{ID: "grant-1", Email: "first@example.com", Provider: domain.ProviderGoogle})) + require.NoError(t, grantStore.SetDefaultGrant("grant-1")) + configStore := newMockConfigStore() + configStore.config.DefaultGrant = "stale-config-default" + server := &mockOAuthServer{ + redirectURI: "http://localhost:8080/callback", + code: "auth-code-123", + } + browser := &mockBrowser{} + + svc := NewService(client, grantStore, configStore, server, browser) + + grant, err := svc.Login(context.Background(), domain.ProviderMicrosoft) + + require.NoError(t, err) + require.NotNil(t, grant) + defaultID, err := grantStore.GetDefaultGrant() + require.NoError(t, err) + assert.Equal(t, "grant-1", defaultID) + assert.Equal(t, "grant-1", configStore.config.DefaultGrant) + assert.Empty(t, configStore.config.Grants) + }) + + t.Run("server start failure returns error", func(t *testing.T) { + server := &mockOAuthServer{ + startErr: errors.New("server start failed"), + } + svc := NewService(nylas.NewMockClient(), newMockGrantStore(), newMockConfigStore(), server, &mockBrowser{}) + + _, err := svc.Login(context.Background(), domain.ProviderGoogle) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "server start failed") + }) + + t.Run("browser open failure returns error", func(t *testing.T) { + browser := &mockBrowser{ + openErr: errors.New("browser open failed"), + } + svc := NewService( + nylas.NewMockClient(), + newMockGrantStore(), + newMockConfigStore(), + &mockOAuthServer{redirectURI: "http://localhost:8080/callback"}, + browser, + ) + + _, err := svc.Login(context.Background(), domain.ProviderGoogle) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "browser open failed") + }) + + t.Run("callback wait failure returns error", func(t *testing.T) { + server := &mockOAuthServer{ + redirectURI: "http://localhost:8080/callback", + waitErr: errors.New("callback wait failed"), + } + svc := NewService( + nylas.NewMockClient(), + newMockGrantStore(), + newMockConfigStore(), + server, + &mockBrowser{}, + ) + + _, err := svc.Login(context.Background(), domain.ProviderGoogle) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "callback wait failed") + }) + + t.Run("code exchange failure returns error", func(t *testing.T) { + client := nylas.NewMockClient() + client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { + return nil, errors.New("code exchange failed") + } + + server := &mockOAuthServer{ + redirectURI: "http://localhost:8080/callback", + code: "auth-code", + } + svc := NewService(client, newMockGrantStore(), newMockConfigStore(), server, &mockBrowser{}) + + _, err := svc.Login(context.Background(), domain.ProviderGoogle) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "code exchange failed") + }) +} + +func TestService_LoginWithCredentials(t *testing.T) { + t.Run("successful icloud login saves grant and sets default", func(t *testing.T) { + client := nylas.NewMockClient() + client.CreateCustomGrantFunc = func(ctx context.Context, provider string, settings map[string]any) (*domain.Grant, error) { + assert.Equal(t, "icloud", provider) + assert.Equal(t, "user@icloud.com", settings["username"]) + return &domain.Grant{ + ID: "icloud-grant-001", + Email: "user@icloud.com", + Provider: domain.ProviderICloud, + GrantStatus: "valid", + }, nil + } + + grantStore := newMockGrantStore() + configStore := newMockConfigStore() + + svc := NewService(client, grantStore, configStore, &mockOAuthServer{}, &mockBrowser{}) + + grant, err := svc.LoginWithCredentials(context.Background(), "icloud", map[string]any{ + "username": "user@icloud.com", + "password": "app-specific-password", + }) + + require.NoError(t, err) + assert.Equal(t, "icloud-grant-001", grant.ID) + assert.Equal(t, "user@icloud.com", grant.Email) + + savedGrant, err := grantStore.GetGrant("icloud-grant-001") + require.NoError(t, err) + assert.Equal(t, domain.ProviderICloud, savedGrant.Provider) + + defaultID, err := grantStore.GetDefaultGrant() + require.NoError(t, err) + assert.Equal(t, "icloud-grant-001", defaultID) + }) + + t.Run("does not override existing default", func(t *testing.T) { + client := nylas.NewMockClient() + client.CreateCustomGrantFunc = func(ctx context.Context, provider string, settings map[string]any) (*domain.Grant, error) { + return &domain.Grant{ + ID: "yahoo-grant-001", + Email: "user@yahoo.com", + Provider: domain.ProviderIMAP, + }, nil + } + + grantStore := newMockGrantStore() + require.NoError(t, grantStore.SaveGrant(domain.GrantInfo{ID: "existing-grant", Email: "first@example.com"})) + require.NoError(t, grantStore.SetDefaultGrant("existing-grant")) + configStore := newMockConfigStore() + + svc := NewService(client, grantStore, configStore, &mockOAuthServer{}, &mockBrowser{}) + + grant, err := svc.LoginWithCredentials(context.Background(), "imap", map[string]any{ + "imap_username": "user@yahoo.com", + "imap_password": "secret", + "imap_host": "imap.mail.yahoo.com", + "imap_port": 993, + "type": "yahoo", + }) + + require.NoError(t, err) + assert.Equal(t, "yahoo-grant-001", grant.ID) + + defaultID, err := grantStore.GetDefaultGrant() + require.NoError(t, err) + assert.Equal(t, "existing-grant", defaultID) + }) + + t.Run("api error propagates", func(t *testing.T) { + client := nylas.NewMockClient() + client.CreateCustomGrantFunc = func(ctx context.Context, provider string, settings map[string]any) (*domain.Grant, error) { + return nil, errors.New("Invalid IMAP credentials") + } + + svc := NewService(client, newMockGrantStore(), newMockConfigStore(), &mockOAuthServer{}, &mockBrowser{}) + + _, err := svc.LoginWithCredentials(context.Background(), "imap", map[string]any{ + "imap_username": "bad@example.com", + "imap_password": "wrong", + "imap_host": "mail.example.com", + "imap_port": 993, + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "Invalid IMAP credentials") + }) +} + +func pkceChallenge(verifier string) string { + hash := sha256.Sum256([]byte(verifier)) + hexHash := fmt.Sprintf("%x", hash) + return base64.RawStdEncoding.EncodeToString([]byte(hexHash)) +} diff --git a/internal/app/auth/service_test.go b/internal/app/auth/service_test.go index 4289fb0..8b88ed8 100644 --- a/internal/app/auth/service_test.go +++ b/internal/app/auth/service_test.go @@ -2,10 +2,7 @@ package auth import ( "context" - "crypto/sha256" - "encoding/base64" "errors" - "fmt" "testing" "github.com/nylas/cli/internal/adapters/nylas" @@ -346,201 +343,6 @@ func TestNewService(t *testing.T) { assert.Equal(t, browser, svc.browser) } -func TestService_Login(t *testing.T) { - t.Run("successful login sets grant as default", func(t *testing.T) { - client := nylas.NewMockClient() - var capturedState string - var capturedChallenge string - client.BuildAuthURLFunc = func(provider domain.Provider, redirectURI, state, codeChallenge string) string { - capturedState = state - capturedChallenge = codeChallenge - return "https://mock.nylas.com/auth?state=" + state - } - client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { - assert.Equal(t, "auth-code-123", code) - assert.Equal(t, capturedChallenge, pkceChallenge(codeVerifier)) - return &domain.Grant{ - ID: "grant-123", - Email: "user@example.com", - Provider: domain.ProviderGoogle, - }, nil - } - - grantStore := newMockGrantStore() - configStore := newMockConfigStore() - server := &mockOAuthServer{ - redirectURI: "http://localhost:8080/callback", - code: "auth-code-123", - } - var stateWhenBrowserOpened string - browser := &mockBrowser{onOpen: func() { stateWhenBrowserOpened = server.setState }} - - svc := NewService(client, grantStore, configStore, server, browser) - - grant, err := svc.Login(context.Background(), domain.ProviderGoogle) - - require.NoError(t, err) - assert.NotNil(t, grant) - assert.Equal(t, "grant-123", grant.ID) - assert.Equal(t, "user@example.com", grant.Email) - - // Verify server was started and stopped - assert.True(t, server.startCalled) - assert.True(t, server.stopCalled) - - assert.Equal(t, capturedState, stateWhenBrowserOpened, - "a fast redirect must be checked against this login's state, not an unset one") - - // Verify browser and callback state/PKCE values were wired through. - assert.Equal(t, "https://mock.nylas.com/auth?state="+capturedState, browser.openedURL) - assert.Equal(t, capturedState, server.expectedState) - assert.NotEmpty(t, capturedState) - assert.NotEmpty(t, capturedChallenge) - - // Verify grant was saved - savedGrant, err := grantStore.GetGrant("grant-123") - require.NoError(t, err) - assert.Equal(t, "grant-123", savedGrant.ID) - - // Verify grant was set as default - defaultID, err := grantStore.GetDefaultGrant() - require.NoError(t, err) - assert.Equal(t, "grant-123", defaultID) - assert.Equal(t, "grant-123", configStore.config.DefaultGrant) - assert.Empty(t, configStore.config.Grants) - }) - - t.Run("default persistence failure returns error", func(t *testing.T) { - client := nylas.NewMockClient() - client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { - return &domain.Grant{ - ID: "grant-123", - Email: "user@example.com", - Provider: domain.ProviderGoogle, - }, nil - } - - grantStore := newMockGrantStore() - configStore := &failingSaveConfigStore{config: &domain.Config{Region: "us"}} - server := &mockOAuthServer{ - redirectURI: "http://localhost:8080/callback", - code: "auth-code-123", - } - browser := &mockBrowser{} - - svc := NewService(client, grantStore, configStore, server, browser) - - grant, err := svc.Login(context.Background(), domain.ProviderGoogle) - - require.Error(t, err) - assert.Nil(t, grant) - assert.ErrorIs(t, err, domain.ErrInvalidInput) - _, defaultErr := grantStore.GetDefaultGrant() - assert.ErrorIs(t, defaultErr, domain.ErrNoDefaultGrant) - }) - - t.Run("existing default is mirrored to config after login", func(t *testing.T) { - client := nylas.NewMockClient() - client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { - return &domain.Grant{ - ID: "grant-2", - Email: "second@example.com", - Provider: domain.ProviderMicrosoft, - }, nil - } - - grantStore := newMockGrantStore() - require.NoError(t, grantStore.SaveGrant(domain.GrantInfo{ID: "grant-1", Email: "first@example.com", Provider: domain.ProviderGoogle})) - require.NoError(t, grantStore.SetDefaultGrant("grant-1")) - configStore := newMockConfigStore() - configStore.config.DefaultGrant = "stale-config-default" - server := &mockOAuthServer{ - redirectURI: "http://localhost:8080/callback", - code: "auth-code-123", - } - browser := &mockBrowser{} - - svc := NewService(client, grantStore, configStore, server, browser) - - grant, err := svc.Login(context.Background(), domain.ProviderMicrosoft) - - require.NoError(t, err) - require.NotNil(t, grant) - defaultID, err := grantStore.GetDefaultGrant() - require.NoError(t, err) - assert.Equal(t, "grant-1", defaultID) - assert.Equal(t, "grant-1", configStore.config.DefaultGrant) - assert.Empty(t, configStore.config.Grants) - }) - - t.Run("server start failure returns error", func(t *testing.T) { - server := &mockOAuthServer{ - startErr: errors.New("server start failed"), - } - svc := NewService(nylas.NewMockClient(), newMockGrantStore(), newMockConfigStore(), server, &mockBrowser{}) - - _, err := svc.Login(context.Background(), domain.ProviderGoogle) - - assert.Error(t, err) - assert.Contains(t, err.Error(), "server start failed") - }) - - t.Run("browser open failure returns error", func(t *testing.T) { - browser := &mockBrowser{ - openErr: errors.New("browser open failed"), - } - svc := NewService( - nylas.NewMockClient(), - newMockGrantStore(), - newMockConfigStore(), - &mockOAuthServer{redirectURI: "http://localhost:8080/callback"}, - browser, - ) - - _, err := svc.Login(context.Background(), domain.ProviderGoogle) - - assert.Error(t, err) - assert.Contains(t, err.Error(), "browser open failed") - }) - - t.Run("callback wait failure returns error", func(t *testing.T) { - server := &mockOAuthServer{ - redirectURI: "http://localhost:8080/callback", - waitErr: errors.New("callback wait failed"), - } - svc := NewService( - nylas.NewMockClient(), - newMockGrantStore(), - newMockConfigStore(), - server, - &mockBrowser{}, - ) - - _, err := svc.Login(context.Background(), domain.ProviderGoogle) - - assert.Error(t, err) - assert.Contains(t, err.Error(), "callback wait failed") - }) - - t.Run("code exchange failure returns error", func(t *testing.T) { - client := nylas.NewMockClient() - client.ExchangeCodeFunc = func(ctx context.Context, code, redirectURI, codeVerifier string) (*domain.Grant, error) { - return nil, errors.New("code exchange failed") - } - - server := &mockOAuthServer{ - redirectURI: "http://localhost:8080/callback", - code: "auth-code", - } - svc := NewService(client, newMockGrantStore(), newMockConfigStore(), server, &mockBrowser{}) - - _, err := svc.Login(context.Background(), domain.ProviderGoogle) - - assert.Error(t, err) - assert.Contains(t, err.Error(), "code exchange failed") - }) -} - func TestService_Logout(t *testing.T) { t.Run("successful logout revokes and deletes grant", func(t *testing.T) { client := nylas.NewMockClient() @@ -757,99 +559,3 @@ func TestService_RemoveLocalGrant(t *testing.T) { assert.Equal(t, "grant-2", configStore.config.DefaultGrant) assert.Empty(t, configStore.config.Grants) } - -func TestService_LoginWithCredentials(t *testing.T) { - t.Run("successful icloud login saves grant and sets default", func(t *testing.T) { - client := nylas.NewMockClient() - client.CreateCustomGrantFunc = func(ctx context.Context, provider string, settings map[string]any) (*domain.Grant, error) { - assert.Equal(t, "icloud", provider) - assert.Equal(t, "user@icloud.com", settings["username"]) - return &domain.Grant{ - ID: "icloud-grant-001", - Email: "user@icloud.com", - Provider: domain.ProviderICloud, - GrantStatus: "valid", - }, nil - } - - grantStore := newMockGrantStore() - configStore := newMockConfigStore() - - svc := NewService(client, grantStore, configStore, &mockOAuthServer{}, &mockBrowser{}) - - grant, err := svc.LoginWithCredentials(context.Background(), "icloud", map[string]any{ - "username": "user@icloud.com", - "password": "app-specific-password", - }) - - require.NoError(t, err) - assert.Equal(t, "icloud-grant-001", grant.ID) - assert.Equal(t, "user@icloud.com", grant.Email) - - savedGrant, err := grantStore.GetGrant("icloud-grant-001") - require.NoError(t, err) - assert.Equal(t, domain.ProviderICloud, savedGrant.Provider) - - defaultID, err := grantStore.GetDefaultGrant() - require.NoError(t, err) - assert.Equal(t, "icloud-grant-001", defaultID) - }) - - t.Run("does not override existing default", func(t *testing.T) { - client := nylas.NewMockClient() - client.CreateCustomGrantFunc = func(ctx context.Context, provider string, settings map[string]any) (*domain.Grant, error) { - return &domain.Grant{ - ID: "yahoo-grant-001", - Email: "user@yahoo.com", - Provider: domain.ProviderIMAP, - }, nil - } - - grantStore := newMockGrantStore() - require.NoError(t, grantStore.SaveGrant(domain.GrantInfo{ID: "existing-grant", Email: "first@example.com"})) - require.NoError(t, grantStore.SetDefaultGrant("existing-grant")) - configStore := newMockConfigStore() - - svc := NewService(client, grantStore, configStore, &mockOAuthServer{}, &mockBrowser{}) - - grant, err := svc.LoginWithCredentials(context.Background(), "imap", map[string]any{ - "imap_username": "user@yahoo.com", - "imap_password": "secret", - "imap_host": "imap.mail.yahoo.com", - "imap_port": 993, - "type": "yahoo", - }) - - require.NoError(t, err) - assert.Equal(t, "yahoo-grant-001", grant.ID) - - defaultID, err := grantStore.GetDefaultGrant() - require.NoError(t, err) - assert.Equal(t, "existing-grant", defaultID) - }) - - t.Run("api error propagates", func(t *testing.T) { - client := nylas.NewMockClient() - client.CreateCustomGrantFunc = func(ctx context.Context, provider string, settings map[string]any) (*domain.Grant, error) { - return nil, errors.New("Invalid IMAP credentials") - } - - svc := NewService(client, newMockGrantStore(), newMockConfigStore(), &mockOAuthServer{}, &mockBrowser{}) - - _, err := svc.LoginWithCredentials(context.Background(), "imap", map[string]any{ - "imap_username": "bad@example.com", - "imap_password": "wrong", - "imap_host": "mail.example.com", - "imap_port": 993, - }) - - require.Error(t, err) - assert.Contains(t, err.Error(), "Invalid IMAP credentials") - }) -} - -func pkceChallenge(verifier string) string { - hash := sha256.Sum256([]byte(verifier)) - hexHash := fmt.Sprintf("%x", hash) - return base64.RawStdEncoding.EncodeToString([]byte(hexHash)) -} diff --git a/internal/app/dashboard/auth_service.go b/internal/app/dashboard/auth_service.go index 1482c2e..4e0b5e2 100644 --- a/internal/app/dashboard/auth_service.go +++ b/internal/app/dashboard/auth_service.go @@ -184,7 +184,8 @@ func (s *AuthService) SSOPoll(ctx context.Context, flowID, orgPublicID string) ( // IsOAuthSession reports whether the stored session was exchanged from // `nylas oauth login`. Such a session is bound to the organization chosen at // sign-in, so switching organization means signing in again. -func (s *AuthService) IsOAuthSession() bool { +// A keyring that cannot be read is reported rather than answered as false. +func (s *AuthService) IsOAuthSession() (bool, error) { return isOAuthSession(s.secrets) } @@ -362,15 +363,20 @@ func (s *AuthService) loadTokens(ctx context.Context) (userToken, orgToken strin func (s *AuthService) refreshTokens(ctx context.Context, userToken, orgToken string) (string, string, error) { // The server refuses to refresh a session exchanged from OAuth; exchange - // a fresh access token for a new one instead. - if isOAuthSession(s.secrets) { + // a fresh access token for a new one instead. A failed read of the origin + // is returned rather than taken for a refreshable session. + fromOAuth, err := isOAuthSession(s.secrets) + if err != nil { + return "", "", err + } + if fromOAuth { if s.renewer == nil { return "", "", errOAuthSessionNotRefreshable } return s.renewer.renewRejected(ctx, userToken) } - err := s.withSessionLock(ctx, func() error { + err = s.withSessionLock(ctx, func() error { // Another process may have refreshed while this one waited; its // tokens are the live ones, and refreshing the spent pair would fail. if stored, storedOrg, err := loadDashboardTokens(s.secrets, s.server); err == nil && stored != userToken { diff --git a/internal/app/dashboard/auth_service_sync_test.go b/internal/app/dashboard/auth_service_sync_test.go new file mode 100644 index 0000000..b2fe2e1 --- /dev/null +++ b/internal/app/dashboard/auth_service_sync_test.go @@ -0,0 +1,181 @@ +package dashboard + +import ( + "context" + "errors" + "testing" + + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + dashboardadapter "github.com/nylas/cli/internal/adapters/dashboard" +) + +// TestAuthService_SyncSessionOrg +// --------------------------------------------------------------------------- + +func TestAuthService_SyncSessionOrg(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + seedUser string + seedOrg string + storeFactory func() ports.SecretStore + mockFn func(ctx context.Context, userToken, orgToken string) (*domain.DashboardSessionResponse, error) + wantErr bool + wantErrIs error + wantOrgPublicID string + wantNoOrgPublicID bool + }{ + { + name: "stores CurrentOrg.PublicID on success", + seedUser: "ut-abc", + mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { + return &domain.DashboardSessionResponse{ + CurrentOrg: "org-synced", + }, nil + }, + wantOrgPublicID: "org-synced", + }, + { + name: "returns error when GetCurrentSession fails", + seedUser: "ut-abc", + mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { + return nil, errors.New("session fetch failed") + }, + wantErr: true, + wantNoOrgPublicID: true, + }, + { + name: "returns error when not logged in", + wantErr: true, + wantErrIs: domain.ErrDashboardNotLoggedIn, + wantNoOrgPublicID: true, + }, + { + name: "does not store empty CurrentOrg.PublicID", + seedUser: "ut-abc", + mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { + return &domain.DashboardSessionResponse{ + CurrentOrg: "", + }, nil + }, + wantNoOrgPublicID: true, + }, + { + name: "overwrites pre-existing org public ID with server value", + seedUser: "ut-abc", + seedOrg: "ot-xyz", + mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { + return &domain.DashboardSessionResponse{ + CurrentOrg: "org-from-server", + }, nil + }, + wantOrgPublicID: "org-from-server", + }, + { + name: "returns error when storing synced org fails", + seedUser: "ut-abc", + storeFactory: func() ports.SecretStore { + return &failingSecretStore{ + memSecretStore: newMemSecretStore(), + failSetKey: ports.KeyDashboardOrgPublicID, + } + }, + mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { + return &domain.DashboardSessionResponse{ + CurrentOrg: "org-synced", + }, nil + }, + wantErr: true, + wantNoOrgPublicID: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var store ports.SecretStore + if tt.storeFactory != nil { + store = tt.storeFactory() + } else { + store = newMemSecretStore() + } + seedTokens(store, tt.seedUser, tt.seedOrg) + + mock := &dashboardadapter.MockAccountClient{ + GetCurrentSessionFn: tt.mockFn, + } + + svc := NewAuthService(mock, store) + err := svc.SyncSessionOrg(context.Background()) + + if tt.wantErr { + require.Error(t, err) + if tt.wantErrIs != nil { + assert.ErrorIs(t, err, tt.wantErrIs) + } + } else { + require.NoError(t, err) + } + + stored, _ := store.Get(ports.KeyDashboardOrgPublicID) + if tt.wantOrgPublicID != "" { + assert.Equal(t, tt.wantOrgPublicID, stored) + } + if tt.wantNoOrgPublicID { + assert.Empty(t, stored) + } + }) + } +} + +func TestAuthServiceStoreTokens(t *testing.T) { + t.Parallel() + + t.Run("stores org when there is exactly one organization", func(t *testing.T) { + t.Parallel() + + store := newMemSecretStore() + svc := NewAuthService(&dashboardadapter.MockAccountClient{}, store) + + err := svc.storeTokens(context.Background(), &domain.DashboardAuthResponse{ + UserToken: "user-token", + User: domain.DashboardUser{PublicID: "user-1"}, + Organizations: []domain.DashboardOrganization{ + {PublicID: "org-only"}, + }, + }) + + require.NoError(t, err) + + storedOrgID, _ := store.Get(ports.KeyDashboardOrgPublicID) + assert.Equal(t, "org-only", storedOrgID) + }) + + t.Run("does not guess active org when multiple organizations exist", func(t *testing.T) { + t.Parallel() + + store := newMemSecretStore() + svc := NewAuthService(&dashboardadapter.MockAccountClient{}, store) + + err := svc.storeTokens(context.Background(), &domain.DashboardAuthResponse{ + UserToken: "user-token", + User: domain.DashboardUser{PublicID: "user-1"}, + Organizations: []domain.DashboardOrganization{ + {PublicID: "org-1"}, + {PublicID: "org-2"}, + }, + }) + + require.NoError(t, err) + + storedOrgID, _ := store.Get(ports.KeyDashboardOrgPublicID) + assert.Empty(t, storedOrgID) + }) +} diff --git a/internal/app/dashboard/auth_service_test.go b/internal/app/dashboard/auth_service_test.go index 913d1ff..48b0d92 100644 --- a/internal/app/dashboard/auth_service_test.go +++ b/internal/app/dashboard/auth_service_test.go @@ -525,169 +525,3 @@ func TestAuthService_RefreshRollsBackOnStoreFailure(t *testing.T) { } // --------------------------------------------------------------------------- -// TestAuthService_SyncSessionOrg -// --------------------------------------------------------------------------- - -func TestAuthService_SyncSessionOrg(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - seedUser string - seedOrg string - storeFactory func() ports.SecretStore - mockFn func(ctx context.Context, userToken, orgToken string) (*domain.DashboardSessionResponse, error) - wantErr bool - wantErrIs error - wantOrgPublicID string - wantNoOrgPublicID bool - }{ - { - name: "stores CurrentOrg.PublicID on success", - seedUser: "ut-abc", - mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { - return &domain.DashboardSessionResponse{ - CurrentOrg: "org-synced", - }, nil - }, - wantOrgPublicID: "org-synced", - }, - { - name: "returns error when GetCurrentSession fails", - seedUser: "ut-abc", - mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { - return nil, errors.New("session fetch failed") - }, - wantErr: true, - wantNoOrgPublicID: true, - }, - { - name: "returns error when not logged in", - wantErr: true, - wantErrIs: domain.ErrDashboardNotLoggedIn, - wantNoOrgPublicID: true, - }, - { - name: "does not store empty CurrentOrg.PublicID", - seedUser: "ut-abc", - mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { - return &domain.DashboardSessionResponse{ - CurrentOrg: "", - }, nil - }, - wantNoOrgPublicID: true, - }, - { - name: "overwrites pre-existing org public ID with server value", - seedUser: "ut-abc", - seedOrg: "ot-xyz", - mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { - return &domain.DashboardSessionResponse{ - CurrentOrg: "org-from-server", - }, nil - }, - wantOrgPublicID: "org-from-server", - }, - { - name: "returns error when storing synced org fails", - seedUser: "ut-abc", - storeFactory: func() ports.SecretStore { - return &failingSecretStore{ - memSecretStore: newMemSecretStore(), - failSetKey: ports.KeyDashboardOrgPublicID, - } - }, - mockFn: func(_ context.Context, _, _ string) (*domain.DashboardSessionResponse, error) { - return &domain.DashboardSessionResponse{ - CurrentOrg: "org-synced", - }, nil - }, - wantErr: true, - wantNoOrgPublicID: true, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - var store ports.SecretStore - if tt.storeFactory != nil { - store = tt.storeFactory() - } else { - store = newMemSecretStore() - } - seedTokens(store, tt.seedUser, tt.seedOrg) - - mock := &dashboardadapter.MockAccountClient{ - GetCurrentSessionFn: tt.mockFn, - } - - svc := NewAuthService(mock, store) - err := svc.SyncSessionOrg(context.Background()) - - if tt.wantErr { - require.Error(t, err) - if tt.wantErrIs != nil { - assert.ErrorIs(t, err, tt.wantErrIs) - } - } else { - require.NoError(t, err) - } - - stored, _ := store.Get(ports.KeyDashboardOrgPublicID) - if tt.wantOrgPublicID != "" { - assert.Equal(t, tt.wantOrgPublicID, stored) - } - if tt.wantNoOrgPublicID { - assert.Empty(t, stored) - } - }) - } -} - -func TestAuthServiceStoreTokens(t *testing.T) { - t.Parallel() - - t.Run("stores org when there is exactly one organization", func(t *testing.T) { - t.Parallel() - - store := newMemSecretStore() - svc := NewAuthService(&dashboardadapter.MockAccountClient{}, store) - - err := svc.storeTokens(context.Background(), &domain.DashboardAuthResponse{ - UserToken: "user-token", - User: domain.DashboardUser{PublicID: "user-1"}, - Organizations: []domain.DashboardOrganization{ - {PublicID: "org-only"}, - }, - }) - - require.NoError(t, err) - - storedOrgID, _ := store.Get(ports.KeyDashboardOrgPublicID) - assert.Equal(t, "org-only", storedOrgID) - }) - - t.Run("does not guess active org when multiple organizations exist", func(t *testing.T) { - t.Parallel() - - store := newMemSecretStore() - svc := NewAuthService(&dashboardadapter.MockAccountClient{}, store) - - err := svc.storeTokens(context.Background(), &domain.DashboardAuthResponse{ - UserToken: "user-token", - User: domain.DashboardUser{PublicID: "user-1"}, - Organizations: []domain.DashboardOrganization{ - {PublicID: "org-1"}, - {PublicID: "org-2"}, - }, - }) - - require.NoError(t, err) - - storedOrgID, _ := store.Get(ports.KeyDashboardOrgPublicID) - assert.Empty(t, storedOrgID) - }) -} diff --git a/internal/app/dashboard/oauth_session.go b/internal/app/dashboard/oauth_session.go index a788215..44f73f2 100644 --- a/internal/app/dashboard/oauth_session.go +++ b/internal/app/dashboard/oauth_session.go @@ -29,7 +29,7 @@ const oauthSessionRenewBefore = time.Minute // session from `nylas dashboard login` is stored. It is left in place: its // app selection and organization are the user's, and replacing it would leave // it live on the server. -var ErrDashboardLoginSessionKept = errors.New("the dashboard commands stay signed in with `nylas dashboard login`") +var ErrDashboardLoginSessionKept = errors.New("the dashboard commands stay signed in with `nylas dashboard login`; run `nylas dashboard logout` first to use this login instead, or if that session has expired") var oauthSessionStateKeys = append( append([]string{}, dashboardSessionStateKeys...), @@ -98,7 +98,14 @@ func (r *SessionRenewer) Login(ctx context.Context) (*domain.DashboardOAuthExcha return ErrDashboardLoginSessionKept } resp, err = r.exchange(ctx, true) - if err != nil && isOAuthSession(r.secrets) { + if err == nil { + return nil + } + fromOAuth, originErr := isOAuthSession(r.secrets) + if originErr != nil { + return errors.Join(err, originErr) + } + if fromOAuth { // The OAuth session this one was exchanged from has just been // replaced. Keeping it would leave every dashboard command acting // as the previous user or organization until it expires. @@ -113,15 +120,23 @@ func (r *SessionRenewer) Login(ctx context.Context) (*domain.DashboardOAuthExcha // EnsureFresh re-exchanges the stored session when it came from OAuth and is // about to expire. Sessions from `nylas dashboard login` are left alone. func (r *SessionRenewer) EnsureFresh(ctx context.Context) error { - if r == nil || !isOAuthSession(r.secrets) || !r.needsRenewal() { + if r == nil { return nil } + // A keyring that cannot say where the session came from is reported: read + // as "not from OAuth", the session would go to a refresh the server + // refuses, or be sent past its expiry. + fromOAuth, err := isOAuthSession(r.secrets) + if err != nil || !fromOAuth || !r.needsRenewal() { + return err + } return r.withLock(ctx, func() error { // Another process may have renewed while this one waited. - if !isOAuthSession(r.secrets) || !r.needsRenewal() { - return nil + fromOAuth, err := isOAuthSession(r.secrets) + if err != nil || !fromOAuth || !r.needsRenewal() { + return err } - _, err := r.exchange(ctx, false) + _, err = r.exchange(ctx, false) return err }) } @@ -133,7 +148,11 @@ func (r *SessionRenewer) EnsureFresh(ctx context.Context) error { // refused like on every other load path rather than sent here. func (r *SessionRenewer) renewRejected(ctx context.Context, rejected string) (userToken, orgToken string, err error) { err = r.withLock(ctx, func() error { - if isOAuthSession(r.secrets) && !r.needsRenewal() { + fromOAuth, err := isOAuthSession(r.secrets) + if err != nil { + return err + } + if fromOAuth && !r.needsRenewal() { u, o, loadErr := loadDashboardTokens(r.secrets, r.server) if loadErr == nil && u != rejected { userToken, orgToken = u, o @@ -225,7 +244,7 @@ func (r *SessionRenewer) ClearIfOAuthThen(ctx context.Context, then func() error // A keyring that cannot say where the session came from is reported, // not read as "not from OAuth": that would leave the session behind // while the logout reports success. - switch fromOAuth, err := oauthSessionOrigin(r.secrets); { + switch fromOAuth, err := isOAuthSession(r.secrets); { case err != nil: clearErr = err case fromOAuth: @@ -242,14 +261,16 @@ func (r *SessionRenewer) ClearIfOAuthThen(ctx context.Context, then func() error return errors.Join(clearErr, lockErr), thenErr } -func isOAuthSession(secrets ports.SecretStore) bool { - fromOAuth, err := oauthSessionOrigin(secrets) - return err == nil && fromOAuth +// IsOAuthSession reports whether the session in secrets was exchanged from +// `nylas oauth login`. A keyring that cannot be read is an error, not "no": +// callers decide from the answer whether to revoke, refresh or keep a session. +func IsOAuthSession(secrets ports.SecretStore) (bool, error) { + return isOAuthSession(secrets) } -// oauthSessionOrigin reports whether the stored session was exchanged from -// `nylas oauth login`, telling a failed read apart from no marker. -func oauthSessionOrigin(secrets ports.SecretStore) (bool, error) { +// isOAuthSession tells a failed read apart from no marker; only a marker that +// is genuinely absent means the session did not come from OAuth. +func isOAuthSession(secrets ports.SecretStore) (bool, error) { origin, err := secrets.Get(ports.KeyDashboardSessionOrigin) if errors.Is(err, domain.ErrSecretNotFound) { return false, nil @@ -266,8 +287,8 @@ func oauthSessionOrigin(secrets ports.SecretStore) (bool, error) { // keyring that cannot be read is reported rather than taken for "no session", // which would let the exchange overwrite one. func (r *SessionRenewer) hasDashboardLoginSession() (bool, error) { - if isOAuthSession(r.secrets) { - return false, nil + if fromOAuth, err := isOAuthSession(r.secrets); err != nil || fromOAuth { + return false, err } _, _, err := loadDashboardTokens(r.secrets, r.server) switch { diff --git a/internal/app/dashboard/oauth_session_origin_test.go b/internal/app/dashboard/oauth_session_origin_test.go new file mode 100644 index 0000000..7d834bd --- /dev/null +++ b/internal/app/dashboard/oauth_session_origin_test.go @@ -0,0 +1,78 @@ +package dashboard + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + dashboardadapter "github.com/nylas/cli/internal/adapters/dashboard" + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" +) + +// originUnreadable is an OAuth session whose origin marker cannot be read, +// like a keychain that locks between two reads. +func originUnreadable() *failingSecretStore { + return &failingSecretStore{ + memSecretStore: oauthSessionSecrets(time.Now().Add(-time.Minute)), + failGetKey: ports.KeyDashboardSessionOrigin, + } +} + +func TestIsOAuthSession_ReportsAFailedReadInsteadOfNo(t *testing.T) { + fromOAuth, err := IsOAuthSession(originUnreadable()) + + require.ErrorIs(t, err, domain.ErrSecretStoreFailed) + assert.False(t, fromOAuth) +} + +func TestAuthService_RefreshDoesNotRefreshWhenTheOriginCannotBeRead(t *testing.T) { + // Read as "not from OAuth", the session would go to a refresh the server + // refuses for OAuth sessions, hiding the keyring failure behind it. + refreshed := false + account := &dashboardadapter.MockAccountClient{ + RefreshFn: func(context.Context, string, string) (*domain.DashboardRefreshResponse, error) { + refreshed = true + return &domain.DashboardRefreshResponse{UserToken: "refreshed"}, nil + }, + } + + _, _, err := NewAuthService(account, originUnreadable()).refreshTokens(context.Background(), "old-user", "old-org") + + require.ErrorIs(t, err, domain.ErrSecretStoreFailed) + assert.False(t, refreshed) +} + +func TestSessionRenewer_EnsureFreshReportsAnUnreadableOrigin(t *testing.T) { + // An expired OAuth session must not be sent as-is because its origin + // could not be read. + err := NewSessionRenewer(&dashboardadapter.MockAccountClient{}, originUnreadable(), &fakeOAuthTokens{token: "at"}, newTestLock(t)). + EnsureFresh(context.Background()) + + require.ErrorIs(t, err, domain.ErrSecretStoreFailed) +} + +func TestSessionRenewer_LoginDoesNotExchangeWhenTheOriginCannotBeRead(t *testing.T) { + // Without the origin it is unknown whether a stored session is the + // user's own dashboard login, so it must not be overwritten. + secrets := &failingSecretStore{memSecretStore: newMemSecretStore(), failGetKey: ports.KeyDashboardSessionOrigin} + exchanged := false + account := &dashboardadapter.MockAccountClient{ + ExchangeOAuthTokenFn: func(context.Context, string) (*domain.DashboardOAuthExchangeResponse, error) { + exchanged = true + return &domain.DashboardOAuthExchangeResponse{ + DashboardAuthResponse: domain.DashboardAuthResponse{UserToken: "new-user"}, + ExpiresAt: time.Now().Add(time.Hour), + }, nil + }, + } + + _, err := NewSessionRenewer(account, secrets, &fakeOAuthTokens{token: "at"}, newTestLock(t)).Login(context.Background()) + + require.ErrorIs(t, err, domain.ErrSecretStoreFailed) + assert.NotErrorIs(t, err, ErrDashboardLoginSessionKept) + assert.False(t, exchanged) +} diff --git a/internal/app/dashboard/session_store.go b/internal/app/dashboard/session_store.go index 2ea1ed0..efbae51 100644 --- a/internal/app/dashboard/session_store.go +++ b/internal/app/dashboard/session_store.go @@ -95,8 +95,8 @@ func checkSessionServer(secrets ports.SecretStore, server string) error { // OAuth when the OAuth session it came from belongs to another account server. // That is the one case where the issuer of an old session is known. func checkLegacyOAuthSessionServer(secrets ports.SecretStore, server string) error { - if !isOAuthSession(secrets) { - return nil + if fromOAuth, err := isOAuthSession(secrets); err != nil || !fromOAuth { + return err } issuedBy, err := secrets.Get(ports.KeyOAuthServerURL) if err != nil || issuedBy == "" { diff --git a/internal/cli/dashboard/helpers.go b/internal/cli/dashboard/helpers.go index aa7231d..b59ead2 100644 --- a/internal/cli/dashboard/helpers.go +++ b/internal/cli/dashboard/helpers.go @@ -95,13 +95,13 @@ func createAppService() (*dashboardapp.AppService, error) { } // OAuthTokenSource builds the OAuth session a dashboard session is exchanged -// from. The oauth command package registers it: it owns that wiring and -// already imports this package, so this package cannot import it back. +// from. main wires it through oauth.WireDashboard: the oauth package owns +// that wiring and already imports this package, so this one cannot import it. var OAuthTokenSource func() (dashboardapp.OAuthAccessTokens, error) // OAuthRelogin runs `nylas oauth login` again and exchanges the new session, // which is how `orgs switch` changes the organization of an OAuth session. -// The oauth command package registers it, like OAuthTokenSource. +// Wired by oauth.WireDashboard, like OAuthTokenSource. var OAuthRelogin func(ctx context.Context) (*domain.DashboardOAuthExchangeResponse, error) // lazyOAuthTokens defers building the OAuth session until a renewal needs a @@ -141,12 +141,58 @@ func ExchangeOAuthSession(ctx context.Context, tokens dashboardapp.OAuthAccessTo // then runs then, holding the dashboard session lock across both; see // SessionRenewer.ClearIfOAuthThen. func ClearOAuthSessionThen(ctx context.Context, then func() error) (clearErr, thenErr error) { - dpopSvc, secretStore, err := createDPoPService() + secretStore, err := keyring.NewSecretStore(config.DefaultConfigDir()) + if err != nil { + return err, then() + } + return clearOAuthSessionThen(ctx, secretStore, common.DashboardSessionLock(secretStore), then) +} + +// clearOAuthSessionThen checks where the dashboard session came from before +// building the DPoP service, because building it creates a key when there is +// none: `nylas oauth logout` without a dashboard session from OAuth has +// nothing to revoke and must not leave a key behind. +func clearOAuthSessionThen(ctx context.Context, secretStore ports.SecretStore, lock ports.CrossProcessLock, then func() error) (clearErr, thenErr error) { + fromOAuth, err := dashboardapp.IsOAuthSession(secretStore) + if err != nil { + return err, then() + } + if !fromOAuth { + ran, clearErr, thenErr := thenUnlessOAuthSession(ctx, secretStore, lock, then) + if ran { + return clearErr, thenErr + } + // A concurrent `nylas oauth login` stored one in the meantime. + } + dpopSvc, err := loadOrCreateDPoPKey(secretStore) if err != nil { return err, then() } accountClient := dashboard.NewAccountClient(AccountBaseURL(), dpopSvc) - return newSessionRenewer(accountClient, secretStore).ClearIfOAuthThen(ctx, then) + return dashboardapp.NewSessionRenewer(accountClient, secretStore, lazyOAuthTokens{}, lock). + WithServer(SessionServer()). + ClearIfOAuthThen(ctx, then) +} + +// thenUnlessOAuthSession runs then under the dashboard session lock when no +// dashboard session from OAuth is stored, so a concurrent login cannot store +// one between the check and then. It reports ran=false, without running then, +// when one has appeared by the time the lock is held. +func thenUnlessOAuthSession(ctx context.Context, secretStore ports.SecretStore, lock ports.CrossProcessLock, then func() error) (ran bool, clearErr, thenErr error) { + unlock, err := lock.Lock(ctx) + if err != nil { + return true, fmt.Errorf("failed to acquire the dashboard session lock: %w", err), then() + } + fromOAuth, err := dashboardapp.IsOAuthSession(secretStore) + if err == nil && fromOAuth { + _ = unlock() + return false, nil, nil + } + thenErr = then() + if unlockErr := unlock(); unlockErr != nil { + err = errors.Join(err, fmt.Errorf("failed to release the dashboard session lock: %w", unlockErr)) + } + return true, err, thenErr } // createDomainService creates the dashboard domain management service. diff --git a/internal/cli/dashboard/oauth_logout_dpop_test.go b/internal/cli/dashboard/oauth_logout_dpop_test.go new file mode 100644 index 0000000..b432f5b --- /dev/null +++ b/internal/cli/dashboard/oauth_logout_dpop_test.go @@ -0,0 +1,69 @@ +package dashboard + +import ( + "context" + "errors" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/nylas/cli/internal/adapters/filelock" + "github.com/nylas/cli/internal/ports" +) + +// originFailingStore fails reads of the session origin, like a keychain that +// locks between two reads. +type originFailingStore struct{ *memSecretStore } + +func (s originFailingStore) Get(key string) (string, error) { + if key == ports.KeyDashboardSessionOrigin { + return "", errors.New("keychain locked") + } + return s.memSecretStore.Get(key) +} + +func TestClearOAuthSessionThen_WithoutAnOAuthSessionCreatesNoDPoPKey(t *testing.T) { + // `nylas oauth logout` has nothing to revoke here. Building the DPoP + // service would create and store a key as a side effect of logging out. + secrets := newMemSecretStore() + secrets.data[ports.KeyDashboardUserToken] = "dashboard-login-token" + lock := filelock.New(filepath.Join(t.TempDir(), "dashboard-session.lock")) + thenRan := false + + clearErr, thenErr := clearOAuthSessionThen(context.Background(), secrets, lock, func() error { + thenRan = true + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if unlock, err := lock.Lock(ctx); err == nil { + _ = unlock() + return errors.New("the dashboard lock was not held") + } + return nil + }) + + require.NoError(t, clearErr) + require.NoError(t, thenErr) + assert.True(t, thenRan) + assert.NotContains(t, secrets.data, ports.KeyDashboardDPoPKey) + assert.Equal(t, "dashboard-login-token", secrets.data[ports.KeyDashboardUserToken], + "a session from `nylas dashboard login` is not the OAuth logout's to end") +} + +func TestClearOAuthSessionThen_ReportsAnUnreadableOriginAndStillLogsOut(t *testing.T) { + secrets := originFailingStore{newMemSecretStore()} + lock := filelock.New(filepath.Join(t.TempDir(), "dashboard-session.lock")) + thenRan := false + + clearErr, thenErr := clearOAuthSessionThen(context.Background(), secrets, lock, func() error { + thenRan = true + return nil + }) + + require.Error(t, clearErr, "logout must not report success while it cannot tell whether a session remains") + require.NoError(t, thenErr) + assert.True(t, thenRan) + assert.NotContains(t, secrets.data, ports.KeyDashboardDPoPKey) +} diff --git a/internal/cli/dashboard/switch_org.go b/internal/cli/dashboard/switch_org.go index 0e555b0..bffd17d 100644 --- a/internal/cli/dashboard/switch_org.go +++ b/internal/cli/dashboard/switch_org.go @@ -36,7 +36,11 @@ organization there. --org then checks that the one chosen is the one named.`, if err != nil { return wrapDashboardError(err) } - if authSvc.IsOAuthSession() { + fromOAuth, err := authSvc.IsOAuthSession() + if err != nil { + return wrapDashboardError(err) + } + if fromOAuth { ctx, cancel := common.CreateLongContext() defer cancel() return switchOAuthSessionOrg(ctx, cmd.OutOrStdout(), orgFlag, OAuthRelogin) diff --git a/internal/cli/oauth/helpers.go b/internal/cli/oauth/helpers.go index eca9dc7..952f80c 100644 --- a/internal/cli/oauth/helpers.go +++ b/internal/cli/oauth/helpers.go @@ -39,15 +39,19 @@ var ( clearDashboardSessionFn = dashboard.ClearOAuthSessionThen ) -// The dashboard commands renew a session from `nylas oauth login` through the -// same login service, so refreshes share this machine's session lock. -func init() { - dashboard.OAuthTokenSource = func() (dashboardapp.OAuthAccessTokens, error) { - return createLoginService() - } +// WireDashboard connects the dashboard commands to `nylas oauth login`: they +// renew a session from it through the same login service, so refreshes share +// this machine's session lock. main calls it when registering commands; until +// then the dashboard commands treat an OAuth-backed session as unavailable. +func WireDashboard() { + dashboard.OAuthTokenSource = dashboardTokenSource dashboard.OAuthRelogin = relogin } +func dashboardTokenSource() (dashboardapp.OAuthAccessTokens, error) { + return createLoginService() +} + // relogin signs in again with the stored session's scopes and resource and // exchanges the result for a dashboard session. func relogin(ctx context.Context) (*domain.DashboardOAuthExchangeResponse, error) { diff --git a/internal/cli/oauth/login.go b/internal/cli/oauth/login.go index cc80544..1430151 100644 --- a/internal/cli/oauth/login.go +++ b/internal/cli/oauth/login.go @@ -1,6 +1,7 @@ package oauth import ( + "context" "errors" "fmt" "strings" @@ -89,13 +90,25 @@ dev authorization server.`, ctx, cancel := common.CreateLongContext() defer cancel() - _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Opening your browser to complete sign-in...") + // Structured output keeps stdout for the report, so the prompt + // goes to stderr there; --quiet drops it. + if !quietOutput(cmd) { + prompt := cmd.OutOrStdout() + if structuredOutput(cmd) { + prompt = cmd.ErrOrStderr() + } + _, _ = fmt.Fprintln(prompt, "Opening your browser to complete sign-in...") + } result, err := svc.Login(ctx, opts) if err != nil { return wrapOAuthError(err) } + if structuredOutput(cmd) { + return writeLoginReport(ctx, cmd, svc, result) + } + out := cmd.OutOrStdout() _, _ = common.Green.Fprintln(out, "✓ Logged in") _, _ = fmt.Fprintf(out, " Issuer: %s\n", result.Issuer) @@ -124,7 +137,7 @@ dev authorization server.`, defer cancelExchange() dashboardSession, err := exchangeDashboardSessionFn(exchangeCtx, svc) if errors.Is(err, dashboardapp.ErrDashboardLoginSessionKept) { - _, _ = fmt.Fprintln(out, " Dashboard: still signed in with `nylas dashboard login` (run `nylas dashboard logout` first to use this login instead)") + _, _ = fmt.Fprintln(out, " Dashboard: still signed in with `nylas dashboard login`. To use this login instead, or if that session has expired, run `nylas dashboard logout` and then this command again") return nil } if err != nil { @@ -148,6 +161,28 @@ dev authorization server.`, return cmd } +// writeLoginReport is the structured form of the login output. It runs the +// same dashboard exchange as the text form, and like it reports a failed +// exchange instead of returning it. +func writeLoginReport(ctx context.Context, cmd *cobra.Command, svc loginService, result *oauthlogin.LoginResult) error { + report := newLoginReport(result) + + exchangeCtx, cancelExchange := afterConsentContext(ctx) + defer cancelExchange() + dashboardSession, err := exchangeDashboardSessionFn(exchangeCtx, svc) + switch { + case errors.Is(err, dashboardapp.ErrDashboardLoginSessionKept): + report.Dashboard.KeptDashboardLogin = true + case err != nil: + report.Dashboard.Error = err.Error() + default: + report.Dashboard.SignedIn = true + report.Dashboard.OrgPublicID = dashboardSession.OrgPublicID + } + + return common.GetOutputWriter(cmd).Write(report) +} + // loginOptions turns the flags into a login request. A preset decides both // the scopes and the resource, so it cannot be combined with --scope: a // hand-picked scope list under the MCP resource is a token the MCP tools diff --git a/internal/cli/oauth/login_kept_session_test.go b/internal/cli/oauth/login_kept_session_test.go new file mode 100644 index 0000000..261f95c --- /dev/null +++ b/internal/cli/oauth/login_kept_session_test.go @@ -0,0 +1,27 @@ +package oauth + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + dashboardapp "github.com/nylas/cli/internal/app/dashboard" + "github.com/nylas/cli/internal/app/oauthlogin" + "github.com/nylas/cli/internal/cli/testutil" +) + +func TestLoginCmd_KeptDashboardLoginSessionNamesTheWayOutOfAnExpiredOne(t *testing.T) { + // A kept `nylas dashboard login` session is not checked for expiry, so an + // expired one blocks the exchange on every login until it is logged out. + // The message is the only place the user learns how to get past it. + svc := &fakeService{loginResult: &oauthlogin.LoginResult{Issuer: "i", ClientID: "c", HasRefresh: true}} + dash := withService(t, svc) + dash.exchangeErr = dashboardapp.ErrDashboardLoginSessionKept + + stdout, _, err := testutil.ExecuteSubCommand(newLoginCmd()) + require.NoError(t, err) + + assert.Contains(t, stdout, "if that session has expired") + assert.Contains(t, stdout, "nylas dashboard logout") +} diff --git a/internal/cli/oauth/logout.go b/internal/cli/oauth/logout.go index 842c58b..4c40de7 100644 --- a/internal/cli/oauth/logout.go +++ b/internal/cli/oauth/logout.go @@ -31,6 +31,13 @@ session that came from 'nylas oauth login' is ended too; one from if logoutErr != nil { return wrapOAuthError(logoutErr) } + if structuredOutput(cmd) { + report := logoutReport{LoggedOut: true} + if dashboardErr != nil { + report.DashboardError = dashboardErr.Error() + } + return common.GetOutputWriter(cmd).Write(report) + } if dashboardErr != nil { _, _ = common.Yellow.Fprintf(cmd.OutOrStdout(), " Could not end the dashboard session: %v\n", dashboardErr) diff --git a/internal/cli/oauth/output.go b/internal/cli/oauth/output.go new file mode 100644 index 0000000..d89f613 --- /dev/null +++ b/internal/cli/oauth/output.go @@ -0,0 +1,150 @@ +package oauth + +import ( + "time" + + "github.com/spf13/cobra" + + "github.com/nylas/cli/internal/app/oauthlogin" + "github.com/nylas/cli/internal/cli/common" + "github.com/nylas/cli/internal/domain" +) + +// The reports below are what --json and --format yaml print. They never +// carry a token: only facts about the session. Each returns an empty +// QuietField so --quiet prints nothing and the exit code is the answer. + +// statusReport is `nylas oauth status` as structured output. +type statusReport struct { + LoggedIn bool `json:"logged_in" yaml:"logged_in"` + Expired bool `json:"expired" yaml:"expired"` + Server string `json:"server,omitempty" yaml:"server,omitempty"` + Issuer string `json:"issuer" yaml:"issuer"` + ClientID string `json:"client_id" yaml:"client_id"` + Resource string `json:"resource,omitempty" yaml:"resource,omitempty"` + Scopes string `json:"scopes,omitempty" yaml:"scopes,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty" yaml:"expires_at,omitempty"` + HasRefresh bool `json:"has_refresh_token" yaml:"has_refresh_token"` + Claims *claimsReport `json:"token_claims,omitempty" yaml:"token_claims,omitempty"` + ClaimsErr string `json:"token_claims_error,omitempty" yaml:"token_claims_error,omitempty"` + UserInfo *userInfoReport `json:"userinfo,omitempty" yaml:"userinfo,omitempty"` +} + +func (statusReport) QuietField() string { return "" } + +// claimsReport is what the access token says about itself, decoded locally +// and NOT verified. +type claimsReport struct { + Verified bool `json:"verified" yaml:"verified"` + Audience []string `json:"audience,omitempty" yaml:"audience,omitempty"` + Scopes string `json:"scopes,omitempty" yaml:"scopes,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty" yaml:"expires_at,omitempty"` + Grants []claimGrantEntry `json:"grants,omitempty" yaml:"grants,omitempty"` +} + +type claimGrantEntry struct { + ID string `json:"id" yaml:"id"` + ApplicationID string `json:"application_id" yaml:"application_id"` +} + +type userInfoReport struct { + Subject string `json:"subject" yaml:"subject"` + Email string `json:"email,omitempty" yaml:"email,omitempty"` + EmailVerified bool `json:"email_verified" yaml:"email_verified"` + Org string `json:"org,omitempty" yaml:"org,omitempty"` +} + +// loginReport is `nylas oauth login` as structured output. +type loginReport struct { + LoggedIn bool `json:"logged_in" yaml:"logged_in"` + Issuer string `json:"issuer" yaml:"issuer"` + ClientID string `json:"client_id" yaml:"client_id"` + Resource string `json:"resource,omitempty" yaml:"resource,omitempty"` + Scopes string `json:"scopes,omitempty" yaml:"scopes,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty" yaml:"expires_at,omitempty"` + HasRefresh bool `json:"has_refresh_token" yaml:"has_refresh_token"` + DroppedScopes []string `json:"dropped_scopes,omitempty" yaml:"dropped_scopes,omitempty"` + Dashboard dashboardOutcome `json:"dashboard" yaml:"dashboard"` +} + +func (loginReport) QuietField() string { return "" } + +// dashboardOutcome is what the login did to the dashboard commands. +type dashboardOutcome struct { + SignedIn bool `json:"signed_in" yaml:"signed_in"` + // KeptDashboardLogin: a `nylas dashboard login` session was left in place. + KeptDashboardLogin bool `json:"kept_dashboard_login,omitempty" yaml:"kept_dashboard_login,omitempty"` + OrgPublicID string `json:"org_public_id,omitempty" yaml:"org_public_id,omitempty"` + Error string `json:"error,omitempty" yaml:"error,omitempty"` +} + +// logoutReport is `nylas oauth logout` as structured output. +type logoutReport struct { + LoggedOut bool `json:"logged_out" yaml:"logged_out"` + DashboardError string `json:"dashboard_error,omitempty" yaml:"dashboard_error,omitempty"` +} + +func (logoutReport) QuietField() string { return "" } + +func newStatusReport(session *oauthlogin.Session, now time.Time) statusReport { + report := statusReport{ + LoggedIn: true, + Expired: session.Tokens.IsExpired(now), + Server: session.ServerURL, + Issuer: session.Issuer, + ClientID: session.ClientID, + Resource: session.Resource, + Scopes: session.Tokens.Scope, + ExpiresAt: timePtr(session.Tokens.ExpiresAt), + HasRefresh: session.Tokens.RefreshToken != "", + } + claims, err := domain.DecodeOAuthAccessToken(session.Tokens.AccessToken) + if err != nil { + report.ClaimsErr = err.Error() + return report + } + report.Claims = &claimsReport{ + Audience: claims.Audience, + Scopes: claims.Scope, + ExpiresAt: timePtr(claims.ExpiresAt), + } + for _, grant := range claims.Grants { + report.Claims.Grants = append(report.Claims.Grants, + claimGrantEntry{ID: grant.ID, ApplicationID: grant.ApplicationID}) + } + return report +} + +func newLoginReport(result *oauthlogin.LoginResult) loginReport { + return loginReport{ + LoggedIn: true, + Issuer: result.Issuer, + ClientID: result.ClientID, + Resource: result.Resource, + Scopes: result.Scope, + ExpiresAt: timePtr(result.ExpiresAt), + HasRefresh: result.HasRefresh, + DroppedScopes: result.DroppedScopes, + } +} + +func timePtr(t time.Time) *time.Time { + if t.IsZero() { + return nil + } + return &t +} + +// structuredOutput reports whether --json, --format json|yaml|quiet or +// --quiet was given, in which case the commands write a report instead of +// the human-readable text. +func structuredOutput(cmd *cobra.Command) bool { + return common.IsStructuredOutput(cmd) +} + +// quietOutput reports whether informational text must be suppressed. +func quietOutput(cmd *cobra.Command) bool { + quiet, _ := cmd.Flags().GetBool("quiet") + format, _ := cmd.Flags().GetString("format") + return quiet || format == "quiet" || common.IsQuiet() +} diff --git a/internal/cli/oauth/output_test.go b/internal/cli/oauth/output_test.go new file mode 100644 index 0000000..b31c8e9 --- /dev/null +++ b/internal/cli/oauth/output_test.go @@ -0,0 +1,209 @@ +//go:build !integration + +package oauth + +import ( + "encoding/json" + "errors" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + dashboardapp "github.com/nylas/cli/internal/app/dashboard" + "github.com/nylas/cli/internal/app/oauthlogin" + "github.com/nylas/cli/internal/cli/dashboard" + "github.com/nylas/cli/internal/cli/testutil" + "github.com/nylas/cli/internal/domain" +) + +// secrets are the token values in loggedInSession; structured output must +// never contain any of them. +var sessionSecrets = []string{"at-1", "rt-1", "idt-1"} + +func assertNoTokens(t *testing.T, out string) { + t.Helper() + for _, secret := range sessionSecrets { + assert.NotContains(t, out, secret, "structured output must not leak a token") + } +} + +func TestStatusCmd_JSONIsAStructuredReportWithoutTokens(t *testing.T) { + session := loggedInSession() + session.ServerURL = "https://dashboard.example.test" + session.Resource = "https://mcp.us.nylas.com" + withService(t, &fakeService{session: session}) + + stdout, _, err := testutil.ExecuteSubCommand(newStatusCmd(), "--json") + require.NoError(t, err) + + var report map[string]any + require.NoError(t, json.Unmarshal([]byte(stdout), &report), "stdout must be pure JSON: %q", stdout) + assert.Equal(t, true, report["logged_in"]) + assert.Equal(t, false, report["expired"]) + assert.Equal(t, "https://dashboard.example.test", report["server"]) + assert.Equal(t, "https://auth.example.test", report["issuer"]) + assert.Equal(t, "client-1", report["client_id"]) + assert.Equal(t, "https://mcp.us.nylas.com", report["resource"]) + assert.Equal(t, "openid email offline_access", report["scopes"]) + assert.Equal(t, true, report["has_refresh_token"]) + require.Contains(t, report, "expires_at") + _, err = time.Parse(time.RFC3339, report["expires_at"].(string)) + assert.NoError(t, err, "expires_at is a timestamp") + assertNoTokens(t, stdout) +} + +func TestStatusCmd_JSONCarriesDecodedClaimsAsUnverified(t *testing.T) { + session := loggedInSession() + session.Tokens.AccessToken = jwtWithClaims(t, map[string]any{ + "aud": "https://mcp.us.nylas.com", + "scope": "email.read", + "grants": []map[string]string{{"id": "grant-1", "application_id": "app-1"}}, + }) + withService(t, &fakeService{session: session}) + + stdout, _, err := testutil.ExecuteSubCommand(newStatusCmd(), "--json") + require.NoError(t, err) + + var report statusReport + require.NoError(t, json.Unmarshal([]byte(stdout), &report)) + require.NotNil(t, report.Claims) + assert.False(t, report.Claims.Verified, "claims are decoded locally, never verified") + assert.Equal(t, []string{"https://mcp.us.nylas.com"}, report.Claims.Audience) + assert.Equal(t, []claimGrantEntry{{ID: "grant-1", ApplicationID: "app-1"}}, report.Claims.Grants) + assert.NotContains(t, stdout, session.Tokens.AccessToken) +} + +func TestStatusCmd_JSONVerifyIncludesUserInfo(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(), "--json", "--verify") + require.NoError(t, err) + + var report statusReport + require.NoError(t, json.Unmarshal([]byte(stdout), &report)) + require.NotNil(t, report.UserInfo) + assert.Equal(t, "user-1", report.UserInfo.Subject) +} + +func TestStatusCmd_JSONNotLoggedInIsAnError(t *testing.T) { + withService(t, &fakeService{statusErr: domain.ErrOAuthNotLoggedIn}) + + stdout, _, err := testutil.ExecuteSubCommand(newStatusCmd(), "--json") + + require.ErrorIs(t, err, domain.ErrOAuthNotLoggedIn) + assert.NotContains(t, stdout, `"logged_in"`, "no report is written when there is no session") +} + +func TestOAuthCmds_QuietSuppressesOutput(t *testing.T) { + tests := []struct { + name string + run func(t *testing.T) (string, string, error) + }{ + {"status", func(t *testing.T) (string, string, error) { + withService(t, &fakeService{session: loggedInSession()}) + return testutil.ExecuteSubCommand(newStatusCmd(), "--quiet") + }}, + {"login", func(t *testing.T) (string, string, error) { + withService(t, &fakeService{loginResult: &oauthlogin.LoginResult{Issuer: "i", ClientID: "c"}}) + return testutil.ExecuteSubCommand(newLoginCmd(), "--quiet") + }}, + {"logout", func(t *testing.T) (string, string, error) { + withService(t, &fakeService{}) + return testutil.ExecuteSubCommand(newLogoutCmd(), "--quiet") + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stdout, stderr, err := tt.run(t) + require.NoError(t, err) + assert.Empty(t, strings.TrimSpace(stdout)) + assert.Empty(t, strings.TrimSpace(stderr)) + }) + } +} + +func TestLoginCmd_JSONReportsSessionAndDashboard(t *testing.T) { + svc := &fakeService{loginResult: &oauthlogin.LoginResult{ + Issuer: "https://auth.example.test", + ClientID: "client-1", + Scope: "openid email", + Resource: "https://mcp.us.nylas.com", + ExpiresAt: time.Now().Add(time.Hour), + DroppedScopes: []string{"notetaker.read"}, + }} + withService(t, svc) + + stdout, stderr, err := testutil.ExecuteSubCommand(newLoginCmd(), "--json") + require.NoError(t, err) + + var report loginReport + require.NoError(t, json.Unmarshal([]byte(stdout), &report), "stdout must be pure JSON: %q", stdout) + assert.True(t, report.LoggedIn) + assert.Equal(t, "https://mcp.us.nylas.com", report.Resource) + assert.Equal(t, []string{"notetaker.read"}, report.DroppedScopes) + assert.False(t, report.HasRefresh) + assert.True(t, report.Dashboard.SignedIn) + assert.Equal(t, "org_1", report.Dashboard.OrgPublicID) + assert.Contains(t, stderr, "Opening your browser", "the prompt moves to stderr, off the JSON stream") +} + +func TestLoginCmd_JSONDashboardOutcomes(t *testing.T) { + tests := []struct { + name string + exchangeErr error + want dashboardOutcome + }{ + {"kept dashboard login", dashboardapp.ErrDashboardLoginSessionKept, dashboardOutcome{KeptDashboardLogin: true}}, + {"exchange failed", errors.New("no exchange"), dashboardOutcome{Error: "no exchange"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dash := withService(t, &fakeService{loginResult: &oauthlogin.LoginResult{Issuer: "i", ClientID: "c"}}) + dash.exchangeErr = tt.exchangeErr + + stdout, _, err := testutil.ExecuteSubCommand(newLoginCmd(), "--json") + require.NoError(t, err, "the OAuth login itself succeeded") + + var report loginReport + require.NoError(t, json.Unmarshal([]byte(stdout), &report)) + assert.Equal(t, tt.want, report.Dashboard) + }) + } +} + +func TestLogoutCmd_JSONReport(t *testing.T) { + svc := &fakeService{} + dash := withService(t, svc) + dash.clearErr = errors.New("keyring locked") + + stdout, _, err := testutil.ExecuteSubCommand(newLogoutCmd(), "--json") + require.NoError(t, err) + + var report logoutReport + require.NoError(t, json.Unmarshal([]byte(stdout), &report), "stdout must be pure JSON: %q", stdout) + assert.True(t, report.LoggedOut) + assert.Equal(t, "keyring locked", report.DashboardError) + assert.True(t, svc.logoutCalled) +} + +func TestWireDashboard_SetsTheDashboardHooks(t *testing.T) { + originalSource, originalRelogin := dashboard.OAuthTokenSource, dashboard.OAuthRelogin + t.Cleanup(func() { + dashboard.OAuthTokenSource, dashboard.OAuthRelogin = originalSource, originalRelogin + }) + + assert.Nil(t, dashboard.OAuthTokenSource, "importing the oauth package must not wire the dashboard") + assert.Nil(t, dashboard.OAuthRelogin, "importing the oauth package must not wire the dashboard") + + WireDashboard() + + assert.NotNil(t, dashboard.OAuthTokenSource) + assert.NotNil(t, dashboard.OAuthRelogin) +} diff --git a/internal/cli/oauth/status.go b/internal/cli/oauth/status.go index 89acf35..f5220bb 100644 --- a/internal/cli/oauth/status.go +++ b/internal/cli/oauth/status.go @@ -23,7 +23,11 @@ func newStatusCmd() *cobra.Command { The access token's claims (audience, grants, scopes, expiry) are DECODED locally and NOT VERIFIED: the CLI does not check the token's signature. They describe what the stored token says about itself; only the resource server's -answer is authoritative. Use --verify to ask the authorization server.`, +answer is authoritative. Use --verify to ask the authorization server. + +--json (or --format yaml) prints the same facts as structured output, still +without any token; --quiet prints nothing and leaves the exit code to say +whether a session is stored.`, RunE: func(cmd *cobra.Command, _ []string) error { svc, err := createLoginServiceFn() if err != nil { @@ -38,6 +42,23 @@ answer is authoritative. Use --verify to ask the authorization server.`, ctx, cancel := common.CreateContext() defer cancel() + if structuredOutput(cmd) { + report := newStatusReport(session, time.Now()) + if remote { + info, err := svc.UserInfo(ctx) + if err != nil { + return wrapOAuthError(err) + } + report.UserInfo = &userInfoReport{ + Subject: info.Subject, + Email: info.Email, + EmailVerified: info.EmailVerified, + Org: info.Org, + } + } + return common.GetOutputWriter(cmd).Write(report) + } + out := cmd.OutOrStdout() expired := session.Tokens.IsExpired(time.Now()) if expired {