Ebbe BaßandClaude Sonnet 5.5
3fd8f27127
Profile page at /profile with first name, last name, display name, telephone and mobile read from AD and shown with the username and e-mail. Only changed fields are written through the directory adapter in one transaction with the audit entry (field names only); numbers are accepted in any common notation and stored as +country code and digits; names are required. Plain-language errors per field, German text and tests; review findings logged in the plan. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FocPJ75XPma7TcuYEMy8zm
25 lines
3.8 KiB
Markdown
25 lines
3.8 KiB
Markdown
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-platform-and-sign-in/story-audit-throttle-and-session-foundations-plan.md`
|
|
summary: Concurrent writers on the throttle counters are not tested.
|
|
evidence: The test database is SQLite in memory with a shared cache, which answers "table is locked" to a second writer at once, so a threaded test fails for reasons that say nothing about production (a WAL file database with a 20 s busy timeout and IMMEDIATE transactions). A real test needs a file database; tried and dropped in the refactor sweep.
|
|
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-account-self-service/story-password-change-plan.md`
|
|
summary: Code review 2026-10-19: the throttle check and the failure count are separate steps, so parallel requests can all pass the check before any failure is counted and more wrong binds reach AD than the limit allows.
|
|
evidence: Pre-existing design of the throttle foundation (AD-7); `portal/accounts/throttle.py` `check` and `hit`. Needs a reserve-then-confirm step to fix, for every endpoint at once.
|
|
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-account-self-service/story-password-change-plan.md`
|
|
summary: Code review 2026-10-19 (unverified, would be medium): a timeout after AD applied the password change is reported as "nothing changed", so sessions are not ended and no mail is queued.
|
|
evidence: Settle by checking whether `LdapDirectory._service` / `modify_s` turns a lost response into `DirectoryUnavailable`, and whether `verify_password` with the new password after a timeout would tell.
|
|
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-account-self-service/story-password-change-plan.md`
|
|
summary: Code review 2026-10-19: no signed-in page sends `Cache-Control: no-store`, so the back button can show a page after sign-out.
|
|
evidence: Pre-existing; only the health check uses `never_cache`. Decide once for every authenticated page (middleware), not per page.
|
|
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-account-self-service/story-password-change-plan.md`
|
|
summary: Code review 2026-10-19: AD-2 in the architecture spine names a `msDS-Lockout*` wildcard, but the adapter reads only `msDS-LockoutThreshold` and `msDS-LockoutObservationWindow`.
|
|
evidence: Fix edits the architecture document; reword AD-2 to name the two attributes.
|
|
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-account-self-service/story-password-change-plan.md`
|
|
summary: Known limitation, accepted in the code review of 2026-10-19: when AD never forgets failed attempts (a lockout observation window of 0), the portal's pause is still 15 minutes, so its limit does not keep up with AD's count.
|
|
evidence: Unusual AD setup; not the owner's. Revisit if a deployment sets the observation window to never.
|
|
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-account-self-service/story-password-change-plan.md`
|
|
summary: Known limitation, accepted in the code review of 2026-10-19: failed binds made elsewhere (authentik, other devices) count toward AD's lockout but not toward the portal's limit.
|
|
evidence: Already stated in the plan's design notes. Fixing it needs a new directory read of the bad-password count (new adapter surface).
|
|
- source_plan: `_bmad-output/initiative-ping-mee-portal/epic-account-self-service/story-profile-edit-plan.md`
|
|
summary: A `Forbidden` from the directory (service account lacks a right) is not mapped to a message in the password change either, so the user gets a raw 500 page.
|
|
evidence: Found reviewing profile edit; `LdapDirectory._service` raises `Forbidden` on INSUFFICIENT_ACCESS and `services.change_password` / `views.password` catch only DirectoryUnavailable, PasswordRejected and OutcomeUnknown. Pre-existing from the password change; fix in one place (AD-17) for every service.
|