Skip to content

Make Disconnect safe on a copied site (1.1.0) - #14

Open
DAAAAAAAAAAAAAAAAN wants to merge 1 commit into
masterfrom
feature/copy-safe-disconnect
Open

DAAAAAAAAAAAAAAAAN wants to merge 1 commit into
masterfrom
feature/copy-safe-disconnect

Conversation

@DAAAAAAAAAAAAAAAAN

Copy link
Copy Markdown

Why

A partner's site was database-copied to a second domain. The copy carried
printeers_connected, printeers_store_url and the WooCommerce API key of the
original. Clicking Disconnect on the copy posted the original's URL and
consumer secret to /woocommerce/disconnect, and Printeers uninstalled the
original store (verified in prod on 2026-09-14/15).

What

  • Send site_url. The disconnect body now contains exactly
    store_url, consumer_secret and site_url (home_url() at click time).
    The callback API compares site_url with the install's store URL and refuses
    with 409 when they differ. Unknown keys are rejected server-side, so no other
    keys are sent.
  • Show the server's reason. A failed disconnect appends the response body
    to the error notice, so a 409 reads as
    this connection belongs to <store>, not to <site>; it was left intact.
  • Detect a copied site before calling Printeers. The admin page compares
    the stored store URL with home_url(), both normalized the way the server
    canonicalizes store URLs (host lower-cased, :443 dropped, trailing slash
    trimmed, scheme compared as https). On a mismatch it shows a warning state
    with both URLs and one button, Remove connection from this site, which
    deletes the local API key row and the plugin options and never contacts
    Printeers. The regular Disconnect form is not rendered in that state. After
    removal the page is "Not connected" and Connect works as a fresh connect for
    the copy's own URL.
  • A genuinely moved site lands in the same state on purpose: remove locally,
    connect the new domain as its own store, retire the old one in the Dashboard.
    The notice says so.
  • Connect::disconnect() is unchanged apart from the body; the guard lives in
    the UI path so a stray caller still gets the server-side 409.
  • readme: the "External services" disconnect sentence now says a disconnect
    pauses the store and that a copied site cannot disconnect the original.
    1.1.0 changelog, stable tag and version bump.

uninstall.php is untouched: deleting the plugin already removes local state
without notifying Printeers, which is right for both real and copied sites.

Server side

Pairs with ipp branch agent/woocommerce-plugin-disconnect-sideline
(1c4d9f029), which adds the site_url check and turns a plugin disconnect
into a pause instead of an uninstall. Ordering does not matter: the server
accepts bodies without site_url, so 1.0.0 plugins keep working, and this
plugin against the current server just gets its extra key rejected with a 400
that is shown in the notice. Merge the server first for a clean rollout.

Testing

Done:

  • php -l on every file under PHP 7.4 and 8.3.
  • normalize_url() checked standalone: case, :443, trailing slash,
    http/https, query and fragment, non-default port, IPv6, empty input.
  • Admin template rendered with WordPress shims in all states: same site
    (Disconnect shown), copied site (Disconnect hidden, remove form with its own
    nonce), after removal (Not connected + success notice), failed disconnect
    (409 reason shown).

Not done, needs the ipp branch deployed to the local cluster:

  1. Normal Disconnect then Connect again: 200, callback-api logs
    store sidelined, Dashboard banner appears and clears on reconnect.
  2. Simulated copy (wp option update printeers_store_url https://other.example):
    mismatch state shown, remove clears the four options and the
    wp_woocommerce_api_keys row, no HTTP request in callback-api logs.
  3. curl with a foreign site_url: 409, install unchanged.
  4. Body with an extra key: 400.

🤖 Generated with Claude Code

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.

1 participant