diff --git a/lib/ash/actions/create.ex b/lib/ash/actions/create.ex index 54c9423a..35bd2ecc 100644 --- a/lib/ash/actions/create.ex +++ b/lib/ash/actions/create.ex @@ -30,13 +30,14 @@ defmodule Ash.Actions.Create do if params[:authorization] do auth_request = Ash.Authorization.Request.new( + state_key: :data, resource: resource, authorization_steps: action.authorization_steps, changeset: changeset, source: "create request" ) - Authorizer.authorize(user, %{}, [auth_request]) + Authorizer.authorize(user, [auth_request]) else :authorized end diff --git a/lib/ash/actions/destroy.ex b/lib/ash/actions/destroy.ex index ea1bfaf1..a0256a7a 100644 --- a/lib/ash/actions/destroy.ex +++ b/lib/ash/actions/destroy.ex @@ -32,7 +32,7 @@ defmodule Ash.Actions.Destroy do source: "destroy request" ) - Authorizer.authorize(user, %{}, [auth_request]) + Authorizer.authorize(user, [auth_request]) else :authorized end diff --git a/lib/ash/actions/read.ex b/lib/ash/actions/read.ex index 4bdf4d64..5fae95f1 100644 --- a/lib/ash/actions/read.ex +++ b/lib/ash/actions/read.ex @@ -14,15 +14,21 @@ defmodule Ash.Actions.Read do with %Ash.Filter{errors: [], authorizations: filter_auths} = filter <- Ash.Filter.parse(resource, filter), {:ok, side_load_auths} <- SideLoad.process(resource, side_loads, filter), - {:ok, auth_callback} <- do_authorize(params, side_load_auths ++ filter_auths), query <- Ash.DataLayer.resource_to_query(resource), {:ok, sort} <- Ash.Actions.Sort.process(resource, sort), {:ok, sorted_query} <- Ash.DataLayer.sort(query, sort, resource), {:ok, filtered_query} <- Ash.DataLayer.filter(sorted_query, filter, resource), {:ok, paginator} <- Ash.Actions.Paginator.paginate(api, resource, action, filtered_query, page_params), - {:ok, found} <- Ash.DataLayer.run_query(paginator.query, resource), - :ok <- auth_callback.(found), + {:ok, %{data: found}} <- + do_authorized( + paginator.query, + params, + filter, + resource, + action, + side_load_auths ++ filter_auths + ), paginator <- %{paginator | results: found} do # TODO: side loading is a read only, filter based operation, and as such should be covered # by the strict checks. Figure out if that is true for sure. @@ -33,19 +39,34 @@ defmodule Ash.Actions.Read do end end - defp do_authorize(params, auths) do + defp do_authorized(query, params, filter, resource, action, auths) do if params[:authorization] do - strict_access = + filter_authorization_request = + Ash.Authorization.Request.new( + resource: resource, + authorization_steps: action.authorization_steps, + filter: filter, + action_type: action.type, + fetcher: fn -> Ash.DataLayer.run_query(query, resource) end, + state_key: :data, + relationship: [], + source: "#{action.type} - `#{action.name}`" + ) + + strict_access? = case Keyword.fetch(params[:authorization], :strict_access?) do {:ok, value} -> value :error -> true end - auths = Enum.map(auths, fn auth -> %{auth | strict_access?: strict_access} end) - - Authorizer.authorize(params[:authorization][:user], %{}, auths) + Authorizer.authorize(params[:authorization][:user], [filter_authorization_request | auths], + strict_access?: strict_access? + ) else - {:ok, fn _ -> :ok end} + case Ash.DataLayer.run_query(query, resource) do + {:ok, found} -> {:ok, %{data: found}} + {:error, error} -> {:error, error} + end end end end diff --git a/lib/ash/actions/update.ex b/lib/ash/actions/update.ex index 4e782a7d..f85054eb 100644 --- a/lib/ash/actions/update.ex +++ b/lib/ash/actions/update.ex @@ -36,13 +36,14 @@ defmodule Ash.Actions.Update do if params[:authorization] do auth_request = Ash.Authorization.Request.new( + state_key: :data, resource: resource, authorization_steps: action.authorization_steps, changeset: changeset, source: "update action" ) - Authorizer.authorize(user, %{}, [auth_request]) + Authorizer.authorize(user, [auth_request]) else :authorized end diff --git a/lib/ash/authorization/authorizer.ex b/lib/ash/authorization/authorizer.ex index 0b583dbd..7a9d068c 100644 --- a/lib/ash/authorization/authorizer.ex +++ b/lib/ash/authorization/authorizer.ex @@ -16,39 +16,38 @@ defmodule Ash.Authorization.Authorizer do @type result :: :authorized | :forbidden alias Ash.Authorization.SatSolver + alias Ash.Authorization.Request - # TODO: remove _context - def authorize(user, _context, requests) do - requests_by_relationship = requests_by_relationship(requests) + def authorize(user, requests, opts \\ []) do + strict_access? = Keyword.get(opts, :strict_access?, true) - authorization_steps = authorization_steps_with_relationship_path(requests_by_relationship) - - if Enum.any?(authorization_steps, &Enum.empty?/1) do + if Enum.any?(requests, fn request -> Enum.empty?(request.authorization_steps) end) do {:error, Ash.Error.Forbidden.exception(no_steps_configured?: true)} else - facts = strict_check_facts(user, requests) + facts = strict_check_facts(user, requests, strict_access?) - solve(authorization_steps, facts, facts, %{user: user}) + solve(requests, user, facts, facts, %{user: user}, strict_access?) end end - defp solve(authorization_steps, facts, strict_check_facts, state) do - case sat_solver(authorization_steps, facts, [], state) do + defp solve(requests, user, facts, strict_check_facts, state, strict_access?) do + case sat_solver(requests, facts, [], state) do {:error, :unsatisfiable} -> {:error, Ash.Error.Forbidden.exception( - authorization_steps: authorization_steps, + requests: requests, facts: facts, strict_check_facts: strict_check_facts, + strict_access?: strict_access?, state: state )} {:ok, scenario} -> - authorization_steps + requests |> get_all_scenarios(scenario, facts, state) |> Enum.uniq() |> remove_irrelevant_clauses() - |> verify_scenarios(authorization_steps, facts, strict_check_facts, state) + |> verify_scenarios(user, requests, facts, strict_check_facts, state, strict_access?) end end @@ -90,7 +89,7 @@ defmodule Ash.Authorization.Authorizer do end defp get_all_scenarios( - authorization_steps, + requests, scenario, facts, state, @@ -111,14 +110,14 @@ defmodule Ash.Authorization.Authorizer do negations_assuming_scenario_false = [scenario | negations] case sat_solver( - authorization_steps, + requests, facts, negations_assuming_scenario_false, state ) do {:ok, scenario_after_negation} -> get_all_scenarios( - authorization_steps, + requests, scenario_after_negation, facts, state, @@ -132,76 +131,78 @@ defmodule Ash.Authorization.Authorizer do end end - defp sat_solver(authorization_steps, facts, negations, state) do + defp sat_solver(requests, facts, negations, state) do case state do %{data: [%resource{} | _] = data} -> # TODO: Needs primary key pkey = Ash.primary_key(resource) ids = Enum.map(data, &Map.take(&1, pkey)) - SatSolver.solve(authorization_steps, facts, negations, ids) + SatSolver.solve(requests, facts, negations, ids) _ -> - SatSolver.solve(authorization_steps, facts, negations, nil) + SatSolver.solve(requests, facts, negations, nil) end end defp verify_scenarios( scenarios, - authorization_steps, + user, + requests, facts, strict_check_facts, - state + state, + strict_access? ) do if any_scenarios_reality?(scenarios, facts) do - if Map.has_key?(state, :data) do - :ok - else - {:ok, fn _ -> :ok end} - end + fetch_must_fetch(requests, state) else - if Map.has_key?(state, :data) do - case fetch_facts(scenarios, facts, state) do - :all_scenarios_known -> - {:error, - Ash.Error.Forbidden.exception( - scenarios: scenarios, - authorization_steps: authorization_steps, - facts: facts, - strict_check_facts: strict_check_facts, - state: state - )} - - {:error, error} -> - {:error, error} - - {:ok, new_facts, state} -> - solve(authorization_steps, new_facts, strict_check_facts, state) - end - else - callback = fn data -> - data = List.wrap(data) - - if data == [] do - {:ok, fn _ -> :ok end} - else - verify_scenarios( - scenarios, - authorization_steps, - facts, - strict_check_facts, - Map.put(state, :data, List.wrap(data)) + case Ash.Authorization.Checker.run_checks( + scenarios, + user, + requests, + facts, + state, + strict_access? + ) do + :all_scenarios_known -> + error = + Ash.Error.Forbidden.exception( + scenarios: scenarios, + requests: requests, + facts: facts, + strict_check_facts: strict_check_facts, + state: state, + strict_access?: strict_access? ) - end - end - {:ok, callback} + {:error, error} + + {:error, error} -> + {:error, error} + + {:ok, new_facts, state} -> + solve(requests, user, new_facts, strict_check_facts, state, strict_access?) end end end - defp fetch_facts(scenarios, facts, state) do - Ash.Authorization.Checker.run_checks(scenarios, facts, state) + defp fetch_must_fetch(requests, state) do + Enum.reduce_while(requests, {:ok, state}, fn request, {:ok, state} -> + case Request.fetch_request_state(state, request) do + {:ok, _state} -> + {:cont, {:ok, state}} + + :error -> + case request.fetcher.() do + {:ok, value} -> + {:cont, {:ok, Request.put_request_state(state, request, value)}} + + {:error, error} -> + {:halt, {:error, error}} + end + end + end) end defp any_scenarios_reality?(scenarios, facts) do @@ -233,23 +234,9 @@ defmodule Ash.Authorization.Authorizer do end) end - defp strict_check_facts(user, requests) do + defp strict_check_facts(user, requests, strict_access?) do Enum.reduce(requests, %{true: true, false: false}, fn request, facts -> - Ash.Authorization.Checker.strict_check(user, request, facts) + Ash.Authorization.Checker.strict_check(user, request, facts, strict_access?) end) end - - defp authorization_steps_with_relationship_path(requests_by_relationship) do - Enum.flat_map(requests_by_relationship, fn {path, requests} -> - Enum.map(requests, fn request -> - Enum.map(request.authorization_steps, fn {step, fact} -> - {step, Ash.Authorization.Clause.new(path, request.resource, fact)} - end) - end) - end) - end - - defp requests_by_relationship(requests) do - Enum.group_by(requests, &Map.get(&1, :relationship)) - end end diff --git a/lib/ash/authorization/check/check.ex b/lib/ash/authorization/check/check.ex index 22f6fd27..2c99ba1f 100644 --- a/lib/ash/authorization/check/check.ex +++ b/lib/ash/authorization/check/check.ex @@ -10,8 +10,8 @@ defmodule Ash.Authorization.Check do @callback prepare(options) :: list(Ash.Authorization.prepare_instruction()) | {:error, Ash.error()} - @callback check(Ash.user(), list(Ash.record()), term, options) :: - {:ok, list(Ash.record())} | {:error, Ash.error()} + @callback check(Ash.user(), list(Ash.record()), map, options) :: + {:ok, list(Ash.record()) | boolean} | {:error, Ash.error()} @callback describe(options()) :: String.t() @optional_callbacks check: 4, prepare: 1 diff --git a/lib/ash/authorization/check/related_to_user_via.ex b/lib/ash/authorization/check/related_to_user_via.ex index b6400afa..59977a91 100644 --- a/lib/ash/authorization/check/related_to_user_via.ex +++ b/lib/ash/authorization/check/related_to_user_via.ex @@ -1,13 +1,17 @@ defmodule Ash.Authorization.Check.RelatedToUserVia do use Ash.Authorization.Check - def related_to_user_via(relationship) do - {__MODULE__, relationship: List.wrap(relationship)} + defmacro related_to_user_via(relationship) do + quote do + {unquote(__MODULE__), relationship: List.wrap(unquote(relationship)), source: __MODULE__} + end end @impl true def describe(opts) do - "#{Enum.join(opts[:relationship], ".")} is the user" + description = describe_relationship(opts[:source], opts[:relationship]) + + description <> "this_record is the user" end @impl true @@ -51,6 +55,28 @@ defmodule Ash.Authorization.Check.RelatedToUserVia do {:ok, matches} end + defp describe_relationship(resource, relationships) do + reversed_relationships = + relationships + |> Enum.reduce({resource, []}, fn relationship_name, {resource, acc} -> + relationship = Ash.relationship(resource, relationship_name) + {relationship.destination, [relationship | acc]} + end) + |> elem(1) + + do_describe_relationship(reversed_relationships) + end + + defp do_describe_relationship([]), do: "" + + defp do_describe_relationship([%{name: name, cardinality: :many} | rest]) do + "one of the #{name} of " <> do_describe_relationship(rest) + end + + defp do_describe_relationship([%{name: name, cardinality: :one} | rest]) do + "the #{name} of " <> do_describe_relationship(rest) + end + defp get_related(record, []), do: record defp get_related(record, [relationship | rest]) do diff --git a/lib/ash/authorization/checker.ex b/lib/ash/authorization/checker.ex index c29596cc..c181862e 100644 --- a/lib/ash/authorization/checker.ex +++ b/lib/ash/authorization/checker.ex @@ -1,159 +1,236 @@ defmodule Ash.Authorization.Checker do alias Ash.Authorization.Clause + alias Ash.Authorization.Request - def strict_check(user, request, facts) do + def strict_check(user, request, facts, strict_access?) do request.authorization_steps - |> Enum.reduce(facts, fn {_step, condition}, facts -> - case Map.fetch(facts, {request.relationship, condition}) do + |> Enum.reduce(facts, fn {_step, clause}, facts -> + case Map.fetch(facts, {request.relationship, clause}) do {:ok, _boolean_result} -> facts :error -> - case do_strict_check(condition, user, request) do + case do_strict_check(clause, user, request, strict_access?) do :unknown -> facts :unknowable -> - Clause.put_new_fact( - facts, - request.relationship, - request.resource, - condition, - :unknowable - ) + Map.put(facts, clause, :unknowable) boolean -> - Clause.put_new_fact( - facts, - request.relationship, - request.resource, - condition, - boolean - ) + Map.put(facts, clause, boolean) end end end) end - # TODO: Work on this. Gunna move it to checker - def run_checks(scenarios, facts, state) do + # TODO: Now, auth requests have a `fetcher`. We need to do the following: + # Going in *descending order of relationship path length*, take each request + # If it is `bypass_strict_access?`, fetch its data if necessary, and rerun auth.` + # Store that data in state, so we can see that we've already fetched it later. + # If not, move on and do the same for the rest of the relationships + # If there aree no facts left to check for bypass_strict_checks, and `strict_access?` (the overall flag) + # is true, then we return a forbidden error. + # If it is not, we do the same process as above where we go in descending order of path length + # except we run all fetchers, regardless of `bypass_strict_access?`, checking auth in between each one, + # and iteratively fetching facts, that kind of thing. + # + # + # In retrospect, this could perhaps be done by calling `run_checks` repeatedly, until it returns `{:require, blah}` + # and if we are in strict mode, then we bail. It will run fetchers for anything with `bypass_strict_access?` if + # it needs/wants to. THis is a better solution.` + + def run_checks(scenarios, user, requests, facts, state, strict_access?) do + all_checkable_clauses = all_checkable_clauses_from_scenarios(scenarios, facts) + + case clauses_checkable_without_fetching_data(all_checkable_clauses, requests, state) do + {[], []} -> + :all_scenarios_known + + {[], _clauses_requiring_fetch} -> + case fetch_requests(requests, state, strict_access?) do + {:ok, new_state} -> + run_checks(scenarios, user, requests, facts, new_state, strict_access?) + + :all_scenarios_known -> + :all_scenarios_known + + {:error, error} -> + {:error, error} + end + + {clauses, _} -> + # TODO: We could limit/smartly choose the checks that we prepare and run here as an optimization + case prepare_checks(clauses, state) do + {:ok, new_state} -> + do_run_checks(clauses, user, requests, facts, new_state, strict_access?) + + {:error, error} -> + {:error, error} + end + end + end + + # TODO: We could be smart here, and maybe fetch multiple requests + defp fetch_requests(requests, state, strict_access?) do + unfetched_requests = + Enum.reject(requests, fn request -> + Request.fetched?(state, request) + end) + + requests_without_strict_access = + if strict_access? do + Enum.filter(unfetched_requests, fn request -> + request.bypass_strict_access? + end) + else + unfetched_requests + end + + # We want to do these requests first regardless, + # as they would generally be more efficient checks + requests_without_strict_access + |> Enum.sort_by(fn request -> + {Enum.count(request.relationship), not request.bypass_strict_access?, request.relationship} + end) + |> Enum.at(0) + |> case do + nil -> + :all_scenarios_known + + request -> + case request.fetcher.() do + {:ok, value} -> + {:ok, Request.put_request_state(state, request, value)} + + {:error, error} -> + {:error, error} + end + end + end + + defp do_run_checks(clauses, user, requests, facts, state, strict_access?) do + Enum.reduce_while(clauses, {:ok, facts, state}, fn clause, {:ok, facts, state} -> + request = + requests + # This puts all requests with `bypass_strict_access?` in the front + # because if we can we want to find one of those first for the check below + |> Enum.sort_by(fn request -> + not request.bypass_strict_access? + end) + |> Enum.find(fn request -> + Request.contains_clause?(request, clause) + end) || raise "Internal assumption failed" + + {:ok, request_state} = Request.fetch_request_state(state, request) + request_state = List.wrap(request_state) + + check_module = clause.check_module + check_opts = clause.check_opts + + cond do + request_state == [] and strict_access? and !request.bypass_strict_access? -> + {:ok, Map.put(facts, clause, :unknowable), state} + + request_state == [] -> + {:ok, Map.put(facts, clause, :irrelevant), state} + + true -> + # TODO: Determine whether or not checks need the ability to generate additional state. + # If they do, we need to store that additional check state in `state` and pass it in here + case check_module.check(user, request_state, %{}, check_opts) do + {:error, error} -> + {:halt, {:error, error}} + + {:ok, check_result} -> + {:cont, + {:ok, add_check_results_to_facts(clause, check_result, request_state, facts), + state}} + end + end + end) + end + + defp clauses_checkable_without_fetching_data([], _, _), do: {[], []} + + defp clauses_checkable_without_fetching_data(clauses, requests, state) do + Enum.split_with(clauses, fn clause -> + Enum.any?(requests, fn request -> + Request.fetched?(state, request) && Request.contains_clause?(request, clause) + end) + end) + end + + defp all_checkable_clauses_from_scenarios(scenarios, facts) do scenarios |> Enum.flat_map(fn scenario -> scenario |> Map.drop([true, false]) - |> Enum.map(fn {clause, _value} -> - clause - end) + |> Enum.map(&elem(&1, 0)) end) - |> Enum.uniq() - |> choose_checks_to_run(facts, state) - |> case do - [] -> - :all_scenarios_known + |> Enum.reject(fn clause -> + Map.has_key?(facts, clause) + end) + end - clauses -> - clauses - |> prepare_checks(state) - |> do_run_checks(facts) + # Check returning `{:ok, true}` means all records are authorized + # while `{:ok, false}` means all records are not + defp add_check_results_to_facts(clause, boolean, _data, facts) when is_boolean(boolean) do + Map.put(facts, clause, boolean) + end + + defp add_check_results_to_facts(clause, [], _data, facts), do: Map.put(facts, clause, false) + + defp add_check_results_to_facts(clause, [%resource{} | _] = records, data, facts) do + pkey = Ash.primary_key(resource) + record_pkeys = Enum.map(records, &Map.take(&1, pkey)) + + case Enum.split_with(data, fn record -> + Map.take(record, pkey) in record_pkeys + end) do + {[], _} -> + Map.put(facts, clause, false) + + {_, []} -> + Map.put(facts, clause, false) + + {true_data, false_data} -> + facts = set_records_to(true_data, facts, clause, true, pkey) + + set_records_to(false_data, facts, clause, false, pkey) end end - defp do_run_checks({clauses, %{data: data, user: user} = state}, facts) do - Enum.reduce_while(clauses, {:ok, facts, state}, fn clause, {:ok, facts, state} -> - mod = clause.check_module - opts = clause.check_opts + defp set_records_to(data, facts, clause, value, pkey) do + Enum.reduce(data, facts, fn record, facts -> + pkey_clause = %{clause | pkey: Map.take(record, pkey)} - case mod.check(user, data, %{}, opts) do - {:error, error} -> - {:halt, {:error, error}} - - {:ok, []} -> - {:cont, {:ok, Map.put(facts, clause, false), state}} - - {:ok, true} -> - facts = Map.put(facts, clause, false) - {:cont, {:ok, facts, state}} - - {:ok, false} -> - facts = Map.put(facts, clause, false) - {:cont, {:ok, facts, state}} - - {:ok, [%resource{} | _] = records} -> - # TODO: Yet another thing that requires a primary key, - # figure it out - pkey = Ash.primary_key(resource) - record_pkeys = Enum.map(records, &Map.take(&1, pkey)) - - data - |> Enum.split_with(fn record -> - Map.take(record, pkey) in record_pkeys - end) - |> case do - {[], _} -> - facts = Map.put(facts, clause, false) - {:cont, {:ok, facts, state}} - - {_, []} -> - facts = Map.put(facts, clause, false) - {:cont, {:ok, facts, state}} - - {true_data, false_data} -> - facts = set_records_to(true_data, facts, clause, true, pkey) - - facts = set_records_to(false_data, facts, clause, false, pkey) - - {:cont, {:ok, facts, state}} - end - end - end) - end - - defp set_records_to(records, facts, clause, value, pkey) do - Enum.reduce(records, facts, fn record, facts -> - pkey_value = Map.take(record, pkey) - - Map.put(facts, %{clause | pkey: pkey_value}, value) + facts + |> Map.put(pkey_clause, value) end) end defp prepare_checks(clauses, state) do - Enum.reduce(clauses, {[], state}, fn - %{relationship: [], check_module: mod, check_opts: opts} = clause, {clauses, state} -> + Enum.reduce_while(clauses, {:ok, state}, fn + %{relationship: [], check_module: mod, check_opts: opts}, {:ok, state} -> case mod.prepare(opts) do - [] -> {[clause | clauses], state} - _ -> raise "No preparations supported yet!" + [] -> {:cont, {:ok, state}} + _ -> {:halt, {:error, "No preparations supported yet!"}} end _check, {_checks, _state} -> - raise "Can't prepare/handle checks with a relationship" + {:halt, {:error, "Can't prepare/handle checks with a relationship"}} end) end - # TODO: Get smart here - defp choose_checks_to_run([], _, _), do: [] - - defp choose_checks_to_run(checks, facts, _) do - checks - |> Enum.reject(fn check -> - Clause.find(facts, check) != :error - end) - |> case do - [] -> - [] - - checks -> - [List.first(checks)] - end - end - - defp do_strict_check({module, opts}, user, request) do + defp do_strict_check(%{check_module: module, check_opts: opts}, user, request, strict_access?) do case module.strict_check(user, request, opts) do {:ok, boolean} when is_boolean(boolean) -> boolean {:ok, :unknown} -> cond do - request.strict_access? -> + strict_access? -> # This means "we needed a fact that we have no way of getting" # Because the fact was needed in the `strict_check` step :unknowable diff --git a/lib/ash/authorization/request.ex b/lib/ash/authorization/request.ex index 6e239556..0b0fc69a 100644 --- a/lib/ash/authorization/request.ex +++ b/lib/ash/authorization/request.ex @@ -4,9 +4,12 @@ defmodule Ash.Authorization.Request do :authorization_steps, :filter, :action_type, + :bypass_strict_access?, :relationship, - :strict_access?, - :source + :fetcher, + :source, + :must_fetch?, + :state_key ] @type t :: %__MODULE__{ @@ -14,17 +17,51 @@ defmodule Ash.Authorization.Request do resource: Ash.resource(), authorization_steps: list(term), filter: Ash.Filter.t(), + # TODO: fetcher is a function + fetcher: term, relationship: list(atom), - strict_access?: boolean, - source: String.t() + bypass_strict_access?: boolean, + source: String.t(), + must_fetch?: boolean, + state_key: term } def new(opts) do opts = opts |> Keyword.put_new(:relationship, []) - |> Keyword.put_new(:strict_access?, true) + |> Keyword.put_new(:authorization_steps, []) + |> Keyword.put_new(:bypass_strict_access?, false) + |> Keyword.update!(:authorization_steps, fn steps -> + Enum.map(steps, fn {step, fact} -> + {step, Ash.Authorization.Clause.new(opts[:relationship] || [], opts[:resource], fact)} + end) + end) struct!(__MODULE__, opts) end + + def contains_clause?(request, clause) do + Enum.any?(request.authorization_steps, fn {_step, request_clause} -> + clause == request_clause + end) + end + + def fetched?(state, request) do + case fetch_request_state(state, request) do + {:ok, _} -> true + :error -> false + end + end + + def put_request_state(state, %{state_key: state_key} = request, value) do + state_key = state_key || request + Map.put(state, state_key, value) + end + + def fetch_request_state(state, %{state_key: state_key} = request) do + state_key = state_key || request + + Map.fetch(state, state_key) + end end diff --git a/lib/ash/authorization/sat_solver.ex b/lib/ash/authorization/sat_solver.ex index 279833cc..1d9ca314 100644 --- a/lib/ash/authorization/sat_solver.ex +++ b/lib/ash/authorization/sat_solver.ex @@ -1,13 +1,16 @@ defmodule Ash.Authorization.SatSolver do alias Ash.Authorization.Clause - def solve(sets_of_authorization_steps, facts, negations, ids) when is_nil(ids) do - sets_of_authorization_steps + def solve(requests, facts, negations, ids) when is_nil(ids) do + requests + |> Enum.map(&Map.get(&1, :authorization_steps)) |> build_requirements_expression(facts, nil) |> add_negations_and_solve(negations) end - def solve(sets_of_authorization_steps, facts, negations, ids) do + def solve(requests, facts, negations, ids) do + sets_of_authorization_steps = Enum.map(requests, &Map.get(&1, :authorization_steps)) + ids |> Enum.reduce(nil, fn id, expr -> requirements_expression = diff --git a/lib/ash/error/forbidden.ex b/lib/ash/error/forbidden.ex index 5aa4ca41..d5574541 100644 --- a/lib/ash/error/forbidden.ex +++ b/lib/ash/error/forbidden.ex @@ -3,19 +3,19 @@ defmodule Ash.Error.Forbidden do defexception [ :resource, :scenarios, - :authorization_steps, + :requests, :facts, :strict_check_facts, :state, + :strict_access?, no_steps_configured?: false ] # TODO: Use better logic to format this + # TODO: Remove reliance on "this_record" description alias Ash.Authorization.Clause - # TODO: Put `resource` in the pkey info so that we can display what kind of record it is - def message(%{no_steps_configured?: true}) do "One of the authorizations required had no authorization steps configured." end @@ -27,18 +27,21 @@ defmodule Ash.Error.Forbidden do explained_steps = case error.state do %{data: data} -> - explain_steps_with_data(error.authorization_steps, error.facts, data) + explain_steps_with_data(error.requests, error.facts, data) _ -> - explain_steps(error.authorization_steps, error.facts) + if error.strict_access? do + "\n\nAuthorization run with `strict_access?: true`. This is the only safe way to authorize requests for lists of filtered data.\n" <> + "Some checks may still fetch data from the database, like filters on related data when their primary key was given.\n" <> + explain_steps(error.requests, error.facts) + else + explain_steps(error.requests, error.facts) + end end explained_facts = explain_facts(error.facts, error.strict_check_facts || %{}) - main_message = - header <> - "Facts Gathered\n" <> - indent(explained_facts) <> "\n\nAuthorization Steps:\n" <> indent(explained_steps) + main_message = header <> "Facts Gathered\n" <> indent(explained_facts) <> explained_steps main_message <> "\n\nScenarios:\n" <> indent(explain_scenarios(error.scenarios)) end @@ -57,61 +60,70 @@ defmodule Ash.Error.Forbidden do """ end - defp explain_steps_with_data(sets_of_authorization_steps, facts, data) do - sets_of_authorization_steps - |> Enum.map_join("\n---\n", fn [{_, %{relationship: relationship, resource: resource}} | _] = - steps -> - title = - if relationship == [] do - inspect(resource) - else - # Enum.join(relationship, ".") <> " - #{inspect(resource)}" - raise "Ack, can't do relationships now!" - end + defp explain_steps_with_data(requests, facts, data) do + title = "\n\nAuthorization Steps:\n\n" - authorization_steps_legend = - steps - |> Enum.with_index() - |> Enum.map_join("\n", fn {{step, check}, index} -> - "#{index + 1}| " <> - to_string(step) <> ": " <> check.check_module.describe(check.check_opts) - end) + contents = + requests + |> Enum.map_join("\n---\n", fn request -> + relationship = request.relationship + resource = request.resource - pkey = Ash.primary_key(resource) + inner_title = + if relationship == [] do + request.source <> " -> " <> inspect(resource) <> ": " + else + # Enum.join(relationship, ".") <> " - #{inspect(resource)}" + raise "Ack, can't do relationships now!" + end - # TODO: data has to change with relationships - data_info = - data - |> Enum.map(fn item -> - formatted = - item - |> Map.take(pkey) - |> format_pkey() + authorization_steps_legend = + request.authorization_steps + |> Enum.with_index() + |> Enum.map_join("\n", fn {{step, check}, index} -> + "#{index + 1}| " <> + to_string(step) <> ": " <> check.check_module.describe(check.check_opts) + end) - {formatted, Map.take(item, pkey)} - end) - |> add_header_line(title) - |> pad() - |> add_step_info(steps, facts) + pkey = Ash.primary_key(resource) - authorization_steps_legend <> "\n\n" <> data_info <> "\n" - end) + # TODO: data has to change with relationships + data_info = + data + |> Enum.map(fn item -> + formatted = + item + |> Map.take(pkey) + |> format_pkey() + + {formatted, Map.take(item, pkey)} + end) + |> add_header_line(indent("Record")) + |> pad() + |> add_step_info(request.authorization_steps, facts) + + inner_title <> ":\n" <> indent(authorization_steps_legend <> "\n\n" <> data_info <> "\n") + end) + + title <> indent(contents) end defp add_step_info([header | rest], steps, facts) do key = Enum.join(1..Enum.count(steps), "|") header <> - "|" <> - key <> - "|\n" <> - do_add_step_info(rest, steps, facts) + indent( + " |" <> + key <> + "|\n" <> + do_add_step_info(rest, steps, facts) + ) end defp do_add_step_info(pkeys, steps, facts) do Enum.map_join(pkeys, "\n", fn {pkey_line, pkey} -> steps - |> Enum.reduce({true, pkey_line}, fn + |> Enum.reduce({true, pkey_line <> " "}, fn {_step, _clause}, {false, string} -> {false, string <> "|~"} @@ -183,29 +195,39 @@ defmodule Ash.Error.Forbidden do title = format_pkey(pkey) <> " facts" contents = - Enum.map_join(clauses_and_statuses, fn {clause, status} -> - gets_star? = - Clause.find(strict_check_facts, clause) in [ - {:ok, true}, - {:ok, false} - ] + clauses_and_statuses + |> Enum.group_by(fn {clause, _} -> + clause.relationship + end) + |> Enum.sort_by(fn {relationship, _} -> + {Enum.count(relationship), relationship} + end) + |> Enum.map_join("\n", fn {relationship, clauses_and_statuses} -> + contents = + Enum.map_join(clauses_and_statuses, "\n", fn {clause, status} -> + gets_star? = + Clause.find(strict_check_facts, clause) in [ + {:ok, true}, + {:ok, false} + ] - star = - if gets_star? do - " ⭑" - else - "" - end + star = + if gets_star? do + " ⭑" + else + "" + end - relationship = clause.relationship - mod = clause.check_module - opts = clause.check_opts + mod = clause.check_module + opts = clause.check_opts - if clause.relationship == [] do - status_to_mark(status) <> " " <> mod.describe(opts) <> star + status_to_mark(status) <> " " <> mod.describe(opts) <> star + end) + + if relationship == [] do + contents else - status_to_mark(status) <> - " " <> Enum.join(relationship, ".") <> " " <> mod.describe(opts) <> star + "Related " <> Enum.join(relationship) <> ":\n" <> indent(contents) end end) @@ -235,37 +257,52 @@ defmodule Ash.Error.Forbidden do |> Enum.join("\n") end - defp explain_steps(sets_of_authorization_steps, facts) do - Enum.map_join(sets_of_authorization_steps, "---", fn authorization_steps -> - authorization_steps - |> Enum.map(fn {step, clause} -> - status = - case Clause.find(facts, clause) do - {:ok, value} -> value - _ -> nil - end + defp explain_steps(requests, facts) do + title = "\n\nAuthorization Steps:\n" - status_mark = status_to_mark(status) + contents = + Enum.map_join(requests, "\n------\n", fn request -> + title = request.source - mark = status_mark <> " " <> step_to_mark(step, status) + contents = + request.authorization_steps + |> Enum.sort_by(fn {_step, clause} -> + {Enum.count(clause.relationship), clause.relationship} + end) + |> Enum.map(fn {step, clause} -> + status = + case Clause.find(facts, clause) do + {:ok, value} -> value + _ -> nil + end - mod = clause.check_module - opts = clause.check_opts - relationship = clause.relationship + mark = + case {status_to_mark(status), step_to_mark(step, status)} do + {"?", "↓"} -> "? " + {status_mark, step_mark} -> status_mark <> " " <> step_mark + end - if relationship == [] do - mark <> - " | " <> to_string(step) <> ": " <> mod.describe(opts) - else - mark <> - " | " <> - to_string(step) <> - ": #{Enum.join(relationship, ".")} " <> - mod.describe(opts) - end + mod = clause.check_module + opts = clause.check_opts + relationship = clause.relationship + + if relationship == [] do + mark <> + " | " <> to_string(step) <> ": " <> mod.describe(opts) + else + mark <> + " | " <> + to_string(step) <> + ": #{Enum.join(relationship, ".")}: " <> + mod.describe(opts) + end + end) + |> Enum.join("\n") + + title <> ":\n" <> indent(contents) end) - |> Enum.join("\n") - end) + + title <> indent(contents) end defp step_to_mark(:authorize_if, true), do: "✓" diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index ae999b54..fab7b771 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -32,27 +32,54 @@ defmodule Ash.Filter do or: Ash.Filter.Or } - @spec parse(Ash.resource(), Keyword.t(), relationship_path :: list(atom)) :: t() + @spec parse( + Ash.resource(), + Keyword.t(), + relationship_path :: list(atom) + ) :: t() def parse(resource, filter, path \\ []) do - authorization_steps = Ash.primary_action(resource, :read).authorization_steps - parsed_filter = filter |> do_parse(%Ash.Filter{resource: resource, path: path}) |> lift_ors() |> add_not_filter_info() - add_authorization( - parsed_filter, - Ash.Authorization.Request.new( - resource: resource, - authorization_steps: authorization_steps, - filter: parsed_filter, - action_type: :read, - relationship: path, - source: "#{Enum.join(path, ".")} filter" + source = + case path do + [] -> "filter" + path -> "related #{Enum.join(path, ".")} filter" + end + + if path == [] do + parsed_filter + else + authorization_request = + Ash.Authorization.Request.new( + resource: resource, + authorization_steps: Ash.primary_action(resource, :read).authorization_steps, + filter: parsed_filter, + fetcher: fn -> + query = Ash.DataLayer.resource_to_query(resource) + + case Ash.DataLayer.filter(query, parsed_filter, resource) do + {:ok, filtered_query} -> + Ash.DataLayer.run_query(filtered_query, resource) + + {:error, error} -> + {:error, error} + end + end, + action_type: :read, + bypass_strict_access?: bypass_strict_access?(parsed_filter), + relationship: path, + source: source + ) + + add_authorization( + parsed_filter, + authorization_request ) - ) + end end @doc """ @@ -82,6 +109,45 @@ defmodule Ash.Filter do attributes_contained? or relationships_contained? end + defp bypass_strict_access?(%{ors: ors, not: not_filter} = filter) when is_nil(not_filter) do + pkey_fields = Ash.primary_key(filter.resource) + + Enum.all?(pkey_fields, &is_filtering_on_known_value_for_attribute(filter, &1)) && + Enum.all?(ors || [], fn filter -> + Enum.all?(pkey_fields, &is_filtering_on_known_value_for_attribute(filter, &1)) + end) + end + + defp bypass_strict_access?(_), do: false + + defp is_filtering_on_known_value_for_attribute(filter, field) do + case Map.fetch(filter.attributes, field) do + {:ok, attribute_filter} -> + known_value_filter?(attribute_filter) + + :error -> + false + end + end + + defp known_value_filter?(%Ash.Filter.And{left: left, right: right}) do + known_value_filter?(left) or known_value_filter?(right) + end + + defp known_value_filter?(%Ash.Filter.Or{left: left, right: right}) do + known_value_filter?(left) and known_value_filter?(right) + end + + defp known_value_filter?(%Ash.Filter.In{values: values}) when values != [] do + true + end + + defp known_value_filter?(%Ash.Filter.Eq{value: value}) when not is_nil(value) do + true + end + + defp known_value_filter?(_), do: false + defp contains_relationship?(filter, relationship, candidate_relationship_filter) do case filter.relationships do %{^relationship => relationship_filter} -> diff --git a/test/authorization/read_authorization_test.exs b/test/authorization/read_authorization_test.exs index 59fd2c1e..9923f2d5 100644 --- a/test/authorization/read_authorization_test.exs +++ b/test/authorization/read_authorization_test.exs @@ -204,6 +204,22 @@ defmodule Ash.Test.Authorization.ReadAuthorizationTest do Api.create!(Author, attributes: %{name: "foo", fired: false, self_manager: false}) user = Api.create!(User, attributes: %{manager: false, id: author.id}) - Api.read!(Author, authorization: [user: user, strict_access?: false]) + assert_raise Ash.Error.Forbidden, ~r/forbidden/, fn -> + Api.read!(Author, authorization: [user: user, strict_access?: false]) + end + end + + test "it handles authorizing destination records properly" do + author = Api.create!(Author, attributes: %{name: "foo", fired: false, self_manager: true}) + + other_author = + Api.create!(Author, attributes: %{name: "foo", fired: false, self_manager: false}) + + user = Api.create!(User, attributes: %{manager: false, id: author.id}) + + Api.read!(Post, + filter: [authors: [id: other_author.id]], + authorization: [user: user, strict_access?: false] + ) end end