ensures a user cannot be created/updated to have a role higher than c…

…urrent users role and some ui improvements
apache · Sep 13, 2017 · 82f270a · 82f270a
1 parent 703867d
commit 82f270a
Show file tree

Hide file tree

Showing 11 changed files with 55 additions and 32 deletions.
diff --git a/traffic_ops/app/lib/API/User.pm b/traffic_ops/app/lib/API/User.pm
@@ -715,8 +715,10 @@ sub is_valid {
 		# Checks to perform on all fields
 		checks => [
 
-			# All of these are required
-			[qw/fullName username email role/] => is_required("is required"),
+			fullName	=> [ is_required("is required") ],
+			username	=> [ is_required("is required") ],
+			email		=> [ is_required("is required") ],
+			role		=> [ is_required("is required"), sub { is_valid_role($self, @_) } ],
 
 			# pass2 must be equal to pass
 			localPasswd => sub {
@@ -817,7 +819,7 @@ sub is_valid_role {
 	my $my_role_priv_level = $self->db->resultset("Role")->search( { id => $my_role } )->get_column('priv_level')->single();
 
 	if ( $role_priv_level > $my_role_priv_level ) {
-		return "cannot exceed current user's role";
+		return "cannot exceed current user's privilege level ($my_role_priv_level)";
 	}
 
 	return undef;

diff --git a/traffic_ops/app/lib/UI/Server.pm b/traffic_ops/app/lib/UI/Server.pm
@@ -1035,7 +1035,7 @@ sub postupdatequeue {
 			$message = $host;
 		}
 		$update->update( { upd_pending => $setqueue } );
-		&log( $self, "Flip Update bit ($wording) for " . $message, "OPER" );
+		&log( $self, "Flip Update bit ($wording) for " . $message, "UICHANGE" );
 	}
 	elsif ( defined($cdn) && defined($cachegroup) ) {
 		my @profiles;
@@ -1071,7 +1071,7 @@ sub postupdatequeue {
 		if ( $update->count() > 0 ) {
 			$update->update( { upd_pending => $setqueue } );
 			$self->app->log->debug("Flip Update bit ($wording) for servers in CDN: $cdn, Cachegroup: $cachegroup");
-			&log( $self, "Flip Update bit ($wording) for servers in CDN:" . $cdn . " cachegroup:" . $cachegroup, "OPER" );
+			&log( $self, "Flip Update bit ($wording) for servers in CDN:" . $cdn . " cachegroup:" . $cachegroup, "UICHANGE" );
 		}
 		else {
 			$self->app->log->debug("No Queue Updates for servers in CDN: $cdn, Cachegroup: $cachegroup");

diff --git a/traffic_ops/app/t/api/1.1/user.t b/traffic_ops/app/t/api/1.1/user.t
@@ -51,23 +51,27 @@ $t->get_ok('/api/1.1/user/current.json')->status_is(200)->or( sub { diag $t->tx-
 
 # Test required fields
 $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 } } )
+	json => { user => { username => Test::TestHelper::PORTAL_USER, fullName => 'tom sawyer', email => 'testportal1@kabletown.com', address_line1 => 'newaddress', role => 6 } } )
 	->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "UserProfile 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)" );
+
 # 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 => 2 } } )
+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 } } )
 	->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/level", "success" ),
 	"Verify that the emails are unique";
 
-ok $t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', username => Test::TestHelper::PORTAL_USER, email => '@kabletown.com', role => 2 } } )
+ok $t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', username => Test::TestHelper::PORTAL_USER, email => '@kabletown.com', role => 6 } } )
 	->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/level", "error" ),
 	"Verify that the emails are properly formatted";
 
-ok $t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', username => Test::TestHelper::PORTAL_USER, email => '@kabletown.com', role => 2 } } )
+ok $t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', username => Test::TestHelper::PORTAL_USER, email => '@kabletown.com', role => 6 } } )
 	->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/level", "error" ),
 	"Verify that the usernames are unique";
 
-$t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', email => 'testportal1@kabletown.com', "role" => 4 } } )->status_is(400)
+$t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', email => 'testportal1@kabletown.com', "role" => 6 } } )->status_is(400)
 	->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "username is required" );
 
 $t->post_ok( '/api/1.1/user/current/update', json => { user => { fullName => 'tom sawyer', username => Test::TestHelper::PORTAL_USER, "role" => 6 } } )->status_is(400)

diff --git a/traffic_ops/app/t/api/1.2/user.t b/traffic_ops/app/t/api/1.2/user.t
@@ -61,10 +61,15 @@ sub run_ut {
 
 	# Test required fields
 	$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 => 6 } } )
 		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
 		->json_is( "/alerts/0/text", "UserProfile 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 } } )
+		->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)" );
+
 	#verify tenancy	
 	$t->get_ok('/api/1.2/user/current.json')->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
 		->json_is( "/response/username", $login_user )
