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

Masterbar: send wpcom user ID to wpcom when attempting to log out #13777

Merged
merged 2 commits into from
Mar 18, 2020
Merged

Masterbar: send wpcom user ID to wpcom when attempting to log out #13777

merged 2 commits into from
Mar 18, 2020

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Oct 18, 2019 •
edited
Loading

Fixes #13680

Changes proposed in this Pull Request:

  • Until now, we would send a sync event to WordPress.com anytime someone attempted to log out from a site using the "Sign Out" button in the masterbar.
    From now on, we'll only do that when we have info about the connected WordPress.com user linked to that local user, and we'll send their wpcom user ID to WordPress.com so WordPress.com can disconnect them (and no one else) from WordPress.com.

Related discussion: p5TWut-lX-p2#comment-1098

Testing instructions:

  • Start from a connected site without this patch.
  • There should be 2 users on that site, both connected to 2 different WordPress.com accounts.
  • Apply D34223-code and point your site to your WordPress.com sandbox with JETPACK__SANDBOX_DOMAIN. This has now been committed.
  • Log in with one of the users, and log in to their associated WordPress.com account.
  • In the admin bar, click on the Gravatar, and then on "Sign Out".
  • You should be logged out of the site with no errors on your site, nor in your WordPress.com sandbox.
  • You should not be logged out of WordPress.com.
  • Apply this patch.
  • Log back in to your site.
  • Attempt to sign out again.
  • You should now be logged out of both your local and wpcom accounts, with no errors.
  • You can repeat the same test with the other user, and ensure that logging out with one does not log you out from the other user's accounts.

Proposed changelog entry for your changes:

  • WordPress.com Toolbar: log user out of WordPress.com when attempting to log out from the site.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Pri] Low [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations labels Oct 18, 2019
@jeherve jeherve added this to the 7.9 milestone Oct 18, 2019
@jeherve jeherve requested review from mdawaffe and a team October 18, 2019 16:38
@jeherve jeherve self-assigned this Oct 18, 2019
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Oct 18, 2019
@jetpackbot
Copy link

jetpackbot commented Oct 18, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against 5616873

@jeherve jeherve added [Status] Blocked / Hold and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 23, 2019
@jeherve
Copy link
Member Author

jeherve commented Oct 23, 2019

On hold pending the matching wpcom diff.

@jeherve jeherve modified the milestones: 7.9, 8.0 Oct 28, 2019
@jeherve jeherve modified the milestones: 8.0, 8.1 Nov 19, 2019
@jeherve jeherve removed this from the 8.1 milestone Dec 13, 2019
jeherve added 2 commits March 11, 2020 15:06
@jeherve
Masterbar: send wpcom user ID to wpcom when attempting to log out
c93789a 
Fixes #13680

Until now, we would send a sync event to WordPress.com anytime someone attempted to log out from a site using the "Sign Out" button in the masterbar.
From now on, we'll only do that when we have info about the connected WordPress.com user linked to that local user, and we'll send their wpcom user ID to WordPress.com so WordPress.com can disconnect them (and no one else) from WordPress.com.
@jeherve
Masterbar: hook into logout_redirect to handle post logout action
5616873 
WP 5.3 changed the way logouts worked:
https://core.trac.wordpress.org/changeset/46467

As a result we cannot pull the user's ID as they are logging out via the `wp_logout` filter anymore, because by then their info has already been cleared.

Let's instead hooked into `logout_redirect`, where we still have info about the user that logged out.
@jeherve jeherve force-pushed the fix/masterbar-logout-wpcom branch from a2872ff to 5616873 Compare March 11, 2020 14:06
@jeherve jeherve added this to the 8.4 milestone Mar 17, 2020
@jeherve jeherve added [Pri] Normal [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Pri] Low [Status] Blocked / Hold labels Mar 17, 2020
leogermani
leogermani approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

Works as expected for me.

Also did a quick search for maybe_logout_user_from_wpcom in a8c org to see if it could introduce any backward compatibility but it looks safe.

@leogermani leogermani added the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 18, 2020
@jeherve jeherve removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Mar 18, 2020
@jeherve jeherve merged commit 84cd060 into master Mar 18, 2020
@jeherve jeherve deleted the fix/masterbar-logout-wpcom branch March 18, 2020 18:17
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 18, 2020
jeherve added a commit that referenced this pull request Mar 23, 2020
@jeherve
Changelog: add #13777
1d6b2f9
jeherve added a commit that referenced this pull request Mar 31, 2020
@jeherve
8.4 Changelog (#15075)
0eb0b35 
* Initial changelog entry

* Changelog: add #14904

* Changelog: add #14910

* Changelog: add #14913

* Changelog: add #14916

* Changelog: add #14922

* Changelog: add #14924

* Changelog: add #14925

* Changelog: add #14928

* Changelog: add #14840

* Changelog: add #14841

* Changelog: add #14842

* Changelog: add #14826

* Changelog: add #14835

* Changelog: add #14859

* Changelog: add #14884

* Changelog: add #14888

* Changelog: add #14817

* Changelog: add #14814

* Changelog: add #14819

* Changelog;: add #14797

* Changelog: add #14798

* Changelog: add #14802

* Changelog: add #13676

* Changelog: add #13744

* Changelog: add #13777

* Changelog: add #14446

* Changelog: add #14739

* Changelog: add #14770

* Changelog: add #14784

* Changelog: add #14897

* Changelog: add #14898

* Changelog: add #14968

* Changelog: add #14985

* Changelog: add #15044

* Changelog: add #15052

* Update to remove Podcast since it remains in Beta

* Changelog: add #14803

* Changelog: add #15028

* Changelog: add #15065

* Changelog:add #14886

* Changelog: add #15118

* Changelog: add #14990

* Changelog: add #14528

* Changelog: add #15120

* Changelog: add #15126

* Changelog: add #15049

* Chanegelog: add #14852

* Changelog: add #15090

* Changelog: add #15138

* Changelog: add #15124

* Changelog:add #15055

* Changelog: add #15017

* Changelog: add #15109

* Changelog: add #15145

* Changelog:add #15096

* Changelog:add #15153

* Changelog: add #15133

* Changelog: add #14960

* Changelog: add #15127

* Changelog: add #15056

* Copy current changelog to changelog archive.

* Clarify changelog description
@jeherve jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leogermani leogermani leogermani approved these changes

@mdawaffe mdawaffe Awaiting requested review from mdawaffe

Assignees

@jeherve jeherve

Labels
[Feature] Masterbar WordPress.com Toolbar and Dashboard customizations [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
8.4
Development

Successfully merging this pull request may close these issues.

Masterbar: logging out logs out Jetpack Master User from WordPress.com
5 participants
@jeherve @jetpackbot @leogermani @xknown @matticbot