0

This is only my second task (bug I need to fix) in a Python\Flask\SQLAlchemy\Marshmallow system I need to work on. So please try to be easy with me :)

In short: I'd like to approve an apparently invalid request.

In details:

I need to handle a case in which a user might send a request with some json in which he included by mistake a duplicate value in a list.

For example:

{
    "ciphers": [
        "TLS_AES_256_GCM_SHA384",
        "AES256-SHA256"
    ],
    "is_default": true,
    "tls_versions": [
        "tls10",
        "tls10",
        "tls11",
    ]
}

What I need to do is to eliminate one of the duplicated tls1.0 values, but consider the request as valid, update the db with the correct and distinct tls versions, and in the response return the non duplicated json in body.

Current code segments are as follows:

tls Controller:

...
@client_side_tls_bp.route('/<string:tls_profile_id>', methods=['PUT'])
def update_tls_profile_by_id(tls_profile_id):
    return update_entity_by_id(TlsProfileOperator, entity_name, tls_profile_id)
...

general entity controller:

...
def update_entity_by_id(operator, entity_name, entity_id):
    """flask route for updating a resource"""
    try:
        entity_body = request.get_json()
    except Exception:
        return make_custom_response("Bad Request", HTTPStatus.BAD_REQUEST)

    entity_obj = operator.get(g.tenant, entity_id, g.correlation)
    if not entity_obj:
        response = make_custom_response(http_not_found_message(entity_name, entity_id), HTTPStatus.NOT_FOUND)
    else:
        updated = operator.update(g.tenant, entity_id, entity_body, g.correlation)
        if updated == "accepted":
            response = make_custom_response("Accepted", HTTPStatus.ACCEPTED)
        else:
            response = make_custom_response(updated, HTTPStatus.OK)

    return response
...

tls operator:

...
@staticmethod
def get(tenant, name, correlation_id=None):
    try:
        tls_profile = TlsProfile.get_by_name(tenant, name)
        return schema.dump(tls_profile)
    except NoResultFound:
        return None
    except Exception:
        apm_logger.error(f"Failed to get {name} TLS profile", tenant=tenant,
                         consumer=LogConsumer.customer, correlation=correlation_id)
        raise

@staticmethod
def update(tenant, name, json_data, correlation_id=None):

    schema.load(json_data)

    try:
        dependant_vs_names = VirtualServiceOperator.get_dependant_vs_names_locked_by_client_side_tls(tenant, name)
        # locks virtual services and tls profile table simultaneously

        to_update = TlsProfile.get_by_name(tenant, name)
        to_update.update(json_data, commit=False)
        db.session.flush()  # TODO - need to change when 2 phase commit will be implemented
        snapshots = VirtualServiceOperator.get_snapshots_dict(tenant, dependant_vs_names)

        # update QWE
        # TODO handle QWE update atomically!
        for snapshot in snapshots:
            QWEController.update_abc_services(tenant, correlation_id, snapshot)

        db.session.commit()
        apm_logger.info(f"Update successfully {len(dependant_vs_names)} virtual services", tenant=tenant,
                        correlation=correlation_id)
        return schema.dump(to_update)
    except Exception:
        db.session.rollback()
        apm_logger.error(f"Failed to update {name} TLS profile", tenant=tenant,
                         consumer=LogConsumer.customer, correlation=correlation_id)
        raise
...

and in the api schema class:

...
@validates('_tls_versions')
def validate_client_side_tls_versions(self, value):
    if len(noDuplicatatesList) < 1:
        raise ValidationError("At least a single TLS version must be provided")
    for tls_version in noDuplicatatesList:
        if tls_version not in TlsProfile.allowed_tls_version_values:
            raise ValidationError("Not a valid TLS version")
...

I would have prefer to solve the problem in the schema level, so it won't accept the duplication.

So, as easy as it is to remove the duplication from the "value" parameter value, how can I propagate the non duplicates list back in order to use it to update the db and the response?

Thanks.

dushkin
  • 1,939
  • 3
  • 37
  • 82

1 Answers1

1

I didn't test but I think mutating value in the validation function would work.

However, this is not really guaranteed by marshmallow's API.

The proper way to do it would be to add a post_load method to de-duplicate.

    @post_load
    def deduplicate_tls(self, data, **kwargs):
        if "tls_versions" in data:
            data["tls_version"] = list(set(data["tls_version"]))
        return data

This won't maintain the order, so if the order matters, or for issues related to deduplication itself, see https://stackoverflow.com/a/7961390/4653485.

Jérôme
  • 13,328
  • 7
  • 56
  • 106
  • Thank you Jerome! Looks like a very interesting direction. I will check it tomorrow morning at office. – dushkin Jun 21 '20 at 19:43
  • I added the post_load method successfully, but I don't see that the original data is being updated outside this method. Meaning, the json_data passed to schema.load(json_data) is left unchanged. And I want it to be without duplications. Is there a way to achieve that? – dushkin Jun 22 '20 at 05:51
  • Jerome, the ultimate solution was eventually to use pre_load instead of post_load. Using the pre_load updated the json_data as I need. You can update your answer and I'l happily accept it has the question answer. – dushkin Jun 22 '20 at 06:22
  • It should work with `post_load`. Using `pre_load` means doing it before validation, so you have to handle all potential issues -> duplicate validation to avoid a crash. – Jérôme Jun 22 '20 at 12:42