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.

Sign up for GitHub

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

Jump to bottom

Fixed issue [security] #19118: Improper permission management on bulk actions #3580

Merged
merged 3 commits into from
Nov 6, 2023
Merged

Fixed issue [security] #19118: Improper permission management on bulk actions #3580

merged 3 commits into from
Nov 6, 2023

Conversation

Shnoulle
Copy link
Collaborator

Dev: fix send reset email : $userManager->canEdit()
Dev: fix Permission userManager->canAssignPermissions()
Dev: fix roles : Permission::model()->hasGlobalPermission('superadmin', 'create')
Dev: no need Permission on group : already done

@Shnoulle
Fixed issue [security] #19118: Improper permission management on bulk…
2b5ef9b 
… actions

Dev: fix send reset email : $userManager->canEdit()
Dev: fix Permission userManager->canAssignPermissions()
Dev: fix roles : Permission::model()->hasGlobalPermission('superadmin', 'create')
Dev: no need Permission on group : already done
@Shnoulle Shnoulle requested review from Trischi80 and gabrieljenik and removed request for Trischi80 October 27, 2023 15:19
@Shnoulle Shnoulle mentioned this pull request Oct 27, 2023
Merged
gabrieljenik
gabrieljenik approved these changes Oct 30, 2023
View reviewed changes
Copy link
Collaborator

@gabrieljenik gabrieljenik left a comment

Choose a reason for hiding this comment

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

Code structure looks OK.
Left a comment on something which could be nothing or it may need amendment.

@gabrieljenik gabrieljenik added Code review done Version checked for code issue without testing and removed Needs code review labels Oct 30, 2023
@Shnoulle
Dev: more 403 and 494
890fd29 
Dev: fix returned array
gabrieljenik
gabrieljenik approved these changes Oct 31, 2023
View reviewed changes
Copy link
Collaborator

@gabrieljenik gabrieljenik left a comment

Choose a reason for hiding this comment

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

Code structure looks good.

tiborpacalat
tiborpacalat requested changes Nov 3, 2023
View reviewed changes
Copy link
Collaborator

@tiborpacalat tiborpacalat left a comment

Choose a reason for hiding this comment

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

Can you please change the sentence to what I suggested? Thanks

application/controllers/UserManagementController.php Outdated Show resolved Hide resolved
@tiborpacalat tiborpacalat added Tested OK This PR has been tested by QA and works as expected and removed Needs testing labels Nov 6, 2023
@tiborpacalat tiborpacalat merged commit fbd4724 into master Nov 6, 2023
16 of 20 checks passed
@tiborpacalat tiborpacalat deleted the bug/13118_bulkActionUser branch November 6, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabrieljenik gabrieljenik gabrieljenik approved these changes

@tiborpacalat tiborpacalat Awaiting requested review from tiborpacalat

Assignees
No one assigned
Labels
Code review done Version checked for code issue without testing Tested OK This PR has been tested by QA and works as expected
Projects
None yet
Milestone
No milestone
3 participants
@Shnoulle @gabrieljenik @tiborpacalat