Commit 5d9a9224 authored by Ron Chan's avatar Ron Chan

Adds redirect page to OAuth

The goal is to make sure the user to go through js-based redirect

Changelog: security
parent 2ae25491
...@@ -14,8 +14,9 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController ...@@ -14,8 +14,9 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
if pre_auth.authorizable? if pre_auth.authorizable?
if skip_authorization? || matching_token? if skip_authorization? || matching_token?
auth = authorization.authorize auth = authorization.authorize
parsed_redirect_uri = URI.parse(auth.redirect_uri)
session.delete(:user_return_to) session.delete(:user_return_to)
redirect_to auth.redirect_uri render "doorkeeper/authorizations/redirect", locals: { redirect_uri: parsed_redirect_uri }, layout: false
else else
render "doorkeeper/authorizations/new" render "doorkeeper/authorizations/new"
end end
......
%h3.page-title= _("Redirecting")
%div
%a{ :href => redirect_uri } Click here to redirect to #{redirect_uri}
:javascript
window.location= "#{redirect_uri}";
...@@ -27006,6 +27006,9 @@ msgstr "" ...@@ -27006,6 +27006,9 @@ msgstr ""
msgid "Redirect to SAML provider to test configuration" msgid "Redirect to SAML provider to test configuration"
msgstr "" msgstr ""
msgid "Redirecting"
msgstr ""
msgid "Redis" msgid "Redis"
msgstr "" msgstr ""
......
...@@ -70,76 +70,59 @@ RSpec.describe Oauth::AuthorizationsController do ...@@ -70,76 +70,59 @@ RSpec.describe Oauth::AuthorizationsController do
describe 'GET #new' do describe 'GET #new' do
subject { get :new, params: params } subject { get :new, params: params }
include_examples 'OAuth Authorizations require confirmed user'
include_examples "Implicit grant can't be used in confidential application" include_examples "Implicit grant can't be used in confidential application"
context 'rendering of views based on the ownership of the application' do context 'when the user is confirmed' do
shared_examples 'render views' do let(:confirmed_at) { 1.hour.ago }
render_views
it 'returns 200 and renders view with correct info', :aggregate_failures do
subject
expect(response).to have_gitlab_http_status(:ok) context 'when there is already an access token for the application with a matching scope' do
expect(response.body).to include(application.owner.name) before do
expect(response).to render_template('doorkeeper/authorizations/new') scopes = Doorkeeper::OAuth::Scopes.from_string('api')
end
end
subject { get :new, params: params } allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes)
context 'when auth app owner is a user' do create(:oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes)
context 'with valid params' do
it_behaves_like 'render views'
end end
end
context 'when auth app owner is a group' do
let(:group) { create(:group) }
context 'when auth app owner is a root group' do it 'authorizes the request and shows the user a page that redirects' do
let(:application) { create(:oauth_application, owner_id: group.id, owner_type: 'Namespace') } subject
it_behaves_like 'render views' expect(request.session['user_return_to']).to be_nil
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('doorkeeper/authorizations/redirect')
end end
end
context 'when auth app owner is a subgroup' do context 'without valid params' do
let(:subgroup) { create(:group, parent: group) } it 'returns 200 code and renders error view' do
let(:application) { create(:oauth_application, owner_id: subgroup.id, owner_type: 'Namespace') } get :new
it_behaves_like 'render views' expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('doorkeeper/authorizations/error')
end end
end end
context 'when there is no owner associated' do context 'with valid params' do
let(:application) { create(:oauth_application, owner_id: nil, owner_type: nil) } render_views
it 'renders view' do it 'returns 200 code and renders view' do
subject 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')
end end
end
end
context 'without valid params' do it 'deletes session.user_return_to and redirects when skip authorization' do
it 'returns 200 code and renders error view' do application.update!(trusted: true)
get :new request.session['user_return_to'] = 'http://example.com'
expect(response).to have_gitlab_http_status(:ok) subject
expect(response).to render_template('doorkeeper/authorizations/error')
end
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'
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(:ok)
expect(response).to render_template('doorkeeper/authorizations/redirect')
end
end
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