Skip to content

Conversation

gbunkoczi
Copy link

In the current implementation, SQLAchemyBase.resolve_id uses simple string conversion to convert object primary keys into string format, and SQLAlchemyBase.get_node passes the obtained key to a SQLAlchemy session to retrieve the object. However, this does not work for composite primary keys, as the string would first need to be converted back to a tuple. While this can be done with eval, this would be a security hole, since it would allow execution of arbitrary code, and ids are normally received as untrusted output.

The simplest and relatively general solution for the above would be to use the json module to serialize and deserialize the keys. However, not all allowed primary keys are json-serializable (e.g. DATETIME), so additional functionality may be required for a general case.

Instead of trying to provide a general solution for the above, the patch abstracts serialization of primary keys to a class attribute that can be configured based on the needs of the SQLAlchemy schema on a per-table basis. Schemas that do not contain tables with composite keys can continue to use the current implementation, which is provided as default.

As an example, to handle classes with json-serializable composite primary keys, the following configuration would be necessary:

import json
from graphene_sqlalchemy.types import SQLAlchemyObjectType, SQLAlchemyPrimaryKeySerializer

serializer = SQLAlchemyPrimaryKeySerializer(
    serialize=json.dumps,
    deserialize=json.loads, # this could be replaced with a wrapper that converts json.decoder.JSONDecodeError to ValueError
)  

...

class MyGrapheneClass(SQLAlchemyObjectType):
    class Meta(object):
        model=MySQLAlchemyModel
        interfaces=(Node,)
        serializer=serializer

I am open to suggestions on code restructuring, and will add tests to exercise the functionality.

- serialize/deserialize keys with json module as opposed to simple string conversion
- introduce serializer attribute to SQLAlchemyBase to allow customisation
- use graphene_type to access serializer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant