From e561b142fa7c9dd636fd056fc0a6c84961d0cd46 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Mon, 24 Jul 2017 17:45:12 -0300 Subject: Backport gitlab-ee!2456 --- app/controllers/admin/applications_controller.rb | 2 +- app/views/admin/applications/_form.html.haml | 8 ++++++++ app/views/admin/applications/index.html.haml | 2 ++ app/views/admin/applications/show.html.haml | 6 ++++++ .../skip-oauth-authorization-for-trusted-applications.yml | 4 ++++ config/initializers/doorkeeper.rb | 6 +++--- ...0717200542_add_trusted_column_to_oauth_applications.rb | 15 +++++++++++++++ db/schema.rb | 1 + doc/integration/oauth_provider.md | 3 +++ spec/controllers/admin/applications_controller_spec.rb | 11 ++++++++--- spec/controllers/oauth/authorizations_controller_spec.rb | 2 +- spec/features/admin/admin_manage_applications_spec.rb | 5 +++++ 12 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/skip-oauth-authorization-for-trusted-applications.yml create mode 100644 db/migrate/20170717200542_add_trusted_column_to_oauth_applications.rb diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index 434ff6b2a62..4cb6e63d5e3 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[: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 1ec25c7d46f..0f14f8ac0ba 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -997,6 +997,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 -- cgit v1.2.1