diff --git a/lib/asciinema.ex b/lib/asciinema.ex index 0c8f4a3d4..fdb69d463 100644 --- a/lib/asciinema.ex +++ b/lib/asciinema.ex @@ -32,9 +32,8 @@ defmodule Asciinema do def send_account_deletion_email(user) do token = Accounts.generate_deletion_token(user) - Emails.send_email(:account_deletion, user.email, token) - :ok + Emails.send_email(:account_deletion, user.email, token) end defdelegate verify_login_token(token), to: Accounts diff --git a/lib/asciinema/emails.ex b/lib/asciinema/emails.ex index 74d8826f5..16157747c 100644 --- a/lib/asciinema/emails.ex +++ b/lib/asciinema/emails.ex @@ -1,47 +1,33 @@ defmodule Asciinema.Emails do alias Asciinema.Emails.{Email, Mailer} - defmodule Job do - use Oban.Worker, queue: :emails - - @impl Oban.Worker - def perform(job) do - case job.args["type"] do - "signup" -> - job.args["to"] - |> Email.signup_email(job.args["token"]) - |> deliver() - - "login" -> - job.args["to"] - |> Email.login_email(job.args["token"]) - |> deliver() - - "account_deletion" -> - job.args["to"] - |> Email.account_deletion_email(job.args["token"]) - |> deliver() - end - - :ok - end - - defp deliver(email) do - with {:permanent_failure, _, _} <- Mailer.deliver_now!(email) do - {:cancel, :permanent_failure} - end - end + def send_email(:signup, to, token) do + to + |> Email.signup_email(token) + |> deliver() end - def send_email(type, to, token) do - Oban.insert!(Job.new(%{type: type, to: to, token: token})) + def send_email(:login, to, token) do + to + |> Email.login_email(token) + |> deliver() + end - :ok + def send_email(:account_deletion, to, token) do + to + |> Email.account_deletion_email(token) + |> deliver() end def send_email(:test, to) do to |> Email.test_email() - |> Mailer.deliver_now!() + |> deliver() + end + + defp deliver(email) do + with {:ok, _email} <- Mailer.deliver_now(email) do + :ok + end end end diff --git a/lib/asciinema_web/controllers/login/sent.html.heex b/lib/asciinema_web/controllers/login/sent.html.heex index cb9d44139..16d7d2cfb 100644 --- a/lib/asciinema_web/controllers/login/sent.html.heex +++ b/lib/asciinema_web/controllers/login/sent.html.heex @@ -15,7 +15,7 @@

If the email doesn't show up in your inbox, check your spam folder, - or try again. + or try again in a moment.

