[Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation
Simo Sorce
simo at redhat.com
Wed Jul 13 13:08:52 UTC 2016
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 <mbabinsk at redhat.com>
> > > > > > 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 keep the aliases.
> >
> > However the CLI/UI should probably offer a button/switch to allow
> > to
> > drop all aliases on rename, what it would do would be to modify the
> > entry after the rename and drop the aliases.
> >
> > I am a bit uncertain what to do by default with the "original
> > name".
> > I can see it going both ways on whether to keep it by default or
> > not.
> Ideally we would have e.g.
>
> ipa user-rename oldCN --remove-aliases
>
> which would drop everything including oldCN (I would expect that).
I lean toward this conclusion too given that a later "--remove-alias"
would drop it, and we want to behave consistently.
> Unfortunately we have --rename in mod command
> ipa user-mod oldCN --rename newCn --remove-aliases
>
> And there --remove-aliases might not be the best thing.
Why not ?
> Do we want to support also:
> ipa user-mod CN --remove-aliases
Yes we probably want to give the option to drop aliases if an admin
realizes he should have done it at rename time but didn't.
Simo.
More information about the Freeipa-devel
mailing list