feat(063-entra-signin): Clarify multi-tenant routing, disabled user login, and data model column sizing
This commit incorporates clarifications into the 063-entra-signin feature specification. Key clarifications include: - Multi-tenant login flow: Users with multiple memberships will be redirected to a dedicated chooser page. - Disabled user login: Logins for disabled/soft-deleted users will be blocked, and they will be redirected with a generic error. - Data model column sizing: and columns will be (or UUID type for PostgreSQL). These updates ensure a more precise and robust specification, covering critical UX, security, and data modeling aspects.
This commit is contained in:
Ahmed Darrazi
parent
eef85af990
commit
1ba410457a
35
specs/063-entra-signin/checklists/requirements.md
Normal file
35
specs/063-entra-signin/checklists/requirements.md
Normal file
@ -0,0 +1,35 @@
|
|||||||
|
# Specification Quality Checklist: Entra Sign-in (Tenant Panel) v1
|
||||||
|
|
||||||
|
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
||||||
|
**Created**: 2026-01-26
|
||||||
|
**Feature**: [specs/063-entra-signin/spec.md](specs/063-entra-signin/spec.md)
|
||||||
|
|
||||||
|
## Content Quality
|
||||||
|
|
||||||
|
- [x] No implementation details (languages, frameworks, APIs)
|
||||||
|
- [x] Focused on user value and business needs
|
||||||
|
- [x] Written for non-technical stakeholders
|
||||||
|
- [x] All mandatory sections completed
|
||||||
|
|
||||||
|
## Requirement Completeness
|
||||||
|
|
||||||
|
- [x] No [NEEDS CLARIFICATION] markers remain
|
||||||
|
- [x] Requirements are testable and unambiguous
|
||||||
|
- [x] Success criteria are measurable
|
||||||
|
- [x] Success criteria are technology-agnostic (no implementation details)
|
||||||
|
- [x] All acceptance scenarios are defined
|
||||||
|
- [x] Edge cases are identified
|
||||||
|
- [x] Scope is clearly bounded
|
||||||
|
- [x] Dependencies and assumptions identified
|
||||||
|
|
||||||
|
## Feature Readiness
|
||||||
|
|
||||||
|
- [x] All functional requirements have clear acceptance criteria
|
||||||
|
- [x] User scenarios cover primary flows
|
||||||
|
- [x] Feature meets measurable outcomes defined in Success Criteria
|
||||||
|
- [x] No implementation details leak into specification
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
|
||||||
|
- All validation checks passed. The specification is clear, complete, and ready for the planning phase.
|
||||||
|
- Three clarifications were incorporated: multi-tenant login flow, disabled user login behavior, and data model column sizing.
|
||||||
215
specs/063-entra-signin/spec.md
Normal file
215
specs/063-entra-signin/spec.md
Normal file
@ -0,0 +1,215 @@
|
|||||||
|
# Feature Specification: 063 — Entra Sign-in (Tenant Panel) v1
|
||||||
|
**Feature Branch**: `063-entra-signin`
|
||||||
|
**Created**: 2026-01-26
|
||||||
|
**Status**: Draft (v1)
|
||||||
|
**Scope**: Tenant Admin panel (`/admin`) sign-in only
|
||||||
|
---
|
||||||
|
## Context / Goal
|
||||||
|
TenantAtlas needs a clean, enterprise-grade sign-in flow for the **Tenant Admin panel** (`/admin`) using **Microsoft Entra ID (OIDC)**.
|
||||||
|
|
||||||
|
This feature MUST:
|
||||||
|
- provide **Entra-only** login UX on `/admin/login`
|
||||||
|
- upsert tenant users safely keyed by `(entra_tenant_id, entra_object_id)`
|
||||||
|
- route users after login based on **tenant memberships**
|
||||||
|
- keep pages **DB-only at render time**
|
||||||
|
- log safely (no tokens/claims dumps)
|
||||||
|
|
||||||
|
This feature intentionally does **not** change platform/operator access (`/system`) or break-glass behavior; those belong to Feature **064-auth-structure**.
|
||||||
|
---
|
||||||
|
## Clarifications
|
||||||
|
### Session 2026-01-26
|
||||||
|
- Q: For a user belonging to multiple tenants, what should happen immediately after they sign in? → A: Redirect to a dedicated, full-screen "tenant chooser" page that lists all their memberships. The user must click one to proceed to the main dashboard.
|
||||||
|
- Q: If a user record exists in a disabled state (e.g., `deleted_at` is not null, or an `is_active` flag is false), and that user completes a valid Entra ID sign-in, what should happen? → A: Block the login. Redirect the user to the login page with a generic error message (e.g., "Your account is disabled. Please contact an administrator.").
|
||||||
|
- Q: What should be the maximum character length for the `users.entra_tenant_id` and `users.entra_object_id` columns in the database schema? → A: `VARCHAR(36)`. Both Entra Tenant IDs and Object IDs are GUIDs, which have a standard fixed length of 36 characters.
|
||||||
|
---
|
||||||
|
## Existing System Compatibility (important)
|
||||||
|
The repository already contains:
|
||||||
|
- A **System Panel** at `/system` using guard `platform` and `platform_users` (platform operator access).
|
||||||
|
- Break-glass recovery mechanics for platform operators (banner/middleware/routes).
|
||||||
|
- Tenant membership storage using a pivot table (`tenant_user` or equivalent) with role values via `TenantRole` enum.
|
||||||
|
|
||||||
|
**Compatibility constraints for 063**
|
||||||
|
- 063 MUST NOT modify `/system` panel, platform guards, platform users, or break-glass routes/UX.
|
||||||
|
- 063 MUST NOT refactor tenant RBAC data model or enforcement. It may only **read** memberships to decide where to redirect after login.
|
||||||
|
- 063 MUST NOT introduce Graph calls (or any outbound HTTP) during render/poll/hydration of `/admin` pages.
|
||||||
|
---
|
||||||
|
## Non-Goals (explicit)
|
||||||
|
- No Platform Operator `/system` login implementation (already exists or handled elsewhere).
|
||||||
|
- No break-glass UX on `/admin/login`.
|
||||||
|
- No tenant RBAC redesign or role enforcement changes (assume memberships already exist).
|
||||||
|
- No Graph calls or remote work in login render/poll/hydration.
|
||||||
|
- No storing of Entra access/refresh tokens.
|
||||||
|
- No queue/worker dependence for login (login is synchronous request/response).
|
||||||
|
---
|
||||||
|
## User Scenarios & Testing (mandatory)
|
||||||
|
### User Story 1 — Entra-only login UI on `/admin/login` (Priority: P1)
|
||||||
|
A tenant user can only start Microsoft sign-in from `/admin/login`.
|
||||||
|
|
||||||
|
**Acceptance Scenarios**
|
||||||
|
1. Given I open `/admin/login`, then I see only “Sign in with Microsoft” and no email/password fields.
|
||||||
|
2. Given Entra config is missing/invalid, `/admin/login` still renders and shows a generic message (no secrets).
|
||||||
|
|
||||||
|
**Independent Test**
|
||||||
|
- Render `/admin/login` and assert no password inputs exist.
|
||||||
|
---
|
||||||
|
### User Story 2 — OIDC callback upserts tenant identity safely (Priority: P1)
|
||||||
|
The callback upserts a tenant user using Entra claims.
|
||||||
|
|
||||||
|
**Acceptance Scenarios**
|
||||||
|
1. Given Entra callback includes `tid` and `oid`, when sign-in completes, then `users` is upserted keyed by:
|
||||||
|
- `entra_tenant_id = tid`
|
||||||
|
- `entra_object_id = oid`
|
||||||
|
2. Given sign-in succeeds, then the session is regenerated.
|
||||||
|
3. Given callback is missing `tid` or `oid`, then redirect to `/admin/login` with a generic error.
|
||||||
|
4. Given a user exists but is disabled (soft-deleted), when they complete a valid sign-in, then they are redirected to /admin/login with a generic error.
|
||||||
|
|
||||||
|
**Independent Test**
|
||||||
|
- Fake a callback payload and assert `(tid, oid)` uniqueness is enforced.
|
||||||
|
---
|
||||||
|
### User Story 3 — Post-login routing is membership-based (Priority: P1)
|
||||||
|
After login, routing depends on Suite tenant memberships.
|
||||||
|
|
||||||
|
**Acceptance Scenarios**
|
||||||
|
1. Given I have 0 memberships, then redirect to `/admin/no-access`.
|
||||||
|
2. Given I have exactly 1 membership, then redirect into that tenant’s dashboard.
|
||||||
|
3. Given I have >1 memberships, then redirect to a dedicated, full-screen tenant chooser page.
|
||||||
|
|
||||||
|
**Independent Test**
|
||||||
|
- Seed memberships and assert each redirect path.
|
||||||
|
---
|
||||||
|
### User Story 4 — Filament-native “No access” page (Priority: P2)
|
||||||
|
Users without memberships get a safe, Filament-native page.
|
||||||
|
|
||||||
|
**Acceptance Scenarios**
|
||||||
|
1. Given I have 0 memberships, `/admin/no-access` renders using Filament UI (no raw HTML pages).
|
||||||
|
2. The page does not leak internal details; it provides next steps (“Ask an admin to add you”).
|
||||||
|
---
|
||||||
|
## Requirements (mandatory)
|
||||||
|
### Functional Requirements
|
||||||
|
- **FR-001**: `/admin/login` MUST be Entra-only (no password login form).
|
||||||
|
- **FR-002**: Tenant user upsert MUST be keyed by `(entra_tenant_id, entra_object_id)` and MUST NOT store Entra access/refresh tokens.
|
||||||
|
- **FR-003**: Post-login routing MUST be based on memberships:
|
||||||
|
- 0 → `/admin/no-access`
|
||||||
|
- 1 → tenant dashboard
|
||||||
|
- N → dedicated tenant chooser page
|
||||||
|
- **FR-004**: OIDC failures MUST be handled safely:
|
||||||
|
- redirect to `/admin/login` with generic error
|
||||||
|
- log stable `reason_code` + `correlation_id`
|
||||||
|
- never log token/claims payloads
|
||||||
|
- **FR-005**: Logging MUST be privacy-safe:
|
||||||
|
- success: minimal (user_id, tid, oid hash, timestamp, correlation_id)
|
||||||
|
- failure: `reason_code`, correlation_id, minimal diagnostics
|
||||||
|
- **FR-006**: `/admin/login` and `/admin/no-access` MUST be DB-only at render time (no outbound HTTP).
|
||||||
|
- **FR-007**: 063 MUST NOT surface break-glass links or platform login UX on `/admin/login`.
|
||||||
|
- **FR-008**: Session separation MUST prevent implicit crossover:
|
||||||
|
- a tenant session MUST NOT grant access to `/system`
|
||||||
|
- a platform session MUST NOT grant access to `/admin` tenant membership routes
|
||||||
|
(Implementation is via separate guards/panels; 063 only asserts behavior via tests.)
|
||||||
|
- **FR-009**: Login flow MUST NOT require queue workers; it must complete synchronously.
|
||||||
|
- **FR-010**: If a user record exists but is in a disabled or soft-deleted state, a successful Entra ID authentication MUST be blocked, and the user redirected to the login page with a generic error.
|
||||||
|
### Non-Functional Requirements (NFR)
|
||||||
|
- **NFR-01 (Security)**: Do not persist secrets/tokens. Sanitize all error output and logs.
|
||||||
|
- **NFR-02 (Stability)**: Callback is idempotent; safe to retry without creating duplicates.
|
||||||
|
- **NFR-03 (Performance)**: Callback returns within ~2s under normal conditions.
|
||||||
|
- **NFR-04 (Maintainability)**: Minimal diff; do not refactor membership/RBAC models.
|
||||||
|
---
|
||||||
|
## Data Model
|
||||||
|
Uses existing `users` table. Required columns:
|
||||||
|
- `entra_tenant_id` (string)
|
||||||
|
- `entra_object_id` (string)
|
||||||
|
- `name` (string)
|
||||||
|
- `email` (nullable)
|
||||||
|
|
||||||
|
**Index**
|
||||||
|
- Unique index on `(entra_tenant_id, entra_object_id)`.
|
||||||
|
|
||||||
|
Optional (if already present / needed later):
|
||||||
|
- `last_tenant_id` (nullable FK to `tenants.id`) to optimize redirect for multi-membership users.
|
||||||
|
|
||||||
|
No new tables required.
|
||||||
|
---
|
||||||
|
## Routes / UI Surfaces
|
||||||
|
- `/admin/login` — Filament login override (Entra-only CTA)
|
||||||
|
- `/auth/entra/redirect` — starts OIDC redirect (Socialite)
|
||||||
|
- `/auth/entra/callback` — handles callback, upsert, routing
|
||||||
|
- `/admin/no-access` — Filament page for 0-membership users
|
||||||
|
- `/admin/choose-tenant` — Filament page for N-membership users to select a tenant
|
||||||
|
---
|
||||||
|
## OIDC Handling & Failure Semantics
|
||||||
|
### Claims requirements
|
||||||
|
- `tid` (tenant id) MUST be present.
|
||||||
|
- `oid` (object id) MUST be present.
|
||||||
|
If missing → fail safely.
|
||||||
|
|
||||||
|
### Generic user-facing error
|
||||||
|
- “Authentication failed. Please try again.”
|
||||||
|
|
||||||
|
### Server-side logging
|
||||||
|
Log event `auth.entra.login` with:
|
||||||
|
- `success`: boolean
|
||||||
|
- `reason_code`: string (on failure)
|
||||||
|
- `user_id`: (on success)
|
||||||
|
- `entra_tenant_id`: tid (or hashed)
|
||||||
|
- `entra_object_id_hash`: hash(oid)
|
||||||
|
- `correlation_id`: request id or session id
|
||||||
|
- `timestamp`
|
||||||
|
|
||||||
|
### Reason code examples (stable)
|
||||||
|
- `oidc_missing_claims`
|
||||||
|
- `oidc_invalid_state`
|
||||||
|
- `oidc_user_denied`
|
||||||
|
- `oidc_provider_unavailable`
|
||||||
|
- `oidc_user_upsert_failed`
|
||||||
|
- `user_disabled`
|
||||||
|
---
|
||||||
|
## Implementation Guardrails (hard)
|
||||||
|
- Do not implement a password login form for `/admin`.
|
||||||
|
- Do not call `$this->form->fill()` with default creds.
|
||||||
|
- Do not show break-glass link/button on `/admin/login`.
|
||||||
|
- Do not modify `/system` panel, platform guards, or break-glass logic.
|
||||||
|
- Do not refactor `User::tenants()` or membership schema; use a small resolver/service to decide redirect.
|
||||||
|
- Do not make outbound HTTP during render/poll/hydration of `/admin` pages.
|
||||||
|
- Do not store raw claims or tokens.
|
||||||
|
---
|
||||||
|
## Acceptance Tests (required)
|
||||||
|
### Feature tests (Pest)
|
||||||
|
1. **AdminLoginIsEntraOnlyTest**
|
||||||
|
- GET `/admin/login` contains Microsoft CTA
|
||||||
|
- asserts no `password` input / no local login form
|
||||||
|
2. **EntraCallbackUpsertByTidOidTest**
|
||||||
|
- callback upserts user by `(tid, oid)` (unique)
|
||||||
|
- session is regenerated
|
||||||
|
3. **PostLoginRoutingByMembershipTest**
|
||||||
|
- 0 memberships → `/admin/no-access`
|
||||||
|
- 1 membership → tenant dashboard
|
||||||
|
- N memberships → chooser page
|
||||||
|
4. **OidcFailureRedirectsSafelyTest**
|
||||||
|
- missing tid/oid → redirect `/admin/login`
|
||||||
|
- logs contain `reason_code` + `correlation_id`
|
||||||
|
- logs do not contain tokens/claims dumps
|
||||||
|
5. **SessionSeparationSmokeTest**
|
||||||
|
- tenant session cannot access `/system`
|
||||||
|
- platform session cannot access tenant membership routes without membership
|
||||||
|
6. **DisabledUserLoginIsBlockedTest**
|
||||||
|
- Seed a disabled/soft-deleted user
|
||||||
|
- Fake a successful OIDC callback for that user
|
||||||
|
- Assert redirect to /admin/login
|
||||||
|
- Assert log contains `reason_code: user_disabled`
|
||||||
|
### Quality gate
|
||||||
|
- `./vendor/bin/sail php ./vendor/bin/pint --dirty`
|
||||||
|
- `./vendor/bin/sail artisan test tests/Feature/Auth --stop-on-failure`
|
||||||
|
---
|
||||||
|
## Manual Verification Checklist
|
||||||
|
1. Open `/admin/login` - only Microsoft sign-in CTA visible
|
||||||
|
2. Complete Entra sign-in - user record exists with tid/oid
|
||||||
|
3. 0 memberships → `/admin/no-access`
|
||||||
|
4. 1 membership → tenant dashboard
|
||||||
|
5. >1 memberships → chooser page
|
||||||
|
6. Verify logs:
|
||||||
|
- failures show reason_code + correlation_id
|
||||||
|
- no tokens/claims in logs
|
||||||
|
---
|
||||||
|
## Out of Scope / Follow-ups
|
||||||
|
- **064-auth-structure**: `/system` operator login hardening, break-glass UX, panel separation governance.
|
||||||
|
- **062-tenant-rbac-v1**: role enforcement audit + resource-by-resource authorization hardening.
|
||||||
|
- Advanced Entra topics (v2+): delegated flows, refresh token storage, certificate auth, conditional access UI.
|
||||||
Loading…
Reference in New Issue
Block a user