From aeb71bf4513df151b0744a75984518269dace19b Mon Sep 17 00:00:00 2001
From: David Revelo <david@aperturaradical.org>
Date: Sun, 7 Feb 2021 10:02:04 -0500
Subject: [PATCH 1/4] Allow setting current resource in tests

---
 lib/graphql_devise/schema_plugin.rb  |  25 +++--
 spec/graphql/user_queries_spec.rb    | 149 +++++++++++++++++++++++++++
 spec/support/contexts/schema_test.rb |  15 +++
 3 files changed, 178 insertions(+), 11 deletions(-)
 create mode 100644 spec/graphql/user_queries_spec.rb
 create mode 100644 spec/support/contexts/schema_test.rb

diff --git a/lib/graphql_devise/schema_plugin.rb b/lib/graphql_devise/schema_plugin.rb
index 46f05192..fbdaf59a 100644
--- a/lib/graphql_devise/schema_plugin.rb
+++ b/lib/graphql_devise/schema_plugin.rb
@@ -40,18 +40,21 @@ def trace(event, trace_data)
     private
 
     def set_current_resource(context)
-      controller                 = context[:controller]
-      resource_names             = Array(context[:resource_name])
-      context[:current_resource] = resource_names.find do |resource_name|
-        unless Devise.mappings.key?(resource_name)
-          raise(
-            GraphqlDevise::Error,
-            "Invalid resource_name `#{resource_name}` provided to `graphql_context`. Possible values are: #{Devise.mappings.keys}."
-          )
+      controller     = context[:controller]
+      resource_names = Array(context[:resource_name])
+
+      if context[:current_resource].blank?
+        context[:current_resource] = resource_names.find do |resource_name|
+          unless Devise.mappings.key?(resource_name)
+            raise(
+              GraphqlDevise::Error,
+              "Invalid resource_name `#{resource_name}` provided to `graphql_context`. Possible values are: #{Devise.mappings.keys}."
+            )
+          end
+
+          found = controller.set_resource_by_token(resource_name)
+          break found if found
         end
-
-        found = controller.set_resource_by_token(resource_name)
-        break found if found
       end
 
       context
