[SSSD] [PATCHES] krb5: new option krb5_map_user

Pavel Reichl preichl at redhat.com
Thu Apr 30 11:45:01 UTC 2015


On 04/29/2015 07:00 PM, Lukas Slebodnik wrote:
> On (29/04/15 12:21), Pavel Reichl wrote:
>> Hello,
>>
>> please see attached patches, thanks!
> >From e824d4aa1051bf405243deb06cef1e05ed3ffd25 Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <preichl at redhat.com>
>> Date: Fri, 27 Feb 2015 11:37:50 -0500
>> Subject: [PATCH 1/3] krb5: new option krb5_map_user
>>
>> New option `krb5_map_user` providing mapping of ID provider names to
>> Kerberos principals.
>>
>> Resolves:
>> https://fedorahosted.org/sssd/ticket/2509
>> ---
>> src/config/SSSDConfig/__init__.py.in     |   1 +
>> src/config/SSSDConfigTest.py             |   9 ++-
>> src/config/etc/sssd.api.d/sssd-ad.conf   |   1 +
>> src/config/etc/sssd.api.d/sssd-ipa.conf  |   1 +
>> src/config/etc/sssd.api.d/sssd-krb5.conf |   1 +
>> src/man/sssd-krb5.5.xml                  |  26 ++++++++
>> src/providers/ad/ad_opts.h               |   1 +
>> src/providers/ipa/ipa_opts.h             |   1 +
>> src/providers/krb5/krb5_access.c         |   4 +-
>> src/providers/krb5/krb5_auth.c           |  40 +++++++++++-
>> src/providers/krb5/krb5_auth.h           |   2 +
>> src/providers/krb5/krb5_common.h         |   9 +++
>> src/providers/krb5/krb5_init_shared.c    | 101 +++++++++++++++++++++++++++++++
>> src/providers/krb5/krb5_opts.h           |   1 +
>> src/util/util.c                          |  56 +++++++++++++++++
>> src/util/util.h                          |   6 ++
>> 16 files changed, 254 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/util/util.c b/src/util/util.c
>> index 613c559bb2002686c7833642d0946e46e5a9b5d6..a3da6c51d2b92a529b03632a26b6e07569b5da77 100644
>> --- a/src/util/util.c
>> +++ b/src/util/util.c
>> @@ -30,6 +30,62 @@
>> #include "util/util.h"
>> #include "util/sss_utf8.h"
>>
>> +errno_t split_str(TALLOC_CTX *mem_ctx,
>> +                  const char delimiter,
>> +                  char *str,
>> +                  char **_first,
>> +                  char **_second)
>> +{
> Why do we need new function split_str?
> We already have function split_on_separator.
split_on_separator() does super set of split_str(),  code is IMO better 
with split_str() which is sufficient here. In reviewed patch I used 
both.  Are you sure you don't see the difference?

>
> The function split_on_separator is tested unlike yours function.
>
> I'm sorry but utility function without unit test(s) is automatic NACK.
Please could you point me to the source of such rules? I suppose other 
developers would appreciate such list too as  according to 
https://jhrozek.fedorapeople.org/sssd-coverage/html/util/util.c.gcov.html there 
are already functions without unit tests...how that could happen?
> You should even have thought about sending such patch. :-)
I agree that unit tests are cool and I would hate to lower code coverage 
of util.c so please see updated patch set.
>
>> +    char *sep;
>> +    errno_t ret;
>> +    char *first;
>> +    char *second;
>> +    TALLOC_CTX *tmp_ctx;
>> +
>> +    tmp_ctx = talloc_new(NULL);
>> +    if (tmp_ctx == NULL) {
>> +        ret = ENOMEM;
>> +        goto done;
>> +    }
>> +
>> +    sep = strchr(str, delimiter);
>> +    if (sep == NULL) {
>> +        ret = ENOENT;
>> +        goto done;
>> +    } else {
>> +        if (sep == str) {
>> +            first = NULL;
>> +        } else {
>> +            first = talloc_strndup(tmp_ctx, str, sep - str);
>> +            if (first == NULL) {
>> +                ret = ENOMEM;
>> +                goto done;
>> +            }
>> +        }
>> +
>> +        if (*(sep + 1) == '\0') {
>> +            second = NULL;
>> +        } else {
>> +            second = talloc_strdup(tmp_ctx, sep+1);
>> +            if (second == NULL) {
>> +                ret = ENOMEM;
>> +                goto done;
>> +            }
>> +        }
>> +    }
>> +
>> +    ret = EOK;
>> +
>> +done:
>> +    if (ret == EOK) {
>> +        *_first = talloc_steal(mem_ctx, first);
>> +        *_second = talloc_steal(mem_ctx, second);
>> +    }
>> +
>> +    talloc_free(tmp_ctx);
>> +    return ret;
>> +}
>> +
>> int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
>>                         const char sep, bool trim, bool skip_empty,
>>                         char ***_list, int *size)
>> diff --git a/src/util/util.h b/src/util/util.h
>> index 3c5ce295ec4ad2ad3c3a7ae0e7482c232db79746..479e24cabe947419cf9d63e803b1b662359ad38b 100644
>> --- a/src/util/util.h
>> +++ b/src/util/util.h
>> @@ -408,6 +408,12 @@ const char * const * get_known_services(void);
>>
>> errno_t sss_user_by_name_or_uid(const char *input, uid_t *_uid, gid_t *_gid);
>>
>> +errno_t split_str(TALLOC_CTX *mem_ctx,
>> +                  const char delimiter,
>> +                  char *str,
>> +                  char **_first,
>> +                  char **_second);
>> +
>> int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
>>                         const char sep, bool trim, bool skip_empty,
>>                         char ***_list, int *size);
>> -- 
>> 2.1.0
>>
> >From 50839ec51277cb9aa45c21b71bfb9dc5e8b0b553 Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <preichl at redhat.com>
>> Date: Wed, 29 Apr 2015 06:03:04 -0400
>> Subject: [PATCH 2/3] krb5: remove field run_as_user
>>
>> run_as_user is set set but never read.
>> ---
> LGTM
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-utils-new-function-split_str.patch
Type: text/x-patch
Size: 5487 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150430/ca3fd929/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-krb5-new-option-krb5_map_user.patch
Type: text/x-patch
Size: 16626 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150430/ca3fd929/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-krb5-remove-field-run_as_user.patch
Type: text/x-patch
Size: 2160 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150430/ca3fd929/attachment-0002.bin>


More information about the sssd-devel mailing list