fix: block user from removing own admin roles

[shubhamv-ss]

Description

GOAL:
To ensure that a company can not block itself by removing all Adminroles from their enabled users.

Option 1:

We check how many users in the company have the respective admin role (e.g. CX Admin) and allow the removal only if there are >2 people with this specific role.
Otherwise, the role selector will be greyed out and the tooltip will explain the situation

Option 2:

We grey out all admin roles from a user if he goes into his role configuration.
Only if someone else would try to remove his role this would be possible.

This would at least ensure that someone is not removing his own role on fault but would include a 4-eye principle process.

Option 1 has a dependency on BE as we don't have an API that fetches all users of the company with roles.
So I've implemented the option 2

Why

• User should not be able to change their own roles.
• Ref : https://github.com/eclipse-tractusx/portal-assets/blob/main/docs/user/03.%20User%20Management/01.%20User%20Account/02.%20User%20Account.md

Issue

• User should not be able to change their own roles. #986

Checklist

Please delete options that are not relevant.

• I have performed a self-review of my own code
• I have successfully tested my changes locally

[lavanya-bmw]

Please update changelog

[oyo]

lgtm

[ybidois]

@MaximilianHauer can you review/approve this asap? This is highest priority/security issue for customers. Thanks!

[evegufy]

I request changes to make sure that this PR isn't getting merged until the following conversion is clarified:

Hi @ybidois @shubhamv-ss could you please share for which roles you did observe this?
Under steps to reproduce of #986, you mention the CX Admin role. Which seems expected to me, also for the Company Admin this would be the expected behaviour, if you observe it for any other role, I'd also consider this bug.

Why wouldn't this be a bug for the roles CX Admin / Company Admin?

Thinking it through from a user administration process perspective, it wouldn't make sense for those roles not be able to change their own roles as they are the ones who are allowed to perform the user administration.
If this change gets implemented it would lead to a process where a CX Admin 1 or Company Admin 1 would need to assign the role CX Admin or Company to some other user (CX Admin 2 or Company Admin 2), which would then needed to be asked to change the roles of CX Admin 1 or Company Admin 1, is that really wanted?

And if this process is really wanted, it should be implemented in a secure manner which foremost requires a change in the portal backend, as doing this change in the frontend doesn't prohibit the change directly on the backend.

cc: @MaximilianHauer

[ybidois]

Hi @evegufy,

CX Admin, Company Admin, and IT Admin currently have the ability to modify their own roles. In our perspective, it's an issue because:

• We always needs one Admin role at all times. If there is only one Admin and s/he removes his/her own admin role, then the company is blocked. We believe it is best practice to have always 2 admins in case of issue, but one is really the minimum.
• What is the difference between CX Admin, Company Admin, and IT Admin if they can self-assign/remove all their roles? When we assign admin roles on the onboarding, customers need to be informed of the possibility for those admins to self-assign/remove roles to prevent privilege escalation.

Hence our proposal to restrict self-assignment, and as such, have minimum 1 Admin per company on onboarding and recommend our customers to have minimum two admins.

Ex. In Microsoft 365 admin center, global administrators cannot self-assign roles.

I definitely agree to your proposal to implement a process that is secure, and maybe with some backend changes. In the short run, preventing self-assignment was the best we could think of 🤔

What do you think? Isn't it better in terms of security? Any advice on how to manage it differently?

Again, next week, we're in productive environment.

cc: @MaximilianHauer

[MaximilianHauer]

The core issue of this topic is the "removability" of Admin roles and therefor the risk of paralyzing the whole company.
Beautiful option would be to have a call that retrieves all admins to see if the amount is >2 otherwise block the removal of the admin roles.
efficient and acceptable way would be to restrict the removal to the admin checkboxes this would solve the described risk but would not block the admin to remove/grant other roles to himself.

if you adjust the PR to "grey out the admin roles" and provide a short explanation that the admin is not allowed to remove his- own admin roles by himself i would find it a good in-between solution

[ybidois]

The core issue of this topic is the "removability" of Admin roles and therefor the risk of paralyzing the whole company. Beautiful option would be to have a call that retrieves all admins to see if the amount is >2 otherwise block the removal of the admin roles. efficient and acceptable way would be to restrict the removal to the admin checkboxes this would solve the described risk but would not block the admin to remove/grant other roles to himself.

if you adjust the PR to "grey out the admin roles" and provide a short explanation that the admin is not allowed to remove his- own admin roles by himself i would find it a good in-between solution

Good solution! @shubhamv-ss can you look into this?

[shubhamv-ss]

The core issue of this topic is the "removability" of Admin roles and therefor the risk of paralyzing the whole company. Beautiful option would be to have a call that retrieves all admins to see if the amount is >2 otherwise block the removal of the admin roles. efficient and acceptable way would be to restrict the removal to the admin checkboxes this would solve the described risk but would not block the admin to remove/grant other roles to himself.

if you adjust the PR to "grey out the admin roles" and provide a short explanation that the admin is not allowed to remove his- own admin roles by himself i would find it a good in-between solution

Hello @MaximilianHauer , @ybidois
Please find the attached recording for this change request.

Screen.Recording.2024-08-21.at.11.21.00.AM.mov

The following cases have been covered in this:

