From 79842f7a3b262d96b6657481c3ccafe395d646c4 Mon Sep 17 00:00:00 2001
From: Albert Yi <ayi@desk.com>
Date: Tue, 25 Oct 2016 12:54:25 -0700
Subject: [PATCH] restrict min level constraints for forum topics to mod+admin
 and restrict options based on current user's level. check privileges for
 visiblity in forum posts and topics. deprecate serializable_hash
 (undocumented, internal) for as_json, refactor to use hidden_attributes and
 method_attributes #2658

---
 app/controllers/forum_posts_controller.rb     | 35 ++++++++++++++---
 app/controllers/forum_topics_controller.rb    |  4 ++
 app/models/dmail.rb                           | 18 +--------
 app/models/forum_post.rb                      | 27 +++++++++++--
 app/models/forum_topic.rb                     | 38 ++++++++++++++++++-
 app/models/note.rb                            | 19 +---------
 app/models/pool.rb                            | 24 +-----------
 app/models/post.rb                            | 18 +--------
 app/models/post_appeal.rb                     | 19 +---------
 app/models/post_flag.rb                       |  7 ----
 app/models/post_version.rb                    | 19 +---------
 app/models/upload.rb                          | 19 +---------
 app/models/user.rb                            | 23 +----------
 app/models/wiki_page.rb                       | 19 +---------
 app/views/forum_topics/_form.html.erb         |  9 ++---
 app/views/forum_topics/show.html.erb          |  2 +-
 .../active_record_api_extensions.rb           | 16 +++++++-
 17 files changed, 127 insertions(+), 189 deletions(-)

diff --git a/app/controllers/forum_posts_controller.rb b/app/controllers/forum_posts_controller.rb
index ef2ca91fb..88713fbbb 100644
--- a/app/controllers/forum_posts_controller.rb
+++ b/app/controllers/forum_posts_controller.rb
@@ -1,6 +1,8 @@
 class ForumPostsController < ApplicationController
   respond_to :html, :xml, :json, :js
   before_filter :member_only, :except => [:index, :show]
+  before_filter :load_post, :only => [:edit, :show, :update, :destroy, :undelete]
+  before_filter :check_min_level, :only => [:edit, :show, :update, :destroy, :undelete]
 
   def new
     @forum_topic = ForumTopic.find(params[:topic_id]) if params[:topic_id]
@@ -9,14 +11,13 @@ class ForumPostsController < ApplicationController
   end
 
   def edit
-    @forum_post = ForumPost.find(params[:id])
     check_privilege(@forum_post)
     respond_with(@forum_post)
   end
 
   def index
     @query = ForumPost.search(params[:search])
-    @forum_posts = @query.order("forum_posts.id DESC").paginate(params[:page], :limit => params[:limit], :search_count => params[:search])
+    @forum_posts = @query.includes(:topic).order("forum_posts.id DESC").paginate(params[:page], :limit => params[:limit], :search_count => params[:search])
     respond_with(@forum_posts) do |format|
       format.xml do
         render :xml => @forum_posts.to_xml(:root => "forum-posts")
@@ -28,7 +29,6 @@ class ForumPostsController < ApplicationController
   end
 
   def show
-    @forum_post = ForumPost.find(params[:id])
     if request.format == "text/html" && @forum_post.id == @forum_post.topic.original_post.id
       redirect_to(forum_topic_path(@forum_post.topic, :page => params[:page]))
     else
@@ -43,7 +43,6 @@ class ForumPostsController < ApplicationController
   end
 
   def update
-    @forum_post = ForumPost.find(params[:id])
     check_privilege(@forum_post)
     @forum_post.update_attributes(params[:forum_post])
     page = @forum_post.forum_topic_page if @forum_post.forum_topic_page > 1
@@ -51,20 +50,44 @@ class ForumPostsController < ApplicationController
   end
 
   def destroy
-    @forum_post = ForumPost.find(params[:id])
     check_privilege(@forum_post)
     @forum_post.delete!
     respond_with(@forum_post)
   end
 
   def undelete
-    @forum_post = ForumPost.find(params[:id])
     check_privilege(@forum_post)
     @forum_post.undelete!
     respond_with(@forum_post)
   end
 
 private
+  def load_post
+    @forum_post = ForumPost.find(params[:id])
+    @forum_topic = @forum_post.topic
+  end
+
+  def check_min_level
+    if CurrentUser.user.level < @forum_topic.min_level
+      respond_with(@forum_topic) do |fmt|
+        fmt.html do
+          flash[:notice] = "Access denied"
+          redirect_to forum_topics_path
+        end
+
+        fmt.json do
+          render :nothing => true, :status => 403
+        end
+
+        fmt.xml do
+          render :nothing => true, :status => 403
+        end
+      end
+
+      return false
+    end
+  end
+
   def check_privilege(forum_post)
     if !forum_post.editable_by?(CurrentUser.user)
       raise User::PrivilegeError
diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb
index 2b25311a7..b816f4d26 100644
--- a/app/controllers/forum_topics_controller.rb
+++ b/app/controllers/forum_topics_controller.rb
@@ -20,7 +20,11 @@ class ForumTopicsController < ApplicationController
   def index
     @query = ForumTopic.active.search(params[:search])
     @forum_topics = @query.includes([:creator, :updater]).order("is_sticky DESC, updated_at DESC").paginate(params[:page], :limit => per_page, :search_count => params[:search])
+
     respond_with(@forum_topics) do |format|
+      format.json do
+        render :json => @forum_topics.to_json
+      end
       format.xml do
         render :xml => @forum_topics.to_xml(:root => "forum-topics")
       end
diff --git a/app/models/dmail.rb b/app/models/dmail.rb
index 4cb1f2534..3c00fa0b4 100644
--- a/app/models/dmail.rb
+++ b/app/models/dmail.rb
@@ -86,23 +86,7 @@ class Dmail < ActiveRecord::Base
     end
     
     def method_attributes
-      list = [:hash]
-      list
-    end
-    
-    def serializable_hash(options = {})
-      options ||= {}
-      options[:methods] ||= []
-      options[:methods] += method_attributes
-      super(options)
-    end
-    
-    def to_xml(options = {}, &block)
-      # to_xml ignores the serializable_hash method
-      options ||= {}
-      options[:methods] ||= []
-      options[:methods] += method_attributes
-      super(options, &block)
+      super + [:hash]
     end
   end
   
diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb
index 5dd003a94..d53dd041a 100644
--- a/app/models/forum_post.rb
+++ b/app/models/forum_post.rb
@@ -86,7 +86,30 @@ class ForumPost < ActiveRecord::Base
     end
   end
 
+  module ApiMethods
+    def as_json(options = {})
+      if CurrentUser.user.level < topic.min_level
+        options[:only] = [:id]
+      end
+
+      super(options)
+    end
+
+    def to_xml(options = {})
+      if CurrentUser.user.level < topic.min_level
+        options[:only] = [:id]
+      end
+
+      super(options)
+    end
+    
+    def hidden_attributes
+      [:text_index]
+    end
+  end
+
   extend SearchMethods
+  include ApiMethods
 
   def self.new_reply(params)
     if params[:topic_id]
@@ -215,8 +238,4 @@ class ForumPost < ActiveRecord::Base
       x.body = x.quoted_response
     end
   end
-
-  def hidden_attributes
-    super + [:text_index]
-  end
 end
diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb
index 196099bb8..2348535ad 100644
--- a/app/models/forum_topic.rb
+++ b/app/models/forum_topic.rb
@@ -18,6 +18,7 @@ class ForumTopic < ActiveRecord::Base
   validates_presence_of :title, :creator_id
   validates_associated :original_post
   validates_inclusion_of :category_id, :in => CATEGORIES.keys
+  validates_inclusion_of :min_level, :in => [0, User::Levels::MODERATOR, User::Levels::ADMIN]
   accepts_nested_attributes_for :original_post
   after_update :update_orignal_post
 
@@ -114,10 +115,27 @@ class ForumTopic < ActiveRecord::Base
     end
   end
 
+  module UserLevelMethods
+    extend ActiveSupport::Concern
+
+    module ClassMethods
+      def available_min_user_levels
+        if CurrentUser.is_admin?
+          [["Moderator", User::Levels::MODERATOR], ["Admin", User::Levels::ADMIN]]
+        elsif CurrentUser.is_moderator?
+          [["Moderator", User::Levels::MODERATOR]]
+        else
+          []
+        end
+      end
+    end
+  end
+
   extend SearchMethods
   include CategoryMethods
   include VisitMethods
   include SubscriptionMethods
