Merge branch 'fix/auth0-unsafe-login-10-6' into 'security-10-6'

[10.6] Fix GitLab Auth0 integration signs in the wrong user

See merge request gitlab/gitlabhq!2354

2 files changed, 82 insertions, 43 deletions

diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb
index c639ad32ec6..9fd129e4ee9 100644
--- a/spec/controllers/omniauth_callbacks_controller_spec.rb
+++ b/spec/controllers/omniauth_callbacks_controller_spec.rb
@@ -3,73 +3,90 @@ require 'spec_helper'
 describe OmniauthCallbacksController do
   include LoginHelpers
 
-  let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: provider) }
-  let(:provider) { :github }
+  let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) }
 
   before do
-    mock_auth_hash(provider.to_s, 'my-uid', user.email)
+    mock_auth_hash(provider.to_s, extern_uid, user.email)
     stub_omniauth_provider(provider, context: request)
   end
 
-  it 'allows sign in' do
-    post provider
+  context 'github' do
+    let(:extern_uid) { 'my-uid' }
+    let(:provider) { :github }
 
-    expect(request.env['warden']).to be_authenticated
-  end
+    it 'allows sign in' do
+      post provider
+
+      expect(request.env['warden']).to be_authenticated
+    end
 
-  shared_context 'sign_up' do
-    let(:user) { double(email: 'new@example.com') }
+    shared_context 'sign_up' do
+      let(:user) { double(email: 'new@example.com') }
 
-    before do
-      stub_omniauth_setting(block_auto_created_users: false)
+      before do
+        stub_omniauth_setting(block_auto_created_users: false)
+      end
     end
-  end
 
-  context 'sign up' do
-    include_context 'sign_up'
+    context 'sign up' do
+      include_context 'sign_up'
 
-    it 'is allowed' do
-      post provider
+      it 'is allowed' do
+        post provider
 
-      expect(request.env['warden']).to be_authenticated
+        expect(request.env['warden']).to be_authenticated
+      end
     end
-  end
 
-  context 'when OAuth is disabled' do
-    before do
-      stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
-      settings = Gitlab::CurrentSettings.current_application_settings
-      settings.update(disabled_oauth_sign_in_sources: [provider.to_s])
-    end
+    context 'when OAuth is disabled' do
+      before do
+        stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+        settings = Gitlab::CurrentSettings.current_application_settings
+        settings.update(disabled_oauth_sign_in_sources: [provider.to_s])
+      end
 
-    it 'prevents login via POST' do
-      post provider
+      it 'prevents login via POST' do
+        post provider
 
-      expect(request.env['warden']).not_to be_authenticated
-    end
+        expect(request.env['warden']).not_to be_authenticated
+      end
 
-    it 'shows warning when attempting login' do
-      post provider
+      it 'shows warning when attempting login' do
+        post provider
 
-      expect(response).to redirect_to new_user_session_path
-      expect(flash[:alert]).to eq('Signing in using GitHub has been disabled')
-    end
+        expect(response).to redirect_to new_user_session_path
+        expect(flash[:alert]).to eq('Signing in using GitHub has been disabled')
+      end
 
-    it 'allows linking the disabled provider' do
-      user.identities.destroy_all
-      sign_in(user)
+      it 'allows linking the disabled provider' do
+        user.identities.destroy_all
+        sign_in(user)
 
-      expect { post provider }.to change { user.reload.identities.count }.by(1)
-    end
+        expect { post provider }.to change { user.reload.identities.count }.by(1)
+      end
 
-    context 'sign up' do
-      include_context 'sign_up'
+      context 'sign up' do
+        include_context 'sign_up'
 
-      it 'is prevented' do
-        post provider
+        it 'is prevented' do
+          post provider
 
-        expect(request.env['warden']).not_to be_authenticated
+          expect(request.env['warden']).not_to be_authenticated
+        end
       end
     end
   end
+
+  context 'auth0' do
+    let(:extern_uid) { '' }
+    let(:provider) { :auth0 }
+
+    it 'does not allow sign in without extern_uid' do
+      post 'auth0'
+
+      expect(request.env['warden']).not_to be_authenticated
+      expect(response.status).to eq(302)
+      expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.')
+    end
+  end
 end
diff --git a/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb b/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb
new file mode 100644
index 00000000000..441c4295a40
--- /dev/null
+++ b/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180220150310_remove_empty_extern_uid_auth0_identities.rb')
+
+describe RemoveEmptyExternUidAuth0Identities, :migration do
+  let(:identities) { table(:identities) }
+
+  before do
+    identities.create(provider: 'auth0', extern_uid: '')
+    identities.create(provider: 'auth0', extern_uid: 'valid')
+    identities.create(provider: 'github', extern_uid: '')
+
+    migrate!
+  end
+
+  it 'leaves the correct auth0 identity' do
+    expect(identities.where(provider: 'auth0').pluck(:extern_uid)).to eq(['valid'])
+  end
+
+  it 'leaves the correct github identity' do
+    expect(identities.where(provider: 'github').count).to eq(1)
+  end
+end