Skip to content

fix(eid-wallet): mask security-answer input with reveal toggle#1042

Open
Bekiboo wants to merge 2 commits into
mainfrom
fix/eid-wallet-mask-security-answer-input
Open

fix(eid-wallet): mask security-answer input with reveal toggle#1042
Bekiboo wants to merge 2 commits into
mainfrom
fix/eid-wallet-mask-security-answer-input

Conversation

@Bekiboo

@Bekiboo Bekiboo commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Description of change

Mask the security-answer input in the eID wallet: hidden by default with an eye toggle to reveal. Applied to both screens where the answer is typed — the Restore flow and the Knowledge set/edit sheet — so they're consistent and the answer isn't shown in plain text. (The issue assumed the set screen already masked it; it didn't, so both are fixed.)

Issue Number

Closes #1012

Type of change

  • Fix (a change which fixes an issue) — UX/consistency, P3

How the change has been tested

Masked by default on both screens; eye toggle reveals/hides; reveal resets when the Knowledge sheet reopens. biome + svelte-check clean (no new issues).

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

New Features

  • Answer inputs are now masked by default for enhanced privacy and security.
  • Added an eye icon toggle control that allows users to reveal or hide sensitive answers on demand.
  • Toggle includes proper accessibility labels and state indicators, improving usability across multiple screens and workflows.

@Bekiboo Bekiboo self-assigned this Jun 15, 2026
@Bekiboo Bekiboo requested a review from coodos as a code owner June 15, 2026 12:57
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Bekiboo, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 18 minutes and 51 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0e2dced3-4579-4755-b773-969e67329ad5

📥 Commits

Reviewing files that changed from the base of the PR and between 79a02a1 and cb78da9.

📒 Files selected for processing (1)
  • infrastructure/eid-wallet/src/routes/(public)/recover/+page.svelte
📝 Walkthrough

Walkthrough

Two components — AddKnowledgeSheet.svelte and the account recovery page — each gain a showAnswer boolean state, ViewIcon/ViewOffIcon imports, and an answer input that switches between password and text type. A right-aligned toggle button flips visibility and reflects the state via aria-label and aria-pressed.

Changes

Security Answer Input Masking

Layer / File(s) Summary
AddKnowledgeSheet answer masking
infrastructure/eid-wallet/src/routes/(app)/personal/components/AddKnowledgeSheet.svelte
Adds ViewIcon/ViewOffIcon imports, a showAnswer state reset to false on sheet open, and replaces the plain answer <input> with a masked input wrapped in a relative container plus an eye-toggle button with ARIA attributes.
Recover page answer masking
infrastructure/eid-wallet/src/routes/(public)/recover/+page.svelte
Adds view/view-off icon imports and a showAnswer reactive state flag, then reworks the unverified-answer step to wrap the input in a relative container, toggle type between password/text, and render an accessible eye-toggle button.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 Hop hop, the answer hides away,
Behind a dotted veil it stays.
An eye-toggle blinks, a button gleams,
No plain-text secrets spill their seams.
aria-pressed nods — accessibility wins!
The bunny masks all answered things. 🔒

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately describes the main change: masking the security-answer input with a reveal toggle, which is the primary objective of this PR.
Description check ✅ Passed The description follows the template structure with all required sections filled: issue number (#1012), type of change (Fix), testing details, and complete change checklist with all items checked.
Linked Issues check ✅ Passed The PR implementation fully addresses the objectives in issue #1012: masking the security answer input by default, adding a reveal toggle for usability, and ensuring consistent behavior across both the Restore screen and Knowledge sheet.
Out of Scope Changes check ✅ Passed All changes are scoped to the security answer masking feature: modifications to AddKnowledgeSheet.svelte and recover/+page.svelte with consistent implementation patterns, no unrelated alterations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/eid-wallet-mask-security-answer-input

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
infrastructure/eid-wallet/src/routes/(public)/recover/+page.svelte (1)

558-564: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reset showAnswer when entering the answer step for UX consistency.

When navigating to the unverified-answer step, answerInput, answerError, and lockedUntilLabel are reset, but showAnswer is not. This means if a user reveals the answer, navigates back, and returns, the input will remain in revealed state. For consistency with AddKnowledgeSheet.svelte (which resets showAnswer when the sheet opens at line 36), the answer should start masked when entering this step.

🔒 Suggested fix
 securityQuestion = question;
 recoveryEnvelopeId = latest.node.id;
 recoveredVaultUri = uri;
 answerInput = "";
 answerError = null;
 lockedUntilLabel = null;
+showAnswer = false;
 step = "unverified-answer";
🤖 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 `@infrastructure/eid-wallet/src/routes/`(public)/recover/+page.svelte around
lines 558 - 564, The `showAnswer` variable is not being reset when entering the
unverified-answer step, while other related variables like `answerInput`,
`answerError`, and `lockedUntilLabel` are being reset in this block. Add
`showAnswer = false;` (or the appropriate initial value) to the same reset block
where `step = "unverified-answer"` is set, to ensure the answer input returns to
a masked state when entering this step for UX consistency with
AddKnowledgeSheet.svelte.
🤖 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.

Outside diff comments:
In `@infrastructure/eid-wallet/src/routes/`(public)/recover/+page.svelte:
- Around line 558-564: The `showAnswer` variable is not being reset when
entering the unverified-answer step, while other related variables like
`answerInput`, `answerError`, and `lockedUntilLabel` are being reset in this
block. Add `showAnswer = false;` (or the appropriate initial value) to the same
reset block where `step = "unverified-answer"` is set, to ensure the answer
input returns to a masked state when entering this step for UX consistency with
AddKnowledgeSheet.svelte.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2f4d6b6f-e449-4eae-ba09-776ccad077f5

📥 Commits

Reviewing files that changed from the base of the PR and between 8890fad and 79a02a1.

📒 Files selected for processing (2)
  • infrastructure/eid-wallet/src/routes/(app)/personal/components/AddKnowledgeSheet.svelte
  • infrastructure/eid-wallet/src/routes/(public)/recover/+page.svelte

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security answer input should be masked on the account recovery (Restore) screen

1 participant