villebro commented on a change in pull request #10946:
URL:
https://github.com/apache/incubator-superset/pull/10946#discussion_r492003039
##########
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:
Done
##########
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:
I added an index on `filter_type`, we can probably add more indices if
needed later.
##########
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 changed the complex regex to a full where clause assertion, for the
others I'm just checking that the exact clause is in the query.
##########
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 created new roles for this test: `NameAB` and `NameQ`.
##########
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:
I added a temporary user `NoRlsRoles` that is used to check for the base
filter (same case as `alpha` before`).
##########
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:
I added a temporary user `NoRlsRolesUser` that is used to check for the
base filter (same case as `alpha` before`).
##########
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:
I added a temporary user `NoRlsRoleUser` that is used to check for the
base filter (same case as `alpha` before`).
##########
File path: superset/connectors/sqla/views.py
##########
@@ -241,30 +242,73 @@ class SqlMetricInlineView( # pylint:
disable=too-many-ancestors
edit_form_extra_fields = add_form_extra_fields
+class RowLevelSecurityListWidget(
+ SupersetListWidget
+): # pylint: disable=too-few-public-methods
+ template = "superset/models/rls/list.html"
+
+ def __init__(self, **kwargs: Any):
+ kwargs["appbuilder"] = current_app.appbuilder
+ super().__init__(**kwargs)
+
+
class RowLevelSecurityFiltersModelView( # pylint: disable=too-many-ancestors
SupersetModelView, DeleteMixin
):
datamodel = SQLAInterface(models.RowLevelSecurityFilter)
+ list_widget = cast(SupersetListWidget, RowLevelSecurityListWidget)
Review comment:
For some reason mypy flags `Type[RowLevelSecurityListWidget]` as being
incompatible with `Type[SupersetListWidget]` which is required for
`list_widget`, despite extending from it. Either I'm missing something here or
mypy is just flagging a false positive here.
##########
File path: superset/connectors/sqla/views.py
##########
@@ -241,30 +242,73 @@ class SqlMetricInlineView( # pylint:
disable=too-many-ancestors
edit_form_extra_fields = add_form_extra_fields
+class RowLevelSecurityListWidget(
+ SupersetListWidget
+): # pylint: disable=too-few-public-methods
+ template = "superset/models/rls/list.html"
+
+ def __init__(self, **kwargs: Any):
+ kwargs["appbuilder"] = current_app.appbuilder
+ super().__init__(**kwargs)
+
+
class RowLevelSecurityFiltersModelView( # pylint: disable=too-many-ancestors
SupersetModelView, DeleteMixin
):
datamodel = SQLAInterface(models.RowLevelSecurityFilter)
+ list_widget = cast(SupersetListWidget, RowLevelSecurityListWidget)
Review comment:
For some reason mypy flags `Type[RowLevelSecurityListWidget]` as being
incompatible with `Type[SupersetListWidget]` which is required for
`list_widget`, despite `RowLevelSecurityListWidget` extending from
`SupersetListWidget`. Either I'm missing something fundamental here, or mypy is
just flagging a false positive.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
