Merge branch 'oauth-client-consent' into 'master'

Require user consent on every OAuth public client authorization

See merge request !122819



Merged-by: Vitali Tatarintev <vtatarintev@gitlab.com>
Approved-by: Imre Farkas <ifarkas@gitlab.com>
Approved-by: Vitali Tatarintev <vtatarintev@gitlab.com>
Approved-by: Rohit Shambhuni <rshambhuni@gitlab.com>
Co-authored-by: M Hickford <mirth.hickford@gmail.com>

app/controllers/oauth/authorizations_controller.rb

@@ -14,7 +14,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
   # include the call to session.delete
   def new
     if pre_auth.authorizable?
-      if skip_authorization? || matching_token?
+      if skip_authorization? || (matching_token? && pre_auth.client.application.confidential?)
         auth = authorization.authorize
         parsed_redirect_uri = URI.parse(auth.redirect_uri)
         session.delete(:user_return_to)

spec/controllers/oauth/authorizations_controller_spec.rb

@@ -5,9 +5,15 @@
 RSpec.describe Oauth::AuthorizationsController do
   let(:user) { create(:user) }
   let(:application_scopes) { 'api read_user' }
+  let(:confidential) { true }
 
   let!(:application) do
-    create(:oauth_application, scopes: application_scopes, redirect_uri: 'http://example.com')
+    create(
+      :oauth_application,
+      scopes: application_scopes,
+      redirect_uri: 'http://example.com',
+      confidential: confidential
+    )
   end
 
   let(:params) do
@@ -68,12 +74,27 @@
           create(:oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes)
         end
 
-        it 'authorizes the request and shows the user a page that redirects' do
-          subject
+        context 'when application is confidential' do
+          let(:confidential) { true }
 
-          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')
+          it 'authorizes the request and shows the user a page that redirects' do
+            subject
+
+            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 application is not confidential' do
+          let(:confidential) { false }
+
+          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
 

spec/features/oauth_login_spec.rb

@@ -136,7 +136,7 @@ def login_with_provider(provider, enter_two_factor: false, additional_info: {})
     # record as the host / port depends on whether or not the spec uses
     # JS.
     let(:application) do
-      create(:oauth_application, scopes: 'api', redirect_uri: redirect_uri, confidential: false)
+      create(:oauth_application, scopes: 'api', redirect_uri: redirect_uri, confidential: true)
     end
 
     let(:params) do