Re: [Freeipa-devel] [PATCH] 592-628 Update to PatternFly
Hi, see my review notes below: On Mon, 05 May 2014 18:41:13 +0200 Petr Vobornik pvobo...@redhat.com wrote: This patchset updates Bootstrap 2 based RCUE to Bootstrap 3 based PatternFly (v0.2.4) according to plan described at: http://www.redhat.com/archives/freeipa-devel/2014-April/msg00045.html The rest of the patches are mostly response to new CSS styles + some new functionality and simplification of UI: - css cleanup, images cleanup - adjustment of stand-alone pages to PF - adjustment of DOM structure to Bootstap 3 structure - BS 3 enabled to change absolute positioned layout to responsive fluid layout - new activity indicators (since the old didn't fit into PF navigation) - new pager styles + additional behavior - action select transform into dropdown and moved to control-button section, making the header responsive - fluid layout requested removal of computation of columns widths - removal of login.html and logout.html - new login background (the old one did not work with PF styles) - new dialog styles - + additional adjustments to use PF The result is that UI uses most of PatternFly styles and is responsive. Fixes: https://fedorahosted.org/freeipa/ticket/4177 - Better indication of ongoing activity if dialog is opened - working progress could have a border. if it is over a dialog, sometimes it looks messy over text https://fedorahosted.org/freeipa/ticket/4136 - WebUI unusable on Cellphone screen - when I open the menu in 320x480, and select and navigate to an item, the menu stays open - needs more investigation, if it is freeipa ui issue - qr code is fixed size in otp tokens, doesn't look nice on small screens not a problem, user just clicks on qr code link - when a table header is longer, than the actual screen size, overflow hidden occurs, unable to use buttons at the end of the header eg DNS Resource Records, 320x480px, sometimes delete and add button overflows the table, you can only scroll that table with tap not a problem, responsive table works this way - in 320x480, login page configuration text overflows on a white background, especially if there is a login error, which makes the white text unreadable https://fedorahosted.org/freeipa/ticket/4255 - Web UI: Display Loading message when a list of entries is being loaded see working progress comment above https://fedorahosted.org/freeipa/ticket/3435 - [RFE] Remove width limit in UI ACK - PatternFly 3 handles this very neatly https://fedorahosted.org/freeipa/ticket/3050 - WebUI: it is not clear which row a value belongs to ACK - row color alternation hopefully solves the problem https://fedorahosted.org/freeipa/ticket/4278 - Use Patternfly theme in config and migration pages FreeIPA logo doesn't lead anywhere, no way to navigate to the login page, only by altering the url, or clicking the back button. IMO logo should always lead to login page if not logged in. https://fedorahosted.org/freeipa/ticket/4281 - Remove login.html and logout.html ACK https://fedorahosted.org/freeipa/ticket/4282 Other issues: - unit tests have several fails, possibly because of dom changes - integration tests ran without errors Also, according to the UX meeting with Kyle, this patchset should include the following changes: - placeholder for search, box should be on the left - actions in one place, on the right in search page - actions in one place, on the left in details page - action dropdown list to the right near update button in details page - left align form fields in details page, two columns arrangement if the screen is wide - hbac details pages - leave it as it is, no form modification required - association adder dialog - placeholder for textbox(Filter available), change button text Filter - search page title should be changed - use dark variant text - multi value list - add to button, with undo all button group - multi value list - delete should be also a button - left align firefox configuration page steps - ie. every static page - migration should look like login, (~reset_password), text should go to right - error page return back should be a button Thanks Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 589-590 webui-ci: save screenshot on test failure
On Wed, 30 Apr 2014 10:40:43 +0200 Petr Vobornik pvobo...@redhat.com wrote: Very handy for debugging failures... New decorator: ui_driver.screenshot created. It should be applied on test methods. Screenshot is saved on each exception except SkipTest. Configuration: - add: `save_screenshots: True` to ~/.ipa/ui_test.conf to enable saving screenshots - optionally add `screenshot_dir: /path/to/dir` to specify target directory otherwise screenshots are saved to current directory Hi, LGTM, it even saves screenshots when I use remote testing, so ACK Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation
On Tue, 06 May 2014 11:46:14 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote: On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote: On 6.5.2014 17:13, Nathaniel McCallum wrote: On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote: On 6.5.2014 16:51, Nathaniel McCallum wrote: Specifying the default in the LDAP Object causes the parameter to be specified for non-add operations. This is especially problematic when performing the modify operation as it causes the primary key to change for every modification. https://fedorahosted.org/freeipa/ticket/4227 shouldn't removal of `autofill=True,` be enough? Removing autofill=True results in the default not being used for the otptoken-add operation. That may be a different bug (I'm not sure what the expectation of autofill is). Nathaniel Seems to work form me with: diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644 --- a/ipalib/plugins/otptoken.py +++ b/ipalib/plugins/otptoken.py @@ -121,9 +121,7 @@ class otptoken(LDAPObject): cli_name='id', label=_('Unique ID'), default_from=lambda: unicode(uuid.uuid4()), -autofill=True, primary_key=True, -flags=('optional_create'), ), StrEnum('type?', label=_('Type'), Doing this causes the ipa otptoken-add command to prompt for the Unique ID. This may be the desired behavior, but it is not how it worked previously (no prompt). Here is an alternate patch for this second approach. I have no strong opinion on the correct behavior here. Nathaniel IMO you should update API.txt with ./makeapi Thanks Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 18 webui otptoken test data added
On Tue, 06 May 2014 10:29:32 +0200 Petr Vobornik pvobo...@redhat.com wrote: On 5.5.2014 16:39, Misnyovszki Adam wrote: On Wed, 30 Apr 2014 13:37:10 +0200 Petr Vobornik pvobo...@redhat.com wrote: On 29.4.2014 16:30, Misnyovszki Adam wrote: On Fri, 25 Apr 2014 17:16:48 +0200 Misnyovszki Adam amisn...@redhat.com wrote: Hi, this patch adds some static test data for the webui otptoken part. Adam Attached corrected DN's. Thanks Adam 1) Why otptoken_batch_del.json ends with error? Also there might be a defect in UI that for batch delete operation it asks for batch.json and not $ENTITY_batch_del.json making otptoken_batch_del.json unused - out of scope of this patch. 2) Why otptoken_mod.json ends with error? 3) otptoken_find.json is not needed since the search facet uses paging (combination of otptoken_get_records.json and otptoken_find_pkeys.json is enough). In general, it's OK to fake the data if there is some bug which causes errors and we know that it will be fixed. Hi, see the attached, and corrected 18 patch for otptoken static test data. Also, I've added patch 20, for fixing the batch_del command in static webui tests. Thanks Adam Patch 18-3: 1. otptoken_batch.json, otptoken_batch_del.json, otptoken_mod.json have trailing whitespace after commas 2. otptoken_batch.json was obsoleted by patch 20. Should be removed since both patches are in one patchset. Patch 20: ACK See attached fix for patch 18 Thanks Adam From ff56869ead0b99a5007f40b3a738b1ce80ada069 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 7 May 2014 15:10:32 +0200 Subject: [PATCH] webui OTP token test data added --- install/ui/test/data/otptoken_add.json | 43 +++ install/ui/test/data/otptoken_batch_del.json | 27 ++ install/ui/test/data/otptoken_batch_mod.json | 34 install/ui/test/data/otptoken_find_pkeys.json | 17 ++ install/ui/test/data/otptoken_get_records.json | 57 install/ui/test/data/otptoken_mod.json | 72 ++ install/ui/test/data/otptoken_show.json| 51 ++ 7 files changed, 301 insertions(+) create mode 100644 install/ui/test/data/otptoken_add.json create mode 100644 install/ui/test/data/otptoken_batch_del.json create mode 100644 install/ui/test/data/otptoken_batch_mod.json create mode 100644 install/ui/test/data/otptoken_find_pkeys.json create mode 100644 install/ui/test/data/otptoken_get_records.json create mode 100644 install/ui/test/data/otptoken_mod.json create mode 100644 install/ui/test/data/otptoken_show.json diff --git a/install/ui/test/data/otptoken_add.json b/install/ui/test/data/otptoken_add.json new file mode 100644 index ..c52fc15035e0aad025294a8da1f938ee53c8e5a9 --- /dev/null +++ b/install/ui/test/data/otptoken_add.json @@ -0,0 +1,43 @@ +{ +error: null, +id: null, +result: { +result: { +dn: ipatokenuniqueid=10bd43b5-3204-4695-9225-91064f6c77b3,cn=otp,dc=example,dc=com, +ipatokenmodel: [ +totp +], +ipatokenotpalgorithm: [ +sha1 +], +ipatokenotpdigits: [ +6 +], +ipatokenotpkey: [ +{ +__base64__: 2TUYXOVTaZf/Og== +} +], +ipatokentotpclockoffset: [ +0 +], +ipatokentotptimestep: [ +30 +], +ipatokenuniqueid: [ +footoken +], +ipatokenvendor: [ +FreeIPA +], +objectclass: [ +top, +ipatokentotp, +ipatoken +], +uri: otpauth://totp/EXAMPLE.COM:10bd43b5-3204-4695-9225-91064f6c77b3?digits=6secret=3E2RQXHFKNUZP7Z2period=30algorithm=sha1issuer=EXAMPLE.COM +}, +summary: Added OTP token \10bd43b5-3204-4695-9225-91064f6c77b3\, +value: 10bd43b5-3204-4695-9225-91064f6c77b3 +} +} diff --git a/install/ui/test/data/otptoken_batch_del.json b/install/ui/test/data/otptoken_batch_del.json new file mode 100644 index ..7fff0a0c5cf7cbd0a39d3616c4c58ef7c8d3ccb7 --- /dev/null +++ b/install/ui/test/data/otptoken_batch_del.json @@ -0,0 +1,27 @@ +{ +error: null, +id: null, +result: { +count: 1, +messages: [ +{ +code: 13001, +message: API Version number was not sent, forward compatibility not guaranteed. Assuming server's API version, 2.83, +name: VersionMissing, +type: warning +} +], +results: [ +{ +error: null, +result: { +failed
Re: [Freeipa-devel] [PATCH] 22-23 webui tests extended by checking field disable property
On Tue, 6 May 2014 17:58:09 +0200 Misnyovszki Adam amisn...@redhat.com wrote: Hi, first patch extends webui tests with a callback function, and an assert_disabled function, to check if a field is disabled under certain conditions. Second patch extends range tests with this checking functionality depending on range types. Thanks Adam Fixed issue in 22, when the element, which is checked, doesn't exist, test returns false positive. Thanks AdamFrom 5c080056bfcc2ab3589b7a43e4d01bcd8041 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 7 May 2014 14:30:29 +0200 Subject: [PATCH] webui tests: callback, assert_disabled feature added Added a callback feature to webui tests, to extend functionality. Also added assert_disabled function to ui_driver, to check if a field is disabled in the browser. --- ipatests/test_webui/ui_driver.py | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py index 7cfe21ad8985b04fcb296adccf0277a5f02833b9..ce63d570e978f7fe68d35a89b14cc5714e313856 100644 --- a/ipatests/test_webui/ui_driver.py +++ b/ipatests/test_webui/ui_driver.py @@ -1000,7 +1000,7 @@ class UI_driver(object): key = field[1] val = field[2] -if undo: +if undo and not hasattr(key, '__call__'): self.assert_undo_button(key, False, parent) if widget_type == 'textbox': @@ -1025,8 +1025,13 @@ class UI_driver(object): self.fill_multivalued(key, val, parent) elif widget_type == 'table': self.select_record(val, parent, key) +# this meta field specifies a function, to extend functionality of +# field checking +elif widget_type == 'callback': +if hasattr(key, '__call__'): +key(val) self.wait() -if undo: +if undo and not hasattr(key, '__call__'): self.assert_undo_button(key, True, parent) def validate_fields(self, fields, parent=None): @@ -1551,6 +1556,19 @@ class UI_driver(object): else: assert visible, Element not visible: %s % selector +def assert_disabled(self, selector, parent=None, negative=False): + +Assert that element defined by selector is disabled + +if not parent: +parent = self.get_form() +el = self.find(selector, By.CSS_SELECTOR, parent, strict=True) +dis = self.find(selector+[disabled], By.CSS_SELECTOR, parent) +if negative: +assert dis is None, Element is disabled: %s % selector +else: +assert dis, Element is not disabled: %s % selector + def assert_record(self, pkey, parent=None, table_name=None, negative=False): Assert that record is in current search table -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 629 webui: otptoken-adder dialog - remove obsolete comment
On Tue, 06 May 2014 13:34:28 +0200 Petr Vobornik pvobo...@redhat.com wrote: No longer valid. HOTP tokens are also supported. ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 20 Trust add datetime fix
Hi, this patch fixes trust add, since now datetime object is returned for 'modifytimestamp', which cannot be split like a string, thus causing an error. Thanks AdamFrom afe6d32cb0912c18fa046992a1e27f352b454dcb Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Mon, 5 May 2014 19:21:01 +0200 Subject: [PATCH] Trust add datetime fix Fixes trust add, since now datetime object is returned for 'modifytimestamp', which cannot be split like a string. --- ipaserver/dcerpc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py index 3b89adc084caf5a21021d29ab55d3f088c4422bc..312761662c6fbde0c3c2136e14ac3d4f48c125c7 100644 --- a/ipaserver/dcerpc.py +++ b/ipaserver/dcerpc.py @@ -1107,7 +1107,7 @@ class TrustDomainJoins(object): # Use realmdomains' modification timestamp to judge records last update time entry = self.api.Backend.ldap2.get_entry(realm_domains['dn'], ['modifyTimestamp']) # Convert the timestamp to Windows 64-bit timestamp format -trust_timestamp = long(time.mktime(time.strptime(entry['modifytimestamp'][0][:14], %Y%m%d%H%M%S))*1e7+1164447360) +trust_timestamp = long(time.mktime(entry['modifytimestamp'][0].timetuple())*1e7+1164447360) for dom in realm_domains['associateddomain']: ftinfo = dict() -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 22-23 webui tests extended by checking field disable property
Hi, first patch extends webui tests with a callback function, and an assert_disabled function, to check if a field is disabled under certain conditions. Second patch extends range tests with this checking functionality depending on range types. Thanks AdamFrom ba58847116ea90e129ba009d00f50337b5eee32e Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Tue, 6 May 2014 16:47:35 +0200 Subject: [PATCH] webui tests: callback, assert_disabled feature added Added a callback feature to webui tests, to extend functionality. Also added assert_disabled function to ui_driver, to check if a field is disabled in the browser. --- ipatests/test_webui/ui_driver.py | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py index 7cfe21ad8985b04fcb296adccf0277a5f02833b9..1f695fb279ace2f47a31bf7e7feebf180bf4e65a 100644 --- a/ipatests/test_webui/ui_driver.py +++ b/ipatests/test_webui/ui_driver.py @@ -1000,7 +1000,7 @@ class UI_driver(object): key = field[1] val = field[2] -if undo: +if undo and not hasattr(key, '__call__'): self.assert_undo_button(key, False, parent) if widget_type == 'textbox': @@ -1025,8 +1025,13 @@ class UI_driver(object): self.fill_multivalued(key, val, parent) elif widget_type == 'table': self.select_record(val, parent, key) +# this meta field specifies a function, to extend functionality of +# field checking +elif widget_type == 'callback': +if hasattr(key, '__call__'): +key(val) self.wait() -if undo: +if undo and not hasattr(key, '__call__'): self.assert_undo_button(key, True, parent) def validate_fields(self, fields, parent=None): @@ -1551,6 +1556,19 @@ class UI_driver(object): else: assert visible, Element not visible: %s % selector +def assert_disabled(self, selector, parent=None, negative=False): + +Assert that element defined by selector is disabled + +selector += [disabled] +if not parent: +parent = self.get_form() +el = self.find(selector, By.CSS_SELECTOR, parent) +if negative: +assert el is None, Element not disabled: %s % selector +else: +assert el, Element disabled: %s % selector + def assert_record(self, pkey, parent=None, table_name=None, negative=False): Assert that record is in current search table -- 1.9.0 From 01b00f7a735c8224619460d05ac239d0a42dc94b Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Tue, 6 May 2014 16:49:03 +0200 Subject: [PATCH] webui tests: range test extended Range test extended with checking of disabled field according to trust types. --- ipatests/test_webui/task_range.py | 9 + 1 file changed, 9 insertions(+) diff --git a/ipatests/test_webui/task_range.py b/ipatests/test_webui/task_range.py index 3b9c84a96be00cbe556c04b7c29028c2b2f21d0c..d46d345f03a2b50730e3107ef6f7cda4465c 100644 --- a/ipatests/test_webui/task_range.py +++ b/ipatests/test_webui/task_range.py @@ -95,6 +95,7 @@ class range_tasks(UI_driver): ('textbox', 'ipaidrangesize', str(size)), ('textbox', 'ipabaserid', str(base_rid)), ('radio', 'iparangetype', range_type), +('callback', self.check_range_type_mod, range_type) ] if not sid: @@ -105,3 +106,11 @@ class range_tasks(UI_driver): add.append(('textbox', 'ipanttrusteddomainsid', sid)) return add + +def check_range_type_mod(self, range_type): +if range_type == 'ipa-local': +self.assert_disabled([name=ipanttrusteddomainsid]) +self.assert_disabled([name=ipasecondarybaserid], negative=True) +elif range_type == 'ipa-ad-trust': +self.assert_disabled([name=ipanttrusteddomainsid], negative=True) +self.assert_disabled([name=ipasecondarybaserid]) -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 587 webui-ci: adjust id range tests to new validator
On Fri, 25 Apr 2014 15:02:27 +0200 Petr Vobornik pvobo...@redhat.com wrote: SSIA LGFM, integration tests for range now runs smoothly. ACK Thanks Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 588 webui: fix switching between multiple_choice_section choices
On Fri, 25 Apr 2014 19:24:35 +0200 Petr Vobornik pvobo...@redhat.com wrote: - required indicators are not present for all sections except the last - validation has wrong color for the same sections There was only one layout for all choices. Layout should not be reused because `create` method will reset layout's rows therefore it worked properly only for the last choice. https://fedorahosted.org/freeipa/ticket/4327 Works as expected, integration and manual tests ran. ACK Thanks Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 18 webui otptoken test data added
On Wed, 30 Apr 2014 13:37:10 +0200 Petr Vobornik pvobo...@redhat.com wrote: On 29.4.2014 16:30, Misnyovszki Adam wrote: On Fri, 25 Apr 2014 17:16:48 +0200 Misnyovszki Adam amisn...@redhat.com wrote: Hi, this patch adds some static test data for the webui otptoken part. Adam Attached corrected DN's. Thanks Adam 1) Why otptoken_batch_del.json ends with error? Also there might be a defect in UI that for batch delete operation it asks for batch.json and not $ENTITY_batch_del.json making otptoken_batch_del.json unused - out of scope of this patch. 2) Why otptoken_mod.json ends with error? 3) otptoken_find.json is not needed since the search facet uses paging (combination of otptoken_get_records.json and otptoken_find_pkeys.json is enough). In general, it's OK to fake the data if there is some bug which causes errors and we know that it will be fixed. Hi, see the attached, and corrected 18 patch for otptoken static test data. Also, I've added patch 20, for fixing the batch_del command in static webui tests. Thanks AdamFrom 22577cf672128231cb4b2ced7e7ee1c12da664c7 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 30 Apr 2014 17:53:52 +0200 Subject: [PATCH 1/2] webui OTP token test data added --- install/ui/test/data/otptoken_add.json | 43 +++ install/ui/test/data/otptoken_batch.json | 27 ++ install/ui/test/data/otptoken_batch_del.json | 27 ++ install/ui/test/data/otptoken_batch_mod.json | 34 install/ui/test/data/otptoken_find_pkeys.json | 17 ++ install/ui/test/data/otptoken_get_records.json | 57 install/ui/test/data/otptoken_mod.json | 72 ++ install/ui/test/data/otptoken_show.json| 51 ++ 8 files changed, 328 insertions(+) create mode 100644 install/ui/test/data/otptoken_add.json create mode 100644 install/ui/test/data/otptoken_batch.json create mode 100644 install/ui/test/data/otptoken_batch_del.json create mode 100644 install/ui/test/data/otptoken_batch_mod.json create mode 100644 install/ui/test/data/otptoken_find_pkeys.json create mode 100644 install/ui/test/data/otptoken_get_records.json create mode 100644 install/ui/test/data/otptoken_mod.json create mode 100644 install/ui/test/data/otptoken_show.json diff --git a/install/ui/test/data/otptoken_add.json b/install/ui/test/data/otptoken_add.json new file mode 100644 index ..c52fc15035e0aad025294a8da1f938ee53c8e5a9 --- /dev/null +++ b/install/ui/test/data/otptoken_add.json @@ -0,0 +1,43 @@ +{ +error: null, +id: null, +result: { +result: { +dn: ipatokenuniqueid=10bd43b5-3204-4695-9225-91064f6c77b3,cn=otp,dc=example,dc=com, +ipatokenmodel: [ +totp +], +ipatokenotpalgorithm: [ +sha1 +], +ipatokenotpdigits: [ +6 +], +ipatokenotpkey: [ +{ +__base64__: 2TUYXOVTaZf/Og== +} +], +ipatokentotpclockoffset: [ +0 +], +ipatokentotptimestep: [ +30 +], +ipatokenuniqueid: [ +footoken +], +ipatokenvendor: [ +FreeIPA +], +objectclass: [ +top, +ipatokentotp, +ipatoken +], +uri: otpauth://totp/EXAMPLE.COM:10bd43b5-3204-4695-9225-91064f6c77b3?digits=6secret=3E2RQXHFKNUZP7Z2period=30algorithm=sha1issuer=EXAMPLE.COM +}, +summary: Added OTP token \10bd43b5-3204-4695-9225-91064f6c77b3\, +value: 10bd43b5-3204-4695-9225-91064f6c77b3 +} +} diff --git a/install/ui/test/data/otptoken_batch.json b/install/ui/test/data/otptoken_batch.json new file mode 100644 index ..059b53f96ebe34036394b969f12903e8b52d69fa --- /dev/null +++ b/install/ui/test/data/otptoken_batch.json @@ -0,0 +1,27 @@ +{ +error: null, +id: null, +result: { +count: 1, +messages: [ +{ +code: 13001, +message: API Version number was not sent, forward compatibility not guaranteed. Assuming server's API version, 2.83, +name: VersionMissing, +type: warning +} +], +results: [ +{ +error: null, +result: { +failed: [] +}, +summary: Deleted OTP token \10bd43b5-3204-4695-9225-91064f6c77b3\, +value: [ +10bd43b5-3204-4695-9225-91064f6c77b3 +] +} +] +}, +} diff --git a/install/ui/test/data/otptoken_batch_del.json b/install/ui/test/data
[Freeipa-devel] plugin registration refactoring for pwpolicy
SSIA Thanks AdamFrom eece77de2869e484f0cfede4e05205026fecd709 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 2 May 2014 13:52:49 +0200 Subject: [PATCH] plugin registration refactoring for pwpolicy decorators used for plugin registration in pwpolicy according to: http://www.freeipa.org/page/Coding_Best_Practices#Decorator-based_plugin_registration --- ipalib/plugins/pwpolicy.py | 39 +++ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/ipalib/plugins/pwpolicy.py b/ipalib/plugins/pwpolicy.py index 1d546ea75be61f9bf5b0ab2f571b7d98c9cc2ac1..9c8b711b89db578d1dd771b5fe3ae89d1d88ccf3 100644 --- a/ipalib/plugins/pwpolicy.py +++ b/ipalib/plugins/pwpolicy.py @@ -22,6 +22,7 @@ from ipalib import api from ipalib import Int, Str, DNParam from ipalib.plugins.baseldap import * from ipalib import _ +from ipalib.plugable import Registry from ipalib.request import context from ipapython.ipautil import run from ipapython.dn import DN @@ -70,6 +71,9 @@ EXAMPLES: ipa pwpolicy-mod --minclasses=2 localadmins ) +register = Registry() + +@register() class cosentry(LDAPObject): Class of Service object used for linking policies with groups @@ -127,9 +131,8 @@ class cosentry(LDAPObject): } ) -api.register(cosentry) - +@register() class cosentry_add(LDAPCreate): NO_CLI = True @@ -145,15 +148,13 @@ class cosentry_add(LDAPCreate): del entry_attrs['cn'] return dn -api.register(cosentry_add) - +@register() class cosentry_del(LDAPDelete): NO_CLI = True -api.register(cosentry_del) - +@register() class cosentry_mod(LDAPUpdate): NO_CLI = True @@ -169,24 +170,21 @@ class cosentry_mod(LDAPUpdate): self.obj.check_priority_uniqueness(*keys, **options) return dn -api.register(cosentry_mod) - +@register() class cosentry_show(LDAPRetrieve): NO_CLI = True -api.register(cosentry_show) - +@register() class cosentry_find(LDAPSearch): NO_CLI = True -api.register(cosentry_find) - global_policy_name = 'global_policy' global_policy_dn = DN(('cn', global_policy_name), ('cn', api.env.realm), ('cn', 'kerberos'), api.env.basedn) +@register() class pwpolicy(LDAPObject): Password Policy object @@ -368,9 +366,8 @@ class pwpolicy(LDAPObject): entry['attributelevelrights']['cospriority'] = \ cos_entry['attributelevelrights']['cospriority'] -api.register(pwpolicy) - +@register() class pwpolicy_add(LDAPCreate): __doc__ = _('Add a new group password policy.') @@ -395,9 +392,8 @@ class pwpolicy_add(LDAPCreate): self.obj.convert_time_for_output(entry_attrs, **options) return dn -api.register(pwpolicy_add) - +@register() class pwpolicy_del(LDAPDelete): __doc__ = _('Delete a group password policy.') @@ -423,9 +419,8 @@ class pwpolicy_del(LDAPDelete): pass return True -api.register(pwpolicy_del) - +@register() class pwpolicy_mod(LDAPUpdate): __doc__ = _('Modify a group password policy.') @@ -467,9 +462,8 @@ class pwpolicy_mod(LDAPUpdate): return raise exc -api.register(pwpolicy_mod) - +@register() class pwpolicy_show(LDAPRetrieve): __doc__ = _('Display information about password policy.') @@ -497,9 +491,8 @@ class pwpolicy_show(LDAPRetrieve): self.obj.convert_time_for_output(entry_attrs, **options) return dn -api.register(pwpolicy_show) - +@register() class pwpolicy_find(LDAPSearch): __doc__ = _('Search for group password policies.') @@ -546,5 +539,3 @@ class pwpolicy_find(LDAPSearch): pass return truncated - -api.register(pwpolicy_find) -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] webui: regression - enable fields on idrange type change (add)
On Fri, 25 Apr 2014 15:01:36 +0200 Petr Vobornik pvobo...@redhat.com wrote: ID range adder dialog was not properly addressed in field binding refactoring. The usage of reset caused some weird loops. https://fedorahosted.org/freeipa/ticket/4326 tests with and without trusts ran smoothly, manual tests also, so ACK Thanks Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 18 webui otptoken test data added
On Fri, 25 Apr 2014 17:16:48 +0200 Misnyovszki Adam amisn...@redhat.com wrote: Hi, this patch adds some static test data for the webui otptoken part. Adam Attached corrected DN's. Thanks AdamFrom e5816ae2dca48841c7c3b3edf591257b89fcb49b Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 25 Apr 2014 16:33:11 +0200 Subject: [PATCH] webui OTP token test data added --- install/ui/test/data/otptoken_add.json | 43 +++ install/ui/test/data/otptoken_batch_del.json | 14 +++ install/ui/test/data/otptoken_batch_mod.json | 34 +++ install/ui/test/data/otptoken_del.json | 11 + install/ui/test/data/otptoken_find.json| 54 install/ui/test/data/otptoken_find_pkeys.json | 17 install/ui/test/data/otptoken_get_records.json | 57 ++ install/ui/test/data/otptoken_mod.json | 8 install/ui/test/data/otptoken_show.json| 51 +++ 9 files changed, 289 insertions(+) create mode 100644 install/ui/test/data/otptoken_add.json create mode 100644 install/ui/test/data/otptoken_batch_del.json create mode 100644 install/ui/test/data/otptoken_batch_mod.json create mode 100644 install/ui/test/data/otptoken_del.json create mode 100644 install/ui/test/data/otptoken_find.json create mode 100644 install/ui/test/data/otptoken_find_pkeys.json create mode 100644 install/ui/test/data/otptoken_get_records.json create mode 100644 install/ui/test/data/otptoken_mod.json create mode 100644 install/ui/test/data/otptoken_show.json diff --git a/install/ui/test/data/otptoken_add.json b/install/ui/test/data/otptoken_add.json new file mode 100644 index ..96170d4419ef92ed2f7768ec023c26b35e14548d --- /dev/null +++ b/install/ui/test/data/otptoken_add.json @@ -0,0 +1,43 @@ +{ +error: null, +id: null, +result: { +result: { +dn: ipatokenuniqueid=footoken,cn=otp,dc=example,dc=com, +ipatokenmodel: [ +totp +], +ipatokenotpalgorithm: [ +sha1 +], +ipatokenotpdigits: [ +6 +], +ipatokenotpkey: [ +{ +__base64__: 2TUYXOVTaZf/Og== +} +], +ipatokentotpclockoffset: [ +0 +], +ipatokentotptimestep: [ +30 +], +ipatokenuniqueid: [ +footoken +], +ipatokenvendor: [ +FreeIPA +], +objectclass: [ +top, +ipatokentotp, +ipatoken +], +uri: otpauth://totp/EXAMPLE.COM:footoken?digits=6secret=3E2RQXHFKNUZP7Z2period=30algorithm=sha1issuer=EXAMPLE.COM +}, +summary: Added OTP token \footoken\, +value: footoken +} +} diff --git a/install/ui/test/data/otptoken_batch_del.json b/install/ui/test/data/otptoken_batch_del.json new file mode 100644 index ..8fb6d701d2f4741922482127e54f9a9b6503d43c --- /dev/null +++ b/install/ui/test/data/otptoken_batch_del.json @@ -0,0 +1,14 @@ +{ +error: null, +id: null, +result: { +count: 1, +results: [ +{ +error: footoken: OTP token not found, +error_code: 4001, +error_name: NotFound +} +] +} +} \ No newline at end of file diff --git a/install/ui/test/data/otptoken_batch_mod.json b/install/ui/test/data/otptoken_batch_mod.json new file mode 100644 index ..63b99b684ee2ee8aaede06f4f5f6d8080c71fe8c --- /dev/null +++ b/install/ui/test/data/otptoken_batch_mod.json @@ -0,0 +1,34 @@ +{ +error: null, +id: null, +result: { +count: 1, +results: [ +{ +error: null, +result: { +description: [ +Description +], +ipatokendisabled: [ +FALSE +], +ipatokenmodel: [ +totp +], +ipatokenowner: [ +admin +], +ipatokenuniqueid: [ +10bd43b5-3204-4695-9225-91064f6c77b3 +], +ipatokenvendor: [ +FreeIPA +] +}, +summary: Modified OTP token \10bd43b5-3204-4695-9225-91064f6c77b3\, +value: 10bd43b5-3204-4695-9225-91064f6c77b3 +} +] +} +} \ No newline at end of file diff --git a/install/ui/test/data/otptoken_del.json b/install/ui/test/data/otptoken_del.json new
[Freeipa-devel] [PATCH] 16-17 Attribute box in permission UI is too small
Hi, first patch redesigns attribute box in permission forms, making it a bigger scrollable checkboxlist. Second one adds a filter field to it for better user experience, if the checkboxlist would be too large. Also, webui unit tests for rbac are updated to work properly with the new widget. Thanks AdamFrom e12c8341b8ce10a32841cff8baca03c6b00b14d4 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 25 Apr 2014 12:54:17 +0200 Subject: [PATCH 1/2] Attribute box in permission UI is too small Attributes widget modified to display checkbox list with a limited height scrollable div. Check all attributes option is removed to keep the user read through the attributes which he selects. Webui integration tests modified according to new widget layout. https://fedorahosted.org/freeipa/ticket/4253 --- install/ui/ipa.css | 12 ++ install/ui/src/freeipa/aci.js| 51 ++-- ipatests/test_webui/test_rbac.py | 6 ++--- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/install/ui/ipa.css b/install/ui/ipa.css index 835ec1cc6c81f86e589f71e2d21450363c260850..d4c1c8c31878bddc77324e2d9e87e3ebb4ba2591 100644 --- a/install/ui/ipa.css +++ b/install/ui/ipa.css @@ -992,6 +992,18 @@ table.scrollable tbody { max-width: 150px; } +.option_widget.columns.attribute_widget { +overflow-y: auto; +max-height: 36em; +} + +.option_widget.columns.attribute_widget li { +float: left; +width: 50%; +min-width: 90px; +max-width: 200px; +} + .combobox-widget-input { display: inline-block; position: relative; diff --git a/install/ui/src/freeipa/aci.js b/install/ui/src/freeipa/aci.js index e26ecd27d9987f629415c45f36cd8be410bf4c3b..2a0c669d1b90b3662e3b59fb00bb9b739296775c 100644 --- a/install/ui/src/freeipa/aci.js +++ b/install/ui/src/freeipa/aci.js @@ -556,36 +556,15 @@ aci.attributes_widget = function(spec) { that.create = function(container) { that.container = container; -var attr_container = $('div/', { -'class': 'aci-attribute-table-container' +that.$node = that.attr_container = attr_container = $('div/', { +'class': 'widget radio-widget' }).appendTo(container); -that.$node = that.table = $('table/', { -id: id, -name: that.name, -'class': 'search-table aci-attribute-table scrollable' -}). -append('thead/'). -append('tbody/'). -appendTo(attr_container); - -var tr = $('tr/tr').appendTo($('thead', that.table)); - -var th = $('th/').appendTo(tr); -IPA.standalone_option({ -type: checkbox, -click: function() { -$('.aci-attribute', that.table). -prop('checked', $(this).prop('checked')); -that.value_changed.notify([], that); -that.emit('value-change', { source: that }); -} -}, th); - -tr.append($('th/', { -'class': 'aci-attribute-column', -html: text.get('@i18n:objects.aci.attribute') -})); +var ul = $('ul/',{ +'class' : 'option_widget columns attribute_widget', +'id' : id, +'name' : that.name +}).appendTo(attr_container); if (that.undo) { that.create_undo(container); @@ -599,14 +578,13 @@ aci.attributes_widget = function(spec) { }; that.create_options = function(options) { -var tbody = $('tbody', that.table); +var ul = $('ul.attribute_widget', that.attr_container); for (var i=0; ioptions.length ; i++){ var option = options[i]; var value = option.value.toLowerCase(); -var tr = $('tr/').appendTo(tbody); +var li = $('li/').appendTo(ul); -var td = $('td/').appendTo(tr); var name = that.get_input_name(); var id = that._option_next_id + name; var opt = IPA.standalone_option({ @@ -619,12 +597,7 @@ aci.attributes_widget = function(spec) { that.value_changed.notify([], that); that.emit('value-change', { source: that }); } -}, td); -td = $('td/').appendTo(tr); -td.append($('label/',{ -text: value, -'for': id -})); +}, li, value); option.input_node = opt[0]; that.new_option_id(); } @@ -653,7 +626,7 @@ aci.attributes_widget = function(spec) { that.populate = function(object_type) { -$('tbody tr', that.table).remove(); +$('ul.attribute_widget li', that.attr_container).remove(); if (!object_type || object_type === '') return; @@ -1081,4 +1054,4 @@ aci.register = function() { phases.on('registration', aci.register); return aci; -}); \ No newline at end of file
[Freeipa-devel] [PATCH] 18 webui otptoken test data added
Hi, this patch adds some static test data for the webui otptoken part. AdamFrom a119f23cde594a0c9a4a2bf3cb91d259c5ce06b1 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 25 Apr 2014 16:33:11 +0200 Subject: [PATCH] webui OTP token test data added --- install/ui/test/data/otptoken_add.json | 43 +++ install/ui/test/data/otptoken_batch_del.json | 14 +++ install/ui/test/data/otptoken_batch_mod.json | 34 +++ install/ui/test/data/otptoken_del.json | 11 + install/ui/test/data/otptoken_find.json| 54 install/ui/test/data/otptoken_find_pkeys.json | 17 install/ui/test/data/otptoken_get_records.json | 57 ++ install/ui/test/data/otptoken_mod.json | 9 install/ui/test/data/otptoken_show.json| 51 +++ 9 files changed, 290 insertions(+) create mode 100644 install/ui/test/data/otptoken_add.json create mode 100644 install/ui/test/data/otptoken_batch_del.json create mode 100644 install/ui/test/data/otptoken_batch_mod.json create mode 100644 install/ui/test/data/otptoken_del.json create mode 100644 install/ui/test/data/otptoken_find.json create mode 100644 install/ui/test/data/otptoken_find_pkeys.json create mode 100644 install/ui/test/data/otptoken_get_records.json create mode 100644 install/ui/test/data/otptoken_mod.json create mode 100644 install/ui/test/data/otptoken_show.json diff --git a/install/ui/test/data/otptoken_add.json b/install/ui/test/data/otptoken_add.json new file mode 100644 index ..5b20d1271c51d6afc5ff80cdcab580be89ccb231 --- /dev/null +++ b/install/ui/test/data/otptoken_add.json @@ -0,0 +1,43 @@ +{ +error: null, +id: null, +result: { +result: { +dn: ipatokenuniqueid=footoken,cn=otp,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com, +ipatokenmodel: [ +totp +], +ipatokenotpalgorithm: [ +sha1 +], +ipatokenotpdigits: [ +6 +], +ipatokenotpkey: [ +{ +__base64__: 2TUYXOVTaZf/Og== +} +], +ipatokentotpclockoffset: [ +0 +], +ipatokentotptimestep: [ +30 +], +ipatokenuniqueid: [ +footoken +], +ipatokenvendor: [ +FreeIPA +], +objectclass: [ +top, +ipatokentotp, +ipatoken +], +uri: otpauth://totp/IDM.LAB.ENG.BRQ.REDHAT.COM:footoken?digits=6secret=3E2RQXHFKNUZP7Z2period=30algorithm=sha1issuer=IDM.LAB.ENG.BRQ.REDHAT.COM +}, +summary: Added OTP token \footoken\, +value: footoken +} +} \ No newline at end of file diff --git a/install/ui/test/data/otptoken_batch_del.json b/install/ui/test/data/otptoken_batch_del.json new file mode 100644 index ..8fb6d701d2f4741922482127e54f9a9b6503d43c --- /dev/null +++ b/install/ui/test/data/otptoken_batch_del.json @@ -0,0 +1,14 @@ +{ +error: null, +id: null, +result: { +count: 1, +results: [ +{ +error: footoken: OTP token not found, +error_code: 4001, +error_name: NotFound +} +] +} +} \ No newline at end of file diff --git a/install/ui/test/data/otptoken_batch_mod.json b/install/ui/test/data/otptoken_batch_mod.json new file mode 100644 index ..63b99b684ee2ee8aaede06f4f5f6d8080c71fe8c --- /dev/null +++ b/install/ui/test/data/otptoken_batch_mod.json @@ -0,0 +1,34 @@ +{ +error: null, +id: null, +result: { +count: 1, +results: [ +{ +error: null, +result: { +description: [ +Description +], +ipatokendisabled: [ +FALSE +], +ipatokenmodel: [ +totp +], +ipatokenowner: [ +admin +], +ipatokenuniqueid: [ +10bd43b5-3204-4695-9225-91064f6c77b3 +], +ipatokenvendor: [ +FreeIPA +] +}, +summary: Modified OTP token \10bd43b5-3204-4695-9225-91064f6c77b3\, +value: 10bd43b5-3204-4695-9225-91064f6c77b3 +} +] +} +} \ No newline at end of file diff --git a/install/ui/test/data/otptoken_del.json b/install/ui/test/data/otptoken_del.json new file mode 100644 index
Re: [Freeipa-devel] [PATCH] 14 webui: select all checkbox remains selected after operation
On Wed, 23 Apr 2014 16:57:35 +0200 Petr Vobornik pvobo...@redhat.com wrote: On 18.4.2014 10:43, Misnyovszki Adam wrote: Hi, this patch fixes select_all checkbox issue, after any bulk modify or delete operation, the checkbox is deselected. https://fedorahosted.org/freeipa/ticket/4245 Thanks Adam The issue still exists in association facets and also maybe in attribute facet (group/external) (not tested). Hi, thanks for the review, see the attached corrections! AdamFrom 53e406c33d51a3af3c83cab079ab81374d05a91e Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 23 Apr 2014 17:41:45 +0200 Subject: [PATCH] webui: select all checkbox remains selected after operation The select all checkbox remained selected after bulk operation. This patch fixes it, after any bulk modify or delete operation, unselect_all function is called. https://fedorahosted.org/freeipa/ticket/4245 --- install/ui/src/freeipa/association.js | 2 ++ install/ui/src/freeipa/dialog.js | 6 +- install/ui/src/freeipa/search.js | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js index bf11de0051bdd88fa5bfb58122a5ede95e10bab2..aee95f184cd2543f56bfc63a508be797628e76d6 100644 --- a/install/ui/src/freeipa/association.js +++ b/install/ui/src/freeipa/association.js @@ -1203,6 +1203,7 @@ exp.association_facet = IPA.association_facet = function (spec, no_init) { method: that.remove_method, on_success: function(data) { that.refresh(); +that.table.unselect_all(); var succeeded = IPA.get_succeeded(data); var msg = text.get('@i18n:association.removed').replace('${count}', succeeded); @@ -1473,6 +1474,7 @@ exp.attribute_facet = IPA.attribute_facet = function(spec, no_init) { function(data) { that.load(data); that.show_content(); +that.table.unselect_all(); var succeeded = IPA.get_succeeded(data); var msg = text.get('@i18n:association.removed').replace('${count}', succeeded); diff --git a/install/ui/src/freeipa/dialog.js b/install/ui/src/freeipa/dialog.js index 4c6c37f88e628aaf93f353d245bd2763db830529..6fdfbc62da48cf6fbb31a9a467bb989cbf78ec18 100644 --- a/install/ui/src/freeipa/dialog.js +++ b/install/ui/src/freeipa/dialog.js @@ -885,6 +885,8 @@ IPA.adder_dialog = function(spec) { that.add = function() { var rows = that.available_table.remove_selected_rows(); that.selected_table.add_rows(rows); +that.available_table.unselect_all(); +that.selected_table.unselect_all(); }; /** @@ -893,6 +895,8 @@ IPA.adder_dialog = function(spec) { that.remove = function() { var rows = that.selected_table.remove_selected_rows(); that.available_table.add_rows(rows); +that.available_table.unselect_all(); +that.selected_table.unselect_all(); }; /** @@ -1357,4 +1361,4 @@ dialog_builder.factory = IPA.dialog; reg.set('dialog', dialog_builder.registry); return {}; -}); \ No newline at end of file +}); diff --git a/install/ui/src/freeipa/search.js b/install/ui/src/freeipa/search.js index 9400b6aec133935fc7c3ed21c695fe3c6bc7b7de..8701c33c3b3752cdeddffebe3cb325d26ad81dee 100644 --- a/install/ui/src/freeipa/search.js +++ b/install/ui/src/freeipa/search.js @@ -353,6 +353,7 @@ IPA.search_deleter_dialog = function(spec) { batch.on_success = function(data, text_status, xhr) { that.facet.refresh(); that.facet.on_update.notify([],that.facet); +that.facet.table.unselect_all(); var succeeded = batch.commands.length - batch.errors.errors.length; var msg = text.get('@i18n:search.deleted').replace('${count}', succeeded); IPA.notify_success(msg); @@ -505,6 +506,7 @@ IPA.batch_items_action = function(spec) { that.on_success = function(facet, data, text_status, xhr) { facet.on_update.notify(); facet.refresh(); +facet.table.unselect_all(); if (that.success_msg) { var succeeded = that.batch.commands.length - that.batch.errors.errors.length; -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 13 - Log pretty-printed request and response
On Wed, 16 Apr 2014 11:42:00 -0400 Rob Crittenden rcrit...@redhat.com wrote: Misnyovszki Adam wrote: Hi, this patch enables logging json dumps of request and response, using the --log-payload switch in ipa cli. RFC tag is to ensure that I handled the --log-payload switch correctly in ipa cli. Be careful, it only logs, so --log-payload without -v switch doesn't make the dump visible in command line, -v does! https://fedorahosted.org/freeipa/ticket/4233 Not a NACK but using -vvv makes this a much simpler operation as you can then just compare verbose = 3. This seems like a lot of work just to pretty-print some output. rob I've found out, that in RPCClient.create_connection, according to ipalib/backend.py:164, the variable verbose is not an int, rather a bool ( verbose=(self.env.verbose = 2) ), so I decided not to break the workflow of this variable, but rather create a new one(log-payload). I was thinking, making verbose to an int would cause more work than to do it this way. Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 15 webui doc: typo fixes in guides
Hi, $SUBJ tells everything. Thanks AdamFrom 38ecbfc95dde8f2a968165e1db42922c9a8b8fa1 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 11 Apr 2014 19:31:19 +0200 Subject: [PATCH] webui doc: typo fixes in guides --- install/ui/doc/guides.json | 4 ++-- install/ui/doc/guides/debugging_web_ui/README.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/install/ui/doc/guides.json b/install/ui/doc/guides.json index e85f88a3ea69b14af8dbb5ba3a81eb002137da20..bbed8e8d96b69ae8dd0b23681a791d193460543d 100644 --- a/install/ui/doc/guides.json +++ b/install/ui/doc/guides.json @@ -23,7 +23,7 @@ { name: Phases, url: guides/phases, -title: Appliaciton phases, +title: Application phases, description: Introduction to application phases }, { @@ -34,4 +34,4 @@ } ] } -] \ No newline at end of file +] diff --git a/install/ui/doc/guides/debugging_web_ui/README.md b/install/ui/doc/guides/debugging_web_ui/README.md index bac3c30a6d697dd6d87880647c5e346a1f5b145c..1bf9bd9a22e050b4dd6835080a1fae79f9205898 100644 --- a/install/ui/doc/guides/debugging_web_ui/README.md +++ b/install/ui/doc/guides/debugging_web_ui/README.md @@ -67,7 +67,7 @@ Notes: ## Conclusion -While reporting an UI bug it's good the check if there is some JavaScript error and if so send a call stack with line numbers, preferably the ones by using source codes. If source codes are not available, pretty print function should be used and send also code (~15 lines on both sides) around the bug. +While reporting an UI bug it's good to check if there is some JavaScript error and if so, send a call stack with line numbers, preferably the ones by using source codes. If source codes are not available, pretty print function should be used and send also code (~15 lines on both sides) around the bug. The most valuable information in order of preference are: -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 12 Call generate-rndc-key.sh during ipa-server-install
On Thu, 17 Apr 2014 16:21:19 +0200 Martin Kosek mko...@redhat.com wrote: On 04/17/2014 04:10 PM, Rob Crittenden wrote: Misnyovszki Adam wrote: Hi, this patch modifies ipa-server-install to warn the user, if there is a lack of entropy, also runs generate-rndc-key.sh before named restart, to ensure, that it can start before systemd timeouts. I think the exception should be logged in check_entropy() in case this every does fail (the file name changes, the format changes, etc). There should be a try/except around the run() call. I noticed that /etc/rndc.key isn't removed on uninstall, which I guess means the same key will be re-used. Should we be removing that? rob Also, bare exceptions are bad! +except: +service.print_msg(Could not determine entropy, possible long delays) Next, you do all the checks in ipa-server-install, while they should be in service files, like krbinstance.py so that it is also checked in other installers, like ipa-replica-install. Same for DNS, it should be a separate step in bindinstance.py so that when the installation is hanging, you can see [X/Y] Generating rndc key file and know that it is hanging on that part. I would not misuse service.print_msg for regular messages, I would only do the service.print_msg(WARNING: Your system is running out of entropy, expect long delays!) others can be either turn into separate installation step or debug log message. Martin Hi, according to personal discussion with Martin, see the corrected patch! Thanks Adam From 13b267ed4a06c8c3a2f6ed74b2ef7d7ba55c0f36 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 18 Apr 2014 15:44:11 +0200 Subject: [PATCH] Call generate-rndc-key.sh during ipa-server-install Since systemd has by default a 2 minute timeout to start a service, the end of ipa-server-install might fail because starting named times out. This patch ensures that generate-rndc-key.sh runs before named service restart. Also, warning message is displayed before KDC install and generate-rndc-key.sh, if there is a lack of entropy, to notify the user that the process could take more time than expected. https://fedorahosted.org/freeipa/ticket/4210 --- ipaserver/install/bindinstance.py | 7 +++ ipaserver/install/installutils.py | 20 +++- ipaserver/install/krbinstance.py | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index 613af5c9139a3f52102a6baadcff017d64b60c3e..c5ff76726ddd6d0c1abcec353badd636af81395e 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -523,6 +523,9 @@ class BindInstance(service.Service): if installutils.record_in_hosts(self.ip_address, self.fqdn) is None: installutils.add_record_to_hosts(self.ip_address, self.fqdn) +# Make sure generate-rndc-key.sh runs before named restart +self.step(generating rndc key file, self.__generate_rndc_key) + if self.first_instance: self.step(adding DNS container, self.__setup_dns_container) @@ -820,6 +823,10 @@ class BindInstance(service.Service): except IOError as e: root_logger.error('Could not write to resolv.conf: %s', e) +def __generate_rndc_key(self): +installutils.check_entropy() +ipautil.run(['/usr/libexec/generate-rndc-key.sh']) + def add_master_dns_records(self, fqdn, ip_address, realm_name, domain_name, reverse_zone, ntp=False, ca_configured=None): self.fqdn = fqdn diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index daf81e890c33e9a9bd30763ff8d0788313a1dbda..d2662046f569d99f4e2bffbddaa704628e6d1504 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -41,7 +41,7 @@ from ipalib.util import validate_hostname from ipapython import config from ipalib import errors from ipapython.dn import DN -from ipaserver.install import certs +from ipaserver.install import certs, service from ipapython import services as ipaservices # Used to determine install status @@ -846,3 +846,21 @@ def stopped_service(service, instance_name=): finally: root_logger.debug('Starting %s%s.', service, log_instance_name) ipaservices.knownservices[service].start(instance_name) + +def check_entropy(): +''' +Checks if the system has enough entropy, if not, displays warning message +''' +try: +with open('/proc/sys/kernel/random/entropy_avail', 'r') as efname: +if int(efname.read()) 200: +emsg = WARNING: Your system is running out of entropy, expect long delays! +service.print_msg(emsg) +root_logger.debug(emsg) +except IOError as e: +root_logger.debug(Could not open /proc/sys/kernel/random
[Freeipa-devel] [PATCH] 12 Call generate-rndc-key.sh during ipa-server-install
Hi, this patch modifies ipa-server-install to warn the user, if there is a lack of entropy, also runs generate-rndc-key.sh before named restart, to ensure, that it can start before systemd timeouts. Thanks Adam From d405cea8dae5a03ab0f9d429d3251e8be9ae9fe2 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 16 Apr 2014 16:11:33 +0200 Subject: [PATCH] Call generate-rndc-key.sh during ipa-server-install Since systemd has by default a 2 minute timeout to start a service, the end of ipa-server-install might fail because starting named times out. This patch ensures that generate-rndc-key.sh runs before named service restart. Also, warning message is displayed before KDC install and generate-rndc-key.sh, if there is a lack of entropy, to notify the user that the process could take more time than expected. https://fedorahosted.org/freeipa/ticket/4210 --- install/tools/ipa-server-install | 16 1 file changed, 16 insertions(+) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 34393b7df0a95a76b0c2660dcaafca13b21d2dfb..0e8a21cecc50578bc8bea84df3b7dc7afca1624e 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -38,6 +38,7 @@ import nss.error import base64 import pwd import textwrap +import string from optparse import OptionGroup, OptionValueError try: @@ -568,6 +569,14 @@ def set_subject_in_config(realm_name, dm_password, suffix, subject_base): conn.update_entry(entry_attrs) conn.disconnect() +def check_entropy(): +try: +with open('/proc/sys/kernel/random/entropy_avail', 'r') as efname: +if string.atoi(efname.read()) 200: +service.print_msg(WARNING: Your system is running out of entropy, expect long delays!) +except: +service.print_msg(Could not determine entropy, possible long delays) + def main(): global ds @@ -1119,6 +1128,7 @@ def main(): # This is done within stopped_service context, which restarts CA ca.enable_client_auth_to_db() +check_entropy() krb = krbinstance.KrbInstance(fstore) if options.pkinit_pkcs12: krb.create_instance(realm_name, host_name, domain_name, @@ -1175,6 +1185,12 @@ def main(): service.print_msg(Restarting the certificate server) ca.restart(dogtag.configured_constants().PKI_INSTANCE_NAME) +# Make sure generate-rndc-key.sh runs before named restart +if options.setup_dns: +check_entropy() +service.print_msg(Generate rndc key file) +run(['/usr/libexec/generate-rndc-key.sh']) + # Create a BIND instance bind = bindinstance.BindInstance(fstore, dm_password) bind.setup(host_name, ip_address, realm_name, domain_name, dns_forwarders, -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH][RFC] 13 - Log pretty-printed request and response
Hi, this patch enables logging json dumps of request and response, using the --log-payload switch in ipa cli. RFC tag is to ensure that I handled the --log-payload switch correctly in ipa cli. Be careful, it only logs, so --log-payload without -v switch doesn't make the dump visible in command line, -v does! https://fedorahosted.org/freeipa/ticket/4233 Thanks AdamFrom f2230d5200feeb6fa413f4b248736b38ba66d317 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 16 Apr 2014 14:58:18 +0200 Subject: [PATCH] Log pretty-printed request and response With the --log-payload option, every request/response is logged with json.dumps. https://fedorahosted.org/freeipa/ticket/4233 --- ipalib/constants.py | 1 + ipalib/plugable.py | 7 +-- ipalib/rpc.py | 14 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/ipalib/constants.py b/ipalib/constants.py index 6cc50eacf44678840ad0048a1ef60c05736879cb..6acd7cef549d8b06366ee07adcbeb0a4d1b411d2 100644 --- a/ipalib/constants.py +++ b/ipalib/constants.py @@ -158,6 +158,7 @@ DEFAULT_CONFIG = ( ('interactive', True), ('fallback', True), ('delegate', False), +('log_payload', False), # Enable certain optional plugins: ('enable_ra', False), diff --git a/ipalib/plugable.py b/ipalib/plugable.py index 216f9c08a8b5d22bdb1e7853013967e8fe3f88b0..47e52b662f1421f0476fd7b301cd62043448a50d 100644 --- a/ipalib/plugable.py +++ b/ipalib/plugable.py @@ -597,7 +597,10 @@ class API(DictProxy): parser.add_option('-f', '--no-fallback', action='store_false', dest='fallback', help='Only use the server configured in /etc/ipa/default.conf' -) +) +parser.add_option('--log-payload', action='store_true', +help='Logs formatted json payload', +) return parser @@ -617,7 +620,7 @@ class API(DictProxy): pass overrides[str(key.strip())] = value.strip() for key in ('conf', 'debug', 'verbose', 'prompt_all', 'interactive', -'fallback', 'delegate'): +'fallback', 'delegate', 'log_payload'): value = getattr(options, key, None) if value is not None: overrides[key] = value diff --git a/ipalib/rpc.py b/ipalib/rpc.py index 2b47d1c0e25bbeec0dde38089f444e0399e1670e..fa13e5519de51a2a2e341fb94ca452f71087d102 100644 --- a/ipalib/rpc.py +++ b/ipalib/rpc.py @@ -738,6 +738,8 @@ class RPCClient(Connectible): for url in urls: kw = dict(allow_none=True, encoding='UTF-8') kw['verbose'] = verbose +if self.server_proxy_class == JSONServerProxy: +kw['log_payload'] = self.env.log_payload if url.startswith('https://'): if delegate: transport_class = DelegatedKerbTransport @@ -783,6 +785,7 @@ class RPCClient(Connectible): except Exception, e: # This shouldn't happen if we have a session but it isn't fatal. pass + return self.create_connection(ccache, verbose, fallback, delegate) if not fallback: raise @@ -900,7 +903,8 @@ class xmlclient(RPCClient): class JSONServerProxy(object): -def __init__(self, uri, transport, encoding, verbose, allow_none): +def __init__(self, uri, transport, encoding, verbose, allow_none, +log_payload): type, uri = urllib.splittype(uri) if type not in (http, https): raise IOError(unsupported XML-RPC protocol) @@ -910,6 +914,7 @@ class JSONServerProxy(object): assert encoding == 'UTF-8' assert allow_none self.__verbose = verbose +self.__log_payload = log_payload # FIXME: Some of our code requires ServerProxy internals. # But, xmlrpclib.ServerProxy's _ServerProxy__transport can be accessed @@ -919,6 +924,10 @@ class JSONServerProxy(object): def __request(self, name, args): payload = {'method': unicode(name), 'params': args, 'id': 0} +if self.__log_payload: +root_logger.info('Request: %s', json.dumps(payload, sort_keys=True, + indent=4)) + response = self.__transport.request( self.__host, self.__handler, @@ -931,6 +940,9 @@ class JSONServerProxy(object): except ValueError, e: raise JSONError(str(e)) +if self.__log_payload: +root_logger.info('Response: %s', json.dumps(response, sort_keys=True, +indent=4)) error = response.get('error') if error: try: -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com
Re: [Freeipa-devel] [PATCH] 11 - CI - test_forced_client_reenrollment stability fix
On Wed, 16 Apr 2014 07:59:39 +0200 Martin Kosek mko...@redhat.com wrote: On 04/15/2014 05:36 PM, Misnyovszki Adam wrote: On Tue, 15 Apr 2014 12:51:47 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 04/15/2014 12:41 PM, Misnyovszki Adam wrote: Hi, this patch fixes FreeIPA Jenkins CI test freeipa-integration-forced_client_reenrollment-f19, by turning sshfp records into a set, and sorting them before assertion. https://fedorahosted.org/freeipa/ticket/4298 Greets Adam The list.sort() method sorts in-place and returns None, so now the test would not really test anything. Use the sorted() function. You might want to log the value before returning it. My mistake, see the attached, corrected patch. Thanks Adam Adam, Petr - why can't we use a set as I already proposed in the ticket description? set(['i', 'p', 'a']) == set(['a', 'p', 'i']) True Martin Hi, see the attached patch. Thanks AdamFrom 3c6a371b9c4abfead5d55b8655eb7d047054b1c0 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 16 Apr 2014 16:18:28 +0200 Subject: [PATCH] CI - test_forced_client_reenrollment stability fix fixes FreeIPA Jenkins CI test freeipa-integration-forced_client_reenrollment-f19 https://fedorahosted.org/freeipa/ticket/4298 --- ipatests/test_integration/test_forced_client_reenrollment.py | 4 1 file changed, 4 insertions(+) diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py index 4ba4cda1d4fe509110fffa91e1c13d78b457f64d..cece522f8d81e0de72735f60167393423152c717 100644 --- a/ipatests/test_integration/test_forced_client_reenrollment.py +++ b/ipatests/test_integration/test_forced_client_reenrollment.py @@ -256,6 +256,10 @@ class TestForcedClientReenrollment(IntegrationTest): sshfp_record = line.replace('SSHFP record:', '').strip() assert sshfp_record, 'SSHFP record not found' + +sshfp_record = set(sshfp_record.split(', ')) +self.log.debug(SSHFP record for host %s: %s, client_host, str(sshfp_record)) + return sshfp_record def backup_keytab(self): -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 569-583 New Login Screen
On Tue, 15 Apr 2014 09:39:54 +0200 Petr Vobornik pvobo...@redhat.com wrote: On 11.4.2014 14:31, Misnyovszki Adam wrote: On Fri, 28 Mar 2014 14:04:13 +0100 Petr Vobornik pvobo...@redhat.com wrote: Attached patches replace IPA.unauthorized dialog with new Login Screen. To make it happen, a support for standalone facets had to be developed because current framework was limited by facets dependent on entities and a container with menu. This new feature was already used for Load facet which is part of this patchset and also will be a basis for API browser and OTP sync page. Patches should fix these tickets: https://fedorahosted.org/freeipa/ticket/3903 https://fedorahosted.org/freeipa/ticket/4017 Depends on patches #565-#568. [PATCH] webui: facet container -- A widget which servers as container for facets. FacetContainer is a base class. App is specialization. Doing this abstraction will allow us to implement various facet containers. [PATCH] webui: FormMixin a mixin used for fields validation. Basically implements a logic which is already in details facet and dialog. Now this logic can be used in any component. The long term goal is to replace the logic in details facet and dialog with this mixin. [PATCH] webui: ContainerMixin - A mixin which implements widget storing logic. Similar logic is already implemented in details facet and dialog. Long term goal is to replace that with this one. Separating the logic into mixin makes it usable in other components. [PATCH] webui: standalone facet --- `facet.Facet` is a new base class for facets. It doesn't have any dependencies on entities so it's usable for general purpose facets, e.g., future API browser, load facet or login facet. [PATCH] webui: activity widget -- A widget for showing ongoing activity. Displays a text with changing dots. It listens to `network-activity-start` and `network-activity-end` topics. [PATCH] webui: publish network activity topics -- Network activity is now published through global topics. It allows other components like activity_widget to listen to them. [PATCH] webui: load page Load page is a simple facet which is displayed up to 'runtime' phase. On application start it tells the user that there is ongoing activity. [PATCH] webui: validation summary widget A widget which aggregates warnings and errors and shows them on one place. [PATCH] webui: login screen widget -- Reimplementation of unauthorized dialog into separate widget. It uses RCUE design. New features compared to unauthorized dialog: - reflects auth methods from `auth` module - validation summary - differentiates Kerberos auth failure with session expiration - Caps Lock warning - form based method doesn't allow password only submission https://fedorahosted.org/freeipa/ticket/4017 https://fedorahosted.org/freeipa/ticket/3903 [PATCH] webui: login page - A facet with login sreen widget. [PATCH] webui: authentication module General purpose authentication interface and state. See doc of 'freeipa/auth' module. [PATCH] webui: use asynchronous call for authentication Change `IPA.login_password` and `IPA.get_credentials` to use async AJAX and to return promise instead of blocking the code. IPA.get_credentials is still partially blocking because of negotiate process. We can't do anything about that. It allows activity indicators to do their job. [PATCH] webui: fix combobox styles to work with selenium testing [PATCH] webui-ci: adapt to new login screen [PATCH] webui: remove IPA.unauthorized_dialog Hi, - Attached patch fixes weird combobox behaviour - opens automatically on facet load Thank you. I squashed it into patch 581 since it's a fix for unpushed code. - When trying to log in with password only(username field is empty), there is an error message Authentication with Kerberos failed, which is not the desired behaviour. It should sign that the username field is invalid. New, attached version of patch #577 should fix that. It was a typo. - When trying to log in with kerberos credentials, and the realm of the krb ticket is not the same as the realm of freeipa(eg freeipa realm is IPA.TEST.COM, and the ticket's is TEST.COM), firefox goes into an endless cycle calling the kerberos auth url. Currently it seems to me as a browser issue. Anyways, with correct krb ticket, authentication works fine. As investigated with Adam - not a FreeIPA issue
Re: [Freeipa-devel] [PATCH] 585 webui: fix OTP Token add regression
On Tue, 15 Apr 2014 09:54:22 +0200 Petr Vobornik pvobo...@redhat.com wrote: OTP Token add failed because of invalid function call. qr_widget doesn't contain `on_value_changed` method since it inherits from `IPA.widget` and not from `IPA.input_widget`. Emitting the event was preserved for future possible usage. https://fedorahosted.org/freeipa/ticket/4306 ACK Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 11 - CI - test_forced_client_reenrollment stability fix
On Tue, 15 Apr 2014 12:51:47 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 04/15/2014 12:41 PM, Misnyovszki Adam wrote: Hi, this patch fixes FreeIPA Jenkins CI test freeipa-integration-forced_client_reenrollment-f19, by turning sshfp records into a set, and sorting them before assertion. https://fedorahosted.org/freeipa/ticket/4298 Greets Adam The list.sort() method sorts in-place and returns None, so now the test would not really test anything. Use the sorted() function. You might want to log the value before returning it. My mistake, see the attached, corrected patch. Thanks AdamFrom a738f565c9167759af2c47fa7471d20d8b7783a6 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Thu, 10 Apr 2014 18:44:51 +0200 Subject: [PATCH] CI - test_forced_client_reenrollment stability fix fixes FreeIPA Jenkins CI test freeipa-integration-forced_client_reenrollment-f19 https://fedorahosted.org/freeipa/ticket/4298 --- ipatests/test_integration/test_forced_client_reenrollment.py | 4 1 file changed, 4 insertions(+) diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py index 4ba4cda1d4fe509110fffa91e1c13d78b457f64d..e3bb2b44d42476735ba52c4737668741f0fd5102 100644 --- a/ipatests/test_integration/test_forced_client_reenrollment.py +++ b/ipatests/test_integration/test_forced_client_reenrollment.py @@ -256,6 +256,10 @@ class TestForcedClientReenrollment(IntegrationTest): sshfp_record = line.replace('SSHFP record:', '').strip() assert sshfp_record, 'SSHFP record not found' + +sshfp_record = sorted(sshfp_record.split(', ')) +self.log.debug(SSHFP record for host %s: %s, client_host, str(sshfp_record)) + return sshfp_record def backup_keytab(self): -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 569-583 New Login Screen
On Fri, 28 Mar 2014 14:04:13 +0100 Petr Vobornik pvobo...@redhat.com wrote: Attached patches replace IPA.unauthorized dialog with new Login Screen. To make it happen, a support for standalone facets had to be developed because current framework was limited by facets dependent on entities and a container with menu. This new feature was already used for Load facet which is part of this patchset and also will be a basis for API browser and OTP sync page. Patches should fix these tickets: https://fedorahosted.org/freeipa/ticket/3903 https://fedorahosted.org/freeipa/ticket/4017 Depends on patches #565-#568. [PATCH] webui: facet container -- A widget which servers as container for facets. FacetContainer is a base class. App is specialization. Doing this abstraction will allow us to implement various facet containers. [PATCH] webui: FormMixin a mixin used for fields validation. Basically implements a logic which is already in details facet and dialog. Now this logic can be used in any component. The long term goal is to replace the logic in details facet and dialog with this mixin. [PATCH] webui: ContainerMixin - A mixin which implements widget storing logic. Similar logic is already implemented in details facet and dialog. Long term goal is to replace that with this one. Separating the logic into mixin makes it usable in other components. [PATCH] webui: standalone facet --- `facet.Facet` is a new base class for facets. It doesn't have any dependencies on entities so it's usable for general purpose facets, e.g., future API browser, load facet or login facet. [PATCH] webui: activity widget -- A widget for showing ongoing activity. Displays a text with changing dots. It listens to `network-activity-start` and `network-activity-end` topics. [PATCH] webui: publish network activity topics -- Network activity is now published through global topics. It allows other components like activity_widget to listen to them. [PATCH] webui: load page Load page is a simple facet which is displayed up to 'runtime' phase. On application start it tells the user that there is ongoing activity. [PATCH] webui: validation summary widget A widget which aggregates warnings and errors and shows them on one place. [PATCH] webui: login screen widget -- Reimplementation of unauthorized dialog into separate widget. It uses RCUE design. New features compared to unauthorized dialog: - reflects auth methods from `auth` module - validation summary - differentiates Kerberos auth failure with session expiration - Caps Lock warning - form based method doesn't allow password only submission https://fedorahosted.org/freeipa/ticket/4017 https://fedorahosted.org/freeipa/ticket/3903 [PATCH] webui: login page - A facet with login sreen widget. [PATCH] webui: authentication module General purpose authentication interface and state. See doc of 'freeipa/auth' module. [PATCH] webui: use asynchronous call for authentication Change `IPA.login_password` and `IPA.get_credentials` to use async AJAX and to return promise instead of blocking the code. IPA.get_credentials is still partially blocking because of negotiate process. We can't do anything about that. It allows activity indicators to do their job. [PATCH] webui: fix combobox styles to work with selenium testing [PATCH] webui-ci: adapt to new login screen [PATCH] webui: remove IPA.unauthorized_dialog Hi, - Attached patch fixes weird combobox behaviour - opens automatically on facet load - When trying to log in with password only(username field is empty), there is an error message Authentication with Kerberos failed, which is not the desired behaviour. It should sign that the username field is invalid. - When trying to log in with kerberos credentials, and the realm of the krb ticket is not the same as the realm of freeipa(eg freeipa realm is IPA.TEST.COM, and the ticket's is TEST.COM), firefox goes into an endless cycle calling the kerberos auth url. Currently it seems to me as a browser issue. Anyways, with correct krb ticket, authentication works fine. Although, unit tests ran, integration tests ran as expected, and browsing through the code manually was ok for me, so if that validation issue is corrected, than it will be an ACK. Thanks: Adam From 633b162c414ecc8c156d90d5c4c1860b1e418288 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 4 Apr 2014 16:47:15 +0200 Subject: [PATCH] combobox widget fix this patch fixes the combobox widget, so it doesn't open on
Re: [Freeipa-devel] [PATCH] 0454 Test fixes
On Tue, 25 Mar 2014 10:23:56 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 01/28/2014 03:35 PM, Petr Viktorin wrote: On 01/23/2014 01:54 PM, Petr Viktorin wrote: [...] Patch 454 changes the cert generation script for CA-less tests to use sequential serial numbers rather than random ones, to prevent collisions. This one is still useful though. Ping, could someone review this? all tests ran clean, so ACK Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Tue, 08 Apr 2014 17:31:25 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 04/08/2014 04:17 PM, Misnyovszki Adam wrote: On Mon, 07 Apr 2014 09:43:10 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 03/27/2014 03:37 PM, Misnyovszki Adam wrote: On Wed, 26 Mar 2014 13:15:55 +0100 Petr Viktorin pvikt...@redhat.com wrote: [...] Looks great! I'm just concerned about the error returned when the task takes too long: $ ipa automember-rebuild --type group ipa: ERROR: LDAP timeout I don't think it's sufficiently clear from this that waiting for the task timed out, but the task was actually started successfully. A custom error with a more descriptive message would be useful. Also I've noticed that the nstaskstatus of a successful task is: Automember rebuild task finished. Processed (1) entries. This looks helpful; we could return it as the summary. Hi, both fixed. Greets Adam Sorry for the delay! 'Automember' is a translatable string, so please wrap it in _() when raising TaskTimeout. Also please update the tests. Otherwise with a little rebase it's good to go. Hi, see the attached modifications, tests corrected, and added for no-wait, also rebased for current master. Greets Adam Looks good overall, but why do you now set `self.msg_summary`? Keep in mind that currently the same Command object is reused for every automember_rebuild command, including commands that run in parallel in different threads. It should never be modified. Hi, corrected. Greets Adam From e437d45d5d3b2aaba486e6359b7334bffb657723 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Tue, 25 Mar 2014 14:47:03 +0100 Subject: [PATCH 1/2] automember rebuild nowait feature added automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. this patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished. Old usage can be enabled using --nowait, and returns the DN of the task for further polling. New tests added also. https://fedorahosted.org/freeipa/ticket/4239 --- API.txt| 7 ++- VERSION| 4 +- ipalib/errors.py | 16 ++ ipalib/plugins/automember.py | 70 +- ipatests/test_xmlrpc/test_automember_plugin.py | 67 ipatests/test_xmlrpc/xmlrpc_test.py| 10 6 files changed, 149 insertions(+), 25 deletions(-) diff --git a/API.txt b/API.txt index 14dde56832793f8dd9fa6795a5ba79d0a2431d51..a0285d49466b887bc9aeb4b4190cc0d99687cf6d 100644 --- a/API.txt +++ b/API.txt @@ -201,12 +201,15 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_rebuild -args: 0,4,3 +args: 0,7,3 +option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Str('hosts*') +option: Flag('no_wait?', autofill=True, default=False) +option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup')) option: Str('users*') option: Str('version?', exclude='webui') -output: Output('result', type 'bool', None) +output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_remove_condition diff --git a/VERSION b/VERSION index 7c6722965bc3b37b71e036ce7f2b2472fd662877..e787e371318b2a817a7d18c1bb1750db9130192e 100644 --- a/VERSION +++ b/VERSION @@ -89,5 +89,5 @@ IPA_DATA_VERSION=2010061412 # # IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=81 -# Last change: amisnyov - user plugin extend +IPA_API_VERSION_MINOR=82 +# Last change: amisnyov - automember nowait add diff --git a/ipalib/errors.py b/ipalib/errors.py index 311127f62e54017c85541d27276020a9f950ab0f..8ef35f590390eda1e847589d669cd4d28644a6a5 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -1530,6 +1530,22 @@ class DNSDataMismatch(ExecutionError): format = _('DNS check failed: Expected {%(expected)s} got {%(got)s}') +class TaskTimeout(DatabaseError): + +**4213** Raised when an LDAP task times out + +For example: + + raise TaskTimeout() +Traceback (most recent call last): + ... +TaskTimeout: Automember LDAP task timeout, Task DN: '' + + +errno = 4213
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Wed, 09 Apr 2014 14:53:34 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 04/09/2014 01:45 PM, Petr Viktorin wrote: On 04/09/2014 01:43 PM, Misnyovszki Adam wrote: On Tue, 08 Apr 2014 17:31:25 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 04/08/2014 04:17 PM, Misnyovszki Adam wrote: On Mon, 07 Apr 2014 09:43:10 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 03/27/2014 03:37 PM, Misnyovszki Adam wrote: On Wed, 26 Mar 2014 13:15:55 +0100 Petr Viktorin pvikt...@redhat.com wrote: [...] Looks great! I'm just concerned about the error returned when the task takes too long: $ ipa automember-rebuild --type group ipa: ERROR: LDAP timeout I don't think it's sufficiently clear from this that waiting for the task timed out, but the task was actually started successfully. A custom error with a more descriptive message would be useful. Also I've noticed that the nstaskstatus of a successful task is: Automember rebuild task finished. Processed (1) entries. This looks helpful; we could return it as the summary. Hi, both fixed. Greets Adam Sorry for the delay! 'Automember' is a translatable string, so please wrap it in _() when raising TaskTimeout. Also please update the tests. Otherwise with a little rebase it's good to go. Hi, see the attached modifications, tests corrected, and added for no-wait, also rebased for current master. Greets Adam Looks good overall, but why do you now set `self.msg_summary`? Keep in mind that currently the same Command object is reused for every automember_rebuild command, including commands that run in parallel in different threads. It should never be modified. Hi, corrected. Greets Adam ACK Pushed to master: 3f61bbaef582ff42b151f2bb01f312a94a70632c I spoke too soon. There is one more doctest failure. This patch should fix it, can you review? works for me, thanks! Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Mon, 07 Apr 2014 09:43:10 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 03/27/2014 03:37 PM, Misnyovszki Adam wrote: On Wed, 26 Mar 2014 13:15:55 +0100 Petr Viktorin pvikt...@redhat.com wrote: [...] Looks great! I'm just concerned about the error returned when the task takes too long: $ ipa automember-rebuild --type group ipa: ERROR: LDAP timeout I don't think it's sufficiently clear from this that waiting for the task timed out, but the task was actually started successfully. A custom error with a more descriptive message would be useful. Also I've noticed that the nstaskstatus of a successful task is: Automember rebuild task finished. Processed (1) entries. This looks helpful; we could return it as the summary. Hi, both fixed. Greets Adam Sorry for the delay! 'Automember' is a translatable string, so please wrap it in _() when raising TaskTimeout. Also please update the tests. Otherwise with a little rebase it's good to go. Hi, see the attached modifications, tests corrected, and added for no-wait, also rebased for current master. Greets AdamFrom 942a83926d3af0a314c5ad8a2f78ef02d5be553e Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Tue, 25 Mar 2014 14:47:03 +0100 Subject: [PATCH 1/2] automember rebuild nowait feature added automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. this patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished. Old usage can be enabled using --nowait, and returns the DN of the task for further polling. New tests added also. https://fedorahosted.org/freeipa/ticket/4239 --- API.txt| 7 ++- VERSION| 4 +- ipalib/errors.py | 16 ++ ipalib/plugins/automember.py | 70 +- ipatests/test_xmlrpc/test_automember_plugin.py | 67 ipatests/test_xmlrpc/xmlrpc_test.py| 10 6 files changed, 149 insertions(+), 25 deletions(-) diff --git a/API.txt b/API.txt index 14dde56832793f8dd9fa6795a5ba79d0a2431d51..a0285d49466b887bc9aeb4b4190cc0d99687cf6d 100644 --- a/API.txt +++ b/API.txt @@ -201,12 +201,15 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_rebuild -args: 0,4,3 +args: 0,7,3 +option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Str('hosts*') +option: Flag('no_wait?', autofill=True, default=False) +option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup')) option: Str('users*') option: Str('version?', exclude='webui') -output: Output('result', type 'bool', None) +output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_remove_condition diff --git a/VERSION b/VERSION index 7c6722965bc3b37b71e036ce7f2b2472fd662877..e787e371318b2a817a7d18c1bb1750db9130192e 100644 --- a/VERSION +++ b/VERSION @@ -89,5 +89,5 @@ IPA_DATA_VERSION=2010061412 # # IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=81 -# Last change: amisnyov - user plugin extend +IPA_API_VERSION_MINOR=82 +# Last change: amisnyov - automember nowait add diff --git a/ipalib/errors.py b/ipalib/errors.py index 311127f62e54017c85541d27276020a9f950ab0f..8ef35f590390eda1e847589d669cd4d28644a6a5 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -1530,6 +1530,22 @@ class DNSDataMismatch(ExecutionError): format = _('DNS check failed: Expected {%(expected)s} got {%(got)s}') +class TaskTimeout(DatabaseError): + +**4213** Raised when an LDAP task times out + +For example: + + raise TaskTimeout() +Traceback (most recent call last): + ... +TaskTimeout: Automember LDAP task timeout, Task DN: '' + + +errno = 4213 +format = _(%(task)s LDAP task timeout, Task DN: '%(task_dn)s') + + class CertificateError(ExecutionError): **4300** Base class for Certificate execution errors (*4300 - 4399*). diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py index a12bfb52522e38bc083d0750dc66c894a4aeba53..56bba18f0df48134e42714e89f691d7e4ee98da5 100644 --- a/ipalib/plugins/automember.py +++ b/ipalib/plugins/automember.py @@ -17,8
[Freeipa-devel] [PATCH][RFC] 9 CA-less tests generate failure
Hi, CA-less test suite always generate failures when installing revoked certificates. This is a known issue, described in https://fedorahosted.org/freeipa/ticket/4270 , this fix skips these tests, outputting a notification message for the ticket. Now it outputs this: [amisnyov@host freeipa]$ ./make-test ipatests/test_integration/test_caless.py:TestServerInstall.test_revoked_http /usr/bin/nosetests -v --with-doctest --doctest-tests --exclude=plugins ipatests/test_integration/test_caless.py:TestServerInstall.test_revoked_http IPA server install with revoked HTTP certificate ... SKIP: Known CA-less installation defect, see https://fedorahosted.org/freeipa/ticket/4270 -- Ran 1 test in 1020.253s OK (SKIP=1) == passed under '/usr/bin/python2.7' ** pass ** https://fedorahosted.org/freeipa/ticket/4271 There could be another possible solution, I could write a nose plugin to enable raising warnings instead of skipping a test. This could be achieved by adding a @unittest.expectedFailure for a specific test, so if it fails, it counts as an error/warning. There is a poc in a nose ticket located in http://code.google.com/p/python-nose/issues/detail?id=428 , not sure how much time it takes to implement it as a plugin, or is it even worth, because if this is implemented, we could also use this feature when eg. DNS is not configured, this is why RFC. Thanks AdamFrom e3ccd04b19675dfe1ecdcffdcf229d1f54d4d9e2 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 4 Apr 2014 10:41:51 +0200 Subject: [PATCH] CA-less tests generate failure CA-less test suite always generate failures when installing revoked certificates. This is a known issue, described in https://fedorahosted.org/freeipa/ticket/4270 , this fix skips these tests, outputting a warning for the later ticket. https://fedorahosted.org/freeipa/ticket/4271 --- ipatests/test_integration/test_caless.py | 37 1 file changed, 37 insertions(+) diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py index 87c523a43ed64dbf0f32fb8e0d594c8af60ce8fc..d20a8511c3741ff730e6fad13fe3d69c391cd31a 100644 --- a/ipatests/test_integration/test_caless.py +++ b/ipatests/test_integration/test_caless.py @@ -23,6 +23,7 @@ import shutil import base64 import glob import contextlib +import nose from ipalib import x509 from ipapython import ipautil @@ -557,6 +558,12 @@ class TestServerInstall(CALessBase): result = self.install_server(http_pkcs12='http.p12', dirsrv_pkcs12='dirsrv.p12') + +if result.returncode == 0: +raise nose.SkipTest( +Known CA-less installation defect, see ++ https://fedorahosted.org/freeipa/ticket/4270;) + assert result.returncode 0 def test_revoked_ds(self): @@ -569,6 +576,12 @@ class TestServerInstall(CALessBase): result = self.install_server(http_pkcs12='http.p12', dirsrv_pkcs12='dirsrv.p12') + +if result.returncode == 0: +raise nose.SkipTest( +Known CA-less installation defect, see ++ https://fedorahosted.org/freeipa/ticket/4270;) + assert result.returncode 0 def test_http_intermediate_ca(self): @@ -917,6 +930,12 @@ class TestReplicaInstall(CALessBase): result = self.prepare_replica(http_pkcs12='http.p12', dirsrv_pkcs12='dirsrv.p12') + +if result.returncode == 0: +raise nose.SkipTest( +Known CA-less installation defect, see ++ https://fedorahosted.org/freeipa/ticket/4270;) + assert result.returncode 0 def test_revoked_ds(self): @@ -927,6 +946,12 @@ class TestReplicaInstall(CALessBase): result = self.prepare_replica(http_pkcs12='http.p12', dirsrv_pkcs12='dirsrv.p12') + +if result.returncode == 0: +raise nose.SkipTest( +Known CA-less installation defect, see ++ https://fedorahosted.org/freeipa/ticket/4270;) + assert result.returncode 0 def test_http_intermediate_ca(self): @@ -1336,12 +1361,24 @@ class TestCertinstall(CALessBase): Install new revoked HTTP certificate result = self.certinstall('w', 'ca1/server-revoked') + +if result.returncode == 0: +raise nose.SkipTest( +Known CA-less installation defect, see ++ https://fedorahosted.org/freeipa/ticket/4270;) + assert result.returncode 0 def test_revoked_ds(self): Install new revoked DS certificate
Re: [Freeipa-devel] [PATCH] 565-568 webui: field and widget binding refactoring
On Thu, 27 Mar 2014 16:07:55 +0100 Petr Vobornik pvobo...@redhat.com wrote: The last refactoring I did while implementing RCUE login or more precisely support for standalone facets which have forms but are not details facets. [PATCH] webui: field and widget binding refactoring This is a Web UI wide change. Fields and Widgets binding was refactored to enable proper two-way binding between them. This should allow to have one source of truth (field) for multiple consumers - widgets or something else. One of the goal is to have fields and widget implementations independent on each other. So that one could use a widget without field or use one field for multiple widgets, etc.. Basically a fields logic was split into separate components: - adapters - parsers formatters - binder Adapters - extract data from data source (FreeIPA RPC command result) - prepares them for commands. Parsers - parse extracted data to format expected by field - parse widget value to format expected by field Formatters - format field value to format suitable for widgets - format field value to format suitable for adapter Binder - is a communication bridge between field and widget - listens to field's and widget's events and call appropriate methods Some side benefits: - better validation reporting in multivalued widget [PATCH] webui: replace widget's hidden property with visible Hidden was used only in ACI. There is no reason to have two properties which are negations of each other. [PATCH] webui: change widget updated event into value change event This change allow us to use proper two way binding between a field and a widget. In previous implementation field was not changed if something changed the value of a widget in 'update'. Now listeners are notified when the widget value is changed by: calling 'update', 'set_value' or by user change. [PATCH] webui-tests: binding test suite Add basic tests for two-way binding between a field and two widgets Integration tests and unit tests ran as expected, looking through the code, and manually testing it confirmed that, so ACK Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 562-563 webui: move RPC code from IPA module to its own module
On Wed, 19 Mar 2014 16:02:29 +0100 Petr Vobornik pvobo...@redhat.com wrote: depends on path #561(make navigation module independent on app module) [PATCH] 562 webui: move RPC code from IPA module to its own module - moves RPC code from ipa.js to it's own module - part of ongoing effort where the ultimate goal is to get rid of ipa.js and IPA namespace [PATCH] 563 webui: replace IPA.command usage with rpc.command Replace all IPA.command, IPA.batch_command and IPA.concurrent_command usages by equivalents from rpc module. ACK Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Wed, 26 Mar 2014 13:15:55 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/25/2014 03:36 PM, Misnyovszki Adam wrote: On Mon, 24 Mar 2014 17:06:41 +0100 Martin Kosek mko...@redhat.com wrote: On 03/24/2014 11:42 AM, Misnyovszki Adam wrote: On Fri, 21 Mar 2014 13:06:21 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/21/2014 12:58 PM, Martin Kosek wrote: On 03/21/2014 12:38 PM, Petr Viktorin wrote: On 03/21/2014 12:00 PM, Misnyovszki Adam wrote: On Fri, 21 Mar 2014 10:33:00 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/21/2014 10:29 AM, Petr Viktorin wrote: On 03/20/2014 04:22 PM, Misnyovszki Adam wrote: On Thu, 20 Mar 2014 14:19:51 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 14 Mar 2014 13:26:15 -0400 Rob Crittenden rcrit...@redhat.com wrote: Misnyovszki Adam wrote: Hi, automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. This patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished, according to http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 Request for comments: - Should I add a parameter to specify the polling time? (now 1ms) - Should I add a parameter to specify the maximum polling number? Now if something fails about creating the task, it polls forever. - Obviously, if these parameters should be added, there should be a reasonable default for them (~ Required=False, Default=X). I don't think you need a polling time, esp since this is hidden from the user, but I think that is probably too short and you may end up hammering the LDAP server. I also wonder if there should be some maximum wait time. I don't like loops that can never exit. I'm at a loss for what that time should be though. And we'd need to spell out that we gave up waiting, not that the task necessarily failed. So rather than having a polling time option, rename nowait into wait_for=20, so wait for 20 seconds. Or something like that. I'd suggest using get_entry since you already know the full DN and there is only ever one. It would make this much more readable. I wonder if some top-level documentation should be added to flesh this out some more. This does, for example, return False in one case. The meaning for that should be spelled out. rob Hi, personally I would keep --no-wait, with a delay of 1 seconds, and a maximum waiting time for 60 seconds. Questions is, do we need to parameterize with other parameters(wait-for to the maximum time, and/or poll-delay for the delay time, both not required), or it is not a case which worth to develop? Greets Adam Hi, here are the corrections Petr asked, also the other patch conatins the plugin registration refactor. Thanks! You forgot an alternate summary for the --no-wait case. (Make sure to output the DN in this case, so it's possible to poll for it.) Instead of `task['nstaskexitcode'][0]` please use `task.single_value['nstaskexitcode']`. There's one more `task['nstaskexitcode'][0]`. (Perhaps putting it in a variable would be better?) Here: raise errors.DatabaseError( desc=_(Automember rebuild membership task failed), info=_(nstaskexitcode = '%s' % str(task['nstaskexitcode'][0]))) there's no need to call str() on %'s argument. Also, use natural language (like Task exit code: %s), otherwise there's no need to mark the message for translation. And one more thing: Please bump the minor version in the VERSION file when API.txt changes. Hi, everything is corrected! Greets Adam Sorry for dragging this so long, but: The DN should not be a part of the summary, because that makes it hard to parse when using the API. It should be returned as a part of the result. To do that, you'd need to change the output type: has_output = output.standard_entry has_output_params = ( DNParam( 'dn', label=_('Task automember'), doc=_('DN of the started task'), ), ) and provide a dict in result, instead of True. (And with --no-wait, add the dn to that dict.) Do really want to use 'dn' for the DNParam? dn is already used for standard entry DN when --all is used, right? automembertaskdn may be better. Well, I think DN of the added entry is exactly what this is. Also, Task automember label sounds strange to me, would Automember task DN be better? I meant Task DN, sorry for the thinko. One more thing, which came to my mind after reviewing the code for myself once again
Re: [Freeipa-devel] [PATCH]Extending user plugin with employeenumber field
On Wed, 26 Mar 2014 14:07:50 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Tue, 25 Mar 2014 18:26:53 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Tue, 25 Mar 2014 14:31:15 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 21.3.2014 11:00, Misnyovszki Adam wrote: On Fri, 21 Mar 2014 10:13:55 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 21 Feb 2014 16:06:27 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 21.2.2014 15:45, Adam Misnyovszki wrote: Hi, According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with employeenumber field. https://fedorahosted.org/freeipa/ticket/4165 Question is, that should we extend user with other fields which are in the RFC, (carLicense, departmentNumber, employeeType, etc) if we already touched this code? Thanks Adam +Int('employeenumber?', +label=_('Employee ID'), +minvalue=1, +), Why Int and different label? IMO it should be Str and 'Employee Number' 2.4. Employee Number Numeric or alphanumeric identifier assigned to a person, typically based on order of hire or association with an organization. Single valued. ( 2.16.840.1.113730.3.1.3 NAME 'employeeNumber' DESC 'numerically identifies an employee within an organization' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE ) Hi, fixed, also some other fields added. Note, that according to the rfc, licence plate field should be multivalue, should I cange that(it is an existing field). yes Also, should I write test cases(especially for preferredlanguage)? Testing new functionality helps. Greets Adam self NACK, VERSION bump because API change It requires another rebase. Greets Adam 1) Is there a reason to have label 'Employee ID' instead of 'Employee Number' which is in RFC 2798? +label=_('Employee ID'), 2) Department number seems to be multivalued as well: ( 2.16.840.1.113730.3.1.2 NAME 'departmentNumber' DESC 'identifies a department within an organization' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 ) 3) The regex for preferredlanguage: +pattern='^[a-zA-Z]{1,8}[-[a-zA-Z]{1,8}]?$', doesn't match the expression in RFC 2068. It's only part of it. Accept-Language = Accept-Language : 1#( language-range [ ; q = qvalue ] ) language-range = ( ( 1*8ALPHA *( - 1*8ALPHA ) ) | * ) http://tools.ietf.org/html/rfc2068#section-14.4 RFC 2798 ( http://tools.ietf.org/html/rfc2798#section-2.7 ) says that you should omit only the `Accept-Language :` sequence. See the updates in the attached patch. Greets Adam The preferredLanguage regex pattern and error message has been modified to comply with RFC, according to conversation with Petr. Thanks Adam The regex pattern corrected, unit tests added for preferredlanguage. Greets AdamFrom 06c07fa11fe2fbdf92e0d51333a5265de38a0bdb Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Thu, 27 Mar 2014 16:26:08 +0100 Subject: [PATCH] Extending user plugin with inetOrgPerson fields According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with inetOrgPerson fields: - employeenumber - employeetype - preferredlanguage - departmentnumber carlicenseplate is now multivalued https://fedorahosted.org/freeipa/ticket/4165 --- API.txt | 24 ++--- VERSION | 4 +- install/ui/src/freeipa/user.js | 10 +++- ipalib/plugins/user.py | 17 +- ipatests/test_xmlrpc/test_user_plugin.py | 92 5 files changed, 136 insertions(+), 11 deletions(-) diff --git a/API.txt b/API.txt index 326b051e79cb914a2ff2ea603084d7d741f2aa70..14dde56832793f8dd9fa6795a5ba79d0a2431d51 100644 --- a/API.txt +++ b/API.txt @@ -3791,13 +3791,16 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: user_add -args: 1,39,3 +args: 1,43,3 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, required=True) option: Str('addattr*', cli_name='addattr', exclude='webui') option: Flag('all', autofill=True, cli_name='all', default=False
Re: [Freeipa-devel] [PATCH]Extending user plugin with employeenumber field
On Tue, 25 Mar 2014 18:26:53 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Tue, 25 Mar 2014 14:31:15 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 21.3.2014 11:00, Misnyovszki Adam wrote: On Fri, 21 Mar 2014 10:13:55 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 21 Feb 2014 16:06:27 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 21.2.2014 15:45, Adam Misnyovszki wrote: Hi, According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with employeenumber field. https://fedorahosted.org/freeipa/ticket/4165 Question is, that should we extend user with other fields which are in the RFC, (carLicense, departmentNumber, employeeType, etc) if we already touched this code? Thanks Adam +Int('employeenumber?', +label=_('Employee ID'), +minvalue=1, +), Why Int and different label? IMO it should be Str and 'Employee Number' 2.4. Employee Number Numeric or alphanumeric identifier assigned to a person, typically based on order of hire or association with an organization. Single valued. ( 2.16.840.1.113730.3.1.3 NAME 'employeeNumber' DESC 'numerically identifies an employee within an organization' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE ) Hi, fixed, also some other fields added. Note, that according to the rfc, licence plate field should be multivalue, should I cange that(it is an existing field). yes Also, should I write test cases(especially for preferredlanguage)? Testing new functionality helps. Greets Adam self NACK, VERSION bump because API change It requires another rebase. Greets Adam 1) Is there a reason to have label 'Employee ID' instead of 'Employee Number' which is in RFC 2798? +label=_('Employee ID'), 2) Department number seems to be multivalued as well: ( 2.16.840.1.113730.3.1.2 NAME 'departmentNumber' DESC 'identifies a department within an organization' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 ) 3) The regex for preferredlanguage: +pattern='^[a-zA-Z]{1,8}[-[a-zA-Z]{1,8}]?$', doesn't match the expression in RFC 2068. It's only part of it. Accept-Language = Accept-Language : 1#( language-range [ ; q = qvalue ] ) language-range = ( ( 1*8ALPHA *( - 1*8ALPHA ) ) | * ) http://tools.ietf.org/html/rfc2068#section-14.4 RFC 2798 ( http://tools.ietf.org/html/rfc2798#section-2.7 ) says that you should omit only the `Accept-Language :` sequence. See the updates in the attached patch. Greets Adam The preferredLanguage regex pattern and error message has been modified to comply with RFC, according to conversation with Petr. Thanks Adam From d61e5a04d158f714588f03dbf12eb3fc24db271a Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Wed, 26 Mar 2014 14:04:02 +0100 Subject: [PATCH] Extending user plugin with inetOrgPerson fields According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with inetOrgPerson fields: - employeenumber - employeetype - preferredlanguage - departmentnumber carlicenseplate is now multivalued https://fedorahosted.org/freeipa/ticket/4165 --- API.txt| 24 ++-- VERSION| 4 ++-- install/ui/src/freeipa/user.js | 10 -- ipalib/plugins/user.py | 17 - 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/API.txt b/API.txt index 326b051e79cb914a2ff2ea603084d7d741f2aa70..a6030d1826ffdd28223cacc1fa5512a01aac1b8e 100644 --- a/API.txt +++ b/API.txt @@ -3791,13 +3791,16 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: user_add -args: 1,39,3 +args: 1,43,3 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, required=True) option: Str('addattr*', cli_name='addattr', exclude='webui') option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') -option: Str('carlicense', attribute=True, cli_name='carlicense', multivalue=False, required=False) +option: Str('carlicense', attribute=True, cli_name='carlicense', multivalue=True, required=False) option: Str('cn', attribute=True, autofill=True, cli_name='cn', multivalue=False, required=True) +option: Str('departmentnumber', attribute=True, cli_name
Re: [Freeipa-devel] [PATCH] 561 webui: make navigation module independent on app module
On Wed, 19 Mar 2014 16:02:19 +0100 Petr Vobornik pvobo...@redhat.com wrote: When some module used 'freeipa/navigation' it pulled the entire Web UI because navigation depended on app. This patch splits the app into two modules: app and app_container. App specifies the entities which are part of final application. app_container module represents the application boot classes. Navigation now depends on app_container. Hi, tests ran as expected, and it works, so ACK. Note: there are two typos in copyright, app.js:5(./), app_container.js:1(space before comment), should be corrected before push. Thanks Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Mon, 24 Mar 2014 17:06:41 +0100 Martin Kosek mko...@redhat.com wrote: On 03/24/2014 11:42 AM, Misnyovszki Adam wrote: On Fri, 21 Mar 2014 13:06:21 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/21/2014 12:58 PM, Martin Kosek wrote: On 03/21/2014 12:38 PM, Petr Viktorin wrote: On 03/21/2014 12:00 PM, Misnyovszki Adam wrote: On Fri, 21 Mar 2014 10:33:00 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/21/2014 10:29 AM, Petr Viktorin wrote: On 03/20/2014 04:22 PM, Misnyovszki Adam wrote: On Thu, 20 Mar 2014 14:19:51 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 14 Mar 2014 13:26:15 -0400 Rob Crittenden rcrit...@redhat.com wrote: Misnyovszki Adam wrote: Hi, automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. This patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished, according to http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 Request for comments: - Should I add a parameter to specify the polling time? (now 1ms) - Should I add a parameter to specify the maximum polling number? Now if something fails about creating the task, it polls forever. - Obviously, if these parameters should be added, there should be a reasonable default for them (~ Required=False, Default=X). I don't think you need a polling time, esp since this is hidden from the user, but I think that is probably too short and you may end up hammering the LDAP server. I also wonder if there should be some maximum wait time. I don't like loops that can never exit. I'm at a loss for what that time should be though. And we'd need to spell out that we gave up waiting, not that the task necessarily failed. So rather than having a polling time option, rename nowait into wait_for=20, so wait for 20 seconds. Or something like that. I'd suggest using get_entry since you already know the full DN and there is only ever one. It would make this much more readable. I wonder if some top-level documentation should be added to flesh this out some more. This does, for example, return False in one case. The meaning for that should be spelled out. rob Hi, personally I would keep --no-wait, with a delay of 1 seconds, and a maximum waiting time for 60 seconds. Questions is, do we need to parameterize with other parameters(wait-for to the maximum time, and/or poll-delay for the delay time, both not required), or it is not a case which worth to develop? Greets Adam Hi, here are the corrections Petr asked, also the other patch conatins the plugin registration refactor. Thanks! You forgot an alternate summary for the --no-wait case. (Make sure to output the DN in this case, so it's possible to poll for it.) Instead of `task['nstaskexitcode'][0]` please use `task.single_value['nstaskexitcode']`. There's one more `task['nstaskexitcode'][0]`. (Perhaps putting it in a variable would be better?) Here: raise errors.DatabaseError( desc=_(Automember rebuild membership task failed), info=_(nstaskexitcode = '%s' % str(task['nstaskexitcode'][0]))) there's no need to call str() on %'s argument. Also, use natural language (like Task exit code: %s), otherwise there's no need to mark the message for translation. And one more thing: Please bump the minor version in the VERSION file when API.txt changes. Hi, everything is corrected! Greets Adam Sorry for dragging this so long, but: The DN should not be a part of the summary, because that makes it hard to parse when using the API. It should be returned as a part of the result. To do that, you'd need to change the output type: has_output = output.standard_entry has_output_params = ( DNParam( 'dn', label=_('Task automember'), doc=_('DN of the started task'), ), ) and provide a dict in result, instead of True. (And with --no-wait, add the dn to that dict.) Do really want to use 'dn' for the DNParam? dn is already used for standard entry DN when --all is used, right? automembertaskdn may be better. Well, I think DN of the added entry is exactly what this is. Also, Task automember label sounds strange to me, would Automember task DN be better? I meant Task DN, sorry for the thinko. One more thing, which came to my mind after reviewing the code for myself once again, if the task fails with an exit code other than 0, there is a DatabaseError raised, which is just fine. But in the info, should I
Re: [Freeipa-devel] [PATCH] 560 webui: rename domNode to dom_node
On Tue, 25 Mar 2014 12:49:24 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 20.3.2014 16:51, Misnyovszki Adam wrote: On Wed, 19 Mar 2014 16:02:12 +0100 Petr Vobornik pvobo...@redhat.com wrote: - unites domNode and dom_node usage to dom_node Nack, install/ui/test/details_tests.js:236 install/ui/test/details_tests.js:242 only finds element, because context(ie domNode) is undefined, so it falls back to html, not the best idea install/ui/src/freeipa/widgets/App.js:55 not sure if this causes errors, but it's worth renaming for consistency Greets, Adam all fixed, patch attached. ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Fri, 21 Mar 2014 13:06:21 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/21/2014 12:58 PM, Martin Kosek wrote: On 03/21/2014 12:38 PM, Petr Viktorin wrote: On 03/21/2014 12:00 PM, Misnyovszki Adam wrote: On Fri, 21 Mar 2014 10:33:00 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/21/2014 10:29 AM, Petr Viktorin wrote: On 03/20/2014 04:22 PM, Misnyovszki Adam wrote: On Thu, 20 Mar 2014 14:19:51 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 14 Mar 2014 13:26:15 -0400 Rob Crittenden rcrit...@redhat.com wrote: Misnyovszki Adam wrote: Hi, automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. This patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished, according to http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 Request for comments: - Should I add a parameter to specify the polling time? (now 1ms) - Should I add a parameter to specify the maximum polling number? Now if something fails about creating the task, it polls forever. - Obviously, if these parameters should be added, there should be a reasonable default for them (~ Required=False, Default=X). I don't think you need a polling time, esp since this is hidden from the user, but I think that is probably too short and you may end up hammering the LDAP server. I also wonder if there should be some maximum wait time. I don't like loops that can never exit. I'm at a loss for what that time should be though. And we'd need to spell out that we gave up waiting, not that the task necessarily failed. So rather than having a polling time option, rename nowait into wait_for=20, so wait for 20 seconds. Or something like that. I'd suggest using get_entry since you already know the full DN and there is only ever one. It would make this much more readable. I wonder if some top-level documentation should be added to flesh this out some more. This does, for example, return False in one case. The meaning for that should be spelled out. rob Hi, personally I would keep --no-wait, with a delay of 1 seconds, and a maximum waiting time for 60 seconds. Questions is, do we need to parameterize with other parameters(wait-for to the maximum time, and/or poll-delay for the delay time, both not required), or it is not a case which worth to develop? Greets Adam Hi, here are the corrections Petr asked, also the other patch conatins the plugin registration refactor. Thanks! You forgot an alternate summary for the --no-wait case. (Make sure to output the DN in this case, so it's possible to poll for it.) Instead of `task['nstaskexitcode'][0]` please use `task.single_value['nstaskexitcode']`. There's one more `task['nstaskexitcode'][0]`. (Perhaps putting it in a variable would be better?) Here: raise errors.DatabaseError( desc=_(Automember rebuild membership task failed), info=_(nstaskexitcode = '%s' % str(task['nstaskexitcode'][0]))) there's no need to call str() on %'s argument. Also, use natural language (like Task exit code: %s), otherwise there's no need to mark the message for translation. And one more thing: Please bump the minor version in the VERSION file when API.txt changes. Hi, everything is corrected! Greets Adam Sorry for dragging this so long, but: The DN should not be a part of the summary, because that makes it hard to parse when using the API. It should be returned as a part of the result. To do that, you'd need to change the output type: has_output = output.standard_entry has_output_params = ( DNParam( 'dn', label=_('Task automember'), doc=_('DN of the started task'), ), ) and provide a dict in result, instead of True. (And with --no-wait, add the dn to that dict.) Do really want to use 'dn' for the DNParam? dn is already used for standard entry DN when --all is used, right? automembertaskdn may be better. Well, I think DN of the added entry is exactly what this is. Also, Task automember label sounds strange to me, would Automember task DN be better? I meant Task DN, sorry for the thinko. One more thing, which came to my mind after reviewing the code for myself once again, if the task fails with an exit code other than 0, there is a DatabaseError raised, which is just fine. But in the info, should I specify not only the exit code, but also the DN of the task, or is it unnecessary? Thanks Adam
Re: [Freeipa-devel] [PATCH]Extending user plugin with employeenumber field
On Fri, 21 Feb 2014 16:06:27 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 21.2.2014 15:45, Adam Misnyovszki wrote: Hi, According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with employeenumber field. https://fedorahosted.org/freeipa/ticket/4165 Question is, that should we extend user with other fields which are in the RFC, (carLicense, departmentNumber, employeeType, etc) if we already touched this code? Thanks Adam +Int('employeenumber?', +label=_('Employee ID'), +minvalue=1, +), Why Int and different label? IMO it should be Str and 'Employee Number' 2.4. Employee Number Numeric or alphanumeric identifier assigned to a person, typically based on order of hire or association with an organization. Single valued. ( 2.16.840.1.113730.3.1.3 NAME 'employeeNumber' DESC 'numerically identifies an employee within an organization' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE ) Hi, fixed, also some other fields added. Note, that according to the rfc, licence plate field should be multivalue, should I cange that(it is an existing field). Also, should I write test cases(especially for preferredlanguage)? Greets AdamFrom 097fe5e9460806431bdd759a9e77538d5ed26d15 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 21 Mar 2014 09:59:01 +0100 Subject: [PATCH] Extending user plugin with inetOrgPerson fields According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with inetOrgPerson fields: - employeenumber - employeetype - preferredlanguage - departmentnumber https://fedorahosted.org/freeipa/ticket/4165 --- API.txt| 18 +++--- install/ui/src/freeipa/user.js | 6 +- ipalib/plugins/user.py | 15 +++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/API.txt b/API.txt index 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..16b72963302d39a79a4f961635750ff66412b690 100644 --- a/API.txt +++ b/API.txt @@ -3791,13 +3791,16 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: user_add -args: 1,39,3 +args: 1,43,3 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, required=True) option: Str('addattr*', cli_name='addattr', exclude='webui') option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Str('carlicense', attribute=True, cli_name='carlicense', multivalue=False, required=False) option: Str('cn', attribute=True, autofill=True, cli_name='cn', multivalue=False, required=True) +option: Str('departmentnumber', attribute=True, cli_name='departmentnumber', multivalue=False, required=False) option: Str('displayname', attribute=True, autofill=True, cli_name='displayname', multivalue=False, required=False) +option: Str('employeenumber', attribute=True, cli_name='employeenumber', multivalue=False, required=False) +option: Str('employeetype', attribute=True, cli_name='employeetype', multivalue=False, required=False) option: Str('facsimiletelephonenumber', attribute=True, cli_name='fax', multivalue=True, required=False) option: Str('gecos', attribute=True, autofill=True, cli_name='gecos', multivalue=False, required=False) option: Int('gidnumber', attribute=True, cli_name='gidnumber', minvalue=1, multivalue=False, required=False) @@ -3820,6 +3823,7 @@ option: Bool('nsaccountlock', attribute=True, cli_name='nsaccountlock', multival option: Str('ou', attribute=True, cli_name='orgunit', multivalue=False, required=False) option: Str('pager', attribute=True, cli_name='pager', multivalue=True, required=False) option: Str('postalcode', attribute=True, cli_name='postalcode', multivalue=False, required=False) +option: Str('preferredlanguage', attribute=True, cli_name='preferredlanguage', multivalue=False, pattern='^[a-zA-Z]{1,8}[-[a-zA-Z]{1,8}]?$', required=False) option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False) option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('setattr*', cli_name='setattr', exclude='webui') @@ -3858,12 +3862,15 @@ output: Output('result', type 'bool', None) output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: user_find -args: 1,49,4 +args: 1,53,4 arg: Str('criteria?', noextrawhitespace=False) option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Str('carlicense', attribute=True, autofill=False, cli_name='carlicense', multivalue=False,
Re: [Freeipa-devel] [PATCH] typo in migrate-ds
On Tue, 18 Mar 2014 19:31:31 -0600 Gabe Alford redhatri...@gmail.com wrote: All, It looks like the only typos exist in the uk and fr .po files for this ticket https://fedorahosted.org/freeipa/ticket/2546 Point me in the right direction if I am wrong. Thanks, Gabe ACK, thanks for the patch! Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH]Extending user plugin with employeenumber field
On Fri, 21 Mar 2014 10:13:55 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 21 Feb 2014 16:06:27 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 21.2.2014 15:45, Adam Misnyovszki wrote: Hi, According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with employeenumber field. https://fedorahosted.org/freeipa/ticket/4165 Question is, that should we extend user with other fields which are in the RFC, (carLicense, departmentNumber, employeeType, etc) if we already touched this code? Thanks Adam +Int('employeenumber?', +label=_('Employee ID'), +minvalue=1, +), Why Int and different label? IMO it should be Str and 'Employee Number' 2.4. Employee Number Numeric or alphanumeric identifier assigned to a person, typically based on order of hire or association with an organization. Single valued. ( 2.16.840.1.113730.3.1.3 NAME 'employeeNumber' DESC 'numerically identifies an employee within an organization' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE ) Hi, fixed, also some other fields added. Note, that according to the rfc, licence plate field should be multivalue, should I cange that(it is an existing field). Also, should I write test cases(especially for preferredlanguage)? Greets Adam self NACK, VERSION bump because API change Greets Adam From 5b657e13580635a0c7862d22de76841c4c9a13a2 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 21 Mar 2014 10:59:26 +0100 Subject: [PATCH] Extending user plugin with inetOrgPerson fields According to http://tools.ietf.org/html/rfc2798 ipa client and web ui extended with inetOrgPerson fields: - employeenumber - employeetype - preferredlanguage - departmentnumber https://fedorahosted.org/freeipa/ticket/4165 --- API.txt| 18 +++--- VERSION| 4 ++-- install/ui/src/freeipa/user.js | 6 +- ipalib/plugins/user.py | 15 +++ 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/API.txt b/API.txt index 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..16b72963302d39a79a4f961635750ff66412b690 100644 --- a/API.txt +++ b/API.txt @@ -3791,13 +3791,16 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: user_add -args: 1,39,3 +args: 1,43,3 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, required=True) option: Str('addattr*', cli_name='addattr', exclude='webui') option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Str('carlicense', attribute=True, cli_name='carlicense', multivalue=False, required=False) option: Str('cn', attribute=True, autofill=True, cli_name='cn', multivalue=False, required=True) +option: Str('departmentnumber', attribute=True, cli_name='departmentnumber', multivalue=False, required=False) option: Str('displayname', attribute=True, autofill=True, cli_name='displayname', multivalue=False, required=False) +option: Str('employeenumber', attribute=True, cli_name='employeenumber', multivalue=False, required=False) +option: Str('employeetype', attribute=True, cli_name='employeetype', multivalue=False, required=False) option: Str('facsimiletelephonenumber', attribute=True, cli_name='fax', multivalue=True, required=False) option: Str('gecos', attribute=True, autofill=True, cli_name='gecos', multivalue=False, required=False) option: Int('gidnumber', attribute=True, cli_name='gidnumber', minvalue=1, multivalue=False, required=False) @@ -3820,6 +3823,7 @@ option: Bool('nsaccountlock', attribute=True, cli_name='nsaccountlock', multival option: Str('ou', attribute=True, cli_name='orgunit', multivalue=False, required=False) option: Str('pager', attribute=True, cli_name='pager', multivalue=True, required=False) option: Str('postalcode', attribute=True, cli_name='postalcode', multivalue=False, required=False) +option: Str('preferredlanguage', attribute=True, cli_name='preferredlanguage', multivalue=False, pattern='^[a-zA-Z]{1,8}[-[a-zA-Z]{1,8}]?$', required=False) option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False) option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('setattr*', cli_name='setattr', exclude='webui') @@ -3858,12 +3862,15 @@ output: Output('result', type 'bool', None) output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: user_find -args: 1,49,4 +args: 1,53,4 arg: Str
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Fri, 21 Mar 2014 10:33:00 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/21/2014 10:29 AM, Petr Viktorin wrote: On 03/20/2014 04:22 PM, Misnyovszki Adam wrote: On Thu, 20 Mar 2014 14:19:51 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 14 Mar 2014 13:26:15 -0400 Rob Crittenden rcrit...@redhat.com wrote: Misnyovszki Adam wrote: Hi, automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. This patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished, according to http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 Request for comments: - Should I add a parameter to specify the polling time? (now 1ms) - Should I add a parameter to specify the maximum polling number? Now if something fails about creating the task, it polls forever. - Obviously, if these parameters should be added, there should be a reasonable default for them (~ Required=False, Default=X). I don't think you need a polling time, esp since this is hidden from the user, but I think that is probably too short and you may end up hammering the LDAP server. I also wonder if there should be some maximum wait time. I don't like loops that can never exit. I'm at a loss for what that time should be though. And we'd need to spell out that we gave up waiting, not that the task necessarily failed. So rather than having a polling time option, rename nowait into wait_for=20, so wait for 20 seconds. Or something like that. I'd suggest using get_entry since you already know the full DN and there is only ever one. It would make this much more readable. I wonder if some top-level documentation should be added to flesh this out some more. This does, for example, return False in one case. The meaning for that should be spelled out. rob Hi, personally I would keep --no-wait, with a delay of 1 seconds, and a maximum waiting time for 60 seconds. Questions is, do we need to parameterize with other parameters(wait-for to the maximum time, and/or poll-delay for the delay time, both not required), or it is not a case which worth to develop? Greets Adam Hi, here are the corrections Petr asked, also the other patch conatins the plugin registration refactor. Thanks! You forgot an alternate summary for the --no-wait case. (Make sure to output the DN in this case, so it's possible to poll for it.) Instead of `task['nstaskexitcode'][0]` please use `task.single_value['nstaskexitcode']`. Here: raise errors.DatabaseError( desc=_(Automember rebuild membership task failed), info=_(nstaskexitcode = '%s' % str(task['nstaskexitcode'][0]))) there's no need to call str() on %'s argument. Also, use natural language (like Task exit code: %s), otherwise there's no need to mark the message for translation. And one more thing: Please bump the minor version in the VERSION file when API.txt changes. Hi, everything is corrected! Greets Adam From 049a163583e18d63716a8419849b5f1880cb567f Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 21 Mar 2014 11:58:44 +0100 Subject: [PATCH] automember rebuild nowait feature added automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. this patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished. Old usage can be enabled using --nowait, and returns the DN of the task for further polling. https://fedorahosted.org/freeipa/ticket/4239 --- API.txt | 3 ++- VERSION | 4 ++-- ipalib/plugins/automember.py | 56 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/API.txt b/API.txt index 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..fa28a88f78270dd5858d97b88ee22ae7cdd8b13c 100644 --- a/API.txt +++ b/API.txt @@ -201,8 +201,9 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_rebuild -args: 0,4,3 +args: 0,5,3 option: Str('hosts*') +option: Flag('no_wait?', autofill=True, default=False) option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup')) option: Str('users*') option: Str('version?', exclude='webui') diff --git a/VERSION b/VERSION index 4f01e38c0c3a52ae6293e458fdb4d3e0a14aa8aa
Re: [Freeipa-devel] [PATCHES] 0499-0502 permission CLI: rename --permissions to --right
On Fri, 21 Mar 2014 11:14:43 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/20/2014 07:20 PM, Misnyovszki Adam wrote: On Tue, 18 Mar 2014 12:02:06 +0100 Petr Viktorin pvikt...@redhat.com wrote: Hello, This renames --permissions to --right. The old name is kept as a deprecated alias. FreeIPA didn't have a mechanism for doing this, so I added one. Also, while I was digging around in this part, I made the new IntEnum (and all future Enums) act like StrEnum in --help output. https://fedorahosted.org/freeipa/ticket/4231 499 ACK 500 ACK 501 ACK - although should it allow mixing deprecated and current aliases(eg --permission=read --right=write)? You're right, this is a strange edge case, but detecting this would need need a much more complicated approach than sharing the option's `dest`. I don't think it's worth it. - works fine with cli / webui also - help displays nicely 502 - tested with more than one deprecated alias - API.txt validation doesn't match, although it has the same output: Got StrEnum('ipapermright', attribute=True, cli_name='right', deprecated_cli_aliases=set(['testalias', 'permissions']), multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all')) Expected StrEnum('ipapermright', attribute=True, cli_name='right', deprecated_cli_aliases=set(['testalias','permissions']), multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all')) API.txt: option: StrEnum('ipapermright', attribute=True, cli_name='right', deprecated_cli_aliases=set(['testalias','permissions']), multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all')) ipalib/plugins/permission.py: StrEnum( 'ipapermright*', cli_name='right', deprecated_cli_aliases={'permissions','testalias'}, label=_('Granted rights'), doc=_('Rights to grant ' '(read, search, compare, write, add, delete, all)'), values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'), ), don't know if it is a problem anyways - other tests(cli, webui) works fine for me - unit tests related to this ran as expected so besides the multiple deprecated_cli_aliases issue, it's an ACK It looks like you've edited API.txt by hand and forgot a space after the comma in ['testalias','permissions']. Does it work if you use makeapi to regenerate API.txt? You are right, my mistake, with ./makeapi, it works, even when the CLI got this for parameters: --right=read --permission=search --testparam=write ACK Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Fri, 14 Mar 2014 13:26:15 -0400 Rob Crittenden rcrit...@redhat.com wrote: Misnyovszki Adam wrote: Hi, automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. This patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished, according to http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 Request for comments: - Should I add a parameter to specify the polling time? (now 1ms) - Should I add a parameter to specify the maximum polling number? Now if something fails about creating the task, it polls forever. - Obviously, if these parameters should be added, there should be a reasonable default for them (~ Required=False, Default=X). I don't think you need a polling time, esp since this is hidden from the user, but I think that is probably too short and you may end up hammering the LDAP server. I also wonder if there should be some maximum wait time. I don't like loops that can never exit. I'm at a loss for what that time should be though. And we'd need to spell out that we gave up waiting, not that the task necessarily failed. So rather than having a polling time option, rename nowait into wait_for=20, so wait for 20 seconds. Or something like that. I'd suggest using get_entry since you already know the full DN and there is only ever one. It would make this much more readable. I wonder if some top-level documentation should be added to flesh this out some more. This does, for example, return False in one case. The meaning for that should be spelled out. rob Hi, personally I would keep --no-wait, with a delay of 1 seconds, and a maximum waiting time for 60 seconds. Questions is, do we need to parameterize with other parameters(wait-for to the maximum time, and/or poll-delay for the delay time, both not required), or it is not a case which worth to develop? Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Thu, 20 Mar 2014 14:19:51 +0100 Misnyovszki Adam amisn...@redhat.com wrote: On Fri, 14 Mar 2014 13:26:15 -0400 Rob Crittenden rcrit...@redhat.com wrote: Misnyovszki Adam wrote: Hi, automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. This patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished, according to http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 Request for comments: - Should I add a parameter to specify the polling time? (now 1ms) - Should I add a parameter to specify the maximum polling number? Now if something fails about creating the task, it polls forever. - Obviously, if these parameters should be added, there should be a reasonable default for them (~ Required=False, Default=X). I don't think you need a polling time, esp since this is hidden from the user, but I think that is probably too short and you may end up hammering the LDAP server. I also wonder if there should be some maximum wait time. I don't like loops that can never exit. I'm at a loss for what that time should be though. And we'd need to spell out that we gave up waiting, not that the task necessarily failed. So rather than having a polling time option, rename nowait into wait_for=20, so wait for 20 seconds. Or something like that. I'd suggest using get_entry since you already know the full DN and there is only ever one. It would make this much more readable. I wonder if some top-level documentation should be added to flesh this out some more. This does, for example, return False in one case. The meaning for that should be spelled out. rob Hi, personally I would keep --no-wait, with a delay of 1 seconds, and a maximum waiting time for 60 seconds. Questions is, do we need to parameterize with other parameters(wait-for to the maximum time, and/or poll-delay for the delay time, both not required), or it is not a case which worth to develop? Greets Adam Hi, here are the corrections Petr asked, also the other patch conatins the plugin registration refactor. Thanks Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel From 32c4fdb505b02a582afbff65366382216982359d Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Thu, 20 Mar 2014 15:50:29 +0100 Subject: [PATCH] automember rebuild nowait feature added automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. this patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 --- API.txt | 3 ++- ipalib/plugins/automember.py | 52 +++- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/API.txt b/API.txt index 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..52959922241d0df11556c2890bf56d7b8107ed62 100644 --- a/API.txt +++ b/API.txt @@ -201,10 +201,11 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_rebuild -args: 0,4,3 +args: 0,5,3 option: Str('hosts*') option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup')) option: Str('users*') +option: Flag('no_wait?', autofill=True, default=False) option: Str('version?', exclude='webui') output: Output('result', type 'bool', None) output: Output('summary', (type 'unicode', type 'NoneType'), None) diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py index a12bfb52522e38bc083d0750dc66c894a4aeba53..feb2cd7637fcdbf31bf5dea86be1667ffd51876b 100644 --- a/ipalib/plugins/automember.py +++ b/ipalib/plugins/automember.py @@ -17,7 +17,10 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/. import uuid +import time + import ldap as _ldap + from ipalib import api, errors, Str, StrEnum, _, ngettext from ipalib.plugins.baseldap import * from ipalib.request import context @@ -623,9 +626,14 @@ class automember_rebuild(Command): label=_('Hosts'), doc=_('Rebuild membership for specified hosts'), ), +Flag( +'no_wait?', +default=False
Re: [Freeipa-devel] [PATCH] 560 webui: rename domNode to dom_node
On Wed, 19 Mar 2014 16:02:12 +0100 Petr Vobornik pvobo...@redhat.com wrote: - unites domNode and dom_node usage to dom_node Nack, install/ui/test/details_tests.js:236 install/ui/test/details_tests.js:242 only finds element, because context(ie domNode) is undefined, so it falls back to html, not the best idea install/ui/src/freeipa/widgets/App.js:55 not sure if this causes errors, but it's worth renaming for consistency Greets, Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0499-0502 permission CLI: rename --permissions to --right
On Tue, 18 Mar 2014 12:02:06 +0100 Petr Viktorin pvikt...@redhat.com wrote: Hello, This renames --permissions to --right. The old name is kept as a deprecated alias. FreeIPA didn't have a mechanism for doing this, so I added one. Also, while I was digging around in this part, I made the new IntEnum (and all future Enums) act like StrEnum in --help output. https://fedorahosted.org/freeipa/ticket/4231 499 ACK 500 ACK 501 ACK - although should it allow mixing deprecated and current aliases(eg --permission=read --right=write)? - works fine with cli / webui also - help displays nicely 502 - tested with more than one deprecated alias - API.txt validation doesn't match, although it has the same output: Got StrEnum('ipapermright', attribute=True, cli_name='right', deprecated_cli_aliases=set(['testalias', 'permissions']), multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all')) Expected StrEnum('ipapermright', attribute=True, cli_name='right', deprecated_cli_aliases=set(['testalias','permissions']), multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all')) API.txt: option: StrEnum('ipapermright', attribute=True, cli_name='right', deprecated_cli_aliases=set(['testalias','permissions']), multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all')) ipalib/plugins/permission.py: StrEnum( 'ipapermright*', cli_name='right', deprecated_cli_aliases={'permissions','testalias'}, label=_('Granted rights'), doc=_('Rights to grant ' '(read, search, compare, write, add, delete, all)'), values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'), ), don't know if it is a problem anyways - other tests(cli, webui) works fine for me - unit tests related to this ran as expected so besides the multiple deprecated_cli_aliases issue, it's an ACK Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 550 webui-css: improve radio, checkbox keyboard support and color
On Fri, 14 Mar 2014 18:39:14 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 13.3.2014 16:55, Petr Vobornik wrote: On 7.3.2014 15:34, Petr Vobornik wrote: checkboxes and radio buttons: - do not change color on hover when disabled - are focusable and checkable by keyboard again. This uses a little trick where the real checkbox is hidden under the artificial checkbox. That way it has the same position and therefore it works even in containers with overflow set. https://fedorahosted.org/freeipa/ticket/4217 Self-NACK. Breaks Automount Key deletion in ipa/ui/#/e/automountmap/keys/ Fixed in attached patch #551. Also attaching new version of #550 with CI fixes. works now, ACK Greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 552-557 Permissions v2 Web UI
On Wed, 19 Mar 2014 10:52:10 +0100 Petr Viktorin pvikt...@redhat.com wrote: On 03/18/2014 04:56 PM, Petr Vobornik wrote: On 18.3.2014 15:07, Petr Viktorin wrote: On 03/18/2014 01:09 PM, Petr Vobornik wrote: New revision for patch patch #557 attached. On 17.3.2014 15:22, Petr Viktorin wrote: On 03/14/2014 06:47 PM, Petr Vobornik wrote: Main ACI UI changes are in patch #557. The rest are prerequisites. With this UI it is impossible to change from Type-based permissions to General ones. This seems to be remaining from the old model where permissions were type/filter/subtree/targetgroup were classes of a permission rather than co-existing as attributes. Rather the Target section should IMO look the same for all (non-managed) permissions, with the first items being: Type:[drop-down with a None option] Subtree: [textbox that is disabled when a Type is selected] The Subtree should be a one-line textbox. It would be acceptable if the whole DN doesn't always fit, it's the first part that's important. Remember to only send Subtree if Type is (staying as | being set to) None. Also, the Add dialog should use this instead of the Define by. Done With managed permissions, if I try to change both included/excluded attribute list and the effective attributes, I get a validation error, which is good in CLI but it doesn't work well for the UI. I think it would be better to move Managed permission overrides below Target, and make it read-only. And perhaps rename it to something like Attribute breakdown. Managing the included/excluded lists directly is only useful for upgrades with a heavily customized policy, and for upgrades you need the CLI anyway. Normally, having only the attribute list editable should be fine. Done For SYSTEM permissions (those which only have the SYSTEM flag), such as 'Add Automember Rebuild Membership Task', Permissions should not be editable. For old-style permissions (those without any flags), nothing is editable but everything should be. The attributelevelrights are missing because the entry doesn't have the ipaPermissionV2 objectclass yet (although it's being reported, which is my bug -- #4257). Fields were set to be editable if attributes level rights are missing. That solves things for normal legacy permissions, but not for the SYSTEM ones - those should be completely read-only. Also, changing the Permisisons checkboxes on these permissions doesn't mark them dirty. Otherwise the patches work great! Fixed New versions of 556 and 557 attached. Great, thanks! ACK for the functionality, I can't really judge the javascript though. ACK for the code and the test, besides these two issues(don't know if it has to be corrected): 555: - typo in commit message(~delimeter) 557: - install/ui/test/aci_tests.js tab error at first row Greets: Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
Hi, automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. This patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished, according to http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 Request for comments: - Should I add a parameter to specify the polling time? (now 1ms) - Should I add a parameter to specify the maximum polling number? Now if something fails about creating the task, it polls forever. - Obviously, if these parameters should be added, there should be a reasonable default for them (~ Required=False, Default=X). Thanks, AdamFrom 62215a10a826d9e529ac861b40c1f1bf68823472 Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Fri, 14 Mar 2014 17:22:09 +0100 Subject: [PATCH] automember rebuild nowait feature added automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. this patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished. Old usage can be enabled using --nowait. https://fedorahosted.org/freeipa/ticket/4239 --- ipalib/plugins/automember.py | 25 + 1 file changed, 25 insertions(+) diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py index a12bfb52522e38bc083d0750dc66c894a4aeba53..1f36b36b63bf94345f48e18867dbdd3316d6ecb0 100644 --- a/ipalib/plugins/automember.py +++ b/ipalib/plugins/automember.py @@ -17,6 +17,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/. import uuid +import time import ldap as _ldap from ipalib import api, errors, Str, StrEnum, _, ngettext from ipalib.plugins.baseldap import * @@ -623,6 +624,13 @@ class automember_rebuild(Command): label=_('Hosts'), doc=_('Rebuild membership for specified hosts'), ), +Flag( +'nowait', +required=False, +default=False, +label=_('No wait'), +doc=_('Don\'t wait for rebuilding membership'), +), ) has_output = output.standard_value msg_summary = _('Automember rebuild membership task completed') @@ -707,6 +715,23 @@ class automember_rebuild(Command): scope=['sub'] ) ldap.add_entry(entry) + +while options.get('nowait'): +tasks = ldap.get_entries( +DN( +('cn', cn), +('cn', 'automember rebuild membership'), +('cn', 'tasks'), +('cn', 'config'), +), +) + +if len(tasks) 0: +task = tasks[0] +if 'nstaskexitcode' in task.single_value: +return dict(result=task.single_value['nstaskexitcode'] == '0', value=unicode(task.single_value['nstaskexitcode'])) +time.sleep(0.001) + return dict(result=True, value=u'') api.register(automember_rebuild) -- 1.8.5.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 546 webui: Datetime parsing and formatting
On Thu, 06 Mar 2014 13:26:03 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 6.3.2014 13:01, Misnyovszki Adam wrote: On Tue, 25 Feb 2014 18:05:28 +0100 Petr Vobornik pvobo...@redhat.com wrote: prerequisite for patch 547, 548 depends on tbabej's datetime patch this patch implements: - output_formatter in field. It should be used in par with formatter. Formatter serves for datasource-widget conversion, output_formatter for widget-datasource format conversion. - datetime module which parses/format strings in subset of ISO 8601 and LDAP generalized time format to Date. - utc formatter replaced with new datetime formatter - datetime_validator introduced - new datetime field, extension of text field, which by default uses datetime formatter and validator Dojo was regenerated to include dojo/string module https://fedorahosted.org/freeipa/ticket/4194 Hi, these are the results of my review: - if incorrect number specified for any of the parts(ie 2013-01-01 25:00:00), then it counts forward(result: 2013-01-02 01:00:00), does it supposed to work this way? at least some warning should be given to the user, that the date is incorrect(imho) It's standard behavior of JavaScript Date object's setUTCFullYear method. I did not find better methods which would not require pulling third-party lib or do real evaluation of the dates. In the end it's not that bad. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/setUTCFullYear - couldn't test non utc datetime input(no test cases in the ui yet), but other tests(integration and ui) passed which are connected to this issue Non UTC are not supported therefore it's disabled. But there are unit tests in test/utils_tests.js - validity fields accept non existing timeframe(ie start: 2013-01-01 00:00:00Z, end: 2012-01-01 00:00:00Z) I don't think it's checked even on a server. Maybe it should be. - validity fields only accept UTC time, it's good so besides that timeframe issue(which the api should handle i think), it's an ACK Be careful, it should be pushed to master with pvoborni's 531-541 and 546-548, wait for the review of those! Greets: Adam ACK, can push to master. Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 548 webui: change ipatokennotbefore and ipatokennotafter types to datetime
On Tue, 25 Feb 2014 18:10:13 +0100 Petr Vobornik pvobo...@redhat.com wrote: Depends on tbabej's patches # 137, 140 and pvoborni's 546 and 531-541. https://fedorahosted.org/freeipa/ticket/3369 ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 531-541 OTP UI
On Wed, 12 Mar 2014 15:41:31 +0100 Petr Vobornik pvobo...@redhat.com wrote: On 7.3.2014 18:10, Petr Vobornik wrote: Attaching new version of 537 which adds combobox control for owner attribute instead of textbox. All other patches are attached as well to reduce confusion in case of ACK :). The entire patchset was rebased. Review log: - add otp token: OK - delete otp token: OK - bulk delete: OK - edit otp token: OK - single disable token: OK - single enable token: OK - bulk enable token: OK - bulk disable token: OK - configuration url working: OK - after bulk operation, checkboxes for items unchecked, which is good, but the bulk check checkbox remains checked, propose: either all checkboxes should remain checked, or all of them unchecked - https://fedorahosted.org/freeipa/ticket/4245 - validity fields accept non existing timeframe(ie start: 2013-01-01 00:00:00Z, end: 2012-01-01 00:00:00Z) - https://fedorahosted.org/freeipa/ticket/4244 - after edit, screen doesn't go back to list, any other action errors with a [UUID]: OTP token not found because of uniqueid change - https://fedorahosted.org/freeipa/ticket/4227 all the opened tickets are not the scope of this patch, so ACK greets Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 549 webui: use unique ids for checkboxes
On Tue, 25 Feb 2014 18:12:20 +0100 Petr Vobornik pvobo...@redhat.com wrote: This is a minor fix. Please don't close ticket 3904 yet if committed. Checkboxes have not used unique ids across the whole UI. It broke checking by clicking on label for later displayed instances. It became serious problem when rcue introduced new checkbox styles with 'label clicking' as default check method. https://fedorahosted.org/freeipa/ticket/3904 ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 546 webui: Datetime parsing and formatting
On Tue, 25 Feb 2014 18:05:28 +0100 Petr Vobornik pvobo...@redhat.com wrote: prerequisite for patch 547, 548 depends on tbabej's datetime patch this patch implements: - output_formatter in field. It should be used in par with formatter. Formatter serves for datasource-widget conversion, output_formatter for widget-datasource format conversion. - datetime module which parses/format strings in subset of ISO 8601 and LDAP generalized time format to Date. - utc formatter replaced with new datetime formatter - datetime_validator introduced - new datetime field, extension of text field, which by default uses datetime formatter and validator Dojo was regenerated to include dojo/string module https://fedorahosted.org/freeipa/ticket/4194 Hi, these are the results of my review: - if incorrect number specified for any of the parts(ie 2013-01-01 25:00:00), then it counts forward(result: 2013-01-02 01:00:00), does it supposed to work this way? at least some warning should be given to the user, that the date is incorrect(imho) - couldn't test non utc datetime input(no test cases in the ui yet), but other tests(integration and ui) passed which are connected to this issue - validity fields accept non existing timeframe(ie start: 2013-01-01 00:00:00Z, end: 2012-01-01 00:00:00Z) - validity fields only accept UTC time, it's good so besides that timeframe issue(which the api should handle i think), it's an ACK Be careful, it should be pushed to master with pvoborni's 531-541 and 546-548, wait for the review of those! Greets: Adam ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel