MemberJunction / MemberJunction/MJ

Entity permissions should be per-verb trinary (Allow/Deny/No Access), matching Entity Field Permissions

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

Description

Raised from review feedback on #3367, anchored on the field-level-security trinary table: *"This is great, but we don't have `deny` at the Entity level and we should really add this."*

**The ask is about per-verb granularity, not about Deny existing at all.** An earlier version of this issue read it the other way; that framing was wrong and is corrected here.

## The asymmetry

| | Verbs | How Deny is expressed |
|---|---|---|
| `EntityFieldPermission` | `ReadAccess` / `UpdateAccess` / `CreateAccess` — each **trinary** (`Allow` / `Deny` / `No Access`) | per verb, in one row |
| `EntityPermission` | `CanCreate` / `CanRead` / `CanUpdate` / `CanDelete` — each a **`bit`** | a single row-level `Type` (`Allow` / `Deny`) covering all four |

A boolean cannot distinguish **"no opinion"** from **"false"**. That distinction is the whole point of `No Access` in the field model, where it is the neutral identity element that lets another role's `Allow` win.

## What that costs today

On an `EntityPermission` row with `Type = 'Deny'`, `CanRead = 1` means "deny read". `CanRead = 0` means — nothing. It cannot be read as "explicitly permit read", and it cannot be read as "stay neutral on read", because those are the same value.

Consequences:

- **You cannot express mixed intent in one row.** "Deny read, stay neutral on update" requires a second row with a different `Type`, and the two must be reasoned about together.
- **The two permission systems aggregate differently**, which is a trap for anyone reading one after the other. Field permissions fold per verb: `effective = (any Allow) AND NOT (any Deny)`. Entity permissions fold per *row type* into Allow/Deny buckets and then subtract — the same net result today, but reached by a different mechanism, so the models drift apart under any future change.
- **The FLS snapshot derives entity-level intent through this ambiguity.** `fieldPermissionDelta.ts` reads entity permissions to decide the ceiling for the per-field snapshot, mapping four booleans plus a row `Type` onto three trinary verbs. That mapping is only sound while the boolean semantics stay exactly as they are.

## What already exists (do not rebuild it)

- `EntityPermission.Type` is a real column with an `Allow` / `Deny` value list.
- `EntityInfo.GetUserPermisions()` already aggregates Allow and Deny into separate buckets and subtracts per action (added in Phase 2b of the unified permissions work).

So Deny **is** enforced at the app tier. What is missing is the per-verb shape — and two tiers that never learned about `Type` at all:

## Gap A — CodeGen ignores `Type` entirely

A grep for `ep.Type` / `permission.Type` across `packages/CodeGenLib/` returns nothing. `generateViewPermissions` grants SELECT to every role holding *any* permission row:

```ts
for (const ep of entity.Permissions) {
if (ep.RoleSQLName && ep.RoleSQLName.length > 0) {
sOutput += ... `GRANT SELECT ON [schema].[BaseView] TO [role]`
}
}
```

A row that says **Deny** emits a **GRANT**. For a role with a `SQLName` the app tier denies and the database tier grants — and the database tier is the one an administrator cannot see. `generateCRUDPermissions` has the same shape: it consults `ep.CanCreate` / `CanUpdate` / `CanDelete` but never `Type`, so a Deny row with `CanUpdate = 1` emits `GRANT EXECUTE` on `spUpdate`.

Adjacent bug in the same emitter: `generateViewPermissions` never checks `ep.CanRead`, so a role holding a create-only permission still receives `GRANT SELECT` on the base view.

#3367 established the pattern to copy — `generateFieldSecurityDenies` emits column-level `DENY` restricted to custom roles, skipping any role a service login belongs to (a DENY there beats every sibling GRANT and would strip access from the API's own login). **PostgreSQL has no `DENY` primitive**, so as with FLS the app tier stays the only enforcement there.

## Gap B — no UI

The entity-permissions grid (`packages/Angular/Explorer/entity-permissions/`) renders the four booleans and has no notion of `Type`. New rows are created without it and take the `Allow` default, so a Deny row cannot be authored in the product at all today. Mirrors #4297 (field-level security has no UI to enable it).

## Scope

This is a schema change to one of the most load-bearing tables in the platform. **291 references across 84 non-generated, non-test files** read `.CanRead` / `.CanCreate` / `.CanUpdate` / `.CanDelete`. It needs a migration with a semantics-preserving backfill, a CodeGen regeneration, and a deliberate decision about whether the boolean accessors remain as derived compatibility shims.

Suggested sequence:

1. Migration: add trinary columns, backfill from `(Type, Can*)`, keep the booleans as generated/derived or retire them explicitly.
2. Rework `GetUserPermisions()` to fold per verb, matching `EntityFieldInfo.AggregateFieldRulesForUser`.
3. Teach the CodeGen permission emitters to honor the result — suppress the GRANT and emit an explicit `DENY` for custom roles, with the service-login backstop.
4. Expose the verbs in the entity-permissions grid, with Deny visually distinct.
5. Revisit `fieldPermissionDelta.ts`, which currently bridges the two models by hand.

## References

- `packages/MJCore/src/generic/entityInfo.ts` — `EntityPermissionInfo`, `GetUserPermisions()`, and `EntityFieldInfo.AggregateFieldRulesForUser` as the target shape
- `packages/CodeGenLib/src/Database/providers/sqlserver/SQLServerCodeGenProvider.ts` — `generateViewPermissions`, `generateCRUDPermissions`, `generateFieldSecurityDenies`
- `packages/MJCoreEntitiesServer/src/custom/fieldPermissionDelta.ts` — the current hand-written bridge between the two models
- `guides/FIELD_LEVEL_SECURITY_GUIDE.md` §1.2 (the trinary model) and §4 (why the tiers must agree)

Contributor guide

Open the contributing guide

Research direction

Start with EntityPermissionInfo and GetUserPermisions() in packages/MJCore/src/generic/entityInfo.ts, comparing them with EntityFieldInfo.AggregateFieldRulesForUser. Then trace the SQL Server emitters, the entity-permissions grid, and fieldPermissionDelta.ts; completion requires the schema, aggregation, generated permissions, UI, and bridge to agree without breaking existing semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
angular, postgresql, sql, typescript
Domain
authorization, backend, database, frontend, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.