Merge pull request #26 from TelosLabs/improvements-authentication

Improvements | Authentication

.env.template

@@ -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

.github/workflows/github-actions.yml

@@ -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

app/controllers/application_controller.rb

@@ -1,5 +1,3 @@
 class ApplicationController < ActionController::Base
   include Authentication
-
-  before_action :authenticate_user!
 end

app/controllers/concerns/authentication.rb

@@ -4,31 +4,41 @@ module Authentication
   included do
     helper_method :current_user, :user_signed_in?
 
-    def authenticate_user!
-      redirect_to root_path, alert: t("controllers.concerns.authentication.unauthorized") unless user_signed_in?
-    end
+    before_action :authenticate_user, :authenticate_user!
+  end
 
-    def current_user
-      Current.user ||= authenticate_user_from_session
+  class_methods do
+    def allow_unauthenticated_access(**options)
+      skip_before_action :authenticate_user!, **options
     end
+  end
 
-    def authenticate_user_from_session
-      User.find_by(id: session[:user_id])
-    end
+  private
 
-    def user_signed_in?
-      current_user.present?
-    end
+  def current_user = Current.user
 
-    def login(user)
-      Current.user = user
-      reset_session
-      session[:user_id] = user.id
-    end
+  def user_signed_in? = Current.user.present?
 
-    def logout
-      Current.user = nil
-      reset_session
+  def authenticate_user
+    Current.user = User.find_by(id: session[:user_id])
+  end
+
+  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)
+    Current.user = user
+    reset_session
+    session[:user_id] = user.id
+  end
+
+  def logout
+    Current.user = nil
+    reset_session
+  end
 end

app/controllers/main_controller.rb

@@ -1,5 +1,6 @@
 class MainController < ApplicationController
-  skip_before_action :authenticate_user!
+  allow_unauthenticated_access
+
   def index
   end
 end

app/controllers/password_resets_controller.rb

@@ -1,5 +1,6 @@
 class PasswordResetsController < ApplicationController
-  skip_before_action :authenticate_user!, only: [:new, :create]
+  allow_unauthenticated_access
+
   before_action :set_user_by_token, only: [:edit, :update]
 
   def new
@@ -15,10 +16,10 @@ def create
       PasswordMailer.with(
         user: @user,
         token: @user.generate_token_for(:password_reset)
-      ).password_reset.deliver_now
+      ).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
@@ -33,8 +34,9 @@ def update
 
   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") if @user.blank?
+    redirect_to new_password_reset_path, alert: t("controllers.password_resets.errors.invalid_token")
   end
 
   def password_params

app/controllers/passwords_controller.rb

@@ -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

app/controllers/registrations_controller.rb

@@ -1,5 +1,5 @@
 class RegistrationsController < ApplicationController
-  skip_before_action :authenticate_user!
+  allow_unauthenticated_access
 
   def new
     @user = User.new

app/controllers/sessions_controller.rb

@@ -1,7 +1,8 @@
 class SessionsController < ApplicationController
-  skip_before_action :authenticate_user!
+  allow_unauthenticated_access only: [:new, :create]
 
   def new
+    @user = User.new
   end
 
   def create
@@ -11,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
 

app/models/user.rb

@@ -6,6 +6,7 @@
 #  email                        :string           not null
 #  in_app_notifications_enabled :boolean          default(TRUE), not null
 #  mail_notifications_enabled   :boolean          default(TRUE), not null
+#  password_digest              :string           not null
 #  role                         :string
 #  created_at                   :datetime         not null
 #  updated_at                   :datetime         not null
@@ -15,20 +16,19 @@
 #  index_users_on_email  (email) UNIQUE
 #
 class User < ApplicationRecord
-  PASSWORD_RESET_EXPIRATION = 15.minutes
+  PASSWORD_RESET_EXPIRATION = 60.minutes
 
   normalizes :email, with: ->(email) { email.strip.downcase }
 
+  has_secure_password
+
   has_one :profile, as: :profileable, dependent: :destroy
 
   has_and_belongs_to_many :events
 
   validates :email, presence: true, uniqueness: true
   validates :password_digest, presence: true
-
-  normalizes :email, with: ->(email) { email.strip.downcase }
-
-  has_secure_password
+  validates :password, length: {minimum: 8}, if: -> { password.present? }
 
   generates_token_for :password_reset, expires_in: PASSWORD_RESET_EXPIRATION do
     password_salt&.last(10)

app/views/passwords/edit.html.erb

@@ -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>

app/views/registrations/new.html.erb

@@ -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 %>

app/views/sessions/new.html.erb

@@ -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 %>
 

config/locales/en.yml

@@ -1,50 +1,21 @@
-# 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."
       update:
         notice: "Your password has been reset successfully. Please login."
       errors:
-        invalid_token: "Invalid token, please try again"
+        invalid_token: "Invalid token, please try again."
     passwords:
       update:
         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."

db/seeds.rb

@@ -1,7 +1,7 @@
-conference = Conference.create!(name: "RailsWorld 2024")
+conference = Conference.find_or_create_by!(name: "RailsWorld 2024")
 
 # Users
-user = User.create!(email: "dev@example.com")
+user = User.create!(email: "dev@example.com", password: "foobar2024", password_confirmation: "foobar2024")
 
 # Tags
 Tag.create!(name: "Hotwire")

spec/controllers/password_resets_controller_spec.rb

@@ -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
@@ -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
@@ -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

spec/controllers/passwords_controller_spec.rb

@@ -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

spec/controllers/registrations_controller_spec.rb

@@ -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