Просмотр исходного кода

enhance(ux): able to remove graph access or leave a graph

Tienson Qin 3 недель назад
Родитель
Сommit
59e1cd9af9

+ 11 - 0
deps/db-sync/src/logseq/db_sync/index.cljs

@@ -254,6 +254,17 @@
                   graph-id
                   user-id))
 
+(defn <graph-member-role [db graph-id user-id]
+  (when (and (string? graph-id) (string? user-id))
+    (p/let [result (common/<d1-all db
+                                   "select role from graph_members where graph_id = ? and user_id = ?"
+                                   graph-id
+                                   user-id)
+            rows (common/get-sql-rows result)
+            row (first rows)]
+      (when row
+        (aget row "role")))))
+
 (defn <user-has-access-to-graph? [db graph-id user-id]
   (when (and (string? graph-id) (string? user-id))
     (p/let [result (common/<d1-all db

+ 14 - 4
deps/db-sync/src/logseq/db_sync/worker.cljs

@@ -1014,11 +1014,21 @@
                 (bad-request "invalid user id")
 
                 :else
-                (p/let [manager? (index/<user-is-manager? db graph-id user-id)]
-                  (if (not manager?)
-                    (forbidden)
+                (p/let [manager? (index/<user-is-manager? db graph-id user-id)
+                        target-role (index/<graph-member-role db graph-id member-id)
+                        self-leave? (and (= user-id member-id)
+                                         (= "member" target-role))]
+                  (cond
+                    (and manager? (not= "manager" target-role))
+                    (p/let [_ (index/<graph-member-delete! db graph-id member-id)]
+                      (json-response :graph-members/delete {:ok true}))
+
+                    self-leave?
                     (p/let [_ (index/<graph-member-delete! db graph-id member-id)]
-                      (json-response :graph-members/delete {:ok true}))))))
+                      (json-response :graph-members/delete {:ok true}))
+
+                    :else
+                    (forbidden)))))
 
             (and (= method "GET")
                  (= ["e2ee" "user-keys"] parts))

+ 227 - 0
deps/db-sync/test/logseq/db_sync/worker_members_test.cljs

