Merge pull request #97 from danschultzer/redirect-uri

Update assent requirement and set redirect uri in config

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## v0.4.1 (TBA)
+
+* Use Assent `v0.1.2` and set `:redirect_uri` in config for OAuth 2.0 callback phase
+
 ## v0.4.0 (2019-10-06)
 
 **This release consists of major breaking changes.**

lib/pow_assent/plug.ex

@@ -30,10 +30,9 @@ defmodule PowAssent.Plug do
   """
   @spec authorize_url(Conn.t(), binary(), binary()) :: {:ok, binary(), Conn.t()} | {:error, any(), Conn.t()}
   def authorize_url(conn, provider, redirect_uri) do
-    {strategy, provider_config} = get_provider_config(conn, provider)
+    {strategy, provider_config} = get_provider_config(conn, provider, redirect_uri)
 
     provider_config
-    |> Config.put(:redirect_uri, redirect_uri)
     |> maybe_gen_nonce()
     |> strategy.authorize_url()
     |> maybe_put_session_params(conn)
@@ -68,8 +67,7 @@ defmodule PowAssent.Plug do
   """
   @spec callback(Conn.t(), binary(), map(), binary()) :: {:ok, map(), map(), Conn.t()} | {:error, any(), Conn.t()}
   def callback(conn, provider, params, redirect_uri) do
-    {strategy, provider_config} = get_provider_config(conn, provider)
-    params                      = Map.put(params, "redirect_uri", redirect_uri)
+    {strategy, provider_config} = get_provider_config(conn, provider, redirect_uri)
 
     provider_config
     |> maybe_set_session_params_config(conn)
@@ -228,16 +226,19 @@ defmodule PowAssent.Plug do
     |> Keyword.merge(Keyword.get(config, :pow_assent, []))
   end
 
-  defp get_provider_config(%Conn{} = conn, provider) do
+  defp get_provider_config(%Conn{} = conn, provider, redirect_uri) do
     conn
     |> fetch_config()
-    |> get_provider_config(provider)
+    |> get_provider_config(provider, redirect_uri)
   end
-  defp get_provider_config(config, provider) do
+  defp get_provider_config(config, provider, redirect_uri) do
     provider        = String.to_atom(provider)
     config          = Config.get_provider_config(config, provider)
     strategy        = config[:strategy]
-    provider_config = Keyword.delete(config, :strategy)
+    provider_config =
+      config
+      |> Keyword.delete(:strategy)
+      |> Config.put(:redirect_uri, redirect_uri)
 
     {strategy, provider_config}
   end

mix.exs

