MemberJunction / MemberJunction/MJ

Security: adopt least-privilege filtering of client-shipped metadata (AllMetadata / MJ_Metadata dataset)

Open
#3,485 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## Summary

MJ ships the entire `MJ_Metadata` dataset — every row and every column of ~30 metadata entities — to every authenticated browser client at startup, and persists it in the browser's IndexedDB. There is **no per-user filtering, projection, or redaction anywhere in the pipeline**. This is a long-standing convention ("all metadata ships"), and it should change to a least-privilege model: users should only receive the metadata they need to operate, not a complete map of what everyone else can access and how the system is secured.

This surfaced during review of the field-level security plan (#3367): shipping `EntityFieldPermission` records to all clients would reveal *which roles are denied which sensitive fields* — for a feature about compensation/donor confidentiality, the restriction shape is itself sensitive. We agreed to ship them for now for consistency with the existing convention, and to fix the convention holistically here.

## Current pipeline (no filtering exists)

`ProviderBase.Config()` → `GetAllMetadata()` → GraphQL `GetDatasetByName("MJ_Metadata")` → `DatasetResolver.GetDatasetByName` → `SELECT * FROM ` × 30 → client `AllMetadata` → IndexedDB (survives until logout).

- `DatasetResolver` performs no entity-permission check for browser sessions (the only gate, `CheckAPIKeyScopeAuthorization`, is a no-op for OAuth/JWT callers) and does not pass `contextUser` (`packages/MJServer/src/resolvers/DatasetResolver.ts`).
- Every `DatasetItem` row for `MJ_Metadata` has `Columns = NULL` and `WhereClause = NULL`, so the provider emits `SELECT *` with no WHERE for all 30 items (`packages/GenericDatabaseProvider/src/GenericDatabaseProvider.ts`, `getColumnsForDatasetItem`).
- No RLS is applied on this path; entities the user has zero permissions on ship in full.
- Schema filters exist (`BuildDatasetFilterFromConfig`) but are explicitly CodeGen-only: "We always load all schemas" (`packages/MJCore/src/generic/providerBase.ts` ~3839).
- The payload is written to IndexedDB twice (typed `AllMetadata` snapshot + raw dataset blob) and cleared only on explicit logout — so it survives on disk on shared/managed devices.

## What ships today that a least-privilege model would withhold or reduce

Ordered roughly by sensitivity:

1. **`RowLevelSecurityFilters` — raw SQL filter templates.** `FilterText` (the WHERE-clause template) and `PlatformVariants` ship for every RLS filter. A constrained user can read the exact predicate constraining them and the token vocabulary (`{{UserID}}`, etc.) — a blueprint of the row-security design. (`securityInfo.ts` ~481-515)
2. **`EntityPermissions` — the complete role → entity CRUD matrix for every role**, including `RoleSQLName` and which RLS filter is bound to each verb per role. Every user can see what every other role may do. (`entityInfo.ts` ~303-334)
3. **`EntityFieldPermission` (incoming, #3367)** — will add the role → field matrix for the most sensitive columns in the system (compensation, donor data) under the same convention.
4. **`Queries` + `QuerySQLs` — raw SQL of every saved query**, per dialect, including `OriginalSQL` and `CacheValidationSQL`, regardless of whether the user can run the query (`UserCanRun` is evaluated client-side over the already-shipped SQL). Query SQL reveals joins, schema/table names, business logic, and hardcoded filter constants. `QueryPermissions` (role → query grants) also ship. (`queryInfo.ts`)
5. **`Authorizations` / `AuthorizationRoles` / `AuditLogTypes`** — the full authorization → role Allow/Deny matrix, plus which action types are audited (and by omission, which are not). (`securityInfo.ts` ~646-879)
6. **`Roles`** — the full org role list including `Description`, `SQLName` (DB-level role name), and `DirectoryID` (IdP/AD group object ID). (`securityInfo.ts` ~436-470)
7. **Entity/schema internals for all schemas** — `BaseTable`, `BaseView`, `SchemaName`, `spCreate`/`spUpdate`/`spDelete`, full-text objects, `AllowDirectSQL*` flags, plus every `EntityField` (including `GeneratedValidationFunctionCode` and encryption topology: `Encrypt` flags + `EncryptionKeyID`s), for entities the user has no permission to touch.
8. **`Dashboards` / `Dashboard_Categories`** — full rows for *every user's* dashboards including `UserID`, `User` display name, `UIConfigDetails`, and `Code`. Notably these are not even consumed into the typed `AllMetadata` (no `AllMetadataArrays` entry) — they ship on the wire and land in IndexedDB for nothing. This is also the one place other users' identities leak into the payload.
9. **`ApplicationSettings` / `EntitySettings`** — free-form Name/Value config tables shipped wholesale. Nothing prevents a deployment from storing a token or internal URL in one; a key/value bag broadcast to all browsers is a standing secret-leak hazard.
10. **`Libraries`** — `ExportedItems`, `TypeDefinitions`, `SampleCode`: internal API surface with no runtime need on most clients.
11. **`ExplorerNavigationItems`** — all routes including inactive/admin ones, plus internal `Comments`.
12. **Secondary datasets, same unchecked path** — e.g. `Template_Metadata` ships full template body text (`TemplateContents`) to Explorer clients via the same `GetDatasetByName` mechanism with no permission check.

Two adjacent observations worth capturing while in here:

- **Dataset decryption is skipped only by accident**: `PostProcessRows` decryption is gated on `contextUser`, which the resolver happens not to pass. If someone "fixes" the resolver to pass it, `Encrypt=true` field plaintext would start shipping in datasets. The skip should become explicit policy, not a side effect.
- The `Columns` projection mechanism on `DatasetItem` already exists and is honored — it's just unused (`NULL` everywhere). Some reductions here are pure metadata changes, no code.

## Proposed direction

Adopt a **need-to-know tiering** for client-bound metadata:

- **Tier A — operational shape (ship to all):** entity/field definitions the client needs to render forms and grids for entities the user can access, relationships, value lists, applications, navigation the user can reach.
- **Tier B — current-user-effective (compute server-side, ship the result):** instead of the raw role → permission matrices, ship the requesting user's *effective* permissions — entity CRUD flags, field `{CanRead, CanUpdate}` (per #3367), query runnability. The client keeps working UX (`GetUserPermisions()` et al. return precomputed flags) without ever seeing other roles' grants.
- **Tier C — never ship:** RLS `FilterText`/`PlatformVariants` (the server applies RLS; the client only needs to know *that* a filter applies, if that), `RoleSQLName`/`DirectoryID`, saved-query SQL for queries the user cannot run (arguably for runnable ones too — the server executes them), authorization matrices beyond the user's own effective set, other users' dashboards, `SampleCode`/`TypeDefinitions`, settings values not whitelisted as client-safe.

Implementation sketch (phases, each independently shippable):

1. **Quick wins, metadata-only:** populate `DatasetItem.Columns` to drop obviously unneeded columns (e.g. `RowLevelSecurityFilter.FilterText`, `Role.DirectoryID`/`SQLName`, `Query.OriginalSQL`); remove the dead `Dashboards`/`Dashboard_Categories` items from `MJ_Metadata`.
2. **Per-user post-processing step in the dataset/metadata path:** a server-side redaction/projection hook in `DatasetResolver`/`GetDatasetByName` for the metadata dataset — the natural home for computing Tier B effective permissions and stripping Tier C rows (e.g. queries the user can't run). Must account for the server-side `LocalCacheManager` write-through (cache the superset, project per user at send time — same pattern as the FLS cache design in #3367).
3. **Client API compatibility layer:** `EntityInfo.GetUserPermisions()`, `QueryInfo.UserCanRun()`, etc. read the shipped effective flags instead of aggregating raw records, so downstream client code is unchanged.
4. **Convention + docs:** a guide establishing the default for any *new* metadata collection (like `EntityFieldPermission`): effective-permissions ship, raw matrices don't, and anything with SQL text or cross-user data requires explicit justification to enter a client-bound dataset.

## Compatibility notes

- Client-side permission evaluation currently depends on raw records being present; Tier B must land together with the compatibility layer (phase 3) to avoid breaking Explorer.
- Admin UIs (permission editors, query editors) legitimately need raw records — they should load them through the normal entity API, which is already governed by entity-level permissions, not through the broadcast metadata payload.
- The IndexedDB cache format/version should bump when the payload shape changes so stale full-fat payloads don't linger on disk.

## Related

- #3367 — field-level security plan; its §1.7 records the interim decision to ship `EntityFieldPermission` records under the current convention, with this issue as the long-term fix.

Contributor guide

Open the contributing guide

Research direction

Start with packages/MJServer/src/resolvers/DatasetResolver.ts and follow GetDatasetByName into packages/GenericDatabaseProvider/src/GenericDatabaseProvider.ts and packages/MJCore/src/generic/providerBase.ts. Review the metadata structures in securityInfo.ts, entityInfo.ts, and queryInfo.ts before choosing a phase. Done means client-bound metadata is projected by user need, raw sensitive matrices and unrelated rows are withheld, compatibility is preserved, and the IndexedDB payload version is updated.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, sql, typescript
Domain
backend-api-design, databases, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.