Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
7b1cefe
feat(analytics): metric-results keys on person_id — the identity cutover
mozhaev-dev Jul 31, 2026
8eaad13
fix(analytics): close the cutover review gaps — validator probe, cont…
mozhaev-dev Jul 31, 2026
70cecb4
Merge remote-tracking branch 'upstream/fix/2035-metric-results-authz-…
mozhaev-dev Jul 31, 2026
f285e08
feat(analytics,identity): integrate the visibility gate with the pers…
mozhaev-dev Jul 31, 2026
60cddef
fix(analytics): un-shadow the peer CTEs' aggregate alias (ILLEGAL_AGG…
mozhaev-dev Jul 31, 2026
93bb535
test(e2e): bind every fixture persona in identity, not just requested…
mozhaev-dev Jul 31, 2026
99aa3ce
test(analytics): put the gate's live tests on the UUID contract
mozhaev-dev Jul 31, 2026
b564d97
fix(identity,analytics): publish the UUID visibility contract; reject…
mozhaev-dev Jul 31, 2026
319ded5
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Jul 31, 2026
19b1500
feat(identity): resolve profiles by person_id
mozhaev-dev Jul 31, 2026
724d0ff
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Jul 31, 2026
71c5681
docs(analytics): restore the canonical key order of the OpenAPI doc
mozhaev-dev Jul 31, 2026
75d3d9c
fix(ingestion): match the dumped terminator style for the contract-ve…
mozhaev-dev Jul 31, 2026
8197f56
fix(metrics): cap submitted entity ids before parsing, and pin the ga…
mozhaev-dev Jul 31, 2026
9bf194f
feat(analytics): key the persons facade on person_id
mozhaev-dev Jul 31, 2026
c19a853
fix(analytics): gate person filters on the legacy query endpoints
mozhaev-dev Jul 31, 2026
3c3f39e
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Jul 31, 2026
bf4c4f4
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Aug 1, 2026
9fb2914
test(analytics): key the drilldown-capability live test on a person id
mozhaev-dev Aug 1, 2026
8d604ab
docs(analytics): state the drilldown key invariant without an issue n…
mozhaev-dev Aug 1, 2026
ffc2396
refactor(metrics): make entity_id the canonical person id in gold
mozhaev-dev Aug 3, 2026
19a0b5f
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Aug 3, 2026
3ab4f32
fix(metrics): collapse a person's day flags across their source aliases
mozhaev-dev Aug 3, 2026
f1865d4
chore(ddl): regenerate the connectors-ddl snapshot for canonical gold
mozhaev-dev Aug 3, 2026
7a2aee5
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Aug 3, 2026
187ed8a
fix(metrics): serve ai and collab observations at canonical grain
mozhaev-dev Aug 3, 2026
79c7f0c
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Aug 3, 2026
1cfba6d
fix(analytics): cut metric-drilldown over to person_id and gate it
mozhaev-dev Aug 3, 2026
5d95532
style: rustfmt after the drilldown and wildcard fixes
mozhaev-dev Aug 3, 2026
409b94c
refactor(metrics): one build-scoped identity snapshot for every relation
mozhaev-dev Aug 3, 2026
6eb954e
refactor(analytics): type the entity selection instead of pairing str…
mozhaev-dev Aug 3, 2026
782a11b
Merge remote-tracking branch 'upstream/main' into feat/metrics-person…
mozhaev-dev Aug 3, 2026
26f5b07
chore(ddl): regenerate the snapshot for resolved evidence and cohort …
mozhaev-dev Aug 3, 2026
bfc6d8c
style: rustfmt import order and ruff over the changed e2e files
mozhaev-dev Aug 3, 2026
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
10 changes: 7 additions & 3 deletions docs/components/backend/analytics/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ 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 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` | IDOR on person entity ids | `POST /v1/metric-results`, `POST /v1/metrics/{id}/query`, `POST /v1/metrics/queries` | Requested person ids — `entity.ids`, or `person_id` in `$filter` — filtered through identity's `POST /v1/visible-persons`; the whole request is refused when any one is not visible | Gate tests + filter-extraction 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 @@ -137,7 +137,9 @@ Every ClickHouse query includes `insight_tenant_id`. User-supplied OData `$filte

