From 628275fc311d08139072bfdc6c565c07ade2b53f Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 21 Nov 2017 12:22:24 -0500 Subject: [PATCH] FIX: Some badge routes were still working even with badges disabled --- app/controllers/user_badges_controller.rb | 6 ++++++ app/controllers/users_controller.rb | 8 +++++++- config/routes.rb | 2 +- spec/controllers/user_badges_controller_spec.rb | 10 ++++++++-- spec/requests/users_controller_spec.rb | 13 +++++++++++++ 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/app/controllers/user_badges_controller.rb b/app/controllers/user_badges_controller.rb index d8f44a41ea2..11ca9878fee 100644 --- a/app/controllers/user_badges_controller.rb +++ b/app/controllers/user_badges_controller.rb @@ -1,4 +1,6 @@ class UserBadgesController < ApplicationController + before_action :ensure_badges_enabled + def index params.permit [:granted_before, :offset, :username] @@ -106,4 +108,8 @@ class UserBadgesController < ApplicationController master_api_call = current_user.nil? && is_api? master_api_call || guardian.can_grant_badges?(user) end + + def ensure_badges_enabled + raise Discourse::NotFound unless SiteSetting.enable_badges? + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 22e73584198..db3182bc5ba 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -8,7 +8,7 @@ require_dependency 'admin_confirmation' class UsersController < ApplicationController skip_before_action :authorize_mini_profiler, only: [:avatar] - skip_before_action :check_xhr, only: [:show, :password_reset, :update, :account_created, :activate_account, :perform_account_activation, :user_preferences_redirect, :avatar, :my_redirect, :toggle_anon, :admin_login, :confirm_admin] + 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, :pick_avatar, :destroy_user_image, :destroy, :check_emails, :topic_tracking_state] @@ -67,6 +67,7 @@ class UsersController < ApplicationController format.html do @restrict_fields = guardian.restrict_user_fields?(@user) store_preloaded("user_#{@user.username}", MultiJson.dump(user_serializer)) + render :show end format.json do @@ -75,6 +76,11 @@ class UsersController < ApplicationController end end + def badges + raise Discourse::NotFound unless SiteSetting.enable_badges? + show + end + def card_badge end diff --git a/config/routes.rb b/config/routes.rb index 776a61264e2..9a4baf6fa13 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -391,7 +391,7 @@ Discourse::Application.routes.draw do get "#{root_path}/:username/activity.rss" => "posts#user_posts_feed", format: :rss, constraints: { username: USERNAME_ROUTE_FORMAT } get "#{root_path}/:username/activity" => "users#show", constraints: { username: USERNAME_ROUTE_FORMAT } get "#{root_path}/:username/activity/:filter" => "users#show", constraints: { username: USERNAME_ROUTE_FORMAT } - get "#{root_path}/:username/badges" => "users#show", constraints: { username: USERNAME_ROUTE_FORMAT } + get "#{root_path}/:username/badges" => "users#badges", constraints: { username: USERNAME_ROUTE_FORMAT } get "#{root_path}/:username/notifications" => "users#show", constraints: { username: USERNAME_ROUTE_FORMAT } get "#{root_path}/:username/notifications/:filter" => "users#show", constraints: { username: USERNAME_ROUTE_FORMAT } get "#{root_path}/:username/activity/pending" => "users#show", constraints: { username: USERNAME_ROUTE_FORMAT } diff --git a/spec/controllers/user_badges_controller_spec.rb b/spec/controllers/user_badges_controller_spec.rb index a9a276e6794..161809bdb51 100644 --- a/spec/controllers/user_badges_controller_spec.rb +++ b/spec/controllers/user_badges_controller_spec.rb @@ -5,19 +5,25 @@ describe UserBadgesController do let(:badge) { Fabricate(:badge) } context 'index' do + let(:badge) { Fabricate(:badge, target_posts: true, show_posts: false) } it 'does not leak private info' do - badge = Fabricate(:badge, target_posts: true, show_posts: false) p = create_post UserBadge.create(badge: badge, user: user, post_id: p.id, granted_by_id: -1, granted_at: Time.now) get :index, params: { badge_id: badge.id }, format: :json - expect(response.status).to eq(200) + expect(response).to be_success parsed = JSON.parse(response.body) expect(parsed["topics"]).to eq(nil) expect(parsed["badges"].length).to eq(1) expect(parsed["user_badge_info"]["user_badges"][0]["post_id"]).to eq(nil) end + + it "fails when badges are disabled" do + SiteSetting.enable_badges = false + get :index, params: { badge_id: badge.id }, format: :json + expect(response).not_to be_success + end end context 'index' do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 94fb106ec9d..b6e3184c1c5 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -26,6 +26,19 @@ RSpec.describe UsersController do end end + describe "#badges" do + it "renders fine by default" do + get "/u/#{user.username}/badges" + expect(response).to be_success + end + + it "fails if badges are disabled" do + SiteSetting.enable_badges = false + get "/u/#{user.username}/badges" + expect(response).not_to be_success + end + end + describe "updating a user" do before do sign_in(user)