Avoid returning user information on password reset mutation

David Revelo · David Revelo · commit af0af5143ab8 · 2020-06-12T08:22:05.000-05:00
diff --git a/lib/graphql_devise/default_operations/mutations.rb b/lib/graphql_devise/default_operations/mutations.rb
@@ -9,12 +9,13 @@
 module GraphqlDevise
   module DefaultOperations
     MUTATIONS = {
-      login:               GraphqlDevise::Mutations::Login,
-      logout:              GraphqlDevise::Mutations::Logout,
-      sign_up:             GraphqlDevise::Mutations::SignUp,
-      update_password:     GraphqlDevise::Mutations::UpdatePassword,
-      send_password_reset: GraphqlDevise::Mutations::SendPasswordReset,
-      resend_confirmation: GraphqlDevise::Mutations::ResendConfirmation
+      login:               { klass: GraphqlDevise::Mutations::Login, authenticable: true },
+      logout:              { klass: GraphqlDevise::Mutations::Logout, authenticable: true },
+      sign_up:             { klass: GraphqlDevise::Mutations::SignUp, authenticable: true },
+      update_password:     { klass: GraphqlDevise::Mutations::UpdatePassword, authenticable: true },
+      send_password_reset: { klass: GraphqlDevise::Mutations::SendPasswordReset, authenticable: true },
+      resend_confirmation: { klass: GraphqlDevise::Mutations::ResendConfirmation, authenticable: true }
+
     }.freeze
   end
 end
diff --git a/lib/graphql_devise/default_operations/resolvers.rb b/lib/graphql_devise/default_operations/resolvers.rb
@@ -5,8 +5,8 @@
 module GraphqlDevise
   module DefaultOperations
     QUERIES = {
-      confirm_account:      GraphqlDevise::Resolvers::ConfirmAccount,
-      check_password_token: GraphqlDevise::Resolvers::CheckPasswordToken
+      confirm_account:      { klass: GraphqlDevise::Resolvers::ConfirmAccount },
+      check_password_token: { klass: GraphqlDevise::Resolvers::CheckPasswordToken }
     }.freeze
   end
 end
diff --git a/lib/graphql_devise/mount_method/operation_preparers/default_operation_preparer.rb b/lib/graphql_devise/mount_method/operation_preparers/default_operation_preparer.rb
@@ -10,14 +10,18 @@ def initialize(selected_operations:, custom_keys:, mapping_name:, preparer:)
         end
 
         def call
-          @selected_operations.except(*@custom_keys).each_with_object({}) do |(action, operation), result|
+          @selected_operations.except(*@custom_keys).each_with_object({}) do |(action, operation_info), result|
             mapped_action = "#{@mapping_name}_#{action}"
+            operation     = operation_info[:klass]
+            options       = operation_info.except(:klass)
 
             result[mapped_action.to_sym] = [
               OperationPreparers::GqlNameSetter.new(mapped_action),
               @preparer,
               OperationPreparers::ResourceNameSetter.new(@mapping_name)
-            ].reduce(child_class(operation)) { |prepared_operation, preparer| preparer.call(prepared_operation) }
+            ].reduce(child_class(operation)) do |prepared_operation, preparer|
+              preparer.call(prepared_operation, **options)
+            end
           end
         end
 
diff --git a/lib/graphql_devise/mount_method/operation_preparers/gql_name_setter.rb b/lib/graphql_devise/mount_method/operation_preparers/gql_name_setter.rb
@@ -6,7 +6,7 @@ def initialize(mapping_name)
           @mapping_name = mapping_name
         end
 
-        def call(operation)
+        def call(operation, **)
           operation.graphql_name(graphql_name)
 
           operation
diff --git a/lib/graphql_devise/mount_method/operation_preparers/mutation_field_setter.rb b/lib/graphql_devise/mount_method/operation_preparers/mutation_field_setter.rb
@@ -6,9 +6,10 @@ def initialize(authenticatable_type)
           @authenticatable_type = authenticatable_type
         end
 
