From 5625458f69ed8d9ba06f2e7c2640523122d3b6ab Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 8 Mar 2020 21:03:36 -0500 Subject: [PATCH] users: refactor password reset flow. The old password reset flow: * User requests a password reset. * Danbooru generates a password reset nonce. * Danbooru emails user a password reset confirmation link. * User follows link to password reset confirmation page. * The link contains a nonce authenticating the user. * User confirms password reset. * Danbooru resets user's password to a random string. * Danbooru emails user their new password in plaintext. The new password reset flow: * User requests a password reset. * Danbooru emails user a password reset link. * User follows link to password edit page. * The link contains a signed_user_id param authenticating the user. * User changes their own password. --- .../user/password_resets_controller.rb | 38 ------ app/controllers/password_resets_controller.rb | 14 +++ app/controllers/passwords_controller.rb | 2 +- app/helpers/application_helper.rb | 2 +- app/logical/danbooru/message_verifier.rb | 8 +- app/logical/danbooru_maintenance.rb | 1 - app/logical/session_loader.rb | 7 ++ app/mailers/application_mailer.rb | 3 + .../maintenance/user/password_reset_mailer.rb | 17 --- app/mailers/user_mailer.rb | 8 +- app/models/dmail.rb | 3 +- app/models/user.rb | 37 +----- app/models/user_password_reset_nonce.rb | 28 ----- .../confirmation.html.erb | 5 - .../reset_request.html.erb | 4 - .../user/password_resets/edit.html.erb | 19 --- .../user/password_resets/new.html.erb | 20 ---- app/views/password_resets/show.html.erb | 22 ++++ app/views/passwords/edit.html.erb | 8 +- app/views/sessions/new.html.erb | 2 +- app/views/user_mailer/password_reset.html.erb | 22 ++++ config/routes.rb | 2 +- ...9035334_drop_user_password_reset_nonces.rb | 7 ++ db/structure.sql | 50 +------- test/factories/user_password_reset_nonce.rb | 3 - .../user/password_resets_controller_test.rb | 112 ------------------ .../password_resets_controller_test.rb | 25 ++++ test/mailers/previews/user_mailer_preview.rb | 5 + test/unit/user_password_reset_nonce_test.rb | 48 -------- test/unit/user_test.rb | 6 - 30 files changed, 133 insertions(+), 395 deletions(-) delete mode 100644 app/controllers/maintenance/user/password_resets_controller.rb create mode 100644 app/controllers/password_resets_controller.rb create mode 100644 app/mailers/application_mailer.rb delete mode 100644 app/mailers/maintenance/user/password_reset_mailer.rb delete mode 100644 app/models/user_password_reset_nonce.rb delete mode 100644 app/views/maintenance/user/password_reset_mailer/confirmation.html.erb delete mode 100644 app/views/maintenance/user/password_reset_mailer/reset_request.html.erb delete mode 100644 app/views/maintenance/user/password_resets/edit.html.erb delete mode 100644 app/views/maintenance/user/password_resets/new.html.erb create mode 100644 app/views/password_resets/show.html.erb create mode 100644 app/views/user_mailer/password_reset.html.erb create mode 100644 db/migrate/20200309035334_drop_user_password_reset_nonces.rb delete mode 100644 test/factories/user_password_reset_nonce.rb delete mode 100644 test/functional/maintenance/user/password_resets_controller_test.rb create mode 100644 test/functional/password_resets_controller_test.rb delete mode 100644 test/unit/user_password_reset_nonce_test.rb diff --git a/app/controllers/maintenance/user/password_resets_controller.rb b/app/controllers/maintenance/user/password_resets_controller.rb deleted file mode 100644 index 79b7ca5fc..000000000 --- a/app/controllers/maintenance/user/password_resets_controller.rb +++ /dev/null @@ -1,38 +0,0 @@ -module Maintenance - module User - class PasswordResetsController < ApplicationController - def new - @nonce = UserPasswordResetNonce.new - end - - def create - @nonce = UserPasswordResetNonce.create(nonce_params) - if @nonce.errors.any? - redirect_to new_maintenance_user_password_reset_path, :notice => @nonce.errors.full_messages.join("; ") - else - redirect_to new_maintenance_user_password_reset_path, :notice => "Email request sent" - end - end - - def edit - @nonce = UserPasswordResetNonce.where(:email => params[:email], :key => params[:key]).first - end - - def update - @nonce = UserPasswordResetNonce.where(:email => params[:email], :key => params[:key]).first - - if @nonce - @nonce.reset_user! - @nonce.destroy - redirect_to new_maintenance_user_password_reset_path, :notice => "Password reset; email delivered with new password" - else - redirect_to new_maintenance_user_password_reset_path, :notice => "Invalid key" - end - end - - def nonce_params - params.fetch(:nonce, {}).permit([:email]) - end - end - end -end diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb new file mode 100644 index 000000000..ba22cf9b8 --- /dev/null +++ b/app/controllers/password_resets_controller.rb @@ -0,0 +1,14 @@ +class PasswordResetsController < ApplicationController + respond_to :html, :xml, :json + + def create + @user = User.find_by_name(params.dig(:user, :name)) + UserMailer.password_reset(@user).deliver_later + + flash[:notice] = "Password reset email sent. Check your email" + respond_with(@user, location: new_session_path) + end + + def show + end +end diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 2d147f07f..41278a482 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -26,6 +26,6 @@ class PasswordsController < ApplicationController end def user_params - params.require(:user).permit(%i[old_password password password_confirmation]) + params.require(:user).permit(%i[signed_user_id old_password password password_confirmation]) end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3de619905..8a5af6ace 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -337,7 +337,7 @@ module ApplicationHelper def nav_link_match(controller, url) url =~ case controller - when "sessions", "users", "maintenance/user/password_resets", "admin/users" + when "sessions", "users", "admin/users" /^\/(session|users)/ when "comments" diff --git a/app/logical/danbooru/message_verifier.rb b/app/logical/danbooru/message_verifier.rb index 602d9e71e..cea28c978 100644 --- a/app/logical/danbooru/message_verifier.rb +++ b/app/logical/danbooru/message_verifier.rb @@ -8,12 +8,12 @@ module Danbooru @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON, digest: "SHA256") end - def generate(*options) - verifier.generate(*options, purpose: purpose) + def generate(*args, **options) + verifier.generate(*args, purpose: purpose, **options) end - def verified(*options) - verifier.verified(*options, purpose: purpose) + def verify(*args, **options) + verifier.verify(*args, purpose: purpose, **options) end end end diff --git a/app/logical/danbooru_maintenance.rb b/app/logical/danbooru_maintenance.rb index a76cfc9b1..bcd082f85 100644 --- a/app/logical/danbooru_maintenance.rb +++ b/app/logical/danbooru_maintenance.rb @@ -19,7 +19,6 @@ module DanbooruMaintenance end def weekly - safely { UserPasswordResetNonce.prune! } safely { TagRelationshipRetirementService.find_and_retire! } safely { ApproverPruner.dmail_inactive_approvers! } end diff --git a/app/logical/session_loader.rb b/app/logical/session_loader.rb index 64e0baf21..78b28798e 100644 --- a/app/logical/session_loader.rb +++ b/app/logical/session_loader.rb @@ -16,6 +16,8 @@ class SessionLoader if has_api_authentication? load_session_for_api + elsif params[:signed_user_id] + load_param_user(params[:signed_user_id]) elsif session[:user_id] load_session_user elsif cookie_password_hash_valid? @@ -79,6 +81,11 @@ class SessionLoader end end + def load_param_user(signed_user_id) + session[:user_id] = Danbooru::MessageVerifier.new(:login).verify(signed_user_id) + load_session_user + end + def load_session_user user = User.find_by_id(session[:user_id]) CurrentUser.user = user if user diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb new file mode 100644 index 000000000..965194ecd --- /dev/null +++ b/app/mailers/application_mailer.rb @@ -0,0 +1,3 @@ +class ApplicationMailer < ActionMailer::Base + default from: "#{Danbooru.config.canonical_app_name} <#{Danbooru.config.contact_email}>", content_type: "text/html" +end diff --git a/app/mailers/maintenance/user/password_reset_mailer.rb b/app/mailers/maintenance/user/password_reset_mailer.rb deleted file mode 100644 index 7910bacbd..000000000 --- a/app/mailers/maintenance/user/password_reset_mailer.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Maintenance - module User - class PasswordResetMailer < ActionMailer::Base - def reset_request(user, nonce) - @user = user - @nonce = nonce - mail(:to => @user.email, :subject => "#{Danbooru.config.app_name} password reset request", :from => Danbooru.config.contact_email) - end - - def confirmation(user, new_password) - @user = user - @new_password = new_password - mail(:to => @user.email, :subject => "#{Danbooru.config.app_name} password reset confirmation", :from => Danbooru.config.contact_email) - end - end - end -end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index ddbe65632..00bdb1281 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -1,10 +1,14 @@ -class UserMailer < ActionMailer::Base +class UserMailer < ApplicationMailer add_template_helper ApplicationHelper add_template_helper UsersHelper - default :from => Danbooru.config.contact_email, :content_type => "text/html" def dmail_notice(dmail) @dmail = dmail mail(:to => "#{dmail.to.name} <#{dmail.to.email}>", :subject => "#{Danbooru.config.app_name} - Message received from #{dmail.from.name}") end + + def password_reset(user) + @user = user + mail to: "#{@user.name} <#{@user.email}>", subject: "#{Danbooru.config.app_name} password reset request" + end end diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 53174ddbe..d59e50ced 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -121,8 +121,7 @@ class Dmail < ApplicationRecord end def valid_key?(key) - decoded_id = verifier.verified(key) - id == decoded_id + id == verifier.verify(key) end def visible_to?(user, key) diff --git a/app/models/user.rb b/app/models/user.rb index dd7f7f7f8..07cea2710 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -68,7 +68,7 @@ class User < ApplicationRecord has_bit_flags BOOLEAN_ATTRIBUTES, :field => "bit_prefs" - attr_accessor :password, :old_password + attr_accessor :password, :old_password, :signed_user_id after_initialize :initialize_attributes, if: :new_record? validates :name, user_name: true, on: :create @@ -185,36 +185,15 @@ class User < ApplicationRecord def encrypt_password_on_update return if password.blank? - return if old_password.blank? - if bcrypt_password == User.sha1(old_password) + if signed_user_id.present? && id == Danbooru::MessageVerifier.new(:login).verify(signed_user_id) + self.bcrypt_password_hash = User.bcrypt(password) + elsif old_password.present? && bcrypt_password == User.sha1(old_password) self.bcrypt_password_hash = User.bcrypt(password) - return true else errors[:old_password] << "is incorrect" - return false end end - - def reset_password - consonants = "bcdfghjklmnpqrstvqxyz" - vowels = "aeiou" - pass = "" - - 6.times do - pass << consonants[rand(21), 1] - pass << vowels[rand(5), 1] - end - - pass << rand(100).to_s - update_column(:bcrypt_password_hash, User.bcrypt(pass)) - pass - end - - def reset_password_and_deliver_notice - new_password = reset_password - Maintenance::User::PasswordResetMailer.confirmation(self, new_password).deliver_now - end end module AuthenticationMethods @@ -637,14 +616,6 @@ class User < ApplicationRecord end module SearchMethods - def with_email(email) - if email.blank? - where("FALSE") - else - where("email = ?", email) - end - end - def search(params) q = super diff --git a/app/models/user_password_reset_nonce.rb b/app/models/user_password_reset_nonce.rb deleted file mode 100644 index faf1c9d0c..000000000 --- a/app/models/user_password_reset_nonce.rb +++ /dev/null @@ -1,28 +0,0 @@ -class UserPasswordResetNonce < ApplicationRecord - has_secure_token :key - validates_presence_of :email - validate :validate_existence_of_email - after_create :deliver_notice - - def self.prune! - where("created_at < ?", 1.week.ago).destroy_all - end - - def deliver_notice - Maintenance::User::PasswordResetMailer.reset_request(user, self).deliver_now - end - - def validate_existence_of_email - if !User.with_email(email).exists? - errors[:email] << "is invalid" - end - end - - def reset_user! - user.reset_password_and_deliver_notice - end - - def user - @user ||= User.with_email(email).first - end -end diff --git a/app/views/maintenance/user/password_reset_mailer/confirmation.html.erb b/app/views/maintenance/user/password_reset_mailer/confirmation.html.erb deleted file mode 100644 index 93f758add..000000000 --- a/app/views/maintenance/user/password_reset_mailer/confirmation.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -

Password Reset Confirmation

- -

The password for the user "<%= @user.name %>" for the website <%= Danbooru.config.app_name %> has been reset. It is now <%= @new_password %>.

- -

Please log in to the website and <%= link_to "change your password", edit_user_url(@user, :host => Danbooru.config.hostname, :only_path => false) %> as soon as possible.

diff --git a/app/views/maintenance/user/password_reset_mailer/reset_request.html.erb b/app/views/maintenance/user/password_reset_mailer/reset_request.html.erb deleted file mode 100644 index 9f8652f8a..000000000 --- a/app/views/maintenance/user/password_reset_mailer/reset_request.html.erb +++ /dev/null @@ -1,4 +0,0 @@ -

Password Reset Request

- -

Someone has requested that the password for "<%= @user.name %>" for the website <%= Danbooru.config.app_name %> be reset. If you did not request this, then you can ignore this email.

-

To reset your password, please visit <%= link_to "this link", edit_maintenance_user_password_reset_url(:host => Danbooru.config.hostname, :only_path => false, :key => @nonce.key, :email => @nonce.email) %>.

diff --git a/app/views/maintenance/user/password_resets/edit.html.erb b/app/views/maintenance/user/password_resets/edit.html.erb deleted file mode 100644 index 926ded6bb..000000000 --- a/app/views/maintenance/user/password_resets/edit.html.erb +++ /dev/null @@ -1,19 +0,0 @@ -<% page_title "Reset Password" %> -<%= render "sessions/secondary_links" %> - -
-
-

