[Freeipa-devel] [PATCH] 297 Fixed enroll labels.
Labels using the word enroll (except for host enrollment) have been modified to use more relevant words. The IPA.add_dialog has been renamed into IPA.entity_adder_dialog for clarity. Ticket #1642 -- Endi S. Dewata From ae1a5d9d7c1ed811848453c080b316aa3710975c Mon Sep 17 00:00:00 2001 From: Endi S. Dewata edew...@redhat.com Date: Mon, 24 Oct 2011 19:20:14 -0500 Subject: [PATCH] Fixed enroll labels. Labels using the word enroll (except for host enrollment) have been modified to use more relevant words. The IPA.add_dialog has been renamed into IPA.entity_adder_dialog for clarity. Ticket #1642 --- install/ui/add.js |2 +- install/ui/association.js |6 +++--- install/ui/automount.js|2 +- install/ui/dialog.js |2 +- install/ui/dns.js |2 +- install/ui/entity.js |2 +- install/ui/group.js|2 +- install/ui/host.js |2 +- install/ui/service.js |6 +++--- install/ui/test/data/ipa_init.json |9 - ipalib/plugins/internal.py |9 - 11 files changed, 21 insertions(+), 23 deletions(-) diff --git a/install/ui/add.js b/install/ui/add.js index fd99b02c5eb77acc99484995a6fb0c65598128b7..640604d08692077c2c5cfd67db4858506204f2a5 100644 --- a/install/ui/add.js +++ b/install/ui/add.js @@ -23,7 +23,7 @@ /* REQUIRES: ipa.js */ -IPA.add_dialog = function (spec) { +IPA.entity_adder_dialog = function(spec) { spec = spec || {}; diff --git a/install/ui/association.js b/install/ui/association.js index ebb6e421ff3b8538116471de240b1f972e08e6bf..d3b66132d5043b0dfe60b8847896e9f27f676059 100644 --- a/install/ui/association.js +++ b/install/ui/association.js @@ -889,7 +889,7 @@ IPA.association_facet = function (spec) { that.add_button = IPA.action_button({ name: 'add', -label: IPA.messages.buttons.enroll, +label: IPA.messages.buttons.add, icon: 'add-icon', click: function() { if (!that.add_button.hasClass('action-button-disabled')) { @@ -923,7 +923,7 @@ IPA.association_facet = function (spec) { }).appendTo(span); $('label/', { -text: IPA.messages.association.direct_enrollment, +text: IPA.messages.association.direct_membership, 'for': direct_id }).appendTo(span); @@ -944,7 +944,7 @@ IPA.association_facet = function (spec) { }).appendTo(span); $('label/', { -text: IPA.messages.association.indirect_enrollment, +text: IPA.messages.association.indirect_membership, 'for': indirect_id }).appendTo(span); } diff --git a/install/ui/automount.js b/install/ui/automount.js index 6b740d8e6d4abc1d0e402f5a1a3b79e8a4a77420..c17e55fe9408b3787a8f3bdb22cb73d7f8c1f498 100644 --- a/install/ui/automount.js +++ b/install/ui/automount.js @@ -221,7 +221,7 @@ IPA.automount_key_column = function(spec) { IPA.automountmap_adder_dialog = function(spec) { -var that = IPA.add_dialog(spec); +var that = IPA.entity_adder_dialog(spec); that.create = function() { that.add_dialog_create(); diff --git a/install/ui/dialog.js b/install/ui/dialog.js index b55cce715d9ba16b9062378603b4210301012872..41b35fb42120a015fcdeb56c10e80cf638456bd8 100644 --- a/install/ui/dialog.js +++ b/install/ui/dialog.js @@ -492,7 +492,7 @@ IPA.adder_dialog = function(spec) { var add_button = that.create_button({ name: 'add', -label: IPA.messages.buttons.enroll, +label: IPA.messages.buttons.add, click: function() { if (!add_button.is_enabled()) return; that.execute(); diff --git a/install/ui/dns.js b/install/ui/dns.js index 2b98f0dfec4c8f70770123930ea06081ff47633a..a1c4bbf68c294aae2563845b65ac952ff7e32d33 100644 --- a/install/ui/dns.js +++ b/install/ui/dns.js @@ -383,7 +383,7 @@ IPA.dnszone_adder_dialog = function(spec) { spec = spec || {}; -var that = IPA.add_dialog(spec); +var that = IPA.entity_adder_dialog(spec); that.create = function() { that.add_dialog_create(); diff --git a/install/ui/entity.js b/install/ui/entity.js index c82f4a8df727c1a3dd75b133d2b005867e0015ab..704b5c43b2ce7ef9dcc7221f5a73e4bf5df4bf15 100644 --- a/install/ui/entity.js +++ b/install/ui/entity.js @@ -914,7 +914,7 @@ IPA.entity_builder = function(){ }; that.adder_dialog = function(spec) { -spec.factory = spec.factory || IPA.add_dialog; +spec.factory = spec.factory || IPA.entity_adder_dialog; spec.name = spec.name || 'add'; if (!spec.title) { diff --git a/install/ui/group.js b/install/ui/group.js index a63a7800a0b668152188bfccfd372e8548e484fd..8c18c14a252f10c763bfed332b14d0248ecb1c25 100644 --- a/install/ui/group.js
Re: [Freeipa-devel] [PATCH] 028 Code cleanup of HBAC, Sudo rules
Comments in text. New patch not supplied yet - some topics may require further discussion. Most of the comments should be part of the 'Nesting widgets' thread. On 10/24/2011 06:29 PM, Endi Sukma Dewata wrote: On 10/24/2011 3:29 AM, Petr Vobornik wrote: https://fedorahosted.org/freeipa/ticket/1515 Some issues: 1. The IPA.hbacrule_details_facet uses IPA.sudo.enable_widget(). This makes HBAC unnecessarily dependent on sudo. The enable_widget should be moved to widget.js or rule.js which is shared between HBAC and sudo. * enable_widget moved to widget.js * rule_association_table_widget and rule_association_adder_dialog moved to rule.js 2. The set_facet() is added to widget. I don't think we want to make widget dependent on facet. The facet so far is only used by IPA.sudo.enable_widget. In IPA.sudo.options_section the facet object is passed as a parameter in spec. Are you saying that dependency on facet in ALL widgets is bad and only in a few is good, or in any is bad? I think it doesn't matter if all is dependant when one is dependant. Anyway currently all association table widgets are dependant (association.js:386). Btw similar topic could be: How to get current entity's pkey?'. Dependancy on facet.get_primary_key() or IPA.nav.get_state(that.entity.name+'-pkey') seems wrong too. 3. When updating sudo rule, in the original code the rule is enabled/disabled after all the modifications are done. In the new code it's reversed. I'm not sure the importance of the execution order for this particular situation, but suppose it is, is there a way to maintain the original order? I was thinking about a command or widget priority. The 'mod' command would have some default priority or it would get it from facet. The commands would be sorted by priority before adding them to batch. This way we can set exact order in facet update. The order which is implemented now is reflecting the order in HBAC details facet - there were errors when 'mod' was executed before clearing associations. 4. The IPA.header_widget is not really a widget, it's actually part of layout. In the current implementation a widget always corresponds to an attribute, so it will be loaded, saved, etc. Since there is no attribute name matching the header name currently this is not a problem, but it can pollute the namespace. This is part of widget nesting topic. In this manner composite_widget isn't a proper widget too (it can correspond to several or none attribute). Purely html rendering widgets can be separated from attribute widgets, but it would be nice to have them because they can provide easier form composing. Without them it would be required to override the create method (in facet or composite_widget/section) which is sometimes better but often it creates only code duplication with little added logic. 5. In details facet update(), instead of storing the callbacks in update_custom_on_win and update_custom_on_fail and requiring the subclasses to call them, it might be better to call them from the update() itself: var command = IPA.command({ ... on_success: function(data, text_status, xhr) { that.on_update_success(data, text_status, xhr) if (on_win) on_win.call(this, data, text_status, xhr); }, on_error: function(xhr, text_status, error_thrown) { that.on_update_error(xhr, text_status, error_thrown); if (on_fail) on_fail.call( this, xhr, text_status, error_thrown); } }); There are two types of success/error handlers: 1) the default ones in details facet 2) the ones supplied to update(win, fail) method. In my implementation I have separated 1) from update method so if default behaviour isn't suitable these methods can be overridden without overriding whole update method. Overriding isn't required. This is IMHO good (less code duplication). 6. Can IPA.batch_details_facet be combined into IPA.details_facet? I think the base details facet should support both regular mod and batch. The only point where they overlap is the update method. It's possible to add a logic to switch between command creations - maybe based on attribute (could be defined in spec). But should we do that? Isn't this the reason for inheriting the class? 7. In sudo details page, the As Whom category is missing the RunAs User category radio buttons. Fixed 8. The IPA.sudo.rule_details_runas_widget uses composite widget instead of section. They are right now functionally identical, but I suppose the composite widget is an abstract class and the section will later contain code specific to section. Part of widget nesting topic. This was briefly discussed with Adam (in that topic). The important question is what is a section? I understand section as top level container (in facet) which contains widgets (even composites) and semantically separates parts of the facet (with headers and section folding...). From this point of view, composite_widget isn't an abstract class, but it isn't used used anywhere else
Re: [Freeipa-devel] Keytab for talking to PKI CA from IPA
Adam Young wrote: When setting up replication, it should not be necessary to cache any passwords, anywhere, until the replication agreemsnts are set up, and then, all caching should be using known secure mechanisms. The two main repositories we care about are the Directory Server instances managed by IPA and the Certificate Authority. Currently, these are not in the same Dir Srv isntance (although they could be) due to the fact that we expect to replicate them seperately. Specifically, we expect to have more IPA instances than CA instances, and we will not have a CA instance without a co-located IPA instance. Not really. I created two instances to provide logical separation and prevent us from having to deal with multiple naming contexts. There was some serious investigation over the summer to see if we could consolidate into a single instance. It was determined to be too much too late in the cycle. During normal operations, the IPA instance should not need to talk to the directory server instance of the CA. All communication between IPA and the CA should happen via the HTTPS secured via Certificates issued by the CA. Right, this has never been in question AFAIK. Once the replication process starts, the file generated by ipa-prepare replicate should not need an passwords. Instead, when the replicated server is installed, the user performing the install should get a ticket as an administrative user. All authentication for the replication should be based on that ticket. The very first step is to install an new Directory server for IPA. For this, the replication process can generate a single use password and use it as the Directory server password for the local instance. Next, the ticket for the adminiustrative user should be used to download a keytab for the Directory Server to use when communicating back to the original IPA directory server.With these keytab in place, the replicated IPA DS should be able to talk to the original IPA DS and establish the replication agreement. At this point, the single use password should be disabled. I think you mean the first step is to create a 389-ds instance for the CA, right? Why not simply pre-create the keytab and ship it in the prepared file? There is no need to disable the random DM password. Once the installation is complete it will be unknown so effectively disabled. Once the IPA Directory server has been replicated, we can either use the original keytab or download a second keytab to establish a replication agreement between the replicated CA directory server and the original CA directory server. The process would look the same as setting up the keytab for the IPA directory server. Why don't we do this now? Because of permission issues writing the relevant entries to cn=config. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s]| command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com -- Regards, Ondrej Hamada FreeIPA team jabber: oh...@jabbim.cz IRC: ohamada diff --git a/ipalib/plugable.py b/ipalib/plugable.py index b0e4156..2aed1cd 100644 --- a/ipalib/plugable.py +++ b/ipalib/plugable.py @@ -173,6 +173,7 @@ class Plugin(ReadOnly): label = None +__try_finalize = False def __init__(self): self.__api = None @@ -208,6 +209,16 @@ class Plugin(ReadOnly): ) ) +def __getattribute__(self, name): +if name.startswith('_Plugin__') or name.startswith('_ReadOnly__'): +return object.__getattribute__(self, name) +if self.__try_finalize: +self.__try_finalize = False +self.finalize() +if not is_production_mode(self.__api): +assert islocked(self) is True +return object.__getattribute__(self, name) + def __get_api(self): Return `API` instance passed to `finalize()`. @@ -217,6 +228,9 @@ class Plugin(ReadOnly): return self.__api api = property(__get_api) +def prefinalize(self): +self.__try_finalize = True + def finalize(self): @@ -638,9 +652,7 @@ class API(DictProxy): assert p.instance.api is self for p in plugins.itervalues(): -p.instance.finalize() -if not production_mode: -assert islocked(p.instance) is True +p.instance.prefinalize() object.__setattr__(self, '_API__finalized', True) tuple(PluginInfo(p) for p in plugins.itervalues()) object.__setattr__(self, 'plugins', ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s]| command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 155 Fix ipa-managed-entries bind procedure
Make sure that when Directory Manager password is entered, we directly do a simple bind instead of trying binding via GSSAPI. Also capture ldap.INVALID_CREDENTIALS exception and provide nice error message than crash. https://fedorahosted.org/freeipa/ticket/1927 From 332f96ea1e4c77d429adaad858a459138c0bfb9d Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Tue, 25 Oct 2011 15:34:45 +0200 Subject: [PATCH] Fix ipa-managed-entries bind procedure Make sure that when Directory Manager password is entered, we directly do a simple bind instead of trying binding via GSSAPI. Also capture ldap.INVALID_CREDENTIALS exception and provide nice error message than crash. https://fedorahosted.org/freeipa/ticket/1927 --- install/tools/ipa-managed-entries | 20 +--- 1 files changed, 13 insertions(+), 7 deletions(-) diff --git a/install/tools/ipa-managed-entries b/install/tools/ipa-managed-entries index 16f0a956cd2b1398dc3385d3f2254cb56cf23c09..649ef80017d7db57ab1efac859c5eb12450168db 100755 --- a/install/tools/ipa-managed-entries +++ b/install/tools/ipa-managed-entries @@ -106,15 +106,21 @@ def main(): try: filter = '(objectClass=extensibleObject)' conn = ipaldap.IPAdmin(host, 636, cacert=CACERT) -conn.do_sasl_gssapi_bind() -except ldap.LOCAL_ERROR: + if options.dirman_password: -dirman_password = options.dirman_password +conn.do_simple_bind(bindpw=options.dirman_password) else: -dirman_password = get_dirman_password() -if dirman_password is None: -sys.exit(\nDirectory Manager password required) -conn.do_simple_bind(bindpw=dirman_password) +conn.do_sasl_gssapi_bind() +except ldap.LOCAL_ERROR: +dirman_password = get_dirman_password() +if dirman_password is None: +sys.exit(\nDirectory Manager password required) +try: +conn.do_simple_bind(bindpw=dirman_password) +except ldap.INVALID_CREDENTIALS: +sys.exit(Invalid credentials) +except ldap.INVALID_CREDENTIALS: +sys.exit(Invalid credentials) except errors.ExecutionError, lde: sys.exit(An error occurred while connecting to the server.\n%s\n % str(lde)) -- 1.7.6.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On 10/25/2011 04:01 PM, Martin Kosek wrote: On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s]| command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin Sorry, correct version attached -- Regards, Ondrej Hamada FreeIPA team jabber: oh...@jabbim.cz IRC: ohamada From 798d8f8a624f8350974e54c328c1c58c06944b26 Mon Sep 17 00:00:00 2001 From: Ondrej Hamada oham...@redhat.com Date: Tue, 25 Oct 2011 16:20:44 +0200 Subject: [PATCH] lazy initialization patch --- ipalib/plugable.py | 18 +++--- 1 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ipalib/plugable.py b/ipalib/plugable.py index b0e415656e0428eb164c35a2862fcfbf50883381..2aed1cdcda18840728558ed53435ab10ae28e802 100644 --- a/ipalib/plugable.py +++ b/ipalib/plugable.py @@ -173,6 +173,7 @@ class Plugin(ReadOnly): label = None +__try_finalize = False def __init__(self): self.__api = None @@ -208,6 +209,16 @@ class Plugin(ReadOnly): ) ) +def __getattribute__(self, name): +if name.startswith('_Plugin__') or name.startswith('_ReadOnly__'): +return object.__getattribute__(self, name) +if self.__try_finalize: +self.__try_finalize = False +self.finalize() +if not is_production_mode(self.__api): +assert islocked(self) is True +return object.__getattribute__(self, name) + def __get_api(self): Return `API` instance passed to `finalize()`. @@ -217,6 +228,9 @@ class Plugin(ReadOnly): return self.__api api = property(__get_api) +def prefinalize(self): +self.__try_finalize = True + def finalize(self): @@ -638,9 +652,7 @@ class API(DictProxy): assert p.instance.api is self for p in plugins.itervalues(): -p.instance.finalize() -if not production_mode: -assert islocked(p.instance) is True +p.instance.prefinalize() object.__setattr__(self, '_API__finalized', True) tuple(PluginInfo(p) for p in plugins.itervalues()) object.__setattr__(self, 'plugins', -- 1.7.6.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 295 Fixed inconsistent required/optional attributes.
On 10/25/2011 9:20 AM, Petr Vobornik wrote: ACK Pushed to master. Also proposing minor visual enhancement (see attached patch). ACK and pushed to master. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0030 Quote worker option
https://fedorahosted.org/freeipa/ticket/2023 -- / Alexander Bokovoy From 29eb102e9319eff837d71e4da6ad45796f3e7868 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy aboko...@redhat.com Date: Tue, 25 Oct 2011 18:41:32 +0300 Subject: [PATCH] Quote multiple workers option https://fedorahosted.org/freeipa/ticket/2023 --- ipaserver/install/krbinstance.py |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index b4f0a5446be0dfb1621e256b65ac34906df7351e..ce70c231dfb7e7b6b59c0496721cced0d09f1604 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -365,7 +365,7 @@ class KrbInstance(service.Service): replacevars = {'KRB5REALM':self.realm} appendvars = {} if workers and cpus 1: -appendvars = {'KRB5KDC_ARGS': -w %s % str(cpus)} +appendvars = {'KRB5KDC_ARGS': '-w %s' % str(cpus)} ipautil.backup_config_and_replace_variables(self.fstore, /etc/sysconfig/krb5kdc, replacevars=replacevars, appendvars=appendvars) -- 1.7.7 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0030 Quote worker option
On Tue, 2011-10-25 at 18:44 +0300, Alexander Bokovoy wrote: https://fedorahosted.org/freeipa/ticket/2023 ACK. Pushed to master, ipa-2-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 028 Code cleanup of HBAC, Sudo rules
On 10/25/2011 7:49 AM, Petr Vobornik wrote: 2. The set_facet() is added to widget. I don't think we want to make widget dependent on facet. The facet so far is only used by IPA.sudo.enable_widget. In IPA.sudo.options_section the facet object is passed as a parameter in spec. Are you saying that dependency on facet in ALL widgets is bad and only in a few is good, or in any is bad? In general less dependency is better. More dependency will limit its usage. This is inline with the goal to make purely HTML widget. But if a widget is meant to be used only with a facet then it's certainly ok to make it depends on facet. However, the rest should not become unnecessarily dependent on facet too. I think it doesn't matter if all is dependant when one is dependant. Anyway currently all association table widgets are dependant (association.js:386). Let's use this as an example. The association table explicitly checks if the facet is dirty and asks the user whether to update/reset/cancel. Suppose we want to use this inside a dialog, it will still check whether the current facet is dirty instead of the dialog itself, which may not be what we want. Btw similar topic could be: How to get current entity's pkey?'. Dependancy on facet.get_primary_key() or IPA.nav.get_state(that.entity.name+'-pkey') seems wrong too. Yes. Ideally we should have a path (REST-style URL) instead of the current parameter-based URL. We should get the pkey from that path. 3. When updating sudo rule, in the original code the rule is enabled/disabled after all the modifications are done. In the new code it's reversed. I'm not sure the importance of the execution order for this particular situation, but suppose it is, is there a way to maintain the original order? I was thinking about a command or widget priority. The 'mod' command would have some default priority or it would get it from facet. The commands would be sorted by priority before adding them to batch. This way we can set exact order in facet update. It's possible, but managing priority could be problematic if they are spread out, because we have to know all existing priorities before we can add a new one. The original code uses an ordered list of commands. The order which is implemented now is reflecting the order in HBAC details facet - there were errors when 'mod' was executed before clearing associations. Right, so for certain things the order is important. 4. The IPA.header_widget is not really a widget, it's actually part of layout. In the current implementation a widget always corresponds to an attribute, so it will be loaded, saved, etc. Since there is no attribute name matching the header name currently this is not a problem, but it can pollute the namespace. This is part of widget nesting topic. In this manner composite_widget isn't a proper widget too (it can correspond to several or none attribute). Purely html rendering widgets can be separated from attribute widgets, but it would be nice to have them because they can provide easier form composing. Without them it would be required to override the create method (in facet or composite_widget/section) which is sometimes better but often it creates only code duplication with little added logic. One other solution is to split widgets into non-visual fields and purely HTML components. Then in the facet we use separate lists for the fields and HTML components. The fields will have a reference to the corresponding HTML components. There can be more HTML components than fields. But this will require a significant restructuring. 5. In details facet update(), instead of storing the callbacks in update_custom_on_win and update_custom_on_fail and requiring the subclasses to call them, it might be better to call them from the update() itself: var command = IPA.command({ ... on_success: function(data, text_status, xhr) { that.on_update_success(data, text_status, xhr) if (on_win) on_win.call(this, data, text_status, xhr); }, on_error: function(xhr, text_status, error_thrown) { that.on_update_error(xhr, text_status, error_thrown); if (on_fail) on_fail.call( this, xhr, text_status, error_thrown); } }); There are two types of success/error handlers: 1) the default ones in details facet 2) the ones supplied to update(win, fail) method. In my implementation I have separated 1) from update method so if default behaviour isn't suitable these methods can be overridden without overriding whole update method. Overriding isn't required. This is IMHO good (less code duplication). There is still code duplication, the subclass that overrides the on_update_success/error (#1) will still have to call update_custom_on_win/error (#2) manually. In the changes that I proposed you don't have to do that because #2 are called inside update(). 6. Can IPA.batch_details_facet be combined into IPA.details_facet? I think the base details facet should support both regular mod and batch. The only point
Re: [Freeipa-devel] [PATCH] 136 Fix ipa-managed-entries password option long form
Martin Kosek wrote: https://fedorahosted.org/freeipa/ticket/1913 ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 151 Add --zonemgr validator
Martin Kosek wrote: On Mon, 2011-10-24 at 17:08 +0200, Martin Kosek wrote: On Mon, 2011-10-24 at 09:02 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Fri, 2011-10-21 at 11:31 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Fri, 2011-10-14 at 14:11 -0400, Rob Crittenden wrote: Martin Kosek wrote: Do at least a basic validation of DNS zone manager mail address. Do not require '@' to be in the mail address as it is not used in common DNS zone configuration (in bind for example) and people may be used to configure it that way. '@' is always removed by the installer before the DNS zone is created. https://fedorahosted.org/freeipa/ticket/1966 There is already a zonemgr_callback defined for this option, can the verify_zonemgr call be either integrated or called from that? rob Right. Please, try this one. I also added a parser error when more than one '@' is in the checked value. Martin A couple of things: In the block where you are counting @ why not add an : else: raise ValueError('address is not fully qualified') rather than looking for '.' in the result? I think it will be clearer that way. I wonder if the error should contain an example as well, are people going to know what a fully-qualified means? The regex is very strict for e-mail addresses, perhaps too much so. It doesn't allow upper-case characters, periods or _, both of which are allowed in login names. A common e-mail format is first.last@domain. rob I reorganized the validator a little and let people enter _ in the domain name. I also added a small explanation of what we mean by fully-qualified. Since we have the zonemgr validator available, why not use it for the DNS plugin too? I enhanced the plugin to use this validator too. Please, see attached patch. Martin NACK, a client might not have the server sub-package installed so the import of bindinstance will fail. I think that moving the validator into dns.py as a central location should work though. Otherwise looks good. rob Thanks. I didn't realize that user can have freeipa-admintools without freeipa-server installed. I placed the validator to ipalib/util.py as we cannot place it to the dns plugin directly as all our plugins assume that we have initialized api. Updated patch attached. Martin And now also with API.txt change. This patch must be cursed or something. No need to bump API version, just the validator has been added. Martin ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 298 Fixed host Enrolled column.
The Enrolled column in the host search page has been added back to show the host enrollment status based on has_keytab attribute. Ticket #2020 -- Endi S. Dewata From 05c85f889e1f58716fd6368451bdc8c9a22afb33 Mon Sep 17 00:00:00 2001 From: Endi S. Dewata edew...@redhat.com Date: Tue, 25 Oct 2011 14:25:31 -0500 Subject: [PATCH] Fixed host Enrolled column. The Enrolled column in the host search page has been added back to show the host enrollment status based on has_keytab attribute. Ticket #2020 --- install/ui/host.js |6 +++- install/ui/test/data/host_find.json | 59 -- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/install/ui/host.js b/install/ui/host.js index b50287291568c95830b275cdfa8dc3332677e2c2..4c0ce6ed0e461a38a565c1450cd483098b0c2dc7 100644 --- a/install/ui/host.js +++ b/install/ui/host.js @@ -31,7 +31,11 @@ IPA.entity_factories.host = function () { search_facet({ columns: [ 'fqdn', -'description' +'description', +{ +name: 'has_keytab', +label: IPA.messages.objects.host.enrolled +} ] }). details_facet({ diff --git a/install/ui/test/data/host_find.json b/install/ui/test/data/host_find.json index 48b1fcb893f8f94f50a369dcd16775297a449e64..34ca71a2ee48e9b947cafed67b4cbdc9b7460d63 100644 --- a/install/ui/test/data/host_find.json +++ b/install/ui/test/data/host_find.json @@ -1,6 +1,6 @@ { error: null, -id: 0, +id: null, result: { count: 2, result: [ @@ -8,46 +8,44 @@ cn: [ dev.example.com ], -dn: fqdn=dev.example.com,cn=computers,cn=accounts,dc=dev,dc=example,dc=com, +dn: fqdn=dev.example.com,cn=computers,cn=accounts,dc=example,dc=com, +enrolledby_user: [ +admin +], fqdn: [ dev.example.com ], +has_keytab: true, +has_password: false, ipauniqueid: [ -fc6a6d5a-f388-11df-9c01-00163e72f2d9 +d2c1f41c-ff41-11e0-bde8-525400e135d8 ], krbextradata: [ { -__base64__: AAL+5+VMYWRtaW4vYWRtaW5AREVWLkVYQU1QTEUuQ09NAA== -}, -{ -__base64__: AAgBAA== +__base64__: AAIIEqdOaG9zdC9kZXYuZXhhbXBsZS5jb21ASURNLkxBQi5CT1MuUkVESEFULkNPTQA= } ], krblastpwdchange: [ -20101119025910Z -], -krbpasswordexpiration: [ -1970010100Z +20111025194616Z ], krbprincipalname: [ -host/dev.example@dev.example.com +host/dev.example@example.com ], -krbticketflags: [ -0 +managedby_host: [ +dev.example.com ], -managedby: [ -fqdn=dev.example.com,cn=computers,cn=accounts,dc=dev,dc=example,dc=com +managing_host: [ +dev.example.com ], objectclass: [ -top, ipaobject, nshost, ipahost, -ipaservice, pkiuser, +ipaservice, krbprincipalaux, krbprincipal, -krbticketpolicyaux +top ], serverhostname: [ dev @@ -57,18 +55,31 @@ cn: [ test.example.com ], -dn: fqdn=test.example.com,cn=computers,cn=accounts,dc=dev,dc=example,dc=com, +dn: fqdn=test.example.com,cn=computers,cn=accounts,dc=example,dc=com, +enrolledby_user: [ +admin +], fqdn: [ test.example.com ], +has_keytab: false, +has_password: false, ipauniqueid: [ -ac28dca0-f3b5-11df-879f-00163e72f2d9 +d5d1400e-ff41-11e0-9a21-525400e135d8 +], +krbextradata: [ +{ +__base64__: AAIiEqdOaG9zdC90ZXN0LmV4YW1wbGUuY29tQElETS5MQUIuQk9TLlJFREhBVC5DT00A +} ], krbprincipalname: [ -host/test.example@dev.example.com +
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Ondrej Hamada wrote: On 10/25/2011 04:01 PM, Martin Kosek wrote: On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s] | command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin Sorry, correct version attached Wow, this is very impressive, great job Ondra and Honza! Martin, ACK from me but I'd like a second opinion. The patch is very straightforward and clean, just want to make sure we're not missing a corner case. I ran the self-tests and didn't see any problem there. Before pushing please add the ticket # to the commit. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel