Merge pull request #140 from graphql-devise/whitelist-redirect-urls

Add redirect whitelist validation to all queries and mutations

config/locales/en.yml

@@ -1,13 +1,13 @@
 en:
   graphql_devise:
+    redirect_url_not_allowed: "Redirect to '%{redirect_url}' not allowed."
     registration_failed: "User couldn't be registered"
     resource_build_failed: "Resource couldn't be built, execution stopped."
     not_authenticated: "User is not logged in."
     user_not_found: "User was not found or was not logged in."
     invalid_resource: "Errors present in the resource."
     registrations:
       missing_confirm_redirect_url: "Missing 'confirm_success_url' parameter. Required when confirmable module is enabled."
-      redirect_url_not_allowed: "Redirect to '%{redirect_url}' not allowed."
     passwords:
       update_password_error: "Unable to update user password"
       missing_passwords: "You must fill out the fields labeled 'Password' and 'Password confirmation'."

lib/graphql_devise/concerns/controller_methods.rb

@@ -7,6 +7,12 @@ module ControllerMethods
 
       private
 
+      def check_redirect_url_whitelist!(redirect_url)
+        if blacklisted_redirect_url?(redirect_url)
+          raise_user_error(I18n.t('graphql_devise.redirect_url_not_allowed', redirect_url: redirect_url))
+        end
+      end
+
       def raise_user_error(message)
         raise GraphqlDevise::UserError, message
       end

lib/graphql_devise/mutations/resend_confirmation.rb

@@ -9,6 +9,8 @@ class ResendConfirmation < Base
       field :message, String, null: false
 
       def resolve(email:, redirect_url:)
+        check_redirect_url_whitelist!(redirect_url)
+
         resource = find_confirmable_resource(email)
 
         if resource

lib/graphql_devise/mutations/send_password_reset.rb

@@ -9,6 +9,8 @@ class SendPasswordReset < Base
       field :message, String, null: false
 
       def resolve(email:, redirect_url:)
+        check_redirect_url_whitelist!(redirect_url)
+
         resource = find_resource(:email, get_case_insensitive_field(:email, email))
 
         if resource

lib/graphql_devise/mutations/sign_up.rb

@@ -22,9 +22,7 @@ def resolve(confirm_success_url: nil, **attrs)
           raise_user_error(I18n.t('graphql_devise.registrations.missing_confirm_redirect_url'))
         end
 
-        if blacklisted_redirect_url?(redirect_url)
-          raise_user_error(I18n.t('graphql_devise.registrations.redirect_url_not_allowed', redirect_url: redirect_url))
-        end
+        check_redirect_url_whitelist!(redirect_url)
 
         resource.skip_confirmation_notification! if resource.respond_to?(:skip_confirmation_notification!)
 

lib/graphql_devise/resolvers/check_password_token.rb

@@ -27,6 +27,7 @@ def resolve(reset_password_token:, redirect_url: nil)
           )
 
           if redirect_url.present?
+            check_redirect_url_whitelist!(redirect_url)
             controller.redirect_to(resource.build_auth_url(redirect_url, built_redirect_headers))
           else
             set_auth_headers(resource)

lib/graphql_devise/resolvers/confirm_account.rb

@@ -7,6 +7,8 @@ class ConfirmAccount < Base
       argument :redirect_url,       String, required: true
 
       def resolve(confirmation_token:, redirect_url:)
+        check_redirect_url_whitelist!(redirect_url)
+
         resource = resource_class.confirm_by_token(confirmation_token)
 
         if resource.errors.empty?

spec/dummy/config/initializers/devise_token_auth.rb

@@ -39,6 +39,8 @@
 
   config.default_confirm_success_url = 'https://google.com'
 
+  config.redirect_whitelist = ['https://google.com']
+
   # By default we will use callbacks for single omniauth.
   # It depends on fields like email, provider and uid.
   # config.default_callbacks = true

spec/requests/mutations/additional_mutations_spec.rb

@@ -9,7 +9,6 @@
   let(:password)              { Faker::Internet.password }
   let(:password_confirmation) { password }
   let(:email)                 { Faker::Internet.email }
-  let(:redirect)              { Faker::Internet.url }
 
   context 'when using the user model' do
     let(:query) do

spec/requests/mutations/resend_confirmation_spec.rb

@@ -9,7 +9,7 @@
   let!(:user)        { create(:user, confirmed_at: nil, email: 'mwallace@wallaceinc.com') }
   let(:email)        { user.email }
   let(:id)           { user.id }
-  let(:redirect)     { Faker::Internet.url }
+  let(:redirect)     { 'https://google.com' }
   let(:query) do
     <<-GRAPHQL
       mutation {
@@ -23,6 +23,21 @@
     GRAPHQL
   end
 
+  context 'when redirect_url is not whitelisted' do
+    let(:redirect) { 'https://not-safe.com' }
+
+    it 'returns a not whitelisted redirect url error' do
+      expect { post_request }.to not_change(ActionMailer::Base.deliveries, :count)
+
+      expect(json_response[:errors]).to containing_exactly(
+        hash_including(
+          message:    "Redirect to '#{redirect}' not allowed.",
+          extensions: { code: 'USER_ERROR' }
+        )
+      )
+    end
+  end
+
   context 'when params are correct' do
     context 'when using the gem schema' do
       it 'sends an email to the user with confirmation url and returns a success message' do

spec/requests/mutations/send_password_reset_spec.rb

@@ -7,7 +7,7 @@
 
   let!(:user)        { create(:user, :confirmed, email: 'jwinnfield@wallaceinc.com') }
   let(:email)        { user.email }
-  let(:redirect_url) { Faker::Internet.url }
+  let(:redirect_url) { 'https://google.com' }
   let(:query) do
     <<-GRAPHQL
       mutation {
@@ -21,6 +21,21 @@
     GRAPHQL
   end
 
+  context 'when redirect_url is not whitelisted' do
+    let(:redirect_url) { 'https://not-safe.com' }
+
+    it 'returns a not whitelisted redirect url error' do
+      expect { post_request }.to not_change(ActionMailer::Base.deliveries, :count)
+
+      expect(json_response[:errors]).to containing_exactly(
+        hash_including(
+          message:    "Redirect to '#{redirect_url}' not allowed.",
+          extensions: { code: 'USER_ERROR' }
+        )
+      )
+    end
+  end
+
   context 'when params are correct' do
     context 'when using the gem schema' do
       it 'sends password reset  email' do

spec/requests/mutations/sign_up_spec.rb

@@ -8,7 +8,7 @@
   let(:name)     { Faker::Name.name }
   let(:password) { Faker::Internet.password }
   let(:email)    { Faker::Internet.email }
-  let(:redirect) { Faker::Internet.url }
+  let(:redirect) { 'https://google.com' }
 
   context 'when using the user model' do
     let(:query) do
@@ -31,6 +31,24 @@
       GRAPHQL
     end
 
+    context 'when redirect_url is not whitelisted' do
+      let(:redirect) { 'https://not-safe.com' }
+
+      it 'returns a not whitelisted redirect url error' do
+        expect { post_request }.to(
+          not_change(User, :count)
+          .and(not_change(ActionMailer::Base.deliveries, :count))
+        )
+
+        expect(json_response[:errors]).to containing_exactly(
+          hash_including(
+            message:    "Redirect to '#{redirect}' not allowed.",
+            extensions: { code: 'USER_ERROR' }
+          )
+        )
+      end
+    end
+
     context 'when params are correct' do
       it 'creates a new resource that requires confirmation' do
         expect { post_request }.to(

spec/requests/queries/check_password_token_spec.rb

@@ -54,6 +54,21 @@
           expect(response.body).to include('uid=')
           expect(response.body).to include('expiry=')
         end
+
+        context 'when redirect_url is not whitelisted' do
+          let(:redirect_url) { 'https://not-safe.com' }
+
+          before { post_request }
+
+          it 'returns a not whitelisted redirect url error' do
+            expect(json_response[:errors]).to containing_exactly(
+              hash_including(
+                message:    "Redirect to '#{redirect_url}' not allowed.",
+                extensions: { code: 'USER_ERROR' }
+              )
+            )
+          end
+        end
       end
 
       context 'when token has expired' do

spec/requests/queries/confirm_account_spec.rb

@@ -7,7 +7,7 @@
 
   context 'when using the user model' do
     let(:user)     { create(:user, confirmed_at: nil) }
-    let(:redirect) { Faker::Internet.url }
+    let(:redirect) { 'https://google.com' }
     let(:query) do
       <<-GRAPHQL
         {
@@ -43,6 +43,21 @@
         expect(user).to be_active_for_authentication
       end
 
+      context 'when redirect_url is not whitelisted' do
+        let(:redirect) { 'https://not-safe.com' }
+
+        it 'returns a not whitelisted redirect url error' do
+          expect { post_request }.to not_change(ActionMailer::Base.deliveries, :count)
+
+          expect(json_response[:errors]).to containing_exactly(
+            hash_including(
+              message:    "Redirect to '#{redirect}' not allowed.",
+              extensions: { code: 'USER_ERROR' }
+            )
+          )
+        end
+      end
+
       context 'when unconfirmed_email is present' do
         let(:user) { create(:user, :confirmed, unconfirmed_email: 'vvega@wallaceinc.com') }
 
@@ -81,7 +96,7 @@
 
   context 'when using the admin model' do
     let(:admin)    { create(:admin, confirmed_at: nil) }
-    let(:redirect) { Faker::Internet.url }
+    let(:redirect) { 'https://google.com' }
     let(:query) do
       <<-GRAPHQL
         {