From 41986cdb2f5cfd690fa0d7defaf4ce2a88e5259d Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 1 Feb 2018 15:17:59 +1100
Subject: [PATCH] Refactor requires login logic, reduce duplicate code

This also corrects the positioning in the chain of the check
and removes misuse of prepend_before_action
---
 app/controllers/about_controller.rb           |  4 ++-
 app/controllers/admin/admin_controller.rb     |  4 +--
 app/controllers/application_controller.rb     | 25 ++++++++++++++++++-
 app/controllers/categories_controller.rb      |  3 ++-
 .../category_hashtags_controller.rb           |  2 +-
 app/controllers/composer_controller.rb        |  2 +-
 .../composer_messages_controller.rb           |  2 +-
 app/controllers/draft_controller.rb           |  3 ++-
 app/controllers/groups_controller.rb          |  2 +-
 app/controllers/inline_onebox_controller.rb   |  2 +-
 app/controllers/invites_controller.rb         |  2 +-
 app/controllers/notifications_controller.rb   |  2 +-
 app/controllers/onebox_controller.rb          |  2 +-
 app/controllers/post_actions_controller.rb    |  3 ++-
 app/controllers/posts_controller.rb           |  2 +-
 app/controllers/steps_controller.rb           |  2 +-
 app/controllers/tag_groups_controller.rb      |  3 ++-
 app/controllers/tags_controller.rb            |  2 +-
 app/controllers/topics_controller.rb          |  2 +-
 app/controllers/uploads_controller.rb         |  3 ++-
 app/controllers/user_api_keys_controller.rb   |  2 +-
 app/controllers/users_controller.rb           |  2 +-
 app/controllers/users_email_controller.rb     |  2 +-
 app/controllers/wizard_controller.rb          |  5 ++--
 24 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb
index 48c401dee9a..a95cedfca73 100644
--- a/app/controllers/about_controller.rb
+++ b/app/controllers/about_controller.rb
@@ -1,7 +1,9 @@
 require_dependency 'rate_limiter'
 
 class AboutController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in, only: [:live_post_counts]
+
+  requires_login only: [:live_post_counts]
+
   skip_before_action :check_xhr, only: [:index]
 
   def index
diff --git a/app/controllers/admin/admin_controller.rb b/app/controllers/admin/admin_controller.rb
index e4dd34119f6..2a33fd6c906 100644
--- a/app/controllers/admin/admin_controller.rb
+++ b/app/controllers/admin/admin_controller.rb
@@ -1,7 +1,7 @@
 class Admin::AdminController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in
-  prepend_before_action :check_xhr, :ensure_staff
+  requires_login
+  before_action :ensure_staff
 
   def index
     render body: nil
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 479082fc6ac..93e05225b20 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -48,8 +48,9 @@ class ApplicationController < ActionController::Base
   before_action :set_mobile_view
   before_action :block_if_readonly_mode
   before_action :authorize_mini_profiler
-  before_action :preload_json
   before_action :redirect_to_login_if_required
+  before_action :block_if_requires_login
+  before_action :preload_json
   before_action :check_xhr
   after_action  :add_readonly_header
   after_action  :perform_refresh_session
@@ -570,6 +571,28 @@ class ApplicationController < ActionController::Base
       raise RenderEmpty.new unless ((request.format && request.format.json?) || request.xhr?)
     end
 
+    def self.requires_login(arg = {})
+      @requires_login_arg = arg
+    end
+
+    def self.requires_login_arg
+      @requires_login_arg
+    end
+
+    def block_if_requires_login
+      if arg = self.class.requires_login_arg
+        check =
+          if except = arg[:except]
+            !except.include?(action_name.to_sym)
+          elsif only = arg[:only]
+            only.include?(action_name.to_sym)
+          else
+            true
+          end
+        ensure_logged_in if check
+      end
+    end
+
     def ensure_logged_in
       raise Discourse::NotLoggedIn.new unless current_user.present?
     end
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index 7b9a25ec5cd..58a95610c4d 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -2,7 +2,8 @@ require_dependency 'category_serializer'
 
 class CategoriesController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in, except: [:index, :categories_and_latest, :show, :redirect, :find_by_slug]
+  requires_login except: [:index, :categories_and_latest, :show, :redirect, :find_by_slug]
+
   before_action :fetch_category, only: [:show, :update, :destroy]
   before_action :initialize_staff_action_logger, only: [:create, :update, :destroy]
   skip_before_action :check_xhr, only: [:index, :categories_and_latest, :redirect]
