Hi,
I’ve written a plug that checks if the user has privileges. If it doesn’t it returns:
defp allow_superusers(_, conn) do
conn
|> put_status(403)
end
Is there any way phoenix can catch those status and render a 403.html or a
text? Is a good idea to use the render view inside the plug?
I’ve also tried:
defp allow_superusers(_, conn) do
conn
|> send_resp(403, "Not allowed")
|> halt
end
And my final choice was using custom errors so I created a custom exception and the function was like:
defp allow_superusers(_, _conn) do
raise App.UnauthorizedError
end
where the exception is deffined like:
defmodule App.UnauthorizedError do
defexception message: "Unauthorized", accepts: [], plug_status: 403
end
But I have some concerns. Reading the docs and some books (specially Programing Elixir 1.2), It’s said that in elixir exceptions should be used for things that should never happen, so just for very exceptional cases.
As an example, it says that mix has no exception handlers in the code, and elixir has a total of five. I’ve read some code for Ecto and Phoenix and both define a few exceptions:
I’m totally new to elixir and phoenix, but I think 403 statuses don’t fall under the “exceptional event” that takes a process down like if the database goes down.
Maybe some experience developers could give us some advise here.
def do_unauthorized(conn) do
ControllerCallbacks.unauthorized(conn)
end
Where unauthorized/1 is:
def unauthorized(conn, _params \\ %{}) do
last_path = case conn.method do
"GET" -> true
_ -> false
end |> case do
false -> nil
true -> conn.request_path <> if byte_size(conn.query_string) > 0 do "?" <> conn.query_string else "" end
end
conn
|> put_session(:last_path, last_path)
|> put_flash(:error, gettext("The current account does not have acces to that location, please log in with an account that does or contact an admin to add access if it is required."))
|> redirect(to: auth_path(conn, :ldap_request))
end
It just gets the location of where they were, the query, stories it, and redirects to the auth controller to have them log in with a better account (which immediately clears the stored path from the conn but stores it in another place internally and will redirect to it if they successfully log in with another account, among some other stuff). This unauthorized/1 function could easily just set a 403 status and render a template though.
Is it a good practice to call ControllerCallbacks.unauthorized(conn) from the plug? Or maybe unauthorized/1 could be inside the plug?
This is the plug:
defmodule App.Auth do
import Plug.Conn
def init(opts) do
opts
end
def call(conn, _opts) do
conn.assigns[:current_user]
|> is_admin
|> allow_superusers(conn)
end
defp is_admin(%{is_superuser: true}) do
true
end
defp is_admin(_) do
false
end
defp allow_superusers(true, conn) do
conn
end
defp allow_superusers(_, _conn) do
raise App.UnauthorizedError
# This is an alternative
# conn
# |> put_status(403)
# |> send_resp(403, "Unauthorized")
# |> halt
end
end
I like to have everything that mutates a connection in a controller somewhere, or the helpers for my shared bits, absolutely unnecessary though, just an organizational thing for me.
EDIT: I would not really raise the error there, I have a wrapper around all my leaf-node controllers to catch my custom errors.
So, If I understand correctly, you propose to alter the connection in a controller module calling the function in the plug like:
defp allow_superusers(_, _conn) do
OtherModule.unauthorized(conn)
end
what do you mean with having a wrapper around all the leaf-node controllers? I want to avoid using custom errors, so I really like the idea you proposed of using a function like this:
def unauthorized(conn, _params \\ %{}) do
last_path = case conn.method do
"GET" -> true
_ -> false
end |> case do
false -> nil
true -> conn.request_path <> if byte_size(conn.query_string) > 0 do "?" <> conn.query_string else "" end
end
conn
|> put_session(:last_path, last_path)
|> put_flash(:error, gettext("The current account does not have acces to that location, please log in with an account that does or contact an admin to add access if it is required."))
|> redirect(to: auth_path(conn, :ldap_request))
end
The happy_path!(else: handle_error(conn)) do handles the errors. The lines @perm true = conn |> can?(show(%Perms.CheckInOut{section: true})) and @perm true = conn |> can?(show(%Perms.CheckInOut{section: section.slug})) test permissions and return true if successful, so if that fails then it fails the match to true, and thus gets tagged with a :perm tuple via the @perm tag, which gets tossed into the handle_error(conn) call, which enters the path you saw before. I have very fine-grained permissions that are built up based on data that gets accessed (with a lot of early tests, like here it tests if they have access to ‘anything’ in the section, if true then it gets the section information and tests if they have access to this specific section, if true then it continues). This is why a Plug style permission tests would not even remotely work for me, they are far too coarse-grained, at least without multiple DB lookups, which…why, or stuffing the connassign with lots of stuff?
As an aside: for the benefit of fellow programmers and your future self, you should consider renaming your errors and functions from “Unauthorized” to “Forbidden”, as the former suggests (at least to me) a 401 error rather than 403.
I was the one who suggested @adrian to use custom errors on the #phoenix-talk google group (as a fellow beginner). I personally used the method when implementing parameter validation for my routes (using https://github.com/vic/params, raising InvalidParamsError on not ch.valid?). My justification is that I wanted to treat it as 422, and the Phoenix docs seem to suggest using custom errors for that (as they have been doing with Ecto.NoResultsError and others).
The ParamsValidator.validate/1 function is the one that will raise the error. This is just a small API project and I don’t handle authentication and permission at all. I see @OvermindDL1’s solution as kind of convoluted but maybe it’s because I just haven’t worked on something of that scale yet (or because I haven’t seen the bigger picture ).
I do want feedbacks if there is a more recommended approach than using custom errors, or if there are drawbacks by using it.
True true, unauthorized should be 401 if you are returning codes.[quote=“bobbypriambodo, post:9, topic:2191”]
The ParamsValidator.validate/1 function is the one that will raise the error. This is just a small API project and I don’t handle authentication and permission at all. I see @OvermindDL1’s solution as kind of convoluted but maybe it’s because I just haven’t worked on something of that scale yet (or because I haven’t seen the bigger picture ).
[/quote]
It could very well be convoluted! ^.^
It grew this way because the error handling has become massive and can come from a variety of parts, like on my example above the verify_single_record call will cause a Match Error on the {:ok, section} if there is not a single record (I.E., it is not found), the special error tag it returns it matched for in my error handlers to return an appropriate NotFound code or so:
I have a long list here of various generic things that can be returned from controllers or functions they call. The not found and unprocessable entities, like most, are more generic than my unauthorized (which redirects to let them login better):
def do_not_found(conn) do
conn
|> put_status(:not_found)
|> render(MyServer.ErrorView, :not_found)
end
def do_unprocessable_entity(conn) do
conn
|> put_status(:unprocessable_entity)
|> render(MyServer.ErrorView, :unprocessable_entity)
end
).
