after_encode_and_sign result not used
jon-mcclung-fortyau opened this issue · 3 comments
Steps to Reproduce
I have updated the after_encode_and_sign
result to conditionally return a different token from the one passed to me. However, this token is not used at all by the code.
Expected Result
I expected that by returning {:ok, different_token}
, Guardian.encode_and_sign
would return {:ok, different_token, claims}
Actual Result
It returns {:ok, original_token, claims}
.
You can see the problem code here. By using the underscore instead of token
, the value of after_encode_and_sign
is ignored except to check success/failure.
@spec encode_and_sign(module, any, Guardian.Token.claims(), options) ::
{:ok, Guardian.Token.token(), Guardian.Token.claims()} | {:error, any}
def encode_and_sign(mod, resource, claims \\ %{}, opts \\ []) do
claims =
claims
|> Enum.into(%{})
|> Guardian.stringify_keys()
token_mod = Guardian.token_module(mod)
with {:ok, subject} <- returning_tuple({mod, :subject_for_token, [resource, claims]}),
{:ok, claims} <- returning_tuple({token_mod, :build_claims, [mod, resource, subject, claims, opts]}),
{:ok, claims} <- returning_tuple({mod, :build_claims, [claims, resource, opts]}),
{:ok, token} <- returning_tuple({token_mod, :create_token, [mod, claims, opts]}),
{:ok, _} <- returning_tuple({mod, :after_encode_and_sign, [resource, claims, token, opts]}) do
{:ok, token, claims}
end
end
I could have just made a Pull Request, but I thought perhaps this behavior is what some people actually want? At a minimum I would expect it to look for :ok
instead of a tuple since that implies you use the result.
Would something like this be a good workaround?
defmodule MyApp.Guardian do
use Guardian, otp_app: :my_app
defoverridable [encode_and_sign: 3]
...
@spec encode_and_sign(any, Guardian.Token.claims(), Guardian.options()) ::
{:ok, Guardian.Token.token(), Guardian.Token.claims()} | {:error, any}
def encode_and_sign(resource, claims \\ %{}, opts \\ []) do
mod = __MODULE__
claims =
claims
|> Enum.into(%{})
|> Guardian.stringify_keys()
token_mod = Guardian.token_module(mod)
with {:ok, subject} <- Guardian.returning_tuple({mod, :subject_for_token, [resource, claims]}),
{:ok, claims} <- Guardian.returning_tuple({token_mod, :build_claims, [mod, resource, subject, claims, opts]}),
{:ok, claims} <- Guardian.returning_tuple({mod, :build_claims, [claims, resource, opts]}),
{:ok, token} <- Guardian.returning_tuple({token_mod, :create_token, [mod, claims, opts]}),
{:ok, token} <- Guardian.returning_tuple({mod, :after_encode_and_sign, [resource, claims, token, opts]}) do
{:ok, token, claims}
end
end
...
end
After a quick search: https://github.com/search?q=after_encode_and_sign+language%3AElixir&type=code&l=Elixir&p=1
They are many people who misuse Guardian.DB.after_encode_and_sign/4
function which does not return {:ok, Guardian.Token.token()} | {:error, atom}
but instead they it returns {:ok, {resource, type, claims, jwt}}
Based on the contract alone, it is safe to assume they are making a mistake today.
But I digress.
I could have just made a Pull Request, but I thought perhaps this behavior is what some people actually want?
The token was already created at this point in the pipeline, so you wouldn't "modify" the token.
I do agree with you that it is weird expecting {:ok, token}
back instead of :ok.
The tricky situation is that it would be a breaking change to change the signature.
@doomspork do you remember what was your intent here? It feels that it should be :ok
as a return (aside from the breaking change situation).