summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/identity.rb4
-rw-r--r--app/models/user.rb2
-rw-r--r--app/models/user_synced_attributes_metadata.rb10
-rw-r--r--changelogs/unreleased/dm-ldap-email-readonly.yml5
-rw-r--r--config/gitlab.yml.example1
-rw-r--r--doc/integration/omniauth.md12
-rw-r--r--lib/gitlab/ldap/user.rb4
-rw-r--r--lib/gitlab/o_auth/provider.rb12
-rw-r--r--lib/gitlab/o_auth/user.rb38
-rw-r--r--spec/lib/gitlab/ldap/user_spec.rb15
-rw-r--r--spec/lib/gitlab/o_auth/user_spec.rb50
11 files changed, 108 insertions, 45 deletions
diff --git a/app/models/identity.rb b/app/models/identity.rb
index ff811e19f8a..99d99bc6deb 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -14,11 +14,11 @@ class Identity < ActiveRecord::Base
end
def ldap?
- provider.starts_with?('ldap')
+ Gitlab::OAuth::Provider.ldap_provider?(provider)
end
def self.normalize_uid(provider, uid)
- if provider.to_s.starts_with?('ldap')
+ if Gitlab::OAuth::Provider.ldap_provider?(provider)
Gitlab::LDAP::Person.normalize_dn(uid)
else
uid.to_s
diff --git a/app/models/user.rb b/app/models/user.rb
index 92b461ce3ed..51941f43919 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -738,7 +738,7 @@ class User < ActiveRecord::Base
def ldap_user?
if identities.loaded?
- identities.find { |identity| identity.provider.start_with?('ldap') && !identity.extern_uid.nil? }
+ identities.find { |identity| Gitlab::OAuth::Provider.ldap_provider?(identity.provider) && !identity.extern_uid.nil? }
else
identities.exists?(["provider LIKE ? AND extern_uid IS NOT NULL", "ldap%"])
end
diff --git a/app/models/user_synced_attributes_metadata.rb b/app/models/user_synced_attributes_metadata.rb
index 9f374304164..548b99b69d9 100644
--- a/app/models/user_synced_attributes_metadata.rb
+++ b/app/models/user_synced_attributes_metadata.rb
@@ -6,11 +6,11 @@ class UserSyncedAttributesMetadata < ActiveRecord::Base
SYNCABLE_ATTRIBUTES = %i[name email location].freeze
def read_only?(attribute)
- Gitlab.config.omniauth.sync_profile_from_provider && synced?(attribute)
+ sync_profile_from_provider? && synced?(attribute)
end
def read_only_attributes
- return [] unless Gitlab.config.omniauth.sync_profile_from_provider
+ return [] unless sync_profile_from_provider?
SYNCABLE_ATTRIBUTES.select { |key| synced?(key) }
end
@@ -22,4 +22,10 @@ class UserSyncedAttributesMetadata < ActiveRecord::Base
def set_attribute_synced(attribute, value)
write_attribute("#{attribute}_synced", value)
end
+
+ private
+
+ def sync_profile_from_provider?
+ Gitlab::OAuth::Provider.sync_profile_from_provider?(provider)
+ end
end
diff --git a/changelogs/unreleased/dm-ldap-email-readonly.yml b/changelogs/unreleased/dm-ldap-email-readonly.yml
new file mode 100644
index 00000000000..744b21f901e
--- /dev/null
+++ b/changelogs/unreleased/dm-ldap-email-readonly.yml
@@ -0,0 +1,5 @@
+---
+title: Make sure user email is read only when synced with LDAP
+merge_request: 15915
+author:
+type: fixed
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index c8b6018bc1b..f2f05b3eeb2 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -383,6 +383,7 @@ production: &base
# Sync user's profile from the specified Omniauth providers every time the user logs in (default: empty).
# Define the allowed providers using an array, e.g. ["cas3", "saml", "twitter"],
# or as true/false to allow all providers or none.
+ # When authenticating using LDAP, the user's email is always synced.
# sync_profile_from_provider: []
# Select which info to sync from the providers above. (default: email).
diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md
index 0e20b8096e9..20087a981f9 100644
--- a/doc/integration/omniauth.md
+++ b/doc/integration/omniauth.md
@@ -229,16 +229,18 @@ In order to enable/disable an OmniAuth provider, go to Admin Area -> Settings ->
## Keep OmniAuth user profiles up to date
You can enable profile syncing from selected OmniAuth providers and for all or for specific user information.
-
+
+When authenticating using LDAP, the user's email is always synced.
+
```ruby
gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2']
gitlab_rails['sync_profile_attributes'] = ['name', 'email', 'location']
```
-
+
**For installations from source**
-
+
```yaml
omniauth:
sync_profile_from_provider: ['twitter', 'google_oauth2']
- sync_profile_claims_from_provider: ['email', 'location']
- ``` \ No newline at end of file
+ sync_profile_attributes: ['email', 'location']
+ ```
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index 3945df27eed..84ee94e38e4 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -36,10 +36,6 @@ module Gitlab
ldap_config.block_auto_created_users
end
- def sync_profile_from_provider?
- true
- end
-
def allowed?
Gitlab::LDAP::Access.allowed?(gl_user)
end
diff --git a/lib/gitlab/o_auth/provider.rb b/lib/gitlab/o_auth/provider.rb
index ac9d66c836d..657db29c85a 100644
--- a/lib/gitlab/o_auth/provider.rb
+++ b/lib/gitlab/o_auth/provider.rb
@@ -19,6 +19,18 @@ module Gitlab
name.to_s.start_with?('ldap')
end
+ def self.sync_profile_from_provider?(provider)
+ return true if ldap_provider?(provider)
+
+ providers = Gitlab.config.omniauth.sync_profile_from_provider
+
+ if providers.is_a?(Array)
+ providers.include?(provider)
+ else
+ providers
+ end
+ end
+
def self.config_for(name)
name = name.to_s
if ldap_provider?(name)
diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb
index 552133234a3..d33f33d192f 100644
--- a/lib/gitlab/o_auth/user.rb
+++ b/lib/gitlab/o_auth/user.rb
@@ -12,7 +12,7 @@ module Gitlab
def initialize(auth_hash)
self.auth_hash = auth_hash
- update_profile if sync_profile_from_provider?
+ update_profile
add_or_update_user_identities
end
@@ -195,29 +195,31 @@ module Gitlab
end
def sync_profile_from_provider?
- providers = Gitlab.config.omniauth.sync_profile_from_provider
-
- if providers.is_a?(Array)
- providers.include?(auth_hash.provider)
- else
- providers
- end
+ Gitlab::OAuth::Provider.sync_profile_from_provider?(auth_hash.provider)
end
def update_profile
- user_synced_attributes_metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata
-
- UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key|
- if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key)
- gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend
- user_synced_attributes_metadata.set_attribute_synced(key, true)
- else
- user_synced_attributes_metadata.set_attribute_synced(key, false)
+ return unless sync_profile_from_provider? || creating_linked_ldap_user?
+
+ metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata
+
+ if sync_profile_from_provider?
+ UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key|
+ if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key)
+ gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend
+ metadata.set_attribute_synced(key, true)
+ else
+ metadata.set_attribute_synced(key, false)
+ end
end
+
+ metadata.provider = auth_hash.provider
end
- user_synced_attributes_metadata.provider = auth_hash.provider
- gl_user.user_synced_attributes_metadata = user_synced_attributes_metadata
+ if creating_linked_ldap_user? && gl_user.email == ldap_person.email.first
+ metadata.set_attribute_synced(:email, true)
+ metadata.provider = ldap_person.provider
+ end
end
def log
diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb
index 260df6e4dae..048caa38fcf 100644
--- a/spec/lib/gitlab/ldap/user_spec.rb
+++ b/spec/lib/gitlab/ldap/user_spec.rb
@@ -38,7 +38,6 @@ describe Gitlab::LDAP::User do
it "does not mark existing ldap user as changed" do
create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain')
- ldap_user.gl_user.user_synced_attributes_metadata(provider: 'ldapmain', email: true)
expect(ldap_user.changed?).to be_falsey
end
end
@@ -144,11 +143,15 @@ describe Gitlab::LDAP::User do
expect(ldap_user.gl_user.email).to eq(info[:email])
end
- it "has user_synced_attributes_metadata email set to true" do
+ it "has email set as synced" do
expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_truthy
end
- it "has synced_attribute_provider set to ldapmain" do
+ it "has email set as read-only" do
+ expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_truthy
+ end
+
+ it "has synced attributes provider set to ldapmain" do
expect(ldap_user.gl_user.user_synced_attributes_metadata.provider).to eql 'ldapmain'
end
end
@@ -162,9 +165,13 @@ describe Gitlab::LDAP::User do
expect(ldap_user.gl_user.temp_oauth_email?).to be_truthy
end
- it "has synced attribute email set to false" do
+ it "has email set as not synced" do
expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_falsey
end
+
+ it "does not have email set as read-only" do
+ expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_falsey
+ end
end
end
diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb
index 2f19fb7312d..6334bcd0156 100644
--- a/spec/lib/gitlab/o_auth/user_spec.rb
+++ b/spec/lib/gitlab/o_auth/user_spec.rb
@@ -202,11 +202,13 @@ describe Gitlab::OAuth::User do
end
context "and no account for the LDAP user" do
- it "creates a user with dual LDAP and omniauth identities" do
+ before do
allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user)
oauth_user.save
+ end
+ it "creates a user with dual LDAP and omniauth identities" do
expect(gl_user).to be_valid
expect(gl_user.username).to eql uid
expect(gl_user.email).to eql 'johndoe@example.com'
@@ -219,6 +221,18 @@ describe Gitlab::OAuth::User do
]
)
end
+
+ it "has email set as synced" do
+ expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy
+ end
+
+ it "has email set as read-only" do
+ expect(gl_user.read_only_attribute?(:email)).to be_truthy
+ end
+
+ it "has synced attributes provider set to ldapmain" do
+ expect(gl_user.user_synced_attributes_metadata.provider).to eql 'ldapmain'
+ end
end
context "and LDAP user has an account already" do
@@ -440,11 +454,15 @@ describe Gitlab::OAuth::User do
expect(gl_user.email).to eq(info_hash[:email])
end
- it "has external_attributes set to true" do
- expect(gl_user.user_synced_attributes_metadata).not_to be_nil
+ it "has email set as synced" do
+ expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy
+ end
+
+ it "has email set as read-only" do
+ expect(gl_user.read_only_attribute?(:email)).to be_truthy
end
- it "has attributes_provider set to my-provider" do
+ it "has synced attributes provider set to my-provider" do
expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider'
end
end
@@ -458,10 +476,13 @@ describe Gitlab::OAuth::User do
expect(gl_user.email).not_to eq(info_hash[:email])
end
- it "has user_synced_attributes_metadata set to nil" do
- expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider'
+ it "has email set as not synced" do
expect(gl_user.user_synced_attributes_metadata.email_synced).to be_falsey
end
+
+ it "does not have email set as read-only" do
+ expect(gl_user.read_only_attribute?(:email)).to be_falsey
+ end
end
end
@@ -508,11 +529,15 @@ describe Gitlab::OAuth::User do
expect(gl_user.email).to eq(info_hash[:email])
end
- it "has email_synced_attribute set to true" do
+ it "has email set as synced" do
expect(gl_user.user_synced_attributes_metadata.email_synced).to be(true)
end
- it "has my-provider as attributes_provider" do
+ it "has email set as read-only" do
+ expect(gl_user.read_only_attribute?(:email)).to be_truthy
+ end
+
+ it "has synced attributes provider set to my-provider" do
expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider'
end
end
@@ -524,7 +549,14 @@ describe Gitlab::OAuth::User do
it "does not update the user email" do
expect(gl_user.email).not_to eq(info_hash[:email])
- expect(gl_user.user_synced_attributes_metadata.email_synced).to be(false)
+ end
+
+ it "has email set as not synced" do
+ expect(gl_user.user_synced_attributes_metadata.email_synced).to be_falsey
+ end
+
+ it "does not have email set as read-only" do
+ expect(gl_user.read_only_attribute?(:email)).to be_falsey
end
end
end
View Lorry Controller Status