Reset Password

- - <% if @nonce %> - <%= form_tag(maintenance_user_password_reset_path, :method => :put) do %> - <%= hidden_field_tag :email, params[:email] %> - <%= hidden_field_tag :key, params[:key] %> -

Do you wish to reset your password? A new password will be emailed to you.

- <%= submit_tag "Reset" %> - <% end %> - <% else %> -

Invalid key

- <% end %> -
-
diff --git a/app/views/maintenance/user/password_resets/new.html.erb b/app/views/maintenance/user/password_resets/new.html.erb deleted file mode 100644 index 2de383ed6..000000000 --- a/app/views/maintenance/user/password_resets/new.html.erb +++ /dev/null @@ -1,20 +0,0 @@ -<% page_title "Reset Password" %> -<%= render "sessions/secondary_links" %> - -
-
-

Reset Password

- -

If you supplied an email address when signing up, <%= Danbooru.config.app_name %> can reset your password. You will receive an email confirming your request for a new password.

- -

If you didn't supply a valid email address, you are out of luck.

- - <%= form_tag(maintenance_user_password_reset_path, :class => "simple_form") do %> - - <%= submit_tag "Submit" %> - <% end %> -
-
diff --git a/app/views/password_resets/show.html.erb b/app/views/password_resets/show.html.erb new file mode 100644 index 000000000..e6fbed154 --- /dev/null +++ b/app/views/password_resets/show.html.erb @@ -0,0 +1,22 @@ +<% page_title "Reset Password" %> +<%= render "sessions/secondary_links" %> + +
+
+

