You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by nir-sopher <gi...@git.apache.org> on 2017/08/17 21:00:38 UTC
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
GitHub user nir-sopher opened a pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831
[TC-535] url sig - tenancy checks
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nir-sopher/incubator-trafficcontrol tc-535
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafficcontrol/pull/831.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 #831
----
commit ef783a34b16d6ee53b6520b49e9934a8f295cbb4
Author: nir-sopher <ni...@qwilt.com>
Date: 2017-08-13T15:25:37Z
url sig - 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #831: [TC-535] url sig - tenancy checks
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/831
This test had no issues on my VM.
It can be removed, but I do not understand what is the cause of the failure.
Any additional inputs in the log from the failure?
10x,
Nir
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831#discussion_r134007130
--- Diff: traffic_ops/app/lib/API/DeliveryService/KeysUrlSig.pm ---
@@ -69,14 +86,17 @@ sub copy_url_sig_keys {
else {
return $self->alert("Delivery Service to copy from '$copy_from_xml_id' does not exist.");
}
+ if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $copy_rs->tenant_id)) {
+ return $self->forbidden("Forbidden. Source delivery-service tenant is not available to the user.");
+ }
my $copy_config_file = $self->url_sig_config_file_name($copy_from_xml_id);
my $helper = new Utils::Helper( { mojo => $self } );
my $url_sig_key_values_json;
#verify we can copy keys out
if ( $helper->is_valid_delivery_service($copy_ds_id) ) {
- if ( $is_admin || $helper->is_delivery_service_assigned($copy_ds_id) ) {
+ if ( $is_admin || $helper->is_delivery_service_assigned($copy_ds_id) || $tenant_utils->use_tenancy()) {
--- End diff --
I don't understand this line. If use_tenancy is 1 then it doesn't matter if you are an admin or you have the ds assigned you simply have to have the right tenant.....right?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #831: [TC-535] url sig - tenancy checks
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/831
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/93/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831#discussion_r134098817
--- Diff: traffic_ops/app/lib/API/DeliveryService/KeysUrlSig.pm ---
@@ -69,14 +86,17 @@ sub copy_url_sig_keys {
else {
return $self->alert("Delivery Service to copy from '$copy_from_xml_id' does not exist.");
}
+ if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $copy_rs->tenant_id)) {
+ return $self->forbidden("Forbidden. Source delivery-service tenant is not available to the user.");
+ }
my $copy_config_file = $self->url_sig_config_file_name($copy_from_xml_id);
my $helper = new Utils::Helper( { mojo => $self } );
my $url_sig_key_values_json;
#verify we can copy keys out
if ( $helper->is_valid_delivery_service($copy_ds_id) ) {
- if ( $is_admin || $helper->is_delivery_service_assigned($copy_ds_id) ) {
+ if ( $is_admin || $helper->is_delivery_service_assigned($copy_ds_id) || $tenant_utils->use_tenancy()) {
--- End diff --
Correct, the "|| use_tenancy" in this line come only to disable the legacy tests in case we use tenancy.
If the tenancy is on, we will not even get to this line (we will exit on line 90)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #831: [TC-535] url sig - tenancy checks
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/831
I'm getting a failure when i run your tests. I think it has to do with your set_param_value subroutine...I think...
$ prove -qrp t/api/1.1/deliveryservice/keys_url_sig.t
t/api/1.1/deliveryservice/keys_url_sig.t .. 1/? DBI bind_columns: invalid number of arguments: got handle + 0, expected handle + between 1 and -1
Usage: $h->bind_columns(\$var1 [, \$var2, ...]) at /code/src/github.com/apache/incubator-trafficcontrol/traffic_ops/app/local/lib/perl5/darwin-thread-multi-2level/DBI.pm line 2074.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 6.
Test Summary Report
-------------------
t/api/1.1/deliveryservice/keys_url_sig.t (Wstat: 65280 Tests: 6 Failed: 0)
Non-zero exit status: 255
Parse errors: No plan found in TAP output
Files=1, Tests=6, 2 wallclock secs ( 0.02 usr 0.01 sys + 1.76 cusr 0.53 csys = 2.32 CPU)
Result: FAIL
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831#discussion_r134509030
--- Diff: traffic_ops/app/lib/API/DeliveryService/KeysUrlSig.pm ---
@@ -69,14 +86,17 @@ sub copy_url_sig_keys {
else {
return $self->alert("Delivery Service to copy from '$copy_from_xml_id' does not exist.");
}
+ if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $copy_rs->tenant_id)) {
+ return $self->forbidden("Forbidden. Source delivery-service tenant is not available to the user.");
+ }
my $copy_config_file = $self->url_sig_config_file_name($copy_from_xml_id);
my $helper = new Utils::Helper( { mojo => $self } );
my $url_sig_key_values_json;
#verify we can copy keys out
if ( $helper->is_valid_delivery_service($copy_ds_id) ) {
- if ( $is_admin || $helper->is_delivery_service_assigned($copy_ds_id) ) {
+ if ( $is_admin || $helper->is_delivery_service_assigned($copy_ds_id) || $tenant_utils->use_tenancy()) {
--- End diff --
i see
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831#discussion_r134007236
--- Diff: traffic_ops/app/lib/API/DeliveryService/KeysUrlSig.pm ---
@@ -98,7 +118,7 @@ sub copy_url_sig_keys {
if ( defined($url_sig_key_values_json) ) { # verify we got keys copied
# Admins can always do this, otherwise verify the user
--- End diff --
this comment is not true. admins cannot always do this anymore. not if use_tenancy=1
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831#discussion_r134007307
--- Diff: traffic_ops/app/lib/API/DeliveryService/KeysUrlSig.pm ---
@@ -134,17 +154,22 @@ sub generate {
my $current_user = $self->current_user()->{username};
&log( $self, "Generated new url_sig_keys for " . $xml_id, "APICHANGE" );
+ my $tenant_utils = Utils::Tenant->new($self);
+ my $tenants_data = $tenant_utils->create_tenants_data_from_db();
my $rs = $self->db->resultset("Deliveryservice")->find( { xml_id => $xml_id } );
my $ds_id;
if ( defined($rs) ) {
$ds_id = $rs->id;
+ if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $rs->tenant_id)) {
+ return $self->forbidden("Forbidden. Delivery-service tenant is not available to the user.");
+ }
}
my $helper = new Utils::Helper( { mojo => $self } );
# Admins can always do this, otherwise verify the user
if ( ( defined($rs) && $helper->is_valid_delivery_service($ds_id) ) ) {
- if ( &is_admin($self) || $helper->is_delivery_service_assigned($ds_id) ) {
+ if ( &is_admin($self) || $helper->is_delivery_service_assigned($ds_id) || $tenant_utils->use_tenancy()) {
--- End diff --
same comment as before
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #831: [TC-535] url sig - tenancy checks
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/831
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR-trafficops-test/53/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #831: [TC-535] url sig - tenancy checks
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/831
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/121/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #831: [TC-535] url sig - tenancy checks
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/831
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR-trafficops-test/81/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831#discussion_r134098841
--- Diff: traffic_ops/app/lib/API/DeliveryService/KeysUrlSig.pm ---
@@ -98,7 +118,7 @@ sub copy_url_sig_keys {
if ( defined($url_sig_key_values_json) ) { # verify we got keys copied
# Admins can always do this, otherwise verify the user
if ( $helper->is_valid_delivery_service($ds_id) ) {
- if ( $is_admin || $helper->is_delivery_service_assigned($ds_id) ) {
+ if ( $is_admin || $helper->is_delivery_service_assigned($ds_id) || $tenant_utils->use_tenancy()) {
--- End diff --
Same as before, covered by line 76
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #831: [TC-535] url sig - tenancy checks
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/831
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-traffic_ops-test-PR/55/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/831#discussion_r134007263
--- Diff: traffic_ops/app/lib/API/DeliveryService/KeysUrlSig.pm ---
@@ -98,7 +118,7 @@ sub copy_url_sig_keys {
if ( defined($url_sig_key_values_json) ) { # verify we got keys copied
# Admins can always do this, otherwise verify the user
if ( $helper->is_valid_delivery_service($ds_id) ) {
- if ( $is_admin || $helper->is_delivery_service_assigned($ds_id) ) {
+ if ( $is_admin || $helper->is_delivery_service_assigned($ds_id) || $tenant_utils->use_tenancy()) {
--- End diff --
same comment as before
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #831: [TC-535] url sig - tenancy check...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafficcontrol/pull/831
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---