From e5c5fd3d809328e2b9fd0134d3c8a6a669220abc Mon Sep 17 00:00:00 2001 From: chouseknecht Date: Thu, 12 Sep 2013 00:23:02 -0400 Subject: [PATCH] Finished security around user->permissions and team->permissions. Found bugs in permission controller that appeared as though some development had not been finished. User was denied access to edit/view when clicking button but not when entering URL directly in browser. Now non-privileged user can view permssions for Users and Teams to which access is granted. Add and Delete buttons are now hidden where access is disallowed and navigating directly to add page throws an error when Save button clicked. Fixed issue where new CSS for adding asterisk to required fields caused login dialog fields to display multiple asterisks. Modified form generator to show help icon to the left of * on required fields. --- .gitignore | 1 + awx/ui/static/js/controllers/Permissions.js | 71 +++++++++++++-------- awx/ui/static/js/controllers/Teams.js | 7 +- awx/ui/static/js/controllers/Users.js | 14 ++-- awx/ui/static/js/forms/JobTemplates.js | 6 +- awx/ui/static/js/forms/Teams.js | 6 +- awx/ui/static/js/forms/Users.js | 5 +- awx/ui/static/js/lists/Permissions.js | 6 +- awx/ui/static/less/ansible-ui.less | 6 +- awx/ui/static/lib/ansible/form-generator.js | 56 ++++++++++++---- awx/ui/templates/ui/index.html | 8 +-- 11 files changed, 119 insertions(+), 67 deletions(-) diff --git a/.gitignore b/.gitignore index 90997baa23..5089545ffb 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ coverage.xml pep8.txt .vagrant* .tox +nohup.out diff --git a/awx/ui/static/js/controllers/Permissions.js b/awx/ui/static/js/controllers/Permissions.js index 59024325fe..785eeb6765 100644 --- a/awx/ui/static/js/controllers/Permissions.js +++ b/awx/ui/static/js/controllers/Permissions.js @@ -14,7 +14,9 @@ function PermissionsList ($scope, $rootScope, $location, $log, $routeParams, Res var view = GenerateList; var scope = view.inject(list, { mode: 'edit' }); // Inject our view scope.selected = []; - + + CheckAccess({ scope: scope }); + SearchInit({ scope: scope, set: 'permissions', list: list, url: defaultUrl }); PaginateInit({ scope: scope, list: list, url: defaultUrl }); scope.search(list.iterator); @@ -22,15 +24,13 @@ function PermissionsList ($scope, $rootScope, $location, $log, $routeParams, Res LoadBreadCrumbs(); scope.addPermission = function() { - if (CheckAccess()) { + if (scope.PermissionAddAllowed) { $location.path($location.path() + '/add'); } } scope.editPermission = function(id) { - if (CheckAccess()) { - $location.path($location.path() + '/' + id); - } + $location.path($location.path() + '/' + id); } scope.deletePermission = function(id, name) { @@ -45,11 +45,11 @@ function PermissionsList ($scope, $rootScope, $location, $log, $routeParams, Res .error( function(data, status, headers, config) { $('#prompt-modal').modal('hide'); ProcessErrors(scope, data, status, null, - { hdr: 'Error!', msg: 'Call to ' + url + ' failed. DELETE returned status: ' + status }); + { hdr: 'Error!', msg: 'Call to ' + url + ' failed. DELETE returned status: ' + status }); }); }; - if (checkAccess()) { + if (scope.PermissionAddAllowed) { Prompt({ hdr: 'Delete', body: 'Are you sure you want to delete ' + name + '?', action: action @@ -66,7 +66,7 @@ PermissionsList.$inject = [ '$scope', '$rootScope', '$location', '$log', '$route function PermissionsAdd ($scope, $rootScope, $compile, $location, $log, $routeParams, PermissionsForm, GenerateForm, Rest, Alert, ProcessErrors, LoadBreadCrumbs, ClearScope, - GetBasePath, ReturnToCaller, InventoryList, ProjectList, LookUpInit) + GetBasePath, ReturnToCaller, InventoryList, ProjectList, LookUpInit, CheckAccess) { ClearScope('htmlTemplate'); //Garbage collection. Don't leave behind any listeners/watchers from the prior //scope. @@ -79,7 +79,8 @@ function PermissionsAdd ($scope, $rootScope, $compile, $location, $log, $routePa var defaultUrl = GetBasePath(base) + id + '/permissions'; var scope = generator.inject(form, {mode: 'add', related: false}); var master = {}; - + + CheckAccess({ scope: scope }) generator.reset(); LoadBreadCrumbs(); @@ -108,20 +109,25 @@ function PermissionsAdd ($scope, $rootScope, $compile, $location, $log, $routePa // Save scope.formSave = function() { - var data = {}; - for (var fld in form.fields) { - data[fld] = scope[fld]; + if (scope.PermissionAddAllowed) { + var data = {}; + for (var fld in form.fields) { + data[fld] = scope[fld]; + } + var url = (base == 'teams') ? GetBasePath('teams') + id + '/permissions/' : GetBasePath('users') + id + '/permissions/'; + Rest.setUrl(url); + Rest.post(data) + .success( function(data, status, headers, config) { + ReturnToCaller(1); + }) + .error( function(data, status, headers, config) { + ProcessErrors(scope, data, status, PermissionsForm, + { hdr: 'Error!', msg: 'Failed to create new permission. Post returned status: ' + status }); + }); + } + else { + Alert('Access Denied', 'You do not have access to create new permission objects. Please contact a system administrator.', 'alert-danger'); } - var url = (base == 'teams') ? GetBasePath('teams') + id + '/permissions/' : GetBasePath('users') + id + '/permissions/'; - Rest.setUrl(url); - Rest.post(data) - .success( function(data, status, headers, config) { - ReturnToCaller(1); - }) - .error( function(data, status, headers, config) { - ProcessErrors(scope, data, status, PermissionsForm, - { hdr: 'Error!', msg: 'Failed to create new permission. Post returned status: ' + status }); - }); }; // Cancel @@ -146,13 +152,13 @@ function PermissionsAdd ($scope, $rootScope, $compile, $location, $log, $routePa PermissionsAdd.$inject = [ '$scope', '$rootScope', '$compile', '$location', '$log', '$routeParams', 'PermissionsForm', 'GenerateForm', 'Rest', 'Alert', 'ProcessErrors', 'LoadBreadCrumbs', 'ClearScope', 'GetBasePath', - 'ReturnToCaller', 'InventoryList', 'ProjectList', 'LookUpInit' + 'ReturnToCaller', 'InventoryList', 'ProjectList', 'LookUpInit', 'CheckAccess' ]; function PermissionsEdit ($scope, $rootScope, $compile, $location, $log, $routeParams, PermissionsForm, GenerateForm, Rest, Alert, ProcessErrors, LoadBreadCrumbs, ReturnToCaller, - ClearScope, Prompt, GetBasePath, InventoryList, ProjectList, LookUpInit) + ClearScope, Prompt, GetBasePath, InventoryList, ProjectList, LookUpInit, CheckAccess) { ClearScope('htmlTemplate'); //Garbage collection. Don't leave behind any listeners/watchers from the prior //scope. @@ -169,6 +175,7 @@ function PermissionsEdit ($scope, $rootScope, $compile, $location, $log, $routeP var master = {}; var relatedSets = {}; + CheckAccess({ scope: scope }) // Retrieve detail record and prepopulate the form Rest.setUrl(defaultUrl); @@ -215,6 +222,20 @@ function PermissionsEdit ($scope, $rootScope, $compile, $location, $log, $routeP list: ProjectList, field: 'project' }); + + if (!scope.PermissionAddAllowed) { + // If not a privileged user, disable access + $('form[name="permission_form"]').find('select, input, button').each(function(index){ + if ($(this).is('input') || $(this).is('select')) { + $(this).attr('readonly','readonly'); + } + if ( $(this).is('input[type="checkbox"]') || + $(this).is('input[type="radio"]') || + $(this).is('button') ) { + $(this).attr('disabled','disabled'); + } + }); + } }) .error( function(data, status, headers, config) { ProcessErrors(scope, data, status, form, @@ -263,6 +284,6 @@ function PermissionsEdit ($scope, $rootScope, $compile, $location, $log, $routeP PermissionsEdit.$inject = [ '$scope', '$rootScope', '$compile', '$location', '$log', '$routeParams', 'PermissionsForm', 'GenerateForm', 'Rest', 'Alert', 'ProcessErrors', 'LoadBreadCrumbs', 'ReturnToCaller', - 'ClearScope', 'Prompt', 'GetBasePath', 'InventoryList', 'ProjectList', 'LookUpInit' + 'ClearScope', 'Prompt', 'GetBasePath', 'InventoryList', 'ProjectList', 'LookUpInit', 'CheckAccess' ]; diff --git a/awx/ui/static/js/controllers/Teams.js b/awx/ui/static/js/controllers/Teams.js index cefdc45bde..450ce7f05c 100644 --- a/awx/ui/static/js/controllers/Teams.js +++ b/awx/ui/static/js/controllers/Teams.js @@ -274,12 +274,7 @@ function TeamsEdit ($scope, $rootScope, $compile, $location, $log, $routeParams, scope.edit = function(set, id, name) { $rootScope.flashMessage = null; if (set == 'permissions') { - if (scope.PermissionAddAllowed) { - $location.path('/' + base + '/' + $routeParams.team_id + '/' + set + '/' + id); - } - else { - Alert('Access Denied', 'You do not have access to this function. Please contact your system administrator.'); - } + $location.path('/' + base + '/' + $routeParams.team_id + '/' + set + '/' + id); } else { $location.path('/' + set + '/' + id); diff --git a/awx/ui/static/js/controllers/Users.js b/awx/ui/static/js/controllers/Users.js index 6f55c8958c..c71f666047 100644 --- a/awx/ui/static/js/controllers/Users.js +++ b/awx/ui/static/js/controllers/Users.js @@ -201,11 +201,14 @@ function UsersEdit ($scope, $rootScope, $compile, $location, $log, $routeParams, scope.PermissionAddAllowed = false; // After the Organization is loaded, retrieve each related set - scope.$on('userLoaded', function() { + if (scope.removeUserLoaded) { + scope.removeUserLoaded(); + } + scope.removeUserLoaded = scope.$on('userLoaded', function() { for (var set in relatedSets) { scope.search(relatedSets[set].iterator); } - CheckAccess({ scope: scope }); //Does the user have access add Permissions? + CheckAccess({ scope: scope }); //Does the user have access to add/edit Permissions? }); // Retrieve detail record and prepopulate the form @@ -311,12 +314,7 @@ function UsersEdit ($scope, $rootScope, $compile, $location, $log, $routeParams, scope.edit = function(set, id, name) { $rootScope.flashMessage = null; if (set == 'permissions') { - if (scope.PermissionAddAllowed) { - $location.path('/users/' + $routeParams.user_id + '/permissions/' + id); - } - else { - Alert('Access Denied', 'You do not have access to this function. Please contact your system administrator.'); - } + $location.path('/users/' + $routeParams.user_id + '/permissions/' + id); } else { $location.path('/' + set + '/' + id); diff --git a/awx/ui/static/js/forms/JobTemplates.js b/awx/ui/static/js/forms/JobTemplates.js index b5e0c3363b..d55bf78492 100644 --- a/awx/ui/static/js/forms/JobTemplates.js +++ b/awx/ui/static/js/forms/JobTemplates.js @@ -85,10 +85,10 @@ angular.module('JobTemplateFormDefinition', []) forks: { label: 'Forks', id: 'forks-number', - type: 'number', - integer: true, + type: 'text', + /*integer: true, min: 0, - spinner: true, + spinner: true,*/ "default": '0', addRequired: false, editRequired: false, diff --git a/awx/ui/static/js/forms/Teams.js b/awx/ui/static/js/forms/Teams.js index 19430dd042..f791d3449e 100644 --- a/awx/ui/static/js/forms/Teams.js +++ b/awx/ui/static/js/forms/Teams.js @@ -116,7 +116,8 @@ angular.module('TeamFormDefinition', []) ngClick: "add('permissions')", icon: 'icon-plus', label: 'Add', - awToolTip: 'Add a permission for this user' + awToolTip: 'Add a permission for this user', + ngShow: 'PermissionAddAllowed' } }, @@ -157,7 +158,8 @@ angular.module('TeamFormDefinition', []) ngClick: "delete('permissions', \{\{ permission.id \}\}, '\{\{ permission.name \}\}', 'permissions')", icon: 'icon-trash', "class": 'btn-danger', - awToolTip: 'Delete the permission' + awToolTip: 'Delete the permission', + ngShow: 'PermissionAddAllowed' } } diff --git a/awx/ui/static/js/forms/Users.js b/awx/ui/static/js/forms/Users.js index a0236dab10..ccbafe2993 100644 --- a/awx/ui/static/js/forms/Users.js +++ b/awx/ui/static/js/forms/Users.js @@ -164,7 +164,7 @@ angular.module('UserFormDefinition', []) icon: 'icon-plus', label: 'Add', awToolTip: 'Add a permission for this user', - ngShow: 'PermissionAddAllowed == true' + ngShow: 'PermissionAddAllowed' } }, @@ -206,7 +206,8 @@ angular.module('UserFormDefinition', []) ngClick: "delete('permissions', \{\{ permission.id \}\}, '\{\{ permission.name \}\}', 'permissions')", icon: 'icon-trash', "class": 'btn-danger', - awToolTip: 'Delete the permission' + awToolTip: 'Delete the permission', + ngShow: 'PermissionAddAllowed' } } diff --git a/awx/ui/static/js/lists/Permissions.js b/awx/ui/static/js/lists/Permissions.js index 6a34fa5c64..12800746fb 100644 --- a/awx/ui/static/js/lists/Permissions.js +++ b/awx/ui/static/js/lists/Permissions.js @@ -48,7 +48,8 @@ angular.module('PermissionListDefinition', []) mode: 'all', // One of: edit, select, all ngClick: 'addPermission()', "class": 'btn-success btn-sm', - awToolTip: 'Add a new permission' + awToolTip: 'Add a new permission', + ngShow: 'PermissionAddAllowed' } }, @@ -66,7 +67,8 @@ angular.module('PermissionListDefinition', []) ngClick: "deletePermission(\{\{ permission.id \}\},'\{\{ permission.name \}\}')", icon: 'icon-trash', "class": 'btn-xs btn-danger', - awToolTip: 'Delete permission' + awToolTip: 'Delete permission', + ngShow: 'PermissionAddAllowed' } } }); diff --git a/awx/ui/static/less/ansible-ui.less b/awx/ui/static/less/ansible-ui.less index 6af34ce1fd..6eb88905aa 100644 --- a/awx/ui/static/less/ansible-ui.less +++ b/awx/ui/static/less/ansible-ui.less @@ -49,7 +49,7 @@ body { } .prepend-asterisk:before { - content: "\002A"; + content: "\002A\00A0"; } .subtitle { @@ -82,6 +82,10 @@ hr { border-color: #e3e3e3; } +.help { + display: inline-block; +} + .nowrap { white-space: nowrap; } diff --git a/awx/ui/static/lib/ansible/form-generator.js b/awx/ui/static/lib/ansible/form-generator.js index 9cffcc49e7..02099a299e 100644 --- a/awx/ui/static/lib/ansible/form-generator.js +++ b/awx/ui/static/lib/ansible/form-generator.js @@ -93,7 +93,6 @@ angular.module('FormGenerator', ['GeneratorHelpers', 'ngCookies']) $('.form-control[required]').each(function() { var label = $(this).parent().parent().find('label'); if (label && !label.hasClass('prepend-asterisk')) { - console.log('adding prepend'); label.addClass('prepend-asterisk'); } }); @@ -377,21 +376,24 @@ angular.module('FormGenerator', ['GeneratorHelpers', 'ngCookies']) //text fields if (field.type == 'text' || field.type == 'password' || field.type == 'email') { - html += "' + "\n"; + html += "\n"; html += "
\n"; html += (field.clear || field.genMD5) ? "
\n" : ""; - if (field.control === null || field.control === undefined || field.control) { html += "'; + html += "
'; html += field.label + '' + "\n"; + html += "
\n"; html += "
\n"; @@ -536,10 +544,16 @@ angular.module('FormGenerator', ['GeneratorHelpers', 'ngCookies']) } //select field - if (field.type == 'select') { - html += "