Migre les rôles vers une table dédiée user_roles

app/controllers/admin/users_with_roles_controller.rb

@@ -1,6 +1,6 @@
 class Admin::UsersWithRolesController < AdminController
   def index
-    @search_engine = User.with_roles.includes(:organizations).ransack(params[:search_query])
+    @search_engine = User.with_roles.includes(:organizations, :user_roles).ransack(params[:search_query])
     @search_engine.sorts = 'created_at desc' if @search_engine.sorts.empty?
 
     @users = @search_engine.result(distinct: true).page(params[:page]).per(50)

app/interactors/admin/remove_habilitation_type_roles.rb

@@ -1,12 +1,5 @@
 class Admin::RemoveHabilitationTypeRoles < ApplicationInteractor
   def call
-    uid = context.habilitation_type.uid
-    fd_slug = context.habilitation_type.data_provider.slug
-
-    exact_roles = User::ROLES.map { |role_type| "#{fd_slug}:#{uid}:#{role_type}" }
-    User.with_role_matching(exact_roles).find_each do |user|
-      user.roles.reject! { |r| ParsedRole.parse(r).definition_id == uid }
-      user.save!
-    end
+    UserRole.where(authorization_definition_id: context.habilitation_type.uid).destroy_all
   end
 end

app/interactors/admin/update_user_roles_attribute.rb

@@ -1,14 +1,47 @@
 class Admin::UpdateUserRolesAttribute < ApplicationInteractor
   def call
-    user.roles = valid_roles
-    user.roles.uniq!
-    user.save
+    remove_obsolete_roles
+    add_missing_roles
+    user.user_roles.reset
+  end
+
+  def remove_obsolete_roles
+    user.user_roles.reload.reject { |ur| desired_attrs.any? { |d| role_matches?(ur, d) } }.each(&:destroy!)
+  end
+
+  def add_missing_roles
+    desired_attrs.reject { |d| user.user_roles.reload.any? { |ur| role_matches?(ur, d) } }
+      .each { |attrs| user.user_roles.create!(attrs) }
   end
 
   private
 
-  def valid_roles
-    context.roles.select { |role| ParsedRole.valid?(role) }
+  def desired_attrs
+    @desired_attrs ||= valid_role_attrs
+  end
+
+  def valid_role_attrs
+    context.roles.select { |r| ParsedRole.valid?(r) }.filter_map { |r| build_attrs(r) }
+  end
+
+  def build_attrs(role_string)
+    parsed = ParsedRole.parse(role_string)
+    return { role: 'admin' } if parsed.admin?
+
+    dp = DataProvider.find_by(slug: parsed.provider_slug)
+
+    {
+      role: parsed.role,
+      data_provider: dp,
+      data_provider_slug: parsed.provider_slug,
+      authorization_definition_id: parsed.fd_level? ? nil : parsed.definition_id,
+    }
+  end
+
+  def role_matches?(user_role, attrs)
+    user_role.role == attrs[:role] &&
+      user_role.data_provider_slug == attrs[:data_provider_slug] &&
+      user_role.authorization_definition_id == attrs[:authorization_definition_id]
   end
 
   def user

app/lib/seeds.rb

@@ -175,9 +175,11 @@ def api_entreprise_instructor
       email: '[email protected]',
       external_id: '4',
       job_title: 'Responsable des instructions',
-      phone_number: '0423456789',
-      roles: ['dinum:api_entreprise:instructor', 'dinum:api_entreprise:developer']
-    )
+      phone_number: '0423456789'
+    ).tap do |user|
+      user.grant_role(:instructor, 'api_entreprise')
+      user.grant_role(:developer, 'api_entreprise')
+    end
   end
 
   def api_entreprise_reporter
@@ -187,30 +189,27 @@ def api_entreprise_reporter
       email: '[email protected]',
       external_id: '12',
       job_title: 'Responsable des reporteurs',
-      phone_number: '0423456789',
-      roles: ['dinum:api_entreprise:reporter']
-    )
+      phone_number: '0423456789'
+    ).tap do |user|
+      user.grant_role(:reporter, 'api_entreprise')
+    end
   end
 
   def data_pass_admin
-    @data_pass_admin ||= User.create!(
-      email: '[email protected]',
-      roles: ['admin'] + all_authorization_definition_manager_roles + ['dinum:api_entreprise:developer', 'dinum:api_particulier:developer'],
-    )
+    @data_pass_admin ||= User.create!(email: '[email protected]').tap do |user|
+      user.grant_admin_role
+      AuthorizationDefinition.all.each do |definition|
+        user.grant_role(:manager, definition.id) if definition.provider_slug
+      end
+      user.grant_role(:developer, 'api_entreprise')
+      user.grant_role(:developer, 'api_particulier')
+    end
   end
 
   def dgfip_instructor_developer
