feat: public verification view (verifier side) - #32
Conversation
…ials Refactor /verify/[token] to canonical next-intl + @acta-products/ui patterns. Add VerificationBanner, presentation token codec, and marked SDK seam for future RPC verification against vc-vault. Closes ACTA-Team#20
|
@felipevega2x is attempting to deploy a commit to the ACTA Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR adds a public verification flow for shared presentation tokens. It introduces a token codec, a server route that passes the token into a client view, a verification banner, localized copy, and credential rendering for loading, invalid, and ready states. ChangesPublic verification flow
Sequence Diagram(s)sequenceDiagram
participant VerifyPage
participant PublicVerificationView
participant decodePresentationToken
participant getCredentialSource
participant VerificationBanner
VerifyPage->>PublicVerificationView: token
PublicVerificationView->>decodePresentationToken: decode(token)
alt token invalid or expired
PublicVerificationView->>VerificationBanner: render invalid banner
else token valid
PublicVerificationView->>getCredentialSource: fetch matching credentials and holder profile
getCredentialSource-->>PublicVerificationView: credentials and profile
PublicVerificationView->>VerificationBanner: render validity banner
end
Estimated review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/ui/src/components/verification-banner.tsx (1)
74-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
<h2>can break the document outline.The banner always emits an
<h2>. Inpublic-verification-view.tsxthe banner renders before the page<h1>(Line 195 precedes Line 211), and in the invalid state (Line 168) the banner is the only heading with no<h1>present. This yields an out-of-order / orphaned heading hierarchy for screen-reader users.Consider making the heading level configurable so consumers can keep a valid outline.
♻️ Example: configurable heading level
export interface VerificationBannerProps extends React.HTMLAttributes<HTMLDivElement> { status: CredentialStatusKind; title: string; description: string; icon?: React.ReactNode; + /** Heading level for the title. Defaults to "h2". */ + titleAs?: 'h1' | 'h2' | 'h3'; }Then render the chosen tag for the title instead of a fixed
<h2>.🤖 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 `@packages/ui/src/components/verification-banner.tsx` around lines 74 - 76, The VerificationBanner title is hardcoded as an h2, which can produce an invalid heading hierarchy in consumers like public-verification-view. Update the VerificationBanner component to make the heading level configurable (for example via a prop) and render the chosen tag instead of always using h2, so callers can place the banner before or after the page h1 without breaking the document outline.apps/credit-history/src/components/verify/public-verification-view.tsx (1)
196-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompute the overall status once.
overallPresentationStatus(state.credentials)is invoked three times to drivestatus,title, anddescription. Compute it once into a local for clarity.♻️ Proposed tweak
- <VerificationBanner - status={overallPresentationStatus(state.credentials)} - title={ - overallPresentationStatus(state.credentials) === 'revoked' - ? t('banner.revoked.title') - : t('banner.valid.title') - } - description={ - overallPresentationStatus(state.credentials) === 'revoked' - ? t('banner.revoked.description') - : t('banner.valid.description') - } - /> + <VerificationBanner + status={overall} + title={overall === 'revoked' ? t('banner.revoked.title') : t('banner.valid.title')} + description={ + overall === 'revoked' + ? t('banner.revoked.description') + : t('banner.valid.description') + } + />Declare
const overall = overallPresentationStatus(state.credentials);at the start of thereadybranch.🤖 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 196 - 206, Compute the overall presentation status once in the ready branch of public-verification-view.tsx instead of calling overallPresentationStatus(state.credentials) three times. Add a local like overall at the start of that branch, then reuse it for status, title, and description so the logic is clearer and avoids repeated evaluation.
🤖 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/components/verify/public-verification-view.tsx`:
- Around line 328-335: The issued date pair in public-verification-view.tsx is
using the same translation for both the semantic label and the visible value, so
the <dt> is not actually labeling the <dd>. Update the issued field to use a
distinct label key in the <dt> within the credential issued display block,
similar to how the issuer section uses a separate label, while keeping the <dd>
on the existing item.issued translation with the date value.
- Around line 124-129: The public verification flow in
public-verification-view.tsx is still fetching every credential via
source.listCredentials() and filtering client-side, which exposes more data than
needed. Update the credential-loading logic around the Promise.all block to
request only the credential IDs in payload.ids using source.getCredential(id)
(or introduce a batched id-based API if needed), and keep the profile summary
fetch unchanged. Make sure the credentials variable is built only from the
returned requested credentials, not from a full list.
In `@apps/credit-history/src/i18n/messages/en.json`:
- Line 102: The locale files contain an unused expiredAt translation key that is
no longer referenced by the credit history view. Remove the expiredAt entry from
the message catalogs in en.json and es.json, and keep the remaining keys aligned
with the existing expiration, neverExpires, and invalid.expiredDescription usage
in the credit-history i18n messages.
In `@apps/credit-history/src/lib/presentation-token.ts`:
- Around line 35-47: decodePresentationToken currently trusts any parsed payload
as long as ids is a non-empty array, so malformed tokens can still pass through.
Tighten the schema in decodePresentationToken by validating that each ids entry
is the expected type and that exp is present as either a number or null before
returning the payload, otherwise return null. Use the existing
decodePresentationToken and PresentationPayload shape as the place to enforce
this fail-closed validation.
---
Nitpick comments:
In `@apps/credit-history/src/components/verify/public-verification-view.tsx`:
- Around line 196-206: Compute the overall presentation status once in the ready
branch of public-verification-view.tsx instead of calling
overallPresentationStatus(state.credentials) three times. Add a local like
overall at the start of that branch, then reuse it for status, title, and
description so the logic is clearer and avoids repeated evaluation.
In `@packages/ui/src/components/verification-banner.tsx`:
- Around line 74-76: The VerificationBanner title is hardcoded as an h2, which
can produce an invalid heading hierarchy in consumers like
public-verification-view. Update the VerificationBanner component to make the
heading level configurable (for example via a prop) and render the chosen tag
instead of always using h2, so callers can place the banner before or after the
page h1 without breaking the document outline.
🪄 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
Run ID: 97696497-c30f-4825-834f-7d8b8a4bb667
📒 Files selected for processing (7)
apps/credit-history/src/app/verify/[token]/page.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-token.tspackages/ui/src/components/verification-banner.tsxpackages/ui/src/index.ts
| const [allCredentials, profile] = await Promise.all([ | ||
| source.listCredentials(), | ||
| source.getProfileSummary(), | ||
| ]); | ||
|
|
||
| const credentials = allCredentials.filter((c) => payload.ids.includes(c.id)); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the credential source can fetch by id rather than listing all.
rg -nP 'listCredentials|getCredentialSource|getProfileSummary' --type=ts -C2Repository: ACTA-Team/products-acta
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the surrounding implementation.
git ls-files 'apps/credit-history/src/components/verify/public-verification-view.tsx'
wc -l apps/credit-history/src/components/verify/public-verification-view.tsx
sed -n '1,220p' apps/credit-history/src/components/verify/public-verification-view.tsx
# Look for the credential source API shape in nearby files.
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'listCredentials|getCredentialSource|getProfileSummary|credentials' apps/credit-history/src -C 2Repository: ACTA-Team/products-acta
Length of output: 47085
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect any source implementations referenced by the public verification view.
fd -a '.*(credential|profile|source).*' apps/credit-history/srcRepository: ACTA-Team/products-acta
Length of output: 705
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the reviewed file and related data-loading patterns.
git ls-files 'apps/credit-history/src/components/verify/*'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'Promise\.all\(|payload\.ids|source\.' apps/credit-history/src/components/verify -C 3Repository: ACTA-Team/products-acta
Length of output: 1699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the credential source API to see whether fetching by id(s) is already available.
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'getCredential\(|getCredentials\(|listCredentials\(|getCredentialSource\(' .Repository: ACTA-Team/products-acta
Length of output: 1731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the public verification component and the credential source/types around it.
sed -n '1,220p' apps/credit-history/src/components/verify/public-verification-view.tsx
printf '\n---\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'getCredential\(|listCredentials\(|getProfileSummary\(|getCredentialSource\(' apps/credit-history/src -C 2Repository: ACTA-Team/products-acta
Length of output: 10144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the credential source API to see whether fetching by id(s) is already available.
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'getCredential\(|getCredentials\(|listCredentials\(|getCredentialSource\(' .Repository: ACTA-Team/products-acta
Length of output: 1731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the public verification component and the credential source/types around it.
sed -n '1,220p' apps/credit-history/src/components/verify/public-verification-view.tsx
printf '\n---\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'getCredential\(|listCredentials\(|getProfileSummary\(|getCredentialSource\(' apps/credit-history/src -C 2Repository: ACTA-Team/products-acta
Length of output: 10144
Fetch only the presented credentials. source.listCredentials() still sends the full credential set to the browser and filters it afterward. Since getCredential(id) already exists, this flow should request only payload.ids (or add a batched id-based RPC) instead.
🤖 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 124 - 129, The public verification flow in
public-verification-view.tsx is still fetching every credential via
source.listCredentials() and filtering client-side, which exposes more data than
needed. Update the credential-loading logic around the Promise.all block to
request only the credential IDs in payload.ids using source.getCredential(id)
(or introduce a batched id-based API if needed), and keep the profile summary
fetch unchanged. Make sure the credentials variable is built only from the
returned requested credentials, not from a full list.
| <div className="flex flex-col gap-1"> | ||
| <dt className="sr-only"> | ||
| {tCredentials('item.issued', { date: new Date(credential.issueDate) })} | ||
| </dt> | ||
| <dd className="text-sm text-foreground"> | ||
| {tCredentials('item.issued', { date: new Date(credential.issueDate) })} | ||
| </dd> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
<dt> duplicates the <dd> value instead of labeling it.
Both the sr-only term and the description render item.issued with the same date, so screen readers announce the issued string twice and the term provides no actual label. Compare with the issuer field above (Lines 323-326) which uses a distinct label. Use a proper label key for the <dt>.
🤖 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 328 - 335, The issued date pair in public-verification-view.tsx is
using the same translation for both the semantic label and the visible value, so
the <dt> is not actually labeling the <dd>. Update the issued field to use a
distinct label key in the <dt> within the credential issued display block,
similar to how the issuer section uses a separate label, while keeping the <dd>
on the existing item.issued translation with the date value.
| "holderDid": "Holder DID", | ||
| "expiration": "Expires", | ||
| "neverExpires": "Never expires", | ||
| "expiredAt": "Expired at", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP "expiredAt" --type=ts --type=tsx -g '!**/messages/**' || rg -nP "expiredAt" -g '*.ts' -g '*.tsx' -g '!**/messages/**'Repository: ACTA-Team/products-acta
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the i18n files and search for expiredAt / related expiry keys in source.
git ls-files 'apps/credit-history/src/i18n/messages/*.json' 'apps/credit-history/src/**/*.{ts,tsx}' | sed -n '1,200p'
printf '\n--- expiredAt occurrences ---\n'
grep -RIn --exclude-dir=node_modules --exclude='*.json' --include='*.ts' --include='*.tsx' --include='*.json' 'expiredAt\|invalid.expiredDescription\|neverExpires\|expiration' apps/credit-history/src | sed -n '1,200p'Repository: ACTA-Team/products-acta
Length of output: 3325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the i18n files and search for expiredAt / related expiry keys in source.
git ls-files 'apps/credit-history/src/i18n/messages/*.json' 'apps/credit-history/src/**/*.{ts,tsx}' | sed -n '1,200p'
printf '\n--- expiredAt occurrences ---\n'
grep -RIn --exclude-dir=node_modules --exclude='*.json' --include='*.ts' --include='*.tsx' --include='*.json' 'expiredAt\|invalid.expiredDescription\|neverExpires\|expiration' apps/credit-history/src | sed -n '1,200p'Repository: ACTA-Team/products-acta
Length of output: 3325
Remove the unused expiredAt translation key. The view already uses expiration, neverExpires, and invalid.expiredDescription; expiredAt is not referenced anywhere, so drop it from apps/credit-history/src/i18n/messages/en.json and apps/credit-history/src/i18n/messages/es.json to keep the locale files aligned.
🤖 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/i18n/messages/en.json` at line 102, The locale files
contain an unused expiredAt translation key that is no longer referenced by the
credit history view. Remove the expiredAt entry from the message catalogs in
en.json and es.json, and keep the remaining keys aligned with the existing
expiration, neverExpires, and invalid.expiredDescription usage in the
credit-history i18n messages.
| export function decodePresentationToken(token: string): PresentationPayload | null { | ||
| try { | ||
| const jsonStr = base64UrlDecode(token); | ||
| const parsed = JSON.parse(jsonStr) as PresentationPayload; | ||
|
|
||
| if (!parsed || !Array.isArray(parsed.ids) || parsed.ids.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return parsed; | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tighten the token schema before trusting it.
decodePresentationToken() only checks that ids is a non-empty array. A forged payload with a missing or non-numeric exp still decodes, and the verifier view will treat that value as usable when deciding whether the link is expired. Validate ids item types and require exp to be number | null here so malformed links fail closed.
🔧 Suggested fix
export function decodePresentationToken(token: string): PresentationPayload | null {
try {
const jsonStr = base64UrlDecode(token);
const parsed = JSON.parse(jsonStr) as PresentationPayload;
- if (!parsed || !Array.isArray(parsed.ids) || parsed.ids.length === 0) {
+ if (
+ !parsed ||
+ !Array.isArray(parsed.ids) ||
+ parsed.ids.length === 0 ||
+ !parsed.ids.every((id) => typeof id === 'string' && id.length > 0) ||
+ !(parsed.exp === null || (typeof parsed.exp === 'number' && Number.isFinite(parsed.exp)))
+ ) {
return null;
}
return parsed;
} catch {📝 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.
| export function decodePresentationToken(token: string): PresentationPayload | null { | |
| try { | |
| const jsonStr = base64UrlDecode(token); | |
| const parsed = JSON.parse(jsonStr) as PresentationPayload; | |
| if (!parsed || !Array.isArray(parsed.ids) || parsed.ids.length === 0) { | |
| return null; | |
| } | |
| return parsed; | |
| } catch { | |
| return null; | |
| } | |
| export function decodePresentationToken(token: string): PresentationPayload | null { | |
| try { | |
| const jsonStr = base64UrlDecode(token); | |
| const parsed = JSON.parse(jsonStr) as PresentationPayload; | |
| if ( | |
| !parsed || | |
| !Array.isArray(parsed.ids) || | |
| parsed.ids.length === 0 || | |
| !parsed.ids.every((id) => typeof id === 'string' && id.length > 0) || | |
| !(parsed.exp === null || (typeof parsed.exp === 'number' && Number.isFinite(parsed.exp))) | |
| ) { | |
| return null; | |
| } | |
| return parsed; | |
| } catch { | |
| 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-token.ts` around lines 35 - 47,
decodePresentationToken currently trusts any parsed payload as long as ids is a
non-empty array, so malformed tokens can still pass through. Tighten the schema
in decodePresentationToken by validating that each ids entry is the expected
type and that exp is present as either a number or null before returning the
payload, otherwise return null. Use the existing decodePresentationToken and
PresentationPayload shape as the place to enforce this fail-closed validation.
Summary
/verify/[token]to canonical next-intl +@acta-products/uipatterns (aligned with credential detail Credential detail view #9).VerificationBannerin@acta-products/ui, presentation token codec, and a marked SDK seam for future RPC verification against vc-vault.Closes #20
Screenshots
Valid presentation
Invalid or expired link
Revoked