Commit 3029be8b authored by Ash McKenzie's avatar Ash McKenzie

Merge branch...

Merge branch '24605-allow-admins-to-disable-users-ability-to-change-profile-name-premium-only' into 'master'

Move "Allow admins to disable users ability to change profile name" feature to EE Premium

See merge request gitlab-org/gitlab!23034
parents bc5a8494 e213d115
......@@ -284,7 +284,6 @@ module ApplicationSettingsHelper
:unique_ips_limit_enabled,
:unique_ips_limit_per_user,
:unique_ips_limit_time_window,
:updating_name_disabled_for_users,
:usage_ping_enabled,
:instance_statistics_visibility_private,
:user_default_external,
......
......@@ -13,11 +13,6 @@ class UserPolicy < BasePolicy
desc "The user is blocked"
condition(:blocked_user, scope: :subject, score: 0) { @subject.blocked? }
condition(:updating_name_disabled_for_users) do
::Gitlab::CurrentSettings.current_application_settings
.updating_name_disabled_for_users
end
rule { ~restricted_public_level }.enable :read_user
rule { ~anonymous }.enable :read_user
......@@ -27,8 +22,8 @@ class UserPolicy < BasePolicy
enable :update_user_status
end
rule { can?(:update_user) & ( admin | ~updating_name_disabled_for_users ) }.enable :update_name
rule { default }.enable :read_user_profile
rule { (private_profile | blocked_user) & ~(user_is_self | admin) }.prevent :read_user_profile
end
UserPolicy.prepend_if_ee('EE::UserPolicy')
......@@ -54,7 +54,6 @@ module Users
def discard_read_only_attributes
discard_synced_attributes
discard_name unless name_updatable?
end
def discard_synced_attributes
......@@ -65,14 +64,6 @@ module Users
end
end
def discard_name
params.delete(:name)
end
def name_updatable?
can?(current_user, :update_name, @user)
end
def assign_attributes
@user.assign_attributes(params.except(*identity_attributes)) unless params.empty?
end
......
......@@ -51,13 +51,8 @@
= f.check_box :user_show_add_ssh_key_message, class: 'form-check-input'
= f.label :user_show_add_ssh_key_message, class: 'form-check-label' do
= _("Inform users without uploaded SSH keys that they can't push over SSH until one is added")
.form-group
= f.label :updating_name_disabled_for_users, _('User restrictions'), class: 'label-bold'
.form-check
= f.check_box :updating_name_disabled_for_users, class: 'form-check-input'
= f.label :updating_name_disabled_for_users, class: 'form-check-label' do
= _("Prevent users from changing their profile name")
= render_if_exists 'admin/application_settings/updating_name_disabled_for_users', form: f
= render_if_exists 'admin/application_settings/availability_on_namespace_setting', form: f
= f.submit _('Save changes'), class: 'btn btn-success qa-save-changes-button'
- if user.read_only_attribute?(:name)
= form.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9 qa-full-name rspec-full-name' },
help: s_("Profiles|Your name was automatically set based on your %{provider_label} account, so people you know can recognize you") % { provider_label: attribute_provider_label(:name) }
- elsif can?(current_user, :update_name, user)
= form.text_field :name, label: s_('Profiles|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 rspec-full-name' }, help: s_("Profiles|Enter your name, so people you know can recognize you")
- else
= form.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9 qa-full-name rspec-full-name' },
help: s_("Profiles|The ability to update your name has been disabled by your administrator.")
= form.text_field :name, label: s_('Profiles|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 rspec-full-name' }, help: s_("Profiles|Enter your name, so people you know can recognize you")
......@@ -117,7 +117,7 @@ Once a lifetime for personal access tokens is set, GitLab will:
allowed lifetime. Three hours is given to allow administrators to change the allowed lifetime,
or remove it, before revocation takes place.
## Disabling user profile name changes **(CORE ONLY)**
## Disabling user profile name changes **(PREMIUM ONLY)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/24605) in GitLab 12.7.
......
......@@ -46,6 +46,10 @@ module EE
attrs << :required_instance_ci_template
end
if License.feature_available?(:disable_name_update_for_users)
attrs << :updating_name_disabled_for_users
end
attrs
end
......
......@@ -89,6 +89,7 @@ module EE
file_template_project_id
default_project_deletion_protection
deletion_adjourned_period
updating_name_disabled_for_users
]
end
end
......
......@@ -64,6 +64,7 @@ class License < ApplicationRecord
dependency_proxy
deploy_board
design_management
disable_name_update_for_users
email_additional_text
extended_audit_events
external_authorization_service_api_management
......
# frozen_string_literal: true
module EE
module UserPolicy
extend ActiveSupport::Concern
prepended do
condition(:updating_name_disabled_for_users) do
::License.feature_available?(:disable_name_update_for_users) &&
::Gitlab::CurrentSettings.current_application_settings.updating_name_disabled_for_users
end
rule { can?(:update_user) }.enable :update_name
rule do
updating_name_disabled_for_users &
~admin
end.prevent :update_name
end
end
end
......@@ -31,6 +31,21 @@ module EE
@user
end
override :discard_read_only_attributes
def discard_read_only_attributes
super
discard_name unless name_updatable?
end
def discard_name
params.delete(:name)
end
def name_updatable?
can?(current_user, :update_name, model)
end
override :identity_params
def identity_params
if group_id_for_saml.present?
......
- return unless ::License.feature_available?(:disable_name_update_for_users)
- form = local_assigns.fetch(:form)
.form-group
= form.label :updating_name_disabled_for_users, _('User restrictions'), class: 'label-bold'
.form-check
= form.check_box :updating_name_disabled_for_users, class: 'form-check-input'
= form.label :updating_name_disabled_for_users, class: 'form-check-label' do
= _("Prevent users from changing their profile name")
- if user.read_only_attribute?(:name)
= form.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9 qa-full-name rspec-full-name' },
help: s_("Profiles|Your name was automatically set based on your %{provider_label} account, so people you know can recognize you") % { provider_label: attribute_provider_label(:name) }
- elsif can?(current_user, :update_name, user)
= form.text_field :name, label: s_('Profiles|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 rspec-full-name' }, help: s_("Profiles|Enter your name, so people you know can recognize you")
- else
= form.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9 qa-full-name rspec-full-name' },
help: s_("Profiles|The ability to update your name has been disabled by your administrator.")
---
title: Move 'Allow admins to disable users ability to change profile name' feature to Premium tier
merge_request: 23034
author:
type: changed
......@@ -209,6 +209,7 @@ module EE
expose :file_template_project_id, if: ->(_instance, _opts) { ::License.feature_available?(:custom_file_templates) }
expose :default_project_deletion_protection, if: ->(_instance, _opts) { ::License.feature_available?(:default_project_deletion_protection) }
expose :deletion_adjourned_period, if: ->(_instance, _opts) { ::License.feature_available?(:marking_project_for_deletion) }
expose :updating_name_disabled_for_users, if: ->(_instance, _opts) { ::License.feature_available?(:disable_name_update_for_users) }
end
end
......
......@@ -37,6 +37,7 @@ module EE
optional :file_template_project_id, type: Integer, desc: 'ID of project where instance-level file templates are stored.'
optional :repository_storages, type: Array[String], desc: 'A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random.'
optional :usage_ping_enabled, type: Grape::API::Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
optional :updating_name_disabled_for_users, type: Grape::API::Boolean, desc: 'Flag indicating if users are permitted to update their profile name'
end
end
......
......@@ -31,6 +31,10 @@ module EE
attrs = attrs.except(:deletion_adjourned_period)
end
unless License.feature_available?(:disable_name_update_for_users)
attrs = attrs.except(:updating_name_disabled_for_users)
end
attrs
end
end
......
......@@ -104,6 +104,13 @@ describe Admin::ApplicationSettingsController do
it_behaves_like 'settings for licensed features'
end
context 'updating name disabled for users setting' do
let(:settings) { { updating_name_disabled_for_users: true } }
let(:feature) { :disable_name_update_for_users }
it_behaves_like 'settings for licensed features'
end
context 'project deletion adjourned period' do
let(:settings) { { deletion_adjourned_period: 6 } }
let(:feature) { :marking_project_for_deletion }
......
......@@ -10,6 +10,56 @@ describe Admin::UsersController do
sign_in(admin)
end
describe 'POST update' do
context 'updating name' do
shared_examples_for 'admin can update the name of a user' do
it 'updates the name' do
params = {
id: user.to_param,
user: {
name: 'New Name'
}
}
put :update, params: params
expect(response).to redirect_to(admin_user_path(user))
expect(user.reload.name).to eq('New Name')
end
end
context 'when `disable_name_update_for_users` feature is available' do
before do
stub_licensed_features(disable_name_update_for_users: true)
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
it_behaves_like 'admin can update the name of a user'
end
context 'when the ability to update their name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
end
it_behaves_like 'admin can update the name of a user'
end
end
context 'when `disable_name_update_for_users` feature is not available' do
before do
stub_licensed_features(disable_name_update_for_users: false)
end
it_behaves_like 'admin can update the name of a user'
end
end
end
describe 'POST #reset_runner_minutes' do
subject { post :reset_runners_minutes, params: { id: user } }
......
# frozen_string_literal: true
require('spec_helper')
describe ProfilesController, :request_store do
let_it_be(:user) { create(:user) }
let_it_be(:admin) { create(:admin) }
describe 'PUT update' do
context 'updating name' do
subject { put :update, params: { user: { name: 'New Name' } } }
shared_examples_for 'a user can update their name' do
before do
sign_in(current_user)
end
it 'updates their name' do
subject
expect(response.status).to eq(302)
expect(current_user.reload.name).to eq('New Name')
end
end
context 'when `disable_name_update_for_users` feature is available' do
before do
stub_licensed_features(disable_name_update_for_users: true)
end
context 'when the ability to update thier name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
end
it_behaves_like 'a user can update their name' do
let(:current_user) { user }
end
it_behaves_like 'a user can update their name' do
let(:current_user) { admin }
end
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
context 'as a regular user' do
before do
sign_in(user)
end
it 'does not update their name' do
subject
expect(response.status).to eq(302)
expect(user.reload.name).not_to eq('New Name')
end
end
it_behaves_like 'a user can update their name' do
let(:current_user) { admin }
end
end
end
context 'when `disable_name_update_for_users` feature is not available' do
before do
stub_licensed_features(disable_name_update_for_users: false)
end
it_behaves_like 'a user can update their name' do
let(:current_user) { user }
end
it_behaves_like 'a user can update their name' do
let(:current_user) { admin }
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe UserPolicy do
let(:current_user) { create(:user) }
let(:user) { create(:user) }
subject { described_class.new(current_user, user) }
shared_examples 'changing a user' do |ability|
context 'when a regular user tries to update another regular user' do
it { is_expected.not_to be_allowed(ability) }
end
context 'when a regular user tries to update themselves' do
let(:current_user) { user }
it { is_expected.to be_allowed(ability) }
end
context 'when an admin user tries to update a regular user' do
let(:current_user) { create(:user, :admin) }
it { is_expected.to be_allowed(ability) }
end
context 'when an admin user tries to update a ghost user' do
let(:current_user) { create(:user, :admin) }
let(:user) { create(:user, :ghost) }
it { is_expected.not_to be_allowed(ability) }
end
end
describe "updating a user's name" do
context 'when `disable_name_update_for_users` feature is available' do
before do
stub_licensed_features(disable_name_update_for_users: true)
end
context 'when the ability to update their name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
end
it_behaves_like 'changing a user', :update_name
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
context 'for a regular user' do
it { is_expected.not_to be_allowed(:update_name) }
end
context 'for a ghost user' do
let(:current_user) { create(:user, :ghost) }
it { is_expected.not_to be_allowed(:update_name) }
end
context 'for an admin user' do
let(:current_user) { create(:admin) }
it { is_expected.to be_allowed(:update_name) }
end
end
end
context 'when `disable_name_update_for_users` feature is not available' do
before do
stub_licensed_features(disable_name_update_for_users: false)
end
it_behaves_like 'changing a user', :update_name
end
end
end
......@@ -156,4 +156,11 @@ describe API::Settings, 'EE Settings' do
it_behaves_like 'settings for licensed features'
end
context 'updating name disabled for users' do
let(:settings) { { updating_name_disabled_for_users: true } }
let(:feature) { :disable_name_update_for_users }
it_behaves_like 'settings for licensed features'
end
end
......@@ -6,6 +6,47 @@ describe API::Users do
let(:user) { create(:user) }
let(:admin) { create(:admin) }
context 'updating name' do
shared_examples_for 'admin can update the name of a user' do
it 'updates the user with new name' do
put api("/users/#{user.id}", admin), params: { name: 'New Name' }
expect(response).to have_gitlab_http_status(200)
expect(json_response['name']).to eq('New Name')
end
end
context 'when `disable_name_update_for_users` feature is available' do
before do
stub_licensed_features(disable_name_update_for_users: true)
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
it_behaves_like 'admin can update the name of a user'
end
context 'when the ability to update their name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
end
it_behaves_like 'admin can update the name of a user'
end
end
context 'when `disable_name_update_for_users` feature is not available' do
before do
stub_licensed_features(disable_name_update_for_users: false)
end
it_behaves_like 'admin can update the name of a user'
end
end
context 'extended audit events' do
describe "PUT /users/:id" do
it "creates audit event when updating user with new password" do
......
......@@ -5,6 +5,72 @@ describe Users::UpdateService do
let(:user) { create(:user) }
describe '#execute' do
context 'updating name' do
let(:admin) { create(:admin) }
shared_examples_for 'a user can update the name' do
it 'updates the name' do
result = described_class.new(current_user, { user: user, name: 'New Name' }).execute!
expect(result).to be_truthy
expect(user.name).to eq('New Name')
end
end
context 'when `disable_name_update_for_users` feature is available' do
before do
stub_licensed_features(disable_name_update_for_users: true)
end
context 'when the ability to update their name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
end
it_behaves_like 'a user can update the name' do
let(:current_user) { user }
end
it_behaves_like 'a user can update the name' do
let(:current_user) { admin }
end
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
context 'as a regular user' do
it 'does not update the name' do
result = update_user(user, name: 'New Name')
expect(result).to be_truthy
expect(user.name).not_to eq('New Name')
end
end
it_behaves_like 'a user can update the name' do
let(:current_user) { admin }
end
end
end
context 'when `disable_name_update_for_users` feature is not available' do
before do
stub_licensed_features(disable_name_update_for_users: false)
end
it_behaves_like 'a user can update the name' do
let(:current_user) { user }
end
it_behaves_like 'a user can update the name' do
let(:current_user) { admin }
end
end
end
it 'does not update email if an user has group managed account' do
allow(user).to receive(:group_managed_account?).and_return(true)
......
......@@ -142,7 +142,6 @@ module API
requires :sourcegraph_url, type: String, desc: 'The configured Sourcegraph instance URL'
end
optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
optional :updating_name_disabled_for_users, type: Boolean, desc: 'Flag indicating if users are permitted to update their profile name'
optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins'
optional :local_markdown_version, type: Integer, desc: 'Local markdown version, increase this value when any cached markdown should be invalidated'
......
......@@ -102,13 +102,6 @@ describe Admin::ApplicationSettingsController do
expect(ApplicationSetting.current.minimum_password_length).to eq(10)
end
it 'updates updating_name_disabled_for_users setting' do
put :update, params: { application_setting: { updating_name_disabled_for_users: true } }
expect(response).to redirect_to(admin_application_settings_path)
expect(ApplicationSetting.current.updating_name_disabled_for_users).to eq(true)
end
context 'external policy classification settings' do
let(:settings) do
{
......
......@@ -257,28 +257,6 @@ describe Admin::UsersController do
end
describe 'POST update' do
context 'updating name' do
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
it 'updates the name' do
params = {
id: user.to_param,
user: {
name: 'New Name'
}
}
put :update, params: params
expect(response).to redirect_to(admin_user_path(user))
expect(user.reload.name).to eq('New Name')
end
end
end
context 'when the password has changed' do
def update_password(user, password, password_confirmation = nil)
params = {
......
......@@ -81,54 +81,6 @@ describe ProfilesController, :request_store do
expect(ldap_user.location).to eq('City, Country')
end
context 'updating name' do
subject { put :update, params: { user: { name: 'New Name' } } }
context 'when the ability to update thier name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
sign_in(user)
end
it 'updates the name' do
subject
expect(response.status).to eq(302)
expect(user.reload.name).to eq('New Name')
end
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
context 'as a regular user' do
it 'does not update the name' do
sign_in(user)
subject
expect(response.status).to eq(302)
expect(user.reload.name).not_to eq('New Name')
end
end
context 'as an admin user' do
it 'updates the name' do
admin = create(:admin)
sign_in(admin)
subject
expect(response.status).to eq(302)
expect(admin.reload.name).to eq('New Name')
end
end
end
end
it 'allows setting a user status' do
sign_in(user)
......
......@@ -48,36 +48,4 @@ describe UserPolicy do
describe "updating a user" do
it_behaves_like 'changing a user', :update_user
end
describe "updating a user's name" do
context 'when the ability to update their name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
end
it_behaves_like 'changing a user', :update_name
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
context 'for a regular user' do
it { is_expected.not_to be_allowed(:update_name) }
end
context 'for a ghost user' do
let(:current_user) { create(:user, :ghost) }
it { is_expected.not_to be_allowed(:update_name) }
end
context 'for an admin user' do
let(:current_user) { create(:admin) }
it { is_expected.to be_allowed(:update_name) }
end
end
end
end
......@@ -136,14 +136,6 @@ describe API::Settings, 'Settings' do
expect(json_response['performance_bar_allowed_group_id']).to eq(group.id)
end
it "supports updating_name_disabled_for_users" do
put api("/application/settings", admin),
params: { updating_name_disabled_for_users: true }
expect(response).to have_gitlab_http_status(200)
expect(json_response['updating_name_disabled_for_users']).to eq(true)
end
it "supports legacy performance_bar_enabled" do
put api("/application/settings", admin),
params: {
......
......@@ -645,21 +645,6 @@ describe API::Users do
expect(response).to have_gitlab_http_status(200)
end
context 'updating name' do
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
it 'updates the user with new name' do
put api("/users/#{user.id}", admin), params: { name: 'New Name' }
expect(response).to have_gitlab_http_status(200)
expect(json_response['name']).to eq('New Name')
end
end
end
it "updates user with new bio" do
put api("/users/#{user.id}", admin), params: { bio: 'new test bio' }
......
......@@ -6,46 +6,6 @@ describe Users::UpdateService do
let(:user) { create(:user) }
describe '#execute' do
context 'updating name' do
context 'when the ability to update their name is not disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: false)
end
it 'updates the name' do
result = update_user(user, name: 'New Name')
expect(result).to eq(status: :success)
expect(user.name).to eq('New Name')
end
end
context 'when the ability to update their name is disabled for users' do
before do
stub_application_setting(updating_name_disabled_for_users: true)
end
context 'executing as a regular user' do
it 'does not update the name' do
result = update_user(user, name: 'New Name')
expect(result).to eq(status: :success)
expect(user.name).not_to eq('New Name')
end
end
context 'executing as an admin user' do
it 'updates the name' do
admin = create(:admin)
result = described_class.new(admin, { user: user, name: 'New Name' }).execute
expect(result).to eq(status: :success)
expect(user.name).to eq('New Name')
end
end
end
end
it 'updates time preferences' do
result = update_user(user, timezone: 'Europe/Warsaw', time_display_relative: true, time_format_in_24h: false)
......
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