Refactored Hub Unlock

[overheadhunter]

This fixes #3247 by changing the order of GET /api/devices/{deviceId} and GET /api/vaults/{vaultId}/access-token. By first retrieving the device data, we can identify non-registered devices before attempting vault unlock. This also fixes a TODO to immediately unlock after device registration.

Furthermore this PR changes how we distinguish legacy (pre Hub 1.3.0) vs current vault unlock. Instead of relying on HTTP status code 404 returned from GET /api/vaults/{vaultId}/access-token, we will look at the apiLevel returned from GET /api/config.

This is the first step to adjust the workflow as depicted in the following diagram, however it does not yet contain the Legacy Device Migration (separate PR will follow):

• Tested Unlock on Hub 1.3.x
• Tested Device Registration with consecutive Unlock on Hub 1.3.x
• Tested Unlock on Hub 1.2.x
• Tested Device Registration with consecutive Unlock on Hub 1.2.x

Summary by CodeRabbit

• New Features

  • Implemented a new key loading workflow for enhanced security.
  • Added the ability to configure API base URLs for key retrieval.

• Refactor

  • Streamlined the key retrieval process with new methods and updated parameters.
  • Improved the registration success flow by handling window events more effectively.

• Style

  • Updated button order in the user interface for better usability.

• Chores

  • Removed unused legacy access token request method.
  • Cleaned up imports and unused fields to optimize the codebase.

[coderabbitai]

Walkthrough

The recent updates to Cryptomator's UI components reflect a significant shift towards a more user-friendly experience, particularly in the domain of Hub device creation for vault access. By allowing the setup of a Hub device prior to having vault access, the updates aim to streamline the process, removing unnecessary steps and creating a smoother flow, which aligns with the objectives to enhance user experience and eliminate policy limitations.

Changes

File Path	Change Summary
.../ui/keyloading/hub/HubConfig.java	Removed org.jetbrains.annotations.NotNull import, added getApiBaseUrl() method with associated documentation.
.../ui/keyloading/hub/ReceiveKeyController.java	Added import statement for com.google.common.base.Preconditions, added/removed fields and methods, including API base URL handling and key retrieval logic.
.../ui/keyloading/hub/RegisterSuccessController.java	Constructor now takes additional parameters, replaced close with complete method, added windowClosed method.
.../resources/fxml/hub_register_success.fxml	Altered the button order and actions in the UI.
.../resources/i18n/strings.properties	Added message hub.register.occupiedMsg under the "Register Device Legacy" section.

Assessment against linked issues

