diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider.go b/internal/grpc/services/publicshareprovider/publicshareprovider.go index 33b7bcd3dc5..bbe429142c6 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider.go @@ -21,9 +21,11 @@ package publicshareprovider import ( "context" "regexp" + "strconv" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" + "github.com/cs3org/reva/v2/pkg/password" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" "google.golang.org/grpc" @@ -38,6 +40,8 @@ import ( "github.com/cs3org/reva/v2/pkg/rgrpc/status" ) +const getUserCtxErrMsg = "error getting user from context" + func init() { rgrpc.Register("publicshareprovider", New) } @@ -48,6 +52,17 @@ type config struct { 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() { @@ -60,6 +75,7 @@ type service struct { conf *config sm publicshare.Manager allowedPathsForShares []*regexp.Regexp + passwordValidator password.Validator } func getShareManager(c *config) (publicshare.Manager, error) { @@ -90,6 +106,15 @@ func parseConfig(m map[string]interface{}) (*config, error) { 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 conf") + 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) { @@ -97,6 +122,10 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { if err != nil { return nil, err } + p, err := parsePasswordPolicy(c.PasswordPolicy) + if err != nil { + return nil, err + } c.init() @@ -118,11 +147,26 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { conf: c, sm: sm, 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,29 +183,77 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS log := appctx.GetLogger(ctx) log.Info().Str("publicshareprovider", "create").Msg("create public share") + // check that user has share permissions + // TODO Check if we need to stat again, because the client could send fake permissions + if !req.GetResourceInfo().GetPermissionSet().AddGrant { + err := errors.New("no share permission") + return &link.CreatePublicShareResponse{ + Status: status.NewPermissionDenied(ctx, err, err.Error()), + }, nil + } + if !conversions.SufficientCS3Permissions(req.GetResourceInfo().GetPermissionSet(), req.GetGrant().GetPermissions().GetPermissions()) { return &link.CreatePublicShareResponse{ - Status: status.NewInvalid(ctx, "insufficient permissions to create that kind of share"), + Status: status.NewFailedPrecondition(ctx, nil, "insufficient permissions to create that kind of share"), }, nil } if !s.isPathAllowed(req.ResourceInfo.Path) { return &link.CreatePublicShareResponse{ - 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 + } + + // check that this is a valid share + if req.GetResourceInfo().GetId().GetOpaqueId() == req.GetResourceInfo().GetId().GetSpaceId() && + req.GetResourceInfo().GetSpace().GetSpaceType() == "personal" { + return &link.CreatePublicShareResponse{ + Status: status.NewFailedPrecondition(ctx, nil, "cannot create link on space root"), }, nil } + // check if a quicklink should be created + quickString, ok := req.GetResourceInfo().GetArbitraryMetadata().GetMetadata()["quicklink"] + // no need to check the error - defaults to zero value! + quick, _ := strconv.ParseBool(quickString) + if ok && quick { + f := []*link.ListPublicSharesRequest_Filter{publicshare.ResourceIDFilter(req.GetResourceInfo().GetId())} + req := link.ListPublicSharesRequest{Filters: f} + res, err := s.ListPublicShares(ctx, &req) + if err != nil || res.Status.Code != 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 == "" { + 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.NewFailedPrecondition(ctx, nil, "password protection is enforced"), }, nil } + if len(setPassword) > 0 { + if err := s.passwordValidator.Validate(setPassword); err != nil { + return &link.CreatePublicShareResponse{ + Status: status.NewFailedPrecondition(ctx, nil, 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{} @@ -227,7 +319,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 +373,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 +389,14 @@ 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 +} diff --git a/internal/grpc/services/usershareprovider/usershareprovider.go b/internal/grpc/services/usershareprovider/usershareprovider.go index 313928c39e9..bc4ccd46d06 100644 --- a/internal/grpc/services/usershareprovider/usershareprovider.go +++ b/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 } diff --git a/pkg/publicshare/manager/json/json.go b/pkg/publicshare/manager/json/json.go index 5d7673b8244..a186f955eab 100644 --- a/pkg/publicshare/manager/json/json.go +++ b/pkg/publicshare/manager/json/json.go @@ -76,7 +76,7 @@ func NewFile(c map[string]interface{}) (publicshare.Manager, error) { return nil, err } - return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p, conf.WriteableShareMustHavePassword) + return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p) } // NewMemory returns a new in-memory public shares manager. @@ -93,7 +93,7 @@ func NewMemory(c map[string]interface{}) (publicshare.Manager, error) { return nil, err } - return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p, conf.WriteableShareMustHavePassword) + return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p) } // NewCS3 returns a new cs3 public shares manager. @@ -115,19 +115,18 @@ func NewCS3(c map[string]interface{}) (publicshare.Manager, error) { return nil, err } - return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p, conf.WriteableShareMustHavePassword) + return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p) } // New returns a new public share manager instance -func New(gwAddr string, pwHashCost, janitorRunInterval int, enableCleanup bool, p persistence.Persistence, writeableShareMustHavePassword bool) (publicshare.Manager, error) { +func New(gwAddr string, pwHashCost, janitorRunInterval int, enableCleanup bool, p persistence.Persistence) (publicshare.Manager, error) { m := &manager{ - gatewayAddr: gwAddr, - mutex: &sync.Mutex{}, - passwordHashCost: pwHashCost, - janitorRunInterval: janitorRunInterval, - enableExpiredSharesCleanup: enableCleanup, - persistence: p, - writeableShareMustHavePassword: writeableShareMustHavePassword, + gatewayAddr: gwAddr, + mutex: &sync.Mutex{}, + passwordHashCost: pwHashCost, + janitorRunInterval: janitorRunInterval, + enableExpiredSharesCleanup: enableCleanup, + persistence: p, } go m.startJanitorRun() @@ -140,6 +139,7 @@ type commonConfig struct { JanitorRunInterval int `mapstructure:"janitor_run_interval"` EnableExpiredSharesCleanup bool `mapstructure:"enable_expired_shares_cleanup"` WriteableShareMustHavePassword bool `mapstructure:"writeable_share_must_have_password"` + PublicShareMustHavePassword bool `mapstructure:"public_share_must_have_password"` } type fileConfig struct { @@ -171,10 +171,9 @@ type manager struct { mutex *sync.Mutex persistence persistence.Persistence - passwordHashCost int - janitorRunInterval int - enableExpiredSharesCleanup bool - writeableShareMustHavePassword bool + passwordHashCost int + janitorRunInterval int + enableExpiredSharesCleanup bool } func (m *manager) startJanitorRun() { @@ -343,12 +342,6 @@ func (m *manager) UpdatePublicShare(ctx context.Context, u *user.User, req *link old, _ := json.Marshal(share.Permissions) new, _ := json.Marshal(req.Update.GetGrant().Permissions) - if m.writeableShareMustHavePassword && - publicshare.IsWriteable(req.GetUpdate().GetGrant().GetPermissions()) && - (!share.PasswordProtected && req.GetUpdate().GetGrant().GetPassword() == "") { - return nil, publicshare.ErrShareNeedsPassword - } - if req.GetUpdate().GetGrant().GetPassword() != "" { passwordChanged = true h, err := bcrypt.GenerateFromPassword([]byte(req.Update.GetGrant().Password), m.passwordHashCost) @@ -369,10 +362,6 @@ func (m *manager) UpdatePublicShare(ctx context.Context, u *user.User, req *link case link.UpdatePublicShareRequest_Update_TYPE_PASSWORD: passwordChanged = true if req.Update.GetGrant().Password == "" { - if m.writeableShareMustHavePassword && publicshare.IsWriteable(share.Permissions) { - return nil, publicshare.ErrShareNeedsPassword - } - share.PasswordProtected = false newPasswordEncoded = "" } else { diff --git a/pkg/publicshare/manager/json/json_test.go b/pkg/publicshare/manager/json/json_test.go index d3922573366..f9385896ae0 100644 --- a/pkg/publicshare/manager/json/json_test.go +++ b/pkg/publicshare/manager/json/json_test.go @@ -190,7 +190,7 @@ var _ = Describe("Json", func() { persistence := cs3.New(storage) Expect(persistence.Init(context.Background())).To(Succeed()) - m, err = json.New("https://localhost:9200", 11, 60, false, persistence, false) + m, err = json.New("https://localhost:9200", 11, 60, false, persistence) Expect(err).ToNot(HaveOccurred()) ctx = ctxpkg.ContextSetUser(context.Background(), user1) @@ -228,7 +228,7 @@ var _ = Describe("Json", func() { p := cs3.New(storage) Expect(p.Init(context.Background())).To(Succeed()) - m, err = json.New("https://localhost:9200", 11, 60, false, p, false) + m, err = json.New("https://localhost:9200", 11, 60, false, p) Expect(err).ToNot(HaveOccurred()) ps, err := m.ListPublicShares(ctx, user1, []*link.ListPublicSharesRequest_Filter{}, false) diff --git a/pkg/publicshare/publicshare.go b/pkg/publicshare/publicshare.go index 187c3165c59..4eca5cdccb8 100644 --- a/pkg/publicshare/publicshare.go +++ b/pkg/publicshare/publicshare.go @@ -24,7 +24,6 @@ import ( "crypto/sha256" "crypto/sha512" "encoding/hex" - "errors" "time" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -41,11 +40,6 @@ const ( StorageIDFilterType link.ListPublicSharesRequest_Filter_Type = 4 ) -var ( - // ErrShareNeedsPassword is an error which is returned when a public share must have a password. - ErrShareNeedsPassword = errors.New("the public share needs to have a password") -) - // Manager manipulates public shares. type Manager interface { CreatePublicShare(ctx context.Context, u *user.User, md *provider.ResourceInfo, g *link.Grant) (*link.PublicShare, error)