Skip to content

Commit

Permalink
return the users username instead of the uuid in share responses
Browse files Browse the repository at this point in the history
A while ago we decided to use the users uuid only internally and to only display or return the users username. The ocs share api did still return the uuid.

Closes owncloud/ocis#990
  • Loading branch information
David Christofas committed Dec 9, 2020
1 parent e791b55 commit b5098b8
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 26 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/ocs-share-api-userid.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Change: return the users username instead of the uuid in share responses

A while ago we decided to use the users uuid only internally and to only display or return the users username.
The ocs share api did still return the uuid.

https://github.com/owncloud/ocis/issues/990
58 changes: 41 additions & 17 deletions internal/http/services/owncloud/ocs/conversions/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ import (

"github.com/cs3org/reva/pkg/publicshare"
"github.com/cs3org/reva/pkg/user"
"github.com/pkg/errors"

gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
Expand Down Expand Up @@ -273,15 +275,38 @@ func AsCS3Permissions(p int, rp *provider.ResourcePermissions) *provider.Resourc
return rp
}

// UserServiceUserIDResolver returns a userId resolver using the UserService
func UserServiceUserIDResolver(ctx context.Context, client gatewayv1beta1.GatewayAPIClient) func(*userpb.UserId) (string, error) {
return func(id *userpb.UserId) (string, error) {
resp, err := client.GetUser(ctx, &userpb.GetUserRequest{UserId: id})
if err != nil {
return "", err
}
return resp.GetUser().GetUsername(), nil
}
}

// UserShare2ShareData converts a cs3api user share into shareData data model
// TODO(jfd) merge userShare2ShareData with publicShare2ShareData
func UserShare2ShareData(ctx context.Context, share *collaboration.Share) (*ShareData, error) {
func UserShare2ShareData(ctx context.Context, share *collaboration.Share, userIDResolver func(*userpb.UserId) (string, error)) (*ShareData, error) {
creator, err := userIDResolver(share.GetCreator())
if err != nil {
return nil, errors.Wrapf(err, "could not resolve user for id: %s", share.GetCreator())
}
owner, err := userIDResolver(share.GetOwner())
if err != nil {
return nil, errors.Wrapf(err, "could not resolve user for id: %s", share.GetOwner())
}
grantee, err := userIDResolver(share.GetGrantee().GetId())
if err != nil {
return nil, errors.Wrapf(err, "could not resolve user for id: %s", share.GetGrantee().GetId())
}
sd := &ShareData{
Permissions: UserSharePermissions2OCSPermissions(share.GetPermissions()),
ShareType: ShareTypeUser,
UIDOwner: LocalUserIDToString(share.GetCreator()),
UIDFileOwner: LocalUserIDToString(share.GetOwner()),
ShareWith: LocalUserIDToString(share.GetGrantee().GetId()),
UIDOwner: creator,
UIDFileOwner: owner,
ShareWith: grantee,
}

if share.Id != nil && share.Id.OpaqueId != "" {
Expand All @@ -297,14 +322,21 @@ func UserShare2ShareData(ctx context.Context, share *collaboration.Share) (*Shar
}

// PublicShare2ShareData converts a cs3api public share into shareData data model
func PublicShare2ShareData(share *link.PublicShare, r *http.Request, publicURL string) *ShareData {
func PublicShare2ShareData(share *link.PublicShare, r *http.Request, publicURL string, userIDResolver func(*userpb.UserId) (string, error)) (*ShareData, error) {
var expiration string
if share.Expiration != nil {
expiration = timestampToExpiration(share.Expiration)
} else {
expiration = ""
}

creator, err := userIDResolver(share.GetCreator())
if err != nil {
return nil, errors.Wrapf(err, "could not resolve user for id: %s", share.GetCreator())
}
owner, err := userIDResolver(share.GetOwner())
if err != nil {
return nil, errors.Wrapf(err, "could not resolve user for id: %s", share.GetOwner())
}
sd := &ShareData{
// share.permissions are mapped below
// Displaynames are added later
Expand All @@ -318,28 +350,20 @@ func PublicShare2ShareData(share *link.PublicShare, r *http.Request, publicURL s
MailSend: 0,
URL: publicURL + path.Join("/", "#/s/"+share.Token),
Permissions: publicSharePermissions2OCSPermissions(share.GetPermissions()),
UIDOwner: LocalUserIDToString(share.Creator),
UIDFileOwner: LocalUserIDToString(share.Owner),
UIDOwner: creator,
UIDFileOwner: owner,
}

if share.PasswordProtected {
sd.ShareWith = "***redacted***"
sd.ShareWithDisplayname = "***redacted***"
}

return sd
return sd, nil
// actually clients should be able to GET and cache the user info themselves ...
// TODO check grantee type for user vs group
}

// LocalUserIDToString transforms a cs3api user id into an ocs data model without domain name
func LocalUserIDToString(userID *userpb.UserId) string {
if userID == nil || userID.OpaqueId == "" {
return ""
}
return userID.OpaqueId
}

// UserIDToString transforms a cs3api user id into an ocs data model
// TODO This should be used instead of LocalUserIDToString bit it requires interpreting an @ on the client side
// TODO An alternative would be to send the idp / iss as an additional attribute. might be less intrusive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ func (h *Handler) createPublicLinkShare(w http.ResponseWriter, r *http.Request)
return
}

s := conversions.PublicShare2ShareData(createRes.Share, r, h.publicURL)
s, err := conversions.PublicShare2ShareData(createRes.Share, r, h.publicURL, conversions.UserServiceUserIDResolver(ctx, c))
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err)
return
}
err = h.addFileInfo(ctx, s, statRes.Info)
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err)
Expand Down Expand Up @@ -199,7 +203,10 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh
continue
}

