Add ApplicationMetalController to relevant controllers (#4937)

* Add ApplicationMetalController to relevant controllers

* Add request forgery protection to metal

* Remove logger call

* Add logger keyword and skip protect_from_forgery in test

* Uncomment main_image in test

app/controllers/application_controller.rb

@@ -2,6 +2,7 @@ class ApplicationController < ActionController::Base
   protect_from_forgery with: :exception, prepend: true
 
   include SessionCurrentUser
+  include ValidRequest
   include Pundit
 
   def require_http_auth
@@ -47,24 +48,6 @@ class ApplicationController < ActionController::Base
   end
   helper_method :internal_navigation?
 
-  def valid_request_origin?
-    # This manually does what it was supposed to do on its own.
-    # We were getting this issue:
-    # HTTP Origin header (https://dev.to) didn't match request.base_url (http://dev.to)
-    # Not sure why, but once we work it out, we can delete this method.
-    # We are at least secure for now.
-    return if Rails.env.test?
-
-    if request.referer.present?
-      request.referer.start_with?(ApplicationConfig["APP_PROTOCOL"].to_s + ApplicationConfig["APP_DOMAIN"].to_s)
-    else
-      logger.info "**REQUEST ORIGIN CHECK** #{request.origin}"
-      raise InvalidAuthenticityToken, NULL_ORIGIN_MESSAGE if request.origin == "null"
-
-      request.origin.nil? || request.origin.gsub("https", "http") == request.base_url.gsub("https", "http")
-    end
-  end
-
   def set_no_cache_header
     response.headers["Cache-Control"] = "no-cache, no-store"
     response.headers["Pragma"] = "no-cache"

app/controllers/application_metal_controller.rb

@@ -1,4 +1,18 @@
 class ApplicationMetalController < ActionController::Metal
   # Any shared behavior across metal-oriented controllers can go here.
+
+  # These are basic things we likely want for any metal controllers
+  include ActionController::Cookies
+  include ActionController::RequestForgeryProtection
+  # ActionController modules which may not be used in each controller can go in
+  # the specific controller.
+
+  protect_from_forgery with: :exception, prepend: true unless Rails.env.test?
+
   include SessionCurrentUser
+  include ValidRequest
+
+  def logger
+    ActionController::Base.logger
+  end
 end

app/controllers/concerns/valid_request.rb

@@ -0,0 +1,20 @@
+module ValidRequest
+  extend ActiveSupport::Concern
+
+  def valid_request_origin?
+    # This manually does what it was supposed to do on its own.
+    # We were getting this issue:
+    # HTTP Origin header (https://dev.to) didn't match request.base_url (http://dev.to)
+    # Not sure why, but once we work it out, we can delete this method.
+    # We are at least secure for now.
+    return if Rails.env.test?
+
+    if request.referer.present?
+      request.referer.start_with?(ApplicationConfig["APP_PROTOCOL"].to_s + ApplicationConfig["APP_DOMAIN"].to_s)
+    else
+      raise InvalidAuthenticityToken, NULL_ORIGIN_MESSAGE if request.origin == "null"
+
+      request.origin.nil? || request.origin.gsub("https", "http") == request.base_url.gsub("https", "http")
+    end
+  end
+end

app/controllers/display_ad_events_controller.rb

@@ -1,5 +1,7 @@
-class DisplayAdEventsController < ApplicationController
+class DisplayAdEventsController < ApplicationMetalController
+  include ActionController::Head
   # No policy needed. All views are for all users
+
   def create
     # Only tracking for logged in users at the moment
     display_ad_event_create_params = display_ad_event_params.merge(user_id: session_current_user_id)
@@ -25,6 +27,6 @@ class DisplayAdEventsController < ApplicationController
   end
 
   def display_ad_event_params
-    params.require(:display_ad_event).permit(%i[context_type category display_ad_id])
+    params[:display_ad_event].slice(:context_type, :category, :display_ad_id)
   end
 end

app/controllers/html_variant_successes_controller.rb

@@ -1,4 +1,6 @@
-class HtmlVariantSuccessesController < ApplicationController
+class HtmlVariantSuccessesController < ApplicationMetalController
+  include ActionController::Head
+
   def create
     HtmlVariantSuccessCreateJob.perform_later(html_variant_id: params[:html_variant_id], article_id: params[:article_id])
     head :ok

app/controllers/html_variant_trials_controller.rb

@@ -1,4 +1,6 @@
-class HtmlVariantTrialsController < ApplicationController
+class HtmlVariantTrialsController < ApplicationMetalController
+  include ActionController::Head
+
   def create
     HtmlVariantTrialCreateJob.perform_later(html_variant_id: params[:html_variant_id], article_id: params[:article_id])
     head :ok

spec/factories/articles.rb

@@ -15,7 +15,7 @@ FactoryBot.define do
       with_collection { nil }
     end
     association :user, factory: :user, strategy: :create
-    description   { Faker::Hipster.paragraph(sentence_count: 1)[0..100] }
+    description { Faker::Hipster.paragraph(sentence_count: 1)[0..100] }
     main_image    { Faker::Avatar.image }
     language { "en" }
     experience_level_rating { rand(4..6) }