Re: [Freeipa-devel] [PATCH 0550] host-find: do not show SSH keys by default

2016-07-13 Thread Petr Vobornik 
On 07/12/2016 12:33 PM, Stanislav Laznicka wrote:
> On 07/08/2016 01:52 PM, Martin Basti wrote:
>> Reproducible only with 2+ hosts, patch attached.
>>
>> https://fedorahosted.org/freeipa/ticket/6043
>>
>>
> ACK.
> 

master:
* 2874fdbfef1e191cf858dabdd34d5a0cbdc5ef16 host-find: do not show SSH
key by default
-- 
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 0184] vault-add: set the default vault type on the client side if none was given

2016-07-13 Thread Petr Vobornik 
On 07/12/2016 03:53 PM, Stanislav Laznicka wrote:
> On 07/12/2016 02:10 PM, Martin Babinsky wrote:
>> Quick fix for https://fedorahosted.org/freeipa/ticket/6047
>>
>> Note that it depends on mbasti's patch 552 (already acked) otherwise
>> client-side vault commands would not be even visible in CLI.
>>
>>
> ACK.
> 

master:
* a1a7ecdc7bf6686adf8558cedd3964f9e4805469 vault-add: set the default
vault type on the client side if none was given

-- 
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 0185] messages: specify message type for ResultFormattingError

2016-07-13 Thread Martin Babinsky 

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

--
Martin^3 Babinsky
From dd2dfe4bf0a629716145af83c1b7f73595290079 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 13 Jul 2016 18:22:04 +0200
Subject: [PATCH] messages: specify message type for ResultFormattingError

the ResultFormattingError message class was missing a `type` member which
could cause `otptoken-add` command to crash during QR image rendering using
suboptimal TTY settings

https://fedorahosted.org/freeipa/ticket/6081
---
 ipalib/messages.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 7288606f6ac923c2c87fadba5f2a6a2d9dadb7f5..6abad64a8259a8e164db60f63e75bbb9c230e7bf 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -363,6 +363,7 @@ class ResultFormattingError(PublicMessage):
 """
 **13019** Unable to correctly format some part of the result
 """
+type = "warning"
 errno = 13019
 
 
-- 
2.5.5

-- 
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][Tests] RFE: Support UPN for trusted domains

2016-07-13 Thread Martin Babinsky 

On 07/01/2016 04:45 PM, Lenka Doudova wrote:



On 07/01/2016 03:04 PM, Martin Babinsky wrote:

On 07/01/2016 11:13 AM, Lenka Doudova wrote:

And, of course, a patch file :)


On 07/01/2016 11:09 AM, Lenka Doudova wrote:

Hi all,

here's patch with basic test suite for support of UPN.

Note: it needs to be applied on top of my patch 0025.2 (or later, if
there's will be more fixes to that patch).


Lenka







Hi Lenka,

test data such as usernames, etc. should be stored either in separate
resource files or at least as class attributes like this:

diff --git a/ipatests/test_integration/test_trust.py
b/ipatests/test_integration/test_trust.py
index e8fdc6b..86ba7cc 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -394,28 +394,33 @@ class TestTrustWithUPN(ADTrustBase):
 """
 Test support of UPN for trusted domains
 """
+upn_suffix = 'UPNsuffix.com'
+upn_username = 'upnuser'
+upn_princ = '{}@{}'.format(upn_username, upn_suffix)
+upn_password = 'Secret123456'
+
 def test_upn_in_nonposix_trust(self):
 """ Check that UPN is listed as trust attribute """
 result = self.master.run_command(['ipa', 'trust-show',
self.ad_domain,
   '--all', '--raw'])

-assert "ipantadditionalsuffixes: UPNsuffix.com" in
result.stdout_text
+assert ("ipantadditionalsuffixes: {}".format(self.upn_suffix) in
+result.stdout_text)

 def test_upn_user_resolution_in_nonposix_trust(self):
 """ Check that user with UPN can be resolved """
-upnuser = 'upnu...@upnsuffix.com'
-result = self.master.run_command(['getent', 'passwd', upnuser])
+result = self.master.run_command(['getent', 'passwd',
self.upn_princ])

 # result will contain AD domain, not UPN
-upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN
User:/:$".format(
-self.ad_domain)
+upnuser_regex = "^{}@{}:\*:(\d+):(\d+):UPN User:/:$".format(
+self.upn_username, self.ad_domain)
 assert re.search(upnuser_regex, result.stdout_text)

 def test_upn_user_authentication(self):
 """ Check that AD user with UPN can authenticate in IPA """
 self.master.run_command(['systemctl', 'restart', 'krb5kdc'])
-self.master.run_command(['kinit', '-C', '-E',
'upnu...@upnsuffix.com'],
-stdin_text='Secret123456')
+self.master.run_command(['kinit', '-C', '-E', self.upn_princ],
+stdin_text=self.upn_password)

otherwise LGTM.


Thanks for review, fixed patch attached.

Few notes:
1. mbabinsky's suggestion to store testdata as class attributes or
separate resource file: I decided to use the class attribute approach.
The separate resource file is a nice idea, which I have already put on
my "to do" list - there's a lot of hardcoded stuff in the trust tests,
even in the original ones (before my patches), so when there's time I'll
work on a way how to dynamically provide this data as test configuration
2. previous discussion about getent vs. pwd.getpwnam(): I'll leave the
getent command, since according to mbasti the alternative would not work
in CI.

Lenka


Hi Lenka,

I am not sure 'test_all_trustdomains_found' should be run as a part of 
this test suite. Maybe yes, I'm not sure.


Also I would add a 60 second sleep after KDC restart in 
'test_upn_user_authentication' so that MS-PAC cache gets refreshed 
before trying to kinit as enterprise principal.


Two of the tests fail on my setup but that is probably due to 
https://fedorahosted.org/freeipa/ticket/6082 .


--
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] 0089 caacl: expand plugin documentation

