summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-01-13 10:44:41 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-01-13 10:44:41 +0000
commite861919633e0aac16509c0415f71eda69902bff9 (patch)
tree3e65f7fbe7e738bf0dba2a66c0bf82b962daba34
parent1ef3b81f122ba52e955bee694c38d6fb4dae3068 (diff)
downloadgitlab-ce-e861919633e0aac16509c0415f71eda69902bff9.tar.gz
Add latest changes from gitlab-org/security/gitlab@13-7-stable-ee
-rw-r--r--app/controllers/oauth/authorizations_controller.rb23
-rw-r--r--changelogs/unreleased/security-deny-implicit-confidential.yml5
-rw-r--r--spec/controllers/oauth/authorizations_controller_spec.rb33
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