jfrolich / authorize

Rule based authorization for Elixir
99 stars 4 forks source link

custom authorization code for reference #7

Closed goravbhootra closed 6 years ago

goravbhootra commented 6 years ago

Hi @jfrolich

I wrote custom authorization code for my current project. Posting it here for ideas:

defmodule AppWeb.Authorize do
  import Phoenix.Controller
  import Ecto.Query

  alias App.Accounts.User
  alias App.Members.Nomination
  alias App.Settings
  # alias App.Settings.Batch

  def auth_action(conn, module) do
    conn
    |> get_action()
    |> authorize(get_current_user(conn))
    |> check_auth_success(conn, module)
  end

  # defp check_auth_success(%Ecto.Query{} = query, conn, module) do
  defp check_auth_success(%{} = query, conn, module),
    do: apply(module, action_name(conn), [conn, conn.params, query])

  defp check_auth_success(true, conn, module),
    do: apply(module, action_name(conn), [conn, conn.params])

  defp check_auth_success(_, _, _), do: {:error, :unauthorized}

  defp authorize("NominationController_index", user),
    do: from(n in Nomination, where: n.nominated_by_id == ^user.id)

  defp authorize("NominationController_new", user), do: %Nomination{nominated_by_id: user.id}

  defp authorize("NominationController_create", user),
    do: %{"nominated_by_id" => user.id, "status" => 1}

  defp authorize("NominationController_show", user),
    do: from(n in Nomination, where: n.nominated_by_id == ^user.id)

  defp authorize("NominationController_edit", user),
    do: from(n in Nomination, where: n.nominated_by_id == ^user.id)

  defp authorize("NominationController_update", user),
    do: from(n in Nomination, where: n.nominated_by_id == ^user.id)

  defp authorize("BatchController_index", _user), do: Settings.active_batches_query()

  defp authorize(action, %User{roles: %{"admin" => true}}), do: authorize(action)
  defp authorize(_, _), do: :unauthorized

  # Authorise all actions for admin
  defp authorize(_), do: true

  defp get_action(conn) do
    Enum.join(
      String.split(
        "#{conn.private[:phoenix_controller]}_#{conn.private[:phoenix_action]}",
        "Elixir.AppWeb."
      ),
      ""
    )
  end

  defp get_current_user(%Plug.Conn{assigns: %{current_user: current_user}}) do
    current_user
  end
end

Basically, I am returning scoped queries based on user privileges, that can be merged in controller so that authorization logic is in one place. It also helps re-usability of the controllers methods for different use cases eg. for users with different access levels.

In the above example, I am returning true for all admin actions as admin section (controllers) are different but they could very well return queries without any scopes.

I recently read about dynamic queries. Will try to implement that to clean up the logic further. Still not got time to get to it.

Any suggestions for improvement or discussion regarding above is welcome.

jfrolich commented 6 years ago

Yes I think this is a reasonable approach, as long as the authorization rules are simple. When your functions become longer the Authorize DSL might make the code more readable.

jfrolich commented 6 years ago

The thing I would not do is link authorization directly to controllers, but to keep it within your business logic (context), and just call the context from the controller. Like Posts.update(id, params, user), and then handling authorization in the Posts module.

goravbhootra commented 6 years ago

@jfrolich understand that approach - it has its own advantages.

I like to avoid spreading authorization logic throughout the app. Anyway, created this just for sharing the idea. Pls feel free to close this.

jfrolich commented 6 years ago

@goravbhootra: It's still possible to combine them in one module and not directly tie them to a controller. Hmm I actually like that. Perhaps an api could be:

Defmodule Authorization
  authorize Post do
    ...rules 
  end
  authorize Comment do
    ...rules
  end
end

Authorization.authorize(%Post{}, user, :read)
Authorization.authorize(%Comment{}, user, :update)
goravbhootra commented 6 years ago

This looks good.

The idea is to leverage on composable and dynamic queries so that when the query actually hits the DB, it scopes out what the user is not allowed to access and fetches the minimal data required.

There will be two kinds of access:

  1. where the data exists but the specific user does not have privileges, we return 404 instead of 403 - its as if the resource does not exist.

  2. Another use case will be whether an end-point can be accessed or not eg. whether user is allowed to create a new resource or dashboard access. If we dont return true, response will be 403.

The advantage of going thru the controllers is that you can still do things without user context, if you need to as well as testing remains sane.