[Freeipa-devel] [PATCH] 0032 Validate sudo RunAsUser/RunAsGroup arguments

[Rob Crittenden]

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