Replace how .load is called

https://marshmallow.readthedocs.io/en/stable/upgrading.html#schemas-are-always-strict

`.load` doesn't return a `(data, errors)` tuple any more - only data is
returned. A `ValidationError` is raised if validation fails. The code
now relies on the `marshmallow_validation_error` error handler to handle
errors instead of having to raise an `InvalidRequest`. This has no
effect on the response that is returned (a test has been modified to
check).

Also added a new `password` field to the `UserSchema` so that we don't
have to specially check for password errors in the `.create_user` endpoint
- we can let marshmallow handle them.

app/events/rest.py

@@ -11,6 +11,6 @@ register_errors(events)
 @events.route('', methods=['POST'])
 def create_event():
     data = request.get_json()
-    event = event_schema.load(data).data
+    event = event_schema.load(data)
     dao_create_event(event)
     return jsonify(data=event_schema.dump(event).data), 201

app/job/rest.py

@@ -79,7 +79,7 @@ def cancel_letter_job(service_id, job_id):
 
 @job_blueprint.route('/<job_id>/notifications', methods=['GET'])
 def get_all_notifications_for_service_job(service_id, job_id):
-    data = notifications_filter_schema.load(request.args).data
+    data = notifications_filter_schema.load(request.args)
     page = data['page'] if 'page' in data else 1
     page_size = data['page_size'] if 'page_size' in data else current_app.config.get('PAGE_SIZE')
     paginated_notifications = get_notifications_for_job(
@@ -173,7 +173,7 @@ def create_job(service_id):
 
     data.update({"template_version": template.version})
 
-    job = job_schema.load(data).data
+    job = job_schema.load(data)
 
     if job.scheduled_for:
         job.job_status = JOB_STATUS_SCHEDULED

app/notifications/rest.py

@@ -48,7 +48,8 @@ def get_notification_by_id(notification_id):
 
 @notifications.route('/notifications', methods=['GET'])
 def get_all_notifications():
-    data = notifications_filter_schema.load(request.args).data
+    data = notifications_filter_schema.load(request.args)
+
     include_jobs = data.get('include_jobs', False)
     page = data.get('page', 1)
     page_size = data.get('page_size', current_app.config.get('API_PAGE_SIZE'))
@@ -83,13 +84,10 @@ def send_notification(notification_type):
         msg = msg + ", please use the latest version of the client" if notification_type == LETTER_TYPE else msg
         raise InvalidRequest(msg, 400)
 
-    notification_form, errors = (
+    notification_form = (
         sms_template_notification_schema if notification_type == SMS_TYPE else email_notification_schema
     ).load(request.get_json())
 
-    if errors:
-        raise InvalidRequest(errors, status_code=400)
-
     check_rate_limiting(authenticated_service, api_user)
 
     template, template_with_content = validate_template(

app/schemas.py

@@ -88,6 +88,7 @@ class UserSchema(BaseSchema):
     password_changed_at = field_for(models.User, 'password_changed_at', format=DATETIME_FORMAT_NO_TIMEZONE)
     created_at = field_for(models.User, 'created_at', format=DATETIME_FORMAT_NO_TIMEZONE)
     auth_type = field_for(models.User, 'auth_type')
+    password = fields.String(required=True, load_only=True)
 
     def user_permissions(self, usr):
         retval = {}

app/service/rest.py

@@ -274,7 +274,7 @@ def update_service(service_id):
     current_data = dict(service_schema.dump(fetched_service).data.items())
     current_data.update(request.get_json())
 
-    service = service_schema.load(current_data).data
+    service = service_schema.load(current_data)
 
     if 'email_branding' in req_json:
         email_branding_id = req_json['email_branding']
@@ -301,7 +301,7 @@ def update_service(service_id):
 @service_blueprint.route('/<uuid:service_id>/api-key', methods=['POST'])
 def create_api_key(service_id=None):
     fetched_service = dao_fetch_service_by_id(service_id=service_id)
-    valid_api_key = api_key_schema.load(request.get_json()).data
+    valid_api_key = api_key_schema.load(request.get_json())
     valid_api_key.service = fetched_service
     save_model_api_key(valid_api_key)
     unsigned_api_key = get_unsigned_secret(valid_api_key.id)
@@ -408,11 +408,11 @@ def get_service_history(service_id):
 @service_blueprint.route('/<uuid:service_id>/notifications', methods=['GET', 'POST'])
 def get_all_notifications_for_service(service_id):
     if request.method == 'GET':
-        data = notifications_filter_schema.load(request.args).data
+        data = notifications_filter_schema.load(request.args)
     elif request.method == 'POST':
         # Must transform request.get_json() to MultiDict as NotificationsFilterSchema expects a MultiDict.
         # Unlike request.args, request.get_json() does not return a MultiDict but instead just a dict.
-        data = notifications_filter_schema.load(MultiDict(request.get_json())).data
+        data = notifications_filter_schema.load(MultiDict(request.get_json()))
 
     if data.get('to'):
         notification_type = data.get('template_type')[0] if data.get('template_type') else None
@@ -772,7 +772,8 @@ def get_email_reply_to_address(service_id, reply_to_id):
 
 @service_blueprint.route('/<uuid:service_id>/email-reply-to/verify', methods=['POST'])
 def verify_reply_to_email_address(service_id):
-    email_address, errors = email_data_request_schema.load(request.get_json())
+    email_address = email_data_request_schema.load(request.get_json())
+
     check_if_reply_to_address_already_in_use(service_id, email_address["email"])
     template = dao_get_template_by_id(current_app.config['REPLY_TO_EMAIL_ADDRESS_VERIFICATION_TEMPLATE_ID'])
     notify_service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID'])

app/service_invite/rest.py

@@ -26,7 +26,7 @@ register_errors(service_invite)
 @service_invite.route('/service/<service_id>/invite', methods=['POST'])
 def create_invited_user(service_id):
     request_json = request.get_json()
-    invited_user, errors = invited_user_schema.load(request_json)
+    invited_user = invited_user_schema.load(request_json)
     save_invited_user(invited_user)
 
     if invited_user.service.has_permission(BROADCAST_TYPE):
@@ -79,7 +79,7 @@ def update_invited_user(service_id, invited_user_id):
 
     current_data = dict(invited_user_schema.dump(fetched).data.items())
     current_data.update(request.get_json())
-    update_dict = invited_user_schema.load(current_data).data
+    update_dict = invited_user_schema.load(current_data)
     save_invited_user(update_dict)
     return jsonify(data=invited_user_schema.dump(fetched).data), 200
 

app/template/rest.py

@@ -156,7 +156,7 @@ def update_template(service_id, template_id):
         errors = {'content': [message]}
         raise InvalidRequest(errors, status_code=400)
 
-    update_dict = template_schema.load(updated_template).data
+    update_dict = template_schema.load(updated_template)
     if update_dict.archived:
         update_dict.folder = None
     dao_update_template(update_dict)

app/user/rest.py

@@ -86,11 +86,9 @@ def handle_integrity_error(exc):
 
 @user_blueprint.route('', methods=['POST'])
 def create_user():
-    user_to_create, errors = create_user_schema.load(request.get_json())
     req_json = request.get_json()
-    if not req_json.get('password', None):
+    user_to_create = create_user_schema.load(req_json)
-        errors.update({'password': ['Missing data for required field.']})
+
-        raise InvalidRequest(errors, status_code=400)
     save_model_user(user_to_create, password=req_json.get('password'), validated_email_access=True)
     result = user_to_create.serialize()
     return jsonify(data=result), 201
@@ -105,9 +103,8 @@ def update_user_attribute(user_id):
     else:
         updated_by = None
 
-    update_dct, errors = user_update_schema_load_json.load(req_json)
+    update_dct = user_update_schema_load_json.load(req_json)
-    if errors:
+
-        raise InvalidRequest(errors, status_code=400)
     save_user_attribute(user_to_update, update_dict=update_dct)
     if updated_by:
         if 'email_address' in update_dct:
@@ -345,9 +342,8 @@ def create_2fa_code(template_id, user_to_send_to, secret_code, recipient, person
 @user_blueprint.route('/<uuid:user_id>/change-email-verification', methods=['POST'])
 def send_user_confirm_new_email(user_id):
     user_to_send_to = get_user_by_id(user_id=user_id)
-    email, errors = email_data_request_schema.load(request.get_json())
+
-    if errors:
+    email = email_data_request_schema.load(request.get_json())
-        raise InvalidRequest(message=errors, status_code=400)
 
     template = dao_get_template_by_id(current_app.config['CHANGE_EMAIL_CONFIRMATION_TEMPLATE_ID'])
     service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID'])
@@ -407,7 +403,8 @@ def send_new_user_email_verification(user_id):
 
 @user_blueprint.route('/<uuid:user_id>/email-already-registered', methods=['POST'])
 def send_already_registered_email(user_id):
-    to, errors = email_data_request_schema.load(request.get_json())
+    to = email_data_request_schema.load(request.get_json())
+
     template = dao_get_template_by_id(current_app.config['ALREADY_REGISTERED_EMAIL_TEMPLATE_ID'])
     service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID'])
 
@@ -472,10 +469,7 @@ def set_permissions(user_id, service_id):
 
 @user_blueprint.route('/email', methods=['POST'])
 def fetch_user_by_email():
-
+    email = email_data_request_schema.load(request.get_json())
-    email, errors = email_data_request_schema.load(request.get_json())
-    if errors:
-        raise InvalidRequest(message=errors, status_code=400)
 
     fetched_user = get_user_by_email(email['email'])
     result = fetched_user.serialize()
@@ -496,7 +490,8 @@ def get_by_email():
 
 @user_blueprint.route('/find-users-by-email', methods=['POST'])
 def find_users_by_email():
-    email, errors = partial_email_data_request_schema.load(request.get_json())
+    email = partial_email_data_request_schema.load(request.get_json())
+
     fetched_users = get_users_by_partial_email(email['email'])
     result = [user.serialize_for_users_list() for user in fetched_users]
     return jsonify(data=result), 200
@@ -505,7 +500,8 @@ def find_users_by_email():
 @user_blueprint.route('/reset-password', methods=['POST'])
 def send_user_reset_password():
     request_json = request.get_json()
-    email, errors = email_data_request_schema.load(request_json)
+    email = email_data_request_schema.load(request_json)
+
     user_to_send_to = get_user_by_email(email['email'])
     template = dao_get_template_by_id(current_app.config['PASSWORD_RESET_TEMPLATE_ID'])
     service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID'])
@@ -538,9 +534,9 @@ def update_password(user_id):
     user = get_user_by_id(user_id=user_id)
     req_json = request.get_json()
     password = req_json.get('_password')
-    update_dct, errors = user_update_password_schema_load_json.load(req_json)
+
-    if errors:
+    user_update_password_schema_load_json.load(req_json)
-        raise InvalidRequest(errors, status_code=400)
+
     update_user_password(user, password)
     return jsonify(data=user.serialize()), 200
 

tests/app/test_schemas.py

@@ -64,7 +64,7 @@ def test_user_update_schema_accepts_valid_attribute_pairs(user_attribute, user_v
     }
     from app.schemas import user_update_schema_load_json
 
-    data, errors = user_update_schema_load_json.load(update_dict)
+    errors = user_update_schema_load_json.validate(update_dict)
     assert not errors
 
 
@@ -81,7 +81,7 @@ def test_user_update_schema_rejects_invalid_attribute_pairs(user_attribute, user
     }
 
     with pytest.raises(ValidationError):
-        data, errors = user_update_schema_load_json.load(update_dict)
+        user_update_schema_load_json.load(update_dict)
 
 
 @pytest.mark.parametrize('user_attribute', [
@@ -96,7 +96,7 @@ def test_user_update_schema_rejects_disallowed_attribute_keys(user_attribute):
     from app.schemas import user_update_schema_load_json
 
     with pytest.raises(ValidationError) as excinfo:
-        data, errors = user_update_schema_load_json.load(update_dict)
+        user_update_schema_load_json.load(update_dict)
 
     assert excinfo.value.messages['_schema'][0] == 'Unknown field name {}'.format(user_attribute)
 

tests/app/user/test_rest.py

@@ -116,6 +116,7 @@ def test_post_user(admin_request, notify_db_session):
     json_resp = admin_request.post('user.create_user', _data=data, _expected_status=201)
 
     user = User.query.filter_by(email_address='user@digital.cabinet-office.gov.uk').first()
+    assert user.check_password("password")
     assert json_resp['data']['email_address'] == user.email_address
     assert json_resp['data']['id'] == str(user.id)
     assert user.auth_type == EMAIL_AUTH_TYPE
@@ -888,7 +889,7 @@ def test_cannot_update_user_password_using_attributes_method(admin_request, samp
         _data={'password': 'foo'},
         _expected_status=400
     )
-    assert resp['message']['_schema'] == ['Unknown field name password']
+    assert resp == {'message': {'_schema': ['Unknown field name password']}, 'result': 'error'}
 
 
 def test_get_orgs_and_services_nests_services(admin_request, sample_user):