Sign up
View stargazersSign up

refactor(client-runtime): share DPoP proof creation across web and mobile#30540

merged1
refactor/signing-key-services
refactor/shared-dpop
opened1d agobyJJulius Marminge
9 files changed
View on GitHubTwo-way sync enabledPR last imported 6h ago

Stacked on #30529. Web and mobile each built relay DPoP proofs their own way: web through effect/jose, mobile through a hand-built JWT with its own header and payload schemas. Only the ManagedRelayDpopSigner interface lived in client-runtime, so the proof format (claims, jwk header, ath, htu normalization, thumbprint) was written twice.

What changed

  • client-runtime/src/relay/dpopProof.ts (DpopProof):
    • createProof builds the RFC 9449 proof: header with the jwk, the htm/htu/jti/iat/ath claims, and a JWS signed by the platform key.
    • thumbprint computes the RFC 7638 thumbprint via Digest.
    • layerSigner({ keyStore, loadOrCreateKey }) provides ManagedRelayDpopSigner. It loads the key once and keeps it, and a failed load is retried.
  • Platform adapters. Each app now supplies a DpopProofKey = { publicJwk, sign(signingInput) → raw r‖s }:
    • Web: the IndexedDB storage is unchanged, and the key is still a non-extractable CryptoKey. sign goes through Effect's Signature via privateKeyFromCryptoKey.
    • Mobile: the SecureStore storage is unchanged. sign is SHA-256 via the existing expo Digest, then @noble/curves p256.

Why the boundary sits at sign rather than at a Signature service: Effect's key import always runs through crypto.subtle.importKey, and ECDSA private keys have no raw-bytes constructor, so on Hermes there's no way to build the Signature.PrivateKey that Jwt.sign needs without a SubtleCrypto polyfill.

Stored keys and thumbprints are unchanged. Web still writes the same IndexedDB record, including the thumbprint field older versions read.

Tests

  • New dpopProof.test.ts signs with a non-extractable WebCrypto key, the same path as web, and checks the proof with shared/dpop's verifyDpopProof, the verifier the relay and server use. It covers htu normalization and the ath binding.
  • Removed duplicates: the web proof test and the two mobile proof-signing unit tests. Mobile's end-to-end signing is still covered by remoteRegistration.test, which verifies a real mobile-signed proof with verifyDpopProof. Mobile's tests for key persistence and the expo digest stay.
  • vp run check:changed passes.

Not yet done: a live relay sign-in on a device. It needs a real T3 Connect account, so it's left for a manual pass.

Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

Checks

22 successful · 17 skipped
All checks passed
15e9fa0
MmacroscopeappBot1d ago
Synced from GitHub

Approvability

Verdict: Approved at 15e9fa0

Macroscope's review found this PR approvable — This consolidates DPoP proof creation and signing services while preserving existing keys, token policies, and platform storage. The assessed changes introduce no material access-policy or operational change requiring human judgment.

Notes:

  • All code in this push has already been reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot1d ago
Synced from GitHub

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 171431f52276851030d859356db06812a2b8a07a and 15e9fa052c1c756a8374259095ffca6d069cb288.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

Client-runtime now provides shared DPoP proof creation and signer logic. Mobile and web supply platform-specific persisted key loaders and use the shared signer in their managed relay layers.

Changes: DPoP proof generation and relay signing

Layer / File(s)Summary
Shared proof creation and signer
packages/client-runtime/src/relay/dpopProof.ts, packages/client-runtime/src/relay/dpopProof.test.ts, packages/client-runtime/src/relay/index.ts		Client-runtime adds DPoP proof creation, a signer layer, tests for accepted proofs and invalid URLs, and a DpopProof namespace export.
Mobile and browser key adapters
apps/mobile/src/features/cloud/dpop.ts, apps/mobile/src/features/cloud/dpop.test.ts, apps/web/src/cloud/dpop.ts, apps/web/src/cloud/dpop.test.ts		Mobile and web provide persisted keys through the shared signing-key interface. Their local proof-generation code and related proof tests are removed.
Managed relay signer wiring
apps/mobile/src/features/cloud/managedRelayLayer.ts, apps/web/src/cloud/managedRelayLayer.ts		Both managed relay layers use DpopProof.layerSigner with their platform key stores and key loaders.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant ManagedRelayDpopSigner
3 participant DpopProofLayerSigner
4 participant PlatformKeyLoader
5 participant PlatformKeyStore
6 participant CreateProof
7 ManagedRelayDpopSigner->>DpopProofLayerSigner: request a DPoP proof
8 DpopProofLayerSigner->>PlatformKeyLoader: load or create a signing key
9 PlatformKeyLoader->>PlatformKeyStore: read or persist the key
10 DpopProofLayerSigner->>CreateProof: create proof with key and request data
11 CreateProof-->>ManagedRelayDpopSigner: return signed proof

Suggested reviewers: t3dotgg

Merge Risk: External image Minimal · previous review a2676

This refactor moves DPoP proof creation into a shared module while keeping platform key storage and signing in each app. It should not change behavior for existing users. A manual relay sign-in check on a real device is still worth doing before release, but nothing found blocks the merge.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main change: sharing DPoP proof creation across the web and mobile clients.
Description checkPassedThe description explains the problem, implementation, affected platforms, tests, and the untested live sign-in scenario. It references the stacked pull request for scope, although explicit maintainer …
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, implementation, affected platforms, tests, and the untested live sign-in scenario. It references the stacked pull request for scope, although explicit maintainer approval is not stated.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 18eb6fd158485abd8b767690601372e7206b7265 and a267662e206b76921c0a59fe946767d81851ae1c.

External image Files selected for processing: 9
  • apps/mobile/src/features/cloud/dpop.test.ts
  • apps/mobile/src/features/cloud/dpop.ts
  • apps/mobile/src/features/cloud/managedRelayLayer.ts
  • apps/web/src/cloud/dpop.test.ts
  • apps/web/src/cloud/dpop.ts
  • apps/web/src/cloud/managedRelayLayer.ts
  • packages/client-runtime/src/relay/dpopProof.test.ts
  • packages/client-runtime/src/relay/dpopProof.ts
  • packages/client-runtime/src/relay/index.ts
External image Files with no reviewable changes: 1
  • apps/web/src/cloud/dpop.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Added 1 commit

  • refactor(client-runtime): share DPoP proof creation across web and mobile15e9fa0
CcoderabbitaiBot11h ago
Synced from GitHub
Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.