Convert users.managing_group_id to loose foreign key
What does this MR do and why?
Related issue: #409837 (closed)
- What happens currently ?
When a group is deleted, a foreign key cascade nullifies managing_group_id
on users
table
- What happens now ?
When a group is deleted, we queue in loose_foreign_keys a job to eventually nullify managing_group_id
on users
table
- Will things be OK while we are in the queue ?
Based on code present in ee/app/model/ee/user.rb
, it should be OK.
-
user.managing_group
will returnnil
but all calls are guarded byuser.group_managed_account?
. For example, seeee/app/views/profiles/notifications/_email_settings.html.haml
.- One place needs to be updated in
ee/app/workers/personal_access_tokens/instance/policy_worker.rb
to handle this use case
- One place needs to be updated in
- For group, well, it's deleted :)
Application code where managing_group
is used:
ee/app/helpers/ee/personal_access_tokens_helper.rb: current_user.managing_group.max_personal_access_token_lifetime_from_now
ee/app/models/ee/group.rb: has_many :managed_users, class_name: 'User', foreign_key: 'managing_group_id', inverse_of: :managing_group
ee/app/models/ee/identity.rb: validate :validate_managing_group
ee/app/models/ee/identity.rb: def validate_managing_group
ee/app/models/ee/identity.rb: errors.add(:base, _('Group requires separate account')) if saml_provider.group != user.managing_group
ee/app/models/ee/personal_access_token.rb: user.managing_group.max_personal_access_token_lifetime_from_now
ee/app/models/ee/user.rb: belongs_to :managing_group, class_name: 'Group', optional: true, inverse_of: :managed_users
ee/app/models/ee/user.rb: scope = where(managing_group_id: nil)
ee/app/models/ee/user.rb: scope = scope.or(where.not(managing_group_id: group.id)) if group
ee/app/models/ee/user.rb: scope :managed_by, ->(group) { where(managing_group: group) }
ee/app/models/ee/user.rb: managing_group.present?
ee/app/models/ee/user.rb: self.group_managed_account? && self.managing_group.owned_by?(user)
ee/app/services/ee/personal_access_tokens/revoke_service.rb: token.user&.managing_group == group &&
ee/app/services/group_saml/group_managed_accounts/transfer_membership_service.rb: current_user.managing_group = group
ee/app/services/group_saml/sign_up_service.rb: new_user.managing_group = group if group.saml_provider&.enforced_group_managed_accounts?
ee/app/views/profiles/notifications/_email_settings.html.haml:- 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
ee/lib/gitlab/auth/group_saml/gma_membership_enforcer.rb: root_ancestor == user.managing_group
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Thong Kuah