feat: verify JWTs before accessing and storing #46
Conversation
This file contains 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
# Conflicts: # lib/__tests__/sdk/oauth2-flows/ClientCredentials.spec.ts # lib/sdk/clients/browser/authcode-with-pkce.ts # lib/sdk/clients/server/with-auth-utilities.ts # lib/sdk/utilities/token-claims.ts # lib/sdk/utilities/token-utils.ts
WalkthroughThe updates focus on enhancing token handling and validation across various OAuth2 flows and utility functions within the SDK. Key changes include the introduction of the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/*.yaml
Files selected for processing (21)
- lib/tests/mocks.ts (4 hunks)
- lib/tests/sdk/oauth2-flows/AuthCodeWithPKCE.spec.ts (10 hunks)
- lib/tests/sdk/oauth2-flows/AuthorizationCode.spec.ts (14 hunks)
- lib/tests/sdk/oauth2-flows/ClientCredentials.spec.ts (7 hunks)
- lib/tests/sdk/utilities/feature-flags.spec.ts (6 hunks)
- lib/tests/sdk/utilities/token-claims.spec.ts (5 hunks)
- lib/tests/sdk/utilities/token-utils.spec.ts (4 hunks)
- lib/sdk/clients/browser/authcode-with-pkce.ts (11 hunks)
- lib/sdk/clients/server/authorization-code.ts (2 hunks)
- lib/sdk/clients/server/client-credentials.ts (1 hunks)
- lib/sdk/clients/server/with-auth-utilities.ts (11 hunks)
- lib/sdk/oauth2-flows/AuthCodeAbstract.ts (5 hunks)
- lib/sdk/oauth2-flows/AuthCodeWithPKCE.ts (1 hunks)
- lib/sdk/oauth2-flows/AuthorizationCode.ts (1 hunks)
- lib/sdk/oauth2-flows/ClientCredentials.ts (4 hunks)
- lib/sdk/oauth2-flows/types.ts (2 hunks)
- lib/sdk/utilities/feature-flags.ts (5 hunks)
- lib/sdk/utilities/remote-jwks-cache.ts (1 hunks)
- lib/sdk/utilities/token-claims.ts (7 hunks)
- lib/sdk/utilities/token-utils.ts (3 hunks)
- lib/sdk/utilities/types.ts (2 hunks)
Additional comments (58)
lib/sdk/utilities/remote-jwks-cache.ts (2)
- 10-12: Consider adding error handling for the
createRemoteJWKSetcall. Network or parsing errors could occur when fetching the JWKS, and handling these gracefully can improve the robustness of your JWT verification process.- 11-11: The cache expiration is set to 24 hours (
cacheMaxAge: 1000 * 60 * 60 * 24). Ensure this duration aligns with the expected update frequency of the JWKS from the domain. If the keys are rotated more frequently, consider reducing the cache duration.lib/sdk/utilities/types.ts (1)
- 46-50: The
TokenValidationDetailsTypeinterface is well-defined for JWT validation purposes. However, consider documenting theissuer,audience, andkeyProviderproperties to clarify their roles and expected values, enhancing maintainability and readability.lib/sdk/oauth2-flows/types.ts (1)
- 39-39: Adding the
jwksfield toOAuth2FlowOptionsis a good approach for directly providing JSON Web Key Sets for JWT verification. Ensure that the consumers of this interface are aware of the precedence rules when bothjwksand remote JWKS URLs (viaauthDomain) are available.lib/sdk/clients/server/client-credentials.ts (1)
- 43-43: Including
client.tokenValidationDetailsin thewithAuthUtilitiesfunction call is a crucial step for integrating JWT validation into the authentication utilities. Ensure that thewithAuthUtilitiesfunction is properly updated to handle this new argument and perform the necessary validation logic.Verification successful
Including
client.tokenValidationDetailsin thewithAuthUtilitiesfunction call aligns with the function's definition and expected usage for integrating JWT validation into the authentication utilities. The function is properly updated to handle this new argument and perform the necessary validation logic.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of `withAuthUtilities` to ensure it properly handles the new argument. rg --type ts "function withAuthUtilities" -A 10Length of output: 49
Script:
#!/bin/bash # Broaden the search to find any reference to `withAuthUtilities`, including its definition or usage, across all files. rg "withAuthUtilities" -A 3 -B 3Length of output: 2704
lib/sdk/utilities/feature-flags.ts (1)
- 24-34: The addition of the
validationDetailsparameter to thegetFlagfunction and its usage in token validation is a significant improvement for security. Ensure that all calls togetFlagand related functions are updated to include this parameter.Verification successful
The verification process confirms that all calls to
getFlagand related functions (getIntegerFlag,getStringFlag,getBooleanFlag) across the codebase correctly include thevalidationDetailsparameter. This aligns with the enhancement for security through enhanced token validation during flag retrieval, as mentioned in the review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to `getFlag` and related functions to ensure they include the `validationDetails` parameter. rg --type ts "getFlag\(" -A 2 rg --type ts "getIntegerFlag\(" -A 2 rg --type ts "getStringFlag\(" -A 2 rg --type ts "getBooleanFlag\(" -A 2Length of output: 4182
lib/__tests__/sdk/utilities/feature-flags.spec.ts (1)
- 39-39: The update to include
validationDetailsin the test cases forgetFlagis necessary for ensuring the new validation logic works as expected. Consider adding more test cases to cover scenarios where the validation fails due to incorrectvalidationDetails.lib/__tests__/mocks.ts (1)
- 9-23: The introduction of
getKeysfor managing key pairs and its use in JWT signing for test tokens is a solid approach for simulating real-world JWT creation and verification. Ensure that the keys are securely managed and not exposed in any public or insecure contexts.lib/__tests__/sdk/utilities/token-claims.spec.ts (1)
- 42-47: Updating the test cases to include
validationDetailsingetClaimValuecalls is crucial for testing the enhanced validation logic. Consider adding negative test cases where the validation fails due to incorrectvalidationDetailsto ensure robust error handling.lib/sdk/utilities/token-claims.ts (6)
- 15-22: The implementation of
jwtVerifyfor token validation ingetClaimValueis a significant security enhancement. It ensures that the token's integrity and authenticity are verified before any claims are extracted. This change aligns with best practices for handling JWTs securely.- 37-42: The modification of
getClaimto includevalidationDetailsfor token validation is correctly implemented. This ensures that the token is validated before any claim is extracted, enhancing the security of the operation.- 56-71: The update to
getPermissionto incorporatevalidationDetailsfor token validation is a positive change. It ensures that permissions are only extracted from tokens that have been verified, which is crucial for maintaining the integrity of permission checks.- 81-89: The addition of
validationDetailsingetOrganizationfor token validation before extracting the organization code is a good practice. This ensures that the organization code is only retrieved from a verified token, enhancing the security of the operation.- 99-114: The implementation of
validationDetailsingetPermissionsfor token validation is correctly done. It ensures that permissions and organization codes are extracted from verified tokens, aligning with secure handling of JWTs.- 129-137: The update to
getUserOrganizationsto includevalidationDetailsfor token validation before extracting organization codes is a secure practice. This ensures that the data is only retrieved from tokens that have been authenticated and verified.lib/sdk/utilities/token-utils.ts (4)
- 17-37: The renaming of
commitTokenToMemorytocommitTokenToSessionand the addition ofvalidationDetailsfor token validation are significant improvements. This change not only clarifies the function's purpose but also enhances security by ensuring tokens are validated before being stored.- 49-72: Renaming
commitTokensToMemorytocommitTokensToSessionand includingvalidationDetailsfor token validation incommitTokensToSessionare well-implemented changes. These modifications improve the clarity and security of the token handling process.- 110-129: The update to
getUserFromSession, including the addition ofvalidationDetailsfor token validation, is a crucial security enhancement. It ensures that user information is only extracted from verified ID tokens, aligning with secure JWT handling practices.- 137-152: Changing
isTokenExpiredto an async function and addingvalidationDetailsfor token validation are appropriate updates. These changes ensure that the token's expiration is checked only after its validity has been confirmed, enhancing the security of the operation.lib/sdk/oauth2-flows/AuthorizationCode.ts (1)
- 76-80: Replacing
commitTokensToMemorywithcommitTokensToSessionand passingtokenValidationDetailsin theAuthorizationCodeclass is a positive change. It enhances the security by ensuring tokens are validated before being stored and improves the clarity of where tokens are being committed.lib/__tests__/sdk/utilities/token-utils.spec.ts (1)
- 5-9: Updating the test file to reflect the changes in
token-utils.ts, including the renaming of functions and the addition ofvalidationDetailsfor token validation, ensures that the tests remain relevant and effective. It's important to keep tests aligned with the implementation to ensure comprehensive coverage.lib/sdk/oauth2-flows/ClientCredentials.ts (2)
- 17-41: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-38]
The addition of
createLocalJWKSetfrom 'jose' and the introduction oftokenValidationDetailsproperty in theClientCredentialsclass are significant improvements. These changes enhance the security by ensuring that tokens are validated using the correct keys and validation details.
- 50-63: Modifying the
getTokenmethod to usetokenValidationDetailsfor token validation in theClientCredentialsclass is a crucial update. It ensures that the access token is only used if it has been validated, aligning with secure JWT handling practices.lib/sdk/clients/server/authorization-code.ts (2)
- 125-128: Replacing
getUserFromMemorywithgetUserFromSessionand includingclient.tokenValidationDetailsin thecreateAuthorizationCodeClientfunction are well-implemented changes. These modifications enhance the security by ensuring user information is only extracted from validated tokens and improve the clarity of token handling.- 165-165: Including
client.tokenValidationDetailsinwithAuthUtilitieswithin thecreateAuthorizationCodeClientfunction ensures that token validation details are consistently used across utility functions. This is a good practice for maintaining security and consistency in token handling.lib/sdk/oauth2-flows/AuthCodeWithPKCE.ts (1)
- 85-89: Replacing
commitTokensToMemorywithcommitTokensToSessionand passingtokenValidationDetailsin theAuthCodeWithPKCEclass is a positive change. It enhances the security by ensuring tokens are validated before being stored and improves the clarity of where tokens are being committed.lib/__tests__/sdk/oauth2-flows/ClientCredentials.spec.ts (1)
- 1-8: Updating the test file to reflect the changes in
ClientCredentials.ts, including the import of new functions from 'jose', the introduction ofTokenValidationDetailsType, and modifications to token handling functions, ensures that the tests remain relevant and effective. It's important to keep tests aligned with the implementation to ensure comprehensive coverage.lib/sdk/clients/server/with-auth-utilities.ts (11)
- 11-12: The addition of
validationDetailsas a parameter towithAuthUtilitiesis a good practice for enhancing token validation. This change ensures that token validation details are consistently applied across various utility functions, improving security.- 34-39: Passing
validationDetailstofeatureFlags.getIntegerFlagensures that token validation is performed using the provided details. This is a crucial step for maintaining the integrity and security of feature flag retrieval based on the access token.- 60-65: Similar to
getIntegerFlag, passingvalidationDetailstofeatureFlags.getStringFlagis essential for secure feature flag retrieval. This ensures that the string flag value is extracted from a validated token.- 86-91: The use of
validationDetailsinfeatureFlags.getBooleanFlagis consistent with the approach taken in other flag retrieval methods. This consistency in applying token validation enhances the security posture of the library.- 112-118: Incorporating
validationDetailsintotokenClaims.getClaimValueis a prudent measure for ensuring that claims are extracted from tokens that have been validated. This approach reinforces the security of claim retrieval operations.- 139-144: The application of
validationDetailsintokenClaims.getClaimaligns with the overall strategy of enhancing token security. This ensures that the claims are securely retrieved from validated tokens.- 164-164: Utilizing
validationDetailsintokenClaims.getPermissionis a key step in ensuring that permissions are checked against a validated token. This is crucial for maintaining the security and integrity of permission checks.- 180-180: The use of
validationDetailsintokenClaims.getOrganizationensures that the organization code is retrieved from a token that has undergone validation. This practice enhances the security of organization-related operations.- 197-197: Passing
validationDetailstotokenClaims.getUserOrganizationsis consistent with the secure handling of tokens across the library. This ensures that organization codes are extracted from validated tokens.- 217-217: Incorporating
validationDetailsintotokenClaims.getPermissionsis a critical measure for ensuring that permissions are retrieved from a validated token. This approach is essential for maintaining the security of permission retrieval.- 240-246: The application of
validationDetailsinfeatureFlags.getFlagis a prudent step towards ensuring that feature flags are retrieved from tokens that have been validated. This consistency in validation enhances the library's security.lib/sdk/oauth2-flows/AuthCodeAbstract.ts (3)
- 31-50: The introduction of
tokenValidationDetailsin the constructor and its initialization based on the configuration is a significant enhancement. This ensures that token validation details are readily available for use in token-related operations, enhancing security.- 110-114: Including
tokenValidationDetailswhen committing tokens to the session inhandleRedirectFromAuthDomainis crucial for ensuring that tokens are stored with validation details. This is a key security measure.- 131-134: The use of
tokenValidationDetailsinisAccessTokenExpiredto check token expiration is a good practice. It ensures that the expiration check is performed against a token that has been validated, enhancing security.lib/sdk/clients/browser/authcode-with-pkce.ts (11)
- 104-107: Including
client.tokenValidationDetailsinutilities.getUserFromSessionis a critical security measure. It ensures that user details are retrieved from a session that has been validated, enhancing the security of user data retrieval.- 126-131: Passing
client.tokenValidationDetailstofeatureFlags.getIntegerFlagfor retrieving integer feature flags ensures that the flags are extracted from a validated token. This is a good practice for maintaining the integrity and security of feature flag retrieval.- 150-155: Similar to integer flags, using
client.tokenValidationDetailsinfeatureFlags.getStringFlagfor string feature flags is essential for secure feature flag retrieval. This ensures that the flags are extracted from a validated token.- 174-179: The use of
client.tokenValidationDetailsinfeatureFlags.getBooleanFlagfor boolean feature flags is consistent with the approach taken for other flag types. This consistency in applying token validation enhances the security posture of the library.- 198-204: Incorporating
client.tokenValidationDetailsintotokenClaims.getClaimValueis a prudent measure for ensuring that claims are extracted from tokens that have been validated. This approach reinforces the security of claim retrieval operations.- 223-228: The application of
client.tokenValidationDetailsintokenClaims.getClaimaligns with the overall strategy of enhancing token security. This ensures that the claims are securely retrieved from validated tokens.- 246-250: Utilizing
client.tokenValidationDetailsintokenClaims.getPermissionis a key step in ensuring that permissions are checked against a validated token. This is crucial for maintaining the security and integrity of permission checks.- 263-266: The use of
client.tokenValidationDetailsintokenClaims.getOrganizationensures that the organization code is retrieved from a token that has undergone validation. This practice enhances the security of organization-related operations.- 280-283: Passing
client.tokenValidationDetailstotokenClaims.getUserOrganizationsis consistent with the secure handling of tokens across the library. This ensures that organization codes are extracted from validated tokens.- 300-303: Incorporating
client.tokenValidationDetailsintotokenClaims.getPermissionsis a critical measure for ensuring that permissions are retrieved from a validated token. This approach is essential for maintaining the security of permission retrieval.- 342-348: The application of
client.tokenValidationDetailsinfeatureFlags.getFlagis a prudent step towards ensuring that feature flags are retrieved from tokens that have been validated. This consistency in validation enhances the library's security.lib/__tests__/sdk/oauth2-flows/AuthCodeWithPKCE.spec.ts (3)
- 19-22: Adding a
beforeAllhook to set upjwksinclientConfigis a good practice for ensuring that the tests have the necessary setup for JWT verification. This enhances the reliability of the tests.- 127-130: Changing calls to
mocks.getMockAccessTokenandmocks.getMockIdTokento be awaited ensures that the tests correctly handle asynchronous operations. This is crucial for accurately testing token handling logic.- 169-171: The update to token handling logic to use
awaitfor token retrieval is a necessary change for correctly handling asynchronous operations in tests. This ensures that the tests accurately reflect the asynchronous nature of token handling in the SDK.lib/__tests__/sdk/oauth2-flows/AuthorizationCode.spec.ts (3)
- 24-27: The addition of a
beforeAllblock to setjwksinclientConfigis a positive change for ensuring that JWT verification has the necessary setup in tests. This preparation step enhances the accuracy and reliability of the tests.- 172-175: Changing calls to
mocks.getMockAccessTokenandmocks.getMockIdTokento be awaited is essential for handling asynchronous operations correctly in tests. This adjustment ensures that the tests accurately simulate the asynchronous behavior of token retrieval and handling.- 210-212: Updating token handling logic to use
awaitfor token retrieval in tests is a necessary improvement for accurately testing asynchronous operations. This change ensures that the tests reflect the true behavior of the SDK when handling tokens.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Explain your changes
Verify the JWTs to ensure they haven't been tampered with. Also deprecated storing user data in session storage as it can be derived from the id token.
Tested the three auth flows. Tested browser and server. JWKS are cached for 24 hours, or can be provided in config.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.
Summary by CodeRabbit