Merge branch 'bvl-restrict-api-git-for-terms' into 'master'

Block access to API & git when terms are enforced

Closes #45849

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

3 files changed, 22 insertions, 21 deletions

diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb
index e803cd3a8d8..ce9373f5883 100644
--- a/app/helpers/users_helper.rb
+++ b/app/helpers/users_helper.rb
@@ -42,22 +42,11 @@ module UsersHelper
 
     items << :sign_out if current_user
 
-    # TODO: Remove these conditions when the permissions are prevented in
-    # https://gitlab.com/gitlab-org/gitlab-ce/issues/45849
-    terms_not_enforced = !Gitlab::CurrentSettings
-                                .current_application_settings
-                                .enforce_terms?
-    required_terms_accepted = terms_not_enforced || current_user.terms_accepted?
+    return items if current_user&.required_terms_not_accepted?
 
-    items << :help if required_terms_accepted
-
-    if can?(current_user, :read_user, current_user) && required_terms_accepted
-      items << :profile
-    end
-
-    if can?(current_user, :update_user, current_user) && required_terms_accepted
-      items << :settings
-    end
+    items << :help
+    items << :profile if can?(current_user, :read_user, current_user)
+    items << :settings if can?(current_user, :update_user, current_user)
 
     items
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index a9cfd39f604..884f3bbb364 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1193,6 +1193,11 @@ class User < ActiveRecord::Base
     accepted_term_id.present?
   end
 
+  def required_terms_not_accepted?
+    Gitlab::CurrentSettings.current_application_settings.enforce_terms? &&
+      !terms_accepted?
+  end
+
   protected
 
   # override, from Devise::Validatable
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index 64e550d19d0..1cf5515d9d7 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -1,22 +1,24 @@
 class GlobalPolicy < BasePolicy
   desc "User is blocked"
   with_options scope: :user, score: 0
-  condition(:blocked) { @user.blocked? }
+  condition(:blocked) { @user&.blocked? }
 
   desc "User is an internal user"
   with_options scope: :user, score: 0
-  condition(:internal) { @user.internal? }
+  condition(:internal) { @user&.internal? }
 
   desc "User's access has been locked"
   with_options scope: :user, score: 0
-  condition(:access_locked) { @user.access_locked? }
+  condition(:access_locked) { @user&.access_locked? }
 
-  condition(:can_create_fork, scope: :user) { @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } }
+  condition(:can_create_fork, scope: :user) { @user && @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } }
+
+  condition(:required_terms_not_accepted, scope: :user, score: 0) do
+    @user&.required_terms_not_accepted?
+  end
 
   rule { anonymous }.policy do
     prevent :log_in
-    prevent :access_api
-    prevent :access_git
     prevent :receive_notifications
     prevent :use_quick_actions
     prevent :create_group
@@ -38,6 +40,11 @@ class GlobalPolicy < BasePolicy
     prevent :use_quick_actions
   end
 
+  rule { required_terms_not_accepted }.policy do
+    prevent :access_api
+    prevent :access_git
+  end
+
   rule { can_create_group }.policy do
     enable :create_group
   end