Merge branch 'sh-backport-10-3-4-security-fixes' into 'master'

Backport 10.3.4 security fixes into master

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

Author: osw-gitlab

app/assets/javascripts/deploy_keys/components/key.vue

@@ -1,11 +1,15 @@
 <script>
   import actionBtn from './action_btn.vue';
   import { getTimeago } from '../../lib/utils/datetime_utility';
+  import tooltip from '../../vue_shared/directives/tooltip';
 
   export default {
     components: {
       actionBtn,
     },
+    directives: {
+      tooltip,
+    },
     props: {
       deployKey: {
         type: Object,
@@ -32,6 +36,9 @@
       isEnabled(id) {
         return this.store.findEnabledKey(id) !== undefined;
       },
+      tooltipTitle(project) {
+        return project.can_push ? 'Write access allowed' : 'Read access only';
+      },
     },
   };
 </script>
@@ -52,21 +59,23 @@
       <div class="description">
         {{ deployKey.fingerprint }}
       </div>
-      <div
-        v-if="deployKey.can_push"
-        class="write-access-allowed"
-      >
-        Write access allowed
-      </div>
     </div>
     <div class="deploy-key-content prepend-left-default deploy-key-projects">
       <a
-        v-for="(project, i) in deployKey.projects"
-        class="label deploy-project-label"
-        :href="project.full_path"
+        v-for="(deployKeysProject, i) in deployKey.deploy_keys_projects"
         :key="i"
+        class="label deploy-project-label"
+        :href="deployKeysProject.project.full_path"
+        :title="tooltipTitle(deployKeysProject)"
+        v-tooltip
       >
-        {{ project.full_name }}
+        {{ deployKeysProject.project.full_name }}
+        <i
+          v-if="!deployKeysProject.can_push"
+          aria-hidden="true"
+          class="fa fa-lock"
+        >
+        </i>
       </a>
     </div>
     <div class="deploy-key-content">

app/assets/javascripts/labels_select.js

@@ -231,7 +231,7 @@ export default class LabelsSelect {
             selectedClass.push('label-item');
             $a.attr('data-label-id', label.id);
           }
-          $a.addClass(selectedClass.join(' ')).html(colorEl + " " + label.title);
+          $a.addClass(selectedClass.join(' ')).html(`${colorEl} ${_.escape(label.title)}`);
           // Return generated html
           return $li.html($a).prop('outerHTML');
         },

app/assets/javascripts/notebook/cells/markdown.vue

@@ -1,6 +1,7 @@
 <script>
   /* global katex */
   import marked from 'marked';
+  import sanitize from 'sanitize-html';
   import Prompt from './prompt.vue';
 
   const renderer = new marked.Renderer();
@@ -82,7 +83,12 @@
     },
     computed: {
       markdown() {
-        return marked(this.cell.source.join('').replace(/\\/g, '\\\\'));
+        return sanitize(marked(this.cell.source.join('').replace(/\\/g, '\\\\')), {
+          allowedTags: false,
+          allowedAttributes: {
+            '*': ['class'],
+          },
+        });
       },
     },
   };

app/assets/javascripts/notebook/cells/output/html.vue

@@ -1,4 +1,5 @@
 <script>
+  import sanitize from 'sanitize-html';
   import Prompt from '../prompt.vue';
 
   export default {
@@ -11,12 +12,24 @@
         required: true,
       },
     },
+    computed: {
+      sanitizedOutput() {
+        return sanitize(this.rawCode, {
+          allowedTags: sanitize.defaults.allowedTags.concat([
+            'img', 'svg',
+          ]),
+          allowedAttributes: {
+            img: ['src'],
+          },
+        });
+      },
+    },
   };
 </script>
 
 <template>
   <div class="output">
     <prompt />
-    <div v-html="rawCode"></div>
+    <div v-html="sanitizedOutput"></div>
   </div>
 </template>

app/controllers/admin/deploy_keys_controller.rb

@@ -50,10 +50,10 @@ def deploy_keys
   end
 
   def create_params
-    params.require(:deploy_key).permit(:key, :title, :can_push)
+    params.require(:deploy_key).permit(:key, :title)
   end
 
   def update_params
-    params.require(:deploy_key).permit(:title, :can_push)
+    params.require(:deploy_key).permit(:title)
   end
 end

app/controllers/groups/milestones_controller.rb

@@ -75,8 +75,6 @@ def milestone_path
   end
 
   def milestones
-    search_params = params.merge(group_ids: group.id)
-
     milestones = MilestonesFinder.new(search_params).execute
     legacy_milestones = GroupMilestone.build_collection(group, group_projects, params)
 
@@ -94,4 +92,8 @@ def milestone
 
     render_404 unless @milestone
   end
+
+  def search_params
+    params.permit(:state).merge(group_ids: group.id)
+  end
 end

app/controllers/omniauth_callbacks_controller.rb

@@ -112,6 +112,8 @@ def handle_omniauth
 
       continue_login_process
     end
+  rescue Gitlab::OAuth::SigninDisabledForProviderError
+    handle_disabled_provider
   rescue Gitlab::OAuth::SignupDisabledError
     handle_signup_error
   end
@@ -168,6 +170,13 @@ def fail_ldap_login
     redirect_to new_user_session_path
   end
 
+  def handle_disabled_provider
+    label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
+    flash[:alert] = "Signing in using #{label} has been disabled"
+
+    redirect_to new_user_session_path
+  end
+
   def log_audit_event(user, options = {})
     AuditEventService.new(user, user, options)
       .for_authentication.security_event

app/controllers/projects/deploy_keys_controller.rb

@@ -24,7 +24,7 @@ def new
   def create
     @key = DeployKeys::CreateService.new(current_user, create_params).execute
 
-    unless @key.valid? && @project.deploy_keys << @key
+    unless @key.valid?
       flash[:alert] = @key.errors.full_messages.join(', ').html_safe
     end
 
@@ -71,11 +71,14 @@ def deploy_key
   end
 
   def create_params
-    params.require(:deploy_key).permit(:key, :title, :can_push)
+    create_params = params.require(:deploy_key)
+                          .permit(:key, :title, deploy_keys_projects_attributes: [:can_push])
+    create_params.dig(:deploy_keys_projects_attributes, '0')&.merge!(project_id: @project.id)
+    create_params
   end
 
   def update_params
-    params.require(:deploy_key).permit(:title, :can_push)
+    params.require(:deploy_key).permit(:title, deploy_keys_projects_attributes: [:id, :can_push])
   end
 
   def authorize_update_deploy_key!

app/controllers/projects/milestones_controller.rb

@@ -92,12 +92,6 @@ def destroy
 
   def milestones
     @milestones ||= begin
-      if @project.group && can?(current_user, :read_group, @project.group)
-        group = @project.group
-      end
-
-      search_params = params.merge(project_ids: @project.id, group_ids: group&.id)
-
       MilestonesFinder.new(search_params).execute
     end
   end
@@ -113,4 +107,12 @@ def authorize_admin_milestone!
   def milestone_params
     params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
   end
+
+  def search_params
+    if @project.group && can?(current_user, :read_group, @project.group)
+      group = @project.group
+    end
+
+    params.permit(:state).merge(project_ids: @project.id, group_ids: group&.id)
+  end
 end

app/finders/milestones_finder.rb

@@ -46,11 +46,7 @@ def by_state(items)
   end
 
   def order(items)
-    if params.has_key?(:order)
-      items.reorder(params[:order])
-    else
-      order_statement = Gitlab::Database.nulls_last_order('due_date', 'ASC')
-      items.reorder(order_statement)
-    end
+    order_statement = Gitlab::Database.nulls_last_order('due_date', 'ASC')
+    items.reorder(order_statement).order('title ASC')
   end
 end

app/models/deploy_key.rb

@@ -1,10 +1,16 @@
 class DeployKey < Key
-  has_many :deploy_keys_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  include IgnorableColumn
+
+  has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :projects, through: :deploy_keys_projects
 
   scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
   scope :are_public,  -> { where(public: true) }
 
+  ignore_column :can_push
+
+  accepts_nested_attributes_for :deploy_keys_projects
+
   def private?
     !public?
   end
@@ -22,10 +28,18 @@ def destroyed_when_orphaned?
   end
 
   def has_access_to?(project)
-    projects.include?(project)
+    deploy_keys_project_for(project).present?
   end
 
   def can_push_to?(project)
-    can_push? && has_access_to?(project)
+    !!deploy_keys_project_for(project)&.can_push?
+  end
+
+  def deploy_keys_project_for(project)
+    deploy_keys_projects.find_by(project: project)
+  end
+
+  def projects_with_write_access
+    Project.preload(:route).where(id: deploy_keys_projects.with_write_access.select(:project_id))
   end
 end

app/models/deploy_keys_project.rb

@@ -1,8 +1,14 @@
 class DeployKeysProject < ActiveRecord::Base
   belongs_to :project
-  belongs_to :deploy_key
+  belongs_to :deploy_key, inverse_of: :deploy_keys_projects
 
-  validates :deploy_key_id, presence: true
+  scope :without_project_deleted,  -> { joins(:project).where(projects: { pending_delete: false }) }
+  scope :in_project, ->(project) { where(project: project) }
+  scope :with_write_access, -> { where(can_push: true) }
+
+  accepts_nested_attributes_for :deploy_key
+
+  validates :deploy_key, presence: true
   validates :deploy_key_id, uniqueness: { scope: [:project_id], message: "already exists in project" }
   validates :project_id, presence: true
 

app/models/global_milestone.rb

@@ -44,10 +44,10 @@ def self.states_count(projects, group = nil)
   def self.group_milestones_states_count(group)
     return STATE_COUNT_HASH unless group
 
-    params = { group_ids: [group.id], state: 'all', order: nil }
+    params = { group_ids: [group.id], state: 'all' }
 
     relation = MilestonesFinder.new(params).execute
-    grouped_by_state = relation.group(:state).count
+    grouped_by_state = relation.reorder(nil).group(:state).count
 
     {
       opened: grouped_by_state['active'] || 0,
@@ -60,10 +60,10 @@ def self.group_milestones_states_count(group)
   def self.legacy_group_milestone_states_count(projects)
     return STATE_COUNT_HASH unless projects
 
-    params = { project_ids: projects.map(&:id), state: 'all', order: nil }
+    params = { project_ids: projects.map(&:id), state: 'all' }
 
     relation = MilestonesFinder.new(params).execute
-    project_milestones_by_state_and_title = relation.group(:state, :title).count
+    project_milestones_by_state_and_title = relation.reorder(nil).group(:state, :title).count
 
     opened = count_by_state(project_milestones_by_state_and_title, 'active')
     closed = count_by_state(project_milestones_by_state_and_title, 'closed')

app/models/hooks/web_hook.rb

@@ -4,6 +4,7 @@ class WebHook < ActiveRecord::Base
   has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
 
   validates :url, presence: true, url: true
+  validates :token, format: { without: /\n/ }
 
   def execute(data, hook_name)
     WebHookService.new(self, data, hook_name).execute

app/models/service.rb

@@ -118,6 +118,11 @@ def event_field(event)
     nil
   end
 
+  def api_field_names
+    fields.map { |field| field[:name] }
+      .reject { |field_name| field_name =~ /(password|token|key)/ }
+  end
+
   def global_fields
     fields
   end

app/presenters/projects/settings/deploy_keys_presenter.rb

@@ -7,7 +7,7 @@ class DeployKeysPresenter < Gitlab::View::Presenter::Simple
       delegate :size, to: :available_public_keys, prefix: true
 
       def new_key
-        @key ||= DeployKey.new
+        @key ||= DeployKey.new.tap { |dk| dk.deploy_keys_projects.build }
       end
 
       def enabled_keys

app/serializers/deploy_key_entity.rb

@@ -3,19 +3,20 @@ class DeployKeyEntity < Grape::Entity
   expose :user_id
   expose :title
   expose :fingerprint
-  expose :can_push
   expose :destroyed_when_orphaned?, as: :destroyed_when_orphaned
   expose :almost_orphaned?, as: :almost_orphaned
   expose :created_at
   expose :updated_at
-  expose :projects, using: ProjectEntity do |deploy_key|
-    deploy_key.projects.without_deleted.select { |project| options[:user].can?(:read_project, project) }
+  expose :deploy_keys_projects, using: DeployKeysProjectEntity do |deploy_key|
+    deploy_key.deploy_keys_projects
+              .without_project_deleted
+              .select { |deploy_key_project| Ability.allowed?(options[:user], :read_project, deploy_key_project.project) }
   end
   expose :can_edit
 
   private
 
   def can_edit
-    options[:user].can?(:update_deploy_key, object)
+    Ability.allowed?(options[:user], :update_deploy_key, object)
   end
 end

app/serializers/deploy_keys_project_entity.rb

@@ -0,0 +1,4 @@
+class DeployKeysProjectEntity < Grape::Entity
+  expose :can_push
+  expose :project, using: ProjectEntity
+end