Re: [Freeipa-devel] [PATCH] 0001 Provide Kerberos over HTTP (MS-KKDCP)
On 2015-06-12 23:58, Adam Young wrote: So...I've been spoiled a bit by Gerrit. Here is what I just did to get them to apply: cd freeipa git clean -xdf . #use the -3 to do 3 way merge git am -3 ~/Documents/freeipa/patches/cheimes/freeipa-cheimes-0001-3-Provide-Kerberos-over-HTTP-MS-KKDCP.patch @git status show conflicts in both modified: install/share/Makefile.am both modified: ipaplatform/base/paths.py Which were due to this change and another making changes to the same section of the file, but they were accept both type conflicts Updated patch is attached. Christian, please confirm it is OK. Hi Adam, awesome! The three-way-merge option is a great trick. I didn't know it before. Your patch looks like the patch, that I was about to upload now. :) Christian signature.asc Description: OpenPGP digital signature -- 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] Unable to prepare a replica file on a replica
Hi all, In my letter from 06/09/2015 03:55 PM, I indicated 2 issues related to the topology plugin. One of them was later successfully fixed, another one is still there: ofayans@f22replica1:~]$ sudo ipa-replica-prepare --ip-address 192.168.122.140 f22replica2.bagam.net Directory Manager (existing master) password: Preparing replica for f22replica2.bagam.net from f22replica1.bagam.net Creating SSL certificate for the Directory Server Certificate issuance failed So, I am unable to prepare a replica on an existing replica - only on master. Do you have any ideas on how to deal with it? The corresponding line in dirsrv error.log is: [15/Jun/2015:06:33:51 -0400] - Entry uid=admin,ou=people,o=ipaca -- attribute krbExtraData not allowed -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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] 873-874 ipa-replica-manage: adjust del to work with managed topology
On 06/12/2015 04:18 PM, Petr Vobornik wrote:
Some notes:
1. As mentioned in the WIP patch thread: original 'del' worked also with
winsync agreements. I'm not sure why is that. Shouldn't 'disconnect' be
used for winsync agreements? At least man page says that. This patch
doesn't support it if domain level 0. Is it a blocker?
Following should be addressed in beta:
2. If `ipa-replica-manage del` is run before `ipa-csreplica-manage del`
then the `ipa-csreplica-manage del` will fail unless run with --force
options.
3. Check for orphaned server is missing. I want to use proper graph
traversing algorithm for that given that we have the whole topology.
4. Probably a work for topology plugin: I've seen that the removed
master doesn't remove its segments and agreements even though that it
knows about its removal (doesn't have its own entry in cn=masters). It
leads to failed replication connection attempts. Not a big issue, but
also not wanted.
Martin3 found that there is wrong hostname in one error message. Fixed.
Patch 873 rebased.
--
Petr Vobornik
From 4456f0a8b515a2b1db1b0e1a0725394150a1dce4 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 11 Jun 2015 15:38:32 +0200
Subject: [PATCH] server: add del command
this command is internal and is supposed to be used by ipa-replica-managed to
delete replica.
---
API.txt | 8
VERSION | 4 ++--
ipalib/plugins/server.py | 7 +++
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/API.txt b/API.txt
index 853d26a59bb5bb1ebff698924a36a30b7757c398..ff53e9457ebaa36004556feebd88515aea2a7a8d 100644
--- a/API.txt
+++ b/API.txt
@@ -3799,6 +3799,14 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: PrimaryKey('value', None, None)
+command: server_del
+args: 1,2,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=True, primary_key=True, query=True, required=True)
+option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Str('version?', exclude='webui')
+output: Output('result', type 'dict', None)
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: ListOfPrimaryKeys('value', None, None)
command: server_find
args: 1,10,4
arg: Str('criteria?', noextrawhitespace=False)
diff --git a/VERSION b/VERSION
index 741d50f2d9b7e564b6a480c73a378500e8d1aca1..2a835122143aa3a2e7c02a888f638ce5e5fcdf83 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=132
-# Last change: dkupka: User life cycle permissions naming and split
+IPA_API_VERSION_MINOR=133
+# Last change: pvoborni - add server-del internal command
diff --git a/ipalib/plugins/server.py b/ipalib/plugins/server.py
index d22f1ea368ad09ab2cff00429f509c99d92f0f60..7fc44197343dbb651782fbf79993cbbe8818efed 100644
--- a/ipalib/plugins/server.py
+++ b/ipalib/plugins/server.py
@@ -87,3 +87,10 @@ class server_find(LDAPSearch):
@register()
class server_show(LDAPRetrieve):
__doc__ = _('Show IPA server.')
+
+
+@register()
+class server_del(LDAPDelete):
+__doc__ = _('Delete IPA server.')
+NO_CLI = True
+msg_summary = _('Deleted IPA server %(value)s')
--
2.1.0
From 402bd94281fd3d654d019f18536e09cdbb8e9781 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 12 Jun 2015 15:56:30 +0200
Subject: [PATCH] ipa-replica-manage: adjust del to work with managed topology
Introduces new method for deletion of replica. This method is used if
managed topology is enabled.
part of https://fedorahosted.org/freeipa/ticket/4302
---
install/tools/ipa-replica-manage | 228 ---
1 file changed, 165 insertions(+), 63 deletions(-)
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index a2b2c820d8e25a2587358e00dc4afc54b309d77b..1c82a6aa6f2c014f2238626435c5002c6b823bab 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -25,6 +25,7 @@ import traceback
from urllib2 import urlparse
import ldap
import socket
+import time
from ipapython import ipautil
from ipaserver.install import replication, dsinstance, installutils
@@ -560,6 +561,13 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
else:
return None
+def check_last_link_managed(api, masters, hostname, force):
+# segments = api.Command.topologysegment_find(u'realm', sizelimit=0).get('result')
+# replica_names = [m.single_value('cn') for m in masters]
+# orphaned = []
+# TODO add proper graph traversing algorithm here
+return None
+
def enforce_host_existence(host, message=None):
if host
Re: [Freeipa-devel] [PATCH] 873-874 ipa-replica-manage: adjust del to work with managed topology
On 06/15/2015 10:57 AM, Petr Vobornik wrote: On 06/12/2015 04:18 PM, Petr Vobornik wrote: Some notes: 1. As mentioned in the WIP patch thread: original 'del' worked also with winsync agreements. I'm not sure why is that. Shouldn't 'disconnect' be used for winsync agreements? At least man page says that. This patch doesn't support it if domain level 0. Is it a blocker? Following should be addressed in beta: 2. If `ipa-replica-manage del` is run before `ipa-csreplica-manage del` then the `ipa-csreplica-manage del` will fail unless run with --force options. 3. Check for orphaned server is missing. I want to use proper graph traversing algorithm for that given that we have the whole topology. 4. Probably a work for topology plugin: I've seen that the removed master doesn't remove its segments and agreements even though that it knows about its removal (doesn't have its own entry in cn=masters). It leads to failed replication connection attempts. Not a big issue, but also not wanted. Martin3 found that there is wrong hostname in one error message. Fixed. Patch 873 rebased. Sorry but NACK. When I try to test the removal of last CA master I get a generic error like this: unexpected error: no such entry Traceback leading to this error is here: http://pastebin.test.redhat.com/290131 This is caused by the following test which assumes that 'master' is a string, but this is in fact the whole result dictionary returned by api.Command.server_find +if master == hostname: +this_services = services_cns the following quick hack fixes this: +if str(master['dn'][0]['cn']) == hostname: +this_services = services_cn but there is certainly a more elegant approach, like transforming the results to a list of master FQDNs directly after calling API command on line 679. -- 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 0329] ipa-replica-manage: Do not allow topology altering commands
On 06/10/2015 07:23 PM, Petr Vobornik wrote: On 06/10/2015 04:39 PM, Petr Vobornik wrote: On 06/10/2015 04:06 PM, Petr Vobornik wrote: On 06/02/2015 02:24 PM, Ludwig Krispenz wrote: hi, is there a real replacement for del, it is not in the scope of the topology commands, the removal of teh agreement is rejected and later done by the plugin, but what about removal of the host, services, cleanruv ? Ludwig On 06/02/2015 02:10 PM, Tomas Babej wrote: Hi, With Domain Level 1 and above, the usage of ipa-replica-manage commands that alter the replica topology is deprecated. Following commands are prohibited: * connect * disconnect * del Upon executing any of these commands, users are pointed out to the ipa topologysegment-* replacements. Part of: https://fedorahosted.org/freeipa/ticket/4302 Tomas is on vacation. I've removed 'del' from his patch and will create a new one for handling of 'del'. If that's OK, we can push this one. NACK 'connect' and 'disconnect' serve also for setting up/removing of winsync replication agreements. This patch forbids it. attaching patch which addresses this issue and replaces Tomas' patch(which was used as a basis). Patch for 'del' will follow. I've not tested if topology plugin ignores winsync agreements. Does it? ACK for the patch. I think that winsync agreements should be ignored because they live in 'cn=replicas,cn=ipa,cn=etc,$SUFFIX', not among cn=masters (but I may be wrong). I have just now setup winsync agreement and it doesn't show up in cn=topology at all. -- 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] 873-874 ipa-replica-manage: adjust del to work with managed topology
On 06/15/2015 02:44 PM, Martin Babinsky wrote: On 06/15/2015 02:15 PM, Petr Vobornik wrote: On 06/15/2015 01:46 PM, Martin Babinsky wrote: On 06/15/2015 10:57 AM, Petr Vobornik wrote: On 06/12/2015 04:18 PM, Petr Vobornik wrote: Some notes: 1. As mentioned in the WIP patch thread: original 'del' worked also with winsync agreements. I'm not sure why is that. Shouldn't 'disconnect' be used for winsync agreements? At least man page says that. This patch doesn't support it if domain level 0. Is it a blocker? Following should be addressed in beta: 2. If `ipa-replica-manage del` is run before `ipa-csreplica-manage del` then the `ipa-csreplica-manage del` will fail unless run with --force options. 3. Check for orphaned server is missing. I want to use proper graph traversing algorithm for that given that we have the whole topology. 4. Probably a work for topology plugin: I've seen that the removed master doesn't remove its segments and agreements even though that it knows about its removal (doesn't have its own entry in cn=masters). It leads to failed replication connection attempts. Not a big issue, but also not wanted. Martin3 found that there is wrong hostname in one error message. Fixed. Patch 873 rebased. Sorry but NACK. When I try to test the removal of last CA master I get a generic error like this: unexpected error: no such entry Traceback leading to this error is here: http://pastebin.test.redhat.com/290131 This is caused by the following test which assumes that 'master' is a string, but this is in fact the whole result dictionary returned by api.Command.server_find +if master == hostname: +this_services = services_cns the following quick hack fixes this: +if str(master['dn'][0]['cn']) == hostname: +this_services = services_cn but there is certainly a more elegant approach, like transforming the results to a list of master FQDNs directly after calling API command on line 679. ah, had this originally when serverservice object was used instead of direct ldap find in the WIP patch. Dict allow us to get dn directly for the service search. CN is also in the dict: master['cn'][0] so not need to get it from dn. Thanks for finding it. Updated patch attached. Everything seems to work as expected. ACK. pushed to master * d58bdf29a514a7868c63b767f4954891b10a574d server: add del command * e9e4509b10e5064556f0aa9a6f0124f38f14b31b ipa-replica-manage: adjust del to work with managed topology -- 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] 873-874 ipa-replica-manage: adjust del to work with managed topology
On 06/15/2015 01:46 PM, Martin Babinsky wrote:
On 06/15/2015 10:57 AM, Petr Vobornik wrote:
On 06/12/2015 04:18 PM, Petr Vobornik wrote:
Some notes:
1. As mentioned in the WIP patch thread: original 'del' worked also with
winsync agreements. I'm not sure why is that. Shouldn't 'disconnect' be
used for winsync agreements? At least man page says that. This patch
doesn't support it if domain level 0. Is it a blocker?
Following should be addressed in beta:
2. If `ipa-replica-manage del` is run before `ipa-csreplica-manage del`
then the `ipa-csreplica-manage del` will fail unless run with --force
options.
3. Check for orphaned server is missing. I want to use proper graph
traversing algorithm for that given that we have the whole topology.
4. Probably a work for topology plugin: I've seen that the removed
master doesn't remove its segments and agreements even though that it
knows about its removal (doesn't have its own entry in cn=masters). It
leads to failed replication connection attempts. Not a big issue, but
also not wanted.
Martin3 found that there is wrong hostname in one error message. Fixed.
Patch 873 rebased.
Sorry but NACK.
When I try to test the removal of last CA master I get a generic error
like this:
unexpected error: no such entry
Traceback leading to this error is here:
http://pastebin.test.redhat.com/290131
This is caused by the following test which assumes that 'master' is a
string, but this is in fact the whole result dictionary returned by
api.Command.server_find
+if master == hostname:
+this_services = services_cns
the following quick hack fixes this:
+if str(master['dn'][0]['cn']) == hostname:
+this_services = services_cn
but there is certainly a more elegant approach, like transforming the
results to a list of master FQDNs directly after calling API command on
line 679.
ah, had this originally when serverservice object was used instead of
direct ldap find in the WIP patch. Dict allow us to get dn directly for
the service search. CN is also in the dict: master['cn'][0] so not need
to get it from dn.
Thanks for finding it.
Updated patch attached.
--
Petr Vobornik
From 40c0886df6a958a757eebd221863911acea8b98f Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 12 Jun 2015 15:56:30 +0200
Subject: [PATCH] ipa-replica-manage: adjust del to work with managed topology
Introduces new method for deletion of replica. This method is used if
managed topology is enabled.
part of https://fedorahosted.org/freeipa/ticket/4302
---
install/tools/ipa-replica-manage | 229 ---
1 file changed, 166 insertions(+), 63 deletions(-)
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index a2b2c820d8e25a2587358e00dc4afc54b309d77b..1b93166bce5c1d1fa6cba41cd87bc0833b2efe57 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -25,6 +25,7 @@ import traceback
from urllib2 import urlparse
import ldap
import socket
+import time
from ipapython import ipautil
from ipaserver.install import replication, dsinstance, installutils
@@ -560,6 +561,13 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
else:
return None
+def check_last_link_managed(api, masters, hostname, force):
+# segments = api.Command.topologysegment_find(u'realm', sizelimit=0).get('result')
+# replica_names = [m.single_value('cn') for m in masters]
+# orphaned = []
+# TODO add proper graph traversing algorithm here
+return None
+
def enforce_host_existence(host, message=None):
if host is not None and not ipautil.host_exists(host):
if message is None:
@@ -567,8 +575,161 @@ def enforce_host_existence(host, message=None):
sys.exit(message)
+def ensure_last_services(conn, hostname, masters, options):
+
+1. When deleting master, check if there will be at least one remaining
+ DNS and CA server.
+2. Pick CA renewal master
+
+Return this_services, other_services, ca_hostname
+
+
+this_services = []
+other_services = []
+ca_hostname = None
+
+for master in masters:
+master_cn = master['cn'][0]
+try:
+services = conn.get_entries(master['dn'], conn.SCOPE_ONELEVEL)
+except errors.NotFound:
+continue
+services_cns = [s.single_value['cn'] for s in services]
+if master_cn == hostname:
+this_services = services_cns
+else:
+other_services.append(services_cns)
+if ca_hostname is None and 'CA' in services_cns:
+ca_hostname = master_cn
+
+if 'CA' in this_services and not any(['CA' in o for o in other_services]):
+print Deleting this server is not allowed as it would leave your installation without a CA.
+sys.exit(1)
+
+other_dns = True
+if 'DNS' in this_services and not any(['DNS' in o for o in
Re: [Freeipa-devel] [PATCH] 873-874 ipa-replica-manage: adjust del to work with managed topology
On 06/15/2015 02:15 PM, Petr Vobornik wrote: On 06/15/2015 01:46 PM, Martin Babinsky wrote: On 06/15/2015 10:57 AM, Petr Vobornik wrote: On 06/12/2015 04:18 PM, Petr Vobornik wrote: Some notes: 1. As mentioned in the WIP patch thread: original 'del' worked also with winsync agreements. I'm not sure why is that. Shouldn't 'disconnect' be used for winsync agreements? At least man page says that. This patch doesn't support it if domain level 0. Is it a blocker? Following should be addressed in beta: 2. If `ipa-replica-manage del` is run before `ipa-csreplica-manage del` then the `ipa-csreplica-manage del` will fail unless run with --force options. 3. Check for orphaned server is missing. I want to use proper graph traversing algorithm for that given that we have the whole topology. 4. Probably a work for topology plugin: I've seen that the removed master doesn't remove its segments and agreements even though that it knows about its removal (doesn't have its own entry in cn=masters). It leads to failed replication connection attempts. Not a big issue, but also not wanted. Martin3 found that there is wrong hostname in one error message. Fixed. Patch 873 rebased. Sorry but NACK. When I try to test the removal of last CA master I get a generic error like this: unexpected error: no such entry Traceback leading to this error is here: http://pastebin.test.redhat.com/290131 This is caused by the following test which assumes that 'master' is a string, but this is in fact the whole result dictionary returned by api.Command.server_find +if master == hostname: +this_services = services_cns the following quick hack fixes this: +if str(master['dn'][0]['cn']) == hostname: +this_services = services_cn but there is certainly a more elegant approach, like transforming the results to a list of master FQDNs directly after calling API command on line 679. ah, had this originally when serverservice object was used instead of direct ldap find in the WIP patch. Dict allow us to get dn directly for the service search. CN is also in the dict: master['cn'][0] so not need to get it from dn. Thanks for finding it. Updated patch attached. Everything seems to work as expected. 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 0329] ipa-replica-manage: Do not allow topology altering commands
On 06/15/2015 02:59 PM, Martin Babinsky wrote: On 06/10/2015 07:23 PM, Petr Vobornik wrote: On 06/10/2015 04:39 PM, Petr Vobornik wrote: On 06/10/2015 04:06 PM, Petr Vobornik wrote: On 06/02/2015 02:24 PM, Ludwig Krispenz wrote: hi, is there a real replacement for del, it is not in the scope of the topology commands, the removal of teh agreement is rejected and later done by the plugin, but what about removal of the host, services, cleanruv ? Ludwig On 06/02/2015 02:10 PM, Tomas Babej wrote: Hi, With Domain Level 1 and above, the usage of ipa-replica-manage commands that alter the replica topology is deprecated. Following commands are prohibited: * connect * disconnect * del Upon executing any of these commands, users are pointed out to the ipa topologysegment-* replacements. Part of: https://fedorahosted.org/freeipa/ticket/4302 Tomas is on vacation. I've removed 'del' from his patch and will create a new one for handling of 'del'. If that's OK, we can push this one. NACK 'connect' and 'disconnect' serve also for setting up/removing of winsync replication agreements. This patch forbids it. attaching patch which addresses this issue and replaces Tomas' patch(which was used as a basis). Patch for 'del' will follow. I've not tested if topology plugin ignores winsync agreements. Does it? ACK for the patch. I think that winsync agreements should be ignored because they live in 'cn=replicas,cn=ipa,cn=etc,$SUFFIX', not among cn=masters (but I may be wrong). I have just now setup winsync agreement and it doesn't show up in cn=topology at all. Pushed to master: 45dccedd12e6d26e146ad9c30c2c304e6b2eded1 -- 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] 0005 User life cycle: del/mod/find/show stageuser commands
On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False Simo. -- Simo Sorce * Red Hat, Inc * New York -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. Simo. -- 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] Use Exception class instead of StandardError
Niranjan wrote: Niranjan wrote: Greetings, Please find the modified patch for ipapython/adminutil.py. I have run few tests manually like running ipa-server-install as non-root user or provide --quiet and --verbose to see if it raises ScriptError properly. Also i checked by running ipa-server-install and using CTRL-C to break and see if the KeyboardInterrupt is properly caught. Please let me know your views on this. Could anyone have a look at the modified patch please. Regards Niranjan From aa74dad193a42b8d7ea1715391c461bcbad888b4 Mon Sep 17 00:00:00 2001 From: Niranjan Mallapadi mrniran...@fedoraproject.org Date: Wed, 10 Jun 2015 04:19:46 +0530 Subject: [PATCH] Use Exception class instead of StandardError In except clause, use of , is not recommended (PEP 3110) Signed-off-by: Niranjan Mallapadi mrniran...@fedoraproject.org --- ipapython/admintool.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipapython/admintool.py b/ipapython/admintool.py index d55bd18499ac427db8adc0c04096bc2aabdc2bbd..5aa1c19bb70f9d9049130d1e2a253abb4b86677b 100644 --- a/ipapython/admintool.py +++ b/ipapython/admintool.py @@ -32,7 +32,7 @@ from ipapython import config from ipapython import ipa_log_manager -class ScriptError(StandardError): +class ScriptError(Exception): An exception that records an error message and a return value def __init__(self, msg='', rval=1): @@ -169,7 +169,7 @@ class AdminTool(object): self.ask_for_options() self.setup_logging() return_value = self.run() -except BaseException, exception: +except BaseException as exception: traceback = sys.exc_info()[2] error_message, return_value = self.handle_error(exception) if return_value: -- 1.9.3 -- 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 pgpXSTMzzCymv.pgp Description: PGP signature -- 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] 877 fix force-sync, re-initialize of replica and a check for replication agreement existence
in other words limit usage of `agreement_dn` method only for manipulation
and search of agreements which are not managed by topology plugin.
For other cases is safer to search for the agreement.
https://fedorahosted.org/freeipa/ticket/5066
--
Petr Vobornik
From 8c711919c5201e73a228ddb3a1d5b45892c4d971 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 15 Jun 2015 19:14:37 +0200
Subject: [PATCH] fix force-sync, re-initialize of replica and a check for
replication agreement existence
in other words limit usage of `agreement_dn` method only for manipulation
and search of agreements which are not managed by topology plugin.
For other cases is safer to search for the agreement.
https://fedorahosted.org/freeipa/ticket/5066
---
ipaserver/install/replication.py | 12 +++-
ipaserver/install/server/replicainstall.py | 8 +---
2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 6a8daadce44c59ea7e3960b5f3387df4d04c85fd..efdb7dfdfed1800c2a7f9720e1ce4f5e9ccf42b7 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -1167,10 +1167,8 @@ class ReplicationManager(object):
conn.modify_s(dn, mod)
def get_agreement_type(self, hostname):
-cn, dn = self.agreement_dn(hostname)
-
-entry = self.conn.get_entry(dn)
+entry = self.get_replication_agreement(hostname)
objectclass = entry.get(objectclass)
for o in objectclass:
@@ -1578,9 +1576,7 @@ class ReplicationManager(object):
Disable the replication agreement to hostname.
-cn, dn = self.agreement_dn(hostname)
-
-entry = self.conn.get_entry(dn)
+entry = self.get_replication_agreement(hostname)
entry['nsds5ReplicaEnabled'] = 'off'
try:
@@ -1594,9 +1590,7 @@ class ReplicationManager(object):
Note: for replication to work it needs to be enabled both ways.
-cn, dn = self.agreement_dn(hostname)
-
-entry = self.conn.get_entry(dn)
+entry = self.get_replication_agreement(hostname)
entry['nsds5ReplicaEnabled'] = 'on'
try:
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 34580ce198b40f922ea984c1eea2dcd0c3aebb08..ae1d325c20a3cf3ff9d27468d4a9f3c021df17bc 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -407,13 +407,7 @@ def install_check(installer):
config.dirman_password)
# Check that we don't already have a replication agreement
-try:
-(agreement_cn, agreement_dn) = replman.agreement_dn(
-config.host_name)
-entry = conn.get_entry(agreement_dn, ['*'])
-except errors.NotFound:
-pass
-else:
+if replman.get_replication_agreement(config.host_name):
root_logger.info('Error: A replication agreement for this '
'host already exists.')
print('A replication agreement for this host already exists. '
--
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 0039] ipa-kdb: common function to get key encodings/salt types
On 05/28/2015 02:55 PM, Simo Sorce wrote: On Thu, 2015-05-28 at 14:43 +0200, Martin Babinsky wrote: A small improvement upon simo's fix for https://fedorahosted.org/freeipa/ticket/4914 -- Martin^3 Babinsky LGTM. Simo. Anyone else to review this patch? It also incidentally fixes a recently reported resource leak. -- 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
[Freeipa-devel] upstream build failure
Hi guys, The attempt to build the latest upstream branch fails with the following error: aci: (targetattr = krblastpwdchange || krbpasswordexpiration || krbprincipalkey || userpassword)(target = ldap:///uid=*,cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Reset Preserved User password;allow (read,search,write) groupdn = ldap:///cn=System: Reset Preserved User password,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: dc=ipa,dc=example aci: (target_to = ldap:///cn=users,cn=accounts,dc=ipa,dc=example;)(target_from = ldap:///cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=nsContainer))(version 3.0;acl permission:System: Undelete User;allow (moddn) groupdn = ldap:///cn=System: Undelete User,cn=permissions,cn=pbac,dc=ipa,dc=example;) -dn: cn=users,cn=accounts,dc=ipa,dc=example -aci: (targetattr = uid)(target = ldap:///uid=*,cn=users,cn=accounts,dc=ipa,dc=example;)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Write Active Users RDN by administrators;allow (write) groupdn = ldap:///cn=System: Write Active Users RDN by administrators,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=sudocmds,cn=sudo,dc=ipa,dc=example aci: (targetfilter = (objectclass=ipasudocmd))(version 3.0;acl permission:System: Add Sudo Command;allow (add) groupdn = ldap:///cn=System: Add Sudo Command,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=sudocmds,cn=sudo,dc=ipa,dc=example Managed permission ACI validation failed. Re-check permission changes and run `makeaci`. ACI.txt validation failed Makefile:130: recipe for target 'version-update' failed make: *** [version-update] Error 1 -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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] 876 regenerate ACI.txt after stage user permission rename
./makeaci was not run.. Pushed to master: 4137f2a8ed6bf1457c7dadf0ed4e6a4465abc621 under one-liner/simple rule -- Petr Vobornik From a92a51f94f5ef9cb6bc6b58d358eeec2f701c2f6 Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Mon, 15 Jun 2015 10:18:52 +0200 Subject: [PATCH] regenerate ACI.txt after stage user permission rename ./makeaci was not run --- ACI.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ACI.txt b/ACI.txt index 08fc05ebc202a64b0e1584303c8dda5b5a1aa074..6fd6353d19f028f1329255c623f7dec0add55490 100644 --- a/ACI.txt +++ b/ACI.txt @@ -252,6 +252,8 @@ dn: cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example aci: (targetattr = *)(target = ldap:///uid=*,cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Modify Preserved Users;allow (write) groupdn = ldap:///cn=System: Modify Preserved Users,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example aci: (targetattr = *)(target = ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=*))(version 3.0;acl permission:System: Modify Stage User;allow (write) groupdn = ldap:///cn=System: Modify Stage User,cn=permissions,cn=pbac,dc=ipa,dc=example;) +dn: cn=users,cn=accounts,dc=ipa,dc=example +aci: (targetattr = uid)(target = ldap:///uid=*,cn=users,cn=accounts,dc=ipa,dc=example;)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Modify User RDN;allow (write) groupdn = ldap:///cn=System: Modify User RDN,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: dc=ipa,dc=example aci: (target_to = ldap:///cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(target_from = ldap:///cn=users,cn=accounts,dc=ipa,dc=example;)(targetfilter = (objectclass=nsContainer))(version 3.0;acl permission:System: Preserve User;allow (moddn) groupdn = ldap:///cn=System: Preserve User,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example @@ -266,8 +268,6 @@ dn: cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example aci: (targetattr = krblastpwdchange || krbpasswordexpiration || krbprincipalkey || userpassword)(target = ldap:///uid=*,cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Reset Preserved User password;allow (read,search,write) groupdn = ldap:///cn=System: Reset Preserved User password,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: dc=ipa,dc=example aci: (target_to = ldap:///cn=users,cn=accounts,dc=ipa,dc=example;)(target_from = ldap:///cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=nsContainer))(version 3.0;acl permission:System: Undelete User;allow (moddn) groupdn = ldap:///cn=System: Undelete User,cn=permissions,cn=pbac,dc=ipa,dc=example;) -dn: cn=users,cn=accounts,dc=ipa,dc=example -aci: (targetattr = uid)(target = ldap:///uid=*,cn=users,cn=accounts,dc=ipa,dc=example;)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Write Active Users RDN by administrators;allow (write) groupdn = ldap:///cn=System: Write Active Users RDN by administrators,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=sudocmds,cn=sudo,dc=ipa,dc=example aci: (targetfilter = (objectclass=ipasudocmd))(version 3.0;acl permission:System: Add Sudo Command;allow (add) groupdn = ldap:///cn=System: Add Sudo Command,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=sudocmds,cn=sudo,dc=ipa,dc=example -- 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] upstream build failure
On 06/15/2015 10:16 AM, Oleg Fayans wrote: Hi guys, The attempt to build the latest upstream branch fails with the following error: aci: (targetattr = krblastpwdchange || krbpasswordexpiration || krbprincipalkey || userpassword)(target = ldap:///uid=*,cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Reset Preserved User password;allow (read,search,write) groupdn = ldap:///cn=System: Reset Preserved User password,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: dc=ipa,dc=example aci: (target_to = ldap:///cn=users,cn=accounts,dc=ipa,dc=example;)(target_from = ldap:///cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=nsContainer))(version 3.0;acl permission:System: Undelete User;allow (moddn) groupdn = ldap:///cn=System: Undelete User,cn=permissions,cn=pbac,dc=ipa,dc=example;) -dn: cn=users,cn=accounts,dc=ipa,dc=example -aci: (targetattr = uid)(target = ldap:///uid=*,cn=users,cn=accounts,dc=ipa,dc=example;)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Write Active Users RDN by administrators;allow (write) groupdn = ldap:///cn=System: Write Active Users RDN by administrators,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=sudocmds,cn=sudo,dc=ipa,dc=example aci: (targetfilter = (objectclass=ipasudocmd))(version 3.0;acl permission:System: Add Sudo Command;allow (add) groupdn = ldap:///cn=System: Add Sudo Command,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=sudocmds,cn=sudo,dc=ipa,dc=example Managed permission ACI validation failed. Re-check permission changes and run `makeaci`. ACI.txt validation failed Makefile:130: recipe for target 'version-update' failed make: *** [version-update] Error 1 fixed by [PATCH] 876 regenerate ACI.txt after stage user permission rename -- 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] Password vault
On 6/15/2015 2:22 AM, Jan Cholasta wrote:
I think it would be better to use a new attribute type which inherits
from ipaPublicKey (ipaVaultPublicKey?) rather than ipaPublicKey directly
for assymetric vault public keys, so that assymetric public key and
escrow public key are on the same level and you can still use
ipaPublicKey to refer to either one:
ipaPublicKey
ipaVaultPublicKey
ipaEscrowPublicKey
( 2.16.840.1.113730.3.8.18.2.? NAME 'ipaVaultPublicKey' DESC
'Assymetric vault public key as DER-encoded SubjectPublicKeyInfo (RFC
5280)' SUP ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
( 2.16.840.1.113730.3.8.18.2.3 NAME 'ipaEscrowPublicKey' DESC 'IPA
escrow public key as DER-encoded SubjectPublicKeyInfo (RFC 5280)' SUP
ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
OK. To be consistent the parameters need to be renamed too:
--vault-public-key and --vault-public-key-file.
1. The vault_add was split into a client-side vault_add and server-side
vault_add_internal since the parameters are different (i.e. public
key file and
future escrow-related params). Since vault_add inherits from Local all
non-primary-key attributes have to be added explicitly.
The split is not really necessary, since the only difference is the
public_key_file option, which exists only because of the lack of proper
file support in the framework. This is a different situation from
vault_{archive,retrieve}, which has two different sets of options on
client and server side. Escrow adds only ipaescrowpublickey and
escrow_public_key_file, right? If yes, we can safely keep the command in
a single piece.
We know the vault-add will have at least two client-only parameters:
vault_public_key_file and escrow_public_key_file. Keeping these
parameters on the server API would be wrong and confusing. If the API is
called on the server side with vault_public_key_file the operation will
fail. In the previous discussion you considered this as broken API:
Server API is used not only by the server itself, but also by installers
for example. Anyway the point is that there *can't* be a broken API like
this, you should at least raise an error if the command is called from
server API, although actually separating it into client and server parts
would be preferable.
Also, originally the vault was designed like this: when you create a
symmetric vault you're supposed to specify the password as well, similar
to adding a public key when creating an asymmetric vault. When you
archive, you're supposed to enter the same password for verification,
not a new password. So it would look like this:
$ ipa vault-add test --type symmetric
New password:
Verify password:
$ ipa vault-archive test --in secret1.txt
Password: (same password)
$ ipa vault-archive test --in secret2.txt
Password: (same password)
In the original design the vault-add would also archive a blank data,
which later could be used to verify the password during vault-archive by
decrypting the existing data first. There's also a plan to add a
mechanism to change the password after the ACL patch.
In the current design the vault-add doesn't archive anything, so during
vault-archive it cannot verify the password because there is nothing to
decrypt. In other words you can specify different passwords on each
archival, regardless of previous archivals:
$ ipa vault-add test --type symmetric
$ ipa vault-archive test --in secret1.txt
New password:
Verify password:
$ ipa vault-archive test --in secret2.txt
New password:
Verify password:
So basically here are the options:
1. Specify the crypto parameters once during vault creation, then
reuse/verify the parameters on each archival retrieval. You can change
the parameters only with a special command.
2. Don't specify the crypto parameters during vault creation, but
specify new parameters on each archival. For retrieval you'd have to
use/verify the parameters specified in the last archival.
I think the first one makes more sense and is easier to use. That also
means the vault-add will have additional client-only parameters such as
--password and --password-file.
2. Since the vault_archive_internal inherits from Update, it accepts
all non
primary-key attributes automatically. This is incorrect since we
don't want to
update these parameters during archival. Can this behavior be
overridden?
Inherit from PKQuery instead (don't forget to add has_output =
output.standard_entry).
Previously you didn't want to use LDAPQuery because of semantics
reasons. Is PKQuery fine semantically? Why not use LDAPQuery since vault
is an LDAPObject? And to be consistent should vault_retrieve_internal
inherit from the same class?
BTW the correct solution would be to have a separate object and commands
for vault data (e.g.
Re: [Freeipa-devel] [PATCH] Password vault
Dne 10.6.2015 v 08:13 Martin Kosek napsal(a):
On 06/09/2015 11:13 PM, Endi Sukma Dewata wrote:
Please take a look at the attached patch to add symmetric asymmetric vaults.
Some comments about the patch:
I think it would be better to use a new attribute type which inherits
from ipaPublicKey (ipaVaultPublicKey?) rather than ipaPublicKey directly
for assymetric vault public keys, so that assymetric public key and
escrow public key are on the same level and you can still use
ipaPublicKey to refer to either one:
ipaPublicKey
ipaVaultPublicKey
ipaEscrowPublicKey
( 2.16.840.1.113730.3.8.18.2.? NAME 'ipaVaultPublicKey' DESC
'Assymetric vault public key as DER-encoded SubjectPublicKeyInfo (RFC
5280)' SUP ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
( 2.16.840.1.113730.3.8.18.2.3 NAME 'ipaEscrowPublicKey' DESC 'IPA
escrow public key as DER-encoded SubjectPublicKeyInfo (RFC 5280)' SUP
ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
1. The vault_add was split into a client-side vault_add and server-side
vault_add_internal since the parameters are different (i.e. public key file and
future escrow-related params). Since vault_add inherits from Local all
non-primary-key attributes have to be added explicitly.
The split is not really necessary, since the only difference is the
public_key_file option, which exists only because of the lack of proper
file support in the framework. This is a different situation from
vault_{archive,retrieve}, which has two different sets of options on
client and server side. Escrow adds only ipaescrowpublickey and
escrow_public_key_file, right? If yes, we can safely keep the command in
a single piece.
2. Since the vault_archive_internal inherits from Update, it accepts all non
primary-key attributes automatically. This is incorrect since we don't want to
update these parameters during archival. Can this behavior be overridden?
Inherit from PKQuery instead (don't forget to add has_output =
output.standard_entry).
BTW the correct solution would be to have a separate object and commands
for vault data (e.g. vaultdata object, vault_archive - vaultdata_mod,
vault_retrieve - vauldata_show), then we wouldn't have to deal with
mixing vault attributes with vault data and could use proper crud base
classes.
Just for the record, this changes API, right? It would be better to have this
in Alpha planned for this week. Not a blocker for Alpha though, we can give
warning that the internal API may change before GA.
--
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] 869 topology: restrict direction changes
On 06/12/2015 06:34 PM, Martin Babinsky wrote: On 06/11/2015 01:41 PM, Petr Vobornik wrote: On 06/11/2015 01:11 PM, Ludwig Krispenz wrote: On 06/11/2015 12:53 PM, Petr Vobornik wrote: On 06/11/2015 12:35 PM, Ludwig Krispenz wrote: On 06/11/2015 12:19 PM, Petr Vobornik wrote: On 06/11/2015 10:22 AM, Martin Babinsky wrote: On 06/10/2015 03:13 PM, Petr Vobornik wrote: topology plugin doesn't properly handle: - creation of segment with direction 'none' and then upgrade to other direction - downgrade of direction These situations are now forbidden in API. part of: https://fedorahosted.org/freeipa/ticket/4302 ACK Looking at Ludwig's path 12, the patch completely forbids mod of ipaReplTopoSegmentDirection? that's what I thought we agreed on, I thought, that we will only complain loudly on downgrade of connection. so you would have to add a segment in the opposite direction an they would be merged to both, but maybe this is a bit strict. This could work as well, but: I just tried (without patch 12) to create: 1. A to B, left-right: success 2. B to A, right-left: Server is unwilling to perform: Segment already exists in topology or is self referential. Add rejected. yes, B to A, right-left is the same as A-B, left right Sorry, you are right, I wrote it badly. I'm not sure if the servers are broken from testing and previous bugs. Maybe I should reinstalled, but I'm experiencing following weird behavior: A-B segment, doesn't exist. 1. A to B, left-right: success 2. A to B, right-left: Server is unwilling to perform: Segment already exists in topology or is self referential. Add rejected. If I try different direction (started with 4 segments): 1. A to B, right-left: success, 5 segments exist 2. A to B, left-right: success, 4 segments exist - the new ones are gone Martin, can you reproduce it? I.e., the upgrade didn't happen. I could allow for ipaReplTopoSegmentDirection replace: both So that upgrade from right-left and left-right to both is not allowed? If so then this patch needs to be updated. depends a bit on what you prefer and what we can get in for alpha. Depends what's better, I already have adjusted patch for ^^ so it's not about the work. so lets take the changes to your patch and we could still extend functionality a bit for beta or later OK, attaching rebased patch. ACK Pushed to master: 6b153ba876edf1ed9249ed29420a4af2b2e1830d -- 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] 875 topology: fix swapped topologysegment-reinitialize behavior
On 06/12/2015 06:31 PM, Martin Babinsky wrote: On 06/12/2015 04:19 PM, Petr Vobornik wrote: setting nsds5BeginReplicaRefresh;left to start reinintializes the right node and not the left node. This patch fixes API to match the behavior. part of: https://fedorahosted.org/freeipa/ticket/4302 ACK Pushed to master: bb6c0b9c634f26ae5d16079b3a66841ac0ce60cc -- 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 0244] DNSSEC: fix traceback in ipa-dnskeysyncd during shutdown phase
On 06/11/2015 05:03 PM, Petr Spacek wrote: On 12.5.2015 14:51, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4657 Patch attached. ACK Pushed to: master: f763b137ee1eee228f53b456b8245b1499185ef7 ipa-4-1: a5d8d79f76ce39817e16a64fe937c9bb34aa5d6a -- 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 0052] Stage User: Fix permissions naming and split them where, apropriate.
On 06/11/2015 07:49 PM, thierry bordaz wrote:
On 06/11/2015 04:34 PM, David Kupka wrote:
Dne 11.6.2015 v 16:17 Martin Kosek napsal(a):
On 06/11/2015 03:55 PM, David Kupka wrote:
Dne 11.6.2015 v 14:12 thierry bordaz napsal(a):
On 06/10/2015 02:14 PM, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5057
Hello David,
The patch looks ok except it removes a permission to update 'uid' from
an active user. This permission is required to delete(preserve) an
active user.
-# Active container
-#
-# Stage user administrators need write right on RDN when
-# the active user is deleted (preserved)
-'System: Write Active Users RDN by administrators': {
-'ipapermlocation': DN(baseuser.active_container_dn,
api.env.basedn),
-'ipapermbindruletype': 'permission',
-'ipapermtarget': DN('uid=*',
baseuser.active_container_dn, api.env.basedn),
-'ipapermtargetfilter':
{'(objectclass=posixaccount)'},
-'ipapermright': {'write'},
-'ipapermdefaultattr': {'uid'},
-'default_privileges': {'Stage User Administrators'},
-},
-#
I prepared a new patch (attached) with that permission and it makes
'user-del --preserve' happy.
Now I think the name would rather be something like: 'System: Preserve
an active user (user-del --preserve)'
I also added back this comment in two permissions 'Note:
targetfilter is
the target parent container'.
This was to say that the targetfilter setting was intentional.
If you think it is not the right place, you may remove those comments.
Thanks
thierry
Hello Thierry,
Indeed, I accidentally removed these. Thank you for careful review.
Rebase is needed but it is due to change in VERSION and is useless
to do it
before push as there are too much patches going to master right now.
Martin, are you (as a reporter) OK with the patch?
Not entirely. I still see some weird permission in stageuser.py:
#
# Active container
#
# Stage user administrators need write right on RDN when
# the active user is deleted (preserved)
'System: Write Active Users RDN by administrators': {
'ipapermlocation': DN(baseuser.active_container_dn,
api.env.basedn),
'ipapermbindruletype': 'permission',
'ipapermtarget': DN('uid=*', baseuser.active_container_dn,
api.env.basedn),
'ipapermtargetfilter': {'(objectclass=posixaccount)'},
'ipapermright': {'write'},
'ipapermdefaultattr': {'uid'},
'default_privileges': {'Stage User Administrators'},
},
This was supposed to be System: Modify User RDN. When the name is
also
fixed, I am fine.
Updated patch attached.
Hi David,
All the tests are ok. The patch is fine for me. ACK
Pushed to master: 44cced658bde224957a605bfa083821d8fbf94c0
--
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] Password vault
Dne 15.6.2015 v 09:22 Jan Cholasta napsal(a):
Dne 10.6.2015 v 08:13 Martin Kosek napsal(a):
On 06/09/2015 11:13 PM, Endi Sukma Dewata wrote:
Please take a look at the attached patch to add symmetric
asymmetric vaults.
Some comments about the patch:
I think it would be better to use a new attribute type which inherits
from ipaPublicKey (ipaVaultPublicKey?) rather than ipaPublicKey directly
for assymetric vault public keys, so that assymetric public key and
escrow public key are on the same level and you can still use
ipaPublicKey to refer to either one:
ipaPublicKey
ipaVaultPublicKey
ipaEscrowPublicKey
( 2.16.840.1.113730.3.8.18.2.? NAME 'ipaVaultPublicKey' DESC
'Assymetric vault public key as DER-encoded SubjectPublicKeyInfo (RFC
5280)' SUP ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
( 2.16.840.1.113730.3.8.18.2.3 NAME 'ipaEscrowPublicKey' DESC 'IPA
escrow public key as DER-encoded SubjectPublicKeyInfo (RFC 5280)' SUP
ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
BTW what is the reason for that vault type is idendified by ipaVaultType
attribute rather than object class?
1. The vault_add was split into a client-side vault_add and server-side
vault_add_internal since the parameters are different (i.e. public
key file and
future escrow-related params). Since vault_add inherits from Local all
non-primary-key attributes have to be added explicitly.
The split is not really necessary, since the only difference is the
public_key_file option, which exists only because of the lack of proper
file support in the framework. This is a different situation from
vault_{archive,retrieve}, which has two different sets of options on
client and server side. Escrow adds only ipaescrowpublickey and
escrow_public_key_file, right? If yes, we can safely keep the command in
a single piece.
2. Since the vault_archive_internal inherits from Update, it accepts
all non
primary-key attributes automatically. This is incorrect since we
don't want to
update these parameters during archival. Can this behavior be
overridden?
Inherit from PKQuery instead (don't forget to add has_output =
output.standard_entry).
BTW the correct solution would be to have a separate object and commands
for vault data (e.g. vaultdata object, vault_archive - vaultdata_mod,
vault_retrieve - vauldata_show), then we wouldn't have to deal with
mixing vault attributes with vault data and could use proper crud base
classes.
Just for the record, this changes API, right? It would be better to
have this
in Alpha planned for this week. Not a blocker for Alpha though, we can
give
warning that the internal API may change before GA.
--
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 0264] Server Upgrade: disconnect ldap2 connection before DS restart
On 06/12/2015 03:26 PM, Martin Babinsky wrote: On 06/10/2015 01:47 PM, Martin Basti wrote: Without this patch, upgrade may failed when api.Backend.ldap2 was connected before DS restart. Patch attached. ACK Pushed to master: c1d484afde34cb68cfb0d187004e107342180399 -- 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
