From 5f387995d9ae6f89c93518518233c6d9874f6621 Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Mon, 24 Dec 2018 19:06:14 +0100
Subject: Limit maximum visibility of local silenced users to unlisted (#9583)

Fixes #9580
---
 spec/services/post_status_service_spec.rb | 7 +++++++
 1 file changed, 7 insertions(+)

(limited to 'spec/services')

diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb
index 349ad861b..8f3552224 100644
--- a/spec/services/post_status_service_spec.rb
+++ b/spec/services/post_status_service_spec.rb
@@ -68,6 +68,13 @@ RSpec.describe PostStatusService, type: :service do
     expect(status.visibility).to eq "private"
   end
 
+  it 'creates a status with limited visibility for silenced users' do
+    status = subject.call(Fabricate(:account, silenced: true), 'test', nil, visibility: :public)
+
+    expect(status).to be_persisted
+    expect(status.visibility).to eq "unlisted"
+  end
+
   it 'creates a status for the given application' do
     application = Fabricate(:application)
 
-- 
cgit 


From 5d2fc6de321ac556ded72e5a8fa9fcf47d172e16 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Mon, 24 Dec 2018 19:12:38 +0100
Subject: Add REST API for creating an account (#9572)

* Add REST API for creating an account

The method is available to apps with a token obtained via the client
credentials grant. It creates a user and account records, as well as
an access token for the app that initiated the request. The user is
unconfirmed, and an e-mail is sent as usual.

The method returns the access token, which the app should save for
later. The REST API is not available to users with unconfirmed
accounts, so the app must be smart to wait for the user to click a
link in their e-mail inbox.

The method is rate-limited by IP to 5 requests per 30 minutes.

* Redirect users back to app from confirmation if they were created with an app

* Add tests

* Return 403 on the method if registrations are not open

* Require agreement param to be true in the API when creating an account
---
 app/controllers/api/base_controller.rb             |  2 +-
 app/controllers/api/v1/accounts_controller.rb      | 26 ++++++++++++--
 app/controllers/auth/confirmations_controller.rb   | 10 +++++-
 app/controllers/auth/registrations_controller.rb   |  1 +
 app/models/user.rb                                 |  7 ++--
 app/services/app_sign_up_service.rb                | 23 ++++++++++++
 .../confirmation_instructions.html.haml            |  8 +++--
 .../user_mailer/confirmation_instructions.text.erb |  2 +-
 config/initializers/rack_attack.rb                 |  4 +++
 config/locales/devise.en.yml                       |  1 +
 config/routes.rb                                   |  2 +-
 ...35220_add_created_by_application_id_to_users.rb |  8 +++++
 db/schema.rb                                       |  5 ++-
 lib/mastodon/accounts_cli.rb                       |  2 +-
 .../controllers/api/v1/accounts_controller_spec.rb | 34 ++++++++++++++++++
 spec/fabricators/user_fabricator.rb                |  1 +
 spec/models/user_spec.rb                           | 14 ++++----
 spec/services/app_sign_up_service_spec.rb          | 41 ++++++++++++++++++++++
 18 files changed, 171 insertions(+), 20 deletions(-)
 create mode 100644 app/services/app_sign_up_service.rb
 create mode 100644 db/migrate/20181219235220_add_created_by_application_id_to_users.rb
 create mode 100644 spec/services/app_sign_up_service_spec.rb

(limited to 'spec/services')

diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb
index ac8de5fc0..2bf8e82db 100644
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@@ -68,7 +68,7 @@ class Api::BaseController < ApplicationController
   end
 
   def require_user!
-    if current_user && !current_user.disabled?
+    if current_user && !current_user.disabled? && current_user.confirmed?
       set_user_activity
     elsif current_user
       render json: { error: 'Your login is currently disabled' }, status: 403
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index f711c4676..6e4084c4e 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -1,14 +1,16 @@
 # frozen_string_literal: true
 
 class Api::V1::AccountsController < Api::BaseController
-  before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
+  before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:create, :follow, :unfollow, :block, :unblock, :mute, :unmute]
   before_action -> { doorkeeper_authorize! :follow, :'write:follows' }, only: [:follow, :unfollow]
   before_action -> { doorkeeper_authorize! :follow, :'write:mutes' }, only: [:mute, :unmute]
   before_action -> { doorkeeper_authorize! :follow, :'write:blocks' }, only: [:block, :unblock]
+  before_action -> { doorkeeper_authorize! :write, :'write:accounts' }, only: [:create]
 
-  before_action :require_user!, except: [:show]
-  before_action :set_account
+  before_action :require_user!, except: [:show, :create]
+  before_action :set_account, except: [:create]
   before_action :check_account_suspension, only: [:show]
+  before_action :check_enabled_registrations, only: [:create]
 
   respond_to :json
 
@@ -16,6 +18,16 @@ class Api::V1::AccountsController < Api::BaseController
     render json: @account, serializer: REST::AccountSerializer
   end
 
+  def create
+    token    = AppSignUpService.new.call(doorkeeper_token.application, account_params)
+    response = Doorkeeper::OAuth::TokenResponse.new(token)
+
+    headers.merge!(response.headers)
+
+    self.response_body = Oj.dump(response.body)
+    self.status        = response.status
+  end
+
   def follow
     FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs))
 
@@ -62,4 +74,12 @@ class Api::V1::AccountsController < Api::BaseController
   def check_account_suspension
     gone if @account.suspended?
   end
+
+  def account_params
+    params.permit(:username, :email, :password, :agreement)
+  end
+
+  def check_enabled_registrations
+    forbidden if single_user_mode? || !Setting.open_registrations
+  end
 end
diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb
index 7af9cbe81..c28c7471c 100644
--- a/app/controllers/auth/confirmations_controller.rb
+++ b/app/controllers/auth/confirmations_controller.rb
@@ -6,9 +6,9 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
   before_action :set_body_classes
   before_action :set_user, only: [:finish_signup]
 
-  # GET/PATCH /users/:id/finish_signup
   def finish_signup
     return unless request.patch? && params[:user]
+
     if @user.update(user_params)
       @user.skip_reconfirmation!
       bypass_sign_in(@user)
@@ -31,4 +31,12 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
   def user_params
     params.require(:user).permit(:email)
   end
+
+  def after_confirmation_path_for(_resource_name, user)
+    if user.created_by_application && truthy_param?(:redirect_to_app)
+      user.created_by_application.redirect_uri
+    else
+      super
+    end
+  end
 end
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 088832be3..f2a832542 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -26,6 +26,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController
 
     resource.locale      = I18n.locale
     resource.invite_code = params[:invite_code] if resource.invite_code.blank?
+    resource.agreement   = true
 
     resource.build_account if resource.account.nil?
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 44e0d1113..77e48ed4b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -36,6 +36,7 @@
 #  invite_id                 :bigint(8)
 #  remember_token            :string
 #  chosen_languages          :string           is an Array
+#  created_by_application_id :bigint(8)
 #
 
 class User < ApplicationRecord
@@ -66,6 +67,7 @@ class User < ApplicationRecord
 
   belongs_to :account, inverse_of: :user
   belongs_to :invite, counter_cache: :uses, optional: true
+  belongs_to :created_by_application, class_name: 'Doorkeeper::Application', optional: true
   accepts_nested_attributes_for :account
 
   has_many :applications, class_name: 'Doorkeeper::Application', as: :owner
@@ -74,6 +76,7 @@ class User < ApplicationRecord
   validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale?
   validates_with BlacklistedEmailValidator, if: :email_changed?
   validates_with EmailMxValidator, if: :validate_email_dns?
+  validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create
 
   scope :recent, -> { order(id: :desc) }
   scope :admins, -> { where(admin: true) }
@@ -294,7 +297,7 @@ class User < ApplicationRecord
       end
 
     if resource.blank?
-      resource = new(email: attributes[:email])
+      resource = new(email: attributes[:email], agreement: true)
       if Devise.check_at_sign && !resource[:email].index('@')
         resource[:email] = Rpam2.getenv(resource.find_pam_service, attributes[:email], attributes[:password], 'email', false)
         resource[:email] = "#{attributes[:email]}@#{resource.find_pam_suffix}" unless resource[:email]
@@ -307,7 +310,7 @@ class User < ApplicationRecord
     resource = joins(:account).find_by(accounts: { username: attributes[Devise.ldap_uid.to_sym].first })
 
     if resource.blank?
-      resource = new(email: attributes[:mail].first, account_attributes: { username: attributes[Devise.ldap_uid.to_sym].first })
+      resource = new(email: attributes[:mail].first, agreement: true, account_attributes: { username: attributes[Devise.ldap_uid.to_sym].first })
       resource.ldap_setup(attributes)
     end
 
diff --git a/app/services/app_sign_up_service.rb b/app/services/app_sign_up_service.rb
new file mode 100644
index 000000000..1878587e8
--- /dev/null
+++ b/app/services/app_sign_up_service.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class AppSignUpService < BaseService
+  def call(app, params)
+    return unless allowed_registrations?
+
+    user_params    = params.slice(:email, :password, :agreement)
+    account_params = params.slice(:username)
+    user           = User.create!(user_params.merge(created_by_application: app, password_confirmation: user_params[:password], account_attributes: account_params))
+
+    Doorkeeper::AccessToken.create!(application: app,
+                                    resource_owner_id: user.id,
+                                    scopes: app.scopes,
+                                    expires_in: Doorkeeper.configuration.access_token_expires_in,
+                                    use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?)
+  end
+
+  private
+
+  def allowed_registrations?
+    Setting.open_registrations && !Rails.configuration.x.single_user_mode
+  end
+end
diff --git a/app/views/user_mailer/confirmation_instructions.html.haml b/app/views/user_mailer/confirmation_instructions.html.haml
index 1f088a16f..f75f7529a 100644
--- a/app/views/user_mailer/confirmation_instructions.html.haml
+++ b/app/views/user_mailer/confirmation_instructions.html.haml
@@ -55,8 +55,12 @@
                             %tbody
                               %tr
                                 %td.button-primary
-                                  = link_to confirmation_url(@resource, confirmation_token: @token) do
-                                    %span= t 'devise.mailer.confirmation_instructions.action'
+                                  - if @resource.created_by_application
+                                    = link_to confirmation_url(@resource, confirmation_token: @token, redirect_to_app: 'true') do
+                                      %span= t 'devise.mailer.confirmation_instructions.action_with_app', app: @resource.created_by_application.name
+                                  - else
+                                    = link_to confirmation_url(@resource, confirmation_token: @token) do
+                                      %span= t 'devise.mailer.confirmation_instructions.action'
 
 %table.email-table{ cellspacing: 0, cellpadding: 0 }
   %tbody
diff --git a/app/views/user_mailer/confirmation_instructions.text.erb b/app/views/user_mailer/confirmation_instructions.text.erb
index e01eecb27..65b4626c6 100644
--- a/app/views/user_mailer/confirmation_instructions.text.erb
+++ b/app/views/user_mailer/confirmation_instructions.text.erb
@@ -4,7 +4,7 @@
 
 <%= t 'devise.mailer.confirmation_instructions.explanation', host: site_hostname %>
 
-=> <%= confirmation_url(@resource, confirmation_token: @token) %>
+=> <%= confirmation_url(@resource, confirmation_token: @token, redirect_to_app: @resource.created_by_application ? 'true' : nil) %>
 
 <%= strip_tags(t('devise.mailer.confirmation_instructions.extra_html', terms_path: about_more_url, policy_path: terms_url)) %>
 
diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb
index 8756b8fbf..35302e37b 100644
--- a/config/initializers/rack_attack.rb
+++ b/config/initializers/rack_attack.rb
@@ -57,6 +57,10 @@ class Rack::Attack
     req.authenticated_user_id if req.post? && req.path.start_with?('/api/v1/media')
   end
 
+  throttle('throttle_api_sign_up', limit: 5, period: 30.minutes) do |req|
+    req.ip if req.post? && req.path == '/api/v1/accounts'
+  end
+
   throttle('protected_paths', limit: 25, period: 5.minutes) do |req|
     req.ip if req.post? && req.path =~ PROTECTED_PATHS_REGEX
   end
diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml
index 20938e47b..bd0642b25 100644
--- a/config/locales/devise.en.yml
+++ b/config/locales/devise.en.yml
@@ -18,6 +18,7 @@ en:
     mailer:
       confirmation_instructions:
         action: Verify email address
+        action_with_app: Confirm and return to %{app}
         explanation: You have created an account on %{host} with this email address. You are one click away from activating it. If this wasn't you, please ignore this email.
         extra_html: Please also check out <a href="%{terms_path}">the rules of the instance</a> and <a href="%{policy_path}">our terms of service</a>.
         subject: 'Mastodon: Confirmation instructions for %{instance}'
diff --git a/config/routes.rb b/config/routes.rb
index 7723a08af..808bb5acd 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -336,7 +336,7 @@ Rails.application.routes.draw do
         resources :relationships, only: :index
       end
 
-      resources :accounts, only: [:show] do
+      resources :accounts, only: [:create, :show] do
         resources :statuses, only: :index, controller: 'accounts/statuses'
         resources :followers, only: :index, controller: 'accounts/follower_accounts'
         resources :following, only: :index, controller: 'accounts/following_accounts'
diff --git a/db/migrate/20181219235220_add_created_by_application_id_to_users.rb b/db/migrate/20181219235220_add_created_by_application_id_to_users.rb
new file mode 100644
index 000000000..17ce900af
--- /dev/null
+++ b/db/migrate/20181219235220_add_created_by_application_id_to_users.rb
@@ -0,0 +1,8 @@
+class AddCreatedByApplicationIdToUsers < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    add_reference :users, :created_by_application, foreign_key: { to_table: 'oauth_applications', on_delete: :nullify }, index: false
+    add_index :users, :created_by_application_id, algorithm: :concurrently
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 51a7b5e74..e47960b16 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2018_12_13_185533) do
+ActiveRecord::Schema.define(version: 2018_12_19_235220) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -637,8 +637,10 @@ ActiveRecord::Schema.define(version: 2018_12_13_185533) do
     t.bigint "invite_id"
     t.string "remember_token"
     t.string "chosen_languages", array: true
+    t.bigint "created_by_application_id"
     t.index ["account_id"], name: "index_users_on_account_id"
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
+    t.index ["created_by_application_id"], name: "index_users_on_created_by_application_id"
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
   end
@@ -730,6 +732,7 @@ ActiveRecord::Schema.define(version: 2018_12_13_185533) do
   add_foreign_key "subscriptions", "accounts", name: "fk_9847d1cbb5", on_delete: :cascade
   add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade
   add_foreign_key "users", "invites", on_delete: :nullify
+  add_foreign_key "users", "oauth_applications", column: "created_by_application_id", on_delete: :nullify
   add_foreign_key "web_push_subscriptions", "oauth_access_tokens", column: "access_token_id", on_delete: :cascade
   add_foreign_key "web_push_subscriptions", "users", on_delete: :cascade
   add_foreign_key "web_settings", "users", name: "fk_11910667b2", on_delete: :cascade
diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb
index b21968223..bbda244ea 100644
--- a/lib/mastodon/accounts_cli.rb
+++ b/lib/mastodon/accounts_cli.rb
@@ -73,7 +73,7 @@ module Mastodon
     def create(username)
       account  = Account.new(username: username)
       password = SecureRandom.hex
-      user     = User.new(email: options[:email], password: password, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: Time.now.utc)
+      user     = User.new(email: options[:email], password: password, agreement: true, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: options[:confirmed] ? Time.now.utc : nil)
 
       if options[:reattach]
         account = Account.find_local(username) || Account.new(username: username)
diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb
index c506fb5f0..f5f65c000 100644
--- a/spec/controllers/api/v1/accounts_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts_controller_spec.rb
@@ -19,6 +19,40 @@ RSpec.describe Api::V1::AccountsController, type: :controller do
     end
   end
 
+  describe 'POST #create' do
+    let(:app) { Fabricate(:application) }
+    let(:token) { Doorkeeper::AccessToken.find_or_create_for(app, nil, 'read write', nil, false) }
+    let(:agreement) { nil }
+
+    before do
+      post :create, params: { username: 'test', password: '12345678', email: 'hello@world.tld', agreement: agreement }
+    end
+
+    context 'given truthy agreement' do
+      let(:agreement) { 'true' }
+
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
+
+      it 'returns a new access token as JSON' do
+        expect(body_as_json[:access_token]).to_not be_blank
+      end
+
+      it 'creates a user' do
+        user = User.find_by(email: 'hello@world.tld')
+        expect(user).to_not be_nil
+        expect(user.created_by_application_id).to eq app.id
+      end
+    end
+
+    context 'given no agreement' do
+      it 'returns http unprocessable entity' do
+        expect(response).to have_http_status(422)
+      end
+    end
+  end
+
   describe 'GET #show' do
     let(:scopes) { 'read:accounts' }
 
diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb
index 7dfbdb52d..8f5956501 100644
--- a/spec/fabricators/user_fabricator.rb
+++ b/spec/fabricators/user_fabricator.rb
@@ -3,4 +3,5 @@ Fabricator(:user) do
   email        { sequence(:email) { |i| "#{i}#{Faker::Internet.email}" } }
   password     "123456789"
   confirmed_at { Time.zone.now }
+  agreement    true
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index c82919597..856254ce4 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -106,19 +106,19 @@ RSpec.describe User, type: :model do
     end
 
     it 'should allow a non-blacklisted user to be created' do
-      user = User.new(email: 'foo@example.com', account: account, password: password)
+      user = User.new(email: 'foo@example.com', account: account, password: password, agreement: true)
 
       expect(user.valid?).to be_truthy
     end
 
     it 'should not allow a blacklisted user to be created' do
-      user = User.new(email: 'foo@mvrht.com', account: account, password: password)
+      user = User.new(email: 'foo@mvrht.com', account: account, password: password, agreement: true)
 
       expect(user.valid?).to be_falsey
     end
 
     it 'should not allow a subdomain blacklisted user to be created' do
-      user = User.new(email: 'foo@mvrht.com.topdomain.tld', account: account, password: password)
+      user = User.new(email: 'foo@mvrht.com.topdomain.tld', account: account, password: password, agreement: true)
 
       expect(user.valid?).to be_falsey
     end
@@ -210,17 +210,17 @@ RSpec.describe User, type: :model do
     end
 
     it 'should not allow a user to be created unless they are whitelisted' do
-      user = User.new(email: 'foo@example.com', account: account, password: password)
+      user = User.new(email: 'foo@example.com', account: account, password: password, agreement: true)
       expect(user.valid?).to be_falsey
     end
 
     it 'should allow a user to be created if they are whitelisted' do
-      user = User.new(email: 'foo@mastodon.space', account: account, password: password)
+      user = User.new(email: 'foo@mastodon.space', account: account, password: password, agreement: true)
       expect(user.valid?).to be_truthy
     end
 
     it 'should not allow a user with a whitelisted top domain as subdomain in their email address to be created' do
-      user = User.new(email: 'foo@mastodon.space.userdomain.com', account: account, password: password)
+      user = User.new(email: 'foo@mastodon.space.userdomain.com', account: account, password: password, agreement: true)
       expect(user.valid?).to be_falsey
     end
 
@@ -242,7 +242,7 @@ RSpec.describe User, type: :model do
 
   it_behaves_like 'Settings-extended' do
     def create!
-      User.create!(account: Fabricate(:account), email: 'foo@mastodon.space', password: 'abcd1234')
+      User.create!(account: Fabricate(:account), email: 'foo@mastodon.space', password: 'abcd1234', agreement: true)
     end
 
     def fabricate
