diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 1ebb061239c..88a2ce0a276 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -351,7 +351,7 @@ class InvitesController < ApplicationController if invite.present? begin - attrs = { ip_address: request.remote_ip, session: server_session } + attrs = { ip_address: request.remote_ip, session: session } if redeeming_user attrs[:redeeming_user] = redeeming_user @@ -532,8 +532,8 @@ class InvitesController < ApplicationController # Show email if the user already authenticated their email different_external_email = false - if server_session[:authentication] - auth_result = Auth::Result.from_session_data(server_session[:authentication], user: nil) + if session[:authentication] + auth_result = Auth::Result.from_session_data(session[:authentication], user: nil) if invite.email == auth_result.email email = invite.email else diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 1841f269f1b..16c0de8b7df 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -240,9 +240,9 @@ class SessionController < ApplicationController topic = invite.topics.first return_path = topic.present? ? path(topic.relative_url) : path("/") elsif !user.active? - activation = UserActivator.new(user, request, server_session, cookies) + activation = UserActivator.new(user, request, session, cookies) activation.finish - server_session["user_created_message"] = activation.message + session["user_created_message"] = activation.message return redirect_to(users_account_created_path) else login_sso_user(sso, user) @@ -891,7 +891,7 @@ class SessionController < ApplicationController username: sso.username, name: sso.name, ip_address: request.remote_ip, - session: server_session, + session: session, email: sso.email, redeeming_user: redeeming_user, ).redeem diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index f144a1be424..4281e60d1ff 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -149,7 +149,7 @@ class Users::OmniauthCallbacksController < ApplicationController elsif invite_required? @auth_result.requires_invite = true else - server_session[:authentication] = @auth_result.session_data + session[:authentication] = @auth_result.session_data end end @@ -207,7 +207,7 @@ class Users::OmniauthCallbacksController < ApplicationController log_on_user(user, { authenticated_with_oauth: true }) Invite.invalidate_for_email(user.email) # invite link can't be used to log in anymore - server_session.delete(:authentication) # don't carry around old auth info + session[:authentication] = nil # don't carry around old auth info, perhaps move elsewhere @auth_result.authenticated = true else if SiteSetting.must_approve_users? && !user.approved? @@ -221,7 +221,7 @@ class Users::OmniauthCallbacksController < ApplicationController def persist_auth_token(auth) secret = SecureRandom.hex key = Users::AssociateAccountsController.key(secret) - server_session.set(key, auth.to_json, expires: 10.minutes) + server_session.set key, auth.to_json, expires: 10.minutes "#{Discourse.base_path}/associate/#{secret}" end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5a6bb22737e..470e948c498 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -740,7 +740,7 @@ class UsersController < ApplicationController end end - authentication = UserAuthenticator.new(user, server_session) + authentication = UserAuthenticator.new(user, session) if !authentication.has_authenticator? && !SiteSetting.enable_local_logins && !(current_user&.admin? && is_api?) @@ -754,7 +754,7 @@ class UsersController < ApplicationController return fail_with("login.incorrect_username_email_or_password") end - activation = UserActivator.new(user, request, server_session, cookies) + activation = UserActivator.new(user, request, session, cookies) activation.start if user.save @@ -767,7 +767,7 @@ class UsersController < ApplicationController server_session.delete(CHALLENGE_KEY) # save user email in session, to show on account-created page - server_session["user_created_message"] = activation.message + session["user_created_message"] = activation.message session[SessionController::ACTIVATE_USER_KEY] = user.id # If the user was created as active this will @@ -780,7 +780,7 @@ class UsersController < ApplicationController ) elsif SiteSetting.hide_email_address_taken && user.errors[:primary_email]&.include?(I18n.t("errors.messages.taken")) - server_session["user_created_message"] = activation.success_message + session["user_created_message"] = activation.success_message existing_user = User.find_by_email(user.primary_email&.email) if !existing_user && SiteSetting.normalize_emails @@ -1091,7 +1091,7 @@ class UsersController < ApplicationController end @custom_body_class = "static-account-created" - @message = server_session["user_created_message"] || I18n.t("activation.missing_session") + @message = session["user_created_message"] || I18n.t("activation.missing_session") @account_created = { message: @message, show_controls: false } if session_user_id = session[SessionController::ACTIVATE_USER_KEY] diff --git a/app/services/user_activator.rb b/app/services/user_activator.rb index 2e8c721cc7f..1e611235b1e 100644 --- a/app/services/user_activator.rb +++ b/app/services/user_activator.rb @@ -29,7 +29,7 @@ class UserActivator end def factory - invite = Invite.find_by(email: Email.downcase(user.email)) + invite = Invite.find_by(email: Email.downcase(@user.email)) if !user.active? EmailActivator @@ -67,7 +67,7 @@ class LoginActivator < UserActivator include CurrentUser def activate - log_on_user(user, { authenticated_with_oauth: session["authenticated_with_oauth"] }) + log_on_user(user, { authenticated_with_oauth: @session["authenticated_with_oauth"] }) user.enqueue_welcome_message("welcome_user") success_message end diff --git a/app/services/user_authenticator.rb b/app/services/user_authenticator.rb index 64596b1111e..2a91ff2d64e 100644 --- a/app/services/user_authenticator.rb +++ b/app/services/user_authenticator.rb @@ -9,7 +9,7 @@ class UserAuthenticator ) @user = user @session = session - if session.try(:[], :authentication).is_a?(Hash) + if session&.dig(:authentication) && session[:authentication].is_a?(Hash) @auth_result = Auth::Result.from_session_data(session[:authentication], user: user) end @authenticator_finder = authenticator_finder @@ -35,9 +35,8 @@ class UserAuthenticator authenticator.after_create_account(@user, @auth_result) confirm_email end - if @session.try(:[], :authentication) - @session.delete(:authentication) - @auth_result = nil + if @session&.dig(:authentication) + @session[:authentication] = @auth_result = nil @session[:authenticated_with_oauth] = true end end diff --git a/plugins/discourse-apple-auth/spec/requests/auth_apple_spec.rb b/plugins/discourse-apple-auth/spec/requests/auth_apple_spec.rb index 44868d2d9f9..2e0891264d5 100644 --- a/plugins/discourse-apple-auth/spec/requests/auth_apple_spec.rb +++ b/plugins/discourse-apple-auth/spec/requests/auth_apple_spec.rb @@ -124,7 +124,7 @@ describe "sign in with apple" do expect(response.status).to eq(302) expect(response.location).to eq("http://test.localhost/") - result = Auth::Result.from_session_data(server_session[:authentication], user: nil) + result = Auth::Result.from_session_data(session[:authentication], user: nil) expect(result.email).to eq("verified-email@example.com") expect(result.name).to eq("Disco Bot") expect(result.extra_data[:uid]).to eq("unique-user-id") diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 07d4ae14fbb..b48a76be4f1 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -904,14 +904,12 @@ RSpec.configure do |config| class TestCurrentUserProvider < Auth::DefaultCurrentUserProvider def log_on_user(user, session, cookies, opts = {}) - # Try using the main session as `session` sometimes is a server session - (cookies.try(:request).try(:session) || session)[:current_user_id] = user.id + session[:current_user_id] = user.id super end def log_off_user(session, cookies) - # Try using the main session as `session` sometimes is a server session - (cookies.try(:request).try(:session) || session).delete(:current_user_id) + session[:current_user_id] = nil super end end @@ -1132,7 +1130,7 @@ def swap_2_different_characters(str) end def create_request_env(path: nil) - env = Rails.application.env_config.dup.merge("rack.session" => ActionController::TestSession.new) + env = Rails.application.env_config.dup env.merge!(Rack::MockRequest.env_for(path)) if path env end diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index d0ca1f49eb0..10e96aa53b5 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -47,7 +47,15 @@ RSpec.describe InvitesController do end context "when email data is present in authentication data" do - before { server_session[:authentication] = { email: invite.email } } + let(:store) { ActionDispatch::Session::CookieStore.new({}) } + let(:session_stub) do + ActionDispatch::Request::Session.create(store, ActionDispatch::TestRequest.create, {}) + end + + before do + session_stub[:authentication] = { email: invite.email } + ActionDispatch::Request.any_instance.stubs(:session).returns(session_stub) + end it "shows unobfuscated email" do get "/invites/#{invite.invite_key}" diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 64616c1f856..f4a19f2e6e6 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1052,8 +1052,8 @@ RSpec.describe UsersController do expect(response.status).to eq(200) expect(response.parsed_body["active"]).to be_falsey - # should save user_created_message in server session - expect(server_session["user_created_message"]).to be_present + # should save user_created_message in session + expect(session["user_created_message"]).to be_present expect(session[SessionController::ACTIVATE_USER_KEY]).to be_present expect(Jobs::SendSystemMessage.jobs.size).to eq(0) @@ -1072,8 +1072,8 @@ RSpec.describe UsersController do expect(response.parsed_body["active"]).to be_falsey - # should save user_created_message in server session - expect(server_session["user_created_message"]).to be_present + # should save user_created_message in session + expect(session["user_created_message"]).to be_present expect(session[SessionController::ACTIVATE_USER_KEY]).to be_present expect(Jobs::SendSystemMessage.jobs.size).to eq(0) @@ -1114,7 +1114,7 @@ RSpec.describe UsersController do end.to_not change { User.count } expect(response.status).to eq(200) - expect(server_session["user_created_message"]).to be_present + expect(session["user_created_message"]).to be_present end end @@ -1145,7 +1145,7 @@ RSpec.describe UsersController do }.to_not change { User.count } expect(response.status).to eq(200) - expect(server_session["user_created_message"]).to be_present + expect(session["user_created_message"]).to be_present json = response.parsed_body expect(json["active"]).to be_falsey @@ -1365,8 +1365,8 @@ RSpec.describe UsersController do post_user expect(response.status).to eq(200) - # should save user_created_message in server session - expect(server_session["user_created_message"]).to be_present + # should save user_created_message in session + expect(session["user_created_message"]).to be_present expect(session[SessionController::ACTIVATE_USER_KEY]).to be_present end @@ -1547,7 +1547,7 @@ RSpec.describe UsersController do expect(json["success"]).to eq(true) # should not change the session - expect(server_session["user_created_message"]).to be_blank + expect(session["user_created_message"]).to be_blank expect(session[SessionController::ACTIVATE_USER_KEY]).to be_blank end end @@ -1607,7 +1607,7 @@ RSpec.describe UsersController do expect(json["success"]).not_to eq(true) # should not change the session - expect(server_session["user_created_message"]).to be_blank + expect(session["user_created_message"]).to be_blank expect(session[SessionController::ACTIVATE_USER_KEY]).to be_blank end end