diff --git a/Gemfile b/Gemfile index 0c442ca2c6..88ad191231 100644 --- a/Gemfile +++ b/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 diff --git a/Gemfile.lock b/Gemfile.lock index fa904aedfc..f0678f9a11 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 17585a7237..e9cb17480b 100644 --- a/app/controllers/application_controller.rb +++ b/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'" diff --git a/app/controllers/spree/user_sessions_controller.rb b/app/controllers/spree/user_sessions_controller.rb index df42f622e1..7ff26a3437 100644 --- a/app/controllers/spree/user_sessions_controller.rb +++ b/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 diff --git a/app/controllers/user_confirmations_controller.rb b/app/controllers/user_confirmations_controller.rb index 1e15a6a296..0915447bc5 100644 --- a/app/controllers/user_confirmations_controller.rb +++ b/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 diff --git a/app/mailers/spree/user_mailer.rb b/app/mailers/spree/user_mailer.rb index ba25809b5d..a93aff05a5 100644 --- a/app/mailers/spree/user_mailer.rb +++ b/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 diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 3df57a9cf8..da276ec936 100644 --- a/app/models/spree/user.rb +++ b/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 diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index e20b748148..5a101fbe4d 100644 --- a/config/initializers/devise.rb +++ b/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 diff --git a/spec/controllers/user_confirmations_controller_spec.rb b/spec/controllers/user_confirmations_controller_spec.rb index 9920330161..335b46af79 100644 --- a/spec/controllers/user_confirmations_controller_spec.rb +++ b/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 diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb index c172f40f38..ad1da5cd77 100644 --- a/spec/features/consumer/authentication_spec.rb +++ b/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 diff --git a/spec/features/consumer/confirm_invitation_spec.rb b/spec/features/consumer/confirm_invitation_spec.rb index bfdd4baddc..43645a9e31 100644 --- a/spec/features/consumer/confirm_invitation_spec.rb +++ b/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) diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 9b4790e5ad..642ffd1406 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/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 diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index ba5b4cd488..fc89796e3c 100644 --- a/spec/models/spree/user_spec.rb +++ b/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 diff --git a/spec/support/email_helper.rb b/spec/support/email_helper.rb index f622785425..4291844575 100644 --- a/spec/support/email_helper.rb +++ b/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