• If a logged-in user has less than 2 admin roles selected, s/he cannot change the selected admin roles but can add new roles.
• In the above case, checkboxes will grey out.
• If a logged-in user has more than 2 roles he can add/remove admin roles, but as soon as admin roles count reaches to 2, checkboxes will grey out.
• Same conditions will apply to other users if a logged-in user wants to change their admin roles. Then that user can have at least 2 admin roles.

[MaximilianHauer]

GOAL:
To ensure that a company can not block itself by removing all Adminroles from their enabled users.

Option 1:

• We check how many users in the company have the respective admin role (e.g. CX Admin) and allow the removal only if there are >2 person with this specific role.
• Otherwise the role selector will be greyed out and the tooltip will explain the situation

Option 2:
We grey out all admin roles from a user if he goes into his role configuration.
Only if someone else would try to remove his role this would be possible.

this would at least ensure that someone is not removing his own role on fault but would include a 4-eye principle process.

current implementation proposal
You delivered a mix of both that does not prevent the initial issue but only prevents the person to remove all of his roles.
The issue is by your solution you block someone at all to get downgraded from admin to user and the logic itself is not intuitive

[shubhamv-ss]

GOAL: To ensure that a company can not block itself by removing all Adminroles from their enabled users.

Option 1:

• We check how many users in the company have the respective admin role (e.g. CX Admin) and allow the removal only if there are >2 person with this specific role.
• Otherwise the role selector will be greyed out and the tooltip will explain the situation

Option 2: We grey out all admin roles from a user if he goes into his role configuration. Only if someone else would try to remove his role this would be possible.

this would at least ensure that someone is not removing his own role on fault but would include a 4-eye principle process.

current implementation proposal You delivered a mix of both that does not prevent the initial issue but only prevents the person to remove all of his roles. The issue is by your solution you block someone at all to get downgraded from admin to user and the logic itself is not intuitive

Option 1 has a dependency on BE as we don't have API that fetches all users of company with roles.
So I'll go with Option 2 and will adjust the PR accordingly.
Thanks for the detailed explaination.

[shubhamv-ss]

GOAL: To ensure that a company can not block itself by removing all Adminroles from their enabled users.

Option 1:

• We check how many users in the company have the respective admin role (e.g. CX Admin) and allow the removal only if there are >2 person with this specific role.
• Otherwise the role selector will be greyed out and the tooltip will explain the situation

Option 2: We grey out all admin roles from a user if he goes into his role configuration. Only if someone else would try to remove his role this would be possible.

this would at least ensure that someone is not removing his own role on fault but would include a 4-eye principle process.

current implementation proposal You delivered a mix of both that does not prevent the initial issue but only prevents the person to remove all of his roles. The issue is by your solution you block someone at all to get downgraded from admin to user and the logic itself is not intuitive

Screen.Recording.2024-08-21.at.4.16.48.PM.mov

Hi @MaximilianHauer

• In recording, Shubham is logged in user, so all the admin roles are grey out/disabled for him.
• But for other users like Operator Cx Admin, all options are available to update.

[dhiren-singh-007]

GOAL: To ensure that a company can not block itself by removing all Adminroles from their enabled users.

Option 1:

• We check how many users in the company have the respective admin role (e.g. CX Admin) and allow the removal only if there are >2 person with this specific role.
• Otherwise the role selector will be greyed out and the tooltip will explain the situation

Option 2: We grey out all admin roles from a user if he goes into his role configuration. Only if someone else would try to remove his role this would be possible.

this would at least ensure that someone is not removing his own role on fault but would include a 4-eye principle process.

current implementation proposal You delivered a mix of both that does not prevent the initial issue but only prevents the person to remove all of his roles. The issue is by your solution you block someone at all to get downgraded from admin to user and the logic itself is not intuitive

Hi @MaximilianHauer , I think we have to implement this validation in BE also else someone can do this via api as well.
IMO its better to keep it simple i.e User can not change its own role irrespective of existing admin users.
Another validation could be that only Admin users can assign/remove the roles of other user
Please share your thought than i will implement it in BE accordingly

[MaximilianHauer]

GOAL: To ensure that a company can not block itself by removing all Adminroles from their enabled users.
Option 1:

• We check how many users in the company have the respective admin role (e.g. CX Admin) and allow the removal only if there are >2 person with this specific role.
• Otherwise the role selector will be greyed out and the tooltip will explain the situation

Option 2: We grey out all admin roles from a user if he goes into his role configuration. Only if someone else would try to remove his role this would be possible.
this would at least ensure that someone is not removing his own role on fault but would include a 4-eye principle process.
current implementation proposal You delivered a mix of both that does not prevent the initial issue but only prevents the person to remove all of his roles. The issue is by your solution you block someone at all to get downgraded from admin to user and the logic itself is not intuitive

Screen.Recording.2024-08-21.at.4.16.48.PM.mov
Hi @MaximilianHauer

• In recording, Shubham is logged in user, so all the admin roles are grey out/disabled for him.
• But for other users like Operator Cx Admin, all options are available to update.

looks good to me :)

[shubhamv-ss]

Hi @MaximilianHauer , @evegufy ,
can you please review/approve this?
Thanks!

[MaximilianHauer]

business approval given , @oyo will have a look on the implementation

[oyo]

two small changes

[evegufy]

Could you please update the PR title and description to what this change is now about?

[shubhamv-ss]

Could you please update the PR title and description to what this change is now about?

Updated the description and translation files as suggested.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[evegufy]

@oyo could you please recheck?