diff --git a/app/controllers/category_hashtags_controller.rb b/app/controllers/category_hashtags_controller.rb
index 34e8bb0367c..4bbb85a4061 100644
--- a/app/controllers/category_hashtags_controller.rb
+++ b/app/controllers/category_hashtags_controller.rb
@@ -1,5 +1,5 @@
 class CategoryHashtagsController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in
+  requires_login
 
   def check
     category_slugs = params[:category_slugs]
diff --git a/app/controllers/composer_controller.rb b/app/controllers/composer_controller.rb
index 7d103c634e9..f5cb462ba47 100644
--- a/app/controllers/composer_controller.rb
+++ b/app/controllers/composer_controller.rb
@@ -2,7 +2,7 @@ require_dependency 'html_to_markdown'
 
 class ComposerController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in
+  requires_login
 
   def parse_html
     markdown_text = HtmlToMarkdown.new(params[:html]).to_markdown
diff --git a/app/controllers/composer_messages_controller.rb b/app/controllers/composer_messages_controller.rb
index 64b3bc53822..26e879b109c 100644
--- a/app/controllers/composer_messages_controller.rb
+++ b/app/controllers/composer_messages_controller.rb
@@ -2,7 +2,7 @@ require_dependency 'composer_messages_finder'
 
 class ComposerMessagesController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in
+  requires_login
 
   def index
     finder = ComposerMessagesFinder.new(current_user, params.slice(:composer_action, :topic_id, :post_id))
diff --git a/app/controllers/draft_controller.rb b/app/controllers/draft_controller.rb
index c3f9345c5b2..ebd0dd1bccb 100644
--- a/app/controllers/draft_controller.rb
+++ b/app/controllers/draft_controller.rb
@@ -1,5 +1,6 @@
 class DraftController < ApplicationController
