darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-02-04 02:11:11 -05:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1383 from alphagov/user-mobile-optional

Browse Source 
User mobile optional
This commit is contained in:
Leo Hemsted
2017-11-14 15:59:31 +00:00
committed by GitHub
parent 62629e4eae 834eecd0f1
commit 27bf927f43
6 changed files with 189 additions and 140 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/errors.py
View File

@@ -94,7 +94,7 @@ def register_errors(blueprint):
current_app.logger.exception(e)
if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror and \
('duplicate key value violates unique constraint "services_name_key"' in e.orig.pgerror or
'duplicate key value violates unique constraint "services_email_from_key"' in e.orig.pgerror):
'duplicate key value violates unique constraint "services_email_from_key"' in e.orig.pgerror):
return jsonify(
result='error',
message={'name': ["Duplicate service name '{}'".format(

7
app/models.py
View File

@@ -9,7 +9,7 @@ from sqlalchemy.dialects.postgresql import (
UUID,
JSON
)
from sqlalchemy import UniqueConstraint, and_
from sqlalchemy import UniqueConstraint, CheckConstraint, and_
from sqlalchemy.orm import foreign, remote
from notifications_utils.recipients import (
validate_email_address,
@@ -97,7 +97,7 @@ class User(db.Model):
nullable=True,
onupdate=datetime.datetime.utcnow)
_password = db.Column(db.String, index=False, unique=False, nullable=False)
mobile_number = db.Column(db.String, index=False, unique=False, nullable=False)
mobile_number = db.Column(db.String, index=False, unique=False, nullable=True)
password_changed_at = db.Column(db.DateTime, index=False, unique=False, nullable=False,
default=datetime.datetime.utcnow)
logged_in_at = db.Column(db.DateTime, nullable=True)
@@ -107,6 +107,9 @@ class User(db.Model):
current_session_id = db.Column(UUID(as_uuid=True), nullable=True)
auth_type = db.Column(db.String, db.ForeignKey('auth_type.name'), index=True, nullable=False, default=SMS_AUTH_TYPE)
# either email auth or a mobile number must be provided
CheckConstraint("auth_type = 'email_auth' or mobile_number is not null")
services = db.relationship(
'Service',
secondary='user_to_service',

23
app/schemas.py
View File

@@ -105,6 +105,26 @@ class UserSchema(BaseSchema):
"_password", "verify_codes")
strict = True
@validates('name')
def validate_name(self, value):
if not value:
raise ValidationError('Invalid name')
@validates('email_address')
def validate_email_address(self, value):
try:
validate_email_address(value)
except InvalidEmailError as e:
raise ValidationError(str(e))
@validates('mobile_number')
def validate_mobile_number(self, value):
try:
if value is not None:
validate_phone_number(value, international=True)
except InvalidPhoneError as error:
raise ValidationError('Invalid phone number: {}'.format(error))
class UserUpdateAttributeSchema(BaseSchema):
auth_type = field_for(models.User, 'auth_type')
@@ -132,7 +152,8 @@ class UserUpdateAttributeSchema(BaseSchema):
@validates('mobile_number')
def validate_mobile_number(self, value):
try:
validate_phone_number(value, international=True)
if value is not None:
validate_phone_number(value, international=True)
except InvalidPhoneError as error:
raise ValidationError('Invalid phone number: {}'.format(error))

31
app/user/rest.py
View File

@@ -4,6 +4,7 @@ from datetime import datetime
from urllib.parse import urlencode
from flask import (jsonify, request, Blueprint, current_app, abort)
from sqlalchemy.exc import IntegrityError
from app.config import QueueNames
from app.dao.users_dao import (
@@ -52,6 +53,19 @@ user_blueprint = Blueprint('user', __name__)
register_errors(user_blueprint)
@user_blueprint.errorhandler(IntegrityError)
def handle_integrity_error(exc):
"""
Handle integrity errors caused by the auth type/mobile number check constraint
"""
if 'ck_users_mobile_or_email_auth' in str(exc):
# we don't expect this to trip, so still log error
current_app.logger.exception('Check constraint ck_users_mobile_or_email_auth triggered')
return jsonify(result='error', message='Mobile number must be set if auth_type is set to sms_auth'), 400
raise
@user_blueprint.route('', methods=['POST'])
def create_user():
user_to_create, errors = user_schema.load(request.get_json())
@@ -63,23 +77,6 @@ def create_user():
return jsonify(data=user_schema.dump(user_to_create).data), 201
@user_blueprint.route('/<uuid:user_id>', methods=['PUT'])
def update_user(user_id):
user_to_update = get_user_by_id(user_id=user_id)
req_json = request.get_json()
update_dct, errors = user_schema_load_json.load(req_json)
# TODO don't let password be updated in this PUT method (currently used by the forgot password flow)
pwd = req_json.get('password', None)
if pwd is not None:
if not pwd:
errors.update({'password': ['Invalid data for field']})
raise InvalidRequest(errors, status_code=400)
else:
reset_failed_login_count(user_to_update)
save_model_user(user_to_update, update_dict=update_dct, pwd=pwd)
return jsonify(data=user_schema.dump(user_to_update).data), 200
@user_blueprint.route('/<uuid:user_id>', methods=['POST'])
def update_user_attribute(user_id):
user_to_update = get_user_by_id(user_id=user_id)

28
migrations/versions/0136_user_mobile_nullable.py Normal file
View File

@@ -0,0 +1,28 @@
"""
Revision ID: 0136_user_mobile_nullable
Revises: 0135_stats_template_usage
Create Date: 2017-11-08 11:49:05.773974
"""
from alembic import op
import sqlalchemy as sa
from sqlalchemy.sql import column
from sqlalchemy.dialects import postgresql
revision = '0136_user_mobile_nullable'
down_revision = '0135_stats_template_usage'
def upgrade():
op.alter_column('users', 'mobile_number', nullable=True)
op.create_check_constraint(
'ck_users_mobile_or_email_auth',
'users',
"auth_type = 'email_auth' or mobile_number is not null"
)
def downgrade():
op.alter_column('users', 'mobile_number', nullable=False)
op.drop_constraint('ck_users_mobile_or_email_auth', 'users')

238
tests/app/user/test_rest.py
View File

@@ -159,40 +159,53 @@ def test_create_user_missing_attribute_password(client, notify_db, notify_db_ses
assert {'password': ['Missing data for required field.']} == json_resp['message']
def test_put_user(client, sample_service):
"""
Tests PUT endpoint '/' to update a user.
"""
assert User.query.count() == 1
sample_user = sample_service.users[0]
sample_user.failed_login_count = 1
new_email = 'new@digital.cabinet-office.gov.uk'
def test_can_create_user_with_email_auth_and_no_mobile(admin_request, notify_db_session):
data = {
'name': sample_user.name,
'email_address': new_email,
'mobile_number': sample_user.mobile_number
'name': 'Test User',
'email_address': 'user@digital.cabinet-office.gov.uk',
'password': 'password',
'mobile_number': None,
'auth_type': EMAIL_AUTH_TYPE
}
auth_header = create_authorization_header()
headers = [('Content-Type', 'application/json'), auth_header]
resp = client.put(
url_for('user.update_user', user_id=sample_user.id),
data=json.dumps(data),
headers=headers)
assert resp.status_code == 200
assert User.query.count() == 1
json_resp = json.loads(resp.get_data(as_text=True))
assert json_resp['data']['email_address'] == new_email
expected_permissions = default_service_permissions
fetched = json_resp['data']
assert str(sample_user.id) == fetched['id']
assert sample_user.name == fetched['name']
assert sample_user.mobile_number == fetched['mobile_number']
assert new_email == fetched['email_address']
assert sample_user.state == fetched['state']
assert sorted(expected_permissions) == sorted(fetched['permissions'][str(sample_service.id)])
# password wasn't updated, so failed_login_count stays the same
assert sample_user.failed_login_count == 1
json_resp = admin_request.post('user.create_user', _data=data, _expected_status=201)
assert json_resp['data']['auth_type'] == EMAIL_AUTH_TYPE
assert json_resp['data']['mobile_number'] is None
def test_cannot_create_user_with_sms_auth_and_no_mobile(admin_request, notify_db_session):
data = {
'name': 'Test User',
'email_address': 'user@digital.cabinet-office.gov.uk',
'password': 'password',
'mobile_number': None,
'auth_type': SMS_AUTH_TYPE
}
json_resp = admin_request.post('user.create_user', _data=data, _expected_status=400)
assert json_resp['message'] == 'Mobile number must be set if auth_type is set to sms_auth'
def test_cannot_create_user_with_empty_strings(admin_request, notify_db_session):
data = {
'name': '',
'email_address': '',
'password': 'password',
'mobile_number': '',
'auth_type': EMAIL_AUTH_TYPE
}
resp = admin_request.post(
'user.create_user',
_data=data,
_expected_status=400
)
assert resp['message'] == {
'email_address': ['Not a valid email address'],
'mobile_number': ['Invalid phone number: Not enough digits'],
'name': ['Invalid name']
}
@pytest.mark.parametrize('user_attribute, user_value', [
@@ -218,63 +231,6 @@ def test_post_user_attribute(client, sample_user, user_attribute, user_value):
assert json_resp['data'][user_attribute] == user_value
def test_put_user_update_password(client, sample_service):
"""
Tests PUT endpoint '/' to update a user including their password.
"""
assert User.query.count() == 1
sample_user = sample_service.users[0]
new_password = '1234567890'
data = {
'name': sample_user.name,
'email_address': sample_user.email_address,
'mobile_number': sample_user.mobile_number,
'password': new_password
}
auth_header = create_authorization_header()
headers = [('Content-Type', 'application/json'), auth_header]
resp = client.put(
url_for('user.update_user', user_id=sample_user.id),
data=json.dumps(data),
headers=headers)
assert resp.status_code == 200
assert User.query.count() == 1
json_resp = json.loads(resp.get_data(as_text=True))
assert json_resp['data']['password_changed_at'] is not None
data = {'password': new_password}
auth_header = create_authorization_header()
headers = [('Content-Type', 'application/json'), auth_header]
resp = client.post(
url_for('user.verify_user_password', user_id=str(sample_user.id)),
data=json.dumps(data),
headers=headers)
assert resp.status_code == 204
def test_put_user_not_exists(client, sample_user, fake_uuid):
"""
Tests PUT endpoint '/' to update a user doesn't exist.
"""
assert User.query.count() == 1
new_email = 'new@digital.cabinet-office.gov.uk'
data = {'email_address': new_email}
auth_header = create_authorization_header()
headers = [('Content-Type', 'application/json'), auth_header]
resp = client.put(
url_for('user.update_user', user_id=fake_uuid),
data=json.dumps(data),
headers=headers)
assert resp.status_code == 404
assert User.query.count() == 1
user = User.query.filter_by(id=str(sample_user.id)).first()
json_resp = json.loads(resp.get_data(as_text=True))
assert json_resp['result'] == "error"
assert json_resp['message'] == 'No result found'
assert user == sample_user
assert user.email_address != new_email
def test_get_user_by_email(client, sample_service):
sample_user = sample_service.users[0]
header = create_authorization_header()
@@ -529,37 +485,6 @@ def test_update_user_password_saves_correctly(client, sample_service):
assert resp.status_code == 204
def test_update_user_resets_failed_login_count_if_updating_password(client, sample_service):
user = sample_service.users[0]
user.failed_login_count = 1
resp = client.put(
url_for('user.update_user', user_id=user.id),
data=json.dumps({
'name': user.name,
'email_address': user.email_address,
'mobile_number': user.mobile_number,
'password': 'foo'
}),
headers=[('Content-Type', 'application/json'), create_authorization_header()]
)
assert resp.status_code == 200
assert user.failed_login_count == 0
def test_update_user_auth_type(admin_request, sample_user):
assert sample_user.auth_type == 'sms_auth'
resp = admin_request.post(
'user.update_user_attribute',
user_id=sample_user.id,
_data={'auth_type': 'email_auth'},
)
assert resp['data']['id'] == str(sample_user.id)
assert resp['data']['auth_type'] == 'email_auth'
def test_activate_user(admin_request, sample_user):
sample_user.state = 'pending'
@@ -574,3 +499,78 @@ def test_activate_user_fails_if_already_active(admin_request, sample_user):
resp = admin_request.post('user.activate_user', user_id=sample_user.id, _expected_status=400)
assert resp['message'] == 'User already active'
assert sample_user.state == 'active'
def test_update_user_auth_type(admin_request, sample_user):
assert sample_user.auth_type == 'sms_auth'
resp = admin_request.post(
'user.update_user_attribute',
user_id=sample_user.id,
_data={'auth_type': 'email_auth'},
)
assert resp['data']['id'] == str(sample_user.id)
assert resp['data']['auth_type'] == 'email_auth'
def test_can_set_email_auth_and_remove_mobile_at_same_time(admin_request, sample_user):
sample_user.auth_type = SMS_AUTH_TYPE
admin_request.post(
'user.update_user_attribute',
user_id=sample_user.id,
_data={
'mobile_number': None,
'auth_type': EMAIL_AUTH_TYPE,
}
)
assert sample_user.mobile_number is None
assert sample_user.auth_type == EMAIL_AUTH_TYPE
def test_cannot_remove_mobile_if_sms_auth(admin_request, sample_user):
sample_user.auth_type = SMS_AUTH_TYPE
json_resp = admin_request.post(
'user.update_user_attribute',
user_id=sample_user.id,
_data={'mobile_number': None},
_expected_status=400
)
assert json_resp['message'] == 'Mobile number must be set if auth_type is set to sms_auth'
def test_can_remove_mobile_if_email_auth(admin_request, sample_user):
sample_user.auth_type = EMAIL_AUTH_TYPE
admin_request.post(
'user.update_user_attribute',
user_id=sample_user.id,
_data={'mobile_number': None},
)
assert sample_user.mobile_number is None
def test_cannot_update_user_with_mobile_number_as_empty_string(admin_request, sample_user):
sample_user.auth_type = EMAIL_AUTH_TYPE
resp = admin_request.post(
'user.update_user_attribute',
user_id=sample_user.id,
_data={'mobile_number': ''},
_expected_status=400
)
assert resp['message']['mobile_number'] == ['Invalid phone number: Not enough digits']
def test_cannot_update_user_password_using_attributes_method(admin_request, sample_user):
resp = admin_request.post(
'user.update_user_attribute',
user_id=sample_user.id,
_data={'password': 'foo'},
_expected_status=400
)
assert resp['message']['_schema'] == ['Unknown field name password']