You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2018/02/09 17:57:55 UTC
[GitHub] dewrich closed pull request #1857: [Issue-1841] - loosens permissions on ds ssl key management. relies on tenancy.
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-root@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 => 'testportal1@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 => 'testportal1@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 => 'testportal1@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 => 'testportal1@kabletown.com', address_line1 => 'newaddress', tenantId => $tenant_id, role => 2 } } )
+ json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testportal1@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:
users@infra.apache.org
With regards,
Apache Git Services