Remove Expired Link on Access (#1361)

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

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,25 +318,24 @@ 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
 		}
 
 		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()