From e22d0476b6e78a710a1550a95bbf598aa51b14c1 Mon Sep 17 00:00:00 2001 From: Khemissi Amir Date: Tue, 9 Aug 2022 20:17:44 +0100 Subject: [PATCH] GIT-3086: Fixed the sessions persistent active state after a password update. (#3096) + TODO: Tests. Co-authored-by: Ahmad Farhat --- app/controllers/application_controller.rb | 29 +++++++++++++++---- app/controllers/concerns/authenticator.rb | 5 ++-- app/controllers/password_resets_controller.rb | 8 +++-- app/controllers/users_controller.rb | 9 +++++- config/locales/en.yml | 2 ++ ...0119203008_add_last_pwd_update_to_users.rb | 7 +++++ db/schema.rb | 1 + 7 files changed, 50 insertions(+), 11 deletions(-) create mode 100644 db/migrate/20220119203008_add_last_pwd_update_to_users.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 65caa1399a..a362daf9a7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -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 diff --git a/app/controllers/concerns/authenticator.rb b/app/controllers/concerns/authenticator.rb index 64ad144518..7259cf5830 100644 --- a/app/controllers/concerns/authenticator.rb +++ b/app/controllers/concerns/authenticator.rb @@ -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) diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb index 46667f3e18..37d39e97a9 100644 --- a/app/controllers/password_resets_controller.rb +++ b/app/controllers/password_resets_controller.rb @@ -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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d95117ea51..db5acf3631 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 8cfb76d04b..0684d66622 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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.). diff --git a/db/migrate/20220119203008_add_last_pwd_update_to_users.rb b/db/migrate/20220119203008_add_last_pwd_update_to_users.rb new file mode 100644 index 0000000000..4e4f75a1c1 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index d45c46839b..1ec1af443a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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"