Merge pull request #3446 from metabase/remove-unreadable-cards-and-series

Remove unreadable cards and series from dashcards 

frontend/src/metabase/dashboard/dashboard.js

@@ -139,6 +139,16 @@ export const fetchCardData = createThunkAction(FETCH_CARD_DATA, function(card, d
             dispatch(clearCardData(card.id, dashcard.id));
         }
 
+        // If the dataset_query was filtered then we don't have permisison to view this card, so
+        // shortcircuit and return a fake 403
+        if (!card.dataset_query) {
+            return {
+                dashcard_id: dashcard.id,
+                card_id: card.id,
+                result: { error: { status: 403 }}
+            };
+        }
+
         let result = null;
 
         // if we have a parameter, apply it to the card query before we execute
@@ -198,7 +208,8 @@ export const fetchDashboard = createThunkAction(FETCH_DASHBOARD, function(dashId
         _.chain(result.ordered_cards)
             .map((dc) => [dc.card].concat(dc.series))
             .flatten()
-            .map(card => card.dataset_query && card.dataset_query.database)
+            .filter(card => card && card.dataset_query && card.dataset_query.database)
+            .map(card => card.dataset_query.database)
             .uniq()
             .each((dbId) => dispatch(fetchDatabaseMetadata(dbId)));
 

src/metabase/api/dashboard.clj

@@ -37,12 +37,28 @@
    parameters [ArrayOfMaps]}
   (dashboard/create-dashboard! dashboard *current-user-id*))
 
+(defn- hide-unreadable-card
+  "If CARD is unreadable, replace it with an object containing only its `:id`."
+  [card]
+  (if (models/can-read? card)
+    card
+    (select-keys card [:id])))
+
+(defn- hide-unreadable-cards
+  "Replace the `:card` and `:series` entries from dashcards that they user isn't allowed to read with empty objects."
+  [dashboard]
+  (update dashboard :ordered_cards (partial mapv (fn [dashcard]
+                                                   (-> dashcard
+                                                       (update :card hide-unreadable-card)
+                                                       (update :series (partial mapv hide-unreadable-card)))))))
 
 (defendpoint GET "/:id"
   "Get `Dashboard` with ID."
   [id]
-  (u/prog1 (read-check (-> (Dashboard id)
-                           (hydrate :creator [:ordered_cards [:card :creator] :series])))
+  (u/prog1 (-> (Dashboard id)
+               (hydrate :creator [:ordered_cards [:card :creator] :series])
+               read-check
+               hide-unreadable-cards)
     (events/publish-event :dashboard-read (assoc <> :actor_id *current-user-id*))))
 
 

src/metabase/models/dashboard_card.clj

@@ -3,13 +3,26 @@
             (metabase [db :as db]
                       [events :as events])
             (metabase.models  [card :refer [Card]]
-                              [hydrate :refer :all]
                               [dashboard-card-series :refer [DashboardCardSeries]]
+                              [hydrate :refer :all]
                               [interface :as i])
             [metabase.util :as u]))
 
 (i/defentity DashboardCard :report_dashboardcard)
 
+(declare series)
+
+(defn- perms-objects-set
+  "Return the set of permissions required to READ-OR-WRITE this `DashboardCard`.
+  If `:card` and `:series` are already hydrated this method doesn't need to make any DB calls."
+  [dashcard read-or-write]
+  (let [card   (or (:card dashcard)
+                   (db/select-one [Card :dataset_query] :id (u/get-id (:card_id dashcard))))
+        series (or (:series dashcard)
+                   (series dashcard))]
+    (apply set/union (i/perms-objects-set card read-or-write) (for [series-card series]
+                                                                (i/perms-objects-set series-card read-or-write)))))
+
 (defn- pre-insert [dashcard]
   (let [defaults {:sizeX              2
                   :sizeY              2
@@ -24,6 +37,9 @@
   (merge i/IEntityDefaults
          {:timestamped?       (constantly true)
           :types              (constantly {:parameter_mappings :json})
+          :perms-objects-set  perms-objects-set
+          :can-read?          (partial i/current-user-has-full-permissions? :read)
+          :can-write?         (partial i/current-user-has-full-permissions? :write)
           :pre-insert         pre-insert
           :pre-cascade-delete pre-cascade-delete
           :post-select        (u/rpartial set/rename-keys {:sizex :sizeX, :sizey :sizeY})}))