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

Verify confirmed email for OAuth Authorize POST endpoint

Similar to the recent change to require email confirmation/verification
for the OAuth Authorize GET (:new) endpoint, require the same
for the OAuth Authorize POST (:create) endpoint. This will prevent
forcing a POST request to authenticate to an external service
with an unconfirmed email address.
parent dfa3953c
...@@ -4,7 +4,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController ...@@ -4,7 +4,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
include Gitlab::Experimentation::ControllerConcern include Gitlab::Experimentation::ControllerConcern
include InitializesCurrentUserMode include InitializesCurrentUserMode
before_action :verify_confirmed_email!, only: [:new] before_action :verify_confirmed_email!
layout 'profile' layout 'profile'
......
---
title: Verify confirmed email for OAuth Authorize POST endpoint
merge_request:
author:
type: security
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Oauth::AuthorizationsController do RSpec.describe Oauth::AuthorizationsController do
let(:user) { create(:user, confirmed_at: confirmed_at) }
let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') } let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') }
let(:params) do let(:params) do
{ {
...@@ -17,9 +18,47 @@ RSpec.describe Oauth::AuthorizationsController do ...@@ -17,9 +18,47 @@ RSpec.describe Oauth::AuthorizationsController do
sign_in(user) sign_in(user)
end end
shared_examples 'OAuth Authorizations require confirmed user' do
context 'when the user is confirmed' do
let(:confirmed_at) { 1.hour.ago }
context 'when there is already an access token for the application with a matching scope' do
before do
scopes = Doorkeeper::OAuth::Scopes.from_string('api')
allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes)
create(:oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes)
end
it 'authorizes the request and redirects' do
subject
expect(request.session['user_return_to']).to be_nil
expect(response).to have_gitlab_http_status(:found)
end
end
end
context 'when the user is unconfirmed' do
let(:confirmed_at) { nil }
it 'returns 200 and renders error view' 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 describe 'GET #new' do
subject { get :new, params: params }
include_examples 'OAuth Authorizations require confirmed user'
context 'when the user is confirmed' do context 'when the user is confirmed' do
let(:user) { create(:user) } let(:confirmed_at) { 1.hour.ago }
context 'without valid params' do context 'without valid params' do
it 'returns 200 code and renders error view' do it 'returns 200 code and renders error view' do
...@@ -34,7 +73,7 @@ RSpec.describe Oauth::AuthorizationsController do ...@@ -34,7 +73,7 @@ RSpec.describe Oauth::AuthorizationsController do
render_views render_views
it 'returns 200 code and renders view' do it 'returns 200 code and renders view' do
get :new, params: params subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('doorkeeper/authorizations/new') expect(response).to render_template('doorkeeper/authorizations/new')
...@@ -44,42 +83,24 @@ RSpec.describe Oauth::AuthorizationsController do ...@@ -44,42 +83,24 @@ RSpec.describe Oauth::AuthorizationsController do
application.update(trusted: true) application.update(trusted: true)
request.session['user_return_to'] = 'http://example.com' request.session['user_return_to'] = 'http://example.com'
get :new, params: params subject
expect(request.session['user_return_to']).to be_nil expect(request.session['user_return_to']).to be_nil
expect(response).to have_gitlab_http_status(:found) expect(response).to have_gitlab_http_status(:found)
end 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')
allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes)
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
expect(request.session['user_return_to']).to be_nil
expect(response).to have_gitlab_http_status(:found)
end
end
end
end end
end end
end
context 'when the user is unconfirmed' do describe 'POST #create' do
let(:user) { create(:user, confirmed_at: nil) } subject { post :create, params: params }
it 'returns 200 and renders error view' do include_examples 'OAuth Authorizations require confirmed user'
get :new, params: params end
expect(response).to have_gitlab_http_status(:ok) describe 'DELETE #destroy' do
expect(response).to render_template('doorkeeper/authorizations/error') subject { delete :destroy, params: params }
end
end include_examples 'OAuth Authorizations require confirmed user'
end 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