summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-05-26 14:34:23 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-05-26 14:34:23 +0000
commitf7fd30fc74686685b9582f291e27afe9466655e5 (patch)
tree4530f70d4affeef98b3bd385a0bb5bba6dbdbe7d
parente80b54a5d37aa0fad54e90a7bcc2af0de2f18be6 (diff)
downloadgitlab-ce-f7fd30fc74686685b9582f291e27afe9466655e5.tar.gz
Add latest changes from gitlab-org/security/gitlab@12-10-stable-ee
-rw-r--r--app/controllers/oauth/authorizations_controller.rb11
-rw-r--r--changelogs/unreleased/security-dblessing-oauth-email-verification.yml5
-rw-r--r--config/locales/doorkeeper.en.yml1
-rw-r--r--spec/controllers/oauth/authorizations_controller_spec.rb78
-rw-r--r--spec/features/oauth_provider_authorize_spec.rb21
-rw-r--r--spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb19
6 files changed, 103 insertions, 32 deletions
diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb
index 2a4e659c5b9..f6ad2bf5312 100644
--- a/app/controllers/oauth/authorizations_controller.rb
+++ b/app/controllers/oauth/authorizations_controller.rb
@@ -4,6 +4,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
include Gitlab::Experimentation::ControllerConcern
include InitializesCurrentUserMode
+ before_action :verify_confirmed_email!, only: [:new]
+
layout 'profile'
# Overridden from Doorkeeper::AuthorizationsController to
@@ -21,4 +23,13 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
render "doorkeeper/authorizations/error"
end
end
+
+ private
+
+ def verify_confirmed_email!
+ return if current_user&.confirmed?
+
+ pre_auth.error = :unconfirmed_email
+ render "doorkeeper/authorizations/error"
+ end
end
diff --git a/changelogs/unreleased/security-dblessing-oauth-email-verification.yml b/changelogs/unreleased/security-dblessing-oauth-email-verification.yml
new file mode 100644
index 00000000000..1f9a06d10d5
--- /dev/null
+++ b/changelogs/unreleased/security-dblessing-oauth-email-verification.yml
@@ -0,0 +1,5 @@
+---
+title: Require confirmed email address for GitLab OAuth authentication
+merge_request:
+author:
+type: security
diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml
index c9dbde23d35..e0e167a914d 100644
--- a/config/locales/doorkeeper.en.yml
+++ b/config/locales/doorkeeper.en.yml
@@ -36,6 +36,7 @@ en:
access_denied: 'The resource owner or authorization server denied the request.'
invalid_scope: 'The requested scope is invalid, unknown, or malformed.'
server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.'
+ unconfirmed_email: 'Verify the email address in your account profile before you sign in.'
temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.'
#configuration error messages
diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb
index 1b4bebd9707..f975502ca4e 100644
--- a/spec/controllers/oauth/authorizations_controller_spec.rb
+++ b/spec/controllers/oauth/authorizations_controller_spec.rb
@@ -3,7 +3,6 @@
require 'spec_helper'
describe Oauth::AuthorizationsController do
- let(:user) { create(:user) }
let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') }
let(:params) do
{
@@ -19,53 +18,68 @@ describe Oauth::AuthorizationsController do
end
describe 'GET #new' do
- context 'without valid params' do
- it 'returns 200 code and renders error view' do
- get :new
+ context 'when the user is confirmed' do
+ let(:user) { create(:user) }
- expect(response).to have_gitlab_http_status(:ok)
- expect(response).to render_template('doorkeeper/authorizations/error')
+ context 'without valid params' do
+ it 'returns 200 code and renders error view' do
+ get :new
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response).to render_template('doorkeeper/authorizations/error')
+ end
end
- end
- context 'with valid params' do
- render_views
+ context 'with valid params' do
+ render_views
- it 'returns 200 code and renders view' do
- get :new, params: params
+ it 'returns 200 code and renders view' do
+ get :new, params: params
- expect(response).to have_gitlab_http_status(:ok)
- expect(response).to render_template('doorkeeper/authorizations/new')
- end
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response).to render_template('doorkeeper/authorizations/new')
+ end
- it 'deletes session.user_return_to and redirects when skip authorization' do
- application.update(trusted: true)
- request.session['user_return_to'] = 'http://example.com'
+ it 'deletes session.user_return_to and redirects when skip authorization' do
+ application.update(trusted: true)
+ request.session['user_return_to'] = 'http://example.com'
- get :new, params: params
+ get :new, params: params
- expect(request.session['user_return_to']).to be_nil
- expect(response).to have_gitlab_http_status(:found)
- end
+ expect(request.session['user_return_to']).to be_nil
+ expect(response).to have_gitlab_http_status(:found)
+ end
- context 'when there is already an access token for the application' do
- context 'when the request scope matches any of the created token scopes' do
- before do
- scopes = Doorkeeper::OAuth::Scopes.from_string('api')
+ context 'when there is already an access token for the application' do
+ context 'when the request scope matches any of the created token scopes' do
+ before do
+ scopes = Doorkeeper::OAuth::Scopes.from_string('api')
- allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes)
+ allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes)
- create :oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes
- end
+ create :oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes
+ end
- it 'authorizes the request and redirects' do
- get :new, params: params
+ it 'authorizes the request and redirects' do
+ get :new, params: params
- expect(request.session['user_return_to']).to be_nil
- expect(response).to have_gitlab_http_status(:found)
+ expect(request.session['user_return_to']).to be_nil
+ expect(response).to have_gitlab_http_status(:found)
+ end
end
end
end
end
+
+ context 'when the user is unconfirmed' do
+ let(:user) { create(:user, confirmed_at: nil) }
+
+ it 'returns 200 and renders error view' do
+ get :new, params: params
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response).to render_template('doorkeeper/authorizations/error')
+ end
+ end
end
end
diff --git a/spec/features/oauth_provider_authorize_spec.rb b/spec/features/oauth_provider_authorize_spec.rb
new file mode 100644
index 00000000000..284fe3b0af9
--- /dev/null
+++ b/spec/features/oauth_provider_authorize_spec.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'OAuth Provider' do
+ describe 'Standard OAuth Authorization' do
+ let(:application) { create(:oauth_application, scopes: 'read_user') }
+
+ before do
+ sign_in(user)
+
+ visit oauth_authorization_path(client_id: application.uid,
+ redirect_uri: application.redirect_uri.split.first,
+ response_type: 'code',
+ state: 'my_state',
+ scope: 'read_user')
+ end
+
+ it_behaves_like 'Secure OAuth Authorizations'
+ end
+end
diff --git a/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb b/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb
new file mode 100644
index 00000000000..028e075c87a
--- /dev/null
+++ b/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'Secure OAuth Authorizations' do
+ context 'when user is confirmed' do
+ let(:user) { create(:user) }
+
+ it 'asks the user to authorize the application' do
+ expect(page).to have_text "Authorize #{application.name} to use your account?"
+ end
+ end
+
+ context 'when user is unconfirmed' do
+ let(:user) { create(:user, confirmed_at: nil) }
+
+ it 'displays an error' do
+ expect(page).to have_text I18n.t('doorkeeper.errors.messages.unconfirmed_email')
+ end
+ end
+end