+  include UserLevelMethods
 
   def editable_by?(user)
     creator_id == user.id || user.is_moderator?
@@ -142,9 +160,25 @@ class ForumTopic < ActiveRecord::Base
   def presenter(forum_posts)
     @presenter ||= ForumTopicPresenter.new(self, forum_posts)
   end
-  
+
+  def as_json(options = {})
+    if CurrentUser.user.level < min_level
+      options[:only] = [:id]
+    end
+
+    super(options)
+  end
+
+  def to_xml(options = {})
+    if CurrentUser.user.level < min_level
+      options[:only] = [:id]
+    end
+
+    super(options)
+  end
+
   def hidden_attributes
-    super + [:text_index]
+    [:text_index, :min_level]
   end
 
   def merge(topic)
diff --git a/app/models/note.rb b/app/models/note.rb
index abcf77676..48281e1e6 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -81,23 +81,8 @@ class Note < ActiveRecord::Base
       super + [:body_index]
     end
 
-    def serializable_hash(options = {})
-      options ||= {}
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-      unless options[:builder]
-        options[:methods] ||= []
-        options[:methods] += [:creator_name]
-      end
-      hash = super(options)
-      hash
-    end
-
-    def to_xml(options = {}, &block)
-      options ||= {}
-      options[:methods] ||= []
-      options[:methods] += [:creator_name]
-      super(options, &block)
+    def method_attributes
+      super + [:creator_name]
     end
   end
 
diff --git a/app/models/pool.rb b/app/models/pool.rb
index 46a811423..6c29c9d23 100644
--- a/app/models/pool.rb
+++ b/app/models/pool.rb
@@ -360,34 +360,14 @@ class Pool < ActiveRecord::Base
     clear_post_id_array
   end
 
-  def to_xml(options = {}, &block)
-    # to_xml ignores the serializable_hash method
-    options ||= {}
-    options[:methods] = [:creator_name]
-    super(options, &block)
+  def method_attributes
+    super + [:creator_name]
   end
 
   def strip_name
     self.name = name.to_s.strip
   end
 
-  def serializable_hash(options = {})
-    return {
-      "category" => category,
-      "created_at" => created_at,
-      "creator_id" => creator_id,
-      "creator_name" => creator_name,
-      "description" => description,
-      "id" => id,
-      "is_active" => is_active?,
-      "is_deleted" => is_deleted?,
-      "name" => name,
-      "post_count" => post_count,
-      "post_ids" => post_ids,
-      "updated_at" => updated_at
-    }
-  end
-
   def update_category_pseudo_tags_for_posts_async
     if category_changed?
       delay(:queue => "default").update_category_pseudo_tags_for_posts
diff --git a/app/models/post.rb b/app/models/post.rb
index 2adf8352c..64f02929a 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -1472,25 +1472,11 @@ class Post < ActiveRecord::Base
       [ :pixiv_ugoira_frame_data ]
     end
 
-    def serializable_hash(options = {})
+    def as_json(options = {})
       options ||= {}
       options[:include] ||= []
       options[:include] += associated_attributes
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-      unless options[:builder]
-        options[:methods] ||= []
-        options[:methods] += method_attributes
-      end
-      hash = super(options)
-      hash
-    end
-
-    def to_xml(options = {}, &block)
-      options ||= {}
-      options[:methods] ||= []
-      options[:methods] += method_attributes
-      super(options, &block)
+      super(options)
     end
 
     def to_legacy_json
diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb
index 11d85515b..7e933ccf1 100644
--- a/app/models/post_appeal.rb
+++ b/app/models/post_appeal.rb
@@ -103,22 +103,7 @@ class PostAppeal < ActiveRecord::Base
     PostAppeal.for_user(creator_id).recent.count
   end
 