sData := conversions.PublicShare2ShareData(share, r, h.publicURL)
sData, err := conversions.PublicShare2ShareData(share, r, h.publicURL, conversions.UserServiceUserIDResolver(ctx, c))
if err != nil {
return nil, nil, errors.Wrap(err, "could not convert share to share data")
}

sData.Name = share.DisplayName

Expand Down Expand Up @@ -425,7 +432,12 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar
return
}

s := conversions.PublicShare2ShareData(publicShare, r, h.publicURL)
s, err := conversions.PublicShare2ShareData(publicShare, r, h.publicURL, conversions.UserServiceUserIDResolver(r.Context(), gwC))
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err)
return
}

err = h.addFileInfo(r.Context(), s, statRes.Info)
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ func (h *Handler) createUserShare(w http.ResponseWriter, r *http.Request) {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc create share request failed", err)
return
}
s, err := conversions.UserShare2ShareData(ctx, createShareResponse.Share)
s, err := conversions.UserShare2ShareData(ctx, createShareResponse.Share, conversions.UserServiceUserIDResolver(ctx, c))
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err)
return
Expand Down Expand Up @@ -451,7 +451,11 @@ func (h *Handler) getShare(w http.ResponseWriter, r *http.Request, shareID strin
*/

if err == nil && psRes.GetShare() != nil {
share = conversions.PublicShare2ShareData(psRes.Share, r, h.publicURL)
share, err = conversions.PublicShare2ShareData(psRes.Share, r, h.publicURL, conversions.UserServiceUserIDResolver(ctx, client))
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err)
return
}
resourceID = psRes.Share.ResourceId
}

Expand Down Expand Up @@ -486,7 +490,7 @@ func (h *Handler) getShare(w http.ResponseWriter, r *http.Request, shareID strin

if err == nil && uRes.GetShare() != nil {
resourceID = uRes.Share.ResourceId
share, err = conversions.UserShare2ShareData(ctx, uRes.Share)
share, err = conversions.UserShare2ShareData(ctx, uRes.Share, conversions.UserServiceUserIDResolver(ctx, client))
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err)
return
Expand Down Expand Up @@ -614,7 +618,7 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, shareID st
return
}

share, err := conversions.UserShare2ShareData(ctx, gRes.Share)
share, err := conversions.UserShare2ShareData(ctx, gRes.Share, conversions.UserServiceUserIDResolver(ctx, uClient))
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err)
return
Expand Down Expand Up @@ -793,7 +797,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) {
info = statRes.GetInfo()
}

data, err := conversions.UserShare2ShareData(r.Context(), rs.Share)
data, err := conversions.UserShare2ShareData(r.Context(), rs.Share, conversions.UserServiceUserIDResolver(ctx, gwc))
if err != nil {
log.Debug().Interface("share", rs.Share).Interface("shareData", data).Err(err).Msg("could not UserShare2ShareData, skipping")
continue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (h *Handler) listUserShares(r *http.Request, filters []*collaboration.ListS

// build OCS response payload
for _, s := range lsUserSharesResponse.Shares {
data, err := conversions.UserShare2ShareData(ctx, s)
data, err := conversions.UserShare2ShareData(ctx, s, conversions.UserServiceUserIDResolver(ctx, c))
if err != nil {
log.Debug().Interface("share", s).Interface("shareData", data).Err(err).Msg("could not UserShare2ShareData, skipping")
continue
Expand Down

0 comments on commit b5098b8

Please sign in to comment.