[jira] [Commented] (TC-461) Delivery-service tenancy based access control - Steering Target

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-461?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106629#comment-16106629
 ] 

ASF GitHub Bot commented on TC-461:
---

GitHub user nir-sopher opened a pull request:

https://github.com/apache/incubator-trafficcontrol/pull/766

[TC-461] DS steering tenancy checks

WIP
Waiting to merge with PR 765

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nir-sopher/incubator-trafficcontrol 
ds-tenancy-validation-steering

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafficcontrol/pull/766.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #766


commit 8bd9c437d2b50cd6dbe09b8b23050b15b625f483
Author: nir-sopher 
Date:   2017-07-17T20:25:42Z

DS steering tenancy checks




> Delivery-service tenancy based access control - Steering Target
> ---
>
> Key: TC-461
> URL: https://issues.apache.org/jira/browse/TC-461
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS steering target: In order to add a 
> steering target DS1 to steering DS ST1, the user should have access to the 
> tenants of DS1 and ST1. In order to read the record, access to ST1 is enough 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #766: [TC-461] DS steering tenancy che...

[nir-sopher]

GitHub user nir-sopher opened a pull request:

https://github.com/apache/incubator-trafficcontrol/pull/766

[TC-461] DS steering tenancy checks

WIP
Waiting to merge with PR 765

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nir-sopher/incubator-trafficcontrol 
ds-tenancy-validation-steering

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafficcontrol/pull/766.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #766


commit 8bd9c437d2b50cd6dbe09b8b23050b15b625f483
Author: nir-sopher 
Date:   2017-07-17T20:25:42Z

DS steering tenancy checks




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-485) Steering Target API - Redundent parameters

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-485?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106625#comment-16106625
 ] 

ASF GitHub Bot commented on TC-485:
---

GitHub user nir-sopher opened a pull request:

https://github.com/apache/incubator-trafficcontrol/pull/765

[TC-485] Steering target fix

Removing redundant params and fixing the UT

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nir-sopher/incubator-trafficcontrol 
steering-target-fix

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafficcontrol/pull/765.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #765


commit 9ed1ab4be77724c3f96779b02ef0af76fecf677b
Author: nir-sopher 
Date:   2017-07-17T21:21:45Z

SteeringTarget API - removing redundent parameters

commit a842acd4a4c9b27fd6d89fe0d80b2f608c7a5ab0
Author: nir-sopher 
Date:   2017-07-17T21:25:30Z

SteeringTarget Negative UT




> Steering Target API - Redundent parameters
> --
>
> Key: TC-485
> URL: https://issues.apache.org/jira/browse/TC-485
> Project: Traffic Control
>  Issue Type: Bug
>  Components: Traffic Ops API
>Affects Versions: 2.1.0
>Reporter: Nir Sopher
>
> Traffic Ops POST endpoint for "/api/1.2/steering" have the 
> delivery-service and target IDs in the path, but use the values retrieved as 
> parameters.
> Therefore, these parameters needs to be removed



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Created] (TC-485) Steering Target API - Redundent parameters

[Nir Sopher (JIRA)]

Nir Sopher created TC-485:
-

 Summary: Steering Target API - Redundent parameters
 Key: TC-485
 URL: https://issues.apache.org/jira/browse/TC-485
 Project: Traffic Control
  Issue Type: Bug
  Components: Traffic Ops API
Affects Versions: 2.1.0
Reporter: Nir Sopher


Traffic Ops POST endpoint for "/api/1.2/steering" have the delivery-service 
and target IDs in the path, but use the values retrieved as parameters.
Therefore, these parameters needs to be removed



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106457#comment-16106457
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130240112
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -1021,6 +1030,10 @@ sub details {
}
 
my $rs_ds_data = $row->deliveryservice_servers;
+   #FOR THE REVIEWER - Currently I do not check DS tenancy 
here.
--- End diff --

done


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106458#comment-16106458
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130240113
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -947,6 +952,10 @@ sub details_v11 {
}
 
my $rs_ds_data = $row->deliveryservice_servers;
+   #FOR THE REVIEWER - Currently I do not check DS tenancy here.
--- End diff --

done


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130240113
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -947,6 +952,10 @@ sub details_v11 {
}
 
my $rs_ds_data = $row->deliveryservice_servers;
+   #FOR THE REVIEWER - Currently I do not check DS tenancy here.
--- End diff --

done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130240112
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -1021,6 +1030,10 @@ sub details {
}
 
my $rs_ds_data = $row->deliveryservice_servers;
+   #FOR THE REVIEWER - Currently I do not check DS tenancy 
here.
--- End diff --

done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106444#comment-16106444
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130239129
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -569,10 +575,17 @@ sub get_unassigned_servers_by_dsid {
$ds_server_criteria{'deliveryservice.id'} = $ds_id;
 
my @assigned_servers;
-   if ( _privileged($self) || 
$self->is_delivery_service_assigned($ds_id) ) {
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
--- End diff --

Seems ok to me. 

case 1: "use=tenancy" == true, and resource is accessible to tenant:  
1. "if" test is false, continue
2. "elseif" test is true (as ignore_ds_users_table==true), so we get the 
list of servers

case 2: "use=tenancy" == true, and resource is not accessible to tenant:  
1. "if" test is true, return forbidden

case 3: "use=tenancy" == false, and ds is assigned to user
1. "if" test is false, continue
2. "elseif" test is true (as the ds is assigned to user), so we get the 
list of servers


case 3: "use=tenancy" == false, and ds is not assigned to user
1. "if" test is false, continue
2. "elseif" test is false (as the ds is not assigned to user),
3. We reach the failure



> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130239129
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -569,10 +575,17 @@ sub get_unassigned_servers_by_dsid {
$ds_server_criteria{'deliveryservice.id'} = $ds_id;
 
my @assigned_servers;
-   if ( _privileged($self) || 
$self->is_delivery_service_assigned($ds_id) ) {
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
--- End diff --

Seems ok to me. 

case 1: "use=tenancy" == true, and resource is accessible to tenant:  
1. "if" test is false, continue
2. "elseif" test is true (as ignore_ds_users_table==true), so we get the 
list of servers

case 2: "use=tenancy" == true, and resource is not accessible to tenant:  
1. "if" test is true, return forbidden

case 3: "use=tenancy" == false, and ds is assigned to user
1. "if" test is false, continue
2. "elseif" test is true (as the ds is assigned to user), so we get the 
list of servers


case 3: "use=tenancy" == false, and ds is not assigned to user
1. "if" test is false, continue
2. "elseif" test is false (as the ds is not assigned to user),
3. We reach the failure



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106433#comment-16106433
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238813
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -726,41 +746,26 @@ sub get_servers_by_type {
my $orderby   = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
 
+   my $forbidden;
--- End diff --

This function previously filtered records by the ds_user table (only 
servers used for the DSes viewable to the user)
We agreed on mail to remove the test and put "is-oper"


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238813
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -726,41 +746,26 @@ sub get_servers_by_type {
my $orderby   = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
 
+   my $forbidden;
--- End diff --

This function previously filtered records by the ds_user table (only 
servers used for the DSes viewable to the user)
We agreed on mail to remove the test and put "is-oper"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106429#comment-16106429
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238764
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -569,10 +575,17 @@ sub get_unassigned_servers_by_dsid {
$ds_server_criteria{'deliveryservice.id'} = $ds_id;
 
my @assigned_servers;
-   if ( _privileged($self) || 
$self->is_delivery_service_assigned($ds_id) ) {
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->forbidden();
+   }
+   elsif ( _privileged($self) || $tenant_utils->ignore_ds_users_table() 
|| $self->is_delivery_service_assigned($ds_id) ) {
@assigned_servers = 
$self->db->resultset('DeliveryserviceServer')->search( \%ds_server_criteria, { 
prefetch => [ 'deliveryservice', 'server' ] } )->get_column('server')->all();
}
else {
+   #for the reviewer - I believe it should turn into forbidden as 
well
--- End diff --

Fixed


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106430#comment-16106430
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238784
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -651,7 +664,14 @@ sub get_eligible_servers_by_dsid {
my %ds_server_criteria;
$ds_server_criteria{'deliveryservice.id'} = $ds_id;
 
-   if ( !_privileged($self) && 
!$self->is_delivery_service_assigned($ds_id) ) {
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->forbidden();
+   }
+   elsif ( !_privileged($self) && 
!$tenant_utils->ignore_ds_users_table() && 
!$self->is_delivery_service_assigned($ds_id) ) {
+   #for the reviewer - I believe it should turn into forbidden as 
well
return $self->alert("Forbidden. Delivery service not assigned 
to user.");
--- End diff --

done


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238764
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -569,10 +575,17 @@ sub get_unassigned_servers_by_dsid {
$ds_server_criteria{'deliveryservice.id'} = $ds_id;
 
my @assigned_servers;
-   if ( _privileged($self) || 
$self->is_delivery_service_assigned($ds_id) ) {
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->forbidden();
+   }
+   elsif ( _privileged($self) || $tenant_utils->ignore_ds_users_table() 
|| $self->is_delivery_service_assigned($ds_id) ) {
@assigned_servers = 
$self->db->resultset('DeliveryserviceServer')->search( \%ds_server_criteria, { 
prefetch => [ 'deliveryservice', 'server' ] } )->get_column('server')->all();
}
else {
+   #for the reviewer - I believe it should turn into forbidden as 
well
--- End diff --

Fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238784
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -651,7 +664,14 @@ sub get_eligible_servers_by_dsid {
my %ds_server_criteria;
$ds_server_criteria{'deliveryservice.id'} = $ds_id;
 
-   if ( !_privileged($self) && 
!$self->is_delivery_service_assigned($ds_id) ) {
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->forbidden();
+   }
+   elsif ( !_privileged($self) && 
!$tenant_utils->ignore_ds_users_table() && 
!$self->is_delivery_service_assigned($ds_id) ) {
+   #for the reviewer - I believe it should turn into forbidden as 
well
return $self->alert("Forbidden. Delivery service not assigned 
to user.");
--- End diff --

done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106425#comment-16106425
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238709
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -400,40 +401,25 @@ sub get_servers_by_status {
my $orderby   = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
 
+   my $forbidden;
--- End diff --

This function tested the "ds_user" table and filtered DSes accordingly.
We discussed it (by mail) and decided to remove this test and add only a 
"is-oper" test



> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238734
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -489,11 +484,22 @@ sub get_edge_servers_by_dsid {
my $self= shift;
my $ds_id   = $self->param('id');
 
+   my $ds = $self->db->resultset('Deliveryservice')->search( { 'me.id' => 
$ds_id } )->single();
+   if ( !defined($ds) ) {
+   return $self->not_found();
+   }
+
my $ds_servers;
-   if ( _privileged($self) || 
$self->is_delivery_service_assigned($ds_id) ) {
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->forbidden();
--- End diff --

Done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106426#comment-16106426
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238728
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -445,18 +431,27 @@ sub get_servers_by_dsid {
my $orderby_snakecase = lcfirst( decamelize($orderby) );
my $helper= new Utils::Helper( { mojo => $self } );
 
+   my $ds = $self->db->resultset('Deliveryservice')->search( { 'me.id' => 
$ds_id }, { prefetch => ['type'] } )->single();
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
my @ds_servers;
my $forbidden;
-   if ( _privileged($self) || 
$self->is_delivery_service_assigned($ds_id) ) {
+   my $servers;
+
+   if (defined($ds) && 
!$tenant_utils->is_ds_resource_accessible($tenants_data, $ds->tenant_id)) {
+   $forbidden = "Forbidden. Delivery service not available for 
user's tenant.";
+   return ($forbidden, $servers);
+   }
+   elsif ( _privileged($self) || $tenant_utils->ignore_ds_users_table() 
|| $self->is_delivery_service_assigned($ds_id) ) {
--- End diff --

Same, answer:)


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238728
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -445,18 +431,27 @@ sub get_servers_by_dsid {
my $orderby_snakecase = lcfirst( decamelize($orderby) );
my $helper= new Utils::Helper( { mojo => $self } );
 
+   my $ds = $self->db->resultset('Deliveryservice')->search( { 'me.id' => 
$ds_id }, { prefetch => ['type'] } )->single();
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
my @ds_servers;
my $forbidden;
-   if ( _privileged($self) || 
$self->is_delivery_service_assigned($ds_id) ) {
+   my $servers;
+
+   if (defined($ds) && 
!$tenant_utils->is_ds_resource_accessible($tenants_data, $ds->tenant_id)) {
+   $forbidden = "Forbidden. Delivery service not available for 
user's tenant.";
+   return ($forbidden, $servers);
+   }
+   elsif ( _privileged($self) || $tenant_utils->ignore_ds_users_table() 
|| $self->is_delivery_service_assigned($ds_id) ) {
--- End diff --

Same, answer:)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238709
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -400,40 +401,25 @@ sub get_servers_by_status {
my $orderby   = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
 
+   my $forbidden;
--- End diff --

This function tested the "ds_user" table and filtered DSes accordingly.
We discussed it (by mail) and decided to remove this test and add only a 
"is-oper" test



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106423#comment-16106423
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238653
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -400,40 +401,25 @@ sub get_servers_by_status {
my $orderby   = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
 
+   my $forbidden;
--- End diff --

The function was previously testing the ds_user table as well and filtering 
by it.
We agreed it should be removed and replaced  only with "is-oper" test


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238653
  
--- Diff: traffic_ops/app/lib/API/Server.pm ---
@@ -400,40 +401,25 @@ sub get_servers_by_status {
my $orderby   = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
 
+   my $forbidden;
--- End diff --

The function was previously testing the ds_user table as well and filtering 
by it.
We agreed it should be removed and replaced  only with "is-oper" test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106419#comment-16106419
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238561
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceServer.pm ---
@@ -223,8 +240,19 @@ sub remove_server_from_ds {
my $ds_id   = $self->param('dsId');
my $server_id   = $self->param('serverId');
 
-   if ( !_privileged($self) && 
!$self->is_delivery_service_assigned($ds_id) ) {
-   $self->forbidden("Forbidden. Delivery service not assigned to 
user.");
+   my $tenant_utils = Utils::Tenant->new($self);
+   if ( !_privileged($self) && !$tenant_utils->ignore_ds_users_table() 
&& !$self->is_delivery_service_assigned($ds_id) ) {
--- End diff --

Probably yes, we need to decide if we merge only the 2 parameters 
(use-tenancy & ignore_ds_user_assignment) or the functions as well.
If we merge also the fucntions, lets do it in one "search replace" commit


> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238561
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceServer.pm ---
@@ -223,8 +240,19 @@ sub remove_server_from_ds {
my $ds_id   = $self->param('dsId');
my $server_id   = $self->param('serverId');
 
-   if ( !_privileged($self) && 
!$self->is_delivery_service_assigned($ds_id) ) {
-   $self->forbidden("Forbidden. Delivery service not assigned to 
user.");
+   my $tenant_utils = Utils::Tenant->new($self);
+   if ( !_privileged($self) && !$tenant_utils->ignore_ds_users_table() 
&& !$self->is_delivery_service_assigned($ds_id) ) {
--- End diff --

Probably yes, we need to decide if we merge only the 2 parameters 
(use-tenancy & ignore_ds_user_assignment) or the functions as well.
If we merge also the fucntions, lets do it in one "search replace" commit


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-460) Delivery-service tenancy based access control - DS/Server Assignment

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106418#comment-16106418
 ] 

ASF GitHub Bot commented on TC-460:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238531
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceServer.pm ---
@@ -80,15 +86,21 @@ sub assign_servers_to_ds {
return $self->forbidden();
}
 
-   if ( ref($servers) ne 'ARRAY' ) {
-   return $self->alert("Servers must be an array");
-   }
-
my $ds = $self->db->resultset('Deliveryservice')->find( { id => $ds_id 
} );
if ( !defined($ds) ) {
return $self->not_found();
}
 
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->alert("Invalid delivery-service assignment. The 
delivery-service is not avaialble for your tenant.");
--- End diff --

Are you sure?
This is not part of the path

$r->post("/api/$version/deliveryserviceserver")->over( authenticated => 1, 
not_ldap => 1 )->to( 'DeliveryServiceServer#assign_servers_to_ds', namespace => 
$namespace );





> Delivery-service tenancy based access control - DS/Server Assignment
> 
>
> Key: TC-460
> URL: https://issues.apache.org/jira/browse/TC-460
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS assignment to servers. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #752: [TC-460] Ds tenancy validation s...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/752#discussion_r130238531
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceServer.pm ---
@@ -80,15 +86,21 @@ sub assign_servers_to_ds {
return $self->forbidden();
}
 
-   if ( ref($servers) ne 'ARRAY' ) {
-   return $self->alert("Servers must be an array");
-   }
-
my $ds = $self->db->resultset('Deliveryservice')->find( { id => $ds_id 
} );
if ( !defined($ds) ) {
return $self->not_found();
}
 
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->alert("Invalid delivery-service assignment. The 
delivery-service is not avaialble for your tenant.");
--- End diff --

Are you sure?
This is not part of the path

$r->post("/api/$version/deliveryserviceserver")->over( authenticated => 1, 
not_ldap => 1 )->to( 'DeliveryServiceServer#assign_servers_to_ds', namespace => 
$namespace );





---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #751: [TC-462] Ds tenancy validation r...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237388
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceRegexes.pm ---
@@ -222,6 +262,18 @@ sub delete {
return $self->forbidden();
}
 
+   my $ds = $self->db->resultset('Deliveryservice')->find( { id => $ds_id 
} );
+   if ( !defined($ds) ) {
+   #allow deletion if the ds is not valid
--- End diff --

If sue to some bug or inconsitency - the a regex exists but the parent DS 
does not, we cannot check tenancy.
However we want to allow the deletion of the regex - which for itself 
exists.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-462) Delivery-service tenancy based access control - Regexes

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106396#comment-16106396
 ] 

ASF GitHub Bot commented on TC-462:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237388
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceRegexes.pm ---
@@ -222,6 +262,18 @@ sub delete {
return $self->forbidden();
}
 
+   my $ds = $self->db->resultset('Deliveryservice')->find( { id => $ds_id 
} );
+   if ( !defined($ds) ) {
+   #allow deletion if the ds is not valid
--- End diff --

If sue to some bug or inconsitency - the a regex exists but the parent DS 
does not, we cannot check tenancy.
However we want to allow the deletion of the regex - which for itself 
exists.


> Delivery-service tenancy based access control - Regexes
> ---
>
> Key: TC-462
> URL: https://issues.apache.org/jira/browse/TC-462
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS regexes. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TC-462) Delivery-service tenancy based access control - Regexes

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106392#comment-16106392
 ] 

ASF GitHub Bot commented on TC-462:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237244
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceRegexes.pm ---
@@ -66,6 +73,12 @@ sub index {
return $self->not_found();
}
 
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->forbidden();
--- End diff --

yes, this code was written before we discussed this issue. Will fix in all 
3 PRs


> Delivery-service tenancy based access control - Regexes
> ---
>
> Key: TC-462
> URL: https://issues.apache.org/jira/browse/TC-462
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS regexes. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #751: [TC-462] Ds tenancy validation r...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237244
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceRegexes.pm ---
@@ -66,6 +73,12 @@ sub index {
return $self->not_found();
}
 
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+   if (!$tenant_utils->is_ds_resource_accessible($tenants_data, 
$ds->tenant_id)) {
+   return $self->forbidden();
--- End diff --

yes, this code was written before we discussed this issue. Will fix in all 
3 PRs


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-462) Delivery-service tenancy based access control - Regexes

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106391#comment-16106391
 ] 

ASF GitHub Bot commented on TC-462:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237223
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceRegexes.pm ---
@@ -27,12 +28,18 @@ use Validate::Tiny ':all';
 sub all {
my $self = shift;
 
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
my $rs;
-   if ( _privileged($self) ) {
+   if ( _privileged($self) or $tenant_utils->ignore_ds_users_table()) {
--- End diff --

This code was written earlier...
BTW, I thought you were just concerned about having 2 separate parameters. 
Would you like to merge the "use-tenancy" and "ignore-ds-user-table" functions 
as well?


> Delivery-service tenancy based access control - Regexes
> ---
>
> Key: TC-462
> URL: https://issues.apache.org/jira/browse/TC-462
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS regexes. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #751: [TC-462] Ds tenancy validation r...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237223
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceRegexes.pm ---
@@ -27,12 +28,18 @@ use Validate::Tiny ':all';
 sub all {
my $self = shift;
 
+   my $tenant_utils = Utils::Tenant->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db();
+
my $rs;
-   if ( _privileged($self) ) {
+   if ( _privileged($self) or $tenant_utils->ignore_ds_users_table()) {
--- End diff --

This code was written earlier...
BTW, I thought you were just concerned about having 2 separate parameters. 
Would you like to merge the "use-tenancy" and "ignore-ds-user-table" functions 
as well?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-462) Delivery-service tenancy based access control - Regexes

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TC-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16106390#comment-16106390
 ] 

ASF GitHub Bot commented on TC-462:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237222
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceMatches.pm ---
@@ -28,11 +29,19 @@ sub index {
my $format = $self->param("format") || "";
 
my $rs;
-   if ( _privileged($self) ) {
+   # TO the reviewer: Do we need to override the "is_priviledged" here byt 
the standard "ignore_ds_user_table" flag?
+   # What is the reason of the is_priv test - was someone just dussmissed 
the ds_tmuser table tests
+   if ( _privileged($self)) {
--- End diff --

I was trying to keep the functionality as is, and just add the tenancy 
tests.
Do you know why was it restricted to a privileged user in the first place?


> Delivery-service tenancy based access control - Regexes
> ---
>
> Key: TC-462
> URL: https://issues.apache.org/jira/browse/TC-462
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Reporter: Nir Sopher
>
> We have recently added "tenancy" to the project. 
> With tenancy, every resource have a tenant, where resource can be a 
> delivery-service, a server (future) and even a user.
> We are now starting to enforce access-control based on the resource tenancy. 
> A user can manage a resource only if the resource is under the user tenancy.
> This JIRA deals with another step of "delivery-service as a resource" - 
> enforcing via the API access control on DS regexes. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #751: [TC-462] Ds tenancy validation r...

[nir-sopher]

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/751#discussion_r130237222
  
--- Diff: traffic_ops/app/lib/API/DeliveryServiceMatches.pm ---
@@ -28,11 +29,19 @@ sub index {
my $format = $self->param("format") || "";
 
my $rs;
-   if ( _privileged($self) ) {
+   # TO the reviewer: Do we need to override the "is_priviledged" here byt 
the standard "ignore_ds_user_table" flag?
+   # What is the reason of the is_priv test - was someone just dussmissed 
the ds_tmuser table tests
+   if ( _privileged($self)) {
--- End diff --

I was trying to keep the functionality as is, and just add the tenancy 
tests.
Do you know why was it restricted to a privileged user in the first place?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---