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

AUT-2: Reset Password doesn't work against latest versions of 2.6.x C… #3

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

mogoodrich
Copy link
Member

…ore and higher

@mogoodrich mogoodrich requested a review from mseaton November 8, 2024 20:24
@@ -38,6 +38,7 @@ public void reset(@RequestParam(value = "username") String username,
try {
Context.addProxyPrivilege(PrivilegeConstants.GET_USERS);
Context.addProxyPrivilege(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES);
Context.addProxyPrivilege(PrivilegeConstants.EDIT_USER_PASSWORDS);
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if there are any security concerns here? I'm assuming not?

Copy link
Member

@mseaton mseaton left a comment

Choose a reason for hiding this comment

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

Approving, though we might want to raise the broader issue at some point that we shouldn't need to have to elevate privileges to allow a user to self-service their account, and maybe whatever was done to core was too broad of a stroke.

@mogoodrich
Copy link
Member Author

Approving, though we might want to raise the broader issue at some point that we shouldn't need to have to elevate privileges to allow a user to self-service their account, and maybe whatever was done to core was too broad of a stroke.

FWIW, the addtion was just annotating this method with a privilege... haven't thought it through, but it does seem reasonable that this method should annotated with a privilege

https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/UserService.java#L576

@mogoodrich mogoodrich merged commit 9a6cdc1 into main Nov 8, 2024
1 check passed
@mogoodrich mogoodrich deleted the AUT-2 branch November 8, 2024 21:26
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