diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-01-13 13:44:22 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-01-13 13:44:22 +0300 |
commit | a21195dc6ae25bc969625918bc87ab0b6a133edd (patch) | |
tree | 1a11b16b26dc1c916912fe8f2b58c00669ead959 | |
parent | 0027e00e2ee1a581370530493eea9b0bfa597990 (diff) |
Add latest changes from gitlab-org/security/gitlab@13-6-stable-ee
3 files changed, 36 insertions, 25 deletions
diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index ade698baa7f..857f36e3833 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -4,7 +4,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController include Gitlab::Experimentation::ControllerConcern include InitializesCurrentUserMode - before_action :verify_confirmed_email! + before_action :verify_confirmed_email!, :verify_confidential_application! layout 'profile' @@ -24,18 +24,19 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController end end - def create - # Confidential apps require the client_secret to be sent with the request. - # Doorkeeper allows implicit grant flow requests (response_type=token) to - # work without client_secret regardless of the confidential setting. - if pre_auth.authorizable? && pre_auth.response_type == 'token' && pre_auth.client.application.confidential - render "doorkeeper/authorizations/error" - else - super - end + private + + # Confidential apps require the client_secret to be sent with the request. + # Doorkeeper allows implicit grant flow requests (response_type=token) to + # work without client_secret regardless of the confidential setting. + # This leads to security vulnerabilities and we want to block it. + def verify_confidential_application! + render 'doorkeeper/authorizations/error' if authorizable_confidential? end - private + def authorizable_confidential? + pre_auth.authorizable? && pre_auth.response_type == 'token' && pre_auth.client.application.confidential + end def verify_confirmed_email! return if current_user&.confirmed? diff --git a/changelogs/unreleased/security-deny-implicit-confidential.yml b/changelogs/unreleased/security-deny-implicit-confidential.yml new file mode 100644 index 00000000000..bbf2d95b3fb --- /dev/null +++ b/changelogs/unreleased/security-deny-implicit-confidential.yml @@ -0,0 +1,5 @@ +--- +title: Deny implicit flow for confidential apps +merge_request: +author: +type: security diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index f811f13def8..2df94a06b3e 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -51,10 +51,27 @@ RSpec.describe Oauth::AuthorizationsController do end end + shared_examples "Implicit grant can't be used in confidential application" do + context 'when application is confidential' do + before do + application.update(confidential: true) + params[:response_type] = 'token' + end + + it 'does not allow the implicit flow' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/error') + end + end + end + describe 'GET #new' do subject { get :new, params: params } include_examples 'OAuth Authorizations require confirmed user' + include_examples "Implicit grant can't be used in confidential application" context 'when the user is confirmed' do let(:confirmed_at) { 1.hour.ago } @@ -95,26 +112,14 @@ RSpec.describe Oauth::AuthorizationsController do subject { post :create, params: params } include_examples 'OAuth Authorizations require confirmed user' - - context 'when application is confidential' do - before do - application.update(confidential: true) - params[:response_type] = 'token' - end - - it 'does not allow the implicit flow' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template('doorkeeper/authorizations/error') - end - end + include_examples "Implicit grant can't be used in confidential application" end describe 'DELETE #destroy' do subject { delete :destroy, params: params } include_examples 'OAuth Authorizations require confirmed user' + include_examples "Implicit grant can't be used in confidential application" end it 'includes Two-factor enforcement concern' do |