Organization on JT as read-only field

Set JT.organization with value from its project

Remove validation requiring JT.organization

Undo some of the additional org definitions in tests

Revert some tests no longer needed for feature

exclude workflow approvals from unified organization field

revert awxkit changes for providing organization

Roll back additional JT creation permission requirement

Fix up more issues by persisting organization field when project is removed

Restrict project org editing, logging, and testing

Grant removed inventory org admin permissions in migration

Add special validate_unique for job templates
  this deals with enforcing name-organization uniqueness

Add back in special message where config is unknown
  when receiving 403 on job relaunch

Fix logical and performance bugs with data migration

within JT.inventory.organization make-permission-explicit migration

remove nested loops so we do .iterator() on JT queryset

in reverse migration, carefully remove execute role on JT
  held by org admins of inventory organization,
  as well as the execute_role holders

Use current state of Role model in logic, with 1 notable exception
  that is used to filter on ancestors
  the ancestor and descentent relationship in the migration model
    is not reliable
  output of this is saved as an integer list to avoid future
    compatibility errors

make the parents rebuilding logic skip over irrelevant models
  this is the largest performance gain for small resource numbers

awx/main/tests/factories/fixtures.py

@@ -159,8 +159,7 @@ def mk_job_template(name, job_type='run',
         extra_vars = json.dumps(extra_vars)
 
     jt = JobTemplate(name=name, job_type=job_type, extra_vars=extra_vars,
-                     webhook_service=webhook_service, playbook='helloworld.yml',
-                     organization=organization)
+                     webhook_service=webhook_service, playbook='helloworld.yml')
 
     jt.inventory = inventory
     if jt.inventory is None:

awx/main/tests/factories/tower.py

@@ -255,7 +255,7 @@ def create_job_template(name, roles=None, persisted=True, webhook_service='', **
     jt = mk_job_template(name, project=proj, inventory=inv, credential=cred,
                          network_credential=net_cred, cloud_credential=cloud_cred,
                          job_type=job_type, spec=spec, extra_vars=extra_vars,
-                         persisted=persisted, webhook_service=webhook_service, organization=org)
+                         persisted=persisted, webhook_service=webhook_service)
 
     if 'jobs' in kwargs:
         for i in kwargs['jobs']:

awx/main/tests/functional/api/test_job.py

