fix: login button does not render (#19685)

* fix: login button does not render * add type guard
apache · May 27, 2022 · c3604dc · c3604dc
1 parent 17e10b7
commit c3604dc
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 48 deletions.
diff --git 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
@@ -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';
 
-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
@@ -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;
@@ -52,3 +55,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;
+}