Skip to content
Snippets Groups Projects

Convert ASCII-8BIT LDAP DNs to UTF-8 to avoid unnecessary user deletions

Merged Stan Hu requested to merge sh-ldap-handle-utf8-dn into master

Issue #1159 (closed) exposed a bug where LDAP DNs would be loaded in ASCII-8BIT encoding but compared against UTF-8-encoded values. This comparison would always fail, causing the LDAP group sync to evict users with Unicode characters. The problem was quietly masked because the user would be re-added later in the group sync worker.

This commit forces the UTF-8 encoding and falls back to the original value if that fails.

The net-ldap library has an outstanding issue (https://github.com/ruby-ldap/ruby-net-ldap/issues/4) to load data in UTF-8 format instead of ASCII-8BIT. Per https://tools.ietf.org/html/rfc4514#section-3, LDAP DNs should be in UTF-8.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Stan Hu Added 2 commits:

    Added 2 commits:

    Compare with previous version

  • Stan Hu Added 1 commit:

    Added 1 commit:

    Compare with previous version

  • Stan Hu Reassigned to @rymai

    Reassigned to @rymai

  • username-removed-128633
  • Stan Hu Resolved all discussions

    Resolved all discussions

  • Stan Hu Added 1 commit:

    Added 1 commit:

    • 69a28d5f - Simplify rescuing of clean_encoding

    Compare with previous version

  • Stan Hu Added 1 commit:

    Added 1 commit:

    Compare with previous version

  • Thanks @stanhu! ❤️

  • Added ldap label

  • username-removed-128633 Status changed to merged

    Status changed to merged

  • Mentioned in commit 7a1a90f2

  • 76 76 next
    77 77 end
    78 78
    79 # If there is more than one key/value set we must have a full DN,
    80 # or at least the probability is higher.
    81 if parsed_dn.count > 2
    82 dn
    83 elsif parsed_dn[0] == 'uid'
    84 dn_for_uid(parsed_dn[1])
    85 else
    86 logger.warn { "Found potentially malformed/incomplete DN: '#{dn}'" }
    87 dn
    88 end
    79 final_dn =
    • Would it be better to move this in to a method? As written, it's kind of confusing. If not a method maybe it would improve readability to hang the statement to the right of the = like final_dn = if parsed_dn.count > 2 and then indent everything. (I tried to illustrate but it's harder than heck to get the indentation to cooperate in our web editor).

    • @dblessing This should definitely moved to a method, yes! I think the goal was to have it fixed quickly in that case, but even if the MR is merged, your remarks are valid! Thanks!

    • Please register or sign in to reply
  • 89 end
    90
    91 clean_encoding(final_dn)
    89 92 end
    90 93
    91 94 # Remove `nil` values generated by the rescue above.
    92 95 dns.compact!
    93 96 end
    94 97
    98 # net-ldap only returns ASCII-8BIT and does not support UTF-8 out-of-the-box:
    99 # https://github.com/ruby-ldap/ruby-net-ldap/issues/4
    100 def clean_encoding(dn)
    101 return dn unless dn.present?
    102
    103 dn.force_encoding('UTF-8')
    104 rescue
  • Whoops, just saw this was already merged. Comments aren't that important in this case.

  • Author Maintainer

    @dblessing Still important, thanks.

  • Stan Hu Mentioned in issue #1308 (closed)

    Mentioned in issue #1308 (closed)

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading