With the addition of has_permissions on the dashboard, jobs, and manage_users pages a platform admin user or a users with no permissions on the service could no longer see the page.

A new permission has been added, view_activity, to resolve this issue.
Another pull request in notifications-admin will be required to update all users with a default permission of view_activity.
This commit is contained in:
Rebecca Law
2016-03-29 13:23:36 +01:00
parent 8a55e61dac
commit b79901fe28
7 changed files with 47 additions and 21 deletions
+1 -1
View File
@@ -15,7 +15,7 @@ from app.utils import user_has_permissions
@main.route("/services/<service_id>/dashboard") @main.route("/services/<service_id>/dashboard")
@login_required @login_required
@user_has_permissions() @user_has_permissions('view_activity', admin_override=True)
def service_dashboard(service_id): def service_dashboard(service_id):
templates = templates_dao.get_service_templates(service_id)['data'] templates = templates_dao.get_service_templates(service_id)['data']
jobs = job_api_client.get_job(service_id)['data'] jobs = job_api_client.get_job(service_id)['data']
+7 -11
View File
@@ -6,24 +6,20 @@ from flask import (
render_template, render_template,
abort, abort,
jsonify, jsonify,
flash, request
redirect,
request,
url_for
) )
from flask_login import login_required from flask_login import login_required
from utils.template import Template from utils.template import Template
from app import job_api_client, notification_api_client from app import job_api_client, notification_api_client
from app.main import main from app.main import main
from app.main.dao import templates_dao from app.main.dao import (services_dao, templates_dao)
from app.main.dao import services_dao
from app.utils import (get_page_from_request, generate_previous_next_dict, user_has_permissions) from app.utils import (get_page_from_request, generate_previous_next_dict, user_has_permissions)
@main.route("/services/<service_id>/jobs") @main.route("/services/<service_id>/jobs")
@login_required @login_required
@user_has_permissions() @user_has_permissions('view_activity', admin_override=True)
def view_jobs(service_id): def view_jobs(service_id):
jobs = job_api_client.get_job(service_id)['data'] jobs = job_api_client.get_job(service_id)['data']
return render_template( return render_template(
@@ -35,7 +31,7 @@ def view_jobs(service_id):
@main.route("/services/<service_id>/jobs/<job_id>") @main.route("/services/<service_id>/jobs/<job_id>")
@login_required @login_required
@user_has_permissions() @user_has_permissions('view_activity', admin_override=True)
def view_job(service_id, job_id): def view_job(service_id, job_id):
service = services_dao.get_service_by_id_or_404(service_id) service = services_dao.get_service_by_id_or_404(service_id)
job = job_api_client.get_job(service_id, job_id)['data'] job = job_api_client.get_job(service_id, job_id)['data']
@@ -64,7 +60,7 @@ def view_job(service_id, job_id):
@main.route("/services/<service_id>/jobs/<job_id>.json") @main.route("/services/<service_id>/jobs/<job_id>.json")
@login_required @login_required
@user_has_permissions() @user_has_permissions('view_activity')
def view_job_updates(service_id, job_id): def view_job_updates(service_id, job_id):
service = services_dao.get_service_by_id_or_404(service_id) service = services_dao.get_service_by_id_or_404(service_id)
job = job_api_client.get_job(service_id, job_id)['data'] job = job_api_client.get_job(service_id, job_id)['data']
@@ -92,7 +88,7 @@ def view_job_updates(service_id, job_id):
@main.route('/services/<service_id>/notifications') @main.route('/services/<service_id>/notifications')
@login_required @login_required
@user_has_permissions() @user_has_permissions('view_activity', admin_override=True)
def view_notifications(service_id): def view_notifications(service_id):
# TODO get the api to return count of pages as well. # TODO get the api to return count of pages as well.
page = get_page_from_request() page = get_page_from_request()
@@ -121,7 +117,7 @@ def view_notifications(service_id):
@main.route("/services/<service_id>/jobs/<job_id>/notification/<string:notification_id>") @main.route("/services/<service_id>/jobs/<job_id>/notification/<string:notification_id>")
@login_required @login_required
@user_has_permissions() @user_has_permissions('view_activity', admin_override=True)
def view_notification(service_id, job_id, notification_id): def view_notification(service_id, job_id, notification_id):
now = time.strftime('%H:%M') now = time.strftime('%H:%M')
+5 -1
View File
@@ -66,7 +66,11 @@ def get_page_headings(template_type):
@main.route("/services/<service_id>/send/<template_type>", methods=['GET']) @main.route("/services/<service_id>/send/<template_type>", methods=['GET'])
@login_required @login_required
@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', 'manage_api_keys', @user_has_permissions('view_activity',
'send_texts',
'send_emails',
'manage_templates',
'manage_api_keys',
admin_override=True, or_=True) admin_override=True, or_=True)
def choose_template(service_id, template_type): def choose_template(service_id, template_type):
+5 -2
View File
@@ -2,15 +2,17 @@
<h2 class="navigation-service-name"> <h2 class="navigation-service-name">
<a href="{{ url_for('.service_dashboard', service_id=service_id) }}">{{ session.get('service_name', 'Service') }}</a> <a href="{{ url_for('.service_dashboard', service_id=service_id) }}">{{ session.get('service_name', 'Service') }}</a>
</h2> </h2>
{% if current_user.has_permissions(['view_activity'], admin_override=True) %}
<ul> <ul>
<li><a href="{{ url_for('.view_notifications', service_id=service_id) }}">View activity</a></li> <li><a href="{{ url_for('.view_notifications', service_id=service_id) }}">View activity</a></li>
</ul> </ul>
{% endif %}
{% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
<ul> <ul>
<li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Send text messages</a></li> <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Send text messages</a></li>
<li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='email') }}">Send emails</a></li> <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='email') }}">Send emails</a></li>
</ul> </ul>
{% elif current_user.has_permissions(['manage_templates','manage_api_keys'], admin_override=True, or_=True) %} {% elif current_user.has_permissions(['view_activity', 'manage_templates','manage_api_keys'], admin_override=True, or_=True) %}
<ul> <ul>
<li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Text message templates</a></li> <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Text message templates</a></li>
<li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='email') }}">Email templates</a></li> <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='email') }}">Email templates</a></li>
@@ -21,7 +23,8 @@
<li><a href="{{ url_for('.manage_users', service_id=service_id) }}">Manage team</a></li> <li><a href="{{ url_for('.manage_users', service_id=service_id) }}">Manage team</a></li>
<li><a href="{{ url_for('.service_settings', service_id=service_id) }}">Manage settings</a></li> <li><a href="{{ url_for('.service_settings', service_id=service_id) }}">Manage settings</a></li>
</ul> </ul>
{% else %} {% endif %}
{% if current_user.has_permissions(['view_activity'], admin_override=True) %}
<ul> <ul>
<li><a href="{{ url_for('.manage_users', service_id=service_id) }}">View team members</a></li> <li><a href="{{ url_for('.manage_users', service_id=service_id) }}">View team members</a></li>
</ul> </ul>
+3 -1
View File
@@ -63,7 +63,9 @@ Manage users GOV.UK Notify
{{ boolean_field(item.has_permissions(permissions=['manage_api_keys', 'access_developer_docs'])) }} {{ boolean_field(item.has_permissions(permissions=['manage_api_keys', 'access_developer_docs'])) }}
{% if item.status == 'pending' %} {% if item.status == 'pending' %}
{% call field(align='right') %} {% call field(align='right') %}
<a href="{{ url_for('.cancel_invited_user', service_id=service_id, invited_user_id=item.id)}}">Cancel invitation</a> {% if current_user.has_permissions(['manage_users']) %}
<a href="{{ url_for('.cancel_invited_user', service_id=service_id, invited_user_id=item.id)}}">Cancel invitation</a>
{% endif %}
{% endcall %} {% endcall %}
{% else %} {% else %}
{% call field() %} {% call field() %}
+4 -4
View File
@@ -48,7 +48,7 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one, mock_get
app_, app_,
api_user_active, api_user_active,
service_one, service_one,
['send_texts', 'send_emails', 'send_letters']) ['view_activity', 'send_texts', 'send_emails', 'send_letters'])
page = resp.get_data(as_text=True) page = resp.get_data(as_text=True)
assert url_for( assert url_for(
'main.choose_template', 'main.choose_template',
@@ -74,7 +74,7 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_ge
app_, app_,
api_user_active, api_user_active,
service_one, service_one,
['manage_users', 'manage_templates', 'manage_settings']) ['view_activity', 'manage_users', 'manage_templates', 'manage_settings'])
page = resp.get_data(as_text=True) page = resp.get_data(as_text=True)
assert url_for( assert url_for(
'main.choose_template', 'main.choose_template',
@@ -99,7 +99,7 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g
app_, app_,
api_user_active, api_user_active,
service_one, service_one,
['manage_api_keys', 'access_developer_docs']) ['view_activity', 'manage_api_keys', 'access_developer_docs'])
page = resp.get_data(as_text=True) page = resp.get_data(as_text=True)
assert url_for( assert url_for(
'main.choose_template', 'main.choose_template',
@@ -159,6 +159,6 @@ def test_route_for_service_permissions(mocker,
url_for( url_for(
route, route,
service_id=service_one['id']), service_id=service_one['id']),
[], ['view_activity'],
api_user_active, api_user_active,
service_one) service_one)
+22 -1
View File
@@ -154,6 +154,27 @@ def test_route_permissions(mocker,
service_one) service_one)
def test_route_permissions_for_choose_tempalte(mocker,
app_,
api_user_active,
service_one,
mock_get_service_template):
with app_.test_request_context():
validate_route_permission(
mocker,
app_,
"GET",
200,
url_for(
'main.choose_template',
service_id=service_one['id'],
template_type='sms',
template_id=123),
['view_activity'],
api_user_active,
service_one)
def test_route_invalid_permissions(mocker, def test_route_invalid_permissions(mocker,
app_, app_,
api_user_active, api_user_active,
@@ -175,6 +196,6 @@ def test_route_invalid_permissions(mocker,
service_id=service_one['id'], service_id=service_one['id'],
template_type='sms', template_type='sms',
template_id=123), template_id=123),
['blah'], ['view_activity'],
api_user_active, api_user_active,
service_one) service_one)