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
rpc: harden closeIssue access checks/validation #287
Conversation
also, i think i found bug: // FIXME: this doesn't validate that the status belongs to $issue_id's project
$status_id = Status::getStatusID($new_status);
if (!$status_id) {
throw new RemoteApiException("Invalid status: $new_status");
} how do you suggest to fix this (if at all) is there already method for that? |
2dc60d0
to
fb733be
Compare
@balsdorf ping, also have you finished your parts for 3.3.0 release? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I've added one specific comment about the bug you reported and once that is fixed this looks good to merge.
src/RPC/RemoteApi.php
Outdated
$status_id = Status::getStatusID($new_status); | ||
if (!$status_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this I'd change it to
if (!$status_id || !in_array(Issue::getProjectID($issue_id), Status::getAssociatedProjects($status_id)) {
which will take care of checking if the status belongs to the project.
Sorry I totally missed this. Review submitted. I don't think I had anything else for 3.3.0, just merging the attachment changes. If you know of anything, let me know and I'll knock it out. |
documentation update that 5.6 is now required version. this what i had in mind. |
fb733be
to
50e7f57
Compare
@glensc I've updated the docs to say that only 5.6 is supported now. |
similarly to #280, validate parameters at server side.
i didn't bother to think much, just added similar checks as #280 has:
also checks if issue is not already closed: