Revert "Merge branch 'security-pipeline-trigger-tokens-exposure-11-4' into 'security-11-4'"11-4-stable

This reverts commit 426716d1556f821988e67cc178e4ecffc4115897

11 files changed, 17 insertions, 130 deletions

diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb
index 0c0f81a69c6..3a1344651df 100644
--- a/app/controllers/projects/settings/ci_cd_controller.rb
+++ b/app/controllers/projects/settings/ci_cd_controller.rb
@@ -99,9 +99,7 @@ module Projects
 
       def define_triggers_variables
         @triggers = @project.triggers
-          .present(current_user: current_user)
         @trigger = ::Ci::Trigger.new
-          .present(current_user: current_user)
       end
 
       def define_badges_variables
diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb
index c7b4ebb2b24..f5fdfb8accc 100644
--- a/app/controllers/projects/triggers_controller.rb
+++ b/app/controllers/projects/triggers_controller.rb
@@ -66,11 +66,12 @@ class Projects::TriggersController < Projects::ApplicationController
   end
 
   def trigger
-    @trigger ||= project.triggers.find(params[:id])
-      .present(current_user: current_user)
+    @trigger ||= project.triggers.find(params[:id]) || render_404
   end
 
   def trigger_params
-    params.require(:trigger).permit(:description)
+    params.require(:trigger).permit(
+      :description
+    )
   end
 end
diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb
index 637148c4ce4..55db42162ca 100644
--- a/app/models/ci/trigger.rb
+++ b/app/models/ci/trigger.rb
@@ -4,7 +4,6 @@ module Ci
   class Trigger < ActiveRecord::Base
     extend Gitlab::Ci::Model
     include IgnorableColumn
-    include Presentable
 
     ignore_column :deleted_at
 
@@ -30,7 +29,7 @@ module Ci
     end
 
     def short_token
-      token[0...4] if token.present?
+      token[0...4]
     end
 
     def legacy?
diff --git a/app/presenters/ci/trigger_presenter.rb b/app/presenters/ci/trigger_presenter.rb
deleted file mode 100644
index 605c8f328a4..00000000000
--- a/app/presenters/ci/trigger_presenter.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-# frozen_string_literal: true
-
-module Ci
-  class TriggerPresenter < Gitlab::View::Presenter::Delegated
-    presents :trigger
-
-    def has_token_exposed?
-      can?(current_user, :admin_trigger, trigger)
-    end
-
-    def token
-      if has_token_exposed?
-        trigger.token
-      else
-        trigger.short_token
-      end
-    end
-  end
-end
diff --git a/app/views/projects/triggers/_trigger.html.haml b/app/views/projects/triggers/_trigger.html.haml
index 6f6f1e5e0c5..7e4618e1a88 100644
--- a/app/views/projects/triggers/_trigger.html.haml
+++ b/app/views/projects/triggers/_trigger.html.haml
@@ -1,6 +1,6 @@
 %tr
   %td
-    - if trigger.has_token_exposed?
+    - if can?(current_user, :admin_trigger, trigger)
       %span= trigger.token
       = clipboard_button(text: trigger.token, title: "Copy trigger token to clipboard")
     - else
diff --git a/changelogs/unreleased/security-pipeline-trigger-tokens-exposure.yml b/changelogs/unreleased/security-pipeline-trigger-tokens-exposure.yml
deleted file mode 100644
index 97d743eead1..00000000000
--- a/changelogs/unreleased/security-pipeline-trigger-tokens-exposure.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Expose CI/CD trigger token only to the trigger owner
-merge_request:
-author:
-type: security
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 9c2683702e7..fd36381aa51 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1161,11 +1161,8 @@ module API
     end
 
     class Trigger < Grape::Entity
-      include ::API::Helpers::Presentable
-
       expose :id
-      expose :token
-      expose :description
+      expose :token, :description
       expose :created_at, :updated_at, :last_used
       expose :owner, using: Entities::UserBasic
     end
diff --git a/lib/api/helpers/presentable.rb b/lib/api/helpers/presentable.rb
deleted file mode 100644
index 973c2132efe..00000000000
--- a/lib/api/helpers/presentable.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-# frozen_string_literal: true
-
-module API
-  module Helpers
-    ##
-    # This module makes it possible to use `app/presenters` with
-    # Grape Entities. It instantiates model presenter and passes
-    # options defined in the API endpoint to the presenter itself.
-    #
-    #   present object, with: Entities::Something,
-    #                   current_user: current_user,
-    #                   another_option: 'my options'
-    #
-    # Example above will make `current_user` and `another_option`
-    # values available in the subclass of `Gitlab::View::Presenter`
-    # thorough a separate method in the presenter.
-    #
-    # The model class needs to have `::Presentable` module mixed in
-    # if you want to use `API::Helpers::Presentable`.
-    #
-    module Presentable
-      extend ActiveSupport::Concern
-
-      def initialize(object, options = {})
-        super(object.present(options), options)
-      end
-    end
-  end
-end
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index f3a0d7b7b43..f784c857883 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -51,7 +51,7 @@ module API
 
         triggers = user_project.triggers.includes(:trigger_requests)
 
