darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-07-13 09:50:08 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1598 from gov-cjwaszczuk/master

Browse Source 
Email auth for inviting members and editing permissions
This commit is contained in:
Chris Waszczuk
2017-11-06 10:00:15 +00:00
committed by GitHub
parent 8985f3978e c6ea90a7d8
commit 98c8ddca38
11 changed files with 325 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
app/assets/stylesheets/components/tick-cross.scss
View File

@@ -63,6 +63,11 @@
right: -135px;
}
&-hint {
color: #6F777B;
padding-top: 5px;
}
}
}

8
app/main/forms.py
View File

@@ -186,6 +186,14 @@ class PermissionsForm(Form):
manage_templates = BooleanField("Add and edit templates")
manage_service = BooleanField("Modify this service and its team")
manage_api_keys = BooleanField("Create and revoke API keys")
login_authentication = RadioField(
'Sign in using',
choices=[
('sms_auth', 'Text message code'),
('email_auth', 'Email link'),
],
validators=[DataRequired()]
)
class InviteUserForm(PermissionsForm):

38
app/main/views/manage_users.py
View File

@@ -19,7 +19,7 @@ from app.main.forms import (
InviteUserForm,
PermissionsForm
)
from app import (user_api_client, service_api_client, invite_api_client)
from app import (user_api_client, current_service, service_api_client, invite_api_client)
from app.utils import user_has_permissions
@@ -38,6 +38,7 @@ def manage_users(service_id):
users = user_api_client.get_users_for_service(service_id=service_id)
invited_users = [invite for invite in invite_api_client.get_invites_for_service(service_id=service_id)
if invite.status != 'accepted']
return render_template(
'views/manage-users.html',
users=users,
@@ -51,7 +52,13 @@ def manage_users(service_id):
@user_has_permissions('manage_users', admin_override=True)
def invite_user(service_id):
form = InviteUserForm(invalid_email_address=current_user.email_address)
form = InviteUserForm(
invalid_email_address=current_user.email_address
)
service_has_email_auth = 'email_auth' in current_service['permissions']
if not service_has_email_auth:
form.login_authentication.data = 'sms_auth'
if form.validate_on_submit():
email_address = form.email_address.data
@@ -60,7 +67,8 @@ def invite_user(service_id):
current_user.id,
service_id,
email_address,
permissions
permissions,
form.login_authentication.data
)
flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick')
@@ -68,7 +76,8 @@ def invite_user(service_id):
return render_template(
'views/invite-user.html',
form=form
form=form,
service_has_email_auth=service_has_email_auth
)
@@ -76,26 +85,35 @@ def invite_user(service_id):
@login_required
@user_has_permissions('manage_users', admin_override=True)
def edit_user_permissions(service_id, user_id):
service_has_email_auth = 'email_auth' in current_service['permissions']
# TODO we should probably using the service id here in the get user
# call as well. eg. /user/<user_id>?&service=service_id
user = user_api_client.get_user(user_id)
# Need to make the email address read only, or a disabled field?
# Do it through the template or the form class?
form = PermissionsForm(**{
role: user.has_permissions(permissions=permissions) for role, permissions in roles.items()
})
user_has_no_mobile_number = user.mobile_number is None
form = PermissionsForm(
**{role: user.has_permissions(permissions=permissions) for role, permissions in roles.items()},
login_authentication=user.auth_type
)
if form.validate_on_submit():
user_api_client.set_user_permissions(
user_id, service_id,
permissions=set(get_permissions_from_form(form)),
)
if service_has_email_auth:
user_api_client.update_user_attribute(
user_id,
auth_type=form.login_authentication.data
)
return redirect(url_for('.manage_users', service_id=service_id))
return render_template(
'views/edit-user-permissions.html',
user=user,
form=form
form=form,
service_has_email_auth=service_has_email_auth,
user_has_no_mobile_number=user_has_no_mobile_number
)

5
app/notify_client/invite_api_client.py
View File

@@ -12,12 +12,13 @@ class InviteApiClient(NotifyAdminAPIClient):
self.service_id = app.config['ADMIN_CLIENT_USER_NAME']
self.api_key = app.config['ADMIN_CLIENT_SECRET']
def create_invite(self, invite_from_id, service_id, email_address, permissions):
def create_invite(self, invite_from_id, service_id, email_address, permissions, auth_type):
data = {
'service': str(service_id),
'email_address': email_address,
'from_user': invite_from_id,
'permissions': permissions
'permissions': permissions,
'auth_type': auth_type
}
data = _attach_current_user(data)
resp = self.post(url='/service/{}/invite'.format(service_id), data=data)

15
app/notify_client/models.py
View File

@@ -10,6 +10,7 @@ class User(UserMixin):
self._mobile_number = fields.get('mobile_number')
self._password_changed_at = fields.get('password_changed_at')
self._permissions = fields.get('permissions')
self._auth_type = fields.get('auth_type')
self._failed_login_count = fields.get('failed_login_count')
self._state = fields.get('state')
self.max_failed_login_count = max_failed_login_count
@@ -108,6 +109,14 @@ class User(UserMixin):
return set(self._permissions[service_id]) >= set(permissions)
return False
@property
def auth_type(self):
return self._auth_type
@auth_type.setter
def auth_type(self, auth_type):
self._auth_type = auth_type
@property
def failed_login_count(self):
return self._failed_login_count
@@ -155,6 +164,7 @@ class InvitedUser(object):
self.permissions = []
self.status = status
self.created_at = created_at
self.auth_type = auth_type
def has_permissions(self, permissions):
return set(self.permissions) > set(permissions)
@@ -164,10 +174,12 @@ class InvitedUser(object):
self.service,
self.from_user,
self.email_address,
self.auth_type,
self.status) == (other.id,
other.service,
other.from_user,
other.email_address,
other.auth_type,
other.status))
def serialize(self, permissions_as_string=False):
@@ -176,7 +188,8 @@ class InvitedUser(object):
'from_user': self.from_user,
'email_address': self.email_address,
'status': self.status,
'created_at': str(self.created_at)
'created_at': str(self.created_at),
'auth_type': self.auth_type
}
if permissions_as_string:
data['permissions'] = ','.join(self.permissions)

3
app/notify_client/user_api_client.py
View File

@@ -6,7 +6,8 @@ from app.notify_client.models import User
ALLOWED_ATTRIBUTES = {
'name',
'email_address',
'mobile_number'
'mobile_number',
'auth_type',
}

18
app/templates/views/manage-users.html
View File

@@ -63,6 +63,15 @@
user.has_permissions(permissions=['manage_api_keys']),
'Access API keys'
) }}
{% if 'email_auth' in current_service['permissions'] %}
<div class="tick-cross-list-hint">
{% if user.auth_type == 'sms_auth' %}
Signs in with a text message code
{% else %}
Signs in with an email link
{% endif %}
</div>
{% endif %}
</div>
{% if current_user.has_permissions(['manage_users'], admin_override=True) %}
{% if current_user.id != user.id %}
@@ -104,6 +113,15 @@
user.has_permissions(permissions=['manage_api_keys']),
'Access API keys'
) }}
{% if 'email_auth' in current_service['permissions'] %}
<div class="tick-cross-list-hint">
{% if user.auth_type == 'sms_auth' %}
Signs in with a text message code
{% else %}
Signs in with an email link
{% endif %}
</div>
{% endif %}
</div>
<li class="tick-cross-list-edit-link">
{% if user.status == 'pending' and current_user.has_permissions(['manage_users']) %}

5
app/templates/views/manage-users/permissions.html
View File

@@ -1,4 +1,5 @@
{% from "components/checkbox.html" import checkbox %}
{% from "components/radios.html" import radios %}
<fieldset class="form-group">
<legend class="form-label">
@@ -20,3 +21,7 @@
<li>who the other team members are</li>
</ul>
</div>
{% if service_has_email_auth %}
{{ radios(form.login_authentication, disable=['sms_auth' if user_has_no_mobile_number]) }}
{% endif %}

4
tests/__init__.py
View File

