mirror/opencloud
1
0
Fork 0
mirror of https://github.com/opencloud-eu/opencloud.git synced 2026-04-21 18:28:27 -05:00
Code Issues Packages Projects Releases Wiki Activity

graph/groups: Handle quoted search terms in GetGroups

Browse Source 
Fixes: #7990
This commit is contained in:
Ralf Haferkamp
2023-12-21 12:22:14 +01:00
committed by Ralf Haferkamp
parent 8489170715
commit a1ed2ce2e5
9 changed files with 109 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelog/unreleased/fix-search-by-exact-mail.md
+6 -4
View File
@@ -1,7 +1,9 @@
Bugfix: Fix search by exact email
Bugfix: Fix search by containing special characters
Users can be searched by exact email by using double quotes on the search
parameter. Note that double quotes are required because the "@" char is
being interpreted by the parser.
As the OData query parser interprets characters like '@' or '-' in a special
way. Search request for users or groups needs to be quoted. We fixed the libregraph
users and groups endpoints to handle quoted search terms correctly.
https://github.com/owncloud/ocis/pull/8050
https://github.com/owncloud/ocis/pull/8035
https://github.com/owncloud/ocis/issues/7990
services/graph/pkg/identity/backend.go
+1 -1
View File
@@ -37,7 +37,7 @@ type Backend interface {
// UpdateGroupName updates the group name
UpdateGroupName(ctx context.Context, groupID string, groupName string) error
GetGroup(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.Group, error)
GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error)
GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*libregraph.Group, error)
// GetGroupMembers list all members of a group
GetGroupMembers(ctx context.Context, id string, oreq *godata.GoDataRequest) ([]*libregraph.User, error)
// AddMembersToGroup adds new members (reference by a slice of IDs) to supplied group in the identity backend.
services/graph/pkg/identity/cs3.go
+4 -4
View File
@@ -112,7 +112,7 @@ func (i *CS3) GetUsers(ctx context.Context, oreq *godata.GoDataRequest) ([]*libr
}
// GetGroups implements the Backend Interface.
func (i *CS3) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error) {
func (i *CS3) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*libregraph.Group, error) {
logger := i.Logger.SubloggerWithRequestID(ctx)
logger.Debug().Str("backend", "cs3").Msg("GetGroups")
gatewayClient, err := i.GatewaySelector.Next()
@@ -121,9 +121,9 @@ func (i *CS3) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregra
return nil, errorcode.New(errorcode.ServiceNotAvailable, err.Error())
}
search := queryParam.Get("search")
if search == "" {
search = queryParam.Get("$search")
search, err := GetSearchValues(oreq.Query)
if err != nil {
return nil, err
}
res, err := gatewayClient.FindGroups(ctx, &cs3group.FindGroupsRequest{
services/graph/pkg/identity/ldap_group.go
+14 -7
View File
@@ -55,19 +55,26 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val
}
// GetGroups implements the Backend Interface for the LDAP Backend
func (i *LDAP) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error) {
func (i *LDAP) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*libregraph.Group, error) {
logger := i.logger.SubloggerWithRequestID(ctx)
logger.Debug().Str("backend", "ldap").Msg("GetGroups")
search := queryParam.Get("search")
if search == "" {
search = queryParam.Get("$search")
search, err := GetSearchValues(oreq.Query)
if err != nil {
return nil, err
}
var expandMembers bool
sel := strings.Split(queryParam.Get("$select"), ",")
exp := strings.Split(queryParam.Get("$expand"), ",")
if slices.Contains(sel, "members") || slices.Contains(exp, "members") {
exp, err := GetExpandValues(oreq.Query)
if err != nil {
return nil, err
}
sel, err := GetSelectValues(oreq.Query)
if err != nil {
return nil, err
}
if slices.Contains(exp, "members") || slices.Contains(sel, "members") {
expandMembers = true
}
services/graph/pkg/identity/ldap_group_test.go
+39 -4
View File
@@ -6,6 +6,7 @@ import (
"net/url"
"testing"
"github.com/CiscoM31/godata"
"github.com/go-ldap/ldap/v3"
"github.com/owncloud/ocis/v2/services/graph/mocks"
"github.com/stretchr/testify/assert"
@@ -219,15 +220,19 @@ func TestGetGroupReadOnlySubtree(t *testing.T) {
func TestGetGroups(t *testing.T) {
lm := &mocks.Client{}
oDataReq, err := godata.ParseRequest(context.Background(), "", url.Values{})
if err != nil {
t.Errorf("Expected success, got '%s'", err.Error())
}
lm.On("Search", mock.Anything).Return(nil, ldap.NewError(ldap.LDAPResultOperationsError, errors.New("mock")))
b, _ := getMockedBackend(lm, lconfig, &logger)
_, err := b.GetGroups(context.Background(), url.Values{})
_, err = b.GetGroups(context.Background(), oDataReq)
assert.ErrorContains(t, err, "itemNotFound:")
lm = &mocks.Client{}
lm.On("Search", mock.Anything).Return(&ldap.SearchResult{}, nil)
b, _ = getMockedBackend(lm, lconfig, &logger)
g, err := b.GetGroups(context.Background(), url.Values{})
g, err := b.GetGroups(context.Background(), oDataReq)
if err != nil {
t.Errorf("Expected success, got '%s'", err.Error())
} else if g == nil || len(g) != 0 {
@@ -239,7 +244,7 @@ func TestGetGroups(t *testing.T) {
Entries: []*ldap.Entry{groupEntry},
}, nil)
b, _ = getMockedBackend(lm, lconfig, &logger)
g, err = b.GetGroups(context.Background(), url.Values{})
g, err = b.GetGroups(context.Background(), oDataReq)
if err != nil {
t.Errorf("Expected GetGroup to succeed. Got %s", err.Error())
} else if *g[0].Id != groupEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id) {
@@ -264,11 +269,15 @@ func TestGetGroups(t *testing.T) {
}
for _, param := range []url.Values{queryParamSelect, queryParamExpand} {
oDataReq, err := godata.ParseRequest(context.Background(), "", param)
if err != nil {
t.Errorf("Expected success, got '%s'", err.Error())
}
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)
g, err = b.GetGroups(context.Background(), param)
g, err = b.GetGroups(context.Background(), oDataReq)
switch {
case err != nil:
t.Errorf("Expected GetGroup to succeed. Got %s", err.Error())
@@ -281,6 +290,32 @@ func TestGetGroups(t *testing.T) {
}
}
}
func TestGetGroupsSearch(t *testing.T) {
lm := &mocks.Client{}
odataReqDefault, err := godata.ParseRequest(context.Background(), "",
url.Values{
"$search": []string{"\"term\""},
},
)
if err != nil {
t.Errorf("Expected success got '%s'", err.Error())
}
// only match if the filter contains the search term unquoted
lm.On("Search", mock.MatchedBy(
func(req *ldap.SearchRequest) bool {
return req.Filter == "(&(objectClass=groupOfNames)(|(cn=*term*)(entryUUID=*term*)))"
})).
Return(&ldap.SearchResult{}, nil)
b, _ := getMockedBackend(lm, lconfig, &logger)
g, err := b.GetGroups(context.Background(), odataReqDefault)
if err != nil {
t.Errorf("Expected success, got '%s'", err.Error())
} else if g == nil || len(g) != 0 {
t.Errorf("Expected zero length user slice")
}
}
func TestUpdateGroupName(t *testing.T) {
groupDn := "cn=TheGroup,ou=groups,dc=owncloud,dc=com"
services/graph/pkg/identity/mocks/backend.go
+9 -9
View File
@@ -165,25 +165,25 @@ func (_m *Backend) GetGroupMembers(ctx context.Context, id string, oreq *godata.
return r0, r1
}
// GetGroups provides a mock function with given fields: ctx, queryParam
func (_m *Backend) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error) {
ret := _m.Called(ctx, queryParam)
// GetGroups provides a mock function with given fields: ctx, oreq
func (_m *Backend) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*libregraph.Group, error) {
ret := _m.Called(ctx, oreq)
var r0 []*libregraph.Group
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, url.Values) ([]*libregraph.Group, error)); ok {
return rf(ctx, queryParam)
if rf, ok := ret.Get(0).(func(context.Context, *godata.GoDataRequest) ([]*libregraph.Group, error)); ok {
return rf(ctx, oreq)
}
if rf, ok := ret.Get(0).(func(context.Context, url.Values) []*libregraph.Group); ok {
r0 = rf(ctx, queryParam)
if rf, ok := ret.Get(0).(func(context.Context, *godata.GoDataRequest) []*libregraph.Group); ok {
r0 = rf(ctx, oreq)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]*libregraph.Group)
}
}
if rf, ok := ret.Get(1).(func(context.Context, url.Values) error); ok {
r1 = rf(ctx, queryParam)
if rf, ok := ret.Get(1).(func(context.Context, *godata.GoDataRequest) error); ok {
r1 = rf(ctx, oreq)
} else {
r1 = ret.Error(1)
}
services/graph/pkg/identity/odata.go
+17
View File
@@ -29,6 +29,23 @@ func GetExpandValues(req *godata.GoDataQuery) ([]string, error) {
return expand, nil
}
// GetSelectValues extracts the values of the $select query parameter and
// returns them in a []string, rejects any $select value that consists of more
// than just a single path segment
func GetSelectValues(req *godata.GoDataQuery) ([]string, error) {
if req == nil || req.Select == nil {
return []string{}, nil
}
sel := make([]string, 0, len(req.Select.SelectItems))
for _, item := range req.Select.SelectItems {
if len(item.Segments) > 1 {
return []string{}, godata.NotImplementedError("multiple segments in $select not supported")
}
sel = append(sel, item.Segments[0].Value)
}
return sel, nil
}
// GetSearchValues extracts the value of the $search query parameter and returns
// it as a string. Rejects any search query that is more than just a simple string
func GetSearchValues(req *godata.GoDataQuery) (string, error) {
services/graph/pkg/service/v0/groups.go
+12 -2
View File
@@ -32,7 +32,17 @@ func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
return
}
ctxHasFullPerms := g.contextUserHasFullAccountPerms(r.Context())
if !ctxHasFullPerms && (odataReq.Query == nil || odataReq.Query.Search == nil || len(odataReq.Query.Search.RawValue) < g.config.API.IdentitySearchMinLength) {
searchHasAcceptableLength := false
if odataReq.Query != nil && odataReq.Query.Search != nil {
minSearchLength := g.config.API.IdentitySearchMinLength
if strings.HasPrefix(odataReq.Query.Search.RawValue, "\"") {
// if search starts with double quotes then it must finish with double quotes
// add +2 to the minimum search length in this case
minSearchLength += 2
}
searchHasAcceptableLength = len(odataReq.Query.Search.RawValue) >= minSearchLength
}
if !ctxHasFullPerms && !searchHasAcceptableLength {
// for regular user the search term must have a minimum length
logger.Debug().Interface("query", r.URL.Query()).Msgf("search with less than %d chars for a regular user", g.config.API.IdentitySearchMinLength)
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "search term too short")
@@ -46,7 +56,7 @@ func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
return
}
groups, err := g.identityBackend.GetGroups(r.Context(), r.URL.Query())
groups, err := g.identityBackend.GetGroups(r.Context(), odataReq)
if err != nil {
logger.Debug().Err(err).Msg("could not get groups: backend error")
errorcode.RenderError(w, r, err)
services/graph/pkg/service/v0/groups_test.go
+7
View File
@@ -225,6 +225,13 @@ var _ = Describe("Groups", func() {
Expect(rr.Code).To(Equal(http.StatusForbidden))
})
It("denies using to short quoted search terms for unprivileged users", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{}, nil)
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/groups?$search=%22ab%22", nil)
svc.GetGroups(rr, r)
Expect(rr.Code).To(Equal(http.StatusForbidden))
})
It("only returns a restricted set of attributes for unprivileged users", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{}, nil)
group := &libregraph.Group{}