Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions docs/components/backend/analytics/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ The API Gateway mounts this service at `/api/analytics`. All endpoints are versi
| `cpt-insightspec-nfr-be-tenant-isolation` | Tenant data isolation | Query builder | `insight_tenant_id = ?` injected on every query from SecurityContext | Cross-tenant query test |
| `cpt-insightspec-nfr-be-api-conventions` | RFC 9457, cursor pagination | All endpoints | `{ items, page_info }` envelopes, Problem Details errors, OData query conventions | Response format tests |
| `cpt-insightspec-nfr-be-rate-limiting` | Per-route rate limiting | API Gateway (upstream) | Governor-based rate limiter | Load test |
| `cpt-insightspec-nfr-be-idor-prevention` | IDOR on org_unit_id | Query engine | `org_unit_id` from `$filter` validated against user's AccessScope before query execution | Cross-org-unit query test |
| `cpt-insightspec-nfr-be-idor-prevention` | IDOR on person entity ids | `POST /v1/metric-results` | Requested person ids filtered through identity's `POST /v1/visible-persons`; the whole request is refused when any one is not visible | Gate tests + e2e 403 cases |
| `cpt-insightspec-nfr-be-idor-prevention-org-unit` | IDOR on org_unit_id | Query engine | Planned: `org_unit_id` from `$filter` validated against the caller's AccessScope before execution. Not implemented — the OData `$filter` path carries no visibility check | Cross-org-unit query test |

### 1.3 Architecture Layers

Expand Down Expand Up @@ -132,9 +133,11 @@ The service never exposes ClickHouse table names to the frontend. All queries go

- [ ] `p1` - **ID**: `cpt-insightspec-principle-analytics-security-filters`

Every ClickHouse query includes `insight_tenant_id` and org-unit scope filters injected from the SecurityContext and AccessScope. User-supplied OData `$filter` values are ANDed with security filters. Users can narrow their view but never widen it.
Every ClickHouse query includes `insight_tenant_id`. User-supplied OData `$filter` values are ANDed with security filters, so users can narrow their view but never widen it.

**IDOR prevention**: When the frontend includes `org_unit_id eq 'uuid'` in `$filter`, the query engine validates that the requested org unit is within the user's AccessScope before executing the query. Accepting a client-supplied UUID without authorization check would allow any user within a tenant to query any team's data by guessing or enumerating UUIDs.
**IDOR prevention on person entities**: `POST /v1/metric-results` resolves the caller from the gateway JWT and asks identity, in one batch call, which of the requested person ids that caller may see (`POST /v1/visible-persons`). Any requested id outside the answer refuses the whole request with 403 — never a partial response, which would be indistinguishable from absent data. Reaching identity is required: an unconfigured or unreachable identity service is a server error, so an authorization backend that is down cannot read as "permitted".

**Planned — IDOR prevention on org units**: validating `org_unit_id` from `$filter` against the caller's AccessScope is *not* implemented. The `POST /v1/metrics/{id}/query` and `POST /v1/metrics/queries` paths interpolate client-supplied `person_id` / `org_unit_id` filter values with no visibility check, so they remain reachable for any id in the tenant.

**Why**: Tenant isolation and org-scoped visibility are enforced at the query level, not at the application level.

Expand Down Expand Up @@ -417,7 +420,7 @@ GET /v1/persons/{person_id}/aliases
| 400 | `invalid_order_by` | Column not in metric's schema |
| 404 | `metric_not_found` | Metric ID doesn't exist or is disabled |
| 401 | `unauthorized` | Missing or invalid JWT |
| 403 | `forbidden` | RBAC role insufficient or org_unit_id not in user's AccessScope |
| 403 | `forbidden` | RBAC role insufficient, or a requested person id is outside the caller's visible set |

### 3.4 Internal Dependencies

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ ClickHouse coordinates for the persons-seed reader, and — when set —
| Endpoint | Description |
|---|---|
| `POST /v1/profiles` | Profile lookup by email or source-native id. Body-form replacement for the retired path-form `GET /v1/persons/{email}` (dropped with the .NET decommission — zero callers). |
| `POST /v1/visible-persons` | Filters a list of emails to the ones the caller may see. Authenticated, not admin-gated — the caller comes from the gateway JWT, so the answer is always their own visible set (ADR-0015). |
| `GET /health` | DB ping. 200 / 503. |
| `GET /healthz` | Process liveness. 200 `text/plain "ok"`. |

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# ADR-0015: Self-Scoped Visibility Read Without Admin

<!-- toc -->

- [Context and Problem Statement](#context-and-problem-statement)
- [Decision Drivers](#decision-drivers)
- [Considered Options](#considered-options)
- [Decision Outcome](#decision-outcome)
- [Consequences](#consequences)
- [Confirmation](#confirmation)
- [Pros and Cons of the Options](#pros-and-cons-of-the-options)
- [Non-admin, self-scoped batch read (chosen)](#non-admin-self-scoped-batch-read-chosen)
- [Admin-gate it like the rest of the family](#admin-gate-it-like-the-rest-of-the-family)
- [Let each consumer derive the visible set itself](#let-each-consumer-derive-the-visible-set-itself)
- [More Information](#more-information)
- [Traceability](#traceability)

<!-- /toc -->

**ID**: `cpt-insightspec-adr-0015-self-scoped-visibility-read-without-admin`

**Status:** Accepted

## Context and Problem Statement

ADR-0012 gates every read of the OrgChart Visibility tables behind
`CallerAdminCheck`, and names as a decision driver that the whole family
"behaves identically under the same filter". It also leaves the door open:
a later ADR may relax specific reads, and offers
`GET /v1/person-roles?person=<self>` as the example.

Consumers now need that relaxation. A service serving per-person data must
know which of the people in a request the caller is allowed to see, and it
must ask on every request — the answer changes with grants and org moves.
Under an admin-only rule the only callers able to ask are administrators,
which is precisely inverted: the question is asked *on behalf of* ordinary
users.

## Decision Drivers

- The visible set is a property of the caller, so answering it for the
caller reveals nothing the caller may not already reach.
- Consumers must not re-derive visibility. Two implementations of
"who may see whom" drift, and the copy is the one that gets it wrong.
- One question per request, not one per person: the answer is needed for a
whole batch on a hot path.

## Considered Options

- Non-admin, self-scoped batch read (chosen)
- Admin-gate it like the rest of the family
- Let each consumer derive the visible set itself

## Decision Outcome

`POST /v1/visible-persons` is authenticated but **not** admin-gated. It
takes emails and answers with the subset the caller may see, evaluated by
the same union the rest of the service uses: the caller, their active
grants, the whole tenant on a wildcard grant, and their `org_chart`
descendants.

Three properties keep it least-privilege despite the missing admin gate:

- **Self-scoped.** The caller is taken from the gateway JWT. There is no
acting-as parameter, so a caller can only ever ask about their own
visible set.
- **No new disclosure.** The response echoes back a subset of the ids the
caller supplied. Everything it reveals is already reachable through
`POST /v1/profiles` one id at a time.
- **Absence carries the denial.** An id the caller may not see and an id
that resolves to nobody are both simply absent, so the endpoint is not
an existence oracle.

Roles stay out of the predicate. Holding the `admin` role confers no
visibility, exactly as before — administering identity and seeing people
remain separate powers.

### Consequences

- **Positive:** consumers gate on the same predicate the service enforces,
so authorization cannot drift between services.
- **Positive:** a batch answer replaces one traversal per person.
- **Negative:** ADR-0012's "every endpoint behaves identically" no longer
holds for the family as a whole. A reader must consult per-endpoint auth
rather than assume the family rule; the route lives outside the
`/v1/visibility` prefix to make the difference visible in the path.
- **Negative:** a consumer that fails closed on this endpoint makes
identity a hard dependency of its own read path.

### Confirmation

Live-MariaDB cases assert the predicate directly: a caller with no reports
still sees themselves, a manager sees a transitive descendant and not an
unrelated person, an explicit grant reaches outside the reporting line, a
wildcard grant covers the tenant, and a holder of the `admin` role sees
no one extra. The e2e suite asserts the non-admin path end to end with a
non-admin caller.

## Pros and Cons of the Options

### Non-admin, self-scoped batch read (chosen)

- **Pro:** answers the question the consumers actually have, for the
callers who actually have it.
- **Pro:** one round trip per request; a wildcard grant short-circuits
before any traversal.
- **Con:** breaks the family's uniform auth shape (see Consequences).

### Admin-gate it like the rest of the family

- **Pro:** preserves ADR-0012 verbatim; nothing to re-reason about.
- **Con:** unusable for its purpose. Ordinary users are the ones whose
visible set must be checked, and they would be refused.
- **Con:** pushes consumers toward a service token, which would make every
check un-attributable to a person.

### Let each consumer derive the visible set itself

- **Pro:** no new endpoint.
- **Con:** a second implementation of the visibility rule. Grants and
wildcard grants are easy to miss, and the copy fails open when it does.
- **Con:** requires reaching another service's tables, against the
service-owned-schema rule.

## More Information

The predicate itself is unchanged by this ADR — only who may ask it, and
for how many people at once. A consumer that cannot reach this endpoint
must fail closed; treating an unreachable authorization backend as an
allow would be the failure this endpoint exists to prevent.

## Traceability

- Endpoint: `services/identity-resolution/src/api/visible_persons.rs`
- SQL: `subchart_repo::visible_targets`, `subchart_repo::has_wildcard_grant`,
`persons_repo::resolve_person_ids_by_emails`
- Tests: `infra::db::visible_set_live_tests`,
`src/ingestion/tests/e2e/identity/test_visible_persons.py`
- Related: ADR-0012 (admin-only reads — relaxed here for one read),
ADR-0010 (org-chart cache), ADR-0011 (persons collation)
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Architecture-shaping decisions are captured as ADRs in
- [`cpt-insightspec-adr-0010-org-chart-cache`](ADR/0010-org-chart-cache.md) — Materialised SCD2 cache for person parent/child edges (`org_chart`).
- [`cpt-insightspec-adr-0011-persons-relax-uniqueness-and-collation`](ADR/0011-persons-relax-uniqueness-and-collation.md) — Persons relax UNIQUE + switch `value_id` to case-insensitive collation.
- [`cpt-insightspec-adr-0012-admin-only-orgchart-visibility-reads`](ADR/0012-admin-only-orgchart-visibility-reads.md) — Admin-only reads on `/v1/visibility`, `/v1/roles`, `/v1/person-roles`.
- [`cpt-insightspec-adr-0015-self-scoped-visibility-read-without-admin`](ADR/0015-self-scoped-visibility-read-without-admin.md) — `POST /v1/visible-persons` answers the caller's own visible set without the admin gate.
- [`cpt-insightspec-adr-0013-roles-hard-delete-with-in-use-guard`](ADR/0013-roles-hard-delete-with-in-use-guard.md) — `roles` hard-DELETE guarded by active-assignment count (422 `urn:insight:error:role_in_use`).
- [`cpt-insightspec-adr-0014-last-admin-protection`](ADR/0014-last-admin-protection.md) — Refuse to revoke the last active admin assignment in a tenant.

Expand All @@ -79,6 +80,7 @@ Architecture-shaping decisions are captured as ADRs in
| [`cpt-insightspec-fr-identity-schema-relax-uniqueness`](PRD.md#schema-allows-recording-state-transitions) | Migration `004_persons_relax_constraints.sql` drops `UNIQUE uq_person_observation` on `(..., value_hash)` and adds the same name on `(..., created_at)`. The seeder's `INSERT IGNORE` in step 7 now dedupes by `created_at` (re-runs idempotent) while genuine transitions on the same partition (Active->Inactive->Active) persist as separate rows. ADR-0011 documents the design decision. |
| [`cpt-insightspec-fr-identity-schema-case-insensitive-value-id`](PRD.md#value-comparisons-are-case-insensitive) | The same migration `ALTER COLUMN value_id MODIFY ... COLLATE utf8mb4_unicode_ci`. `idx_value_id` rebuilds under the new collation; existing SQL (`WHERE value_id = @x`) is now case-insensitive without code changes. `value_full_text` is already `utf8mb4_unicode_ci`; `value` (TEXT) uses table default `utf8mb4_unicode_ci`; `value_hash` (CHAR ascii) stays `ascii_bin` as it is a SHA-256 digest. |
| [`cpt-insightspec-fr-identity-profile-resolve`](PRD.md#resolve-profile-by-email-or-source-native-id) | The `POST /v1/profiles` handler (`api/handlers.rs::resolve_profile`) routes by the request's `value_type` (`email` or `id`) to `persons_repo::resolve_person_ids_by_email` / `resolve_person_ids_by_source_id`. Both queries are CTEs with partition `(insight_tenant_id, person_id, insight_source_type, insight_source_id, value_type)` and `rn=1` filter — the canonical latest-per-source-instance projection. |
| `cpt-insightspec-fr-identity-visible-persons-batch` | `POST /v1/visible-persons` (`api/visible_persons.rs::filter_visible_persons`) answers which of the requested emails the caller may see. `subchart_repo::visible_targets` materialises the visible-set union once — caller, active grants, the whole tenant on a wildcard grant, `org_chart` descendants — and joins the resolved candidates against it; `has_wildcard_grant` short-circuits the traversal. Emails resolve through `persons_repo::resolve_person_ids_by_emails`, keyed by input position because `value_id` compares case- and accent-insensitively. Roles are absent from the predicate, so the `admin` role confers no visibility (ADR-0015). |
| [`cpt-insightspec-fr-identity-profile-ambiguous-422`](PRD.md#surface-single-result-invariant-via-422) | The resolve step distinguishes found / not-found / ambiguous. When the reader returns `>1` distinct `person_id`, the handler emits an RFC 7807 extension body carrying the lookup body + the matched `person_ids` list with status 422. |
| [`cpt-insightspec-fr-identity-profile-ids-list`](PRD.md#project-full-alias-list-on-response) | `persons_repo::current_source_ids_for_person` returns the latest `value_type='id'` per source instance; the profile assembler ships the list unchanged into the `ProfileResponse` wire shape (`domain/profile.rs::ProfileIdEntry`). |
| [`cpt-insightspec-fr-identity-profile-org-tree`](PRD.md#project-the-same-org-tree-shape-as-v1persons) | The profile handler hydrates the same `Person` tree (`hydrate_person`) that the retired GET endpoint returned, copying `supervisor_email` / `supervisor_name` / `parent_email` / `parent_id` / `parent_person_id` / `subordinates` straight off the projection. Identical `Person` shape across callers — guaranteed by reusing the recursion. |
Expand Down
57 changes: 57 additions & 0 deletions docs/components/backend/identity-resolution/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,34 @@
"reason"
],
"type": "object"
},
"VisiblePersonsCommandModel": {
"properties": {
"emails": {
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"emails"
],
"type": "object"
},
"VisiblePersonsResponse": {
"properties": {
"visible": {
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"visible"
],
"type": "object"
}
}
},
Expand Down Expand Up @@ -588,6 +616,35 @@
"Insight.Identity.Api"
]
}
},
"/v1/visible-persons": {
"post": {
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/VisiblePersonsCommandModel"
}
}
},
"required": true
},
"responses": {
"200": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/VisiblePersonsResponse"
}
}
},
"description": "OK"
}
},
"tags": [
"Insight.Identity.Api"
]
}
}
},
"tags": [
Expand Down
4 changes: 1 addition & 3 deletions src/backend/services/analytics/src/api/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ pub async fn get_person(

// Forward the caller's gateway JWT to identity (G1): this is a user-context
// fan-out, so it propagates the incoming Authorization header verbatim.
let authorization = headers
.get(axum::http::header::AUTHORIZATION)
.and_then(|v| v.to_str().ok());
let authorization = super::forwarded_authorization(&headers);

let person = state
.identity
Expand Down
Loading
Loading