diff --git a/README.md b/README.md index f19daca9..41e462fd 100644 --- a/README.md +++ b/README.md @@ -138,4 +138,14 @@ end variable from data somehow. Authorization fetchers will need to take state as an argument or something like that, and maybe need to specify dependencies?. * Validate that checks have the correct action type when compiling an action - +* Make sure updating foreign key attributes behaves the same as setting a + relationship, or just disallow having editable attributes for relationship fkeys +* Validate `dependencies` and `must_fetch` (all `must_fetch` with dependencies + must have those dependencies as `must_fetch` also) +* Support branching/more complicated control flow in authorization steps +* The Authorization flow for creates/updates may be insufficient. Instead of + adding requests if relationships/attributes are changing, we may instead want + to embed that knowledge inside the sat solver itself. Basically a + `relationship_foo_is_changing?` fact, *and*ed with the resulting conditions. + I'm not even sure if thats possible though. +* We need to validate incoming attributes/relationships better. diff --git a/lib/ash/actions/changeset_helpers.ex b/lib/ash/actions/changeset_helpers.ex index ce94aeb3..09924bc6 100644 --- a/lib/ash/actions/changeset_helpers.ex +++ b/lib/ash/actions/changeset_helpers.ex @@ -111,7 +111,7 @@ defmodule Ash.Actions.ChangesetHelpers do authorization ) do before_change(changeset, fn changeset -> - case api.get(destination, filter, authorization: authorization) do + case api.get(destination, filter, authorization: false) do {:ok, record} when not is_nil(record) -> changeset |> Ecto.Changeset.put_change(source_field, Map.get(record, destination_field)) @@ -147,7 +147,7 @@ defmodule Ash.Actions.ChangesetHelpers do filter: [{destination_field, value}], limit: 1, paginate?: false, - authorization: authorization + authorization: false ) do {:ok, %{results: []}} -> changeset @@ -176,11 +176,11 @@ defmodule Ash.Actions.ChangesetHelpers do value = Map.get(result, source_field) with {:ok, record} <- - api.get(destination, filter, authorization: authorization), + api.get(destination, filter, authorization: false), {:ok, updated_record} <- api.update(record, attributes: %{destination_field => value}, - authorization: authorization + authorization: false ) do {:ok, Map.put(result, relationship.name, updated_record)} end @@ -251,7 +251,7 @@ defmodule Ash.Actions.ChangesetHelpers do params = [ filter: [{relationship.reverse_relationship, currently_related_filter}], paginate?: false, - authorization: authorization + authorization: false ] with {:ok, %{results: related}} <- @@ -274,7 +274,7 @@ defmodule Ash.Actions.ChangesetHelpers do case api.read(rel.destination, filter: [{rel.reverse_relationship, pkey_filter}], - authorization: authorization + authorization: false ) do {:ok, %{results: currently_related}} -> Enum.reduce_while(filters, {:ok, []}, fn filter, {:ok, records} -> @@ -319,7 +319,7 @@ defmodule Ash.Actions.ChangesetHelpers do {:not, [or: identifiers]} ] - case api.read(rel.destination, filter: filter, authorization: authorization) do + case api.read(rel.destination, filter: filter, authorization: false) do {:error, error} -> {:error, error} @@ -332,7 +332,7 @@ defmodule Ash.Actions.ChangesetHelpers do {rel.source_field_on_join_table, source_field_value}, {rel.destination_field_on_join_table, destination_field_value} ]), - {:ok, _} <- api.destroy(record, authorization: authorization) do + {:ok, _} <- api.destroy(record, authorization: false) do {:cont, :ok} else {:ok, nil} -> {:cont, :ok} @@ -343,7 +343,7 @@ defmodule Ash.Actions.ChangesetHelpers do end defp do_fetch_and_ensure_related(api, result, id_filter, rel, authorization) do - case api.get(rel.destination, id_filter, authorization: authorization) do + case api.get(rel.destination, id_filter, authorization: false) do {:error, error} -> {:error, error} @@ -362,11 +362,11 @@ defmodule Ash.Actions.ChangesetHelpers do # unless we feel that verifying at *runtime* that the record exists before-hand is a good # idea - case api.get(rel.through, filter, authorization: authorization) do + case api.get(rel.through, filter, authorization: false) do {:ok, nil} -> create_result = api.create(rel.through, - authorization: authorization, + authorization: false, attributes: %{ rel.destination_field_on_join_table => destination_field_value, rel.source_field_on_join_table => source_field_value @@ -392,7 +392,7 @@ defmodule Ash.Actions.ChangesetHelpers do to_be_related, {:ok, now_related} -> case api.update(to_be_related, attributes: %{destination_field => destination_field_value}, - authorization: authorization + authorization: false ) do {:ok, newly_related} -> {:ok, [newly_related | now_related]} {:error, error} -> {:error, error} @@ -408,7 +408,7 @@ defmodule Ash.Actions.ChangesetHelpers do record, :ok -> case api.update(record, attributes: %{destination_key => nil}, - authorization: authorization + authorization: false ) do {:ok, _} -> :ok {:error, error} -> {:error, error} @@ -438,7 +438,7 @@ defmodule Ash.Actions.ChangesetHelpers do # TODO: Only fetch the ones that we don't already have Enum.reduce(filters, {:ok, []}, fn filter, {:ok, to_relate} -> - case api.get(destination, filter, authorization: authorization) do + case api.get(destination, filter, authorization: false) do {:ok, to_relate_item} -> {:ok, [to_relate_item | to_relate]} {:error, errors} -> {:error, errors} end diff --git a/lib/ash/actions/create.ex b/lib/ash/actions/create.ex index 341fe55f..ca549e31 100644 --- a/lib/ash/actions/create.ex +++ b/lib/ash/actions/create.ex @@ -7,8 +7,9 @@ defmodule Ash.Actions.Create do def run(api, resource, action, params) do if Keyword.get(params, :side_load, []) in [[], nil] do with %{valid?: true} = changeset <- prepare_create_params(api, resource, params), - {:ok, %{data: created}} <- do_authorized(changeset, params, action, resource, api) do - ChangesetHelpers.run_after_changes(changeset, created) + {:ok, %{data: created} = state} <- + do_authorized(changeset, params, action, resource, api) do + {:ok, Ash.Actions.Relationships.add_relationships_to_result(resource, created, state)} else %Ecto.Changeset{} = changeset -> {:error, changeset} @@ -22,60 +23,75 @@ defmodule Ash.Actions.Create do end defp do_authorized(changeset, params, action, resource, api) do - if params[:authorization] do - create_authorization_request = + create_authorization_request = + Ash.Authorization.Request.new( + api: api, + authorization_steps: action.authorization_steps, + resource: resource, + changeset: changeset, + action_type: action.type, + fetcher: fn -> + do_create(resource, changeset) + end, + state_key: :data, + must_fetch?: true, + relationship: [], + source: "#{action.type} - `#{action.name}`" + ) + + attribute_requests = + resource + |> Ash.attributes() + |> Enum.reject(fn attribute -> + attribute.primary_key? + end) + |> Enum.reject(fn attribute -> + attribute.name in Map.get(changeset, :__ash_skip_authorization_fields__, []) + end) + |> Enum.filter(fn attribute -> + attribute.authorization_steps != false && Map.has_key?(changeset.changes, attribute.name) + end) + |> Enum.map(fn attribute -> Ash.Authorization.Request.new( api: api, - authorization_steps: action.authorization_steps, + authorization_steps: attribute.authorization_steps, resource: resource, changeset: changeset, action_type: action.type, - fetcher: fn -> do_create(resource, changeset) end, + fetcher: fn -> :ok end, state_key: :data, relationship: [], - source: "#{action.type} - `#{action.name}`" + source: "change on `#{attribute.name}`" ) + end) - attribute_requests = - resource - |> Ash.attributes() - |> Enum.reject(fn attribute -> - attribute.primary_key? - end) - |> Enum.filter(fn attribute -> - attribute.authorization_steps && Map.has_key?(changeset.changes, attribute.name) - end) - |> Enum.map(fn attribute -> - Ash.Authorization.Request.new( - api: api, - authorization_steps: attribute.authorization_steps, - resource: resource, - changeset: changeset, - action_type: action.type, - fetcher: fn -> :ok end, - state_key: :data, - relationship: [], - source: "change on `#{attribute.name}`" + case Ash.Actions.Relationships.relationship_change_authorizations(api, resource, changeset) do + {:ok, relationship_auths} -> + if params[:authorization] do + strict_access? = + case Keyword.fetch(params[:authorization], :strict_access?) do + {:ok, value} -> value + :error -> true + end + + Authorizer.authorize( + params[:authorization][:user], + [create_authorization_request | attribute_requests] ++ relationship_auths, + strict_access?: strict_access?, + log_final_report?: params[:authorization][:log_final_report?] || false ) - end) + else + authorization = params[:authorization] || [] - strict_access? = - case Keyword.fetch(params[:authorization], :strict_access?) do - {:ok, value} -> value - :error -> true + Authorizer.authorize( + authorization[:user], + [create_authorization_request | attribute_requests], + fetch_only?: true + ) end - Authorizer.authorize( - params[:authorization][:user], - [create_authorization_request | attribute_requests], - strict_access?: strict_access?, - log_final_report?: params[:authorization][:log_final_report?] || false - ) - else - case do_create(resource, changeset) do - {:ok, result} -> {:ok, %{data: result}} - {:error, error} -> {:error, error} - end + {:error, error} -> + {:error, error} end end @@ -98,16 +114,84 @@ defmodule Ash.Actions.Create do defp prepare_create_params(api, resource, params) do attributes = Keyword.get(params, :attributes, %{}) relationships = Keyword.get(params, :relationships, %{}) + + old_relationships = + Enum.reduce(relationships, %{}, fn {key, value}, acc -> + if Ash.relationship(resource, key).type == :has_many do + acc + else + Map.put(acc, key, value) + end + end) + authorization = Keyword.get(params, :authorization, false) case prepare_create_attributes(resource, attributes) do %{valid?: true} = changeset -> + # TODO: __ash_api__ should be unnecessary in the new way. + # TODO: If you are saying to `add` somethign to a to_one relationship + # but not removing the old thing, that should be a validation error + # assuming you were authorized to read the original data. + # If you are changing a foreign key, that needs to map to a relationship update changeset = Map.put(changeset, :__ash_api__, api) + changeset = + relationships + |> Enum.reduce(changeset, fn {key, value}, changeset -> + case Ash.relationship(resource, key) do + # TODO remove the `type` checks here + # TODO: ew + %{cardinality: :many, type: :has_many, destination: destination, name: name} -> + case Ash.Actions.PrimaryKeyHelpers.values_to_primary_key_filters( + destination, + value + ) do + {:ok, values} -> + Map.update!(changeset, :__ash_relationships__, fn ash_relationships -> + Map.put(ash_relationships, key, %{add: values}) + end) + + {:error, _error} -> + Ecto.Changeset.add_error(changeset, name, "Invalid Identifiers") + end + + %{ + cardinality: :one, + type: :belongs_to, + destination: destination, + name: name, + source_field: source_field, + destination_field: destination_field + } -> + case Ash.Actions.PrimaryKeyHelpers.value_to_primary_key_filter(destination, value) do + {:ok, value} -> + changeset + |> Map.update!(:__ash_relationships__, fn ash_relationships -> + Map.put(ash_relationships, key, %{add: value}) + end) + # Does this assumption hold? + |> Ecto.Changeset.put_change( + source_field, + Keyword.fetch!(value, destination_field) + ) + |> Map.put_new(:__ash_skip_authorization_fields__, []) + |> Map.update!(:__ash_skip_authorization_fields__, fn fields -> + [source_field | fields] + end) + + {:error, _error} -> + Ecto.Changeset.add_error(changeset, name, "Invalid Identifier(s)") + end + + _ -> + Ecto.Changeset.add_error(changeset, key, "No such relationship") + end + end) + ChangesetHelpers.prepare_relationship_changes( changeset, resource, - relationships, + old_relationships, authorization ) @@ -122,10 +206,11 @@ defmodule Ash.Actions.Create do |> Ash.attributes() |> Enum.map(& &1.name) + # TODO: Reject any changes for attributes that are the source field of any `belongs_to` relationships! attributes_with_defaults = resource |> Ash.attributes() - |> Stream.filter(&(not is_nil(&1.default))) + |> Enum.filter(&(not is_nil(&1.default))) |> Enum.reduce(attributes, fn attr, attributes -> if Map.has_key?(attributes, attr.name) do attributes @@ -139,6 +224,7 @@ defmodule Ash.Actions.Create do |> struct() |> Ecto.Changeset.cast(attributes_with_defaults, allowed_keys) |> Map.put(:action, :create) + |> Map.put(:__ash_relationships__, %{}) resource |> Ash.attributes() diff --git a/lib/ash/actions/primary_key_helpers.ex b/lib/ash/actions/primary_key_helpers.ex index 62c4f407..ab71e0a6 100644 --- a/lib/ash/actions/primary_key_helpers.ex +++ b/lib/ash/actions/primary_key_helpers.ex @@ -38,8 +38,11 @@ defmodule Ash.Actions.PrimaryKeyHelpers do attr = Ash.attribute(resource, key) case Ash.Type.cast_input(attr.type, val) do - {:ok, casted} -> {:ok, Keyword.put(filter, attr.name, casted)} - :error -> {:error, {key, "is invalid"}} + {:ok, casted} -> + {:ok, Keyword.put(filter, attr.name, casted)} + + :error -> + {:error, "#{key} is invalid"} end _, {:error, error} -> diff --git a/lib/ash/actions/read.ex b/lib/ash/actions/read.ex index 2e17ec09..a1a09a59 100644 --- a/lib/ash/actions/read.ex +++ b/lib/ash/actions/read.ex @@ -41,20 +41,21 @@ defmodule Ash.Actions.Read do end defp do_authorized(query, params, filter, resource, api, action, auths) do - if params[:authorization] do - filter_authorization_request = - Ash.Authorization.Request.new( - api: api, - 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}`" - ) + filter_authorization_request = + Ash.Authorization.Request.new( + api: api, + resource: resource, + authorization_steps: action.authorization_steps, + filter: filter, + action_type: action.type, + fetcher: fn -> Ash.DataLayer.run_query(query, resource) end, + must_fetch?: true, + state_key: :data, + relationship: [], + source: "#{action.type} - `#{action.name}`" + ) + if params[:authorization] do strict_access? = case Keyword.fetch(params[:authorization], :strict_access?) do {:ok, value} -> value @@ -66,10 +67,11 @@ defmodule Ash.Actions.Read do log_final_report?: params[:authorization][:log_final_report?] || false ) else - case Ash.DataLayer.run_query(query, resource) do - {:ok, found} -> {:ok, %{data: found}} - {:error, error} -> {:error, error} - end + authorization = params[:authorization] || [] + + Authorizer.authorize(authorization[:user], [filter_authorization_request | auths], + fetch_only?: true + ) end end end diff --git a/lib/ash/actions/relationships.ex b/lib/ash/actions/relationships.ex new file mode 100644 index 00000000..d91746f0 --- /dev/null +++ b/lib/ash/actions/relationships.ex @@ -0,0 +1,182 @@ +defmodule Ash.Actions.Relationships do + alias Ash.Actions.PrimaryKeyHelpers + + def relationship_change_authorizations(api, resource, changeset) do + resource + |> Ash.relationships() + |> Enum.filter(fn relationship -> + Map.has_key?(changeset.__ash_relationships__, relationship.name) + end) + |> Enum.reduce_while({:ok, []}, fn relationship, {:ok, authorizations} -> + case add_related_authorizations(resource, api, relationship, changeset) do + {:ok, new_authorizations} -> {:cont, {:ok, authorizations ++ new_authorizations}} + {:error, error} -> {:halt, {:error, error}} + end + end) + end + + def add_relationships_to_result(resource, result, state) do + state + |> Map.get(:relationships, %{}) + |> Enum.reduce(result, fn {name, value}, result -> + # TODO: Figure out `to_remove` + # how does that look for has_one? + case Map.fetch(value, :to_add) do + {:ok, to_add} -> + case Ash.relationship(resource, name) do + %{cardinality: :many} -> + Map.put(result, name, Map.keys(to_add)) + + %{cardinality: :one} -> + Map.put(result, name, to_add) + end + end + end) + end + + defp wrap_in_list(list) do + if Keyword.keyword?(list) do + [list] + else + List.wrap(list) + end + end + + defp add_related_authorizations( + resource, + api, + %{destination: destination} = relationship, + changeset + ) do + default_read = Ash.primary_action(resource, :read) || raise "Need a default read action for #{resource}" + relationship_name = relationship.name + + changeset.__ash_relationships__ + |> Map.get(relationship_name) + |> Map.get(:add, []) + |> wrap_in_list() + |> Enum.reduce_while({:ok, []}, fn related_read, {:ok, authorizations} -> + with {:ok, filters} <- + PrimaryKeyHelpers.values_to_primary_key_filters( + destination, + wrap_in_list(related_read) + ), + %{errors: []} = filter <- Ash.Filter.parse(destination, [or: filters], api) do + read_request = + Ash.Authorization.Request.new( + api: api, + authorization_steps: default_read.authorization_steps, + resource: relationship.destination, + action_type: :read, + filter: filter, + state_key: [:relationships, relationship_name, :to_add], + fetcher: fn -> + api.read(destination, filter: filter) + end, + relationship: [relationship.name], + source: "read prior to write related #{relationship.name}" + ) + + related_requests = + related_add_authorization_requests(api, related_read, relationship, changeset) + + {:cont, {:ok, [read_request | authorizations] ++ related_requests}} + else + {:error, error} -> {:halt, {:error, error}} + %{errors: errors} -> {:halt, {:error, errors}} + end + end) + end + + defp related_add_authorization_requests( + api, + identifier, + %{destination: destination, name: name, type: :has_many} = relationship, + changeset + ) do + pkey = Ash.primary_key(destination) + default_update = Ash.primary_action(destination, :update) + + [ + Ash.Authorization.Request.new( + api: api, + authorization_steps: relationship.authorization_steps, + resource: relationship.source, + changeset: changeset, + action_type: :create, + state_key: [:data], + depends_on: [:data], + fetcher: fn %{data: data} -> data end, + relationship: [], + bypass_strict_access?: true, + source: "Update relationship #{name}" + ), + Ash.Authorization.Request.new( + api: api, + authorization_steps: default_update.authorization_steps, + resource: relationship.destinion, + action_type: :update, + state_key: [:relationships, relationship.name, Map.take(identifier, pkey)], + bypass_strict_access?: true, + dependencies: [[:relationships, name, :to_add], :data], + changeset: fn %{data: data, relationships: %{^name => %{:to_add => to_add}}} -> + related = + Enum.find(to_add, fn to_relate -> + Map.take(to_relate, pkey) == Map.take(identifier, pkey) + end) + + {:ok, + Ecto.Changeset.cast( + related, + %{ + relationship.destination_field => Map.get(data, relationship.source_field) + }, + [relationship.destination_field] + )} + end, + fetcher: fn %{data: data, relationships: %{^name => %{:to_add => to_add}}} -> + related = + Enum.find(to_add, fn to_relate -> + Map.take(to_relate, pkey) == Map.take(identifier, pkey) + end) + + api.update(related, %{ + relationship.destination_field => Map.get(data, relationship.source_field) + }) + end, + relationship: [relationship.name], + source: "Update related #{name} from create" + ) + ] + end + + defp related_add_authorization_requests( + api, + identifier, + %{type: :belongs_to} = relationship, + changeset + ) do + [ + Ash.Authorization.Request.new( + api: api, + authorization_steps: relationship.authorization_steps, + resource: relationship.source, + action_type: :update, + state_key: :data, + dependencies: [:data], + bypass_strict_access?: true, + changeset: + Ecto.Changeset.put_change( + changeset, + relationship.source_field, + Keyword.get(identifier, relationship.destination_field) + ), + fetcher: fn %{data: data} -> + data + end, + relationship: [], + source: "Set relationship #{relationship.name}" + ) + ] + end +end diff --git a/lib/ash/authorization/authorizer.ex b/lib/ash/authorization/authorizer.ex index a7306a65..b8672648 100644 --- a/lib/ash/authorization/authorizer.ex +++ b/lib/ash/authorization/authorizer.ex @@ -22,24 +22,30 @@ defmodule Ash.Authorization.Authorizer do def authorize(user, requests, opts \\ []) do strict_access? = Keyword.get(opts, :strict_access?, true) - if Enum.any?(requests, fn request -> Enum.empty?(request.authorization_steps) end) do - {:error, - Ash.Error.Forbidden.exception( - no_steps_configured?: true, - log_final_report?: opts[:log_final_report?] || false - )} + if opts[:fetch_only?] do + fetch_must_fetch(requests, %{}) else - facts = strict_check_facts(user, requests, strict_access?) + case Enum.find(requests, fn request -> Enum.empty?(request.authorization_steps) end) do + nil -> + {new_requests, facts} = strict_check_facts(user, requests, strict_access?) - solve( - requests, - user, - facts, - facts, - %{user: user}, - strict_access?, - opts[:log_final_report?] || false - ) + solve( + new_requests, + user, + facts, + facts, + %{user: user}, + strict_access?, + opts[:log_final_report?] || false + ) + + request -> + {:error, + Ash.Error.Forbidden.exception( + no_steps_configured: request, + log_final_report?: opts[:log_final_report?] || false + )} + end end end @@ -175,7 +181,8 @@ defmodule Ash.Authorization.Authorizer do defp sat_solver(requests, facts, negations, state) do case state do %{data: [%resource{} | _] = data} -> - # TODO: Needs primary key + # TODO: Needs primary key, looks like some kind of primary key is necessary for + # almost everything ash does :/ pkey = Ash.primary_key(resource) ids = Enum.map(data, &Map.take(&1, pkey)) @@ -241,36 +248,75 @@ defmodule Ash.Authorization.Authorizer do {:error, error} -> {:error, error} - {:ok, new_facts, state} -> - solve( - requests, - user, - new_facts, - strict_check_facts, - state, - strict_access?, - log_final_report? - ) + {:ok, new_requests, new_facts, new_state} -> + if new_requests == requests && new_facts == new_facts && state == new_state do + exception = + Ash.Error.Forbidden.exception( + scenarios: scenarios, + requests: requests, + facts: facts, + strict_check_facts: strict_check_facts, + state: state, + strict_access?: strict_access? + ) + + if log_final_report? do + Logger.info(Ash.Error.Forbidden.report_text(exception)) + end + + {:error, exception} + else + solve( + new_requests, + user, + new_facts, + strict_check_facts, + new_state, + strict_access?, + log_final_report? + ) + end end end end 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}} + unfetched = Enum.reject(requests, &Request.fetched?(state, &1)) - :error -> - case request.fetcher.() do - {:ok, value} -> - {:cont, {:ok, Request.put_request_state(state, request, value)}} + {safe_to_fetch, unmet} = + Enum.split_with(unfetched, fn request -> Request.dependencies_met?(state, request) end) - {:error, error} -> - {:halt, {:error, error}} - end - end - end) + case Enum.filter(safe_to_fetch, &Map.get(&1, :must_fetch?)) do + [] -> + if unmet == [] do + {:ok, state} + else + {:error, + "Could not fetch all required data due to data dependency issues, unmet dependencies existed"} + end + + must_fetch -> + new_state = + Enum.reduce_while(must_fetch, {:ok, state}, fn request, {:ok, state} -> + case Request.fetch(state, request) do + {:ok, new_state} -> {:cont, {:ok, new_state}} + {:error, error} -> {:halt, {:error, error}} + end + end) + + case new_state do + {:ok, new_state} -> + if new_state == state do + {:error, + "Could not fetch all required data due to data dependency issues, no step affected state"} + else + fetch_must_fetch(unfetched, new_state) + end + + {:error, error} -> + {:error, error} + end + end end defp any_scenarios_reality?(scenarios, facts) do @@ -306,8 +352,11 @@ defmodule Ash.Authorization.Authorizer do end 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, strict_access?) + Enum.reduce(requests, {[], %{true: true, false: false}}, fn request, {requests, facts} -> + {new_request, new_facts} = + Ash.Authorization.Checker.strict_check(user, request, facts, strict_access?) + + {[new_request | requests], new_facts} end) end end diff --git a/lib/ash/authorization/check/built_in_checks.ex b/lib/ash/authorization/check/built_in_checks.ex index 24a65bb7..a9fb6fac 100644 --- a/lib/ash/authorization/check/built_in_checks.ex +++ b/lib/ash/authorization/check/built_in_checks.ex @@ -13,11 +13,12 @@ defmodule Ash.Authorization.Check.BuiltInChecks do {Ash.Authorization.Check.AttributeEquals, field: field, value: value} end - defmacro related_to_user_via(relationship) do - quote do - {Ash.Authorization.Check.RelatedToUserVia, - relationship: List.wrap(unquote(relationship)), source: __MODULE__} - end + def related_to_user_via(relationship) do + {Ash.Authorization.Check.RelatedToUserVia, relationship: List.wrap(relationship)} + end + + def setting_relationship(relationship) do + {Ash.Authorization.Check.SettingRelationship, relationship_name: relationship} end def setting_attribute(name, opts) do @@ -37,4 +38,13 @@ defmodule Ash.Authorization.Check.BuiltInChecks do {Ash.Authorization.Check.UserAttributeMatchesRecord, user_field: user_field, record_field: record_field} end + + def relating_to_user(relationship_name, opts) do + {Ash.Authorization.Check.RelatingToUser, + Keyword.put(opts, :relationship_name, relationship_name)} + end + + def relationship_set(relationship_name) do + {Ash.Authorization.Check.RelationshipSet, [relationship_name: relationship_name]} + end end diff --git a/lib/ash/authorization/check/related_to_user_via.ex b/lib/ash/authorization/check/related_to_user_via.ex index e6908aaf..c8b6b831 100644 --- a/lib/ash/authorization/check/related_to_user_via.ex +++ b/lib/ash/authorization/check/related_to_user_via.ex @@ -3,13 +3,13 @@ defmodule Ash.Authorization.Check.RelatedToUserVia do @impl true def describe(opts) do - description = describe_relationship(opts[:source], opts[:relationship]) + description = describe_relationship(opts[:resource], opts[:relationship]) description <> "this_record is the user" end @impl true - def strict_check(%user_resource{} = user, request, opts) do + def strict_check(%user_resource{} = user, request = %{action_type: :read}, opts) do full_relationship_path = request.relationship ++ opts[:relationship] pkey_filter = user |> Map.take(Ash.primary_key(user_resource)) |> Map.to_list() @@ -29,6 +29,8 @@ defmodule Ash.Authorization.Check.RelatedToUserVia do end end + def strict_check(_, _, _), do: {:ok, :unknown} + @impl true def prepare(opts) do [side_load: put_into_relationship_path(opts[:relationship], [])] diff --git a/lib/ash/authorization/check/relating_to_user.ex b/lib/ash/authorization/check/relating_to_user.ex new file mode 100644 index 00000000..154abb1d --- /dev/null +++ b/lib/ash/authorization/check/relating_to_user.ex @@ -0,0 +1,71 @@ +defmodule Ash.Authorization.Check.RelatingToUser do + use Ash.Authorization.Check, action_types: [:update, :delete] + + @impl true + def describe(opts) do + "relating #{opts[:relationship_name]} to the user" + end + + # TODO: Maybe we should check to see if the pkey of the destination is less fields + # and as such we'd need to fetch it before we could determine the answer to this check. + + @impl true + def strict_check(%user_resource{} = user, %{changeset: changeset}, opts) do + pkey = Ash.primary_key(user_resource) + pkey_value = Map.take(user, pkey) |> Map.to_list() + + {:ok, + strict_check_relating_via_attribute?(pkey, pkey_value, opts) || + strict_check_relating?(pkey, pkey_value, changeset, opts)} + end + + def strict_check_relating?(pkey, pkey_value, changeset, opts) do + case Map.fetch(changeset.__ash_relationships__, opts[:relationship_name]) do + {:ok, %{add: relationship_change}} when is_list(relationship_change) -> + op = + if opts[:allow_additional?] do + :any? + else + :all? + end + + relationship_change = + if Keyword.keyword?(relationship_change) do + [relationship_change] + else + relationship_change + end + + found? = + apply(Enum, op, [ + relationship_change, + fn relationship_change -> + Keyword.take(relationship_change, pkey) == pkey_value + end + ]) + + found? + + %{add: nil} -> + false + + _ -> + false + end + end + + def strict_check_relating_via_attribute?([pkey_field], pkey_value, changeset, opts) do + relationship = Ash.relationship(opts[:resource], opts[:relationship_name]) + + case relationship do + %{cardinality: :one, source_field: source_field, destination_field: destination_field} -> + destination_field == pkey_field && + Map.get(pkey_value, pkey_field) == Ecto.Changeset.get_change(changeset, source_field) + + _ -> + false + end + end + + def strict_check_relating_via_attribute?(_, _, _), do: false +end diff --git a/lib/ash/authorization/check/relationship_built_in_checks.ex b/lib/ash/authorization/check/relationship_built_in_checks.ex new file mode 100644 index 00000000..1a06f61c --- /dev/null +++ b/lib/ash/authorization/check/relationship_built_in_checks.ex @@ -0,0 +1,11 @@ +defmodule Ash.Authorization.Check.RelationshipBuiltInChecks do + @moduledoc "The relationship specific authorization checks built into ash" + + def relating_to_user(opts \\ []) do + {Ash.Authorization.Check.RelatingToUser, opts} + end + + def relationship_set() do + {Ash.Authorization.Check.RelationshipSet, []} + end +end diff --git a/lib/ash/authorization/check/relationship_set.ex b/lib/ash/authorization/check/relationship_set.ex new file mode 100644 index 00000000..a11143c2 --- /dev/null +++ b/lib/ash/authorization/check/relationship_set.ex @@ -0,0 +1,27 @@ +defmodule Ash.Authorization.Check.RelationshipSet do + use Ash.Authorization.Check, action_types: [:create, :update, :read, :delete] + + @impl true + def describe(opts) do + "#{opts[:relationship_name]} is already set" + end + + @impl true + # TODO: Add a filter for "has_something_related", and then check for that here for read actions + # TODO: Make this support a nested relationship path? + def strict_check(_user, _request, _options) do + {:ok, :unknown} + end + + @impl true + def prepare(opts) do + [side_load: opts[:relationship_name]] + end + + @impl true + def check(_user, records, _state, opts) do + Enum.reject(records, fn record -> + Map.get(record, opts[:relationship_name]) in [nil, []] + end) + end +end diff --git a/lib/ash/authorization/check/setting_attribute.ex b/lib/ash/authorization/check/setting_attribute.ex index 39541bf9..d8798425 100644 --- a/lib/ash/authorization/check/setting_attribute.ex +++ b/lib/ash/authorization/check/setting_attribute.ex @@ -3,14 +3,29 @@ defmodule Ash.Authorization.Check.SettingAttribute do @impl true def describe(opts) do - "setting #{opts[:attribute_name]} to #{inspect(opts[:to])}" + case Keyword.fetch(opts, :to) do + {:ok, should_equal} -> + "setting #{opts[:attribute_name]} to #{inspect(should_equal)}" + + :error -> + "setting #{opts[:attribute_name]}" + end end @impl true def strict_check(_user, %{changeset: %Ecto.Changeset{} = changeset}, opts) do case Ecto.Changeset.fetch_change(changeset, opts[:attribute_name]) do - {:ok, value} -> {:ok, value == opts[:to]} - :error -> {:ok, false} + {:ok, value} -> + case Keyword.fetch(opts, :to) do + {:ok, should_equal} -> + {:ok, value == should_equal} + + :error -> + {:ok, true} + end + + :error -> + {:ok, false} end end end diff --git a/lib/ash/authorization/check/setting_relationship.ex b/lib/ash/authorization/check/setting_relationship.ex new file mode 100644 index 00000000..bfcf3546 --- /dev/null +++ b/lib/ash/authorization/check/setting_relationship.ex @@ -0,0 +1,13 @@ +defmodule Ash.Authorization.Check.SettingRelationship do + use Ash.Authorization.Check, action_types: [:create, :update] + + @impl true + def describe(opts) do + "setting #{opts[:relationship_name]}" + end + + @impl true + def strict_check(_user, %{changeset: changeset}, options) do + {:ok, Map.has_key?(changeset.__ash_relationships__, options[:relationship_name])} + end +end diff --git a/lib/ash/authorization/checker.ex b/lib/ash/authorization/checker.ex index 8452ea2b..dad90568 100644 --- a/lib/ash/authorization/checker.ex +++ b/lib/ash/authorization/checker.ex @@ -2,29 +2,46 @@ defmodule Ash.Authorization.Checker do alias Ash.Authorization.Request alias Ash.Actions.SideLoad - def strict_check(user, request, facts, strict_access?) do - request.authorization_steps - |> Enum.reduce(facts, fn {_step, clause}, facts -> - case Map.fetch(facts, {request.relationship, clause}) do - {:ok, _boolean_result} -> - facts + # TODO: strict_check can't do things with dependencies. Meaning, + # we need to run strict check for things with dependencies in the + # second phase. So we should prioritize things in this way: + # 1.) Things who's dependencies unlock strict checks + # 2.) things who's strict checks were never run + # 3.) Generate the changeset for those + # 3.5) probably make it invalid to have an auth request with a changeset function + # but no dependencies. + # 4.) run strict checks - :error -> - case do_strict_check(clause, user, request, strict_access?) do - :unknown -> + def strict_check(user, request, facts, strict_access?) do + if Request.can_strict_check?(request) do + new_facts = + request.authorization_steps + |> Enum.reduce(facts, fn {_step, clause}, facts -> + case Map.fetch(facts, {request.relationship, clause}) do + {:ok, _boolean_result} -> facts - :unknowable -> - Map.put(facts, clause, :unknowable) + :error -> + case do_strict_check(clause, user, request, strict_access?) do + :unknown -> + facts - :irrelevant -> - Map.put(facts, clause, :irrelevant) + :unknowable -> + Map.put(facts, clause, :unknowable) - boolean -> - Map.put(facts, clause, boolean) + :irrelevant -> + Map.put(facts, clause, :irrelevant) + + boolean -> + Map.put(facts, clause, boolean) + end end - end - end) + end) + + {Map.put(request, :strict_check_completed?, true), new_facts} + else + {request, facts} + end end def run_checks(scenarios, user, requests, facts, state, strict_access?) do @@ -60,39 +77,48 @@ defmodule Ash.Authorization.Checker do # TODO: We could be smart here, and likely fetch multiple requests at a time defp fetch_requests(requests, state, strict_access?) do - unfetched_requests = - Enum.reject(requests, fn request -> + fetchable_requests = + requests + |> Enum.reject(fn request -> Request.fetched?(state, request) end) + |> Enum.filter(fn request -> + Request.dependencies_met?(state, request) + end) requests_without_strict_access = if strict_access? do - Enum.filter(unfetched_requests, fn request -> + Enum.filter(fetchable_requests, fn request -> request.bypass_strict_access? end) else - unfetched_requests + fetchable_requests end requests_without_strict_access + |> Enum.filter(fn request -> + Request.dependencies_met?(state, request) && request.strict_check_completed? + end) + |> Enum.map(fn request -> + Request.fetch_changeset(state, request) + end) |> Enum.sort_by(fn request -> # Requests that bypass strict access should generally perform well # as they would generally be more efficient checks {Enum.count(request.relationship), not request.bypass_strict_access?, request.relationship} end) - |> Enum.at(0) |> case do - nil -> - :all_scenarios_known + [request | _] = requests -> + case Request.fetch_request_state(state, request) do + {:ok, new_state} -> + {:ok, {requests, new_state}} - request -> - case request.fetcher.() do - {:ok, value} -> - {:ok, Request.put_request_state(state, request, value)} - - {:error, error} -> - {:error, error} + :error -> + {:ok, {requests, state}} end + + _ -> + :all_scenarios_known end end @@ -143,7 +169,8 @@ defmodule Ash.Authorization.Checker 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) + Request.fetched?(state, request) && Request.contains_clause?(request, clause) && + Request.dependencies_met?(state, request) end) end) end diff --git a/lib/ash/authorization/report.ex b/lib/ash/authorization/report.ex index 99c93c25..a3ccda95 100644 --- a/lib/ash/authorization/report.ex +++ b/lib/ash/authorization/report.ex @@ -10,11 +10,12 @@ defmodule Ash.Authorization.Report do :strict_access?, :header, :authorized?, - no_steps_configured?: false + no_steps_configured: false ] - def report(%{no_steps_configured?: true}) do - "One of the authorizations required had no authorization steps configured." + def report(%{no_steps_configured: %Ash.Authorization.Request{} = request}) do + "forbidden:\n" <> + request.source <> ": no authorization steps configured. Resource: #{request.resource}" end # We know that each group of authorization steps shares the same relationship diff --git a/lib/ash/authorization/request.ex b/lib/ash/authorization/request.ex index 7c775e34..efba6448 100644 --- a/lib/ash/authorization/request.ex +++ b/lib/ash/authorization/request.ex @@ -4,12 +4,14 @@ defmodule Ash.Authorization.Request do :authorization_steps, :filter, :action_type, + :dependencies, :bypass_strict_access?, :relationship, :fetcher, :source, :must_fetch?, :state_key, + :strict_check_completed?, :api, :changeset ] @@ -20,10 +22,12 @@ defmodule Ash.Authorization.Request do authorization_steps: list(term), filter: Ash.Filter.t(), changeset: Ecto.Changeset.t(), + dependencies: list(term), # TODO: fetcher is a function fetcher: term, relationship: list(atom), bypass_strict_access?: boolean, + strict_check_completed?: boolean, source: String.t(), must_fetch?: boolean, state_key: term, @@ -36,6 +40,8 @@ defmodule Ash.Authorization.Request do |> Keyword.put_new(:relationship, []) |> Keyword.put_new(:authorization_steps, []) |> Keyword.put_new(:bypass_strict_access?, false) + |> Keyword.put_new(:dependencies, []) + |> Keyword.put_new(:strict_check_completed?, false) |> Keyword.update!(:authorization_steps, fn steps -> Enum.map(steps, fn {step, fact} -> {step, Ash.Authorization.Clause.new(opts[:relationship] || [], opts[:resource], fact)} @@ -45,6 +51,20 @@ defmodule Ash.Authorization.Request do struct!(__MODULE__, opts) end + def can_strict_check?(%{changeset: changeset}) when is_function(changeset), do: false + def can_strict_check?(_), do: true + + def dependencies_met?(_state, %{dependencies: []}), do: true + + def dependencies_met?(state, %{dependencies: dependencies}) do + Enum.all?(dependencies, fn dependency -> + case fetch_nested_value(state, dependency) do + {:ok, _} -> true + _ -> false + end + end) + end + def contains_clause?(request, clause) do Enum.any?(request.authorization_steps, fn {_step, request_clause} -> clause == request_clause @@ -60,12 +80,95 @@ defmodule Ash.Authorization.Request do def put_request_state(state, %{state_key: state_key} = request, value) do state_key = state_key || request - Map.put(state, state_key, value) + + key = + state_key + |> Kernel.||(request) + |> List.wrap() + + put_nested_key(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) + key = + state_key + |> Kernel.||(request) + |> List.wrap() + + fetch_nested_value(state, key) + end + + def fetch(state, %{fetcher: fetcher, dependencies: []} = request) do + case fetcher.() do + {:ok, value} -> + {:ok, put_request_state(state, request, value)} + + {:error, error} -> + {:error, error} + end + end + + def fetch(state, %{fetcher: fetcher, dependencies: dependencies} = request) do + arg = + Enum.reduce(dependencies, %{}, fn dependency, acc -> + {:ok, value} = fetch_nested_value(state, dependency) + put_nested_key(acc, dependency, value) + end) + + case fetcher.(arg) do + {:ok, value} -> + {:ok, put_request_state(state, request, value)} + + {:error, error} -> + {:error, error} + end + end + + def fetch_changeset(state, %{dependencies: dependencies, changeset: changeset} = request) + when is_function(changeset) do + arg = + Enum.reduce(dependencies, %{}, fn dependency, acc -> + {:ok, value} = fetch_nested_value(state, dependency) + put_nested_key(acc, dependency, value) + end) + + case changeset.(arg) do + {:ok, new_changeset} -> + {:ok, %{request | changeset: new_changeset}} + + {:error, error} -> + {:error, error} + end + end + + defp fetch_nested_value(state, [key]) when is_map(state) do + Map.fetch(state, key) + end + + defp fetch_nested_value(state, [key | rest]) when is_map(state) do + case Map.fetch(state, key) do + {:ok, value} -> fetch_nested_value(value, rest) + :error -> :error + end + end + + defp fetch_nested_value(state, key) when is_map(state) do + Map.fetch(state, key) + end + + defp put_nested_key(state, [key], value) do + Map.put(state, key, value) + end + + defp put_nested_key(state, [key | rest], value) do + case Map.fetch(state, key) do + {:ok, nested_state} when is_map(nested_state) -> + Map.put(state, key, put_nested_key(nested_state, rest, value)) + + :error -> + Map.put(state, key, put_nested_key(%{}, rest, value)) + end end end diff --git a/lib/ash/error/forbidden.ex b/lib/ash/error/forbidden.ex index 9c76efaa..86634098 100644 --- a/lib/ash/error/forbidden.ex +++ b/lib/ash/error/forbidden.ex @@ -10,7 +10,7 @@ defmodule Ash.Error.Forbidden do :strict_check_facts, :state, :strict_access?, - no_steps_configured?: false + no_steps_configured: false ] def message(error) do @@ -21,7 +21,7 @@ defmodule Ash.Error.Forbidden do strict_check_facts: error.strict_check_facts, state: error.state, strict_access?: error.strict_access?, - no_steps_configured?: error.no_steps_configured?, + no_steps_configured: error.no_steps_configured, header: "forbidden:", authorized?: false } @@ -37,7 +37,7 @@ defmodule Ash.Error.Forbidden do strict_check_facts: error.strict_check_facts, state: error.state, strict_access?: error.strict_access?, - no_steps_configured?: error.no_steps_configured?, + no_steps_configured: error.no_steps_configured, header: header, authorized?: false } diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index c6f9843f..71470441 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -99,6 +99,7 @@ defmodule Ash.Filter do def strict_subset_of?(_, nil), do: false def strict_subset_of?(filter, candidate) do + # TODO: Finish this! unless filter.ors in [[], nil], do: raise("Can't do ors contains yet") unless filter.not in [[], nil], do: raise("Can't do not contains yet") unless candidate.ors in [[], nil], do: raise("Can't do ors contains yet") diff --git a/lib/ash/resource.ex b/lib/ash/resource.ex index ef5094b0..90a16fb5 100644 --- a/lib/ash/resource.ex +++ b/lib/ash/resource.ex @@ -125,7 +125,7 @@ defmodule Ash.Resource do case opts[:primary_key] do true -> {:ok, attribute} = - Ash.Resource.Attributes.Attribute.new(:id, :uuid, + Ash.Resource.Attributes.Attribute.new(mod, :id, :uuid, primary_key?: true, default: &Ecto.UUID.generate/0 ) @@ -137,7 +137,7 @@ defmodule Ash.Resource do opts -> {:ok, attribute} = - Ash.Resource.Attributes.Attribute.new(opts[:field], opts[:type], primary_key?: true) + Ash.Resource.Attributes.Attribute.new(mod, opts[:field], opts[:type], primary_key?: true) Module.put_attribute(mod, :attributes, attribute) end diff --git a/lib/ash/resource/actions/actions.ex b/lib/ash/resource/actions/actions.ex index 8f1bdf6f..a6c49dd2 100644 --- a/lib/ash/resource/actions/actions.ex +++ b/lib/ash/resource/actions/actions.ex @@ -49,7 +49,7 @@ defmodule Ash.Resource.Actions do path: [:actions, :create] end - case Ash.Resource.Actions.Create.new(name, opts) do + case Ash.Resource.Actions.Create.new(__MODULE__, name, opts) do {:ok, action} -> @actions action @@ -80,7 +80,7 @@ defmodule Ash.Resource.Actions do path: [:actions, :read] end - case Ash.Resource.Actions.Read.new(name, opts) do + case Ash.Resource.Actions.Read.new(__MODULE__, name, opts) do {:ok, action} -> @actions action @@ -111,7 +111,7 @@ defmodule Ash.Resource.Actions do path: [:actions, :update] end - case Ash.Resource.Actions.Update.new(name, opts) do + case Ash.Resource.Actions.Update.new(__MODULE__, name, opts) do {:ok, action} -> @actions action @@ -142,7 +142,7 @@ defmodule Ash.Resource.Actions do path: [:actions, :destroy] end - case Ash.Resource.Actions.Destroy.new(name, opts) do + case Ash.Resource.Actions.Destroy.new(__MODULE__, name, opts) do {:ok, action} -> @actions action diff --git a/lib/ash/resource/actions/create.ex b/lib/ash/resource/actions/create.ex index 5fea2d09..e8c5d6c9 100644 --- a/lib/ash/resource/actions/create.ex +++ b/lib/ash/resource/actions/create.ex @@ -29,16 +29,31 @@ defmodule Ash.Resource.Actions.Create do @doc false def opt_schema(), do: @opt_schema - @spec new(atom, Keyword.t()) :: {:ok, t()} | {:error, term} - def new(name, opts \\ []) do + @spec new(Ash.resource(), atom, Keyword.t()) :: {:ok, t()} | {:error, term} + def new(resource, name, opts \\ []) do case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, type: :create, primary?: opts[:primary?], - authorization_steps: opts[:authorization_steps] + authorization_steps: authorization_steps }} {:error, error} -> diff --git a/lib/ash/resource/actions/destroy.ex b/lib/ash/resource/actions/destroy.ex index 354780e4..75118059 100644 --- a/lib/ash/resource/actions/destroy.ex +++ b/lib/ash/resource/actions/destroy.ex @@ -30,16 +30,32 @@ defmodule Ash.Resource.Actions.Destroy do @doc false def opt_schema(), do: @opt_schema - @spec new(atom, Keyword.t()) :: {:ok, t()} | {:error, term} - def new(name, opts \\ []) do + @spec new(Ash.resource(), atom, Keyword.t()) :: {:ok, t()} | {:error, term} + def new(resource, name, opts \\ []) do + # Don't call functions on the resource! We don't want it to compile here case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, type: :destroy, primary?: opts[:primary?], - authorization_steps: opts[:authorization_steps] + authorization_steps: authorization_steps }} {:error, error} -> diff --git a/lib/ash/resource/actions/read.ex b/lib/ash/resource/actions/read.ex index 18ba343d..ffeef3df 100644 --- a/lib/ash/resource/actions/read.ex +++ b/lib/ash/resource/actions/read.ex @@ -35,16 +35,32 @@ defmodule Ash.Resource.Actions.Read do @doc false def opt_schema(), do: @opt_schema - @spec new(atom, Keyword.t()) :: {:ok, t()} | {:error, term} - def new(name, opts \\ []) do + @spec new(Ash.resource(), atom, Keyword.t()) :: {:ok, t()} | {:error, term} + def new(resource, name, opts \\ []) do + # Don't call functions on the resource! We don't want it to compile here case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, type: :read, primary?: opts[:primary?], - authorization_steps: opts[:authorization_steps], + authorization_steps: authorization_steps, paginate?: opts[:paginate?] }} diff --git a/lib/ash/resource/actions/update.ex b/lib/ash/resource/actions/update.ex index cc9d3402..2f3fdffd 100644 --- a/lib/ash/resource/actions/update.ex +++ b/lib/ash/resource/actions/update.ex @@ -30,16 +30,32 @@ defmodule Ash.Resource.Actions.Update do @doc false def opt_schema(), do: @opt_schema - @spec new(atom, Keyword.t()) :: {:ok, t()} | {:error, term} - def new(name, opts \\ []) do + @spec new(Ash.resource(), atom, Keyword.t()) :: {:ok, t()} | {:error, term} + def new(resource, name, opts \\ []) do + # Don't call functions on the resource! We don't want it to compile here case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, type: :update, primary?: opts[:primary?], - authorization_steps: opts[:authorization_steps] + authorization_steps: authorization_steps }} {:error, error} -> diff --git a/lib/ash/resource/attributes/attribute.ex b/lib/ash/resource/attributes/attribute.ex index 985b0048..87383bbb 100644 --- a/lib/ash/resource/attributes/attribute.ex +++ b/lib/ash/resource/attributes/attribute.ex @@ -28,10 +28,6 @@ defmodule Ash.Resource.Attributes.Attribute do authorization_steps: [] ], describe: [ - authorization_steps: """ - Rules applied on an attribute during create or update. If no rules are defined, authorization to change will fail. - If set to false, no rules are applied and any changes are allowed (assuming the action was authorized as a whole) - """, allow_nil?: """ Whether or not to allow `null` values. Ash can perform optimizations with this information, so if you do not expect any null values, make sure to set this switch. @@ -39,15 +35,20 @@ defmodule Ash.Resource.Attributes.Attribute do primary_key?: "Whether this field is, or is part of, the primary key of a resource.", default: - "A one argument function that returns a default value, an mfa that does the same, or a raw value via specifying `{:constant, value}`." + "A one argument function that returns a default value, an mfa that does the same, or a raw value via specifying `{:constant, value}`.", + authorization_steps: """ + Rules applied on an attribute during create or update. If no rules are defined, authorization to change will fail. + If set to false, no rules are applied and any changes are allowed (assuming the action was authorized as a whole) + """ ] ) @doc false def attribute_schema(), do: @schema - @spec new(atom, Ash.Type.t(), Keyword.t()) :: {:ok, t()} | {:error, term} - def new(name, type, opts) do + @spec new(Ash.resource(), atom, Ash.Type.t(), Keyword.t()) :: {:ok, t()} | {:error, term} + def new(resource, name, type, opts) do + # Don't call functions on the resource! We don't want it to compile here with {:ok, opts} <- Ashton.validate(opts, @schema), {:default, {:ok, default}} <- {:default, cast_default(type, opts)} do authorization_steps = @@ -58,7 +59,8 @@ defmodule Ash.Resource.Attributes.Attribute do steps -> base_attribute_opts = [ attribute_name: name, - attribute_type: type + attribute_type: type, + resource: resource ] Enum.map(steps, fn {step, {mod, opts}} -> diff --git a/lib/ash/resource/attributes/attributes.ex b/lib/ash/resource/attributes/attributes.ex index f1f96725..7db5e296 100644 --- a/lib/ash/resource/attributes/attributes.ex +++ b/lib/ash/resource/attributes/attributes.ex @@ -58,7 +58,7 @@ defmodule Ash.Resource.Attributes do path: [:attributes, :attribute, name] end - case Ash.Resource.Attributes.Attribute.new(name, type, opts) do + case Ash.Resource.Attributes.Attribute.new(__MODULE__, name, type, opts) do {:ok, attribute} -> @attributes attribute diff --git a/lib/ash/resource/relationships/belongs_to.ex b/lib/ash/resource/relationships/belongs_to.ex index d4f232e5..1a890091 100644 --- a/lib/ash/resource/relationships/belongs_to.ex +++ b/lib/ash/resource/relationships/belongs_to.ex @@ -11,7 +11,8 @@ defmodule Ash.Resource.Relationships.BelongsTo do :destination_field, :source_field, :source, - :reverse_relationship + :reverse_relationship, + :authorization_steps ] @type t :: %__MODULE__{ @@ -24,7 +25,8 @@ defmodule Ash.Resource.Relationships.BelongsTo do define_field?: boolean, field_type: Ash.Type.t(), destination_field: atom, - source_field: atom | nil + source_field: atom | nil, + authorization_steps: Keyword.t() } @opt_schema Ashton.schema( @@ -34,13 +36,15 @@ defmodule Ash.Resource.Relationships.BelongsTo do primary_key?: :boolean, define_field?: :boolean, field_type: :atom, - reverse_relationship: :atom + reverse_relationship: :atom, + authorization_steps: :keyword ], defaults: [ destination_field: :id, primary_key?: false, define_field?: true, - field_type: :uuid + field_type: :uuid, + authorization_steps: [] ], describe: [ reverse_relationship: @@ -53,7 +57,12 @@ defmodule Ash.Resource.Relationships.BelongsTo do source_field: "The field on this resource that should match the `destination_field` on the related resource. Default: [relationship_name]_id", primary_key?: - "Whether this field is, or is part of, the primary key of a resource." + "Whether this field is, or is part of, the primary key of a resource.", + authorization_steps: """ + Steps applied on an relationship during create or update. If no steps are defined, authorization to change will fail. + If set to false, no steps are applied and any changes are allowed (assuming the action was authorized as a whole) + Remember that any changes against the destination records *will* still be authorized regardless of this setting. + """ ] ) @@ -70,9 +79,27 @@ defmodule Ash.Resource.Relationships.BelongsTo do # Don't call functions on the resource! We don't want it to compile here case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + relationship_name: name, + destination: related_resource, + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, + authorization_steps: authorization_steps, source: resource, type: :belongs_to, cardinality: :one, diff --git a/lib/ash/resource/relationships/has_many.ex b/lib/ash/resource/relationships/has_many.ex index 01529ed8..8bebba5c 100644 --- a/lib/ash/resource/relationships/has_many.ex +++ b/lib/ash/resource/relationships/has_many.ex @@ -6,14 +6,17 @@ defmodule Ash.Resource.Relationships.HasMany do :destination, :destination_field, :source_field, + :authorization_steps, :source, - :reverse_relationship + :reverse_relationship, + :authorization_steps ] @type t :: %__MODULE__{ type: :has_many, cardinality: :many, source: Ash.resource(), + authorization_steps: Keyword.t(), name: atom, type: Ash.Type.t(), destination: Ash.resource(), @@ -26,10 +29,12 @@ defmodule Ash.Resource.Relationships.HasMany do opts: [ destination_field: :atom, source_field: :atom, + authorization_steps: :keyword, reverse_relationship: :atom ], defaults: [ - source_field: :id + source_field: :id, + authorization_steps: [] ], describe: [ reverse_relationship: @@ -37,7 +42,12 @@ defmodule Ash.Resource.Relationships.HasMany do destination_field: "The field on the related resource that should match the `source_field` on this resource. Default: [resource.name]_id", source_field: - "The field on this resource that should match the `destination_field` on the related resource." + "The field on this resource that should match the `destination_field` on the related resource.", + authorization_steps: """ + Steps applied on an relationship during create or update. If no steps are defined, authorization to change will fail. + If set to false, no steps are applied and any changes are allowed (assuming the action was authorized as a whole) + Remember that any changes against the destination records *will* still be authorized regardless of this setting. + """ ] ) @@ -54,9 +64,27 @@ defmodule Ash.Resource.Relationships.HasMany do # Don't call functions on the resource! We don't want it to compile here case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + relationship_name: name, + destination: related_resource, + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, + authorization_steps: authorization_steps, source: resource, type: :has_many, cardinality: :many, diff --git a/lib/ash/resource/relationships/has_one.ex b/lib/ash/resource/relationships/has_one.ex index 0f596ac1..576023c4 100644 --- a/lib/ash/resource/relationships/has_one.ex +++ b/lib/ash/resource/relationships/has_one.ex @@ -8,7 +8,8 @@ defmodule Ash.Resource.Relationships.HasOne do :destination, :destination_field, :source_field, - :reverse_relationship + :reverse_relationship, + :authorization_steps ] @type t :: %__MODULE__{ @@ -17,6 +18,7 @@ defmodule Ash.Resource.Relationships.HasOne do source: Ash.resource(), name: atom, type: Ash.Type.t(), + authorization_steps: Keyword.t(), destination: Ash.resource(), destination_field: atom, source_field: atom, @@ -27,10 +29,12 @@ defmodule Ash.Resource.Relationships.HasOne do opts: [ destination_field: :atom, source_field: :atom, - reverse_relationship: :atom + reverse_relationship: :atom, + authorization_steps: :keyword ], defaults: [ - source_field: :id + source_field: :id, + authorization_steps: [] ], describe: [ reverse_relationship: @@ -38,7 +42,12 @@ defmodule Ash.Resource.Relationships.HasOne do destination_field: "The field on the related resource that should match the `source_field` on this resource. Default: [resource.name]_id", source_field: - "The field on this resource that should match the `destination_field` on the related resource." + "The field on this resource that should match the `destination_field` on the related resource.", + authorization_steps: """ + Steps applied on an relationship during create or update. If no steps are defined, authorization to change will fail. + If set to false, no steps are applied and any changes are allowed (assuming the action was authorized as a whole) + Remember that any changes against the destination records *will* still be authorized regardless of this setting. + """ ] ) @@ -57,6 +66,23 @@ defmodule Ash.Resource.Relationships.HasOne do # Don't call functions on the resource! We don't want it to compile here case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + relationship_name: name, + destination: related_resource, + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, @@ -66,7 +92,8 @@ defmodule Ash.Resource.Relationships.HasOne do destination: related_resource, destination_field: opts[:destination_field] || :"#{resource_type}_id", source_field: opts[:source_field], - reverse_relationship: opts[:reverse_relationship] + reverse_relationship: opts[:reverse_relationship], + authorization_steps: authorization_steps }} {:error, errors} -> diff --git a/lib/ash/resource/relationships/many_to_many.ex b/lib/ash/resource/relationships/many_to_many.ex index 693aa3f3..59377baf 100644 --- a/lib/ash/resource/relationships/many_to_many.ex +++ b/lib/ash/resource/relationships/many_to_many.ex @@ -10,7 +10,8 @@ defmodule Ash.Resource.Relationships.ManyToMany do :destination_field, :source_field_on_join_table, :destination_field_on_join_table, - :reverse_relationship + :reverse_relationship, + :authorization_steps ] @type t :: %__MODULE__{ @@ -24,7 +25,8 @@ defmodule Ash.Resource.Relationships.ManyToMany do destination_field: atom, source_field_on_join_table: atom, destination_field_on_join_table: atom, - reverse_relationship: atom + reverse_relationship: atom, + authorization_steps: Keyword.t() } @opt_schema Ashton.schema( @@ -33,12 +35,15 @@ defmodule Ash.Resource.Relationships.ManyToMany do destination_field_on_join_table: :atom, source_field: :atom, destination_field: :atom, + authorization_steps: :keyword, through: :atom, - reverse_relationship: :atom + reverse_relationship: :atom, + authorization_steps: :keyword ], defaults: [ source_field: :id, - destination_field: :id + destination_field: :id, + authorization_steps: [] ], required: [ :through @@ -54,7 +59,12 @@ defmodule Ash.Resource.Relationships.ManyToMany do source_field: "The field on this resource that should line up with `source_field_on_join_table` on the join table.", destination_field: - "The field on the related resource that should line up with `destination_field_on_join_table` on the join table." + "The field on the related resource that should line up with `destination_field_on_join_table` on the join table.", + authorization_steps: """ + Steps applied on an relationship during create or update. If no steps are defined, authorization to change will fail. + If set to false, no steps are applied and any changes are allowed (assuming the action was authorized as a whole) + Remember that any changes against the destination records *will* still be authorized regardless of this setting. + """ ] ) @@ -72,6 +82,23 @@ defmodule Ash.Resource.Relationships.ManyToMany do # Don't call functions on the resource! We don't want it to compile here case Ashton.validate(opts, @opt_schema) do {:ok, opts} -> + authorization_steps = + case opts[:authorization_steps] do + false -> + false + + steps -> + base_attribute_opts = [ + relationship_name: name, + destination: related_resource, + resource: resource + ] + + Enum.map(steps, fn {step, {mod, opts}} -> + {step, {mod, Keyword.merge(base_attribute_opts, opts)}} + end) + end + {:ok, %__MODULE__{ name: name, @@ -83,6 +110,7 @@ defmodule Ash.Resource.Relationships.ManyToMany do reverse_relationship: opts[:reverse_relationship], source_field: opts[:source_field], destination_field: opts[:destination_field], + authorization_steps: authorization_steps, source_field_on_join_table: opts[:source_field_on_join_table] || :"#{resource_name}_id", destination_field_on_join_table: diff --git a/lib/ash/resource/relationships/relationships.ex b/lib/ash/resource/relationships/relationships.ex index 0fe856dc..1685f83a 100644 --- a/lib/ash/resource/relationships/relationships.ex +++ b/lib/ash/resource/relationships/relationships.ex @@ -12,7 +12,13 @@ defmodule Ash.Resource.Relationships do defmacro relationships(do: block) do quote do import Ash.Resource.Relationships + import Ash.Authorization.Check.BuiltInChecks + import Ash.Authorization.Check.RelationshipBuiltInChecks + unquote(block) + + import Ash.Authorization.Check.BuiltInChecks, only: [] + import Ash.Authorization.Check.RelationshipBuiltInChecks, only: [] import Ash.Resource.Relationships, only: [relationships: 1] end end @@ -124,6 +130,7 @@ defmodule Ash.Resource.Relationships do if relationship.define_field? do {:ok, attribute} = Ash.Resource.Attributes.Attribute.new( + __MODULE__, relationship.source_field, relationship.field_type, primary_key?: relationship.primary_key? diff --git a/test/authorization/create_authorization_test.exs b/test/authorization/create_authorization_test.exs index 1318853d..c060f8b0 100644 --- a/test/authorization/create_authorization_test.exs +++ b/test/authorization/create_authorization_test.exs @@ -1,6 +1,35 @@ defmodule Ash.Test.Authorization.CreateAuthorizationTest do use ExUnit.Case, async: true + defmodule Draft do + use Ash.Resource, name: "drafts", type: "draft" + use Ash.DataLayer.Ets, private?: true + + actions do + read :default, + authorization_steps: [ + authorize_if: always() + ] + + create :default, + authorization_steps: [ + forbid_unless: setting_relationship(:author), + authorize_if: user_attribute(:author, true) + ] + end + + attributes do + attribute :contents, :string, authorization_steps: false + end + + relationships do + belongs_to :author, Ash.Test.Authorization.CreateAuthorizationTest.Author, + authorization_steps: [ + authorize_if: relating_to_user() + ] + end + end + defmodule Author do use Ash.Resource, name: "authors", type: "author" use Ash.DataLayer.Ets, private?: true @@ -44,6 +73,28 @@ defmodule Ash.Test.Authorization.CreateAuthorizationTest do end end + defmodule Bio do + use Ash.Resource, name: "bios", type: "bio" + use Ash.DataLayer.Ets, private?: true + + actions do + read :default + + create :default, + authorization_steps: [ + forbid_unless: setting_relationship(:author), + authorize_if: user_attribute(:author, true) + ] + end + + relationships do + belongs_to :author, Author, + authorization_steps: [ + authorize_if: relating_to_user() + ] + end + end + defmodule User do use Ash.Resource, name: "users", type: "user" use Ash.DataLayer.Ets, private?: true @@ -57,6 +108,7 @@ defmodule Ash.Test.Authorization.CreateAuthorizationTest do attribute :name, :string attribute :manager, :boolean, default: {:constant, false} attribute :admin, :boolean, default: {:constant, false} + attribute :author, :boolean, default: {:constant, false} end end @@ -102,15 +154,14 @@ defmodule Ash.Test.Authorization.CreateAuthorizationTest do end relationships do - has_many :author_posts, AuthorPost - many_to_many :authors, Author, through: :author_posts + many_to_many :authors, Author, through: AuthorPost end end defmodule Api do use Ash.Api - resources [Post, Author, AuthorPost, User] + resources [Post, Author, AuthorPost, User, Draft] end test "should fail if a user does not match the action requirements" do @@ -137,4 +188,28 @@ defmodule Ash.Test.Authorization.CreateAuthorizationTest do Api.create!(Author, attributes: %{name: "foo", state: "open"}, authorization: [user: user]) end + + test "forbids belongs_to relationships properly" do + user = Api.create!(User, attributes: %{name: "foo", author: true}) + author = Api.create!(Author, attributes: %{name: "someone else"}) + + assert_raise Ash.Error.Forbidden, ~r/forbidden/, fn -> + Api.create!(Draft, + attributes: %{contents: "best ever"}, + relationships: %{author: author.id}, + authorization: [user: user] + ) + end + end + + test "allows belongs_to relationships properly" do + user = Api.create!(User, attributes: %{name: "foo", author: true}) + author = Api.create!(Author, attributes: %{name: "someone else", id: user.id}) + + Api.create!(Draft, + attributes: %{contents: "best ever"}, + relationships: %{author: author.id}, + authorization: [user: user] + ) + end end diff --git a/test/authorization/get_authorization_test.exs b/test/authorization/get_authorization_test.exs index 4ff1bc96..2a37c4ef 100644 --- a/test/authorization/get_authorization_test.exs +++ b/test/authorization/get_authorization_test.exs @@ -30,6 +30,8 @@ defmodule Ash.Test.Authorization.GetAuthorizationTest do relationships do many_to_many :posts, Ash.Test.Authorization.AuthorizationTest.Post, through: Ash.Test.Authorization.AuthorizationTest.AuthorPost + + has_many :drafts, Draft end end @@ -89,7 +91,7 @@ defmodule Ash.Test.Authorization.GetAuthorizationTest do relationships do has_many :author_posts, AuthorPost - many_to_many :authors, Author, through: :author_posts + many_to_many :authors, Author, through: AuthorPost end end diff --git a/test/authorization/read_authorization_test.exs b/test/authorization/read_authorization_test.exs index 368505d0..141f7656 100644 --- a/test/authorization/read_authorization_test.exs +++ b/test/authorization/read_authorization_test.exs @@ -89,7 +89,7 @@ defmodule Ash.Test.Authorization.ReadAuthorizationTest do relationships do has_many :author_posts, AuthorPost - many_to_many :authors, Author, through: :author_posts + many_to_many :authors, Author, through: AuthorPost end end