Review comments fixed. All tests passing.

This commit is contained in:
Nicholas Staples
2016-03-03 09:02:56 +00:00
parent dde2fba705
commit 1b59e5c7f1
7 changed files with 76 additions and 31 deletions

View File

@@ -44,7 +44,7 @@ manage_templates_page_headings = {
def get_page_headings(template_type):
if current_user.has_permissions(session.get('service_id', ''), 'manage_service'):
if current_user.has_permissions(['manage_service']):
return manage_service_page_headings[template_type]
else:
return manage_templates_page_headings[template_type]

View File

@@ -1,4 +1,5 @@
from flask.ext.login import (UserMixin, login_fresh)
from flask import session
class User(UserMixin):
@@ -81,7 +82,9 @@ class User(UserMixin):
def permissions(self, permissions):
raise AttributeError("Read only property")
def has_permissions(self, service_id, permissions, or_=False):
def has_permissions(self, permissions, service_id=None, or_=False):
if service_id is None:
service_id = session.get('service_id', '')
if service_id in self._permissions:
if or_:
return any([x in self._permissions[service_id] for x in permissions])

View File

@@ -2,26 +2,26 @@
<h2 class="navigation-service-name">
<a href="{{ url_for('.service_dashboard', service_id=service_id) }}">{{ session.get('service_name', 'Service') }}</a>
</h2>
{% if current_user.has_permissions(session.get('service_id', ''), ['send_messages']) %}
{% if current_user.has_permissions(['send_messages']) %}
<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='email') }}">Send emails</a></li>
<li><a href="{{ url_for('.letters_stub', service_id=service_id) }}">Send letters</a></li>
</ul>
{% elif current_user.has_permissions(session.get('service_id', ''), ['manage_templates']) %}
{% elif current_user.has_permissions(['manage_templates']) %}
<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='email') }}">Text email templates</a></li>
<li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">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('.letters_stub', service_id=service_id) }}">Send letters</a></li>
</ul>
{% endif %}
{% if current_user.has_permissions(session.get('service_id', ''), ['manage_service']) %}
{% if current_user.has_permissions(['manage_service']) %}
<ul>
<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>
</ul>
{% endif %}
{% if current_user.has_permissions(session.get('service_id', ''), ['manage_api_keys']) %}
{% if current_user.has_permissions(['manage_api_keys']) %}
<ul>
<li><a href="{{ url_for('.api_keys', service_id=service_id) }}">API keys</a></li>
<li><a href="{{ url_for('.documentation', service_id=service_id) }}">Developer documentation</a></li>

View File

@@ -16,7 +16,7 @@
{% if templates %}
{% if not has_jobs %}
{% if current_user.has_permissions(session.get('service_id', ''), ['manage_service']) %}
{% if current_user.has_permissions(['manage_service']) %}
{{ banner(
"""
Send yourself a test message
@@ -41,11 +41,11 @@
</div>
<div class="column-one-third">
<div class="sms-message-use-links">
{% if current_user.has_permissions(session.get('service_id', ''), ['manage_service']) %}
{% if current_user.has_permissions(['manage_service']) %}
<a href="{{ url_for(".send_messages", service_id=service_id, template_id=template.id) }}">Add recipients</a>
<a href="{{ url_for(".send_message_to_self", service_id=service_id, template_id=template.id) }}">Send yourself a test</a>
{% endif %}
{% if current_user.has_permissions(session.get('service_id', ''), ['manage_templates']) %}
{% if current_user.has_permissions(['manage_templates']) %}
<a href="{{ url_for(".edit_service_template", service_id=service_id, template_id=template.id) }}">Edit template</a>
{% endif %}
</div>
@@ -55,7 +55,7 @@
{% endif %}
<p>
{% if current_user.has_permissions(session.get('service_id', ''), ['manage_templates']) %}
{% if current_user.has_permissions(['manage_templates']) %}
<a href="{{ url_for('.add_service_template', service_id=service_id, template_type=template_type) }}" class="button">Add a new template</a>
{% endif %}
</p>

View File

@@ -100,10 +100,8 @@ def user_has_permissions(*permissions, or_=False):
def wrap(func):
@wraps(func)
def wrap_func(*args, **kwargs):
# We are making the assumption that the user is logged in.
from flask_login import current_user
service_id = session.get('service_id', '')
if current_user and current_user.has_permissions(service_id, permissions, or_=or_):
if current_user and current_user.has_permissions(permissions, or_=or_):
return func(*args, **kwargs)
else:
abort(403)

View File

@@ -6,18 +6,53 @@ from app.main.views.index import index
from werkzeug.exceptions import Forbidden
# def test_user_has_permissions(app_,
# api_user_active,
# mock_get_user,
# mock_get_user_by_email,
# mock_login):
# with app_.test_request_context():
# with app_.test_client() as client:
# client.login(api_user_active)
# decorator = user_has_permissions('something')
# decorated_index = decorator(index)
# try:
# response = decorated_index()
# pytest.fail("Failed to throw a forbidden exception")
# except Forbidden:
# pass
def test_user_has_permissions_on_endpoint_fail(app_,
api_user_active,
mock_login,
mock_get_user_with_permissions):
with app_.test_request_context():
with app_.test_client() as client:
client.login(api_user_active)
decorator = user_has_permissions('something')
decorated_index = decorator(index)
try:
response = decorated_index()
pytest.fail("Failed to throw a forbidden exception")
except Forbidden:
pass
def test_user_has_permissions_success(app_,
api_user_active,
mock_login,
mock_get_user_with_permissions):
with app_.test_request_context():
with app_.test_client() as client:
client.login(api_user_active)
decorator = user_has_permissions('manage_users')
decorated_index = decorator(index)
response = decorated_index()
def test_user_has_permissions_or(app_,
api_user_active,
mock_login,
mock_get_user_with_permissions):
with app_.test_request_context():
with app_.test_client() as client:
client.login(api_user_active)
decorator = user_has_permissions('something', 'manage_users', or_=True)
decorated_index = decorator(index)
response = decorated_index()
def test_user_has_permissions_multiple(app_,
api_user_active,
mock_login,
mock_get_user_with_permissions):
with app_.test_request_context():
with app_.test_client() as client:
client.login(api_user_active)
decorator = user_has_permissions('manage_templates', 'manage_users')
decorated_index = decorator(index)
response = decorated_index()

View File

@@ -305,6 +305,15 @@ def mock_get_user_by_email(mocker, api_user_active):
return mocker.patch('app.user_api_client.get_user_by_email', side_effect=_get_user)
@pytest.fixture(scope='function')
def mock_get_user_with_permissions(mocker, api_user_active):
def _get_user(id):
api_user_active._permissions[''] = ['manage_users', 'manage_templates', 'manage_settings']
return api_user_active
return mocker.patch(
'app.user_api_client.get_user', side_effect=_get_user)
@pytest.fixture(scope='function')
def mock_dont_get_user_by_email(mocker):
@@ -523,7 +532,7 @@ def mock_get_jobs(mocker):
@pytest.fixture(scope='function')
def mock_has_permissions(mocker):
def _has_permission(service_id, permissions, or_=False):
def _has_permission(permissions, service_id=None, or_=False):
return True
return mocker.patch(
'app.notify_client.user_api_client.User.has_permissions',