providers/ldap: Improve compatibility with LDAP clients (#4750)

* Fixed invalid LDAP attributes by replacing '.'s and '/'s with '-'

* Leave old fields for now for backward compatibility

* Add forgotten depreceated field

* Fix tests

* Fix tests

* use shorter attribute names

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

* sanitize attributes

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

* keep both sanitized and unsanitized user fields

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

* add sanitized fields to test

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

---------

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
Co-authored-by: Jens Langhammer <jens@goauthentik.io>

internal/outpost/ldap/entries.go

@@ -11,7 +11,9 @@ import (
 
 func (pi *ProviderInstance) UserEntry(u api.User) *ldap.Entry {
 	dn := pi.GetUserDN(u.Username)
-	attrs := utils.AKAttrsToLDAP(u.Attributes)
+	attrs := utils.AttributesToLDAP(u.Attributes, false)
+	sanitizedAttrs := utils.AttributesToLDAP(u.Attributes, true)
+	attrs = append(attrs, sanitizedAttrs...)
 
 	if u.IsActive == nil {
 		u.IsActive = api.PtrBool(false)
@@ -20,9 +22,13 @@ func (pi *ProviderInstance) UserEntry(u api.User) *ldap.Entry {
 		u.Email = api.PtrString("")
 	}
 	attrs = utils.EnsureAttributes(attrs, map[string][]string{
-		"memberOf":                      pi.GroupsForUser(u),
+		// Old fields for backwards compatibility
 		"goauthentik.io/ldap/active":    {strconv.FormatBool(*u.IsActive)},
 		"goauthentik.io/ldap/superuser": {strconv.FormatBool(u.IsSuperuser)},
+		// End old fields
+		"ak-active":      {strconv.FormatBool(*u.IsActive)},
+		"ak-superuser":   {strconv.FormatBool(u.IsSuperuser)},
+		"memberOf":       pi.GroupsForUser(u),
 		"cn":             {u.Username},
 		"sAMAccountName": {u.Username},
 		"uid":            {u.Uid},

internal/outpost/ldap/group/group.go

@@ -22,7 +22,9 @@ type LDAPGroup struct {
 }
 
 func (lg *LDAPGroup) Entry() *ldap.Entry {
-	attrs := utils.AKAttrsToLDAP(lg.AKAttributes)
+	attrs := utils.AttributesToLDAP(lg.AKAttributes, false)
+	sanitizedAttrs := utils.AttributesToLDAP(lg.AKAttributes, true)
+	attrs = append(attrs, sanitizedAttrs...)
 
 	objectClass := []string{constants.OCGroup, constants.OCGroupOfUniqueNames, constants.OCGroupOfNames, constants.OCAKGroup}
 	if lg.IsVirtualGroup {
@@ -30,9 +32,12 @@ func (lg *LDAPGroup) Entry() *ldap.Entry {
 	}
 
 	attrs = utils.EnsureAttributes(attrs, map[string][]string{
+		// Old fields for backwards compatibility
+		"goauthentik.io/ldap/superuser": {strconv.FormatBool(lg.IsSuperuser)},
+		// End old fields
+		"ak-superuser":   {strconv.FormatBool(lg.IsSuperuser)},
 		"objectClass":    objectClass,
 		"member":         lg.Member,
-		"goauthentik.io/ldap/superuser": {strconv.FormatBool(lg.IsSuperuser)},
 		"cn":             {lg.CN},
 		"uid":            {lg.Uid},
 		"sAMAccountName": {lg.CN},

internal/outpost/ldap/utils/utils.go

@@ -9,6 +9,14 @@ import (
 	ldapConstants "goauthentik.io/internal/outpost/ldap/constants"
 )
 
+func AttributeKeySanitize(key string) string {
+	return strings.ReplaceAll(
+		strings.ReplaceAll(key, "/", "-"),
+		".",
+		"",
+	)
+}
+
 func stringify(in interface{}) *string {
 	switch t := in.(type) {
 	case string:
@@ -36,13 +44,16 @@ func stringify(in interface{}) *string {
 	}
 }
 
-func AKAttrsToLDAP(attrs map[string]interface{}) []*ldap.EntryAttribute {
+func AttributesToLDAP(attrs map[string]interface{}, sanitize bool) []*ldap.EntryAttribute {
 	attrList := []*ldap.EntryAttribute{}
 	if attrs == nil {
 		return attrList
 	}
 	for attrKey, attrValue := range attrs {
 		entry := &ldap.EntryAttribute{Name: attrKey}
+		if sanitize {
+			entry.Name = AttributeKeySanitize(attrKey)
+		}
 		switch t := attrValue.(type) {
 		case []string:
 			entry.Values = t

internal/outpost/ldap/utils/utils_test.go

@@ -19,16 +19,16 @@ func TestAKAttrsToLDAP_String(t *testing.T) {
 	u.Attributes = map[string]interface{}{
 		"foo": "bar",
 	}
-	assert.Equal(t, 1, len(AKAttrsToLDAP(u.Attributes)))
+	assert.Equal(t, 1, len(AttributesToLDAP(u.Attributes, true)))
-	assert.Equal(t, "foo", AKAttrsToLDAP(u.Attributes)[0].Name)
+	assert.Equal(t, "foo", AttributesToLDAP(u.Attributes, true)[0].Name)
-	assert.Equal(t, []string{"bar"}, AKAttrsToLDAP(u.Attributes)[0].Values)
+	assert.Equal(t, []string{"bar"}, AttributesToLDAP(u.Attributes, true)[0].Values)
 	// pointer string
 	u.Attributes = map[string]interface{}{
 		"foo": api.PtrString("bar"),
 	}
-	assert.Equal(t, 1, len(AKAttrsToLDAP(u.Attributes)))
+	assert.Equal(t, 1, len(AttributesToLDAP(u.Attributes, true)))
-	assert.Equal(t, "foo", AKAttrsToLDAP(u.Attributes)[0].Name)
+	assert.Equal(t, "foo", AttributesToLDAP(u.Attributes, true)[0].Name)
-	assert.Equal(t, []string{"bar"}, AKAttrsToLDAP(u.Attributes)[0].Values)
+	assert.Equal(t, []string{"bar"}, AttributesToLDAP(u.Attributes, true)[0].Values)
 }
 
 func TestAKAttrsToLDAP_String_List(t *testing.T) {
@@ -37,16 +37,16 @@ func TestAKAttrsToLDAP_String_List(t *testing.T) {
 	u.Attributes = map[string]interface{}{
 		"foo": []string{"bar"},
 	}
-	assert.Equal(t, 1, len(AKAttrsToLDAP(u.Attributes)))
+	assert.Equal(t, 1, len(AttributesToLDAP(u.Attributes, true)))
-	assert.Equal(t, "foo", AKAttrsToLDAP(u.Attributes)[0].Name)
+	assert.Equal(t, "foo", AttributesToLDAP(u.Attributes, true)[0].Name)
-	assert.Equal(t, []string{"bar"}, AKAttrsToLDAP(u.Attributes)[0].Values)
+	assert.Equal(t, []string{"bar"}, AttributesToLDAP(u.Attributes, true)[0].Values)
 	// pointer string list
 	u.Attributes = map[string]interface{}{
 		"foo": &[]string{"bar"},
 	}
-	assert.Equal(t, 1, len(AKAttrsToLDAP(u.Attributes)))
+	assert.Equal(t, 1, len(AttributesToLDAP(u.Attributes, true)))
-	assert.Equal(t, "foo", AKAttrsToLDAP(u.Attributes)[0].Name)
+	assert.Equal(t, "foo", AttributesToLDAP(u.Attributes, true)[0].Name)
-	assert.Equal(t, []string{"bar"}, AKAttrsToLDAP(u.Attributes)[0].Values)
+	assert.Equal(t, []string{"bar"}, AttributesToLDAP(u.Attributes, true)[0].Values)
 }
 
 func TestAKAttrsToLDAP_Dict(t *testing.T) {
@@ -56,9 +56,9 @@ func TestAKAttrsToLDAP_Dict(t *testing.T) {
 			"foo": "bar",
 		},
 	}
-	assert.Equal(t, 1, len(AKAttrsToLDAP(d)))
+	assert.Equal(t, 1, len(AttributesToLDAP(d, true)))
-	assert.Equal(t, "foo", AKAttrsToLDAP(d)[0].Name)
+	assert.Equal(t, "foo", AttributesToLDAP(d, true)[0].Name)
-	assert.Equal(t, []string{"map[foo:bar]"}, AKAttrsToLDAP(d)[0].Values)
+	assert.Equal(t, []string{"map[foo:bar]"}, AttributesToLDAP(d, true)[0].Values)
 }
 
 func TestAKAttrsToLDAP_Mixed(t *testing.T) {
@@ -69,7 +69,7 @@ func TestAKAttrsToLDAP_Mixed(t *testing.T) {
 			6,
 		},
 	}
-	assert.Equal(t, 1, len(AKAttrsToLDAP(d)))
+	assert.Equal(t, 1, len(AttributesToLDAP(d, true)))
-	assert.Equal(t, "foo", AKAttrsToLDAP(d)[0].Name)
+	assert.Equal(t, "foo", AttributesToLDAP(d, true)[0].Name)
-	assert.Equal(t, []string{"foo", "6"}, AKAttrsToLDAP(d)[0].Values)
+	assert.Equal(t, []string{"foo", "6"}, AttributesToLDAP(d, true)[0].Values)
 }

tests/e2e/test_provider_ldap.py

@@ -225,10 +225,16 @@ class TestProviderLDAP(SeleniumTestCase):
                         "uidNumber": [str(2000 + o_user.pk)],
                         "gidNumber": [str(2000 + o_user.pk)],
                         "memberOf": [],
+                        # Old fields for backwards compatibility
                         "goauthentik.io/ldap/active": ["true"],
                         "goauthentik.io/ldap/superuser": ["false"],
                         "goauthentik.io/user/override-ips": ["true"],
                         "goauthentik.io/user/service-account": ["true"],
+                        # End old fields
+                        "ak-active": ["true"],
+                        "ak-superuser": ["false"],
+                        "goauthentikio-user-override-ips": ["true"],
+                        "goauthentikio-user-service-account": ["true"],
                     },
                     "type": "searchResEntry",
                 },
@@ -250,10 +256,16 @@ class TestProviderLDAP(SeleniumTestCase):
                         "uidNumber": [str(2000 + embedded_account.pk)],
                         "gidNumber": [str(2000 + embedded_account.pk)],
                         "memberOf": [],
+                        # Old fields for backwards compatibility
                         "goauthentik.io/ldap/active": ["true"],
                         "goauthentik.io/ldap/superuser": ["false"],
                         "goauthentik.io/user/override-ips": ["true"],
                         "goauthentik.io/user/service-account": ["true"],
+                        # End old fields
+                        "ak-active": ["true"],
+                        "ak-superuser": ["false"],
+                        "goauthentikio-user-override-ips": ["true"],
+                        "goauthentikio-user-service-account": ["true"],
                     },
                     "type": "searchResEntry",
                 },
@@ -278,8 +290,12 @@ class TestProviderLDAP(SeleniumTestCase):
                             f"cn={group.name},ou=groups,dc=ldap,dc=goauthentik,dc=io"
                             for group in self.user.ak_groups.all()
                         ],
+                        # Old fields for backwards compatibility
                         "goauthentik.io/ldap/active": ["true"],
                         "goauthentik.io/ldap/superuser": ["true"],
+                        # End old fields
+                        "ak-active": ["true"],
+                        "ak-superuser": ["true"],
                         "extraAttribute": ["bar"],
                     },
                     "type": "searchResEntry",

website/docs/providers/ldap/index.md

@@ -29,8 +29,13 @@ The following fields are currently sent for users:
     -   "organizationalPerson"
     -   "goauthentik.io/ldap/user"
 -   `memberOf`: A list of all DNs that the user is a member of
--   `goauthentik.io/ldap/active`: "true" if the account is active, otherwise "false"
+-   `ak-active`: "true" if the account is active, otherwise "false"
--   `goauthentik.io/ldap/superuser`: "true" if the account is part of a group with superuser permissions, otherwise "false"
+-   `ak-superuser`: "true" if the account is part of a group with superuser permissions, otherwise "false"
+
+:::warning
+The use of the `goauthentik.io/ldap/active` and `goauthentik.io/ldap/superuser` attributes is deprecated as of authentik 2023.3. They will be removed completely in a future release.
+Use the replacements fields above instead.
+:::
 
 The following fields are current set for groups:
 
@@ -51,6 +56,10 @@ The virtual groups gidNumber is equal to the uidNumber of the user.
 Starting with 2021.9.1, custom attributes will override the inbuilt attributes.
 :::
 
+:::info
+Starting with 2023.3, periods and slashes in custom attributes will be sanitized.
+:::
+
 ## SSL
 
 You can also configure SSL for your LDAP Providers by selecting a certificate and a server name in the provider settings.