-        def call(mutation)
-          mutation.field(:authenticatable, @authenticatable_type, null: false)
+        def call(mutation, authenticable: true)
+          return unless authenticable
 
+          mutation.field(:authenticatable, @authenticatable_type, null: false)
           mutation
         end
       end
diff --git a/lib/graphql_devise/mount_method/operation_preparers/resolver_type_setter.rb b/lib/graphql_devise/mount_method/operation_preparers/resolver_type_setter.rb
@@ -6,7 +6,7 @@ def initialize(authenticatable_type)
           @authenticatable_type = authenticatable_type
         end
 
-        def call(resolver)
+        def call(resolver, **)
           resolver.type(@authenticatable_type, null: false)
 
           resolver
diff --git a/lib/graphql_devise/mount_method/operation_preparers/resource_name_setter.rb b/lib/graphql_devise/mount_method/operation_preparers/resource_name_setter.rb
@@ -6,7 +6,7 @@ def initialize(name)
           @name = name
         end
 
-        def call(operation)
+        def call(operation, **)
           operation.instance_variable_set(:@resource_name, @name)
 
           operation
diff --git a/lib/graphql_devise/mutations/send_password_reset.rb b/lib/graphql_devise/mutations/send_password_reset.rb
@@ -4,6 +4,8 @@ class SendPasswordReset < Base
       argument :email,        String, required: true
       argument :redirect_url, String, required: true
 
+      field :message, String, null: false
+
       def resolve(email:, redirect_url:)
         resource = find_resource(:email, get_case_insensitive_field(:email, email))
 
@@ -18,7 +20,7 @@ def resolve(email:, redirect_url:)
           )
 
           if resource.errors.empty?
-            { authenticatable: resource }
+            { message: I18n.t('devise.passwords.send_instructions') }
           else
             raise_user_error_list(I18n.t('graphql_devise.invalid_resource'), errors: resource.errors.full_messages)
           end
diff --git a/spec/requests/mutations/send_password_reset_spec.rb b/spec/requests/mutations/send_password_reset_spec.rb
@@ -13,9 +13,7 @@
           email:       "#{email}",
           redirectUrl: "#{redirect_url}"
         ) {
-          authenticatable {
-            email
-          }
+          message
         }
       }
     GRAPHQL
@@ -25,6 +23,10 @@
     it 'sends password reset  email' do
       expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by(1)
 
+      expect(json_response.dig(:data, :userSendPasswordReset)).to include(
+        message: 'You will receive an email with instructions on how to reset your password in a few minutes.'
+      )
+
       email = Nokogiri::HTML(ActionMailer::Base.deliveries.last.body.encoded)
       link  = email.css('a').first
 
@@ -41,6 +43,9 @@
 
     it 'honors devise configuration for case insensitive fields' do
       expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by(1)
+      expect(json_response.dig(:data, :userSendPasswordReset)).to include(
+        message: 'You will receive an email with instructions on how to reset your password in a few minutes.'
+      )
     end
   end
 
diff --git a/spec/services/mount_method/operation_preparer_spec.rb b/spec/services/mount_method/operation_preparer_spec.rb
@@ -14,10 +14,15 @@
 
     let(:logout_class) { Class.new(GraphQL::Schema::Resolver) }
     let(:mapping)      { :user }
-    let(:selected)     { { login: double(:login_default), logout: logout_class } }
     let(:preparer)     { double(:preparer, call: logout_class) }
     let(:custom)       { { login: double(:custom_login, graphql_name: nil) } }
     let(:additional)   { { user_additional: double(:user_additional) } }
+    let(:selected) do
+      {
+        login: { klass: double(:login_default) },
+        logout:{ klass: logout_class }
+      }
+    end
 
     it 'is expected to return all provided operation keys' do
       expect(prepared_operations.keys).to contain_exactly(
diff --git a/spec/services/mount_method/operation_preparers/default_operation_preparer_spec.rb b/spec/services/mount_method/operation_preparers/default_operation_preparer_spec.rb
@@ -11,18 +11,25 @@
     let(:logout_operation)  { double(:sign_up_operation, graphql_name: nil) }
     let(:mapping_name)      { :user }
     let(:preparer)          { double(:preparer) }
-    let(:operations)        { { login: login_operation, logout: logout_operation, sign_up: sign_up_operation, confirm: confirm_operation } }
     let(:custom_keys)       { [:login, :logout] }
+    let(:operations) do
+      {
+        confirm: { klass: confirm_operation, authenticable: false },
+        sign_up: { klass: sign_up_operation, authenticable: true },
+        login:   { klass: login_operation, authenticable: true },
+        logout:  { klass: logout_operation, authenticable: true }
+      }
+    end
 
     before do
       allow(default_preparer).to receive(:child_class).with(confirm_operation).and_return(confirm_operation)
       allow(default_preparer).to receive(:child_class).with(sign_up_operation).and_return(sign_up_operation)
       allow(default_preparer).to receive(:child_class).with(login_operation).and_return(login_operation)
       allow(default_preparer).to receive(:child_class).with(logout_operation).and_return(logout_operation)
-      allow(preparer).to receive(:call).with(confirm_operation).and_return(confirm_operation)
-      allow(preparer).to receive(:call).with(sign_up_operation).and_return(sign_up_operation)
-      allow(preparer).to receive(:call).with(login_operation).and_return(login_operation)
-      allow(preparer).to receive(:call).with(logout_operation).and_return(logout_operation)
+      allow(preparer).to receive(:call).with(confirm_operation, authenticable: false).and_return(confirm_operation)
+      allow(preparer).to receive(:call).with(sign_up_operation, authenticable: true).and_return(sign_up_operation)
+      allow(preparer).to receive(:call).with(login_operation, authenticable: true).and_return(login_operation)
+      allow(preparer).to receive(:call).with(logout_operation, authenticable: true).and_return(logout_operation)
     end
 
     it 'returns only those operations with no custom operation provided' do
@@ -32,8 +39,8 @@
     it 'prepares default operations' do
       expect(confirm_operation).to receive(:graphql_name).with('UserConfirm')
       expect(sign_up_operation).to receive(:graphql_name).with('UserSignUp')
-      expect(preparer).to receive(:call).with(confirm_operation)
-      expect(preparer).to receive(:call).with(sign_up_operation)
+      expect(preparer).to receive(:call).with(confirm_operation, authenticable: false)
+      expect(preparer).to receive(:call).with(sign_up_operation, authenticable: true)
 
       prepared
 
diff --git a/spec/services/mount_method/operation_preparers/mutation_field_setter_spec.rb b/spec/services/mount_method/operation_preparers/mutation_field_setter_spec.rb
@@ -2,15 +2,29 @@
 
 RSpec.describe GraphqlDevise::MountMethod::OperationPreparers::MutationFieldSetter do
   describe '#call' do
-    subject(:prepared_operation) { described_class.new(field_type).call(operation) }
+    subject(:prepared_operation) { described_class.new(field_type).call(operation, authenticable: authenticable) }
 
     let(:operation)  { double(:operation) }
     let(:field_type) { double(:type) }
 
-    it 'sets a field for the mutation' do
-      expect(operation).to receive(:field).with(:authenticatable, field_type, null: false)
+    context 'when resource is authtenticable' do
+      let(:authenticable) { true }
 
-      prepared_operation
+      it 'sets a field for the mutation' do
+        expect(operation).to receive(:field).with(:authenticatable, field_type, null: false)
+
+        prepared_operation
+      end
+    end
+
+    context 'when resource is *NOT* authtenticable' do
+      let(:authenticable) { false }
+
+      it 'does *NOT* set a field for the mutation' do
+        expect(operation).not_to receive(:field)
+
+        prepared_operation
+      end
     end
   end
 end
