From 8b812cca574c710ce05a98a926c2897eecaeaa60 Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Sat, 29 May 2021 23:06:10 -0500 Subject: [PATCH 1/3] Add register mutation and alternate confirmation flow --- .../mailer/confirmation_instructions.html.erb | 8 +- .../default_operations/mutations.rb | 6 +- .../mutations/confirm_account_with_token.rb | 30 ++++ lib/graphql_devise/mutations/register.rb | 60 +++++++ lib/graphql_devise/mutations/sign_up.rb | 2 +- spec/dummy/app/graphql/mutations/register.rb | 14 ++ spec/dummy/config/routes.rb | 9 +- .../confirm_account_with_token_spec.rb | 117 ++++++++++++ spec/requests/mutations/register_spec.rb | 166 ++++++++++++++++++ 9 files changed, 405 insertions(+), 7 deletions(-) create mode 100644 lib/graphql_devise/mutations/confirm_account_with_token.rb create mode 100644 lib/graphql_devise/mutations/register.rb create mode 100644 spec/dummy/app/graphql/mutations/register.rb create mode 100644 spec/requests/mutations/confirm_account_with_token_spec.rb create mode 100644 spec/requests/mutations/register_spec.rb diff --git a/app/views/graphql_devise/mailer/confirmation_instructions.html.erb b/app/views/graphql_devise/mailer/confirmation_instructions.html.erb index a5a214f5..6c323d84 100644 --- a/app/views/graphql_devise/mailer/confirmation_instructions.html.erb +++ b/app/views/graphql_devise/mailer/confirmation_instructions.html.erb @@ -2,4 +2,10 @@

<%= t('.confirm_link_msg') %>

-

<%= link_to t('.confirm_account_link'), "#{message['schema_url']}?#{confirmation_query(resource_name: @resource.class.to_s, redirect_url: message['redirect-url'], token: @token).to_query}" %>

+

+ <% if message['schema_url'].present? %> + <%= link_to t('.confirm_account_link'), "#{message['schema_url']}?#{confirmation_query(resource_name: @resource.class.to_s, redirect_url: message['redirect-url'], token: @token).to_query}" %> + <% else %> + <%= link_to t('.confirm_account_link'), "#{message['redirect-url'].to_s}?#{{ confirmationToken: @token }.to_query}" %> + <% end %> +

diff --git a/lib/graphql_devise/default_operations/mutations.rb b/lib/graphql_devise/default_operations/mutations.rb index 2b53e5ce..43d335f7 100644 --- a/lib/graphql_devise/default_operations/mutations.rb +++ b/lib/graphql_devise/default_operations/mutations.rb @@ -7,8 +7,10 @@ require 'graphql_devise/mutations/send_password_reset' require 'graphql_devise/mutations/send_password_reset_with_token' require 'graphql_devise/mutations/sign_up' +require 'graphql_devise/mutations/register' require 'graphql_devise/mutations/update_password' require 'graphql_devise/mutations/update_password_with_token' +require 'graphql_devise/mutations/confirm_account_with_token' module GraphqlDevise module DefaultOperations @@ -16,11 +18,13 @@ module DefaultOperations login: { klass: GraphqlDevise::Mutations::Login, authenticatable: true }, logout: { klass: GraphqlDevise::Mutations::Logout, authenticatable: true }, sign_up: { klass: GraphqlDevise::Mutations::SignUp, authenticatable: true }, + register: { klass: GraphqlDevise::Mutations::Register, authenticatable: true }, update_password: { klass: GraphqlDevise::Mutations::UpdatePassword, authenticatable: true }, update_password_with_token: { klass: GraphqlDevise::Mutations::UpdatePasswordWithToken, authenticatable: true }, send_password_reset: { klass: GraphqlDevise::Mutations::SendPasswordReset, authenticatable: false }, send_password_reset_with_token: { klass: GraphqlDevise::Mutations::SendPasswordResetWithToken, authenticatable: false }, - resend_confirmation: { klass: GraphqlDevise::Mutations::ResendConfirmation, authenticatable: false } + resend_confirmation: { klass: GraphqlDevise::Mutations::ResendConfirmation, authenticatable: false }, + confirm_account_with_token: { klass: GraphqlDevise::Mutations::ConfirmAccountWithToken, authenticatable: true } }.freeze end end diff --git a/lib/graphql_devise/mutations/confirm_account_with_token.rb b/lib/graphql_devise/mutations/confirm_account_with_token.rb new file mode 100644 index 00000000..eae5dadb --- /dev/null +++ b/lib/graphql_devise/mutations/confirm_account_with_token.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module GraphqlDevise + module Mutations + class ConfirmAccountWithToken < Base + argument :confirmation_token, String, required: true + + field :credentials, + GraphqlDevise::Types::CredentialType, + null: true, + description: 'Authentication credentials. Null unless user is signed in after confirmation.' + + def resolve(confirmation_token:) + resource = resource_class.confirm_by_token(confirmation_token) + + if resource.errors.empty? + yield resource if block_given? + + response_payload = { authenticatable: resource } + + response_payload[:credentials] = set_auth_headers(resource) if resource.active_for_authentication? + + response_payload + else + raise_user_error(I18n.t('graphql_devise.confirmations.invalid_token')) + end + end + end + end +end diff --git a/lib/graphql_devise/mutations/register.rb b/lib/graphql_devise/mutations/register.rb new file mode 100644 index 00000000..d2bb68eb --- /dev/null +++ b/lib/graphql_devise/mutations/register.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module GraphqlDevise + module Mutations + class Register < Base + argument :email, String, required: true + argument :password, String, required: true + argument :password_confirmation, String, required: true + argument :confirm_url, String, required: false + + field :credentials, + GraphqlDevise::Types::CredentialType, + null: true, + description: 'Authentication credentials. Null if after signUp resource is not active for authentication (e.g. Email confirmation required).' + + def resolve(confirm_url: nil, **attrs) + resource = build_resource(attrs.merge(provider: provider)) + raise_user_error(I18n.t('graphql_devise.resource_build_failed')) if resource.blank? + + redirect_url = confirm_url || DeviseTokenAuth.default_confirm_success_url + if confirmable_enabled? && redirect_url.blank? + raise_user_error(I18n.t('graphql_devise.registrations.missing_confirm_redirect_url')) + end + + check_redirect_url_whitelist!(redirect_url) + + resource.skip_confirmation_notification! if resource.respond_to?(:skip_confirmation_notification!) + + if resource.save + yield resource if block_given? + + unless resource.confirmed? + resource.send_confirmation_instructions( + redirect_url: redirect_url, + template_path: ['graphql_devise/mailer'] + ) + end + + response_payload = { authenticatable: resource } + + response_payload[:credentials] = set_auth_headers(resource) if resource.active_for_authentication? + + response_payload + else + resource.try(:clean_up_passwords) + raise_user_error_list( + I18n.t('graphql_devise.registration_failed'), + errors: resource.errors.full_messages + ) + end + end + + private + + def build_resource(attrs) + resource_class.new(attrs) + end + end + end +end diff --git a/lib/graphql_devise/mutations/sign_up.rb b/lib/graphql_devise/mutations/sign_up.rb index 219a1f72..3b19917e 100644 --- a/lib/graphql_devise/mutations/sign_up.rb +++ b/lib/graphql_devise/mutations/sign_up.rb @@ -31,7 +31,7 @@ def resolve(confirm_success_url: nil, **attrs) unless resource.confirmed? resource.send_confirmation_instructions( - redirect_url: confirm_success_url, + redirect_url: redirect_url, template_path: ['graphql_devise/mailer'], schema_url: controller.full_url_without_params ) diff --git a/spec/dummy/app/graphql/mutations/register.rb b/spec/dummy/app/graphql/mutations/register.rb new file mode 100644 index 00000000..434737eb --- /dev/null +++ b/spec/dummy/app/graphql/mutations/register.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Mutations + class Register < GraphqlDevise::Mutations::Register + argument :name, String, required: false + + field :user, Types::UserType, null: true + + def resolve(email:, **attrs) + original_payload = super + original_payload.merge(user: original_payload[:authenticatable]) + end + end +end diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index bc090e35..f47eec32 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -2,8 +2,9 @@ Rails.application.routes.draw do mount_graphql_devise_for 'User', at: '/api/v1/graphql_auth', operations: { - login: Mutations::Login, - sign_up: Mutations::SignUp + login: Mutations::Login, + sign_up: Mutations::SignUp, + register: Mutations::Register }, additional_mutations: { register_confirmed_user: Mutations::RegisterConfirmedUser }, additional_queries: { @@ -13,7 +14,7 @@ mount_graphql_devise_for( Admin, authenticatable_type: Types::CustomAdminType, - skip: [:sign_up, :check_password_token], + skip: [:sign_up, :register, :check_password_token], operations: { confirm_account: Resolvers::ConfirmAdminAccount, update_password_with_token: Mutations::ResetAdminPasswordWithToken @@ -23,7 +24,7 @@ mount_graphql_devise_for( 'Guest', - only: [:login, :logout, :sign_up], + only: [:login, :logout, :sign_up, :register], at: '/api/v1/guest/graphql_auth' ) diff --git a/spec/requests/mutations/confirm_account_with_token_spec.rb b/spec/requests/mutations/confirm_account_with_token_spec.rb new file mode 100644 index 00000000..2bbf3b69 --- /dev/null +++ b/spec/requests/mutations/confirm_account_with_token_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Account confirmation with token' do + include_context 'with graphql query request' + + context 'when using the user model' do + let(:user) { create(:user, confirmed_at: nil) } + let(:query) do + <<-GRAPHQL + mutation { + userConfirmAccountWithToken( + confirmationToken: "#{token}" + ) { + authenticatable { + email + name + } + credentials { client } + } + } + GRAPHQL + end + + context 'when confirmation token is correct' do + let(:token) { user.confirmation_token } + + before do + user.send_confirmation_instructions( + template_path: ['graphql_devise/mailer'] + ) + end + + it 'confirms the resource and returns credentials' do + expect do + post_request + user.reload + end.to(change(user, :confirmed_at).from(nil)) + + expect(json_response[:data][:userConfirmAccountWithToken]).to include( + authenticatable: { email: user.email, name: user.name }, + credentials: { client: user.tokens.keys.first } + ) + + expect(user).to be_active_for_authentication + end + + context 'when unconfirmed_email is present' do + let(:user) { create(:user, :confirmed, unconfirmed_email: 'vvega@wallaceinc.com') } + + it 'confirms the unconfirmed email' do + expect do + post_request + user.reload + end.to change(user, :email).from(user.email).to('vvega@wallaceinc.com').and( + change(user, :unconfirmed_email).from('vvega@wallaceinc.com').to(nil) + ) + end + end + end + + context 'when reset password token is not found' do + let(:token) { "#{user.confirmation_token}-invalid" } + + it 'does *NOT* confirm the user' do + expect do + post_request + user.reload + end.not_to change(user, :confirmed_at).from(nil) + + expect(json_response[:errors]).to contain_exactly( + hash_including( + message: 'Invalid confirmation token. Please try again', + extensions: { code: 'USER_ERROR' } + ) + ) + end + end + end + + context 'when using the admin model' do + let(:admin) { create(:admin, confirmed_at: nil) } + let(:query) do + <<-GRAPHQL + mutation { + adminConfirmAccountWithToken( + confirmationToken: "#{token}" + ) { + authenticatable { email } + } + } + GRAPHQL + end + + context 'when confirmation token is correct' do + let(:token) { admin.confirmation_token } + + before do + admin.send_confirmation_instructions( + template_path: ['graphql_devise/mailer'] + ) + end + + it 'confirms the resource and persists credentials on the DB' do + expect do + get_request + admin.reload + end.to change(admin, :confirmed_at).from(nil).and( + change { admin.tokens.keys.count }.from(0).to(1) + ) + + expect(admin).to be_active_for_authentication + end + end + end +end diff --git a/spec/requests/mutations/register_spec.rb b/spec/requests/mutations/register_spec.rb new file mode 100644 index 00000000..2aa7d4e1 --- /dev/null +++ b/spec/requests/mutations/register_spec.rb @@ -0,0 +1,166 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Registration process' do + include_context 'with graphql query request' + + let(:name) { Faker::Name.name } + let(:password) { Faker::Internet.password } + let(:email) { Faker::Internet.email } + let(:redirect) { 'https://google.com' } + + context 'when using the user model' do + let(:query) do + <<-GRAPHQL + mutation { + userRegister( + email: "#{email}" + name: "#{name}" + password: "#{password}" + passwordConfirmation: "#{password}" + confirmUrl: "#{redirect}" + ) { + credentials { accessToken } + user { + email + name + } + } + } + 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( + change(User, :count).by(1) + .and(change(ActionMailer::Base.deliveries, :count).by(1)) + ) + + user = User.last + + expect(user).not_to be_active_for_authentication + expect(user.confirmed_at).to be_nil + expect(user).to be_valid_password(password) + expect(json_response[:data][:userRegister]).to include( + credentials: nil, + user: { + email: email, + name: name + } + ) + + email = Nokogiri::HTML(ActionMailer::Base.deliveries.last.body.encoded) + confirm_link = email.css('a').first['href'] + confirm_token = confirm_link.match(/\?confirmationToken\=(?.+)\z/)[:token] + + expect(User.confirm_by_token(confirm_token)).to eq(user) + end + + context 'when email address uses different casing' do + let(:email) { 'miaWallace@wallaceinc.com' } + + it 'honors devise configuration for case insensitive fields' do + expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by(1) + expect(User.last.email).to eq('miawallace@wallaceinc.com') + expect(json_response[:data][:userRegister]).to include(user: { email: 'miawallace@wallaceinc.com', name: name }) + end + end + end + + context 'when required params are missing' do + let(:email) { '' } + + it 'does *NOT* create resource a resource nor send an email' do + expect { post_request }.to( + not_change(User, :count) + .and(not_change(ActionMailer::Base.deliveries, :count)) + ) + + expect(json_response[:data][:userRegister]).to be_nil + expect(json_response[:errors]).to containing_exactly( + hash_including( + message: "User couldn't be registered", + extensions: { code: 'USER_ERROR', detailed_errors: ["Email can't be blank"] } + ) + ) + end + end + end + + context 'when using the admin model' do + let(:query) do + <<-GRAPHQL + mutation { + adminRegister( + email: "#{email}" + password: "#{password}" + passwordConfirmation: "#{password}" + ) { + authenticatable { + email + } + } + } + GRAPHQL + end + + before { post_request } + + it 'skips the register mutation' do + expect(json_response[:errors]).to contain_exactly( + hash_including(message: "Field 'adminRegister' doesn't exist on type 'Mutation'") + ) + end + end + + context 'when using the guest model' do + let(:query) do + <<-GRAPHQL + mutation { + guestRegister( + email: "#{email}" + password: "#{password}" + passwordConfirmation: "#{password}" + ) { + credentials { accessToken client uid } + authenticatable { + email + } + } + } + GRAPHQL + end + + it 'returns credentials as no confirmation is required' do + expect { post_request }.to change(Guest, :count).from(0).to(1) + + expect(json_response[:data][:guestRegister]).to include( + authenticatable: { email: email }, + credentials: hash_including( + uid: email, + client: Guest.last.tokens.keys.first + ) + ) + end + end +end From c7ca85849e82a1728fc399c869af627f0d457789 Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Sun, 30 May 2021 20:47:40 -0500 Subject: [PATCH 2/3] Allow `update_with_email` to use new reconfirmation flow --- .../mailer/confirmation_instructions.html.erb | 2 +- .../model/with_email_updater.rb | 34 +++- .../model/with_email_updater_spec.rb | 165 ++++++++++-------- 3 files changed, 123 insertions(+), 78 deletions(-) diff --git a/app/views/graphql_devise/mailer/confirmation_instructions.html.erb b/app/views/graphql_devise/mailer/confirmation_instructions.html.erb index 6c323d84..88f12751 100644 --- a/app/views/graphql_devise/mailer/confirmation_instructions.html.erb +++ b/app/views/graphql_devise/mailer/confirmation_instructions.html.erb @@ -6,6 +6,6 @@ <% if message['schema_url'].present? %> <%= link_to t('.confirm_account_link'), "#{message['schema_url']}?#{confirmation_query(resource_name: @resource.class.to_s, redirect_url: message['redirect-url'], token: @token).to_query}" %> <% else %> - <%= link_to t('.confirm_account_link'), "#{message['redirect-url'].to_s}?#{{ confirmationToken: @token }.to_query}" %> + <%= link_to t('.confirm_account_link'), "#{CGI.escape(message['redirect-url'].to_s)}?#{{ confirmationToken: @token }.to_query}" %> <% end %>

diff --git a/lib/graphql_devise/model/with_email_updater.rb b/lib/graphql_devise/model/with_email_updater.rb index eaf54151..d5ec262d 100644 --- a/lib/graphql_devise/model/with_email_updater.rb +++ b/lib/graphql_devise/model/with_email_updater.rb @@ -9,7 +9,7 @@ def initialize(resource, attributes) end def call - resource_attributes = @attributes.except(:schema_url, :confirmation_success_url) + resource_attributes = @attributes.except(:schema_url, :confirmation_success_url, :confirmation_url) return @resource.update(resource_attributes) unless requires_reconfirmation?(resource_attributes) @resource.assign_attributes(resource_attributes) @@ -27,7 +27,7 @@ def call else raise( GraphqlDevise::Error, - 'Method `update_with_email` requires attributes `confirmation_success_url` and `schema_url` for email reconfirmation to work' + 'Method `update_with_email` requires attribute `confirmation_url` for email reconfirmation to work' ) end end @@ -35,8 +35,19 @@ def call private def required_reconfirm_attributes? - @attributes[:schema_url].present? && - (@attributes[:confirmation_success_url].present? || DeviseTokenAuth.default_confirm_success_url.present?) + if @attributes[:schema_url].present? + ActiveSupport::Deprecation.warn(<<-DEPRECATION.strip_heredoc, caller) + Providing `schema_url` and `confirmation_success_url` to `update_with_email` is deprecated and will be + removed in a future version of this gem. + + Now you must only provide `confirmation_url` and the email will contain the new format of the confirmation + url that needs to be used with the new `confirmAccountWithToken` on the client application. + DEPRECATION + + [@attributes[:confirmation_success_url], DeviseTokenAuth.default_confirm_success_url].any?(&:present?) + else + [@attributes[:confirmation_url], DeviseTokenAuth.default_confirm_success_url].any?(&:present?) + end end def requires_reconfirmation?(resource_attributes) @@ -60,13 +71,22 @@ def email_in_database end end + def confirmation_method_params + if @attributes[:schema_url].present? + { + redirect_url: @attributes[:confirmation_success_url] || DeviseTokenAuth.default_confirm_success_url, + schema_url: @attributes[:schema_url] + } + else + { redirect_url: @attributes[:confirmation_url] || DeviseTokenAuth.default_confirm_success_url } + end + end + def send_confirmation_instructions(saved) return unless saved @resource.send_confirmation_instructions( - redirect_url: @attributes[:confirmation_success_url] || DeviseTokenAuth.default_confirm_success_url, - template_path: ['graphql_devise/mailer'], - schema_url: @attributes[:schema_url] + confirmation_method_params.merge(template_path: ['graphql_devise/mailer']) ) end end diff --git a/spec/graphql_devise/model/with_email_updater_spec.rb b/spec/graphql_devise/model/with_email_updater_spec.rb index ca7182c7..4ec8d3db 100644 --- a/spec/graphql_devise/model/with_email_updater_spec.rb +++ b/spec/graphql_devise/model/with_email_updater_spec.rb @@ -4,6 +4,57 @@ RSpec.describe GraphqlDevise::Model::WithEmailUpdater do describe '#call' do + shared_examples 'all required arguments are provided' do |base_attributes| + let(:attributes) { base_attributes.merge(email: 'new@gmail.com', name: 'Updated Name') } + + it 'postpones email update' do + expect do + updater + resource.reload + end.to not_change(resource, :email).from(resource.email).and( + not_change(resource, :uid).from(resource.uid) + ).and( + change(resource, :unconfirmed_email).from(nil).to('new@gmail.com') + ).and( + change(resource, :name).from(resource.name).to('Updated Name') + ) + end + + it 'sends out a confirmation email to the unconfirmed_email' do + expect { updater }.to change(ActionMailer::Base.deliveries, :count).by(1) + + email = ActionMailer::Base.deliveries.first + expect(email.to).to contain_exactly('new@gmail.com') + end + + context 'when email value is the same on the DB' do + let(:attributes) { base_attributes.merge(email: resource.email, name: 'changed') } + + it 'updates attributes and does not send confirmation email' do + expect do + updater + resource.reload + end.to change(resource, :name).from(resource.name).to('changed').and( + not_change(resource, :email).from(resource.email) + ).and( + not_change(ActionMailer::Base.deliveries, :count).from(0) + ) + end + end + + context 'when provided params are invalid' do + let(:attributes) { base_attributes.merge(email: 'newgmail.com', name: '') } + + it 'returns false and adds errors to the model' do + expect(updater).to be_falsey + expect(resource.errors.full_messages).to contain_exactly( + 'Email is not an email', + "Name can't be blank" + ) + end + end + end + subject(:updater) { described_class.new(resource, attributes).call } context 'when the model does not have an unconfirmed_email column' do @@ -38,90 +89,64 @@ end context 'when attributes contain email' do - context 'when schema_url is missing' do - let(:attributes) { { email: 'new@gmail.com', name: 'Updated Name' } } - - it 'raises an error' do - expect { updater }.to raise_error( - GraphqlDevise::Error, - 'Method `update_with_email` requires attributes `confirmation_success_url` and `schema_url` for email reconfirmation to work' - ) - end + context 'when confirmation_success_url is used' do + it_behaves_like 'all required arguments are provided', schema_url: 'http://localhost/test', confirmation_success_url: 'https://google.com' - context 'when email will not change' do - let(:attributes) { { email: resource.email, name: 'changed' } } - - it 'updates name and does not raise an error' do - expect do - updater - resource.reload - end.to change(resource, :name).from(resource.name).to('changed').and( - not_change(resource, :email).from(resource.email) - ).and( - not_change(ActionMailer::Base.deliveries, :count).from(0) - ) - end - end - end + context 'when confirmation_success_url is missing and no default is set' do + let(:attributes) { { email: 'new@gmail.com', name: 'Updated Name', schema_url: 'http://localhost/test' } } - context 'when only confirmation_success_url is missing' do - let(:attributes) { { email: 'new@gmail.com', name: 'Updated Name', schema_url: 'http://localhost/test' } } + before { allow(DeviseTokenAuth).to receive(:default_confirm_success_url).and_return(nil) } - it 'uses DTA default_confirm_success_url on the email' do - expect { updater }.to change(ActionMailer::Base.deliveries, :count).by(1) + it 'raises an error' do + expect { updater }.to raise_error( + GraphqlDevise::Error, + 'Method `update_with_email` requires attribute `confirmation_url` for email reconfirmation to work' + ) + end - email = ActionMailer::Base.deliveries.first - expect(email.body.decoded).to include(CGI.escape('https://google.com')) + context 'when email will not change' do + let(:attributes) { { email: resource.email, name: 'changed', confirmation_success_url: 'https://google.com' } } + + it 'updates name and does not raise an error' do + expect do + updater + resource.reload + end.to change(resource, :name).from(resource.name).to('changed').and( + not_change(resource, :email).from(resource.email) + ).and( + not_change(ActionMailer::Base.deliveries, :count).from(0) + ) + end + end end end - context 'when both required urls are provided' do - let(:attributes) { { email: 'new@gmail.com', name: 'Updated Name', schema_url: 'http://localhost/test', confirmation_success_url: 'https://google.com' } } - - it 'postpones email update' do - expect do - updater - resource.reload - end.to not_change(resource, :email).from(resource.email).and( - not_change(resource, :uid).from(resource.uid) - ).and( - change(resource, :unconfirmed_email).from(nil).to('new@gmail.com') - ).and( - change(resource, :name).from(resource.name).to('Updated Name') - ) - end + context 'when confirm_url is used' do + it_behaves_like 'all required arguments are provided', confirmation_url: 'https://google.com' + end - it 'sends out a confirmation email to the unconfirmed_email' do - expect { updater }.to change(ActionMailer::Base.deliveries, :count).by(1) + context 'when no confirmation url is provided is provided' do + context 'when schema_url is provided' do + let(:attributes) { { email: 'new@gmail.com', name: 'Updated Name', schema_url: 'http://localhost/test' } } - email = ActionMailer::Base.deliveries.first - expect(email.to).to contain_exactly('new@gmail.com') - end + it 'uses DTA default_confirm_success_url on the email with redirect flow' do + expect { updater }.to change(ActionMailer::Base.deliveries, :count).by(1) - context 'when email value is the same on the DB' do - let(:attributes) { { email: resource.email, name: 'changed', schema_url: 'http://localhost/test', confirmation_success_url: 'https://google.com' } } - - it 'updates attributes and does not send confirmation email' do - expect do - updater - resource.reload - end.to change(resource, :name).from(resource.name).to('changed').and( - not_change(resource, :email).from(resource.email) - ).and( - not_change(ActionMailer::Base.deliveries, :count).from(0) - ) + email = ActionMailer::Base.deliveries.first + expect(email.body.decoded).to include(CGI.escape('https://google.com')) + expect(email.body.decoded).to include(CGI.escape('ConfirmAccount(')) end end - context 'when provided params are invalid' do - let(:attributes) { { email: 'newgmail.com', name: '', schema_url: 'http://localhost/test', confirmation_success_url: 'https://google.com' } } + context 'when schema_url is not provided' do + let(:attributes) { { email: 'new@gmail.com', name: 'Updated Name' } } - it 'returns false and adds errors to the model' do - expect(updater).to be_falsey - expect(resource.errors.full_messages).to contain_exactly( - 'Email is not an email', - "Name can't be blank" - ) + it 'uses DTA default_confirm_success_url on the email and new confirmation flow' do + expect { updater }.to change(ActionMailer::Base.deliveries, :count).by(1) + + email = ActionMailer::Base.deliveries.first + expect(email.body.decoded).to include(CGI.escape('https://google.com')) + expect(email.body.decoded).to include('?confirmationToken=') end end end From 337fa0bbcd7adfd7a565eaf22e2d5cbe385d0a92 Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Sun, 6 Jun 2021 16:00:13 -0500 Subject: [PATCH 3/3] Rename confirm_registration_with_token --- .../default_operations/mutations.rb | 22 +++++++++---------- .../model/with_email_updater.rb | 14 ++++++++---- ....rb => confirm_registration_with_token.rb} | 2 +- .../model/with_email_updater_spec.rb | 4 ++++ ...> confirm_registration_with_token_spec.rb} | 8 +++---- 5 files changed, 30 insertions(+), 20 deletions(-) rename lib/graphql_devise/mutations/{confirm_account_with_token.rb => confirm_registration_with_token.rb} (94%) rename spec/requests/mutations/{confirm_account_with_token_spec.rb => confirm_registration_with_token_spec.rb} (92%) diff --git a/lib/graphql_devise/default_operations/mutations.rb b/lib/graphql_devise/default_operations/mutations.rb index 43d335f7..4eb3281c 100644 --- a/lib/graphql_devise/default_operations/mutations.rb +++ b/lib/graphql_devise/default_operations/mutations.rb @@ -10,21 +10,21 @@ require 'graphql_devise/mutations/register' require 'graphql_devise/mutations/update_password' require 'graphql_devise/mutations/update_password_with_token' -require 'graphql_devise/mutations/confirm_account_with_token' +require 'graphql_devise/mutations/confirm_registration_with_token' module GraphqlDevise module DefaultOperations MUTATIONS = { - login: { klass: GraphqlDevise::Mutations::Login, authenticatable: true }, - logout: { klass: GraphqlDevise::Mutations::Logout, authenticatable: true }, - sign_up: { klass: GraphqlDevise::Mutations::SignUp, authenticatable: true }, - register: { klass: GraphqlDevise::Mutations::Register, authenticatable: true }, - update_password: { klass: GraphqlDevise::Mutations::UpdatePassword, authenticatable: true }, - update_password_with_token: { klass: GraphqlDevise::Mutations::UpdatePasswordWithToken, authenticatable: true }, - send_password_reset: { klass: GraphqlDevise::Mutations::SendPasswordReset, authenticatable: false }, - send_password_reset_with_token: { klass: GraphqlDevise::Mutations::SendPasswordResetWithToken, authenticatable: false }, - resend_confirmation: { klass: GraphqlDevise::Mutations::ResendConfirmation, authenticatable: false }, - confirm_account_with_token: { klass: GraphqlDevise::Mutations::ConfirmAccountWithToken, authenticatable: true } + login: { klass: GraphqlDevise::Mutations::Login, authenticatable: true }, + logout: { klass: GraphqlDevise::Mutations::Logout, authenticatable: true }, + sign_up: { klass: GraphqlDevise::Mutations::SignUp, authenticatable: true }, + register: { klass: GraphqlDevise::Mutations::Register, authenticatable: true }, + update_password: { klass: GraphqlDevise::Mutations::UpdatePassword, authenticatable: true }, + update_password_with_token: { klass: GraphqlDevise::Mutations::UpdatePasswordWithToken, authenticatable: true }, + send_password_reset: { klass: GraphqlDevise::Mutations::SendPasswordReset, authenticatable: false }, + send_password_reset_with_token: { klass: GraphqlDevise::Mutations::SendPasswordResetWithToken, authenticatable: false }, + resend_confirmation: { klass: GraphqlDevise::Mutations::ResendConfirmation, authenticatable: false }, + confirm_registration_with_token: { klass: GraphqlDevise::Mutations::ConfirmRegistrationWithToken, authenticatable: true } }.freeze end end diff --git a/lib/graphql_devise/model/with_email_updater.rb b/lib/graphql_devise/model/with_email_updater.rb index d5ec262d..cc93d648 100644 --- a/lib/graphql_devise/model/with_email_updater.rb +++ b/lib/graphql_devise/model/with_email_updater.rb @@ -4,11 +4,13 @@ module GraphqlDevise module Model class WithEmailUpdater def initialize(resource, attributes) - @attributes = attributes + @attributes = attributes.with_indifferent_access @resource = resource end def call + check_deprecated_attributes + resource_attributes = @attributes.except(:schema_url, :confirmation_success_url, :confirmation_url) return @resource.update(resource_attributes) unless requires_reconfirmation?(resource_attributes) @@ -34,16 +36,20 @@ def call private - def required_reconfirm_attributes? - if @attributes[:schema_url].present? + def check_deprecated_attributes + if [@attributes[:schema_url], @attributes[:confirmation_success_url]].any?(&:present?) ActiveSupport::Deprecation.warn(<<-DEPRECATION.strip_heredoc, caller) Providing `schema_url` and `confirmation_success_url` to `update_with_email` is deprecated and will be removed in a future version of this gem. Now you must only provide `confirmation_url` and the email will contain the new format of the confirmation - url that needs to be used with the new `confirmAccountWithToken` on the client application. + url that needs to be used with the new `confirmRegistrationWithToken` on the client application. DEPRECATION + end + end + def required_reconfirm_attributes? + if @attributes[:schema_url].present? [@attributes[:confirmation_success_url], DeviseTokenAuth.default_confirm_success_url].any?(&:present?) else [@attributes[:confirmation_url], DeviseTokenAuth.default_confirm_success_url].any?(&:present?) diff --git a/lib/graphql_devise/mutations/confirm_account_with_token.rb b/lib/graphql_devise/mutations/confirm_registration_with_token.rb similarity index 94% rename from lib/graphql_devise/mutations/confirm_account_with_token.rb rename to lib/graphql_devise/mutations/confirm_registration_with_token.rb index eae5dadb..1d1ca0f8 100644 --- a/lib/graphql_devise/mutations/confirm_account_with_token.rb +++ b/lib/graphql_devise/mutations/confirm_registration_with_token.rb @@ -2,7 +2,7 @@ module GraphqlDevise module Mutations - class ConfirmAccountWithToken < Base + class ConfirmRegistrationWithToken < Base argument :confirmation_token, String, required: true field :credentials, diff --git a/spec/graphql_devise/model/with_email_updater_spec.rb b/spec/graphql_devise/model/with_email_updater_spec.rb index 4ec8d3db..6f65cf45 100644 --- a/spec/graphql_devise/model/with_email_updater_spec.rb +++ b/spec/graphql_devise/model/with_email_updater_spec.rb @@ -123,6 +123,10 @@ context 'when confirm_url is used' do it_behaves_like 'all required arguments are provided', confirmation_url: 'https://google.com' + + context 'when arguments hash has strings as keys' do + it_behaves_like 'all required arguments are provided', 'confirmation_url' => 'https://google.com' + end end context 'when no confirmation url is provided is provided' do diff --git a/spec/requests/mutations/confirm_account_with_token_spec.rb b/spec/requests/mutations/confirm_registration_with_token_spec.rb similarity index 92% rename from spec/requests/mutations/confirm_account_with_token_spec.rb rename to spec/requests/mutations/confirm_registration_with_token_spec.rb index 2bbf3b69..ec2feb40 100644 --- a/spec/requests/mutations/confirm_account_with_token_spec.rb +++ b/spec/requests/mutations/confirm_registration_with_token_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe 'Account confirmation with token' do +RSpec.describe 'Registration confirmation with token' do include_context 'with graphql query request' context 'when using the user model' do @@ -10,7 +10,7 @@ let(:query) do <<-GRAPHQL mutation { - userConfirmAccountWithToken( + userConfirmRegistrationWithToken( confirmationToken: "#{token}" ) { authenticatable { @@ -38,7 +38,7 @@ user.reload end.to(change(user, :confirmed_at).from(nil)) - expect(json_response[:data][:userConfirmAccountWithToken]).to include( + expect(json_response[:data][:userConfirmRegistrationWithToken]).to include( authenticatable: { email: user.email, name: user.name }, credentials: { client: user.tokens.keys.first } ) @@ -84,7 +84,7 @@ let(:query) do <<-GRAPHQL mutation { - adminConfirmAccountWithToken( + adminConfirmRegistrationWithToken( confirmationToken: "#{token}" ) { authenticatable { email }