Attribute tags not working (?) about lico HOT 8 CLOSED

The behavior that I saw was that it was requesting the "text" (or "binary") attribute.

Yes, that is a bug. (As I said)

from lico.

They were trying to set the IDP_LDAP_UUID_ATTRIBUTE_TYPE=text env variable, but it didn't have the expected effect: instead of requesting the text version of the UUID,

I think this is a missunderstanding of what IDP_LDAP_UUID_ATTRIBUTE_TYPE actually does. IDP_LDAP_UUID_ATTRIBUTE_TYPE in ocis basically sets LDAP_UUID_ATTRIBUTE_TYPE in lico. LDAP_UUID_ATTRIBUTE_TYPE.

The setting is purely about how lico interprets values of the uuid attribute (defined in LDAP_UUID_ATTRIBUTE). It is not supposed to add any LDAP Attribute Options (like ;binary or ;text) to the search request (AFAIK).
For details see, this where the "%s_type" value is used again:

it requested both the uuid attribute and the text attribute (which doesn't exist).

This is a bug, I think.

The expected format for an attribute tag would be uuid;text.

No. I don't think so. Are there actually LDAP servers that support Attribute Options like text on a wider range of Attribute Type.

from lico.

If I read the code correctly it seems that we fetch a uuid_text or uuid_binary attribute (based on the value of LDAP_UUID_ATTRIBUTE_TYPE) for no reason. While this should be fixed I don't see how this can cause a problem? The LDAP server simply won't return this attribute if it does not exist.

I think we have no direct support for LDAP attribute options as of now.

If the LDAP server returns a requested attribute name with tags in the response, it might be possible to map the default uuid attribute name to uuid;text with a configuration like so: LDAP_UUID_ATTRIBUTE="uuid;text".

What is the problem with the uuid in the first place? The goal for LDAP_UUID_ATTRIBUTE_TYPE is to deal with the cases there the LDAP server returns uuid as binary (either some binary or as uuid bytes) as internally lico requires the uuid as string value.

from lico.

The behavior that I saw was that it was requesting the "text" (or "binary") attribute.

It's not a problem per se, as you said, the server will just not return the attribute if it doesn't exist, it's just a surprising behavior. In our case I think that leaving the env variable empty didn't request the attribute? Not sure about that (it's not my setup, I can't test things).

If you don't want to support attribute tags, no worries. It was just my guess as to the intention of the env variable. LLDAP doesn't actually support them, so it's easier for me :D

No, not a show-stopper bug by far, but I thought you'd want to know since requesting the extra attribute is not intended.

from lico.

All right, but how can this happen @rhafer - will you make a fix?

If you don't want to support attribute tags, no worries. It was just my guess as to the intention of the env variable. LLDAP doesn't actually support them, so it's easier for me :D

The question is more: do we need to support attribute tags for a valid production scenario?

from lico.

All right, but how can this happen

Because the LDAP_UUID_ATTRIBUTE_TYPE is just added to the ldapAttributeMapping map with the "_type" string appended. The functions using ldapAttributeMapping would need to ignore keys ending in "_type". E.g. the attributes() method:

https://github.com/libregraph/lico/blob/master/identifier/backends/ldap/ldap.go#L88

and newLDAPUser:

https://github.com/libregraph/lico/blob/master/identifier/backends/ldap/ldap.go#L106

I think putting that setting into the ldapAttributeMapping map was not a particular good idea.

@rhafer - will you make a fix?

I can't promise yet. But let's see if I can squeeze it in somehow. Good thing is that this does not really break anything. It's mostly an annoyance.

from lico.

The question is more: do we need to support attribute tags for a valid production scenario?

No. I don't think that support for "tags" (or attribute options as they are called in the RFCs IIRC) is needed anywhere in lico.

from lico.

Closing this, as for now we won't do anythin about this.

from lico.