Skip to content
Snippets Groups Projects
Unverified Commit f342fe17 authored by dpsutton's avatar dpsutton Committed by GitHub
Browse files

Fix numeric parameter values in jwt (#26969)

* Fix numeric parameter values in jwt

Don't call `seq` on numbers. If its a string, assert it isn't blank,
otherwise check that it is non-null.

* bind (:value request-param)
parent b43eaf7e
No related branches found
No related tags found
No related merge requests found
(ns metabase.query-processor.dashboard
"Code for running a query in the context of a specific DashboardCard."
(:require [clojure.tools.logging :as log]
(:require [clojure.string :as str]
[clojure.tools.logging :as log]
[medley.core :as m]
[metabase.api.common :as api]
[metabase.driver.common.parameters.operators :as params.ops]
......@@ -73,10 +74,13 @@
request-param
;; if value comes in as a lone value for an operator filter type (as will be the case for embedding) wrap it in a
;; vector so the parameter handling code doesn't explode.
(when (and (params.ops/operator? (:type matching-param))
(seq (:value request-param))
(not (sequential? (:value request-param))))
{:value [(:value request-param)]})
(let [value (:value request-param)]
(when (and (params.ops/operator? (:type matching-param))
(if (string? value)
(not (str/blank? value))
(some? value))
(not (sequential? value)))
{:value [value]}))
{:id param-id
:target (:target matching-mapping)}))))
......
......@@ -11,6 +11,18 @@
;; there are more tests in [[metabase.api.dashboard-test]]
(defn- run-query-for-dashcard [dashboard-id card-id dashcard-id & options]
;; TODO -- we shouldn't do the perms checks if there is no current User context. It seems like API-level perms check
;; stuff doesn't belong in the Dashboard QP namespace
(binding [api/*current-user-permissions-set* (atom #{"/"})]
(apply qp.dashboard/run-query-for-dashcard-async
:dashboard-id dashboard-id
:card-id card-id
:dashcard-id dashcard-id
:run (fn [query info]
(qp/process-query (assoc query :async? false) info))
options)))
(deftest resolve-parameters-validation-test
(api.dashboard-test/with-chain-filter-fixtures [{{dashboard-id :id} :dashboard
{card-id :id} :card
......@@ -32,19 +44,42 @@
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"Invalid parameter type :number/!= for parameter \"_PRICE_\".*"
(resolve-params [{:id "_PRICE_", :value 4, :type :number/!=}])))))))
(defn- run-query-for-dashcard [dashboard-id card-id dashcard-id & options]
;; TODO -- we shouldn't do the perms checks if there is no current User context. It seems like API-level perms check
;; stuff doesn't belong in the Dashboard QP namespace
(binding [api/*current-user-permissions-set* (atom #{"/"})]
(apply qp.dashboard/run-query-for-dashcard-async
:dashboard-id dashboard-id
:card-id card-id
:dashcard-id dashcard-id
:run (fn [query info]
(qp/process-query (assoc query :async? false) info))
options)))
(resolve-params [{:id "_PRICE_", :value 4, :type :number/!=}]))))))
(testing "Resolves new operator type arguments without error (#25031)"
(mt/dataset sample-dataset
(let [query (mt/native-query {:query "select COUNT(*) from \"ORDERS\" where true [[AND quantity={{qty_locked}}]]"
:template-tags {"qty_locked"
{:id "_query_id_"
:name "qty_locked"
:display-name "quantity locked"
:type :number
:default nil}}})]
(mt/with-temp* [Card [{card-id :id} {:dataset_query query}]
Dashboard [{dashboard-id :id} {:parameters [{:name "param"
:slug "param"
:id "_dash_id_"
:type :number/=}]}]
DashboardCard [{dashcard-id :id} {:parameter_mappings [{:parameter_id "_dash_id_"
:card_id card-id
:target [:variable [:template-tag "qty_locked"]]}]
:card_id card-id
:visualization_settings {}
:dashboard_id dashboard-id}]]
(let [params [{:id "_dash_id_" :value 4}]]
(is (= [{:id "_dash_id_"
:type :number/=
:value [4]
:target [:variable [:template-tag "qty_locked"]]}]
(#'qp.dashboard/resolve-params-for-query dashboard-id card-id dashcard-id params)))
;; test the full query with two different values to ensure it is actually used
(is (= [[2391]]
(mt/rows
(run-query-for-dashcard dashboard-id card-id dashcard-id
{:parameters params}))))
(is (= [[2738]]
(mt/rows
(run-query-for-dashcard dashboard-id card-id dashcard-id
{:parameters (assoc-in params [0 :value] 3)}))))))))))
(deftest card-and-dashcard-id-validation-test
(mt/with-temp* [Dashboard [{dashboard-id :id} {:parameters []}]
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment