FIX: don't return 200s when login is required to paths

When running `ensure_login_required` it should always happen prior to
`check_xhr` cause check xhr will trigger a 200 response
This commit is contained in:
Sam
2018-02-01 12:26:45 +11:00
parent 7d2283167a
commit f2e7b74d88
28 changed files with 81 additions and 59 deletions

View File

@ -1,8 +1,8 @@
require_dependency 'rate_limiter' require_dependency 'rate_limiter'
class AboutController < ApplicationController class AboutController < ApplicationController
prepend_before_action :check_xhr, :ensure_logged_in, only: [:live_post_counts]
skip_before_action :check_xhr, only: [:index] skip_before_action :check_xhr, only: [:index]
before_action :ensure_logged_in, only: [:live_post_counts]
def index def index
return redirect_to path('/login') if SiteSetting.login_required? && current_user.nil? return redirect_to path('/login') if SiteSetting.login_required? && current_user.nil?

View File

@ -1,7 +1,7 @@
class Admin::AdminController < ApplicationController class Admin::AdminController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
before_action :ensure_staff prepend_before_action :check_xhr, :ensure_staff
def index def index
render body: nil render body: nil

View File

@ -1,7 +1,5 @@
class Admin::EmbeddableHostsController < Admin::AdminController class Admin::EmbeddableHostsController < Admin::AdminController
before_action :ensure_logged_in, :ensure_staff
def create def create
save_host(EmbeddableHost.new) save_host(EmbeddableHost.new)
end end

View File

@ -2,7 +2,7 @@ require_dependency 'embedding'
class Admin::EmbeddingController < Admin::AdminController class Admin::EmbeddingController < Admin::AdminController
before_action :ensure_logged_in, :ensure_staff, :fetch_embedding before_action :fetch_embedding
def show def show
render_serialized(@embedding, EmbeddingSerializer, root: 'embedding', rest_serializer: true) render_serialized(@embedding, EmbeddingSerializer, root: 'embedding', rest_serializer: true)

View File

@ -2,7 +2,7 @@ require_dependency 'category_serializer'
class CategoriesController < ApplicationController class CategoriesController < ApplicationController
before_action :ensure_logged_in, except: [:index, :categories_and_latest, :show, :redirect, :find_by_slug] prepend_before_action :check_xhr, :ensure_logged_in, except: [:index, :categories_and_latest, :show, :redirect, :find_by_slug]
before_action :fetch_category, only: [:show, :update, :destroy] before_action :fetch_category, only: [:show, :update, :destroy]
before_action :initialize_staff_action_logger, only: [:create, :update, :destroy] before_action :initialize_staff_action_logger, only: [:create, :update, :destroy]
skip_before_action :check_xhr, only: [:index, :categories_and_latest, :redirect] skip_before_action :check_xhr, only: [:index, :categories_and_latest, :redirect]

View File

@ -1,5 +1,5 @@
class CategoryHashtagsController < ApplicationController class CategoryHashtagsController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
def check def check
category_slugs = params[:category_slugs] category_slugs = params[:category_slugs]

View File

@ -2,7 +2,7 @@ require_dependency 'html_to_markdown'
class ComposerController < ApplicationController class ComposerController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
def parse_html def parse_html
markdown_text = HtmlToMarkdown.new(params[:html]).to_markdown markdown_text = HtmlToMarkdown.new(params[:html]).to_markdown

View File

@ -2,7 +2,7 @@ require_dependency 'composer_messages_finder'
class ComposerMessagesController < ApplicationController class ComposerMessagesController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
def index def index
finder = ComposerMessagesFinder.new(current_user, params.slice(:composer_action, :topic_id, :post_id)) finder = ComposerMessagesFinder.new(current_user, params.slice(:composer_action, :topic_id, :post_id))

View File

@ -1,6 +1,5 @@
class DraftController < ApplicationController class DraftController < ApplicationController
before_action :ensure_logged_in prepend_before_action :ensure_logged_in
# TODO really do we need to skip this?
skip_before_action :check_xhr, :preload_json skip_before_action :check_xhr, :preload_json
def show def show

View File

@ -1,7 +1,7 @@
class EmailController < ApplicationController class EmailController < ApplicationController
skip_before_action :check_xhr, :preload_json, :redirect_to_login_if_required
layout 'no_ember' layout 'no_ember'
skip_before_action :check_xhr, :preload_json, :redirect_to_login_if_required
before_action :ensure_logged_in, only: :preferences_redirect before_action :ensure_logged_in, only: :preferences_redirect
def preferences_redirect def preferences_redirect

View File

