pplate/plate-auth
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Page: Sprint 0 Plan Review v3
Pages
Architecture Home Integration Guide Migration InspectFlow Open Questions Roadmap Sprint 0 Assessment Sprint 0 Plan Review v2 Sprint 0 Plan Review v3 Sprint 0 Plan Review Sprint 0 Plan Sprint 0 Testplan Sprint 1 Assessment Sprint 1 Plan Sprint 1 Testplan Vision
Clone
1
Sprint 0 Plan Review v3
Patrick Plate edited this page 2026-06-24 21:22:07 +02:00
Unescape Escape
Table of Contents
This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

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 (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 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 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: 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 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 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 line 22 declares:

<artifactId>spring-boot-starter-web</artifactId>

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. 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 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 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() 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 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 §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

10. Plan Reviewer Quality Gate Verdict

Date: 2026-06-24 Reviewer: Roo (Plan Reviewer mode) Parent Session: 217dcb83-fb10-4706-8bd2-ae005f4767a8 Documents reviewed:

Code verification: Review v3 claims were spot-checked against actual source on feature/sprint-0/review-v2-fixes:

  • pom.xml line 22: spring-boot-starter-web confirmed (N1 real)
  • PlateAuthAutoConfiguration: @Import of 8 classes, no @ComponentScan (B2 fix confirmed)
  • SecurityConfig: 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 v2 (W10-2..W10-9 rewritten, W11-2 rewritten) and 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 lines 157, 166
ADV-2 Q09 still 🟠 Open — W9 uses tsup, W12-3 lists as "decided" Update Open-Questions.md Q09 to ✅
ADV-3 A5 says "16 security checklist items" — §9 has ~43 Update count in Sprint-0-Plan.md §10.5
ADV-4 Testplan paths stale (backend/src/test/, InspectFlow paths) Update 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 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.