fix: revoke access when a member is removed#14
Merged
Conversation
resolveActor built an actor for any valid session or API key and defaulted the role to "member" whenever the user had no membership row. So removing a member (deleting their member row) left their session and API keys fully working, and an account that signed up but never accepted its invite was treated as a member too. Require a live workspace membership in resolveActor: no member row means no actor, on both the session and the API-key path. Removal now revokes access uniformly (all writes, stats, keys, forget, collections actions), and the orphaned-invite hole is closed. resolveRole keeps its "member" default for the dashboard pages that only use it to compute isAdmin, where a non-member reading as a non-admin is harmless. Tests: resolveMembership (null for non-member, role for member), getActor returning null after the member row is deleted, and the stats endpoint going from 403 to 401 in production once membership is removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
resolveActor(the single chokepoint behindgetActor/requireOwner) built an actor for any valid session or API key, andresolveRoledefaulted to"member"when the user had no membership row. Two consequences, both confirmed against the code:memberrow (whatremoveMemberdoes) left the user's session and API keys working. They disappeared from the Team list, but could keep publishing, revoking, viewing stats, and minting keys.acceptInvitationhas a session and no membership, yet was treated as a"member".The fix
Require a live workspace membership in
resolveActor: no membership row means no actor, on both the session and the API-key path. This revokes access uniformly on removal (writes, stats, keys, forget, collections actions) and closes the orphaned-invite hole, for sessions and keys alike, through one place.resolveRoleis kept as a thin wrapper that still defaults to"member"for the dashboard pages that only use it to computeisAdmin(a non-member reading as a non-admin is harmless there). Authorization now flows through the newresolveMembership, which fails closed.Scope note (a separate, smaller item for a follow-up)
After this change, a removed user can still load their own dashboard pages read-only until their session expires. They cannot mutate anything or see anyone else's data (every write/stats/keys call is now 401). Fully blocking the dashboard shell for non-members is a separate change because it also changes the login-page redirect behavior and needs a live smoke of the invite-acceptance flow, so I've left it out of this PR by design.
Verification
npm run typecheckcleannpm run lintcleannpm test247/247 (added 5). New tests:resolveMembership(null for non-member, role for member),getActorreturning null after the member row is deleted, a valid key with no membership rejected, and the stats endpoint going from 403 (authed member, not owner) to 401 (no longer authed) in production after removal.npm run buildexit 0One fixture correction:
setupWithSessionssigned Bob up but never gave him a membership, relying on the old"member"default. It now inserts his member row (the accepted-invite state) so the IDOR tests still exercise a real member.