Skip to content

Search and replace breaks URL change detection #12962

Description

@aaemnnosttv

Bug Description

When connected to the SK service, we keep track of the connected URL of the site via the googlesitekit_connected_proxy_url setting. This value is based on the home URL of the site and stored in plain text as a simple key-value option. When SK detects that the current URL is different than this (via Connected_Proxy_URL::matches_url), the user is disconnected and prompted to reconnect, at which point the user will go through the initial connection flow again and the site will receive updated credentials.

This process is fragile in that it relies on the setting to reliably store the URL of the site that was connected to the SK service. This is because various tools and common workflows rewrite the site's base URL throughout the database which breaks this detection when SK's connected URL reference is also rewritten.

Site Kit should update the storage of this value such that it is no longer rewritten by a simple search and replace while still providing the same detection.

As such, one aspect of this issue needs to address the fragility in general, while supporting backwards compatibility for sites which currently store the connected URL verbatim.

Steps to reproduce

  1. Create a new site in InstaWP with SK and follow the minimal initial connection (note the URL)
  2. In the IWP dashboard, use the option to Change Suffix Domain to any other (e.g. .xyz or .link)
  3. Once updated, in WP notice SK does not flag about a URL change (it was automatically replaced in the DB)
  4. In wp-admin/options.php, find the googlesitekit_connected_proxy_url setting, and update it to the original suffix domain it had when created and initially connected
  5. See URL change immediately notice on save

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The persisted connected proxy URL used for URL change detection must be protected from rewriting by search and replace operations in the database such that a URL change is properly detected afterwards
  • This must support new and existing saved values for googlesitekit_connected_proxy_url

Implementation Brief

  • Update Connected_Proxy_URL to encode the value on save using base64
    • It needs to account for an already encoded value to avoid double encoding so it can attempt to decode and existing value first so that the next steps are always the same
    • It needs to support a legacy/unencoded value (i.e. skip decoding if the saved value starts with http)
    • Update get to decode the value before returning
  • Add a new Migration_ class to update an existing unencoded value to the new format
    • The actual migration can be as simple as CPU->set( CPU->get() )

Test Coverage

  • Add coverage for changes to Connected_Proxy_URL
  • Add a test for simulating the DB rewrite scenario similar to AuthenticationTest::test_check_connected_proxy_url
  • Add coverage for new migration

QA Brief

Changelog entry

Metadata

Metadata

Assignees

No one assigned

    Labels

    Next UpIssues to prioritize for definitionP1Medium priorityType: BugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions