From 229e7a0ae633577fb34b253a5312fd49c8df7283 Mon Sep 17 00:00:00 2001 From: Dan Schultzer Date: Thu, 7 Mar 2019 21:42:31 -0800 Subject: [PATCH 1/3] Add schema handling for invited user --- lib/pow_assent/ecto/schema.ex | 8 ++++++++ test/pow_assent/ecto/schema_test.exs | 12 ++++++++++++ test/support/extensions/invitation/user.ex | 19 +++++++++++++++++++ .../extensions/invitation/user_identity.ex | 10 ++++++++++ 4 files changed, 49 insertions(+) create mode 100644 test/support/extensions/invitation/user.ex create mode 100644 test/support/extensions/invitation/user_identity.ex diff --git a/lib/pow_assent/ecto/schema.ex b/lib/pow_assent/ecto/schema.ex index 89f59ea..2dfc76c 100644 --- a/lib/pow_assent/ecto/schema.ex +++ b/lib/pow_assent/ecto/schema.ex @@ -88,11 +88,19 @@ defmodule PowAssent.Ecto.Schema do def changeset(user_or_changeset, user_identity, attrs, user_id_attrs, _config) do user_or_changeset |> Changeset.change() + |> maybe_accept_invitation() |> user_id_field_changeset(attrs, user_id_attrs) |> Changeset.cast(%{user_identities: [user_identity]}, []) |> Changeset.cast_assoc(:user_identities) end + defp maybe_accept_invitation(%Changeset{data: %user_mod{invitation_token: token, invitation_accepted_at: nil} = changeset}) when not is_nil(token) do + accepted_at = Pow.Ecto.Schema.__timestamp_for__(user_mod, :invitation_accepted_at) + + Changeset.change(changeset, invitation_accepted_at: accepted_at) + end + defp maybe_accept_invitation(changeset), do: changeset + defp user_id_field_changeset(changeset, attrs, nil) do changeset |> changeset.data.__struct__.pow_user_id_field_changeset(attrs) diff --git a/test/pow_assent/ecto/schema_test.exs b/test/pow_assent/ecto/schema_test.exs index b801d04..833a09d 100644 --- a/test/pow_assent/ecto/schema_test.exs +++ b/test/pow_assent/ecto/schema_test.exs @@ -17,6 +17,7 @@ defmodule PowAssent.Ecto.SchemaTest do alias PowAssent.Test.Ecto.{Repo, Users.User} alias PowAssent.Test.EmailConfirmation.Users.User, as: UserConfirmEmail + alias PowAssent.Test.Invitation.Users.User, as: InvitationUser test "user_schema/1" do user = %User{} @@ -65,6 +66,17 @@ defmodule PowAssent.Ecto.SchemaTest do changeset = User.user_identity_changeset(%User{}, @user_identity, %{email: "test@example.com"}, nil) refute changeset.changes[:email_confirmed_at] end + + test "sets :invitation_accepted_at when is invited user" do + changeset = InvitationUser.user_identity_changeset(%InvitationUser{}, @user_identity, %{}, %{email: "test@example.com"}) + refute changeset.changes[:invitation_accepted_at] + + changeset = InvitationUser.user_identity_changeset(%InvitationUser{invitation_token: "token", invitation_accepted_at: DateTime.utc_now()}, @user_identity, %{}, %{email: "test@example.com"}) + refute changeset.changes[:invitation_accepted_at] + + changeset = InvitationUser.user_identity_changeset(%InvitationUser{invitation_token: "token"}, @user_identity, %{}, %{email: "test@example.com"}) + assert changeset.changes[:invitation_accepted_at] + end end defmodule OverrideAssocUser do diff --git a/test/support/extensions/invitation/user.ex b/test/support/extensions/invitation/user.ex new file mode 100644 index 0000000..89eaba3 --- /dev/null +++ b/test/support/extensions/invitation/user.ex @@ -0,0 +1,19 @@ +defmodule PowAssent.Test.Invitation.Users.User do + @moduledoc false + use Ecto.Schema + use Pow.Ecto.Schema + use Pow.Extension.Ecto.Schema, + extensions: [PowInvitation] + use PowAssent.Ecto.Schema + + schema "users" do + has_many :user_identities, + PowAssent.Test.Invitation.UserIdentities.UserIdentity, + on_delete: :delete_all, + foreign_key: :user_id + + pow_user_fields() + + timestamps() + end +end diff --git a/test/support/extensions/invitation/user_identity.ex b/test/support/extensions/invitation/user_identity.ex new file mode 100644 index 0000000..146c0f9 --- /dev/null +++ b/test/support/extensions/invitation/user_identity.ex @@ -0,0 +1,10 @@ +defmodule PowAssent.Test.Invitation.UserIdentities.UserIdentity do + @moduledoc false + use Ecto.Schema + use PowAssent.Ecto.UserIdentities.Schema, + user: PowAssent.Test.Invitation.Users.User + + schema "user_identities" do + pow_assent_user_identity_fields() + end +end From 46c06af72caf4790da2fe0a62aa0b4e29cba716b Mon Sep 17 00:00:00 2001 From: Dan Schultzer Date: Sat, 9 Mar 2019 08:37:37 -0800 Subject: [PATCH 2/3] Add invitation token to authorization links --- README.md | 6 ++++++ lib/pow_assent/phoenix/views/view_helpers.ex | 9 +++++++-- test/pow_assent/phoenix/views/view_helpers_test.exs | 7 +++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c5a9bb6..ead842a 100644 --- a/README.md +++ b/README.md @@ -308,6 +308,12 @@ config :my_app, :pow_assent, The e-mail fetched from the provider is assumed already confirmed, and the user will have `:email_confirmed_at` set when inserted. If a user enters an e-mail then the user will have to confirm their e-mail before they can sign in. +### PowInvitation + +PowAssent works out of the box with PowInvitation. If a user identity is created, the an invited user will have the `:invitation_accepted_at` set. + +Provider links will have an `invitation_token` query param if an invited user exists in the connection. This will be used in the authorization callback flow to load the invited user. + ## Security concerns All sessions created through PowAssent provider authentication are temporary. However, it's a good idea to do some housekeeping in your app and make sure that you have the level of security as warranted by the scope of your app. That may include requiring users to re-authenticate before viewing or editing their user details. diff --git a/lib/pow_assent/phoenix/views/view_helpers.ex b/lib/pow_assent/phoenix/views/view_helpers.ex index 31bc553..88f9b45 100644 --- a/lib/pow_assent/phoenix/views/view_helpers.ex +++ b/lib/pow_assent/phoenix/views/view_helpers.ex @@ -33,9 +33,14 @@ defmodule PowAssent.Phoenix.ViewHelpers do end end - defp oauth_signin_link(conn, provider) do + defp oauth_signin_link(%{assigns: %{invited_user: %{invitation_token: token}}} = conn, provider) when not is_nil(token) do + do_oauth_signin_link(conn, provider, invitation_token: token) + end + defp oauth_signin_link(conn, provider), do: do_oauth_signin_link(conn, provider) + + defp do_oauth_signin_link(conn, provider, query_params \\[]) do msg = AuthorizationController.messages(conn).login_with_provider(%{conn | params: %{"provider" => provider}}) - path = AuthorizationController.routes(conn).path_for(conn, AuthorizationController, :new, [provider]) + path = AuthorizationController.routes(conn).path_for(conn, AuthorizationController, :new, [provider], query_params) Link.link(msg, to: path) end diff --git a/test/pow_assent/phoenix/views/view_helpers_test.exs b/test/pow_assent/phoenix/views/view_helpers_test.exs index 7766448..cde9bb2 100644 --- a/test/pow_assent/phoenix/views/view_helpers_test.exs +++ b/test/pow_assent/phoenix/views/view_helpers_test.exs @@ -35,4 +35,11 @@ defmodule PowAssent.ViewHelpersTest do [safe: iodata] = ViewHelpers.provider_links(conn) assert {:safe, iodata} == Link.link("Remove Test provider authentication", to: "/auth/test_provider", method: "delete") end + + test "provider_links/1 with invited_user", %{conn: conn} do + conn = PowInvitation.Plug.assign_invited_user(conn, %PowAssent.Test.Invitation.Users.User{invitation_token: "token"}) + + [safe: iodata] = ViewHelpers.provider_links(conn) + assert {:safe, iodata} == Link.link("Sign in with Test provider", to: "/auth/test_provider/new?invitation_token=token") + end end From 2e72541a1d99c5057cb90405f14c537317f1eb56 Mon Sep 17 00:00:00 2001 From: Dan Schultzer Date: Sat, 9 Mar 2019 08:38:05 -0800 Subject: [PATCH 3/3] Handle invited user in controller --- config/test.exs | 6 +++- .../controllers/authorization_controller.ex | 30 ++++++++++++++--- .../authorization_controller_test.exs | 25 +++++++++++++- .../extensions/invitation/phoenix/endpoint.ex | 33 +++++++++++++++++++ .../extensions/invitation/repo_mock.ex | 9 +++++ test/test_helper.exs | 1 + 6 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 test/support/extensions/invitation/phoenix/endpoint.ex create mode 100644 test/support/extensions/invitation/repo_mock.ex diff --git a/config/test.exs b/config/test.exs index c54cc15..287bf70 100644 --- a/config/test.exs +++ b/config/test.exs @@ -13,7 +13,11 @@ config :pow_assent, PowAssent.Test.EmailConfirmation.Phoenix.Endpoint, secret_key_base: String.duplicate("abcdefghijklmnopqrstuvxyz0123456789", 2), render_errors: [view: PowAssent.Test.Phoenix.ErrorView, accepts: ~w(html json)] - config :pow_assent, PowAssent.Test.NoRegistration.Phoenix.Endpoint, + config :pow_assent, PowAssent.Test.Invitation.Phoenix.Endpoint, + secret_key_base: String.duplicate("abcdefghijklmnopqrstuvxyz0123456789", 2), + render_errors: [view: PowAssent.Test.Phoenix.ErrorView, accepts: ~w(html json)] + +config :pow_assent, PowAssent.Test.NoRegistration.Phoenix.Endpoint, secret_key_base: String.duplicate("abcdefghijklmnopqrstuvxyz0123456789", 2), render_errors: [view: PowAssent.Test.Phoenix.ErrorView, accepts: ~w(html json)] diff --git a/lib/pow_assent/phoenix/controllers/authorization_controller.ex b/lib/pow_assent/phoenix/controllers/authorization_controller.ex index d49b4d0..fa68968 100644 --- a/lib/pow_assent/phoenix/controllers/authorization_controller.ex +++ b/lib/pow_assent/phoenix/controllers/authorization_controller.ex @@ -9,6 +9,7 @@ defmodule PowAssent.Phoenix.AuthorizationController do plug :require_authenticated when action in [:delete] plug :assign_callback_url when action in [:new, :callback] + plug :load_user_by_invitation_token when action in [:callback] @spec process_new(Conn.t(), map()) :: {:ok, binary(), Conn.t()} | {:error, any(), Conn.t()} def process_new(conn, %{"provider" => provider}) do @@ -19,6 +20,7 @@ defmodule PowAssent.Phoenix.AuthorizationController do def respond_new({:ok, url, conn}) do conn |> maybe_store_state() + |> maybe_store_invitation_token() |> redirect(external: url) end def respond_new({:error, error, _conn}), do: handle_strategy_error(error) @@ -26,6 +28,9 @@ defmodule PowAssent.Phoenix.AuthorizationController do defp maybe_store_state(%{private: %{pow_assent_state: state}} = conn), do: store_state(conn, state) defp maybe_store_state(conn), do: conn + defp maybe_store_invitation_token(%{params: %{"invitation_token" => token}} = conn), do: store_invitation_token(conn, token) + defp maybe_store_invitation_token(conn), do: conn + @spec process_callback(Conn.t(), map()) :: {:ok, Conn.t()} | {:error, Conn.t()} | {:error, {atom(), map()} | map(), Conn.t()} def process_callback(conn, %{"provider" => provider} = params) do conn @@ -41,10 +46,13 @@ defmodule PowAssent.Phoenix.AuthorizationController do end end - defp handle_callback({:ok, user, conn}, provider) do + defp handle_callback({:ok, user_params, %{assigns: %{invited_user: invited_user}} = conn}, provider) do + authenticate_or_create_identity(invited_user, provider, user_params, conn) + end + defp handle_callback({:ok, user_params, conn}, provider) do conn |> Pow.Plug.current_user() - |> authenticate_or_create_identity(provider, user, conn) + |> authenticate_or_create_identity(provider, user_params, conn) end defp handle_callback({:error, error, _conn}, _provider), do: handle_strategy_error(error) @@ -154,14 +162,26 @@ defmodule PowAssent.Phoenix.AuthorizationController do assign(conn, :callback_url, url) end - defp store_state(conn, state) do - Conn.put_session(conn, :pow_assent_state, state) - end + defp store_state(conn, state), do: Conn.put_session(conn, :pow_assent_state, state) defp fetch_state(%{private: %{plug_session: %{"pow_assent_state" => state}}} = conn) do {state, Conn.put_session(conn, :pow_assent_state, nil)} end defp fetch_state(conn), do: conn + defp store_invitation_token(conn, token), do: Conn.put_session(conn, :pow_assent_invitation_token, token) + + defp load_user_by_invitation_token(%{private: %{plug_session: %{"pow_assent_invitation_token" => token}}} = conn, _opts) do + conn = Conn.delete_session(conn,:pow_assent_invitation_token) + + conn + |> PowInvitation.Plug.invited_user_from_token(token) + |> case do + nil -> conn + user -> PowInvitation.Plug.assign_invited_user(conn, user) + end + end + defp load_user_by_invitation_token(conn, _opts), do: conn + defp handle_strategy_error(error), do: raise error end diff --git a/test/pow_assent/phoenix/controllers/authorization_controller_test.exs b/test/pow_assent/phoenix/controllers/authorization_controller_test.exs index 2b7b38a..78516ab 100644 --- a/test/pow_assent/phoenix/controllers/authorization_controller_test.exs +++ b/test/pow_assent/phoenix/controllers/authorization_controller_test.exs @@ -25,6 +25,12 @@ defmodule PowAssent.Phoenix.AuthorizationControllerTest do assert Plug.Conn.get_session(conn, :pow_assent_state) end + test "redirects with invitation_token saved", %{conn: conn} do + conn = get conn, Routes.pow_assent_authorization_path(conn, :new, @provider, invitation_token: "token") + + assert Plug.Conn.get_session(conn, :pow_assent_invitation_token) == "token" + end + test "with error", %{conn: conn, bypass: bypass} do put_oauth2_env(bypass, fail_authorize_url: true) @@ -86,7 +92,6 @@ defmodule PowAssent.Phoenix.AuthorizationControllerTest do assert redirected_to(conn) == "/session_created" assert get_flash(conn, :info) == "signed_in_test_provider" - assert Pow.Plug.current_user(conn) == user refute Plug.Conn.get_session(conn, :pow_assent_state) end @@ -167,6 +172,24 @@ defmodule PowAssent.Phoenix.AuthorizationControllerTest do end end + alias PowAssent.Test.Invitation.Phoenix.Endpoint, as: InvitationEndpoint + describe "GET /auth/:provider/callback as authentication with invitation" do + test "with invitation_token updates user as accepted invtation", %{conn: conn, bypass: bypass} do + expect_oauth2_flow(bypass, user: %{uid: "new_identity"}) + + conn = + conn + |> Plug.Conn.put_session(:pow_assent_state, "token") + |> Plug.Conn.put_session(:pow_assent_invitation_token, "token") + |> Phoenix.ConnTest.dispatch(InvitationEndpoint, :get, Routes.pow_assent_authorization_path(conn, :callback, @provider, @callback_params)) + + assert redirected_to(conn) == "/session_created" + assert get_flash(conn, :info) == "signed_in_test_provider" + refute Plug.Conn.get_session(conn, :pow_assent_invitation_token) + refute Plug.Conn.get_session(conn, :pow_assent_state) + end + end + alias PowAssent.Test.NoRegistration.Phoenix.Endpoint, as: NoRegistrationEndpoint describe "GET /auth/:provider/callback as authentication with missing registration routes" do test "can't register", %{conn: conn, bypass: bypass} do diff --git a/test/support/extensions/invitation/phoenix/endpoint.ex b/test/support/extensions/invitation/phoenix/endpoint.ex new file mode 100644 index 0000000..83bd379 --- /dev/null +++ b/test/support/extensions/invitation/phoenix/endpoint.ex @@ -0,0 +1,33 @@ +defmodule PowAssent.Test.Invitation.Phoenix.Endpoint do + @moduledoc false + use Phoenix.Endpoint, otp_app: :pow_assent + + plug Plug.RequestId + plug Plug.Logger + + plug Plug.Parsers, + parsers: [:urlencoded, :multipart, :json], + pass: ["*/*"], + json_decoder: Phoenix.json_library() + + plug Plug.MethodOverride + plug Plug.Head + + plug Plug.Session, + store: :cookie, + key: "_binaryid_key", + signing_salt: "secret" + + plug Pow.Plug.Session, + user: PowAssent.Test.Invitation.Users.User, + routes_backend: PowAssent.Test.Phoenix.Routes, + messages_backend: PowAssent.Test.Phoenix.Messages, + mailer_backend: PowAssent.Test.Phoenix.MailerMock, + repo: PowAssent.Test.Invitation.RepoMock, + otp_app: :pow_assent, + pow_assent: [ + user_identities_context: PowAssent.Test.UserIdentitiesMock + ] + + plug PowAssent.Test.Phoenix.Router +end diff --git a/test/support/extensions/invitation/repo_mock.ex b/test/support/extensions/invitation/repo_mock.ex new file mode 100644 index 0000000..c87bc43 --- /dev/null +++ b/test/support/extensions/invitation/repo_mock.ex @@ -0,0 +1,9 @@ +defmodule PowAssent.Test.Invitation.RepoMock do + @moduledoc false + + alias PowAssent.Test.Invitation.Users.User + + @user %User{id: 1, email: "test@example.com"} + + def get_by(User, [invitation_token: "token"]), do: %{@user | invitation_token: "token"} +end diff --git a/test/test_helper.exs b/test/test_helper.exs index fe114ef..ace6ef1 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -19,4 +19,5 @@ Ecto.Adapters.SQL.Sandbox.mode(PowAssent.Test.Ecto.Repo, :manual) {:ok, _pid} = PowAssent.Test.Phoenix.Endpoint.start_link() {:ok, _pid} = PowAssent.Test.EmailConfirmation.Phoenix.Endpoint.start_link() +{:ok, _pid} = PowAssent.Test.Invitation.Phoenix.Endpoint.start_link() {:ok, _pid} = PowAssent.Test.NoRegistration.Phoenix.Endpoint.start_link()