Merge branch 'bvl-enforce-terms' into 'master'

Enforce application wide terms

Closes #44798

See merge request gitlab-org/gitlab-ce!18570

5 files changed, 137 insertions, 10 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 8ad13a82f89..2caffec66ac 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -13,12 +13,14 @@ class ApplicationController < ActionController::Base
 
   before_action :authenticate_sessionless_user!
   before_action :authenticate_user!
+  before_action :enforce_terms!, if: -> { Gitlab::CurrentSettings.current_application_settings.enforce_terms },
+                                 unless: :peek_request?
   before_action :validate_user_service_ticket!
   before_action :check_password_expiration
   before_action :ldap_security_check
   before_action :sentry_context
   before_action :default_headers
-  before_action :add_gon_variables, unless: -> { request.path.start_with?('/-/peek') }
+  before_action :add_gon_variables, unless: :peek_request?
   before_action :configure_permitted_parameters, if: :devise_controller?
   before_action :require_email, unless: :devise_controller?
 
@@ -269,6 +271,27 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  def enforce_terms!
+    return unless current_user
+    return if current_user.terms_accepted?
+
+    if sessionless_user?
+      render_403
+    else
+      # Redirect to the destination if the request is a get.
+      # Redirect to the source if it was a post, so the user can re-submit after
+      # accepting the terms.
+      redirect_path = if request.get?
+                        request.fullpath
+                      else
+                        URI(request.referer).path if request.referer
+                      end
+
+      flash[:notice] = _("Please accept the Terms of Service before continuing.")
+      redirect_to terms_path(redirect: redirect_path), status: :found
+    end
+  end
+
   def import_sources_enabled?
     !Gitlab::CurrentSettings.import_sources.empty?
   end
@@ -342,4 +365,12 @@ class ApplicationController < ActionController::Base
     # Per https://tools.ietf.org/html/rfc5987, headers need to be ISO-8859-1, not UTF-8
     response.headers['Page-Title'] = URI.escape(page_title('GitLab'))
   end
+
+  def sessionless_user?
+    current_user && !session.keys.include?('warden.user.user.key')
+  end
+
+  def peek_request?
+    request.path.start_with?('/-/peek')
+  end
 end
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/concerns/internal_redirect.rb b/app/controllers/concerns/internal_redirect.rb
new file mode 100644
index 00000000000..7409b2e89a5
--- /dev/null
+++ b/app/controllers/concerns/internal_redirect.rb
@@ -0,0 +1,35 @@
+module InternalRedirect
+  extend ActiveSupport::Concern
+
+  def safe_redirect_path(path)
+    return unless path
+    # Verify that the string starts with a `/` but not a double `/`.
+    return unless path =~ %r{^/\w.*$}
+
+    uri = URI(path)
+    # Ignore anything path of the redirect except for the path, querystring and,
+    # fragment, forcing the redirect within the same host.
+    full_path_for_uri(uri)
+  rescue URI::InvalidURIError
+    nil
+  end
+
+  def safe_redirect_path_for_url(url)
+    return unless url
+
+    uri = URI(url)
+    safe_redirect_path(full_path_for_uri(uri)) if host_allowed?(uri)
+  rescue URI::InvalidURIError
+    nil
+  end
+
+  def host_allowed?(uri)
+    uri.host == request.host &&
+      uri.port == request.port
+  end
+
+  def full_path_for_uri(uri)
+    path_with_query = [uri.path, uri.query].compact.join('?')
+    [path_with_query, uri.fragment].compact.join("#")
+  end
+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
diff --git a/app/controllers/users/terms_controller.rb b/app/controllers/users/terms_controller.rb
new file mode 100644
index 00000000000..95c5c3432d5
--- /dev/null
+++ b/app/controllers/users/terms_controller.rb
@@ -0,0 +1,66 @@
+module Users
+  class TermsController < ApplicationController
+    include InternalRedirect
+
+    skip_before_action :enforce_terms!
+    before_action :terms
+
+    layout 'terms'
+
+    def index
+      @redirect = redirect_path
+    end
+
+    def accept
+      agreement = Users::RespondToTermsService.new(current_user, viewed_term)
+                    .execute(accepted: true)
+
+      if agreement.persisted?
+        redirect_to redirect_path
+      else
+        flash[:alert] = agreement.errors.full_messages.join(', ')
+        redirect_to terms_path, redirect: redirect_path
+      end
+    end
+
+    def decline
+      agreement = Users::RespondToTermsService.new(current_user, viewed_term)
+                    .execute(accepted: false)
+
+      if agreement.persisted?
+        sign_out(current_user)
+        redirect_to root_path
+      else
+        flash[:alert] = agreement.errors.full_messages.join(', ')
+        redirect_to terms_path, redirect: redirect_path
+      end
+    end
+
+    private
+
+    def viewed_term
+      @viewed_term ||= ApplicationSetting::Term.find(params[:id])
+    end
+
+    def terms
+      unless @term = Gitlab::CurrentSettings.current_application_settings.latest_terms
+        redirect_to redirect_path
+      end
+    end
+
+    def redirect_path
+      redirect_to_path = safe_redirect_path(params[:redirect]) || safe_redirect_path_for_url(request.referer)
+
+      if redirect_to_path &&
+          excluded_redirect_paths.none? { |excluded| redirect_to_path.include?(excluded) }
+        redirect_to_path
+      else
+        root_path
+      end
+    end
+
+    def excluded_redirect_paths
+      [terms_path, new_user_session_path]
+    end
+  end
+end