fix(auth): repair PAT login by validating token with asana@3 SDK#31
Merged
Conversation
`asana auth login --token <pat>` called the removed v1 helper `asana.Client.create().useAccessToken()`, which throws "undefined is not an object (evaluating 'Client.create')" on asana@3. Validate the token through the v3 SDK (`ApiClient.instance` + `UsersApi`) before persisting it, matching how `getAsanaClient()` already authenticates, and reset the cached client so the new credential is picked up. Adds a regression test that mocks the SDK and config (no disk/network) to verify the PAT path validates the token and saves a `pat` credential.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR contains two independent changes: (1) PAT token validation in ChangesPAT Auth SDK v3 Migration
E2E Test Credential Gating
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Personal Access Token (PAT) authentication path in the CLI to use the Asana v3 SDK instead of the deprecated v1 helper, and introduces a regression test to verify this behavior. The reviewer recommended adding a defensive null check on the API client's token authentication object to prevent potential runtime errors during token assignment.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/commands/auth-login-pat.test.ts (1)
17-55: ⚡ Quick winStrengthen this regression by asserting
resetClient()andgetUser('me').This test proves persistence, but it doesn’t currently lock the two key contract points introduced by this PR (
resetClientcall and'me'target argument).💡 Suggested test hardening
test('validates the token via the v3 SDK and saves a PAT config', async () => { let tokenSeenByApi: string | undefined + let userGidSeen: string | undefined const tokenAuth: { accessToken?: string } = {} // Stub the asana SDK: capture the token the UsersApi call would use. mock.module('asana', () => ({ default: { ApiClient: { instance: { authentications: { token: tokenAuth } } }, UsersApi: class { - async getUser() { + async getUser(userGid: string) { + userGidSeen = userGid tokenSeenByApi = tokenAuth.accessToken return { data: { gid: '1', name: 'Test User', email: 'test@example.com' } } } }, }, })) + + const resetClientMock = mock(() => {}) + mock.module('../../src/lib/asana-client', () => ({ + getAsanaClient: () => ({ users: { me: async () => ({}) } }), + resetClient: resetClientMock, + })) // Capture saveConfig instead of writing to ~/.asana-cli. let savedConfig: any = null mock.module('../../src/lib/config', () => ({ @@ await auth.parseAsync(['login', '--token', 'pat-secret-123'], { from: 'user' }) // The token was validated against the API before being persisted... expect(tokenSeenByApi).toBe('pat-secret-123') + expect(userGidSeen).toBe('me') + expect(resetClientMock).toHaveBeenCalledTimes(1) // ...and stored as a PAT credential. expect(savedConfig).toMatchObject({ accessToken: 'pat-secret-123', authType: 'pat', }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/commands/auth-login-pat.test.ts` around lines 17 - 55, The test needs to be strengthened to verify that resetClient() is called and that getUser() is called with the 'me' argument. In the mock setup for ApiClient, add tracking to capture when resetClient() is called. In the mock UsersApi class's getUser method, capture the argument passed to verify it is 'me'. Then add assertions after the existing expect statements to verify both that resetClient was called and that getUser was called with the 'me' argument as the target.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/commands/auth-login-pat.test.ts`:
- Around line 17-55: The test needs to be strengthened to verify that
resetClient() is called and that getUser() is called with the 'me' argument. In
the mock setup for ApiClient, add tracking to capture when resetClient() is
called. In the mock UsersApi class's getUser method, capture the argument passed
to verify it is 'me'. Then add assertions after the existing expect statements
to verify both that resetClient was called and that getUser was called with the
'me' argument as the target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d5d8c2b-b6c3-4f69-ab75-f7aa488441f4
📒 Files selected for processing (2)
src/commands/auth.tstest/commands/auth-login-pat.test.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
Architecture diagram
sequenceDiagram
participant CLI as CLI Runner
participant AuthCmd as auth login Command
participant ApiClient as asana ApiClient
participant UsersApi as asana UsersApi
participant Config as Config Module
participant Env as User Environment
Note over CLI,Env: PAT Login Flow
CLI->>AuthCmd: parse(['login', '--token', 'pat-secret-123'])
AuthCmd->>AuthCmd: Check for --token flag
alt PAT authentication
AuthCmd->>AuthCmd: Display "Authenticating with Personal Access Token..."
AuthCmd->>ApiClient: Set accessToken on ApiClient.instance.authentications.token
AuthCmd->>UsersApi: new UsersApi().getUser('me', {})
UsersApi->>ApiClient: Use stored token for auth header
ApiClient->>Asana API: GET /users/me (validate token)
Asana API-->>ApiClient: User data (or 401)
alt Token valid
ApiClient-->>UsersApi: User object
UsersApi-->>AuthCmd: { data: { gid, name, email } }
AuthCmd->>Config: saveConfig({ accessToken, authType: 'pat', workspace })
Config->>Env: Write to ~/.asana-cli config file
Config-->>AuthCmd: Config saved
AuthCmd->>AuthCmd: resetClient() - force re-init with new creds
AuthCmd->>AuthCmd: Display "Successfully authenticated with PAT"
else Token invalid
ApiClient-->>UsersApi: 401 error
UsersApi-->>AuthCmd: Error thrown
AuthCmd->>AuthCmd: Display authentication failure message
AuthCmd->>AuthCmd: Return with error
end
else OAuth flow (unchanged)
AuthCmd->>AuthCmd: Proceed with existing OAuth logic
end
AuthCmd-->>CLI: Command result (success/failure)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
asana auth login --token <pat>is broken. It calls the v1 helperasana.Client.create().useAccessToken()(src/commands/auth.ts), which wasremoved in asana@3 — the installed SDK. Running it throws:
The rest of the CLI already authenticates correctly via the v3 SDK in
getAsanaClient()(ApiClient.instance+ per-API classes); only the PATlogin validation step still used the old API.
Fix
set
asana.ApiClient.instance.authentications.token.accessTokenand callnew asana.UsersApi().getUser('me', {}).resetClient()after saving so the new credential is picked up.Test
Adds
test/commands/auth-login-pat.test.ts— a regression test that mocks theasana SDK and the config module (no disk, no network) and asserts the PAT login
path validates the token and saves a
{ authType: 'pat' }credential. Failsagainst the old
Client.create()code, passes with the fix.Notes
bun run lintand the new test pass locally.Summary by cubic
Fixes PAT login by validating tokens with the
asanav3 SDK instead of the removed v1 helper. Prevents the runtime error and ensures the saved PAT is used right away.Bug Fixes
asana@3by settingApiClient.instance.authentications.token.accessToken, guarding when the token auth scheme is missing, and callingnew asana.UsersApi().getUser('me', {}); thenresetClient()after saving so the new PAT is used.Tests
asanaand config, and skip E2E suites whenASANA_ACCESS_TOKEN/ASANA_WORKSPACEare missing via ahasE2ECredentialshelper.Written for commit ef4e16f. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
auth logincommand by validating the token before saving credentials, with extra safeguards for token-auth initialization.Tests
auth login --tokento ensure token validation occurs and the expected authentication settings are persisted.