[SSSD] [PATCH] Use new schema for HBAC service checks

Stephen Gallagher sgallagh at redhat.com
Sun May 23 16:31:06 UTC 2010


On 05/23/2010 12:14 PM, Stephen Gallagher wrote:
> On 05/21/2010 02:46 PM, Sumit Bose wrote:
>> Hi,
>>
>> here are two patches for the IPA access provider. The first one checks
>> if the rule is enable or not the second should fix #475 by using the new
>> service attribute and object in the IPA schema. Although the code should
>> handle service groups it currently does not work, because the IPA server
>> does not set the memberOf attribute to the service objects. If this is
>> fixed on the server side service groups should work, too.
>>
>> There are some areas of the code which are begging for refactoring, but
>> I think this is a post-1.2 task.
>
>
> Patch 0001: Ack
>
> Patch 0002: Ack


Pushed to sssd-1-2.


>
> This patch is functional, compiles cleanly and appears to work as
> advertised. I have some concerns (listed below), but they are not
> serious enough to warrant a Nack, given our time constraints for the
> SSSD 1.2.0 release.
>
> I'm comfortable with pushing this patch as-is to meet mandatory
> feature-completion and fixing the remains later.
>
>
> Fixes for the future (post 1.2.0 - don't spend time on it now):
>
> IPA_UNIQUE_ID exists as a patch on the FreeIPA development list. Please
> fix the assorted "TODO: I would prefer IPA_UNIQUE_ID but currently it is
> not available" spots post 1.2.0. (Qualifies as a bugfix).
>
> There is a complete lack of comments in the entire procedure. This made
> it extremely difficult to review. I would much prefer some comments
> explaining each step, but this isn't a nack in itself (under the current
> time constraints).
>
> If the online auth fails because of a lost connection during processing,
> we should retry from the cache before reporting an error.
>
> Please wrap the SYSDB cache update into its own complete tevent_req, so
> that a failure to update the cached service data is not a complete
> failure for the access-control. If the cache update request fails, the
> transaction should be cancelled, but the online operation should proceed
> based on the currently-available live data. A log message should be
> reported that the cache updated failed at level 0, but otherwise it's
> acceptable to return live data.
>
> Also, if the operation is proceeding offline, it seems useless to
> refresh the cache with the data we just got FROM the cache.
>


-- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/



More information about the sssd-devel mailing list