@ -1,6 +1,6 @@
class GroupsController < ApplicationController class GroupsController < ApplicationController
before_action :ensure_logged_in, only: [ prepend_before_action :check_xhr, :ensure_logged_in, only: [
:set_notifications, :set_notifications,
:mentionable, :mentionable,
:messageable, :messageable,

View File

@ -1,7 +1,7 @@
require_dependency 'inline_oneboxer' require_dependency 'inline_oneboxer'
class InlineOneboxController < ApplicationController class InlineOneboxController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
def show def show
oneboxes = InlineOneboxer.new(params[:urls] || []).process oneboxes = InlineOneboxer.new(params[:urls] || []).process

View File

@ -2,11 +2,15 @@ require_dependency 'rate_limiter'
class InvitesController < ApplicationController class InvitesController < ApplicationController
prepend_before_action :check_xhr, :ensure_logged_in, only: [
:destroy, :create, :create_invite_link, :rescind_all_invites,
:resend_invite, :resend_all_invites, :upload_csv
]
skip_before_action :check_xhr, except: [:perform_accept_invitation] skip_before_action :check_xhr, except: [:perform_accept_invitation]
skip_before_action :preload_json, except: [:show] skip_before_action :preload_json, except: [:show]
skip_before_action :redirect_to_login_if_required skip_before_action :redirect_to_login_if_required
before_action :ensure_logged_in, only: [:destroy, :create, :create_invite_link, :rescind_all_invites, :resend_invite, :resend_all_invites, :upload_csv]
before_action :ensure_new_registrations_allowed, only: [:show, :perform_accept_invitation] before_action :ensure_new_registrations_allowed, only: [:show, :perform_accept_invitation]
before_action :ensure_not_logged_in, only: [:show, :perform_accept_invitation] before_action :ensure_not_logged_in, only: [:show, :perform_accept_invitation]

View File

@ -2,7 +2,7 @@ require_dependency 'notification_serializer'
class NotificationsController < ApplicationController class NotificationsController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
def index def index
user = user =

View File

@ -1,7 +1,7 @@
require_dependency 'oneboxer' require_dependency 'oneboxer'
class OneboxController < ApplicationController class OneboxController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
def show def show
unless params[:refresh] == 'true' unless params[:refresh] == 'true'

View File

@ -1,7 +1,7 @@
require_dependency 'discourse' require_dependency 'discourse'
class PostActionsController < ApplicationController class PostActionsController < ApplicationController
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
before_action :fetch_post_from_params before_action :fetch_post_from_params
before_action :fetch_post_action_type_id_from_params before_action :fetch_post_action_type_id_from_params

View File

@ -8,7 +8,7 @@ require_dependency 'post_locker'
class PostsController < ApplicationController class PostsController < ApplicationController
before_action :ensure_logged_in, except: [ prepend_before_action :check_xhr, :ensure_logged_in, except: [
:show, :show,
:replies, :replies,
:by_number, :by_number,

View File

@ -5,7 +5,7 @@ require_dependency 'wizard/step_updater'
class StepsController < ApplicationController class StepsController < ApplicationController
before_action :ensure_wizard_enabled before_action :ensure_wizard_enabled
before_action :ensure_logged_in prepend_before_action :check_xhr, :ensure_logged_in
before_action :ensure_admin before_action :ensure_admin
def update def update

View File

@ -1,6 +1,6 @@
class TagGroupsController < ApplicationController class TagGroupsController < ApplicationController
prepend_before_action :check_xhr, :ensure_logged_in, except: [:index, :show]
skip_before_action :check_xhr, only: [:index, :show] skip_before_action :check_xhr, only: [:index, :show]
before_action :ensure_logged_in, except: [:index, :show]
before_action :fetch_tag_group, only: [:show, :update, :destroy] before_action :fetch_tag_group, only: [:show, :update, :destroy]
def index def index

View File

@ -7,8 +7,7 @@ class TagsController < ::ApplicationController
before_action :ensure_tags_enabled before_action :ensure_tags_enabled
skip_before_action :check_xhr, only: [:tag_feed, :show, :index] prepend_before_action :check_xhr, :ensure_logged_in, except: [
before_action :ensure_logged_in, except: [
:index, :index,
:show, :show,
:tag_feed, :tag_feed,
@ -16,7 +15,11 @@ class TagsController < ::ApplicationController
:check_hashtag, :check_hashtag,
Discourse.anonymous_filters.map { |f| :"show_#{f}" } Discourse.anonymous_filters.map { |f| :"show_#{f}" }
].flatten ].flatten
before_action :set_category_from_params, except: [:index, :update, :destroy, :tag_feed, :search, :notifications, :update_notifications]
skip_before_action :check_xhr, only: [:tag_feed, :show, :index]
before_action :set_category_from_params, except: [:index, :update, :destroy,
:tag_feed, :search, :notifications, :update_notifications]
def index def index
@description_meta = I18n.t("tags.title") @description_meta = I18n.t("tags.title")

View File

@ -6,31 +6,32 @@ require_dependency 'discourse_event'
require_dependency 'rate_limiter' require_dependency 'rate_limiter'
class TopicsController < ApplicationController class TopicsController < ApplicationController
before_action :ensure_logged_in, only: [:timings, prepend_before_action :check_xhr, :ensure_logged_in, only: [
:destroy_timings, :timings,
:update, :destroy_timings,
:star, :update,
:destroy, :destroy,
:recover, :recover,
:status, :status,
:invite, :invite,
:mute, :mute,
:unmute, :unmute,
:set_notifications, :set_notifications,
:move_posts, :move_posts,
:merge_topic, :merge_topic,
:clear_pin, :clear_pin,
:re_pin, :re_pin,
:status_update, :status_update,
:timer, :timer,
:bulk, :bulk,
:reset_new, :reset_new,
:change_post_owners, :change_post_owners,
:change_timestamps, :change_timestamps,
:archive_message, :archive_message,
:move_to_inbox, :move_to_inbox,
:convert_topic, :convert_topic,
:bookmark] :bookmark
]
before_action :consider_user_for_promotion, only: :show before_action :consider_user_for_promotion, only: :show

View File

@ -2,7 +2,7 @@ require "mini_mime"
require_dependency 'upload_creator' require_dependency 'upload_creator'
class UploadsController < ApplicationController class UploadsController < ApplicationController
before_action :ensure_logged_in, except: [:show] prepend_before_action :check_xhr, :ensure_logged_in, except: [:show]
skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show] skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show]
def create def create

View File

@ -2,9 +2,9 @@ class UserApiKeysController < ApplicationController
layout 'no_ember' layout 'no_ember'
prepend_before_action :check_xhr, :ensure_logged_in, only: [:create, :revoke, :undo_revoke]
skip_before_action :redirect_to_login_if_required, only: [:new] skip_before_action :redirect_to_login_if_required, only: [:new]
skip_before_action :check_xhr, :preload_json skip_before_action :check_xhr, :preload_json
before_action :ensure_logged_in, only: [:create, :revoke, :undo_revoke]
AUTH_API_VERSION ||= 2 AUTH_API_VERSION ||= 2

View File

@ -8,10 +8,18 @@ require_dependency 'admin_confirmation'
class UsersController < ApplicationController class UsersController < ApplicationController
skip_before_action :authorize_mini_profiler, only: [:avatar] skip_before_action :authorize_mini_profiler, only: [:avatar]
skip_before_action :check_xhr, only: [:show, :badges, :password_reset, :update, :account_created, :activate_account, :perform_account_activation, :user_preferences_redirect, :avatar, :my_redirect, :toggle_anon, :admin_login, :confirm_admin]
before_action :ensure_logged_in, only: [:username, :update, :user_preferences_redirect, :upload_user_image, prepend_before_action :check_xhr, :ensure_logged_in, only: [
:pick_avatar, :destroy_user_image, :destroy, :check_emails, :topic_tracking_state] :username, :update, :user_preferences_redirect, :upload_user_image,
:pick_avatar, :destroy_user_image, :destroy, :check_emails, :topic_tracking_state,
:preferences
]
skip_before_action :check_xhr, only: [
:show, :badges, :password_reset, :update, :account_created,
:activate_account, :perform_account_activation, :user_preferences_redirect, :avatar,
:my_redirect, :toggle_anon, :admin_login, :confirm_admin
]
before_action :respond_to_suspicious_request, only: [:create] before_action :respond_to_suspicious_request, only: [:create]

View File

@ -4,7 +4,7 @@ require_dependency 'email_updater'
class UsersEmailController < ApplicationController class UsersEmailController < ApplicationController
before_action :ensure_logged_in, only: [:index, :update] prepend_before_action :check_xhr, :ensure_logged_in, only: [:index, :update]
skip_before_action :check_xhr, only: [:confirm] skip_before_action :check_xhr, only: [:confirm]
skip_before_action :redirect_to_login_if_required, only: [:confirm] skip_before_action :redirect_to_login_if_required, only: [:confirm]

View File

@ -2,10 +2,9 @@ require_dependency 'wizard'
require_dependency 'wizard/builder' require_dependency 'wizard/builder'
class WizardController < ApplicationController class WizardController < ApplicationController
prepend_before_action :check_xhr, :ensure_admin, except: [:qunit]
prepend_before_action :check_xhr, :ensure_logged_in, except: [:qunit]
before_action :ensure_wizard_enabled, only: [:index] before_action :ensure_wizard_enabled, only: [:index]
before_action :ensure_logged_in, except: [:qunit]
before_action :ensure_admin, except: [:qunit]
skip_before_action :check_xhr, :preload_json skip_before_action :check_xhr, :preload_json
layout false layout false

View File

@ -14,6 +14,13 @@ describe WizardController do
expect(response.status).to eq(403) expect(response.status).to eq(403)
end end
it 'needs you to be logged in' do
get :index
# for whatever reason, no access is 404
# we may want to revisit this at some point and make it 403
expect(response.status).to eq(404)
end
it "raises an error if you aren't an admin" do it "raises an error if you aren't an admin" do
log_in(:moderator) log_in(:moderator)
get :index, format: :json get :index, format: :json

View File

@ -4,5 +4,8 @@ RSpec.describe Admin::AdminController do
it "should return the right response if user isn't a staff" do it "should return the right response if user isn't a staff" do
get "/admin", params: { api_key: 'asdiasiduga' } get "/admin", params: { api_key: 'asdiasiduga' }
expect(response.status).to eq(404) expect(response.status).to eq(404)
get "/admin"
expect(response.status).to eq(404)
end end
end end