pplate/plate-auth

Fork 0

Table of Contents

Plan Review: plate-auth Sprint 0 — Shared Auth Library Extraction

Summary
Reviewed Documents
Checklist

Assessment
Implementation Plan
Test Plan

Expert Panel Review

🏛️ Domain Expert — "Does extracting T1+T2 from InspectFlow leave InspectFlow's auth contract intact?"
🔧 Architecture Expert — "Are the 5 SPI seams properly factored? Is the wire-contract HMAC envelope sound? Is the polymorphic FK stable?"
🛡️ Risk/Compliance Expert — "What's the impact if InspectFlow can't migrate cleanly? Versioning + lockstep risk? Security gaps?"

Findings

⚠️ W-1 — OrgValidator default behavior inconsistency (non-blocking, must resolve before W4)
⚠️ W-2 — Migration count inconsistency: V5 vs V6 (non-blocking, must align before W5)

Traceability Matrix
Verdict

✅ APPROVED

Cross-references

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: plate-auth Sprint 0 — Shared Auth Library Extraction

Date: 2026-06-24 Module: plate-auth Reviewer: Roo (Plan Reviewer) Documents: Sprint-0-Assessment.md v1, Sprint-0-Plan.md v1, Sprint-0-Testplan.md v1 Verdict: ✅ APPROVED (with 2 warnings to resolve before code starts)

Summary

Sprint 0 plans the extraction of InspectFlow's Sprint 14.1–14.6 auth system into a standalone, versioned library (de.platesoft:plate-auth-starter:0.1.0 + @platesoft/auth@0.1.0). The planning is thorough, well-structured, and architecturally sound. All referenced InspectFlow source paths were verified to exist. The 5 SPI seams are correctly factored, the Flyway strategy is viable, and the testplan provides 43 test cases covering all 8 acceptance criteria. Two internal document inconsistencies must be clarified before code begins, but neither requires re-architecture.

Reviewed Documents

Document	Version	Rating
Sprint-0-Assessment.md	v1	✅
Sprint-0-Plan.md	v1	✅ (2 warnings)
Sprint-0-Testplan.md	v1	✅ (1 warning)
Architecture.md	v1	⚠️ Minor inconsistency
Vision.md	v1	✅
Roadmap.md	v1	⚠️ Minor inconsistency
Open-Questions.md	v1	✅
Integration-Guide.md	v1	⚠️ See finding W-1
Migration-InspectFlow.md	v1	✅

Checklist

Assessment

#	Check	Result	Notes
1	Problem statement complete	✅	Clear forcing function (Sparkboard). Matches Vision.md. Well-motivated "option 2" over copy-paste.
2	Affected components identified	✅	Class-by-class inventory with lines, coupling analysis, status classification (65% lift / 25% reshape / 10% leave).
3	Current state accurate	✅	All 23 referenced InspectFlow Java classes verified to exist at stated paths. All 6 Flyway migrations (V26–V31) verified. Frontend files (`auth-config.ts`, `exchange.ts`, `auth.ts`) verified.
4	Risk assessment realistic	✅	R1–R12 with proper likelihood/impact/mitigation. R5 (in-memory nonce store) correctly rated "Certain" with v0.3 mitigation. R1 (Flyway corruption) mitigated by separate history table.
5	Solution options evaluated	✅	Section 6 gives clear GO recommendation with 6 explicit constraints. Open-Questions Q01–Q12 properly deferred to Ask Phase.

Implementation Plan

