From b0cd8779a7212886904d71db58e331914f0b8902 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dani=C3=ABl=20Franke?= <df@ponc.tech>
Date: Thu, 12 Jan 2023 17:25:43 +0100
Subject: [PATCH] Consistently add user attributes to AddRequest (#5392)

To make the `CreateUser` flow easier to test, the attributes of the
LDAP add request need to be added consistently. This way we can
expect that argument with the mock.

This PR does the following:

* Add a method `getUserAttrTypes` that returns a static list of attr names.
* Alter `userToAddRequest` to iterate over the static list, and add the attr
  if it exists in the `attrMap`.

Closes #5390
---
 services/graph/pkg/identity/ldap.go      | 21 +++++++++++++++++++--
 services/graph/pkg/identity/ldap_test.go | 24 +++++++++++++++++-------
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go
index 735f4ee6a4..460628ab6d 100644
--- a/services/graph/pkg/identity/ldap.go
+++ b/services/graph/pkg/identity/ldap.go
@@ -1026,6 +1026,20 @@ func (i *LDAP) userToLDAPAttrValues(user libregraph.User) (map[string][]string,
 	return attrs, nil
 }
 
+func (i *LDAP) getUserAttrTypes() []string {
+	return []string{
+		i.userAttributeMap.displayName,
+		i.userAttributeMap.userName,
+		i.userAttributeMap.mail,
+		i.userAttributeMap.surname,
+		i.userAttributeMap.givenName,
+		"objectClass",
+		"cn",
+		"owncloudUUID",
+		"userPassword",
+	}
+}
+
 func (i *LDAP) getUserLDAPDN(user libregraph.User) string {
 	return fmt.Sprintf("uid=%s,%s", oldap.EscapeDNAttributeValue(*user.OnPremisesSamAccountName), i.userBaseDN)
 }
@@ -1037,8 +1051,11 @@ func (i *LDAP) userToAddRequest(user libregraph.User) (*ldap.AddRequest, error)
 	if err != nil {
 		return nil, err
 	}
-	for attrType, values := range attrMap {
-		ar.Attribute(attrType, values)
+
+	for _, attrType := range i.getUserAttrTypes() {
+		if values, ok := attrMap[attrType]; ok {
+			ar.Attribute(attrType, values)
+		}
 	}
 	return ar, nil
 }
diff --git a/services/graph/pkg/identity/ldap_test.go b/services/graph/pkg/identity/ldap_test.go
index e36466d17e..92e7c156e5 100644
--- a/services/graph/pkg/identity/ldap_test.go
+++ b/services/graph/pkg/identity/ldap_test.go
@@ -3,6 +3,7 @@ package identity
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/url"
 	"testing"
 
@@ -106,6 +107,21 @@ func TestNewLDAPBackend(t *testing.T) {
 }
 
 func TestCreateUser(t *testing.T) {
+	displayName := "DisplayName"
+	mail := "user@example"
+	userName := "user"
+	surname := "surname"
+	givenName := "givenName"
+
+	ar := ldap.NewAddRequest(fmt.Sprintf("uid=user,%s", lconfig.UserBaseDN), nil)
+	ar.Attribute(lconfig.UserDisplayNameAttribute, []string{displayName})
+	ar.Attribute(lconfig.UserNameAttribute, []string{userName})
+	ar.Attribute(lconfig.UserEmailAttribute, []string{mail})
+	ar.Attribute("sn", []string{surname})
+	ar.Attribute("givenname", []string{givenName})
+	ar.Attribute("objectClass", []string{"inetOrgPerson", "organizationalPerson", "person", "top"})
+	ar.Attribute("cn", []string{userName})
+
 	l := &mocks.Client{}
 	l.On("Search", mock.Anything).
 		Return(
@@ -113,15 +129,9 @@ func TestCreateUser(t *testing.T) {
 				Entries: []*ldap.Entry{userEntry},
 			},
 			nil)
-	l.On("Add", mock.Anything).Return(nil)
+	l.On("Add", ar).Return(nil)
 	logger := log.NewLogger(log.Level("debug"))
 
-	displayName := "DisplayName"
-	mail := "user@example"
-	userName := "user"
-	surname := "surname"
-	givenName := "givenName"
-
 	user := libregraph.NewUser()
 	user.SetDisplayName(displayName)
 	user.SetMail(mail)