2016-07-13 Thread Petr Vobornik 
On 07/12/2016 08:45 AM, Alexander Bokovoy wrote:
> On Tue, 12 Jul 2016, Fraser Tweedale wrote:
>> Attached patch is a doc change, addressing
>> https://fedorahosted.org/freeipa/ticket/6002.
>>
>> Thanks,
>> Fraser
> 
>> From 19c5fc60391d37c9d0500feb5d5d5a6628bc4d27 Mon Sep 17 00:00:00 2001
>> From: Fraser Tweedale 
>> Date: Tue, 12 Jul 2016 15:11:11 +1000
>> Subject: [PATCH] caacl: expand plugin documentation
>>
>> Expand the 'caacl' plugin documentation to explain some common
>> confusions including the fact that CA ACLs apply to the target
>> subject principal (not necessarily the principal requesting the
>> cert), and the fact that CA-less CA ACL implies the 'ipa' CA.
>>
>> Fixes: https://fedorahosted.org/freeipa/ticket/6002
>> ---
>> ipaserver/plugins/caacl.py | 34 --
>> 1 file changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/ipaserver/plugins/caacl.py b/ipaserver/plugins/caacl.py
>> index
>> 9a60f7e27809c4f41b160647efafde94dbe90bf0..d316cc7c48cf2997d6be6b052dc1efa6d6fcdb6a
>> 100644
>> --- a/ipaserver/plugins/caacl.py
>> +++ b/ipaserver/plugins/caacl.py
>> @@ -23,14 +23,36 @@ if six.PY3:
>> __doc__ = _("""
>> Manage CA ACL rules.
>>
>> -This plugin is used to define rules governing which principals are
>> -permitted to have certificates issued using a given certificate
>> -profile.
>> +This plugin is used to define rules governing which CAs and profiles
>> +may be used to issue certificates to particular principals or groups
>> +of principals.
>>
>> -PROFILE ID SYNTAX:
>> +SUBJECT PRINCIPAL SCOPE:
>>
>> -A Profile ID is a string without spaces or punctuation starting with
>> a letter
>> -and followed by a sequence of letters, digits or underscore ("_").
>> +For a certificate request to be allowed, the principal(s) that are
>> +the subject of a certificate request (not necessarily the principal
>> +actually requesting the certificate) must be included in the scope
>> +of a CA ACL that also includes the target CA and profile.
>> +
>> +Users can be included by name, group or the "all users" category.
>> +Hosts can be included by name, hostgroup or the "all hosts"
>> +category.  Services can be included by service name or the "all
>> +services" category.  CA ACLs may be associated with a single type of
>> +principal, or multiple types.
>> +
>> +CERTIFICATE AUTHORITY SCOPE:
>> +
>> +A CA ACL can be associated with one or more CAs by name, or by the
>> +"all CAs" category.  For compatibility reasons, a CA ACL with no CA
>> +association implies an association with the 'ipa' CA (and only this
>> +CA).
>> +
>> +PROFILE SCOPE:
>> +
>> +A CA ACL can be associated with one or more profiles by Profile ID.
>> +The Profile ID is a string without spaces or punctuation starting
>> +with a letter and followed by a sequence of letters, digits or
>> +underscore ("_").
>>
>> EXAMPLES:
>>
> ACK. Reads well.
> 

Pushed to master: 8cd87d12d53a98a8e386c06a7c5fddb1d38d990d

-- 
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 0056] removed unused parameter from migrate-ds

2016-07-13 Thread Petr Vobornik 
On 07/12/2016 12:35 PM, Martin Babinsky wrote:
> On 07/11/2016 12:40 PM, Stanislav Laznicka wrote:
>> https://fedorahosted.org/freeipa/ticket/6034
>>
>>
>>
> ACK
> 

master:
* 6c74bd2bcca46b586b07c3acd9670dae6e1f07b9 Removed unused method
parameter from migrate-ds


-- 
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 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Alexander Bokovoy 

On Wed, 13 Jul 2016, Martin Babinsky wrote:
In that case, if nobody objects then the second revision of the patch 
may be pushed since Alexander already acked it, right Alexander?

Correct. ACK.
--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0023 Bug in the ipapwd plugin

2016-07-13 Thread Lukas Slebodnik 
On (13/07/16 16:50), thierry bordaz wrote:
>https://fedorahosted.org/freeipa/ticket/6030

>>From 4efedc5e674db92f9f7c160429df543422ed8afb Mon Sep 17 00:00:00 2001
>From: Thierry Bordaz 
>Date: Wed, 13 Jul 2016 15:34:20 +0200
>Subject: [PATCH] Ticket 6030 Bug in the ipapwd plugin
>
>ipapwd_encrypt_encode_key allocates 'kset' on the heap but
>with num_keys and keys not being initialized.
>Then ipa_krb5_generate_key_data initializes them with the
>generated keys.
>If ipa_krb5_generate_key_data fails (here EINVAL meaning no
>principal->realm.data), num_keys and keys are left uninitialized.
>Upon failure, ipapwd_keyset_free is called to free 'kset'
>that contains random num_keys and keys.
>
>allocates kset with calloc so that kset->num_keys==0 and
>kset->keys==NULL
>
>https://fedorahosted.org/freeipa/ticket/6030
>---
> daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c 
>b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
>index 5ca155d..46bf79a 100644
>--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
>+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
>@@ -148,7 +148,7 @@ Slapi_Value **ipapwd_encrypt_encode_key(struct 
>ipapwd_krbcfg *krbcfg,
> pwd.length = strlen(data->password);
> }
> 
>-kset = malloc(sizeof(struct ipapwd_keyset));
>+kset = calloc(sizeof(struct ipapwd_keyset));
I though that calloc need two arguments

man malloc says:
   void *malloc(size_t size);
   void *calloc(size_t nmemb, size_t size);

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0057] Don't show part of warning containing --force-ntpd in replica install

2016-07-13 Thread Petr Vobornik 
On 07/13/2016 08:26 AM, Stanislav Laznicka wrote:
> On 07/12/2016 08:44 AM, Stanislav Laznicka wrote:
>> On 07/11/2016 04:27 PM, Petr Vobornik wrote:
>>> On 07/11/2016 01:23 PM, Stanislav Laznicka wrote:
 https://fedorahosted.org/freeipa/ticket/6046



>>> Isn't the bug about something else?
>>>
>>> The issue was that ipa-replica-install doesn't have --force-ntpd option.
>>> It is an option of ipa-client-install which is run from replica
>>> installer.
>>>
>>> The unattended mode is unrelated.
>>
>> My understanding is that the bug says that '--force-ntpd' option
>> should not be shown when ipa-client-install is run during replica
>> installation.
>>
>> During replica installation, the ipa-client-install script is run with
>> the '--unattended' flag in the 'ensure_enrolled()' function. Being a
>> separate script, there's not many options on how to pass the
>> information not to show the message to ipa-client-install. Using the
>> already used flag to get rid of the message seemed easiest to me.
>> Introducing a new 'hidden' flag (like '--from-replica'), on the other
>> hand, seems a bit harsh.
>>
> Just to throw it out there - it's possible that the '--force-join'
> client option would also appear as a hint from the client install script
> (during replica installation). Should this also be muted somehow? To me,
> it seems reasonable to rather add it as an argument to
> ipa-replica-install to pass it to the client install script.
> 

IMO client installation initiated from replica needs to have a special
option(hidden in help) similar to --on-server (or what's its name). E.g.
the name can be --replica-install. Maybe --on-server can be used but it
may have other implication which might not be valid for this use case.

Anything else are just workarounds. Imagine that admin runs
ipa-client-install with --unattended or --force-join. He would then not
get the message as now.
-- 
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 0057] Don't show part of warning containing --force-ntpd in replica install

2016-07-13 Thread Stanislav Laznicka 

On 07/12/2016 08:44 AM, Stanislav Laznicka wrote:

On 07/11/2016 04:27 PM, Petr Vobornik wrote:

On 07/11/2016 01:23 PM, Stanislav Laznicka wrote:

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




Isn't the bug about something else?

The issue was that ipa-replica-install doesn't have --force-ntpd option.
It is an option of ipa-client-install which is run from replica 
installer.


The unattended mode is unrelated.


My understanding is that the bug says that '--force-ntpd' option 
should not be shown when ipa-client-install is run during replica 
installation.


During replica installation, the ipa-client-install script is run with 
the '--unattended' flag in the 'ensure_enrolled()' function. Being a 
separate script, there's not many options on how to pass the 
information not to show the message to ipa-client-install. Using the 
already used flag to get rid of the message seemed easiest to me. 
Introducing a new 'hidden' flag (like '--from-replica'), on the other 
hand, seems a bit harsh.


Just to throw it out there - it's possible that the '--force-join' 
client option would also appear as a hint from the client install script 
(during replica installation). Should this also be muted somehow? To me, 
it seems reasonable to rather add it as an argument to 
ipa-replica-install to pass it to the client install script.


--
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 0003] Fix several small typos

2016-07-13 Thread Ben Lipton 

Nothing too exciting, just fixes a few typos I've noticed in comments.

Thanks,
Ben
From 26d9ba08e06a145fa9d67a039d23c3fdb272b23e Mon Sep 17 00:00:00 2001
From: Ben Lipton 
Date: Fri, 8 Jul 2016 11:41:43 -0400
Subject: [PATCH] Fix several small typos

---
 ipatests/test_xmlrpc/tracker/base.py| 2 +-
 ipatests/test_xmlrpc/tracker/user_plugin.py | 2 +-
 ipatests/test_xmlrpc/xmlrpc_test.py | 5 +++--
 ipatests/util.py| 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_xmlrpc/tracker/base.py b/ipatests/test_xmlrpc/tracker/base.py
index 6a0af510f52aa1d7ccd94450c0848149d9abab48..f7fc55d54b6bcd31308d3eaaea9dffb0228e38bf 100644
--- a/ipatests/test_xmlrpc/tracker/base.py
+++ b/ipatests/test_xmlrpc/tracker/base.py
@@ -289,5 +289,5 @@ class Tracker(object):
  set(expected_updates.keys()))
 
 def check_update(self, result, extra_keys=()):
-"""Check the plugin's `find` command result"""
+"""Check the plugin's `mod` command result"""
 raise NotImplementedError(self._override_me_msg)
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index 1a85e93327e5d517249fd67e208e83a922509002..10093a9530dbdd57b53e412ad6c4d4f2e6174fd5 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -184,7 +184,7 @@ class UserTracker(Tracker):
 Overriding Tracker method for setting self.attrs correctly;
  * most attributes stores its value in list
  * the rest can be overridden by expected_updates
- * allow deleting parametrs if update value is None
+ * allow deleting parameters if update value is None
 """
 if expected_updates is None:
 expected_updates = {}
diff --git a/ipatests/test_xmlrpc/xmlrpc_test.py b/ipatests/test_xmlrpc/xmlrpc_test.py
index ddfe9c17c5ca110bc5eb66dead618383b861eda9..78d96388d36f586a3f95c7dffd7a072da43b1f81 100644
--- a/ipatests/test_xmlrpc/xmlrpc_test.py
+++ b/ipatests/test_xmlrpc/xmlrpc_test.py
@@ -97,8 +97,9 @@ fuzzy_issuer = Fuzzy(type=six.string_types)
 
 fuzzy_hex = Fuzzy('^0x[0-9a-fA-F]+$', type=six.string_types)
 
-# Matches password - password consists of all printable characters without whitespaces
-# The only exception is space, but space cannot be at the beggingin or end of the pwd
+# Matches password - password consists of all printable characters without
+# whitespaces. The only exception is space, but space cannot be at the
+# beginning or end of the pwd.
 fuzzy_password = Fuzzy('^\S([\S ]*\S)*$')
 
 # Matches generalized time value. Time format is: %Y%m%d%H%M%SZ
diff --git a/ipatests/util.py b/ipatests/util.py
index 118c47a12e0d97907cb559d716989a9ca6c5f304..796f87cbbfb25028a5135394a0a97a6b1f7180b5 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -205,7 +205,7 @@ class Fuzzy(object):
 >>> fuzzy.test  # doctest:+ELLIPSIS
  at 0x...>
 
-To aid debugging, `Fuzzy.__repr__()` revealse these kwargs as well:
+To aid debugging, `Fuzzy.__repr__()` reveals these kwargs as well:
 
 >>> fuzzy  # doctest:+ELLIPSIS
 Fuzzy('.+', ,  at 0x...>)
-- 
2.5.5

-- 
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 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Martin Babinsky 

On 07/12/2016 04:19 PM, Simo Sorce wrote:

On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:

On 07/12/2016 02:00 PM, Martin Babinsky wrote:


On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:


On Mon, 11 Jul 2016, Martin Babinsky wrote:


From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep 17
00:00:00 2001
From: Martin Babinsky 
Date: Fri, 1 Jul 2016 18:09:04 +0200
Subject: [PATCH] Preserve user principal aliases during rename
operation

When a MODRDN is performed on the user entry, the MODRDN plugin
resets
both
krbPrincipalName and krbCanonicalName to the value constructed
from
uid. In
doing so, hovewer, any principal aliases added to the
krbPrincipalName
are
wiped clean. In this patch old aliases are fetched before the
MODRDN
operation
takes place and inserted back after it is performed.

This also preserves previous user logins which can be used
further for
authentication as aliases.

https://fedorahosted.org/freeipa/ticket/6028
---
ipaserver/plugins/baseuser.py | 46
+++
1 file changed, 46 insertions(+)

diff --git a/ipaserver/plugins/baseuser.py
b/ipaserver/plugins/baseuser.py
index
0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a131157815ffb2
452692a7edb342f6ac3

100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
len =
int(config.get('ipamaxusernamelength')[0])
)
)
+
+def preserve_krbprincipalname_pre(self, ldap, entry_attrs,
*keys,
**options):
+"""
+preserve user principal aliases during rename
operation. This
is the
+pre-callback part of this. Another method called
during
post-callback
+shall insert the principals back
+"""
+if options.get('rename', None) is None:
+return
+
+try:
+old_entry = ldap.get_entry(
+entry_attrs.dn, attrs_list=(
+'krbprincipalname', 'krbcanonicalname'))
+
+if 'krbcanonicalname' not in old_entry:
+return
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
+self.context.krbprincipalname = old_entry.get(
+'krbprincipalname', [])
+
+def preserve_krbprincipalname_post(self, ldap,
entry_attrs,
**options):
+"""
+Insert the preserved aliases back to the user entry
during
rename
+operation
+"""
+if options.get('rename', None) is None or not hasattr(
+self.context, 'krbprincipalname'):
+return
+
+obj_pkey =
self.obj.get_primary_key_from_dn(entry_attrs.dn)
+canonical_name = entry_attrs['krbcanonicalname'][0]
+
+principals_to_add = tuple(p for p in
self.context.krbprincipalname if
+  p != canonical_name)
+
+if principals_to_add:
+result = self.api.Command.user_add_principal(
+obj_pkey, principals_to_add)['result']
+
+entry_attrs['krbprincipalname'] =
result.get('krbprincipalname', [])
+
def check_mail(self, entry_attrs):
if 'mail' in entry_attrs:
entry_attrs['mail'] =
self.obj.normalize_and_validate_email(entry_attrs['mail'])
@@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):

self.check_objectclass(ldap, dn, entry_attrs)
self.obj.convert_usercertificate_pre(entry_attrs)
+self.preserve_krbprincipalname_pre(ldap, entry_attrs,
*keys,
**options)

def post_common_callback(self, ldap, dn, entry_attrs,
*keys,
**options):
assert isinstance(dn, DN)
+self.preserve_krbprincipalname_post(ldap, entry_attrs,
**options)
if options.get('random', False):
try:
entry_attrs['randompassword'] =
unicode(getattr(context, 'randompassword'))
--
2.5.5


The approach looks good.

For the record, we also support aliases for hosts and service
kerberos
principals but we don't support rename options for them, so there
is no
need to add similar logic there.



That's right, I have updated the corresponding section of the
design
page [1] for future reference.

[1]
http://www.freeipa.org/page/V4/Kerberos_principal_aliases#Managemen
t_framework



Adding Simo to the loop since he is not convinced that this is the
right
behavior. As I see it, it seems to not be a security issue but more
of a
different expectations about the perceived correct behavior in this
particular situation.

I can see the use case in keeping the old aliases, e.g. keeping the
old
credentials after legal name change, but I can also see the
available
space for user principal names piling up and cluttering quickly in
large
organizations.


after some thinking I think it is ok to keep by default and then drop
as it avoid races if you do really want to keep the aliases.

However the CLI/UI should probably offer a button/switch to allow to
drop all aliases on

Re: [Freeipa-devel] [PATCH 0057] Don't show part of warning containing --force-ntpd in replica install

2016-07-13 Thread Stanislav Laznicka 

On 07/13/2016 09:51 AM, Petr Vobornik wrote:

On 07/13/2016 08:26 AM, Stanislav Laznicka wrote:

On 07/12/2016 08:44 AM, Stanislav Laznicka wrote:

On 07/11/2016 04:27 PM, Petr Vobornik wrote:

On 07/11/2016 01:23 PM, Stanislav Laznicka wrote:

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




Isn't the bug about something else?

The issue was that ipa-replica-install doesn't have --force-ntpd option.
It is an option of ipa-client-install which is run from replica
installer.

The unattended mode is unrelated.

My understanding is that the bug says that '--force-ntpd' option
should not be shown when ipa-client-install is run during replica
installation.

During replica installation, the ipa-client-install script is run with
the '--unattended' flag in the 'ensure_enrolled()' function. Being a
separate script, there's not many options on how to pass the
information not to show the message to ipa-client-install. Using the
already used flag to get rid of the message seemed easiest to me.
Introducing a new 'hidden' flag (like '--from-replica'), on the other
hand, seems a bit harsh.


Just to throw it out there - it's possible that the '--force-join'
client option would also appear as a hint from the client install script
(during replica installation). Should this also be muted somehow? To me,
it seems reasonable to rather add it as an argument to
ipa-replica-install to pass it to the client install script.


IMO client installation initiated from replica needs to have a special
option(hidden in help) similar to --on-server (or what's its name). E.g.
the name can be --replica-install. Maybe --on-server can be used but it
may have other implication which might not be valid for this use case.

Anything else are just workarounds. Imagine that admin runs
ipa-client-install with --unattended or --force-join. He would then not
get the message as now.


The --on-master option won't do here as it seems that the client would 
require some IPA pre-configuration for successful install. A new option 
will have to be created, then.


As I was trying to point out, the situation about --force-join is a bit 
different. The option again would be shown and is not available in 
ipa-replica-install. I think it should be available to allow direct 
replica installation even when previous installation failed/left some 
mess on the master (ofc the user could run `ipa-replica-manage del 
 --cleanup` on the master instead).


--
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 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Petr Vobornik 
On 07/12/2016 04:19 PM, Simo Sorce wrote:
> On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:
>> On 07/12/2016 02:00 PM, Martin Babinsky wrote:
>>>
>>> On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:

 On Mon, 11 Jul 2016, Martin Babinsky wrote:
>
> From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep 17
> 00:00:00 2001
> From: Martin Babinsky 
> Date: Fri, 1 Jul 2016 18:09:04 +0200
> Subject: [PATCH] Preserve user principal aliases during rename
> operation
>
> When a MODRDN is performed on the user entry, the MODRDN plugin
> resets
> both
> krbPrincipalName and krbCanonicalName to the value constructed
> from
> uid. In
> doing so, hovewer, any principal aliases added to the
> krbPrincipalName
> are
> wiped clean. In this patch old aliases are fetched before the
> MODRDN
> operation
> takes place and inserted back after it is performed.
>
> This also preserves previous user logins which can be used
> further for
> authentication as aliases.
>
> https://fedorahosted.org/freeipa/ticket/6028
> ---
> ipaserver/plugins/baseuser.py | 46
> +++
> 1 file changed, 46 insertions(+)
>
> diff --git a/ipaserver/plugins/baseuser.py
> b/ipaserver/plugins/baseuser.py
> index
> 0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a131157815ffb2
> 452692a7edb342f6ac3
>
> 100644
> --- a/ipaserver/plugins/baseuser.py
> +++ b/ipaserver/plugins/baseuser.py
> @@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
> len =
> int(config.get('ipamaxusernamelength')[0])
> )
> )
> +
> +def preserve_krbprincipalname_pre(self, ldap, entry_attrs,
> *keys,
> **options):
> +"""
> +preserve user principal aliases during rename
> operation. This
> is the
> +pre-callback part of this. Another method called
> during
> post-callback
> +shall insert the principals back
> +"""
> +if options.get('rename', None) is None:
> +return
> +
> +try:
> +old_entry = ldap.get_entry(
> +entry_attrs.dn, attrs_list=(
> +'krbprincipalname', 'krbcanonicalname'))
> +
> +if 'krbcanonicalname' not in old_entry:
> +return
> +except errors.NotFound:
> +self.obj.handle_not_found(*keys)
> +
> +self.context.krbprincipalname = old_entry.get(
> +'krbprincipalname', [])
> +
> +def preserve_krbprincipalname_post(self, ldap,
> entry_attrs,
> **options):
> +"""
> +Insert the preserved aliases back to the user entry
> during
> rename
> +operation
> +"""
> +if options.get('rename', None) is None or not hasattr(
> +self.context, 'krbprincipalname'):
> +return
> +
> +obj_pkey =
> self.obj.get_primary_key_from_dn(entry_attrs.dn)
> +canonical_name = entry_attrs['krbcanonicalname'][0]
> +
> +principals_to_add = tuple(p for p in
> self.context.krbprincipalname if
> +  p != canonical_name)
> +
> +if principals_to_add:
> +result = self.api.Command.user_add_principal(
> +obj_pkey, principals_to_add)['result']
> +
> +entry_attrs['krbprincipalname'] =
> result.get('krbprincipalname', [])
> +
> def check_mail(self, entry_attrs):
> if 'mail' in entry_attrs:
> entry_attrs['mail'] =
> self.obj.normalize_and_validate_email(entry_attrs['mail'])
> @@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):
>
> self.check_objectclass(ldap, dn, entry_attrs)
> self.obj.convert_usercertificate_pre(entry_attrs)
> +self.preserve_krbprincipalname_pre(ldap, entry_attrs,
> *keys,
> **options)
>
> def post_common_callback(self, ldap, dn, entry_attrs,
> *keys,
> **options):
> assert isinstance(dn, DN)
> +self.preserve_krbprincipalname_post(ldap, entry_attrs,
> **options)
> if options.get('random', False):
> try:
> entry_attrs['randompassword'] =
> unicode(getattr(context, 'randompassword'))
> --
> 2.5.5
>
 The approach looks good.

 For the record, we also support aliases for hosts and service
 kerberos
 principals but we don't support rename options for them, so there
 is no
 need to add similar logic there.


>>> That's right, I have updated the corresponding section of

[Freeipa-devel] [PATCH 0027][Tests] Fix failing automember tests in 4.3

2016-07-13 Thread Lenka Doudova 

Hi,

providing patch to fix two failing automember tests in 4.3 branch. The 
reason of the failure was the output normalization (specifically manager 
attribute for user).


The patch is intended for ipa-4-3 branch only.


Lenka

From 5c03137b1ea0adf756c05438c8dcceae98f0748a Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 13 Jul 2016 15:14:11 +0200
Subject: [PATCH] Tests: Fix failing automember tests

Two tests in xmlrpc/automember suite were failing as a result of manager data normalization in user attributes. Tests are fixed to reflect the change.
---
 ipatests/test_xmlrpc/test_automember_plugin.py | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py
index be0f7390565ed739aa66bc0c5c6d23d25d67df92..dde386286d26ddf4537fbc89647f1afecf51fcad 100644
--- a/ipatests/test_xmlrpc/test_automember_plugin.py
+++ b/ipatests/test_xmlrpc/test_automember_plugin.py
@@ -472,8 +472,7 @@ class test_automember(Declarative):
 summary=u'Added user "tuser1"',
 result=get_user_result(
 user1, u'Test', u'User1', 'add',
-manager=[DN(('uid', 'mscott'), ('cn', 'users'),
-('cn', 'accounts'), api.env.basedn)]
+manager=[manager1]
 )
 ),
 ),
@@ -1394,8 +1393,7 @@ class test_automember(Declarative):
 result=get_user_result(
 user1, u'Test', u'User1', 'add',
 memberof_group=[group1, u'ipausers'],
-manager=[DN(('uid', 'mscott'), ('cn', 'users'),
-('cn', 'accounts'), api.env.basedn)]
+manager=[manager1]
 ),
 ),
 ),
-- 
2.7.4

-- 
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 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Simo Sorce 
On Wed, 2016-07-13 at 13:53 +0200, Martin Babinsky wrote:
> On 07/12/2016 04:19 PM, Simo Sorce wrote:
> > 
> > On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:
> > > 
> > > On 07/12/2016 02:00 PM, Martin Babinsky wrote:
> > > > 
> > > > 
> > > > On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:
> > > > > 
> > > > > 
> > > > > On Mon, 11 Jul 2016, Martin Babinsky wrote:
> > > > > > 
> > > > > > 
> > > > > > From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep 17
> > > > > > 00:00:00 2001
> > > > > > From: Martin Babinsky 
> > > > > > Date: Fri, 1 Jul 2016 18:09:04 +0200
> > > > > > Subject: [PATCH] Preserve user principal aliases during
> > > > > > rename
> > > > > > operation
> > > > > > 
> > > > > > When a MODRDN is performed on the user entry, the MODRDN
> > > > > > plugin
> > > > > > resets
> > > > > > both
> > > > > > krbPrincipalName and krbCanonicalName to the value
> > > > > > constructed
> > > > > > from
> > > > > > uid. In
> > > > > > doing so, hovewer, any principal aliases added to the
> > > > > > krbPrincipalName
> > > > > > are
> > > > > > wiped clean. In this patch old aliases are fetched before
> > > > > > the
> > > > > > MODRDN
> > > > > > operation
> > > > > > takes place and inserted back after it is performed.
> > > > > > 
> > > > > > This also preserves previous user logins which can be used
> > > > > > further for
> > > > > > authentication as aliases.
> > > > > > 
> > > > > > https://fedorahosted.org/freeipa/ticket/6028
> > > > > > ---
> > > > > > ipaserver/plugins/baseuser.py | 46
> > > > > > +++
> > > > > > 1 file changed, 46 insertions(+)
> > > > > > 
> > > > > > diff --git a/ipaserver/plugins/baseuser.py
> > > > > > b/ipaserver/plugins/baseuser.py
> > > > > > index
> > > > > > 0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a131157815
> > > > > > ffb2
> > > > > > 452692a7edb342f6ac3
> > > > > > 
> > > > > > 100644
> > > > > > --- a/ipaserver/plugins/baseuser.py
> > > > > > +++ b/ipaserver/plugins/baseuser.py
> > > > > > @@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
> > > > > > len =
> > > > > > int(config.get('ipamaxusernamelength')[0])
> > > > > > )
> > > > > > )
> > > > > > +
> > > > > > +def preserve_krbprincipalname_pre(self, ldap,
> > > > > > entry_attrs,
> > > > > > *keys,
> > > > > > **options):
> > > > > > +"""
> > > > > > +preserve user principal aliases during rename
> > > > > > operation. This
> > > > > > is the
> > > > > > +pre-callback part of this. Another method called
> > > > > > during
> > > > > > post-callback
> > > > > > +shall insert the principals back
> > > > > > +"""
> > > > > > +if options.get('rename', None) is None:
> > > > > > +return
> > > > > > +
> > > > > > +try:
> > > > > > +old_entry = ldap.get_entry(
> > > > > > +entry_attrs.dn, attrs_list=(
> > > > > > +'krbprincipalname',
> > > > > > 'krbcanonicalname'))
> > > > > > +
> > > > > > +if 'krbcanonicalname' not in old_entry:
> > > > > > +return
> > > > > > +except errors.NotFound:
> > > > > > +self.obj.handle_not_found(*keys)
> > > > > > +
> > > > > > +self.context.krbprincipalname = old_entry.get(
> > > > > > +'krbprincipalname', [])
> > > > > > +
> > > > > > +def preserve_krbprincipalname_post(self, ldap,
> > > > > > entry_attrs,
> > > > > > **options):
> > > > > > +"""
> > > > > > +Insert the preserved aliases back to the user
> > > > > > entry
> > > > > > during
> > > > > > rename
> > > > > > +operation
> > > > > > +"""
> > > > > > +if options.get('rename', None) is None or not
> > > > > > hasattr(
> > > > > > +self.context, 'krbprincipalname'):
> > > > > > +return
> > > > > > +
> > > > > > +obj_pkey =
> > > > > > self.obj.get_primary_key_from_dn(entry_attrs.dn)
> > > > > > +canonical_name =
> > > > > > entry_attrs['krbcanonicalname'][0]
> > > > > > +
> > > > > > +principals_to_add = tuple(p for p in
> > > > > > self.context.krbprincipalname if
> > > > > > +  p != canonical_name)
> > > > > > +
> > > > > > +if principals_to_add:
> > > > > > +result = self.api.Command.user_add_principal(
> > > > > > +obj_pkey, principals_to_add)['result']
> > > > > > +
> > > > > > +entry_attrs['krbprincipalname'] =
> > > > > > result.get('krbprincipalname', [])
> > > > > > +
> > > > > > def check_mail(self, entry_attrs):
> > > > > > if 'mail' in entry_attrs:
> > > > > > entry_attrs['mail'] =
> > > > > > self.obj.normalize_and_validate_email(entry_attrs['mail'])
> > > > > > @@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):
> > > > > > 
> > > > > > self.check_objectclass(ldap,

Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Simo Sorce 
On Wed, 2016-07-13 at 14:37 +0200, Petr Vobornik wrote:
> On 07/12/2016 04:19 PM, Simo Sorce wrote:
> > 
> > On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:
> > > 
> > > On 07/12/2016 02:00 PM, Martin Babinsky wrote:
> > > > 
> > > > 
> > > > On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:
> > > > > 
> > > > > 
> > > > > On Mon, 11 Jul 2016, Martin Babinsky wrote:
> > > > > > 
> > > > > > 
> > > > > > From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep 17
> > > > > > 00:00:00 2001
> > > > > > From: Martin Babinsky 
> > > > > > Date: Fri, 1 Jul 2016 18:09:04 +0200
> > > > > > Subject: [PATCH] Preserve user principal aliases during
> > > > > > rename
> > > > > > operation
> > > > > > 
> > > > > > When a MODRDN is performed on the user entry, the MODRDN
> > > > > > plugin
> > > > > > resets
> > > > > > both
> > > > > > krbPrincipalName and krbCanonicalName to the value
> > > > > > constructed
> > > > > > from
> > > > > > uid. In
> > > > > > doing so, hovewer, any principal aliases added to the
> > > > > > krbPrincipalName
> > > > > > are
> > > > > > wiped clean. In this patch old aliases are fetched before
> > > > > > the
> > > > > > MODRDN
> > > > > > operation
> > > > > > takes place and inserted back after it is performed.
> > > > > > 
> > > > > > This also preserves previous user logins which can be used
> > > > > > further for
> > > > > > authentication as aliases.
> > > > > > 
> > > > > > https://fedorahosted.org/freeipa/ticket/6028
> > > > > > ---
> > > > > > ipaserver/plugins/baseuser.py | 46
> > > > > > +++
> > > > > > 1 file changed, 46 insertions(+)
> > > > > > 
> > > > > > diff --git a/ipaserver/plugins/baseuser.py
> > > > > > b/ipaserver/plugins/baseuser.py
> > > > > > index
> > > > > > 0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a131157815
> > > > > > ffb2
> > > > > > 452692a7edb342f6ac3
> > > > > > 
> > > > > > 100644
> > > > > > --- a/ipaserver/plugins/baseuser.py
> > > > > > +++ b/ipaserver/plugins/baseuser.py
> > > > > > @@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
> > > > > > len =
> > > > > > int(config.get('ipamaxusernamelength')[0])
> > > > > > )
> > > > > > )
> > > > > > +
> > > > > > +def preserve_krbprincipalname_pre(self, ldap,
> > > > > > entry_attrs,
> > > > > > *keys,
> > > > > > **options):
> > > > > > +"""
> > > > > > +preserve user principal aliases during rename
> > > > > > operation. This
> > > > > > is the
> > > > > > +pre-callback part of this. Another method called
> > > > > > during
> > > > > > post-callback
> > > > > > +shall insert the principals back
> > > > > > +"""
> > > > > > +if options.get('rename', None) is None:
> > > > > > +return
> > > > > > +
> > > > > > +try:
> > > > > > +old_entry = ldap.get_entry(
> > > > > > +entry_attrs.dn, attrs_list=(
> > > > > > +'krbprincipalname',
> > > > > > 'krbcanonicalname'))
> > > > > > +
> > > > > > +if 'krbcanonicalname' not in old_entry:
> > > > > > +return
> > > > > > +except errors.NotFound:
> > > > > > +self.obj.handle_not_found(*keys)
> > > > > > +
> > > > > > +self.context.krbprincipalname = old_entry.get(
> > > > > > +'krbprincipalname', [])
> > > > > > +
> > > > > > +def preserve_krbprincipalname_post(self, ldap,
> > > > > > entry_attrs,
> > > > > > **options):
> > > > > > +"""
> > > > > > +Insert the preserved aliases back to the user
> > > > > > entry
> > > > > > during
> > > > > > rename
> > > > > > +operation
> > > > > > +"""
> > > > > > +if options.get('rename', None) is None or not
> > > > > > hasattr(
> > > > > > +self.context, 'krbprincipalname'):
> > > > > > +return
> > > > > > +
> > > > > > +obj_pkey =
> > > > > > self.obj.get_primary_key_from_dn(entry_attrs.dn)
> > > > > > +canonical_name =
> > > > > > entry_attrs['krbcanonicalname'][0]
> > > > > > +
> > > > > > +principals_to_add = tuple(p for p in
> > > > > > self.context.krbprincipalname if
> > > > > > +  p != canonical_name)
> > > > > > +
> > > > > > +if principals_to_add:
> > > > > > +result = self.api.Command.user_add_principal(
> > > > > > +obj_pkey, principals_to_add)['result']
> > > > > > +
> > > > > > +entry_attrs['krbprincipalname'] =
> > > > > > result.get('krbprincipalname', [])
> > > > > > +
> > > > > > def check_mail(self, entry_attrs):
> > > > > > if 'mail' in entry_attrs:
> > > > > > entry_attrs['mail'] =
> > > > > > self.obj.normalize_and_validate_email(entry_attrs['mail'])
> > > > > > @@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):
> > > > > > 
> > > > > > self.check_objectclass(ldap, dn,

Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Simo Sorce 
On Wed, 2016-07-13 at 16:35 +0200, Martin Babinsky wrote:
> On 07/13/2016 04:28 PM, Simo Sorce wrote:
> > 
> > On Wed, 2016-07-13 at 16:19 +0200, Martin Babinsky wrote:
> > > 
> > > On 07/13/2016 03:08 PM, Simo Sorce wrote:
> > > > 
> > > > 
> > > > On Wed, 2016-07-13 at 14:37 +0200, Petr Vobornik wrote:
> > > > > 
> > > > > 
> > > > > On 07/12/2016 04:19 PM, Simo Sorce wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 07/12/2016 02:00 PM, Martin Babinsky wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On Mon, 11 Jul 2016, Martin Babinsky wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon
> > > > > > > > > > Sep
> > > > > > > > > > 17
> > > > > > > > > > 00:00:00 2001
> > > > > > > > > > From: Martin Babinsky 
> > > > > > > > > > Date: Fri, 1 Jul 2016 18:09:04 +0200
> > > > > > > > > > Subject: [PATCH] Preserve user principal aliases
> > > > > > > > > > during
> > > > > > > > > > rename
> > > > > > > > > > operation
> > > > > > > > > > 
> > > > > > > > > > When a MODRDN is performed on the user entry, the
> > > > > > > > > > MODRDN
> > > > > > > > > > plugin
> > > > > > > > > > resets
> > > > > > > > > > both
> > > > > > > > > > krbPrincipalName and krbCanonicalName to the value
> > > > > > > > > > constructed
> > > > > > > > > > from
> > > > > > > > > > uid. In
> > > > > > > > > > doing so, hovewer, any principal aliases added to
> > > > > > > > > > the
> > > > > > > > > > krbPrincipalName
> > > > > > > > > > are
> > > > > > > > > > wiped clean. In this patch old aliases are fetched
> > > > > > > > > > before
> > > > > > > > > > the
> > > > > > > > > > MODRDN
> > > > > > > > > > operation
> > > > > > > > > > takes place and inserted back after it is
> > > > > > > > > > performed.
> > > > > > > > > > 
> > > > > > > > > > This also preserves previous user logins which can
> > > > > > > > > > be
> > > > > > > > > > used
> > > > > > > > > > further for
> > > > > > > > > > authentication as aliases.
> > > > > > > > > > 
> > > > > > > > > > https://fedorahosted.org/freeipa/ticket/6028
> > > > > > > > > > ---
> > > > > > > > > > ipaserver/plugins/baseuser.py | 46
> > > > > > > > > > +++
> > > > > > > > > > 1 file changed, 46 insertions(+)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/ipaserver/plugins/baseuser.py
> > > > > > > > > > b/ipaserver/plugins/baseuser.py
> > > > > > > > > > index
> > > > > > > > > > 0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a1
> > > > > > > > > > 3115
> > > > > > > > > > 7815
> > > > > > > > > > ffb2
> > > > > > > > > > 452692a7edb342f6ac3
> > > > > > > > > > 
> > > > > > > > > > 100644
> > > > > > > > > > --- a/ipaserver/plugins/baseuser.py
> > > > > > > > > > +++ b/ipaserver/plugins/baseuser.py
> > > > > > > > > > @@ -498,6 +498,50 @@ class
> > > > > > > > > > baseuser_mod(LDAPUpdate):
> > > > > > > > > > len =
> > > > > > > > > > int(config.get('ipamaxusernamelength')[0])
> > > > > > > > > > )
> > > > > > > > > > )
> > > > > > > > > > +
> > > > > > > > > > +def preserve_krbprincipalname_pre(self, ldap,
> > > > > > > > > > entry_attrs,
> > > > > > > > > > *keys,
> > > > > > > > > > **options):
> > > > > > > > > > +"""
> > > > > > > > > > +preserve user principal aliases during
> > > > > > > > > > rename
> > > > > > > > > > operation. This
> > > > > > > > > > is the
> > > > > > > > > > +pre-callback part of this. Another method
> > > > > > > > > > called
> > > > > > > > > > during
> > > > > > > > > > post-callback
> > > > > > > > > > +shall insert the principals back
> > > > > > > > > > +"""
> > > > > > > > > > +if options.get('rename', None) is None:
> > > > > > > > > > +return
> > > > > > > > > > +
> > > > > > > > > > +try:
> > > > > > > > > > +old_entry = ldap.get_entry(
> > > > > > > > > > +entry_attrs.dn, attrs_list=(
> > > > > > > > > > +'krbprincipalname',
> > > > > > > > > > 'krbcanonicalname'))
> > > > > > > > > > +
> > > > > > > > > > +if 'krbcanonicalname' not in
> > > > > > > > > > old_entry:
> > > > > > > > > > +return
> > > > > > > > > > +except errors.NotFound:
> > > > > > > > > > +self.obj.handle_not_found(*keys)
> > > > > > > > > > +
> > > > > > > > > > +self.context.krbprincipalname =
> > > > > > > > > > old_entry.get(
> > > > > > > > > > +'krbprincipalname', [])
> > > > > > > > > > +
> > > > > > > > > > +def

Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Martin Babinsky 

On 07/13/2016 05:00 PM, Simo Sorce wrote:

On Wed, 2016-07-13 at 16:35 +0200, Martin Babinsky wrote:

On 07/13/2016 04:28 PM, Simo Sorce wrote:


On Wed, 2016-07-13 at 16:19 +0200, Martin Babinsky wrote:


On 07/13/2016 03:08 PM, Simo Sorce wrote:



On Wed, 2016-07-13 at 14:37 +0200, Petr Vobornik wrote:



On 07/12/2016 04:19 PM, Simo Sorce wrote:




On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:




On 07/12/2016 02:00 PM, Martin Babinsky wrote:





On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:





On Mon, 11 Jul 2016, Martin Babinsky wrote:





From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon
Sep
17
00:00:00 2001
From: Martin Babinsky 
Date: Fri, 1 Jul 2016 18:09:04 +0200
Subject: [PATCH] Preserve user principal aliases
during
rename
operation

When a MODRDN is performed on the user entry, the
MODRDN
plugin
resets
both
krbPrincipalName and krbCanonicalName to the value
constructed
from
uid. In
doing so, hovewer, any principal aliases added to
the
krbPrincipalName
are
wiped clean. In this patch old aliases are fetched
before
the
MODRDN
operation
takes place and inserted back after it is
performed.

This also preserves previous user logins which can
be
used
further for
authentication as aliases.

https://fedorahosted.org/freeipa/ticket/6028
---
ipaserver/plugins/baseuser.py | 46
+++
1 file changed, 46 insertions(+)

diff --git a/ipaserver/plugins/baseuser.py
b/ipaserver/plugins/baseuser.py
index
0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a1
3115
7815
ffb2
452692a7edb342f6ac3

100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -498,6 +498,50 @@ class
baseuser_mod(LDAPUpdate):
len =
int(config.get('ipamaxusernamelength')[0])
)
)
+
+def preserve_krbprincipalname_pre(self, ldap,
entry_attrs,
*keys,
**options):
+"""
+preserve user principal aliases during
rename
operation. This
is the
+pre-callback part of this. Another method
called
during
post-callback
+shall insert the principals back
+"""
+if options.get('rename', None) is None:
+return
+
+try:
+old_entry = ldap.get_entry(
+entry_attrs.dn, attrs_list=(
+'krbprincipalname',
'krbcanonicalname'))
+
+if 'krbcanonicalname' not in
old_entry:
+return
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
+self.context.krbprincipalname =
old_entry.get(
+'krbprincipalname', [])
+
+def preserve_krbprincipalname_post(self, ldap,
entry_attrs,
**options):
+"""
+Insert the preserved aliases back to the
user
entry
during
rename
+operation
+"""
+if options.get('rename', None) is None or
not
hasattr(
+self.context, 'krbprincipalname'):
+return
+
+obj_pkey =
self.obj.get_primary_key_from_dn(entry_attrs.dn)
+canonical_name =
entry_attrs['krbcanonicalname'][0]
+
+principals_to_add = tuple(p for p in
self.context.krbprincipalname if
+  p !=
canonical_name)
+
+if principals_to_add:
+result =
self.api.Command.user_add_principal(
+obj_pkey,
principals_to_add)['result']
+
+entry_attrs['krbprincipalname'] =
result.get('krbprincipalname', [])
+
def check_mail(self, entry_attrs):
if 'mail' in entry_attrs:
entry_attrs['mail'] =
self.obj.normalize_and_validate_email(entry_attrs['
mail
'])
@@ -557,9 +601,11 @@ class
baseuser_mod(LDAPUpdate):

self.check_objectclass(ldap, dn,
entry_attrs)
self.obj.convert_usercertificate_pre(entry_
attr
s)
+self.preserve_krbprincipalname_pre(ldap,
entry_attrs,
*keys,
**options)

def post_common_callback(self, ldap, dn,
entry_attrs,
*keys,
**options):
assert isinstance(dn, DN)
+self.preserve_krbprincipalname_post(ldap,
entry_attrs,
**options)
if options.get('random', False):
try:
entry_attrs['randompassword'] =
unicode(getattr(context, 'randompassword'))
--
2.5.5


The approach looks good.

For the record, we also support aliases for hosts and
service
kerberos
principals but we don't support rename options for
them,
so
there
is no
need to add similar logic there.



That's right, I have updated the corresponding section
of
the
design
page [1] for future reference.

[1]
http://www.freeipa.org/page/V4/Kerberos_principal_alias
es#M
anag
emen
t_framework



Adding Simo to the loop since he is not convinced that
this
is
the
right
behavior. As I see it, it seems to not be a security
issue
but
more
of a
different expectations about the perceived correct
behavior
in
this
particular situation.

I can see the use case in keeping the old aliases, e.g.
keeping
the
old
credentials after legal name change, but I can

Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Martin Babinsky 

On 07/13/2016 03:08 PM, Simo Sorce wrote:

On Wed, 2016-07-13 at 14:37 +0200, Petr Vobornik wrote:

On 07/12/2016 04:19 PM, Simo Sorce wrote:


On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:


On 07/12/2016 02:00 PM, Martin Babinsky wrote:



On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:



On Mon, 11 Jul 2016, Martin Babinsky wrote:



From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep 17
00:00:00 2001
From: Martin Babinsky 
Date: Fri, 1 Jul 2016 18:09:04 +0200
Subject: [PATCH] Preserve user principal aliases during
rename
operation

When a MODRDN is performed on the user entry, the MODRDN
plugin
resets
both
krbPrincipalName and krbCanonicalName to the value
constructed
from
uid. In
doing so, hovewer, any principal aliases added to the
krbPrincipalName
are
wiped clean. In this patch old aliases are fetched before
the
MODRDN
operation
takes place and inserted back after it is performed.

This also preserves previous user logins which can be used
further for
authentication as aliases.

https://fedorahosted.org/freeipa/ticket/6028
---
ipaserver/plugins/baseuser.py | 46
+++
1 file changed, 46 insertions(+)

diff --git a/ipaserver/plugins/baseuser.py
b/ipaserver/plugins/baseuser.py
index
0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a131157815
ffb2
452692a7edb342f6ac3

100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
len =
int(config.get('ipamaxusernamelength')[0])
)
)
+
+def preserve_krbprincipalname_pre(self, ldap,
entry_attrs,
*keys,
**options):
+"""
+preserve user principal aliases during rename
operation. This
is the
+pre-callback part of this. Another method called
during
post-callback
+shall insert the principals back
+"""
+if options.get('rename', None) is None:
+return
+
+try:
+old_entry = ldap.get_entry(
+entry_attrs.dn, attrs_list=(
+'krbprincipalname',
'krbcanonicalname'))
+
+if 'krbcanonicalname' not in old_entry:
+return
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
+self.context.krbprincipalname = old_entry.get(
+'krbprincipalname', [])
+
+def preserve_krbprincipalname_post(self, ldap,
entry_attrs,
**options):
+"""
+Insert the preserved aliases back to the user
entry
during
rename
+operation
+"""
+if options.get('rename', None) is None or not
hasattr(
+self.context, 'krbprincipalname'):
+return
+
+obj_pkey =
self.obj.get_primary_key_from_dn(entry_attrs.dn)
+canonical_name =
entry_attrs['krbcanonicalname'][0]
+
+principals_to_add = tuple(p for p in
self.context.krbprincipalname if
+  p != canonical_name)
+
+if principals_to_add:
+result = self.api.Command.user_add_principal(
+obj_pkey, principals_to_add)['result']
+
+entry_attrs['krbprincipalname'] =
result.get('krbprincipalname', [])
+
def check_mail(self, entry_attrs):
if 'mail' in entry_attrs:
entry_attrs['mail'] =
self.obj.normalize_and_validate_email(entry_attrs['mail'])
@@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):

self.check_objectclass(ldap, dn, entry_attrs)
self.obj.convert_usercertificate_pre(entry_attrs)
+self.preserve_krbprincipalname_pre(ldap,
entry_attrs,
*keys,
**options)

def post_common_callback(self, ldap, dn, entry_attrs,
*keys,
**options):
assert isinstance(dn, DN)
+self.preserve_krbprincipalname_post(ldap,
entry_attrs,
**options)
if options.get('random', False):
try:
entry_attrs['randompassword'] =
unicode(getattr(context, 'randompassword'))
--
2.5.5


The approach looks good.

For the record, we also support aliases for hosts and service
kerberos
principals but we don't support rename options for them, so
there
is no
need to add similar logic there.



That's right, I have updated the corresponding section of the
design
page [1] for future reference.

[1]
http://www.freeipa.org/page/V4/Kerberos_principal_aliases#Manag
emen
t_framework



Adding Simo to the loop since he is not convinced that this is
the
right
behavior. As I see it, it seems to not be a security issue but
more
of a
different expectations about the perceived correct behavior in
this
particular situation.

I can see the use case in keeping the old aliases, e.g. keeping
the
old
credentials after legal name change, but I can also see the
available
space for user principal names piling up and cluttering quickly
in
large
organizations.

after some thinking I think it is ok to keep by default and then
drop
as it avoid races if you do really want to

Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Simo Sorce 
On Wed, 2016-07-13 at 16:19 +0200, Martin Babinsky wrote:
> On 07/13/2016 03:08 PM, Simo Sorce wrote:
> > 
> > On Wed, 2016-07-13 at 14:37 +0200, Petr Vobornik wrote:
> > > 
> > > On 07/12/2016 04:19 PM, Simo Sorce wrote:
> > > > 
> > > > 
> > > > On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:
> > > > > 
> > > > > 
> > > > > On 07/12/2016 02:00 PM, Martin Babinsky wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On Mon, 11 Jul 2016, Martin Babinsky wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep
> > > > > > > > 17
> > > > > > > > 00:00:00 2001
> > > > > > > > From: Martin Babinsky 
> > > > > > > > Date: Fri, 1 Jul 2016 18:09:04 +0200
> > > > > > > > Subject: [PATCH] Preserve user principal aliases during
> > > > > > > > rename
> > > > > > > > operation
> > > > > > > > 
> > > > > > > > When a MODRDN is performed on the user entry, the
> > > > > > > > MODRDN
> > > > > > > > plugin
> > > > > > > > resets
> > > > > > > > both
> > > > > > > > krbPrincipalName and krbCanonicalName to the value
> > > > > > > > constructed
> > > > > > > > from
> > > > > > > > uid. In
> > > > > > > > doing so, hovewer, any principal aliases added to the
> > > > > > > > krbPrincipalName
> > > > > > > > are
> > > > > > > > wiped clean. In this patch old aliases are fetched
> > > > > > > > before
> > > > > > > > the
> > > > > > > > MODRDN
> > > > > > > > operation
> > > > > > > > takes place and inserted back after it is performed.
> > > > > > > > 
> > > > > > > > This also preserves previous user logins which can be
> > > > > > > > used
> > > > > > > > further for
> > > > > > > > authentication as aliases.
> > > > > > > > 
> > > > > > > > https://fedorahosted.org/freeipa/ticket/6028
> > > > > > > > ---
> > > > > > > > ipaserver/plugins/baseuser.py | 46
> > > > > > > > +++
> > > > > > > > 1 file changed, 46 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/ipaserver/plugins/baseuser.py
> > > > > > > > b/ipaserver/plugins/baseuser.py
> > > > > > > > index
> > > > > > > > 0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a13115
> > > > > > > > 7815
> > > > > > > > ffb2
> > > > > > > > 452692a7edb342f6ac3
> > > > > > > > 
> > > > > > > > 100644
> > > > > > > > --- a/ipaserver/plugins/baseuser.py
> > > > > > > > +++ b/ipaserver/plugins/baseuser.py
> > > > > > > > @@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
> > > > > > > > len =
> > > > > > > > int(config.get('ipamaxusernamelength')[0])
> > > > > > > > )
> > > > > > > > )
> > > > > > > > +
> > > > > > > > +def preserve_krbprincipalname_pre(self, ldap,
> > > > > > > > entry_attrs,
> > > > > > > > *keys,
> > > > > > > > **options):
> > > > > > > > +"""
> > > > > > > > +preserve user principal aliases during rename
> > > > > > > > operation. This
> > > > > > > > is the
> > > > > > > > +pre-callback part of this. Another method
> > > > > > > > called
> > > > > > > > during
> > > > > > > > post-callback
> > > > > > > > +shall insert the principals back
> > > > > > > > +"""
> > > > > > > > +if options.get('rename', None) is None:
> > > > > > > > +return
> > > > > > > > +
> > > > > > > > +try:
> > > > > > > > +old_entry = ldap.get_entry(
> > > > > > > > +entry_attrs.dn, attrs_list=(
> > > > > > > > +'krbprincipalname',
> > > > > > > > 'krbcanonicalname'))
> > > > > > > > +
> > > > > > > > +if 'krbcanonicalname' not in old_entry:
> > > > > > > > +return
> > > > > > > > +except errors.NotFound:
> > > > > > > > +self.obj.handle_not_found(*keys)
> > > > > > > > +
> > > > > > > > +self.context.krbprincipalname = old_entry.get(
> > > > > > > > +'krbprincipalname', [])
> > > > > > > > +
> > > > > > > > +def preserve_krbprincipalname_post(self, ldap,
> > > > > > > > entry_attrs,
> > > > > > > > **options):
> > > > > > > > +"""
> > > > > > > > +Insert the preserved aliases back to the user
> > > > > > > > entry
> > > > > > > > during
> > > > > > > > rename
> > > > > > > > +operation
> > > > > > > > +"""
> > > > > > > > +if options.get('rename', None) is None or not
> > > > > > > > hasattr(
> > > > > > > > +self.context, 'krbprincipalname'):
> > > > > > > > +return
> > > > > > > > +
> > > > > > > > +obj_pkey =
> > > > > > > > self.obj.get_primary_key_from_dn(entry_attrs.dn)
> > > > > > > > +canonical_name =
> > > > > > > > entry_attrs['krbcanonicalname'][0]
> > > > > > > > +
> > > > > > > > +principals_to_add = tuple(p for p in
> > > > > > > > self.context.krbprincipalname

Re: [Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

2016-07-13 Thread Martin Babinsky 

On 07/13/2016 04:28 PM, Simo Sorce wrote:

On Wed, 2016-07-13 at 16:19 +0200, Martin Babinsky wrote:

On 07/13/2016 03:08 PM, Simo Sorce wrote:


On Wed, 2016-07-13 at 14:37 +0200, Petr Vobornik wrote:


On 07/12/2016 04:19 PM, Simo Sorce wrote:



On Tue, 2016-07-12 at 15:46 +0200, Martin Babinsky wrote:



On 07/12/2016 02:00 PM, Martin Babinsky wrote:




On 07/12/2016 01:05 PM, Alexander Bokovoy wrote:




On Mon, 11 Jul 2016, Martin Babinsky wrote:




From 185bde00a76459430d95ff207bf1fb3fe31e811a Mon Sep
17
00:00:00 2001
From: Martin Babinsky 
Date: Fri, 1 Jul 2016 18:09:04 +0200
Subject: [PATCH] Preserve user principal aliases during
rename
operation

When a MODRDN is performed on the user entry, the
MODRDN
plugin
resets
both
krbPrincipalName and krbCanonicalName to the value
constructed
from
uid. In
doing so, hovewer, any principal aliases added to the
krbPrincipalName
are
wiped clean. In this patch old aliases are fetched
before
the
MODRDN
operation
takes place and inserted back after it is performed.

This also preserves previous user logins which can be
used
further for
authentication as aliases.

https://fedorahosted.org/freeipa/ticket/6028
---
ipaserver/plugins/baseuser.py | 46
+++
1 file changed, 46 insertions(+)

diff --git a/ipaserver/plugins/baseuser.py
b/ipaserver/plugins/baseuser.py
index
0052e718afe639bcc1c0a698ded39ea8407a0551..e4288a5a13115
7815
ffb2
452692a7edb342f6ac3

100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -498,6 +498,50 @@ class baseuser_mod(LDAPUpdate):
len =
int(config.get('ipamaxusernamelength')[0])
)
)
+
+def preserve_krbprincipalname_pre(self, ldap,
entry_attrs,
*keys,
**options):
+"""
+preserve user principal aliases during rename
operation. This
is the
+pre-callback part of this. Another method
called
during
post-callback
+shall insert the principals back
+"""
+if options.get('rename', None) is None:
+return
+
+try:
+old_entry = ldap.get_entry(
+entry_attrs.dn, attrs_list=(
+'krbprincipalname',
'krbcanonicalname'))
+
+if 'krbcanonicalname' not in old_entry:
+return
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
+self.context.krbprincipalname = old_entry.get(
+'krbprincipalname', [])
+
+def preserve_krbprincipalname_post(self, ldap,
entry_attrs,
**options):
+"""
+Insert the preserved aliases back to the user
entry
during
rename
+operation
+"""
+if options.get('rename', None) is None or not
hasattr(
+self.context, 'krbprincipalname'):
+return
+
+obj_pkey =
self.obj.get_primary_key_from_dn(entry_attrs.dn)
+canonical_name =
entry_attrs['krbcanonicalname'][0]
+
+principals_to_add = tuple(p for p in
self.context.krbprincipalname if
+  p != canonical_name)
+
+if principals_to_add:
+result =
self.api.Command.user_add_principal(
+obj_pkey, principals_to_add)['result']
+
+entry_attrs['krbprincipalname'] =
result.get('krbprincipalname', [])
+
def check_mail(self, entry_attrs):
if 'mail' in entry_attrs:
entry_attrs['mail'] =
self.obj.normalize_and_validate_email(entry_attrs['mail
'])
@@ -557,9 +601,11 @@ class baseuser_mod(LDAPUpdate):

self.check_objectclass(ldap, dn, entry_attrs)
self.obj.convert_usercertificate_pre(entry_attr
s)
+self.preserve_krbprincipalname_pre(ldap,
entry_attrs,
*keys,
**options)

def post_common_callback(self, ldap, dn,
entry_attrs,
*keys,
**options):
assert isinstance(dn, DN)
+self.preserve_krbprincipalname_post(ldap,
entry_attrs,
**options)
if options.get('random', False):
try:
entry_attrs['randompassword'] =
unicode(getattr(context, 'randompassword'))
--
2.5.5


The approach looks good.

For the record, we also support aliases for hosts and
service
kerberos
principals but we don't support rename options for them,
so
there
is no
need to add similar logic there.



That's right, I have updated the corresponding section of
the
design
page [1] for future reference.

[1]
http://www.freeipa.org/page/V4/Kerberos_principal_aliases#M
anag
emen
t_framework



Adding Simo to the loop since he is not convinced that this
is
the
right
behavior. As I see it, it seems to not be a security issue
but
more
of a
different expectations about the perceived correct behavior
in
this
particular situation.

I can see the use case in keeping the old aliases, e.g.
keeping
the
old
credentials after legal name change, but I can also see the
available
space for user principal names piling up and cluttering
quickly
in
large
organizations.

Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators

2016-07-13 Thread Milan Kubík 

On 07/11/2016 01:34 PM, Lenka Doudova wrote:




On 07/08/2016 02:24 PM, Milan Kubík wrote:

On 07/01/2016 05:13 PM, Lenka Doudova wrote:




On 07/01/2016 02:42 PM, Milan Kubík wrote:

On 06/16/2016 03:23 PM, Lenka Doudova wrote:

Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list of 
unimplemented methods is in doc. These methods can be filled in 
when existing declarative tests are refactored.


2. patch 0015 depends on 0014, so it should not be pushed without it.


Lenka





patch 0014:

In the update method, what happens when the updated attributes 
contain addattr? It is not clear to me. Is it necessary?



Example:
ipa service-mod SRV --addattr="authind=radius"

Result:
The way the tracker works, this adds 
/u'addattr="authind=radius"'/ to the list of expected results 
(result of /self.attrs.update(updates)/. Of course nothing like that 
appears anywhere, so in case there's the /--addattr/ option, it's 
necessary to ensure it won't get to the /self.attrs/ atribute.



patch 0015:

host1 and service2 do not tell anything about the purpose of the 
fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service entry 
exist during the second and third test case?



Renamed.


patch 0016:

Per offline discussion, admin user has no special privileges here, 
LGTM.


--
Milan Kubik


Thanks for review, fixed patches (14.2 and 15.2) attached.
Lenka


NACK,

the update method of ServiceTracker creates the entry if it doesn't 
exist. Why? I know the base class has this problem also [1], though.
Given this will be addressed, the fixtures in the xmlrpc test will 
fail since the fixture scope is wrong - function instead of class.


[1]: https://fedorahosted.org/freeipa/ticket/6045

--
Milan Kubik

Hi,

fixed patch attached. I chose to leave the scope as it was (in case 
one test breaks and entry is not even created, the other tests can 
still be successful), but I removed the creation command from 
ServiceTracker update method and called it directly in the tests, so 
there shouldn't be hidden actions.


Lenka


Thanks for the updated patches. There are still some issues I haven't 
noticed before:


service tracker: track_create method doesn't set self.exists flag which 
is needed


In the xmlrpc test method `test_adding_second_indicator` uses 'addattr' 
to set the indicator, why?


--
Milan Kubik

-- 
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