Handler access checks in SOAP API #518
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The mc_issue_add() API was missing a check to validate that specified handler has the appropriate access level. Fixes #16993
The mc_issue_update() wasn't check if the handler has access level to handle issues.
Users who report an issue with handler set must have the access level required to assign issues. Issue #16993
vboctor
changed the title
| if( ( $t_handler_id != 0 ) && !user_exists( $t_handler_id ) ) { | ||
| return SoapObjectsFactory::newSoapFault( 'Client', 'User \'' . $t_handler_id . '\' does not exist.' ); | ||
| if( $t_handler_id != 0 ) { | ||
| if( !access_has_project_level( config_get( 'update_bug_assign_threshold' ), $t_project_id, $t_user_id ) ) { |
There was a problem hiding this comment.
Maybe it's worth extracting the validation code in a separate function.
+1 otherwise
Disclose the handler id only if the user has the appropriate access level to see such information. Issue #16993
Move the check to a shared method used by mc_issue_add() and mc_issue_update(). Issue #16993
|
@rombert I've applied the refactoring and also added a missing check to mc_issue_get() for user having access to view the user assigned the issue. |
|
@vboctor - if I read the core correctly, mci_issue_handler_access_check, potentiall returns a soap_fault. Should the calling methods check for that and return the soap_fault if applicable? It does not trigger an error or throw an exception so it does not affect the flow control. |
Now that access check is done in its own function, the caller need to propagate the soap faults since we are not using real exceptions (unfortunately!).
|
@rombert I fixed the check for soap. My mind switched to exceptions mode :) |
|
@vboctor :-) +1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The mc_issue_add() and mc_issue_update() APIs were missing checks to validate that specified handler has the appropriate access level and that logged in user have access level to assign issues.
mc_issue_get() was also missing the check that the user has access to view the handler assigned the issue.
I suggest that we port this fix to master-1.2.x to make it in 1.2.18.
Fixes #16993