pow-auth / assent

Multi-provider framework in Elixir
https://powauth.com
MIT License
391 stars 45 forks source link

Callback param "user" is not parsed when using Apple strategy with "name" scope #85

Closed jtormey closed 3 years ago

jtormey commented 3 years ago

When using the "name" scope with sign-in with Apple, it doesn't seem like the "user" map that contains "firstName" and "lastName" is decoded, or added to the token map in OAuth2.callback/3. This means that when Strategy.Apple attempts to merge the name params into the other user params, nothing is there. In my application this results in "given_name" and "family_name" being absent when trying to insert a new user into the database.

I was able to fix this in my project by decoding the "user" map in OAuth2.callback/3, and assigning it to token before passing it off to fetch_user_with_strategy as follows:

  @impl true
  @spec callback(Config.t(), map(), atom()) :: {:ok, %{user: map(), token: map()}} | {:error, term()}
  def callback(config, params, strategy \\ __MODULE__) do
    with {:ok, session_params} <- Config.fetch(config, :session_params),
         :ok                   <- check_error_params(params),
         {:ok, code}           <- fetch_code_param(params),
         {:ok, redirect_uri}   <- Config.fetch(config, :redirect_uri),
         :ok                   <- maybe_check_state(session_params, params),
-        {:ok, token}          <- grant_access_token(config, "authorization_code", code: code, redirect_uri: redirect_uri) do
+        {:ok, token}          <- grant_access_token(config, "authorization_code", code: code, redirect_uri: redirect_uri),
+        token                 <- maybe_add_user_callback_params(config, token, params) do

      fetch_user_with_strategy(config, token, strategy)
    end
  end

+ defp maybe_add_user_callback_params(config, token, %{"provider" => "apple", "user" => user}) do
+   Map.put(token, "user", Config.json_library(config).decode!(user))
+ end
+ defp maybe_add_user_callback_params(_config, token, _params), do: token

Does this seem right? I've looked pretty closely at the code paths involved, and don't see any other place where this information seems to be handled. Happy to open a PR if this is a real issue, and if the fix seems appropriate!

danschultzer commented 3 years ago

Thanks @jtormey!

I've fixed it in #86, but haven't had the chance to test it out. Let me know if it works for you!

The reason I did it this way is that Apple for the most part adheres to the OIDC specs but not always. This is one such case. As I understand it, OIDC only permits code, state, and sometimes id_token to be returned in the auth code flow. OAuth 2.0 does permit additional params but they have to be explicitly recognized. And on top of that the value for the query param is JSON encoded.

Anyway, I hope this resolves your issue!

danschultzer commented 3 years ago

v0.1.25 has been released, thanks!