**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.
**IDOR prevention on the legacy query paths**: `POST /v1/metrics/{id}/query` and `POST /v1/metrics/queries` accept `person_id eq …` / `person_id in (…)` in `$filter`. Those ids go through the same gate as `/v1/metric-results` before any ClickHouse work, with the same failure matrix, so one id outside the caller's visible set refuses the whole request with 403. A `person_id` value that is not a person UUID is a 400 — since the cutover the column is a UUID, so a pre-cutover email would otherwise surface as a cast failure the caller cannot act on.

**Planned — IDOR prevention on org units**: validating `org_unit_id` from `$filter` against the caller's scope is *not* implemented — no visibility rule for org units exists yet, and failing them closed would take out every department distribution metric. Those filter values remain reachable for any org unit in the tenant.

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

Expand All @@ -147,7 +149,9 @@ Every ClickHouse query includes `insight_tenant_id`. User-supplied OData `$filte

The frontend sends Insight person IDs (golden records from Identity Resolution). The service resolves them to source-specific aliases transparently. The frontend never knows about emails, usernames, or source account IDs.

**Why**: Source-specific identifiers are an implementation detail. The frontend works with a unified person model.
Since the identity cutover this holds for every person-keyed route: `POST /v1/metric-results` takes canonical person UUIDs in `entity.ids`, and `GET /v1/persons/{person_id}` — the profile facade that delegates to identity's `POST /v1/profiles` — takes the same key, so an id read off a metric result resolves to a profile with no second mapping. A pre-cutover email in either position is a 400, never a 404 that would read as "no such person".

**Why**: Source-specific identifiers are an implementation detail. The frontend works with a unified person model. One key across the routes means a caller never has to hold two identifiers for the same person.

#### Server-Side Threshold Evaluation

Expand Down
6 changes: 4 additions & 2 deletions docs/components/backend/analytics/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,7 @@
"MetricResultsEntity": {
"properties": {
"ids": {
"description": "Canonical person UUIDs (since the identity cutover; the\npre-cutover email shape is rejected with a 400).",
"items": {
"type": "string"
},
Expand All @@ -1444,6 +1445,7 @@
"MetricResultsEntityDto": {
"properties": {
"ids": {
"description": "Canonical person UUIDs (since the identity cutover; the\npre-cutover email shape is rejected with a 400).",
"items": {
"type": "string"
},
Expand Down Expand Up @@ -4642,7 +4644,7 @@
"summary": "Update a threshold"
}
},
"/v1/persons/{email}": {
"/v1/persons/{person_id}": {
"get": {
"operationId": "analytics_api.persons.get",
"responses": {
Expand Down Expand Up @@ -4732,7 +4734,7 @@
"bearerAuth": []
}
],
"summary": "Resolve a person by email"
"summary": "Resolve a person by canonical person id"
}
},
"/v1/queries": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ 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). |
| `POST /v1/profiles` | Profile lookup by `value_type`: `email` (tenant-wide), `id` (source-native account id, needs both source fields), or `person_id` (the canonical UUID — the key the metrics runtime and the SPA routes use since the identity cutover). 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 canonical person ids (UUIDs) 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
Expand Up @@ -54,10 +54,20 @@ users.
## 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.
takes canonical person ids (UUIDs) 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.

The request shape is UUIDs because the metrics runtime keys on `person_id`
since the identity cutover; the email-taking first cut of this endpoint
(and its `resolve_person_ids_by_emails` position-keyed resolution, needed
because `value_id` compares case- and accent-insensitively) is gone with
it. On a wildcard grant the answer is the request intersected with the
tenant's persons log: the grant covers everyone in the tenant, not everyone
whose UUID the caller can type, and consumers read this answer as
authorization — echoing a foreign tenant's id back would confirm it as
visible.

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

Expand All @@ -69,7 +79,9 @@ Three properties keep it least-privilege despite the missing admin gate:
`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.
an existence oracle beyond what the caller's own grants already imply —
a wildcard holder learns tenant membership, which their grant lets them
enumerate through `POST /v1/profiles` anyway.

