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..9c920c170b 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/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) diff --git a/internal/http/services/owncloud/ocdav/report.go b/internal/http/services/owncloud/ocdav/report.go index 02f6886767..1eeaa20661 100644 --- a/internal/http/services/owncloud/ocdav/report.go +++ b/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]}}) 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..051783b541 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,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,15 +70,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, permission.WritePublicLink, c) if err != nil { return nil, &ocsError{ Code: response.MetaServerError.StatusCode, @@ -85,8 +78,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 +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") } 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..1ec428fce0 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 @@ -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) 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..d67d0f2d2e 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 @@ -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, 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..fb82add443 100644 --- a/pkg/permission/permission.go +++ b/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 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)