Reset Password

+ +

+ Enter your username below to reset your password. You will be sent an + email containing a link to reset your password. +

+ +

+ If your account doesn't have a valid email address, then your password can't be reset. +

+ + <%= edit_form_for(:user, url: password_reset_path, action: :post) do |f| %> + <%= f.input :name, label: "Username", input_html: { "data-autocomplete": "user" } %> + <%= f.submit "Submit" %> + <% end %> +
+
diff --git a/app/views/passwords/edit.html.erb b/app/views/passwords/edit.html.erb index 5f495ad1d..9ac74628d 100644 --- a/app/views/passwords/edit.html.erb +++ b/app/views/passwords/edit.html.erb @@ -4,8 +4,14 @@

Change Password

+

Enter a new password below.

+ <%= edit_form_for(@user, url: user_password_path(@user)) do |f| %> - <%= f.input :old_password, as: :password, hint: "Re-enter your current password." %> + <% if params[:signed_user_id] %> + <%= f.input :signed_user_id, as: :hidden, input_html: { value: params[:signed_user_id] } %> + <% else %> + <%= f.input :old_password, as: :password, hint: "Re-enter your current password." %> + <% end %> <%= f.input :password, label: "New password", hint: "Must be at least 5 characters long." %> <%= f.input :password_confirmation, label: "Confirm new password" %> <%= f.submit "Save" %> diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 438011153..7ce1ffa31 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -11,7 +11,7 @@ <%= simple_form_for(:session, url: session_path) do |f| %> <%= f.input :url, as: :hidden, input_html: { value: params[:url] } %> <%= f.input :name %> - <%= f.input :password, hint: link_to("Forgot password?", new_maintenance_user_password_reset_path), input_html: { autocomplete: "password" } %> + <%= f.input :password, hint: link_to("Forgot password?", password_reset_path), input_html: { autocomplete: "password" } %> <%= f.submit "Login" %> <% end %> diff --git a/app/views/user_mailer/password_reset.html.erb b/app/views/user_mailer/password_reset.html.erb new file mode 100644 index 000000000..37044075b --- /dev/null +++ b/app/views/user_mailer/password_reset.html.erb @@ -0,0 +1,22 @@ + + + +

Hi <%= @user.name %>,

+ +

+ You recently requested your password to be reset for your <%= Danbooru.config.app_name %> + account. Click the link below to login to <%= Danbooru.config.app_name %> + and reset your password. +

+ +

+ <%= link_to "Reset password", edit_user_password_url(@user, signed_user_id: Danbooru::MessageVerifier.new(:login).generate(@user.id, expires_in: 30.minutes)) %> +

+ +

+ If you did not request for your <%= Danbooru.config.app_name %> password to + be reset, please ignore this email or reply to let us know. This link + will only be valid for the next 30 minutes. +

+ + diff --git a/config/routes.rb b/config/routes.rb index 131ff675f..4da99cd9f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -45,7 +45,6 @@ Rails.application.routes.draw do namespace :user do resource :count_fixes, only: [:new, :create] resource :email_notification, :only => [:show, :destroy] - resource :password_reset, :only => [:new, :create, :edit, :update] resource :deletion, :only => [:show, :destroy] resource :email_change, :only => [:new, :create] resource :api_key, :only => [:show, :view, :update, :destroy] do @@ -153,6 +152,7 @@ Rails.application.routes.draw do end resources :note_versions, :only => [:index, :show] resource :note_previews, :only => [:show] + resource :password_reset, only: [:create, :show] resources :pixiv_ugoira_frame_data, only: [:index] resources :pools do member do diff --git a/db/migrate/20200309035334_drop_user_password_reset_nonces.rb b/db/migrate/20200309035334_drop_user_password_reset_nonces.rb new file mode 100644 index 000000000..c73a7b6d1 --- /dev/null +++ b/db/migrate/20200309035334_drop_user_password_reset_nonces.rb @@ -0,0 +1,7 @@ +require_relative "20110717010705_create_user_password_reset_nonces" + +class DropUserPasswordResetNonces < ActiveRecord::Migration[6.0] + def change + revert CreateUserPasswordResetNonces + end +end diff --git a/db/structure.sql b/db/structure.sql index fdd100ef7..9afdbd96b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -3049,38 +3049,6 @@ CREATE SEQUENCE public.user_name_change_requests_id_seq ALTER SEQUENCE public.user_name_change_requests_id_seq OWNED BY public.user_name_change_requests.id; --- --- Name: user_password_reset_nonces; Type: TABLE; Schema: public; Owner: - --- - -CREATE TABLE public.user_password_reset_nonces ( - id integer NOT NULL, - key character varying NOT NULL, - email character varying NOT NULL, - created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL -); - - --- --- Name: user_password_reset_nonces_id_seq; Type: SEQUENCE; Schema: public; Owner: - --- - -CREATE SEQUENCE public.user_password_reset_nonces_id_seq - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - - --- --- Name: user_password_reset_nonces_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - --- - -ALTER SEQUENCE public.user_password_reset_nonces_id_seq OWNED BY public.user_password_reset_nonces.id; - - -- -- Name: users_id_seq; Type: SEQUENCE; Schema: public; Owner: - -- @@ -4142,13 +4110,6 @@ ALTER TABLE ONLY public.user_feedback ALTER COLUMN id SET DEFAULT nextval('publi ALTER TABLE ONLY public.user_name_change_requests ALTER COLUMN id SET DEFAULT nextval('public.user_name_change_requests_id_seq'::regclass); --- --- Name: user_password_reset_nonces id; Type: DEFAULT; Schema: public; Owner: - --- - -ALTER TABLE ONLY public.user_password_reset_nonces ALTER COLUMN id SET DEFAULT nextval('public.user_password_reset_nonces_id_seq'::regclass); - - -- -- Name: users id; Type: DEFAULT; Schema: public; Owner: - -- @@ -4506,14 +4467,6 @@ ALTER TABLE ONLY public.user_name_change_requests ADD CONSTRAINT user_name_change_requests_pkey PRIMARY KEY (id); --- --- Name: user_password_reset_nonces user_password_reset_nonces_pkey; Type: CONSTRAINT; Schema: public; Owner: - --- - -ALTER TABLE ONLY public.user_password_reset_nonces - ADD CONSTRAINT user_password_reset_nonces_pkey PRIMARY KEY (id); - - -- -- Name: users users_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -7323,6 +7276,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200223042415'), ('20200223234015'), ('20200306202253'), -('20200307021204'); +('20200307021204'), +('20200309035334'); diff --git a/test/factories/user_password_reset_nonce.rb b/test/factories/user_password_reset_nonce.rb deleted file mode 100644 index 03d1edc6a..000000000 --- a/test/factories/user_password_reset_nonce.rb +++ /dev/null @@ -1,3 +0,0 @@ -FactoryBot.define do - factory(:user_password_reset_nonce) -end diff --git a/test/functional/maintenance/user/password_resets_controller_test.rb b/test/functional/maintenance/user/password_resets_controller_test.rb deleted file mode 100644 index e74442935..000000000 --- a/test/functional/maintenance/user/password_resets_controller_test.rb +++ /dev/null @@ -1,112 +0,0 @@ -require "test_helper" - -module Maintenance - module User - class PasswordResetsControllerTest < ActionDispatch::IntegrationTest - context "A password resets controller" do - setup do - @user = create(:user, :email => "abc@com.net") - ActionMailer::Base.delivery_method = :test - ActionMailer::Base.deliveries.clear - end - - should "render the new page" do - get new_maintenance_user_password_reset_path - assert_response :success - end - - context "create action" do - context "given invalid parameters" do - setup do - post maintenance_user_password_reset_path, params: {:nonce => {:email => ""}} - end - - should "not create a new nonce" do - assert_equal(0, UserPasswordResetNonce.count) - end - - should "redirect to the new page" do - assert_redirected_to new_maintenance_user_password_reset_path - end - - should "not deliver an email" do - assert_equal(0, ActionMailer::Base.deliveries.size) - end - end - - context "given valid parameters" do - setup do - post maintenance_user_password_reset_path, params: {:nonce => {:email => @user.email}} - end - - should "create a new nonce" do - assert_equal(1, UserPasswordResetNonce.where(:email => @user.email).count) - end - - should "redirect to the new page" do - assert_redirected_to new_maintenance_user_password_reset_path - end - - should "deliver an email to the supplied email address" do - assert_equal(1, ActionMailer::Base.deliveries.size) - end - end - end - - context "edit action" do - context "with invalid parameters" do - setup do - get edit_maintenance_user_password_reset_path, params: {:email => "a@b.c"} - end - - should "succeed silently" do - assert_response :success - end - end - - context "with valid parameters" do - setup do - @user = create(:user) - @nonce = create(:user_password_reset_nonce, :email => @user.email) - ActionMailer::Base.deliveries.clear - get edit_maintenance_user_password_reset_path, params: {:email => @nonce.email, :key => @nonce.key} - end - - should "succeed" do - assert_response :success - end - end - end - - context "update action" do - context "with valid parameters" do - setup do - @user = create(:user) - @nonce = create(:user_password_reset_nonce, :email => @user.email) - ActionMailer::Base.deliveries.clear - @old_password = @user.bcrypt_password_hash - put maintenance_user_password_reset_path, params: {:email => @nonce.email, :key => @nonce.key} - end - - should "succeed" do - assert_redirected_to new_maintenance_user_password_reset_path - end - - should "send an email" do - assert_equal(1, ActionMailer::Base.deliveries.size) - end - - should "change the password" do - @user.reload - assert_not_equal(@old_password, @user.bcrypt_password_hash) - end - - should "delete the nonce" do - assert_equal(0, UserPasswordResetNonce.count) - end - end - end - end - end - end -end diff --git a/test/functional/password_resets_controller_test.rb b/test/functional/password_resets_controller_test.rb new file mode 100644 index 000000000..a4d172f0b --- /dev/null +++ b/test/functional/password_resets_controller_test.rb @@ -0,0 +1,25 @@ +require 'test_helper' + +class PasswordResetsControllerTest < ActionDispatch::IntegrationTest + context "The passwords resets controller" do + setup do + @user = create(:user) + end + + context "show action" do + should "work" do + get password_reset_path + assert_response :success + end + end + + context "create action" do + should "work" do + post password_reset_path, params: { user: { name: @user.name } } + + assert_redirected_to new_session_path + assert_enqueued_email_with UserMailer, :password_reset, args: [@user] + end + end + end +end diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb index ec87e1e02..595b47a50 100644 --- a/test/mailers/previews/user_mailer_preview.rb +++ b/test/mailers/previews/user_mailer_preview.rb @@ -3,4 +3,9 @@ class UserMailerPreview < ActionMailer::Preview dmail = User.admins.first.dmails.first UserMailer.dmail_notice(dmail) end + + def password_reset + user = User.find(params[:id]) + UserMailer.password_reset(user) + end end diff --git a/test/unit/user_password_reset_nonce_test.rb b/test/unit/user_password_reset_nonce_test.rb deleted file mode 100644 index a794fc1fa..000000000 --- a/test/unit/user_password_reset_nonce_test.rb +++ /dev/null @@ -1,48 +0,0 @@ -require 'test_helper' - -class UserPasswordResetNonceTest < ActiveSupport::TestCase - context "Creating a new nonce" do - context "with a valid email" do - setup do - @user = FactoryBot.create(:user, :email => "aaa@b.net") - @nonce = FactoryBot.create(:user_password_reset_nonce, :email => @user.email) - end - - should "validate" do - assert_equal([], @nonce.errors.full_messages) - end - - should "populate the key with a random string" do - assert_equal(24, @nonce.key.size) - end - - should "reset the password when reset" do - @nonce.user.expects(:reset_password_and_deliver_notice) - @nonce.reset_user! - end - end - - context "with a blank email" do - setup do - @user = FactoryBot.create(:user, :email => "") - @nonce = UserPasswordResetNonce.new(:email => "") - end - - should "not validate" do - @nonce.save - assert_equal(["Email can't be blank", "Email is invalid"], @nonce.errors.full_messages.sort) - end - end - - context "with an invalid email" do - setup do - @nonce = UserPasswordResetNonce.new(:email => "z@z.net") - end - - should "not validate" do - @nonce.save - assert_equal(["Email is invalid"], @nonce.errors.full_messages) - end - end - end -end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 8c8056f54..3a114ebea 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -221,12 +221,6 @@ class UserTest < ActiveSupport::TestCase assert_equal(["Password is too short (minimum is 5 characters)"], @user.errors.full_messages) end - should "should be reset" do - @user = FactoryBot.create(:user) - new_pass = @user.reset_password - assert(User.authenticate(@user.name, new_pass), "Authentication should have succeeded") - end - should "not change the password if the password and old password are blank" do @user = FactoryBot.create(:user, :password => "67890") @user.update(password: "", old_password: "")