Create an API key with `POST /api/api-key` (#36595)

This creates a new endpoint, `POST /api/api-key` that takes two arguments, a `group_id` and a `name`, and creates an
API key (along with a special API key user).

I also went ahead and threw in the `api-key/count` endpoint here since
it's so small.

enterprise/backend/src/metabase_enterprise/serialization/v2/models.clj

@@ -37,6 +37,7 @@
 (def excluded-models
   "List of models which are not going to be serialized ever."
   ["Activity"
+   "ApiKey"
    "ApplicationPermissionsRevision"
    "AuditLog"
    "BookmarkOrdering"

enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj

@@ -32,7 +32,8 @@
   - not exported in serialization; or
   - exported as a child of something else (eg. timeline_event under timeline)
   so they don't need a generated entity_id."
-  #{:model/HTTPAction
+  #{:model/ApiKey
+    :model/HTTPAction
     :model/ImplicitAction
     :model/QueryAction
     :model/Activity

resources/migrations/001_update_migrations.yaml

@@ -4350,6 +4350,22 @@ databaseChangeLog:
               );
       rollback: # no change
 
+  - changeSet:
+      id: v49.00-010
+      author: johnswanson
+      comment: Add a name to API Keys
+      changes:
+        - addColumn:
+            tableName: api_key
+            columns:
+              - column:
+                  name: name
+                  type: varchar(254)
+                  constraints:
+                    nullable: false
+                    unique: true
+                  remarks: 'The user-defined name of the API key.'
+
   # >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
 
 

src/metabase/api/api_key.clj

+(ns metabase.api.api-key
+  "/api/api-key endpoints for CRUD management of API Keys"
+  (:require
+   [compojure.core :refer [POST GET]]
+   [metabase.api.common :as api]
+   [metabase.models.api-key :as api-key]
+   [metabase.models.permissions-group :as perms-group]
+   [metabase.models.user :as user]
+   [metabase.util :as u]
+   [metabase.util.i18n :refer [tru]]
+   [metabase.util.malli.schema :as ms]
+   [toucan2.core :as t2]))
+
+(defn- key-with-unique-prefix []
+  (u/auto-retry 5
+   (let [api-key (api-key/generate-key)
+         prefix (api-key/prefix api-key)]
+     ;; we could make this more efficient by generating 5 API keys up front and doing one select to remove any
+     ;; duplicates. But a duplicate should be rare enough to just do multiple queries for now.
+     (if-not (t2/exists? :model/ApiKey :key_prefix prefix)
+       api-key
+       (throw (ex-info (tru "could not generate key with unique prefix") {}))))))
+
+(api/defendpoint POST "/"
+  "Create a new API key (and an associated `User`) with the provided name and group ID."
+  [:as {{:keys [group_id name] :as _body} :body}]
+  {group_id ms/PositiveInt
+   name     ms/NonBlankString}
+  (api/check-superuser)
+  (api/checkp (not (t2/exists? :model/ApiKey :name name))
+    "name" "An API key with this name already exists.")
+  (let [api-key (key-with-unique-prefix)
+        email   (format "api-key-user-%s@api-key.invalid" (u/slugify name))]
+    (t2/with-transaction [_conn]
+      (let [user (user/insert-new-user! {:email      email
+                                         :first_name name
+                                         :type       :api-key})]
+        (user/set-permissions-groups! user [(perms-group/all-users) group_id])
+        (-> (t2/insert-returning-instance! :model/ApiKey
+                                           {:user_id      (u/the-id user)
+                                            :name         name
+                                            :unhashed_key api-key
+                                            :created_by   api/*current-user-id*})
+            (select-keys [:created_at :updated_at :id])
+            (assoc :name name
+                   :group_id group_id
+                   :unmasked_key api-key
+                   :masked_key (api-key/mask api-key)))))))
+
+(api/defendpoint GET "/count"
+  "Get the count of API keys in the DB"
+  [:as _body]
+  (api/check-superuser)
+  (t2/count :model/ApiKey))
+
+(api/define-routes)

src/metabase/api/routes.clj

@@ -5,6 +5,7 @@
    [metabase.api.action :as api.action]
    [metabase.api.activity :as api.activity]
    [metabase.api.alert :as api.alert]
+   [metabase.api.api-key :as api.api-key]
    [metabase.api.automagic-dashboards :as api.magic]
    [metabase.api.bookmark :as api.bookmark]
    [metabase.api.card :as api.card]
@@ -113,5 +114,6 @@
   (context "/timeline-event"       [] (+auth api.timeline-event/routes))
   (context "/transform"            [] (+auth api.transform/routes))
   (context "/user"                 [] (+auth api.user/routes))
+  (context "/api-key"              [] (+auth api.api-key/routes))
   (context "/util"                 [] api.util/routes)
   (route/not-found (constantly {:status 404, :body (deferred-tru "API endpoint does not exist.")})))

src/metabase/api/user.clj

@@ -218,7 +218,9 @@
                     (vec (cons User (user-visible-columns)))
                     (cond-> clauses
                       (and (some? group_id) group-id-clause) (sql.helpers/order-by [:core_user.is_superuser :desc] [:is_group_manager :desc])
-                      true             (sql.helpers/order-by [:%lower.first_name :asc] [:%lower.last_name :asc])))
+                      true             (sql.helpers/order-by [:%lower.first_name :asc]
+                                                             [:%lower.last_name :asc]
+                                                             [:id :asc])))
              ;; For admins also include the IDs of Users' Personal Collections
              api/*is-superuser?*
              (t2/hydrate :personal_collection_id)

src/metabase/models/api_key.clj

+(ns metabase.models.api-key
+  (:require [crypto.random :as crypto-random]
+            [metabase.util.password :as u.password]
+            [methodical.core :as methodical]
+            [toucan2.core :as t2]))
+
+;; the prefix length, the length of `mb_1234`
+(def ^:private prefix-length 7)
+
+;; the total number of bytes of randomness we generate for API keys
+(def ^:private bytes-key-length 32)
+
+(methodical/defmethod t2/table-name :model/ApiKey [_model] :api_key)
+
+(doto :model/ApiKey
+  (derive :metabase/model)
+  (derive :hook/timestamped?))
+
+(defn prefix
+  "Given an API key, returns the standardized prefix for that API key."
+  [key]
+  (apply str (take prefix-length key)))
+
+(defn- add-prefix [{:keys [unhashed_key] :as api-key}]
+  (cond-> api-key
+    unhashed_key (assoc :key_prefix (prefix unhashed_key))))
+
+(defn mask
+  "Given an API key, returns a string of the same length with all but the prefix masked with `*`s"
+  [key]
+  (->> (concat (prefix key) (repeat "*"))
+       (take (count key))
+       (apply str)))
+
+(defn generate-key
+  "Generates a new API key - a random base64 string prefixed with `mb_`"
+  []
+  (str "mb_" (crypto-random/base64 bytes-key-length)))
+
+(defn- add-key
+  "Adds the `key` based on the `unhashed_key` passed in."
+  [{:keys [unhashed_key] :as api-key}]
+  (cond-> api-key
+    unhashed_key (assoc :key (u.password/hash-bcrypt unhashed_key))
+    true (dissoc :unhashed_key)))
+
+(t2/define-before-insert :model/ApiKey
+  [api-key]
+  (-> api-key
+      add-prefix
+      add-key))
+
+(t2/define-before-update :model/ApiKey
+  [api-key]
+  (-> api-key
+      add-prefix
+      add-key))

src/metabase/models/user.clj

@@ -56,7 +56,7 @@
    :type             mi/transform-keyword})
 
 (def ^:private allowed-user-types
-  #{:internal :personal})
+  #{:internal :personal :api-key})
 
 (def ^:private insert-default-values
   {:date_joined  :%now
@@ -326,7 +326,8 @@
    [:email                             ms/Email]
    [:password         {:optional true} [:maybe ms/NonBlankString]]
    [:login_attributes {:optional true} [:maybe LoginAttributes]]
-   [:sso_source       {:optional true} [:maybe ms/NonBlankString]]])
+   [:sso_source       {:optional true} [:maybe ms/NonBlankString]]
+   [:type             {:optional true} [:maybe ms/KeywordOrString]]])
 
 (def ^:private Invitor
   "Map with info about the admin creating the user, used in the new user notification code"
@@ -334,7 +335,7 @@
    [:email      ms/Email]
    [:first_name [:maybe ms/NonBlankString]]])
 
-(mu/defn ^:private insert-new-user!
+(mu/defn insert-new-user!
   "Creates a new user, defaulting the password when not provided"
   [new-user :- NewUser]
   (first (t2/insert-returning-instances! User (update new-user :password #(or % (str (random-uuid)))))))

test/metabase/api/api_key_test.clj

+(ns ^:mb/once metabase.api.api-key-test
+  "Tests for /api/api-key endpoints"
+  (:require
+   [clojure.test :refer [deftest testing is]]
+   [metabase.models.api-key :as api-key]
+   [metabase.models.permissions-group :as perms-group]
+   [metabase.test :as mt]
+   [toucan2.tools.with-temp :as t2.with-temp]))
+
+(deftest api-key-creation-test
+  (t2.with-temp/with-temp [:model/PermissionsGroup {group-id :id} {:name "Cool Friends"}]
+    (testing "POST /api/api-key works"
+      (let [name (str (random-uuid))
+            resp (mt/user-http-request :crowberto :post 200 "api-key"
+                                       {:group_id group-id
+                                        :name     name})]
+        (is (= #{:name :group_id :unmasked_key :masked_key :id :created_at :updated_at}
+               (-> resp keys set)))
+        (is (= name (:name resp)))))
+    (testing "Trying to create another API key with the same name fails"
+      (let [key-name (str (random-uuid))]
+        ;; works once...
+        (is (= #{:unmasked_key :masked_key :group_id :name :id :created_at :updated_at}
+               (set (keys (mt/user-http-request :crowberto :post 200 "api-key"
+                                                {:group_id group-id
+                                                 :name     key-name})))))
+        (is (= {:errors {:name "An API key with this name already exists."}}
+               (mt/user-http-request :crowberto :post 400 "api-key"
+                                     {:group_id group-id
+                                      :name     key-name})))))
+    (testing "API key generation is retried if a prefix collision occurs"
+      ;; mock out the `api-key/generate-key` function to generate the same key (with the same prefix) repeatedly
+      (let [generated-key  (api-key/generate-key)
+            generated-keys (atom [generated-key
+                                  generated-key
+                                  generated-key
+                                  generated-key
+                                  (api-key/generate-key)])]
+        (with-redefs [api-key/generate-key (fn [] (let [next-val (first @generated-keys)]
+                                                    (swap! generated-keys next)
+                                                    next-val))]
+          ;; put an API Key in the database with that key.
+          (t2.with-temp/with-temp [:model/ApiKey _ {:unhashed_key generated-key
+                                                    :name         "my cool name"
+                                                    :user_id      (mt/user->id :crowberto)
+                                                    :created_by   (mt/user->id :crowberto)}]
+            ;; this will try to generate a new API key
+            (mt/user-http-request :crowberto :post 200 "api-key"
+                                  {:group_id group-id
+                                   :name     (str (random-uuid))})
+            ;; we've exhausted the `generated-keys` we mocked
+            (is (empty? @generated-keys))))))
+    (testing "We don't retry forever if prefix collision keeps happening"
+      (let [generated-key (api-key/generate-key)]
+        (with-redefs [api-key/generate-key (constantly generated-key)]
+          (t2.with-temp/with-temp [:model/ApiKey _ {:unhashed_key generated-key
+                                                    :name         "my cool name"
+                                                    :user_id      (mt/user->id :crowberto)
+                                                    :created_by   (mt/user->id :crowberto)}]
+            (is (= "could not generate key with unique prefix"
+                   (:message (mt/user-http-request :crowberto :post 500 "api-key"
+                                                   {:group_id group-id
+                                                    :name     (str (random-uuid))}))))))))
+    (testing "A group is required"
+      (is (= {:errors          {:group_id "value must be an integer greater than zero."}
+              :specific-errors {:group_id ["value must be an integer greater than zero., received: nil"]}}
+             (mt/user-http-request :crowberto :post 400 "api-key"
+                                   {:group_id nil
+                                    :name     (str (random-uuid))}))))
+    (testing "The group can be 'All Users'"
+      (is (mt/user-http-request :crowberto :post 200 "api-key"
+                                {:group_id (:id (perms-group/all-users))
+                                 :name     (str (random-uuid))})))
+    (testing "A non-empty name is required"
+      (is (= {:errors          {:name "value must be a non-blank string."}
+              :specific-errors {:name ["should be at least 1 characters, received: \"\"" "non-blank string, received: \"\""]}}
+             (mt/user-http-request :crowberto :post 400 "api-key"
+                                   {:group_id group-id
+                                    :name     ""}))))
+    (testing "A non-blank name is required"
+      (is (= {:errors          {:name "value must be a non-blank string."}
+              :specific-errors {:name ["non-blank string, received: \"   \""]}}
+             (mt/user-http-request :crowberto :post 400 "api-key"
+                                   {:group_id group-id
+                                    :name     "   "}))))))
+
+(deftest api-count-works
+  (mt/with-empty-h2-app-db
+    (is (zero? (mt/user-http-request :crowberto :get 200 "api-key/count")))
+    (t2.with-temp/with-temp [:model/ApiKey _ {:unhashed_key "prefix_key"
+                                              :name         "my cool name"
+                                              :user_id      (mt/user->id :crowberto)
+                                              :created_by   (mt/user->id :crowberto)}]
+      (is (= 1 (mt/user-http-request :crowberto :get 200 "api-key/count")))
+      (t2.with-temp/with-temp [:model/ApiKey _ {:unhashed_key "some_other_key"
+                                                :name         "my cool OTHER name"
+                                                :user_id      (mt/user->id :crowberto)
+                                                :created_by   (mt/user->id :crowberto)}]
+        (is (= 2 (mt/user-http-request :crowberto :get 200 "api-key/count")))))))

test/metabase/api/user_test.clj

@@ -66,9 +66,10 @@
         (let [result (->> ((mt/user-http-request :crowberto :get 200 "user") :data)
                           (filter mt/test-user?))]
           ;; since this is an admin, all keys are available on each user
-          (is (= (set (concat
-                       user/admin-or-self-visible-columns
-                       [:common_name :group_ids :personal_collection_id]))
+          (is (= (set
+                  (concat
+                   user/admin-or-self-visible-columns
+                   [:common_name :group_ids :personal_collection_id]))
                  (->> result first keys set)))
           ;; just make sure all users are there by checking the emails
           (is (= #{"crowberto@metabase.com"

test/metabase/cmd/copy_test.clj

@@ -25,7 +25,8 @@
 
 (def ^:private models-to-exclude
   "Models that should *not* be migrated in `load-from-h2`."
-  #{:model/TaskHistory
+  #{:model/ApiKey
+    :model/TaskHistory
     :model/Query
     :model/QueryCache
     :model/QueryExecution