diff --git a/spec/services/app_sign_up_service_spec.rb b/spec/services/app_sign_up_service_spec.rb
new file mode 100644
index 000000000..d480df348
--- /dev/null
+++ b/spec/services/app_sign_up_service_spec.rb
@@ -0,0 +1,41 @@
+require 'rails_helper'
+
+RSpec.describe AppSignUpService, type: :service do
+  let(:app) { Fabricate(:application, scopes: 'read write') }
+  let(:good_params) { { username: 'alice', password: '12345678', email: 'good@email.com', agreement: true } }
+
+  subject { described_class.new }
+
+  describe '#call' do
+    it 'returns nil when registrations are closed' do
+      Setting.open_registrations = false
+      expect(subject.call(app, good_params)).to be_nil
+    end
+
+    it 'raises an error when params are missing' do
+      expect { subject.call(app, {}) }.to raise_error ActiveRecord::RecordInvalid
+    end
+
+    it 'creates an unconfirmed user with access token' do
+      access_token = subject.call(app, good_params)
+      expect(access_token).to_not be_nil
+      user = User.find_by(id: access_token.resource_owner_id)
+      expect(user).to_not be_nil
+      expect(user.confirmed?).to be false
+    end
+
+    it 'creates access token with the app\'s scopes' do
+      access_token = subject.call(app, good_params)
+      expect(access_token).to_not be_nil
+      expect(access_token.scopes.to_s).to eq 'read write'
+    end
+
+    it 'creates an account' do
+      access_token = subject.call(app, good_params)
+      expect(access_token).to_not be_nil
+      user = User.find_by(id: access_token.resource_owner_id)
+      expect(user).to_not be_nil
+      expect(user.account).to_not be_nil
+    end
+  end
+end
-- 
cgit 


From 290932602b32e3bdc2a4c5cb278fb73755a2cd52 Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Sun, 30 Dec 2018 09:48:59 +0100
Subject: Reduce usage of LD signatures (#9659)

* Do not LDS-sign Follow, Accept, Reject, Undo, Block

* Do not use LDS for Create activities of private toots

* Minor cleanup

* Ignore unsigned activities instead of misattributing them

* Use status.distributable? instead of querying visibility directly
---
 app/lib/activitypub/activity/follow.rb                     |  2 +-
 app/services/activitypub/process_collection_service.rb     |  2 +-
 app/services/after_block_domain_from_account_service.rb    |  4 ++--
 app/services/authorize_follow_service.rb                   |  4 ++--
 app/services/block_service.rb                              |  6 ++----
 app/services/follow_service.rb                             |  6 ++----
 app/services/process_mentions_service.rb                   |  6 ++++--
 app/services/reject_follow_service.rb                      |  4 ++--
 app/services/unblock_service.rb                            |  4 ++--
 app/services/unfollow_service.rb                           |  4 ++--
 app/workers/activitypub/distribution_worker.rb             | 14 +++++++++-----
 app/workers/activitypub/reply_distribution_worker.rb       | 12 ++++++++----
 .../activitypub/process_collection_service_spec.rb         |  6 +++---
 13 files changed, 40 insertions(+), 34 deletions(-)

(limited to 'spec/services')

diff --git a/app/lib/activitypub/activity/follow.rb b/app/lib/activitypub/activity/follow.rb
index 7ca650fdf..1e805c0d1 100644
--- a/app/lib/activitypub/activity/follow.rb
+++ b/app/lib/activitypub/activity/follow.rb
@@ -28,7 +28,7 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity
   end
 
   def reject_follow_request!(target_account)
-    json = Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(FollowRequest.new(account: @account, target_account: target_account, uri: @json['id']), serializer: ActivityPub::RejectFollowSerializer, adapter: ActivityPub::Adapter).as_json).sign!(target_account))
+    json = ActiveModelSerializers::SerializableResource.new(FollowRequest.new(account: @account, target_account: target_account, uri: @json['id']), serializer: ActivityPub::RejectFollowSerializer, adapter: ActivityPub::Adapter).to_json
     ActivityPub::DeliveryWorker.perform_async(json, target_account.id, @account.inbox_url)
   end
 end
diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb
index 79cdca297..5c54aad89 100644
--- a/app/services/activitypub/process_collection_service.rb
+++ b/app/services/activitypub/process_collection_service.rb
@@ -27,7 +27,7 @@ class ActivityPub::ProcessCollectionService < BaseService
   private
 
   def different_actor?
-    @json['actor'].present? && value_or_id(@json['actor']) != @account.uri && @json['signature'].present?
+    @json['actor'].present? && value_or_id(@json['actor']) != @account.uri
   end
 
   def process_items(items)
diff --git a/app/services/after_block_domain_from_account_service.rb b/app/services/after_block_domain_from_account_service.rb
index 56cc819fb..180f13403 100644
--- a/app/services/after_block_domain_from_account_service.rb
+++ b/app/services/after_block_domain_from_account_service.rb
@@ -31,11 +31,11 @@ class AfterBlockDomainFromAccountService < BaseService
 
     return unless follow.account.activitypub?
 
