User tenancy - fixing API return codes in some cases 403->400
Project: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/commit/54200a52 Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/tree/54200a52 Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/diff/54200a52 Branch: refs/heads/master Commit: 54200a52df8e4c56a923230fcb0861f3a522064a Parents: 9b95ab9 Author: nir-sopher <n...@qwilt.com> Authored: Tue Jun 27 09:57:18 2017 +0300 Committer: Jeremy Mitchell <mitchell...@gmail.com> Committed: Wed Jul 19 15:55:31 2017 -0600 ---------------------------------------------------------------------- traffic_ops/app/lib/API/User.pm | 6 +++--- traffic_ops/app/t/api/1.2/tenant_access.t | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/54200a52/traffic_ops/app/lib/API/User.pm ---------------------------------------------------------------------- diff --git a/traffic_ops/app/lib/API/User.pm b/traffic_ops/app/lib/API/User.pm index 5b09d74..0da0ac1 100644 --- a/traffic_ops/app/lib/API/User.pm +++ b/traffic_ops/app/lib/API/User.pm @@ -125,7 +125,7 @@ sub show { while ( my $row = $rs_data->next ) { if (!$tenant_utils->is_user_resource_accessible($tenants_data, $row->tenant_id)) { - return $self->forbidden();#FOR the reviewer - what is the correct response - 403 or 200 with empty list? + return $self->forbidden(); } push( @data, { @@ -182,7 +182,7 @@ sub update { } if (!$tenant_utils->is_user_resource_accessible($tenants_data, $tenant_id)) { #no access to target tenancy - return $self->forbidden(); + return $self->alert("Invalid tenant. This tenant is not available to you for assignment."); } my ( $is_valid, $result ) = $self->is_valid( $params, $user_id ); @@ -264,7 +264,7 @@ sub create { my $tenant_id = exists( $params->{tenantId} ) ? $params->{tenantId} : $tenant_utils->current_user_tenant(); my $tenants_data = $tenant_utils->create_tenants_data_from_db(); if (!$tenant_utils->is_user_resource_accessible($tenants_data, $tenant_id)) { - return $self->forbidden(); + return $self->alert("Invalid tenant. This tenant is not available to you for assignment."); } my ( $is_valid, $result ) = $self->is_valid( $params, 0 ); http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/54200a52/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 b764502..fddef17 100644 --- a/traffic_ops/app/t/api/1.2/tenant_access.t +++ b/traffic_ops/app/t/api/1.2/tenant_access.t @@ -350,6 +350,14 @@ sub logout_from_tenant_admin { ok $t->get_ok('/logout')->status_is(302)->or( sub { diag $t->tx->res->content->asset->{content}; } ); } +sub is_tenant_active{ + my $tenant_name = shift; + if ($tenant_name eq "none"){ + return 1; + } + return $schema->resultset('Tenant')->find( { name => $tenant_name } )->active; +} + sub test_user_resource_read_allow_access { my $login_tenant = shift; my $resource_tenant = shift; @@ -453,6 +461,7 @@ sub test_user_resource_write_block_access { my $tenants_data = shift; login_to_tenant_admin($login_tenant, $tenants_data); + my $is_login_tenant_active = is_tenant_active($login_tenant); #adding a user my $new_username="test_user"; ok $t->post_ok('/api/1.2/users' => {Accept => 'application/json'} => json => { @@ -464,7 +473,8 @@ sub test_user_resource_write_block_access { "role" => 4, "tenantId" => $tenants_data->{$resource_tenant}->{'id'}, }) - ->status_is(403)->or( sub { diag $t->tx->res->content->asset->{content}; } ) + ->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } ) + ->json_is( "/alerts/0/text" => "Invalid tenant. This tenant is not available to you for assignment." ) , 'Cannot add user: login tenant:'.$login_tenant.' resource tenant: '.$resource_tenant.'?'; @@ -544,10 +554,11 @@ sub test_user_resource_write_block_access { logout_from_tenant_admin(); login_to_tenant_admin($login_tenant, $tenants_data); - #changing only its tenancy + #changing only its tenancy (403 if the basic resource cannot be accessed, 400 if the change is invalid) $response2edit2->{"tenantId"} = $tenants_data->{$resource_tenant}->{'id'}; ok $t->put_ok('/api/1.2/users/'.$new_userid2 => {Accept => 'application/json'} => json => $response2edit2) - ->status_is(403)->or( sub { diag $t->tx->res->content->asset->{content}; } ) + ->status_is($is_login_tenant_active ? 400 : 403)->or( sub { diag $t->tx->res->content->asset->{content}; } ) + ->json_is( "/alerts/0/text" => $is_login_tenant_active ? "Invalid tenant. This tenant is not available to you for assignment." : "Forbidden") , 'Cannot change user tenant to the target resource tenant: login tenant:'.$login_tenant.' resource tenant: '.$resource_tenant.'?'; logout_from_tenant_admin();
