From 48422e467a46dbb7795d734c92766646cf21b089 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 30 Mar 2023 17:22:58 +0200 Subject: [PATCH 1/2] Bump libre-graph-go --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 1f536fcab1c..aa517a0a1de 100644 --- a/go.mod +++ b/go.mod @@ -67,7 +67,7 @@ require ( github.com/onsi/gomega v1.27.4 github.com/open-policy-agent/opa v0.50.2 github.com/orcaman/concurrent-map v1.0.0 - github.com/owncloud/libre-graph-api-go v1.0.2-0.20230309112802-ff71ba8c90aa + github.com/owncloud/libre-graph-api-go v1.0.2-0.20230330145712-ea267ccd404a github.com/pkg/errors v0.9.1 github.com/pkg/xattr v0.4.9 github.com/prometheus/client_golang v1.14.0 diff --git a/go.sum b/go.sum index 11485ca34dc..b13914f9025 100644 --- a/go.sum +++ b/go.sum @@ -1373,8 +1373,8 @@ github.com/oracle/oci-go-sdk v24.3.0+incompatible/go.mod h1:VQb79nF8Z2cwLkLS35uk github.com/orcaman/concurrent-map v1.0.0 h1:I/2A2XPCb4IuQWcQhBhSwGfiuybl/J0ev9HDbW65HOY= github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CFcDWnWD9XkenwhI= github.com/ovh/go-ovh v1.1.0/go.mod h1:AxitLZ5HBRPyUd+Zl60Ajaag+rNTdVXWIkzfrVuTXWA= -github.com/owncloud/libre-graph-api-go v1.0.2-0.20230309112802-ff71ba8c90aa h1:Lab1HHZ7Z8cBjojLDSfGYbGSZT08iww25uTW+EV1Sao= -github.com/owncloud/libre-graph-api-go v1.0.2-0.20230309112802-ff71ba8c90aa/go.mod h1:iKdVH6nYpI8RBeK9sjeLfzrPByST6r9d+NG2IJHoJmU= +github.com/owncloud/libre-graph-api-go v1.0.2-0.20230330145712-ea267ccd404a h1:C7YCoyXn/8pkapUhw2KoIxEMdgIFUx3JjZQKtsXaaLc= +github.com/owncloud/libre-graph-api-go v1.0.2-0.20230330145712-ea267ccd404a/go.mod h1:iKdVH6nYpI8RBeK9sjeLfzrPByST6r9d+NG2IJHoJmU= github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c h1:rp5dCmg/yLR3mgFuSOe4oEnDDmGLROTvMragMUXpTQw= github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c/go.mod h1:X07ZCGwUbLaax7L0S3Tw4hpejzu63ZrrQiUe6W0hcy0= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= From 3a475843d4ac3333473eca33a6b0b5bc18ad4503 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 30 Mar 2023 17:31:14 +0200 Subject: [PATCH 2/2] graph: new config option GRAPH_LDAP_GROUP_CREATE_BASE_DN By setting GRAPH_LDAP_GROUP_CREATE_BASE_DN a distinct subtree can be configured where new LDAP groups are created. That subtree needs to be subordinate to GRAPH_LDAP_GROUP_BASE_DN. All groups outside for GRAPH_LDAP_GROUP_CREATE_BASE_DN are considered read-only and only groups below that DN can be updated and deleted. This is introduced for a pretty specific usecase where most groups are managed in an external source (e.g. a read-only replica of an LDAP tree). But we still want to allow the local administrator to create groups in a writeable subtree attached to that replica. --- .../enhancement-ldap-group-create-basedn.md | 13 ++ services/graph/pkg/config/config.go | 1 + .../pkg/config/defaults/defaultconfig.go | 4 + services/graph/pkg/config/parser/parse.go | 30 +++- services/graph/pkg/identity/ldap.go | 2 + services/graph/pkg/identity/ldap_group.go | 61 ++++++++- .../graph/pkg/identity/ldap_group_test.go | 128 ++++++++++++++---- .../pkg/service/v0/errorcode/errorcode.go | 2 + services/graph/pkg/service/v0/groups.go | 9 +- 9 files changed, 215 insertions(+), 35 deletions(-) create mode 100644 changelog/unreleased/enhancement-ldap-group-create-basedn.md diff --git a/changelog/unreleased/enhancement-ldap-group-create-basedn.md b/changelog/unreleased/enhancement-ldap-group-create-basedn.md new file mode 100644 index 00000000000..cfe791665ed --- /dev/null +++ b/changelog/unreleased/enhancement-ldap-group-create-basedn.md @@ -0,0 +1,13 @@ +Enhancement: Make the LDAP base DN for new groups configurable + +The LDAP backend for the Graph service introduced a new config option for setting the +Parent DN for new groups created via the `/groups/` endpoint. (`GRAPH_LDAP_GROUP_CREATE_BASE_DN`) + +It defaults to the value of `GRAPH_LDAP_GROUP_BASE_DN`. If set to a different value the +`GRAPH_LDAP_GROUP_CREATE_BASE_DN` needs to be a subordinate DN of `GRAPH_LDAP_GROUP_BASE_DN`. + +All existing groups with a DN outside the `GRAPH_LDAP_GROUP_CREATE_BASE_DN` tree will be treated as +read-only groups. So it is not possible to edit these groups. + +https://github.com/owncloud/ocis/pull/5974 + diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 15b1d966cc5..10a4b9762dc 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -69,6 +69,7 @@ type LDAP struct { LdapDisabledUsersGroupDN string `yaml:"ldap_disabled_users_group_dn" env:"LDAP_DISABLED_USERS_GROUP_DN;GRAPH_DISABLED_USERS_GROUP_DN" desc:"The distinguished name of the group to which added users will be classified as disabled when 'disable_user_mechanism' is set to 'group'."` GroupBaseDN string `yaml:"group_base_dn" env:"LDAP_GROUP_BASE_DN;GRAPH_LDAP_GROUP_BASE_DN" desc:"Search base DN for looking up LDAP groups."` + GroupCreateBaseDN string `yaml:"group_create_base_dn" env:"GRAPH_LDAP_GROUP_CREATE_BASE_DN" desc:"Parent DN under which new groups are created. This DN needs to be subordinate to the 'GRAPH_LDAP_GROUP_BASE_DN'. This setting is only relevant when 'GRAPH_LDAP_SERVER_WRITE_ENABLED' is 'true'. It defaults to the value of 'GRAPH_LDAP_GROUP_BASE_DN'. All groups outside of this subtree are treated as readonly groups and cannot be updated."` GroupSearchScope string `yaml:"group_search_scope" env:"LDAP_GROUP_SCOPE;GRAPH_LDAP_GROUP_SEARCH_SCOPE" desc:"LDAP search scope to use when looking up groups. Supported scopes are 'base', 'one' and 'sub'."` GroupFilter string `yaml:"group_filter" env:"LDAP_GROUP_FILTER;GRAPH_LDAP_GROUP_FILTER" desc:"LDAP filter to add to the default filters for group searches."` GroupObjectClass string `yaml:"group_objectclass" env:"LDAP_GROUP_OBJECTCLASS;GRAPH_LDAP_GROUP_OBJECTCLASS" desc:"The object class to use for groups in the default group search filter ('groupOfNames'). "` diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index ae9bfab02f2..eb016c4310a 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -153,6 +153,10 @@ func EnsureDefaults(cfg *config.Config) { if cfg.MachineAuthAPIKey == "" && cfg.Commons != nil && cfg.Commons.MachineAuthAPIKey != "" { cfg.MachineAuthAPIKey = cfg.Commons.MachineAuthAPIKey } + + if cfg.Identity.LDAP.GroupCreateBaseDN == "" { + cfg.Identity.LDAP.GroupCreateBaseDN = cfg.Identity.LDAP.GroupBaseDN + } } // Sanitize sanitized the configuration diff --git a/services/graph/pkg/config/parser/parse.go b/services/graph/pkg/config/parser/parse.go index 6f48d5c4833..893870f4543 100644 --- a/services/graph/pkg/config/parser/parse.go +++ b/services/graph/pkg/config/parser/parse.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" + "github.com/go-ldap/ldap/v3" ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config" defaults2 "github.com/owncloud/ocis/v2/ocis-pkg/config/defaults" "github.com/owncloud/ocis/v2/ocis-pkg/shared" @@ -40,8 +41,10 @@ func Validate(cfg *config.Config) error { return shared.MissingJWTTokenError(cfg.Service.Name) } - if cfg.Identity.Backend == "ldap" && cfg.Identity.LDAP.BindPassword == "" { - return shared.MissingLDAPBindPassword(cfg.Service.Name) + if cfg.Identity.Backend == "ldap" { + if err := validateLDAPSettings(cfg); err != nil { + return err + } } if cfg.Application.ID == "" { @@ -64,3 +67,26 @@ func Validate(cfg *config.Config) error { return nil } + +func validateLDAPSettings(cfg *config.Config) error { + if cfg.Identity.LDAP.BindPassword == "" { + return shared.MissingLDAPBindPassword(cfg.Service.Name) + } + + // ensure that "GroupBaseDN" is below "GroupBaseDN" + if cfg.Identity.LDAP.WriteEnabled && cfg.Identity.LDAP.GroupCreateBaseDN != cfg.Identity.LDAP.GroupBaseDN { + baseDN, err := ldap.ParseDN(cfg.Identity.LDAP.GroupBaseDN) + if err != nil { + return fmt.Errorf("Unable to parse the LDAP Group Base DN '%s': %w ", cfg.Identity.LDAP.GroupBaseDN, err) + } + createBaseDN, err := ldap.ParseDN(cfg.Identity.LDAP.GroupCreateBaseDN) + if err != nil { + return fmt.Errorf("Unable to parse the LDAP Group Create Base DN '%s': %w ", cfg.Identity.LDAP.GroupCreateBaseDN, err) + } + + if !baseDN.AncestorOfFold(createBaseDN) { + return fmt.Errorf("The LDAP Group Create Base DN (%s) must be subordinate to the LDAP Group Base DN (%s)", cfg.Identity.LDAP.GroupCreateBaseDN, cfg.Identity.LDAP.GroupBaseDN) + } + } + return nil +} diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index 696d1647772..e784c3f2ffd 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -58,6 +58,7 @@ type LDAP struct { localUserDisableGroupDN string groupBaseDN string + groupCreateBaseDN string groupFilter string groupObjectClass string groupScope int @@ -148,6 +149,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD userScope: userScope, userAttributeMap: uam, groupBaseDN: config.GroupBaseDN, + groupCreateBaseDN: config.GroupCreateBaseDN, groupFilter: config.GroupFilter, groupObjectClass: config.GroupObjectClass, groupScope: groupScope, diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index 820d44804ee..b3a2472f9a8 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -204,10 +204,16 @@ func (i *LDAP) DeleteGroup(ctx context.Context, id string) error { if !i.writeEnabled { return errorcode.New(errorcode.NotAllowed, "server is configured read-only") } + e, err := i.getLDAPGroupByID(id, false) if err != nil { return err } + + if i.isLDAPGroupReadOnly(e) { + return errorcode.New(errorcode.NotAllowed, "group is read-only") + } + dr := ldap.DelRequest{DN: e.DN} if err = i.conn.Del(&dr); err != nil { return err @@ -219,12 +225,19 @@ func (i *LDAP) DeleteGroup(ctx context.Context, id string) error { func (i *LDAP) UpdateGroupName(ctx context.Context, groupID string, groupName string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("AddMembersToGroup") + if !i.writeEnabled { + return errorcode.New(errorcode.NotAllowed, "server is configured read-only") + } ge, err := i.getLDAPGroupByID(groupID, true) if err != nil { return err } + if i.isLDAPGroupReadOnly(ge) { + return errorcode.New(errorcode.NotAllowed, "group is read-only") + } + if ge.GetEqualFoldAttributeValue(i.groupAttributeMap.name) == groupName { return nil } @@ -258,11 +271,18 @@ func (i *LDAP) UpdateGroupName(ctx context.Context, groupID string, groupName st func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs []string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("AddMembersToGroup") + if !i.writeEnabled { + return errorcode.New(errorcode.NotAllowed, "server is configured read-only") + } ge, err := i.getLDAPGroupByNameOrID(groupID, true) if err != nil { return err } + if i.isLDAPGroupReadOnly(ge) { + return errorcode.New(errorcode.NotAllowed, "group is read-only") + } + mr := ldap.ModifyRequest{DN: ge.DN} // Handle empty groups (using the empty member attribute) current := ge.GetEqualFoldAttributeValues(i.groupAttributeMap.member) @@ -326,11 +346,20 @@ func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("RemoveMemberFromGroup") + if !i.writeEnabled { + return errorcode.New(errorcode.NotAllowed, "server is configured read-only") + } + ge, err := i.getLDAPGroupByID(groupID, true) if err != nil { logger.Debug().Str("backend", "ldap").Str("groupID", groupID).Msg("Error looking up group") return err } + + if i.isLDAPGroupReadOnly(ge) { + return errorcode.New(errorcode.NotAllowed, "group is read-only") + } + me, err := i.getLDAPUserByID(memberID) if err != nil { logger.Debug().Str("backend", "ldap").Str("memberID", memberID).Msg("Error looking up group member") @@ -345,7 +374,7 @@ func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, member } func (i *LDAP) groupToAddRequest(group libregraph.Group) (*ldap.AddRequest, error) { - ar := ldap.NewAddRequest(i.getGroupLDAPDN(group), nil) + ar := ldap.NewAddRequest(i.getGroupCreateLDAPDN(group), nil) attrMap, err := i.groupToLDAPAttrValues(group) if err != nil { @@ -357,12 +386,12 @@ func (i *LDAP) groupToAddRequest(group libregraph.Group) (*ldap.AddRequest, erro return ar, nil } -func (i *LDAP) getGroupLDAPDN(group libregraph.Group) string { +func (i *LDAP) getGroupCreateLDAPDN(group libregraph.Group) string { attributeTypeAndValue := ldap.AttributeTypeAndValue{ Type: "cn", Value: group.GetDisplayName(), } - return fmt.Sprintf("%s,%s", attributeTypeAndValue.String(), i.groupBaseDN) + return fmt.Sprintf("%s,%s", attributeTypeAndValue.String(), i.groupCreateBaseDN) } func (i *LDAP) groupToLDAPAttrValues(group libregraph.Group) (map[string][]string, error) { @@ -482,17 +511,43 @@ func (i *LDAP) getGroupsForUser(dn string) ([]*ldap.Entry, error) { func (i *LDAP) createGroupModelFromLDAP(e *ldap.Entry) *libregraph.Group { name := e.GetEqualFoldAttributeValue(i.groupAttributeMap.name) id := e.GetEqualFoldAttributeValue(i.groupAttributeMap.id) + groupTypes := []string{} + + if i.isLDAPGroupReadOnly(e) { + groupTypes = []string{"ReadOnly"} + } if id != "" && name != "" { return &libregraph.Group{ DisplayName: &name, Id: &id, + GroupTypes: groupTypes, } } i.logger.Warn().Str("dn", e.DN).Msg("Group is missing name or id") return nil } +func (i *LDAP) isLDAPGroupReadOnly(e *ldap.Entry) bool { + if !i.writeEnabled { + return true + } + + groupDN, err := ldap.ParseDN(e.DN) + if err != nil { + i.logger.Warn().Err(err).Str("dn", e.DN).Msg("Failed to parse DN") + return false + } + + baseDN, err := ldap.ParseDN(i.groupCreateBaseDN) + if err != nil { + i.logger.Warn().Err(err).Str("dn", i.groupCreateBaseDN).Msg("Failed to parse DN") + return false + } + + return !baseDN.AncestorOfFold(groupDN) +} + func (i *LDAP) groupsFromLDAPEntries(e []*ldap.Entry) []libregraph.Group { groups := make([]libregraph.Group, 0, len(e)) for _, g := range e { diff --git a/services/graph/pkg/identity/ldap_group_test.go b/services/graph/pkg/identity/ldap_group_test.go index 585b5e65af6..f3557cb1978 100644 --- a/services/graph/pkg/identity/ldap_group_test.go +++ b/services/graph/pkg/identity/ldap_group_test.go @@ -21,22 +21,42 @@ var groupEntry = ldap.NewEntry("cn=group", "uid=invalid,ou=people,dc=test", }, }) + var invalidGroupEntry = ldap.NewEntry("cn=invalid", map[string][]string{ "cn": {"invalid"}, }) +var queryParamExpand = url.Values{ + "$expand": []string{"members"}, +} + +var queryParamSelect = url.Values{ + "$select": []string{"members"}, +} + +var groupLookupSearchRequest = &ldap.SearchRequest{ + BaseDN: "ou=groups,dc=test", + Scope: 2, + SizeLimit: 1, + Filter: "(&(objectClass=groupOfNames)(|(cn=group)(entryUUID=group)))", + Attributes: []string{"cn", "entryUUID", "member"}, + Controls: []ldap.Control(nil), +} + +var groupListSeachRequest = &ldap.SearchRequest{ + BaseDN: "ou=groups,dc=test", + Scope: 2, + Filter: "(&(objectClass=groupOfNames))", + Attributes: []string{"cn", "entryUUID", "member"}, + Controls: []ldap.Control(nil), +} + func TestGetGroup(t *testing.T) { // Mock a Sizelimit Error lm := &mocks.Client{} lm.On("Search", mock.Anything).Return(nil, ldap.NewError(ldap.LDAPResultSizeLimitExceeded, errors.New("mock"))) - queryParamExpand := url.Values{ - "$expand": []string{"members"}, - } - queryParamSelect := url.Values{ - "$select": []string{"members"}, - } b, _ := getMockedBackend(lm, lconfig, &logger) _, err := b.GetGroup(context.Background(), "group", nil) if err == nil || err.Error() != "itemNotFound" { @@ -89,14 +109,6 @@ func TestGetGroup(t *testing.T) { // Mock a valid Search Result lm = &mocks.Client{} - sr1 := &ldap.SearchRequest{ - BaseDN: "ou=groups,dc=test", - Scope: 2, - SizeLimit: 1, - Filter: "(&(objectClass=groupOfNames)(|(cn=group)(entryUUID=group)))", - Attributes: []string{"cn", "entryUUID", "member"}, - Controls: []ldap.Control(nil), - } sr2 := &ldap.SearchRequest{ BaseDN: "uid=user,ou=people,dc=test", SizeLimit: 1, @@ -112,7 +124,7 @@ func TestGetGroup(t *testing.T) { Controls: []ldap.Control(nil), } - lm.On("Search", sr1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{groupEntry}}, nil) + lm.On("Search", groupLookupSearchRequest).Return(&ldap.SearchResult{Entries: []*ldap.Entry{groupEntry}}, nil) 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) @@ -146,13 +158,80 @@ func TestGetGroup(t *testing.T) { } } -func TestGetGroups(t *testing.T) { - queryParamExpand := url.Values{ - "$expand": []string{"members"}, +func TestGetGroupReadOnlyBackend(t *testing.T) { + readOnlyConfig := lconfig + readOnlyConfig.WriteEnabled = false + + lm := &mocks.Client{} + lm.On("Search", groupLookupSearchRequest).Return(&ldap.SearchResult{Entries: []*ldap.Entry{groupEntry}}, nil) + b, _ := getMockedBackend(lm, readOnlyConfig, &logger) + g, err := b.GetGroup(context.Background(), "group", url.Values{}) + switch { + case err != nil: + t.Errorf("Expected GetGroup to succeed. Got %s", err.Error()) + case g.GetId() != groupEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id): + t.Errorf("Expected GetGroup to return a valid group") + } + types := g.GetGroupTypes() + switch { + case len(types) == 0: + t.Errorf("No groupTypes attribute on readonly Group") + case len(types) > 1: + t.Errorf("Expected a single groupTypes value on readonly Group") + case types[0] != "ReadOnly": + t.Errorf("Expected a groupTypes 'ReadOnly' on readonly Group") + } +} +func TestGetGroupReadOnlySubtree(t *testing.T) { + readOnlyTreeConfig := lconfig + readOnlyTreeConfig.GroupCreateBaseDN = "ou=write,ou=groups,dc=test" + var writeGroupEntry = ldap.NewEntry("cn=group,ou=write,ou=groups,dc=test", + map[string][]string{ + "cn": {"group"}, + "entryuuid": {"abcd-defg"}, + "member": { + "uid=user,ou=people,dc=test", + "uid=invalid,ou=people,dc=test", + }, + }) + + lm := &mocks.Client{} + lm.On("Search", groupLookupSearchRequest).Return(&ldap.SearchResult{Entries: []*ldap.Entry{groupEntry}}, nil) + b, _ := getMockedBackend(lm, readOnlyTreeConfig, &logger) + g, err := b.GetGroup(context.Background(), "group", url.Values{}) + switch { + case err != nil: + t.Errorf("Expected GetGroup to succeed. Got %s", err.Error()) + case g.GetId() != groupEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id): + t.Errorf("Expected GetGroup to return a valid group") } - queryParamSelect := url.Values{ - "$select": []string{"members"}, + types := g.GetGroupTypes() + switch { + case len(types) == 0: + t.Errorf("No groupTypes attribute on readonly Group") + case len(types) > 1: + t.Errorf("Expected a single groupTypes value on readonly Group") + case types[0] != "ReadOnly": + t.Errorf("Expected a groupTypes 'ReadOnly' on readonly Group") } + + lm = &mocks.Client{} + lm.On("Search", groupLookupSearchRequest).Return(&ldap.SearchResult{Entries: []*ldap.Entry{writeGroupEntry}}, nil) + b, _ = getMockedBackend(lm, readOnlyTreeConfig, &logger) + g, err = b.GetGroup(context.Background(), "group", url.Values{}) + switch { + case err != nil: + t.Errorf("Expected GetGroup to succeed. Got %s", err.Error()) + case g.GetId() != groupEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id): + t.Errorf("Expected GetGroup to return a valid group") + } + types = g.GetGroupTypes() + if len(types) != 0 { + t.Errorf("No groupTypes attribute expected on writeable Group") + } +} + +func TestGetGroups(t *testing.T) { lm := &mocks.Client{} lm.On("Search", mock.Anything).Return(nil, ldap.NewError(ldap.LDAPResultOperationsError, errors.New("mock"))) b, _ := getMockedBackend(lm, lconfig, &logger) @@ -185,13 +264,6 @@ func TestGetGroups(t *testing.T) { // Mock a valid Search Result with expanded group members lm = &mocks.Client{} - sr1 := &ldap.SearchRequest{ - BaseDN: "ou=groups,dc=test", - Scope: 2, - Filter: "(&(objectClass=groupOfNames))", - Attributes: []string{"cn", "entryUUID", "member"}, - Controls: []ldap.Control(nil), - } sr2 := &ldap.SearchRequest{ BaseDN: "uid=user,ou=people,dc=test", SizeLimit: 1, @@ -208,7 +280,7 @@ func TestGetGroups(t *testing.T) { } for _, param := range []url.Values{queryParamSelect, queryParamExpand} { - lm.On("Search", sr1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{groupEntry}}, nil) + lm.On("Search", groupListSeachRequest).Return(&ldap.SearchResult{Entries: []*ldap.Entry{groupEntry}}, nil) 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) diff --git a/services/graph/pkg/service/v0/errorcode/errorcode.go b/services/graph/pkg/service/v0/errorcode/errorcode.go index 4f2f9301044..911bb27f9df 100644 --- a/services/graph/pkg/service/v0/errorcode/errorcode.go +++ b/services/graph/pkg/service/v0/errorcode/errorcode.go @@ -106,6 +106,8 @@ func (e Error) Render(w http.ResponseWriter, r *http.Request) { status = http.StatusNotFound case NameAlreadyExists: status = http.StatusConflict + case NotAllowed: + status = http.StatusForbidden default: status = http.StatusInternalServerError } diff --git a/services/graph/pkg/service/v0/groups.go b/services/graph/pkg/service/v0/groups.go index 0e2dbaa68c0..895d21ab9d3 100644 --- a/services/graph/pkg/service/v0/groups.go +++ b/services/graph/pkg/service/v0/groups.go @@ -82,8 +82,13 @@ func (g Graph) PostGroup(w http.ResponseWriter, r *http.Request) { } if grp, err = g.identityBackend.CreateGroup(r.Context(), *grp); err != nil { - logger.Debug().Interface("group", grp).Msg("could not create group: backend error") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + var errcode errorcode.Error + if errors.As(err, &errcode) { + errcode.Render(w, r) + } else { + logger.Debug().Interface("group", grp).Msg("could not create group: backend error") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + } return }