Add new API for Legacy Device Migration #252
Conversation
`{"vault1": "jwe1", "vault2": "jwe2"}` instead of `[{"vaultId": "vault1", "token": "jwe1"}, ...]`
|
Warning Rate Limit Exceeded@overheadhunter has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 16 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates involve enhancing the management of legacy access tokens and user-specific vault key handling. Key changes include the introduction of methods to list and update legacy tokens, the addition of new imports to support these functionalities, and the adjustment of methods to include user authentication. Test cases have been added and updated to ensure the correct behavior of the new and modified endpoints. Changes
Poem
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 selected for processing (5)
- backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java (4 hunks)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (4 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/LegacyAccessToken.java (3 hunks)
- backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java (1 hunks)
- backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java (2 hunks)
Additional comments: 14
backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java (2)
- 8-8: Removed import for
org.hamcrest.CoreMatchersand addedio.restassured.http.ContentType. This change is consistent with the rest of the file whereContentTypeis used.- 73-83: The new test method
testPostAccessTokenscorrectly tests the POST request for access tokens. It uses a JSON body with UUIDs as keys and strings as values, which aligns with the PR's objective to handle user-specific vault keys.backend/src/main/java/org/cryptomator/hub/entities/LegacyAccessToken.java (2)
- 26-32: The
@NamedNativeQueryannotation forLegacyAccessToken.getByDevicehas been modified toLegacyAccessToken.getByDeviceAndOwner. This change is consistent with the PR's objective to clarify method names and align with the new functionality.- 54-58: The method
getByDeviceAndOwnerreplaces the oldgetByDevicemethod, reflecting the change in the@NamedNativeQueryname. This change is correct and improves clarity by specifying that the tokens are fetched by both device and owner.backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (4)
- 8-8: Added import for
jakarta.validation.constraints.NotEmpty. This is necessary for the newupdateMyAccessTokensmethod which requires a non-empty map as input.- 19-19: Added import for
org.cryptomator.hub.entities.AuditEventVaultAccessGrant. This is used in the newupdateMyAccessTokensmethod to log an audit event after updating access tokens.- 22-22: Added import for
org.cryptomator.hub.entities.Vault. This is used in the newupdateMyAccessTokensmethod to find vaults by their ID.- 70-94: The new method
updateMyAccessTokenscorrectly handles the updating of user-specific vault keys. It takes a map of UUIDs to strings as input, checks for non-archived vaults, and updates or creates access tokens accordingly. It also logs an audit event for each access token update, which is a good security practice.backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java (4)
- 26-26: Added import for
org.cryptomator.hub.entities.LegacyAccessToken. This is necessary for the newgetLegacyAccessTokensmethod which retrieves legacy access tokens.- 44-46: Added imports for
java.util.Map,java.util.UUID, andjava.util.stream.Collectors. These are used in the newgetLegacyAccessTokensmethod to collect and return the tokens in a map.- 123-135: The new method
getLegacyAccessTokensis correctly annotated as deprecated, which aligns with the PR's objective to support legacy devices temporarily. It retrieves legacy access tokens for a specific device and formats them into a map, which is consistent with the PR's description.- 175-175: The new record
LegacyAccessTokenDtois added but not used within the provided code. It's unclear if this is an oversight or if it's intended for future use. However, since it's not causing any issues, it's acceptable.backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java (2)
- 81-87: The test method
testGetLegacyAccessTokens1correctly tests the retrieval of legacy access tokens for a specific device. It asserts the status code and the body content, ensuring the legacy tokens are returned as expected.- 90-96: The test method
testGetLegacyAccessTokens2performs a similar check astestGetLegacyAccessTokens1for a different device. It also asserts the status code and the body content correctly.
backend/src/main/java/org/cryptomator/hub/entities/LegacyAccessToken.java
Show resolved
Hide resolved
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (4 hunks)
- backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java
- backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java
This adds two new API endpoints:
GET /api/devices/{deviceId}/legacy-access-tokens(to be removed after all users migrated from 1.2.x)POST /api/users/me/access-tokensThe response body of the former and the request body of the latter are both JSON objects with these key-value-pairs:
{ "vault1": "token1", "vault2": "token2", ... }The difference is that the deprecated legacy access tokens are encrypted using the device key, while the current access tokens posted to the users resource shall be encrypted using the user key.
Both endpoints are accessible to all users, as they only expose information owned by the currently logged in user.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests