fix enforce-password issue for public links

.drone.env

@@ -1,4 +1,4 @@
 # The test runner source for API tests
-APITESTS_COMMITID=fc686d8cd827fe7b2bea55894c781fb72ad24865
+APITESTS_COMMITID=3e2180d5e4187c26003b2a6f7f026b6694a6f084
 APITESTS_BRANCH=master
 APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git

changelog/unreleased/fix-enforce-password.md

@@ -0,0 +1,6 @@
+Bugfix: fix enforce-password issue
+
+Fix updating public share without password when enforce-password is enabled 
+
+https://github.com/cs3org/reva/pull/3970
+https://github.com/owncloud/ocis/issues/6476

changelog/unreleased/fix-panic.md

@@ -1,4 +1,3 @@
 Bugfix: fix panic
 
 https://github.com/cs3org/reva/pull/3955
-

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go

@@ -23,20 +23,21 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 
 	permissionsv1beta1 "github.com/cs3org/go-cs3apis/cs3/permissions/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"
 	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/rs/zerolog/log"
 
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/conversions"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/response"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/publicshare"
-	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/pkg/errors"
 )
 
@@ -275,9 +276,10 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh
 
 func (h *Handler) isPublicShare(r *http.Request, oid string) (*link.PublicShare, bool) {
 	logger := appctx.GetLogger(r.Context())
-	client, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+	client, err := h.getClient()
 	if err != nil {
 		logger.Err(err)
+		return nil, false
 	}
 
 	psRes, err := client.GetPublicShare(r.Context(), &link.GetPublicShareRequest{
@@ -301,7 +303,7 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar
 	updates := []*link.UpdatePublicShareRequest_Update{}
 	logger := appctx.GetLogger(r.Context())
 
-	gwC, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+	gwC, err := h.getClient()
 	if err != nil {
 		log.Err(err).Str("shareID", share.GetId().GetOpaqueId()).Msg("updatePublicShare")
 		response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, "error getting a connection to the gateway service", nil)
@@ -447,6 +449,14 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar
 
 	// Password
 	newPassword, ok := r.Form["password"]
+	// enforcePassword
+	if h.enforcePassword(permKey) {
+		if (!ok && !share.PasswordProtected) || (ok && len(strings.TrimSpace(newPassword[0])) == 0) {
+			response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, "missing required password", err)
+			return
+		}
+	}
+
 	// update or clear password
 	if ok {
 		updatesFound = true
@@ -500,7 +510,7 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar
 func (h *Handler) removePublicShare(w http.ResponseWriter, r *http.Request, share *link.PublicShare) {
 	ctx := r.Context()
 
-	c, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+	c, err := h.getClient()
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
 		return
@@ -548,6 +558,39 @@ func (h *Handler) removePublicShare(w http.ResponseWriter, r *http.Request, shar
 	response.WriteOCSSuccess(w, r, nil)
 }
 
+// enforcePassword validate Password enforce based on configuration
+// read_only:           1
+// read_write:          3 or 5
+// read_write_delete:   15
+// upload_only:         4
+func (h *Handler) enforcePassword(pk *int) bool {
+	if pk == nil {
+		return false
+	}
+	p, err := conversions.NewPermissions(decreasePermissionsIfNecessary(*pk))
+	if err != nil {
+		return false
+	}
+	if h.publicPasswordEnforced.EnforcedForReadOnly &&
+		p == conversions.PermissionRead {
+		return true
+	}
+	if h.publicPasswordEnforced.EnforcedForReadWrite &&
+		(p == conversions.PermissionRead|conversions.PermissionWrite ||
+			p == conversions.PermissionRead|conversions.PermissionCreate) {
+		return true
+	}
+	if h.publicPasswordEnforced.EnforcedForReadWriteDelete &&
+		p == conversions.PermissionRead|conversions.PermissionWrite|conversions.PermissionCreate|conversions.PermissionDelete {
+		return true
+	}
+	if h.publicPasswordEnforced.EnforcedForUploadOnly &&
+		p == conversions.PermissionCreate {
+		return true
+	}
+	return false
+}
+
 // for public links oc10 api decreases all permissions to read: stay compatible!
 func decreasePermissionsIfNecessary(perm int) int {
 	if perm == int(conversions.PermissionAll) {

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public_test.go

@@ -0,0 +1,103 @@
+package shares
+
+import (
+	"testing"
+)
+
+func TestHandler_enforcePassword(t *testing.T) {
+	tests := []struct {
+		name    string
+		h       *Handler
+		permKey int
+		exp     bool
+	}{
+		{
+			name: "enforce permission 1",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForReadOnly: true,
+				},
+			},
+			permKey: 1,
+			exp:     true,
+		},
+		{
+			name: "enforce permission 3",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForReadWrite: true,
+				},
+			},
+			permKey: 3,
+			exp:     true,
+		},
+		{
+			name: "enforce permission 4",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForUploadOnly: true,
+				},
+			},
+			permKey: 4,
+			exp:     true,
+		},
+		{
+			name: "enforce permission 5",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForReadWrite: true,
+				},
+			},
+			permKey: 5,
+			exp:     true,
+		},
+		{
+			name: "enforce permission 15",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForReadWriteDelete: true,
+				},
+			},
+			permKey: 15,
+			exp:     true,
+		},
+		{
+			name: "enforce permission 3 failed",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForReadOnly: true,
+				},
+			},
+			permKey: 3,
+			exp:     false,
+		},
+		{
+			name: "enforce permission 8 failed",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForReadWriteDelete: true,
+					EnforcedForUploadOnly:      true,
+				},
+			},
+			permKey: 8,
+			exp:     false,
+		},
+		{
+			name: "enforce permission 11 failed",
+			h: &Handler{
+				publicPasswordEnforced: passwordEnforced{
+					EnforcedForReadWriteDelete: true,
+				},
+			},
+			permKey: 11,
+			exp:     false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.h.enforcePassword(&tt.permKey) != tt.exp {
+				t.Errorf("enforcePassword(\"%v\") returned %v instead of expected %v", tt.permKey, tt.h.enforcePassword(&tt.permKey), tt.exp)
+			}
+		})
+	}
+}

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -86,6 +86,7 @@ type Handler struct {
 	statCache                             cache.StatCache
 	deniable                              bool
 	resharing                             bool
+	publicPasswordEnforced                passwordEnforced
 
 	getClient GatewayClientGetter
 }
