Tenant is the resource owbner of itself + move some 403 to 400 on tenancy checks
Project: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/commit/13b60e64 Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/tree/13b60e64 Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/diff/13b60e64 Branch: refs/heads/master Commit: 13b60e6463e46ac25f107121e888b78293e8967e Parents: 2be62a6 Author: nir-sopher <n...@qwilt.com> Authored: Tue Jun 27 06:46:21 2017 +0300 Committer: Jeremy Mitchell <mitchell...@gmail.com> Committed: Tue Jul 18 12:12:32 2017 -0600 ---------------------------------------------------------------------- traffic_ops/app/lib/API/Tenant.pm | 51 ++++++++++++++++----------------- traffic_ops/app/t/api/1.2/tenant.t | 4 +-- 2 files changed, 26 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/13b60e64/traffic_ops/app/lib/API/Tenant.pm ---------------------------------------------------------------------- diff --git a/traffic_ops/app/lib/API/Tenant.pm b/traffic_ops/app/lib/API/Tenant.pm index 8600871..f1c12e5 100644 --- a/traffic_ops/app/lib/API/Tenant.pm +++ b/traffic_ops/app/lib/API/Tenant.pm @@ -108,18 +108,21 @@ sub update { } if ( $params->{name} ne $self->getTenantName($id) ) { - my $name = $params->{name}; + my $name = $params->{name}; my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single(); if ($existing) { return $self->alert("A tenant with name \"$name\" already exists."); } - } - + } my $tenant_utils = Utils::Tenant->new($self); my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef); - if ( $tenant_utils->is_root_tenant($tenants_data, $id) ) { + if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $id)) { + return $self->forbidden(); #Current owning tenant is not under user's tenancy + } + + if ( $tenant_utils->is_root_tenant($tenants_data, $id) ) { return $self->alert("Root tenant cannot be updated."); } @@ -137,23 +140,19 @@ sub update { return $self->alert("Root tenant cannot be in-active."); } - #this is a write operation, allowed only by parents of the tenant (which are the owners of the resource of type tenant) - my $current_resource_tenancy = $self->db->resultset('Tenant')->search( { id => $id } )->get_column('parent_id')->single(); - - if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $current_resource_tenancy)) { - return $self->forbidden(); #Current owning tenant is not under user's tenancy - } - - if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $params->{parentId})) { - return $self->forbidden(); #Parent tenant to be set is not under user's tenancy - } - - if ($params->{parentId} != $tenant->parent) { #parent replacement + if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $tenant->parent)) { + #Current owning tenant is not under user's tenancy + return $self>alert("Invalid parent tenant change. The current tenant parent is not avaialble for you to edit"); + } if (!defined($tenant_utils->get_tenant_by_id($tenants_data, $params->{parentId}))) { return $self->alert("Parent tenant does not exists."); } + if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $params->{parentId})) { + #Parent tenant to be set is not under user's tenancy + return $self->alert("Invalid parent tenant. This tenant is not available to you for parent assignment."); + } my $parent_depth = $tenant_utils->get_tenant_heirarchy_depth($tenants_data, $params->{parentId}); if (!defined($parent_depth)) { @@ -242,19 +241,19 @@ sub create { if ( !defined($parent_id) ) { return $self->alert("Parent Id is required."); } - + my $tenant_utils = Utils::Tenant->new($self); my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef); - if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $params->{parentId})) { - return $self->forbidden(); #Parent tenant to be set is not under user's tenancy - } - if (!defined($tenant_utils->get_tenant_by_id($tenants_data, $params->{parentId}))) { return $self->alert("Parent tenant does not exists."); } - - my $parent_depth = $tenant_utils->get_tenant_heirarchy_depth($tenants_data, $params->{parentId}); + + if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $params->{parentId})) { + return $self->alert("Invalid parent tenant. This tenant is not available to you for parent assignment."); + } + + my $parent_depth = $tenant_utils->get_tenant_heirarchy_depth($tenants_data, $params->{parentId}); if (!defined($parent_depth)) { @@ -326,13 +325,11 @@ sub delete { return $self->not_found(); } - my $parent_tenant = $tenant->parent_id; - my $tenant_utils = Utils::Tenant->new($self); my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef); - if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $parent_tenant)) { - return $self->forbidden(); #Parent tenant is not under user's tenancy + if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, $id)) { + return $self->forbidden(); #tenant is not under user's tenancy } my $name = $tenant->name; http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/13b60e64/traffic_ops/app/t/api/1.2/tenant.t ---------------------------------------------------------------------- diff --git a/traffic_ops/app/t/api/1.2/tenant.t b/traffic_ops/app/t/api/1.2/tenant.t index 7f18ede..a6193c0 100644 --- a/traffic_ops/app/t/api/1.2/tenant.t +++ b/traffic_ops/app/t/api/1.2/tenant.t @@ -344,11 +344,11 @@ ok $t->post_ok( '/login', => form => { u => Test::TestHelper::ADMIN_USER, p => T ->or( sub { diag $t->tx->res->content->asset->{content}; } ), 'Should login?'; ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => { - "name" => "tenantA", "parentId" => $root_tenant_id })->status_is(403)->or( sub { diag $t->tx->res->content->asset->{content}; } ); + "name" => "tenantA", "parentId" => $root_tenant_id })->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } ); ok $t->put_ok('/api/1.2/tenants/' . $root_tenant_id => {Accept => 'application/json'} => json => { "name" => "rooty", "active" => 1, "parentId" => undef}) - ->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } ); + ->status_is(403)->or( sub { diag $t->tx->res->content->asset->{content}; } ); #no tenants in the list ok $t->get_ok("/api/1.2/tenants")->status_is(200)
