Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cache key with guest token rls #19110

Merged
merged 5 commits into from
Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions superset/common/query_context_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from pandas import DateOffset
from typing_extensions import TypedDict

from superset import app, is_feature_enabled
from superset import app
from superset.annotation_layers.dao import AnnotationLayerDAO
from superset.charts.dao import ChartDAO
from superset.common.chart_data import ChartDataResultFormat
Expand Down Expand Up @@ -159,10 +159,7 @@ def query_cache_key(self, query_obj: QueryObject, **kwargs: Any) -> Optional[str
query_obj.cache_key(
datasource=datasource.uid,
extra_cache_keys=extra_cache_keys,
rls=security_manager.get_rls_ids(datasource)
if is_feature_enabled("ROW_LEVEL_SECURITY")
and datasource.is_rls_supported
else [],
rls=security_manager.get_rls_cache_key(datasource),
changed_on=datasource.changed_on,
**kwargs,
)
Expand Down
14 changes: 14 additions & 0 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,20 @@ def get_rls_ids(self, table: "BaseDatasource") -> List[int]:
ids.sort() # Combinations rather than permutations
return ids

def get_guest_rls_filters_str(self, table: "BaseDatasource") -> List[str]:
return [f.get("clause", "") for f in self.get_guest_rls_filters(table)]

def get_rls_cache_key(self, datasource: "BaseDatasource") -> List[str]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the get_rls_ids function anymore if we're using this instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's used for getting the ids on line 1234

# pylint: disable=import-outside-toplevel
from superset import is_feature_enabled

rls_ids = []
if is_feature_enabled("ROW_LEVEL_SECURITY") and datasource.is_rls_supported:
rls_ids = self.get_rls_ids(datasource)
rls_str = [str(rls_id) for rls_id in rls_ids]
guest_rls = self.get_guest_rls_filters_str(datasource)
return guest_rls + rls_str

@staticmethod
def raise_for_user_activity_access(user_id: int) -> None:
user = g.user if g.user and g.user.get_id() else None
Expand Down
9 changes: 2 additions & 7 deletions superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
from geopy.point import Point
from pandas.tseries.frequencies import to_offset

from superset import app, is_feature_enabled
from superset import app
from superset.common.db_query_status import QueryStatus
from superset.constants import NULL_STRING
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
Expand Down Expand Up @@ -458,12 +458,7 @@ def cache_key(self, query_obj: QueryObjectDict, **extra: Any) -> str:
cache_dict["time_range"] = self.form_data.get("time_range")
cache_dict["datasource"] = self.datasource.uid
cache_dict["extra_cache_keys"] = self.datasource.get_extra_cache_keys(query_obj)
cache_dict["rls"] = (
security_manager.get_rls_ids(self.datasource)
if is_feature_enabled("ROW_LEVEL_SECURITY")
and self.datasource.is_rls_supported
else []
)
cache_dict["rls"] = security_manager.get_rls_cache_key(self.datasource)
cache_dict["changed_on"] = self.datasource.changed_on
json_data = self.json_dumps(cache_dict, sort_keys=True)
return md5_sha_from_str(json_data)
Expand Down