Skip to content

Commit 0b07f65

Browse files
committed
Test authorization code
Pull code that creates a comment on a pull request out of the authorization check, and add tests around the authorization checks. After pulling out the code that creates a comment, we still need to know what the text of the comment will be, so change from a function that returns a True on success and a False on failure to a function that returns a True on success and raises an Exception (with the failure comment) on failure.
1 parent c10e90e commit 0b07f65

File tree

5 files changed

+749
-268
lines changed

5 files changed

+749
-268
lines changed

.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ install:
77
- pip install flake8
88
script:
99
- flake8 homu
10-
- pip install -e .
10+
- pip install -e .[test]
1111
- python setup.py test

homu/auth.py

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,29 @@
11
import requests
2+
from enum import IntEnum
23

34

45
RUST_TEAM_BASE = "https://team-api.infra.rust-lang.org/v1/"
56

67

8+
class AuthorizationException(Exception):
9+
"""
10+
The exception thrown when a user is not authorized to perform an action
11+
"""
12+
13+
comment = None
14+
15+
def __init__(self, message, comment):
16+
super().__init__(message)
17+
self.comment = comment
18+
19+
20+
class AuthState(IntEnum):
21+
# Higher is more privileged
22+
REVIEWER = 3
23+
TRY = 2
24+
NONE = 1
25+
26+
727
def fetch_rust_team(repo_label, level):
828
repo = repo_label.replace('-', '_')
929
url = RUST_TEAM_BASE + "permissions/bors." + repo + "." + level + ".json"
@@ -31,18 +51,14 @@ def verify_level(username, repo_label, repo_cfg, state, toml_keys,
3151
return authorized
3252

3353

34-
def verify(username, repo_label, repo_cfg, state, auth, realtime, my_username):
35-
# The import is inside the function to prevent circular imports: main.py
36-
# requires auth.py and auth.py requires main.py
37-
from .main import AuthState
38-
54+
def assert_authorized(username, repo_label, repo_cfg, state, auth, botname):
3955
# In some cases (e.g. non-fully-qualified r+) we recursively talk to
4056
# ourself via a hidden markdown comment in the message. This is so that
4157
# when re-synchronizing after shutdown we can parse these comments and
4258
# still know the SHA for the approval.
4359
#
4460
# So comments from self should always be allowed
45-
if username == my_username:
61+
if username == botname:
4662
return True
4763

4864
authorized = False
@@ -59,14 +75,13 @@ def verify(username, repo_label, repo_cfg, state, auth, realtime, my_username):
5975
if authorized:
6076
return True
6177
else:
62-
if realtime:
63-
reply = '@{}: :key: Insufficient privileges: '.format(username)
64-
if auth == AuthState.REVIEWER:
65-
if repo_cfg.get('auth_collaborators', False):
66-
reply += 'Collaborator required'
67-
else:
68-
reply += 'Not in reviewers'
69-
elif auth == AuthState.TRY:
70-
reply += 'not in try users'
71-
state.add_comment(reply)
72-
return False
78+
reply = '@{}: :key: Insufficient privileges: '.format(username)
79+
if auth == AuthState.REVIEWER:
80+
if repo_cfg.get('auth_collaborators', False):
81+
reply += 'Collaborator required'
82+
else:
83+
reply += 'Not in reviewers'
84+
elif auth == AuthState.TRY:
85+
reply += 'not in try users'
86+
raise AuthorizationException(
87+
'Authorization failed for user {}'.format(username), reply)

0 commit comments

Comments
 (0)