Commit e213d115 authored by manojmj's avatar manojmj

Move to EE Premium: Allow admins to disable users ability to change name

This change moves the feature
“Allow admins to disable users ability to change name”
to EE Premium and above
parent a38b6ab6
......@@ -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
......
......@@ -65,6 +65,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