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

Long lines cleanup

parent 17b773df
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/embed.clj
+ 32
− 26
View file @ b27e4150
......@@ -35,7 +35,7 @@
[schema.core :as s]
[toucan.db :as db]))
;;; ------------------------------------------------------------ Param Checking ------------------------------------------------------------
;;; ------------------------------------------------- Param Checking -------------------------------------------------
(defn- validate-params-are-allowed
"Check that the conditions specified by `object-embedding-params` are satisfied."
......@@ -64,15 +64,15 @@
[400 (format "Unknown parameter %s." k)]))))
(defn- validate-param-sets
"Validate that sets of params passed as part of the JWT token and by the user (as query params, i.e. as part of the URL)
are valid for the OBJECT-EMBEDDING-PARAMS. TOKEN-PARAMS and USER-PARAMS should be sets of all valid param keys specified in
the JWT or by the user, respectively."
"Validate that sets of params passed as part of the JWT token and by the user (as query params, i.e. as part of the
URL) are valid for the OBJECT-EMBEDDING-PARAMS. TOKEN-PARAMS and USER-PARAMS should be sets of all valid param keys
specified in the JWT or by the user, respectively."
[object-embedding-params token-params user-params]
;; TODO - maybe make this log/debug once embedding is wrapped up
(log/info "Validating params for embedded object:\n"
"object embedding params:" object-embedding-params
"token params:" token-params
"user params:" user-params)
(log/debug "Validating params for embedded object:\n"
"object embedding params:" object-embedding-params
"token params:" token-params
"user params:" user-params)
(validate-params-are-allowed object-embedding-params token-params user-params)
(validate-params-exist object-embedding-params (set/union token-params user-params)))
......@@ -84,9 +84,10 @@
(not (str/blank? v)))))
(s/defn ^:private ^:always-validate validate-params
"Validate that the TOKEN-PARAMS passed in the JWT and the USER-PARAMS (passed as part of the URL) are allowed, and that ones that
are required are specified by checking them against a Card or Dashboard's OBJECT-EMBEDDING-PARAMS (the object's value of
`:embedding_params`). Throws a 400 if any of the checks fail. If all checks are successful, returns a merged parameters map."
"Validate that the TOKEN-PARAMS passed in the JWT and the USER-PARAMS (passed as part of the URL) are allowed, and
that ones that are required are specified by checking them against a Card or Dashboard's OBJECT-EMBEDDING-PARAMS
(the object's value of `:embedding_params`). Throws a 400 if any of the checks fail. If all checks are successful,
returns a merged parameters map."
[object-embedding-params :- su/EmbeddingParams, token-params user-params]
(validate-param-sets object-embedding-params
(set (keys (m/filter-vals valid-param? token-params)))
......@@ -95,7 +96,7 @@
(merge user-params token-params))
;;; ------------------------------------------------------------ Other Param Util Fns ------------------------------------------------------------
;;; ---------------------------------------------- Other Param Util Fns ----------------------------------------------
(defn- remove-params-in-set
"Remove any PARAMS from the list whose `:slug` is in the PARAMS-TO-REMOVE set."
......@@ -105,8 +106,9 @@
param))
(s/defn ^:private ^:always-validate remove-locked-and-disabled-params
"Remove the `:parameters` for DASHBOARD-OR-CARD that listed as `disabled` or `locked` in the EMBEDDING-PARAMS whitelist,
or not present in the whitelist. This is done so the frontend doesn't display widgets for params the user can't set."
"Remove the `:parameters` for DASHBOARD-OR-CARD that listed as `disabled` or `locked` in the EMBEDDING-PARAMS
whitelist, or not present in the whitelist. This is done so the frontend doesn't display widgets for params the user
can't set."
[dashboard-or-card, embedding-params :- su/EmbeddingParams]
(let [params-to-remove (set (concat (for [[param status] embedding-params
:when (not= status "enabled")]
......@@ -144,7 +146,8 @@
(update card :parameters concat (template-tag-parameters card)))
(defn- apply-parameter-values
"Adds `value` to parameters with `slug` matching a key in `parameter-values` and removes parameters without a `value`"
"Adds `value` to parameters with `slug` matching a key in `parameter-values` and removes parameters without a
`value`."
[parameters parameter-values]
(for [param parameters
:let [value (get parameter-values (keyword (:slug param)))]
......@@ -163,9 +166,9 @@
"Returns parameters for a card on a dashboard with `:target` resolved via `:parameter_mappings`."
[dashboard-id dashcard-id card-id]
(let [param-id->param (u/key-by :id (db/select-one-field :parameters Dashboard :id dashboard-id))]
;; throw a 404 if there's no matching DashboardCard so people can't get info about other Cards that aren't in this Dashboard
;; we don't need to check that card-id matches the DashboardCard because we might be trying to get param info for
;; a series belonging to this dashcard (card-id might be for a series)
;; throw a 404 if there's no matching DashboardCard so people can't get info about other Cards that aren't in this
;; Dashboard we don't need to check that card-id matches the DashboardCard because we might be trying to get param
;; info for a series belonging to this dashcard (card-id might be for a series)
(for [param-mapping (api/check-404 (db/select-one-field :parameter_mappings DashboardCard
:id dashcard-id
:dashboard_id dashboard-id))
......@@ -175,7 +178,7 @@
(assoc param :target (:target param-mapping)))))
;;; ------------------------------------------------------------ Card Fns used by both /api/embed and /api/preview_embed ------------------------------------------------------------
;;; ---------------------------- Card Fns used by both /api/embed and /api/preview_embed -----------------------------
(defn card-for-unsigned-token
"Return the info needed for embedding about Card specified in TOKEN.
......@@ -201,7 +204,7 @@
(apply public-api/run-query-for-card-with-id card-id parameters, :context :embedded-question, options)))
;;; ------------------------------------------------------------ Dashboard Fns used by both /api/embed and /api/preview_embed ------------------------------------------------------------
;;; -------------------------- Dashboard Fns used by both /api/embed and /api/preview_embed --------------------------
(defn dashboard-for-unsigned-token
"Return the info needed for embedding about Dashboard specified in TOKEN.
......@@ -219,13 +222,14 @@
"Return results for running the query belonging to a DashboardCard."
{:style/indent 0}
[& {:keys [dashboard-id dashcard-id card-id embedding-params token-params query-params]}]
{:pre [(integer? dashboard-id) (integer? dashcard-id) (integer? card-id) (u/maybe? map? embedding-params) (map? token-params) (map? query-params)]}
{:pre [(integer? dashboard-id) (integer? dashcard-id) (integer? card-id) (u/maybe? map? embedding-params)
(map? token-params) (map? query-params)]}
(let [parameter-values (validate-params embedding-params token-params query-params)
parameters (apply-parameter-values (resolve-dashboard-parameters dashboard-id dashcard-id card-id) parameter-values)]
(public-api/public-dashcard-results dashboard-id card-id parameters, :context :embedded-dashboard)))
;;; ------------------------------------------------------------ Other /api/embed-specific utility fns ------------------------------------------------------------
;;; ------------------------------------- Other /api/embed-specific utility fns --------------------------------------
(defn- check-embedding-enabled-for-object
"Check that embedding is enabled, that OBJECT exists, and embedding for OBJECT is enabled."
......@@ -242,7 +246,7 @@
(def ^:private ^{:arglists '([card-id])} check-embedding-enabled-for-card (partial check-embedding-enabled-for-object Card))
;;; ------------------------------------------------------------ /api/embed/card endpoints ------------------------------------------------------------
;;; ------------------------------------------- /api/embed/card endpoints --------------------------------------------
(api/defendpoint GET "/card/:token"
"Fetch a Card via a JSON Web Token signed with the `embedding-secret-key`.
......@@ -257,7 +261,8 @@
(defn- run-query-for-unsigned-token
"Run the query belonging to Card identified by UNSIGNED-TOKEN. Checks that embedding is enabled both globally and for this Card."
"Run the query belonging to Card identified by UNSIGNED-TOKEN. Checks that embedding is enabled both globally and
for this Card."
[unsigned-token query-params & options]
(let [card-id (eu/get-in-unsigned-token-or-throw unsigned-token [:resource :question])]
(check-embedding-enabled-for-card card-id)
......@@ -288,7 +293,7 @@
(run-query-for-unsigned-token (eu/unsign token) query-params, :constraints nil)))
;;; ------------------------------------------------------------ /api/embed/dashboard endpoints ------------------------------------------------------------
;;; ----------------------------------------- /api/embed/dashboard endpoints -----------------------------------------
(api/defendpoint GET "/dashboard/:token"
......@@ -304,7 +309,8 @@
(api/defendpoint GET "/dashboard/:token/dashcard/:dashcard-id/card/:card-id"
"Fetch the results of running a Card belonging to a Dashboard using a JSON Web Token signed with the `embedding-secret-key`.
"Fetch the results of running a Card belonging to a Dashboard using a JSON Web Token signed with the
`embedding-secret-key`.
Token should have the following format:
......
src/metabase/routes.clj
+ 7
− 2
View file @ b27e4150
(ns metabase.routes
"Main Compojure routes tables. See https://github.com/weavejester/compojure/wiki/Routes-In-Detail for details about
how these work. `/api/` routes are in `metabase.api.routes`."
(:require [cheshire.core :as json]
[clojure.java.io :as io]
[clojure.string :as str]
......@@ -47,7 +49,9 @@
(fallback-localization *locale*)))
(fallback-localization *locale*)))
(defn- entrypoint [entry embeddable? {:keys [uri]}]
(defn- entrypoint
"Repsonse that serves up an entrypoint into the Metabase application, e.g. `index.html`."
[entry embeddable? {:keys [uri]}]
(-> (if (init-status/complete?)
(load-template (str "frontend_client/" entry ".html")
{:bootstrap_json (escape-script (json/generate-string (public-settings/public-settings)))
......@@ -86,7 +90,8 @@
{:status 503, :body {:status "initializing", :progress (init-status/progress)}}))
;; ^/api/ -> All other API routes
(context "/api" [] (fn [& args]
;; if Metabase is not finished initializing, return a generic error message rather than something potentially confusing like "DB is not set up"
;; if Metabase is not finished initializing, return a generic error message rather than
;; something potentially confusing like "DB is not set up"
(if-not (init-status/complete?)
{:status 503, :body "Metabase is still initializing. Please sit tight..."}
(apply api/routes args))))
......
test/metabase/api/embed_test.clj
+ 43
− 22
View file @ b27e4150
(ns metabase.api.embed-test
"Tests for /api/embed endpoints."
(:require [buddy.sign.jwt :as jwt]
[crypto.random :as crypto-random]
[dk.ative.docjure.spreadsheet :as spreadsheet]
......@@ -48,7 +49,9 @@
(defmacro with-temp-dashcard {:style/indent 1} [[dashcard-binding {:keys [dash card dashcard]}] & body]
`(with-temp-card [card# ~card]
(tt/with-temp* [Dashboard [dash# ~dash]
DashboardCard [~dashcard-binding (merge {:card_id (u/get-id card#), :dashboard_id (u/get-id dash#)} ~dashcard)]]
DashboardCard [~dashcard-binding (merge {:card_id (u/get-id card#)
:dashboard_id (u/get-id dash#)}
~dashcard)]]
~@body)))
(defmacro with-embedding-enabled-and-new-secret-key {:style/indent 0} [& body]
......@@ -59,9 +62,9 @@
(defn successful-query-results
([]
{:data {:columns ["count"]
:cols [{:description nil, :table_id nil, :special_type "type/Number", :name "count", :source "aggregation",
:extra_info {}, :id nil, :target nil, :display_name "count", :base_type "type/Integer"
:remapped_from nil, :remapped_to nil}]
:cols [{:description nil, :table_id nil, :special_type "type/Number", :name "count",
:source "aggregation", :extra_info {}, :id nil, :target nil, :display_name "count",
:base_type "type/Integer", :remapped_from nil, :remapped_to nil}]
:rows [[100]]}
:json_query {:parameters []}
:status "completed"})
......@@ -95,7 +98,7 @@
{:description nil, :parameters (), :ordered_cards (), :param_values nil})
;; ------------------------------------------------------------ GET /api/embed/card/:token ------------------------------------------------------------
;;; ------------------------------------------- GET /api/embed/card/:token -------------------------------------------
(defn- card-url [card & [additional-token-params]] (str "embed/card/" (card-token card additional-token-params)))
......@@ -122,7 +125,8 @@
(with-temp-card [card]
(http/client :get 400 (card-url card)))))
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong key
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong
;; key
(expect
"Message seems corrupt or manipulated."
(with-embedding-enabled-and-new-secret-key
......@@ -142,7 +146,7 @@
(:parameters (http/client :get 200 (card-url card {:params {:c 100}}))))))
;; ------------------------------------------------------------ GET /api/embed/card/:token/query (and JSON/CSV/XLSX variants) ------------------------------------------------------------
;;; ------------------------- GET /api/embed/card/:token/query (and JSON/CSV/XLSX variants) --------------------------
(defn- card-query-url [card response-format & [additional-token-params]]
(str "embed/card/"
......@@ -150,7 +154,9 @@
"/query"
response-format))
(defmacro ^:private expect-for-response-formats {:style/indent 1} [[response-format-binding request-options-binding] expected actual]
(defmacro ^:private expect-for-response-formats
{:style/indent 1}
[[response-format-binding request-options-binding] expected actual]
`(do
~@(for [[response-format request-options] [[""] ["/json"] ["/csv"] ["/xlsx" {:as :byte-array}]]]
`(expect
......@@ -168,11 +174,14 @@
(with-temp-card [card {:enable_embedding true}]
(http/client :get 200 (card-query-url card response-format) request-options))))
;; but if the card has an invalid query we should just get a generic "query failed" exception (rather than leaking query info)
;; but if the card has an invalid query we should just get a generic "query failed" exception (rather than leaking
;; query info)
(expect-for-response-formats [response-format]
"An error occurred while running the query."
(with-embedding-enabled-and-new-secret-key
(with-temp-card [card {:enable_embedding true, :dataset_query {:database (data/id), :type :native, :native {:query "SELECT * FROM XYZ"}}}]
(with-temp-card [card {:enable_embedding true, :dataset_query {:database (data/id)
:type :native
:native {:query "SELECT * FROM XYZ"}}}]
(http/client :get 400 (card-query-url card response-format)))))
;; check that the endpoint doesn't work if embedding isn't enabled
......@@ -190,16 +199,19 @@
(with-temp-card [card]
(http/client :get 400 (card-query-url card response-format)))))
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong key
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong
;; key
(expect-for-response-formats [response-format]
"Message seems corrupt or manipulated."
(with-embedding-enabled-and-new-secret-key
(with-temp-card [card {:enable_embedding true}]
(http/client :get 400 (with-new-secret-key (card-query-url card response-format))))))
;;; LOCKED params
;; check that if embedding is enabled globally and for the object requests fail if the token is missing a `:locked` parameter
;; check that if embedding is enabled globally and for the object requests fail if the token is missing a `:locked`
;; parameter
(expect-for-response-formats [response-format]
"You must specify a value for :abc in the JWT."
(with-embedding-enabled-and-new-secret-key
......@@ -220,6 +232,7 @@
(with-temp-card [card {:enable_embedding true, :embedding_params {:abc "locked"}}]
(http/client :get 400 (str (card-query-url card response-format {:params {:abc 100}}) "?abc=100")))))
;;; DISABLED params
;; check that if embedding is enabled globally and for the object requests fail if they pass a `:disabled` parameter
......@@ -260,9 +273,10 @@
(http/client :get 200 (str (card-query-url card response-format) "?abc=200") request-options))))
;; ------------------------------------------------------------ GET /api/embed/dashboard/:token ------------------------------------------------------------
;;; ---------------------------------------- GET /api/embed/dashboard/:token -----------------------------------------
(defn- dashboard-url [dashboard & [additional-token-params]] (str "embed/dashboard/" (dash-token dashboard additional-token-params)))
(defn- dashboard-url [dashboard & [additional-token-params]]
(str "embed/dashboard/" (dash-token dashboard additional-token-params)))
;; it should be possible to call this endpoint successfully
(expect
......@@ -287,7 +301,8 @@
(tt/with-temp Dashboard [dash]
(http/client :get 400 (dashboard-url dash)))))
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong key
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong
;; key
(expect
"Message seems corrupt or manipulated."
(with-embedding-enabled-and-new-secret-key
......@@ -307,7 +322,7 @@
(:parameters (http/client :get 200 (dashboard-url dash {:params {:c 100}}))))))
;; ------------------------------------------------------------ GET /api/embed/dashboard/:token/dashcard/:dashcard-id/card/:card-id ------------------------------------------------------------
;;; ---------------------- GET /api/embed/dashboard/:token/dashcard/:dashcard-id/card/:card-id -----------------------
(defn- dashcard-url [dashcard & [additional-token-params]]
(str "embed/dashboard/" (dash-token (:dashboard_id dashcard) additional-token-params)
......@@ -321,12 +336,15 @@
(with-temp-dashcard [dashcard {:dash {:enable_embedding true}}]
(http/client :get 200 (dashcard-url dashcard)))))
;; but if the card has an invalid query we should just get a generic "query failed" exception (rather than leaking query info)
;; but if the card has an invalid query we should just get a generic "query failed" exception (rather than leaking
;; query info)
(expect
"An error occurred while running the query."
(with-embedding-enabled-and-new-secret-key
(with-temp-dashcard [dashcard {:dash {:enable_embedding true}
:card {:dataset_query {:database (data/id), :type :native, :native {:query "SELECT * FROM XYZ"}}}}]
:card {:dataset_query {:database (data/id)
:type :native,
:native {:query "SELECT * FROM XYZ"}}}}]
(http/client :get 400 (dashcard-url dashcard)))))
;; check that the endpoint doesn't work if embedding isn't enabled
......@@ -344,7 +362,8 @@
(with-temp-dashcard [dashcard]
(http/client :get 400 (dashcard-url dashcard)))))
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong key
;; check that if embedding is enabled globally and for the object that requests fail if they are signed with the wrong
;; key
(expect
"Message seems corrupt or manipulated."
(with-embedding-enabled-and-new-secret-key
......@@ -353,7 +372,8 @@
;;; LOCKED params
;; check that if embedding is enabled globally and for the object requests fail if the token is missing a `:locked` parameter
;; check that if embedding is enabled globally and for the object requests fail if the token is missing a `:locked`
;; parameter
(expect
"You must specify a value for :abc in the JWT."
(with-embedding-enabled-and-new-secret-key
......@@ -414,9 +434,10 @@
(http/client :get 200 (str (dashcard-url dashcard) "?abc=200")))))
;;; ------------------------------------------------------------ Other Tests ------------------------------------------------------------
;;; -------------------------------------------------- Other Tests ---------------------------------------------------
;; parameters that are not in the `embedding-params` map at all should get removed by `remove-locked-and-disabled-params`
;; parameters that are not in the `embedding-params` map at all should get removed by
;; `remove-locked-and-disabled-params`
(expect
{:parameters []}
(#'embed-api/remove-locked-and-disabled-params {:parameters {:slug "foo"}} {}))
......
test/metabase/api/public_test.clj
+ 11
− 7
View file @ b27e4150
......@@ -22,7 +22,7 @@
(:import java.io.ByteArrayInputStream
java.util.UUID))
;;; ------------------------------------------------------------ Helper Fns ------------------------------------------------------------
;;; --------------------------------------------------- Helper Fns ---------------------------------------------------
(defn count-of-venues-card []
{:dataset_query {:database (data/id)
......@@ -62,7 +62,7 @@
;;; ------------------------------------------------------------ GET /api/public/card/:uuid ------------------------------------------------------------
;;; ------------------------------------------- GET /api/public/card/:uuid -------------------------------------------
;; Check that we *cannot* fetch a PublicCard if the setting is disabled
(expect
......@@ -99,7 +99,10 @@
:human_readable_values {}
:field_id (data/id :categories :name)}}
(tt/with-temp Card [card {:dataset_query {:type :native
:native {:query "SELECT COUNT(*) FROM venues LEFT JOIN categories ON venues.category_id = categories.id WHERE {{category}}"
:native {:query (str "SELECT COUNT(*) "
"FROM venues "
"LEFT JOIN categories ON venues.category_id = categories.id "
"WHERE {{category}}")
:collection "CATEGORIES"
:template_tags {:category {:name "category"
:display_name "Category"
......@@ -111,7 +114,7 @@
(update-in [(data/id :categories :name) :values] count))))
;;; ------------------------------------------------------------ GET /api/public/card/:uuid/query (and JSON/CSV/XSLX versions) ------------------------------------------------------------
;;; ------------------------- GET /api/public/card/:uuid/query (and JSON/CSV/XSLX versions) --------------------------
;; Check that we *cannot* execute a PublicCard if the setting is disabled
(expect
......@@ -173,8 +176,9 @@
(get-in (http/client :get 200 (str "public/card/" uuid "/query"), :parameters (json/encode [{:type "category", :value 2}]))
[:json_query :parameters]))))
;; make sure CSV (etc.) downloads take editable params into account (#6407)
;;; ------------------------------------------------------------ GET /api/public/dashboard/:uuid ------------------------------------------------------------
;;; ---------------------------------------- GET /api/public/dashboard/:uuid -----------------------------------------
;; Check that we *cannot* fetch PublicDashboard if setting is disabled
(expect
......@@ -211,7 +215,7 @@
(fetch-public-dashboard dash))))
;;; ------------------------------------------------------------ GET /api/public/dashboard/:uuid/card/:card-id ------------------------------------------------------------
;;; --------------------------------- GET /api/public/dashboard/:uuid/card/:card-id ----------------------------------
(defn- dashcard-url-path [dash card]
(str "public/dashboard/" (:public_uuid dash) "/card/" (u/get-id card)))
......@@ -281,7 +285,7 @@
(qp-test/rows (http/client :get 200 (dashcard-url-path dash card-2))))))))
;;; ------------------------------------------------------------ Check that parameter information comes back with Dashboard ------------------------------------------------------------
;;; --------------------------- Check that parameter information comes back with Dashboard ---------------------------
;; double-check that the Field has FieldValues
(expect
......
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