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

Add DB migration to delete orphaned tokens #8511

Merged
merged 1 commit into from
Feb 15, 2024
Merged

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Feb 12, 2024

This should be merged after #8510 (which will make sure that we don't orphan any more tokens).

Checking what the SQLAlchemy query evaluates to:

$ make shell
>>> from sqlalchemy import select, and_, func
>>> str(
...     select(m.Token.userid)
...     .outerjoin(
...         m.User,
...         and_(
...             m.User._username
...             == func.substring(func.split_part(m.Token.userid, "@", 1), 6),
...             m.User.authority == func.split_part(m.Token.userid, "@", 2),
...         )
...     )
...     .where(m.User.id.is_(None))
...     .distinct()
... )
'SELECT DISTINCT token.userid \nFROM token LEFT OUTER JOIN "user" ON "user".username = substring(split_part(token.userid, :split_part_1, :split_part_2), :substring_1) AND "user".authority = split_part(token.userid, :split_part_3, :split_part_4) \nWHERE "user".id IS NULL'

Testing

  1. Log in as devdata_user, go to http://localhost:5000/account/developer, and generate a token
  2. Log out, log back in as devdata_admin, go to http://localhost:5000/account/developer, and generate a token, then go to http://localhost:5000/admin/users and delete devdata_user

At this point you should have two rows in the token table in your DB: one belonging to devdata_admin and one orphaned row belonging to devdata_user.

If you now run:

$ make db args='upgrade +1'

you should see Deleted 1 orphaned tokens and the devdata_user token should be deleted from your DB.

@seanh seanh requested a review from marcospri February 12, 2024 16:47
Copy link
Member

@marcospri marcospri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good. I don't think it will be an issue to run it prod but we'll have to keep an eye while the migration runs 👍 .

@seanh seanh merged commit a62d037 into main Feb 15, 2024
9 checks passed
@seanh seanh deleted the delete-orphaned-tokens branch February 15, 2024 08:28
@seanh seanh mentioned this pull request Feb 15, 2024
27 tasks
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.

2 participants