From 3145a9d9fc49ec543cd1cd765fa7a5097f4afa4e Mon Sep 17 00:00:00 2001 From: Igor Drozdov <idrozdov@gitlab.com> Date: Tue, 1 Sep 2020 08:13:01 +0300 Subject: [PATCH] Bump doorkeeper to 5.3.0 Migrate the existing code to the new version according to: https://github.com/doorkeeper-gem/doorkeeper/wiki/Migration-from-old-versions#from-51x-to-52x --- Gemfile | 4 ++-- Gemfile.lock | 12 ++++++------ app/views/admin/applications/_form.html.haml | 5 ----- .../doorkeeper/applications/_form.html.haml | 3 --- changelogs/unreleased/id-bump-doorkeeper-5-3.yml | 5 +++++ config/initializers/doorkeeper.rb | 7 ------- config/locales/doorkeeper.en.yml | 7 ++++++- .../oauth/jira/authorizations_controller.rb | 1 + lib/api/applications.rb | 16 ++++++++++++++++ lib/mattermost/session.rb | 2 +- locale/gitlab.pot | 3 --- 11 files changed, 37 insertions(+), 28 deletions(-) create mode 100644 changelogs/unreleased/id-bump-doorkeeper-5-3.yml diff --git a/Gemfile b/Gemfile index a9f9912969f0..651fd0ea4cec 100644 --- a/Gemfile +++ b/Gemfile @@ -26,8 +26,8 @@ gem 'marginalia', '~> 1.9.0' # Authentication libraries gem 'devise', '~> 4.6' -gem 'doorkeeper', '~> 5.1.1' -gem 'doorkeeper-openid_connect', '~> 1.6.3' +gem 'doorkeeper', '~> 5.3.0' +gem 'doorkeeper-openid_connect', '~> 1.7.4' gem 'omniauth', '~> 1.8' gem 'omniauth-auth0', '~> 2.0.0' gem 'omniauth-azure-oauth2', '~> 0.0.9' diff --git a/Gemfile.lock b/Gemfile.lock index 0f5cfcdc056d..9fb860beab6f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -254,11 +254,11 @@ GEM docile (1.3.2) domain_name (0.5.20180417) unf (>= 0.0.5, < 1.0.0) - doorkeeper (5.1.1) + doorkeeper (5.3.3) railties (>= 5) - doorkeeper-openid_connect (1.6.3) - doorkeeper (>= 5.0, < 5.2) - json-jwt (~> 1.6) + doorkeeper-openid_connect (1.7.4) + doorkeeper (>= 5.2, < 5.5) + json-jwt (>= 1.11.0) dry-configurable (0.11.5) concurrent-ruby (~> 1.0) dry-core (~> 0.4, >= 0.4.7) @@ -1265,8 +1265,8 @@ DEPENDENCIES diff_match_patch (~> 0.1.0) diffy (~> 3.3) discordrb-webhooks-blackst0ne (~> 3.3) - doorkeeper (~> 5.1.1) - doorkeeper-openid_connect (~> 1.6.3) + doorkeeper (~> 5.3.0) + doorkeeper-openid_connect (~> 1.7.4) ed25519 (~> 1.2) elasticsearch-api (~> 6.8) elasticsearch-model (~> 6.1) diff --git a/app/views/admin/applications/_form.html.haml b/app/views/admin/applications/_form.html.haml index 8338401bea5c..0d01f1c57e08 100644 --- a/app/views/admin/applications/_form.html.haml +++ b/app/views/admin/applications/_form.html.haml @@ -16,11 +16,6 @@ = doorkeeper_errors_for application, :redirect_uri %span.form-text.text-muted Use one line per URI - - if Doorkeeper.configuration.native_redirect_uri - %span.form-text.text-muted - Use - %code= Doorkeeper.configuration.native_redirect_uri - for local tests = content_tag :div, class: 'form-group row' do .col-sm-2.col-form-label.pt-0 diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml index 7fbaa35d1d5c..f99db696fd6a 100644 --- a/app/views/doorkeeper/applications/_form.html.haml +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -11,9 +11,6 @@ %span.form-text.text-muted = _('Use one line per URI') - - if Doorkeeper.configuration.native_redirect_uri - %span.form-text.text-muted - = html_escape(_('Use %{native_redirect_uri} for local tests')) % { native_redirect_uri: tag.code(Doorkeeper.configuration.native_redirect_uri) } .form-group.form-check = f.check_box :confidential, class: 'form-check-input' diff --git a/changelogs/unreleased/id-bump-doorkeeper-5-3.yml b/changelogs/unreleased/id-bump-doorkeeper-5-3.yml new file mode 100644 index 000000000000..53c641aecbfc --- /dev/null +++ b/changelogs/unreleased/id-bump-doorkeeper-5-3.yml @@ -0,0 +1,5 @@ +--- +title: Bump doorkeeper to 5.3.0 +merge_request: 40929 +author: +type: changed diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 76e29fb6c027..5e8f3777a34b 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -79,13 +79,6 @@ # Check out the wiki for more information on customization access_token_methods :from_access_token_param, :from_bearer_authorization, :from_bearer_param - # Change the native redirect uri for client apps - # When clients register with the following redirect uri, they won't be redirected to any server and the authorization code will be displayed within the provider - # The value can be any string. Use nil to disable this feature. When disabled, clients must provide a valid URL - # (Similar behaviour: https://developers.google.com/accounts/docs/OAuth2InstalledApp#choosingredirecturi) - # - native_redirect_uri nil # 'urn:ietf:wg:oauth:2.0:oob' - # Specify what grant flows are enabled in array of Strings. The valid # strings and the flows they enable are: # diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 8469b72c312c..81e4f73e6b23 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -30,7 +30,6 @@ en: errors: messages: # Common error messages - invalid_request: 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.' invalid_redirect_uri: 'The redirect URI included is not valid.' unauthorized_client: 'The client is not authorized to perform this request using this method.' access_denied: 'The resource owner or authorization server denied the request.' @@ -54,6 +53,12 @@ en: # Password Access token errors invalid_resource_owner: 'The provided resource owner credentials are not valid, or resource owner cannot be found' + invalid_request: + unknown: 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.' + missing_param: 'Missing required parameter: %{value}.' + not_support_pkce: 'Invalid code_verifier parameter. Server does not support pkce.' + request_not_authorized: 'Request need to be authorized. Required parameter for authorizing request is missing or invalid.' + invalid_token: revoked: "The access token was revoked" expired: "The access token expired" diff --git a/ee/app/controllers/oauth/jira/authorizations_controller.rb b/ee/app/controllers/oauth/jira/authorizations_controller.rb index a3e30ffc9938..f552b0dc10c4 100644 --- a/ee/app/controllers/oauth/jira/authorizations_controller.rb +++ b/ee/app/controllers/oauth/jira/authorizations_controller.rb @@ -14,6 +14,7 @@ def new redirect_to oauth_authorization_path(client_id: params['client_id'], response_type: 'code', + scope: params['scope'], redirect_uri: oauth_jira_callback_url) end diff --git a/lib/api/applications.rb b/lib/api/applications.rb index 4e8d68c8d094..4f2c3ee79ef4 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -6,6 +6,15 @@ class Applications < Grape::API::Instance before { authenticated_as_admin! } resource :applications do + helpers do + def validate_redirect_uri(value) + uri = ::URI.parse(value) + !uri.is_a?(URI::HTTP) || uri.host + rescue URI::InvalidURIError + false + end + end + desc 'Create a new application' do detail 'This feature was introduced in GitLab 10.5' success Entities::ApplicationWithSecret @@ -19,6 +28,13 @@ class Applications < Grape::API::Instance desc: 'Application will be used where the client secret is confidential' end post do + # Validate that host in uri is specified + # Please remove it when https://github.com/doorkeeper-gem/doorkeeper/pull/1440 is merged + # and the doorkeeper gem version is bumped + unless validate_redirect_uri(declared_params[:redirect_uri]) + render_api_error!({ redirect_uri: ["must be an absolute URI."] }, :bad_request) + end + application = Doorkeeper::Application.new(declared_params) if application.save diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb index eea7daa3d8e1..ccdd1443fb05 100644 --- a/lib/mattermost/session.rb +++ b/lib/mattermost/session.rb @@ -52,7 +52,7 @@ def with_session # Next methods are needed for Doorkeeper def pre_auth @pre_auth ||= Doorkeeper::OAuth::PreAuthorization.new( - Doorkeeper.configuration, server.client_via_uid, params) + Doorkeeper.configuration, params) end def authorization diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9ed5691de91b..432934ade897 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26847,9 +26847,6 @@ msgstr "" msgid "Use %{code_start}::%{code_end} to create a %{link_start}scoped label set%{link_end} (eg. %{code_start}priority::1%{code_end})" msgstr "" -msgid "Use %{native_redirect_uri} for local tests" -msgstr "" - msgid "Use Service Desk to connect with your users (e.g. to offer customer support) through email right inside GitLab" msgstr "" -- GitLab