Merge branch 'connorshea/gitlab-ce-revoke-authorized-application' into 'master'

Fix revoking of authorized OAuth applications

Users were not able to revoke access to authorized OAuth applications. Clicking the "Revoke" button would result in a 404 page, and the application would still be authorized.

Added a spec and also found that the `gon` variables were not being set for this view.

Closes #14370

See merge request !3690

Signed-off-by: Rémy Coutable <remy@rymai.me>

10 files changed, 113 insertions, 16 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 3af66024ea4..db4e01a8d12 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.7.0 (unreleased)
+  - The Projects::HousekeepingService class has extra instrumentation (Yorick Peterse)
   - All service classes (those residing in app/services) are now instrumented (Yorick Peterse)
   - Enable gzip for assets, makes the page size significantly smaller. !3544 / !3632 (Connor Shea)
   - Load award emoji images separately unless opening the full picker. Saves several hundred KBs of data for most pages. (Connor Shea)
@@ -46,6 +47,7 @@ v 8.7.0 (unreleased)
 v 8.6.6
   - Expire the exists cache before deletion to ensure project dir actually exists (Stan Hu). !3413
   - Fix error on language detection when repository has no HEAD (e.g., master branch) (Jeroen Bobbeldijk). !3654
+  - Fix revoking of authorized OAuth applications (Connor Shea). !3690
 
 v 8.6.5
   - Fix importing from GitHub Enterprise. !3529
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 3a0eb96a460..1e0b87b0e26 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -3,6 +3,7 @@ require 'fogbugz'
 
 class ApplicationController < ActionController::Base
   include Gitlab::CurrentSettings
+  include Gitlab::GonHelper
   include GitlabRoutingHelper
   include PageLayoutHelper
 
@@ -150,20 +151,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def add_gon_variables
-    gon.api_version            = API::API.version
-    gon.default_avatar_url     = URI::join(Gitlab.config.gitlab.url, ActionController::Base.helpers.image_path('no_avatar.png')).to_s
-    gon.default_issues_tracker = Project.new.default_issue_tracker.to_param
-    gon.max_file_size          = current_application_settings.max_attachment_size
-    gon.relative_url_root      = Gitlab.config.gitlab.relative_url_root
-    gon.user_color_scheme      = Gitlab::ColorSchemes.for_user(current_user).css_class
-
-    if current_user
-      gon.current_user_id = current_user.id
-      gon.api_token = current_user.private_token
-    end
-  end
-
   def validate_user_service_ticket!
     return unless signed_in? && session[:service_tickets]
 
diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb
index d1e4ac10f6c..c6bdd0602c1 100644
--- a/app/controllers/oauth/applications_controller.rb
+++ b/app/controllers/oauth/applications_controller.rb
@@ -1,9 +1,11 @@
 class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   include Gitlab::CurrentSettings
+  include Gitlab::GonHelper
   include PageLayoutHelper
 
   before_action :verify_user_oauth_applications_enabled
   before_action :authenticate_user!
+  before_action :add_gon_variables
 
   layout 'profile'
 
diff --git a/app/models/oauth_access_token.rb b/app/models/oauth_access_token.rb
new file mode 100644
index 00000000000..c78c7f4aa0e
--- /dev/null
+++ b/app/models/oauth_access_token.rb
@@ -0,0 +1,19 @@
+# == Schema Information
+#
+# Table name: oauth_access_tokens
+#
+#  id                :integer          not null, primary key
+#  resource_owner_id :integer
+#  application_id    :integer
+#  token             :string           not null
+#  refresh_token     :string
+#  expires_in        :integer
+#  revoked_at        :datetime
+#  created_at        :datetime         not null
+#  scopes            :string
+#
+
+class OauthAccessToken < ActiveRecord::Base
+  belongs_to :resource_owner, class_name: 'User'
+  belongs_to :application, class_name: 'Doorkeeper::Application'
+end
diff --git a/app/views/doorkeeper/applications/index.html.haml b/app/views/doorkeeper/applications/index.html.haml
index 55f4a6f287d..0aff79749ef 100644
--- a/app/views/doorkeeper/applications/index.html.haml
+++ b/app/views/doorkeeper/applications/index.html.haml
@@ -68,7 +68,7 @@
                   %td= app.name
                   %td= token.created_at
                   %td= token.scopes
-                  %td= render 'delete_form', application: app
+                  %td= render 'doorkeeper/authorized_applications/delete_form', application: app
               - @authorized_anonymous_tokens.each do |token|
                 %tr
                   %td
diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb
new file mode 100644
index 00000000000..5ebaad6ca6e
--- /dev/null
+++ b/lib/gitlab/gon_helper.rb
@@ -0,0 +1,17 @@
+module Gitlab
+  module GonHelper
+    def add_gon_variables
+      gon.api_version            = API::API.version
+      gon.default_avatar_url     = URI::join(Gitlab.config.gitlab.url, ActionController::Base.helpers.image_path('no_avatar.png')).to_s
+      gon.default_issues_tracker = Project.new.default_issue_tracker.to_param
+      gon.max_file_size          = current_application_settings.max_attachment_size
+      gon.relative_url_root      = Gitlab.config.gitlab.relative_url_root
+      gon.user_color_scheme      = Gitlab::ColorSchemes.for_user(current_user).css_class
+
+      if current_user
+        gon.current_user_id = current_user.id
+        gon.api_token = current_user.private_token
+      end
+    end
+  end
+end
diff --git a/spec/factories/oauth_access_tokens.rb b/spec/factories/oauth_access_tokens.rb
new file mode 100644
index 00000000000..7700b15d538
--- /dev/null
+++ b/spec/factories/oauth_access_tokens.rb
@@ -0,0 +1,22 @@
+# == Schema Information
+#
+# Table name: oauth_access_tokens
+#
+#  id                :integer          not null, primary key
+#  resource_owner_id :integer
+#  application_id    :integer
+#  token             :string           not null
+#  refresh_token     :string
+#  expires_in        :integer
+#  revoked_at        :datetime
+#  created_at        :datetime         not null
+#  scopes            :string
+#
+
+FactoryGirl.define do
+  factory :oauth_access_token do
+    resource_owner
+    application
+    token '123456'
+  end
+end
diff --git a/spec/factories/oauth_applications.rb b/spec/factories/oauth_applications.rb
new file mode 100644
index 00000000000..d116a573830
--- /dev/null
+++ b/spec/factories/oauth_applications.rb
@@ -0,0 +1,9 @@
+FactoryGirl.define do
+  factory :oauth_application, class: 'Doorkeeper::Application', aliases: [:application] do
+    name { FFaker::Name.name }
+    uid { FFaker::Name.name }
+    redirect_uri { FFaker::Internet.uri('http') }
+    owner
+    owner_type 'User'
+  end
+end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index a5c60c51c5b..a9b2148bd2a 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -1,7 +1,7 @@
 FactoryGirl.define do
   sequence(:name) { FFaker::Name.name }
 
-  factory :user, aliases: [:author, :assignee, :recipient, :owner, :creator] do
+  factory :user, aliases: [:author, :assignee, :recipient, :owner, :creator, :resource_owner] do
     email { FFaker::Internet.email }
     name
     sequence(:username) { |n| "#{FFaker::Internet.user_name}#{n}" }
diff --git a/spec/features/profiles/oauth_applications_spec.rb b/spec/features/profiles/oauth_applications_spec.rb
new file mode 100644
index 00000000000..1a5a9059dbd
--- /dev/null
+++ b/spec/features/profiles/oauth_applications_spec.rb
@@ -0,0 +1,39 @@
+require 'spec_helper'
+
+describe 'Profile > Applications', feature: true do
+  let(:user) { create(:user) }
+
+  before do
+    login_as(user)
+  end
+
+  describe 'User manages applications', js: true do
+    it 'deletes an application' do
+      create(:oauth_application, owner: user)
+      visit oauth_applications_path
+
+      page.within('.oauth-applications') do
+        expect(page).to have_content('Your applications (1)')
+        click_button 'Destroy'
+      end
+
+      expect(page).to have_content('The application was deleted successfully')
+      expect(page).to have_content('Your applications (0)')
+      expect(page).to have_content('Authorized applications (0)')
+    end
+
+    it 'deletes an authorized application' do
+      create(:oauth_access_token, resource_owner: user)
+      visit oauth_applications_path
+
+      page.within('.oauth-authorized-applications') do
+        expect(page).to have_content('Authorized applications (1)')
+        click_button 'Revoke'
+      end
+
+      expect(page).to have_content('The application was revoked access.')
+      expect(page).to have_content('Your applications (0)')
+      expect(page).to have_content('Authorized applications (0)')
+    end
+  end
+end