Commit 96b6fb1a authored by Douwe Maan's avatar Douwe Maan Committed by Robert Speicher

Merge branch '19312-confidential-issue' into 'master'

Fix privilege escalation issue with OAuth external users

Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/19312

This MR fixes a privilege escalation issue, where manually set external users would be reverted back to internal users if they logged in via OAuth and that provider was not in the `external_providers` list.

/cc @douwe

See merge request !1975
(cherry picked from commit 5e6342b7)
parent 9c43e624
Please view this file on the master branch, on stable branches it's out of date.
v 8.8.7
- Fix privilege escalation issue with OAuth external users.
v 8.8.6
- Fix visibility of snippets when searching.
- Update omniauth-saml to 1.6.0 !4951
......
......@@ -127,9 +127,15 @@ The chosen OmniAuth provider is now active and can be used to sign in to GitLab
This setting was introduced with version 8.7 of GitLab
You can define which OmniAuth providers you want to be `external` so that all users
creating accounts via these providers will not be able to have access to internal
projects. You will need to use the full name of the provider, like `google_oauth2`
for Google. Refer to the examples for the full names of the supported providers.
**creating accounts, or logging in via these providers** will not be able to have
access to internal projects. You will need to use the full name of the provider,
like `google_oauth2` for Google. Refer to the examples for the full names of the
supported providers.
>**Note:**
If you decide to remove an OmniAuth provider from the external providers list
you will need to manually update the users that use this method to login, if you
want their accounts to be upgraded to full internal accounts.
**For Omnibus installations**
......
......@@ -56,8 +56,6 @@ module Gitlab
if external_provider? && @user
@user.external = true
elsif @user
@user.external = false
end
@user
......
......@@ -51,12 +51,25 @@ describe Gitlab::OAuth::User, lib: true do
end
context 'provider was external, now has been removed' do
it 'should mark existing user internal' do
it 'should 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'])
oauth_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_falsey
expect(gl_user.external).to be_truthy
end
end
context 'provider is not external' do
context 'when adding a new OAuth identity' do
it 'should not promote an external user to internal' do
user = create(:user, email: 'john@mail.com', external: true)
user.identities.create(provider: provider, extern_uid: uid)
oauth_user.save
expect(gl_user).to be_valid
expect(gl_user.external).to be_truthy
end
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