Skip to content

Commit 87ec187

Browse files
committed
Check redirect url whitelist in all queries and mutations
1 parent 184ce7f commit 87ec187

11 files changed

Lines changed: 78 additions & 8 deletions

lib/graphql_devise/concerns/controller_methods.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ module ControllerMethods
77

88
private
99

10+
def check_redirect_url_whitelist!(redirect_url)
11+
if blacklisted_redirect_url?(redirect_url)
12+
raise_user_error(I18n.t('graphql_devise.redirect_url_not_allowed', redirect_url: redirect_url))
13+
end
14+
end
15+
1016
def raise_user_error(message)
1117
raise GraphqlDevise::UserError, message
1218
end

lib/graphql_devise/mutations/resend_confirmation.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ class ResendConfirmation < Base
99
field :message, String, null: false
1010

1111
def resolve(email:, redirect_url:)
12+
check_redirect_url_whitelist!(redirect_url)
13+
1214
resource = find_confirmable_resource(email)
1315

1416
if resource

lib/graphql_devise/mutations/send_password_reset.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ class SendPasswordReset < Base
99
field :message, String, null: false
1010

1111
def resolve(email:, redirect_url:)
12+
check_redirect_url_whitelist!(redirect_url)
13+
1214
resource = find_resource(:email, get_case_insensitive_field(:email, email))
1315

1416
if resource

lib/graphql_devise/mutations/sign_up.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@ def resolve(confirm_success_url: nil, **attrs)
2222
raise_user_error(I18n.t('graphql_devise.registrations.missing_confirm_redirect_url'))
2323
end
2424

25-
if blacklisted_redirect_url?(redirect_url)
26-
raise_user_error(I18n.t('graphql_devise.redirect_url_not_allowed', redirect_url: redirect_url))
27-
end
25+
check_redirect_url_whitelist!(redirect_url)
2826

2927
resource.skip_confirmation_notification! if resource.respond_to?(:skip_confirmation_notification!)
3028

lib/graphql_devise/resolvers/check_password_token.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ def resolve(reset_password_token:, redirect_url: nil)
2727
)
2828

2929
if redirect_url.present?
30+
check_redirect_url_whitelist!(redirect_url)
3031
controller.redirect_to(resource.build_auth_url(redirect_url, built_redirect_headers))
3132
else
3233
set_auth_headers(resource)

lib/graphql_devise/resolvers/confirm_account.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ class ConfirmAccount < Base
77
argument :redirect_url, String, required: true
88

99
def resolve(confirmation_token:, redirect_url:)
10+
check_redirect_url_whitelist!(redirect_url)
11+
1012
resource = resource_class.confirm_by_token(confirmation_token)
1113

1214
if resource.errors.empty?

spec/requests/mutations/additional_mutations_spec.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
let(:password) { Faker::Internet.password }
1010
let(:password_confirmation) { password }
1111
let(:email) { Faker::Internet.email }
12-
let(:redirect) { Faker::Internet.url }
1312

1413
context 'when using the user model' do
1514
let(:query) do

spec/requests/mutations/resend_confirmation_spec.rb

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
let!(:user) { create(:user, confirmed_at: nil, email: 'mwallace@wallaceinc.com') }
1010
let(:email) { user.email }
1111
let(:id) { user.id }
12-
let(:redirect) { Faker::Internet.url }
12+
let(:redirect) { 'https://google.com' }
1313
let(:query) do
1414
<<-GRAPHQL
1515
mutation {
@@ -23,6 +23,21 @@
2323
GRAPHQL
2424
end
2525

26+
context 'when redirect_url is not whitelisted' do
27+
let(:redirect) { 'https://not-safe.com' }
28+
29+
it 'returns a not whitelisted redirect url error' do
30+
expect { post_request }.to not_change(ActionMailer::Base.deliveries, :count)
31+
32+
expect(json_response[:errors]).to containing_exactly(
33+
hash_including(
34+
message: "Redirect to '#{redirect}' not allowed.",
35+
extensions: { code: 'USER_ERROR' }
36+
)
37+
)
38+
end
39+
end
40+
2641
context 'when params are correct' do
2742
context 'when using the gem schema' do
2843
it 'sends an email to the user with confirmation url and returns a success message' do

spec/requests/mutations/send_password_reset_spec.rb

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
let!(:user) { create(:user, :confirmed, email: 'jwinnfield@wallaceinc.com') }
99
let(:email) { user.email }
10-
let(:redirect_url) { Faker::Internet.url }
10+
let(:redirect_url) { 'https://google.com' }
1111
let(:query) do
1212
<<-GRAPHQL
1313
mutation {
@@ -21,6 +21,21 @@
2121
GRAPHQL
2222
end
2323

24+
context 'when redirect_url is not whitelisted' do
25+
let(:redirect_url) { 'https://not-safe.com' }
26+
27+
it 'returns a not whitelisted redirect url error' do
28+
expect { post_request }.to not_change(ActionMailer::Base.deliveries, :count)
29+
30+
expect(json_response[:errors]).to containing_exactly(
31+
hash_including(
32+
message: "Redirect to '#{redirect_url}' not allowed.",
33+
extensions: { code: 'USER_ERROR' }
34+
)
35+
)
36+
end
37+
end
38+
2439
context 'when params are correct' do
2540
context 'when using the gem schema' do
2641
it 'sends password reset email' do

spec/requests/queries/check_password_token_spec.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,21 @@
5454
expect(response.body).to include('uid=')
5555
expect(response.body).to include('expiry=')
5656
end
57+
58+
context 'when redirect_url is not whitelisted' do
59+
let(:redirect_url) { 'https://not-safe.com' }
60+
61+
before { post_request }
62+
63+
it 'returns a not whitelisted redirect url error' do
64+
expect(json_response[:errors]).to containing_exactly(
65+
hash_including(
66+
message: "Redirect to '#{redirect_url}' not allowed.",
67+
extensions: { code: 'USER_ERROR' }
68+
)
69+
)
70+
end
71+
end
5772
end
5873

5974
context 'when token has expired' do

0 commit comments

Comments
 (0)