Skip to content

Commit

Permalink
feat(graph): add support for filtering users by type
Browse files Browse the repository at this point in the history
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: owncloud#9702
  • Loading branch information
rhafer committed Aug 13, 2024
1 parent 80631bd commit 23ba6e4
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 24 deletions.
14 changes: 14 additions & 0 deletions changelog/unreleased/ocm_users_graph.md
Original file line number Diff line number Diff line change
@@ -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
28 changes: 8 additions & 20 deletions services/graph/pkg/service/v0/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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())
Expand All @@ -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))
}
Expand Down
25 changes: 25 additions & 0 deletions services/graph/pkg/service/v0/users_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
66 changes: 62 additions & 4 deletions services/graph/pkg/service/v0/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand All @@ -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))
Expand All @@ -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"))
})
})
})
})

0 comments on commit 23ba6e4

Please sign in to comment.