[Issue-24] - Add a handler for resources not found #29
Conversation
… used, if the can? function returns nil
…ning the not found handler.
pedromlcosta
changed the title
| opts = unauthorized_handler.init(opts) | ||
| unauthorized_handler.call(conn, opts) | ||
|
|
||
| nil -> |
There was a problem hiding this comment.
How would you feel about explicitly returning {:error, :not_found} instead of nil? Feels more explicit to me. cc @naps62 - any particular thoughts?
There was a problem hiding this comment.
this is the return value of :can?/3. That function has a boolean response (enforced by the question mark, expanding that API would break the semantics)
I didn't notice at first that this is what the PR was doing: allowing :can?/3 to return nil. But that seems like a no-go to me
It also makes me wonder that this might be outside of the scope of dictator?
we could maybe allow an optional exists? callback to the policy, to be called before can?. But it would have to be opt-in, and I'm not sure how to handle that correctly
There was a problem hiding this comment.
My original issue with verifying this outside Dictator and before the Dictator plug was called, was information could be leaked about whether or not an object exists to someone who we didn't check yet if they have the authorization to access that information.
But thinking a bit about what you're saying, maybe the approach was not the most correct one, since indeed it seems to fall out of the current responsibilities of the can? function. As for the scope of dictator, an optional exists? to me would look ok, just because of the reason above about leaking information when verifying authorizations, however, depending on the implementation we could incur on the same problem...
| conn | ||
| |> send_resp(:not_found, "Object not found") | ||
| |> halt() | ||
| end |
There was a problem hiding this comment.
expanding on my comment above, I'm not comfortable with the default response of an authorization library being a 404.
At most, this should be opt-in. Default behaviour should be the standard Unauthorized. Otherwise you risk leaking information to unauthorized users (i.e.: allowing the user to see that a certain object does not exist, even though he wouldn't have access to it in the first place)
** What's new **
Issue-24 - Implemented a "not found" handler, with the possibility of the user configuring a custom not found handler. Closes #24 .
Previously, a policy only allowed a boolean response. The not found handler adds the possibility of returning nil within the can? function of a policy, and the dep will then call the not found handler instead of the unauthorized handler.
Does not contain breaking changes.