diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-06-30 15:42:15 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-06-30 14:38:49 -0400 |
commit | 142324ac131c1d23ccb7dfa2c6471f182b8bb514 (patch) | |
tree | 25bae456b4c565f3b5086b03c40782f091060c85 | |
parent | e754c90b4bc10325d5d6373aa3e0199e2c4ee2dc (diff) | |
download | gitlab-ce-142324ac131c1d23ccb7dfa2c6471f182b8bb514.tar.gz |
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 5e6342b7ac08b4b37b233cad54f4aeaf0144b977)
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | doc/integration/omniauth.md | 12 | ||||
-rw-r--r-- | lib/gitlab/o_auth/user.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/o_auth/user_spec.rb | 17 |
4 files changed, 27 insertions, 7 deletions
diff --git a/CHANGELOG b/CHANGELOG index 37694a41570..bc9ac9218c5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,8 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.7.9 + - Fix privilege escalation issue with OAuth external users. + v 8.7.8 - Fix visibility of snippets when searching. - Update omniauth-saml to 1.6.0. !4951 diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index cab329c0dec..931405f6821 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -126,9 +126,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** diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 356e96fcbab..a4b66f7f74f 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -56,8 +56,6 @@ module Gitlab if external_provider? && @user @user.external = true - elsif @user - @user.external = false end @user diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 6727a83e58a..fbb5895c2ef 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -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 |