You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2017/08/02 20:48:55 UTC
[4/6] incubator-trafficcontrol git commit: DS/Server tenancy checks -
additional endpoints
DS/Server tenancy checks - additional endpoints
Project: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/commit/78bd448f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/tree/78bd448f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/diff/78bd448f
Branch: refs/heads/master
Commit: 78bd448f3aca8d4ac82eb27d24c8038ca419e4b0
Parents: 4f48cb3
Author: nir-sopher <ni...@qwilt.com>
Authored: Wed Jul 5 17:16:06 2017 +0300
Committer: Jeremy Mitchell <mi...@gmail.com>
Committed: Wed Aug 2 14:48:21 2017 -0600
----------------------------------------------------------------------
.../app/lib/API/DeliveryServiceServer.pm | 9 +-
traffic_ops/app/lib/API/Deliveryservice.pm | 12 ++-
traffic_ops/app/lib/API/Server.pm | 93 +++++++-------------
traffic_ops/app/t/api/1.2/tenant_access.t | 1 -
4 files changed, 47 insertions(+), 68 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/78bd448f/traffic_ops/app/lib/API/DeliveryServiceServer.pm
----------------------------------------------------------------------
diff --git a/traffic_ops/app/lib/API/DeliveryServiceServer.pm b/traffic_ops/app/lib/API/DeliveryServiceServer.pm
index c44bfae..5ec469c 100644
--- a/traffic_ops/app/lib/API/DeliveryServiceServer.pm
+++ b/traffic_ops/app/lib/API/DeliveryServiceServer.pm
@@ -28,6 +28,11 @@ sub index {
my @data;
my $orderby = $self->param('orderby') || "deliveryservice";
+ #FOR THE REVIEWER - Currently I do not check DS tenancy here.
+ #I assume the operation is of a CDN owner for debug and I would not like to hide data here.
+ # Additionally I assume the operation is protected by "roles"
+ #Also note that the ds/user table is note tested here originally
+
# defaulted pagination and limits because there are 38129 rows in this table and counting...
my $page = $self->param('page') || 1;
my $limit = $self->param('limit') || 20;
@@ -237,7 +242,7 @@ sub remove_server_from_ds {
my $tenant_utils = Utils::Tenant->new($self);
if ( !&is_privileged($self) && !$tenant_utils->ignore_ds_users_table() && !$self->is_delivery_service_assigned($ds_id) ) {
- $self->forbidden("Forbidden. Delivery service not assigned to user.");
+ return $self->forbidden("Forbidden. Delivery service not assigned to user.");
}
my $ds = $self->db->resultset('Deliveryservice')->find( { id => $ds_id } );
@@ -247,7 +252,7 @@ sub remove_server_from_ds {
}
my $tenants_data = $tenant_utils->create_tenants_data_from_db();
if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $ds_tenant)) {
- $self->forbidden("Forbidden. Delivery service not available on user tenancy.");
+ return $self->forbidden("Forbidden. Delivery service not available on user tenancy.");
}
my $ds_server = $self->db->resultset('DeliveryserviceServer')->search( { deliveryservice => $ds_id, server => $server_id }, { prefetch => [ 'deliveryservice', 'server' ] } );
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/78bd448f/traffic_ops/app/lib/API/Deliveryservice.pm
----------------------------------------------------------------------
diff --git a/traffic_ops/app/lib/API/Deliveryservice.pm b/traffic_ops/app/lib/API/Deliveryservice.pm
index 40705c8..ea3cb6b 100644
--- a/traffic_ops/app/lib/API/Deliveryservice.pm
+++ b/traffic_ops/app/lib/API/Deliveryservice.pm
@@ -821,10 +821,17 @@ sub assign_servers {
return $self->alert("Parameter 'serverNames' is required.");
}
- my $dsid = $self->db->resultset('Deliveryservice')->search( { xml_id => $ds_xml_Id } )->get_column('id')->single();
- if ( !defined($dsid) ) {
+ my $ds = $self->db->resultset('Deliveryservice')->find( { xml_id => $ds_xml_Id } );
+ if ( !defined($ds) ) {
return $self->alert( "DeliveryService[" . $ds_xml_Id . "] is not found." );
}
+ my $dsid = $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();
+ }
my @server_ids;
my $svrs = $params->{serverNames};
@@ -1224,6 +1231,7 @@ sub request {
}
}
+
sub is_deliveryservice_request_valid {
my $self = shift;
my $details = shift;
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/78bd448f/traffic_ops/app/lib/API/Server.pm
----------------------------------------------------------------------
diff --git a/traffic_ops/app/lib/API/Server.pm b/traffic_ops/app/lib/API/Server.pm
index 72f6860..f1add40 100644
--- a/traffic_ops/app/lib/API/Server.pm
+++ b/traffic_ops/app/lib/API/Server.pm
@@ -47,7 +47,7 @@ sub index {
( $forbidden, $servers ) = $self->get_servers_by_dsid( $current_user, $ds_id, $status );
}
elsif ( defined $type ) {
- $servers = $self->get_servers_by_type( $current_user, $type, $status );
+ ( $forbidden, $servers ) = $self->get_servers_by_type( $current_user, $type, $status );
}
elsif ( defined $profile_id ) {
( $forbidden, $servers ) = $self->get_servers_by_profile_id($profile_id);
@@ -62,7 +62,7 @@ sub index {
( $forbidden, $servers ) = $self->get_servers_by_phys_loc($phys_loc_id);
}
else {
- $servers = $self->get_servers_by_status( $current_user, $status );
+ ( $forbidden, $servers ) = $self->get_servers_by_status( $current_user, $status );
}
if ( defined($forbidden) ) {
@@ -401,44 +401,25 @@ sub get_servers_by_status {
my $orderby = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
- ##get the data of all DSes, filtered for user if the feature is in use
- my $all_dses;
- my $tenant_utils = Utils::Tenant->new($self);
- my $tenants_data = $tenant_utils->create_tenants_data_from_db();
- if ( &is_privileged($self) || $tenant_utils->ignore_ds_users_table()) {
- $all_dses = $self->db->resultset("Deliveryservice")->search( undef )
- }
- else {
- my $tm_user = $self->db->resultset('TmUser')->search( { username => $current_user } )->single();
- my @base_ds_ids = $self->db->resultset('DeliveryserviceTmuser')->search( { tm_user_id =>
- $tm_user->id } )->get_column('deliveryservice')->all();
- $all_dses = $self->db->resultset("Deliveryservice")->search( { id => { -in => \@base_ds_ids } } )
- }
-
- #filter by tenancy
- my @ds_ids = ();
- while ( my $row = $all_dses->next ) {
- if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $row->tenant_id)) {
- next;
- }
- push( @ds_ids, $row->id );
+ my $forbidden;
+ my $servers;
+ if ( !&is_oper($self) ) {
+ $forbidden = "Forbidden. You must have the operations role to perform this operation.";
+ return ( $forbidden, $servers );
}
- my @ds_servers =
- $self->db->resultset('DeliveryserviceServer')->search( { deliveryservice => { -in => \@ds_ids } } )->get_column('server')->all();
-
- my %criteria = ( 'me.id' => { -in => \@ds_servers } );
+ my %criteria;
if ( defined $status ) {
$criteria{'status.name'} = $status;
}
- my $servers = $self->db->resultset('Server')->search(
+ $servers = $self->db->resultset('Server')->search(
\%criteria, {
prefetch => [ 'cdn', 'cachegroup', 'type', 'profile', 'status', 'phys_location' ],
order_by => 'me.' . $orderby_snakecase,
}
);
- return $servers;
+ return ($forbidden, $servers);
}
sub get_servers_by_dsid {
@@ -451,27 +432,25 @@ sub get_servers_by_dsid {
my $helper = new Utils::Helper( { mojo => $self } );
my $ds = $self->db->resultset('Deliveryservice')->search( { 'me.id' => $ds_id }, { prefetch => ['type'] } )->single();
- if ( !defined($ds) ) {
- return $self->not_found();
- }
-
my $tenant_utils = Utils::Tenant->new($self);
my $tenants_data = $tenant_utils->create_tenants_data_from_db();
my @ds_servers;
my $forbidden;
+ my $servers;
- if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $ds->tenant_id)) {
+ 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 ( &is_privileged($self) || $tenant_utils->ignore_ds_users_table() || $self->is_delivery_service_assigned($ds_id) ) {
@ds_servers = $self->db->resultset('DeliveryserviceServer')->search( { deliveryservice => $ds_id } )->get_column('server')->all();
}
else {
$forbidden = "Forbidden. Delivery service not assigned to user.";
+ return ($forbidden, $servers);
}
- my $servers;
if ( scalar(@ds_servers) ) {
my %criteria = ( -or => [ 'me.id' => { -in => \@ds_servers } ] );
@@ -767,46 +746,26 @@ sub get_servers_by_type {
my $orderby = $self->param('orderby') || "hostName";
my $orderby_snakecase = lcfirst( decamelize($orderby) );
- ##get the data of all DSes, filtered for user if the feature is in use
- my $all_dses;
- my $tenant_utils = Utils::Tenant->new($self);
- my $tenants_data = $tenant_utils->create_tenants_data_from_db();
- if ( &is_privileged($self) || $tenant_utils->ignore_ds_users_table()) {
- $all_dses = $self->db->resultset("Deliveryservice")->search( undef )
- }
- else {
- my $tm_user = $self->db->resultset('TmUser')->search( { username => $current_user } )->single();
- my @base_ds_ids = $self->db->resultset('DeliveryserviceTmuser')->search( { tm_user_id =>
- $tm_user->id } )->get_column('deliveryservice')->all();
- $all_dses = $self->db->resultset("Deliveryservice")->search( { id => { -in => \@base_ds_ids } } )
- }
-
- #filter by tenancy
- my @ds_ids = ();
- while ( my $row = $all_dses->next ) {
- if (!$tenant_utils->is_ds_resource_accessible($tenants_data, $row->tenant_id)) {
- next;
- }
- push( @ds_ids, $row->id );
+ my $forbidden;
+ my $servers;
+ if ( !&is_oper($self) ) {
+ $forbidden = "Forbidden. You must have the operations role to perform this operation.";
+ return ( $forbidden, $servers );
}
- my @ds_servers =
- $self->db->resultset('DeliveryserviceServer')->search( { deliveryservice =>
- { -in => \@ds_ids } } )->get_column('server')->all();
-
- my %criteria = ( 'me.id' => { -in => \@ds_servers }, 'type.name' => $type );
+ my %criteria = ( 'type.name' => $type );
if (defined $status) {
$criteria{'status.name'} = $status;
}
- my $servers = $self->db->resultset('Server')->search(
+ $servers = $self->db->resultset('Server')->search(
\%criteria, {
prefetch => [ 'cdn', 'cachegroup', 'type', 'profile', 'status', 'phys_location' ],
order_by => 'me.' . $orderby_snakecase,
}
);
- return $servers;
+ return ($forbidden, $servers);
}
sub totals {
@@ -993,6 +952,10 @@ sub details_v11 {
}
my $rs_ds_data = $row->deliveryservice_servers;
+ #FOR THE REVIEWER - Currently I do not check DS tenancy here.
+ #I assume the operation is of a CDN owner for debug and I would not like to hide data here.
+ # Additionally I assume the operation is protected by "roles"
+ #Also note that the ds/user table is note tested here originally
while ( my $dsrow = $rs_ds_data->next ) {
push( @{ $serv->{deliveryservices} }, $dsrow->deliveryservice->id );
}
@@ -1067,6 +1030,10 @@ sub details {
}
my $rs_ds_data = $row->deliveryservice_servers;
+ #FOR THE REVIEWER - Currently I do not check DS tenancy here.
+ #I assume the operation is of a CDN owner for debug and I would not like to hide data here.
+ # Additionally I assume the operation is protected by "roles"
+ #Also note that the ds/user table is note tested here originally
while ( my $dsrow = $rs_ds_data->next ) {
push( @{ $serv->{deliveryservices} }, $dsrow->deliveryservice->id );
}
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/78bd448f/traffic_ops/app/t/api/1.2/tenant_access.t
----------------------------------------------------------------------
diff --git a/traffic_ops/app/t/api/1.2/tenant_access.t b/traffic_ops/app/t/api/1.2/tenant_access.t
index e11fe3e..a028f37 100644
--- a/traffic_ops/app/t/api/1.2/tenant_access.t
+++ b/traffic_ops/app/t/api/1.2/tenant_access.t
@@ -113,7 +113,6 @@ ok $t->post_ok( '/login', => form => { u => Test::TestHelper::ADMIN_USER, p => T
my $fixture_num_of_tenants = $t->get_ok('/api/1.2/tenants')->status_is(200)->$responses_counter();
my $fixture_num_of_users = $t->get_ok('/api/1.2/users')->status_is(200)->$responses_counter();
my $fixture_num_of_dses = $t->get_ok('/api/1.2/deliveryservices')->status_is(200)->$responses_counter();
-
ok $t->get_ok('/logout')->status_is(302)->or( sub { diag $t->tx->res->content->asset->{content}; } );