From 1bb5be68a5d5c1e56636b7ed4439b6a8c3342544 Mon Sep 17 00:00:00 2001 From: John Ma Date: Wed, 17 Oct 2018 11:42:50 -0400 Subject: [PATCH] Fix for email verification issue (GRN-36) (#300) * * * <> * Delete env * Update development.rb --- app/controllers/users_controller.rb | 22 +++++++++++++++---- config/environments/development.rb | 4 ++-- config/environments/production.rb | 16 ++++++++++++++ config/locales/en.yml | 1 + sample.env | 2 +- spec/controllers/users_controller_spec.rb | 26 +++++++++++++++++++++++ 6 files changed, 64 insertions(+), 7 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f3f29e3f..50f9e7ca 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -29,8 +29,13 @@ class UsersController < ApplicationController @user.provider = "greenlight" if Rails.configuration.enable_email_verification && @user.save - UserMailer.verify_email(@user, verification_link(@user)).deliver - login(@user) + begin + UserMailer.verify_email(@user, verification_link(@user)).deliver + login(@user) + rescue => e + logger.error "Error in email delivery: #{e}" + mailer_delivery_fail + end elsif @user.save login(@user) else @@ -134,8 +139,13 @@ class UsersController < ApplicationController elsif current_user.email_verified login(current_user) elsif params[:email_verified] == "false" - UserMailer.verify_email(current_user, verification_link(current_user)).deliver - render 'verify' + begin + UserMailer.verify_email(current_user, verification_link(current_user)).deliver + render 'verify' + rescue => e + logger.error "Error in email delivery: #{e}" + mailer_delivery_fail + end else render 'verify' end @@ -143,6 +153,10 @@ class UsersController < ApplicationController private + def mailer_delivery_fail + redirect_to root_path, notice: I18n.t(params[:message], default: I18n.t("delivery_error")) + end + def verification_link(user) request.base_url + confirm_path(user.uid) end diff --git a/config/environments/development.rb b/config/environments/development.rb index 672afa10..b3178310 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -41,8 +41,8 @@ Rails.application.configure do enable_starttls_auto: ENV['SMTP_STARTTLS_AUTO'], } - # Don't care if the mailer can't send. - config.action_mailer.raise_delivery_errors = false + # Do care if the mailer can't send. + config.action_mailer.raise_delivery_errors = true config.action_mailer.perform_caching = false diff --git a/config/environments/production.rb b/config/environments/production.rb index f3b7cc2c..94228a23 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -54,6 +54,22 @@ Rails.application.configure do # Use a different cache store in production. # config.cache_store = :mem_cache_store + # Tell Action Mailer to use smtp server + config.action_mailer.delivery_method = :smtp + + ActionMailer::Base.smtp_settings = { + address: ENV['SMTP_SERVER'], + port: ENV["SMTP_PORT"], + domain: ENV['SMTP_DOMAIN'], + user_name: ENV['SMTP_USERNAME'], + password: ENV['SMTP_PASSWORD'], + authentication: ENV['SMTP_AUTH'], + enable_starttls_auto: ENV['SMTP_STARTTLS_AUTO'], + } + + # Don't care if the mailer can't send. + config.action_mailer.raise_delivery_errors = true + # Use a real queuing backend for Active Job (and separate queues per environment) # config.active_job.queue_adapter = :resque # config.active_job.queue_name_prefix = "greenlight-2_0_#{Rails.env}" diff --git a/config/locales/en.yml b/config/locales/en.yml index b951d9f6..b18f3eea 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -25,6 +25,7 @@ en: cancel: Cancel copy: Copy delete: Delete + delivery_error: An error occured during email delivery. Please contact an administrator! docs: Documentation email: Email enter_your_name: Enter your name! diff --git a/sample.env b/sample.env index 5c0b9af2..0337caee 100644 --- a/sample.env +++ b/sample.env @@ -73,7 +73,7 @@ ALLOW_GREENLIGHT_ACCOUNTS=true # Set this to true if you want GreenLight to send verification emails upon # the creation of a new account # -# SMTP variables can be taken from the list in the following table: +# SMTP variables can be configured by following the template: # # ALLOW_MAIL_NOTIFICATIONS=true # SMTP_SERVER=smtp.gmail.com diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index ad06b32e..8814872e 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -101,6 +101,19 @@ describe UsersController, type: :controller do end end + context "allow email verification" do + before { allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) } + + it "should raise if there there is a delivery failure" do + params = random_valid_user_params + + expect do + post :create, params: params + raise :anyerror + end.to raise_error { :anyerror } + end + end + it "redirects to main room if already authenticated" do user = create(:user) @request.session[:user_id] = user.id @@ -155,6 +168,19 @@ describe UsersController, type: :controller do expect { post :resend, params: { email_verified: false } }.to change { ActionMailer::Base.deliveries.count }.by(1) expect(response).to render_template(:verify) end + + it "should raise if there there is a delivery failure" do + params = random_valid_user_params + post :create, params: params + + u = User.find_by(name: params[:user][:name], email: params[:user][:email]) + u.email_verified = false + + expect do + post :resend, params: { email_verified: false } + raise Net::SMTPAuthenticationError + end.to raise_error { Net::SMTPAuthenticationError } + end end describe "GET | POST #confirm" do