-  def serializable_hash(options = {})
-    options ||= {}
-    options[:except] ||= []
-    options[:except] += hidden_attributes
-    unless options[:builder]
-      options[:methods] ||= []
-      options[:methods] += [:is_resolved]
-    end
-    hash = super(options)
-    hash
-  end
-
-  def to_xml(options = {}, &block)
-    options ||= {}
-    options[:methods] ||= []
-    options[:methods] += [:is_resolved]
-    super(options, &block)
+  def method_attributes
+    super + [:is_resolved]
   end
 end
diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb
index 9ba8378e7..1467be852 100644
--- a/app/models/post_flag.rb
+++ b/app/models/post_flag.rb
@@ -85,13 +85,6 @@ class PostFlag < ActiveRecord::Base
       end
       super + list
     end
-
-    def serializable_hash(options = {})
-      options ||= {}
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-      super(options)
-    end
   end
 
   extend SearchMethods
diff --git a/app/models/post_version.rb b/app/models/post_version.rb
index 04e345e58..dc4a9f40b 100644
--- a/app/models/post_version.rb
+++ b/app/models/post_version.rb
@@ -176,22 +176,7 @@ class PostVersion < ActiveRecord::Base
     User.id_to_name(updater_id)
   end
 
-  def serializable_hash(options = {})
-    options ||= {}
-    options[:except] ||= []
-    options[:except] += hidden_attributes
-    unless options[:builder]
-      options[:methods] ||= []
-      options[:methods] += [:added_tags, :removed_tags, :obsolete_added_tags, :obsolete_removed_tags, :unchanged_tags, :updater_name]
-    end
-    hash = super(options)
-    hash
-  end
-
-  def to_xml(options = {}, &block)
-    options ||= {}
-    options[:methods] ||= []
-    options[:methods] += [:added_tags, :removed_tags, :obsolete_added_tags, :obsolete_removed_tags, :unchanged_tags, :updater_name]
-    super(options, &block)
+  def method_attributes
+    super + [:added_tags, :removed_tags, :obsolete_added_tags, :obsolete_removed_tags, :unchanged_tags, :updater_name]
   end
 end
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 5d7e28a2f..22c13ea70 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -497,23 +497,8 @@ class Upload < ActiveRecord::Base
   end
 
   module ApiMethods
-    def serializable_hash(options = {})
-      options ||= {}
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-      unless options[:builder]
-        options[:methods] ||= []
-        options[:methods] += [:uploader_name]
-      end
-      hash = super(options)
-      hash
-    end
-
-    def to_xml(options = {}, &block)
-      options ||= {}
-      options[:methods] ||= []
-      options[:methods] += [:uploader_name]
-      super(options, &block)
+    def method_attributes
+      super + [:uploader_name]
     end
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 0c95c1b6f..f3bddb175 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -656,36 +656,17 @@ class User < ActiveRecord::Base
 
   module ApiMethods
     def hidden_attributes
-      super + [:password_hash, :bcrypt_password_hash, :email, :email_verification_key, :time_zone, :updated_at, :receive_email_notifications, :last_logged_in_at, :last_forum_read_at, :has_mail, :default_image_size, :comment_threshold, :always_resize_images, :favorite_tags, :blacklisted_tags, :recent_tags, :enable_privacy_mode, :enable_post_navigation, :new_post_navigation_layout, :enable_sequential_post_navigation, :hide_deleted_posts, :per_page, :style_usernames, :enable_auto_complete, :custom_style, :show_deleted_children, :has_saved_searches, :last_ip_addr, :bit_prefs]
+      super + [:password_hash, :bcrypt_password_hash, :email, :email_verification_key, :time_zone, :updated_at, :receive_email_notifications, :last_logged_in_at, :last_forum_read_at, :has_mail, :default_image_size, :comment_threshold, :always_resize_images, :favorite_tags, :blacklisted_tags, :recent_tags, :enable_privacy_mode, :enable_post_navigation, :new_post_navigation_layout, :enable_sequential_post_navigation, :hide_deleted_posts, :per_page, :style_usernames, :enable_auto_complete, :custom_style, :show_deleted_children, :has_saved_searches, :last_ip_addr, :bit_prefs, :favorite_count]
     end
 
     def method_attributes
-      list = [:is_banned, :can_approve_posts, :can_upload_free, :is_super_voter, :level_string]
+      list = super + [:is_banned, :can_approve_posts, :can_upload_free, :is_super_voter, :level_string]
       if id == CurrentUser.user.id
         list += [:remaining_api_hourly_limit, :remaining_api_hourly_limit_read, :remaining_api_hourly_limit_write]
       end
       list
     end
 
