diff --git a/src/metabase/api/activity.clj b/src/metabase/api/activity.clj index d27a9c5dee0976628a8d578853df5eba5d14b618..187eca09980338ec448a6ded46d715f03ca8dd3c 100644 --- a/src/metabase/api/activity.clj +++ b/src/metabase/api/activity.clj @@ -13,8 +13,8 @@ [hydrate :refer [hydrate]]])) (defn- dashcard-activity? [activity] - (contains? #{:dashboard-add-cards :dashboard-remove-cards} - (:topic activity))) + (#{:dashboard-add-cards :dashboard-remove-cards} + (:topic activity))) (defn- activities->referenced-objects "Get a map of model name to a set of referenced IDs in these ACTIVITIES. diff --git a/src/metabase/events/activity_feed.clj b/src/metabase/events/activity_feed.clj index aad5306c961b1fede7ab226e31979af27c6a06e1..228c76dacefe25411b6cc653e49598cde04f18d3 100644 --- a/src/metabase/events/activity_feed.clj +++ b/src/metabase/events/activity_feed.clj @@ -15,7 +15,7 @@ [toucan.db :as db])) (def ^:private activity-feed-topics - "The `Set` of event topics which are subscribed to for use in the Metabase activity feed." + "The set of event topics which are subscribed to for use in the Metabase activity feed." #{:alert-create :alert-delete :card-create @@ -34,7 +34,7 @@ :segment-create :segment-update :segment-delete - :user-login}) + :user-login}) ; this is only used these days the first time someone logs in to record 'user-joined' events (def ^:private activity-feed-channel "Channel for receiving event notifications we want to subscribe to for the activity feed." diff --git a/src/metabase/models/activity.clj b/src/metabase/models/activity.clj index db2bf1da1807dae091fb16de150eef5fdd14dafa..c88a0e8cb79451273c6cb0d7ee79cd8236ecb999 100644 --- a/src/metabase/models/activity.clj +++ b/src/metabase/models/activity.clj @@ -2,6 +2,7 @@ (:require [metabase [events :as events] [util :as u]] + [metabase.api.common :as api] [metabase.models [card :refer [Card]] [dashboard :refer [Dashboard]] @@ -23,10 +24,26 @@ "pulse" Pulse "segment" Segment}) -(defn- can-? [f {model :model, model-id :model_id, :as activity}] +(defmulti can-? + "Implementation for `can-read?`/`can-write?` for items in the activity feed. Dispatches off of the activity `:topic`, + e.g. `:user-joined`. `perms-check-fn` is `can-read?` or `can-write?` and should be called as needed on models the + activity records." + {:arglists '([perms-check-fn activity])} + (fn [_ {:keys [topic]}] + topic)) + +;; For now only admins can see when another user joined -- we don't want every user knowing about every other user. In +;; the future we might want to change this and come up with some sort of system where we can determine which users get +;; to see other users -- perhaps if they are in a group together other than 'All Users' +(defmethod can-? :user-joined [_ _] + api/*is-superuser?*) + +;; For every other activity topic we'll look at the read/write perms for the object the activty is about (e.g. a Card +;; or Dashboard). For all other activity feed items with no model everyone can read/write +(defmethod can-? :default [perms-check-fn {model :model, model-id :model_id, :as activity}] (if-let [object (when-let [entity (model->entity model)] (entity model-id))] - (f object) + (perms-check-fn object) true)) @@ -47,6 +64,7 @@ i/IObjectPermissions (merge i/IObjectPermissionsDefaults {:can-read? (partial can-? i/can-read?) + ;; TODO - when do people *write* activities? :can-write? (partial can-? i/can-write?)})) diff --git a/test/metabase/api/activity_test.clj b/test/metabase/api/activity_test.clj index f5bd666940bb601b5c67c575625eaeecaa79bf6b..72497b5e1f071f3b2ad02842e4ab6c5868f2fde3 100644 --- a/test/metabase/api/activity_test.clj +++ b/test/metabase/api/activity_test.clj @@ -97,8 +97,8 @@ :details $})] ;; clear any other activities from the DB just in case; not sure this step is needed any more (do (db/delete! Activity :id [:not-in #{(:id activity1) - (:id activity2) - (:id activity3)}]) + (:id activity2) + (:id activity3)}]) (for [activity ((user->client :crowberto) :get 200 "activity")] (dissoc activity :timestamp)))) @@ -211,3 +211,17 @@ {:model "card", :model_id 0} {:model "dashboard", :model_id 0, :topic :dashboard-remove-cards, :details {:dashcards [{:card_id card-id} {:card_id 0}]}}])) + +;; Only admins should get to see user-joined activities +(defn- user-can-see-user-joined-activity? [user] + ;; clear out all existing Activity entries + (db/delete! Activity) + (-> (tt/with-temp Activity [activity {:topic "user-joined" + :details {} + :timestamp (du/->Timestamp #inst "2019-02-15T11:55:00.000Z")}] + ((user->client user) :get 200 "activity")) + seq + boolean)) + +(expect true (user-can-see-user-joined-activity? :crowberto)) +(expect false (user-can-see-user-joined-activity? :rasta))