Unlock Hub vaults in write mode via the Hub-iOS-License JWT#466
Unlock Hub vaults in write mode via the Hub-iOS-License JWT#466tobihagemann wants to merge 1 commit into
Conversation
WalkthroughThis PR introduces iOS license token verification for Hub authentication. It adds a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/HubLicenseVerifier.swift`:
- Around line 150-156: The mock's verify(token:) exits early when
verifyTokenThrowableError is set, so increment verifyTokenCallsCount and set
verifyTokenReceivedToken before throwing; update the method in
HubLicenseVerifier.verify(token:) to first increment verifyTokenCallsCount and
assign verifyTokenReceivedToken = token, then if verifyTokenThrowableError is
non-nil throw it, otherwise proceed to call verifyTokenClosure or return
verifyTokenReturnValue (keeping references to verifyTokenClosure and
verifyTokenReturnValue as currently used).
In
`@CryptomatorCommon/Tests/CryptomatorCommonCoreTests/Hub/HubAuthenticationViewModelTests.swift`:
- Around line 234-240: The test is setting hubLicenseVerifier only inside
withDependencies but HubAuthenticationViewModel (which captures
`@Dependency`(\.hubLicenseVerifier) at init) is created earlier in setUpWithError,
so resolveSubscriptionState(from:) may call the default verifier; fix by
instantiating a new HubAuthenticationViewModel inside the same withDependencies
block (or provide a factory helper that constructs HubAuthenticationViewModel
using current dependencies) immediately after overriding $0.hubLicenseVerifier
and before calling continueToAccessCheck(); apply the same pattern for the other
occurrences mentioned (lines around 266-272, 298-304, 329-335).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0097921-37ef-4d43-911b-147594a4ee17
📒 Files selected for processing (5)
CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/HubAuthenticationViewModel.swiftCryptomatorCommon/Sources/CryptomatorCommonCore/Hub/HubLicenseVerifier.swiftCryptomatorCommon/Tests/CryptomatorCommonCoreTests/Hub/HubAuthenticationViewModelTests.swiftCryptomatorCommon/Tests/CryptomatorCommonCoreTests/Hub/HubLicenseVerifierTests.swiftSharedResources/en.lproj/Localizable.strings
| func verify(token: String) throws -> HubLicenseVerificationResult { | ||
| if let error = verifyTokenThrowableError { | ||
| throw error | ||
| } | ||
| verifyTokenCallsCount += 1 | ||
| verifyTokenReceivedToken = token | ||
| return try verifyTokenClosure.map({ try $0(token) }) ?? verifyTokenReturnValue |
There was a problem hiding this comment.
Record mock calls before throwing.
When verifyTokenThrowableError is set, Line 151 exits before updating verifyTokenCallsCount and verifyTokenReceivedToken, so failure-path tests cannot assert that the verifier was actually consulted.
Suggested fix
func verify(token: String) throws -> HubLicenseVerificationResult {
+ verifyTokenCallsCount += 1
+ verifyTokenReceivedToken = token
if let error = verifyTokenThrowableError {
throw error
}
- verifyTokenCallsCount += 1
- verifyTokenReceivedToken = token
return try verifyTokenClosure.map({ try $0(token) }) ?? verifyTokenReturnValue
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/HubLicenseVerifier.swift`
around lines 150 - 156, The mock's verify(token:) exits early when
verifyTokenThrowableError is set, so increment verifyTokenCallsCount and set
verifyTokenReceivedToken before throwing; update the method in
HubLicenseVerifier.verify(token:) to first increment verifyTokenCallsCount and
assign verifyTokenReceivedToken = token, then if verifyTokenThrowableError is
non-nil throw it, otherwise proceed to call verifyTokenClosure or return
verifyTokenReturnValue (keeping references to verifyTokenClosure and
verifyTokenReturnValue as currently used).
| await withDependencies({ | ||
| $0.hubKeyService = hubKeyServiceMock | ||
| $0.cryptomatorHubKeyProvider = hubKeyProviderMock | ||
| $0.hubLicenseVerifier = licenseVerifierMock | ||
| }, operation: { | ||
| await self.viewModel.continueToAccessCheck() | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="CryptomatorCommon/Tests/CryptomatorCommonCoreTests/Hub/HubAuthenticationViewModelTests.swift"
# Show setUpWithError and where viewModel is initialized
rg -n "setUpWithError|viewModel\s*=" "$FILE"
echo "---- setUpWithError context ----"
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'
# Show the region around the reported lines 200-360
echo "---- region 200-360 ----"
sed -n '180,370p' "$FILE" | nl -ba
# Also locate HubAuthenticationViewModel init usage (if referenced in file)
rg -n "HubAuthenticationViewModel|continueToAccessCheck\(" "$FILE"Repository: cryptomator/ios
Length of output: 259
🌐 Web query:
swift-dependencies @dependency capture active dependency snapshot when enclosing instance is initialized timing
💡 Result:
In the swift-dependencies library, the @Dependency property wrapper captures an active snapshot of the dependency container at the moment the enclosing instance is initialized [1][2]. When an object containing an @Dependency property is initialized, the property wrapper records the current state of DependencyValues._current [1]. This snapshot is subsequently merged with the current task-local values whenever the dependency is read [1]. This behavior is designed to provide a scoping mechanism similar to how @TaskLocal values function in Swift, ensuring that dependencies reflect the context at the time of the object's creation [2]. However, because this snapshot occurs at initialization, there are specific implications for dependency propagation: 1. Lifecycle Caveats: Because dependencies are held in a @TaskLocal, they propagate automatically through structured concurrency (such as within Tasks) [2][3]. However, if dependencies are needed across escaping closures—such as in callbacks or certain Combine operators—they may not automatically reflect updated values unless you use tools like withEscapedDependencies to bridge the context [2]. 2. Static vs. Dynamic Value Resolution: A dependency key's value is typically cached on the first access [1]. While this is generally intended to prevent re-computation, it means that if a dependency is implemented using a computed property rather than a static definition, it will only be resolved once [1][4]. 3. Scoping: The snapshotting mechanism effectively "pins" the dependency context to the instance, which is useful for maintaining consistency within an object's lifecycle but requires careful handling if you intend for dependencies to be dynamic or to change in response to broader context updates after the object has already been initialized [2]. For most use cases, especially in architectures like The Composable Architecture, this initialization-time capture ensures that objects are correctly wired with the expected dependencies from their creation context [5][6]. If you encounter issues where dependencies are not updating as expected, it is often because the dependencies have been "captured" in an initialization context that does not include the desired overrides, or because they are crossing boundaries where task-local propagation does not naturally reach [2][7].
Citations:
- 1: https://adamz.dev/blog/swift-dependencies-warning/
- 2: https://github.com/pointfreeco/swift-dependencies/blob/main/Sources/Dependencies/Documentation.docc/Articles/Lifetimes.md
- 3: https://github.com/pointfreeco/swift-dependencies/blob/2ee34c93c717700eb58ab945fd0444cebe8a646f/Sources/Dependencies/DependencyValues.swift
- 4: Dependency injection strategy in a multi-layered application pointfreeco/swift-dependencies#182
- 5: https://github.com/pointfreeco/swift-dependencies/
- 6: https://pointfreeco-swift-composable-architecture.mintlify.app/api/dependency
- 7: Dependencies not set within async tests pointfreeco/swift-dependencies#55
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="CryptomatorCommon/Tests/CryptomatorCommonCoreTests/Hub/HubAuthenticationViewModelTests.swift"
echo "---- setUpWithError (lines ~1-80) ----"
sed -n '1,90p' "$FILE"
echo "---- region around reported lines 210-360 ----"
sed -n '200,360p' "$FILE" | cat -n
echo "---- find any withDependencies blocks ----"
rg -n "withDependencies\s*\(" "$FILE"Repository: cryptomator/ios
Length of output: 13314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate HubAuthenticationViewModel implementation
rg -n "struct HubAuthenticationViewModel|final class HubAuthenticationViewModel|class HubAuthenticationViewModel|HubAuthenticationViewModel\(" -S CryptomatorCommon CryptomatorCommonCore 2>/dev/null || true
fd -a -t f "HubAuthenticationViewModel.swift" . | head -n 20Repository: cryptomator/ios
Length of output: 970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for hubLicenseVerifier dependency key usage
rg -n "hubLicenseVerifier|HubLicenseVerifier|LicenseVerifying|`@Dependency`" -S CryptomatorCommon CryptomatorCommonCore 2>/dev/null || trueRepository: cryptomator/ios
Length of output: 8577
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Open the view model file around init and continueToAccessCheck
FILE="$(fd -a -t f "HubAuthenticationViewModel.swift" . | head -n 1)"
if [ -z "$FILE" ]; then
echo "HubAuthenticationViewModel.swift not found"
exit 0
fi
echo "VIEW MODEL FILE: $FILE"
rg -n "init\\(|continueToAccessCheck\\(" "$FILE"
# Print around init and continueToAccessCheck
sed -n '1,220p' "$FILE"Repository: cryptomator/ios
Length of output: 5977
🌐 Web query:
swift-dependencies Lifetimes withDependencies merges task-local values whenever dependency read caching on first access
💡 Result:
Short answer: No—withDependencies does not “merge task-local values whenever a dependency read is cached on first access.” Instead, swift-dependencies stores the current dependency set in a @TaskLocal, so changes to dependencies are scoped to the task-local context created by withDependencies and are inherited by child tasks created within that scope. However, individual dependency keys are resolved/cached on first access, so if a dependency’s value (often via a computed liveValue) is first resolved before a later withDependencies override, that cached value will not be recomputed to reflect the later override. Evidence: 1) Dependencies are task-local and follow task-local inheritance rules. The lifetimes docs state that “Under the hood, dependencies are held as a @TaskLocal” and that many task-local rules apply (e.g., inherited by tasks, not generally across escaping boundaries).[1] 2) withDependencies scopes dependency changes to an operation, implemented via task-local storage. The withDependencies implementation merges new dependency values with the current dependency values and runs the provided operation in that scope.[2] 3) Dependency values are cached on first resolution, which can prevent later overrides from being observed. In a discussion of an observed behavior, the maintainer explains it “is to be expected” because “Parent.liveValue is resolved from cache upon its first resolution … [so] the parent will not be recalculated capturing its child’s initial value.”[3] Mapping to your specific phrasing: - What is correct: withDependencies uses task-local dependency state, so when dependency reads happen while the task-local scope is active, they see the overridden values.[1][2] - What is misleading: the library’s cache-on-first-access means that once a specific dependency key has been resolved and cached, subsequent reads may return the cached instance/value rather than re-merging the latest task-local overrides for the dependencies that were used during that first resolution.[3] If you paste the exact code snippet you’re referring to (e.g., parent DependencyKey.liveValue reading @Dependency(\.child) inside liveValue, or an escaping closure / Task creation), I can pinpoint the precise “when snapshot is taken” vs “when cached first access happens” in that scenario.
Citations:
- 1: https://github.com/pointfreeco/swift-dependencies/blob/main/Sources/Dependencies/Documentation.docc/Articles/Lifetimes.md
- 2: https://github.com/pointfreeco/swift-dependencies/blob/main/Sources/Dependencies/WithDependencies.swift
- 3: Parent dependency is resolved from cache after child has changed, instead of being recalculated pointfreeco/swift-dependencies#238
🌐 Web query:
pointfree swift-dependencies @dependency captures dependency snapshot at initialization Lifetimes.md exact wording
💡 Result:
The official documentation for swift-dependencies, specifically in the Lifetimes.md article, states: "When the @Dependency property wrapper is initialized it captures the current state of the dependency at that moment." [1] This behavior provides a scoping mechanism similar to @TaskLocal values, which are inherited by new asynchronous tasks, but with specific caveats regarding their propagation [1]. Because dependencies are managed under the hood as a @TaskLocal, they typically propagate within structured concurrency, but they do not automatically persist across escaping boundaries (such as callbacks or certain Combine operators) without explicit handling via tools like withEscapedDependencies [1].
Citations:
🌐 Web query:
site:github.com pointfreeco swift-dependencies @dependency captures snapshot when instance is initialized
💡 Result:
When an @Dependency property wrapper is initialized as a property of a class or struct, it captures the current set of dependency values at the moment the parent instance is initialized [1]. This behavior is an intentional design choice of the library rather than a general property of all Swift property wrappers [1]. By capturing the dependency values during the parent's initialization, the library ensures that the resolved dependency instance is stored and reused for the lifetime of that parent object [2]. Key points regarding this behavior: 1. Initialization Timing: When a model instance is initialized, all its @Dependency property wrappers are initialized [1]. These wrappers resolve their values from the current global or scoped dependency environment at that specific time [1]. 2. Persistence: Because these values are captured and stored upon the initialization of the parent, accessing the dependency later within the instance will continue to use the initially resolved value, rather than re-resolving it from the current global dependency environment [2]. 3. Comparison with Local Access: This differs from declaring an @Dependency property locally inside a method or function [2]. When declared as a local variable, the dependency is resolved every time the function is executed, reflecting the dependency environment at the time of that function call rather than the time of the parent's initialization [2]. 4. Practical Implications: While this behavior provides stable dependencies for a model, it can lead to situations where a dependency (such as a nested dependency or a singleton) is "cached" in its initial state [3][4]. If the underlying dependency is intended to be dynamic or overridable later, developers should be aware that the captured instance will not automatically update if the environment changes after the parent has already been initialized [3]. This mechanism is why tools like withDependencies(from:) exist—they allow you to extract the dependency context captured by an existing object and forward it to subsequent operations [5][1].
Citations:
- 1: When should `withDependencies(from: self)` be used? pointfreeco/swift-dependencies#49
- 2: Difference between declaring @Dependency inside func and inside class/struct? pointfreeco/swift-dependencies#410
- 3: Parent dependency is resolved from cache after child has changed, instead of being recalculated pointfreeco/swift-dependencies#238
- 4: Dependency injection strategy in a multi-layered application pointfreeco/swift-dependencies#182
- 5: https://github.com/pointfreeco/swift-dependencies/blob/main/Sources/Dependencies/WithDependencies.swift
🌐 Web query:
adamz blog swift-dependencies warning @dependency captures active snapshot at moment of initialization wording
💡 Result:
The @Dependency property wrapper in the swift-dependencies library initializes by capturing a snapshot of the current dependency values at the moment the enclosing instance is initialized [1][2]. This behavior is a core design choice intended to provide a scoping mechanism similar to @TaskLocal values, ensuring that dependencies are inherited within structured concurrency [2]. However, this capture-at-initialization mechanism can lead to issues in specific scenarios: 1. Escaping Closures: Because dependencies are held in Task-local storage, they do not automatically propagate across escaping boundaries (e.g., in completion handlers, Combine operators, or certain Dispatch queue callbacks) [2]. When a dependency is captured inside an escaping closure, it may "freeze" to the value present at the time the closure was constructed, or fail to reflect runtime overrides [3]. To address this, the library provides the withEscapedDependencies(_:) tool, which allows you to explicitly pass the current dependency context into the escaping closure [2][3]. 2. Dependency Caching: A dependency's value is cached upon its first access and is generally not recomputed [1][4]. This means if a dependency depends on another, and the child dependency is later overridden, the parent may continue to use the stale, cached value from its initial resolution [4]. Critics, such as those in the referenced blog post, argue that this design can make it difficult to manage complex dependency lifecycles and requires developers to be highly vigilant about when and where dependencies are accessed [1]. The library's official documentation highlights these mechanics, noting that understanding these lifetimes is essential for correctly managing dependencies across different execution contexts [2]. To avoid unexpected behavior, developers are advised to audit call sites, use withEscapedDependencies for escaping closures, and be mindful of the initialization timing of their types [1][2].
Citations:
- 1: https://adamz.dev/blog/swift-dependencies-warning/
- 2: https://github.com/pointfreeco/swift-dependencies/blob/main/Sources/Dependencies/Documentation.docc/Articles/Lifetimes.md
- 3: @Dependency(\.date.now) is returning incorrect dates (either in future or past) when used in escaping closures pointfreeco/swift-dependencies#335
- 4: Parent dependency is resolved from cache after child has changed, instead of being recalculated pointfreeco/swift-dependencies#238
Instantiate HubAuthenticationViewModel inside the withDependencies scope for hubLicenseVerifier overrides.
The tests override hubLicenseVerifier only around continueToAccessCheck(), but HubAuthenticationViewModel is created in setUpWithError(), and its @Dependency(\.hubLicenseVerifier) is captured at instance initialization time. That means resolveSubscriptionState(from:) may still call the default verifier when hubLicenseVerifier.verify(...) runs. Recreate the view model within the same withDependencies block that sets hubLicenseVerifier (or use a helper that constructs it using the overridden dependencies) before calling continueToAccessCheck().
Also applies to: 266-272, 298-304, 329-335
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@CryptomatorCommon/Tests/CryptomatorCommonCoreTests/Hub/HubAuthenticationViewModelTests.swift`
around lines 234 - 240, The test is setting hubLicenseVerifier only inside
withDependencies but HubAuthenticationViewModel (which captures
`@Dependency`(\.hubLicenseVerifier) at init) is created earlier in setUpWithError,
so resolveSubscriptionState(from:) may call the default verifier; fix by
instantiating a new HubAuthenticationViewModel inside the same withDependencies
block (or provide a factory helper that constructs HubAuthenticationViewModel
using current dependencies) immediately after overriding $0.hubLicenseVerifier
and before calling continueToAccessCheck(); apply the same pattern for the other
occurrences mentioned (lines around 266-272, 298-304, 329-335).
Source: Learnings
1 participant
Hub now returns a
Hub-iOS-Licenseheader on the vault access-token response: a JWT signed by the License Server (ES512). It replacesHub-Subscription-Statefor deciding whether a vault covered by a paid Hub license unlocks in write mode for a freemium user. The old header can't go away yet (older Hub versions are still in production), so it keeps working but is deprecated, and the new header takes precedence when present.This adds JWS verification, which the codebase didn't do before (only JWE). A stateless
HubLicenseVerifierchecks the ES512/P-521 signature against the License Server's embedded public key and enforcesexpmanually (60s leeway). It's wired intoHubAuthenticationViewModel.receivedExistingKeybehind a@Dependencyseam so tests can inject a test key. Precedence: whenHub-iOS-Licenseis present the legacy header is ignored entirely; a bad signature surfaces an error and blocks the unlock rather than silently falling back. The result feeds the existingHubSubscriptionStatepersistence unchanged, so there's no DB migration andPermissionProvideris untouched.The timeless
.active/.inactivesnapshot model is kept on purpose:expgates once at unlock, matching howHub-Subscription-Statealready behaves.Flow
flowchart TD A[access-token response] --> B{Hub-iOS-License present?} B -- no --> C[Legacy Hub-Subscription-State path] B -- yes --> D[Verify ES512 signature] D -- bad signature or malformed --> E[Surface error and block unlock] D -- valid --> F{Not expired?} F -- yes --> G[Write mode .active] F -- no --> H[Read-only .inactive]