add option to suppress platform admin temporarily

so that platform admins (us) can view pages as regular users do easily.
Simply adds a flag in the session cookie that overrides the actual
platform admin flag on the user model if set. This way it's safe, since
this only downgrades existing functionality, so if someone managed to
alter it they could only get less permissions, not more.

You can change this value from the user profile page if either:

* you're a platform admin
* the flag is set (to any value) on the cookie.

This slightly weird check means that we don't check the underlying
`user._platform_admin` flag anywhere in the code, even when toggling
the suppression.
This commit is contained in:
Leo Hemsted
2019-06-13 19:00:17 +01:00
parent c7325576d6
commit 7b02cb72c6
7 changed files with 147 additions and 4 deletions

View File

@@ -1,6 +1,6 @@
import json import json
from flask import current_app, redirect, render_template, session, url_for from flask import current_app, redirect, render_template, session, url_for, abort
from flask_login import current_user, login_required from flask_login import current_user, login_required
from notifications_utils.url_safe_token import check_token from notifications_utils.url_safe_token import check_token
@@ -12,6 +12,7 @@ from app.main.forms import (
ChangeNameForm, ChangeNameForm,
ChangePasswordForm, ChangePasswordForm,
ConfirmPasswordForm, ConfirmPasswordForm,
ServiceOnOffSettingForm,
TwoFactorForm, TwoFactorForm,
) )
from app.models.user import User from app.models.user import User
@@ -192,3 +193,26 @@ def user_profile_password():
'views/user-profile/change-password.html', 'views/user-profile/change-password.html',
form=form form=form
) )
@main.route("/user-profile/suppress-platform-admin", methods=['GET', 'POST'])
@login_required
def user_profile_suppress_platform_admin():
if not current_user.platform_admin and not session.get('suppress_platform_admin'):
abort(403)
form = ServiceOnOffSettingForm(
name="This setting will be cleared if you sign out and sign in again",
enabled=session.get('suppress_platform_admin', False),
truthy='Yes (view as regular user)',
falsey='No (view as platform admin)',
)
if form.validate_on_submit():
session['suppress_platform_admin'] = form.enabled.data
return redirect(url_for('.user_profile'))
return render_template(
'views/user-profile/suppress-platform-admin.html',
form=form
)

View File

@@ -49,6 +49,7 @@ class User(JSONModel, UserMixin):
super().__init__(_dict) super().__init__(_dict)
self.permissions = _dict.get('permissions', {}) self.permissions = _dict.get('permissions', {})
self.max_failed_login_count = current_app.config['MAX_FAILED_LOGIN_COUNT'] self.max_failed_login_count = current_app.config['MAX_FAILED_LOGIN_COUNT']
self._platform_admin = _dict['platform_admin']
@classmethod @classmethod
def from_id(cls, user_id): def from_id(cls, user_id):
@@ -173,6 +174,10 @@ class User(JSONModel, UserMixin):
super(User, self).is_authenticated super(User, self).is_authenticated
) )
@property
def platform_admin(self):
return self._platform_admin and not session.get('suppress_platform_admin', False)
def has_permissions(self, *permissions, restrict_admin_usage=False): def has_permissions(self, *permissions, restrict_admin_usage=False):
unknown_permissions = set(permissions) - all_permissions unknown_permissions = set(permissions) - all_permissions
if unknown_permissions: if unknown_permissions:

View File

@@ -45,6 +45,14 @@
{{ edit_field('Change', url_for('.user_profile_password')) }} {{ edit_field('Change', url_for('.user_profile_password')) }}
{% endcall %} {% endcall %}
{% if current_user.platform_admin or session.get('suppress_platform_admin') %}
{% call row() %}
{{ text_field('Suppress Platform Admin') }}
{{ text_field('On' if session.get('suppress_platform_admin') else 'Off') }}
{{ edit_field('Change', url_for('.user_profile_suppress_platform_admin')) }}
{% endcall %}
{% endif %}
{% endcall %} {% endcall %}
</div> </div>

View File

@@ -0,0 +1,27 @@
{% extends "withoutnav_template.html" %}
{% from "components/form.html" import form_wrapper %}
{% from "components/page-footer.html" import page_footer %}
{% from "components/page-header.html" import page_header %}
{% from "components/radios.html" import radios %}
{% block per_page_title %}
Suppress platform admin view
{% endblock %}
{% block maincolumn_content %}
<div class="grid-row">
<div class="column-five-sixths">
{{ page_header(
'Suppress platform admin',
back_link=url_for('.user_profile')
) }}
{% call form_wrapper() %}
{{ radios(form.enabled) }}
{{ page_footer('Save') }}
{% endcall %}
</div>
</div>
{% endblock %}

View File

