Let any vault member grant access, not just owners.

[ylangisc]

Replaces the owner-gated per-vault endpoints with the member-callable global pending list and access-tokens/auto, which also turns one request per vault into a single request per scheduler tick. The Web of Trust now verifies the signature chains against the very candidate keys the access tokens are encrypted for. Fixes #574.

[Copilot]

🟡 Changes recommended

Preserve constructor compatibility and add member-authorization and key-binding integration coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request makes automatic access grants member-callable through global polling and verifies candidate keys via the Web of Trust.

Changes:

• Replaces owner-only per-vault polling with global pending-grant processing.
• Adds authority-based candidate resolution and key-bound verification.
• Updates grant services, scheduler integration, and tests.

File summaries

File	Description
hub/src/test/java/cloud/katta/workflows/GrantAccessServiceImplTest.java	Tests global polling, failure handling, and key behavior.
hub/src/main/java/cloud/katta/workflows/WoTServiceImpl.java	Adds candidate-specific trust verification.
hub/src/main/java/cloud/katta/workflows/WoTService.java	Extends the Web of Trust service contract.
hub/src/main/java/cloud/katta/workflows/GrantAccessServiceImpl.java	Implements global discovery and automatic uploads.
hub/src/main/java/cloud/katta/workflows/GrantAccessService.java	Extends the grant service contract.
hub/src/main/java/cloud/katta/protocols/hub/HubGrantAccessSchedulerService.java	Uses the new batch grant flow.

Review details

Suppressed comments (2)

hub/src/main/java/cloud/katta/workflows/GrantAccessServiceImpl.java:73

• The core role change is only covered with mocked API calls here; the existing integration workflows invoke the grant flow from Alice, who owns the vault. Add an integration scenario where a user who is only a vault member runs this path and successfully grants a pending member, so authorization of the global list and /access-tokens/auto endpoint is verified rather than assumed.

    public void grantAccessToUsersRequiringAccessGrant(final UserKeys userKeys) throws ApiException {
        // Single request for all vaults the current user can re-share, regardless of role
        final Map<String, Set<String>> usersRequiringAccessGrant = vaultResourceApi.apiVaultsUsersRequiringAccessGrantGet(NO_LONG_POLL);
        log.info("{} vaults with users requiring access grant", usersRequiringAccessGrant.size());
        for(final Map.Entry<String, Set<String>> pending : usersRequiringAccessGrant.entrySet()) {

hub/src/test/java/cloud/katta/workflows/GrantAccessServiceImplTest.java:187

• This test only verifies that the grant service passes the authority response into a mocked WoTService; it never executes the new WoTServiceImpl.getTrustLevelsPerUserId(UserKeys, List<UserDto>) implementation. Since WoTServiceImplTest currently covers only the old overload, add a test with a mismatched authority ECDH key and assert that chain verification rejects it, otherwise the key-binding security guarantee can regress unnoticed.

        // The Web of Trust is handed the very users the keys are taken from, so the server cannot substitute a key
        final ArgumentCaptor<java.util.List<UserDto>> verified = ArgumentCaptor.forClass(java.util.List.class);
        verify(wotServiceMock).getTrustLevelsPerUserId(eq(aliceKeys), verified.capture());
        assertEquals(Collections.singletonList(bob.getEcdhPublicKey()),
                verified.getValue().stream().map(UserDto::getEcdhPublicKey).collect(java.util.stream.Collectors.toList()));

• Files reviewed: 6/6 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The member-grant integration test can be nondeterministic because the owner scheduler may consume the pending grant first.

Review details

Suppressed comments (1)

hub/src/test/java/cloud/katta/workflows/AbstractHubMemberGrantTest.java:131

• Because hubSession is logged in before this point, its periodic grant scheduler is active; automatic grants are enabled before Bob is left pending. The owner scheduler can consume Bob's pending grant before this member call, so the test can pass without exercising the non-owner path (or fail nondeterministically). Isolate the owner setup from its scheduler, or otherwise prevent the owner from granting while this assertion runs.

            log.info("S05 admin grants access although admin is only MEMBER of the vault");
            // None of the endpoints of the automatic access grant flow bypasses the vault role for the realm admin role
            new GrantAccessServiceImpl(new VaultResourceApi(adminApiClient), new UsersResourceApi(adminApiClient))
                    .grantAccessToUsersRequiringAccessGrant(adminKeys);

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Lite

[ylangisc]

On the suppressed comment about the owner scheduler racing the member grant in AbstractHubMemberGrantTest (addressed in 067447b):

The premise does not hold for the tests — no scheduler is ever started there.

• HubSession.login() only constructs HubGrantAccessSchedulerService. Cyberducks ThreadPoolSchedulerFeature constructor stores the period and creates a ScheduledThreadPool, but schedules nothing until repeat(...) is called.
• There is no call of Scheduler.repeat(...) or execute(...) anywhere in this repository. The only consumer of HubSession.getFeature(Scheduler.class) is Cyberducks BrowserController, which starts it after a successful mount in the UI.
• AbstractHubTest.setupConnection goes through LoginConnectionService.check(...), i.e. login(), and nothing else.

So the only grants in that test are the two explicit service calls in S03 and S05, and the assertion on the pending list immediately before S05 would fail loudly rather than pass silently if anything else granted.

The reasoning would be right if a scheduler did run, though: with trustThreshold -1 the owner would grant to the pending user and S05 would become a no-op while the final assertions still passed. Two changes in 067447b so this cannot go unnoticed either way:

• The test now asserts that the grantee gets a 403 from GET /vaults/{vaultId}/access-token right before S05, so the access token demonstrably does not exist until the member call creates it.
• AbstractHubTest no longer sets hub.protocol.scheduler.period. The property was only ever read by the scheduler constructor and had no effect, but it reads like the scheduler runs every 30 ms in tests, which is presumably where this finding came from.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Authorization, scheduling, and Web-of-Trust behavior changed across multiple workflows and requires human review.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🟡 Changes recommended

The negative trust-threshold path bypasses candidate Web of Trust verification before encryption, leaving key substitution unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Verify candidate keys even when trust threshold is negative · New

[chenkins]

Verify access is granted from member to owner as well (only memberKey but w/o failure), parameterized if possible. How is owner then also granted recoveryKey?

[chenkins]

Relates to cryptomator/hub#505.