-    def serializable_hash(options = {})
-      options ||= {}
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-      options[:methods] ||= []
-      options[:methods] += method_attributes
-      super(options)
-    end
-
-    def to_xml(options = {}, &block)
-      # to_xml ignores the serializable_hash method
-      options ||= {}
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-      options[:methods] ||= []
-      options[:methods] += method_attributes
-      super(options, &block)
-    end
-
     def to_legacy_json
       return {
         "name" => name,
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index 1021f8c36..1ef615a30 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -94,23 +94,8 @@ class WikiPage < ActiveRecord::Base
       super + [:body_index, :other_names_index]
     end
 
-    def serializable_hash(options = {})
-      options ||= {}
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-      unless options[:builder]
-        options[:methods] ||= []
-        options[:methods] += [:creator_name, :category_name]
-      end
-      hash = super(options)
-      hash
-    end
-
-    def to_xml(options = {}, &block)
-      options ||= {}
-      options[:methods] ||= []
-      options[:methods] += [:creator_name, :category_name]
-      super(options, &block)
+    def method_attributes
+      super + [:creator_name, :category_name]
     end
   end
 
diff --git a/app/views/forum_topics/_form.html.erb b/app/views/forum_topics/_form.html.erb
index a29dc3583..e4e58a006 100644
--- a/app/views/forum_topics/_form.html.erb
+++ b/app/views/forum_topics/_form.html.erb
@@ -18,12 +18,9 @@
     <% end %>
 
     <% if CurrentUser.is_moderator? %>
-      <%= f.input :is_sticky %>
-      <%= f.input :is_locked %>
-    <% end %>
-
-    <% if CurrentUser.is_builder? %>
-      <%= f.input :min_level, :as => :select, :collection => User.level_hash.to_a %>
+      <%= f.input :min_level, :as => :select, :collection => ForumTopic.available_min_user_levels.to_a %>
+      <%= f.input :is_sticky, :label => "Sticky" %>
+      <%= f.input :is_locked, :label => "Locked" %>
     <% end %>
 
     <%= f.button :submit, "Submit", :data => { :disable_with => "Submitting..." } %>
diff --git a/app/views/forum_topics/show.html.erb b/app/views/forum_topics/show.html.erb
index ecc2b1d98..bffa2ded8 100644
--- a/app/views/forum_topics/show.html.erb
+++ b/app/views/forum_topics/show.html.erb
@@ -4,7 +4,7 @@
       Topic: <%= @forum_topic.title %>
 
       <% if @forum_topic.min_level >= User::Levels::BUILDER %>
-        <span class="level-topic">(<%= User.level_string(@forum_topic.min_level).downcase %> only)</span>
+        <span class="level-topic">(<%= User.level_string(@forum_topic.min_level).downcase %>+ only)</span>
       <% end %>
 
       <% if @forum_topic.is_deleted? %>
diff --git a/config/initializers/active_record_api_extensions.rb b/config/initializers/active_record_api_extensions.rb
index 1516fb181..266a2678a 100644
--- a/config/initializers/active_record_api_extensions.rb
+++ b/config/initializers/active_record_api_extensions.rb
@@ -3,18 +3,26 @@ module Danbooru
     module ActiveRecordApi
       extend ActiveSupport::Concern
 
-      def serializable_hash(options = {})
+      def as_json(options = {})
         options ||= {}
         options[:except] ||= []
         options[:except] += hidden_attributes
+
+        options[:methods] ||= []
+        options[:methods] += method_attributes
+
         super(options)
       end
 
       def to_xml(options = {}, &block)
-        # to_xml ignores serializable_hash
         options ||= {}
+
         options[:except] ||= []
         options[:except] += hidden_attributes
+
+        options[:methods] ||= []
+        options[:methods] += method_attributes
+
         super(options, &block)
       end
 
@@ -22,6 +30,10 @@ module Danbooru
       def hidden_attributes
         [:uploader_ip_addr, :updater_ip_addr, :creator_ip_addr, :ip_addr]
       end
+
+      def method_attributes
+        []
+      end
     end
   end
 end