Skip to content

Conversation

@LZoog
Copy link
Contributor

@LZoog LZoog commented Oct 2, 2025

Because:

  • We've been storing long-lived access tokens for Pocket in our oauth db as a work around to not use refresh tokens, and these should be deleted

This commit:

  • Adds a script intended for a one-time run once Pocket removal code has landed in production, to delete these tokens

closes FXA-12490


Testing this locally

See this, where we've got special casing for Pocket: } else if (POCKET_IDS.includes(hex(vals.clientId))) { ..., which will be removed in that linked PR.

If that PR has been merged already then re-add the else/if and change it to this, or if it hasn't been merged then simply update the line to the 123done client ID:

} else if (['dcdb5ae7add825d2'].includes(hex(vals.clientId))) {

After running yarn mysql, you can USE fxa_oauth and select * from tokens; to see sign-ins from 123done add tokens to that table. If you revert the changes and sign in normally you won't see a new row added to the table.

Next, change the POCKET_CLIENT_IDS array in the script to:

const POCKET_CLIENT_IDS = ['dcdb5ae7add825d2'];

Then run the script with yarn workspace fxa-auth-server prune-pocket-access-tokens to see the dry-run output in the console. yarn workspace fxa-auth-server prune-pocket-access-tokens --dry-run=false runs it for real. (You might need to comment out, throw new Error(Config '${key}' must be set in production))

@LZoog LZoog requested a review from vbudhram October 2, 2025 19:43
const clientIdBuffer = Buffer.from(clientId, 'hex');

const countResult = await db('tokens')
.where('clientId', clientIdBuffer)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a generic access token pruning script that may also work?

That script handles expiration, but in this case we are deleting all access tokens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, seems I potentially could have repurposed that one a bit instead. That one's deleting expired authorization codes while this one deletes the access tokens for specific client ids, but they hook into the same db. I guess since this one's already written I'll leave it but that's good to know.

Copy link
Contributor

@vbudhram vbudhram left a comment

Choose a reason for hiding this comment

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

Code looks right, gonna give it a run locally

@LZoog LZoog marked this pull request as ready for review October 2, 2025 23:01
@LZoog LZoog requested a review from a team as a code owner October 2, 2025 23:01
@LZoog LZoog force-pushed the FXA-12490 branch 2 times, most recently from f2e1d08 to 18aecfb Compare October 2, 2025 23:43
nonPocketClientId = buf(randomString(8));
userId = buf(randomString(16));

// Pocket clients may already exist in CI, so wrap in try/catch.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still squinting at this - without these checks all the tests pass locally for me but in CI, I get an ER_DUP_ENTRY, so this seemed fine... everything is passing now locally and in CI but now I'm wondering if I'm deleting something in this test DB and then not restoring it at the end heh, I could do a check and add it back at the end 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I modified this a bit to restore the original clients at the end of the test. Let's see if it passes too.

Because:
* We've been storing long-lived access tokens for Pocket in our oauth db as a work around to not use refresh tokens, and these should be deleted

This commit:
* Adds a script intended for a one-time run once Pocket removal code has landed in production, to delete these tokens

closes FXA-12490
@LZoog LZoog merged commit b86702e into main Oct 7, 2025
19 checks passed
@LZoog LZoog deleted the FXA-12490 branch October 7, 2025 17:04
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.

3 participants