Skip to content

Commit 6213ec8

Browse files
committed
Improve OAuth flow detection and redirect handling
- Use start_with?(oauth_authorization_path) instead of include?('/oauth/authorize') for stricter path matching - Use stored_location_for(:user) instead of session[:user_return_to] for automatic cleanup and explicit Devise scope usage - OAuth flow detection is tied to session[:user_return_to], so the :user scope is explicit and correct - Add ? suffix to predicate method name Improves security (prevents substring matches), maintainability (uses framework APIs with explicit scope), and follows Ruby conventions.
1 parent 26a4fe8 commit 6213ec8

1 file changed

Lines changed: 9 additions & 5 deletions

File tree

app/controllers/application_controller.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ def store_location
6767
# rubocop:disable Metrics/AbcSize
6868
def after_sign_in_path_for(_resource)
6969
# ensure oauth2 authorization flow is not interrupted
70-
return session[:user_return_to] if user_is_in_oauth_flow
70+
# TODO: Unless nil, should stored_location_for(resource) always be returned?
71+
return stored_location_for(:user) if user_is_in_oauth_flow?
7172

7273
referer_path = URI(request.referer).path unless request.referer.nil?
7374
if from_external_domain? || referer_path.eql?(new_user_session_path) ||
@@ -80,9 +81,11 @@ def after_sign_in_path_for(_resource)
8081
end
8182
# rubocop:enable Metrics/AbcSize
8283

83-
def after_sign_up_path_for(_resource) # rubocop:todo Metrics/AbcSize
84+
# rubocop:disable Metrics/AbcSize
85+
def after_sign_up_path_for(_resource)
8486
# ensure oauth2 authorization flow is not interrupted
85-
return session[:user_return_to] if user_is_in_oauth_flow
87+
# TODO: Unless nil, should stored_location_for(resource) always be returned?
88+
return stored_location_for(:user) if user_is_in_oauth_flow?
8689

8790
referer_path = URI(request.referer).path unless request.referer.nil?
8891
if from_external_domain? ||
@@ -93,6 +96,7 @@ def after_sign_up_path_for(_resource) # rubocop:todo Metrics/AbcSize
9396
request.referer
9497
end
9598
end
99+
# rubocop:enable Metrics/AbcSize
96100

97101
def after_sign_in_error_path_for(_resource)
98102
(from_external_domain? ? root_path : request.referer || root_path)
@@ -204,7 +208,7 @@ def render_respond_to_format_with_error_message(msg, url_or_path, http_status, e
204208
end
205209
end
206210

207-
def user_is_in_oauth_flow
208-
session[:user_return_to].present? && session[:user_return_to].include?('/oauth/authorize')
211+
def user_is_in_oauth_flow?
212+
session[:user_return_to]&.start_with?(oauth_authorization_path)
209213
end
210214
end

0 commit comments

Comments
 (0)