Browse Source

Feature: Mobile - Make GraphQL CSRF checks configurable.

Martin Gruner 3 years ago
parent
commit
530855dfc3

+ 9 - 4
app/graphql/gql/concern/handles_authorization.rb

@@ -9,14 +9,19 @@ module Gql::Concern::HandlesAuthorization
     # Customizable methods
     #
 
+    # Override this method to specify if an object needs CSRF verification.
+    def self.requires_csrf_verification?
+      false # No check required by default, only by mutations.
+    end
+
     # Override this method to specify if an object needs an authenticated user.
     def self.requires_authentication?
-      true
+      true # Authentication required by default for everything.
     end
 
-    # Override this method if an object requires authorization, e.g. based on Pundit.
+    # Override this method if an object requires custom authorization, e.g. based on Pundit.
     def self.authorize(...)
-      true
+      true # Authorization is granted by default.
     end
 
     #
@@ -28,7 +33,7 @@ module Gql::Concern::HandlesAuthorization
       ctx = args[-1] # This may be called with 2 or 3 params, context is last.
 
       # CSRF
-      verify_csrf_token(ctx)
+      verify_csrf_token(ctx) if requires_csrf_verification?
 
       # Authenticate
       if requires_authentication? && !ctx[:current_user]

+ 1 - 2
app/graphql/gql/entry_points/mutations.rb

@@ -4,8 +4,7 @@ module Gql::EntryPoints
   class Mutations < Gql::Types::BaseObject
     description 'All available mutations.'
 
-    # No auth required for the main mutation entry point, Gql::mutations
-    #   perform their own auth handling.
+    # No auth required for the main mutation entry point, Gql::mutations perform their own auth handling.
     def self.requires_authentication?
       false
     end

+ 5 - 0
app/graphql/gql/mutations/base_mutation.rb

@@ -8,5 +8,10 @@ module Gql::Mutations
     field_class Gql::Types::BaseField
     # input_object_class Gql::Types::BaseInputObject
     object_class Gql::Types::BaseObject
+
+    def self.requires_csrf_verification?
+      true
+    end
+
   end
 end

+ 4 - 0
app/graphql/gql/mutations/logout.rb

@@ -6,6 +6,10 @@ module Gql::Mutations
 
     field :success, Boolean, null: false, description: 'Was the logout successful?'
 
+    def self.requires_csrf_verification?
+      false
+    end
+
     def resolve(...)
 
       context[:controller].reset_session

+ 10 - 0
spec/requests/graphql/gql/mutations/login_spec.rb

@@ -30,6 +30,16 @@ RSpec.describe Gql::Mutations::Login, type: :request do
       end
     end
 
+    context 'without CSRF token', allow_forgery_protection: true do
+      it 'fails with error message' do
+        expect(graphql_response['errors'][0]['message']).to eq('CSRF token verification failed!')
+      end
+
+      it 'fails with error type' do
+        expect(graphql_response['errors'][0]['extensions']).to include({ 'type' => 'Exceptions::NotAuthorized' })
+      end
+    end
+
     context 'with wrong password' do
       let(:password) { 'wrong' }
 

+ 10 - 0
spec/requests/graphql/gql/mutations/logout_spec.rb

@@ -27,5 +27,15 @@ RSpec.describe Gql::Mutations::Logout, type: :request do
         expect(graphql_response['errors'][0]['extensions']).to include({ 'type' => 'Exceptions::NotAuthorized' })
       end
     end
+
+    context 'without authenticated session and missing CSRF token', allow_forgery_protection: true do
+      it 'fails with error message, not with CSRF validation failed' do
+        expect(graphql_response['errors'][0]['message']).to eq('Authentication required by Gql::Mutations::Logout')
+      end
+
+      it 'fails with error type, not with CSRF validation failed' do
+        expect(graphql_response['errors'][0]['extensions']).to include({ 'type' => 'Exceptions::NotAuthorized' })
+      end
+    end
   end
 end

+ 1 - 17
spec/requests/graphql/gql/queries/session_id_spec.rb

@@ -12,28 +12,12 @@ RSpec.describe Gql::Queries::SessionId, type: :request do
       json_response
     end
 
-    context 'with authenticated session, CRSF checking disabled', authenticated_as: :agent do
+    context 'with authenticated session', authenticated_as: :agent do
       it 'has data' do
         expect(graphql_response['data']['sessionId']).to be_present
       end
     end
 
-    context 'with authenticated session, CRSF checking enabled but inactive due to basic_auth usage', authenticated_as: :agent, allow_forgery_protection: true do
-      it 'has data' do
-        expect(graphql_response['data']['sessionId']).to be_present
-      end
-    end
-
-    context 'with authenticated session, but missing CSRF token', allow_forgery_protection: true do
-      it 'fails with error message' do
-        expect(graphql_response['errors'][0]['message']).to eq('CSRF token verification failed!')
-      end
-
-      it 'fails with error type' do
-        expect(graphql_response['errors'][0]['extensions']).to include({ 'type' => 'Exceptions::NotAuthorized' })
-      end
-    end
-
     context 'without authenticated session', authenticated_as: false do
       it 'fails with error message' do
         expect(graphql_response['errors'][0]['message']).to eq('Authentication required by Gql::Queries::SessionId')