Allow using AD UUID as userId values

Active Directory treats UUID attributes like 'objectGUID' as octet string. This means they need some special treatment: - When storing them into UserId Object we need to convert the Raw Value to a string - When using them in LDAP filter they need to be correctly escaped as backslash escaped hex values. Partial Fix for #2523
cs3org · Feb 28, 2022 · 43eaf7a · 43eaf7a
1 parent 3e80be7
commit 43eaf7a
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 30 deletions.
diff --git a/changelog/unreleased/ldap-guid.md b/changelog/unreleased/ldap-guid.md
@@ -0,0 +1,7 @@
+Enhancement: Allow using AD UUID as userId values
+
+Active Directory UUID attributes (like e.g. objectGUID) use the LDAP octectString
+Syntax. In order to be able to use them as userids in reva, they need to be converted
+to their string representation.
+
+https://github.com/cs3org/reva/pull/2525
diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go
@@ -36,6 +36,7 @@ import (
 	"github.com/cs3org/reva/pkg/sharedconf"
 	"github.com/cs3org/reva/pkg/utils"
 	"github.com/go-ldap/ldap/v3"
+	"github.com/google/uuid"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 )
@@ -63,7 +64,8 @@ type attributes struct {
 	// DN is the distinguished name in ldap, e.g. `cn=einstein,ou=users,dc=example,dc=org`
 	DN string `mapstructure:"dn"`
 	// UID is an immutable user id, see https://docs.microsoft.com/en-us/azure/active-directory/hybrid/plan-connect-design-concepts
-	UID string `mapstructure:"uid"`
+	UID              string `mapstructure:"uid"`
+	UIDIsOctetString bool   `mapstructure:"uidIsOctetString"`
 	// CN is the username, typically `cn`, `uid` or `samaccountname`
 	CN string `mapstructure:"cn"`
 	// Mail is the email address of a user
@@ -78,13 +80,14 @@ type attributes struct {
 
 // Default attributes (Active Directory)
 var ldapDefaults = attributes{
-	DN:          "dn",
-	UID:         "ms-DS-ConsistencyGuid", // you can fall back to objectguid or even samaccountname but you will run into trouble when user names change. You have been warned.
-	CN:          "cn",
-	Mail:        "mail",
-	DisplayName: "displayName",
-	UIDNumber:   "uidNumber",
-	GIDNumber:   "gidNumber",
+	DN:               "dn",
+	UID:              "ms-DS-ConsistencyGuid", // you can fall back to objectguid or even samaccountname but you will run into trouble when user names change. You have been warned.
+	UIDIsOctetString: true,
+	CN:               "cn",
+	Mail:             "mail",
+	DisplayName:      "displayName",
+	UIDNumber:        "uidNumber",
+	GIDNumber:        "gidNumber",
 }
 
 func parseConfig(m map[string]interface{}) (*config, error) {
@@ -166,9 +169,19 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 		return nil, nil, err
 	}
 
+	var uid string
+	if am.c.Schema.UIDIsOctetString {
+		rawValue := sr.Entries[0].GetEqualFoldRawAttributeValue(am.c.Schema.UID)
+		if value, err := uuid.FromBytes(rawValue); err == nil {
+			uid = value.String()
+		}
+	} else {
+		uid = sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.UID)
+	}
+
 	userID := &user.UserId{
 		Idp:      am.c.Idp,
-		OpaqueId: sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.UID),
+		OpaqueId: uid,
 		Type:     user.UserType_USER_TYPE_PRIMARY, // TODO: assign the appropriate user type
 	}
 	gwc, err := pool.GetGatewayServiceClient(am.c.GatewaySvc)

