From bd5c6ae6bbb35bd446d50224e6024218931d9ddf Mon Sep 17 00:00:00 2001
From: songtianlun <tianlun.song@foxmail.com>
Date: Thu, 2 Jan 2025 11:59:27 +0800
Subject: [PATCH] feat: enhance user session management

- Update session creation to use safe navigation operator
- Implement log_out method in SessionsHelper
- Add session reset and login on user creation
- Improve user login tests for better coverage

These changes improve the user session management by ensuring
that the session is handled more safely and efficiently. The
addition of the log_out method centralizes session termination,
while the updated tests ensure that both login and logout
functionality are thoroughly validated.
---
 app/controllers/sessions_controller.rb |  5 +++-
 app/controllers/users_controller.rb    |  3 +++
 app/helpers/sessions_helper.rb         |  6 +++++
 app/models/user.rb                     |  6 +++++
 app/views/layouts/_header.html.erb     |  4 +--
 test/fixtures/users.yml                |  5 ++++
 test/integration/users_login_test.rb   | 37 ++++++++++++++++++++++++++
 test/integration/users_signup_test.rb  |  1 +
 test/test_helper.rb                    |  4 +++
 9 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index dbdb886..d7101f6 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -5,7 +5,8 @@ class SessionsController < ApplicationController
 
   def create
     user = User.find_by(email: params[:session][:email].downcase)
-    if user && user.authenticate(params[:session][:password])
+    # if user && user.authenticate(params[:session][:password])
+    if user&.authenticate(params[:session][:password])
       reset_session
       log_in user
       redirect_to user
@@ -16,5 +17,7 @@ class SessionsController < ApplicationController
   end
 
   def destroy
+    log_out
+    redirect_to root_url
   end
 end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 367fdd3..bc85b72 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,4 +1,5 @@
 class UsersController < ApplicationController
+  include SessionsHelper
   def show
     @user = User.find(params[:id])
     # debugger
@@ -11,6 +12,8 @@ class UsersController < ApplicationController
   def create
     @user = User.new(user_params)
     if @user.save
+      reset_session
+      log_in @user
       flash[:success] = "Welcome to the Sample App!"
       redirect_to @user
       # redirect_to user_url(@user)
diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb
index f81bc8e..6deb590 100644
--- a/app/helpers/sessions_helper.rb
+++ b/app/helpers/sessions_helper.rb
@@ -12,4 +12,10 @@ module SessionsHelper
   def logged_in?
     !current_user.nil?
   end
+
+  def log_out
+    reset_session
+    @current_user = nil
+  end
+
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6c288ab..483ea4a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -8,4 +8,10 @@ class User < ApplicationRecord
             uniqueness:  true
   has_secure_password
   validates :password, presence: true, length: { minimum: 6 }
+
+  def User.digest(string)
+    cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST :
+             BCrypt::Engine.cost
+    BCrypt::Password.create(string, cost: cost)
+  end
 end
diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb
index 335d2b9..c5aea77 100644
--- a/app/views/layouts/_header.html.erb
+++ b/app/views/layouts/_header.html.erb
@@ -9,8 +9,8 @@
                 aria-expanded="false">
           <span class="sr-only">Toggle navigation</span>
           <span class="icon-bar"></span>
-          <span class="icon-bar>"></span>
-          <span class="icon-bar>"></span>
+          <span class="icon-bar"></span>
+          <span class="icon-bar"></span>
         </button>
       </div>
       <ul class="nav navbar-nav navbar-right collapse navbar-collapse"
diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml
index 46789ac..ff617e0 100644
--- a/test/fixtures/users.yml
+++ b/test/fixtures/users.yml
@@ -3,3 +3,8 @@
 one:
   name: MyString
   email: MyString
+
+michael:
+  name: Michael Example
+  email: michael@example.com
+  password_digest: <%= User.digest('password') %>
\ No newline at end of file
diff --git a/test/integration/users_login_test.rb b/test/integration/users_login_test.rb
index 8df72c1..522f58c 100644
--- a/test/integration/users_login_test.rb
+++ b/test/integration/users_login_test.rb
@@ -1,6 +1,22 @@
 require "test_helper"
 
 class UsersLoginTest < ActionDispatch::IntegrationTest
+  def setup
+    @user = users(:michael)
+  end
+
+  test "login with valid email/invalid password" do
+    get login_path
+    assert_template 'sessions/new'
+    post login_path, params: { session: { email: @user.email,
+                                          password: "invalid" } }
+    assert_not is_logged_in?
+    assert_template 'sessions/new'
+    assert_not flash.empty?
+    get root_path
+    assert flash.empty?
+  end
+
   test "login with invalid information" do
     get login_path
     assert_template 'sessions/new'
@@ -11,4 +27,25 @@ class UsersLoginTest < ActionDispatch::IntegrationTest
     get root_path
     assert flash.empty?
   end
+
+  test "login with valid information followed by logout" do
+    get login_path
+    post login_path, params: { session: { email:
+                                            @user.email,
+                                          password: 'password' } }
+    assert is_logged_in?
+    assert_redirected_to @user
+    follow_redirect!
+    assert_template 'users/show'
+    assert_select "a[href=?]", login_path, count: 0
+    assert_select "a[href=?]", logout_path
+    assert_select "a[href=?]", user_path(@user)
+    delete logout_path
+    assert_not is_logged_in?
+    assert_redirected_to root_url
+    follow_redirect!
+    assert_select "a[href=?]", login_path
+    assert_select "a[href=?]", logout_path, count: 0
+    assert_select "a[href=?]", user_path(@user), count: 0
+  end
 end
diff --git a/test/integration/users_signup_test.rb b/test/integration/users_signup_test.rb
index f18a70b..159e452 100644
--- a/test/integration/users_signup_test.rb
+++ b/test/integration/users_signup_test.rb
@@ -25,5 +25,6 @@ class UsersSignupTest < ActionDispatch::IntegrationTest
     follow_redirect!
     assert_template 'users/show'
     assert_not flash.notice
+    assert is_logged_in?
   end
 end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index e10ed8c..45f3e7e 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -13,6 +13,10 @@ module ActiveSupport
     fixtures :all
     include ApplicationHelper
 
+    def is_logged_in?
+      !session[:user_id].nil?
+    end
+
     # Add more helper methods to be used by all tests here...
   end
 end