Commit 4e6f445c authored by Douwe Maan's avatar Douwe Maan
Browse files

Merge branch '21554-mark-new-user-as-external' into 'master'

Login via OAuth marked as "external" should only mark new users as "external", not existing ones

Closes #21554

See merge request gitlab-org/gitlab-ce!16672
---
title: Login via OAuth now only marks new users as external
merge_request: 16672
author:
type: fixed
......@@ -55,7 +55,7 @@ def find_user
user ||= find_or_build_ldap_user if auto_link_ldap_user?
user ||= build_new_user if signup_enabled?
user.external = true if external_provider? && user
user.external = true if external_provider? && user&.new_record?
user
end
......
......@@ -44,6 +44,18 @@ def stub_ldap_config(messages)
let(:provider) { 'twitter' }
describe 'when account exists on server' do
it 'does not mark the user as external' do
create(:omniauth_user, extern_uid: 'my-uid', provider: provider)
stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider])
oauth_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_falsey
end
end
describe 'signup' do
context 'when signup is disabled' do
before do
......@@ -51,7 +63,7 @@ def stub_ldap_config(messages)
end
it 'creates the user' do
stub_omniauth_config(allow_single_sign_on: ['twitter'])
stub_omniauth_config(allow_single_sign_on: [provider])
oauth_user.save
......@@ -65,7 +77,7 @@ def stub_ldap_config(messages)
end
it 'creates and confirms the user anyway' do
stub_omniauth_config(allow_single_sign_on: ['twitter'])
stub_omniauth_config(allow_single_sign_on: [provider])
oauth_user.save
......@@ -75,7 +87,7 @@ def stub_ldap_config(messages)
end
it 'marks user as having password_automatically_set' do
stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter'])
stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider])
oauth_user.save
......@@ -86,7 +98,7 @@ def stub_ldap_config(messages)
shared_examples 'to verify compliance with allow_single_sign_on' do
context 'provider is marked as external' do
it 'marks user as external' do
stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter'])
stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider])
oauth_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_truthy
......@@ -95,8 +107,8 @@ def stub_ldap_config(messages)
context 'provider was external, now has been removed' do
it 'does not mark external user as internal' do
create(:omniauth_user, extern_uid: 'my-uid', provider: 'twitter', external: true)
stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['facebook'])
create(:omniauth_user, extern_uid: 'my-uid', provider: provider, external: true)
stub_omniauth_config(allow_single_sign_on: [provider], external_providers: ['facebook'])
oauth_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_truthy
......@@ -118,7 +130,7 @@ def stub_ldap_config(messages)
context 'with new allow_single_sign_on enabled syntax' do
before do
stub_omniauth_config(allow_single_sign_on: ['twitter'])
stub_omniauth_config(allow_single_sign_on: [provider])
end
it "creates a user from Omniauth" do
......@@ -127,7 +139,7 @@ def stub_ldap_config(messages)
expect(gl_user).to be_valid
identity = gl_user.identities.first
expect(identity.extern_uid).to eql uid
expect(identity.provider).to eql 'twitter'
expect(identity.provider).to eql provider
end
end
......@@ -142,7 +154,7 @@ def stub_ldap_config(messages)
expect(gl_user).to be_valid
identity = gl_user.identities.first
expect(identity.extern_uid).to eql uid
expect(identity.provider).to eql 'twitter'
expect(identity.provider).to eql provider
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