Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code
On Mon, 2015-03-16 at 13:30 +0100, Martin Babinsky wrote:
> On 03/16/2015 12:15 PM, Martin Kosek wrote:
> > On 03/13/2015 05:37 PM, Martin Babinsky wrote:
> >> Attaching the next iteration of patches.
> >>
> >> I have tried my best to reword the ipa-client-install man page bit about
> >> the
> >> new option. Any suggestions to further improve it are welcome.
> >>
> >> I have also slightly modified the 'kinit_keytab' function so that in
> >> Kerberos
> >> errors are reported for each attempt and the text of the last error is
> >> retained
> >> when finally raising exception.
> >
> > The approach looks very good. I think that my only concern with this patch
> > is
> > this part:
> >
> > +ccache.init_creds_keytab(keytab=ktab, principal=princ)
> > ...
> > +except krbV.Krb5Error as e:
> > +last_exc = str(e)
> > +root_logger.debug("Attempt %d/%d: failed: %s"
> > + % (attempt, attempts, last_exc))
> > +time.sleep(1)
> > +
> > +root_logger.debug("Maximum number of attempts (%d) reached"
> > + % attempts)
> > +raise StandardError("Error initializing principal %s: %s"
> > +% (principal, last_exc))
> >
> > The problem here is that this function will raise the super-generic
> > StandardError instead of the proper with all the context and information
> > about
> > the error that the caller can then process.
> >
> > I think that
> >
> > except krbV.Krb5Error as e:
> > if attempt == max_attempts:
> > log something
> > raise
> >
> > would be better.
> >
>
> Yes that seems reasonable. I'm just thinking whether we should re-raise
> Krb5Error or raise ipalib.errors.KerberosError? the latter options makes
> more sense to me as we would not have to additionally import Krb5Error
> everywhere and it would also make the resulting errors more consistent.
>
> I am thinking about someting like this:
>
> except krbV.Krb5Error as e:
> if attempt == attempts:
> # log that we have reaches maximum number of attempts
> raise KerberosError(minor=str(e))
>
> What do you think?
Are you retrying on any error ?
Please do *not* do that, if you retry many times on an error that
indicates the password is wrong you may end up locking an administrative
account. If you want to retry you should do it only for very specific
timeout errors.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH 0021] fix improper handling of boolean option during KRA install
https://fedorahosted.org/freeipa/ticket/4530
--
Martin^3 Babinsky
From 7a674d795b94b4179aae5e17a522fd19ff343b4f Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Tue, 17 Mar 2015 13:43:46 +0100
Subject: [PATCH] fix improper handling of boolean option in
read_replica_info_kra_enabled
This patch fixes https://fedorahosted.org/freeipa/ticket/4530.
---
ipaserver/install/installutils.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 1a4c170f40982573eb910fcab9a01a4d20e91d7a..787a1207abfbd378074719a88734338ef79485c8 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -572,7 +572,7 @@ def read_replica_info_kra_enabled(config_dir):
config = SafeConfigParser()
config.readfp(fd)
-enable_kra = bool(config.get("global", "enable_kra"))
+enable_kra = config.getboolean("global", "enable_kra")
return enable_kra
--
2.1.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code
On 03/17/2015 12:09 PM, Petr Spacek wrote:
On 16.3.2015 17:20, Martin Babinsky wrote:
On 03/16/2015 01:35 PM, Jan Cholasta wrote:
Dne 16.3.2015 v 13:30 Martin Babinsky napsal(a):
On 03/16/2015 12:15 PM, Martin Kosek wrote:
On 03/13/2015 05:37 PM, Martin Babinsky wrote:
Attaching the next iteration of patches.
Very good! I hopefully have last two nitpicks :-) See below.
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index
4116d974e620341119b56fad3cff1bda48af3bab..cd03e9fd17b60b8b7324d0ccd436a10f7556baf0
100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1175,27 +1175,61 @@ def wait_for_open_socket(socket_name, timeout=0):
else:
raise e
-def kinit_hostprincipal(keytab, ccachedir, principal):
+
+def kinit_keytab(keytab, ccache_path, principal, attempts=1):
"""
-Given a ccache directory and a principal kinit as that user.
+Given a ccache_path , keytab file and a principal kinit as that user.
+
+The optional parameter 'attempts' specifies how many times the credential
+initialization should be attempted before giving up and raising
+StandardError.
This blindly overwrites the current CCNAME so if you need to save
it do so before calling this function.
+This function is also not thread-safe since it modifies environment
+variables.
+
Thus far this is used to kinit as the local host.
This note can be deleted because it is used elsewhere too.
"""
-try:
-ccache_file = 'FILE:%s/ccache' % ccachedir
-krbcontext = krbV.default_context()
-ktab = krbV.Keytab(name=keytab, context=krbcontext)
-princ = krbV.Principal(name=principal, context=krbcontext)
-os.environ['KRB5CCNAME'] = ccache_file
-ccache = krbV.CCache(name=ccache_file, context=krbcontext,
primary_principal=princ)
-ccache.init(princ)
-ccache.init_creds_keytab(keytab=ktab, principal=princ)
-return ccache_file
-except krbV.Krb5Error, e:
-raise StandardError('Error initializing principal %s in %s: %s' %
(principal, keytab, str(e)))
+root_logger.debug("Initializing principal %s using keytab %s"
+ % (principal, keytab))
I'm sorry for nitpicking but it would be nice to log ccache_file too. Krb5
libs return quite weird errors when CC cache is not accessible so it helps to
have the path at hand.
Attaching updated patches.
--
Martin^3 Babinsky
From 5888e5924c8b6aac30a8a893b9d0045545773ceb Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 16 Mar 2015 16:28:54 +0100
Subject: [PATCH 1/3] ipautil: new functions kinit_keytab and kinit_password
kinit_keytab replaces kinit_hostprincipal and performs Kerberos auth using
keytab file. Function is also able to repeat authentication multiple times
before giving up and raising Krb5Error.
kinit_password wraps kinit auth using password and also supports FAST
authentication using httpd armor ccache.
---
ipapython/ipautil.py | 62 +++-
1 file changed, 47 insertions(+), 15 deletions(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 6a06a8e956552597dfd48128b60a1dd6a4cc92f6..212f39d7e77eaa1c9655e06b2758e173e0bd9a42 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1185,27 +1185,59 @@ def wait_for_open_socket(socket_name, timeout=0):
else:
raise e
-def kinit_hostprincipal(keytab, ccachedir, principal):
+
+def kinit_keytab(keytab, ccache_path, principal, attempts=1):
"""
-Given a ccache directory and a principal kinit as that user.
+Given a ccache_path , keytab file and a principal kinit as that user.
+
+The optional parameter 'attempts' specifies how many times the credential
+initialization should be attempted before giving up and raising Krb5Error.
This blindly overwrites the current CCNAME so if you need to save
it do so before calling this function.
-Thus far this is used to kinit as the local host.
+This function is also not thread-safe since it modifies environment
+variables.
"""
-try:
-ccache_file = 'FILE:%s/ccache' % ccachedir
-krbcontext = krbV.default_context()
-ktab = krbV.Keytab(name=keytab, context=krbcontext)
-princ = krbV.Principal(name=principal, context=krbcontext)
-os.environ['KRB5CCNAME'] = ccache_file
-ccache = krbV.CCache(name=ccache_file, context=krbcontext, primary_principal=princ)
-ccache.init(princ)
-ccache.init_creds_keytab(keytab=ktab, principal=princ)
-return ccache_file
-except krbV.Krb5Error, e:
-raise StandardError('Error initializing principal %s in %s: %s' % (principal, keytab, str(e)))
+root_logger.debug("Initializing principal %s using keytab %s"
+ % (principal, keytab))
+root_logger.debug("using ccache path %s" % ccache_path)
+for attempt in range(1, attempts + 1):
Re: [Freeipa-devel] [PATCH 0208] Remove --test option from upgrade
On 12/03/15 16:10, David Kupka wrote:
On 03/06/2015 06:00 PM, Martin Basti wrote:
Upgrade plugins which modify LDAP data directly should not be executed
in --test mode.
This patch is a workaround, to ensure update with --test option will not
modify any LDAP data.
https://fedorahosted.org/freeipa/ticket/3448
Patch attached.
Ideally we want to fix all plugins to dry-run the upgrade not just
skip when there is '--test' option but it is a good first step.
Works for me, ACK.
We had long discussion, and we decided to remove this option from upgrade.
Reasons:
* users are not supposed to use this option to test if upgrade will be
successful, it can not guarantee it.
* option is not used for developing, as it can not catch all issues with
upgrade, using snapshots is better
Attached patch removes the option.
--
Martin Basti
From 0ec9ebfbc55aa2e749d8f9155f41d5913e0832dd Mon Sep 17 00:00:00 2001
From: Martin Basti
Date: Tue, 17 Mar 2015 12:23:06 +0100
Subject: [PATCH] Server Upgrade: remove --test option
As --test option is not used for developing, and it is not recommended
to test if upgrade will pass, this path removes it copmletely.
https://fedorahosted.org/freeipa/ticket/3448
---
install/tools/man/ipa-ldap-updater.1 | 5 ---
ipaserver/install/ipa_ldap_updater.py | 21 ++--
ipaserver/install/ldapupdate.py | 53 +--
ipaserver/install/plugins/updateclient.py | 21 ++--
ipaserver/install/schemaupdate.py | 10 ++
ipaserver/install/upgradeinstance.py | 8 ++---
ipatests/test_install/test_updates.py | 2 +-
7 files changed, 47 insertions(+), 73 deletions(-)
diff --git a/install/tools/man/ipa-ldap-updater.1 b/install/tools/man/ipa-ldap-updater.1
index 79cc316501512879fa39ba4c15fd898b976eb25e..5ab77e047d523975caeb13943d9a14069f1c3a6d 100644
--- a/install/tools/man/ipa-ldap-updater.1
+++ b/install/tools/man/ipa-ldap-updater.1
@@ -81,9 +81,6 @@ Schema files should be in LDIF format, and may only specify attributeTypes and o
\fB\-d\fR, \fB\-\-debug\fR
Enable debug logging when more verbose output is needed
.TP
-\fB\-t\fR, \fB\-\-test\fR
-Run through the update without changing anything. If changes are available then the command returns 2. If no updates are available it returns 0.
-.TP
\fB\-y\fR
File containing the Directory Manager password
.TP
@@ -108,5 +105,3 @@ Specify a schema file. May be used multiple times. Implies \-\-schema.
0 if the command was successful
1 if an error occurred
-
-2 if run with in test mode (\-t) and updates are available
diff --git a/ipaserver/install/ipa_ldap_updater.py b/ipaserver/install/ipa_ldap_updater.py
index 5df7cdf4275b75cc63045e29b92902698dc011a9..3d6c8043764d7f5ad398bf43606927e3a7b8bb1a 100644
--- a/ipaserver/install/ipa_ldap_updater.py
+++ b/ipaserver/install/ipa_ldap_updater.py
@@ -46,9 +46,6 @@ class LDAPUpdater(admintool.AdminTool):
def add_options(cls, parser):
super(LDAPUpdater, cls).add_options(parser, debug_option=True)
-parser.add_option("-t", "--test", action="store_true", dest="test",
-default=False,
-help="run through the update without changing anything")
parser.add_option("-y", dest="password",
help="file containing the Directory Manager password")
parser.add_option("-l", '--ldapi', action="store_true", dest="ldapi",
@@ -139,7 +136,7 @@ class LDAPUpdater_Upgrade(LDAPUpdater):
updates = None
realm = krbV.default_context().default_realm
-upgrade = IPAUpgrade(realm, self.files, live_run=not options.test,
+upgrade = IPAUpgrade(realm, self.files,
schema_files=options.schema_files)
upgrade.create_instance()
upgradefailed = upgrade.upgradefailed
@@ -149,9 +146,10 @@ class LDAPUpdater_Upgrade(LDAPUpdater):
'Bad syntax detected in upgrade file(s).', 1)
elif upgrade.upgradefailed:
raise admintool.ScriptError('IPA upgrade failed.', 1)
-elif upgrade.modified and options.test:
-self.log.info('Update complete, changes to be made, test mode')
-return 2
+elif upgrade.modified:
+self.log.info('Update complete')
+else:
+self.log.info('Update complete, no data were modified')
class LDAPUpdater_NonUpgrade(LDAPUpdater):
@@ -195,13 +193,11 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater):
modified = schemaupdate.update_schema(
options.schema_files,
dm_password=self.dirman_password,
-live_run=not options.test,
ldapi=options.ldapi) or modified
ld = LDAPUpdate(
dm_password=self.dirman_password,
sub_dict={},
-live_run=not options.test,
ldapi=options.ldapi,
plugins=options.plugins or self.run_plugins)
@@ -210,6 +206,7 @@ class LDAPUpdater_NonUpgrade(LDAP
Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes
On 03/16/2015 03:58 PM, Martin Kosek wrote: On 03/16/2015 01:56 PM, Martin Babinsky wrote: On 03/13/2015 10:13 AM, Martin Kosek wrote: On 03/12/2015 09:43 PM, Nathan Kinder wrote: I have tested the patches on F21 client and they work as expected. I take that as an ACK. Before pushing the change, I just changed one print format from "%s" to "%d" given a number was printed. Pushed to: master: a58b77ca9cd3620201306258dd6bd05ea1c73c73 ipa-4-1: 80aeb445e2034776f08668bf04dfd711af477b25 Petr1, it would be nice to get this one built on F21+, to unblock Ipsilon project. F21 update: - https://admin.fedoraproject.org/updates/freeipa-4.1.3-3.fc21 F22 update: - https://admin.fedoraproject.org/updates/freeipa-4.1.3-3.fc22 -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code
On 16.3.2015 17:20, Martin Babinsky wrote:
> On 03/16/2015 01:35 PM, Jan Cholasta wrote:
>> Dne 16.3.2015 v 13:30 Martin Babinsky napsal(a):
>>> On 03/16/2015 12:15 PM, Martin Kosek wrote:
On 03/13/2015 05:37 PM, Martin Babinsky wrote:
> Attaching the next iteration of patches.
Very good! I hopefully have last two nitpicks :-) See below.
> diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
> index
> 4116d974e620341119b56fad3cff1bda48af3bab..cd03e9fd17b60b8b7324d0ccd436a10f7556baf0
> 100644
> --- a/ipapython/ipautil.py
> +++ b/ipapython/ipautil.py
> @@ -1175,27 +1175,61 @@ def wait_for_open_socket(socket_name, timeout=0):
> else:
> raise e
>
> -def kinit_hostprincipal(keytab, ccachedir, principal):
> +
> +def kinit_keytab(keytab, ccache_path, principal, attempts=1):
> """
> -Given a ccache directory and a principal kinit as that user.
> +Given a ccache_path , keytab file and a principal kinit as that user.
> +
> +The optional parameter 'attempts' specifies how many times the credential
> +initialization should be attempted before giving up and raising
> +StandardError.
>
> This blindly overwrites the current CCNAME so if you need to save
> it do so before calling this function.
>
> +This function is also not thread-safe since it modifies environment
> +variables.
> +
> Thus far this is used to kinit as the local host.
This note can be deleted because it is used elsewhere too.
> """
> -try:
> -ccache_file = 'FILE:%s/ccache' % ccachedir
> -krbcontext = krbV.default_context()
> -ktab = krbV.Keytab(name=keytab, context=krbcontext)
> -princ = krbV.Principal(name=principal, context=krbcontext)
> -os.environ['KRB5CCNAME'] = ccache_file
> -ccache = krbV.CCache(name=ccache_file, context=krbcontext,
> primary_principal=princ)
> -ccache.init(princ)
> -ccache.init_creds_keytab(keytab=ktab, principal=princ)
> -return ccache_file
> -except krbV.Krb5Error, e:
> -raise StandardError('Error initializing principal %s in %s: %s' %
> (principal, keytab, str(e)))
> +root_logger.debug("Initializing principal %s using keytab %s"
> + % (principal, keytab))
I'm sorry for nitpicking but it would be nice to log ccache_file too. Krb5
libs return quite weird errors when CC cache is not accessible so it helps to
have the path at hand.
--
Petr^2 Spacek
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] ipatests: port of p11helper test from github
Hi,
On 03/16/2015 05:23 PM, Martin Basti wrote:
On 16/03/15 15:32, Milan Kubik wrote:
On 03/16/2015 12:03 PM, Milan Kubik wrote:
On 03/13/2015 02:59 PM, Milan Kubik wrote:
Hi,
this is a patch with port of [1] to pytest.
[1]:
https://github.com/spacekpe/freeipa-pkcs11/blob/master/python/run.py
Cheers,
Milan
Added few more asserts in methods where the test could fail and
cause other errors.
New version of the patch after brief discussion with Martin Basti.
Removed unnecessary variable assignments and separated a new test case.
Hello,
thank you for the patch.
I have a few nitpicks:
1)
You can remove this and use just hexlify(s)
+def str_to_hex(s):
+return ''.join("{:02x}".format(ord(c)) for c in s)
done
2)
+ def test_find_secret_key(self, p11):
+ assert p11.find_keys(_ipap11helper.KEY_CLASS_SECRET_KEY,
label=u"žžž-aest")
In tests before you tested the exact number of expected IDs returned
by find_keys method, why not here?
Lack of attention.
Fixed the assert in `test_search_for_master_key` which does the same
thing. Merged `test_find_secret_key` with `test_search_for_master_key`
where it belongs.
Martin^2
Milan
>From 03b4ab2134fde396b4fbd5879199d5a417f1a1d0 Mon Sep 17 00:00:00 2001
From: Milan Kubik
Date: Thu, 12 Mar 2015 16:52:33 +0100
Subject: [PATCH] ipatests: port of p11helper test from github
Ported the github hosted [1] script to use pytest's abilities
and included it in ipatests/test_ipapython directory.
[1]: https://github.com/spacekpe/freeipa-pkcs11/blob/master/python/run.py
https://fedorahosted.org/freeipa/ticket/4829
---
ipatests/test_ipapython/test_ipap11helper.py | 207 +++
make-lint| 2 +-
2 files changed, 208 insertions(+), 1 deletion(-)
create mode 100644 ipatests/test_ipapython/test_ipap11helper.py
diff --git a/ipatests/test_ipapython/test_ipap11helper.py b/ipatests/test_ipapython/test_ipap11helper.py
new file mode 100644
index ..fb632870503039038cee33133077fb74f7581fc0
--- /dev/null
+++ b/ipatests/test_ipapython/test_ipap11helper.py
@@ -0,0 +1,207 @@
+# -*- coding: utf-8 -*-
+# Authors:
+# Milan Kubik
+#
+# Copyright (C) 2015 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
Re: [Freeipa-devel] [PATCH] 0041 Always reload StateFile before getting or modifying the, stored values.
On 16/03/15 13:54, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4901 ACK, it works as expected -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0003-3 User life cycle: new stageuser plugin with add verb
Dne 16.3.2015 v 12:06 David Kupka napsal(a): On 03/06/2015 07:30 PM, thierry bordaz wrote: On 02/19/2015 04:19 PM, Martin Basti wrote: On 19/02/15 13:01, thierry bordaz wrote: On 02/04/2015 05:14 PM, Jan Cholasta wrote: Hi, Dne 4.2.2015 v 15:25 David Kupka napsal(a): On 02/03/2015 11:50 AM, thierry bordaz wrote: On 09/17/2014 12:32 PM, thierry bordaz wrote: On 09/01/2014 01:08 PM, Petr Viktorin wrote: On 08/08/2014 03:54 PM, thierry bordaz wrote: Hi, The attached patch is related to 'User Life Cycle' (https://fedorahosted.org/freeipa/ticket/3813) It creates a stageuser plugin with a first function stageuser-add. Stage user entries are provisioned under 'cn=staged users,cn=accounts,cn=provisioning,SUFFIX'. Thanks thierry Avoid `from ipalib.plugins.baseldap import *` in new code; instead import the module itself and use e.g. `baseldap.LDAPObject`. The stageuser help (docstring) is copied from the user plugin, and discusses things like account lockout and disabling users. It should rather explain what stageuser itself does. (And I don't very much like the Note about the interface being badly designed...) Also decide if the docs should call it "staged user" or "stage user" or "stageuser". A lot of the code is copied and pasted over from the users plugin. Don't do that. Either import things (e.g. validate_nsaccountlock) from the users plugin, or move the reused code into a shared module. For the `user` object, since so much is the same, it might be best to create a common base class for user and stageuser; and similarly for the Command plugins. The default permissions need different names, and you don't need another copy of the 'non_object' ones. Also, run the makeaci script. Hello, This modified patch is mainly moving common base class into a new plugin: accounts.py. user/stageuser plugin inherits from accounts. It also creates a better description of what are stage user, how to add a new stage user, updates ACI.txt and separate active/stage user managed permissions. thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Thanks David for the reviews. Here the last patches ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel The freeipa-tbordaz-0002 patch had trailing whitespaces on few lines so I'm attaching fixed version (and unchanged patch freeipa-tbordaz-0003-3 to keep them together). The ULC feature is still WIP but these patches look good to me and don't break anything as far as I tested. We should push them now to avoid further rebases. Thierry can then prepare other patches delivering the rest of ULC functionality. Few comments from just reading the patches: 1) I would name the base class "baseuser", "account" does not necessarily mean user account. 2) This is very wrong: -class user_add(LDAPCreate): +class user_add(user, LDAPCreate): You are creating a plugin which is both an object and an command. 3) This is purely subjective, but I don't like the name "deleteuser", as it has a verb in it. We usually don't do that and IMHO we shouldn't do that. Honza Thank you for the review. I am attaching the updates patches ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello, I'm getting errors during make rpms: if [ "" != "yes" ]; then \ ./makeapi --validate; \ ./makeaci --validate; \ fi /root/freeipa/ipalib/plugins/baseuser.py:641 command "baseuser_add" doc is not internationalized /root/freeipa/ipalib/plugins/baseuser.py:653 command "baseuser_find" doc is not internationalized /root/freeipa/ipalib/plugins/baseuser.py:647 command "baseuser_mod" doc is not internationalized 0 commands without doc, 3 commands whose doc is not i18n Command baseuser_add in ipalib, not in API Command baseuser_find in ipalib, not in API Command baseuser_mod in ipalib, not in API There are one or more new commands defined. Update API.txt and increment the minor version in VERSION. There are one or more documentation problems. You must fix these before preceeding Issues probably caused by this: 1) You should not use the register decorator, if this class is just for inheritance @register() class baseuser_add(LDAPCreate): @register() class baseuser_mod(LDAPUpdate): @register() class baseuser_find(LDAPSearch): see dns.py plugin and "DNSZoneBase" and "dnszone" classes 2) there might be an issue with @register() class baseuser(LDAPObject): the register decorator should not be there, I was warned by Petr^3 to not use permission in parent class. The same permission should be specified only in one place (for example user class), (otherwise they will be generated twice??) I don't know more details about it. -- Martin Basti Hello
