diff --git a/AGENTS.md b/AGENTS.md index 7a152f2..f8ffac6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -320,25 +320,30 @@ Auth model facts (the canonical statement of "which tokens are accepted" lives i `oid` → contributor via `entra_identities`. *Unchanged.* - **Service (app / managed-identity)** — `scp` absent: authorized by an Entra **App Role** alone — `Contributor` (write+read) or `Reader` (read-only: - `POST /cypher`, `GET /blobs/*`). `created_by` = `service_identities[oid]` if - mapped, else the stable `appid`. App-only / MI tokens (which carry `roles`, - not `scp`) **are now accepted** on this path. + `POST /cypher`, `GET /blobs/*`). `created_by` = the contributor id mapped + for `oid` in the **shared identity store** (same store `entra_identities` + uses). A service + `oid` with no mapping is **403** (fail-loud, names the principal) — + `created_by` is **never** `appid`/`azp`/`oid`/display name. App-only / MI + tokens (which carry `roles`, not `scp`) **are now accepted** on this path. - `scp` + `idtyp=="app"` → 401 (ambiguous, fail-closed). - The matched `created_by` surfaces on graph nodes — same provenance path as static mode. - Config fields: required for boot — `azure_client_id`, `azure_tenant_id`, - `entra_identities`. Optional service-path — `service_identities` (friendly - `created_by` map, **not** an auth gate, **no runtime CRUD** — config + redeploy), + `entra_identities`. Optional service-path — `service_identities` (a + **first-boot-only seed** into the same shared identity store; may be empty — + service oids are onboarded/removed at runtime via `/admin/identities`, + no redeploy needed), `service_data_role` (default `Contributor`), `reader_role` (default `Reader`). Env prefix `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_`. - **Fail-closed:** misconfig (missing field / empty or malformed `entra_identities`) is a HARD startup error. `allow_unauthenticated` defaults to `false`; a server with no auth configured refuses to start. - **401** = bad/expired/wrong-audience/missing/ambiguous token; **403** = valid - token lacking authorization — a user whose `oid` is unmapped, or a service token - with no qualifying App Role (the 403 body names the principal + required roles). - **Behavior change (M2):** a token with no `scp` and no qualifying role now returns - **403** (was **401**). + token lacking authorization — a user or service principal whose `oid` is + unmapped, or a service token with no qualifying App Role (the 403 body names + the principal + reason/required roles). **Behavior change (M2):** a token + with no `scp` and no qualifying role now returns **403** (was **401**). > 🔒 **Secret hygiene — NO real identifiers in this product repo.** An `oid` is a > persistent personal identifier (PII). Never commit real oids, client IDs, or @@ -357,6 +362,10 @@ Both auth modes can add/remove identities **at runtime, no restart**, via the resolver holds the dict **by reference**, so a `/admin` `PUT`/`DELETE` is visible on the next request. **No cache, no TTL** — safe because the pilot runs a **single replica** (the in-process map is the source of truth). +- `entra_identities_store_path` backs **both** user and service identities — + one shared store, disjoint oid space. There is deliberately no + separate `/admin/services` endpoint; service identities are managed through + `PUT`/`DELETE`/`GET /admin/identities`. - `IdentityStore` (`identity_store.py`) commits **write-file-then-swap-memory** (atomic file replace first, then memory) and **fails closed** on a corrupt file (empty map + loud log, never a crash-loop). diff --git a/README.md b/README.md index 01b95ff..35b3da6 100644 --- a/README.md +++ b/README.md @@ -265,16 +265,18 @@ resolver: token's `oid` maps to a contributor via `entra_identities`. *Unchanged.* - **Service (app / managed-identity)** — `scp` absent: authorized by an Entra **App Role** alone — `Contributor` (write + read) or `Reader` (read-only: - `POST /cypher`, `GET /blobs/*`). `created_by` is `service_identities[oid]` if - mapped, else the stable `appid`. + `POST /cypher`, `GET /blobs/*`). `created_by` is the contributor id mapped + for `oid` in the same shared identity store that `entra_identities` uses. + An unmapped `oid` is **403**, never falling back to `appid`/`azp`/`oid`/display name. See [docs/entra-auth-setup.md](docs/entra-auth-setup.md) for the canonical model. The matched contributor id is stamped onto the graph as the write-once `created_by` provenance field. A missing/invalid credential is a **401**; a valid credential that lacks the needed binding or role is a **403** — a delegated user whose `oid` -is unmapped, or a service token with no qualifying App Role (its 403 body names the -`appid`/`oid` and the required roles). **Behavior change (M2):** a token with no +is unmapped, a service token with no qualifying App Role, or a service token whose +`oid` is not in the service identity map (each 403 body names the rejected +principal and the reason). **Behavior change (M2):** a token with no `scp` and no qualifying role now returns **403** (previously **401**). The server is headless, so there is a single fixed exempt set that never requires a token: `{/status, /version, /docs, /openapi.json}` — health/version plus the always-on @@ -374,7 +376,7 @@ Values are resolved with this priority (highest first): | `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_AZURE_CLIENT_ID` | `azure_client_id` | *(empty)* | App Registration (client) GUID. **Required when `auth_mode=entra`** (startup refuses otherwise). See [docs/entra-auth-setup.md](docs/entra-auth-setup.md). | | `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_AZURE_TENANT_ID` | `azure_tenant_id` | *(empty)* | Azure AD tenant GUID. **Required when `auth_mode=entra`**. See [docs/entra-auth-setup.md](docs/entra-auth-setup.md). | | `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_ENTRA_IDENTITIES` (JSON) | `entra_identities` | *(empty)* | Identity map `oid -> {id: }` for the **user (delegated)** path (oids are Azure Object IDs — **PII**, never commit real values). **Required (non-empty) when `auth_mode=entra`**; the matched `id` is recorded as `created_by`. See [docs/entra-auth-setup.md](docs/entra-auth-setup.md). | -| `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_SERVICE_IDENTITIES` (JSON) | `service_identities` | *(empty)* | **Entra service path — optional.** `oid -> {id: }` map giving a **friendly `created_by`** name to a service principal / managed identity. **Not an auth gate** (App Roles authorize; see below) and **never required** for boot. Unmapped services still authorize, with `created_by` = `appid`. No runtime CRUD — edit config and redeploy. See [docs/entra-auth-setup.md](docs/entra-auth-setup.md). | +| `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_SERVICE_IDENTITIES` (JSON) | `service_identities` | *(empty)* | **Entra service path — optional, first-boot seed only.** `oid -> {id: }` map seeding the **friendly `created_by`** identity for a service principal / managed identity into the same shared identity store `entra_identities` uses. **Not an auth gate** (App Roles authorize; see below) and **never required** for boot — may be empty. An unmapped service `oid` is now **403** (no `appid` fallback). Runtime add/remove is via `PUT`/`DELETE /admin/identities` — no redeploy needed. See [docs/entra-auth-setup.md](docs/entra-auth-setup.md). | | `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_SERVICE_DATA_ROLE` | `service_data_role` | `Contributor` | **Entra service path.** App Role name whose presence in an app token's `roles` claim grants service **write + read**. `""`/`null` disables the service write path. See [docs/entra-auth-setup.md](docs/entra-auth-setup.md). | | `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_READER_ROLE` | `reader_role` | `Reader` | **Entra service path.** App Role name granting service **read-only** access (`POST /cypher`, `GET /blobs/*`). `""`/`null` disables read-only app-token gating. See [docs/entra-auth-setup.md](docs/entra-auth-setup.md). | | `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_ADMIN_API_KEY` | `admin_api_key` | *(empty — admin API disabled)* | **Static-mode admin credential** — separate from the data `api_keys`; it is the only key allowed to call the `/admin/*` identity-map endpoints. Sent as a bearer token; the middleware recognizes it before the data keystore lookup. Empty → admin API returns `503`; regular data keys get `403` on `/admin/*`. Cannot be deleted/shadowed via the API. See [docs/identity-management.md](docs/identity-management.md). | diff --git a/context_intelligence_server/auth.py b/context_intelligence_server/auth.py index a395a1d..93f5008 100644 --- a/context_intelligence_server/auth.py +++ b/context_intelligence_server/auth.py @@ -84,18 +84,6 @@ def __init__(self, status_code: int, reason: str) -> None: self.reason = reason -def _first_nonblank(*values: Any) -> str | None: - """Return the first value that is a non-empty, non-whitespace str, else None. - - Used to chain service created_by candidates with truthiness semantics: - empty/whitespace/non-string candidates fall through (B6/B8). - """ - for v in values: - if isinstance(v, str) and v.strip(): - return v - return None - - def _resolve_token(token: str, keystore: dict[str, str]) -> str | None: """Return the contributor id for *token*, or ``None`` if not found. @@ -478,29 +466,29 @@ def resolve( f"in Azure Entra, then re-request a token.", ) - # --- created_by derivation [B6/B8]: stable claims, truthiness chaining, - # NEVER app_displayname (spoofable in Entra — B8), fail-loud. - # Order: service_map[oid] > appid > azp > oid. + # created_by comes only from the service identity map; an unmapped oid + # is 403, never appid/azp/oid/display_name (spoofable, not a contributor id). _oid_raw = claims.get("oid") oid_str = _oid_raw if isinstance(_oid_raw, str) and _oid_raw.strip() else "" oid_lower = oid_str.lower() mapped = self._service_identity_map.get(oid_lower) if oid_lower else None - created_by = _first_nonblank( - mapped, # 1. operator-assigned contributor id - claims.get("appid"), # 2. app client id (v1.0 token) - claims.get("azp"), # 3. authorized party (v2.0 token) - oid_str, # 4. SP object id (always present; last resort) - ) - if created_by is None: - # Unreachable in practice (oid always present); fail-loud, never null. + if mapped is None: + _appid_raw = claims.get("appid") + _principal = ( + _appid_raw + if isinstance(_appid_raw, str) and _appid_raw.strip() + else (oid_str or "(unknown)") + ) raise AuthError( - 401, - "Service token has no resolvable identity claim " - "(service map miss and appid/azp/oid all blank)", + 403, + f"Service principal {_principal!r} is not authorized: oid " + f"{oid_lower!r} is not in the service identity map; contact " + f"the server administrator to add this identity " + f"(tenant {self._tenant_id!r})", ) - return (created_by, roles, True) + return (mapped, roles, True) class StaticKeyResolver: diff --git a/context_intelligence_server/config.py b/context_intelligence_server/config.py index fd72680..4e36ba0 100644 --- a/context_intelligence_server/config.py +++ b/context_intelligence_server/config.py @@ -568,8 +568,9 @@ def build_identity_map(self) -> dict[str, str]: # M2 non-interactive auth: service / app-token identity path # ------------------------------------------------------------------------- # service_identities: the OID → contributor map for service principals / - # managed identities. Same shape as entra_identities; lives in config - # only (no durable store — service identities don't need runtime mutation). + # managed identities. Same shape as entra_identities; first-boot-only seed + # into the SAME shared store entra_identities uses (oids are disjoint across + # users and service principals, so one oid -> contributor map serves both). # # Shape: { "": {"id": ""} } # @@ -587,12 +588,12 @@ def build_identity_map(self) -> dict[str, str]: def _validate_service_identities( cls, v: dict[str, dict[str, str]] | None ) -> dict[str, dict[str, str]] | None: - """Fail-closed: same GUID-map rules as entra_identities (shared helper). + """Same GUID-map rules as entra_identities (shared helper); empty map allowed. - Delegates to ``_validate_identity_map()``. See that function's docstring - for the full rule set. + Delegates to ``_validate_identity_map()`` with ``allow_empty=True`` -- + an empty map is a supported bootstrap state, not a startup error. """ - return _validate_identity_map(v, "service_identities") + return _validate_identity_map(v, "service_identities", allow_empty=True) def build_service_identity_map(self) -> dict[str, str]: """Return ``{oid_lower -> contributor_id}`` for all configured service identities. diff --git a/context_intelligence_server/main.py b/context_intelligence_server/main.py index ef0dd74..6ef8965 100644 --- a/context_intelligence_server/main.py +++ b/context_intelligence_server/main.py @@ -641,13 +641,21 @@ def create_asgi_app( # Build and load the entra identity store. entra_store = IdentityStore(Path(s.entra_identities_store_path)) entra_store.load() + _config_entra_map = s.build_identity_map() + _config_service_map = s.build_service_identity_map() if not entra_store.path.exists(): - # First boot: seed in-process map from config. Converts the flat - # {oid -> contributor_id} from build_identity_map() to the rich - # {oid -> {"id": contributor_id}} format that IdentityStore expects. - config_map = s.build_identity_map() - if config_map: - rich_seed = {oid: {"id": cid} for oid, cid in config_map.items()} + # First boot: seed in-process map from config -- ONE shared store + # serves both user and service oids (disjoint key spaces make sharing + # safe; the disjointness check below enforces that the two config + # maps never name the same oid). + rich_seed: dict[str, dict[str, str]] = { + oid: {"id": cid} + for oid, cid in ( + *_config_entra_map.items(), + *_config_service_map.items(), + ) + } + if rich_seed: entra_store.seed(rich_seed) _entra_identity_store = entra_store app.state.entra_identity_store = entra_store @@ -667,16 +675,35 @@ def create_asgi_app( s.entra_identities_store_path, ) + # service_identities is a FIRST-BOOT seed only: once the store file + # exists, config is never re-read into it. An operator who sets + # SERVICE_IDENTITIES on an already-deployed server would otherwise get + # no signal at all — the only symptom is a 403 on the service token, + # whose message points at the administrator, not at the ignored config. + # Name the ignored oids here so the wrong lever is obvious at boot. + _ignored_service_oids = sorted( + oid for oid in _config_service_map if oid not in entra_store.flat_dict + ) + if _ignored_service_oids: + logger.warning( + "service_identities config lists %d oid(s) that are NOT in the " + "identity store and are being IGNORED: %r. service_identities " + "seeds the store on FIRST BOOT ONLY and store=%s already exists, " + "so config changes after the first boot have no effect and these " + "principals will receive 403. Add them at runtime with an " + "IdentityAdmin-role token via PUT /admin/identities/{oid} — no " + "redeploy required.", + len(_ignored_service_oids), + _ignored_service_oids, + s.entra_identities_store_path, + ) + # Boot disjointness invariant — each oid must belong to exactly one - # identity source. Building the service map here (not inline in the - # EntraResolver call) lets us check the overlap BEFORE construction so - # the server fails loudly at startup rather than silently misbehaving. - # Existing logic already keeps app tokens off the human map at - # request time; this prevents a same-oid-in-both misconfiguration. - _service_id_map = s.build_service_identity_map() - _entra_oids = set(entra_store.flat_dict.keys()) - _service_oids = set(_service_id_map.keys()) - _overlap = _entra_oids & _service_oids + # identity source. Checked against the two CONFIG maps, not the + # store's flat_dict: one shared store now holds user and service oids + # alike, so the store itself can no longer show the overlap. This + # prevents a same-oid-in-both misconfiguration at startup. + _overlap = set(_config_entra_map) & set(_config_service_map) if _overlap: raise RuntimeError( f"Boot invariant violated: oid(s) {sorted(_overlap)!r} appear " @@ -687,13 +714,15 @@ def create_asgi_app( # EntraResolver raises RuntimeError at construction if the JWKS # prefetch fails (eager fail-closed by design). - # Pass entra_store.flat_dict (the LIVE dict) so the resolver sees - # any put()/delete() made by /admin immediately, no restart required. + # Pass entra_store.flat_dict (the LIVE dict) as BOTH maps so the + # resolver sees any put()/delete() made by /admin immediately on + # either path, no restart required. resolver: StaticKeyResolver | EntraResolver = EntraResolver( s.azure_client_id, # type: ignore[arg-type] — validated non-None by config s.azure_tenant_id, # type: ignore[arg-type] — validated non-None by config entra_store.flat_dict, # live reference — mutations visible immediately - service_identity_map=_service_id_map, # pre-built, disjointness verified + # SAME live reference (one shared store, disjoint oid keyspace). + service_identity_map=entra_store.flat_dict, service_data_role=s.service_data_role, reader_role=s.reader_role, entra_admin_role=s.entra_admin_role, diff --git a/docs/architecture/06-auth-flow.dot b/docs/architecture/06-auth-flow.dot index 241fe64..42d9c88 100644 --- a/docs/architecture/06-auth-flow.dot +++ b/docs/architecture/06-auth-flow.dot @@ -220,12 +220,12 @@ digraph auth_flow { ] ServiceCreatedBy [ - label = "created_by derivation (STABLE CLAIMS ONLY \u2014 D7)\nTrusted map: service_identities[oid] \u2192 friendly name\n (optional override \u2014 NOT a gate; no unmapped\u2192403 here)\nFallback (truthiness, NOT key-presence):\n appid \u2192 azp \u2192 oid\nNEVER app_displayname (caller-spoofable, often absent)\nFail-loud 403 if all stable claims absent (B8)" + label = "created_by derivation (SHARED IDENTITY STORE)\noid \u2192 identity store lookup (SAME store entra_identities uses;\n oids are disjoint across users and services)\nMapped \u2192 contributor id. Unmapped \u2192 403 (mirrors user path)\nNEVER appid / azp / oid / app_displayname as created_by" fillcolor = "#BBDEFB" ] ServiceContrib [ - label = "contributor_id = stable service id (str \u2014 never null)\n map name (\"ci-pipeline\") or Azure GUID (appid/oid)\nroles = token roles claim\n\u2192 return (contributor_id, roles)" + label = "contributor_id = mapped contributor id (str \u2014 never null,\n never appid/oid)\nroles = token roles claim\n\u2192 return (contributor_id, roles)" fillcolor = "#C8E6C9" fontname = "Helvetica-Bold" penwidth = 2.5 @@ -469,7 +469,7 @@ digraph auth_flow { RoleGate -> ServiceCreatedBy [label = "role present\n(Contributor\n| Reader\n| IdentityAdmin)"] // created_by derivation: stable claims only; fail-loud if all absent - ServiceCreatedBy -> AuthErrCatch [label = "all stable claims\nabsent \u2192 403 (B8)", style = dashed, color = "#BF360C", fontcolor = "#BF360C"] + ServiceCreatedBy -> AuthErrCatch [label = "oid unmapped\n\u2192 403 (mirrors user path)", style = dashed, color = "#BF360C", fontcolor = "#BF360C"] ServiceCreatedBy -> ServiceContrib [label = "contributor_id\nresolved"] // Service success \u2014 contributor_id str (stable id) returns to middleware diff --git a/docs/architecture/06-auth-flow.png b/docs/architecture/06-auth-flow.png index e136e4b..6ac74c0 100644 Binary files a/docs/architecture/06-auth-flow.png and b/docs/architecture/06-auth-flow.png differ diff --git a/docs/architecture/07-auth-startup.dot b/docs/architecture/07-auth-startup.dot index 61ed797..e2b8e44 100644 --- a/docs/architecture/07-auth-startup.dot +++ b/docs/architecture/07-auth-startup.dot @@ -8,7 +8,7 @@ digraph auth_startup { ranksep = 0.80 fontname = "Helvetica" fontsize = 11 - label = "Auth Wiring at Boot \u2014 Mode Selection & Fail-Closed Startup Gates\n(context_intelligence_server/main.py \u2014 create_asgi_app() \u00b7 auth.py \u2014 resolvers)\nM2: dual-map construction (entra_identities + service_identities) + B4 disjointness gate before JWKS prefetch" + label = "Auth Wiring at Boot \u2014 Mode Selection & Fail-Closed Startup Gates\n(context_intelligence_server/main.py \u2014 create_asgi_app() \u00b7 auth.py \u2014 resolvers)\nM2: ONE shared IdentityStore seeded from entra_identities\nand service_identities (disjoint oids) + B4 disjointness gate before JWKS prefetch" labelloc = t labeljust = c ] @@ -85,18 +85,18 @@ digraph auth_startup { { rank = same; BuildIdentityMap; BuildServiceMap } BuildIdentityMap [ - label = "settings.build_identity_map()\n{oid_lower \u2192 contributor_id}\n(from entra_identities config; keys lowercased)\nLIVE via IdentityStore.flat_dict \u2014 runtime-mutable by /admin/*" + label = "settings.build_identity_map()\n{oid_lower \u2192 contributor_id}\n(from entra_identities config; keys lowercased)\nFirst-boot seed into shared IdentityStore\nLIVE via IdentityStore.flat_dict \u2014 runtime-mutable by /admin/identities" fillcolor = "#BBDEFB" ] BuildServiceMap [ - label = "build_service_identity_map()\n{oid_lower \u2192 friendly_name}\n(from service_identities config; keys lowercased)\nPLAIN DICT \u2014 NOT an IdentityStore, NO durable file\nStatic config only; empty dict if not configured\nNo /admin/services \u2014 managed by config change + redeploy" + label = "build_service_identity_map()\n{oid_lower \u2192 contributor_id}\n(from service_identities config; keys lowercased)\nFirst-boot seed into the SAME shared IdentityStore as\n entra_identities; may be empty\nOnce the store file exists this config is IGNORED \u2014\n boot logs a WARNING naming the ignored oids\nRuntime CRUD via /admin/identities \u2014\n no separate /admin/services endpoint" fillcolor = "#BBDEFB" ] // B4: Disjointness invariant -- fail-closed gate DisjointCheck [ - label = "B4: disjointness invariant\nentra_identities.keys() \u2229 service_identities.keys() = \u2205?\n(same oid in both maps \u2192 name collision at resolve time)\nfail-closed: any overlap \u2192 RuntimeError; server refuses to start\n\u2014\nNote: B1 (idtyp-first branch) is the security-critical\nhuman/service separation; B4 guards friendly-name hygiene" + label = "B4: disjointness invariant (checked against the two\n CONFIG maps directly \u2014 not the merged store\'s flat_dict,\n which now legitimately holds both kinds of oid)\nbuild_identity_map().keys() \u2229 build_service_identity_map().keys() = \u2205?\nfail-closed: any overlap \u2192 RuntimeError; server refuses to start\n\u2014\nNote: B1 (idtyp-first branch) is the security-critical\nhuman/service separation; B4 guards friendly-name hygiene" shape = diamond fillcolor = "#FFF9C4" style = filled @@ -128,7 +128,7 @@ digraph auth_startup { ] EntraResolver [ - label = "EntraResolver(\n client_id, tenant_id,\n identity_map, \u2190 entra flat_dict (live \u2014 via IdentityStore)\n service_identity_map, \u2190 plain dict from config (static, read-only)\n service_data_role, \u2190 \"Contributor\" (default)\n reader_role, \u2190 \"Reader\"\n entra_admin_role \u2190 e.g. \"IdentityAdmin\"\n)\nauth_enabled = True (always \u2014 identity_map non-empty by config validation)\nexpected_aud = [client_id, \"api://client_id\"]\nexpected_issuer = https://login.microsoftonline.com/{tenant_id}/v2.0" + label = "EntraResolver(\n client_id, tenant_id,\n identity_map, \u2190 entra_store.flat_dict (live, via IdentityStore)\n service_identity_map, \u2190 SAME entra_store.flat_dict (one\n shared store, disjoint oid keyspace)\n service_data_role, \u2190 \"Contributor\" (default)\n reader_role, \u2190 \"Reader\"\n entra_admin_role \u2190 e.g. \"IdentityAdmin\"\n)\nauth_enabled = True (always \u2014 identity_map non-empty by config validation)\nexpected_aud = [client_id, \"api://client_id\"]\nexpected_issuer = https://login.microsoftonline.com/{tenant_id}/v2.0" fillcolor = "#BBDEFB" fontname = "Helvetica-Bold" ] diff --git a/docs/architecture/07-auth-startup.png b/docs/architecture/07-auth-startup.png index 141e1b5..6c44c3c 100644 Binary files a/docs/architecture/07-auth-startup.png and b/docs/architecture/07-auth-startup.png differ diff --git a/docs/architecture/08-identity-map-management.dot b/docs/architecture/08-identity-map-management.dot index da7f2fe..557da44 100644 --- a/docs/architecture/08-identity-map-management.dot +++ b/docs/architecture/08-identity-map-management.dot @@ -8,7 +8,7 @@ digraph identity_map_management { ranksep = 0.80 fontname = "Helvetica" fontsize = 11 - label = "Runtime Identity-Map Management \u2014 Admin API + Live Store\n(context_intelligence_server/routers/admin.py \u00b7 identity_store.py \u00b7 auth.py)\nM2: /admin/identities + /admin/keys CRUD UNCHANGED \u00b7 service_identities is static config only (no /admin/services \u2014 see annotation below)" + label = "Runtime Identity-Map Management \u2014 Admin API + Live Store\n(context_intelligence_server/routers/admin.py \u00b7 identity_store.py \u00b7 auth.py)\nService identities now share this SAME IdentityStore + /admin/identities API\n(one oid \u2192 contributor map; no separate /admin/services endpoint \u2014 see annotation below)" labelloc = t labeljust = c ] @@ -228,14 +228,14 @@ digraph identity_map_management { { rank = same; FileEntra; FileKeys } FileEntra [ - label = "entra-identities.json\n{\"\": {\"id\": \"\", \"display_name?\": \"...\"}}\nread at boot into IdentityStore; resolver uses flat_dict at runtime" + label = "entra-identities.json (shared by user AND service oids)\n{\"\": {\"id\": \"\", \"display_name?\": \"...\"}}\nread at boot into IdentityStore; resolver uses flat_dict at runtime\nload() never rewrites this file \u2014 only put()/delete() persist" shape = note style = filled fillcolor = "#FFF8E1" ] FileKeys [ - label = "api-keys.json\n{\"\": {\"id\": \"\"}}\nread at boot into IdentityStore; resolver uses flat_dict at runtime" + label = "api-keys.json\n{\"\": {\"id\": \"\"}}\nread at boot into IdentityStore; resolver uses flat_dict at runtime\n(same IdentityStore class backs this file too \u2014 static mode only,\n never loaded in entra mode)" shape = note style = filled fillcolor = "#FFF8E1" @@ -431,12 +431,13 @@ digraph identity_map_management { EffLookup -> EffResolved // ==================================================================== - // SERVICE IDENTITIES \u2014 STATIC CONFIG ANNOTATION - // (explicitly outside the runtime-CRUD scope of this diagram) + // SERVICE IDENTITIES \u2014 SAME STORE + // service oids are NOT a separate system \u2014 same IdentityStore/flat_dict + // as entra_identities above, disjoint keyspace // ==================================================================== subgraph cluster_service_static { graph [ - label = "service_identities \u2014 STATIC CONFIG (outside this diagram\u2019s scope)\nno /admin/services endpoint \u00b7 no IdentityStore \u00b7 no durable file on /data" + label = "service_identities \u2014 FIRST-BOOT SEED into the SAME shared store\n(runtime-managed thereafter via /admin/identities, same as entra_identities)" style = "dashed" color = "#E65100" fillcolor = "#FFF3E0" @@ -445,16 +446,16 @@ digraph identity_map_management { ] ServiceStaticNote [ - label = "service_identities \u2014 STATIC CONFIG ONLY\n\u2022 Plain dict built at boot: build_service_identity_map()\n\u2022 Source: settings.service_identities (env-var / config)\n\u2022 NOT an IdentityStore \u00b7 NO durable file on /data\n\u2022 NO /admin/services endpoint (deliberately excluded)\n \u2014 adding a service caller = config change + redeploy\n\u2022 Passed as read-only reference to EntraResolver at boot\n\u2022 B4: disjoint from entra_identities by startup invariant\n (see diagram 07 for boot wiring + B4 gate)\n\u2022 Optional friendly-name override; NOT an authz gate" + label = "service_identities \u2014 FIRST-BOOT SEED ONLY\n\u2022 settings.service_identities (env-var / config) seeds oid\u2192id\n pairs into the SAME entra-identities.json IdentityStore\n \u2014 may be empty (bootstrap)\n\u2022 Config is read ONLY when the store file does not exist;\n otherwise boot logs a WARNING naming the ignored oids\n\u2022 NOT a separate store, NOT a separate durable file\n\u2022 Runtime CRUD via the SAME /admin/identities endpoint\n (PUT/DELETE/GET) \u2014 NO /admin/services\n endpoint exists (deliberately \u2014 one endpoint, one store)\n\u2022 An unmapped role-bearing service oid is now 403, never a\n fallback created_by (mirrors the user path)\n\u2022 B4: disjoint from entra_identities by startup invariant,\n checked against the two CONFIG maps (see diagram 07)\n\u2022 Passed to EntraResolver as the SAME live flat_dict\n reference as entra_identities (one shared store)" fillcolor = "#FFF3E0" fontname = "Helvetica" fontsize = 10 ] } - // Annotation edge: contrast live entra dict (runtime-mutable) with static service map + // Annotation edge: service oids resolve through the SAME live dict as user oids LiveEntra -> ServiceStaticNote [ - label = "contrast: service_identities\nis static config, not via\nIdentityStore / /admin/*" + label = "service_identities shares THIS\nlive dict \u2014 same IdentityStore,\none oid \u2192 contributor map" style = dashed color = "#E65100" fontcolor = "#E65100" @@ -500,7 +501,7 @@ digraph identity_map_management { fillcolor = "#FFF8E1" ] LegOrange [ - label = "static config annotation\n(no CRUD / no /admin/* API)" + label = "service identities\n(first-boot seed, same store)" fillcolor = "#FFF3E0" ] LegOval [ diff --git a/docs/architecture/08-identity-map-management.png b/docs/architecture/08-identity-map-management.png index 263bd5c..e6ee07b 100644 Binary files a/docs/architecture/08-identity-map-management.png and b/docs/architecture/08-identity-map-management.png differ diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 965a3ac..fc52761 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -65,12 +65,13 @@ per-route capability gate before the data route handler. authz gate** — admit iff `roles` contains `Contributor` (write + read), `Reader` (read only), or `IdentityAdmin` (admin); no role → `AuthError(403)` whose message names the missing role (`"app has no Contributor/Reader role on this API — assign - one"`). `created_by` is derived from **stable Azure-assigned claims only**: trusted - `service_identities[oid]` map (optional friendly-name override, _not_ an authorization - gate) → `appid`/`azp` → `oid` (truthiness chain, never key-presence); **`app_displayname` - is never used** (caller-spoofable, often absent on v2.0/MI tokens); fail-loud 403 if all - stable claims are absent (B8). There is no "unmapped → 403" on the service path — the map - is optional; an unmapped SP gets a stable GUID until an admin adds a friendly override. + one"`). `created_by` is then resolved **exactly like the user path**: `oid` → the + **shared identity store** (the same store backing `entra_identities`; + contributor id. An `oid` with **no** + mapping is a second, distinct `AuthError(403)` naming the principal (fail-loud, + mirrors the user path's unmapped-oid 403) — **`created_by` is never** + `appid`/`azp`/`oid`/`app_displayname` (those are raw, spoofable-or-machine claims, + not contributors). - **Ambiguous token** (both `scp` + `idtyp=="app"`, or neither) → `AuthError(401)`, fail-closed (B1 mutual-exclusion, prevents namespace bleed). - **(B2)** `idtyp` is normalized before comparison: non-string → `""`, lower/strip. @@ -109,25 +110,28 @@ How authentication is wired at boot inside `create_asgi_app()`. The function bra `auth_mode`: - **`static`:** builds `StaticKeyResolver(build_keystore())` — pure dict, no network. -- **`entra` (M2 updated):** builds both identity maps from config, checks the B4 - disjointness invariant, eagerly fetches JWKS, and constructs `EntraResolver` with five - parameters. Specifically: - 1. **`build_identity_map()`** — `{oid_lower → contributor_id}` from `entra_identities` - config; passed as the **live** `IdentityStore.flat_dict` (runtime-mutable by - `/admin/identities` — see diagram 08). - 2. **`build_service_identity_map()`** — `{oid_lower → friendly_name}` from - `service_identities` config; a **plain dict** (NOT an `IdentityStore`, NO durable - file). Static config only — managed by config change + redeploy, never by an admin - API. - 3. **B4 disjointness invariant** (fail-closed gate before JWKS fetch): if - `entra_identities.keys() ∩ service_identities.keys() ≠ ∅` the server raises - `RuntimeError` and refuses to start. Note: B1 (`idtyp`-first branching) is the - security-critical human/service separation; B4 guards friendly-name collision hygiene. - 4. **JWKS prefetch** — `PyJWKClient.fetch_data()` eagerly; fail-closed if the endpoint +- **`entra` (M2 updated):** seeds **one shared `IdentityStore`** + from config, checks the B4 disjointness invariant, eagerly fetches JWKS, and + constructs `EntraResolver` with five parameters. Specifically: + 1. **First-boot seed:** `build_identity_map()` (`entra_identities`) and + `build_service_identity_map()` (`service_identities`) are merged into one + `rich_seed` and written into the **same** `entra_identities_store_path` + `IdentityStore` via `seed()` — only when the store file doesn't exist yet (a + pre-existing store, e.g. after an `/admin/identities` mutation, is loaded as-is; + config never overwrites runtime-managed data). + 2. **B4 disjointness invariant** (fail-closed gate before JWKS fetch): checked + against the **two config maps directly** — `build_identity_map().keys() ∩ + build_service_identity_map().keys() ≠ ∅` → `RuntimeError`, refuses to start. + (Checked against the config maps, not the merged store's `flat_dict`, which now + legitimately holds both kinds of oid by design.) + 3. **JWKS prefetch** — `PyJWKClient.fetch_data()` eagerly; fail-closed if the endpoint is unreachable or returns zero keys (`RuntimeError`, server refuses to start). - 5. **`EntraResolver(client_id, tenant_id, identity_map, service_identity_map, - service_data_role, reader_role, entra_admin_role)`** — constructed after both maps - are built and B4 + JWKS gates pass. + 4. **`EntraResolver(client_id, tenant_id, identity_map, service_identity_map, + service_data_role, reader_role, entra_admin_role)`** — both `identity_map` and + `service_identity_map` are passed the **same live `IdentityStore.flat_dict` + reference** (one shared store, disjoint oid keyspace makes this safe): an + admin-onboarded service mapping (`PUT /admin/identities`) + resolves immediately, exactly like a user mapping — see diagram 08. A fail-closed gate then rejects boot if `resolver.auth_enabled` is `False` and `allow_unauthenticated` is not set (the latter is a test/dev-only opt-out, never for @@ -149,14 +153,13 @@ required for any mutation. The diagram covers both the **entra-identities store* contributor) and the **api-keys store** (sha256 hash → contributor) through a single shared `IdentityStore` abstraction. -> **M2 scope note:** this diagram's admin API is **unchanged** from M1. The M2 dual-path adds -> a `service_identities` map for service/app tokens, but **there is no `/admin/services` -> endpoint** — that was deliberately excluded. `service_identities` is **static config only**: -> a plain dict built from settings at boot via `build_service_identity_map()`, with no durable -> file on `/data` and no runtime CRUD. Adding or removing a service caller requires a config -> change and redeploy. The diagram annotates this explicitly (orange dashed cluster) to prevent -> readers from assuming services are runtime-manageable. See diagram 07 for how the service -> map is wired at boot alongside the B4 disjointness gate. +> **Update:** service identities are now managed through this **same** +> `/admin/identities` API — there is deliberately **no separate** `/admin/services` +> endpoint. `service_identities` config remains a **first-boot-only seed** into the same +> durable store `entra_identities` uses — it may be empty/omitted, and service callers +> added after boot go through `PUT`/`DELETE /admin/identities`, no redeploy needed. +> See diagram 07 for how the shared store is seeded at boot alongside the B4 +> disjointness gate. **Authorization gate (`require_admin`):** applied router-wide via `APIRouter(dependencies=[Depends(require_admin)])`. The middleware (`BearerTokenMiddleware`) diff --git a/docs/azure-deployment.md b/docs/azure-deployment.md index ec3cb78..f2b843b 100644 --- a/docs/azure-deployment.md +++ b/docs/azure-deployment.md @@ -569,10 +569,17 @@ variables are: | `AZURE_CLIENT_ID` | yes | App Registration (client) GUID | | `AZURE_TENANT_ID` | yes | Azure AD tenant GUID | | `ENTRA_IDENTITIES` (JSON) | yes | `oid → {id}` map for the **user (delegated)** path (**PII** — seed via admin API, do not commit) | -| `SERVICE_IDENTITIES` (JSON) | no | Optional friendly-`created_by` map for **service** principals — not an auth gate, no runtime CRUD | +| `SERVICE_IDENTITIES` (JSON) | no | First-boot seed for **service** principals into the same shared identity store `entra_identities` uses — not itself an auth gate, but an unmapped role-bearing service now gets **403**; may be empty, and identities can be onboarded/removed later via `/admin/identities`, no redeploy | | `SERVICE_DATA_ROLE` | no | App Role granting service write+read (default `Contributor`) | | `READER_ROLE` | no | App Role granting service read-only (default `Reader`) | +> **Note: `SERVICE_IDENTITIES` seeds only on first boot.** The `service_identities` +> config seeds the shared identity store on **first boot only**. Once the store file +> (`/data/identity/entra-identities.json`) exists, config changes to `service_identities` +> have no effect. The server logs a **WARNING** at startup naming any `service_identities` +> oids being ignored for this reason, and points the operator at `PUT /admin/identities/{oid}` +> instead for runtime onboarding (no redeploy needed). + > **Operator note — service callers must use Managed Identity or federated OIDC, > not client secrets.** In a locked-down tenant, the service path (app-only tokens > authorized by an Entra App Role) should be driven by a **Managed Identity** or a diff --git a/docs/entra-auth-setup.md b/docs/entra-auth-setup.md index f4dc3b1..5c1332c 100644 --- a/docs/entra-auth-setup.md +++ b/docs/entra-auth-setup.md @@ -72,7 +72,7 @@ validation (signature / audience / issuer / `tid`, below), a single | Token shape | Path | How it is authorized | `created_by` | |---|---|---|---| | **`scp` present** (and `idtyp != "app"`) | **User (delegated)** — *unchanged* | `scp` must contain `access_as_user`; then `oid` → `entra_identities` map | the mapped contributor `id` | -| **`scp` absent** | **Service (app / daemon / managed-identity)** — *new* | an **App Role** alone: `roles` must contain `Contributor`, `Reader`, or `IdentityAdmin` | `service_identities[oid]` if mapped, else the stable `appid` | +| **`scp` absent** | **Service (app / daemon / managed-identity)** — *new* | an **App Role** alone: `roles` must contain `Contributor`, `Reader`, or `IdentityAdmin` | the mapped contributor `id` for `oid` in the **shared identity store** (same store `entra_identities` uses); unmapped `oid` → **403** | | **`scp` present *and* `idtyp == "app"`** | — | anomalous (no legitimate Entra token is both) → **401**, fail-closed | — | - **User path — delegated, byte-for-byte unchanged.** A token Entra issues **in @@ -95,12 +95,20 @@ validation (signature / audience / issuer / `tid`, below), a single assignment **is** the authorization decision — there is no server-side allow-list of service principals and no pre-registration step. -> **`service_identities` is *not* an auth gate.** It is an **optional, static** -> `oid → {id: }` map (env/YAML, same shape as `entra_identities`) -> that only supplies a **friendly `created_by` name**. An unmapped but -> role-bearing service is fully authorized; its `created_by` simply falls back to -> the stable `appid`. There is **no** runtime `/admin/services` endpoint — service -> identities change only by editing config and redeploying. + A role-bearing service token whose `oid` is **not** in the identity store is + **also a 403** (fail-loud, naming the rejected principal) — mirroring the + user path. `created_by` is **never** derived from `appid`/`azp`/`oid`/display + name; the mapped contributor `id` is the only valid value. + +> **`service_identities` is a config *seed*, not the runtime source of truth.** +> Service (and user) identities both live in the **same durable `IdentityStore`** +> (one shared JSON file, `entra_identities_store_path`). `service_identities` +> (env/YAML, same shape as `entra_identities`) only **seeds** that store on +> **first boot** — it may be empty (bootstrap). After boot, service identities +> are added/removed/listed at runtime through the **same** `/admin/identities` +> endpoint `entra_identities` uses — there is deliberately **no separate** +> `/admin/services` endpoint. An unmapped role-bearing service is **not** +> authorized: it gets a 403, not a fallback `created_by`. > **Tenant policy note.** In a locked-down tenant that blocks client secrets, > service callers must obtain tokens via **Managed Identity** or **federated @@ -151,18 +159,21 @@ discriminator (see the model table above): | User | Scope (`scp`) | must contain **`access_as_user`** | `"access_as_user" in scp.split()` | | User | Object ID (`oid`) | looked up in `entra_identities` → `created_by` | `identity_map[oid.lower()]` | | Service | App Role (`roles`) | must contain `Contributor`, `Reader`, **or** `IdentityAdmin` | `service_data_role`/`reader_role`/`entra_admin_role in roles` | -| Service | Identity (`created_by`) | `service_identities[oid]` if mapped, else `appid` (never `app_displayname`) | truthiness chain | +| Service | Identity (`created_by`) | mapped contributor `id` for `oid` in the shared identity store; unmapped → **403** | `identity_map[oid.lower()]` (never `appid`/`azp`/`oid`/`app_displayname`) | A valid **user** token whose `oid` is **not** in the map is a **403** (identity unbound). A valid **service** token with **no** qualifying App Role is a **403** -(named principal + required roles). Any other failure is a **401**. - -> **`created_by` legend — a GUID means a machine.** When `created_by` is a **GUID** -> it is an **`appid`** (a service principal's application ID) — i.e. a **machine** -> identity, resolvable in Entra by that app id. A **friendly** `created_by` name -> appears only when the service's `oid` is present in the optional -> `service_identities` map. (Delegated **users** always resolve to the friendly -> contributor `id` from `entra_identities`.) +(named principal + required roles); a role-bearing service token whose `oid` +is **not** in the identity store is **also a 403** (named principal). Any +other failure is a **401**. + +> **`created_by` legend — always a contributor id, never a machine claim.** +> Both paths resolve `created_by` the same way: `oid` → the shared identity +> store → a friendly contributor `id`. There is no fallback to `appid`, `azp`, +> raw `oid`, or `app_displayname` for either users or services — an unmapped +> `oid` is unauthorized (403), not attributed under a machine identifier. Seed +> service identities via `service_identities` (first boot only), or manage them +> at runtime via `PUT /admin/identities` — the same call used for a user. ### Admin authority and service roles — the `roles` claim @@ -261,19 +272,27 @@ otherwise — see §2.5): | Identity map | `entra_identities` | `ENTRA_IDENTITIES` (JSON) | `oid → {id: }` (the **user** path) | These four boot the **user** path. The **service** path needs **no required -settings** — it works out of the box once an App Role is assigned in Entra. Its -settings are all **optional** and have working defaults: +config fields to boot**, but a service caller also needs its `oid` mapped in +the identity store (seeded from `service_identities`, or added later via +`/admin/identities`) — an App Role alone is no longer sufficient. Its +settings are all **optional at boot** and have working defaults: | Field | YAML key | Env var (`AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_` + …) | Default | Meaning | |---|---|---|---|---| | Service data role | `service_data_role` | `SERVICE_DATA_ROLE` | `Contributor` | App Role granting service **write + read**. `""`/`null` disables it. | | Reader role | `reader_role` | `READER_ROLE` | `Reader` | App Role granting service **read-only** (`POST /cypher`, `GET /blobs/*`). `""`/`null` disables it. | -| Service identities | `service_identities` | `SERVICE_IDENTITIES` (JSON) | *(unset)* | **Optional** `oid → {id: }` map — a friendly `created_by` override only, **not** an auth gate. Unmapped services still authorize (via App Role); their `created_by` falls back to `appid`. No runtime CRUD — edit config and redeploy. | +| Service identities | `service_identities` | `SERVICE_IDENTITIES` (JSON) | *(unset)* | **First-boot seed only**, `oid → {id: }` — seeds the mapped contributor `id` into the same shared identity store `entra_identities` uses. **Not itself an auth gate** (the App Role check gates authorization), but **is now required for attribution**: a role-bearing service whose `oid` isn't mapped gets **403**, never a fallback `created_by`. May be empty/omitted; add mappings later via `/admin/identities`, no redeploy. | > `service_identities` is validated with the **same** GUID-key / non-empty-`id` -> rules as `entra_identities`, but it is **never** required for boot and never -> participates in the entra startup validator. Omit it entirely if you don't need -> friendly machine names. +> rules as `entra_identities`, and — like `entra_identities` — an explicit +> empty map is accepted (bootstrap on a fresh `/data` volume). Omit it, or +> leave it empty, and onboard service identities at runtime instead. +> +> **First-boot-only seed:** `service_identities` populates the shared identity +> store only on initial boot. Once the store file exists, config changes to +> `service_identities` are ignored. The server logs a WARNING at startup naming +> any ignored `service_identities` oids and points operators at +> `PUT /admin/identities/{oid}` for runtime onboarding. ### 2.2 YAML config @@ -449,9 +468,9 @@ To bind them: their own oid is not yet mapped** (the `/admin`-path bootstrap exemption). Full runbook: [identity-management.md](identity-management.md). - **Config + restart (OPTIONAL seed):** add `"": {id: }` to - `entra_identities` and restart. A config seed is **no longer required** — it - only pre-populates an initially-empty store. The primary path is boot-empty + - the `/admin` API above. + `entra_identities` in `service_identities` (if the store doesn't exist yet) + and restart. A config seed is **no longer required** — it only pre-populates + an initially-empty store. The primary path is boot-empty + the `/admin` API above. The next call from that user → `created_by = `. @@ -534,6 +553,7 @@ the service path instead, where the question is App Roles, not scope. | **401** | both | **Token problem.** Missing / expired / wrong audience / wrong tenant / missing `oid`; or a **user** token whose `scp` lacks `access_as_user`; or an **ambiguous** token (`scp` *and* `idtyp=="app"`). | Re-acquire the token (§3.3). Confirm `--resource api://`. | | **403** | user | **Identity not bound.** Your delegated token is valid, but your `oid` isn't in the operator's map. The body names your oid. | Send your oid (§3.2) to the operator to be added (§2.6). | | **403** | service | **No qualifying App Role.** Your app/service token is valid, but its `roles` has none of `Contributor`/`Reader`/`IdentityAdmin`. The body names your `appid`/`oid` and the required roles. | Have an admin assign the App Role in Entra ([service callers](identity-management.md#service-callers-entra-app-tokens)). | +| **403** | service | **Identity not bound.** Your app/service token has a qualifying App Role, but its `oid` isn't in the shared identity store. The body names your oid. | Send your oid to the operator to add via `PUT /admin/identities` (§2.6 / [identity-management.md](identity-management.md)). | > **Behavior change (M2): 401 → 403 for role-less app tokens.** Before M2, an > app-only token (with `roles`, no `scp`) failed the `access_as_user` check and @@ -626,8 +646,10 @@ legibly instead of as a bare `Invalid isoformat string: ''`. - **User** (`scp` present): `scp` must contain `access_as_user`, `oid` → contributor; **403** when the `oid` is unbound. - **Service** (`scp` absent): `roles` must contain `Contributor`/`Reader`/ - `IdentityAdmin`; `created_by` = `service_identities[oid]` → `appid` → `azp` → - `oid` (never `app_displayname`); **403** when no qualifying role. + `IdentityAdmin`; `created_by` = the shared identity store's mapped + contributor `id` for `oid` (never `appid`/`azp`/`oid`/`app_displayname`); + **403** when no qualifying role, **or** when the role-bearing `oid` is + unmapped. - `scp` + `idtyp=="app"` → **401** (ambiguous, fail-closed). - **401** otherwise = invalid/expired/missing/wrong-audience/wrong-tenant token. - Per-route capability gates: `context_intelligence_server/authz.py` diff --git a/docs/identity-management.md b/docs/identity-management.md index cd00102..549234c 100644 --- a/docs/identity-management.md +++ b/docs/identity-management.md @@ -55,7 +55,7 @@ server checks the role named by `entra_admin_role` (**default `IdentityAdmin`**) > `AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_ENTRA_ADMIN_ROLE`. Setting it empty > (`null`) **disables** the admin API → callers get `503`. -#### Entra App Roles and service identities (no runtime CRUD) +#### Entra App Roles and service identities `IdentityAdmin` is one of **three** App Roles the server recognizes in an entra token's `roles` claim. The other two authorize the **service path** (app / @@ -70,16 +70,22 @@ managed-identity tokens — see A service principal is authorized by an App Role **alone** — assigning the role in Entra *is* the onboarding step (§4). There is **no** server-side -pre-registration of service principals. - -> **`service_identities` is OPTIONAL STATIC config — there is no runtime CRUD.** -> Unlike the user `entra_identities` map (mutable live via `/admin/identities`), -> the `service_identities` map (`oid → {id: }`) lives **only** in -> config (env/YAML). It is **not** an authorization gate — it only supplies a -> **friendly `created_by`** name for a mapped service. An unmapped but -> role-bearing service is still fully authorized; its `created_by` falls back to -> the stable `appid` (a GUID). **There is no `/admin/services` endpoint** — to add -> or change a friendly name, edit `service_identities` in config and redeploy. +pre-registration of service principals. Authorization and identity are +separate concerns, though: a role-bearing service also needs its `oid` mapped +(below) or its requests get **403**. + +> **`service_identities` is a first-boot seed into the SAME store as +> `entra_identities` — not a separate, static system.** Service identities live +> in the identical durable `IdentityStore` behind `entra_identities`. +> `service_identities` (env/YAML) only **seeds** that store on first boot and +> may be empty. Once the store file exists, config changes to `service_identities` +> have no effect; the server logs a WARNING at startup naming ignored `service_identities` +> oids. It is **not itself** the authorization gate (the App Role check is), but it +> **is** required for attribution: a role-bearing service whose `oid` isn't mapped now +> gets **403** — there is no fallback to the stable `appid` GUID. To onboard, change, +> or remove a service identity **at runtime, no restart**, use the **same** +> `/admin/identities` endpoint as `entra_identities` (§3–4). **There is deliberately +> no separate `/admin/services` endpoint** — one endpoint, one store. ### Static mode — set `admin_api_key` @@ -117,7 +123,11 @@ the inactive mode is not loaded). ### Entra identities — `auth_mode=entra` -| Method & path | Body | Success | +One store, one endpoint, serves **both** user and service identities — Entra +`oid`s are disjoint across users and service principals, so a single +`oid → contributor` map covers both: + +| Method & path | Body / query | Success | |---|---|---| | `PUT /admin/identities/{oid}` | `{"id": "", "display_name": ""}` | `200 {"oid","id"[,"display_name"]}` | | `DELETE /admin/identities/{oid}` | — | `200 {"oid","deleted":true}` (`404` if absent) | @@ -125,6 +135,8 @@ the inactive mode is not loaded). - `{oid}` must be a **lowercase-hex GUID** (`xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx`), not the all-zeros sentinel → otherwise `422`. +- There is deliberately **no separate `/admin/services` endpoint** — service and + user identities are managed through the same endpoint. ### Static API keys — `auth_mode=static` @@ -173,22 +185,31 @@ principals. Three steps: this API's App Registration, as an **Application**-type role assignment (*Enterprise Applications → your app → Users and groups*, or via Graph). This assignment **is** the authorization — no server change, no redeploy. -2. **The caller requests a token** for **`api:///.default`** via +2. **Map the service principal's `oid` to a contributor.** A role-bearing + service token whose `oid` isn't mapped now gets **403** (names the + principal) — an App Role alone is no longer sufficient. Map it either + before first use (seed `service_identities` in config) or at **runtime, no + restart**, the same way a user is onboarded (§4, above): + + ```bash + OID="aaaaaaaa-0000-0000-0000-000000000002" # the service principal's oid + curl -sS -X PUT "$SERVER/admin/identities/$OID" \ + -H "Authorization: Bearer [REDACTED:SECRET]" \ + -H "Content-Type: application/json" \ + -d '{"id": ""}' + ``` +3. **The caller requests a token** for **`api:///.default`** via **Managed Identity** or **federated OIDC** (a client secret only where the tenant permits — see the tenant policy note in [entra-auth-setup.md](entra-auth-setup.md#the-model--two-authentication-paths-user--service)). The token carries the App Role in `roles` and **no `scp`**, so it takes the - service path. On the next call it authenticates; `created_by` defaults to the - service principal's **`appid`** (a GUID). -3. **(Optional) Add a friendly `created_by` name.** To stamp a human-readable - contributor instead of the `appid` GUID, add the service principal's `oid` to - the **static** `service_identities` map (`oid → {id: }`) in - config and **redeploy**. This is config-only — there is **no** runtime endpoint - for it. - -> To **remove** a service caller, unassign the App Role in Entra. (Deleting a -> `service_identities` entry only drops the friendly name; the role assignment is -> what authorizes.) + service path. On the next call it authenticates; `created_by` is the + contributor `id` mapped in step 2 — **never** a fallback `appid`/`azp`/`oid`. + +> To **remove** a service caller's authorization, unassign the App Role in +> Entra. To remove its identity mapping, `DELETE /admin/identities/{oid}` (same +> endpoint as users) — but note the App Role is what authorizes; deleting only +> the mapping leaves a role-bearing token unable to resolve `created_by` (403). ### Static (`auth_mode=static`) diff --git a/docs/m2-auth-acceptance.md b/docs/m2-auth-acceptance.md index 624e175..b9bf104 100644 --- a/docs/m2-auth-acceptance.md +++ b/docs/m2-auth-acceptance.md @@ -12,6 +12,19 @@ > token (no `idtyp`/`appidacr` emitted) — see §6. The offline synthetic-token > unit tests remain the fast regression layer. +> **Superseded by the service-identity alignment change.** The resolver's `created_by` **fallback chain +> described below no longer exists**: `service_identities[oid] → appid → azp +> → oid` has been replaced by a single lookup into the shared identity store +> (the same store `entra_identities` uses) — a role-bearing service token +> whose `oid` isn't mapped is now **403**, never attributed under `appid`/ +> `azp`/`oid`. The "green on 2026-06-30" evidence above remains valid +> historical proof that a real app-only token has the claim shape +> `EntraResolver` expects; it does **not** re-validate `created_by` +> derivation, since that assertion (STEP A.1 #4, STEP B) targeted the +> now-removed fallback. Re-running this gate after this change requires the test +> caller's `oid` to be pre-mapped (`service_identities` seed or +> `PUT /admin/identities`) — see the inline notes below. + > **Secret hygiene.** This document uses **placeholder** identifiers only. > Never commit real client IDs, tenant IDs, or object IDs to this repo — see > the PII warning in [`entra-auth-setup.md`](entra-auth-setup.md). @@ -29,8 +42,10 @@ Team Pulse precedent (`auth.py` docstring), **not** from having seen one: user branch). - `roles` carries the App Role assignment as a list of strings (never `groups`). -- `appid` / `azp` are present and stable, usable as a `created_by` fallback - when no `service_identities` mapping exists. +- `appid` / `azp` are present and stable claims on the token — **superseded by + the service-identity alignment change**: neither is ever used for `created_by` any more. The resolver + requires the token's `oid` to be mapped in the shared identity store and + returns 403 otherwise (see the note at the top of this document). - `idtyp == "app"` is *sometimes* present (only actually checked by the resolver in the `[B1]` ambiguous-token case — when `scp` is unexpectedly also present). @@ -244,8 +259,12 @@ fine): present. 3. `roles` contains at least one of the server's configured App Roles (`Contributor` / `Reader` / `IdentityAdmin`, or your overrides from §3.5). -4. `appid` or `azp` is present (the resolver's `created_by` fallback chain - needs one of these before it would fall back to bare `oid`). +4. *(historical assertion, now superseded)* `appid` or `azp` is present — this + targeted the old `created_by` fallback chain, which no longer exists (see + the note at the top of this document). It no longer gates anything in the + resolver; the real requirement now is that the token's `oid` is + mapped in the shared identity store (`service_identities` seed, or + `PUT /admin/identities`) — otherwise STEP A.2 now fails with 403. A redacted claim summary is always printed (`aud`, `iss`, `tid`, `scp` presence, `idtyp`, `appidacr`, `roles`, `appid`, `azp`, and a truncated @@ -272,9 +291,14 @@ or wrong. If it fails with a 401, re-check the federated credential subject Only runs when `ACCEPTANCE_SERVER_URL` is set: -- `POST /events` with the real token → asserts **HTTP 202**. +- `POST /events` with the real token → asserts **HTTP 202**. **This now + requires the caller's `oid` to already be mapped** in the shared + identity store (`service_identities` seed or `PUT /admin/identities`) — + an unmapped oid gets **403** here instead of 202. - Polls `POST /cypher` (best-effort, ~30s) for a node with - `created_by == appid` under the probe's `session_id`. **This match + `created_by == ` under the probe's `session_id` + *(previously this checked `created_by == appid`; that fallback no longer + exists — see the note at the top of this document)*. **This match pattern is a best guess at the live graph schema** — the event pipeline is persist-then-202 (async drain to Neo4j), so this is inherently a polling check, not an immediate read. A failure here is a warning, not a @@ -344,6 +368,11 @@ A green run is only useful if it is auditable. Attach to the PR: > stricter than the resolver) was therefore **deliberately broadened** to > accept `scp`-absent + (`appid` | `azp`) as a valid app-only signal. The > pre-run uncertainties below are retained for history. +> +> **Note:** the `azp = created_by source ✓` line above reflects +> historical behavior — `azp` fed the old fallback chain. `created_by` +> now comes solely from the shared identity store's mapped contributor id; `azp` +> is no longer read for this purpose. We have never seen a real Microsoft Entra app-only access token from this tenant, only Microsoft's general documentation and the Team Pulse mirror @@ -376,9 +405,10 @@ first reading the printed claim summary and the resolver's actual error. ## 7. Reference — accurate to the code - Resolver under test: `context_intelligence_server/auth.py` — - `EntraResolver.resolve()`, the `scp`/`idtyp` discriminator, the - `created_by` fallback chain (`service_identities[oid]` → `appid` → - `azp` → `oid`). + `EntraResolver.resolve()`, the `scp`/`idtyp` discriminator, and + `created_by` resolution via the shared identity store lookup (`oid` → + mapped contributor id; unmapped → 403 — no more `appid`/`azp`/`oid` + fallback chain). - Config fields referenced: `context_intelligence_server/config.py` — `azure_client_id`, `azure_tenant_id`, `service_data_role`, `reader_role`, `entra_admin_role`. diff --git a/tests/test_config.py b/tests/test_config.py index 957d71a..9eb7d2e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -829,7 +829,8 @@ def test_empty_dict_now_returns_empty_dict(self) -> None: (Previously: raised ValidationError 'at least one entry'. The empty map is now accepted via allow_empty=True so the server boots on a fresh /data volume and is populated at runtime via PUT /admin/identities. - service_identities={} still raises — see the service-identities suite.) + service_identities={} is ALSO now accepted — see + test_empty_dict_accepted in the service-identities suite.) """ from context_intelligence_server.config import Settings @@ -1520,14 +1521,18 @@ def test_default_is_none(self) -> None: s = Settings() assert s.service_identities is None - def test_empty_dict_raises(self) -> None: - """service_identities={} is a misconfiguration (fail-closed, mirrors entra_identities).""" - from pydantic import ValidationError + def test_empty_dict_accepted(self) -> None: + """service_identities={} is a SUPPORTED bootstrap state (mirrors entra_identities). + Previously: raised ValidationError 'at least one entry'. allow_empty=True + now accepts an explicit empty map so the server boots on a fresh /data + volume and service identities are onboarded at runtime via the same + /admin/identities endpoint entra_identities uses. + """ from context_intelligence_server.config import Settings - with pytest.raises(ValidationError, match="at least one entry"): - Settings(service_identities={}) + s = Settings(service_identities={}) + assert s.service_identities == {} def test_non_guid_key_raises(self) -> None: """Non-GUID key raises ValidationError.""" @@ -1729,8 +1734,8 @@ def test_entra_empty_dict_now_returns_empty_dict(self) -> None: """entra_identities={} is now a SUPPORTED bootstrap state — returns {}. (Previously: raised ValidationError 'at least one entry'. allow_empty=True - is passed ONLY for entra_identities; service_identities={} still raises — - see test_service_identities_empty_dict_raises.) + is now passed for BOTH entra_identities and service_identities — + see test_empty_dict_accepted.) """ from context_intelligence_server.config import Settings diff --git a/tests/test_entra_resolver.py b/tests/test_entra_resolver.py index 64afb9d..c7cb8de 100644 --- a/tests/test_entra_resolver.py +++ b/tests/test_entra_resolver.py @@ -1199,31 +1199,48 @@ def test_b1a_scp_and_idtyp_app_raises_401_ambiguous(self) -> None: assert "Ambiguous" in err.reason def test_b1b_no_scp_no_idtyp_service_admitted(self) -> None: - """B1-b: no scp, no idtyp, roles=["Contributor"], appid → service admitted.""" + """B1-b: no scp, no idtyp, roles=["Contributor"], mapped oid → service admitted. + + Created_by now comes from the service map (never appid fallback), + so this resolver is built with a service_map hit for FAKE_SERVICE_OID. + """ claims = _service_claims(roles=["Contributor"], appid=FAKE_APPID) - cid, roles, is_service = self._resolve_service(claims) - assert cid == FAKE_APPID + resolver = _make_service_resolver( + service_map={FAKE_SERVICE_OID.lower(): FAKE_SERVICE_CONTRIBUTOR} + ) + cid, roles, is_service = self._resolve_service(claims, resolver=resolver) + assert cid == FAKE_SERVICE_CONTRIBUTOR assert roles == ["Contributor"] assert is_service is True # -- B2: idtyp normalization --------------------------------------------- def test_b2a_idtyp_mixed_case_space_service_branch(self) -> None: - """B2-a: idtyp=" App " (mixed case+space) → normalized "app"; service branch admitted.""" + """B2-a: idtyp=" App " (mixed case+space) → normalized "app"; service branch admitted. + + Uses a mapped resolver since an unmapped oid now 403s outright. + """ claims = _service_claims(roles=["Reader"]) claims["idtyp"] = " App " # mixed case + surrounding whitespace # has_scp=False → B1 check is False → service branch - cid, roles, is_service = self._resolve_service( - claims, resolver=_make_service_resolver() + resolver = _make_service_resolver( + service_map={FAKE_SERVICE_OID.lower(): FAKE_SERVICE_CONTRIBUTOR} ) + cid, roles, is_service = self._resolve_service(claims, resolver=resolver) assert is_service is True assert roles == ["Reader"] def test_b2b_idtyp_int_normalized_to_empty(self) -> None: - """B2-b: idtyp=123 (int) → normalized to ""; service branch; admit/deny by roles.""" + """B2-b: idtyp=123 (int) → normalized to ""; service branch; admit/deny by roles. + + Uses a mapped resolver since an unmapped oid now 403s outright. + """ claims = _service_claims(roles=["Contributor"]) claims["idtyp"] = 123 # int, not str → normalized to "" - cid, roles, is_service = self._resolve_service(claims) + resolver = _make_service_resolver( + service_map={FAKE_SERVICE_OID.lower(): FAKE_SERVICE_CONTRIBUTOR} + ) + cid, roles, is_service = self._resolve_service(claims, resolver=resolver) assert is_service is True assert roles == ["Contributor"] @@ -1243,33 +1260,41 @@ def test_b6a_service_map_wins_over_appid(self) -> None: assert cid == FAKE_SERVICE_CONTRIBUTOR assert is_service is True - def test_b6b_blank_appid_falls_through_to_azp(self) -> None: - """B6-b: map miss, appid blank → falls through to azp → created_by = azp.""" + def test_b6b_unmapped_oid_blank_appid_raises_403(self) -> None: + """B6-b: map miss + blank appid → AuthError(403) naming the oid. + + Never falls through to azp -- fail-loud, not a fallback chain. + """ claims = _service_claims( oid=FAKE_SERVICE_OID, - appid=" ", # blank — skipped by _first_nonblank + appid=" ", # blank azp=FAKE_AZP, roles=["Reader"], ) - cid, _, _ = self._resolve_service(claims) - assert cid == FAKE_AZP + err = self._resolve_raises(claims) + assert err.status_code == 403 + assert FAKE_SERVICE_OID.lower() in err.reason.lower() + assert FAKE_AZP not in err.reason, "azp must never appear as created_by" + + def test_b6c_unmapped_oid_no_appid_no_azp_raises_403(self) -> None: + """B6-c: map miss, no appid, no azp → AuthError(403) naming the oid. - def test_b6c_oid_last_resort(self) -> None: - """B6-c: map miss, no appid, no azp → created_by = oid (last resort).""" + Never falls back to using oid itself as created_by -- fail-loud. + """ claims = _service_claims(oid=FAKE_SERVICE_OID, roles=["Contributor"]) # No appid, no azp in claims - cid, _, is_service = self._resolve_service(claims) - assert cid == FAKE_SERVICE_OID - assert is_service is True + err = self._resolve_raises(claims) + assert err.status_code == 403 + assert FAKE_SERVICE_OID.lower() in err.reason.lower() # -- B8: anti-spoof — app_displayname must never be used ----------------- def test_b8_anti_spoof_app_displayname_not_used(self) -> None: - """B8: app_displayname="alice@contoso.com" (a human UPN) → created_by = appid, NOT display name. + """B8: unmapped oid + app_displayname (a human UPN) → AuthError(403). - app_displayname is operator-mutable in Entra (spoofable). It is deliberately - excluded from the derivation chain. A service whose app_displayname happens - to look like a human UPN must never inherit that UPN as its created_by. + app_displayname is operator-mutable in Entra (spoofable) and was never + part of the derivation; an unmapped service oid is now rejected outright + (never falls back to appid, azp, oid, or app_displayname). """ claims = _service_claims( oid=FAKE_SERVICE_OID, @@ -1277,10 +1302,10 @@ def test_b8_anti_spoof_app_displayname_not_used(self) -> None: roles=["Contributor"], ) claims["app_displayname"] = "alice@contoso.com" # human-looking display name - cid, _, _ = self._resolve_service(claims) - assert cid == FAKE_APPID, "created_by must be appid, not app_displayname" - assert cid != "alice@contoso.com", ( - "app_displayname must never become created_by" + err = self._resolve_raises(claims) + assert err.status_code == 403 + assert "alice@contoso.com" not in err.reason, ( + "app_displayname must never appear as created_by (or in the 403)" ) # -- B7: aud / iss enforced in shared validation (no new code) ----------- @@ -1342,9 +1367,16 @@ def test_rg_unknown_unrecognized_role_raises_403(self) -> None: assert err.status_code == 403 def test_rg_reader_admitted(self) -> None: - """RG-reader: service token, roles=["Reader"] → admitted (…, ["Reader"], True).""" + """RG-reader: service token, roles=["Reader"] → admitted (…, ["Reader"], True). + + Uses a mapped resolver since an unmapped oid now 403s outright + (this test's purpose is the role gate, not created_by derivation). + """ claims = _service_claims(roles=["Reader"], appid=FAKE_APPID) - cid, roles, is_service = self._resolve_service(claims) + resolver = _make_service_resolver( + service_map={FAKE_SERVICE_OID.lower(): FAKE_SERVICE_CONTRIBUTOR} + ) + cid, roles, is_service = self._resolve_service(claims, resolver=resolver) assert is_service is True assert "Reader" in roles @@ -1361,9 +1393,16 @@ def test_rg_disabled_empty_reader_role_raises_403(self) -> None: # -- SV-ADM: service IdentityAdmin path ---------------------------------- def test_sv_adm_identity_admin_admitted(self) -> None: - """SV-ADM: service token with IdentityAdmin role admitted; (…, ["IdentityAdmin"], True).""" + """SV-ADM: service token with IdentityAdmin role admitted; (…, ["IdentityAdmin"], True). + + Uses a mapped resolver since an unmapped oid now 403s outright + (this test's purpose is the role gate, not created_by derivation). + """ claims = _service_claims(roles=["IdentityAdmin"], appid=FAKE_APPID) - cid, roles, is_service = self._resolve_service(claims) + resolver = _make_service_resolver( + service_map={FAKE_SERVICE_OID.lower(): FAKE_SERVICE_CONTRIBUTOR} + ) + cid, roles, is_service = self._resolve_service(claims, resolver=resolver) assert is_service is True assert "IdentityAdmin" in roles diff --git a/tests/test_identity_map_wire.py b/tests/test_identity_map_wire.py index 1de9ad3..156fd29 100644 --- a/tests/test_identity_map_wire.py +++ b/tests/test_identity_map_wire.py @@ -498,6 +498,118 @@ def test_resolver_identity_map_is_store_flat_dict(self, tmp_path: Path) -> None: assert middleware.resolver._identity_map is store.flat_dict # type: ignore[union-attr] + def test_service_oid_seeded_and_resolves_via_shared_flat_dict( + self, tmp_path: Path + ) -> None: + """A service oid seeded from config lands in the SAME store/flat_dict. + + One IdentityStore serves both user and service oids (disjoint key + spaces). The resolver's ``_identity_map`` and ``_service_identity_map`` + are literally the SAME dict object as ``store.flat_dict`` -- no + separate service map, no separate store. + """ + from context_intelligence_server.config import Settings # noqa: PLC0415 + from context_intelligence_server.main import ( # noqa: PLC0415 + create_asgi_app, + get_entra_identity_store, + ) + + service_oid = "cccccccc-dddd-eeee-ffff-000011112222" + service_contributor = "my-automation-service" + + settings = Settings( + auth_mode="entra", + azure_client_id=FAKE_CLIENT_ID, + azure_tenant_id=FAKE_TENANT_ID, + entra_identities={FAKE_OID: {"id": FAKE_CONTRIBUTOR_ENTRA}}, + service_identities={service_oid: {"id": service_contributor}}, + entra_identities_store_path=str(tmp_path / "entra-identities.json"), + api_keys_store_path=str(tmp_path / "api-keys.json"), + ) + + middleware = create_asgi_app(settings=settings, _jwks_client=_StubJWKSClient()) + store = get_entra_identity_store() + assert store is not None + + # Both the user oid and the service oid landed in the ONE store. + assert store.flat_dict[FAKE_OID] == FAKE_CONTRIBUTOR_ENTRA + assert store.flat_dict[service_oid] == service_contributor + + # The resolver's user map, service map, and the store's flat_dict are + # all the SAME object -- a live admin PUT is visible to either path. + assert middleware.resolver._identity_map is store.flat_dict # type: ignore[union-attr] + assert ( + middleware.resolver._service_identity_map is store.flat_dict # type: ignore[union-attr] + ) + + +class TestIgnoredServiceIdentitiesWarning: + """service_identities is a first-boot seed; say so when it is being ignored. + + Once the store file exists, config is never re-read into it. An operator + who sets SERVICE_IDENTITIES on an already-deployed server gets no effect + at all, and the only symptom is a 403 whose message points at the + administrator rather than at the ignored config. Boot must name the + ignored oids so the wrong lever is obvious. + """ + + _SERVICE_OID = "cccccccc-dddd-eeee-ffff-000011112222" + + def _settings(self, tmp_path: Path, store_path: Path) -> Any: + from context_intelligence_server.config import Settings # noqa: PLC0415 + + return Settings( + auth_mode="entra", + azure_client_id=FAKE_CLIENT_ID, + azure_tenant_id=FAKE_TENANT_ID, + entra_identities={FAKE_OID: {"id": FAKE_CONTRIBUTOR_ENTRA}}, + service_identities={self._SERVICE_OID: {"id": "my-automation-service"}}, + entra_identities_store_path=str(store_path), + api_keys_store_path=str(tmp_path / "api-keys.json"), + ) + + def test_warns_naming_oids_ignored_because_store_already_exists( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """Store file present → config service oid is ignored → warning names it.""" + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + store_path = tmp_path / "entra-identities.json" + # An already-deployed server: the store file was written on an earlier boot. + store_path.write_text(json.dumps({FAKE_OID: {"id": FAKE_CONTRIBUTOR_ENTRA}})) + + settings = self._settings(tmp_path, store_path) + + with caplog.at_level("WARNING", logger="context_intelligence_server.main"): + create_asgi_app(settings=settings, _jwks_client=_StubJWKSClient()) + + warnings = [r.getMessage() for r in caplog.records if r.levelname == "WARNING"] + matching = [m for m in warnings if "service_identities config lists" in m] + assert matching, ( + f"expected an ignored-service_identities warning, got {warnings!r}" + ) + # The oid must be named — a warning that does not say WHICH principal is + # ignored leaves the operator exactly as stuck as no warning at all. + assert self._SERVICE_OID in matching[0] + assert "PUT /admin/identities" in matching[0] + + def test_no_warning_on_first_boot_when_config_is_actually_seeded( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """No store file → config IS seeded → the warning must not fire.""" + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + store_path = tmp_path / "entra-identities.json" + assert not store_path.exists(), "Pre-condition: first boot, no store file" + + settings = self._settings(tmp_path, store_path) + + with caplog.at_level("WARNING", logger="context_intelligence_server.main"): + create_asgi_app(settings=settings, _jwks_client=_StubJWKSClient()) + + warnings = [r.getMessage() for r in caplog.records if r.levelname == "WARNING"] + assert not [m for m in warnings if "service_identities config lists" in m] + # =========================================================================== # T3: Mode-specific accessors diff --git a/tests/test_m2_service_auth.py b/tests/test_m2_service_auth.py index a6f9781..8a888a2 100644 --- a/tests/test_m2_service_auth.py +++ b/tests/test_m2_service_auth.py @@ -306,13 +306,18 @@ async def test_cap_hw_human_write_capable( async def test_cap_sc_contributor_write_capable( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], monkeypatch: pytest.MonkeyPatch, ) -> None: - """CAP-SC: service Contributor → POST /events → 2xx.""" + """CAP-SC: service Contributor → POST /events → 2xx. + + Uses service_asgi_with_map (mapped oid) since resolving created_by + requires a service identity map hit -- an unmapped oid now 403s + before role gating even runs. + """ import context_intelligence_server.main as main_module # noqa: PLC0415 - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["Contributor"])) monkeypatch.setattr( @@ -368,13 +373,16 @@ async def test_cap_sr_w_reader_write_blocked( async def test_cap_sr_r_reader_read_capable( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], monkeypatch: pytest.MonkeyPatch, ) -> None: - """CAP-SR-r: service Reader → GET /blobs/{sid} → 2xx (require_read passes).""" + """CAP-SR-r: service Reader → GET /blobs/{sid} → 2xx (require_read passes). + + Uses service_asgi_with_map (mapped oid) -- see CAP-SC docstring. + """ import context_intelligence_server.main as main_module # noqa: PLC0415 - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["Reader"])) monkeypatch.setattr(main_module, "AsyncDiskBlobStore", _MockBlobStore) @@ -420,15 +428,16 @@ async def test_cap_sadm_w_admin_only_write_blocked( async def test_cap_sadm_a_admin_role_passes_require_admin( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], ) -> None: """CAP-SADM-a: service IdentityAdmin → GET /admin/identities → 2xx. require_admin (admin.py) checks 'IdentityAdmin' in roles; the service branch sets is_service=True and roles=["IdentityAdmin"] on scope state. - No change to admin.py needed (Q4 / §1.4). + No change to admin.py needed (Q4 / §1.4). Uses service_asgi_with_map + (mapped oid) -- see CAP-SC docstring. """ - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["IdentityAdmin"])) async with _make_client(asgi) as c: @@ -447,13 +456,16 @@ async def test_cap_sadm_a_admin_role_passes_require_admin( async def test_cap_sc_r_contributor_read_capable( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], monkeypatch: pytest.MonkeyPatch, ) -> None: - """CAP-SC-r: service Contributor → GET /blobs/{sid} → 2xx (write-capable → read-capable).""" + """CAP-SC-r: service Contributor → GET /blobs/{sid} → 2xx (write-capable → read-capable). + + Uses service_asgi_with_map (mapped oid) -- see CAP-SC docstring. + """ import context_intelligence_server.main as main_module # noqa: PLC0415 - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["Contributor"])) monkeypatch.setattr(main_module, "AsyncDiskBlobStore", _MockBlobStore) @@ -472,17 +484,18 @@ async def test_cap_sc_r_contributor_read_capable( async def test_cap_cypher_read_reader_can_reach_cypher( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], monkeypatch: pytest.MonkeyPatch, ) -> None: """CAP-cypher-read: service Reader → POST /cypher (read query) → 2xx. /cypher is wired with require_read (NOT require_write per spec §5.3). - Reader is exactly the role meant to reach /cypher + /blobs. + Reader is exactly the role meant to reach /cypher + /blobs. Uses + service_asgi_with_map (mapped oid) -- see CAP-SC docstring. """ from context_intelligence_server.main import app # noqa: PLC0415 - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["Reader"])) # Mock the neo4j QUERY (read-intent) driver on app.state -- /cypher reads @@ -508,7 +521,7 @@ async def test_cap_cypher_read_reader_can_reach_cypher( async def test_cap_cypher_soft_reader_can_mutate_soft_m2( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], monkeypatch: pytest.MonkeyPatch, ) -> None: """CAP-cypher-soft: service Reader + MUTATING Cypher query → 2xx + mutation succeeds. @@ -520,10 +533,11 @@ async def test_cap_cypher_soft_reader_can_mutate_soft_m2( HARDENS TO 403 AT M3 via a read-only Neo4j DB user on the read path. Do NOT assert "cannot mutate" — that assertion would be false at M2. + Uses service_asgi_with_map (mapped oid) -- see CAP-SC docstring. """ from context_intelligence_server.main import app # noqa: PLC0415 - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["Reader"])) # Mock the neo4j QUERY (read-intent) driver — /cypher reads @@ -671,6 +685,37 @@ def test_disjoint_config_boots_cleanly( # Must not raise — disjoint config is valid create_asgi_app(settings=settings, _jwks_client=_StubJWKSClient(public_key)) + def test_empty_service_identities_boots_without_error( + self, + rsa_keypair_cap: tuple[Any, Any], + tmp_path: Any, + ) -> None: + """service_identities={} boots cleanly (bootstrap state, not a startup error). + + This was previously a hard ValidationError. An explicit empty map is + now accepted -- the same allow_empty=True treatment entra_identities + already had -- so a fresh deployment can onboard its first service + identity at runtime via PUT /admin/identities. + """ + from context_intelligence_server.config import Settings # noqa: PLC0415 + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + _, public_key = rsa_keypair_cap + + settings = Settings( + auth_mode="entra", + azure_client_id=FAKE_CLIENT_ID, + azure_tenant_id=FAKE_TENANT_ID, + entra_identities={FAKE_OID_HUMAN: {"id": FAKE_CONTRIBUTOR_HUMAN}}, + service_identities={}, + entra_identities_store_path=str(tmp_path / "entra-ids-empty-svc.json"), + api_keys_store_path=str(tmp_path / "api-keys-empty-svc.json"), + ) + + # Must not raise -- an explicit empty service_identities map is a + # supported bootstrap state. + create_asgi_app(settings=settings, _jwks_client=_StubJWKSClient(public_key)) + # --------------------------------------------------------------------------- # /status additive fields (M2) @@ -727,15 +772,16 @@ class TestP3VerticalSlice: Captures queue-append bytes to assert the final created_by value. """ - async def test_p3_unmapped_service_contributor_created_by_is_appid( + async def test_p3_unmapped_service_contributor_is_rejected( self, service_asgi: tuple[Any, Any], monkeypatch: pytest.MonkeyPatch, ) -> None: - """P3 unmapped: service Contributor, oid NOT in any map → 202, created_by == appid. + """P3 unmapped: service Contributor, oid NOT in the service map → 403, no event ingested. service_asgi has no service_identities, so FAKE_OID_SERVICE is unmapped. - The resolver falls through to appid as the stable created_by. + The resolver now fails loud (mirrors the user branch's + unmapped-oid 403) instead of falling back to appid as created_by. """ import context_intelligence_server.main as main_module # noqa: PLC0415 @@ -768,17 +814,13 @@ async def _fake_append(worker_key: str, raw: bytes) -> None: headers={"Authorization": f"Bearer {token}"}, ) - assert resp.status_code == 202, ( - f"P3-unmapped: expected 202 for service Contributor, " + assert resp.status_code == 403, ( + f"P3-unmapped: expected 403 for unmapped service oid, " f"got {resp.status_code}: {resp.text}" ) - assert len(captured) == 1, ( - f"P3-unmapped: expected 1 queue append, got {len(captured)}" - ) - body_obj = json.loads(captured[0]) - assert body_obj["created_by"] == FAKE_APPID, ( - f"P3-unmapped: created_by should be appid {FAKE_APPID!r}, " - f"got {body_obj.get('created_by')!r}" + assert len(captured) == 0, ( + f"P3-unmapped: expected NO queue append for a rejected request, " + f"got {len(captured)}" ) async def test_p3_mapped_service_oid_created_by_is_friendly_name( @@ -1020,14 +1062,16 @@ async def test_no_role_403_names_contributor_and_reader_roles( async def test_require_write_403_names_write_role( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], ) -> None: """Service Reader → POST /events → require_write 403 naming 'Contributor'. Reader passes the resolver (qualifying role), but require_write rejects it because Contributor is absent. The 403 detail must name Contributor. + Uses service_asgi_with_map (mapped oid) so the resolver itself succeeds + (an unmapped oid now 403s before require_write ever runs). """ - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["Reader"])) async with _make_client(asgi) as c: @@ -1052,14 +1096,16 @@ async def test_require_write_403_names_write_role( async def test_require_read_403_names_reader_and_write_roles( self, - service_asgi: tuple[Any, Any], + service_asgi_with_map: tuple[Any, Any], ) -> None: """Service IdentityAdmin-only → GET /blobs → require_read 403 naming Reader and Contributor. IdentityAdmin passes the resolver but is neither Contributor nor Reader, so require_read rejects it. The 403 detail must name both read roles. + Uses service_asgi_with_map (mapped oid) so the resolver itself succeeds + (an unmapped oid now 403s before require_read ever runs). """ - private_key, asgi = service_asgi + private_key, asgi = service_asgi_with_map token = _sign_jwt(private_key, _service_claims(roles=["IdentityAdmin"])) async with _make_client(asgi) as c: