Re: [Freeipa-devel] [PATCH 0190] DNSSEC: add support for CKM_RSA_PKCS_OAEP mechanism
On 03/05/2015 02:45 PM, Petr Spacek wrote:
On 26.2.2015 16:59, Martin Basti wrote:
On 26/02/15 12:47, Petr Spacek wrote:
On 11.2.2015 14:10, Martin Basti wrote:
https://fedorahosted.org/freeipa/ticket/4657#comment:13
Patch attached.
--
Martin Basti
freeipa-mbasti-0190-DNSSEC-add-support-for-CKM_RSA_PKCS_OAEP-mechanism.patch
From 4d698a5adaa94eb854c75bd9bcaf3093f31a11e5 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 11 Feb 2015 14:05:46 +0100
Subject: [PATCH] DNSSEC add support for CKM_RSA_PKCS_OAEP mechanism
Ticket: https://fedorahosted.org/freeipa/ticket/4657#comment:13
---
ipapython/ipap11helper/p11helper.c | 72
--
1 file changed, 69 insertions(+), 3 deletions(-)
diff --git a/ipapython/ipap11helper/p11helper.c
b/ipapython/ipap11helper/p11helper.c
index
4e0f262057b377124793f1e3091a8c9df4794164..c638bbe849f1bbddc8004bd1c41128b1c9e7
100644
--- a/ipapython/ipap11helper/p11helper.c
+++ b/ipapython/ipap11helper/p11helper.c
@@ -53,6 +53,22 @@
// TODO
#define CKA_COPYABLE (0x0017)
+#define CKG_MGF1_SHA1 (0x0001)
+
+#define CKZ_DATA_SPECIFIED(0x0001)
+
+struct ck_rsa_pkcs_oaep_params {
+ CK_MECHANISM_TYPE hash_alg;
+ unsigned long mgf;
+ unsigned long source;
+ void *source_data;
+ unsigned long source_data_len;
+};
+
+typedef struct ck_rsa_pkcs_oaep_params CK_RSA_PKCS_OAEP_PARAMS;
+typedef struct ck_rsa_pkcs_oaep_params *CK_RSA_PKCS_OAEP_PARAMS_PTR;
+
+
CK_BBOOL true = CK_TRUE;
CK_BBOOL false = CK_FALSE;
@@ -118,6 +134,17 @@ CK_BBOOL* bool;
} PyObj2Bool_mapping_t;
/**
+ * Constants
+ */
+static const CK_RSA_PKCS_OAEP_PARAMS CONST_RSA_PKCS_OAEP_PARAMS = {
+.hash_alg = CKM_SHA_1,
+.mgf = CKG_MGF1_SHA1,
+.source = CKZ_DATA_SPECIFIED,
+.source_data = NULL,
+.source_data_len = 0
+};
+
+/**
* ipap11helper Exceptions
*/
static PyObject *ipap11helperException; //parent class for all exceptions
@@ -1359,17 +1386,36 @@ P11_Helper_export_wrapped_key(P11_Helper* self,
PyObject *args, PyObject *kwds)
CK_BYTE_PTR wrapped_key = NULL;
CK_ULONG wrapped_key_len = 0;
CK_MECHANISM wrapping_mech = { CKM_RSA_PKCS, NULL, 0 };
-CK_MECHANISM_TYPE wrapping_mech_type = CKM_RSA_PKCS;
/* currently we don't support parameter in mechanism */
static char *kwlist[] = { key, wrapping_key, wrapping_mech,
NULL };
//TODO check long overflow
//TODO export method
if (!PyArg_ParseTupleAndKeywords(args, kwds, kkk|, kwlist,
object_key,
-object_wrapping_key, wrapping_mech_type)) {
+object_wrapping_key, wrapping_mech.mechanism)) {
return NULL;
}
-wrapping_mech.mechanism = wrapping_mech_type;
+
+// fill mech parameters
+switch(wrapping_mech.mechanism){
+case CKM_RSA_PKCS:
+case CKM_AES_KEY_WRAP:
+case CKM_AES_KEY_WRAP_PAD:
+//default params
+break;
+
+case CKM_RSA_PKCS_OAEP:
+/* Use the same configuration as openSSL
+ * https://www.openssl.org/docs/crypto/RSA_public_encrypt.html
+ */
+ wrapping_mech.pParameter = (void*) CONST_RSA_PKCS_OAEP_PARAMS;
+ wrapping_mech.ulParameterLen =
sizeof(CONST_RSA_PKCS_OAEP_PARAMS);
+break;
+
+default:
+PyErr_SetString(ipap11helperError, Unsupported wrapping
mechanism);
+return NULL;
+}
rv = self-p11-C_WrapKey(self-session, wrapping_mech,
object_wrapping_key, object_key, NULL, wrapped_key_len);
@@ -1452,6 +1498,26 @@ P11_Helper_import_wrapped_secret_key(P11_Helper*
self, PyObject *args,
return NULL;
}
+switch(wrapping_mech.mechanism){
+case CKM_RSA_PKCS:
+case CKM_AES_KEY_WRAP:
+case CKM_AES_KEY_WRAP_PAD:
+//default params
+break;
NACK. This switch is duplicate of the previous one. Please split it into an
auxiliary function and call it twice.
Thank you!
Thanks. Updated patch attached.
Pushed to master, ipa-4-1.
ACK, it works for me.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0194] Remove unused method to export secret key from ipapkcs11helper module
On 03/05/2015 02:45 PM, Petr Spacek wrote: On 25.2.2015 14:24, Martin Basti wrote: The method never been used, and never will be, because we do not want to export secrets. Ticket: https://fedorahosted.org/freeipa/ticket/4657 Patch attached (may require mbasti-0195, mbasti-0190) ACK, it works for me. Pushed to master, ipa-4-1. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] UI plugins
Sounds great. Just wanted to know if I was going to be reinventing my own
wheel again.
Thanks again.
Corey
On Mar 6, 2015 6:58 AM, Petr Vobornik pvobo...@redhat.com wrote:
On 03/06/2015 02:31 PM, Corey Kovacs wrote:
I almost forgot to ask. Since you don't point it out, I am assuming (yeah
I
know) the plugin code methods have not changed from 3.3 to 4.1? That is to
day I should be able to use the same techniques?
Same techniques should be applicable, there were no major improvements in
plugin support in 4.1. But Web UI doesn't have any stable API so little
things could be different.
Basically plugins interact with Web UI's core code which is not
perfect(easy to break things) but it's powerful and better than nothing.
Thanks again!
Corey
On Fri, Mar 6, 2015 at 3:51 AM, Petr Vobornik pvobo...@redhat.com
wrote:
On 03/06/2015 03:54 AM, Corey Kovacs wrote:
After reading the extending freeipa training document I was able
successfully add us to meet attributes and add/modify them using the
cli
which was pretty cool. Now that I got the cli out of the way I want to
add
the fields to the ui. Because of the similarities between what I want to
do
and the example given in the docs I just followed along and changed
variables where it made sense to do so. I cannot however get the new
field
to show up. The Apache logs don't show any errors but they do show the
plugin being read as the page (user details) is loaded. After searching
around I found a document which attempts to explain the process but it
assumes some knowledge held by the reader which I don't possess.
It looks like I supposed to create some sort of index loader which
loads
all of the modules which are part of the plugin. I can't seem to find
any
good documents telling the whole process or least non that I can make
sense
of.
Plugin could be just one file, if the plugin name is myplugin it has
to
be:
/usr/share/ipa/ui/js/plugins/myplugin/myplugin.js
IPA Web UI uses AMD module format:
- https://dojotoolkit.org/documentation/tutorials/1.10/modules/
- http://requirejs.org/docs/whyamd.html#amd
I have some example plugins here:
- https://pvoborni.fedorapeople.org/plugins/
For your case I would look at:
- https://pvoborni.fedorapeople.org/plugins/employeenumber/
employeenumber.js
Web UI basics and introspection:
(this section is little redundant to your question, but it might help
others)
FreeIPA Web UI is semi declarative. That means that part of the pages
could be thrown away, modified or extended by plugins before the page is
constructed. To do that, one has to modify specification object of
particular module.
Here, I would assume that you don't have UI sources(before minification),
so all introspection will be done in running web ui. Otherwise it's
easier
to inspect sources in install/ui/src/freeipa, i.e. checkout ipa-3-3
branch
from upstream git.
List of modules could be find(after authentication) in browse developer
tools console in object:
window.require.cache
or (depends on IPA version)
window.dojo.config.cache
One can then obtain the module by:
var user_module = require('freeipa/user')
specification object is usually in 'entity_spec' or '$enity_name_spec'
property.
user_module.entity_spec
UI is internally organized into entities. Entity corresponds to ipalib
object. Entity, e.g. user, usually have multiple pages called facets. To
get a list of facets:
user_module.entity_spec.facets
The one with fields has usually a name details. For users it's the
second facet:
var details_facet = user_module.entity_spec.facets[1]
IF i simplify it a bit, we can say that fields on a page are organized in
sections:
details_facet.sections
Section has fields. A field usually represents an editable element on
page, e.g. a textbox with label, validators and other stuff.
Example of inspection:
https://pvoborni.fedorapeople.org/images/inspect_ui.png
Your goal is to pick a section, or create a new one an add a field there.
To know what to define, just examine a definition of already existing
field
and just amend name, label, ...
It would help also to understand how to debug such a thing.
In browser developer tools. There is javascript console (use in the
text
above), javascript debugger, network tab for inspecting loaded files.
For developing RHEL 7 plugin, I would suggest you to install test
instance
of FreeIPA 3.3 and use Debugging with source codes method described in:
- https://pvoborni.fedorapeople.org/doc/#!/guide/Debugging
Some tips:
- if you get a weird dojo loader messegate, you probably have a syntax
error in the plugin or you don't return a plugin object or the plugin
could
not be loaded (bad name)
- it's good to use some JavaScript liner - jsl or jshint to catch syntax
errors early.
I running version 3.3 on rhel 7. Any help or pointers to more
documentation would be greatly appreciated.
Re: [Freeipa-devel] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
On 03/06/2015 02:45 PM, Martin Babinsky wrote: On 03/06/2015 02:08 PM, Jan Cholasta wrote: Hi Martin, Dne 6.3.2015 v 13:05 Martin Babinsky napsal(a): This series of patches for the master/4.1 branch attempts to implement some of the Rob's and Petr Vobornik's ideas which originated from a discussion on this list regarding my original patch fixing https://fedorahosted.org/freeipa/ticket/4808. I suppose that these patches are just a first iteration, we may further discuss if this is the right thing to do. Below is a quote from the original discussion just to get the context: 3) I think it would be nice to support ccache types other than FILE. According to Petr Vobornik (see his reply), the user is limited mostly to FILE ccache type, so I don't know if it will make sense to support also other types. Actually I agree with Honza. I wanted to say that with your implementation user can't use anything else - are limited to FILE type. -- 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] SPEC: Require python2 version of sssd bindings
On (06/03/15 16:13), Alexander Bokovoy wrote: On Fri, 06 Mar 2015, Lukas Slebodnik wrote: On (05/03/15 16:20), Petr Vobornik wrote: On 03/05/2015 11:23 AM, Lukas Slebodnik wrote: On (05/03/15 08:54), Petr Vobornik wrote: On 02/27/2015 09:50 PM, Lukas Slebodnik wrote: ehlo, Please review attached patches and fix freeipa in fedora 22 ASAP. I think the most critical is 1st patch sh$ git grep SSSDConfig | grep import install/tools/ipa-upgradeconfig:import SSSDConfig ipa-client/ipa-install/ipa-client-automount:import SSSDConfig ipa-client/ipa-install/ipa-client-install:import SSSDConfig BTW package python-sssdconfig is provides since sssd-1.10.0alpha1 (2013-04-02) but it was not explicitely required. The latest python3 changes in sssd (fedora 22) is just a result of negligent packaging of freeipa. LS Fedora 22 was amended. Patch 1: ACK Patch 2: ACK Patch3: the package name is libsss_nss_idmap-python not python-libsss_nss_idmap which already is required in adtrust package In sssd upstream we decided to rename package libsss_nss_idmap-python to python-libsss_nss_idmap according to new rpm python guidelines. The python3 version has alredy correct name. We will rename package in downstream with next major release (1.13). Of course it we will add Provides: libsss_nss_idmap-python. We can push 3rd patch later or I can update 3rd patch. What do you prefer? Than you very much for review. LS Patch 3 should be updated to not forget the remaining change in ipa-python package. It then should be updated downstream and master when 1.13 is released in Fedora, or in master sooner if SSSD 1.13 becomes the minimal version required by master. Fixed. BTW Why ther is a pylint comment for some sssd modules I did not kave any pylint problems after removing comment. ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401 ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401 And why are these modules optional (try except) Because they are needed to properly load in the case trust subpackages are not installed, to generate proper messages to users who will try these commands, like 'ipa trust-add' while the infrastructure is not in place. pylint is dumb for such cases. Yes but my patches added requires to all necessary packages. How can I get pylint warning? I modified spec file and make-lint was called in %check phase. and I did not have any pylint problems in mock. LS -- 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] SPEC: Require python2 version of sssd bindings
On Fri, 06 Mar 2015, Lukas Slebodnik wrote: On (06/03/15 16:13), Alexander Bokovoy wrote: On Fri, 06 Mar 2015, Lukas Slebodnik wrote: On (05/03/15 16:20), Petr Vobornik wrote: On 03/05/2015 11:23 AM, Lukas Slebodnik wrote: On (05/03/15 08:54), Petr Vobornik wrote: On 02/27/2015 09:50 PM, Lukas Slebodnik wrote: ehlo, Please review attached patches and fix freeipa in fedora 22 ASAP. I think the most critical is 1st patch sh$ git grep SSSDConfig | grep import install/tools/ipa-upgradeconfig:import SSSDConfig ipa-client/ipa-install/ipa-client-automount:import SSSDConfig ipa-client/ipa-install/ipa-client-install:import SSSDConfig BTW package python-sssdconfig is provides since sssd-1.10.0alpha1 (2013-04-02) but it was not explicitely required. The latest python3 changes in sssd (fedora 22) is just a result of negligent packaging of freeipa. LS Fedora 22 was amended. Patch 1: ACK Patch 2: ACK Patch3: the package name is libsss_nss_idmap-python not python-libsss_nss_idmap which already is required in adtrust package In sssd upstream we decided to rename package libsss_nss_idmap-python to python-libsss_nss_idmap according to new rpm python guidelines. The python3 version has alredy correct name. We will rename package in downstream with next major release (1.13). Of course it we will add Provides: libsss_nss_idmap-python. We can push 3rd patch later or I can update 3rd patch. What do you prefer? Than you very much for review. LS Patch 3 should be updated to not forget the remaining change in ipa-python package. It then should be updated downstream and master when 1.13 is released in Fedora, or in master sooner if SSSD 1.13 becomes the minimal version required by master. Fixed. BTW Why ther is a pylint comment for some sssd modules I did not kave any pylint problems after removing comment. ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401 ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401 And why are these modules optional (try except) Because they are needed to properly load in the case trust subpackages are not installed, to generate proper messages to users who will try these commands, like 'ipa trust-add' while the infrastructure is not in place. pylint is dumb for such cases. Yes but my patches added requires to all necessary packages. How can I get pylint warning? I modified spec file and make-lint was called in %check phase. and I did not have any pylint problems in mock. If you don't have those packages installed, it will complain. -- / Alexander Bokovoy -- 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] SPEC: Require python2 version of sssd bindings
On (05/03/15 16:20), Petr Vobornik wrote:
On 03/05/2015 11:23 AM, Lukas Slebodnik wrote:
On (05/03/15 08:54), Petr Vobornik wrote:
On 02/27/2015 09:50 PM, Lukas Slebodnik wrote:
ehlo,
Please review attached patches and fix freeipa in fedora 22 ASAP.
I think the most critical is 1st patch
sh$ git grep SSSDConfig | grep import
install/tools/ipa-upgradeconfig:import SSSDConfig
ipa-client/ipa-install/ipa-client-automount:import SSSDConfig
ipa-client/ipa-install/ipa-client-install:import SSSDConfig
BTW package python-sssdconfig is provides since sssd-1.10.0alpha1
(2013-04-02)
but it was not explicitely required.
The latest python3 changes in sssd (fedora 22) is just a result of negligent
packaging of freeipa.
LS
Fedora 22 was amended.
Patch 1: ACK
Patch 2: ACK
Patch3:
the package name is libsss_nss_idmap-python not python-libsss_nss_idmap
which already is required in adtrust package
In sssd upstream we decided to rename package libsss_nss_idmap-python to
python-libsss_nss_idmap according to new rpm python guidelines.
The python3 version has alredy correct name.
We will rename package in downstream with next major release (1.13).
Of course it we will add Provides: libsss_nss_idmap-python.
We can push 3rd patch later or I can update 3rd patch.
What do you prefer?
Than you very much for review.
LS
Patch 3 should be updated to not forget the remaining change in ipa-python
package.
It then should be updated downstream and master when 1.13 is released in
Fedora, or in master sooner if SSSD 1.13 becomes the minimal version required
by master.
Fixed.
BTW Why ther is a pylint comment for some sssd modules
I did not kave any pylint problems after removing comment.
ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401
ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401
And why are these modules optional (try except)
ipalib/plugins/trust.py-31-try:
ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401
ipalib/plugins/trust.py-33-_murmur_installed = True
ipalib/plugins/trust.py-34-except Exception, e:
ipalib/plugins/trust.py-35-_murmur_installed = False
ipalib/plugins/trust.py-36-
ipalib/plugins/trust.py-37-try:
ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401
ipalib/plugins/trust.py-39-_nss_idmap_installed = True
ipalib/plugins/trust.py-40-except Exception, e:
ipalib/plugins/trust.py-41-_nss_idmap_installed = False
LS
From 41523fd6ab9ea95644cac1a6cd20386a620a1df5 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Fri, 27 Feb 2015 20:40:06 +0100
Subject: [PATCH 1/3] SPEC: Explicitly requires python-sssdconfig
Resolves:
https://fedorahosted.org/freeipa/ticket/4929
---
freeipa.spec.in | 2 ++
1 file changed, 2 insertions(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index
b186d9fdff31118ea4d929f024f4dc16a75b1d0b..9513f45c6c933a1109390393cb90d68e8c697dc7
100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -122,6 +122,7 @@ Requires: mod_auth_kerb = 5.4-16
Requires: mod_nss = 1.0.8-26
Requires: python-ldap = 2.4.15
Requires: python-krbV
+Requires: python-sssdconfig
Requires: acl
Requires: python-pyasn1
Requires: memcached
@@ -228,6 +229,7 @@ Requires: wget
Requires: libcurl = 7.21.7-2
Requires: xmlrpc-c = 1.27.4
Requires: sssd = 1.12.3
+Requires: python-sssdconfig
Requires: certmonger = 0.76.8
Requires: nss-tools
Requires: bind-utils
--
2.3.1
From 69c4f4bfc911990dc7ec650c2958aaf716c53ac5 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Fri, 27 Feb 2015 20:43:38 +0100
Subject: [PATCH 2/3] SPEC: Require python2 version of sssd bindings
Python modules pysss and pysss_murmur was part of package sssd-common.
Fedora 22 tries to get rid of python2 and therefore these modules were
extracted from package sssd-common to separate packages python-sss and
python-sss-murmur and python3 version of packages python3-sss
python3-sss-murmur
git grep pysss | grep import
ipalib/plugins/trust.py:import pysss_murmur #pylint: disable=F0401
ipaserver/dcerpc.py:import pysss
ipaserver/dcerpc.py is pacakged in freeipa-server-trust-ad
palib/plugins/trust.py is packaged in freeipa-python
Resolves:
https://fedorahosted.org/freeipa/ticket/4929
---
freeipa.spec.in | 6 ++
1 file changed, 6 insertions(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index
9513f45c6c933a1109390393cb90d68e8c697dc7..7a1ff8b50ef1b462ad14fb2328149c3c2ed2fb38
100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -195,6 +195,9 @@ Requires: samba = %{samba_version}
Requires: samba-winbind
Requires: libsss_idmap
Requires: libsss_nss_idmap-python
+%if (0%{?fedora} = 22)
+Requires: python-sss
+%endif
# We use alternatives to divert winbind_krb5_locator.so plugin to libkrb5
# on the installes where server-trust-ad subpackage is installed because
# IPA AD trusts cannot be used at the same time with the locator plugin
@@ -288,6 +291,9 @@ Requires:
Re: [Freeipa-devel] [PATCHES] SPEC: Require python2 version of sssd bindings
On Fri, 06 Mar 2015, Lukas Slebodnik wrote: On (05/03/15 16:20), Petr Vobornik wrote: On 03/05/2015 11:23 AM, Lukas Slebodnik wrote: On (05/03/15 08:54), Petr Vobornik wrote: On 02/27/2015 09:50 PM, Lukas Slebodnik wrote: ehlo, Please review attached patches and fix freeipa in fedora 22 ASAP. I think the most critical is 1st patch sh$ git grep SSSDConfig | grep import install/tools/ipa-upgradeconfig:import SSSDConfig ipa-client/ipa-install/ipa-client-automount:import SSSDConfig ipa-client/ipa-install/ipa-client-install:import SSSDConfig BTW package python-sssdconfig is provides since sssd-1.10.0alpha1 (2013-04-02) but it was not explicitely required. The latest python3 changes in sssd (fedora 22) is just a result of negligent packaging of freeipa. LS Fedora 22 was amended. Patch 1: ACK Patch 2: ACK Patch3: the package name is libsss_nss_idmap-python not python-libsss_nss_idmap which already is required in adtrust package In sssd upstream we decided to rename package libsss_nss_idmap-python to python-libsss_nss_idmap according to new rpm python guidelines. The python3 version has alredy correct name. We will rename package in downstream with next major release (1.13). Of course it we will add Provides: libsss_nss_idmap-python. We can push 3rd patch later or I can update 3rd patch. What do you prefer? Than you very much for review. LS Patch 3 should be updated to not forget the remaining change in ipa-python package. It then should be updated downstream and master when 1.13 is released in Fedora, or in master sooner if SSSD 1.13 becomes the minimal version required by master. Fixed. BTW Why ther is a pylint comment for some sssd modules I did not kave any pylint problems after removing comment. ipalib/plugins/trust.py:32:import pysss_murmur #pylint: disable=F0401 ipalib/plugins/trust.py:38:import pysss_nss_idmap #pylint: disable=F0401 And why are these modules optional (try except) Because they are needed to properly load in the case trust subpackages are not installed, to generate proper messages to users who will try these commands, like 'ipa trust-add' while the infrastructure is not in place. pylint is dumb for such cases. -- / Alexander Bokovoy -- 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] New freeipa-devel footer
Hi list, Are you also annoyed by freeipa-devel footers and having to delete them with every reply? I was certainly annoyed, and after yesterday bump from other FreeIPA developer I finally updated the freeipa-devel configuration to follow freeipa-users example and set a better footer, with proper -- prefix so that MUAs can trim it. See the footer below. If you have any improvements proposals, just tell me. -- Martin Kosek mko...@redhat.com Supervisor, Software Engineering - Identity Management Team Red Hat Inc. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Go to http://www.freeipa.org/page/Contribute/Code for more info on how to contribute to the project
[Freeipa-devel] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
This series of patches for the master/4.1 branch attempts to implement
some of the Rob's and Petr Vobornik's ideas which originated from a
discussion on this list regarding my original patch fixing
https://fedorahosted.org/freeipa/ticket/4808.
I suppose that these patches are just a first iteration, we may further
discuss if this is the right thing to do.
Below is a quote from the original discussion just to get the context:
--
Martin^3 Babinsky
Martin Babinsky wrote:
On 03/02/2015 04:28 PM, Rob Crittenden wrote:
Petr Vobornik wrote:
On 01/12/2015 05:45 PM, Martin Babinsky wrote:
related to ticket https://fedorahosted.org/freeipa/ticket/4808
this patch seems to be a bit forgotten.
It works, looks fine.
One minor issue: trailing whitespaces in the man page.
I also wonder if it shouldn't be used in other tools which call kinit
with keytab:
* ipa-client-automount:434
* ipa-client-install:2591 (this usage should be fine since it's used for
server installation)
* dcerpc.py:545
* rpcserver.py: 971, 981 (armor for web ui forms base auth)
Most importantly the ipa-client-automount because it's called from
ipa-client-install (if location is specified) and therefore it might
fail during client installation.
Or also, kinit call with admin creadentials worked for the user but I
wonder if it was just a coincidence and may break under slightly
different but similar conditions.
I think that's a fine idea. In fact there is already a function that
could be extended, kinit_hostprincipal().
rob
So in principle we could add multiple TGT retries to
kinit_hostprincipal() and then plug this function to all the places
Petr mentioned in order to provide this functionality each time TGT is
requested using keytab.
Do I understand it correctly?
Honestly I think I'd only do the retries on client installation. I
don't know that the other uses would really benefit or need this.
But this is an opportunity to consolidate some code, so I guess the
approach I'd take is to add an option to kinit_hostprincipal of
retries=0 so that only a single kinit is done. The client installers
would pass in some value.
This change is quite a bit more invasive but it's also early in the
release cycle so the risk will be spread out.
rob
From fda86199e97aa661e4ee3e73858966c7086a3ee0 Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Fri, 6 Mar 2015 12:40:42 +0100
Subject: [PATCH 1/5] modifications to ipautil.kinit_hostprincipal
1.) the function can now perform multiple attempts to get host TGT before
failing.
2.) instead of a directory name specifiying the location of credentials cache
the function now takes the full path including ccache filename.
---
ipapython/ipautil.py | 46 --
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4116d974e620341119b56fad3cff1bda48af3bab..90a8d4035bce218c4cd000c9434125131b311dd9 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1175,27 +1175,45 @@ def wait_for_open_socket(socket_name, timeout=0):
else:
raise e
-def kinit_hostprincipal(keytab, ccachedir, principal):
+def kinit_hostprincipal(keytab, ccache_name, principal, attempts=1):
-Given a ccache directory and a principal kinit as that user.
+Given a ccache_name 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.
Thus far this is used to kinit as the local host.
-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)))
+curr_attempt = 0
+ccache_file = 'FILE:%s' % ccache_name
+root_logger.debug(Initializing principal %s % principal)
+while True:
+curr_attempt += 1
+try:
+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)
+
Re: [Freeipa-devel] [PATCHES 0200-0202] DNS fixes related to unsupported records
On 4.3.2015 16:35, Martin Basti wrote: On 04/03/15 16:17, Martin Basti wrote: Ticket: https://fedorahosted.org/freeipa/ticket/4930 0200: 4.1, master Fixes traceback, which was raised if LDAP contained a record that was marked as unsupported. Now unsupported records are shown, if LDAP contains them. 0200: 4.1, master Records marked as unsupported will not show options for editing parts. 0202: only master Removes NSEC3PARAM record from record types. NSEC3PARAM can contain only zone, value is allowed only in idnszone objectclass, so do not confuse users. and patches attached :-) ACK. It works for me and can be pushed to branches 4.1 and master. -- 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] [PATCHES 0015-0019] changes to the way host TGT is obtained using keytab
On 03/06/2015 02:08 PM, Jan Cholasta wrote: Hi Martin, Dne 6.3.2015 v 13:05 Martin Babinsky napsal(a): This series of patches for the master/4.1 branch attempts to implement some of the Rob's and Petr Vobornik's ideas which originated from a discussion on this list regarding my original patch fixing https://fedorahosted.org/freeipa/ticket/4808. I suppose that these patches are just a first iteration, we may further discuss if this is the right thing to do. Below is a quote from the original discussion just to get the context: 1) Why 5 patches for 2 changes (kinit_hostprincipal instead of exec kinit, ipa-client-install --kinit-attempts)? Will squish them to a smaller number (2-3) 2) IMO a for loop would be better than an infinite while loop: for attempt in range(attempts): try: # kinit ... except krbV.Krb5Error as e: # kinit failed ... else: break else: # max attempts reached ... That's true. Infinite loops are tad scary anyway. 3) I think it would be nice to support ccache types other than FILE. According to Petr Vobornik (see his reply), the user is limited mostly to FILE ccache type, so I don't know if it will make sense to support also other types. 4) I would prefer if you kept using the full ccache name returned from kinit_hostprincipal when connecting to LDAP. 5) Given that the ccache path usually ends with /ccache, I would retain the old way of calling kinit_hostprincipal. You can do something like this to support all of the above: def kinit_hostprincipal(keytab, ccache_file, principal, attempts=1): if os.path.isdir(ccache_file): ccache_file = os.path.join(ccache_file, 'ccache') ... return ccache_file (You don't need to prepend FILE:, as it is the default ccache type.) Honza Dumb me didn't realize that 'ccache_file' is a reference to an actual filesystem path and that the filename can be set dynamically depending on path type (directory vs. file). Thank you for your comments. Will update the patches accordingly. -- Martin^3 Babinsky -- 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-0019] changes to the way host TGT is obtained using keytab
On 03/06/2015 01:05 PM, Martin Babinsky wrote: This series of patches for the master/4.1 branch attempts to implement some of the Rob's and Petr Vobornik's ideas which originated from a discussion on this list regarding my original patch fixing https://fedorahosted.org/freeipa/ticket/4808. I suppose that these patches are just a first iteration, we may further discuss if this is the right thing to do. Below is a quote from the original discussion just to get the context: The original kinit_hostprincipal had `ccachedir` argument, the new one has `ccache_name`. But the new code still prepends FILE ccache type: old: ccache_file = 'FILE:%s/ccache' % ccachedir new: ccache_file = 'FILE:%s' % ccache_name I would remove the line because I understand the use of 'ccache_name' name as equivalent of KRB5CCNAME and therefore I would expect that the value of this argument would be used to set the environment variable WITHOUT any modification. And mainly, user is limited only to FILE ccache type. I also wonder if os.environ['KRB5CCNAME'] = ccache_file has to be set when ccache is defined by krbV call: ccache = krbV.CCache(name=ccache_file, ... krbV snipped doesn't use it so maybe we can remove it. https://git.fedorahosted.org/cgit/python-krbV.git/tree/krbV-code-snippets.py -- 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] New freeipa-devel footer
On 6.3.2015 12:01, Martin Kosek wrote: On 03/06/2015 11:55 AM, Jan Pazdziora wrote: On Fri, Mar 06, 2015 at 11:43:07AM +0100, Martin Kosek wrote: See the footer below. If you have any improvements proposals, just tell me. Given the information about the list actions is in the List-* header of ever email, do you need the Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel bits? I like Go to http://www.freeipa.org/page/Contribute/Code for more info on how to contribute to the project but having it wrapped to 80 characters would be even nicer. Or even just Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code might be enough. Good idea. I fixed that part. As for link to mailman, I see it as an option for people now knowing that something as mail headers exists. But given this is freeipa-*devel* list, we may indeed remove it. I will see if there are any more opinions on that matter. Personally I would nuke the footer from devel list completely :-) You are already on devel list so what is the point of inviting you to it? :-) -- 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] UI plugins
I almost forgot to ask. Since you don't point it out, I am assuming (yeah I
know) the plugin code methods have not changed from 3.3 to 4.1? That is to
day I should be able to use the same techniques?
Thanks again!
Corey
On Fri, Mar 6, 2015 at 3:51 AM, Petr Vobornik pvobo...@redhat.com wrote:
On 03/06/2015 03:54 AM, Corey Kovacs wrote:
After reading the extending freeipa training document I was able
successfully add us to meet attributes and add/modify them using the cli
which was pretty cool. Now that I got the cli out of the way I want to add
the fields to the ui. Because of the similarities between what I want to
do
and the example given in the docs I just followed along and changed
variables where it made sense to do so. I cannot however get the new field
to show up. The Apache logs don't show any errors but they do show the
plugin being read as the page (user details) is loaded. After searching
around I found a document which attempts to explain the process but it
assumes some knowledge held by the reader which I don't possess.
It looks like I supposed to create some sort of index loader which loads
all of the modules which are part of the plugin. I can't seem to find any
good documents telling the whole process or least non that I can make
sense
of.
Plugin could be just one file, if the plugin name is myplugin it has to
be:
/usr/share/ipa/ui/js/plugins/myplugin/myplugin.js
IPA Web UI uses AMD module format:
- https://dojotoolkit.org/documentation/tutorials/1.10/modules/
- http://requirejs.org/docs/whyamd.html#amd
I have some example plugins here:
- https://pvoborni.fedorapeople.org/plugins/
For your case I would look at:
- https://pvoborni.fedorapeople.org/plugins/employeenumber/
employeenumber.js
Web UI basics and introspection:
(this section is little redundant to your question, but it might help
others)
FreeIPA Web UI is semi declarative. That means that part of the pages
could be thrown away, modified or extended by plugins before the page is
constructed. To do that, one has to modify specification object of
particular module.
Here, I would assume that you don't have UI sources(before minification),
so all introspection will be done in running web ui. Otherwise it's easier
to inspect sources in install/ui/src/freeipa, i.e. checkout ipa-3-3 branch
from upstream git.
List of modules could be find(after authentication) in browse developer
tools console in object:
window.require.cache
or (depends on IPA version)
window.dojo.config.cache
One can then obtain the module by:
var user_module = require('freeipa/user')
specification object is usually in 'entity_spec' or '$enity_name_spec'
property.
user_module.entity_spec
UI is internally organized into entities. Entity corresponds to ipalib
object. Entity, e.g. user, usually have multiple pages called facets. To
get a list of facets:
user_module.entity_spec.facets
The one with fields has usually a name details. For users it's the
second facet:
var details_facet = user_module.entity_spec.facets[1]
IF i simplify it a bit, we can say that fields on a page are organized in
sections:
details_facet.sections
Section has fields. A field usually represents an editable element on
page, e.g. a textbox with label, validators and other stuff.
Example of inspection:
https://pvoborni.fedorapeople.org/images/inspect_ui.png
Your goal is to pick a section, or create a new one an add a field there.
To know what to define, just examine a definition of already existing field
and just amend name, label, ...
It would help also to understand how to debug such a thing.
In browser developer tools. There is javascript console (use in the text
above), javascript debugger, network tab for inspecting loaded files.
For developing RHEL 7 plugin, I would suggest you to install test instance
of FreeIPA 3.3 and use Debugging with source codes method described in:
- https://pvoborni.fedorapeople.org/doc/#!/guide/Debugging
Some tips:
- if you get a weird dojo loader messegate, you probably have a syntax
error in the plugin or you don't return a plugin object or the plugin could
not be loaded (bad name)
- it's good to use some JavaScript liner - jsl or jshint to catch syntax
errors early.
I running version 3.3 on rhel 7. Any help or pointers to more
documentation would be greatly appreciated.
--
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-0019] changes to the way host TGT is obtained using keytab
Hi Martin, Dne 6.3.2015 v 13:05 Martin Babinsky napsal(a): This series of patches for the master/4.1 branch attempts to implement some of the Rob's and Petr Vobornik's ideas which originated from a discussion on this list regarding my original patch fixing https://fedorahosted.org/freeipa/ticket/4808. I suppose that these patches are just a first iteration, we may further discuss if this is the right thing to do. Below is a quote from the original discussion just to get the context: 1) Why 5 patches for 2 changes (kinit_hostprincipal instead of exec kinit, ipa-client-install --kinit-attempts)? 2) IMO a for loop would be better than an infinite while loop: for attempt in range(attempts): try: # kinit ... except krbV.Krb5Error as e: # kinit failed ... else: break else: # max attempts reached ... 3) I think it would be nice to support ccache types other than FILE. 4) I would prefer if you kept using the full ccache name returned from kinit_hostprincipal when connecting to LDAP. 5) Given that the ccache path usually ends with /ccache, I would retain the old way of calling kinit_hostprincipal. You can do something like this to support all of the above: def kinit_hostprincipal(keytab, ccache_file, principal, attempts=1): if os.path.isdir(ccache_file): ccache_file = os.path.join(ccache_file, 'ccache') ... return ccache_file (You don't need to prepend FILE:, as it is the default ccache type.) Honza -- Jan Cholasta -- 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] New freeipa-devel footer
On 03/06/2015 01:42 PM, Petr Spacek wrote: On 6.3.2015 12:01, Martin Kosek wrote: On 03/06/2015 11:55 AM, Jan Pazdziora wrote: On Fri, Mar 06, 2015 at 11:43:07AM +0100, Martin Kosek wrote: See the footer below. If you have any improvements proposals, just tell me. Given the information about the list actions is in the List-* header of ever email, do you need the Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel bits? I like Go to http://www.freeipa.org/page/Contribute/Code for more info on how to contribute to the project but having it wrapped to 80 characters would be even nicer. Or even just Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code might be enough. Good idea. I fixed that part. As for link to mailman, I see it as an option for people now knowing that something as mail headers exists. But given this is freeipa-*devel* list, we may indeed remove it. I will see if there are any more opinions on that matter. Personally I would nuke the footer from devel list completely :-) You are already on devel list so what is the point of inviting you to it? :-) Example: you are googling for something FreeIPA devel related, it gives you mail from archive. Then having convenient link may be useful (if you are lazy googling freeipa-devel) :-) The http://www.freeipa.org/page/Contribute/Code is especially useful IMO, I want everyone to know how to contribute patches to FreeIPA... Martin -- 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] IPA 4.2 server upgrade refactoring - summary
On 03/04/2015 07:04 PM, Martin Basti wrote: Summary extracted from thread [Freeipa-devel] IPA Server upgrade 4.2 design Design page: http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring * ipa-server-upgrade will not allow to use DM password, only LDAPI will be used for upgrade * upgrade files will be executed in alphabetical order, updater will not require number in update file name. But we should still keep the numbering in new upgrade files. * LDAP updates will be applied per file, in order specified in file (from top to bottom) * new directive in update files *plugin:plugin-name* will execute update plugins (renamed form update-plugin to plugin) * option --skip-version-check will override version check in ipactl and ipa-server-upgrade commands (was --force before, but this collides with existing --force option in ipactl) * huge warning, this may broke everything, in help, man, or CLI for --skip-version-check option * ipa-upgradeconfig will be removed * ipa-ldap-updater will be changed to not allow overall update. It stays as util for execute particular update files. Makes sense to me. Everyone ok with above so that Martin can start working on the changes? How and when execute upgrades (after discussion with Honza) -- not updated in design page yet A) ipactl*: A.1) compare build platform and platform from last upgrade/installation (based on used ipaplatform file) A.1.i) if platform mismatch, raise error and prevent to start services A.2) version of LDAP data(+schema included) compared to current version (VENDOR_VERSION will be used) A.2.i) if version of LDAP data is newer than version of build, raise error and prevent services to start A.2.ii) if version of LDAP data is older than version of build, upgrade is required A.2.iii) if versions are the same, continue A.3) check if services requires update (this should be available after installer refactoring)** A.3.i) if any service requires configuration upgrade, upgrade is required A.3.ii) if any service raises an error about wrong configuration (which cannot be automatically fixed and requires manual fix by user), raise error and prevent to start services A.4.i) if upgrade is needed, ipactl will prevent to start services, and promt user to run ipa-server-upgrade manually (ipactl will not execute upgrade itself) A.4.ii) otherwise start services B) ipa-server-upgrade* B.0) services should be in shutdown state, if not, stop services (services will be started during upgrade on demand, then stopped) B.1) compare build platform and platform from last upgrade/installation (based on used ipaplatform file) B.1.i) if platform mismatch, raise error stop upgrade B.2) check version of LDAP data B.2.i) if LDAP data version is newer than build version, raise error stop upgrade B.2.ii) if LDAP data version is the same as build version, skip schema and LDAP data upgrade B.2.iii) if LDAP data version is older than build version -- data upgrade required B.3) Check if services require upgrade, detect errors as in A.3) (?? this step may not be there)** B.4) if data upgrade required, upgrade schema, then upgrade data, if successful store current build version as data version B.5) Run service upgrade (if needed?)** B.6) if upgrade is successful, inform user that the one can now start IPA (upgrade will not start IPA after it is done) * with --skip-version-check option, ipactl will start services, ipa-server-upgrade will upgrade everything ** services will handle local configuration upgrade by themselves. They will not use data version to decide if upgrade is required (TODO implementation details, Honza wants it in this way - sharing code with installers) Upgrade in different enviroments: 1) Upgrade during RPM transaction (as we do now) -- if it is possible, upgrade will be executed during RPM transaction, service will be started after upgrade (+ add messages IPA is currently upgrading, please wait) 2) Upgrade cannot be executed during RPM transaction (fedup, --no-script, containers) -- IPA will not start if update is required, user have to run upgrade manually, using ipa-server-upgrade command (+ log/print errors that there is upgrade required) Martin^2 -- 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] UI plugins
On 03/06/2015 02:31 PM, Corey Kovacs wrote:
I almost forgot to ask. Since you don't point it out, I am assuming (yeah I
know) the plugin code methods have not changed from 3.3 to 4.1? That is to
day I should be able to use the same techniques?
Same techniques should be applicable, there were no major improvements
in plugin support in 4.1. But Web UI doesn't have any stable API so
little things could be different.
Basically plugins interact with Web UI's core code which is not
perfect(easy to break things) but it's powerful and better than nothing.
Thanks again!
Corey
On Fri, Mar 6, 2015 at 3:51 AM, Petr Vobornik pvobo...@redhat.com wrote:
On 03/06/2015 03:54 AM, Corey Kovacs wrote:
After reading the extending freeipa training document I was able
successfully add us to meet attributes and add/modify them using the cli
which was pretty cool. Now that I got the cli out of the way I want to add
the fields to the ui. Because of the similarities between what I want to
do
and the example given in the docs I just followed along and changed
variables where it made sense to do so. I cannot however get the new field
to show up. The Apache logs don't show any errors but they do show the
plugin being read as the page (user details) is loaded. After searching
around I found a document which attempts to explain the process but it
assumes some knowledge held by the reader which I don't possess.
It looks like I supposed to create some sort of index loader which loads
all of the modules which are part of the plugin. I can't seem to find any
good documents telling the whole process or least non that I can make
sense
of.
Plugin could be just one file, if the plugin name is myplugin it has to
be:
/usr/share/ipa/ui/js/plugins/myplugin/myplugin.js
IPA Web UI uses AMD module format:
- https://dojotoolkit.org/documentation/tutorials/1.10/modules/
- http://requirejs.org/docs/whyamd.html#amd
I have some example plugins here:
- https://pvoborni.fedorapeople.org/plugins/
For your case I would look at:
- https://pvoborni.fedorapeople.org/plugins/employeenumber/
employeenumber.js
Web UI basics and introspection:
(this section is little redundant to your question, but it might help
others)
FreeIPA Web UI is semi declarative. That means that part of the pages
could be thrown away, modified or extended by plugins before the page is
constructed. To do that, one has to modify specification object of
particular module.
Here, I would assume that you don't have UI sources(before minification),
so all introspection will be done in running web ui. Otherwise it's easier
to inspect sources in install/ui/src/freeipa, i.e. checkout ipa-3-3 branch
from upstream git.
List of modules could be find(after authentication) in browse developer
tools console in object:
window.require.cache
or (depends on IPA version)
window.dojo.config.cache
One can then obtain the module by:
var user_module = require('freeipa/user')
specification object is usually in 'entity_spec' or '$enity_name_spec'
property.
user_module.entity_spec
UI is internally organized into entities. Entity corresponds to ipalib
object. Entity, e.g. user, usually have multiple pages called facets. To
get a list of facets:
user_module.entity_spec.facets
The one with fields has usually a name details. For users it's the
second facet:
var details_facet = user_module.entity_spec.facets[1]
IF i simplify it a bit, we can say that fields on a page are organized in
sections:
details_facet.sections
Section has fields. A field usually represents an editable element on
page, e.g. a textbox with label, validators and other stuff.
Example of inspection:
https://pvoborni.fedorapeople.org/images/inspect_ui.png
Your goal is to pick a section, or create a new one an add a field there.
To know what to define, just examine a definition of already existing field
and just amend name, label, ...
It would help also to understand how to debug such a thing.
In browser developer tools. There is javascript console (use in the text
above), javascript debugger, network tab for inspecting loaded files.
For developing RHEL 7 plugin, I would suggest you to install test instance
of FreeIPA 3.3 and use Debugging with source codes method described in:
- https://pvoborni.fedorapeople.org/doc/#!/guide/Debugging
Some tips:
- if you get a weird dojo loader messegate, you probably have a syntax
error in the plugin or you don't return a plugin object or the plugin could
not be loaded (bad name)
- it's good to use some JavaScript liner - jsl or jshint to catch syntax
errors early.
I running version 3.3 on rhel 7. Any help or pointers to more
documentation would be greatly appreciated.
--
Petr Vobornik
--
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
[Freeipa-devel] [PATCHES 0204-0207] Server upgrade: Make LDAP data upgrade deterministic
The patchset ensure, the upgrade order will respect ordering of entries
in *.update files.
Required for: https://fedorahosted.org/freeipa/ticket/4904
Patch 205 also fixes https://fedorahosted.org/freeipa/ticket/3560
Required patch mbasti-0203
Patches attached.
--
Martin Basti
From 9dd32e80f37b852feb980fd4ef2ec7c082ffc1a5 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Thu, 26 Feb 2015 12:01:19 +0100
Subject: [PATCH 2/5] Server Upgrade: do not sort updates by DN
Ticket: https://fedorahosted.org/freeipa/ticket/4904
---
ipaserver/install/ldapupdate.py | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 53d5407d5e8a15abe13f2f6d8b3df74ca100ea5a..e8516ff86a951f828c4213f8e70db613b99ed8c4 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -784,22 +784,11 @@ class LDAPUpdate:
raise RuntimeError(Offline updates are not supported.)
def _run_updates(self, all_updates):
-# For adds and updates we want to apply updates from shortest
-# to greatest length of the DN.
-# For deletes we want the reverse
-def update_sort_key(dn_update):
-dn, update = dn_update
-assert isinstance(dn, DN)
-return len(dn)
-sorted_updates = sorted(all_updates.iteritems(), key=update_sort_key)
-
-for dn, update in sorted_updates:
+for dn, update in all_updates.iteritems():
self._update_record(update)
-# Now run the deletes in reversed order
-sorted_updates.reverse()
-for dn, update in sorted_updates:
+for dn, update in all_updates.iteritems():
self._delete_record(update)
def update(self, files, ordered=False):
--
2.1.0
From 689caca4d322a18108756b530522be625f5b0964 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Thu, 5 Mar 2015 16:56:02 +0100
Subject: [PATCH 3/5] Server Upgrade: Upgrade one file per time
* Files are sorted alphabetically, no numbering required anymore
* One file updated per time
Ticket: https://fedorahosted.org/freeipa/ticket/3560
---
ipaserver/install/ldapupdate.py | 54 ++---
1 file changed, 18 insertions(+), 36 deletions(-)
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index e8516ff86a951f828c4213f8e70db613b99ed8c4..3b4aa58d9e84006510c23e4aa7d52a84c205f79c 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -793,44 +793,27 @@ class LDAPUpdate:
def update(self, files, ordered=False):
Execute the update. files is a list of the update files to use.
+:param ordered: Update files are executed in alphabetical order
- If ordered is True then the updates the file must be of the form
- ##-name.update where ## is an integer between 10 and 89. The
- changes are applied to LDAP at the end of each value divisible
- by 10, so after 20, 30, etc.
-
- returns True if anything was changed, otherwise False
+returns True if anything was changed, otherwise False
-pat = re.compile(r'(\d+)-.*\.update')
all_updates = {}
-r = 20
-if self.plugins:
-self.info('PRE_UPDATE')
-updates = api.Backend.updateclient.update(PRE_UPDATE, self.dm_password, self.ldapi, self.live_run)
-self.merge_updates(all_updates, updates)
try:
self.create_connection()
-if ordered and all_updates:
+if self.plugins:
+self.info('PRE_UPDATE')
+updates = api.Backend.updateclient.update(PRE_UPDATE, self.dm_password, self.ldapi, self.live_run)
+self.merge_updates(all_updates, updates)
# flush out PRE_UPDATE plugin updates before we begin
self._run_updates(all_updates)
all_updates = {}
-for f in files:
-name = os.path.basename(f)
-if ordered:
-m = pat.match(name)
-if not m:
-raise RuntimeError(Filename does not match format #-name.update: %s % f)
-index = int(m.group(1))
-if index 10 or index 90:
-raise RuntimeError(Index not legal range: %d % index)
-
-if index = r:
-self._run_updates(all_updates)
-all_updates = {}
-r += 10
+upgrade_files = files
+if ordered:
+upgrade_files = sorted(files)
+for f in upgrade_files:
try:
self.info(Parsing update file '%s' % f)
data = self.read_file(f)
@@ -839,17 +822,16 @@ class LDAPUpdate:
[Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
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.
--
Martin Basti
From 0616696080c25531d9dd5c30d7e32e0a7da9ed6c Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Fri, 6 Mar 2015 17:44:47 +0100
Subject: [PATCH] Server Upgrade: respect --test option in plugins
Several plugins do the LDAP data modification directly.
In test mode these plugis should not be executed.
https://fedorahosted.org/freeipa/ticket/3448
---
ipaserver/install/plugins/dns.py| 9 +
ipaserver/install/plugins/fix_replica_agreements.py | 4
ipaserver/install/plugins/update_idranges.py| 7 +++
ipaserver/install/plugins/update_managed_permissions.py | 4
ipaserver/install/plugins/update_pacs.py| 4
ipaserver/install/plugins/update_referint.py| 3 +++
ipaserver/install/plugins/update_services.py| 4
7 files changed, 35 insertions(+)
diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index f562978bcbcc02321c0e9a668af88b4f596f8556..2e33982e7d53cad794cca5867aa94a0df766eff8 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -60,6 +60,10 @@ class update_dnszones(PostUpdate):
order=MIDDLE
def execute(self, **options):
+if not options.get('live_run'):
+self.log.info(Test mode: skipping 'update_dnszones')
+return False, False, ()
+
ldap = self.obj.backend
if not dns_container_exists(ldap):
return (False, False, [])
@@ -159,6 +163,11 @@ class update_master_to_dnsforwardzones(PostUpdate):
backup_path = u'%s%s' % (backup_dir, backup_filename)
def execute(self, **options):
+if not options.get('live_run'):
+self.log.info(Test mode: skipping
+ 'update_master_to_dnsforwardzones')
+return False, False, ()
+
ldap = self.obj.backend
# check LDAP if forwardzones already uses new semantics
dns_container_dn = DN(api.env.container_dns, api.env.basedn)
diff --git a/ipaserver/install/plugins/fix_replica_agreements.py b/ipaserver/install/plugins/fix_replica_agreements.py
index a5ff4819fa4c432b378a9f1c0f6952bc312a6792..597a133145e1cad2265386bf6eb55d47fefa86e3 100644
--- a/ipaserver/install/plugins/fix_replica_agreements.py
+++ b/ipaserver/install/plugins/fix_replica_agreements.py
@@ -38,6 +38,10 @@ class update_replica_attribute_lists(PreUpdate):
order = MIDDLE
def execute(self, **options):
+if not options.get('live_run'):
+self.log.info(Test mode: skipping
+ 'update_replica_attribute_lists')
+return False, False, ()
# We need an IPAdmin connection to the backend
self.log.debug(Start replication agreement exclude list update task)
conn = ipaldap.IPAdmin(api.env.host, ldapi=True, realm=api.env.realm)
diff --git a/ipaserver/install/plugins/update_idranges.py b/ipaserver/install/plugins/update_idranges.py
index 1aa5fa7631fd35a7aaf4a23a5eee44e4e0a2e904..cc462ef1265e3bee2137df1c787d93b048981e25 100644
--- a/ipaserver/install/plugins/update_idranges.py
+++ b/ipaserver/install/plugins/update_idranges.py
@@ -33,6 +33,9 @@ class update_idrange_type(PostUpdate):
order = MIDDLE
def execute(self, **options):
+if not options.get('live_run'):
+self.log.info(Test mode: skipping 'update_idrange_type')
+return False, False, ()
ldap = self.obj.backend
base_dn = DN(api.env.container_ranges, api.env.basedn)
@@ -121,6 +124,10 @@ class update_idrange_baserid(PostUpdate):
order = LAST
def execute(self, **options):
+if not options.get('live_run'):
+self.log.info(Test mode: skipping 'update_idrange_baserid')
+return False, False, ()
+
ldap = self.obj.backend
base_dn = DN(api.env.container_ranges, api.env.basedn)
diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py
index 430a2919a315bfd8d8e6174a915890d44b782c5c..55e1068540d99ad189aa85682ba9ef1e96293abb 100644
--- a/ipaserver/install/plugins/update_managed_permissions.py
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -402,6 +402,10 @@ class update_managed_permissions(PostUpdate):
def execute(self, **options):
+if not options.get('live_run'):
+self.log.info(Test mode: skipping 'update_managed_permissions')
+return False, False, ()
+
ldap = self.api.Backend[ldap2]
anonymous_read_aci = self.get_anonymous_read_aci(ldap)
diff --git a/ipaserver/install/plugins/update_pacs.py b/ipaserver/install/plugins/update_pacs.py
[Freeipa-devel] [PATCH 0203] Remove unused PRE_SCHEMA upgrade
This upgrade step is not used anymore.
Required by: https://fedorahosted.org/freeipa/ticket/4904
Patch attached.
--
Martin Basti
From caf35d4ed7fd5309fc4a547cc0342a8295c263ef Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 25 Feb 2015 17:55:11 +0100
Subject: [PATCH 1/5] Server Upgrade: Remove unused PRE_SCHEMA_UPDATE
This is not used anymore.
Ticket: https://fedorahosted.org/freeipa/ticket/4904
---
ipaserver/install/ipa_ldap_updater.py | 16 +++-
ipaserver/install/ldapupdate.py | 15 +--
ipaserver/install/plugins/__init__.py | 1 -
ipaserver/install/plugins/baseupdate.py | 14 +-
ipaserver/install/plugins/dns.py| 3 +--
ipaserver/install/upgradeinstance.py| 17 -
6 files changed, 10 insertions(+), 56 deletions(-)
diff --git a/ipaserver/install/ipa_ldap_updater.py b/ipaserver/install/ipa_ldap_updater.py
index 18970ce409cab7abd801dcf048b7e7241609c98a..4ad7b972795e3aaacd1f5b43a2ad1e26b34a1367 100644
--- a/ipaserver/install/ipa_ldap_updater.py
+++ b/ipaserver/install/ipa_ldap_updater.py
@@ -191,15 +191,6 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater):
modified = False
-ld = LDAPUpdate(
-dm_password=self.dirman_password,
-sub_dict={},
-live_run=not options.test,
-ldapi=options.ldapi,
-plugins=options.plugins or self.run_plugins)
-
-modified = ld.pre_schema_update(ordered=True)
-
if options.update_schema:
modified = schemaupdate.update_schema(
options.schema_files,
@@ -207,6 +198,13 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater):
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)
+
if not self.files:
self.files = ld.get_all_files(UPDATES_DIR)
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 47f0399b928b3b0da3954592d56750450454aac7..53d5407d5e8a15abe13f2f6d8b3df74ca100ea5a 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -42,8 +42,7 @@ from ipalib import api
from ipaplatform.paths import paths
from ipapython.dn import DN
from ipapython.ipa_log_manager import *
-from ipaserver.install.plugins import (PRE_UPDATE, POST_UPDATE,
- PRE_SCHEMA_UPDATE)
+from ipaserver.install.plugins import (PRE_UPDATE, POST_UPDATE)
from ipaserver.plugins import ldap2
UPDATES_DIR=paths.UPDATES_DIR
@@ -803,18 +802,6 @@ class LDAPUpdate:
for dn, update in sorted_updates:
self._delete_record(update)
-def pre_schema_update(self, ordered=False):
-Execute the update before the LDPA schema is updated.
-
-if self.plugins:
-self.info('PRE_SCHEMA_UPDATE')
-all_updates = {}
-updates = api.Backend.updateclient.update(PRE_SCHEMA_UPDATE, self.dm_password, self.ldapi, self.live_run)
-self.merge_updates(all_updates, updates)
-self._run_updates(all_updates)
-
-return self.modified
-
def update(self, files, ordered=False):
Execute the update. files is a list of the update files to use.
diff --git a/ipaserver/install/plugins/__init__.py b/ipaserver/install/plugins/__init__.py
index 210c56ef79b7018a572ff92dc584ae15400596fe..49bef4df80d9b8ea2f5861dcb69c7ae2fb882472 100644
--- a/ipaserver/install/plugins/__init__.py
+++ b/ipaserver/install/plugins/__init__.py
@@ -20,7 +20,6 @@
Provide a separate api for updates.
-PRE_SCHEMA_UPDATE = 0
PRE_UPDATE = 1
POST_UPDATE = 2
diff --git a/ipaserver/install/plugins/baseupdate.py b/ipaserver/install/plugins/baseupdate.py
index dc6672ac5b1edeb4147686d1b05d9bf5d66f68b7..fa997c9dbe933be28fe462923a84cce338e05b6c 100644
--- a/ipaserver/install/plugins/baseupdate.py
+++ b/ipaserver/install/plugins/baseupdate.py
@@ -20,8 +20,7 @@
from ipalib import api
from ipalib import Updater, Object
from ipaserver.install import service
-from ipaserver.install.plugins import (PRE_UPDATE, POST_UPDATE,
- PRE_SCHEMA_UPDATE, MIDDLE)
+from ipaserver.install.plugins import (PRE_UPDATE, POST_UPDATE, MIDDLE)
class DSRestart(service.Service):
@@ -57,17 +56,6 @@ class update(Object):
api.register(update)
-class PreSchemaUpdate(Updater):
-
-Base class for updates that run after file processing.
-
-updatetype = PRE_SCHEMA_UPDATE
-order = MIDDLE
-
-def __init__(self):
-super(PreSchemaUpdate, self).__init__()
-
-
class PreUpdate(Updater):
Base class for updates that run prior to file processing.
diff --git a/ipaserver/install/plugins/dns.py
Re: [Freeipa-devel] New freeipa-devel footer
On Fri, Mar 06, 2015 at 11:43:07AM +0100, Martin Kosek wrote: See the footer below. If you have any improvements proposals, just tell me. Given the information about the list actions is in the List-* header of ever email, do you need the Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel bits? I like Go to http://www.freeipa.org/page/Contribute/Code for more info on how to contribute to the project but having it wrapped to 80 characters would be even nicer. Or even just Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code might be enough. -- Jan Pazdziora Principal Software Engineer, Identity Management Engineering, Red Hat -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Go to http://www.freeipa.org/page/Contribute/Code for more info on how to contribute to the project
Re: [Freeipa-devel] New freeipa-devel footer
On 03/06/2015 11:55 AM, Jan Pazdziora wrote: On Fri, Mar 06, 2015 at 11:43:07AM +0100, Martin Kosek wrote: See the footer below. If you have any improvements proposals, just tell me. Given the information about the list actions is in the List-* header of ever email, do you need the Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel bits? I like Go to http://www.freeipa.org/page/Contribute/Code for more info on how to contribute to the project but having it wrapped to 80 characters would be even nicer. Or even just Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code might be enough. Good idea. I fixed that part. As for link to mailman, I see it as an option for people now knowing that something as mail headers exists. But given this is freeipa-*devel* list, we may indeed remove it. I will see if there are any more opinions on that matter. -- 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 0023-0025] p11helper improvements
On 03/05/2015 04:10 PM, Martin Basti wrote: On 05/03/15 15:37, Petr Spacek wrote: On 5.3.2015 14:50, Petr Spacek wrote: Hello, please review this patch set. It should be applied on top of your previous p11helper patch set. Thank you! Reviewer requested reworded version of the error message, here it is. Thank you for patches. Required patches: mbasti-190-2, mbasti-195-2, mbasti-196 0023: ACK 0024: ACK 0025-2: ACK Martin^2 Pushed to: master: 459f0a8401cf0fa4f7ba119646b797340c0a796c ipa-4-1: 8fefd63152d5f5a28ac6cf51b504a150d8e7b360 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] UI plugins
On 03/06/2015 03:54 AM, Corey Kovacs wrote:
After reading the extending freeipa training document I was able
successfully add us to meet attributes and add/modify them using the cli
which was pretty cool. Now that I got the cli out of the way I want to add
the fields to the ui. Because of the similarities between what I want to do
and the example given in the docs I just followed along and changed
variables where it made sense to do so. I cannot however get the new field
to show up. The Apache logs don't show any errors but they do show the
plugin being read as the page (user details) is loaded. After searching
around I found a document which attempts to explain the process but it
assumes some knowledge held by the reader which I don't possess.
It looks like I supposed to create some sort of index loader which loads
all of the modules which are part of the plugin. I can't seem to find any
good documents telling the whole process or least non that I can make sense
of.
Plugin could be just one file, if the plugin name is myplugin it has
to be:
/usr/share/ipa/ui/js/plugins/myplugin/myplugin.js
IPA Web UI uses AMD module format:
- https://dojotoolkit.org/documentation/tutorials/1.10/modules/
- http://requirejs.org/docs/whyamd.html#amd
I have some example plugins here:
- https://pvoborni.fedorapeople.org/plugins/
For your case I would look at:
- https://pvoborni.fedorapeople.org/plugins/employeenumber/employeenumber.js
Web UI basics and introspection:
(this section is little redundant to your question, but it might help
others)
FreeIPA Web UI is semi declarative. That means that part of the pages
could be thrown away, modified or extended by plugins before the page is
constructed. To do that, one has to modify specification object of
particular module.
Here, I would assume that you don't have UI sources(before
minification), so all introspection will be done in running web ui.
Otherwise it's easier to inspect sources in install/ui/src/freeipa, i.e.
checkout ipa-3-3 branch from upstream git.
List of modules could be find(after authentication) in browse developer
tools console in object:
window.require.cache
or (depends on IPA version)
window.dojo.config.cache
One can then obtain the module by:
var user_module = require('freeipa/user')
specification object is usually in 'entity_spec' or '$enity_name_spec'
property.
user_module.entity_spec
UI is internally organized into entities. Entity corresponds to ipalib
object. Entity, e.g. user, usually have multiple pages called facets. To
get a list of facets:
user_module.entity_spec.facets
The one with fields has usually a name details. For users it's the
second facet:
var details_facet = user_module.entity_spec.facets[1]
IF i simplify it a bit, we can say that fields on a page are organized
in sections:
details_facet.sections
Section has fields. A field usually represents an editable element on
page, e.g. a textbox with label, validators and other stuff.
Example of inspection:
https://pvoborni.fedorapeople.org/images/inspect_ui.png
Your goal is to pick a section, or create a new one an add a field
there. To know what to define, just examine a definition of already
existing field and just amend name, label, ...
It would help also to understand how to debug such a thing.
In browser developer tools. There is javascript console (use in the text
above), javascript debugger, network tab for inspecting loaded files.
For developing RHEL 7 plugin, I would suggest you to install test
instance of FreeIPA 3.3 and use Debugging with source codes method
described in:
- https://pvoborni.fedorapeople.org/doc/#!/guide/Debugging
Some tips:
- if you get a weird dojo loader messegate, you probably have a syntax
error in the plugin or you don't return a plugin object or the plugin
could not be loaded (bad name)
- it's good to use some JavaScript liner - jsl or jshint to catch syntax
errors early.
I running version 3.3 on rhel 7. Any help or pointers to more
documentation would be greatly appreciated.
--
Petr Vobornik
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Go to http://www.freeipa.org/page/Contribute/Code for more info on how to
contribute to the project
Re: [Freeipa-devel] [PATCH 0195] Fix memory leaks in ipapkcs11helper module
On 03/05/2015 02:45 PM, Petr Spacek wrote:
On 26.2.2015 17:01, Martin Basti wrote:
On 26/02/15 13:06, Petr Spacek wrote:
Hello Martin,
thank you for patch! This NACK is only aesthetic :-)
On 25.2.2015 14:21, Martin Basti wrote:
if (!check_return_value(rv, import_wrapped_key: key unwrapping)) {
+error = 1;
+goto final;
+}
This exact sequence is repeated many times in the code.
I would prefer a C macro like this:
#define GOTO_FAIL\
do {\
error = 1;\
goto final;\
} while(0)
This allows more dense code like:
if (!test)
GOTO_FAIL;
and does not have the risk of missing error = 1 somewhere.
+final:
if (pkey != NULL)
EVP_PKEY_free(pkey);
+if (label != NULL) PyMem_Free(label);
+if (error){
+return NULL;
+}
return ret;
}
Apparently, this is inconsistent with itself.
Please pick one style and use it, e.g.
if (label != NULL)
PyMem_Free(label)
... and do not add curly braces when unnecessary.
If you want, we can try running $ indent on current sources and committing
changes separately so you do not have to make changes like this by hand.
Thanks. Updated patch attached.
ACK, it works for me.
Pushed to master, ipa-4-1.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 134-136] extdom: handle ERANGE return code for getXXYYY_r()
On Thu, 05 Mar 2015, Sumit Bose wrote:
On Thu, Mar 05, 2015 at 09:16:36AM +0100, Sumit Bose wrote:
On Wed, Mar 04, 2015 at 06:14:53PM +0100, Sumit Bose wrote:
On Wed, Mar 04, 2015 at 04:17:55PM +0200, Alexander Bokovoy wrote:
On Mon, 02 Mar 2015, Sumit Bose wrote:
diff --git
a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index
20fdd62b20f28f5384cf83b8be5819f721c6c3db..84aeb28066f25f05a89d0c2d42e8b060e2399501 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -49,6 +49,220 @@
#define MAX(a,b) (((a)(b))?(a):(b))
#define SSSD_DOMAIN_SEPARATOR '@'
+#define MAX_BUF (1024*1024*1024)
+
+
+
+static int get_buffer(size_t *_buf_len, char **_buf)
+{
+long pw_max;
+long gr_max;
+size_t buf_len;
+char *buf;
+
+pw_max = sysconf(_SC_GETPW_R_SIZE_MAX);
+gr_max = sysconf(_SC_GETGR_R_SIZE_MAX);
+
+if (pw_max == -1 gr_max == -1) {
+buf_len = 16384;
+} else {
+buf_len = MAX(pw_max, gr_max);
+}
Here you'd get buf_len equal to 1024 by default on Linux which is too
low for our use case. I think it would be beneficial to add one more
MAX(buf_len, 16384):
-if (pw_max == -1 gr_max == -1) {
-buf_len = 16384;
-} else {
-buf_len = MAX(pw_max, gr_max);
-}
+buf_len = MAX(16384, MAX(pw_max, gr_max));
with MAX(MAX(),..) you also get rid of if() statement as resulting
rvalue would be guaranteed to be positive.
done
The rest is going along the common lines but would it be better to
allocate memory once per LDAP client request rather than always ask for
it per each NSS call? You can guarantee a sequential use of the buffer
within the LDAP client request processing so there is no problem with
locks but having this memory re-allocated on subsequent
getpwnam()/getpwuid()/... calls within the same request processing seems
suboptimal to me.
ok, makes sense, I moved get_buffer() back to the callers.
New version attached.
Please ignore this patch, I will send a revised version soon.
Please find attached a revised version which properly reports missing
objects and out-of-memory cases and makes sure buf and buf_len are in
sync.
ACK to patches 0135 and 0136. This concludes the review, thanks!
--
/ Alexander Bokovoy
--
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
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 Martin, Jan, Thanks for your review. I changed the patch so that it does not
Re: [Freeipa-devel] [PATCH] Password vault
Hi Endi,
Dne 24.2.2015 v 04:09 Endi Sukma Dewata napsal(a):
On 2/16/2015 2:50 AM, Endi Sukma Dewata wrote:
Hi,
Attached are the updated patches for the password vault, and some new
ones (please disregard previous patch submissions). Please give them a
try. Thanks.
New patches attached replacing all previous vault patches. They include
the new escrow functionality, changes to the asymmetric vault, and some
cleanups. Thanks.
Patch 353:
1) Please follow PEP8 in new code.
The pep8 tool reports these errors in existing files:
./ipalib/constants.py:98:80: E501 line too long (84 79 characters)
./ipalib/plugins/baseldap.py:1527:80: E501 line too long (81 79
characters)
./ipalib/plugins/user.py:915:80: E501 line too long (80 79 characters)
as well as many errors in the files this patch adds.
2) Pylint reports the following error:
ipatests/test_xmlrpc/test_vault_plugin.py:153:
[E0102(function-redefined), test_vault] class already defined line 27)
3) The container_vault config option should be renamed to
container_vaultcontainer, as it is used in the vaultcontainer plugin,
not the vault plugin.
4) The vault object should be child of the vaultcontainer object.
Not only is this correct from the object model perspective, but it would
also make all the container_id hacks go away.
5) When specifying param flags, use set literals.
This is especially wrong, because it's not a tuple, but a string in
parentheses:
+flags=('virtual_attribute'),
6) The `container` param of vault should actually be an option in
vault_* commands.
Also it should be renamed to `container_id`, for consistency with
vaultcontainer.
7) The `vault_id` param of vault should have no_option in flags, since
it is output-only.
8) Don't translate docstrings where not necessary:
+def get_dn(self, *keys, **options):
+__doc__ = _(
+Generates vault DN from vault ID.
+)
Only plugin modules and classes should have translated docstrings.
9) This looks wrong in vault.get_dn() and vaultcontainer.get_dn():
+name = None
+if keys:
+name = keys[0]
Primary key of the object should always be set, so the if statement
should not be there.
Also, primary key of any given object is always last in keys, so use
keys[-1] instead of keys[0].
10) Use self.api instead of api to access the API in plugins.
11) No clever optimizations like this please:
+# vault DN cannot be the container base DN
+if len(dn) == len(api.Object.vaultcontainer.base_dn):
+raise ValueError('Invalid vault DN: %s' % dn)
Compare the DNs by value instead.
12) vault.split_id() is not used anywhere.
13) Bytes is not base64-encoded data:
+Bytes('data?',
+cli_name='data',
+doc=_('Base-64 encoded binary data to archive'),
+),
It is base64-encoded in the CLI, but on the API level it is not. The doc
should say just Binary data to archive.
14) Use File instead of Str for input files:
+Str('in?',
+cli_name='in',
+doc=_('File containing data to archive'),
+),
15) Use MutuallyExclusiveError instead of ValidationError when there are
mutually exclusive options specified.
16) You do way too much stuff in vault_add.forward(). Only code that
must be done on the client needs to be there, i.e. handling of the
data, text and in options.
The vault_archive call must be in vault_add.execute(), otherwise a) we
will be making 2 RPC calls from the client and b) it won't be called at
all when api.env.in_server is True.
17) Why are vaultcontainer objects automatically created in vault_add?
If you have to automatically create them, you also have to automatically
delete them when the command fails. But that's a hassle, so I would just
not create them automatically.
18) Why are vaultcontainer objects automatically created in vault_find?
This is just plain wrong and has to be removed, now.
19) What is the reason behind all the json stuff in vault_transport_cert?
vault_transport_cert.__json__() is exactly the same as
Command.__json__() and hence redundant.
20) Are vault_transport_cert, vault_archive and vault_retrieve supposed
to be runnable by users? If not, add NO_CLI = True to the class
definition.
21) vault_archive is not a retrieve operation, it should be based on
LDAPUpdate instead of LDAPRetrieve. Or Command actually, since it does
not do anything with LDAP. The same applies to vault_retrieve.
22) vault_archive will break with binary data that is not UTF-8 encoded
text.
This is where it occurs:
+vault_data[u'data'] = unicode(data)
Generally, don't use unicode() on str values and str() on unicode values
directly, always use .decode() and .encode().
23) Since vault containers are nested, the vaultcontainer object should
be child of itself.
There is no support for nested objects in the framework, but it
shouldn't be too hard to do
