diff --git a/changelog/unreleased/populate-expanded-properties.md b/changelog/unreleased/populate-expanded-properties.md new file mode 100644 index 00000000000..b3ea06a4357 --- /dev/null +++ b/changelog/unreleased/populate-expanded-properties.md @@ -0,0 +1,6 @@ +Bugfix: Populate expanded properties + +We now return an empty array when an expanded relation has no entries. This makes consuming the responses a little easier. + +https://github.com/owncloud/ocis/pull/5421 +https://github.com/owncloud/ocis/issues/5419 \ No newline at end of file diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index a7a34ec932d..ade4a9e2d04 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -387,10 +387,7 @@ func (i *LDAP) GetUser(ctx context.Context, nameOrID string, queryParam url.Valu if err != nil { return nil, err } - if len(userGroups) > 0 { - groups := i.groupsFromLDAPEntries(userGroups) - u.MemberOf = groups - } + u.MemberOf = i.groupsFromLDAPEntries(userGroups) } return u, nil } @@ -454,14 +451,7 @@ func (i *LDAP) GetUsers(ctx context.Context, queryParam url.Values) ([]*libregra if err != nil { return nil, err } - if len(userGroups) > 0 { - expand := ldap.EscapeFilter(queryParam.Get("expand")) - if expand == "" { - expand = "false" - } - groups := i.groupsFromLDAPEntries(userGroups) - u.MemberOf = groups - } + u.MemberOf = i.groupsFromLDAPEntries(userGroups) } users = append(users, u) } diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index 49a0d1e6399..70027ed1f6b 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -41,14 +41,13 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val if err != nil { return nil, err } + g.Members = make([]libregraph.User, 0, len(members)) if len(members) > 0 { - m := make([]libregraph.User, 0, len(members)) for _, ue := range members { if u := i.createUserModelFromLDAP(ue); u != nil { - m = append(m, *u) + g.Members = append(g.Members, *u) } } - g.Members = m } } return g, nil @@ -120,14 +119,13 @@ func (i *LDAP) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregr if err != nil { return nil, err } + g.Members = make([]libregraph.User, 0, len(members)) if len(members) > 0 { - m := make([]libregraph.User, 0, len(members)) for _, ue := range members { if u := i.createUserModelFromLDAP(ue); u != nil { - m = append(m, *u) + g.Members = append(g.Members, *u) } } - g.Members = m } } groups = append(groups, g) diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index c106d2ed781..29a4e27ad8b 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -478,8 +478,10 @@ func (g Graph) formatDrives(ctx context.Context, baseURL *url.URL, storageSpaces // can't access disabled space if utils.ReadPlainFromOpaque(storageSpace.Opaque, "trashed") != "trashed" { res.Special = g.getExtendedSpaceProperties(ctx, baseURL, storageSpace) - res.Quota, err = g.getDriveQuota(ctx, storageSpace) + quota, err := g.getDriveQuota(ctx, storageSpace) + res.Quota = "a if err != nil { + //logger.Debug().Err(err).Interface("id", sp.Id).Msg("error calling get quota on drive") return nil, err } } @@ -718,6 +720,8 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa lastModified := cs3TimestampToTime(space.Mtime) drive.LastModifiedDateTime = &lastModified } + + drive.Quota = &libregraph.Quota{} if space.Quota != nil { var t int64 if space.Quota.QuotaMaxBytes > math.MaxInt64 { @@ -725,15 +729,13 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa } else { t = int64(space.Quota.QuotaMaxBytes) } - drive.Quota = &libregraph.Quota{ - Total: &t, - } + drive.Quota.Total = &t } return drive, nil } -func (g Graph) getDriveQuota(ctx context.Context, space *storageprovider.StorageSpace) (*libregraph.Quota, error) { +func (g Graph) getDriveQuota(ctx context.Context, space *storageprovider.StorageSpace) (libregraph.Quota, error) { logger := g.logger.SubloggerWithRequestID(ctx) client := g.GetGatewayClient() @@ -747,13 +749,13 @@ func (g Graph) getDriveQuota(ctx context.Context, space *storageprovider.Storage switch { case err != nil: logger.Error().Err(err).Interface("ref", req.Ref).Msg("could not call GetQuota: transport error") - return nil, nil + return libregraph.Quota{}, nil case res.GetStatus().GetCode() == cs3rpc.Code_CODE_UNIMPLEMENTED: logger.Debug().Msg("get quota is not implemented on the storage driver") - return nil, nil + return libregraph.Quota{}, nil case res.GetStatus().GetCode() != cs3rpc.Code_CODE_OK: logger.Debug().Str("grpc", res.GetStatus().GetMessage()).Msg("error sending get quota grpc request") - return nil, errors.New(res.GetStatus().GetMessage()) + return libregraph.Quota{}, errors.New(res.GetStatus().GetMessage()) } var remaining int64 @@ -783,7 +785,7 @@ func (g Graph) getDriveQuota(ctx context.Context, space *storageprovider.Storage state := calculateQuotaState(t, used) qta.State = &state - return &qta, nil + return qta, nil } func calculateQuotaState(total int64, used int64) (state string) { diff --git a/services/graph/pkg/service/v0/drives_test.go b/services/graph/pkg/service/v0/drives_test.go index 28bcb7cf56e..9abacf8403d 100644 --- a/services/graph/pkg/service/v0/drives_test.go +++ b/services/graph/pkg/service/v0/drives_test.go @@ -111,7 +111,7 @@ var sortTests = []sortTest{ } func drive(ID string, dType string, name string, lastModified *time.Time) *libregraph.Drive { - return &libregraph.Drive{Id: libregraph.PtrString(ID), DriveType: libregraph.PtrString(dType), Name: name, LastModifiedDateTime: lastModified} + return &libregraph.Drive{Id: libregraph.PtrString(ID), DriveType: libregraph.PtrString(dType), Name: name, LastModifiedDateTime: lastModified, Quota: &libregraph.Quota{}} } // TestSort tests the available orderby queries diff --git a/services/graph/pkg/service/v0/educationuser.go b/services/graph/pkg/service/v0/educationuser.go index 8a5cdb3c724..1ce9dae2a24 100644 --- a/services/graph/pkg/service/v0/educationuser.go +++ b/services/graph/pkg/service/v0/educationuser.go @@ -257,7 +257,7 @@ func (g Graph) GetEducationUser(w http.ResponseWriter, r *http.Request) { if err != nil { logger.Debug().Err(err).Interface("id", sp.Id).Msg("error calling get quota on drive") } - d.Quota = quota + d.Quota = "a if slices.Contains(sel, "drive") || slices.Contains(exp, "drive") { if *d.DriveType == _spaceTypePersonal { user.Drive = d diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go index 82a896c282b..e7e6d86fe5e 100644 --- a/services/graph/pkg/service/v0/graph_test.go +++ b/services/graph/pkg/service/v0/graph_test.go @@ -143,6 +143,7 @@ var _ = Describe("Graph", func() { "driveType":"aspacetype", "id":"pro-1$sameID", "name":"aspacename", + "quota": {}, "root":{ "id":"pro-1$sameID", "webDavUrl":"https://localhost:9200/dav/spaces/pro-1$sameID" @@ -213,6 +214,7 @@ var _ = Describe("Graph", func() { "driveType":"aspacetype", "id":"pro-1$asameID", "name":"aspacename", + "quota": {}, "root":{ "eTag":"101112131415", "id":"pro-1$asameID", @@ -225,6 +227,7 @@ var _ = Describe("Graph", func() { "driveType":"bspacetype", "id":"pro-1$bsameID", "name":"bspacename", + "quota": {}, "root":{ "eTag":"123456789", "id":"pro-1$bsameID", diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index 201d64af177..c0bddc77752 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/services/graph/pkg/service/v0/users.go @@ -1,6 +1,7 @@ package svc import ( + "context" "encoding/json" "errors" "fmt" @@ -57,13 +58,15 @@ func (g Graph) GetMe(w http.ResponseWriter, r *http.Request) { } return } + if me.MemberOf == nil { + me.MemberOf = []libregraph.Group{} + } } // expand appRoleAssignments if requested if slices.Contains(exp, "appRoleAssignments") { - lrar, err := g.roleService.ListRoleAssignments(r.Context(), &settings.ListRoleAssignmentsRequest{ - AccountUuid: me.GetId(), - }) + var err error + me.AppRoleAssignments, err = g.fetchAppRoleAssignments(r.Context(), me.GetId()) if err != nil { logger.Debug().Err(err).Str("userid", me.GetId()).Msg("could not get appRoleAssignments for self") var errcode errorcode.Error @@ -74,18 +77,27 @@ func (g Graph) GetMe(w http.ResponseWriter, r *http.Request) { } return } - - values := make([]libregraph.AppRoleAssignment, 0, len(lrar.GetAssignments())) - for _, assignment := range lrar.GetAssignments() { - values = append(values, g.assignmentToAppRoleAssignment(assignment)) - } - me.AppRoleAssignments = values } render.Status(r, http.StatusOK) render.JSON(w, r, me) } +func (g Graph) fetchAppRoleAssignments(ctx context.Context, accountuuid string) ([]libregraph.AppRoleAssignment, error) { + lrar, err := g.roleService.ListRoleAssignments(ctx, &settings.ListRoleAssignmentsRequest{ + AccountUuid: accountuuid, + }) + if err != nil { + return []libregraph.AppRoleAssignment{}, err + } + + values := make([]libregraph.AppRoleAssignment, 0, len(lrar.Assignments)) + for _, assignment := range lrar.GetAssignments() { + values = append(values, g.assignmentToAppRoleAssignment(assignment)) + } + return values, nil +} + // GetUsers implements the Service interface. func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) { logger := g.logger.SubloggerWithRequestID(r.Context()) @@ -113,21 +125,21 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) { // expand appRoleAssignments if requested exp := strings.Split(r.URL.Query().Get("$expand"), ",") - if slices.Contains(exp, "appRoleAssignments") { - for _, u := range users { - lrar, err := g.roleService.ListRoleAssignments(r.Context(), &settings.ListRoleAssignmentsRequest{ - AccountUuid: u.GetId(), - }) + expandAppRoleAssignments := slices.Contains(exp, "appRoleAssignments") + expandMemberOf := slices.Contains(exp, "memberOf") + for _, u := range users { + if expandAppRoleAssignments { + u.AppRoleAssignments, err = g.fetchAppRoleAssignments(r.Context(), u.GetId()) if err != nil { + // TODO I think we should not continue here, see http://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_SystemQueryOptionexpand + // > The $expand system query option indicates the related entities and stream values that MUST be represented inline. The service MUST return the specified content, and MAY choose to return additional information. logger.Debug().Err(err).Str("userid", u.GetId()).Msg("could not get appRoleAssignments when listing user") - continue } - - values := make([]libregraph.AppRoleAssignment, 0, len(lrar.GetAssignments())) - for _, assignment := range lrar.GetAssignments() { - values = append(values, g.assignmentToAppRoleAssignment(assignment)) + } + if expandMemberOf { + if u.MemberOf == nil { + u.MemberOf = []libregraph.Group{} } - u.AppRoleAssignments = values } } @@ -307,7 +319,7 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) { if err != nil { logger.Debug().Err(err).Interface("id", sp.Id).Msg("error calling get quota on drive") } - d.Quota = quota + d.Quota = "a if slices.Contains(sel, "drive") || slices.Contains(exp, "drive") { if *d.DriveType == "personal" { user.Drive = d @@ -320,10 +332,7 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) { } // expand appRoleAssignments if requested if slices.Contains(exp, "appRoleAssignments") { - - lrar, err := g.roleService.ListRoleAssignments(r.Context(), &settings.ListRoleAssignmentsRequest{ - AccountUuid: user.GetId(), - }) + user.AppRoleAssignments, err = g.fetchAppRoleAssignments(r.Context(), user.GetId()) if err != nil { logger.Debug().Err(err).Str("userid", user.GetId()).Msg("could not get appRoleAssignments for user") var errcode errorcode.Error @@ -334,12 +343,6 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) { } return } - - values := make([]libregraph.AppRoleAssignment, 0, len(lrar.GetAssignments())) - for _, assignment := range lrar.GetAssignments() { - values = append(values, g.assignmentToAppRoleAssignment(assignment)) - } - user.AppRoleAssignments = values } render.Status(r, http.StatusOK)