Store selected user sync roles as custom attributes.

Selected roles are saved as provider custom attributes.

If roles are changed between different syncs, user memberships from
previously selected roles should be removed.

app/models/manageiq/providers/openstack/identity_sync_mixin.rb

@@ -1,4 +1,9 @@
 module ManageIQ::Providers::Openstack::IdentitySyncMixin
+  IDENTITY_SYNC_ADMIN_ROLE_ID = "identity_sync_admin_role_id".freeze
+  IDENTITY_SYNC_MEMBER_ROLE_ID = "identity_sync_member_role_id".freeze
+  IDENTITY_SYNC_ADMIN_ROLE_ID_NEW = "identity_sync_admin_role_id_new".freeze
+  IDENTITY_SYNC_MEMBER_ROLE_ID_NEW = "identity_sync_member_role_id_new".freeze
+
   def list_users
     # V2 requires adminURL as endpoint_type
     connection_options = {:service => "Identity", :openstack_endpoint_type => 'adminURL'}
@@ -39,12 +44,16 @@ def sync_users_queue(userid, admin_role_id, member_role_id, password_digest)
   end
 
   def sync_users(admin_role_id, member_role_id, password_digest)
+    ext_management_system.miq_custom_set(IDENTITY_SYNC_ADMIN_ROLE_ID_NEW, admin_role_id)
+    ext_management_system.miq_custom_set(IDENTITY_SYNC_MEMBER_ROLE_ID_NEW, member_role_id)
     myusers = list_users
+    _log.info("list_users: #{myusers}")
     myusers.each do |u|
       email = u["email"]
       username = u["name"]
       user_uuid = u["id"]
       enabled = u["enabled"]
+      _log.info("user: #{username}")
 
       next if skip_user?(username) || enabled == false
 
@@ -53,26 +62,42 @@ def sync_users(admin_role_id, member_role_id, password_digest)
       # in this case we don't do anything
       next if user.nil?
 
-      user_projects = keystone.list_user_projects_tenants(user_uuid)
-      user_projects.each do |p|
-        project_name = p["name"]
-        project_uuid = p["id"]
-
-        # skip service tenant
-        next if project_name == "service"
-
-        cloud_tenant = CloudTenant.find_by(:name => project_name, :ems_id => id)
-        next if cloud_tenant.nil?
-        tenant = Tenant.find_by(:source_id => cloud_tenant.id, :source_type => 'CloudTenant')
-        next if tenant.nil?
-
-        # Find roles that this user has for this project/tenant
-        roles = keystone.list_project_tenant_user_roles(project_uuid, user_uuid)
-        roles.each do |r|
-          role_name = r["name"]
-          create_or_find_miq_group_and_add_user(user, tenant, role_name, admin_role_id, member_role_id)
-        end
-      end
+      sync_user_projects_and_roles(user, user_uuid)
+    end
+    ext_management_system.miq_custom_set(IDENTITY_SYNC_ADMIN_ROLE_ID, admin_role_id)
+    ext_management_system.miq_custom_set(IDENTITY_SYNC_MEMBER_ROLE_ID, member_role_id)
+  end
+
+  def validate_and_sync_user_roles(project, user, user_uuid)
+    project_name = project["name"]
+    project_uuid = project["id"]
+    _log.info("project: #{project_name}")
+
+    # skip service tenant
+    return if project_name == "service"
+
+    cloud_tenant = CloudTenant.find_by(:name => project_name, :ems_id => id)
+    return if cloud_tenant.nil?
+    tenant = Tenant.find_by(:source_id => cloud_tenant.id, :source_type => 'CloudTenant')
+    return if tenant.nil?
+
+    sync_user_roles(user, user_uuid, tenant, project_uuid)
+  end
+
+  def sync_user_projects_and_roles(user, user_uuid)
+    user_projects = keystone.list_user_projects_tenants(user_uuid)
+    user_projects.each do |p|
+      validate_and_sync_user_roles(p, user, user_uuid)
+    end
+  end
+
+  def sync_user_roles(user, user_uuid, tenant, project_uuid)
+    # Find roles that this user has for this project/tenant
+    roles = keystone.list_project_tenant_user_roles(project_uuid, user_uuid)
+    _log.info("roles: #{roles}")
+    roles.each do |r|
+      role_name = r["name"]
+      create_or_find_miq_group_and_add_user(user, tenant, role_name)
     end
   end
 
@@ -114,24 +139,54 @@ def create_or_find_user(openstack_uuid, username, email, password_digest)
     user
   end
 
-  def create_or_find_miq_group_and_add_user(user, tenant, role_name, admin_role_id, member_role_id)
+  def remove_user_from_group(user, tenant, role_id)
+    miq_group = MiqGroup.joins(:entitlement).where(:tenant_id => tenant.id).where('entitlements.miq_user_role_id' => role_id).take
+    _log.info("removing user from group: #{miq_group.name}")
+    miq_group&.users&.delete(user)
+    miq_group&.save!
+    if user.current_group == miq_group
+      _log.info("setting current_group to nil: #{user.current_group.name}")
+      user.current_group = nil
+      user.save!
+      _log.info("current_group after save: #{user.current_group}")
+    end
+  end
+
+  def remove_from_previous_role_if_role_has_changed(existing_role_id, selected_role_id, user, tenant)
+    existing_role_id = ext_management_system.miq_custom_get(existing_role_id)
+    _log.info("existing role id: #{existing_role_id}") if existing_role_id
+    unless existing_role_id.to_s == selected_role_id.to_s
+      remove_user_from_group(user, tenant, existing_role_id) unless existing_role_id.nil?
+    end
+  end
+
+  def create_or_find_miq_group_and_add_user(user, tenant, role_name)
     # Find MiqGroup corresponding to this role and project/tenant
     # create one if it doesn't exist
     # add user to the MiqGroup
+    admin_role_id = ext_management_system.miq_custom_get(IDENTITY_SYNC_ADMIN_ROLE_ID_NEW)
     admin_role = MiqUserRole.find(admin_role_id)
+    member_role_id = ext_management_system.miq_custom_get(IDENTITY_SYNC_MEMBER_ROLE_ID_NEW)
     user_role = MiqUserRole.find(member_role_id)
 
     this_role = nil
     if role_name == "admin"
       this_role = admin_role
+      _log.info("selected admin_role: #{admin_role.name} id: #{admin_role.id}")
+      remove_from_previous_role_if_role_has_changed(IDENTITY_SYNC_ADMIN_ROLE_ID, admin_role_id, user, tenant)
     elsif role_name == "_member_"
       this_role = user_role
+      _log.info("new member_role: #{user_role.name} id: #{user_role.id}")
+      remove_from_previous_role_if_role_has_changed(IDENTITY_SYNC_MEMBER_ROLE_ID, member_role_id, user, tenant)
     else
       return
     end
 
+    _log.info("this_role: #{this_role.name} id: #{this_role.id}")
+
     if this_role
       miq_group = MiqGroup.joins(:entitlement).where(:tenant_id => tenant.id).where('entitlements.miq_user_role_id' => this_role.id).take
+      _log.info("existing group id: #{miq_group.id} name: #{miq_group.name}") if miq_group
       if miq_group.nil?
         miq_group = MiqGroup.new
         miq_group.tenant = tenant
@@ -144,15 +199,18 @@ def create_or_find_miq_group_and_add_user(user, tenant, role_name, admin_role_id
         miq_group.entitlement = entitlement
         miq_group.description = create_group_name(ext_management_system, tenant, this_role)
         miq_group.save!
+        _log.info("new group id: #{miq_group.id} name: #{miq_group.name}")
       end
 
       unless miq_group.users.include?(user)
         miq_group.users << user
         miq_group.save!
       end
       unless user.current_group
+        _log.info("setting current_group to: #{miq_group.name} from: #{user.current_group}")
         user.current_group = miq_group
         user.save!
+        _log.info("current_group after save: #{user.current_group.name}")
       end
       miq_group
     end

spec/models/manageiq/providers/openstack/identity_sync_mixin_spec.rb

@@ -41,7 +41,25 @@
       ems.sync_users(admin_role.id, member_role.id, "changeme")
       expect(User.count).to eq(user_count)
       user = User.find_by(:userid => "project1-admin")
+      expect(user.current_group.miq_user_role).to eq(admin_role)
+      expect(user.current_group.tenant).to eq(tenant)
+      expect(user.miq_groups.count).to eq(2)
+
+      # switch roles in sync should unmap user from old groups and roles
+      user = User.find_by(:userid => "project1-admin")
+      expect(user.current_group.miq_user_role.id).to eq(admin_role.id)
+      new_admin_role = FactoryGirl.create(:miq_user_role, :name => "EvmRole-operator")
+      new_member_role = FactoryGirl.create(:miq_user_role, :name => "EvmRole-vm_user")
+      ems.sync_users(new_admin_role.id, new_member_role.id, "changeme")
+      user = User.find_by(:userid => "project1-admin")
       expect(user.miq_groups.count).to eq(2)
+      user.miq_groups.each do |group|
+        expect(group.miq_user_role).not_to eq(admin_role)
+        expect(group.miq_user_role).not_to eq(member_role)
+      end
+      expect(user.current_group.miq_user_role.id.to_s).not_to eq(admin_role.id.to_s)
+      expect(user.current_group.miq_user_role.id.to_s).not_to eq(member_role.id.to_s)
+      expect(user.current_group.miq_user_role.id.to_s).to eq(new_admin_role.id.to_s)
     end
 
     it "should create realuser, but skip admin and other special cases" do
@@ -93,7 +111,9 @@
       # admin
       miq_group = MiqGroup.joins(:entitlement).where(:tenant_id => tenant.id).where('entitlements.miq_user_role_id' => admin_role.id).take
       expect(miq_group).to be_nil
-      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "admin", admin_role.id, member_role.id)
+      ems.miq_custom_set(ManageIQ::Providers::Openstack::IdentitySyncMixin::IDENTITY_SYNC_ADMIN_ROLE_ID_NEW, admin_role.id)
+      ems.miq_custom_set(ManageIQ::Providers::Openstack::IdentitySyncMixin::IDENTITY_SYNC_MEMBER_ROLE_ID_NEW, member_role.id)
+      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "admin")
 
       expect(miq_group.tenant).to eq(tenant)
       expect(miq_group.entitlement.miq_user_role).to eq(admin_role)
@@ -102,7 +122,7 @@
       # member
       miq_group = MiqGroup.joins(:entitlement).where(:tenant_id => tenant.id).where('entitlements.miq_user_role_id' => member_role.id).take
       expect(miq_group).to be_nil
-      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "_member_", admin_role.id, member_role.id)
+      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "_member_")
 
       expect(miq_group.tenant).to eq(tenant)
       expect(miq_group.entitlement.miq_user_role).to eq(member_role)
@@ -115,7 +135,9 @@
       tenant = FactoryGirl.create(:tenant, :name => "project1")
       admin_role = FactoryGirl.create(:miq_user_role, :name => "EvmRole-tenant_administrator")
       member_role = FactoryGirl.create(:miq_user_role, :name => "EvmRole-user")
-      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "admin", admin_role.id, member_role.id)
+      ems.miq_custom_set(ManageIQ::Providers::Openstack::IdentitySyncMixin::IDENTITY_SYNC_ADMIN_ROLE_ID_NEW, admin_role.id)
+      ems.miq_custom_set(ManageIQ::Providers::Openstack::IdentitySyncMixin::IDENTITY_SYNC_MEMBER_ROLE_ID_NEW, member_role.id)
+      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "admin")
       expect(miq_group.name).to eq("#{ems.name}-#{ems.keystone_v3_domain_id}-#{tenant.name}-#{admin_role.name}")
     end
 
@@ -124,7 +146,9 @@
       tenant = FactoryGirl.create(:tenant, :name => "project1")
       admin_role = FactoryGirl.create(:miq_user_role, :name => "EvmRole-tenant_administrator")
       member_role = FactoryGirl.create(:miq_user_role, :name => "EvmRole-user")
-      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "admin", admin_role.id, member_role.id)
+      ems.miq_custom_set(ManageIQ::Providers::Openstack::IdentitySyncMixin::IDENTITY_SYNC_ADMIN_ROLE_ID_NEW, admin_role.id)
+      ems.miq_custom_set(ManageIQ::Providers::Openstack::IdentitySyncMixin::IDENTITY_SYNC_MEMBER_ROLE_ID_NEW, member_role.id)
+      miq_group = ems.create_or_find_miq_group_and_add_user(user, tenant, "admin")
       expect(miq_group.name).to eq("#{ems.name}-#{tenant.name}-#{admin_role.name}")
     end