Merge branch 'security-implicit-confidential' into 'master'

Deny implicit flow for confidential apps

See merge request gitlab-org/security/gitlab!1118

app/controllers/oauth/authorizations_controller.rb

@@ -24,6 +24,17 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
     end
   end
 
+  def create
+    # Confidential apps require the client_secret to be sent with the request.
+    # Doorkeeper allows implicit grant flow requests (response_type=token) to
+    # work without client_secret regardless of the confidential setting.
+    if pre_auth.authorizable? && pre_auth.response_type == 'token' && pre_auth.client.application.confidential
+      render "doorkeeper/authorizations/error"
+    else
+      super
+    end
+  end
+
   private
 
   def verify_confirmed_email!

changelogs/unreleased/security-implicit-confidential.yml

+---
+title: Deny implicit flow for confidential apps
+merge_request:
+author:
+type: security

spec/controllers/oauth/authorizations_controller_spec.rb

@@ -95,6 +95,20 @@ RSpec.describe Oauth::AuthorizationsController do
     subject { post :create, params: params }
 
     include_examples 'OAuth Authorizations require confirmed user'
+
+    context 'when application is confidential' do
+      before do
+        application.update(confidential: true)
+        params[:response_type] = 'token'
+      end
+
+      it 'does not allow the implicit flow' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template('doorkeeper/authorizations/error')
+      end
+    end
   end
 
   describe 'DELETE #destroy' do