From 3d218d5fca5b705bf3b34988ffc7303fb8d8781a Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 11 Aug 2016 11:00:02 -0400 Subject: [PATCH] Revert "Fix to ensure org auditors can see team credentials" This reverts commit 5dcb0e57d80a3bb0553ca8194890a938257a6e93. New clarification on what the actual desired behavior of this whole system means this commit is trash, fixing in a much better way. --- .../0030_audit_team_credential_changes.py | 35 --------------- .../0031_audit_team_credential_migrations.py | 44 ------------------- awx/main/models/credential.py | 6 --- awx/main/models/organization.py | 5 +-- awx/main/signals.py | 35 --------------- .../tests/functional/api/test_credential.py | 17 ------- 6 files changed, 1 insertion(+), 141 deletions(-) delete mode 100644 awx/main/migrations/0030_audit_team_credential_changes.py delete mode 100644 awx/main/migrations/0031_audit_team_credential_migrations.py diff --git a/awx/main/migrations/0030_audit_team_credential_changes.py b/awx/main/migrations/0030_audit_team_credential_changes.py deleted file mode 100644 index c2b20d1336..0000000000 --- a/awx/main/migrations/0030_audit_team_credential_changes.py +++ /dev/null @@ -1,35 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from django.db import migrations, models -import awx.main.fields - - -class Migration(migrations.Migration): - - dependencies = [ - ('main', '0029_v302_add_ask_skip_tags'), - ] - - operations = [ - migrations.AddField( - model_name='credential', - name='teams', - field=models.ManyToManyField(related_name='credentials', to='main.Team', blank=True), - ), - migrations.AddField( - model_name='team', - name='auditor_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=b'organization.auditor_role', to='main.Role', null=b'True'), - ), - migrations.AlterField( - model_name='credential', - name='read_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'singleton:system_auditor', b'organization.auditor_role', b'teams.auditor_role', b'use_role', b'admin_role'], to='main.Role', null=b'True'), - ), - migrations.AlterField( - model_name='team', - name='read_role', - field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'auditor_role', b'member_role'], to='main.Role', null=b'True'), - ), - ] diff --git a/awx/main/migrations/0031_audit_team_credential_migrations.py b/awx/main/migrations/0031_audit_team_credential_migrations.py deleted file mode 100644 index 3febc278cc..0000000000 --- a/awx/main/migrations/0031_audit_team_credential_migrations.py +++ /dev/null @@ -1,44 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from awx.main.migrations import _rbac as rbac -from awx.main.migrations import _migration_utils as migration_utils -from django.db.models import Q -from django.db import migrations - - -def synchronize_role_changes(apps, schema_editor): - # The implicit parent roles have been updated for Credential.read_role and - # Team.read_role so these saves will pickup those changes and fix things up. - Team = apps.get_model('main', 'Team') - Credential = apps.get_model('main', 'Credential') - - for credential in Credential.objects.iterator(): - credential.save() - for team in Team.objects.iterator(): - team.save() - -def populate_credential_teams_field(apps, schema_editor): - Team = apps.get_model('main', 'Team') - Credential = apps.get_model('main', 'Credential') - - for credential in Credential.objects.iterator(): - teams_qs = Team.objects.filter( - Q(member_role__children=credential.use_role) | - Q(member_role__children=credential.admin_role) - ) - for team in teams_qs.iterator(): - credential.teams.add(team) - -class Migration(migrations.Migration): - - dependencies = [ - ('main', '0030_audit_team_credential_changes'), - ] - - operations = [ - migrations.RunPython(migration_utils.set_current_apps_for_migrations), - migrations.RunPython(synchronize_role_changes), - migrations.RunPython(populate_credential_teams_field), - migrations.RunPython(rbac.rebuild_role_hierarchy), - ] diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 937f476480..1bd11ec68e 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -87,11 +87,6 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): on_delete=models.CASCADE, related_name='credentials', ) - teams = models.ManyToManyField( - 'Team', - blank=True, - related_name='credentials', - ) kind = models.CharField( max_length=32, choices=KIND_CHOICES, @@ -231,7 +226,6 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): read_role = ImplicitRoleField(parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, 'organization.auditor_role', - 'teams.auditor_role', 'use_role', 'admin_role', ]) diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index aea0f41a77..5f3dc9d7c9 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -107,11 +107,8 @@ class Team(CommonModelNameNotUnique, ResourceMixin): member_role = ImplicitRoleField( parent_role='admin_role', ) - auditor_role = ImplicitRoleField( - parent_role='organization.auditor_role', - ) read_role = ImplicitRoleField( - parent_role=['auditor_role', 'member_role'], + parent_role=['organization.auditor_role', 'member_role'], ) def get_absolute_url(self): diff --git a/awx/main/signals.py b/awx/main/signals.py index ce3c2b45f2..7389f01763 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -9,7 +9,6 @@ import json # Django from django.db.models.signals import post_save, pre_delete, post_delete, m2m_changed -from django.db.models import Q from django.dispatch import receiver # Django-CRUM @@ -121,39 +120,6 @@ def rebuild_role_ancestor_list(reverse, model, instance, pk_set, action, **kwarg else: model.rebuild_role_ancestor_list([], [instance.id]) -def link_credentials_to_teams(reverse, model, instance, pk_set, action, **kwargs): - 'When a team is granted access to a credential, add the team to the credential teams m2m' - - # If reverse is true, then we got here by something like - # team.member_role.children.add(credential.(use|admin)_role) - # else - # credential.(use|admin)_role.parents.add(team.member_role) - - if action in ['post_add', 'post_remove']: - if reverse: - teams = [co for co in [instance.content_object] if isinstance(co, Team)] - credentials = [role.content_object for role in Role.objects.filter(id__in=pk_set).all() - if isinstance(role.content_object, Credential) and role.role_field != 'read_role'] # exclude read role to prevent signal looping - else: - credentials = [co for co in [instance.content_object] if isinstance(co, Credential) and instance.role_field != 'read_role'] - teams = [role.content_object for role in Role.objects.filter(id__in=pk_set).all() - if isinstance(role.content_object, Team)] - - if not teams or not credentials: - return - - if action == 'post_add': - for credential in credentials: - for team in teams: - credential.teams.add(team) - - if action == 'post_remove': - for credential in credentials: - for team in teams: - if not Team.objects.filter(Q(member_role__children=credential.use_role) | Q(member_role__children=credential.admin_role), id=team.id).exists(): - credential.teams.remove(team) - credential.save() - def sync_superuser_status_to_rbac(instance, **kwargs): 'When the is_superuser flag is changed on a user, reflect that in the membership of the System Admnistrator role' if instance.is_superuser: @@ -243,7 +209,6 @@ post_save.connect(emit_update_inventory_on_created_or_deleted, sender=Job) post_delete.connect(emit_update_inventory_on_created_or_deleted, sender=Job) post_save.connect(emit_job_event_detail, sender=JobEvent) post_save.connect(emit_ad_hoc_command_event_detail, sender=AdHocCommandEvent) -m2m_changed.connect(link_credentials_to_teams, Role.parents.through) m2m_changed.connect(rebuild_role_ancestor_list, Role.parents.through) m2m_changed.connect(org_admin_edit_members, Role.members.through) m2m_changed.connect(rbac_activity_stream, Role.members.through) diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index aa60ba65b8..eb6391ba05 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -113,23 +113,6 @@ def test_create_team_credential_by_team_member_xfail(post, team, alice, team_mem assert response.status_code == 403 -@pytest.mark.django_db -def test_team_credential_visibility_by_org_admins(team, credential, organization, user): - org_auditor = user('org_auditor') - organization.auditor_role.members.add(org_auditor) - assert org_auditor not in credential.read_role - - team.member_role.children.add(credential.use_role) - assert org_auditor in credential.read_role - team.member_role.children.remove(credential.use_role) - assert org_auditor not in credential.read_role - - credential.use_role.parents.add(team.member_role) - assert org_auditor in credential.read_role - credential.use_role.parents.remove(team.member_role) - assert org_auditor not in credential.read_role - - # # organization credentials