You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/08/24 14:57:32 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request #6135: Endpoint Required Permissions
ocket8888 opened a new pull request #6135:
URL: https://github.com/apache/trafficcontrol/pull/6135
This PR adds a default middleware to all routes that checks if an authenticated user has the Permissions required when a route requires Permissions. This allows routes in the `routes.go` file to specify what Permissions, if any, they require to be used at all by a client, just like how PrivLevel is currently specified. Right now, no routes require any Permissions, which reduces the middleware to a "no-op".
Permissions are only checked if the configuration's `role_based_permissions` property is set to `true`, in which case PrivLevels are now ignored and only Permissions are checked.
<!-- **^ Add meaningful description above** --><hr>
## Which Traffic Control components are affected by this PR?
- Traffic Ops
## What is the best way to verify this PR?
I feel the tests are fairly comprehensive, but to manually test what I did was add a route definition for `GET /test-ping` in API version 4.0 to routes.go, requiring the non-existent Permission "foo". The route definition simply wrote `"success
n"` to the response. I then compiled and ran Traffic Ops with `role_based_permissions` set to `true` and - after authenticating - attempted to send a GET request to `/api/4.0/test-ping`, which failed with an error about missing the Permission `foo`.
## PR submission checklist
- [x] This PR has tests
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #6135: Endpoint Required Permissions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on pull request #6135:
URL: https://github.com/apache/trafficcontrol/pull/6135#issuecomment-905114403
I'd just like to say that since I added a field to the `Route` struct that ~2000 lines changed is a bit misleading - about 1400 of those were from the route definitions alone
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6135: Endpoint Required Permissions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6135:
URL: https://github.com/apache/trafficcontrol/pull/6135#discussion_r695881520
##########
File path: traffic_ops/traffic_ops_golang/auth/authorize.go
##########
@@ -42,6 +42,26 @@ type CurrentUser struct {
TenantID int `json:"tenantId" db:"tenant_id"`
Role int `json:"role" db:"role"`
Capabilities pq.StringArray `json:"capabilities" db:"capabilities"`
+ perms map[string]struct{}
Review comment:
yes, so that it isn't exported. That way no code outside of the auth package can mutate it, so nothing can accidentally give a user more Permissions than they actually have.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp merged pull request #6135: Endpoint Required Permissions
Posted by GitBox <gi...@apache.org>.
rawlinp merged pull request #6135:
URL: https://github.com/apache/trafficcontrol/pull/6135
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #6135: Endpoint Required Permissions
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6135:
URL: https://github.com/apache/trafficcontrol/pull/6135#discussion_r695804468
##########
File path: traffic_ops/traffic_ops_golang/auth/authorize.go
##########
@@ -42,6 +42,26 @@ type CurrentUser struct {
TenantID int `json:"tenantId" db:"tenant_id"`
Role int `json:"role" db:"role"`
Capabilities pq.StringArray `json:"capabilities" db:"capabilities"`
+ perms map[string]struct{}
Review comment:
Any reason `perms` is lowercase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org