Skip to content

Conversation

@capcom6
Copy link
Member

@capcom6 capcom6 commented Nov 23, 2025

Summary by CodeRabbit

  • New Features

    • JWT Bearer authentication added alongside Basic Auth; client now supports token lifecycle (generate/revoke) and automatic selection between Basic and JWT.
    • Public token management API and JWT-related types exposed for token creation, inspection, and revocation.
  • Documentation

    • README, Quickstart and Authentication sections updated to demonstrate JWT workflow, configuration guidance, and security notes.
  • Tests

    • Expanded coverage for JWT flows and Basic Auth backward compatibility.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 23, 2025

Walkthrough

Adds JWT Bearer support alongside Basic Auth, introduces token lifecycle APIs (generateToken, revokeToken) and TokenRequest/TokenResponse types, makes httpClient optional with an internal default, updates client auth selection and tests, and rewrites README Quickstart and auth docs for JWT workflows.

Changes

Cohort / File(s) Change Summary
Documentation
README.md
Renamed project title; added Authentication section and Authentication Configuration; rewrote Quickstart to show JWT lifecycle (generate, use, revoke); documented Token Management API, security notes, and updated TOC/anchors.
Domain Types
src/domain.ts
Added exported interfaces TokenRequest (scopes: string[], ttl?: number) and TokenResponse (access_token, token_type, id, expires_at).
Client Implementation
src/client.ts
Made httpClient optional and added private getDefaultHttpClient(); adjusted constructor behavior and auth header logic to use Bearer when login is empty and Basic when login provided; added generateToken(request: TokenRequest): Promise<TokenResponse> and revokeToken(jti: string): Promise<void>; consolidated default headers and response handling.
Tests
src/client.test.ts
Added JWT-related tests for token-based client usage, token generation (with/without TTL), and revocation; preserved and validated Basic-auth backward-compat tests; expanded coverage across client operations (messages, state, devices, inbox export, logs, settings, webhooks).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Client
    participant API as SMSGate API

    rect rgb(248,249,255)
    note over User,Client: Obtain JWT (optional) using Basic credentials
    User->>Client: new Client(login, password)
    Client->>API: POST /token (Authorization: Basic ...)
    API-->>Client: 200 TokenResponse
    User->>Client: new Client("", access_token)
    end

    rect rgb(240,255,240)
    note over User,Client: JWT-authenticated operations
    User->>Client: sendMessage(...)
    Client->>API: POST /messages (Authorization: Bearer access_token)
    API-->>Client: 200 OK
    User->>Client: getState()
    Client->>API: GET /state (Authorization: Bearer access_token)
    API-->>Client: 200 State
    end

    rect rgb(255,250,240)
    note over User,Client: Token lifecycle management
    User->>Client: generateToken({scopes, ttl})
    Client->>API: POST /token (Auth: Basic or Bearer)
    API-->>Client: 200 TokenResponse
    User->>Client: revokeToken(jti)
    Client->>API: POST /token/revoke (Auth)
    API-->>Client: 204 No Content
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review constructor auth-selection edge cases (empty login with empty token, missing password).
  • Verify API endpoints/payloads for generateToken and revokeToken match server contract and TokenResponse shape.
  • Inspect getDefaultHttpClient() for header merging, 204 handling, JSON/text parsing, and error propagation.
  • Confirm tests correctly exercise both Basic and JWT flows and that new domain types are exported.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title '[client] add JWT support' accurately and concisely summarizes the main change: introducing JWT authentication functionality to the client.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch client/jwt-support

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eca215d and 23653ed.

📒 Files selected for processing (4)
  • README.md (9 hunks)
  • src/client.test.ts (3 hunks)
  • src/client.ts (3 hunks)
  • src/domain.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/domain.ts
  • README.md
🧰 Additional context used
🧬 Code graph analysis (2)
src/client.test.ts (3)
src/client.ts (2)
  • Client (18-337)
  • BASE_URL (16-16)
src/http.ts (1)
  • HttpClient (1-7)
src/domain.ts (2)
  • TokenRequest (477-487)
  • TokenResponse (492-512)
src/client.ts (2)
src/http.ts (1)
  • HttpClient (1-7)
src/domain.ts (2)
  • TokenRequest (477-487)
  • TokenResponse (492-512)
🔇 Additional comments (8)
src/client.test.ts (4)

14-17: LGTM! Token types properly imported.

The addition of TokenRequest and TokenResponse imports aligns with the new token management functionality being tested.


346-432: Excellent JWT authentication test coverage!

The JWT authentication tests are well-structured and comprehensive:

  • Client creation with empty login and JWT token
  • Authorization header verification (Bearer fake-token-123)
  • Error handling for missing tokens

Note: The past review concern about JWT-like strings triggering secret scanners has been properly addressed by using 'fake-token-123' instead of actual JWT-formatted strings.


434-491: LGTM! Backward compatibility thoroughly tested.

The tests confirm that existing Basic Auth functionality remains intact, including proper error handling when credentials are incomplete.


514-601: Token management tests are comprehensive.

Covers all token lifecycle scenarios:

  • Token generation with and without TTL
  • Token revocation
  • Proper endpoint URLs and header verification
src/client.ts (4)

10-12: LGTM! Imports updated for token management.

TokenRequest and TokenResponse are properly imported to support the new JWT token lifecycle methods.


24-25: Constructor improvements are well-designed.

The optional httpClient parameter with automatic fallback to a default implementation provides flexibility while maintaining ease of use. JSDoc accurately reflects the dual authentication modes.

Also applies to: 29-34


41-51: Authentication logic is sound, and cross-platform fix applied correctly.

The constructor properly differentiates between JWT (empty login) and Basic Auth (non-empty login) with clear error messages for incomplete credentials.

The switch from Buffer.from() to btoa() on line 50 resolves the past review concern about Node.js-specific APIs, making the client more portable across JavaScript runtimes.


310-336: Token management methods are well-implemented.

Both generateToken and revokeToken correctly:

  • Use appropriate HTTP methods and endpoints
  • Include required headers (Content-Type where needed, Authorization)
  • Return proper types (TokenResponse and void respectively)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 915de6d and f942f32.

📒 Files selected for processing (4)
  • README.md (9 hunks)
  • src/client.test.ts (3 hunks)
  • src/client.ts (3 hunks)
  • src/domain.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/client.test.ts (3)
src/client.ts (2)
  • Client (18-320)
  • BASE_URL (16-16)
src/http.ts (1)
  • HttpClient (1-7)
src/domain.ts (4)
  • Message (52-88)
  • MessageState (32-47)
  • TokenRequest (477-487)
  • TokenResponse (492-512)
src/client.ts (2)
src/http.ts (1)
  • HttpClient (1-7)
src/domain.ts (2)
  • TokenRequest (477-487)
  • TokenResponse (492-512)
🪛 Gitleaks (8.29.0)
src/client.test.ts

[high] 350-350: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 536-536: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 563-563: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (5)
src/domain.ts (1)

474-512: JWT token domain types look consistent with client and docs

TokenRequest / TokenResponse are well-shaped for the new token endpoints and align with how Client.generateToken/revokeToken and the README describe them. No changes needed here.

src/client.test.ts (1)

434-491: Backward-compatibility tests correctly guard the Basic Auth path

The “Client Backward Compatibility” block re-asserts the legacy Basic Auth behavior (including the missing-password error), which is important after changing the constructor semantics. These tests look correct and should help prevent regressions.

README.md (1)

281-311: Authentication and JWT token management docs are consistent with the client API

  • The constructor arguments table and “Authentication Configuration” examples correctly describe the new Client(login, passwordOrToken, httpClient?, baseUrl?) semantics and how Basic vs JWT is selected based on login.
  • The Core Methods table and JWT Token Management rows (generateToken, revokeToken) match the signatures and behavior in src/client.ts.
  • The TokenRequest/TokenResponse type snippets mirror the domain definitions, which helps keep the docs and types aligned.

No changes needed here.

Also applies to: 342-345, 412-446

src/client.ts (2)

23-52: Constructor changes correctly separate Basic vs JWT auth and keep legacy usage working

The updated constructor and header setup:

  • Correctly distinguish JWT (login === "") from Basic (login !== "") and enforce the right required value (passwordOrToken) in each case.
  • Preserve existing call sites like new Client('login', 'password', httpClient) while enabling the new two-arg form that uses the built-in HTTP client.
  • Produce Authorization headers (Bearer / Basic <base64>) that match the new tests and README examples.

This looks coherent and backward compatible.


293-319: Token generation and revocation methods correctly wrap the new endpoints

generateToken and revokeToken:

  • Use the expected /auth/token and /auth/token/{jti} URLs.
  • Apply the same header conventions as other JSON endpoints (including Content-Type where appropriate).
  • Respect the TokenRequest/TokenResponse types from the domain layer.

These methods look correct and are well-covered by the new tests.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f942f32 and 58ecb81.

📒 Files selected for processing (3)
  • README.md (9 hunks)
  • src/client.test.ts (3 hunks)
  • src/client.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/client.test.ts (3)
src/client.ts (2)
  • Client (18-337)
  • BASE_URL (16-16)
src/http.ts (1)
  • HttpClient (1-7)
src/domain.ts (3)
  • Message (52-88)
  • TokenRequest (477-487)
  • TokenResponse (492-512)
🔇 Additional comments (13)
src/client.test.ts (4)

