5fb148a9eb
Add comprehensive QA evidence including manual testing reports, RLS isolation tests, API CRUD verification, JWT decoded claims, and auth evidence files. Include updated notepads with decisions, issues, and learnings from full-stack debugging sessions. Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
73 lines
2.9 KiB
Markdown
73 lines
2.9 KiB
Markdown
# Decisions — Club Work Manager
|
|
|
|
_Architectural choices and technical decisions made during implementation_
|
|
|
|
---
|
|
|
|
## Decision 1: Comma-Separated Clubs Format (2026-03-05)
|
|
|
|
### Context
|
|
Keycloak sends clubs as comma-separated UUIDs instead of JSON dictionary, but the system expected dictionary format `{"uuid": "role"}`.
|
|
|
|
### Decision: Accept Comma-Separated UUIDs, Lookup Roles from Database
|
|
|
|
**Rationale:**
|
|
- Cleaner claim format (less space in JWT)
|
|
- Single source of truth: Member table in database
|
|
- Decouples authorization from Keycloak configuration
|
|
- Simplifies Keycloak setup (no complex mapper needed)
|
|
|
|
**Implementation:**
|
|
1. TenantValidationMiddleware: Split comma-separated string, validate requested tenant
|
|
2. ClubRoleClaimsTransformation: Split clubs claim, lookup Member role from database
|
|
3. Both use synchronous `.FirstOrDefault()` for consistency and performance
|
|
|
|
**Implications:**
|
|
- Database must be accessible during authentication (fast query with indexed user + tenant)
|
|
- Role changes require database update (not JWT refresh)
|
|
- Members table is authoritative for role assignment
|
|
|
|
## Decision 2: Synchronous Database Query in IClaimsTransformation (2026-03-05)
|
|
|
|
### Context
|
|
Initially implemented async database query with `FirstOrDefaultAsync()` in IClaimsTransformation.
|
|
|
|
### Decision: Use Synchronous `.FirstOrDefault()` Query
|
|
|
|
**Rationale:**
|
|
- IClaimsTransformation.TransformAsync() must return Task<ClaimsPrincipal>
|
|
- Hot reload (dotnet watch) fails when making method async (ENC0098)
|
|
- Synchronous query is acceptable: single database lookup, minimal blocking
|
|
- Avoids async/await complexity in authentication pipeline
|
|
|
|
**Implications:**
|
|
- Slightly less async but prevents hot reload issues
|
|
- Query performance is milliseconds (indexed on ExternalUserId + TenantId)
|
|
- Error handling via try/catch returns null role (fallback behavior)
|
|
|
|
|
|
## Decision 3: Entity Framework Connection Interceptor Architecture (2026-03-05)
|
|
|
|
### Context
|
|
Attempted to set PostgreSQL session variable (`SET LOCAL app.current_tenant_id`) in `ConnectionOpeningAsync` but failed with "Connection is not open" exception.
|
|
|
|
### Decision: Move SQL Execution to ConnectionOpened Lifecycle Phase
|
|
|
|
**Rationale:**
|
|
- `ConnectionOpeningAsync` executes before actual connection establishment
|
|
- `ConnectionOpened` is guaranteed to have an open connection
|
|
- Synchronous execution is acceptable in ConnectionOpened callback
|
|
- Logging/validation can happen in ConnectionOpeningAsync without SQL
|
|
|
|
**Implementation:**
|
|
- ConnectionOpeningAsync: Only logs warnings about missing tenant
|
|
- ConnectionOpened: Executes SET LOCAL command synchronously
|
|
- Both methods check for tenant context, only opened executes SQL
|
|
|
|
**Implications:**
|
|
- Tenant isolation guaranteed at connection open time
|
|
- RLS (Row-Level Security) policies see correct tenant_id
|
|
- Error handling via try/catch with logging
|
|
- Synchronous operation in callback is expected pattern
|
|
|