diff --git a/README.md b/README.md index b4a26c4..5b68c79 100644 --- a/README.md +++ b/README.md @@ -604,70 +604,37 @@ class UsersController < ApplicationController end ``` -## Step 9: Add Password Reset Columns to Users Table +## Step 9: Add Password Reset Functionality -1. Create migration. - -```bash -rails g migration add_password_reset_token_to_users password_reset_token:string password_reset_sent_at:datetime -``` - -2. Update the migration. - -```ruby -# db/migrate/[timestamp]_add_password_reset_token_to_users.rb -class AddPasswordResetTokenToUsers < ActiveRecord::Migration[6.1] - def change - add_column :users, :password_reset_token, :string, null: false - add_column :users, :password_reset_sent_at, :datetime - add_index :users, :password_reset_token, unique: true - end -end -``` - -> **What's Going On Here?** -> -> - The `password_reset_token` column will store a random value created through the [has_secure_token](https://api.rubyonrails.org/classes/ActiveRecord/SecureToken/ClassMethods.html#method-i-has_secure_token) method when a record is saved. This will be used to identify users in a secure way when they need to reset their password. We add `null: false` to prevent empty values and also add a unique index to ensure that no two users will have the same `password_reset_token`. You can think of this as a secure alternative to the `id` column. -> - The `password_reset_sent_at` column will be used to ensure a password reset link has not expired. This is an added layer of security to prevent a `password_reset_token` from being used multiple times. - -3. Run migration. - -```bash -rails db:migrate -``` - -4. Update User Model. +1. Update User Model. ```ruby # app/models/user.rb class User < ApplicationRecord ... - PASSWORD_RESET_TOKEN_EXPIRATION_IN_SECONDS = 10.minutes.to_i - ... - has_secure_token :password_reset_token + PASSWORD_RESET_TOKEN_EXPIRATION = 10.minutes ... - def password_reset_token_has_expired? - return true if password_reset_sent_at.nil? - (Time.current - password_reset_sent_at) >= User::PASSWORD_RESET_TOKEN_EXPIRATION_IN_SECONDS + def generate_password_reset_token + signed_id expires_in: PASSWORD_RESET_TOKEN_EXPIRATION, purpose: :reset_password end - + ... def send_password_reset_email! - regenerate_password_reset_token - update_columns(password_reset_sent_at: Time.current) - UserMailer.password_reset(self).deliver_now + password_reset_token = generate_password_reset_token + UserMailer.password_reset(self, password_reset_token).deliver_now end ... end ``` -5. Update User Mailer. +2. Update User Mailer. ```ruby # app/mailers/user_mailer.rb class UserMailer < ApplicationMailer ... - def password_reset(user) + def password_reset(user, password_reset_token) @user = user + @password_reset_token = password_reset_token mail to: @user.email, subject: "Password Reset Instructions" end @@ -678,21 +645,20 @@ end

Password Reset Instructions

