diff --git a/changelog/unreleased/enforce-permissions.md b/changelog/unreleased/enforce-permissions.md new file mode 100644 index 0000000000..63ae23ff18 --- /dev/null +++ b/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 diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index 492348d407..d36aebd518 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -39,6 +39,7 @@ import ( ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" rstatus "github.com/cs3org/reva/v2/pkg/rgrpc/status" + "github.com/cs3org/reva/v2/pkg/utils" "github.com/rs/zerolog" ) @@ -217,6 +218,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, "Favorites.Write", 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 +287,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, "Favorites.Write", 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) diff --git a/internal/http/services/owncloud/ocdav/report.go b/internal/http/services/owncloud/ocdav/report.go index 02f6886767..aaeaef7f07 100644 --- a/internal/http/services/owncloud/ocdav/report.go +++ b/internal/http/services/owncloud/ocdav/report.go @@ -30,6 +30,7 @@ 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/utils" ) const ( @@ -73,20 +74,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, "Favorites.List", 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]}}) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go index 36d038ee8e..cc0a9f6476 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go @@ -26,13 +26,13 @@ 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/rgrpc/todo/pool" + "github.com/cs3org/reva/v2/pkg/utils" "github.com/huandu/xstrings" "github.com/rs/zerolog/log" @@ -69,15 +69,7 @@ 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, "PublicLink.Write", c) if err != nil { return nil, &ocsError{ Code: response.MetaServerError.StatusCode, @@ -85,8 +77,7 @@ func (h *Handler) createPublicLinkShare(w http.ResponseWriter, r *http.Request, 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 +326,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, "PublicLink.Write", 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 +693,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, "ReadOnlyPublicLinkPassword.Delete", 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") } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index c5114128ba..b03ab761c0 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -233,6 +233,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, "Shares.Write", 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 +737,18 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, share *col return } + ok, err := utils.CheckPermission(ctx, "Shares.Write", 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) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go index e820f1aed1..b207433be1 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go +++ b/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() { diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go index e88ef16a67..16b834e531 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/user.go @@ -163,6 +163,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, "Shares.Write", 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, diff --git a/pkg/permission/manager/demo/demo.go b/pkg/permission/manager/demo/demo.go index fd55783dd5..cc28024463 100644 --- a/pkg/permission/manager/demo/demo.go +++ b/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 diff --git a/pkg/permission/permission.go b/pkg/permission/permission.go index 405f72bbc8..6248032809 100644 --- a/pkg/permission/permission.go +++ b/pkg/permission/permission.go @@ -29,6 +29,12 @@ 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" ) // Manager defines the interface for the permission service driver diff --git a/pkg/utils/grpc.go b/pkg/utils/grpc.go index 0be65a67db..ae49c7ab8e 100644 --- a/pkg/utils/grpc.go +++ b/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)