@@ -53,7 +53,7 @@ def service_json(
created_at=None,
letter_contact_block=None,
inbound_api=None,
permissions=['email', 'sms'],
permissions=None,
organisation_type='central',
free_sms_fragment_limit=250000,
prefix_sms_with_service_name=None,
@@ -61,7 +61,7 @@ def service_json(
if users is None:
users = []
if permissions is None:
permissions = []
permissions = ['email', 'sms']
if inbound_api is None:
inbound_api = []
if prefix_sms_with_service_name is None:

198
tests/app/main/views/test_manage_users.py
View File

@@ -10,6 +10,7 @@ from tests.conftest import (
SERVICE_ONE_ID,
active_user_with_permissions,
active_user_view_permissions,
active_user_no_mobile,
active_user_manage_template_permission,
)
@@ -55,6 +56,94 @@ def test_should_show_overview_page(
app.user_api_client.get_users_for_service.assert_called_once_with(service_id=SERVICE_ONE_ID)
@pytest.mark.parametrize('endpoint, extra_args, service_has_email_auth, auth_options_hidden', [
(
'main.edit_user_permissions',
{'user_id': 0},
True,
False
),
(
'main.edit_user_permissions',
{'user_id': 0},
False,
True
),
(
'main.invite_user',
{},
True,
False
),
(
'main.invite_user',
{},
False,
True
)
])
def test_service_with_no_email_auth_hides_auth_type_options(
client_request,
endpoint,
extra_args,
service_has_email_auth,
auth_options_hidden,
service_one
):
if service_has_email_auth:
service_one['permissions'].append('email_auth')
page = client_request.get(endpoint, service_id=service_one['id'], **extra_args)
assert (page.find('input', attrs={"name": "login_authentication"}) is None) == auth_options_hidden
@pytest.mark.parametrize('service_has_email_auth, displays_auth_type', [
(True, True),
(False, False)
])
def test_manage_users_page_shows_member_auth_type_if_service_has_email_auth_activated(
client_request,
service_has_email_auth,
service_one,
mock_get_users_by_service,
mock_get_invites_for_service,
displays_auth_type
):
if service_has_email_auth:
service_one['permissions'].append('email_auth')
page = client_request.get('main.manage_users', service_id=service_one['id'])
assert bool(page.select_one('.tick-cross-list-hint')) == displays_auth_type
@pytest.mark.parametrize('user, sms_option_disabled', [
(
active_user_no_mobile,
True,
),
(
active_user_with_permissions,
False,
),
])
def test_user_with_no_mobile_number_cant_be_set_to_sms_auth(
client_request,
user,
sms_option_disabled,
service_one,
mocker
):
service_one['permissions'].append('email_auth')
test_user = mocker.patch('app.user_api_client.get_user', return_value=user(mocker))
page = client_request.get(
'main.edit_user_permissions',
service_id=service_one['id'],
user_id=test_user.id
)
sms_auth_radio_button = page.select_one('input[value="sms_auth"]')
assert sms_auth_radio_button.has_attr("disabled") == sms_option_disabled
@pytest.mark.parametrize('endpoint, extra_args, expected_checkboxes', [
(
'main.edit_user_permissions',
@@ -167,6 +256,60 @@ def test_edit_some_user_permissions(
)
@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth'])
def test_edit_user_permissions_including_authentication_with_email_auth_service(
logged_in_client,
active_user_with_permissions,
mocker,
mock_get_invites_for_service,
mock_set_user_permissions,
mock_update_user_attribute,
service_one,
auth_type
):
service_one['permissions'].append('email_auth')
response = logged_in_client.post(
url_for(
'main.edit_user_permissions',
service_id=service_one['id'],
user_id=active_user_with_permissions.id
),
data={
'email_address': active_user_with_permissions.email_address,
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
'login_authentication': auth_type
}
)
mock_set_user_permissions.assert_called_with(
str(active_user_with_permissions.id),
service_one['id'],
permissions={
'send_texts',
'send_emails',
'send_letters',
'manage_users',
'manage_templates',
'manage_settings',
'manage_api_keys',
'view_activity'
}
)
mock_update_user_attribute.assert_called_with(
str(active_user_with_permissions.id),
auth_type=auth_type
)
assert response.status_code == 302
assert response.location == url_for(
'main.manage_users', service_id=service_one['id'], _external=True
)
def test_should_show_page_for_inviting_user(
logged_in_client,
active_user_with_permissions,
@@ -220,7 +363,60 @@ def test_invite_user(
app.invite_api_client.create_invite.assert_called_once_with(sample_invite['from_user'],
sample_invite['service'],
email_address,
expected_permissions)
expected_permissions,
'sms_auth')
@pytest.mark.parametrize('auth_type', [
('sms_auth'),
('email_auth')
])
@pytest.mark.parametrize('email_address, gov_user', [
('test@example.gov.uk', True),
('test@nonwhitelist.com', False)
])
def test_invite_user_with_email_auth_service(
logged_in_client,
active_user_with_permissions,
sample_invite,
email_address,
gov_user,
mocker,
service_one,
auth_type
):
service_one['permissions'].append('email_auth')
sample_invite['email_address'] = 'test@example.gov.uk'
data = [InvitedUser(**sample_invite)]
assert is_gov_user(email_address) == gov_user
mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
mocker.patch('app.invite_api_client.create_invite', return_value=InvitedUser(**sample_invite))
response = logged_in_client.post(
url_for('main.invite_user', service_id=service_one['id']),
data={'email_address': email_address,
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
'login_authentication': auth_type},
follow_redirects=True
)
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Team members'
flash_banner = page.find('div', class_='banner-default-with-tick').string.strip()
assert flash_banner == 'Invite sent to test@example.gov.uk'
expected_permissions = 'manage_api_keys,manage_settings,manage_templates,manage_users,send_emails,send_letters,send_texts,view_activity' # noqa
app.invite_api_client.create_invite.assert_called_once_with(sample_invite['from_user'],
sample_invite['service'],
email_address,
expected_permissions,
auth_type)
def test_cancel_invited_user_cancels_user_invitations(

49
tests/conftest.py
View File

@@ -1063,7 +1063,8 @@ def platform_admin_user(fake_uuid):
'manage_settings',
'manage_api_keys',
'view_activity']},
'platform_admin': True
'platform_admin': True,
'auth_type': 'sms_auth'
}
user = User(user_data)
return user
@@ -1081,6 +1082,7 @@ def api_user_active(fake_uuid, email_address='test@user.gov.uk'):
'failed_login_count': 0,
'permissions': {},
'platform_admin': False,
'auth_type': 'sms_auth',
'password_changed_at': str(datetime.utcnow())
}
user = User(user_data)
@@ -1099,6 +1101,7 @@ def api_nongov_user_active(fake_uuid):
'failed_login_count': 0,
'permissions': {},
'platform_admin': False,
'auth_type': 'sms_auth',
'password_changed_at': str(datetime.utcnow())
}
user = User(user_data)
@@ -1125,7 +1128,35 @@ def active_user_with_permissions(fake_uuid):
'manage_settings',
'manage_api_keys',
'view_activity']},
'platform_admin': False
'platform_admin': False,
'auth_type': 'sms_auth'
}
user = User(user_data)
return user
@pytest.fixture(scope='function')
def active_user_no_mobile(fake_uuid):
from app.notify_client.user_api_client import User
user_data = {'id': fake_uuid,
'name': 'Test User',
'password': 'somepassword',
'password_changed_at': str(datetime.utcnow()),
'email_address': 'test@user.gov.uk',
'mobile_number': None,
'state': 'active',
'failed_login_count': 0,
'permissions': {SERVICE_ONE_ID: ['send_texts',
'send_emails',
'send_letters',
'manage_users',
'manage_templates',
'manage_settings',
'manage_api_keys',
'view_activity']},
'platform_admin': False,
'auth_type': 'email_auth'
}
user = User(user_data)
return user
@@ -1144,7 +1175,8 @@ def active_user_view_permissions(fake_uuid):
'state': 'active',
'failed_login_count': 0,
'permissions': {SERVICE_ONE_ID: ['view_activity']},
'platform_admin': False
'platform_admin': False,
'auth_type': 'sms_auth'
}
user = User(user_data)
return user
@@ -1167,7 +1199,8 @@ def active_user_manage_template_permission(fake_uuid):
'manage_templates',
'view_activity',
]},
'platform_admin': False
'platform_admin': False,
'auth_type': 'sms_auth'
}
user = User(user_data)
return user
@@ -1183,7 +1216,8 @@ def api_user_locked(fake_uuid):
'mobile_number': '07700 900762',
'state': 'active',
'failed_login_count': 5,
'permissions': {}
'permissions': {},
'auth_type': 'sms_auth'
}
user = User(user_data)
return user
@@ -1200,7 +1234,8 @@ def api_user_request_password_reset(fake_uuid):
'state': 'active',
'failed_login_count': 5,
'permissions': {},
'password_changed_at': None
'password_changed_at': None,
'auth_type': 'sms_auth'
}
user = User(user_data)
return user
@@ -1217,6 +1252,7 @@ def api_user_changed_password(fake_uuid):
'state': 'active',
'failed_login_count': 5,
'permissions': {},
'auth_type': 'sms_auth',
'password_changed_at': str(datetime.utcnow() + timedelta(minutes=1))
}
user = User(user_data)
@@ -1813,6 +1849,7 @@ def mock_get_users_by_service(mocker):
'password_changed_at': None,
'name': 'Test User',
'email_address': 'notify@digital.cabinet-office.gov.uk',
'auth_type': 'sms_auth',
'failed_login_count': 0}]
return [User(data[0])]