Re: [Freeipa-devel] [PATCH]admiyo-freeipa-0047-tab-objects.patch

2010-09-29 Thread Adam Young

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

2010-09-29 Thread Adam Young

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

2010-09-29 Thread Adam Young

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

2010-09-29 Thread Rob Crittenden
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

2010-09-29 Thread Adam Young

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().

2010-09-29 Thread Endi Sukma Dewata
- 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

2010-09-29 Thread Adam Young

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().

2010-09-29 Thread Adam Young

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

2010-09-29 Thread Rob Crittenden
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

2010-09-29 Thread Adam Young

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().

2010-09-29 Thread Adam Young

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

2010-09-29 Thread Endi Sukma Dewata
- 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().

2010-09-29 Thread Endi Sukma Dewata
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

2010-09-29 Thread Adam Young

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().

2010-09-29 Thread Adam Young

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

2010-09-29 Thread Dmitri Pal
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

2010-09-29 Thread Adam Young

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().

2010-09-29 Thread Endi Sukma Dewata
- 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().

2010-09-29 Thread Adam Young

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

2010-09-29 Thread Adam Young

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().

2010-09-29 Thread Adam Young

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().

2010-09-29 Thread Endi Sukma Dewata
- 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

2010-09-29 Thread JR Aquino
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

2010-09-29 Thread Rob Crittenden

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

2010-09-29 Thread Dmitri Pal
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