Commit bc42ef72 authored by Drew Blessing's avatar Drew Blessing Committed by Drew Blessing

Require confirmed email address for GitLab OAuth authentication

Allowing users with unconfirmed/unverified email addresses to
authenticate to an external service using GitLab OAuth is not
secure. This change enforces that a user must have a confirmed
primary email address to proceed with OAuth.
parent 0a066d83
......@@ -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
---
title: Require confirmed email address for GitLab OAuth authentication
merge_request:
author:
type: security
......@@ -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
......
# frozen_string_literal: true
require 'spec_helper'
describe 'JIRA OAuth Provider' do
describe 'JIRA DVCS OAuth Authorization' do
let(:application) { create(:oauth_application, redirect_uri: oauth_jira_callback_url, scopes: 'read_user') }
before do
sign_in(user)
visit oauth_jira_authorize_path(client_id: application.uid,
redirect_uri: oauth_jira_callback_url,
response_type: 'code',
state: 'my_state',
scope: 'read_user')
end
it_behaves_like 'Secure OAuth Authorizations'
end
end
......@@ -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
# 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
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment