From b897e0cc1c5b51a8160f4338cfcabc74d1148a8f Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 12 Nov 2020 20:07:42 +0100 Subject: [PATCH 1/3] [GraphQL] add better errors when attachments are not properly used --- app/graphql/mutations/base_mutation.rb | 12 ++++++++ app/graphql/mutations/dossier_accepter.rb | 4 +++ .../mutations/dossier_classer_sans_suite.rb | 4 +++ .../mutations/dossier_envoyer_message.rb | 4 +++ app/graphql/mutations/dossier_refuser.rb | 4 +++ .../api/v2/graphql_controller_spec.rb | 28 +++++++++++++++++++ 6 files changed, 56 insertions(+) diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index c543a75f6..5113603d0 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -1,4 +1,16 @@ module Mutations class BaseMutation < GraphQL::Schema::RelayClassicMutation + def validate_blob(blob_id) + if blob_id.present? + begin + blob = ActiveStorage::Blob.find_signed(blob_id) + blob.identify + rescue ActiveStorage::FileNotFoundError + return { errors: ['Le fichier n’a pas été correctement téléversé sur le serveur de stockage'] } + rescue ActiveSupport::MessageVerifier::InvalidSignature + return { errors: ['L’identifiant du fichier téléversé est invalide'] } + end + end + end end end diff --git a/app/graphql/mutations/dossier_accepter.rb b/app/graphql/mutations/dossier_accepter.rb index 15ba05326..f90af1756 100644 --- a/app/graphql/mutations/dossier_accepter.rb +++ b/app/graphql/mutations/dossier_accepter.rb @@ -14,6 +14,10 @@ module Mutations def resolve(dossier:, instructeur:, motivation: nil, justificatif: nil) if dossier.en_instruction? + errors = validate_blob(justificatif) + if errors + return errors + end dossier.accepter!(instructeur, motivation, justificatif) { dossier: dossier } diff --git a/app/graphql/mutations/dossier_classer_sans_suite.rb b/app/graphql/mutations/dossier_classer_sans_suite.rb index 5e4307967..e858d8688 100644 --- a/app/graphql/mutations/dossier_classer_sans_suite.rb +++ b/app/graphql/mutations/dossier_classer_sans_suite.rb @@ -14,6 +14,10 @@ module Mutations def resolve(dossier:, instructeur:, motivation:, justificatif: nil) if dossier.en_instruction? + errors = validate_blob(justificatif) + if errors + return errors + end dossier.classer_sans_suite!(instructeur, motivation, justificatif) { dossier: dossier } diff --git a/app/graphql/mutations/dossier_envoyer_message.rb b/app/graphql/mutations/dossier_envoyer_message.rb index 75c7bb4ae..603be72f4 100644 --- a/app/graphql/mutations/dossier_envoyer_message.rb +++ b/app/graphql/mutations/dossier_envoyer_message.rb @@ -11,6 +11,10 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, instructeur:, body:, attachment: nil) + errors = validate_blob(attachment) + if errors + return errors + end message = CommentaireService.build(instructeur, dossier, body: body, piece_jointe: attachment) if message.save diff --git a/app/graphql/mutations/dossier_refuser.rb b/app/graphql/mutations/dossier_refuser.rb index 10ffeec36..028d62d4e 100644 --- a/app/graphql/mutations/dossier_refuser.rb +++ b/app/graphql/mutations/dossier_refuser.rb @@ -14,6 +14,10 @@ module Mutations def resolve(dossier:, instructeur:, motivation:, justificatif: nil) if dossier.en_instruction? + errors = validate_blob(justificatif) + if errors + return errors + end dossier.refuser!(instructeur, motivation, justificatif) { dossier: dossier } diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb index 07adc00cb..1a9c36c48 100644 --- a/spec/controllers/api/v2/graphql_controller_spec.rb +++ b/spec/controllers/api/v2/graphql_controller_spec.rb @@ -604,6 +604,34 @@ describe API::V2::GraphqlController do }) end end + + context 'upload error' do + let(:query) do + "mutation { + dossierEnvoyerMessage(input: { + dossierId: \"#{dossier.to_typed_id}\", + instructeurId: \"#{instructeur.to_typed_id}\", + body: \"Hello world\", + attachment: \"fake\" + }) { + message { + body + } + errors { + message + } + } + }" + end + + it "should fail" do + expect(gql_errors).to eq(nil) + expect(gql_data).to eq(dossierEnvoyerMessage: { + errors: [{ message: "L’identifiant du fichier téléversé est invalide" }], + message: nil + }) + end + end end describe 'dossierPasserEnInstruction' do From d9dea779ea6c173445a07dbbe7c0b01e7b16d44e Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Tue, 17 Nov 2020 13:46:13 +0100 Subject: [PATCH 2/3] Remove foreign key dossier on dossier operations logs --- app/models/dossier.rb | 2 +- ...0201117122923_remove_dossier_operation_log_foreign_key.rb | 5 +++++ db/schema.rb | 3 +-- 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20201117122923_remove_dossier_operation_log_foreign_key.rb diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 7c4ddc06e..beefeb48e 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -74,7 +74,7 @@ class Dossier < ApplicationRecord has_many :avis, inverse_of: :dossier, dependent: :destroy has_many :traitements, -> { order(:processed_at) }, inverse_of: :dossier, dependent: :destroy - has_many :dossier_operation_logs, -> { order(:created_at) }, dependent: :nullify, inverse_of: :dossier + has_many :dossier_operation_logs, -> { order(:created_at) }, inverse_of: :dossier belongs_to :groupe_instructeur, optional: false belongs_to :revision, class_name: 'ProcedureRevision', optional: false diff --git a/db/migrate/20201117122923_remove_dossier_operation_log_foreign_key.rb b/db/migrate/20201117122923_remove_dossier_operation_log_foreign_key.rb new file mode 100644 index 000000000..b472d7a7e --- /dev/null +++ b/db/migrate/20201117122923_remove_dossier_operation_log_foreign_key.rb @@ -0,0 +1,5 @@ +class RemoveDossierOperationLogForeignKey < ActiveRecord::Migration[6.0] + def change + remove_foreign_key :dossier_operation_logs, :dossiers + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d51fe378..398f529b7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_11_10_155516) do +ActiveRecord::Schema.define(version: 2020_11_17_122923) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -709,7 +709,6 @@ ActiveRecord::Schema.define(version: 2020_11_10_155516) do add_foreign_key "closed_mails", "procedures" add_foreign_key "commentaires", "dossiers" add_foreign_key "dossier_operation_logs", "bill_signatures" - add_foreign_key "dossier_operation_logs", "dossiers" add_foreign_key "dossier_operation_logs", "instructeurs" add_foreign_key "dossiers", "groupe_instructeurs" add_foreign_key "dossiers", "procedure_revisions", column: "revision_id" From 6f3bf48d3096a64b0c84d875a9156c01fd0f92b2 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Mon, 16 Nov 2020 16:18:06 +0100 Subject: [PATCH 3/3] add specific totp label for dev env --- app/controllers/super_admins_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/super_admins_controller.rb b/app/controllers/super_admins_controller.rb index 7b5bf18d4..e6c726426 100644 --- a/app/controllers/super_admins_controller.rb +++ b/app/controllers/super_admins_controller.rb @@ -22,6 +22,7 @@ class SuperAdminsController < ApplicationController def generate_qr_code issuer = 'DSManager' + issuer += "-dev" if Rails.env.development? label = "#{issuer}:#{current_super_admin.email}" RQRCode::QRCode.new(current_super_admin.otp_provisioning_uri(label, issuer: issuer)) end