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

Use Contact User as Relay, Report, Subscribe. #9661

Merged
merged 6 commits into from Jan 5, 2019

Conversation

Projects
None yet
4 participants
@yukimochi
Copy link
Contributor

yukimochi commented Dec 30, 2018

Following relay, sending report and subscribing are used user selected by Account.local.where(suspend: false) in Mastodon.
This method may give responsibility someone who does not know anything.
Use Contact User if set.

@yukimochi yukimochi force-pushed the yukimochi:use-contact-username branch from a58874f to c7c6496 Dec 30, 2018

@@ -68,7 +68,7 @@ def unfollow_activity(activity_id)
end

def some_local_account
@some_local_account ||= Account.local.find_by(suspended: false)
@some_local_account ||= Account.find_local(Setting.site_contact_username.gsub(/\A@/, '')) || Account.local.find_by(suspended: false)

This comment has been minimized.

@Gargron

Gargron Dec 30, 2018

Member

Perhaps create Account.representative method?

This comment has been minimized.

@yukimochi

yukimochi Dec 30, 2018

Author Contributor

OK.
Sorry for many force-pushes, all done.

@yukimochi yukimochi force-pushed the yukimochi:use-contact-username branch 2 times, most recently from dda40e0 to 00c4f51 Dec 30, 2018

@yukimochi yukimochi force-pushed the yukimochi:use-contact-username branch from 00c4f51 to d1e4342 Dec 30, 2018

@@ -43,7 +43,7 @@ def subscription_params
end

def some_local_account
@some_local_account ||= Account.local.where(suspended: false).first
@some_local_account ||= Account.representative || Account.local.where(suspended: false).first

This comment has been minimized.

@nightpool

nightpool Dec 30, 2018

Collaborator

no reason to use Account.representative here.

This comment has been minimized.

@yukimochi

yukimochi Dec 30, 2018

Author Contributor

Certainly, that's right.

@nightpool

This comment has been minimized.

Copy link
Collaborator

nightpool commented Dec 30, 2018

I'm a little nervous about using some_local_account or Account.representative interchangeably—if we're saying relay subscription confers responsibility in some way, then the silent fallback could be dangerous. Certainly federated reports do confer responsibility. But at the same time, we can't expect that all instances have a contact_username set.

I think we should resolve this by having a "server" actor as suggested a couple of times when the relay and federated reports were being developed—the combined usecases of relays and federated reports make this make sense, even if we wouldn't want the complexity for either alone.

@ThibG

This comment has been minimized.

Copy link
Collaborator

ThibG commented Jan 1, 2019

A server actor would also enable authenticating fetches without compromising privacy too much (for non-single-user instances at least).
This (combined with dropping LDS) would pave the way for stricter control of who gets to see toots (even public ones), allowing stronger instance blocks.

@@ -4,6 +4,10 @@ module AccountFinderConcern
extend ActiveSupport::Concern

class_methods do
def representative!

This comment has been minimized.

@Gargron

Gargron Jan 4, 2019

Member

Since this is not used anywhere, I do not think it is required.

@@ -12,6 +16,10 @@ def find_remote!(username, domain)
find_remote(username, domain) || raise(ActiveRecord::RecordNotFound)
end

def representative
find_local(Setting.site_contact_username.gsub(/\A@/, ''))

This comment has been minimized.

@Gargron

Gargron Jan 4, 2019

Member

I believe the || Account.local.find_by(suspended: false) logic should be included here, since it is repeated everywhere this method is called.

@yukimochi

This comment has been minimized.

Copy link
Contributor Author

yukimochi commented Jan 5, 2019

I thought too server actor may best way to solve problem, but that may need more time to implement.
Now non Mastodon ActivityPub implement may misread "who report" "who follow relay" is large problem, this is mitigation measures.

@Gargron

Gargron approved these changes Jan 5, 2019

@Gargron Gargron merged commit fae3263 into tootsuite:master Jan 5, 2019

11 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate All good!
Details

@yukimochi yukimochi deleted the yukimochi:use-contact-username branch Jan 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment