Skip to content
This repository has been archived by the owner on Mar 20, 2024. It is now read-only.

Permissions checks broken on master #150

Closed
pietroalbini opened this issue Mar 19, 2018 · 3 comments
Closed

Permissions checks broken on master #150

pietroalbini opened this issue Mar 19, 2018 · 3 comments

Comments

@pietroalbini
Copy link

After updating bors to the latest master in the rust-lang/rust repository, we saw a few problems:

  • Users in the reviewers list could post comments without any problem
  • Users in the try list received a "you're not a reviewer" message
  • Normal users received both "you're not a reviewer" and "you can't use try" messages

I think the cause of that is #149. Before that PR, the verify_auth calls were "configured" at the start of process_command and then called when they were actually needed. Instead, now the verify_auth calls are directly executed at the start of process_command, which means bors check if the user is a reviewer even if they send a simple comment without any command.

The simple solution to this issue is to reintroduce functools.partial in auth checks.

cc @alexrs @jdm @alexcrichton

@alexrs
Copy link

alexrs commented Mar 20, 2018

@pietroalbini Oh, my bad. Thanks for pointing the problem out. I'll create a PR reintroducing the functools.partial in auth checks and I'll try to create some tests to avoid this problem in the future.
We're working on refactoring Homu to make it testable (unfortunately, with the current code, it's really hard to test), so please, if you detect any other problem, report it!

@alexrs
Copy link

alexrs commented Mar 20, 2018

@pietroalbini I have created this PR that should solve the problem. #151

bors-servo pushed a commit that referenced this issue Mar 20, 2018
Fix auth in parse_commands

As discussed in #150, the auth shouldn't' be done at the beginning of `parse_commands`, but when some action is going to be executed. This PR reintroduces `functools.partial` to fix the issue.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/homu/151)
<!-- Reviewable:end -->
@pietroalbini
Copy link
Author

Thank you so much!

yshui pushed a commit to hadeaninc/homu that referenced this issue Apr 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants