Skip to content

Commit

Permalink
Fix incorrectly tenant default groups
Browse files Browse the repository at this point in the history
When creating a tenant, if there are already groups in the database,
it would assign a random group.
This would manifest by having a seemingly random group assigned to newly
discovered VMs/Hosts. There can then be privilege issues as the admins
possibly don't have access to the group.

This only happens for customers who had a database before tenancy
was introduced in 2015. It incorrectly assigned during the migration
process in 20151021174140_assign_tenant_default_group

This also will probably only happen for the default tenant.

The fix: When a tenant points to a group that is not a tenant group,
change the default group to point to a new tenant group.
The old group was an incorrect group.

https://bugzilla.redhat.com/show_bug.cgi?id=1625788

I also changed the MiqGroup code to choose the correct group.
  • Loading branch information
kbrock committed Sep 28, 2018
1 parent 621e5eb commit fb30ad6
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 0 deletions.
57 changes: 57 additions & 0 deletions db/migrate/20180926152238_fix_default_tenant_group.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
class FixDefaultTenantGroup < ActiveRecord::Migration[5.0]
class Tenant < ActiveRecord::Base
belongs_to :default_miq_group, :class_name => "::FixDefaultTenantGroup::MiqGroup"

def add_default_miq_group
tenant_group = ::FixDefaultTenantGroup::MiqGroup.create_tenant_group(self)
update_attributes!(:default_miq_group_id => tenant_group.id)
end

def root?
ancestry.nil?
end
end

class MiqUserRole < ActiveRecord::Base
DEFAULT_TENANT_ROLE_NAME = "EvmRole-tenant_administrator".freeze

# if there is no role, that is ok
# MiqGroup.seed will populate

def self.default_tenant_role
@default_role ||= find_by(:name => DEFAULT_TENANT_ROLE_NAME)
end
end

class MiqGroup < ActiveRecord::Base
TENANT_GROUP = "tenant".freeze
USER_GROUP = "user".freeze

def tenant_group?
group_type == TENANT_GROUP
end

def self.create_tenant_group(tenant)
role = ::FixDefaultTenantGroup::MiqUserRole.default_tenant_role
create_with(
:description => "Tenant #{tenant.name} #{tenant.id} access",
:sequence => 1,
:guid => SecureRandom.uuid,
:miq_user_role_id => role.try(:id)
).find_or_create_by!(
:tenant_id => tenant.id,
:group_type => TENANT_GROUP,
)
end
end

def up
say_with_time "adding default tenant groups" do
Tenant.find_each do |t|
next if t.default_miq_group&.tenant_group?
t.default_miq_group_id = nil
t.add_default_miq_group
end
end
end
end
39 changes: 39 additions & 0 deletions spec/migrations/20180926152238_fix_default_tenant_group_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
require_migration

describe FixDefaultTenantGroup do
let(:tenant_stub) { migration_stub(:Tenant) }
let(:group_stub) { migration_stub(:MiqGroup) }
let(:role_stub) { migration_stub(:MiqUserRole) }
let(:tenant_role) { role_stub.create!(:name => role_stub::DEFAULT_TENANT_ROLE_NAME) }

migration_context :up do
context "role exists" do
before do
tenant_role # make sure it exists
end

it "creates a new group" do
g = group_stub.create!(:group_type => group_stub::USER_GROUP)
t = tenant_stub.create!(:default_miq_group_id => g.id)
expect(t.default_miq_group).not_to be_tenant_group

migrate

t.reload
expect(t.default_miq_group).to be_tenant_group
end

it "keeps valid tenant groups" do
t = tenant_stub.create!
t.add_default_miq_group
g_id = t.default_miq_group_id
t.save

migrate

t.reload
expect(t.default_miq_group_id).to eq(g_id)
end
end
end
end

0 comments on commit fb30ad6

Please sign in to comment.