Ensure collection_id is included in series dashcards

Hydrating series dashcards wasn't including the related collection
id. The collection id is one way in which permissions are
checked. This would manifest itself in the user having access to
something they shouldn't. As an example, if a dashboard had a single
card that was part of a collection the user wouldn't have access to,
and the user attempted to access it, they would get a 403. If a second
card was added and that card contained multiple (i.e. a series) due to
this bug, the second card wouldn't have a collection_id and thus we'd
assume the user has access to it. The permissions checking on the
dashboard only checks whether or not the user has access to at least
one card in the dashboard, thus giving user access to the dashboard.

Fixes #5266

src/metabase/models/dashboard_card.clj

@@ -68,7 +68,7 @@
 (defn ^:hydrate series
   "Return the `Cards` associated as additional series on this `DashboardCard`."
   [{:keys [id]}]
-  (db/select [Card :id :name :description :display :dataset_query :visualization_settings]
+  (db/select [Card :id :name :description :display :dataset_query :visualization_settings :collection_id]
     (mdb/join [Card :id] [DashboardCardSeries :card_id])
     (db/qualify DashboardCardSeries :dashboardcard_id) id
     {:order-by [[(db/qualify DashboardCardSeries :position) :asc]]}))

test/metabase/api/dashboard_test.clj

@@ -11,6 +11,7 @@
              [dashboard :as dashboard-api]]
             [metabase.models
              [card :refer [Card]]
+             [collection :refer [Collection]]
              [dashboard :refer [Dashboard]]
              [dashboard-card :refer [DashboardCard retrieve-dashboard-card]]
              [dashboard-card-series :refer [DashboardCardSeries]]
@@ -143,6 +144,19 @@
                   DashboardCard [_                  {:dashboard_id dashboard-id, :card_id card-id}]]
     (dashboard-response ((user->client :rasta) :get 200 (format "dashboard/%d" dashboard-id)))))
 
+;; ## GET /api/dashboard/:id with a series, should fail if the user doesn't have access to the collection
+(expect
+  "You don't have permissions to do that."
+  (tt/with-temp* [Collection          [{coll-id :id}      {:name "Collection 1"}]
+                  Dashboard           [{dashboard-id :id} {:name       "Test Dashboard"
+                                                           :creator_id (user->id :crowberto)}]
+                  Card                [{card-id :id}      {:name          "Dashboard Test Card"
+                                                           :collection_id coll-id}]
+                  Card                [{card-id2 :id}     {:name          "Dashboard Test Card 2"
+                                                           :collection_id coll-id}]
+                  DashboardCard       [{dbc_id :id}       {:dashboard_id dashboard-id, :card_id card-id}]
+                  DashboardCardSeries [_                  {:dashboardcard_id dbc_id, :card_id card-id2, :position 0}]]
+    ((user->client :rasta) :get 403 (format "dashboard/%d" dashboard-id))))
 
 ;; ## PUT /api/dashboard/:id
 (expect