insert next-gen permission paths alongside currently active permission paths +...

insert next-gen permission paths alongside currently active permission paths + classification (#27911)

* implement move, which returns v2 paths

- TODO: insert these into the db

(move v1-path) => [v2 paths]

* cleanup + add some schemas

* generative tests 4 permission path classification

* whitespace lint

* detect data, query, and paths for v2

* calling move on v2 paths is a no-op

* differentiate between v1 and v2 permissions

quickchecking for move, classify-path, and classify-data-path

* fix tests + add idempotency test

* add tests for classification of permission paths

- rename move to ->v2-path
- move some fxns around
- ascii art in test

* making the legos line up

- need to insert both v1 and v2 versions of paths (of course)
- valid-path? has to allow v2 paths to be inserted

* replace mu/with-api-error-message

* linter + code quality fixes

* privatize rx->kind

* remove some changes that should be 4 anotherbranch

* revert ns

* delete v2 permissions in

- they aren't handled by the perm graph parser, so they don't get propagated into "old graph", so the diff between old and new indicates that they need to be rewritten.

* only delete v2 paths for the current group_id

- reorder declarations in models.permissions

* remove extra line break

* Update src/metabase/models/permissions.clj

fix typo

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

---------

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

.dir-locals.el

@@ -61,6 +61,7 @@
   (eval . (put-clojure-indent 'u/prog1 1))
   (eval . (put-clojure-indent 'u/select-keys-when 1))
   (eval . (put-clojure-indent 'with-meta '(:form)))
+  (eval . (put-clojure-indent 'tc/quick-check 1))
   ;; these ones have to be done with `define-clojure-indent' for now because of upstream bug
   ;; https://github.com/clojure-emacs/clojure-mode/issues/600 once that's resolved we should use `put-clojure-indent'
   ;; instead. Please don't add new entries unless they don't work with `put-clojure-indent'

dev/src/dev.clj

@@ -45,6 +45,7 @@
 
 (defn stop!
   []
+  (malli-dev/stop!)
   (metabase.server/stop-web-server!))
 
 (defn restart!

enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj

@@ -249,7 +249,7 @@
       preprocess-source-query
       (source-query-form-ensure-metadata table-id card-id)))
 
-(s/defn ^:private gtap->perms-set :- #{perms/Path}
+(s/defn ^:private gtap->perms-set :- #{perms/PathSchema}
   "Calculate the set of permissions needed to run the query associated with a GTAP; this set of permissions is excluded
   during the normal QP perms check.
 

src/metabase/api/search.clj

@@ -29,7 +29,7 @@
   "Map with the various allowed search parameters, used to construct the SQL query"
   {:search-string                (s/maybe su/NonBlankString)
    :archived?                    s/Bool
-   :current-user-perms           #{perms/Path}
+   :current-user-perms           #{perms/PathSchema}
    (s/optional-key :models)      (s/maybe #{su/NonBlankString})
    (s/optional-key :table-db-id) (s/maybe s/Int)
    (s/optional-key :limit-int)   (s/maybe s/Int)

src/metabase/models/collection.clj

@@ -538,7 +538,7 @@
 ;;; |                                    Recursive Operations: Moving & Archiving                                    |
 ;;; +----------------------------------------------------------------------------------------------------------------+
 
-(s/defn perms-for-archiving :- #{perms/Path}
+(s/defn perms-for-archiving :- #{perms/PathSchema}
   "Return the set of Permissions needed to archive or unarchive a `collection`. Since archiving a Collection is
   *recursive* (i.e., it applies to all the descendant Collections of that Collection), we require write ('curate')
   permissions for the Collection itself and all its descendants, but not for its parent Collection.
@@ -567,7 +567,7 @@
                             (db/select-ids Collection :location [:like (str (children-location collection) "%")])))]
      (perms/collection-readwrite-path collection-or-id))))
 
-(s/defn perms-for-moving :- #{perms/Path}
+(s/defn perms-for-moving :- #{perms/PathSchema}
   "Return the set of Permissions needed to move a `collection`. Like archiving, moving is recursive, so we require
   perms for both the Collection and its descendants; we additionally require permissions for its new parent Collection.
 

src/metabase/models/permissions/parse.clj

@@ -157,7 +157,7 @@
   [paths]
   (->> paths
        (reduce (fn [paths path]
-                 (if (every? vector? path) ;; handle case wher /db/x/ returns two vectors
+                 (if (every? vector? path) ;; handle case where /db/x/ returns two vectors
                    (into paths path)
                    (conj paths path)))
                [])
@@ -178,6 +178,7 @@
                                    [:block :all :some :write :read :segmented :full :limited :yes]))
                            x)))))
 
+
 (defn permissions->graph
   "Given a set of permission strings, return a graph that expresses the most permissions possible for the set"
   [permissions]

src/metabase/models/query/permissions.clj

@@ -69,7 +69,7 @@
    (s/eq ::native)
    su/IntGreaterThanZero))
 
-(s/defn tables->permissions-path-set :- #{perms/Path}
+(s/defn tables->permissions-path-set :- #{perms/PathSchema}
   "Given a sequence of `tables-or-ids` referenced by a query, return a set of required permissions. A truthy value for
   `segmented-perms?` will return segmented permissions for the table rather that full table permissions.
 
@@ -100,7 +100,7 @@
                              (table-or-id->schema table-or-id)
                              (u/the-id table-or-id)))))))
 
-(s/defn ^:private source-card-read-perms :- #{perms/Path}
+(s/defn ^:private source-card-read-perms :- #{perms/PathSchema}
   "Calculate the permissions needed to run an ad-hoc query that uses a Card with `source-card-id` as its source
   query."
   [source-card-id :- su/IntGreaterThanZero]
@@ -115,7 +115,7 @@
   (binding [api/*current-user-id* nil]
     ((resolve 'metabase.query-processor/preprocess) query)))
 
-(s/defn ^:private mbql-permissions-path-set :- #{perms/Path}
+(s/defn ^:private mbql-permissions-path-set :- #{perms/PathSchema}
   "Return the set of required permissions needed to run an adhoc `query`.
 
   Also optionally specify `throw-exceptions?` -- normally this function avoids throwing Exceptions to avoid breaking
@@ -145,7 +145,7 @@
         (log/error e))
       #{"/db/0/"})))                    ; DB 0 will never exist
 
-(s/defn ^:private perms-set* :- #{perms/Path}
+(s/defn ^:private perms-set* :- #{perms/PathSchema}
   "Does the heavy lifting of creating the perms set. `opts` will indicate whether exceptions should be thrown and
   whether full or segmented table permissions should be returned."
   [{query-type :type, database :database, :as query} perms-opts :- PermsOptions]

src/metabase/util/regex.clj

@@ -10,7 +10,7 @@
 
 (defn re-or
   "Combine regex `patterns` into a single pattern by joining with or (i.e., a logical disjunction)."
-  [& patterns]
+  [patterns]
   (non-capturing-group (str/join "|" (map non-capturing-group patterns))))
 
 (defn re-optional
@@ -18,50 +18,51 @@
   [pattern]
   (str (non-capturing-group pattern) "?"))
 
+(defn re-negate
+  "Make regex `pattern` negated."
+  [pattern]
+  (str "(?!" (non-capturing-group pattern) "$).*"))
+
 (defmulti ^:private rx-dispatch
   {:arglists '([listt])}
   first)
 
 (declare rx*)
 
-(defmethod rx-dispatch :default
-  [x]
-  x)
+(defmethod rx-dispatch :default [x] x)
 
-(defmethod rx-dispatch 'opt
+(defmethod rx-dispatch :?
   [[_ & args]]
-  `(re-optional (rx* (~'and ~@args))))
+  (re-optional (rx* (into [:and] args))))
 
-(defmethod rx-dispatch 'or
+(defmethod rx-dispatch :or
   [[_ & args]]
-  `(re-or ~@(for [arg args]
-              `(rx* ~arg))))
+  (re-or (map rx* args)))
 
-(defmethod rx-dispatch 'and
+(defmethod rx-dispatch :and
   [[_ & args]]
-  `(str ~@(for [arg args]
-            `(rx* ~arg))))
+  (apply str (map rx* args)))
 
-(defmacro rx*
-  "Impl of `rx` macro."
-  [x]
-  (if (seqable? x)
-    (rx-dispatch x)
-    x))
+(defmethod rx-dispatch :not
+  [[_ arg]]
+  (re-negate (rx* arg)))
 
-(defmacro rx
-  "Cam's quick-and-dirty port of the Emacs Lisp `rx` macro (`C-h f rx`) but not currently as fully-featured. Convenient
-  macro for building mega-huge regular expressions from a sexpr representation.
+(defn- rx*
+  [x]
+  (if (seqable? x) (rx-dispatch x) x))
 
-    (rx (and (or \"Cam\" \"can\") (opt #\"\\s+\") #\"\\d+\"))
-    ;; -> #\"(?:(?:Cam)|(?:can))\\s+?\\d+\"
+(def ^{:doc
+       "A quick-and-dirty port of the Emacs Lisp `rx` macro (`C-h f rx`) implemented as a function but not currently as fully-featured.
+       Convenient for building mega-huge regular expressions from a hiccup-like representation.
+       Feel free to add support for more stuff as needed.
 
-  Feel free to add support for more stuff as needed.
-  ([x]
-   `(re-pattern (str (rx* ~x))))"
+       This is memoized because arguments to rx are less optimal than they should be, in favor of better clarity -- hence skipping recompilation makes sense."
+       :arglists '([x] [x & more])
+       } rx
+  (memoize (fn  rx
+             ;; (rx [:and [:or "Cam" "can"] [:? #"\s+"] #"\d+"])
+             ;; -> #\"(?:(?:Cam)|(?:can))(?:\s+)?\d+\"
 
-  ([x]
-   `(re-pattern (rx* ~x)))
+             ([x] (re-pattern (rx* x)))
 
-  ([x & more]
-   `(rx (~'and ~x ~@more))))
+             ([x & more] (rx (into [:and x] more))))))

test/metabase/api/card_test.clj

@@ -737,8 +737,8 @@
           (testing "Permissions errors should be meaningful and include info for debugging (#14931)"
             (is (schema= {:message        (s/eq "You cannot save this Question because you do not have permissions to run its query.")
                           :query          (s/eq (mt/obj->json->obj query))
-                          :required-perms [perms/Path]
-                          :actual-perms   [perms/Path]
+                          :required-perms [perms/PathSchema]
+                          :actual-perms   [perms/PathSchema]
                           :trace          [s/Any]
                           s/Keyword       s/Any}
                          (create-card! :rasta 403)))))))))
@@ -1264,8 +1264,8 @@
               (testing "Permissions errors should be meaningful and include info for debugging (#14931)"
                 (is (schema= {:message        (s/eq "You cannot save this Question because you do not have permissions to run its query.")
                               :query          (s/eq (mt/obj->json->obj (mt/mbql-query users)))
-                              :required-perms [perms/Path]
-                              :actual-perms   [perms/Path]
+                              :required-perms [perms/PathSchema]
+                              :actual-perms   [perms/PathSchema]
                               :trace          [s/Any]
                               s/Keyword       s/Any}
                              (update-card! :rasta 403 {:dataset_query (mt/mbql-query users)}))))

test/metabase/models/permissions_group_test.clj

@@ -69,7 +69,7 @@
 
 (s/defn ^:private group-has-full-access?
   "Does a group have permissions for `object` and *all* of its children?"
-  [group-id :- su/IntGreaterThanOrEqualToZero object :- perms/Path]
+  [group-id :- su/IntGreaterThanOrEqualToZero object :- perms/PathSchema]
   ;; e.g. WHERE (object || '%') LIKE '/db/1000/'
   (db/exists? Permissions
     :group_id group-id

test/metabase/models/permissions_test.clj

 (ns metabase.models.permissions-test
   (:require
    [clojure.test :refer :all]
+   [malli.generator :as mg]
    [metabase.models.collection :as collection :refer [Collection]]
    [metabase.models.database :refer [Database]]
    [metabase.models.permissions :as perms :refer [Permissions]]
@@ -856,3 +857,114 @@
         (testing (format "Able to revoke `%s` permission" (name perm-type))
           (perms/revoke-application-permissions! group-id perm-type)
           (is (not (= (perms) #{perm-path}))))))))
+
+(deftest permission-classify-path
+  (is (= :admin           (perms/classify-path "/")))
+  (is (= :block           (perms/classify-path "/block/db/0/")))
+  (is (= :collection      (perms/classify-path "/collection/7/")))
+  (is (= :data            (perms/classify-path "/db/3/")))
+  (is (= :data-model      (perms/classify-path "/data-model/db/0/schema/\\/\\/\\/񊏱\\\\\\\\\\\\򍕦\\/\\/\\\\\\/\\/񴹰󿧊񢣣\\/𪄬\\/\\\\\\/񺟭\\\\򾔪\\/\\\\\\/򛱞򰫰\\\\𰑨񓊼\\\\\\/\\\\\\/\\/\\/򐮫\\/\\\\妕\\/\\/򆀀񚷮\\/\\/󷨾򂁚\\\\\\/󽝅\\/\\\\\\/\\\\\\\\񡳮񯲱󴲱\\\\𹂆𦅧\\\\񰑯\\/\\/\\\\\\\\񜍖\\\\\\\\\\/\\/\\\\\\\\񯦊\\/󗦯\\/\\\\\\\\􇿤\\\\\\\\򄩂\\/𵚰񝐜\\\\\\\\􅸷\\/\\\\󙼳\\\\򍁀񷓧\\\\򛍐𽸁\\\\񂲝񢄘\\\\􊻄\\/𰊸\\/\\/\\\\\\\\󭽾򼪿\\/𖉠\\/\\\\\\\\\\/򂬘\\\\\\\\\\\\\\/\\\\󉁡\\/\\\\\\\\󰈤\\\\\\\\/table/4/")))
+  (is (= :data-v2         (perms/classify-path "/data/db/3/schema/򰴕\\/\\\\\\/\\\\\\/\\/\\/\\\\\\/󇄤\\\\\\/\\/\\/󬽐\\\\\\/\\/򟇌\\\\񅂲\\/\\/\\/\\/\\\\񋐡􌵬\\\\\\\\񄵕\\/\\/򪰫\\\\򉍼\\/\\/\\/\\/򻍄\\\\񄤆\\\\\\/\\\\\\\\񷱨\\/򷣙\\/񰅡򲏪\\/\\/맳\\\\\\\\\\\\\\/񥟬\\\\蝝\\\\\\/\\/\\/\\\\񶫑\\/\\/\\\\򿄚򜬹\\/򄫑\\/\\/\\\\\\/\\\\\\/񠉅\\\\\\\\󽜔\\/\\\\\\\\\\\\\\\\젭񅄾\\/\\/񵊊\\\\\\/󫊽\\\\𻴄\\\\𳇖\\/\\/\\\\\\\\𨴟򫔌\\/񿶮\\/񦀯덱\\/󤯲\\/򡔾􆎱\\\\\\/\\/󕅀򩤞\\\\\\\\􊍧\\/\\\\\\/󤄹\\\\\\\\\\\\󡎨񃩍𬛫\\\\𗦣\\/󭭤\\\\\\/򈅎\\\\\\/\\\\\\/\\\\\\/🯐\\\\󍬠񸇊񆰕錪󰱗񎛣񆛀\\\\\\\\򻒉𝬘\\\\\\\\򺬅\\/\\\\\\/\\\\򡱉\\\\񸇹\\\\񅴅\\\\㏎𸷙\\\\\\/\\/\\/򤟉\\/\\/񑴍\\/\\/\\/\\/\\\\𠳏\\/\\\\󙇅\\\\\\\\􇟞\\\\\\\\󛞯\\\\\\\\\\\\\\\\󮕧\\\\񇰞񡿳\\/\\/\\\\󒧡\\/\\/\\/\\\\\\\\\\/򍊫\\\\\\/󼹅\\/񘖰񠔕򕀏\\/\\\\񀫸\\/󓤧\\\\\\/\\/\\\\\\/\\/Ꚋ\\\\\\/\\/\\/\\/\\\\󏏎󬂊\\/\\/\\\\􌍎\\\\\\\\ꗴ\\\\\\\\\\\\󡉕\\\\񖮖\\\\\\\\\\\\\\/瓮񋞈򺏽𺵩\\\\򷗇\\/\\\\\\\\\\\\\\/\\\\󀍋󦜩򱽙\\\\\\\\򓩥\\\\\\/\\/\\\\\\\\󂻑\\/򪲮\\/񣁛\\/𝵼\\/\\/򁗩\\/\\\\\\\\\\\\\\\\\\/\\\\􇸧􈣤\\\\򭤃\\/\\\\򽗗\\\\\\\\\\\\\\\\\\/\\/\\\\\\/񒆐򿞿\\/񌥊\\/򼻪\\/\\\\\\/\\\\\\\\\\/\\\\񻽁\\/𹈏\\/\\\\\\\\󃝈\\/\\/\\/\\/𫟗\\\\𦁺\\\\\\/\\\\\\\\ျ􍂮򧚾󹵞\\\\\\/\\\\󮋵\\/\\/𧻄\\/\\\\󊢔\\\\\\/\\\\\\/\\/󵲴\\\\\\\\\\/\\/\\\\𫲉\\/\\\\\\\\󥞥\\\\\\/\\/\\/󯇥\\\\\\\\\\/\\/􄢒\\/\\/\\/\\/򉢬\\/򭻄\\/\\/􀤻\\/\\/񌒾󦼿\\/\\/\\\\񎊢\\/\\\\򚻇\\\\񛀪𦢵\\\\􈀤\\/\\/\\\\\\/\\\\\\\\󀹎/table/3/")))
+  (is (= :db-conn-details (perms/classify-path "/details/db/6/")))
+  (is (= :download        (perms/classify-path "/download/db/7/")))
+  (is (= :execute         (perms/classify-path "/execute/")))
+  (is (= :non-scoped      (perms/classify-path "/application/monitoring/")))
+  (is (= :query-v2        (perms/classify-path "/query/db/0/native/"))))
+
+(deftest data-permissions-classify-path
+  (is (= :data (perms/classify-path "/db/3/")))
+  (is (= :data (perms/classify-path "/db/3/native/")))
+  (is (= :data (perms/classify-path "/db/3/schema/")))
+  (is (= :data (perms/classify-path "/db/3/schema//")))
+  (is (= :data (perms/classify-path "/db/3/schema/secret_base/")))
+  (is (= :data (perms/classify-path "/db/3/schema/secret_base/table/3/")))
+  (is (= :data (perms/classify-path "/db/3/schema/secret_base/table/3/read/")))
+  (is (= :data (perms/classify-path "/db/3/schema/secret_base/table/3/query/")))
+  (is (= :data (perms/classify-path "/db/3/schema/secret_base/table/3/query/segmented/"))))
+
+(deftest data-permissions-v2-migration-data-perm-classification-test
+  (is (= :dk/db                                 (perms/classify-data-path "/db/3/")))
+  (is (= :dk/db-native                          (perms/classify-data-path "/db/3/native/")))
+  (is (= :dk/db-schema                          (perms/classify-data-path "/db/3/schema/")))
+  (is (= :dk/db-schema-name                     (perms/classify-data-path "/db/3/schema//")))
+  (is (= :dk/db-schema-name                     (perms/classify-data-path "/db/3/schema/secret_base/")))
+  (is (= :dk/db-schema-name-and-table           (perms/classify-data-path "/db/3/schema/secret_base/table/3/")))
+  (is (= :dk/db-schema-name-table-and-read      (perms/classify-data-path "/db/3/schema/secret_base/table/3/read/")))
+  (is (= :dk/db-schema-name-table-and-query     (perms/classify-data-path "/db/3/schema/secret_base/table/3/query/")))
+  (is (= :dk/db-schema-name-table-and-segmented (perms/classify-data-path "/db/3/schema/secret_base/table/3/query/segmented/"))))
+
+(deftest idempotent-move-test
+  (let [;; all v1 paths:
+        v1-paths ["/db/3/" "/db/3/native/" "/db/3/schema/" "/db/3/schema//" "/db/3/schema/secret_base/"
+                  "/db/3/schema/secret_base/table/3/" "/db/3/schema/secret_base/table/3/read/"
+                  "/db/3/schema/secret_base/table/3/query/" "/db/3/schema/secret_base/table/3/query/segmented/"]
+        ;; cooresponding v2 paths:
+        v2-paths ["/data/db/3/" "/query/db/3/" "/data/db/3/" "/query/db/3/" "/data/db/3/" "/query/db/3/schema/"
+                  "/data/db/3/schema//" "/query/db/3/schema//" "/data/db/3/schema/secret_base/"
+                  "/query/db/3/schema/secret_base/" "/data/db/3/schema/secret_base/table/3/"
+                  "/query/db/3/schema/secret_base/table/3/" "/data/db/3/schema/secret_base/table/3/"
+                  "/query/db/3/schema/secret_base/table/3/" "/data/db/3/schema/secret_base/table/3/"
+                  "/query/db/3/schema/secret_base/table/3/"]]
+    (is (= v2-paths (mapcat #'perms/->v2-path v1-paths)))
+    (is (= v2-paths (mapcat #'perms/->v2-path v2-paths)))
+    (let [w (partial mapcat #'perms/->v2-path)]
+      (is (= v2-paths (->
+                                    v1-paths
+                          w w                       w w;
+                          w w                       w w;
+                          w w w w w w w w w w w w w w w;
+                          w w w w w w w w w w w w w w w;
+                          w w w                   w w w;
+                          w w      w         w      w w
+                          w w     w w       w w     w w
+                          w w           w           w w
+                          w w           w           w w
+                          w w w w w w w w w w w w w w w;
+                              w w w w w w w w w w w;;;;
+                              w w w w w w w w w w w;;
+                                  w w w w w w w;
+                                      w   w;
+                                      w   w
+                                    w w   w w;
+                                    ) ) ) ) ) )
+
+(deftest data-permissions-v2-migration-move-test
+  (testing "move admin"
+    (is (= ["/"] (#'perms/->v2-path "/"))))
+  (testing "move block"
+    (is (= []
+           (#'perms/->v2-path "/block/db/1/"))))
+  (testing "move data"
+    (is (= ["/data/db/1/" "/query/db/1/"]
+           (#'perms/->v2-path "/db/1/")))
+    (is (= ["/data/db/1/" "/query/db/1/"]
+           (#'perms/->v2-path "/db/1/native/")))
+    (is (= ["/data/db/1/" "/query/db/1/schema/"]
+           (#'perms/->v2-path "/db/1/schema/")))
+    (is (= ["/data/db/1/schema//" "/query/db/1/schema//"]
+           (#'perms/->v2-path "/db/1/schema//")))
+    (is (= ["/data/db/1/schema/PUBLIC/" "/query/db/1/schema/PUBLIC/"]
+           (#'perms/->v2-path "/db/1/schema/PUBLIC/")))
+    (is (= ["/data/db/1/schema/PUBLIC/table/1/" "/query/db/1/schema/PUBLIC/table/1/"]
+           (#'perms/->v2-path "/db/1/schema/PUBLIC/table/1/")))
+    (is (= []
+           (#'perms/->v2-path "/db/1/schema/PUBLIC/table/1/read/")))
+    (is (= ["/data/db/1/schema/PUBLIC/table/1/" "/query/db/1/schema/PUBLIC/table/1/"]
+           (#'perms/->v2-path "/db/1/schema/PUBLIC/table/1/query/")))
+    (is (= ["/data/db/1/schema/PUBLIC/table/1/" "/query/db/1/schema/PUBLIC/table/1/"]
+           (#'perms/->v2-path "/db/1/schema/PUBLIC/table/1/query/segmented/")))))
+
+(defn- check-fn! [fn-var & [iterations]]
+  (let [iterations (or iterations 5000)]
+    (if-let [result ((mg/function-checker (:schema (meta fn-var)) {::mg/=>iterations iterations}) @fn-var)]
+      result
+      {:pass? true :iterations iterations})))
+
+(deftest quickcheck-perm-path-classification-test
+  (is (:pass? (check-fn! #'perms/classify-path))))
+
+(deftest quickcheck-data-path-classification-test
+  (is (:pass? (check-fn! #'perms/classify-data-path))))
+
+(deftest quickcheck-->v2-path-test
+  (is (:pass? (check-fn! #'perms/->v2-path))))

test/metabase/query_processor_test/date_bucketing_test.clj

@@ -1270,10 +1270,8 @@
                                    ;; guess you could make a case for either June 30th or July 1st. I don't really know
                                    ;; how you can get June 29th from this, but that's what Vertica returns. :shrug: The
                                    ;; main thing here is that it's not barfing.
-                                   (or (and "06-" (or "29" "30")) "07-01")
+                                   [:or [:and "06-" [:or "29" "30"]] "07-01"]
                                    ;; We also don't really care if this is returned as a date or a timestamp with or
                                    ;; without time zone.
-                                   (opt (or "T" #"\s")
-                                        "00:00:00"
-                                        (opt "Z")))
+                                   [:? [:or "T" #"\s"] "00:00:00" [:? "Z"]])
                        (first (mt/first-row (qp/process-query query))))))))))))

test/metabase/util/regex_test.clj

@@ -4,10 +4,10 @@
    [metabase.util.regex :as u.regex]))
 
 (deftest ^:parallel rx-test
-  (let [regex (u.regex/rx (and "^" (or "Cam" "can") (opt #"\s+") #"\d+"))]
+  (let [regex (u.regex/rx [:and "^" [:or "Cam" "can"] [:? #"\s+"] #"\d+"])]
     (is (instance? java.util.regex.Pattern regex))
     (is (= (str #"^(?:(?:Cam)|(?:can))(?:\s+)?\d+")
            (str regex)))
     (testing "`opt` with multiple args should work (#21971)"
       (is (= (str #"^2022-(?:(?:06-30)|(?:07-01))(?:(?:(?:T)|(?:\s))00:00:00(?:Z)?)?")
-             (str (u.regex/rx #"^2022-" (or "06-30" "07-01") (opt (or "T" #"\s") "00:00:00" (opt "Z")))))))))
+             (str (u.regex/rx #"^2022-" [:or "06-30" "07-01"] [:? [:or "T" #"\s"] "00:00:00" [:? "Z"]])))))))