-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make organization a field on job templates #3903
Comments
Also, I believe that this special case can be removed after this is done: Lines 227 to 239 in 20e5d82
We need to very carefully consider whether some other similar signals needs to be implemented after this model change is made. I don't think so, but there are dragons here. |
@mabashian @jlmitch5 The major UI item will be to add the organization selector to the job template here. I'm advocating handling this as its own vertical slice work item, which will involved API & UI. |
Another RBAC detail that I want triaged before starting on this:
|
@wenottingham @matburt Users inherit admin, execute, and auditing permissions via the inventory of a JT. current: Lines 244 to 252 in ca0e810
With this change, will we revoke these permissions obtained via inventory? If we keep any of these, that will significantly limit the simplifications that can be obtained here. proposed: diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py
index 59b26c66e5..f1ec15c17a 100644
--- a/awx/main/models/jobs.py
+++ b/awx/main/models/jobs.py
@@ -242,13 +249,13 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour
)
admin_role = ImplicitRoleField(
- parent_role=['project.organization.job_template_admin_role', 'inventory.organization.job_template_admin_role']
+ parent_role=['organization.job_template_admin_role']
)
execute_role = ImplicitRoleField(
- parent_role=['admin_role', 'project.organization.execute_role', 'inventory.organization.execute_role'],
+ parent_role=['admin_role', 'organization.execute_role'],
)
read_role = ImplicitRoleField(
- parent_role=['project.organization.auditor_role', 'inventory.organization.auditor_role', 'execute_role', 'admin_role'],
+ parent_role=['organization.auditor_role', 'execute_role', 'admin_role'],
) |
Sorry, just for clarity: inherit admin, execute, and auditing of ???, via the inventory of a JT they have ??? role on? |
Say I have a JT with a project from There are users Hypothetical users |
I did not think these permissions would be there, since the implied org on the JT only came from the project. I think it's fine to remove these going forwards. I could see migrating explicit roles on the JT into existence on upgrade for this case, although I don't know how complex the migration would be. |
Thanks. The above proposal isn't done yet, but it's workable by writing a data migration. |
@matburt has suggested providing some read access via project & inventory in order to mitigate the backlash from removing admin-via-inventory permissions. Here is what that might look like: diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py
index 12c691d195..30730f0417 100644
--- a/awx/main/models/jobs.py
+++ b/awx/main/models/jobs.py
@@ -242,13 +242,17 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour
)
admin_role = ImplicitRoleField(
- parent_role=['project.organization.job_template_admin_role', 'inventory.organization.job_template_admin_role']
+ parent_role=['organization.job_template_admin_role']
)
execute_role = ImplicitRoleField(
- parent_role=['admin_role', 'project.organization.execute_role', 'inventory.organization.execute_role'],
+ parent_role=['admin_role', 'organization.execute_role'],
)
read_role = ImplicitRoleField(
- parent_role=['project.organization.auditor_role', 'inventory.organization.auditor_role', 'execute_role', 'admin_role'],
+ parent_role=[
+ 'organization.auditor_role',
+ 'project.organization.project_admin_role', 'inventory.organization.inventory_admin_role',
+ 'execute_role', 'admin_role'
+ ],
) There are minor things about this I still don't like. In terms of the general principle and usability, this is much better than the previous proposal. |
@jlmitch5 I'm trying to poke around and find more places that will need updates for this change. One is the job templates list for organizations.
You can see, this is listing JTs for which project or inventory are in the organization. That will need to be changed as a part of this work. I also checked deletion warnings. It looks like no changes are needed here. Credentials are the only thing that cascade delete with organizations (I have created all types of resources I can think of in the screenshot). That remains the same before / after JT.organization thing is implemented. |
Next finding The permissions model proposed in prior comment: is not currently possible. If you grant a user admin_role to the inventory organization, it will display Where In [14]: u in jt.admin_role
Out[14]: False
In [15]: u in jt.read_role
Out[15]: True This is a re-emergence of the display issue described in #4108. Like several other features, the implementation of this proposal is blocked by that incompleteness of the permissions list. |
To be clear re: above - that is showing a user who has |
^ correct |
I don't think it's a display issue. I think it's an API bug that we could work around in the UI.... |
To be more clear: the API returns roles in indirect_access that it should not, and are not actually roles that apply to the object in question. |
The UI permission display is like describing a point on the To answer your question, the API is showing the correct roles in the list, and it gives the correct forms of access obtained by that role. In the example for "has_inventory_admin..." user in my last screenshot, see the API data: {
"id": 8,
"type": "user",
"url": "/api/v2/users/8/",
"related": {
"teams": "/api/v2/users/8/teams/",
"organizations": "/api/v2/users/8/organizations/",
"admin_of_organizations": "/api/v2/users/8/admin_of_organizations/",
"projects": "/api/v2/users/8/projects/",
"credentials": "/api/v2/users/8/credentials/",
"roles": "/api/v2/users/8/roles/",
"activity_stream": "/api/v2/users/8/activity_stream/",
"access_list": "/api/v2/users/8/access_list/",
"tokens": "/api/v2/users/8/tokens/",
"authorized_tokens": "/api/v2/users/8/authorized_tokens/",
"personal_tokens": "/api/v2/users/8/personal_tokens/"
},
"summary_fields": {
"direct_access": [],
"indirect_access": [
{
"role": {
"id": 1635,
"name": "Admin",
"description": "Can manage all aspects of the organization",
"resource_name": "alan",
"resource_type": "organization",
"related": {
"organization": "/api/v2/organizations/84/"
},
"user_capabilities": {
"unattach": true
}
},
"descendant_roles": [
"read_role",
"use_role"
]
}
]
},
"created": "2019-08-19T16:08:06.211769Z",
"username": "alan_org_admin",
"first_name": "alan_org_admin",
"last_name": "",
"email": "[email protected]",
"is_superuser": false,
"is_system_auditor": false,
"ldap_dn": "",
"last_login": null,
"external_account": null
} The UI does not do anything with the To clarify, the logic to add the |
I don't think that's correct. Note, I'm working on the case from #4108 here. Here is that case:
Default:
Make
Explicitly grant team C
Explicitly grant team C
That last point is the bug. The org admin status of B does not grant them any use/read role on the object, and should not be in the indirect_roles. |
@pytest.mark.django_db
def test_team_admin_access():
from awx.main.models import Inventory, User, Project, Organization, Team
A = Organization.objects.create(name='A')
B = Organization.objects.create(name='B')
c = Team.objects.create(name='c', organization=B)
# user 'fred' in org B, in a team C
fred = User.objects.create(username='fred')
B.member_role.members.add(fred)
c.member_role.members.add(fred)
# inventory 'foo' in org A
foo = Inventory.objects.create(name='foo', organization=A)
# nothing in access_list for foo
assert fred not in foo.read_role
# Make fred an admin of org B:
B.admin_role.members.add(fred)
# nothing in access_list for foo
assert fred not in foo.admin_role
# Explicitly grant team C use on foo, when fred is not an org admin of B:
B.admin_role.members.remove(fred)
c.member_role.children.add(foo.use_role)
# direct use role now in access_list
assert fred in foo.use_role
# Explicitly grant team C use on foo, when fred is an org admin of B:
B.admin_role.members.add(fred) # team C already has foo.use_role
# direct use role now in access_list
assert fred in foo.use_role
# The org admin status of B does not grant them any use/read role on the object, and should not be in the indirect_roles.
c.member_role.members.remove(fred)
assert fred not in foo.use_role # yes it does result:
The access list is correct. Org admins inherit team permissions from teams inside their organization. This RBAC rule is defined via this fields: awx/awx/main/models/organization.py Lines 126 to 131 in dc1bf3e
If you have |
Flipping back to needs_devel as #4338 was closed. |
When is this coming in the release? Thanks in advance. |
pin to runner==1.4.4
Any word on this? @AlanCoding |
Notes for @tvo318 In Tower 3.7, Job Templates now have a read-only field that shows what organization they are affiliated with. They inherit this from the Project that provides the playbook for the Job Template. In situations where Job Templates have inventories and projects from different organizations, this changes the RBAC relationship subtly. For Job Templates created before migrating to Ansible Tower 3.7, all previous permissions are preserved through explicit permission assignments to job templates. For newly created objects permissions are inherited from roles on the Project’s Organization, which is now explicitly the Job Template’s organization. The only users implicit admin role to the Job Template will be those with job_template_admin_role in the project’s organization. All other users will have explicit permissions to the job template either set on the job template or on the organization that the job template now explicitly belongs to. The only users from the inventory org that will have any access to the Job Template for newly created Job Templates shall be auditors, who will continue to have audit permissions to the Job Template because it affects hosts in the inventory. Additionally, because now Job Templates belong to the same Organization as the project, job templates are only required to have names unique within their organization. This has the consequence that the “named url” for the job template is now officially /api/v2/job_templates/MyJobTemplate++MyOrganization/ . However, for backwards compatibility the deprecated /api/v2/job_templates/MyJobTemplate will still work for Tower 3.7. |
First Y/N means had permission on prior version of awx/tower Second Y/N means has permission after upgrade to new version of awx/tower on existing objects that were upgraded Third Y/N means has permission on newly created resources in new version of awx/tower
|
closing as we are done verifying and regression tests merged |
ISSUE TYPE
SUMMARY
Right now, job templates have an implicit organization, based on the organization of its project (and sometimes inventory).
An
organization
field will be added directly to the job template. In a data migration, the JT's project's organization will be used for the default value. From then on, the JT organization will be used instead of the project organization for permissions considerations.This issue should replace:
Some more speculative statements about changes which will come with this:
organization
field should also be added to jobs, which is copied on launch. Permission rules will reference this organizationThe text was updated successfully, but these errors were encountered: