From 26482c70fbb3215cf1367d8506396cfaba61ed54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 26 Feb 2019 10:57:53 +0100 Subject: [PATCH 01/19] Add separate partial for email settings --- app/views/profiles/_email_settings.html.haml | 10 ++++++++++ app/views/profiles/show.html.haml | 13 +------------ ee/app/views/profiles/_email_settings.html.haml | 12 ++++++++++++ 3 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 app/views/profiles/_email_settings.html.haml create mode 100644 ee/app/views/profiles/_email_settings.html.haml diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml new file mode 100644 index 00000000000000..81bb47fcd529f9 --- /dev/null +++ b/app/views/profiles/_email_settings.html.haml @@ -0,0 +1,10 @@ +- readonly = @user.read_only_attribute?(:email) +- help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) += form.text_field :email, required: true, class: 'input-lg', readonly: readonly, value: (@user.email unless @user.temp_oauth_email?), help: help_text += form.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), + { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, + control_class: 'select2 input-lg' +- commit_email_docs_link = link_to s_('Profiles|Learn more'), help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') += form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), + { help: s_("Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}").html_safe % { learn_more: commit_email_docs_link } }, + control_class: 'select2 input-lg' diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 4d3d92d09c05a8..1fffea08dae6f6 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -83,18 +83,7 @@ = f.text_field :name, label: 'Full name', required: true, title: s_("Profiles|Using emojis in names seems fun, but please try to set a status message instead"), wrapper: { class: 'col-md-9 qa-full-name' }, help: s_("Profiles|Enter your name, so people you know can recognize you") = f.text_field :id, readonly: true, label: 'User ID', wrapper: { class: 'col-md-3' } - - if @user.read_only_attribute?(:email) - = f.text_field :email, required: true, class: 'input-lg', readonly: true, help: s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } - - else - = f.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), - help: user_email_help_text(@user) - = f.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), - { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, - control_class: 'select2 input-lg' - - commit_email_docs_link = link_to s_('Profiles|Learn more'), help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') - = f.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), - { help: s_("Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}").html_safe % { learn_more: commit_email_docs_link } }, - control_class: 'select2 input-lg' + = render_if_exists 'profiles/email_settings', form: f = f.text_field :skype, class: 'input-md', placeholder: s_("Profiles|username") = f.text_field :linkedin, class: 'input-md', help: s_("Profiles|Your LinkedIn profile name from linkedin.com/in/profilename") = f.text_field :twitter, class: 'input-md', placeholder: s_("Profiles|@username") diff --git a/ee/app/views/profiles/_email_settings.html.haml b/ee/app/views/profiles/_email_settings.html.haml new file mode 100644 index 00000000000000..b53562d2222bda --- /dev/null +++ b/ee/app/views/profiles/_email_settings.html.haml @@ -0,0 +1,12 @@ +- readonly = @user.read_only_attribute?(:email) +- help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) +- group_managed_account = true #@user.group_managed_account? + += form.text_field :email, required: true, class: 'input-lg', readonly: readonly || group_managed_account, value: (@user.email unless @user.temp_oauth_email?), help: help_text += form.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), + { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, + control_class: 'select2 input-lg', disabled: group_managed_account +- commit_email_docs_link = link_to s_('Profiles|Learn more'), help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') += form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), + { help: s_("Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}").html_safe % { learn_more: commit_email_docs_link } }, + control_class: 'select2 input-lg', disabled: group_managed_account -- GitLab From d190b10983974679eafa6d4e711375dbe5eddd71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 26 Feb 2019 14:30:48 +0100 Subject: [PATCH 02/19] Update service to respect group managed rules --- ee/app/models/saml_provider.rb | 2 ++ ee/app/services/ee/users/update_service.rb | 5 +++ ee/spec/services/users/update_service_spec.rb | 34 +++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 ee/spec/services/users/update_service_spec.rb diff --git a/ee/app/models/saml_provider.rb b/ee/app/models/saml_provider.rb index ca332995b18f43..abcf4977e47c4d 100644 --- a/ee/app/models/saml_provider.rb +++ b/ee/app/models/saml_provider.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class SamlProvider < ActiveRecord::Base + BLOCKED_USER_ATTRIBUTES = [:email, :public_email, :commit_email].freeze + belongs_to :group has_many :identities diff --git a/ee/app/services/ee/users/update_service.rb b/ee/app/services/ee/users/update_service.rb index 1640e8ea41fd06..0b71ee4e46dfdd 100644 --- a/ee/app/services/ee/users/update_service.rb +++ b/ee/app/services/ee/users/update_service.rb @@ -19,6 +19,11 @@ def notify_success(user_exists) def model @user end + + def assign_attributes + params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if @user.group_managed_account? + super + end end end end diff --git a/ee/spec/services/users/update_service_spec.rb b/ee/spec/services/users/update_service_spec.rb new file mode 100644 index 00000000000000..e66a46f4c472f8 --- /dev/null +++ b/ee/spec/services/users/update_service_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe Users::UpdateService do + let(:user) { create(:user) } + + describe '#execute' do + it 'does not update email if an user has group managed account' do + allow(user).to receive(:group_managed_account?).and_return(true) + expect do + update_user(user, { email: 'foreign@email' }) + end.not_to change { user.reload.email } + end + + it 'does not update commit email if an user has group managed account' do + allow(user).to receive(:group_managed_account?).and_return(true) + expect do + update_user(user, { commit_email: 'foreign@email' }) + end.not_to change { user.reload.commit_email } + end + + + it 'does not update public if an user has group managed account' do + allow(user).to receive(:group_managed_account?).and_return(true) + expect do + update_user(user, { public_email: 'foreign@email' }) + end.not_to change { user.reload.public_email } + end + + + def update_user(user, opts) + described_class.new(user, opts.merge(user: user)).execute! + end + end +end -- GitLab From bb44df345dbadf21f93c2d7d33ad7ca98815055e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 26 Feb 2019 17:21:17 +0100 Subject: [PATCH 03/19] Add separate partial for notification email --- .../profiles/notifications/_email_settings.html.haml | 1 + app/views/profiles/notifications/show.html.haml | 2 +- ee/app/models/saml_provider.rb | 2 +- .../profiles/notifications/_email_settings.html.haml | 2 ++ ee/spec/services/users/update_service_spec.rb | 8 +++++++- 5 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 app/views/profiles/notifications/_email_settings.html.haml create mode 100644 ee/app/views/profiles/notifications/_email_settings.html.haml diff --git a/app/views/profiles/notifications/_email_settings.html.haml b/app/views/profiles/notifications/_email_settings.html.haml new file mode 100644 index 00000000000000..7e2857db872c06 --- /dev/null +++ b/app/views/profiles/notifications/_email_settings.html.haml @@ -0,0 +1 @@ += form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2" diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index 712eb2a4573f91..d4e829b9132679 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -24,7 +24,7 @@ = form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications prepend-top-default' } do |f| .form-group = f.label :notification_email, class: "label-bold" - = f.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2" + = render_if_exists 'profiles/notifications/email_settings', form: f = label_tag :global_notification_level, "Global notification level", class: "label-bold" %br diff --git a/ee/app/models/saml_provider.rb b/ee/app/models/saml_provider.rb index abcf4977e47c4d..0673269440736b 100644 --- a/ee/app/models/saml_provider.rb +++ b/ee/app/models/saml_provider.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class SamlProvider < ActiveRecord::Base - BLOCKED_USER_ATTRIBUTES = [:email, :public_email, :commit_email].freeze + BLOCKED_USER_ATTRIBUTES = [:email, :public_email, :commit_email, :notification_email].freeze belongs_to :group has_many :identities diff --git a/ee/app/views/profiles/notifications/_email_settings.html.haml b/ee/app/views/profiles/notifications/_email_settings.html.haml new file mode 100644 index 00000000000000..bf0d0a2afbcc83 --- /dev/null +++ b/ee/app/views/profiles/notifications/_email_settings.html.haml @@ -0,0 +1,2 @@ +- group_managed_account = true #@user.group_managed_account += form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: group_managed_account diff --git a/ee/spec/services/users/update_service_spec.rb b/ee/spec/services/users/update_service_spec.rb index e66a46f4c472f8..bfb8093ea2fcca 100644 --- a/ee/spec/services/users/update_service_spec.rb +++ b/ee/spec/services/users/update_service_spec.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'spec_helper' describe Users::UpdateService do @@ -18,7 +19,6 @@ end.not_to change { user.reload.commit_email } end - it 'does not update public if an user has group managed account' do allow(user).to receive(:group_managed_account?).and_return(true) expect do @@ -26,6 +26,12 @@ end.not_to change { user.reload.public_email } end + it 'does not update public if an user has group managed account' do + allow(user).to receive(:group_managed_account?).and_return(true) + expect do + update_user(user, { notification_email: 'foreign@email' }) + end.not_to change { user.reload.notification_email } + end def update_user(user, opts) described_class.new(user, opts.merge(user: user)).execute! -- GitLab From 2c4e752229eb46103cbe2de570d2641f1b6c47e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 27 Feb 2019 09:52:06 +0100 Subject: [PATCH 04/19] Add help-block in the view --- ee/app/services/ee/users/update_service.rb | 2 +- ee/app/views/profiles/notifications/_email_settings.html.haml | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/app/services/ee/users/update_service.rb b/ee/app/services/ee/users/update_service.rb index 0b71ee4e46dfdd..05194eed631ecc 100644 --- a/ee/app/services/ee/users/update_service.rb +++ b/ee/app/services/ee/users/update_service.rb @@ -21,7 +21,7 @@ def model end def assign_attributes - params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if @user.group_managed_account? + # params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if @user.group_managed_account? super end end diff --git a/ee/app/views/profiles/notifications/_email_settings.html.haml b/ee/app/views/profiles/notifications/_email_settings.html.haml index bf0d0a2afbcc83..3419e00fb7dbe1 100644 --- a/ee/app/views/profiles/notifications/_email_settings.html.haml +++ b/ee/app/views/profiles/notifications/_email_settings.html.haml @@ -1,2 +1,4 @@ - group_managed_account = true #@user.group_managed_account -= form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: group_managed_account += form.select :notification_email, @user.all_emails, { help: s_("here"), include_blank: false }, class: "select2", disabled: group_managed_account +.help-block + -- GitLab From bcf84ed33232b45d44ab767f380f475cdb88bf97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Fri, 1 Mar 2019 14:00:03 +0100 Subject: [PATCH 05/19] Update commented out methods --- ee/app/services/ee/users/update_service.rb | 2 +- ee/app/views/profiles/_email_settings.html.haml | 2 +- ee/app/views/profiles/notifications/_email_settings.html.haml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ee/app/services/ee/users/update_service.rb b/ee/app/services/ee/users/update_service.rb index 05194eed631ecc..0b71ee4e46dfdd 100644 --- a/ee/app/services/ee/users/update_service.rb +++ b/ee/app/services/ee/users/update_service.rb @@ -21,7 +21,7 @@ def model end def assign_attributes - # params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if @user.group_managed_account? + params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if @user.group_managed_account? super end end diff --git a/ee/app/views/profiles/_email_settings.html.haml b/ee/app/views/profiles/_email_settings.html.haml index b53562d2222bda..d6208310b94b9b 100644 --- a/ee/app/views/profiles/_email_settings.html.haml +++ b/ee/app/views/profiles/_email_settings.html.haml @@ -1,6 +1,6 @@ - readonly = @user.read_only_attribute?(:email) - help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) -- group_managed_account = true #@user.group_managed_account? +- group_managed_account = @user.group_managed_account? = form.text_field :email, required: true, class: 'input-lg', readonly: readonly || group_managed_account, value: (@user.email unless @user.temp_oauth_email?), help: help_text = form.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), diff --git a/ee/app/views/profiles/notifications/_email_settings.html.haml b/ee/app/views/profiles/notifications/_email_settings.html.haml index 3419e00fb7dbe1..3279dfee4eb221 100644 --- a/ee/app/views/profiles/notifications/_email_settings.html.haml +++ b/ee/app/views/profiles/notifications/_email_settings.html.haml @@ -1,4 +1,4 @@ -- group_managed_account = true #@user.group_managed_account -= form.select :notification_email, @user.all_emails, { help: s_("here"), include_blank: false }, class: "select2", disabled: group_managed_account +- group_managed_account = @user.group_managed_account += form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: group_managed_account .help-block -- GitLab From fbfeb83a211f11136d9ff49cfd4e68acec26e3f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 4 Mar 2019 12:00:30 +0100 Subject: [PATCH 06/19] Fix rubocop issues --- ee/app/services/ee/users/update_service.rb | 2 +- ee/app/views/profiles/notifications/_email_settings.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/services/ee/users/update_service.rb b/ee/app/services/ee/users/update_service.rb index 0b71ee4e46dfdd..9971ad49b9487f 100644 --- a/ee/app/services/ee/users/update_service.rb +++ b/ee/app/services/ee/users/update_service.rb @@ -21,7 +21,7 @@ def model end def assign_attributes - params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if @user.group_managed_account? + params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if model.group_managed_account? super end end diff --git a/ee/app/views/profiles/notifications/_email_settings.html.haml b/ee/app/views/profiles/notifications/_email_settings.html.haml index 3279dfee4eb221..92c1ec4888810b 100644 --- a/ee/app/views/profiles/notifications/_email_settings.html.haml +++ b/ee/app/views/profiles/notifications/_email_settings.html.haml @@ -1,4 +1,4 @@ -- group_managed_account = @user.group_managed_account +- group_managed_account = @user.group_managed_account? = form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: group_managed_account .help-block -- GitLab From 257c04ca77017e8d636b319bec17486c683c2745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 4 Mar 2019 15:44:06 +0100 Subject: [PATCH 07/19] Add copy to notifications page --- ee/app/views/profiles/notifications/_email_settings.html.haml | 2 ++ locale/gitlab.pot | 3 +++ 2 files changed, 5 insertions(+) diff --git a/ee/app/views/profiles/notifications/_email_settings.html.haml b/ee/app/views/profiles/notifications/_email_settings.html.haml index 92c1ec4888810b..59380af2ac742d 100644 --- a/ee/app/views/profiles/notifications/_email_settings.html.haml +++ b/ee/app/views/profiles/notifications/_email_settings.html.haml @@ -1,4 +1,6 @@ - group_managed_account = @user.group_managed_account? +- help_text = group_managed_account ? s_("Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO.").html_safe % { group_name: @user.managing_group.name } : nil = form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: group_managed_account .help-block + = help_text diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 773186c445f425..717e08deb79cbc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11732,6 +11732,9 @@ msgstr "" msgid "Your U2F device was registered!" msgstr "" +msgid "Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO." +msgstr "" + msgid "Your applications (%{size})" msgstr "" -- GitLab From fe302800fbd03280f15d18769d41588d7670a54b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 5 Mar 2019 13:28:18 +0100 Subject: [PATCH 08/19] Add changelog changes --- .../unreleased/force-notification-to-saml-email-ee.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml diff --git a/ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml b/ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml new file mode 100644 index 00000000000000..aed21e935d08fd --- /dev/null +++ b/ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml @@ -0,0 +1,5 @@ +--- +title: Block possibility to change email for users with group managed account +merge_request: +author: +type: added -- GitLab From 117885646991a716c71326db2d74d16e8c472ebb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Thu, 7 Mar 2019 14:04:26 +0100 Subject: [PATCH 09/19] Add cr remarks --- ee/app/models/saml_provider.rb | 2 +- ee/app/services/ee/users/update_service.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/models/saml_provider.rb b/ee/app/models/saml_provider.rb index 0673269440736b..f2ad6ac01fff39 100644 --- a/ee/app/models/saml_provider.rb +++ b/ee/app/models/saml_provider.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class SamlProvider < ActiveRecord::Base - BLOCKED_USER_ATTRIBUTES = [:email, :public_email, :commit_email, :notification_email].freeze + USER_ATTRIBUTES_LOCKED_FOR_MANAGED_ACCOUNTS = [:email, :public_email, :commit_email, :notification_email].freeze belongs_to :group has_many :identities diff --git a/ee/app/services/ee/users/update_service.rb b/ee/app/services/ee/users/update_service.rb index 9971ad49b9487f..1261b1ebf441a9 100644 --- a/ee/app/services/ee/users/update_service.rb +++ b/ee/app/services/ee/users/update_service.rb @@ -21,7 +21,7 @@ def model end def assign_attributes - params.reject! { |key, _| SamlProvider::BLOCKED_USER_ATTRIBUTES.include?(key.to_sym) } if model.group_managed_account? + params.reject! { |key, _| SamlProvider::USER_ATTRIBUTES_LOCKED_FOR_MANAGED_ACCOUNTS.include?(key.to_sym) } if model.group_managed_account? super end end -- GitLab From 20d28a408a3c89fd1b14a5f64d9cc60283738ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Thu, 7 Mar 2019 17:46:12 +0100 Subject: [PATCH 10/19] Refactor views into partials --- app/views/profiles/notifications/_email_settings.html.haml | 7 ++++++- app/views/profiles/notifications/show.html.haml | 4 +--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/views/profiles/notifications/_email_settings.html.haml b/app/views/profiles/notifications/_email_settings.html.haml index 7e2857db872c06..4cafc132dce100 100644 --- a/app/views/profiles/notifications/_email_settings.html.haml +++ b/app/views/profiles/notifications/_email_settings.html.haml @@ -1 +1,6 @@ -= form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2" +- form = local_assigns.fetch(:form) +.form-group + = form.label :notification_email, class: "label-bold" + = form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:group_managed_account, nil) + .help-block + = local_assigns.fetch(:help_text, nil) diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index d4e829b9132679..e616e5546b31ff 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -22,9 +22,7 @@ Global notification settings = form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications prepend-top-default' } do |f| - .form-group - = f.label :notification_email, class: "label-bold" - = render_if_exists 'profiles/notifications/email_settings', form: f + = render_if_exists 'profiles/notifications/email_settings', form: f = label_tag :global_notification_level, "Global notification level", class: "label-bold" %br -- GitLab From 4904a2792c021d61b7ec75c487b0dae8e272b254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Thu, 7 Mar 2019 17:46:25 +0100 Subject: [PATCH 11/19] Add ee partial --- ee/app/views/profiles/notifications/_email_settings.html.haml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ee/app/views/profiles/notifications/_email_settings.html.haml b/ee/app/views/profiles/notifications/_email_settings.html.haml index 59380af2ac742d..fcff9c7074d3fc 100644 --- a/ee/app/views/profiles/notifications/_email_settings.html.haml +++ b/ee/app/views/profiles/notifications/_email_settings.html.haml @@ -1,6 +1,4 @@ - group_managed_account = @user.group_managed_account? - help_text = group_managed_account ? s_("Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO.").html_safe % { group_name: @user.managing_group.name } : nil -= form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: group_managed_account -.help-block - = help_text += render_ce 'profiles/notifications/email_settings', group_managed_account: group_managed_account, help_text: help_text, form: form -- GitLab From 199f11d1ad7b9408d837b542ff0c59d59086a8d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Fri, 8 Mar 2019 09:35:58 +0100 Subject: [PATCH 12/19] Refactor partial to avoid duplication --- app/views/profiles/_email_settings.html.haml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml index 81bb47fcd529f9..4c6b9b3f6f60b5 100644 --- a/app/views/profiles/_email_settings.html.haml +++ b/app/views/profiles/_email_settings.html.haml @@ -1,10 +1,13 @@ +- form = local_assigns.fetch(:form) - readonly = @user.read_only_attribute?(:email) +- group_managed_account = local_assigns.fetch(:group_managed_account, nil) - help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) -= form.text_field :email, required: true, class: 'input-lg', readonly: readonly, value: (@user.email unless @user.temp_oauth_email?), help: help_text + += form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text, readonly: readonly || group_managed_account = form.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, - control_class: 'select2 input-lg' + control_class: 'select2 input-lg', disabled: group_managed_account - commit_email_docs_link = link_to s_('Profiles|Learn more'), help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') = form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), { help: s_("Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}").html_safe % { learn_more: commit_email_docs_link } }, - control_class: 'select2 input-lg' + control_class: 'select2 input-lg', disabled: group_managed_account -- GitLab From c9d48fea1dba52f29cb63672a45daf8a9878cd28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Fri, 8 Mar 2019 09:36:27 +0100 Subject: [PATCH 13/19] Refactor ee partial to reuse ce one --- ee/app/views/profiles/_email_settings.html.haml | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/ee/app/views/profiles/_email_settings.html.haml b/ee/app/views/profiles/_email_settings.html.haml index d6208310b94b9b..7a9d91317c72d7 100644 --- a/ee/app/views/profiles/_email_settings.html.haml +++ b/ee/app/views/profiles/_email_settings.html.haml @@ -1,12 +1,3 @@ -- readonly = @user.read_only_attribute?(:email) -- help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) - group_managed_account = @user.group_managed_account? -= form.text_field :email, required: true, class: 'input-lg', readonly: readonly || group_managed_account, value: (@user.email unless @user.temp_oauth_email?), help: help_text -= form.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), - { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, - control_class: 'select2 input-lg', disabled: group_managed_account -- commit_email_docs_link = link_to s_('Profiles|Learn more'), help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') -= form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), - { help: s_("Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}").html_safe % { learn_more: commit_email_docs_link } }, - control_class: 'select2 input-lg', disabled: group_managed_account += render_ce 'profiles/email_settings', form: form, group_managed_account: group_managed_account -- GitLab From 7787d4da5fe7c8d54542d3124f44a7ff4ff02e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 12 Mar 2019 10:20:07 +0100 Subject: [PATCH 14/19] Use different naming --- app/views/profiles/_email_settings.html.haml | 8 ++++---- .../profiles/notifications/_email_settings.html.haml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml index 4c6b9b3f6f60b5..b24105f4e8aeee 100644 --- a/app/views/profiles/_email_settings.html.haml +++ b/app/views/profiles/_email_settings.html.haml @@ -1,13 +1,13 @@ - form = local_assigns.fetch(:form) - readonly = @user.read_only_attribute?(:email) -- group_managed_account = local_assigns.fetch(:group_managed_account, nil) +- email_change_disabled = local_assigns.fetch(:email_change_disabled, nil) - help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) -= form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text, readonly: readonly || group_managed_account += form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text, readonly: readonly || email_change_disabled = form.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, - control_class: 'select2 input-lg', disabled: group_managed_account + control_class: 'select2 input-lg', disabled: email_change_disabled - commit_email_docs_link = link_to s_('Profiles|Learn more'), help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') = form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), { help: s_("Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}").html_safe % { learn_more: commit_email_docs_link } }, - control_class: 'select2 input-lg', disabled: group_managed_account + control_class: 'select2 input-lg', disabled: email_change_disabled diff --git a/app/views/profiles/notifications/_email_settings.html.haml b/app/views/profiles/notifications/_email_settings.html.haml index 4cafc132dce100..34dcf8f5402d5c 100644 --- a/app/views/profiles/notifications/_email_settings.html.haml +++ b/app/views/profiles/notifications/_email_settings.html.haml @@ -1,6 +1,6 @@ - form = local_assigns.fetch(:form) .form-group = form.label :notification_email, class: "label-bold" - = form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:group_managed_account, nil) + = form.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil) .help-block = local_assigns.fetch(:help_text, nil) -- GitLab From 5727c9e5c1948ec9cdeb9704b9d65ae2b3d68945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 12 Mar 2019 10:39:11 +0100 Subject: [PATCH 15/19] Modify partials to respect ce changes --- ee/app/views/profiles/_email_settings.html.haml | 2 +- ee/app/views/profiles/notifications/_email_settings.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/views/profiles/_email_settings.html.haml b/ee/app/views/profiles/_email_settings.html.haml index 7a9d91317c72d7..2873d28c0787fe 100644 --- a/ee/app/views/profiles/_email_settings.html.haml +++ b/ee/app/views/profiles/_email_settings.html.haml @@ -1,3 +1,3 @@ - group_managed_account = @user.group_managed_account? -= render_ce 'profiles/email_settings', form: form, group_managed_account: group_managed_account += render_ce 'profiles/email_settings', form: form, email_change_disabled: group_managed_account diff --git a/ee/app/views/profiles/notifications/_email_settings.html.haml b/ee/app/views/profiles/notifications/_email_settings.html.haml index fcff9c7074d3fc..eb68bc3a389e0a 100644 --- a/ee/app/views/profiles/notifications/_email_settings.html.haml +++ b/ee/app/views/profiles/notifications/_email_settings.html.haml @@ -1,4 +1,4 @@ - group_managed_account = @user.group_managed_account? - help_text = group_managed_account ? s_("Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO.").html_safe % { group_name: @user.managing_group.name } : nil -= render_ce 'profiles/notifications/email_settings', group_managed_account: group_managed_account, help_text: help_text, form: form += render_ce 'profiles/notifications/email_settings', email_change_disabled: group_managed_account, help_text: help_text, form: form -- GitLab From 7331eaf1141d25a1233229339696c37c42915646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 18 Mar 2019 11:27:30 +0100 Subject: [PATCH 16/19] Add cr remarks --- ee/app/models/saml_provider.rb | 2 +- ee/app/services/ee/users/update_service.rb | 2 ++ .../unreleased/force-notification-to-saml-email-ee.yml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/app/models/saml_provider.rb b/ee/app/models/saml_provider.rb index f2ad6ac01fff39..83f4e28b5dcf31 100644 --- a/ee/app/models/saml_provider.rb +++ b/ee/app/models/saml_provider.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class SamlProvider < ActiveRecord::Base - USER_ATTRIBUTES_LOCKED_FOR_MANAGED_ACCOUNTS = [:email, :public_email, :commit_email, :notification_email].freeze + USER_ATTRIBUTES_LOCKED_FOR_MANAGED_ACCOUNTS = %i(email public_email commit_email notification_email).freeze belongs_to :group has_many :identities diff --git a/ee/app/services/ee/users/update_service.rb b/ee/app/services/ee/users/update_service.rb index 1261b1ebf441a9..d9c75e789a8521 100644 --- a/ee/app/services/ee/users/update_service.rb +++ b/ee/app/services/ee/users/update_service.rb @@ -3,6 +3,7 @@ module EE module Users module UpdateService + extend ::Gitlab::Utils::Override include EE::Audit::Changes # rubocop: disable Cop/InjectEnterpriseEditionModule private @@ -20,6 +21,7 @@ def model @user end + override :assign_attributes def assign_attributes params.reject! { |key, _| SamlProvider::USER_ATTRIBUTES_LOCKED_FOR_MANAGED_ACCOUNTS.include?(key.to_sym) } if model.group_managed_account? super diff --git a/ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml b/ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml index aed21e935d08fd..55049c02a412d3 100644 --- a/ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml +++ b/ee/changelogs/unreleased/force-notification-to-saml-email-ee.yml @@ -1,5 +1,5 @@ --- title: Block possibility to change email for users with group managed account -merge_request: +merge_request: 9712 author: type: added -- GitLab From ec50e1f7a1da383e0034f9d684e32d7b88c9a47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 18 Mar 2019 12:25:24 +0100 Subject: [PATCH 17/19] Add line between setup and expectations --- ee/spec/services/users/update_service_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ee/spec/services/users/update_service_spec.rb b/ee/spec/services/users/update_service_spec.rb index bfb8093ea2fcca..b5446d874d3f95 100644 --- a/ee/spec/services/users/update_service_spec.rb +++ b/ee/spec/services/users/update_service_spec.rb @@ -7,6 +7,7 @@ describe '#execute' do it 'does not update email if an user has group managed account' do allow(user).to receive(:group_managed_account?).and_return(true) + expect do update_user(user, { email: 'foreign@email' }) end.not_to change { user.reload.email } @@ -14,6 +15,7 @@ it 'does not update commit email if an user has group managed account' do allow(user).to receive(:group_managed_account?).and_return(true) + expect do update_user(user, { commit_email: 'foreign@email' }) end.not_to change { user.reload.commit_email } @@ -21,6 +23,7 @@ it 'does not update public if an user has group managed account' do allow(user).to receive(:group_managed_account?).and_return(true) + expect do update_user(user, { public_email: 'foreign@email' }) end.not_to change { user.reload.public_email } @@ -28,6 +31,7 @@ it 'does not update public if an user has group managed account' do allow(user).to receive(:group_managed_account?).and_return(true) + expect do update_user(user, { notification_email: 'foreign@email' }) end.not_to change { user.reload.notification_email } -- GitLab From 24be7d8c5603e6f5835bcdeecd2e1633dfcd9b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 18 Mar 2019 12:25:31 +0100 Subject: [PATCH 18/19] Add cr remarks --- app/views/profiles/_email_settings.html.haml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml index b24105f4e8aeee..4344e1a0f60c75 100644 --- a/app/views/profiles/_email_settings.html.haml +++ b/app/views/profiles/_email_settings.html.haml @@ -1,13 +1,16 @@ - form = local_assigns.fetch(:form) - readonly = @user.read_only_attribute?(:email) - email_change_disabled = local_assigns.fetch(:email_change_disabled, nil) -- help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) +- read_only_help_text = readonly ? s_("Profiles|Your email address was automatically set based on your %{provider_label} account") % { provider_label: attribute_provider_label(:email) } : user_email_help_text(@user) +- help_text = email_change_disabled ? s_("Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO.") % { group_name: @user.managing_group.name } : read_only_help_text -= form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text, readonly: readonly || email_change_disabled += form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text.html_safe, readonly: readonly || email_change_disabled = form.select :public_email, options_for_select(@user.all_emails, selected: @user.public_email), { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, control_class: 'select2 input-lg', disabled: email_change_disabled -- commit_email_docs_link = link_to s_('Profiles|Learn more'), help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') +- commit_email_link_url = help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') +- commit_email_link_start = ''.html_safe % { url: zones_link_url } +- commit_email_docs_link = s_('Profiles|This email will be used for web based operations, such as edits and merges. %{commit_email_link_start}Learn more%{commit_email_link_end}').html_safe % { commit_email_link_start: zones_link_start, commit_email_link_end: ''.html_safe } = form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), - { help: s_("Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}").html_safe % { learn_more: commit_email_docs_link } }, + { help: commit_email_docs_link }, control_class: 'select2 input-lg', disabled: email_change_disabled -- GitLab From 0d45721436120e126bd520d93eb1818063f3cf09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 18 Mar 2019 13:55:28 +0100 Subject: [PATCH 19/19] Fix failing specs --- app/views/profiles/_email_settings.html.haml | 4 ++-- locale/gitlab.pot | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml index 4344e1a0f60c75..fb4da08e129c89 100644 --- a/app/views/profiles/_email_settings.html.haml +++ b/app/views/profiles/_email_settings.html.haml @@ -9,8 +9,8 @@ { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, control_class: 'select2 input-lg', disabled: email_change_disabled - commit_email_link_url = help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') -- commit_email_link_start = ''.html_safe % { url: zones_link_url } -- commit_email_docs_link = s_('Profiles|This email will be used for web based operations, such as edits and merges. %{commit_email_link_start}Learn more%{commit_email_link_end}').html_safe % { commit_email_link_start: zones_link_start, commit_email_link_end: ''.html_safe } +- commit_email_link_start = ''.html_safe % { url: commit_email_link_url } +- commit_email_docs_link = s_('Profiles|This email will be used for web based operations, such as edits and merges. %{commit_email_link_start}Learn more%{commit_email_link_end}').html_safe % { commit_email_link_start: commit_email_link_start, commit_email_link_end: ''.html_safe } = form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: selected_commit_email(@user)), { help: commit_email_docs_link }, control_class: 'select2 input-lg', disabled: email_change_disabled diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 717e08deb79cbc..8d74b65b57da71 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7788,7 +7788,7 @@ msgstr "" msgid "Profiles|This email will be displayed on your public profile" msgstr "" -msgid "Profiles|This email will be used for web based operations, such as edits and merges. %{learn_more}" +msgid "Profiles|This email will be used for web based operations, such as edits and merges. %{commit_email_link_start}Learn more%{commit_email_link_end}" msgstr "" msgid "Profiles|This emoji and message will appear on your profile and throughout the interface." -- GitLab