diff --git a/lib/asciinema_web/controllers/login_controller.ex b/lib/asciinema_web/controllers/login_controller.ex index ac47d3194..27e2566c6 100644 --- a/lib/asciinema_web/controllers/login_controller.ex +++ b/lib/asciinema_web/controllers/login_controller.ex @@ -9,7 +9,7 @@ defmodule AsciinemaWeb.LoginController do end def create(%{assigns: %{bot: true}} = conn, params) do - Logger.warn("bot login attempt: #{inspect(params)}") + Logger.warning("bot login attempt: #{inspect(params)}") redirect(conn, to: ~p"/login/sent") end @@ -35,6 +35,11 @@ defmodule AsciinemaWeb.LoginController do {:error, :email_missing} -> redirect(conn, to: ~p"/login/sent") + + {:error, reason} -> + Logger.warning("email delivery error: #{inspect(reason)}") + + render(conn, :new, error: "Error sending email, please try again later.") end end diff --git a/lib/asciinema_web/controllers/user_controller.ex b/lib/asciinema_web/controllers/user_controller.ex index 95198e59d..435dbb19d 100644 --- a/lib/asciinema_web/controllers/user_controller.ex +++ b/lib/asciinema_web/controllers/user_controller.ex @@ -2,6 +2,7 @@ defmodule AsciinemaWeb.UserController do use AsciinemaWeb, :controller alias Asciinema.{Accounts, Streaming, Recordings} alias AsciinemaWeb.Auth + require Logger plug :require_current_user when action in [:edit, :update] @@ -152,10 +153,18 @@ defmodule AsciinemaWeb.UserController do user = conn.assigns.current_user address = user.email - :ok = Asciinema.send_account_deletion_email(user) + case Asciinema.send_account_deletion_email(user) do + :ok -> + conn + |> put_flash(:info, "Account removal initiated - check your inbox (#{address})") + |> redirect(to: profile_path(conn)) - conn - |> put_flash(:info, "Account removal initiated - check your inbox (#{address})") - |> redirect(to: profile_path(conn)) + {:error, reason} -> + Logger.warning("email delivery error: #{inspect(reason)}") + + conn + |> put_flash(:error, "Error sending email, please try again later") + |> redirect(to: ~p"/user/edit") + end end end diff --git a/test/asciinema_test.exs b/test/asciinema_test.exs index 939a29227..1f50e05e4 100644 --- a/test/asciinema_test.exs +++ b/test/asciinema_test.exs @@ -1,5 +1,6 @@ defmodule AsciinemaTest do import Asciinema.Factory + import Bamboo.Test use Asciinema.DataCase use Oban.Testing, repo: Asciinema.Repo alias Asciinema.Recordings @@ -28,10 +29,7 @@ defmodule AsciinemaTest do assert Asciinema.send_login_email("TEST@EXAMPLE.COM") == :ok - assert_enqueued( - worker: Asciinema.Emails.Job, - args: %{"type" => "login", "to" => "test@example.com"} - ) + assert_email_delivered_with(to: [{nil, "test@example.com"}], subject: "Login to localhost") end test "existing user, by username" do @@ -39,37 +37,34 @@ defmodule AsciinemaTest do assert Asciinema.send_login_email("foobar") == :ok - assert_enqueued( - worker: Asciinema.Emails.Job, - args: %{"type" => "login", "to" => "foobar123@example.com"} + assert_email_delivered_with( + to: [{nil, "foobar123@example.com"}], + subject: "Login to localhost" ) end test "non-existing user, by email" do assert Asciinema.send_login_email("NEW@EXAMPLE.COM") == :ok - assert_enqueued( - worker: Asciinema.Emails.Job, - args: %{"type" => "signup", "to" => "new@example.com"} - ) + assert_email_delivered_with(to: [{nil, "new@example.com"}], subject: "Welcome to localhost") end test "non-existing user, by email, when sign up is disabled" do assert Asciinema.send_login_email("new@example.com", false) == {:error, :user_not_found} - refute_enqueued(worker: Asciinema.Emails.Job) + assert_no_emails_delivered() end test "non-existing user, by email, when email is invalid" do assert Asciinema.send_login_email("new@") == {:error, :email_invalid} - refute_enqueued(worker: Asciinema.Emails.Job) + assert_no_emails_delivered() end test "non-existing user, by username" do assert Asciinema.send_login_email("idontexist") == {:error, :user_not_found} - refute_enqueued(worker: Asciinema.Emails.Job) + assert_no_emails_delivered() end end diff --git a/test/controllers/login_controller_test.exs b/test/controllers/login_controller_test.exs index c21482a86..f05d06664 100644 --- a/test/controllers/login_controller_test.exs +++ b/test/controllers/login_controller_test.exs @@ -1,41 +1,47 @@ defmodule Asciinema.LoginControllerTest do use AsciinemaWeb.ConnCase use Oban.Testing, repo: Asciinema.Repo + import Bamboo.Test @honeypot_detection_header "x-melliculum" test "with valid email", %{conn: conn} do conn = post conn, "/login", %{login: %{email: "new@example.com", username: ""}} + assert redirected_to(conn, 302) == "/login/sent" assert get_resp_header(conn, @honeypot_detection_header) == [] - assert_enqueued(worker: Asciinema.Emails.Job, args: %{"to" => "new@example.com"}) + assert_email_delivered_with(to: [{nil, "new@example.com"}]) end test "with valid email (uppercase)", %{conn: conn} do conn = post conn, "/login", %{login: %{email: "NEW@EXAMPLE.COM", username: ""}} + assert redirected_to(conn, 302) == "/login/sent" assert get_resp_header(conn, @honeypot_detection_header) == [] - assert_enqueued(worker: Asciinema.Emails.Job, args: %{"to" => "new@example.com"}) + assert_email_delivered_with(to: [{nil, "new@example.com"}]) end test "with invalid email", %{conn: conn} do conn = post conn, "/login", %{login: %{email: "new@", username: ""}} + assert html_response(conn, 200) =~ "correct email" assert get_resp_header(conn, @honeypot_detection_header) == [] - refute_enqueued(worker: Asciinema.Emails.Job) + assert_no_emails_delivered() end test "as bot with username", %{conn: conn} do conn = post conn, "/login", %{login: %{email: "bot@example.com", username: "bot"}} + assert redirected_to(conn, 302) == "/login/sent" assert List.first(get_resp_header(conn, @honeypot_detection_header)) == "machina" - refute_enqueued(worker: Asciinema.Emails.Job) + assert_no_emails_delivered() end test "as bot with terms", %{conn: conn} do conn = post conn, "/login", %{login: %{email: "bot@example.com", username: "", terms: "1"}} + assert redirected_to(conn, 302) == "/login/sent" assert List.first(get_resp_header(conn, @honeypot_detection_header)) == "machina" - refute_enqueued(worker: Asciinema.Emails.Job) + assert_no_emails_delivered() end end diff --git a/test/controllers/user_controller_test.exs b/test/controllers/user_controller_test.exs index aee561e3a..e09a7a2f2 100644 --- a/test/controllers/user_controller_test.exs +++ b/test/controllers/user_controller_test.exs @@ -2,6 +2,7 @@ defmodule Asciinema.UserControllerTest do use AsciinemaWeb.ConnCase use Oban.Testing, repo: Asciinema.Repo import Asciinema.Factory + import Bamboo.Test alias Asciinema.Accounts describe "sign-up" do @@ -133,14 +134,14 @@ defmodule Asciinema.UserControllerTest do describe "account deletion" do test "phase 1", %{conn: conn} do - user = insert(:user) + user = insert(:user, email: "test@example.com") conn = log_in(conn, user) conn = delete(conn, ~p"/user") assert response(conn, 302) assert flash(conn, :info) =~ ~r/initiated/i - assert_enqueued(worker: Asciinema.Emails.Job, args: %{"type" => "account_deletion"}) + assert_email_delivered_with(to: [{nil, "test@example.com"}], subject: "Account deletion") end test "phase 2", %{conn: conn} do