Skip to content

Commit

Permalink
GIT-3086: Fixed the sessions persistent active state after a password…
Browse files Browse the repository at this point in the history
… update. (#3096)

+ TODO: Tests.

Co-authored-by: Ahmad Farhat <ahmad.af.farhat@gmail.com>
  • Loading branch information
KH-Amir-TN and farhatahmad committed Aug 9, 2022
1 parent 2793114 commit e22d047
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 11 deletions.
29 changes: 24 additions & 5 deletions app/controllers/application_controller.rb
Expand Up @@ -28,13 +28,32 @@ class ApplicationController < ActionController::Base
# Retrieves the current user.
def current_user
@current_user ||= User.includes(:role, :main_room).find_by(id: session[:user_id])

if Rails.configuration.loadbalanced_configuration && (@current_user && !@current_user.has_role?(:super_admin) &&
@current_user.provider != @user_domain)
@current_user = nil
session.clear
end

reset_session
return nil # This stops the session validation checks for loadbalanced configurations.
end
# For backward compatibility and for seamless integration with existing and running deployments:
# The active sessions will be declared as active on first interaction after the update.

# This keeps alive the already active sessions before the upgrade for accounts having no password updates.
session[:activated_at] ||= Time.zone.now.to_i if @current_user&.last_pwd_update.nil?

# Once a request is issued back to the server with a session that had been active before
# the last password update it will automatically get invalidated and the request will get
# redirected back to the root path.
# This solves #3086.
unless session[:activated_at].to_i >= @current_user&.last_pwd_update.to_i
# For backward compatibility and for seamless integration with existing and running deployments:
# The last_pwd_update attribute will default to nil and nil.to_i will always be 0.
# This with the activated_at fallback to the first connection after the upgrade will result in
# keeping alive old sessions and ensuring a seamless intergation.
# In cases where the account has a password update after the upgrade, all of old the active sessions
# which haven't updated their state and all the other active updated sessions before the password
# update event will be cought and declared as invalid where users will get unauthenticated and redirected to root path.
reset_session
redirect_to root_path, flash: { alert: I18n.t("session.expired") } and return
end
@current_user
end
helper_method :current_user
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/concerns/authenticator.rb
Expand Up @@ -24,10 +24,9 @@ def login(user)
migrate_twitter_user(user)

session[:user_id] = user.id
user.update(last_login: Time.zone.now)

# Upon login the session metadata activated_at has to match to login event timestamp.
session[:activated_at] = user.last_login.to_i if user.without_terms_acceptance { user.update(last_login: Time.zone.now) }
logger.info("Support: #{user.email} has successfully logged in.")

# If there are not terms, or the user has accepted them, check for email verification
if !Rails.configuration.terms || user.accepted_terms
check_email_verified(user)
Expand Down
8 changes: 6 additions & 2 deletions app/controllers/password_resets_controller.rb
Expand Up @@ -57,12 +57,16 @@ def update
# Clear the user's social uid if they are switching from a social to a local account
@user.update_attribute(:social_uid, nil) if @user.social_uid.present?
# Deactivate the reset digest in use disabling the reset link.
@user.update(reset_digest: nil, reset_sent_at: nil)
@user.update(reset_digest: nil, reset_sent_at: nil, last_pwd_update: Time.zone.now)
# For password resets the last_pwd_update has to match the resetting event timestamp.
# And the activated_at session metadata has to match it only if the authenticated user
# is the user with the account having its password reset.
# This keeps that user session only alive while invalidating all others for the same account.
session[:activated_at] = @user.last_pwd_update.to_i if current_user&.id == @user.id
}
# Successfully reset password
return redirect_to root_path, flash: { success: I18n.t("password_reset_success") }
end

render 'edit'
end

Expand Down
9 changes: 8 additions & 1 deletion app/controllers/users_controller.rb
Expand Up @@ -125,7 +125,14 @@ def update_password
end

# Notify the user that their account has been updated.
if @user.errors.empty? && @user.without_terms_acceptance { @user.save }
if @user.errors.empty? && @user.without_terms_acceptance {
@user.save && @user.update(last_pwd_update: Time.zone.now)
}
# Changing the password has to update the last_pwd_update to match the event timestamp.
# The activated_at session metadata has to match it too to reset the state of the active session
# making this change password request.
# This keeps only this session alive.
session[:activated_at] = @user.last_pwd_update.to_i if current_user.id == @user.id
return redirect_to change_password_path, flash: { success: I18n.t("info_update_success") }
end
# redirect_to change_password_path
Expand Down
2 changes: 2 additions & 0 deletions config/locales/en.yml
Expand Up @@ -683,3 +683,5 @@ en:
signin: Please sign in to access your account.
title: Verify your email
verification: Verification
session:
expired: Session expired (Please sign in again.).
7 changes: 7 additions & 0 deletions db/migrate/20220119203008_add_last_pwd_update_to_users.rb
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class AddLastPwdUpdateToUsers < ActiveRecord::Migration[5.2]
def change
add_column :users, :last_pwd_update, :datetime
end
end
1 change: 1 addition & 0 deletions db/schema.rb
Expand Up @@ -146,6 +146,7 @@
t.datetime "last_login"
t.integer "failed_attempts"
t.datetime "last_failed_attempt"
t.datetime "last_pwd_update"
t.index ["created_at"], name: "index_users_on_created_at"
t.index ["deleted"], name: "index_users_on_deleted"
t.index ["email"], name: "index_users_on_email"
Expand Down

0 comments on commit e22d047

Please sign in to comment.