From 406d05a47aac137ca43be1a3ca92a08e305bf0f6 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 22 Jul 2024 11:03:25 +0200 Subject: [PATCH 1/4] chore(spec): remove duplicated specs --- .../api/v2/graphql_controller_spec.rb | 39 ------------------- 1 file changed, 39 deletions(-) diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb index 0d6f109fa..26750062e 100644 --- a/spec/controllers/api/v2/graphql_controller_spec.rb +++ b/spec/controllers/api/v2/graphql_controller_spec.rb @@ -1464,43 +1464,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 From 38243434d2a77f9491899c279544290250313a5b Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 22 Jul 2024 11:04:51 +0200 Subject: [PATCH 2/4] refactor(graphql): add graphql_error helper --- app/controllers/api/v2/base_controller.rb | 16 ++++++- app/controllers/api/v2/graphql_controller.rb | 48 ++------------------ 2 files changed, 18 insertions(+), 46 deletions(-) diff --git a/app/controllers/api/v2/base_controller.rb b/app/controllers/api/v2/base_controller.rb index f247e2a12..70f850e89 100644 --- a/app/controllers/api/v2/base_controller.rb +++ b/app/controllers/api/v2/base_controller.rb @@ -58,13 +58,25 @@ class API::V2::BaseController < ApplicationController 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 diff --git a/app/controllers/api/v2/graphql_controller.rb b/app/controllers/api/v2/graphql_controller.rb index c258613cf..678f4c62d 100644 --- a/app/controllers/api/v2/graphql_controller.rb +++ b/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 From c31321d69545e5cbcb51c031a13576d6c7b5bf2a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 22 Jul 2024 11:05:58 +0200 Subject: [PATCH 3/4] chore(graphql): API::V2::DossiersController is not using tokens --- app/controllers/api/v2/dossiers_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/api/v2/dossiers_controller.rb b/app/controllers/api/v2/dossiers_controller.rb index 3c627bd36..a07ef34eb 100644 --- a/app/controllers/api/v2/dossiers_controller.rb +++ b/app/controllers/api/v2/dossiers_controller.rb @@ -1,5 +1,6 @@ class API::V2::DossiersController < API::V2::BaseController before_action :ensure_dossier_present + skip_before_action :authenticate_from_token def pdf @acls = PiecesJustificativesService.new(user_profile: Administrateur.new, export_template: nil).acl_for_dossier_export(dossier.procedure) From d6f9e57e7778e409c2dbcff72abec97b01bb79af Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 22 Jul 2024 11:11:06 +0200 Subject: [PATCH 4/4] secu(graphql): without a token, only persisted queries are allowed --- app/controllers/api/v2/base_controller.rb | 7 +++++++ app/controllers/api/v2/dossiers_controller.rb | 1 + .../api/v2/graphql_controller_spec.rb | 4 ++-- .../graphql_controller_stored_queries_spec.rb | 18 ++++++++++++++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v2/base_controller.rb b/app/controllers/api/v2/base_controller.rb index 70f850e89..d093b0c91 100644 --- a/app/controllers/api/v2/base_controller.rb +++ b/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,6 +56,12 @@ 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) diff --git a/app/controllers/api/v2/dossiers_controller.rb b/app/controllers/api/v2/dossiers_controller.rb index a07ef34eb..a13d0d473 100644 --- a/app/controllers/api/v2/dossiers_controller.rb +++ b/app/controllers/api/v2/dossiers_controller.rb @@ -1,6 +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) diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb index 26750062e..0c4a65312 100644 --- a/spec/controllers/api/v2/graphql_controller_spec.rb +++ b/spec/controllers/api/v2/graphql_controller_spec.rb @@ -131,7 +131,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 @@ -158,7 +158,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 diff --git a/spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb b/spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb index dfd5ed005..da2189872 100644 --- a/spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb +++ b/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' }