Re: [Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption

2013-11-29 Thread Ana Krivokapic
On 11/29/2013 02:45 PM, Ana Krivokapic wrote:
> On 11/25/2013 04:48 PM, Ana Krivokapic wrote:
>> On 11/15/2013 02:26 PM, Petr Vobornik wrote:
>>> Hello list,
>>>
>>> this is a first part of RCUE adoption effort. Main themes of this patch set 
>>> are:
>>>
>>> - use RCUE navigation <https://fedorahosted.org/freeipa/ticket/3902>
>>> - new styles for textboxes, textareas, radio/checkbox buttons and buttons-
>>> part of <https://fedorahosted.org/freeipa/ticket/3904>
>>> - new internal form layout (tables replaced by divs)
>>> - layout does not have fixed size 
>>> <https://fedorahosted.org/freeipa/ticket/3435>
>>> - new dialog styles + removed dependency on jQuery UI dialog
>>> - icons replaced by Font Awesome glyphs
>>>
>>> Example is at: <http://pvoborni.fedorapeople.org/rcue/>
>>>
>>> Some reasonings and additional info:
>>>
>>> 1. RCUE includes Bootstrap which defines o lot of styles for a lot of 
>>> things.
>>> That messed up the UI and therefore I did the form changes now.
>>>
>>> 2. jQuery UI is pretty big lib and we used it only for dialog and buttons.
>>> Buttons were replaced by RCUE buttons so removal of dialog dependency was a
>>> obvious step to get rid of the whole lib. The lib is removed from main UI 
>>> but
>>> is still present in separate pages - will be removed later.
>>>
>>> 3. Dojo and jQuery were upgraded to latest
>>> versions.<https://fedorahosted.org/freeipa/ticket/2811>
>>>
>>> This approach was ACKed by Kyle from a design perspective with a note that 
>>> we
>>> will review and fixed some styling after second phase. We should not release
>>> until then.
>>>
>>> The second phase, which I'm working on right now, will consist of:
>>>  * login screen <https://fedorahosted.org/freeipa/ticket/3903>
>>>  * new styles for standalone pages
>>>  * necessary responsive enhancement (the ultimate future goal is responsive
>>> layout)
>>>
>>> It's quite a lot of patches so I did not attach them here. You can see the
>>> code in my private repo:  
>>> branch
>>> 'rcue'.
>> I tested this phase of RCUE adoption effort, overall it looks and works 
>> great. A
>> couple of findings:
>>
>> 1) Two ui integration tests are failing, I guess this is due to the 
>> re-arranging
>> of elements on the automember page. So the tests should be amended to reflect
>> that change.

Commit ee4f6540490a16f0fbb5cdd02097a9b3ff354252 works around the navigation
issue which caused tests to fail. ACK.

>>
>> 2) In the 'Add Host' dialog, validation error messages do not have the proper
>> styles. E.g. if you don't fill any values, and just click 'Add', 'Required
>> field' messages appear, but they are not red, and the related fields do not 
>> get
>> a red frame.
>>
>>
> I have reviewed Petr's fix for the issue with the host adder dialog (issue 2)
> above). The fix is contained in commit 
> 829eb4f69e004c20f2d06b16ebd8d9bd176a81dd
> in his private repo. I can confirm that the adder dialog now behaves 
> correctly,
> so ACK for that fix.
>


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption

2013-11-29 Thread Ana Krivokapic
On 11/25/2013 04:48 PM, Ana Krivokapic wrote:
> On 11/15/2013 02:26 PM, Petr Vobornik wrote:
>> Hello list,
>>
>> this is a first part of RCUE adoption effort. Main themes of this patch set 
>> are:
>>
>> - use RCUE navigation <https://fedorahosted.org/freeipa/ticket/3902>
>> - new styles for textboxes, textareas, radio/checkbox buttons and buttons-
>> part of <https://fedorahosted.org/freeipa/ticket/3904>
>> - new internal form layout (tables replaced by divs)
>> - layout does not have fixed size 
>> <https://fedorahosted.org/freeipa/ticket/3435>
>> - new dialog styles + removed dependency on jQuery UI dialog
>> - icons replaced by Font Awesome glyphs
>>
>> Example is at: <http://pvoborni.fedorapeople.org/rcue/>
>>
>> Some reasonings and additional info:
>>
>> 1. RCUE includes Bootstrap which defines o lot of styles for a lot of things.
>> That messed up the UI and therefore I did the form changes now.
>>
>> 2. jQuery UI is pretty big lib and we used it only for dialog and buttons.
>> Buttons were replaced by RCUE buttons so removal of dialog dependency was a
>> obvious step to get rid of the whole lib. The lib is removed from main UI but
>> is still present in separate pages - will be removed later.
>>
>> 3. Dojo and jQuery were upgraded to latest
>> versions.<https://fedorahosted.org/freeipa/ticket/2811>
>>
>> This approach was ACKed by Kyle from a design perspective with a note that we
>> will review and fixed some styling after second phase. We should not release
>> until then.
>>
>> The second phase, which I'm working on right now, will consist of:
>>  * login screen <https://fedorahosted.org/freeipa/ticket/3903>
>>  * new styles for standalone pages
>>  * necessary responsive enhancement (the ultimate future goal is responsive
>> layout)
>>
>> It's quite a lot of patches so I did not attach them here. You can see the
>> code in my private repo:  
>> branch
>> 'rcue'.
> I tested this phase of RCUE adoption effort, overall it looks and works 
> great. A
> couple of findings:
>
> 1) Two ui integration tests are failing, I guess this is due to the 
> re-arranging
> of elements on the automember page. So the tests should be amended to reflect
> that change.
>
> 2) In the 'Add Host' dialog, validation error messages do not have the proper
> styles. E.g. if you don't fill any values, and just click 'Add', 'Required
> field' messages appear, but they are not red, and the related fields do not 
> get
> a red frame.
>
>

I have reviewed Petr's fix for the issue with the host adder dialog (issue 2)
above). The fix is contained in commit 829eb4f69e004c20f2d06b16ebd8d9bd176a81dd
in his private repo. I can confirm that the adder dialog now behaves correctly,
so ACK for that fix.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0079 Enable Retro Changelog and Content Synchronization DS plugins

2013-11-28 Thread Ana Krivokapic
On 10/29/2013 06:57 PM, Ana Krivokapic wrote:
> On 10/29/2013 12:46 PM, Martin Kosek wrote:
>> On 10/25/2013 05:03 PM, Ana Krivokapic wrote:
>>> Hello,
>>>
>>> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3967.
>>>
>> NACK. I do not think this will work well with the case when DNS is not used. 
>> As
>> bind-dyndb-ldap is not required component, FreeIPA could be installed on a
>> machine without bind installed. And in that case, /var/named/ won't be there.
>>
>> I think that this directory will follow similar pattern as
>> %{_localstatedir}/lib/ipa/pki-ca/publish
>> and be just %ghost and be created in when bind-dyndb-ldap is being configured
>> in bindinstance.py.
>>
>> Martin
> Fixed, updated patch attached.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

The patch needed a rebase.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 0b0b9e24afadcac34e5c02428b736b9d9b019594 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 25 Oct 2013 12:41:25 +0200
Subject: [PATCH] Enable Retro Changelog and Content Synchronization DS plugins

Enable Retro Changelog and Content Synchronization DS plugins which are required
for SyncRepl support.

Create a working directory /var/named/ipa required by bind-dyndb-ldap v4+.

https://fedorahosted.org/freeipa/ticket/3967
---
 freeipa.spec.in|  1 +
 install/tools/ipa-upgradeconfig|  5 -
 install/updates/20-syncrepl.update |  9 +
 install/updates/Makefile.am|  1 +
 ipaserver/install/bindinstance.py  | 13 +
 5 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 install/updates/20-syncrepl.update

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 35b87148c1074ae7e1e8909e981d3473c4a46258..97c47983106be0a2b04a121636b628b032721427 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -745,6 +745,7 @@ fi
 %{_mandir}/man1/ipa-backup.1.gz
 %{_mandir}/man1/ipa-restore.1.gz
 %{_mandir}/man1/ipa-advise.1.gz
+%ghost %{_localstatedir}/named/ipa
 
 %files server-trust-ad
 %{_sbindir}/ipa-adtrust-install
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 10526f226798c78ae75972b82a2f72b200a8aacf..9b1cc91f2570c9359d14814184135d214ca73001 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -1079,6 +1079,10 @@ def main():
 setup_firefox_extension(fstore)
 add_ca_dns_records()
 
+bind = bindinstance.BindInstance(fstore)
+if bind.is_configured():
+bind.create_dir('/var/named/ipa', 0700)
+
 # Any of the following functions returns True iff the named.conf file
 # has been altered
 named_conf_changes = (
@@ -1092,7 +1096,6 @@ def main():
 if any(named_conf_changes):
 # configuration has changed, restart the name server
 root_logger.info('Changes to named.conf have been made, restart named')
-bind = bindinstance.BindInstance(fstore)
 try:
 bind.restart()
 except ipautil.CalledProcessError, e:
diff --git a/install/updates/20-syncrepl.update b/install/updates/20-syncrepl.update
new file mode 100644
index ..6901370f9cb302ff2c0c8bcc3f7b51aadef83e8e
--- /dev/null
+++ b/install/updates/20-syncrepl.update
@@ -0,0 +1,9 @@
+# Enable Retro changelog
+dn: cn=Retro Changelog Plugin,cn=plugins,cn=config
+only:nsslapd-pluginEnabled: on
+add:nsslapd-attribute: nsuniqueid:targetUniqueId
+add:nsslapd-changelogmaxage: nsslapd-changelogmaxage: 2d
+
+# Enable SyncRepl
+dn: cn=Content Synchronization,cn=plugins,cn=config
+only:nsslapd-pluginEnabled: on
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index 66f0cd57617b6902fd4a74a8e7ac986f29babf20..67c33eef5ef31efffd7d3940a45f04bbf31927e9 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -14,6 +14,7 @@ app_DATA =\
 	20-indices.update		\
 	20-nss_ldap.update		\
 	20-replication.update		\
+	20-syncrepl.update		\
 	20-user_private_groups.update	\
 	20-winsync_index.update		\
 	21-replicas_container.update	\
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 6d5a1d44d30c89278c24fe7ab5278355cb65b0b4..4baeb4e077c64a7abebd1c071012f6c1e02dc1ae 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -22,6 +22,7 @@
 import pwd
 import netaddr
 import re
+import errno
 
 import ldap
 
@@ -509,6 +510,16 @@ def create_sample_bind_zone(self):
 os.close(bind_fd)
 print "Sample zone file for bind has been created in "+bind_name
 
+def create_dir(self, path, mode):
+try:
+os.makedirs(path, mode)
+  

[Freeipa-devel] [PATCH] 0086 Make Expression field required when adding automember condition

2013-11-26 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/4053.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 32fe5becfa530e629602433e0942fccb1a06e677 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Tue, 26 Nov 2013 12:09:44 +0100
Subject: [PATCH] Make Expression field required when adding automember
 condition

https://fedorahosted.org/freeipa/ticket/4053
---
 install/ui/src/freeipa/automember.js | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/install/ui/src/freeipa/automember.js b/install/ui/src/freeipa/automember.js
index 2dff34227351c582d028a43bc71bc2fe6a7d3142..c18f619e5b06bf9962b687689c9ab013fc6797c1 100644
--- a/install/ui/src/freeipa/automember.js
+++ b/install/ui/src/freeipa/automember.js
@@ -285,7 +285,8 @@ IPA.automember.rule_details_facet = function(spec) {
 },
 {
 name: 'automemberinclusiveregex',
-label: '@i18n:objects.automember.expression'
+label: '@i18n:objects.automember.expression',
+required: true
 }
 ]
 }
@@ -314,7 +315,8 @@ IPA.automember.rule_details_facet = function(spec) {
 },
 {
 name: 'automemberexclusiveregex',
-label: '@i18n:objects.automember.expression'
+label: '@i18n:objects.automember.expression',
+required: true
 }
 ]
 }
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption

2013-11-25 Thread Ana Krivokapic
On 11/15/2013 02:26 PM, Petr Vobornik wrote:
> Hello list,
>
> this is a first part of RCUE adoption effort. Main themes of this patch set 
> are:
>
> - use RCUE navigation <https://fedorahosted.org/freeipa/ticket/3902>
> - new styles for textboxes, textareas, radio/checkbox buttons and buttons-
> part of <https://fedorahosted.org/freeipa/ticket/3904>
> - new internal form layout (tables replaced by divs)
> - layout does not have fixed size 
> <https://fedorahosted.org/freeipa/ticket/3435>
> - new dialog styles + removed dependency on jQuery UI dialog
> - icons replaced by Font Awesome glyphs
>
> Example is at: <http://pvoborni.fedorapeople.org/rcue/>
>
> Some reasonings and additional info:
>
> 1. RCUE includes Bootstrap which defines o lot of styles for a lot of things.
> That messed up the UI and therefore I did the form changes now.
>
> 2. jQuery UI is pretty big lib and we used it only for dialog and buttons.
> Buttons were replaced by RCUE buttons so removal of dialog dependency was a
> obvious step to get rid of the whole lib. The lib is removed from main UI but
> is still present in separate pages - will be removed later.
>
> 3. Dojo and jQuery were upgraded to latest
> versions.<https://fedorahosted.org/freeipa/ticket/2811>
>
> This approach was ACKed by Kyle from a design perspective with a note that we
> will review and fixed some styling after second phase. We should not release
> until then.
>
> The second phase, which I'm working on right now, will consist of:
>  * login screen <https://fedorahosted.org/freeipa/ticket/3903>
>  * new styles for standalone pages
>  * necessary responsive enhancement (the ultimate future goal is responsive
> layout)
>
> It's quite a lot of patches so I did not attach them here. You can see the
> code in my private repo:  branch
> 'rcue'.

I tested this phase of RCUE adoption effort, overall it looks and works great. A
couple of findings:

1) Two ui integration tests are failing, I guess this is due to the re-arranging
of elements on the automember page. So the tests should be amended to reflect
that change.

2) In the 'Add Host' dialog, validation error messages do not have the proper
styles. E.g. if you don't fill any values, and just click 'Add', 'Required
field' messages appear, but they are not red, and the related fields do not get
a red frame.


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES] 0276-0277 Break long doc strings for translations

2013-11-20 Thread Ana Krivokapic
On 10/09/2013 04:11 PM, Petr Viktorin wrote:
> On 09/16/2013 05:13 PM, Petr Viktorin wrote:
>> Hello,
>> The first patch allow concatenating LazyText objects using `+`. This
>> means we can break up long docstrings into multiple parts. Translators
>> can then work on each part separately, and the whole translation is not
>> lost when a small part is changed or added.
>>
>> The second patch splits up the docstring for Host*. I chose Host because
>> it was updated since last release, so translators would have
>> to-retranslate the text. In the future, whenever a long docstring is
>> changed it should be broken up like this.
>> The patch also updates translations, and adds the broken-up texts for
>> French and Ukraininan. You can test by viewing Host help in one of these
>> languages -- only the new section should be untranslated.
>
> Rebased to current master.
>
> I've added a patch to update translations, please apply it before the other
> two. It makes the functional changes a bit clearer.
>
> Test with:
> $ LANG=fr_FR ipa help host
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Looks good to me. ACK.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 111] ipa-client-install: Publish CA certificate to systemwide store

2013-11-20 Thread Ana Krivokapic
On 11/18/2013 01:54 PM, Tomas Babej wrote:
> On 11/15/2013 03:36 PM, Rob Crittenden wrote:
>> Tomas Babej wrote:
>>> On 11/15/2013 02:46 PM, Ana Krivokapic wrote:
>>>> On 11/13/2013 02:57 PM, Tomas Babej wrote:
>>>>> On 09/27/2013 10:14 AM, Martin Kosek wrote:
>>>>>> On 09/26/2013 04:46 PM, Jan Cholasta wrote:
>>>>>>> On 26.9.2013 12:59, Tomas Babej wrote:
>>>>>>>> On 09/26/2013 12:54 PM, Jan Cholasta wrote:
>>>>>>>>> On 24.9.2013 18:14, Nalin Dahyabhai wrote:
>>>>>>>>>> On Tue, Sep 24, 2013 at 01:30:10PM +0200, Jan Cholasta wrote:
>>>>>>>>>>> We discussed this with Tomáš off-line and it turns out that
>>>>>>>>>>> ipa-client-install fails if the CA cert is not added to
>>>>>>>>>>> /etc/pki/nssdb.
>>>>>>>>>>>
>>>>>>>>>>> However, according to p11-kit docs it should work:
>>>>>>>>>>> <http://p11-glue.freedesktop.org/doc/p11-kit/trust-nss.html>. I
>>>>>>>>>>> wonder what needs to be done to make it work in IPA...
>>>>>>>>>>
>>>>>>>>>> On my system, there's no symlink to libnssckbi.so (or the right
>>>>>>>>>> location
>>>>>>>>>> in the link farm under /etc/alternatives) in /etc/pki/nssdb, so
>>>>>>>>>> that
>>>>>>>>>> database isn't going to automatically pull in the list of
>>>>>>>>>> trusted CAs
>>>>>>>>>> that p11-kit maintains.
>>>>>>>>>>
>>>>>>>>>> Whether the database under /etc/pki/nssdb should automatically
>>>>>>>>>> include
>>>>>>>>>> the usual set of trust anchors is probably a different
>>>>>>>>>> conversation.
>>>>>>>>>
>>>>>>>>> Thanks for the info.
>>>>>>>>>
>>>>>>>>> Tomáš, the patch is fine then. I have one more nitpick though:
>>>>>>>>> why did
>>>>>>>>> you change "the default NSS database" to "the NSS database"? The
>>>>>>>>> database in /etc/pki/nssdb *is* the default NSS database, so please
>>>>>>>>> change it back. Also I think "systemwide CA trust database" is
>>>>>>>>> better
>>>>>>>>> than "systemwide CA store".
>>>>>>>>>
>>>>>>>>> Honza
>>>>>>>>>
>>>>>>>> I fixed the descriptions. Updated patch attached.
>>>>>>>>
>>>>>>>> Tomas
>>>>>>>>
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>> There's one more thing: we should probably check if
>>>>>>> /usr/bin/update-ca-trust
>>>>>>> exists before using it, for the sake of cross-distro compatibility.
>>>>>>>
>>>>>>
>>>>>> Right. I am also thinking if this functionality should not be
>>>>>> somehow integrated into the platform files so that it can be
>>>>>> overriden in platforms that do not have the systemwide storage.
>>>>>>
>>>>>> Martin
>>>>>
>>>>> Updated patch attached, requires my patch 130.
>>>>>
>>>>>
>>>>>
>>>>> ___
>>>>> Freeipa-devel mailing list
>>>>> Freeipa-devel@redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>
>>>> The patch works fine; a couple of nitpicks:
>>>>
>>>> 1) The import of root_logger in services.py.in is unused.
>>>>
>>>> 2) In ipa-client-install, you log the return values of functions
>>>> insert_ca_cert_into_systemwide_ca_store() and
>>>> remove_ca_cert_from_systemwide_ca_store(). But these functions do not
>>>> return any values, so you will always be logging `None`.
>>>>
>>> Thanks for the review,
>>>
>>> I removed the code (it was meant for debugging purposes only).
>>>
>>> Updated patch attached.
>>
>> Adding the CA to the NSS cert database is considered a fatal error. Should
>> adding it to the global trust database be fatal as well?
>>
>> I don't know the answer, but if we want to do this at some point should these
>> functions return True/False to denote success/failure?
>>
>> rob
>
> I don't think it should be considered fatal, at least not now.
>
> I updated the patch to return the success/failure status, even though, this
> could be done when it will be required. But doesn't hurt anything either, at
> least other platform files will develop systemwide CA store functions with
> this approach in mind.
>
> Updated patch attached.
>

Looks good, ACK.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts

2013-11-19 Thread Ana Krivokapic
On 11/19/2013 12:52 PM, Ana Krivokapic wrote:
> On 11/14/2013 10:04 AM, Petr Vobornik wrote:
>> On 11/13/2013 01:33 PM, Ana Krivokapic wrote:
>>> On 11/12/2013 01:27 PM, Ana Krivokapic wrote:
>>>> On 10/30/2013 09:56 PM, Martin Kosek wrote:
>>>>> - Original Message -----
>>>>>> From: "Simo Sorce" 
>>>>>> To: "Ana Krivokapic" 
>>>>>> Cc: "Martin Kosek" , "freeipa-devel"
>>>>>> 
>>>>>> Sent: Wednesday, October 30, 2013 7:11:20 PM
>>>>>> Subject: Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes
>>>>>> for users and hosts
>>>>>>
>>>>>> On Wed, 2013-10-30 at 19:01 +0100, Ana Krivokapic wrote:
>>>>>>> On 10/29/2013 02:04 PM, Simo Sorce wrote:
>>>>>>>> On Tue, 2013-10-29 at 12:42 +0100, Martin Kosek wrote:
>>>>>>>>> On 10/29/2013 10:49 AM, Ana Krivokapic wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> Patch 0080 adds userClass attribute for users to IPA CLI.
>>>>>>>>>> Patch 0081 adds userClass attribute for users and hosts to the web 
>>>>>>>>>> UI.
>>>>>>>>>>
>>>>>>>>>> Design page:
>>>>>>>>>> http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
>>>>>>>>>>
>>>>>>>>>> Tickets:
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3588
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3590
>>>>>>>>> NACK to just extending posixAccount objectclass. This is a standard
>>>>>>>>> objectclass
>>>>>>>>> defined by RFC 2307 and we cannot just simply extend and overwrite it 
>>>>>>>>> as
>>>>>>>>> we wish.
>>>>>>>> Uhh indeed this is a big No-no.
>>>>>>>>
>>>>>>>>> We will need to come up with some custom objectclass, like ipaUser. 
>>>>>>>>> This
>>>>>>>>> is the
>>>>>>>>> reason why I wrote to ticket "A second goal of this ticket is to 
>>>>>>>>> review
>>>>>>>>> current
>>>>>>>>> objectClass hierarchy of users and do changes if needed." so that we 
>>>>>>>>> can
>>>>>>>>> pick
>>>>>>>>> the best option where to place it.
>>>>>>>> userClass is used in ipaHost, so I guess it could be instead add to an
>>>>>>>> ipa objectclass. ipaObject might be used perhaps, otherwise we'll need 
>>>>>>>> a
>>>>>>>> new ipaUser objectlass.
>>>>>>>>
>>>>>>>> Simo.
>>>>>>>>
>>>>>>> If there are no objections to using the ipaObject objectclass, the 
>>>>>>> attached
>>>>>>> patches implement this approach.
>>>>>> After some thinking ipaObject is more generic than just users, not sure
>>>>>> that attaching userClass there is appropriate. I think we really need
>>>>>> ipaUser at this point.
>>>>> +1. I also do not think that ipaObject is the right OC to place the
>>>>> attribute, it is just too general.
>>>>>
>>>>> Let's go with the ipaUser objectClass, looking something like that:
>>>>>
>>>>> (  NAME 'ipaUser' AUXILIARY MUST ( uid ) MAY ( userClass ) X-ORIGIN
>>>>> 'IPA v3' )
>>>>>
>>>>> We will need to add the OC when needed, we cannot just add it to default
>>>>> list. Ideally, we could also implement
>>>>> https://fedorahosted.org/freeipa/ticket/3922
>>>>> in scope of this effort as this need to add additional OCs is piling up.
>>>>>
>>>>> Martin
>>>> This implementation introduces a new objectclass 'ipaUser'.
>>>>
>>> The web UI patch needed an update as well, as we need to allow writing the
>>> userClass attribute even when the ipaUser objectclass is not (yet) set on 
>>> the
>>> user obje

Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts

2013-11-19 Thread Ana Krivokapic
On 11/14/2013 10:04 AM, Petr Vobornik wrote:
> On 11/13/2013 01:33 PM, Ana Krivokapic wrote:
>> On 11/12/2013 01:27 PM, Ana Krivokapic wrote:
>>> On 10/30/2013 09:56 PM, Martin Kosek wrote:
>>>> - Original Message -
>>>>> From: "Simo Sorce" 
>>>>> To: "Ana Krivokapic" 
>>>>> Cc: "Martin Kosek" , "freeipa-devel"
>>>>> 
>>>>> Sent: Wednesday, October 30, 2013 7:11:20 PM
>>>>> Subject: Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes
>>>>> for users and hosts
>>>>>
>>>>> On Wed, 2013-10-30 at 19:01 +0100, Ana Krivokapic wrote:
>>>>>> On 10/29/2013 02:04 PM, Simo Sorce wrote:
>>>>>>> On Tue, 2013-10-29 at 12:42 +0100, Martin Kosek wrote:
>>>>>>>> On 10/29/2013 10:49 AM, Ana Krivokapic wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Patch 0080 adds userClass attribute for users to IPA CLI.
>>>>>>>>> Patch 0081 adds userClass attribute for users and hosts to the web UI.
>>>>>>>>>
>>>>>>>>> Design page:
>>>>>>>>> http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
>>>>>>>>>
>>>>>>>>> Tickets:
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3588
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3590
>>>>>>>> NACK to just extending posixAccount objectclass. This is a standard
>>>>>>>> objectclass
>>>>>>>> defined by RFC 2307 and we cannot just simply extend and overwrite it 
>>>>>>>> as
>>>>>>>> we wish.
>>>>>>> Uhh indeed this is a big No-no.
>>>>>>>
>>>>>>>> We will need to come up with some custom objectclass, like ipaUser. 
>>>>>>>> This
>>>>>>>> is the
>>>>>>>> reason why I wrote to ticket "A second goal of this ticket is to review
>>>>>>>> current
>>>>>>>> objectClass hierarchy of users and do changes if needed." so that we 
>>>>>>>> can
>>>>>>>> pick
>>>>>>>> the best option where to place it.
>>>>>>> userClass is used in ipaHost, so I guess it could be instead add to an
>>>>>>> ipa objectclass. ipaObject might be used perhaps, otherwise we'll need a
>>>>>>> new ipaUser objectlass.
>>>>>>>
>>>>>>> Simo.
>>>>>>>
>>>>>> If there are no objections to using the ipaObject objectclass, the 
>>>>>> attached
>>>>>> patches implement this approach.
>>>>> After some thinking ipaObject is more generic than just users, not sure
>>>>> that attaching userClass there is appropriate. I think we really need
>>>>> ipaUser at this point.
>>>> +1. I also do not think that ipaObject is the right OC to place the
>>>> attribute, it is just too general.
>>>>
>>>> Let's go with the ipaUser objectClass, looking something like that:
>>>>
>>>> (  NAME 'ipaUser' AUXILIARY MUST ( uid ) MAY ( userClass ) X-ORIGIN
>>>> 'IPA v3' )
>>>>
>>>> We will need to add the OC when needed, we cannot just add it to default
>>>> list. Ideally, we could also implement
>>>> https://fedorahosted.org/freeipa/ticket/3922
>>>> in scope of this effort as this need to add additional OCs is piling up.
>>>>
>>>> Martin
>>> This implementation introduces a new objectclass 'ipaUser'.
>>>
>> The web UI patch needed an update as well, as we need to allow writing the
>> userClass attribute even when the ipaUser objectclass is not (yet) set on the
>> user object. Thanks Petr for pointing it out.
>>
>> Attaching both patches again (the CLI patch has not changed since the last
>> iteration).
>>
>
> ACK for Web UI patch (81-03)
>
> 80-03 seems good to me as well, but I don't feel strong about my ability to
> judge correctness of ldap/ipalib part so I let somebody else to look at it.

Patch 80 rebased.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

Re: [Freeipa-devel] [PATCHES] 0258-0265 Add schema updater based on IPA schema files

2013-11-18 Thread Ana Krivokapic
On 11/15/2013 05:28 PM, Petr Viktorin wrote:
> On 11/15/2013 02:09 PM, Petr Viktorin wrote:
>> On 11/11/2013 04:18 PM, Ana Krivokapic wrote:
>>> On 11/11/2013 02:53 PM, Ana Krivokapic wrote:
>>>> On 11/11/2013 12:32 PM, Petr Viktorin wrote:
>>>>> On 11/07/2013 02:34 PM, Ana Krivokapic wrote:
>>>>>> On 11/01/2013 03:26 PM, Petr Viktorin wrote:
>>>>>>> On 09/13/2013 06:44 PM, Petr Viktorin wrote:
>>>>>>>> On 08/01/2013 04:52 PM, Petr Viktorin wrote:
>>>>>>>>> Hello,
>>>>>>>>> With these patches, schema updates will be based on the ldif
>>>>>>>>> files we
>>>>>>>>> use for installation.
>>>>>>>>>
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3454
>>>>>>>>>
>>>>>>>>> This is a RFE, here is the design doc:
>>>>>>>>> http://www.freeipa.org/page/V3/Improved_schema_updater
>>>>>>>>>
>>>>>>>> I found and filed a bug in python-ldap[0]: it sometimes ignores
>>>>>>>> parts of
>>>>>>>> schema LDIFs when parsing them.
>>>>>>>> Patch 0275 works around the bug. Please apply on top of 0258-0265
>>>>>>>> (they
>>>>>>>> still apply cleanly).
>>>>>>>>
>>>>>>>>
>>>>>>>> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820
>>>>>>>>
>>>>>>> The recent ipaldap patches resulted in a small conflict. Attaching
>>>>>>> rebased patches.
>>>>>> I have tested the patches and overall they seem to work fine. Some
>>>>>> questions/comments are below.
>>>>>>
>>>>>>
>>>>>> Patch 258:
>>>>>> You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is
>>>>>> called from `__init__()`, so no need to catch it again in
>>>>>> `__init__()`.
>>>>> I've added a comment instead of the try/catch
>>>>>
>>>>>> Patch 259:
>>>>>> ACK
>>>>>>
>>>>>> Patch 260:
>>>>>>
>>>>>>   >   # Usually the modlist order does not matter.
>>>>>>   >   # However, for schema updates, we want 'attributetypes'
>>>>>> before
>>>>>>   >   # 'objectclasses'.
>>>>>>   >   # A simple sort will ensure this.
>>>>>>   >   modlist.sort()
>>>>>>
>>>>>> Since `modlist` is a list of tuples, it is sorted by the first
>>>>>> elements
>>>>>> in the tuples, then by the seconds elements, etc. Which means the
>>>>>> resulting list will be sorted by the modification type first
>>>>>> (`MOD_ADD`,
>>>>>> `MOD_DELETE`, etc), and by `attributetypes`/`objectclasses` second.
>>>>>> Was
>>>>>> this the desired effect?
>>>>> I've added a sort key; it should be safer now.
>>>> Hmm, the key you added still retains the default sorting behavior -
>>>> it will sort
>>>> by the first elements of the tuples first. Since we need sorting by
>>>> the second
>>>> elements, I think the key here should be: key=lambda m: m[1].lower()
>>
>> Right, I see what you mean.
>> I've made the change and tested it a bit.
>>
>>>>>> Patch 261:
>>>>>> Man page updates look good, but several options in the man page have 3
>>>>>> dashes in the long form instead of 2. I have attached a mini-patch
>>>>>> that
>>>>>> fixes this along with a couple of typos in the man page. Feel free to
>>>>>> squash it to your patch 261.
>>>>> Nice catch! Squashed.
>>>>>
>>>>>> Patch 262:
>>>>>> Whitespace warnings.
>>>>> I didn't see any with my settings; could you be more specific?
>>>> $ git am
>>>> ~/freeipa-pviktori-0262.3-Remove-schema-modifications-from-update-files.patch
>>>>
>>>> Applying: Remove schema modifications from update files
>>>> /home/ana/freeipa/.git/rebase-apply/patch:497: new blank line at EOF.
>>>> +
>>>&

