Do not expose trigger token when user should not see it

8 files changed, 68 insertions, 13 deletions

diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb
index f5fdfb8accc..c7b4ebb2b24 100644
--- a/app/controllers/projects/triggers_controller.rb
+++ b/app/controllers/projects/triggers_controller.rb
@@ -66,12 +66,11 @@ class Projects::TriggersController < Projects::ApplicationController
   end
 
   def trigger
-    @trigger ||= project.triggers.find(params[:id]) || render_404
+    @trigger ||= project.triggers.find(params[:id])
+      .present(current_user: current_user)
   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 55db42162ca..3a9cdfcc35e 100644
--- a/app/models/ci/trigger.rb
+++ b/app/models/ci/trigger.rb
@@ -4,6 +4,7 @@ module Ci
   class Trigger < ActiveRecord::Base
     extend Gitlab::Ci::Model
     include IgnorableColumn
+    include Presentable
 
     ignore_column :deleted_at
 
diff --git a/app/presenters/ci/trigger_presenter.rb b/app/presenters/ci/trigger_presenter.rb
new file mode 100644
index 00000000000..605c8f328a4
--- /dev/null
+++ b/app/presenters/ci/trigger_presenter.rb
@@ -0,0 +1,19 @@
+# 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 7e4618e1a88..6f6f1e5e0c5 100644
--- a/app/views/projects/triggers/_trigger.html.haml
+++ b/app/views/projects/triggers/_trigger.html.haml
@@ -1,6 +1,6 @@
 %tr
   %td
-    - if can?(current_user, :admin_trigger, trigger)
+    - if trigger.has_token_exposed?
       %span= trigger.token
       = clipboard_button(text: trigger.token, title: "Copy trigger token to clipboard")
     - else
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index a2a3c0a16d7..829d6fb13d4 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1216,8 +1216,11 @@ module API
     end
 
     class Trigger < Grape::Entity
+      include ::API::Helpers::Presentable
+
       expose :id
-      expose :token, :description
+      expose :token
+      expose :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
new file mode 100644
index 00000000000..973c2132efe
--- /dev/null
+++ b/lib/api/helpers/presentable.rb
@@ -0,0 +1,29 @@
+# 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 3ce1529f259..7b8691171f0 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
+        present paginate(triggers), with: Entities::Trigger, current_user: current_user
       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
+        present trigger, with: Entities::Trigger, current_user: current_user
       end
 
       desc 'Create a trigger' do
diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb
index 15dc901d06e..f0f01e97f1d 100644
--- a/spec/requests/api/triggers_spec.rb
+++ b/spec/requests/api/triggers_spec.rb
@@ -1,8 +1,9 @@
 require 'spec_helper'
 
 describe API::Triggers do
-  let(:user) { create(:user) }
-  let(:user2) { create(:user) }
+  set(:user) { create(:user) }
+  set(:user2) { create(:user) }
+
   let!(:trigger_token) { 'secure_token' }
   let!(:trigger_token_2) { 'secure_token_2' }
   let!(:project) { create(:project, :repository, creator: user) }
@@ -132,14 +133,17 @@ describe API::Triggers do
   end
 
   describe 'GET /projects/:id/triggers' do
-    context 'authenticated user with valid permissions' do
-      it 'returns list of triggers' do
+    context 'authenticated user who can access triggers' do
+      it 'returns a list of triggers with tokens exposed correctly' 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[0]).to have_key('token')
+        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]
       end
     end