refactor(saml): harden multi-account user handling (#15395)
Improves SAML user handling for users associated with multiple accounts. Restricts cross-account invitations and skips provider updates for multi-account users. Aligns SAML authentication and provider reset behavior with these eligibility rules.
This commit is contained in:
@@ -1,5 +1,7 @@
|
||||
module Enterprise::AgentBuilder
|
||||
def perform
|
||||
validate_saml_invitation!
|
||||
|
||||
super.tap do |user|
|
||||
convert_to_saml_provider(user) if user.persisted? && account.saml_enabled?
|
||||
end
|
||||
@@ -7,6 +9,17 @@ module Enterprise::AgentBuilder
|
||||
|
||||
private
|
||||
|
||||
def validate_saml_invitation!
|
||||
return unless account.saml_enabled?
|
||||
|
||||
existing_user = User.from_email(email)
|
||||
return unless existing_user
|
||||
return unless existing_user.account_users.where.not(account_id: account.id).exists?
|
||||
|
||||
existing_user.errors.add(:base, I18n.t('errors.saml.multi_account_invitation_not_allowed'))
|
||||
raise ActiveRecord::RecordInvalid, existing_user
|
||||
end
|
||||
|
||||
def convert_to_saml_provider(user)
|
||||
user.update!(provider: 'saml') unless user.provider == 'saml'
|
||||
end
|
||||
|
||||
@@ -19,7 +19,7 @@ class SamlUserBuilder
|
||||
user = User.from_email(auth_attribute('email'))
|
||||
|
||||
return create_user unless user
|
||||
return existing_user_for_account(user) if user_belongs_to_account?(user)
|
||||
return existing_user_for_account(user) if user_belongs_to_account?(user) && !user_has_additional_accounts?(user)
|
||||
|
||||
raise AuthenticationFailed, I18n.t('auth.saml.authentication_failed')
|
||||
end
|
||||
@@ -34,6 +34,10 @@ class SamlUserBuilder
|
||||
user.account_users.exists?(account_id: @account_id)
|
||||
end
|
||||
|
||||
def user_has_additional_accounts?(user)
|
||||
user.account_users.where.not(account_id: @account_id).exists?
|
||||
end
|
||||
|
||||
def confirm_user_if_required(user)
|
||||
return if user.confirmed?
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ class Saml::UpdateAccountUsersProviderJob < ApplicationJob
|
||||
# This job is triggered when SAML settings are created or destroyed
|
||||
def perform(account_id, provider)
|
||||
account = Account.find(account_id)
|
||||
account.users.find_each(batch_size: 1000) do |user|
|
||||
users_for_provider_update(account, provider).find_each(batch_size: 1000) do |user|
|
||||
next unless should_update_user_provider?(user, provider)
|
||||
|
||||
# rubocop:disable Rails/SkipsModelValidations
|
||||
@@ -25,6 +25,13 @@ class Saml::UpdateAccountUsersProviderJob < ApplicationJob
|
||||
true
|
||||
end
|
||||
|
||||
def users_for_provider_update(account, provider)
|
||||
return account.users.where.not(id: AccountUser.where.not(account_id: account.id).select(:user_id)) if provider == 'saml'
|
||||
return account.users.where(provider: 'saml') if provider == 'email'
|
||||
|
||||
account.users
|
||||
end
|
||||
|
||||
# Checks if the user belongs to any other accounts that have SAML configured
|
||||
# Used to preserve SAML authentication when one account disables SAML but others still use it
|
||||
def user_has_other_saml_accounts?(user)
|
||||
|
||||
Reference in New Issue
Block a user