From 2f34b747b3f765a37d7b23e70de42005c0b62f58 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 23 Aug 2018 23:26:29 +0200 Subject: Allow mods to disable login, improve message when login disabled (#8329) * Allow moderators to disable/enable login * Instead of rejecting login, show forbidden error when login disabled Avoid confusion because when login is rejected, the message is that the account is not activated, which is wrong. * Fix tests --- app/controllers/api/base_controller.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/controllers/api') diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 770a69921..0b3735087 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -7,6 +7,8 @@ class Api::BaseController < ApplicationController include RateLimitHeaders skip_before_action :store_current_location + skip_before_action :check_user_permissions + protect_from_forgery with: :null_session rescue_from ActiveRecord::RecordInvalid, Mastodon::ValidationError do |e| -- cgit From da13fa50215c5153ee199eb8e70e7fa030523a00 Mon Sep 17 00:00:00 2001 From: Quint Guvernator Date: Sun, 26 Aug 2018 13:22:46 -0400 Subject: Fix low-hanging rubocop gripes (#8458) * rubocop: quit being so picky * rubocop: miscellany * rubocop: prefer present to blank --- .rubocop.yml | 6 +++++ .../api/v1/lists/accounts_controller.rb | 2 +- app/controllers/api/v1/lists_controller.rb | 2 +- app/lib/feed_manager.rb | 2 +- app/mailers/user_mailer.rb | 2 +- app/models/concerns/expireable.rb | 2 +- app/models/user.rb | 4 ++-- lib/tasks/db.rake | 2 +- lib/tasks/mastodon.rake | 28 ++++++++++------------ 9 files changed, 26 insertions(+), 24 deletions(-) (limited to 'app/controllers/api') diff --git a/.rubocop.yml b/.rubocop.yml index 4f9e09b43..4ba5903bd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -62,6 +62,9 @@ Metrics/ParameterLists: Metrics/PerceivedComplexity: Max: 20 +Naming/MemoizedInstanceVariableName: + Enabled: false + Rails: Enabled: true @@ -71,6 +74,9 @@ Rails/HasAndBelongsToMany: Rails/SkipsModelValidations: Enabled: false +Rails/HttpStatus: + Enabled: false + Style/ClassAndModuleChildren: Enabled: false diff --git a/app/controllers/api/v1/lists/accounts_controller.rb b/app/controllers/api/v1/lists/accounts_controller.rb index 19de56732..ec4477034 100644 --- a/app/controllers/api/v1/lists/accounts_controller.rb +++ b/app/controllers/api/v1/lists/accounts_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Api::V1::Lists::AccountsController < Api::BaseController - before_action -> { doorkeeper_authorize! :read, :'read:lists' }, only: [:show] + before_action -> { doorkeeper_authorize! :read, :'read:lists' }, only: [:show] before_action -> { doorkeeper_authorize! :write, :'write:lists' }, except: [:show] before_action :require_user! diff --git a/app/controllers/api/v1/lists_controller.rb b/app/controllers/api/v1/lists_controller.rb index b42b8b971..054172bee 100644 --- a/app/controllers/api/v1/lists_controller.rb +++ b/app/controllers/api/v1/lists_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Api::V1::ListsController < Api::BaseController - before_action -> { doorkeeper_authorize! :read, :'read:lists' }, only: [:index, :show] + before_action -> { doorkeeper_authorize! :read, :'read:lists' }, only: [:index, :show] before_action -> { doorkeeper_authorize! :write, :'write:lists' }, except: [:index, :show] before_action :require_user! diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 14cba70dc..b59a9f1cd 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -288,7 +288,7 @@ class FeedManager # remains in the set. We could pick a random element, but this # set should generally be small, and it seems ideal to show the # oldest potential such reblog. - other_reblog = redis.smembers(reblog_set_key).map(&:to_i).sort.first + other_reblog = redis.smembers(reblog_set_key).map(&:to_i).min redis.zadd(timeline_key, other_reblog, other_reblog) if other_reblog diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 9848c34a2..aa76b4dfe 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -16,7 +16,7 @@ class UserMailer < Devise::Mailer return if @resource.disabled? I18n.with_locale(@resource.locale || I18n.default_locale) do - mail to: @resource.unconfirmed_email.blank? ? @resource.email : @resource.unconfirmed_email, + mail to: @resource.unconfirmed_email.presence || @resource.email, subject: I18n.t(@resource.pending_reconfirmation? ? 'devise.mailer.reconfirmation_instructions.subject' : 'devise.mailer.confirmation_instructions.subject', instance: @instance), template_name: @resource.pending_reconfirmation? ? 'reconfirmation_instructions' : 'confirmation_instructions' end diff --git a/app/models/concerns/expireable.rb b/app/models/concerns/expireable.rb index 444ccdfdb..2c0631476 100644 --- a/app/models/concerns/expireable.rb +++ b/app/models/concerns/expireable.rb @@ -9,7 +9,7 @@ module Expireable attr_reader :expires_in def expires_in=(interval) - self.expires_at = interval.to_i.seconds.from_now unless interval.blank? + self.expires_at = interval.to_i.seconds.from_now if interval.present? @expires_in = interval end diff --git a/app/models/user.rb b/app/models/user.rb index 75b7e9e7c..25f77ed14 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -98,7 +98,7 @@ class User < ApplicationRecord :reduce_motion, :system_font_ui, :noindex, :theme, :display_sensitive_media, :hide_network, :default_language, to: :settings, prefix: :setting, allow_nil: false - attr_accessor :invite_code + attr_reader :invite_code def pam_conflict(_) # block pam login tries on traditional account @@ -258,7 +258,7 @@ class User < ApplicationRecord end def invite_code=(code) - self.invite = Invite.find_by(code: code) unless code.blank? + self.invite = Invite.find_by(code: code) if code.present? @invite_code = code end diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index 32039c31d..b76e90131 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -18,7 +18,7 @@ def each_schema_load_environment # needing to do the same, and we can't even use the same method # to do it. - if Rails.env == 'development' + if Rails.env.development? test_conf = ActiveRecord::Base.configurations['test'] if test_conf['database']&.present? diff --git a/lib/tasks/mastodon.rake b/lib/tasks/mastodon.rake index 7455478b6..649a22a0b 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -280,14 +280,14 @@ namespace :mastodon do begin ActionMailer::Base.smtp_settings = { - :port => env['SMTP_PORT'], - :address => env['SMTP_SERVER'], - :user_name => env['SMTP_LOGIN'].presence, - :password => env['SMTP_PASSWORD'].presence, - :domain => env['LOCAL_DOMAIN'], - :authentication => env['SMTP_AUTH_METHOD'] == 'none' ? nil : env['SMTP_AUTH_METHOD'] || :plain, - :openssl_verify_mode => env['SMTP_OPENSSL_VERIFY_MODE'], - :enable_starttls_auto => true, + port: env['SMTP_PORT'], + address: env['SMTP_SERVER'], + user_name: env['SMTP_LOGIN'].presence, + password: env['SMTP_PASSWORD'].presence, + domain: env['LOCAL_DOMAIN'], + authentication: env['SMTP_AUTH_METHOD'] == 'none' ? nil : env['SMTP_AUTH_METHOD'] || :plain, + openssl_verify_mode: env['SMTP_OPENSSL_VERIFY_MODE'], + enable_starttls_auto: true, } ActionMailer::Base.default_options = { @@ -326,13 +326,11 @@ namespace :mastodon do if prompt.yes?('Prepare the database now?') prompt.say 'Running `RAILS_ENV=production rails db:setup` ...' - prompt.say "\n" + prompt.say "\n\n" if cmd.run!({ RAILS_ENV: 'production', SAFETY_ASSURED: 1 }, :rails, 'db:setup').failure? - prompt.say "\n" prompt.error 'That failed! Perhaps your configuration is not right' else - prompt.say "\n" prompt.ok 'Done!' end end @@ -343,13 +341,11 @@ namespace :mastodon do if prompt.yes?('Compile the assets now?') prompt.say 'Running `RAILS_ENV=production rails assets:precompile` ...' - prompt.say "\n" + prompt.say "\n\n" if cmd.run!({ RAILS_ENV: 'production' }, :rails, 'assets:precompile').failure? - prompt.say "\n" prompt.error 'That failed! Maybe you need swap space?' else - prompt.say "\n" prompt.say 'Done!' end end @@ -715,10 +711,10 @@ namespace :mastodon do pastel = Pastel.new duplicate_masters.each do |account| - puts pastel.yellow("First of their name: ") + pastel.bold(account.username) + " (#{admin_account_url(account.id)})" + puts pastel.yellow('First of their name: ') + pastel.bold(account.username) + " (#{admin_account_url(account.id)})" Account.where('lower(username) = ?', account.username.downcase).where.not(id: account.id).each do |duplicate| - puts " " + pastel.red("Duplicate: ") + admin_account_url(duplicate.id) + puts ' ' + pastel.red('Duplicate: ') + admin_account_url(duplicate.id) end end end -- cgit From f3a12ddfd0b5b9379c7cfe4229697765851f4738 Mon Sep 17 00:00:00 2001 From: Jakub Mendyk Date: Sun, 26 Aug 2018 21:30:17 +0200 Subject: Make Api::V1::MutesController paginate properly (#8472) Fixes #8463 --- app/controllers/api/v1/mutes_controller.rb | 26 +++++------ spec/controllers/api/v1/mutes_controller_spec.rb | 56 +++++++++++++++++++++--- 2 files changed, 61 insertions(+), 21 deletions(-) (limited to 'app/controllers/api') diff --git a/app/controllers/api/v1/mutes_controller.rb b/app/controllers/api/v1/mutes_controller.rb index faa7d16cd..df6c8e86c 100644 --- a/app/controllers/api/v1/mutes_controller.rb +++ b/app/controllers/api/v1/mutes_controller.rb @@ -15,19 +15,17 @@ class Api::V1::MutesController < Api::BaseController private def load_accounts - default_accounts.merge(paginated_mutes).to_a - end - - def default_accounts - Account.includes(:muted_by).references(:muted_by) + paginated_mutes.map(&:target_account) end def paginated_mutes - Mute.where(account: current_account).paginate_by_max_id( - limit_param(DEFAULT_ACCOUNTS_LIMIT), - params[:max_id], - params[:since_id] - ) + @paginated_mutes ||= Mute.eager_load(:target_account) + .where(account: current_account) + .paginate_by_max_id( + limit_param(DEFAULT_ACCOUNTS_LIMIT), + params[:max_id], + params[:since_id] + ) end def insert_pagination_headers @@ -41,21 +39,21 @@ class Api::V1::MutesController < Api::BaseController end def prev_path - unless @accounts.empty? + unless paginated_mutes.empty? api_v1_mutes_url pagination_params(since_id: pagination_since_id) end end def pagination_max_id - @accounts.last.muted_by_ids.last + paginated_mutes.last.id end def pagination_since_id - @accounts.first.muted_by_ids.first + paginated_mutes.first.id end def records_continue? - @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) + paginated_mutes.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) end def pagination_params(core_params) diff --git a/spec/controllers/api/v1/mutes_controller_spec.rb b/spec/controllers/api/v1/mutes_controller_spec.rb index f9603b7ff..a2b814a69 100644 --- a/spec/controllers/api/v1/mutes_controller_spec.rb +++ b/spec/controllers/api/v1/mutes_controller_spec.rb @@ -3,19 +3,61 @@ require 'rails_helper' RSpec.describe Api::V1::MutesController, type: :controller do render_views - let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:mutes') } + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:scopes) { 'read:mutes' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } - before do - Fabricate(:mute, account: user.account, hide_notifications: false) - allow(controller).to receive(:doorkeeper_token) { token } - end + before { allow(controller).to receive(:doorkeeper_token) { token } } describe 'GET #index' do - it 'returns http success' do + it 'limits according to limit parameter' do + 2.times.map { Fabricate(:mute, account: user.account) } get :index, params: { limit: 1 } + expect(body_as_json.size).to eq 1 + end + + it 'queries mutes in range according to max_id' do + mutes = 2.times.map { Fabricate(:mute, account: user.account) } + + get :index, params: { max_id: mutes[1] } + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq mutes[0].target_account_id.to_s + end + + it 'queries mutes in range according to since_id' do + mutes = 2.times.map { Fabricate(:mute, account: user.account) } + + get :index, params: { since_id: mutes[0] } + + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq mutes[1].target_account_id.to_s + end + + it 'sets pagination header for next path' do + mutes = 2.times.map { Fabricate(:mute, account: user.account) } + get :index, params: { limit: 1, since_id: mutes[0] } + expect(response.headers['Link'].find_link(['rel', 'next']).href).to eq api_v1_mutes_url(limit: 1, max_id: mutes[1]) + end + + it 'sets pagination header for previous path' do + mute = Fabricate(:mute, account: user.account) + get :index + expect(response.headers['Link'].find_link(['rel', 'prev']).href).to eq api_v1_mutes_url(since_id: mute) + end + + it 'returns http success' do + get :index expect(response).to have_http_status(200) end + + context 'with wrong scopes' do + let(:scopes) { 'write:mutes' } + + it 'returns http forbidden' do + get :index + expect(response).to have_http_status(403) + end + end end end -- cgit