Add OAuth2 token management and storage implementation #78
Conversation
- Introduced `OAuth2TokenManager` for managing access and refresh tokens, including validation and automatic refreshing. - Added `FileTokenStorage` for persistent token storage, allowing tokens to be saved and loaded from a file. - Created `IOAuth2TokenManager` and `TokenRefreshProvider` interfaces to define the structure for token management and refreshing. - Implemented `OAuth2TokenRefreshProvider` to handle token refresh logic using QuickBooks Online API. - Added constants for token configuration and error messages to improve maintainability and clarity. These changes enhance the application's OAuth2 integration, ensuring robust token management and storage capabilities.
🔍 Vulnerabilities of
|
| digest | sha256:5c12eec75619200160da0e7d5bc8bff95e75bcf87a3273f218cb4f24f94b9d49 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 243 MB |
| packages | 1628 |
📦 Base Image node:20-alpine
| also known as |
|
| digest | sha256:37a5a350292926f98d48de9af160b0a3f7fcb141566117ee452742739500a5bd |
| vulnerabilities |    |
Description
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
|
📝 WalkthroughWalkthroughThis change introduces a comprehensive OAuth2 token management subsystem. It adds new classes for file-based token storage, token refresh via QuickBooks Online, and a token manager that handles token lifecycle, concurrency, and error handling. Supporting TypeScript interfaces, constants, and error classes are included. Extensive unit tests validate storage, refresh, and error scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuth2TokenManager
participant FileTokenStorage
participant OAuth2TokenRefreshProvider
Client->>OAuth2TokenManager: getAccessToken()
alt Token is valid
OAuth2TokenManager-->>Client: Return cached access token
else Token expired or missing
OAuth2TokenManager->>FileTokenStorage: load()
alt Token data valid and not expired
OAuth2TokenManager-->>Client: Return loaded access token
else Need refresh
OAuth2TokenManager->>OAuth2TokenRefreshProvider: refreshToken(refresh_token)
alt Refresh success
OAuth2TokenManager->>FileTokenStorage: save(new token data)
OAuth2TokenManager-->>Client: Return new access token
else Refresh fails (invalid_grant, etc.)
OAuth2TokenManager->>FileTokenStorage: clear()
OAuth2TokenManager-->>Client: Throw OAuth2Error
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
workers/main/src/services/OAuth2/OAuth2TokenManager.test.ts (1)
91-95: Same issue: avoid type casting to access private methods.This duplicates the same problematic pattern of accessing private methods via type casting.
🧹 Nitpick comments (7)
workers/main/src/services/OAuth2/types.ts (1)
1-43: Excellent type definitions with clear separation of concerns.The interfaces are well-designed with good separation between raw API responses (
TokenResponse) and internal data structures (TokenData). The conversion fromexpires_intoexpires_attimestamp is smart for easier validation logic.Consider adding JSDoc comments to clarify that
expires_atrepresents Unix timestamp in milliseconds, and when the optional fields inTokenManagementResultare populated.workers/main/src/services/OAuth2/FileTokenStorage.ts (3)
33-50: Consider making the load method async for consistency.The API mixes async (
save,clear) and sync (load) methods. Makingloadasync would provide consistency and allow for better error handling.- load(): TokenData | null { + async load(): Promise<TokenData | null> { try { - const data = readFileSync(this.tokenFilePath, 'utf8'); + const data = await fs.readFile(this.tokenFilePath, 'utf8');Remember to update the interface and all callers accordingly.
47-49: Remove redundant null return.Line 48 returns null after the ENOENT check already returned null on line 45. This code is unreachable.
- return null; - } - - return null; + } + return null;
8-72: Consider security implications of plaintext token storage.Storing OAuth2 tokens in plaintext files poses security risks. Consider implementing encryption or using more secure storage mechanisms, especially for production environments.
You might want to:
- Encrypt tokens before writing to disk
- Set restrictive file permissions (600)
- Use environment-specific secure storage (e.g., AWS Secrets Manager, Azure Key Vault)
- Add file locking to prevent concurrent access issues
workers/main/src/services/OAuth2/IOAuth2TokenManager.ts (1)
1-61: Excellent interface design with clear separation of concerns.The interfaces are well-structured with good separation of responsibilities:
TokenStorageProviderhandles persistence operationsTokenRefreshProviderfocuses on token refresh logicIOAuth2TokenManagerprovides the main public APIThe JSDoc documentation is comprehensive and the method signatures are clean and intuitive.
Minor suggestion: Consider documenting whether
getCurrentRefreshToken()can return empty/null values and how callers should handle such cases.workers/main/src/services/OAuth2/OAuth2TokenManager.ts (2)
21-27: Consider dependency injection for better testabilityWhile the implementation works, hardcoding dependencies makes unit testing more difficult. Consider accepting storage and refresh provider as constructor parameters with defaults.
- constructor(serviceName: string, defaultRefreshToken: string) { - this.storage = new FileTokenStorage(serviceName); - this.refreshProvider = new OAuth2TokenRefreshProvider(); + constructor( + serviceName: string, + defaultRefreshToken: string, + storage?: TokenStorageProvider, + refreshProvider?: TokenRefreshProvider + ) { + this.storage = storage ?? new FileTokenStorage(serviceName); + this.refreshProvider = refreshProvider ?? new OAuth2TokenRefreshProvider(); this.defaultRefreshToken = defaultRefreshToken; this.loadTokens(); }
162-165: Incomplete logging implementationThe method has a condition check but no implementation. Either implement the logging or remove the method if not needed.
Would you like me to implement the refresh token change logging or create an issue to track this TODO?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
workers/main/src/services/OAuth2/FileTokenStorage.ts(1 hunks)workers/main/src/services/OAuth2/IOAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.ts(1 hunks)workers/main/src/services/OAuth2/constants.ts(1 hunks)workers/main/src/services/OAuth2/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
🔇 Additional comments (6)
workers/main/src/services/OAuth2/constants.ts (1)
1-14: LGTM! Well-structured constants with proper typing.The constants are well-organized and appropriately typed with
as const. The token buffer configuration (5 minutes + 60 seconds) provides reasonable safety margins for token refresh operations, and the error messages are descriptive.workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.ts (3)
9-36: Well-implemented OAuth2 token refresh flow!The method correctly implements the OAuth2 refresh token grant type with proper form encoding and Basic authentication.
38-69: Comprehensive error handling with clear user guidance!The error handler effectively covers all common OAuth2 error scenarios with actionable error messages.
71-86: Smart token expiry calculation with buffer!The method correctly implements early token refresh by subtracting a buffer from the expiry time, preventing token expiration during API calls.
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (2)
47-57: Thorough token validation logic!Good defensive programming with explicit checks for null, undefined, and empty tokens.
73-90: Excellent concurrency control for token refresh!The implementation properly prevents race conditions by tracking and awaiting existing refresh operations.
- Removed console logging and the `logRefreshTokenChange` method from `OAuth2TokenManager` to streamline the token management process. - Deleted unused interfaces `TokenManagementResult` and `TokenInfo` from `types.ts`, simplifying the codebase and improving maintainability. These changes enhance the clarity and efficiency of the OAuth2 token management implementation.
- Introduced `OAuth2Error` class for better error handling in OAuth2 processes. - Updated `FileTokenStorage` and `OAuth2TokenManager` to throw `OAuth2Error` instead of generic errors, improving error specificity. - Added unit tests for `OAuth2Error` to ensure correct functionality. These changes enhance the robustness of the OAuth2 token management implementation by providing clearer error messages and improving maintainability.
anatolyshipitz
changed the base branch from
main
to
feature/65031_format_date_util
- Added tests to verify the behavior of `getAccessToken` and `getCurrentRefreshToken` methods in various scenarios, including valid, expired, and malformed tokens. - Implemented checks for concurrent access to ensure that multiple calls to `getAccessToken` handle token refresh correctly without causing multiple refresh requests. - Improved error handling tests for token refresh failures, ensuring that appropriate errors are thrown and tokens are cleared when necessary. These changes strengthen the test coverage of the OAuth2 token management implementation, ensuring robustness and reliability in token handling.
- Added a `setTokenDataForTesting` method to `OAuth2TokenManager` for improved testing capabilities. - Removed unnecessary comments and streamlined the `TokenData` interface in `types.ts`. - Introduced unit tests for `OAuth2TokenManager` focusing on storage and error handling scenarios, ensuring robustness in token management. These changes improve the maintainability and testability of the OAuth2 token management implementation, enhancing overall code quality.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (2)
60-72: Avoid empty catch blocks - log storage errorsThe empty catch block silently ignores storage errors, making debugging difficult.
if (tokenData) { this.setTokenData(tokenData); } - } catch {} + } catch (error) { + console.warn('Failed to load tokens from storage:', error); + }
113-122: Log storage clear errorsThe empty catch block silently ignores storage errors.
try { await this.storage.clear(); - } catch {} + } catch (error) { + console.warn('Failed to clear stored tokens:', error); + }
🧹 Nitpick comments (1)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (1)
156-158: Consider restricting test method visibilityThe
setTokenDataForTestingmethod is public and could be misused in production code.Consider adding a comment to clarify its purpose:
+ /** + * @internal For testing purposes only. Do not use in production code. + */ setTokenDataForTesting(tokenData: TokenData): void { this.setTokenData(tokenData); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
workers/main/src/common/errors/OAuth2Error.ts(1 hunks)workers/main/src/services/OAuth2/IOAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.errors.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- workers/main/src/common/errors/OAuth2Error.ts
- workers/main/src/services/OAuth2/IOAuth2TokenManager.ts
- workers/main/src/services/OAuth2/types.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
🪛 GitHub Check: SonarQube
workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts
[failure] 105-105: src/services/OAuth2/OAuth2TokenManager.storage.test.ts > OAuth2TokenManager - Storage & Edge Cases > should handle malformed token data from storage
AssertionError: expected 'refresh-token' to be 'test-refresh-token' // Object.is equality
Expected: "test-refresh-token"
Received: "refresh-token"
❯ src/services/OAuth2/OAuth2TokenManager.storage.test.ts:105:54
workers/main/src/services/OAuth2/OAuth2TokenManager.errors.test.ts
[failure] 141-141: src/services/OAuth2/OAuth2TokenManager.errors.test.ts > OAuth2TokenManager - Error Handling > token refresh logic > should handle refresh token failure with other errors
AssertionError: expected [Function] to throw error including 'Network error' but got 'Cannot read properties of undefined (…'
Expected: "Network error"
Received: "Cannot read properties of undefined (reading 'access_token')"
❯ src/services/OAuth2/OAuth2TokenManager.errors.test.ts:141:7
[failure] 114-114: src/services/OAuth2/OAuth2TokenManager.errors.test.ts > OAuth2TokenManager - Error Handling > token refresh logic > should handle refresh token failure with invalid token error
AssertionError: expected [Function] to throw error including 'invalid or expired refresh token' but got 'Cannot read properties of undefined (…'
Expected: "invalid or expired refresh token"
Received: "Cannot read properties of undefined (reading 'access_token')"
❯ src/services/OAuth2/OAuth2TokenManager.errors.test.ts:114:7
[failure] 86-86: src/services/OAuth2/OAuth2TokenManager.errors.test.ts > OAuth2TokenManager - Error Handling > getAccessToken error scenarios > should handle empty access token after refresh
AssertionError: expected [Function] to throw error including 'Failed to obtain access token' but got 'Cannot read properties of undefined (…'
Expected: "Failed to obtain access token"
Received: "Cannot read properties of undefined (reading 'access_token')"
❯ src/services/OAuth2/OAuth2TokenManager.errors.test.ts:86:7
[failure] 57-57: src/services/OAuth2/OAuth2TokenManager.errors.test.ts > OAuth2TokenManager - Error Handling > getAccessToken error scenarios > should throw error when no access token available after refresh
AssertionError: expected [Function] to throw error including 'Failed to obtain access token' but got 'Cannot read properties of undefined (…'
Expected: "Failed to obtain access token"
Received: "Cannot read properties of undefined (reading 'access_token')"
❯ src/services/OAuth2/OAuth2TokenManager.errors.test.ts:57:7
workers/main/src/services/OAuth2/OAuth2TokenManager.ts
[failure] 98-98: src/services/OAuth2/OAuth2TokenManager.basic.test.ts > OAuth2TokenManager - Basic > getAccessToken > should refresh token when expired and return new access token
TypeError: this.refreshProvider.refreshToken is not a function
❯ OAuth2TokenManager.performTokenRefresh src/services/OAuth2/OAuth2TokenManager.ts:98:36
❯ OAuth2TokenManager.refreshAccessToken src/services/OAuth2/OAuth2TokenManager.ts:85:32
❯ OAuth2TokenManager.getAccessToken src/services/OAuth2/OAuth2TokenManager.ts:35:16
❯ src/services/OAuth2/OAuth2TokenManager.basic.test.ts:142:46
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (1)
workers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts (1)
1-147: LGTM! Well-structured test suite with comprehensive coverageThe test file demonstrates good practices:
- Clear test organization with descriptive test names
- Proper setup and teardown with
beforeEachandafterEach- Comprehensive coverage of basic functionality
- Good use of test data setup
The mocking approach here is more robust than in the error tests file.
workers/main/src/services/OAuth2/OAuth2TokenManager.errors.test.ts
Outdated
Show resolved
Hide resolved
workers/main/src/services/OAuth2/OAuth2TokenManager.errors.test.ts
Outdated
Show resolved
Hide resolved
workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts
Outdated
Show resolved
Hide resolved
- Added new error messages for loading and clearing OAuth2 tokens in `constants.ts`. - Updated `OAuth2TokenManager` to throw specific errors when loading and clearing tokens fails. - Refactored tests to ensure comprehensive coverage of token validation scenarios, including handling of empty, null, and expired tokens. These changes improve the robustness and clarity of the OAuth2 token management implementation, enhancing error handling and overall code quality.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (1)
139-143: Token data validation trade-off acknowledged.While previous feedback suggested adding validation to
setTokenDatafor malformed token data, I understand your preference for keeping implementations simple during early development stages. The current approach prioritizes core functionality over comprehensive validation.If you encounter issues with malformed token data during testing, validation can be added later when moving toward production readiness.
🧹 Nitpick comments (1)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (1)
22-28: Consider async initialization pattern for better error handling.The constructor calls
loadTokens()which can throwOAuth2Error(line 68), making the constructor potentially throw. This can be surprising to users and makes error handling more complex.Consider implementing an async
initialize()method that can be called after construction to handle storage loading errors more gracefully.constructor(serviceName: string, defaultRefreshToken: string) { this.storage = new FileTokenStorage(serviceName); this.refreshProvider = new OAuth2TokenRefreshProvider(); this.defaultRefreshToken = defaultRefreshToken; - - this.loadTokens(); } + + async initialize(): Promise<void> { + this.loadTokens(); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.errors.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/constants.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- workers/main/src/services/OAuth2/constants.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- workers/main/src/services/OAuth2/OAuth2TokenManager.errors.test.ts
- workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts
- workers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
📚 Learning: user anatolyshipitz prefers to keep code implementations simple during early development stages rath...
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
Applied to files:
workers/main/src/services/OAuth2/OAuth2TokenManager.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (3)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (3)
30-58: LGTM! Core token management methods are well-implemented.The token validity logic is comprehensive, including checks for null values, empty strings, and expiry. The non-null assertion in
getAccessTokenis safe due to the preceding validity check.
115-126: LGTM! Token clearing logic is comprehensive.The method properly clears all token state including the refresh promise, and the error handling has been appropriately improved to throw specific errors instead of silent failures.
160-162: LGTM! Testing method is appropriately designed.The
setTokenDataForTestingmethod provides clean test access to internal state without exposing unnecessary implementation details.
…mprovements - Added a new method `isValidTokenData` in `OAuth2TokenManager` to validate token data before processing. - Updated the token loading logic to ensure only valid token data is set. - Introduced a new test case to verify the handling of malformed token data, ensuring the manager gracefully manages invalid tokens. These changes improve the robustness of the OAuth2 token management implementation by ensuring only valid tokens are processed and enhancing error handling capabilities.
- Updated the `isTokenValid` method to include checks for the presence of the refresh token. - Improved the `isValidTokenData` method to validate token data structure and values more thoroughly, ensuring that access and refresh tokens are non-empty strings and that the expiry date is a valid finite number. - Modified the `setTokenData` method to throw an error if invalid token data is provided. - Enhanced the `setTokenDataForTesting` method to handle null token data appropriately. These changes further strengthen the OAuth2 token management implementation by ensuring comprehensive validation of token data, improving error handling, and enhancing overall code reliability.
- Removed the `IOAuth2TokenManager` interface and integrated its functionality into the new `OAuth2TokenManagerInterface` in `types.ts`. - Updated imports in `FileTokenStorage`, `OAuth2TokenManager`, and `OAuth2TokenRefreshProvider` to reference the consolidated types. - This refactor improves code organization and maintainability by centralizing type definitions related to OAuth2 token management.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (1)
140-148: Fragile error handling still relies on message text.The error handling continues to check for 'invalid or expired' in the error message, making it brittle and dependent on specific error message formatting from the refresh provider.
🧹 Nitpick comments (2)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (2)
22-28: Consider async initialization pattern for token loading.Calling
loadTokens()synchronously in the constructor can cause issues if token loading fails, as constructors cannot be made async. Consider implementing an asyncinitialize()method that must be called after construction.constructor(serviceName: string, defaultRefreshToken: string) { this.storage = new FileTokenStorage(serviceName); this.refreshProvider = new OAuth2TokenRefreshProvider(); this.defaultRefreshToken = defaultRefreshToken; - - this.loadTokens(); } +async initialize(): Promise<void> { + await this.loadTokens(); +}
200-211: Consider adding runtime guard for testing method.The testing method bypasses validation and could be dangerous if accidentally used in production. Consider adding a runtime environment check or making it conditional on a test flag.
setTokenDataForTesting(tokenData: TokenData): void { + if (process.env.NODE_ENV === 'production') { + throw new Error('setTokenDataForTesting is not available in production'); + } if (tokenData) { this.accessToken = tokenData.access_token; this.refreshToken = tokenData.refresh_token; this.tokenExpiry = new Date(tokenData.expires_at); } else { this.accessToken = null; this.refreshToken = null; this.tokenExpiry = null; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
workers/main/src/services/OAuth2/FileTokenStorage.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.ts(1 hunks)workers/main/src/services/OAuth2/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- workers/main/src/services/OAuth2/types.ts
- workers/main/src/services/OAuth2/FileTokenStorage.ts
- workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
📚 Learning: user anatolyshipitz prefers to keep code implementations simple during early development stages rath...
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
Applied to files:
workers/main/src/services/OAuth2/OAuth2TokenManager.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (5)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (5)
1-21: Well-structured class definition with proper dependency management.The imports are clean, and the class follows good practices with private state management and dependency injection.
30-58: Public API methods are well-implemented.The token management logic is sound with proper validation, refresh handling, and fallback mechanisms.
60-110: Comprehensive token validation with proper error handling.The token loading and validation logic is thorough and appropriate for security-critical OAuth token handling. The improved error handling addresses previous concerns.
151-162: Storage clearing logic is properly implemented.The method correctly resets all token state and includes appropriate error handling for storage operations.
164-198: Helper methods are well-designed with proper validation.The buffer calculation, token data validation, and storage operations are correctly implemented with appropriate safeguards.
- Updated the `load` method in `FileTokenStorage` to be asynchronous, utilizing `fs.promises.readFile` for improved performance and non-blocking behavior. - Modified the `loadTokens` method in `OAuth2TokenManager` to handle the asynchronous loading of token data. - Adjusted unit tests for `OAuth2TokenManager` to accommodate the asynchronous nature of the `load` method, ensuring proper initialization and handling of token data. These changes enhance the efficiency and responsiveness of the OAuth2 token management implementation by allowing for non-blocking token loading operations.
- Updated the `OAuth2Error` class to include a `code` property, allowing for custom error codes alongside the message and name. - Modified unit tests for `OAuth2Error` to verify the correct setting of default and custom error codes. - Introduced new error codes in `constants.ts` for better error handling across the OAuth2 service. These changes improve the error handling capabilities of the OAuth2 implementation by providing more granular error information, enhancing debugging and user feedback.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workers/main/src/common/errors/OAuth2Error.test.ts(1 hunks)workers/main/src/common/errors/OAuth2Error.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.ts(1 hunks)workers/main/src/services/OAuth2/constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- workers/main/src/common/errors/OAuth2Error.ts
- workers/main/src/services/OAuth2/constants.ts
- workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#78
File: workers/main/src/services/OAuth2/OAuth2TokenManager.ts:0-0
Timestamp: 2025-08-01T13:15:19.633Z
Learning: In workers/main/src/services/OAuth2/OAuth2TokenManager.ts, the user anatolyshipitz enhanced the OAuth2Error class with error code functionality, including a code property and ERROR_CODES constants. The OAuth2TokenRefreshProvider now throws typed errors with specific codes, and the OAuth2TokenManager uses these codes for conditional error handling instead of checking message text.
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
📚 Learning: in workers/main/src/services/oauth2/oauth2tokenmanager.ts, the user anatolyshipitz enhanced the oaut...
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#78
File: workers/main/src/services/OAuth2/OAuth2TokenManager.ts:0-0
Timestamp: 2025-08-01T13:15:19.633Z
Learning: In workers/main/src/services/OAuth2/OAuth2TokenManager.ts, the user anatolyshipitz enhanced the OAuth2Error class with error code functionality, including a code property and ERROR_CODES constants. The OAuth2TokenRefreshProvider now throws typed errors with specific codes, and the OAuth2TokenManager uses these codes for conditional error handling instead of checking message text.
Applied to files:
workers/main/src/common/errors/OAuth2Error.test.tsworkers/main/src/services/OAuth2/OAuth2TokenManager.ts
📚 Learning: user anatolyshipitz prefers to keep code implementations simple during early development stages rath...
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
Applied to files:
workers/main/src/services/OAuth2/OAuth2TokenManager.ts
🧬 Code Graph Analysis (2)
workers/main/src/common/errors/OAuth2Error.test.ts (1)
workers/main/src/common/errors/OAuth2Error.ts (1)
OAuth2Error(3-10)
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (5)
workers/main/src/services/OAuth2/types.ts (4)
OAuth2TokenManagerInterface(27-33)TokenStorageProvider(15-21)TokenRefreshProvider(23-25)TokenData(8-13)workers/main/src/services/OAuth2/FileTokenStorage.ts (1)
FileTokenStorage(8-72)workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.ts (1)
OAuth2TokenRefreshProvider(9-93)workers/main/src/common/errors/OAuth2Error.ts (1)
OAuth2Error(3-10)workers/main/src/services/OAuth2/constants.ts (3)
ERROR_MESSAGES(9-16)ERROR_CODES(18-27)TOKEN_CONFIG(1-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (12)
workers/main/src/common/errors/OAuth2Error.test.ts (1)
1-21: LGTM! Comprehensive test coverage for OAuth2Error class.The test suite properly validates both the default and custom error code scenarios. The test structure follows established patterns in the codebase and covers all essential properties of the OAuth2Error class.
workers/main/src/services/OAuth2/OAuth2TokenManager.ts (11)
1-11: Clean imports and good separation of concerns.The imports are well-organized, pulling in error handling, configuration constants, storage, and refresh provider components. The separation between interfaces and concrete implementations follows good design principles.
12-26: Solid class structure with proper dependency injection.The class properly implements the interface and uses dependency injection for storage and refresh providers. The constructor parameters allow for flexibility while maintaining sensible defaults.
32-46: Robust token retrieval with proper error handling.The
getAccessTokenmethod includes proper initialization, validation, and refresh logic. The double-check for token availability after refresh ensures reliability.
48-50: Good fallback mechanism for refresh token.The null coalescing operator provides a clean fallback to the default refresh token when no stored token is available.
52-62: Comprehensive token validation logic.The validation checks for token presence, non-empty strings, and expiry time. The logic is thorough and handles edge cases well.
71-73: Improved error handling addresses previous concerns.The empty catch block issue from previous reviews has been properly addressed by throwing a specific OAuth2Error with an appropriate error message.
80-114: Thorough token data validation with comprehensive checks.The
isValidTokenDatamethod includes extensive validation for all token properties, including type checking, length validation, and date validity. This prevents invalid token data from being processed.
116-133: Excellent concurrency control implementation.The refresh mechanism properly prevents concurrent refresh operations using a promise-based approach. The buffer check and cleanup logic in the finally block ensure proper state management.
144-152: Enhanced error handling using error codes.The error handling now uses specific error codes instead of checking message text, addressing the fragile error handling concern from previous reviews. This makes the code more robust and maintainable.
163-165: Consistent error handling for storage operations.The storage clear operation now properly throws specific errors instead of using empty catch blocks, maintaining consistency with the improved error handling approach.
179-187: Good validation before setting token data.The
setTokenDatamethod includes proper validation before updating internal state, preventing invalid token data from corrupting the manager's state.
- Updated the `setTokenDataForTesting` method to throw an error if called outside of the test environment, ensuring it is only used in appropriate contexts. - This change improves the safety and reliability of the testing process by preventing misuse of the method in production environments.
- Introduced comprehensive unit tests for the `FileTokenStorage` class, covering methods for saving, loading, and clearing token data, including error handling scenarios. - Added tests for the `OAuth2TokenRefreshProvider` to ensure proper instantiation and method availability. - These tests enhance the reliability and maintainability of the OAuth2 token management implementation by ensuring critical functionalities are thoroughly validated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
workers/main/src/services/OAuth2/FileTokenStorage.load.test.ts (1)
21-117: Consider adding JSON parsing error test scenario.The test suite provides excellent coverage of the load functionality. Consider adding one more test case for malformed JSON that would cause
JSON.parse()to throw aSyntaxError:+ it('should return null when JSON is malformed', async () => { + mockReadFile.mockResolvedValue('{ invalid json'); + + const result = await fileTokenStorage.load(); + + expect(result).toBeNull(); + });This would ensure the implementation handles JSON parsing errors gracefully.
workers/main/src/services/OAuth2/FileTokenStorage.basic.test.ts (1)
43-62: Comprehensive constructor testing.The tests cover all constructor scenarios: default service name, custom service name, and custom token file path. However, the tests only verify instance creation but don't validate the internal state or configuration.
Consider adding assertions to verify that the constructor parameters are properly stored internally, if the class exposes any getters or properties for validation:
it('should create instance with custom service name', () => { const storage = new FileTokenStorage('custom-service'); expect(storage).toBeInstanceOf(FileTokenStorage); + // If FileTokenStorage exposes serviceName property: + // expect(storage.serviceName).toBe('custom-service'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workers/main/src/services/OAuth2/FileTokenStorage.basic.test.ts(1 hunks)workers/main/src/services/OAuth2/FileTokenStorage.clear.test.ts(1 hunks)workers/main/src/services/OAuth2/FileTokenStorage.load.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- workers/main/src/services/OAuth2/OAuth2TokenRefreshProvider.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- workers/main/src/services/OAuth2/OAuth2TokenManager.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#78
File: workers/main/src/services/OAuth2/OAuth2TokenManager.ts:0-0
Timestamp: 2025-08-01T13:15:19.633Z
Learning: In workers/main/src/services/OAuth2/OAuth2TokenManager.ts, the user anatolyshipitz enhanced the OAuth2Error class with error code functionality, including a code property and ERROR_CODES constants. The OAuth2TokenRefreshProvider now throws typed errors with specific codes, and the OAuth2TokenManager uses these codes for conditional error handling instead of checking message text.
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
📚 Learning: in workers/main/src/services/oauth2/oauth2tokenmanager.ts, the user anatolyshipitz enhanced the oaut...
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#78
File: workers/main/src/services/OAuth2/OAuth2TokenManager.ts:0-0
Timestamp: 2025-08-01T13:15:19.633Z
Learning: In workers/main/src/services/OAuth2/OAuth2TokenManager.ts, the user anatolyshipitz enhanced the OAuth2Error class with error code functionality, including a code property and ERROR_CODES constants. The OAuth2TokenRefreshProvider now throws typed errors with specific codes, and the OAuth2TokenManager uses these codes for conditional error handling instead of checking message text.
Applied to files:
workers/main/src/services/OAuth2/FileTokenStorage.clear.test.tsworkers/main/src/services/OAuth2/FileTokenStorage.load.test.tsworkers/main/src/services/OAuth2/FileTokenStorage.basic.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (16)
workers/main/src/services/OAuth2/FileTokenStorage.clear.test.ts (4)
1-19: LGTM! Well-structured imports and comprehensive mocking.The imports are properly organized and the mock setup covers all necessary filesystem operations. The OAuth2Error import aligns with the enhanced error handling mentioned in the retrieved learnings.
21-38: LGTM! Proper test setup with good isolation.The beforeEach/afterEach pattern ensures proper mock initialization and cleanup between tests. The default mock behaviors are sensible and the test isolation is well-implemented.
41-45: LGTM! Clean happy path test.The test properly verifies that the clear method calls unlink with the expected file path. Simple and focused validation.
47-63: LGTM! Comprehensive error handling tests.Both error scenarios are well-tested:
- ENOENT handling correctly doesn't throw (appropriate for idempotent clear operation)
- Other errors are properly wrapped in OAuth2Error with descriptive messages
- Async error handling patterns are correctly implemented
The OAuth2Error usage aligns with the enhanced error handling mentioned in the retrieved learnings.
workers/main/src/services/OAuth2/FileTokenStorage.load.test.ts (5)
1-20: LGTM! Well-structured imports and comprehensive mocking setup.The imports are clean and focused, and the mock configuration properly covers all filesystem operations that the FileTokenStorage class would need.
21-46: LGTM! Proper test setup with clean mock management.The test setup follows best practices with proper mock typing, realistic test data, and clean isolation between tests using beforeEach/afterEach hooks.
47-56: LGTM! Comprehensive success scenario test.The test properly validates the happy path, ensuring the file is read with correct parameters and valid token data is returned as expected.
58-67: LGTM! Proper handling of file not found scenario.The test correctly simulates the ENOENT error case and validates that null is returned when the token file doesn't exist, which is appropriate behavior.
69-116: LGTM! Comprehensive error scenario coverage.These test cases thoroughly cover all failure modes: generic file read errors, invalid token data structures, missing required fields, null data, and non-object data. The consistent null return behavior for all error scenarios is appropriate.
workers/main/src/services/OAuth2/FileTokenStorage.basic.test.ts (7)
1-7: LGTM! Clean import structure and proper type imports.The imports are well-organized, importing necessary testing utilities, error classes, and types. Good separation of concerns with proper relative imports.
8-15: Comprehensive fs module mocking setup.The mock configuration properly covers all the fs.promises methods that the FileTokenStorage class would need (mkdir, writeFile, readFile, unlink). This ensures complete isolation during testing.
22-27: Well-structured mock token data.The mock token data includes all required fields with realistic values. The
expires_attimestamp is properly set to a future time (1 hour from now) which is appropriate for testing.
29-37: Proper mock setup in beforeEach hook.Good practice using
vi.mocked()to get properly typed mock functions and setting up default resolved values. The test instance is created with a clear service name for testing.
39-41: Clean mock cleanup in afterEach.Proper cleanup using
vi.clearAllMocks()ensures test isolation between test cases.
64-75: Effective save method success test.The test properly verifies that both
mkdirandwriteFileare called with appropriate parameters. Usingexpect.any(String)for paths is good practice since the exact paths may be implementation details.
77-97: Thorough error‐handling tests for FileTokenStorage.save
The tests cover bothwriteFileandmkdirfailures, asserting thatsave()rejects with anOAuth2Errorand the expected message. SinceFileTokenStoragecurrently throwsnew OAuth2Error('…')without a custom code (it defaults to"UNKNOWN_OAUTH2_ERROR"), there’s no need to assert onerror.codetoday. If you introduce specific error codes for these failures in the future, please update the tests to verifyerror.codeagainst the new constants.
- Introduced new test cases in `OAuth2TokenManager` to handle scenarios for empty access and refresh tokens, as well as invalid expiry dates, ensuring robust validation of token data. - Added a test in `OAuth2TokenRefreshProvider` to verify that the `refreshToken` method returns a promise, enhancing the reliability of the token refresh functionality. - These tests improve the overall coverage and reliability of the OAuth2 token management implementation by validating critical edge cases and ensuring proper asynchronous behavior.
- Introduced new test cases in `OAuth2TokenManager` to handle edge cases for negative, zero, and very small expiry dates, ensuring the `isTokenValid` method correctly identifies invalid token data. - These tests enhance the robustness of the token validation logic by covering additional scenarios that could lead to incorrect token handling. This update improves the overall reliability of the OAuth2 token management implementation by validating critical edge cases in token expiry handling.
- Introduced new test cases in `OAuth2TokenManager` to verify the handling of token refresh when the token is within the buffer time, ensuring that valid tokens are correctly returned. - Added tests for concurrent token refresh requests to ensure that multiple simultaneous requests are handled properly, returning the expected new access token. - Included tests for handling null and undefined token data in `setTokenDataForTesting`, ensuring that the token manager correctly identifies invalid token states. These updates enhance the reliability and robustness of the OAuth2 token management implementation by validating critical scenarios related to token refresh behavior and data integrity.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
workers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts (1)
37-40: Consider clarifying the async initialization pattern.The
setTimeout(resolve, 0)suggests the OAuth2TokenManager constructor has async initialization. Consider adding a comment explaining why this is needed, or verify if there's a more explicit way to wait for initialization completion.beforeEach(async () => { tokenManager = new OAuth2TokenManager('qbo', 'test-refresh-token'); + // Wait for async initialization to complete await new Promise((resolve) => setTimeout(resolve, 0)); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts(1 hunks)workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#78
File: workers/main/src/services/OAuth2/OAuth2TokenManager.ts:0-0
Timestamp: 2025-08-01T13:15:19.633Z
Learning: In workers/main/src/services/OAuth2/OAuth2TokenManager.ts, the user anatolyshipitz enhanced the OAuth2Error class with error code functionality, including a code property and ERROR_CODES constants. The OAuth2TokenRefreshProvider now throws typed errors with specific codes, and the OAuth2TokenManager uses these codes for conditional error handling instead of checking message text.
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
📚 Learning: in workers/main/src/services/oauth2/oauth2tokenmanager.ts, the user anatolyshipitz enhanced the oaut...
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#78
File: workers/main/src/services/OAuth2/OAuth2TokenManager.ts:0-0
Timestamp: 2025-08-01T13:15:19.633Z
Learning: In workers/main/src/services/OAuth2/OAuth2TokenManager.ts, the user anatolyshipitz enhanced the OAuth2Error class with error code functionality, including a code property and ERROR_CODES constants. The OAuth2TokenRefreshProvider now throws typed errors with specific codes, and the OAuth2TokenManager uses these codes for conditional error handling instead of checking message text.
Applied to files:
workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.tsworkers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts
📚 Learning: user anatolyshipitz prefers to keep code implementations simple during early development stages rath...
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
Applied to files:
workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (8)
workers/main/src/services/OAuth2/OAuth2TokenManager.basic.test.ts (3)
1-32: LGTM! Comprehensive mock setup.The imports and mock configurations are well-structured. The mocks provide realistic return values and proper async/Promise handling for all dependencies.
46-106: LGTM! Comprehensive basic functionality coverage.The tests effectively cover the core OAuth2TokenManager functionality including token validation, refresh token retrieval, and access token retrieval scenarios. The test data structures are realistic and the assertions are appropriate.
163-175: LGTM! Good buffer time edge case coverage.This test verifies that tokens within the buffer time (10 minutes) are still considered valid and don't trigger unnecessary refresh. This is important for performance optimization and avoiding excessive API calls.
workers/main/src/services/OAuth2/OAuth2TokenManager.storage.test.ts (5)
1-32: LGTM! Consistent mock setup.The mock configuration is identical to the basic test file, ensuring consistent testing environment across the test suite.
46-49: LGTM! Proper initialization testing.This test correctly verifies that the OAuth2TokenManager loads tokens from storage during initialization and handles the case where no valid stored tokens exist.
63-73: Good coverage of null value edge cases.Testing null access_token with type assertion is valuable for runtime robustness, ensuring the implementation handles unexpected data types gracefully.
129-150: Excellent concurrency testing!This test is crucial for verifying that multiple simultaneous requests for access tokens don't trigger multiple refresh operations. The test correctly verifies that all concurrent requests resolve to the same refreshed token, preventing race conditions and unnecessary API calls.
152-160: Good edge case coverage for testing utilities.Testing that
setTokenDataForTestinghandles null and undefined values gracefully ensures the testing utility is robust and doesn't introduce false positives in the test suite.
OAuth2TokenManagerfor managing access and refresh tokens, including validation and automatic refreshing.FileTokenStoragefor persistent token storage, allowing tokens to be saved and loaded from a file.IOAuth2TokenManagerandTokenRefreshProviderinterfaces to define the structure for token management and refreshing.OAuth2TokenRefreshProviderto handle token refresh logic using QuickBooks Online API.These changes enhance the application's OAuth2 integration, ensuring robust token management and storage capabilities.