From 7f8279b4b32f1f6aaf4d23a0e80321df87934e2e Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Tue, 3 May 2022 12:58:06 -0700 Subject: [PATCH] chore: get embedded user with roles and permissions (#19813) * feat: get user roles endpoint * add tests * fix test * get user with permission and roles with full user * frontend * type juggling * the hash slinging slasher * user reducer and action * make it happy * result * lint Co-authored-by: Lily Kuang --- .../src/dashboard/util/findPermission.test.ts | 2 +- .../src/dashboard/util/findPermission.ts | 3 +- superset-frontend/src/embedded/index.tsx | 43 ++++++++++++++++--- .../ExploreReport.tsx | 2 +- superset-frontend/src/preamble.ts | 4 +- superset-frontend/src/types/bootstrapTypes.ts | 24 ++++++++--- .../src/views/CRUD/welcome/Welcome.tsx | 6 +-- superset-frontend/src/views/store.ts | 23 +++++++++- superset/views/users/api.py | 32 ++++++++++++++ superset/views/utils.py | 8 ++++ tests/integration_tests/security_tests.py | 1 + tests/integration_tests/users/api_tests.py | 15 +++++++ 12 files changed, 143 insertions(+), 20 deletions(-) diff --git a/superset-frontend/src/dashboard/util/findPermission.test.ts b/superset-frontend/src/dashboard/util/findPermission.test.ts index 1c80770f5..0752bad40 100644 --- a/superset-frontend/src/dashboard/util/findPermission.test.ts +++ b/superset-frontend/src/dashboard/util/findPermission.test.ts @@ -58,7 +58,7 @@ const outsiderUser: UserWithPermissionsAndRoles = { const owner: Owner = { first_name: 'Test', - id: ownerUser.userId, + id: ownerUser.userId!, last_name: 'User', username: ownerUser.username, }; diff --git a/superset-frontend/src/dashboard/util/findPermission.ts b/superset-frontend/src/dashboard/util/findPermission.ts index 496f993bd..6edefbc99 100644 --- a/superset-frontend/src/dashboard/util/findPermission.ts +++ b/superset-frontend/src/dashboard/util/findPermission.ts @@ -18,14 +18,13 @@ */ import memoizeOne from 'memoize-one'; import { + UserRoles, isUserWithPermissionsAndRoles, UndefinedUser, UserWithPermissionsAndRoles, } from 'src/types/bootstrapTypes'; import Dashboard from 'src/types/Dashboard'; -type UserRoles = Record; - const findPermission = memoizeOne( (perm: string, view: string, roles?: UserRoles | null) => !!roles && diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx index afea2fd8b..52e0aee8d 100644 --- a/superset-frontend/src/embedded/index.tsx +++ b/superset-frontend/src/embedded/index.tsx @@ -19,16 +19,17 @@ import React, { lazy, Suspense } from 'react'; import ReactDOM from 'react-dom'; import { BrowserRouter as Router, Route } from 'react-router-dom'; -import { t } from '@superset-ui/core'; +import { makeApi, t } from '@superset-ui/core'; import { Switchboard } from '@superset-ui/switchboard'; import { bootstrapData } from 'src/preamble'; import setupClient from 'src/setup/setupClient'; import { RootContextProviders } from 'src/views/RootContextProviders'; -import { store } from 'src/views/store'; +import { store, USER_LOADED } from 'src/views/store'; import ErrorBoundary from 'src/components/ErrorBoundary'; import Loading from 'src/components/Loading'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import ToastContainer from 'src/components/MessageToasts/ToastContainer'; +import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; const debugMode = process.env.WEBPACK_MODE === 'development'; @@ -69,8 +70,13 @@ const appMountPoint = document.getElementById('app')!; const MESSAGE_TYPE = '__embedded_comms__'; if (!window.parent || window.parent === window) { - appMountPoint.innerHTML = - 'This page is intended to be embedded in an iframe, but it looks like that is not the case.'; + showFailureMessage( + 'This page is intended to be embedded in an iframe, but it looks like that is not the case.', + ); +} + +function showFailureMessage(message: string) { + appMountPoint.innerHTML = message; } // if the page is embedded in an origin that hasn't @@ -109,6 +115,33 @@ function guestUnauthorizedHandler() { ); } +function start() { + const getMeWithRole = makeApi({ + method: 'GET', + endpoint: '/api/v1/me/roles/', + }); + return getMeWithRole().then( + ({ result }) => { + // fill in some missing bootstrap data + // (because at pageload, we don't have any auth yet) + // this allows the frontend's permissions checks to work. + bootstrapData.user = result; + store.dispatch({ + type: USER_LOADED, + user: result, + }); + ReactDOM.render(, appMountPoint); + }, + err => { + // something is most likely wrong with the guest token + console.error(err); + showFailureMessage( + 'Something went wrong with embedded authentication. Check the dev console for details.', + ); + }, + ); +} + /** * Configures SupersetClient with the correct settings for the embedded dashboard page. */ @@ -153,7 +186,7 @@ window.addEventListener('message', function embeddedPageInitializer(event) { switchboard.defineMethod('guestToken', ({ guestToken }) => { setupGuestClient(guestToken); if (!started) { - ReactDOM.render(, appMountPoint); + start(); started = true; } }); diff --git a/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreReport.tsx b/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreReport.tsx index 967903d7b..4ec5ceb0a 100644 --- a/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreReport.tsx +++ b/superset-frontend/src/explore/components/ExploreAdditionalActionsMenu/ExploreReport.tsx @@ -53,7 +53,7 @@ export const ExploreReport = ({ }); const { userId, email } = useSelector< ExplorePageState, - { userId: number; email: string } + { userId?: number; email?: string } >(state => pick(state.explore.user, ['userId', 'email'])); const handleReportDelete = useCallback(() => { diff --git a/superset-frontend/src/preamble.ts b/superset-frontend/src/preamble.ts index 8d89104bf..ab6d696f5 100644 --- a/superset-frontend/src/preamble.ts +++ b/superset-frontend/src/preamble.ts @@ -26,7 +26,7 @@ import setupClient from './setup/setupClient'; import setupColors from './setup/setupColors'; import setupFormatters from './setup/setupFormatters'; import setupDashboardComponents from './setup/setupDasboardComponents'; -import { User } from './types/bootstrapTypes'; +import { BootstrapUser, User } from './types/bootstrapTypes'; if (process.env.WEBPACK_MODE === 'development') { setHotLoaderConfig({ logLevel: 'debug', trackTailUpdates: false }); @@ -34,7 +34,7 @@ if (process.env.WEBPACK_MODE === 'development') { // eslint-disable-next-line import/no-mutable-exports export let bootstrapData: { - user?: User | undefined; + user?: BootstrapUser; common?: any; config?: any; embedded?: { diff --git a/superset-frontend/src/types/bootstrapTypes.ts b/superset-frontend/src/types/bootstrapTypes.ts index 3feb32f7a..a6c1a3244 100644 --- a/superset-frontend/src/types/bootstrapTypes.ts +++ b/superset-frontend/src/types/bootstrapTypes.ts @@ -20,26 +20,40 @@ import { isPlainObject } from 'lodash'; * under the License. */ export type User = { - createdOn: string; - email: string; + createdOn?: string; + email?: string; firstName: string; isActive: boolean; isAnonymous: boolean; lastName: string; - userId: number; + userId?: number; // optional because guest user doesn't have a user id username: string; }; -export interface UserWithPermissionsAndRoles extends User { +export type UserRoles = Record; +export interface PermissionsAndRoles { permissions: { database_access?: string[]; datasource_access?: string[]; }; - roles: Record; + roles: UserRoles; } +export type UserWithPermissionsAndRoles = User & PermissionsAndRoles; + export type UndefinedUser = {}; +export type BootstrapUser = UserWithPermissionsAndRoles | undefined; + +export type Dashboard = { + dttm: number; + id: number; + url: string; + title: string; + creator?: string; + creator_url?: string; +}; + export type DashboardData = { dashboard_title?: string; created_on_delta_humanized?: string; diff --git a/superset-frontend/src/views/CRUD/welcome/Welcome.tsx b/superset-frontend/src/views/CRUD/welcome/Welcome.tsx index 2d564bc66..3660b8acc 100644 --- a/superset-frontend/src/views/CRUD/welcome/Welcome.tsx +++ b/superset-frontend/src/views/CRUD/welcome/Welcome.tsx @@ -151,7 +151,7 @@ export const LoadingCards = ({ cover }: LoadingProps) => ( function Welcome({ user, addDangerToast }: WelcomeProps) { const userid = user.userId; - const id = userid.toString(); + const id = userid!.toString(); // confident that user is not a guest user const recent = `/superset/recent_activity/${user.userId}/?limit=6`; const [activeChild, setActiveChild] = useState('Loading'); const userKey = dangerouslyGetItemDoNotUse(id, null); @@ -180,7 +180,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { useEffect(() => { const activeTab = getItem(LocalStorageKeys.homepage_activity_filter, null); setActiveState(collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR); - getRecentAcitivtyObjs(user.userId, recent, addDangerToast) + getRecentAcitivtyObjs(user.userId!, recent, addDangerToast) .then(res => { const data: ActivityData | null = {}; data.Examples = res.examples; @@ -295,7 +295,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { activityData.Created) && activeChild !== 'Loading' ? ( { + if (action.type === USER_LOADED) { + return action.user; + } + return user; +}; + // exported for tests export const rootReducer = combineReducers({ messageToasts: messageToastReducer, common: noopReducer(bootstrap.common || {}), - user: noopReducer(bootstrap.user || {}), + user: userReducer, impressionId: noopReducer(shortid.generate()), ...dashboardReducers, }); diff --git a/superset/views/users/api.py b/superset/views/users/api.py index 584e8145e..29d376935 100644 --- a/superset/views/users/api.py +++ b/superset/views/users/api.py @@ -18,6 +18,8 @@ from flask import g, Response from flask_appbuilder.api import BaseApi, expose, safe from flask_jwt_extended.exceptions import NoAuthorizationError +from superset.views.utils import bootstrap_user_data + from .schemas import UserResponseSchema user_response_schema = UserResponseSchema() @@ -59,3 +61,33 @@ class CurrentUserRestApi(BaseApi): return self.response_401() return self.response(200, result=user_response_schema.dump(g.user)) + + @expose("/roles/", methods=["GET"]) + @safe + def get_my_roles(self) -> Response: + """Get the user roles corresponding to the agent making the request + --- + get: + description: >- + Returns the user roles corresponding to the agent making the request, + or returns a 401 error if the user is unauthenticated. + responses: + 200: + description: The current user + content: + application/json: + schema: + type: object + properties: + result: + $ref: '#/components/schemas/UserResponseSchema' + 401: + $ref: '#/components/responses/401' + """ + try: + if g.user is None or g.user.is_anonymous: + return self.response_401() + except NoAuthorizationError: + return self.response_401() + user = bootstrap_user_data(g.user, include_perms=True) + return self.response(200, result=user) diff --git a/superset/views/utils.py b/superset/views/utils.py index 202f87d99..e0f97cba1 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -72,6 +72,14 @@ def bootstrap_user_data(user: User, include_perms: bool = False) -> Dict[str, An if user.is_anonymous: payload = {} user.roles = (security_manager.find_role("Public"),) + elif security_manager.is_guest_user(user): + payload = { + "username": user.username, + "firstName": user.first_name, + "lastName": user.last_name, + "isActive": user.is_active, + "isAnonymous": user.is_anonymous, + } else: payload = { "username": user.username, diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 82b4d8717..3add863de 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -889,6 +889,7 @@ class TestRolePermission(SupersetTestCase): ["AuthDBView", "login"], ["AuthDBView", "logout"], ["CurrentUserRestApi", "get_me"], + ["CurrentUserRestApi", "get_my_roles"], # TODO (embedded) remove Dashboard:embedded after uuids have been shipped ["Dashboard", "embedded"], ["EmbeddedView", "embedded"], diff --git a/tests/integration_tests/users/api_tests.py b/tests/integration_tests/users/api_tests.py index ee965f6f2..f4c897b6a 100644 --- a/tests/integration_tests/users/api_tests.py +++ b/tests/integration_tests/users/api_tests.py @@ -37,6 +37,21 @@ class TestCurrentUserApi(SupersetTestCase): self.assertEqual(True, response["result"]["is_active"]) self.assertEqual(False, response["result"]["is_anonymous"]) + def test_get_me_with_roles(self): + self.login(username="admin") + + rv = self.client.get(meUri + "roles/") + self.assertEqual(200, rv.status_code) + response = json.loads(rv.data.decode("utf-8")) + roles = list(response["result"]["roles"].keys()) + self.assertEqual("Admin", roles.pop()) + + @patch("superset.security.manager.g") + def test_get_my_roles_anonymous(self, mock_g): + mock_g.user = security_manager.get_anonymous_user + rv = self.client.get(meUri + "roles/") + self.assertEqual(401, rv.status_code) + def test_get_me_unauthorized(self): self.logout() rv = self.client.get(meUri)