tenancy checks - return 403
Project: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/commit/512d1ef5 Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/tree/512d1ef5 Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/diff/512d1ef5 Branch: refs/heads/master Commit: 512d1ef51782453cc0f2e23b2b6f3c779d0d872c Parents: ed169cd Author: nir-sopher <nirsop...@gmail.com> Authored: Sun Jun 4 20:14:25 2017 +0300 Committer: Jeremy Mitchell <mitchell...@gmail.com> Committed: Tue Jul 18 12:12:32 2017 -0600 ---------------------------------------------------------------------- traffic_ops/app/lib/API/Tenant.pm | 13 ++++--- traffic_ops/app/t/api/1.2/tenant.t | 65 ++++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/512d1ef5/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 f224967..1c583d8 100644 --- a/traffic_ops/app/lib/API/Tenant.pm +++ b/traffic_ops/app/lib/API/Tenant.pm @@ -83,6 +83,9 @@ sub show { } ); } + else { + return $self->forbidden(); + } } $self->success( \@data ); } @@ -144,11 +147,11 @@ sub update { } if (!$tenant_utils->is_tenant_resource_writeable($tenants_data, $current_resource_tenancy)) { - return $self->alert("Current owning tenant is not under user's tenancy."); + return $self->forbidden(); #Current owning tenant is not under user's tenancy } if (!$tenant_utils->is_tenant_resource_writeable($tenants_data, $params->{parentId})) { - return $self->alert("Parent tenant to be set is not under user's tenancy."); + return $self->forbidden(); #Parent tenant to be set is not under user's tenancy } @@ -250,7 +253,7 @@ sub create { my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef); if (!$tenant_utils->is_tenant_resource_writeable($tenants_data, $params->{parentId})) { - return $self->alert("Parent tenant to be set is not under user's tenancy."); + return $self->forbidden(); #Parent tenant to be set is not under user's tenancy } if (!defined($tenant_utils->get_tenant($tenants_data, $params->{parentId}))) { @@ -329,13 +332,13 @@ sub delete { return $self->not_found(); } - my $parent_tenant = $tenant->parent_id; + my $parent_tenant = $tenant->parent_id; my $tenant_utils = UI::TenantUtils->new($self); my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef); if (!$tenant_utils->is_tenant_resource_writeable($tenants_data, $parent_tenant)) { - return $self->alert("Parent tenant is not under user's tenancy."); + return $self->forbidden(); #Parent tenant is not under user's tenancy } my $name = $tenant->name; http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/512d1ef5/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 903b2f1..ac722a7 100644 --- a/traffic_ops/app/t/api/1.2/tenant.t +++ b/traffic_ops/app/t/api/1.2/tenant.t @@ -39,7 +39,7 @@ ok $t->post_ok( '/login', => form => { u => Test::TestHelper::ADMIN_ROOT_USER, p ->or( sub { diag $t->tx->res->content->asset->{content}; } ), 'Should login?'; #verifying the basic cfg -$t->get_ok("/api/1.2/tenants")->status_is(200)->json_is( "/response/0/name", "root" )->or( sub { diag $t->tx->res->content->asset->{content}; } );; +ok $t->get_ok("/api/1.2/tenants")->status_is(200)->json_is( "/response/0/name", "root" )->or( sub { diag $t->tx->res->content->asset->{content}; } );; my $root_tenant_id = &get_tenant_id('root'); @@ -149,7 +149,7 @@ my $tenantD_id = &get_tenant_id('tenantD'); my $tenantE_id = &get_tenant_id('tenantE'); #list tenants- verify heirachic order - order by id -$t->get_ok("/api/1.2/tenants?orderby=id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants?orderby=id")->status_is(200) ->json_is( "/response/0/id", $root_tenant_id ) ->json_is( "/response/1/id", $tenantA_id) ->json_is( "/response/2/id", $tenantD_id) @@ -157,7 +157,7 @@ $t->get_ok("/api/1.2/tenants?orderby=id")->status_is(200) ->json_is( "/response/4/id", $tenantB_id)->or( sub { diag $t->tx->res->content->asset->{content}; } );; #list tenants- verify heirachic order - order by name -$t->get_ok("/api/1.2/tenants?orderby=name")->status_is(200) +ok $t->get_ok("/api/1.2/tenants?orderby=name")->status_is(200) ->json_is( "/response/0/id", $root_tenant_id ) ->json_is( "/response/2/id", $tenantA_id) ->json_is( "/response/3/id", $tenantD_id) @@ -165,7 +165,7 @@ $t->get_ok("/api/1.2/tenants?orderby=name")->status_is(200) ->json_is( "/response/1/id", $tenantB_id)->or( sub { diag $t->tx->res->content->asset->{content}; } );; #list tenants- verify heirachic order - order by name (default) -$t->get_ok("/api/1.2/tenants")->status_is(200) +ok $t->get_ok("/api/1.2/tenants")->status_is(200) ->json_is( "/response/0/id", $root_tenant_id ) ->json_is( "/response/2/id", $tenantA_id) ->json_is( "/response/3/id", $tenantD_id) @@ -173,27 +173,27 @@ $t->get_ok("/api/1.2/tenants")->status_is(200) ->json_is( "/response/1/id", $tenantB_id)->or( sub { diag $t->tx->res->content->asset->{content}; } );; #tenants heirarchy- test depth and height -$t->get_ok("/api/1.2/tenants/$root_tenant_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$root_tenant_id")->status_is(200) ->json_is( "/response/0/heirarchyDepth", 0) ->json_is( "/response/0/heirarchyHeight", 2) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; -$t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) ->json_is( "/response/0/heirarchyDepth", 1) ->json_is( "/response/0/heirarchyHeight", 1) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; -$t->get_ok("/api/1.2/tenants/$tenantB_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$tenantB_id")->status_is(200) ->json_is( "/response/0/heirarchyDepth", 1) ->json_is( "/response/0/heirarchyHeight", 0) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; -$t->get_ok("/api/1.2/tenants/$tenantD_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$tenantD_id")->status_is(200) ->json_is( "/response/0/heirarchyDepth", 2) ->json_is( "/response/0/heirarchyHeight", 0) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; -$t->get_ok("/api/1.2/tenants/$tenantE_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$tenantE_id")->status_is(200) ->json_is( "/response/0/heirarchyDepth", 2) ->json_is( "/response/0/heirarchyHeight", 0) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; @@ -204,21 +204,21 @@ ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id => {Accept => 'application/json "active" => 1, "parentId" => $tenantB_id, name => "tenantA2"}) ->status_is(200); -$t->get_ok("/api/1.2/tenants/$tenantB_id")->status_is(200) - ->json_is( "/response/0/heirarchyDepth", 2) - ->json_is( "/response/0/heirarchyHeight", 3) +ok $t->get_ok("/api/1.2/tenants/$tenantB_id")->status_is(200) + ->json_is( "/response/0/heirarchyDepth", 1) + ->json_is( "/response/0/heirarchyHeight", 2) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; -$t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) ->json_is( "/response/0/parentId", $tenantB_id) - ->json_is( "/response/0/heirarchyDepth", 3) - ->json_is( "/response/0/heirarchyHeight", 2) + ->json_is( "/response/0/heirarchyDepth", 2) + ->json_is( "/response/0/heirarchyHeight", 1) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; -$t->get_ok("/api/1.2/tenants/$tenantD_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$tenantD_id")->status_is(200) ->json_is( "/response/0/parentId", $tenantA_id) - ->json_is( "/response/0/heirarchyDepth", 4) - ->json_is( "/response/0/heirarchyHeight", 1) + ->json_is( "/response/0/heirarchyDepth", 3) + ->json_is( "/response/0/heirarchyHeight", 0) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; @@ -239,10 +239,10 @@ ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id => {Accept => 'application/json "active" => 1, "parentId" => $root_tenant_id, name => "tenantA2"}) ->status_is(200); -$t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) +ok $t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) ->json_is( "/response/0/parentId", $root_tenant_id) - ->json_is( "/response/0/heirarchyDepth", 2) - ->json_is( "/response/0/heirarchyHeight", 2) + ->json_is( "/response/0/heirarchyDepth", 1) + ->json_is( "/response/0/heirarchyHeight", 1) ->or( sub { diag $t->tx->res->content->asset->{content}; } );; #cannot delete a tenant that have children @@ -269,6 +269,29 @@ ok $t->delete_ok('/api/1.2/tenants/' . 10**9)->status_is(400) ->or( sub { diag $t->tx->res->content->asset->{content}; } ); ok $t->get_ok('/logout')->status_is(302)->or( sub { diag $t->tx->res->content->asset->{content}; } ); + +# verify a null tenant user cannot access tenats resources he shouldn't +ok $t->post_ok( '/login', => form => { u => Test::TestHelper::ADMIN_USER, p => Test::TestHelper::ADMIN_USER_PASSWORD } )->status_is(302) + ->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}; } ); + +ok $t->put_ok('/api/1.2/tenants/' . $root_tenant_id => {Accept => 'application/json'} => json => { + "name" => "rooty", "active" => 1, "parentId" => undef}) + ->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) + ->json_is( "/response/0/id", undef) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +ok $t->get_ok("/api/1.2/tenants/$root_tenant_id")->status_is(403) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +ok $t->get_ok('/logout')->status_is(302)->or( sub { diag $t->tx->res->content->asset->{content}; } ); + + $dbh->disconnect(); done_testing();
