MemberJunction / MemberJunction/MJ

Magic-link redemption silently converts an open Application to an allowlist of one, revoking it from every existing user

Open
#3,537 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

## 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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.