Skip to content

Don't allow users to remove their account or user. - #7242

Merged
DaanHoogland merged 2 commits into
apache:mainfrom
leaseweb:don't-allow-user-remove-themselves
Apr 17, 2023
Merged

DaanHoogland merged 2 commits into
apache:mainfrom
leaseweb:don't-allow-user-remove-themselves

Conversation

@soreana

@soreana soreana commented Feb 16, 2023

Copy link
Copy Markdown
Member

Description

Cloudstack users can delete their account accidentally and lock themselves from accessing the CloudStack panel. A link to an account placed next to ISO, template, VM and other entities in CloudStack's UI. If customers doesn't paying attention, they would click on the account link instead of the link to the entity they want (vm, template, ISO etc.) Then, if they had wanted to delete that entity, they would press delete button without realising they were on the account page, and press Confirm.

Mailing list [thread]

This PR don't allow users to remove their account or their user.

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)
  • 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
  • Minor

Screenshots (if appropriate):

Remove.user.mp4

How Has This Been Tested?

  1. Create two account, Acc1 and Acc2.
  2. Login as the Acc1
  3. Go to the Acc1 page, delete button is not there.
  4. Go to the Acc2 page, delete button is there.
  5. Logout and login as the Acc2
  6. Again, you can see the delete button for the Acc1 but not for yourself.

@weizhouapache

Copy link
Copy Markdown
Member

@soreana
is there a way to disable it in backend ?

@soreana

soreana commented Feb 16, 2023

Copy link
Copy Markdown
Member Author

@weizhouapache I guess we can disable that in the backend as well, but I think anyone who uses the API, or the CloudMonkey knows what he does. So, I didn't disable that.

Another reason was that in the UI we are not able to delete other admin but we can do that using the CloudMonkey.

@codecov

codecov Bot commented Feb 16, 2023

Copy link
Copy Markdown

Codecov Report

Merging #7242 (05eb030) into main (597a803) will not change coverage.
The diff coverage is n/a.

❗ Current head 05eb030 differs from pull request most recent head 66ae105. Consider uploading reports for the commit 66ae105 to get more accurate results

@@            Coverage Diff            @@
##               main    #7242   +/-   ##
=========================================
  Coverage     12.67%   12.67%           
  Complexity     8639     8639           
=========================================
  Files          2716     2716           
  Lines        256112   256112           
  Branches      39926    39926           
=========================================
  Hits          32456    32456           
  Misses       219528   219528           
  Partials       4128     4128           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment thread ui/src/config/section/account.js Outdated
@soreana
soreana requested review from JoaoJandre and harikrishna-patnala and removed request for JoaoJandre and harikrishna-patnala February 24, 2023 16:44
@JoaoJandre

Copy link
Copy Markdown
Contributor

@soreana I tested these changes manually, I do not know if my environment is too slow, but if I am fast enough, I can click the delete button before it becomes disabled.

I'm not sure how to fix this though...

@soreana

soreana commented Feb 24, 2023

Copy link
Copy Markdown
Member Author

@soreana I tested these changes manually, I do not know if my environment is too slow, but if I am fast enough, I can click the delete button before it becomes disabled.

I'm not sure how to fix this though...

@JoaoJandre I guess the issue is related to the way UI is loaded. I'm not sure if I can control that, that is beyond my knowledge 😃

@DaanHoogland DaanHoogland added this to the 4.18.1.0 milestone Feb 27, 2023
@kiranchavala

Copy link
Copy Markdown
Member

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/7242 (QA-JID-58)

@kiranchavala

kiranchavala commented Feb 28, 2023

Copy link
Copy Markdown
Member

@soreana Tested the ui LGTM

The delete account icon is graded out when trying to delete its own account ,

The delete account icon is not graded out when trying to delete other accounts

domainadmin

@JoaoJandre JoaoJandre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Apart from the one caveat I mentioned, it worked fine in my tests and the code looks good to me.

@soreana

soreana commented Apr 7, 2023

Copy link
Copy Markdown
Member Author

@harikrishna-patnala Can you please check this pr when you have time? Thanks.

@kiranchavala kiranchavala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, tested the changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants