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.

Sign up for GitHub

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

Jump to bottom

OAuth: use a custom table for transients #3106

Merged
merged 6 commits into from
May 13, 2024
Merged

OAuth: use a custom table for transients #3106

merged 6 commits into from
May 13, 2024

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented May 10, 2024 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Transients are not reliable when used between requests. This PR adds use of a custom transient table for OAuth purposes.

How to test the changes in this Pull Request:

  1. Set up a test site to use a Google OAuth proxy by providing a NEWSPACK_GOOGLE_OAUTH_PROXY env. variable
  2. Insert a Registration block somewhere, use it on the front-end to sign in as a reader using a Google account
  3. Observe you're signed in
  4. Now test the cleanup – click the "Sign in with Google" button but abandon the process by closing the popup window
  5. Look at the DB – there should be one row in the wp_newspack_oauth_transients table
  6. Copy this row, changing the datetime to something more than an hour ago
  7. Run wp cron event run np_oauth_transients_cleanup to trigger the cleanup
  8. Observe the older row was deleted

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo changed the base branch from trunk to release May 10, 2024 14:46
@adekbadek adekbadek changed the title feat: start new custom table for oauth transients data OAuth: use a custom table for transients May 12, 2024
@adekbadek adekbadek marked this pull request as ready for review May 12, 2024 14:40
@adekbadek adekbadek requested a review from a team as a code owner May 12, 2024 14:40
@adekbadek adekbadek added the [Status] Needs Review The issue or pull request needs to be reviewed label May 12, 2024
adekbadek
adekbadek approved these changes May 12, 2024
View reviewed changes
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

@dkoo – you won't be able to approve your own PR, so I'm approving it and awaiting your CR as a PR comment (or a CR from someone else!)

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels May 12, 2024
claudiulodro
claudiulodro approved these changes May 13, 2024
View reviewed changes
Copy link
Contributor

@claudiulodro claudiulodro left a comment

Choose a reason for hiding this comment

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

This looks solid and works well!

Do we need to think about people that are in the middle of a login session as this gets deployed? I think their token wouldn't work if they started the session under the old transient system and then this was deployed. It's probably a very tiny amount, so probably not worth adding special handling?

@dkoo
Copy link
Contributor Author

dkoo commented May 13, 2024

It's probably a very tiny amount, so probably not worth adding special handling?

Yeah, I'd say let's rip the bandaid off :)

@dkoo dkoo merged commit f9e8387 into release May 13, 2024
7 checks passed
@dkoo dkoo deleted the hotfix/oauth-transients-custom-table branch May 13, 2024 22:06
adekbadek added a commit that referenced this pull request May 14, 2024
@dkoo @adekbadek
fix(google-oauth): use a custom table for transients (#3106)
d4a2f5c 
Co-authored-by: Adam Cassis <adam.cassis@automattic.com>
matticbot pushed a commit that referenced this pull request May 14, 2024
@semantic-release-bot
chore(release): 3.8.4 [skip ci]
5498764 
## [3.8.4](v3.8.3...v3.8.4) (2024-05-14)

### Bug Fixes

* **google-oauth:** use a custom table for transients ([#3106](#3106)) ([d4a2f5c](d4a2f5c))
@adekbadek
Copy link
Member

(released in v3.8.4)

claudiulodro pushed a commit that referenced this pull request May 15, 2024
@adekbadek @matticbot
@dependabot @dkoo @jaredrethman @semantic-release-bot
Release May 15 (#3103)
2f0fa92 
* chore(deps-dev): bump @wordpress/browserslist-config

Bumps [@wordpress/browserslist-config](https://github.com/WordPress/gutenberg/tree/HEAD/packages/browserslist-config) from 5.37.0 to 5.38.0.
- [Release notes](https://github.com/WordPress/gutenberg/releases)
- [Changelog](https://github.com/WordPress/gutenberg/blob/trunk/packages/browserslist-config/CHANGELOG.md)
- [Commits](https://github.com/WordPress/gutenberg/commits/@wordpress/browserslist-config@5.38.0/packages/browserslist-config)

---
updated-dependencies:
- dependency-name: "@wordpress/browserslist-config"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps-dev): bump @rushstack/eslint-patch from 1.10.1 to 1.10.2

Bumps [@rushstack/eslint-patch](https://github.com/microsoft/rushstack/tree/HEAD/eslint/eslint-patch) from 1.10.1 to 1.10.2.
- [Changelog](https://github.com/microsoft/rushstack/blob/main/eslint/eslint-patch/CHANGELOG.md)
- [Commits](https://github.com/microsoft/rushstack/commits/@rushstack/eslint-patch_v1.10.2/eslint/eslint-patch)

---
updated-dependencies:
- dependency-name: "@rushstack/eslint-patch"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps-dev): bump @types/wordpress__blocks from 12.5.13 to 12.5.14

Bumps [@types/wordpress__blocks](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/wordpress__blocks) from 12.5.13 to 12.5.14.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/wordpress__blocks)

---
updated-dependencies:
- dependency-name: "@types/wordpress__blocks"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* refactor(data-events): add abstract Connector class for shared methods (#3015)

* refactor: add abstract Connector class for shared methods

* chore: remove unused imports

* chore(deps-dev): bump @wordpress/browserslist-config

Bumps [@wordpress/browserslist-config](https://github.com/WordPress/gutenberg/tree/HEAD/packages/browserslist-config) from 5.38.0 to 5.39.0.
- [Release notes](https://github.com/WordPress/gutenberg/releases)
- [Changelog](https://github.com/WordPress/gutenberg/blob/trunk/packages/browserslist-config/CHANGELOG.md)
- [Commits](https://github.com/WordPress/gutenberg/commits/@wordpress/browserslist-config@5.39.0/packages/browserslist-config)

---
updated-dependencies:
- dependency-name: "@wordpress/browserslist-config"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps): bump qs and @types/qs

Bumps [qs](https://github.com/ljharb/qs) and [@types/qs](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/qs). These dependencies needed to be updated together.

Updates `qs` from 6.12.0 to 6.12.1
- [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md)
- [Commits](ljharb/qs@v6.12.0...v6.12.1)

Updates `@types/qs` from 6.9.14 to 6.9.15
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/qs)

---
updated-dependencies:
- dependency-name: qs
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: "@types/qs"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix: ensure only admins can reset starter content and newspack options (#3081)

* feat: add product option to autocomplete orders (#3072)

BREAKING CHANGE: Changes order autocompletion behavior for existing products!

* feat: add product option to autocomplete orders

* chore: update outdated docblock description

* feat(ras): skip campaign setup (#3051)

* feat(ras): skip campaign setup boilerplate

added new route, added fe logic

* feat(ras): added `is_skipped` to prerequisite status endpoint

* refactor: moved callback to named function

added ui to indicate async skipping flow

* feat: added skip logic to prerequisite component

combined "skipped" and "active" into single `isValid` const

* feat: `is_skipped` property to prerequisite type

added skip logic to RAS wizard

* fix: circle ci / typescript

* fix: ci / eslint

* refactor: remove redirect after skip

added allow continue when skipped.

* refactor: allow parametrized skip

* refactor: revert any type cast

* feat: pr feedback - added redirect, activation skip and remove `activationStep[0]`

* feat: pr feedback, appended `[skipped]` to select items

* chore(release): 4.0.0-alpha.1 [skip ci]

# [4.0.0-alpha.1](v3.8.0...v4.0.0-alpha.1) (2024-04-25)

### Bug Fixes

* ensure only admins can reset starter content and newspack options ([#3081](#3081)) ([4606721](4606721))

### Features

* add product option to autocomplete orders ([#3072](#3072)) ([4a2859b](4a2859b))
* **ras:** skip campaign setup ([#3051](#3051)) ([9ef0e6d](9ef0e6d))

### BREAKING CHANGES

* Changes order autocompletion behavior for existing products!

* feat: add product option to autocomplete orders

* chore: update outdated docblock description

* chore(release): 4.0.0-alpha.2 [skip ci]

# [4.0.0-alpha.2](v4.0.0-alpha.1...v4.0.0-alpha.2) (2024-04-25)

### Bug Fixes

* remove deprecated filter callback ([#3090](#3090)) ([5d7d0bf](5d7d0bf))

* chore(release): 4.0.0-alpha.3 [skip ci]

# [4.0.0-alpha.3](v4.0.0-alpha.2...v4.0.0-alpha.3) (2024-04-26)

### Bug Fixes

* enable Memberships fix cron job only when environment constant is defined ([#3087](#3087)) ([5d40297](5d40297))

* OAuth: use a custom table for transients (#3106)

* feat: start new custom table for oauth transients data

* fix: copy/paste error

* fix(google-oauth): use custom table for transients

* feat: cleanup old transients

* fix: clean up on the fly too, and limit to deleting 1000 at a time max

---------

Co-authored-by: Adam Cassis <adam.cassis@automattic.com>

* fix: autocomplete orders only for virtual products (#3111)

* fix: autocomplete orders only for virtual products

* chore: remove console.log

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: matticbot <sysops+ghmatticbot@automattic.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>
Co-authored-by: Jared Rethman <jaredrethman@gmail.com>
Co-authored-by: matticbot <semantic-release-bot@martynus.net>
Co-authored-by: dkoo <derrick.koo@automattic.com>
matticbot pushed a commit that referenced this pull request May 15, 2024
@semantic-release-bot
chore(release): 4.0.0-alpha.4 [skip ci]
a2175a5 
# [4.0.0-alpha.4](v4.0.0-alpha.3...v4.0.0-alpha.4) (2024-05-15)

### Bug Fixes

* autocomplete orders only for virtual products ([#3111](#3111)) ([bfbe554](bfbe554))
* **google-login:** get the email from the /tokeninfo endpoint ([#3117](#3117)) ([3296f1a](3296f1a))
* **google-oauth:** use a custom table for transients ([#3106](#3106)) ([d4a2f5c](d4a2f5c))
* **oauth-transients:** remove redundant cleanup ([#3112](#3112)) ([c123c02](c123c02))
* **ras:** sync purchase data only for most recent order/subscription ([#3086](#3086)) ([2c7763a](2c7763a))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 4.0.0-alpha.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 4.0.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 4.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claudiulodro claudiulodro claudiulodro approved these changes

@adekbadek adekbadek adekbadek approved these changes

Assignees
No one assigned
Labels
released on @alpha released on @epic/ras-acc released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dkoo @adekbadek @matticbot @claudiulodro