MemberJunction / MemberJunction/MJ
The MJ: Users guard leaves Email mutable on the strength of a DB constraint it never asserts
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
**Found by** the PR gauntlet on #4275 (`fix/4260-new-user-roles-default`), 2026-09-07. Latent coupling introduced with the guard in that PR; not a live bug on today's schema.
### Where
`packages/MJCoreEntitiesServer/src/custom/MJUserEntityServer.server.ts` — invariant 4 in the class docstring, and `validateNameImmutable`.
### What happens
The guard freezes `Name` for non-Owner callers but deliberately leaves `Email` mutable. Both are rungs of the same ladder: `resolvePrincipalFrom` (`packages/MJServer/src/auth/principals.ts`) matches the configured context user against `User.Name` first, then `User.Email`, and **neither rung filters by `Type`**. The stated justification for the asymmetry is that `MJ: Users.Email` carries the database's `UQ_User_Email` unique constraint, so redirecting the Email rung would require the configured candidate to match no active user at all.
That reasoning is sound, and the constraint does exist today. The problem is that it is a *schema* fact load-bearing for an *authorization* decision, asserted nowhere. The class is otherwise pure — it reads only its own field state and the caller — so nothing in it would notice if the constraint went away.
### Why it matters
If `UQ_User_Email` were ever dropped, renamed, or simply not emitted by a future provider, the Email rung would become exactly as redirectable as `Name` was before #4260 — and the only signal would be a paragraph of prose in a docstring that no longer matched reality. The failure mode is silent and the blast radius is "which user the server acts as".
### Repro
Not reproducible on the current schema; this is a guard against future drift. To see the coupling:
```sql
-- the constraint the decision depends on
SELECT name, type_desc FROM sys.indexes
WHERE object_id = OBJECT_ID('__mj.User') AND is_unique = 1;
```
### Evidence
The docstring's own argument: "(a) `MJ: Users.Email` carries the database's `UQ_User_Email` unique constraint, so redirecting the Email rung to your own row requires the configured candidate to match NO active user at all".
### Suggested fix
Add an integration check asserting that a unique constraint exists on `__mj.User(Email)`, in the same bundle style as IT88's checks, and reference it from the docstring paragraph that relies on it ("enforced by "). An integration check rather than a unit test, because the claim is about the live database, and cheap because it is one catalogue query. Do **not** freeze `Email` in the guard instead — that would add friction to an already-narrow, already-loud misconfiguration path without closing any route that is still open, which the docstring argues correctly.
### Definition of done
- [ ] A check that fails if the unique constraint on `__mj.User(Email)` is absent, then green against the current schema
- [ ] The docstring names the check that enforces its assumption
- [ ] Existing suite and gates green; no changed expectation in an existing test
### Verify by
Dropping the constraint on a scratch database makes the new check fail with a message naming the guard decision that depends on it.
Contributor guide
Research direction
Start with packages/MJCoreEntitiesServer/src/custom/MJUserEntityServer.server.ts, especially invariant 4 and validateNameImmutable, then read the IT88 integration-check bundle style. Review packages/MJServer/src/auth/principals.ts and verify the catalogue query against __mj.User. Done means the check fails when the unique Email constraint is absent, passes on the current schema, and the docstring references its check ID.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- sql, typescript
- Domain
- databases, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100