Plans for V3

[schrockwell]

I'd like to explore some options for how Bodyguard can evolve in the next major revision. I think there is still a place in the ecosystem for a standardized way to perform authorization, but surely it doesn't need to be as complicated and opinionated as how Bodyguard currently works.

When I peruse the codebase and the docs, I'm struck by a few negative aspects:

• Boy, that readme is long and dense. Why does this need to be so complicated?
• The ratio of configuration to code is too high, e.g. the Authorize plug
• There shouldn't be any dependencies on Plug, etc. for the core functionality
• Schema scopes are misguided, beyond the scope (ha!) of this project, and should go away
• The indirection between the permit functions and the authorize callbacks seems like an unnecessary impedance mismatch
• There are no affordances for LiveView

Are there any other problems or rough edges that people see?

Here are a few options for how V3 (or a new library??) could function.

Option A - Very simple core, opt-in extensions

In this model, there would be a core bodyguard library with no dependencies, which basically defines a behaviour and maybe has some very simple wrapper or handler functions.

Additional functionality could be opted in using other libraries like bodyguard_plug, bodyguard_liveview, bodyguard_phoenix, bodyguard_absinthe, etc etc

Option B - Very simple core with generators

In this version, there would still be a basic Bodyguard.Policy behaviour to hand your authorization hat on, but all integrations INTO the policies would be created by mix generators.

This solves a lot of my complexity gripes, while still pushing developers down a common road.

Option C - All-in on generators

Why do we need a behaviour at all? It's all just functions, man!

This has the benefit of being the absolute most flexible and future-proof implementation for nearly all use cases, but at the risk of losing sight of what value this library even provides at all.

Thoughts? Concerns? Ideas? Let's chat.

[benkimpel]

This is a great setup for a discussion! ❤️

[benkimpel]

I don't know if it's a rough edge per se, but here's a specific requirement that we had to do a little work to solve. (Unless we missed something obvious!)

We wanted to make sure that authorize/3 wasn't raising any FunctionClauseErrors at runtime and that any unexpected/unspecified patterns would return false|:error. Having a default deny meant that we also needed some insight into when the authorize/3 pattern matching "missed". For some environments we wanted it to raise and others we wanted it to log.

Here's the way it came together for us...

# my_app/policy.ex
defmodule MyApp.Policy do
  # This is a base policy of sorts
  @behaviour Bodyguard.Policy

  # Set behaviour and generate "catch-all" authorize
  defmacro __using__(_opts) do
    quote do
      @behaviour Bodyguard.Policy
      @before_compile MyApp.Policy
    end
  end

  # Add catch-all
  defmacro __before_compile__(_env) do
    quote do
      # Define a catch-all authorize *at the end* of the module. This will deny and
      # handle the missed pattern match according to application config.
      def authorize(action, user, params) do
        MyApp.Policy.authorize(action, user, params, __MODULE__)
      end
    end
  end

  # Handle per config and deny
  def authorize(action, user, params, module \\ __MODULE__) do
    config = Application.get_env(:my_app, __MODULE__, [])
    message = "Unmatched authorize call for #{inspect([module, action, user, params])}"

    if Keyword.get(config, :unmatched_authorize) == :raise,
      do: raise(UnmatchedAuthorizeError, message: message),
      else: Logger.warn(message)

    :error
  end
end

# my_app/foos.ex
defmodule MyApp.Foos do
  defdelegate authorize(action, user, params), to: MyApp.Foos.Policy

  # regular context stuff ...
end

# my_app/foos/policy.ex
defmodule MyApp.Foos.Policy do
  use MyApp.Policy

  # regular policy stuff ...

  # => catch-all generated here with before_compile
end

[benkimpel]

Note that this is not necessarily a suggestion that this be integrated into Bodyguard, but rather that it's something that we found we needed and didn't have an obvious solution at hand.

[schrockwell]

The __before_compile__ catch-all clause is a great little trick. In theory, there could be a way to configure Bodyguard to hook into authorization success or failures, but I do agree that's pretty application-specific.

[benkimpel]

Boy, that readme is long and dense. Why does this need to be so complicated?

😄

The ratio of configuration to code is too high, e.g. the Authorize plug

👍

There shouldn't be any dependencies on Plug, etc. for the core functionality

👍

Schema scopes are misguided, beyond the scope (ha!) of this project, and should go away

👍 We do use them, but I agree.

The indirection between the permit functions and the authorize callbacks seems like an unnecessary impedance mismatch

👍

There are no affordances for LiveView

I'm interested in what you have in mind for this. Hooks to move authorization out of the main handle_* bodies?

Options A and B seem best to me. I think a big part of the value of the library is "this is the way".

[schrockwell]

More and more, I'm leaning towards exploring B. Generators seem to be the right balance of providing guidance with reasonable defaults, but are also completely optional and fully customizable. And from a library maintenance perspective, the prospect of keeping up with multiple plugin repos for different deps is not really appealing.

[schrockwell]

I've been doing some soul-searching on the type spec for new callback as well. Here's where I've landed so far

defmodule Bodyguard.Policy do
  @callback permit?(user :: term, action :: atom | String.t(), context :: map) :: boolean
end

Argument ordering

With the action argument being first, it makes it easy to read and scan through a big module of callbacks.

But having user first can have nicer calling ergonomics, e.g.

current_user
|> MyPolicy.permit?(:edit, %{post: post})
|> case do
  # ...
end

and it sort of reads nicer, like "permit user X to perform action Y on thing Z".

Rethinking params as context

I like how Absinthe has a big context map, and I think that Bodyguard should push in that direction. I.e. instead of having post as the third arg, it's %{post: post}.

Bools only

The callbacks should only return boolean values. The responsibility of how success/failure is raised should be lofted to the caller. I also want to do away with the Bodyguard wrapper functions, and instead push folks towards just calling into their policy modules directly.

[schrockwell]

@benkimpel Here is something I was playing around with for v3 which I think would solve your logging/raising issue AND provide general functionality.

The idea is to be able to define a "fallback" policy if none of the clauses in the current policy match. So, that would enable you to defer to a general module with your special case.

defmodule MyApp.ThingPolicy do
  use Bodyguard.Policy, fallback_to: MyApp.FallbackPolicy

  def permit?(user, :view, %{post: post}), do: true
  # ... etc ...
end

defmodule MyApp.FallbackPolicy do
  @behaviour Bodyguard.Policy
  
  def permit?(user, action, context) do
    # do your fancy stuff here
    false
  end

or you could just do

use Bodyguard.Policy, fallback_to: false

to deny any unmatched clauses by default.

[benkimpel]

Sorry for the delayed response. I love it! ❤️

[benkimpel]

That would definitely address our fallback use case

[dagumak]

Please go with C (or B)!

For context, I'm new to Elixir/Phoenix coming from a JavaScript and Rails world, and I really dislike some of the magic that the libraries have. The main problem I have with the ecosystem in Elixir is that there isn't enough documentation for many libraries. However, if it's all generated then I can just play around with it, debug, and learn it more quickly.

I would like to share my experience with Pow and phx.auth.gen. They are both authentication and I would say Pow is similar to option A while phx.auth.gen is option C. I really like how simple and clear phx.auth.gen is and it is incredibly easy to work around. For Pow, I spent too much time trying to figure out how to leverage the existing library functions and a lot of the documentation was either outdated or scattered between Elixir Forum and github. It required too much library knowledge for me to be productive and I was really frustrated with how enigmatic it was.

[schrockwell]

Thank you for this feedback - this is how I felt reading my own readme. I am actively exploring both of these options.