-    json = Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    json = ActiveModelSerializers::SerializableResource.new(
       follow,
       serializer: ActivityPub::RejectFollowSerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(@account))
+    ).to_json
 
     ActivityPub::DeliveryWorker.perform_async(json, @account.id, follow.account.inbox_url)
   end
diff --git a/app/services/authorize_follow_service.rb b/app/services/authorize_follow_service.rb
index 1674239df..f2e3ebe7d 100644
--- a/app/services/authorize_follow_service.rb
+++ b/app/services/authorize_follow_service.rb
@@ -24,11 +24,11 @@ class AuthorizeFollowService < BaseService
   end
 
   def build_json(follow_request)
-    Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    ActiveModelSerializers::SerializableResource.new(
       follow_request,
       serializer: ActivityPub::AcceptFollowSerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(follow_request.target_account))
+    ).to_json
   end
 
   def build_xml(follow_request)
diff --git a/app/services/block_service.rb b/app/services/block_service.rb
index b39c3eef2..140b238df 100644
--- a/app/services/block_service.rb
+++ b/app/services/block_service.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class BlockService < BaseService
-  include StreamEntryRenderer
-
   def call(account, target_account)
     return if account.id == target_account.id
 
@@ -27,11 +25,11 @@ class BlockService < BaseService
   end
 
   def build_json(block)
-    Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    ActiveModelSerializers::SerializableResource.new(
       block,
       serializer: ActivityPub::BlockSerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(block.account))
+    ).to_json
   end
 
   def build_xml(block)
diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb
index 862926260..9d36a1449 100644
--- a/app/services/follow_service.rb
+++ b/app/services/follow_service.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class FollowService < BaseService
-  include StreamEntryRenderer
-
   # Follow a remote user, notify remote user about the follow
   # @param [Account] source_account From which to follow
   # @param [String, Account] uri User URI to follow in the form of username@domain (or account record)
@@ -82,10 +80,10 @@ class FollowService < BaseService
   end
 
   def build_json(follow_request)
-    Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    ActiveModelSerializers::SerializableResource.new(
       follow_request,
       serializer: ActivityPub::FollowSerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(follow_request.account))
+    ).to_json
   end
 end
diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb
index ec7d33b1d..2595c5fd3 100644
--- a/app/services/process_mentions_service.rb
+++ b/app/services/process_mentions_service.rb
@@ -60,11 +60,13 @@ class ProcessMentionsService < BaseService
   end
 
   def activitypub_json
-    @activitypub_json ||= Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    return @activitypub_json if defined?(@activitypub_json)
+    payload = ActiveModelSerializers::SerializableResource.new(
       @status,
       serializer: ActivityPub::ActivitySerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(@status.account))
+    ).as_json
+    @activitypub_json = Oj.dump(@status.distributable? ? ActivityPub::LinkedDataSignature.new(payload).sign!(@status.account) : payload)
   end
 
   def resolve_account_service
diff --git a/app/services/reject_follow_service.rb b/app/services/reject_follow_service.rb
index c1f7bcb60..a91266aa4 100644
--- a/app/services/reject_follow_service.rb
+++ b/app/services/reject_follow_service.rb
@@ -19,11 +19,11 @@ class RejectFollowService < BaseService
   end
 
   def build_json(follow_request)
-    Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    ActiveModelSerializers::SerializableResource.new(
       follow_request,
       serializer: ActivityPub::RejectFollowSerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(follow_request.target_account))
+    ).to_json
   end
 
   def build_xml(follow_request)
diff --git a/app/services/unblock_service.rb b/app/services/unblock_service.rb
index 869f62d1c..72fc5ab15 100644
--- a/app/services/unblock_service.rb
+++ b/app/services/unblock_service.rb
@@ -20,11 +20,11 @@ class UnblockService < BaseService
   end
 
   def build_json(unblock)
-    Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    ActiveModelSerializers::SerializableResource.new(
       unblock,
       serializer: ActivityPub::UndoBlockSerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(unblock.account))
+    ).to_json
   end
 
   def build_xml(block)
diff --git a/app/services/unfollow_service.rb b/app/services/unfollow_service.rb
index 73a64929f..03e45912d 100644
--- a/app/services/unfollow_service.rb
+++ b/app/services/unfollow_service.rb
@@ -43,11 +43,11 @@ class UnfollowService < BaseService
   end
 
   def build_json(follow)
