Skip to content

Conversation

@mattwiebe
Copy link
Contributor

@mattwiebe mattwiebe commented Apr 1, 2025

Very much a WIP, utterly untested, but the general shape of things is there. There's no listener on the home option yet, just working to have a Move::change_domain method that would ultimately connect to it, and the beginnings of serving the $from Actor distinctly from the $to Actor.

Fixes #1513

Proposed changes:

Other information:

  • Have you written new tests for your changes, if applicable?

Testing instructions:

  • Apply this PR to a WoA test blog.
  • Go to https://wordpress.com/domains/manage/SITE and update the primary domain.
  • Dump the saved old actor json and make sure the domains are correct (the old primary domain)
var_dump( get_option( 'activitypub_old_domain' ), json_decode( \get_option( 'activitypub_blog_user_old_domain_data' ) ), json_decode( \get_user_option( 'activitypub_old_domain_data', 12082294 ) ) );exit;
  • Make sure there's a Move activity in the outbox with the correct target property (new domain).

Changelog entry

  • Automatically create a changelog entry from the details below.
Changelog Entry Details

Significance

  • Patch
  • Minor
  • Major

Type

  • Added - for new features
  • Changed - for changes in existing functionality
  • Deprecated - for soon-to-be removed features
  • Removed - for now removed features
  • Fixed - for any bug fixes
  • Security - in case of vulnerabilities

Message

Support same-server domain migrations

@mattwiebe mattwiebe force-pushed the add/same-server-domain-move branch from ecd491f to b789d94 Compare April 1, 2025 18:59
@mattwiebe
Copy link
Contributor Author

Sorry I overwrote your changes @obenland - but I changed the approach radically enough that that function doesn't exist anymore :)

@mattwiebe
Copy link
Contributor Author

mattwiebe commented Apr 1, 2025

Some remaining tasks:

  • Handle the lifecycle of receiving follows on the new ID and unfollows on the old ID
  • Ensure that the old domain ID is used when POSTing the Move Activity to Follower inboxes
  • Add a home option listener, gated behind a constant for further testing

@obenland
Copy link
Member

obenland commented Apr 1, 2025

Ensure that the old domain ID is used when POSTing to Follower inboxes

Could you share more on that? I would have expected no changes since followers will send new Follow requests to the new account.

@mattwiebe
Copy link
Contributor Author

Ensure that the old domain ID is used when POSTing to Follower inboxes

Could you share more on that? I would have expected no changes since followers will send new Follow requests to the new account.

When we send the Move activity from the outbox to our Followers, we need to make sure we're using that we're using the correct Actor ID here. The "main"/default Actor at this point will be the new domain, but these requests need to be sent/signed by the old domain.

@obenland
Copy link
Member

obenland commented Apr 1, 2025

Ah, in the context of Move activities, only. I thought in general. Thank you!

@mattwiebe
Copy link
Contributor Author

mattwiebe commented Apr 1, 2025

Also note that some concerns we're raising here probably also apply to Move::internally.

I think we need to serve $from and $to differently in that case as well. So I think we ultimately need to follow a very similar caching behaviour there as well, but it's going to be harder to detect say https://wp.test/?author=1 vs https://wp.test/@username in that case.

@obenland
Copy link
Member

obenland commented Apr 4, 2025

Tested this on WoA a bit, with little success. Primary domain changes happen there through direct SQL updates, bypassing the pre_update_option_* hook we rely on.

Maybe we should keep track of the current domain ourselves and use that to populate our $from values. It would also allow us to switch back to the update_option_home action, that fires after it's actually been updated.

@obenland
Copy link
Member

obenland commented Apr 7, 2025

I think I made a fair bit of progress today:

  • Found a way to make change_domain() work on the update_option_home hook. It's not very elegant, it just replaces all instances of the new domain in the actor json with the old domain, before saving it. Maybe that's too naïve? Do we need. toworry about the signature we're storing?
  • Made a few updates to domain checks in order to allow actor identification with the old domain.
  • Updated the test to reflect the changes in hooks used.

@pfefferle I'd love for you to test this on a WoA sites or something more real world than localhost and see what you find!

@obenland obenland requested a review from Copilot April 7, 2025 21:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (14)
  • .github/changelog/add-same-server-domain-move: Language not supported
  • activitypub.php: Language not supported
  • includes/class-activitypub.php: Language not supported
  • includes/class-dispatcher.php: Language not supported
  • includes/class-move.php: Language not supported
  • includes/class-query.php: Language not supported
  • includes/class-signature.php: Language not supported
  • includes/collection/class-actors.php: Language not supported
  • includes/functions.php: Language not supported
  • includes/model/class-blog.php: Language not supported
  • includes/model/class-user.php: Language not supported
  • tests/includes/class-test-move.php: Language not supported
  • tests/includes/model/class-test-blog.php: Language not supported
  • tests/includes/model/class-test-user.php: Language not supported

