Browse Source

CLDC-4332: Check that a support user can only be in certain orgs (#3305)

* CLDC-4332: Check that a support user can only be in certain orgs

* CLDC-4332: Add tests

* CLDC-4332: Lint

* CLDC-4332: Only show the check for wrong org on edit

* CLDC-4332: Add the error to role if role is wrong

* fixup! CLDC-4332: Only show the check for wrong org on edit

lint

* CLDC-4332: TEMP: Add MHCLG filter for review apps

* Revert "CLDC-4332: TEMP: Add MHCLG filter for review apps"

This reverts commit b65c630ede.
pull/3303/head^2 v0.6.10
Samuel Young 2 days ago committed by GitHub
parent
commit
e079455f65
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 10
      app/controllers/users_controller.rb
  2. 17
      app/models/user.rb
  3. 6
      app/services/feature_toggle.rb
  4. 4
      config/locales/en.yml
  5. 47
      spec/models/user_spec.rb

10
app/controllers/users_controller.rb

@ -221,8 +221,14 @@ private
@user.errors.add :phone
end
if user_params.key?(:organisation_id) && user_params[:organisation_id].blank?
@user.errors.add :organisation_id, :blank
if user_params.key?(:organisation_id)
if user_params[:organisation_id].blank?
@user.errors.add :organisation_id, :blank
elsif !@user.role_is_allowed_to_be_in_organisation?(override_organisation_id: user_params[:organisation_id].to_i) && @user.id.present?
# this will also be flagged by the validation in user.rb.
# for convenience we show the error early before they go through the change org flow (involves reassigning logs).
@user.errors.add :organisation_id, I18n.t("validations.user.support_user_in_wrong_organisation.change_organisation")
end
end
end

17
app/models/user.rb

@ -25,6 +25,7 @@ class User < ApplicationRecord
validates :organisation_id, presence: true
validate :organisation_not_merged
validate :support_user_is_in_correct_organisation
has_paper_trail ignore: %w[last_sign_in_at
current_sign_in_at
@ -390,6 +391,12 @@ class User < ApplicationRecord
end
end
def role_is_allowed_to_be_in_organisation?(override_organisation_id: nil)
return true unless support? && FeatureToggle.support_organisation_allow_list.present?
FeatureToggle.support_organisation_allow_list.include?(override_organisation_id || organisation_id)
end
protected
# Checks whether a password is needed or not. For validations only.
@ -407,6 +414,16 @@ private
end
end
def support_user_is_in_correct_organisation
return if role_is_allowed_to_be_in_organisation?
if role_changed?
errors.add :role, I18n.t("validations.user.support_user_in_wrong_organisation.change_role")
else
errors.add :organisation_id, I18n.t("validations.user.support_user_in_wrong_organisation.change_organisation")
end
end
def send_data_protection_confirmation_reminder
return unless persisted?
return unless is_dpo?

6
app/services/feature_toggle.rb

@ -34,4 +34,10 @@ class FeatureToggle
def self.sales_export_enabled?
Time.zone.now >= Time.zone.local(2025, 4, 1) || (Rails.env.review? || Rails.env.staging?)
end
# IDs of organisations a user must be in to be allowed the support role
# if nil this feature will be disabled
def self.support_organisation_allow_list
[1] if Rails.env.production?
end
end

4
config/locales/en.yml

@ -260,6 +260,10 @@ en:
blank: "Enter an email address."
role:
invalid: "Role must be data accessor, data provider or data coordinator."
user:
support_user_in_wrong_organisation:
change_role: "You cannot create a support account type for a user in this organisation. Support accounts should only be created for MHCLG and contractor staff as they are administrator level accounts with access to all organisations' data. Any support accounts for housing organisations would be a data protection breach."
change_organisation: "You cannot move a user with a support account to a non-MHCLG organisation. If you need to move the user, change their role type to data coordinator or data provider."
setup:
saledate:

47
spec/models/user_spec.rb

@ -540,6 +540,53 @@ RSpec.describe User, type: :model do
.to raise_error(ActiveRecord::RecordInvalid, error_message)
end
end
describe "#support_user_is_in_correct_organisation" do
let(:organisation) { create(:organisation) }
context "when the user is not a support user" do
let(:user) { build(:user, :data_coordinator, organisation:) }
it "is valid regardless of the allow list" do
allow(FeatureToggle).to receive(:support_organisation_allow_list).and_return([999])
expect(user).to be_valid
end
end
context "when the user is a support user" do
let(:user) { build(:user, :support, organisation:) }
context "and the allow list is nil" do
before do
allow(FeatureToggle).to receive(:support_organisation_allow_list).and_return(nil)
end
it "is valid" do
expect(user).to be_valid
end
end
context "and the organisation is in the allow list" do
before do
allow(FeatureToggle).to receive(:support_organisation_allow_list).and_return([organisation.id])
end
it "is valid" do
expect(user).to be_valid
end
end
context "and the organisation is not in the allow list" do
before do
allow(FeatureToggle).to receive(:support_organisation_allow_list).and_return([organisation.id + 1])
end
it "is not valid" do
expect(user).not_to be_valid
end
end
end
end
end
describe "delete" do

Loading…
Cancel
Save