diff --git a/spec/graphql/user_queries_spec.rb b/spec/graphql/user_queries_spec.rb
new file mode 100644
index 00000000..780f5765
--- /dev/null
+++ b/spec/graphql/user_queries_spec.rb
@@ -0,0 +1,149 @@
+
+require 'rails_helper'
+
+RSpec.describe 'Sign Up process' do
+  include_context 'with graphql schema test'
+
+  let(:schema) { DummySchema }
+  let(:user)   { create(:user, :confirmed) }
+
+  describe 'publicField' do
+    let(:query) do
+      <<-GRAPHQL
+        query {
+          publicField
+        }
+      GRAPHQL
+    end
+
+    context 'when using a regular schema' do
+      it 'does not require authentication' do
+        expect(response[:data][:publicField]).to eq('Field does not require authentication')
+      end
+    end
+  end
+
+  describe 'privateField' do
+    let(:query) do
+      <<-GRAPHQL
+        query {
+          privateField
+        }
+      GRAPHQL
+    end
+
+    context 'when using a regular schema' do
+      context 'when user is authenticated' do
+        let(:resource) { user }
+
+        it 'allows to perform the query' do
+          expect(response[:data][:privateField]).to eq('Field will always require authentication')
+        end
+
+        context 'when using a SchemaUser' do
+          let(:resource) { create(:schema_user, :confirmed) }
+
+          it 'allows to perform the query' do
+            expect(response[:data][:privateField]).to eq('Field will always require authentication')
+          end
+        end
+      end
+
+      context 'when user is not authenticated' do
+        it 'returns a must sign in error' do
+          expect(response[:errors]).to contain_exactly(
+            hash_including(
+              message:    'privateField field requires authentication',
+              extensions: { code: 'AUTHENTICATION_ERROR' }
+            )
+          )
+        end
+      end
+    end
+
+    context 'when using an interpreter schema' do
+      let(:schema) { InterpreterSchema }
+
+      context 'when user is authenticated' do
+        let(:resource) { user }
+
+        it 'allows to perform the query' do
+          expect(response[:data][:privateField]).to eq('Field will always require authentication')
+        end
+      end
+
+      context 'when user is not authenticated' do
+        it 'returns a must sign in error' do
+          expect(response[:errors]).to contain_exactly(
+            hash_including(
+              message:    'privateField field requires authentication',
+              extensions: { code: 'AUTHENTICATION_ERROR' }
+            )
+          )
+        end
+      end
+    end
+  end
+
+  describe 'user' do
+    let(:query) do
+      <<-GRAPHQL
+        query {
+          user(id: #{user.id}) {
+            id
+            email
+          }
+        }
+      GRAPHQL
+    end
+
+    context 'when using a regular schema' do
+      context 'when user is authenticated' do
+        let(:resource) { user }
+
+        it 'allows to perform the query' do
+          expect(response[:data][:user]).to match(
+            email: user.email,
+            id:    user.id
+          )
+        end
+      end
+
+      context 'when user is not authenticated' do
+        it 'returns a must sign in error' do
+          expect(response[:errors]).to contain_exactly(
+            hash_including(
+              message: 'user field requires authentication',
+              extensions: { code: 'AUTHENTICATION_ERROR' }
+            )
+          )
+        end
+      end
+    end
+
+    context 'when using an interpreter schema' do
+      let(:schema) { InterpreterSchema }
+
+      context 'when user is authenticated' do
+        let(:resource) { user }
+
+        it 'allows to perform the query' do
+          expect(response[:data][:user]).to match(
+            email: user.email,
+            id:    user.id
+          )
+        end
+      end
+
+      context 'when user is not authenticated' do
+        # Interpreter schema fields are public unless specified otherwise (plugin setting)
+        it 'allows to perform the query' do
+          expect(response[:data][:user]).to match(
+            email: user.email,
+            id:    user.id
+          )
+        end
+      end
+    end
+  end
+end
diff --git a/spec/support/contexts/schema_test.rb b/spec/support/contexts/schema_test.rb
new file mode 100644
index 00000000..4f15ae47
--- /dev/null
+++ b/spec/support/contexts/schema_test.rb
@@ -0,0 +1,15 @@
+RSpec.shared_context 'with graphql schema test' do
+  let(:variables)  { {} }
+  let(:resource)   { nil }
+  let(:controller) { instance_double(Api::V1::GraphqlController) }
+  let(:context)    { { current_resource: resource, controller: controller } }
+  let(:response) do
+    schema.execute(query, context: context, variables: variables).deep_symbolize_keys
+  end
+
+  # before do
+  #   allow_any_instance_of(GraphqlDevise::Mutations::Login).to receive(:set_auth_headers)
+  #   allow(controller).to receive(:request).and_return(request)
+  #   # allow(controller).to receive(:response).and_return(response)
+  # end
+end

From 302f367f76ce6a1501e1e758af70914b1534c01f Mon Sep 17 00:00:00 2001
From: David Revelo <david@aperturaradical.org>
Date: Mon, 8 Feb 2021 08:27:29 -0500
Subject: [PATCH 2/4] Remove comments, apply suggestions and DRY up specs

---
 lib/graphql_devise/schema_plugin.rb  | 20 ++++-----
 spec/graphql/user_queries_spec.rb    | 61 ++++++++++++----------------
 spec/support/contexts/schema_test.rb |  8 +---
 3 files changed, 35 insertions(+), 54 deletions(-)

diff --git a/lib/graphql_devise/schema_plugin.rb b/lib/graphql_devise/schema_plugin.rb
index fbdaf59a..00c878bb 100644
--- a/lib/graphql_devise/schema_plugin.rb
+++ b/lib/graphql_devise/schema_plugin.rb
@@ -43,18 +43,16 @@ def set_current_resource(context)
       controller     = context[:controller]
       resource_names = Array(context[:resource_name])
 
