Ash Framework

barnabasj:

https://github.com/traveltechdeluxe/ash-rbac

natterstefan:

Awesome! 🚀

Eduardo B. Alexandre:

Does this generate, under the hood, a policies code block, meaning it still uses the policies mechanism from Ash? Will it always check for the roles field in a resource for its role or is there any way to configure that (for example, for my users, I normally have a roles and organization_roles fields that mean different things).

barnabasj:

It is just generating normal policy and field_policy blocks. I was thinking of making the role getter function configurable, but as of now it expects an actor with a roles attribute

barnabasj:

We also combine this with normal policy blocks to add filter policies on resources that need them, as the extension is not handling that case at the moment. Tests are the best docs atm

barnabasj:

As relationships work a bit differently, e.g. not supported by field_policies. But you can specify that a user can only use an action if it is accessed from a relationship

Eduardo B. Alexandre:

Super cool man, this seems to have the potential to make my projects policies way simpler, amazing work 👍

barnabasj:

Thanks, field policies really brought everything together.

Eduardo B. Alexandre:

Would you be interested in a PR adding support for custom checkers?

I was able to change the code to allow it like this:

  rbac do
    role :admin do
      fields [:*]
      actions [:create, :read]
    end

     role :user, checker: HasRole2 do
       fields [:id, :child, :children, :number]
       actions [:read]
     end
  end

I can’t figure out how to make the bypass part take 2 arguments instead of one to allow something like this:

bypass :admin, checker: HasRole2

barnabasj:

Sure, if you open the PR we can check what is needed for the bypass and stuff

barnabasj:

also not sure if changing the whole module is the right approach, as the semantics of the check shouldn’t change. Would it be enough for you use case if you were able to pass a function that takes the actor and returns the roles?

Eduardo B. Alexandre:

Sure, I thought about passing a module just to try to make less verbose, but I guess something like this:

role :user, field: &get_role_field/1 do

Is fine too.

What would be a good key name for it? Or should it not even be a keyword and just the function directly like this:

role :user, &get_role_field/1 do

barnabasj:

could be just a function if it is an argument i think. Could you pleaseopen an issue on the repo. I think that is the better place for this discussion

barnabasj:

we probably also need a way to configure this globaly if the actor looks different in general, having to specify it on every resource would become tedious very quickly

Eduardo B. Alexandre:

So, another unrelated question about the lib (not sure if I should ask that here or in the repo since that is not a PR). Does AshRbac works alongside a normal policies block?

For example, let’s say I have this policies:

  policies do
    policy action(:read) do
      forbid_if actor_attribute_equals(:confirmed?, false)
      authorize_if always()
    end

    policy action(:read) do
      authorize_if expr(:user in ^actor(:roles))
    end
  end

I was under the impression that this would be equivalent to this:

  rbac do
    role :user do
      actions [:read]
    end
  end

  policies do
    policy action(:read) do
      forbid_if actor_attribute_equals(:confirmed?, false)
      authorize_if always()
    end
  end

But from what I can tell from my testings the rbac rule is simply ignored. Is that not supported right now?

barnabasj:

As of now it is creating policies like this

# this 

rbac do
  role :user do
    actions [:read]
  end
end

# ends up being this

policies do
  policy action(:read), has_role(:user) do
    authorize_if always()
  end
end

As of now I’m not completely sure what the best way is to make this composable. I think allowing custom conditions shouldn’t be to hard and might solve this problem.

  rbac do
    role :user do
      actions [:read]
    end
  end

  policies do
    policy action(:read) do
      forbid_if actor_attribute_equals(:confirmed?, false)
      authorize_if always()
    end
  end

In your example your read check would forbid it if the user was not confirmed but otherwise would authrize it. the block created from rbac would not lead to a definitive answer, therefore the authorization from your policy would allow the query to go through

