Browse Source

webdav: user caching is now mandatory

we cache the lock system with the user, without user caching we cannot
support locks for resource
Nicola Murino 4 years ago
parent
commit
0119fd03a6
7 changed files with 18 additions and 27 deletions
  1. 0 2
      config/config.go
  2. 2 3
      docs/webdav.md
  3. 0 1
      sftpgo.json
  4. 1 1
      webdavd/file.go
  5. 0 2
      webdavd/internal_test.go
  6. 13 15
      webdavd/server.go
  7. 2 3
      webdavd/webdavd.go

+ 0 - 2
config/config.go

@@ -104,7 +104,6 @@ func init() {
 			},
 			Cache: webdavd.Cache{
 				Users: webdavd.UsersCacheConfig{
-					Enabled:        true,
 					ExpirationTime: 0,
 					MaxSize:        50,
 				},
@@ -399,7 +398,6 @@ func setViperDefaults() {
 	viper.SetDefault("webdavd.cors.exposed_headers", globalConf.WebDAVD.Cors.ExposedHeaders)
 	viper.SetDefault("webdavd.cors.allow_credentials", globalConf.WebDAVD.Cors.AllowCredentials)
 	viper.SetDefault("webdavd.cors.max_age", globalConf.WebDAVD.Cors.MaxAge)
-	viper.SetDefault("webdavd.cache.users.enabled", globalConf.WebDAVD.Cache.Users.Enabled)
 	viper.SetDefault("webdavd.cache.users.expiration_time", globalConf.WebDAVD.Cache.Users.ExpirationTime)
 	viper.SetDefault("webdavd.cache.users.max_size", globalConf.WebDAVD.Cache.Users.MaxSize)
 	viper.SetDefault("webdavd.cache.mime_types.enabled", globalConf.WebDAVD.Cache.MimeTypes.Enabled)

+ 2 - 3
docs/webdav.md

@@ -4,13 +4,12 @@ The experimental `WebDAV` support can be enabled by setting a `bind_port` inside
 
 Each user has his own path like `http/s://<SFTPGo ip>:<WevDAVPORT>/<username>` 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.
+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, to improve performance SFTPGo caches authenticated users. This way SFTPGo don't need to do a dataprovider query and a password check for each request.
 
 The user caching configuration allows to set:
 
 - `expiration_time` in minutes. If a user is cached for more than the specified 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 if 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 date will be 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.
+- `max_size`. Maximum number of users to cache. When this limit is reached the user with the oldest expiration date will be 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 set this value to 0.
 
 Users are automatically removed from the cache after an update/delete.
 

+ 0 - 1
sftpgo.json

@@ -63,7 +63,6 @@
     },
     "cache": {
       "users": {
-        "enabled": true,
         "expiration_time": 0,
         "max_size": 50
       },

+ 1 - 1
webdavd/file.go

@@ -72,8 +72,8 @@ func (fi *webDavFileInfo) ContentType(ctx context.Context) (string, error) {
 		return contentType, nil
 	}
 	contentType, err := fi.Fs.GetMimeType(fi.fsPath)
-	mimeTypeCache.addMimeToCache(extension, contentType)
 	if contentType != "" {
+		mimeTypeCache.addMimeToCache(extension, contentType)
 		return contentType, err
 	}
 	return "", webdav.ErrNotImplemented

+ 0 - 2
webdavd/internal_test.go

@@ -647,7 +647,6 @@ func TestBasicUsersCache(t *testing.T) {
 		BindPort: 9000,
 		Cache: Cache{
 			Users: UsersCacheConfig{
-				Enabled:        true,
 				MaxSize:        50,
 				ExpirationTime: 1,
 			},
@@ -752,7 +751,6 @@ func TestUsersCacheSizeAndExpiration(t *testing.T) {
 		BindPort: 9000,
 		Cache: Cache{
 			Users: UsersCacheConfig{
-				Enabled:        true,
 				MaxSize:        3,
 				ExpirationTime: 1,
 			},

+ 13 - 15
webdavd/server.go

@@ -104,7 +104,7 @@ func (s *webDavServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if path.Clean(r.URL.Path) == "/" && (r.Method == "GET" || r.Method == "PROPFIND" || r.Method == "OPTIONS") {
+	if path.Clean(r.URL.Path) == "/" && (r.Method == http.MethodGet || r.Method == "PROPFIND" || r.Method == http.MethodOptions) {
 		http.Redirect(w, r, path.Join("/", user.Username), http.StatusMovedPermanently)
 		return
 	}
@@ -143,7 +143,7 @@ func (s *webDavServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	prefix := path.Join("/", user.Username)
 	// see RFC4918, section 9.4
-	if r.Method == "GET" {
+	if r.Method == http.MethodGet {
 		p := strings.TrimPrefix(path.Clean(r.URL.Path), prefix)
 		info, err := connection.Stat(ctx, p)
 		if err == nil && info.IsDir() {
@@ -170,19 +170,17 @@ func (s *webDavServer) authenticate(r *http.Request) (dataprovider.User, bool, w
 	if !ok {
 		return user, false, nil, err401
 	}
-	if s.config.Cache.Users.Enabled {
-		result, ok := dataprovider.GetCachedWebDAVUser(username)
-		if ok {
-			cachedUser := result.(*dataprovider.CachedUser)
-			if cachedUser.IsExpired() {
-				dataprovider.RemoveCachedWebDAVUser(username)
-			} else {
-				if len(password) > 0 && cachedUser.Password == password {
-					return cachedUser.User, true, cachedUser.LockSystem, nil
-				}
-				updateLoginMetrics(username, r.RemoteAddr, dataprovider.ErrInvalidCredentials)
-				return user, false, nil, dataprovider.ErrInvalidCredentials
+	result, ok := dataprovider.GetCachedWebDAVUser(username)
+	if ok {
+		cachedUser := result.(*dataprovider.CachedUser)
+		if cachedUser.IsExpired() {
+			dataprovider.RemoveCachedWebDAVUser(username)
+		} else {
+			if len(password) > 0 && cachedUser.Password == password {
+				return cachedUser.User, true, cachedUser.LockSystem, nil
 			}
+			updateLoginMetrics(username, r.RemoteAddr, dataprovider.ErrInvalidCredentials)
+			return user, false, nil, dataprovider.ErrInvalidCredentials
 		}
 	}
 	user, err = dataprovider.CheckUserAndPass(username, password, utils.GetIPFromRemoteAddress(r.RemoteAddr), common.ProtocolWebDAV)
@@ -191,7 +189,7 @@ func (s *webDavServer) authenticate(r *http.Request) (dataprovider.User, bool, w
 		return user, false, nil, err
 	}
 	lockSystem := webdav.NewMemLS()
-	if s.config.Cache.Users.Enabled && len(password) > 0 {
+	if password != "" {
 		cachedUser := &dataprovider.CachedUser{
 			User:       user,
 			Password:   password,

+ 2 - 3
webdavd/webdavd.go

@@ -36,9 +36,8 @@ type Cors struct {
 
 // UsersCacheConfig defines the cache configuration for users
 type UsersCacheConfig struct {
-	Enabled        bool `json:"enabled" mapstructure:"enabled"`
-	ExpirationTime int  `json:"expiration_time" mapstructure:"expiration_time"`
-	MaxSize        int  `json:"max_size" mapstructure:"max_size"`
+	ExpirationTime int `json:"expiration_time" mapstructure:"expiration_time"`
+	MaxSize        int `json:"max_size" mapstructure:"max_size"`
 }
 
 // MimeCacheConfig defines the cache configuration for mime types