-      if context[:current_resource].blank?
-        context[:current_resource] = resource_names.find do |resource_name|
-          unless Devise.mappings.key?(resource_name)
-            raise(
-              GraphqlDevise::Error,
-              "Invalid resource_name `#{resource_name}` provided to `graphql_context`. Possible values are: #{Devise.mappings.keys}."
-            )
-          end
-
-          found = controller.set_resource_by_token(resource_name)
-          break found if found
+      context[:current_resource] ||= resource_names.find do |resource_name|
+        unless Devise.mappings.key?(resource_name)
+          raise(
+            GraphqlDevise::Error,
+            "Invalid resource_name `#{resource_name}` provided to `graphql_context`. Possible values are: #{Devise.mappings.keys}."
+          )
         end
+
+        found = controller.set_resource_by_token(resource_name)
+        break found if found
       end
 
       context
diff --git a/spec/graphql/user_queries_spec.rb b/spec/graphql/user_queries_spec.rb
index 780f5765..5ec1854b 100644
--- a/spec/graphql/user_queries_spec.rb
+++ b/spec/graphql/user_queries_spec.rb
@@ -1,11 +1,21 @@
+# frozen_string_literal: true
 
 require 'rails_helper'
 
 RSpec.describe 'Sign Up process' do
   include_context 'with graphql schema test'
 
-  let(:schema) { DummySchema }
-  let(:user)   { create(:user, :confirmed) }
+  let(:schema)          { DummySchema }
+  let(:user)            { create(:user, :confirmed) }
+  let(:field)           { 'privateField' }
+  let(:public_message)  { 'Field does not require authentication' }
+  let(:private_message) { 'Field will always require authentication' }
+  let(:private_error) do
+    {
+      message:    "#{field} field requires authentication",
+      extensions: { code: 'AUTHENTICATION_ERROR' }
+    }
+  end
 
   describe 'publicField' do
     let(:query) do
@@ -18,7 +28,7 @@
 
     context 'when using a regular schema' do
       it 'does not require authentication' do
-        expect(response[:data][:publicField]).to eq('Field does not require authentication')
+        expect(response[:data][:publicField]).to eq(public_message)
       end
     end
   end
@@ -37,26 +47,21 @@
         let(:resource) { user }
 
         it 'allows to perform the query' do
-          expect(response[:data][:privateField]).to eq('Field will always require authentication')
+          expect(response[:data][:privateField]).to eq(private_message)
         end
 
         context 'when using a SchemaUser' do
           let(:resource) { create(:schema_user, :confirmed) }
 
           it 'allows to perform the query' do
-            expect(response[:data][:privateField]).to eq('Field will always require authentication')
+            expect(response[:data][:privateField]).to eq(private_message)
           end
         end
       end
 
       context 'when user is not authenticated' do
         it 'returns a must sign in error' do
-          expect(response[:errors]).to contain_exactly(
-            hash_including(
-              message:    'privateField field requires authentication',
-              extensions: { code: 'AUTHENTICATION_ERROR' }
-            )
-          )
+          expect(response[:errors]).to contain_exactly(hash_including(**private_error))
         end
       end
     end
@@ -68,24 +73,20 @@
         let(:resource) { user }
 
         it 'allows to perform the query' do
-          expect(response[:data][:privateField]).to eq('Field will always require authentication')
+          expect(response[:data][:privateField]).to eq(private_message)
         end
       end
 
       context 'when user is not authenticated' do
         it 'returns a must sign in error' do
-          expect(response[:errors]).to contain_exactly(
-            hash_including(
-              message:    'privateField field requires authentication',
-              extensions: { code: 'AUTHENTICATION_ERROR' }
-            )
-          )
+          expect(response[:errors]).to contain_exactly(hash_including(**private_error))
         end
       end
     end
   end
 
   describe 'user' do
+    let(:user_data) { { email: user.email, id: user.id } }
     let(:query) do
       <<-GRAPHQL
         query {
@@ -102,21 +103,15 @@
         let(:resource) { user }
 
         it 'allows to perform the query' do
-          expect(response[:data][:user]).to match(
-            email: user.email,
-            id:    user.id
-          )
+          expect(response[:data][:user]).to match(**user_data)
         end
       end
 
       context 'when user is not authenticated' do
+        let(:field) { 'user' }
+
         it 'returns a must sign in error' do
-          expect(response[:errors]).to contain_exactly(
-            hash_including(
-              message: 'user field requires authentication',
-              extensions: { code: 'AUTHENTICATION_ERROR' }
-            )
-          )
+          expect(response[:errors]).to contain_exactly(hash_including(**private_error))
         end
       end
     end
@@ -128,20 +123,14 @@
         let(:resource) { user }
 
         it 'allows to perform the query' do
-          expect(response[:data][:user]).to match(
-            email: user.email,
-            id:    user.id
-          )
+          expect(response[:data][:user]).to match(**user_data)
         end
       end
 
       context 'when user is not authenticated' do
         # Interpreter schema fields are public unless specified otherwise (plugin setting)
         it 'allows to perform the query' do
-          expect(response[:data][:user]).to match(
-            email: user.email,
-            id:    user.id
-          )
+          expect(response[:data][:user]).to match(**user_data)
         end
       end
     end
diff --git a/spec/support/contexts/schema_test.rb b/spec/support/contexts/schema_test.rb
index 4f15ae47..205bb329 100644
--- a/spec/support/contexts/schema_test.rb
+++ b/spec/support/contexts/schema_test.rb
@@ -1,15 +1,9 @@
 RSpec.shared_context 'with graphql schema test' do
   let(:variables)  { {} }
   let(:resource)   { nil }
-  let(:controller) { instance_double(Api::V1::GraphqlController) }
+  let(:controller) { instance_double(GraphqlDevise::GraphqlController) }
   let(:context)    { { current_resource: resource, controller: controller } }
   let(:response) do
     schema.execute(query, context: context, variables: variables).deep_symbolize_keys
   end
-
-  # before do
-  #   allow_any_instance_of(GraphqlDevise::Mutations::Login).to receive(:set_auth_headers)
-  #   allow(controller).to receive(:request).and_return(request)
-  #   # allow(controller).to receive(:response).and_return(response)
-  # end
 end

From d050585651f1dcefb3b173cdd9f7f4f7e7971277 Mon Sep 17 00:00:00 2001
From: David Revelo <david@aperturaradical.org>
Date: Tue, 9 Feb 2021 10:24:03 -0500
Subject: [PATCH 3/4] Allow setting resource name in gql test shared context

---
 spec/support/contexts/schema_test.rb | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/spec/support/contexts/schema_test.rb b/spec/support/contexts/schema_test.rb
index 205bb329..f346e78e 100644
--- a/spec/support/contexts/schema_test.rb
+++ b/spec/support/contexts/schema_test.rb
@@ -1,8 +1,11 @@
 RSpec.shared_context 'with graphql schema test' do
-  let(:variables)  { {} }
-  let(:resource)   { nil }
-  let(:controller) { instance_double(GraphqlDevise::GraphqlController) }
-  let(:context)    { { current_resource: resource, controller: controller } }
+  let(:variables)      { {} }
+  let(:resource_names) { [] }
+  let(:resource)       { nil }
+  let(:controller)     { instance_double(GraphqlDevise::GraphqlController) }
+  let(:context) do
+    { current_resource: resource, controller: controller, resource_name: resource_names }
+  end
   let(:response) do
     schema.execute(query, context: context, variables: variables).deep_symbolize_keys
   end

From ab716ff5e0895e8628b1594f589b65b8de484d4c Mon Sep 17 00:00:00 2001
From: David Revelo <david@aperturaradical.org>
Date: Wed, 10 Feb 2021 08:55:59 -0500
Subject: [PATCH 4/4] Avoid setting current resource on public fields

---
 lib/graphql_devise/schema_plugin.rb   | 15 ++++++++-------
 spec/graphql/user_queries_spec.rb     | 22 +---------------------
 spec/requests/user_controller_spec.rb | 18 +++++++++---------
 spec/support/contexts/schema_test.rb  |  4 +++-
 4 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/lib/graphql_devise/schema_plugin.rb b/lib/graphql_devise/schema_plugin.rb
index 00c878bb..c7b5a6ca 100644
--- a/lib/graphql_devise/schema_plugin.rb
+++ b/lib/graphql_devise/schema_plugin.rb
@@ -24,13 +24,12 @@ def trace(event, trace_data)
       # Authenticate only root level queries
       return yield unless event == 'execute_field' && path(trace_data).count == 1
 
-      field = traced_field(trace_data)
-      provided_value = authenticate_option(field, trace_data)
-      context = set_current_resource(context_from_data(trace_data))
+      field         = traced_field(trace_data)
+      auth_required = authenticate_option(field, trace_data)
+      context       = context_from_data(trace_data)
 
-      if !provided_value.nil?
-        raise_on_missing_resource(context, field) if provided_value
-      elsif @authenticate_default
+      if auth_required
+        context = set_current_resource(context)
         raise_on_missing_resource(context, field)
       end
 
@@ -89,11 +88,13 @@ def traced_field(trace_data)
     end
 
     def authenticate_option(field, trace_data)
-      if trace_data[:context]
+      auth_required = if trace_data[:context]
         field.metadata[:authenticate]
       else
         field.graphql_definition.metadata[:authenticate]
       end
+
+      auth_required.nil? ? @authenticate_default : auth_required
     end
 
     def reconfigure_warden!
diff --git a/spec/graphql/user_queries_spec.rb b/spec/graphql/user_queries_spec.rb
index 5ec1854b..17be663a 100644
--- a/spec/graphql/user_queries_spec.rb
+++ b/spec/graphql/user_queries_spec.rb
@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe 'Sign Up process' do
+RSpec.describe 'Users controller specs' do
   include_context 'with graphql schema test'
 
   let(:schema)          { DummySchema }
@@ -58,12 +58,6 @@
           end
         end
       end
-
-      context 'when user is not authenticated' do
-        it 'returns a must sign in error' do
-          expect(response[:errors]).to contain_exactly(hash_including(**private_error))
-        end
-      end
     end
 
     context 'when using an interpreter schema' do
@@ -76,12 +70,6 @@
           expect(response[:data][:privateField]).to eq(private_message)
         end
       end
-
-      context 'when user is not authenticated' do
-        it 'returns a must sign in error' do
-          expect(response[:errors]).to contain_exactly(hash_including(**private_error))
-        end
-      end
     end
   end
 
@@ -106,14 +94,6 @@
           expect(response[:data][:user]).to match(**user_data)
         end
       end
-
-      context 'when user is not authenticated' do
-        let(:field) { 'user' }
-
-        it 'returns a must sign in error' do
-          expect(response[:errors]).to contain_exactly(hash_including(**private_error))
-        end
-      end
     end
 
     context 'when using an interpreter schema' do
diff --git a/spec/requests/user_controller_spec.rb b/spec/requests/user_controller_spec.rb
index a16fa2f1..29fadac5 100644
--- a/spec/requests/user_controller_spec.rb
+++ b/spec/requests/user_controller_spec.rb
@@ -31,15 +31,6 @@
         expect(json_response[:data][:publicField]).to eq('Field does not require authentication')
       end
     end
-
-    context 'when using the failing route' do
-      it 'raises an invalid resource_name error' do
-        expect { post_request('/api/v1/failing') }.to raise_error(
-          GraphqlDevise::Error,
-          'Invalid resource_name `fail` provided to `graphql_context`. Possible values are: [:user, :admin, :guest, :users_customer, :schema_user].'
-        )
-      end
-    end
   end
 
   describe 'privateField' do
@@ -77,6 +68,15 @@
           )
         end
       end
+
+      context 'when using the failing route' do
+        it 'raises an invalid resource_name error' do
+          expect { post_request('/api/v1/failing') }.to raise_error(
+            GraphqlDevise::Error,
+            'Invalid resource_name `fail` provided to `graphql_context`. Possible values are: [:user, :admin, :guest, :users_customer, :schema_user].'
+          )
+        end
+      end
     end
 
     context 'when using an interpreter schema' do
diff --git a/spec/support/contexts/schema_test.rb b/spec/support/contexts/schema_test.rb
index f346e78e..6fcba5a8 100644
--- a/spec/support/contexts/schema_test.rb
+++ b/spec/support/contexts/schema_test.rb
@@ -1,6 +1,8 @@
+# frozen_string_literal: true
+
 RSpec.shared_context 'with graphql schema test' do
   let(:variables)      { {} }
-  let(:resource_names) { [] }
+  let(:resource_names) { [:user] }
   let(:resource)       { nil }
   let(:controller)     { instance_double(GraphqlDevise::GraphqlController) }
   let(:context) do