fix: Remove user.permissions, resolve it from accounts (#9990)

Remove the `user.permissions` field and resolve the permissions directly
from the accounts array in the user. This change ensures that the cache
or previous values from the last active account don't affect the
permissions.

In this PR: 
- Remove user.permissions usage, replace it with getUserPermissions
method.
- Remove json.permissions from user.json.jbuilder
This commit is contained in:
Pranav
2024-08-21 11:36:26 +05:30
committed by GitHub
parent 04b67eb431
commit 77b718c22c
10 changed files with 87 additions and 30 deletions

View File

@@ -8,7 +8,10 @@ import { useRoute, useRouter } from 'dashboard/composables/route';
import PrimarySidebar from './sidebarComponents/Primary.vue'; import PrimarySidebar from './sidebarComponents/Primary.vue';
import SecondarySidebar from './sidebarComponents/Secondary.vue'; import SecondarySidebar from './sidebarComponents/Secondary.vue';
import { routesWithPermissions } from '../../routes'; import { routesWithPermissions } from '../../routes';
import { hasPermissions } from '../../helper/permissionsHelper'; import {
getUserPermissions,
hasPermissions,
} from '../../helper/permissionsHelper';
export default { export default {
components: { components: {
@@ -113,7 +116,10 @@ export default {
return getSidebarItems(this.accountId); return getSidebarItems(this.accountId);
}, },
primaryMenuItems() { primaryMenuItems() {
const userPermissions = this.currentUser.permissions; const userPermissions = getUserPermissions(
this.currentUser,
this.accountId
);
const menuItems = this.sideMenuConfig.primaryMenu; const menuItems = this.sideMenuConfig.primaryMenu;
return menuItems.filter(menuItem => { return menuItems.filter(menuItem => {
const isAvailableForTheUser = hasPermissions( const isAvailableForTheUser = hasPermissions(

View File

@@ -4,7 +4,10 @@ import SecondaryNavItem from './SecondaryNavItem.vue';
import AccountContext from './AccountContext.vue'; import AccountContext from './AccountContext.vue';
import { mapGetters } from 'vuex'; import { mapGetters } from 'vuex';
import { FEATURE_FLAGS } from '../../../featureFlags'; import { FEATURE_FLAGS } from '../../../featureFlags';
import { hasPermissions } from '../../../helper/permissionsHelper'; import {
getUserPermissions,
hasPermissions,
} from '../../../helper/permissionsHelper';
import { routesWithPermissions } from '../../../routes'; import { routesWithPermissions } from '../../../routes';
export default { export default {
@@ -59,7 +62,10 @@ export default {
accessibleMenuItems() { accessibleMenuItems() {
const menuItemsFilteredByPermissions = this.menuConfig.menuItems.filter( const menuItemsFilteredByPermissions = this.menuConfig.menuItems.filter(
menuItem => { menuItem => {
const { permissions: userPermissions = [] } = this.currentUser; const userPermissions = getUserPermissions(
this.currentUser,
this.accountId
);
return hasPermissions( return hasPermissions(
routesWithPermissions[menuItem.toStateName], routesWithPermissions[menuItem.toStateName],
userPermissions userPermissions

View File

@@ -1,7 +1,11 @@
<script setup> <script setup>
import { useStoreGetters } from 'dashboard/composables/store'; import { useStoreGetters } from 'dashboard/composables/store';
import { computed } from 'vue'; import { computed } from 'vue';
import { hasPermissions } from '../helper/permissionsHelper'; import {
getUserPermissions,
hasPermissions,
} from '../helper/permissionsHelper';
const props = defineProps({ const props = defineProps({
permissions: { permissions: {
type: Array, type: Array,
@@ -10,12 +14,17 @@ const props = defineProps({
}); });
const getters = useStoreGetters(); const getters = useStoreGetters();
const user = getters.getCurrentUser.value; const user = computed(() => getters.getCurrentUser.value);
const hasPermission = computed(() => const accountId = computed(() => getters.getCurrentAccountId.value);
hasPermissions(props.permissions, user.permissions) const userPermissions = computed(() => {
); return getUserPermissions(user.value, accountId.value);
});
const hasPermission = computed(() => {
return hasPermissions(props.permissions, userPermissions.value);
});
</script> </script>
<!-- eslint-disable vue/no-root-v-if -->
<template> <template>
<div v-if="hasPermission"> <div v-if="hasPermission">
<slot /> <slot />

View File

@@ -7,6 +7,15 @@ export const hasPermissions = (
); );
}; };
export const getCurrentAccount = ({ accounts } = {}, accountId = null) => {
return accounts.find(account => Number(account.id) === Number(accountId));
};
export const getUserPermissions = (user, accountId) => {
const currentAccount = getCurrentAccount(user, accountId) || {};
return currentAccount.permissions || [];
};
const isPermissionsPresentInRoute = route => const isPermissionsPresentInRoute = route =>
route.meta && route.meta.permissions; route.meta && route.meta.permissions;

View File

@@ -1,9 +1,8 @@
import { hasPermissions } from './permissionsHelper'; import {
hasPermissions,
// eslint-disable-next-line default-param-last getUserPermissions,
export const getCurrentAccount = ({ accounts } = {}, accountId) => { getCurrentAccount,
return accounts.find(account => account.id === accountId); } from './permissionsHelper';
};
export const routeIsAccessibleFor = (route, userPermissions = []) => { export const routeIsAccessibleFor = (route, userPermissions = []) => {
const { meta: { permissions: routePermissions = [] } = {} } = route; const { meta: { permissions: routePermissions = [] } = {} } = route;
@@ -19,7 +18,9 @@ const validateActiveAccountRoutes = (to, user) => {
return accountDashboardURL; return accountDashboardURL;
} }
const isAccessible = routeIsAccessibleFor(to, user.permissions); const userPermissions = getUserPermissions(user, to.params.accountId);
const isAccessible = routeIsAccessibleFor(to, userPermissions);
// If the route is not accessible for the user, return to dashboard screen // If the route is not accessible for the user, return to dashboard screen
return isAccessible ? null : accountDashboardURL; return isAccessible ? null : accountDashboardURL;
}; };

View File

@@ -1,8 +1,31 @@
import { import {
buildPermissionsFromRouter, buildPermissionsFromRouter,
getCurrentAccount,
getUserPermissions,
hasPermissions, hasPermissions,
} from '../permissionsHelper'; } from '../permissionsHelper';
describe('#getCurrentAccount', () => {
it('should return the current account', () => {
expect(getCurrentAccount({ accounts: [{ id: 1 }] }, 1)).toEqual({ id: 1 });
expect(getCurrentAccount({ accounts: [] }, 1)).toEqual(undefined);
});
});
describe('#getUserPermissions', () => {
it('should return the correct permissions', () => {
const user = {
accounts: [
{ id: 1, permissions: ['conversations_manage'] },
{ id: 3, permissions: ['contacts_manage'] },
],
};
expect(getUserPermissions(user, 1)).toEqual(['conversations_manage']);
expect(getUserPermissions(user, '3')).toEqual(['contacts_manage']);
expect(getUserPermissions(user, 2)).toEqual([]);
});
});
describe('hasPermissions', () => { describe('hasPermissions', () => {
it('returns true if permission is present', () => { it('returns true if permission is present', () => {
expect( expect(

View File

@@ -1,19 +1,11 @@
import { import {
getConversationDashboardRoute, getConversationDashboardRoute,
getCurrentAccount,
isAConversationRoute, isAConversationRoute,
routeIsAccessibleFor, routeIsAccessibleFor,
validateLoggedInRoutes, validateLoggedInRoutes,
isAInboxViewRoute, isAInboxViewRoute,
} from '../routeHelpers'; } from '../routeHelpers';
describe('#getCurrentAccount', () => {
it('should return the current account', () => {
expect(getCurrentAccount({ accounts: [{ id: 1 }] }, 1)).toEqual({ id: 1 });
expect(getCurrentAccount({ accounts: [] }, 1)).toEqual(undefined);
});
});
describe('#routeIsAccessibleFor', () => { describe('#routeIsAccessibleFor', () => {
it('should return the correct access', () => { it('should return the correct access', () => {
let route = { meta: { permissions: ['administrator'] } }; let route = { meta: { permissions: ['administrator'] } };

View File

@@ -34,8 +34,14 @@ describe('#validateAuthenticateRoutePermission', () => {
getCurrentUser: { getCurrentUser: {
account_id: 1, account_id: 1,
id: 1, id: 1,
permissions: ['agent'], accounts: [
accounts: [{ id: 1, role: 'agent', status: 'active' }], {
permissions: ['agent'],
id: 1,
role: 'agent',
status: 'active',
},
],
}, },
}; };
validateAuthenticateRoutePermission(to, next, { getters }); validateAuthenticateRoutePermission(to, next, { getters });
@@ -55,8 +61,14 @@ describe('#validateAuthenticateRoutePermission', () => {
getCurrentUser: { getCurrentUser: {
account_id: 1, account_id: 1,
id: 1, id: 1,
permissions: ['administrator'], accounts: [
accounts: [{ id: 1, role: 'administrator', status: 'active' }], {
id: 1,
role: 'administrator',
permissions: ['administrator'],
status: 'active',
},
],
}, },
}; };
validateAuthenticateRoutePermission(to, next, { getters }); validateAuthenticateRoutePermission(to, next, { getters });

View File

@@ -14,7 +14,6 @@ json.provider resource.provider
json.pubsub_token resource.pubsub_token json.pubsub_token resource.pubsub_token
json.custom_attributes resource.custom_attributes if resource.custom_attributes.present? json.custom_attributes resource.custom_attributes if resource.custom_attributes.present?
json.role resource.active_account_user&.role json.role resource.active_account_user&.role
json.permissions resource.active_account_user&.permissions
json.ui_settings resource.ui_settings json.ui_settings resource.ui_settings
json.uid resource.uid json.uid resource.uid
json.type resource.type json.type resource.type

View File

@@ -50,7 +50,7 @@ RSpec.describe 'Session', type: :request do
as: :json as: :json
expect(response).to have_http_status(:success) expect(response).to have_http_status(:success)
expect(response.parsed_body['data']['permissions']).to eq(['agent']) expect(response.parsed_body['data']['accounts'].first['permissions']).to eq(['agent'])
end end
end end