-  prepend_before_action :ensure_logged_in
+  requires_login
+
   skip_before_action :check_xhr, :preload_json
 
   def show
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index e199c7cc89b..74f5479d261 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -1,6 +1,6 @@
 class GroupsController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in, only: [
+  requires_login only: [
     :set_notifications,
     :mentionable,
     :messageable,
diff --git a/app/controllers/inline_onebox_controller.rb b/app/controllers/inline_onebox_controller.rb
index 33d2a61f271..832f3d66a07 100644
--- a/app/controllers/inline_onebox_controller.rb
+++ b/app/controllers/inline_onebox_controller.rb
@@ -1,7 +1,7 @@
 require_dependency 'inline_oneboxer'
 
 class InlineOneboxController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in
+  requires_login
 
   def show
     oneboxes = InlineOneboxer.new(params[:urls] || []).process
diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
index aea0c9782ff..8773a349705 100644
--- a/app/controllers/invites_controller.rb
+++ b/app/controllers/invites_controller.rb
@@ -2,7 +2,7 @@ require_dependency 'rate_limiter'
 
 class InvitesController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in, only: [
+  requires_login only: [
     :destroy, :create, :create_invite_link, :rescind_all_invites,
     :resend_invite, :resend_all_invites, :upload_csv
   ]
diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index 16780e44d9d..e86545c01b7 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -2,7 +2,7 @@ require_dependency 'notification_serializer'
 
 class NotificationsController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in
+  requires_login
 
   def index
     user =
diff --git a/app/controllers/onebox_controller.rb b/app/controllers/onebox_controller.rb
index 65b66b5964c..7c0d16af322 100644
--- a/app/controllers/onebox_controller.rb
+++ b/app/controllers/onebox_controller.rb
@@ -1,7 +1,7 @@
 require_dependency 'oneboxer'
 
 class OneboxController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in
+  requires_login
 
   def show
     unless params[:refresh] == 'true'
diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb
index 022a3072e2c..fa0314b999c 100644
--- a/app/controllers/post_actions_controller.rb
+++ b/app/controllers/post_actions_controller.rb
@@ -1,7 +1,8 @@
 require_dependency 'discourse'
 
 class PostActionsController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in
+  requires_login
+
   before_action :fetch_post_from_params
   before_action :fetch_post_action_type_id_from_params
 
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index 0ac234182d8..e3e4a21910c 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -8,7 +8,7 @@ require_dependency 'post_locker'
 
 class PostsController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in, except: [
+  requires_login except: [
     :show,
     :replies,
     :by_number,
diff --git a/app/controllers/steps_controller.rb b/app/controllers/steps_controller.rb
index 8a341d53d3f..d2b9285c67d 100644
--- a/app/controllers/steps_controller.rb
+++ b/app/controllers/steps_controller.rb
@@ -3,9 +3,9 @@ require_dependency 'wizard/builder'
 require_dependency 'wizard/step_updater'
 
 class StepsController < ApplicationController
+  requires_login
 
   before_action :ensure_wizard_enabled
-  prepend_before_action :check_xhr, :ensure_logged_in
   before_action :ensure_admin
 
   def update
diff --git a/app/controllers/tag_groups_controller.rb b/app/controllers/tag_groups_controller.rb
index e039fb6cb7c..8d819827f76 100644
--- a/app/controllers/tag_groups_controller.rb
+++ b/app/controllers/tag_groups_controller.rb
@@ -1,5 +1,6 @@
 class TagGroupsController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in, except: [:index, :show]
+  requires_login except: [:index, :show]
+
   skip_before_action :check_xhr, only: [:index, :show]
   before_action :fetch_tag_group, only: [:show, :update, :destroy]
 
diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb
index 0c63c7a7eed..b97de2b8178 100644
--- a/app/controllers/tags_controller.rb
+++ b/app/controllers/tags_controller.rb
@@ -7,7 +7,7 @@ class TagsController < ::ApplicationController
 
   before_action :ensure_tags_enabled
 
-  prepend_before_action :check_xhr, :ensure_logged_in, except: [
+  requires_login except: [
     :index,
     :show,
     :tag_feed,
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 6d4f0555344..76e6c879e11 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -6,7 +6,7 @@ require_dependency 'discourse_event'
 require_dependency 'rate_limiter'
 
 class TopicsController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in, only: [
+  requires_login only: [
     :timings,
     :destroy_timings,
     :update,
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index 13754d3f274..39fc24124be 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -2,7 +2,8 @@ require "mini_mime"
 require_dependency 'upload_creator'
 
 class UploadsController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_logged_in, except: [:show]
+  requires_login except: [:show]
+
   skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show]
 
   def create
diff --git a/app/controllers/user_api_keys_controller.rb b/app/controllers/user_api_keys_controller.rb
index 2dbc75ea238..498a055e5d0 100644
--- a/app/controllers/user_api_keys_controller.rb
+++ b/app/controllers/user_api_keys_controller.rb
@@ -2,7 +2,7 @@ class UserApiKeysController < ApplicationController
 
   layout 'no_ember'
 
-  prepend_before_action :check_xhr, :ensure_logged_in, only: [:create, :revoke, :undo_revoke]
+  requires_login only: [:create, :revoke, :undo_revoke]
   skip_before_action :redirect_to_login_if_required, only: [:new]
   skip_before_action :check_xhr, :preload_json
 
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 29e52290c44..72e706e86c8 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -9,7 +9,7 @@ class UsersController < ApplicationController
 
   skip_before_action :authorize_mini_profiler, only: [:avatar]
 
-  prepend_before_action :check_xhr, :ensure_logged_in, only: [
+  requires_login only: [
     :username, :update, :user_preferences_redirect, :upload_user_image,
     :pick_avatar, :destroy_user_image, :destroy, :check_emails, :topic_tracking_state,
     :preferences
diff --git a/app/controllers/users_email_controller.rb b/app/controllers/users_email_controller.rb
index e34e905232c..e408a84f2a8 100644
--- a/app/controllers/users_email_controller.rb
+++ b/app/controllers/users_email_controller.rb
@@ -4,7 +4,7 @@ require_dependency 'email_updater'
 
 class UsersEmailController < ApplicationController
 
-  prepend_before_action :check_xhr, :ensure_logged_in, only: [:index, :update]
+  requires_login only: [:index, :update]
 
   skip_before_action :check_xhr, only: [:confirm]
   skip_before_action :redirect_to_login_if_required, only: [:confirm]
diff --git a/app/controllers/wizard_controller.rb b/app/controllers/wizard_controller.rb
index 78f224b17f2..1bfa774f148 100644
--- a/app/controllers/wizard_controller.rb
+++ b/app/controllers/wizard_controller.rb
@@ -2,8 +2,9 @@ require_dependency 'wizard'
 require_dependency 'wizard/builder'
 
 class WizardController < ApplicationController
-  prepend_before_action :check_xhr, :ensure_admin, except: [:qunit]
-  prepend_before_action :check_xhr, :ensure_logged_in, except: [:qunit]
+  requires_login except: [:qunit]
+
+  before_action :ensure_admin, except: [:qunit]
   before_action :ensure_wizard_enabled, only: [:index]
   skip_before_action :check_xhr, :preload_json