feat: enforce passwords on public share provider

changelog/unreleased/add-validation-to-public-share-provider.md

@@ -0,0 +1,6 @@
+Enhancement: Add validation to the public share provider
+
+We added validation to the public share provider. The idea behind it is that the cs3 clients will become much simpler. The provider can do the validation and return different status codes. The API clients then just need to convert CS3 status codes to http status codes.
+
+https://github.com/cs3org/reva/pull/4372/
+https://github.com/owncloud/ocis/issues/6993

internal/grpc/services/publicshareprovider/publicshareprovider.go

@@ -20,10 +20,19 @@ package publicshareprovider
 
 import (
 	"context"
+	"fmt"
 	"regexp"
+	"strconv"
+	"time"
 
+	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
+	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+	"github.com/cs3org/reva/v2/pkg/password"
+	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/v2/pkg/sharedconf"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 	"google.golang.org/grpc"
@@ -38,16 +47,30 @@ import (
 	"github.com/cs3org/reva/v2/pkg/rgrpc/status"
 )
 
+const getUserCtxErrMsg = "error getting user from context"
+
 func init() {
 	rgrpc.Register("publicshareprovider", New)
 }
 
 type config struct {
 	Driver                         string                            `mapstructure:"driver"`
 	Drivers                        map[string]map[string]interface{} `mapstructure:"drivers"`
+	GatewayAddr                    string                            `mapstructure:"gateway_addr"`
 	AllowedPathsForShares          []string                          `mapstructure:"allowed_paths_for_shares"`
 	EnableExpiredSharesCleanup     bool                              `mapstructure:"enable_expired_shares_cleanup"`
 	WriteableShareMustHavePassword bool                              `mapstructure:"writeable_share_must_have_password"`
+	PublicShareMustHavePassword    bool                              `mapstructure:"public_share_must_have_password"`
+	PasswordPolicy                 map[string]interface{}            `mapstructure:"password_policy"`
+}
+
+type passwordPolicy struct {
+	MinCharacters          int                 `mapstructure:"min_characters"`
+	MinLowerCaseCharacters int                 `mapstructure:"min_lowercase_characters"`
+	MinUpperCaseCharacters int                 `mapstructure:"min_uppercase_characters"`
+	MinDigits              int                 `mapstructure:"min_digits"`
+	MinSpecialCharacters   int                 `mapstructure:"min_special_characters"`
+	BannedPasswordsList    map[string]struct{} `mapstructure:"banned_passwords_list"`
 }
 
 func (c *config) init() {
@@ -59,7 +82,9 @@ func (c *config) init() {
 type service struct {
 	conf                  *config
 	sm                    publicshare.Manager
+	gatewaySelector       pool.Selectable[gateway.GatewayAPIClient]
 	allowedPathsForShares []*regexp.Regexp
+	passwordValidator     password.Validator
 }
 
 func getShareManager(c *config) (publicshare.Manager, error) {
@@ -84,19 +109,32 @@ func (s *service) Register(ss *grpc.Server) {
 func parseConfig(m map[string]interface{}) (*config, error) {
 	c := &config{}
 	if err := mapstructure.Decode(m, c); err != nil {
-		err = errors.Wrap(err, "error decoding conf")
+		err = errors.Wrap(err, "error decoding config")
 		return nil, err
 	}
 	return c, nil
 }
 
+func parsePasswordPolicy(m map[string]interface{}) (*passwordPolicy, error) {
+	p := &passwordPolicy{}
+	if err := mapstructure.Decode(m, p); err != nil {
+		err = errors.Wrap(err, "error decoding password policy config")
+		return nil, err
+	}
+	return p, nil
+}
+
 // New creates a new user share provider svc
 func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {
 
 	c, err := parseConfig(m)
 	if err != nil {
 		return nil, err
 	}
+	p, err := parsePasswordPolicy(c.PasswordPolicy)
+	if err != nil {
+		return nil, err
+	}
 
 	c.init()
 
@@ -114,15 +152,36 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {
 		allowedPathsForShares = append(allowedPathsForShares, regex)
 	}
 
+	gatewaySelector, err := pool.GatewaySelector(sharedconf.GetGatewaySVC(c.GatewayAddr))
+	if err != nil {
+		return nil, err
+	}
+
 	service := &service{
 		conf:                  c,
 		sm:                    sm,
+		gatewaySelector:       gatewaySelector,
 		allowedPathsForShares: allowedPathsForShares,
+		passwordValidator:     newPasswordPolicy(p),
 	}
 
 	return service, nil
 }
 
+func newPasswordPolicy(c *passwordPolicy) password.Validator {
+	if c == nil {
+		return password.NewPasswordPolicy(0, 0, 0, 0, 0, nil)
+	}
+	return password.NewPasswordPolicy(
+		c.MinCharacters,
+		c.MinLowerCaseCharacters,
+		c.MinUpperCaseCharacters,
+		c.MinDigits,
+		c.MinSpecialCharacters,
+		c.BannedPasswordsList,
+	)
+}
+
 func (s *service) isPathAllowed(path string) bool {
 	if len(s.allowedPathsForShares) == 0 {
 		return true
@@ -139,33 +198,110 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS
 	log := appctx.GetLogger(ctx)
 	log.Info().Str("publicshareprovider", "create").Msg("create public share")
 
-	if !conversions.SufficientCS3Permissions(req.GetResourceInfo().GetPermissionSet(), req.GetGrant().GetPermissions().GetPermissions()) {
+	gatewayClient, err := s.gatewaySelector.Next()
+	if err != nil {
+		return nil, err
+	}
+
+	sRes, err := gatewayClient.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: req.GetResourceInfo().GetId()}})
+	if err != nil {
+		log.Err(err).Interface("resource_id", req.GetResourceInfo().GetId()).Msg("failed to stat resource to share")
+		return &link.CreatePublicShareResponse{
+			Status: status.NewInternal(ctx, "failed to stat resource to share"),
+		}, err
+	}
+	// check that user has share permissions
+	if !sRes.GetInfo().GetPermissionSet().AddGrant {
+		return &link.CreatePublicShareResponse{
+			Status: status.NewInvalidArg(ctx, "no share permission"),
+		}, nil
+	}
+
+	// check if the user can share with the desired permissions
+	if !conversions.SufficientCS3Permissions(sRes.GetInfo().GetPermissionSet(), req.GetGrant().GetPermissions().GetPermissions()) {
+		return &link.CreatePublicShareResponse{
+			Status: status.NewInvalidArg(ctx, "insufficient permissions to create that kind of share"),
+		}, nil
+	}
+
+	// validate path
+	if !s.isPathAllowed(req.GetResourceInfo().GetPath()) {
+		return &link.CreatePublicShareResponse{
+			Status: status.NewFailedPrecondition(ctx, nil, "share creation is not allowed for the specified path"),
+		}, nil
+	}
+
+	// check that this is a not a personal space root
+	if req.GetResourceInfo().GetId().GetOpaqueId() == req.GetResourceInfo().GetId().GetSpaceId() &&
+		req.GetResourceInfo().GetSpace().GetSpaceType() == "personal" {
 		return &link.CreatePublicShareResponse{
-			Status: status.NewInvalid(ctx, "insufficient permissions to create that kind of share"),
+			Status: status.NewInvalidArg(ctx, "cannot create link on personal space root"),
 		}, nil
 	}
 
-	if !s.isPathAllowed(req.ResourceInfo.Path) {
+	// quick link returns the existing one if already present
+	quickLink, err := checkQuicklink(req.GetResourceInfo())
+	if err != nil {
 		return &link.CreatePublicShareResponse{
-			Status: status.NewInvalid(ctx, "share creation is not allowed for the specified path"),
+			Status: status.NewInvalidArg(ctx, "invalid quicklink value"),
 		}, nil
 	}
+	if quickLink {
+		f := []*link.ListPublicSharesRequest_Filter{publicshare.ResourceIDFilter(req.GetResourceInfo().GetId())}
+		req := link.ListPublicSharesRequest{Filters: f}
+		res, err := s.ListPublicShares(ctx, &req)
+		if err != nil || res.GetStatus().GetCode() != rpc.Code_CODE_OK {
+			return &link.CreatePublicShareResponse{
+				Status: status.NewInternal(ctx, "could not list public links"),
+			}, nil
+		}
+		for _, l := range res.GetShare() {
+			if l.Quicklink {
+				return &link.CreatePublicShareResponse{
+					Status: status.NewOK(ctx),
+					Share:  l,
+				}, nil
+			}
+		}
+	}
 
 	grant := req.GetGrant()
-	if grant != nil && s.conf.WriteableShareMustHavePassword &&
-		publicshare.IsWriteable(grant.GetPermissions()) && grant.Password == "" {
+
+	// validate expiration date
+	if grant.GetExpiration() != nil {
+		expirationDateTime := cs3TimestampToTime(grant.GetExpiration()).UTC()
+		if expirationDateTime.Before(time.Now().UTC()) {
+			msg := fmt.Sprintf("expiration date is in the past: %s", expirationDateTime.Format(time.RFC3339))
+			return &link.CreatePublicShareResponse{
+				Status: status.NewInvalidArg(ctx, msg),
+			}, nil
+		}
+	}
+
+	// enforce password if needed
+	setPassword := grant.GetPassword()
+	if enforcePassword(grant, s.conf) && len(setPassword) == 0 {
 		return &link.CreatePublicShareResponse{
-			Status: status.NewInvalid(ctx, "writeable shares must have a password protection"),
+			Status: status.NewInvalidArg(ctx, "password protection is enforced"),
 		}, nil
 	}
 
+	// validate password policy
+	if len(setPassword) > 0 {
+		if err := s.passwordValidator.Validate(setPassword); err != nil {
+			return &link.CreatePublicShareResponse{
+				Status: status.NewInvalidArg(ctx, err.Error()),
+			}, nil
+		}
+	}
+
 	u, ok := ctxpkg.ContextGetUser(ctx)
 	if !ok {
-		log.Error().Msg("error getting user from context")
+		log.Error().Msg(getUserCtxErrMsg)
 	}
 
 	res := &link.CreatePublicShareResponse{}
-	share, err := s.sm.CreatePublicShare(ctx, u, req.ResourceInfo, req.Grant)
+	share, err := s.sm.CreatePublicShare(ctx, u, req.GetResourceInfo(), req.GetGrant())
 	switch {
 	case err != nil:
 		log.Error().Err(err).Interface("request", req).Msg("could not write public share")
@@ -179,11 +315,37 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS
 }
 
 func (s *service) RemovePublicShare(ctx context.Context, req *link.RemovePublicShareRequest) (*link.RemovePublicShareResponse, error) {
+	gatewayClient, err := s.gatewaySelector.Next()
+	if err != nil {
+		return nil, err
+	}
+
 	log := appctx.GetLogger(ctx)
 	log.Info().Str("publicshareprovider", "remove").Msg("remove public share")
 
 	user := ctxpkg.ContextMustGetUser(ctx)
-	err := s.sm.RevokePublicShare(ctx, user, req.Ref)
+	ps, err := s.sm.GetPublicShare(ctx, user, req.GetRef(), false)
+	if err != nil {
+		return &link.RemovePublicShareResponse{
+			Status: status.NewInternal(ctx, "error loading public share"),
+		}, err
+	}
+	if !publicshare.IsCreatedByUser(*ps, user) {
+		sRes, err := gatewayClient.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: ps.ResourceId}})
+		if err != nil {
+			log.Err(err).Interface("resource_id", ps.ResourceId).Msg("failed to stat shared resource")
+			return &link.RemovePublicShareResponse{
+				Status: status.NewInternal(ctx, "failed to stat shared resource"),
+			}, err
+		}
+
+		if !sRes.GetInfo().GetPermissionSet().RemoveGrant {
+			return &link.RemovePublicShareResponse{
+				Status: status.NewPermissionDenied(ctx, nil, "no permission to delete public share"),
+			}, err
+		}
+	}
+	err = s.sm.RevokePublicShare(ctx, user, req.Ref)
 	if err != nil {
 		return &link.RemovePublicShareResponse{
 			Status: status.NewInternal(ctx, "error deleting public share"),
@@ -227,7 +389,7 @@ func (s *service) GetPublicShare(ctx context.Context, req *link.GetPublicShareRe
 
 	u, ok := ctxpkg.ContextGetUser(ctx)
 	if !ok {
-		log.Error().Msg("error getting user from context")
+		log.Error().Msg(getUserCtxErrMsg)
 	}
 
 	ps, err := s.sm.GetPublicShare(ctx, u, req.Ref, req.GetSign())
@@ -281,16 +443,11 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS
 
 	u, ok := ctxpkg.ContextGetUser(ctx)
 	if !ok {
-		log.Error().Msg("error getting user from context")
+		log.Error().Msg(getUserCtxErrMsg)
 	}
 
 	updateR, err := s.sm.UpdatePublicShare(ctx, u, req)
 	if err != nil {
-		if errors.Is(err, publicshare.ErrShareNeedsPassword) {
-			return &link.UpdatePublicShareResponse{
-				Status: status.NewInvalid(ctx, err.Error()),
-			}, nil
-		}
 		return &link.UpdatePublicShareResponse{
 			Status: status.NewInternal(ctx, err.Error()),
 		}, nil
@@ -302,3 +459,37 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS
 	}
 	return res, nil
 }
+
+func enforcePassword(grant *link.Grant, conf *config) bool {
+	if conf.PublicShareMustHavePassword {
+		return true
+	}
+	isReadOnly := conversions.SufficientCS3Permissions(conversions.NewViewerRole(true).CS3ResourcePermissions(), grant.GetPermissions().GetPermissions())
+	if !isReadOnly && conf.WriteableShareMustHavePassword {
+		return true
+	}
+	return false
+}
+
+func checkQuicklink(info *provider.ResourceInfo) (bool, error) {
+	if info == nil {
+		return false, nil
+	}
+	if m := info.GetArbitraryMetadata().GetMetadata(); m != nil {
+		q, ok := m["quicklink"]
+		// empty string would trigger an error in ParseBool()
+		if !ok || q == "" {
+			return false, nil
+		}
+		quickLink, err := strconv.ParseBool(q)
+		if err != nil {
+			return false, err
+		}
+		return quickLink, nil
+	}
+	return false, nil
+}
+
+func cs3TimestampToTime(t *typesv1beta1.Timestamp) time.Time {
+	return time.Unix(int64(t.Seconds), int64(t.Nanos))
+}

internal/grpc/services/usershareprovider/usershareprovider.go

@@ -156,13 +156,13 @@ func (s *service) CreateShare(ctx context.Context, req *collaboration.CreateShar
 		req.GetGrant().GetPermissions().GetPermissions(),
 	); !shareCreationAllowed {
 		return &collaboration.CreateShareResponse{
-			Status: status.NewInvalid(ctx, "insufficient permissions to create that kind of share"),
+			Status: status.NewPermissionDenied(ctx, nil, "insufficient permissions to create that kind of share"),
 		}, nil
 	}
 
 	if !s.isPathAllowed(req.GetResourceInfo().GetPath()) {
 		return &collaboration.CreateShareResponse{
-			Status: status.NewInvalid(ctx, "share creation is not allowed for the specified path"),
+			Status: status.NewFailedPrecondition(ctx, nil, "share creation is not allowed for the specified path"),
 		}, nil
 	}
 

internal/grpc/services/usershareprovider/usershareprovider_test.go

@@ -115,7 +115,7 @@ var _ = Describe("user share provider service", func() {
 				"insufficient permissions",
 				conversions.RoleFromName("spaceeditor", true).CS3ResourcePermissions(),
 				conversions.RoleFromName("manager", true).CS3ResourcePermissions(),
-				rpcpb.Code_CODE_INVALID_ARGUMENT,
+				rpcpb.Code_CODE_PERMISSION_DENIED,
 				0,
 			),
 			Entry(