From dbed110d020e43520785ee4d556925d0b919ea62 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Mon, 31 Aug 2020 19:25:17 +0200 Subject: [PATCH] WebDAV: add caching for authenticated users In this way we get a big performance boost --- config/config.go | 5 + dataprovider/dataprovider.go | 77 +++++++++-- dataprovider/user.go | 17 ++- docs/full-configuration.md | 4 + docs/webdav.md | 11 +- go.mod | 2 +- go.sum | 4 +- sftpd/internal_test.go | 4 +- sftpd/sftpd_test.go | 43 +++++- sftpd/transfer.go | 4 +- sftpgo.json | 5 + webdavd/internal_test.go | 261 +++++++++++++++++++++++++++++++++++ webdavd/server.go | 43 ++++-- webdavd/webdavd.go | 9 ++ webdavd/webdavd_test.go | 10 +- 15 files changed, 465 insertions(+), 34 deletions(-) diff --git a/config/config.go b/config/config.go index da0f2573..0e1f2434 100644 --- a/config/config.go +++ b/config/config.go @@ -101,6 +101,11 @@ func init() { AllowCredentials: false, MaxAge: 0, }, + Cache: webdavd.Cache{ + Enabled: true, + ExpirationTime: 0, + MaxSize: 50, + }, }, ProviderConf: dataprovider.Config{ Driver: "sqlite", diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index ec8d56b8..52781b2e 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -98,10 +98,13 @@ var ( ValidProtocols = []string{"SSH", "FTP", "DAV"} // ErrNoInitRequired defines the error returned by InitProvider if no inizialization is required ErrNoInitRequired = errors.New("Data provider initialization is not required") - config Config - provider Provider - sqlPlaceholders []string - hashPwdPrefixes = []string{argonPwdPrefix, bcryptPwdPrefix, pbkdf2SHA1Prefix, pbkdf2SHA256Prefix, + // ErrInvalidCredentials defines the error to return if the supplied credentials are invalid + ErrInvalidCredentials = errors.New("Invalid credentials") + webDAVUsersCache sync.Map + config Config + provider Provider + sqlPlaceholders []string + hashPwdPrefixes = []string{argonPwdPrefix, bcryptPwdPrefix, pbkdf2SHA1Prefix, pbkdf2SHA256Prefix, pbkdf2SHA512Prefix, pbkdf2SHA256B64SaltPrefix, md5cryptPwdPrefix, md5cryptApr1PwdPrefix, sha512cryptPwdPrefix} pbkdfPwdPrefixes = []string{pbkdf2SHA1Prefix, pbkdf2SHA256Prefix, pbkdf2SHA512Prefix, pbkdf2SHA256B64SaltPrefix} pbkdfPwdB64SaltPrefixes = []string{pbkdf2SHA256B64SaltPrefix} @@ -507,6 +510,9 @@ func UpdateUserQuota(user User, filesAdd int, sizeAdd int64, reset bool) error { if config.ManageUsers == 0 { return &MethodDisabledError{err: manageUsersDisabledError} } + if filesAdd == 0 && sizeAdd == 0 && !reset { + return nil + } return provider.updateQuota(user.Username, filesAdd, sizeAdd, reset) } @@ -519,6 +525,9 @@ func UpdateVirtualFolderQuota(vfolder vfs.BaseVirtualFolder, filesAdd int, sizeA if config.ManageUsers == 0 { return &MethodDisabledError{err: manageUsersDisabledError} } + if filesAdd == 0 && sizeAdd == 0 && !reset { + return nil + } return provider.updateFolderQuota(vfolder.MappedPath, filesAdd, sizeAdd, reset) } @@ -543,7 +552,7 @@ func UserExists(username string) (User, error) { return provider.userExists(username) } -// AddUser adds a new SFTP user. +// AddUser adds a new SFTPGo user. // ManageUsers configuration must be set to 1 to enable this method func AddUser(user User) error { if config.ManageUsers == 0 { @@ -556,7 +565,7 @@ func AddUser(user User) error { return err } -// UpdateUser updates an existing SFTP user. +// UpdateUser updates an existing SFTPGo user. // ManageUsers configuration must be set to 1 to enable this method func UpdateUser(user User) error { if config.ManageUsers == 0 { @@ -564,6 +573,7 @@ func UpdateUser(user User) error { } err := provider.updateUser(user) if err == nil { + RemoveCachedWebDAVUser(user.Username) go executeAction(operationUpdate, user) } return err @@ -577,6 +587,7 @@ func DeleteUser(user User) error { } err := provider.deleteUser(user) if err == nil { + RemoveCachedWebDAVUser(user.Username) go executeAction(operationDelete, user) } return err @@ -1092,12 +1103,12 @@ func checkUserAndPass(user User, password, ip, protocol string) (User, error) { password = hookResponse.ToVerify default: providerLog(logger.LevelDebug, "password rejected by check password hook, status: %v", hookResponse.Status) - return user, errors.New("Invalid credentials") + return user, ErrInvalidCredentials } match, err := isPasswordOK(&user, password) if !match { - err = errors.New("Invalid credentials") + err = ErrInvalidCredentials } return user, err } @@ -1108,7 +1119,7 @@ func checkUserAndPubKey(user User, pubKey []byte) (User, string, error) { return user, "", err } if len(user.PublicKeys) == 0 { - return user, "", errors.New("Invalid credentials") + return user, "", ErrInvalidCredentials } for i, k := range user.PublicKeys { storedPubKey, comment, _, _, err := ssh.ParseAuthorizedKey([]byte(k)) @@ -1126,7 +1137,7 @@ func checkUserAndPubKey(user User, pubKey []byte) (User, string, error) { return user, fmt.Sprintf("%v:%v%v", ssh.FingerprintSHA256(storedPubKey), comment, certInfo), nil } } - return user, "", errors.New("Invalid credentials") + return user, "", ErrInvalidCredentials } func compareUnixPasswordAndHash(user *User, password string) (bool, error) { @@ -1803,7 +1814,7 @@ func doExternalAuth(username, password string, pubKey []byte, keyboardInteractiv return user, fmt.Errorf("Invalid external auth response: %v", err) } if len(user.Username) == 0 { - return user, errors.New("Invalid credentials") + return user, ErrInvalidCredentials } if len(password) > 0 { user.Password = password @@ -1919,3 +1930,47 @@ func updateVFoldersQuotaAfterRestore(foldersToScan []string) { providerLog(logger.LevelDebug, "quota updated for virtual folder %#v, error: %v", vfolder.MappedPath, err) } } + +// CacheWebDAVUser add a user to the WebDAV cache +func CacheWebDAVUser(cachedUser CachedUser, maxSize int) { + if maxSize > 0 { + var cacheSize int + var userToRemove string + var expirationTime time.Time + + webDAVUsersCache.Range(func(k, v interface{}) bool { + cacheSize++ + if len(userToRemove) == 0 { + userToRemove = k.(string) + expirationTime = v.(CachedUser).Expiration + return true + } + expireTime := v.(CachedUser).Expiration + if !expireTime.IsZero() && expireTime.Before(expirationTime) { + userToRemove = k.(string) + expirationTime = expireTime + } + return true + }) + + if cacheSize >= maxSize { + RemoveCachedWebDAVUser(userToRemove) + } + } + + if len(cachedUser.User.Username) > 0 { + webDAVUsersCache.Store(cachedUser.User.Username, cachedUser) + } +} + +// GetCachedWebDAVUser returns a previously cached WebDAV user +func GetCachedWebDAVUser(username string) (interface{}, bool) { + return webDAVUsersCache.Load(username) +} + +// RemoveCachedWebDAVUser removes a cached WebDAV user +func RemoveCachedWebDAVUser(username string) { + if len(username) > 0 { + webDAVUsersCache.Delete(username) + } +} diff --git a/dataprovider/user.go b/dataprovider/user.go index 402bc531..7260dd23 100644 --- a/dataprovider/user.go +++ b/dataprovider/user.go @@ -60,6 +60,21 @@ var ( errNoMatchingVirtualFolder = errors.New("no matching virtual folder found") ) +// CachedUser adds fields useful for caching to a SFTPGo user +type CachedUser struct { + User User + Expiration time.Time + Password string +} + +// IsExpired returns true if the cached user is expired +func (c CachedUser) IsExpired() bool { + if c.Expiration.IsZero() { + return false + } + return c.Expiration.Before(time.Now()) +} + // ExtensionsFilter defines filters based on file extensions. // These restrictions do not apply to files listing for performance reasons, so // a denied file cannot be downloaded/overwritten/renamed but will still be @@ -112,7 +127,7 @@ type Filesystem struct { GCSConfig vfs.GCSFsConfig `json:"gcsconfig,omitempty"` } -// User defines an SFTP user +// User defines a SFTPGo user type User struct { // Database unique identifier ID int64 `json:"id"` diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 5aff930f..a116d245 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -103,6 +103,10 @@ The configuration file contains the following sections: - `exposed_headers`, list of strings. - `allow_credentials` boolean. - `max_age`, integer. + - `cache` struct containing cache configuration. + - `enabled`, boolean, set to true to enable user caching. Default: true. + - `expiration_time`, integer. Expiration time, in minutes, for the cached users. 0 means unlimited. Default: 0. + - `max_size`, integer. Maximum number of users to cache. 0 means unlimited. Default: 50. - **"data_provider"**, the configuration for the data provider - `driver`, string. Supported drivers are `sqlite`, `mysql`, `postgresql`, `bolt`, `memory` - `name`, string. Database name. For driver `sqlite` this can be the database name relative to the config dir or the absolute path to the SQLite database. For driver `memory` this is the (optional) path relative to the config dir or the absolute path to the users dump, obtained using the `dumpdata` REST API, to load. This dump will be loaded at startup and can be reloaded on demand sending a `SIGHUP` signal on Unix based systems and a `paramchange` request to the running service on Windows. The `memory` provider will not modify the provided file so quota usage and last login will not be persisted diff --git a/docs/webdav.md b/docs/webdav.md index 95ad6d66..72e91496 100644 --- a/docs/webdav.md +++ b/docs/webdav.md @@ -4,6 +4,16 @@ The experimental `WebDAV` support can be enabled setting a `bind_port` inside th Each user has his own path like `http/s://:/` and it must authenticate using password credentials. +WebDAV is quite a different protocol than SCP/FTP, there is no session concept, each command is a separate HTTP request and must be authenticated, performance can be greatly improved enabling caching for the authenticated users (it is enabled by default). This way SFTPGo don't need to do a dataprovider query and a password check for each request. +If you enable quota support a dataprovider query is required, to update the user quota, after each file upload. + +The caching configuration allows to set: + +- `expiration_time` in minutes. If a user is cached for more than the specificied minutes it will be removed from the cache and a new dataprovider query will be performed. Please note that the `last_login` field will not be updated and `external_auth_hook`, `pre_login_hook` and `check_password_hook` will not be executed is the user is obtained from the cache. +- `max_size`. Maximum number of users to cache. When this limit is reached the user with the oldest `expiration_time` is removed from the cache. 0 means no limit however the cache size cannot exceed the number of users so if you have a small number of users you can leave this setting to 0. + +Users are automatically removed from the cache after an update/delete. + WebDAV should work as expected for most use cases but there are some minor issues and some missing features. Know issues: @@ -11,7 +21,6 @@ Know issues: - removing a directory tree on Cloud Storage backends could generate a `not found` error when removing the last (virtual) directory. This happen if the client cycles the directories tree itself and removes files and directories one by one instead of issuing a single remove command - the used [WebDAV library](https://pkg.go.dev/golang.org/x/net/webdav?tab=doc) asks to open a file to execute a `stat` and sometime reads some bytes to find the content type. We are unable to distinguish a `stat` from a `download` for now, so to be able to proper list a directory you need to grant both `list` and `download` permissions - the used `WebDAV library` not always returns a proper error code/message, most of the times it simply returns `Method not Allowed`. I'll try to improve the library error codes in the future -- WebDAV is quite a different protocol than SCP/FTP, there is no session concept, each command is a separate HTTP request, we could improve the performance by caching, for a small time, the user info so we don't need a user lookup (and so a dataprovider query) for each request. Some clients issue a lot of requests only for listing a directory contents. This needs more investigation and a design decision anyway the protocol itself is quite heavy - if an object within a directory cannot be accessed, for example due to OS permissions issues or because is a missing mapped path for a virtual folder, the directory listing will fail. In SFTP/FTP the directory listing will succeed and you'll only get an error if you try to access to the problematic file/directory We plan to add [Dead Properties](https://tools.ietf.org/html/rfc4918#section-3) support in future releases. We need a design decision here, probably the best solution is to store dead properties inside the data provider but this could increase a lot its size. Alternately we could store them on disk for local filesystem and add as metadata for Cloud Storage, this means that we need to do a separate `HEAD` request to retrieve dead properties for an S3 file. For big folders will do a lot of requests to the Cloud Provider, I don't like this solution. Another option is to expose a hook and allow you to implement `dead properties` outside SFTPGo. diff --git a/go.mod b/go.mod index c8fca85b..716fd3b7 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/alexedwards/argon2id v0.0.0-20200802152012-2464efd3196b github.com/aws/aws-sdk-go v1.34.13 github.com/eikenb/pipeat v0.0.0-20200430215831-470df5986b6d - github.com/fclairamb/ftpserverlib v0.8.1-0.20200824203441-87a3864e6de5 + github.com/fclairamb/ftpserverlib v0.8.1-0.20200828235935-8e22c5f260e1 github.com/fsnotify/fsnotify v1.4.9 // indirect github.com/go-chi/chi v4.1.2+incompatible github.com/go-chi/render v1.0.1 diff --git a/go.sum b/go.sum index dbd53561..e8775489 100644 --- a/go.sum +++ b/go.sum @@ -126,8 +126,8 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= -github.com/fclairamb/ftpserverlib v0.8.1-0.20200824203441-87a3864e6de5 h1:Kr7UEYS2FqcSUyItHImjszBJqJdmt4noGKIhMi0Ul4Y= -github.com/fclairamb/ftpserverlib v0.8.1-0.20200824203441-87a3864e6de5/go.mod h1:ShLpSOXbtoMDYxTb5eRs9wDBfkQ7VINYghclB4P2z4E= +github.com/fclairamb/ftpserverlib v0.8.1-0.20200828235935-8e22c5f260e1 h1:0futNS5JlIOTHAPljFKGcCdnO9U2o4JDI94wuTIuZAQ= +github.com/fclairamb/ftpserverlib v0.8.1-0.20200828235935-8e22c5f260e1/go.mod h1:ShLpSOXbtoMDYxTb5eRs9wDBfkQ7VINYghclB4P2z4E= github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4= github.com/franela/goreq v0.0.0-20171204163338-bcd34c9993f8/go.mod h1:ZhphrRTfi2rbfLwlschooIH4+wKKDR4Pdxhh+TRoA20= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index 94ad6ca4..3699f7c8 100644 --- a/sftpd/internal_test.go +++ b/sftpd/internal_test.go @@ -1671,7 +1671,7 @@ func TestTransferFailingReader(t *testing.T) { assert.EqualError(t, err, sftp.ErrSSHFxOpUnsupported.Error()) if c, ok := transfer.(io.Closer); ok { err = c.Close() - assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error()) + assert.NoError(t, err) } fsPath := filepath.Join(os.TempDir(), "afile.txt") @@ -1685,7 +1685,7 @@ func TestTransferFailingReader(t *testing.T) { assert.EqualError(t, err, errRead.Error()) err = tr.Close() - assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error()) + assert.NoError(t, err) err = os.Remove(fsPath) assert.NoError(t, err) diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index d0f0d45f..449b32b1 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -365,8 +365,6 @@ func TestOpenReadWrite(t *testing.T) { u.QuotaSize = 6553600 user, _, err := httpd.AddUser(u, http.StatusOK) assert.NoError(t, err) - err = os.RemoveAll(user.GetHomeDir()) - assert.NoError(t, err) client, err := getSftpClient(user, usePubKey) if assert.NoError(t, err) { defer client.Close() @@ -381,7 +379,8 @@ func TestOpenReadWrite(t *testing.T) { assert.EqualError(t, err, io.EOF.Error()) assert.Equal(t, len(testData)-1, n) assert.Equal(t, testData[1:], buffer[:n]) - sftpFile.Close() + err = sftpFile.Close() + assert.NoError(t, err) } sftpFile, err = client.OpenFile(testFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC) if assert.NoError(t, err) { @@ -394,8 +393,42 @@ func TestOpenReadWrite(t *testing.T) { assert.EqualError(t, err, io.EOF.Error()) assert.Equal(t, len(testData)-1, n) assert.Equal(t, testData[1:], buffer[:n]) - sftpFile.Close() - sftpFile.Close() + err = sftpFile.Close() + assert.NoError(t, err) + } + } + _, err = httpd.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) + err = os.RemoveAll(user.GetHomeDir()) + assert.NoError(t, err) +} + +func TestOpenReadWritePerm(t *testing.T) { + usePubKey := true + u := getTestUser(usePubKey) + // we cannot read inside "/sub" + u.Permissions["/sub"] = []string{dataprovider.PermUpload, dataprovider.PermListItems} + user, _, err := httpd.AddUser(u, http.StatusOK) + assert.NoError(t, err) + client, err := getSftpClient(user, usePubKey) + if assert.NoError(t, err) { + defer client.Close() + err = client.Mkdir("sub") + assert.NoError(t, err) + sftpFileName := path.Join("sub", "file.txt") + sftpFile, err := client.OpenFile(sftpFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC) + if assert.NoError(t, err) { + testData := []byte("test data") + n, err := sftpFile.Write(testData) + assert.NoError(t, err) + assert.Equal(t, len(testData), n) + buffer := make([]byte, 128) + _, err = sftpFile.ReadAt(buffer, 1) + if assert.Error(t, err) { + assert.Contains(t, strings.ToLower(err.Error()), "permission denied") + } + err = sftpFile.Close() + assert.NoError(t, err) } } _, err = httpd.RemoveUser(user, http.StatusOK) diff --git a/sftpd/transfer.go b/sftpd/transfer.go index 110817d8..7afc087f 100644 --- a/sftpd/transfer.go +++ b/sftpd/transfer.go @@ -89,7 +89,9 @@ func (t *transfer) ReadAt(p []byte, off int64) (n int, err error) { atomic.AddInt64(&t.BytesSent, int64(readed)) if e != nil && e != io.EOF { - t.TransferError(e) + if t.GetType() == common.TransferDownload { + t.TransferError(e) + } return readed, e } t.HandleThrottle() diff --git a/sftpgo.json b/sftpgo.json index 0c1d253b..b8c97481 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -59,6 +59,11 @@ "exposed_headers": [], "allow_credentials": false, "max_age": 0 + }, + "cache": { + "enabled": true, + "expiration_time": 0, + "max_size": 50 } }, "data_provider": { diff --git a/webdavd/internal_test.go b/webdavd/internal_test.go index f5fb0385..9b8690d7 100644 --- a/webdavd/internal_test.go +++ b/webdavd/internal_test.go @@ -20,6 +20,7 @@ import ( "github.com/drakkan/sftpgo/common" "github.com/drakkan/sftpgo/dataprovider" + "github.com/drakkan/sftpgo/httpd" "github.com/drakkan/sftpgo/vfs" ) @@ -611,3 +612,263 @@ func TestTransferSeek(t *testing.T) { err = os.Remove(testFilePath) assert.NoError(t, err) } + +func TestBasicUsersCache(t *testing.T) { + username := "webdav_internal_test" + password := "pwd" + u := dataprovider.User{ + Username: username, + Password: password, + HomeDir: filepath.Join(os.TempDir(), username), + Status: 1, + ExpirationDate: 0, + } + u.Permissions = make(map[string][]string) + u.Permissions["/"] = []string{dataprovider.PermAny} + user, _, err := httpd.AddUser(u, http.StatusOK) + assert.NoError(t, err) + + c := &Configuration{ + BindPort: 9000, + Cache: Cache{ + Enabled: true, + MaxSize: 50, + ExpirationTime: 1, + }, + } + server, err := newServer(c, configDir) + assert.NoError(t, err) + + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user.Username), nil) + assert.NoError(t, err) + + _, _, err = server.authenticate(req) + assert.Error(t, err) + + now := time.Now() + req.SetBasicAuth(username, password) + _, isCached, err := server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + // now the user should be cached + var cachedUser dataprovider.CachedUser + result, ok := dataprovider.GetCachedWebDAVUser(username) + if assert.True(t, ok) { + cachedUser = result.(dataprovider.CachedUser) + assert.False(t, cachedUser.IsExpired()) + assert.True(t, cachedUser.Expiration.After(now.Add(time.Duration(c.Cache.ExpirationTime)*time.Minute))) + // authenticate must return the cached user now + authUser, isCached, err := server.authenticate(req) + assert.NoError(t, err) + assert.True(t, isCached) + assert.Equal(t, cachedUser.User, authUser) + } + // a wrong password must fail + req.SetBasicAuth(username, "wrong") + _, _, err = server.authenticate(req) + assert.EqualError(t, err, dataprovider.ErrInvalidCredentials.Error()) + req.SetBasicAuth(username, password) + + // force cached user expiration + cachedUser.Expiration = now + dataprovider.CacheWebDAVUser(cachedUser, c.Cache.MaxSize) + result, ok = dataprovider.GetCachedWebDAVUser(username) + if assert.True(t, ok) { + cachedUser = result.(dataprovider.CachedUser) + assert.True(t, cachedUser.IsExpired()) + } + // now authenticate should get the user from the data provider and update the cache + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + result, ok = dataprovider.GetCachedWebDAVUser(username) + if assert.True(t, ok) { + cachedUser = result.(dataprovider.CachedUser) + assert.False(t, cachedUser.IsExpired()) + } + // cache is invalidated after a user modification + user, _, err = httpd.UpdateUser(user, http.StatusOK) + assert.NoError(t, err) + _, ok = dataprovider.GetCachedWebDAVUser(username) + assert.False(t, ok) + + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + _, ok = dataprovider.GetCachedWebDAVUser(username) + assert.True(t, ok) + // cache is invalidated after user deletion + _, err = httpd.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) + _, ok = dataprovider.GetCachedWebDAVUser(username) + assert.False(t, ok) +} + +func TestUsersCacheSizeAndExpiration(t *testing.T) { + username := "webdav_internal_test" + password := "pwd" + u := dataprovider.User{ + HomeDir: filepath.Join(os.TempDir(), username), + Status: 1, + ExpirationDate: 0, + } + u.Username = username + "1" + u.Password = password + "1" + u.Permissions = make(map[string][]string) + u.Permissions["/"] = []string{dataprovider.PermAny} + user1, _, err := httpd.AddUser(u, http.StatusOK) + assert.NoError(t, err) + u.Username = username + "2" + u.Password = password + "2" + user2, _, err := httpd.AddUser(u, http.StatusOK) + assert.NoError(t, err) + u.Username = username + "3" + u.Password = password + "3" + user3, _, err := httpd.AddUser(u, http.StatusOK) + assert.NoError(t, err) + u.Username = username + "4" + u.Password = password + "4" + user4, _, err := httpd.AddUser(u, http.StatusOK) + assert.NoError(t, err) + + c := &Configuration{ + BindPort: 9000, + Cache: Cache{ + Enabled: true, + MaxSize: 3, + ExpirationTime: 1, + }, + } + server, err := newServer(c, configDir) + assert.NoError(t, err) + + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user1.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user1.Username, password+"1") + _, isCached, err := server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user2.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user2.Username, password+"2") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user3.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user3.Username, password+"3") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + + // the first 3 users are now cached + _, ok := dataprovider.GetCachedWebDAVUser(user1.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user2.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user3.Username) + assert.True(t, ok) + + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user4.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user4.Username, password+"4") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + // user1, the first cached, should be removed now + _, ok = dataprovider.GetCachedWebDAVUser(user1.Username) + assert.False(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user2.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user3.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user4.Username) + assert.True(t, ok) + + // user1 logins, user2 should be removed + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user1.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user1.Username, password+"1") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + _, ok = dataprovider.GetCachedWebDAVUser(user2.Username) + assert.False(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user1.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user3.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user4.Username) + assert.True(t, ok) + + // user2 logins, user3 should be removed + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user2.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user2.Username, password+"2") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + _, ok = dataprovider.GetCachedWebDAVUser(user3.Username) + assert.False(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user1.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user2.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user4.Username) + assert.True(t, ok) + + // user3 logins, user4 should be removed + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user3.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user3.Username, password+"3") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + _, ok = dataprovider.GetCachedWebDAVUser(user4.Username) + assert.False(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user1.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user2.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user3.Username) + assert.True(t, ok) + + // now remove user1 after an update + user1, _, err = httpd.UpdateUser(user1, http.StatusOK) + assert.NoError(t, err) + _, ok = dataprovider.GetCachedWebDAVUser(user1.Username) + assert.False(t, ok) + + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user4.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user4.Username, password+"4") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("/%v", user1.Username), nil) + assert.NoError(t, err) + req.SetBasicAuth(user1.Username, password+"1") + _, isCached, err = server.authenticate(req) + assert.NoError(t, err) + assert.False(t, isCached) + _, ok = dataprovider.GetCachedWebDAVUser(user2.Username) + assert.False(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user1.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user3.Username) + assert.True(t, ok) + _, ok = dataprovider.GetCachedWebDAVUser(user4.Username) + assert.True(t, ok) + + _, err = httpd.RemoveUser(user1, http.StatusOK) + assert.NoError(t, err) + _, err = httpd.RemoveUser(user2, http.StatusOK) + assert.NoError(t, err) + _, err = httpd.RemoveUser(user3, http.StatusOK) + assert.NoError(t, err) + _, err = httpd.RemoveUser(user4, http.StatusOK) + assert.NoError(t, err) +} diff --git a/webdavd/server.go b/webdavd/server.go index c79a29ce..bd02499a 100644 --- a/webdavd/server.go +++ b/webdavd/server.go @@ -90,7 +90,7 @@ func (s *webDavServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.Error(w, common.ErrConnectionDenied.Error(), http.StatusForbidden) return } - user, err := s.authenticate(r) + user, isCached, err := s.authenticate(r) if err != nil { w.Header().Set("WWW-Authenticate", "Basic realm=\"SFTPGo WebDAV\"") http.Error(w, err401.Error(), http.StatusUnauthorized) @@ -123,10 +123,11 @@ func (s *webDavServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { common.Connections.Add(connection) defer common.Connections.Remove(connection.GetID()) - connection.Fs.CheckRootPath(connection.GetUsername(), user.GetUID(), user.GetGID()) - connection.Log(logger.LevelInfo, "User id: %d, logged in with WebDAV, method: %v, username: %#v, home_dir: %#v remote addr: %#v", - user.ID, r.Method, user.Username, user.HomeDir, r.RemoteAddr) - dataprovider.UpdateLastLogin(user) //nolint:errcheck + if !isCached { + // we update last login and check for home directory only if the user is not cached + connection.Fs.CheckRootPath(connection.GetUsername(), user.GetUID(), user.GetGID()) + dataprovider.UpdateLastLogin(user) //nolint:errcheck + } prefix := path.Join("/", user.Username) // see RFC4918, section 9.4 @@ -150,19 +151,43 @@ func (s *webDavServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { handler.ServeHTTP(w, r.WithContext(ctx)) } -func (s *webDavServer) authenticate(r *http.Request) (dataprovider.User, error) { +func (s *webDavServer) authenticate(r *http.Request) (dataprovider.User, bool, error) { var user dataprovider.User var err error username, password, ok := r.BasicAuth() if !ok { - return user, err401 + return user, false, err401 + } + if s.config.Cache.Enabled { + result, ok := dataprovider.GetCachedWebDAVUser(username) + if ok { + if result.(dataprovider.CachedUser).IsExpired() { + dataprovider.RemoveCachedWebDAVUser(username) + } else { + if len(password) > 0 && result.(dataprovider.CachedUser).Password == password { + return result.(dataprovider.CachedUser).User, true, nil + } + updateLoginMetrics(username, r.RemoteAddr, dataprovider.ErrInvalidCredentials) + return user, false, dataprovider.ErrInvalidCredentials + } + } } user, err = dataprovider.CheckUserAndPass(username, password, utils.GetIPFromRemoteAddress(r.RemoteAddr), common.ProtocolWebDAV) if err != nil { updateLoginMetrics(username, r.RemoteAddr, err) - return user, err + return user, false, err } - return user, err + if s.config.Cache.Enabled && len(password) > 0 { + cachedUser := dataprovider.CachedUser{ + User: user, + Password: password, + } + if s.config.Cache.ExpirationTime > 0 { + cachedUser.Expiration = time.Now().Add(time.Duration(s.config.Cache.ExpirationTime) * time.Minute) + } + dataprovider.CacheWebDAVUser(cachedUser, s.config.Cache.MaxSize) + } + return user, false, err } func (s *webDavServer) validateUser(user dataprovider.User, r *http.Request) (string, error) { diff --git a/webdavd/webdavd.go b/webdavd/webdavd.go index fd2c32e3..a2e2ff0e 100644 --- a/webdavd/webdavd.go +++ b/webdavd/webdavd.go @@ -34,6 +34,13 @@ type Cors struct { MaxAge int `json:"max_age" mapstructure:"max_age"` } +// Cache configuration +type Cache struct { + Enabled bool `json:"enabled" mapstructure:"enabled"` + ExpirationTime int `json:"expiration_time" mapstructure:"expiration_time"` + MaxSize int `json:"max_size" mapstructure:"max_size"` +} + // Configuration defines the configuration for the WevDAV server type Configuration struct { // The port used for serving FTP requests @@ -48,6 +55,8 @@ type Configuration struct { CertificateKeyFile string `json:"certificate_key_file" mapstructure:"certificate_key_file"` // CORS configuration Cors Cors `json:"cors" mapstructure:"cors"` + // Cache configuration + Cache Cache `json:"cache" mapstructure:"cache"` } // Initialize configures and starts the WebDav server diff --git a/webdavd/webdavd_test.go b/webdavd/webdavd_test.go index 0edc8bed..27d06c27 100644 --- a/webdavd/webdavd_test.go +++ b/webdavd/webdavd_test.go @@ -277,8 +277,10 @@ func TestLoginInvalidPwd(t *testing.T) { u := getTestUser() user, _, err := httpd.AddUser(u, http.StatusOK) assert.NoError(t, err) - user.Password = "wrong" client := getWebDavClient(user) + assert.NoError(t, checkBasicFunc(client)) + user.Password = "wrong" + client = getWebDavClient(user) assert.Error(t, checkBasicFunc(client)) _, err = httpd.RemoveUser(user, http.StatusOK) assert.NoError(t, err) @@ -374,8 +376,14 @@ func TestPreLoginHook(t *testing.T) { assert.NoError(t, checkBasicFunc(client)) err = ioutil.WriteFile(preLoginPath, getPreLoginScriptContent(user, true), os.ModePerm) assert.NoError(t, err) + // update the user to remove it from the cache + user, _, err = httpd.UpdateUser(user, http.StatusOK) + assert.NoError(t, err) client = getWebDavClient(user) assert.Error(t, checkBasicFunc(client)) + // update the user to remove it from the cache + user, _, err = httpd.UpdateUser(user, http.StatusOK) + assert.NoError(t, err) user.Status = 0 err = ioutil.WriteFile(preLoginPath, getPreLoginScriptContent(user, false), os.ModePerm) assert.NoError(t, err)