This is an automated email from the ASF dual-hosted git repository. dewrich pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-trafficcontrol.git
commit 2afdf7c72678cee39221acc7eb8f89da4aacaf99 Author: Jeremy Mitchell <mitchell...@gmail.com> AuthorDate: Wed Feb 7 15:02:08 2018 -0700 loosen permissions on delivery service ssl key management (portal role required). tenancy is also used to enforce scope. --- .../traffic_ops_api/v11/deliveryservice.rst | 8 +-- traffic_ops/app/lib/API/DeliveryService/SslKeys.pm | 66 +++++++++++----------- traffic_ops/app/lib/Fixtures/Role.pm | 2 +- traffic_ops/app/lib/Fixtures/TmUser.pm | 26 +++++++++ traffic_ops/app/lib/Test/TestHelper.pm | 3 + traffic_ops/app/lib/UI/Utils.pm | 3 +- .../app/t/api/1.1/deliveryservice/ssl_keys.t | 14 ++--- traffic_ops/app/t/api/1.1/user.t | 4 +- traffic_ops/app/t/api/1.2/user.t | 4 +- 9 files changed, 79 insertions(+), 51 deletions(-) diff --git a/docs/source/development/traffic_ops_api/v11/deliveryservice.rst b/docs/source/development/traffic_ops_api/v11/deliveryservice.rst index dc8bb68..2c13e6d 100644 --- a/docs/source/development/traffic_ops_api/v11/deliveryservice.rst +++ b/docs/source/development/traffic_ops_api/v11/deliveryservice.rst @@ -851,7 +851,7 @@ SSL Keys Authentication Required: Yes - Role(s) Required: Admin + Role(s) Required: Portal **Request Route Parameters** @@ -989,7 +989,7 @@ SSL Keys Authentication Required: Yes - Role Required: Admin + Role Required: PORTAL **Request Route Parameters** @@ -1029,7 +1029,7 @@ SSL Keys Authentication Required: Yes - Role(s) Required: Admin + Role(s) Required: Portal **Request Properties** @@ -1097,7 +1097,7 @@ SSL Keys Authentication Required: Yes - Role(s) Required: Admin + Role(s) Required: Portal **Request Properties** diff --git a/traffic_ops/app/lib/API/DeliveryService/SslKeys.pm b/traffic_ops/app/lib/API/DeliveryService/SslKeys.pm index d2d494a..59d86be 100644 --- a/traffic_ops/app/lib/API/DeliveryService/SslKeys.pm +++ b/traffic_ops/app/lib/API/DeliveryService/SslKeys.pm @@ -261,42 +261,42 @@ sub delete { my $version = $self->param('version'); my $response_container; my $response; - if ( !&is_admin($self) ) { - return $self->alert( { Error => " - You must be an ADMIN to perform this operation!" } ); + + if ( !&is_portal($self) ) { + return $self->forbidden(); + } + + my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single(); + if (!$ds) { + return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_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("Forbidden. Delivery-service tenant is not available to the user."); + } + my $key = $xml_id; + if ($version) { + $key = $key . "-" . $version; + $self->app->log->info("deleting key_type = ssl, key = $key"); + $response_container = $self->riak_delete( "ssl", $key ); + $response = $response_container->{"response"}; } else { - my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => $xml_id })->single(); - if (!$ds) { - return $self->alert( { Error => " - Could not found delivery service with xml_id=$xml_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("Forbidden. Delivery-service tenant is not available to the user."); - } - my $key = $xml_id; - if ($version) { - $key = $key . "-" . $version; - $self->app->log->info("deleting key_type = ssl, key = $key"); - $response_container = $self->riak_delete( "ssl", $key ); - $response = $response_container->{"response"}; - } - else { - #TODO figure out riak searching so we dont have to hardcode "latest" - $key = "$key-latest"; - $self->app->log->info("deleting key_type = ssl, key = $key"); - $response_container = $self->riak_delete( "ssl", $key ); - $response = $response_container->{"response"}; - } + #TODO figure out riak searching so we dont have to hardcode "latest" + $key = "$key-latest"; + $self->app->log->info("deleting key_type = ssl, key = $key"); + $response_container = $self->riak_delete( "ssl", $key ); + $response = $response_container->{"response"}; + } - # $self->app->log->info("delete rc = $rc"); - if ( $response->is_success() ) { - &log( $self, "Deleted ssl keys for Delivery Service $xml_id", "APICHANGE" ); - return $self->success("Successfully deleted ssl keys for $key"); - } - else { - return $self->alert( $response->content ); - } + # $self->app->log->info("delete rc = $rc"); + if ( $response->is_success() ) { + &log( $self, "Deleted ssl keys for Delivery Service $xml_id", "APICHANGE" ); + return $self->success("Successfully deleted ssl keys for $key"); + } + else { + return $self->alert( $response->content ); } } diff --git a/traffic_ops/app/lib/Fixtures/Role.pm b/traffic_ops/app/lib/Fixtures/Role.pm index 5e7fe87..f203933 100644 --- a/traffic_ops/app/lib/Fixtures/Role.pm +++ b/traffic_ops/app/lib/Fixtures/Role.pm @@ -70,7 +70,7 @@ my %definition_for = ( id => 6, name => 'portal', description => 'Portal User', - priv_level => 2, + priv_level => 15, }, }, steering => { diff --git a/traffic_ops/app/lib/Fixtures/TmUser.pm b/traffic_ops/app/lib/Fixtures/TmUser.pm index 35f82e7..d4c2570 100644 --- a/traffic_ops/app/lib/Fixtures/TmUser.pm +++ b/traffic_ops/app/lib/Fixtures/TmUser.pm @@ -225,6 +225,32 @@ my %definition_for = ( }, }, + read_only_root => { + new => 'TmUser', + using => { + id => 1000, + username => 'read-only-root', + tenant_id => 10**9, + role => 2, + uid => '1', + gid => '1', + local_passwd => $local_passwd, + confirm_local_passwd => $local_passwd, + full_name => 'The Read Only User for the "root" tenant', + email => 'read-only-r...@kabletown.com', + new_user => '1', + address_line1 => 'address_line3', + address_line2 => 'address_line4', + city => 'city', + state_or_province => 'state_or_province', + phone_number => '222-222-2222', + postal_code => '80122', + country => 'United States', + token => '', + registration_sent => '1999-01-01 00:00:00', + }, + }, + ); sub get_definition { diff --git a/traffic_ops/app/lib/Test/TestHelper.pm b/traffic_ops/app/lib/Test/TestHelper.pm index d5ce836..6c38f32 100644 --- a/traffic_ops/app/lib/Test/TestHelper.pm +++ b/traffic_ops/app/lib/Test/TestHelper.pm @@ -77,6 +77,9 @@ use constant ADMIN_ROOT_USER_PASSWORD => 'password'; use constant PORTAL_ROOT_USER => 'portal-root'; use constant PORTAL_ROOT_USER_PASSWORD => 'password'; +use constant READ_ONLY_ROOT_USER => 'read-only-root'; +use constant READ_ONLY_ROOT_USER_PASSWORD => 'password'; + sub load_all_fixtures { my $self = shift; my $fixture = shift; diff --git a/traffic_ops/app/lib/UI/Utils.pm b/traffic_ops/app/lib/UI/Utils.pm index 4170abb..e5c1c7d 100644 --- a/traffic_ops/app/lib/UI/Utils.pm +++ b/traffic_ops/app/lib/UI/Utils.pm @@ -42,7 +42,7 @@ use constant ADMIN => 30; our %EXPORT_TAGS = ( 'all' => [ - qw(trim_whitespace is_admin is_oper is_federation is_ldap is_privileged log is_ipaddress is_ip6address is_netmask in_same_net is_hostname admin_status_id type_id type_ids + qw(trim_whitespace is_admin is_oper is_federation is_portal is_ldap is_privileged log is_ipaddress is_ip6address is_netmask in_same_net is_hostname admin_status_id type_id type_ids profile_id profile_ids tm_version tm_url name_version_string is_regexp stash_role navbarpage rascal_hosts_by_cdn is_steering defined_or_default) ] ); @@ -260,7 +260,6 @@ sub is_portal() { return &has_priv( $self, PORTAL ); } - # returns true if the user is logged in via LDAP. sub is_ldap() { my $self = shift; diff --git a/traffic_ops/app/t/api/1.1/deliveryservice/ssl_keys.t b/traffic_ops/app/t/api/1.1/deliveryservice/ssl_keys.t index 40d1ab5..da70983 100644 --- a/traffic_ops/app/t/api/1.1/deliveryservice/ssl_keys.t +++ b/traffic_ops/app/t/api/1.1/deliveryservice/ssl_keys.t @@ -48,7 +48,7 @@ my $hostname = "foober.com"; # PORTAL #NEGATIVE TESTING -- No Privs -ok $t->post_ok( '/api/1.1/user/login', json => { u => Test::TestHelper::PORTAL_USER, p => Test::TestHelper::PORTAL_USER_PASSWORD } )->status_is(200), +ok $t->post_ok( '/api/1.1/user/login', json => { u => Test::TestHelper::READ_ONLY_ROOT_USER, p => Test::TestHelper::READ_ONLY_ROOT_USER_PASSWORD } )->status_is(200), 'Log into the portal user?'; #create @@ -58,16 +58,16 @@ ok $t->post_ok( key => $key, version => $version, } - )->status_is(400)->json_has("Error - You do not have permissions to perform this operation!") + )->status_is(403) ->or( sub { diag $t->tx->res->content->asset->{content}; } ); #get_object -ok $t->get_ok("/api/1.1/deliveryservices/xmlId/$key/sslkeys.json")->status_is(400) - ->json_has("Error - You do not have permissions to perform this operation!")->or( sub { diag $t->tx->res->content->asset->{content}; } ); +ok $t->get_ok("/api/1.1/deliveryservices/xmlId/$key/sslkeys.json")->status_is(403) + ->or( sub { diag $t->tx->res->content->asset->{content}; } ); # #delete -ok $t->get_ok("/api/1.1/deliveryservices/xmlId/$key/sslkeys/delete.json")->status_is(400) - ->json_has("Error - You do not have permissions to perform this operation!")->or( sub { diag $t->tx->res->content->asset->{content}; } ); +ok $t->get_ok("/api/1.1/deliveryservices/xmlId/$key/sslkeys/delete.json")->status_is(403) + ->or( sub { diag $t->tx->res->content->asset->{content}; } ); # # logout ok $t->get_ok('/logout')->status_is(302)->or( sub { diag $t->tx->res->content->asset->{content}; } ); @@ -210,7 +210,7 @@ ok $t->get_ok("/api/1.1/deliveryservices/xmlId/$key/sslkeys.json") my $fake_get_404 = HTTP::Response->new( 404, undef, HTTP::Headers->new, "Not found" ); $fake_lwp->mock( 'get', sub { return $fake_get_404 } ); -ok $t->get_ok("/api/1.1/deliveryservices/xmlId/foo/sslkeys.json")->status_is(400)->json_has("A record for ssl key foo could not be found") +ok $t->get_ok("/api/1.1/deliveryservices/xmlId/foo/sslkeys.json")->status_is(404)->json_has("A record for ssl key foo could not be found") ->or( sub { diag $t->tx->res->content->asset->{content}; } ); # TODO: Implement functionality to satisfy this test? diff --git a/traffic_ops/app/t/api/1.1/user.t b/traffic_ops/app/t/api/1.1/user.t index 16731d3..51ae912 100644 --- a/traffic_ops/app/t/api/1.1/user.t +++ b/traffic_ops/app/t/api/1.1/user.t @@ -55,8 +55,8 @@ $t->post_ok( '/api/1.1/user/current/update', ->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "User profile was successfully updated" ); $t->post_ok( '/api/1.1/user/current/update', - json => { user => { username => Test::TestHelper::PORTAL_USER, fullName => 'tom sawyer', email => 'testport...@kabletown.com', address_line1 => 'newaddress', role => 2 } } ) - ->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "role cannot exceed current user's privilege level (2)" ); + json => { user => { username => Test::TestHelper::PORTAL_USER, fullName => 'tom sawyer', email => 'testport...@kabletown.com', address_line1 => 'newaddress', role => 3 } } ) + ->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "role cannot exceed current user's privilege level (15)" ); # Ensure unique emails ok $t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', username => Test::TestHelper::PORTAL_USER, email => 'testport...@kabletown.com', role => 6 } } ) diff --git a/traffic_ops/app/t/api/1.2/user.t b/traffic_ops/app/t/api/1.2/user.t index 8536d9f..938811c 100644 --- a/traffic_ops/app/t/api/1.2/user.t +++ b/traffic_ops/app/t/api/1.2/user.t @@ -66,9 +66,9 @@ sub run_ut { ->json_is( "/alerts/0/text", "User profile was successfully updated" ); $t->post_ok( '/api/1.2/user/current/update', - json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testport...@kabletown.com', address_line1 => 'newaddress', tenantId => $tenant_id, role => 2 } } ) + json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testport...@kabletown.com', address_line1 => 'newaddress', tenantId => $tenant_id, role => 3 } } ) ->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } ) - ->json_is( "/alerts/0/text", "role cannot exceed current user's privilege level (2)" ); + ->json_is( "/alerts/0/text", "role cannot exceed current user's privilege level (15)" ); #verify tenancy $t->get_ok('/api/1.2/user/current.json')->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } ) -- To stop receiving notification emails like this one, please contact dewr...@apache.org.