search

kvesteri / sqlalchemy-continuum

Versioning extension for SQLAlchemy.
BSD 3-Clause "New" or "Revised" License
578 stars 127 forks source link

Revert "FlaskPlugin: use `get_id()` instead of `id` attr" #325

Closed anthraxx closed 1 year ago

anthraxx commented 1 year ago

This reverts commit 7eda52765ee8bb1c1c5ff193fc3a7ee032e5323f.

This was actually a misinterpretation of the API:

The API expects get_id() to be a unique string, that may or may not be an integer. This feature is only used to uniquely identify the user, but its not the same as the user's primary key. https://flask-login.readthedocs.io/en/latest/#your-user-class

Please take a note on how get_id() is expected to behave on this feature of flask-login: https://flask-login.readthedocs.io/en/latest/#alternative-tokens

This means it can return any arbitrary string to identify the user which can be swapped to any other value to invalidate all sessions. This explicitly states that it is not the same as the user's primary id which is also used as foreign key on the table.

sqlalchemy-continuum's transaction tables reference the user with a foreign key of the actual id. If a downstream application uses the alternative-tokens feature as described by the flask-login documentation, this breaks horribly as sqlalchemy-continuum will then try to insert an arbitrary unique string of the user as the foreign key to reference the user in the user table.

Fixes #316 Caused by #149

marksteward commented 1 year ago

Thanks for this! I've fixed only the get_id part in 6c4dfc2 and released as 1.3.14.