Centralize authorization enforcement - #2220
Open
pehbehbeh wants to merge 24 commits into
Open
Conversation
Centralize all can?/3 checks behind a single module with preflight (can?/can_all?) and gate (authorize!/authorize_all!) functions, and route the existing raise sites in the index, show and form views through it.
Gate insert/update/delete_all/update_all through Backpex.Authorization before change/6 runs, so user changeset and before_changeset code never executes for an unauthorized request. Breaking changes: - delete_all(items, live_resource) -> delete_all(items, assigns, live_resource, opts) - update_all(items, updates, event_name, live_resource) -> update_all(items, updates, assigns, live_resource, opts) The is_map(assigns) guard makes the old update_all/4 call fail loudly instead of authorizing against the event name string.
Resolve client-supplied action keys against the registered actions instead of String.to_existing_atom/1, so an unknown key raises Backpex.NoResultsError rather than an ArgumentError. Reject stale or forged item ids at the item-action event and ignore them for update-selected-items, so nil never enters selected_items and never reaches the user's can?/3.
Item actions authorize every selected item before the confirm modal opens and again immediately before handle/3, and receive assigns.item_action_key. Resource action submits re-check the resource action key, closing the window between opening the modal and submitting it. Item action submits now take the action key from the server-side action_to_confirm assign instead of the phx-value-action-key DOM parameter, which the client could set to any registered key.
Disable a bulk item action button when the selection is empty or contains any unauthorized item, matching the strict gate. Drop the duplicate can?(:edit) check from index-editable fields, which Backpex.Resource.update/6 now enforces at the same effective point.
Pass assigns and the item action key to update_all, use authorize?: false for the cross-resource post nullification cascade, and reraise ForbiddenError from the rescue instead of turning it into a flash message.
Cover forged item-action events, unknown item ids and action keys, forged selection ids, mixed selections, the submit-time re-check and the reraise clauses in the built-in and demo delete actions.
Add the v0.21 upgrade guide and register it in the docs extras, add an Enforcement section to the LiveResource authorization guide, and add Authorization sections to the item and resource action guides. Also fix the item-actions update_all example, which matched no signature that ever existed and had a syntactically broken rescue.
Move the AllowAll / DenyAll / NoAdmins / KeyAware / OnlyCustomKey / Recording stubs and the stub adapter into test/support so the authorization, item action and resource tests alias one definition instead of redefining them two or three times.
Keyword.pop/3 returns an explicitly passed nil rather than the default, and nil is an atom, so authorization_action: nil reached can?(assigns, nil, item) where a permissive catch-all clause silently authorized the mutation. Validate the option like :authorize? already is.
Swap is_map/1 for is_non_struct_map/1 in the new Backpex.Resource and
Backpex.Authorization guards. A %Phoenix.LiveView.Socket{} passed where
assigns belong must fail loudly instead of authorizing against the wrong
context.
The pre-0.21 head had a default event_name argument, so it defined both update_all/3 and update_all/4. The guide only covered the arity-4 form; the arity-3 form now raises UndefinedFunctionError.
Migrate the direct live_resource.can?/3 calls in the HTML components, the show-page action buttons and the association fields. Behavior is unchanged; the point is that every check goes through one module.
It parses a client-supplied action key from an HTTP event, not LiveResource configuration, so it does not belong in the user-facing docs. Mark it @doc false and move the doctest into live_resource_test.
The modal branch is gated before the dialog opens, the immediate branch is gated inside handle_item_action/5 — the authoritative execution gate. Running both meant evaluating the user's can?/3 up to three times per item for one decision, which is what the removed field.ex check was criticized for. Because Backpex guarantees handle/3 only runs after the gate covered exactly those items under that key, the built-in delete action and the demo soft delete now pass authorize?: false and drop the reraise clauses: with the check skipped, no Forbidden/NoResults can originate from those Resource calls. Also extracts the resolve-key-then-route step shared by the index and show views into Backpex.ItemAction.resolve_item_action!/3.
The assign was set before handle/3 and never cleared, so it leaked into later dispatches and into every component rendered afterwards. Move setting and clearing it into Backpex.ItemAction.dispatch/5, used by both the immediate and the form dispatch path.
empty_item_action_selection/2 repeated the success branch of run_form_item_action byte for byte; both now call one close_item_action/2. Also drops the re-destructuring of assigns between handle_form_item_action and run_form_item_action — the values are passed along instead.
Strict enforcement disabled the toolbar button for a mixed selection with nothing saying why. A row that is authorized for none of the bulk actions is now unselectable (visible but disabled, with an accessible explanation), select-all skips those rows, and every disabled toolbar button carries a title saying what is wrong with the selection.
The suite had no resource action case at all. Adds an end-to-end authorized submit and a component-level check that the submit gate raises ForbiddenError — the route already refuses an action the user may not open, so a denial at submit can only come from a permission that changed while the modal was open.
Phoenix LiveView maps plug_status to an HTTP status only while a view mounts, not in handle_event. The comments and guides claimed a denied item action reaches the router as a 403; on a connected socket the LiveView crashes and the client reloads, with no error page and no message. Say that instead.
Flo0807
reviewed
Aug 27, 2026
|
|
||
| # Gate before any changeset work: permission may have been revoked, or the selection widened, | ||
| # while the modal was open. | ||
| Authorization.authorize_all!(live_resource, assigns, action_key, selected_items) |
Collaborator
There was a problem hiding this comment.
High: This execution gate authorizes selected_items cached when rows were selected, rather than freshly loaded records. Index.update_item/2 refreshes assigns.items but leaves selected_items stale. If another actor changes a selected user from role:user to role:admin while the modal is open, this line still authorizes the old role:user snapshot; dispatch then reaches update_all with authorize?: false, and the Ecto adapter mutates the current row by primary key. This defeats the promised submit-time permission re-check. Please reload the target records immediately before the authoritative gate and dispatch the fresh records; ideally keep reload/lock, authorization, and mutation in the same transaction.
Collaborator
Should we drop support for Elixir 1.16? |
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
2 participants
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.
Summary
Enforces authorization centrally instead of relying on every LiveView caller to remember
can?/3checks. All mutations inBackpex.Resourcenow gate before any user code runs, and item/resource actions are re-checked at execution time — closing an active bypass where the submitted action key was read from a forgeable DOM parameter. Unauthorized items in a selection now raiseBackpex.ForbiddenErrorinstead of being silently filtered.Changes
Backpex.Authorizationmodule:can?/can_all?for UI preflight,authorize!/authorize_all!as the execution gateResource.insert/update/delete_all/update_allauthorize by default, withauthorization_action:override andauthorize?: falseescape hatch (breaking signatures fordelete_all/update_all)NoResultsErrorv0.21.mdplus updated authorization and action guides