From 4c03e05a4e1a237f8a414a0861c03abe3269dbc8 Mon Sep 17 00:00:00 2001 From: James Kiesel Date: Tue, 6 Nov 2018 06:53:25 +1300 Subject: Allow joining several hashtags in a single column (#8904) * Nascent tag menu on frontend * Hook up frontend to search * Tag intersection backend first pass * Update yarnlock * WIP * Fix for tags not searching correctly * Make radio buttons function * Simplify radio buttons with modeOption * Better naming * Rearrange options * Add all/any/none functionality on backend * Small PR cleanup * Move to service from scope * Small cleanup, add proper service tests * Don't use send with user input :D * Set appropriate column header * Handle auto updating timeline * Fix up toggle function * Use tag value correctly * A bit more correct to use 'self' rather than 'all' in status scope * Fix some style issues * Fix more code style issues * Style select dropdown more better * Only use to_id'ed value to ensure no SQL injection * Revamp frontend to allow for multiple selects * Update backend / col header to account for more flexible tagging * Update brakeman ignore * Codeclimate suggestions * Fix presenter tag_url * Implement initial PR feedback * Handle additional tag streaming * CodeClimate tweak --- app/controllers/api/v1/timelines/tag_controller.rb | 2 +- app/controllers/tags_controller.rb | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v1/timelines/tag_controller.rb b/app/controllers/api/v1/timelines/tag_controller.rb index cf58d5cf4..92c32c178 100644 --- a/app/controllers/api/v1/timelines/tag_controller.rb +++ b/app/controllers/api/v1/timelines/tag_controller.rb @@ -45,7 +45,7 @@ class Api::V1::Timelines::TagController < Api::BaseController end def tag_timeline_statuses - Status.as_tag_timeline(@tag, current_account, truthy_param?(:local)) + HashtagQueryService.new.call(@tag, params.slice(:any, :all, :none), current_account, truthy_param?(:local)) end def insert_pagination_headers diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 8772509d5..8e4051834 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -16,14 +16,15 @@ class TagsController < ApplicationController end format.rss do - @statuses = Status.as_tag_timeline(@tag).limit(PAGE_SIZE) + @statuses = HashtagQueryService.new.call(@tag, params.slice(:any, :all, :none)).limit(PAGE_SIZE) @statuses = cache_collection(@statuses, Status) render xml: RSS::TagSerializer.render(@tag, @statuses) end format.json do - @statuses = Status.as_tag_timeline(@tag, current_account, params[:local]).paginate_by_max_id(PAGE_SIZE, params[:max_id]) + @statuses = HashtagQueryService.new.call(@tag, params.slice(:any, :all, :none), current_account, params[:local]) + .paginate_by_max_id(PAGE_SIZE, params[:max_id]) @statuses = cache_collection(@statuses, Status) render json: collection_presenter, @@ -46,7 +47,7 @@ class TagsController < ApplicationController def collection_presenter ActivityPub::CollectionPresenter.new( - id: tag_url(@tag), + id: tag_url(@tag, params.slice(:any, :all, :none)), type: :ordered, size: @tag.statuses.count, items: @statuses.map { |s| ActivityPub::TagManager.instance.uri_for(s) } -- cgit From 6d59dfa15d873da75c731b79367ab6b3d1b2f5a5 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 8 Nov 2018 21:05:42 +0100 Subject: Optimize the process of following someone (#9220) * Eliminate extra accounts select query from FollowService * Optimistically update follow state in web UI and hide loading bar Fix #6205 * Asynchronize NotifyService in FollowService And fix failing test * Skip Webfinger resolve routine when called from FollowService if possible If an account is ActivityPub, then webfinger re-resolving is not necessary when called from FollowService. Improve options of ResolveAccountService --- app/controllers/api/v1/accounts_controller.rb | 2 +- app/javascript/mastodon/actions/accounts.js | 18 +++++++++--- app/javascript/mastodon/reducers/relationships.js | 12 ++++++++ app/services/concerns/author_extractor.rb | 2 +- app/services/follow_service.rb | 8 +++--- app/services/process_mentions_service.rb | 2 +- app/services/resolve_account_service.rb | 32 ++++++++++++++-------- app/workers/local_notification_worker.rb | 13 +++++++-- .../authorize_interactions_controller_spec.rb | 4 ++- 9 files changed, 67 insertions(+), 26 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 1d5372a8c..f711c4676 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -17,7 +17,7 @@ class Api::V1::AccountsController < Api::BaseController end def follow - FollowService.new.call(current_user.account, @account.acct, reblogs: truthy_param?(:reblogs)) + FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs)) options = @account.locked? ? {} : { following_map: { @account.id => { reblogs: truthy_param?(:reblogs) } }, requested_map: { @account.id => false } } diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js index cbae62a0f..d4a824e2c 100644 --- a/app/javascript/mastodon/actions/accounts.js +++ b/app/javascript/mastodon/actions/accounts.js @@ -145,12 +145,14 @@ export function fetchAccountFail(id, error) { export function followAccount(id, reblogs = true) { return (dispatch, getState) => { const alreadyFollowing = getState().getIn(['relationships', id, 'following']); - dispatch(followAccountRequest(id)); + const locked = getState().getIn(['accounts', id, 'locked'], false); + + dispatch(followAccountRequest(id, locked)); api(getState).post(`/api/v1/accounts/${id}/follow`, { reblogs }).then(response => { dispatch(followAccountSuccess(response.data, alreadyFollowing)); }).catch(error => { - dispatch(followAccountFail(error)); + dispatch(followAccountFail(error, locked)); }); }; }; @@ -167,10 +169,12 @@ export function unfollowAccount(id) { }; }; -export function followAccountRequest(id) { +export function followAccountRequest(id, locked) { return { type: ACCOUNT_FOLLOW_REQUEST, id, + locked, + skipLoading: true, }; }; @@ -179,13 +183,16 @@ export function followAccountSuccess(relationship, alreadyFollowing) { type: ACCOUNT_FOLLOW_SUCCESS, relationship, alreadyFollowing, + skipLoading: true, }; }; -export function followAccountFail(error) { +export function followAccountFail(error, locked) { return { type: ACCOUNT_FOLLOW_FAIL, error, + locked, + skipLoading: true, }; }; @@ -193,6 +200,7 @@ export function unfollowAccountRequest(id) { return { type: ACCOUNT_UNFOLLOW_REQUEST, id, + skipLoading: true, }; }; @@ -201,6 +209,7 @@ export function unfollowAccountSuccess(relationship, statuses) { type: ACCOUNT_UNFOLLOW_SUCCESS, relationship, statuses, + skipLoading: true, }; }; @@ -208,6 +217,7 @@ export function unfollowAccountFail(error) { return { type: ACCOUNT_UNFOLLOW_FAIL, error, + skipLoading: true, }; }; diff --git a/app/javascript/mastodon/reducers/relationships.js b/app/javascript/mastodon/reducers/relationships.js index f46049297..8322780de 100644 --- a/app/javascript/mastodon/reducers/relationships.js +++ b/app/javascript/mastodon/reducers/relationships.js @@ -1,6 +1,10 @@ import { ACCOUNT_FOLLOW_SUCCESS, + ACCOUNT_FOLLOW_REQUEST, + ACCOUNT_FOLLOW_FAIL, ACCOUNT_UNFOLLOW_SUCCESS, + ACCOUNT_UNFOLLOW_REQUEST, + ACCOUNT_UNFOLLOW_FAIL, ACCOUNT_BLOCK_SUCCESS, ACCOUNT_UNBLOCK_SUCCESS, ACCOUNT_MUTE_SUCCESS, @@ -37,6 +41,14 @@ const initialState = ImmutableMap(); export default function relationships(state = initialState, action) { switch(action.type) { + case ACCOUNT_FOLLOW_REQUEST: + return state.setIn([action.id, action.locked ? 'requested' : 'following'], true); + case ACCOUNT_FOLLOW_FAIL: + return state.setIn([action.id, action.locked ? 'requested' : 'following'], false); + case ACCOUNT_UNFOLLOW_REQUEST: + return state.setIn([action.id, 'following'], false); + case ACCOUNT_UNFOLLOW_FAIL: + return state.setIn([action.id, 'following'], true); case ACCOUNT_FOLLOW_SUCCESS: case ACCOUNT_UNFOLLOW_SUCCESS: case ACCOUNT_BLOCK_SUCCESS: diff --git a/app/services/concerns/author_extractor.rb b/app/services/concerns/author_extractor.rb index 1e00eb803..c2419e9ec 100644 --- a/app/services/concerns/author_extractor.rb +++ b/app/services/concerns/author_extractor.rb @@ -18,6 +18,6 @@ module AuthorExtractor acct = "#{username}@#{domain}" end - ResolveAccountService.new.call(acct, update_profile) + ResolveAccountService.new.call(acct, update_profile: update_profile) end end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index f6888a68d..0020bc9fe 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -7,9 +7,9 @@ class FollowService < BaseService # @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) # @param [true, false, nil] reblogs Whether or not to show reblogs, defaults to true - def call(source_account, uri, reblogs: nil) + def call(source_account, target_account, reblogs: nil) reblogs = true if reblogs.nil? - target_account = uri.is_a?(Account) ? uri : ResolveAccountService.new.call(uri) + target_account = ResolveAccountService.new.call(target_account, skip_webfinger: true) raise ActiveRecord::RecordNotFound if target_account.nil? || target_account.id == source_account.id || target_account.suspended? raise Mastodon::NotPermittedError if target_account.blocking?(source_account) || source_account.blocking?(target_account) @@ -42,7 +42,7 @@ class FollowService < BaseService follow_request = FollowRequest.create!(account: source_account, target_account: target_account, show_reblogs: reblogs) if target_account.local? - NotifyService.new.call(target_account, follow_request) + LocalNotificationWorker.perform_async(target_account.id, follow_request.id, follow_request.class.name) elsif target_account.ostatus? NotificationWorker.perform_async(build_follow_request_xml(follow_request), source_account.id, target_account.id) AfterRemoteFollowRequestWorker.perform_async(follow_request.id) @@ -57,7 +57,7 @@ class FollowService < BaseService follow = source_account.follow!(target_account, reblogs: reblogs) if target_account.local? - NotifyService.new.call(target_account, follow) + LocalNotificationWorker.perform_async(target_account.id, follow.id, follow.class.name) else Pubsubhubbub::SubscribeWorker.perform_async(target_account.id) unless target_account.subscribed? NotificationWorker.perform_async(build_follow_xml(follow), source_account.id, target_account.id) diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index b4641c4b4..ec7d33b1d 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -47,7 +47,7 @@ class ProcessMentionsService < BaseService mentioned_account = mention.account if mentioned_account.local? - LocalNotificationWorker.perform_async(mention.id) + LocalNotificationWorker.perform_async(mentioned_account.id, mention.id, mention.class.name) elsif mentioned_account.ostatus? && !@status.stream_entry.hidden? NotificationWorker.perform_async(ostatus_xml, @status.account_id, mentioned_account.id) elsif mentioned_account.activitypub? diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 4323e7f06..c3064211d 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -9,17 +9,27 @@ class ResolveAccountService < BaseService # Find or create a local account for a remote user. # When creating, look up the user's webfinger and fetch all # important information from their feed - # @param [String] uri User URI in the form of username@domain + # @param [String, Account] uri User URI in the form of username@domain + # @param [Hash] options # @return [Account] - def call(uri, update_profile = true, redirected = nil) - @username, @domain = uri.split('@') - @update_profile = update_profile + def call(uri, options = {}) + @options = options - return Account.find_local(@username) if TagManager.instance.local_domain?(@domain) + if uri.is_a?(Account) + @account = uri + @username = @account.username + @domain = @account.domain + + return @account if @account.local? || !webfinger_update_due? + else + @username, @domain = uri.split('@') - @account = Account.find_remote(@username, @domain) + return Account.find_local(@username) if TagManager.instance.local_domain?(@domain) - return @account unless webfinger_update_due? + @account = Account.find_remote(@username, @domain) + + return @account unless webfinger_update_due? + end Rails.logger.debug "Looking up webfinger for #{uri}" @@ -30,8 +40,8 @@ class ResolveAccountService < BaseService if confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero? @username = confirmed_username @domain = confirmed_domain - elsif redirected.nil? - return call("#{confirmed_username}@#{confirmed_domain}", update_profile, true) + elsif options[:redirected].nil? + return call("#{confirmed_username}@#{confirmed_domain}", options.merge(redirected: true)) else Rails.logger.debug 'Requested and returned acct URIs do not match' return @@ -76,7 +86,7 @@ class ResolveAccountService < BaseService end def webfinger_update_due? - @account.nil? || @account.possibly_stale? + @account.nil? || ((!@options[:skip_webfinger] || @account.ostatus?) && @account.possibly_stale?) end def activitypub_ready? @@ -93,7 +103,7 @@ class ResolveAccountService < BaseService end def update_profile? - @update_profile + @options[:update_profile] end def handle_activitypub diff --git a/app/workers/local_notification_worker.rb b/app/workers/local_notification_worker.rb index 748270563..48635e498 100644 --- a/app/workers/local_notification_worker.rb +++ b/app/workers/local_notification_worker.rb @@ -3,9 +3,16 @@ class LocalNotificationWorker include Sidekiq::Worker - def perform(mention_id) - mention = Mention.find(mention_id) - NotifyService.new.call(mention.account, mention) + def perform(receiver_account_id, activity_id = nil, activity_class_name = nil) + if activity_id.nil? && activity_class_name.nil? + activity = Mention.find(receiver_account_id) + receiver = activity.account + else + receiver = Account.find(receiver_account_id) + activity = activity_class_name.constantize.find(activity_id) + end + + NotifyService.new.call(receiver, activity) rescue ActiveRecord::RecordNotFound true end diff --git a/spec/controllers/authorize_interactions_controller_spec.rb b/spec/controllers/authorize_interactions_controller_spec.rb index 81fd9ceb7..ce4257b68 100644 --- a/spec/controllers/authorize_interactions_controller_spec.rb +++ b/spec/controllers/authorize_interactions_controller_spec.rb @@ -99,10 +99,12 @@ describe AuthorizeInteractionsController do allow(ResolveAccountService).to receive(:new).and_return(service) allow(service).to receive(:call).with('user@hostname').and_return(target_account) + allow(service).to receive(:call).with(target_account, skip_webfinger: true).and_return(target_account) + post :create, params: { acct: 'acct:user@hostname' } - expect(service).to have_received(:call).with('user@hostname') + expect(service).to have_received(:call).with(target_account, skip_webfinger: true) expect(account.following?(target_account)).to be true expect(response).to render_template(:success) end -- cgit From 46155122859657e674a0fab097c6812349c35274 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 8 Nov 2018 21:35:58 +0100 Subject: Reduce connect timeout limit and limit signature failures by source IP (#9236) * Reduce connect timeout from 10s to 1s * Limit failing signature verifications per source IP --- app/controllers/concerns/signature_verification.rb | 7 ++++++- app/lib/request.rb | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index e5d5e2ca6..7e491641b 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -43,7 +43,12 @@ module SignatureVerification return end - account = account_from_key_id(signature_params['keyId']) + account_stoplight = Stoplight("source:#{request.ip}") { account_from_key_id(signature_params['keyId']) } + .with_fallback { nil } + .with_threshold(1) + .with_cool_off_time(5.minutes.seconds) + + account = account_stoplight.run if account.nil? @signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}" diff --git a/app/lib/request.rb b/app/lib/request.rb index 36c211dbf..73b495ce1 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -94,7 +94,7 @@ class Request end def timeout - { write: 10, connect: 10, read: 10 } + { connect: 1, read: 10, write: 10 } end def http_client -- cgit