@@ -53,7 +53,7 @@ def test_job_relaunch_permission_denied_response(
     # Job has prompted extra_credential, launch denied w/ message
     job.launch_config.credentials.add(net_credential)
     r = post(reverse('api:job_relaunch', kwargs={'pk':job.pk}), {}, jt_user, expect=403)
-    assert 'launched with prompted fields which you do not have access to' in r.data['detail']
+    assert 'launched with prompted fields you do not have access to' in r.data['detail']
 
 
 @pytest.mark.django_db
@@ -73,7 +73,6 @@ def test_job_relaunch_prompts_not_accepted_response(
     # Job has prompted extra_credential, launch denied w/ message
     job.launch_config.credentials.add(net_credential)
     r = post(reverse('api:job_relaunch', kwargs={'pk':job.pk}), {}, jt_user, expect=403)
-    assert 'no longer accepts the prompts provided for this job' in r.data['detail']
 
 
 @pytest.mark.django_db
@@ -220,8 +219,7 @@ def test_block_unprocessed_events(delete, admin_user, mocker):
 def test_block_related_unprocessed_events(mocker, organization, project, delete, admin_user):
     job_template = JobTemplate.objects.create(
         project=project,
-        playbook='helloworld.yml',
-        organization=organization
+        playbook='helloworld.yml'
     )
     time_of_finish = parse("Thu Feb 23 14:17:24 2012 -0500")
     Job.objects.create(
@@ -230,7 +228,7 @@ def test_block_related_unprocessed_events(mocker, organization, project, delete,
         finished=time_of_finish,
         job_template=job_template,
         project=project,
-        organization=organization
+        organization=project.organization
     )
     view = RelatedJobsPreventDeleteMixin()
     time_of_request = time_of_finish + relativedelta(seconds=2)

awx/main/tests/functional/api/test_job_template.py

@@ -6,7 +6,7 @@ import pytest
 # AWX
 from awx.api.serializers import JobTemplateSerializer
 from awx.api.versioning import reverse
-from awx.main.models import Job, JobTemplate, CredentialType, WorkflowJobTemplate, Organization
+from awx.main.models import Job, JobTemplate, CredentialType, WorkflowJobTemplate, Organization, Project
 from awx.main.migrations import _save_password_keys as save_password_keys
 
 # Django
@@ -32,50 +32,16 @@ def test_create(post, project, machine_credential, inventory, alice, grant_proje
         inventory.use_role.members.add(alice)
     project.organization.job_template_admin_role.members.add(alice)
 
-    r = post(reverse('api:job_template_list'), {
-        'name': 'Some name',
-        'project': project.id,
-        'inventory': inventory.id,
-        'playbook': 'helloworld.yml',
-        'organization': project.organization_id
-    }, alice)
-    assert r.status_code == expect
-
-
-@pytest.mark.django_db
-def test_creation_uniqueness_rules(post, project, inventory, admin_user):
-    orgA = Organization.objects.create(name='orga')
-    orgB = Organization.objects.create(name='orgb')
-    create_data = {
-        'name': 'this_unique_name',
-        'project': project.pk,
-        'inventory': inventory.pk,
-        'playbook': 'helloworld.yml',
-        'organization': orgA.pk
-    }
     post(
         url=reverse('api:job_template_list'),
-        data=create_data,
-        user=admin_user,
-        expect=201
-    )
-    r = post(
-        url=reverse('api:job_template_list'),
-        data=create_data,
-        user=admin_user,
-        expect=400
-    )
-    msg = str(r.data['__all__'][0])
-    assert "JobTemplate with this (" in msg
-    assert ") combination already exists" in msg
-
-    # can create JT with same name, only if it is in different org
-    create_data['organization'] = orgB.pk
-    post(
-        url=reverse('api:job_template_list'),
-        data=create_data,
-        user=admin_user,
-        expect=201
+        data={
+            'name': 'Some name',
+            'project': project.id,
+            'inventory': inventory.id,
+            'playbook': 'helloworld.yml'
+        },
+        user=alice,
+        expect=expect
     )
 
 
@@ -162,14 +128,18 @@ def test_create_with_forks_exceeding_maximum_xfail(alice, post, project, invento
     project.use_role.members.add(alice)
     inventory.use_role.members.add(alice)
     settings.MAX_FORKS = 10
-    response = post(reverse('api:job_template_list'), {
-        'name': 'Some name',
-        'project': project.id,
-        'inventory': inventory.id,
-        'playbook': 'helloworld.yml',
-        'forks': 11,
-    }, alice)
-    assert response.status_code == 400
+    response = post(
+        url=reverse('api:job_template_list'),
+        data={
+            'name': 'Some name',
+            'project': project.id,
+            'inventory': inventory.id,
+            'playbook': 'helloworld.yml',
+            'forks': 11,
+        },
+        user=alice,
+        expect=400
+    )
     assert 'Maximum number of forks (10) exceeded' in str(response.data)
 
 
@@ -549,6 +519,72 @@ def test_job_template_unset_custom_virtualenv(get, patch, organization_factory,
     assert resp.data['custom_virtualenv'] is None
 
 
+@pytest.mark.django_db
+def test_jt_organization_follows_project(post, patch, admin_user):
+    org1 = Organization.objects.create(name='foo1')
+    org2 = Organization.objects.create(name='foo2')
+    project_common = dict(scm_type='git', playbook_files=['helloworld.yml'])
+    project1 = Project.objects.create(name='proj1', organization=org1, **project_common)
+    project2 = Project.objects.create(name='proj2', organization=org2, **project_common)
+    r = post(
+        url=reverse('api:job_template_list'),
+        data={
+            "name": "fooo",
+            "ask_inventory_on_launch": True,
+            "project": project1.pk,
+            "playbook": "helloworld.yml"
+        },
+        user=admin_user,
+        expect=201
+    )
+    data = r.data
+    assert data['organization'] == project1.organization_id
+    data['project'] = project2.id
+    jt = JobTemplate.objects.get(pk=data['id'])
+    r = patch(
+        url=jt.get_absolute_url(),
+        data=data,
+        user=admin_user,
+        expect=200
+    )
+    assert r.data['organization'] == project2.organization_id
+
+
+@pytest.mark.django_db
+def test_jt_organization_field_is_read_only(patch, post, project, admin_user):
+    org = project.organization
+    jt = JobTemplate.objects.create(
+        name='foo_jt',
+        ask_inventory_on_launch=True,
+        project=project, playbook='helloworld.yml'
+    )
+    org2 = Organization.objects.create(name='foo2')
+    r = patch(
+        url=jt.get_absolute_url(),
+        data={'organization': org2.id},
+        user=admin_user,
+        expect=200
+    )
+    assert r.data['organization'] == org.id
+    assert JobTemplate.objects.get(pk=jt.pk).organization == org
+
+    # similar test, but on creation
+    r = post(
+        url=reverse('api:job_template_list'),
+        data={
+            'name': 'foobar',
+            'project': project.id,
+            'organization': org2.id,
+            'ask_inventory_on_launch': True,
+            'playbook': 'helloworld.yml'
+        },
+        user=admin_user,
+        expect=201
+    )
+    assert r.data['organization'] == org.id
+    assert JobTemplate.objects.get(pk=r.data['id']).organization == org
+
+
 @pytest.mark.django_db
 def test_callback_disallowed_null_inventory(project):
     jt = JobTemplate.objects.create(
@@ -563,14 +599,13 @@ def test_callback_disallowed_null_inventory(project):
 
 
 @pytest.mark.django_db
-def test_job_template_branch_error(project, inventory, organization, post, admin_user):
+def test_job_template_branch_error(project, inventory, post, admin_user):
     r = post(
         url=reverse('api:job_template_list'),
         data={
             "name": "fooo",
             "inventory": inventory.pk,
             "project": project.pk,
-            "organization": organization.pk,
             "playbook": "helloworld.yml",
             "scm_branch": "foobar"
         },
@@ -581,14 +616,13 @@ def test_job_template_branch_error(project, inventory, organization, post, admin
 
 
 @pytest.mark.django_db
-def test_job_template_branch_prompt_error(project, inventory, post, organization, admin_user):
+def test_job_template_branch_prompt_error(project, inventory, post, admin_user):
     r = post(
         url=reverse('api:job_template_list'),
         data={
             "name": "fooo",
             "inventory": inventory.pk,
             "project": project.pk,
-            "organization": organization.pk,
             "playbook": "helloworld.yml",
             "ask_scm_branch_on_launch": True
         },

awx/main/tests/functional/api/test_rbac_displays.py

@@ -61,7 +61,7 @@ class TestJobTemplateCopyEdit:
     def jt_copy_edit(self, job_template_factory, project):
         objects = job_template_factory(
             'copy-edit-job-template',
-            project=project, organization=project.organization)
+            project=project)
         return objects.job_template
 
     def fake_context(self, user):
@@ -129,8 +129,9 @@ class TestJobTemplateCopyEdit:
 
         # random user given JT and project admin abilities
         jt_copy_edit.admin_role.members.add(rando)
+        jt_copy_edit.save()
         jt_copy_edit.project.admin_role.members.add(rando)
-        jt_copy_edit.organization.job_template_admin_role.members.add(rando)
+        jt_copy_edit.project.save()
 
         serializer = JobTemplateSerializer(jt_copy_edit, context=self.fake_context(rando))
         response = serializer.to_representation(jt_copy_edit)

awx/main/tests/functional/api/test_unified_job_template.py

@@ -39,29 +39,6 @@ class TestUnifiedOrganization:
             data['ask_inventory_on_launch'] = True
         return data
 
-    def test_organization_required_on_creation(self, model, admin_user, post):
-        cls = getattr(models, model)
-        data = self.data_for_model(model)
-        r = post(
-            url=reverse('api:{}_list'.format(get_type_for_model(cls))),
-            data=data,
-            user=admin_user,
-            expect=400
-        )
-        assert 'organization' in r.data
-        assert 'required for new object' in r.data['organization'][0]
-        # Surprising behavior - not providing the key can often give
-        # different behavior from giving it as null on create
-        data.pop('organization')
-        r = post(
-            url=reverse('api:{}_list'.format(get_type_for_model(cls))),
-            data=data,
-            user=admin_user,
-            expect=400
-        )
-        assert 'organization' in r.data
-        assert 'required' in r.data['organization'][0]
-
     def test_organization_blank_on_edit_of_orphan(self, model, admin_user, patch):
         cls = getattr(models, model)
         data = self.data_for_model(model, orm_style=True)
@@ -107,15 +84,3 @@ class TestUnifiedOrganization:
         )
         obj.refresh_from_db()
         assert obj.name == 'foooooo'
-
-    def test_organization_cannot_change_to_null(self, model, admin_user, patch, organization):
-        cls = getattr(models, model)
-        data = self.data_for_model(model, orm_style=True)
-        data['organization'] = organization
-        obj = cls.objects.create(**data)
-        patch(
-            url=obj.get_absolute_url(),
-            data={'organization': None},
-            user=admin_user,
-            expect=400
-        )

awx/main/tests/functional/conftest.py

@@ -75,26 +75,24 @@ def user():
 
 
 @pytest.fixture
-def check_jobtemplate(project, inventory, credential, organization):
+def check_jobtemplate(project, inventory, credential):
     jt = JobTemplate.objects.create(
         job_type='check',
         project=project,
         inventory=inventory,
-        name='check-job-template',
-        organization=organization
+        name='check-job-template'
     )
     jt.credentials.add(credential)
     return jt
 
 
 @pytest.fixture
-def deploy_jobtemplate(project, inventory, credential, organization):
+def deploy_jobtemplate(project, inventory, credential):
     jt = JobTemplate.objects.create(
         job_type='run',
         project=project,
         inventory=inventory,
-        name='deploy-job-template',
-        organization=organization
+        name='deploy-job-template'
     )
     jt.credentials.add(credential)
     return jt

awx/main/tests/functional/models/test_job.py

@@ -1,6 +1,9 @@
 import pytest
 
-from awx.main.models import JobTemplate, Job, JobHostSummary, WorkflowJob, Inventory
+from awx.main.models import (
+    JobTemplate, Job, JobHostSummary,
+    WorkflowJob, Inventory, Project, Organization
+)
 
 
 @pytest.mark.django_db
@@ -79,6 +82,22 @@ def test_job_host_summary_representation(host):
     assert 'N/A changed=1 dark=2 failures=3 ignored=4 ok=5 processed=6 rescued=7 skipped=8' == str(jhs)
 
 
+@pytest.mark.django_db
+def test_jt_organization_follows_project():
+    org1 = Organization.objects.create(name='foo1')
+    org2 = Organization.objects.create(name='foo2')
+    project1 = Project.objects.create(name='proj1', organization=org1)
+    project2 = Project.objects.create(name='proj2', organization=org2)
+    jt = JobTemplate.objects.create(
+        name='foo', playbook='helloworld.yml',
+        project=project1
+    )
+    assert jt.organization == org1
+    jt.project = project2
+    jt.save()
+    assert JobTemplate.objects.get(pk=jt.id).organization == org2
+
+
 @pytest.mark.django_db
 class TestSlicingModels:
 

awx/main/tests/functional/models/test_unified_job.py

@@ -13,6 +13,7 @@ from awx.main.models import (
     WorkflowApprovalTemplate, Project, WorkflowJob, Schedule,
     Credential
 )
+from awx.api.versioning import reverse
 
 
 @pytest.mark.django_db
@@ -26,6 +27,29 @@ def test_subclass_types(rando):
     ])
 
 
+@pytest.mark.django_db
+def test_soft_unique_together(post, project, admin_user):
+    """This tests that SOFT_UNIQUE_TOGETHER restrictions are applied correctly.
+    """
+    jt1 = JobTemplate.objects.create(
+        name='foo_jt',
+        project=project
+    )
+    assert jt1.organization == project.organization
+    r = post(
+        url=reverse('api:job_template_list'),
+        data=dict(
+            name='foo_jt',  # same as first
+            project=project.id,
+            ask_inventory_on_launch=True,
+            playbook='helloworld.yml'
+        ),
+        user=admin_user,
+        expect=400
+    )
+    assert 'combination already exists' in str(r.data)
+
+
 @pytest.mark.django_db
 class TestCreateUnifiedJob:
     '''

awx/main/tests/functional/test_copy.py

@@ -11,11 +11,10 @@ from awx.main.tasks import deep_copy_model_obj
 
 
 @pytest.mark.django_db
-def test_job_template_copy(post, get, project, inventory, organization, machine_credential, vault_credential,
+def test_job_template_copy(post, get, project, inventory, machine_credential, vault_credential,
                            credential, alice, job_template_with_survey_passwords, admin):
     job_template_with_survey_passwords.project = project
     job_template_with_survey_passwords.inventory = inventory
-    job_template_with_survey_passwords.organization = organization
     job_template_with_survey_passwords.save()
     job_template_with_survey_passwords.credentials.add(credential)
     job_template_with_survey_passwords.credentials.add(machine_credential)
@@ -23,7 +22,6 @@ def test_job_template_copy(post, get, project, inventory, organization, machine_
     job_template_with_survey_passwords.admin_role.members.add(alice)
     project.admin_role.members.add(alice)
     inventory.admin_role.members.add(alice)
-    organization.job_template_admin_role.members.add(alice)
     assert get(
         reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
         alice, expect=200

awx/main/tests/functional/test_instances.py

@@ -1,7 +1,7 @@
 import pytest
 from unittest import mock
 
-from awx.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate, Organization
+from awx.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate
 from awx.main.models.ha import Instance, InstanceGroup
 from awx.main.tasks import apply_cluster_membership_policies
 from awx.api.versioning import reverse
@@ -253,7 +253,7 @@ def test_inherited_instance_group_membership(instance_group_factory, default_ins
     j.inventory = inventory
     ig_org = instance_group_factory("basicA", [default_instance_group.instances.first()])
     ig_inv = instance_group_factory("basicB", [default_instance_group.instances.first()])
-    j.organization.instance_groups.add(ig_org)
+    j.project.organization.instance_groups.add(ig_org)
     j.inventory.instance_groups.add(ig_inv)
     assert ig_org in j.preferred_instance_groups
     assert ig_inv in j.preferred_instance_groups
@@ -320,14 +320,13 @@ class TestInstanceGroupOrdering:
         assert pu.preferred_instance_groups == [ig_tmp, ig_org]
 
     def test_job_instance_groups(self, instance_group_factory, inventory, project, default_instance_group):
-        org = Organization.objects.create(name='foo')
-        jt = JobTemplate.objects.create(inventory=inventory, project=project, organization=org)
-        job = Job.objects.create(inventory=inventory, job_template=jt, project=project, organization=org)
+        jt = JobTemplate.objects.create(inventory=inventory, project=project)
+        job = jt.create_unified_job()
         assert job.preferred_instance_groups == [default_instance_group]
         ig_org = instance_group_factory("OrgIstGrp", [default_instance_group.instances.first()])
         ig_inv = instance_group_factory("InvIstGrp", [default_instance_group.instances.first()])
         ig_tmp = instance_group_factory("TmpIstGrp", [default_instance_group.instances.first()])
-        jt.organization.instance_groups.add(ig_org)
+        project.organization.instance_groups.add(ig_org)
         inventory.instance_groups.add(ig_inv)
         assert job.preferred_instance_groups == [ig_inv, ig_org]
         job.job_template.instance_groups.add(ig_tmp)

awx/main/tests/functional/test_rbac_job.py

@@ -1,5 +1,7 @@
 import pytest
 
+from rest_framework.exceptions import PermissionDenied
+
 from awx.main.access import (
     JobAccess,
     JobLaunchConfigAccess,
@@ -171,9 +173,11 @@ class TestJobRelaunchAccess:
             machine_credential.use_role.members.add(u)
 
         access = JobAccess(u)
-        assert access.can_start(job_with_links, validate_license=False) == can_start, (
-            "Inventory access: {}\nCredential access: {}\n Expected access: {}".format(inv_access, cred_access, can_start)
-        )
+        if can_start:
+            assert access.can_start(job_with_links, validate_license=False)
+        else:
+            with pytest.raises(PermissionDenied):
+                access.can_start(job_with_links, validate_license=False)
 
     def test_job_relaunch_credential_access(
             self, inventory, project, credential, net_credential):
@@ -188,7 +192,8 @@ class TestJobRelaunchAccess:
 
         # Job has prompted net credential, launch denied w/ message
         job = jt.create_unified_job(credentials=[net_credential])
-        assert not jt_user.can_access(Job, 'start', job, validate_license=False)
+        with pytest.raises(PermissionDenied):
+            jt_user.can_access(Job, 'start', job, validate_license=False)
 
     def test_prompted_credential_relaunch_denied(
             self, inventory, project, net_credential, rando):
@@ -201,7 +206,8 @@ class TestJobRelaunchAccess:
 
         # Job has prompted net credential, rando lacks permission to use it
         job = jt.create_unified_job(credentials=[net_credential])
-        assert not rando.can_access(Job, 'start', job, validate_license=False)
+        with pytest.raises(PermissionDenied):
+            rando.can_access(Job, 'start', job, validate_license=False)
 
     def test_prompted_credential_relaunch_allowed(
             self, inventory, project, net_credential, rando):

awx/main/tests/functional/test_rbac_job_start.py

@@ -1,5 +1,7 @@
 import pytest
 
+from rest_framework.exceptions import PermissionDenied
+
 from awx.main.models.inventory import Inventory
 from awx.main.models.credential import Credential
 from awx.main.models.jobs import JobTemplate, Job
@@ -121,6 +123,7 @@ class TestJobRelaunchAccess:
 
     def test_orphan_relaunch_via_organization(self, job_no_prompts, rando, organization):
         "JT for job has been deleted, relevant organization roles will allow management"
+        assert job_no_prompts.organization == organization
         organization.execute_role.members.add(rando)
         job_no_prompts.job_template.delete()
         job_no_prompts.job_template = None  # Django should do this for us, but it does not
@@ -129,7 +132,9 @@ class TestJobRelaunchAccess:
     def test_no_relaunch_without_prompted_fields_access(self, job_with_prompts, rando):
         "Has JT execute_role but no use_role on inventory & credential - deny relaunch"
         job_with_prompts.job_template.execute_role.members.add(rando)
-        assert not rando.can_access(Job, 'start', job_with_prompts)
+        with pytest.raises(PermissionDenied) as exc:
+            rando.can_access(Job, 'start', job_with_prompts)
+        assert 'Job was launched with prompted fields you do not have access to' in str(exc)
 
     def test_can_relaunch_with_prompted_fields_access(self, job_with_prompts, rando):
         "Has use_role on the prompted inventory & credential - allow relaunch"
@@ -148,11 +153,15 @@ class TestJobRelaunchAccess:
         jt.ask_limit_on_launch = False
         jt.save()
         jt.execute_role.members.add(rando)
-        assert not rando.can_access(Job, 'start', job_with_prompts)
+        with pytest.raises(PermissionDenied):
+            rando.can_access(Job, 'start', job_with_prompts)
 
     def test_can_relaunch_if_limit_was_prompt(self, job_with_prompts, rando):
         "Job state differs from JT, but only on prompted fields - allow relaunch"
         job_with_prompts.job_template.execute_role.members.add(rando)
         job_with_prompts.limit = 'webservers'
         job_with_prompts.save()
-        assert not rando.can_access(Job, 'start', job_with_prompts)
+        job_with_prompts.inventory.use_role.members.add(rando)
+        for cred in job_with_prompts.credentials.all():
+            cred.use_role.members.add(rando)
+        assert rando.can_access(Job, 'start', job_with_prompts)

awx/main/tests/functional/test_rbac_job_templates.py

@@ -8,8 +8,7 @@ from awx.main.access import (
     ScheduleAccess
 )
 from awx.main.models.jobs import JobTemplate
-from awx.main.models.organization import Organization
-from awx.main.models.schedules import Schedule
+from awx.main.models import Project, Organization, Inventory, Schedule, User
 
 
 @mock.patch.object(BaseAccess, 'check_license', return_value=None)
@@ -126,11 +125,11 @@ def test_job_template_extra_credentials_prompts_access(
     )
     jt.credentials.add(machine_credential)
     jt.execute_role.members.add(rando)
-    r = post(
+    post(
         reverse('api:job_template_launch', kwargs={'pk': jt.id}),
-        {'credentials': [machine_credential.pk, vault_credential.pk]}, rando
+        {'credentials': [machine_credential.pk, vault_credential.pk]}, rando,
+        expect=403
     )
-    assert r.status_code == 403
 
 
 @pytest.mark.django_db
@@ -188,16 +187,12 @@ def test_job_template_creator_access(project, organization, rando, post):
 
 @pytest.mark.django_db
 @pytest.mark.job_permissions
-@pytest.mark.parametrize('lacking', ['project', 'inventory', 'organization'])
+@pytest.mark.parametrize('lacking', ['project', 'inventory'])
 def test_job_template_insufficient_creator_permissions(lacking, project, inventory, organization, rando, post):
     if lacking != 'project':
         project.use_role.members.add(rando)
     else:
         project.read_role.members.add(rando)
-    if lacking != 'organization':
-        organization.job_template_admin_role.members.add(rando)
-    else:
-        organization.member_role.members.add(rando)
     if lacking != 'inventory':
         inventory.use_role.members.add(rando)
     else:
@@ -206,7 +201,6 @@ def test_job_template_insufficient_creator_permissions(lacking, project, invento
         name='newly-created-jt',
         inventory=inventory.id,
         project=project.pk,
-        organization=organization.id,
         playbook='helloworld.yml'
     ), user=rando, expect=403)
 
@@ -278,25 +272,104 @@ class TestJobTemplateSchedules:
 
 
 @pytest.mark.django_db
-def test_jt_org_ownership_change(user, jt_linked):
-    admin1 = user('admin1')
-    org1 = jt_linked.organization
-    org1.admin_role.members.add(admin1)
-    a1_access = JobTemplateAccess(admin1)
+class TestProjectOrganization:
+    """Tests stories related to management of JT organization via its project
+    which have some bearing on RBAC integrity
+    """
 
-    assert a1_access.can_read(jt_linked)
+    def test_new_project_org_change(self, project, patch, admin_user):
+        org2 = Organization.objects.create(name='bar')
+        patch(
+            url=project.get_absolute_url(),
+            data={'organization': org2.id},
+            user=admin_user,
+            expect=200
+        )
+        assert Project.objects.get(pk=project.id).organization_id == org2.id
 
+    def test_jt_org_cannot_change(self, project, post, patch, admin_user):
+        post(
+            url=reverse('api:job_template_list'),
+            data={
+                'name': 'foo_template',
+                'project': project.id,
+                'playbook': 'helloworld.yml',
+                'ask_inventory_on_launch': True
+            },
+            user=admin_user,
+            expect=201
+        )
+        org2 = Organization.objects.create(name='bar')
+        r = patch(
+            url=project.get_absolute_url(),
+            data={'organization': org2.id},
+            user=admin_user,
+            expect=400
+        )
+        assert 'Organization cannot be changed' in str(r.data)
 
-    admin2 = user('admin2')
-    org2 = Organization.objects.create(name='mrroboto', description='domo')
-    org2.admin_role.members.add(admin2)
-    a2_access = JobTemplateAccess(admin2)
+    def test_orphan_JT_adoption(self, project, patch, admin_user, org_admin):
+        jt = JobTemplate.objects.create(
+            name='bar',
+            ask_inventory_on_launch=True,
+            playbook='helloworld.yml'
+        )
+        assert org_admin not in jt.admin_role
+        patch(
+            url=jt.get_absolute_url(),
+            data={'project': project.id},
+            user=admin_user,
+            expect=200
+        )
+        assert org_admin in jt.admin_role
 
-    assert not a2_access.can_read(jt_linked)
+    def test_inventory_read_transfer_direct(self, patch):
+        orgs = []
+        invs = []
+        admins = []
+        for i in range(2):
+            org = Organization.objects.create(name='org{}'.format(i))
+            org_admin = User.objects.create(username='user{}'.format(i))
+            inv = Inventory.objects.create(
+                organization=org,
+                name='inv{}'.format(i)
+            )
+            org.auditor_role.members.add(org_admin)
 
+            orgs.append(org)
+            admins.append(org_admin)
+            invs.append(inv)
 
-    jt_linked.organization = org2
-    jt_linked.save()
+        jt = JobTemplate.objects.create(name='foo', inventory=invs[0])
+        assert admins[0] in jt.read_role
+        assert admins[1] not in jt.read_role
 
-    assert a2_access.can_read(jt_linked)
-    assert not a1_access.can_read(jt_linked)
+        jt.inventory = invs[1]
+        jt.save(update_fields=['inventory'])
+        assert admins[0] not in jt.read_role
+        assert admins[1] in jt.read_role
+
+    def test_inventory_read_transfer_indirect(self, patch):
+        orgs = []
+        admins = []
+        for i in range(2):
+            org = Organization.objects.create(name='org{}'.format(i))
+            org_admin = User.objects.create(username='user{}'.format(i))
+            org.auditor_role.members.add(org_admin)
+
+            orgs.append(org)
+            admins.append(org_admin)
+
+        inv = Inventory.objects.create(
+            organization=orgs[0],
+            name='inv{}'.format(i)
+        )
+
+        jt = JobTemplate.objects.create(name='foo', inventory=inv)
+        assert admins[0] in jt.read_role
+        assert admins[1] not in jt.read_role
+
+        inv.organization = orgs[1]
+        inv.save(update_fields=['organization'])
+        assert admins[0] not in jt.read_role
+        assert admins[1] in jt.read_role

awx/main/tests/functional/test_rbac_migration.py

@@ -1,11 +1,14 @@
 import pytest
 
+from django.apps import apps
+
 from awx.main.migrations import _rbac as rbac
 from awx.main.models import (
     UnifiedJobTemplate,
     InventorySource, Inventory,
     JobTemplate, Project,
-    Organization
+    Organization,
+    User
 )
 
 
@@ -62,3 +65,37 @@ def test_implied_organization_subquery_job_template():
                 assert jt.test_field is None
             else:
                 assert jt.test_field == jt.project.organization_id
+
+
+@pytest.mark.django_db
+def test_give_explicit_inventory_permission():
+    dual_admin = User.objects.create(username='alice')
+    inv_admin = User.objects.create(username='bob')
+    inv_org = Organization.objects.create(name='inv-org')
+    proj_org = Organization.objects.create(name='proj-org')
+
+    inv_org.admin_role.members.add(inv_admin, dual_admin)
+    proj_org.admin_role.members.add(dual_admin)
+
+    proj = Project.objects.create(
+        name="test-proj",
+        organization=proj_org
+    )
+    inv = Inventory.objects.create(
+        name='test-inv',
+        organization=inv_org
+    )
+
+    jt = JobTemplate.objects.create(
+        name='foo',
+        project=proj,
+        inventory=inv
+    )
+
+    assert dual_admin in jt.admin_role
+
+    rbac.restore_inventory_admins(apps, None)
+
+    assert inv_admin in jt.admin_role.members.all()
+    assert dual_admin not in jt.admin_role.members.all()
+    assert dual_admin in jt.admin_role

awx/main/tests/unit/models/test_unified_job_unit.py

@@ -6,6 +6,7 @@ from awx.main.models import (
     UnifiedJobTemplate,
     WorkflowJob,
     WorkflowJobNode,
+    WorkflowApprovalTemplate,
     Job,
     User,
     Project,
@@ -70,7 +71,9 @@ def test_organization_copy_to_jobs():
     All unified job types should infer their organization from their template organization
     '''
     for cls in UnifiedJobTemplate.__subclasses__():
-        assert 'organization' in cls._get_unified_job_field_names()
+        if cls is WorkflowApprovalTemplate:
+            continue  # these do not track organization
+        assert 'organization' in cls._get_unified_job_field_names(), cls
 
 
 def test_log_representation():