Commit 1e61fc76 authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/security/gitlab@13-0-stable-ee

parent 57b9b49b
...@@ -40,7 +40,9 @@ export default class Profile { ...@@ -40,7 +40,9 @@ export default class Profile {
bindEvents() { bindEvents() {
$('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm); $('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm);
$('.js-group-notification-email').on('change', this.submitForm); $('.js-group-notification-email').on('change', this.submitForm);
$('#user_notification_email').on('change', this.submitForm); $('#user_notification_email').on('select2-selecting', event => {
setTimeout(this.submitForm.bind(event.currentTarget));
});
$('#user_notified_of_own_activity').on('change', this.submitForm); $('#user_notified_of_own_activity').on('change', this.submitForm);
this.form.on('submit', this.onSubmitForm); this.form.on('submit', this.onSubmitForm);
} }
......
...@@ -37,6 +37,8 @@ class Projects::DeployKeysController < Projects::ApplicationController ...@@ -37,6 +37,8 @@ class Projects::DeployKeysController < Projects::ApplicationController
end end
def update def update
access_denied! unless deploy_key
if deploy_key.update(update_params) if deploy_key.update(update_params)
flash[:notice] = _('Deploy key was successfully updated.') flash[:notice] = _('Deploy key was successfully updated.')
redirect_to_repository redirect_to_repository
...@@ -85,10 +87,12 @@ class Projects::DeployKeysController < Projects::ApplicationController ...@@ -85,10 +87,12 @@ class Projects::DeployKeysController < Projects::ApplicationController
end end
def update_params def update_params
permitted_params = [deploy_keys_projects_attributes: [:id, :can_push]] permitted_params = [deploy_keys_projects_attributes: [:can_push]]
permitted_params << :title if can?(current_user, :update_deploy_key, deploy_key) permitted_params << :title if can?(current_user, :update_deploy_key, deploy_key)
params.require(:deploy_key).permit(*permitted_params) key_update_params = params.require(:deploy_key).permit(*permitted_params)
key_update_params.dig(:deploy_keys_projects_attributes, '0')&.merge!(id: deploy_keys_project.id)
key_update_params
end end
def authorize_update_deploy_key! def authorize_update_deploy_key!
......
...@@ -14,6 +14,7 @@ class NotificationSetting < ApplicationRecord ...@@ -14,6 +14,7 @@ class NotificationSetting < ApplicationRecord
validates :user_id, uniqueness: { scope: [:source_type, :source_id], validates :user_id, uniqueness: { scope: [:source_type, :source_id],
message: "already exists in source", message: "already exists in source",
allow_nil: true } allow_nil: true }
validate :owns_notification_email, if: :notification_email_changed?
scope :for_groups, -> { where(source_type: 'Namespace') } scope :for_groups, -> { where(source_type: 'Namespace') }
...@@ -97,6 +98,13 @@ class NotificationSetting < ApplicationRecord ...@@ -97,6 +98,13 @@ class NotificationSetting < ApplicationRecord
def event_enabled?(event) def event_enabled?(event)
respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend
end end
def owns_notification_email
return if user.temp_oauth_email?
return if notification_email.empty?
errors.add(:notification_email, _("is not an email you own")) unless user.verified_emails.include?(notification_email)
end
end end
NotificationSetting.prepend_if_ee('EE::NotificationSetting') NotificationSetting.prepend_if_ee('EE::NotificationSetting')
...@@ -756,15 +756,15 @@ class User < ApplicationRecord ...@@ -756,15 +756,15 @@ class User < ApplicationRecord
end end
def owns_notification_email def owns_notification_email
return if temp_oauth_email? return if new_record? || temp_oauth_email?
errors.add(:notification_email, _("is not an email you own")) unless all_emails.include?(notification_email) errors.add(:notification_email, _("is not an email you own")) unless verified_emails.include?(notification_email)
end end
def owns_public_email def owns_public_email
return if public_email.blank? return if public_email.blank?
errors.add(:public_email, _("is not an email you own")) unless all_emails.include?(public_email) errors.add(:public_email, _("is not an email you own")) unless verified_emails.include?(public_email)
end end
def owns_commit_email def owns_commit_email
...@@ -1212,18 +1212,20 @@ class User < ApplicationRecord ...@@ -1212,18 +1212,20 @@ class User < ApplicationRecord
all_emails all_emails
end end
def all_public_emails def verified_emails(include_private_email: true)
all_emails(include_private_email: false)
end
def verified_emails
verified_emails = [] verified_emails = []
verified_emails << email if primary_email_verified? verified_emails << email if primary_email_verified?
verified_emails << private_commit_email verified_emails << private_commit_email if include_private_email
verified_emails.concat(emails.confirmed.pluck(:email)) verified_emails.concat(emails.confirmed.pluck(:email))
verified_emails verified_emails
end end
def public_verified_emails
emails = verified_emails(include_private_email: false)
emails << email unless temp_oauth_email?
emails.uniq
end
def any_email?(check_email) def any_email?(check_email)
downcased = check_email.downcase downcased = check_email.downcase
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
- 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 - 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.html_safe, 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_public_emails, selected: @user.public_email), = form.select :public_email, options_for_select(@user.public_verified_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") }, { 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 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_url = help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank')
......
- form = local_assigns.fetch(:form) - form = local_assigns.fetch(:form)
.form-group .form-group
= form.label :notification_email, class: "label-bold" = form.label :notification_email, class: "label-bold"
= form.select :notification_email, @user.all_public_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil) = form.select :notification_email, @user.public_verified_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil)
.help-block .help-block
= local_assigns.fetch(:help_text, nil) = local_assigns.fetch(:help_text, nil)
...@@ -13,4 +13,4 @@ ...@@ -13,4 +13,4 @@
.table-section.section-30 .table-section.section-30
= form_for setting, url: profile_notifications_group_path(group), method: :put, html: { class: 'update-notifications' } do |f| = form_for setting, url: profile_notifications_group_path(group), method: :put, html: { class: 'update-notifications' } do |f|
= f.select :notification_email, @user.all_public_emails, { include_blank: 'Global notification email' }, class: 'select2 js-group-notification-email' = f.select :notification_email, @user.public_verified_emails, { include_blank: 'Global notification email' }, class: 'select2 js-group-notification-email'
- page_title 'Edit Deploy Key' - page_title 'Edit Deploy Key'
%h3.page-title Edit Deploy Key %h3.page-title= _('Edit Deploy Key')
%hr %hr
%div %div
= form_for [@project.namespace.becomes(Namespace), @project, @deploy_key], html: { class: 'js-requires-input' } do |f| = form_for [@project.namespace.becomes(Namespace), @project, @deploy_key], include_id: false, html: { class: 'js-requires-input' } do |f|
= render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key } = render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key }
.form-actions .form-actions
= f.submit 'Save changes', class: 'btn-success btn' = f.submit 'Save changes', class: 'btn-success btn'
......
---
title: Added data integrity check before updating a deploy key.
merge_request:
author:
type: security
---
title: Display only verified emails on notifications and profile page
merge_request:
author:
type: security
...@@ -5,8 +5,8 @@ require 'spec_helper' ...@@ -5,8 +5,8 @@ require 'spec_helper'
describe Profiles::NotificationsController do describe Profiles::NotificationsController do
let(:user) do let(:user) do
create(:user) do |user| create(:user) do |user|
user.emails.create(email: 'original@example.com') user.emails.create(email: 'original@example.com', confirmed_at: Time.current)
user.emails.create(email: 'new@example.com') user.emails.create(email: 'new@example.com', confirmed_at: Time.current)
user.notification_email = 'original@example.com' user.notification_email = 'original@example.com'
user.save! user.save!
end end
......
...@@ -256,7 +256,7 @@ describe Projects::DeployKeysController do ...@@ -256,7 +256,7 @@ describe Projects::DeployKeysController do
end end
def deploy_key_params(title, can_push) def deploy_key_params(title, can_push)
deploy_keys_projects_attributes = { '0' => { id: deploy_keys_project, can_push: can_push } } deploy_keys_projects_attributes = { '0' => { can_push: can_push } }
{ deploy_key: { title: title, deploy_keys_projects_attributes: deploy_keys_projects_attributes } } { deploy_key: { title: title, deploy_keys_projects_attributes: deploy_keys_projects_attributes } }
end end
...@@ -300,6 +300,42 @@ describe Projects::DeployKeysController do ...@@ -300,6 +300,42 @@ describe Projects::DeployKeysController do
expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true)
end end
end end
context 'when a different deploy key id param is injected' do
let(:extra_params) { deploy_key_params('updated title', '1') }
let(:hacked_params) do
extra_params.reverse_merge(id: other_deploy_key_id,
namespace_id: project.namespace,
project_id: project)
end
subject { put :update, params: hacked_params }
context 'and that deploy key id exists' do
let(:other_project) { create(:project) }
let(:other_deploy_key) do
key = create(:deploy_key)
project.deploy_keys << key
key
end
let(:other_deploy_key_id) { other_deploy_key.id }
it 'does not update the can_push attribute' do
expect { subject }.not_to change { deploy_key.deploy_keys_project_for(project).can_push }
end
end
context 'and that deploy key id does not exist' do
let(:other_deploy_key_id) { 9999 }
it 'returns 404' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end end
context 'with admin as project maintainer' do context 'with admin as project maintainer' do
......
...@@ -110,6 +110,11 @@ describe Group do ...@@ -110,6 +110,11 @@ describe Group do
let(:group_notification_email) { 'user+group@example.com' } let(:group_notification_email) { 'user+group@example.com' }
let(:subgroup_notification_email) { 'user+subgroup@example.com' } let(:subgroup_notification_email) { 'user+subgroup@example.com' }
before do
create(:email, :confirmed, user: user, email: group_notification_email)
create(:email, :confirmed, user: user, email: subgroup_notification_email)
end
subject { subgroup.notification_email_for(user) } subject { subgroup.notification_email_for(user) }
context 'when both group notification emails are set' do context 'when both group notification emails are set' do
......
...@@ -48,6 +48,33 @@ RSpec.describe NotificationSetting do ...@@ -48,6 +48,33 @@ RSpec.describe NotificationSetting do
expect(notification_setting.reopen_merge_request).to eq(false) expect(notification_setting.reopen_merge_request).to eq(false)
end end
end end
context 'notification_email' do
let_it_be(:user) { create(:user) }
subject { described_class.new(source_id: 1, source_type: 'Project', user_id: user.id) }
it 'allows to change email to verified one' do
email = create(:email, :confirmed, user: user)
subject.update(notification_email: email.email)
expect(subject).to be_valid
end
it 'does not allow to change email to not verified one' do
email = create(:email, user: user)
subject.update(notification_email: email.email)
expect(subject).to be_invalid
end
it 'allows to change email to empty one' do
subject.update(notification_email: '')
expect(subject).to be_valid
end
end
end end
describe '#for_projects' do describe '#for_projects' do
......
...@@ -310,7 +310,7 @@ describe User do ...@@ -310,7 +310,7 @@ describe User do
end end
it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :public_email, :notification_email do it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :public_email, :notification_email do
subject { build(:user).tap { |user| user.emails << build(:email, email: email_value) } } subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } }
end end
describe '#commit_email' do describe '#commit_email' do
...@@ -567,6 +567,32 @@ describe User do ...@@ -567,6 +567,32 @@ describe User do
user = build(:user, email: "temp-email-for-oauth@example.com") user = build(:user, email: "temp-email-for-oauth@example.com")
expect(user).to be_valid expect(user).to be_valid
end end
it 'does not accept not verified emails' do
email = create(:email)
user = email.user
user.update(notification_email: email.email)
expect(user).to be_invalid
end
end
context 'owns_public_email' do
it 'accepts verified emails' do
email = create(:email, :confirmed, email: 'test@test.com')
user = email.user
user.update(public_email: email.email)
expect(user).to be_valid
end
it 'does not accept not verified emails' do
email = create(:email)
user = email.user
user.update(public_email: email.email)
expect(user).to be_invalid
end
end end
context 'set_commit_email' do context 'set_commit_email' do
...@@ -2171,6 +2197,31 @@ describe User do ...@@ -2171,6 +2197,31 @@ describe User do
end end
end end
describe '#public_verified_emails' do
let(:user) { create(:user) }
it 'returns only confirmed public emails' do
email_confirmed = create :email, user: user, confirmed_at: Time.current
create :email, user: user
expect(user.public_verified_emails).to contain_exactly(
user.email,
email_confirmed.email
)
end
it 'returns confirmed public emails plus main user email when user is not confirmed' do
user = create(:user, confirmed_at: nil)
email_confirmed = create :email, user: user, confirmed_at: Time.current
create :email, user: user
expect(user.public_verified_emails).to contain_exactly(
user.email,
email_confirmed.email
)
end
end
describe '#verified_email?' do describe '#verified_email?' do
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -4333,9 +4384,10 @@ describe User do ...@@ -4333,9 +4384,10 @@ describe User do
context 'when an ancestor has a level other than Global' do context 'when an ancestor has a level other than Global' do
let(:ancestor) { create(:group) } let(:ancestor) { create(:group) }
let(:group) { create(:group, parent: ancestor) } let(:group) { create(:group, parent: ancestor) }
let(:email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) }
before do before do
create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: 'ancestor@example.com') create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: email.email)
end end
it 'has the same level set' do it 'has the same level set' do
...@@ -4360,10 +4412,12 @@ describe User do ...@@ -4360,10 +4412,12 @@ describe User do
let(:grand_ancestor) { create(:group) } let(:grand_ancestor) { create(:group) }
let(:ancestor) { create(:group, parent: grand_ancestor) } let(:ancestor) { create(:group, parent: grand_ancestor) }
let(:group) { create(:group, parent: ancestor) } let(:group) { create(:group, parent: ancestor) }
let(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) }
let(:grand_email) { create(:email, :confirmed, email: 'grand@example.com', user: user) }
before do before do
create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: 'grand@example.com') create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: grand_email.email)
create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: 'ancestor@example.com') create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: ancestor_email.email)
end end
it 'has the same email set' do it 'has the same email set' do
...@@ -4401,7 +4455,7 @@ describe User do ...@@ -4401,7 +4455,7 @@ describe User do
context 'when group has notification email set' do context 'when group has notification email set' do
it 'returns group notification email' do it 'returns group notification email' do
group_notification_email = 'user+group@example.com' group_notification_email = 'user+group@example.com'
create(:email, :confirmed, user: user, email: group_notification_email)
create(:notification_setting, user: user, source: group, notification_email: group_notification_email) create(:notification_setting, user: user, source: group, notification_email: group_notification_email)
is_expected.to eq(group_notification_email) is_expected.to eq(group_notification_email)
......
...@@ -19,7 +19,7 @@ describe API::NotificationSettings do ...@@ -19,7 +19,7 @@ describe API::NotificationSettings do
end end
describe "PUT /notification_settings" do describe "PUT /notification_settings" do
let(:email) { create(:email, user: user) } let(:email) { create(:email, :confirmed, user: user) }
it "updates global notification settings for the current user" do it "updates global notification settings for the current user" do
put api("/notification_settings", user), params: { level: 'watch', notification_email: email.email } put api("/notification_settings", user), params: { level: 'watch', notification_email: email.email }
......
...@@ -9,15 +9,11 @@ describe 'OpenID Connect requests' do ...@@ -9,15 +9,11 @@ describe 'OpenID Connect requests' do
name: 'Alice', name: 'Alice',
username: 'alice', username: 'alice',
email: 'private@example.com', email: 'private@example.com',
emails: [public_email],
public_email: public_email.email,
website_url: 'https://example.com', website_url: 'https://example.com',
avatar: fixture_file_upload('spec/fixtures/dk.png') avatar: fixture_file_upload('spec/fixtures/dk.png')
) )
end end
let(:public_email) { build :email, email: 'public@example.com' }
let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id } let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id }
let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id } let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id }
...@@ -37,7 +33,7 @@ describe 'OpenID Connect requests' do ...@@ -37,7 +33,7 @@ describe 'OpenID Connect requests' do
'name' => 'Alice', 'name' => 'Alice',
'nickname' => 'alice', 'nickname' => 'alice',
'email' => 'public@example.com', 'email' => 'public@example.com',
'email_verified' => false, 'email_verified' => true,
'website' => 'https://example.com', 'website' => 'https://example.com',
'profile' => 'http://localhost/alice', 'profile' => 'http://localhost/alice',
'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png", 'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png",
...@@ -62,6 +58,11 @@ describe 'OpenID Connect requests' do ...@@ -62,6 +58,11 @@ describe 'OpenID Connect requests' do
get '/oauth/userinfo', params: {}, headers: { 'Authorization' => "Bearer #{access_token.token}" } get '/oauth/userinfo', params: {}, headers: { 'Authorization' => "Bearer #{access_token.token}" }
end end
before do
email = create(:email, :confirmed, email: 'public@example.com', user: user)
user.update!(public_email: email.email)
end
context 'Application without OpenID scope' do context 'Application without OpenID scope' do
let(:application) { create :oauth_application, scopes: 'api' } let(:application) { create :oauth_application, scopes: 'api' }
...@@ -123,7 +124,7 @@ describe 'OpenID Connect requests' do ...@@ -123,7 +124,7 @@ describe 'OpenID Connect requests' do
end end
it 'has false in email_verified claim' do it 'has false in email_verified claim' do
expect(json_response['email_verified']).to eq(false) expect(json_response['email_verified']).to eq(true)
end end
end end
......
...@@ -5,8 +5,8 @@ require 'spec_helper' ...@@ -5,8 +5,8 @@ require 'spec_helper'
describe 'view user notifications' do describe 'view user notifications' do
let(:user) do let(:user) do
create(:user) do |user| create(:user) do |user|
user.emails.create(email: 'original@example.com') user.emails.create(email: 'original@example.com', confirmed_at: Time.current)
user.emails.create(email: 'new@example.com') user.emails.create(email: 'new@example.com', confirmed_at: Time.current)
user.notification_email = 'original@example.com' user.notification_email = 'original@example.com'
user.save! user.save!
end end
......
...@@ -2457,6 +2457,8 @@ describe NotificationService, :mailer do ...@@ -2457,6 +2457,8 @@ describe NotificationService, :mailer do
group = create(:group) group = create(:group)
project.update(group: group) project.update(group: group)
create(:email, :confirmed, user: u_custom_notification_enabled, email: group_notification_email)
create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email) create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email)
end end
...@@ -2491,6 +2493,7 @@ describe NotificationService, :mailer do ...@@ -2491,6 +2493,7 @@ describe NotificationService, :mailer do
group = create(:group) group = create(:group)
project.update(group: group) project.update(group: group)
create(:email, :confirmed, user: u_member, email: group_notification_email)
create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email) create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
end end
...@@ -2584,6 +2587,7 @@ describe NotificationService, :mailer do ...@@ -2584,6 +2587,7 @@ describe NotificationService, :mailer do
group = create(:group) group = create(:group)
project.update(group: group) project.update(group: group)
create(:email, :confirmed, user: u_member, email: group_notification_email)
create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email) create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
end end
......
...@@ -28,6 +28,7 @@ RSpec.shared_examples 'an email sent to a user' do ...@@ -28,6 +28,7 @@ RSpec.shared_examples 'an email sent to a user' do
it 'is sent to user\'s group notification email' do it 'is sent to user\'s group notification email' do
group_notification_email = 'user+group@example.com' group_notification_email = 'user+group@example.com'
create(:email, :confirmed, user: recipient, email: group_notification_email)
create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email) create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email)
expect(subject).to deliver_to(group_notification_email) expect(subject).to deliver_to(group_notification_email)
......
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