Skip to content
Snippets Groups Projects
Commit 47205fa9 authored by Cam Saul's avatar Cam Saul
Browse files

Check Pulse permissions before deleting

parent 94595879
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/pulse.clj
+ 8
− 3
View file @ 47205fa9
......@@ -78,6 +78,7 @@
"Delete a `Pulse`."
[id]
(api/let-404 [pulse (Pulse id)]
(api/write-check Pulse id)
(db/delete! Pulse :id id)
(events/publish-event! :pulse-delete (assoc pulse :actor_id api/*current-user-id*)))
api/generic-204-no-content)
......@@ -106,8 +107,10 @@
"Get HTML rendering of a `Card` with ID."
[id]
(let [card (api/read-check Card id)
result (qp/process-query-and-save-execution! (:dataset_query card) {:executed-by api/*current-user-id*, :context :pulse, :card-id id})]
{:status 200, :body (html [:html [:body {:style "margin: 0;"} (binding [render/*include-title* true
result (qp/process-query-and-save-execution! (:dataset_query card) {:executed-by api/*current-user-id*
:context :pulse
:card-id id})]
{:status 200, :body (html [:html [:body {:style "margin: 0;"} (binding [render/*include-title* true
render/*include-buttons* true]
(render/render-pulse-card (p/defaulted-timezone card) card result))]])}))
......@@ -115,7 +118,9 @@
"Get JSON object containing HTML rendering of a `Card` with ID and other information."
[id]
(let [card (api/read-check Card id)
result (qp/process-query-and-save-execution! (:dataset_query card) {:executed-by api/*current-user-id*, :context :pulse, :card-id id})
result (qp/process-query-and-save-execution! (:dataset_query card) {:executed-by api/*current-user-id*
:context :pulse
:card-id id})
data (:data result)
card-type (render/detect-pulse-card-type card data)
card-html (html (binding [render/*include-title* true]
......
test/metabase/api/pulse_test.clj
+ 43
− 16
View file @ 47205fa9
......@@ -3,10 +3,16 @@
(:require [expectations :refer :all]
[metabase
[http-client :as http]
[middleware :as middleware]]
[middleware :as middleware]
[util :as u]]
[metabase.models
[card :refer [Card]]
[pulse :as pulse :refer [Pulse]]]
[database :refer [Database]]
[permissions :as perms]
[permissions-group :as perms-group]
[pulse :as pulse :refer [Pulse]]
[pulse-card :refer [PulseCard]]
[table :refer [Table]]]
[metabase.test.data.users :refer :all]
[metabase.test.util :as tu]
[toucan.db :as db]
......@@ -22,7 +28,8 @@
(update :display name)))
(defn- pulse-channel-details [channel]
(select-keys channel [:schedule_type :schedule_details :channel_type :updated_at :details :pulse_id :id :enabled :created_at]))
(select-keys channel [:schedule_type :schedule_details :channel_type :updated_at :details :pulse_id :id :enabled
:created_at]))
(defn- pulse-details [pulse]
(tu/match-$ pulse
......@@ -160,16 +167,17 @@
:details {:channels "#general"}
:recipients []}]
:skip_if_empty false}
(-> (pulse-response ((user->client :rasta) :put 200 (format "pulse/%d" (:id pulse)) {:name "Updated Pulse"
:cards [{:id (:id card)}]
:channels [{:enabled true
:channel_type "slack"
:schedule_type "hourly"
:schedule_hour 12
:schedule_day "mon"
:recipients []
:details {:channels "#general"}}]
:skip_if_empty false}))
(-> (pulse-response ((user->client :rasta) :put 200 (format "pulse/%d" (:id pulse))
{:name "Updated Pulse"
:cards [{:id (:id card)}]
:channels [{:enabled true
:channel_type "slack"
:schedule_type "hourly"
:schedule_hour 12
:schedule_day "mon"
:recipients []
:details {:channels "#general"}}]
:skip_if_empty false}))
(update :channels remove-extra-channels-fields)))
......@@ -180,15 +188,34 @@
((user->client :rasta) :delete 204 (format "pulse/%d" (:id pulse)))
(pulse/retrieve-pulse (:id pulse))))
;; Check that a rando isn't allowed to delete a pulse
(expect
"You don't have permissions to do that."
(tt/with-temp* [Database [{database-id :id}]
Table [{table-id :id} {:db_id database-id}]
Card [card {:dataset_query {:database database-id
:type "query"
:query {:source-table table-id,
:aggregation {:aggregation-type "count"}}}}]
Pulse [pulse {:name "Daily Sad Toucans"}]
PulseCard [pulse-card {:pulse_id (u/get-id pulse), :card_id (u/get-id card)}]]
;; revoke permissions for default group to this database
(perms/delete-related-permissions! (perms-group/all-users) (perms/object-path database-id))
;; now a user without permissions to the Card in question should *not* be allowed to delete the pulse
((user->client :rasta) :delete 403 (format "pulse/%d" (u/get-id pulse)))))
;; ## GET /api/pulse -- should come back in alphabetical order
(tt/expect-with-temp [Pulse [pulse1 {:name "ABCDEF"}]
Pulse [pulse2 {:name "GHIJKL"}]]
[(assoc (pulse-details pulse1) :read_only false)
(assoc (pulse-details pulse2) :read_only false)]
(do (db/delete! Pulse :id [:not-in #{(:id pulse1)
(:id pulse2)}]) ; delete anything else in DB just to be sure; this step may not be neccesary any more
((user->client :rasta) :get 200 "pulse")))
(do
;; delete anything else in DB just to be sure; this step may not be neccesary any more
(db/delete! Pulse :id [:not-in #{(:id pulse1)
(:id pulse2)}])
((user->client :rasta) :get 200 "pulse")))
;; ## GET /api/pulse/:id
......
test/metabase/test/util.clj
+ 5
− 0
View file @ 47205fa9
......@@ -20,6 +20,7 @@
[metric :refer [Metric]]
[permissions-group :refer [PermissionsGroup]]
[pulse :refer [Pulse]]
[pulse-card :refer [PulseCard]]
[pulse-channel :refer [PulseChannel]]
[revision :refer [Revision]]
[segment :refer [Segment]]
......@@ -174,6 +175,10 @@
{:with-temp-defaults (fn [_] {:creator_id (rasta-id)
:name (random-name)})})
(u/strict-extend (class PulseCard)
test/WithTempDefaults
{:with-temp-defaults (fn [_] {:position 0})})
(u/strict-extend (class PulseChannel)
test/WithTempDefaults
{:with-temp-defaults (constantly {:channel_type :email
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment