Merge pull request #39 from alphagov/add_password_update

Added support for allowing password to updated from the PUT request t…

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']

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)

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('/<int:user_id>/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('/<int:user_id>', methods=['GET'])

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.