iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat: customize recent activity access (#17589) 

* feat: customize recent activity access

* address comments

* fix and add tests

* add alert assertion and UPDATING.md entry

* replace .get_id() with .id

* fix updating comment

* update config name
This commit is contained in:
Ville Brofeldt 2021-12-08 11:30:23 +02:00 committed by GitHub
parent 12f1d914bd
commit c4b04952d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 142 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
UPDATING.md
View File

@ -43,6 +43,7 @@ assists people when migrating to a new version.
### Other
- [17589](https://github.com/apache/incubator-superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager.
- [16809](https://github.com/apache/incubator-superset/pull/16809): When building the superset frontend assets manually, you should now use Node 16 (previously Node 14 was required/recommended). Node 14 will most likely still work for at least some time, but is no longer actively tested for on CI.
- [17536](https://github.com/apache/superset/pull/17536): introduced a key-value endpoint to store dashboard filter state. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `FILTER_STATE_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/).

34
superset-frontend/src/components/TableLoader/TableLoader.test.tsx
View File

@ -24,7 +24,10 @@ import { storeWithState } from 'spec/fixtures/mockStore';
import ToastContainer from 'src/components/MessageToasts/ToastContainer';
import TableLoader, { TableLoaderProps } from '.';
fetchMock.get('glob:*/api/v1/mock', [
const NO_DATA_TEXT = 'No data available';
const MOCK_GLOB = 'glob:*/api/v1/mock';
fetchMock.get(MOCK_GLOB, [
{ id: 1, name: 'John Doe' },
{ id: 2, name: 'Jane Doe' },
]);
@ -32,6 +35,7 @@ fetchMock.get('glob:*/api/v1/mock', [
const defaultProps: TableLoaderProps = {
dataEndpoint: '/api/v1/mock',
addDangerToast: jest.fn(),
noDataText: NO_DATA_TEXT,
};
function renderWithProps(props: TableLoaderProps = defaultProps) {
@ -83,12 +87,36 @@ test('renders with mutator', async () => {
expect(await screen.findAllByRole('heading', { level: 4 })).toHaveLength(2);
});
test('renders error message', async () => {
fetchMock.mock('glob:*/api/v1/mock', 500, {
test('renders empty message', async () => {
fetchMock.mock(MOCK_GLOB, [], {
overwriteRoutes: true,
});
renderWithProps();
expect(await screen.findByText('No data available')).toBeInTheDocument();
});
test('renders blocked message', async () => {
fetchMock.mock(MOCK_GLOB, 403, {
overwriteRoutes: true,
});
renderWithProps();
expect(
await screen.findByText('Access to user activity data is restricted'),
).toBeInTheDocument();
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
});
test('renders error message', async () => {
fetchMock.mock(MOCK_GLOB, 500, {
overwriteRoutes: true,
});
renderWithProps();
expect(await screen.findByText(NO_DATA_TEXT)).toBeInTheDocument();
expect(await screen.findByRole('alert')).toBeInTheDocument();
});

17
superset-frontend/src/components/TableLoader/index.tsx
View File

@ -27,12 +27,14 @@ export interface TableLoaderProps {
dataEndpoint?: string;
mutator?: (data: JsonObject) => any[];
columns?: string[];
noDataText?: string;
addDangerToast(text: string): any;
}
const TableLoader = (props: TableLoaderProps) => {
const [data, setData] = useState<Array<any>>([]);
const [isLoading, setIsLoading] = useState(true);
const [isBlocked, setIsBlocked] = useState(false);
useEffect(() => {
const { dataEndpoint, mutator } = props;
@ -41,16 +43,22 @@ const TableLoader = (props: TableLoaderProps) => {
.then(({ json }) => {
const data = (mutator ? mutator(json) : json) as Array<any>;
setData(data);
setIsBlocked(false);
setIsLoading(false);
})
.catch(() => {
.catch(response => {
setIsLoading(false);
props.addDangerToast(t('An error occurred'));
if (response.status === 403) {
setIsBlocked(true);
} else {
setIsBlocked(false);
props.addDangerToast(t('An error occurred'));
}
});
}
}, [props]);
const { columns, ...tableProps } = props;
const { columns, noDataText, ...tableProps } = props;
const memoizedColumns = useMemo(() => {
let tableColumns = columns;
@ -79,6 +87,9 @@ const TableLoader = (props: TableLoaderProps) => {
pageSize={50}
loading={isLoading}
emptyWrapperType={EmptyWrapperType.Small}
noDataText={
isBlocked ? t('Access to user activity data is restricted') : noDataText
}
{...tableProps}
/>
);

3
superset/config.py
View File

@ -1293,6 +1293,9 @@ MENU_HIDE_USER_INFO = False
SQLALCHEMY_DOCS_URL = "https://docs.sqlalchemy.org/en/13/core/engines.html"
SQLALCHEMY_DISPLAY_TEXT = "SQLAlchemy docs"
# Set to False to only allow viewing own recent activity
ENABLE_BROAD_ACTIVITY_ACCESS = True
# -------------------------------------------------------------------
# * WARNING: STOP EDITING HERE *
# -------------------------------------------------------------------

1
superset/errors.py
View File

@ -63,6 +63,7 @@ class SupersetErrorType(str, Enum):
DATABASE_SECURITY_ACCESS_ERROR = "DATABASE_SECURITY_ACCESS_ERROR"
QUERY_SECURITY_ACCESS_ERROR = "QUERY_SECURITY_ACCESS_ERROR"
MISSING_OWNERSHIP_ERROR = "MISSING_OWNERSHIP_ERROR"
USER_ACTIVITY_SECURITY_ACCESS_ERROR = "USER_ACTIVITY_SECURITY_ACCESS_ERROR"
# Other errors
BACKEND_TIMEOUT_ERROR = "BACKEND_TIMEOUT_ERROR"

15
superset/security/manager.py
View File

@ -1160,6 +1160,21 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
ids.sort() # Combinations rather than permutations
return ids
@staticmethod
def raise_for_user_activity_access(user_id: int) -> None:
user = g.user if g.user and g.user.get_id() else None
if not user or (
not current_app.config["ENABLE_BROAD_ACTIVITY_ACCESS"]
and user_id != user.id
):
raise SupersetSecurityException(
SupersetError(
error_type=SupersetErrorType.USER_ACTIVITY_SECURITY_ACCESS_ERROR,
message="Access to user's activity data is restricted",
level=ErrorLevel.ERROR,
)
)
@staticmethod
def raise_for_dashboard_access(dashboard: "Dashboard") -> None:
"""

59
superset/views/core.py
View File

@ -1391,14 +1391,26 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
_("Unexpected error occurred, please check your logs for details"), 400
)
@staticmethod
def get_user_activity_access_error(user_id: int) -> Optional[FlaskResponse]:
try:
security_manager.raise_for_user_activity_access(user_id)
except SupersetSecurityException as ex:
return json_error_response(ex.message, status=403,)
return None
@api
@has_access_api
@event_logger.log_this
@expose("/recent_activity/<int:user_id>/", methods=["GET"])
def recent_activity( # pylint: disable=no-self-use
def recent_activity( # pylint: disable=too-many-locals
self, user_id: int
) -> FlaskResponse:
"""Recent activity (actions) for a given user"""
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
return error_obj
limit = request.args.get("limit")
limit = int(limit) if limit and limit.isdigit() else 100
actions = request.args.get("actions", "explore,dashboard").split(",")
@ -1526,15 +1538,16 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
def fave_dashboards_by_username(self, username: str) -> FlaskResponse:
"""This lets us use a user's username to pull favourite dashboards"""
user = security_manager.find_user(username=username)
return self.fave_dashboards(user.get_id())
return self.fave_dashboards(user.id)
@api
@has_access_api
@event_logger.log_this
@expose("/fave_dashboards/<int:user_id>/", methods=["GET"])
def fave_dashboards( # pylint: disable=no-self-use
self, user_id: int
) -> FlaskResponse:
def fave_dashboards(self, user_id: int) -> FlaskResponse:
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
return error_obj
qry = (
db.session.query(Dashboard, FavStar.dttm)
.join(
@ -1567,9 +1580,10 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
@has_access_api
@event_logger.log_this
@expose("/created_dashboards/<int:user_id>/", methods=["GET"])
def created_dashboards( # pylint: disable=no-self-use
self, user_id: int
) -> FlaskResponse:
def created_dashboards(self, user_id: int) -> FlaskResponse:
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
return error_obj
Dash = Dashboard
qry = (
db.session.query(Dash)
@ -1595,12 +1609,13 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
@event_logger.log_this
@expose("/user_slices", methods=["GET"])
@expose("/user_slices/<int:user_id>/", methods=["GET"])
def user_slices( # pylint: disable=no-self-use
self, user_id: Optional[int] = None
) -> FlaskResponse:
def user_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
"""List of slices a user owns, created, modified or faved"""
if not user_id:
user_id = g.user.get_id()
user_id = cast(int, g.user.id)
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
return error_obj
owner_ids_query = (
db.session.query(Slice.id)
@ -1647,12 +1662,13 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
@event_logger.log_this
@expose("/created_slices", methods=["GET"])
@expose("/created_slices/<int:user_id>/", methods=["GET"])
def created_slices( # pylint: disable=no-self-use
self, user_id: Optional[int] = None
) -> FlaskResponse:
def created_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
"""List of slices created by this user"""
if not user_id:
user_id = g.user.get_id()
user_id = cast(int, g.user.id)
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
return error_obj
qry = (
db.session.query(Slice)
.filter( # pylint: disable=comparison-with-callable
@ -1677,12 +1693,13 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
@event_logger.log_this
@expose("/fave_slices", methods=["GET"])
@expose("/fave_slices/<int:user_id>/", methods=["GET"])
def fave_slices( # pylint: disable=no-self-use
self, user_id: Optional[int] = None
) -> FlaskResponse:
def fave_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
"""Favorite slices for a user"""
if not user_id:
user_id = g.user.get_id()
if user_id is None:
user_id = g.user.id
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
return error_obj
qry = (
db.session.query(Slice, FavStar.dttm)
.join(

54
tests/integration_tests/core_tests.py
View File

@ -790,6 +790,19 @@ class TestCore(SupersetTestCase):
for k in keys:
self.assertIn(k, resp.keys())
@staticmethod
def _get_user_activity_endpoints(user: str):
userid = security_manager.find_user(user).id
return (
f"/superset/recent_activity/{userid}/",
f"/superset/created_slices/{userid}/",
f"/superset/created_dashboards/{userid}/",
f"/superset/fave_slices/{userid}/",
f"/superset/fave_dashboards/{userid}/",
f"/superset/user_slices/{userid}/",
f"/superset/fave_dashboards_by_username/{user}/",
)
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_user_profile(self, username="admin"):
self.login(username=username)
@ -805,23 +818,34 @@ class TestCore(SupersetTestCase):
resp = self.get_json_resp(url)
self.assertEqual(resp["count"], 1)
userid = security_manager.find_user("admin").id
resp = self.get_resp(f"/superset/profile/{username}/")
self.assertIn('"app"', resp)
data = self.get_json_resp(f"/superset/recent_activity/{userid}/")
self.assertNotIn("message", data)
data = self.get_json_resp(f"/superset/created_slices/{userid}/")
self.assertNotIn("message", data)
data = self.get_json_resp(f"/superset/created_dashboards/{userid}/")
self.assertNotIn("message", data)
data = self.get_json_resp(f"/superset/fave_slices/{userid}/")
self.assertNotIn("message", data)
data = self.get_json_resp(f"/superset/fave_dashboards/{userid}/")
self.assertNotIn("message", data)
data = self.get_json_resp(f"/superset/user_slices/{userid}/")
self.assertNotIn("message", data)
data = self.get_json_resp(f"/superset/fave_dashboards_by_username/{username}/")
self.assertNotIn("message", data)
for endpoint in self._get_user_activity_endpoints(username):
data = self.get_json_resp(endpoint)
self.assertNotIn("message", data)
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_user_activity_access(self, username="gamma"):
self.login(username=username)
# accessing own and other users' activity is allowed by default
for user in ("admin", "gamma"):
for endpoint in self._get_user_activity_endpoints(user):
resp = self.client.get(endpoint)
assert resp.status_code == 200
# disabling flag will block access to other users' activity data
access_flag = app.config["ENABLE_BROAD_ACTIVITY_ACCESS"]
app.config["ENABLE_BROAD_ACTIVITY_ACCESS"] = False
for user in ("admin", "gamma"):
for endpoint in self._get_user_activity_endpoints(user):
resp = self.client.get(endpoint)
expected_status_code = 200 if user == username else 403
assert resp.status_code == expected_status_code
# restore flag
app.config["ENABLE_BROAD_ACTIVITY_ACCESS"] = access_flag
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_slice_id_is_always_logged_correctly_on_web_request(self):