Sync email address from specified omniauth provider
This commit is contained in:
parent
8fdba5fac2
commit
469acd190e
|
@ -9,7 +9,7 @@ class ProfilesController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def update
|
||||
user_params.except!(:email) if @user.ldap_user?
|
||||
user_params.except!(:email) if @user.external_email?
|
||||
|
||||
respond_to do |format|
|
||||
if @user.update_attributes(user_params)
|
||||
|
@ -76,7 +76,7 @@ class ProfilesController < Profiles::ApplicationController
|
|||
end
|
||||
|
||||
def user_params
|
||||
params.require(:user).permit(
|
||||
@user_params ||= params.require(:user).permit(
|
||||
:avatar,
|
||||
:bio,
|
||||
:email,
|
||||
|
|
|
@ -0,0 +1,7 @@
|
|||
module ProfilesHelper
|
||||
def email_provider_label
|
||||
return unless current_user.external_email?
|
||||
|
||||
current_user.email_provider.present? ? Gitlab::OAuth::Provider.label_for(current_user.email_provider) : "LDAP"
|
||||
end
|
||||
end
|
|
@ -49,10 +49,10 @@
|
|||
|
||||
.form-group
|
||||
= f.label :email, class: "label-light"
|
||||
- if @user.ldap_user? && @user.ldap_email?
|
||||
- if @user.external_email?
|
||||
= f.text_field :email, class: "form-control", required: true, readonly: true
|
||||
%span.help-block.light
|
||||
Your email address was automatically set based on the LDAP server.
|
||||
Your email address was automatically set based on your #{email_provider_label} account.
|
||||
- else
|
||||
- if @user.temp_oauth_email?
|
||||
= f.text_field :email, class: "form-control", required: true, value: nil
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
title: Sync email address from specified omniauth provider
|
||||
merge_request: 11268
|
||||
author: Robin Bobbitt
|
|
@ -337,6 +337,10 @@ production: &base
|
|||
# showing GitLab's sign-in page (default: show the GitLab sign-in page)
|
||||
# auto_sign_in_with_provider: saml
|
||||
|
||||
# Sync user's email address from the specified Omniauth provider every time the user logs
|
||||
# in (default: nil). And consequently make this field read-only.
|
||||
# sync_email_from_provider: cas3
|
||||
|
||||
# CAUTION!
|
||||
# This allows users to login without having a user account first. Define the allowed providers
|
||||
# using an array, e.g. ["saml", "twitter"], or as true/false to allow all providers or none.
|
||||
|
|
|
@ -156,6 +156,7 @@ Settings.omniauth['external_providers'] = [] if Settings.omniauth['external_prov
|
|||
Settings.omniauth['block_auto_created_users'] = true if Settings.omniauth['block_auto_created_users'].nil?
|
||||
Settings.omniauth['auto_link_ldap_user'] = false if Settings.omniauth['auto_link_ldap_user'].nil?
|
||||
Settings.omniauth['auto_link_saml_user'] = false if Settings.omniauth['auto_link_saml_user'].nil?
|
||||
Settings.omniauth['sync_email_from_provider'] ||= nil
|
||||
|
||||
Settings.omniauth['providers'] ||= []
|
||||
Settings.omniauth['cas3'] ||= Settingslogic.new({})
|
||||
|
|
|
@ -41,11 +41,6 @@ module Gitlab
|
|||
|
||||
def update_user_attributes
|
||||
if persisted?
|
||||
if auth_hash.has_email?
|
||||
gl_user.skip_reconfirmation!
|
||||
gl_user.email = auth_hash.email
|
||||
end
|
||||
|
||||
# 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)
|
||||
|
@ -55,10 +50,6 @@ module Gitlab
|
|||
# For an existing identity with no change in DN, this line changes nothing.
|
||||
identity.extern_uid = auth_hash.uid
|
||||
end
|
||||
|
||||
gl_user.ldap_email = auth_hash.has_email?
|
||||
|
||||
gl_user
|
||||
end
|
||||
|
||||
def changed?
|
||||
|
@ -69,6 +60,10 @@ module Gitlab
|
|||
ldap_config.block_auto_created_users
|
||||
end
|
||||
|
||||
def sync_email_from_provider?
|
||||
true
|
||||
end
|
||||
|
||||
def allowed?
|
||||
Gitlab::LDAP::Access.allowed?(gl_user)
|
||||
end
|
||||
|
|
|
@ -12,6 +12,7 @@ module Gitlab
|
|||
|
||||
def initialize(auth_hash)
|
||||
self.auth_hash = auth_hash
|
||||
update_email
|
||||
end
|
||||
|
||||
def persisted?
|
||||
|
@ -174,6 +175,22 @@ module Gitlab
|
|||
}
|
||||
end
|
||||
|
||||
def sync_email_from_provider?
|
||||
auth_hash.provider.to_s == Gitlab.config.omniauth.sync_email_from_provider.to_s
|
||||
end
|
||||
|
||||
def update_email
|
||||
if auth_hash.has_email? && sync_email_from_provider?
|
||||
if persisted?
|
||||
gl_user.skip_reconfirmation!
|
||||
gl_user.email = auth_hash.email
|
||||
end
|
||||
|
||||
gl_user.external_email = true
|
||||
gl_user.email_provider = auth_hash.provider
|
||||
end
|
||||
end
|
||||
|
||||
def log
|
||||
Gitlab::AppLogger
|
||||
end
|
||||
|
|
|
@ -0,0 +1,31 @@
|
|||
require('spec_helper')
|
||||
|
||||
describe ProfilesController do
|
||||
describe "PUT update" do
|
||||
it "allows an email update from a user without an external email address" do
|
||||
user = create(:user)
|
||||
sign_in(user)
|
||||
|
||||
put :update,
|
||||
user: { email: "john@gmail.com", name: "John" }
|
||||
|
||||
user.reload
|
||||
|
||||
expect(response.status).to eq(302)
|
||||
expect(user.unconfirmed_email).to eq('john@gmail.com')
|
||||
end
|
||||
|
||||
it "ignores an email update from a user with an external email address" do
|
||||
ldap_user = create(:omniauth_user, external_email: true)
|
||||
sign_in(ldap_user)
|
||||
|
||||
put :update,
|
||||
user: { email: "john@gmail.com", name: "John" }
|
||||
|
||||
ldap_user.reload
|
||||
|
||||
expect(response.status).to eq(302)
|
||||
expect(ldap_user.unconfirmed_email).not_to eq('john@gmail.com')
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,36 @@
|
|||
require 'rails_helper'
|
||||
|
||||
describe ProfilesHelper do
|
||||
describe '#email_provider_label' do
|
||||
it "returns nil for users without external email" do
|
||||
user = create(:user)
|
||||
allow(helper).to receive(:current_user).and_return(user)
|
||||
|
||||
expect(helper.email_provider_label).to be_nil
|
||||
end
|
||||
|
||||
it "returns omniauth provider label for users with external email" do
|
||||
stub_cas_omniauth_provider
|
||||
cas_user = create(:omniauth_user, provider: 'cas3', external_email: true, email_provider: 'cas3')
|
||||
allow(helper).to receive(:current_user).and_return(cas_user)
|
||||
|
||||
expect(helper.email_provider_label).to eq('CAS')
|
||||
end
|
||||
|
||||
it "returns 'LDAP' for users with external email but no email provider" do
|
||||
ldap_user = create(:omniauth_user, external_email: true)
|
||||
allow(helper).to receive(:current_user).and_return(ldap_user)
|
||||
|
||||
expect(helper.email_provider_label).to eq('LDAP')
|
||||
end
|
||||
end
|
||||
|
||||
def stub_cas_omniauth_provider
|
||||
provider = OpenStruct.new(
|
||||
'name' => 'cas3',
|
||||
'label' => 'CAS'
|
||||
)
|
||||
|
||||
stub_omniauth_setting(providers: [provider])
|
||||
end
|
||||
end
|
|
@ -37,7 +37,7 @@ describe Gitlab::LDAP::User, lib: true do
|
|||
end
|
||||
|
||||
it "does not mark existing ldap user as changed" do
|
||||
create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain', ldap_email: true)
|
||||
create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain', external_email: true, email_provider: 'ldapmain')
|
||||
expect(ldap_user.changed?).to be_falsey
|
||||
end
|
||||
end
|
||||
|
@ -141,8 +141,12 @@ describe Gitlab::LDAP::User, lib: true 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)
|
||||
it "has external_email set to true" do
|
||||
expect(ldap_user.gl_user.external_email?).to be(true)
|
||||
end
|
||||
|
||||
it "has email_provider set to provider" do
|
||||
expect(ldap_user.gl_user.email_provider).to eql 'ldapmain'
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -155,8 +159,8 @@ describe Gitlab::LDAP::User, lib: true 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)
|
||||
it "has external_email set to false" do
|
||||
expect(ldap_user.gl_user.external_email?).to be(false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -28,11 +28,11 @@ describe Gitlab::OAuth::User, lib: true do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#save' do
|
||||
def stub_omniauth_config(messages)
|
||||
allow(Gitlab.config.omniauth).to receive_messages(messages)
|
||||
end
|
||||
|
||||
describe '#save' do
|
||||
def stub_ldap_config(messages)
|
||||
allow(Gitlab::LDAP::Config).to receive_messages(messages)
|
||||
end
|
||||
|
@ -377,4 +377,40 @@ describe Gitlab::OAuth::User, lib: true do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'updating email' do
|
||||
let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') }
|
||||
|
||||
before do
|
||||
stub_omniauth_config(sync_email_from_provider: 'my-provider')
|
||||
end
|
||||
|
||||
context "when provider sets an email" do
|
||||
it "updates the user email" do
|
||||
expect(gl_user.email).to eq(info_hash[:email])
|
||||
end
|
||||
|
||||
it "has external_email set to true" do
|
||||
expect(gl_user.external_email?).to be(true)
|
||||
end
|
||||
|
||||
it "has email_provider set to provider" do
|
||||
expect(gl_user.email_provider).to eql 'my-provider'
|
||||
end
|
||||
end
|
||||
|
||||
context "when provider doesn't set an email" do
|
||||
before do
|
||||
info_hash.delete(:email)
|
||||
end
|
||||
|
||||
it "does not update the user email" do
|
||||
expect(gl_user.email).not_to eq(info_hash[:email])
|
||||
end
|
||||
|
||||
it "has external_email set to false" do
|
||||
expect(gl_user.external_email?).to be(false)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue