pplate/plate-auth
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Page: Sprint 0 Plan Review
Pages
Architecture Home Integration Guide Migration InspectFlow Open Questions Roadmap Sprint 0 Assessment Sprint 0 Plan Review Sprint 0 Plan Sprint 0 Testplan Vision
Clone
1
Sprint 0 Plan Review
Patrick Plate edited this page 2026-06-24 15:08:47 +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: 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):

  1. 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.
  2. 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).