From 107c2e62a8e6a4d40633460387595fa7c5abae56 Mon Sep 17 00:00:00 2001 From: Trevor Vallender Date: Sat, 19 Aug 2023 15:38:38 +0100 Subject: [PATCH] Email confirmation workflow --- .../email_confirmations_controller.rb | 13 +++++++ app/controllers/sessions_controller.rb | 18 ++++++++- app/controllers/users_controller.rb | 1 - app/mailers/application_mailer.rb | 2 +- app/mailers/user_mailer.rb | 8 ++++ app/models/user.rb | 13 +++++++ app/views/user_mailer/confirm_email.text.erb | 10 +++++ config/environments/development.rb | 37 +++++-------------- .../controllers/email_confirmations/en.yml | 4 ++ config/locales/controllers/sessions/en.yml | 1 + config/locales/controllers/users/en.yml | 2 +- config/locales/mailers/user/en.yml | 3 ++ config/routes.rb | 3 ++ ...8_add_email_confirmation_string_to_user.rb | 5 +++ db/schema.rb | 3 +- devenv.nix | 4 ++ .../email_confirmations_controller_test.rb | 16 ++++++++ test/fixtures/users.yml | 9 +++++ test/models/user_test.rb | 29 +++++++++++++++ test/system/new_session_test.rb | 16 +++++--- test/system/register_users_test.rb | 2 + 21 files changed, 160 insertions(+), 39 deletions(-) create mode 100644 app/controllers/email_confirmations_controller.rb create mode 100644 app/mailers/user_mailer.rb create mode 100644 app/views/user_mailer/confirm_email.text.erb create mode 100644 config/locales/controllers/email_confirmations/en.yml create mode 100644 config/locales/mailers/user/en.yml create mode 100644 db/migrate/20230818201128_add_email_confirmation_string_to_user.rb create mode 100644 test/controllers/email_confirmations_controller_test.rb diff --git a/app/controllers/email_confirmations_controller.rb b/app/controllers/email_confirmations_controller.rb new file mode 100644 index 0000000..e0197a0 --- /dev/null +++ b/app/controllers/email_confirmations_controller.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class EmailConfirmationsController < ApplicationController + def confirm + @user = User.find_by(email: params[:email]) + if params[:confirmation_string] == @user.email_confirmation_string + @user.update(email_confirmation_string: nil) + redirect_to new_session_path, notice: t(".email_confirmed") + else + redirect_to root_path, notice: t(".email_confirmation_failed") + end + end +end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 832a5b1..5b5d67f 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true class SessionsController < ApplicationController + before_action :set_user, only: [:create] + before_action :ensure_email_confirmed, only: [:create] + def new; end def create - @user = User.find_by(username: params[:username]) - if @user.present? && @user.authenticate(params[:password]) session[:user_id] = @user.id redirect_to @user, notice: t(".logged_in") @@ -19,4 +20,17 @@ class SessionsController < ApplicationController reset_session redirect_to root_path, notice: t(".logged_out") end + + private + + def set_user + @user = User.find_by(username: params[:username]) + end + + def ensure_email_confirmed + return unless @user.requires_confirmation? + + flash.alert = t(".account_not_confirmed") + redirect_to new_session_path + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5201e3c..87adaec 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -10,7 +10,6 @@ class UsersController < ApplicationController def create @user = User.new(user_params) if @user.save - # TODO: Add email confirmation workflow session[:user_id] = @user.id redirect_to root_path, notice: t(".account_created") else diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 3c34c81..1f6be61 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,4 +1,4 @@ class ApplicationMailer < ActionMailer::Base - default from: "from@example.com" + default from: "hello@summonplayer.com" layout "mailer" end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb new file mode 100644 index 0000000..f8b7769 --- /dev/null +++ b/app/mailers/user_mailer.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class UserMailer < ApplicationMailer + def confirm_email + @user = params[:user] + mail(to: @user.email, subject: t(".confirm_email")) + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 12cc701..0bb7824 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "securerandom" + class User < ApplicationRecord has_secure_password @@ -26,6 +28,8 @@ class User < ApplicationRecord presence: true, if: :validate_password? + after_create :require_confirmation + def to_param username end @@ -34,8 +38,17 @@ class User < ApplicationRecord "#{first_name} #{last_names}" end + def requires_confirmation? + email_confirmation_string.present? + end + private + def require_confirmation + update(email_confirmation_string: SecureRandom.uuid) + UserMailer.with(user: self).confirm_email.deliver_later + end + def validate_password? new_record? || password_digest_changed? end diff --git a/app/views/user_mailer/confirm_email.text.erb b/app/views/user_mailer/confirm_email.text.erb new file mode 100644 index 0000000..fba5a8f --- /dev/null +++ b/app/views/user_mailer/confirm_email.text.erb @@ -0,0 +1,10 @@ +Hi, + +Please visit the following link to confirm your email and access your new +account at Summon Player: + +<%= confirm_email_url(email: @user.email, confirmation_string: @user.email_confirmation_string) %> + +Thanks, + +The Summon Player team. diff --git a/config/environments/development.rb b/config/environments/development.rb index 685ef2b..838305c 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -3,20 +3,9 @@ require "active_support/core_ext/integer/time" Rails.application.configure do - # Settings specified here will take precedence over those in config/application.rb. - - # In the development environment your application's code is reloaded any time - # it changes. This slows down response time but is perfect for development - # since you don't have to restart the web server when you make code changes. config.cache_classes = false - - # Do not eager load code on boot. config.eager_load = false - - # Show full error reports. config.consider_all_requests_local = true - - # Enable server timing config.server_timing = true # Enable/disable caching. By default caching is disabled. @@ -27,7 +16,7 @@ Rails.application.configure do config.cache_store = :memory_store config.public_file_server.headers = { - "Cache-Control" => "public, max-age=#{2.days.to_i}" + "Cache-Control" => "public, max-age=#{2.days.to_i}", } else config.action_controller.perform_caching = false @@ -35,38 +24,30 @@ Rails.application.configure do config.cache_store = :null_store end - # Store uploaded files on the local file system (see config/storage.yml for options). config.active_storage.service = :local - # Don't care if the mailer can't send. - config.action_mailer.raise_delivery_errors = false - + config.action_mailer.raise_delivery_errors = true config.action_mailer.perform_caching = false - # Print deprecation notices to the Rails logger. config.active_support.deprecation = :log - - # Raise exceptions for disallowed deprecations. config.active_support.disallowed_deprecation = :raise - - # Tell Active Support which deprecation messages to disallow. config.active_support.disallowed_deprecation_warnings = [] - # Raise an error on page load if there are pending migrations. config.active_record.migration_error = :page_load - - # Highlight code that triggered database queries in logs. config.active_record.verbose_query_logs = true - # Suppress logger output for asset requests. config.assets.quiet = true # Raises error for missing translations. config.i18n.raise_on_missing_translations = true # Annotate rendered view with file names. - # config.action_view.annotate_rendered_view_with_filenames = true + config.action_view.annotate_rendered_view_with_filenames = true - # Uncomment if you wish to allow Action Cable access from any origin. - # config.action_cable.disable_request_forgery_protection = true + # Use Mailhog + config.action_mailer.perform_deliveries = true + config.action_mailer.smtp_settings = { + address: "localhost", + port: 1025, + } end diff --git a/config/locales/controllers/email_confirmations/en.yml b/config/locales/controllers/email_confirmations/en.yml new file mode 100644 index 0000000..067dd32 --- /dev/null +++ b/config/locales/controllers/email_confirmations/en.yml @@ -0,0 +1,4 @@ +en: + email_confirmations: + email_confirmed: Thanks, your email has been confirmed. + email_confirmation_failed: Sorry, we could not confirm your email. diff --git a/config/locales/controllers/sessions/en.yml b/config/locales/controllers/sessions/en.yml index 33c88c2..1fb6e01 100644 --- a/config/locales/controllers/sessions/en.yml +++ b/config/locales/controllers/sessions/en.yml @@ -3,3 +3,4 @@ en: logged_in: You’ve been logged in. login_fail: Sorry, we couldn’t log you in. logged_out: You have been logged out. + account_not_confirmed: Please confirm your email to log in. diff --git a/config/locales/controllers/users/en.yml b/config/locales/controllers/users/en.yml index 9356f9d..8794d84 100644 --- a/config/locales/controllers/users/en.yml +++ b/config/locales/controllers/users/en.yml @@ -1,6 +1,6 @@ en: users: create_failed: Could not create user account, please correct the errors below. - account_created: Account created! + account_created: Account created! Please check your email to confirm. account_updated: Your details have been updated. update_failed: Could not update your details. diff --git a/config/locales/mailers/user/en.yml b/config/locales/mailers/user/en.yml new file mode 100644 index 0000000..efbfcb7 --- /dev/null +++ b/config/locales/mailers/user/en.yml @@ -0,0 +1,3 @@ +en: + user_mailer: + confirm_email: Please confirm your email. diff --git a/config/routes.rb b/config/routes.rb index 37e1d9f..1626ad2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,9 +1,12 @@ # frozen_string_literal: true Rails.application.routes.draw do + default_url_options :host => "summonplayer.com" + root "sessions#new" resources :users, only: [:new, :create, :show, :edit, :update] resources :sessions, only: [:new, :create] delete "log_out", to: "sessions#destroy_session" + get "confirm_email", to: "email_confirmations#confirm" end diff --git a/db/migrate/20230818201128_add_email_confirmation_string_to_user.rb b/db/migrate/20230818201128_add_email_confirmation_string_to_user.rb new file mode 100644 index 0000000..c869575 --- /dev/null +++ b/db/migrate/20230818201128_add_email_confirmation_string_to_user.rb @@ -0,0 +1,5 @@ +class AddEmailConfirmationStringToUser < ActiveRecord::Migration[7.0] + def change + add_column :users, :email_confirmation_string, :uuid, default: nil + end +end diff --git a/db/schema.rb b/db/schema.rb index 671fc65..84348a8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_08_08_190158) do +ActiveRecord::Schema[7.0].define(version: 2023_08_18_201128) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -22,6 +22,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_08_190158) do t.string "email", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.uuid "email_confirmation_string" t.index ["email"], name: "index_users_on_email", unique: true t.index ["username"], name: "index_users_on_username", unique: true end diff --git a/devenv.nix b/devenv.nix index 3c7abaf..86cd9d5 100644 --- a/devenv.nix +++ b/devenv.nix @@ -25,4 +25,8 @@ CREATE ROLE summon_player WITH LOGIN PASSWORD 'postgres' SUPERUSER; ''; }; + + services.mailhog = { + enable = true; + }; } diff --git a/test/controllers/email_confirmations_controller_test.rb b/test/controllers/email_confirmations_controller_test.rb new file mode 100644 index 0000000..8789848 --- /dev/null +++ b/test/controllers/email_confirmations_controller_test.rb @@ -0,0 +1,16 @@ +require "test_helper" + +class EmailConfirmationsControllerTest < ActionDispatch::IntegrationTest + test "should confirm user account" do + user = users(:unconfirmed_user) + assert user.requires_confirmation? + get confirm_email_url( + email: user.email, + confirmation_string: user.email_confirmation_string, + ) + follow_redirect! + assert_includes @response.body, I18n.t("email_confirmations.email_confirmed") + user.reload + assert_not user.requires_confirmation? + end +end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 3916d86..3352c8a 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -4,3 +4,12 @@ user: first_name: Gimli last_names: son of Glóin email: gimli@example.com + +unconfirmed_user: + username: pippin + password_digest: <%= BCrypt::Password.create('tolkien-abercrombie-hobb-barker', cost: 5) %> + first_name: Peregrin + last_names: Took + email: pippin@example.com + email_confirmation_string: 5a68514b-7cd2-4ac8-bec1-22c2526f9a52 + diff --git a/test/models/user_test.rb b/test/models/user_test.rb index a99f01b..1ac14e8 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -1,9 +1,38 @@ +# frozen_string_literal: true + require "test_helper" class UserTest < ActiveSupport::TestCase + include ActionMailer::TestHelper + test "email must resemble an email" do user = users(:user) user.email = "foobar" assert_not user.valid? end + + test "a new user requires confirmation" do + user = User.create( + username: "gorkamorka", + email: "gorka@morka.com", + first_name: "Ork", + last_names: "Boyz", + password: "snotlings-are-for-squashing", + password_confirmation: "snotlings-are-for-squashing", + ) + assert user.requires_confirmation? + end + + test "a new user requests confirmation" do + assert_emails 1 do + User.create( + username: "gorkamorka", + email: "gorka@morka.com", + first_name: "Ork", + last_names: "Boyz", + password: "snotlings-are-for-squashing", + password_confirmation: "snotlings-are-for-squashing", + ) + end + end end diff --git a/test/system/new_session_test.rb b/test/system/new_session_test.rb index a46bdea..5b8ccaa 100644 --- a/test/system/new_session_test.rb +++ b/test/system/new_session_test.rb @@ -3,17 +3,23 @@ require "application_system_test_case" class SessionsTest < ApplicationSystemTestCase - setup do - @user = users(:user) - end - test "can log in and log out existing user" do + user = users(:user) visit new_session_path - fill_in "username", with: @user.username + fill_in "username", with: user.username fill_in "password", with: "tolkien-abercrombie-hobb-barker" click_button I18n.t("log_in") assert_text I18n.t("sessions.logged_in") click_link I18n.t("log_out") assert_text I18n.t("sessions.logged_out") end + + test "cannot log in as an unconfirmed user" do + user = users(:unconfirmed_user) + visit new_session_path + fill_in "username", with: user.username + fill_in "password", with: "tolkien-abercrombie-hobb-barker" + click_button I18n.t("log_in") + assert_text I18n.t("sessions.account_not_confirmed") + end end diff --git a/test/system/register_users_test.rb b/test/system/register_users_test.rb index e83755a..7f44d2a 100644 --- a/test/system/register_users_test.rb +++ b/test/system/register_users_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "application_system_test_case" class RegisterUsersTest < ApplicationSystemTestCase