outposts/ldap: add more tests (#6188)

* outposts/ldap: add tests Signed-off-by: Jens Langhammer <[email protected]> * fix missing posixAccount Signed-off-by: Jens Langhammer <[email protected]> * attempt to expand attributes Signed-off-by: Jens Langhammer <[email protected]> * fix routing without base DN Signed-off-by: Jens Langhammer <[email protected]> * more logging Signed-off-by: Jens Langhammer <[email protected]> * remove our custom attribute filtering since this is done by the ldap library Signed-off-by: Jens Langhammer <[email protected]> * add test for schema Signed-off-by: Jens Langhammer <[email protected]> * fix tests Signed-off-by: Jens Langhammer <[email protected]> --------- Signed-off-by: Jens Langhammer <[email protected]>
goauthentik · Jul 9, 2023 · 9358218 · 9358218
1 parent 5fe7373
commit 9358218
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 56 deletions.
diff --git a/internal/outpost/ldap/entries.go b/internal/outpost/ldap/entries.go
@@ -40,11 +40,17 @@ func (pi *ProviderInstance) UserEntry(u api.User) *ldap.Entry {
 		"name":           {u.Name},
 		"displayName":    {u.Name},
 		"mail":           {*u.Email},
-		"objectClass":    {constants.OCUser, constants.OCOrgPerson, constants.OCInetOrgPerson, constants.OCAKUser},
-		"uidNumber":      {pi.GetUidNumber(u)},
-		"gidNumber":      {pi.GetUidNumber(u)},
-		"homeDirectory":  {fmt.Sprintf("/home/%s", u.Username)},
-		"sn":             {u.Name},
+		"objectClass": {
+			constants.OCUser,
+			constants.OCOrgPerson,
+			constants.OCInetOrgPerson,
+			constants.OCAKUser,
+			constants.OCPosixAccount,
+		},
+		"uidNumber":     {pi.GetUidNumber(u)},
+		"gidNumber":     {pi.GetUidNumber(u)},
+		"homeDirectory": {fmt.Sprintf("/home/%s", u.Username)},
+		"sn":            {u.Name},
 	})
 	return &ldap.Entry{DN: dn, Attributes: attrs}
 }
diff --git a/internal/outpost/ldap/entries_test.go b/internal/outpost/ldap/entries_test.go
@@ -0,0 +1,27 @@
+package ldap_test
+
+import (
+	"testing"
+
+	"beryju.io/ldap"
+	"github.com/stretchr/testify/assert"
+	"goauthentik.io/api/v3"
+)
+
+func Test_UserEntry(t *testing.T) {
+	pi := ProviderInstance()
+	u := api.User{
+		Username: "foo",
+		Name:     "bar",
+	}
+	entry := pi.UserEntry(u)
+	assert.Equal(t, "cn=foo,ou=users,dc=ldap,dc=goauthentik,dc=io", entry.DN)
+	assert.Contains(t, entry.Attributes, &ldap.EntryAttribute{
+		Name:   "cn",
+		Values: []string{u.Username},
+	})
+	assert.Contains(t, entry.Attributes, &ldap.EntryAttribute{
+		Name:   "displayName",
+		Values: []string{u.Name},
+	})
+}
diff --git a/internal/outpost/ldap/ldap_test.go b/internal/outpost/ldap/ldap_test.go
@@ -0,0 +1,31 @@
+package ldap_test
+
+import (
+	"testing"
+
+	"beryju.io/ldap"
+	"github.com/stretchr/testify/assert"
+	oldap "goauthentik.io/internal/outpost/ldap"
+)
+
+func ProviderInstance() *oldap.ProviderInstance {
+	return &oldap.ProviderInstance{
+		BaseDN:         "dc=ldap,dc=goauthentik,dc=io",
+		UserDN:         "ou=users,dc=ldap,dc=goauthentik,dc=io",
+		VirtualGroupDN: "ou=virtual-groups,dc=ldap,dc=goauthentik,dc=io",
+		GroupDN:        "ou=groups,dc=ldap,dc=goauthentik,dc=io",
+	}
+}
+
+func AssertLDAPAttributes(t *testing.T, attrs []*ldap.EntryAttribute, expected *ldap.EntryAttribute) {
+	found := false
+	for _, attr := range attrs {
+		if attr.Name == expected.Name {
+			assert.Equal(t, expected.Values, attr.Values)
+			found = true
+		}
+	}
+	if !found {
+		t.Fatalf("Key %s not found in ldap attributes", expected.Name)
+	}
+}
diff --git a/internal/outpost/ldap/search.go b/internal/outpost/ldap/search.go
@@ -22,7 +22,7 @@ func (ls *LDAPServer) Search(bindDN string, searchReq ldap.SearchRequest, conn n
 			"type":         "search",
 			"app":          selectedApp,
 		}).Observe(float64(span.EndTime.Sub(span.StartTime)))
-		req.Log().WithField("took-ms", span.EndTime.Sub(span.StartTime).Milliseconds()).Info("Search request")
+		req.Log().WithField("attributes", searchReq.Attributes).WithField("took-ms", span.EndTime.Sub(span.StartTime).Milliseconds()).Info("Search request")
 	}()
 
 	defer func() {
@@ -40,10 +40,7 @@ func (ls *LDAPServer) Search(bindDN string, searchReq ldap.SearchRequest, conn n
 	}
 	selectedApp = selectedProvider.GetAppSlug()
 	result, err := ls.searchRoute(req, selectedProvider)
-	if err != nil {
-		return result, nil
-	}
-	return ls.filterResultAttributes(req, result), nil
+	return result, err
 }
 
 func (ls *LDAPServer) fallbackRootDSE(req *search.Request) (ldap.ServerSearchResult, error) {

diff --git a/internal/outpost/ldap/search/direct/direct.go b/internal/outpost/ldap/search/direct/direct.go
@@ -87,7 +87,7 @@ func (ds *DirectSearcher) Search(req *search.Request) (ldap.ServerSearchResult,
 	c := api.NewAPIClient(ds.si.GetAPIClient().GetConfig())
 	c.GetConfig().AddDefaultHeader("X-authentik-outpost-ldap-query", req.Filter)
 
-	scope := req.SearchRequest.Scope
+	scope := req.Scope
 	needUsers, needGroups := ds.si.GetNeededObjects(scope, req.BaseDN, req.FilterObjectClass)
 
 	if scope >= 0 && strings.EqualFold(req.BaseDN, baseDN) {

diff --git a/internal/outpost/ldap/search/request.go b/internal/outpost/ldap/search/request.go
@@ -75,19 +75,3 @@ func (r *Request) Log() *log.Entry {
 func (r *Request) RemoteAddr() string {
 	return utils.GetIP(r.conn.RemoteAddr())
 }
-
-func (r *Request) FilterLDAPAttributes(res ldap.ServerSearchResult, cb func(attr *ldap.EntryAttribute) bool) ldap.ServerSearchResult {
-	for _, e := range res.Entries {
-		newAttrs := []*ldap.EntryAttribute{}
-		for _, attr := range e.Attributes {
-			include := cb(attr)
-			if include {
-				newAttrs = append(newAttrs, attr)
-			} else {
-				r.Log().WithField("key", attr.Name).Trace("filtering out field based on LDAP request")
-			}
-		}
-		e.Attributes = newAttrs
-	}
-	return res
-}
diff --git a/internal/outpost/ldap/search_route.go b/internal/outpost/ldap/search_route.go
@@ -53,32 +53,3 @@ func (ls *LDAPServer) searchRoute(req *search.Request, pi *ProviderInstance) (ld
 	req.Log().Trace("routing to default")
 	return pi.searcher.Search(req)
 }
-
-func (ls *LDAPServer) filterResultAttributes(req *search.Request, result ldap.ServerSearchResult) ldap.ServerSearchResult {
-	allowedAttributes := []string{}
-	if len(req.Attributes) == 1 && req.Attributes[0] == constants.SearchAttributeNone {
-		allowedAttributes = []string{"objectClass"}
-	}
-	if len(req.Attributes) > 0 {
-		// Only strictly filter allowed attributes if we haven't already narrowed the attributes
-		// down
-		if len(allowedAttributes) < 1 {
-			allowedAttributes = req.Attributes
-		}
-		// Filter LDAP returned attributes by search requested attributes, taking "1.1"
-		// into consideration
-		return req.FilterLDAPAttributes(result, func(attr *ldap.EntryAttribute) bool {
-			for _, allowed := range allowedAttributes {
-				if allowed == constants.SearchAttributeAllUser ||
-					allowed == constants.SearchAttributeAllOperational {
-					return true
-				}
-				if strings.EqualFold(allowed, attr.Name) {
-					return true
-				}
-			}
-			return false
-		})
-	}
-	return result
-}
diff --git a/tests/e2e/test_provider_ldap.py b/tests/e2e/test_provider_ldap.py
@@ -250,6 +250,7 @@ def test_ldap_bind_search(self):
                             "organizationalPerson",
                             "inetOrgPerson",
                             "goauthentik.io/ldap/user",
+                            "posixAccount",
                         ],
                         "uidNumber": 2000 + o_user.pk,
                         "gidNumber": 2000 + o_user.pk,
@@ -277,6 +278,7 @@ def test_ldap_bind_search(self):
                             "organizationalPerson",
                             "inetOrgPerson",
                             "goauthentik.io/ldap/user",
+                            "posixAccount",
                         ],
                         "uidNumber": 2000 + embedded_account.pk,
                         "gidNumber": 2000 + embedded_account.pk,
@@ -304,6 +306,7 @@ def test_ldap_bind_search(self):
                             "organizationalPerson",
                             "inetOrgPerson",
                             "goauthentik.io/ldap/user",
+                            "posixAccount",
                         ],
                         "uidNumber": 2000 + self.user.pk,
                         "gidNumber": 2000 + self.user.pk,
@@ -320,3 +323,24 @@ def test_ldap_bind_search(self):
                 },
             ],
         )
+
+    @retry()
+    @apply_blueprint(
+        "default/flow-default-authentication-flow.yaml",
+        "default/flow-default-invalidation-flow.yaml",
+    )
+    @reconcile_app("authentik_outposts")
+    def test_ldap_schema(self):
+        """Test LDAP Schema"""
+        self._prepare()
+        server = Server("ldap://localhost:3389", get_info=ALL)
+        _connection = Connection(
+            server,
+            raise_exceptions=True,
+            user=f"cn={self.user.username},ou=users,dc=ldap,dc=goauthentik,dc=io",
+            password=self.user.username,
+        )
+        _connection.bind()
+        self.assertIsNotNone(server.schema)
+        self.assertTrue(server.schema.is_valid())
+        self.assertIsNotNone(server.schema.object_classes["goauthentik.io/ldap/user"])