Skip to content
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

Add Test to Catch Missing Permissions #6610

Closed
wants to merge 1 commit into from

Conversation

shamrickus
Copy link
Member

This PR adds a test that checks that at least one user (admin) has all the permissions for the TO API routes. It will catch #6603 and #6609 with a test failure.

Note: until #6609 is fixed, the TO API tests on this PR will fail.


Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Run the TO API tests and verify they pass.

PR submission checklist

@shamrickus shamrickus added database relating to setup/installation/structure of the Traffic Ops database Traffic Ops related to Traffic Ops labels Mar 3, 2022
@ocket8888
Copy link
Contributor

The definition of the "admin" Role is that it has all Permissions past, present, and future. The set of Permissions afforded to the "admin" Role isn't even checked by Traffic Ops, so this is testing unintended behavior.

For example: The year is 2024, and I'm adding Foos as a new object to the Traffic Ops API. Foos are not related to any existing objects, and don't affect them in any way. Which users should be granted the FOO:READ and FOO:CREATE Permissions?

The intended answer is "none" and an "admin"-Role user will be responsible for doling out those new Permissions to whatever Roles need them. That's why we enforce an "admin" Role must exist, and why we treat it specially the way we do.

@shamrickus shamrickus closed this Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database relating to setup/installation/structure of the Traffic Ops database Traffic Ops related to Traffic Ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants