General: Keep the passkey prompt closed after choosing "Set up later"#13121
Open
az108 wants to merge 1 commit into
Open
General: Keep the passkey prompt closed after choosing "Set up later"#13121az108 wants to merge 1 commit into
General: Keep the passkey prompt closed after choosing "Set up later"#13121az108 wants to merge 1 commit into
Conversation
The setup-passkey prompt is a singleton in the app shell that opens on every authentication-state emission. "Set up later" only hid it without recording the dismissal, so a later re-emission (e.g. after changing the AI experience) reopened it. Remember the dismissal for the current session and skip reopening. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
client
Contributor
WalkthroughThe setup-passkey-modal component now tracks a ChangesPasskey Modal Dismissal
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@src/main/webapp/app/course/overview/setup-passkey-modal/setup-passkey-modal.component.ts`:
- Around line 28-34: The session dismissal flag in
setup-passkey-modal.component.ts is not being cleared when the user logs out, so
a later login in the same SPA session can incorrectly skip the passkey prompt.
Update the logic around dismissedForCurrentSession in SetupPasskeyModalComponent
so it is reset when getAuthenticationState() emits undefined (logout), while
still preserving the current-session dismissal behavior for normal re-emissions
after a login.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c8f310f6-9455-4ff6-bf23-d278c1222870
📒 Files selected for processing (2)
src/main/webapp/app/course/overview/setup-passkey-modal/setup-passkey-modal.component.spec.tssrc/main/webapp/app/course/overview/setup-passkey-modal/setup-passkey-modal.component.ts
Comment on lines
+28
to
+34
| /** | ||
| * Set once the user dismisses the prompt during the current session (e.g. via "Set up later"). | ||
| * The modal is a singleton in the app shell, so this prevents it from reopening when the | ||
| * authentication state re-emits (e.g. after changing the AI experience). Reset on a full reload. | ||
| */ | ||
| private dismissedForCurrentSession = false; | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for logout implementation to check if it triggers a full page reload
rg -n -A10 -B2 'logout\s*\(' src/main/webapp/app/core/auth --type=ts
rg -n 'window.location' src/main/webapp/app/core/auth --type=tsRepository: ls1intum/Artemis
Length of output: 2563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== logout usages =='
rg -n -A4 -B4 '\.logout\(\)' src/main/webapp/app --type=ts
echo '== auth state sources =='
rg -n -A6 -B4 'getAuthenticationState\(' src/main/webapp/app --type=ts
rg -n -A6 -B4 'authenticationState|authenticationStateSubject|authState' src/main/webapp/app/core/auth --type=ts
echo '== any reload or location change around logout =='
rg -n -A4 -B4 'window\.location|location\.reload|document\.location|reload\(' src/main/webapp/app --type=tsRepository: ls1intum/Artemis
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the concrete logout flow and whether a reload is involved.
rg -n -A8 -B4 'logout\(' src/main/webapp/app --type=ts
rg -n -A6 -B4 'api/core/public/logout|clearCaches\(' src/main/webapp/app --type=ts
rg -n -A6 -B4 'navigate\(|navigateByUrl\(|window\.location|location\.reload|document\.location' src/main/webapp/app --type=tsRepository: ls1intum/Artemis
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'account.service.ts|authentication.*service.ts|auth.*service.ts' src/main/webapp/app/core/auth src/main/webapp/app/core -t fRepository: ls1intum/Artemis
Length of output: 428
Reset dismissedForCurrentSession on logout Logout keeps the SPA mounted, and getAuthenticationState() emits undefined on logout. Without clearing this flag here, a later login in the same session inherits the dismissal and suppresses the passkey prompt.
🤖 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
`@src/main/webapp/app/course/overview/setup-passkey-modal/setup-passkey-modal.component.ts`
around lines 28 - 34, The session dismissal flag in
setup-passkey-modal.component.ts is not being cleared when the user logs out, so
a later login in the same SPA session can incorrectly skip the passkey prompt.
Update the logic around dismissedForCurrentSession in SetupPasskeyModalComponent
so it is reset when getAuthenticationState() emits undefined (logout), while
still preserving the current-session dismissal behavior for normal re-emissions
after a login.
github-project-automation
Bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
az108
temporarily deployed
to
playwright-e2e-tests
GitHub Actions
Inactive
End-to-End Test Results
❌ Failed Tests (Phase 1)
Flakiness Scores for Failed Tests
Test Strategy: Two-phase execution
Overall: ❌ E2E tests failed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After choosing Set up later on the "Stay Secure with Passkeys" prompt, the prompt reappeared shortly after — e.g. when selecting the AI experience in the profile settings. The setup-passkey modal is a singleton in the app shell (
app.component) that opens on every authentication-state emission. Set up later only hid the dialog (visible = false) without recording that the user dismissed it, so the next authentication-state re-emission reopened it. This PR remembers the dismissal for the current session so the prompt no longer re-appears.Checklist
General
Client
Motivation and Context
Closes #13074.
Being re-prompted to set up a passkey immediately after explicitly declining ("Set up later") is confusing and annoying. The two dismissal actions have distinct intents:
localStorage(already worked).Only the first was actually being honored; Set up later recorded nothing.
Description
dismissedForCurrentSessionflag toSetupPasskeyModalComponent, set incloseModal()and checked first inopenIfNeeded(). Because the modal is a singleton in the app shell, the flag persists across navigation and intercepts any later authentication-state re-emission, so the prompt is not shown again during the session. It is reset naturally on a full page reload / new login (a new component instance).Remind Me in 30 Days(persisted 30-day suppression) is unchanged.closeModal(), emits the authentication state again, and asserts the prompt stays closed.Steps for Testing
Prerequisites:
earliestSetupPasskeyReminderDatein local storage)Screenshots
To be added.
Summary by CodeRabbit