Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlaskPlugin: use get_id() method instead of id attribute #149

Closed
jirikuncar opened this issue Nov 8, 2016 · 2 comments · Fixed by #150
Closed

FlaskPlugin: use get_id() method instead of id attribute #149

jirikuncar opened this issue Nov 8, 2016 · 2 comments · Fixed by #150

Comments

@jirikuncar
Copy link
Contributor

Following Flask-Login documentation we should use current_user.get_id() instead of current_user.id. See https://flask-login.readthedocs.io/en/latest/#your-user-class

jirikuncar added a commit to jirikuncar/sqlalchemy-continuum that referenced this issue Nov 8, 2016
Following Flask-Login documentation we should use current_user.get_id()
instead of current_user.id.  (closes kvesteri#149)
https://flask-login.readthedocs.io/en/latest/#your-user-class

Signed-off-by: Jiri Kuncar <[email protected]>
jirikuncar added a commit to jirikuncar/sqlalchemy-continuum that referenced this issue Nov 8, 2016
Following Flask-Login documentation we should use current_user.get_id()
instead of current_user.id.  (closes kvesteri#149)
https://flask-login.readthedocs.io/en/latest/#your-user-class

Signed-off-by: Jiri Kuncar <[email protected]>
marksteward pushed a commit that referenced this issue Feb 10, 2022
Following Flask-Login documentation we should use current_user.get_id()
instead of current_user.id.  (closes #149)
https://flask-login.readthedocs.io/en/latest/#your-user-class

Signed-off-by: Jiri Kuncar <[email protected]>
marksteward pushed a commit that referenced this issue Feb 10, 2022
Following Flask-Login documentation we should use current_user.get_id()
instead of current_user.id.  (closes #149)
https://flask-login.readthedocs.io/en/latest/#your-user-class

Signed-off-by: Jiri Kuncar <[email protected]>
AbdealiLoKo pushed a commit to AbdealiLoKo/sqlalchemy-continuum that referenced this issue Aug 23, 2022
Following Flask-Login documentation we should use current_user.get_id()
instead of current_user.id.  (closes kvesteri#149)
https://flask-login.readthedocs.io/en/latest/#your-user-class

Signed-off-by: Jiri Kuncar <[email protected]>
indiVar0508 pushed a commit to indiVar0508/sqlalchemy-continuum that referenced this issue Sep 11, 2022
Following Flask-Login documentation we should use current_user.get_id()
instead of current_user.id.  (closes kvesteri#149)
https://flask-login.readthedocs.io/en/latest/#your-user-class

Signed-off-by: Jiri Kuncar <[email protected]>
@anthraxx
Copy link

anthraxx commented Jan 3, 2023

@marksteward @jirikuncar This is actually a misinterpretation of the API, this commit should be reverted:

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.

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
It 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.

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Jan 3, 2023
kvesteri/sqlalchemy-continuum#149 (comment)


git-svn-id: file:///srv/repos/svn-community/svn@1374979 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Jan 3, 2023
kvesteri/sqlalchemy-continuum#149 (comment)

git-svn-id: file:///srv/repos/svn-community/svn@1374979 9fca08f4-af9d-4005-b8df-a31f2cc04f65
anthraxx added a commit to anthraxx/sqlalchemy-continuum that referenced this issue Jan 3, 2023
This reverts commit 7eda527.

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 kvesteri#316
Caused by kvesteri#149
@marksteward
Copy link
Collaborator

Thanks, yes that does look wrong. I misunderstood the context it was being called in - we shouldn't need to call get_id because we're the implementer.

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 a pull request may close this issue.

3 participants