Roles stay out of the predicate. Holding the `admin` role confers no
visibility, exactly as before — administering identity and seeing people
Expand Down Expand Up @@ -132,8 +144,7 @@ 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`
- SQL: `subchart_repo::visible_targets`, `subchart_repo::has_wildcard_grant`
- 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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Architecture-shaping decisions are captured as ADRs in
| Requirement | Design Response |
|-------------|-----------------|
| [`cpt-insightspec-fr-identity-lookup-resolve-by-email`](PRD.md#resolve-email-to-person_id) | `persons_repo::resolve_person_ids_by_email` issues a `SELECT person_id FROM persons WHERE value_type='email' AND value_id=? AND insight_tenant_id=? ORDER BY created_at DESC LIMIT 1` against the `idx_value_id` covered index. |
| `cpt-insightspec-fr-identity-lookup-resolve-by-person-id` | `value_type='person_id'` needs no resolution step: `api/handlers.rs::resolve_person_id_mode` validates the UUID (nil and non-UUID are 400s, never a silent empty resolution), rejects the source fields (a person id is tenant-wide), and confirms the person exists in the tenant via `persons_repo::person_exists`; an unobserved id yields no candidate so the handler answers 404 like an unknown email. Visibility applies unchanged, so a person's name and their metrics answer to ONE permission — the SPA routes on `person_id` since the identity cutover, and a person without a current email is reachable only this way. |
| [`cpt-insightspec-fr-identity-lookup-hydrate`](PRD.md#hydrate-person-attributes) | `persons_repo::fetch_person_observations` runs a `ROW_NUMBER() OVER (PARTITION BY ...)` CTE returning one row per (source, value_type); the assembler in `domain/profile.rs` then picks the latest across sources. |
| [`cpt-insightspec-fr-identity-lookup-404`](PRD.md#not-found-returns-rfc-7807) | The profile handler returns an RFC 7807 body with `type=urn:insight:error:person_not_found`, `status=404` when the resolve step matches nothing. |
| [`cpt-insightspec-fr-identity-lookup-400-tenant`](PRD.md#missing-tenant-returns-rfc-7807) | The tenant comes from the verified JWT's `tenant_id` claim (oidc-authn-plugin SecurityContext); when no tenant resolves the handler returns an RFC 7807 body (`type=urn:insight:error:tenant_unresolved`, `status=400`). |
Expand All @@ -80,7 +81,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-visible-persons-batch` | `POST /v1/visible-persons` (`api/visible_persons.rs::filter_visible_persons`) answers which of the requested canonical person ids (UUIDs) 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 requested ids against it; `has_wildcard_grant` short-circuits the traversal (echoing the request, so its answer is a subset of the input rather than a tenant-existence check). No email resolution step exists: the metrics runtime keys on `person_id` since the identity cutover, so the ids arrive canonical. 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
14 changes: 12 additions & 2 deletions docs/components/backend/identity-resolution/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@
"type": "string"
},
"value_type": {
"description": "Which key `value` carries: an email (tenant-wide), a source-native account id (needs both source fields), or the canonical person UUID.",
"enum": [
"email",
"id",
"person_id"
],
"nullable": true,
"type": "string"
}
Expand Down Expand Up @@ -125,22 +131,26 @@
},
"VisiblePersonsCommandModel": {
"properties": {
"emails": {
"person_ids": {
"items": {
"format": "uuid",
"type": "string"
},
"maxItems": 1000,
"minItems": 1,
"type": "array"
}
},
"required": [
"emails"
"person_ids"
],
"type": "object"
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
"VisiblePersonsResponse": {
"properties": {
"visible": {
"items": {
"format": "uuid",
"type": "string"
},
"type": "array"
Expand Down
Loading
Loading