Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR hardens the three Changes
Review Notes
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Revocation authentication and error handling need correction, and JSON token-only output must include client identity.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Updates token issuance and revocation to preserve client identity and use the SDK consistently.
Changes:
- Adds shared client-ID resolution and server JWT classification.
- Includes identity in token output.
- Migrates token revocation to SDK calls with per-target handling.
- Updates related tests and REST mocks.
| File | Description |
|---|---|
test/unit/commands/auth/revoke-token.test.ts |
Tests SDK revocation behavior. |
test/unit/commands/auth/issue-jwt-token.test.ts |
Tests identity and server claims. |
test/unit/commands/auth/issue-ably-token.test.ts |
Tests identity resolution and warnings. |
test/helpers/mock-ably-rest.ts |
Adds revocation mock support. |
src/services/client-identity.ts |
Updates wildcard validation messaging. |
src/commands/auth/revoke-token.ts |
Uses SDK token revocation. |
src/commands/auth/issue-jwt-token.ts |
Adds server claims and identity output. |
src/commands/auth/issue-ably-token.ts |
Uses shared identity resolution. |
src/base-command.ts |
Resolves token client identity. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| try { | ||
| const rest = await this.createAblyRestClient({ | ||
| ...flags, | ||
| "api-key": apiKey, | ||
| }); |
| clientId: tokenDetails.clientId ?? null, | ||
| capability: tokenDetails.capability, |
| clientId: clientId ?? null, | ||
| ...(clientType ? { clientType } : {}), |
- `auth issue-ably-token` / `issue-jwt-token` resolve the token's client ID through one helper: --client-id, else the client ID the CLI acts as. "*" is refused, and "none" still issues an anonymous token but warns that apps requiring identified clients reject it. - Token output always states the identity: `Client ID: anonymous` in human output and `clientId: null` in JSON, rather than omitting it. - `auth issue-jwt-token --client-type server` adds the signed `x-ably-clientType=server` claim, which is the only way a token-authenticated client can be classified as a server. - `auth revoke-token` goes through the SDK's `auth.revokeTokens` instead of a hand-rolled HTTPS call to a hardcoded rest.ably.io, so it honours the configured endpoint and reports per-target failures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
47d5b85 to
74efc8b
Compare
umair-ably
left a comment
There was a problem hiding this comment.
approving with the caveat you fix this


auth issue-ably-token/issue-jwt-tokenresolve the token's clientID through one helper: --client-id, else the client ID the CLI acts
as. "*" is refused, and "none" still issues an anonymous token but
warns that apps requiring identified clients reject it.
Client ID: anonymousinhuman output and
clientId: nullin JSON, rather than omitting it.auth issue-jwt-token --client-type serveradds the signedx-ably-clientType=serverclaim, which is the only way atoken-authenticated client can be classified as a server.
auth revoke-tokengoes through the SDK'sauth.revokeTokensinsteadof a hand-rolled HTTPS call to a hardcoded rest.ably.io, so it honours
the configured endpoint and reports per-target failures.