Add latest changes from gitlab-org/gitlab@master

8 files changed, 64 insertions, 16 deletions

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 817f272d458..f3f0ddd968a 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -181,6 +181,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
         end
 
         accept_pending_invitations(user: user) if new_user
+        persist_accepted_terms_if_required(user) if new_user
+
         store_after_sign_up_path_for_user if intent_to_register?
         sign_in_and_redirect(user, event: :authentication)
       end
@@ -301,6 +303,15 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     redirect_to new_admin_session_path, alert: _('Invalid login or password')
   end
 
+  def persist_accepted_terms_if_required(user)
+    return unless Feature.enabled?(:update_oauth_registration_flow)
+    return unless user.persisted?
+    return unless Gitlab::CurrentSettings.current_application_settings.enforce_terms?
+
+    terms = ApplicationSetting::Term.latest
+    Users::RespondToTermsService.new(user, terms).execute(accepted: true)
+  end
+
   def store_after_sign_up_path_for_user
     store_location_for(:user, users_sign_up_welcome_path)
   end
diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index bcbf43ee38b..dcba136d163 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -55,13 +55,6 @@ class ProjectHook < WebHook
       redis.set(key, time) if !prev || prev < time
     end
   end
-
-  private
-
-  override :web_hooks_disable_failed?
-  def web_hooks_disable_failed?
-    Feature.enabled?(:web_hooks_disable_failed, project)
-  end
 end
 
 ProjectHook.prepend_mod_with('ProjectHook')
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index ed04b0c3d1f..71794964c99 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -53,6 +53,10 @@ class WebHook < ApplicationRecord
     where('recent_failures > ? OR disabled_until >= ?', FAILURE_THRESHOLD, Time.current)
   end
 
+  def self.web_hooks_disable_failed?(hook)
+    Feature.enabled?(:web_hooks_disable_failed, hook.parent)
+  end
+
   def executable?
     !temporarily_disabled? && !permanently_disabled?
   end
@@ -197,7 +201,7 @@ class WebHook < ApplicationRecord
   private
 
   def web_hooks_disable_failed?
-    Feature.enabled?(:web_hooks_disable_failed)
+    self.class.web_hooks_disable_failed?(self)
   end
 
   def initialize_url_variables
diff --git a/app/models/user.rb b/app/models/user.rb
index 16ed3205d93..74035414970 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -301,6 +301,7 @@ class User < ApplicationRecord
   before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? }
   before_validation :ensure_namespace_correct
   before_save :ensure_namespace_correct # in case validation is skipped
+  before_save :ensure_user_detail_assigned
   after_validation :set_username_errors
   after_update :username_changed_hook, if: :saved_change_to_username?
   after_destroy :post_destroy_hook
@@ -1589,6 +1590,11 @@ class User < ApplicationRecord
     end
   end
 
+  # Temporary, will be removed when user_detail fields are fully migrated
+  def ensure_user_detail_assigned
+    user_detail.assign_changed_fields_from_user if UserDetail.user_fields_changed?(self)
+  end
+
   def set_username_errors
     namespace_path_errors = self.errors.delete(:"namespace.path")
 
diff --git a/app/models/user_detail.rb b/app/models/user_detail.rb
index 3787ad1c380..2e662faea6a 100644
--- a/app/models/user_detail.rb
+++ b/app/models/user_detail.rb
@@ -12,15 +12,55 @@ class UserDetail < ApplicationRecord
   validates :job_title, length: { maximum: 200 }
   validates :bio, length: { maximum: 255 }, allow_blank: true
 
+  DEFAULT_FIELD_LENGTH = 500
+
+  validates :linkedin, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true
+  validates :twitter, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true
+  validates :skype, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true
+  validates :location, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true
+  validates :organization, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true
+  validates :website_url, length: { maximum: DEFAULT_FIELD_LENGTH }, url: true, allow_blank: true
+
+  before_validation :sanitize_attrs
   before_save :prevent_nil_bio
 
   enum registration_objective: REGISTRATION_OBJECTIVE_PAIRS, _suffix: true
 
+  def self.user_fields_changed?(user)
+    (%w[linkedin skype twitter website_url location organization] & user.changed).any?
+  end
+
+  def sanitize_attrs
+    %i[linkedin skype twitter website_url].each do |attr|
+      value = self[attr]
+      self[attr] = Sanitize.clean(value) if value.present?
+    end
+    %i[location organization].each do |attr|
+      value = self[attr]
+      self[attr] = Sanitize.clean(value).gsub('&amp;', '&') if value.present?
+    end
+  end
+
+  def assign_changed_fields_from_user
+    self.linkedin = trim_field(user.linkedin) if user.linkedin_changed?
+    self.twitter = trim_field(user.twitter) if user.twitter_changed?
+    self.skype = trim_field(user.skype) if user.skype_changed?
+    self.website_url = trim_field(user.website_url) if user.website_url_changed?
+    self.location = trim_field(user.location) if user.location_changed?
+    self.organization = trim_field(user.organization) if user.organization_changed?
+  end
+
   private
 
   def prevent_nil_bio
     self.bio = '' if bio_changed? && bio.nil?
   end
+
+  def trim_field(value)
+    return '' unless value
+
+    value.first(DEFAULT_FIELD_LENGTH)
+  end
 end
 
 UserDetail.prepend_mod_with('UserDetail')
diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb
index a412c97b219..df065b24e64 100644
--- a/app/policies/issuable_policy.rb
+++ b/app/policies/issuable_policy.rb
@@ -22,12 +22,6 @@ class IssuablePolicy < BasePolicy
     enable :reopen_issue
   end
 
-  # This rule replicates permissions in NotePolicy#can_read_confidential and it's used in
-  # TodoPolicy for performance reasons
-  rule { can?(:reporter_access) | assignee_or_author | admin }.policy do
-    enable :read_confidential_notes
-  end
-
   rule { can?(:read_merge_request) & assignee_or_author }.policy do
     enable :update_merge_request
     enable :reopen_merge_request
diff --git a/app/policies/todo_policy.rb b/app/policies/todo_policy.rb
index 5c24964f24a..d63eb9407f8 100644
--- a/app/policies/todo_policy.rb
+++ b/app/policies/todo_policy.rb
@@ -16,7 +16,7 @@ class TodoPolicy < BasePolicy
 
   desc "User can read the todo's confidential note"
   condition(:can_read_todo_confidential_note) do
-    @user && @user.can?(:read_confidential_notes, @subject.target)
+    @user && @user.can?(:read_internal_note, @subject.target)
   end
 
   rule { own_todo & can_read_target }.enable :read_todo
diff --git a/app/services/web_hooks/log_execution_service.rb b/app/services/web_hooks/log_execution_service.rb
index 5be8aee3ae8..1a40c877bda 100644
--- a/app/services/web_hooks/log_execution_service.rb
+++ b/app/services/web_hooks/log_execution_service.rb
@@ -17,7 +17,7 @@ module WebHooks
     end
 
     def execute
-      update_hook_failure_state
+      update_hook_failure_state if WebHook.web_hooks_disable_failed?(hook)
       log_execution
     end