-<%= link_to "Click here to reset your password.", edit_password_url(@user.password_reset_token) %> +<%= link_to "Click here to reset your password.", edit_password_url(@password_reset_token) %> ``` ```text Password Reset Instructions -<%= edit_password_url(@user.password_reset_token) %> +<%= edit_password_url(@password_reset_token) %> ``` > **What's Going On Here?** > -> - The `has_secure_token :password_reset_token` method is added to give us an [API](https://api.rubyonrails.org/classes/ActiveRecord/SecureToken/ClassMethods.html#method-i-has_secure_token) to work with the `password_reset_token` column. -> - The `password_reset_token_has_expired?` method tells us if the password reset token is expired or not. This can be controlled by changing the value of the `PASSWORD_RESET_TOKEN_EXPIRATION_IN_SECONDS` constant. This will be useful when we build the password reset mailer. -> - The `send_password_reset_email!` method will create a new `password_reset_token` and update the value of `password_reset_sent_at`. This is to ensure password reset links expire and cannot be reused. It will also send the password reset email to the user. We still need to build this. +> - The `generate_password_reset_token` method creates a [signed_id](https://api.rubyonrails.org/classes/ActiveRecord/SignedId.html#method-i-signed_id) that will be used to securely identify the user. For added security, we ensure that this ID will expire in 10 minutes (this can be controlled with the `PASSWORD_RESET_TOKEN_EXPIRATION` constant) and give it an explicit purpose of `:reset_password`. +> - The `send_password_reset_email!` method will create a new `password_reset_token`. This is to ensure password reset links expire and cannot be reused. It will also send the password reset email to the user. ## Step 10: Build Password Reset Forms @@ -722,10 +688,10 @@ class PasswordsController < ApplicationController end def edit - @user = User.find_by(password_reset_token: params[:password_reset_token]) + @user = User.find_signed(params[:password_reset_token], purpose: :reset_password) if @user.present? && @user.unconfirmed? redirect_to new_confirmation_path, alert: "You must confirm your email before you can sign in." - elsif @user.nil? || @user.password_reset_token_has_expired? + elsif @user.nil? redirect_to new_password_path, alert: "Invalid or expired token." end end @@ -734,21 +700,18 @@ class PasswordsController < ApplicationController end def update - @user = User.find_by(password_reset_token: params[:password_reset_token]) + @user = User.find_signed(params[:password_reset_token], purpose: :reset_password) if @user if @user.unconfirmed? redirect_to new_confirmation_path, alert: "You must confirm your email before you can sign in." - elsif @user.password_reset_token_has_expired? - redirect_to new_password_path, alert: "Incorrect email or password." elsif @user.update(password_params) - @user.regenerate_password_reset_token - redirect_to login_path, notice: "Signed in." + redirect_to login_path, notice: "Sign in." else flash.now[:alert] = @user.errors.full_messages.to_sentence render :edit, status: :unprocessable_entity end else - flash.now[:alert] = "Incorrect email or password." + flash.now[:alert] = "Invalid or expired token." render :new, status: :unprocessable_entity end end @@ -797,7 +760,7 @@ end ```html+ruby -<%= form_with url: password_path(@user.password_reset_token), scope: :user, method: :put do |form| %> +<%= form_with url: password_path(params[:password_reset_token]), scope: :user, method: :put do |form| %>
<%= form.label :password %> <%= form.password_field :password, required: true %> diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 814bdce..abb6576 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -16,10 +16,10 @@ def create end def edit - @user = User.find_by(password_reset_token: params[:password_reset_token]) + @user = User.find_signed(params[:password_reset_token], purpose: :reset_password) if @user.present? && @user.unconfirmed? redirect_to new_confirmation_path, alert: "You must confirm your email before you can sign in." - elsif @user.nil? || @user.password_reset_token_has_expired? + elsif @user.nil? redirect_to new_password_path, alert: "Invalid or expired token." end end @@ -28,21 +28,18 @@ def new end def update - @user = User.find_by(password_reset_token: params[:password_reset_token]) + @user = User.find_signed(params[:password_reset_token], purpose: :reset_password) if @user if @user.unconfirmed? redirect_to new_confirmation_path, alert: "You must confirm your email before you can sign in." - elsif @user.password_reset_token_has_expired? - redirect_to new_password_path, alert: "Incorrect email or password." elsif @user.update(password_params) - @user.regenerate_password_reset_token - redirect_to login_path, notice: "Password updated." + redirect_to login_path, notice: "Sign in." else flash.now[:alert] = @user.errors.full_messages.to_sentence render :edit, status: :unprocessable_entity end else - flash.now[:alert] = "Incorrect email or password." + flash.now[:alert] = "Invalid or expired token." render :new, status: :unprocessable_entity end end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 533a0e2..ae2c631 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -13,8 +13,9 @@ def confirmation(user, confirmation_token) mail to: @user.confirmable_email, subject: "Confirmation Instructions" end - def password_reset(user) + def password_reset(user, password_reset_token) @user = user + @password_reset_token = password_reset_token mail to: @user.email, subject: "Password Reset Instructions" end diff --git a/app/models/user.rb b/app/models/user.rb index 92fa724..3e42202 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,12 +1,11 @@ class User < ApplicationRecord CONFIRMATION_TOKEN_EXPIRATION = 10.minutes MAILER_FROM_EMAIL = "no-reply@example.com" - PASSWORD_RESET_TOKEN_EXPIRATION_IN_SECONDS = 10.minutes.to_i + PASSWORD_RESET_TOKEN_EXPIRATION = 10.minutes attr_accessor :current_password has_secure_password - has_secure_token :password_reset_token has_secure_token :remember_token has_secure_token :session_token @@ -58,9 +57,8 @@ def generate_confirmation_token signed_id expires_in: CONFIRMATION_TOKEN_EXPIRATION, purpose: :confirm_email end - def password_reset_token_has_expired? - return true if password_reset_sent_at.nil? - (Time.current - password_reset_sent_at) >= User::PASSWORD_RESET_TOKEN_EXPIRATION_IN_SECONDS + def generate_password_reset_token + signed_id expires_in: PASSWORD_RESET_TOKEN_EXPIRATION, purpose: :reset_password end def send_confirmation_email! @@ -69,9 +67,8 @@ def send_confirmation_email! end def send_password_reset_email! - regenerate_password_reset_token - update_columns(password_reset_sent_at: Time.current) - UserMailer.password_reset(self).deliver_now + password_reset_token = generate_password_reset_token + UserMailer.password_reset(self, password_reset_token).deliver_now end def reconfirming? diff --git a/app/views/passwords/edit.html.erb b/app/views/passwords/edit.html.erb index b9801b3..899edd5 100644 --- a/app/views/passwords/edit.html.erb +++ b/app/views/passwords/edit.html.erb @@ -1,4 +1,4 @@ -<%= form_with url: password_path(@user.password_reset_token), scope: :user, method: :put do |form| %> +<%= form_with url: password_path(params[:password_reset_token]), scope: :user, method: :put do |form| %>
<%= form.label :password %> <%= form.password_field :password, required: true %> diff --git a/app/views/user_mailer/password_reset.html.erb b/app/views/user_mailer/password_reset.html.erb index ad2c8e8..944c522 100644 --- a/app/views/user_mailer/password_reset.html.erb +++ b/app/views/user_mailer/password_reset.html.erb @@ -1,3 +1,3 @@

Password Reset Instructions

-<%= link_to "Click here to reset your password.", edit_password_url(@user.password_reset_token) %> \ No newline at end of file +<%= link_to "Click here to reset your password.", edit_password_url(@password_reset_token) %> \ No newline at end of file diff --git a/app/views/user_mailer/password_reset.text.erb b/app/views/user_mailer/password_reset.text.erb index 9e45a26..57bd0ad 100644 --- a/app/views/user_mailer/password_reset.text.erb +++ b/app/views/user_mailer/password_reset.text.erb @@ -1,3 +1,3 @@ Password Reset Instructions -<%= edit_password_url(@user.password_reset_token) %> \ No newline at end of file +<%= edit_password_url(@password_reset_token) %> \ No newline at end of file diff --git a/db/migrate/20211123101413_add_password_reset_token_to_users.rb b/db/migrate/20211123101413_add_password_reset_token_to_users.rb deleted file mode 100644 index f3f6890..0000000 --- a/db/migrate/20211123101413_add_password_reset_token_to_users.rb +++ /dev/null @@ -1,7 +0,0 @@ -class AddPasswordResetTokenToUsers < ActiveRecord::Migration[6.1] - def change - add_column :users, :password_reset_token, :string, null: false - add_column :users, :password_reset_sent_at, :datetime - add_index :users, :password_reset_token, unique: true - end -end diff --git a/db/schema.rb b/db/schema.rb index e4c8a8e..c1b52bc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -18,13 +18,10 @@ t.datetime "updated_at", precision: 6, null: false t.datetime "confirmed_at" t.string "password_digest", null: false - t.string "password_reset_token", null: false - t.datetime "password_reset_sent_at" t.string "unconfirmed_email" t.string "remember_token", null: false t.string "session_token", null: false t.index ["email"], name: "index_users_on_email", unique: true - t.index ["password_reset_token"], name: "index_users_on_password_reset_token", unique: true t.index ["remember_token"], name: "index_users_on_remember_token", unique: true t.index ["session_token"], name: "index_users_on_session_token", unique: true end diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb index f2439cf..b5bbb8a 100644 --- a/test/controllers/passwords_controller_test.rb +++ b/test/controllers/passwords_controller_test.rb @@ -6,24 +6,23 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest end test "should get edit" do - @confirmed_user.send_password_reset_email! + password_reset_token = @confirmed_user.generate_password_reset_token - get edit_password_path(@confirmed_user.password_reset_token) + get edit_password_path(password_reset_token) assert_response :ok end test "should redirect from edit if password link expired" do - @confirmed_user.send_password_reset_email! + password_reset_token = @confirmed_user.generate_password_reset_token travel_to 601.seconds.from_now - get edit_password_path(@confirmed_user.password_reset_token) + get edit_password_path(password_reset_token) assert_redirected_to new_password_path assert_not_nil flash[:alert] end test "should redirect from edit if password link is incorrect" do - travel_to 601.seconds.from_now get edit_password_path("not_a_real_token") assert_redirected_to new_password_path @@ -32,16 +31,20 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest test "should redirect from edit if user is not confirmed" do @confirmed_user.update!(confirmed_at: nil) - get edit_password_path(@confirmed_user.password_reset_token) + password_reset_token = @confirmed_user.generate_password_reset_token + + get edit_password_path(password_reset_token) assert_redirected_to new_confirmation_path assert_not_nil flash[:alert] end test "should redirect from edit if user is authenticated" do + password_reset_token = @confirmed_user.generate_password_reset_token + login @confirmed_user - get edit_password_path(@confirmed_user.password_reset_token) + get edit_password_path(password_reset_token) assert_redirected_to root_path end @@ -71,25 +74,23 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest end test "should update password" do - @confirmed_user.send_password_reset_email! + password_reset_token = @confirmed_user.generate_password_reset_token - assert_changes "@confirmed_user.reload.password_reset_token" do - put password_path(@confirmed_user.password_reset_token), params: { - user: { - password: "password", - password_confirmation: "password" - } + put password_path(password_reset_token), params: { + user: { + password: "password", + password_confirmation: "password" } - end + } assert_redirected_to login_path assert_not_nil flash[:notice] end test "should handle errors" do - @confirmed_user.send_password_reset_email! + password_reset_token = @confirmed_user.generate_password_reset_token - put password_path(@confirmed_user.password_reset_token), params: { + put password_path(password_reset_token), params: { user: { password: "password", password_confirmation: "password_that_does_not_match" @@ -100,9 +101,11 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest end test "should not update password if authenticated" do + password_reset_token = @confirmed_user.generate_password_reset_token + login @confirmed_user - put password_path(@confirmed_user.password_reset_token), params: { + put password_path(password_reset_token), params: { user: { password: "password", password_confirmation: "password" @@ -113,4 +116,4 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest get new_password_path assert_redirected_to root_path end -end +end \ No newline at end of file diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb index 6f0cf6a..9f9c9e0 100644 --- a/test/mailers/user_mailer_test.rb +++ b/test/mailers/user_mailer_test.rb @@ -15,10 +15,11 @@ class UserMailerTest < ActionMailer::TestCase end test "password_reset" do - mail = UserMailer.password_reset(@user) + password_reset_token = @user.generate_password_reset_token + mail = UserMailer.password_reset(@user, password_reset_token) assert_equal "Password Reset Instructions", mail.subject assert_equal [@user.email], mail.to assert_equal [User::MAILER_FROM_EMAIL], mail.from - assert_match @user.password_reset_token, mail.body.encoded + assert_match password_reset_token, mail.body.encoded end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 48882c0..e89660a 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -97,28 +97,10 @@ class UserTest < ActiveSupport::TestCase test "should respond to send_password_reset_email!" do @user.save! - original_password_reset_token = @user.password_reset_token - - freeze_time - - assert_nil @user.password_reset_sent_at assert_emails 1 do @user.send_password_reset_email! end - - assert_not_equal original_password_reset_token, @user.reload.password_reset_token - assert_equal Time.now, @user.password_reset_sent_at - end - - test "should respond to password_reset_token_has_expired?" do - assert @user.password_reset_token_has_expired? - - @user.password_reset_sent_at = 1.minute.ago - assert_not @user.password_reset_token_has_expired? - - @user.password_reset_sent_at = 601.seconds.ago - assert @user.password_reset_token_has_expired? end test "should downcase unconfirmed_email" do @@ -187,4 +169,18 @@ class UserTest < ActiveSupport::TestCase assert_not_nil @user.reload.session_token end + + test "should generate confirmation token" do + @user.save! + confirmation_token = @user.generate_confirmation_token + + assert_equal @user, User.find_signed(confirmation_token, purpose: :confirm_email) + end + + test "should generate password reset token" do + @user.save! + password_reset_token = @user.generate_password_reset_token + + assert_equal @user, User.find_signed(password_reset_token, purpose: :reset_password) + end end