Alexander Bokovoy wrote:
On Mon, 12 Dec 2011, Rob Crittenden wrote:
actual members, it treats it as a no-op. We should probably be
consistent.
Don't understand. Did you mean 'to not provide any actual members'?

In case you did, attached patch removes remaining checks for
runas_{user,group) to be False.


It would probably be better to return the value as passed in by the
user rather than user[0].value.
The issue here is that names come to the callback already as DNs from
LDAPAddMember's execute() method. Strictly speaking it is already
different to what user has entered as we do expansion by default to
add $SUFFIX and appropriate container.

In the updated patch I tried to reduce DN to something reasonable by
relying on known containers and only showing full DN for cases when
these are not users/groups containers.


ACK on this patch.

Do we need to add similar to HBAC plugin and sudorule-add-user,
add-command, etc?
I was thinking about it as well, probably makes sense, indeed. What
about reduction code to be a method of DN itself?

Something like

class DN:
     def relative_to(self, env, cn_name):
         try:
             cn_ = 'container_%s' % (cn_name)
             if cn_ in env:
                 cn = DN(env[cn_])+DN(env.basedn)
             else:
                 return self
          except:
             return self
          if self.endswith(cn):
              return self[0].value
          return self


print dn.relative_to(env, 'user')

If this is acceptable, I can do refactoring in a different ticket.

NACK.

We still have the value passed in by the user, right (in options['user'] and options['group'])? We basically take that, create a DN out of it, then pull the same value out. Why not skip all that and just look at the raw values instead?

Or there is already a helper to get the key out of a dn, see self.Object.user.get_primary_key_from_dn(str(group))

Also, I found this doesn't handle a list of users or groups. If you pass in --users=joe,all then both get added as external users (assuming joe doesn't already exist, of course).

rob

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to