Fix NoMethodError: undefined method +status+ for nil:NilClass (public…

…lab#5028)

* Fix NoMethodError: undefined method +status+ for nil:NilClass

* Disable codeclimate false postiive

* Fix yml

* Parse with URI

* HTML safe on show page

* Too many of these are false postives
With reference from their own repo and reason why they are false positives:
presidentbeef/brakeman#597

* Get rid of return

.codeclimate.yml

@@ -2,6 +2,9 @@
 engines:
   brakeman:
     enabled: true
+    checks:
+      cross_site_scripting:
+        enabled: false
   bundler-audit:
     enabled: true
   duplication:

app/controllers/application_controller.rb

@@ -130,17 +130,18 @@ def redirect_back_or_default(default)
     session[:return_to] = nil
   end
 
-  def check_and_redirect_node(node)
-    if !node.nil? && node.type[/^redirect\|/]
-      node = Node.find(node.type[/\|\d+/][1..-1])
-      redirect_to node.path, status: 301
-      return true
-    end
-    false
+  def redirect_to_node_path?(node)
+    return false unless node.present? && node.type[/^redirect\|/]
+
+    node = Node.find(node.type[/\|\d+/][1..-1])
+
+    redirect_to URI.parse(node.path).path, status: :moved_permanently
+
+    true
   end
 
   def alert_and_redirect_moderated
-    if @node.author.status == 0 && !(current_user && (current_user.role == 'admin' || current_user.role == 'moderator'))
+    if @node.author.status == User::Status::BANNED && !(current_user && (current_user.role == 'admin' || current_user.role == 'moderator'))
       flash[:error] = I18n.t('application_controller.author_has_been_banned')
       redirect_to '/'
     elsif @node.status == 4 && (current_user && (current_user.role == 'admin' || current_user.role == 'moderator'))
@@ -153,7 +154,7 @@ def alert_and_redirect_moderated
       # if it's spam or a draft
       # no notification; don't let people easily fish for existing draft titles; we should try to 404 it
       redirect_to '/'
-    elsif @node.author.status == 5
+    elsif @node.author.status == User::Status::MODERATED
       flash.now[:warning] = "The user '#{@node.author.username}' has been placed <a href='https://#{request.host}/wiki/moderators'>in moderation</a> and will not be able to respond to comments."
     end
   end
@@ -170,21 +171,9 @@ def set_locale
   end
 
   def comments_node_and_path
-    @node = if @comment.aid == 0
-              # finding node for node comments
-              @comment.node
-            else
-              # finding node for answer comments
-              @comment.answer.node
-    end
+    @node = @comment.aid == 0 ? @comment.node : @comment.answer.node
 
-    @path = if params[:type] && params[:type] == 'question'
-              # questions path
-              @node.path(:question)
-            else
-              # notes path
-              @node.path
-    end
+    @path = params[:type] && params[:type] == 'question' ? @node.path(:question) : @node.path
   end
 
   # used for url redirects for friendly_id
@@ -201,4 +190,8 @@ def redirect_old_urls
   def signed_in?
     !current_user.nil?
   end
+
+  def page_not_found
+    render file: "#{Rails.root}/public/404.html", layout: false, status: :not_found
+  end
 end

app/controllers/notes_controller.rb

@@ -1,6 +1,7 @@
 class NotesController < ApplicationController
   respond_to :html
   before_action :require_user, only: %i(create edit update delete rsvp publish_draft)
+  before_action :set_node, only: %i(show)
 
   def index
     @title = I18n.t('notes_controller.research_notes')
@@ -46,17 +47,10 @@ def raw
   end
 
   def show
-    if params[:author] && params[:date]
-      @node = Node.find_notes(params[:author], params[:date], params[:id])
-      @node ||= Node.where(path: "/report/#{params[:id]}").first
-    else
-      @node = Node.find params[:id]
-    end
-
-    if @node.status == 3 && !params[:token].nil? && @node.slug.split('token:').last == params[:token]
-    else
+    return if redirect_to_node_path?(@node)
 
-      if @node.status == 3 && current_user.nil?
+    if @node
+      if @node.status == 3 && current_user.blank?
         flash[:warning] = "You need to login to view the page"
         redirect_to '/login'
         return
@@ -65,33 +59,25 @@ def show
         redirect_to '/'
         return
       end
-  end
-
-    if @node.has_power_tag('question')
-      redirect_to @node.path(:question)
-      return
-    end
 
-    if @node.has_power_tag('redirect')
-      if current_user.nil? || !current_user.can_moderate?
-        redirect_to URI.parse(Node.find(@node.power_tag('redirect')).path).path
+      if @node.has_power_tag('question')
+        redirect_to @node.path(:question)
         return
-      elsif current_user.can_moderate?
-        flash.now[:warning] = "Only moderators and admins see this page, as it is redirected to #{Node.find(@node.power_tag('redirect')).title}.
-        To remove the redirect, delete the tag beginning with 'redirect:'"
       end
-    end
 
-    return if check_and_redirect_node(@node)
+      redirect_power_tag_redirect
 
-    alert_and_redirect_moderated
+      alert_and_redirect_moderated
 
-    impressionist(@node, 'show', unique: [:ip_address])
-    @title = @node.latest.title
-    @tags = @node.tags
-    @tagnames = @tags.collect(&:name)
+      impressionist(@node, 'show', unique: [:ip_address])
+      @title = @node.latest.title
+      @tags = @node.tags
+      @tagnames = @tags.collect(&:name)
 
-    set_sidebar :tags, @tagnames
+      set_sidebar :tags, @tagnames
+    else
+      page_not_found
+    end
   end
 
   def image
@@ -409,6 +395,24 @@ def publish_draft
 
   private
 
+  def set_node
+    @node = if params[:author] && params[:date] && params[:id]
+              Node.find_notes(params[:author], params[:date], params[:id]) || Node.where(path: "/report/#{params[:id]}").first
+            else
+              Node.find(params[:id])
+            end
+  end
+
+  def redirect_power_tag_redirect
+    if @node.has_power_tag('redirect')
+      if current_user.blank? || !current_user.can_moderate?
+        redirect_to URI.parse(Node.find(@node.power_tag('redirect')).path).path
+      elsif current_user.can_moderate?
+        flash.now[:warning] = "Only moderators and admins see this page, as it is redirected to #{Node.find(@node.power_tag('redirect')).title}. To remove the redirect, delete the tag beginning with 'redirect:'"
+      end
+    end
+  end
+
   def new_note
     Node.new_note(uid: current_user.uid,
                   title: params[:title],

app/controllers/wiki_controller.rb

@@ -59,7 +59,7 @@ def show
     #   return redirect_to @node.path, :status => :moved_permanently
     # end
 
-    return if check_and_redirect_node(@node)
+    return if redirect_to_node_path?(@node)
 
     if !@node.nil? # it's a place page!
       @tags = @node.tags
@@ -238,7 +238,7 @@ def revert
   # also just redirect anything else matching /____ to /wiki/____
   def root
     @node = Node.find_by(path: "/" + params[:id])
-    return if check_and_redirect_node(@node)
+    return if redirect_to_node_path?(@node)
 
     if @node
       @revision = @node.latest

app/views/notes/show.html.erb

@@ -110,7 +110,9 @@
       </p><br />
       <p><% if !current_user %><%= raw t('notes.show.login_to', :url1 => new_user_session_path( return_to: request.path )) %> <% end %><a class="btn-lg btn-primary" href="/rsvp/<%= @node.id %>">RSVP</a>
         <% attendees = @node.power_tags('rsvp').length %>
-        <% if attendees > 1 %> (<%= raw t('notes.show.people_will_attend', :count => attendees) %>)<% end %>
+        <% if attendees > 1 %>
+          <%= t('notes.show.people_will_attend', :count => attendees).html_safe %>
+        <% end %>
       <hr />
     <% end %>
 

test/functional/notes_controller_test.rb

@@ -122,6 +122,8 @@ def teardown
   test 'admins and moderators view redirect-tagged notes with flash warning' do
     note = nodes(:one)
     blog = nodes(:blog)
+    flash_msg = "Only moderators and admins see this page, as it is redirected to #{blog.title}. To remove the redirect, delete the tag beginning with 'redirect:'"
+
     note.add_tag("redirect:#{blog.nid}", users(:jeff))
     assert_equal blog.nid.to_s, note.power_tag('redirect')
     UserSession.find.destroy if UserSession.find
@@ -135,8 +137,7 @@ def teardown
         }
 
     assert_response :success
-    assert_equal "Only moderators and admins see this page, as it is redirected to #{blog.title}.
-        To remove the redirect, delete the tag beginning with 'redirect:'", flash[:warning]
+    assert_equal flash_msg, flash[:warning]
     UserSession.find.destroy
   end
 
@@ -157,6 +158,18 @@ def teardown
     assert_select "a#other-activities[href = '/wiki/spectrometer']", 1
   end
 
+  test 'return 404 when node is not found' do
+    note = nodes(:one)
+
+    get :show, params: {
+      author: note.author.name,
+      date: Time.at(note.created).strftime('%m-%d-%Y'),
+      id: "doesn't_exist"
+    }
+
+    assert_response :not_found
+  end
+
   test "don't show note by spam author" do
     note = nodes(:spam) # spam fixture
 
@@ -946,7 +959,7 @@ def test_get_rss_feed
              id: node.nid,
              token: @token
          }
-     assert_response :success
+     assert_redirected_to '/login'
    end
 
    test 'no notification email if user posts draft' do