[SSSD] [PATCHES] PAC responder and client
Jan Zelený
jzeleny at redhat.com
Mon Jun 11 11:04:14 UTC 2012
Sumit sent me his set of patches last Friday. I've done some merges, filled
commit messages and other cosmetic adjustments. I've also done initial review
and I'm sending my comments:
get_rid() doesn't need to be in the header file, it can be static in the module
Defining idmap_talloc() and idmap_free() twice, please remove one of them.
The condition if (pac_ctx->idmap_ctx == NULL) in domsid_rid_to_uid() will be
always false, won't it? If it was NULL, the entire responder would fail way
earlier.
Missing debug_level initialization in main() in PAC responder
pac_save_memberships_next() is missing static in its definition (declaration is
ok)
pac_save_memberships_recv() is missing static
you are not assigning the result to *_pwd in get_pwd_from_pac()
Is the TODO in save_pac_user() valid and important? Do we need to address this
before pushing the patch set?
Nitpick: please use initialization instead of "g = 0;" in get_gids_from_pac()
Typo: Remote users are members ... (comment in pac_save_memberships_send())
Please remove the sysdb_search_group_by_gid() call in
pac_save_memberships_next(), it is redundant there and can be replaced by
checking the return code after calling pac_store_memberships().
You are not freeing pr_ctx in some error cases. The only place pr_ctx is
free'd is pac_save_memberships_done(), i.e. only when everything went ok.
Please add some talloc_free() calls where necessary. My first guess is that
pac_get_domains_done() and pac_add_pac_user() are the only two places where it
is necessary.
In function local_sid_to_id(): I think another check should be in place:
if (rid-remote_min+local_min > local_max) return ERANGE;
Sumit, please contact me if you need help with anything.
Thanks
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-PAC-responder-add-basic-infrastructure.patch
Type: text/x-patch
Size: 15851 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-PAC-responder-add-some-utility-functions.patch
Type: text/x-patch
Size: 4060 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-PAC-responder-add-the-core-functionality.patch
Type: text/x-patch
Size: 28654 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-PAC-responder-support-in-spec-file.patch
Type: text/x-patch
Size: 1098 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-PAC-responder-test-suite.patch
Type: text/x-patch
Size: 4841 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-PAC-client-add-basic-support-in-common-client-code.patch
Type: text/x-patch
Size: 3138 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-PAC-client-add-krb5-authdata-plugin.patch
Type: text/x-patch
Size: 19838 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1cdf038a/attachment.sig>
More information about the sssd-devel
mailing list