From 28ea44feb528d7bcdaf3bd62ac81e291a4a54dfb Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Sat, 20 Jun 2020 21:02:09 -0500 Subject: [PATCH] Add update_with_email method. Enable email reconfirmation. --- .../concerns/set_user_by_token.rb | 4 + app/models/graphql_devise/concerns/model.rb | 10 +++ .../mailer/confirmation_instructions.html.erb | 2 +- .../reset_password_instructions.html.erb | 2 +- .../model/with_email_updater.rb | 49 +++++++++++ .../mutations/resend_confirmation.rb | 2 +- .../mutations/send_password_reset.rb | 2 +- lib/graphql_devise/mutations/sign_up.rb | 2 +- .../resolvers/confirm_account.rb | 4 +- .../app/graphql/mutations/update_user.rb | 20 +++++ spec/dummy/app/graphql/types/mutation_type.rb | 1 + .../config/initializers/devise_token_auth.rb | 2 + ...414_remove_uncofirmed_email_from_admins.rb | 5 ++ spec/dummy/db/schema.rb | 3 +- .../model/with_email_updater_spec.rb | 87 +++++++++++++++++++ spec/requests/queries/confirm_account_spec.rb | 2 +- spec/requests/user_controller_spec.rb | 39 +++++++++ 17 files changed, 226 insertions(+), 10 deletions(-) create mode 100644 lib/graphql_devise/model/with_email_updater.rb create mode 100644 spec/dummy/app/graphql/mutations/update_user.rb create mode 100644 spec/dummy/db/migrate/20200621182414_remove_uncofirmed_email_from_admins.rb create mode 100644 spec/graphql_devise/model/with_email_updater_spec.rb diff --git a/app/controllers/graphql_devise/concerns/set_user_by_token.rb b/app/controllers/graphql_devise/concerns/set_user_by_token.rb index e0718177..58ade4b7 100644 --- a/app/controllers/graphql_devise/concerns/set_user_by_token.rb +++ b/app/controllers/graphql_devise/concerns/set_user_by_token.rb @@ -5,6 +5,10 @@ module Concerns SetUserByToken.module_eval do attr_accessor :client_id, :token, :resource + def full_url_without_params + request.base_url + request.path + end + def set_resource_by_token(resource) set_user_by_token(resource) end diff --git a/app/models/graphql_devise/concerns/model.rb b/app/models/graphql_devise/concerns/model.rb index 5d434c99..3433aa6f 100644 --- a/app/models/graphql_devise/concerns/model.rb +++ b/app/models/graphql_devise/concerns/model.rb @@ -1,5 +1,15 @@ +require 'graphql_devise/model/with_email_updater' + module GraphqlDevise module Concerns Model = DeviseTokenAuth::Concerns::User + + Model.module_eval do + attr_accessor :confirmation_url, :confirmation_success_url + + def update_with_email(attributes = {}) + GraphqlDevise::Model::WithEmailUpdater.new(self, attributes).call + end + end end end diff --git a/app/views/graphql_devise/mailer/confirmation_instructions.html.erb b/app/views/graphql_devise/mailer/confirmation_instructions.html.erb index 1e4f3435..a5a214f5 100644 --- a/app/views/graphql_devise/mailer/confirmation_instructions.html.erb +++ b/app/views/graphql_devise/mailer/confirmation_instructions.html.erb @@ -2,4 +2,4 @@

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

-

<%= link_to t('.confirm_account_link'), url_for(controller: message['controller'], action: message['action'], **confirmation_query(resource_name: @resource.class.to_s, redirect_url: message['redirect-url'], token: @token)) %>

+

<%= 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}" %>

diff --git a/app/views/graphql_devise/mailer/reset_password_instructions.html.erb b/app/views/graphql_devise/mailer/reset_password_instructions.html.erb index 853ce976..a9b63721 100644 --- a/app/views/graphql_devise/mailer/reset_password_instructions.html.erb +++ b/app/views/graphql_devise/mailer/reset_password_instructions.html.erb @@ -2,7 +2,7 @@

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

-

<%= link_to t('.password_change_link'), url_for(controller: message['controller'], action: message['action'], **password_reset_query(token: @token, redirect_url: message['redirect-url'], resource_name: @resource.class.to_s)) %>

+

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

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

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

diff --git a/lib/graphql_devise/model/with_email_updater.rb b/lib/graphql_devise/model/with_email_updater.rb new file mode 100644 index 00000000..19fa3926 --- /dev/null +++ b/lib/graphql_devise/model/with_email_updater.rb @@ -0,0 +1,49 @@ +module GraphqlDevise + module Model + class WithEmailUpdater + def initialize(resource, attributes) + @attributes = attributes + @resource = resource + end + + def call + resource_attributes = @attributes.except(:schema_url, :confirmation_success_url) + + if resource_attributes.key?(:email) && @resource.respond_to?(:unconfirmed_email=) + unless @attributes[:schema_url].present? && (@attributes[:confirmation_success_url].present? || DeviseTokenAuth.default_confirm_success_url.present?) + raise( + GraphqlDevise::Error, + 'Method `update_with_email` requires attributes `confirmation_success_url` and `schema_url` for email reconfirmation to work' + ) + end + + @resource.assign_attributes(resource_attributes) + return false unless @resource.valid? + + @resource.unconfirmed_email = @resource.email + @resource.confirmation_token = nil + @resource.email = if Devise.activerecord51? + @resource.email_in_database + else + @resource.email_was + end + @resource.send(:generate_confirmation_token) + + saved = @resource.save + + if 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] + ) + end + + saved + else + @resource.update(resource_attributes) + end + end + end + end +end diff --git a/lib/graphql_devise/mutations/resend_confirmation.rb b/lib/graphql_devise/mutations/resend_confirmation.rb index 5ef5132d..c8fe9b1b 100644 --- a/lib/graphql_devise/mutations/resend_confirmation.rb +++ b/lib/graphql_devise/mutations/resend_confirmation.rb @@ -20,7 +20,7 @@ def resolve(email:, redirect_url:) resource.send_confirmation_instructions( redirect_url: redirect_url, template_path: ['graphql_devise/mailer'], - **controller.params.permit('controller', 'action').to_h.symbolize_keys + schema_url: controller.full_url_without_params ) { message: I18n.t('graphql_devise.confirmations.send_instructions', email: email) } diff --git a/lib/graphql_devise/mutations/send_password_reset.rb b/lib/graphql_devise/mutations/send_password_reset.rb index c72c22a5..8ac3d69a 100644 --- a/lib/graphql_devise/mutations/send_password_reset.rb +++ b/lib/graphql_devise/mutations/send_password_reset.rb @@ -17,7 +17,7 @@ def resolve(email:, redirect_url:) provider: 'email', redirect_url: redirect_url, template_path: ['graphql_devise/mailer'], - **controller.params.permit('controller', 'action').to_h.symbolize_keys + schema_url: controller.full_url_without_params ) if resource.errors.empty? diff --git a/lib/graphql_devise/mutations/sign_up.rb b/lib/graphql_devise/mutations/sign_up.rb index a184ca4c..9ed1bdad 100644 --- a/lib/graphql_devise/mutations/sign_up.rb +++ b/lib/graphql_devise/mutations/sign_up.rb @@ -28,7 +28,7 @@ def resolve(confirm_success_url: nil, **attrs) resource.send_confirmation_instructions( redirect_url: confirm_success_url, template_path: ['graphql_devise/mailer'], - **controller.params.permit('controller', 'action').to_h.symbolize_keys + schema_url: controller.full_url_without_params ) end diff --git a/lib/graphql_devise/resolvers/confirm_account.rb b/lib/graphql_devise/resolvers/confirm_account.rb index 489f8be9..ef1086ff 100644 --- a/lib/graphql_devise/resolvers/confirm_account.rb +++ b/lib/graphql_devise/resolvers/confirm_account.rb @@ -13,10 +13,10 @@ def resolve(confirmation_token:, redirect_url:) redirect_header_options = { account_confirmation_success: true } redirect_to_link = if controller.signed_in?(resource_name) - signed_in_resource.build_auth_url( + resource.build_auth_url( redirect_url, redirect_headers( - client_and_token(controller.signed_in_resource.create_token), + client_and_token(resource.create_token), redirect_header_options ) ) diff --git a/spec/dummy/app/graphql/mutations/update_user.rb b/spec/dummy/app/graphql/mutations/update_user.rb new file mode 100644 index 00000000..55189635 --- /dev/null +++ b/spec/dummy/app/graphql/mutations/update_user.rb @@ -0,0 +1,20 @@ +module Mutations + class UpdateUser < GraphQL::Schema::Mutation + field :user, Types::UserType, null: false + + argument :email, String, required: false + argument :name, String, required: false + + def resolve(**attrs) + user = context[:current_resource] + + schema_url = context[:controller].full_url_without_params + + user.update_with_email( + attrs.merge(schema_url: schema_url, confirmation_success_url: 'https://google.com') + ) + + { user: user } + end + end +end diff --git a/spec/dummy/app/graphql/types/mutation_type.rb b/spec/dummy/app/graphql/types/mutation_type.rb index 5948ce61..7b7878b5 100644 --- a/spec/dummy/app/graphql/types/mutation_type.rb +++ b/spec/dummy/app/graphql/types/mutation_type.rb @@ -1,6 +1,7 @@ module Types class MutationType < Types::BaseObject field :dummy_mutation, String, null: false, authenticate: true + field :update_user, mutation: Mutations::UpdateUser def dummy_mutation 'Necessary so GraphQL gem does not complain about empty mutation type' diff --git a/spec/dummy/config/initializers/devise_token_auth.rb b/spec/dummy/config/initializers/devise_token_auth.rb index 731a7785..30d9c694 100644 --- a/spec/dummy/config/initializers/devise_token_auth.rb +++ b/spec/dummy/config/initializers/devise_token_auth.rb @@ -37,6 +37,8 @@ # password is updated. config.check_current_password_before_update = :password + config.default_confirm_success_url = '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 diff --git a/spec/dummy/db/migrate/20200621182414_remove_uncofirmed_email_from_admins.rb b/spec/dummy/db/migrate/20200621182414_remove_uncofirmed_email_from_admins.rb new file mode 100644 index 00000000..1ec9519a --- /dev/null +++ b/spec/dummy/db/migrate/20200621182414_remove_uncofirmed_email_from_admins.rb @@ -0,0 +1,5 @@ +class RemoveUncofirmedEmailFromAdmins < ActiveRecord::Migration[6.0] + def change + remove_column :admins, :unconfirmed_email, :string + end +end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index f11049b8..78ffe12e 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_03_21_121807) do +ActiveRecord::Schema.define(version: 2020_06_21_182414) do create_table "admins", force: :cascade do |t| t.string "provider", default: "email", null: false @@ -22,7 +22,6 @@ t.string "confirmation_token" t.datetime "confirmed_at" t.datetime "confirmation_sent_at" - t.string "unconfirmed_email" t.string "email" t.text "tokens" t.datetime "created_at", null: false diff --git a/spec/graphql_devise/model/with_email_updater_spec.rb b/spec/graphql_devise/model/with_email_updater_spec.rb new file mode 100644 index 00000000..81aac614 --- /dev/null +++ b/spec/graphql_devise/model/with_email_updater_spec.rb @@ -0,0 +1,87 @@ +require 'rails_helper' + +RSpec.describe GraphqlDevise::Model::WithEmailUpdater do + describe '#call' do + subject(:updater) { described_class.new(resource, attributes).call } + + context 'when the model does not have an unconfirmed_email column' do + let(:resource) { create(:admin, :confirmed) } + + context 'when attributes contain email' do + let(:attributes) { { email: 'new@gmail.com', schema_url: 'http://localhost/test', confirmation_success_url: 'https://google.com' } } + + it 'does not postpone email update' do + expect do + updater + resource.reload + end.to change(resource, :email).from(resource.email).to('new@gmail.com').and( + change(resource, :uid).from(resource.uid).to('new@gmail.com') + ) + end + end + end + + context 'when the model has an unconfirmed_email column' do + let(:resource) { create(:user, :confirmed) } + + context 'when attributes do not contain email' do + let(:attributes) { { name: 'Updated Name', schema_url: 'http://localhost/test', confirmation_success_url: 'https://google.com' } } + + it 'updates resource, ignores url params' do + expect do + updater + resource.reload + end.to change(resource, :name).from(resource.name).to('Updated Name') + end + 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 + end + + context 'when only confirmation_success_url is missing' do + let(:attributes) { { email: 'new@gmail.com', name: 'Updated Name', schema_url: 'http://localhost/test' } } + + it 'uses DTA default_confirm_success_url on the email' 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')) + 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 + + 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 + end + end + end + end +end diff --git a/spec/requests/queries/confirm_account_spec.rb b/spec/requests/queries/confirm_account_spec.rb index 7f73787e..8dd17b57 100644 --- a/spec/requests/queries/confirm_account_spec.rb +++ b/spec/requests/queries/confirm_account_spec.rb @@ -26,7 +26,7 @@ user.send_confirmation_instructions( template_path: ['graphql_devise/mailer'], controller: 'graphql_devise/graphql', - action: 'auth' + schema_url: 'http://not-using-this-value.com/gql' ) end diff --git a/spec/requests/user_controller_spec.rb b/spec/requests/user_controller_spec.rb index 49089155..93b332f5 100644 --- a/spec/requests/user_controller_spec.rb +++ b/spec/requests/user_controller_spec.rb @@ -202,4 +202,43 @@ end end end + + describe 'updateUser' do + let(:headers) { user.create_new_auth_token } + let(:query) do + <<-GRAPHQL + mutation { + updateUser(email: "updated@gmail.com", name: "updated name") { + user { email name } + } + } + GRAPHQL + end + + it 'requires new email confirmation' do + original_email = user.email + + expect do + post_request('/api/v1/graphql?test=value') + user.reload + end.to not_change(user, :email).from(original_email).and( + change(user, :unconfirmed_email).from(nil).to('updated@gmail.com') + ).and( + not_change(user, :uid).from(original_email) + ).and( + change(user, :name).from(user.name).to('updated name') + ) + + email = Nokogiri::HTML(ActionMailer::Base.deliveries.last.body.encoded) + link = email.css('a').first + expect(link['href']).to include('/api/v1/graphql') + + expect do + get link['href'] + user.reload + end.to change(user, :email).from(original_email).to('updated@gmail.com').and( + change(user, :uid).from(original_email).to('updated@gmail.com') + ) + end + end end