Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation
On 05/06/2015 03:20 PM, Martin Basti wrote: On 05/05/15 15:00, Martin Basti wrote: On 30/04/15 15:37, David Kupka wrote: On 04/24/2015 02:56 PM, Martin Basti wrote: Patches attached. Hi, thanks for patches. 1. You changed message in DNSServerNotRespondingWarning class but not the test in ipatest/test_xmlrpc/test_dns_plugin.py nitpick. Please spell 'edns' correctly. I've seen several instances of 'ends'. Thank you, updated patches attached: * new error messages * logging to debug log server output if exception was raised * fixed test * fixed spelling Fixed tests (again) Updated patches attached The code looks good to me and tests are no longer broken. (I would prefer better fix of the tests but given that the priorities are different now it can wait.) Petr, can you please confirm that the patch set works for you? -- David Kupka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 821 webui: add pwpolicy link to group details page if group has associated pwpolicy
On 04/30/2015 01:57 PM, Martin Babinsky wrote: On 04/17/2015 05:58 PM, Petr Vobornik wrote: https://fedorahosted.org/freeipa/ticket/4982 ACK Pushed to master: b61f4bc538573fcf76c69b52698045dcb0f99e55 -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 381 Fix stop_tracking_certificates call in ipa-restore
On 12/03/2014 10:23 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4775. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi, thanks for patch (with 5 months delay :-) The patch needs a trivial rebase but otherwise works for me and is needed for ipa-restore to work. -- David Kupka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 381 Fix stop_tracking_certificates call in ipa-restore
Dne 7.5.2015 v 10:04 David Kupka napsal(a): On 12/03/2014 10:23 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4775. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi, thanks for patch (with 5 months delay :-) The patch needs a trivial rebase but otherwise works for me and is needed for ipa-restore to work. Here you go. -- Jan Cholasta From d57a48c031904cdd20b32f280d46cfac5accbb26 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Wed, 3 Dec 2014 09:12:18 + Subject: [PATCH] Fix stop_tracking_certificates call in ipa-restore CAInstance.stop_tracking_certificates() no longer has dogtag_constants argument. https://fedorahosted.org/freeipa/ticket/4775 --- ipaserver/install/ipa_restore.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py index cc466c2..56d3137 100644 --- a/ipaserver/install/ipa_restore.py +++ b/ipaserver/install/ipa_restore.py @@ -28,7 +28,7 @@ import ldif import itertools from ipalib import api, errors, constants -from ipapython import version, ipautil, certdb, dogtag +from ipapython import version, ipautil, certdb from ipapython.ipautil import run, user_input from ipapython import admintool from ipapython.dn import DN @@ -785,8 +785,7 @@ class Restore(admintool.AdminTool): self.log.error('%s', e) def cert_restore_prepare(self): -cainstance.CAInstance().stop_tracking_certificates( -dogtag.configured_constants()) +cainstance.CAInstance().stop_tracking_certificates() httpinstance.HTTPInstance().stop_tracking_certificates() try: dsinstance.DsInstance().stop_tracking_certificates( -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 807 webui-ci: do not open 2 browser windows
On 05/06/2015 04:28 PM, Milan Kubik wrote: On 02/19/2015 03:51 PM, Petr Vobornik wrote: Only for master branch. ACK Milan Pushed to master: a1ccdc33dfc5de3480b9ad407f4a95d01258008d -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0047] Unsaved changes dialog inconsistent
On 05/05/2015 02:36 PM, Gabe Alford wrote: Thanks Petr. I thought I had grepped all that out. Guess I didn't do it from the top of the tree. Updated patch attached. ACK Pushed to: master: d1a0474d1851cd54a1fa7bdf4a0dc0ed452a0090 ipa-4-1: 5ac5564227757dfc367ca74fe86e99114d5b582b On Tue, May 5, 2015 at 5:15 AM, Petr Vobornik pvobo...@redhat.com wrote: On 04/30/2015 07:43 PM, Gabe Alford wrote: Thanks Kyle and Petr. Update patch attached. Renaming the buttons also requires to update webui integration tests in ipatests/test_webui, quick search: ipatests/test_webui/test_realmdomains.py:42,48 ipatests/test_webui/ui_driver.py:1221,1246,1464,1483 On Wed, Apr 29, 2015 at 7:59 AM, Kyle Baker kyba...@redhat.com wrote: - Original Message - On 04/27/2015 03:03 PM, Gabe Alford wrote: Hello, Fix for https://fedorahosted.org/freeipa/ticket/4926 Thanks, Gabe PatternFly has new recommendations for terminology and wording [1]. I'm not entirely sure if the usage of 'save' here is good. PF defines 'edit' as the recommended term. The page doesn't say if 'save' is not recommended, though. Save seems to me as a confirmation of editing. Yes I think save would be best here based on the message given. Thanks for checking out the Terminology screen! Kyle, could you advise what is the best term for reflecting user changes and for confirmation of this action? Technical notes: 1. it would be better to add a new string and then use it in the button instead of having 'Save' text for '@i18n:buttons.update' definition. 2. String changes in internal.py should be also reflected in install/ui/test/data/ipa_init.json (for static web ui demo). 3. optional: in addition to text change, buttons and related actions could also be renamed (same reasons as in 1). It's more proper but much more complicated. [1] https://www.patternfly.org/styles/terminology-and-wording/#action-labels -- Petr Vobornik -- Petr Vobornik -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 381 Fix stop_tracking_certificates call in ipa-restore
Dne 7.5.2015 v 10:21 David Kupka napsal(a): On 05/07/2015 10:11 AM, Jan Cholasta wrote: Dne 7.5.2015 v 10:04 David Kupka napsal(a): On 12/03/2014 10:23 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4775. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi, thanks for patch (with 5 months delay :-) The patch needs a trivial rebase but otherwise works for me and is needed for ipa-restore to work. Here you go. ACK. Pushed to master: 37784625eba7d9f7f3cbd18483708a45ece3061e -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 381 Fix stop_tracking_certificates call in ipa-restore
On 05/07/2015 10:11 AM, Jan Cholasta wrote: Dne 7.5.2015 v 10:04 David Kupka napsal(a): On 12/03/2014 10:23 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4775. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi, thanks for patch (with 5 months delay :-) The patch needs a trivial rebase but otherwise works for me and is needed for ipa-restore to work. Here you go. ACK. -- David Kupka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0238] Server Upgrade: fix broken memberUid index
On 05/06/2015 01:23 PM, Martin Basti wrote: On 06/05/15 13:22, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5007 Patch attached. Requires patch mbasti-231-4 -- Martin Basti ACK -- Martin^3 Babinsky -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 814-818 migrate-ds: optimize adding users to default group
On 05/06/2015 04:49 PM, Martin Basti wrote: On 05/05/15 10:59, Petr Vobornik wrote: On 04/30/2015 03:53 PM, Martin Basti wrote: On 10/04/15 12:55, Petr Vobornik wrote: The essential patch is 814. 815 a proposal for new option. 816 and 818 are cleanup patches. 817 little optimization. == [PATCH] 814 migrate-ds: optimize adding users to default group == Migrate-ds searches for user without a group and adds them to default group. There is no point in checking if the user's selected by previous query are not member of default group because they are not member of any group. The operation is also speeded up by not fetching the default group. Users are added right away. https://fedorahosted.org/freeipa/ticket/4950 NACK Users haven't been added into ipa default group after migration. Fixed in patch 815. Just nitpick 1) too many parentheses api.log.error(('Adding new members to default group failed: %s \n' 'members: %s') % (e, (','.join(member_dns You can use this instead: api.log.error('Adding new members to default group failed: %s \n' 'members: %s', e, ','.join(member_dns)) Fixed. == [PATCH] 815 migrate-ds: skip default group options == New option --use-default-group=False could be used to disable adding of migrated users into default group. By default, the default group is no longer POSIX therefore it doesn't fulfill the original idea of providing GID and therefore it could be skipped during migration. LGTM the option got so the default would be applied. +autofill=True, == [PATCH] 816 migrate-ds: remove unused def_group_gid context property == it's no longer used anywhere 1) You can remove the unused variable 'g_attrs' removed == [PATCH] 817 migrate-ds: optimize gid checks by utilizing dictionary nature of set == LGTM == [PATCH] 818 migrate-ds: log migrated group members only on debug level == It pollutes error_log. 1) you do not need % formatting in logger api.log.debug('migrating %s group %s' , member_attr, m) fixed and also changed migrating %s user %s' line to debug, which was the actual reason for this patch. Martin^2 Thanks. 1) Please create new API file, probably missing autofill in API.txt: Option 'use_def_group?' in command 'migrate_ds' in API file not found Options count in migrate_ds of 18 doesn't match expected: 19 Option use_def_group? of command migrate_ds in ipalib, not in API file: Bool('use_def_group?', autofill=True, cli_name='use_default_group', default=True) There are one or more changes to the API. Either undo the API changes or update API.txt and increment the major version in VERSION. you could just wrote that I forgot to run ./makeapi ;) 2) ipa: error: --use-default-group option does not take a value Attached new patch #826 which should fix the issue. Not sure if Honza(CCd) will like it. (default: true) added to option description for better UX. and a nitpick: patch 814 1) Why this change? -api.log.debug('Adding %d users to group%s duration %s', -len(new_members), mode, d) +api.log.info('Adding %d users to group%s duration %s', + len(member_dns), mode, d) So that there will be a record in a default(not debug) log that something happened. The reason is that it also affects users, without a group, that are already present on the system. -- Petr Vobornik From 568b6b233d99b65af7d9d4d140164a7b693027a6 Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Thu, 9 Apr 2015 17:17:09 +0200 Subject: [PATCH] migrate-ds: log migrated group members only on debug level It pollutes error_log. --- ipalib/plugins/migration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py index 22bf6a8f6a53343cba3869ad5924131c7861d819..8b7dd9ef6c5e16ef39997f04ca935c4de3e56aa9 100644 --- a/ipalib/plugins/migration.py +++ b/ipalib/plugins/migration.py @@ -338,11 +338,11 @@ def _pre_migrate_group(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwarg continue if m.endswith(search_bases['user']): -api.log.info('migrating %s user %s' % (member_attr, m)) +api.log.debug('migrating %s user %s', member_attr, m) m = DN((api.Object.user.primary_key.name, rdnval), api.env.container_user, api.env.basedn) elif m.endswith(search_bases['group']): -api.log.info('migrating %s group %s' % (member_attr, m)) +api.log.debug('migrating %s group %s', member_attr, m) m = DN((api.Object.group.primary_key.name, rdnval), api.env.container_group, api.env.basedn) else: -- 2.1.0 From 2560a055c8cec039fd8e109d8e33c61967783203 Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Mon, 23 Mar 2015 12:18:23
Re: [Freeipa-devel] [PATCH 0032] prevent duplicate IDs when setting up multiple replicas against single master
On 05/07/2015 01:05 PM, Martin Babinsky wrote: On 05/06/2015 07:41 PM, thierry bordaz wrote: On 05/06/2015 05:56 PM, Martin Babinsky wrote: On 05/06/2015 04:25 PM, thierry bordaz wrote: On 05/06/2015 03:19 PM, Martin Babinsky wrote: Hello Thierry, replies are inline. On 05/06/2015 02:22 PM, thierry bordaz wrote: On 05/06/2015 01:54 PM, Martin Babinsky wrote: The attached patch tries to fix https://fedorahosted.org/freeipa/ticket/4378 After discussion with Thierry we concluded that while this issue is more complex than it seems, the transition from REPLACE to DEL/ADD operations when updating nsDS5ReplicaId should suffice for this ticket. Hello Martin, Few comments, you are using MOD_DEL 'replicaID' with None value. So this is going to delete all previous values and it should be equivalent to a MOD_REPL. I was thinking you wanted to retrieve the id_value and call MOD_DEL 'replicaID' current_value. So that if by the time you fetched the replicaId, an other replica updated the replicaId, the MOD_DEL/MOD_ADD would fail and you need a new iteration. Sorry I didn't know you can MOD_DEL a particular value (I'm LDAP noob I know). Will fix this. If replicaId was multi-valued and you want to make it single valued, you may want to do create a more complex MOD (e.g. (ldap.MOD_DELETE, 'nsDS5ReplicaId', str(value1), (ldap.MOD_DELETE, 'nsDS5ReplicaId', str(value2)...) AFAIK ReplicaId is single-valued (looking at the schema right now) so this shouldn't be problem. If it is updating successfully do you want to return 'retval' or 'retval+1' ? If several replicas try to update the replicaId of the master and the current replicaId is 1000. Replica1 successfully updates the replicaId and gets 1001 as the new value. Replica2 successfully updates the replicaId and gets 1002. The final value on master will be 1002, but replica1 will assum it is 1001. Is it a problem ? I studied the code in the master branch and IIUC (and please correct me if I got this wrong) nsDS5ReplicaId attribute in 'cn=replication,cn=etc,$SUFFIX' represents replicaID of the _next_ replica that will be installed. So if a replica is installed, it sets the current value of nsDS5ReplicaId as its replica ID (the function returns 'retval') and then increments it in 'cn=replication,cn=etc,$SUFFIX' entry ('retval + 1' is written to master) so that the next installed replica fetches this updated value. So the case you described should be the expected behavior. To change it would require different patch IMHO. Thank for your precious explanations, in fact the value in 'cn=replication,cn=etc,SUFFIX' is the next available replicaId value and is the centralized mechanism that assign unique replicaID. The risk was that several replica pick the same value. So yes it is important that the MOD_DEL specifies the previously read value so that the test/set will be atomic. If several replicas read the same value, only the faster one will use it to install the replica. thanks thierry thanks thierry Attaching updated patch with fixed MOD_DELETE operation. Hi Martin, The fix looks good to me except I think you need to do (ldap.MOD_DELETE, 'nsDS5ReplicaId', *str(*retval*)*) thanks thierry Attaching updated patch. Thanks Martin, The fix is good for me. Ack. thierry -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0032] prevent duplicate IDs when setting up multiple replicas against single master
On 05/07/2015 01:28 PM, thierry bordaz wrote: On 05/07/2015 01:05 PM, Martin Babinsky wrote: On 05/06/2015 07:41 PM, thierry bordaz wrote: Attaching updated patch. Thanks Martin, The fix is good for me. Ack. thierry Pushed to master: e2a42efe33d5e6cb08e1988f7253caf56eda11df -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH 0338] Add includes to zone.c to improve compatibility with BIND 9.9.4
Hello, This is minor improvement for patch set related to ticket #155. Add includes to zone.c to improve compatibility with BIND 9.9.4. -- Petr^2 Spacek From 203ee6dceb5ec3488cd8b33f4e70bb299d58cf7d Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Thu, 7 May 2015 14:53:08 +0200 Subject: [PATCH] Add includes to zone.c to improve compatibility with BIND 9.9.4. --- src/zone.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/zone.c b/src/zone.c index 3feec1d80936f749912203d53cdbff253c83a232..014d36ed7b7f3b7f82fb637f8e7f3443cf1a1737 100644 --- a/src/zone.c +++ b/src/zone.c @@ -3,11 +3,14 @@ */ #include isc/types.h +#include isc/util.h +#include dns/diff.h #include dns/journal.h #include dns/rdatalist.h #include dns/rdataset.h #include dns/soa.h +#include dns/types.h #include dns/update.h #include dns/zone.h -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 801-806 webui-ci: otptoken tests
On 02/19/2015 03:51 PM, Petr Vobornik wrote: https://fedorahosted.org/freeipa/ticket/4307 For ipa-4-1 apply: - patch 800 (different thread) - patches 801-806 For master apply: - patch 800 (different thread) - patch 807 (different thread) - patch 801-master - patches 802-806 Patch 801 allows to use ipalib rpc client in Web UI test suite. Patches 802-805 are various ui_driver fixes to allow stuff in patch 806. == [PATCH] 806 webui-ci: otptoken tests == Basic otptoken Web UI CI coverage. tests: * crud for otptokens as admin * crud for normal users * checks fields of adder dialog for both token types and user role (admin/user) * token actions as admin (enable, disable, delete) * token actions as normal user (delete) * login as normal user with hotp and totp token * sync token hotp and totp token as normal user and then login https://fedorahosted.org/freeipa/ticket/4307 == [PATCH] 805 webui-ci: allow custom names for disable/enable actions == Not all disable and enable actions are called 'disable' and 'enable'. == [PATCH] 804 webui-ci: allow to update pkey in post-add in basic-crud tests == == [PATCH] 803 webui-ci: add post_add_action == post add action allows to fill autogenerated values, e.g. a pkey of new otptoken. This value can be then used in other subsequent test which would depend on it - like crud tests. == [PATCH] 802 webui-ci: fix negative visibility check == Allow to define, that element doesn't have to be present on a page for negative visible checks. E.g. if element is added only if it's displayed and is removed otherwise. == [PATCH] 801 webui-ci: support direct IPA API calls == Add IPA API support to ui_driver. It leverages new ipalib RPC client's forms based authentication. It then allows to call an IPA API while the machine is not an IPA client nor is kerberized. api's environment values are taken from test configuration and therefore duplication in ~/.ipa/default.conf is not required. Since the machine doesn't have to be IPA client, it then also doesn't have nss database with IPA's CA certificate. Therefore on each API initialization a new NSS database is created with a CA certificate downloaded from IPA. This db is deleted in tearDown phase. Usage: 1. as admin one can immediately call rpc commands, api will be initialized upon first request and is available under self.api (assuming self is ui_driver): self.api.Command.user_del(USER_ID, **{'continue': True}) 2. to reconnect as other user: self.reconnect_api(USER_ID, USER_PW) 3. reconnect back as admin: self.reconnect_api() Patch #803 needed rebase. -- Petr Vobornik From 643564ca13fac71f303bba9147914aa42a17153b Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Wed, 7 Jan 2015 13:56:07 +0100 Subject: [PATCH] webui-ci: otptoken tests Basic otptoken Web UI CI coverage. tests: * crud for otptokens as admin * crud for normal users * checks fields of adder dialog for both token types and user role (admin/user) * token actions as admin (enable, disable, delete) * token actions as normal user (delete) * login as normal user with hotp and totp token * sync token hotp and totp token as normal user and then login https://fedorahosted.org/freeipa/ticket/4307 --- ipatests/test_webui/test_otptoken.py | 374 +++ ipatests/test_webui/ui_driver.py | 2 + 2 files changed, 376 insertions(+) create mode 100644 ipatests/test_webui/test_otptoken.py diff --git a/ipatests/test_webui/test_otptoken.py b/ipatests/test_webui/test_otptoken.py new file mode 100644 index ..465dc8da0e3a5de224cf9266829aadb7968eaab5 --- /dev/null +++ b/ipatests/test_webui/test_otptoken.py @@ -0,0 +1,374 @@ +# Authors: +# Petr Vobornik pvobo...@redhat.com +# +# Copyright (C) 2014 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/. + + +OTP Token Web UI Tests + + +import base64 +import hashlib +import hmac +import struct +import re +import time +from urlparse import urlparse + +from ipatests.test_webui.ui_driver import UI_driver +from ipatests.test_webui.ui_driver import screenshot +import ipatests.test_webui.data_user as user + +ENTITY = 'otptoken' +TOKEN_RE = r'otpauth://(hotp|totp)/.*:(?Ptokenid.*)\?' + +USER_ID = u'tuser1' +USER_PW = u'Secret123' +USER_ADD_DATA = { +'givenname': u'test', +'sn':
Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation
On 7.5.2015 08:59, David Kupka wrote: On 05/06/2015 03:20 PM, Martin Basti wrote: On 05/05/15 15:00, Martin Basti wrote: On 30/04/15 15:37, David Kupka wrote: On 04/24/2015 02:56 PM, Martin Basti wrote: Patches attached. Hi, thanks for patches. 1. You changed message in DNSServerNotRespondingWarning class but not the test in ipatest/test_xmlrpc/test_dns_plugin.py nitpick. Please spell 'edns' correctly. I've seen several instances of 'ends'. Thank you, updated patches attached: * new error messages * logging to debug log server output if exception was raised * fixed test * fixed spelling Fixed tests (again) Updated patches attached The code looks good to me and tests are no longer broken. (I would prefer better fix of the tests but given that the priorities are different now it can wait.) Petr, can you please confirm that the patch set works for you? Sorry, NACK: $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: ERROR: an internal error has occurred # /var/log/httpd/error_log ipa: ERROR: non-public: AssertionError: Traceback (most recent call last): File /usr/lib/python2.7/site-packages/ipaserver/rpcserver.py, line 350, in wsgi_execute result = self.Command[name](*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 443, in __call__ ret = self.run(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 760, in run return self.execute(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line , in execute **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line 4405, in _warning_if_forwarders_do_not_work log=self.log) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 715, in validate_dnssec_zone_forwarder_step2 timeout=timeout) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 610, in _resolve_record assert isinstance(nameserver_ip, basestring) AssertionError ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(DNS name ptr.test., idnsforwarders=(u'10.34.47.236',), all=False, raw=False, version=u'2.116'): AssertionError This is constantly reproducible in my vm-090.abc. Let me know if you want to take a look. I'm attaching little response.patch which improves compatibility with older python-dns packages. This patch allows IPA to work while error messages are simply not as nice as they could be with latest python-dns :-) check_fwd_msg.patch is a little nitpick, just to make sure everyone understands the message. BTW why some messages in check_forwarders() are printed using 'print' and others using logger? I would prefer to use logger for everything to make sure that logs contain all the information, including warnings. Thank you for your time! -- Petr^2 Spacek --- a/ipalib/util.py 2015-05-07 11:33:28.67900 +0200 +++ b/ipalib/util.py 2015-05-07 11:54:54.96000 +0200 @@ -593,7 +593,7 @@ assert isinstance(e, DNSException) if log is not None: -response = e.kwargs.get('response') +response = getattr(e, 'kwargs', {}).get('response') if response: log.debug(DNSException: %s; server response: %s, e, response) diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index 725324c..77ff342 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -464,7 +464,7 @@ def check_reverse_zones(ip_addresses, reverse_zones, options, unattended, search return ret_reverse_zones def check_forwarders(dns_forwarders, logger): -print Checking forwarders, please wait ... +print Checking DNS forwarders, please wait ... forwarders_dnssec_valid = True for forwarder in dns_forwarders: logger.debug(Checking DNS server: %s, forwarder) -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 800 rpc-client: add forms based auth support
On 05/06/2015 04:25 PM, Milan Kubik wrote: On 02/19/2015 03:51 PM, Petr Vobornik wrote: This patch is a prerequisite for patch 801 which will follow. It was developed to enable to use ipalib RPC client in Web UI tests. Plus it will enable to significantly speed up Web UI tests suite (if preparation of data is transformed to use this method). Partly related https://fedorahosted.org/freeipa/ticket/4772 and https://fedorahosted.org/freeipa/ticket/4307 Leverage session support to enable forms-based authenticate in rpc client. In order to do that session support in KerbTransport was moved to new SessionTransport. RPCClient.create_connection is then modified to force forms-based auth if new optional options - user and password are specified. For this case SessionTransport is used and user is authenticated by calling 'https://ipa.server/ipa/session/login_password'. Session cookie is stored and used in subsequent calls. This feature is usable for use cases where one wants to call the API without being on ipa client. Non-being on ipa client also means that IPA's NSS database and configuration is not available. Therefore one has to define ~/.ipa/default.conf in a similar way as ipa client does and prepare a NSS database with IPA CA cert. Usage: api.Backend.rpcclient.connect( nss_dir=my_nss_dir_path, user=user, password=password ) It's possible to switch users with: api.Backend.rpcclient.disconnect() api.Backend.rpcclient.connect( nss_dir=my_nss_dir_path, user=other_user, password=other_password ) Or check connection with: api.Backend.rpcclient.isconnected() Example: download a CA cert and add it to a new temporary NSS database: from urllib2 import urlparse from ipaplatform.paths import paths from ipapython import certdb, ipautil from ipapython.ipautil import run from ipalib import x509 # create new NSSDatabase tmp_db = certdb.NSSDatabase() pwd_file = ipautil.write_tmp_file(ipautil.ipa_generate_password()) tmp_db.create_db(pwd_file.name) # download and add cert url = urlparse.urlunparse(('http', ipautil.format_netloc(ipa_server), '/ipa/config/ca.crt', '', '', '')) stdout, stderr, rc = run([paths.BIN_WGET, -O, -, url]) certs = x509.load_certificate_list(stdout, tmp_db.secdir) ca_certs = [cert.der_data for cert in certs] for i, cert in enumerate(ca_certs): tmp_db.add_cert(cert, 'CA certificate %d' % (i + 1), 'C,,') my_nss_dir_path = tmp_db.secdir thanks for the patch. Please, fix the pep8 complaints. I've fixed existing E128 error in imports. But the remaining: $ git diff HEAD~1 -U0 | pep8 --diff ./ipalib/rpc.py:518:80: E501 line too long (86 79 characters) ./ipalib/rpc.py:524:80: E501 line too long (84 79 characters) ./ipalib/rpc.py:609:80: E501 line too long (80 79 characters) ./ipalib/rpc.py:634:80: E501 line too long (82 79 characters) ./ipalib/rpc.py:641:80: E501 line too long (94 79 characters) ./ipalib/rpc.py:796:80: E501 line too long (80 79 characters) ./ipalib/rpc.py:800:80: E501 line too long (82 79 characters) ./ipalib/rpc.py:913:80: E501 line too long (84 79 characters) I won't fix. Since it's just E501 in moved code. Can someone else look at the code as well, please? Thanks, Milan -- Petr Vobornik From 39ba81ccbe184c50d9f3a55a076f50396a6e66cf Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Wed, 10 Dec 2014 18:57:51 +0100 Subject: [PATCH] rpc-client: add forms based auth support Leverage session support to enable forms-based authenticate in rpc client. In order to do that session support in KerbTransport was moved to new SessionTransport. RPCClient.create_connection is then modified to force forms-based auth if new optional options - user and password are specified. For this case SessionTransport is used and user is authenticated by calling 'https://ipa.server/ipa/session/login_password'. Session cookie is stored and used in subsequent calls. This feature is usable for use cases where one wants to call the API without being on ipa client. Non-being on ipa client also means that IPA's NSS database and configuration is not available. Therefore one has to define ~/.ipa/default.conf in a similar way as ipa client does and prepare a NSS database with IPA CA cert. Usage: api.Backend.rpcclient.connect( nss_dir=my_nss_dir_path, user=user, password=password ) It's possible to switch users with: api.Backend.rpcclient.disconnect() api.Backend.rpcclient.connect( nss_dir=my_nss_dir_path, user=other_user, password=other_password ) Or check connection with: api.Backend.rpcclient.isconnected() Example: download a CA cert and add it to a new temporary NSS database: from urllib2 import urlparse from ipaplatform.paths import paths from ipapython import certdb, ipautil from ipapython.ipautil
[Freeipa-devel] [PATCH] 827 Update BUILD.txt
Add note about `dnf builddep` command and link to http://www.freeipa.org/page/Build page which contains information about copr repos -- Petr Vobornik From 59c7073f5eb30c79c71e70a2d692a267e47a0e3a Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Thu, 7 May 2015 14:53:45 +0200 Subject: [PATCH] Update BUILD.txt Add note about `dnf builddep` command and link to http://www.freeipa.org/page/Build page which contains information about copr repos --- BUILD.txt | 29 ++--- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/BUILD.txt b/BUILD.txt index 40121041cc63b7f5891b35fde66e69711f294b91..6a28beba1e0844971fb5625c0e1adf3f0c0fc0e3 100644 --- a/BUILD.txt +++ b/BUILD.txt @@ -3,23 +3,30 @@ Here is a quickie guide to get you started in IPA development. Dependencies +For more information, see http://www.freeipa.org/page/Build + The quickest way to get the dependencies needed for building is: +# dnf builddep -b freeipa.spec.in + +or + # yum install rpm-build `grep ^BuildRequires freeipa.spec.in | awk '{ print $2 }' | grep -v ^/` -This is currently (2014-02-11): +This is currently (2015-05-07): yum install rpm-build 389-ds-base-devel svrcore-devel policycoreutils \ -systemd-units samba-devel samba-python libwbclient-devel samba4-devel \ -samba4-python libtalloc-devel libtevent-devel nspr-devel nss-devel \ -openssl-devel openldap-devel krb5-devel krb5-workstation libuuid-devel \ -libcurl-devel xmlrpc-c-devel popt-devel autoconf automake m4 libtool gettext \ -python-devel python-ldap python-setuptools python-krbV python-nss \ -python-netaddr python-kerberos python-rhsm pyOpenSSL pylint python-polib \ -libipa_hbac-python python-memcached sssd python-lxml python-pyasn1 \ -python-qrcode python-dns m2crypto check libsss_idmap-devel \ -libsss_nss_idmap-devel java-1.7.0-openjdk libverto-devel systemd \ -libunistring-devel python-lesscpy +systemd-units samba-devel samba-python libwbclient-devel libtalloc-devel \ +libtevent-devel nspr-devel nss-devel openssl-devel openldap-devel krb5-devel \ +krb5-workstation libuuid-devel libcurl-devel xmlrpc-c-devel popt-devel \ +autoconf automake m4 libtool gettext python-devel python-ldap \ +python-setuptools python-krbV python-nss python-netaddr python-kerberos \ +python-rhsm pyOpenSSL pylint python-polib libipa_hbac-python python-memcached \ +sssd python-lxml python-pyasn1 python-qrcode-core python-dns m2crypto \ +check libsss_idmap-devel libsss_nss_idmap-devel java-headless rhino \ +libverto-devel systemd libunistring-devel python-lesscpy python-yubico \ +python-backports-ssl_match_hostname softhsm-devel openssl-devel \ +p11-kit-devel pki-base python-pytest-multihost python-pytest-sourceorder Building -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0007] Changed in-tree development setup instructions
On 01/03/2015 09:53 PM, Thorsten Scherf wrote: Changed in-tree development setup instructions Instructions on how to setup an in-tree development server were not were clear in the existing BUILD.txt. Setup procedure has been extended and corrected. Hello, thanks for the patch. There were trailing whitespace errors, but I've fixed them. ACK Pushed to master: 83e2552cdd99e67415148c0a7a317f3e3c45b831 -- Petr Vobornik From 61fe46d3d5761af1256fb8432a6a034eb11ad52e Mon Sep 17 00:00:00 2001 From: Thorsten Scherf tsch...@redhat.com Date: Sat, 3 Jan 2015 21:43:39 +0100 Subject: [PATCH] Changed in-tree development setup instructions Instructions on how to setup an in-tree development server were not were clear in the existing BUILD.txt. Setup procedure has been extended and corrected. --- BUILD.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/BUILD.txt b/BUILD.txt index 62cbbd95b123128e978ad25907ba4d67ded1ffd4..40121041cc63b7f5891b35fde66e69711f294b91 100644 --- a/BUILD.txt +++ b/BUILD.txt @@ -47,8 +47,10 @@ install the rpms and then configure IPA using ipa-server-install. Get a TGT for the admin user with: kinit admin Next you'll need 2 sessions in the source tree. In the first session run -python lite-server.py. In the second session you can run the ./ipa -tool and it will make requests to the lite-server listening on 127.0.0.1:8080. +python lite-server.py. In the second session copy /etc/ipa/default.conf into +~/.ipa/default.conf and replace xmlrpc_uri with http://127.0.0.1:/ipa/xml. +Finally run the ./ipa tool and it will make requests to the lite-server +listening on 127.0.0.1:. This makes developing plugins much faster and you can also make use of the Python pdb debugger on the server side. -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0026] ipa-server-install: deprecate manual setting of master KDC password
On 04/01/2015 05:07 PM, Martin Babinsky wrote: https://fedorahosted.org/freeipa/ticket/4516 ACK Pushed to master: 059a4c188760ec7360ccb68a5c8a292afb21d35e -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0001 Test Objectclass of postdetach group
On 03/05/2015 07:29 AM, Lenka Ryznarova wrote: Patch related to ticket https://fedorahosted.org/freeipa/ticket/4909 Lenka 1. There is a trailing whitespace test_group_plugin.py:1128, I've fixed it. ACK Pushed to master: b7af1825468720dfac6ee1259c845ec70d12ca43 -- Petr Vobornik From daa87a775f29d8ffd81fdffedd85b81ab0f7519a Mon Sep 17 00:00:00 2001 From: Lenka Ryznarova lenka.ryznar...@gmail.com Date: Wed, 4 Mar 2015 16:47:04 +0100 Subject: [PATCH] Test Objectclass of postdetach group Add regression test to check whether a post detach group has a full set of objectclass. Add regression test to check whether group-add-member is successfull for a post detach group. https://fedorahosted.org/freeipa/ticket/4909 --- ipatests/test_xmlrpc/test_group_plugin.py | 69 +++ 1 file changed, 69 insertions(+) diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index 26d71c4fae1a312709f6f27e0397ef3315cba33f..af9e653ff2d1accca99a859ba88beb16f278d4a8 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -1078,3 +1078,72 @@ class test_group_remove_group_from_protected_group(Declarative): ), ), ] + +class test_group_full_set_of_objectclass_not_available_post_detach(Declarative): +# https://fedorahosted.org/freeipa/ticket/4909#comment:1 +cleanup_commands = [ +('group_del', [user1], {}), +('user_del', [user1], {}), +] + +tests = [ +dict( +desc='Create %r' % user1, +command=( +'user_add', [], dict(givenname=u'Test', sn=u'User1') +), +expected=dict( +value=user1, +summary=u'Added user %s' % user1, +result=get_user_result(user1, u'Test', u'User1', 'add'), +), +), + +dict( +desc='Detach managed group %r' % user1, +command=('group_detach', [user1], {}), +expected=dict( +result=True, +value=user1, +summary=u'Detached group %s from user %s' % (user1, user1), +), +), + +dict( +desc='Show group - check objectclass', +command=('group_show', [user1], dict(all=True)), +expected=dict( +value=user1, +result={ +'cn':[user1], +'description': [u'User private group for tuser1'], +'gidnumber': [fuzzy_digits], +'dn': get_group_dn('tuser1'), +'ipauniqueid': [fuzzy_uuid], +'objectclass': objectclasses.posixgroup, +}, +summary=None, +), +), + +dict( +desc='Add member back to the detached group', +command=('group_add_member', [user1], dict(user=user1)), +expected=dict( +completed=1, +failed=dict( +member=dict( +group=tuple(), +user=tuple(), +), +), +result={ +'dn': get_group_dn('tuser1'), +'member_user': [user1], +'gidnumber': [fuzzy_digits], +'cn': [user1], +'description': [u'User private group for tuser1'], +}, +), +), +] \ No newline at end of file -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0028] update 'api.env.ca_host' if a different hostname is used during server install
On 04/13/2015 05:54 PM, Martin Babinsky wrote: https://fedorahosted.org/freeipa/ticket/4936 ACK Pushed to master: 825d4fc9e7fc80e07a68daf35b5eb0c171e821af -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 823 ipaldap: raise DatabaseError on unbind if disconnected
On 04/24/2015 01:31 PM, Petr Vobornik wrote: On 04/24/2015 07:19 AM, Jan Cholasta wrote: Dne 23.4.2015 v 22:18 Nathaniel McCallum napsal(a): On Thu, 2015-04-23 at 14:12 +0200, Petr Vobornik wrote: On 04/23/2015 12:24 PM, Petr Vobornik wrote: If unbind was called when disconnected it raised: AttributeError: 'NoneType' object has no attribute 'unbind_s' AttributeError is not a public error and therefore it prevented ldap2.destroy_connection() to be called multiple times. fixes: https://fedorahosted.org/freeipa/ticket/4991 Note: this issue also prevented rpcserver.change_password from working. Therefore I think that there might have been an error in recent ipaldap refactoring and if #4991 was not run on master then there might have been other issue, which probably have been fixed by the refactoring. After discussion with Honza, the approach was changed. Also I've added patch which removes unnecessary incorrect code which revealed the regression. Additional testing shows that these patches actually don't fix the original issue of #4991. See https://fedorahosted.org/freeipa/ticket/4991#comment:4 0823 - ACK 0824 - ACK Nathaniel I would prefer if the connection was closed manually in patch 824, IMO it is a good practice to release resources once you are done with them just in time, and I don't think you can always trust the automatic disconnect at the end of request. Changed (also in user-status command). ACK -- Martin^3 Babinsky -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 823 ipaldap: raise DatabaseError on unbind if disconnected
On 05/07/2015 06:04 PM, Martin Babinsky wrote: On 04/24/2015 01:31 PM, Petr Vobornik wrote: On 04/24/2015 07:19 AM, Jan Cholasta wrote: Dne 23.4.2015 v 22:18 Nathaniel McCallum napsal(a): On Thu, 2015-04-23 at 14:12 +0200, Petr Vobornik wrote: On 04/23/2015 12:24 PM, Petr Vobornik wrote: If unbind was called when disconnected it raised: AttributeError: 'NoneType' object has no attribute 'unbind_s' AttributeError is not a public error and therefore it prevented ldap2.destroy_connection() to be called multiple times. After discussion with Honza, the approach was changed. Also I've added patch which removes unnecessary incorrect code which revealed the regression. Additional testing shows that these patches actually don't fix the original issue of #4991. See https://fedorahosted.org/freeipa/ticket/4991#comment:4 0823 - ACK 0824 - ACK Nathaniel I would prefer if the connection was closed manually in patch 824, IMO it is a good practice to release resources once you are done with them just in time, and I don't think you can always trust the automatic disconnect at the end of request. Changed (also in user-status command). ACK 823-2: Pushed to master: 7d10547ae3098b96762846ff36e813042a503d59 824-2: Pushed to master: 375eb7583334f231c27420c80b1b074e0cc554fe -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation
On 07/05/15 12:19, Petr Spacek wrote: On 7.5.2015 08:59, David Kupka wrote: On 05/06/2015 03:20 PM, Martin Basti wrote: On 05/05/15 15:00, Martin Basti wrote: On 30/04/15 15:37, David Kupka wrote: On 04/24/2015 02:56 PM, Martin Basti wrote: Patches attached. Hi, thanks for patches. 1. You changed message in DNSServerNotRespondingWarning class but not the test in ipatest/test_xmlrpc/test_dns_plugin.py nitpick. Please spell 'edns' correctly. I've seen several instances of 'ends'. Thank you, updated patches attached: * new error messages * logging to debug log server output if exception was raised * fixed test * fixed spelling Fixed tests (again) Updated patches attached The code looks good to me and tests are no longer broken. (I would prefer better fix of the tests but given that the priorities are different now it can wait.) Petr, can you please confirm that the patch set works for you? Sorry, NACK: $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: ERROR: an internal error has occurred # /var/log/httpd/error_log ipa: ERROR: non-public: AssertionError: Traceback (most recent call last): File /usr/lib/python2.7/site-packages/ipaserver/rpcserver.py, line 350, in wsgi_execute result = self.Command[name](*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 443, in __call__ ret = self.run(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 760, in run return self.execute(*args, **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line , in execute **options) File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line 4405, in _warning_if_forwarders_do_not_work log=self.log) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 715, in validate_dnssec_zone_forwarder_step2 timeout=timeout) File /usr/lib/python2.7/site-packages/ipalib/util.py, line 610, in _resolve_record assert isinstance(nameserver_ip, basestring) AssertionError ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(DNS name ptr.test., idnsforwarders=(u'10.34.47.236',), all=False, raw=False, version=u'2.116'): AssertionError This is constantly reproducible in my vm-090.abc. Let me know if you want to take a look. I'm attaching little response.patch which improves compatibility with older python-dns packages. This patch allows IPA to work while error messages are simply not as nice as they could be with latest python-dns :-) check_fwd_msg.patch is a little nitpick, just to make sure everyone understands the message. BTW why some messages in check_forwarders() are printed using 'print' and others using logger? I would prefer to use logger for everything to make sure that logs contain all the information, including warnings. Thank you for your time! Thank you, fixed. I added missing except block after forwarders validation step2. Updated patch attached. -- Martin Basti From 5dcd3704914a830b6b33837ded1448b0035b3726 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 22 Apr 2015 15:29:21 +0200 Subject: [PATCH 1/2] DNSSEC: Improve global forwarders validation Validation now provides more detailed information and less false positives failures. https://fedorahosted.org/freeipa/ticket/4657 --- ipalib/messages.py | 23 +- ipalib/plugins/dns.py | 64 +--- ipalib/util.py | 130 ++-- ipaserver/install/bindinstance.py | 32 +--- ipatests/test_xmlrpc/test_dns_plugin.py | 5 +- 5 files changed, 187 insertions(+), 67 deletions(-) diff --git a/ipalib/messages.py b/ipalib/messages.py index b44beca729f5483a7241e4c98a9f724ed663e70f..236b683b30692d88e5257d9189c559dd9f848885 100644 --- a/ipalib/messages.py +++ b/ipalib/messages.py @@ -179,14 +179,14 @@ class OptionSemanticChangedWarning(PublicMessage): u%(hint)s) -class DNSServerNotRespondingWarning(PublicMessage): +class DNSServerValidationWarning(PublicMessage): -**13006** Used when a DNS server is not responding to queries +**13006** Used when a DNS server is not to able to resolve query errno = 13006 type = warning -format = _(uDNS server %(server)s not responding.) +format = _(uDNS server %(server)s: %(error)s.) class DNSServerDoesNotSupportDNSSECWarning(PublicMessage): @@ -196,10 +196,11 @@ class DNSServerDoesNotSupportDNSSECWarning(PublicMessage): errno = 13007 type = warning -format = _(uDNS server %(server)s does not support DNSSEC. +format = _(uDNS server %(server)s does not support DNSSEC: %(error)s.\n uIf DNSSEC validation is enabled on IPA server(s), uplease disable it.) + class ForwardzoneIsNotEffectiveWarning(PublicMessage): **13008**
Re: [Freeipa-devel] [PATCH] 827 Update BUILD.txt
On 07/05/15 15:57, Petr Vobornik wrote: Add note about `dnf builddep` command and link to http://www.freeipa.org/page/Build page which contains information about copr repos ACK -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 827 Update BUILD.txt
On 05/07/2015 05:06 PM, Martin Basti wrote: On 07/05/15 15:57, Petr Vobornik wrote: Add note about `dnf builddep` command and link to http://www.freeipa.org/page/Build page which contains information about copr repos ACK Pushed to master: b88f5333ec039cf8a40c0377323554a451730ac7 -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] manage replication topology in the shared tree
On 04/29/2015 11:18 AM, Ludwig Krispenz wrote: Hi, thanks again, so there is some work to do, but see some answers inline On 04/27/2015 10:18 AM, thierry bordaz wrote: On 04/21/2015 10:49 AM, Ludwig Krispenz wrote: On 04/20/2015 06:00 PM, thierry bordaz wrote: On 04/13/2015 10:56 AM, Ludwig Krispenz wrote: Hi, in the attachment you find the latest state of the topology plugin, it implements what is defined in the design page: http://www.freeipa.org/page/V4/Manage_replication_topology (which is also waiting for a reviewer) It contains the plugin itself and a core of ipa commands to manage a topology. to be really applicable, some work outside is required, eg the management of the domain level and a decision where the binddn group should be maintained. Thanks, Ludwig Hello Ludwig, Quite long review to do. So far I only looked at the startup phase and I have only few questions and comments. Thanks for your time, and I'm looking forward to your review of the other parts, you raise some valid points. I'll try to answer some of them inline, but will integrate some into a next version of the patch In ipa_topo_start, do you need to get argc/argv as you are not using plugin-argxx attributes ? no. It was a leftover from a standard plugin topo_plugin_conf configuration parameters are not freed when the plugin is closed. Is it closed only at shutdown ? Also I would initiatlize it to {NULL}. So far it is not planned to be dynamic, but I will addres the memory management In case the config does not contain any nsslapd-topo-plugin-shared-replica-root, I wonder if ipa_topo_apply_shared_config may crash as shared_replica_root will be NULL. or at least in ipa_topo_apply_shared_replica_config/ipa_topo_util_get_replica_conf. Also if nsslapd-topo-plugin-shared-replica-root contains an invalid root suffix (typo), topoRepl remains NULL and ipa_topo_util_get_replica_conf/ipa_topo_cfg_replica_add can crash. for the two comments above, I was assuming that plugin conf and shared tree would be setup by ipa tools and server setup, so assuming only valid data, but you are right, checking for bad data doesn't hurt. In ipa_topo_util_segment_from_entry, if the config entry has no direction/left/right it will crash. Shouldn't it return an error if the config is invalid. adding a segment should be done with the ipa command 'ipa topologysegment-add ...' and this always provides a direction (param or default). If you try to add a segment directly, direction is a required attribute of teh segment objectclass, so it should be rejected- The update of domainLevel may start the plugin. If two mods update the domainLevel they could be done in parallele. yes :-( In ipa_topo_util_update_agmt_list, if there is a marked agmnt but no segment it deletes the agreement. Is it possible there is a segment but no agmnt ? For example, if the server were stopped or crashed after the segment was created but before the local config was updated. then it should be created from the segment Hosts are taken from shared config tree (cn=masters,cfg), is it possible to have a replica agreement to a host that is not under 'cn=masters,cfg' yes, it will be ignored by the plugin thanks thierry Hi Ludwig, I continued the review of the design/topology plugin code. This is really an interesting plugin but unfortunately I have not yet reviewed all the parts. I went through the design and digging the related parts in the code. So far I need to review the rest starting at http://www.freeipa.org/page/V4/Manage_replication_topology#connectivity_management. I think I did ~50% design but may be more than 50% of the code. Here are additional points: in ipa_topo_set_domain_level, you may record the new Domain level value as FATAL (it is already recorded in case of oneline import) this just records the actual domain level, I don't think we need to log it every time, only if it is changed should be sufficient (to verify) ipa_topo_be_state_change is called for any backend going online. Domain level and start should be done only for a backend mapping a shared-replica-root. yes, the comment is already there, but the actual check isn't, so it would be recreated at online init of any backend, think it is not too bad, but will change it Also the plugin can be started many times (each online init), ipa_topo_util_start is not protected by a lock Some fields will leak (in ipa_topo_init_shared_config) Also I wonder if you reinit several times the same replica-root, its previous config will leak. (replica-repl_segments) you shouldn't :-), but needs to be made safer In ipa_topo_apply_shared_replica_config, I do not see where replica_config is kept (leak ?) it is created and set to the shared_conf data, but if it aready exists, it will leak (that's probably the case above), it should be freed when the plugin is stopped
Re: [Freeipa-devel] [PATCH] manage replication topology in the shared tree
Thanks, I will look into it and try to add what's missing and also try to make the design a bit more clear. Ludwig On 05/07/2015 04:32 PM, thierry bordaz wrote: On 04/29/2015 11:18 AM, Ludwig Krispenz wrote: Hi, thanks again, so there is some work to do, but see some answers inline On 04/27/2015 10:18 AM, thierry bordaz wrote: On 04/21/2015 10:49 AM, Ludwig Krispenz wrote: On 04/20/2015 06:00 PM, thierry bordaz wrote: On 04/13/2015 10:56 AM, Ludwig Krispenz wrote: Hi, in the attachment you find the latest state of the topology plugin, it implements what is defined in the design page: http://www.freeipa.org/page/V4/Manage_replication_topology (which is also waiting for a reviewer) It contains the plugin itself and a core of ipa commands to manage a topology. to be really applicable, some work outside is required, eg the management of the domain level and a decision where the binddn group should be maintained. Thanks, Ludwig Hello Ludwig, Quite long review to do. So far I only looked at the startup phase and I have only few questions and comments. Thanks for your time, and I'm looking forward to your review of the other parts, you raise some valid points. I'll try to answer some of them inline, but will integrate some into a next version of the patch In ipa_topo_start, do you need to get argc/argv as you are not using plugin-argxx attributes ? no. It was a leftover from a standard plugin topo_plugin_conf configuration parameters are not freed when the plugin is closed. Is it closed only at shutdown ? Also I would initiatlize it to {NULL}. So far it is not planned to be dynamic, but I will addres the memory management In case the config does not contain any nsslapd-topo-plugin-shared-replica-root, I wonder if ipa_topo_apply_shared_config may crash as shared_replica_root will be NULL. or at least in ipa_topo_apply_shared_replica_config/ipa_topo_util_get_replica_conf. Also if nsslapd-topo-plugin-shared-replica-root contains an invalid root suffix (typo), topoRepl remains NULL and ipa_topo_util_get_replica_conf/ipa_topo_cfg_replica_add can crash. for the two comments above, I was assuming that plugin conf and shared tree would be setup by ipa tools and server setup, so assuming only valid data, but you are right, checking for bad data doesn't hurt. In ipa_topo_util_segment_from_entry, if the config entry has no direction/left/right it will crash. Shouldn't it return an error if the config is invalid. adding a segment should be done with the ipa command 'ipa topologysegment-add ...' and this always provides a direction (param or default). If you try to add a segment directly, direction is a required attribute of teh segment objectclass, so it should be rejected- The update of domainLevel may start the plugin. If two mods update the domainLevel they could be done in parallele. yes :-( In ipa_topo_util_update_agmt_list, if there is a marked agmnt but no segment it deletes the agreement. Is it possible there is a segment but no agmnt ? For example, if the server were stopped or crashed after the segment was created but before the local config was updated. then it should be created from the segment Hosts are taken from shared config tree (cn=masters,cfg), is it possible to have a replica agreement to a host that is not under 'cn=masters,cfg' yes, it will be ignored by the plugin thanks thierry Hi Ludwig, I continued the review of the design/topology plugin code. This is really an interesting plugin but unfortunately I have not yet reviewed all the parts. I went through the design and digging the related parts in the code. So far I need to review the rest starting at http://www.freeipa.org/page/V4/Manage_replication_topology#connectivity_management. I think I did ~50% design but may be more than 50% of the code. Here are additional points: in ipa_topo_set_domain_level, you may record the new Domain level value as FATAL (it is already recorded in case of oneline import) this just records the actual domain level, I don't think we need to log it every time, only if it is changed should be sufficient (to verify) ipa_topo_be_state_change is called for any backend going online. Domain level and start should be done only for a backend mapping a shared-replica-root. yes, the comment is already there, but the actual check isn't, so it would be recreated at online init of any backend, think it is not too bad, but will change it Also the plugin can be started many times (each online init), ipa_topo_util_start is not protected by a lock Some fields will leak (in ipa_topo_init_shared_config) Also I wonder if you reinit several times the same replica-root, its previous config will leak. (replica-repl_segments) you shouldn't :-), but needs to be made safer In ipa_topo_apply_shared_replica_config, I do not see where replica_config is kept (leak ?) it is
Re: [Freeipa-devel] [PATCH] 814-818 migrate-ds: optimize adding users to default group
On 07/05/15 13:17, Petr Vobornik wrote: On 05/06/2015 04:49 PM, Martin Basti wrote: On 05/05/15 10:59, Petr Vobornik wrote: On 04/30/2015 03:53 PM, Martin Basti wrote: On 10/04/15 12:55, Petr Vobornik wrote: The essential patch is 814. 815 a proposal for new option. 816 and 818 are cleanup patches. 817 little optimization. == [PATCH] 814 migrate-ds: optimize adding users to default group == Migrate-ds searches for user without a group and adds them to default group. There is no point in checking if the user's selected by previous query are not member of default group because they are not member of any group. The operation is also speeded up by not fetching the default group. Users are added right away. https://fedorahosted.org/freeipa/ticket/4950 NACK Users haven't been added into ipa default group after migration. Fixed in patch 815. Just nitpick 1) too many parentheses api.log.error(('Adding new members to default group failed: %s \n' 'members: %s') % (e, (','.join(member_dns You can use this instead: api.log.error('Adding new members to default group failed: %s \n' 'members: %s', e, ','.join(member_dns)) Fixed. == [PATCH] 815 migrate-ds: skip default group options == New option --use-default-group=False could be used to disable adding of migrated users into default group. By default, the default group is no longer POSIX therefore it doesn't fulfill the original idea of providing GID and therefore it could be skipped during migration. LGTM the option got so the default would be applied. +autofill=True, == [PATCH] 816 migrate-ds: remove unused def_group_gid context property == it's no longer used anywhere 1) You can remove the unused variable 'g_attrs' removed == [PATCH] 817 migrate-ds: optimize gid checks by utilizing dictionary nature of set == LGTM == [PATCH] 818 migrate-ds: log migrated group members only on debug level == It pollutes error_log. 1) you do not need % formatting in logger api.log.debug('migrating %s group %s' , member_attr, m) fixed and also changed migrating %s user %s' line to debug, which was the actual reason for this patch. Martin^2 Thanks. 1) Please create new API file, probably missing autofill in API.txt: Option 'use_def_group?' in command 'migrate_ds' in API file not found Options count in migrate_ds of 18 doesn't match expected: 19 Option use_def_group? of command migrate_ds in ipalib, not in API file: Bool('use_def_group?', autofill=True, cli_name='use_default_group', default=True) There are one or more changes to the API. Either undo the API changes or update API.txt and increment the major version in VERSION. you could just wrote that I forgot to run ./makeapi ;) 2) ipa: error: --use-default-group option does not take a value Attached new patch #826 which should fix the issue. Not sure if Honza(CCd) will like it. (default: true) added to option description for better UX. and a nitpick: patch 814 1) Why this change? -api.log.debug('Adding %d users to group%s duration %s', -len(new_members), mode, d) +api.log.info('Adding %d users to group%s duration %s', + len(member_dns), mode, d) So that there will be a record in a default(not debug) log that something happened. The reason is that it also affects users, without a group, that are already present on the system. As we personally talked with Honza, and he agreed with param modification, then ACK for all patches. -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0032] prevent duplicate IDs when setting up multiple replicas against single master
On 05/06/2015 07:41 PM, thierry bordaz wrote: On 05/06/2015 05:56 PM, Martin Babinsky wrote: On 05/06/2015 04:25 PM, thierry bordaz wrote: On 05/06/2015 03:19 PM, Martin Babinsky wrote: Hello Thierry, replies are inline. On 05/06/2015 02:22 PM, thierry bordaz wrote: On 05/06/2015 01:54 PM, Martin Babinsky wrote: The attached patch tries to fix https://fedorahosted.org/freeipa/ticket/4378 After discussion with Thierry we concluded that while this issue is more complex than it seems, the transition from REPLACE to DEL/ADD operations when updating nsDS5ReplicaId should suffice for this ticket. Hello Martin, Few comments, you are using MOD_DEL 'replicaID' with None value. So this is going to delete all previous values and it should be equivalent to a MOD_REPL. I was thinking you wanted to retrieve the id_value and call MOD_DEL 'replicaID' current_value. So that if by the time you fetched the replicaId, an other replica updated the replicaId, the MOD_DEL/MOD_ADD would fail and you need a new iteration. Sorry I didn't know you can MOD_DEL a particular value (I'm LDAP noob I know). Will fix this. If replicaId was multi-valued and you want to make it single valued, you may want to do create a more complex MOD (e.g. (ldap.MOD_DELETE, 'nsDS5ReplicaId', str(value1), (ldap.MOD_DELETE, 'nsDS5ReplicaId', str(value2)...) AFAIK ReplicaId is single-valued (looking at the schema right now) so this shouldn't be problem. If it is updating successfully do you want to return 'retval' or 'retval+1' ? If several replicas try to update the replicaId of the master and the current replicaId is 1000. Replica1 successfully updates the replicaId and gets 1001 as the new value. Replica2 successfully updates the replicaId and gets 1002. The final value on master will be 1002, but replica1 will assum it is 1001. Is it a problem ? I studied the code in the master branch and IIUC (and please correct me if I got this wrong) nsDS5ReplicaId attribute in 'cn=replication,cn=etc,$SUFFIX' represents replicaID of the _next_ replica that will be installed. So if a replica is installed, it sets the current value of nsDS5ReplicaId as its replica ID (the function returns 'retval') and then increments it in 'cn=replication,cn=etc,$SUFFIX' entry ('retval + 1' is written to master) so that the next installed replica fetches this updated value. So the case you described should be the expected behavior. To change it would require different patch IMHO. Thank for your precious explanations, in fact the value in 'cn=replication,cn=etc,SUFFIX' is the next available replicaId value and is the centralized mechanism that assign unique replicaID. The risk was that several replica pick the same value. So yes it is important that the MOD_DEL specifies the previously read value so that the test/set will be atomic. If several replicas read the same value, only the faster one will use it to install the replica. thanks thierry thanks thierry Attaching updated patch with fixed MOD_DELETE operation. Hi Martin, The fix looks good to me except I think you need to do (ldap.MOD_DELETE, 'nsDS5ReplicaId', *str(*retval*)*) thanks thierry Attaching updated patch. -- Martin^3 Babinsky From 80f3bca039ada9c2b99d74b8e6022d48f46af0fb Mon Sep 17 00:00:00 2001 From: Martin Babinsky mbabi...@redhat.com Date: Mon, 4 May 2015 18:33:44 +0200 Subject: [PATCH] prevent duplicate IDs when setting up multiple replicas against single master This patch forces replicas to use DELETE+ADD operations to increment 'nsDS5ReplicaId' in 'cn=replication,cn=etc,$SUFFIX' on master, and retry multiple times in the case of conflict with another update. Thus when multiple replicas are set-up against single master none of them will have duplicate ID. https://fedorahosted.org/freeipa/ticket/4378 --- ipaserver/install/replication.py | 78 ++-- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py index 66764c22f69328942fe2e4581cfafb3806438d7c..4c16dc22507237945188f01d51a63a391b872b18 100644 --- a/ipaserver/install/replication.py +++ b/ipaserver/install/replication.py @@ -21,6 +21,7 @@ import time import datetime import sys import os +from random import randint import ldap @@ -230,34 +231,59 @@ class ReplicationManager(object): # Ok, either the entry doesn't exist or the attribute isn't set # so get it from the other master -retval = -1 +return self._get_and_update_id_from_master(master_conn) + +def _get_and_update_id_from_master(self, master_conn, attempts=5): + +Fetch replica ID from remote master and update nsDS5ReplicaId attribute +on 'cn=replication,cn=etc,$SUFFIX' entry. Do it as MOD_DELETE+MOD_ADD +operations and retry when conflict occurs, e.g. due to simultaneous +update from another replica. +:param master_conn: LDAP connection to master +
[Freeipa-devel] [PATCH] 828 webui: don't log in back after logout
Automatic login attempt is initiated by first failed xhr request which happens in metadata phase. New phase was added before metadata phase. It interrupts UI load and shows login page if it's directly after logout(marked in session storage). Successfull manual login resolves the phase so that metadata phase can follow. https://fedorahosted.org/freeipa/ticket/5008 -- Petr Vobornik From c31b8a1cb8c931a69748ac25a06b940a318accad Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Thu, 7 May 2015 19:14:08 +0200 Subject: [PATCH] webui: don't log in back after logout Automatic login attempt is initiated by first failed xhr request which happens in metadata phase. New phase was added before metadata phase. It interrupts UI load and shows login page if it's directly after logout(marked in session storage). Successfull manual login resolves the phase so that metadata phase can follow. https://fedorahosted.org/freeipa/ticket/5008 --- install/ui/src/freeipa/app_container.js | 18 +- install/ui/src/freeipa/ipa.js | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/install/ui/src/freeipa/app_container.js b/install/ui/src/freeipa/app_container.js index 7f5aacca0758f19ee6209f1b253158266b97aff4..0a49307e31f4875724ee430f32c9483a23c6b51e 100644 --- a/install/ui/src/freeipa/app_container.js +++ b/install/ui/src/freeipa/app_container.js @@ -21,13 +21,14 @@ define([ 'dojo/_base/lang', 'dojo/Deferred', +'dojo/on', 'dojo/when', './plugin_loader', './phases', './reg', './Application_controller', 'exports' -],function(lang, Deferred, when, plugin_loader, phases, reg, Application_controller, app) { +],function(lang, Deferred, on, when, plugin_loader, phases, reg, Application_controller, app) { /** * Application wrapper @@ -60,6 +61,21 @@ define([ return app; })); +phases.on('init', lang.hitch(this, function() { +var deferred = new Deferred(); +if (window.sessionStorage.getItem('logout')) { +window.sessionStorage.removeItem('logout'); +var login_facet = reg.facet.get('login'); +this.app.show_facet(login_facet); +on.once(login_facet, logged_in, function() { +deferred.resolve(); +}); +} else { +deferred.resolve(); +} +return deferred.promise; +})); + phases.on('metadata', lang.hitch(this, function() { var deferred = new Deferred(); diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js index 836ad689bd82253632c28f54b9a9386996c80b12..75dd73c379815a0e0e1dc2c4d786fdcf3be7c1b0 100644 --- a/install/ui/src/freeipa/ipa.js +++ b/install/ui/src/freeipa/ipa.js @@ -409,6 +409,7 @@ IPA.logout = function() { } function reload () { +window.sessionStorage.setItem('logout', true); var l = window.location; l.assign(l.href.split('#')[0]); } -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code