Devise / ActionMailer sending duplicate emails for registration confirmation

Question

My rails application uses devise to handle registration, authentication, etc. I'm using the confirmable module. The bug is this– when a user registers with email, Devise is sending two confirmation emails with different confirmation links. One link works, the other directs the user to an error page.

Devise spits out a message associated with the error: "Confirmation token is invalid" and takes the user to the Resend Confirmation Email page.

I'm hosting with heroku and using sendgrid to send the emails. update: The bug also occurs on localhost.

I have no idea where the root of this bug is, and this might be more code than what you need to see:


models/user.rb

...

devise :database_authenticatable, :registerable, :omniauthable,
     :recoverable, :rememberable, :trackable, :validatable, 
     :confirmable, :authentication_keys => [:login]

...

## callbacks
after_create :account_created

# called after the account is first created
def account_created

  # check if this activiy has already been created
  if !self.activities.where(:kind => "created_account").blank?
    puts "WARNING: user ##{self.id} already has a created account activity!"
    return
  end

  # update points
  self.points += 50
  self.save

  # create activity
  act = self.activities.new
  act.kind = "created_account"
  act.created_at = self.created_at
  act.save

end

...

def confirmation_required?
  super && (self.standard_account? || self.email_changed)
end

...

controllers/registrations_controller.rb

class RegistrationsController < Devise::RegistrationsController
  def update
    unless @user.last_sign_in_at.nil?

      puts "--------------double checking whether password confirmation is required--"
      ## if the user has not signed in yet, we don't want to do this.

      @user = User.find(current_user.id)
      # uncomment if you want to require password for email change
      email_changed = @user.email != params[:user][:email]
      password_changed = !params[:user][:password].empty?

      # uncomment if you want to require password for email change
      # successfully_updated = if email_changed or password_changed

      successfully_updated = if password_changed
        params[:user].delete(:current_password) if params[:user][:current_password].blank?
        @user.update_with_password(params[:user])
      else
        params[:user].delete(:current_password)
        @user.update_without_password(params[:user])
      end

      if successfully_updated
        # Sign in the user bypassing validation in case his password changed
        sign_in @user, :bypass => true
        if email_changed
          flash[:blue] = "Your account has been updated! Check your email to confirm your new address. Until then, your email will remain unchanged."
        else
          flash[:blue] = "Account info has been updated!"
        end
        redirect_to edit_user_registration_path
      else
        render "edit"
      end
    end
  end
end

controllers/omniauth_callbacks_controller

class OmniauthCallbacksController < Devise::OmniauthCallbacksController

  skip_before_filter :verify_authenticity_token

    def facebook
        user = User.from_omniauth(request.env["omniauth.auth"])
    if user.persisted?
      flash.notice = "Signed in!"

      # if the oauth_token is expired or nil, update it...
      if (DateTime.now > (user.oauth_expires_at || 99.years.ago) )
        user.update_oauth_token(request.env["omniauth.auth"])
      end

      sign_in_and_redirect user
    else
      session["devise.user_attributes"] = user.attributes
      redirect_to new_user_registration_url
    end
    end
end

config/routes.rb

...

devise_for :users, controllers: {omniauth_callbacks: "omniauth_callbacks", 
                                :registrations => "registrations"}

...

I'm happy to provide more information if needed. I'm also open to customizing/overriding the devise mailer behavior, but I don't know how to go about that.

Much thanks!

Answer 1

Solved!

I was able to override Devise::Mailer and force a stack trace to find out exactly what was causing duplicate emails. Devise::Mailer#confirmation_instructions was being called twice, and I found out that the problem was with my :after_create callback, shown below:

in models/user.rb...

after_create :account_created

# called after the account is first created
def account_created

...

  # update points
  self.points += 50
  self.save

...

end

Calling self.save somehow caused the mailer to be triggered again. I solved the problem by changing when the points are added. I got rid of the after_create call and overrode the confirm! method in devise to look like this:

def confirm!
  super
  account_created
end

So now the user record doesn't get modified (adding points) until after confirmation. No more duplicate emails!

Answer 2

I originally went with Thomas Klemm's answer but I went back to look at this when I had some spare time to try and figure out what was happening as it didn't feel right.

I tracked the 'problem' down and noticed that it only happens when :confirmable is set in your devise (User) model and reconfirmable is enabled in the devise initializer - which in hindsight makes a lot of sense because essentially in the after_create we ARE changing the User model, although we aren't changing the email address - I suspect Devise may do this because the account isn't confirmed yet, but in any case it is easy to stop the second email just by calling self.skip_reconfirmation! in the after_create method.

I created a sample rails project with a couple of tests just to ensure the correct behaviour. Below are the key excerpts. If you have far too much time on your hands, you can see the project here: https://github.com/richhollis/devise-reconfirmable-test

app/models/User.rb

class User < ActiveRecord::Base
  # Include default devise modules. Others available are:
  # :token_authenticatable, :confirmable,
  # :lockable, :timeoutable and :omniauthable
  devise :database_authenticatable, :registerable,
         :recoverable, :rememberable, :trackable, :validatable, :confirmable

  # Setup accessible (or protected) attributes for your model
  attr_accessible :email, :password, :password_confirmation, :remember_me

  after_create :add_attribute

  private

  def add_attribute
    self.skip_reconfirmation!
    self.update_attributes({ :status => 200 }, :without_protection => true)
  end
end

initializers/devise.rb

# Use this hook to configure devise mailer, warden hooks and so forth.
# Many of these configuration options can be set straight in your model.
Devise.setup do |config|

  ..
  ..

  # If true, requires any email changes to be confirmed (exactly the same way as
  # initial account confirmation) to be applied. Requires additional unconfirmed_email
  # db field (see migrations). Until confirmed new email is stored in
  # unconfirmed email column, and copied to email column on successful confirmation.
  config.reconfirmable = true

  ..
  ..

end

spec/models/user_spec.rb

require 'spec_helper'

describe User do

  subject(:user) { User.create(:email => 'nobody@nobody.com', :password => 'abcdefghijk') }

  it "should only send one email during creation" do
    expect {
      user
    }.to change(ActionMailer::Base.deliveries, :count).by(1)
  end

  it "should set attribute in after_create as expected" do
    user.status.should eq(200)
  end

end

Running the rspec tests to ensure only one email is sent confirms the behaviour:

..

Finished in 0.87571 seconds 2 examples, 0 failures

Answer 3

Thanks for your great solution, Stephen! I've tried it and it works perfectly to hook into the confirm! method. However, in this case, the function is being called (as the name says) when the user clicks the confirmation link in the email he receives.

An alternative is to hook into the generate_confirmation_token method, so your method is called directly when the confirmation token is created and the email is sent.

# app/models/user.rb
def generate_confirmation_token
  make_owner_an_account_member
  super # includes a call to save(validate: false), 
        # so be sure to call whatever you like beforehand
end

def make_owner_an_account_member
  self.account = owned_account if owned_account?
end

Relevant source of the confirmation module.