From 1ba410457abc7d8fce037ad4dffbbab9bd001939 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Mon, 26 Jan 2026 23:52:40 +0100 Subject: [PATCH] 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. --- .../checklists/requirements.md | 35 +++ specs/063-entra-signin/spec.md | 215 ++++++++++++++++++ 2 files changed, 250 insertions(+) create mode 100644 specs/063-entra-signin/checklists/requirements.md create mode 100644 specs/063-entra-signin/spec.md diff --git a/specs/063-entra-signin/checklists/requirements.md b/specs/063-entra-signin/checklists/requirements.md new file mode 100644 index 0000000..9c836e8 --- /dev/null +++ b/specs/063-entra-signin/checklists/requirements.md @@ -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. \ No newline at end of file diff --git a/specs/063-entra-signin/spec.md b/specs/063-entra-signin/spec.md new file mode 100644 index 0000000..478dc4b --- /dev/null +++ b/specs/063-entra-signin/spec.md @@ -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. \ No newline at end of file