#	Check	Result	Notes
6	All requirements covered	✅	Vision success criteria (§ "What success looks like") → workstreams W1–W7 → acceptance A1–A8. Every Vision goal traceable.
7	Correct patterns referenced	✅	Spring Boot 4 auto-config (`@AutoConfiguration`, `@ConditionalOnMissingBean`, `@ConfigurationProperties`), SPI with default no-ops, Flyway multi-location, NextAuth v5 `auth()` + Edge proxy. All correct for the stated tech stack.
8	File paths correct	✅	All InspectFlow source paths verified via filesystem. Package rename `de.platesoft.inspectflow.` → `de.platesoft.auth.` is valid (no naming collision with existing packages).
9	Implementation order logical	✅	W1 (scaffolding) → W2/W3 (parallel extraction) → W4 (SPI, depends on W2) → W5 (Flyway, depends on W2) → W6 (publish, depends on all) → W7 (integration, depends on all). Correct dependency graph.
10	No gaps in steps	⚠️	See finding W-1 (OrgValidator default inconsistency) and W-2 (migration count V5 vs V6). Both are document alignment issues, not missing steps.
11	Flyway migrations planned	✅	Separate `flyway_schema_history_auth` table. V26→V1, V27→V2, V28→V3, V29→V4, V31→V5. V30 stays in InspectFlow (correctly identified as T3). Second Flyway bean runs before JPA init.
12	Error handling planned	✅	`@NotBlank @Size(min=32)` on secrets → fail-fast at boot. SPI fail-fast message for missing `OrgValidator`. Generic "invalid credentials" on login failure. No stack traces in 401/403.
13	No scope creep	✅	Ground rule §1.3: "No new features. This sprint is an extraction." Deferred items explicitly listed in §11. HMAC wire-version constant (from Q06) is the only net-new protocol addition — justified for lockstep safety.

Test Plan

#	Check	Result	Notes
14	Coverage complete	✅	Every workstream has mapped tests. A1–A8 matrix in §9 shows full coverage. 43 test cases across 6 types.
15	Test types appropriate	✅	Unit (15) for service logic, Integration (9) for DB+auto-config, Frontend-Unit (5) for npm package, E2E (6) as InspectFlow regression, Security (10) for auth-specific threats, Performance (3) for baseline.
16	Edge cases covered	✅	Nonce replay (T-UT06), HMAC tamper (T-UT07), clock skew (T-UT08), short secret (T-SEC06), refresh rotation (T-SEC10), SQL injection probe (T-SEC08), constant-time compare (T-SEC09).
17	Test class naming correct	✅	Follows `<Feature>Test` / `<Feature>IT` convention. Method naming: `method_scenario_expected()`.
18	Test method naming correct	✅	Consistent `verbNoun_scenario_expectation()` pattern throughout.
19	Test data defined	✅	§10 defines users, orgs, secrets, time control (injected `Clock`), recording mailers. Fixtures in stated paths.
20	SSH tests identified	N/A	Not a PAISY project. No SSH testing applicable. E2E via InspectFlow Playwright suite serves as the live integration environment.

Expert Panel Review

🏛️ Domain Expert — "Does extracting T1+T2 from InspectFlow leave InspectFlow's auth contract intact?"

Confidence: 85%

Assessment: Yes. The extraction is non-lossy:

All InspectFlow auth entities (User, UserIdentity, Membership, Invitation, AccessRequest, LoginEvent) move to plate-auth with identical column names and JPA @Table annotations → Hibernate maps to the same physical tables.
T3 entities (Company, OnboardingService, TenantAutoMapService) stay in InspectFlow, connected via the 5 SPIs.
The OrgValidator SPI replaces the companyRepository.findById(...) call — InspectFlow implements it wrapping CompanyRepository.existsById(...). No behavioral change.
OnboardingHook SPI replaces the direct OnboardingService call in OAuthService → InspectFlow implements it by delegating to its existing OnboardingService.
The polymorphic FK design (memberships.org_type = 'COMPANY', memberships.org_id = <company_uuid>) was already designed into Sprint 14.2 — this is not a new pattern being introduced.

Risk noted: The existing fn_membership_org_fk() trigger (from V27) is dropped from plate-auth's migration. InspectFlow must retain this trigger in their own migration set or accept SPI-only validation. Migration-InspectFlow.md documents this correctly (§5.3 Step W5-3).

🔧 Architecture Expert — "Are the 5 SPI seams properly factored? Is the wire-contract HMAC envelope sound? Is the polymorphic FK stable?"

