Skip to content

feat(auth): #28 OAuth messaging password — modal setup mode + unlock polish #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TortoiseWolfe merged 4 commits into from
May 7, 2026
Merged

feat(auth): #28 OAuth messaging password — modal setup mode + unlock polish #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ebab63
docs(speckit): #28 Phase A+B — clear wireframe gate, encode clarifica…
TurtleWolfe May 6, 2026
cb24211
docs(speckit): #28 Phase C — checklist + tasks + analysis cascade
TurtleWolfe May 6, 2026
5075e89
feat(auth): #28 modal-based OAuth messaging password setup + unlock p…
TurtleWolfe May 6, 2026
d75afc1
test(e2e): #28 add Playwright spec for OAuth Messaging Password modal
TurtleWolfe May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions features/auth-oauth/013-oauth-messaging-password/analysis.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
# Cross-Artifact Analysis: OAuth Messaging Password

**Generated**: 2026-05-06 by `/speckit.analyze` step
**Mode**: Read-only consistency check across [spec.md](./spec.md), [plan.md](./plan.md), [tasks.md](./tasks.md), and `.specify/memory/constitution.md` v1.0.2
**Note**: `scripts/constitution-check.py` is out-of-date (expects old `<feature>/spec/spec.md` layout; current layout is flat `<feature>/spec.md`). Manual analysis below.

---

## A. Summary

**Status**: PASS — all 6 categories below verify clean. No blocking issues. Implementation can proceed via `/speckit.implement`.

| Category | Result | Notes |
| --------------------------------- | ------- | ------------------------------------------------------------------------------------------------- |
| FR coverage in tasks.md | ✅ PASS | All 22 FRs traced to at least one task or marked as already-shipped. |
| User-story → task mapping | ✅ PASS | US-1 → T002–T008, US-2 → T009–T012, US-3 → T013–T015. Each story independently testable. |
| Plan ↔ tasks file-path alignment | ✅ PASS | All file paths in tasks.md match plan.md "Source code" tree. |
| Constitution compliance | ✅ PASS | All 6 principles verified (see Section E). |
| Wireframe coverage in tasks | ✅ PASS | Wireframe 01 (setup) drives T006 + T011 copy/layout. Wireframe 02 (unlock) drives T011. |
| Cascade discipline | ✅ PASS | All v1.0.2 cascade steps walked in order: specify → clarify → wireframe → plan → tasks → analyze. |

---

## B. FR-to-task traceability matrix

Every functional requirement maps to a task (or is marked already-shipped).

| FR | Statement | Task(s) | Status |
| ------ | -------------------------------------------------------------------- | ------------------------------------ | ----------------------------------------------------------------------- |
| FR-001 | Detect OAuth user | (already shipped) | `isOAuthUser` exists |
| FR-002 | Detect existing encryption keys | (already shipped) | `hasKeysForUser` exists |
| FR-003 | Distinguish OAuth from email/password users | (already shipped) | both helpers exist |
| FR-004 | Show "Create a Messaging Password" mode for OAuth users without keys | T006, T007 | New |
| FR-005 | Setup mode includes password + confirm fields | T006 | New |
| FR-006 | Validate passwords match | T006 + T004 (test) | New |
| FR-007 | Display password strength feedback | T006 (existing minLength=8 bound) | Reused |
| FR-008 | Explain why a separate messaging password is needed | T006 (warning copy per wireframe 01) | New |
| FR-009 | Show "Unlock Your Messages" mode for OAuth users with keys | T011 | Polish |
| FR-010 | Unlock mode shows single password field | (already shipped) | unchanged |
| FR-011 | Subtext explains password is separate from OAuth credentials | T011 | Polish |
| FR-012 | Identify OAuth provider | T006, T011 | Provider badge |
| FR-013 | Email user sees unchanged behavior | T013, T014 | Regression-tested |
| FR-014 | No OAuth-specific messaging to email users | T014 | Regression-tested |
| FR-015 | Form fields have proper labels | (already shipped) | preserved |
| FR-016 | Error messages announced to assistive tech | T005, T006 | Preserved + tested |
| FR-017 | Focus management on mode change | T005, T006 | Preserved + tested |
| FR-018 | NEVER persist messaging password | T006 | Enforced by construction (no setState/localStorage write of `password`) |
| FR-019 | NO recovery flow | (out of scope) | Spec.md Constraints |
| FR-020 | Visual provider badge from `getOAuthProvider(user)` | T006, T011 | New |
| FR-021 | Modal opens in setup mode inline; no redirect | T007 | EncryptionKeyGate flip |
| FR-022 | `/messages/setup` page kept as fallback | (no-change) | Plan.md NO-CHANGE annotation |

**No orphan FRs.** Every requirement has a home.

---

## C. User-story independence

Each story has a discrete acceptance test that can run in isolation:

