From 2a2a733a41a1aa355a3728d836daebc873384d5b Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 21 Oct 2016 14:24:21 +0100 Subject: [PATCH 1/5] Fix 500s when requesting json after logged out: * Update permissions decorator to make sure user is logged in first, else 401 * Stop further ajax json calls on failure --- app/assets/javascripts/updateContent.js | 2 +- app/utils.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/updateContent.js b/app/assets/javascripts/updateContent.js index 41dd41ff5..89c87c550 100644 --- a/app/assets/javascripts/updateContent.js +++ b/app/assets/javascripts/updateContent.js @@ -26,7 +26,7 @@ ).done( response => flushQueue(queue, response) ).fail( - () => clearQueue(queue) + () => poll = function(){} ); setTimeout( diff --git a/app/utils.py b/app/utils.py index 66d787ce0..240245e5e 100644 --- a/app/utils.py +++ b/app/utils.py @@ -42,11 +42,18 @@ def user_has_permissions(*permissions, admin_override=False, any_=False): @wraps(func) def wrap_func(*args, **kwargs): from flask_login import current_user - if current_user and current_user.has_permissions(permissions=permissions, - admin_override=admin_override, any_=any_): - return func(*args, **kwargs) + + if current_user and current_user.is_authenticated: + if current_user.has_permissions( + permissions=permissions, + admin_override=admin_override, + any_=any_ + ): + return func(*args, **kwargs) + else: + abort(403) else: - abort(403) + abort(401) return wrap_func return wrap From a05a36f2a86484487bad8e6d1e4b90e3818e817b Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 21 Oct 2016 14:28:55 +0100 Subject: [PATCH 2/5] Remove newline --- app/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/utils.py b/app/utils.py index 240245e5e..72232cc6b 100644 --- a/app/utils.py +++ b/app/utils.py @@ -42,7 +42,6 @@ def user_has_permissions(*permissions, admin_override=False, any_=False): @wraps(func) def wrap_func(*args, **kwargs): from flask_login import current_user - if current_user and current_user.is_authenticated: if current_user.has_permissions( permissions=permissions, From 76a61ff737159684c1f4a840777ae1b4e7cceb11 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 21 Oct 2016 17:36:42 +0100 Subject: [PATCH 3/5] Update dashboard json endpoints: * Remove login_required decorator --- app/main/views/dashboard.py | 1 - app/main/views/jobs.py | 1 - 2 files changed, 2 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 54407681f..d91cbbef9 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -53,7 +53,6 @@ def service_dashboard(service_id): @main.route("/services//dashboard.json") -@login_required @user_has_permissions('view_activity', admin_override=True) def service_dashboard_updates(service_id): return jsonify(**get_dashboard_partials(service_id)) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index a7fd87154..eed140221 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -172,7 +172,6 @@ def cancel_job(service_id, job_id): @main.route("/services//jobs/.json") -@login_required @user_has_permissions('view_activity', admin_override=True) def view_job_updates(service_id, job_id): return jsonify(**get_job_partials( From 2b5894bed9b316c72cec57ab8feb63ee3e7e8d24 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Mon, 24 Oct 2016 17:36:53 +0100 Subject: [PATCH 4/5] Add check for current user not authenticated --- tests/app/main/test_permissions.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index f168a3a39..d20262fdf 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -1,7 +1,7 @@ import pytest from app.utils import user_has_permissions from app.main.views.index import index -from werkzeug.exceptions import Forbidden +from werkzeug.exceptions import Forbidden, Unauthorized from flask import request @@ -9,7 +9,8 @@ def _test_permissions(app_, usr, permissions, service_id, will_succeed, any_=Fal with app_.test_request_context() as ctx: request.view_args.update({'service_id': service_id}) with app_.test_client() as client: - client.login(usr) + if usr: + client.login(usr) decorator = user_has_permissions(*permissions, any_=any_, admin_override=admin_override) decorated_index = decorator(index) if will_succeed: @@ -17,8 +18,8 @@ def _test_permissions(app_, usr, permissions, service_id, will_succeed, any_=Fal else: try: response = decorated_index() - pytest.fail("Failed to throw a forbidden exception") - except Forbidden: + pytest.fail("Failed to throw a forbidden or unauthorised exception") + except (Forbidden, Unauthorized): pass @@ -107,6 +108,17 @@ def test_platform_admin_user_can_not_access_page(app_, admin_override=False) +def test_user_with_permissions_returns_401_unauthenticated_user(app_): + from flask_login import current_user + assert not current_user + _test_permissions( + app_, + None, + [], + '', + will_succeed=False) + + def _user_with_permissions(): from app.notify_client.user_api_client import User From 8f10eae9c31f649661387e2f52ef362153ca236c Mon Sep 17 00:00:00 2001 From: imdadahad Date: Tue, 25 Oct 2016 10:46:10 +0100 Subject: [PATCH 5/5] Rename test --- tests/app/main/test_permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index d20262fdf..62c873dff 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -108,7 +108,7 @@ def test_platform_admin_user_can_not_access_page(app_, admin_override=False) -def test_user_with_permissions_returns_401_unauthenticated_user(app_): +def test_no_user_returns_401_unauth(app_): from flask_login import current_user assert not current_user _test_permissions(