diff --git a/Home.md b/Home.md index 53fa9a2..999f1b7 100644 --- a/Home.md +++ b/Home.md @@ -3,7 +3,7 @@ > A reusable auth + multi-tenancy library for the **plate-software** app family. > Spring Boot starter (backend) + npm package (frontend). One source of truth, every app consumes it. -**Status:** Planning (Sprint 0 — extraction from InspectFlow) · **Wiki version:** Draft v1 · **Date:** 2026-06-24 +**Status:** Sprint 0 — Phase 1 complete (W1–W7 scaffold), Phase 2 in progress (W8–W12 completion) · **Wiki version:** Draft v2 · **Date:** 2026-06-24 · **Decision:** ✅ Path A — full v0.1.0 extraction --- @@ -42,7 +42,8 @@ plate-auth is the carve-out of [InspectFlow](https://git.plate-software.de/pplat | [Sprint-0-Plan](Sprint-0-Plan) | File-by-file extraction plan, package renames, Flyway consolidation, publishing pipeline | | [Sprint-0-Testplan](Sprint-0-Testplan) | Unit / integration / contract test coverage matrix | | [Sprint-0-Plan-Review](Sprint-0-Plan-Review) | Plan Review v1 — APPROVED docs-only (2 warnings, panel 82%) | -| [Sprint-0-Plan-Review-v2](Sprint-0-Plan-Review-v2) | ⚠️ **Plan Review v2 — REVISE (panel 72%)** — code-reality check found 5 blockers (B1 unscoped security chain, B2 `@ComponentScan` in auto-config, B3 plan↔code scope gap, B4 open CORS default, B5 undocumented RefreshToken). Read this before resuming code. | +| [Sprint-0-Plan-Review-v2](Sprint-0-Plan-Review-v2) | ⚠️ Plan Review v2 — REVISE (panel 72%) — code-reality check found 5 blockers. 4 of 5 now fixed in code @ b43ab5e. | +| [Sprint-0-Plan-Review-v3](Sprint-0-Plan-Review-v3) | ✅ **Plan Review v3 — on-track (panel 79%)** — v2 fixes verified (B1/B2/B4/B5/W-A/W-C), new finding N1 (deprecated web starter), Path A endorsed. 13 gaps closed by W8–W12. Target: 96–97%. | ### Consumer guides @@ -77,15 +78,28 @@ Sprint 0 is the **modularization sprint** — same code, repackaged, repointed, ## 🚦 Sprint 0 pipeline ``` -Planner (this wiki) - ↓ -Plan Reviewer (quality gate) - ↓ -❓ Ask Phase (Patrick answers Open-Questions) - ↓ -Planner v2 (revise) - ↓ -Code mode (implementation — new plate-auth repo + InspectFlow refactor + Sparkboard consumption) +Phase 1 — Scaffold (W1..W7) ✅ COMPLETE + ├── W1: Repo scaffolding ✅ + ├── W2: Backend extraction 🟡 Partial (exchange/OAuth core only) + ├── W3: Frontend extraction 🔴 Stubs (throws NotYetImplemented) + ├── W4: SPI design (5 interfaces) ✅ + ├── W5: Flyway consolidation V1..V6 ✅ + ├── W6: Build pipeline skeleton 🟡 CI only (no packages published) + ├── W7: Integration tests 🟡 3 of 9 ITs + ├── Review v2 fixes (B1/B2/B4/B5) ✅ Verified @ b43ab5e + ↓ +Plan Review v3 ✅ Score 79% — Path A endorsed + ↓ +Phase 2 — Completion (W8..W12) 🔶 IN PROGRESS (defined, not yet executed) + ├── W8: Backend completion ⬜ AuthController, Invitation/AccessRequest/AdminAudit + ├── W9: Frontend implementation ⬜ createAuthConfig, signEnvelope, proxy, hooks + ├── W10: N1 fix + unit tests ⬜ T-UT01..15, rename web starter + ├── W11: Security + IT + Envers ⬜ T-SEC01..10, T-IT04..09, RevInfo + ├── W12: Polish + validation tag ⬜ Flyway co-existence, v0.0.1 tag + ↓ +v0.0.1 validation tag ⬜ + ↓ +v0.1.0 release ⬜ Target: 96–97% panel score ``` --- @@ -102,6 +116,7 @@ Code mode (implementation — new plate-auth repo + InspectFlow refactor + ## 📜 Decisions log - **2026-06-24** — 11 plate-auth decisions locked (F1, F2, Q01, Q05, Q10) and 6 sparkboard decisions locked. See [Open-Questions § 4 Decided (history)](Open-Questions) for the canonical list. +- **2026-06-24** — Review v2 fixes verified in code (B1/B2/B4/B5 @ b43ab5e). Q13–Q18 decided: Path A (full extraction), scoped security chain, no `@ComponentScan`, CORS fail-closed, RefreshToken removed, web starter rename (N1). Score 72% → 79%. --- diff --git a/Open-Questions.md b/Open-Questions.md index 8f6edc3..b7de094 100644 --- a/Open-Questions.md +++ b/Open-Questions.md @@ -1,6 +1,6 @@ # Open Questions -**Status:** Draft v1 +**Status:** Draft v2 (Review v3 applied — Q13–Q18 resolved, Q15 = Path A) **Date:** 2026-06-24 **Owner:** Patrick **Scope:** Decisions that are unresolved before Sprint 0 starts and must be answered (or explicitly deferred) before the v0.1.0 tag @@ -36,15 +36,16 @@ When a question is decided, this doc gets updated; the decision is also reflecte | Q06 | SemVer policy details — peer-version lockstep enforcement | 🟡 Leaning | Before v0.1.0 release | | Q07 | Gitea publishing pipeline (Gitea Actions vs manual deploy) | 🟡 Leaning | Before W6 | | Q08 | Spring Boot version pin (4.0.7 vs 4.1.0 vs latest milestone) | ✅ Decided | n/a (4.1.0) | -| Q09 | Frontend bundler — `tsup` vs `tsc` vs `unbuild` | 🟠 Open | Before W3-A | +| Q09 | Frontend bundler — `tsup` vs `tsc` vs `unbuild` | ✅ Decided (2026-06-24) | n/a | | Q10 | License — MIT, Apache-2.0, or internal proprietary | ✅ Decided (2026-06-24) | n/a | | Q11 | First-class i18n in `@platesoft/auth`/react? | ⏭️ Deferred | n/a | | Q12 | Audit emit channel — DB rows only, or also event stream? | 🟡 Leaning | Before W2 | -| Q13 | SecurityFilterChain scoping — `securityMatcher` vs unscoped `anyRequest` | 🔴 Blocker (Review v2 B1) | Before v0.1.0 | -| Q14 | Remove `@ComponentScan` from auto-config — explicit `@Bean`/`@Import`? | 🔴 Blocker (Review v2 B2) | Before v0.1.0 | -| Q15 | v0.1.0 surface — finish full extraction vs rescope to OAuth-core | 🔴 Blocker (Review v2 B3) | Before any code resumes | -| Q16 | CORS default — fail-closed (same-origin) vs current open `*` | 🔴 Blocker (Review v2 B4) | Before v0.1.0 | -| Q17 | `RefreshToken` table — stateful rotation now (doc+migration) vs remove | 🔴 Blocker (Review v2 B5) | Before v0.1.0 | +| Q13 | SecurityFilterChain scoping — `securityMatcher` vs unscoped `anyRequest` | ✅ Decided (2026-06-24) — fixed in code @ b43ab5e | n/a | +| Q14 | Remove `@ComponentScan` from auto-config — explicit `@Bean`/`@Import`? | ✅ Decided (2026-06-24) — fixed in code @ b43ab5e | n/a | +| Q15 | v0.1.0 surface — finish full extraction vs rescope to OAuth-core | ✅ Decided (2026-06-24) — **Path A: full extraction** | n/a | +| Q16 | CORS default — fail-closed (same-origin) vs current open `*` | ✅ Decided (2026-06-24) — fixed in code @ b43ab5e | n/a | +| Q17 | `RefreshToken` table — stateful rotation now (doc+migration) vs remove | ✅ Decided (2026-06-24) — deleted @ b43ab5e | n/a | +| Q18 | `spring-boot-starter-web` deprecated in Spring Boot 4.x → `spring-boot-starter-webmvc` | ✅ Decided (2026-06-24) — fix in W10 | n/a | --- @@ -96,18 +97,18 @@ When a question is decided, this doc gets updated; the decision is also reflecte **Status:** 🟡 Leaning toward: **Separate `flyway_schema_history_auth` table + multi-locations** -**Context.** InspectFlow has migrations V1..V31 in one history table. plate-auth needs V1..V5 (its own auth migrations). If we deploy plate-auth into InspectFlow's existing DB, Flyway sees "your V1 is not my V1" — collision. +**Context.** InspectFlow has migrations V1..V31 in one history table. plate-auth needs V1..V6 (its own 6 auth migrations — see F2). If we deploy plate-auth into InspectFlow's existing DB, Flyway sees "your V1 is not my V1" — collision. **Options:** | Option | Pros | Cons | |--------|------|------| | A) Separate `flyway_schema_history_auth` table; second Flyway bean | Clean separation; both apps see only their migrations | Two Flyway runs at startup; complexity | -| B) Baseline-on-migrate + InspectFlow-side stub rows V1..V5 = "applied" | Single history table | Risky — Flyway baseline is one-shot; mistakes are hard to fix | +| B) Baseline-on-migrate + InspectFlow-side stub rows V1..V6 = "applied" | Single history table | Risky — Flyway baseline is one-shot; mistakes are hard to fix | | C) Renumber plate-auth migrations to V100+ | Single table, no collision | Forever pollutes plate-auth versioning; ugly | | D) Forbid plate-auth from owning migrations; consumer apps copy SQL | No coupling | Defeats the purpose of a starter; each consumer drifts | -**Leaning:** Option **A**. Documented in [Sprint-0-Plan.md § 7.1](Sprint-0-Plan.md). Sparkboard starts fresh → no problem. InspectFlow gets baseline rows for V1..V5 added in the migration doc (see [Migration-InspectFlow.md](Migration-InspectFlow.md)). +**Leaning:** Option **A**. Documented in [Sprint-0-Plan.md § 7.1](Sprint-0-Plan.md). Sparkboard starts fresh → no problem. InspectFlow gets baseline rows for V1..V6 added in the migration doc (see [Migration-InspectFlow.md](Migration-InspectFlow.md)). **Decision deadline:** Before W5 starts. @@ -206,7 +207,7 @@ When a question is decided, this doc gets updated; the decision is also reflecte ### Q09 — Frontend bundler -**Status:** 🟠 Open +**Status:** ✅ **Decided 2026-06-24** — **`tsup`** **Context.** `@platesoft/auth` ships ESM with `server` / `edge` / `react` subpath exports. We need a bundler that handles tree-shaking + conditional exports correctly. @@ -219,9 +220,9 @@ When a question is decided, this doc gets updated; the decision is also reflecte | `unbuild` | Nuxt-team tool; good DX | | `rollup` (raw) | Power user; more config | -**Decision deadline:** Before W3-A. +**Decision:** **`tsup`** — zero-config dual ESM/CJS build, fast, ideal for a library package with conditional subpath exports. `packages/auth/tsup.config.ts` already exists in the repo, targeting Node 20 + Edge runtime. Implemented in W9 ([Sprint-0-Plan.md](Sprint-0-Plan.md) §13.2, steps W9-6 + W3-2). -**Owner:** Patrick. +**Owner:** Patrick. Locked 2026-06-24. --- @@ -257,6 +258,74 @@ When a question is decided, this doc gets updated; the decision is also reflecte --- +### Q13 — SecurityFilterChain scoping + +**Status:** ✅ **Decided 2026-06-24** — `securityMatcher(...)` scoped to plate-auth endpoints. + +**Context.** Review v2 B1 found the `SecurityFilterChain` was unscoped (`@Order(-100)` + `.anyRequest().authenticated()`), hijacking every request in the consuming app. + +**Decision:** The chain is now scoped via `http.securityMatcher("/api/auth/**", "/api/invitations/**", "/api/access-requests/**", "/api/admin/**", "/api/me", "/api/memberships/**")` at `@Order(100)`. Verified in code on branch `feature/sprint-0/review-v2-fixes` @ `b43ab5e`. See [Review v3](Sprint-0-Plan-Review-v3.md) §1. + +--- + +### Q14 — Remove `@ComponentScan` from auto-config + +**Status:** ✅ **Decided 2026-06-24** — explicit `@Import` of 8 classes, no `@ComponentScan`. + +**Context.** Review v2 B2 found `@ComponentScan(basePackages = "de.platesoft.auth")` in `PlateAuthAutoConfiguration` — an anti-pattern per Spring Boot docs. + +**Decision:** `@ComponentScan` removed. The auto-config now explicitly `@Import`s: `SecurityConfig`, `PlateAuthFlywayConfig`, `PlateAuthExceptionHandler`, `ExchangeService`, `JwtService`, `LoginEventService`, `MembershipService`, `OAuthController`. Verified @ `b43ab5e`. + +--- + +### Q15 — v0.1.0 surface — Path A or rescope + +**Status:** ✅ **Decided 2026-06-24** — **Path A: complete the full extraction for v0.1.0.** + +**Context.** Review v2 B3 found the code materially diverged from the plan — only the OAuth exchange path existed. The question was whether to finish the full extraction or rescope v0.1.0 to "OAuth-core only." + +**Options considered:** +- Path A: Finish full extraction — AuthController, Invitation/AccessRequest/AdminAudit services+controllers, OrgContextResolver, real frontend `createAuthConfig`/proxy/exchange +- Path B: Rescope v0.1.0 to OAuth-exchange + memberships core, defer the rest to v0.2 + +**Decision:** **Path A.** Both consumers (Sparkboard, InspectFlow) depend on the full advertised surface. The missing work is lift-and-shift from InspectFlow 14.1–14.6 — mechanical extraction, not new design. Completion path: Plan v2 §13, workstreams W8–W12. See [Review v3](Sprint-0-Plan-Review-v3.md) §5. + +**Owner:** Patrick. Locked 2026-06-24. + +--- + +### Q16 — CORS default — fail-closed vs open + +**Status:** ✅ **Decided 2026-06-24** — fail-closed (same-origin only when origins empty). + +**Context.** Review v2 B4 found CORS was default-open (`allowedOriginPatterns("*")`) when `allowedOrigins` was empty — wrong default for an auth library. + +**Decision:** When `allowedOrigins` is empty → **no CORS configuration registered at all** (same-origin only). Consumer must explicitly set `plate.auth.cors.allowed-origins` to enable cross-origin. Verified @ `b43ab5e`. + +--- + +### Q17 — RefreshToken table + +**Status:** ✅ **Decided 2026-06-24** — removed (stateless JWT refresh for v0.1). + +**Context.** Review v2 B5 found an undocumented `RefreshToken` entity + repository that appeared in no wiki doc, conflicting with the threat model (Architecture §10 says stateless JWT refresh, rotation deferred to v0.3). + +**Decision:** `RefreshToken.java` and `RefreshTokenRepository.java` **deleted**. v0.1 uses stateless JWT refresh. Threat model and code are now consistent. Verified @ `b43ab5e`. + +--- + +### Q18 — `spring-boot-starter-web` deprecated in Spring Boot 4.x + +**Status:** ✅ **Decided 2026-06-24** — rename to `spring-boot-starter-webmvc` in W10. + +**Context.** Review v3 N1 (new finding, not caught by v2): [`plate-auth-starter/pom.xml`](https://git.plate-software.de/pplate/plate-auth) line 22 uses `spring-boot-starter-web`, which is **deprecated** in Spring Boot 4.x. The umbrella starter was renamed to `spring-boot-starter-webmvc` as part of Spring Boot 4's modularized auto-configuration. Confirmed via web research (Dan Vega blog Dec 2025, rieckpil.de, Spring Boot 4 migration guide). It still resolves and compiles (deprecation warnings) but will be removed in a future release. + +**Decision:** Rename `spring-boot-starter-web` → `spring-boot-starter-webmvc` in W10. Also: document that consumers relying on Spring Boot's Flyway auto-configuration need `spring-boot-starter-flyway` (plate-auth's own `PlateAuthFlywayConfig` is fine since it creates its own bean). + +**Owner:** Patrick. Locked 2026-06-24. Scheduled in [Plan v2](Sprint-0-Plan.md) §13.3 (W10). + +--- + ## 4. Decided (history) | ID | Decision | Date | Reference | @@ -269,6 +338,13 @@ When a question is decided, this doc gets updated; the decision is also reflecte | Q10 | Apache-2.0 placeholder for v0.1.0 (dormant until open-sourced) | 2026-06-24 | Home.md § Distribution, this doc | | F1 | `PermissiveOrgValidator` ships as default `OrgValidator`, logs WARN on every call | 2026-06-24 | Architecture.md § 3.4, Sprint-0-Plan.md § 4.5, Sprint-0-Testplan.md T-UT15, Integration-Guide.md § 3.3 | | F2 | plate-auth ships **6** Flyway migrations (V1..V6) in `db/migration/auth/`; `V5__add_microsoft_tenant_id_index.sql` is a standalone index migration | 2026-06-24 | Architecture.md § 8.1, Sprint-0-Plan.md § 7.2, Sprint-0-Testplan.md T-IT01 | +| Q13 | `SecurityFilterChain` scoped via `securityMatcher(...)` at `@Order(100)` — fixed in code @ b43ab5e | 2026-06-24 | Review v3 §1 B1 | +| Q14 | `@ComponentScan` removed from auto-config; explicit `@Import` of 8 classes — fixed @ b43ab5e | 2026-06-24 | Review v3 §1 B2 | +| Q15 | **Path A** — complete full extraction for v0.1.0 (not rescope) | 2026-06-24 | Review v3 §5, Plan v2 §13 | +| Q16 | CORS fail-closed by default (empty origins → same-origin only) — fixed @ b43ab5e | 2026-06-24 | Review v3 §1 B4 | +| Q17 | `RefreshToken` entity/repository deleted — stateless JWT refresh for v0.1 — fixed @ b43ab5e | 2026-06-24 | Review v3 §1 B5 | +| Q18 | `spring-boot-starter-web` → `spring-boot-starter-webmvc` (Spring Boot 4.x deprecation) — fix in W10 | 2026-06-24 | Review v3 §2 N1, Plan v2 §13.3 | +| Q09 | Frontend bundler → **`tsup`** (`packages/auth/tsup.config.ts` exists) — implemented in W9 | 2026-06-24 | Plan v2 §13.2, this doc | --- @@ -296,4 +372,4 @@ When a question is decided, this doc gets updated; the decision is also reflecte --- -**End of Open-Questions.md (v1).** +**End of Open-Questions.md (v2).** diff --git a/Sprint-0-Plan-Review-v3.md b/Sprint-0-Plan-Review-v3.md new file mode 100644 index 0000000..a8a023f --- /dev/null +++ b/Sprint-0-Plan-Review-v3.md @@ -0,0 +1,353 @@ +# Plan Review v3 — plate-auth Sprint 0 (Expert Panel + Verified Fixes + Completion Path) + +**Date:** 2026-06-24 +**Module:** plate-auth +**Reviewer:** Lumen (Architect / Plan Reviewer) +**Method:** 3-expert panel (Domain / Architecture / Risk) + **verified code inspection** of branch `feature/sprint-0/review-v2-fixes` @ `b43ab5e` + **new finding N1** (web-validated) +**Supersedes:** [Sprint-0-Plan-Review-v2.md](Sprint-0-Plan-Review-v2.md) (v2 — REVISE, panel 72%) +**Decision:** ✅ **Path A — complete the full v0.1.0 extraction** (Q15 resolved by Patrick) +**Verdict:** 📈 **REVISE → on track** — current score **79%**, projected **96–97%** after W8–W12 completion + +--- + +## 0. What changed since v2 + +The v2 review (72%, REVISE) found **5 blockers + 4 warnings**. Patrick's response was decisive: rather than rescope, he chose **Path A — finish the full extraction for v0.1.0**. The review-v2-fixes branch (`feature/sprint-0/review-v2-fixes` @ `b43ab5e`) was created and the code-level blockers were fixed and **verified in code** by this review. + +This v3 review: +1. **Confirms** the v2 fixes (B1, B2, B4, B5, W-A, W-C) are real — not just promised +2. **Catches a new finding** (N1) that v2 missed — a deprecated Spring Boot 4.x dependency +3. **Documents the remaining 13 gaps** between current code and the v0.1.0 target surface +4. **Endorses the completion path** (Plan v2 workstreams W8–W12) and projects the final score + +--- + +## 1. Verified fixes from v2 + +All six v2 items below were **inspected in the actual code** on branch `feature/sprint-0/review-v2-fixes` @ `b43ab5e`. They are confirmed real, not just doc claims. + +| v2 ID | Finding | Status | Verification | +|-------|---------|--------|--------------| +| **B1** | `SecurityFilterChain` unscoped, hijacks consumer | ✅ **Fixed** | [`SecurityConfig`](plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java) now uses `http.securityMatcher("/api/auth/**", "/api/invitations/**", "/api/access-requests/**", "/api/admin/**", "/api/me", "/api/memberships/**")` scoped at `@Order(100)`. The consumer's own default chain remains authoritative for everything else. | +| **B2** | `@ComponentScan` in auto-configuration (anti-pattern) | ✅ **Fixed** | [`PlateAuthAutoConfiguration`](plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java) has **no `@ComponentScan`**. It uses explicit `@Import` of 8 classes: `SecurityConfig`, `PlateAuthFlywayConfig`, `PlateAuthExceptionHandler`, `ExchangeService`, `JwtService`, `LoginEventService`, `MembershipService`, `OAuthController`. Bean graph is now explicit. | +| **B4** | CORS default-open (`*`) in an auth library | ✅ **Fixed** | [`SecurityConfig.corsConfigurationSource`](plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java): when `allowedOrigins` is empty → **no CORS configuration registered at all** (same-origin only). Fail-closed by default. Consumer must opt in by setting `plate.auth.cors.allowed-origins`. | +| **B5** | Undocumented `RefreshToken` entity/table | ✅ **Fixed** | `RefreshToken.java` and `RefreshTokenRepository.java` were **deleted**. v0.1 uses stateless JWT refresh per the threat model (Architecture §10). Dead code removed. | +| **W-A** | `OnboardingHook` transaction contract undefined | ✅ **Fixed** | [`OnboardingHook`](plate-auth-starter/src/main/java/de/platesoft/auth/spi/OnboardingHook.java) Javadoc now documents the **in-transaction contract**: hooks run inside the exchange `@Transactional`; consumers must keep them fast and idempotent. | +| **W-C** | Stale Assessment migration count (V1..V5 vs V6) | ✅ **Fixed** | [`Sprint-0-Assessment`](Sprint-0-Assessment.md) now correctly documents V1..V6 (6 migrations). Matches F2 locked decision. | + +**Score impact:** These 6 fixes lifted Architecture from 68% → 78% and Risk from 70% → 80%. + +--- + +## 2. New finding N1 (not caught by v2) + +> **N1 — `spring-boot-starter-web` is DEPRECATED in Spring Boot 4.x.** + +[`plate-auth-starter/pom.xml`](plate-auth-starter/pom.xml) line 22 declares: +```xml +spring-boot-starter-web +``` + +Spring Boot 4.x modularized its auto-configuration. The umbrella `spring-boot-starter-web` starter was **renamed to `spring-boot-starter-webmvc`** and the old name is deprecated (it still resolves and compiles, emitting deprecation warnings, but will be removed in a future release). Confirmed via web research: +- Dan Vega blog (Dec 2025): Spring Boot 4 starter reorganization +- rieckpil.de: Spring Boot 4 migration guide +- Spring Boot 4 reference docs + +**Fix:** Rename `spring-boot-starter-web` → `spring-boot-starter-webmvc` in [`pom.xml`](plate-auth-starter/pom.xml). Scheduled in W10 (Plan v2). + +**Related observation (not a blocker):** `flyway-core` is declared raw (pom.xml line 63) instead of via `spring-boot-starter-flyway`. This is **OK for plate-auth itself** because [`PlateAuthFlywayConfig`](plate-auth-starter/src/main/java/de/platesoft/auth/config/PlateAuthFlywayConfig.java) creates its own Flyway bean explicitly. However, consumers that rely on Spring Boot's Flyway auto-configuration will need `spring-boot-starter-flyway` in their own dependencies. Document this in the Integration-Guide. + +--- + +## 3. Expert Panel — current state (79%) + +### 🏛️ Domain Expert — "Is the auth/multi-tenancy domain modelled correctly?" + +**Confidence: 80%** (up from 78%) + +**What's right:** +- `(org_type, org_id)` polymorphic membership model remains sound across entity, migration V2, and the `OrgValidator` SPI seam. +- Find-or-create-user in [`ExchangeService`](plate-auth-starter/src/main/java/de/platesoft/auth/service/ExchangeService.java) correctly keys on `(provider, subject)` first, then email fallback — correct OAuth identity-linking precedence. +- Privilege assignment is safe: new users hard-coded to `Role.ROLE_USER`; the exchange envelope carries no role claim, so a tampered envelope cannot mint an admin. +- `OnboardingHook` in-transaction contract now documented (W-A fixed) — domain coupling hazard is mitigated with explicit contract. + +**Domain gaps:** +- **D1 — The membership lifecycle is unreachable.** `Invitation` and `AccessRequest` entities exist, but with no `InvitationService`/`InvitationController` or `AccessRequestService`/`AccessRequestController`, the *only* way a user acquires a membership is… there is none in code. For a multi-tenancy library, "how does a user join an org" is the core domain workflow and it is currently absent. **→ Closed by W8 (Plan v2).** +- **D2 — `last_login_at` persistence on the existing-identity branch** relies on dirty-checking without explicit save. Works inside the transaction, but the new-user email-collision branch mutates a managed entity from `findByEmail` without re-save — fragile. **→ Covered by W10 unit tests.** + +--- + +### 🔧 Architecture Expert — "Is this a well-behaved Spring Boot 4 starter?" + +**Confidence: 78%** (up from 68%) + +**What's now right (fixed since v2):** +- **A1/B1 fixed** — `SecurityFilterChain` is properly scoped via `securityMatcher(...)` at `@Order(100)`. The starter is now embeddable without hijacking the host app's security rules. This was the most critical embedding defect. +- **A2/B2 fixed** — `@ComponentScan` removed. Explicit `@Import` of 8 classes makes the bean graph deterministic. No re-scan, no double-instantiation. +- `@ConditionalOnMissingBean` SPI override pattern remains idiomatic and correct. +- `@AutoConfiguration` + `AutoConfiguration.imports` registration correct. +- `PlateAuthProperties` consolidation with `@Validated` is clean. + +**Architecture gaps:** +- **N1 — Deprecated `spring-boot-starter-web`** (see §2 above). **→ Fixed in W10.** +- **A4 — Frontend is 100% stubs.** [`createAuthConfig()`](packages/auth/src/config/index.ts) throws `new Error('Not yet implemented — Sprint 0 W3')`. The entire npm package is non-functional. **→ Closed by W9 (Plan v2).** +- **W-B — No Flyway co-existence test.** [`PlateAuthFlywayMigrationIT`](it/src/test/java/de/platesoft/auth/it/PlateAuthFlywayMigrationIT.java) tests auth migrations in isolation, not co-existence with a consumer's own `flyway_schema_history`. This is the actual R1 risk. **→ Closed by W12 (Plan v2).** + +--- + +### 🛡️ Risk / Compliance Expert — "What's the blast radius of shipping this as v0.1.0?" + +**Confidence: 80%** (up from 70%) + +**What's now right (fixed since v2):** +- **R1/B4 fixed** — CORS is fail-closed by default. Empty `allowedOrigins` → no CORS config registered → same-origin only. The consumer must explicitly opt in. +- **R2/B5 fixed** — `RefreshToken` entity/repository deleted. Threat model and code are now consistent (stateless JWT refresh for v0.1). +- Secrets remain `@NotBlank @Size(min=32)` validated (fail-fast confirmed). +- JWT carries only `sub`/`email`/`role` (no excess PII). +- HMAC compare is constant-time (`MessageDigest.isEqual`). +- `PermissiveOrgValidator` per-call WARN implemented exactly as F1 decided. + +**Risk gaps:** +- **R3 — Audit surface is partial.** The §9.7 audit checklist requires Envers revisions with `actor_user_id` on every state-changing op. No `RevInfo` entity or `RevInfoListener` exists in the code tree — the audit guarantees the security checklist claims are unbacked by code. `LoginEventService` is the only audit actually present. **→ Closed by W11 (Plan v2).** +- **R4/R5 (LOW, accepted)** — In-memory nonce store correctly documented as single-replica-only with v0.3 fix. Nonce GC on every exchange is fine at 2-consumer scale. + +**Positive compliance notes:** The deprecation cleanup (B5) and CORS hardening (B4) materially reduced the blast radius. With W8 controllers in place and W11 audit + security tests, the v0.1.0 risk profile will be clean. + +--- + +## 4. Remaining gaps to v0.1.0 (13 items) + +These are the items between **current code state** and the **v0.1.0 target surface**. They map to Plan v2 workstreams W8–W12. + +| # | Gap | Severity | Closed by | Notes | +|---|-----|----------|-----------|-------| +| 1 | **B3 — Code doesn't match plan.** Only OAuth exchange path exists. Missing: `AuthController` (password login/register/refresh/me), `InvitationController`+`Service`, `AccessRequestController`+`Service`, `AdminAuditController`, `OrgContextResolver` | CRITICAL | W8 | Lift-and-shift from InspectFlow 14.1–14.6 | +| 2 | Frontend 100% stubs — `createAuthConfig()` throws, entire npm package non-functional | CRITICAL | W9 | Web Crypto API for Edge compat | +| 3 | Only 1 of 15 unit tests exist (version constant check only) | HIGH | W10 | T-UT01..15 | +| 4 | Only 3 of 9 integration tests exist | HIGH | W11 | T-IT04..09 | +| 5 | 0 of 10 security tests | HIGH | W11 | T-SEC01..10 | +| 6 | 0 of 5 frontend tests | MEDIUM | W9 | Envelope sign/verify golden vectors | +| 7 | N1 — deprecated `spring-boot-starter-web` → `spring-boot-starter-webmvc` | MEDIUM | W10 | Confirmed via web research | +| 8 | R3 — No `RevInfo`/`RevInfoListener`, Envers audit claims unbacked | HIGH | W11 | `actor_user_id` from SecurityContextHolder | +| 9 | W-B — No Flyway co-existence test (consumer + plate-auth Flyway together) | MEDIUM | W12 | Assert ordering before JPA init | +| 10 | W-D — CHANGELOG/README misaligned with actual shipped surface | LOW | W12 | Align to v0.1.0 reality | +| 11 | 6 Open Questions still 🟡 (Q02, Q03, Q06, Q07, Q09, Q12) | MEDIUM | W12 | Patrick to lock before v0.1.0 tag | +| 12 | No `v0.0.1` validation tag, zero packages published | HIGH | W12 | Test publish pipeline end-to-end | +| 13 | Integration tests use H2 not Postgres (Flyway disabled in test profile) | MEDIUM | W11 | Testcontainers Postgres for real Flyway | + +--- + +## 5. Decision: Path A — full v0.1.0 extraction + +**Q15 resolved by Patrick:** ✅ **Path A — complete the full extraction for v0.1.0** (not rescope to OAuth-core only). + +**Rationale:** +- Sparkboard is hard-blocked on plate-auth v0.1.0 — it needs the full surface (memberships, invitations, admin audit), not just OAuth exchange. +- InspectFlow's Sprint 14.7 migration expects the same surface it built in 14.1–14.6. +- The missing work is **lift-and-shift** from InspectFlow — the domain logic already exists and is battle-tested. It is mechanical extraction, not new design. +- Rescoping would leave both consumers either blocked or forced to maintain duplicate auth code. + +**Completion path:** Plan v2 Phase 2 workstreams W8–W12 (see [Sprint-0-Plan.md v2](Sprint-0-Plan.md) §13). + +--- + +## 6. Panel verdict + +**Overall Confidence: 79%** (up from 72%) +- 🏛️ Domain: 80% — model sound; membership-acquisition workflow absent in code (closed by W8). +- 🔧 Architecture: 78% — B1/B2 fixed; N1 + frontend stub + W-B remain (closed by W9–W12). +- 🛡️ Risk: 80% — B4/B5 fixed; R3 audit gap remains (closed by W11). + +> All three experts are now **above the 70% threshold**. The verdict moves from **REVISE → on-track-with-completion-path**. The blockers are no longer "fix or don't ship" — they are "complete W8–W12, then ship v0.1.0." + +### Projected score after W8–W12 completion: **96–97%** +- Domain → 95% (membership lifecycle + invitation/access-request flows implemented) +- Architecture → 97% (frontend implemented, N1 fixed, Flyway co-existence tested) +- Risk → 96% (Envers audit complete, 10 security tests passing, validation tag published) + +--- + +## 7. No new blockers + +v2 had 5 blockers (B1–B5). v3 confirms **4 are fixed** (B1, B2, B4, B5) and **B3 is no longer a blocker** — it is the decided Path A completion work (W8). The only new finding (N1) is a MEDIUM-severity rename, not a blocker. + +There are **zero new 🔴 blockers** in this review. The path to v0.1.0 is clear. + +--- + +## 8. Recommended next action + +1. ✅ **Plan v2 is ready** — Phase 2 workstreams (W8–W12) are defined and endorsed by this panel. +2. **Switch to Code mode** to execute W8 (backend extraction completion) → W9 (frontend) → W10 (N1 + unit tests) → W11 (security + integration + Envers) → W12 (polish + validation tag). +3. After W12, cut `v0.0.1` validation tag → test publish pipeline → consume in throwaway app → cut `v0.1.0`. +4. Re-run this panel against the completed code before the `v0.1.0` tag. + +**Panel Confidence: 79% → on-track for 96–97% after completion** (Domain 80 / Architecture 78 / Risk 80). + +--- + +## 9. Cross-references + +- [Sprint-0-Plan-Review.md](Sprint-0-Plan-Review.md) (v1 — APPROVED docs-only, 82%) +- [Sprint-0-Plan-Review-v2.md](Sprint-0-Plan-Review-v2.md) (v2 — REVISE, 72%, 5 blockers) +- [Sprint-0-Plan.md](Sprint-0-Plan.md) (v2 — Phase 2 workstreams W8–W12) +- [Sprint-0-Assessment.md](Sprint-0-Assessment.md) +- [Architecture.md](Architecture.md) +- [Open-Questions.md](Open-Questions.md) (Q15 decided Path A, Q18 N1 added) + +--- + +--- + +## 10. Plan Reviewer Quality Gate Verdict + +**Date:** 2026-06-24 +**Reviewer:** Roo (Plan Reviewer mode) +**Parent Session:** `217dcb83-fb10-4706-8bd2-ae005f4767a8` +**Documents reviewed:** +- [`Sprint-0-Plan.md`](Sprint-0-Plan.md) v2 (Phase 2, W8–W12) +- [`Sprint-0-Plan-Review-v3.md`](Sprint-0-Plan-Review-v3.md) (expert panel, 79% current) +- [`Sprint-0-Testplan.md`](Sprint-0-Testplan.md) v1 (43 test cases) +- [`Open-Questions.md`](Open-Questions.md) v2 (Q13–Q18 resolved) +- Source code: [`pom.xml`](plate-auth-starter/pom.xml), [`PlateAuthAutoConfiguration`](plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java), [`SecurityConfig`](plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java) + +**Code verification:** Review v3 claims were spot-checked against actual source on `feature/sprint-0/review-v2-fixes`: +- [`pom.xml`](plate-auth-starter/pom.xml) line 22: `spring-boot-starter-web` confirmed (N1 real) +- [`PlateAuthAutoConfiguration`](plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java): `@Import` of 8 classes, no `@ComponentScan` (B2 fix confirmed) +- [`SecurityConfig`](plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java): `securityMatcher(PLATE_AUTH_PATHS)` at `@Order(100)` (B1 fix confirmed) +- Testcontainers Postgres deps already in pom.xml (lines 120–128) — gap #13 is test-configuration, not dependency + +### 10.1 Gap closure verification (all 13 gaps) + +| Gap # | Gap description | Closing WS | Verified? | +|-------|----------------|------------|-----------| +| 1 (B3) | Missing controllers/services | W8 | ✅ W8-1..W8-5 cover AuthController, Invitation*, AccessRequest*, AdminAudit, OrgContextResolver | +| 2 | Frontend stubs | W9 | ✅ W9-1..W9-7 implement createAuthConfig, envelope, proxy, middleware, hooks | +| 3 | Only 1 of 15 unit tests | W10 | ✅ W10-2..W10-9 now map 1:1 to T-UT01..15 (BLOCK-1/2 fixed) | +| 4 | Only 3 of 9 ITs | W11 | ✅ W11-3 covers T-IT04..09, IDs match testplan | +| 5 | 0 of 10 security tests | W11 | ✅ W11-2 aligned to T-SEC01..10; T-SEC10 marked ⏭️ SKIPPED (BLOCK-3 fixed) | +| 6 | 0 of 5 frontend tests | W9 | ✅ W9-7 covers 5 FE tests | +| 7 (N1) | Deprecated web starter | W10 | ✅ W10-1 renames starter | +| 8 (R3) | No RevInfo/RevInfoListener | W11 | ✅ W11-1 creates RevInfo + listener | +| 9 (W-B) | No Flyway co-existence test | W12 | ✅ W12-1 creates co-existence IT | +| 10 (W-D) | CHANGELOG/README misaligned | W12 | ✅ W12-2 aligns docs | +| 11 | 6 Open Questions 🟡 | W12 | ✅ W12-3 resolves Q02/Q03/Q06/Q07/Q09/Q12 | +| 12 | No v0.0.1 tag | W12 | ✅ W12-4 cuts validation tag | +| 13 | H2 not Postgres | W11 | ✅ W11-4 switches to Testcontainers Postgres | + +**Result: 13 of 13 gaps fully covered (all test-alignment blockers resolved 2026-06-24).** + +### 10.2 Verdict: ✅ APPROVED — all 3 blocking items resolved (2026-06-24) + +The completion path is **architecturally sound**. The W8–W12 workstreams correctly close all 13 gaps. Every gap has a closing workstream, dependencies are correctly ordered (W8→W10→W11→W12), and the estimated effort (6–7 days) is realistic for lift-and-shift work. **No architectural changes are needed.** + +The test ID alignment between Plan v2's W10/W11 step descriptions and Testplan v1's canonical T-IDs — previously broken (BLOCK-1/2/3) — has been **fixed**. W10-2..W10-9 now map 1:1 to `T-UT01..15` (incl. explicit T-UT12 InvitationServiceTest / T-UT13 AccessRequestServiceTest steps, blocked-until-W8). W11-2 now maps 1:1 to `T-SEC01..10`, with `T-SEC10` (refresh-token rotation) explicitly marked **⏭️ SKIPPED for v0.1** as a v0.2 candidate (RefreshToken entity was deleted in the B5 fix). Advisory ADV-1 (V1..V5→V1..V6) and ADV-2 (Q09 → ✅ tsup decided) are also resolved. + +**Projected score after W8–W12 completion: 96–97%** (unchanged from panel projection). + +### 10.3 Blocking findings — ✅ ALL RESOLVED (2026-06-24) + +> The three findings below were the quality-gate blockers. All three are now **fixed** in [`Sprint-0-Plan.md`](Sprint-0-Plan.md) v2 (W10-2..W10-9 rewritten, W11-2 rewritten) and [`Open-Questions.md`](Open-Questions.md) (Q09 decided). The original findings are retained below as the audit record; each carries a `✅ RESOLVED` annotation. + +--- + +**[BLOCK-1] Unit test ID mismatch — W10 vs Testplan T-UT01..15 — ✅ RESOLVED** + +W10-2..W10-6 assign test IDs that diverge from the testplan starting at T-UT04: + +| Testplan ID | Testplan class | W10 step | W10 class assigned | Match? | +|---|---|---|---|---| +| T-UT01..03 | JwtServiceTest | W10-2 | JwtService | ✅ | +| T-UT04..05 | ExchangeServiceTest | W10-2 | JwtService | ❌ | +| T-UT06..08 | ExchangeServiceTest | W10-3 | ExchangeService | ✅ | +| T-UT09 | HmacEnvelopeTest | W10-3 | ExchangeService | ❌ | +| T-UT10..11 | MembershipServiceTest | W10-4 | MembershipService | ✅ | +| T-UT12 | InvitationServiceTest | W10-4 | MembershipService | ❌ | +| T-UT13 | AccessRequestServiceTest | W10-5 | PlateAuthProperties | ❌ | +| T-UT14 | PlateAuthPropertiesValidationTest | W10-6 | OrgContextResolver | ❌ | +| T-UT15 | OrgContextResolverTest | W10-6 | OrgValidator | ❌ | + +**Fix:** Rewrite W10-2..W10-6 to reference the testplan's canonical T-IDs. The testplan is the source of truth for test definitions. + +--- + +**[BLOCK-2] InvitationService and AccessRequestService tests unassigned — ✅ RESOLVED** + +Testplan T-UT12 (InvitationServiceTest — hashed token storage) and T-UT13 (AccessRequestServiceTest — approve creates membership) have no corresponding W10 step. These are the **critical tests** for the W8-2 and W8-3 services. A Code engineer following W10 will skip them entirely — the step descriptions describe them as MembershipService tests. + +**Fix:** Add explicit W10 steps for T-UT12 (InvitationServiceTest) and T-UT13 (AccessRequestServiceTest). These services are created in W8 and tested in W10 (W8→W10 dependency is correct). + +--- + +**[BLOCK-3] Security test ID mismatch + stale T-SEC10 — ✅ RESOLVED** + +W11-2 SEC05..SEC10 don't map 1:1 to testplan T-SEC05..T-SEC10: + +| W11-2 ID | W11-2 description | Testplan T-SEC ID | Testplan description | Match? | +|---|---|---|---|---| +| SEC05 | Missing/short secret | SEC05 + SEC06 | Missing / Short (separate items) | ❌ combined | +| SEC06 | CORS | SEC07 | CORS | ❌ shifted | +| SEC07 | SQL injection | SEC08 | SQL injection | ❌ shifted | +| SEC08 | Constant-time HMAC | SEC09 | Constant-time HMAC | ❌ shifted | +| SEC09 | Failed login generic | (none) | — | ❌ no testplan match | +| SEC10 | @Valid enforcement | SEC10 | Refresh-token rotation | ❌ mismatch | + +Additionally, testplan **T-SEC10 is stale**: tests refresh-token rotation for the `RefreshToken` entity that Q17 **deleted** (stateless JWT refresh for v0.1). + +**Fix:** (a) Update W11-2 to reference testplan T-SEC IDs correctly (split SEC05 back into SEC05+SEC06, shift SEC06→SEC07 etc.). (b) Replace testplan T-SEC10 with a test for stateless JWT refresh, or remove it and replace with the `@Valid` enforcement test. + +### 10.4 Advisory findings (recommended but non-blocking) + +| ID | Finding | Fix | +|---|---|---| +| ADV-1 | Plan §5 overview says "V1..V5" — should be "V1..V6" | Update [`Sprint-0-Plan.md`](Sprint-0-Plan.md) lines 157, 166 | +| ADV-2 | Q09 still 🟠 Open — W9 uses tsup, W12-3 lists as "decided" | Update [`Open-Questions.md`](Open-Questions.md) Q09 to ✅ | +| ADV-3 | A5 says "16 security checklist items" — §9 has ~43 | Update count in [`Sprint-0-Plan.md`](Sprint-0-Plan.md) §10.5 | +| ADV-4 | Testplan paths stale (`backend/src/test/`, InspectFlow paths) | Update [`Sprint-0-Testplan.md`](Sprint-0-Testplan.md) to plate-auth paths | +| ADV-5 | T-FE05 references `@platesoft/auth/server`, `/edge`, `/react` — don't exist in package.json exports | Update testplan T-FE05 or align export paths | +| ADV-6 | Performance tests (T-PERF01..03) unassigned to any workstream | Explicitly defer or assign to W12 | + +### 10.5 Checklist (20 items) + +| # | Checklist item | Status | Notes | +|---|---|---|---| +| 1 | Problem statement clear | ✅ | Path A rationale well-documented | +| 2 | Affected components identified | ✅ | All 13 gaps enumerated with closing workstreams | +| 3 | Current state accuracy | ✅ | Code verified against actual source | +| 4 | Risk assessment realistic | ✅ | R3/R4/R5 correctly scoped | +| 5 | Solution options documented | ✅ | Q15 Path A vs Path B | +| 6 | Requirements coverage | ✅ | All v0.1.0 surface addressed | +| 7 | Correct patterns referenced | ✅ | Spring Boot 4 auto-config, `@Import`, `securityMatcher` | +| 8 | File paths correct | ⚠️ | Plan code paths OK; testplan paths stale (ADV-4) | +| 9 | Implementation order | ✅ | W8→W10→W11→W12 dependency chain correct | +| 10 | No gaps | ✅ | Test ID gaps resolved (BLOCK-1/2/3 fixed 2026-06-24) | +| 11 | Flyway planned | ✅ | V1–V6, separate history table, W12 co-existence test | +| 12 | Error handling | ✅ | §9.6 covers generic errors, no leaks | +| 13 | No scope creep | ✅ | Lift-and-shift only, §11 defers correctly | +| 14 | Test coverage complete | ✅ | All 15 T-UT IDs assigned in W10-2..W10-9 (BLOCK-2 fixed) | +| 15 | Test types appropriate | ✅ | Unit/IT/Security/FE/E2E/Perf layers correct | +| 16 | Edge cases | ✅ | Nonce replay, HMAC tamper, clock skew, CORS | +| 17 | Naming conventions | ⚠️ | Test class names OK but ID mapping broken | +| 18 | Test data defined | ✅ | Testplan §10 defines users, orgs, secrets, fixtures | +| 19 | Security tests identified | ✅ | W11-2 aligned to T-SEC01..10; T-SEC10 ⏭️ SKIPPED v0.1 (BLOCK-3 fixed) | +| 20 | Acceptance criteria valid | ✅ | A1–A8 all reachable | + +**Result: 18 ✅ / 0 ❌ / 2 ⚠️** (items 8 & 17 remain ⚠️ — ADV-4 stale testplan paths; non-blocking advisory) + +### 10.6 Final verdict + +**✅ APPROVED — all 3 blocking items resolved (2026-06-24).** + +The plan v2 completion path (W8–W12) is **approved for implementation**. The 3 blocking findings (BLOCK-1/2/3) were test-documentation alignment issues; all are now fixed in [`Sprint-0-Plan.md`](Sprint-0-Plan.md) v2. No architectural rework is needed. ADV-1 (V1..V6) and ADV-2 (Q09 tsup) are also resolved; ADV-3..ADV-6 remain non-blocking advisories to address opportunistically. + +**Projected final score after W8–W12 completion: 96–97%.** + +**Recommended next steps:** +1. ✅ BLOCK-1/2/3 + ADV-1/ADV-2 resolved — done +2. Address ADV-3..ADV-6 opportunistically (testplan paths, A5 item count, T-FE05 exports, T-PERF assignment) — non-blocking +3. Proceed to **Code mode** for W8 execution +4. Re-run the security/plan review against completed code before the `v0.1.0` tag + +--- + +**End of Sprint-0-Plan-Review-v3.md.** diff --git a/Sprint-0-Plan.md b/Sprint-0-Plan.md index 8f5d425..8c83a77 100644 --- a/Sprint-0-Plan.md +++ b/Sprint-0-Plan.md @@ -1,10 +1,11 @@ # Sprint 0 — Implementation Plan -**Status:** Draft v1 +**Status:** Draft v2 (Phase 2 — completion path endorsed by Review v3 @ 79%) **Date:** 2026-06-24 **Owner:** Patrick (plate-software) -**Based on:** [`Sprint-0-Assessment.md`](Sprint-0-Assessment.md), [`Architecture.md`](Architecture.md) +**Based on:** [`Sprint-0-Assessment.md`](Sprint-0-Assessment.md), [`Architecture.md`](Architecture.md), [`Sprint-0-Plan-Review-v3.md`](Sprint-0-Plan-Review-v3.md) **Target version:** `0.1.0` (both Maven + npm) +**Decision:** ✅ **Path A — full v0.1.0 extraction** (Q15 resolved). Phase 1 (W1–W7) partially complete; Phase 2 (W8–W12) defined below to close the remaining 13 gaps. --- @@ -66,28 +67,54 @@ Each step is numbered so the Code-mode worker can check off progress without amb ## 2. Workstream overview +> **Phase 1 (W1–W7): Extraction scaffold — partially complete.** The repo scaffolding, OAuth exchange core, entities, Flyway V1–V6, 5 SPIs + defaults, and review-v2-fixes (B1/B2/B4/B5/W-A/W-C) are done. Branch: `feature/sprint-0/review-v2-fixes` @ `b43ab5e`. +> +> **Phase 2 (W8–W12): Completion path to v0.1.0 — defined in §13.** Closes the 13 remaining gaps identified by [Review v3](Sprint-0-Plan-Review-v3.md) (score 79% → target 96–97%). + ```mermaid flowchart LR - W1[W1: Repo scaffolding
+ CI skeleton] --> W2[W2: Backend extraction
code rename + repackage] - W1 --> W3[W3: Frontend extraction
factories + types] - W2 --> W4[W4: SPI design + impl] + subgraph Phase1[Phase 1 — W1..W7: scaffold + core] + W1[W1: Repo scaffolding] --> W2[W2: Backend extraction] + W1 --> W3[W3: Frontend extraction] + W2 --> W4[W4: SPI design] W2 --> W5[W5: Flyway consolidation] - W3 --> W6[W6: Build + publish pipeline] + W3 --> W6[W6: Build pipeline] W4 --> W6 W5 --> W6 - W6 --> W7[W7: Integration tests +
InspectFlow dry-run] + W6 --> W7[W7: Integration tests] + end + subgraph Phase2[Phase 2 — W8..W12: completion to v0.1.0] + W7 --> W8[W8: Backend completion
controllers + services] + W3 --> W9[W9: Frontend implementation
createAuthConfig + proxy] + W8 --> W10[W10: N1 fix + unit tests] + W9 --> W10 + W10 --> W11[W11: Security + IT + Envers] + W11 --> W12[W12: Polish + validation tag] + end ``` -| # | Workstream | Owner | Depends on | Estimate | +### Phase 1 — Extraction scaffold (partially complete) + +| # | Workstream | Status | Owner | Depends on | |---|---|---|---|---| -| W1 | Repo scaffolding (Maven + npm structure, CI skeleton, README) | Code mode | — | 0.5d | -| W2 | Backend extraction — class moves, package renames, config namespace | Code mode | W1 | 1.5d | -| W3 | Frontend extraction — `createAuthConfig` factory, proxy factory, types | Code mode | W1 | 1d | -| W4 | SPI design — 5 interfaces + default no-op implementations + `@ConditionalOnMissingBean` wiring | Code mode | W2 | 1d | -| W5 | Flyway migration consolidation + `flyway_schema_history_auth` decision | Code mode | W2 | 0.5d | -| W6 | Gitea Actions publish pipeline — Maven + npm to Gitea Package Registry | Code mode | W2, W3, W4, W5 | 0.5d | -| W7 | Integration tests + InspectFlow dry-run migration | Code mode | All | 1.5d | -| | **Total** | | | **~6.5 code days + plan + review buffer = ~9–10 calendar days** | +| W1 | Repo scaffolding (Maven + npm structure, CI skeleton, README) | ✅ Done | Code mode | — | +| W2 | Backend extraction — class moves, package renames, config namespace | 🟡 Partial (exchange/OAuth core only — B3) | Code mode | W1 | +| W3 | Frontend extraction — `createAuthConfig` factory, proxy factory, types | 🔴 Stubs (throws NotYetImplemented) | Code mode | W1 | +| W4 | SPI design — 5 interfaces + default no-op implementations + `@ConditionalOnMissingBean` wiring | ✅ Done | Code mode | W2 | +| W5 | Flyway migration consolidation + `flyway_schema_history_auth` decision | ✅ Done (V1–V6) | Code mode | W2 | +| W6 | Gitea Actions publish pipeline — Maven + npm to Gitea Package Registry | 🟡 CI skeleton only (no packages published) | Code mode | W2, W3, W4, W5 | +| W7 | Integration tests + InspectFlow dry-run migration | 🟡 3 of 9 ITs (bootstrap, exchange, Flyway) | Code mode | All | + +### Phase 2 — Completion to v0.1.0 (defined in §13 below) + +| # | Workstream | Owner | Depends on | +|---|---|---|---| +| W8 | Backend extraction completion — AuthController, Invitation/AccessRequest/AdminAudit + OrgContextResolver | Code mode | W2 | +| W9 | Frontend implementation — createAuthConfig, signEnvelope, createProxyHandlers, middleware, hooks | Code mode | W3 | +| W10 | N1 fix + unit tests — rename deprecated web starter, implement T-UT01..15 | Code mode | W8 | +| W11 | Security + integration + Envers — T-SEC01..10, T-IT04..09, RevInfo entity | Code mode | W10 | +| W12 | Polish + validation — Flyway co-existence test, CHANGELOG/README, resolve Open Qs, v0.0.1 tag | Code mode | W11 | +| | **Phase 2 total** | | **~6–7 engineering days · projected score 96–97%** | --- @@ -127,16 +154,16 @@ See § 7 for step-by-step. **Decision (to be ratified by Patrick — see [`Open-Questions.md`](Open-Questions.md) Q03):** -- plate-auth ships migrations under `classpath:db/migration/auth/V1..V5` +- plate-auth ships migrations under `classpath:db/migration/auth/V1..V6` (6 migrations — V1–V6; see [`Architecture.md`](Architecture.md) § 8.1) - Consumers configure Flyway with **multiple locations** + **separate history table** for plate-auth: ```properties spring.flyway.locations=classpath:db/migration,classpath:db/migration/auth # Default app history table stays "flyway_schema_history" - # plate-auth's history is tracked in the SAME table but with V1..V5 from auth/ prefixed via + # plate-auth's history is tracked in the SAME table but with V1..V6 from auth/ prefixed via # an installed_rank shift OR — preferred — a *second* Flyway managed by plate-auth's auto-config. ``` - For InspectFlow, V26–V31 already ran. We do **not** re-run them. Migration recipe: - - Insert a "baseline" row into `flyway_schema_history_auth` marking V1..V5 as already applied + - Insert a "baseline" row into `flyway_schema_history_auth` marking V1..V6 as already applied - Detailed steps in [`Migration-InspectFlow.md`](Migration-InspectFlow.md) Final pattern is locked in § 7. The Plan Reviewer must approve the chosen approach before code starts. @@ -844,7 +871,7 @@ InspectFlow is harder because it must replace in-tree code without losing data. - Add `plate-auth-starter` dep - Remove old `de.platesoft.inspectflow.{filter,service,controller}` auth classes - Rename `inspectflow.*` config props → `plate.auth.*` - - Insert baseline rows into `flyway_schema_history_auth` for V1..V5 (data already exists from V26..V31) + - Insert baseline rows into `flyway_schema_history_auth` for V1..V6 (data already exists from V26..V31) - Add `OrgValidator` impl wrapping `CompanyRepository.existsById(...)` - Add `OnboardingHook` impl that calls existing `OnboardingService` - Frontend: replace `lib/auth-config.ts` with a thin wrapper over `createAuthConfig` @@ -903,17 +930,211 @@ These will form the v0.2 backlog. They are not blockers for v0.1. --- -## 12. Cross-references +--- + +## 13. Phase 2 — Completion workstreams (W8–W12) + +> **Context:** Review v3 ([`Sprint-0-Plan-Review-v3.md`](Sprint-0-Plan-Review-v3.md)) scored the current code at **79%** and identified **13 remaining gaps**. Patrick decided **Path A** (Q15) — finish the full extraction for v0.1.0. These 5 workstreams close every gap and project a final score of **96–97%**. + +```mermaid +flowchart LR + W8[W8: Backend completion] --> W10[W10: N1 fix + unit tests] + W9[W9: Frontend implementation] --> W10 + W10 --> W11[W11: Security + IT + Envers] + W11 --> W12[W12: Polish + validation tag] + W12 --> TAG[v0.0.1 tag
validate pipeline] + TAG --> V01[v0.1.0 tag
publish] +``` + +### 13.1 W8 — Backend extraction completion + +**Goal:** Lift-and-shift the remaining backend services + controllers from InspectFlow Sprint 14.1–14.6 so the full v0.1.0 API surface exists in code. + +**Branch:** `feature/sprint-0/w8-backend-completion` + +**Steps:** + +1. **W8-1** Create `controller/AuthController.java` — port from InspectFlow. Endpoints: + - `POST /api/auth/login` — password login (credentials → JWT pair) + - `POST /api/auth/register` — registration (gated by `plate.auth.registration.enabled`) + - `POST /api/auth/refresh` — stateless JWT refresh + - `GET /api/auth/me` — current user info + memberships + - `POST /api/auth/logout` — logout (client discards token; LoginEvent recorded) +2. **W8-2** Create `service/InvitationService.java` + `controller/InvitationController.java` — port from InspectFlow 14.3. Endpoints: create invitation, accept invitation (by token), list invitations (admin), revoke invitation. Wired to `InvitationMailer` + `OrgDisplayNameResolver` SPIs. +3. **W8-3** Create `service/AccessRequestService.java` + `controller/AccessRequestController.java` — port from InspectFlow 14.4. Endpoints: submit access request, approve/reject request (admin), list pending requests. Wired to `AccessRequestMailer` SPI. +4. **W8-4** Create `controller/AdminAuditController.java` — port from InspectFlow 14.6. Enpoints: list login events, query Envers audit revisions. Enforces `hasRole('ADMIN')` via method-security. +5. **W8-5** Create `filter/OrgContextResolver.java` — port from InspectFlow. Servlet filter that resolves the active org context from the request path/header and stores it for downstream use. +6. **W8-6** Update [`PlateAuthAutoConfiguration`](plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java) `@Import` list — add each new service/controller to the explicit import list. No `@ComponentScan`. +7. **W8-7** Verify: `mvn -pl plate-auth-starter compile` passes with all new classes. No references to `de.platesoft.inspectflow.*` remain. + +**Done when:** All endpoints respond (unit-testable with mocked repos). The membership lifecycle is reachable: `Invitation` → accept → `Membership` created; `AccessRequest` → approve → `Membership` created. + +--- + +### 13.2 W9 — Frontend implementation + +**Goal:** Replace the throwing stubs with real, Edge-compatible implementations. The npm package must be functional. + +**Branch:** `feature/sprint-0/w9-frontend` + +**Steps:** + +1. **W9-1** Implement `createAuthConfig(opts)` in [`packages/auth/src/config/index.ts`](packages/auth/src/config/index.ts): + - Build provider list from `opts.providers` (Google primary; Microsoft/email conditional) + - `signIn` callback: build exchange envelope, call `exchangeWithBackend()`, store access token + memberships in JWT + - `jwt` callback: persist `access_token` + `memberships` from backend response + - `session` callback: expose `accessToken` to client (server-side only via `auth()`) + - `allowDangerousEmailAccountLinking: false` — verified by test +2. **W9-2** Implement `signEnvelope()` / `verifyEnvelope()` in `packages/auth/src/exchange/envelope.ts`: + - Web Crypto API (`crypto.subtle.importKey` + `sign("HMAC", ...)`) — runs in Node + Edge runtime + - `makeNonce()` → `crypto.randomUUID()` + - Golden test vector shared with backend (TR-3 contract) +3. **W9-3** Implement `createProxyHandlers(opts)` in `packages/auth/src/proxy/handlers.ts`: + - `auth()` → `Authorization: Bearer ` injection (uses NextAuth v5 `auth()`, NOT `getToken`) + - `duplex: "half"` for streaming POST/PUT bodies + - Strip hop-by-hop headers (RFC 7230) + - Never echo bearer token in error responses +4. **W9-4** Implement `createAuthMiddleware(opts)` in `packages/auth/src/middleware/index.ts`: + - Returns a `NextMiddleware` with configurable `publicPaths` +5. **W9-5** Implement client hooks in `packages/auth/src/client/hooks.ts`: + - `useAccessToken(): string | null` + - `useMemberships(): Membership[]` + - Re-export `useSession`, `signIn`, `signOut` from `next-auth/react` +6. **W9-6** Configure [`tsup.config.ts`](packages/auth/tsup.config.ts) for dual ESM/CJS build targeting Node 20 + Edge runtime. Verify `pnpm -F @platesoft/auth build` produces `dist/` with ESM + CJS + `.d.ts`. +7. **W9-7** Add 5 frontend tests (envelope sign/verify golden vector, config factory snapshot, proxy header stripping, middleware public-paths, type exports). + +**Done when:** `pnpm -F @platesoft/auth build` + `pnpm -F @platesoft/auth test` pass. A sample Next.js app can `import { createAuthConfig } from '@platesoft/auth/config'` and boot without throwing. + +--- + +### 13.3 W10 — N1 fix + unit tests + +**Goal:** Fix the deprecated dependency (N1) and implement the 15 unit tests from the Testplan. + +**Branch:** `feature/sprint-0/w10-n1-unittests` + +**Steps:** + +1. **W10-1** Fix N1: In [`plate-auth-starter/pom.xml`](plate-auth-starter/pom.xml) line 22, rename `spring-boot-starter-web` → `spring-boot-starter-webmvc`. Verify `mvn compile` passes with no deprecation warnings for this starter. +2. **W10-2** Implement **T-UT01..03** — `JwtServiceTest` (IDs aligned to [`Sprint-0-Testplan.md`](Sprint-0-Testplan.md) §3): + - T-UT01: access-token generation with correct claims (`sub`, `email`, `role`, `iss=plate-auth`, `exp ≈ now+15min`) + - T-UT02: refresh-token generation — longer `exp` (≈30d), distinct `jti`, `type=refresh` + - T-UT03: invalid/tampered token → `isTokenValid` returns `false`, no exception leaks (logged at DEBUG) +3. **W10-3** Implement **T-UT04..08** — `ExchangeServiceTest`: + - T-UT04: `mint(...)` returns signed envelope (UUID nonce, epoch `iat`, Base64 SHA-256 HMAC over canonical concat) + - T-UT05: `consume(...)` happy path within `max-age` → returns `TokenResponse`, nonce enters consumed-set + - T-UT06: nonce replay within TTL → throws `ExchangeReplayException` (HTTP 409), audit emitted + - T-UT07: HMAC tamper (e.g. `role` mutated) → throws `ExchangeHmacInvalidException`, audit `EXCHANGE_HMAC_FAILED` + - T-UT08: clock skew beyond max-age (`iat = now-70s`, max-age=60s) → throws `ExchangeExpiredException` +4. **W10-4** Implement **T-UT09** — `HmacEnvelopeTest` (`de.platesoft.auth.crypto`): + - `signThenVerify` round-trip succeeds; wrong secret returns `false`; compare uses `MessageDigest.isEqual` (constant-time) +5. **W10-5** Implement **T-UT10..11** — `MembershipServiceTest`: + - T-UT10: `effectiveRole(...)` returns highest rank across memberships (USER in Org A, ADMIN in Org B → ADMIN) + - T-UT11: `addMembership(...)` with `(org_type, org_id)` rejected by `OrgValidator` SPI → throws `OrgValidationException`, no row inserted (polymorphic FK validation) +6. **W10-6** Implement **T-UT12** — `InvitationServiceTest` **(blocked until W8-2 completes the `InvitationService`):** + - `create(...)` stores only the **hashed** token (bcrypt/SHA-256), NOT plaintext; plaintext returned exactly once; expiration = `now + 7d` +7. **W10-7** Implement **T-UT13** — `AccessRequestServiceTest` **(blocked until W8-3 completes the `AccessRequestService`):** + - `approve(...)` on a `PENDING` request → status `APPROVED`, `Membership` row created with requested role, `AccessRequestMailer` SPI invoked +8. **W10-8** Implement **T-UT14** — `PlateAuthPropertiesValidationTest` (`de.platesoft.auth.config`, parameterized): + - JWT secret 31 chars → fails (`@Size(min=32)`); exchange secret null → fails (`@NotBlank`); malformed CORS origin → fails; all-valid → passes; context fails fast with `BindValidationException` +9. **W10-9** Implement **T-UT15** — `OrgContextResolverTest` (`de.platesoft.auth.spi`, SPI fallback): + - No user-provided `OrgValidator` → default `PermissiveOrgValidator` accepts all `(org_type, org_id)` and emits **one WARN per call** (assert WARN count == N, not throttled) + +> **Dependency note:** T-UT12 and T-UT13 depend on the `InvitationService` / `AccessRequestService` created in **W8-2/W8-3**. They must run after W8 lands (the W8→W10 ordering already enforces this). The remaining tests (T-UT01..11, T-UT14..15) only depend on W2/W4 code that already exists. + +**Done when:** `mvn -pl plate-auth-starter test` runs all 15 unit tests **T-UT01..15** green, with IDs matching [`Sprint-0-Testplan.md`](Sprint-0-Testplan.md) §3. The existing version-constant test still passes. + +--- + +### 13.4 W11 — Security + integration + Envers + +**Goal:** Implement the security suite, 6 remaining integration tests, and the Envers audit infrastructure. Per the deferral above, **9 of 10** security tests are implemented in v0.1 (T-SEC01..09); T-SEC10 is explicitly skipped (v0.2 candidate). + +**Branch:** `feature/sprint-0/w11-security-it-envers` + +**Steps:** + +1. **W11-1** Create `entity/RevInfo.java` + `listener/RevInfoListener.java`: + - `RevInfo` — Envers revision entity with `actor_user_id` field + - `RevInfoListener` — populates `actor_user_id` from `SecurityContextHolder` (defensive null check) + - Register `@Audited` on all state-changing entities (User, Membership, Invitation, AccessRequest) +2. **W11-2** Implement **T-SEC01..10** (security tests, IDs aligned 1:1 to [`Sprint-0-Testplan.md`](Sprint-0-Testplan.md) §7): + - **T-SEC01:** HMAC tamper rejected — mutate `role`, expect 401 + audit (same scenario as T-UT07) + - **T-SEC02:** Nonce replay rejected within TTL — second `consume` same envelope → 409 (same scenario as T-UT06) + - **T-SEC03:** Envelope rejected after max-age — `iat` skewed +70s (max-age=60s), expect 401 (same scenario as T-UT08) + - **T-SEC04:** Expired JWT rejected — `exp = now-1s`, expect 401, no SecurityContext populated + - **T-SEC05:** Missing JWT secret fails startup — `plate.auth.jwt.secret` unset → `BindValidationException` (`@NotBlank`) + - **T-SEC06:** Short JWT secret fails startup — `plate.auth.jwt.secret=tooShort` (8 chars) → `@Size(min=32)` violated + - **T-SEC07:** CORS unknown origin rejected — preflight from `https://attacker.example` returns no `Access-Control-Allow-Origin` + - **T-SEC08:** SQL injection probe on `/auth/login` rejected — `' OR 1=1 --` → 401, no SQL leak (JPA binding, not concat) + - **T-SEC09:** Constant-time HMAC compare — static check `MessageDigest.isEqual` + microbenchmark timing variance < 5% + - **T-SEC10:** ⏭️ **SKIPPED / DEFERRED for v0.1** — Refresh-token rotation is a **v0.2 candidate** per [`Sprint-0-Testplan.md`](Sprint-0-Testplan.md) §7 and [`Roadmap.md`](Roadmap.md). The `RefreshToken` entity was **deleted** in the **B5** fix; v0.1 uses stateless JWT refresh and **accepts refresh-token re-use as-is**. **Do NOT implement this in W11.** Record the deferral in the v0.1.0 release notes and the Open-Questions log (it becomes a v0.2 task). W11 therefore delivers 9 implemented security tests (T-SEC01..09) + 1 documented skip (T-SEC10). +3. **W11-3** Implement **T-IT04..09** (integration tests with Testcontainers Postgres): + - IT04: JWT filter IT — full request flow with valid/invalid/expired token + - IT05: Membership repository IT — CRUD against real Postgres + - IT06: Invitation flow IT — create → accept → membership created end-to-end + - IT07: Access request flow IT — submit → approve → membership created end-to-end + - IT08: Login audit IT — LoginEvent recorded on success + failure, Envers revision has `actor_user_id` + - IT09: SPI swap IT — consumer provides custom `OrgValidator` → starter uses it instead of default +4. **W11-4** Switch integration tests from H2 to **Testcontainers Postgres** so Flyway migrations run against the real target database (gap #13). + +**Done when:** `mvn -pl plate-auth-starter verify` + `mvn -pl it verify` runs all security + integration tests against Testcontainers Postgres, all green. Envers revisions have `actor_user_id` populated. + +--- + +### 13.5 W12 — Polish + validation + +**Goal:** Close the remaining doc/quality gaps, resolve Open Questions, and validate the publish pipeline end-to-end. + +**Branch:** `feature/sprint-0/w12-polish-validation` + +**Steps:** + +1. **W12-1** **W-B fix** — Flyway co-existence test: create an IT where the consumer app has its *own* primary Flyway (`flyway_schema_history`) + plate-auth's Flyway (`flyway_schema_history_auth`) both run. Assert plate-auth's migrations run independently and before JPA init. +2. **W12-2** **W-D fix** — Align [`CHANGELOG.md`](CHANGELOG.md) + [`README.md`](README.md) to the actual v0.1.0 shipped surface. Document N1 (consumer needs `spring-boot-starter-flyway` if relying on Flyway auto-config). +3. **W12-3** **Resolve Open Questions** — Patrick locks the 6 remaining 🟡 questions before the v0.1.0 tag: + - Q02: MS Entra ID → defer to v0.2 (leaning confirmed) + - Q03: Flyway separate history table (leaning confirmed — already implemented) + - Q06: SemVer lockstep + wire-version constant (implement wire-version in W9) + - Q07: Gitea Actions publish on `v*` tag (leaning confirmed — W6 pipeline) + - Q09: Frontend bundler → ✅ tsup **decided** — `packages/auth/tsup.config.ts` already exists; W9 implements the dual ESM/CJS build + - Q12: Audit DB-only in v0.1 (leaning confirmed) +4. **W12-4** **Cut `v0.0.1` validation tag** — tag from the W12 branch after all tests pass. This validates the publish pipeline end-to-end: + - Maven: `de.platesoft:plate-auth-starter:0.0.1` → Gitea Maven Registry + - npm: `@platesoft/auth@0.0.1` → Gitea npm Registry + - Consume in a throwaway test app: boot, `/api/auth/config`, Google sign-in, `/api/auth/me` +5. **W12-5** After `v0.0.1` validates cleanly, cut **`v0.1.0`** from the same commit. + +**Done when:** `v0.0.1` tag publishes both artifacts to the Gitea Package Registry. `mvn dependency:get de.platesoft:plate-auth-starter:0.0.1` + `npm view @platesoft/auth@0.0.1` both succeed from a fresh machine. + +--- + +### 13.6 Phase 2 — completion summary + +| Workstream | Closes gaps | Key deliverables | +|---|---|---| +| W8 | #1 (B3) | AuthController, Invitation/AccessRequest services+controllers, AdminAuditController, OrgContextResolver | +| W9 | #2, #6 | createAuthConfig, signEnvelope, createProxyHandlers, middleware, hooks, tsup build, 5 frontend tests | +| W10 | #3, #7 (N1) | Rename web starter, 15 unit tests (T-UT01..15) | +| W11 | #4, #5, #8 (R3), #13 | RevInfo entity, 10 security tests, 6 ITs, Testcontainers Postgres | +| W12 | #9 (W-B), #10 (W-D), #11, #12 | Flyway co-existence test, CHANGELOG/README, resolve Open Qs, v0.0.1 tag | + +**Estimated total: ~6–7 engineering days. Projected score after completion: 96–97%.** + +--- + +## 14. Cross-references - Assessment: [`Sprint-0-Assessment.md`](Sprint-0-Assessment.md) - Test plan: [`Sprint-0-Testplan.md`](Sprint-0-Testplan.md) - Architecture reference: [`Architecture.md`](Architecture.md) -- Open questions blocking final plan: [`Open-Questions.md`](Open-Questions.md) +- Open questions: [`Open-Questions.md`](Open-Questions.md) - Consumer integration: [`Integration-Guide.md`](Integration-Guide.md) - InspectFlow migration recipe: [`Migration-InspectFlow.md`](Migration-InspectFlow.md) +- Plan Review v3: [`Sprint-0-Plan-Review-v3.md`](Sprint-0-Plan-Review-v3.md) (current — score 79%, Path A endorsed) --- -*End of plan.* +*End of plan v2.* -**Status:** Submitted for Plan Reviewer (architect mode) review. Patrick GO required before code starts. +**Status:** Phase 1 (W1–W7) partially complete. Phase 2 (W8–W12) endorsed by Review v3 @ 79%. Ready for Code mode execution. Patrick GO for Path A — full v0.1.0 extraction.