-    @dgfip_instructor_developer ||= User.create!(
-      email: '[email protected]',
-      roles: %w[dgfip:*:instructor dgfip:*:developer]
-    )
-  end
-
-  def all_authorization_definition_manager_roles
-    AuthorizationDefinition.all.filter_map do |definition|
-      next unless definition.provider_slug
-
-      "#{definition.provider_slug}:#{definition.id}:manager"
+    @dgfip_instructor_developer ||= User.create!(email: '[email protected]').tap do |user|
+      user.grant_fd_role(:instructor, 'dgfip')
+      user.grant_fd_role(:developer, 'dgfip')
     end
   end
 

app/models/data_provider.rb

@@ -17,6 +17,7 @@ class DataProvider < ApplicationRecord
   validates :link, presence: true, format: { with: URL_REGEX, message: I18n.t('activemodel.errors.messages.url_format') }
   validates :logo, content_type: %w[image/png image/jpeg image/svg+xml], if: -> { logo.attached? }
 
+  before_destroy :cleanup_user_roles
   after_save :reset_static_caches
 
   def linked_habilitation_types?
@@ -66,6 +67,12 @@ def set_slug_from_name
   end
 
   def users_for_roles(roles)
-    User.with_role_for_provider(slug, roles)
+    User.joins(:user_roles).merge(
+      UserRole.for_roles(roles).for_provider(slug)
+    ).distinct
+  end
+
+  def cleanup_user_roles
+    UserRole.where(data_provider_id: id).destroy_all
   end
 end

app/models/role_set.rb

@@ -1,34 +1,20 @@
 class RoleSet
-  def initialize(roles_array, kind)
-    qualifying = RoleHierarchy.qualifying_roles(kind)
-    @roles = roles_array.filter_map do |r|
-      parsed = ParsedRole.parse(r)
-      parsed if qualifying.include?(parsed.role)
-    end
+  def initialize(user_roles_relation, kind)
+    @roles = user_roles_relation.effective_for_role(kind)
   end
 
   def covers?(definition_id = nil)
-    return @roles.any? unless definition_id
+    return @roles.exists? unless definition_id
 
-    fd_slug = ParsedRole.resolve_provider_slug(definition_id)
-
-    @roles.any? do |parsed|
-      parsed.definition_id == definition_id || (parsed.fd_level? && parsed.provider_slug == fd_slug)
-    end
+    @roles.effective_for_definition(definition_id).exists?
   end
 
-  delegate :any?, to: :@roles
+  def any?
+    @roles.exists?
+  end
 
   def definition_ids
-    @definition_ids ||= @roles.flat_map { |parsed|
-      if parsed.fd_level?
-        AuthorizationDefinition.all
-          .select { |ad| ad.provider_slug == parsed.provider_slug }
-          .map(&:id)
-      else
-        [parsed.definition_id]
-      end
-    }.uniq
+    @definition_ids ||= @roles.flat_map(&:covered_definition_ids).compact.uniq
   end
 
   def authorization_request_types

app/models/user.rb

@@ -55,39 +55,20 @@ def current_identity_provider
     inverse_of: :admin,
     dependent: :restrict_with_exception
 
-  scope :with_roles, -> { where("roles <> '{}'") }
-  scope :banned, -> { where.not(banned_at: nil) }
+  has_many :user_roles, dependent: :destroy
 
-  scope :with_role_matching, lambda { |role_strings|
-    where(
-      'EXISTS (SELECT 1 FROM unnest(roles) AS r WHERE r IN (?))',
-      role_strings
-    )
-  }
-
-  scope :with_role_for_definition, lambda { |definition_id, kind|
-    fd_slug = ParsedRole.resolve_provider_slug(definition_id)
-    qualifying = RoleHierarchy.qualifying_roles(kind)
-
-    with_role_matching(
-      qualifying.flat_map { |role_type| ["#{fd_slug}:#{definition_id}:#{role_type}", "#{fd_slug}:*:#{role_type}"] }
-    )
-  }
-
-  scope :with_role_for_provider, lambda { |provider_slug, roles|
-    where(
-      'EXISTS (SELECT 1 FROM unnest(roles) AS r WHERE r LIKE ANY(ARRAY[?]))',
-      roles.map { |role| "#{provider_slug}:%:#{role}" }
-    )
-  }
+  scope :with_roles, -> { where(id: UserRole.select(:user_id)) }
+  scope :banned, -> { where.not(banned_at: nil) }
 
   %i[instructor developer manager reporter].each do |role|
-    scope :"#{role}_for", ->(type) { with_role_for_definition(type.underscore, role) }
+    scope :"#{role}_for", lambda { |type|
+      joins(:user_roles).merge(
+        UserRole.effective_for_role(role).effective_for_definition(type.underscore)
+      ).distinct
+    }
   end
 
-  scope :admin, lambda {
-    where("'admin' = ANY(roles)")
-  }
+  scope :admin, -> { joins(:user_roles).merge(UserRole.admin_role).distinct }
 
   add_instruction_boolean_settings :submit_notifications, :messages_notifications
 
@@ -121,7 +102,7 @@ def full_name
 
   def roles_for(kind)
     @role_sets ||= {}
-    @role_sets[kind] ||= RoleSet.new(roles, kind)
+    @role_sets[kind] ||= RoleSet.new(user_roles, kind)
   end
 
   def instructor?(definition_id = nil)
@@ -150,34 +131,37 @@ def authorization_request_types_for(kind)
     roles_for(kind).authorization_request_types
   end
 
+  def authorization_definition_roles_as(kind)
+    roles_for(kind).authorization_definitions
+  end
+
   def grant_role(kind, definition_id)
     fd = ParsedRole.resolve_provider_slug(definition_id)
     raise ParsedRole::UnknownDefinitionError, "Unknown definition: #{definition_id}" unless fd
 
-    roles << "#{fd}:#{definition_id}:#{kind}"
-    roles.uniq!
+    dp = DataProvider.find_by(slug: fd)
+    user_roles.find_or_create_by!(role: kind.to_s, data_provider: dp, data_provider_slug: fd, authorization_definition_id: definition_id)
     @role_sets = nil
   end
 
   def grant_fd_role(kind, provider_slug)
-    roles << "#{provider_slug}:*:#{kind}"
-    roles.uniq!
+    dp = DataProvider.find_by(slug: provider_slug)
+    user_roles.find_or_create_by!(role: kind.to_s, data_provider: dp, data_provider_slug: provider_slug, authorization_definition_id: nil)
     @role_sets = nil
   end
 
   def grant_admin_role
-    roles << 'admin'
-    roles.uniq!
+    user_roles.find_or_create_by!(role: 'admin')
     @role_sets = nil
   end
 
   def revoke_all_roles
-    self.roles = []
+    user_roles.destroy_all
     @role_sets = nil
   end
 
   def admin?
-    roles.include?('admin') ||
+    user_roles.admin_role.exists? ||
       bug_bounty_users_within_staging_env?
   end
 
@@ -186,8 +170,20 @@ def bug_bounty_users_within_staging_env?
       /[email protected]$/.match?(email)
   end
 
-  def authorization_definition_roles_as(kind)
-    roles_for(kind).authorization_definitions
+  def roles_as_strings
+    user_roles.map do |ur|
+      if ur.admin?
+        'admin'
+      elsif ur.fd_level?
+        "#{ur.data_provider_slug}:*:#{ur.role}"
+      else
+        "#{ur.data_provider_slug}:#{ur.authorization_definition_id}:#{ur.role}"
+      end
+    end
+  end
+
+  def roles
+    roles_as_strings
   end
 
   def self.ransackable_attributes(_auth_object = nil)
@@ -213,9 +209,9 @@ def self.api_role_ransacker_sql
     <<~SQL.squish
       COALESCE(
         (SELECT string_agg(DISTINCT def_id, ',') FROM (
-          SELECT split_part(elem, ':', 2) AS def_id
-          FROM unnest(users.roles) AS elem
-          WHERE elem ~ '^[^:]+:[^:]+:[^:]+$' AND split_part(elem, ':', 2) <> '*'
+          SELECT ur.authorization_definition_id AS def_id
+          FROM user_roles ur
+          WHERE ur.user_id = users.id AND ur.authorization_definition_id IS NOT NULL
           #{api_role_fd_expansion_sql}
         ) expanded),
         ''
@@ -234,10 +230,10 @@ def self.api_role_fd_expansion_sql
     <<~SQL.squish
       UNION
       SELECT ad_map.def_id
-      FROM unnest(users.roles) AS elem
+      FROM user_roles ur
       JOIN (VALUES #{values}) AS ad_map(def_id, provider_slug)
-        ON ad_map.provider_slug = split_part(elem, ':', 1)
-      WHERE split_part(elem, ':', 2) = '*'
+        ON ad_map.provider_slug = ur.data_provider_slug
+      WHERE ur.user_id = users.id AND ur.authorization_definition_id IS NULL
     SQL
   end