GitLab steht aufgrund wichtiger Wartungsarbeiten am Montag, den 8. März, zwischen 17:00 und 19:00 Uhr nicht zur Verfügung.

Commit 98e1a5b6 authored by Douwe Maan's avatar Douwe Maan

Allow LDAP users to change their email if it was not set by the LDAP server

parent 425f8d6f
......@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.5.0 (unreleased)
v 8.4.0 (unreleased)
- Allow LDAP users to change their email if it was not set by the LDAP server
- Ensure Gravatar host looks like an actual host
- Consider re-assign as a mention from a notification point of view
- Add pagination headers to already paginated API resources
......
......@@ -664,7 +664,10 @@ def avatar_url(size = nil, scale = 2)
end
def all_emails
[self.email, *self.emails.map(&:email)]
all_emails = []
all_emails << self.email unless self.temp_oauth_email?
all_emails.concat(self.emails.map(&:email))
all_emails
end
def hook_attrs
......
......@@ -21,10 +21,10 @@
.form-group
= f.label :email, class: "control-label"
.col-sm-10
- if @user.ldap_user?
- if @user.ldap_user? && @user.ldap_email?
= f.text_field :email, class: "form-control", required: true, readonly: true
%span.help-block.light
Email is read-only for LDAP user
Your email address was automatically set based on the LDAP server.
- else
- if @user.temp_oauth_email?
= f.text_field :email, class: "form-control", required: true, value: nil
......
class AddLdapEmailToUsers < ActiveRecord::Migration
def up
add_column :users, :ldap_email, :boolean, default: false, null: false
if Gitlab::Database.mysql?
execute %{
UPDATE users, identities
SET users.ldap_email = TRUE
WHERE identities.user_id = users.id
AND users.email LIKE 'temp-email-for-oauth%'
AND identities.provider LIKE 'ldap%'
AND identities.extern_uid IS NOT NULL
}
else
execute %{
UPDATE users
SET ldap_email = TRUE
FROM identities
WHERE identities.user_id = users.id
AND users.email LIKE 'temp-email-for-oauth%'
AND identities.provider LIKE 'ldap%'
AND identities.extern_uid IS NOT NULL
}
end
end
def down
remove_column :users, :ldap_email
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20160113111034) do
ActiveRecord::Schema.define(version: 20160119145451) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -850,6 +850,7 @@
t.boolean "hide_project_limit", default: false
t.string "unlock_token"
t.datetime "otp_grace_period_started_at"
t.boolean "ldap_email", default: false, null: false
end
add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
......
......@@ -30,28 +30,31 @@ def gl_user
end
def find_by_uid_and_provider
self.class.find_by_uid_and_provider(
auth_hash.uid, auth_hash.provider)
self.class.find_by_uid_and_provider(auth_hash.uid, auth_hash.provider)
end
def find_by_email
::User.find_by(email: auth_hash.email.downcase)
::User.find_by(email: auth_hash.email.downcase) if auth_hash.has_email?
end
def update_user_attributes
return unless persisted?
if persisted?
if auth_hash.has_email?
gl_user.skip_reconfirmation!
gl_user.email = auth_hash.email
end
gl_user.skip_reconfirmation!
gl_user.email = auth_hash.email
# find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved.
identity = gl_user.identities.find { |identity| identity.provider == auth_hash.provider }
identity ||= gl_user.identities.build(provider: auth_hash.provider)
# find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved.
identity = gl_user.identities.find { |identity| identity.provider == auth_hash.provider }
identity ||= gl_user.identities.build(provider: auth_hash.provider)
# For a new identity set extern_uid to the LDAP DN
# For an existing identity with matching email but changed DN, update the DN.
# For an existing identity with no change in DN, this line changes nothing.
identity.extern_uid = auth_hash.uid
end
# For a new user set extern_uid to the LDAP DN
# For an existing user with matching email but changed DN, update the DN.
# For an existing user with no change in DN, this line changes nothing.
identity.extern_uid = auth_hash.uid
gl_user.ldap_email = auth_hash.has_email?
gl_user
end
......
......@@ -32,6 +32,10 @@ def password
@password ||= Gitlab::Utils.force_utf8(Devise.friendly_token[0, 8].downcase)
end
def has_email?
get_info(:email).present?
end
private
def info
......@@ -46,8 +50,8 @@ def get_info(key)
def username_and_email
@username_and_email ||= begin
username = get_info(:username) || get_info(:nickname)
email = get_info(:email)
username = get_info(:username).presence || get_info(:nickname).presence
email = get_info(:email).presence
username ||= generate_username(email) if email
email ||= generate_temporarily_email(username) if username
......
......@@ -111,7 +111,7 @@ def signup_enabled?
def block_after_signup?
if creating_linked_ldap_user?
ldap_config.block_auto_created_users
else
else
Gitlab.config.omniauth.block_auto_created_users
end
end
......@@ -135,16 +135,16 @@ def build_new_user
def user_attributes
# Give preference to LDAP for sensitive information when creating a linked account
if creating_linked_ldap_user?
username = ldap_person.username
email = ldap_person.email.first
else
username = auth_hash.username
email = auth_hash.email
username = ldap_person.username.presence
email = ldap_person.email.first.presence
end
username ||= auth_hash.username
email ||= auth_hash.email
name = auth_hash.name
name = ::Namespace.clean_path(username) if name.strip.empty?
{
name: name,
username: ::Namespace.clean_path(username),
......
......@@ -37,7 +37,7 @@
end
it "dont marks existing ldap user as changed" do
create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain')
create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain', ldap_email: true)
expect(ldap_user.changed?).to be_falsey
end
end
......@@ -110,6 +110,32 @@
end
end
describe 'updating email' do
context "when LDAP sets an email" do
it "has a real email" do
expect(ldap_user.gl_user.email).to eq(info[:email])
end
it "has ldap_email set to true" do
expect(ldap_user.gl_user.ldap_email?).to be(true)
end
end
context "when LDAP doesn't set an email" do
before do
info.delete(:email)
end
it "has a temp email" do
expect(ldap_user.gl_user.temp_oauth_email?).to be(true)
end
it "has ldap_email set to false" do
expect(ldap_user.gl_user.ldap_email?).to be(false)
end
end
end
describe 'blocking' do
def configure_block(value)
allow_any_instance_of(Gitlab::LDAP::Config).
......
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