Re: [Freeipa-devel] [PATCH]admiyo-freeipa-0047-tab-objects.patch
On 09/29/2010 12:44 AM, Endi Sukma Dewata wrote: - Adam Youngayo...@redhat.com wrote: tab objects Convert the tab lists to arrays of objects with four potential fields: tab[0] - tab.name tab[1] - tab.label tab[2] - tab.setup or tab.children Added unit tests ACK, but could you add the unit tests into the index.html and all_tests.html? I'll add unit tests too and rebase my patch on your patch. Thanks. diff --git a/install/static/test/all_tests.html b/install/static/test/all_tests.html index 687bea1..93c4de2 100644 --- a/install/static/test/all_tests.html +++ b/install/static/test/all_tests.html @@ -5,15 +5,19 @@ link rel=stylesheet href=qunit.css type=text/css media=screen script type=text/javascript src=qunit.js/script script type=text/javascript src=../jquery.js/script +script type=text/javascript src=../jquery.ba-bbq.js/script +script type=text/javascript src=../jquery-ui.js/script script type=text/javascript src=../ipa.js/script script type=text/javascript src=../details.js/script script type=text/javascript src=../search.js/script script type=text/javascript src=../add.js/script script type=text/javascript src=../entity.js/script script type=text/javascript src=../associate.js/script +script type=text/javascript src=../navigation.js/script script type=text/javascript src=ipa_tests.js/script script type=text/javascript src=entity_tests.js/script script type=text/javascript src=association_tests.js/script +script type=text/javascript src=navigation_tests.js/script /head body h1 id=qunit-headerComplete Test Suite/h1 diff --git a/install/static/test/index.html b/install/static/test/index.html index 14ca7f0..581be24 100644 --- a/install/static/test/index.html +++ b/install/static/test/index.html @@ -27,6 +27,7 @@ lia href=ipa_tests.htmlCore Test Suite/a lia href=entity_tests.htmlEntity Test Suite/a lia href=association_tests.htmlAssociation Test Suite/a +lia href=navigation_tests.htmlNavigation Test Suite/a /ul /div -- Endi S. Dewata Yeah. I'll do that before checkin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH]admiyo-freeipa-0047-tab-objects.patch
On 09/29/2010 12:44 AM, Endi Sukma Dewata wrote: - Adam Youngayo...@redhat.com wrote: tab objects Convert the tab lists to arrays of objects with four potential fields: tab[0] - tab.name tab[1] - tab.label tab[2] - tab.setup or tab.children Added unit tests ACK, but could you add the unit tests into the index.html and all_tests.html? I'll add unit tests too and rebase my patch on your patch. Thanks. Done, and pushed to master. diff --git a/install/static/test/all_tests.html b/install/static/test/all_tests.html index 687bea1..93c4de2 100644 --- a/install/static/test/all_tests.html +++ b/install/static/test/all_tests.html @@ -5,15 +5,19 @@ link rel=stylesheet href=qunit.css type=text/css media=screen script type=text/javascript src=qunit.js/script script type=text/javascript src=../jquery.js/script +script type=text/javascript src=../jquery.ba-bbq.js/script +script type=text/javascript src=../jquery-ui.js/script script type=text/javascript src=../ipa.js/script script type=text/javascript src=../details.js/script script type=text/javascript src=../search.js/script script type=text/javascript src=../add.js/script script type=text/javascript src=../entity.js/script script type=text/javascript src=../associate.js/script +script type=text/javascript src=../navigation.js/script script type=text/javascript src=ipa_tests.js/script script type=text/javascript src=entity_tests.js/script script type=text/javascript src=association_tests.js/script +script type=text/javascript src=navigation_tests.js/script /head body h1 id=qunit-headerComplete Test Suite/h1 diff --git a/install/static/test/index.html b/install/static/test/index.html index 14ca7f0..581be24 100644 --- a/install/static/test/index.html +++ b/install/static/test/index.html @@ -27,6 +27,7 @@ lia href=ipa_tests.htmlCore Test Suite/a lia href=entity_tests.htmlEntity Test Suite/a lia href=association_tests.htmlAssociation Test Suite/a +lia href=navigation_tests.htmlNavigation Test Suite/a /ul /div -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 548 use consistent CA nickname
On 09/28/2010 11:11 PM, Rob Crittenden wrote: Use consistent, specific nickname for the IPA CA certificate. Also fix some imports for sha. We have a compat module for it, use it. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 550 estimated install times
Add estimated install times to the installation. I also log a duration for each step in /var/log/ipaserver-install.log if anyone wants to compare their times to mine. ticket 139 rob freeipa-550-install.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 549 remove reliance on admin user
On 09/29/2010 01:55 PM, Rob Crittenden wrote: Change the finals aci so that the login admin is no longer special. The group admins is now controls the super-user group. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Do I read it right that now you can delete an admin user? What if there is only one Admin user, and you delete that? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Added error handler for ipa_cmd().
- Adam Young ayo...@redhat.com wrote: Endi, can you add in a Unit test for this? If need be, extend the ipa_cmd so that it looks for an optional command line param that makes it try to fetch a .json file that doesn't exist: Something like var suffix = $.bbq.get(cmd_suffix); if (suffix){ method += suffix; } method += '.json'; ... Attached is a new patch based on the latest master with unit tests. ipa_cmd() error can be simulated by overriding $.ajax with a mockup function. Thanks! -- Endi S. Dewata From 9684ca8f41277792d8873fe8052078b51c87acb3 Mon Sep 17 00:00:00 2001 From: Endi S. Dewata edew...@redhat.com Date: Wed, 29 Sep 2010 00:52:56 -0500 Subject: [PATCH] Added error handler for ipa_cmd(). The ipa_cmd() has been modified such that when an error occurs a dialog box will appear showing the error message with 2 buttons: Retry and Cancel. If Retry is clicked, it will attempt to execute the same operation again. If Cancel is clicked, the operation will be canceled and the control is returned to the caller. New unit tests have been added to test ipa_cmd() on successfull and unsuccessfull cases. The associate.js, details.js, entity.js, search.js, and webui.js have been modified to display the error message inside the page. This behavior can be changed in the future (e.g. redirect to error page). The navigation.js and webui.js have been modified to render only the visible tabs. This improves the performance and reduce hidden errors. The navigation unit test has been modified to reflect this behavior. Some variables/functions also have been renamed for consistency. --- install/static/associate.js | 42 install/static/details.js | 29 +++-- install/static/entity.js|5 +- install/static/ipa.js | 38 +- install/static/navigation.js| 64 ++- install/static/search.js| 38 -- install/static/test/ipa_tests.html |3 + install/static/test/ipa_tests.js| 201 +++ install/static/test/navigation_tests.js | 12 +- install/static/webui.js | 67 ++- 10 files changed, 380 insertions(+), 119 deletions(-) diff --git a/install/static/associate.js b/install/static/associate.js index 207c745..c3453e9 100644 --- a/install/static/associate.js +++ b/install/static/associate.js @@ -222,30 +222,30 @@ function AssociationList(obj, pkey, manyObj, associationColumns, jobj) this.manyObj = manyObj; this.parentTab = jobj; -this.populate = function(userData) { - var tbody = this.parentTab.find('.search-table tbody'); - tbody.empty(); - var associationList = userData.result.result[this.associationColumns[0].column]; - for (var j = 0; j associationList.length; j++){ -var row = $(tr/).appendTo(tbody); -for (var k = 0; k associationColumns.length ;k++){ -var column = this.associationColumns[k].column; -$(td/td,{ -html: userData.result.result[column][j] -}).appendTo(row); +this.refresh = function() { + +function refresh_on_success(userData) { + var tbody = this.parentTab.find('.search-table tbody'); + tbody.empty(); + var associationList = userData.result.result[this.associationColumns[0].column]; + for (var j = 0; j associationList.length; j++){ +var row = $(tr/).appendTo(tbody); +for (var k = 0; k associationColumns.length ;k++){ +var column = this.associationColumns[k].column; +$(td/td,{ +html: userData.result.result[column][j] +}).appendTo(row); +} } } -} -this.refresh = function() { -ipa_cmd( 'show', [this.pkey], {}, - function(result){ - form.populate(result); - }, - function(){ - alert(associationListFailure); - }, - form.obj); +function refresh_on_error(xhr, text_status, error_thrown) { +var search_results = $('.search-results', jobj).empty(); +search_results.append('pError: '+error_thrown.name+'/p'); +search_results.append('p'+error_thrown.message+'/p'); +} + +ipa_cmd('show', [this.pkey], {}, refresh_on_success, refresh_on_error, form.obj); } this.setup = function() { diff --git a/install/static/details.js b/install/static/details.js index f68c773..4e37ed6 100644 --- a/install/static/details.js +++ b/install/static/details.js @@ -37,20 +37,24 @@ function ipa_details_create(obj_name, dls, container) container.attr('title', obj_name); container.addClass('details-container'); -container.append('div class=details-buttons/div'); -var jobj =
[Freeipa-devel] admiyo-freeipa-0048-Item-Level-Undo.patch
Item Level Undo Also adding some unit tests for details. Using JQuery UI buttons for update and reset This has the added benefit of letting the user know when the screen has been submitted, as the undo buttons go away. From d6e723dcba77e1da9d67b01a397b6e9f863503f9 Mon Sep 17 00:00:00 2001 From: Adam Young ayo...@redhat.com Date: Wed, 29 Sep 2010 14:58:30 -0400 Subject: [PATCH] Item Level Undo Also adding some unit tests for details. Using JQuery UI buttons for update and reset --- install/static/details.js | 145 +-- install/static/test/data/json_metadata.json |2 +- install/static/test/details_tests.html | 22 install/static/test/details_tests.js| 92 + 4 files changed, 208 insertions(+), 53 deletions(-) create mode 100644 install/static/test/details_tests.html create mode 100644 install/static/test/details_tests.js diff --git a/install/static/details.js b/install/static/details.js index f68c773..0740f65 100644 --- a/install/static/details.js +++ b/install/static/details.js @@ -1,5 +1,6 @@ /* Authors: *Pavel Zuna pz...@redhat.com + *Adam Young ayo...@redhat.com * * Copyright (C) 2010 Red Hat * see file 'COPYING' for use and warranty information @@ -39,8 +40,8 @@ function ipa_details_create(obj_name, dls, container) container.append('div class=details-buttons/div'); var jobj = container.children().last(); -jobj.append('a class=details-reset href=jslinkReset/a'); -jobj.append('a class=details-update href=jslinkUpdate/a'); +jobj.append('a class=details-reset ui-state-default ui-corner-all input_link href=jslinkspan class=ui-icon ui-icon-refresh /span Reset/a'); +jobj.append('a class=details-update ui-state-default ui-corner-all input_link href=jslinkspan class=ui-icon ui-icon-check /spanUpdate/a'); container.append('hr /'); @@ -54,21 +55,23 @@ function ipa_details_create(obj_name, dls, container) jobj.append('a href=#details-viewtypeBack to Top/a'); } -var _ipa_h2_template = 'h2 onclick=_h2_on_click(this)#8722; I/h2'; -var _ipa_dl_template = 'dl id=I class=entryattrs/dl'; -var _ipa_dt_template = 'dt title=TN:/dt'; function ipa_generate_dl(jobj, id, name, dts) { if (!dts) return; -var obj_name = jobj.parent().attr('title'); +var parent = jobj.parent(); +var obj_name = parent.attr('title'); -jobj.after(_ipa_h2_template.replace('I', name)); -jobj = jobj.next(); -jobj.after(_ipa_dl_template.replace('I', id)); -jobj = jobj.next(); +parent.append($(h2/,{ +click: function(){_h2_on_click(this)}, +html:#8722; +name +})); + +var dl = $('dl/dl',{ +id:id, +class:entryattrs}) for (var i = 0; i dts.length; ++i) { var label = ''; @@ -79,12 +82,14 @@ function ipa_generate_dl(jobj, id, name, dts) } if ((!label) (dts[i].length 1)) label = dts[i][1]; -jobj.append( -_ipa_dt_template.replace('T', dts[i][0]).replace('N', label) +dl.append( +$('dt/',{ +title:dts[i][0], +html:label+:}) ); } - -jobj.after('hr /'); +parent.append(dl); +parent.append('hr/'); } function ipa_details_load(obj_name, pkey, on_win, on_fail, sampleData) @@ -201,6 +206,7 @@ function ipa_details_update(obj_name, pkey, on_win, on_fail) var _ipa_a_add_template = 'a href=jslink onclick=return (_ipa_add_on_click(this)) title=AAdd/a'; var _ipa_span_doc_template = 'span class=attrhintHint: D/span'; +var _ipa_span_hint_template = 'span class=attrhintHint: D/span'; /* populate definition lists with the class 'entryattrs' with entry attributes * @@ -239,26 +245,29 @@ function ipa_details_display(obj_name, entry_attrs) } else { /* title contains attribute name - default behaviour */ var multivalue = false; -var hint_span = ''; +var hint_span = null; var param_info = ipa_get_param_info(obj_name, attr); if (param_info) { if (param_info['multivalue'] || param_info['class'] == 'List') multivalue = true; var hint = param_info['hint']; -if (hint) -hint_span = _ipa_span_hint_template.replace('D', hint); +if (hint){ +hint_span = $(span /,{ +class:attrhint, +html:Hint: + hint}); +} } var value = entry_attrs[attr]; if (value) { ipa_insert_first_dd( -jobj, ipa_create_input(obj_name, attr, value[0]) + hint_span +jobj, ipa_create_input(obj_name, attr, value[0],hint_span) ); for (var i = 1; i value.length; ++i) { jobj = jobj.next();
Re: [Freeipa-devel] [PATCH] Added error handler for ipa_cmd().
On 09/29/2010 03:17 PM, Endi Sukma Dewata wrote: - Adam Youngayo...@redhat.com wrote: Endi, can you add in a Unit test for this? If need be, extend the ipa_cmd so that it looks for an optional command line param that makes it try to fetch a .json file that doesn't exist: Something like var suffix = $.bbq.get(cmd_suffix); if (suffix){ method += suffix; } method += '.json'; ... Attached is a new patch based on the latest master with unit tests. ipa_cmd() error can be simulated by overriding $.ajax with a mockup function. Thanks! -- Endi S. Dewata ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 552 handle setattr/addattr better
When doing an addattr check to see if we are creating a multi-value attribute and see if that is allowed by the Param and/or the attribute in the schema (SINGLE-VALUE). Pavel, check my fix in the exception callback. It was passing attrs_list but that isn't set until later. I decided to send an empty list instead. Also catch RDN update exceptions and return an error about primary keys (which this essentially means). ticket 230 rob freeipa-552-mod.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] admiyo-freeipa-0048-Item-Level-Undo.patch
On 09/29/2010 03:41 PM, Adam Young wrote: Item Level Undo Also adding some unit tests for details. Using JQuery UI buttons for update and reset This has the added benefit of letting the user know when the screen has been submitted, as the undo buttons go away. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Rebased on top of Endi's last patch, and added my tests to index and all tests From 0d921487c27b6d4e26a9472529909759eff68c31 Mon Sep 17 00:00:00 2001 From: Adam Young ayo...@redhat.com Date: Wed, 29 Sep 2010 16:57:07 -0400 Subject: [PATCH] Item Level Undo Also adding some unit tests for details. Using JQuery UI buttons for update and reset --- install/static/details.js | 145 +-- install/static/test/all_tests.html |1 + install/static/test/data/json_metadata.json |2 +- install/static/test/details_tests.html | 22 install/static/test/details_tests.js| 92 + install/static/test/index.html |1 + 6 files changed, 210 insertions(+), 53 deletions(-) create mode 100644 install/static/test/details_tests.html create mode 100644 install/static/test/details_tests.js diff --git a/install/static/details.js b/install/static/details.js index 4e37ed6..c0a5073 100644 --- a/install/static/details.js +++ b/install/static/details.js @@ -1,5 +1,6 @@ /* Authors: *Pavel Zuna pz...@redhat.com + *Adam Young ayo...@redhat.com * * Copyright (C) 2010 Red Hat * see file 'COPYING' for use and warranty information @@ -43,8 +44,8 @@ function ipa_details_create(obj_name, dls, container) details.append('div class=details-buttons/div'); var jobj = details.children().last(); -jobj.append('a class=details-reset href=jslinkReset/a'); -jobj.append('a class=details-update href=jslinkUpdate/a'); +jobj.append('a class=details-reset ui-state-default ui-corner-all input_link href=jslinkspan class=ui-icon ui-icon-refresh /span Reset/a'); +jobj.append('a class=details-update ui-state-default ui-corner-all input_link href=jslinkspan class=ui-icon ui-icon-check /spanUpdate/a'); details.append('hr /'); @@ -58,21 +59,23 @@ function ipa_details_create(obj_name, dls, container) jobj.append('a href=#details-viewtypeBack to Top/a'); } -var _ipa_h2_template = 'h2 onclick=_h2_on_click(this)#8722; I/h2'; -var _ipa_dl_template = 'dl id=I class=entryattrs/dl'; -var _ipa_dt_template = 'dt title=TN:/dt'; function ipa_generate_dl(jobj, id, name, dts) { if (!dts) return; -var obj_name = jobj.parent().attr('title'); +var parent = jobj.parent(); +var obj_name = parent.attr('title'); -jobj.after(_ipa_h2_template.replace('I', name)); -jobj = jobj.next(); -jobj.after(_ipa_dl_template.replace('I', id)); -jobj = jobj.next(); +parent.append($(h2/,{ +click: function(){_h2_on_click(this)}, +html:#8722; +name +})); + +var dl = $('dl/dl',{ +id:id, +class:entryattrs}) for (var i = 0; i dts.length; ++i) { var label = ''; @@ -83,12 +86,14 @@ function ipa_generate_dl(jobj, id, name, dts) } if ((!label) (dts[i].length 1)) label = dts[i][1]; -jobj.append( -_ipa_dt_template.replace('T', dts[i][0]).replace('N', label) +dl.append( +$('dt/',{ +title:dts[i][0], +html:label+:}) ); } - -jobj.after('hr /'); +parent.append(dl); +parent.append('hr/'); } function ipa_details_load(jobj, pkey, on_win, on_fail) @@ -210,6 +215,7 @@ function ipa_details_update(obj_name, pkey, on_win, on_fail) var _ipa_a_add_template = 'a href=jslink onclick=return (_ipa_add_on_click(this)) title=AAdd/a'; var _ipa_span_doc_template = 'span class=attrhintHint: D/span'; +var _ipa_span_hint_template = 'span class=attrhintHint: D/span'; /* populate definition lists with the class 'entryattrs' with entry attributes * @@ -248,26 +254,29 @@ function ipa_details_display(obj_name, entry_attrs) } else { /* title contains attribute name - default behaviour */ var multivalue = false; -var hint_span = ''; +var hint_span = null; var param_info = ipa_get_param_info(obj_name, attr); if (param_info) { if (param_info['multivalue'] || param_info['class'] == 'List') multivalue = true; var hint = param_info['hint']; -if (hint) -hint_span = _ipa_span_hint_template.replace('D', hint); +if (hint){ +hint_span = $(span /,{ +class:attrhint, +html:Hint: + hint}); +} } var value =
Re: [Freeipa-devel] [PATCH] Added error handler for ipa_cmd().
On 09/29/2010 04:53 PM, Adam Young wrote: On 09/29/2010 03:17 PM, Endi Sukma Dewata wrote: - Adam Youngayo...@redhat.com wrote: Endi, can you add in a Unit test for this? If need be, extend the ipa_cmd so that it looks for an optional command line param that makes it try to fetch a .json file that doesn't exist: Something like var suffix = $.bbq.get(cmd_suffix); if (suffix){ method += suffix; } method += '.json'; ... Attached is a new patch based on the latest master with unit tests. ipa_cmd() error can be simulated by overriding $.ajax with a mockup function. Thanks! -- Endi S. Dewata ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] admiyo-freeipa-0048-Item-Level-Undo.patch
- Adam Young ayo...@redhat.com wrote: Item Level Undo Also adding some unit tests for details. Using JQuery UI buttons for update and reset This has the added benefit of letting the user know when the screen has been submitted, as the undo buttons go away. Rebased on top of Endi's last patch, and added my tests to index and all tests It doesn't work with attributes that originally don't have any value. The previous_value will be undefined so the input field wouldn't be reset. details.js: 406 var previous_value = entry_attrs[key]; 407 if (previous_value){ 408 this.previousElementSibling.value = previous_value; 409 } -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] Checking empty AJAX response in ipa_cmd().
Hi, Please review the attached patch. Thanks! Some errors (e.g. server down) are reported as AJAX success with empty data and/or HTTP error code != 200. The ipa_cmd() has been modified so that it will detect such errors and invoke the error handler. -- Endi S. Dewata From 6b04c0f28cfd369f77c8f482fb6c7bafbd03499b Mon Sep 17 00:00:00 2001 From: Endi S. Dewata edew...@redhat.com Date: Wed, 29 Sep 2010 16:17:03 -0500 Subject: [PATCH] Checking empty AJAX response in ipa_cmd(). Some errors (e.g. server down) are reported as AJAX success with empty data and/or HTTP error code != 200. The ipa_cmd() has been modified so that it will detect such errors and invoke the error handler. --- install/static/ipa.js | 24 1 files changed, 20 insertions(+), 4 deletions(-) diff --git a/install/static/ipa.js b/install/static/ipa.js index 25a3f1b..e16a71c 100644 --- a/install/static/ipa.js +++ b/install/static/ipa.js @@ -56,10 +56,10 @@ function ipa_init(url, use_static_files, on_win, on_error) $.ajaxSetup(ipa_ajax_options); ipa_cmd('json_metadata', [], {}, -function(data, status, xhr) { +function(data, text_status, xhr) { ipa_objs = data.result.metadata; ipa_messages = data.result.messages; -if (on_win) on_win(data, status, xhr); +if (on_win) on_win(data, text_status, xhr); }, on_error ); @@ -75,10 +75,26 @@ function ipa_init(url, use_static_files, on_win, on_error) * objname - name of an IPA object (optional) */ function ipa_cmd(name, args, options, win_callback, fail_callback, objname) { +function ipa_success_handler(data, text_status, xhr) { +if (!data || xhr.status != 200) { +var error_thrown = { +name: 'HTTP Error '+xhr.status, +message: data ? xhr.statusText : No response +} +ipa_error_handler(xhr, text_status, error_thrown); + +} else if (win_callback) { +win_callback(data, text_status, xhr); +} +} + function ipa_error_handler(xhr, text_status, error_thrown) { ipa_dialog.empty(); ipa_dialog.attr('title', 'Error: '+error_thrown.name); -ipa_dialog.append('p'+error_thrown.message+'/p'); + +if (error_thrown.message) { +ipa_dialog.append('p'+error_thrown.message+'/p'); +} ipa_dialog.dialog({ modal: true, @@ -120,7 +136,7 @@ function ipa_cmd(name, args, options, win_callback, fail_callback, objname) var request = { url: url, data: JSON.stringify(data), -success: win_callback, +success: ipa_success_handler, error: ipa_error_handler }; -- 1.6.6.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] admiyo-freeipa-0048-Item-Level-Undo.patch
On 09/29/2010 06:19 PM, Endi Sukma Dewata wrote: - Adam Youngayo...@redhat.com wrote: Item Level Undo Also adding some unit tests for details. Using JQuery UI buttons for update and reset This has the added benefit of letting the user know when the screen has been submitted, as the undo buttons go away. Rebased on top of Endi's last patch, and added my tests to index and all tests It doesn't work with attributes that originally don't have any value. The previous_value will be undefined so the input field wouldn't be reset. details.js: 406 var previous_value = entry_attrs[key]; 407 if (previous_value){ 408 this.previousElementSibling.value = previous_value; 409 } -- Endi S. Dewata Ah..good catch. OK I'll fix ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Added error handler for ipa_cmd().
On 09/29/2010 05:05 PM, Adam Young wrote: On 09/29/2010 04:53 PM, Adam Young wrote: On 09/29/2010 03:17 PM, Endi Sukma Dewata wrote: - Adam Youngayo...@redhat.com wrote: Endi, can you add in a Unit test for this? If need be, extend the ipa_cmd so that it looks for an optional command line param that makes it try to fetch a .json file that doesn't exist: Something like var suffix = $.bbq.get(cmd_suffix); if (suffix){ method += suffix; } method += '.json'; ... Attached is a new patch based on the latest master with unit tests. ipa_cmd() error can be simulated by overriding $.ajax with a mockup function. Thanks! -- Endi S. Dewata ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Should have remembered this approach, standard JS way to deal with undefined values. From ca9f615842b9cfd61f3ad28b755975aea83fada0 Mon Sep 17 00:00:00 2001 From: Adam Young ayo...@redhat.com Date: Wed, 29 Sep 2010 16:57:07 -0400 Subject: [PATCH] Item Level Undo Also adding some unit tests for details. Using JQuery UI buttons for update and reset --- install/static/details.js | 143 +-- install/static/test/all_tests.html |1 + install/static/test/data/json_metadata.json |2 +- install/static/test/details_tests.html | 22 install/static/test/details_tests.js| 92 + install/static/test/index.html |1 + 6 files changed, 208 insertions(+), 53 deletions(-) create mode 100644 install/static/test/details_tests.html create mode 100644 install/static/test/details_tests.js diff --git a/install/static/details.js b/install/static/details.js index 4e37ed6..cd230a8 100644 --- a/install/static/details.js +++ b/install/static/details.js @@ -1,5 +1,6 @@ /* Authors: *Pavel Zuna pz...@redhat.com + *Adam Young ayo...@redhat.com * * Copyright (C) 2010 Red Hat * see file 'COPYING' for use and warranty information @@ -43,8 +44,8 @@ function ipa_details_create(obj_name, dls, container) details.append('div class=details-buttons/div'); var jobj = details.children().last(); -jobj.append('a class=details-reset href=jslinkReset/a'); -jobj.append('a class=details-update href=jslinkUpdate/a'); +jobj.append('a class=details-reset ui-state-default ui-corner-all input_link href=jslinkspan class=ui-icon ui-icon-refresh /span Reset/a'); +jobj.append('a class=details-update ui-state-default ui-corner-all input_link href=jslinkspan class=ui-icon ui-icon-check /spanUpdate/a'); details.append('hr /'); @@ -58,21 +59,23 @@ function ipa_details_create(obj_name, dls, container) jobj.append('a href=#details-viewtypeBack to Top/a'); } -var _ipa_h2_template = 'h2 onclick=_h2_on_click(this)#8722; I/h2'; -var _ipa_dl_template = 'dl id=I class=entryattrs/dl'; -var _ipa_dt_template = 'dt title=TN:/dt'; function ipa_generate_dl(jobj, id, name, dts) { if (!dts) return; -var obj_name = jobj.parent().attr('title'); +var parent = jobj.parent(); +var obj_name = parent.attr('title'); -jobj.after(_ipa_h2_template.replace('I', name)); -jobj = jobj.next(); -jobj.after(_ipa_dl_template.replace('I', id)); -jobj = jobj.next(); +parent.append($(h2/,{ +click: function(){_h2_on_click(this)}, +html:#8722; +name +})); + +var dl = $('dl/dl',{ +id:id, +class:entryattrs}) for (var i = 0; i dts.length; ++i) { var label = ''; @@ -83,12 +86,14 @@ function ipa_generate_dl(jobj, id, name, dts) } if ((!label) (dts[i].length 1)) label = dts[i][1]; -jobj.append( -_ipa_dt_template.replace('T', dts[i][0]).replace('N', label) +dl.append( +$('dt/',{ +title:dts[i][0], +html:label+:}) ); } - -jobj.after('hr /'); +parent.append(dl); +parent.append('hr/'); } function ipa_details_load(jobj, pkey, on_win, on_fail) @@ -210,6 +215,7 @@ function ipa_details_update(obj_name, pkey, on_win, on_fail) var _ipa_a_add_template = 'a href=jslink onclick=return (_ipa_add_on_click(this)) title=AAdd/a'; var _ipa_span_doc_template = 'span class=attrhintHint: D/span'; +var _ipa_span_hint_template = 'span class=attrhintHint: D/span'; /* populate definition lists with the class 'entryattrs' with entry attributes * @@ -248,26 +254,29 @@ function ipa_details_display(obj_name, entry_attrs) } else { /* title contains attribute name - default behaviour */ var multivalue = false; -var hint_span = ''; +var hint_span = null; var param_info =
Re: [Freeipa-devel] Sudo Schema Bug
JR Aquino wrote: I believe we have made an oversight in the way that sudo processes 'deny' or negations via ldap... Currently our IPA sudo Schema has ipasudorule objects set to contain an attribute: accessRuleType Unfortunately, sudo does not have a means to do a 'deny' in this way... For a command, user, or host to be 'denied' it must be proceeded with an exclamation point: ! Due to the RFC, LDAP will return entries in an arbitrary order, as such sudo will do first match on the ! negations. However, this is only true within the same Rule, I.E. if a user belongs to multiple groups, one which allows the command, and separate one which negates the command, sudo can and will pass or fail depending on which object ldap returns back for the search results. It occurs to me that we have 2 ways to proceed. 0) I suggest we remove the attribute: accessRuleType from ipasudorule. 1) Add the attribute: accessRuleType to ipasudocmdgrp. -This has the benefit of not having to duplicate new ipasudocmd's only to prepend a ! in front of them since an ipasudorule can contain multiple ipasudocmdgrp's. I.E. /usr/bin/less can be added to an 'allow' command group and remain unchanged, but when also added to a 'deny' command group, the compat layer should prepend the ! for us. Please let me know if anyone has any objections or observations. May be I misunderstood how things work but my assumption was that SUDO will inspect multiple rules not just a rule returned by LDAP. Is this not the case? If it is the case then you are right that current schema is different and requires different grouping of the commands than with the standard schema but end result will be same. Let me try to illustrate it on example: Standard schema: Rule 1 has command A and !B Rule 2 has command C and !D In the new schema Rule X will have A C Rule Y will be negative and have C D Of cause Rules 1/2 and X/Y can't apply to the same user groups as the current rules . The thought was that other set of groups will potentially used by the records in the new schema. The new schema directs people to think in better abstraction terms : These users on these hosts can do something. or These users on these hosts can't do something. It is a much cleaner and more intuitive administrative model than what standard SUDO schema offers. But if it is a wrong assumption and really does not add a value we should definitely reconsider. If proposed approach is really wrong then I would rather remove the accessRuleType and add another attribute memberNotCmd similar to memberCmd that will point to groups or individual commands that need to be prohibited. And then support additional value of cmdCategory equal none that will imply that all sudo commands are prohibited. However I would argue that this is and overhead that none can be accomplished by totally disabling SUDO via HBAC. I would also argue that memberNotCmd memberCmd in one rule are equivalent to two rules using same users/hosts but one for allowed commands and another for prohibited commands. So back to the example the assumption currently is that the SUDO will run through all the rules and match negative commands and then will do positive commands. In case of existing schema the prohibited commands will be scattered across multiple entries while in case of new schema they will be grouped in entries. Does this really matter for the SUDO usility how they are grouped and in what order they come? Based on the RFC it should not matter so I really do not see an issue other than forcing people to change rule definitions to do things in a more intuitive way. Please correct me if I am wrong. Thanks Dmitri ~ Jr Aquino, GCIH | Information Security Specialist Citrix Online | 6500 Hollister Avenue | Goleta, CA 93117 T: +1 805.690.3478 jr.aqu...@citrixonline.commailto:jr.aqu...@citrixonline.com http://www.citrixonline.comhttp://www.citrixonline.com/ -- Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [Transifex] File submitted via email to FreeIPA | master
On 09/28/2010 11:06 AM, ad...@transifex.net wrote: Hello freeipa, this is Transifex at http://www.transifex.net. The following attached files were submitted to FreeIPA | master by yurchoryurc...@ukr.net Please, visit Transifex at http://www.transifex.net/projects/p/freeipa/c/master/ in order to see the component page. Thank you, Transifex ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Merged and pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Checking empty AJAX response in ipa_cmd().
- Endi Sukma Dewata edew...@redhat.com wrote: Some errors (e.g. server down) are reported as AJAX success with empty data and/or HTTP error code != 200. The ipa_cmd() has been modified so that it will detect such errors and invoke the error handler. It turns out that pulling local JSON files will result in HTTP error code 0, so it's not a good indicator for error. I've removed it in the new patch. -- Endi S. Dewata From de58c9fbea0292d899ca7b0585a05c0ee89ac8b1 Mon Sep 17 00:00:00 2001 From: Endi S. Dewata edew...@redhat.com Date: Wed, 29 Sep 2010 16:17:03 -0500 Subject: [PATCH] Checking empty AJAX response in ipa_cmd(). Some errors (e.g. server down) are reported as AJAX success with empty data. The ipa_cmd() has been modified so that it will detect such errors and invoke the error handler. --- install/static/ipa.js | 24 1 files changed, 20 insertions(+), 4 deletions(-) diff --git a/install/static/ipa.js b/install/static/ipa.js index 25a3f1b..9e84bb9 100644 --- a/install/static/ipa.js +++ b/install/static/ipa.js @@ -56,10 +56,10 @@ function ipa_init(url, use_static_files, on_win, on_error) $.ajaxSetup(ipa_ajax_options); ipa_cmd('json_metadata', [], {}, -function(data, status, xhr) { +function(data, text_status, xhr) { ipa_objs = data.result.metadata; ipa_messages = data.result.messages; -if (on_win) on_win(data, status, xhr); +if (on_win) on_win(data, text_status, xhr); }, on_error ); @@ -75,10 +75,26 @@ function ipa_init(url, use_static_files, on_win, on_error) * objname - name of an IPA object (optional) */ function ipa_cmd(name, args, options, win_callback, fail_callback, objname) { +function ipa_success_handler(data, text_status, xhr) { +if (!data) { +var error_thrown = { +name: 'HTTP Error '+xhr.status, +message: data ? xhr.statusText : No response +} +ipa_error_handler(xhr, text_status, error_thrown); + +} else if (win_callback) { +win_callback(data, text_status, xhr); +} +} + function ipa_error_handler(xhr, text_status, error_thrown) { ipa_dialog.empty(); ipa_dialog.attr('title', 'Error: '+error_thrown.name); -ipa_dialog.append('p'+error_thrown.message+'/p'); + +if (error_thrown.message) { +ipa_dialog.append('p'+error_thrown.message+'/p'); +} ipa_dialog.dialog({ modal: true, @@ -120,7 +136,7 @@ function ipa_cmd(name, args, options, win_callback, fail_callback, objname) var request = { url: url, data: JSON.stringify(data), -success: win_callback, +success: ipa_success_handler, error: ipa_error_handler }; -- 1.6.6.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Checking empty AJAX response in ipa_cmd().
On 09/29/2010 08:34 PM, Endi Sukma Dewata wrote: - Endi Sukma Dewataedew...@redhat.com wrote: Some errors (e.g. server down) are reported as AJAX success with empty data and/or HTTP error code != 200. The ipa_cmd() has been modified so that it will detect such errors and invoke the error handler. It turns out that pulling local JSON files will result in HTTP error code 0, so it's not a good indicator for error. I've removed it in the new patch. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Need Metadata for phone, email, other objects not in json_metadata yet
Rob, You indicated that there was a way to get the params we needed to produce meta-data. I'm working on the phonenumber issues right now, and right now have no way of telling that it is a multi value attribute. There is logic in the code, but it relies on the metadata. v Can you point me in the right direction? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Checking empty AJAX response in ipa_cmd().
On 09/29/2010 09:41 PM, Adam Young wrote: On 09/29/2010 08:34 PM, Endi Sukma Dewata wrote: - Endi Sukma Dewataedew...@redhat.com wrote: Some errors (e.g. server down) are reported as AJAX success with empty data and/or HTTP error code != 200. The ipa_cmd() has been modified so that it will detect such errors and invoke the error handler. It turns out that pulling local JSON files will result in HTTP error code 0, so it's not a good indicator for error. I've removed it in the new patch. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Added error handler for ipa_cmd().
- Adam Young ayo...@redhat.com wrote: Should have remembered this approach, standard JS way to deal with undefined values. admiyo-freeipa-0048-3-Item-Level-Undo.patch A few notes: 1. You're replying to the wrong thread :) 2. The undo button will only appear when the input field loses focus. Ideally it should appear as soon as the value is changed, but I'm not sure if it's possible to do that in JS. This can be addressed in the future. 3. The hint_span doesn't seem to be used consistently in details.js:272-297: ipa_insert_first_dd( jobj, ipa_create_input(obj_name, attr, value[0],hint_span) ); ipa_insert_other_dd( jobj, ipa_create_input(obj_name, attr, value[i],hint_span) ); ipa_insert_other_dd( jobj.next(), _ipa_a_add_template.replace('A', attr) ); ipa_insert_first_dd( jobj, _ipa_a_add_template.replace('A', attr) /*.append( hint_span)*/ ); ipa_insert_first_dd( jobj, ipa_create_input(obj_name, attr, '')/*.append( hint_span)*/ ); 4. I think the statement on line 341 should be removed because it redefines the input variable: var input = $(label,{html:value.toString()}); 5. There is a trailing whitespace on line 337. Thanks! -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Sudo Schema Bug
I have encountered and troubleshot several instances recently where a user was present in more than 1 sudo rule. One that permitted the user, the host, and commands, and another that permited the user, and host, but no commands. It was discovered that: * Sudo is a stop on first match... * When sudo encounters a rule that matches the user and host it will stop even if it does not match the command that was executed. We saw this to be the case even if there were other more permissive sudo rules matching the user and host. If FreeIPA keeps the current schema, a sudorule marked as a deny, would only randomly be enforced over more permissive rules matching host and user. Sudo will only return a 'negation command' ahead of a permissive command /within the same rule/. It is a subtle and frustrating bug/feature to have to encounter and identify. We could ask Todd Miller to confirm. From the Sudo Ldap Man Page: =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= -Differences between LDAP and non-LDAP sudoers- There are some subtle differences in the way sudoers is handled once in LDAP. Probably the biggest is that according to the RFC, LDAP ordering is arbitrary and you cannot expect that Attributes and Entries are returned in any specific order. If there are conflicting command rules on /an entry/, the negative takes precedence. This is called paranoid behavior (not necessarily the most specific match). dn: cn=role1,ou=Sudoers,dc=my-domain,dc=com objectClass: sudoRole objectClass: top cn: role1 sudoUser: johnny sudoHost: ALL sudoCommand: ALL sudoCommand: !/bin/sh =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Jr Aquino | Information Security Specialist Citrix Online Division May be I misunderstood how things work but my assumption was that SUDO will inspect multiple rules not just a rule returned by LDAP. Is this not the case? If it is the case then you are right that current schema is different and requires different grouping of the commands than with the standard schema but end result will be same. Let me try to illustrate it on example: Standard schema: Rule 1 has command A and !B Rule 2 has command C and !D In the new schema Rule X will have A C Rule Y will be negative and have C D Of cause Rules 1/2 and X/Y can't apply to the same user groups as the current rules . The thought was that other set of groups will potentially used by the records in the new schema. The new schema directs people to think in better abstraction terms : These users on these hosts can do something. or These users on these hosts can't do something. It is a much cleaner and more intuitive administrative model than what standard SUDO schema offers. But if it is a wrong assumption and really does not add a value we should definitely reconsider. If proposed approach is really wrong then I would rather remove the accessRuleType and add another attribute memberNotCmd similar to memberCmd that will point to groups or individual commands that need to be prohibited. And then support additional value of cmdCategory equal none that will imply that all sudo commands are prohibited. However I would argue that this is and overhead that none can be accomplished by totally disabling SUDO via HBAC. I would also argue that memberNotCmd memberCmd in one rule are equivalent to two rules using same users/hosts but one for allowed commands and another for prohibited commands. So back to the example the assumption currently is that the SUDO will run through all the rules and match negative commands and then will do positive commands. In case of existing schema the prohibited commands will be scattered across multiple entries while in case of new schema they will be grouped in entries. Does this really matter for the SUDO usility how they are grouped and in what order they come? Based on the RFC it should not matter so I really do not see an issue other than forcing people to change rule definitions to do things in a more intuitive way. Please correct me if I am wrong. Thanks Dmitri ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Need Metadata for phone, email, other objects not in json_metadata yet
Adam Young wrote: Rob, You indicated that there was a way to get the params we needed to produce meta-data. I'm working on the phonenumber issues right now, and right now have no way of telling that it is a multi value attribute. There is logic in the code, but it relies on the metadata. v Can you point me in the right direction? I was thinking purely for output purposes so the Label could get set properly, so I'm not 100% sure this will work for what you are doing, but look at setting has_output_params: output_params = ( Str('telephoneNumber*', label=_('Home Phone'), ), ) class user_add(LDAPCreate): ... has_output_params = LDAPCreate.has_output_params + output_params rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Sudo Schema Bug
JR Aquino wrote: I have encountered and troubleshot several instances recently where a user was present in more than 1 sudo rule. One that permitted the user, the host, and commands, and another that permited the user, and host, but no commands. It was discovered that: * Sudo is a stop on first match... * When sudo encounters a rule that matches the user and host it will stop even if it does not match the command that was executed. We saw this to be the case even if there were other more permissive sudo rules matching the user and host. If FreeIPA keeps the current schema, a sudorule marked as a deny, would only randomly be enforced over more permissive rules matching host and user. Sudo will only return a 'negation command' ahead of a permissive command /within the same rule/. It is a subtle and frustrating bug/feature to have to encounter and identify. We could ask Todd Miller to confirm. From the Sudo Ldap Man Page: =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= -Differences between LDAP and non-LDAP sudoers- There are some subtle differences in the way sudoers is handled once in LDAP. Probably the biggest is that according to the RFC, LDAP ordering is arbitrary and you cannot expect that Attributes and Entries are returned in any specific order. If there are conflicting command rules on /an entry/, the negative takes precedence. This is called paranoid behavior (not necessarily the most specific match). dn: cn=role1,ou=Sudoers,dc=my-domain,dc=com objectClass: sudoRole objectClass: top cn: role1 sudoUser: johnny sudoHost: ALL sudoCommand: ALL sudoCommand: !/bin/sh =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Jr Aquino | Information Security Specialist Citrix Online Division I was aware of this writeup however I did not read it as there is a problem when there are multiple rules with negation. It actually nowhere says how SUDO handles multiple rules if they are mutually exclusive. Even in the current schema there is a problem when you have two rules and they contradict each other according to RFC this is a valid situation and thus should be handled correctly by SUDO. Do not take me wrong, I am willing to adjust the schema but if the SUDO utility can't handle contradicting rules even with the existing schema this is a very serious bug that we either should fix in SUDO or have a workaround. If you are right above that it does not look at other rules before making a decision and makes just based on one rule we can add the attribute(s) as you or I suggested but this generally limits the flexibility of the solution. Does anyone have experience with this behavior and can confirm the limitation? Thanks Dmitri May be I misunderstood how things work but my assumption was that SUDO will inspect multiple rules not just a rule returned by LDAP. Is this not the case? If it is the case then you are right that current schema is different and requires different grouping of the commands than with the standard schema but end result will be same. Let me try to illustrate it on example: Standard schema: Rule 1 has command A and !B Rule 2 has command C and !D In the new schema Rule X will have A C Rule Y will be negative and have C D Of cause Rules 1/2 and X/Y can't apply to the same user groups as the current rules . The thought was that other set of groups will potentially used by the records in the new schema. The new schema directs people to think in better abstraction terms : These users on these hosts can do something. or These users on these hosts can't do something. It is a much cleaner and more intuitive administrative model than what standard SUDO schema offers. But if it is a wrong assumption and really does not add a value we should definitely reconsider. If proposed approach is really wrong then I would rather remove the accessRuleType and add another attribute memberNotCmd similar to memberCmd that will point to groups or individual commands that need to be prohibited. And then support additional value of cmdCategory equal none that will imply that all sudo commands are prohibited. However I would argue that this is and overhead that none can be accomplished by totally disabling SUDO via HBAC. I would also argue that memberNotCmd memberCmd in one rule are equivalent to two rules using same users/hosts but one for allowed commands and another for prohibited commands. So back to the example the assumption currently is that the SUDO will run through all the rules and match negative commands and then will do positive commands. In case of existing schema the prohibited commands will be scattered across multiple entries while in case of new schema they will be grouped in entries. Does this really matter for the SUDO usility how they are grouped and in what order they come? Based on the RFC it should not matter so I really do not see an issue other