- **US-1** (T008 checkpoint): OAuth user no-keys → modal setup mode → submit → routes to `/messages`. Independent of US-2 (no existing keys to unlock) and US-3 (different user).
- **US-2** (T012 checkpoint): OAuth user with keys + cleared IndexedDB → modal unlock mode → submit → routes to `/messages`. Independent of US-1 (different setup) and US-3.
- **US-3** (T015 checkpoint): Email user → modal unlock mode → byte-identical to pre-feature behavior. Independent of US-1/US-2.

**Shippable boundaries:**

- T001–T008 ship US-1 alone (the largest scope, MVP).
- T009–T012 add US-2 polish.
- T013–T015 pin US-3 invariants without code change (regression tests only).

---

## D. File-path alignment

Plan.md "Source code" tree files vs tasks.md file paths:

| File | Plan says | Tasks reference | Match |
| -------------------------------------------------------------------- | ---------------- | ------------------------ | ------------------ |
| `src/components/auth/ReAuthModal/ReAuthModal.tsx` | EXTEND | T006, T011 | ✅ |
| `src/components/auth/ReAuthModal/ReAuthModal.test.tsx` | EXTEND | T004, T010, T014 | ✅ |
| `src/components/auth/ReAuthModal/ReAuthModal.accessibility.test.tsx` | EXTEND | T005 | ✅ |
| `src/components/auth/ReAuthModal/ReAuthModal.stories.tsx` | grow 1→5 | T016 | ✅ |
| `src/components/auth/EncryptionKeyGate/EncryptionKeyGate.tsx` | MODIFY line 78 | T007 | ✅ |
| `src/app/messages/setup/page.tsx` | NO CHANGE except | T002 (one-line refactor) | ✅ (allowed minor) |
| `src/lib/messaging/welcome/send-welcome-message.ts` | NEW | T001 | ✅ |
| `tests/e2e/messaging/oauth-setup-modal.spec.ts` | NEW | T003, T009, T013 | ✅ |
| `src/lib/auth/oauth-utils.ts` | NO CHANGE | (no task touches) | ✅ |
| `src/services/messaging/key-service.ts` | NO CHANGE | (no task touches) | ✅ |

**One nuance**: Plan.md says `setup/page.tsx` is "NO CHANGE", but T002 makes a small refactor (extract welcome-message dispatch to the new helper). This is a minor inconsistency — the refactor is necessary so both modal AND page paths share the helper. **Resolution**: T002 is the cleaner-long-term move per memory rule; the plan should be read as "no behavior change" rather than "no diff at all". Already noted in plan.md "Source Decision" paragraph.

---

## E. Constitution v1.0.2 compliance

| Principle | Compliance | Where verified |
| ----------------------------------------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| I. Component Structure (5-file pattern) | ✅ | ReAuthModal already 5-file; tasks T004/T005/T016 update existing test/a11y/stories files in place. No new component dirs. |
| II. Test-First Development | ✅ | Tasks T003, T004, T005 (RED) precede T006 (GREEN). T009, T010 (RED) precede T011 (GREEN). T013, T014 (regression) ship as part of US-3 by design. |
| III. PRP Methodology + Mandatory Wireframe Gate | ✅ | Cascade order: spec.md (clarifications encoded) → wireframes (PASSED) → plan.md → tasks.md → analysis.md → next: implement. T018 schedules post-implement screenshots step. |
| IV. Docker-First Development | ✅ | Every task that runs commands prefixes with `docker compose exec scripthammer ...`. T008, T012, T015, T017, T018, T019 are all containerized. |
| V. Progressive Enhancement | ✅ | Modal already client-only; mobile-first preserved (44px touch targets, mobile breakpoint flagged in plan watch-out). No JS-required-for-core-flow regressions. |
| VI. Privacy & Compliance First | ✅ | FR-018 forbids password persistence in any client-side store. FR-019 keeps recovery flow out-of-scope. No new analytics events. |

---

## F. Risks & follow-ups

### Already-known watch-outs (from plan.md + session plan, all flagged)

1. Modal-setup mode mobile height — must verify in browser at 360×720 during T006/T018.
2. Credential Management API auto-fill differences in modal vs page — verify in Chrome + Firefox during T018 manual smoke.
3. EncryptionKeyGate hot-path edit — T007 must preserve email-user redirect to `/messages/setup` when they have no keys (only OAuth users get the modal setup path).
4. Welcome-message extraction (T001 + T002) — both modal-setup and page-setup paths must dispatch the welcome message; covered by extracting once and calling from both.

### New finding from analysis (low-impact)

- `scripts/constitution-check.py` expects an outdated `<feature>/spec/spec.md` layout. **Recommendation**: file a separate issue and PR to fix the script's `spec_dir = feature_dir / "spec"` to `spec_dir = feature_dir`. Not a blocker for #28; a one-line fix that benefits all future `/speckit.analyze` invocations. **Out of scope for this feature.**

---

## G. Recommendation

**Proceed to `/speckit.implement`.** No remediation needed. The cascade is intact, constitution is satisfied, FRs are traceable, user stories are independent, file paths align.
76 changes: 76 additions & 0 deletions .../auth-oauth/013-oauth-messaging-password/checklists/implementation-readiness.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Implementation Readiness Checklist: OAuth Messaging Password

**Purpose**: Validate plan.md + spec.md are complete, consistent, and unambiguous enough to drive `/speckit.tasks` and `/speckit.implement` without surprise rework.

**Created**: 2026-05-06
**Feature**: [spec.md](../spec.md) · [plan.md](../plan.md)
**Theme**: Implementation-readiness (depth: standard; audience: PR reviewer + LLM executing /speckit.implement)

---

## Scope clarity

- [x] Modal-vs-page primary path is explicit (FR-021, FR-022, plan.md "Source code" tree)
- [x] Setup-mode trigger condition is encoded in clarifications and FRs (`isOAuthUser(user) && !hasKeysForUser(user.id)`)
- [x] Email-user flow guaranteed unchanged (US-3 + FR-013, FR-014; regression-only Playwright spec named in plan)
- [x] /messages/setup page kept as fallback, not deleted (FR-022 + plan.md "NO CHANGE" line for setup/page.tsx)
- [x] Welcome-message dispatch extraction location explicit (`src/lib/messaging/welcome/send-welcome-message.ts`)
- [x] No new schema columns introduced (plan.md "No schema changes")

## Storage & security

- [x] Messaging password persistence is forbidden in unambiguous terms (FR-018 lists localStorage / sessionStorage / cookies / IndexedDB / "any other client-side store")
- [x] Recovery flow explicitly out of scope (FR-019 + spec.md Constraints)
- [x] Existing post-batch-8 invariant preserved: derived `CryptoKey` is non-extractable in IndexedDB (plan.md "Storage" section names it)
- [x] No new analytics or tracking events introduced (plan.md Constitution Check VI)

## UI conformance

- [x] Wireframe 01 (setup) and 02 (unlock) are signed off and linked from spec.md `## UI Mockup`
- [x] Provider badge text source named (`getOAuthProvider(user)`) and condition (`isOAuthUser(user)`)
- [x] WCAG AAA contrast preserved (plan.md Constitution Check; Phase 0 closure already enforces AAA)
- [x] 44px touch targets enforced (plan.md Constraints)
- [x] Mobile breakpoint constraint stated (360×720 with two-pw + confirm + badge fits) — flagged as a watch-out, must verify during implementation

## Test coverage

- [x] Unit-test list is concrete: setup-mode renders confirm field, unlock-mode does not, mismatch validation, submit branches by mode, badge conditional
- [x] Storybook story count specified (5: email-unlock, OAuth-Google setup/unlock, OAuth-GitHub setup/unlock)
- [x] New E2E spec path named (`tests/e2e/messaging/oauth-setup-modal.spec.ts`)
- [x] Existing E2E coverage retained (plan.md NO-CHANGE line for `tests/e2e/messaging/encrypted-messaging.spec.ts`)
- [x] Accessibility tests scoped (existing `.accessibility.test.tsx` runs against both modes)

## Cascade compliance

- [x] Wireframe gate (Constitution III) cleared before plan.md was written (validate.py PASSED 2026-05-06)
- [x] Clarifications session encoded with date heading per `/speckit.clarify` convention
- [x] No `[NEEDS CLARIFICATION]` markers remain in spec.md
- [x] All FRs reference concrete behavior, no vague adjectives
- [x] Constitution Check pass on all 6 principles documented in plan.md
- [x] Test-first ordering specified for `/speckit.tasks` (RED → GREEN → REFACTOR)

## Watch-outs flagged

- [x] Credential Management API behavior in modal vs page (cited as watch-out 2 in plan, plus session plan)
- [x] EncryptionKeyGate hot-path edit cited as watch-out 3 in session plan
- [x] Welcome-message extraction is the cleaner-long-term move (memory rule + session plan)
- [x] Local Supabase profile (`docker compose --profile supabase up`) named for manual smoke-test (memory rule)

---

## Validation Results

**Status**: PASSED — all items checked.

The plan and spec are concrete enough that `/speckit.tasks` can sequence the work
without re-asking decisions. Implementation is greenlighted.

## Notes

- Per `/speckit.checklist` "unit-tests-for-English" framing, this checklist
validates **requirements quality**, not code correctness. The actual code-
correctness gates live in the test suite (Vitest, Playwright, Pa11y) and run
via the constitution's TDD cycle in `/speckit.implement`.
- The original `requirements.md` checklist (PASSED 2025-12-30) covered spec
quality. This file covers the additional dimension of plan-readiness now that
plan.md exists.
Loading
Loading