CODEWITHSUNDEEP
elixirphoenix-framework
KA01
KA01

Reputation: 4251

Custom Elixir Plug to prevent users from requesting API resources that aren't theirs

So I'm trying to develop a Plug that prevents users from requesting API resources that aren't theirs. For example, if John tries to update Alex's profile the application throws an unauthorized error because that resource is not his. I am using Guardian for authentication and unless I missed it, the Guardian docs don't provide an API for this. So I figured I create a plug myself.

However, it seems my implementation is rejecting every request that I applied (or rather 'piped') the custom Plug through. See the Plug below:

defmodule ExampleApp.Authorization do
  use ExampleApp.Web, :controller

  def init(opts \\ nil), do: opts

  def call(conn, _opts) do
    case Guardian.Plug.current_resource(conn) do
      nil -> 
        conn
        |> put_status(:unauthorized)
        |> render(ExampleApp.SessionView, "forbidden.json", error: "Not Authorized")
        |> halt
      user ->
        if user.id == conn.params["id"] do
          conn
        else
          conn
          |> put_status(:unauthorized)
          |> render(ExampleApp.SessionView, "forbidden.json", error: "Not Authorized")
          |> halt
        end
    end
  end
end

So I've done some console printing to compare the user.id to conn.params["id"] and the comparisons are true for test cases where the user id is the requested user's id. However the 'else' condition in the if-else statement seems to run. Quite confused.

Here are the two tests that are failing

test "valid UPDATE of user at /users/:id route", %{conn: conn, id: id} do
  conn = put conn, user_path(conn, :update, id), user: %{first_name: "bob"}
  assert json_response(conn, 200)["first_name"] == "bob"
end

test "valid DELETE of user at /users/:id route", %{conn: conn, id: id} do
  conn = delete conn, user_path(conn, :delete, id)
  assert json_response(conn, 200)
  refute Repo.get(User, id)
end

Yet these errors are being thrown for both

** (RuntimeError) expected response with status 200, got: 401, with body:
  {"error":"Not Authorized"}

Based on this error, the Plug from above is running the 'else' condition.

Here is the UserController

defmodule ExampleApp.UserController do
  use ExampleApp.Web, :controller

  plug Guardian.Plug.EnsureAuthenticated, handler: ExampleApp.SessionController
  plug ExampleApp.Authorization when action in [:update, :delete] # Plug only runs for update and delete actions

  alias ExampleApp.{Repo, User, SessionView}

  #....

  def update(conn, %{"id" => id}) do
    body = conn.params["user"]
    changeset =
      User
      |> Repo.get!(id)
      |> User.edit_changeset(body)
    case Repo.update(changeset) do
      {:ok, updated_user} -> 
        conn
        |> put_status(:ok)
        |> render(SessionView, "show.json", %{user: updated_user})
      {:error, _ } -> 
        conn
        |> put_status(:bad_request)
        |> render(SessionView, "error.json", %{error: "User could not be persisted to DB"})
    end
  end

  def delete(conn, %{"id" => id}) do
    user = Repo.get!(User, id)
    case Repo.delete(user) do
      {:ok, _} ->
        conn
        |> put_status(:ok)
        |> render(SessionView, "delete.json")
      {:error, _} ->
        conn
        |> put_status(:bad_request)
        |> render(SessionView, "error.json")
    end
  end

end

So I've come to the conclusion that it has to do with my Plug implementation. I may be wrong but I'd like a second eye to look at my code. Why are the test cases where authenticated and authorized requests getting rejected?

Upvotes: 0

Views: 640

Answers (1)

Dogbert
Dogbert

Reputation: 222198

Unless you changed the default type of id, user.id will be an integer while conn.params["id"] will be a string, so you should compare the string representation of id:

if Integer.to_string(user.id) == conn.params["id"] do
  conn
else
  ...
end

So I've done some console printing to compare the user.id to conn.params["id"] and the comparisons are true for test cases where the user id is the requested user's id.

You probably used IO.puts, which would print the same thing for both "123" and 123. It's almost always better to default to IO.inspect when printing stuff for debugging.

Upvotes: 1

Related Questions