sevenhill/QSfera
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

graph/groups: Allow unprivileged users to search for groups

Browse Source
This commit is contained in:
Ralf Haferkamp
2023-12-07 10:21:45 +01:00
committed by Ralf Haferkamp
parent 6fab0ad05a
commit 1ace257064
6 changed files with 120 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelog/unreleased/enhancement-allow-listing-regular-users.md
+5 -3
View File
@@ -1,12 +1,14 @@
Enhancement: Allow regular users to list other users
Regular users can search for other users. The following limitations
Regular users can search for other users and groups. The following limitations
apply:
* Only search queries are allowed (using the `$search=term` query parameter)
* The search term needs to have at least 3 characters
* The result set only contains the attribute `displayName`, `userType`, `mail`
and `id`
* for user searches the result set only contains the attributes `displayName`,
`userType`, `mail` and `id`
* for group searches the result set only contains the attributes `displayName`,
`groupTypes` and `id`
https://github.com/owncloud/ocis/pull/7887
https://github.com/owncloud/ocis/issues/7782
services/graph/pkg/identity/ldap_group.go
+1 -1
View File
@@ -75,7 +75,7 @@ func (i *LDAP) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregr
if search != "" {
search = ldap.EscapeFilter(search)
groupFilter = fmt.Sprintf(
"(|(%s=%s*)(%s=%s*))",
"(|(%s=*%s*)(%s=*%s*))",
i.groupAttributeMap.name, search,
i.groupAttributeMap.id, search,
)
services/graph/pkg/service/v0/groups.go
+26
View File
@@ -31,6 +31,20 @@ func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
return
}
ctxHasFullPerms := g.contextUserHasFullAccountPerms(r.Context())
if !ctxHasFullPerms && (odataReq.Query == nil || odataReq.Query.Search == nil || len(odataReq.Query.Search.RawValue) < g.config.API.IdentitySearchMinLength) {
// 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")
return
}
if !ctxHasFullPerms && (odataReq.Query.Filter != nil || odataReq.Query.Apply != nil || odataReq.Query.Expand != nil || odataReq.Query.Compute != nil) {
// regular users can't use filter, apply, expand and compute
logger.Debug().Interface("query", r.URL.Query()).Msg("forbidden query elements for a regular user")
errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "query has forbidden elements for regular users")
return
}
groups, err := g.identityBackend.GetGroups(r.Context(), r.URL.Query())
if err != nil {
@@ -38,6 +52,18 @@ func (g Graph) GetGroups(w http.ResponseWriter, r *http.Request) {
errorcode.RenderError(w, r, err)
return
}
// If the user isn't admin, we'll show just the minimum group attibutes
if !ctxHasFullPerms {
finalGroups := make([]*libregraph.Group, len(groups))
for i, grp := range groups {
finalGroups[i] = &libregraph.Group{
Id: grp.Id,
DisplayName: grp.DisplayName,
GroupTypes: grp.GroupTypes,
}
}
groups = finalGroups
}
groups, err = sortGroups(odataReq, groups)
if err != nil {
services/graph/pkg/service/v0/groups_test.go
+87 -7
View File
@@ -19,6 +19,8 @@ import (
. "github.com/onsi/gomega"
libregraph "github.com/owncloud/libre-graph-api-go"
"github.com/owncloud/ocis/v2/ocis-pkg/shared"
settingsmsg "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0"
settings "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0"
"github.com/owncloud/ocis/v2/services/graph/mocks"
"github.com/owncloud/ocis/v2/services/graph/pkg/config"
"github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults"
@@ -35,13 +37,14 @@ type groupList struct {
var _ = Describe("Groups", func() {
var (
svc service.Service
ctx context.Context
cfg *config.Config
gatewayClient *cs3mocks.GatewayAPIClient
gatewaySelector pool.Selectable[gateway.GatewayAPIClient]
eventsPublisher mocks.Publisher
identityBackend *identitymocks.Backend
svc service.Service
ctx context.Context
cfg *config.Config
gatewayClient *cs3mocks.GatewayAPIClient
gatewaySelector pool.Selectable[gateway.GatewayAPIClient]
eventsPublisher mocks.Publisher
identityBackend *identitymocks.Backend
permissionService *mocks.Permissions
rr *httptest.ResponseRecorder
@@ -65,6 +68,7 @@ var _ = Describe("Groups", func() {
return gatewayClient
},
)
permissionService = &mocks.Permissions{}
identityBackend = &identitymocks.Backend{}
newGroup = libregraph.NewGroup()
@@ -85,6 +89,7 @@ var _ = Describe("Groups", func() {
service.WithGatewaySelector(gatewaySelector),
service.EventsPublisher(&eventsPublisher),
service.WithIdentityBackend(identityBackend),
service.PermissionService(permissionService),
)
})
@@ -97,6 +102,12 @@ var _ = Describe("Groups", func() {
})
It("handles invalid sorting queries", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{
Permission: &settingsmsg.Permission{
Operation: settingsmsg.Permission_OPERATION_UNKNOWN,
Constraint: settingsmsg.Permission_CONSTRAINT_ALL,
},
}, nil)
identityBackend.On("GetGroups", ctx, mock.Anything).Return([]*libregraph.Group{newGroup}, nil)
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/groups?$orderby=invalid", nil)
@@ -113,6 +124,12 @@ var _ = Describe("Groups", func() {
})
It("handles unknown backend errors", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{
Permission: &settingsmsg.Permission{
Operation: settingsmsg.Permission_OPERATION_UNKNOWN,
Constraint: settingsmsg.Permission_CONSTRAINT_ALL,
},
}, nil)
identityBackend.On("GetGroups", ctx, mock.Anything).Return(nil, errors.New("failed"))
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/groups", nil)
@@ -128,6 +145,12 @@ var _ = Describe("Groups", func() {
})
It("handles backend errors", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{
Permission: &settingsmsg.Permission{
Operation: settingsmsg.Permission_OPERATION_UNKNOWN,
Constraint: settingsmsg.Permission_CONSTRAINT_ALL,
},
}, nil)
identityBackend.On("GetGroups", ctx, mock.Anything).Return(nil, errorcode.New(errorcode.AccessDenied, "access denied"))
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/groups", nil)
@@ -144,6 +167,12 @@ var _ = Describe("Groups", func() {
})
It("renders an empty list of groups", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{
Permission: &settingsmsg.Permission{
Operation: settingsmsg.Permission_OPERATION_UNKNOWN,
Constraint: settingsmsg.Permission_CONSTRAINT_ALL,
},
}, nil)
identityBackend.On("GetGroups", ctx, mock.Anything).Return([]*libregraph.Group{}, nil)
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/groups", nil)
@@ -160,6 +189,12 @@ var _ = Describe("Groups", func() {
})
It("renders a list of groups", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{
Permission: &settingsmsg.Permission{
Operation: settingsmsg.Permission_OPERATION_UNKNOWN,
Constraint: settingsmsg.Permission_CONSTRAINT_ALL,
},
}, nil)
identityBackend.On("GetGroups", ctx, mock.Anything).Return([]*libregraph.Group{newGroup}, nil)
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/groups", nil)
@@ -176,6 +211,51 @@ var _ = Describe("Groups", func() {
Expect(len(res.Value)).To(Equal(1))
Expect(res.Value[0].GetId()).To(Equal("group1"))
})
It("denies listing for unprivileged users", func() {
permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settings.GetPermissionByIDResponse{}, nil)
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/users", nil)
svc.GetGroups(rr, r)
Expect(rr.Code).To(Equal(http.StatusForbidden))
})
It("denies using to short 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/users?$search=a", 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{}
group.SetId("group1")
group.SetDisplayName("Group Name")
group.SetMembers(
[]libregraph.User{
libregraph.User{
Id: libregraph.PtrString("userid"),
},
},
)
groups := []*libregraph.Group{group}
identityBackend.On("GetGroups", mock.Anything, mock.Anything, mock.Anything).Return(groups, nil)
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/groups?$search=abc", nil)
svc.GetGroups(rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))
data, err := io.ReadAll(rr.Body)
Expect(err).ToNot(HaveOccurred())
res := groupList{}
err = json.Unmarshal(data, &res)
Expect(err).ToNot(HaveOccurred())
groupMap, err := res.Value[0].ToMap()
Expect(err).ToNot(HaveOccurred())
for k, _ := range groupMap {
Expect(k).Should(BeElementOf([]string{"displayName", "id", "groupTypes"}))
}
})
})
Describe("GetGroup", func() {
services/graph/pkg/service/v0/service.go
+1 -1
View File
@@ -265,7 +265,7 @@ func NewService(opts ...Option) (Graph, error) {
})
})
r.Route("/groups", func(r chi.Router) {
r.With(requireAdmin).Get("/", svc.GetGroups)
r.Get("/", svc.GetGroups)
r.With(requireAdmin).Post("/", svc.PostGroup)
r.Route("/{groupID}", func(r chi.Router) {
r.Get("/", svc.GetGroup)
tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md
-6
View File
@@ -115,15 +115,9 @@ The expected failures in this file are from features in the owncloud/ocis repo.
- [apiGraph/editGroup.feature:35](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/editGroup.feature#L35)
- [apiGraph/editGroup.feature:34](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/editGroup.feature#L34)
- [apiGraph/editGroup.feature:36](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/editGroup.feature#L36)
- [apiGraph/getGroup.feature:54](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L54)
- [apiGraph/getGroup.feature:55](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L55)
- [apiGraph/getGroup.feature:56](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L56)
- [apiGraph/getGroup.feature:103](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L103)
- [apiGraph/getGroup.feature:104](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L104)
- [apiGraph/getGroup.feature:105](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L105)
- [apiGraph/getGroup.feature:267](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L267)
- [apiGraph/getGroup.feature:268](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L268)
- [apiGraph/getGroup.feature:269](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getGroup.feature#L269)
- [apiGraph/removeUserFromGroup.feature:191](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/removeUserFromGroup.feature#L191)
- [apiGraph/removeUserFromGroup.feature:192](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/removeUserFromGroup.feature#L192)
- [apiGraph/removeUserFromGroup.feature:193](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/removeUserFromGroup.feature#L193)