MemberJunction / MemberJunction/MJ
Magic-link redemption silently converts an open Application to an allowlist of one, revoking it from every existing user
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
`MagicLinkService.ensureAppAccess()` inserts an `ApplicationRole` row when a magic link is redeemed. For an application that has no `ApplicationRole` rows, that single insert **changes the application's access model from open to allowlist**, and the allowlist contains exactly one role: the invited one. Every existing user of that application loses it.
No error is raised, nothing is logged, and the trigger is an anonymous end user clicking a link — not an administrative action.
## Where
`packages/MJServer/src/auth/magicLink/MagicLinkService.ts` — `ensureAppAccess(roleId, applicationId, contextUser)`:
```ts
// idempotent per (role, application) — but not neutral for the application
const appRole = await md.GetEntityObject('MJ: Application Roles', contextUser);
appRole.ApplicationID = applicationId;
appRole.RoleID = roleId;
appRole.CanAccess = true;
```
The interpretation that makes it destructive is in `UserInfoEngine.UserHasApplicationAccess()`:
```ts
const appRoles = this._applicationRoles.filter(ar => UUIDsEqual(ar.ApplicationID, applicationId));
if (appRoles.length === 0) { return true; } // zero rows = OPEN
return user.UserRoles.some(ur => appRoles.some(ar => UUIDsEqual(ar.RoleID, ur.RoleID) && ar.CanAccess));
```
So `ApplicationRole` is a mode switch, not an additive grant table: the count going 0 → 1 is a policy change, and `ensureAppAccess` is a writer that cannot see that it is making one.
## Observed
On an Open App instance: the application had zero `ApplicationRole` rows, i.e. open to all staff. The first anonymous widget visitor redeemed an invite carrying the `Caliber Widget Visitor` role, `ensureAppAccess` inserted its row, and the application immediately became visible only to that role. Staff opening MJExplorer found the app gone.
## Suggested fix (any of these closes it)
1. **Do not create the row when the application is currently open.** If `SELECT COUNT(*) FROM ApplicationRole WHERE ApplicationID = @app` is 0, the invited role already has access; writing the row only removes it from everyone else. This is the smallest change and preserves the intent (the invitee can reach the app).
2. **Warn loudly when it is the first row.** If the insert would flip an application to allowlist mode, `LogError`/`LogStatus` saying so, naming the application and the roles that just lost access.
3. **Separate the two meanings.** A grant table whose emptiness means "everyone" cannot be safely written to by anything that is not making a policy decision — an explicit `Application.AccessMode` (`Open` | `Allowlist`) would let `ensureAppAccess` add a grant without changing the mode.
Happy to open a PR for (1) + (2) if that's the direction you'd take.
## Workaround for app authors
Declare the application's full access list up front so it is never empty — the count never transitions 0 → 1 and `ensureAppAccess` stays a no-op. Include the magic-link role in the list even though its holders never open Explorer.
Contributor guide
Research direction
Start in packages/MJServer/src/auth/magicLink/MagicLinkService.ts at ensureAppAccess(), then trace UserInfoEngine.UserHasApplicationAccess() to understand how zero ApplicationRole rows represent open access. Reproduce redemption for an open application and verify that existing users retain access while the invited user can reach it; confirm the chosen fix also handles the first-row transition safely.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authorization, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100