diff --git a/frontend/src/metabase/dashboard/dashboard.js b/frontend/src/metabase/dashboard/dashboard.js index e0c7b3edd6ed3e8dccbbffc1cc79991216608e34..222b3df30be73dbbc180b6d5530888a367d20663 100644 --- a/frontend/src/metabase/dashboard/dashboard.js +++ b/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))); diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj index 2f8a9b324ac503d30c9af1b013ab12bdab26a7be..9dd7519f485081720050648f444852b573645664 100644 --- a/src/metabase/api/dashboard.clj +++ b/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*)))) diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj index 60d7f509a09dd120cb3a940dc7db82f8adfd2cc0..9f5f5c518660cda1a18afe6e5532aca0aa13128a 100644 --- a/src/metabase/models/dashboard_card.clj +++ b/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})}))