You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2022/04/13 13:04:11 UTC
[superset] branch master updated: fix: login button does not render (#19685)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 2ba484fe43 fix: login button does not render (#19685)
2ba484fe43 is described below
commit 2ba484fe43880ee09d6e61d778ad467ab7b0e459
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Wed Apr 13 16:04:04 2022 +0300
fix: login button does not render (#19685)
* fix: login button does not render
* add type guard
---
.../src/dashboard/util/findPermission.test.ts | 100 ++++++++++++---------
.../src/dashboard/util/findPermission.ts | 25 ++++--
superset-frontend/src/types/bootstrapTypes.ts | 13 +++
3 files changed, 90 insertions(+), 48 deletions(-)
diff --git a/superset-frontend/src/dashboard/util/findPermission.test.ts b/superset-frontend/src/dashboard/util/findPermission.test.ts
index 8930549f4a..1c80770f50 100644
--- a/superset-frontend/src/dashboard/util/findPermission.test.ts
+++ b/superset-frontend/src/dashboard/util/findPermission.test.ts
@@ -16,10 +16,54 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import {
+ UndefinedUser,
+ UserWithPermissionsAndRoles,
+} from 'src/types/bootstrapTypes';
import Dashboard from 'src/types/Dashboard';
import Owner from 'src/types/Owner';
-import findPermission, { canUserEditDashboard } from './findPermission';
+import findPermission, {
+ canUserEditDashboard,
+ isUserAdmin,
+} from './findPermission';
+
+const ownerUser: UserWithPermissionsAndRoles = {
+ createdOn: '2021-05-12T16:56:22.116839',
+ email: 'user@example.com',
+ firstName: 'Test',
+ isActive: true,
+ isAnonymous: false,
+ lastName: 'User',
+ userId: 1,
+ username: 'owner',
+ permissions: {},
+ roles: { Alpha: [['can_write', 'Dashboard']] },
+};
+
+const adminUser: UserWithPermissionsAndRoles = {
+ ...ownerUser,
+ roles: {
+ ...(ownerUser?.roles || {}),
+ Admin: [['can_write', 'Dashboard']],
+ },
+ userId: 2,
+ username: 'admin',
+};
+
+const outsiderUser: UserWithPermissionsAndRoles = {
+ ...ownerUser,
+ userId: 3,
+ username: 'outsider',
+};
+
+const owner: Owner = {
+ first_name: 'Test',
+ id: ownerUser.userId,
+ last_name: 'User',
+ username: ownerUser.username,
+};
+
+const undefinedUser: UndefinedUser = {};
describe('findPermission', () => {
it('findPermission for single role', () => {
@@ -70,42 +114,6 @@ describe('findPermission', () => {
});
describe('canUserEditDashboard', () => {
- const ownerUser: UserWithPermissionsAndRoles = {
- createdOn: '2021-05-12T16:56:22.116839',
- email: 'user@example.com',
- firstName: 'Test',
- isActive: true,
- isAnonymous: false,
- lastName: 'User',
- userId: 1,
- username: 'owner',
- permissions: {},
- roles: { Alpha: [['can_write', 'Dashboard']] },
- };
-
- const adminUser: UserWithPermissionsAndRoles = {
- ...ownerUser,
- roles: {
- ...ownerUser.roles,
- Admin: [['can_write', 'Dashboard']],
- },
- userId: 2,
- username: 'admin',
- };
-
- const outsiderUser: UserWithPermissionsAndRoles = {
- ...ownerUser,
- userId: 3,
- username: 'outsider',
- };
-
- const owner: Owner = {
- first_name: 'Test',
- id: ownerUser.userId,
- last_name: 'User',
- username: ownerUser.username,
- };
-
const dashboard: Dashboard = {
id: 1,
dashboard_title: 'Test Dash',
@@ -136,9 +144,7 @@ describe('canUserEditDashboard', () => {
it('rejects missing roles', () => {
// in redux, when there is no user, the user is actually set to an empty object,
// so we need to handle missing roles as well as a missing user.s
- expect(
- canUserEditDashboard(dashboard, {} as UserWithPermissionsAndRoles),
- ).toEqual(false);
+ expect(canUserEditDashboard(dashboard, {})).toEqual(false);
});
it('rejects "admins" if the admin role does not have edit rights for some reason', () => {
expect(
@@ -149,3 +155,15 @@ describe('canUserEditDashboard', () => {
).toEqual(false);
});
});
+
+test('isUserAdmin returns true for admin user', () => {
+ expect(isUserAdmin(adminUser)).toEqual(true);
+});
+
+test('isUserAdmin returns false for undefined user', () => {
+ expect(isUserAdmin(undefinedUser)).toEqual(false);
+});
+
+test('isUserAdmin returns false for non-admin user', () => {
+ expect(isUserAdmin(ownerUser)).toEqual(false);
+});
diff --git a/superset-frontend/src/dashboard/util/findPermission.ts b/superset-frontend/src/dashboard/util/findPermission.ts
index d3a8b61eca..496f993bdf 100644
--- a/superset-frontend/src/dashboard/util/findPermission.ts
+++ b/superset-frontend/src/dashboard/util/findPermission.ts
@@ -17,7 +17,11 @@
* under the License.
*/
import memoizeOne from 'memoize-one';
-import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import {
+ isUserWithPermissionsAndRoles,
+ UndefinedUser,
+ UserWithPermissionsAndRoles,
+} from 'src/types/bootstrapTypes';
import Dashboard from 'src/types/Dashboard';
type UserRoles = Record<string, [string, string][]>;
@@ -36,18 +40,25 @@ export default findPermission;
// but is hardcoded in backend logic already, so...
const ADMIN_ROLE_NAME = 'admin';
-export const isUserAdmin = (user: UserWithPermissionsAndRoles) =>
- Object.keys(user.roles).some(role => role.toLowerCase() === ADMIN_ROLE_NAME);
+export const isUserAdmin = (
+ user: UserWithPermissionsAndRoles | UndefinedUser,
+) =>
+ isUserWithPermissionsAndRoles(user) &&
+ Object.keys(user.roles || {}).some(
+ role => role.toLowerCase() === ADMIN_ROLE_NAME,
+ );
const isUserDashboardOwner = (
dashboard: Dashboard,
- user: UserWithPermissionsAndRoles,
-) => dashboard.owners.some(owner => owner.username === user.username);
+ user: UserWithPermissionsAndRoles | UndefinedUser,
+) =>
+ isUserWithPermissionsAndRoles(user) &&
+ dashboard.owners.some(owner => owner.username === user.username);
export const canUserEditDashboard = (
dashboard: Dashboard,
- user?: UserWithPermissionsAndRoles | null,
+ user?: UserWithPermissionsAndRoles | UndefinedUser | null,
) =>
- !!user?.roles &&
+ isUserWithPermissionsAndRoles(user) &&
(isUserAdmin(user) || isUserDashboardOwner(dashboard, user)) &&
findPermission('can_write', 'Dashboard', user.roles);
diff --git a/superset-frontend/src/types/bootstrapTypes.ts b/superset-frontend/src/types/bootstrapTypes.ts
index 33314e7e46..8918ea8489 100644
--- a/superset-frontend/src/types/bootstrapTypes.ts
+++ b/superset-frontend/src/types/bootstrapTypes.ts
@@ -1,4 +1,5 @@
import { JsonObject, Locale } from '@superset-ui/core';
+import { isPlainObject } from 'lodash';
/**
* Licensed to the Apache Software Foundation (ASF) under one
@@ -37,6 +38,8 @@ export interface UserWithPermissionsAndRoles extends User {
roles: Record<string, [string, string][]>;
}
+export type UndefinedUser = {};
+
export type Dashboard = {
dttm: number;
id: number;
@@ -62,3 +65,13 @@ export interface CommonBootstrapData {
locale: Locale;
feature_flags: Record<string, boolean>;
}
+
+export function isUser(user: any): user is User {
+ return isPlainObject(user) && 'username' in user;
+}
+
+export function isUserWithPermissionsAndRoles(
+ user: any,
+): user is UserWithPermissionsAndRoles {
+ return isUser(user) && 'permissions' in user && 'roles' in user;
+}