Objective	Addressed	Explanation
Relax policy to create a Hub device when unlocking a vault (#3247)	✅	The changes in ReceiveKeyController and RegisterSuccessController indicate that the application now supports creating a device before having the key, aligning with the objective.
Enhance user experience by reducing the need for additional interactions after vault permission updates (#3247)	✅	Changes in ReceiveKeyController and RegisterSuccessController suggest a more streamlined process, reducing additional interactions.
Provide a smoother user experience by eliminating the limitation that requires device creation after vault privilege updates (#3247)	✅	The addition of methods like receiveKey and changes in the registration success flow indicate that the device creation can now occur without the previous limitations.

Poem

In the vaults of code, a rabbit did hop,
Refactoring secrets, not a single flop.
Ease for the users, a device setup dance,
With smoother access, we advance! 🎉🐇

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 8bf0acf and 87084e8.

Files selected for processing (4)

• src/main/java/org/cryptomator/ui/keyloading/hub/HubConfig.java (2 hunks)
• src/main/java/org/cryptomator/ui/keyloading/hub/ReceiveKeyController.java (5 hunks)
• src/main/java/org/cryptomator/ui/keyloading/hub/RegisterSuccessController.java (1 hunks)
• src/main/resources/fxml/hub_register_success.fxml (1 hunks)

Additional comments: 14

src/main/java/org/cryptomator/ui/keyloading/hub/HubConfig.java (1)

• 21-26: The new method getApiBaseUrl() is correctly implemented to ensure the URI ends with a slash and handles the presence or absence of apiBaseUrl.

src/main/java/org/cryptomator/ui/keyloading/hub/RegisterSuccessController.java (1)

• 24-29: The constructor changes, the replacement of the close method with complete, and the addition of the windowClosed method are all aligned with the PR objectives and are correctly implemented.

src/main/resources/fxml/hub_register_success.fxml (1)

• 44-46: The changes to the ButtonBar component's button order and the associated button actions in the FXML file are consistent with the updated UI flow.

src/main/java/org/cryptomator/ui/keyloading/hub/ReceiveKeyController.java (11)

• 6-6: The addition of the com.google.common.base.Preconditions import is appropriate for performing state checks within the methods.
• 50-60: The new fields added, such as vaultId, and the removal of vaultBaseUri, align with the changes in the vault ID handling process.
• 84-85: The receiveKey method is correctly implemented to initiate the key retrieval process.
• 91-100: The requestApiConfig method is correctly implemented to request the API configuration, handling the response asynchronously.
• 107-120: The receivedApiConfig method correctly processes the API configuration response and decides the next step based on the apiLevel.
• 125-133: The requestVaultMasterkey method is correctly implemented to request the vault master key, using the new API_BASE field for URI resolution.
• 142-150: The receivedVaultMasterkey method correctly handles the various HTTP status codes and the associated actions.
• 156-164: The requestDeviceData method is correctly implemented to request the user key for the device.
• 173-181: The receivedDeviceData method correctly processes the device data response and requests the vault master key if successful.
• 193-200: The receivedBothEncryptedKeys method is correctly implemented to handle the reception of both encrypted keys and complete the result.
• 291-300: The resolveRelativeToApiBase and extractVaultId methods are correctly implemented to resolve URIs relative to the API base and extract the vault ID.

[infeo]

Apart from an already existing issue, a very clean solution! 👍

[infeo]

Regarding

Tested Device Registration with consecutive Unlock on Hub 1.2.x

currently an exception is thrown due to missing translation key. WIll fix it asap.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 87084e8 and 2e32f39.

Files selected for processing (1)

• src/main/resources/i18n/strings.properties (1 hunks)

Additional comments: 1

src/main/resources/i18n/strings.properties (1)

• 166-166: The addition of the message key hub.register.occupiedMsg with the value "Name already in use" is clear and follows the existing naming conventions and structure of the properties file.

[infeo]

Tested with 1.2.2 the unlock and device registration.

[overheadhunter]

@infeo could you also run another test on both 1.2.x and 1.3.x:

1. download vault template
2. unregister device
3. revoke access from vault
4. add vault template to desktop app
5. attempt unlock

Expected result: 1.3.x lets you register the device but fails to unlock. After granting access again, unlock succeeds immediately.

1.2.x should still require device access grant first.

[infeo]

Expected result: 1.3.x lets you register the device but fails to unlock.

✅ Success.

After granting access again, unlock succeeds immediately.

❌ Failure.

Note that i only "shared" the vault with bob.
If i grant permissions with the buttonin the Hub vault options, it works.

1.2.x should still require device access grant first.

Hub tells me "no access", Cryptomator lets me register my device.

After registering device, the unlock fails. A second attempt fails instantly. After sharing the vault with bob, Cryptomator still says access denied and an "grant permission" is required.

[overheadhunter]

Note that i only "shared" the vault with bob.
If i grant permissions with the buttonin the Hub vault options, it works.

Let's have a call, as I suspect this is the expected behavior. I'd like to better understand why you think this is a failure.

[infeo]

Let's have a call

call made, misunderstandings resolved. The (second) test expectations for 1.3.x was not clear enough/ we might have found a bug in the frontend. This will be documented somewhere else.