From ca3e2e5121b8e8aefb85820348d5cfcadca83bd2 Mon Sep 17 00:00:00 2001
From: Uku Taht <Uku.taht@gmail.com>
Date: Tue, 23 May 2023 11:37:58 +0300
Subject: [PATCH] Add unique constraint to api keys (#2931)

* Add unique constraint to api keys

* Fix test

* Remove `user_id` from api key index
---
 lib/plausible/auth/api_key.ex                 |  1 +
 .../templates/auth/new_api_key.html.eex       |  7 +--
 ...516131041_add_unique_index_to_api_keys.exs |  7 +++
 .../controllers/auth_controller_test.exs      | 43 +++++++++++++++++++
 4 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 priv/repo/migrations/20230516131041_add_unique_index_to_api_keys.exs

diff --git a/lib/plausible/auth/api_key.ex b/lib/plausible/auth/api_key.ex
index edff9ad5a..adb77ddf8 100644
--- a/lib/plausible/auth/api_key.ex
+++ b/lib/plausible/auth/api_key.ex
@@ -24,6 +24,7 @@ defmodule Plausible.Auth.ApiKey do
     |> validate_required(@required)
     |> generate_key()
     |> process_key()
+    |> unique_constraint(:key_hash, error_key: :key)
   end
 
   def update(schema, attrs \\ %{}) do
diff --git a/lib/plausible_web/templates/auth/new_api_key.html.eex b/lib/plausible_web/templates/auth/new_api_key.html.eex
index e45e4eb9c..3e2b1bf53 100644
--- a/lib/plausible_web/templates/auth/new_api_key.html.eex
+++ b/lib/plausible_web/templates/auth/new_api_key.html.eex
@@ -11,9 +11,10 @@
     <%= label f, :key, class: "block text-sm font-medium text-gray-700 dark:text-gray-300" %>
     <div class="relative mt-1">
       <%= text_input f, :key, id: "key-input", class: "dark:text-gray-300 shadow-sm bg-gray-50 dark:bg-gray-850 focus:ring-indigo-500 focus:border-indigo-500 block w-full sm:text-sm border-gray-300 dark:border-gray-500 rounded-md pr-16", readonly: "readonly" %>
-        <a onclick="var textarea = document.getElementById('key-input'); textarea.focus(); textarea.select(); document.execCommand('copy');" href="javascript:void(0)" class="absolute flex items-center text-xs font-medium text-indigo-600 no-underline hover:underline"  style="top: 12px; right: 12px;">
-          <svg class="pr-1 text-indigo-600 dark:text-indigo-500" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect x="9" y="9" width="13" height="13" rx="2" ry="2"></rect><path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1"></path></svg>COPY
-        </a>
+      <a onclick="var textarea = document.getElementById('key-input'); textarea.focus(); textarea.select(); document.execCommand('copy');" href="javascript:void(0)" class="absolute flex items-center text-xs font-medium text-indigo-600 no-underline hover:underline"  style="top: 12px; right: 12px;">
+        <svg class="pr-1 text-indigo-600 dark:text-indigo-500" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect x="9" y="9" width="13" height="13" rx="2" ry="2"></rect><path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1"></path></svg>COPY
+      </a>
+      <%= error_tag f, :key %>
       <p class="mt-2 text-sm text-gray-500 dark:text-gray-200">Make sure to store the key in a secure place. Once created, we will not be able to show it again.</p>
     </div>
   </div>
diff --git a/priv/repo/migrations/20230516131041_add_unique_index_to_api_keys.exs b/priv/repo/migrations/20230516131041_add_unique_index_to_api_keys.exs
new file mode 100644
index 000000000..f15b92ab0
--- /dev/null
+++ b/priv/repo/migrations/20230516131041_add_unique_index_to_api_keys.exs
@@ -0,0 +1,7 @@
+defmodule Plausible.Repo.Migrations.AddUniqueIndexToApiKeys do
+  use Ecto.Migration
+
+  def change do
+    create unique_index(:api_keys, :key_hash)
+  end
+end
diff --git a/test/plausible_web/controllers/auth_controller_test.exs b/test/plausible_web/controllers/auth_controller_test.exs
index 8edf0c0fc..73514abe9 100644
--- a/test/plausible_web/controllers/auth_controller_test.exs
+++ b/test/plausible_web/controllers/auth_controller_test.exs
@@ -709,6 +709,49 @@ defmodule PlausibleWeb.AuthControllerTest do
     setup [:create_user, :log_in]
     import Ecto.Query
 
+    test "can create an API key", %{conn: conn, user: user} do
+      site = insert(:site)
+      insert(:site_membership, site: site, user: user, role: "owner")
+
+      conn =
+        post(conn, "/settings/api-keys", %{
+          "api_key" => %{
+            "user_id" => user.id,
+            "name" => "all your code are belong to us",
+            "key" => "swordfish"
+          }
+        })
+
+      key = Plausible.Auth.ApiKey |> where(user_id: ^user.id) |> Repo.one()
+      assert conn.status == 302
+      assert key.name == "all your code are belong to us"
+    end
+
+    test "cannot create a duplicate API key", %{conn: conn, user: user} do
+      site = insert(:site)
+      insert(:site_membership, site: site, user: user, role: "owner")
+
+      conn =
+        post(conn, "/settings/api-keys", %{
+          "api_key" => %{
+            "user_id" => user.id,
+            "name" => "all your code are belong to us",
+            "key" => "swordfish"
+          }
+        })
+
+      conn2 =
+        post(conn, "/settings/api-keys", %{
+          "api_key" => %{
+            "user_id" => user.id,
+            "name" => "all your code are belong to us",
+            "key" => "swordfish"
+          }
+        })
+
+      assert html_response(conn2, 200) =~ "has already been taken"
+    end
+
     test "can't create api key into another site", %{conn: conn, user: me} do
       my_site = insert(:site)
       insert(:site_membership, site: my_site, user: me, role: "owner")