Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix populating user drive and drives #5426

Merged
merged 3 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion changelog/unreleased/populate-expanded-properties.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ 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
https://github.com/owncloud/ocis/issues/5419
https://github.com/owncloud/ocis/pull/5426
39 changes: 29 additions & 10 deletions services/graph/pkg/service/v0/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,20 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) {
}
sel := strings.Split(r.URL.Query().Get("$select"), ",")
exp := strings.Split(r.URL.Query().Get("$expand"), ",")
if slices.Contains(sel, "drive") || slices.Contains(sel, "drives") || slices.Contains(exp, "drive") || slices.Contains(exp, "drives") {
listDrives := slices.Contains(sel, "drives") || slices.Contains(exp, "drives")
listDrive := slices.Contains(sel, "drive") || slices.Contains(exp, "drive")

// do we need to list all or only the personal drive
filters := []*storageprovider.ListStorageSpacesRequest_Filter{}
if listDrives || listDrive {
filters = append(filters, listStorageSpacesUserFilter(user.GetId()))
if !listDrives {
// TODO filter by owner when decomposedfs supports the OWNER filter
filters = append(filters, listStorageSpacesTypeFilter("personal"))
}
}

if len(filters) > 0 {
wdu, err := g.getWebDavBaseURL()
if err != nil {
// log error, wrong configuration
Expand All @@ -285,14 +298,13 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) {
render.Status(r, http.StatusInternalServerError)
return
}
logger.Debug().Str("id", user.GetId()).Msg("calling list storage spaces with user id filter")
f := listStorageSpacesUserFilter(user.GetId())
logger.Debug().Str("id", user.GetId()).Msg("calling list storage spaces with filter")
// use the unrestricted flag to get all possible spaces
// users with the canListAllSpaces permission should see all spaces
opaque := utils.AppendPlainToOpaque(nil, "unrestricted", "T")
lspr, err := g.gatewayClient.ListStorageSpaces(r.Context(), &storageprovider.ListStorageSpacesRequest{
Opaque: opaque,
Filters: []*storageprovider.ListStorageSpacesRequest_Filter{f},
Filters: filters,
})
if err != nil {
// transport error, needs to be fixed by admin
Expand All @@ -302,13 +314,18 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) {
return
}
if lspr.GetStatus().GetCode() != cs3rpc.Code_CODE_OK {
logger.Debug().Str("grpc", lspr.GetStatus().GetMessage()).Msg("could not get drive for user")
logger.Debug().Str("grpc", lspr.GetStatus().GetMessage()).Msg("could not list drives for user")
// in case of NOT_OK, we can just return the user object with empty drives
render.Status(r, status.HTTPStatusFromCode(http.StatusOK))
render.JSON(w, r, user)
return
}
drives := []libregraph.Drive{}
if listDrives {
user.Drives = make([]libregraph.Drive, 0, len(lspr.GetStorageSpaces()))
}
if listDrive {
user.Drive = &libregraph.Drive{}
}
for _, sp := range lspr.GetStorageSpaces() {
d, err := g.cs3StorageSpaceToDrive(r.Context(), wdu, sp)
if err != nil {
Expand All @@ -320,16 +337,18 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) {
logger.Debug().Err(err).Interface("id", sp.Id).Msg("error calling get quota on drive")
}
d.Quota = &quota
if slices.Contains(sel, "drive") || slices.Contains(exp, "drive") {
if *d.DriveType == "personal" {
if d.GetDriveType() == "personal" && sp.GetOwner().GetId().GetOpaqueId() == user.GetId() {
if listDrive {
user.Drive = d
}
} else {
drives = append(drives, *d)
user.Drives = drives
if listDrives {
user.Drives = append(user.Drives, *d)
}
Comment on lines +340 to +347
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for clarification: This means that personal spaces do not appear in the drives list. I can only get them as single drive. Is that the intended behaviour?

Copy link
Member Author

@butonic butonic Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there is always only one personal drive you are the owner of: /me/drive

As a secretary you might get access to other personal drives but they will only show up in /me/drives.

The admin might create another drive that is owned by you. I would still use type 'project' for that. We should reserve type 'personal' only for the users personal drive.

}
}
}

// expand appRoleAssignments if requested
if slices.Contains(exp, "appRoleAssignments") {
user.AppRoleAssignments, err = g.fetchAppRoleAssignments(r.Context(), user.GetId())
Expand Down
1 change: 1 addition & 0 deletions services/graph/pkg/service/v0/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ var _ = Describe("Users", func() {
},
{
Id: &provider.StorageSpaceId{OpaqueId: "personal"},
Owner: &userv1beta1.User{Id: &userv1beta1.UserId{OpaqueId: "user1"}},
Root: &provider.ResourceId{SpaceId: "personal", OpaqueId: "personal"},
SpaceType: "personal",
},
Expand Down