Merge pull request #10614 from tchak/graphql-only-allow-saved-queries-without-token

secu(graphql): without a token, only saved queries are allowed

app/controllers/api/v2/base_controller.rb

@@ -10,6 +10,7 @@ class API::V2::BaseController < ApplicationController
   before_action :authenticate_from_token
   before_action :ensure_authorized_network, if: -> { @api_token.present? }
   before_action :ensure_token_is_not_expired, if: -> { @api_token.present? }
+  before_action :allow_only_persisted_queries, if: -> { @api_token.blank? }
 
   before_action do
     Current.browser = 'api'
@@ -55,16 +56,34 @@ class API::V2::BaseController < ApplicationController
     end
   end
 
+  def allow_only_persisted_queries
+    if params[:queryId].blank?
+      render json: graphql_error('Without a token, only persisted queries are allowed', :forbidden), status: :forbidden
+    end
+  end
+
   def ensure_authorized_network
     if @api_token.forbidden_network?(request.remote_ip)
       address = IPAddr.new(request.remote_ip)
-      render json: { errors: ["request issued from a forbidden network. Add #{address.to_string}/#{address.prefix} to your allowed adresses in your /profil"] }, status: :forbidden
+      render json: graphql_error("Request issued from a forbidden network. Add #{address.to_string}/#{address.prefix} to your allowed adresses in your /profil", :forbidden), status: :forbidden
     end
   end
 
   def ensure_token_is_not_expired
     if @api_token.expired?
-      render json: { errors: ['token expired'] }, status: :unauthorized
+      render json: graphql_error('Token expired', :unauthorized), status: :unauthorized
     end
   end
+
+  def graphql_error(message, code, exception_id: nil, backtrace: nil)
+    {
+      errors: [
+        {
+          message:,
+          extensions: { code:, exception_id:, backtrace: }.compact
+        }
+      ],
+      data: nil
+    }
+  end
 end

app/controllers/api/v2/dossiers_controller.rb

@@ -1,5 +1,7 @@
 class API::V2::DossiersController < API::V2::BaseController
   before_action :ensure_dossier_present
+  skip_before_action :authenticate_from_token
+  skip_before_action :allow_only_persisted_queries
 
   def pdf
     @acls = PiecesJustificativesService.new(user_profile: Administrateur.new, export_template: nil).acl_for_dossier_export(dossier.procedure)

app/controllers/api/v2/graphql_controller.rb

@@ -27,17 +27,7 @@ class API::V2::GraphqlController < API::V2::BaseController
   def process_action(*args)
     super
   rescue ActionDispatch::Http::Parameters::ParseError => exception
-    render json: {
-      errors: [
-        {
-          message: exception.cause.message,
-          extensions: {
-            code: :bad_request
-          }
-        }
-      ],
-      data: nil
-    }, status: 400
+    render json: graphql_error(exception.cause.message, :bad_request), status: :bad_request
   end
 
   def query
@@ -77,33 +67,14 @@ class API::V2::GraphqlController < API::V2::BaseController
   end
 
   def handle_parse_error(exception, code)
-    render json: {
-      errors: [
-        {
-          message: exception.message,
-          extensions: { code: }
-        }
-      ],
-      data: nil
-    }, status: 400
+    render json: graphql_error(exception.message, code), status: :bad_request
   end
 
   def handle_error_in_development(exception)
     logger.error exception.message
     logger.error exception.backtrace.join("\n")
 
-    render json: {
-      errors: [
-        {
-          message: exception.message,
-          extensions: {
-            code: :internal_server_error,
-            backtrace: exception.backtrace
-          }
-        }
-      ],
-      data: nil
-    }, status: 500
+    render json: graphql_error(exception.message, :internal_server_error, backtrace: exception.backtrace), status: :internal_server_error
   end
 
   def handle_error_in_production(exception)
@@ -113,17 +84,6 @@ class API::V2::GraphqlController < API::V2::BaseController
       Sentry.capture_exception(exception)
     end
 
-    render json: {
-      errors: [
-        {
-          message: "Internal Server Error",
-          extensions: {
-            code: :internal_server_error,
-            exception_id:
-          }
-        }
-      ],
-      data: nil
-    }, status: 500
+    render json: graphql_error("Internal Server Error", :internal_server_error, exception_id:), status: :internal_server_error
   end
 end

spec/controllers/api/v2/graphql_controller_spec.rb

@@ -132,7 +132,7 @@ describe API::V2::GraphqlController do
         end
 
         it {
-          expect(gql_errors.first[:message]).to eq("An object of type Demarche was hidden due to permissions")
+          expect(gql_errors.first[:message]).to eq("Without a token, only persisted queries are allowed")
         }
       end
 
@@ -159,7 +159,7 @@ describe API::V2::GraphqlController do
 
         it {
           expect(token).not_to be_nil
-          expect(gql_errors.first[:message]).to eq("An object of type Demarche was hidden due to permissions")
+          expect(gql_errors.first[:message]).to eq("Without a token, only persisted queries are allowed")
         }
       end
 
@@ -1525,43 +1525,4 @@ describe API::V2::GraphqlController do
       end
     end
   end
-
-  context "when not authenticated" do
-    it "should return error" do
-      expect(gql_data).to eq(nil)
-      expect(gql_errors).not_to eq(nil)
-    end
-
-    describe "dossier" do
-      let(:query) { "{ dossier(number: #{dossier.id}) { id number usager { email } } }" }
-
-      it "should return error" do
-        expect(gql_data).to eq(nil)
-        expect(gql_errors).not_to eq(nil)
-      end
-    end
-
-    describe "mutation" do
-      let(:query) do
-        "mutation {
-          dossierEnvoyerMessage(input: {
-            dossierId: \"#{dossier.to_typed_id}\",
-            instructeurId: \"#{instructeur.to_typed_id}\",
-            body: \"Bonjour\"
-          }) {
-            message {
-              body
-            }
-            errors {
-              message
-            }
-          }
-        }"
-      end
-
-      it "should return error" do
-        expect(gql_data[:dossierEnvoyerMessage][:errors].first[:message]).to eq("Le jeton utilisé est configuré seulement en lecture")
-      end
-    end
-  end
 end

spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb

@@ -47,6 +47,24 @@ describe API::V2::GraphqlController do
     }
   end
 
+  describe 'when not authenticated' do
+    let(:variables) { { dossierNumber: dossier.id } }
+    let(:operation_name) { 'getDossier' }
+    let!(:authorization_header) { nil }
+
+    context 'with query' do
+      let(:query) { 'query getDossier($dossierNumber: Int!) { dossier(number: $dossierNumber) { id } }' }
+
+      it { expect(gql_errors.first[:message]).to eq('Without a token, only persisted queries are allowed') }
+    end
+
+    context 'with queryId' do
+      let(:query_id) { 'ds-query-v2' }
+
+      it { expect(gql_errors.first[:message]).to eq('An object of type Dossier was hidden due to permissions') }
+    end
+  end
+
   describe 'ds-query-v2' do
     let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure:, depose_at: 4.days.ago) }
     let(:query_id) { 'ds-query-v2' }