*
* @param bool $domain_moves_enabled Whether domain moves are enabled.
*/
$domain_moves_enabled = apply_filters( 'activitypub_enable_primary_domain_moves', false );
Copy link
Member

@pfefferle pfefferle Apr 9, 2025

Choose a reason for hiding this comment

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

@obenland I know that you are not a fan of constants, but this would be the third possibility to enable a feature, aside from advanced settings and constants.

I think we should have a certain level of consistency. I am not arguing for a constant her, maybe we should think about a Developer Preview or Beta Features section in the Advanced settings to enable beta features!?!

Copy link
Member

Choose a reason for hiding this comment

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

I'd be happy to deprecate feature constants in favor of filters or advanced settings

Even if `get_collection` is returning the correct result, it is very specific to the API endpoint and might change over time, so we should not rely on it here.
@pfefferle
Copy link
Member

I think we should merge this PR! It is behind a feature flag, so it is not breaking something and with the functionality in the core plugin, it would allow us to more easily test it on WordPress.com or self hosted installations.

It is a solid state we can improve on.

@obenland obenland merged commit 4293ba4 into trunk Apr 9, 2025
11 checks passed
@obenland obenland deleted the add/same-server-domain-move branch April 9, 2025 13:10
@obenland obenland mentioned this pull request Apr 9, 2025
1 task
pfefferle added a commit that referenced this pull request Apr 10, 2025
* first pass from AI generation. Needs work

* reign in the chaos

* move logic into models

* Move constructor to below properties

* Update function name

* Don't show `movedTo` on the same ID

* DRY old domain handling

* Add options to delete list on uninstall

Updates blog user option to retain naming convention.

* Register user option

* Move actor saving inline

* changelog

* Remove all followers on `Move` to allow the new Actor to be re-followed

* lint lol

* Fix indent after merge

* Combine conditional on host

* Fix unit test

* First pass at testing change_domain

* Revert "lint lol"

This reverts commit c50a441.

* Revert "Remove all followers on `Move` to allow the new Actor to be re-followed"

This reverts commit 1d8948f.

* Encapsulate domain check

* Remove unused defaults

* Hook in with feature flag

* Add filters and handle username in callback

* Remove unused import

* Add unit tests for old actors

* Update reason for pre_update_option hook.

* Instantiate old actors through filters

* Remove unused import

* use query instead of Http client

* store check

* simplify code

* call action before sending an Activity to inboxes

* check class attribute before generation an id

* make old domain request setable

* Account for old domain in two more places

* Fix json creation on update_option_home

* Fix tests

* fix phpcs

* use \WP_Error

* Domain -> Host

* global namespace

* replace `get_collection` with `get_all`

Even if `get_collection` is returning the correct result, it is very specific to the API endpoint and might change over time, so we should not rely on it here.

---------

Co-authored-by: Konstantin Obenland <obenland@gmx.de>
Co-authored-by: Matthias Pfefferle <pfefferle@users.noreply.github.com>
jsit added a commit to jsit/wordpress-activitypub that referenced this pull request Nov 1, 2025
* tag '5.7.0': (29 commits)
  Release 5.7.0 (Automattic#1574)
  Fix: Show error if site uses "Almost Pretty Permalink" structure (Automattic#1570)
  Admin: Add padding to extra fields nav links (Automattic#1569)
  Signature: Add compat for more key encodings (Automattic#1557)
  Reply block: Improve fallback embed (Automattic#1560)
  Add label for "Health Check" and "REST API" (Automattic#1571)
  change changelog items (Automattic#1572)
  Transformers: Allow setting properties to false. (Automattic#1567)
  Upgrades: Delete orphaned extra fields (Automattic#1566)
  Fix: Missing Actor in Outbox-Activities (Automattic#1564)
  Outbox: Properly handle username requests (Automattic#1559)
  Import: Load on admin_init (Automattic#1561)
  Move: support same-server domain migrations (Automattic#1530)
  Follow Me: add a button-only mode (Automattic#1133)
  Tests: Convert timestamp to int for comparison (Automattic#1556)
  Reply: Make Mastodon embeds work (Automattic#1555)
  Add: `Vary` header settings (Automattic#1552)
  Actors: Don't convert non-numeric strings to Blog user id (Automattic#1554)
  Add: Shared Inbox setting (Automattic#1553)
  Add: New Health checks (Automattic#1524)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move: support domain changes

4 participants