dewrich closed pull request #1857: [Issue-1841] - loosens permissions on ds ssl 
key management. relies on tenancy.
URL: https://github.com/apache/incubator-trafficcontrol/pull/1857
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/docs/source/development/traffic_ops_api/v11/deliveryservice.rst 
b/docs/source/development/traffic_ops_api/v11/deliveryservice.rst
index dc8bb68b0c..2c13e6d351 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 716a3f5d8d..59d86beea7 100644
--- a/traffic_ops/app/lib/API/DeliveryService/SslKeys.pm
+++ b/traffic_ops/app/lib/API/DeliveryService/SslKeys.pm
@@ -38,8 +38,8 @@ sub add {
        my $cdn = $self->req->json->{cdn};
        my $deliveryservice = $self->req->json->{deliveryservice};
 
-       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 => 
$deliveryservice })->single();
@@ -92,9 +92,10 @@ sub generate {
        my $deliveryservice = $self->req->json->{deliveryservice};
        my $tmp_location = "/var/tmp";
 
-       if ( !&is_admin($self) ) {
-               return $self->alert( { Error => " - You must be an ADMIN to 
perform this operation!" } );
+       if ( !&is_portal($self) ) {
+               return $self->forbidden();
        }
+
        if (defined($deliveryservice)) {
                my $ds = $self->db->resultset('Deliveryservice')->search( { 
xml_id => $deliveryservice })->single();
                if (!$ds) {
@@ -142,42 +143,42 @@ sub view_by_xml_id {
                $decode = 0;
        }
 
-       if ( !&is_admin($self) ) {
-               return $self->alert( { Error => " - You must be an ADMIN to 
perform this operation!" } );
+       if ( !$version ) {
+               $version = 'latest';
        }
-       else {
-               if ( !$version ) {
-                       $version = 'latest';
-               }
-               my $key = "$xml_id-$version";
-               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 $response_container = $self->riak_get( "ssl", $key );
-               my $response = $response_container->{"response"};
 
+       if ( !&is_portal($self) ) {
+               return $self->forbidden();
+       }
 
-               if ( $response->is_success() ){
-                       my $toSend = decode_json( $response->content );
+       my $key = "$xml_id-$version";
+       my $ds = $self->db->resultset('Deliveryservice')->search( { xml_id => 
$xml_id })->single();
+       if (!$ds) {
+               return $self->not_found();
+       }
+       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 $response_container = $self->riak_get( "ssl", $key );
+       my $response = $response_container->{"response"};
 
-                       if ( $decode ){
-                               $toSend->{certificate}->{csr} = 
decode_base64($toSend->{certificate}->{csr});
-                               $toSend->{certificate}->{crt} = 
decode_base64($toSend->{certificate}->{crt});
-                               $toSend->{certificate}->{key} = 
decode_base64($toSend->{certificate}->{key});
-                       }
 
-               
-                       $self->success( $toSend )
+       if ( $response->is_success() ){
+               my $toSend = decode_json( $response->content );
 
-               } else {
-                       $self->success({}, " - A record for ssl key $key could 
not be found. ");
+               if ( $decode ){
+                       $toSend->{certificate}->{csr} = 
decode_base64($toSend->{certificate}->{csr});
+                       $toSend->{certificate}->{crt} = 
decode_base64($toSend->{certificate}->{crt});
+                       $toSend->{certificate}->{key} = 
decode_base64($toSend->{certificate}->{key});
                }
+
+
+               $self->success( $toSend )
+
+       } else {
+               $self->success({}, " - A record for ssl key $key could not be 
found. ");
        }
 }
 
@@ -260,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 5e7fe872ba..f2039333f7 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 35f82e7961..d4c25707e2 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 d5ce8360a6..6c38f32539 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 30fff2c122..e5c1c7dde7 100644
--- a/traffic_ops/app/lib/UI/Utils.pm
+++ b/traffic_ops/app/lib/UI/Utils.pm
@@ -36,12 +36,13 @@ our @ISA = qw(Exporter);
 
 use constant READ       => 10;
 use constant FEDERATION => 15;
+use constant PORTAL            => 15;
 use constant OPER       => 20;
 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)
        ]
 );
@@ -253,6 +254,12 @@ sub is_admin() {
        return &has_priv( $self, ADMIN );
 }
 
+sub is_portal() {
+       my $self = shift;
+
+       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 40d1ab5f56..da709833d0 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 16731d3864..51ae9126a1 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 8536d9fb4e..938811c890 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}; } )


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to