diff --git a/pkg/user/manager/ldap/ldap.go b/pkg/user/manager/ldap/ldap.go
@@ -34,6 +34,7 @@ import (
 	"github.com/cs3org/reva/pkg/user/manager/registry"
 	"github.com/cs3org/reva/pkg/utils"
 	"github.com/go-ldap/ldap/v3"
+	"github.com/google/uuid"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 )
@@ -64,6 +65,9 @@ type attributes struct {
 	DN string `mapstructure:"dn"`
 	// UID is an immutable user id, see https://docs.microsoft.com/en-us/azure/active-directory/hybrid/plan-connect-design-concepts
 	UID string `mapstructure:"uid"`
+	// UIDIsOctetString set this to true if the values of the UID attribute are returned as OCTET STRING values (binary byte sequences)
+	// by the Directory Service. This is e.g. the case for the 'objectGUID' and	'ms-DS-ConsistencyGuid' Attributes in AD
+	UIDIsOctetString bool `mapstructure:"uidIsOctetString"`
 	// CN is the username, typically `cn`, `uid` or `samaccountname`
 	CN string `mapstructure:"cn"`
 	// Mail is the email address of a user
@@ -80,14 +84,15 @@ type attributes struct {
 
 // Default attributes (Active Directory)
 var ldapDefaults = attributes{
-	DN:          "dn",
-	UID:         "ms-DS-ConsistencyGuid", // you can fall back to objectguid or even samaccountname but you will run into trouble when user names change. You have been warned.
-	CN:          "cn",
-	Mail:        "mail",
-	DisplayName: "displayName",
-	UIDNumber:   "uidNumber",
-	GIDNumber:   "gidNumber",
-	GID:         "cn",
+	DN:               "dn",
+	UID:              "ms-DS-ConsistencyGuid", // you can fall back to objectguid or even samaccountname but you will run into trouble when user names change. You have been warned.
+	UIDIsOctetString: false,
+	CN:               "cn",
+	Mail:             "mail",
+	DisplayName:      "displayName",
+	UIDNumber:        "uidNumber",
+	GIDNumber:        "gidNumber",
+	GID:              "cn",
 }
 
 func parseConfig(m map[string]interface{}) (*config, error) {
@@ -157,10 +162,9 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User
 
 	log.Debug().Interface("entry", userEntry).Msg("entries")
 
-	id := &userpb.UserId{
-		Idp:      m.c.Idp,
-		OpaqueId: userEntry.GetEqualFoldAttributeValue(m.c.Schema.UID),
-		Type:     userpb.UserType_USER_TYPE_PRIMARY,
+	id, err := m.ldapEntryToUserID(userEntry)
+	if err != nil {
+		return nil, err
 	}
 
 	groups, err := m.getLDAPUserGroups(ctx, l, userEntry)
@@ -241,10 +245,9 @@ func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*use
 
 	log.Debug().Interface("entries", sr.Entries).Msg("entries")
 
-	id := &userpb.UserId{
-		Idp:      m.c.Idp,
-		OpaqueId: sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.UID),
-		Type:     userpb.UserType_USER_TYPE_PRIMARY,
+	id, err := m.ldapEntryToUserID(sr.Entries[0])
+	if err != nil {
+		return nil, err
 	}
 	groups, err := m.getLDAPUserGroups(ctx, l, sr.Entries[0])
 	if err != nil {
@@ -304,10 +307,9 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User,
 	users := []*userpb.User{}
 
 	for _, entry := range sr.Entries {
-		id := &userpb.UserId{
-			Idp:      m.c.Idp,
-			OpaqueId: entry.GetEqualFoldAttributeValue(m.c.Schema.UID),
-			Type:     userpb.UserType_USER_TYPE_PRIMARY,
+		id, err := m.ldapEntryToUserID(entry)
+		if err != nil {
+			return nil, err
 		}
 		groups, err := m.getLDAPUserGroups(ctx, l, entry)
 		if err != nil {
@@ -358,6 +360,26 @@ func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]stri
 	return m.getLDAPUserGroups(ctx, l, userEntry)
 }
 
+func (m *manager) ldapEntryToUserID(entry *ldap.Entry) (*userpb.UserId, error) {
+	var uid string
+	if m.c.Schema.UIDIsOctetString {
+		rawValue := entry.GetEqualFoldRawAttributeValue(m.c.Schema.UID)
+		if value, err := uuid.FromBytes(rawValue); err == nil {
+			uid = value.String()
+		} else {
+			return nil, err
+		}
+	} else {
+		uid = entry.GetEqualFoldAttributeValue(m.c.Schema.UID)
+	}
+
+	return &userpb.UserId{
+		Idp:      m.c.Idp,
+		OpaqueId: uid,
+		Type:     userpb.UserType_USER_TYPE_PRIMARY,
+	}, nil
+}
+
 func (m *manager) getLDAPUserByID(ctx context.Context, conn *ldap.Conn, uid *userpb.UserId) (*ldap.Entry, error) {
 	log := appctx.GetLogger(ctx)
 	// Search for the given clientID, use a sizeLimit of 1 to be able
@@ -414,14 +436,34 @@ func (m *manager) getLDAPUserGroups(ctx context.Context, conn *ldap.Conn, userEn
 }
 
 func (m *manager) getUserFilter(uid *userpb.UserId) string {
+	uidTmp := uid
+	if m.c.Schema.UIDIsOctetString {
+		uuid, err := uuid.Parse(uid.OpaqueId)
+		if err != nil {
+			err := errors.Wrap(err, fmt.Sprintf("error parsing OpaqueID '%s' as UUID", uid.OpaqueId))
+			panic(err)
+		}
+		escapedUID := *uid
+		escapedUID.OpaqueId = filterEscapeBinaryUUID(uuid)
+		uidTmp = &escapedUID
+	}
+
 	b := bytes.Buffer{}
-	if err := m.userfilter.Execute(&b, uid); err != nil {
+	if err := m.userfilter.Execute(&b, uidTmp); err != nil {
 		err := errors.Wrap(err, fmt.Sprintf("error executing user template: userid:%+v", uid))
 		panic(err)
 	}
 	return b.String()
 }
 
+func filterEscapeBinaryUUID(value uuid.UUID) string {
+	filtered := ""
+	for _, b := range value {
+		filtered = fmt.Sprintf("%s\\%02x", filtered, b)
+	}
+	return filtered
+}
+
 func (m *manager) getAttributeFilter(attribute, value string) string {
 	attr := strings.ReplaceAll(m.c.AttributeFilter, "{{attr}}", ldap.EscapeFilter(attribute))
 	return strings.ReplaceAll(attr, "{{value}}", ldap.EscapeFilter(value))