mirror of
https://github.com/opencloud-eu/opencloud.git
synced 2026-02-28 00:39:16 -06:00
feat(graph): add support for filtering users by type
This add support for equality filters on the `userType` property of users for the /users endpoint. It also changes the behavior of the /users endpoint to only return federated users when explicitly request via a `userType` filter. E.g. to search for federated users with matching "albert" you can use: `$filter=userType eq 'Federated'&$search="albert"` Related Issue: #9702
This commit is contained in:
14
changelog/unreleased/ocm_users_graph.md
Normal file
14
changelog/unreleased/ocm_users_graph.md
Normal file
@@ -0,0 +1,14 @@
|
||||
Enhancement: OCM related adjustments in graph
|
||||
|
||||
The /users enpdoint of the graph service was changed with respect to how
|
||||
it handles OCM federeated users:
|
||||
- The 'userType' property is now alway returned. As new usertype 'Federated'
|
||||
was introduced. To indicate that the user is a federated user.
|
||||
- Supported for filtering users by 'userType' as added. Queries like
|
||||
"$filter=userType eq 'Federated'" are now possible.
|
||||
- Federated users are only returned when explicitly requested via filter.
|
||||
When no filter is provider only 'Member' users are returned.
|
||||
|
||||
https://github.com/owncloud/ocis/pull/9788
|
||||
https://github.com/owncloud/ocis/pull/9757
|
||||
https://github.com/owncloud/ocis/issues/9702
|
||||
@@ -269,25 +269,6 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
if g.config.IncludeOCMSharees {
|
||||
ocmUsers, err := g.searchOCMAcceptedUsers(r.Context(), odataReq)
|
||||
var errcode errorcode.Error
|
||||
var godataerr *godata.GoDataError
|
||||
switch {
|
||||
case err == nil:
|
||||
users = append(users, ocmUsers...)
|
||||
case errors.As(err, &errcode):
|
||||
errcode.Render(w, r)
|
||||
return
|
||||
case errors.As(err, &godataerr):
|
||||
errorcode.GeneralException.Render(w, r, godataerr.ResponseCode, err.Error())
|
||||
return
|
||||
default:
|
||||
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// If the user isn't admin, we'll show just the minimum user attibutes
|
||||
if !ctxHasFullPerms {
|
||||
finalUsers := make([]*libregraph.User, len(users))
|
||||
@@ -1008,6 +989,13 @@ func isValidUserType(userType string) bool {
|
||||
|
||||
func (g Graph) searchOCMAcceptedUsers(ctx context.Context, odataReq *godata.GoDataRequest) ([]*libregraph.User, error) {
|
||||
logger := g.logger.SubloggerWithRequestID(ctx)
|
||||
users := []*libregraph.User{}
|
||||
|
||||
if !g.config.IncludeOCMSharees {
|
||||
logger.Debug().Msg("OCM sharing is disabled")
|
||||
return users, nil
|
||||
}
|
||||
|
||||
gwc, err := g.gatewaySelector.Next()
|
||||
if err != nil {
|
||||
return nil, errorcode.New(errorcode.GeneralException, err.Error())
|
||||
@@ -1027,7 +1015,7 @@ func (g Graph) searchOCMAcceptedUsers(ctx context.Context, odataReq *godata.GoDa
|
||||
}
|
||||
|
||||
cs3Users := remoteUsersRes.GetAcceptedUsers()
|
||||
users := make([]*libregraph.User, 0, len(cs3Users))
|
||||
users = make([]*libregraph.User, 0, len(cs3Users))
|
||||
for _, user := range cs3Users {
|
||||
users = append(users, identity.CreateUserModelFromCS3(user))
|
||||
}
|
||||
|
||||
@@ -135,6 +135,8 @@ func (g Graph) applyFilterLogical(ctx context.Context, req *godata.GoDataRequest
|
||||
return g.applyFilterLogicalAnd(ctx, req, root.Children[0], root.Children[1])
|
||||
case "or":
|
||||
return g.applyFilterLogicalOr(ctx, req, root.Children[0], root.Children[1])
|
||||
case "eq":
|
||||
return g.applyFilterEq(ctx, req, root.Children[0], root.Children[1])
|
||||
}
|
||||
logger.Debug().Str("Token", root.Token.Value).Msg("unsupported logical filter")
|
||||
return users, unsupportedFilterError()
|
||||
@@ -220,6 +222,29 @@ func (g Graph) applyFilterLogicalOr(ctx context.Context, req *godata.GoDataReque
|
||||
}
|
||||
return filteredUsers, nil
|
||||
}
|
||||
|
||||
func (g Graph) applyFilterEq(ctx context.Context, req *godata.GoDataRequest, operand1 *godata.ParseNode, operand2 *godata.ParseNode) (users []*libregraph.User, err error) {
|
||||
// We only support the 'eq' on 'userType' for now
|
||||
switch {
|
||||
case operand1.Token.Type != godata.ExpressionTokenLiteral:
|
||||
fallthrough
|
||||
case operand1.Token.Value != "userType":
|
||||
fallthrough
|
||||
case operand2.Token.Type != godata.ExpressionTokenString:
|
||||
return users, unsupportedFilterError()
|
||||
}
|
||||
|
||||
// unquote
|
||||
value := strings.Trim(operand2.Token.Value, "'")
|
||||
switch value {
|
||||
case "Member", "Guest":
|
||||
return g.identityBackend.GetUsers(ctx, req)
|
||||
case "Federated":
|
||||
return g.searchOCMAcceptedUsers(ctx, req)
|
||||
}
|
||||
return users, unsupportedFilterError()
|
||||
}
|
||||
|
||||
func (g Graph) applyFilterLambda(ctx context.Context, req *godata.GoDataRequest, nodes []*godata.ParseNode) (users []*libregraph.User, err error) {
|
||||
logger := g.logger.SubloggerWithRequestID(ctx)
|
||||
if len(nodes) != 2 {
|
||||
|
||||
@@ -503,8 +503,8 @@ var _ = Describe("Users", func() {
|
||||
"appRoleAssignments/all(n:n/appRoleId eq 'id') and appRoleAssignments/any(n:n/appRoleId eq 'id')", http.StatusNotImplemented),
|
||||
Entry("with unsupported appRoleAssignments lambda filter operation",
|
||||
"appRoleAssignments/any(n:n/appRoleId ne 'id') and appRoleAssignments/any(n:n/appRoleId eq 'id')", http.StatusNotImplemented),
|
||||
Entry("with unsupported appRoleAssignments lambda filter operation",
|
||||
"appRoleAssignments/any(n:n/appRoleId eq 1) and appRoleAssignments/any(n:n/appRoleId eq 'id')", http.StatusNotImplemented),
|
||||
Entry("with unsupported userType in filter", "userType eq 'unsupported'", http.StatusNotImplemented),
|
||||
Entry("with unsupported property in eq filter", "unsupported eq 'unsupported'", http.StatusNotImplemented),
|
||||
)
|
||||
|
||||
DescribeTable("With a valid filter",
|
||||
@@ -1125,7 +1125,33 @@ var _ = Describe("Users", func() {
|
||||
)
|
||||
})
|
||||
Describe("GetUsers", func() {
|
||||
It("lists the users", func() {
|
||||
It("does not list the federated users without a filter", func() {
|
||||
gatewayClient.AssertNotCalled(GinkgoT(), "FindAcceptedUsers, mock.Anything, mock.Anything")
|
||||
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("GetUsers", mock.Anything, mock.Anything, mock.Anything).Return(
|
||||
[]*libregraph.User{}, nil,
|
||||
)
|
||||
|
||||
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/users", nil)
|
||||
svc.GetUsers(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
res := userList{}
|
||||
err = json.Unmarshal(data, &res)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
Expect(len(res.Value)).To(Equal(0))
|
||||
})
|
||||
It("does list federated users when the right filter is used", func() {
|
||||
gatewayClient.On("FindAcceptedUsers", mock.Anything, mock.Anything).Return(&invitepb.FindAcceptedUsersResponse{
|
||||
Status: status.NewOK(ctx),
|
||||
AcceptedUsers: []*userv1beta1.User{
|
||||
@@ -1149,7 +1175,7 @@ var _ = Describe("Users", func() {
|
||||
[]*libregraph.User{}, nil,
|
||||
)
|
||||
|
||||
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/users", nil)
|
||||
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/users?$filter="+url.QueryEscape("userType eq 'Federated'"), nil)
|
||||
svc.GetUsers(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
@@ -1164,6 +1190,38 @@ var _ = Describe("Users", func() {
|
||||
Expect(res.Value[0].GetId()).To(Equal("federated"))
|
||||
Expect(res.Value[0].GetUserType()).To(Equal("Federated"))
|
||||
})
|
||||
It("does not list federated users when filtering for 'Member' users", 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)
|
||||
|
||||
user := &libregraph.User{}
|
||||
user.SetId("user1")
|
||||
user.SetUserType("Member")
|
||||
users := []*libregraph.User{user}
|
||||
|
||||
identityBackend.On("GetUsers", mock.Anything, mock.Anything, mock.Anything).Return(
|
||||
users, nil,
|
||||
)
|
||||
|
||||
r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/users?$filter="+url.QueryEscape("userType eq 'Member'"), nil)
|
||||
svc.GetUsers(rr, r)
|
||||
|
||||
Expect(rr.Code).To(Equal(http.StatusOK))
|
||||
data, err := io.ReadAll(rr.Body)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
res := userList{}
|
||||
err = json.Unmarshal(data, &res)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
Expect(len(res.Value)).To(Equal(1))
|
||||
Expect(res.Value[0].GetId()).To(Equal("user1"))
|
||||
Expect(res.Value[0].GetUserType()).To(Equal("Member"))
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user