Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.58k stars 797 forks source link

Jetpack connection package: How to handle disconnecting from Jetpack? #13843

Open DanReyLop opened 4 years ago

DanReyLop commented 4 years ago

Adding this for visibility and discussion in the open. The Jetpack Connection Task Force kickoff meeting has prompted some discussion about this.

In a future where a third-party plugin is connected to wp.com using the Jetpack connection package, how do we handle the user disconnecting said connection?

kraftbj commented 4 years ago

If no, then that means that disconnecting Jetpack won't affect the rest of the Jetpack-connection-based plugins. So, the site will only be truly disconnected from Jetpack when the last Jetpack-connection-powered plugin is uninstalled/deactivated?

That's my vote. We can add some type of language "Your site is still using X, Y, Z. To fully disassociate with WordPress.com/Jetpack/etc, please remove all of those plugins".

Otherwise, we depend on other plugins to ensure they are accurately representing what is happening -- what if one doesn't implement the UI and just tries to disconnect?

allendav commented 4 years ago

I didn't think deactivating (and re-activating) Jetpack left the user disconnected. Does it? Let's be super careful semantically here to always say disconnect when we mean disconnect.

DanReyLop commented 4 years ago

I didn't think deactivating (and re-activating) Jetpack left the user disconnected. Does it?

I don't know about Jetpack, but I didn't mention deactivating Jetpack at all. I did talk about the possibility of plugins X, Y, or Z deactivating, and that would disconnect the Jetpack connection.

For example, if WooCommerce Shipping uses the Jetpack connection package, the user doesn't have any other plugins installed, and the user deactivates WooCommerce Shipping, what should happen there? Should the connection be fully destroyed? Or should the tokens be preserved in the database so then the plugin is re-activated, the connection can resume working normally? I guess we'll want to have the same behaviour as Jetpack proper.

Let's be super careful semantically here to always say disconnect when we mean disconnect.

I've edited the description to only use "disconnection" all around. What happens when the plugin is disabled/uninstalled is only tangentially related to this issue.

allendav commented 4 years ago

Or should the tokens be preserved in the database so then the plugin is re-activated, the connection can resume working normally? I guess we'll want to have the same behaviour as Jetpack proper.

My take is it should work the same way as Jetpack does today - that if I were to re-activate, it wouldn't require re-connection.

kraftbj commented 4 years ago

Disconnection -- Specifically the technical piece where the Jetpack<->WP.com relationship is severed including invalidation of the shared tokens that secure communication. This destroys both the master connection and any secondary connected users.

Deactivation -- The typical WordPress meaning of deactivating a plugin. In Jetpack, there is a deactivation hook that fires the disconnection functionality link. This would need to change as part of this work so we would only disconnect if we're the last plugin standing.

Deletion -- The typical WordPress meaning. When a deactivated plugin is deleted via wp-admin. Fires uninstall.php if present.

What I'm thinking: The connection package has something like a filter $keep_connection = apply_filters('jetpack_connection_package_anything_still_on', false );

Every consumer somehow adds a add_filter('jetpack_connection_package_anything_still_on', '__return_true' ); when active. When the last plugin is deactivated, that would be false and the connection will be disconnected.

Something similar could be added in the uninstall.php that would fire when a plugin is deleted to be sure we're cleaning up after ourselves.

(None of this has been prototyped, so I reserve the right to be wrong about the pseudocode above :) ).

allendav commented 4 years ago

The typical WordPress meaning of deactivating a plugin. In Jetpack, there is a deactivation hook that fires the disconnection functionality

TIL. I was under the misapprehension that deactivation of Jetpack didn't cause a disconnect

DanReyLop commented 4 years ago

Every consumer somehow adds a add_filter('jetpack_connection_package_anything_still_on', '__return_true' ); when active. When the last plugin is deactivated, that would be true and the connection will be disconnected.

Love this idea. Extra points if the add_filter is called on the jetpack connection package itself, somehow.

Also, with this approach (a plugin disconnecting only disconnects said plugin), the jetpack connection package should keep a map of 'plugin_slug' => true/false to track the status of the connection for that particular plugin. That way, if Jetpack and WC Shipping are connected and WC Shipping gets disconnected, WC Shipping can show a big fat "Connect" button that will just flip that switch back on (no need to run the whole flow to make the connection again, since the connection itself is already active).

LevinMedia commented 4 years ago

We can add some type of language "Your site is still using X, Y, Z. To fully disassociate with WordPress.com/Jetpack/etc, please remove all of those plugins".

Seems like a good first step to get things going.

WC Shipping can show a big fat "Connect" button that will just flip that switch back

Was thinking further down the road, (sorry can't help it) if this gains traction and wider use, adding a connections page to wp-admin settings would be handy. A place where the master user could be transferred, and connections could be managed etc.

Untitled_Artwork

stale[bot] commented 4 years ago

This issue has been marked as stale. This happened because:

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.