diff --git a/changelog/unreleased/remove-public-shares-on-list.md b/changelog/unreleased/remove-public-shares-on-list.md new file mode 100644 index 0000000000..c60614ce02 --- /dev/null +++ b/changelog/unreleased/remove-public-shares-on-list.md @@ -0,0 +1,5 @@ +Enhancement: Remove expired Link on Access + +Since there is no background jobs scheduled to wipe out expired resources, for the time being public links are going to be removed on a "on demand" basis, meaning whenever there is an API call that access the list of shares for a given resource, we will check whether the share is expired and delete it if so. + +https://github.com/cs3org/reva/pull/1361 \ No newline at end of file diff --git a/pkg/publicshare/manager/json/json.go b/pkg/publicshare/manager/json/json.go index 4d61b29751..8c1bb63b4e 100644 --- a/pkg/publicshare/manager/json/json.go +++ b/pkg/publicshare/manager/json/json.go @@ -32,6 +32,8 @@ import ( "sync" "time" + "github.com/rs/zerolog/log" + user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -41,6 +43,7 @@ import ( "github.com/cs3org/reva/pkg/publicshare/manager/registry" "github.com/golang/protobuf/jsonpb" "github.com/mitchellh/mapstructure" + "go.opencensus.io/trace" ) func init() { @@ -279,10 +282,7 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu return nil, err } - // compare ref opaque id with share opaqueid for _, v := range db { - // db[ref.GetId().GetOpaqueId()].(map[string]interface{})["share"] - // fmt.Printf("\nHERE\n%v\n\n", v.(map[string]interface{})["share"]) d := v.(map[string]interface{})["share"] ps := &link.PublicShare{} @@ -297,26 +297,11 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu } return nil, errors.New("no shares found by id:" + ref.GetId().String()) - - // found, ok := db[ref.GetId().GetOpaqueId()].(map[string]interface{})["share"] - // if !ok { - // return nil, errors.New("no shares found by id:" + ref.GetId().String()) - // } - - // ps := publicShare{} - // r := bytes.NewBuffer([]byte(found.(string))) - // if err := m.unmarshaler.Unmarshal(r, &ps); err != nil { - // return nil, err - // } - - // return &ps.PublicShare, nil - } // ListPublicShares retrieves all the shares on the manager that are valid. func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters []*link.ListPublicSharesRequest_Filter, md *provider.ResourceInfo) ([]*link.PublicShare, error) { - shares := []*link.PublicShare{} - now := time.Now() + var shares []*link.PublicShare m.mutex.Lock() defer m.mutex.Unlock() @@ -333,7 +318,7 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters [] return nil, err } - // Skip if the share isn't created by the current user + // skip if the share isn't created by the current user. if local.Creator.GetOpaqueId() != u.Id.OpaqueId || (local.Creator.GetIdp() != "" && u.Id.Idp != local.Creator.GetIdp()) { continue } @@ -341,17 +326,16 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters [] if len(filters) == 0 { shares = append(shares, &local.PublicShare) } else { - for _, f := range filters { - if f.Type == link.ListPublicSharesRequest_Filter_TYPE_RESOURCE_ID { - t := time.Unix(int64(local.Expiration.GetSeconds()), int64(local.Expiration.GetNanos())) - if err != nil { - return nil, err - } - if local.ResourceId.StorageId == f.GetResourceId().StorageId && local.ResourceId.OpaqueId == f.GetResourceId().OpaqueId { - if (local.Expiration != nil && t.After(now)) || local.Expiration == nil { + for i := range filters { + if filters[i].Type == link.ListPublicSharesRequest_Filter_TYPE_RESOURCE_ID { + if local.ResourceId.StorageId == filters[i].GetResourceId().StorageId && local.ResourceId.OpaqueId == filters[i].GetResourceId().OpaqueId { + if notExpired(&local.PublicShare) { shares = append(shares, &local.PublicShare) + } else if err := m.revokeExpiredPublicShare(ctx, &local.PublicShare, u); err != nil { + return nil, err } } + } } } @@ -360,6 +344,40 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters [] return shares, nil } +// notExpired tests whether a public share is expired +func notExpired(s *link.PublicShare) bool { + t := time.Unix(int64(s.Expiration.GetSeconds()), int64(s.Expiration.GetNanos())) + if (s.Expiration != nil && t.After(time.Now())) || s.Expiration == nil { + return true + } + return false +} + +func (m *manager) revokeExpiredPublicShare(ctx context.Context, s *link.PublicShare, u *user.User) error { + m.mutex.Unlock() + defer m.mutex.Lock() + + span := trace.FromContext(ctx) + span.AddAttributes( + trace.StringAttribute("operation", "delete expired share"), + trace.StringAttribute("opaqueId", s.Id.OpaqueId), + ) + + err := m.RevokePublicShare(ctx, u, &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: s.Id.OpaqueId, + }, + }, + }) + if err != nil { + log.Err(err).Msg(fmt.Sprintf("publicShareJSONManager: error deleting public share with opaqueId: %s", s.Id.OpaqueId)) + return err + } + + return nil +} + // RevokePublicShare undocumented. func (m *manager) RevokePublicShare(ctx context.Context, u *user.User, ref *link.PublicShareReference) error { m.mutex.Lock()