@@ -103,6 +104,13 @@ type ocsError struct {
 	Message string
 }
 
+type passwordEnforced struct {
+	EnforcedForReadOnly        bool
+	EnforcedForReadWrite       bool
+	EnforcedForReadWriteDelete bool
+	EnforcedForUploadOnly      bool
+}
+
 func getCacheWarmupManager(c *config.Config) (sharecache.Warmup, error) {
 	if f, ok := warmupreg.NewFuncs[c.CacheWarmupDriver]; ok {
 		return f(c.CacheWarmupDrivers[c.CacheWarmupDriver])
@@ -129,6 +137,7 @@ func (h *Handler) Init(c *config.Config) {
 	_ = h.userIdentifierCache.SetTTL(time.Second * time.Duration(c.UserIdentifierCacheTTL))
 	h.deniable = c.EnableDenials
 	h.resharing = resharing(c)
+	h.publicPasswordEnforced = publicPwdEnforced(c)
 
 	h.statCache = cache.GetStatCache(c.StatCacheStore, c.StatCacheNodes, c.StatCacheDatabase, "stat", time.Duration(c.StatCacheTTL)*time.Second, c.StatCacheSize)
 	if c.CacheWarmupDriver != "" {
@@ -503,7 +512,7 @@ func (h *Handler) GetShare(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	sublog := appctx.GetLogger(ctx).With().Str("shareID", shareID).Logger()
 
-	client, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+	client, err := h.getClient()
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
 		return
@@ -695,7 +704,7 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, share *col
 	ctx := r.Context()
 	sublog := appctx.GetLogger(ctx).With().Str("shareID", share.GetId().GetOpaqueId()).Logger()
 
-	client, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+	client, err := h.getClient()
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
 		return
@@ -1110,7 +1119,7 @@ func (h *Handler) addFilters(w http.ResponseWriter, r *http.Request, ref *provid
 	ctx := r.Context()
 
 	// first check if the file exists
-	client, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+	client, err := h.getClient()
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
 		return nil, nil, err
@@ -1159,7 +1168,7 @@ func (h *Handler) addFileInfo(ctx context.Context, s *conversions.ShareData, inf
 	switch {
 	case h.sharePrefix == "/":
 		s.FileTarget = info.Path
-		client, err := pool.GetGatewayServiceClient(h.gatewayAddr)
+		client, err := h.getClient()
 		if err == nil {
 			gpRes, err := client.GetPath(ctx, &provider.GetPathRequest{
 				ResourceId: info.Id,
@@ -1538,6 +1547,23 @@ func (h *Handler) granteeExists(ctx context.Context, g *provider.Grantee, rid *p
 	return false, nil
 }
 
+func publicPwdEnforced(c *config.Config) passwordEnforced {
+	enf := passwordEnforced{}
+	if c == nil ||
+		c.Capabilities.Capabilities == nil ||
+		c.Capabilities.Capabilities.FilesSharing == nil ||
+		c.Capabilities.Capabilities.FilesSharing.Public == nil ||
+		c.Capabilities.Capabilities.FilesSharing.Public.Password == nil ||
+		c.Capabilities.Capabilities.FilesSharing.Public.Password.EnforcedFor == nil {
+		return enf
+	}
+	enf.EnforcedForReadOnly = bool(c.Capabilities.Capabilities.FilesSharing.Public.Password.EnforcedFor.ReadOnly)
+	enf.EnforcedForReadWrite = bool(c.Capabilities.Capabilities.FilesSharing.Public.Password.EnforcedFor.ReadWrite)
+	enf.EnforcedForReadWriteDelete = bool(c.Capabilities.Capabilities.FilesSharing.Public.Password.EnforcedFor.ReadWriteDelete)
+	enf.EnforcedForUploadOnly = bool(c.Capabilities.Capabilities.FilesSharing.Public.Password.EnforcedFor.UploadOnly)
+	return enf
+}
+
 // sufficientPermissions returns true if the `existing` permissions contain the `requested` permissions
 func sufficientPermissions(existing, requested *provider.ResourcePermissions, islink bool) bool {
 	ep := conversions.RoleFromResourcePermissions(existing, islink).OCSPermissions()