cloudstack icon indicating copy to clipboard operation
cloudstack copied to clipboard

Hide volumes tab in instance page when user does not have permission to list volumes

Open winterhazel opened this issue 11 months ago • 1 comments

Description

In any instance's page, the volumes tab always gets shown, even if the account does not have permission to list the instance's volumes.

This PR adds a check in the UI in order to hide the volumes tab if the current account does not have permission to use the listVolumes API.

Types of changes

  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [X] Enhancement (improves an existing feature and functionality)
  • [ ] Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • [ ] Major
  • [X] Minor

Screenshots (if appropriate):

Before, a user without permission could access the volumes tab, but the volumes would not get listed. With these changes, the volumes tab does not get shown.

Screenshot from 2024-02-27 16-07-56

How Has This Been Tested?

  1. I created a role that had access to listVolumes;
  2. I created an account using the role;
  3. I created a VM using the account;
  4. In the created account, I accessed the VM's page and verified that the volumes tab was shown;
  5. I removed the access to listVolumes from the role created in step 1;
  6. In the created account, I accessed the VM's page again and verified that the volumes tab was not shown.

winterhazel avatar Feb 27 '24 19:02 winterhazel

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 30.98%. Comparing base (c8a4575) to head (2faf3aa).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8713   +/-   ##
=========================================
  Coverage     30.98%   30.98%           
- Complexity    33497    33505    +8     
=========================================
  Files          5355     5355           
  Lines        375780   375780           
  Branches      54913    54913           
=========================================
+ Hits         116429   116445   +16     
+ Misses       243932   243905   -27     
- Partials      15419    15430   +11     
Flag Coverage Δ
simulator-marvin-tests 24.85% <ø> (+<0.01%) :arrow_up:
uitests 4.36% <ø> (ø)
unit-tests 16.58% <ø> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 27 '24 19:02 codecov[bot]

@blueorangutan ui

DaanHoogland avatar Feb 28 '24 07:02 DaanHoogland

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

blueorangutan avatar Feb 28 '24 07:02 blueorangutan

UI build: :heavy_check_mark: Live QA URL: https://qa.cloudstack.cloud/simulator/pr/8713 (QA-JID-291)

blueorangutan avatar Feb 28 '24 08:02 blueorangutan

tested works in qa, but note that also the whole storage tab in the main menu has disappeared: image

was this intended @winterhazel ? (no snapshots/backups)

DaanHoogland avatar Feb 28 '24 08:02 DaanHoogland

tested works in qa, but note that also the whole storage tab in the main menu has disappeared: image

was this intended @winterhazel ? (no snapshots/backups)

@DaanHoogland the changes in this PR do not affect the storage tab in the main menu.

There are two APIs for listing volumes: listVolumesMetrics (used in the volumes listing page) and listVolumes (used in the instance's page, where this PR is adding a check). I suppose that the storage tab disappeared for you because the access to listVolumesMetrics has been denied (this is the current behavior, which has not been affected by this PR).

winterhazel avatar Feb 28 '24 16:02 winterhazel

tested works in qa, but note that also the whole storage tab in the main menu has disappeared: ![image](https:// was this intended @winterhazel ? (no snapshots/backups)

@DaanHoogland the changes in this PR do not affect the storage tab in the main menu.

There are two APIs for listing volumes: listVolumesMetrics (used in the volumes listing page) and listVolumes (used in the instance's page, where this PR is adding a check). I suppose that the storage tab disappeared for you because the access to listVolumesMetrics has been denied (this is the current behavior, which has not been affected by this PR).

Ok, this then is another bug, no storage page when rights to listBackups and listSnapshots do exist :(

DaanHoogland avatar Feb 28 '24 20:02 DaanHoogland

Manually tested this, LGTM. image


tested works in qa, but note that also the whole storage tab in the main menu has disappeared: ![image](https:// was this intended @winterhazel ? (no snapshots/backups)

@DaanHoogland the changes in this PR do not affect the storage tab in the main menu. There are two APIs for listing volumes: listVolumesMetrics (used in the volumes listing page) and listVolumes (used in the instance's page, where this PR is adding a check). I suppose that the storage tab disappeared for you because the access to listVolumesMetrics has been denied (this is the current behavior, which has not been affected by this PR).

Ok, this then is another bug, no storage page when rights to listBackups and listSnapshots do exist :(

Regarding this situation I will create an issue, may even create a PR for this, as this should be an easy fix.

BryanMLima avatar Feb 29 '24 17:02 BryanMLima

@sureshanaparti are your concerns met?

DaanHoogland avatar Mar 11 '24 15:03 DaanHoogland

@sureshanaparti are your concerns met?

Ping @sureshanaparti

winterhazel avatar Apr 24 '24 14:04 winterhazel