Skip to content

Commit

Permalink
Authentication improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergio-e committed Jul 12, 2024
1 parent 461df42 commit 61e2373
Show file tree
Hide file tree
Showing 20 changed files with 155 additions and 70 deletions.
1 change: 1 addition & 0 deletions .env.template
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
OVERMIND_PROCFILE=Procfile.dev
HIVEMIND_PROCFILE=Procfile.dev
PORT=3000
OVERMIND_ENV=./.env.local
HIVEMIND_ENV=./.env.local
RSPEC_RETRY_RETRY_COUNT=0
27 changes: 15 additions & 12 deletions app/controllers/concerns/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,28 @@ module Authentication
included do
helper_method :current_user, :user_signed_in?

before_action :authenticate_user!
before_action :authenticate_user, :authenticate_user!
end

def authenticate_user!
return current_user if user_signed_in?

redirect_to root_path, alert: t("controllers.concerns.authentication.unauthorized")
class_methods do
def allow_unauthenticated_access(**options)
skip_before_action :authenticate_user!, **options
end
end

def current_user
Current.user ||= authenticate_user_from_session
end
private

def authenticate_user_from_session
User.find_by(id: session[:user_id])
def current_user = Current.user

def user_signed_in? = Current.user.present?

def authenticate_user
Current.user = User.find_by(id: session[:user_id])
end

def user_signed_in?
current_user.present?
def authenticate_user!
authenticate_user ||
redirect_to(new_session_path, alert: t("controllers.concerns.authentication.unauthorized"))
end

def login(user)
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/main_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class MainController < ApplicationController
skip_before_action :authenticate_user!
allow_unauthenticated_access

def index
end
end
6 changes: 3 additions & 3 deletions app/controllers/password_resets_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class PasswordResetsController < ApplicationController
skip_before_action :authenticate_user!, only: [:new, :create]
allow_unauthenticated_access

before_action :set_user_by_token, only: [:edit, :update]

Expand All @@ -19,7 +19,7 @@ def create
).password_reset.deliver_later
end

redirect_to root_path, notice: t("controllers.password_resets.create.notice")
redirect_to new_session_path, notice: t("controllers.password_resets.create.notice")
end

def update
Expand All @@ -36,7 +36,7 @@ def set_user_by_token
@user = User.find_by_token_for(:password_reset, params[:token])
return if @user.present?

redirect_to new_password_reset_path alert: t("controllers.password_resets.errors.invalid_token")
redirect_to new_password_reset_path, alert: t("controllers.password_resets.errors.invalid_token")
end

def password_params
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ def edit
end

def update
if current_user.update(password_params)
if Current.user.update(password_params)
redirect_to edit_password_path, notice: t("controllers.passwords.update.notice")
else
render :edit, status: :unprocessable_entity
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/registrations_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class RegistrationsController < ApplicationController
skip_before_action :authenticate_user!
allow_unauthenticated_access

def new
@user = User.new
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class SessionsController < ApplicationController
skip_before_action :authenticate_user!
allow_unauthenticated_access only: [:new, :create]

def new
@user = User.new
Expand All @@ -12,8 +12,7 @@ def create
login @user
redirect_to root_path, notice: t("controllers.sessions.create.notice")
else
flash[:alert] = t("controllers.sessions.create.alert")
render :new, status: :unprocessable_entity
redirect_to new_session_path, alert: t("controllers.sessions.create.alert")
end
end

Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class User < ApplicationRecord

validates :email, presence: true, uniqueness: true
validates :password_digest, presence: true
validates :password, length: {minimum: 8}, if: -> { password.present? }

generates_token_for :password_reset, expires_in: PASSWORD_RESET_EXPIRATION do
password_salt&.last(10)
Expand Down
2 changes: 1 addition & 1 deletion app/views/passwords/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<h1>Update Password</h1>

<%= form_with model: current_user, url: password_path do |form| %>
<%= form_with model: Current.user, url: password_path do |form| %>
<% if form.object.errors.any? %>
<% form.object.errors.full_messages.each do |message| %>
<div><%= message %></div>
Expand Down
8 changes: 4 additions & 4 deletions app/views/registrations/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,20 @@

<div>
<%= form.label :email %>
<%= form.email_field :email %>
<%= form.email_field :email, data: { test_id: "email_field" } %>
</div>

<div>
<%= form.label :password %>
<%= form.password_field :password %>
<%= form.password_field :password, data: { test_id: "password_field" } %>
</div>

<div>
<%= form.label :password_confirmation %>
<%= form.password_field :password_confirmation %>
<%= form.password_field :password_confirmation, data: { test_id: "password_confirmation_field" } %>
</div>

<div>
<%= form.submit "Sign Up" %>
<%= form.submit "Sign Up", data: { test_id: "sign_up_button" } %>
</div>
<% end %>
6 changes: 3 additions & 3 deletions app/views/sessions/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
<%= form_with model: @user, url: session_path do |form| %>
<div>
<%= form.label :email %>
<%= form.email_field :email %>
<%= form.email_field :email, data: { test_id: "email_field" } %>
</div>

<div>
<%= form.label :password %>
<%= form.password_field :password %>
<%= form.password_field :password, data: { test_id: "password_field" } %>
</div>

<div>
<%= form.submit "Log in" %>
<%= form.submit "Log in", data: { test_id: "sign_in_button" } %>
</div>
<% end %>

Expand Down
35 changes: 3 additions & 32 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
@@ -1,37 +1,8 @@
# Files in the config/locales directory are used for internationalization and
# are automatically loaded by Rails. If you want to use locales other than
# English, add the necessary files in this directory.
#
# To use the locales, use `I18n.t`:
#
# I18n.t "hello"
#
# In views, this is aliased to just `t`:
#
# <%= t("hello") %>
#
# To use a different locale, set it with `I18n.locale`:
#
# I18n.locale = :es
#
# This would use the information in config/locales/es.yml.
#
# To learn more about the API, please read the Rails Internationalization guide
# at https://guides.rubyonrails.org/i18n.html.
#
# Be aware that YAML interprets the following case-insensitive strings as
# booleans: `true`, `false`, `on`, `off`, `yes`, `no`. Therefore, these strings
# must be quoted to be interpreted as strings. For example:
#
# en:
# "yes": yup
# enabled: "ON"

en:
controllers:
concerns:
authentication:
unauthorized: "You must be logged in to do that."
unauthorized: "You need to sign in or sign up before continuing."
password_resets:
create:
notice: "Check your email to reset your password."
Expand All @@ -44,7 +15,7 @@ en:
notice: "Your password has been updated successfully."
sessions:
create:
notice: "You have signed successfully."
notice: "Signed in successfully."
alert: "Invalid email or password."
destroy:
notice: "You have been logged out."
notice: "You have been logged out."
10 changes: 6 additions & 4 deletions spec/controllers/password_resets_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
end

it "sends a password reset email" do
expect { post :create, params: params }.to change { ActionMailer::Base.deliveries.count }.by(1)
expect(response).to redirect_to(root_path)
expect {
post :create, params: params
}.to have_enqueued_mail(PasswordMailer, :password_reset)
expect(response).to redirect_to(new_session_path)
end
end
end
Expand All @@ -46,7 +48,7 @@
context "with invalid token" do
it "redirects to new password reset path" do
get :edit, params: {token: "invalid_token"}
expect(response).to redirect_to(new_password_reset_path(alert: I18n.t("controllers.password_resets.errors.invalid_token")))
expect(response).to redirect_to(new_password_reset_path)
end
end
end
Expand Down Expand Up @@ -108,7 +110,7 @@

it "does not update the user password" do
expect { put :update, params: params }.not_to change { user.reload.password_digest }
expect(response).to redirect_to(new_password_reset_path(alert: I18n.t("controllers.password_resets.errors.invalid_token")))
expect(response).to redirect_to(new_password_reset_path)
end
end
end
Expand Down
3 changes: 1 addition & 2 deletions spec/controllers/passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@

RSpec.describe PasswordsController, type: :controller do
let!(:user) { create(:user, password: "password") }
let(:current) { instance_double(Current) }

before do
allow(Current).to receive(:user).and_return(user)
sign_in(user)
end

describe "GET #edit" do
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@

it "does not create a User session" do
expect { post :create, params: params }.not_to change(User, :count)
expect(response).to have_http_status(:unprocessable_content)
expect(response).to render_template(:new)
expect(response).to have_http_status(:found)
expect(response).to redirect_to(new_session_path)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
FactoryBot.define do
factory :user do
email { Faker::Internet.email }
password { "password" }
password { "password2024" }

trait :with_profile do
profile
Expand Down
2 changes: 2 additions & 0 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
abort e.to_s.strip
end
RSpec.configure do |config|
ActiveJob::Base.queue_adapter = :test

config.include FactoryBot::Syntax::Methods
# If you're not using ActiveRecord, or you'd prefer not to run each of your
# examples within a transaction, remove the following line or assign false
Expand Down
17 changes: 17 additions & 0 deletions spec/support/authentication_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module AuthenticationHelper
def sign_in(user, password = "password2024")
if respond_to?(:visit) # System specs
visit new_session_path
find_dti("email_field").set(user.email)
find_dti("password_field").set(password)
find_dti("sign_in_button").click
else # Controller specs
session[:user_id] = user.id
end
end
end

RSpec.configure do |config|
config.include AuthenticationHelper, type: :system
config.include AuthenticationHelper, type: :controller
end
37 changes: 37 additions & 0 deletions spec/system/user_sign_in_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
require "rails_helper"

RSpec.describe "User sign in", type: :system do
let!(:user) { create(:user, email: "[email protected]", password: "foobar2024") }

it "redirects to the login page when trying to access another page" do
visit edit_password_path
expect(page).to have_current_path(new_session_path)
expect(page).to have_content("You need to sign in or sign up before continuing.")
end

context "when the user inputs invalid credentials" do
it "does not sign the user in" do
visit new_session_path
find_dti("email_field").set("[email protected]")
find_dti("password_field").set("wrongpassword")
find_dti("sign_in_button").click
expect(page).to have_content("Invalid email or password.")

visit edit_password_path
expect(page).to have_current_path(new_session_path)
end
end

context "when the user inputs valid credentials" do
it "signs the user in" do
visit new_session_path
find_dti("email_field").set("[email protected]")
find_dti("password_field").set("foobar2024")
find_dti("sign_in_button").click
expect(page).to have_content("Signed in successfully.")

visit edit_password_path
expect(page).to have_current_path(edit_password_path)
end
end
end
Loading

0 comments on commit 61e2373

Please sign in to comment.