after_encode_and_sign result not used

[jon-mcclung-fortyau]

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.

[jon-mcclung-fortyau]

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

[yordis]

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).

[github-actions[bot]]

This issue has been automatically marked as "stale:discard". If this issue still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment.

[github-actions[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still relevant, feel free to re-open the issue. Thank you!