diff --git a/pagure/utils.py b/pagure/utils.py index d575df8..5637bcd 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -86,33 +86,58 @@ def check_api_acls(acls, optional=False): token_str = authorization.split("token", 1)[1].strip() token_auth = False + error_msg = None if token_str: token = pagure.lib.query.get_api_token(flask.g.session, token_str) - if token and not token.expired: - flask.g.authenticated = True - if acls and set(token.acls_list).intersection(set(acls)): - token_auth = True - flask.g.fas_user = token.user - # To get a token, in the `fas` auth user must have signed - # the CLA, so just set it to True - flask.g.fas_user.cla_done = True - flask.g.token = token - flask.g.authenticated = True - elif not acls and optional: - token_auth = True - flask.g.fas_user = token.user - # To get a token, in the `fas` auth user must have signed - # the CLA, so just set it to True - flask.g.fas_user.cla_done = True - flask.g.token = token + if token: + if token.expired: + error_msg = "Expired token" + else: flask.g.authenticated = True + + # Some ACLs are required + if acls: + token_acls_set = set(token.acls_list) + needed_acls_set = set(acls or []) + overlap = token_acls_set.intersection(needed_acls_set) + # Our token has some of the required ACLs: auth successful + if overlap: + token_auth = True + flask.g.fas_user = token.user + # To get a token, in the `fas` auth user must have + # signed the CLA, so just set it to True + flask.g.fas_user.cla_done = True + flask.g.token = token + flask.g.authenticated = True + # Our token has none of the required ACLs -> auth fail + else: + error_msg = "Missing ACLs: %s" % ", ".join( + sorted(set(acls) - set(token.acls_list)) + ) + # No ACL required + else: + if optional: + token_auth = True + flask.g.fas_user = token.user + # To get a token, in the `fas` auth user must have + # signed the CLA, so just set it to True + flask.g.fas_user.cla_done = True + flask.g.token = token + flask.g.authenticated = True + else: + error_msg = "Invalid token" + elif optional: return + else: + error_msg = "Invalid token" + if not token_auth: output = { "error_code": pagure.api.APIERROR.EINVALIDTOK.name, "error": pagure.api.APIERROR.EINVALIDTOK.value, + "errors": error_msg, } jsonout = flask.jsonify(output) jsonout.status_code = 401 diff --git a/tests/test_pagure_flask_api.py b/tests/test_pagure_flask_api.py index 301e3f0..8daea1d 100644 --- a/tests/test_pagure_flask_api.py +++ b/tests/test_pagure_flask_api.py @@ -215,7 +215,8 @@ class PagureFlaskApitests(tests.SimplePagureTest): u'error': u'Invalid or expired token. Please visit ' 'http://localhost.localdomain/settings#api-keys to get or ' 'renew your API token.', - u'error_code': u'EINVALIDTOK' + u'error_code': u'EINVALIDTOK', + u'errors': 'Invalid token', } ) diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index b3f0d8a..92c6440 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -2490,11 +2490,12 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): output = self.app.get('/api/0/test/issue/2', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), ['error', 'error_code', 'errors']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual(data['errors'], 'Invalid token') # Create a new token for another user item = pagure.lib.model.Token( @@ -3047,11 +3048,12 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): '/api/0/foo/issue/1/milestone', data={}, headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), ['error', 'error_code', 'errors']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual(data['errors'], 'Invalid token') @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) @patch( @@ -3561,11 +3563,13 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): '/api/0/foo/issue/1/assign', data=data, headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), ['error', 'error_code', 'errors']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual( + data['errors'], 'Missing ACLs: issue_assign, issue_update') # No comment added repo = pagure.lib.query.get_authorized_project(self.session, 'foo') @@ -3938,11 +3942,12 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): '/api/0/test/issue/1/custom/bugzilla', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), ['error', 'error_code', 'errors']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual(data['errors'], 'Invalid token') headers = {'Authorization': 'token aaabbbcccddd'} diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 80cea7b..704a2e2 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -2112,11 +2112,14 @@ class PagureFlaskApiProjecttests(tests.Modeltests): output = self.app.post('/api/0/new', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), [ + 'error', 'error_code', "errors"]) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual( + data['errors'], "Missing ACLs: create_project") headers = {'Authorization': 'token aaabbbcccddd'} @@ -2353,11 +2356,14 @@ class PagureFlaskApiProjecttests(tests.Modeltests): output = self.app.post('/api/0/new', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), [ + 'error', 'error_code', "errors"]) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual( + data['errors'], "Missing ACLs: create_project") headers = {'Authorization': 'token aaabbbcccddd'} @@ -2529,11 +2535,14 @@ class PagureFlaskApiProjecttests(tests.Modeltests): output = self.app.post('/api/0/fork', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), [ + 'error', 'error_code', "errors"]) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual( + data['errors'], "Missing ACLs: fork_project") headers = {'Authorization': 'token aaabbbcccddd'} @@ -2652,11 +2661,14 @@ class PagureFlaskApiProjecttests(tests.Modeltests): output = self.app.post('/api/0/fork', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), [ + 'error', 'error_code', "errors"]) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual( + data['errors'], "Missing ACLs: fork_project") headers = {'Authorization': 'token aaabbbcccddd'} @@ -3123,11 +3135,14 @@ class PagureFlaskApiProjectFlagtests(tests.Modeltests): headers=headers, data=data) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), [ + 'error', 'error_code', "errors"]) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual( + data['errors'], "Invalid token") def test_flag_commit_invalid_status(self): """ Test flagging a commit with an invalid status. """ @@ -3986,7 +4001,8 @@ class PagureFlaskApiProjectOptionsTests(tests.Modeltests): u'error': u'Invalid or expired token. Please visit ' 'http://localhost.localdomain/settings#api-keys to get ' 'or renew your API token.', - u'error_code': u'EINVALIDTOK' + u'error_code': u'EINVALIDTOK', + u'errors': u'Invalid token', } ) @@ -4049,7 +4065,8 @@ class PagureFlaskApiProjectOptionsTests(tests.Modeltests): u'error': u'Invalid or expired token. Please visit ' 'http://localhost.localdomain/settings#api-keys to get ' 'or renew your API token.', - u'error_code': u'EINVALIDTOK' + u'error_code': u'EINVALIDTOK', + u'errors': u'Invalid token', } ) diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index 166f678..ead19c4 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -2990,11 +2990,12 @@ class PagurePrivateRepotest(tests.Modeltests): output = self.app.get('/api/0/test4/issue/1', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), ['error', 'error_code', 'errors']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual(data['errors'], 'Invalid token') headers = {'Authorization': 'token aaabbbcccddd'} @@ -3216,11 +3217,12 @@ class PagurePrivateRepotest(tests.Modeltests): output = self.app.post('/api/0/test4/issue/1/comment', headers=headers) self.assertEqual(output.status_code, 401) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual(sorted(data.keys()), ['error', 'error_code', 'errors']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.value, data['error']) self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + self.assertEqual(data['errors'], 'Invalid token') headers = {'Authorization': 'token aaabbbcccddd'} # No input diff --git a/tests/test_pagure_flask_rebase.py b/tests/test_pagure_flask_rebase.py index e43afea..f651d7e 100644 --- a/tests/test_pagure_flask_rebase.py +++ b/tests/test_pagure_flask_rebase.py @@ -283,7 +283,8 @@ class PagureRebasetests(tests.Modeltests): u'error': u'Invalid or expired token. Please visit ' 'http://localhost.localdomain/settings#api-keys to get ' 'or renew your API token.', - u'error_code': u'EINVALIDTOK' + u'error_code': u'EINVALIDTOK', + u'errors': 'Invalid token', } )