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 1f60d8c
Show file tree
Hide file tree
Showing 25 changed files with 163 additions and 87 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
File renamed without changes.
2 changes: 2 additions & 0 deletions .github/workflows/github-actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ jobs:
- name: Setup DB
run: |
bundle exec rails db:test:prepare
- name: Assets Precompile
run: bundle exec rails assets:precompile
- name: Run tests
run: |
bundle exec rspec
30 changes: 18 additions & 12 deletions app/controllers/concerns/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,31 @@ 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 current_user = Current.user

def authenticate_user_from_session
User.find_by(id: session[:user_id])
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

if !user_signed_in?
redirect_to new_session_path, alert: t("controllers.concerns.authentication.unauthorized")
end
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."
2 changes: 1 addition & 1 deletion db/seeds.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
conference = Conference.find_or_create_by!(name: "RailsWorld 2024")

# Users
user = User.create!(email: "dev@example.com", password: "foobar", password_confirmation: "foobar")
user = User.create!(email: "dev@example.com", password: "foobar2024", password_confirmation: "foobar2024")

# Tags
Tag.create!(name: "Hotwire")
Expand Down
17 changes: 8 additions & 9 deletions spec/controllers/password_resets_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@
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

describe "GET #edit" do
let(:current) { instance_double(Current) }

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

context "with valid token" do
Expand All @@ -46,17 +46,16 @@
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

describe "PUT #update" do
let(:new_password) { "new_password" }
let(:current) { instance_double(Current) }

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

context "with valid params" do
Expand Down Expand Up @@ -108,7 +107,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
1 change: 0 additions & 1 deletion spec/controllers/registrations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
it "returns a success response" do
get :new
expect(response).to have_http_status(:ok)
assigns(:user).should be_a_new(User)
end
end

Expand Down
5 changes: 2 additions & 3 deletions spec/controllers/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
post :create, params: params
expect(session[:user_id]).to eq(user.id)
expect(response).to redirect_to(root_path)
assigns(:user).should eq(user)
end
end

Expand All @@ -43,8 +42,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
3 changes: 2 additions & 1 deletion 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 Expand Up @@ -58,5 +60,4 @@
config.filter_rails_from_backtrace!
# arbitrary gems may also be filtered via:
# config.filter_gems_from_backtrace("gem name")
config.include FactoryBot::Syntax::Methods
end
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
8 changes: 0 additions & 8 deletions spec/system/test_spec.rb

This file was deleted.

Loading

0 comments on commit 1f60d8c

Please sign in to comment.