feat: back share links with a persisted ACTA presentation - #52
Conversation
Share links used to be a plaintext base64url token carrying
`{ ids, exp }`. Anyone could decode it into the credential ids or edit
the expiry, and expiration was only ever checked in the browser.
The link now carries an opaque 256-bit random reference to a
presentation that is built and persisted server-side:
- `@acta-products/acta/presentation` models a W3C-shaped
VerifiablePresentation that references credentials by id only, with a
canonical payload and SHA-256 digest for the holder proof.
- The holder signs that digest through the wallet connector when the
wallet supports it; the proof is stored with the presentation for the
public verifier to validate against the holder's did:stellar key.
- `POST /api/presentations` persists the presentation behind a
`PresentationStore` seam and returns only the reference;
`GET /api/presentations/[ref]` resolves it, enforces expiration and
burns the reference once expired.
- The verify view resolves the reference instead of decoding the URL,
and the /share UI is unchanged.
Closes ACTA-Team#39
|
@DavidBrenesCR is attempting to deploy a commit to the ACTA Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughChangesThe PR replaces client-encoded presentation tokens with ACTA verifiable presentations stored behind opaque references. It adds presentation contracts, persistence, creation/resolution APIs, wallet-assisted link generation, server-side expiration, and reference-based public verification. Presentation sharing and verification
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SharePage
participant WalletConnector
participant PresentationsAPI
participant PresentationStore
participant PublicVerificationView
SharePage->>WalletConnector: sign presentation digest
WalletConnector-->>SharePage: optional proof
SharePage->>PresentationsAPI: POST presentation data
PresentationsAPI->>PresentationStore: persist presentation
PresentationStore-->>PresentationsAPI: opaque ref
PresentationsAPI-->>SharePage: verifier URL and expiration
PublicVerificationView->>PresentationsAPI: GET opaque ref
PresentationsAPI->>PresentationStore: resolve and enforce expiration
PresentationStore-->>PresentationsAPI: presentation status
PresentationsAPI-->>PublicVerificationView: presentation or invalid status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)apps/credit-history/src/i18n/messages/en.jsonTraceback (most recent call last): apps/credit-history/src/i18n/messages/es.jsonTraceback (most recent call last): packages/acta/package.jsonTraceback (most recent call last): 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/credit-history/src/components/verify/public-verification-view.test.tsx (1)
93-95: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTest generated references, not a hard-coded fixture.
This only proves that
'a'.repeat(43)lackscred-income; it does not protect the share-link contract. Add the assertion where the API or link generator creates a reference, including that credential IDs and expiration are absent.🤖 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 `@apps/credit-history/src/components/verify/public-verification-view.test.tsx` around lines 93 - 95, Update the test around the shared-reference generation flow, rather than asserting against the hard-coded REF fixture. Generate the reference through the relevant API or link-generator symbol and assert that the resulting value excludes credential IDs such as “cred-income” and expiration data.apps/credit-history/src/lib/presentation-link.ts (2)
1-139: 📐 Maintainability & Code Quality | 🔵 TrivialConsider adding unit tests for this module.
createPresentationLink,resolvePresentationRef, andsignPresentationform the client-side entry point for the new share-link security model (holder proof signing, opaque ref creation/resolution), but no test file accompanies this module in the diff, unlikepackages/acta/src/presentation.test.ts. Mockingfetchand the wallet connector would let these paths (401/410/network-failure handling in particular) be exercised directly.🤖 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 `@apps/credit-history/src/lib/presentation-link.ts` around lines 1 - 139, 【issue】The presentation-link client module lacks unit coverage for its security-critical creation, resolution, and signing paths. Add a focused test file covering createPresentationLink, resolvePresentationRef, and signPresentation; mock fetch and WalletConnector to verify successful behavior plus non-OK creation responses, 410 expiration handling, generic errors, network failures, missing signing support, signing failures, and successful proof construction.
121-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
signPresentation's catch-all masks validation bugs as "wallet declined."
buildPresentation/presentationDigestfailures (e.g., a malformedexpiresAt) and genuine wallet-signing failures both fall into the same catch and log the same "wallet could not sign" warning. SincecreatePresentationLinklater re-sends the same invalid input and the server rejects it, there's no user-facing bug — but a real validation error is misreported here, which could slow down debugging.♻️ Suggested separation
if (!connector.signTransaction) return null; + const presentation = buildPresentation(input); + const digest = await presentationDigest(presentation); + try { - const presentation = buildPresentation(input); - const digest = await presentationDigest(presentation); const { signedXdr } = await connector.signTransaction(digest, opts); if (!signedXdr) return null;🤖 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 `@apps/credit-history/src/lib/presentation-link.ts` around lines 121 - 139, Update signPresentation so validation and digest-generation failures from buildPresentation or presentationDigest are not classified as wallet-signing failures. Keep those errors distinguishable or propagated, and restrict the “wallet could not sign” warning and null fallback to failures from connector.signTransaction (while preserving the existing successful signature and missing-signedXdr behavior).apps/credit-history/src/lib/presentation-store.ts (1)
56-92: 🩺 Stability & Availability | 🔵 TrivialNo bound on store growth or stale-entry cleanup.
Entries are only removed when
get()is called on an already-expired ref (Line 173-176); presentations that are never resolved (or created withexpiresAt: null) live in theMapforever until process restart. There's also no cap on the number of entries. Since this is process-local by design (per the doc comment), consider adding either a periodic sweep of expired-but-unresolved entries or a max-size/LRU eviction policy before this ships behind a durable store, especially given the creation endpoint currently has no rate limiting (seeroute.ts).🤖 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 `@apps/credit-history/src/lib/presentation-store.ts` around lines 56 - 92, Bound the process-local presentation store to prevent unbounded growth, including entries with expiresAt: null or never-resolved references. Update InMemoryPresentationStore and its records-backed Map to enforce a maximum size with deterministic LRU eviction, while retaining existing save/get/delete behavior and expired-entry handling. Ensure eviction occurs before adding new entries and does not remove the newly saved presentation.
🤖 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 `@apps/credit-history/src/app/api/presentations/route.ts`:
- Around line 50-103: Update the POST handler to require server-side
authentication or session validation before calling createStoredPresentation,
and reject unauthenticated requests with the established authorization error
response. Bind the authorized caller’s identity to holder and validate that each
credentialId is accessible to that caller instead of trusting these body fields;
preserve the existing validation and presentation-creation flow for authorized
requests.
In `@apps/credit-history/src/components/verify/public-verification-view.tsx`:
- Around line 137-139: Update the credential resolution logic in the public
verification view around the allCredentials filter so the presentation is
accepted only when every ID in presentation.verifiableCredential matches a
locally resolved credential. If any referenced credential is missing, fail
closed and render the presentation as invalid rather than using the partial
credentials list.
---
Nitpick comments:
In `@apps/credit-history/src/components/verify/public-verification-view.test.tsx`:
- Around line 93-95: Update the test around the shared-reference generation
flow, rather than asserting against the hard-coded REF fixture. Generate the
reference through the relevant API or link-generator symbol and assert that the
resulting value excludes credential IDs such as “cred-income” and expiration
data.
In `@apps/credit-history/src/lib/presentation-link.ts`:
- Around line 1-139: 【issue】The presentation-link client module lacks unit
coverage for its security-critical creation, resolution, and signing paths. Add
a focused test file covering createPresentationLink, resolvePresentationRef, and
signPresentation; mock fetch and WalletConnector to verify successful behavior
plus non-OK creation responses, 410 expiration handling, generic errors, network
failures, missing signing support, signing failures, and successful proof
construction.
- Around line 121-139: Update signPresentation so validation and
digest-generation failures from buildPresentation or presentationDigest are not
classified as wallet-signing failures. Keep those errors distinguishable or
propagated, and restrict the “wallet could not sign” warning and null fallback
to failures from connector.signTransaction (while preserving the existing
successful signature and missing-signedXdr behavior).
In `@apps/credit-history/src/lib/presentation-store.ts`:
- Around line 56-92: Bound the process-local presentation store to prevent
unbounded growth, including entries with expiresAt: null or never-resolved
references. Update InMemoryPresentationStore and its records-backed Map to
enforce a maximum size with deterministic LRU eviction, while retaining existing
save/get/delete behavior and expired-entry handling. Ensure eviction occurs
before adding new entries and does not remove the newly saved presentation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cb81cf15-1c80-459b-9cf4-3fe5695fb59e
📒 Files selected for processing (17)
apps/credit-history/src/app/api/presentations/[ref]/route.tsapps/credit-history/src/app/api/presentations/route.tsapps/credit-history/src/app/share/page.tsxapps/credit-history/src/components/verify/public-verification-view.test.tsxapps/credit-history/src/components/verify/public-verification-view.tsxapps/credit-history/src/i18n/messages/en.jsonapps/credit-history/src/i18n/messages/es.jsonapps/credit-history/src/lib/presentation-link.tsapps/credit-history/src/lib/presentation-store.test.tsapps/credit-history/src/lib/presentation-store.tsapps/credit-history/src/lib/presentation-token.test.tsapps/credit-history/src/lib/presentation-token.tsapps/credit-history/src/session/wallet-connector.tspackages/acta/package.jsonpackages/acta/src/index.tspackages/acta/src/presentation.test.tspackages/acta/src/presentation.ts
💤 Files with no reviewable changes (2)
- apps/credit-history/src/lib/presentation-token.ts
- apps/credit-history/src/lib/presentation-token.test.ts
| export async function POST(request: Request): Promise<NextResponse> { | ||
| let body: CreateRequestBody; | ||
|
|
||
| try { | ||
| body = (await request.json()) as CreateRequestBody; | ||
| } catch { | ||
| return NextResponse.json({ error: 'Malformed JSON body.' }, { status: 400 }); | ||
| } | ||
|
|
||
| const { holder, credentialIds, expiresAt, createdAt } = body; | ||
|
|
||
| if (typeof holder !== 'string' || holder.length === 0) { | ||
| return NextResponse.json({ error: 'A holder DID is required.' }, { status: 400 }); | ||
| } | ||
|
|
||
| if (!Array.isArray(credentialIds)) { | ||
| return NextResponse.json({ error: 'credentialIds must be an array.' }, { status: 400 }); | ||
| } | ||
|
|
||
| if (expiresAt !== null && typeof expiresAt !== 'number') { | ||
| return NextResponse.json( | ||
| { error: 'expiresAt must be a timestamp in milliseconds or null.' }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| if (createdAt !== undefined && typeof createdAt !== 'number') { | ||
| return NextResponse.json( | ||
| { error: 'createdAt must be a timestamp in milliseconds.' }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| const { ref, presentation } = await createStoredPresentation({ | ||
| holder, | ||
| credentialIds: credentialIds as string[], | ||
| expiresAt, | ||
| createdAt, | ||
| proof: parseProof(body.proof), | ||
| }); | ||
|
|
||
| return NextResponse.json( | ||
| { ref, expiresAt: presentationExpiresAt(presentation) }, | ||
| { status: 201, headers: { 'Cache-Control': 'no-store' } } | ||
| ); | ||
| } catch (err) { | ||
| if (err instanceof InvalidPresentationError) { | ||
| return NextResponse.json({ error: err.message }, { status: 400 }); | ||
| } | ||
| console.error('Failed to create presentation', err); | ||
| return NextResponse.json({ error: 'Could not create the presentation.' }, { status: 500 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'apps/credit-history/src/app/api/presentations/route\.ts|apps/credit-history/src|middleware|auth|session|next\.config' | head -200
echo "== target route excerpt =="
if [ -f apps/credit-history/src/app/api/presentations/route.ts ]; then
nl -ba apps/credit-history/src/app/api/presentations/route.ts | sed -n '1,180p'
fi
echo "== auth/middleware references in credit-history =="
rg -n --no-heading "next-auth|auth|session|jwt|Middleware|credentials|bearer|Authorization|holder|credentialIds|createStoredPresentation" apps/credit-history -S || true
echo "== package scripts/deps relevant =="
if [ -f apps/credit-history/package.json ]; then cat apps/credit-history/package.json; fiRepository: ACTA-Team/products-acta
Length of output: 2506
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target route excerpt =="
awk '{ printf "%6d %s\n", NR, $0 }' apps/credit-history/src/app/api/presentations/route.ts | sed -n '1,180p'
echo "== GET presentation route =="
awk '{ printf "%6d %s\n", NR, $0 }' "apps/credit-history/src/app/api/presentations/[ref]/route.ts" | sed -n '1,220p'
echo "== auth/session/middleware references in credit-history =="
rg -n --no-heading "next-auth|auth|session|jwt|Middleware|credentials|bearer|Authorization|holder|credentialIds|createStoredPresentation|request\.headers|Authorization" apps/credit-history/src apps/credit-history -S || true
echo "== route middleware files =="
git ls-files apps/credit-history/src apps/credit-history | rg '(^|/)middleware(\.(ts|tsx|js|jsx)|/middleware|\.middleware)\.ts$|middleware'
echo "== package metadata relevant =="
if [ -f apps/credit-history/package.json ]; then awk '{ print NR ": " $0 }' apps/credit-history/package.json | sed -n '1,220p'; fi
echo "== read-only structural verifier for POST route =="
python3 - <<'PY'
from pathlib import Path
p = Path("apps/credit-history/src/app/api/presentations/route.ts")
s = p.read_text()
checks = {
"imports_auth": any(x in s for x in ["next-auth", ".next/server/auth", "SessionProvider"]),
"imports_middleware_auth": "middleware" in p.parent.name.lower() or "middleware" in p.name.lower(),
"uses_request_headers": "request.headers" in s or "headers.get" in s,
"calls_get_auth": any(s.find(x) != -1 for x in ["getAuth", "auth()"]),
"validates_holder_source": any(x in s for x in ["userId", "sub", "credentials", "claims", "owner", "request.auth", "request.user", "session"]),
"validates_credential_ownership": any(x in s for x in ["credentialOwners", "ownerId", "credentialIds", "credentials.filter"]),
}
for k,v in checks.items():
print(f"{k}={v}")
PYRepository: ACTA-Team/products-acta
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== session provider =="
awk '{ printf "%6d %s\n", NR, $0 }' apps/credit-history/src/session/session-provider.tsx | sed -n '1,180p'
echo "== presentation link/create flow =="
awk '{ printf "%6d %s\n", NR, $0 }' apps/credit-history/src/lib/presentation-link.ts | sed -n '1,180p'
echo "== presentation store =="
awk '{ printf "%6d %s\n", NR, $0 }' apps/credit-history/src/lib/presentation-store.ts | sed -n '1,180p'
echo "== share submit excerpt =="
awk '{ printf "%6d %s\n", NR, $0 }' apps/credit-history/src/app/share/page.tsx | sed -n '100,160p'
echo "== middleware/auth presence exact =="
git ls-files apps/credit-history | rg '(^|/)middleware(\.ts|\.tsx|\.js|\.jsx)$|middleware\.ts$' || true
git ls-files apps/credit-history/src/lib apps/credit-history/src/app api | rg 'auth|session|header|bearer' || true
echo "== read-only verifier =="
python3 - <<'PY'
from pathlib import Path
files = {
"route": Path("apps/credit-history/src/app/api/presentations/route.ts").read_text(),
"link": Path("apps/credit-history/src/lib/presentation-link.ts").read_text(),
"store": Path("apps/credit-history/src/lib/presentation-store.ts").read_text(),
"session": Path("apps/credit-history/src/session/session-provider.tsx").read_text(),
}
print("POST route reads request.headers:", "request.headers" in files["route"])
print("POST route calls auth/session:", any(term in files["route"] for term in ["getAuth", "auth()", "session"]))
print("POST route passes stored presentation.ref:", "'ref' in files['route']" is None and "ref" in files["route"])
print("presentation link creates unverified proof:", "signPresentationProof" in files["link"])
print("store constructor accepts unverified holder:", "holder" in files["store"] and "credentialIds" in files["store"])
PYRepository: ACTA-Team/products-acta
Length of output: 21304
Bind presentation creation to an authorized caller.
POST /api/presentations currently trusts holder and credentialIds from the JSON body after only type checks. There is no route or middleware auth for this endpoint, and the present session flow stores the connected wallet address in client-side localStorage, so an unauthenticated caller can forge arbitrary holder/credentialIds and get back a live presentation ref. Gate creation behind server-side auth/session enforcement before this ships more broadly.
🤖 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 `@apps/credit-history/src/app/api/presentations/route.ts` around lines 50 -
103, Update the POST handler to require server-side authentication or session
validation before calling createStoredPresentation, and reject unauthenticated
requests with the established authorization error response. Bind the authorized
caller’s identity to holder and validate that each credentialId is accessible to
that caller instead of trusting these body fields; preserve the existing
validation and presentation-creation flow for authorized requests.
| const credentials = allCredentials.filter((c) => | ||
| presentation.verifiableCredential.includes(c.id) | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject partially resolved presentations.
A presentation for ['cred-a', 'cred-b'] is rendered as valid when only cred-a exists locally. Require every referenced credential to resolve; otherwise fail closed as invalid.
Proposed fix
+ const requestedIds = new Set(presentation.verifiableCredential);
const credentials = allCredentials.filter((c) =>
- presentation.verifiableCredential.includes(c.id)
+ requestedIds.has(c.id)
);
- if (credentials.length === 0) {
+ if (credentials.length !== requestedIds.size) {
if (!cancelled) setState({ phase: 'invalid', reason: 'malformed' });
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const credentials = allCredentials.filter((c) => | |
| presentation.verifiableCredential.includes(c.id) | |
| ); | |
| const requestedIds = new Set(presentation.verifiableCredential); | |
| const credentials = allCredentials.filter((c) => | |
| requestedIds.has(c.id) | |
| ); | |
| if (credentials.length !== requestedIds.size) { | |
| if (!cancelled) setState({ phase: 'invalid', reason: 'malformed' }); | |
| return; | |
| } |
🤖 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 `@apps/credit-history/src/components/verify/public-verification-view.tsx`
around lines 137 - 139, Update the credential resolution logic in the public
verification view around the allCredentials filter so the presentation is
accepted only when every ID in presentation.verifiableCredential matches a
locally resolved credential. If any referenced credential is missing, fail
closed and render the presentation as invalid rather than using the partial
credentials list.
Closes #39
Problem
A share link was a plaintext base64url token carrying
{ ids, exp }straight in the URL. Anyone could decode it into the credential ids, or edit it to extend the expiry — andexpwas only ever checked client-side, in the verify view.What changed
The URL now carries an opaque 256-bit random reference. The presentation itself is built, signed and persisted server-side, out of the verifier's reach.
New
packages/acta/src/presentation.ts— a W3C-shapedVerifiablePresentationthat references credentials by id only (claims are never part of the shared object; the resolving side re-reads them from the vault). Provides a deterministic canonical payload — keys sorted recursively,proofexcluded — and its SHA-256 digest, so a verifier can recompute what the holder signed. Also declares thePresentationStorepersistence seam.apps/credit-history/src/lib/presentation-store.ts— server-side persistence, opaque reference generation, andresolveStoredPresentation, which enforces expiration and deletes the record on the way out so an expired reference stops resolving for good.POST /api/presentations→ persists and returns only{ ref, expiresAt }.GET /api/presentations/[ref]→200/404 not_found/410 expired.apps/credit-history/src/lib/presentation-link.ts— replaces the old codec.signPresentationasks the wallet connector for a holder proof over the digest.Changed
/sharebuilds the presentation through the API instead of encoding a token. The UI and the result screen are untouched — the 1h/1d/7d/30d/custom/never presets behave exactly as before.presentation-token.tsand its test are gone.Acceptance criteria
404@acta-products/acta/presentation, persisted behindPresentationStore, resolved by refresolveStoredPresentationdecides, returns410and burns the reference/shareUX unchanged; lint/typecheck/build passVerification
pnpm lint,pnpm typecheck,pnpm test(57 tests) andpnpm buildall pass. The flow was also exercised end-to-end againstnext start:201→ only{ref, expiresAt}200+ presentation404 not_found410 expired404— reference burnedexpiresAt/ empty selection400Two things reviewers should know
The store is in-memory (a
MaponglobalThis, so it survives dev hot-reloads). It does not survive a restart and is not shared between serverless instances. It sits behind thePresentationStorecontract, so swapping in a durable KV or an ACTA off-chain payload is a one-line change ingetPresentationStore()and touches nothing else. This is documented in the file.The holder proof is best-effort.
signTransactionexpects a Stellar XDR envelope, not a bare digest, so neither the mock connector nor every real wallet will produce one. On refusal the presentation is persisted unsigned and sharing is not blocked — the opaque reference is what makes the link tamper-evident; the proof is the extra attribution layer. Validating that signature against the holder's did:stellar key belongs to Real public verification (verifyVc against the vc-vault) #40.createdAtis sent by the client so the server rebuilds the exact object the holder signed (otherwise the twocreatedtimestamps differ and the digest no longer matches); the server rejects values more than 5 minutes from its own clock, so it can't be used to backdate a presentation.Out of scope
Verifier rendering and the on-chain revocation check — #40.
Summary by CodeRabbit