From b99e7180361136705ed314ff262bc91ec2e2c6ba Mon Sep 17 00:00:00 2001 From: "Elias W. BA" Date: Thu, 15 Aug 2024 11:52:01 +0000 Subject: [PATCH] Ensure Users Confirm Their Accounts After Sign Up (#2364) * Render banner when account is not confirmed * Account confirmation modal * Resend confirmation email using banner * Module doc and tests * Email sent successful alert inside modal content * Change email * Put flash on conn * CSS tweaks * Testing the controller functions * Cleanup closes #2353, closes #2368 * Move message for account confirmation into heex * Format date differently --------- Co-authored-by: Taylor Downs Co-authored-by: Stuart Corbishley --- CHANGELOG.md | 3 + assets/css/app.css | 8 +- lib/lightning/accounts.ex | 13 ++ lib/lightning/accounts/user_notifier.ex | 14 +++ lib/lightning_web/components/icons.ex | 7 +- .../components/layout_components.ex | 14 +++ .../components/layouts/live.html.heex | 14 ++- lib/lightning_web/controllers/user_auth.ex | 6 +- .../user_confirmation_controller.ex | 25 ++++ lib/lightning_web/init_assigns.ex | 11 +- .../live/account_confirmation_modal.ex | 112 ++++++++++++++++++ lib/lightning_web/live/components/common.ex | 75 ++++++++++-- .../live/project_live/settings.ex | 2 +- lib/lightning_web/live/workflow_live/edit.ex | 7 +- lib/lightning_web/plugs/redirect.ex | 3 +- lib/lightning_web/router.ex | 2 + .../lightning/accounts/user_notifier_test.exs | 26 ++++ test/lightning/accounts_test.exs | 20 ++++ .../user_confirmation_controller_test.exs | 40 +++++++ .../live/accounts_confirmation_live_test.exs | 40 +++++++ test/lightning_web/live/project_live_test.exs | 4 +- 21 files changed, 420 insertions(+), 26 deletions(-) create mode 100644 lib/lightning_web/live/account_confirmation_modal.ex create mode 100644 test/lightning_web/live/accounts_confirmation_live_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index c3c4361720..265e89aa5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ and this project adheres to ### Added +- Ensure that all users in an instance have a confirmed email address within 48 + hours [#2389](https://github.com/OpenFn/lightning/issues/2389) + ### Changed ### Fixed diff --git a/assets/css/app.css b/assets/css/app.css index fd6389463b..ee25518b86 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -19,14 +19,14 @@ border-color: #bce8f1; } .alert-warning { - color: #8a6d3b; + color: #8B5F0D; background-color: #fcf8e3; border-color: #faebcc; } .alert-danger { - color: #a94442; - background-color: #f2dede; - border-color: #ebccd1; + color: #DC2626; + background-color: #FDEBEB; + border-color: #FDEBEB; } .alert p { margin-bottom: 0; diff --git a/lib/lightning/accounts.ex b/lib/lightning/accounts.ex index dfd0110cd0..bd10258cfd 100644 --- a/lib/lightning/accounts.ex +++ b/lib/lightning/accounts.ex @@ -829,6 +829,13 @@ defmodule Lightning.Accounts do end end + def remind_account_confirmation(%User{} = user) do + UserNotifier.remind_account_confirmation( + user, + build_email_token(user) + ) + end + @doc """ Confirms a user by the given token. @@ -952,4 +959,10 @@ defmodule Lightning.Accounts do ) |> Repo.all() end + + def confirmation_required?(%User{confirmed_at: nil, inserted_at: inserted_at}) do + DateTime.diff(DateTime.utc_now(), inserted_at, :hour) >= 48 + end + + def confirmation_required?(_user), do: false end diff --git a/lib/lightning/accounts/user_notifier.ex b/lib/lightning/accounts/user_notifier.ex index 8f9ea81d90..601d0940ca 100644 --- a/lib/lightning/accounts/user_notifier.ex +++ b/lib/lightning/accounts/user_notifier.ex @@ -87,6 +87,20 @@ defmodule Lightning.Accounts.UserNotifier do """) end + def remind_account_confirmation(user, token) do + deliver(user.email, "Confirm your OpenFn account", """ + Hello #{user.first_name}, + + Please confirm your OpenFn account by clicking on the URL below: + + #{url(LightningWeb.Endpoint, ~p"/users/confirm/#{token}")} + + If you have not requested an account confirmation email, please ignore this. + + OpenFn + """) + end + @doc """ Deliver email to notify user of his addition of a project. """ diff --git a/lib/lightning_web/components/icons.ex b/lib/lightning_web/components/icons.ex index 1f3af55497..63a7df1b3b 100644 --- a/lib/lightning_web/components/icons.ex +++ b/lib/lightning_web/components/icons.ex @@ -29,7 +29,12 @@ defmodule LightningWeb.Components.Icons do def icon(%{name: "hero-" <> _} = assigns) do ~H""" """ diff --git a/lib/lightning_web/components/layout_components.ex b/lib/lightning_web/components/layout_components.ex index e74ec7e007..1604e2bf3e 100644 --- a/lib/lightning_web/components/layout_components.ex +++ b/lib/lightning_web/components/layout_components.ex @@ -65,6 +65,7 @@ defmodule LightningWeb.LayoutComponents do slot :inner_block def header(assigns) do + # TODO - remove title_height once we confirm that :description is unused title_height = if Enum.any?(assigns[:description]) do "mt-4 h-10" @@ -80,6 +81,19 @@ defmodule LightningWeb.LayoutComponents do ) ~H""" + DateTime.add(48, :hour) |> Timex.format!("%A, %d %B @ %H:%M UTC", :strftime)} to continue using OpenFn."} + action={ + %{ + text: "Resend confirmation email", + target: "/users/send-confirmation-email" + } + } + />
<%= if @current_user do %> diff --git a/lib/lightning_web/components/layouts/live.html.heex b/lib/lightning_web/components/layouts/live.html.heex index 56a6ecf351..5af3b97445 100644 --- a/lib/lightning_web/components/layouts/live.html.heex +++ b/lib/lightning_web/components/layouts/live.html.heex @@ -52,10 +52,22 @@
- <.live_nav_block flash={@flash}> + <.live_nav_block + :if={ + !assigns[:account_confirmation_required?] or + @socket.view == LightningWeb.ProfileLive.Edit + } + flash={@flash} + > <%= @inner_content %> <.live_component module={LightningWeb.ModalPortal} id="modal-portal" /> + <.live_component + :if={assigns[:account_confirmation_required?]} + module={LightningWeb.AccountConfirmationModal} + id="account-confirmation-modal" + current_user={@current_user} + />
diff --git a/lib/lightning_web/controllers/user_auth.ex b/lib/lightning_web/controllers/user_auth.ex index 6798b7413f..38986253ef 100644 --- a/lib/lightning_web/controllers/user_auth.ex +++ b/lib/lightning_web/controllers/user_auth.ex @@ -121,7 +121,11 @@ defmodule LightningWeb.UserAuth do def fetch_current_user(conn, _opts) do {user_token, conn} = ensure_user_token(conn) user = user_token && Accounts.get_user_by_session_token(user_token) - assign(conn, :current_user, user) + confirmation_required? = Accounts.confirmation_required?(user) + + conn + |> assign(:current_user, user) + |> assign(:account_confirmation_required?, confirmation_required?) end defp ensure_user_token(conn) do diff --git a/lib/lightning_web/controllers/user_confirmation_controller.ex b/lib/lightning_web/controllers/user_confirmation_controller.ex index 218a227139..d24113dcd5 100644 --- a/lib/lightning_web/controllers/user_confirmation_controller.ex +++ b/lib/lightning_web/controllers/user_confirmation_controller.ex @@ -42,6 +42,31 @@ defmodule LightningWeb.UserConfirmationController do end end + def send_email( + %{assigns: %{current_user: %{confirmed_at: nil}}} = conn, + _params + ) do + Lightning.Accounts.remind_account_confirmation(conn.assigns.current_user) + + conn + |> put_flash(:info, "Confirmation email sent successfully") + |> redirect(to: get_referer(conn)) + end + + def send_email(conn, _params) do + redirect(conn, to: get_referer(conn)) + end + + defp get_referer(conn) do + conn + |> Plug.Conn.get_req_header("referer") + |> List.first() + |> case do + nil -> "/projects" + referer -> URI.parse(referer) |> Map.get(:path) + end + end + # Do not log in the user after confirmation to avoid a # leaked token giving the user access to the account. def update(conn, %{"token" => token}) do diff --git a/lib/lightning_web/init_assigns.ex b/lib/lightning_web/init_assigns.ex index 6b34bb383b..38751186e4 100644 --- a/lib/lightning_web/init_assigns.ex +++ b/lib/lightning_web/init_assigns.ex @@ -6,9 +6,16 @@ defmodule LightningWeb.InitAssigns do alias Lightning.Accounts def on_mount(:default, _params, session, socket) do + current_user = Accounts.get_user_by_session_token(session["user_token"]) + confirmation_required? = Accounts.confirmation_required?(current_user) + {:cont, - assign_new(socket, :current_user, fn -> - Accounts.get_user_by_session_token(session["user_token"]) + socket + |> assign_new(:current_user, fn -> + current_user + end) + |> assign_new(:account_confirmation_required?, fn -> + confirmation_required? end)} end end diff --git a/lib/lightning_web/live/account_confirmation_modal.ex b/lib/lightning_web/live/account_confirmation_modal.ex new file mode 100644 index 0000000000..b0d4802477 --- /dev/null +++ b/lib/lightning_web/live/account_confirmation_modal.ex @@ -0,0 +1,112 @@ +defmodule LightningWeb.AccountConfirmationModal do + @moduledoc """ + A LiveView component for displaying an account confirmation modal. + + This component is responsible for informing users that access to their + accounts, projects, and workflows is restricted until they confirm + their account. It provides functionality to resend the confirmation + email and allows users to update their email address if needed. + + ## Features + + - Displays a modal with instructions for account confirmation. + - Allows users to resend the confirmation email. + - Provides feedback when the confirmation email is successfully sent. + - Allows users to navigate to the profile page to update their email address. + + ## Usage + + Include this component in your LiveView template where you need to prompt + users to confirm their account. The component determines whether the modal + should be shown based on the current view context. + + ## Examples + + <.live_component + module={LightningWeb.AccountConfirmationModal} + id="account-confirmation-modal" + current_user={@current_user} + /> + + The component uses assigns to manage its state, including: + + - `:show_modal` - Determines if the modal should be visible. + - `:email_sent` - Indicates if the confirmation email was successfully sent. + """ + use LightningWeb, :live_component + + @impl true + def update(assigns, socket) do + show_modal = + case socket.view do + LightningWeb.ProfileLive.Edit -> false + _ -> true + end + + {:ok, + socket + |> assign(assigns) + |> assign(:show_modal, show_modal) + |> assign(:email_sent, false)} + end + + @impl true + def handle_event("resend-confirmation-email", _, socket) do + Lightning.Accounts.remind_account_confirmation(socket.assigns.current_user) + + {:noreply, assign(socket, :email_sent, true)} + end + + @impl true + def render(assigns) do + ~H""" +
+ <.modal + id={@id} + show={@show_modal} + close_on_keydown={false} + close_on_click_away={false} + width="xl:min-w-1/3 min-w-1/2 w-1/3" + > + <:title> +
+ Confirm your account +
+ +
+
+ This account has been blocked pending email confirmation. Please + check your email for a confirmation link, request that a new link be + sent, or update your email address to keep using OpenFn. +
+ + <:message> + A new link has been sent. Please check your email. + + +
+ <.modal_footer class="mt-6 mx-6"> +
+ + <.link + href={~p"/profile"} + class="inline-flex justify-center rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50 sm:w-auto" + > + Update email address + +
+ + +
+ """ + end +end diff --git a/lib/lightning_web/live/components/common.ex b/lib/lightning_web/live/components/common.ex index a8d16021bf..1e2aa455ad 100644 --- a/lib/lightning_web/live/components/common.ex +++ b/lib/lightning_web/live/components/common.ex @@ -6,6 +6,15 @@ defmodule LightningWeb.Components.Common do alias Phoenix.LiveView.JS + defp select_icon(type) do + case type do + "success" -> "hero-check-circle-solid" + "warning" -> "hero-exclamation-triangle-solid" + "danger" -> "hero-x-circle-solid" + _info -> "hero-information-circle-solid" + end + end + attr :id, :string, default: "alert" attr :type, :string, required: true attr :class, :string, default: "" @@ -30,13 +39,7 @@ defmodule LightningWeb.Components.Common do _info -> "blue" end - icon = - case assigns.type do - "success" -> "hero-check-circle-solid" - "warning" -> "hero-exclamation-triangle-solid" - "error" -> "hero-x-circle-solid" - _info -> "hero-information-circle-solid" - end + icon = select_icon(assigns.type) assigns = assign(assigns, @@ -49,7 +52,7 @@ defmodule LightningWeb.Components.Common do
- <.icon name={@icon} class={"mb-1 h-5 w-5 text-#{@color}-400"} /> + <.icon name={@icon} class={"h-5 w-5 text-#{@color}-400"} />
- <%= render_slot(@link_right) %>

- <% else %> + <% end %> + <%= if Enum.count(@actions) > 0 do %>
<%= for item <- @actions do %> @@ -104,6 +107,58 @@ defmodule LightningWeb.Components.Common do """ end + attr :id, :string, default: "banner" + + attr :type, :string, + default: "info", + values: ["info", "success", "warning", "danger"] + + attr :class, :string, default: "" + attr :message, :string, required: true + attr :centered, :boolean, default: false + attr :icon, :boolean, default: false + attr :action, :map, required: false, default: nil + attr :dismissable, :boolean, default: false + + @doc """ + Banners can sometimes be dismissed, take the full width, and can optionally + provide a link or button to perform a single action. + """ + def banner(assigns) do + assigns = + assign(assigns, + class: ["alert-#{assigns.type}" | List.wrap(assigns.class)], + icon_name: select_icon(assigns.type) + ) + + ~H""" +
+

+ <%= if @icon == true do %> + <.icon name={@icon_name} class="h-5 w-5 align-middle mr-1" /> + <% end %> + <%= @message %> + <%= if assigns[:action] do %> + + <%= @action.text %> + + + <% end %> +

+
+ <%!-- todo - add if/when we have a dismissable banner --%> +
+
+ """ + end + attr :id, :string, required: true attr :version, :string, required: true attr :tooltip, :string, required: false diff --git a/lib/lightning_web/live/project_live/settings.ex b/lib/lightning_web/live/project_live/settings.ex index be30bbaaf6..a39a2ad262 100644 --- a/lib/lightning_web/live/project_live/settings.ex +++ b/lib/lightning_web/live/project_live/settings.ex @@ -524,7 +524,7 @@ defmodule LightningWeb.ProjectLive.Settings do else: "Disabled" %> <% true -> %> - Disabled + Unavailable <% end %> """ end diff --git a/lib/lightning_web/live/workflow_live/edit.ex b/lib/lightning_web/live/workflow_live/edit.ex index c1896162ab..db9b687d40 100644 --- a/lib/lightning_web/live/workflow_live/edit.ex +++ b/lib/lightning_web/live/workflow_live/edit.ex @@ -442,11 +442,14 @@ defmodule LightningWeb.WorkflowLive.Edit do ~p"/projects/#{@project.id}/w/#{@workflow.id}?s=#{@selected_job.id}" } /> - <.workflow_info_banner + <.form id="workflow-form" diff --git a/lib/lightning_web/plugs/redirect.ex b/lib/lightning_web/plugs/redirect.ex index 5e22c52368..3630eb8afb 100644 --- a/lib/lightning_web/plugs/redirect.ex +++ b/lib/lightning_web/plugs/redirect.ex @@ -5,8 +5,7 @@ defmodule LightningWeb.Plugs.Redirect do This plug takes an option `:to` which specifies the target URL for the redirection. """ - use Phoenix.Controller - import Plug.Conn + use LightningWeb, :controller @doc """ Initializes the plug options. diff --git a/lib/lightning_web/router.ex b/lib/lightning_web/router.ex index bb2d380c28..4a9fdd3323 100644 --- a/lib/lightning_web/router.ex +++ b/lib/lightning_web/router.ex @@ -113,6 +113,8 @@ defmodule LightningWeb.Router do UserConfirmationController, :confirm_email + get "/users/send-confirmation-email", UserConfirmationController, :send_email + live_session :auth, on_mount: LightningWeb.InitAssigns do live "/auth/confirm_access", ReAuthenticateLive.New, :new end diff --git a/test/lightning/accounts/user_notifier_test.exs b/test/lightning/accounts/user_notifier_test.exs index 2cf5fd7817..b46d7da6e8 100644 --- a/test/lightning/accounts/user_notifier_test.exs +++ b/test/lightning/accounts/user_notifier_test.exs @@ -61,6 +61,32 @@ defmodule Lightning.Accounts.UserNotifierTest do ) end + test "remind_account_confirmation/2" do + token = "sometoken" + + UserNotifier.remind_account_confirmation( + %User{ + email: "real@email.com", + first_name: "Real" + }, + token + ) + + url = + LightningWeb.Router.Helpers.user_confirmation_url( + LightningWeb.Endpoint, + :edit, + token + ) + + assert_email_sent( + subject: "Confirm your OpenFn account", + to: "real@email.com", + text_body: + "Hello Real,\n\nPlease confirm your OpenFn account by clicking on the URL below:\n\n#{url}\n\nIf you have not requested an account confirmation email, please ignore this.\n\nOpenFn\n" + ) + end + test "deliver_confirmation_instructions/2" do token = "sometoken" diff --git a/test/lightning/accounts_test.exs b/test/lightning/accounts_test.exs index c56f566df7..1173c3c218 100644 --- a/test/lightning/accounts_test.exs +++ b/test/lightning/accounts_test.exs @@ -17,6 +17,26 @@ defmodule Lightning.AccountsTest do import Lightning.AccountsFixtures import Lightning.Factories + test "confirmation_required?/1 returns false for users who are already confirmed" do + user = insert(:user, confirmed_at: DateTime.utc_now()) + refute Accounts.confirmation_required?(user) + end + + test "confirmation_required?/1 returns false for users who just created their accounts before 48 hours" do + user = insert(:user, confirmed_at: nil, inserted_at: DateTime.utc_now()) + refute Accounts.confirmation_required?(user) + end + + test "confirmation_required?/1 returns false for users who created their accounts more than 48 hours ago and haven't confirmed them" do + user = + insert(:user, + confirmed_at: nil, + inserted_at: DateTime.utc_now() |> Timex.shift(hours: -50) + ) + + assert Accounts.confirmation_required?(user) + end + test "has_activity_in_projects?/1 returns true if user has activity in a project (is associated with a run) and false otherwise." do user = insert(:user) another_user = insert(:user) diff --git a/test/lightning_web/controllers/user_confirmation_controller_test.exs b/test/lightning_web/controllers/user_confirmation_controller_test.exs index d5e3b8566b..6a61882da8 100644 --- a/test/lightning_web/controllers/user_confirmation_controller_test.exs +++ b/test/lightning_web/controllers/user_confirmation_controller_test.exs @@ -4,6 +4,8 @@ defmodule LightningWeb.UserConfirmationControllerTest do alias Lightning.Accounts alias Lightning.Repo import Lightning.AccountsFixtures + import Lightning.Factories + import Swoosh.TestAssertions setup do %{user: user_fixture()} @@ -17,6 +19,44 @@ defmodule LightningWeb.UserConfirmationControllerTest do end end + describe "GET /users/send-confirmation-email" do + test "sends confirmation email to the logged in user when that user has not confirmed their accounts", + %{conn: conn} do + user = insert(:user, confirmed_at: nil) + + conn = + conn + |> log_in_user(user) + |> get("/users/send-confirmation-email") + + assert redirected_to(conn) == "/projects" + + assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ + "Confirmation email sent successfully" + + assert_email_sent( + subject: "Confirm your OpenFn account", + to: user.email + ) + end + + test "doesn't sends confirmation email to the logged in user when that user has confirmed their accounts", + %{conn: conn} do + user = insert(:user, confirmed_at: DateTime.utc_now()) + + conn = + conn + |> log_in_user(user) + |> get("/users/send-confirmation-email") + + assert redirected_to(conn) == "/projects" + + refute Phoenix.Flash.get(conn.assigns.flash, :info) + + refute_email_sent(subject: "Confirm your OpenFn account") + end + end + describe "POST /users/confirm" do @tag :capture_log test "sends a new confirmation token", %{conn: conn, user: user} do diff --git a/test/lightning_web/live/accounts_confirmation_live_test.exs b/test/lightning_web/live/accounts_confirmation_live_test.exs new file mode 100644 index 0000000000..afe39c5a8d --- /dev/null +++ b/test/lightning_web/live/accounts_confirmation_live_test.exs @@ -0,0 +1,40 @@ +defmodule LightningWeb.AccountsConfirmationLiveTest do + use LightningWeb.ConnCase, async: true + + import Phoenix.LiveViewTest + import Lightning.Factories + + test "Users who have their accounts confirmed do not see the banner nor the modal in the sessions", + %{conn: conn} do + user = insert(:user, confirmed_at: DateTime.utc_now()) + + {:ok, view, _html} = conn |> log_in_user(user) |> live("/projects") + + refute view |> has_element?("#account-confirmation-alert") + refute view |> has_element?("#account-confirmation-modal") + end + + test "Users who have their account not confirmed but created them within 48 hours only see the confirmation alert not the modal", + %{conn: conn} do + user = insert(:user, confirmed_at: nil, inserted_at: DateTime.utc_now()) + + {:ok, view, _html} = conn |> log_in_user(user) |> live("/projects") + + assert view |> has_element?("#account-confirmation-alert") + refute view |> has_element?("#account-confirmation-modal") + end + + test "Users who have their account not confirmed but created them after 48 hours see both the confirmation alert and the modal", + %{conn: conn} do + user = + insert(:user, + confirmed_at: nil, + inserted_at: DateTime.utc_now() |> Timex.shift(hours: -50) + ) + + {:ok, view, _html} = conn |> log_in_user(user) |> live("/projects") + + refute view |> has_element?("#account-confirmation-alert") + assert view |> has_element?("#account-confirmation-modal") + end +end diff --git a/test/lightning_web/live/project_live_test.exs b/test/lightning_web/live/project_live_test.exs index 83a0a58d14..94cfcfb849 100644 --- a/test/lightning_web/live/project_live_test.exs +++ b/test/lightning_web/live/project_live_test.exs @@ -2675,10 +2675,10 @@ defmodule LightningWeb.ProjectLiveTest do # form does not exist refute has_element?(view, "form#failure-alert-#{project_user.id}") - # status is displayed as disabled even though it is enabled on the project user + # status is displayed as Unavailable even though it is enabled on the project user assert view |> element("#failure-alert-status-#{project_user.id}") - |> render() =~ "Disabled" + |> render() =~ "Unavailable" end end