[SSSD] [PATCHES] Make nss responder aware of views and overrides
Pavel Březina
pbrezina at redhat.com
Fri Oct 17 12:05:44 UTC 2014
On 10/15/2014 01:08 PM, Sumit Bose wrote:
> On Tue, Oct 14, 2014 at 05:57:36PM +0200, Sumit Bose wrote:
>> Hi,
>>
>> this patches make the nss responder view aware. A number of new sysdb
>> calls is added as drop in replacements from existing ones. I preferred
>> this instead of modifying the existing ones to not break their usage in
>> other places. But I think they are a good targets, together with the
>> other user and group related sysdb calls, for a refactoring in an
>> upcoming version.
>>
>> bye,
>> Sumit
>
> Please find attached a new version with some minor fixes in fill_pwent()
> and fill_grent() with respect to fully qualified names and an additional
> patch with the "changes" for the pam responder.
>
> bye,
> Sumit
Patch #4 sysdb: add overide lookup calls won't apply to current master.
But the rebase was trivial so it didn't stop me from reviewing.
Patch #1: confdb: add has_views and view_name to sss_domain_info
ACK
Patch #2: new_subdomain: copy view data from parent
ACK
Patch #3: sysdb: add view data to domains
> + if (ret == ENOENT || view_name == NULL
> + || (view_name != NULL
> + && strcmp(view_name, SYSDB_DEFAULT_VIEW_NAME) == 0)) {
view_name != NULL condition can be skipped
And the whole branching in sysdb_master_domain_update() is quite complex
and difficult to understand. Can you add a comment to the top of each
branch that says when the branch will be executed?
Patch #4: sysdb: add overide lookup calls
ACK
Patch #5: sysdb: add sysdb_getpwnam/uid_with_views()
> + /* TODO: check if EOK is returned even if orig_obj is empty, i.e nothgin
> + * found. */
This is strange todo to leave in the code. Can you do it? :-)
Patch #6: sysdb: add sss_view_ldb_msg_find_element/attr_as_string/uint64
ACK
Patch #7: nss: add view support for getpwnam/getpwuid requests
I think you can use sss_view_ldb* functions from the previous patch,
can't you?
Patch #8: sysdb: add sysdb_initgroups_with_views()
ACK
Patch #9: nss: add view support to initgroups request
ACK
Patch #10: sysdb: add sysdb_getgrnam_with_views and
sysdb_getgrgid_with_views
> + /* TODO: check if EOK is returned even if orig_obj is empty, i.e nothing
> + * found. */
Again.
Patch #11: nss: add view support for getgr* requests
Coverity warning:
nsssrv_cmd.c:2856: value_overwrite: Value "22" is overwritten with value
from "sss_packet_set_size(packet, 0UL)".
nsssrv_cmd.c:2810: assigned_value: Value "22" is assigned to "ret" here,
but that stored value is not used before it is overwritten.
Patch #12: sid2name: return name without views applied
> + if (apply_no_view) {
> + orig_name = ldb_msg_find_attr_as_string(msg,
> + ORIGINALAD_PREFIX SYSDB_NAME,
> + NULL);
> + }
> +
> + if (orig_name == NULL) {
> + orig_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
> + }
The name of the variable 'apply_no_view' suggests that if true, no
override should be applied. Thus if false, shouldn't we use
sss_view_ldb* to get overriden value if possible?
Patch #13: pam: make pam responder aware if views
ACK
I was using these patches for quite some time during my work on IFP and
sudo responder part of the views. All known issues seems to be resolved.
Functional ACK.
More information about the sssd-devel
mailing list