Improve unsubscribe link to prevent accidental unsubscribes

lib/keila_web/controllers/public_form_controller.ex

@@ -126,26 +126,51 @@ defmodule KeilaWeb.PublicFormController do
 
   @default_unsubscribe_form %Contacts.Form{settings: %Contacts.Form.Settings{}}
   @spec unsubscribe(Plug.Conn.t(), map()) :: Plug.Conn.t()
-  def unsubscribe(conn, %{
+  def unsubscribe(conn = %{method: "GET"}, %{
         "project_id" => project_id,
         "recipient_id" => recipient_id,
         "hmac" => hmac
       }) do
     if Mailings.valid_unsubscribe_hmac?(project_id, recipient_id, hmac) do
-      Keila.Mailings.unsubscribe_recipient(recipient_id)
-
       form = Contacts.get_project_forms(project_id) |> List.first() || @default_unsubscribe_form
 
       conn
       |> put_meta(:title, gettext("Unsubscribe"))
       |> assign(:form, form)
+      |> assign(:project_id, project_id)
+      |> assign(:recipient_id, recipient_id)
+      |> assign(:hmac, hmac)
       |> assign(:mode, :full)
       |> render("unsubscribe.html")
     else
       conn |> put_status(404) |> halt()
     end
   end
 
+  def unsubscribe(conn = %{method: "POST"}, %{
+        "project_id" => project_id,
+        "recipient_id" => recipient_id,
+        "hmac" => hmac
+      }) do
+    # Validate HMAC and unsubscribe on any POST
+    if Mailings.valid_unsubscribe_hmac?(project_id, recipient_id, hmac) do
+      Keila.Mailings.unsubscribe_recipient(recipient_id)
+
+      form = Contacts.get_project_forms(project_id) |> List.first() || @default_unsubscribe_form
+
+      conn
+      |> put_meta(:title, gettext("Unsubscribed"))
+      |> assign(:form, form)
+      |> assign(:project_id, project_id)
+      |> assign(:recipient_id, recipient_id)
+      |> assign(:hmac, hmac)
+      |> assign(:mode, :full)
+      |> render("unsubscribe_success.html")
+    else
+      conn |> put_status(404) |> halt()
+    end
+  end
+
   # DEPRECATED: This implementation is deprecated and will be removed in a future version
   def unsubscribe(conn, %{"project_id" => project_id, "contact_id" => contact_id}) do
     form = Contacts.get_project_forms(project_id) |> List.first() || @default_unsubscribe_form
@@ -160,7 +185,7 @@ defmodule KeilaWeb.PublicFormController do
     |> put_meta(:title, gettext("Unsubscribe"))
     |> assign(:form, form)
     |> assign(:mode, :full)
-    |> render("unsubscribe.html")
+    |> render("unsubscribe_deprecated.html")
   end
 
   defp fetch(conn, _) do

lib/keila_web/templates/public_form/unsubscribe.html.heex

@@ -1 +1,16 @@
-<%= render_unsubscribe_form(@form) %>
+<div class="contact-form container bg-white rounded py-4 md:py-8 flex flex-col gap-4" style={build_form_styles(@form)}>
+  <div class="text-center">
+    <h1 class="text-2xl font-bold mb-4"><%= gettext("Unsubscribe") %></h1>
+    <p class="text-lg mb-6"><%= gettext("Are you sure you want to unsubscribe from this list?") %></p>
+
+    <form method="post" action={get_unsubscribe_action_url(assigns)}>
+      <div class="space-y-4">
+        <button 
+          type="submit"
+          class="bg-red-600 hover:bg-red-700 text-white font-bold py-3 px-6 rounded cursor-pointer">
+          <%= gettext("Unsubscribe") %>
+        </button>
+      </div>
+    </form>
+  </div>
+</div>

lib/keila_web/templates/public_form/unsubscribe_deprecated.html.heex

@@ -0,0 +1 @@
+<%= render_unsubscribe_deprecated(@form) %>

lib/keila_web/templates/public_form/unsubscribe_success.html.heex

@@ -0,0 +1,6 @@
+<div class="contact-form container bg-white rounded py-4 md:py-8 flex flex-col gap-4" style={build_form_styles(@form)}>
+  <div class="text-center">
+    <h1 class="text-2xl font-bold mb-4"><%= gettext("Unsubscribed") %></h1>
+    <p class="text-lg"><%= gettext("You have been unsubscribed from this list.") %></p>
+  </div>
+</div>

lib/keila_web/views/public_form_view.ex

@@ -83,7 +83,7 @@ defmodule KeilaWeb.PublicFormView do
     end
   end
 
-  defp build_form_styles(form) do
+  def build_form_styles(form) do
     build_styles(%{
       "background-color" => form.settings.form_bg_color,
       "color" => form.settings.text_color
@@ -344,14 +344,27 @@ defmodule KeilaWeb.PublicFormView do
     end
   end
 
-  def render_unsubscribe_form(form) do
+  def render_unsubscribe_deprecated(form) do
     form_styles = build_form_styles(form)
 
     content_tag(:div, class: @form_classes, style: form_styles) do
       gettext("You have been unsubscribed from this list.")
     end
   end
 
+  # Helper functions to handle different route types
+  def get_unsubscribe_action_url(assigns) do
+    cond do
+      Map.get(assigns, :deprecated_route) ->
+        Routes.public_form_url(KeilaWeb.Endpoint, :unsubscribe, assigns.project_id, assigns.contact_id)
+      Map.has_key?(assigns, :hmac) ->
+        Routes.public_form_url(KeilaWeb.Endpoint, :unsubscribe, assigns.project_id, assigns.recipient_id, assigns.hmac)
+      true ->
+        "" # Current page (relative post)
+    end
+  end
+
+
   defp data_field_mapping(form) do
     form.field_settings
     |> Enum.filter(&(&1.field == :data))

priv/gettext/fr/LC_MESSAGES/default.po

@@ -640,7 +640,6 @@ msgstr "Champs"
 msgid "Fine print"
 msgstr "Texte en petits caractères"
 
-#: extra/keila_cloud_web/templates/cloud_account/_user_name.html.heex:24
 #: lib/keila/contacts/contacts.ex:276
 #: lib/keila_web/live/segment_edit_live.ex:9
 #: lib/keila_web/templates/contact/_list.html.heex:32
@@ -798,7 +797,6 @@ msgstr "Italique"
 msgid "Jane"
 msgstr "Truc"
 
-#: extra/keila_cloud_web/templates/cloud_account/_user_name.html.heex:30
 #: lib/keila/contacts/contacts.ex:284
 #: lib/keila_web/live/segment_edit_live.ex:10
 #: lib/keila_web/templates/contact/_list.html.heex:42
@@ -1367,8 +1365,12 @@ msgstr "Liste"
 msgid "Unschedule"
 msgstr "Déprogrammer"
 
-#: lib/keila_web/controllers/public_form_controller.ex:140
-#: lib/keila_web/controllers/public_form_controller.ex:160
+#: lib/keila_web/controllers/public_form_controller.ex:138
+#: lib/keila_web/controllers/public_form_controller.ex:164
+#: lib/keila_web/controllers/public_form_controller.ex:182
+#: lib/keila_web/controllers/public_form_controller.ex:206
+#: lib/keila_web/views/public_form_view.ex:365
+#: lib/keila_web/views/public_form_view.ex:392
 #, elixir-autogen, elixir-format
 msgid "Unsubscribe"
 msgstr "Se désabonner"
@@ -1411,7 +1413,7 @@ msgstr "Nous traitons actuellement votre abonnement. Cela devrait prendre moins
 msgid "You currently have an active subscription. Thanks for supporting Keila!"
 msgstr "Vous avez actuellement un abonnement actif. Merci de soutenir Keila!"
 
-#: lib/keila_web/views/public_form_view.ex:351
+#: lib/keila_web/views/public_form_view.ex:359
 #, elixir-autogen, elixir-format
 msgid "You have been unsubscribed from this list."
 msgstr "Vous avez été désabonné de cette liste."
@@ -3226,13 +3228,6 @@ msgstr ""
 msgid "Address copied to clipboard."
 msgstr "Code copié dans le presse-papier."
 
-#: extra/keila_cloud_web/templates/cloud_account/_onboarding_review_data.html.heex:50
-#: extra/keila_cloud_web/templates/cloud_account/_org_name.html.heex:56
-#: extra/keila_cloud_web/templates/cloud_account/_user_name.html.heex:38
-#, elixir-autogen, elixir-format, fuzzy
-msgid "Continue"
-msgstr "Contenu"
-
 #: lib/keila_web/templates/campaign/share.html.heex:61
 #, elixir-autogen, elixir-format
 msgid "Disable public link"
@@ -3278,3 +3273,23 @@ msgstr ""
 #, elixir-autogen, elixir-format
 msgid "You can enable a public link to share your campaign with others. Your campaign will be accessible to anyone who has the link.\n\nYou can disable the public link later if you want.\n"
 msgstr ""
+
+#: lib/keila_web/views/public_form_view.ex:366
+#, elixir-autogen, elixir-format
+msgid "Are you sure you want to unsubscribe from this list?"
+msgstr ""
+
+#: lib/keila_web/views/public_form_view.ex:414
+#, elixir-autogen, elixir-format
+msgid "Are you sure you want to unsubscribe?"
+msgstr ""
+
+#: lib/keila_web/views/public_form_view.ex:386
+#, elixir-autogen, elixir-format
+msgid "JavaScript is required to unsubscribe. Please enable JavaScript and try again."
+msgstr ""
+
+#: lib/keila_web/views/public_form_view.ex:417
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Unsubscribing..."
+msgstr "Se désabonner"

test/keila_web/controllers/public_form_controller_test.exs

@@ -113,7 +113,95 @@ defmodule KeilaWeb.PublicFormControllerTest do
           )
         )
 
-      assert html_response(conn, 200) =~ "You have been unsubscribed"
+      response = html_response(conn, 200)
+      assert response =~ "Unsubscribe"
+    end
+  end
+
+  describe "HMAC-based unsubscribe routes" do
+    @describetag :public_form_controller
+
+    defp generate_test_hmac(project_id, recipient_id) do
+      key = Application.get_env(:keila, KeilaWeb.Endpoint) |> Keyword.fetch!(:secret_key_base)
+      message = "unsubscribe:" <> project_id <> ":" <> recipient_id
+
+      :crypto.mac(:hmac, :sha256, key, message)
+      |> Base.url_encode64(padding: false)
+    end
+
+    test "GET /unsubscribe/:p_id/:r_id/:hmac shows unsubscribe page", %{conn: conn} do
+      {conn, project} = with_login_and_project(conn)
+      contact = insert!(:contact, project_id: project.id, status: :active)
+      campaign = insert!(:mailings_campaign, project_id: project.id)
+      recipient = insert!(:mailings_recipient, campaign: campaign, contact: contact)
+
+      # Create a valid HMAC for testing
+      hmac = generate_test_hmac(project.id, recipient.id)
+
+      conn = get(conn, Routes.public_form_path(conn, :unsubscribe, project.id, recipient.id, hmac))
+
+      response = html_response(conn, 200)
+      assert response =~ "Unsubscribe"
+      assert response =~ "Are you sure you want to unsubscribe"
+      assert response =~ "handleUnsubscribe()"
+
+      # Contact should still be active (not auto-unsubscribed)
+      assert %{status: :active} = Contacts.get_contact(contact.id)
+    end
+
+    test "POST /unsubscribe/:p_id/:r_id/:hmac processes unsubscribe", %{conn: conn} do
+      {conn, project} = with_login_and_project(conn)
+      contact = insert!(:contact, project_id: project.id, status: :active)
+      campaign = insert!(:mailings_campaign, project_id: project.id)
+      recipient = insert!(:mailings_recipient, campaign: campaign, contact: contact)
+
+      # Create a valid HMAC for testing
+      hmac = generate_test_hmac(project.id, recipient.id)
+
+      conn = post(conn, Routes.public_form_path(conn, :unsubscribe, project.id, recipient.id, hmac), %{})
+
+      response = html_response(conn, 200)
+      assert response =~ "You have been unsubscribed"
+
+      # Check that recipient was unsubscribed
+      updated_recipient = Keila.Repo.get(Keila.Mailings.Recipient, recipient.id)
+      assert not is_nil(updated_recipient.unsubscribed_at)
+    end
+
+    test "GET /unsubscribe/:p_id/:r_id/:hmac rejects invalid HMAC", %{conn: conn} do
+      {conn, project} = with_login_and_project(conn)
+      contact = insert!(:contact, project_id: project.id, status: :active)
+      campaign = insert!(:mailings_campaign, project_id: project.id)
+      recipient = insert!(:mailings_recipient, campaign: campaign, contact: contact)
+
+      # Use invalid HMAC
+      invalid_hmac = "invalid_hmac_string"
+
+      conn = get(conn, Routes.public_form_path(conn, :unsubscribe, project.id, recipient.id, invalid_hmac))
+
+      assert conn.status == 404
+      assert %{status: :active} = Contacts.get_contact(contact.id)
+    end
+
+    test "POST /unsubscribe/:p_id/:r_id/:hmac supports List-Unsubscribe-Post", %{conn: conn} do
+      {conn, project} = with_login_and_project(conn)
+      contact = insert!(:contact, project_id: project.id, status: :active)
+      campaign = insert!(:mailings_campaign, project_id: project.id)
+      recipient = insert!(:mailings_recipient, campaign: campaign, contact: contact)
+
+      # Create a valid HMAC for testing
+      hmac = generate_test_hmac(project.id, recipient.id)
+
+      # Test with List-Unsubscribe=One-Click parameter (used by email clients)
+      conn = post(conn, Routes.public_form_path(conn, :unsubscribe, project.id, recipient.id, hmac), 
+                  %{"List-Unsubscribe" => "One-Click"})
+
+      response = html_response(conn, 200)
+      assert response =~ "You have been unsubscribed"
+
+      # Check that recipient was unsubscribed
+      updated_recipient = Keila.Repo.get(Keila.Mailings.Recipient, recipient.id)
+      assert not is_nil(updated_recipient.unsubscribed_at)
     end
   end
 end