api: disable csrf protection for api requests.

Fixes POST/PUT API requests failing with InvalidAuthenticityToken errors
due to missing CSRF tokens.

CSRF protection is only necessary for cookie-based authentication. For
non-cookie-based authentication we can safely disable it. That is, if
the user is already passing their login + api_key, then we don't need
to additionally verify the request with a CSRF token.

ref: 2e407fa476 (comments)

app/controllers/application_controller.rb

@@ -1,4 +1,5 @@
 class ApplicationController < ActionController::Base
+  skip_forgery_protection if: -> { SessionLoader.new(request).has_api_authentication? }
   helper :pagination
   before_action :reset_current_user
   before_action :set_current_user
@@ -85,6 +86,8 @@ class ApplicationController < ActionController::Base
       render_error_page(500, exception, message: "The database timed out running your query.")
     when ActionController::BadRequest
       render_error_page(400, exception)
+    when ActionController::InvalidAuthenticityToken
+      render_error_page(403, exception)
     when ActiveRecord::RecordNotFound
       render_error_page(404, exception, message: "That record was not found.")
     when ActionController::RoutingError
@@ -111,8 +114,11 @@ class ApplicationController < ActionController::Base
     @backtrace = Rails.backtrace_cleaner.clean(@exception.backtrace)
     format = :html unless format.in?(%i[html json xml js atom])
 
+    # if InvalidAuthenticityToken was raised, CurrentUser isn't set so we have to use the blank layout.
+    layout = CurrentUser.user.present? ? "default" : "blank"
+
     DanbooruLogger.log(@exception, expected: @expected)
-    render "static/error", status: status, formats: format
+    render "static/error", layout: layout, status: status, formats: format
   end
 
   def authentication_failed
@@ -161,8 +167,7 @@ class ApplicationController < ActionController::Base
   end
 
   def set_current_user
-    session_loader = SessionLoader.new(session, cookies, request, params)
-    session_loader.load
+    SessionLoader.new(request).load
   end
 
   def reset_current_user

app/logical/session_loader.rb

@@ -3,18 +3,20 @@ class SessionLoader
 
   attr_reader :session, :cookies, :request, :params
 
-  def initialize(session, cookies, request, params)
-    @session = session
-    @cookies = cookies
+  def initialize(request)
     @request = request
-    @params = params
+    @session = request.session
+    @cookies = request.cookie_jar
+    @params = request.parameters
   end
 
   def load
     CurrentUser.user = User.anonymous
     CurrentUser.ip_addr = request.remote_ip
 
-    if session[:user_id]
+    if has_api_authentication?
+      load_session_for_api
+    elsif session[:user_id]
       load_session_user
     else
       load_session_for_api
@@ -28,6 +30,10 @@ class SessionLoader
     DanbooruLogger.initialize(request, session, CurrentUser.user)
   end
 
+  def has_api_authentication?
+    request.authorization.present? || params[:login].present? || params[:api_key].present? || params[:password_hash].present?
+  end
+
 private
 
   def set_statement_timeout
@@ -40,15 +46,17 @@ private
       authenticate_basic_auth
     elsif params[:login].present? && params[:api_key].present?
       authenticate_api_key(params[:login], params[:api_key])
+    else
+      raise AuthenticationFailure
     end
   end
-  
+
   def authenticate_basic_auth
     credentials = ::Base64.decode64(request.authorization.split(' ', 2).last || '')
     login, api_key = credentials.split(/:/, 2)
     authenticate_api_key(login, api_key)
   end
-  
+
   def authenticate_api_key(name, api_key)
     CurrentUser.user = User.authenticate_api_key(name, api_key)
 

test/functional/application_controller_test.rb

@@ -40,6 +40,12 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
       setup do
         @user = create(:user, password: "password")
         @api_key = ApiKey.generate!(@user)
+
+        ActionController::Base.allow_forgery_protection = true
+      end
+
+      teardown do
+        ActionController::Base.allow_forgery_protection = false
       end
 
       context "using http basic auth" do
@@ -54,6 +60,14 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
           get edit_user_path(@user), headers: { HTTP_AUTHORIZATION: basic_auth_string }
           assert_response 401
         end
+
+        should "succeed for non-GET requests without a CSRF token" do
+          assert_changes -> { @user.reload.enable_safe_mode }, from: false, to: true do
+            basic_auth_string = "Basic #{::Base64.encode64("#{@user.name}:#{@api_key.key}")}"
+            put user_path(@user), headers: { HTTP_AUTHORIZATION: basic_auth_string }, params: { user: { enable_safe_mode: "true" } }, as: :json
+            assert_response :success
+          end
+        end
       end
 
       context "using the api_key parameter" do
@@ -72,6 +86,13 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
           get edit_user_path(@user), params: { login: @user.name, api_key: "bad" }
           assert_response 401
         end
+
+        should "succeed for non-GET requests without a CSRF token" do
+          assert_changes -> { @user.reload.enable_safe_mode }, from: false, to: true do
+            put user_path(@user), params: { login: @user.name, api_key: @api_key.key, user: { enable_safe_mode: "true" } }, as: :json
+            assert_response :success
+          end
+        end
       end
 
       context "using the password_hash parameter" do
@@ -90,6 +111,13 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
           get edit_user_path(@user), params: { login: @user.name, password_hash: "bad" }
           assert_response 401
         end
+
+        should "succeed for non-GET requests without a CSRF token" do
+          assert_changes -> { @user.reload.enable_safe_mode }, from: false, to: true do
+            put user_path(@user), params: { login: @user.name, password_hash: User.sha1("password"), user: { enable_safe_mode: "true" } }, as: :json
+            assert_response :success
+          end
+        end
       end
 
       context "without any authentication" do
@@ -98,6 +126,25 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
           assert_redirected_to new_session_path(url: edit_user_path(@user))
         end
       end
+
+      context "with cookie-based authentication" do
+        should "not allow non-GET requests without a CSRF token" do
+          # get the csrf token from the login page so we can login
+          get new_session_path
+          assert_response :success
+          token = css_select("form input[name=authenticity_token]").first["value"]
+
+          # login
+          post session_path, params: { authenticity_token: token, name: @user.name, password: "password" }
+          assert_redirected_to posts_path
+
+          # try to submit a form with cookies but without the csrf token
+          put user_path(@user), headers: { HTTP_COOKIE: headers["Set-Cookie"] }, params: { user: { enable_safe_mode: "true" } }
+          assert_response 403
+          assert_equal("ActionController::InvalidAuthenticityToken", css_select("p").first.content)
+          assert_equal(false, @user.reload.enable_safe_mode)
+        end
+      end
     end
 
     context "on session cookie authentication" do