[SSSD] [PATCH][PRELIMINARY] sudo: send username and uid while requesting default options

Jakub Hrozek jhrozek at redhat.com
Mon Apr 23 08:03:31 UTC 2012


On Fri, Apr 13, 2012 at 12:46:01PM +0200, Pavel Březina wrote:
> On 03/26/2012 09:47 PM, Jakub Hrozek wrote:
> >On Mon, Mar 19, 2012 at 02:46:02PM +0100, Pavel Březina wrote:
> >>https://fedorahosted.org/sssd/ticket/1239
> >>
> >>[PATCH 1/2]
> >>Finally removes EOK constant from sudo api header. It is not used in
> >>the SUDO code so it does not require their changes.
> >>
> >
> >Looks good to me, it'w weird to use EOK in an external header (even
> >though it is correctly defined).
> >
> >(Note: this patch shouldn't be pushed even though I don't have any
> >comments. It is an ABI break and we need to coordinate with Daniel K.)
> >
> >>[PATCH 2/2]
> >>This does what is requested in the ticket. It seems to be very huge but
> >>in fact it is mainly changing the variable. Basically I tried to get
> >>rid of domain ctx where possible, leave it only in initgroups part and
> >>use command ctx elsewhere.
> >>
> >
> >Still, it is hard to review the huge patch. Can you split it into
> >smaller ones? What about creating one that removes the duplication
> >between _get_sudorules and _get_defaults, one that converts to using
> >cmdctx and one that adds the uid support?
> 
> The patches are attached.
> 
> I want to get those patches acked because I need to deliver the updated
> interface to Dan so he can update the sudo binary.
> 
> It does not implement the in-memory cache (yet? :-)) due to the
> discussion with simo.
> 
> The patches expects "sudo api: check sss_status instead of errnop in
> sss_sudo_send_recv_generic()" (already acked on the list and waiting
> to be pushed).

Patch 0001: sudo api: remove EOK
Ack

Patch 0002: sudo responder: remove code duplication in commands
I think it would be a good idea to also check if rawname[rawname_len] ==
'\0' when parsing username, this would ensure that all the string checks
work OK.

Now that several sudosrv_response_append_* functions were removed from
header, can you make them static? In particular
sudosrv_response_append_string(), sudosrv_response_append_uint32(),
sudosrv_response_append_rule(), sudosrv_response_append_attr().

Patch 0003: sudo sysdb: make sysdb_get_sudo_user_info more configurable
Ack

Patch 0004: sudo api: send uid, username and domainname
Please get rid of the C++ comments in sudosrv_cmd(). If you need to
leave code in but disabled, please use #if 0/#endif The whole hunk that
changes sudosrv_cmd() seems like it belongs to patch #2.

There's another // comment in sudosrv_get_sudorules_from_cache.

The logic looks good to me, though. 

Even when the patches are acked, they shouldn't be pushed right?



More information about the sssd-devel mailing list