-    Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(
+    ActiveModelSerializers::SerializableResource.new(
       follow,
       serializer: ActivityPub::UndoFollowSerializer,
       adapter: ActivityPub::Adapter
-    ).as_json).sign!(follow.account))
+    ).to_json
   end
 
   def build_xml(follow)
diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb
index 17c1ef7ff..59aacabfd 100644
--- a/app/workers/activitypub/distribution_worker.rb
+++ b/app/workers/activitypub/distribution_worker.rb
@@ -12,7 +12,7 @@ class ActivityPub::DistributionWorker
     return if skip_distribution?
 
     ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url|
-      [signed_payload, @account.id, inbox_url]
+      [payload, @account.id, inbox_url]
     end
 
     relay! if relayable?
@@ -35,20 +35,24 @@ class ActivityPub::DistributionWorker
   end
 
   def signed_payload
-    @signed_payload ||= Oj.dump(ActivityPub::LinkedDataSignature.new(payload).sign!(@account))
+    Oj.dump(ActivityPub::LinkedDataSignature.new(unsigned_payload).sign!(@account))
   end
 
-  def payload
-    @payload ||= ActiveModelSerializers::SerializableResource.new(
+  def unsigned_payload
+    ActiveModelSerializers::SerializableResource.new(
       @status,
       serializer: ActivityPub::ActivitySerializer,
       adapter: ActivityPub::Adapter
     ).as_json
   end
 
+  def payload
+    @payload ||= @status.distributable? ? signed_payload : Oj.dump(unsigned_payload)
+  end
+
   def relay!
     ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url|
-      [signed_payload, @account.id, inbox_url]
+      [payload, @account.id, inbox_url]
     end
   end
 end
diff --git a/app/workers/activitypub/reply_distribution_worker.rb b/app/workers/activitypub/reply_distribution_worker.rb
index c0ed3a1f4..892cc1474 100644
--- a/app/workers/activitypub/reply_distribution_worker.rb
+++ b/app/workers/activitypub/reply_distribution_worker.rb
@@ -12,7 +12,7 @@ class ActivityPub::ReplyDistributionWorker
     return unless @account.present? && @status.distributable?
 
     ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url|
-      [signed_payload, @status.account_id, inbox_url]
+      [payload, @status.account_id, inbox_url]
     end
   rescue ActiveRecord::RecordNotFound
     true
@@ -25,14 +25,18 @@ class ActivityPub::ReplyDistributionWorker
   end
 
   def signed_payload
-    @signed_payload ||= Oj.dump(ActivityPub::LinkedDataSignature.new(payload).sign!(@status.account))
+    Oj.dump(ActivityPub::LinkedDataSignature.new(unsigned_payload).sign!(@status.account))
   end
 
-  def payload
-    @payload ||= ActiveModelSerializers::SerializableResource.new(
+  def unsigned_payload
+    ActiveModelSerializers::SerializableResource.new(
       @status,
       serializer: ActivityPub::ActivitySerializer,
       adapter: ActivityPub::Adapter
     ).as_json
   end
+
+  def payload
+    @payload ||= @status.distributable? ? signed_payload : Oj.dump(unsigned_payload)
+  end
 end
diff --git a/spec/services/activitypub/process_collection_service_spec.rb b/spec/services/activitypub/process_collection_service_spec.rb
index bbe97d211..b3baf6b6b 100644
--- a/spec/services/activitypub/process_collection_service_spec.rb
+++ b/spec/services/activitypub/process_collection_service_spec.rb
@@ -26,9 +26,9 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do
     context 'when actor differs from sender' do
       let(:forwarder) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/other_account') }
 
-      it 'processes payload with sender if no signature exists' do
-        expect_any_instance_of(ActivityPub::LinkedDataSignature).not_to receive(:verify_account!)
-        expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), forwarder, instance_of(Hash))
+      it 'does not process payload if no signature exists' do
+        expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_account!).and_return(nil)
+        expect(ActivityPub::Activity).not_to receive(:factory)
 
         subject.call(json, forwarder)
       end
-- 
cgit