Skip to content

Commit

Permalink
Filter out unnecessary options (#2946)
Browse files Browse the repository at this point in the history
* Extract an `isPermitted` function that checks whether user abilities is contained in a list of permitted abilities

* Add an `allowedActivities` function that returns relevant activities given a user's abilities

* Render only relevant activities for user in Activity log page filters

* Improve activity types list generation

* Fix reference to users ability
  • Loading branch information
nelsonkopliku authored Sep 9, 2024
1 parent f657323 commit 68c14a4
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 120 deletions.
7 changes: 2 additions & 5 deletions assets/js/common/DisabledGuard/DisabledGuard.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isPermitted } from '@lib/model/users';

import Tooltip from '@common/Tooltip';

Expand All @@ -22,11 +23,7 @@ function DisabledGuard({
}) {
const permittedFor = ALL_PERMITTED.concat(permitted);

const isPermitted = userAbilities
.map(({ name, resource }) => `${name}:${resource}`)
.some((ability) => permittedFor.includes(ability));

if (isPermitted) {
if (isPermitted(userAbilities, permittedFor)) {
return children;
}

Expand Down
110 changes: 28 additions & 82 deletions assets/js/lib/model/activityLog.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { has } from 'lodash';
import { entries, filter, get, map, pipe } from 'lodash/fp';
import { isPermitted } from './users';

export const LOGIN_ATTEMPT = 'login_attempt';
export const RESOURCE_TAGGING = 'resource_tagging';
export const RESOURCE_UNTAGGING = 'resource_untagging';
Expand Down Expand Up @@ -98,88 +102,6 @@ export const DATABASE_ROLL_UP_REQUESTED = 'database_roll_up_requested';
export const DATABASE_TENANTS_UPDATED = 'database_tenants_updated';
export const DATABASE_TOMBSTONED = 'database_tombstoned';

export const ACTIVITY_TYPES = [
LOGIN_ATTEMPT,
RESOURCE_TAGGING,
RESOURCE_UNTAGGING,
API_KEY_GENERATION,
SAVING_SUMA_SETTINGS,
CHANGING_SUMA_SETTINGS,
CLEARING_SUMA_SETTINGS,
USER_CREATION,
USER_MODIFICATION,
USER_DELETION,
PROFILE_UPDATE,
CLUSTER_CHECKS_EXECUTION_REQUEST,
ACTIVITY_LOG_SETTINGS_UPDATE,
// Host events
HEARTBEAT_FAILED,
HEARTBEAT_SUCCEEDED,
HOST_CHECKS_HEALTH_CHANGED,
HOST_CHECKS_SELECTED,
HOST_DEREGISTERED,
HOST_DEREGISTRATION_REQUESTED,
HOST_DETAILS_UPDATED,
HOST_HEALTH_CHANGED,
HOST_REGISTERED,
HOST_RESTORED,
HOST_ROLLED_UP,
HOST_ROLL_UP_REQUESTED,
HOST_SAPTUNE_HEALTH_CHANGED,
HOST_TOMBSTONED,
PROVIDER_UPDATED,
SAPTUNE_STATUS_UPDATED,
SLES_SUBSCRIPTIONS_UPDATED,
SOFTWARE_UPDATES_DISCOVERY_CLEARED,
SOFTWARE_UPDATES_DISCOVERY_REQUESTED,
SOFTWARE_UPDATES_HEALTH_CHANGED,
// Cluster events
CHECKS_SELECTED,
CLUSTER_CHECKS_HEALTH_CHANGED,
CLUSTER_DEREGISTERED,
CLUSTER_DETAILS_UPDATED,
CLUSTER_DISCOVERED_HEALTH_CHANGED,
CLUSTER_HEALTH_CHANGED,
CLUSTER_REGISTERED,
CLUSTER_RESTORED,
CLUSTER_ROLLED_UP,
CLUSTER_ROLL_UP_REQUESTED,
CLUSTER_TOMBSTONED,
HOST_ADDED_TO_CLUSTER,
HOST_REMOVED_FROM_CLUSTER,
// SAP System events
APPLICATION_INSTANCE_DEREGISTERED,
APPLICATION_INSTANCE_HEALTH_CHANGED,
APPLICATION_INSTANCE_MARKED_ABSENT,
APPLICATION_INSTANCE_MARKED_PRESENT,
APPLICATION_INSTANCE_MOVED,
APPLICATION_INSTANCE_REGISTERED,
SAP_SYSTEM_DATABASE_HEALTH_CHANGED,
SAP_SYSTEM_DEREGISTERED,
SAP_SYSTEM_HEALTH_CHANGED,
SAP_SYSTEM_REGISTERED,
SAP_SYSTEM_RESTORED,
SAP_SYSTEM_ROLLED_UP,
SAP_SYSTEM_ROLL_UP_REQUESTED,
SAP_SYSTEM_TOMBSTONED,
SAP_SYSTEM_UPDATED,
// Database events
DATABASE_DEREGISTERED,
DATABASE_HEALTH_CHANGED,
DATABASE_INSTANCE_DEREGISTERED,
DATABASE_INSTANCE_HEALTH_CHANGED,
DATABASE_INSTANCE_MARKED_ABSENT,
DATABASE_INSTANCE_MARKED_PRESENT,
DATABASE_INSTANCE_REGISTERED,
DATABASE_INSTANCE_SYSTEM_REPLICATION_CHANGED,
DATABASE_REGISTERED,
DATABASE_RESTORED,
DATABASE_ROLLED_UP,
DATABASE_ROLL_UP_REQUESTED,
DATABASE_TENANTS_UPDATED,
DATABASE_TOMBSTONED,
];

const sumaSettingsResourceType = (_entry) => 'SUMA Settings';
const userResourceType = (_entry) => 'User';
const clusterResourceType = (_entry) => 'Cluster';
Expand All @@ -195,12 +117,15 @@ const taggingResourceType = (entry) =>
sap_system: sapSystemResourceType(entry),
})[entry.metadata?.resource_type] ?? 'Unable to determine resource type';

const userManagement = ['all:all', 'all:users'];

export const ACTIVITY_TYPES_CONFIG = {
[LOGIN_ATTEMPT]: {
label: 'Login Attempt',
message: ({ metadata }) =>
metadata?.reason ? 'Login failed' : 'User logged in',
resource: (_entry) => 'Application',
allowedTo: userManagement,
},
[RESOURCE_TAGGING]: {
label: 'Tag Added',
Expand Down Expand Up @@ -238,21 +163,25 @@ export const ACTIVITY_TYPES_CONFIG = {
label: 'User Created',
message: (_entry) => `User was created`,
resource: userResourceType,
allowedTo: userManagement,
},
[USER_MODIFICATION]: {
label: 'User Modified',
message: (_entry) => `User was modified`,
resource: userResourceType,
allowedTo: userManagement,
},
[USER_DELETION]: {
label: 'User Deleted',
message: (_entry) => `User was deleted`,
resource: userResourceType,
allowedTo: userManagement,
},
[PROFILE_UPDATE]: {
label: 'Profile Updated',
message: (_entry) => `User modified profile`,
resource: (_entry) => 'Profile',
allowedTo: userManagement,
},
[CLUSTER_CHECKS_EXECUTION_REQUEST]: {
label: 'Checks Execution Requested',
Expand Down Expand Up @@ -580,6 +509,11 @@ export const ACTIVITY_TYPES_CONFIG = {
},
};

export const ACTIVITY_TYPES = pipe(
entries,
map(([key, _value]) => key)
)(ACTIVITY_TYPES_CONFIG);

const activityTypeConfig = ({ type }) => ACTIVITY_TYPES_CONFIG[type];

export const toLabel = (entry) =>
Expand All @@ -591,6 +525,18 @@ export const toMessage = (entry) =>
export const toResource = (entry) =>
activityTypeConfig(entry)?.resource(entry) ?? 'Unrecognized resource';

export const allowedActivities = (abilities) =>
pipe(
entries,
filter(
([_key, value]) =>
!has(value, 'allowedTo') ||
pipe(get('allowedTo'), (allowedTo) =>
isPermitted(abilities, allowedTo)
)(value)
)
)(ACTIVITY_TYPES_CONFIG);

export const LEVEL_DEBUG = 'debug';
export const LEVEL_INFO = 'info';
export const LEVEL_WARNING = 'warning';
Expand Down
49 changes: 49 additions & 0 deletions assets/js/lib/model/activityLog.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { abilityFactory } from '@lib/test-utils/factories/users';
import { difference } from 'lodash';
import {
ACTIVITY_TYPES,
allowedActivities,
LOGIN_ATTEMPT,
PROFILE_UPDATE,
USER_CREATION,
USER_DELETION,
USER_MODIFICATION,
} from './activityLog';

const nonUserManagementActivities = difference(ACTIVITY_TYPES, [
LOGIN_ATTEMPT,
USER_CREATION,
USER_MODIFICATION,
USER_DELETION,
PROFILE_UPDATE,
]);

describe('activityLog', () => {
it.each`
userAbilities | hasUserMgmtActivities
${[]} | ${false}
${[['all', 'all']]} | ${true}
${[['all', 'users']]} | ${true}
${[['all', 'all'], ['all', 'users']]} | ${true}
${[['all', 'all'], ['foo', 'bar']]} | ${true}
${[['all', 'users'], ['bar', 'baz']]} | ${true}
${[['baz', 'qux'], ['bar', 'baz']]} | ${false}
${[['baz', 'qux']]} | ${false}
${[['qux', 'ber']]} | ${false}
`(
'should return relevant activities for the given user abilities',
({ userAbilities, hasUserMgmtActivities }) => {
const abilities = userAbilities.map(([name, resource]) =>
abilityFactory.build({ name, resource })
);

const relevantActivities = allowedActivities(abilities).map(
([key, _value]) => key
);

hasUserMgmtActivities
? expect(relevantActivities).toEqual(ACTIVITY_TYPES)
: expect(relevantActivities).toEqual(nonUserManagementActivities);
}
);
});
5 changes: 5 additions & 0 deletions assets/js/lib/model/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@ import { getFromConfig } from '@lib/config';
const TRENTO_ADMIN_USERNAME = getFromConfig('adminUsername') || 'admin';

export const isAdmin = (user) => user.username === TRENTO_ADMIN_USERNAME;

export const isPermitted = (userAbilities, permittedFor) =>
userAbilities
.map(({ name, resource }) => `${name}:${resource}`)
.some((ability) => permittedFor.includes(ability));
28 changes: 26 additions & 2 deletions assets/js/lib/model/users.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { adminUser, userFactory } from '@lib/test-utils/factories/users';
import {
adminUser,
userFactory,
abilityFactory,
} from '@lib/test-utils/factories/users';

import { isAdmin } from './users';
import { isPermitted, isAdmin } from './users';

describe('users', () => {
it('should check if a user is admin', () => {
Expand All @@ -10,4 +14,24 @@ describe('users', () => {
const user = userFactory.build({ username: 'other' });
expect(isAdmin(user)).toBe(false);
});

it.each`
permittedFor | expectedIsPermitted
${[]} | ${false}
${['foo:bar']} | ${true}
${['baz:qux', 'bar:baz']} | ${true}
${['baz:qux']} | ${false}
${['qux:ber']} | ${false}
`(
'should check if a user has permissions',
({ permittedFor, expectedIsPermitted }) => {
const abilities = [
abilityFactory.build({ name: 'foo', resource: 'bar' }),
abilityFactory.build({ name: 'bar', resource: 'baz' }),
];

expect(isPermitted(abilities, permittedFor)).toBe(expectedIsPermitted);
expect(isPermitted([], permittedFor)).toBe(false);
}
);
});
59 changes: 33 additions & 26 deletions assets/js/pages/ActivityLogPage/ActivityLogPage.jsx
Original file line number Diff line number Diff line change
@@ -1,48 +1,55 @@
import React, { useState, useEffect } from 'react';
import { useSearchParams } from 'react-router-dom';
import { useSelector } from 'react-redux';

import { map, pipe } from 'lodash/fp';

import { getActivityLog } from '@lib/api/activityLogs';
import { allowedActivities } from '@lib/model/activityLog';

import { getUserProfile } from '@state/selectors/user';

import PageHeader from '@common/PageHeader';
import ActivityLogOverview from '@common/ActivityLogOverview';
import ComposedFilter from '@common/ComposedFilter';

import { getActivityLog } from '@lib/api/activityLogs';
import { ACTIVITY_TYPES_CONFIG } from '@lib/model/activityLog';
import {
filterValueToSearchParams,
searchParamsToAPIParams,
searchParamsToFilterValue,
} from './searchParams';

const filters = [
{
key: 'type',
type: 'select',
title: 'Resource type',
options: Object.entries(ACTIVITY_TYPES_CONFIG).map(([key, value]) => [
key,
value.label,
]),
},
{
key: 'to_date',
title: 'newer than',
type: 'date',
prefilled: true,
},
{
key: 'from_date',
title: 'older than',
type: 'date',
prefilled: true,
},
];

function ActivityLogPage() {
const [searchParams, setSearchParams] = useSearchParams();
const [activityLog, setActivityLog] = useState([]);
const [isLoading, setLoading] = useState(true);
const [activityLogDetailModalOpen, setActivityLogDetailModalOpen] =
useState(false);
const { abilities } = useSelector(getUserProfile);

const filters = [
{
key: 'type',
type: 'select',
title: 'Resource type',
options: pipe(
allowedActivities,
map(([key, value]) => [key, value.label])
)(abilities),
},
{
key: 'to_date',
title: 'newer than',
type: 'date',
prefilled: true,
},
{
key: 'from_date',
title: 'older than',
type: 'date',
prefilled: true,
},
];

const fetchActivityLog = () => {
setLoading(true);
Expand Down
Loading

0 comments on commit 68c14a4

Please sign in to comment.