diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 19cd4089a..a3e87f275 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -9,7 +9,10 @@ def create_secret_code(): return ''.join(map(str, random.sample(range(9), 5))) -def save_model_user(usr, update_dict={}): +def save_model_user(usr, update_dict={}, pwd=None): + if pwd: + usr.password = pwd + usr.password_changed_at = datetime.now() if update_dict: if update_dict.get('id'): del update_dict['id'] diff --git a/app/schemas.py b/app/schemas.py index a6e6c1377..2b701f9b5 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -1,5 +1,6 @@ from . import ma from . import models +from marshmallow import post_load # TODO I think marshmallow provides a better integration and error handling. # Would be better to replace functionality in dao with the marshmallow supported @@ -8,12 +9,28 @@ from . import models class UserSchema(ma.ModelSchema): + + def __init__(self, *args, load_json=False, **kwargs): + self.load_json = load_json + super(UserSchema, self).__init__(*args, **kwargs) + class Meta: model = models.User exclude = ( "updated_at", "created_at", "user_to_service", "_password", "verify_codes") + @post_load + def make_instance(self, data): + """Deserialize data to an instance of the model. Update an existing row + if specified in `self.instance` or loaded by primary key(s) in the data; + else create a new row. + :param data: Data to deserialize. + """ + if self.load_json: + return data + return super(UserSchema, self).make_instance(data) + # TODO process users list, to return a list of user.id # Should that list be restricted by the auth parsed?? @@ -47,6 +64,7 @@ class VerifyCodeSchema(ma.ModelSchema): user_schema = UserSchema() +user_schema_load_json = UserSchema(load_json=True) users_schema = UserSchema(many=True) service_schema = ServiceSchema() services_schema = ServiceSchema(many=True) diff --git a/app/user/rest.py b/app/user/rest.py index 325253fd0..79321099a 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -7,7 +7,8 @@ from app.dao.users_dao import ( get_model_users, save_model_user, delete_model_user, create_user_code, get_user_code, use_user_code, increment_failed_login_count) from app.schemas import ( - user_schema, users_schema, service_schema, services_schema, verify_code_schema) + user_schema, users_schema, service_schema, services_schema, + verify_code_schema, user_schema_load_json) from app import db, notify_alpha_client from flask import Blueprint @@ -20,14 +21,12 @@ def create_user(): user, errors = user_schema.load(request.get_json()) req_json = request.get_json() # TODO password policy, what is valid password - if not req_json.get('password'): - errors = {'password': ['Missing data for required field.']} + if not req_json.get('password', None): + errors.update({'password': ['Missing data for required field.']}) return jsonify(result="error", message=errors), 400 if errors: return jsonify(result="error", message=errors), 400 - - user.password = req_json.get('password') - save_model_user(user) + save_model_user(user, pwd=req_json.get('password')) return jsonify(data=user_schema.dump(user).data), 201 @@ -43,11 +42,17 @@ def update_user(user_id): status_code = 202 delete_model_user(user) else: - # TODO removed some validation checking by using load - # which will need to be done in another way + req_json = request.get_json() + update_dct, errors = user_schema_load_json.load(req_json) + pwd = req_json.get('password', None) + # TODO password validation, it is already done on the admin app + # but would be good to have the same validation here. + if pwd is not None and not pwd: + errors.update({'password': ['Invalid data for field']}) + if errors: + return jsonify(result="error", message=errors), 400 status_code = 200 - db.session.rollback() - save_model_user(user, update_dict=request.get_json()) + save_model_user(user, update_dict=update_dct, pwd=pwd) return jsonify(data=user_schema.dump(user).data), status_code @@ -67,7 +72,7 @@ def verify_user_password(user_id): result="error", message={'password': ['Required field missing data']}), 400 if user.check_password(txt_pwd): - return jsonify(''), 204 + return jsonify({}), 204 else: increment_failed_login_count(user) return jsonify(result='error', message={'password': ['Incorrect password']}), 400 @@ -101,7 +106,7 @@ def verify_user_code(user_id): if datetime.now() > code.expiry_datetime or code.code_used: return jsonify(result="error", message="Code has expired"), 400 use_user_code(code.id) - return jsonify(''), 204 + return jsonify({}), 204 @user.route('//code', methods=['POST']) @@ -138,7 +143,7 @@ def send_user_code(user_id): 'Verification code') else: abort(500) - return jsonify(''), 204 + return jsonify({}), 204 @user.route('/', methods=['GET']) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 729fcf7ab..e49076503 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -164,7 +164,9 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_user, sample_ assert User.query.count() == 2 new_email = 'new@digital.cabinet-office.gov.uk' data = { - 'email_address': new_email + 'name': sample_user.name, + 'email_address': new_email, + 'mobile_number': sample_user.mobile_number } auth_header = create_authorization_header(service_id=sample_admin_service_id, path=url_for('user.update_user', user_id=sample_user.id), @@ -193,6 +195,50 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_user, sample_ assert json_resp['data']['email_address'] == new_email +def test_put_user_update_password(notify_api, + notify_db, + notify_db_session, + sample_user, + sample_admin_service_id): + """ + Tests PUT endpoint '/' to update a user including their password. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert User.query.count() == 2 + 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(service_id=sample_admin_service_id, + path=url_for('user.update_user', user_id=sample_user.id), + method='PUT', + request_body=json.dumps(data)) + 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() == 2 + 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(service_id=sample_admin_service_id, + path=url_for('user.verify_user_password', user_id=sample_user.id), + method='POST', + request_body=json.dumps(data)) + headers = [('Content-Type', 'application/json'), auth_header] + resp = client.post( + url_for('user.verify_user_password', user_id=sample_user.id), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 204 + + def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): """ Tests PUT endpoint '/' to update a user doesn't exist.