From e1ea45c940a0453c916b22fbc946ac2bb0a00498 Mon Sep 17 00:00:00 2001 From: Adam Wight Date: Wed, 7 Feb 2018 16:44:48 -0500 Subject: [PATCH 1/4] Make centralauth work, write tests --- .gitignore | 8 +- jade/centralauth.py | 30 ++-- jade/errors.py | 41 ++++++ jade/tests/__init__.py | 0 jade/tests/test_centralauth.py | 249 +++++++++++++++++++++++++++++++++ jade/tests/util.py | 25 ++++ requirements.txt | 1 + test-requirements.txt | 1 + 8 files changed, 345 insertions(+), 10 deletions(-) create mode 100644 jade/tests/__init__.py create mode 100644 jade/tests/test_centralauth.py create mode 100644 jade/tests/util.py create mode 100644 test-requirements.txt diff --git a/.gitignore b/.gitignore index e430a4d..39f681f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,7 @@ -*.pyc -__pycache__ +/.cache +/.coverage +/htmlcov /*.egg-info +__pycache__ +.*.sw? +*.pyc diff --git a/jade/centralauth.py b/jade/centralauth.py index 9eb1016..639b73a 100644 --- a/jade/centralauth.py +++ b/jade/centralauth.py @@ -9,10 +9,13 @@ ''' # noqa: E501 import mwapi +from . import about from . import errors +USER_AGENT = "{} -- {}".format(about.__name__, about.__author_email__) -class CentralAuth: + +class CentralAuth(object): def __init__(self, ca_session): self.ca_session = ca_session @@ -31,8 +34,7 @@ def get_globaluser_info(self, gu_id): return gui_doc def get_localuser_info(self, name, context_doc): - session = mwapi.Session( - context_doc['url'], self.ca_session.user_agent) + session = mwapi.Session(context_doc['url'], user_agent = USER_AGENT) doc = session.get( action='query', list='users', ususers={name}, usprop={'groups'}) lui_doc = doc['query']['users'][0] @@ -45,8 +47,7 @@ def check_user_rights(self, gu_id, context, requirements): gui_doc = self.get_globaluser_info(gu_id) if 'locked' in gui_doc: - raise errors.UserLockedError( - "The user account with gu_id={0} is locked".format(gu_id)) + raise errors.UserLockedError(gu_id) # Check for local context if context in gui_doc['merged']: @@ -54,8 +55,10 @@ def check_user_rights(self, gu_id, context, requirements): if 'blocked' in context_doc: raise errors.UserBlockedError( - "the user account with gu_id={0} is blocked on {1}: {2}" - .format(gu_id, context, context_doc['blocked'])) + gu_id, + context, + context_doc['blocked']['expiry'], + context_doc['blocked']['reason']) lui_doc = self.get_localuser_info(gui_doc['name'], context_doc) local_groups = lui_doc.get('groups', []) or [] @@ -66,4 +69,15 @@ def check_user_rights(self, gu_id, context, requirements): @classmethod def from_config(cls, config): - return cls(config['centralauth']) + # Extract config. + session_config = config['centralauth'].copy() + if 'user_agent' not in session_config: + session_config['user_agent'] = USER_AGENT + # Pull host out as it's not a keyword arg. + host = session_config['host'] + del(session_config['host']) + + # Create session based on config. + ca_session = mwapi.Session(host, **session_config) + + return cls(ca_session) diff --git a/jade/errors.py b/jade/errors.py index 369ba24..403cd5d 100644 --- a/jade/errors.py +++ b/jade/errors.py @@ -91,6 +91,34 @@ class UserPermissionError(RequestError): HTTP_CODE = 403 +class GlobalUserExistenceError(UserPermissionError): + "This user is globally missing." + SUBTYPE = "user_missing_global" + HTTP_CODE = 403 + + def __init__(self, gu_id): + super().__init__() + self.gu_id = gu_id + + def format_detail(self): + return {'gu_id': self.gu_id} + + +class LocalUserExistenceError(UserPermissionError): + "This user is locally missing." + SUBTYPE = "user_missing_local" + HTTP_CODE = 403 + + def __init__(self, name, context): + super().__init__() + self.name = name + self.context = context + + def format_detail(self): + return {'name': self.name, + 'context': self.context} + + class UserBlockedError(UserPermissionError): "This action cannot be performed because the user is blocked." SUBTYPE = "user_blocked" @@ -110,6 +138,19 @@ def format_detail(self): 'reason': self.reason} +class UserLockedError(UserPermissionError): + "This action cannot be performed because the user is blocked." + SUBTYPE = "user_locked" + HTTP_CODE = 403 + + def __init__(self, gu_id): + super().__init__() + self.gu_id = gu_id + + def format_detail(self): + return {'gu_id': self.gu_id} + + class UserRightsError(UserPermissionError): "This action cannot be performed because the user lacks necessary rights." SUBTYPE = "user_rights" diff --git a/jade/tests/__init__.py b/jade/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/jade/tests/test_centralauth.py b/jade/tests/test_centralauth.py new file mode 100644 index 0000000..0ddbe5b --- /dev/null +++ b/jade/tests/test_centralauth.py @@ -0,0 +1,249 @@ +import mwapi +import pytest + +from jade.centralauth import CentralAuth +from jade import errors +from . import util + +test_config = { + 'centralauth': { + # Global info host can be arbitrary? + 'host': 'https://www.mediawiki.org', + }, +} + +# TODO: Made this up cos I haven't found a missing user yet. +missing_gui_response = { + 'batchcomplete': '', + 'query': { + 'globaluserinfo': { + 'missing': '', + } + } +} + +enwiki_blocked_gui_response = { + 'batchcomplete': '', + 'query': { + 'globaluserinfo': { + 'home': 'enwiki', + 'id': 19729909, + 'registration': '2013-04-14T06:48:25Z', + 'name': 'FUCKYOU', + 'groups': [], + 'rights': [], + 'merged': [ + { + 'wiki': 'enwiki', + 'url': 'https://en.wikipedia.org', + 'timestamp': '2013-04-14T06:48:25Z', + 'method': 'primary', + 'editcount': 0, + 'registration': '2006-03-26T17:01:06Z', + 'blocked': { + 'expiry': 'infinity', + 'reason': '{{username}}' + }, + }, + ] + } + } +} + +locked_gui_response = { + 'batchcomplete': '', + 'query': { + 'globaluserinfo': { + 'home': 'metawiki', + 'id': 33085348, + 'registration': '2014-08-17T23:22:31Z', + 'name': 'Jalexander', + 'locked': '', + 'groups': [], + 'rights': [], + 'merged': [ + { + 'wiki': 'commonswiki', + 'url': 'https://commons.wikimedia.org', + 'timestamp': '2017-12-12T19:44:56Z', + 'method': 'login', + 'editcount': 0, + 'registration': '2017-12-12T19:44:56Z', + }, + { + 'wiki': 'loginwiki', + 'url': 'https://login.wikimedia.org', + 'timestamp': '2014-08-17T23:22:33Z', + 'method': 'login', + 'editcount': 0, + 'registration': '2014-08-17T23:22:33Z', + }, + { + 'wiki': 'metawiki', + 'url': 'https://meta.wikimedia.org', + 'timestamp': '2014-08-17T23:22:31Z', + 'method': 'new', + 'editcount': 0, + 'registration': '2014-08-17T23:22:31Z', + }, + { + 'wiki': 'nowikimedia', + 'url': 'https://no.wikimedia.org', + 'timestamp': '2017-12-13T09:53:07Z', + 'method': 'login', + 'editcount': 0, + 'registration': '2017-12-13T09:53:07Z', + } + ] + } + } +} + +autopatrolled_metawiki_gui_response = { + "batchcomplete": "", + "query": { + "globaluserinfo": { + "home": "enwiki", + "id": 15622560, + "registration": "2012-12-21T22:37:22Z", + "name": "Awight (WMF)", + "groups": [ + "oathauth-tester", + "wmf-researcher" + ], + "merged": [ + { + "wiki": "metawiki", + "url": "https://meta.wikimedia.org", + "timestamp": "2012-12-21T23:59:21Z", + "method": "login", + "editcount": 222, + "registration": "2012-12-21T23:59:21Z", + "groups": [ + "autopatrolled" + ] + }, + { + "wiki": "ptwiki", + "url": "https://pt.wikipedia.org", + "timestamp": "2013-04-09T01:26:35Z", + "method": "login", + "editcount": 0, + "registration": "2013-04-09T01:26:35Z" + }, + ], + "unattached": [] + } + } +} + +autopatrolled_metawiki_lui_response = { + 'batchcomplete': '', + 'query': { + 'users': [{ + 'userid': 2043420, + 'name': 'Awight (WMF)', + 'groups': ['autopatrolled', '*', 'user', 'autoconfirmed'] + }] + } +} + +no_autopatrolled_metawiki_lui_response = { + 'batchcomplete': '', + 'query': { + 'users': [{ + 'userid': 2043420, + 'name': 'Awight (WMF)', + 'groups': ['*', 'user', 'autoconfirmed'] + }] + } +} + + +def test_from_config(): + ca = CentralAuth.from_config(test_config) + assert isinstance(ca, CentralAuth) + + +# TODO: We can't test this using the current mocking strategy, because the +# error comes from within .get +# def test_get_globaluser_info_invalid_user_id(): +# ca = CentralAuth.from_config(test_config) +# with pytest.raises(mwapi.errors.APIError): +# ca.get_globaluser_info(0) + + +def test_get_globaluser_info_missing_user(): + ca = CentralAuth.from_config(test_config) + + with util.mock_mwapi_get(missing_gui_response): + with pytest.raises(errors.GlobalUserExistenceError) as exc_info: + gui_doc = ca.get_globaluser_info(12345) + + assert exc_info.value.gu_id == 12345 + + +def test_get_globaluser_info(): + ca = CentralAuth.from_config(test_config) + + with util.mock_mwapi_get(enwiki_blocked_gui_response): + gui_doc = ca.get_globaluser_info(19729909) + + assert gui_doc['home'] == 'enwiki' + assert gui_doc['name'] == 'FUCKYOU' + + +def test_check_user_rights_blocked_in_context(): + ca = CentralAuth.from_config(test_config) + with util.mock_mwapi_get(enwiki_blocked_gui_response): + with pytest.raises(errors.UserBlockedError) as exc_info: + ca.check_user_rights(19729909, 'enwiki', ['autopatrolled']) + + assert exc_info.value.gu_id == 19729909 + assert exc_info.value.context == 'enwiki' + assert exc_info.value.expiry == 'infinity' + assert exc_info.value.reason == '{{username}}' + + +def test_check_user_rights_blocked_other_context(): + ca = CentralAuth.from_config(test_config) + with util.mock_mwapi_get(enwiki_blocked_gui_response): + ca.check_user_rights(19729909, 'frwiki', ['autopatrolled']) + + assert True + + +def test_check_user_rights_locked(): + ca = CentralAuth.from_config(test_config) + #with mock.patch('mwapi.session.get') as mock_mwapi: + # mock_mwapi.side_effect = [ + # enwiki_blocked_gui_response, + # enwiki_blocked_lui_response, + # ] + with util.mock_mwapi_get(locked_gui_response): + with pytest.raises(errors.UserLockedError) as exc_info: + ca.check_user_rights(33085348, 'enwiki', ['autopatrolled']) + + assert exc_info.value.gu_id == 33085348 + + +def test_check_user_rights_has_autopatrolled(): + ca = CentralAuth.from_config(test_config) + with util.mock_mwapi_get([autopatrolled_metawiki_gui_response, + autopatrolled_metawiki_lui_response]): + ca.check_user_rights(15622560, 'metawiki', ['autopatrolled']) + + assert True + + +def test_check_user_rights_lacks_autopatrolled(): + ca = CentralAuth.from_config(test_config) + with util.mock_mwapi_get([autopatrolled_metawiki_gui_response, + no_autopatrolled_metawiki_lui_response]): + with pytest.raises(errors.UserRightsError) as exc_info: + ca.check_user_rights(15622560, 'metawiki', ['autopatrolled']) + + assert exc_info.value.gu_id == 15622560 + assert exc_info.value.context == 'metawiki' + assert exc_info.value.required == ['autopatrolled'] + assert exc_info.value.user_groups == ['*', 'user', 'autoconfirmed'] diff --git a/jade/tests/util.py b/jade/tests/util.py new file mode 100644 index 0000000..f57b818 --- /dev/null +++ b/jade/tests/util.py @@ -0,0 +1,25 @@ +import contextlib +import copy +import mock + + +@contextlib.contextmanager +def mock_mwapi_get(response): + """ + Helper to mock mwapi responses. + + TODO: Handle a series of responses. + """ + # Our functions mutate their inputs, so make a copy in order to use + # fixtures in multiple tests. + response = copy.deepcopy(response) + with mock.patch('mwapi.Session.get') as mock_get: + if isinstance(response, dict): + # Mocking a single API call. + mock_get.return_value = response + else: + # Mocking a series of API calls, `response` is a list. + mock_get.side_effect = response + + # Call the code under test. + yield diff --git a/requirements.txt b/requirements.txt index c2418cc..f243460 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,5 +5,6 @@ docopt_subcommands flask_swaggerui Flask >= 0.12.2, < 0.12.999 jwt +kafka-python mwapi yamlconf diff --git a/test-requirements.txt b/test-requirements.txt new file mode 100644 index 0000000..932a895 --- /dev/null +++ b/test-requirements.txt @@ -0,0 +1 @@ +mock From 7c5c71eb453e13d64237b499f3cb1694bf67546a Mon Sep 17 00:00:00 2001 From: Adam Wight Date: Thu, 8 Feb 2018 10:58:40 -0500 Subject: [PATCH 2/4] Remove Python 2 polyfill and syntax --- jade/centralauth.py | 2 +- jade/tests/util.py | 2 +- test-requirements.txt | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/jade/centralauth.py b/jade/centralauth.py index 639b73a..461f5cf 100644 --- a/jade/centralauth.py +++ b/jade/centralauth.py @@ -15,7 +15,7 @@ USER_AGENT = "{} -- {}".format(about.__name__, about.__author_email__) -class CentralAuth(object): +class CentralAuth: def __init__(self, ca_session): self.ca_session = ca_session diff --git a/jade/tests/util.py b/jade/tests/util.py index f57b818..428c4ae 100644 --- a/jade/tests/util.py +++ b/jade/tests/util.py @@ -1,6 +1,6 @@ import contextlib import copy -import mock +from unittest import mock @contextlib.contextmanager diff --git a/test-requirements.txt b/test-requirements.txt index 932a895..e69de29 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1 +0,0 @@ -mock From 172d10629a0dd20cc4fc82090a37606960a499b1 Mon Sep 17 00:00:00 2001 From: Adam Wight Date: Thu, 8 Feb 2018 11:04:40 -0500 Subject: [PATCH 3/4] Cover last code path with a test --- jade/tests/test_centralauth.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/jade/tests/test_centralauth.py b/jade/tests/test_centralauth.py index 0ddbe5b..9469e82 100644 --- a/jade/tests/test_centralauth.py +++ b/jade/tests/test_centralauth.py @@ -22,6 +22,16 @@ } } +missing_lui_response = { + "batchcomplete": "", + "query": { + "users": [{ + "name": "Example", + "missing": "" + }] + } +} + enwiki_blocked_gui_response = { 'batchcomplete': '', 'query': { @@ -247,3 +257,14 @@ def test_check_user_rights_lacks_autopatrolled(): assert exc_info.value.context == 'metawiki' assert exc_info.value.required == ['autopatrolled'] assert exc_info.value.user_groups == ['*', 'user', 'autoconfirmed'] + + +def test_check_user_rights_local_missing(): + ca = CentralAuth.from_config(test_config) + with util.mock_mwapi_get([autopatrolled_metawiki_gui_response, + missing_lui_response]): + with pytest.raises(errors.LocalUserExistenceError) as exc_info: + ca.check_user_rights(15622560, 'metawiki', ['autopatrolled']) + + assert exc_info.value.name == 'Awight (WMF)' + assert exc_info.value.context == 'metawiki' From ea572c52ea0fd35b2623d9c8958c0efc69164239 Mon Sep 17 00:00:00 2001 From: Adam Wight Date: Thu, 8 Feb 2018 12:32:25 -0500 Subject: [PATCH 4/4] Clean up and lint --- jade/centralauth.py | 2 +- jade/tests/test_centralauth.py | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/jade/centralauth.py b/jade/centralauth.py index 461f5cf..a725af0 100644 --- a/jade/centralauth.py +++ b/jade/centralauth.py @@ -34,7 +34,7 @@ def get_globaluser_info(self, gu_id): return gui_doc def get_localuser_info(self, name, context_doc): - session = mwapi.Session(context_doc['url'], user_agent = USER_AGENT) + session = mwapi.Session(context_doc['url'], user_agent=USER_AGENT) doc = session.get( action='query', list='users', ususers={name}, usprop={'groups'}) lui_doc = doc['query']['users'][0] diff --git a/jade/tests/test_centralauth.py b/jade/tests/test_centralauth.py index 9469e82..09dfbb8 100644 --- a/jade/tests/test_centralauth.py +++ b/jade/tests/test_centralauth.py @@ -1,4 +1,3 @@ -import mwapi import pytest from jade.centralauth import CentralAuth @@ -185,17 +184,15 @@ def test_from_config(): def test_get_globaluser_info_missing_user(): ca = CentralAuth.from_config(test_config) - with util.mock_mwapi_get(missing_gui_response): with pytest.raises(errors.GlobalUserExistenceError) as exc_info: - gui_doc = ca.get_globaluser_info(12345) + ca.get_globaluser_info(12345) assert exc_info.value.gu_id == 12345 def test_get_globaluser_info(): ca = CentralAuth.from_config(test_config) - with util.mock_mwapi_get(enwiki_blocked_gui_response): gui_doc = ca.get_globaluser_info(19729909) @@ -225,11 +222,6 @@ def test_check_user_rights_blocked_other_context(): def test_check_user_rights_locked(): ca = CentralAuth.from_config(test_config) - #with mock.patch('mwapi.session.get') as mock_mwapi: - # mock_mwapi.side_effect = [ - # enwiki_blocked_gui_response, - # enwiki_blocked_lui_response, - # ] with util.mock_mwapi_get(locked_gui_response): with pytest.raises(errors.UserLockedError) as exc_info: ca.check_user_rights(33085348, 'enwiki', ['autopatrolled'])