Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee

4 files changed, 42 insertions, 48 deletions

diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb
index 857f36e3833..ddf70c1892a 100644
--- a/app/controllers/oauth/authorizations_controller.rb
+++ b/app/controllers/oauth/authorizations_controller.rb
@@ -14,8 +14,9 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
     if pre_auth.authorizable?
       if skip_authorization? || matching_token?
         auth = authorization.authorize
+        parsed_redirect_uri = URI.parse(auth.redirect_uri)
         session.delete(:user_return_to)
-        redirect_to auth.redirect_uri
+        render "doorkeeper/authorizations/redirect", locals: { redirect_uri: parsed_redirect_uri }, layout: false
       else
         render "doorkeeper/authorizations/new"
       end
diff --git a/app/views/doorkeeper/authorizations/redirect.html.haml b/app/views/doorkeeper/authorizations/redirect.html.haml
new file mode 100644
index 00000000000..2fefbac3802
--- /dev/null
+++ b/app/views/doorkeeper/authorizations/redirect.html.haml
@@ -0,0 +1,7 @@
+%h3.page-title= _("Redirecting")
+
+%div
+  %a{ :href => redirect_uri } Click here to redirect to #{redirect_uri}
+
+:javascript
+  window.location= "#{redirect_uri}";
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index af1d2ea1221..49a36e7d5e7 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -26967,6 +26967,9 @@ msgstr ""
 msgid "Redirect to SAML provider to test configuration"
 msgstr ""
 
+msgid "Redirecting"
+msgstr ""
+
 msgid "Redis"
 msgstr ""
 
diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb
index 5fc5cdfc9b9..0e25f6a96d7 100644
--- a/spec/controllers/oauth/authorizations_controller_spec.rb
+++ b/spec/controllers/oauth/authorizations_controller_spec.rb
@@ -70,76 +70,59 @@ RSpec.describe Oauth::AuthorizationsController do
   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 'rendering of views based on the ownership of the application' do
-      shared_examples 'render views' do
-        render_views
-
-        it 'returns 200 and renders view with correct info', :aggregate_failures do
-          subject
+    context 'when the user is confirmed' do
+      let(:confirmed_at) { 1.hour.ago }
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response.body).to include(application.owner.name)
-          expect(response).to render_template('doorkeeper/authorizations/new')
-        end
-      end
+      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')
 
-      subject { get :new, params: params }
+          allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes)
 
-      context 'when auth app owner is a user' do
-        context 'with valid params' do
-          it_behaves_like 'render views'
+          create(:oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes)
         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
-          let(:application) { create(:oauth_application, owner_id: group.id, owner_type: 'Namespace') }
+        it 'authorizes the request and shows the user a page that redirects' do
+          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
 
-        context 'when auth app owner is a subgroup' do
-          let(:subgroup) { create(:group, parent: group) }
-          let(:application) { create(:oauth_application, owner_id: subgroup.id, owner_type: 'Namespace') }
+      context 'without valid params' do
+        it 'returns 200 code and renders error view' do
+          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
 
-      context 'when there is no owner associated' do
-        let(:application) { create(:oauth_application, owner_id: nil, owner_type: nil) }
+      context 'with valid params' do
+        render_views
 
-        it 'renders view' do
+        it 'returns 200 code and renders view' do
           subject
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to render_template('doorkeeper/authorizations/new')
         end
-      end
-    end
 
-    context 'without valid params' do
-      it 'returns 200 code and renders error view' do
-        get :new
+        it 'deletes session.user_return_to and redirects when skip authorization' do
+          application.update!(trusted: true)
+          request.session['user_return_to'] = 'http://example.com'
 
-        expect(response).to have_gitlab_http_status(:ok)
-        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
+          subject
 
-      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(:ok)
+          expect(response).to render_template('doorkeeper/authorizations/redirect')
+        end
+      end
     end
   end