Browse Source

Maintenance: Improved explicitness of CSRF token check logic.

Thorsten Eckel 5 years ago
parent
commit
183a893883

+ 8 - 1
app/controllers/application_controller/prevents_csrf.rb

@@ -2,9 +2,13 @@ module ApplicationController::PreventsCsrf
   extend ActiveSupport::Concern
 
   included do
-    # disable Rails default (>= 5.2) CSRF checks
+    # disable Rails default (>= 5.2) CSRF verification because we
+    # have an advanced use case with our JS App/SPA and the different
+    # Auth mechanisms (e.g. Token- or BasicAuth) that can't be covered
+    # with the built-in functionality
     skip_before_action :verify_authenticity_token, raise: false
 
+    # register custom CSRF verification and provisioning functionality
     before_action :verify_csrf_token
     after_action  :set_csrf_token_headers
   end
@@ -14,6 +18,7 @@ module ApplicationController::PreventsCsrf
   def set_csrf_token_headers
     return true if @_auth_type.present? && @_auth_type != 'session'
 
+    # call Rails method to provide CRSF token
     headers['CSRF-TOKEN'] = form_authenticity_token
   end
 
@@ -22,6 +27,8 @@ module ApplicationController::PreventsCsrf
     return true if request.get?
     return true if request.head?
     return true if %w[token_auth basic_auth].include?(@_auth_type)
+
+    # call Rails method to verify CRSF token
     return true if valid_authenticity_token?(session, params[:authenticity_token] || request.headers['X-CSRF-Token'])
 
     logger.info 'CSRF token verification failed'

+ 7 - 0
spec/requests/api_auth_spec.rb

@@ -393,5 +393,12 @@ RSpec.describe 'Api Auth', type: :request do
       expect(json_response).to be_a_kind_of(Hash)
       expect(json_response).to be_truthy
     end
+
+    it 'does session auth - admin - only with valid CSRF token' do
+      create(:admin_user, login: 'api-admin@example.com', password: 'adminpw')
+
+      post '/api/v1/signin', params: { username: 'api-admin@example.com', password: 'adminpw', fingerprint: '123456789' }
+      expect(response).to have_http_status(:unauthorized)
+    end
   end
 end