Repository navigation
fix(security): take JWT sys_admin/support from the user record - #394
Merged
Merged
Conversation
Port of PlaceOS/rest-api 8779928. authorize! already loads the user row for bearer JWTs, but sys_admin and support were read only from the token claims, so revoking them had no effect until the token expired. The decoded JWT is now capped to the user's current permissions, so every is_admin?/is_support? check sees the effective value. A token is never raised above its claims. API keys keep their stored permissions, which may exceed the owner's. Requests from deleted (hard or soft) users now return 401 on both the JWT and API key paths, rather than proceeding or returning 404 from the RecordNotFound raised by User.find. Mock::Token.office/google claim Admin but named a non-admin user, so the mocks now mark that user sys_admin to keep spec behaviour unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The user record now decides permissions for bearer JWTs, rather than only capping the token's claims, so a promotion also applies to existing sessions without the user signing in again. API keys keep their own stored permissions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
type: bug
stakach
changed the title
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
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.
Port of the matching PlaceOS/rest-api change.
Summary
authorize!already loads the user for thelogged_out_atcheck. It now replaces the token's permissions with the user's currentsys_admin/support, ignoring the claims. Promotions and demotions apply to existing sessions without signing in again. Everyis_admin?/is_support?/user_admin?/user_support?check uses the result.deleted = true) users are refused on both the JWT and API key paths. Previously a hard-deleted user's JWT returned 404 (User.findraisesRecordNotFound) and a soft-deleted user got through.Spec fixture change
Mock::Token.office/googlealways claimAdmin, but by default named a user withsys_admin: false(the flags only choose the user's email). Now that the user record decides, those specs would have dropped to plain-user permissions, so the mocks now mark that usersys_admin. Spec behaviour is unchanged.Tests
New
spec/controllers/live_permissions_spec.cr:🤖 Generated with Claude Code