From 0ffdc70ceaed8dbf395eaf66bf86fe48e946bdc4 Mon Sep 17 00:00:00 2001 From: skrzepto Date: Jun 17 2016 15:00:02 +0000 Subject: increasing unittests, kindof figured out how to test redirection and pep8 the files adding owner of hub editing hub with no data hub edit succesful adding another test for hubs/edit for invalid data pep8 the file pep8 files and added a new function to see if flask.g.auth is set made authenticated function check if flask.g.auth is set and check what the logged_in bool is and return it undoing a pep8 to one line, changed the authenticate function to be shorter pep8 the authenticated function --- diff --git a/hubs/app.py b/hubs/app.py index c283490..4b72cee 100755 --- a/hubs/app.py +++ b/hubs/app.py @@ -31,17 +31,17 @@ app = flask.Flask(__name__) # Register some useful global filters. def days_since(then): return (datetime.datetime.utcnow() - then).days + + app.template_filter('days_since')(days_since) app.template_filter('avatar')(username2avatar) logging.basicConfig() - # TODO - put this in config so we can migrate to pagure # TODO - instead of 'develop', use the version from pkg_resources to figure out # the right tag to link people to. AGPL ftw. -SOURCE_URL = 'https://pagure.io/fedora-hubs/blob/develop/f'#/hubs/widgets/badges.py' - +SOURCE_URL = 'https://pagure.io/fedora-hubs/blob/develop/f' # /hubs/widgets/badges.py' app.config.from_object('hubs.default_config') if 'HUBS_CONFIG' in os.environ: @@ -49,6 +49,7 @@ if 'HUBS_CONFIG' in os.environ: import fedmsg.config import fedmsg.meta + fedmsg_config = fedmsg.config.load_config() fedmsg.meta.make_processors(**fedmsg_config) PATHS = fmn.lib.load_rules(root='fmn.rules') @@ -67,9 +68,17 @@ class CustomJSONEncoder(flask.json.JSONEncoder): return list(iterable) return flask.json.JSONEncoder.default(self, o) + app.json_encoder = CustomJSONEncoder +def authenticated(): + """ Utility function checking if the current auth is set or not.""" + return hasattr(flask.g, 'auth') \ + and flask.g.auth is not None \ + and flask.g.auth.logged_in + + @app.template_filter('commas') def commas(numeric): return "{:,.2f}".format(numeric) @@ -83,7 +92,7 @@ def shutdown_session(exception=None): @app.route('/') def index(): - if not flask.g.auth.logged_in: + if not authenticated(): return flask.redirect(flask.url_for('login_fedora')) return flask.redirect(flask.url_for('hub', name=flask.g.auth.nickname)) @@ -91,7 +100,7 @@ def index(): @app.route('/groups') def groups(): - if not flask.g.auth.logged_in: + if not authenticated(): return flask.redirect(flask.url_for('login_fedora')) # Get the list of promoted and non-promoted group hubs from the DB @@ -156,7 +165,7 @@ def hub_edit_post(name): w.strip().replace('widget-', '') for w in flask.request.form.getlist('right_widgets[]') if w.strip() - ] + ] try: r_widget_ids = [int(w) for w in r_widget_ids] except: @@ -166,7 +175,7 @@ def hub_edit_post(name): r_indexes = [ i.strip() for i in flask.request.form.getlist('right_indexes[]') if i.strip() - ] + ] try: r_indexes = [int(i) for i in r_indexes] @@ -187,7 +196,7 @@ def hub_edit_post(name): w.strip().replace('widget-', '') for w in flask.request.form.getlist('left_widgets[]') if w.strip() - ] + ] try: l_widget_ids = [int(w) for w in l_widget_ids] except: @@ -197,7 +206,7 @@ def hub_edit_post(name): l_indexes = [ i.strip() for i in flask.request.form.getlist('left_indexes[]') if i.strip() - ] + ] try: l_indexes = [int(i) for i in l_indexes] @@ -290,7 +299,7 @@ def hub_add_widget_get(name): widget for widget in hubs.widgets.registry if hubs.widgets.registry[widget].position in ['both', side] - ] + ] return flask.render_template( 'add_widget.html', hub=hub, @@ -311,7 +320,7 @@ def hub_add_widget_post(name): hub = get_hub(session, name) widget = widget = hubs.models.Widget( - plugin=widget_name, index=-1, left=position=='left') + plugin=widget_name, index=-1, left=position == 'left') error = False config = {} for arg in widget.module.data.widget_arguments: @@ -350,8 +359,8 @@ def hub_add_widget_post(name): @app.route('//') def widget_render(hub, idx): widget = get_widget(session, hub, idx) - return widget.render(session)#, edit=False) - #was blocking all widgets from working, sorry! + return widget.render(session) # , edit=False) + # was blocking all widgets from working, sorry! @app.route('///json') @@ -454,7 +463,7 @@ oid = OpenID(app, def login(): default = flask.url_for('index') next_url = flask.request.args.get('next', default) - if flask.g.auth.logged_in: + if authenticated(): return flask.redirect(next_url) openid_server = flask.request.form.get('openid', None) @@ -471,8 +480,8 @@ def login(): @app.route('/login/fedora') @oid.loginhandler def login_fedora(): - #default = flask.url_for('profile_redirect') - #next_url = flask.request.args.get('next', default) + # default = flask.url_for('profile_redirect') + # next_url = flask.request.args.get('next', default) return oid.try_login( 'https://id.fedoraproject.org', ask_for=['email', 'fullname', 'nickname'], @@ -509,15 +518,17 @@ def after_openid_login(resp): def login_required(function): """ Flask decorator to restrict access to logged-in users. """ + @functools.wraps(function) def decorated_function(*args, **kwargs): """ Decorated function, actually does the work. """ - if not flask.g.auth.logged_in: + if not authenticated(): flask.flash('Login required', 'errors') return flask.redirect(flask.url_for( 'login_fedora', next=flask.request.url)) return function(*args, **kwargs) + return decorated_function @@ -542,8 +553,8 @@ def check_auth(): def get_hub(session, name): """ Utility shorthand to get a hub and 404 if not found. """ - hub = session.query(hubs.models.Hub)\ - .filter(hubs.models.Hub.name == name)\ + hub = session.query(hubs.models.Hub) \ + .filter(hubs.models.Hub.name == name) \ .first() if not hub: @@ -568,8 +579,8 @@ def get_widget(session, hub, idx): flask.abort(404) -## Here are a bunch of API methods that should probably be broken out into -## their own file +# Here are a bunch of API methods that should probably be broken out into +# their own file @app.route('/api/hub//subscribe', methods=['POST']) @login_required def hub_subscribe(hub): @@ -660,7 +671,7 @@ def markup_fedmsg(): message = json.loads(data) try: nickname = flask.g.auth.nickname - except AttributeError: # Not logged in + except AttributeError: # Not logged in return flask.abort(403) preference = get_remote_preference(nickname, context) if preference: diff --git a/hubs/tests/__init__.py b/hubs/tests/__init__.py index 96379c2..5c10118 100644 --- a/hubs/tests/__init__.py +++ b/hubs/tests/__init__.py @@ -107,7 +107,7 @@ def auth_set(APP, auth): def handler(sender, **kwargs): g.auth = auth if not auth: - munch.Munch(logged_in=False) + g.auth = munch.Munch(logged_in=False) with appcontext_pushed.connected_to(handler, APP): yield diff --git a/hubs/tests/test_fedora_hubs_flask_api.py b/hubs/tests/test_fedora_hubs_flask_api.py index bf79d06..a0043f0 100644 --- a/hubs/tests/test_fedora_hubs_flask_api.py +++ b/hubs/tests/test_fedora_hubs_flask_api.py @@ -4,6 +4,7 @@ from urlparse import urlparse from flask import json from os.path import dirname import vcr +from werkzeug.datastructures import ImmutableMultiDict import hubs from hubs import tests @@ -17,12 +18,10 @@ cassette_dir = dirname(dirname(__file__)) + '/vcr-request-data/' class HubsAPITest(hubs.tests.APPTest): - @unittest.skip("Can't seem to get redirected to the login in these tests") def test_index_logged_out(self): - result = self.app.get('/', follow_redirects=True) - # its trying to redirect to login id.fedoraproject.org/openid - # assert the status code of the response - self.assertEqual(result.status_code, 200) + result = self.app.get('/', follow_redirects=False) + self.assertEqual(result.status_code, 302) + self.assertEqual(urlparse(result.location).path, "/login/fedora") def test_index_logged_in(self): user = tests.FakeAuthorization('ralph') @@ -31,25 +30,27 @@ class HubsAPITest(hubs.tests.APPTest): # its trying to redirect to login id.fedoraproject.org/openid # assert the status code of the response self.assertEqual(result.status_code, 200) - self.assertFalse('Not logged in. Click to login' in result.data) + self.assertFalse('Not logged in. Click to login' in result.data) def test_hub_logged_out(self): - with tests.auth_set(app, None): - # for some reason def check_auth() is not running and i need to set it to none so it won't crash + with app.test_request_context('/ralph'): + # need to manually call the @app.before_request + # since unittest don't call it + hubs.app.check_auth() result = self.app.get('/ralph', follow_redirects=True) # assert the status code of the response self.assertEqual(result.status_code, 200) - self.assertTrue('Not logged in. Click to login' in result.data) + str_expected = 'Not logged in. Click to ' \ + 'login' + self.assertTrue(str_expected in result.data) - @unittest.skip("Can't seem to get redirected to the login in these tests") def test_groups_logged_out(self): - with tests.auth_set(app, None): - # for some reason def check_auth() is not running and i need to set it to none so it won't crash - result = self.app.get('/groups', follow_redirects=True) - # assert the status code of the response - self.assertEqual(result.status_code, 200) - # this will redirect to fedora.login which unittests can't handle atm - pass + result = self.app.get('/groups', follow_redirects=False) + # assert the status code of the response + self.assertEqual(result.status_code, 302) + # this will redirect to fedora.login + self.assertEqual(urlparse(result.location).path, "/login/fedora") def test_groups_logged_in(self): user = tests.FakeAuthorization('ralph') @@ -64,25 +65,26 @@ class HubsAPITest(hubs.tests.APPTest): with tests.auth_set(app, user): result = self.app.get('/ralph', follow_redirects=True) self.assertEqual(result.status_code, 200) - self.assertFalse('Not logged in. Click to login' in result.data) + self.assertFalse('Not logged in. Click to login' in result.data) def test_hub_json(self): - with tests.auth_set(app, None): - # for some reason def check_auth() is not running and i need to set it to none so it won't crash - result = self.app.get('/ralph/json', follow_redirects=True) - # assert the status code of the response - self.assertEqual(result.status_code, 200) - data = { - "avatar": "https://seccdn.libravatar.org/avatar/9c9f7784935381befc302fe3c814f9136e7a33953d0318761669b8643f4df55c?s=312&d=retro", - "left_width": 8, - "members": ["ralph"], - "name": "ralph", - "owners": ["ralph"], - "subscribers": [], - "summary": "Ralph", - "widgets": [31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 51] - } - self.assertDictEqual(data, json.loads(result.data)) + result = self.app.get('/ralph/json', follow_redirects=True) + # assert the status code of the response + self.assertEqual(result.status_code, 200) + data = { + "avatar": "https://seccdn.libravatar.org/avatar/" + "9c9f7784935381befc302fe3c814f9136e7a339" + "53d0318761669b8643f4df55c?s=312&d=retro", + "left_width": 8, + "members": ["ralph"], + "name": "ralph", + "owners": ["ralph"], + "subscribers": [], + "summary": "Ralph", + "widgets": [31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 51] + } + self.assertDictEqual(data, json.loads(result.data)) @unittest.skip("Authorization layer not present yet") def test_hub_edit_get_logged_out(self): @@ -106,7 +108,7 @@ class HubsAPITest(hubs.tests.APPTest): with tests.auth_set(app, user): result = self.app.get('/ralph/edit', follow_redirects=True) self.assertEqual(result.status_code, 200) - # We do not have the authorization layer in place yet so everyone can edit + # We do not have the authorization layer so everyone can edit @unittest.skip("Authorization layer not present yet") def test_hub_edit_post_logged_out(self): @@ -125,20 +127,85 @@ class HubsAPITest(hubs.tests.APPTest): self.assertEqual(result.status_code, 403) # failing right # We do not have the authorization layer in place yet - # WIP: commenting out so I can push this updates to branch so others can contribute - ''' - def test_hub_edit_post_logged_in_owner_no_data(self): + def test_hub_edit_post_logged_in_owner_empty_data(self): user = tests.FakeAuthorization('ralph') with tests.auth_set(app, user): - result = self.app.post('/ralph/edit', follow_redirects=True) + result = self.app.post('/ralph/edit', data={}, + follow_redirects=True) self.assertEqual(result.status_code, 200) - assert 'Invalid widget identifiers submitted' in result.data - assert 'Invalid indexes submitted' in result.data - assert 'The number of indexes and the number of widgets ' \ - 'are not of the same length' in result.data - assert 'Invalid widget identifiers submitted' in result.data - assert 'Invalid indexes submitted' in result.data - ''' + self.assertFalse('Not logged in. Click to login' in result.data) + self.assertTrue('Full Name: ' + 'fullname: ralph' in result.data) + + def test_hub_edit_post_logged_in_owner_valid_data(self): + user = tests.FakeAuthorization('ralph') + with tests.auth_set(app, user): + data = ImmutableMultiDict( + [('right_indexes[]', u'0'), ('right_indexes[]', u'1'), + ('right_indexes[]', u'2'), ('right_indexes[]', u'3'), + ('right_indexes[]', u'4'), ('right_indexes[]', u'5'), + ('right_indexes[]', u'6'), ('right_indexes[]', u'7'), + ('right_indexes[]', u'8'), ('right_widgets[]', u'32'), + ('right_widgets[]', u'33'), ('right_widgets[]', u'34'), + ('right_widgets[]', u'35'), ('right_widgets[]', u'36'), + ('right_widgets[]', u'37'), ('right_widgets[]', u'38'), + ('right_widgets[]', u'39'), ('right_widgets[]', u'40'), + ('js', u'true'), ('left_indexes[]', u'0'), + ('left_indexes[]', u'1'), ('left_widgets[]', u'31'), + ('left_widgets[]', u'32')]) + result = self.app.post('/ralph/edit', data=data, + follow_redirects=True) + self.assertEqual(result.status_code, 200) + self.assertEqual(result.data, 'ok') + + def test_hub_edit_post_logged_in_owner_invalid_data_1(self): + user = tests.FakeAuthorization('ralph') + with tests.auth_set(app, user): + # some indexes and widgets are not integers + data = ImmutableMultiDict( + [('right_indexes[]', u'0a'), ('right_indexes[]', u'1'), + ('right_indexes[]', u'2'), ('right_indexes[]', u'3'), + ('right_indexes[]', u'4'), ('right_indexes[]', u'5'), + ('right_indexes[]', u'6'), ('right_indexes[]', u'7'), + ('right_indexes[]', u'8'), ('right_widgets[]', u'32'), + ('right_widgets[]', u'33a'), ('right_widgets[]', u'34'), + ('right_widgets[]', u'35'), ('right_widgets[]', u'36'), + ('right_widgets[]', u'37'), ('right_widgets[]', u'38'), + ('right_widgets[]', u'39'), ('right_widgets[]', u'40'), + ('js', u'true'), ('left_indexes[]', u'0a'), + ('left_indexes[]', u'1'), ('left_widgets[]', u'31a'), + ('left_widgets[]', u'32')]) + result = self.app.post('/ralph/edit', data=data, + follow_redirects=True) + self.assertEqual(result.status_code, 400) + + def test_hub_edit_post_logged_in_owner_invalid_data_2(self): + user = tests.FakeAuthorization('ralph') + with tests.auth_set(app, user): + # indexes len don't match widgets len + data = ImmutableMultiDict( + [('right_indexes[]', u'1'), ('right_indexes[]', u'2'), + ('right_indexes[]', u'3'), ('right_indexes[]', u'4'), + ('right_indexes[]', u'5'), ('right_indexes[]', u'6'), + ('right_indexes[]', u'7'), ('right_indexes[]', u'8'), + ('right_widgets[]', u'32'), ('right_widgets[]', u'34'), + ('right_widgets[]', u'35'), ('right_widgets[]', u'36'), + ('right_widgets[]', u'37'), ('right_widgets[]', u'38'), + ('right_widgets[]', u'39'), ('right_widgets[]', u'40'), + ('js', u'true'), ('left_indexes[]', u'0'), + ('left_indexes[]', u'1'), ('left_widgets[]', u'32')]) + result = self.app.post('/ralph/edit', data=data, + follow_redirects=True) + self.assertEqual(result.status_code, 400) + + def test_login_already_loggedin(self): + user = tests.FakeAuthorization('ralph') + with tests.auth_set(app, user): + result = self.app.get('/login', follow_redirects=False) + self.assertEqual(result.status_code, 302) + self.assertEqual(urlparse(result.location).path, "/") + if __name__ == '__main__': unittest.main()