-
Notifications
You must be signed in to change notification settings - Fork 81
Fix malformed ActivityPub handles for email-based logins #2082
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.
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
base: trunk
Are you sure you want to change the base?
Conversation
Sanitize email-based usernames (e.g., from Site Kit Google login) to prevent malformed ActivityPub handles like @[email protected]@domain.com. - Modified get_preferred_username() to detect and sanitize email logins - Added comprehensive test coverage for email username sanitization - Ensures proper webfinger format without double @ symbols Fixes #2070.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes malformed ActivityPub handles for users with email-based login names, such as those created through Site Kit Google authentication. The issue occurred when usernames containing "@" symbols resulted in double "@" symbols in ActivityPub handles.
- Modified
get_preferred_username()
to sanitize email-based usernames usingsanitize_title()
- Added comprehensive test coverage to verify proper email username sanitization
- Ensures ActivityPub handles are properly formatted without malformed double "@" symbols
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
includes/model/class-user.php | Modified get_preferred_username() to detect and sanitize email-based login names |
tests/includes/model/class-test-user.php | Added comprehensive test cases for email username sanitization scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Prefixed str_contains with a backslash to ensure the global PHP function is used, preventing potential issues with overridden or namespaced functions.
…Automattic/wordpress-activitypub into fix/email-username-sanitization
Fix PHPCS alignment warnings for variable assignments.
If the administrator creates a new account using a Google email address but sets the username in a non-email format (a regular ID instead of an email), and then links it via Sign in with Google, this bug can be bypassed. Honestly, I’m not entirely sure whether this should be debugged on the ActivityPub side, or if it’s something that Site Kit should address by changing how it handles email-based username creation. If my memory serves me correctly, Misskey prevents users from creating accounts with email-style IDs at signup (or at least explicitly instructs users not to enter them). However, since the author page ID for email-based accounts does not include special characters, this fix still seems to have some value. |
The profile URL is generated as: To maintain consistency with the ActivityPub handle, instead of replacing @ with a hyphen, the @ should be omitted entirely. 👉 Desired format: is not very “clean” as a username. It may be worth considering a redirect mechanism in the future so that such legacy usernames can point to a cleaner, newly chosen identifier. Of course, this would also require solving the name collision problem (e.g., if two users try to adopt the same “cleaned” username). 👉 In practice, this would mean: Keep the legacy handle for backwards compatibility. Allow the user/admin to register a new canonical handle. Set up a redirect/alias system so that old mentions and followers still resolve correctly. This should be addressed together with the ability for users to edit their blog profile handle. For example, mechanisms that allow arbitrary manipulation of the handle—such as using the posts page slug as the handle—should be prevented. The goal is to maintain consistency and integrity of ActivityPub handles across posts and profiles. |
Use explicit string replacement instead of sanitize_title() to ensure dots are converted to dashes as expected by tests. Also use filter_var() for proper email validation.
Fixes #2070.
Proposed changes:
get_preferred_username()
method inincludes/model/class-user.php
to detect email-based logins and sanitize them usingsanitize_title()
tests/includes/model/class-test-user.php
to verify proper email username sanitization@[email protected]
instead of malformed@[email protected]@domain.com
Other information:
Testing instructions:
Setup
Test Case 1: Email-based Login from Site Kit
[email protected]
@[email protected]
@[email protected]@yourdomain.com
Test Case 2: Manual Email-based Username
[email protected]
@[email protected]
Test Case 3: Normal Username (Regression Test)
normaluser
@[email protected]
(unchanged behavior)Test Case 4: Run Unit Tests
Expected result: All tests should pass
Changelog entry
Changelog Entry Details
Significance
Type
Message
Fix malformed ActivityPub handles for users with email-based logins (e.g., from Site Kit Google authentication)