From 791589dab8d7c0b84bf2dd7e7a941fe657573d6f Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 16 Jun 2016 10:29:17 -0400 Subject: [PATCH 1/3] Fixing Team and Credential access issues --- .../0025_v300_update_rbac_parents.py | 25 +++++++++++++++++++ awx/main/models/organization.py | 4 ++- awx/main/tests/functional/test_rbac_team.py | 7 ++++++ 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 awx/main/migrations/0025_v300_update_rbac_parents.py diff --git a/awx/main/migrations/0025_v300_update_rbac_parents.py b/awx/main/migrations/0025_v300_update_rbac_parents.py new file mode 100644 index 0000000000..0bf458987f --- /dev/null +++ b/awx/main/migrations/0025_v300_update_rbac_parents.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import awx.main.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0024_v300_jobtemplate_allow_simul'), + ] + + operations = [ + migrations.AlterField( + model_name='credential', + name='use_role', + field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'organization.admin_role', b'owner_role'], to='main.Role', null=b'True'), + ), + migrations.AlterField( + model_name='team', + name='member_role', + field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=b'admin_role', to='main.Role', null=b'True'), + ), + ] diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 94d82eae6d..0c14071644 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -104,7 +104,9 @@ class Team(CommonModelNameNotUnique, ResourceMixin): admin_role = ImplicitRoleField( parent_role='organization.admin_role', ) - member_role = ImplicitRoleField() + member_role = ImplicitRoleField( + parent_role='admin_role', + ) read_role = ImplicitRoleField( parent_role=['admin_role', 'organization.auditor_role', 'member_role'], ) diff --git a/awx/main/tests/functional/test_rbac_team.py b/awx/main/tests/functional/test_rbac_team.py index d2c1dd75c5..8396464cfa 100644 --- a/awx/main/tests/functional/test_rbac_team.py +++ b/awx/main/tests/functional/test_rbac_team.py @@ -90,3 +90,10 @@ def test_team_accessible_objects(team, user, project): team.member_role.members.add(u) assert len(Project.accessible_objects(u, 'read_role')) == 1 +@pytest.mark.django_db +def test_team_admin_member_access(team, user, project): + u = user('team_admin', False) + team.member_role.children.add(project.use_role) + team.admin_role.members.add(u) + + assert len(Project.accessible_objects(u, 'use_role')) == 1 From 14809c086dc977f7c4e2b937bf2341738bfe3ecf Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 16 Jun 2016 15:24:07 -0400 Subject: [PATCH 2/3] added some assertions to catch cycles, updated migration --- .../migrations/0025_v300_update_rbac_parents.py | 7 ++++++- awx/main/models/organization.py | 2 +- awx/main/tests/functional/test_rbac_team.py | 13 +++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/awx/main/migrations/0025_v300_update_rbac_parents.py b/awx/main/migrations/0025_v300_update_rbac_parents.py index 0bf458987f..d2ceaab73b 100644 --- a/awx/main/migrations/0025_v300_update_rbac_parents.py +++ b/awx/main/migrations/0025_v300_update_rbac_parents.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals -from django.db import migrations, models +from django.db import migrations import awx.main.fields @@ -22,4 +22,9 @@ class Migration(migrations.Migration): name='member_role', field=awx.main.fields.ImplicitRoleField(related_name='+', parent_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'organization.auditor_role', b'member_role'], to='main.Role', null=b'True'), + ), ] diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 0c14071644..3717171411 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -108,7 +108,7 @@ class Team(CommonModelNameNotUnique, ResourceMixin): parent_role='admin_role', ) read_role = ImplicitRoleField( - parent_role=['admin_role', 'organization.auditor_role', 'member_role'], + parent_role=['organization.auditor_role', 'member_role'], ) def get_absolute_url(self): diff --git a/awx/main/tests/functional/test_rbac_team.py b/awx/main/tests/functional/test_rbac_team.py index 8396464cfa..0c16ba9f6f 100644 --- a/awx/main/tests/functional/test_rbac_team.py +++ b/awx/main/tests/functional/test_rbac_team.py @@ -97,3 +97,16 @@ def test_team_admin_member_access(team, user, project): team.admin_role.members.add(u) assert len(Project.accessible_objects(u, 'use_role')) == 1 + + +@pytest.mark.django_db +def test_org_admin_team_access(organization, team, user, project): + u = user('team_admin', False) + organization.admin_role.members.add(u) + + team.organization = organization + team.save() + + team.member_role.children.add(project.use_role) + + assert len(Project.accessible_objects(u, 'use_role')) == 1 From 58c8cc1703326c4895b6d7d3ccb2f3bf20723eb4 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 16 Jun 2016 15:32:00 -0400 Subject: [PATCH 3/3] adjust team admin_role / member_role assertion --- awx/main/tests/functional/test_teams.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/awx/main/tests/functional/test_teams.py b/awx/main/tests/functional/test_teams.py index f1037f0462..eda57579ce 100644 --- a/awx/main/tests/functional/test_teams.py +++ b/awx/main/tests/functional/test_teams.py @@ -3,8 +3,12 @@ import pytest @pytest.mark.django_db() def test_admin_not_member(team): - "Test to ensure we don't add admin_role as a parent to team.member_role, as " - "this creates a cycle with organization administration, which we've decided " - "to remove support for" + """Test to ensure we don't add admin_role as a parent to team.member_role, as + this creates a cycle with organization administration, which we've decided + to remove support for - assert team.admin_role.is_ancestor_of(team.member_role) is False + (2016-06-16) I think this might have been resolved. I'm asserting + this to be true in the mean time. + """ + + assert team.admin_role.is_ancestor_of(team.member_role) is True