Skip to content

LDAP-166: Can't authenticate if ldap user has attribute with length g…#51

Open
trrenty wants to merge 2 commits intoxwiki-contrib:masterfrom
trrenty:LDAP-166
Open

LDAP-166: Can't authenticate if ldap user has attribute with length g…#51
trrenty wants to merge 2 commits intoxwiki-contrib:masterfrom
trrenty:LDAP-166

Conversation

@trrenty
Copy link
Copy Markdown
Contributor

@trrenty trrenty commented Apr 7, 2026

…reater than 768 characters

Fixes: jira.xwiki.org/browse/LDAP-166

I don't like the idea of trimming the value like that but im not sure of what other solution we might have. Any thoughts, @tmortagne ?

@trrenty trrenty requested a review from tmortagne April 7, 2026 12:50
@tmortagne
Copy link
Copy Markdown
Member

Well, first, note that using a hardcoded 768 value is not very accurate as it only makes sense for XWiki > 13.2, and the LDAP authenticator is supposed to support XWiki 10.11+. Also, in theory, even for recent versions of XWiki, 768 is not supposed to be used directly and might be different with a modified mapping (customized, or for a different database). There is an API to gather this limit ({{XWikiStoreInterface#getLimitSize}}), but it's been introduced in XWiki 11.4. Of course, it's still better than nothing and at the very least, there should be a FIXME to use the right API instead of this hardcoded value one the minimum support of the LDAP extension is upgraded.

Now on the general logic, as you mentioned, there is not much magic. I have the filling that cutting probably does not really make much sense for most of those fields (an email to which it's missing the end is totally unusable, and it does not make much more sense for the first or last name, for example). But not sure setting an empty value is more helpful in practice (at least you see something was set, but it was apparently too big) so I guess trimming with a warning is the less bad generic option. I guess we could imagine some property in {{xwiki.properties}} to control this behavior, possibly per field, but could probably be an improvement for later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants