Skip to content
Snippets Groups Projects
Unverified Commit 627feee0 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Merge pull request #6329 from metabase/check-pulse-perms-before-deleting 

Check Pulse permissions before deleting
parents 94595879 47205fa9
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 @ 627feee0
......@@ -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 @ 627feee0
......@@ -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 @ 627feee0
......@@ -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.
Finish editing this message first!
Please register or to comment