On Mon, Aug 26, 2013 at 11:41:02AM +0200, Jakub Hrozek wrote:
> On Mon, Aug 26, 2013 at 11:26:41AM +0200, Sumit Bose wrote:
> > On Mon, Aug 26, 2013 at 11:02:56AM +0200, Jakub Hrozek wrote:
> > > On Fri, Aug 16, 2013 at 06:25:02PM +0200, Sumit Bose wrote:
> > > > Hi,
> > > > 
> > > > this series of patches contains improvements for the PAC responder
> > > > related to the support of UIDs and GIDs managed by AD.
> > > > 
> > > > The first patch is a fix for https://fedorahosted.org/sssd/ticket/1996.
> > > > The original idea in the ticket was to modify an existing user entry
> > > > instead of deleting and recreating it. But since the PAC does not
> > > > contain any useful information which would improve the entry I decided 
> > > > to
> > > > not touch existing user entries at all and only update the group
> > > > memberships.
> > > > 
> > > > Please find details about the other patches in the commit messages.
> > > > 
> > > > bye,
> > > > Sumit
> > > 
> > > Hi,
> > > 
> > > The patches work well with one addition I sent to the list separately. I
> > > don't have any change requests, just some questions before the patches
> > > are applied, see below:
> > > 
> > > > From 1e54d2061d72d2f72c30d3fced9b43d4ec369a28 Mon Sep 17 00:00:00 2001
> > > > From: Sumit Bose <[email protected]>
> > > > Date: Thu, 1 Aug 2013 12:40:24 +0200
> > > > Subject: [PATCH 1/6] PAC: if user entry already exists keep it
> > > > 
> > > > Currently the PAC responder deletes a user entry and recreates it if
> > > > some attributes seems to be different.
> > > > 
> > > > Two of the attributes where the home directory and the shell of the
> > > > user. Those two attributes are not available from the PAC but where
> > > > generates by the PAC responder. The corresponding ID provider might have
> > > > better means to determine those attributes, e.g. read them from LDAP, so
> > > > we shouldn't change them here.
> > > > 
> > > > The third attribute is the user name. Since the PAC responder does
> > > > lookups only based on the UID we can wait until the ID provider updates
> > > > the entry.
> > > 
> > > The current logic doesn't detect if any of the extra attributes we put
> > > into the "_attrs" variable of get_pwd_from_pac() changed. I think this
> > > can only apply to UPN as the alias is derived from the name and the SID
> > > should never change. Can this be a problem?
> > 
> > I think not. If it is the generated UPN, i.e. username + realm, they
> > are the same and if the UPN in the cache was extracted from the Kerberos
> > ticket during password authentication it would be the more correct one.
> > 
> > > 
> > > Also maybe we should lowercase the alias explicitly as we do in the IPA
> > > extdom handler code. But this patch is good as-is, I think, if there are
> > > any issues, they can be solved separately.
> > 
> > I agree this is a good idea. Maybe we should consider to introduce a new
> > sysdb call like sysdb_attrs_add_lower_case_string()? Would you mind to
> > open a suitable ticket?
> > 
> > Thank you for the review.
> 
> Sure:
> https://fedorahosted.org/sssd/ticket/2056
> 
> ACK to the patch #1.

Pushed to master.
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to