@@ -14,6 +14,18 @@ def test_should_show_overview_page(
): ):
page = client_request.get(('main.user_profile')) page = client_request.get(('main.user_profile'))
assert page.select_one('h1').text.strip() == 'Your profile' assert page.select_one('h1').text.strip() == 'Your profile'
assert 'Suppress Platform Admin' not in page
def test_overview_page_shows_suppress_for_platform_admin(
client_request,
platform_admin_user
):
client_request.login(platform_admin_user)
page = client_request.get(('main.user_profile'))
assert page.select_one('h1').text.strip() == 'Your profile'
suppress_platform_admin_row = page.select('tr')[-1]
assert ' '.join(suppress_platform_admin_row.text.split()) == 'Suppress Platform Admin Off Change'
def test_should_show_name_page( def test_should_show_name_page(
@@ -264,3 +276,49 @@ def test_non_gov_user_cannot_access_change_email_page(
): ):
client_request.login(api_nongov_user_active) client_request.login(api_nongov_user_active)
client_request.get('main.user_profile_email', _expected_status=403) client_request.get('main.user_profile_email', _expected_status=403)
def test_normal_user_doesnt_see_suppress_platform_admin(client_request):
client_request.get('main.user_profile_suppress_platform_admin', _expected_status=403)
def test_platform_admin_can_see_suppress_platform_admin_page(client_request, platform_admin_user):
client_request.login(platform_admin_user)
page = client_request.get('main.user_profile_suppress_platform_admin')
assert page.select_one('h1').text.strip() == 'Suppress platform admin'
assert page.select_one('input[checked]')['value'] == 'False'
def test_can_suppress_platform_admin(client_request, platform_admin_user):
client_request.login(platform_admin_user)
with client_request.session_transaction() as session:
assert 'suppress_platform_admin' not in session
client_request.post(
'main.user_profile_suppress_platform_admin',
_data={'enabled': True},
_expected_status=302,
_expected_redirect=url_for('main.user_profile', _external=True),
)
with client_request.session_transaction() as session:
assert session['suppress_platform_admin'] is True
def test_can_turn_off_suppress_platform_admin(client_request, platform_admin_user):
client_request.login(platform_admin_user)
with client_request.session_transaction() as session:
session['suppress_platform_admin'] = True
client_request.post(
'main.user_profile_suppress_platform_admin',
_data={'enabled': False},
_expected_status=302,
_expected_redirect=url_for('main.user_profile', _external=True),
)
with client_request.session_transaction() as session:
assert session['suppress_platform_admin'] is False

View File

@@ -9,7 +9,8 @@ def test_user(app_):
'email_address': 'test@user.gov.uk', 'email_address': 'test@user.gov.uk',
'mobile_number': '+4412341234', 'mobile_number': '+4412341234',
'state': 'pending', 'state': 'pending',
'failed_login_count': 0 'failed_login_count': 0,
'platform_admin': False,
} }
user = User(user_data) user = User(user_data)
@@ -40,3 +41,21 @@ def test_activate_user(app_, api_user_pending, mock_activate_user):
def test_activate_user_already_active(app_, api_user_active, mock_activate_user): def test_activate_user_already_active(app_, api_user_active, mock_activate_user):
assert User(api_user_active).activate() == User(api_user_active) assert User(api_user_active).activate() == User(api_user_active)
assert mock_activate_user.called is False assert mock_activate_user.called is False
@pytest.mark.parametrize('is_platform_admin, value_in_session, expected_result', [
(True, True, False),
(True, False, True),
(True, None, True),
(False, True, False),
(False, False, False),
(False, None, False),
])
def test_platform_admin_checks_flag_set_in_session(client, mocker, is_platform_admin, value_in_session, expected_result):
session_dict = {}
if value_in_session is not None:
session_dict['suppress_platform_admin'] = value_in_session
mocker.patch.dict('app.models.user.session', values=session_dict, clear=True)
assert User({'platform_admin': is_platform_admin}).platform_admin == expected_result

View File

@@ -1091,6 +1091,7 @@ def api_user_pending(fake_uuid):
'permissions': {}, 'permissions': {},
'organisations': [], 'organisations': [],
'current_session_id': None, 'current_session_id': None,
'password_changed_at': str(datetime.utcnow()),
} }
return user_data return user_data
@@ -1114,6 +1115,7 @@ def platform_admin_user(fake_uuid):
'view_activity']}, 'view_activity']},
'platform_admin': True, 'platform_admin': True,
'auth_type': 'sms_auth', 'auth_type': 'sms_auth',
'password_changed_at': str(datetime.utcnow()),
'services': [], 'services': [],
'organisations': [], 'organisations': [],
'current_session_id': None, 'current_session_id': None,
@@ -1443,7 +1445,7 @@ def api_user_request_password_reset(fake_uuid):
'state': 'active', 'state': 'active',
'failed_login_count': 5, 'failed_login_count': 5,
'permissions': {}, 'permissions': {},
'password_changed_at': None, 'password_changed_at': str(datetime.utcnow()),
'auth_type': 'sms_auth', 'auth_type': 'sms_auth',
'organisations': [], 'organisations': [],
'current_session_id': None, 'current_session_id': None,
@@ -2165,7 +2167,7 @@ def mock_get_users_by_service(mocker):
'manage_settings', 'manage_settings',
'manage_api_keys']}, 'manage_api_keys']},
'state': 'active', 'state': 'active',
'password_changed_at': None, 'password_changed_at': str(datetime.utcnow()),
'name': 'Test User', 'name': 'Test User',
'email_address': 'notify@digital.cabinet-office.gov.uk', 'email_address': 'notify@digital.cabinet-office.gov.uk',
'auth_type': 'sms_auth', 'auth_type': 'sms_auth',