-        present paginate(triggers), with: Entities::Trigger, current_user: current_user
+        present paginate(triggers), with: Entities::Trigger
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
@@ -68,7 +68,7 @@ module API
         trigger = user_project.triggers.find(params.delete(:trigger_id))
         break not_found!('Trigger') unless trigger
 
-        present trigger, with: Entities::Trigger, current_user: current_user
+        present trigger, with: Entities::Trigger
       end
 
       desc 'Create a trigger' do
@@ -85,7 +85,7 @@ module API
           declared_params(include_missing: false).merge(owner: current_user))
 
         if trigger.valid?
-          present trigger, with: Entities::Trigger, current_user: current_user
+          present trigger, with: Entities::Trigger
         else
           render_validation_error!(trigger)
         end
@@ -106,7 +106,7 @@ module API
         break not_found!('Trigger') unless trigger
 
         if trigger.update(declared_params(include_missing: false))
-          present trigger, with: Entities::Trigger, current_user: current_user
+          present trigger, with: Entities::Trigger
         else
           render_validation_error!(trigger)
         end
@@ -127,7 +127,7 @@ module API
 
         if trigger.update(owner: current_user)
           status :ok
-          present trigger, with: Entities::Trigger, current_user: current_user
+          present trigger, with: Entities::Trigger
         else
           render_validation_error!(trigger)
         end
diff --git a/spec/presenters/ci/trigger_presenter_spec.rb b/spec/presenters/ci/trigger_presenter_spec.rb
deleted file mode 100644
index 231b539c188..00000000000
--- a/spec/presenters/ci/trigger_presenter_spec.rb
+++ /dev/null
@@ -1,51 +0,0 @@
-require 'spec_helper'
-
-describe Ci::TriggerPresenter do
-  set(:user) { create(:user) }
-  set(:project) { create(:project) }
-
-  set(:trigger) do
-    create(:ci_trigger, token: '123456789abcd', project: project)
-  end
-
-  subject do
-    described_class.new(trigger, current_user: user)
-  end
-
-  before do
-    project.add_maintainer(user)
-  end
-
-  context 'when user is not a trigger owner' do
-    describe '#token' do
-      it 'exposes only short token' do
-        expect(subject.token).not_to eq trigger.token
-        expect(subject.token).to eq '1234'
-      end
-    end
-
-    describe '#has_token_exposed?' do
-      it 'does not have token exposed' do
-        expect(subject).not_to have_token_exposed
-      end
-    end
-  end
-
-  context 'when user is a trigger owner and builds admin' do
-    before do
-      trigger.update(owner: user)
-    end
-
-    describe '#token' do
-      it 'exposes full token' do
-        expect(subject.token).to eq trigger.token
-      end
-    end
-
-    describe '#has_token_exposed?' do
-      it 'has token exposed' do
-        expect(subject).to have_token_exposed
-      end
-    end
-  end
-end
diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb
index e7ab1a2a097..0ae6796d1e4 100644
--- a/spec/requests/api/triggers_spec.rb
+++ b/spec/requests/api/triggers_spec.rb
@@ -1,9 +1,8 @@
 require 'spec_helper'
 
 describe API::Triggers do
-  set(:user) { create(:user) }
-  set(:user2) { create(:user) }
-
+  let(:user) { create(:user) }
+  let(:user2) { create(:user) }
   let!(:trigger_token) { 'secure_token' }
   let!(:trigger_token_2) { 'secure_token_2' }
   let!(:project) { create(:project, :repository, creator: user) }
@@ -133,17 +132,14 @@ describe API::Triggers do
   end
 
   describe 'GET /projects/:id/triggers' do
-    context 'authenticated user who can access triggers' do
-      it 'returns a list of triggers with tokens exposed correctly' do
+    context 'authenticated user with valid permissions' do
+      it 'returns list of triggers' do
         get api("/projects/#{project.id}/triggers", user)
 
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
-
         expect(json_response).to be_a(Array)
-        expect(json_response.size).to eq 2
-        expect(json_response.dig(0, 'token')).to eq trigger_token
-        expect(json_response.dig(1, 'token')).to eq trigger_token_2[0..3]
+        expect(json_response[0]).to have_key('token')
       end
     end