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.

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

978 plain user can't change his role. #984

Closed

Conversation

TahaKhanAbdalli
Copy link
Collaborator

fixes #978

@IanMayo IanMayo temporarily deployed to rco-review-pr-984 January 10, 2024 13:43 Inactive
@@ -42,6 +42,7 @@ export default function UserForm({ isEdit }: FormProps): React.ReactElement {
role: 'rco-user'
}
const { record } = useEditContext()
const isPowerUser = record?.role === ''
Copy link
Contributor

Choose a reason for hiding this comment

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

@TahaKhanAbdalli - we really need to check they are a power user: rco-power-user

image

@IanMayo IanMayo temporarily deployed to rco-review-pr-984 January 10, 2024 14:37 Inactive
@IanMayo
Copy link
Contributor

IanMayo commented Jan 10, 2024

Aah @TahaKhanAbdalli - I've realised there's a higher level of security here. For the EditShow page, only rco-power-users should be able to Edit a user. The button should be disabled for other roles (or if there is no role).

Aah, but a non power-user could still manually navigate to http://localhost:5173/#/user/2/edit. So, we should disable the EditForm if the current user isn't a power-user.

@IanMayo IanMayo temporarily deployed to rco-review-pr-984 January 10, 2024 14:39 Inactive
@TahaKhanAbdalli
Copy link
Collaborator Author

Sure, I got your point. I will look into it tomorrow.

@IanMayo
Copy link
Contributor

IanMayo commented Jan 11, 2024

I'm happy to leave the linter issue to you @TahaKhanAbdalli

image

@IanMayo
Copy link
Contributor

IanMayo commented Jan 11, 2024

Aah, I've realised the issue isn't quite worded properly.

If I'm not a power user I can't change anyone's role.

I've just had a play, and we're deciding if the current record can be edited based upon if the current record is for a power user. The test should be for if I am a power user.

@TahaKhanAbdalli
Copy link
Collaborator Author

Oky.

@IanMayo
Copy link
Contributor

IanMayo commented Jan 15, 2024

Aah, I just changed my role from rco-power-user to rco-user. I've inspected the database, and I see that the role has changed (good) but my password has been erased (bad).

When I log out, I cannot log back in. Well, I can log back in - but I have to go through the password reset process (20300/20300).

I don't think we should erase the password when the role is changed.

@TahaKhanAbdalli
Copy link
Collaborator Author

Hi Ian,
First of all, sorry for the inconvenience. I am currently working on issue 979, which is the same as the one you are experiencing right now. I will try my best to fix it as soon as possible.
Thank you.

@IanMayo IanMayo temporarily deployed to rco-review-pr-984 January 17, 2024 11:04 Inactive
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@IanMayo
Copy link
Contributor

IanMayo commented Jan 17, 2024

Could you please double-check where we are with this issue @TahaKhanAbdalli ?

@IanMayo IanMayo changed the title plain user can't change his role. 978 plain user can't change his role. Jan 17, 2024
@TahaKhanAbdalli
Copy link
Collaborator Author

Sure.

@TahaKhanAbdalli
Copy link
Collaborator Author

The issue has been addressed. Now, if a user is an RCO Power User they have the ability to edit both their own profile and the profiles of other non RCO Power users. For users who are not RCO Power Users, they are restricted from editing any user profile, including their own. The edit button is disabled for ordinary users, and attempting to manually access the edit form is also restricted.

@TahaKhanAbdalli
Copy link
Collaborator Author

Approach changed to fix this issue in PR #988, therefore closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ordinary users should not be able to change role
2 participants