diff options
author | Rémy Coutable <remy@rymai.me> | 2016-04-14 15:26:09 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-04-14 15:26:09 +0000 |
commit | dd9ced0af9514c0cf511c8f0f10d19c014fa4d19 (patch) | |
tree | 8e2e8c380969bde9b96993bbfdc7791ce47f2b10 | |
parent | 4a514b27e96bfa6024e26b0a7500deec1d89391b (diff) | |
parent | e450892f5688529b8a49e3ae7598f00dbdda7161 (diff) | |
download | gitlab-ce-dd9ced0af9514c0cf511c8f0f10d19c014fa4d19.tar.gz |
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
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/application_controller.rb | 15 | ||||
-rw-r--r-- | app/controllers/oauth/applications_controller.rb | 2 | ||||
-rw-r--r-- | app/models/oauth_access_token.rb | 19 | ||||
-rw-r--r-- | app/views/doorkeeper/applications/index.html.haml | 2 | ||||
-rw-r--r-- | lib/gitlab/gon_helper.rb | 17 | ||||
-rw-r--r-- | spec/factories/oauth_access_tokens.rb | 22 | ||||
-rw-r--r-- | spec/factories/oauth_applications.rb | 9 | ||||
-rw-r--r-- | spec/factories/users.rb | 2 | ||||
-rw-r--r-- | spec/features/profiles/oauth_applications_spec.rb | 39 |
10 files changed, 112 insertions, 16 deletions
diff --git a/CHANGELOG b/CHANGELOG index 49a294226df..9aa40fae18e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,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) + - Fix revoking of authorized OAuth applications (Connor Shea) - All service classes (those residing in app/services) are now instrumented (Yorick Peterse) - Developers can now add custom tags to transactions (Yorick Peterse) - Loading of an issue's referenced merge requests and related branches is now done asynchronously (Yorick Peterse) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 97d53acde94..ce5c84ee9bc 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 @@ -158,20 +159,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 |