feature(ldap): Adds objectSid and objectUUID formatting when using Active Directory

[SeanHood]

Overview

Adds support for formatting objectSid and objectGUID for use with the ldap connector and Active Directory.

What this PR does / why we need it

Active Directory stores these two attributes as byte arrays. This change converts them to strings, allowing them to be used for any *Attr but intended for idAttr.

I believe that the Subject Claim within a JWT is intended to be a static non-changing value. In the context of AD objectGUID never changes and objectSid only changes in certain circumstances. Whereas a DN could change for a user either via a name change or if the object is moved to a new OU.

Prior to this change using either attribute would result in an error like so:

{"time":"2025-02-21T15:29:54.427305352Z","level":"ERROR","msg":"failed to marshal offline session ID","err":"string field contains invalid UTF-8","request_id":"6485c460-48ca-46bd-b1f1-30a4b9d14c6d"}
{"time":"2025-02-21T15:29:54.427340003Z","level":"ERROR","msg":"failed to create new access token","err":"failed to marshal offline session ID: string field contains invalid UTF-8","request_id":"6485c460-48ca-46bd-b1f1-30a4b9d14c6d"}

Closes: #3128

Implementation of formatSidAttr is inspired by: https://github.com/dotnet/runtime/blob/1f0e9eadd29b9c15dae9313234abf486f1c4ef96/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs#L395

Special notes for your reviewer

Test config:

connectors:
  - type: ldap
    id: ldap
    name: AD
    config:
      host: ad.example.com:636

      bindDN: uid=serviceaccount,cn=users,dc=example,dc=com
      bindPW: password

      userSearch:
        baseDN: OU=Users,DC=example,DC=com
        filter: "(objectClass=person)"
        username: sAMAccountName
        idAttr: objectSid
        emailAttr: mail
        nameAttr: cn
        preferredUsernameAttr: sAMAccountName

      groupSearch:
        baseDN: OU=Groups,DC=example,DC=com
        filter: "(objectClass=group)"
        userMatchers:
          - userAttr: DN
            groupAttr: "member:1.2.840.113556.1.4.1941:"
        nameAttr: cn

[slrz]

How Dex handles non-UTF-8 data from LDAP attributes certainly leaves room for improvement, if not to say is totally broken (cf. #4017). So I sympathize with this PR (and we apply something similar as a downstream patch).

But as you say, the OIDC subject is supposed to not change over time but this is what's going to happen for existing Dex users who chose objectSid as the idAttr. One might argue that their usage was broken to begin with but still, this is a major breaking change and should be communicated as such in release notes.

If the log excerpts in your message really mean that using non-UTF8 attributes is no longer possible, then the breaking change already happened (but wasn't part of a release yet, I think),

[slrz]

With regard to the actual code: are there cases where Dex would benefit from being aware of the structure and preferred text representation of NT SIDs and other binary attributes? At least for the sub claim, it is going to stuff it into a Protobuf encoder together with some extra data, Base64-encode the result and use that as the subject.

Might as well simplify the code and treat all binary strings the same, encoding them in some Base32 variant (or a plain hex string), always. Avoid any encoding where distinct inputs may yield outputs that differ in case only, as the etcd backend will strings.ToLower them (see #4016 for the issue).

Of course, if the project decides to do a breaking change, this should rather be fixed directly within the etcd backend.