summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2017-07-27 17:41:40 +0000
committerRobert Speicher <robert@gitlab.com>2017-07-27 17:41:40 +0000
commit86ae883b638d29953f26a87efc16ae613ff865e4 (patch)
tree6d216f1ec842e8cb72a9efac4ba576146aa176a5
parent066f4d8b715aa6c58bffe39b7f9fb35e6e6ff8a9 (diff)
parentf837cd6611a39d3dd38655821f32feb2c2abba8d (diff)
downloadgitlab-ce-86ae883b638d29953f26a87efc16ae613ff865e4.tar.gz
Merge branch 'backport-ee-2456' into 'master'
Skip OAuth authorization for trusted applications See merge request !13061
-rw-r--r--app/controllers/admin/applications_controller.rb2
-rw-r--r--app/views/admin/applications/_form.html.haml8
-rw-r--r--app/views/admin/applications/index.html.haml2
-rw-r--r--app/views/admin/applications/show.html.haml6
-rw-r--r--changelogs/unreleased/skip-oauth-authorization-for-trusted-applications.yml4
-rw-r--r--config/initializers/doorkeeper.rb6
-rw-r--r--db/migrate/20170717200542_add_trusted_column_to_oauth_applications.rb15
-rw-r--r--db/schema.rb1
-rw-r--r--doc/integration/oauth_provider.md3
-rw-r--r--spec/controllers/admin/applications_controller_spec.rb11
-rw-r--r--spec/controllers/oauth/authorizations_controller_spec.rb2
-rw-r--r--spec/features/admin/admin_manage_applications_spec.rb5
12 files changed, 57 insertions, 8 deletions
diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb
index 434ff6b2a62..16590e66d61 100644
--- a/app/controllers/admin/applications_controller.rb
+++ b/app/controllers/admin/applications_controller.rb
@@ -50,6 +50,6 @@ class Admin::ApplicationsController < Admin::ApplicationController
# Only allow a trusted parameter "white list" through.
def application_params
- params[:doorkeeper_application].permit(:name, :redirect_uri, :scopes)
+ params.require(:doorkeeper_application).permit(:name, :redirect_uri, :trusted, :scopes)
end
end
diff --git a/app/views/admin/applications/_form.html.haml b/app/views/admin/applications/_form.html.haml
index 061f8991b11..93827d6a1ab 100644
--- a/app/views/admin/applications/_form.html.haml
+++ b/app/views/admin/applications/_form.html.haml
@@ -6,6 +6,7 @@
.col-sm-10
= f.text_field :name, class: 'form-control'
= doorkeeper_errors_for application, :name
+
= content_tag :div, class: 'form-group' do
= f.label :redirect_uri, class: 'col-sm-2 control-label'
.col-sm-10
@@ -19,6 +20,13 @@
%code= Doorkeeper.configuration.native_redirect_uri
for local tests
+ = content_tag :div, class: 'form-group' do
+ = f.label :trusted, class: 'col-sm-2 control-label'
+ .col-sm-10
+ = f.check_box :trusted
+ %span.help-block
+ Trusted applications are automatically authorized on GitLab OAuth flow.
+
.form-group
= f.label :scopes, class: 'col-sm-2 control-label'
.col-sm-10
diff --git a/app/views/admin/applications/index.html.haml b/app/views/admin/applications/index.html.haml
index eb4293c7e37..94d33fa6489 100644
--- a/app/views/admin/applications/index.html.haml
+++ b/app/views/admin/applications/index.html.haml
@@ -11,6 +11,7 @@
%th Name
%th Callback URL
%th Clients
+ %th Trusted
%th
%th
%tbody.oauth-applications
@@ -19,5 +20,6 @@
%td= link_to application.name, admin_application_path(application)
%td= application.redirect_uri
%td= application.access_tokens.map(&:resource_owner_id).uniq.count
+ %td= application.trusted? ? 'Y': 'N'
%td= link_to 'Edit', edit_admin_application_path(application), class: 'btn btn-link'
%td= render 'delete_form', application: application
diff --git a/app/views/admin/applications/show.html.haml b/app/views/admin/applications/show.html.haml
index 14683cc66e9..5125aa21b06 100644
--- a/app/views/admin/applications/show.html.haml
+++ b/app/views/admin/applications/show.html.haml
@@ -23,6 +23,12 @@
%div
%span.monospace= uri
+ %tr
+ %td
+ Trusted
+ %td
+ = @application.trusted? ? 'Y' : 'N'
+
= render "shared/tokens/scopes_list", token: @application
.form-actions
diff --git a/changelogs/unreleased/skip-oauth-authorization-for-trusted-applications.yml b/changelogs/unreleased/skip-oauth-authorization-for-trusted-applications.yml
new file mode 100644
index 00000000000..7b4ae355978
--- /dev/null
+++ b/changelogs/unreleased/skip-oauth-authorization-for-trusted-applications.yml
@@ -0,0 +1,4 @@
+---
+title: Skip oAuth authorization for trusted applications
+merge_request:
+author:
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb
index 8e2e639fc41..40e635bf2cf 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -92,9 +92,9 @@ Doorkeeper.configure do
# Under some circumstances you might want to have applications auto-approved,
# so that the user skips the authorization step.
# For example if dealing with trusted a application.
- # skip_authorization do |resource_owner, client|
- # client.superapp? or resource_owner.admin?
- # end
+ skip_authorization do |resource_owner, client|
+ client.application.trusted?
+ end
# WWW-Authenticate Realm (default "Doorkeeper").
# realm "Doorkeeper"
diff --git a/db/migrate/20170717200542_add_trusted_column_to_oauth_applications.rb b/db/migrate/20170717200542_add_trusted_column_to_oauth_applications.rb
new file mode 100644
index 00000000000..1a013e6aefb
--- /dev/null
+++ b/db/migrate/20170717200542_add_trusted_column_to_oauth_applications.rb
@@ -0,0 +1,15 @@
+class AddTrustedColumnToOauthApplications < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ add_column_with_default(:oauth_applications, :trusted, :boolean, default: false)
+ end
+
+ def down
+ remove_column(:oauth_applications, :trusted)
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 63030350c5d..93ab79e14e0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1027,6 +1027,7 @@ ActiveRecord::Schema.define(version: 20170725145659) do
t.datetime "updated_at"
t.integer "owner_id"
t.string "owner_type"
+ t.boolean "trusted", default: false, null: false
end
add_index "oauth_applications", ["owner_id", "owner_type"], name: "index_oauth_applications_on_owner_id_and_owner_type", using: :btree
diff --git a/doc/integration/oauth_provider.md b/doc/integration/oauth_provider.md
index af8a1c4e5ed..8ba2e8731c8 100644
--- a/doc/integration/oauth_provider.md
+++ b/doc/integration/oauth_provider.md
@@ -63,6 +63,9 @@ it from the admin area.
![OAuth admin_applications](img/oauth_provider_admin_application.png)
+You're also able to mark an application as _trusted_ when creating it through the admin area. By doing that,
+the user authorization step is automatically skipped for this application.
+
---
## Authorized applications
diff --git a/spec/controllers/admin/applications_controller_spec.rb b/spec/controllers/admin/applications_controller_spec.rb
index e311b8a63b2..7bd6c0e6117 100644
--- a/spec/controllers/admin/applications_controller_spec.rb
+++ b/spec/controllers/admin/applications_controller_spec.rb
@@ -28,13 +28,16 @@ describe Admin::ApplicationsController do
describe 'POST #create' do
it 'creates the application' do
+ create_params = attributes_for(:application, trusted: true)
+
expect do
- post :create, doorkeeper_application: attributes_for(:application)
+ post :create, doorkeeper_application: create_params
end.to change { Doorkeeper::Application.count }.by(1)
application = Doorkeeper::Application.last
expect(response).to redirect_to(admin_application_path(application))
+ expect(application).to have_attributes(create_params.except(:uid, :owner_type))
end
it 'renders the application form on errors' do
@@ -49,10 +52,12 @@ describe Admin::ApplicationsController do
describe 'PATCH #update' do
it 'updates the application' do
- patch :update, id: application.id, doorkeeper_application: { redirect_uri: 'http://example.com/' }
+ patch :update, id: application.id, doorkeeper_application: { redirect_uri: 'http://example.com/', trusted: true }
+
+ application.reload
expect(response).to redirect_to(admin_application_path(application))
- expect(application.reload.redirect_uri).to eq 'http://example.com/'
+ expect(application).to have_attributes(redirect_uri: 'http://example.com/', trusted: true)
end
it 'renders the application form on errors' do
diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb
index d321bfcea9d..ac7f73c6e81 100644
--- a/spec/controllers/oauth/authorizations_controller_spec.rb
+++ b/spec/controllers/oauth/authorizations_controller_spec.rb
@@ -42,8 +42,8 @@ describe Oauth::AuthorizationsController do
end
it 'deletes session.user_return_to and redirects when skip authorization' do
+ doorkeeper.update(trusted: true)
request.session['user_return_to'] = 'http://example.com'
- allow(controller).to receive(:skip_authorization?).and_return(true)
get :new, params
diff --git a/spec/features/admin/admin_manage_applications_spec.rb b/spec/features/admin/admin_manage_applications_spec.rb
index c1ece123230..f979d2f6090 100644
--- a/spec/features/admin/admin_manage_applications_spec.rb
+++ b/spec/features/admin/admin_manage_applications_spec.rb
@@ -13,19 +13,24 @@ RSpec.describe 'admin manage applications' do
fill_in :doorkeeper_application_name, with: 'test'
fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com'
+ check :doorkeeper_application_trusted
click_on 'Submit'
expect(page).to have_content('Application: test')
expect(page).to have_content('Application Id')
expect(page).to have_content('Secret')
+ expect(page).to have_content('Trusted Y')
click_on 'Edit'
expect(page).to have_content('Edit application')
fill_in :doorkeeper_application_name, with: 'test_changed'
+ uncheck :doorkeeper_application_trusted
+
click_on 'Submit'
expect(page).to have_content('test_changed')
expect(page).to have_content('Application Id')
expect(page).to have_content('Secret')
+ expect(page).to have_content('Trusted N')
visit admin_applications_path
page.within '.oauth-applications' do