From c46337ad6860d0d2d237f4dac2e2dd27658dd7dd Mon Sep 17 00:00:00 2001
From: Bryan Maass <bryan.maass@gmail.com>
Date: Thu, 2 Feb 2023 16:03:29 -0700
Subject: [PATCH] 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 | 1 +
dev/src/dev.clj | 1 +
.../middleware/row_level_restrictions.clj | 2 +-
src/metabase/api/search.clj | 2 +-
src/metabase/models/collection.clj | 4 +-
src/metabase/models/permissions.clj | 377 +++++++++++-------
src/metabase/models/permissions/parse.clj | 3 +-
src/metabase/models/query/permissions.clj | 8 +-
src/metabase/util/regex.clj | 61 +--
test/metabase/api/card_test.clj | 8 +-
.../models/permissions_group_test.clj | 2 +-
test/metabase/models/permissions_test.clj | 112 ++++++
.../date_bucketing_test.clj | 6 +-
test/metabase/util/regex_test.clj | 4 +-
14 files changed, 407 insertions(+), 184 deletions(-)
diff --git a/.dir-locals.el b/.dir-locals.el
index 2319b09ebfe..4bbfc19d0a3 100644
--- a/.dir-locals.el
+++ b/.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'
diff --git a/dev/src/dev.clj b/dev/src/dev.clj
index b9f3b2dc627..08ec9eceb08 100644
--- a/dev/src/dev.clj
+++ b/dev/src/dev.clj
@@ -45,6 +45,7 @@
(defn stop!
[]
+ (malli-dev/stop!)
(metabase.server/stop-web-server!))
(defn restart!
diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj b/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj
index c768c848efb..f63b9753d4e 100644
--- a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj
+++ b/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.
diff --git a/src/metabase/api/search.clj b/src/metabase/api/search.clj
index 46d48f42424..dc991f63d13 100644
--- a/src/metabase/api/search.clj
+++ b/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)
diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj
index de0ddcdb60f..7cef20e70c0 100644
--- a/src/metabase/models/collection.clj
+++ b/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.
diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj
index fe55445a71c..adefb97ae08 100644
--- a/src/metabase/models/permissions.clj
+++ b/src/metabase/models/permissions.clj
@@ -171,6 +171,7 @@
[clojure.data :as data]
[clojure.string :as str]
[clojure.tools.logging :as log]
+ [malli.core :as mc]
[medley.core :as m]
[metabase.api.common :refer [*current-user-id*]]
[metabase.api.permission-graph :as api.permission-graph]
@@ -202,12 +203,12 @@
;;; -------------------------------------------------- Dynamic Vars --------------------------------------------------
(def ^:dynamic ^Boolean *allow-root-entries*
- "Show we allow permissions entries like `/`? By default, this is disallowed, but you can temporarily disable it here
+ "Should we allow permissions entries like `/`? By default, this is disallowed, but you can temporarily disable it here
when creating the default entry for `Admin`."
false)
(def ^:dynamic ^Boolean *allow-admin-permissions-changes*
- "Show we allow changes to be made to permissions belonging to the Admin group? By default this is disabled to
+ "Should we allow changes to be made to permissions belonging to the Admin group? By default this is disabled to
prevent accidental tragedy, but you can enable it here when creating the default entry for `Admin`."
false)
@@ -221,93 +222,163 @@
1. Any character other than a slash
2. A forward slash, escaped by a backslash: `\\/`
3. A backslash escaped by a backslash: `\\\\`"
- (u.regex/rx (or #"[^\\/]" #"\\/" #"\\\\")))
-
-(def ^:private path-regex
+ (u.regex/rx [:or #"[^\\/]" #"\\/" #"\\\\"]))
+
+(def ^:private data-rx->data-kind
+ { #"db/\d+/" :dk/db
+ [:and #"db/\d+/" "native" "/"] :dk/db-native
+ [:and #"db/\d+/" "schema" "/"] :dk/db-schema
+ [:and #"db/\d+/" "schema" "/" path-char "*" "/"] :dk/db-schema-name
+ [:and #"db/\d+/" "schema" "/" path-char "*" "/table/\\d+/"] :dk/db-schema-name-and-table
+ [:and #"db/\d+/" "schema" "/" path-char "*" "/table/\\d+/" "read/"] :dk/db-schema-name-table-and-read
+ [:and #"db/\d+/" "schema" "/" path-char "*" "/table/\\d+/" "query/"] :dk/db-schema-name-table-and-query
+ [:and #"db/\d+/" "schema" "/" path-char "*" "/table/\\d+/" "query/" "segmented/"] :dk/db-schema-name-table-and-segmented})
+
+(def ^:private DataKind (into [:enum] (vals data-rx->data-kind)))
+
+(def ^:private v1-data-permissions-rx
+ "Paths starting with /db/ is a DATA ACCESS permissions path
+
+ Paths that do not start with /db/ (e.g. /download/db/...) do not involve granting data access, and are not data-permissions.
+ They are other kinds of paths, for example: see [[download-permissions-rx]]."
+ (into [:or] (keys data-rx->data-kind)))
+
+(def ^:private v2-data-permissions-rx [:and "data/" v1-data-permissions-rx])
+(def ^:private v2-query-permissions-rx [:and "query/" v1-data-permissions-rx])
+
+(def ^:private download-permissions-rx
+ "Any path starting with /download/ is a DOWNLOAD permissions path
+ /download/db/:id/ -> permissions to download 1M rows in query results
+ /download/limited/db/:id/ -> permissions to download 1k rows in query results"
+ [:and "download/" [:? "limited/"]
+ [:and #"db/\d+/"
+ [:? [:or "native/"
+ [:and "schema/"
+ [:? [:and path-char "*/"
+ [:? #"table/\d+/"]]]]]]]])
+
+(def ^:private data-model-permissions-rx
+ "Any path starting with /data-model/ is a DATA MODEL permissions path
+ /download/db/:id/ -> permissions to access the data model for the DB"
+ [:and "data-model/"
+ [:and #"db/\d+/"
+ [:? [:and "schema/"
+ [:? [:and path-char "*/"
+ [:? #"table/\d+/"]]]]]]])
+
+(def ^:private db-conn-details-permissions-rx
+ "any path starting with /details/ is a DATABASE CONNECTION DETAILS permissions path
+ /details/db/:id/ -> permissions to edit the connection details and settings for the DB"
+ [:and "details/" #"db/\d+/"])
+
+(def ^:private execute-permissions-rx
+ ".../execute/ -> permissions to run query actions in the DB"
+ [:and "execute/" [:or "" #"db/\d+/"]])
+
+(def ^:private collection-permissions-rx
+ [:and "collection/"
+ [:or ;; /collection/:id/ -> readwrite perms for a specific Collection
+ [:and #"\d+/"
+ ;; /collection/:id/read/ -> read perms for a specific Collection
+ [:? "read/"]]
+ ;; /collection/root/ -> readwrite perms for the Root Collection
+ [:and "root/"
+ ;; /collection/root/read/ -> read perms for the Root Collection
+ [:? "read/"]]
+ ;; /collection/namespace/:namespace/root/ -> readwrite perms for 'Root' Collection in non-default
+ ;; namespace (only really used for EE)
+ [:and "namespace/" path-char "+/root/"
+ ;; /collection/namespace/:namespace/root/read/ -> read perms for 'Root' Collection in
+ ;; non-default namespace
+ [:? "read/"]]]])
+
+(def ^:private non-scoped-permissions-rx
+ "Any path starting with /application is a permissions that is not scoped by database or collection
+ /application/setting/ -> permissions to access /admin/settings page
+ /application/monitoring/ -> permissions to access tools, audit and troubleshooting
+ /application/subscription/ -> permisisons to create/edit subscriptions and alerts"
+ [:and "application/"
+ [:or "setting/" "monitoring/" "subscription/"]])
+
+(def ^:private block-permissions-rx
+ "Any path starting with /block/ is for BLOCK aka anti-permissions.
+ currently only supported at the DB level.
+ e.g. /block/db/1/ => block collection-based access to Database 1"
+ #"block/db/\d+/")
+
+(def ^:private admin-permissions-rx "Root Permissions, i.e. for admin" "")
+
+(def ^:private path-regex-v1
"Regex for a valid permissions path. The [[metabase.util.regex/rx]] macro is used to make the big-and-hairy regex
somewhat readable."
- (u.regex/rx "^/"
- ;; any path containing /db/ is a DATA permissions path
- ;; any path starting with /db/ is a DATA ACCESS permissions path
- (or
- ;; /db/:id/ -> permissions for the entire DB -- native and all schemas
- (and #"db/\d+/"
- (opt (or
- ;; .../native/ -> permissions to create new native queries for the DB
- "native/"
- ;; .../schema/ -> permissions for all schemas in the DB
- (and "schema/"
- ;; .../schema/:name/ -> permissions for a specific schema
- (opt (and path-char "*/"
- ;; .../schema/:name/table/:id/ -> FULL permissions for a specific table
- (opt (and #"table/\d+/"
- (opt (or
- ;; .../read/ -> Perms to fetch the Metadata for Table
- "read/"
- ;; .../query/ -> Perms to run any sort of query against Table
- (and "query/"
- ;; .../segmented/ -> Permissions to run a query against
- ;; a Table using GTAP
- (opt "segmented/"))))))))))))
- ;; any path starting with /download/ is a DOWNLOAD permissions path
- ;; /download/db/:id/ -> permissions to download 1M rows in query results
- ;; /download/limited/db/:id/ -> permissions to download 1k rows in query results
- (and "download/"
- (opt "limited/")
- (and #"db/\d+/"
- (opt (or
- "native/"
- (and "schema/"
- (opt (and path-char "*/"
- (opt #"table/\d+/"))))))))
- ;; any path starting with /data-model/ is a DATA MODEL permissions path
- ;; /download/db/:id/ -> permissions to access the data model for the DB
- (and "data-model/"
- (and #"db/\d+/"
- (opt (and
- "schema/"
- (opt (and path-char "*/"
- (opt #"table/\d+/")))))))
- ;; any path starting with /details/ is a DATABASE CONNECTION DETAILS permissions path
- ;; /details/db/:id/ -> permissions to edit the connection details and settings for the DB
- (and "details/" #"db/\d+/")
- ;; .../execute/ -> permissions to run query actions in the DB
- (and "execute/"
- (or ""
- #"db/\d+/"))
- ;; any path starting with /collection/ is a COLLECTION permissions path
- (and "collection/"
- (or
- ;; /collection/:id/ -> readwrite perms for a specific Collection
- (and #"\d+/"
- ;; /collection/:id/read/ -> read perms for a specific Collection
- (opt "read/"))
- ;; /collection/root/ -> readwrite perms for the Root Collection
- (and "root/"
- ;; /collection/root/read/ -> read perms for the Root Collection
- (opt "read/"))
- ;; /collection/namespace/:namespace/root/ -> readwrite perms for 'Root' Collection in non-default
- ;; namespace (only really used for EE)
- (and "namespace/" path-char "+/root/"
- ;; /collection/namespace/:namespace/root/read/ -> read perms for 'Root' Collection in
- ;; non-default namespace
- (opt "read/"))))
- ;; any path starting with /application is a permissions that is not scoped by database or collection
- ;; /application/setting/ -> permissions to access /admin/settings page
- ;; /application/monitoring/ -> permissions to access tools, audit and troubleshooting
- ;; /application/subscription/ -> permisisons to create/edit subscriptions and alerts
- (and "application/"
- (or
- "setting/"
- "monitoring/"
- "subscription/"))
- ;; any path starting with /block/ is for BLOCK anti-permissions.
- ;; currently only supported at the DB level, e.g. /block/db/1/ => block collection-based access to
- ;; Database 1
- #"block/db/\d+/"
- ;; root permissions, i.e. for admin
- "")
- "$"))
+ (u.regex/rx
+ "^/" [:or
+ v1-data-permissions-rx
+ download-permissions-rx
+ data-model-permissions-rx
+ db-conn-details-permissions-rx
+ execute-permissions-rx
+ collection-permissions-rx
+ non-scoped-permissions-rx
+ block-permissions-rx
+ admin-permissions-rx]
+ "$"))
+
+(def ^:private rx->kind
+ [[(u.regex/rx "^/" v1-data-permissions-rx "$") :data]
+ [(u.regex/rx "^/" v2-data-permissions-rx "$") :data-v2]
+ [(u.regex/rx "^/" v2-query-permissions-rx "$") :query-v2]
+ [(u.regex/rx "^/" download-permissions-rx "$") :download]
+ [(u.regex/rx "^/" data-model-permissions-rx "$") :data-model]
+ [(u.regex/rx "^/" db-conn-details-permissions-rx "$") :db-conn-details]
+ [(u.regex/rx "^/" execute-permissions-rx "$") :execute]
+ [(u.regex/rx "^/" collection-permissions-rx "$") :collection]
+ [(u.regex/rx "^/" non-scoped-permissions-rx "$") :non-scoped]
+ [(u.regex/rx "^/" block-permissions-rx "$") :block]
+ [(u.regex/rx "^/" admin-permissions-rx "$") :admin]])
+
+(def ^:private path-regex-v2
+ "Regex for a valid permissions path. will not match a path like \"/db/1\" or \"/db/1/\".
+ [[metabase.util.regex/rx]] is used to make the big-and-hairy regex somewhat readable."
+ (u.regex/rx
+ "^/" [:or
+ v2-data-permissions-rx
+ v2-query-permissions-rx
+ download-permissions-rx
+ data-model-permissions-rx
+ db-conn-details-permissions-rx
+ execute-permissions-rx
+ collection-permissions-rx
+ non-scoped-permissions-rx
+ block-permissions-rx
+ admin-permissions-rx]
+ "$"))
+
+(def ^:private Path "A permission path."
+ [:or [:re path-regex-v1] [:re path-regex-v2]])
+
+(def ^:private Kind
+ (into [:enum] (map second rx->kind)))
+
+(mu/defn classify-path :- Kind [path :- Path]
+ (let [result (keep (fn [[permission-rx kind]]
+ (when (re-matches (u.regex/rx permission-rx) path) kind))
+ rx->kind)]
+ (when-not (= 1 (count result))
+ (throw (ex-info (str "Unclassifiable path! " (pr-str {:path path :result result}))
+ {:path path :result result})))
+ (first result)))
+
+(def DataPath "A permissions path that's guaranteed to be a v1 data-permissions path"
+ [:re (u.regex/rx "^/" v1-data-permissions-rx "$")])
+
+(mu/defn classify-data-path :- DataKind [data-path :- DataPath]
+ (let [result (keep (fn [[data-rx kind]]
+ (when (re-matches (u.regex/rx [:and "^/" data-rx]) data-path) kind))
+ data-rx->data-kind)]
+ (when-not (= 1 (count result))
+ (throw (ex-info "Unclassified data path!!" {:data-path data-path :result result})))
+ (first result)))
(def segmented-perm-regex
"Regex that matches a segmented permission. Used internally for some EE stuff
@@ -324,24 +395,22 @@
(str/replace #"\\" "\\\\\\\\") ; \ -> \\
(str/replace #"/" "\\\\/"))) ; / -> \/
-(defn valid-path?
- "Is `path` a valid, known permissions path?"
- ^Boolean [^String path]
- (boolean (when (and (string? path)
- (seq path))
- (re-matches path-regex path))))
+(let [path-validator (mc/validator Path)]
+ (defn valid-path?
+ "Is `path` a valid, known permissions path?"
+ ^Boolean [^String path]
+ (path-validator path)))
-(defn valid-path-format?
- "Is `path` a string with a valid permissions path format? This is a less strict version of [[valid-path?]] which
+(let [path-format-validator (mc/validator [:re (re-pattern (str "^/(" path-char "*/)*$"))])]
+ (defn valid-path-format?
+ "Is `path` a string with a valid permissions path format? This is a less strict version of [[valid-path?]] which
just checks that the path components contain alphanumeric characters or dashes, separated by slashes
This should be used for schema validation in most places, to preserve downgradability when new permissions paths are
added."
- ^Boolean [^String path]
- (boolean (when (and (string? path)
- (seq path))
- (re-matches (re-pattern (str "^/(" path-char "*/)*$")) path))))
+ ^Boolean [^String path]
+ (path-format-validator path)))
-(def Path
+(def PathSchema
"Schema for a permissions path with a valid format."
(s/pred valid-path-format? "Valid permissions path"))
@@ -377,7 +446,7 @@
(def ^:private MapOrID
(s/cond-pre su/Map su/IntGreaterThanZero))
-(s/defn data-perms-path :- Path
+(s/defn data-perms-path :- PathSchema
"Return the [readwrite] permissions path for a Database, schema, or Table. (At the time of this writing, DBs and
schemas don't have separate `read/` and write permissions; you either have 'data access' permissions for them, or
you don't. Tables, however, have separate read and write perms.)"
@@ -390,18 +459,18 @@
([database-or-id :- MapOrID schema-name :- (s/maybe s/Str) table-or-id :- MapOrID]
(str (data-perms-path database-or-id schema-name) "table/" (u/the-id table-or-id) "/")))
-(s/defn adhoc-native-query-path :- Path
+(s/defn adhoc-native-query-path :- PathSchema
"Return the native query read/write permissions path for a database.
This grants you permissions to run arbitary native queries."
[database-or-id :- MapOrID]
(str (data-perms-path database-or-id) "native/"))
-(s/defn all-schemas-path :- Path
+(s/defn all-schemas-path :- PathSchema
"Return the permissions path for a database that grants full access to all schemas."
[database-or-id :- MapOrID]
(str (data-perms-path database-or-id) "schema/"))
-(s/defn collection-readwrite-path :- Path
+(s/defn collection-readwrite-path :- PathSchema
"Return the permissions path for *readwrite* access for a `collection-or-id`."
[collection-or-id :- MapOrID]
(if-not (get collection-or-id :metabase.models.collection.root/is-root?)
@@ -410,12 +479,12 @@
(format "/collection/namespace/%s/root/" (escape-path-component (u/qualified-name collection-namespace)))
"/collection/root/")))
-(s/defn collection-read-path :- Path
+(s/defn collection-read-path :- PathSchema
"Return the permissions path for *read* access for a `collection-or-id`."
[collection-or-id :- MapOrID]
(str (collection-readwrite-path collection-or-id) "read/"))
-(s/defn table-read-path :- Path
+(s/defn table-read-path :- PathSchema
"Return the permissions path required to fetch the Metadata for a Table."
([table-or-id]
(if (integer? table-or-id)
@@ -426,7 +495,7 @@
{:post [(valid-path? %)]}
(str (data-perms-path (u/the-id database-or-id) schema-name (u/the-id table-or-id)) "read/")))
-(s/defn table-query-path :- Path
+(s/defn table-query-path :- PathSchema
"Return the permissions path for *full* query access for a Table. Full query access means you can run any (MBQL) query
you wish against a given Table, with no GTAP-specified mandatory query alterations."
([table-or-id]
@@ -438,7 +507,7 @@
(str (data-perms-path (u/the-id database-or-id) schema-name (u/the-id table-or-id)) "query/")))
;; TODO -- consider renaming this to `table-sandboxed-query-path` since that terminology is used more frequently
-(s/defn table-segmented-query-path :- Path
+(s/defn table-segmented-query-path :- PathSchema
"Return the permissions path for *segmented* query access for a Table. Segmented access means running queries against
the Table will automatically replace the Table with a GTAP-specified question as the new source of the query,
obstensibly limiting access to the results."
@@ -450,20 +519,20 @@
([database-or-id schema-name table-or-id]
(str (data-perms-path (u/the-id database-or-id) schema-name (u/the-id table-or-id)) "query/segmented/")))
-(s/defn execute-query-perms-path :- Path
+(s/defn execute-query-perms-path :- PathSchema
"Return the execute query action permissions path for a database.
This grants you permissions to run arbitary query actions."
[database-or-id :- MapOrID]
(str "/execute" (data-perms-path database-or-id)))
-(s/defn database-block-perms-path :- Path
+(s/defn database-block-perms-path :- PathSchema
"Return the permissions path for the Block 'anti-permissions'. Block anti-permissions means a User cannot run a query
against a Database unless they have data permissions, regardless of whether segmented permissions would normally give
them access or not."
[database-or-id :- MapOrID]
(str "/block" (data-perms-path database-or-id)))
-(s/defn base->feature-perms-path :- Path
+(s/defn base->feature-perms-path :- PathSchema
"Returns the permissions path to use for a given permission type (e.g. download) and value (e.g. full or limited),
given the 'base' permissions path for an entity (the base path is equivalent to the one used for data access
permissions)."
@@ -484,19 +553,19 @@
[:execute :all]
(str "/execute" base-path)))
-(s/defn feature-perms-path :- Path
+(s/defn feature-perms-path :- PathSchema
"Returns the permissions path to use for a given feature-level permission type (e.g. download) and value (e.g. full
or limited), for a database, schema or table."
[perm-type perm-value & path-components]
(base->feature-perms-path perm-type perm-value (apply data-perms-path path-components)))
-(s/defn native-feature-perms-path :- Path
+(s/defn native-feature-perms-path :- PathSchema
"Returns the native permissions path to use for a given feature-level permission type (e.g. download) and value
(e.g. full or limited)."
[perm-type perm-value database-or-id]
(base->feature-perms-path perm-type perm-value (adhoc-native-query-path database-or-id)))
-(s/defn data-model-write-perms-path :- Path
+(s/defn data-model-write-perms-path :- PathSchema
"Returns the permission path required to edit the table specified by the provided args, or a field in the table.
If Enterprise Edition code is available, and a valid :advanced-permissions token is present, returns the data model
permissions path for the table. Otherwise, defaults to the root path ('/'), thus restricting writes to admins."
@@ -508,7 +577,7 @@
(apply f path-components)
"/")))
-(s/defn db-details-write-perms-path :- Path
+(s/defn db-details-write-perms-path :- PathSchema
"Returns the permission path required to edit the table specified by the provided args, or a field in the table.
If Enterprise Edition code is available, and a valid :advanced-permissions token is present, returns the DB details
permissions path for the table. Otherwise, defaults to the root path ('/'), thus restricting writes to admins."
@@ -520,7 +589,7 @@
(f db-id)
"/")))
-(s/defn application-perms-path :- Path
+(s/defn application-perms-path :- PathSchema
"Returns the permissions path for *full* access a application permission."
[perm-type]
(case perm-type
@@ -574,7 +643,7 @@
[permissions-set perm-type]
(set-has-full-permissions? permissions-set (application-perms-path perm-type)))
-(s/defn perms-objects-set-for-parent-collection :- #{Path}
+(s/defn perms-objects-set-for-parent-collection :- #{PathSchema}
"Implementation of `perms-objects-set` for models with a `collection_id`, such as Card, Dashboard, or Pulse.
This simply returns the `perms-objects-set` of the parent Collection (based on `collection_id`) or for the Root
Collection if `collection_id` is `nil`."
@@ -741,15 +810,13 @@
(defn- all-permissions
"Handle '/' permission"
[db-ids]
- (reduce (fn [g db-id]
- (assoc g db-id {:data {:native :write
- :schemas :all}
- :download {:native :full
- :schemas :full}
- :data-model {:schemas :all}
- :details :yes}))
- {}
- db-ids))
+ (into {}
+ (map (fn [db-id]
+ [db-id {:data {:native :write :schemas :all}
+ :download {:native :full :schemas :full}
+ :data-model { :schemas :all}
+ :details :yes}])
+ db-ids)))
(defn- permissions-by-group-ids [where-clause]
(let [permissions (db/select [Permissions [:group_id :group-id] [:object :path]]
@@ -759,7 +826,7 @@
{}
permissions)))
-(s/defn data-perms-graph
+(defn data-perms-graph
"Fetch a graph representing the current *data* permissions status for every Group and all permissioned databases.
See [[metabase.models.collection.graph]] for the Collection permissions graph code."
[]
@@ -769,7 +836,10 @@
db-ids (delay (db/select-ids 'Database))
group-id->graph (m/map-vals
(fn [paths]
- (let [permissions-graph (perms-parse/permissions->graph paths)]
+ ;; Currently we do not use v2 permissions paths, and permissions->graph doesn't handle them.
+ ;; so we ignore those until that work is complete.
+ (let [v1-paths (filter #(mc/validate path-regex-v1 %) paths)
+ permissions-graph (perms-parse/permissions->graph v1-paths)]
(if (= permissions-graph :all)
(all-permissions @db-ids)
(:db permissions-graph))))
@@ -777,7 +847,7 @@
{:revision (perms-revision/latest-id)
:groups group-id->graph}))
-(s/defn execution-perms-graph
+(defn execution-perms-graph
"Fetch a graph representing the current *execution* permissions status for
every Group and all permissioned databases."
[]
@@ -819,7 +889,7 @@
NOTE: This function is meant for internal usage in this namespace only; use one of the other functions like
`revoke-data-perms!` elsewhere instead of calling this directly."
{:style/indent 2}
- [group-or-id :- (s/cond-pre su/Map su/IntGreaterThanZero) path :- Path & other-conditions]
+ [group-or-id :- (s/cond-pre su/Map su/IntGreaterThanZero) path :- PathSchema & other-conditions]
(let [where {:where (apply list
:and
[:= :group_id (u/the-id group-or-id)]
@@ -853,6 +923,37 @@
(delete-related-permissions! group-or-id (apply (partial feature-perms-path :download :full) path-components))
(delete-related-permissions! group-or-id (apply (partial feature-perms-path :download :limited) path-components)))
+
+(letfn [(delete [s to-delete] (str/replace s to-delete ""))
+ (data-query-split [path] [(str "/data" path) (str "/query" path)])]
+ (def ^:private data-kind->rewrite-fn
+ "lookup table to generate v2 query + data permission from a v1 data permission."
+ {:dk/db data-query-split
+ :dk/db-native (fn [path] (data-query-split (delete path "native/")))
+ :dk/db-schema (fn [path] [(str "/data" (delete path "schema/")) (str "/query" path)])
+ :dk/db-schema-name data-query-split
+ :dk/db-schema-name-and-table data-query-split
+ :dk/db-schema-name-table-and-read (constantly [])
+ :dk/db-schema-name-table-and-query (fn [path] (data-query-split (delete path "query/")))
+ :dk/db-schema-name-table-and-segmented (fn [path] (data-query-split (delete path "query/segmented/")))}))
+
+(mu/defn ^:private ->v2-path :- [:vector [:re path-regex-v2]]
+ [path :- [:or [:re path-regex-v1] [:re path-regex-v2]]]
+ ;; See: https://www.notion.so/metabase/Permissions-Refactor-Design-Doc-18ff5e6be32f4a52b9422bd7f4237ca7#5603afe084a7435ca7dc928fc94d4bda
+ (let [kind (classify-path path)]
+ (case kind
+ :data (let [data-permission-kind (classify-data-path path)
+ rewrite-fn (data-kind->rewrite-fn data-permission-kind)]
+ (rewrite-fn path))
+ :admin ["/"]
+ :block []
+
+ ;; for sake of idempotency, v2 perm-paths should be left untouched.
+ (:data-v2 :query-v2) [path]
+
+ ;; other paths should be left untouched too
+ [path])))
+
(defn grant-permissions!
"Grant permissions for `group-or-id`. Two-arity grants any arbitrary Permissions `path`. With > 2 args, grants the
data permissions from calling [[data-perms-path]]."
@@ -860,10 +961,17 @@
(grant-permissions! group-or-id (apply data-perms-path db-id schema more)))
([group-or-id path]
+ ;; TEMPORARY HACK: v2 paths won't be in the graph, so they will not be seen in the old graph, so will be
+ ;; interpreted as being new, and hence will not get deleted.
+ ;; But we can simply delete them here:
+ ;; This must be pulled out once we are properly parsing v2 query and data permissions
+ (db/delete! Permissions :group_id (u/the-id group-or-id) :object [:like "/query/%"])
+ (db/delete! Permissions :group_id (u/the-id group-or-id) :object [:like "/data/%"])
(try
- (db/insert! Permissions
- :group_id (u/the-id group-or-id)
- :object path)
+ (db/insert-many! Permissions
+ (map (fn [path-object]
+ {:group_id (u/the-id group-or-id) :object path-object})
+ (distinct (conj (->v2-path path) path))))
;; on some occasions through weirdness we might accidentally try to insert a key that's already been inserted
(catch Throwable e
(log/error e (u/format-color 'red (tru "Failed to grant permissions")))
@@ -1146,11 +1254,12 @@
(update-fn group-id db-id new-perms)
(throw (ee-permissions-exception perm-type))))
-(mu/defn ^:private update-group-permissions! :- nil?
+(mu/defn ^:private update-group-permissions!
[group-id :- pos-int? new-group-perms :- api.permission-graph/strict-db-graph]
(doseq [[db-id new-db-perms] new-group-perms
[perm-type new-perms] new-db-perms]
(case perm-type
+
:data
(update-db-data-access-permissions! group-id db-id new-perms)
diff --git a/src/metabase/models/permissions/parse.clj b/src/metabase/models/permissions/parse.clj
index eb159fd2147..b6bdd4c97e9 100644
--- a/src/metabase/models/permissions/parse.clj
+++ b/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]
diff --git a/src/metabase/models/query/permissions.clj b/src/metabase/models/query/permissions.clj
index 2a527b5b136..8a2759e4e24 100644
--- a/src/metabase/models/query/permissions.clj
+++ b/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]
diff --git a/src/metabase/util/regex.clj b/src/metabase/util/regex.clj
index b32958b7d1a..5d4ceef6ecc 100644
--- a/src/metabase/util/regex.clj
+++ b/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))))))
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 6d9e2b46fcc..38252d9a10c 100644
--- a/test/metabase/api/card_test.clj
+++ b/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)}))))
diff --git a/test/metabase/models/permissions_group_test.clj b/test/metabase/models/permissions_group_test.clj
index 764aa1b6502..3fbbd31cf32 100644
--- a/test/metabase/models/permissions_group_test.clj
+++ b/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
diff --git a/test/metabase/models/permissions_test.clj b/test/metabase/models/permissions_test.clj
index 19db6adca28..b3d7fbd26bf 100644
--- a/test/metabase/models/permissions_test.clj
+++ b/test/metabase/models/permissions_test.clj
@@ -1,6 +1,7 @@
(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))))
diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
index e39f16c778d..1fee7836b03 100644
--- a/test/metabase/query_processor_test/date_bucketing_test.clj
+++ b/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))))))))))))
diff --git a/test/metabase/util/regex_test.clj b/test/metabase/util/regex_test.clj
index e1be56c4bd1..2a0c228a958 100644
--- a/test/metabase/util/regex_test.clj
+++ b/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"]])))))))
--
GitLab