The default for the rbac extension is to deny acccess and explicilty set conditions where an action is allowed. so to make it play well together you need to either add policies that deny certain stuff or use the same condition as the rbac role creates.

Eduardo B. Alexandre:

By custom conditions you mean add the conditions directly to rbac code block? Would that mean that we would be able to add custom conditions inside the role or actions blocks?

I guess that would alleviate the problem, but seems like something that would create a lot of code duplication if I have to set the same rules for all roles (like my confirmed example since that applies to all roles).

<@197905764424089601> I’m not familiar with the policies code, but is there any technical issue that makes it hard to be composable (meaning that I can split my policies into multiple policies code blocks inside a resource)?

barnabasj:

you can already add conditions to the actions like this:

rbac do
  role :user do
    actions [
      {:read, accessing_from(OtherResource, :relationship)}
    ]
   end
end

It would be possible to also allow a third element in the tuple with custom checks. e.g.

rbac do
  role :user do
    actions [
      {:read, accessing_from(OtherResource, :relationship), [forbid_if: [actor_attribute_equals(:confirmed?, false)]}
    ]
   end
end

which would result in

policies do
  policy has_role(:user), action(:read), accesing_from(OtherResource, :relationship) do
    forbid_if actor_attribute_equals(:confirmed?, false)
    authorize_if always()
  end
end

barnabasj:

Maybe composable was not the right word. Policies follow certain rules, if you follow them you can put your policies into different blocks. If I’m not mistaken:

all policy block with a matching conditions are executed.

a block executes all checks until an answer is found (:authorized | :forbidden) or all checks are done (:unknown)

afterwards the result of policy blocks is checked

if any returned :forbidden the request is stopped if all are :unknown the request is also stopped if one of them :authorized and the rest is :unknown the request is processed

barnabasj:

the extension relies on the default being forbidden and explicitly authorizes if a condition matches.

if you add another policies like your read policy, that policy essentially bypasses the extensions policies because their policy conditions are not matching.

Now that I think about, you could possible already pass your actor_attribute_equals(:confirmed?, false} as condition.

rbac do
  roles :user do
    actions [
      {:read, actor_attribute_equals(:confirmed?, :true)}
     ]
  end
end

That way the :user would only be allowed to execute the read actor is confirmed

policies do
  policy has_role(:user), action(:read), actor_attribute_equals(:confirmed?, :true) do
    authorize_if always()
  end
end

barnabasj:

I have to ask chatgpt to summarize this thread and create docs from it afterwards 🤣

barnabasj:

In this case I think I probably only have to allow an array of conditions. I think this is more in line with allow list philosophy. That way you can define explicit conditions for when a user should be allowed to do something.

zachdaniel:

Policies compose just fine, but it’s important to know the main things: policies apply in order (important to account for bypasses), and all policies that apply to a request must pass .

zachdaniel:

(Except for bypasses)

zachdaniel:

So the essentially question is where does rbac put the policies it adds to your resource.

zachdaniel:

As long as it puts them at the end, then there should be no issue 🙂

zachdaniel:

And all dsl entities from fragments also get put at the end, so if you have a fragment for policies you should put rbac and your policies in the same fragment.

barnabasj:

It prepends bypasses and appends policies.

barnabasj:

But mixing allow-list policies together with deny-list style policies leads to unwanted results

zachdaniel:

Hm…it shouldn’t

zachdaniel:

Every policy has to pass always no matter what

zachdaniel:

So why does that lead to unwanted results?

barnabasj:

Ok, in the example above the policy with the forbid_if also has an authorize_if always in there. That one overrides all the other allow-list style policies. But it’s only the authorize if in there that is the problem

zachdaniel:

that should not override any other policies

zachdaniel:

no policy will ever override another policy

zachdaniel:

the only thing capable of doing that is a bypass higher up in the policy list

barnabasj:


policies do
  policy action(:read) do
    forbid_if some_check
    authorize_if always
   end

   policy has_role(:user), action(:read) do
     authorize_if always()
   end
end

in this example the second one is never looked at if the user has not the role :user and the first policy doesn’t care about the role and only forbids in case of some_check

therefore the request is authorized no matter the role of the user

zachdaniel:

if that’s actually happening then it is a massive bug 🙂

zachdaniel:

All policies that apply have to result in an authorized status

barnabasj:

no the logic is sound

barnabasj:

the second policy just isn’t applied because of the condition

zachdaniel:

you said “therefore the request is authorized no matter the role of the user”

zachdaniel:

that is not what those policies say

barnabasj:

yes because the first policy returns :authorized and the second one does not apply

zachdaniel:

if has_role(:user) is true, the second policy must pass

zachdaniel:

oh

zachdaniel:

well if its just authorize_if always() then yes, you’re right

zachdaniel:

sorry

zachdaniel:

a policy authorize_if always() has no way of failing

zachdaniel:

so is effectively a noop if it applies or not

zachdaniel:

The reason its probably working when you only use the rbac extension is because we require that at least one policy applies

barnabasj:

if no policy applies it is denied right

zachdaniel:

Gotcha, so I think what your rbac extension needs to do is

policy action(:read) do
  authorize_if has_role(:user)
end

barnabasj:

I can not recall at the moment what it was, but I did this at first and had similar problems too.

zachdaniel:

gotcha. Well in the short term the library should probably express that it can’t actually be mixed w/ your own policies on any action w/ rbac applied currently

barnabasj:

you can, but can only add other allow-list style policies

barnabasj:

but yeah, that should be in the docs

zachdaniel:

it doesn’t matter what kind of policies they are

zachdaniel:

if any other policy applies, then they’ve gotten past the “at least one policy must apply” rule

zachdaniel:

a policy that contains only authorize_if always() can never forbid a request

zachdaniel:

So it actually only works if you have no other policies that apply

barnabasj:

yes, but if your policy is explicitly allowing something than that is what you want and it is ok if it applies

zachdaniel:

sorry, but I don’t think so. Let me show you what I mean

zachdaniel:

policy action(:create) do
  authorize_if actor_attribute_equals(:can_create, true)
  forbid_if always()
end

rbac_policy_about_create

zachdaniel:

if that first policy applies, it will override the rbac policy

zachdaniel:

which means you can really easily accidentally poke holes in your authorization

barnabasj:

ok, i got what you mean. What i was thinking about was something with a condition more specific than the one from the extension

zachdaniel:

Yeah, if you are like narrowing it down then it works, but that’s only kind of by chance

zachdaniel:

policy has_role(:foo) do
   authorize_if actor_attribute_equals(:foo_is_active, true)
   forbid_if always()
end

zachdaniel:

i.e if you only want users w/ a given role to be able to work against foo_is_active , you’re actually making any rbac policies about that role ignored

zachdaniel:

based on the rbac stuff, I think what you actually want is for the role to be the condition, and the allowed actions to be the checks

zachdaniel:

i.e

policy has_role(:foo) do
  authorize_if action(:read)
end

barnabasj:

how would I add an filter to this?

zachdaniel:

what kind of filter?

barnabasj:

expr(id == ^actor(:id)) for example

zachdaniel:

is that something the rbac does?

barnabasj:

not at the moment

barnabasj:

I just think filters were where I had my first problem and i started moving it over to conditions

zachdaniel:

if you do

policy has_role(:foo) do
  authorize_if action(:read)
end

policy [has_role(:foo), action(:read)] do
  authorize_if expr(id == ^actor)
end

zachdaniel:

Those two policies would result in :foo role being able to call the read action, and automatically applying the expr(id == ^actor) filter.

zachdaniel:

note: If building them in a transformer you may need to reference the built in Expr check module

barnabasj:

but if you leave of the has_role(:foo) from the second policy you would ave the same problem as before, right?

zachdaniel:

🤔 sorry, what exactly should the behavior be?

zachdaniel:

should all users have the filter applied? or only has_role(:foo) users?

barnabasj:

only has_role

barnabasj:

this also leads to narrowing

zachdaniel:

I don’t see how

zachdaniel:

The first policy says “any user w/ has_role(:foo) can only call the read action”

barnabasj:

policy has_role(:foo) do
  authorize_if action(:read)
end

# narrower condition
policy [has_role(:foo), action(:read)] do
  authorize_if expr(id == ^actor)
end

zachdaniel:

The second policy says “any user w/ `has_role(:foo), calling the read action, can only read themselves”

barnabasj:

I’m just saying that there is no way around narrowing the condition

zachdaniel:

Well, that’s just because its the example you gave

zachdaniel:

are you talking about the rbac extension adding this filter?

zachdaniel:

or you as a user of the rbac extension adding the filter?

barnabasj:

if the rbac extensio would do this

policy has_role(:foo) do
  authorize_if action(:read)
end

you as a user would still need to narrow the condition to add the filter

zachdaniel:

Not necessarily

zachdaniel:

if you as a user just did this:

policy action(:read) do
  authorize_if expr(id == ^actor(:id))
end

zachdaniel:

then both policies would apply

barnabasj:

only if the user has_role(:foo) right?

zachdaniel:

Nope

zachdaniel:

well

zachdaniel:

the first one applies if the user has_role(:foo) , yes

zachdaniel:

the second one applies if action(:read)

zachdaniel:

oh, lol

zachdaniel:

I see what you’re saying

barnabasj:

the circle closes xD

zachdaniel:

you’re right it has to be

policy action(:read) do
  authorize_if has_role(:foo)
end

zachdaniel:

So what you’d need to do to do this right is group all roles for each action

barnabasj:

but the filter would still authorize? or is filter == :unkown

zachdaniel:

the filter authorizes, but applies the filter

barnabasj:

yes but it would apply if no role matches also

zachdaniel:

So what you’d do is, for each action, list all roles that can do it

barnabasj:

and add forbid_if always at the end

zachdaniel:

policy action(:read) do
  authorize_if has_role(:foo)
  authorize_if has_role(:bar)
end

zachdaniel:

you don’t technically need forbid_if always() at the end

zachdaniel:

its implied

barnabasj:

but the filter would still authorize if none of the roles match

zachdaniel:

Nah, this time for real lol

zachdaniel:

# applied by rbac extension
policy action(:read) do
  authorize_if has_role(:foo)
  authorize_if has_role(:bar)
end

# added by user
policy action(:read) do
  authorize_if expr(id == ^actor(:id))
end

zachdaniel:

if action(:read) then both apply this time

zachdaniel:

so if you don’t have an allowed role, the policy is forbidden

zachdaniel:

and the filter will be applied

barnabasj:

yes, but if the actor acutally has_role(:baz)

zachdaniel:

then its forbidden

barnabasj:

why?

zachdaniel:

because the first policy applies (because action(:read) )

zachdaniel:

and you don’t have one of those roles

barnabasj:

ok, now I think i understand where my problem in understanding was. I thought if there was no matching check it would result in :unknown and the other policies would either :authorize or :forbid and if all policies returned :unkown it would default to :forbidden

zachdaniel:

nope, every policy that applies must always result in :authorized 👍

barnabasj:

Thanks for clearing things up for me. gotta fix this tomorrow

.myrmyr:

Did you do any performance tests? I’ve started using this and sadly found out that somehow it made app really unresponsive and pinned my CPU to 100% and while in normal conditions LiveView took max of 1 second to send a response, now it took 35 seconds. I’ve started :observer to check what’s happening and this is what I’ve seen(screenshot attached)

I’ve added

  rbac do
    role "public" do
      fields([:*])
      actions([:read])
    end
  end

to like 6 resources, which are in a relationship. Sadly I don’t have time to debug or test it further but will try if I will have some free-ish time.

zachdaniel:

🤔

zachdaniel:

Something seems strange there

zachdaniel:

well, not “seems”, something is very strange there. It probably isn’t <@360450161845075968>’s fault, but something in core doing something strange.

zachdaniel:

Can you show me the result of Ash.Policy.Info.policies(Resource) of the resource w/ those roles? <@285510586618347521>

barnabasj:

I also felt, that since I started using field policies my tests were executing slower. But I haven’t investigated yet.

zachdaniel:

Interesting

zachdaniel:

It might actually be the way they are modeled with conditions, now that I think about it

zachdaniel:

but with a single one that shouldn’t be enough to cause problems…

barnabasj:

I just published 0.2.0 with the refactor

zachdaniel:

oh, nice 🙂 Maybe you can try that out <@285510586618347521>

barnabasj:

there are only conditions in certain cases. But I didn’t feel like it was faster afterwards.

only if you use the condition for a relationship for example

zachdaniel:

okay, so probably won’t help then. How strange

zachdaniel:

I mean, I don’t see what would cause the issue from a policies perspective, since its basically just generating the policies you would have hand written before

barnabasj:

have you benchmarked field_policies in general?

zachdaniel:

um…no 🙂

zachdaniel:

So maybe its actually just field policies

.myrmyr:

FYI I’m working on providing Ash.Policy.Info.policies but have to bleep some details so might take a while

zachdaniel:

I’d bet it is

zachdaniel:

here is a way to test this out

zachdaniel:

<@360450161845075968> can you make it so that [:*] doesn’t add any field policies?

zachdaniel:

or what does it do for :* currently?

.myrmyr:

I feel like this extension adding a lot of field policies, probably one for every field, is triggering this behavior

barnabasj:

what i”m doing is going over every field and generate one policy each, I think I saw that you were doing that for :* somewhere anyway.. roles with the :* field are added to every policiy as authorize_if check.

zachdaniel:

Well, what we do for :* is generate one field policy for all of those fields

zachdaniel:

since you can do field_policy [:list, :of, :fields]

zachdaniel:

even still, the idea that it could take 35 seconds is pretty wild

barnabasj:

ok, let me try and combine those then.

zachdaniel:

but it would be good to confirm in some way that the cause is field policies, and then I can benchmark them in core and figure out whats going on

.myrmyr:

policies: https://pastebin.com/xiP1xcCK field_policies: https://pastebin.com/NmjhGGca

zachdaniel:

you can also have multiple field policies for a given field, and those can have a condition. So what I might suggest is doing this:

for {role, fields} <- roles do
  # you'd have to use transforms for this of course
  field_policy fields, [has_role(role)] do
    authorize_if always()
  end
end

barnabasj:

12 field policies if I counted correctly

zachdaniel:

no, wait

zachdaniel:

for {role, fields} <- roles do
  # you'd have to use transforms for this of course
  field_policy fields do
    authorize_if has_role(role)
  end
end

.myrmyr:

35 seconds happens only if I use this extension in like 7 resources that are in a relationship. If I only add this to the one I’ve printed policies for above it takes a bit longer(like 2-3 seconds).

zachdaniel:

ah, okay. Even 2-3 seconds is pretty wildly long but that makes sense

barnabasj:

I’ll try and combine the field policies into less policies and see if that helps

zachdaniel:

both of my examples above were wrong 😆

barnabasj:

in what way? I’m currenly looking at my role check and see if it can be done in a more performant way. Right now the check is probably not the fastest and because it is executed a lot it might also be the problem.

barnabasj:

I mean I didn’t see numbers as high as yours but the implementation is definetively more on the it’s nice to read, then on the performance side of things

barnabasj:

Operating System: Linux
CPU Information: AMD Ryzen 9 5950X 16-Core Processor
Number of Available Cores: 32
Available memory: 62.70 GB
Elixir 1.14.3
Erlang 25.2.1

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 5 s
memory time: 0 ns
reduction time: 0 ns
parallel: 1
inputs: none specified
Estimated total run time: 28 s

Benchmarking role_check_atom ...
Benchmarking role_check_string ...
Benchmarking roles_check_atom ...
Benchmarking roles_check_string ...

Name                         ips        average  deviation         median         99th %
roles_check_atom          2.80 M      356.52 ns  ±6899.27%         300 ns         610 ns
role_check_string         2.70 M      369.74 ns  ±9181.27%         310 ns         610 ns
role_check_atom           2.70 M      370.80 ns  ±8415.90%         310 ns         630 ns
roles_check_string        2.58 M      388.06 ns  ±7806.80%         330 ns         640 ns

Comparison: 
roles_check_atom          2.80 M
role_check_string         2.70 M - 1.04x slower +13.22 ns
role_check_atom           2.70 M - 1.04x slower +14.27 ns
roles_check_string        2.58 M - 1.09x slower +31.54 ns

barnabasj:

without changes:

alias AshRbacTest.{Api, ChildResource, RootResource, SharedResource}


root_resource = Api.create!(Ash.Changeset.for_create(RootResource, :create))
shared_resource = Api.create!(Ash.Changeset.for_create(SharedResource, :create))

child_resource =
      Api.create!(Ash.Changeset.for_create(ChildResource, :create, %{root_id: root_resource.id}))

Benchee.run(%{
  "read" => fn ->
             RootResource
             |> Ash.Query.select([:id])
             |> Ash.Query.load([:child, :number, :children])
             |> Api.read(actor: %{roles: [:user]})
  end
})

this is the result:

Operating System: Linux
CPU Information: AMD Ryzen 9 5950X 16-Core Processor
Number of Available Cores: 32
Available memory: 62.70 GB
Elixir 1.14.3
Erlang 25.2.1

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 5 s
memory time: 0 ns
reduction time: 0 ns
parallel: 1
inputs: none specified
Estimated total run time: 7 s

Benchmarking read ...

Name           ips        average  deviation         median         99th %
read        210.20        4.76 ms     ±6.22%        4.65 ms        5.63 ms

.myrmyr:

If I find a bit of time, but sadly that probably will take a while, I will try to recreate this issue in a separate app.

barnabasj:

Thanks, I’ll try and optimize as best I can in the meantime.

zachdaniel:

interesting

zachdaniel:

So your tests show that its quite fast

zachdaniel:

We may need to wait for more instances of this to show up, or a usable reproduction in that case 😦

barnabasj:

I created a pr that should result in a lot less policies, maybe you can try it out and it helps already https://github.com/traveltechdeluxe/ash-rbac/pull/11

Eduardo B. Alexandre:

It took me some time, but I finally was able to reserve some time to finish my PR for adding support for other roles fields. there is the PR <@360450161845075968> https://github.com/traveltechdeluxe/ash-rbac/pull/15

The only thing that I believe is missing there is adding support for bypass .

barnabasj:

I just released a new version with your changes

Eduardo B. Alexandre:

Hey <@360450161845075968> I noticed that when I use ash_rbac, id fields from relationships return ForbiddenField. For example, if resource A has a relationship with resource B, b_id attribute will be returned with ForbiddenField even if I use fields [:*] .

Removing the rbac rules and using this one instead works fine:

  field_policies do
    field_policy :* do
      authorize_if always()
    end
  end

Is this a bug that should be reported or an I just doing something wrong?

Looking at the policy generated by rbac, seems like it filters all the :*_id attributes when generating the policy where field_policy :* doesn’t

barnabasj:

Sounds like a bug to me, I’ll take a look today

barnabasj:

I publish a new version that I think should fix it, if not I have to dig a little deeper.

Eduardo B. Alexandre:

Yep, that fixed it, thanks a lot!

{@thread.name}