@@ -0,0 +1,227 @@
+(ns logseq.db-sync.worker-members-test
+  (:require [cljs.test :refer [deftest is async]]
+            [clojure.string :as string]
+            [logseq.db-sync.index :as index]
+            [logseq.db-sync.worker :as worker]
+            [promesa.core :as p]))
+
+(defn- js-key [k]
+  (cond
+    (keyword? k) (string/replace (name k) "-" "_")
+    (string? k) k
+    :else (str k)))
+
+(defn- js-row [m]
+  (let [o (js-obj)]
+    (doseq [[k v] m]
+      (aset o (js-key k) v))
+    o))
+
+(defn- js-rows [rows]
+  (into-array (map js-row rows)))
+
+(defn- record-exec! [state sql]
+  (swap! state update :executed conj sql))
+
+(defn- run-sql! [state sql args]
+  (record-exec! state sql)
+  (cond
+    (string/includes? sql "insert into graph_members")
+    (let [[user-id graph-id role invited-by created-at] args]
+      (swap! state update :graph-members
+             (fn [members]
+               (let [k [user-id graph-id]
+                     existing (get members k)
+                     created-at (or (:created-at existing) created-at)]
+                 (assoc members k {:user-id user-id
+                                   :graph-id graph-id
+                                   :role role
+                                   :invited-by invited-by
+                                   :created-at created-at})))))
+
+    (string/includes? sql "insert into graphs")
+    (let [[graph-id graph-name user-id schema-version created-at updated-at] args]
+      (swap! state update :graphs assoc graph-id {:graph-id graph-id
+                                                  :graph-name graph-name
+                                                  :user-id user-id
+                                                  :schema-version schema-version
+                                                  :created-at created-at
+                                                  :updated-at updated-at}))
+
+    (string/includes? sql "delete from graph_members")
+    (let [[graph-id user-id] args]
+      (swap! state update :graph-members dissoc [user-id graph-id]))
+
+    :else
+    nil))
+
+(defn- union-access-rows [state sql args]
+  (let [[graph-id user-id] args
+        graph-owner-id (get-in @state [:graphs graph-id :user-id])
+        member (get-in @state [:graph-members [user-id graph-id]])
+        manager-required? (string/includes? sql "role = 'manager'")
+        has-access? (or (= graph-owner-id user-id)
+                        (and member
+                             (or (not manager-required?)
+                                 (= "manager" (:role member)))))]
+    (if has-access?
+      (js-rows [{:graph-id graph-id}])
+      (js-rows []))))
+
+(defn- all-sql [state sql args]
+  (record-exec! state sql)
+  (cond
+    (string/includes? sql "select role from graph_members")
+    (let [[graph-id user-id] args
+          role (get-in @state [:graph-members [user-id graph-id] :role])]
+      (if role
+        (js-rows [{:role role}])
+        (js-rows [])))
+
+    (string/includes? sql "union select graph_id from graph_members")
+    (union-access-rows state sql args)
+
+    :else
+    (js-rows [])))
+
+(defn- make-d1 [state]
+  #js {:prepare (fn [sql]
+                  (let [stmt #js {}]
+                    (set! (.-_sql stmt) sql)
+                    (set! (.-_args stmt) [])
+                    (set! (.-bind stmt)
+                          (fn [& args]
+                            (set! (.-_args stmt) (vec args))
+                            stmt))
+                    (set! (.-run stmt)
+                          (fn []
+                            (run-sql! state (.-_sql stmt) (.-_args stmt))
+                            #js {}))
+                    (set! (.-all stmt)
+                          (fn []
+                            #js {:results (all-sql state (.-_sql stmt) (.-_args stmt))}))
+                    stmt))})
+
+(defn- request-delete [graph-id member-id]
+  (js/Request. (str "http://localhost/graphs/" graph-id "/members/" member-id)
+               #js {:method "DELETE"}))
+
+(defn- response-status [response]
+  (.-status response))
+
+(defn- setup-graph! [db graph-id owner-id]
+  (index/<index-upsert! db graph-id "graph" owner-id "1"))
+
+(deftest manager-can-remove-member-test
+  (async done
+         (let [state (atom {:executed []
+                            :graph-members {}
+                            :graphs {}})
+               db (make-d1 state)
+               graph-id "graph-1"
+               manager-id "manager-1"
+               member-id "member-1"
+               request (request-delete graph-id member-id)
+               self #js {:env #js {} :d1 db}]
+           (with-redefs [worker/auth-claims (fn [_ _]
+                                              (js/Promise.resolve #js {"sub" manager-id}))]
+             (-> (p/do!
+                  (setup-graph! db graph-id manager-id)
+                  (index/<graph-member-upsert! db graph-id manager-id "manager" manager-id)
+                  (index/<graph-member-upsert! db graph-id member-id "member" manager-id))
+                 (p/then (fn [_]
+                           (let [resp (#'worker/handle-index-fetch self request)
+                                 status (response-status resp)
+                                 member (get-in @state [:graph-members [member-id graph-id]])]
+                             (is (= 200 status))
+                             (is (nil? member))
+                             (done))))
+                 (p/catch (fn [e]
+                            (is false (str e))
+                            (done))))))))
+
+(deftest manager-cannot-remove-manager-test
+  (async done
+         (let [state (atom {:executed []
+                            :graph-members {}
+                            :graphs {}})
+               db (make-d1 state)
+               graph-id "graph-1"
+               manager-id "manager-1"
+               other-manager-id "manager-2"
+               request (request-delete graph-id other-manager-id)
+               self #js {:env #js {} :d1 db}]
+           (with-redefs [worker/auth-claims (fn [_ _]
+                                              (js/Promise.resolve #js {"sub" manager-id}))]
+             (-> (p/do!
+                  (setup-graph! db graph-id manager-id)
+                  (index/<graph-member-upsert! db graph-id manager-id "manager" manager-id)
+                  (index/<graph-member-upsert! db graph-id other-manager-id "manager" manager-id))
+                 (p/then (fn [_]
+                           (let [resp (#'worker/handle-index-fetch self request)
+                                 status (response-status resp)
+                                 member (get-in @state [:graph-members [other-manager-id graph-id]])]
+                             (is (= 403 status))
+                             (is (some? member))
+                             (done))))
+                 (p/catch (fn [e]
+                            (is false (str e))
+                            (done))))))))
+
+(deftest member-can-leave-test
+  (async done
+         (let [state (atom {:executed []
+                            :graph-members {}
+                            :graphs {}})
+               db (make-d1 state)
+               graph-id "graph-1"
+               manager-id "manager-1"
+               member-id "member-1"
+               request (request-delete graph-id member-id)
+               self #js {:env #js {} :d1 db}]
+           (with-redefs [worker/auth-claims (fn [_ _]
+                                              (js/Promise.resolve #js {"sub" member-id}))]
+             (-> (p/do!
+                  (setup-graph! db graph-id manager-id)
+                  (index/<graph-member-upsert! db graph-id manager-id "manager" manager-id)
+                  (index/<graph-member-upsert! db graph-id member-id "member" manager-id))
+                 (p/then (fn [_]
+                           (let [resp (#'worker/handle-index-fetch self request)
+                                 status (response-status resp)
+                                 member (get-in @state [:graph-members [member-id graph-id]])]
+                             (is (= 200 status))
+                             (is (nil? member))
+                             (done))))
+                 (p/catch (fn [e]
+                            (is false (str e))
+                            (done))))))))
+
+(deftest member-cannot-remove-others-test
+  (async done
+         (let [state (atom {:executed []
+                            :graph-members {}
+                            :graphs {}})
+               db (make-d1 state)
+               graph-id "graph-1"
+               manager-id "manager-1"
+               member-id "member-1"
+               other-member-id "member-2"
+               request (request-delete graph-id other-member-id)
+               self #js {:env #js {} :d1 db}]
+           (with-redefs [worker/auth-claims (fn [_ _]
+                                              (js/Promise.resolve #js {"sub" member-id}))]
+             (-> (p/do!
+                  (setup-graph! db graph-id manager-id)
+                  (index/<graph-member-upsert! db graph-id manager-id "manager" manager-id)
+                  (index/<graph-member-upsert! db graph-id member-id "member" manager-id)
+                  (index/<graph-member-upsert! db graph-id other-member-id "member" manager-id))
+                 (p/then (fn [_]
+                           (let [resp (#'worker/handle-index-fetch self request)
+                                 status (response-status resp)
+                                 member (get-in @state [:graph-members [other-member-id graph-id]])]
+                             (is (= 403 status))
+                             (is (some? member))
+                             (done))))
+                 (p/catch (fn [e]
+                            (is false (str e))
+                            (done))))))))

+ 10 - 9
deps/outliner/src/logseq/outliner/core.cljs

@@ -936,15 +936,16 @@
 (defn- move-block
   [db block target-block sibling?]
   (let [target-block (d/entity db (:db/id target-block))
-        block (d/entity db (:db/id block))]
-    (if (or
-         ;; target-block doesn't have parent
-         (and sibling? (nil? (:block/parent target-block)))
-         ;; move page to be a child of block
-         (and (not sibling?)
-              (not (ldb/page? target-block))
-              (ldb/page? block)))
-      (throw (ex-info "not-allowed-move-block-page" {}))
+        block (d/entity db (:db/id block))
+        target-without-parent? (and sibling? (nil? (:block/parent target-block)))
+        move-page-as-block-child? (and (not sibling?)
+                                       (not (ldb/page? target-block))
+                                       (ldb/page? block))]
+    (if (or target-without-parent? move-page-as-block-child?)
+      (throw (ex-info "not-allowed-move-block-page"
+                      {:reason (if target-without-parent?
+                                 :move-to-target-without-parent
+                                 :move-page-to-be-child-of-block)}))
       (let [first-block-page (:db/id (:block/page block))
             target-page (get-target-block-page target-block sibling?)
             not-same-page? (not= first-block-page target-page)

+ 19 - 11
src/main/frontend/components/repo.cljs

@@ -165,17 +165,25 @@
               {:key "leave-shared-graph"
                :class "leave-shared-graph-menu-item"
                :on-click (fn []
-                           (notification/show!
-                            "Please ask this graph's manager to rovoke your access."
-                            :info
-                            false)
-                           ;; (let [prompt-str "Are you sure you want to leave this graph?"]
-                           ;;   (-> (shui/dialog-confirm!
-                           ;;        [:p.font-medium.-my-4 prompt-str])
-                           ;;       (p/then
-                           ;;        (fn []
-                           ;;          ))))
-                           )}
+                           (let [prompt-str "Are you sure you want to leave this graph?"]
+                             (-> (shui/dialog-confirm!
+                                  [:p.font-medium.-my-4 prompt-str])
+                                 (p/then
+                                  (fn []
+                                    (state/set-state! :rtc/loading-graphs? true)
+                                    (when (= (state/get-current-repo) repo)
+                                      (state/<invoke-db-worker :thread-api/rtc-stop))
+                                    (-> (rtc-handler/<rtc-leave-graph! GraphUUID)
+                                        (p/then (fn []
+                                                  (notification/show! "Left graph." :success)
+                                                  (rtc-handler/<get-remote-graphs)))
+                                        (p/catch (fn [e]
+                                                   (notification/show! "Failed to leave graph." :error)
+                                                   (log/error :db-sync/leave-graph-failed
+                                                              {:error e
+                                                               :graph-uuid GraphUUID})))
+                                        (p/finally (fn []
+                                                     (state/set-state! :rtc/loading-graphs? false)))))))))}
               "Leave this graph")))))]]]))
 
 (rum/defc repos-cp < rum/reactive

+ 35 - 4
src/main/frontend/components/settings.cljs

@@ -920,6 +920,7 @@
   []
   (let [[invite-email set-invite-email!] (hooks/use-state "")
         current-repo (state/get-current-repo)
+        manager? (user-handler/manager? current-repo)
         [users-info] (hooks/use-atom (:rtc/users-info @state/state))
         users (get users-info current-repo)
         invite-user! (fn []
@@ -938,11 +939,41 @@
      [:div.users.flex.flex-col.gap-1
       (for [{user-name :user/name
              user-email :user/email
+             user-uuid :user/uuid
              graph<->user-user-type :graph<->user/user-type} users]
-        [:div.flex.flex-row.items-center.gap-2 {:key (str "user-" user-name)}
-         [:div user-name]
-         (when user-email [:div.opacity-50.text-sm user-email])
-         (when graph<->user-user-type [:div.opacity-50.text-sm (name graph<->user-user-type)])])]
+        (let [member? (= :member graph<->user-user-type)
+              can-remove? (and manager? member?)]
+          [:div.flex.flex-row.items-center.gap-2
+           {:key (str "user-" (or user-uuid user-name))}
+           [:div user-name]
+           (when user-email [:div.opacity-50.text-sm user-email])
+           (when graph<->user-user-type [:div.opacity-50.text-sm (name graph<->user-user-type)])
+           (when can-remove?
+             (shui/dropdown-menu
+              (shui/dropdown-menu-trigger
+               {:asChild true}
+               (shui/button
+                {:variant "ghost"
+                 :size :sm
+                 :class "px-1 h-7"}
+                (ui/icon "dots" {:size 14})))
+              (shui/dropdown-menu-content
+               {:align "end"}
+               (shui/dropdown-menu-item
+                {:class "remove-member-menu-item"
+                 :on-click (fn []
+                             (let [graph-uuid (ldb/get-graph-rtc-uuid (db/get-db))
+                                   member-id user-uuid]
+                               (when (and graph-uuid member-id)
+                                 (-> (rtc-handler/<rtc-remove-member! graph-uuid member-id)
+                                     (p/then (fn []
+                                               (rtc-handler/<rtc-get-users-info)))
+                                     (p/catch (fn [e]
+                                                (notification/show! "Failed to remove member." :error)
+                                                (log/error :db-sync/remove-member-failed {:error e
+                                                                                          :graph-uuid graph-uuid
+                                                                                          :member-id member-id})))))))}
+                "Remove access"))))]))]
      [:div.flex.flex-col.gap-4.mt-4
       (shui/input
        {:placeholder   "Email address"

+ 24 - 0
src/main/frontend/handler/db_based/db_sync.cljs

@@ -380,3 +380,27 @@
                             :graph-uuid graph-uuid
                             :email email
                             :base base})))))
+
+(defn <rtc-remove-member!
+  [graph-uuid member-id]
+  (let [base (http-base)
+        graph-uuid (some-> graph-uuid str)
+        member-id (some-> member-id str)]
+    (if (and base (string? graph-uuid) (string? member-id))
+      (p/let [_ (js/Promise. user-handler/task--ensure-id&access-token)]
+        (fetch-json (str base "/graphs/" graph-uuid "/members/" member-id)
+                    {:method "DELETE"}
+                    {:response-schema :graph-members/delete}))
+      (p/rejected (ex-info "db-sync missing member info"
+                           {:type :db-sync/invalid-member
+                            :graph-uuid graph-uuid
+                            :member-id member-id
+                            :base base})))))
+
+(defn <rtc-leave-graph!
+  [graph-uuid]
+  (if-let [member-id (user-handler/user-uuid)]
+    (<rtc-remove-member! graph-uuid member-id)
+    (p/rejected (ex-info "db-sync missing user id"
+                         {:type :db-sync/invalid-member
+                          :graph-uuid graph-uuid}))))

+ 17 - 0
src/main/frontend/handler/db_based/sync.cljs

@@ -72,3 +72,20 @@
   (if (db-sync-enabled?)
     (db-sync-handler/<rtc-invite-email graph-uuid email)
     (rtc-handler/<rtc-invite-email graph-uuid email)))
+
+(defn <rtc-remove-member!
+  [graph-uuid member-id]
+  (if (db-sync-enabled?)
+    (db-sync-handler/<rtc-remove-member! graph-uuid member-id)
+    (p/rejected (ex-info "RTC remove member not supported"
+                         {:type :rtc/unsupported-remove-member
+                          :graph-uuid graph-uuid
+                          :member-id member-id}))))
+
+(defn <rtc-leave-graph!
+  [graph-uuid]
+  (if (db-sync-enabled?)
+    (db-sync-handler/<rtc-leave-graph! graph-uuid)
+    (p/rejected (ex-info "RTC leave graph not supported"
+                         {:type :rtc/unsupported-leave-graph
+                          :graph-uuid graph-uuid}))))

+ 49 - 0
src/test/frontend/handler/db_based/db_sync_test.cljs

@@ -1,6 +1,7 @@
 (ns frontend.handler.db-based.db-sync-test
   (:require [cljs.test :refer [deftest is async]]
             [frontend.handler.db-based.db-sync :as db-sync]
+            [frontend.handler.user :as user-handler]
             [promesa.core :as p]))
 
 (deftest download-graph-e2ee-detection-test
@@ -24,3 +25,51 @@
                (p/catch (fn [e]
                           (is false (str e))
                           (done)))))))
+
+(deftest remove-member-request-test
+  (async done
+         (let [called (atom nil)]
+           (with-redefs [db-sync/http-base (fn [] "http://base")
+                         db-sync/fetch-json (fn [url opts _]
+                                              (reset! called {:url url :opts opts})
+                                              (p/resolved {:ok true}))
+                         user-handler/task--ensure-id&access-token (fn [resolve _reject]
+                                                                     (resolve true))]
+             (-> (p/let [_ (db-sync/<rtc-remove-member! "graph-1" "user-2")
+                         {:keys [url opts]} @called]
+                   (is (= "http://base/graphs/graph-1/members/user-2" url))
+                   (is (= "DELETE" (:method opts)))
+                   (done))
+                 (p/catch (fn [e]
+                            (is false (str e))
+                            (done))))))))
+
+(deftest leave-graph-uses-current-user-test
+  (async done
+         (let [called (atom nil)]
+           (with-redefs [db-sync/http-base (fn [] "http://base")
+                         db-sync/fetch-json (fn [url opts _]
+                                              (reset! called {:url url :opts opts})
+                                              (p/resolved {:ok true}))
+                         user-handler/task--ensure-id&access-token (fn [resolve _reject]
+                                                                     (resolve true))
+                         user-handler/user-uuid (fn [] "user-1")]
+             (-> (p/let [_ (db-sync/<rtc-leave-graph! "graph-1")
+                         {:keys [url opts]} @called]
+                   (is (= "http://base/graphs/graph-1/members/user-1" url))
+                   (is (= "DELETE" (:method opts)))
+                   (done))
+                 (p/catch (fn [e]
+                            (is false (str e))
+                            (done))))))))
+
+(deftest leave-graph-missing-user-test
+  (async done
+         (with-redefs [user-handler/user-uuid (fn [] nil)]
+           (-> (db-sync/<rtc-leave-graph! "graph-1")
+               (p/then (fn [_]
+                         (is false "expected rejection")
+                         (done)))
+               (p/catch (fn [e]
+                          (is (= :db-sync/invalid-member (:type (ex-data e))))
+                          (done)))))))