diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-02 20:25:21 +0200 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-04 13:54:43 +0200 |
commit | 39916fdfeddfd75279d13fa976fdb07f3b9b0e26 (patch) | |
tree | 3a05cbb5816d582a72197e417d3fc3539dd6cf59 /app/controllers | |
parent | 7684217d6806408cd338260119364419260d1720 (diff) | |
download | gitlab-ce-39916fdfeddfd75279d13fa976fdb07f3b9b0e26.tar.gz |
Reuses `InternalRedirect` when possible
`InternalRedirect` prevents Open redirect issues by only allowing
redirection to paths on the same host.
It cleans up any unwanted strings from the path that could point to
another host (fe. //about.gitlab.com/hello). While preserving the
querystring and fragment of the uri.
It is already used by:
- `TermsController`
- `ContinueParams`
- `ImportsController`
- `ForksController`
- `SessionsController`: Only for verifying the host in CE. EE allows
redirecting to a different instance using Geo.
Diffstat (limited to 'app/controllers')
-rw-r--r-- | app/controllers/concerns/continue_params.rb | 4 | ||||
-rw-r--r-- | app/controllers/sessions_controller.rb | 9 |
2 files changed, 4 insertions, 9 deletions
diff --git a/app/controllers/concerns/continue_params.rb b/app/controllers/concerns/continue_params.rb index eb3a623acdd..8b7355974df 100644 --- a/app/controllers/concerns/continue_params.rb +++ b/app/controllers/concerns/continue_params.rb @@ -1,4 +1,5 @@ module ContinueParams + include InternalRedirect extend ActiveSupport::Concern def continue_params @@ -6,8 +7,7 @@ module ContinueParams return nil unless continue_params continue_params = continue_params.permit(:to, :notice, :notice_now) - return unless continue_params[:to] && continue_params[:to].start_with?('/') - return if continue_params[:to].start_with?('//') + continue_params[:to] = safe_redirect_path(continue_params[:to]) continue_params end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f3a4aa849c7..1a339f76d26 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,5 @@ class SessionsController < Devise::SessionsController + include InternalRedirect include AuthenticatesWithTwoFactor include Devise::Controllers::Rememberable include Recaptcha::ClientHelper @@ -102,18 +103,12 @@ class SessionsController < Devise::SessionsController # we should never redirect to '/users/sign_in' after signing in successfully. return true if redirect_uri.path == new_user_session_path - redirect_to = redirect_uri.to_s if redirect_allowed_to?(redirect_uri) + redirect_to = redirect_uri.to_s if host_allowed?(redirect_uri) @redirect_to = redirect_to store_location_for(:redirect, redirect_to) end - # Overridden in EE - def redirect_allowed_to?(uri) - uri.host == Gitlab.config.gitlab.host && - uri.port == Gitlab.config.gitlab.port - end - def two_factor_enabled? find_user&.two_factor_enabled? end |