@@ -75,7 +80,7 @@ sub run_ut {
 	if (defined($tenant_id)){
 		#verify the update with no "tenant" do not removed the tenant
 		$t->post_ok( '/api/1.2/user/current/update',
-			json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testportal1@kabletown.com', address_line1 => 'newaddress', role => 2 } } )
+			json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testportal1@kabletown.com', address_line1 => 'newaddress', role => 6 } } )
 			->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
 			->json_is( "/alerts/0/text", "UserProfile was successfully updated." );
 		#verify tenancy	
@@ -86,7 +91,7 @@ sub run_ut {
 
 		#cannot removed the tenant on current user
 		$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 => undef, role => 2 } } )
+			json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testportal1@kabletown.com', address_line1 => 'newaddress', tenantId => undef, role => 6 } } )
 			->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
 			->json_is( "/alerts/0/text", "UserProfile was successfully updated." );
 		#verify tenancy	
@@ -97,7 +102,7 @@ sub run_ut {
 
 		#putting the tenant back the update with no "tenant" removed the tenant
 		$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 => 6 } } )
 			->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
 			->json_is( "/alerts/0/text", "UserProfile was successfully updated." );
 		#verify tenancy	
@@ -107,25 +112,25 @@ sub run_ut {
 	}
 
 	# Ensure unique emails
-	ok $t->post_ok( '/api/1.2/user/current/update', json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testportal1@kabletown.com', tenantId => $tenant_id, role => 2 } } )
+	ok $t->post_ok( '/api/1.2/user/current/update', json => { user => { username => $login_user, fullName => 'tom sawyer', email => 'testportal1@kabletown.com', tenantId => $tenant_id, role => 6 } } )
 		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/level", "success" ),
 		"Tenant $tenant: Verify that the emails are unique";
 
-	ok $t->post_ok( '/api/1.2/user/current/update', json => { user => { username => $login_user, fullName => 'tom sawyer', email => '@kabletown.com', tenantId => $tenant_id, role => 2 } } )
+	ok $t->post_ok( '/api/1.2/user/current/update', json => { user => { username => $login_user, fullName => 'tom sawyer', email => '@kabletown.com', tenantId => $tenant_id, role => 6 } } )
 		->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/level", "error" ),
 		"Tenant $tenant: Verify that the emails are properly formatted";
 
-	ok $t->post_ok( '/api/1.2/user/current/update', json => { user => { username => $login_user, fullName => 'tom sawyer', email => '@kabletown.com', tenantId => $tenant_id, role => 2 } } )
+	ok $t->post_ok( '/api/1.2/user/current/update', json => { user => { username => $login_user, fullName => 'tom sawyer', email => '@kabletown.com', tenantId => $tenant_id, role => 6 } } )
 		->status_is(400)->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/level", "error" ),
 		"Tenant $tenant: Verify that the usernames are unique";
 
-	$t->post_ok( '/api/1.2/user/current/update', json => { user => { fullName => 'tom sawyer', email => 'testportal1@kabletown.com', tenantId => $tenant_id, role => 2 } } )->status_is(400)
+	$t->post_ok( '/api/1.2/user/current/update', json => { user => { fullName => 'tom sawyer', email => 'testportal1@kabletown.com', tenantId => $tenant_id, role => 6 } } )->status_is(400)
 		->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "username is required" );
 
-	$t->post_ok( '/api/1.2/user/current/update', json => { user => { fullName => 'tom sawyer', username => $login_user, tenantId => $tenant_id, role => 2 } } )->status_is(400)
+	$t->post_ok( '/api/1.2/user/current/update', json => { user => { fullName => 'tom sawyer', username => $login_user, tenantId => $tenant_id, role => 6 } } )->status_is(400)
 		->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "email is required" );
 
-	$t->post_ok( '/api/1.2/user/current/update', json => { user => { email => 'testportal1@kabletown.com', username => $login_user, tenantId => $tenant_id, role => 2 } } )->status_is(400)
+	$t->post_ok( '/api/1.2/user/current/update', json => { user => { email => 'testportal1@kabletown.com', username => $login_user, tenantId => $tenant_id, role => 6 } } )->status_is(400)
 		->or( sub { diag $t->tx->res->content->asset->{content}; } )->json_is( "/alerts/0/text", "fullName is required" );
 
 	ok $t->post_ok('/api/1.2/user/logout')->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } );

diff --git a/traffic_portal/app/src/common/api/UserService.js b/traffic_portal/app/src/common/api/UserService.js
@@ -61,8 +61,8 @@ var UserService = function(Restangular, $http, $location, $q, authService, httpS
     this.createUser = function(user) {
         return Restangular.service('users').post(user)
             .then(
-                function() {
-                    messageModel.setMessages([ { level: 'success', text: 'User created' } ], true);
+                function(result) {
+                    messageModel.setMessages(result.data.alerts, true);
                     locationUtils.navigateToPath('/admin/users');
                 },
                 function(fault) {
@@ -74,15 +74,15 @@ var UserService = function(Restangular, $http, $location, $q, authService, httpS
     this.updateUser = function(user) {
         return $http.put(ENV.api['root'] + "users/" + user.id, user)
             .then(
-                function() {
+                function(result) {
                     if (userModel.user.id == user.id) {
                         // if you are updating the currently logged in user...
                         userModel.setUser(user);
                     }
-                    messageModel.setMessages([ { level: 'success', text: 'User updated' } ], false);
+                    messageModel.setMessages(result.data.alerts, false);
                 },
-                function() {
-                    messageModel.setMessages([ { level: 'error', text: 'User updated failed' } ], false);
+                function(fault) {
+                    messageModel.setMessages(fault.data.alerts, false);
                 }
             );
     };

diff --git a/traffic_portal/app/src/common/modules/form/user/FormUserController.js b/traffic_portal/app/src/common/modules/form/user/FormUserController.js
@@ -35,6 +35,10 @@ var FormUserController = function(user, $scope, $location, formUtils, stringUtil
 
     $scope.user = user;
 
+    $scope.label = function(role) {
+        return role.name + ' (' + role.privLevel + ')';
+    };
+
     $scope.labelize = stringUtils.labelize;
 
     $scope.viewDeliveryServices = function() {

diff --git a/traffic_portal/app/src/common/modules/form/user/form.user.tpl.html b/traffic_portal/app/src/common/modules/form/user/form.user.tpl.html
@@ -61,9 +61,9 @@
                 </div>
             </div>
             <div class="form-group" ng-class="{'has-error': hasError(userForm.role), 'has-feedback': hasError(userForm.role)}">
-                <label class="control-label col-md-2 col-sm-2 col-xs-12">Role *</label>
+                <label class="control-label col-md-2 col-sm-2 col-xs-12">Role (privilege level) *</label>
                 <div class="col-md-10 col-sm-10 col-xs-12">
-                    <select id="role" name="role" class="form-control" ng-model="user.role" ng-options="role.id as role.name for role in roles" required>
+                    <select id="role" name="role" class="form-control" ng-model="user.role" ng-options="role.id as label(role) for role in roles" required>
                         <option value="">Select...</option>
                     </select>
                     <small class="input-error" ng-show="hasPropertyError(userForm.role, 'required')">Required</small>

diff --git a/traffic_portal/app/src/common/modules/form/user/register/FormRegisterUserController.js b/traffic_portal/app/src/common/modules/form/user/register/FormRegisterUserController.js
@@ -39,6 +39,10 @@ var FormRegisterUserController = function($scope, $location, formUtils, location
 		userService.registerUser(registration);
 	};
 
+	$scope.label = function(role) {
+		return role.name + ' (' + role.privLevel + ')';
+	};
+
 	$scope.navigateToPath = locationUtils.navigateToPath;
 
 	$scope.hasError = formUtils.hasError;

diff --git a/traffic_portal/app/src/common/modules/form/user/register/form.user.register.tpl.html b/traffic_portal/app/src/common/modules/form/user/register/form.user.register.tpl.html
@@ -39,9 +39,9 @@
                 </div>
             </div>
             <div class="form-group" ng-class="{'has-error': hasError(registerForm.role), 'has-feedback': hasError(registerForm.role)}">
-                <label class="control-label col-md-2 col-sm-2 col-xs-12">Role *</label>
+                <label class="control-label col-md-2 col-sm-2 col-xs-12">Role (privilege level) *</label>
                 <div class="col-md-10 col-sm-10 col-xs-12">
-                    <select id="role" name="role" class="form-control" ng-model="registration.role" ng-options="role.id as role.name for role in roles" required>
+                    <select id="role" name="role" class="form-control" ng-model="registration.role" ng-options="role.id as label(role) for role in roles" required>
                         <option value="">Select...</option>
                     </select>
                     <small class="input-error" ng-show="hasPropertyError(registerForm.role, 'required')">Required</small>

diff --git a/traffic_portal/app/src/modules/private/user/UserController.js b/traffic_portal/app/src/modules/private/user/UserController.js
@@ -29,7 +29,7 @@ var UserController = function($scope, $state, $location, $uibModal, formUtils, l
     };
 
     var getRoles = function() {
-        roleService.getRoles()
+        roleService.getRoles({ orderby: 'priv_level DESC' })
             .then(function(result) {
                 $scope.roles = result;
             });

diff --git a/traffic_portal/app/src/modules/private/user/edit/UserEditController.js b/traffic_portal/app/src/modules/private/user/edit/UserEditController.js
@@ -24,6 +24,10 @@ var UserEditController = function($scope) {
         saveLabel: 'Update'
     };
 
+    $scope.label = function(role) {
+        return role.name + ' (' + role.privLevel + ')';
+    };
+
 };
 
 UserEditController.$inject = ['$scope'];