Merge pull request #4325 from kobergj/EnforcePermissions

Enforce Favorite&Share Permissions

changelog/unreleased/enforce-permissions.md

@@ -0,0 +1,5 @@
+Enhancement: Enforce Permissions
+
+Enforce the new `Favorites.List` `Favorites.Write` and `Shares.Write` Permissions
+
+https://github.com/cs3org/reva/pull/4325

internal/http/services/owncloud/ocdav/proppatch.go

@@ -38,7 +38,9 @@ import (
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
+	"github.com/cs3org/reva/v2/pkg/permission"
 	rstatus "github.com/cs3org/reva/v2/pkg/rgrpc/status"
+	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/rs/zerolog"
 )
 
@@ -217,6 +219,17 @@ func (s *svc) handleProppatch(ctx context.Context, w http.ResponseWriter, r *htt
 						return nil, nil, false
 					}
 					currentUser := ctxpkg.ContextMustGetUser(ctx)
+					ok, err := utils.CheckPermission(ctx, permission.WriteFavorites, client)
+					if err != nil {
+						log.Error().Err(err).Msg("error checking permission")
+						w.WriteHeader(http.StatusInternalServerError)
+						return nil, nil, false
+					}
+					if !ok {
+						log.Info().Interface("user", currentUser).Msg("user not allowed to unset favorite")
+						w.WriteHeader(http.StatusForbidden)
+						return nil, nil, false
+					}
 					err = s.favoritesManager.UnsetFavorite(ctx, currentUser.Id, statRes.Info)
 					if err != nil {
 						w.WriteHeader(http.StatusInternalServerError)
@@ -275,6 +288,17 @@ func (s *svc) handleProppatch(ctx context.Context, w http.ResponseWriter, r *htt
 						return nil, nil, false
 					}
 					currentUser := ctxpkg.ContextMustGetUser(ctx)
+					ok, err := utils.CheckPermission(ctx, permission.WriteFavorites, client)
+					if err != nil {
+						log.Error().Err(err).Msg("error checking permission")
+						w.WriteHeader(http.StatusInternalServerError)
+						return nil, nil, false
+					}
+					if !ok {
+						log.Info().Interface("user", currentUser).Msg("user not allowed to set favorite")
+						w.WriteHeader(http.StatusForbidden)
+						return nil, nil, false
+					}
 					err = s.favoritesManager.SetFavorite(ctx, currentUser.Id, statRes.Info)
 					if err != nil {
 						w.WriteHeader(http.StatusInternalServerError)

internal/http/services/owncloud/ocdav/report.go

@@ -30,6 +30,8 @@ import (
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
+	"github.com/cs3org/reva/v2/pkg/permission"
+	"github.com/cs3org/reva/v2/pkg/utils"
 )
 
 const (
@@ -73,20 +75,31 @@ func (s *svc) doFilterFiles(w http.ResponseWriter, r *http.Request, ff *reportFi
 
 	if ff.Rules.Favorite {
 		// List the users favorite resources.
+		client, err := s.gatewaySelector.Next()
+		if err != nil {
+			log.Error().Err(err).Msg("error selecting next gateway client")
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
 		currentUser := ctxpkg.ContextMustGetUser(ctx)
-		favorites, err := s.favoritesManager.ListFavorites(ctx, currentUser.Id)
+		ok, err := utils.CheckPermission(ctx, permission.ListFavorites, client)
 		if err != nil {
-			log.Error().Err(err).Msg("error getting favorites")
+			log.Error().Err(err).Msg("error checking permission")
 			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
-
-		client, err := s.gatewaySelector.Next()
+		if !ok {
+			log.Info().Interface("user", currentUser).Msg("user not allowed to list favorites")
+			w.WriteHeader(http.StatusForbidden)
+			return
+		}
+		favorites, err := s.favoritesManager.ListFavorites(ctx, currentUser.Id)
 		if err != nil {
-			log.Error().Err(err).Msg("error selecting next gateway client")
+			log.Error().Err(err).Msg("error getting favorites")
 			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
+
 		infos := make([]*provider.ResourceInfo, 0, len(favorites))
 		for i := range favorites {
 			statRes, err := client.Stat(ctx, &providerv1beta1.StatRequest{Ref: &providerv1beta1.Reference{ResourceId: favorites[i]}})

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

@@ -26,13 +26,14 @@ import (
 	"strconv"
 
 	userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
-	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/conversions"
+	"github.com/cs3org/reva/v2/pkg/permission"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/huandu/xstrings"
 	"github.com/rs/zerolog/log"
 
@@ -69,24 +70,15 @@ func (h *Handler) createPublicLinkShare(w http.ResponseWriter, r *http.Request,
 
 	// NOTE: one is allowed to create an internal link without the `Publink.Write` permission
 	if permKey != nil && *permKey != 0 {
-		user := ctxpkg.ContextMustGetUser(ctx)
-		resp, err := c.CheckPermission(ctx, &permissionsv1beta1.CheckPermissionRequest{
-			SubjectRef: &permissionsv1beta1.SubjectReference{
-				Spec: &permissionsv1beta1.SubjectReference_UserId{
-					UserId: user.Id,
-				},
-			},
-			Permission: "PublicLink.Write",
-		})
+		ok, err := utils.CheckPermission(ctx, permission.WritePublicLink, c)
 		if err != nil {
 			return nil, &ocsError{
 				Code:    response.MetaServerError.StatusCode,
 				Message: "failed to check user permission",
 				Error:   err,
 			}
 		}
-
-		if resp.Status.Code != rpc.Code_CODE_OK {
+		if !ok {
 			return nil, &ocsError{
 				Code:    response.MetaForbidden.StatusCode,
 				Message: "user is not allowed to create a public link",
@@ -335,20 +327,12 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar
 
 	// NOTE: you are allowed to update a link TO a public link without the `PublicLink.Write` permission if you created it yourself
 	if (permKey != nil && *permKey != 0) || !createdByUser {
-		resp, err := gwC.CheckPermission(ctx, &permissionsv1beta1.CheckPermissionRequest{
-			SubjectRef: &permissionsv1beta1.SubjectReference{
-				Spec: &permissionsv1beta1.SubjectReference_UserId{
-					UserId: user.Id,
-				},
-			},
-			Permission: "PublicLink.Write",
-		})
+		ok, err := utils.CheckPermission(ctx, permission.WritePublicLink, gwC)
 		if err != nil {
 			response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "failed to check user permission", err)
 			return
 		}
-
-		if resp.Status.Code != rpc.Code_CODE_OK {
+		if !ok {
 			response.WriteOCSError(w, r, response.MetaForbidden.StatusCode, "user is not allowed to update the public link", nil)
 			return
 		}
@@ -710,20 +694,12 @@ func (h *Handler) checkPasswordEnforcement(ctx context.Context, user *userv1beta
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "could not check permission", err)
 		return errors.New("could not check permission")
 	}
-	resp, err := gwC.CheckPermission(ctx, &permissionsv1beta1.CheckPermissionRequest{
-		SubjectRef: &permissionsv1beta1.SubjectReference{
-			Spec: &permissionsv1beta1.SubjectReference_UserId{
-				UserId: user.Id,
-			},
-		},
-		Permission: "ReadOnlyPublicLinkPassword.Delete",
-	})
+	ok, err := utils.CheckPermission(ctx, permission.DeleteReadOnlyPassword, gwC)
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "failed to check user permission", err)
 		return errors.New("failed to check user permission")
 	}
-
-	if resp.Status.Code != rpc.Code_CODE_OK {
+	if !ok {
 		response.WriteOCSError(w, r, response.MetaForbidden.StatusCode, "user is not allowed to delete the password from the public link", nil)
 		return errors.New("user is not allowed to delete the password from the public link")
 	}

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

@@ -42,6 +42,7 @@ import (
 	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/conversions"
 	"github.com/cs3org/reva/v2/pkg/password"
+	"github.com/cs3org/reva/v2/pkg/permission"
 	"github.com/go-chi/chi/v5"
 	"github.com/rs/zerolog"
 	"google.golang.org/grpc/metadata"
@@ -233,6 +234,18 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) {
 	}
 	sublog := appctx.GetLogger(ctx).With().Interface("ref", ref).Logger()
 
+	ok, err := utils.CheckPermission(ctx, permission.WriteShare, client)
+	if err != nil {
+		sublog.Error().Err(err).Msg("error checking user permissions")
+		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error checking user permissions", err)
+		return
+	}
+	if !ok {
+		sublog.Debug().Interface("user", ctxpkg.ContextMustGetUser(ctx).Id).Msg("user not allowed to create share")
+		response.WriteOCSError(w, r, response.MetaForbidden.StatusCode, "permission denied", nil)
+		return
+	}
+
 	statReq := provider.StatRequest{Ref: &ref, FieldMask: &fieldmaskpb.FieldMask{Paths: []string{"space"}}}
 	statRes, err := client.Stat(ctx, &statReq)
 	if err != nil {
@@ -725,6 +738,18 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, share *col
 		return
 	}
 
+	ok, err := utils.CheckPermission(ctx, permission.WriteShare, client)
+	if err != nil {
+		sublog.Error().Err(err).Msg("error checking user permissions")
+		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error checking user permissions", err)
+		return
+	}
+	if !ok {
+		sublog.Debug().Interface("user", ctxpkg.ContextMustGetUser(ctx).Id).Msg("user not allowed to create share")
+		response.WriteOCSError(w, r, response.MetaForbidden.StatusCode, "permission denied", nil)
+		return
+	}
+
 	info, status, err := h.getResourceInfoByID(ctx, client, share.ResourceId)
 	if err != nil || status.Code != rpc.Code_CODE_OK {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err)

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

@@ -107,6 +107,10 @@ var _ = Describe("The ocs API", func() {
 			gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(&collaboration.ListSharesResponse{
 				Status: status.NewOK(context.Background()),
 			}, nil)
+
+			gatewayClient.On("CheckPermission", mock.Anything, mock.Anything).Return(&permissions.CheckPermissionResponse{
+				Status: status.NewOK(context.Background()),
+			}, nil)
 		})
 
 		Context("when sharing the personal space root", func() {

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

@@ -32,6 +32,7 @@ import (
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/conversions"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
+	"github.com/cs3org/reva/v2/pkg/permission"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/v2/pkg/utils"
 )
@@ -163,6 +164,17 @@ func (h *Handler) removeUserShare(w http.ResponseWriter, r *http.Request, share
 		return
 	}
 
+	// TODO: should we use Share.Delete here?
+	ok, err := utils.CheckPermission(ctx, permission.WriteShare, uClient)
+	if err != nil {
+		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error checking user permissions", err)
+		return
+	}
+	if !ok {
+		response.WriteOCSError(w, r, response.MetaForbidden.StatusCode, "permission denied", nil)
+		return
+	}
+
 	shareRef := &collaboration.ShareReference{
 		Spec: &collaboration.ShareReference_Id{
 			Id: share.Id,

pkg/permission/manager/demo/demo.go

@@ -47,6 +47,15 @@ func (m manager) CheckPermission(perm string, subject string, ref *provider.Refe
 	case permission.ListAllSpaces:
 		// TODO introduce an admin role to allow listing all spaces
 		return false
+	case permission.WriteShare:
+		// TODO guest accounts cannot share
+		return true
+	case permission.ListFavorites:
+		// TODO guest accounts cannot list favorites
+		return true
+	case permission.WriteFavorites:
+		// TODO guest accounts cannot write favorites
+		return true
 	default:
 		// We can currently return false all the time.
 		// Once we beginn testing roles we need to somehow check the roles of the users here

pkg/permission/permission.go

@@ -29,6 +29,14 @@ const (
 	CreateSpace string = "Drives.Create"
 	// WritePublicLink is the hardcoded name for the PublicLink.Write permission
 	WritePublicLink string = "PublicLink.Write"
+	// WriteShare is the hardcoded name for the Shares.Write permission
+	WriteShare string = "Shares.Write"
+	// ListFavorites is the hardcoded name for the Favorites.List permission
+	ListFavorites string = "Favorites.List"
+	// WriteFavorites is the hardcoded name for the Favorites.Write permission
+	WriteFavorites string = "Favorites.Write"
+	// DeleteReadOnlyPassword is the hardcoded name for the ReadOnlyPublicLinkPassword.Delete permission
+	DeleteReadOnlyPassword string = "ReadOnlyPublicLinkPassword.Delete"
 )
 
 // Manager defines the interface for the permission service driver

pkg/utils/grpc.go

@@ -9,8 +9,10 @@ import (
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	group "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
 	user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
+	permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	revactx "github.com/cs3org/reva/v2/pkg/ctx"
 
 	"google.golang.org/grpc/metadata"
@@ -164,6 +166,20 @@ func GetResource(ctx context.Context, ref *storageprovider.Reference, gwc gatewa
 	return res.GetInfo(), nil
 }
 
+// CheckPermission checks if the user role contains the given permission
+func CheckPermission(ctx context.Context, perm string, gwc gateway.GatewayAPIClient) (bool, error) {
+	user := ctxpkg.ContextMustGetUser(ctx)
+	resp, err := gwc.CheckPermission(ctx, &permissions.CheckPermissionRequest{
+		SubjectRef: &permissions.SubjectReference{
+			Spec: &permissions.SubjectReference_UserId{
+				UserId: user.Id,
+			},
+		},
+		Permission: perm,
+	})
+	return resp.GetStatus().GetCode() == rpc.Code_CODE_OK, err
+}
+
 // IsStatusCodeError returns true if `err` was caused because of status code `code`
 func IsStatusCodeError(err error, code rpc.Code) bool {
 	sce, ok := err.(statusCodeError)