@@ -32,7 +32,7 @@ defmodule PowAssent.MixProject do
   defp deps do
     [
       {:pow, "~> 1.0.9"},
-      {:assent, "~> 0.1.0"},
+      {:assent, "~> 0.1.2"},
 
       {:phoenix_html, ">= 2.0.0 and <= 3.0.0"},
       {:phoenix_ecto, ">= 3.0.0 and <= 4.0.0"},

mix.lock

@@ -1,5 +1,5 @@
 %{
-  "assent": {:hex, :assent, "0.1.0", "f7700a74625237527b65bdb59b4a1eb715906e8b91b349fab7f0b1b03e4916a8", [:mix], [{:castore, "~> 0.1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:certifi, ">= 0.0.0", [hex: :certifi, repo: "hexpm", optional: true]}, {:jose, "~> 1.8", [hex: :jose, repo: "hexpm", optional: true]}, {:mint, "~> 0.1.0", [hex: :mint, repo: "hexpm", optional: true]}, {:oauther, "~> 1.1", [hex: :oauther, repo: "hexpm", optional: false]}, {:ssl_verify_fun, ">= 0.0.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: true]}], "hexpm"},
+  "assent": {:hex, :assent, "0.1.2", "c3959b9ae24272dc20beb0495793e5fcfc6129ddb2dadd40aa9405f827d306e6", [:mix], [{:castore, "~> 0.1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:certifi, ">= 0.0.0", [hex: :certifi, repo: "hexpm", optional: true]}, {:jose, "~> 1.8", [hex: :jose, repo: "hexpm", optional: true]}, {:mint, ">= 0.1.0 and < 0.5.0", [hex: :mint, repo: "hexpm", optional: true]}, {:oauther, "~> 1.1", [hex: :oauther, repo: "hexpm", optional: false]}, {:ssl_verify_fun, ">= 0.0.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: true]}], "hexpm"},
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"},
   "bypass": {:hex, :bypass, "1.0.0", "b78b3dcb832a71aca5259c1a704b2e14b55fd4e1327ff942598b4e7d1a7ad83d", [:mix], [{:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 1.0 or ~> 2.0", [hex: :plug_cowboy, repo: "hexpm", optional: false]}], "hexpm"},
   "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], [], "hexpm"},

test/pow_assent/plug_test.exs

@@ -6,7 +6,7 @@ defmodule PowAssent.PlugTest do
   alias PowAssent.Plug
   alias PowAssent.Test.{UserIdentitiesMock, Ecto.Users.User}
 
-  import PowAssent.Test.TestProvider, only: [expect_oauth2_flow: 1, expect_oauth2_flow: 2, put_oauth2_env: 1, put_oauth2_env: 2]
+  import PowAssent.Test.TestProvider, only: [expect_oauth2_flow: 2, put_oauth2_env: 1, put_oauth2_env: 2]
 
   @default_config [
     plug: Pow.Plug.Session,
@@ -66,17 +66,22 @@ defmodule PowAssent.PlugTest do
     end
 
     test "returns user params", %{conn: conn, bypass: bypass} do
-      expect_oauth2_flow(bypass)
+      expect_oauth2_flow(bypass, access_token_assert_fn: fn conn ->
+        {:ok, body, _conn} = Conn.read_body(conn, [])
+        params = URI.decode_query(body)
 
-      assert {:ok, user_identity_params, user_params, _conn} = Plug.callback(conn, "test_provider", %{"code" => "access_token"}, "")
+        assert params["redirect_uri"] == "https://example.com/"
+      end)
+
+      assert {:ok, user_identity_params, user_params, _conn} = Plug.callback(conn, "test_provider", %{"code" => "access_token"}, "https://example.com/")
       assert user_identity_params == %{"provider" => "test_provider", "uid" => "new_user", "token" => %{"access_token" => "access_token"}}
       assert user_params == %{"name" => "Dan Schultzer"}
     end
 
     test "returns user params with preferred username as username", %{conn: conn, bypass: bypass} do
       expect_oauth2_flow(bypass, user: %{preferred_username: "john.doe"})
 
-      assert {:ok, user_identity_params, user_params, _conn} = Plug.callback(conn, "test_provider", %{"code" => "access_token"}, "")
+      assert {:ok, user_identity_params, user_params, _conn} = Plug.callback(conn, "test_provider", %{"code" => "access_token"}, "https://example.com/")
       assert user_identity_params == %{"provider" => "test_provider", "uid" => "new_user", "token" => %{"access_token" => "access_token"}}
       assert user_params == %{"username" => "john.doe", "name" => "Dan Schultzer"}
     end

test/support/strategies/oauth2_test_case.ex

@@ -3,7 +3,7 @@ defmodule PowAssent.Test.OAuth2TestCase do
   use ExUnit.CaseTemplate
 
   setup _tags do
-    params = %{"code" => "test", "redirect_uri" => "test", "state" => "test"}
+    params = %{"code" => "test", "state" => "test"}
     bypass = Bypass.open()
     config = [client_secret: "secret", site: "http://localhost:#{bypass.port}", session_params: %{state: "test"}]
 

test/support/test_provider.ex

@@ -22,7 +22,7 @@ defmodule PowAssent.Test.TestProvider do
     token_params = Keyword.get(opts, :token, %{"access_token" => "access_token"})
     user_params  = Map.merge(%{sub: "new_user", name: "Dan Schultzer"}, Keyword.get(opts, :user, %{}))
 
-    PowAssent.Test.OAuth2TestCase.expect_oauth2_access_token_request(bypass, params: token_params)
+    PowAssent.Test.OAuth2TestCase.expect_oauth2_access_token_request(bypass, [params: token_params], opts[:access_token_assert_fn])
     PowAssent.Test.OAuth2TestCase.expect_oauth2_user_request(bypass, user_params)
   end