14-17: LGTM! Import additions align with JWT feature.

The new imports (TokenRequest, TokenResponse, WebHookEventType) correctly support the JWT authentication and token management test additions.


346-432: LGTM! JWT authentication tests are comprehensive and address past feedback.

The JWT auth tests properly verify:

  • Client creation with JWT tokens
  • Bearer token authentication headers
  • Error handling for missing tokens

The use of 'fake-token-123' successfully avoids triggering secret scanners, resolving the previous gitleaks concern.


434-491: LGTM! Backward compatibility tests ensure Basic Auth still works.

The tests properly verify that the legacy authentication path (login + password) continues to function correctly, including:

  • Client initialization
  • Basic Auth header generation
  • Error handling for missing passwords

514-601: LGTM! Token management tests thoroughly cover the JWT lifecycle.

The tests properly verify:

  • Token generation with and without TTL
  • Token revocation by JWT ID
  • Correct HTTP methods and endpoints
  • Response structure validation

The use of a Basic Auth client to perform token operations follows the expected workflow where initial authentication is required to generate JWT tokens.

src/client.ts (4)

10-12: LGTM! Import additions support new functionality.

The added imports (MessagesExportRequest, TokenRequest, TokenResponse) are all used in the file and correctly support the JWT token management and inbox export features.


23-52: LGTM! Constructor cleanly supports both authentication modes.

The updated constructor properly:

  • Accepts optional httpClient with sensible fallback
  • Distinguishes JWT (empty login) from Basic Auth (non-empty login)
  • Validates credentials for each mode with clear error messages
  • Uses correct header formats (Bearer vs Basic)

Note: Line 50 uses Buffer.from(), which is Node.js-specific. This is appropriate given the README states the library is server-side focused and requires Node.js v18+.


54-110: LGTM! Default HTTP client properly handles 204, errors, and content types.

The implementation successfully addresses previous feedback by:

  • Returning null for 204 No Content responses (avoiding JSON parse errors)
  • Providing clear error messages with HTTP status and response text
  • Checking Content-Type before attempting JSON parsing
  • Centralizing response handling across all HTTP methods

This ensures methods like deleteWebhook, deleteDevice, and revokeToken work correctly when the server returns 204.


310-336: LGTM! Token management methods follow established patterns.

Both generateToken and revokeToken are implemented consistently with other client methods:

  • Correct HTTP methods and endpoints
  • Proper header configuration
  • Appropriate return types

The methods integrate seamlessly with the existing client API surface.

README.md (5)

44-65: LGTM! Authentication explanation is clear and well-structured.

The new authentication section effectively:

  • Distinguishes Basic Auth from JWT
  • Provides clear guidance on when to use each method
  • Positions JWT as the production-recommended approach
  • Balances simplicity with security considerations

143-176: LGTM! Quickstart correctly uses .state property.

The example properly references updatedState.state at line 159, correctly matching the MessageState interface. This resolves the past review concern about using .status incorrectly.

The JWT-based workflow (generate token → create client → send message → check state → revoke token) provides a complete and realistic usage example.


281-310: LGTM! Authentication configuration examples are clear and correct.

The side-by-side examples effectively demonstrate both authentication modes, and the automatic detection explanation helps users understand how the client determines which method to use.


412-446: LGTM! JWT type definitions are accurate and well-documented.

The TokenRequest and TokenResponse interface definitions match the actual types in src/domain.ts and include helpful JSDoc comments for developers.


465-476: LGTM! Security notes appropriately cover both authentication modes.

The updated security guidance includes:

  • Existing best practices for credentials
  • JWT-specific recommendations (TTL, scoping, revocation)
  • Principle of least privilege

The notes provide comprehensive security guidance for both Basic Auth and JWT users.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58ecb81 and 245a7f4.

📒 Files selected for processing (1)
  • README.md (9 hunks)
🔇 Additional comments (6)
README.md (6)

142-159: ✅ Past issue resolved: updatedState.state is now correct.

The previous review flagged incorrect use of updatedState.status at line 143 (old numbering). The current example correctly uses updatedState.state at line 158, aligning with the MessageState interface.


129-140: ✅ Past issue resolved: undefined httpFetchClient reference removed.

The previous review flagged an undefined httpFetchClient reference. The current Quickstart example correctly instantiates the JWT client without the third parameter (lines 134–137), relying on the built-in HTTP client fallback.


44-307: Excellent authentication documentation and clear JWT/Basic auth distinction.

The new Authentication section (lines 44–65) clearly delineates when to use Basic vs. JWT, and the Authentication Configuration subsection (lines 287–307) provides side-by-side examples with automatic method detection logic. The Quickstart JWT workflow (lines 102–176) demonstrates the full token lifecycle: generation, client initialization, message sending, and revocation. This is a significant improvement.


339-342: JWT token management methods properly documented.

The Core Methods table now includes generateToken(request: TokenRequest) and revokeToken(jti: string) with clear descriptions and return types. Good addition.


409-443: TokenRequest and TokenResponse types clearly defined.

Both types are well-documented with JSDoc comments explaining scopes, ttl, and token metadata fields (access_token, token_type, id, expires_at). Consistent with the implementation described in the AI summary.


462-473: Security notes updated with JWT-specific guidance.

New bullet points covering token TTL, least privilege, and token revocation workflows strengthen the security guidance. These align with JWT best practices.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 245a7f4 and d28bbf5.

📒 Files selected for processing (2)
  • README.md (9 hunks)
  • src/client.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/client.ts (2)
src/http.ts (1)
  • HttpClient (1-7)
src/domain.ts (2)
  • TokenRequest (477-487)
  • TokenResponse (492-512)
🔇 Additional comments (13)
src/client.ts (4)

10-12: LGTM!

The addition of TokenRequest and TokenResponse imports properly supports the new JWT token management features.


54-110: Excellent implementation addressing previous concerns!

The default HTTP client now properly handles:

  • 204 No Content responses (returns null)
  • HTTP errors with meaningful error messages
  • Content-Type detection to avoid JSON parse errors on non-JSON responses

This resolves the issues raised in the previous review about methods like deleteWebhook, revokeToken, etc. breaking on 204 responses. Based on past review comments, this is exactly what was requested.


310-323: LGTM!

The generateToken method follows the established pattern for API methods and provides proper type safety with TokenRequest (scopes and ttl parameters) and TokenResponse (access_token, token_type, id, expires_at fields).


325-336: LGTM!

The revokeToken method is correctly implemented and will properly handle the expected 204 No Content response thanks to the improved default HTTP client.

README.md (9)

1-1: LGTM!

The updated title "📱 SMSGate JS/TS API Client" is clear and consistent with the project focus.


44-65: Excellent documentation of authentication methods!

The new Authentication section provides clear guidance on when to use Basic vs JWT authentication, helping users make informed decisions based on their use case (development vs production, security requirements, etc.).


102-176: Excellent comprehensive example addressing previous issues!

The Quickstart now demonstrates a complete JWT workflow:

  1. Basic Auth client for token generation
  2. JWT token generation with scopes and TTL
  3. JWT-authenticated client initialization
  4. Message sending and state checking
  5. Token revocation

Previous review issues have been fixed:

  • The undefined httpFetchClient reference has been removed (lines 134-137 now correctly omit the httpClient parameter)
  • Line 158 now correctly uses updatedState.state instead of updatedState.status

Based on past review comments, these were the exact changes requested.


280-286: Configuration table correctly reflects optional httpClient!

The table now properly shows httpClient with fetch as the default, which aligns with the implementation where httpClient is an optional parameter with a default fetch-based client. This addresses the previous review concern about marking it as "Required". Based on past review comments, this is the correct representation.


287-308: Clear authentication configuration guidance!

The Authentication Configuration section effectively explains:

  • How to configure Basic Authentication (with username and password)
  • How to configure JWT Authentication (empty string for login, token for password)
  • The automatic detection mechanism

This makes the dual-auth implementation intuitive for users.


340-342: LGTM!

The JWT Token Management methods are properly documented in the Core Methods table with correct signatures and return types.


409-443: LGTM!

The JWT Authentication type definitions are well-documented and match the actual interfaces, providing users with clear guidance on the token request/response structures.


450-450: LGTM!

The updated HTTP Clients section correctly reflects that the library comes with a fetch-based built-in HTTP client, matching the getDefaultHttpClient implementation.


469-473: Excellent security guidance!

The updated Security Notes provide important JWT-specific recommendations including password rotation, strong passwords, appropriate TTL values, least privilege, and token revocation workflows. These are essential best practices for production JWT implementations.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d28bbf5 and eca215d.

📒 Files selected for processing (1)
  • src/client.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/client.ts (2)
src/http.ts (1)
  • HttpClient (1-7)
src/domain.ts (2)
  • TokenRequest (477-487)
  • TokenResponse (492-512)
🔇 Additional comments (3)
src/client.ts (3)

310-323: LGTM!

The generateToken method correctly implements JWT token generation with proper headers and type safety.


325-336: LGTM!

The revokeToken method correctly implements JWT token revocation. With the 204 handling in getDefaultHttpClient, this will work properly even if the server returns no content.


50-50: The original review comment is incorrect. The project explicitly requires Node.js v18+ (as stated in README.md), and btoa() is available in Node.js v16.0.0+. Since v18 exceeds the minimum version where btoa() is supported, there is no compatibility issue.

Likely an incorrect or invalid review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants