Merge pull request #4084 from openfoodfoundation/dependabot/bundler/devise-3.5.10

[Security] Bump devise from 2.2.8 to 3.5.10

Gemfile

@@ -49,8 +49,9 @@ gem 'stripe'
 # which is needed for Pin Payments (and possibly others).
 gem 'activemerchant', '~> 1.78.0'
 
-gem 'devise', '~> 3.0.1'
+gem 'devise', '~> 3.5.10' # v4.0.0 needs rails 4.1
 gem 'devise-encryptable'
+gem 'devise-token_authenticatable', '~> 0.4.10' # v0.5.0 needs devise v4
 gem 'jwt', '~> 2.2'
 gem 'oauth2', '~> 1.4.4' # Used for Stripe Connect
 

Gemfile.lock

@@ -148,8 +148,6 @@ GEM
       nokogiri (>= 1.4.4)
       uuidtools (~> 2.1)
     bcrypt (3.1.13)
-    bcrypt-ruby (3.1.5)
-      bcrypt (>= 3.1.3)
     bugsnag (6.14.0)
       concurrent-ruby (~> 1.0)
     builder (3.1.4)
@@ -219,13 +217,17 @@ GEM
       delayed_job (> 2.0.3)
       rack-protection (>= 1.5.5)
       sinatra (>= 1.4.4)
-    devise (3.0.4)
-      bcrypt-ruby (~> 3.0)
+    devise (3.5.10)
+      bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 3.2.6, < 5)
+      responders
+      thread_safe (~> 0.1)
       warden (~> 1.2.3)
     devise-encryptable (0.2.0)
       devise (>= 2.1.0)
+    devise-token_authenticatable (0.4.10)
+      devise (>= 3.5.2, < 4.0.0)
     diff-lcs (1.3)
     diffy (3.3.0)
     docile (1.3.2)
@@ -555,6 +557,8 @@ GEM
     redcarpet (3.5.0)
     request_store (1.4.1)
       rack (>= 1.4)
+    responders (1.1.2)
+      railties (>= 3.2, < 4.2)
     rexml (3.2.4)
     roadie (3.4.0)
       css_parser (~> 1.4)
@@ -735,8 +739,9 @@ DEPENDENCIES
   debugger-linecache
   delayed_job_active_record
   delayed_job_web
-  devise (~> 3.0.1)
+  devise (~> 3.5.10)
   devise-encryptable
+  devise-token_authenticatable (~> 0.4.10)
   dfc_provider!
   diffy
   eventmachine (>= 1.2.3)

app/controllers/application_controller.rb

@@ -62,11 +62,15 @@ class ApplicationController < ActionController::Base
   end
 
   def after_sign_out_path_for(_resource_or_scope)
-    session[:shopfront_redirect] || main_app.root_path
+    shopfront_redirect || main_app.root_path
   end
 
   private
 
+  def shopfront_redirect
+    session[:shopfront_redirect]
+  end
+
   def restrict_iframes
     response.headers['X-Frame-Options'] = 'DENY'
     response.headers['Content-Security-Policy'] = "frame-ancestors 'none'"

app/controllers/spree/user_sessions_controller.rb

@@ -39,8 +39,18 @@ module Spree
       end
     end
 
+    def destroy
+      # Logout will clear session data including shopfront_redirect
+      #   Here we store it before actually logging out so that the redirect works correctly
+      @shopfront_redirect = session[:shopfront_redirect]
+
+      super
+    end
+
     private
 
+    attr_reader :shopfront_redirect
+
     def accurate_title
       Spree.t(:login)
     end

app/controllers/user_confirmations_controller.rb

@@ -44,9 +44,10 @@ class UserConfirmationsController < DeviseController
         'not_confirmed'
       end
 
-    if resource.reset_password_token.present?
+    if result == 'confirmed' && resource.reset_password_token.present?
+      raw_reset_password_token = resource.regenerate_reset_password_token
       return spree.edit_spree_user_password_path(
-        reset_password_token: resource.reset_password_token
+        reset_password_token: raw_reset_password_token
       )
     end
 

app/mailers/spree/user_mailer.rb

@@ -5,12 +5,11 @@ module Spree
     include I18nHelper
 
     # Overrides `Devise::Mailer.reset_password_instructions`
-    def reset_password_instructions(user)
-      recipient = user.respond_to?(:id) ? user : Spree.user_class.find(user)
+    def reset_password_instructions(user, token, _opts = {})
       @edit_password_reset_url = spree.
-        edit_spree_user_password_url(reset_password_token: recipient.reset_password_token)
+        edit_spree_user_password_url(reset_password_token: token)
 
-      mail(to: recipient.email, from: from_address,
+      mail(to: user.email, from: from_address,
            subject: Spree::Config[:site_name] + ' ' +
              I18n.t(:subject, scope: [:devise, :mailer, :reset_password_instructions]))
     end
@@ -25,8 +24,9 @@ module Spree
     end
 
     # Overrides `Devise::Mailer.confirmation_instructions`
-    def confirmation_instructions(user, _opts)
+    def confirmation_instructions(user, token, _opts = {})
       @user = user
+      @token = token
       @instance = Spree::Config[:site_name]
       @contact = ContentConfig.footer_email
 

app/models/spree/user.rb

@@ -34,10 +34,6 @@ module Spree
 
     # We use the same options as Spree and add :confirmable
     devise :confirmable, reconfirmable: true
-    # TODO: Later versions of devise have a dedicated after_confirmation callback, so use that
-    after_update :welcome_after_confirm, if: lambda {
-      confirmation_token_changed? && confirmation_token.nil?
-    }
 
     class DestroyWithOrdersError < StandardError; end
 
@@ -49,14 +45,14 @@ module Spree
       has_spree_role?('admin')
     end
 
-    def send_reset_password_instructions
-      generate_reset_password_token!
-      UserMailer.reset_password_instructions(id).deliver
-    end
     # handle_asynchronously will define send_reset_password_instructions_with_delay.
     # If handle_asynchronously is called twice, we get an infinite job loop.
     handle_asynchronously :send_reset_password_instructions unless method_defined? :send_reset_password_instructions_with_delay
 
+    def regenerate_reset_password_token
+      set_reset_password_token
+    end
+
     def known_users
       if admin?
         Spree::User.where(nil)
@@ -82,9 +78,9 @@ module Spree
       customers.find_by(enterprise_id: enterprise)
     end
 
-    def welcome_after_confirm
-      # Send welcome email if we are confirming an user's email
-      # Note: this callback only runs on email confirmation
+    # This is a Devise Confirmable callback that runs on email confirmation
+    # It sends a welcome email after the user email is confirmed
+    def after_confirmation
       return unless confirmed? && unconfirmed_email.nil? && !unconfirmed_email_changed?
 
       send_signup_confirmation

config/initializers/devise.rb

@@ -90,10 +90,6 @@ Devise.setup do |config|
   # Time interval to unlock the account if :time is enabled as unlock_strategy.
   # config.unlock_in = 1.hour
 
-  # ==> Configuration for :token_authenticatable
-  # Defines name of the authentication token params key
-  config.token_authentication_key = :auth_token
-
   # ==> Scopes configuration
   # Turn scoped views on. Before rendering 'sessions/new', it will first check for
   # 'users/sessions/new'. It's turned off by default because it's slower if you
@@ -141,3 +137,8 @@ Devise.setup do |config|
 
   config.case_insensitive_keys = [:email]
 end
+
+Devise::TokenAuthenticatable.setup do |config|
+  # Defines name of the authentication token params key
+  config.token_authentication_key = :auth_token
+end

spec/controllers/user_confirmations_controller_spec.rb

@@ -11,7 +11,7 @@ describe UserConfirmationsController, type: :controller do
 
   before do
     @request.env["devise.mapping"] = Devise.mappings[:spree_user]
-    confirmed_user.confirm!
+    confirmed_user.confirm
   end
 
   context "confirming a user" do
@@ -52,7 +52,8 @@ describe UserConfirmationsController, type: :controller do
         unconfirmed_user.reset_password_token = Devise.friendly_token
         unconfirmed_user.save!
         spree_get :show, confirmation_token: unconfirmed_user.confirmation_token
-        expect(response).to redirect_to spree.edit_spree_user_password_path(reset_password_token: unconfirmed_user.reset_password_token)
+        expect(response).to be_redirect
+        expect(response.body).to include spree.edit_spree_user_password_path
       end
     end
   end

spec/features/consumer/authentication_spec.rb

@@ -28,6 +28,7 @@ feature "Authentication", js: true do
           browse_as_large
           open_login_modal
         end
+
         scenario "showing login" do
           expect(page).to have_login_modal
         end
@@ -106,8 +107,31 @@ feature "Authentication", js: true do
             end.to enqueue_job Delayed::PerformableMethod
             expect(Delayed::Job.last.payload_object.method_name).to eq(:send_reset_password_instructions_without_delay)
           end
+
+          context "user with unconfirmed email" do
+            let(:email) { "test@example.org" }
+            let!(:user) { Spree::User.create(email: email, unconfirmed_email: email, password: "secret") }
+
+            scenario "cannot reset password before confirming email" do
+              fill_in "Your email", with: email
+              click_reset_password_button
+              expect(page).to have_content I18n.t('email_unconfirmed')
+              page.find("a", text: I18n.t('devise.confirmations.resend_confirmation_email')).click
+              expect(page).to have_content I18n.t('devise.confirmations.send_instructions')
+
+              visit spree.spree_user_confirmation_path(confirmation_token: user.confirmation_token)
+              expect(user.reload.confirmed?).to be true
+              expect(page).to have_text I18n.t('devise.confirmations.confirmed')
+
+              select_login_tab "Forgot Password?"
+              fill_in "Your email", with: email
+              click_reset_password_button
+              expect(page).to have_reset_password
+            end
+          end
         end
       end
+
       describe "as medium" do
         before do
           browse_as_medium

spec/features/consumer/confirm_invitation_spec.rb

@@ -1,7 +1,7 @@
 require "spec_helper"
 
 feature "Confirm invitation as manager" do
-  include UIComponentHelper # for be_logged_in_as
+  include UIComponentHelper
   include OpenFoodNetwork::EmailHelper
 
   describe "confirm email and set password" do
@@ -15,8 +15,8 @@ feature "Confirm invitation as manager" do
       user.save!
     end
 
-    it "allows you to set a password" do
-      visit spree.spree_user_confirmation_url(confirmation_token: user.confirmation_token)
+    it "lets the user set a password" do
+      visit spree.spree_user_confirmation_path(confirmation_token: user.confirmation_token)
 
       expect(user.reload.confirmed?).to be true
       expect(page).to have_text I18n.t(:change_my_password)

spec/mailers/user_mailer_spec.rb

@@ -49,7 +49,7 @@ describe Spree::UserMailer do
   describe '#reset_password_instructions' do
     describe 'message contents' do
       before do
-        @message = described_class.reset_password_instructions(user)
+        @message = described_class.reset_password_instructions(user, nil)
       end
 
       context 'subject includes' do
@@ -72,7 +72,7 @@ describe Spree::UserMailer do
     describe 'legacy support for User object' do
       it 'sends an email' do
         expect do
-          Spree::UserMailer.reset_password_instructions(user).deliver
+          Spree::UserMailer.reset_password_instructions(user, nil).deliver
         end.to change(ActionMailer::Base.deliveries, :size).by(1)
       end
     end

spec/models/spree/user_spec.rb

@@ -109,7 +109,7 @@ describe Spree.user_class do
       setup_email
 
       expect do
-        create(:user, confirmed_at: nil).confirm!
+        create(:user, confirmed_at: nil).confirm
       end.to enqueue_job ConfirmSignupJob
     end
   end

spec/support/email_helper.rb

@@ -2,8 +2,6 @@ module OpenFoodNetwork
   module EmailHelper
     # Some specs trigger actions that send emails, for example creating an order.
     # But sending emails doesn't work out-of-the-box. This code sets it up.
-    # It's here in a single place to allow an easy upgrade to Spree 2 which
-    # needs a different implementation of this method.
     def setup_email
       Spree::Config[:mails_from] = "test@ofn.example.org"
     end