Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/locales/en.yml
Original file line number Diff line number Diff line change
@@ -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'."
Expand Down
6 changes: 6 additions & 0 deletions lib/graphql_devise/concerns/controller_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/graphql_devise/mutations/resend_confirmation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/graphql_devise/mutations/send_password_reset.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions lib/graphql_devise/mutations/sign_up.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!)

Expand Down
1 change: 1 addition & 0 deletions lib/graphql_devise/resolvers/check_password_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/graphql_devise/resolvers/confirm_account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/initializers/devise_token_auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion spec/requests/mutations/additional_mutations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 16 additions & 1 deletion spec/requests/mutations/resend_confirmation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
17 changes: 16 additions & 1 deletion spec/requests/mutations/send_password_reset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
20 changes: 19 additions & 1 deletion spec/requests/mutations/sign_up_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
15 changes: 15 additions & 0 deletions spec/requests/queries/check_password_token_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions spec/requests/queries/confirm_account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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') }

Expand Down Expand Up @@ -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
{
Expand Down