Skip to content
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

Ensure User Actor usernames are valid for ActivityPub #75

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/models/discourse_activity_pub_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def self.username_unique?(username, model_id: nil, local: true)
sql += " AND local IS TRUE" if local
args = { username: username }
args[:model_id] = model_id if model_id
self.where(sql, args).exists?
!self.where(sql, args).exists?
end

# Equivalent of mastodon/mastodon/app/models/concerns/account_finder_concern.rb#representative
Expand Down
2 changes: 1 addition & 1 deletion extensions/discourse_activity_pub_category_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def on_custom_fields_change
DiscourseActivityPub::UsernameValidator.perform_validation(self, "activity_pub_username")

if self.errors.blank? &&
DiscourseActivityPubActor.username_unique?(
!DiscourseActivityPubActor.username_unique?(
@custom_fields["activity_pub_username"],
model_id: self.id,
)
Expand Down
8 changes: 7 additions & 1 deletion lib/discourse_activity_pub/user_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,13 @@ def build_actor
end

def update_actor
actor.username = user.username
username = user.username

if !UsernameValidator.new(username).valid_format?
username = UsernameSuggester.suggest(username)
end

actor.username = username
actor.name = user.name if user.name
end

Expand Down
63 changes: 63 additions & 0 deletions lib/discourse_activity_pub/username_suggester.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# frozen_string_literal: true

module DiscourseActivityPub
class UsernameSuggester
LAST_RESORT_USERNAME = "actor"

def self.suggest(username)
username = normalize(username)
username = normalize(I18n.t("fallback_username")) if username.blank?
username = LAST_RESORT_USERNAME if username.blank?

find_available(username)
end

def self.normalize(text)
text.unicode_normalize(:nfkd).encode("ASCII", replace: "")
end

# Based on discourse/discourse/lib/user_name_suggester.rb#find_available_username_based_on
def self.find_available(username)
return username if DiscourseActivityPubActor.username_unique?(username)

similar = "#{username}(0|1|2|3|4|5|6|7|8|9)+"
count = DB.query_single(<<~SQL, like: "#{username}%", similar: similar).first
SELECT count(*) FROM discourse_activity_pub_actors
WHERE local IS TRUE
AND username LIKE :like
AND username SIMILAR TO :similar
SQL

offset =
if count > 0
params = { count: count + 10, username: username }
available = DB.query_single(<<~SQL, params).first
WITH numbers AS (SELECT generate_series(1, :count) AS n)
SELECT n FROM numbers
LEFT JOIN discourse_activity_pub_actors ON (
username = :username || n::varchar
) AND (
local IS TRUE
)
WHERE discourse_activity_pub_actors.id IS NULL
ORDER by n ASC
LIMIT 1
SQL
[available.to_i - 1, 0].max
else
0
end

i = 1
attempt = username
until (DiscourseActivityPubActor.username_unique?(attempt) || i > 100)
suffix = (i + offset).to_s
max_length = User.username_length.end - suffix.length
attempt = "#{UserNameSuggester.truncate(username, max_length)}#{suffix}"
i += 1
end

attempt
end
end
end
1 change: 1 addition & 0 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
../lib/discourse_activity_pub/request.rb
../lib/discourse_activity_pub/webfinger.rb
../lib/discourse_activity_pub/username_validator.rb
../lib/discourse_activity_pub/username_suggester.rb
../lib/discourse_activity_pub/content_parser.rb
../lib/discourse_activity_pub/signature_parser.rb
../lib/discourse_activity_pub/delivery_failure_tracker.rb
Expand Down
13 changes: 13 additions & 0 deletions spec/lib/discourse_activity_pub/user_handler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,18 @@
expect(DiscourseActivityPubActor.all.size).to eq(actor_count)
end
end

context "when user has a username that would be invalid as an ActivityPub username" do
before do
SiteSetting.unicode_usernames = true
user.username = "óengus"
user.save!
end

it "creates an actor with a valid ActivityPub username" do
actor = described_class.update_or_create_actor(user)
expect(actor.username).to eq("oengus")
end
end
end
end
27 changes: 27 additions & 0 deletions spec/lib/discourse_activity_pub/username_suggester_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# frozen_string_literal: true

RSpec.describe DiscourseActivityPub::UsernameSuggester do
describe "#suggest" do
it "converts non-valid usernames to valid usernames" do
expect(described_class.suggest("零卡")).to eq(I18n.t("fallback_username"))
end

it "works with non-latin locales" do
expect(I18n.with_locale(:zh_TW) { described_class.suggest("零卡") }).to eq(
described_class::LAST_RESORT_USERNAME,
)
end

it "returns locally unique usernames" do
expect(described_class.suggest("à")).to eq("a")
Fabricate(:discourse_activity_pub_actor, username: "a", local: true)
expect(described_class.suggest("à")).to eq("a1")
Fabricate(:discourse_activity_pub_actor, username: "a1", local: true)
expect(described_class.suggest("à")).to eq("a2")
Fabricate(:discourse_activity_pub_actor, username: "a2", local: false)
Fabricate(:discourse_activity_pub_actor, username: "a10", local: true)
expect(described_class.suggest("à")).to eq("a2")
expect(described_class.suggest("a10")).to eq("a101")
end
end
end