diff --git a/changelog/unreleased/additional-share-with.md b/changelog/unreleased/additional-share-with.md new file mode 100644 index 0000000000..86d8432b4b --- /dev/null +++ b/changelog/unreleased/additional-share-with.md @@ -0,0 +1,6 @@ +Enhancement: Render additional share with in ocs sharing api + +Recipients can now be distinguished by their email, which is rendered as additional info in the ocs api for share and file owners as well as share recipients. + +https://github.com/cs3org/reva/pull/1451 +https://github.com/owncloud/ocis/issues/1190 diff --git a/internal/http/services/owncloud/ocs/conversions/main.go b/internal/http/services/owncloud/ocs/conversions/main.go index 894ef381ef..f99e120126 100644 --- a/internal/http/services/owncloud/ocs/conversions/main.go +++ b/internal/http/services/owncloud/ocs/conversions/main.go @@ -83,6 +83,8 @@ type ShareData struct { UIDOwner string `json:"uid_owner" xml:"uid_owner"` // The display name of the owner of the share. DisplaynameOwner string `json:"displayname_owner" xml:"displayname_owner"` + // Additional info to identify the share owner, eg. the email or username + AdditionalInfoOwner string `json:"additional_info_owner" xml:"additional_info_owner"` // The permission attribute set on the file. // TODO(jfd) change the default to read only Permissions Permissions `json:"permissions" xml:"permissions"` @@ -98,9 +100,7 @@ type ShareData struct { UIDFileOwner string `json:"uid_file_owner" xml:"uid_file_owner"` // The display name of the user that owns the file or folder being shared. DisplaynameFileOwner string `json:"displayname_file_owner" xml:"displayname_file_owner"` - // ? - AdditionalInfoOwner string `json:"additional_info_owner" xml:"additional_info_owner"` - // ? + // Additional info to identify the file owner, eg. the email or username AdditionalInfoFileOwner string `json:"additional_info_file_owner" xml:"additional_info_file_owner"` // share state, 0 = accepted, 1 = pending, 2 = declined State int `json:"state" xml:"state"` @@ -120,13 +120,14 @@ type ShareData struct { FileParent string `json:"file_parent" xml:"file_parent"` // The basename of the shared file. FileTarget string `json:"file_target" xml:"file_target"` - // The uid of the receiver of the file. This is either + // The uid of the share recipient. This is either // - a GID (group id) if it is being shared with a group or // - a UID (user id) if the share is shared with a user. + // - a password for public links ShareWith string `json:"share_with,omitempty" xml:"share_with,omitempty"` - // The display name of the receiver of the file. + // The display name of the share recipient ShareWithDisplayname string `json:"share_with_displayname,omitempty" xml:"share_with_displayname,omitempty"` - // sharee Additional info + // Additional info to identify the share recipient, eg. the email or username ShareWithAdditionalInfo string `json:"share_with_additional_info" xml:"share_with_additional_info"` // Whether the recipient was notified, by mail, about the share being shared with them. MailSend int `json:"mail_send" xml:"mail_send"` @@ -163,70 +164,71 @@ type MatchData struct { // MatchValueData holds the type and actual value type MatchValueData struct { - ShareType int `json:"shareType" xml:"shareType"` - ShareWith string `json:"shareWith" xml:"shareWith"` + ShareType int `json:"shareType" xml:"shareType"` + ShareWith string `json:"shareWith" xml:"shareWith"` + ShareWithAdditionalInfo string `json:"shareWithAdditionalInfo" xml:"shareWithAdditionalInfo"` } // 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) { sd := &ShareData{ - Permissions: UserSharePermissions2OCSPermissions(share.GetPermissions()), + // share.permissions are mapped below + // Displaynames are added later ShareType: ShareTypeUser, UIDOwner: LocalUserIDToString(share.GetCreator()), UIDFileOwner: LocalUserIDToString(share.GetOwner()), ShareWith: LocalUserIDToString(share.GetGrantee().GetId()), } - - if share.Id != nil && share.Id.OpaqueId != "" { + if share.Id != nil { sd.ID = share.Id.OpaqueId } + if share.GetPermissions() != nil && share.GetPermissions().GetPermissions() != nil { + sd.Permissions = RoleFromResourcePermissions(share.GetPermissions().GetPermissions()).OCSPermissions() + } if share.Ctime != nil { sd.STime = share.Ctime.Seconds // TODO CS3 api birth time = btime } - // actually clients should be able to GET and cache the user info themselves ... - // TODO only return the userid, let the clientso look up the displayname // TODO check grantee type for user vs group return sd, nil } // PublicShare2ShareData converts a cs3api public share into shareData data model func PublicShare2ShareData(share *link.PublicShare, r *http.Request, publicURL string) *ShareData { - var expiration string - if share.Expiration != nil { - expiration = timestampToExpiration(share.Expiration) - } else { - expiration = "" - } - sd := &ShareData{ // share.permissions are mapped below // Displaynames are added later - ID: share.Id.OpaqueId, ShareType: ShareTypePublicLink, - STime: share.Ctime.Seconds, // TODO CS3 api birth time = btime Token: share.Token, - Expiration: expiration, - MimeType: share.Mtime.String(), Name: share.DisplayName, MailSend: 0, URL: publicURL + path.Join("/", "#/s/"+share.Token), - Permissions: publicSharePermissions2OCSPermissions(share.GetPermissions()), UIDOwner: LocalUserIDToString(share.Creator), UIDFileOwner: LocalUserIDToString(share.Owner), } + if share.Id != nil { + sd.ID = share.Id.OpaqueId + } + if share.GetPermissions() != nil && share.GetPermissions().GetPermissions() != nil { + sd.Permissions = RoleFromResourcePermissions(share.GetPermissions().GetPermissions()).OCSPermissions() + } + if share.Expiration != nil { + sd.Expiration = timestampToExpiration(share.Expiration) + } + if share.Ctime != nil { + sd.STime = share.Ctime.Seconds // TODO CS3 api birth time = btime + } + // hide password if share.PasswordProtected { sd.ShareWith = "***redacted***" sd.ShareWithDisplayname = "***redacted***" } return sd - // 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 +// TODO ocs uses user names ... so an additional lookup is needed. see mapUserIds() func LocalUserIDToString(userID *userpb.UserId) string { if userID == nil || userID.OpaqueId == "" { return "" @@ -247,14 +249,6 @@ func UserIDToString(userID *userpb.UserId) string { return userID.OpaqueId + "@" + userID.Idp } -// UserSharePermissions2OCSPermissions transforms cs3api permissions into OCS Permissions data model -func UserSharePermissions2OCSPermissions(sp *collaboration.SharePermissions) Permissions { - if sp != nil { - return RoleFromResourcePermissions(sp.GetPermissions()).OCSPermissions() - } - return PermissionInvalid -} - // GetUserManager returns a connection to a user share manager func GetUserManager(manager string, m map[string]map[string]interface{}) (user.Manager, error) { if f, ok := usermgr.NewFuncs[manager]; ok { @@ -273,15 +267,25 @@ func GetPublicShareManager(manager string, m map[string]map[string]interface{}) return nil, fmt.Errorf("driver %s not found for public shares manager", manager) } -func publicSharePermissions2OCSPermissions(sp *link.PublicSharePermissions) Permissions { - if sp != nil { - return RoleFromResourcePermissions(sp.GetPermissions()).OCSPermissions() - } - return PermissionInvalid -} - // timestamp is assumed to be UTC ... just human readable ... // FIXME and ambiguous / error prone because there is no time zone ... func timestampToExpiration(t *types.Timestamp) string { return time.Unix(int64(t.Seconds), int64(t.Nanos)).UTC().Format("2006-01-02 15:05:05") } + +// ParseTimestamp tries to parses the ocs expiry into a CS3 Timestamp +func ParseTimestamp(timestampString string) (*types.Timestamp, error) { + parsedTime, err := time.Parse("2006-01-02T15:04:05Z0700", timestampString) + if err != nil { + parsedTime, err = time.Parse("2006-01-02", timestampString) + } + if err != nil { + return nil, fmt.Errorf("datetime format invalid: %v", timestampString) + } + final := parsedTime.UnixNano() + + return &types.Timestamp{ + Seconds: uint64(final / 1000000000), + Nanos: uint32(final % 1000000000), + }, nil +} diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/sharees/sharees.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/sharees/sharees.go index 0a2d0f1408..66f0006e9d 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/sharees/sharees.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/sharees/sharees.go @@ -105,9 +105,9 @@ func (h *Handler) userAsMatch(u *userpb.User) *conversions.MatchData { Label: u.DisplayName, Value: &conversions.MatchValueData{ ShareType: int(conversions.ShareTypeUser), - // TODO(jfd) find more robust userid - // username might be ok as it is unique at a given point in time - ShareWith: u.Username, + // api compatibility with oc10: always use the username + ShareWith: u.Username, + ShareWithAdditionalInfo: u.Mail, }, } } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go index 1dbd2582f3..88ce48bf4f 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go @@ -92,7 +92,7 @@ func (h *Handler) createPublicLinkShare(w http.ResponseWriter, r *http.Request, expireTimeString, ok := r.Form["expireDate"] if ok { if expireTimeString[0] != "" { - expireTime, err := parseTimestamp(expireTimeString[0]) + expireTime, err := conversions.ParseTimestamp(expireTimeString[0]) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "invalid datetime format", err) return @@ -130,7 +130,7 @@ func (h *Handler) createPublicLinkShare(w http.ResponseWriter, r *http.Request, response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err) return } - h.addDisplaynames(ctx, c, s) + h.mapUserIds(ctx, c, s) response.WriteOCSSuccess(w, r, s) } @@ -183,7 +183,7 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh log.Debug().Interface("share", share).Interface("info", statResponse.Info).Err(err).Msg("could not add file info, skipping") continue } - h.addDisplaynames(ctx, c, sData) + h.mapUserIds(ctx, c, sData) log.Debug().Interface("share", share).Interface("info", statResponse.Info).Interface("shareData", share).Msg("mapped") @@ -324,7 +324,7 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar updatesFound = true var newExpiration *types.Timestamp if expireTimeString[0] != "" { - newExpiration, err = parseTimestamp(expireTimeString[0]) + newExpiration, err = conversions.ParseTimestamp(expireTimeString[0]) if err != nil { response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, "invalid datetime format", err) return @@ -407,7 +407,7 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err) return } - h.addDisplaynames(r.Context(), gwC, s) + h.mapUserIds(r.Context(), gwC, s) response.WriteOCSSuccess(w, r, s) } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index bcb93e16f4..f02eef6b08 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -27,7 +27,6 @@ import ( "path" "strconv" "strings" - "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -35,7 +34,6 @@ import ( collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/rs/zerolog/log" "github.com/cs3org/reva/internal/http/services/owncloud/ocdav" @@ -51,19 +49,24 @@ import ( // Handler implements the shares part of the ownCloud sharing API type Handler struct { - gatewayAddr string - publicURL string - sharePrefix string - displayNameCache *ttlmap.TTLMap - userNameCache *ttlmap.TTLMap + gatewayAddr string + publicURL string + sharePrefix string + userIdentifierCache *ttlmap.TTLMap +} + +// we only cache the minimal set of data instead of the full user metadata +type userIdentifiers struct { + DisplayName string + UserName string + Mail string } // Init initializes this and any contained handlers func (h *Handler) Init(c *config.Config) error { h.gatewayAddr = c.GatewaySvc h.publicURL = c.Config.Host - h.displayNameCache = ttlmap.New(1000, 60) - h.userNameCache = ttlmap.New(1000, 60) + h.userIdentifierCache = ttlmap.New(1000, 60) h.sharePrefix = c.SharePrefix return nil } @@ -392,7 +395,6 @@ func (h *Handler) getShare(w http.ResponseWriter, r *http.Request, shareID strin log.Error().Err(err).Str("status", statResponse.Status.Code.String()).Msg("error mapping share data") response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) } - h.addDisplaynames(ctx, client, share) h.mapUserIds(ctx, client, share) response.WriteOCSSuccess(w, r, []*conversions.ShareData{share}) @@ -516,7 +518,6 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, shareID st response.WriteOCSError(w, r, response.MetaServerError.StatusCode, err.Error(), err) return } - h.addDisplaynames(ctx, uClient, share) h.mapUserIds(ctx, uClient, share) response.WriteOCSSuccess(w, r, share) @@ -681,7 +682,6 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { log.Debug().Interface("received_share", rs).Interface("info", info).Interface("shareData", data).Err(err).Msg("could not add file info, skipping") continue } - h.addDisplaynames(r.Context(), gwc, data) h.mapUserIds(r.Context(), gwc, data) if data.State == ocsStateAccepted { @@ -871,140 +871,84 @@ func (h *Handler) addFileInfo(ctx context.Context, s *conversions.ShareData, inf return nil } -func (h *Handler) getDisplayname(ctx context.Context, c gateway.GatewayAPIClient, userid string) string { - log := appctx.GetLogger(ctx) +// mustGetUserIdentifiers always returns a struct with identifiers, if the user could not be found they will all be empty +func (h *Handler) mustGetUserIdentifiers(ctx context.Context, c gateway.GatewayAPIClient, userid string) *userIdentifiers { + sublog := appctx.GetLogger(ctx).With().Str("userid", userid).Logger() if userid == "" { - return "" + return &userIdentifiers{} } - if dn := h.displayNameCache.Get(userid); dn != "" { - log.Debug().Str("userid", userid).Msg("cache hit") - return dn + //item := h.userIdentifierCache.Get(userid) + ui, ok := h.userIdentifierCache.Get(userid).(*userIdentifiers) + if ok { + sublog.Debug().Msg("cache hit") + return ui } - log.Debug().Str("userid", userid).Msg("cache miss") + sublog.Debug().Msg("cache miss") res, err := c.GetUser(ctx, &userpb.GetUserRequest{ UserId: &userpb.UserId{ OpaqueId: userid, }, }) if err != nil { - log.Err(err). - Str("userid", userid). - Msg("could not look up user") - return "" + sublog.Err(err).Msg("could not look up user") + return &userIdentifiers{} } if res.GetStatus().GetCode() != rpc.Code_CODE_OK { - log.Err(err). - Str("opaque_id", userid). + sublog.Err(err). Int32("code", int32(res.GetStatus().GetCode())). Str("message", res.GetStatus().GetMessage()). Msg("get user call failed") - return "" + return &userIdentifiers{} } if res.User == nil { - log.Debug(). - Str("opaque_id", userid). + sublog.Debug(). Int32("code", int32(res.GetStatus().GetCode())). Str("message", res.GetStatus().GetMessage()). Msg("user not found") - return "" - } - if res.User.DisplayName == "" { - log.Debug(). - Str("opaque_id", userid). - Int32("code", int32(res.GetStatus().GetCode())). - Str("message", res.GetStatus().GetMessage()). - Msg("Displayname empty") - return "" + return &userIdentifiers{} } - h.displayNameCache.Put(userid, res.User.DisplayName) - h.userNameCache.Put(userid, res.User.Username) - log.Debug().Str("userid", userid).Msg("cache update") - return res.User.DisplayName -} - -func (h *Handler) getUsername(ctx context.Context, c gateway.GatewayAPIClient, userid string) string { - log := appctx.GetLogger(ctx) - if userid == "" { - return "" - } - if un := h.userNameCache.Get(userid); un != "" { - log.Debug().Str("userid", userid).Msg("cache hit") - return un - } - log.Debug().Str("userid", userid).Msg("cache miss") - res, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: &userpb.UserId{ - OpaqueId: userid, - }, - }) - if err != nil { - log.Err(err). - Str("userid", userid). - Msg("could not look up user") - return "" - } - if res.GetStatus().GetCode() != rpc.Code_CODE_OK { - log.Err(err). - Str("opaque_id", userid). - Int32("code", int32(res.GetStatus().GetCode())). - Str("message", res.GetStatus().GetMessage()). - Msg("get user call failed") - return "" - } - if res.User == nil { - log.Debug(). - Str("opaque_id", userid). - Int32("code", int32(res.GetStatus().GetCode())). - Str("message", res.GetStatus().GetMessage()). - Msg("user not found") - return "" - } - if res.User.Username == "" { - log.Debug(). - Str("opaque_id", userid). - Int32("code", int32(res.GetStatus().GetCode())). - Str("message", res.GetStatus().GetMessage()). - Msg("Username empty") - return "" + ui = &userIdentifiers{ + DisplayName: res.User.DisplayName, + UserName: res.User.Username, + Mail: res.User.Mail, } - - h.userNameCache.Put(userid, res.User.Username) - h.displayNameCache.Put(userid, res.User.DisplayName) + h.userIdentifierCache.Put(userid, ui) log.Debug().Str("userid", userid).Msg("cache update") - return res.User.Username -} - -func (h *Handler) addDisplaynames(ctx context.Context, c gateway.GatewayAPIClient, s *conversions.ShareData) { - if s.DisplaynameOwner == "" { - s.DisplaynameOwner = h.getDisplayname(ctx, c, s.UIDOwner) - } - if s.DisplaynameFileOwner == "" { - s.DisplaynameFileOwner = h.getDisplayname(ctx, c, s.UIDFileOwner) - } - if s.ShareWithDisplayname == "" { - s.ShareWithDisplayname = h.getDisplayname(ctx, c, s.ShareWith) - } + return ui } func (h *Handler) mapUserIds(ctx context.Context, c gateway.GatewayAPIClient, s *conversions.ShareData) { - s.UIDOwner = h.getUsername(ctx, c, s.UIDOwner) - s.UIDFileOwner = h.getUsername(ctx, c, s.UIDFileOwner) - s.ShareWith = h.getUsername(ctx, c, s.ShareWith) -} - -func parseTimestamp(timestampString string) (*types.Timestamp, error) { - parsedTime, err := time.Parse("2006-01-02T15:04:05Z0700", timestampString) - if err != nil { - parsedTime, err = time.Parse("2006-01-02", timestampString) + if s.UIDOwner != "" { + owner := h.mustGetUserIdentifiers(ctx, c, s.UIDOwner) + s.UIDOwner = owner.UserName + if s.DisplaynameOwner == "" { + s.DisplaynameOwner = owner.DisplayName + } + if s.AdditionalInfoFileOwner == "" { + s.AdditionalInfoFileOwner = owner.Mail + } } - if err != nil { - return nil, fmt.Errorf("datetime format invalid: %v", timestampString) + + if s.UIDFileOwner != "" { + fileOwner := h.mustGetUserIdentifiers(ctx, c, s.UIDFileOwner) + s.UIDFileOwner = fileOwner.UserName + if s.DisplaynameFileOwner == "" { + s.DisplaynameFileOwner = fileOwner.DisplayName + } + if s.AdditionalInfoOwner == "" { + s.AdditionalInfoOwner = fileOwner.Mail + } } - final := parsedTime.UnixNano() - return &types.Timestamp{ - Seconds: uint64(final / 1000000000), - Nanos: uint32(final % 1000000000), - }, nil + if s.ShareWith != "" && s.ShareWith != "***redacted***" { + shareWith := h.mustGetUserIdentifiers(ctx, c, s.ShareWith) + s.ShareWith = shareWith.UserName + if s.ShareWithDisplayname == "" { + s.ShareWithDisplayname = shareWith.DisplayName + } + if s.ShareWithAdditionalInfo == "" { + s.ShareWithAdditionalInfo = shareWith.Mail + } + } } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go index 3ce051d788..1ea53811df 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go @@ -154,7 +154,6 @@ func (h *Handler) createUserShare(w http.ResponseWriter, r *http.Request, statIn response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error adding fileinfo to share", err) return } - h.addDisplaynames(ctx, c, s) h.mapUserIds(ctx, c, s) response.WriteOCSSuccess(w, r, s) @@ -246,7 +245,6 @@ func (h *Handler) listUserShares(r *http.Request, filters []*collaboration.ListS log.Debug().Interface("share", s).Interface("info", statResponse.Info).Interface("shareData", data).Err(err).Msg("could not add file info, skipping") continue } - h.addDisplaynames(ctx, c, data) h.mapUserIds(ctx, c, data) log.Debug().Interface("share", s).Interface("info", rInfo).Interface("shareData", data).Msg("mapped") diff --git a/pkg/ttlmap/ttlmap.go b/pkg/ttlmap/ttlmap.go index 1867492cd0..30fa17a0ea 100644 --- a/pkg/ttlmap/ttlmap.go +++ b/pkg/ttlmap/ttlmap.go @@ -31,7 +31,7 @@ type TTLMap struct { } type item struct { - value string + value interface{} lastAccess int64 } @@ -58,7 +58,7 @@ func (m *TTLMap) Len() int { } // Put sets or overwrites an item, resetting the ttl -func (m *TTLMap) Put(k, v string) { +func (m *TTLMap) Put(k string, v interface{}) { m.l.Lock() it, ok := m.m[k] if !ok { @@ -70,7 +70,7 @@ func (m *TTLMap) Put(k, v string) { } // Get retrieves an item from the cache, resetting the ttl -func (m *TTLMap) Get(k string) (v string) { +func (m *TTLMap) Get(k string) (v interface{}) { m.l.Lock() if it, ok := m.m[k]; ok { v = it.value