Use collection permissions fns to select correct timelines (#21362)

* Use collection permissions fns to select correct timelines

The Timelines permissions are 1:1 with collection permissions, so on the GET timelines endpoint, use the existing
collection permissions functions to help efficiently check timeline permissions.

* Add permissions tests to timeline api

* Root lacks most collection properties

no need to hydrate parent id, effective location, etc

Co-authored-by: dan sutton <dan@dpsutton.com>

src/metabase/api/timeline.clj

@@ -35,13 +35,19 @@
 (api/defendpoint GET "/"
   "Fetch a list of [[Timelines]]. Can include `archived=true` to return archived timelines."
   [include archived]
-  {include (s/maybe Include)
+  {include  (s/maybe Include)
    archived (s/maybe su/BooleanString)}
   (let [archived? (Boolean/parseBoolean archived)
-        timelines (map timeline/hydrate-root-collection (db/select Timeline [:where [:= :archived archived?]]))]
+        timelines (->> (db/select Timeline
+                         {:where    [:and
+                                     [:= :archived archived?]
+                                     (collection/visible-collection-ids->honeysql-filter-clause
+                                      (collection/permissions-set->visible-collection-ids @api/*current-user-permissions-set*))]
+                          :order-by [[:%lower.name :asc]]})
+                       (map timeline/hydrate-root-collection))]
     (cond->> (hydrate timelines :creator [:collection :can_write])
       (= include "events")
-      (map #(timeline-event/include-events-singular % {:events/all?  archived?})))))
+      (map #(timeline-event/include-events-singular % {:events/all? archived?})))))
 
 (api/defendpoint GET "/:id"
   "Fetch the [[Timeline]] with `id`. Include `include=events` to unarchived events included on the timeline. Add

src/metabase/models/timeline.clj

@@ -26,8 +26,7 @@
 (defn- root-collection
   []
   (-> (collection/root-collection-with-ui-details nil)
-      collection/personal-collection-with-ui-details
-      (hydrate :parent_id :effective_location [:effective_ancestors :can_write] :can_write)))
+      (hydrate :can_write)))
 
 (defn hydrate-root-collection
   "Hydrate `:collection` on [[Timelines]] when the id is `nil`."

test/metabase/api/timeline_test.clj

 (ns metabase.api.timeline-test
   "Tests for /api/timeline endpoints."
   (:require [clojure.test :refer :all]
+            [medley.core :as m]
             [metabase.http-client :as http]
             [metabase.models.collection :refer [Collection]]
+            [metabase.models.permissions :as perms]
+            [metabase.models.permissions-group :as group]
             [metabase.models.timeline :refer [Timeline]]
             [metabase.models.timeline-event :refer [TimelineEvent]]
             [metabase.server.middleware.util :as middleware.u]
@@ -37,7 +40,7 @@
           (testing "check that `:collection` key is hydrated on each timeline"
             (is (= #{id}
                    (->> (mt/user-http-request :rasta :get 200 "timeline")
-                         (filter (comp #{id} :collection_id))
+                        (filter (comp #{id} :collection_id))
                         (map #(get-in % [:collection :id]))
                         set))))
           (testing "check that `:can_write` key is hydrated"
@@ -45,7 +48,28 @@
                  #(contains? % :can_write)
                  (->> (mt/user-http-request :rasta :get 200 "timeline")
                       (filter (comp #{id} :collection_id))
-                      :collection)))))))))
+                      :collection)))))))
+    (testing "checks permissions"
+      (mt/with-temp* [Collection    [{coll-id :id :as collection} {:name "private collection"}]
+                      Timeline      [tl-a {:name "Timeline A" :collection_id coll-id}]
+                      Timeline      [tl-b {:name "Timeline B" :collection_id coll-id}]
+                      TimelineEvent [e-a  {:name "Event 1" :timeline_id (u/the-id tl-a)}]
+                      TimelineEvent [e-b  {:name "Event 2" :timeline_id (u/the-id tl-b)}]]
+        (letfn [(events-for [user events?]
+                  (->> (m/mapply mt/user-http-request user :get 200 "timeline" (when events? {:include "events"}))
+                       (filter (comp #{coll-id} :collection_id))))]
+          (perms/revoke-collection-permissions! (group/all-users) coll-id)
+          (testing "a non-admin user cannot see any timelines"
+            (is (= [] (events-for :rasta true)))
+            (is (= [] (events-for :rasta false))))
+          (testing "an admin user can see these timelines"
+            (is (partial= [{:name "Timeline A"
+                            :events [{:name "Event 1"}]}
+                           {:name "Timeline B"
+                            :events [{:name "Event 2"}]}]
+                          (events-for :crowberto true)))
+            (is (partial= [{:name "Timeline A"} {:name "Timeline B"}]
+                          (events-for :crowberto false)))))))))
 
 (deftest get-timeline-test
   (testing "GET /api/timeline/:id"