MemberJunction / MemberJunction/MJ

UserCache.GetSystemUser() returns a deactivated system user, and its type says it cannot be undefined

Open
#4,234 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

**Found by** the PR gauntlet on #4231 (`fix/4209-provisioning-context-user`), 2026-09-04. Not caused by that PR: verified pre-existing at `938cd9e9e8f9425924dc7d7bd3b55c90dfd90e46`.

### Where
`packages/GenericDatabaseProvider/src/UserCache.ts:57-59` (`GetSystemUser`) and `:141-148`
(`UserByName`).

### What happens
```ts
public GetSystemUser(): UserInfo {
return this.Users.find((u) => u.ID.toLowerCase() === UserCache.Instance.SYSTEM_USER_ID.toLowerCase()) as UserInfo;
}
```
- **No `IsActive` test**, so a deactivated system user is still returned as a principal to the
~25 call sites that use it.
- `u.ID.toLowerCase()` throws on a null `ID` instead of using `UUIDsEqual`, MJ's canonical
null-safe, case-insensitive UUID comparison (`guides/UUID_COMPARISON_GUIDE.md`).
- The return type lies: it is `UserInfo` via `as`, but `find` returns `undefined` on a cold or
Owner-less cache, so every caller that trusts the signature dereferences `undefined`.

`UserByName` calls `u.Name.trim()` with no guard, so a row reaching the cache through the public
`SetUsers` seam with a null `Name` makes it throw.

### Why it matters
Returning a disabled account as a principal contradicts the fail-closed policy elsewhere in the
same subsystem (`MagicLinkService.isInviterActive`) and the `IsActive` requirement PR #4231 put on
every rung of `auth/principals.ts`. `GetSystemUser()` is now the *only* system-user lookup in
MJServer that does not check it, which makes the guarantee depend on which helper a caller picked.

### Repro
```ts
UserCache.Instance.SetUsers([{ ID: 'x', Name: null } as unknown as UserInfo]);
UserCache.Instance.UserByName('anyone'); // TypeError: Cannot read properties of null (reading 'trim')
```
For the `IsActive` half: seed a scratch database with `UPDATE __mj.User SET IsActive = 0` on the
system user, boot, and observe `GetSystemUser()` still returning it.

### Evidence
`plans/provisioning-context-user-resolution.md` §4 records both as failure modes 7 and 11, and §6
lists them as deliberately out of PR #4231's scope. `principals.ts` avoids both by re-implementing
the lookup with `UUIDsEqual` and an `IsActive === true` filter rather than calling this method.

### Suggested fix
In `GetSystemUser()`: add `&& u.IsActive`, switch the comparison to `UUIDsEqual`, and change the
return type to `UserInfo | undefined` so callers must handle the miss (this will surface real
call-site bugs — that is the point). Null-guard `UserByName`. The `IsActive` change is a behaviour
change and needs its own changeset entry.

### Definition of done
- [ ] A failing test that reproduces it, then green
- [ ] `GetSystemUser()` returns `undefined` for a deactivated system user, and its signature says so
- [ ] `UserByName` returns `undefined` rather than throwing on a null `Name`
- [ ] Existing suite and gates green; every call site updated for the widened return type

### Verify by
`cd packages/GenericDatabaseProvider && pnpm test`, then `pnpm run build` across the workspace to
confirm no call site silently swallowed the widened type.

Contributor guide

Open the contributing guide

Research direction

Start in packages/GenericDatabaseProvider/src/UserCache.ts at GetSystemUser and UserByName, then inspect the roughly 25 call sites and the related logic in auth/principals.ts. Add regression coverage for deactivated system users and null names, run cd packages/GenericDatabaseProvider && pnpm test, and finish with the workspace build passing after all widened return types are handled.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
authentication, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.