Revert "DEV: Move more data into the server session" (#35115)

Reverts discourse/discourse#35009

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]
+    if session[:authentication]
-      auth_result = Auth::Result.from_session_data(server_session[:authentication], user: nil)
+      auth_result = Auth::Result.from_session_data(session[:authentication], user: nil)
       if invite.email == auth_result.email
         email = invite.email
       else

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

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

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]

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

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)
+    if @session&.dig(:authentication)
-      @session.delete(:authentication)
+      @session[:authentication] = @auth_result = nil
-      @auth_result = nil
       @session[:authenticated_with_oauth] = true
     end
   end

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")

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
+      session[:current_user_id] = user.id
-      (cookies.try(:request).try(:session) || 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
+      session[:current_user_id] = nil
-      (cookies.try(:request).try(:session) || session).delete(:current_user_id)
       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

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}"

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
+        # should save user_created_message in session
-        expect(server_session["user_created_message"]).to be_present
+        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
+          # should save user_created_message in session
-          expect(server_session["user_created_message"]).to be_present
+          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
+        # should save user_created_message in session
-        expect(server_session["user_created_message"]).to be_present
+        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