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

prevent the last manager from removing manager role #557

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

selvaprakash92
Copy link
Contributor

Type of change

  • Bug fix

@selvaprakash92 selvaprakash92 requested a review from a team as a code owner November 9, 2023 09:16
@dshuffma-ibm dshuffma-ibm changed the title restricting only manager to edit roles prevent the last manager from removing manager role Nov 9, 2023
Copy link
Contributor

@dshuffma-ibm dshuffma-ibm left a comment

Choose a reason for hiding this comment

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

close, see comment on permission_lib.js

{
let admin_count = 0;
for (let user in settings_doc.access_list) {
if (user.roles.includes('manager'))
Copy link
Contributor

Choose a reason for hiding this comment

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

user isn't what you think it is, its just the key of the access_list object, so its just a string. to get to the properies of that user, it should be:

for (let id in settings_doc.access_list) {
	let user = settings_doc.access_list[id]; 
	if (user && user.roles && user.roles.includes('manager')) {
			admin_count = admin_count + 1;
	}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dshuffma-ibm , I have updated the code as mentioned

Signed-off-by: Prakash P S <[email protected]>
Copy link
Contributor

@dshuffma-ibm dshuffma-ibm left a comment

Choose a reason for hiding this comment

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

+1

@dshuffma-ibm dshuffma-ibm merged commit 2fb5037 into hyperledger-labs:main Nov 30, 2023
17 checks passed
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.

2 participants