Discourse/discourse
Discourse/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-10-04 17:32:34 +08:00
Code Packages Activity Actions 10

DEV: Move more data into the server session (#35009)

Browse source 
Now that `ServerSession` can store arbitrary data, we can move some more
data into it.

This PR moves some data related to authentication into it, as sometimes
that kind of data can be pretty big.
This commit is contained in:
Loïc Guitaut 2025-10-01 15:00:48 +02:00 • committed by GitHub
parent 3ca1f8bf42
commit 066d3a1abc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 37 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

6
app/controllers/invites_controller.rb
View file

@ -351,7 +351,7 @@ class InvitesController < ApplicationController

if invite.present?
begin
attrs = { ip_address: request.remote_ip, session: session }
attrs = { ip_address: request.remote_ip, session: server_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 session[:authentication]
auth_result = Auth::Result.from_session_data(session[:authentication], user: nil)
if server_session[:authentication]
auth_result = Auth::Result.from_session_data(server_session[:authentication], user: nil)
if invite.email == auth_result.email
email = invite.email
else

6
app/controllers/session_controller.rb
View file

@ -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, session, cookies)
activation = UserActivator.new(user, request, server_session, cookies)
activation.finish
session["user_created_message"] = activation.message
server_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: session,
session: server_session,
email: sso.email,
redeeming_user: redeeming_user,
).redeem

6
app/controllers/users/omniauth_callbacks_controller.rb
View file

@ -149,7 +149,7 @@ class Users::OmniauthCallbacksController < ApplicationController
elsif invite_required?
@auth_result.requires_invite = true
else
session[:authentication] = @auth_result.session_data
server_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
session[:authentication] = nil # don't carry around old auth info, perhaps move elsewhere
server_session.delete(:authentication) # don't carry around old auth info
@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

10
app/controllers/users_controller.rb
View file

@ -740,7 +740,7 @@ class UsersController < ApplicationController
end
end

authentication = UserAuthenticator.new(user, session)
authentication = UserAuthenticator.new(user, server_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, session, cookies)
activation = UserActivator.new(user, request, server_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
session["user_created_message"] = activation.message
server_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"))
session["user_created_message"] = activation.success_message
server_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 = session["user_created_message"] || I18n.t("activation.missing_session")
@message = server_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]

4
app/services/user_activator.rb
View file

@ -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

7
app/services/user_authenticator.rb
View file

@ -9,7 +9,7 @@ class UserAuthenticator
)
@user = user
@session = session
if session&.dig(:authentication) && session[:authentication].is_a?(Hash)
if session.try(:[], :authentication).is_a?(Hash)
@auth_result = Auth::Result.from_session_data(session[:authentication], user: user)
end
@authenticator_finder = authenticator_finder
@ -35,8 +35,9 @@ class UserAuthenticator
authenticator.after_create_account(@user, @auth_result)
confirm_email
end
if @session&.dig(:authentication)
@session[:authentication] = @auth_result = nil
if @session.try(:[], :authentication)
@session.delete(:authentication)
@auth_result = nil
@session[:authenticated_with_oauth] = true
end
end

2
plugins/discourse-apple-auth/spec/requests/auth_apple_spec.rb
View file

@ -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(session[:authentication], user: nil)
result = Auth::Result.from_session_data(server_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")

8
spec/rails_helper.rb
View file

@ -904,12 +904,14 @@ RSpec.configure do |config|

class TestCurrentUserProvider < Auth::DefaultCurrentUserProvider
def log_on_user(user, session, cookies, opts = {})
session[:current_user_id] = user.id
# Try using the main session as `session` sometimes is a server session
(cookies.try(:request).try(:session) || session)[:current_user_id] = user.id
super
end

def log_off_user(session, cookies)
session[:current_user_id] = nil
# Try using the main session as `session` sometimes is a server session
(cookies.try(:request).try(:session) || session).delete(:current_user_id)
super
end
end
@ -1130,7 +1132,7 @@ def swap_2_different_characters(str)
end

def create_request_env(path: nil)
env = Rails.application.env_config.dup
env = Rails.application.env_config.dup.merge("rack.session" => ActionController::TestSession.new)
env.merge!(Rack::MockRequest.env_for(path)) if path
env
end

10
spec/requests/invites_controller_spec.rb
View file

@ -47,15 +47,7 @@ RSpec.describe InvitesController do
end

context "when email data is present in authentication data" do
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
before { server_session[:authentication] = { email: invite.email } }

it "shows unobfuscated email" do
get "/invites/#{invite.invite_key}"

20
spec/requests/users_controller_spec.rb
View file

@ -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 session
expect(session["user_created_message"]).to be_present
# should save user_created_message in server session
expect(server_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 session
expect(session["user_created_message"]).to be_present
# should save user_created_message in server session
expect(server_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(session["user_created_message"]).to be_present
expect(server_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(session["user_created_message"]).to be_present
expect(server_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 session
expect(session["user_created_message"]).to be_present
# should save user_created_message in server session
expect(server_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(session["user_created_message"]).to be_blank
expect(server_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(session["user_created_message"]).to be_blank
expect(server_session["user_created_message"]).to be_blank
expect(session[SessionController::ACTIVATE_USER_KEY]).to be_blank
end
end