Re: [Freeipa-devel] [PATCH] 0084 Make sure state of services is preserved after client uninstall

2013-11-18 Thread Ana Krivokapic
On 11/15/2013 05:03 PM, Tomas Babej wrote:
> On 11/07/2013 05:25 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3790.
>>
>>
>>
>> ___
>> Freeipa-devel mailing list
>> Freeipa-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
> Looking good..
>
> I have two questions:
>
>
> 1.) Nitpick: I'd suggest we rename the save_state(service) and
> restore_state(service) to more descriptive
> save_service_state/restore_service_state?
>

Well, if the argument you are passing to these function does not have a name
which suggests it is a service (which it should have anyway), you can do:
`save_state(service=x)`. So I don't think `save_service_state(x)` is more 
readable.

> 2.) There are other places in ipa-client-install where we save and restore the
> state of the service. Having abstracted that into a function, should we use
> this at other places as well?
>
>

I looked at other instances in ipa-client-install where service states are saved
and restored. It seems that each of these cases includes some custom logic which
does not make it possible to use the two functions I've added.

>
> -- 
> Tomas Babej
> Associate Software Engeneer | Red Hat | Identity Management
> RHCE | Brno Site | IRC: tbabej | freeipa.org


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 111] ipa-client-install: Publish CA certificate to systemwide store

2013-11-15 Thread Ana Krivokapic
On 11/13/2013 02:57 PM, Tomas Babej wrote:
> On 09/27/2013 10:14 AM, Martin Kosek wrote:
>> On 09/26/2013 04:46 PM, Jan Cholasta wrote:
>>> On 26.9.2013 12:59, Tomas Babej wrote:
>>>> On 09/26/2013 12:54 PM, Jan Cholasta wrote:
>>>>> On 24.9.2013 18:14, Nalin Dahyabhai wrote:
>>>>>> On Tue, Sep 24, 2013 at 01:30:10PM +0200, Jan Cholasta wrote:
>>>>>>> We discussed this with Tomás( off-line and it turns out that
>>>>>>> ipa-client-install fails if the CA cert is not added to
>>>>>>> /etc/pki/nssdb.
>>>>>>>
>>>>>>> However, according to p11-kit docs it should work:
>>>>>>> <http://p11-glue.freedesktop.org/doc/p11-kit/trust-nss.html>. I
>>>>>>> wonder what needs to be done to make it work in IPA...
>>>>>>
>>>>>> On my system, there's no symlink to libnssckbi.so (or the right location
>>>>>> in the link farm under /etc/alternatives) in /etc/pki/nssdb, so that
>>>>>> database isn't going to automatically pull in the list of trusted CAs
>>>>>> that p11-kit maintains.
>>>>>>
>>>>>> Whether the database under /etc/pki/nssdb should automatically include
>>>>>> the usual set of trust anchors is probably a different conversation.
>>>>>
>>>>> Thanks for the info.
>>>>>
>>>>> Tomás(, the patch is fine then. I have one more nitpick though: why did
>>>>> you change "the default NSS database" to "the NSS database"? The
>>>>> database in /etc/pki/nssdb *is* the default NSS database, so please
>>>>> change it back. Also I think "systemwide CA trust database" is better
>>>>> than "systemwide CA store".
>>>>>
>>>>> Honza
>>>>>
>>>> I fixed the descriptions. Updated patch attached.
>>>>
>>>> Tomas
>>>>
>>>
>>> Thanks.
>>>
>>> There's one more thing: we should probably check if /usr/bin/update-ca-trust
>>> exists before using it, for the sake of cross-distro compatibility.
>>>
>>
>> Right. I am also thinking if this functionality should not be somehow
>> integrated into the platform files so that it can be overriden in platforms
>> that do not have the systemwide storage.
>>
>> Martin
>
> Updated patch attached, requires my patch 130.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

The patch works fine; a couple of nitpicks:

1) The import of root_logger in services.py.in is unused.

2) In ipa-client-install, you log the return values of functions
insert_ca_cert_into_systemwide_ca_store() and
remove_ca_cert_from_systemwide_ca_store(). But these functions do not return any
values, so you will always be logging `None`.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0130] platform: Add Fedora 19 platform file

2013-11-15 Thread Ana Krivokapic
On 11/13/2013 02:56 PM, Tomas Babej wrote:
> Hi,
>
> Part of: https://fedorahosted.org/freeipa/ticket/3504
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 0068-0070 Automember rebuild membership

2013-11-14 Thread Ana Krivokapic
On 11/14/2013 03:11 PM, Martin Kosek wrote:
> On 11/13/2013 04:56 PM, Ana Krivokapic wrote:
>> On 11/13/2013 03:08 PM, Martin Kosek wrote:
>>> On 10/29/2013 12:30 PM, Ana Krivokapic wrote:
>>>> On 10/15/2013 06:09 PM, Ana Krivokapic wrote:
>>>>> On 09/30/2013 10:02 AM, Petr Viktorin wrote:
>>>>>> On 09/27/2013 03:12 PM, Martin Kosek wrote:
>>>>>>> On 09/27/2013 03:00 PM, Jan Cholasta wrote:
>>>>>>>> On 23.9.2013 19:41, Ana Krivokapic wrote:
>>>>>>>>> On 09/19/2013 03:29 PM, Ana Krivokapic wrote:
>>>>>>> ...
>>>>>>>> Patch 69:
>>>>>>>>
>>>>>>>> I think the changes in the update file should be also done in the
>>>>>>>> right LDIF
>>>>>>>> files in install/share, though I don't know what is the recent
>>>>>>>> consensus on this.
>>>>>>>>
>>>>>>>>
>>>>>>>> Honza
>>>>>>>>
>>>>>>> Last time I checked, we used to do the change both in LDIF and update
>>>>>>> file. Just to avoid the LDIF become obsolete.
>>>>>>>
>>>>>>> Martin
>>>>>> Rob recently said his preference is to move everything from LDIF to 
>>>>>> updates,
>>>>>> and out of the the LDIF files:
>>>>>> http://www.redhat.com/archives/freeipa-devel/2013-September/msg00106.html
>>>>>>
>>>>>> I would agree, having two places with the same information is redundant 
>>>>>> and
>>>>>> error-prone.
>>>>>>
>>>>> Thanks Honza for the review.
>>>>>
>>>>> I incorporated your suggestions in this updated patchset. I attached all 
>>>>> the
>>>>> patches for more convenient reviewing, but only patches 68 and 70 have 
>>>>> changed.
>>>>>
>>>>> I haven't done any changes in the LDIF files since the consensus seems to 
>>>>> be not
>>>>> to do that.
>>>> Patch 70 needed a rebase, attaching the whole patchset again.
>>> This works pretty fine, I have few comments though:
>>>
>>> 1) 0068: the task should be run only for users/hosts base DN - this is 
>>> where we
>>> confine our automember and I think admin may be surprised that the rebuild 
>>> call
>>> is does not respect it.
>> Fixed.
>>
>>> 2) 0068: I am missing some examples for automember-rebuild in the help. At
>>> least for running rebuild for all users/hosts and for running it for 
>>> specified
>>> user/host.
>> I added some examples, as well as a general description of the new command.
>>
>>> 3) 0068: I think that the labels/doc for the new command/options should be
>>> improved. It is not obvious, that automember-rebuild can run for all
>>> users/hosts, at least from following doc:
>>>
>>> # ipa help automember
>>> ...
>>>   automember-rebuild   Rebuild auto membership for specified 
>>> entries.
>>> ...
>>>
>>> Maybe we should remove the "for specified entries" part?
>>>
>>> As for the options, we now have this:
>>>
>>> # ipa help automember-rebuild
>>> Usage: ipa [global-options] automember-rebuild [options]
>>>
>>> Rebuild auto membership for specified entries.
>>> Options:
>>>   -h, --helpshow this help message and exit
>>>   --type=['group', 'hostgroup']
>>> Grouping to which the rule applies  <--completely 
>>> stray
>>>   --users=STR   Users for which the rebuild task will be run
>>>   --hosts=STR   Hosts for which the rebuild task will be run
>>>
>>>
>>> We should probably also do not mention specified entries here.
>>>
>>> As for option help, maybe the following would better show that it can be run
>>> for all entries?
>>>
>>>   --type=['group', 'hostgroup']
>>> Rebuild membership for all members of a grouping
>>>   --users=STR   Rebuild membership for specified users
>>>   --hosts=STR   Rebuild membership for specified hosts
>> Agreed, labels fixed as per your suggestions.
>>
>>> This 

Re: [Freeipa-devel] [PATCH 0113] ipa-client: Set NIS domain name in the installer

2013-11-14 Thread Ana Krivokapic
On 09/26/2013 10:28 AM, Tomas Babej wrote:
> +if options.no_nisdomain and not options.nisdomain:
This should be `if options.no_nisdomain and options.nisdomain:`.
> +parser.error("--no-nisdomain cannot be used together with 
> --nisdomain")


Shouldn't we also revert the nisdomain authconfig setting on client uninstall?

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES] 0068-0070 Automember rebuild membership

2013-11-13 Thread Ana Krivokapic
On 11/13/2013 03:08 PM, Martin Kosek wrote:
> On 10/29/2013 12:30 PM, Ana Krivokapic wrote:
>> On 10/15/2013 06:09 PM, Ana Krivokapic wrote:
>>> On 09/30/2013 10:02 AM, Petr Viktorin wrote:
>>>> On 09/27/2013 03:12 PM, Martin Kosek wrote:
>>>>> On 09/27/2013 03:00 PM, Jan Cholasta wrote:
>>>>>> On 23.9.2013 19:41, Ana Krivokapic wrote:
>>>>>>> On 09/19/2013 03:29 PM, Ana Krivokapic wrote:
>>>>> ...
>>>>>> Patch 69:
>>>>>>
>>>>>> I think the changes in the update file should be also done in the
>>>>>> right LDIF
>>>>>> files in install/share, though I don't know what is the recent
>>>>>> consensus on this.
>>>>>>
>>>>>>
>>>>>> Honza
>>>>>>
>>>>> Last time I checked, we used to do the change both in LDIF and update
>>>>> file. Just to avoid the LDIF become obsolete.
>>>>>
>>>>> Martin
>>>> Rob recently said his preference is to move everything from LDIF to 
>>>> updates,
>>>> and out of the the LDIF files:
>>>> http://www.redhat.com/archives/freeipa-devel/2013-September/msg00106.html
>>>>
>>>> I would agree, having two places with the same information is redundant and
>>>> error-prone.
>>>>
>>> Thanks Honza for the review.
>>>
>>> I incorporated your suggestions in this updated patchset. I attached all the
>>> patches for more convenient reviewing, but only patches 68 and 70 have 
>>> changed.
>>>
>>> I haven't done any changes in the LDIF files since the consensus seems to 
>>> be not
>>> to do that.
>> Patch 70 needed a rebase, attaching the whole patchset again.
> This works pretty fine, I have few comments though:
>
> 1) 0068: the task should be run only for users/hosts base DN - this is where 
> we
> confine our automember and I think admin may be surprised that the rebuild 
> call
> is does not respect it.

Fixed.

>
> 2) 0068: I am missing some examples for automember-rebuild in the help. At
> least for running rebuild for all users/hosts and for running it for specified
> user/host.

I added some examples, as well as a general description of the new command.

>
> 3) 0068: I think that the labels/doc for the new command/options should be
> improved. It is not obvious, that automember-rebuild can run for all
> users/hosts, at least from following doc:
>
> # ipa help automember
> ...
>   automember-rebuild   Rebuild auto membership for specified 
> entries.
> ...
>
> Maybe we should remove the "for specified entries" part?
>
> As for the options, we now have this:
>
> # ipa help automember-rebuild
> Usage: ipa [global-options] automember-rebuild [options]
>
> Rebuild auto membership for specified entries.
> Options:
>   -h, --helpshow this help message and exit
>   --type=['group', 'hostgroup']
> Grouping to which the rule applies  <--completely 
> stray
>   --users=STR   Users for which the rebuild task will be run
>   --hosts=STR   Hosts for which the rebuild task will be run
>
>
> We should probably also do not mention specified entries here.
>
> As for option help, maybe the following would better show that it can be run
> for all entries?
>
>   --type=['group', 'hostgroup']
> Rebuild membership for all members of a grouping
>   --users=STR   Rebuild membership for specified users
>   --hosts=STR   Rebuild membership for specified hosts

Agreed, labels fixed as per your suggestions.

>
> This makes me thinking we may want to forbid entering both --type and
> --users/--hosts - i.e. either rebuild all or just selected ones - to make the
> selection even more clear. But I am open to discussion on this one.

Validation prevents any invalid combination of options (e.g. --type=group and
--hosts used together, or --type=hostgroup and --users used together). If, for
example, --users is specified, then --type=group is allowed but not required. I
think it's clear enough.

>
> 4) 0069: Add Automember Export Updates Task is currently redundant. I think we
> should either have permissions for all 3 possible tasks or for just the one 
> we use.

I removed the unused permission.

>
> 5) 0069: permissions should be of SYSTEM type as the ACI is out of SUFFIX, so
> that user does not try to modify them (will be able to in future versions).
> Adding Petr

Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts

2013-11-13 Thread Ana Krivokapic
On 11/12/2013 01:27 PM, Ana Krivokapic wrote:
> On 10/30/2013 09:56 PM, Martin Kosek wrote:
>> - Original Message -
>>> From: "Simo Sorce" 
>>> To: "Ana Krivokapic" 
>>> Cc: "Martin Kosek" , "freeipa-devel" 
>>> 
>>> Sent: Wednesday, October 30, 2013 7:11:20 PM
>>> Subject: Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes 
>>> for users and hosts
>>>
>>> On Wed, 2013-10-30 at 19:01 +0100, Ana Krivokapic wrote:
>>>> On 10/29/2013 02:04 PM, Simo Sorce wrote:
>>>>> On Tue, 2013-10-29 at 12:42 +0100, Martin Kosek wrote:
>>>>>> On 10/29/2013 10:49 AM, Ana Krivokapic wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> Patch 0080 adds userClass attribute for users to IPA CLI.
>>>>>>> Patch 0081 adds userClass attribute for users and hosts to the web UI.
>>>>>>>
>>>>>>> Design page:
>>>>>>> http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
>>>>>>>
>>>>>>> Tickets:
>>>>>>> https://fedorahosted.org/freeipa/ticket/3588
>>>>>>> https://fedorahosted.org/freeipa/ticket/3590
>>>>>> NACK to just extending posixAccount objectclass. This is a standard
>>>>>> objectclass
>>>>>> defined by RFC 2307 and we cannot just simply extend and overwrite it as
>>>>>> we wish.
>>>>> Uhh indeed this is a big No-no.
>>>>>
>>>>>> We will need to come up with some custom objectclass, like ipaUser. This
>>>>>> is the
>>>>>> reason why I wrote to ticket "A second goal of this ticket is to review
>>>>>> current
>>>>>> objectClass hierarchy of users and do changes if needed." so that we can
>>>>>> pick
>>>>>> the best option where to place it.
>>>>> userClass is used in ipaHost, so I guess it could be instead add to an
>>>>> ipa objectclass. ipaObject might be used perhaps, otherwise we'll need a
>>>>> new ipaUser objectlass.
>>>>>
>>>>> Simo.
>>>>>
>>>> If there are no objections to using the ipaObject objectclass, the attached
>>>> patches implement this approach.
>>> After some thinking ipaObject is more generic than just users, not sure
>>> that attaching userClass there is appropriate. I think we really need
>>> ipaUser at this point.
>> +1. I also do not think that ipaObject is the right OC to place the 
>> attribute, it is just too general.
>>
>> Let's go with the ipaUser objectClass, looking something like that:
>>
>> (  NAME 'ipaUser' AUXILIARY MUST ( uid ) MAY ( userClass ) X-ORIGIN 
>> 'IPA v3' )
>>
>> We will need to add the OC when needed, we cannot just add it to default 
>> list. Ideally, we could also implement
>> https://fedorahosted.org/freeipa/ticket/3922
>> in scope of this effort as this need to add additional OCs is piling up.
>>
>> Martin
> This implementation introduces a new objectclass 'ipaUser'.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

The web UI patch needed an update as well, as we need to allow writing the
userClass attribute even when the ipaUser objectclass is not (yet) set on the
user object. Thanks Petr for pointing it out.

Attaching both patches again (the CLI patch has not changed since the last
iteration).

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From ccc42e65ae9f6066e4427af82ddf283894cf0e4b Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 25 Oct 2013 16:31:50 +0200
Subject: [PATCH] WebUI: Add userClass attribute to user and host pages

Add userClass attribute to:
- user and host adder dialogs
- user and host detail facets

Design page: http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
https://fedorahosted.org/freeipa/ticket/3590
---
 install/ui/src/freeipa/host.js | 2 ++
 install/ui/src/freeipa/user.js | 9 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/install/ui/src/freeipa/host.js b/install/ui/src/freeipa/host.js
index f5007538e8ad1ea2e372c194b129f6c668d31b3e..460a19a9cda9a7d1d4457bb19f306dd88df8ceac 100644
--- a/install/ui/src/freeipa/host.js
+++ b/install/ui/src/freeipa/host.js
@@ -82,6 +82,7 @@ return {
  

Re: [Freeipa-devel] [PATCH] 0127 Map NT_STATUS_INVALID_PARAMETER to a most likely error cause

2013-11-12 Thread Ana Krivokapic
On 11/12/2013 04:55 PM, Alexander Bokovoy wrote:
> On Tue, 12 Nov 2013, Ana Krivokapic wrote:
>> On 11/12/2013 10:41 AM, Alexander Bokovoy wrote:
>>> +-1073741811: # NT_STATUS_INVALID_PARAMETER
>>> +error.RemoteRetrieveError(
>>   ^ should be "errors"
>>> +reason=_('AD domain controller complains about communication
>>> sequence. It may mean unsynchronized time on both sides, for example')),
>>
>> With this change, the patch works fine.
> What can you break in three lines? :)
>
> Thanks!
>
> Fixed and added the ticket number to commit message.
>

Thanks, ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0127 Map NT_STATUS_INVALID_PARAMETER to a most likely error cause

2013-11-12 Thread Ana Krivokapic
On 11/12/2013 10:41 AM, Alexander Bokovoy wrote:
> +-1073741811: # NT_STATUS_INVALID_PARAMETER
> +error.RemoteRetrieveError(
   ^ should be "errors"
> +reason=_('AD domain controller complains about communication 
> sequence. It may mean unsynchronized time on both sides, for example')),

With this change, the patch works fine.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 0085 Fix regression which prevents creating a winsync agreement

2013-11-12 Thread Ana Krivokapic
Hello,

This patch should fix the regression introduced by the original fix for ticket
https://fedorahosted.org/freeipa/ticket/3989.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 43309fd81f2ac2b46fb1908c79aef2d77fdf39ca Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Tue, 12 Nov 2013 14:50:57 +0100
Subject: [PATCH] Fix regression which prevents creating a winsync agreement

A regression, which prevented creation of a winsync agreement,
was introduced in the original fix for ticket #3989.

https://fedorahosted.org/freeipa/ticket/3989
---
 ipaserver/install/replication.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 1562382e16085e2d52688047502c0b2fa33ab458..4fa8cb8aa91e83c157f6a0c1cb98ee117cb97f41 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -626,8 +626,9 @@ def setup_agreement(self, a_conn, b_hostname, port=389,
 
 if iswinsync:
 self.setup_winsync_agmt(entry, win_subtree)
+else:
+entry['nsds5ReplicaStripAttrs'] = [" ".join(STRIP_ATTRS)]
 
-entry['nsds5ReplicaStripAttrs'] = [" ".join(STRIP_ATTRS)]
 a_conn.add_entry(entry)
 
 try:
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts

2013-11-12 Thread Ana Krivokapic
On 10/30/2013 09:56 PM, Martin Kosek wrote:
> - Original Message -
>> From: "Simo Sorce" 
>> To: "Ana Krivokapic" 
>> Cc: "Martin Kosek" , "freeipa-devel" 
>> 
>> Sent: Wednesday, October 30, 2013 7:11:20 PM
>> Subject: Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes 
>> for users and hosts
>>
>> On Wed, 2013-10-30 at 19:01 +0100, Ana Krivokapic wrote:
>>> On 10/29/2013 02:04 PM, Simo Sorce wrote:
>>>> On Tue, 2013-10-29 at 12:42 +0100, Martin Kosek wrote:
>>>>> On 10/29/2013 10:49 AM, Ana Krivokapic wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Patch 0080 adds userClass attribute for users to IPA CLI.
>>>>>> Patch 0081 adds userClass attribute for users and hosts to the web UI.
>>>>>>
>>>>>> Design page:
>>>>>> http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
>>>>>>
>>>>>> Tickets:
>>>>>> https://fedorahosted.org/freeipa/ticket/3588
>>>>>> https://fedorahosted.org/freeipa/ticket/3590
>>>>> NACK to just extending posixAccount objectclass. This is a standard
>>>>> objectclass
>>>>> defined by RFC 2307 and we cannot just simply extend and overwrite it as
>>>>> we wish.
>>>> Uhh indeed this is a big No-no.
>>>>
>>>>> We will need to come up with some custom objectclass, like ipaUser. This
>>>>> is the
>>>>> reason why I wrote to ticket "A second goal of this ticket is to review
>>>>> current
>>>>> objectClass hierarchy of users and do changes if needed." so that we can
>>>>> pick
>>>>> the best option where to place it.
>>>> userClass is used in ipaHost, so I guess it could be instead add to an
>>>> ipa objectclass. ipaObject might be used perhaps, otherwise we'll need a
>>>> new ipaUser objectlass.
>>>>
>>>> Simo.
>>>>
>>> If there are no objections to using the ipaObject objectclass, the attached
>>> patches implement this approach.
>> After some thinking ipaObject is more generic than just users, not sure
>> that attaching userClass there is appropriate. I think we really need
>> ipaUser at this point.
> +1. I also do not think that ipaObject is the right OC to place the 
> attribute, it is just too general.
>
> Let's go with the ipaUser objectClass, looking something like that:
>
> (  NAME 'ipaUser' AUXILIARY MUST ( uid ) MAY ( userClass ) X-ORIGIN 'IPA 
> v3' )
>
> We will need to add the OC when needed, we cannot just add it to default 
> list. Ideally, we could also implement
> https://fedorahosted.org/freeipa/ticket/3922
> in scope of this effort as this need to add additional OCs is piling up.
>
> Martin

This implementation introduces a new objectclass 'ipaUser'.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From f951c5fb6187c0aa5eb982bb7e22829364d58fb8 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Tue, 12 Nov 2013 11:03:28 +0100
Subject: [PATCH] Add userClass attribute for users

This new freeform user attribute will allow provisioning systems
to add custom tags for user objects which can be later used for
automember rules or for additional local interpretation.

Design page: http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
https://fedorahosted.org/freeipa/ticket/3588
---
 API.txt  |  9 --
 VERSION  |  2 +-
 install/share/60basev3.ldif  |  1 +
 install/updates/10-60basev3.update   |  3 ++
 ipalib/plugins/user.py   | 22 +--
 ipatests/test_xmlrpc/test_user_plugin.py | 48 +---
 6 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index cddb9d719f38dd3d89e2633148311e437aed0bc9..77619c53490bd598ebfe30f9c2c9ec1608f4c7d1 100644
--- a/API.txt
+++ b/API.txt
@@ -3587,7 +3587,7 @@ command: trustdomain_mod
 output: Output('summary', (, ), None)
 output: Output('value', , None)
 command: user_add
-args: 1,36,3
+args: 1,37,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude=&

Re: [Freeipa-devel] [PATCHES] 0258-0265 Add schema updater based on IPA schema files

2013-11-11 Thread Ana Krivokapic
On 11/11/2013 02:53 PM, Ana Krivokapic wrote:
> On 11/11/2013 12:32 PM, Petr Viktorin wrote:
>> On 11/07/2013 02:34 PM, Ana Krivokapic wrote:
>>> On 11/01/2013 03:26 PM, Petr Viktorin wrote:
>>>> On 09/13/2013 06:44 PM, Petr Viktorin wrote:
>>>>> On 08/01/2013 04:52 PM, Petr Viktorin wrote:
>>>>>> Hello,
>>>>>> With these patches, schema updates will be based on the ldif files we
>>>>>> use for installation.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/3454
>>>>>>
>>>>>> This is a RFE, here is the design doc:
>>>>>> http://www.freeipa.org/page/V3/Improved_schema_updater
>>>>>>
>>>>> I found and filed a bug in python-ldap[0]: it sometimes ignores parts of
>>>>> schema LDIFs when parsing them.
>>>>> Patch 0275 works around the bug. Please apply on top of 0258-0265 (they
>>>>> still apply cleanly).
>>>>>
>>>>>
>>>>> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820
>>>>>
>>>> The recent ipaldap patches resulted in a small conflict. Attaching
>>>> rebased patches.
>>> I have tested the patches and overall they seem to work fine. Some
>>> questions/comments are below.
>>>
>>>
>>> Patch 258:
>>> You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is
>>> called from `__init__()`, so no need to catch it again in `__init__()`.
>> I've added a comment instead of the try/catch
>>
>>> Patch 259:
>>> ACK
>>>
>>> Patch 260:
>>>
>>>  >   # Usually the modlist order does not matter.
>>>  >   # However, for schema updates, we want 'attributetypes' before
>>>  >   # 'objectclasses'.
>>>  >   # A simple sort will ensure this.
>>>  >   modlist.sort()
>>>
>>> Since `modlist` is a list of tuples, it is sorted by the first elements
>>> in the tuples, then by the seconds elements, etc. Which means the
>>> resulting list will be sorted by the modification type first (`MOD_ADD`,
>>> `MOD_DELETE`, etc), and by `attributetypes`/`objectclasses` second. Was
>>> this the desired effect?
>> I've added a sort key; it should be safer now.
> Hmm, the key you added still retains the default sorting behavior - it will 
> sort
> by the first elements of the tuples first. Since we need sorting by the second
> elements, I think the key here should be: key=lambda m: m[1].lower()
>
>>> Patch 261:
>>> Man page updates look good, but several options in the man page have 3
>>> dashes in the long form instead of 2. I have attached a mini-patch that
>>> fixes this along with a couple of typos in the man page. Feel free to
>>> squash it to your patch 261.
>> Nice catch! Squashed.
>>
>>> Patch 262:
>>> Whitespace warnings.
>> I didn't see any with my settings; could you be more specific?
> $ git am
> ~/freeipa-pviktori-0262.3-Remove-schema-modifications-from-update-files.patch
> Applying: Remove schema modifications from update files
> /home/ana/freeipa/.git/rebase-apply/patch:497: new blank line at EOF.
> +
> /home/ana/freeipa/.git/rebase-apply/patch:693: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.
>
>
>>> In `60-trusts.update` there are still some `replace:attributeTypes:`
>>> lines. Can those be removed safely?
>> Yes! I've checked they match the ldif, and removed them.
>>
>>> Patch 263:
>>>
>>> +if not force_replace:
>>> +modlist.append((ldap.MOD_DELETE, key, removes))
>>> +elif new_values == []: # delete an empty value
>>> +modlist.append((ldap.MOD_DELETE, key, removes))
>>>
>>> can be combined into one:
>>>
>>> +if not force_replace or not new_values:
>>> +modlist.append((ldap.MOD_DELETE, key, removes))
>> Done
>>
>>> Patch 264:
>>> ACK
>>>
>>> Patch 265:
>>> ACK
>>>
>>> Patch 275:
>>> ACK
>> Thanks for the review!
>> Updated patches attached.
>>
>>
>

I'm also seeing some errors when testing the patches. During ipa-server-install,
restarting of DS is failing:

  [22/38]: restarting directory server
ipa : CRITICAL Failed to restart the directory server (Command
'/bin/system

Re: [Freeipa-devel] [PATCHES] 0258-0265 Add schema updater based on IPA schema files

2013-11-11 Thread Ana Krivokapic
On 11/11/2013 12:32 PM, Petr Viktorin wrote:
> On 11/07/2013 02:34 PM, Ana Krivokapic wrote:
>> On 11/01/2013 03:26 PM, Petr Viktorin wrote:
>>> On 09/13/2013 06:44 PM, Petr Viktorin wrote:
>>>> On 08/01/2013 04:52 PM, Petr Viktorin wrote:
>>>>> Hello,
>>>>> With these patches, schema updates will be based on the ldif files we
>>>>> use for installation.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/3454
>>>>>
>>>>> This is a RFE, here is the design doc:
>>>>> http://www.freeipa.org/page/V3/Improved_schema_updater
>>>>>
>>>>
>>>> I found and filed a bug in python-ldap[0]: it sometimes ignores parts of
>>>> schema LDIFs when parsing them.
>>>> Patch 0275 works around the bug. Please apply on top of 0258-0265 (they
>>>> still apply cleanly).
>>>>
>>>>
>>>> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820
>>>>
>>>
>>> The recent ipaldap patches resulted in a small conflict. Attaching
>>> rebased patches.
>
>>
>> I have tested the patches and overall they seem to work fine. Some
>> questions/comments are below.
>>
>>
>> Patch 258:
>> You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is
>> called from `__init__()`, so no need to catch it again in `__init__()`.
>
> I've added a comment instead of the try/catch
>
>> Patch 259:
>> ACK
>>
>> Patch 260:
>>
>>  >   # Usually the modlist order does not matter.
>>  >   # However, for schema updates, we want 'attributetypes' before
>>  >   # 'objectclasses'.
>>  >   # A simple sort will ensure this.
>>  >   modlist.sort()
>>
>> Since `modlist` is a list of tuples, it is sorted by the first elements
>> in the tuples, then by the seconds elements, etc. Which means the
>> resulting list will be sorted by the modification type first (`MOD_ADD`,
>> `MOD_DELETE`, etc), and by `attributetypes`/`objectclasses` second. Was
>> this the desired effect?
>
> I've added a sort key; it should be safer now.

Hmm, the key you added still retains the default sorting behavior - it will sort
by the first elements of the tuples first. Since we need sorting by the second
elements, I think the key here should be: key=lambda m: m[1].lower()

>
>> Patch 261:
>> Man page updates look good, but several options in the man page have 3
>> dashes in the long form instead of 2. I have attached a mini-patch that
>> fixes this along with a couple of typos in the man page. Feel free to
>> squash it to your patch 261.
>
> Nice catch! Squashed.
>
>> Patch 262:
>> Whitespace warnings.
>
> I didn't see any with my settings; could you be more specific?

$ git am
~/freeipa-pviktori-0262.3-Remove-schema-modifications-from-update-files.patch
Applying: Remove schema modifications from update files
/home/ana/freeipa/.git/rebase-apply/patch:497: new blank line at EOF.
+
/home/ana/freeipa/.git/rebase-apply/patch:693: new blank line at EOF.
+
warning: 2 lines add whitespace errors.


>
>> In `60-trusts.update` there are still some `replace:attributeTypes:`
>> lines. Can those be removed safely?
>
> Yes! I've checked they match the ldif, and removed them.
>
>> Patch 263:
>>
>> +if not force_replace:
>> +modlist.append((ldap.MOD_DELETE, key, removes))
>> +elif new_values == []: # delete an empty value
>> +modlist.append((ldap.MOD_DELETE, key, removes))
>>
>> can be combined into one:
>>
>> +if not force_replace or not new_values:
>> +modlist.append((ldap.MOD_DELETE, key, removes))
>
> Done
>
>> Patch 264:
>> ACK
>>
>> Patch 265:
>> ACK
>>
>> Patch 275:
>> ACK
>
> Thanks for the review!
> Updated patches attached.
>
>


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 440 Server does not detect different server and IPA domain

2013-11-08 Thread Ana Krivokapic

On 11/06/2013 10:19 AM, Martin Kosek wrote:

Server installer does not properly recognize a situation when server
fqdn is not in a subdomain of the IPA domain, but shares the same
suffix.

For example, if server FQDN is ipa-idm.example.com and domain
is idm.example.com, server's FQDN is not in the main domain, but
installer does not recognize that. proper Kerberos realm-domain
mapping is not created in this case and server does not work
(httpd reports gssapi errors).

https://fedorahosted.org/freeipa/ticket/4012



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


ACK

--
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0084 Make sure state of services is preserved after client uninstall

2013-11-07 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3790.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From ca950b2f6efe9c73288818667bfd618f4a2fd689 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 7 Nov 2013 17:18:32 +0100
Subject: [PATCH] Make sure state of services is preserved after client
 uninstall

IPA client installation did not preserve the status of nscd and nslcd services
correctly. E.g. nscd would be started after uninstallation, even though it
wasn't running before client installation. Make sure the state of services is
saved before installation and correctly restored after uninstallation.

https://fedorahosted.org/freeipa/ticket/3790
---
 ipa-client/ipa-install/ipa-client-install | 81 ++-
 1 file changed, 47 insertions(+), 34 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 1f66ae5d635d98ba45df13d92ca7982068d94752..dd5b8933b4eabc0d99fb5e40677d1b42dd5ca527 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -237,6 +237,38 @@ def get_cert_path(cert_path):
 
 return None
 
+
+def save_state(service):
+enabled = service.is_enabled()
+running = service.is_running()
+
+if enabled or running:
+statestore.backup_state(service.service_name, 'enabled', enabled)
+statestore.backup_state(service.service_name, 'running', running)
+
+
+def restore_state(service):
+enabled = statestore.restore_state(service.service_name, 'enabled')
+running = statestore.restore_state(service.service_name, 'running')
+
+if enabled:
+try:
+service.enable()
+except Exception:
+root_logger.warning(
+"Failed to configure automatic startup of the %s daemon",
+service.service_name
+)
+if running:
+try:
+service.start()
+except Exception:
+root_logger.warning(
+"Failed to restart the %s daemon",
+service.service_name
+)
+
+
 # Checks whether nss_ldap or nss-pam-ldapd is installed. If anyone of mandatory files was found returns True and list of all files found.
 def nssldap_exists():
 files_to_check = [{'function':'configure_ldap_conf', 'mandatory':['/etc/ldap.conf','/etc/nss_ldap.conf','/etc/libnss-ldap.conf'], 'optional':['/etc/pam_ldap.conf']},
@@ -555,42 +587,17 @@ def uninstall(options, env):
 ipautil.restore_hostname(statestore)
 
 nscd = ipaservices.knownservices.nscd
-if nscd.is_installed():
-try:
-nscd.restart()
-except Exception:
-root_logger.warning(
-"Failed to restart the %s daemon", nscd.service_name)
-
-try:
-nscd.enable()
-except Exception:
-root_logger.warning(
-"Failed to configure automatic startup of the %s daemon",
-nscd.service_name)
-else:
-# this is optional service, just log
-root_logger.info("%s daemon is not installed, skip configuration",
-nscd.service_name)
-
 nslcd = ipaservices.knownservices.nslcd
-if nslcd.is_installed():
-try:
-nslcd.stop()
-except Exception:
-root_logger.warning(
-"Failed to stop the %s daemon", nslcd.service_name)
 
-try:
-nslcd.disable()
-except Exception:
-root_logger.warning(
-"Failed to disable automatic startup of the %s daemon",
-nslcd.service_name)
-else:
-# this is optional service, just log
-root_logger.info("%s daemon is not installed, skip configuration",
-nslcd.service_name)
+for service in (nscd, nslcd):
+if service.is_installed():
+restore_state(service)
+else:
+# this is an optional service, just log
+root_logger.info(
+"%s daemon is not installed, skip configuration",
+service.service_name
+)
 
 ntp_configured = statestore.has_state('ntp')
 if ntp_configured:
@@ -2415,6 +2422,8 @@ def install(options, env, fstore, statestore):
 #Name Server Caching Daemon. Disable for SSSD, use otherwise (if installed)
 nscd = ipaservices.knownservices.nscd
 if nscd.is_installed():
+save_state(nscd)
+
 try:
 if options.sssd:
 nscd_service_action = 'stop'
@@ -2452,6 +2461,10 @@ def install(options, env, fstore, statestore):
 root_logger.info("%s daemon is not installed, skip conf

Re: [Freeipa-devel] [PATCHES] 0258-0265 Add schema updater based on IPA schema files

2013-11-07 Thread Ana Krivokapic
On 11/01/2013 03:26 PM, Petr Viktorin wrote:
> On 09/13/2013 06:44 PM, Petr Viktorin wrote:
>> On 08/01/2013 04:52 PM, Petr Viktorin wrote:
>>> Hello,
>>> With these patches, schema updates will be based on the ldif files we
>>> use for installation.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3454
>>>
>>> This is a RFE, here is the design doc:
>>> http://www.freeipa.org/page/V3/Improved_schema_updater
>>>
>>
>> I found and filed a bug in python-ldap[0]: it sometimes ignores parts of
>> schema LDIFs when parsing them.
>> Patch 0275 works around the bug. Please apply on top of 0258-0265 (they
>> still apply cleanly).
>>
>>
>> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820
>>
>
> The recent ipaldap patches resulted in a small conflict. Attaching rebased
> patches.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

I have tested the patches and overall they seem to work fine. Some
questions/comments are below.


Patch 258:
You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is called from
`__init__()`, so no need to catch it again in `__init__()`.

Patch 259:
ACK

Patch 260:

>   # Usually the modlist order does not matter.
>   # However, for schema updates, we want 'attributetypes' before
>   # 'objectclasses'.
>   # A simple sort will ensure this.
>   modlist.sort()

Since `modlist` is a list of tuples, it is sorted by the first elements in the
tuples, then by the seconds elements, etc. Which means the resulting list will
be sorted by the modification type first (`MOD_ADD`, `MOD_DELETE`, etc), and by
`attributetypes`/`objectclasses` second. Was this the desired effect?

Patch 261:
Man page updates look good, but several options in the man page have 3 dashes in
the long form instead of 2. I have attached a mini-patch that fixes this along
with a couple of typos in the man page. Feel free to squash it to your patch 
261.

Patch 262:
Whitespace warnings.
In `60-trusts.update` there are still some `replace:attributeTypes:` lines. Can
those be removed safely?

Patch 263:

+if not force_replace:
+modlist.append((ldap.MOD_DELETE, key, removes))
+elif new_values == []: # delete an empty value
+modlist.append((ldap.MOD_DELETE, key, removes))

can be combined into one:

+if not force_replace or not new_values:
+modlist.append((ldap.MOD_DELETE, key, removes))

Patch 264:
ACK

Patch 265:
ACK

Patch 275:
ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 217dcf77de67150e255a9abde07ea080168ac311 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 6 Nov 2013 18:33:33 +0100
Subject: [PATCH] ipa-ldap-updater man page fixes

---
 install/tools/man/ipa-ldap-updater.1 | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/install/tools/man/ipa-ldap-updater.1 b/install/tools/man/ipa-ldap-updater.1
index c79721f1925f4c52ef4b41da7a540e9f0a8d5743..79cc316501512879fa39ba4c15fd898b976eb25e 100644
--- a/install/tools/man/ipa-ldap-updater.1
+++ b/install/tools/man/ipa-ldap-updater.1
@@ -47,7 +47,7 @@ Values is a comma\-separated field so multi\-values may be added at one time. Do
 
 The difference between the default and add keywords is if the DN of the entry exists then default is ignored. So for updating something like schema, which will be under cn=schema, you must always use add (because cn=schema is guaranteed to exist). It will not re\-add the same information again and again.
 
-It alsos provide some things that can be templated such as architecture (for plugin paths), realm and domain name.
+It also provides some things that can be templated such as architecture (for plugin paths), realm and domain name.
 
 The available template variables are:
 
@@ -63,8 +63,8 @@ A few rules:
 
1. Only one rule per line
2. Each line stands alone (e.g. an only followed by an only results in the last only being used)
-   3. adding a value that exists is ok. The request is ignored, duplicate values are not added
-   4. removing a value that doesn't exist is ok. It is simply ignored.
+   3. Adding a value that exists is ok. The request is ignored, duplicate values are not added
+   4. Removing a value that doesn't exist is ok. It is simply ignored.
5. If a DN doesn't exist it is created from the 'default' entry and all updates are applied
6. If a DN does exist the default values are skipped
7. Only the first rule on a line is respected
@@ -90,19 +90,19 @@ File containing the Directory Manager password
 \fB\-l\fR, \fB\-\-ldapi\fR
 Connect to the LDAP se

Re: [Freeipa-devel] [PATCH] 0316 Remove unused utf8_encode_value functions

2013-11-06 Thread Ana Krivokapic
On 11/05/2013 02:02 PM, Petr Viktorin wrote:
> Honza's recent LDAP refactoring left some unused helper functions around. This
> patch removes them.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0083 Remove internaldb pasword from password.conf

2013-11-06 Thread Ana Krivokapic
On 11/06/2013 01:34 PM, Ana Krivokapic wrote:
> Hello,
>
> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/4005.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

And now, with correct spelling in the commit message...

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From d620ae08698db2c1524fbaf1ec6f12caa75e7f86 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 6 Nov 2013 13:29:09 +0100
Subject: [PATCH] Remove internaldb password from password.conf

Remove internaldb password from password.conf after switching over to
client certificate authentication. The password is no longer needed.

https://fedorahosted.org/freeipa/ticket/4005
---
 ipaserver/install/cainstance.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index ac5c81de8c57194160cbfd2fa79c776bf2f39625..db1424c5713e6d87acdc1533684cd8485ec3c13a 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1317,6 +1317,9 @@ def enable_client_auth_to_db(self):
 installutils.set_directive(caconfig,
  'internaldb.ldapconn.secureConn', 'true', quotes=False,
  separator='=')
+# Remove internaldb password as is not needed anymore
+installutils.set_directive(self.dogtag_constants.PASSWORD_CONF_PATH,
+   'internaldb', None)
 
 def uninstall(self):
 if self.is_configured():
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0083 Remove internaldb pasword from password.conf

2013-11-06 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/4005.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From d79edbb9b2224a38636639944ffd9e51d095e920 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 6 Nov 2013 13:29:09 +0100
Subject: [PATCH] Remove internaldb pasword from password.conf

Remove internaldb pasword from password.conf after switching over to
client certificate authentication. The password is no longer needed.

https://fedorahosted.org/freeipa/ticket/4005
---
 ipaserver/install/cainstance.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index ac5c81de8c57194160cbfd2fa79c776bf2f39625..db1424c5713e6d87acdc1533684cd8485ec3c13a 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1317,6 +1317,9 @@ def enable_client_auth_to_db(self):
 installutils.set_directive(caconfig,
  'internaldb.ldapconn.secureConn', 'true', quotes=False,
  separator='=')
+# Remove internaldb password as is not needed anymore
+installutils.set_directive(self.dogtag_constants.PASSWORD_CONF_PATH,
+   'internaldb', None)
 
 def uninstall(self):
 if self.is_configured():
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0082 Use EXTERNAL auth mechanism in ldapmodify

2013-11-05 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3895.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 3bd13d7424a05d3900c13c911bf58899baa8d429 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Tue, 5 Nov 2013 18:38:55 +0100
Subject: [PATCH] Use EXTERNAL auth mechanism in ldapmodify

Default to using the EXTERNAL authorization mechanism in calls to ldapmodify

https://fedorahosted.org/freeipa/ticket/3895
---
 ipaserver/install/service.py | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 4a244abb9135ae4c712abcb27456bc2436728215..0d7a664561fdf2b02353dd7284392e250f61a9f2 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -121,17 +121,15 @@ def ldap_connect(self):
 
 self.admin_conn = conn
 
-
 def ldap_disconnect(self):
 self.admin_conn.unbind()
 self.admin_conn = None
 
-def _ldap_mod(self, ldif, sub_dict = None):
-
+def _ldap_mod(self, ldif, sub_dict=None):
 pw_name = None
 fd = None
 path = ipautil.SHARE_DIR + ldif
-nologlist=[]
+nologlist = []
 
 if sub_dict is not None:
 txt = ipautil.template_file(path, sub_dict)
@@ -139,9 +137,9 @@ def _ldap_mod(self, ldif, sub_dict = None):
 path = fd.name
 
 # do not log passwords
-if sub_dict.has_key('PASSWORD'):
+if 'PASSWORD' in sub_dict:
 nologlist.append(sub_dict['PASSWORD'])
-if sub_dict.has_key('RANDOM_PASSWORD'):
+if 'RANDOM_PASSWORD' in sub_dict:
 nologlist.append(sub_dict['RANDOM_PASSWORD'])
 
 args = ["/usr/bin/ldapmodify", "-v", "-f", path]
@@ -152,16 +150,18 @@ def _ldap_mod(self, ldif, sub_dict = None):
 self.ldap_connect()
 args += ["-H", self.admin_conn.ldap_uri]
 
-auth_parms = []
+# If DM password is available, use it
 if self.dm_password:
 [pw_fd, pw_name] = tempfile.mkstemp()
 os.write(pw_fd, self.dm_password)
 os.close(pw_fd)
 auth_parms = ["-x", "-D", "cn=Directory Manager", "-y", pw_name]
+# Use GSSAPI auth when not using DM password or not being root
+elif os.getegid() != 0:
+auth_parms = ["-Y", "GSSAPI"]
+# Default to EXTERNAL auth mechanism
 else:
-# always try GSSAPI auth when not using DM password or not being root
-if os.getegid() != 0:
-auth_parms = ["-Y", "GSSAPI"]
+auth_parms = ["-Y", "EXTERNAL"]
 
 args += auth_parms
 
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0125] ipatests: Add which package to legacy client advice

2013-11-01 Thread Ana Krivokapic
On 11/01/2013 03:30 PM, Tomas Babej wrote:
> On 11/01/2013 03:27 PM, Ana Krivokapic wrote:
>> On 11/01/2013 03:18 PM, Tomas Babej wrote:
>>> On 10/31/2013 12:10 PM, Ana Krivokapic wrote:
>>>> On 10/30/2013 04:18 PM, Tomas Babej wrote:
>>>>> Hi,
>>>>>
>>>>> Adds which package to the requirements, since older distros do not have it
>>>>> by default.
>>>>>
>>>>> Part of: https://fedorahosted.org/freeipa/ticket/3833
>>>>>
>>>>>
>>>>>
>>>>> ___
>>>>> Freeipa-devel mailing list
>>>>> Freeipa-devel@redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>
>>>> You can use the bash built-in command `command`, instead of `which`, to
>>>> find out if a program exists:
>>>>
>>>> command -v cacertdir_rehash
>>>>
>>>> In other words, just replace `which` with `command -v`; there's no need to
>>>> install any additional packages.
>>>> -- 
>>>> Regards,
>>>>
>>>> Ana Krivokapic
>>>> Associate Software Engineer
>>>> FreeIPA team
>>>> Red Hat Inc.
>>>>
>>>>
>>>> ___
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel@redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>
>>> Thanks!
>>>
>>> Updated patch attached.
>>>
>>>
>>>
>>> -- 
>>> Tomas Babej
>>> Associate Software Engeneer | Red Hat | Identity Management
>>> RHCE | Brno Site | IRC: tbabej | freeipa.org
>>
>> Looks good! Please just amend the commit message to reflect the new content
>> of the patch.
>>
>> -- 
>> Regards,
>>
>> Ana Krivokapic
>> Associate Software Engineer
>> FreeIPA team
>> Red Hat Inc.
>
> Good catch. Fixed.
>
> -- 
> Tomas Babej
> Associate Software Engeneer | Red Hat | Identity Management
> RHCE | Brno Site | IRC: tbabej | freeipa.org

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0125] ipatests: Add which package to legacy client advice

2013-11-01 Thread Ana Krivokapic
On 11/01/2013 03:18 PM, Tomas Babej wrote:
> On 10/31/2013 12:10 PM, Ana Krivokapic wrote:
>> On 10/30/2013 04:18 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> Adds which package to the requirements, since older distros do not have it
>>> by default.
>>>
>>> Part of: https://fedorahosted.org/freeipa/ticket/3833
>>>
>>>
>>>
>>> ___
>>> Freeipa-devel mailing list
>>> Freeipa-devel@redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>> You can use the bash built-in command `command`, instead of `which`, to find
>> out if a program exists:
>>
>> command -v cacertdir_rehash
>>
>> In other words, just replace `which` with `command -v`; there's no need to
>> install any additional packages.
>> -- 
>> Regards,
>>
>> Ana Krivokapic
>> Associate Software Engineer
>> FreeIPA team
>> Red Hat Inc.
>>
>>
>> ___
>> Freeipa-devel mailing list
>> Freeipa-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
> Thanks!
>
> Updated patch attached.
>
>
>
> -- 
> Tomas Babej
> Associate Software Engeneer | Red Hat | Identity Management
> RHCE | Brno Site | IRC: tbabej | freeipa.org

Looks good! Please just amend the commit message to reflect the new content of
the patch.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] Reminder: Patchwork

2013-11-01 Thread Ana Krivokapic
On 10/31/2013 07:24 PM, Simo Sorce wrote:
> On Thu, 2013-10-31 at 18:52 +0100, Ana Krivokapic wrote:
>> Hello IPA developers,
>>
>> I would like to remind everyone about our Patchwork instance[1]. This tool 
>> helps
>> us to better coordinate work and be more efficient, so let's try to remember 
>> to
>> use it consistently. It takes only a few seconds of extra work per patch
>> submission/review, which could potentially save us much more time. It is
>> especially useful to mark the patch as 'Under Review' when you start 
>> reviewing
>> it, so that others are aware of it and they don't start reviewing the same
>> patch. That way, we can avoid the situation of two people accidentally doing 
>> the
>> review of the same patch at the same time.
>>
>> I have just cleaned up the Patchwork instance as best as I could so it should
>> now hopefully reflect the real situation on the state of patches.
>>
>> We also have some instructions on how to use it on our wiki[2].
>>
>> Thanks!
>>
>> [1] https://patchwork.acksyn.org/project/FreeIPA/list/
>> [2] 
>> http://www.freeipa.org/page/Contribute/Code#Tracking_patches_.28Experimental.29
> Ana,
> thanks for going through it, I try to keep it up to date but have had
> little time recently.
> I
> Incidentally the machine run out of memory and the database crashed :-(
> I added more memory and restarted the instance, please let me know if
> everything is ok, it looks good to me.
>
> Simo.
>
Everything seems ok, I'll report back if I find any problems.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] Reminder: Patchwork

2013-10-31 Thread Ana Krivokapic
Hello IPA developers,

I would like to remind everyone about our Patchwork instance[1]. This tool helps
us to better coordinate work and be more efficient, so let's try to remember to
use it consistently. It takes only a few seconds of extra work per patch
submission/review, which could potentially save us much more time. It is
especially useful to mark the patch as 'Under Review' when you start reviewing
it, so that others are aware of it and they don't start reviewing the same
patch. That way, we can avoid the situation of two people accidentally doing the
review of the same patch at the same time.

I have just cleaned up the Patchwork instance as best as I could so it should
now hopefully reflect the real situation on the state of patches.

We also have some instructions on how to use it on our wiki[2].

Thanks!

[1] https://patchwork.acksyn.org/project/FreeIPA/list/
[2] 
http://www.freeipa.org/page/Contribute/Code#Tracking_patches_.28Experimental.29

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0123] ipatests: Do not use /usr/bin hardcoded paths

2013-10-31 Thread Ana Krivokapic
On 10/30/2013 04:01 PM, Tomas Babej wrote:
> Hi,
>
> The RHEL 5.9 clients do not have /usr/bin symlinks.
>
> Part of: https://fedorahosted.org/freeipa/ticket/3833
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0126] ipatests: Restore SELinux context after restoring files from

2013-10-31 Thread Ana Krivokapic
On 10/30/2013 04:19 PM, Tomas Babej wrote:
> Hi,
>
> Without this patch, restored directories get home_t SELinux context.
>
> Part of: https://fedorahosted.org/freeipa/ticket/3833
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0124] ipatests: Extend clear_sssd_cache to support non-systemd

2013-10-31 Thread Ana Krivokapic
On 10/30/2013 04:40 PM, Tomas Babej wrote:
> Hi,
>
> This allows us to clean sssd cache on older, non-systemd platforms.
>
> Part of: https://fedorahosted.org/freeipa/ticket/3833
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Looks good, ACK.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0125] ipatests: Add which package to legacy client advice

2013-10-31 Thread Ana Krivokapic
On 10/30/2013 04:18 PM, Tomas Babej wrote:
> Hi,
>
> Adds which package to the requirements, since older distros do not have it by
> default.
>
> Part of: https://fedorahosted.org/freeipa/ticket/3833
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

You can use the bash built-in command `command`, instead of `which`, to find out
if a program exists:

command -v cacertdir_rehash

In other words, just replace `which` with `command -v`; there's no need to
install any additional packages.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts

2013-10-30 Thread Ana Krivokapic
On 10/29/2013 02:04 PM, Simo Sorce wrote:
> On Tue, 2013-10-29 at 12:42 +0100, Martin Kosek wrote:
>> On 10/29/2013 10:49 AM, Ana Krivokapic wrote:
>>> Hello,
>>>
>>> Patch 0080 adds userClass attribute for users to IPA CLI.
>>> Patch 0081 adds userClass attribute for users and hosts to the web UI.
>>>
>>> Design page:
>>> http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
>>>
>>> Tickets:
>>> https://fedorahosted.org/freeipa/ticket/3588
>>> https://fedorahosted.org/freeipa/ticket/3590
>> NACK to just extending posixAccount objectclass. This is a standard 
>> objectclass
>> defined by RFC 2307 and we cannot just simply extend and overwrite it as we 
>> wish.
> Uhh indeed this is a big No-no.
>
>> We will need to come up with some custom objectclass, like ipaUser. This is 
>> the
>> reason why I wrote to ticket "A second goal of this ticket is to review 
>> current
>> objectClass hierarchy of users and do changes if needed." so that we can pick
>> the best option where to place it.
> userClass is used in ipaHost, so I guess it could be instead add to an
> ipa objectclass. ipaObject might be used perhaps, otherwise we'll need a
> new ipaUser objectlass.
>
> Simo.
>

If there are no objections to using the ipaObject objectclass, the attached
patches implement this approach.

Also, the schema change has been added to the appropriate .ldif file, and the
addeer dialogs in the web UI have been extended to support the new userclass
attribute.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 02b116d8a5fe2ff2e47d7a6ebc227564c6244fbe Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 25 Oct 2013 16:31:50 +0200
Subject: [PATCH] WebUI: Add userClass attribute to user and host pages

Add userClass attribute to:
- user and host adder dialogs
- user and host detail facets

Design page: http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
https://fedorahosted.org/freeipa/ticket/3590
---
 install/ui/src/freeipa/host.js | 2 ++
 install/ui/src/freeipa/user.js | 6 --
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/install/ui/src/freeipa/host.js b/install/ui/src/freeipa/host.js
index f5007538e8ad1ea2e372c194b129f6c668d31b3e..460a19a9cda9a7d1d4457bb19f306dd88df8ceac 100644
--- a/install/ui/src/freeipa/host.js
+++ b/install/ui/src/freeipa/host.js
@@ -82,6 +82,7 @@ return {
 $type: 'textarea',
 name: 'description'
 },
+'userclass',
 'l',
 'nshostlocation',
 'nshardwareplatform',
@@ -234,6 +235,7 @@ return {
 {
 name: 'other',
 fields: [
+'userclass',
 {
 name: 'ip_address',
 validators: [ 'ip_address' ],
diff --git a/install/ui/src/freeipa/user.js b/install/ui/src/freeipa/user.js
index 61bdb43b4ee7d23a5d118c4f29ff81e3b9f56fa1..aeeb52ea6f2bca883a656a8249386b9692711a10 100644
--- a/install/ui/src/freeipa/user.js
+++ b/install/ui/src/freeipa/user.js
@@ -103,7 +103,8 @@ return {
 'cn',
 'displayname',
 'initials',
-'gecos'
+'gecos',
+'userclass'
 ]
 },
         {
@@ -306,7 +307,8 @@ return {
 required: false
 },
 'givenname',
-'sn'
+'sn',
+'userclass'
 ]
 },
 {
-- 
1.8.3.1

From d91df80c081e5c766bae017acb8271a024534b4f Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 25 Oct 2013 16:29:26 +0200
Subject: [PATCH] Add userClass attribute for users

This new freeform user attribute will allow provisioning systems
to add custom tags for user objects which can be later used for
automember rules or for additional local interpretation.

Design page: http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
https://fedorahosted.org/freeipa/ticket/3588
---
 API.txt  |  9 ++---
 VERSION  |  2 +-
 install/share/60basev2.ldif  |  2 +-
 install/updates/10-60basev3.update   |  1 +
 ipalib/plugins/user.py   |  8 +++-
 ipatests/test_xmlrpc/test_user_plugin.py | 33 +

Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts

2013-10-30 Thread Ana Krivokapic
On 10/29/2013 11:08 PM, Dmitri Pal wrote:
> On 10/29/2013 08:04 AM, Petr Vobornik wrote:
>> On 10/29/2013 10:49 AM, Ana Krivokapic wrote:
>>> Hello,
>>>
>>> Patch 0080 adds userClass attribute for users to IPA CLI.
>>> Patch 0081 adds userClass attribute for users and hosts to the web UI.
>>>
>>> Design page:
>>> http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
>>>
>>> Tickets:
>>> https://fedorahosted.org/freeipa/ticket/3588
>>> https://fedorahosted.org/freeipa/ticket/3590
>>>
>> In addition to the posixAccount issue:
>>
>> 1. I think we should also add a field to host and user adder dialogs.
>> This feature was designed to work with automember plugin. Without it
>> in adder dialog, user would have to modify the entry and then run
>> automember-rebuild.
>>
>> That said, I understand that we may decide not to include it in adder
>> dialogs if we say that this feature is only for provisioning systems,
>> which don't use Web UI.
> I agree with this. Please open a separate ticket to expose these fields
> in the UI.
>

I think we can do that within the scope of ticket #3590. I added a comment to
specify exactly what needs to be done:

https://fedorahosted.org/freeipa/ticket/3590#comment:5

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 462 Fix password expiration notification

2013-10-30 Thread Ana Krivokapic
On 10/30/2013 01:24 PM, Petr Vobornik wrote:
> On 10/30/2013 01:18 PM, Ana Krivokapic wrote:
>> The patch fixes the issue.
>>
>> I have just one nitpick:
>>
>>
>>> +# reset password if needed
>>> +if (self.get_auth_dialog()):
>>^^ ^^  <- redundant parentheses
>>
>>
> Fixed

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 462 Fix password expiration notification

2013-10-30 Thread Ana Krivokapic
The patch fixes the issue.

I have just one nitpick:


> +# reset password if needed
> +if (self.get_auth_dialog()):
  ^^ ^^  <- redundant parentheses


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0079 Enable Retro Changelog and Content Synchronization DS plugins

2013-10-29 Thread Ana Krivokapic
On 10/29/2013 12:46 PM, Martin Kosek wrote:
> On 10/25/2013 05:03 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3967.
>>
> NACK. I do not think this will work well with the case when DNS is not used. 
> As
> bind-dyndb-ldap is not required component, FreeIPA could be installed on a
> machine without bind installed. And in that case, /var/named/ won't be there.
>
> I think that this directory will follow similar pattern as
> %{_localstatedir}/lib/ipa/pki-ca/publish
> and be just %ghost and be created in when bind-dyndb-ldap is being configured
> in bindinstance.py.
>
> Martin

Fixed, updated patch attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 238f5fb8e878f0a4437b23e38ba6ca8cb5efc89e Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 25 Oct 2013 12:41:25 +0200
Subject: [PATCH] Enable Retro Changelog and Content Synchronization DS plugins

Enable Retro Changelog and Content Synchronization DS plugins which are required
for SyncRepl support.

Create a working directory /var/named/ipa required by bind-dyndb-ldap v4+.

https://fedorahosted.org/freeipa/ticket/3967
---
 freeipa.spec.in|  4 
 install/tools/ipa-upgradeconfig|  5 -
 install/updates/20-syncrepl.update |  9 +
 install/updates/Makefile.am|  1 +
 ipaserver/install/bindinstance.py  | 13 +
 5 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 install/updates/20-syncrepl.update

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 11ae934d928370eb13f45162a13f40a9acd64b74..30f85dd2fd425b4f4d9fd787a1457087011723a3 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -737,6 +737,7 @@ fi
 %{_mandir}/man1/ipa-backup.1.gz
 %{_mandir}/man1/ipa-restore.1.gz
 %{_mandir}/man1/ipa-advise.1.gz
+%ghost %{_localstatedir}/named/ipa
 
 %files server-trust-ad
 %{_sbindir}/ipa-adtrust-install
@@ -832,6 +833,9 @@ fi
 %endif # ONLY_CLIENT
 
 %changelog
+* Tue Oct 29 2013 Ana Krivokapic  - 3.3.90-5
+- Create ghost entry for /var/named/ipa needed for SyncRepl support
+
 * Fri Oct 25 2013 Martin Kosek  - 3.3.90-4
 - Remove mod_ssl conflict, it can now live with mod_nss installed
 
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 41c51263d5fc8b3a0e2f28bab89fc9d2d184fdca..865f5cd57322e10a95a94801eecf467890030bb7 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -1079,6 +1079,10 @@ def main():
 setup_firefox_extension(fstore)
 add_ca_dns_records()
 
+bind = bindinstance.BindInstance(fstore)
+if bind.is_configured():
+bind.create_dir('/var/named/ipa', 0700)
+
 # Any of the following functions returns True iff the named.conf file
 # has been altered
 named_conf_changes = (
@@ -1092,7 +1096,6 @@ def main():
 if any(named_conf_changes):
 # configuration has changed, restart the name server
 root_logger.info('Changes to named.conf have been made, restart named')
-bind = bindinstance.BindInstance(fstore)
 try:
 bind.restart()
 except ipautil.CalledProcessError, e:
diff --git a/install/updates/20-syncrepl.update b/install/updates/20-syncrepl.update
new file mode 100644
index ..6901370f9cb302ff2c0c8bcc3f7b51aadef83e8e
--- /dev/null
+++ b/install/updates/20-syncrepl.update
@@ -0,0 +1,9 @@
+# Enable Retro changelog
+dn: cn=Retro Changelog Plugin,cn=plugins,cn=config
+only:nsslapd-pluginEnabled: on
+add:nsslapd-attribute: nsuniqueid:targetUniqueId
+add:nsslapd-changelogmaxage: nsslapd-changelogmaxage: 2d
+
+# Enable SyncRepl
+dn: cn=Content Synchronization,cn=plugins,cn=config
+only:nsslapd-pluginEnabled: on
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index 40c3b3c8916faa267254a29d0f458ca53201950c..09965ff9885fce93f3d15dc73b11fa210f68b163 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -22,6 +22,7 @@ app_DATA =\
 	20-indices.update		\
 	20-nss_ldap.update		\
 	20-replication.update		\
+	20-syncrepl.update		\
 	20-user_private_groups.update	\
 	20-winsync_index.update		\
 	21-replicas_container.update	\
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 6d5a1d44d30c89278c24fe7ab5278355cb65b0b4..4baeb4e077c64a7abebd1c071012f6c1e02dc1ae 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -22,6 +22,7 @@
 import pwd
 import netaddr
 import re
+import errno
 
 import ldap
 
@@ -509,6 +510,16 @@ def create_sample_bind_zone(self):
 os.close(bind_fd)
 print "Sample zone file for bind has been created in "+bind_name
 
+def create_dir(self, path, mode):
+try:
+os.makedirs(path, mode)
+except OSError as e:
+if e.er

Re: [Freeipa-devel] [PATCHES] 0072-0074 Add automember rebuild membership to the web UI

2013-10-29 Thread Ana Krivokapic
On 10/24/2013 03:48 PM, Petr Vobornik wrote:
> On 10/16/2013 11:37 PM, Ana Krivokapic wrote:
>> On 09/27/2013 04:38 PM, Petr Vobornik wrote:
>>> On 09/25/2013 11:51 AM, Ana Krivokapic wrote:
>>>> Hello,
>>>>
>>>> This patch set addresses ticket
>>>> https://fedorahosted.org/freeipa/ticket/3928.
>>>>
>>>> Patch 0072 hooks the new automember-rebuild command to the web UI
>>>> (user and host
>>>> pages).
>>>> Patch 0073 adds some fixes to the web UI test driver, which are
>>>> necessary for
>>>> patch 0074.
>>>> Patch 0074 adds web UI integration tests for the new feature.
>>>>
>>>> The patch set applies on top of my patches 0068-0071
>>>>
>>>
>>> patch 72: Me, Ana and Kyle discussed position of the actions and the
>>> decision was to move them to action list.
>>>
>>> `ipa automember-rebuild --type=hostgroup` can't be run from UI
>>> (already discussed with Ana as well)
>>>
>>> I'm thinking about adding/creating refresh facet policies for this
>>> action so that appropriate facet would be marked as dirty and
>>> therefore refreshed so user would not have click association facet
>>> refresh button.
>>>
>>> patch 73: Looks OK but some changes might not be needed.
>>>
>>> patch 74: I would use different methods for certain actions to be
>>> consistent with other tests and to make the test more robust against
>>> Web UI refactoring:
>>>
>>> 1.
>>>> self.click_on_link('Refresh')
>>>
>>> For buttons in .facet-controls use rather
>>> `self.facet_button_click('refresh')` where 'refresh' is the button
>>> name, not text.
>>>
>>> 2.
>>>> self.add_record(
>>>> 'automember',
>>>> {'pkey': 'webservers', 'add': [
>>>> ('selectbox', 'key', 'fqdn'),
>>>> ('textbox', 'automemberinclusiveregex', '^web[1-9]+')
>>>> ]},
>>>> facet='hostgrouprule',
>>>> facet_btn_css_class='widget',
>>>> navigate=False
>>>> )
>>>
>>> use `add_table_record('automemberinclusiveregex', data, parent)`.
>>> Example in test_dns.py:94.
>>>
>>> 3. A nitpick(or not even that): When working on one entity, I would
>>> rather use `navigate_by_breadcrumb('link text') instead of direct
>>> `navigate_to_record` call which changes url. It resembles user's
>>> behavior more. But it depends on situation. Ie. when I'm on hostgroup
>>> page and want to go to host search page, but the last host page was
>>> some details or association I may just use `navigate_to_entity('host',
>>> 'search'). Anyway, the important thing is to avoid navigating to the
>>> same url twice in a row - IE10 does not like that.
>>>
>>> 4. Do not use `wait(1)` for AJAX calls. The test will fail if there is
>>> bigger delay. User rather `wait_for_request(n=X)` where X is number of
>>> AJAX calls you want to wait for.
>>>
>>> 5. Instead of `click_on_link('Rebuild auto membership')` you should
>>> use `action_panel_action(panel_name, action)` There is also similar
>>> call for
>>> executing action list action: `action_list_action(action_name)`.
>>>
>>> 6. Nitpick: you can reduce the cleanup code:
>>>> self.navigate_by_menu('identity/host')
>>>> self.delete('host', [{'pkey': host1}])
>>>> self.delete('host', [{'pkey': host2}])
>>>
>>> Can be written as:
>>>
>>> self.delete('host', [{'pkey': host1}, {'pkey': host2}])
>>>
>>> or
>>> self.navigate_by_menu('identity/host')
>>> self.delete_record('host', [host1, host2])
>>>
>>> `delete` is basically a shortcut for `navigate_to_entity` and
>>> `delete_record` with CRUD data format.
>>
>> All fixed, new patches attached.
>>
>
> Patch 72: ACK
>
> Patch 73: existing UI_driver.select method has the same functionality as new
> select_option method.
>
> Patch 74: ACK

Oops, I missed the existing select(

Re: [Freeipa-devel] [PATCHES] 0068-0070 Automember rebuild membership

2013-10-29 Thread Ana Krivokapic
On 10/15/2013 06:09 PM, Ana Krivokapic wrote:
> On 09/30/2013 10:02 AM, Petr Viktorin wrote:
>> On 09/27/2013 03:12 PM, Martin Kosek wrote:
>>> On 09/27/2013 03:00 PM, Jan Cholasta wrote:
>>>> On 23.9.2013 19:41, Ana Krivokapic wrote:
>>>>> On 09/19/2013 03:29 PM, Ana Krivokapic wrote:
>>> ...
>>>> Patch 69:
>>>>
>>>> I think the changes in the update file should be also done in the
>>>> right LDIF
>>>> files in install/share, though I don't know what is the recent
>>>> consensus on this.
>>>>
>>>>
>>>> Honza
>>>>
>>> Last time I checked, we used to do the change both in LDIF and update
>>> file. Just to avoid the LDIF become obsolete.
>>>
>>> Martin
>> Rob recently said his preference is to move everything from LDIF to updates,
>> and out of the the LDIF files:
>> http://www.redhat.com/archives/freeipa-devel/2013-September/msg00106.html
>>
>> I would agree, having two places with the same information is redundant and
>> error-prone.
>>
> Thanks Honza for the review.
>
> I incorporated your suggestions in this updated patchset. I attached all the
> patches for more convenient reviewing, but only patches 68 and 70 have 
> changed.
>
> I haven't done any changes in the LDIF files since the consensus seems to be 
> not
> to do that.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Patch 70 needed a rebase, attaching the whole patchset again.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 222d7e997f6f334210df79d9e3873bcde618c595 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 19 Sep 2013 14:12:44 +0200
Subject: [PATCH] Add unit tests for automember rebuild command

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3752
---
 ipatests/test_xmlrpc/test_automember_plugin.py | 553 -
 1 file changed, 540 insertions(+), 13 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py
index 7b390a5000302d5bdbd483f9bc76ef6a08233a1f..fe66f101184643a97713f6eeb0a96b49b5f2917e 100644
--- a/ipatests/test_xmlrpc/test_automember_plugin.py
+++ b/ipatests/test_xmlrpc/test_automember_plugin.py
@@ -28,8 +28,8 @@
 from ipatests.test_xmlrpc.test_user_plugin import get_user_result
 
 
-user1=u'tuser1'
-manager1=u'mscott'
+user1 = u'tuser1'
+manager1 = u'mscott'
 fqdn1 = u'web1.%s' % api.env.domain
 short1 = u'web1'
 fqdn2 = u'dev1.%s' % api.env.domain
@@ -41,13 +41,17 @@
 fqdn5 = u'webserver5.%s' % api.env.domain
 short5 = u'webserver5'
 
-group1=u'group1'
-defaultgroup1=u'defaultgroup1'
-hostgroup1=u'hostgroup1'
-hostgroup2=u'hostgroup2'
-hostgroup3=u'hostgroup3'
-hostgroup4=u'hostgroup4'
-defaulthostgroup1=u'defaulthostgroup1'
+group1 = u'group1'
+group1_dn = DN(('cn', group1), ('cn', 'groups'),
+   ('cn', 'accounts'), api.env.basedn)
+defaultgroup1 = u'defaultgroup1'
+hostgroup1 = u'hostgroup1'
+hostgroup1_dn = DN(('cn', hostgroup1), ('cn', 'hostgroups'),
+  ('cn', 'accounts'), api.env.basedn)
+hostgroup2 = u'hostgroup2'
+hostgroup3 = u'hostgroup3'
+hostgroup4 = u'hostgroup4'
+defaulthostgroup1 = u'defaulthostgroup1'
 
 group_include_regex = u'mscott'
 hostgroup_include_regex = u'^web[1-9]'
@@ -81,13 +85,13 @@ class test_automember(Declarative):
 desc='Try to retrieve non-existent group rule %r' % group1,
 command=('automember_add', [group1],
 dict(description=u'Test desc', type=u'group')),
-expected=errors.NotFound(reason=u'Group: %s not found!' % group1),
+expected=errors.NotFound(reason=u'group "%s" not found' % group1),
 ),
 
 dict(
 desc='Try to update non-existent group rule %r' % group1,
 command=('automember_add', [group1], dict(type=u'group')),
-expected=errors.NotFound(reason=u'Group: %s not found!' % group1),
+expected=errors.NotFound(reason=u'group "%s" not found' % group1),
 ),
 
 dict(
@@ -102,14 +106,14 @@ class test_automember(Declarative):
 co

[Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts

2013-10-29 Thread Ana Krivokapic
Hello,

Patch 0080 adds userClass attribute for users to IPA CLI.
Patch 0081 adds userClass attribute for users and hosts to the web UI.

Design page:
http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems

Tickets:
https://fedorahosted.org/freeipa/ticket/3588
https://fedorahosted.org/freeipa/ticket/3590

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From df9c93c145671094fe85401a96c490dd60cf0671 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 25 Oct 2013 16:29:26 +0200
Subject: [PATCH] Add userClass attribute for users

This new freeform user attribute will allow provisioning systems
to add custom tags for user objects which can be later used for
automember rules or for additional local interpretation.

Design page: http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems
https://fedorahosted.org/freeipa/ticket/3588
---
 API.txt  |  9 ++---
 VERSION  |  2 +-
 install/updates/10-60basev3.update   |  1 +
 ipalib/plugins/user.py   |  8 +++-
 ipatests/test_xmlrpc/test_user_plugin.py | 33 +---
 5 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/API.txt b/API.txt
index 40871f6a8b105a7b161df34ce4f6feaf785a6107..feb7f27de1ae2b1e9b4582bf225c50ab8035e595 100644
--- a/API.txt
+++ b/API.txt
@@ -3586,7 +3586,7 @@ command: trustdomain_mod
 output: Output('summary', (, ), None)
 output: Output('value', , None)
 command: user_add
-args: 1,35,3
+args: 1,36,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -3621,6 +3621,7 @@ command: user_add
 option: Str('telephonenumber', attribute=True, cli_name='phone', multivalue=True, required=False)
 option: Str('title', attribute=True, cli_name='title', multivalue=False, required=False)
 option: Int('uidnumber', attribute=True, cli_name='uid', minvalue=1, multivalue=False, required=False)
+option: Str('userclass', attribute=True, cli_name='class', multivalue=True, required=False)
 option: Password('userpassword', attribute=True, cli_name='password', exclude='webui', multivalue=False, required=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
@@ -3649,7 +3650,7 @@ command: user_enable
 output: Output('summary', (, ), None)
 output: Output('value', , None)
 command: user_find
-args: 1,45,4
+args: 1,46,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('carlicense', attribute=True, autofill=False, cli_name='carlicense', multivalue=False, query=True, required=False)
@@ -3693,6 +3694,7 @@ command: user_find
 option: Str('title', attribute=True, autofill=False, cli_name='title', multivalue=False, query=True, required=False)
 option: Str('uid', attribute=True, autofill=False, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=False)
 option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', minvalue=1, multivalue=False, query=True, required=False)
+option: Str('userclass', attribute=True, autofill=False, cli_name='class', multivalue=True, query=True, required=False)
 option: Password('userpassword', attribute=True, autofill=False, cli_name='password', exclude='webui', multivalue=False, query=True, required=False)
 option: Str('version?', exclude='webui')
 option: Flag('whoami', autofill=True, default=False)
@@ -3701,7 +3703,7 @@ command: user_find
 output: Output('summary', (, ), None)
 output: Output('truncated', , None)
 command: user_mod
-args: 1,36,3
+args: 1,37,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude=

[Freeipa-devel] [PATCH] 0079 Enable Retro Changelog and Content Synchronization DS plugins

2013-10-25 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3967.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From c09bba5cc537d2925d7d6498204a403931ba908e Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 25 Oct 2013 12:41:25 +0200
Subject: [PATCH] Enable Retro Changelog and Content Synchronization DS plugins

Enable Retro Changelog and Content Synchronization DS plugins which are required
for SyncRepl support.

Create a working directory /var/named/ipa required by bind-dyndb-ldap v4+.

https://fedorahosted.org/freeipa/ticket/3967
---
 freeipa.spec.in| 5 +
 install/updates/20-syncrepl.update | 9 +
 install/updates/Makefile.am| 1 +
 3 files changed, 15 insertions(+)
 create mode 100644 install/updates/20-syncrepl.update

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 11ae934d928370eb13f45162a13f40a9acd64b74..52f673d27e64bdfd0ab56c28a9c71adb5f4a92a9 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -383,6 +383,7 @@ rm %{buildroot}/%{_libdir}/samba/pdb/ipasam.la
 mkdir -p %{buildroot}/%{_sysconfdir}/ipa/html
 mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysrestore
 mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysupgrade
+mkdir -p %{buildroot}/%{_localstatedir}/named/ipa
 mkdir %{buildroot}%{_usr}/share/ipa/html/
 ln -s ../../../..%{_sysconfdir}/ipa/html/ffconfig.js \
 %{buildroot}%{_usr}/share/ipa/html/ffconfig.js
@@ -737,6 +738,7 @@ fi
 %{_mandir}/man1/ipa-backup.1.gz
 %{_mandir}/man1/ipa-restore.1.gz
 %{_mandir}/man1/ipa-advise.1.gz
+%attr(0700,named,named) %dir %{_localstatedir}/named/ipa
 
 %files server-trust-ad
 %{_sbindir}/ipa-adtrust-install
@@ -832,6 +834,9 @@ fi
 %endif # ONLY_CLIENT
 
 %changelog
+* Fri Oct 25 2013 Ana Krivokapic  - 3.3.90-5
+- Create /var/named/ipa needed for SyncRepl support
+
 * Fri Oct 25 2013 Martin Kosek  - 3.3.90-4
 - Remove mod_ssl conflict, it can now live with mod_nss installed
 
diff --git a/install/updates/20-syncrepl.update b/install/updates/20-syncrepl.update
new file mode 100644
index ..6901370f9cb302ff2c0c8bcc3f7b51aadef83e8e
--- /dev/null
+++ b/install/updates/20-syncrepl.update
@@ -0,0 +1,9 @@
+# Enable Retro changelog
+dn: cn=Retro Changelog Plugin,cn=plugins,cn=config
+only:nsslapd-pluginEnabled: on
+add:nsslapd-attribute: nsuniqueid:targetUniqueId
+add:nsslapd-changelogmaxage: nsslapd-changelogmaxage: 2d
+
+# Enable SyncRepl
+dn: cn=Content Synchronization,cn=plugins,cn=config
+only:nsslapd-pluginEnabled: on
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index 40c3b3c8916faa267254a29d0f458ca53201950c..09965ff9885fce93f3d15dc73b11fa210f68b163 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -22,6 +22,7 @@ app_DATA =\
 	20-indices.update		\
 	20-nss_ldap.update		\
 	20-replication.update		\
+	20-syncrepl.update		\
 	20-user_private_groups.update	\
 	20-winsync_index.update		\
 	21-replicas_container.update	\
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0078 Make sure nsds5ReplicaStripAttrs is set on agreements

2013-10-24 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3989

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From ea2d6ce79b674ddaa681fc64d1fa4de3ee308ebd Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 24 Oct 2013 20:36:30 +0200
Subject: [PATCH] Make sure nsds5ReplicaStripAttrs is set on agreements

Add nsds5ReplicaStripAttrs to the agreement LDAP entry before the agreement
is created.

https://fedorahosted.org/freeipa/ticket/3989
---
 ipaserver/install/replication.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 6269ba6862eaf37c4f8bb560b3358f8f692dec2c..4d8a4687e162155d7855e11ba5048bed2ff13fa5 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -627,6 +627,7 @@ def setup_agreement(self, a_conn, b_hostname, port=389,
 if iswinsync:
 self.setup_winsync_agmt(entry, win_subtree)
 
+entry['nsds5ReplicaStripAttrs'] = [" ".join(STRIP_ATTRS)]
 a_conn.add_entry(entry)
 
 try:
@@ -639,8 +640,6 @@ def setup_agreement(self, a_conn, b_hostname, port=389,
 # that we will have to set the memberof fixup task
 self.need_memberof_fixup = True
 
-entry['nsds5ReplicaStripAttrs'] = [" ".join(STRIP_ATTRS)]
-
 wait_for_entry(a_conn, entry)
 
 def needs_memberof_fixup(self):
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0077 Do not roll back failed client installation on server

2013-10-24 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3990

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 2ec44821d5f422c6ab20202fcdc9e1f1eaad7ab1 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 24 Oct 2013 17:43:21 +0200
Subject: [PATCH] Do not roll back failed client installation on server

In case of a failed enrollment, IPA client rolls back any changes it has made
to the system. In order to have a more debuggable setup, do not roll back these
changes in the case of an IPA server install.

https://fedorahosted.org/freeipa/ticket/3990
---
 ipa-client/ipa-install/ipa-client-install | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index cf27788f8c189721a1f644fa5841466abfbca54e..1f66ae5d635d98ba45df13d92ca7982068d94752 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -2606,6 +2606,11 @@ def main():
 if options.force:
 root_logger.warning(
 "Installation failed. Force set so not rolling back changes.")
+elif options.on_master:
+root_logger.warning(
+"Installation failed. As this is IPA server, changes will not "
+"be rolled back."
+)
 else:
 root_logger.error("Installation failed. Rolling back changes.")
 options.unattended = True
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0076 Add test for external CA installation

2013-10-22 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3819

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 0bc373fe9a8d6f7ed1cc2cd82bf9a69f0a8613d5 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Tue, 22 Oct 2013 20:00:18 +0200
Subject: [PATCH] Add test for external CA installation

https://fedorahosted.org/freeipa/ticket/3819
---
 ipatests/test_integration/test_external_ca.py | 107 ++
 1 file changed, 107 insertions(+)
 create mode 100644 ipatests/test_integration/test_external_ca.py

diff --git a/ipatests/test_integration/test_external_ca.py b/ipatests/test_integration/test_external_ca.py
new file mode 100644
index ..747990cff5d93a481a8032ed0e7be13cdb6f8d55
--- /dev/null
+++ b/ipatests/test_integration/test_external_ca.py
@@ -0,0 +1,107 @@
+# Authors:
+#   Ana Krivokapic 
+#
+# Copyright (C) 2013  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 <http://www.gnu.org/licenses/>.
+import os
+
+from ipatests.test_integration import tasks
+from ipatests.test_integration.base import IntegrationTest
+
+
+class TestExternalCA(IntegrationTest):
+"""
+Test of FreeIPA server installation with exernal CA
+"""
+def test_external_ca(self):
+# Step 1 of ipa-server-install
+self.master.run_command([
+'ipa-server-install', '-U',
+'-a', self.master.config.admin_password,
+'-p', self.master.config.dirman_password,
+'--setup-dns', '--no-forwarders',
+'-r', self.master.domain.name,
+'--external-ca'
+])
+
+nss_db = os.path.join(self.master.config.test_dir, 'testdb')
+external_cert_file = os.path.join(nss_db, 'ipa.crt')
+external_ca_file = os.path.join(nss_db, 'ca.crt')
+noisefile = os.path.join(self.master.config.test_dir, 'noise.txt')
+pwdfile = os.path.join(self.master.config.test_dir, 'pwdfile.txt')
+
+# Create noise and password files for NSS database
+self.master.run_command('date | sha256sum > %s' % noisefile)
+self.master.run_command('echo %s > %s' %
+(self.master.config.admin_password, pwdfile))
+
+# Create NSS database
+self.master.run_command(['mkdir', nss_db])
+self.master.run_command([
+'certutil', '-N',
+'-d', nss_db,
+'-f', pwdfile
+])
+
+# Create external CA
+self.master.run_command([
+'certutil', '-S',
+'-d', nss_db,
+'-f', pwdfile,
+'-n', 'external',
+'-s', 'CN=External CA, O=%s' % self.master.domain.name,
+'-x',
+'-t', 'CTu,CTu,CTu',
+'-g', '2048',
+'-m', '0',
+'-v', '60',
+'-z', noisefile,
+'-2', '-1', '-5'
+], stdin_text='5\n9\nn\ny\n10\ny\n5\n6\n7\n9\nn\n')
+
+# Sign IPA cert request using the external CA
+self.master.run_command([
+'certutil', '-C',
+'-d', nss_db,
+'-f', pwdfile,
+'-c', 'external',
+'-m', '1',
+'-v', '60',
+'-2', '-1', '-5',
+'-i', '/root/ipa.csr',
+'-o', external_cert_file,
+'-a'
+], stdin_text='0\n1\n5\n9\ny\ny\n\ny\n5\n6\n7\n9\nn\n')
+
+# Export external CA file
+self.master.run_command(
+'certutil -L -d %s -n "external" -a > %s' %
+(nss_db, external_ca_file)
+)
+
+# Step 2 of ipa-server-install
+self.master.run_command([
+'ipa-server-install',
+'-a', self.master.config.admin_password,
+'-p', self.master.config.dirman_password,
+'--external_cert_file', external_cert_file,
+'--external_ca_file', external_ca_file
+])
+
+# Make sure IPA server is working properly
+tasks.kinit_admin(self.master)
+result = self.master.run_command(['ipa', 'user-show', 'admin'])
+assert 'User login: admin' in result.stdout_text
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] Old tickets to close

2013-10-21 Thread Ana Krivokapic
On 10/21/2013 09:49 AM, Martin Kosek wrote:
> On 10/21/2013 08:33 AM, Petr Spacek wrote:
>> On 21.10.2013 01:42, Nathan Kinder wrote:
>>> On 10/20/2013 01:33 PM, Simo Sorce wrote:
>>>> I was searching for a ticket in the depth of the backlog when I found
>>>> some tickets we should probably just close as they are not relevant
>>>> anymore or have been superseded by alternative implementations.
>>>>
>>>> If nobody objects I'd like to close the follwing tickets:
>>>>
>>>> #582 - simply old and no more relevant
> +1
>
>>>> #232 - superseded by #3859 (or reference from #3859 ?)
>>>> #233 - superseded by #3859 (or reference from #3859 ?)
> I would rather close the old ones and refer to the new ticket, just for the
> case of easier ticket manipupation.
>
>>>> #192 - it seem we settled, do we still want to do this ?
>>> I don't think that there a big need to do this anymore.  We've added more 
>>> and
>>> more capabilities to the FreeIPA password plug-in over time. In some ways,
>>> it's nice to have a separate password plug-in since  just for FreeIPA as 
>>> we've
>>> added all of these other capabilities that are really FreeIPA specific (such
>>> as OTP).  If we wanted to leverage a 389 DS password policy plug-in that 
>>> also
>>> allows us to plug-in all of these other password related capabilities, the
>>> (not yet developed) 389 DS password policy API might have to be fairly 
>>> complex.
> +1
>
>>>> #191 - cryptic, do we still want to do something here ?
>> Are we speaking about Web UI, right? IMHO a search capability is necessary if
>> you have thousands of users. You simply can't use paging to find a 1 user 
>> from
>> 10 000. (This assumes that people with 10 000 users use Web UI...)
> I would not close this one, I think it could prove useful in future. 
> Currently,
> Web UI cannot search only on a specific user entry, AFAIR.
>
>>>> #126 - I think we should close WONTFIX
>> I agree. We have how-to for people who insist on chroot.
> +1
>
>>>> #908 - I do not think we care much for the openssl dependency anymore
>>>> #2133 - superseded by #3007
>>> Agreed.
> Ok.
>
> Thanks for the archeological work you did, Simo.
>
> Martin
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Found another one:

#2766 - superseeded by #3797

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0274 test_simple_replication: Fix waiting for replication

2013-10-18 Thread Ana Krivokapic
On 09/13/2013 06:24 PM, Petr Viktorin wrote:
> The "simple replication" test is failing intermittently. It's quite hard to
> manually verify if this patch fixes that completely, but my testing says it
> does make a positive difference.
>
> See commit message for details.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

The simple replication test seems to work fine with the patch applied, the code
changes look sane, so ACK from me.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0288 Use a user result template in tests

2013-10-18 Thread Ana Krivokapic
On 09/30/2013 05:05 PM, Petr Viktorin wrote:
> Hello,
>
> This patch introduces an user "template" with the result of a default user
> add/show. The template is then customized and used in each test.
>
> This makes the tests shorter, and highlights the "non-default" (interesting)
> pieces of the result instead of presenting a wall of text.
>
> Also, when a new default attribute is added to user results (as is the case in
> my upcoming ACI patches), there's now only one place to change.
>
>
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK with two tiny nitpicks:

+Attributes named in ``omit`` are removed from the result; any additional
+or non-default values can be specified in``overrides``.
 ^ missing space
+"""
+# sn can be None; this should only used from `get_admin_result`

... this should only *be* used ...



-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0075 Add ipa-advise plugins for nss-pam-ldapd legacy clients

2013-10-18 Thread Ana Krivokapic
On 10/18/2013 01:31 PM, Ana Krivokapic wrote:
> On 10/18/2013 09:48 AM, Martin Kosek wrote:
>> On 10/17/2013 10:29 PM, Alexander Bokovoy wrote:
>>> On Thu, 17 Oct 2013, Ana Krivokapic wrote:
>>>
>>>> Hello,
>>>>
>>>> This patch adds ipa-advise plugins for configuring legacy clients using
>>>> nss-pam-ldapd.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/3672
>>> Thanks. Looks good. I have one comment below
>>>
>>>> +class config_freebsd_nss_pam_ldapd(config_base_legacy_client):
>>>> +"""
>>>> +Legacy client configuration for FreeBSD, using nss-pam-ldapd.
>>>> +"""
>>>> +description = ('Instructions for configuring a FreeBSD system with '
>>>> +   'nss-pam-ldapd. ')
>>>> +
>>>> +def get_info(self):
>>>> +uri, base = self.get_uri_and_base()
>>>> +cacrt = '/usr/local/etc/ipatest.crt'
>>> Is the cert file name is correct? 'ipatest.crt'? Perhaps 'ipaca.crt'
>>> would be a better name?
>> Or simply ipa.crt since it is the filename used everywhere else...
>>
>> Martin
> Cert file name changed to ipa.crt.
>
> Comment added about AES not being available on RHEL5.
>
> Updated patch attached.
>
>
>
> _______
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

This updated patch improves the note about possible issues regarding encryption
algorithms.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 2e84774f3a2045c48bbb3b229c9292793505681c Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 17 Oct 2013 21:58:00 +0200
Subject: [PATCH] Add ipa-advise plugins for nss-pam-ldapd legacy clients

Add three new ipa-advise plugins, to facilitate configuration of
legacy clients using nss-pam-ldapd:

* config-redhat-nss-pam-ldapd
* config-generic-linux-nss-pam-ldapd
* config-freebsd-nss-pam-ldapd

https://fedorahosted.org/freeipa/ticket/3672
---
 install/share/advise/legacy/Makefile.am|   4 +-
 ...nf.template => pam.conf.nss_pam_ldapd.template} |   8 +-
 .../{pam.conf.template => pam.conf.sssd.template}  |   0
 install/share/advise/legacy/pam_conf_sshd.template |  25 +++
 install/share/advise/legacy/sssd.conf.template |   4 +-
 ipaserver/advise/plugins/legacy_clients.py | 212 +++--
 6 files changed, 232 insertions(+), 21 deletions(-)
 copy install/share/advise/legacy/{pam.conf.template => pam.conf.nss_pam_ldapd.template} (79%)
 rename install/share/advise/legacy/{pam.conf.template => pam.conf.sssd.template} (100%)
 create mode 100644 install/share/advise/legacy/pam_conf_sshd.template

diff --git a/install/share/advise/legacy/Makefile.am b/install/share/advise/legacy/Makefile.am
index 73cd2718c343b2f3382a92f0ec8b19fb29a15c58..412185171beca7a74fcebd8bfd5cded8ca0ea62c 100644
--- a/install/share/advise/legacy/Makefile.am
+++ b/install/share/advise/legacy/Makefile.am
@@ -3,7 +3,9 @@ NULL =
 appdir = $(IPA_DATA_DIR)/advise/legacy
 app_DATA =\
 	sssd.conf.template		\
-	pam.conf.template		\
+	pam.conf.sssd.template		\
+	pam.conf.nss_pam_ldapd.template		\
+	pam_conf_sshd.template		\
 	$(NULL)
 
 EXTRA_DIST =\
diff --git a/install/share/advise/legacy/pam.conf.template b/install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
similarity index 79%
copy from install/share/advise/legacy/pam.conf.template
copy to install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
index bdd91821eb6d8259d7f03a6eac78fc264b0cafa8..9c60c27ef6c299a1ad2776b87ae4551fca3e1b64 100644
--- a/install/share/advise/legacy/pam.conf.template
+++ b/install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
@@ -1,22 +1,22 @@
 authrequired  pam_env.so
 authsufficientpam_unix.so nullok try_first_pass
 authrequisite pam_succeed_if.so uid >= 500 quiet
-authsufficientpam_sss.so use_first_pass
+authsufficientpam_ldap.so use_first_pass
 authrequired  pam_deny.so
 
 account required  pam_unix.so broken_shadow
 account sufficientpam_localuser.so
 account sufficientpam_succeed_if.so uid < 500 quiet
-account [default=bad success=ok user_unknown=ignore] pam_sss.so
+account [default=bad success=ok user_unknown=ignore] pam_ldap.so
 account required  pam_permit.so
 
 passwordrequisite pam_cracklib.so try_first_pass retry=3 type=
 passwordsufficientpam_unix.so sha512 shadow nullok try_first_pass use_authtok
-passwordsufficientpam_sss.so use_authtok
+passwordsufficientpam_ldap.so use_authtok
 pa

Re: [Freeipa-devel] [PATCH] 0075 Add ipa-advise plugins for nss-pam-ldapd legacy clients

2013-10-18 Thread Ana Krivokapic
On 10/18/2013 09:48 AM, Martin Kosek wrote:
> On 10/17/2013 10:29 PM, Alexander Bokovoy wrote:
>> On Thu, 17 Oct 2013, Ana Krivokapic wrote:
>>
>>> Hello,
>>>
>>> This patch adds ipa-advise plugins for configuring legacy clients using
>>> nss-pam-ldapd.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3672
>> Thanks. Looks good. I have one comment below
>>
>>> +class config_freebsd_nss_pam_ldapd(config_base_legacy_client):
>>> +"""
>>> +Legacy client configuration for FreeBSD, using nss-pam-ldapd.
>>> +"""
>>> +description = ('Instructions for configuring a FreeBSD system with '
>>> +   'nss-pam-ldapd. ')
>>> +
>>> +def get_info(self):
>>> +uri, base = self.get_uri_and_base()
>>> +cacrt = '/usr/local/etc/ipatest.crt'
>> Is the cert file name is correct? 'ipatest.crt'? Perhaps 'ipaca.crt'
>> would be a better name?
> Or simply ipa.crt since it is the filename used everywhere else...
>
> Martin

Cert file name changed to ipa.crt.

Comment added about AES not being available on RHEL5.

Updated patch attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From fddd4b8ff7eff505db6b632a8e770420e061b27b Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 17 Oct 2013 21:58:00 +0200
Subject: [PATCH] Add ipa-advise plugins for nss-pam-ldapd legacy clients

Add three new ipa-advise plugins, to facilitate configuration of
legacy clients using nss-pam-ldapd:

* config-redhat-nss-pam-ldapd
* config-generic-linux-nss-pam-ldapd
* config-freebsd-nss-pam-ldapd

https://fedorahosted.org/freeipa/ticket/3672
---
 install/share/advise/legacy/Makefile.am|   4 +-
 ...nf.template => pam.conf.nss_pam_ldapd.template} |   8 +-
 .../{pam.conf.template => pam.conf.sssd.template}  |   0
 install/share/advise/legacy/pam_conf_sshd.template |  25 +++
 install/share/advise/legacy/sssd.conf.template |   4 +-
 ipaserver/advise/plugins/legacy_clients.py | 213 +++--
 6 files changed, 233 insertions(+), 21 deletions(-)
 copy install/share/advise/legacy/{pam.conf.template => pam.conf.nss_pam_ldapd.template} (79%)
 rename install/share/advise/legacy/{pam.conf.template => pam.conf.sssd.template} (100%)
 create mode 100644 install/share/advise/legacy/pam_conf_sshd.template

diff --git a/install/share/advise/legacy/Makefile.am b/install/share/advise/legacy/Makefile.am
index 73cd2718c343b2f3382a92f0ec8b19fb29a15c58..412185171beca7a74fcebd8bfd5cded8ca0ea62c 100644
--- a/install/share/advise/legacy/Makefile.am
+++ b/install/share/advise/legacy/Makefile.am
@@ -3,7 +3,9 @@ NULL =
 appdir = $(IPA_DATA_DIR)/advise/legacy
 app_DATA =\
 	sssd.conf.template		\
-	pam.conf.template		\
+	pam.conf.sssd.template		\
+	pam.conf.nss_pam_ldapd.template		\
+	pam_conf_sshd.template		\
 	$(NULL)
 
 EXTRA_DIST =\
diff --git a/install/share/advise/legacy/pam.conf.template b/install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
similarity index 79%
copy from install/share/advise/legacy/pam.conf.template
copy to install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
index bdd91821eb6d8259d7f03a6eac78fc264b0cafa8..9c60c27ef6c299a1ad2776b87ae4551fca3e1b64 100644
--- a/install/share/advise/legacy/pam.conf.template
+++ b/install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
@@ -1,22 +1,22 @@
 authrequired  pam_env.so
 authsufficientpam_unix.so nullok try_first_pass
 authrequisite pam_succeed_if.so uid >= 500 quiet
-authsufficientpam_sss.so use_first_pass
+authsufficientpam_ldap.so use_first_pass
 authrequired  pam_deny.so
 
 account required  pam_unix.so broken_shadow
 account sufficientpam_localuser.so
 account sufficientpam_succeed_if.so uid < 500 quiet
-account [default=bad success=ok user_unknown=ignore] pam_sss.so
+account [default=bad success=ok user_unknown=ignore] pam_ldap.so
 account required  pam_permit.so
 
 passwordrequisite pam_cracklib.so try_first_pass retry=3 type=
 passwordsufficientpam_unix.so sha512 shadow nullok try_first_pass use_authtok
-passwordsufficientpam_sss.so use_authtok
+passwordsufficientpam_ldap.so use_authtok
 passwordrequired  pam_deny.so
 
 session optional  pam_keyinit.so revoke
 session required  pam_limits.so
 session [success=1 default=ignore] pam_succeed_if.so service in crond quiet use_uid
 session required  pam_unix.so
-session optional  pam_sss.so
+session optional  pam_ldap.so
diff --git a/install/share/advise/legacy/pam.conf.template b/install/share/advise/legacy/pam.conf.sssd.tem

[Freeipa-devel] [PATCH] 0075 Add ipa-advise plugins for nss-pam-ldapd legacy clients

2013-10-17 Thread Ana Krivokapic
Hello,

This patch adds ipa-advise plugins for configuring legacy clients using
nss-pam-ldapd.

https://fedorahosted.org/freeipa/ticket/3672

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From faa1247c6fd954aadcf7f4ea47a20e106e672bab Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 17 Oct 2013 21:58:00 +0200
Subject: [PATCH] Add ipa-advise plugins for nss-pam-ldapd legacy clients

Add three new ipa-advise plugins, to facilitate configuration of
legacy clients using nss-pam-ldapd:

* config-redhat-nss-pam-ldapd
* config-generic-linux-nss-pam-ldapd
* config-freebsd-nss-pam-ldapd

https://fedorahosted.org/freeipa/ticket/3672
---
 install/share/advise/legacy/Makefile.am|   4 +-
 ...nf.template => pam.conf.nss_pam_ldapd.template} |   8 +-
 .../{pam.conf.template => pam.conf.sssd.template}  |   0
 install/share/advise/legacy/pam_conf_sshd.template |  25 +++
 install/share/advise/legacy/sssd.conf.template |   4 +-
 ipaserver/advise/plugins/legacy_clients.py | 199 +++--
 6 files changed, 219 insertions(+), 21 deletions(-)
 copy install/share/advise/legacy/{pam.conf.template => pam.conf.nss_pam_ldapd.template} (79%)
 rename install/share/advise/legacy/{pam.conf.template => pam.conf.sssd.template} (100%)
 create mode 100644 install/share/advise/legacy/pam_conf_sshd.template

diff --git a/install/share/advise/legacy/Makefile.am b/install/share/advise/legacy/Makefile.am
index 73cd2718c343b2f3382a92f0ec8b19fb29a15c58..412185171beca7a74fcebd8bfd5cded8ca0ea62c 100644
--- a/install/share/advise/legacy/Makefile.am
+++ b/install/share/advise/legacy/Makefile.am
@@ -3,7 +3,9 @@ NULL =
 appdir = $(IPA_DATA_DIR)/advise/legacy
 app_DATA =\
 	sssd.conf.template		\
-	pam.conf.template		\
+	pam.conf.sssd.template		\
+	pam.conf.nss_pam_ldapd.template		\
+	pam_conf_sshd.template		\
 	$(NULL)
 
 EXTRA_DIST =\
diff --git a/install/share/advise/legacy/pam.conf.template b/install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
similarity index 79%
copy from install/share/advise/legacy/pam.conf.template
copy to install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
index bdd91821eb6d8259d7f03a6eac78fc264b0cafa8..9c60c27ef6c299a1ad2776b87ae4551fca3e1b64 100644
--- a/install/share/advise/legacy/pam.conf.template
+++ b/install/share/advise/legacy/pam.conf.nss_pam_ldapd.template
@@ -1,22 +1,22 @@
 authrequired  pam_env.so
 authsufficientpam_unix.so nullok try_first_pass
 authrequisite pam_succeed_if.so uid >= 500 quiet
-authsufficientpam_sss.so use_first_pass
+authsufficientpam_ldap.so use_first_pass
 authrequired  pam_deny.so
 
 account required  pam_unix.so broken_shadow
 account sufficientpam_localuser.so
 account sufficientpam_succeed_if.so uid < 500 quiet
-account [default=bad success=ok user_unknown=ignore] pam_sss.so
+account [default=bad success=ok user_unknown=ignore] pam_ldap.so
 account required  pam_permit.so
 
 passwordrequisite pam_cracklib.so try_first_pass retry=3 type=
 passwordsufficientpam_unix.so sha512 shadow nullok try_first_pass use_authtok
-passwordsufficientpam_sss.so use_authtok
+passwordsufficientpam_ldap.so use_authtok
 passwordrequired  pam_deny.so
 
 session optional  pam_keyinit.so revoke
 session required  pam_limits.so
 session [success=1 default=ignore] pam_succeed_if.so service in crond quiet use_uid
 session required  pam_unix.so
-session optional  pam_sss.so
+session optional  pam_ldap.so
diff --git a/install/share/advise/legacy/pam.conf.template b/install/share/advise/legacy/pam.conf.sssd.template
similarity index 100%
rename from install/share/advise/legacy/pam.conf.template
rename to install/share/advise/legacy/pam.conf.sssd.template
diff --git a/install/share/advise/legacy/pam_conf_sshd.template b/install/share/advise/legacy/pam_conf_sshd.template
new file mode 100644
index ..488f4998bae5ea95849a84f17a305d6fd8d1c872
--- /dev/null
+++ b/install/share/advise/legacy/pam_conf_sshd.template
@@ -0,0 +1,25 @@
+# PAM configuration for the "sshd" service
+#
+
+# auth
+authsufficient  pam_opie.so no_warn no_fake_prompts
+authrequisite   pam_opieaccess.so   no_warn allow_local
+#auth   sufficient  pam_krb5.so no_warn try_first_pass
+#auth   sufficient  pam_ssh.so  no_warn try_first_pass
+authsufficient  /usr/local/lib/pam_ldap.so  no_warn
+authrequiredpam_unix.so no_warn try_first_pass
+
+# account
+account requiredpam_nologin.so
+#accountrequiredpam_krb5.so
+account required/usr/local/lib/pam_ldap.so  no_warn ignore_authinfo_unavail ignore_unknown_us

Re: [Freeipa-devel] [PATCHES] 0072-0074 Add automember rebuild membership to the web UI

2013-10-16 Thread Ana Krivokapic

On 09/27/2013 04:38 PM, Petr Vobornik wrote:

On 09/25/2013 11:51 AM, Ana Krivokapic wrote:

Hello,

This patch set addresses ticket 
https://fedorahosted.org/freeipa/ticket/3928.


Patch 0072 hooks the new automember-rebuild command to the web UI 
(user and host

pages).
Patch 0073 adds some fixes to the web UI test driver, which are 
necessary for

patch 0074.
Patch 0074 adds web UI integration tests for the new feature.

The patch set applies on top of my patches 0068-0071



patch 72: Me, Ana and Kyle discussed position of the actions and the 
decision was to move them to action list.


`ipa automember-rebuild --type=hostgroup` can't be run from UI 
(already discussed with Ana as well)


I'm thinking about adding/creating refresh facet policies for this 
action so that appropriate facet would be marked as dirty and 
therefore refreshed so user would not have click association facet 
refresh button.


patch 73: Looks OK but some changes might not be needed.

patch 74: I would use different methods for certain actions to be 
consistent with other tests and to make the test more robust against 
Web UI refactoring:


1.

self.click_on_link('Refresh')


For buttons in .facet-controls use rather 
`self.facet_button_click('refresh')` where 'refresh' is the button 
name, not text.


2.

self.add_record(
'automember',
{'pkey': 'webservers', 'add': [
('selectbox', 'key', 'fqdn'),
('textbox', 'automemberinclusiveregex', '^web[1-9]+')
]},
facet='hostgrouprule',
facet_btn_css_class='widget',
navigate=False
)


use `add_table_record('automemberinclusiveregex', data, parent)`. 
Example in test_dns.py:94.


3. A nitpick(or not even that): When working on one entity, I would 
rather use `navigate_by_breadcrumb('link text') instead of direct 
`navigate_to_record` call which changes url. It resembles user's 
behavior more. But it depends on situation. Ie. when I'm on hostgroup 
page and want to go to host search page, but the last host page was 
some details or association I may just use `navigate_to_entity('host',
'search'). Anyway, the important thing is to avoid navigating to the 
same url twice in a row - IE10 does not like that.


4. Do not use `wait(1)` for AJAX calls. The test will fail if there is 
bigger delay. User rather `wait_for_request(n=X)` where X is number of 
AJAX calls you want to wait for.


5. Instead of `click_on_link('Rebuild auto membership')` you should 
use `action_panel_action(panel_name, action)` There is also similar 
call for

executing action list action: `action_list_action(action_name)`.

6. Nitpick: you can reduce the cleanup code:

self.navigate_by_menu('identity/host')
self.delete('host', [{'pkey': host1}])
self.delete('host', [{'pkey': host2}])


Can be written as:

self.delete('host', [{'pkey': host1}, {'pkey': host2}])

or
self.navigate_by_menu('identity/host')
self.delete_record('host', [host1, host2])

`delete` is basically a shortcut for `navigate_to_entity` and 
`delete_record` with CRUD data format.


All fixed, new patches attached.

--
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

>From 7559aeda6632e39c0bd912c567d7eccc5c7710f2 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 25 Sep 2013 11:29:31 +0200
Subject: [PATCH] Add automember rebuild command to the web UI

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3928
---
 install/ui/src/freeipa/automember.js | 44 
 install/ui/src/freeipa/host.js   | 18 ++-
 install/ui/src/freeipa/user.js   | 16 +++--
 install/ui/test/data/ipa_init.json   | 10 
 ipalib/plugins/internal.py   | 10 
 5 files changed, 87 insertions(+), 11 deletions(-)

diff --git a/install/ui/src/freeipa/automember.js b/install/ui/src/freeipa/automember.js
index f8083b89e49ec270ed6170f9826ef4cd45e08865..2dff34227351c582d028a43bc71bc2fe6a7d3142 100644
--- a/install/ui/src/freeipa/automember.js
+++ b/install/ui/src/freeipa/automember.js
@@ -699,12 +699,55 @@ IPA.automember.default_group_widget = function(spec) {
 return that;
 };
 
+IPA.automember.rebuild_action = function(spec) {
+
+spec = spec || {};
+spec.name = spec.name || 'automember_rebuild';
+spec.label = spec.label || '@i18n:actions.automember_rebuild';
+
+var that = IPA.action(spec);
+
+that.execute_action = function(facet) {
+var entity = facet.entity.name;
+if (facet.name == 'search') {
+   

Re: [Freeipa-devel] [PATCHES] 0068-0070 Automember rebuild membership

2013-10-15 Thread Ana Krivokapic
On 09/30/2013 10:02 AM, Petr Viktorin wrote:
> On 09/27/2013 03:12 PM, Martin Kosek wrote:
>> On 09/27/2013 03:00 PM, Jan Cholasta wrote:
>>> On 23.9.2013 19:41, Ana Krivokapic wrote:
>>>> On 09/19/2013 03:29 PM, Ana Krivokapic wrote:
>> ...
>>> Patch 69:
>>>
>>> I think the changes in the update file should be also done in the
>>> right LDIF
>>> files in install/share, though I don't know what is the recent
>>> consensus on this.
>>>
>>>
>>> Honza
>>>
>>
>> Last time I checked, we used to do the change both in LDIF and update
>> file. Just to avoid the LDIF become obsolete.
>>
>> Martin
>
> Rob recently said his preference is to move everything from LDIF to updates,
> and out of the the LDIF files:
> http://www.redhat.com/archives/freeipa-devel/2013-September/msg00106.html
>
> I would agree, having two places with the same information is redundant and
> error-prone.
>

Thanks Honza for the review.

I incorporated your suggestions in this updated patchset. I attached all the
patches for more convenient reviewing, but only patches 68 and 70 have changed.

I haven't done any changes in the LDIF files since the consensus seems to be not
to do that.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 12426d8945396468bf7ecb85ccd6a6ddab4630d2 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 19 Sep 2013 14:01:58 +0200
Subject: [PATCH] Add automember rebuild command

Add a new command to IPA CLI: ipa automember-rebuild

The command integrates the automember rebuild membership task functionality
into IPA CLI. It makes it possible to rebuild automember membership for
groups/hostgroups.

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3752
---
 API.txt  |   9 
 VERSION  |   2 +-
 ipalib/plugins/automember.py | 118 +++
 3 files changed, 118 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index 40871f6a8b105a7b161df34ce4f6feaf785a6107..05c5a988b7738e20e6351f6b3a01026f61cb6063 100644
--- a/API.txt
+++ b/API.txt
@@ -199,6 +199,15 @@ command: automember_mod
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: Output('value', , None)
+command: automember_rebuild
+args: 0,4,3
+option: Str('hosts*')
+option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup'))
+option: Str('users*')
+option: Str('version?', exclude='webui')
+output: Output('result', , None)
+output: Output('summary', (, ), None)
+output: Output('value', , None)
 command: automember_remove_condition
 args: 1,8,5
 arg: Str('cn', cli_name='automember_rule')
diff --git a/VERSION b/VERSION
index c3c6d5a4c28991839a1917f18d2804475a16bcb7..32f6efbc4d4768c77c514a3367cb9feb039205e5 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=65
+IPA_API_VERSION_MINOR=66
diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index 4f563f11953dafac0d5bfd00f0309aca2f346f81..b30182d9ee6545d08b0f57fbf7733f11a17dcb6f 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -16,13 +16,11 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-from ipalib import api, errors
-from ipalib import Str, StrEnum
+import uuid
+import ldap as _ldap
+from ipalib import api, errors, Str, StrEnum, _, ngettext
 from ipalib.plugins.baseldap import *
-from ipalib import _, ngettext
 from ipalib.request import context
-import ldap as _ldap
 from ipapython.dn import DN
 
 __doc__ = _("""
@@ -184,14 +182,17 @@ class automember(LDAPObject):
 ),
 )
 
-def dn_exists(self, grouptype, groupname, *keys):
+def dn_exists(self, otype, oname):
 ldap = self.api.Backend.ldap2
-dn = self.api.Object[grouptype].get_dn(groupname)
+dn = self.api.Object[otype].get_dn(oname)
 try:
-(gdn, entry_attrs) = ldap.get_entry(dn, [])
+entry = ldap.get_entry(dn, [])
 except errors.NotFound:
-raise errors.NotFound(reason=_(u'Group: %s not found!') % groupname)
-return gdn
+raise errors.NotFound(
+reason=_(u'%(otype)s "%

Re: [Freeipa-devel] [PATCH] 451-458 Web UI devel and source code documentation

2013-10-15 Thread Ana Krivokapic
On 10/02/2013 02:20 PM, Petr Vobornik wrote:
> On 09/16/2013 05:24 PM, Ana Krivokapic wrote:
>> On 09/11/2013 12:44 PM, Petr Vobornik wrote:
>>> Hello,
>>>
>
> snip
>
>>>
>>
>> I looked into the documentation effort and (ruby dependency discussion 
>> aside) I
>> don't have any major objections. I like how the generated pages look, and 
>> they
>> are intuitive and easy to navigate.
>>
>> A couple of nitpicks:
>>
>> 1) There are some spelling mistakes (e.g. Apllication_controller)
> Fixed
>>
>> 2) Bulleted lists are not rendered nicely in the html output (see for example
>> the doc string for _base.Builder property 'string_mode'. I think a list 
>> needs to
>> look like this in the source code:
>>
>>  /**
>>   * This is a list:
>>   *
>>   * - 'element1'
>>   * - 'element2'
>>   *
>>   */
>>
>> as opposed to this:
>>
>>  /**
>>   * This is a list:
>>   * - 'element1'
>>   * - 'element2'
>>   */
>>
>
> Fixed on many places. Also fixed the same issue in some code examples.
>
> - _base.Builder doc was heavily revised
> - added doc comments to ./plugin_loader
>
> All patches are rebased but just patches 452 and 453 are changed.

Looks good, ACK.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 448 Load updated Web UI files after server upgrade

2013-10-15 Thread Ana Krivokapic
On 10/02/2013 12:45 PM, Petr Vobornik wrote:
> On 09/27/2013 09:16 AM, Ana Krivokapic wrote:
>> On 08/30/2013 05:21 PM, Petr Vobornik wrote:
>
> snip
>
>>>
>>> https://fedorahosted.org/freeipa/ticket/3798
>>>
>>
>> I tested the patch and it seems to work fine. Code-wise it looks good as 
>> well.
>>
>> Nitpick: There is an unused function 'updated()' in the new loader.js.in 
>> file.
>>
>
> Unused function removed. Updated patch attached.

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 448 Load updated Web UI files after server upgrade

2013-09-27 Thread Ana Krivokapic
On 08/30/2013 05:21 PM, Petr Vobornik wrote:
> Issue:
> * There was no caching policy specified.
> * -> Browsers use their own default policy.
> * -> After upgrade, some Web UI files might have been actualized some not.
> * -> With schema change may result into weird bugs in Web UI
>
> Solution considerations:
>
> 1. Detect server version change and hard-reload at runtime
> Detection is easy. Problem is the reload. Obvious candidate
> 'window.location.reload(true)' works in Firefox but not in Chrome because
> expected behavior when parameter is used is not in standard and therefore
> Chromium/WebKit authors did not implement it.
>
> 2. Application Cache
> HTML 5 technology which lets web apps to run offline. Besides weird issues
> with event handlers which I encountered, this would be an ideal candidate.
> Simple change of manifest file would lead to reload of all files (requires
> reload of page to used the new files).
>
> Showstopper was usage with untrusted certificate. If user did not add
> exception for the cert or its CA and would visit the page for a second time,
> all AJAX calls would fail.
>
> 3. Set Expires to now() for everything
> Web UI rarely changes so this is an overkill. Setting it to different value is
> not a solution either. We can't predict when the upgrade will happen and when
> new Web UI will be needed.
>
> Solution:
> * Implemented a mini loader which loads basic resources. Dojo loader takes
> action after Dojo is loaded.
> * The loader adds a version parameter (?v=__NUM_VERSION__) to all requests.
> * Version is defined in the loader. It's set to current in `make 
> version-update`.
> * All static pages use this loader to fetch their resources.
> * Version is also passed to dojo loader as cache-bust for the same effect.
> * Expire header was set to 'access time plus 1 year' for /ui folder.
> Exceptions are HTML files and loader (set to immediate expiration).
>
> Possible issues:
> * Images are cached but not requested with version param.
>   * Images with version and without are considered different
>   * -> We would have to attach version to all URIs - in CSS and in JS. But we
> should avoid changing jQuery UI CSS.
>   * Proposed solution is to change image name when changing image. Image
> change is done rarely.
> * Version is set by build and therefore updated just on server update. It
> might cause trouble with different update schedule of plugins.
>   * No action taken to address this issue yet.
>   * We might leave it on plugin devs (own .conf in /etc/httpd/conf.d/)
>   * or set expires to now for all plugins
> * running `make version-update` is required in order to use static version of
> UI for testing
>
> https://fedorahosted.org/freeipa/ticket/3798
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

I tested the patch and it seems to work fine. Code-wise it looks good as well.

Nitpick: There is an unused function 'updated()' in the new loader.js.in file.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 459 Allow edit of ipakrbokasdelegate in Web UI when attrlevelrights are unknown

2013-09-25 Thread Ana Krivokapic
On 09/25/2013 05:44 PM, Petr Vobornik wrote:
> On 09/25/2013 02:17 PM, Ana Krivokapic wrote:
>> On 09/24/2013 04:31 PM, Petr Vobornik wrote:
>>> Old host entries are missing object class with krbticketflags attribute.
>>> Therefore UI does not receive attrlevelrights for it. This OC is added when
>>> ipakrbokasdelegate(krbticketflags) is set.
>>>
>>> This patch adds the usual hack for such cases.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3940
>>
>> The patch works well. Could you please also add a regression test?
>>
>
> Test modified. Sadly, the test doesn't cover old entries after server upgrade,
> which are the reason for the UI hack.

ACK!

(There's an unused variable (pkey) at the start of the test_kerberos_flags()
method - you may just remove it before pushing.)

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 459 Allow edit of ipakrbokasdelegate in Web UI when attrlevelrights are unknown

2013-09-25 Thread Ana Krivokapic
On 09/24/2013 04:31 PM, Petr Vobornik wrote:
> Old host entries are missing object class with krbticketflags attribute.
> Therefore UI does not receive attrlevelrights for it. This OC is added when
> ipakrbokasdelegate(krbticketflags) is set.
>
> This patch adds the usual hack for such cases.
>
> https://fedorahosted.org/freeipa/ticket/3940
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

The patch works well. Could you please also add a regression test?

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCHES] 0072-0074 Add automember rebuild membership to the web UI

2013-09-25 Thread Ana Krivokapic
Hello,

This patch set addresses ticket https://fedorahosted.org/freeipa/ticket/3928.

Patch 0072 hooks the new automember-rebuild command to the web UI (user and host
pages).
Patch 0073 adds some fixes to the web UI test driver, which are necessary for
patch 0074.
Patch 0074 adds web UI integration tests for the new feature.

The patch set applies on top of my patches 0068-0071

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 917c0bbd3ac45ff307f899dd50d5d44fac68c6e9 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 25 Sep 2013 11:38:07 +0200
Subject: [PATCH] Add web UI integration tests for automember rebuild

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3928
---
 ipatests/test_webui/test_automember.py | 213 +
 1 file changed, 213 insertions(+)

diff --git a/ipatests/test_webui/test_automember.py b/ipatests/test_webui/test_automember.py
index f51e5d9b1fed593e1522307ab538020121fd95fa..dc664e49e25b4854cd182581e42a3250f32bdf1d 100644
--- a/ipatests/test_webui/test_automember.py
+++ b/ipatests/test_webui/test_automember.py
@@ -81,3 +81,216 @@ def test_crud(self):
 
 # cleanup
 self.delete(hostgroup.ENTITY, [hostgroup.DATA])
+
+def test_rebuild_membership_hosts(self):
+"""
+Test automember rebuild membership feature for hosts
+"""
+self.init_app()
+
+domain = self.config.get('ipa_domain')
+host1 = 'web1.%s' % domain
+host2 = 'web2.%s' % domain
+
+# Add a hostgroup
+self.add_record('hostgroup', {
+'pkey': 'webservers',
+'add': [
+('textbox', 'cn', 'webservers'),
+('textarea', 'description', 'webservers'),
+]
+})
+
+# Add a host
+self.add_record('host', {
+'pkey': host1,
+'add': [
+('textbox', 'hostname', 'web1'),
+('combobox', 'dnszone', domain),
+('checkbox', 'force', 'checked'),
+]
+})
+
+# Add another host
+self.add_record('host', {
+'pkey': host2,
+'add': [
+('textbox', 'hostname', 'web2'),
+('combobox', 'dnszone', domain),
+('checkbox', 'force', 'checked'),
+]
+})
+
+# Add an automember rule
+self.add_record(
+'automember',
+{'pkey': 'webservers', 'add': [('combobox', 'cn', 'webservers')]},
+facet='searchhostgroup'
+)
+
+# Add a condition for automember rule
+self.navigate_to_record('webservers')
+self.add_record(
+'automember',
+{'pkey': 'webservers', 'add': [
+('selectbox', 'key', 'fqdn'),
+('textbox', 'automemberinclusiveregex', '^web[1-9]+')
+]},
+facet='hostgrouprule',
+facet_btn_css_class='widget',
+navigate=False
+)
+
+# Assert that hosts are not members of hostgroup
+self.navigate_to_record('webservers', entity='hostgroup')
+self.click_on_link('Refresh')
+self.wait(1)
+self.assert_record(host1, negative=True)
+self.assert_record(host2, negative=True)
+
+# Rebuild membership for first host, using action on host details facet
+self.navigate_to_record(host1, entity='host')
+self.click_on_link('Rebuild auto membership')
+self.dialog_button_click('ok')
+
+# Assert that host is now a member of hostgroup
+self.navigate_to_record('webservers', entity='hostgroup')
+self.click_on_link('Refresh')
+self.wait(1)
+self.assert_record(host1)
+self.assert_record(host2, negative=True)
+
+# Remove host from hostgroup
+self.delete_record(host1)
+
+# Assert that host is not a member of hostgroup
+self.click_on_link('Refresh')
+self.wait(1)
+self.assert_record(host1, negative=True)
+self.assert_record(host2, negative=True)
+
+# Rebuild membership for all hosts, using action on hosts search facet
+self.navigate_by_menu('iden

Re: [Freeipa-devel] [PATCHES] 0068-0070 Automember rebuild membership

2013-09-23 Thread Ana Krivokapic
On 09/19/2013 03:29 PM, Ana Krivokapic wrote:
> Hello,
>
> This patch set adds automember rebuild membership functionality to IPA CLI.
>
> Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
> Ticket: https://fedorahosted.org/freeipa/ticket/3752
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

This updated patch set introduces only one automember-rebuild command, instead
of three. The changes are covered in the design document:
http://www.freeipa.org/page/V3/Automember_rebuild_membership#CLI.

Also, while working on these patches, I hit this bug again:
https://fedorahosted.org/freeipa/ticket/2708. Since the fix is pretty
straightforward, and the bug is related to automember feature, I included a fix
for it in this patch set (patch 0071).

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 9ceeec11494ddb11dc95d15e0a89597e3423ff38 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 19 Sep 2013 14:01:58 +0200
Subject: [PATCH] Add automember rebuild command

Add a new command to IPA CLI: ipa automember-rebuild

The command integrates the automember rebuild membership task functionality
into IPA CLI. It makes it possible to rebuild automember membership for
groups/hostgroups.

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3752
---
 API.txt  |   9 +++
 VERSION  |   2 +-
 ipalib/plugins/automember.py | 129 +++
 3 files changed, 129 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index 761d1d175b5ce48bb6e27ce60e404f89790bfe6b..bd51634c4f564d7043fb86398b7cabbe1af745bf 100644
--- a/API.txt
+++ b/API.txt
@@ -199,6 +199,15 @@ command: automember_mod
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: Output('value', , None)
+command: automember_rebuild
+args: 0,4,3
+option: Str('hosts', multivalue=True, required=False)
+option: StrEnum('type', required=False, values=(u'group', u'hostgroup'))
+option: Str('users', multivalue=True, required=False)
+option: Str('version?', exclude='webui')
+output: Output('result', , None)
+output: Output('summary', (, ), None)
+output: Output('value', , None)
 command: automember_remove_condition
 args: 1,8,5
 arg: Str('cn', cli_name='automember_rule')
diff --git a/VERSION b/VERSION
index c3c6d5a4c28991839a1917f18d2804475a16bcb7..32f6efbc4d4768c77c514a3367cb9feb039205e5 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=65
+IPA_API_VERSION_MINOR=66
diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index 4f563f11953dafac0d5bfd00f0309aca2f346f81..308f3cdcba5aae8af018aea2f77e09da7541e3e6 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -16,14 +16,13 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-from ipalib import api, errors
-from ipalib import Str, StrEnum
+import uuid
+import ldap as _ldap
+from ipalib import api, errors, Str, StrEnum, _, ngettext
 from ipalib.plugins.baseldap import *
-from ipalib import _, ngettext
 from ipalib.request import context
-import ldap as _ldap
 from ipapython.dn import DN
+from ipapython.ipautil import realm_to_suffix
 
 __doc__ = _("""
 Auto Membership Rule.
@@ -184,14 +183,17 @@ class automember(LDAPObject):
 ),
 )
 
-def dn_exists(self, grouptype, groupname, *keys):
+def dn_exists(self, otype, oname):
 ldap = self.api.Backend.ldap2
-dn = self.api.Object[grouptype].get_dn(groupname)
+dn = self.api.Object[otype].get_dn(oname)
 try:
-(gdn, entry_attrs) = ldap.get_entry(dn, [])
+(odn, entry_attrs) = ldap.get_entry(dn, [])
 except errors.NotFound:
-raise errors.NotFound(reason=_(u'Group: %s not found!') % groupname)
-return gdn
+raise errors.NotFound(
+reason=_(u'%(otype)s: %(oname)s not found!') %
+dict(otype=otype, oname=oname)
+)
+return odn
 
 def get_dn(self, *keys, **options):
 if self.parent_object:
@@ -587,3 +589,110 @@ def execute(self, *keys, **options):
 return result
 
 api.register(automember_default_group_show)
+
+
+cl

Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-23 Thread Ana Krivokapic
On 09/23/2013 09:57 AM, Jan Cholasta wrote:
> On 23.9.2013 09:18, Martin Kosek wrote:
>> On 09/19/2013 03:43 PM, Ana Krivokapic wrote:
>>> On 09/19/2013 03:26 PM, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 12.9.2013 19:59, Ana Krivokapic wrote:
>>>>> Hello,
>>>>>
>>>>> The design document for $SUBJECT can be found at:
>>>>> http://www.freeipa.org/page/V3/Automember_rebuild_membership
>>>>>
>>>>> Related tickets:
>>>>> https://fedorahosted.org/freeipa/ticket/3752
>>>>> https://fedorahosted.org/freeipa/ticket/3928
>>>>>
>>>>> Thoughts, comments, questions welcome.
>>>>>
>>>>
>>>> I don't think naming the commands user-automember-rebuild and
>>>> host-automember-rebuild commands is correct. The names imply they are 
>>>> methods
>>>> of user/host, but they don't directly do anything to user/host objects. I
>>>> would prefer if they were kept in the automember namespace where they
>>>> logically belong (automember-rebuild-user and automember-rebuild-host
>>>> perhaps?)
>>>>
>>>> Honza
>>>>
>>>
>>> That makes sense... I don't have a strong preference one way or other. So if
>>> other agree with this suggestion, I will change it.
>>
>> I think Honza's comment makes sense. We can merge the functionality to
>> automember-rebuild command:
>>
>> $ ipa automember-rebuild --type=group [ENTRY]
>> $ ipa automember-rebuild --type=hostgroup [ENTRY]
>>
>> If no ENTRY is specified, it would run rebuild for all entries. If ENTRY is
>> specified, it would use it as Primary Key the entry - user uid or group name.
>>
>> This way the API should be consistent with the rest of the automember plugin.
>>
>> Makes sense?
>
> Yes, but I think the "--type=group " part might be confusing. What
> about:
>
> $ ipa automember-rebuild --type=group --users  --users  ...
> $ ipa automember-rebuild --type=hostgroup --hosts  --hosts  ...
>
> ?
>
> The --users and --hosts parameters are inspired by group-add-member and
> hostgroup-add-member. Also, the value of --type can be inferred, so it does
> not have to be explicitly specified:
>
> $ ipa automember-rebuild --users  --users  ...
> $ ipa automember-rebuild --hosts  --hosts  ...
>
>>
>> Martin
>>
>
>

I like this suggestion, I updated the design page accordingly:
http://www.freeipa.org/page/V3/Automember_rebuild_membership#CLI

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-20 Thread Ana Krivokapic
On 09/20/2013 04:25 PM, JR Aquino wrote:
> Great work on this.
>
> I've longed to revisit my code and provide a way to refresh/update.
>
> I think it got left off with rich and Nathan as something that should be a 
> 389 plugin mod.
>
> Thanks for working on this!
>
> "You cannot hope to secure that which you do not understand"
> ~
> Jr Aquino
> Sr. Information Security Specialist, Technical Operations
> Sans: GXPN, GCIH, GWAPT
> T: +1 805 690 3478 | M: +1 805 717 0365 | F: +1 805 403 9399
> jr.aqu...@citrix.com
>  
> Powering mobile workstyles and cloud services
>
>> On Sep 20, 2013, at 3:37 AM, "Ana Krivokapic"  wrote:
>>
>>> On 09/19/2013 07:34 PM, JR Aquino wrote:
>>> Does the rebuild support the notion of members belonging to multiple groups 
>>> via automember rules?
>> Yes, all memberships are rebuilt.
>>
>>> "You cannot hope to secure that which you do not first understand"
>>> ~
>>> Jr Aquino | Sr. Information Security Specialist
>>> GXPN | GIAC Exploit Researcher and Advanced Penetration Tester
>>> GCIH | GIAC Certified Incident Handler
>>> GWAPT | GIAC WebApp Penetration Tester
>>>
>>> Citrix Online | 7408 Hollister Avenue | Goleta, CA 
>>> 93117
>>> T:  +1 805.690.3478
>>> C: +1 805.717.0365
>>> jr.aqu...@citrix.com<mailto:jr.aqu...@citrixonline.com>
>>> http://www.citrixonline.com<http://www.citrixonline.com/>
>>>
>>> On Sep 19, 2013, at 6:43 AM, Ana Krivokapic 
>>> mailto:akriv...@redhat.com>> wrote:
>>>
>>> On 09/19/2013 03:26 PM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 12.9.2013 19:59, Ana Krivokapic wrote:
>>> Hello,
>>>
>>> The design document for $SUBJECT can be found at:
>>> http://www.freeipa.org/page/V3/Automember_rebuild_membership
>>>
>>> Related tickets:
>>> https://fedorahosted.org/freeipa/ticket/3752
>>> https://fedorahosted.org/freeipa/ticket/3928
>>>
>>> Thoughts, comments, questions welcome.
>>>
>>>
>>> I don't think naming the commands user-automember-rebuild and
>>> host-automember-rebuild commands is correct. The names imply they are 
>>> methods
>>> of user/host, but they don't directly do anything to user/host objects. I
>>> would prefer if they were kept in the automember namespace where they
>>> logically belong (automember-rebuild-user and automember-rebuild-host 
>>> perhaps?)
>>>
>>> Honza
>>>
>>>
>>> That makes sense... I don't have a strong preference one way or other. So if
>>> other agree with this suggestion, I will change it.
>>>
>>> --
>>> Regards,
>>>
>>> Ana Krivokapic
>>> Associate Software Engineer
>>> FreeIPA team
>>> Red Hat Inc.
>>>
>>> ___
>>> Freeipa-devel mailing list
>>> Freeipa-devel@redhat.com<mailto:Freeipa-devel@redhat.com>
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>> -- 
>> Regards,
>>
>> Ana Krivokapic
>> Associate Software Engineer
>> FreeIPA team
>> Red Hat Inc.
>>

You are welcome, I'm glad you find it useful! :)

BTW, patches are already on the list (minus the web UI part - coming soon), you
can check them out if you want:
https://www.redhat.com/archives/freeipa-devel/2013-September/msg00295.html

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-20 Thread Ana Krivokapic
On 09/19/2013 07:34 PM, JR Aquino wrote:
> Does the rebuild support the notion of members belonging to multiple groups 
> via automember rules?

Yes, all memberships are rebuilt.

>
> "You cannot hope to secure that which you do not first understand"
> ~
> Jr Aquino | Sr. Information Security Specialist
> GXPN | GIAC Exploit Researcher and Advanced Penetration Tester
> GCIH | GIAC Certified Incident Handler
> GWAPT | GIAC WebApp Penetration Tester
>
> Citrix Online | 7408 Hollister Avenue | Goleta, CA 
> 93117
> T:  +1 805.690.3478
> C: +1 805.717.0365
> jr.aqu...@citrix.com<mailto:jr.aqu...@citrixonline.com>
> http://www.citrixonline.com<http://www.citrixonline.com/>
>
> On Sep 19, 2013, at 6:43 AM, Ana Krivokapic 
> mailto:akriv...@redhat.com>> wrote:
>
> On 09/19/2013 03:26 PM, Jan Cholasta wrote:
> Hi,
>
> On 12.9.2013 19:59, Ana Krivokapic wrote:
> Hello,
>
> The design document for $SUBJECT can be found at:
> http://www.freeipa.org/page/V3/Automember_rebuild_membership
>
> Related tickets:
> https://fedorahosted.org/freeipa/ticket/3752
> https://fedorahosted.org/freeipa/ticket/3928
>
> Thoughts, comments, questions welcome.
>
>
> I don't think naming the commands user-automember-rebuild and
> host-automember-rebuild commands is correct. The names imply they are methods
> of user/host, but they don't directly do anything to user/host objects. I
> would prefer if they were kept in the automember namespace where they
> logically belong (automember-rebuild-user and automember-rebuild-host 
> perhaps?)
>
> Honza
>
>
> That makes sense... I don't have a strong preference one way or other. So if
> other agree with this suggestion, I will change it.
>
> --
> Regards,
>
> Ana Krivokapic
> Associate Software Engineer
> FreeIPA team
> Red Hat Inc.
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com<mailto:Freeipa-devel@redhat.com>
> https://www.redhat.com/mailman/listinfo/freeipa-devel
>


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 109] Use getent admin@domain for nss check in, ipa-client-install

2013-09-19 Thread Ana Krivokapic
On 09/19/2013 04:33 PM, Tomas Babej wrote:
> On 09/19/2013 11:59 AM, Tomas Babej wrote:
>> Hi,
>>
>> Use 'getent admin@domain' rather than 'getent admin@REALM' to check if nss
>> is working properly since admin@REALM check fails in case the domain and the
>> realm
>> name does not match.
>>
>> https://fedorahosted.org/freeipa/ticket/3906
>>
>>
>
> Thanks to Ana for telling me I basically copied Stephen's patch:
>
> https://www.redhat.com/archives/freeipa-devel/2013-September/msg00085.html
>
> And I did the same mistake. Fixed patched attached.
>
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-19 Thread Ana Krivokapic
On 09/19/2013 03:26 PM, Jan Cholasta wrote:
> Hi,
>
> On 12.9.2013 19:59, Ana Krivokapic wrote:
>> Hello,
>>
>> The design document for $SUBJECT can be found at:
>> http://www.freeipa.org/page/V3/Automember_rebuild_membership
>>
>> Related tickets:
>> https://fedorahosted.org/freeipa/ticket/3752
>> https://fedorahosted.org/freeipa/ticket/3928
>>
>> Thoughts, comments, questions welcome.
>>
>
> I don't think naming the commands user-automember-rebuild and
> host-automember-rebuild commands is correct. The names imply they are methods
> of user/host, but they don't directly do anything to user/host objects. I
> would prefer if they were kept in the automember namespace where they
> logically belong (automember-rebuild-user and automember-rebuild-host 
> perhaps?)
>
> Honza
>

That makes sense... I don't have a strong preference one way or other. So if
other agree with this suggestion, I will change it.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCHES] 0068-0070 Automember rebuild membership

2013-09-19 Thread Ana Krivokapic
Hello,

This patch set adds automember rebuild membership functionality to IPA CLI.

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
Ticket: https://fedorahosted.org/freeipa/ticket/3752

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 7794da2f8666b1cf98937ba66a34d50df0eb62b9 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 19 Sep 2013 14:01:58 +0200
Subject: [PATCH] Add automember rebuild commands

Add the following commands to IPA CLI:
- ipa automember-rebuild
- ipa user-automember-rebuild
- ipa host-automember-rebuild

The commands integrate the automember rebuild membership task functionality
into IPA CLI. They make it possible to rebuild automember membership for
groups/hostgroups.

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3752
---
 API.txt  | 21 +
 VERSION  |  2 +-
 ipalib/plugins/automember.py | 43 ++
 ipalib/plugins/host.py   | 55 +++-
 ipalib/plugins/user.py   | 43 +-
 5 files changed, 146 insertions(+), 18 deletions(-)

diff --git a/API.txt b/API.txt
index 761d1d175b5ce48bb6e27ce60e404f89790bfe6b..73d9246c4ae4a2b7da791d1f544d6e0e1d383b72 100644
--- a/API.txt
+++ b/API.txt
@@ -199,6 +199,13 @@ command: automember_mod
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: Output('value', , None)
+command: automember_rebuild
+args: 0,2,3
+option: StrEnum('type', values=(u'group', u'hostgroup'))
+option: Str('version?', exclude='webui')
+output: Output('result', , None)
+output: Output('summary', (, ), None)
+output: Output('value', , None)
 command: automember_remove_condition
 args: 1,8,5
 arg: Str('cn', cli_name='automember_rule')
@@ -1732,6 +1739,13 @@ command: host_add_managedby
 output: Output('completed', , None)
 output: Output('failed', , None)
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+command: host_automember_rebuild
+args: 1,1,3
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True)
+option: Str('version?', exclude='webui')
+output: Output('result', , None)
+output: Output('summary', (, ), None)
+output: Output('value', , None)
 command: host_del
 args: 1,2,3
 arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=True, primary_key=True, query=True, required=True)
@@ -3538,6 +3552,13 @@ command: user_add
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: Output('value', , None)
+command: user_automember_rebuild
+args: 1,1,3
+arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
+option: Str('version?', exclude='webui')
+output: Output('result', , None)
+output: Output('summary', (, ), None)
+output: Output('value', , None)
 command: user_del
 args: 1,2,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
diff --git a/VERSION b/VERSION
index c3c6d5a4c28991839a1917f18d2804475a16bcb7..32f6efbc4d4768c77c514a3367cb9feb039205e5 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=65
+IPA_API_VERSION_MINOR=66
diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index 4f563f11953dafac0d5bfd00f0309aca2f346f81..3ec82e1268620fb6498257a20a5580d16323f1e9 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -16,14 +16,13 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-from ipalib import api, errors
-from ipalib import Str, StrEnum
+import uuid
+import ldap as _ldap
+from ipalib import api, errors, Str, StrEnum, _, ngettext
 from ipalib.plugins.baseldap import *
-from ipalib import _, ngettext
 

Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-19 Thread Ana Krivokapic
On 09/19/2013 12:01 PM, Martin Kosek wrote:
> - Original Message -
>> From: "Ana Krivokapic" 
>> To: "Nathan Kinder" 
>> Cc: "Martin Kosek" , "freeipa-devel" 
>> , "Mark Reynolds"
>> 
>> Sent: Thursday, September 19, 2013 11:00:05 AM
>> Subject: Re: [Freeipa-devel] [RFE] Support for automember rebuild membership
>>
>> On 09/18/2013 06:51 PM, Nathan Kinder wrote:
>>> On 09/18/2013 07:26 AM, Martin Kosek wrote:
>>>> On 09/18/2013 01:37 PM, Ana Krivokapic wrote:
>>>>> On 09/13/2013 10:48 AM, Martin Kosek wrote:
>>>>>> On 09/12/2013 07:59 PM, Ana Krivokapic wrote:
>>>> ...
>>>>>> I would rather add an option --dry-run or --test for all new automember
>>>>>> commands which would return how would the updated entry look like. BTW,
>>>>>> did
>>>>>> the
>>>>>> automember export updates task work for you? I tried this LDIF and
>>>>>> /tmp/automember.ldif was empty:
>>>>>>
>>>>>> dn: cn=my export task 1, cn=automember export updates,cn=tasks,cn=config
>>>>>> changetype: add
>>>>>> objectClass: top
>>>>>> objectClass: extensibleObject
>>>>>> cn: my export task 1
>>>>>> basedn: cn=accounts,dc=example,dc=com
>>>>>> filter: (uid=*)
>>>>>> scope: sub
>>>>>> ldif: /tmp/automember.ldif
>>>>>>
>>>>>> Adding Mark to CC in case if he has any advise for utilizing the export
>>>>>> task in
>>>>>> FreeIPA.
>>>>>>
>>>>>> By the way, using this approach I think we would also hit issues with
>>>>>> permissions on the resulting LDIF, given it is created by DS and would
>>>>>> be read
>>>>>> by Apache. SELinux would be complaining as well. To sum it up, I am not
>>>>>> sure
>>>>>> this will be that easy and straightforward.
>>>>> You are right about this. I haven't been able to find a way to make this
>>>>> work.
>>>>> Namely, the resulting LDIF is created by dirsrv user and is not readable
>>>>> by
>>>>> apache user. Any ideas/pointers here would be very much appreciated.
>>>> Right. I would try asking 389-ds guys, Mark, Nathan (CCed) or Rich about
>>>> that.
>>>> If we do not find a better way to transfer the resulting LDIF to Apache,
>>>> we
>>>> will need to leave the --dry-run part until we do.
>>> I don't see an easy way of solving this without changes to 389 DS that
>>> offer
>>> alternate methods for exporting the automember updates, such as making them
>>> available via LDAP.  I'm not even sure if this is a good idea since the
>>> export
>>> could be quite sizable.
>>>
>>> Another approach would be to allow the task to specify the mode to use when
>>> creating the export file.  This still isn't ideal, as you either have to
>>> open
>>> the file up to everyone, or you have to have the httpd and 389 DS users in
>>> the
>>> same group (which opens up other permission issues).
>>>
>>> Thanks,
>>> -NGK
>>>> Martin
>> Thanks Nathan for your input. I opened this ticket to allow a better way of
>> accessing results of the automember export updates task:
>> https://fedorahosted.org/389/ticket/47518. After further conversation with
>> Nathan on IRC, we both agreed that at this point, the --dry-run option seems
>> to
>> be more trouble than it's worth. So I will leave it out for now (until the
>> above
>> ticket is implemented in 389).
> ACK! Please also create FreeIPA RFE ticket pointing to the 389 ticket so that 
> we do no forget about it.
>
> Martin

https://fedorahosted.org/freeipa/ticket/3936

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-19 Thread Ana Krivokapic
On 09/18/2013 06:51 PM, Nathan Kinder wrote:
> On 09/18/2013 07:26 AM, Martin Kosek wrote:
>> On 09/18/2013 01:37 PM, Ana Krivokapic wrote:
>>> On 09/13/2013 10:48 AM, Martin Kosek wrote:
>>>> On 09/12/2013 07:59 PM, Ana Krivokapic wrote:
>> ...
>>>> I would rather add an option --dry-run or --test for all new automember
>>>> commands which would return how would the updated entry look like. BTW, did
>>>> the
>>>> automember export updates task work for you? I tried this LDIF and
>>>> /tmp/automember.ldif was empty:
>>>>
>>>> dn: cn=my export task 1, cn=automember export updates,cn=tasks,cn=config
>>>> changetype: add
>>>> objectClass: top
>>>> objectClass: extensibleObject
>>>> cn: my export task 1
>>>> basedn: cn=accounts,dc=example,dc=com
>>>> filter: (uid=*)
>>>> scope: sub
>>>> ldif: /tmp/automember.ldif
>>>>
>>>> Adding Mark to CC in case if he has any advise for utilizing the export
>>>> task in
>>>> FreeIPA.
>>>>
>>>> By the way, using this approach I think we would also hit issues with
>>>> permissions on the resulting LDIF, given it is created by DS and would be 
>>>> read
>>>> by Apache. SELinux would be complaining as well. To sum it up, I am not 
>>>> sure
>>>> this will be that easy and straightforward.
>>> You are right about this. I haven't been able to find a way to make this 
>>> work.
>>> Namely, the resulting LDIF is created by dirsrv user and is not readable by
>>> apache user. Any ideas/pointers here would be very much appreciated.
>> Right. I would try asking 389-ds guys, Mark, Nathan (CCed) or Rich about 
>> that.
>> If we do not find a better way to transfer the resulting LDIF to Apache, we
>> will need to leave the --dry-run part until we do.
> I don't see an easy way of solving this without changes to 389 DS that offer
> alternate methods for exporting the automember updates, such as making them
> available via LDAP.  I'm not even sure if this is a good idea since the export
> could be quite sizable.
>
> Another approach would be to allow the task to specify the mode to use when
> creating the export file.  This still isn't ideal, as you either have to open
> the file up to everyone, or you have to have the httpd and 389 DS users in the
> same group (which opens up other permission issues).
>
> Thanks,
> -NGK
>>
>> Martin
>

Thanks Nathan for your input. I opened this ticket to allow a better way of
accessing results of the automember export updates task:
https://fedorahosted.org/389/ticket/47518. After further conversation with
Nathan on IRC, we both agreed that at this point, the --dry-run option seems to
be more trouble than it's worth. So I will leave it out for now (until the above
ticket is implemented in 389).

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 0067 Use fqdn when creating msdcs SRV records

2013-09-18 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3908.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From e04aa1b7c19ec7cbef7ef0b8c29d6fca6a890fd5 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 18 Sep 2013 18:39:11 +0200
Subject: [PATCH] Use fqdn when creating msdcs SRV records

https://fedorahosted.org/freeipa/ticket/3908
---
 ipaserver/install/adtrustinstance.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 5839b2f176bc7680fb6d63c9b0321b271e82ef71..6aeb3cb8e686712ecbd262ba17b45f4d18af8d85 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -31,6 +31,7 @@
 from ipaserver.install.bindinstance import get_rr, add_rr, del_rr, \
dns_zone_exists
 from ipalib import errors, api
+from ipalib.util import normalize_zone
 from ipapython.dn import DN
 from ipapython import sysrestore
 from ipapython import ipautil
@@ -506,7 +507,7 @@ def __add_dns_service_records(self):
 """
 
 zone = self.domain_name
-host = self.fqdn.split(".")[0]
+host = normalize_zone(self.fqdn)
 priority = 0
 
 ipa_srv_rec = (
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-18 Thread Ana Krivokapic
On 09/13/2013 10:48 AM, Martin Kosek wrote:
> On 09/12/2013 07:59 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> The design document for $SUBJECT can be found at:
>> http://www.freeipa.org/page/V3/Automember_rebuild_membership
>>
>> Related tickets:
>> https://fedorahosted.org/freeipa/ticket/3752
>> https://fedorahosted.org/freeipa/ticket/3928
>>
>> Thoughts, comments, questions welcome.
>>
> Besides membership reset discussion happening in second thread I other 
> comments.
>
> 1) I think we should also design permission&ACIs for the automember rebuild
> task creation (cn=automember rebuild membership,cn=tasks,cn=config container).
> You can talk to Petr3 about that as he is current working on redesigning ACIs.
> Just so that you are in sync.

I discussed this with Petr. We agreed that since the new redesign of ACIs will
not be implemented for some time, I should use the old way to add the ACIs for
automember tasks. I added a section to the design document to address ACIs,
permissions and privileges:

http://www.freeipa.org/page/V3/Automember_rebuild_membership#Updates_and_Upgrades

>
> 2) About the "Implementation" part and "automember rebuild membership"
>
> I do not think this is good approach. I do not know how do you plan doing the
> confirmation
>
> I do not think this workflow would work with our framework. We do not have
> support for confirming except if you would implement it in
> interactive_prompt_callback. But then the functionality would not be available
> for Web UI.
>
> I would rather add an option --dry-run or --test for all new automember
> commands which would return how would the updated entry look like. BTW, did 
> the
> automember export updates task work for you? I tried this LDIF and
> /tmp/automember.ldif was empty:
>
> dn: cn=my export task 1, cn=automember export updates,cn=tasks,cn=config
> changetype: add
> objectClass: top
> objectClass: extensibleObject
> cn: my export task 1
> basedn: cn=accounts,dc=example,dc=com
> filter: (uid=*)
> scope: sub
> ldif: /tmp/automember.ldif
>
> Adding Mark to CC in case if he has any advise for utilizing the export task 
> in
> FreeIPA.
>
> By the way, using this approach I think we would also hit issues with
> permissions on the resulting LDIF, given it is created by DS and would be read
> by Apache. SELinux would be complaining as well. To sum it up, I am not sure
> this will be that easy and straightforward.

You are right about this. I haven't been able to find a way to make this work.
Namely, the resulting LDIF is created by dirsrv user and is not readable by
apache user. Any ideas/pointers here would be very much appreciated.

>
> Martin


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 451-458 Web UI devel and source code documentation

2013-09-16 Thread Ana Krivokapic
On 09/11/2013 12:44 PM, Petr Vobornik wrote:
> Hello,
>
> This is a part of documentation effort which started couple month ago.
> Attached patches improves devel documentation of Web UI. Mostly by annotating
> source code and then processing it by JSDuck tool[1].
>
> The documentation is not complete - most plugins and member of some core
> widgets and facets are not annotated yet. I'm sending it now because I need to
> focus on more pressing tickets.
>
> You can see current state at my fedorapeople page [2].
>
> I also converted 5 guides/articles which I wrote some time ago. Guides are
> also part of the JSDuck app [3].
>
> The idea is to regularly generate the doc and place it on docs.freeipa.org
> (not in production at the moment) so all doc would be on one place.
>
> Installation of JSDuck is documented on their page [4]. Basically it requires
> ruby and JSDuck gem.
>
> Usage:
> $ cd install/ui/doc
> $ make
>
> Documentation is generated into: install/ui/build/code_doc directory
>
> [1] https://github.com/senchalabs/jsduck
> [2] http://pvoborni.fedorapeople.org/doc
> [3] http://pvoborni.fedorapeople.org/doc/#!/guide
> [4] https://github.com/senchalabs/jsduck/wiki/Installation
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

I looked into the documentation effort and (ruby dependency discussion aside) I
don't have any major objections. I like how the generated pages look, and they
are intuitive and easy to navigate.

A couple of nitpicks:

1) There are some spelling mistakes (e.g. Apllication_controller)

2) Bulleted lists are not rendered nicely in the html output (see for example
the doc string for _base.Builder property 'string_mode'. I think a list needs to
look like this in the source code:

/**
 * This is a list:
 *
 * - 'element1'
 * - 'element2'
 *
 */

as opposed to this:

/**
 * This is a list:
 * - 'element1'
 * - 'element2'
 */

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-13 Thread Ana Krivokapic
On 09/13/2013 10:48 AM, Martin Kosek wrote:
> On 09/12/2013 07:59 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> The design document for $SUBJECT can be found at:
>> http://www.freeipa.org/page/V3/Automember_rebuild_membership
>>
>> Related tickets:
>> https://fedorahosted.org/freeipa/ticket/3752
>> https://fedorahosted.org/freeipa/ticket/3928
>>
>> Thoughts, comments, questions welcome.
>>
> Besides membership reset discussion happening in second thread I other 
> comments.
>
> 1) I think we should also design permission&ACIs for the automember rebuild
> task creation (cn=automember rebuild membership,cn=tasks,cn=config container).
> You can talk to Petr3 about that as he is current working on redesigning ACIs.
> Just so that you are in sync.
>
> 2) About the "Implementation" part and "automember rebuild membership"
>
> I do not think this is good approach. I do not know how do you plan doing the
> confirmation
>
> I do not think this workflow would work with our framework. We do not have
> support for confirming except if you would implement it in
> interactive_prompt_callback. But then the functionality would not be available
> for Web UI.
>
> I would rather add an option --dry-run or --test for all new automember
> commands which would return how would the updated entry look like. 

My plan was to use the interactive_prompt_callback for the CLI, but the approach
with --dry-run sounds better. So effectively, the --dry-run option would invoke
the automember export updates task, while the command without --dry-run would
invoke the automember rebuild membership task. In the web UI, a click on the
"rebuild membership" button/link would present the user with the list of
candidate entries (command with --dry-run is called), and then after
confirmation, the command without --dry-run would be called. Does it make sense?

> BTW, did the
> automember export updates task work for you? I tried this LDIF and
> /tmp/automember.ldif was empty:
>
> dn: cn=my export task 1, cn=automember export updates,cn=tasks,cn=config
> changetype: add
> objectClass: top
> objectClass: extensibleObject
> cn: my export task 1
> basedn: cn=accounts,dc=example,dc=com
> filter: (uid=*)
> scope: sub
> ldif: /tmp/automember.ldif
>
> Adding Mark to CC in case if he has any advise for utilizing the export task 
> in
> FreeIPA.

I came across this issue while doing initial investigation, and reported it
here: https://fedorahosted.org/389/ticket/47507. As a workaround, Mark suggested
to use a different value for the basedn parameter in the above ldif:
cn=computers,cn=accounts,dc=example,dc=com - for hosts
cn=users,cn=accounts,dc=example,dc=com - for users

This worked for me, but seemed to cause a hang of the DS server when the ldif is
executed. The issue seems to be in the schema compat plugin. Bugzillas have been
open to address this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1007451
https://bugzilla.redhat.com/show_bug.cgi?id=1007453

To work around _this_ problem, disabling the schema compat plugin seems to do
the trick:
$ ipa-compat-manage disable

To sum it up, in order to test the automember tasks, you need to:
1. Change the 'basedn' parameter in the above ldif (as described above)
2. Disable the schema compat plugin

>
> By the way, using this approach I think we would also hit issues with
> permissions on the resulting LDIF, given it is created by DS and would be read
> by Apache. SELinux would be complaining as well. To sum it up, I am not sure
> this will be that easy and straightforward.
>
> Martin


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-13 Thread Ana Krivokapic
On 09/13/2013 10:16 AM, Martin Kosek wrote:
> On 09/12/2013 09:46 PM, Dmitri Pal wrote:
>> On 09/12/2013 03:23 PM, Rob Crittenden wrote:
>>> Dmitri Pal wrote:
>>>> On 09/12/2013 01:59 PM, Ana Krivokapic wrote:
>>>>> Hello,
>>>>>
>>>>> The design document for $SUBJECT can be found at:
>>>>> http://www.freeipa.org/page/V3/Automember_rebuild_membership
>>>>>
>>>>> Related tickets:
>>>>> https://fedorahosted.org/freeipa/ticket/3752
>>>>> https://fedorahosted.org/freeipa/ticket/3928
>>>>>
>>>>> Thoughts, comments, questions welcome.
>>>>>
>>>> The names for commands are a bit long.
>>>> I am not sure we need all the commands.
>>>>
>>>> $ ipa automember-rebuild-membership --type=group
>>>>
>>>> I do not understand why type is "group".
>>>> If you say that all the user group memberships will be rebuilt then the
>>>> type is "user".
>>>> But then you can really not have the command at all and use just:
>>>>
>>>> ipa user-automembership
>>>> and
>>>> ipa host-automembership
>>>>
>>>> If in future we have other objects we would add another command for
>>>> those objects.
>>>>
>>>> so
>>>> ipa user-automembership --update
>>>> will update group memberships for all users, or may be it should be
>>>> ipa user-automembership --update *
>>>> (I do not know what are the rules in the framework, we should follow
>>>> them)
>>>>
>>>> ipa user-automember --update LOGIN
>>>> will update group memberships for a specific user
>>>>
>>>> Now we need to differentiate --update and --reset
>>>> --update should update group membership based on the existing filters,
>>>> i.e. based on the automember plugin configuration only add missing
>>>> memberships (if any)
>>>> --reset should clean existing memberships and rebuild them based only
>>>> the default groups + automember. It should pretty much mean "make group
>>>> memberships as if the user was just added".
>>>>
>>>> Makes sense or I am missing something?
>>> Her design is consistent with the current automember commands.
>> What are they?
> ipa automember-show --type=hostgroup webservers
> ipa automember-add --type=group devel
>
>>> I think I'd drop the -membership part though, automember-rebuild is
>>> sufficient. For user/host perhaps user-automember-rebuild.
> +1, this is shorter.

+1, I changed the names of commands to:
ipa automember-rebuild
ipa user-automember-rebuild
ipa host-automember-rebuild

>
>>> I disagree with your update/reset suggestions.
>> We can have a separate RFE for reset but I think it would make sense for
>> the cases when user moves from one org unit to another or moves from
>> intern to full time.
>> In this case I expect something like
>> 1) Update user "class" attribute
>> 2) Run automember reset 
> Hmm, I must say I do not like the reset option very much too. Someone may not
> realize what is the real scope or meaning of the option and find out all his
> membership is gone.
>
> Nothing prevents Administrator to explicitly remove all user membership before
> moving to other role/function. It is a matter of 3 clicks in Web UI.

I agree with Martin and Rob here. Adding the reset option would only introduce
unnecessary risk.

>
>>> Unless a user is doing ALL of its group/hostgroup management via
>>> automember rules then the reset command will almost always do the
>>> wrong thing. 
>> No it is need for the case when you need to get to the state as if this
>> user was just added without actually deleting and re-addign the user.
>> Use case like I mentioned above are examples.
>>
>>> This could raise all sorts of strange permission issues too, as we'd
>>> have to use the currently bound user to do the group membership
>>> removal. We can separately add delegation to create an automember
>>> rebuild task, and that runs as DM IIRC.
>> I agree that this would probably be a different task, like delete and
>> rebuild. This I think we should create a separate ticket for reset and
>> file and RFE with DS. It is not urgent but will become handy when we
>> start supporting user lifecycle management and provisioning
> Still not convinced this the way we should go and that life-cycle management
> would require rinsing all membership data.
>
> Martin
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [RFE] Support for automember rebuild membership

2013-09-12 Thread Ana Krivokapic
Hello,

The design document for $SUBJECT can be found at:
http://www.freeipa.org/page/V3/Automember_rebuild_membership

Related tickets:
https://fedorahosted.org/freeipa/ticket/3752
https://fedorahosted.org/freeipa/ticket/3928

Thoughts, comments, questions welcome.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] IPA-CLIENT: NSS test needs to check against the domain name

2013-09-06 Thread Ana Krivokapic
On 09/05/2013 07:34 PM, Stephen Gallagher wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> In situations where the FreeIPA server is configured with
> different domain and realm values, we will fail to test for the
> admin account in the ipa-client-install script. With this patch,
> we will correctly run 'getent passwd admin@DOMAIN'
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.14 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlIowI8ACgkQeiVVYja6o6ORiwCgnX1uVtWTktYpKdrVxuVTyQjZ
> WCsAn0ULhTR0TsfcCsEpknVwgkAN0d7F
> =KpH4
> -END PGP SIGNATURE-
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

You should also replace 'cli_realm' with 'cli_domain' in the 'if not found:'
block that follows.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 450 Fix redirection on deletion of last dns record entry

2013-09-06 Thread Ana Krivokapic
On 09/06/2013 02:30 PM, Petr Vobornik wrote:
> https://fedorahosted.org/freeipa/ticket/3907
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0066 Do not crash if DS is down during server uninstall

2013-09-06 Thread Ana Krivokapic
Hello,

This patch fixes the regression introduced by the original fix for ticket #3867.

https://fedorahosted.org/freeipa/ticket/3867

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 0ad82d43314ef7e8d9538de76b118ce0922e512d Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 6 Sep 2013 14:53:01 +0200
Subject: [PATCH] Do not crash if DS is down during server uninstall

DS is contacted during server uninstallation, in order to obtain information
about replication agreements. If DS is unavailable, warn and continue with
uninstallation.

https://fedorahosted.org/freeipa/ticket/3867
---
 install/tools/ipa-server-install | 64 +---
 1 file changed, 41 insertions(+), 23 deletions(-)
 mode change 100755 => 100644 install/tools/ipa-server-install

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
old mode 100755
new mode 100644
index 1bf932da731965b342c0c49d39e6a175bf98705b..f46e4b0548998dd5b0c8a12321ba8916ccb40f25
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -624,31 +624,49 @@ def main():
 print "Aborting uninstall operation."
 sys.exit(1)
 
-conn = ipaldap.IPAdmin(api.env.host, ldapi=True, realm=api.env.realm)
-conn.do_external_bind(pwd.getpwuid(os.geteuid()).pw_name)
-rm = replication.ReplicationManager(api.env.realm, api.env.host, None,
-conn=conn)
-agreements = rm.find_ipa_replication_agreements()
-
-if agreements:
-other_masters = [a.get('cn')[0][4:] for a in agreements]
-msg = (
-"\nReplication agreements with the following IPA masters "
-"found: %s. Removing any replication agreements before "
-"uninstalling the server is strongly recommended. You can "
-"remove replication agreements by running the following "
-"command on any other IPA master:\n" % ", ".join(other_masters)
+try:
+conn = ipaldap.IPAdmin(
+api.env.host,
+ldapi=True,
+realm=api.env.realm
 )
-cmd = "$ ipa-replica-manage del %s\n" % api.env.host
+conn.do_external_bind(pwd.getpwuid(os.geteuid()).pw_name)
+except Exception:
+msg = ("\nWARNING: Failed to connect to Directory Server to find "
+   "information about replication agreements. Uninstallation "
+   "will continue dispite the possible existing replication "
+   "agreements.\n\n")
 print textwrap.fill(msg, width=80, replace_whitespace=False)
-print cmd
-if not (options.unattended or user_input("Are you sure you want "
- "to continue with the "
- "uninstall procedure?",
- False)):
-print ""
-print "Aborting uninstall operation."
-sys.exit(1)
+else:
+rm = replication.ReplicationManager(
+realm=api.env.realm,
+hostname=api.env.host,
+dirman_passwd=None,
+conn=conn
+)
+agreements = rm.find_ipa_replication_agreements()
+
+if agreements:
+other_masters = [a.get('cn')[0][4:] for a in agreements]
+msg = (
+"\nReplication agreements with the following IPA masters "
+"found: %s. Removing any replication agreements before "
+"uninstalling the server is strongly recommended. You can "
+"remove replication agreements by running the following "
+"command on any other IPA master:\n" % ", ".join(
+other_masters)
+)
+cmd = "$ ipa-replica-manage del %s\n" % api.env.host
+print textwrap.fill(msg, width=80, replace_whitespace=False)
+print cmd
+if not (options.unattended or user_input("Are you sure you "
+ "want to continue "
+ "with the uninstall "
+ "procedure?",
+ False)):
+print ""
+print "Aborting uninstall operation."
+sys.exit(1)
 
 return uninstall()
 
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0065 Follow tmpfiles.d packaging guidelines

2013-09-04 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3881.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 21264f8da8f0fa9d01b319b33c2b18daea52198a Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 4 Sep 2013 18:18:13 +0200
Subject: [PATCH] Follow tmpfiles.d packaging guidelines

https://fedorahosted.org/freeipa/ticket/3881
---
 freeipa.spec.in | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 67bd3667db42ebb3639f50a4dd0c3a9fda000781..88c02258d6eb9eefe556ad588316c6d8c9a62180 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -423,8 +423,8 @@ install -m 644 init/ipa_memcached.conf %{buildroot}%{_sysconfdir}/sysconfig/ipa_
 mkdir -p %{buildroot}%{_usr}/share/ipa/ui/js/plugins
 
 # NOTE: systemd specific section
-mkdir -p %{buildroot}%{_sysconfdir}/tmpfiles.d/
-install -m 0644 init/systemd/ipa.conf.tmpfiles %{buildroot}%{_sysconfdir}/tmpfiles.d/ipa.conf
+mkdir -p %{buildroot}%{_prefix}/lib/tmpfiles.d
+install -m 0644 init/systemd/ipa.conf.tmpfiles %{buildroot}%{_prefix}/lib/tmpfiles.d/%{name}.conf
 # END
 
 mkdir -p %{buildroot}%{_localstatedir}/run/
@@ -611,7 +611,7 @@ fi
 %dir %attr(0700,apache,apache) %{_localstatedir}/run/ipa_memcached/
 %dir %attr(0700,root,root) %{_localstatedir}/run/ipa/
 # NOTE: systemd specific section
-%config(noreplace) %{_sysconfdir}/tmpfiles.d/ipa.conf
+%{_prefix}/lib/tmpfiles.d/%{name}.conf
 %attr(644,root,root) %{_unitdir}/ipa.service
 %attr(644,root,root) %{_unitdir}/ipa_memcached.service
 %attr(644,root,root) %{_unitdir}/ipa-otpd.socket
@@ -836,6 +836,9 @@ fi
 %endif # ONLY_CLIENT
 
 %changelog
+* Wed Sep 4 2013 Ana Krivokapic  - 3.3.90-3
+- Conform to tmpfiles.d packaging guidelines
+
 * Wed Aug 14 2013 Petr Viktorin  - 3.3.90-2
 - Add man pages to the tests subpackage
 
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0064 Fix invocations of FileError in ipa-client-install

2013-09-04 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3758.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From aa8c36c9bf5f3a349fdf319f3e08a8b56081abef Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 4 Sep 2013 16:04:59 +0200
Subject: [PATCH] Fix invocations of FileError in ipa-client-install

Some of the FileErrors in ipa-client-install were raised incorrectly
(without the 'reason' argument), which resulted in bad error messages
during ipa-client-install.

https://fedorahosted.org/freeipa/ticket/3758
---
 ipa-client/ipa-install/ipa-client-install | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 280edd793326150c416fe1b82f9866435e9c6509..17fcdf34e0578623cc1098cb5ea9bd5ff5fcb977 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1493,33 +1493,34 @@ def get_ca_cert_from_file(url):
 
 try:
 parsed = urlparse.urlparse(url, 'file')
-except Exception, e:
-raise errors.FileError("unable to parse file url '%s'" % (url))
+except Exception:
+raise errors.FileError(reason="unable to parse file url '%s'" % url)
 
 if parsed.scheme != 'file':
-raise errors.FileError("url is not a file scheme '%s'" % (url))
+raise errors.FileError(reason="url is not a file scheme '%s'" % url)
 
 filename = parsed.path
 
 if not os.path.exists(filename):
-raise errors.FileError("file '%s' does not exist" % (filename))
+raise errors.FileError(reason="file '%s' does not exist" % filename)
 
 if not os.path.isfile(filename):
-raise errors.FileError("file '%s' is not a file" % (filename))
+raise errors.FileError(reason="file '%s' is not a file" % filename)
 
 root_logger.debug("trying to retrieve CA cert from file %s", filename)
 try:
 cert = x509.load_certificate_from_file(filename)
-except Exception, e:
+except Exception:
 raise errors.NoCertificateError(entry=filename)
 
 try:
 x509.write_certificate(cert.der_data, CACERT)
 except Exception, e:
-raise errors.FileError(reason =
-u"cannot write certificate file '%s': %s" % (CACERT, e))
+raise errors.FileError(
+reason=u"cannot write certificate file '%s': %s" % (CACERT, e)
+)
 else:
-del(cert)
+del cert
 
 def get_ca_cert_from_http(url, ca_file, warn=True):
 '''
@@ -1600,7 +1601,8 @@ def validate_new_ca_cert(existing_ca_cert, ca_file, ask, override=False):
 new_ca_cert = x509.load_certificate_from_file(ca_file)
 except Exception, e:
 raise errors.FileError(
-"Unable to read new ca cert '%s': %s" % (ca_file, e))
+reason="Unable to read new ca cert '%s': %s" % (ca_file, e)
+)
 
 if existing_ca_cert is None:
 root_logger.info(
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0062 Replace ntpdate calls with ntpd

2013-09-04 Thread Ana Krivokapic
On 09/03/2013 01:15 PM, Petr Viktorin wrote:
> On 09/02/2013 05:05 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3797.
>>
>
> Thanks! I have a question.
>
>> -# retry several times -- logic follows /etc/init.d/ntpdate
>> -# implementation
>> +# retry several times
>>   for retry in range(0, 3):
>
> What's the reason to try several times? Is it still necessary with ntpd?
> I checked /etc/init.d/ntpdate in RHEL (since Fedora doesn't have it any more)
> and I didn't see any repeating.
>

I am not sure what the reason for trying several times was. My testing didn't
reveal any need for that, but I left the code there mainly because it returns
immediately after successful sync, so there is no harm.

Attached is a version of the patch without the repetition.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 4da51666f77ed2d8a1c95cfc32c52eebe0493e35 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Mon, 2 Sep 2013 16:40:55 +0200
Subject: [PATCH] Replace ntpdate calls with ntpd

Due to the upcoming deprecation of the ntpdate program (targeted for Fedora 20),
replace ntpdate calls with ntpd.

https://fedorahosted.org/freeipa/ticket/3797
---
 ipa-client/ipaclient/ntpconf.py | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/ipa-client/ipaclient/ntpconf.py b/ipa-client/ipaclient/ntpconf.py
index eb9afdeeefb24d994701b0f3e6859f69653be39d..714d4fb861d41642fea82b15a8749214a893d830 100644
--- a/ipa-client/ipaclient/ntpconf.py
+++ b/ipa-client/ipaclient/ntpconf.py
@@ -21,7 +21,6 @@
 from ipapython import services as ipaservices
 import shutil
 import os
-import sys
 
 ntp_conf = """# Permit time synchronization with our time source, but do not
 # permit the source to query or modify the service on this system.
@@ -137,24 +136,23 @@ def config_ntp(server_fqdn, fstore = None, sysstore = None):
 
 def synconce_ntp(server_fqdn):
 """
-Syncs time with specified server using ntpdate.
+Syncs time with specified server using ntpd.
 Primarily designed to be used before Kerberos setup
 to get time following the KDC time
 
 Returns True if sync was successful
 """
-ntpdate="/usr/sbin/ntpdate"
-if os.path.exists(ntpdate):
-# retry several times -- logic follows /etc/init.d/ntpdate
-# implementation
-cmd = [ntpdate, "-U", "ntp", "-s", "-b", "-v", server_fqdn]
-for retry in range(0, 3):
-try:
-ipautil.run(cmd)
-return True
-except:
-pass
-return False
+ntpd = '/usr/sbin/ntpd'
+if not os.path.exists(ntpd):
+return False
+
+tmp_ntp_conf = ipautil.write_tmp_file('server %s' % server_fqdn)
+try:
+ipautil.run([ntpd, '-qgc', tmp_ntp_conf.name])
+return True
+except ipautil.CalledProcessError:
+return False
+
 
 class NTPConfigurationError(Exception):
 pass
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0060 Add warning when uninstalling active replica

2013-09-03 Thread Ana Krivokapic
On 09/03/2013 08:25 AM, Martin Kosek wrote:
> On 09/02/2013 06:21 PM, Tomas Babej wrote:
>> On 09/02/2013 06:07 PM, Petr Viktorin wrote:
>>> On 08/29/2013 05:56 PM, Ana Krivokapic wrote:
>>>> Hello,
>>>>
>>>> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3867.
>>>>
>>> Patch works well.
>>> It's temping to restart the discussion about how to wrap text output from
>>> installation tools. Wrapping at 60 characters because it looks better in the
>>> code seems suboptimal.
>>> Does anyone remember if we established some guideline last time this came 
>>> up?
>>>
>>>
>> I'm not sure if I'm missing something, but do we need a guideline here?
>>
>> I don't see any reason why not have best of the both worlds, using print as a
>> function we can wrap the text inside the parenthesis
>> with no effect on the output whatsoever. Or use print statement, but enclose
>> the text in parenthesis. Or use backslash.
>>
> Yes. But whatever we choose, we need to make sure that the resulting text is
> wrapped the same to avoid inconsistent output.
>
> IMO we should do our best to keep the text wrapped at 80 characters in new or
> updated texts. So I would prefer to have Ana's patch refactored a bit, to
> change wrapping of the resulting from 60 to 80 characters.
>
> Martin

Text is wrapped at 80 characters in the updated patch.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From b47a751de3279de38e13a2b626c7ea003c0175a5 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 29 Aug 2013 17:44:02 +0200
Subject: [PATCH] Add warning when uninstalling active replica

Add a warning when trying to uninstall a replica that has active replication
agreements.

https://fedorahosted.org/freeipa/ticket/3867
---
 install/tools/ipa-server-install | 36 +++-
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index bfdef82ab7be1dd76efcab2514cda73445b483a9..1bf932da731965b342c0c49d39e6a175bf98705b 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -28,18 +28,17 @@
 
 import sys
 import os
-import errno
 import grp
-import subprocess
 import signal
 import shutil
-import glob
 import pickle
 import random
 import tempfile
 import nss.error
 import base64
-from optparse import OptionGroup, OptionValueError, SUPPRESS_HELP
+import pwd
+import textwrap
+from optparse import OptionGroup, OptionValueError
 
 from ipaserver.install import dsinstance
 from ipaserver.install import krbinstance
@@ -51,10 +50,11 @@ from ipaserver.install import cainstance
 from ipaserver.install import memcacheinstance
 from ipaserver.install import otpdinstance
 from ipaserver.install import sysupgrade
-
+from ipaserver.install import replication
 from ipaserver.install import service, installutils
 from ipapython import version
 from ipapython import certmonger
+from ipapython import ipaldap
 from ipaserver.install.installutils import *
 from ipaserver.plugins.ldap2 import ldap2
 
@@ -624,6 +624,32 @@ def main():
 print "Aborting uninstall operation."
 sys.exit(1)
 
+conn = ipaldap.IPAdmin(api.env.host, ldapi=True, realm=api.env.realm)
+conn.do_external_bind(pwd.getpwuid(os.geteuid()).pw_name)
+rm = replication.ReplicationManager(api.env.realm, api.env.host, None,
+conn=conn)
+agreements = rm.find_ipa_replication_agreements()
+
+if agreements:
+other_masters = [a.get('cn')[0][4:] for a in agreements]
+msg = (
+"\nReplication agreements with the following IPA masters "
+"found: %s. Removing any replication agreements before "
+"uninstalling the server is strongly recommended. You can "
+"remove replication agreements by running the following "
+"command on any other IPA master:\n" % ", ".join(other_masters)
+)
+cmd = "$ ipa-replica-manage del %s\n" % api.env.host
+print textwrap.fill(msg, width=80, replace_whitespace=False)
+print cmd
+if not (options.unattended or user_input("Are you sure you want "
+ "to continue with the "
+ "uninstall procedure?",
+ False)):
+print ""
+print "Aborting uninstall operation."
+sys.exit(1)
+
 return uninstall()
 
 if options.external_ca:
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0061 Add option to ipa-client-install to configure automount

2013-09-03 Thread Ana Krivokapic
On 09/03/2013 12:27 PM, Petr Viktorin wrote:
> On 09/02/2013 01:31 PM, Ana Krivokapic wrote:
>> On 09/02/2013 12:55 PM, Petr Viktorin wrote:
>>> On 08/30/2013 04:10 PM, Ana Krivokapic wrote:
>>>> Hello,
>>>>
>>>> The attached patch addresses ticket
>>>> https://fedorahosted.org/freeipa/ticket/3740.
>>>
>>> Hello,
>>> Please write a design doc for this RFE.
>>
>> I updated the Minor Enhancements page:
>> http://www.freeipa.org/page/V3_Minor_Enhancements. I think it is sufficient 
>> in
>> this case.
>>
>>> Also you'll need to update the ipa-client-install man page.
>>
>> Done.
>>>
>>> I wonder if `location` is too generic a name for this option.
>>> Did you think about `--automount-location`,
>>
>> Good point, I changed `--location` to `--automount-location`.
>>
>>> plus maybe `--automount` without argument to just use the "default" 
>>> location?
>>> It's a bit longer but it would make it immediately clear what the option is
>>> about.
>>>
>>
>> I think this is a bit of an overkill, as "--automount-location=default" does
>> precisely that. I would rather not complicate things further by adding more
>> options.
>>
>> Thanks for the review, updated patch is attached.
>>
>
> Looks good! One more comment for usability.
> The man page should explain that --automount-location configures automount by
> running ipa-client-automount(1).
>
>

Fixed in updated patch.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 17818950350f189202b4e1a9dfac870d53b58d20 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 30 Aug 2013 16:05:01 +0200
Subject: [PATCH] Add option to ipa-client-install to configure automount

Add the --automount-location option to ipa-client-install. If the option is
used, ipa-client-automount is called at the end of ipa-client-install.

https://fedorahosted.org/freeipa/ticket/3740
---
 ipa-client/ipa-install/ipa-client-install | 27 +++
 ipa-client/man/ipa-client-install.1   |  6 +-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 280edd793326150c416fe1b82f9866435e9c6509..3bfd5a4455c786d638a8ab1984f780dd537a103d 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -148,6 +148,8 @@ def parse_options():
 # only, it isn't meant to be used on clients.
 basic_group.add_option("--on-master", dest="on_master", action="store_true",
   help=SUPPRESS_HELP, default=False)
+basic_group.add_option("--automount-location", dest="location",
+   help="Automount location")
 parser.add_option_group(basic_group)
 
 sssd_group = OptionGroup(parser, "SSSD options")
@@ -1288,6 +1290,28 @@ def configure_sshd_config(fstore, options):
 except Exception, e:
 log_service_error(sshd.service_name, 'restart', e)
 
+
+def configure_automount(options):
+root_logger.info('\nConfiguring automount:')
+
+args = [
+'ipa-client-automount', '--debug', '-U',
+'--location', options.location
+]
+
+if options.server:
+args.extend(['--server', options.server[0]])
+if not options.sssd:
+args.append('--no-sssd')
+
+try:
+stdout, _, _ = run(args)
+except Exception, e:
+root_logger.error('Automount configuration failed: %s', str(e))
+else:
+root_logger.info(stdout)
+
+
 def resolve_ipaddress(server):
 """ Connect to the server's LDAP port in order to determine what ip
 address this machine uses as "public" ip (relative to the server).
@@ -2515,6 +2539,9 @@ def install(options, env, fstore, statestore):
 if options.conf_sshd:
 configure_sshd_config(fstore, options)
 
+if options.location:
+configure_automount(options)
+
 root_logger.info('Client configuration complete.')
 
 return 0
diff --git a/ipa-client/man/ipa-client-install.1 b/ipa-client/man/ipa-client-install.1
index bb19041b13622e3384fb800fca60b7b6f695e8f0..65c5c2f73913ce8ae705f3b1ff46f16c7eb4c5c6 100644
--- a/ipa-client/man/ipa-client-install.1
+++ b/ipa-client/man/ipa-client-install.1
@@ -146,13 +146,17 @@ Print debugging information to stdout
 \fB\-U\fR, \fB\-\-unattended\fR
 Unattended installation. The user will not be prompted.
 .TP
-\fB\-\-ca-cert-file\fR=\fICA_FILE\fR
+\fB\-\-ca\-cert\-file\fR=\fICA_FILE\fR
 Do

[Freeipa-devel] [PATCH] 0063 Do not show unexpected error in ipa-ldap-updater

2013-09-03 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3825.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From f71bace581340667d27a0ad0f1b47eb2b7b9d870 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Tue, 3 Sep 2013 12:42:48 +0200
Subject: [PATCH] Do not show unexpected error in ipa-ldap-updater

Prevent showing of unfriendly "Unexpected error" message, when providing
incorrect DM password to ipa-ldap-updater.

https://fedorahosted.org/freeipa/ticket/3825
---
 ipaserver/install/installutils.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 268279dc9d22b9f983406303cbfc80c00a2b8fa0..67c3fa9befbeb9ecce3c7d53ba582b3d81f73b56 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -673,6 +673,8 @@ def handle_error(error, log_file_name=None):
 if isinstance(error, socket.error):
 return error, 1
 
+if isinstance(error, errors.ACIError):
+return error.message, 1
 if isinstance(error, ldap.INVALID_CREDENTIALS):
 return "Invalid password", 1
 if isinstance(error, ldap.INSUFFICIENT_ACCESS):
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0062 Replace ntpdate calls with ntpd

2013-09-02 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3797.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From cfbf21ce24c677712d8f74a29ff5896cb2ad9396 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Mon, 2 Sep 2013 16:40:55 +0200
Subject: [PATCH] Replace ntpdate calls with ntpd

Due to the upcoming deprecation of the ntpdate program (targeted for Fedora 20),
replace ntpdate calls with ntpd.

https://fedorahosted.org/freeipa/ticket/3797
---
 ipa-client/ipaclient/ntpconf.py | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/ipa-client/ipaclient/ntpconf.py b/ipa-client/ipaclient/ntpconf.py
index eb9afdeeefb24d994701b0f3e6859f69653be39d..f7d76764abe4f13fa57de362d9e2701ac9cc861b 100644
--- a/ipa-client/ipaclient/ntpconf.py
+++ b/ipa-client/ipaclient/ntpconf.py
@@ -21,7 +21,6 @@
 from ipapython import services as ipaservices
 import shutil
 import os
-import sys
 
 ntp_conf = """# Permit time synchronization with our time source, but do not
 # permit the source to query or modify the service on this system.
@@ -137,25 +136,26 @@ def config_ntp(server_fqdn, fstore = None, sysstore = None):
 
 def synconce_ntp(server_fqdn):
 """
-Syncs time with specified server using ntpdate.
+Syncs time with specified server using ntpd.
 Primarily designed to be used before Kerberos setup
 to get time following the KDC time
 
 Returns True if sync was successful
 """
-ntpdate="/usr/sbin/ntpdate"
-if os.path.exists(ntpdate):
-# retry several times -- logic follows /etc/init.d/ntpdate
-# implementation
-cmd = [ntpdate, "-U", "ntp", "-s", "-b", "-v", server_fqdn]
+ntpd = '/usr/sbin/ntpd'
+if os.path.exists(ntpd):
+tmp_ntp_conf = ipautil.write_tmp_file('server %s' % server_fqdn)
+
+# retry several times
 for retry in range(0, 3):
 try:
-ipautil.run(cmd)
+ipautil.run([ntpd, '-qgc', tmp_ntp_conf.name])
 return True
-except:
+except ipautil.CalledProcessError:
 pass
 return False
 
+
 class NTPConfigurationError(Exception):
 pass
 
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0061 Add option to ipa-client-install to configure automount

2013-09-02 Thread Ana Krivokapic
On 09/02/2013 12:55 PM, Petr Viktorin wrote:
> On 08/30/2013 04:10 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> The attached patch addresses ticket
>> https://fedorahosted.org/freeipa/ticket/3740.
>
> Hello,
> Please write a design doc for this RFE.

I updated the Minor Enhancements page:
http://www.freeipa.org/page/V3_Minor_Enhancements. I think it is sufficient in
this case.

> Also you'll need to update the ipa-client-install man page.

Done.

>
> I wonder if `location` is too generic a name for this option.
> Did you think about `--automount-location`,

Good point, I changed `--location` to `--automount-location`.

> plus maybe `--automount` without argument to just use the "default" location?
> It's a bit longer but it would make it immediately clear what the option is
> about.
>

I think this is a bit of an overkill, as "--automount-location=default" does
precisely that. I would rather not complicate things further by adding more 
options.

Thanks for the review, updated patch is attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From c05660356c50924e5ebf54cd6eb2585aab5f4090 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 30 Aug 2013 16:05:01 +0200
Subject: [PATCH] Add option to ipa-client-install to configure automount

Add the --automount-location option to ipa-client-install. If the option is
used, ipa-client-automount is called at the end of ipa-client-install.

https://fedorahosted.org/freeipa/ticket/3740
---
 ipa-client/ipa-install/ipa-client-install | 27 +++
 ipa-client/man/ipa-client-install.1   |  5 -
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 280edd793326150c416fe1b82f9866435e9c6509..3bfd5a4455c786d638a8ab1984f780dd537a103d 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -148,6 +148,8 @@ def parse_options():
 # only, it isn't meant to be used on clients.
 basic_group.add_option("--on-master", dest="on_master", action="store_true",
   help=SUPPRESS_HELP, default=False)
+basic_group.add_option("--automount-location", dest="location",
+   help="Automount location")
 parser.add_option_group(basic_group)
 
 sssd_group = OptionGroup(parser, "SSSD options")
@@ -1288,6 +1290,28 @@ def configure_sshd_config(fstore, options):
 except Exception, e:
 log_service_error(sshd.service_name, 'restart', e)
 
+
+def configure_automount(options):
+root_logger.info('\nConfiguring automount:')
+
+args = [
+'ipa-client-automount', '--debug', '-U',
+'--location', options.location
+]
+
+if options.server:
+args.extend(['--server', options.server[0]])
+if not options.sssd:
+args.append('--no-sssd')
+
+try:
+stdout, _, _ = run(args)
+except Exception, e:
+root_logger.error('Automount configuration failed: %s', str(e))
+else:
+root_logger.info(stdout)
+
+
 def resolve_ipaddress(server):
 """ Connect to the server's LDAP port in order to determine what ip
 address this machine uses as "public" ip (relative to the server).
@@ -2515,6 +2539,9 @@ def install(options, env, fstore, statestore):
 if options.conf_sshd:
 configure_sshd_config(fstore, options)
 
+if options.location:
+configure_automount(options)
+
 root_logger.info('Client configuration complete.')
 
 return 0
diff --git a/ipa-client/man/ipa-client-install.1 b/ipa-client/man/ipa-client-install.1
index bb19041b13622e3384fb800fca60b7b6f695e8f0..fd9a1d5250e1451b553624172722405b22c1f519 100644
--- a/ipa-client/man/ipa-client-install.1
+++ b/ipa-client/man/ipa-client-install.1
@@ -146,13 +146,16 @@ Print debugging information to stdout
 \fB\-U\fR, \fB\-\-unattended\fR
 Unattended installation. The user will not be prompted.
 .TP
-\fB\-\-ca-cert-file\fR=\fICA_FILE\fR
+\fB\-\-ca\-cert\-file\fR=\fICA_FILE\fR
 Do not attempt to acquire the IPA CA certificate via automated means,
 instead use the CA certificate found locally in in \fICA_FILE\fR.  The
 \fICA_FILE\fR must be an absolute path to a PEM formatted certificate
 file. The CA certificate found in \fICA_FILE\fR is considered
 authoritative and will be installed without checking to see if it's
 valid for the IPA domain.
+.TP
+\fB\-\-automount\-location\fR=\fILOCATION\fR
+Automount location
 
 .SS "SSSD OPTIONS"
 .TP
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0059 Create DS user and group during ipa-restore

2013-09-02 Thread Ana Krivokapic
On 08/30/2013 04:37 PM, Petr Viktorin wrote:
> On 08/29/2013 02:16 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3856.
>>
>
> Thanks for the patch!
> I haven't tested it yet, but I've read it and I have some comments below.
>
>
>> diff --git a/install/share/copy-schema-to-ca.py
>> b/install/share/copy-schema-to-ca.py
>> index
>> 1888f12513aa3edf22149e9330afea99f62bf41d..fe99a9256f1298bae1c746ea0c4d41339a4fbebb
>> 100755
>> --- a/install/share/copy-schema-to-ca.py
>> +++ b/install/share/copy-schema-to-ca.py
>> @@ -15,10 +15,11 @@
>>   import pwd
>>   import shutil
>>
>> -from ipapython import services, ipautil, dogtag
>> +from ipapython import services, ipautil
>>   from ipapython.ipa_log_manager import root_logger, standard_logging_setup
>> -from ipaserver.install.dsinstance import DS_USER, schema_dirname
>> +from ipaserver.install.dsinstance import schema_dirname
>>   from ipaserver.install.cainstance import PKI_USER
>> +from ipaserver.install.installutils import DS_USER
>>   from ipalib import api
>
> copy-schema-to-ca.py is intended to be copied to older systems, to prepare the
> CA DSs of old IPA masters for replication with new masters with unified DSs.
> This means that we can't remove anything we import here; the constants need to
> be available at the old locations.

Thanks for clearing that up, I didn't know about this requirement.

>
> That's the hard requirement, but anyway I think the constants, and the
> create_ds_user & create_ds_group functions, are specific to the DS and so
> belong in dsinstance.
> Did I overlook a reason for moving them to installutils?

You are right. I refactored these functions so that they can be more easily used
from other modules, but there is no special reason to put them in installutils.
I moved them back to dsinstance.

>
> [...]
>> diff --git a/ipaserver/install/installutils.py
>> b/ipaserver/install/installutils.py
>> index
>> 268279dc9d22b9f983406303cbfc80c00a2b8fa0..84846221d2800443ba6e291ec9c28b37a482d735
>> 100644
> [...]
>> +def create_ds_user():
>> +"""
>> +Create DS user if it doesn't exist yet
>> +"""
>> +try:
>> +pwd.getpwnam(DS_USER)
>> +root_logger.debug("DS user %s exists" % DS_USER)
>
> Use comma for debug() arguments, i.e. debug("DS group %s exists", DS_GROUP).
> Same in other places.

Fixed.

>
>> +except KeyError:
>> +root_logger.debug("Adding DS user %s" % DS_USER)
>> +args = ["/usr/sbin/useradd", "-g", DS_GROUP,
>> + "-c", "DS System User",
>> + "-d", "/var/lib/dirsrv",
>> + "-s", "/sbin/nologin",
>> + "-M", "-r", DS_USER]
>> +try:
>> +ipautil.run(args)
>> +root_logger.debug("Done adding DS user")
>> +except ipautil.CalledProcessError, e:
>> +root_logger.critical("Failed to add DS user %s" % e)
>> +
>> +
>> +def create_ds_group():
>> +"""
>> +Create DS group if it doesn't exist yet
>> +"""
>
> Please document the return value in the docstring, it's not obvious that this
> returns True iff it didn't do anything.
>

Done.

Updated patch attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 217bc68bbee6ef658e6248fa069252184ca1f171 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Mon, 2 Sep 2013 10:56:19 +0200
Subject: [PATCH] Create DS user and group during ipa-restore

ipa-restore would fail if DS user did not exist. Check for presence of DS
user and group and create them if needed.

https://fedorahosted.org/freeipa/ticket/3856
---
 install/tools/ipa-replica-install | 22 +++--
 install/tools/ipa-server-install  | 11 +--
 ipaserver/install/dsinstance.py   | 66 ---
 ipaserver/install/ipa_restore.py  | 12 +++
 4 files changed, 59 insertions(+), 52 deletions(-)

diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 947c51f6f287ffce52994408352601388faf56a6..2a88c102175f7c9bf7c173f9f9f3437ccc963f29 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -22,7 +22,6 @@ import sys
 import socket
 
 imp

Re: [Freeipa-devel] [PATCH] 0058 Add integration tests for forced client re-enrollment

2013-08-30 Thread Ana Krivokapic
On 08/30/2013 03:28 PM, Petr Viktorin wrote:
> On 08/28/2013 03:04 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> This patch adds integration tests for the forced client re-enrollment
>> feature, according to the test plan at:
>> http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan
>>
>>
>> https://fedorahosted.org/freeipa/ticket/3832
>
> Thank you! The tests are good. As expected I have some nitpicks below.
>
>
> I recommend putting the test case names from the wiki in the method 
> docstrings.
>
> [...]
>> +def restore_client(self):
>> +client = self.clients[0]
>
> I'll ask you to allow SSH here, because the controller can theoretically also
> act as one of the test hosts:
>
> iptables -A INPUT -p tcp --dport 22 -j ACCEPT
>
>> +client.run_command([
>> +'iptables',
>> +'-A', 'INPUT',
>> +'-j', 'REJECT',
>> +'-p', 'all',
>> +'--source', self.master.ip
>> +])
>> +self.uninstall_client()
>> +client.run_command(['iptables', '-F'])
>
> [...]
>> +def backup_keytab(self):
>> +self.clients[0].get_file(CLIENT_KEYTAB, TEMP_KEYTAB)
>> +self.master.put_file(TEMP_KEYTAB, BACKUP_KEYTAB)
>
> Please use get_file_contents & put_file_contents. There might be more tests
> running on the controller machine, we don't want to use a shared file.
> For BACKUP_KEYTAB and EMPTY_KEYTAB please use a file inside
> master.config.test_dir; we don't want to leave files on the testing machines.
> The test_dir gets cleaned up.
>

Thanks for the review!

All issues are fixed in the updated patch.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 3afc75d270e6e899acf98b20a1f65f48811ca8fa Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 28 Aug 2013 14:58:44 +0200
Subject: [PATCH] Add integration tests for forced client re-enrollment

Add integration tests for the forced client re-enrollment feature:
http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan

https://fedorahosted.org/freeipa/ticket/3832
---
 .../test_forced_client_reenrollment.py | 278 +
 1 file changed, 278 insertions(+)
 create mode 100644 ipatests/test_integration/test_forced_client_reenrollment.py

diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py
new file mode 100644
index ..4ba4cda1d4fe509110fffa91e1c13d78b457f64d
--- /dev/null
+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
@@ -0,0 +1,278 @@
+# Authors:
+#   Ana Krivokapic 
+#
+# Copyright (C) 2013  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 <http://www.gnu.org/licenses/>.
+import os
+import subprocess
+
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+
+CLIENT_KEYTAB = '/etc/krb5.keytab'
+
+
+class TestForcedClientReenrollment(IntegrationTest):
+"""
+Forced client re-enrollment
+http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan
+"""
+num_replicas = 1
+num_clients = 1
+
+@classmethod
+def install(cls):
+super(TestForcedClientReenrollment, cls).install()
+tasks.install_master(cls.master)
+tasks.install_replica(cls.master, cls.replicas[0], setup_ca=False)
+cls.BACKUP_KEYTAB = os.path.join(
+cls.master.config.test_dir,
+'krb5.keytab'
+)
+
+def setUp(self):
+tasks.prepare_host(self.clients[0])
+tasks.install_client(self.master, self.clients[0])
+
+def tearDown(self):
+tasks.uninstall_client(self.clients[0])
+self.delete_client_host_entry()
+
+def test_reenroll_with_force_join(self):
+"""
+Client re-enrollment using admin credentials (--force-join)
+"""
+sshfp_record_pre 

[Freeipa-devel] [PATCH] 0061 Add option to ipa-client-install to configure automount

2013-08-30 Thread Ana Krivokapic
Hello,

The attached patch addresses ticket 
https://fedorahosted.org/freeipa/ticket/3740.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 5d86c02b4bf99e701899caff3d9b7d1bd2240621 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Fri, 30 Aug 2013 16:05:01 +0200
Subject: [PATCH] Add option to ipa-client-install to configure automount

Add the --location option to ipa-client-install. If the option is used,
ipa-client-automount is called at the end of ipa-client-install.

https://fedorahosted.org/freeipa/ticket/3740
---
 ipa-client/ipa-install/ipa-client-install | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 280edd793326150c416fe1b82f9866435e9c6509..b34d01bd526637b39193792aceb2576a380de2a9 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -148,6 +148,8 @@ def parse_options():
 # only, it isn't meant to be used on clients.
 basic_group.add_option("--on-master", dest="on_master", action="store_true",
   help=SUPPRESS_HELP, default=False)
+basic_group.add_option("--location", dest="location",
+   help="Automount location")
 parser.add_option_group(basic_group)
 
 sssd_group = OptionGroup(parser, "SSSD options")
@@ -1288,6 +1290,28 @@ def configure_sshd_config(fstore, options):
 except Exception, e:
 log_service_error(sshd.service_name, 'restart', e)
 
+
+def configure_automount(options):
+root_logger.info('\nConfiguring automount:')
+
+args = [
+'ipa-client-automount', '--debug', '-U',
+'--location', options.location
+]
+
+if options.server:
+args.extend(['--server', options.server[0]])
+if not options.sssd:
+args.append('--no-sssd')
+
+try:
+stdout, _, _ = run(args)
+except Exception, e:
+root_logger.error('Automount configuration failed: %s', str(e))
+else:
+root_logger.info(stdout)
+
+
 def resolve_ipaddress(server):
 """ Connect to the server's LDAP port in order to determine what ip
 address this machine uses as "public" ip (relative to the server).
@@ -2515,6 +2539,9 @@ def install(options, env, fstore, statestore):
 if options.conf_sshd:
 configure_sshd_config(fstore, options)
 
+if options.location:
+configure_automount(options)
+
 root_logger.info('Client configuration complete.')
 
 return 0
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0060 Add warning when uninstalling active replica

2013-08-29 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3867.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 29732542431dfa3d2a41607f6537690655a7a027 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 29 Aug 2013 17:44:02 +0200
Subject: [PATCH] Add warning when uninstalling active replica

Add a warning when trying to uninstall a replica that has active replication
agreements.

https://fedorahosted.org/freeipa/ticket/3867
---
 install/tools/ipa-server-install | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 86ca3447bfaab1763324ce57c67c073a8fa93963..7444a6f8720f2c6ca89fcd0bcbf0a2077dabf83f 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -28,18 +28,16 @@
 
 import sys
 import os
-import errno
 import grp
-import subprocess
 import signal
 import shutil
-import glob
 import pickle
 import random
 import tempfile
 import nss.error
 import base64
-from optparse import OptionGroup, OptionValueError, SUPPRESS_HELP
+import pwd
+from optparse import OptionGroup, OptionValueError
 
 from ipaserver.install import dsinstance
 from ipaserver.install import krbinstance
@@ -51,10 +49,11 @@ from ipaserver.install import cainstance
 from ipaserver.install import memcacheinstance
 from ipaserver.install import otpdinstance
 from ipaserver.install import sysupgrade
-
+from ipaserver.install import replication
 from ipaserver.install import service, installutils
 from ipapython import version
 from ipapython import certmonger
+from ipapython import ipaldap
 from ipaserver.install.installutils import *
 from ipaserver.plugins.ldap2 import ldap2
 
@@ -624,6 +623,29 @@ def main():
 print "Aborting uninstall operation."
 sys.exit(1)
 
+conn = ipaldap.IPAdmin(api.env.host, ldapi=True, realm=api.env.realm)
+conn.do_external_bind(pwd.getpwuid(os.geteuid()).pw_name)
+rm = replication.ReplicationManager(api.env.realm, api.env.host, None,
+conn=conn)
+agreements = rm.find_ipa_replication_agreements()
+
+if agreements:
+other_masters = [a.get('cn')[0][4:] for a in agreements]
+print "\nReplication agreements with the following IPA masters "
+print "found: %s." % ", ".join(other_masters)
+print "Removing any replication agreements before uninstalling "
+print "the server is strongly recommended."
+print "You can remove replication agreements by running the "
+print "following command on any other IPA master:"
+print "$ ipa-replica-manage del %s" % api.env.host
+if not (options.unattended or user_input("Are you sure you want "
+ "to continue with the "
+ "uninstall procedure?",
+ False)):
+print ""
+print "Aborting uninstall operation."
+sys.exit(1)
+
 return uninstall()
 
 if options.external_ca:
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0059 Create DS user and group during ipa-restore

2013-08-29 Thread Ana Krivokapic
Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3856.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From f1a2a00f72961c88530be8aa1a62fb15758d90b5 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Thu, 29 Aug 2013 12:11:55 +0200
Subject: [PATCH] Create DS user and group during ipa-restore

ipa-restore would fail if DS user did not exist. Check for presence of DS
user and group and create them if needed.

https://fedorahosted.org/freeipa/ticket/3856
---
 install/share/copy-schema-to-ca.py |  5 ++--
 install/tools/ipa-replica-install  | 15 ++--
 install/tools/ipa-server-install   | 11 +
 ipaserver/install/dsinstance.py| 29 ---
 ipaserver/install/installutils.py  | 47 ++
 ipaserver/install/ipa_backup.py|  4 ++--
 ipaserver/install/ipa_restore.py   |  8 ---
 ipaserver/install/krbinstance.py   |  2 +-
 8 files changed, 66 insertions(+), 55 deletions(-)

diff --git a/install/share/copy-schema-to-ca.py b/install/share/copy-schema-to-ca.py
index 1888f12513aa3edf22149e9330afea99f62bf41d..fe99a9256f1298bae1c746ea0c4d41339a4fbebb 100755
--- a/install/share/copy-schema-to-ca.py
+++ b/install/share/copy-schema-to-ca.py
@@ -15,10 +15,11 @@
 import pwd
 import shutil
 
-from ipapython import services, ipautil, dogtag
+from ipapython import services, ipautil
 from ipapython.ipa_log_manager import root_logger, standard_logging_setup
-from ipaserver.install.dsinstance import DS_USER, schema_dirname
+from ipaserver.install.dsinstance import schema_dirname
 from ipaserver.install.cainstance import PKI_USER
+from ipaserver.install.installutils import DS_USER
 from ipalib import api
 
 SERVERID = "PKI-IPA"
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 947c51f6f287ffce52994408352601388faf56a6..0c2a1c6804e28cfef89da86c0e3b5fcaf2c5bfa1 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -147,7 +147,7 @@ def get_dirman_password():
 return installutils.read_password("Directory Manager (existing master)", confirm=False, validate=False)
 
 def set_owner(config, dir):
-pw = pwd.getpwnam(dsinstance.DS_USER)
+pw = pwd.getpwnam(installutils.DS_USER)
 os.chown(dir, pw.pw_uid, pw.pw_gid)
 
 def install_replica_ds(config):
@@ -574,18 +574,7 @@ def main():
 api.finalize()
 
 # Create DS group if it doesn't exist yet
-try:
-grp.getgrnam(dsinstance.DS_GROUP)
-root_logger.debug("ds group %s exists" % dsinstance.DS_GROUP)
-group_exists = True
-except KeyError:
-group_exists = False
-args = ["/usr/sbin/groupadd", "-r", dsinstance.DS_GROUP]
-try:
-ipautil.run(args)
-root_logger.debug("done adding DS group")
-except ipautil.CalledProcessError, e:
-root_logger.critical("failed to add DS group: %s" % e)
+group_exists = installutils.create_ds_group()
 sstore.backup_state("install", "group_exists", group_exists)
 
 #Automatically disable pkinit w/ dogtag until that is supported
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 86ca3447bfaab1763324ce57c67c073a8fa93963..3054a5c99b5d72d74ea3908cfc3d60647c25ce4b 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -971,16 +971,7 @@ def main():
 ipaservices.backup_and_replace_hostname(fstore, sstore, host_name)
 
 # Create DS group if it doesn't exist yet
-try:
-grp.getgrnam(dsinstance.DS_GROUP)
-root_logger.debug("ds group %s exists" % dsinstance.DS_GROUP)
-except KeyError:
-args = ["/usr/sbin/groupadd", "-r", dsinstance.DS_GROUP]
-try:
-ipautil.run(args)
-root_logger.debug("done adding DS group")
-except ipautil.CalledProcessError, e:
-root_logger.critical("failed to add DS group: %s" % e)
+installutils.create_ds_group()
 
 # Create a directory server instance
 if external != 2:
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index f543efadc6568a022fbb0a2ee07833612f9466f7..872e8c1941608f47a52335b1ac813102abdecff3 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -45,8 +45,6 @@
 SERVER_ROOT_32 = "/usr/lib/dirsrv"
 CACERT="/etc/ipa/ca.crt"
 
-DS_USER = 'dirsrv'
-DS_GROUP = 'dirsrv'
 
 def find_server_root():
 if ipautil.dir_exists(SERVER_ROOT_64):
@@ -194,7 +192,7 @@ def __init__(self, realm_name=None, domain_name=None, dm_password=None,
 
 def __common_setup(self, enable_ssl=False):
 
-self.step("creating directory server user", self.__create_ds_user)
+self.step("creating

[Freeipa-devel] [PATCH] 0058 Add integration tests for forced client re-enrollment

2013-08-28 Thread Ana Krivokapic
Hello,

This patch adds integration tests for the forced client re-enrollment feature, 
according to the test plan at:
http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan


https://fedorahosted.org/freeipa/ticket/3832

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 646ec8995ced69f6538f549edcd08f9a98f84c34 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 28 Aug 2013 14:58:44 +0200
Subject: [PATCH] Add integration tests for forced client re-enrollment

Add integration tests for the forced client re-enrollment feature:
http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan

https://fedorahosted.org/freeipa/ticket/3832
---
 .../test_forced_client_reenrollment.py | 241 +
 1 file changed, 241 insertions(+)
 create mode 100644 ipatests/test_integration/test_forced_client_reenrollment.py

diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py
new file mode 100644
index ..1f8f0c221240d8bd9001d5ceaec833925b552e95
--- /dev/null
+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
@@ -0,0 +1,241 @@
+# Authors:
+#   Ana Krivokapic 
+#
+# Copyright (C) 2013  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 <http://www.gnu.org/licenses/>.
+import subprocess
+
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+
+CLIENT_KEYTAB = '/etc/krb5.keytab'
+BACKUP_KEYTAB = '/root/krb5.keytab'
+EMPTY_KEYTAB = '/tmp/empty.keytab'
+TEMP_KEYTAB = '/tmp/tmp.keytab'
+
+
+class TestForcedClientReenrollment(IntegrationTest):
+"""
+Forced client re-enrollment
+http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan
+"""
+num_replicas = 1
+num_clients = 1
+
+@classmethod
+def install(cls):
+super(TestForcedClientReenrollment, cls).install()
+tasks.install_master(cls.master)
+tasks.install_replica(cls.master, cls.replicas[0], setup_ca=False)
+
+def setUp(self):
+tasks.prepare_host(self.clients[0])
+tasks.install_client(self.master, self.clients[0])
+
+def tearDown(self):
+tasks.uninstall_client(self.clients[0])
+self.delete_client_host_entry()
+
+def test_reenroll_with_force_join(self):
+sshfp_record_pre = self.get_sshfp_record()
+self.restore_client()
+self.check_client_host_entry()
+self.reenroll_client(force_join=True)
+sshfp_record_post = self.get_sshfp_record()
+assert sshfp_record_pre == sshfp_record_post
+
+def test_reenroll_with_keytab(self):
+self.backup_keytab()
+sshfp_record_pre = self.get_sshfp_record()
+self.restore_client()
+self.check_client_host_entry()
+self.restore_keytab()
+self.reenroll_client(keytab=BACKUP_KEYTAB)
+sshfp_record_post = self.get_sshfp_record()
+assert sshfp_record_pre == sshfp_record_post
+
+def test_reenroll_with_both_force_join_and_keytab(self):
+self.backup_keytab()
+sshfp_record_pre = self.get_sshfp_record()
+self.restore_client()
+self.check_client_host_entry()
+self.restore_keytab()
+self.reenroll_client(force_join=True, keytab=BACKUP_KEYTAB)
+sshfp_record_post = self.get_sshfp_record()
+assert sshfp_record_pre == sshfp_record_post
+
+def test_reenroll_to_replica(self):
+self.backup_keytab()
+sshfp_record_pre = self.get_sshfp_record()
+self.restore_client()
+self.check_client_host_entry()
+self.restore_keytab()
+self.reenroll_client(keytab=BACKUP_KEYTAB, to_replica=True)
+sshfp_record_post = self.get_sshfp_record()
+assert sshfp_record_pre == sshfp_record_post
+
+def test_try_to_reenroll_with_disabled_host(self):
+self.backup_keytab()
+self.disable_client_host_entry()
+self.restore_client()
+self.check_client_host_entry(enabled=False)
+self.restore_keytab()
+self.reenroll_client(keytab=BACKUP_KEYTAB, expect_fail=True)
+
+def test_try_to_reenroll_with_uninstalled_host(self):
+

Re: [Freeipa-devel] [PATCH] 446 Update idrange search facet after trust creation

2013-08-28 Thread Ana Krivokapic
On 08/22/2013 04:11 PM, Petr Vobornik wrote:
> Adding a trust creates a range -> range search facet should be marked as 
> expired.
>
> https://fedorahosted.org/freeipa/ticket/3874
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0057 Add integration tests for Kerberos Flags

2013-08-27 Thread Ana Krivokapic
On 08/27/2013 10:45 AM, Petr Viktorin wrote:
> On 08/25/2013 08:56 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> This patch adds integration tests for the Kerberos Flags feature (except the 
>> web
>> UI tests), according to the test plan at:
>> http://www.freeipa.org/page/V3/Kerberos_Flags#Test_Plan.
>>
>> https://fedorahosted.org/freeipa/ticket/3831
>
> Thank you! The tests work well. I do have a few nitpicks though.
>
>
> [...]
>> +class TestKerberosFlags(IntegrationTest):
>> +"""
>> +Test Kerberos Flags
>> +http://www.freeipa.org/page/V3/Kerberos_Flags#Test_Plan
>> +"""
>> +topology = 'line'
>> +num_clients = 1
>> +
>> +def test_set_flag_with_host_add(self):
>> +host = 'host.example.com'
>> +host_service = 'host/host.example.com'
>> +host_keytab = '/tmp/host.keytab'
>
> This function has three nearly identical blocks. Why not say `for trusted in
> (True, False, None):` here?
> Same for the others.
>
> [...]
>> +result = self.master.run_command(args)
>> +assert result.returncode == 0
>
> These asserts are not needed; run_command() checks automatically unless you
> specify raise_on_error.
>
> [...]
>> +def check_flag_klist(self, service, trusted):
>> +result = self.master.run_command(['klist', '-f'])
>> +output_lines = result.stdout_text.split('\n')
>> +flags = ''
>> +
>> +for line in output_lines:
>> +if service in line:
>> +i = output_lines.index(line)
>
> For looping with an index you should generally use `for i, line in
> enumerate(output_lines):`
> In this specific case, you can use the "sliding window iteration" idiom:
>
> for line, next_line in zip(output_lines, output_lines[1:]):
>
>> +flags = output_lines[i+1].replace('Flags:', '').strip()
>> +
>> +if trusted:
>> +assert 'O' in flags
>> +else:
>> +assert 'O' not in flags
>> +
>> +def rekinit(self):
>> +self.master.run_command(['kdestroy'])
>> +tasks.kinit_admin(self.master)
>> +
>> +def getkeytab(self, service, keytab):
>> +result = self.master.run_command([
>> +'ipa-getkeytab',
>> +'-s',
>> +    self.master.hostname,
>> +'-p',
>> +service,
>> +'-k',
>> +keytab
>
> A really minor one -- I find the following more readable:
>
> 'ipa-getkeytab',
> '-s', self.master.hostname,
> '-p', service,
> '-k', keytab,
>
>

Thanks for the review!

All good points, all fixed.

Updated patch is attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 107d80a04a99588785118b8cfe301dfa7177a178 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Sun, 25 Aug 2013 20:45:39 +0200
Subject: [PATCH] Add integration tests for Kerberos Flags

Add integration tests for the Kerberos Flags feature:
http://www.freeipa.org/page/V3/Kerberos_Flags#Test_Plan
(except the web UI tests).

https://fedorahosted.org/freeipa/ticket/3831
---
 ipatests/test_integration/test_kerberos_flags.py | 191 +++
 1 file changed, 191 insertions(+)
 create mode 100644 ipatests/test_integration/test_kerberos_flags.py

diff --git a/ipatests/test_integration/test_kerberos_flags.py b/ipatests/test_integration/test_kerberos_flags.py
new file mode 100644
index ..a7de0295242afde5d9a8cfded5cb55abe520a2a5
--- /dev/null
+++ b/ipatests/test_integration/test_kerberos_flags.py
@@ -0,0 +1,191 @@
+# Authors:
+#   Ana Krivokapic 
+#
+# Copyright (C) 2013  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 <http://www.gnu.o

[Freeipa-devel] [PATCH] 0057 Add integration tests for Kerberos Flags

2013-08-25 Thread Ana Krivokapic
Hello,

This patch adds integration tests for the Kerberos Flags feature (except the web
UI tests), according to the test plan at:
http://www.freeipa.org/page/V3/Kerberos_Flags#Test_Plan.

https://fedorahosted.org/freeipa/ticket/3831

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 09bfdf9398e9c51bede64f14e59348bc7ceeb29d Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Sun, 25 Aug 2013 20:45:39 +0200
Subject: [PATCH] Add integration tests for Kerberos Flags

Add integration tests for the Kerberos Flags feature:
http://www.freeipa.org/page/V3/Kerberos_Flags#Test_Plan
(except the web UI tests).

https://fedorahosted.org/freeipa/ticket/3831
---
 ipatests/test_integration/test_kerberos_flags.py | 260 +++
 1 file changed, 260 insertions(+)
 create mode 100644 ipatests/test_integration/test_kerberos_flags.py

diff --git a/ipatests/test_integration/test_kerberos_flags.py b/ipatests/test_integration/test_kerberos_flags.py
new file mode 100644
index ..7aebe51df5bbd0d2899dea8505fe059c2c470c3c
--- /dev/null
+++ b/ipatests/test_integration/test_kerberos_flags.py
@@ -0,0 +1,260 @@
+# Authors:
+#   Ana Krivokapic 
+#
+# Copyright (C) 2013  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 <http://www.gnu.org/licenses/>.
+
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+
+
+class TestKerberosFlags(IntegrationTest):
+"""
+Test Kerberos Flags
+http://www.freeipa.org/page/V3/Kerberos_Flags#Test_Plan
+"""
+topology = 'line'
+num_clients = 1
+
+def test_set_flag_with_host_add(self):
+host = 'host.example.com'
+host_service = 'host/host.example.com'
+host_keytab = '/tmp/host.keytab'
+
+self.add_object('host', host, trusted=True, force=True)
+self.check_flag_cli('host', host, trusted=True)
+self.rekinit()
+self.getkeytab(host_service, host_keytab)
+self.kvno(host_service)
+self.check_flag_klist(host_service, trusted=True)
+self.del_object('host', host)
+
+self.add_object('host', host, trusted=False, force=True)
+self.check_flag_cli('host', host, trusted=False)
+self.rekinit()
+self.getkeytab(host_service, host_keytab)
+self.kvno(host_service)
+self.check_flag_klist(host_service, trusted=False)
+self.del_object('host', host)
+
+self.add_object('host', host, force=True)
+self.check_flag_cli('host', host, trusted=False)
+self.rekinit()
+self.getkeytab(host_service, host_keytab)
+self.kvno(host_service)
+self.check_flag_klist(host_service, trusted=False)
+self.del_object('host', host)
+
+def test_set_and_clear_flag_with_host_mod(self):
+client_hostname = self.clients[0].hostname
+host_service = 'host/%s' % client_hostname
+
+self.kvno(host_service)
+self.check_flag_cli('host', client_hostname, trusted=False)
+self.check_flag_klist(host_service, trusted=False)
+
+self.mod_object_cli('host', client_hostname, trusted=True)
+self.check_flag_cli('host', client_hostname, trusted=True)
+self.rekinit()
+self.kvno(host_service)
+self.check_flag_klist(host_service, trusted=True)
+
+self.mod_object_cli('host', client_hostname, trusted=False)
+self.check_flag_cli('host', client_hostname, trusted=False)
+self.rekinit()
+self.kvno(host_service)
+self.check_flag_klist(host_service, trusted=False)
+
+self.mod_service_kadmin_local(host_service, trusted=True)
+self.check_flag_cli('host', client_hostname, trusted=True)
+self.rekinit()
+self.kvno(host_service)
+self.check_flag_klist(host_service, trusted=True)
+
+self.mod_service_kadmin_local(host_service, trusted=False)
+self.check_flag_cli('host', client_hostname, trusted=False)
+self.rekinit()
+self.kvno(host_service)
+self.check_flag_klist(host_servi

Re: [Freeipa-devel] [PATCH] 445 Web UI integration tests: ID range types

2013-08-21 Thread Ana Krivokapic
On 08/20/2013 06:19 PM, Nathaniel McCallum wrote:
> On Tue, 2013-08-20 at 15:56 +0200, Petr Vobornik wrote:
>> On 08/19/2013 05:33 PM, Petr Vobornik wrote:
>>> https://fedorahosted.org/freeipa/ticket/3834
>>>
>> New version. Fixes usage of secondary rid for non-trust installation.
> A lot of the functions have spaces between the function declaration an
> the first line. I don't know how strictly we follow PEP8, but I thought
> I'd point it out.
>
> Nathaniel
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

A general rule is that we _try_ to make any new code PEP8 compliant, as well as
make appropriate fixes when refactoring old code.

Personally, I don't have a problem with a few blank lines. Technically, I don't
think this is a PEP8 violation as blank lines are allowed inside functions.

The patch works fine and I have no objections to the code, so ACK from me.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


  1   2   3   >