From eace09ca050c5885a8f6876ea7205484274c025e Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 8 May 2024 15:34:54 +0200 Subject: [PATCH] graph: Allow to set the identities property on users Previously we only allowed setting the "identities" property on education users. This changes move the related code to the main user object. --- services/graph/pkg/identity/ldap.go | 61 +++++++++++++++++-- .../pkg/identity/ldap_education_class_test.go | 2 +- .../graph/pkg/identity/ldap_education_user.go | 39 +----------- .../pkg/identity/ldap_education_user_test.go | 2 +- .../graph/pkg/identity/ldap_group_test.go | 16 ++--- services/graph/pkg/identity/ldap_test.go | 32 +++++----- 6 files changed, 87 insertions(+), 65 deletions(-) diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index efa2fd793..dea9f3a6d 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -21,8 +21,9 @@ import ( ) const ( - _givenNameAttribute = "givenname" - _surNameAttribute = "sn" + _givenNameAttribute = "givenname" + _surNameAttribute = "sn" + _identitiesAttribute = "oCExternalIdentity" ) // DisableUserMechanismType is used instead of directly using the string values from the configuration. @@ -81,6 +82,7 @@ type userAttributeMap struct { surname string accountEnabled string userType string + identities string } type ldapAttributeValues map[string][]string @@ -114,6 +116,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD givenName: _givenNameAttribute, surname: _surNameAttribute, userType: config.UserTypeAttribute, + identities: _identitiesAttribute, } if config.GroupNameAttribute == "" || config.GroupIDAttribute == "" { @@ -338,6 +341,19 @@ func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph. } } + if identities, ok := user.GetIdentitiesOk(); ok { + attrValues := make([]string, 0, len(identities)) + for _, identity := range identities { + identityStr, err := i.identityToLDAPAttrValue(identity) + if err != nil { + return nil, err + } + attrValues = append(attrValues, identityStr) + } + mr.Replace(i.userAttributeMap.identities, attrValues) + updateNeeded = true + } + // Behavior of enabling/disabling of users depends on the "disableUserMechanism" config option: // // "attribute": For the upstream user management service which modifies accountEnabled on the user entry @@ -395,6 +411,7 @@ func (i *LDAP) getUserByDN(dn string) (*ldap.Entry, error) { i.userAttributeMap.givenName, i.userAttributeMap.accountEnabled, i.userAttributeMap.userType, + i.userAttributeMap.identities, } filter := fmt.Sprintf("(objectClass=%s)", i.userObjectClass) @@ -519,9 +536,9 @@ func (i *LDAP) getLDAPUserByFilter(filter string) (*ldap.Entry, error) { i.userAttributeMap.userName, i.userAttributeMap.surname, i.userAttributeMap.givenName, - i.userAttributeMap.accountEnabled, i.userAttributeMap.userType, + i.userAttributeMap.identities, } return i.searchLDAPEntryByFilter(i.userBaseDN, attrs, filter) } @@ -601,6 +618,7 @@ func (i *LDAP) GetUsers(ctx context.Context, oreq *godata.GoDataRequest) ([]*lib i.userAttributeMap.givenName, i.userAttributeMap.accountEnabled, i.userAttributeMap.userType, + i.userAttributeMap.identities, }, nil, ) @@ -785,7 +803,7 @@ func (i *LDAP) createUserModelFromLDAP(e *ldap.Entry) *libregraph.User { surname := e.GetEqualFoldAttributeValue(i.userAttributeMap.surname) if id != "" && opsan != "" { - return &libregraph.User{ + user := &libregraph.User{ DisplayName: pointerOrNil(e.GetEqualFoldAttributeValue(i.userAttributeMap.displayName)), Mail: pointerOrNil(e.GetEqualFoldAttributeValue(i.userAttributeMap.mail)), OnPremisesSamAccountName: &opsan, @@ -795,6 +813,18 @@ func (i *LDAP) createUserModelFromLDAP(e *ldap.Entry) *libregraph.User { UserType: pointerOrNil(e.GetEqualFoldAttributeValue(i.userAttributeMap.userType)), AccountEnabled: booleanOrNil(e.GetEqualFoldAttributeValue(i.userAttributeMap.accountEnabled)), } + var identities []libregraph.ObjectIdentity + for _, identityStr := range e.GetEqualFoldAttributeValues(i.userAttributeMap.identities) { + parts := strings.SplitN(identityStr, "$", 3) + identity := libregraph.NewObjectIdentity() + identity.SetIssuer(strings.TrimSpace(parts[1])) + identity.SetIssuerAssignedId(strings.TrimSpace(parts[2])) + identities = append(identities, *identity) + } + if len(identities) > 0 { + user.SetIdentities(identities) + } + return user } i.logger.Warn().Str("dn", e.DN).Msg("Invalid User. Missing username or id attribute") return nil @@ -810,6 +840,19 @@ func (i *LDAP) userToLDAPAttrValues(user libregraph.User) (map[string][]string, i.userAttributeMap.userType: {user.GetUserType()}, } + if identities, ok := user.GetIdentitiesOk(); ok { + for _, identity := range identities { + identityStr, err := i.identityToLDAPAttrValue(identity) + if err != nil { + return nil, err + } + attrs[i.userAttributeMap.identities] = append( + attrs[i.userAttributeMap.identities], + identityStr, + ) + } + } + if !i.useServerUUID { attrs["owncloudUUID"] = []string{uuid.New().String()} } @@ -844,6 +887,15 @@ func (i *LDAP) userToLDAPAttrValues(user libregraph.User) (map[string][]string, return attrs, nil } +func (i *LDAP) identityToLDAPAttrValue(identity libregraph.ObjectIdentity) (string, error) { + // TODO add support for the "signInType" of objectIdentity + if identity.GetIssuer() == "" || identity.GetIssuerAssignedId() == "" { + return "", fmt.Errorf("missing Attribute for objectIdentity") + } + identityStr := fmt.Sprintf(" $ %s $ %s", identity.GetIssuer(), identity.GetIssuerAssignedId()) + return identityStr, nil +} + func (i *LDAP) getUserAttrTypes() []string { return []string{ i.userAttributeMap.displayName, @@ -857,6 +909,7 @@ func (i *LDAP) getUserAttrTypes() []string { "userPassword", i.userAttributeMap.accountEnabled, i.userAttributeMap.userType, + i.userAttributeMap.identities, } } diff --git a/services/graph/pkg/identity/ldap_education_class_test.go b/services/graph/pkg/identity/ldap_education_class_test.go index 2b9cc199a..1d5bf77e9 100644 --- a/services/graph/pkg/identity/ldap_education_class_test.go +++ b/services/graph/pkg/identity/ldap_education_class_test.go @@ -273,7 +273,7 @@ func TestGetEducationClassMembers(t *testing.T) { Scope: 0, SizeLimit: 1, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), } lm.On("Search", userSr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{userEntry}}, nil) diff --git a/services/graph/pkg/identity/ldap_education_user.go b/services/graph/pkg/identity/ldap_education_user.go index 1ea76ca4e..02814a1c0 100644 --- a/services/graph/pkg/identity/ldap_education_user.go +++ b/services/graph/pkg/identity/ldap_education_user.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "strings" "github.com/go-ldap/ldap/v3" libregraph "github.com/owncloud/libre-graph-api-go" @@ -12,13 +11,11 @@ import ( ) type educationUserAttributeMap struct { - identities string primaryRole string } func newEducationUserAttributeMap() educationUserAttributeMap { return educationUserAttributeMap{ - identities: "oCExternalIdentity", primaryRole: "userClass", } } @@ -164,7 +161,7 @@ func (i *LDAP) UpdateEducationUser(ctx context.Context, nameOrID string, user li } attrValues = append(attrValues, identityStr) } - mr.Replace(i.educationConfig.userAttributeMap.identities, attrValues) + mr.Replace(i.userAttributeMap.identities, attrValues) updateNeeded = true } @@ -261,6 +258,7 @@ func (i *LDAP) educationUserToUser(eduUser libregraph.EducationUser) *libregraph user.DisplayName = eduUser.DisplayName user.Mail = eduUser.Mail user.UserType = eduUser.UserType + user.Identities = eduUser.Identities return user } @@ -281,17 +279,6 @@ func (i *LDAP) userToEducationUser(user libregraph.User, e *ldap.Entry) *libregr if primaryRole := e.GetEqualFoldAttributeValue(i.educationConfig.userAttributeMap.primaryRole); primaryRole != "" { eduUser.SetPrimaryRole(primaryRole) } - var identities []libregraph.ObjectIdentity - for _, identityStr := range e.GetEqualFoldAttributeValues(i.educationConfig.userAttributeMap.identities) { - parts := strings.SplitN(identityStr, "$", 3) - identity := libregraph.NewObjectIdentity() - identity.SetIssuer(strings.TrimSpace(parts[1])) - identity.SetIssuerAssignedId(strings.TrimSpace(parts[2])) - identities = append(identities, *identity) - } - if len(identities) > 0 { - eduUser.SetIdentities(identities) - } } return eduUser @@ -301,29 +288,9 @@ func (i *LDAP) educationUserToLDAPAttrValues(user libregraph.EducationUser, attr if role, ok := user.GetPrimaryRoleOk(); ok { attrs[i.educationConfig.userAttributeMap.primaryRole] = []string{*role} } - if identities, ok := user.GetIdentitiesOk(); ok { - for _, identity := range identities { - identityStr, err := i.identityToLDAPAttrValue(identity) - if err != nil { - return nil, err - } - attrs[i.educationConfig.userAttributeMap.identities] = append( - attrs[i.educationConfig.userAttributeMap.identities], - identityStr, - ) - } - } attrs["objectClass"] = append(attrs["objectClass"], i.educationConfig.userObjectClass) return attrs, nil } -func (i *LDAP) identityToLDAPAttrValue(identity libregraph.ObjectIdentity) (string, error) { - // TODO add support for the "signInType" of objectIdentity - if identity.GetIssuer() == "" || identity.GetIssuerAssignedId() == "" { - return "", fmt.Errorf("missing Attribute for objectIdentity") - } - identityStr := fmt.Sprintf(" $ %s $ %s", identity.GetIssuer(), identity.GetIssuerAssignedId()) - return identityStr, nil -} func (i *LDAP) educationUserToAddRequest(user libregraph.EducationUser) (*ldap.AddRequest, error) { plainUser := i.educationUserToUser(user) @@ -359,7 +326,7 @@ func (i *LDAP) getEducationUserAttrTypes() []string { i.userAttributeMap.givenName, i.userAttributeMap.accountEnabled, i.userAttributeMap.userType, - i.educationConfig.userAttributeMap.identities, + i.userAttributeMap.identities, i.educationConfig.userAttributeMap.primaryRole, i.educationConfig.memberOfSchoolAttribute, } diff --git a/services/graph/pkg/identity/ldap_education_user_test.go b/services/graph/pkg/identity/ldap_education_user_test.go index 3662f8919..a84a1df28 100644 --- a/services/graph/pkg/identity/ldap_education_user_test.go +++ b/services/graph/pkg/identity/ldap_education_user_test.go @@ -194,7 +194,7 @@ func TestUpdateEducationUser(t *testing.T) { Scope: 0, SizeLimit: 1, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, } eduUserLookupReq := &ldap.SearchRequest{ BaseDN: "uid=newtestuser,ou=people,dc=test", diff --git a/services/graph/pkg/identity/ldap_group_test.go b/services/graph/pkg/identity/ldap_group_test.go index b228a16bb..a7efaf955 100644 --- a/services/graph/pkg/identity/ldap_group_test.go +++ b/services/graph/pkg/identity/ldap_group_test.go @@ -83,11 +83,11 @@ func TestGetGroup(t *testing.T) { Entries: []*ldap.Entry{invalidGroupEntry}, }, nil) b, _ = getMockedBackend(lm, lconfig, &logger) - g, err := b.GetGroup(context.Background(), "group", nil) + _, err = b.GetGroup(context.Background(), "group", nil) assert.ErrorContains(t, err, "itemNotFound:") - g, err = b.GetGroup(context.Background(), "group", queryParamExpand) + _, err = b.GetGroup(context.Background(), "group", queryParamExpand) assert.ErrorContains(t, err, "itemNotFound:") - g, err = b.GetGroup(context.Background(), "group", queryParamSelect) + _, err = b.GetGroup(context.Background(), "group", queryParamSelect) assert.ErrorContains(t, err, "itemNotFound:") // Mock a valid Search Result @@ -96,14 +96,14 @@ func TestGetGroup(t *testing.T) { BaseDN: "uid=user,ou=people,dc=test", SizeLimit: 1, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), } sr3 := &ldap.SearchRequest{ BaseDN: "uid=invalid,ou=people,dc=test", SizeLimit: 1, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), } @@ -111,7 +111,7 @@ func TestGetGroup(t *testing.T) { lm.On("Search", sr2).Return(&ldap.SearchResult{Entries: []*ldap.Entry{userEntry}}, nil) lm.On("Search", sr3).Return(&ldap.SearchResult{Entries: []*ldap.Entry{invalidUserEntry}}, nil) b, _ = getMockedBackend(lm, lconfig, &logger) - g, err = b.GetGroup(context.Background(), "group", nil) + g, err := b.GetGroup(context.Background(), "group", nil) if err != nil { t.Errorf("Expected GetGroup to succeed. Got %s", err.Error()) } else if *g.Id != groupEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id) { @@ -257,14 +257,14 @@ func TestGetGroups(t *testing.T) { BaseDN: "uid=user,ou=people,dc=test", SizeLimit: 1, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), } sr3 := &ldap.SearchRequest{ BaseDN: "uid=invalid,ou=people,dc=test", SizeLimit: 1, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), } diff --git a/services/graph/pkg/identity/ldap_test.go b/services/graph/pkg/identity/ldap_test.go index 71431c674..36630cbf7 100644 --- a/services/graph/pkg/identity/ldap_test.go +++ b/services/graph/pkg/identity/ldap_test.go @@ -72,6 +72,8 @@ var invalidUserEntry = ldap.NewEntry("uid=user", var logger = log.NewLogger(log.Level("debug")) +var ldapUserAttributes = []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute", "oCExternalIdentity"} + func TestNewLDAPBackend(t *testing.T) { l := &mocks.Client{} @@ -364,7 +366,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -408,7 +410,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -452,7 +454,7 @@ func TestUpdateUser(t *testing.T) { TimeLimit: 0, TypesOnly: false, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), }, }, @@ -537,7 +539,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -581,7 +583,7 @@ func TestUpdateUser(t *testing.T) { TimeLimit: 0, TypesOnly: false, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), }, }, @@ -666,7 +668,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -762,7 +764,7 @@ func TestUpdateUser(t *testing.T) { TimeLimit: 0, TypesOnly: false, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), }, }, @@ -855,7 +857,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -899,7 +901,7 @@ func TestUpdateUser(t *testing.T) { TimeLimit: 0, TypesOnly: false, Filter: "(objectClass=inetOrgPerson)", - Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + Attributes: ldapUserAttributes, Controls: []ldap.Control(nil), }, }, @@ -985,7 +987,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -1088,7 +1090,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeBaseObject, ldap.NeverDerefAliases, 1, 0, false, "(objectClass=inetOrgPerson)", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, []ldap.Control(nil), ), }, @@ -1151,7 +1153,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -1254,7 +1256,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeBaseObject, ldap.NeverDerefAliases, 1, 0, false, "(objectClass=inetOrgPerson)", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, []ldap.Control(nil), ), }, @@ -1317,7 +1319,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 1, 0, false, "(&(objectClass=inetOrgPerson)(|(uid=testUser)(entryUUID=testUser)))", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, nil, ), }, @@ -1388,7 +1390,7 @@ func TestUpdateUser(t *testing.T) { ldap.ScopeBaseObject, ldap.NeverDerefAliases, 1, 0, false, "(objectClass=inetOrgPerson)", - []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname", "userEnabledAttribute", "userTypeAttribute"}, + ldapUserAttributes, []ldap.Control(nil), ), },