Confidence: 80%

SPI Assessment:

SPI	Factoring	Verdict
`OrgValidator`	Single method `exists(OrgType, UUID)` — minimal surface, clear contract	✅ Correct
`OrgDisplayNameResolver`	Single method `displayName(OrgType, UUID)` — pure presentation	✅ Correct
`InvitationMailer`	Single method `sendInvitation(...)` — side-effect isolated	✅ Correct
`AccessRequestMailer`	Two methods: `notifyAdmins` + `notifyRequester` — clear lifecycle events	✅ Correct
`OnboardingHook`	`onFirstSignIn` + optional `onSubsequentSignIn` — T3 escape hatch	✅ Correct

No SPI is over-broad (too many methods forcing implementation burden) or under-specified (requiring consumers to guess behavior). The @ConditionalOnMissingBean pattern is idiomatic Spring Boot 4.

HMAC Envelope Assessment: Sound. SHA-256 with ≥32-char key, constant-time compare (MessageDigest.isEqual), 60s freshness window, UUID nonce with 5-min dedup TTL. The wire-version constant (Q06 leaning) adds forward compatibility for envelope format changes. In-memory nonce store is the known single-replica limitation — documented and deferred to v0.3.

Polymorphic FK Assessment: Stable. The (org_type, org_id) pattern avoids a generic organizations table that would force cross-domain sync. Runtime validation via OrgValidator SPI is the right trade-off for a multi-consumer library. Optional DB trigger (consumer-defined) provides defense-in-depth for teams that want it.

Concern: See finding W-1 regarding the OrgValidator default behavior inconsistency.

🛡️ Risk/Compliance Expert — "What's the impact if InspectFlow can't migrate cleanly? Versioning + lockstep risk? Security gaps?"

Confidence: 82%

Migration Risk: Low. The rollback strategy (§10.4) is sound — revert pom.xml + redeploy old commit. Database schema is unchanged (same table names, same columns). The flyway_schema_history_auth baseline insertion for existing InspectFlow DBs is clearly documented in Migration-InspectFlow.md.

Lockstep Risk: Mitigated by wire-version constant + npm peer deps + single-repo single-tag release. The risk of version drift is real but the chosen mitigation (fail-fast on wire-version mismatch) is adequate for a 2-consumer internal library.

Security Gaps:

The security review checklist (§9, 10 sections, 30+ sub-items) is comprehensive.
No PII in JWT beyond email/userId/role — correct.
Refresh-token rotation is partially implemented (v0.1 keeps InspectFlow behavior) — documented for v0.3 hardening.
The PermissiveOrgValidator default (if it exists — see W-1) could be a security gap if a consumer forgets to implement OrgValidator and accidentally grants memberships to non-existent orgs. This is mitigated by the "WARN once on startup" behavior described in T-UT15, but should be elevated to a startup log at ERROR level.

Panel Confidence: 82% (above 70% threshold → does not trigger automatic REVISE)

Findings

⚠️ W-1 — OrgValidator default behavior inconsistency (non-blocking, must resolve before W4)

Documents affected: Sprint-0-Plan.md §4.5 W4-2, Integration-Guide.md §3.3, Sprint-0-Testplan.md T-UT15

Issue: Three documents disagree on whether a default OrgValidator exists:

Source	States
Plan §4.5 W4-2	"DefaultOrgValidator — does NOT exist as a default; auto-config requires the consumer to provide one if T2 endpoints are used."
Integration-Guide §3.3	"OrgValidator (default = `PermissiveOrgValidator`) — Accepts any `(org_type, org_id)` — replace via SPI"
Testplan T-UT15	"No user-provided `OrgValidator` bean; default `PermissiveOrgValidator` in effect"

Impact: If a consumer deploys without providing an OrgValidator, either (a) T2 endpoints fail-fast with a clear error (Plan's intent), or (b) any (org_type, org_id) is silently accepted (Guide/Testplan's intent). Option (b) is a potential security gap — memberships could be granted to non-existent orgs.

Recommendation: Resolve in Ask Phase. Suggested approach: ship PermissiveOrgValidator as default (for rapid prototyping) but log a WARN on every call stating "No OrgValidator configured — accepting all org references without validation. Provide a bean to enforce org existence checks." This balances DX for greenfield (Sparkboard can boot immediately) with visibility for production (logs scream until you implement it). Document the choice in Architecture.md.

⚠️ W-2 — Migration count inconsistency: V5 vs V6 (non-blocking, must align before W5)

Documents affected: Sprint-0-Plan.md §7.2, Architecture.md §8.1, Roadmap.md §v0.1

Issue:

Source	Migration count
Plan §7.2	5 migrations (V1–V5): V26→V1, V27→V2, V28→V3, V29→V4, V31→V5
Architecture §8.1	6 files listed: V1, V2, V3, V4, V5__add_microsoft_tenant_id_index.sql, V6__create_login_events
Roadmap §v0.1	"Flyway migrations under `classpath:db/migration/auth/V1..V6`"
Testplan T-IT01	"5 rows (V1..V5)"

Architecture's V5 (add_microsoft_tenant_id_index) appears to be an index on user_identities.tenant_id (T1 data), which is distinct from V30 (companies.microsoft_tenant_id, T3 data). If this is correct, the migration count should be 6 (V1–V6), and the Plan/Testplan need updating from "V5" to "V6".

Recommendation: Clarify: is there a separate index migration for user_identities.tenant_id? If yes → update Plan §7.2 and Testplan T-IT01 to say "V1..V6" and add V5 to the W5 step list. If no → update Architecture §8.1 to remove V5 and renumber V6→V5.

Traceability Matrix

Vision Success Criterion	Plan Workstream	Acceptance	Test IDs
New Spring Boot app + plate-auth = Google SSO in <30min	W1, W2, W4, W6	A4	T-IT02, T-IT03, T-E2E02
New Next.js app + @platesoft/auth = sign-in in <30min	W3, W6	A4	T-FE01, T-FE05
InspectFlow bit-for-bit reproducible from library	W2, W4, W5, W7	A3, A7	T-E2E01–06, T-IT01
Sparkboard adopts on day 1 without seeing InspectFlow code	W1, W3, W6	A4	Integration-Guide validation
SPI seams are clean	W4	A5	T-UT11, T-UT15, T-IT09
Exchange flow works artifact-to-artifact	W2, W3	A2	T-UT04–09, T-IT03, T-FE02, T-SEC01–03
Config namespace `plate.auth.*`	W2-B	A4	T-UT14, T-SEC05, T-SEC06
Frontend artifact bundles + ships ESM	W3-A	A6	T-FE01–05
No performance regression	All	A8	T-PERF01–03

Coverage gaps: None. Every Vision goal traces through at least one workstream, one acceptance criterion, and at least two test cases.

Verdict

✅ APPROVED

The Sprint 0 planning suite (Assessment + Plan + Testplan + supporting docs) is complete, correct, and feasible. The extraction approach (rename + repackage + 5 SPIs) is architecturally sound. The 43-test coverage matrix maps cleanly to all 8 acceptance criteria. The Flyway strategy (separate history table) avoids collision with existing consumer migrations. The security checklist (§9, 30+ items) exceeds the standard for a library wrapping authentication.

Conditions for GO (must resolve before code starts):

W-1: Decide the OrgValidator default behavior (PermissiveOrgValidator vs fail-fast) and align all 3 documents. Add to Open-Questions as Q13 or resolve in Ask Phase.
W-2: Confirm migration count (5 or 6) and align Plan §7.2, Architecture §8.1, Roadmap, and Testplan T-IT01.

Neither condition requires re-architecture or re-planning. Both are document alignment fixes resolvable in <1 hour during the Ask Phase.

Panel Confidence: 82% (Domain: 85%, Architecture: 80%, Risk: 82%)

Cross-references

End of Sprint-0-Plan-Review.md (v1).