You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/18 15:55:42 UTC
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10946: [WIP] feat(row-level-security): add filter grouping an
bkyryliuk commented on a change in pull request #10946:
URL: https://github.com/apache/incubator-superset/pull/10946#discussion_r491036704
##########
File path: superset/migrations/versions/e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py
##########
@@ -0,0 +1,48 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add rls filter type and grouping key
+
+Revision ID: e5ef6828ac4e
+Revises: ae19b4ee3692
+Create Date: 2020-09-15 18:22:40.130985
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "e5ef6828ac4e"
+down_revision = "ae19b4ee3692"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ with op.batch_alter_table("row_level_security_filters") as batch_op:
+ batch_op.add_column(sa.Column("filter_type", sa.VARCHAR(255), nullable=True)),
Review comment:
it would be nice to have index on the filter_type or composite
role_id & filter_type
##########
File path: superset/security/manager.py
##########
@@ -1012,8 +1012,17 @@ def get_rls_filters( # pylint: disable=no-self-use
.filter(assoc_user_role.c.user_id == g.user.id)
.subquery()
)
- filter_roles = (
+ regular_filter_roles = (
self.get_session.query(RLSFilterRoles.c.rls_filter_id)
+ .join(RowLevelSecurityFilter)
+ .filter(RowLevelSecurityFilter.filter_type == "Regular")
Review comment:
maybe make filter_type as an enum
##########
File path: tests/security_tests.py
##########
@@ -1009,88 +1010,143 @@ class TestRowLevelSecurity(SupersetTestCase):
"""
rls_entry = None
+ query_obj = dict(
+ groupby=[],
+ metrics=[],
+ filter=[],
+ is_timeseries=False,
+ columns=["value"],
+ granularity=None,
+ from_dttm=None,
+ to_dttm=None,
+ extras={},
+ )
+ GAMMA_FILTER_REGEX = re.compile(r"'[A,B,Q]%'")
+ BASE_FILTER_REGEX = re.compile(r"'boy'")
def setUp(self):
session = db.session
- # Create the RowLevelSecurityFilter
- self.rls_entry = RowLevelSecurityFilter()
- self.rls_entry.tables.extend(
+ # Create regular RowLevelSecurityFilter (energy_usage, unicode_test)
+ self.rls_entry1 = RowLevelSecurityFilter()
+ self.rls_entry1.tables.extend(
session.query(SqlaTable)
.filter(SqlaTable.table_name.in_(["energy_usage", "unicode_test"]))
.all()
)
- self.rls_entry.clause = "value > {{ cache_key_wrapper(1) }}"
- self.rls_entry.roles.append(
- security_manager.find_role("Gamma")
- ) # db.session.query(Role).filter_by(name="Gamma").first())
- self.rls_entry.roles.append(security_manager.find_role("Alpha"))
- db.session.add(self.rls_entry)
+ self.rls_entry1.filter_type = "Regular"
+ self.rls_entry1.clause = "value > {{ cache_key_wrapper(1) }}"
+ self.rls_entry1.group_key = None
+ self.rls_entry1.roles.append(security_manager.find_role("Gamma"))
+ self.rls_entry1.roles.append(security_manager.find_role("Alpha"))
+ db.session.add(self.rls_entry1)
+
+ # Create regular RowLevelSecurityFilter (birth_names name starts with A or B)
+ self.rls_entry2 = RowLevelSecurityFilter()
+ self.rls_entry2.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry2.filter_type = "Regular"
+ self.rls_entry2.clause = "name like 'A%' or name like 'B%'"
+ self.rls_entry2.group_key = "name"
+ self.rls_entry2.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry2)
+
+ # Create Regular RowLevelSecurityFilter (birth_names name starts with Q)
+ self.rls_entry3 = RowLevelSecurityFilter()
+ self.rls_entry3.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry3.filter_type = "Regular"
+ self.rls_entry3.clause = "name like 'Q%'"
+ self.rls_entry3.group_key = "name"
+ self.rls_entry3.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry3)
+
+ # Create Base RowLevelSecurityFilter (birth_names boys)
+ self.rls_entry4 = RowLevelSecurityFilter()
+ self.rls_entry4.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry4.filter_type = "Base"
+ self.rls_entry4.clause = "gender = 'boy'"
+ self.rls_entry4.group_key = "gender"
+ self.rls_entry4.roles.append(security_manager.find_role("Admin"))
+ db.session.add(self.rls_entry4)
db.session.commit()
def tearDown(self):
session = db.session
- session.delete(self.rls_entry)
+ session.delete(self.rls_entry1)
+ session.delete(self.rls_entry2)
+ session.delete(self.rls_entry3)
+ session.delete(self.rls_entry4)
session.commit()
- # Do another test to make sure it doesn't alter another query
- def test_rls_filter_alters_query(self):
- g.user = self.get_user(
- username="alpha"
- ) # self.login() doesn't actually set the user
+ def test_rls_filter_alters_energy_query(self):
+ g.user = self.get_user(username="alpha")
tbl = self.get_table_by_name("energy_usage")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == [1]
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == [1]
assert "value > 1" in sql
- def test_rls_filter_doesnt_alter_query(self):
+ def test_rls_filter_doesnt_alter_energy_query(self):
g.user = self.get_user(
username="admin"
) # self.login() doesn't actually set the user
tbl = self.get_table_by_name("energy_usage")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == []
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
def test_multiple_table_filter_alters_another_tables_query(self):
g.user = self.get_user(
username="alpha"
) # self.login() doesn't actually set the user
tbl = self.get_table_by_name("unicode_test")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == [1]
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == [1]
assert "value > 1" in sql
+
+ def test_rls_filter_alters_gamma_birth_names_query(self):
+ g.user = self.get_user(username="gamma")
+ tbl = self.get_table_by_name("birth_names")
+ sql = tbl.get_query_str(self.query_obj)
+
+ # establish that both regular and base filters are present
+ assert self.GAMMA_FILTER_REGEX.search(sql)
+ assert self.BASE_FILTER_REGEX.search(sql)
+
+ # establish that they are grouped together correctly with ANDs, ORs
+ # and parens in the correct place (only look for unique bits in the
+ # filters to make the regex simpler)
+ assert re.search(
+ r"\(\s*\(.*'A%'.*\).*OR.*'Q%'\s*\)\s*\)\s+AND\s+\(.*'boy'\s*\)",
+ sql.replace("\n", " "), # remove newlines to make simpler regex
+ )
+
+ def test_rls_filter_alters_alpha_birth_names_query(self):
+ g.user = self.get_user(username="alpha")
+ tbl = self.get_table_by_name("birth_names")
+ sql = tbl.get_query_str(self.query_obj)
+
+ # gamma's filters should not be present query
+ assert not self.GAMMA_FILTER_REGEX.search(sql)
+ # base query should be present
+ assert self.BASE_FILTER_REGEX.search(sql)
+
+ def test_rls_filter_doesnt_alter_admin_birth_names_query(self):
Review comment:
1 more test for the role that has no rls attached to see what will be the behavior would be useful
##########
File path: tests/security_tests.py
##########
@@ -1009,88 +1010,143 @@ class TestRowLevelSecurity(SupersetTestCase):
"""
rls_entry = None
+ query_obj = dict(
+ groupby=[],
+ metrics=[],
+ filter=[],
+ is_timeseries=False,
+ columns=["value"],
+ granularity=None,
+ from_dttm=None,
+ to_dttm=None,
+ extras={},
+ )
+ GAMMA_FILTER_REGEX = re.compile(r"'[A,B,Q]%'")
+ BASE_FILTER_REGEX = re.compile(r"'boy'")
def setUp(self):
session = db.session
- # Create the RowLevelSecurityFilter
- self.rls_entry = RowLevelSecurityFilter()
- self.rls_entry.tables.extend(
+ # Create regular RowLevelSecurityFilter (energy_usage, unicode_test)
+ self.rls_entry1 = RowLevelSecurityFilter()
+ self.rls_entry1.tables.extend(
session.query(SqlaTable)
.filter(SqlaTable.table_name.in_(["energy_usage", "unicode_test"]))
.all()
)
- self.rls_entry.clause = "value > {{ cache_key_wrapper(1) }}"
- self.rls_entry.roles.append(
- security_manager.find_role("Gamma")
- ) # db.session.query(Role).filter_by(name="Gamma").first())
- self.rls_entry.roles.append(security_manager.find_role("Alpha"))
- db.session.add(self.rls_entry)
+ self.rls_entry1.filter_type = "Regular"
+ self.rls_entry1.clause = "value > {{ cache_key_wrapper(1) }}"
+ self.rls_entry1.group_key = None
+ self.rls_entry1.roles.append(security_manager.find_role("Gamma"))
+ self.rls_entry1.roles.append(security_manager.find_role("Alpha"))
+ db.session.add(self.rls_entry1)
+
+ # Create regular RowLevelSecurityFilter (birth_names name starts with A or B)
+ self.rls_entry2 = RowLevelSecurityFilter()
+ self.rls_entry2.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry2.filter_type = "Regular"
+ self.rls_entry2.clause = "name like 'A%' or name like 'B%'"
+ self.rls_entry2.group_key = "name"
+ self.rls_entry2.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry2)
+
+ # Create Regular RowLevelSecurityFilter (birth_names name starts with Q)
+ self.rls_entry3 = RowLevelSecurityFilter()
+ self.rls_entry3.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry3.filter_type = "Regular"
+ self.rls_entry3.clause = "name like 'Q%'"
+ self.rls_entry3.group_key = "name"
+ self.rls_entry3.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry3)
+
+ # Create Base RowLevelSecurityFilter (birth_names boys)
+ self.rls_entry4 = RowLevelSecurityFilter()
+ self.rls_entry4.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry4.filter_type = "Base"
+ self.rls_entry4.clause = "gender = 'boy'"
+ self.rls_entry4.group_key = "gender"
+ self.rls_entry4.roles.append(security_manager.find_role("Admin"))
+ db.session.add(self.rls_entry4)
db.session.commit()
def tearDown(self):
session = db.session
- session.delete(self.rls_entry)
+ session.delete(self.rls_entry1)
+ session.delete(self.rls_entry2)
+ session.delete(self.rls_entry3)
+ session.delete(self.rls_entry4)
session.commit()
- # Do another test to make sure it doesn't alter another query
- def test_rls_filter_alters_query(self):
- g.user = self.get_user(
- username="alpha"
- ) # self.login() doesn't actually set the user
+ def test_rls_filter_alters_energy_query(self):
+ g.user = self.get_user(username="alpha")
tbl = self.get_table_by_name("energy_usage")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == [1]
+ sql = tbl.get_query_str(self.query_obj)
Review comment:
this is nice refactor!
##########
File path: tests/security_tests.py
##########
@@ -1009,88 +1010,143 @@ class TestRowLevelSecurity(SupersetTestCase):
"""
rls_entry = None
+ query_obj = dict(
+ groupby=[],
+ metrics=[],
+ filter=[],
+ is_timeseries=False,
+ columns=["value"],
+ granularity=None,
+ from_dttm=None,
+ to_dttm=None,
+ extras={},
+ )
+ GAMMA_FILTER_REGEX = re.compile(r"'[A,B,Q]%'")
+ BASE_FILTER_REGEX = re.compile(r"'boy'")
def setUp(self):
session = db.session
- # Create the RowLevelSecurityFilter
- self.rls_entry = RowLevelSecurityFilter()
- self.rls_entry.tables.extend(
+ # Create regular RowLevelSecurityFilter (energy_usage, unicode_test)
+ self.rls_entry1 = RowLevelSecurityFilter()
+ self.rls_entry1.tables.extend(
session.query(SqlaTable)
.filter(SqlaTable.table_name.in_(["energy_usage", "unicode_test"]))
.all()
)
- self.rls_entry.clause = "value > {{ cache_key_wrapper(1) }}"
- self.rls_entry.roles.append(
- security_manager.find_role("Gamma")
- ) # db.session.query(Role).filter_by(name="Gamma").first())
- self.rls_entry.roles.append(security_manager.find_role("Alpha"))
- db.session.add(self.rls_entry)
+ self.rls_entry1.filter_type = "Regular"
+ self.rls_entry1.clause = "value > {{ cache_key_wrapper(1) }}"
+ self.rls_entry1.group_key = None
+ self.rls_entry1.roles.append(security_manager.find_role("Gamma"))
+ self.rls_entry1.roles.append(security_manager.find_role("Alpha"))
+ db.session.add(self.rls_entry1)
+
+ # Create regular RowLevelSecurityFilter (birth_names name starts with A or B)
+ self.rls_entry2 = RowLevelSecurityFilter()
+ self.rls_entry2.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry2.filter_type = "Regular"
+ self.rls_entry2.clause = "name like 'A%' or name like 'B%'"
+ self.rls_entry2.group_key = "name"
+ self.rls_entry2.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry2)
+
+ # Create Regular RowLevelSecurityFilter (birth_names name starts with Q)
+ self.rls_entry3 = RowLevelSecurityFilter()
+ self.rls_entry3.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry3.filter_type = "Regular"
+ self.rls_entry3.clause = "name like 'Q%'"
+ self.rls_entry3.group_key = "name"
+ self.rls_entry3.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry3)
+
+ # Create Base RowLevelSecurityFilter (birth_names boys)
+ self.rls_entry4 = RowLevelSecurityFilter()
+ self.rls_entry4.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry4.filter_type = "Base"
+ self.rls_entry4.clause = "gender = 'boy'"
+ self.rls_entry4.group_key = "gender"
+ self.rls_entry4.roles.append(security_manager.find_role("Admin"))
+ db.session.add(self.rls_entry4)
db.session.commit()
def tearDown(self):
session = db.session
- session.delete(self.rls_entry)
+ session.delete(self.rls_entry1)
+ session.delete(self.rls_entry2)
+ session.delete(self.rls_entry3)
+ session.delete(self.rls_entry4)
session.commit()
- # Do another test to make sure it doesn't alter another query
- def test_rls_filter_alters_query(self):
- g.user = self.get_user(
- username="alpha"
- ) # self.login() doesn't actually set the user
+ def test_rls_filter_alters_energy_query(self):
+ g.user = self.get_user(username="alpha")
tbl = self.get_table_by_name("energy_usage")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == [1]
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == [1]
assert "value > 1" in sql
- def test_rls_filter_doesnt_alter_query(self):
+ def test_rls_filter_doesnt_alter_energy_query(self):
g.user = self.get_user(
username="admin"
) # self.login() doesn't actually set the user
tbl = self.get_table_by_name("energy_usage")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == []
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
def test_multiple_table_filter_alters_another_tables_query(self):
g.user = self.get_user(
username="alpha"
) # self.login() doesn't actually set the user
tbl = self.get_table_by_name("unicode_test")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == [1]
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == [1]
assert "value > 1" in sql
+
+ def test_rls_filter_alters_gamma_birth_names_query(self):
+ g.user = self.get_user(username="gamma")
+ tbl = self.get_table_by_name("birth_names")
+ sql = tbl.get_query_str(self.query_obj)
+
+ # establish that both regular and base filters are present
+ assert self.GAMMA_FILTER_REGEX.search(sql)
+ assert self.BASE_FILTER_REGEX.search(sql)
+
+ # establish that they are grouped together correctly with ANDs, ORs
+ # and parens in the correct place (only look for unique bits in the
+ # filters to make the regex simpler)
+ assert re.search(
Review comment:
I have a slight preference for the full sql assertion if that's possible - makes it much more readable.
However with different DB engines that may be tricky :(
##########
File path: tests/security_tests.py
##########
@@ -1009,88 +1010,143 @@ class TestRowLevelSecurity(SupersetTestCase):
"""
rls_entry = None
+ query_obj = dict(
+ groupby=[],
+ metrics=[],
+ filter=[],
+ is_timeseries=False,
+ columns=["value"],
+ granularity=None,
+ from_dttm=None,
+ to_dttm=None,
+ extras={},
+ )
+ GAMMA_FILTER_REGEX = re.compile(r"'[A,B,Q]%'")
+ BASE_FILTER_REGEX = re.compile(r"'boy'")
def setUp(self):
session = db.session
- # Create the RowLevelSecurityFilter
- self.rls_entry = RowLevelSecurityFilter()
- self.rls_entry.tables.extend(
+ # Create regular RowLevelSecurityFilter (energy_usage, unicode_test)
+ self.rls_entry1 = RowLevelSecurityFilter()
+ self.rls_entry1.tables.extend(
session.query(SqlaTable)
.filter(SqlaTable.table_name.in_(["energy_usage", "unicode_test"]))
.all()
)
- self.rls_entry.clause = "value > {{ cache_key_wrapper(1) }}"
- self.rls_entry.roles.append(
- security_manager.find_role("Gamma")
- ) # db.session.query(Role).filter_by(name="Gamma").first())
- self.rls_entry.roles.append(security_manager.find_role("Alpha"))
- db.session.add(self.rls_entry)
+ self.rls_entry1.filter_type = "Regular"
+ self.rls_entry1.clause = "value > {{ cache_key_wrapper(1) }}"
+ self.rls_entry1.group_key = None
+ self.rls_entry1.roles.append(security_manager.find_role("Gamma"))
+ self.rls_entry1.roles.append(security_manager.find_role("Alpha"))
+ db.session.add(self.rls_entry1)
+
+ # Create regular RowLevelSecurityFilter (birth_names name starts with A or B)
+ self.rls_entry2 = RowLevelSecurityFilter()
+ self.rls_entry2.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry2.filter_type = "Regular"
+ self.rls_entry2.clause = "name like 'A%' or name like 'B%'"
+ self.rls_entry2.group_key = "name"
+ self.rls_entry2.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry2)
+
+ # Create Regular RowLevelSecurityFilter (birth_names name starts with Q)
+ self.rls_entry3 = RowLevelSecurityFilter()
+ self.rls_entry3.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry3.filter_type = "Regular"
+ self.rls_entry3.clause = "name like 'Q%'"
+ self.rls_entry3.group_key = "name"
+ self.rls_entry3.roles.append(security_manager.find_role("Gamma"))
+ db.session.add(self.rls_entry3)
+
+ # Create Base RowLevelSecurityFilter (birth_names boys)
+ self.rls_entry4 = RowLevelSecurityFilter()
+ self.rls_entry4.tables.extend(
+ session.query(SqlaTable)
+ .filter(SqlaTable.table_name.in_(["birth_names"]))
+ .all()
+ )
+ self.rls_entry4.filter_type = "Base"
+ self.rls_entry4.clause = "gender = 'boy'"
+ self.rls_entry4.group_key = "gender"
+ self.rls_entry4.roles.append(security_manager.find_role("Admin"))
+ db.session.add(self.rls_entry4)
db.session.commit()
def tearDown(self):
session = db.session
- session.delete(self.rls_entry)
+ session.delete(self.rls_entry1)
+ session.delete(self.rls_entry2)
+ session.delete(self.rls_entry3)
+ session.delete(self.rls_entry4)
session.commit()
- # Do another test to make sure it doesn't alter another query
- def test_rls_filter_alters_query(self):
- g.user = self.get_user(
- username="alpha"
- ) # self.login() doesn't actually set the user
+ def test_rls_filter_alters_energy_query(self):
+ g.user = self.get_user(username="alpha")
tbl = self.get_table_by_name("energy_usage")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == [1]
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == [1]
assert "value > 1" in sql
- def test_rls_filter_doesnt_alter_query(self):
+ def test_rls_filter_doesnt_alter_energy_query(self):
g.user = self.get_user(
username="admin"
) # self.login() doesn't actually set the user
tbl = self.get_table_by_name("energy_usage")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == []
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
def test_multiple_table_filter_alters_another_tables_query(self):
g.user = self.get_user(
username="alpha"
) # self.login() doesn't actually set the user
tbl = self.get_table_by_name("unicode_test")
- query_obj = dict(
- groupby=[],
- metrics=[],
- filter=[],
- is_timeseries=False,
- columns=["value"],
- granularity=None,
- from_dttm=None,
- to_dttm=None,
- extras={},
- )
- sql = tbl.get_query_str(query_obj)
- assert tbl.get_extra_cache_keys(query_obj) == [1]
+ sql = tbl.get_query_str(self.query_obj)
+ assert tbl.get_extra_cache_keys(self.query_obj) == [1]
assert "value > 1" in sql
+
+ def test_rls_filter_alters_gamma_birth_names_query(self):
+ g.user = self.get_user(username="gamma")
+ tbl = self.get_table_by_name("birth_names")
+ sql = tbl.get_query_str(self.query_obj)
+
+ # establish that both regular and base filters are present
+ assert self.GAMMA_FILTER_REGEX.search(sql)
+ assert self.BASE_FILTER_REGEX.search(sql)
+
+ # establish that they are grouped together correctly with ANDs, ORs
+ # and parens in the correct place (only look for unique bits in the
+ # filters to make the regex simpler)
+ assert re.search(
+ r"\(\s*\(.*'A%'.*\).*OR.*'Q%'\s*\)\s*\)\s+AND\s+\(.*'boy'\s*\)",
+ sql.replace("\n", " "), # remove newlines to make simpler regex
+ )
+
+ def test_rls_filter_alters_alpha_birth_names_query(self):
+ g.user = self.get_user(username="alpha")
Review comment:
I think alpha is a bad test case, as alpha is supposed to get full database access, maybe create a new role for this test ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org