URL:
https://github.com/SSSD/sssd/pull/450
Title: #450: sysdb: do not use objectClass for users and groups
jhrozek commented:
"""
I'm sorry for the delay in review. I checked how we use SYSDB_OBJECTCLASS in the
remainder of the code and there are some places that need to be fixed, but also some where
I'm not sure and would like to ask for help:
1. I think this one is mostly harmless, but it's better to fix the class->category
because we never know what purpose will the search result be used for later
```
diff --git a/src/providers/ad/ad_pac.c b/src/providers/ad/ad_pac.c
index ed002e1f9..6b47462cf 100644
--- a/src/providers/ad/ad_pac.c
+++ b/src/providers/ad/ad_pac.c
@@ -31,7 +31,7 @@ static errno_t find_user_entry(TALLOC_CTX *mem_ctx, struct
sss_domain_info *dom,
struct dp_id_data *ar,
struct ldb_message **_msg)
{
- const char *user_attrs[] = { SYSDB_NAME, SYSDB_OBJECTCLASS,
+ const char *user_attrs[] = { SYSDB_NAME, SYSDB_OBJECTCATEGORY,
SYSDB_PAC_BLOB, SYSDB_PAC_BLOB_EXPIRE,
NULL };
struct ldb_message *msg;
```
2. Here attrs is used as input for sysdb_search_by_cert, so I think we should use class
```
diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c
index 3896dc3c7..8f8759f64 100644
--- a/src/providers/ipa/ipa_id.c
+++ b/src/providers/ipa/ipa_id.c
@@ -734,7 +734,7 @@ static void ipa_id_get_account_info_orig_done(struct tevent_req
*subreq)
const char *attrs[] = { SYSDB_NAME,
SYSDB_UIDNUM,
SYSDB_SID_STR,
- SYSDB_OBJECTCLASS,
+ SYSDB_OBJECTCATEGORY,
SYSDB_UUID,
SYSDB_GHOST,
SYSDB_HOMEDIR,
```
3. Similar as the one before, except elsewhere
```
diff --git a/src/providers/ipa/ipa_subdomains_ext_groups.c
b/src/providers/ipa/ipa_subdomains_ext_groups.c
index 0359e0ded..9e1d6c3a9 100644
--- a/src/providers/ipa/ipa_subdomains_ext_groups.c
+++ b/src/providers/ipa/ipa_subdomains_ext_groups.c
@@ -940,7 +940,7 @@ search_user_or_group_by_sid_str(TALLOC_CTX *mem_ctx,
const char *attrs[] = { SYSDB_NAME,
SYSDB_SID_STR,
SYSDB_ORIG_DN,
- SYSDB_OBJECTCLASS,
+ SYSDB_OBJECTCATEGORY,
SYSDB_CACHE_EXPIRE,
NULL };
TALLOC_CTX *tmp_ctx = NULL;
```
4. Just a DEBUG message
```
diff --git a/src/providers/ldap/sdap_async_groups.c
b/src/providers/ldap/sdap_async_groups.c
index f084469e7..b1cfb7e4a 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -94,7 +94,7 @@ static int sdap_find_entry_by_origDN(TALLOC_CTX *memctx,
NULL);
if (objectclass == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "An entry without a %s?\n",
- SYSDB_OBJECTCLASS);
+ SYSDB_OBJECTCATEGORY);
ret = EINVAL;
goto done;
}
```
5. We actually use the class here for comparison
```
diff --git a/src/responder/ifp/ifp_groups.c b/src/responder/ifp/ifp_groups.c
index 8ddbf2d49..b274b8f52 100644
--- a/src/responder/ifp/ifp_groups.c
+++ b/src/responder/ifp/ifp_groups.c
@@ -841,7 +841,7 @@ ifp_groups_group_get_members(TALLOC_CTX *mem_ctx,
int num_groups;
int i;
errno_t ret;
- const char *attrs[] = {SYSDB_OBJECTCLASS, SYSDB_UIDNUM,
+ const char *attrs[] = {SYSDB_OBJECTCATEGORY, SYSDB_UIDNUM,
SYSDB_GIDNUM, NULL};
tmp_ctx = talloc_new(NULL);
```
6. Here I wasn't sure if it's OK to remove the class (I think yes), but at the
same time I see no harm in having both, except maybe confusion of future programmers
```
diff --git a/src/responder/nss/nss_cmd.c b/src/responder/nss/nss_cmd.c
index 545257a0b..ab5a57243 100644
--- a/src/responder/nss/nss_cmd.c
+++ b/src/responder/nss/nss_cmd.c
@@ -1148,7 +1148,9 @@ static errno_t nss_cmd_getorigbyname(struct cli_ctx *cli_ctx)
errno_t ret;
struct nss_ctx *nss_ctx;
const char **attrs;
- static const char *defattrs[] = { SYSDB_NAME, SYSDB_OBJECTCLASS,
+ static const char *defattrs[] = { SYSDB_NAME,
+ SYSDB_OBJECTCLASS,
+ SYSDB_OBJECTCATEGORY,
SYSDB_SID_STR,
ORIGINALAD_PREFIX SYSDB_NAME,
ORIGINALAD_PREFIX SYSDB_UIDNUM,
```
And here I'm not sure:
```
diff --git a/src/providers/ldap/sdap_async_initgroups.c
b/src/providers/ldap/sdap_async_initgroups.c
index a33975cde..f8a844747 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -2341,7 +2341,7 @@ static errno_t rfc2307bis_nested_groups_step(struct tevent_req
*req)
}
ret = sysdb_attrs_get_string(state->groups[state->group_iter],
- SYSDB_OBJECTCLASS, &class);
+ SYSDB_OBJECTCATEGORY, &class);
if (ret == EOK) {
/* If there is a objectClass attribute the object is coming from the
* cache and the name attribute of the object already has the primary
```
The comment makes it sound like category would be the correct one, but I can't tell if
the attrs can come from sdap parsing only or also from a cache search..and I haven't
been able to hit this code either so far.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/450#issuecomment-347042018