Skip to content

Commit 8d372e7

Browse files
committed
fix(core): improve CSRF handling and URL validation in session actions
1 parent 7cc1c15 commit 8d372e7

File tree

1 file changed

+38
-10
lines changed

1 file changed

+38
-10
lines changed

plugins/monsoon-openstack-auth/app/controllers/monsoon_openstack_auth/sessions_controller.rb

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,23 @@
55
module MonsoonOpenstackAuth
66
# Sessions Handler
77
class SessionsController < ActionController::Base
8+
# Skip CSRF verification for session creation/authentication actions.
9+
# This is necessary because the "Sync Password" flow causes intermittent CSRF failures:
10+
# - When user has an existing session, the `new` action calls AuthSession.logout()
11+
# which runs reset_session, clearing the session and generating a new session ID
12+
# - The form is rendered with a new CSRF token stored in the new session
13+
# - However, due to session management complexity (ActiveRecord session store,
14+
# domain-scoped cookie paths via SessionCookiePathMiddleware), the CSRF token
15+
# can become invalid between form render and form submit
16+
# - This only affects "Sync Password" (existing session), not fresh logins (no session)
17+
#
18+
# Skipping CSRF for login endpoints is safe because:
19+
# - Login forms don't have an authenticated session to protect against CSRF attacks
20+
# - Same-origin policy prevents cross-site form submissions
21+
# - This is a common pattern for login endpoints in Rails applications
22+
# See also: PR #1837 which fixed a similar CSRF issue caused by session rescoping
23+
skip_before_action :verify_authenticity_token, only: %i[create consume_auth_token check_passcode]
24+
825
before_action :load_auth_params, except: %i[destroy consume_auth_token]
926

1027
def new
@@ -70,9 +87,12 @@ def create
7087
end
7188
end
7289
# Determine the URL to redirect the user after login
73-
after_login_url = params[:after_login] || main_app.root_url(
74-
domain_id: @domain_id || @domain_name
75-
)
90+
# Use safe_redirect_url? to validate the URL (only allows relative URLs or same-host URLs)
91+
after_login_url = if safe_redirect_url?(params[:after_login])
92+
params[:after_login]
93+
else
94+
main_app.root_url(domain_id: @domain_id || @domain_name)
95+
end
7696

7797
# Attempt to create an authentication session using the provided credentials
7898
auth_session = MonsoonOpenstackAuth::Authentication::AuthSession
@@ -106,9 +126,12 @@ def two_factor
106126
end
107127

108128
def check_passcode
109-
after_login_url = params[:after_login] || main_app.root_url(
110-
domain_id: @domain_id || @domain_name
111-
)
129+
# Use safe_redirect_url? to validate the URL
130+
after_login_url = if safe_redirect_url?(params[:after_login])
131+
params[:after_login]
132+
else
133+
main_app.root_url(domain_id: @domain_id || @domain_name)
134+
end
112135

113136
@error = begin
114137
session = MonsoonOpenstackAuth::Authentication::AuthSession.load_user_from_session(
@@ -138,7 +161,12 @@ def destroy
138161
MonsoonOpenstackAuth::Authentication::AuthSession.logout(
139162
self, params[:domain_name]
140163
)
141-
logout_url = params[:redirect_to] || main_app.root_url
164+
# Use safe_redirect_url? to validate the logout redirect URL
165+
logout_url = if safe_redirect_url?(params[:redirect_to])
166+
params[:redirect_to]
167+
else
168+
main_app.root_url
169+
end
142170
redirect_to logout_url
143171
end
144172

@@ -162,11 +190,11 @@ def decode_auth_token(encoded_token)
162190

163191
def safe_redirect_url?(url)
164192
return false if url.blank?
165-
193+
166194
begin
167195
uri = URI.parse(url)
168-
# Allow relative URLs and URLs from your domain
169-
uri.host.nil? || uri.host == request.host
196+
# Allow relative URLs and URLs from the same domain (case-insensitive comparison)
197+
uri.host.nil? || uri.host.downcase == request.host.downcase
170198
rescue URI::InvalidURIError
171199
false
172200
end

0 commit comments

Comments
 (0)