MemberJunction / MemberJunction/MJ

FLS: a saved User View's WhereClause/OrderByClause bypasses the predicate gate, leaking denied values by inference

Open
#4,347 0 comments 0 reactions 1 assignee Claimed by @jordanfanapour View on GitHub
bug priority: high
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## What

Field-Level Security rejects a caller-authored predicate that names a denied field — `ExtraFilter`, `OrderBy`, `Aggregates[].expression` — because output stripping alone is theater: the filter reconstructs values without the column ever appearing in a result.

A **saved User View's own `WhereClause` and `OrderByClause` never reach that gate.**

## Mechanism

`AssertPredicatesRespectFieldSecurity` (`packages/MJCore/src/generic/providerBase.ts`) scans only:

```ts
const clauses = [ params.ExtraFilter, params.OrderBy, ...(params.Aggregates ?? []).map(a => a?.expression) ];
```

All three non-test call sites — `providerBase.ts:2863`, `providerBase.ts:3034`, `GenericDatabaseProvider.ts:2588` — feed it the same `RunViewParams`. A stored view's clauses live on `viewEntity`, not on params, and are applied later inside `GenericDatabaseProvider.InternalRunView`: the `WhereClause` around line 1862, the `OrderByClause` around 1960. Their only screen is `ValidateUserProvidedSQLClause`, a forbidden-keyword check with no notion of denied columns.

The external-data-source path is worse: `mergeExternalViewParams` folds the stored `WhereClause` into `merged.ExtraFilter` **after** the gate has already run.

## Not admin-gated

Only `CustomWhereClause = 1` requires elevation (`MJUserViewEntityServer.server.ts`). An ordinary FilterState view is saved by any user, and `OrderByClause` is written by clicking a grid column header. `MJUserViewEntityServer` has no field-security gate at all — verified, zero matches for `EnableFieldLevelSecurity` / `GetDeniedReadFields` / `FindReferencedIdentifiers` in that file.

## Failure

Role `Staff` is denied Read on `Employees.Salary`. A Staff user saves a view filtered `Salary > 200000`, then calls `RunViewByID` with no `ExtraFilter`. The gate sees two empty strings and passes; the SQL emits `WHERE ([Salary] > 200000)`. `Salary` is stripped from the projection, but **which rows come back is the answer** — binary-searching the threshold recovers exact values. A saved `Salary DESC` sort leaks the full ranking in one call.

## Fix

Run the gate against the **effective** clauses once the stored view is resolved — inside `InternalRunView` and `mergeExternalViewParams` — rather than only against the caller-supplied `RunViewParams`.

## Status

Documented as an accepted residual in `guides/FIELD_LEVEL_SECURITY_GUIDE.md` §5 (commit `b55c23913f`) so the guide no longer promises what it cannot deliver, and the §2 table row is qualified. This issue tracks closing the gap.

Found reviewing #3367. Not blocking that PR: the flag is set nowhere and #4297 means no UI can set it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.