From 1655377c40927316c15cfd225dda5470cb801efb Mon Sep 17 00:00:00 2001 From: Toby Crawley Date: Sat, 17 Feb 2024 15:29:45 -0500 Subject: [PATCH] Add admin helper to soft-delete a user --- src/clojars/admin.clj | 29 +++++++++++- src/clojars/db.clj | 78 ++++++++++++++++++++++++++------ src/clojars/web/user.clj | 2 +- test/clojars/unit/admin_test.clj | 67 +++++++++++++++++++++++++++ test/clojars/unit/db_test.clj | 62 +++++++++++++------------ 5 files changed, 192 insertions(+), 46 deletions(-) diff --git a/src/clojars/admin.clj b/src/clojars/admin.clj index 7eb8cc22..d9934b7c 100644 --- a/src/clojars/admin.clj +++ b/src/clojars/admin.clj @@ -9,7 +9,8 @@ [clojure.java.shell :as shell] [clojure.pprint :refer [pprint]] [clojure.string :as str] - [clojure.tools.nrepl.server :as nrepl]) + [clojure.tools.nrepl.server :as nrepl] + [next.jdbc :as jdbc]) (:import java.text.SimpleDateFormat)) @@ -175,6 +176,32 @@ (println "User is *not* an active member of the group.")))) (println "Clojars TXT record not found.")))) +(defn delete-user! + "This is really a soft-deletion, where the user is mostly left in place, but has + a new username of the form `deleted_` everywhere appropriate. + + Returns the new username." + [old-username] + (let [user-record (db/find-user *db* old-username) + _ (when (nil? user-record) + (throw (ex-info (format "User %s not found!" old-username) {}))) + user-id (:id user-record) + new-username (format "deleted_%s" (System/currentTimeMillis)) + empty-default-groups (into [] + (filter + #(empty? (db/jars-by-groupname *db* %))) + (db/default-user-groups old-username))] + (jdbc/with-transaction [tx *db*] + (db/disable-otp! tx old-username) + (db/clear-password-reset-code! tx old-username) + (db/disable-deploy-tokens-for-user tx user-id) + (db/rename-user tx old-username new-username) + ;; Clear email, set random password + (db/update-user tx new-username "" + (db/hexadecimalize (db/generate-secure-token 20))) + (run! (fn [group] (db/delete-group tx group)) empty-default-groups)) + new-username)) + (defn handler [mapping] (nrepl/default-handler (with-meta diff --git a/src/clojars/db.clj b/src/clojars/db.clj index 4d1bc609..d0e01541 100644 --- a/src/clojars/db.clj +++ b/src/clojars/db.clj @@ -672,6 +672,11 @@ :added_by added-by :admin (boolean admin?)})) +(defn default-user-groups + [username] + (mapv #(format "%s.clojars.%s" % username) + ["net" "org"])) + (defn add-user [db email username password] (let [record {:email email @@ -680,24 +685,44 @@ :send_deploy_emails true :created (get-time)}] (sql/insert! db :users (set/rename-keys record {:username user-column})) - (doseq [groupname [(str "net.clojars." username) - (str "org.clojars." username)]] + (doseq [groupname (default-user-groups username)] (add-member* db groupname SCOPE-ALL username "clojars" true) (verify-group! db username groupname)) record)) (defn update-user - [db account email username password] - (let [fields {:email email - :username username - :account account}] - (sql/update! db :users - (cond-> fields - true (set/rename-keys {:username user-column}) - true (dissoc :account) - (seq password) (assoc :password (bcrypt password))) - {user-column account}) - fields)) + [db account email password] + (sql/update! db :users + (cond-> {:email email} + (seq password) (assoc :password (bcrypt password))) + {user-column account}) + {:email email + :username account + :account account}) + +(defn rename-user + [db old-name new-name] + (jdbc/with-transaction [tx db] + (sql/update! tx :users + {user-column new-name} + {user-column old-name}) + (sql/update! tx :permissions + {user-column new-name} + {user-column old-name}) + (sql/update! tx :permissions + {:added_by new-name} + {:added_by old-name}) + (sql/update! tx :group_verifications + {:verified_by new-name} + {:verified_by old-name}) + (sql/update! tx :jars + {user-column new-name} + {user-column old-name}) + (sql/update! tx :audit + {user-column new-name} + {user-column old-name}) + {:username new-name + :account new-name})) (defn update-user-notifications [db account prefs] @@ -739,6 +764,12 @@ {user-column username}) reset-code)) +(defn clear-password-reset-code! + [db username] + (sql/update! db :users + {:password_reset_code nil} + {user-column username})) + (defn set-otp-secret-key! [db username] (let [secret-key (ot/generate-secret-key)] @@ -793,6 +824,14 @@ (update record :token bcrypt)) record)) +(defn find-deploy-tokens-for-user + [db user-id] + (mapv #(dissoc % :token) + (q db + {:select :* + :from :deploy_tokens + :where [:= :user_id user-id]}))) + (defn consume-deploy-token [db token-id] (sql/update! db :deploy_tokens @@ -807,6 +846,13 @@ :updated (get-time)} {:id token-id})) +(defn disable-deploy-tokens-for-user + [db user-id] + (sql/update! db :deploy_tokens + {:disabled true + :updated (get-time)} + {:user_id user-id})) + (defn set-deploy-token-used [db token-id] (sql/update! db :deploy_tokens @@ -1002,8 +1048,10 @@ ;; does not delete jars in the group. should it? (defn delete-group - [db group-id] - (sql/delete! db :permissions {:group_name group-id})) + [db groupname] + (jdbc/with-transaction [tx db] + (sql/delete! tx :permissions {:group_name groupname}) + (sql/delete! tx :group_verifications {:group_name groupname}))) (defn- find-groups-jars-information [db group-id] diff --git a/src/clojars/web/user.clj b/src/clojars/web/user.clj index 9a132616..793719e4 100644 --- a/src/clojars/web/user.clj +++ b/src/clojars/web/user.clj @@ -145,7 +145,7 @@ (let [old-email (:email (find-user-by-user-or-email db account)) email-changed? (not= old-email email) password-changed? (seq password)] - (update-user db account email account password) + (update-user db account email password) (log/info {:status :success}) (when email-changed? (event/emit event-emitter :email-changed diff --git a/test/clojars/unit/admin_test.clj b/test/clojars/unit/admin_test.clj index a5144641..ddd39a4a 100644 --- a/test/clojars/unit/admin_test.clj +++ b/test/clojars/unit/admin_test.clj @@ -23,6 +23,7 @@ (delete! [_ group# artifact#] (swap! *search-removals* conj (format "%s/%s" group# artifact#)))) admin/*storage* (storage/fs-storage (:repo (config)))] + (db/add-user admin/*db* "testuser@example.com" "testuser" "password") (help/add-verified-group "testuser" "org.ham") (db/add-jar admin/*db* "testuser" {:group "org.ham" :name "biscuit" :version "1" :description "delete me"}) (db/add-jar admin/*db* "testuser" {:group "org.ham" :name "biscuit" :version "2" :description ""}) @@ -142,3 +143,69 @@ (admin/verify-group! "testuser" "org.hambiscuit"))) (is (some #{"testuser"} (db/group-activenames help/*db* "org.hambiscuit"))))) +(deftest delete-user!-works + ;; Given: a user with: + ;; - otp + ;; - password reset code + ;; - deploy token + (db/enable-otp! admin/*db* "testuser") + (db/set-otp-secret-key! admin/*db* "testuser") + (db/set-password-reset-code! admin/*db* "testuser") + (db/add-deploy-token admin/*db* "testuser" "token" nil nil false (db/get-time)) + + ;; And: a shared default group with a jar + (db/add-admin admin/*db* "org.clojars.testuser" db/SCOPE-ALL "otheruser" "testuser") + (db/add-jar admin/*db* "testuser" {:group "org.clojars.testuser" + :name "jar" + :version "1.0"}) + + ;; And: and audit record + (db/add-audit admin/*db* "testing" "testuser" nil nil nil "a message") + + (let [user-id (:id (db/find-user admin/*db* "testuser")) + + ;; When: we mark the user as deleted + new-username (admin/delete-user! "testuser")] + + ;; Then: the user is really renamed to a deleted placeholder + (is (re-find #"deleted_\d+" new-username)) + (is (nil? (db/find-user admin/*db* "testuser"))) + + ;; And: they have no email, and otp & password recovery is cleared + (is (match? + {:email "" + :otp_active false + :otp_recovery_code nil + :otp_secret_key nil + :password_reset_code nil + :user new-username} + (db/find-user admin/*db* new-username))) + + ;; And: their deploy tokens are disabled + (is (:disabled (first (db/find-deploy-tokens-for-user admin/*db* user-id)))) + + ;; And: empty default groups are deleted + (is (empty? (db/group-allnames admin/*db* "net.clojars.testuser"))) + (is (empty? (db/find-group-verification admin/*db* "net.clojars.testuser"))) + + ;; And: non-empty default groups are left in place, but with the new username + (is (= [new-username "otheruser"] (db/group-allnames admin/*db* "org.clojars.testuser"))) + (is (some? (db/find-group-verification admin/*db* "org.clojars.testuser"))) + + ;; And: existing non-default groups are left in place, but with the new username + (is (= [new-username] (db/group-allnames admin/*db* "org.ham"))) + (is (match? {:verified_by new-username} + (db/find-group-verification admin/*db* "org.ham"))) + + ;; And: any jars they deployed have the new username + (is (seq (db/jars-by-username admin/*db* new-username))) + (is (empty? (db/jars-by-username admin/*db* "testuser"))) + + ;; And: their audit records have been reassigned to the new username + (is (seq (db/find-audit admin/*db* {:username new-username}))) + (is (empty? (db/find-audit admin/*db* {:username "testuser"}))))) + +(deftest delete-user!-with-non-existent-user + (is (thrown-with-msg? + Exception #"User does-not-exist not found!" + (admin/delete-user! "does-not-exist")))) diff --git a/test/clojars/unit/db_test.clj b/test/clojars/unit/db_test.clj index 89498dc3..a30d1ef2 100644 --- a/test/clojars/unit/db_test.clj +++ b/test/clojars/unit/db_test.clj @@ -5,7 +5,8 @@ [clj-time.core :as time] [clojars.db :as db] [clojars.test-helper :as help] - [clojure.test :refer [are deftest is use-fixtures]]) + [clojure.test :refer [are deftest is use-fixtures]] + [matcher-combinators.test]) (:import (clojure.lang ExceptionInfo) @@ -15,20 +16,12 @@ (use-fixtures :each help/with-clean-database) -(defn submap [s m] - (every? (fn [[k v]] (= (get m k) v)) s)) - -(deftest submap-test - (is (not (submap {:a 1} nil))) - (is (not (submap {:a 1} {:a 2}))) - (is (submap {:a 1} {:a 1 :b 2}))) - (deftest added-users-can-be-found (let [email "test@example.com" name "testuser" password "password"] (db/add-user help/*db* email name password) - (are [x] (submap {:email email + (are [x] (match? {:email email :user name} x) (db/find-user help/*db* name) @@ -44,7 +37,7 @@ password "password"] (db/add-user help/*db* email name password) (let [reset-code (db/set-password-reset-code! help/*db* "testuser")] - (is (submap {:email email + (is (match? {:email email :user name :password_reset_code reset-code} (db/find-user-by-password-reset-code help/*db* reset-code))) @@ -52,38 +45,49 @@ (time/do-at (-> 2 time/days time/from-now) (is (not (db/find-user-by-password-reset-code help/*db* reset-code))))))) -(deftest updated-users-can-be-found +(deftest renamed-users-can-be-found (let [email "test@example.com" name "testuser" password "password" created-at (Timestamp. 0) - email2 "test2@example.com" - name2 "testuser2" - password2 "password2"] + name2 "testuser2"] (help/with-time created-at (db/add-user help/*db* email name password) (help/with-time (Timestamp. 1) - (db/update-user help/*db* name email2 name2 password2) - (are [x] (submap {:email email2 - :user name2 + (db/rename-user help/*db* name name2) + (are [x] (match? {:user name2 :created created-at} x) (db/find-user help/*db* name2) - (db/find-user-by-user-or-email help/*db* name2) - (db/find-user-by-user-or-email help/*db* email2))) + (db/find-user-by-user-or-email help/*db* name2))) (is (not (db/find-user help/*db* name)))))) +(deftest updated-users-can-be-found + (let [email "test@example.com" + name "testuser" + password "password" + created-at (Timestamp. 0) + email2 "test2@example.com" + password2 "password2"] + (help/with-time created-at + (db/add-user help/*db* email name password) + (help/with-time (Timestamp. 1) + (db/update-user help/*db* name email2 password2) + (is (match? + {:email email2 + :created created-at} + (db/find-user-by-user-or-email help/*db* email2))))))) + (deftest update-user-works-when-password-is-blank (let [email "test@example.com" name "testuser" password "password" email2 "test2@example.com" - name2 "testuser2" password2 ""] (db/add-user help/*db* email name password) (let [old-user (db/find-user help/*db* name)] - (db/update-user help/*db* name email2 name2 password2) - (let [user (db/find-user help/*db* name2)] + (db/update-user help/*db* name email2 password2) + (let [user (db/find-user help/*db* name)] (is (= email2 (:email user))) (is (= (:password old-user) (:password user))))))) @@ -160,7 +164,7 @@ (help/add-verified-group "test-user" name) (help/with-time created-at (db/add-jar help/*db* "test-user" jarmap) - (are [x] (submap result x) + (are [x] (match? result x) (db/find-jar help/*db* name name) (first (db/jars-by-groupname help/*db* name)) (first (db/jars-by-username help/*db* "test-user")))))) @@ -212,7 +216,7 @@ (db/add-jar help/*db* "test-user" jarmap) (let [deps (db/find-dependencies help/*db* name name "1.0")] (is (= 1 (count deps))) - (is (submap + (is (match? {:jar_name name :group_name name :version "1.0" @@ -234,7 +238,7 @@ (db/add-jar help/*db* "test-user" jarmap) (let [deps (db/find-dependencies help/*db* name name "1.0-SNAPSHOT")] (is (= 1 (count deps))) - (is (submap + (is (match? {:jar_name name :group_name name :version "1.0-SNAPSHOT" @@ -357,7 +361,7 @@ (help/with-time (Timestamp. 3) (db/add-jar help/*db* "test-user2" (assoc jarmap :group "tester-group"))) (let [jars (db/jars-by-groupname help/*db* name)] - (dorun (map #(is (submap %1 %2)) + (dorun (map #(is (match? %1 %2)) [result (assoc result :jar_name "tester2") (assoc result :jar_name "tester3")] @@ -399,7 +403,7 @@ (help/with-time (Timestamp. 3) (db/add-jar help/*db* "test-user" (assoc jarmap :group "tester-group"))) (let [jars (db/jars-by-username help/*db* "test-user")] - (dorun (map #(is (submap %1 %2)) + (dorun (map #(is (match? %1 %2)) [result (assoc result :jar_name "tester2") (assoc result :group_name "tester-group")] @@ -441,7 +445,7 @@ (db/add-jar help/*db* "test-user" (assoc jarmap :name "6"))) (help/with-time (Timestamp. (long 7)) (db/add-jar help/*db* "test-user" (assoc jarmap :version "7"))) - (dorun (map #(is (submap %1 %2)) + (dorun (map #(is (match? %1 %2)) [(assoc result :version "7") (assoc result :jar_name "6") (assoc result :jar_name "4")