-
Notifications
You must be signed in to change notification settings - Fork 86
Update guardian #1248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update guardian #1248
Conversation
1ea6f3a
to
8e0de18
Compare
error_handler: CodeCorps.Auth.ErrorHandler | ||
|
||
plug Guardian.Plug.VerifyHeader, realm: "Bearer" | ||
plug Guardian.Plug.LoadResource, allow_blank: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This didn't need to be allow_blank: true
before but appears to need to be now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is accurate and will need to remain this way.
@@ -0,0 +1,11 @@ | |||
defmodule CodeCorps.Auth.ErrorHandler do | |||
use CodeCorpsWeb, :controller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appeared to be needed even though this isn't a controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure of a better way to handle this.
# send_resp(conn, 401, body) |> IO.inspect() | ||
conn | ||
|> put_status(401) | ||
|> render(CodeCorpsWeb.TokenView, "401.json", message: to_string(type)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is the message we want.
lib/code_corps/guardian.ex
Outdated
end | ||
|
||
def resource_from_claims(%{"sub" => sub}), do: resource_from_sub(sub) | ||
def resource_from_claims(_), do: {:error, :unknown_resource_type} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this can happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this can happen, but the :error
reason is probably not accurate. Maybe :missing_resource
?
{:ok, auth_token, _claims} = updated_user |> Guardian.encode_and_sign(:token) | ||
{:ok, %User{} = updated_user} <- user |> User.reset_password_changeset(params) |> Repo.update(), | ||
{:ok, _auth_token} <- auth_token |> Repo.delete(), | ||
{:ok, auth_token, _claims} = updated_user |> CodeCorps.Guardian.encode_and_sign() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to have changed in ways I don't understand (i.e. not sure why we don't need :token
as a param now).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it just changed quite a bit. This looks like it is the correct implementation now.
else | ||
{:error, %Changeset{} = changeset} -> conn |> put_status(422) |> render(CodeCorpsWeb.ErrorView, :errors, data: changeset) | ||
{:error, _} -> conn |> put_status(:not_found) |> render(CodeCorpsWeb.ErrorView, "404.json") | ||
nil -> conn |> put_status(:not_found) |> render(CodeCorpsWeb.ErrorView, "404.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Letting FallbackController work.
8e0de18
to
bebe44d
Compare
bebe44d
to
d2a42d0
Compare
Going to merge since all seems working and seems not so critical. Happy to have follow-up issues based on anything that seems not quite great. |
What's in this PR?
WIP. Haven't been able to figure it all out yet.