Skip to content

Commit

Permalink
refactor: remove password validation from json share manager
Browse files Browse the repository at this point in the history
  • Loading branch information
micbar committed Dec 6, 2023
1 parent f73ddd1 commit 3dca758
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 38 deletions.
47 changes: 17 additions & 30 deletions pkg/publicshare/manager/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -115,31 +115,29 @@ 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()
return m, nil
}

type commonConfig struct {
GatewayAddr string `mapstructure:"gateway_addr"`
SharePasswordHashCost int `mapstructure:"password_hash_cost"`
JanitorRunInterval int `mapstructure:"janitor_run_interval"`
EnableExpiredSharesCleanup bool `mapstructure:"enable_expired_shares_cleanup"`
WriteableShareMustHavePassword bool `mapstructure:"writeable_share_must_have_password"`
GatewayAddr string `mapstructure:"gateway_addr"`
SharePasswordHashCost int `mapstructure:"password_hash_cost"`
JanitorRunInterval int `mapstructure:"janitor_run_interval"`
EnableExpiredSharesCleanup bool `mapstructure:"enable_expired_shares_cleanup"`
}

type fileConfig struct {
Expand Down Expand Up @@ -171,10 +169,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() {
Expand Down Expand Up @@ -343,12 +340,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)
Expand All @@ -369,10 +360,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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/publicshare/manager/json/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 0 additions & 6 deletions pkg/publicshare/publicshare.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"crypto/sha256"
"crypto/sha512"
"encoding/hex"
"errors"
"time"

user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
Expand All @@ -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)
Expand Down

0 comments on commit 3dca758

Please sign in to comment.