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/07/19 21:56:09 UTC
[08/11] incubator-trafficcontrol git commit: User tenancy - fixing
API return codes in some cases 403->400
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 <ni...@qwilt.com>
Authored: Tue Jun 27 09:57:18 2017 +0300
Committer: Jeremy Mitchell <mi...@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();