Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cluster member role user should not be able to delete node on Harvester Hosts page #83

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: cluster member role user should not be able to delete node on Harvester Hosts page #83

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c791399
fix: disable delete action
houhoucoop Jan 13, 2025
43b10b2
refactor: move logic to node model
houhoucoop Jan 14, 2025
3565b44
refactor: fix lint warnings
houhoucoop Jan 15, 2025
f5961f3
fix: check user delete permission
houhoucoop Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion pkg/harvester/list/harvesterhci.io.host.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import ResourceTable from '@shell/components/ResourceTable';
import Loading from '@shell/components/Loading';
import { STATE, NAME, AGE } from '@shell/config/table-headers';
import {
CAPI, METRIC, NODE, SCHEMA, LONGHORN, POD
CAPI, METRIC, NODE, SCHEMA, LONGHORN, POD, MANAGEMENT, NORMAN
} from '@shell/config/types';
import { allHash } from '@shell/utils/promise';
import metricPoller from '@shell/mixins/metric-poller';
Expand Down Expand Up @@ -62,8 +62,42 @@ export default {
_hash.machines = this.$store.dispatch(`${ inStore }/findAll`, { type: CAPI.MACHINE });
}

if (
this.$store.getters['rancher/schemaFor'](NORMAN.PRINCIPAL) &&
this.$store.getters['rancher/schemaFor'](NORMAN.CLUSTER_ROLE_TEMPLATE_BINDING)
) {
_hash.normanPrincipal = this.$store.dispatch('rancher/findAll', { type: NORMAN.PRINCIPAL });
_hash.clusterRoleTemplateBinding = this.$store.dispatch(`management/findAll`, { type: MANAGEMENT.CLUSTER_ROLE_TEMPLATE_BINDING });
}

const hash = await allHash(_hash);

// Remove delete action if current user role is cluster member
if (hash.normanPrincipal && hash.clusterRoleTemplateBinding) {
const role = hash.clusterRoleTemplateBinding.find(
(template) => template.userPrincipalName === hash.normanPrincipal[0]?.id
);
const isClusterMember = role?.roleTemplateName === 'cluster-member';

if (isClusterMember) {
hash.nodes = hash.nodes.map((node) => {
const updatedActions = node.availableActions.map((action) => {
return action.action === 'promptRemove' ? { ...action, enabled: false } : action;
});

// keep availableActions non-enumerable
Object.defineProperty(node, 'availableActions', {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, availableActions should be defined in the model, in this case it would be pkg/harvester/models/harvester/node.js.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, but the trade-off is that the model will have a broader impact, and using async within the getter could lead to infinite recursion, so we need the _initialized flag to prevent repetition.
update in commit 43b10b2.

value: updatedActions,
writable: true,
enumerable: false,
configurable: true,
});

return node;
});
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if there's a more elegant way to handle this, any suggestions would be helpful. 🙏


this.rows = hash.nodes;
},

Expand Down
Loading