Centralize authorization enforcement - #2220
Open
pehbehbeh wants to merge 29 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
Collaborator
Should we drop support for Elixir 1.16? |
is_non_struct_map/1 only exists since Elixir 1.17, and mix.exs still declares ~> 1.16. Spell the guard as is_map/1 and not is_struct/1, which is guard-safe on every supported version and has the same semantics. The "refuses a socket" test in the authorization suite now hides the struct behind apply/3 like its resource counterpart, because the more precise guard lets the type checker see the mismatch at compile time.
The execution gate authorized the rows cached in selected_items when they were selected. A confirmation modal can stay open indefinitely, so a record another actor changed in the meantime was judged by its old values and then mutated by primary key under a permission it no longer had. The same snapshot fed the direct dispatch path. Backpex.Resource.reload/4 re-reads a list of items through the adapter's get/4, keeping order and yielding nil for a row that is gone or outside item_query/3. Backpex.ItemAction.authorize_fresh!/3 derives the resource fields from live_action, reloads, authorizes the reloaded records and returns them. Both execution paths go through it, and handle/3 receives the reloaded records, which selected_items mirrors for the dispatch. No transaction or lock is taken: handle/3 owns the write and decides what isolation it needs. The residual window is documented. The stub adapter gained a get/4 backed by assigns.stub_records so tests can change or remove a row between selecting it and acting on it.
Select a user, open the soft delete modal, then change the row behind the LiveView's back without a broadcast: a promotion to admin raises Backpex.ForbiddenError and leaves the row undeleted, a hard or soft delete raises Backpex.NoResultsError, and a post attached after the selection is nullified, which only the reloaded record can reach.
update_item/2 refreshed items on a "backpex:updated" broadcast but left
selected_items holding the old record, so the confirm dialog and every
preflight can?/3 kept describing stale values. Replace the row in both
lists, and drop it from the selection on "backpex:deleted" while keeping
select_all consistent.
A row that left the item query's scope between the broadcast and the
re-read made the old {:ok, item} match crash; it is now treated like a
deletion instead of putting nil into either list.
This is a UI nicety: the execution gate re-reads the selection either
way.
Explain in the item action and authorization guides, and in the v0.21 upgrade guide, that Backpex reloads every selected item right before the authoritative gate, authorizes the reloaded records and passes them to handle/3; that a vanished or out-of-scope row raises Backpex.NoResultsError; and that the re-read is not a lock, so an action needing strict atomicity has to lock inside its own handle/3.
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