riverrun / phauxth

Not actively maintained - Authentication library for Phoenix, and other Plug-based, web applications
409 stars 20 forks source link

authenticate callback arity for token cookie module is incorrect #114

Closed jhefreyzz closed 4 years ago

jhefreyzz commented 4 years ago

Environment

Issue

It seems like the arity for authenticate callback is incorrect. Based on the guide from the docs: https://hexdocs.pm/phauxth/Phauxth.Authenticate.Token.html, the function is authenticate/2. I implemented the same way since I'm using cookie to store the token but Phauxth.Authenticate.Token expects authenticate/3.

My implementation:

defmodule ShiritoriWeb.Auth.AuthenticateTokenCookie do
  use Phauxth.Authenticate.Token

  @impl true
  def authenticate(%Plug.Conn{req_cookies: %{"token" => token}}, opts) do
    verify_token(token, opts, opts)
  end
end

The warning from ElixirLS:

got "@impl true" for function authenticate/2 but no behaviour specifies such callback. The known callbacks are:

  * Phauxth.Authenticate.Base.authenticate/3 (function)
  * Plug.call/2 (function)
  * Phauxth.Authenticate.Token.get_token/3 (function)
  * Plug.init/1 (function)
  * Phauxth.Authenticate.Base.report/2 (function)
  * Phauxth.Authenticate.Base.set_user/2 (function)

furthermore verify_token expects 3 parameters but the verify_token from the guide I'm referring to, only pass 2 parameters.

From the docs:

defmodule Phauxth.AuthenticateTokenCookie do
  use Phauxth.Authenticate.Token

  @impl true
  def authenticate(%Plug.Conn{req_cookies: %{"access_token" => token}}, opts) do
    verify_token(token, opts)
  end
end

I found a workaround but involves removing the @impl true for the authenticate/2 and passing opts to 2nd and 3rd parameter of verify_token:

  def authenticate(%Plug.Conn{req_cookies: %{"token" => token}}, opts) do
    verify_token(token, opts, opts)
  end
riverrun commented 4 years ago

Thanks for pointing that out. It looks like the examples in the documentation are wrong.

The example should be:

defmodule Phauxth.AuthenticateTokenCookie do
  use Phauxth.Authenticate.Token

  @impl true
  def authenticate(%Plug.Conn{req_cookies: %{"access_token" => token}}, user_context, opts) do
    verify_token(token, user_context, opts)
  end
end

And I will update the docs later this week.

riverrun commented 4 years ago

Documentation updated in version 2.3.2