From bdb6f585c79242c758a2b2f83656c4b9f12a7f55 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 5 Mar 2021 19:08:22 +0100 Subject: [PATCH] add a setting to skip natural keys validation Enabling the "skip_natural_keys_validation" data provider setting, the natural keys for REST API/Web Admin as usernames, admin names, folder names are not restricted to unreserved URI chars Fixes #334 #308 --- config/config.go | 2 ++ dataprovider/admin.go | 21 ++++++++----- dataprovider/dataprovider.go | 8 +++-- docs/full-configuration.md | 1 + httpd/httpd_test.go | 59 ++++++++++++++++++++++++++++++++++++ sftpgo.json | 3 +- templates/admins.html | 6 ++-- templates/base.html | 8 +++++ templates/folders.html | 10 +++--- templates/users.html | 12 ++++---- 10 files changed, 106 insertions(+), 24 deletions(-) diff --git a/config/config.go b/config/config.go index cd07acd9..4a8e067f 100644 --- a/config/config.go +++ b/config/config.go @@ -210,6 +210,7 @@ func Init() { }, UpdateMode: 0, PreferDatabaseCredentials: false, + SkipNaturalKeysValidation: false, }, HTTPDConfig: httpd.Conf{ Bindings: []httpd.Binding{defaultHTTPDBinding}, @@ -889,6 +890,7 @@ func setViperDefaults() { viper.SetDefault("data_provider.password_hashing.argon2_options.iterations", globalConf.ProviderConf.PasswordHashing.Argon2Options.Iterations) viper.SetDefault("data_provider.password_hashing.argon2_options.parallelism", globalConf.ProviderConf.PasswordHashing.Argon2Options.Parallelism) viper.SetDefault("data_provider.update_mode", globalConf.ProviderConf.UpdateMode) + viper.SetDefault("data_provider.skip_natural_keys_validation", globalConf.ProviderConf.SkipNaturalKeysValidation) viper.SetDefault("httpd.templates_path", globalConf.HTTPDConfig.TemplatesPath) viper.SetDefault("httpd.static_files_path", globalConf.HTTPDConfig.StaticFilesPath) viper.SetDefault("httpd.backups_path", globalConf.HTTPDConfig.BackupsPath) diff --git a/dataprovider/admin.go b/dataprovider/admin.go index b648544e..1efb3377 100644 --- a/dataprovider/admin.go +++ b/dataprovider/admin.go @@ -62,6 +62,17 @@ type Admin struct { AdditionalInfo string `json:"additional_info,omitempty"` } +func (a *Admin) checkPassword() error { + if a.Password != "" && !strings.HasPrefix(a.Password, argonPwdPrefix) { + pwd, err := argon2id.CreateHash(a.Password, argon2Params) + if err != nil { + return err + } + a.Password = pwd + } + return nil +} + func (a *Admin) validate() error { if a.Username == "" { return &ValidationError{err: "username is mandatory"} @@ -69,15 +80,11 @@ func (a *Admin) validate() error { if a.Password == "" { return &ValidationError{err: "please set a password"} } - if !usernameRegex.MatchString(a.Username) { + if !config.SkipNaturalKeysValidation && !usernameRegex.MatchString(a.Username) { return &ValidationError{err: fmt.Sprintf("username %#v is not valid, the following characters are allowed: a-zA-Z0-9-_.~", a.Username)} } - if a.Password != "" && !strings.HasPrefix(a.Password, argonPwdPrefix) { - pwd, err := argon2id.CreateHash(a.Password, argon2Params) - if err != nil { - return err - } - a.Password = pwd + if err := a.checkPassword(); err != nil { + return err } a.Permissions = utils.RemoveDuplicates(a.Permissions) if len(a.Permissions) == 0 { diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index bf6b161c..add82d7c 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -271,6 +271,10 @@ type Config struct { // Cloud Storage) should be stored in the database instead of in the directory specified by // CredentialsPath. PreferDatabaseCredentials bool `json:"prefer_database_credentials" mapstructure:"prefer_database_credentials"` + // SkipNaturalKeysValidation allows to use any UTF-8 character for natural keys as username, admin name, + // folder name. These keys are used in URIs for REST API and Web admin. By default only unreserved URI + // characters are allowed: ALPHA / DIGIT / "-" / "." / "_" / "~". + SkipNaturalKeysValidation bool `json:"skip_natural_keys_validation" mapstructure:"skip_natural_keys_validation"` } // BackupData defines the structure for the backup/restore files @@ -1362,7 +1366,7 @@ func validateBaseParams(user *User) error { if user.Username == "" { return &ValidationError{err: "username is mandatory"} } - if !usernameRegex.MatchString(user.Username) { + if !config.SkipNaturalKeysValidation && !usernameRegex.MatchString(user.Username) { return &ValidationError{err: fmt.Sprintf("username %#v is not valid, the following characters are allowed: a-zA-Z0-9-_.~", user.Username)} } @@ -1395,7 +1399,7 @@ func ValidateFolder(folder *vfs.BaseVirtualFolder) error { if folder.Name == "" { return &ValidationError{err: "folder name is mandatory"} } - if !usernameRegex.MatchString(folder.Name) { + if !config.SkipNaturalKeysValidation && !usernameRegex.MatchString(folder.Name) { return &ValidationError{err: fmt.Sprintf("folder name %#v is not valid, the following characters are allowed: a-zA-Z0-9-_.~", folder.Name)} } diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 7c2a6e34..40fad2c8 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -188,6 +188,7 @@ The configuration file contains the following sections: - `iterations`, unsigned integer. The number of iterations over the memory. Default: 1. - `parallelism`. unsigned 8 bit integer. The number of threads (or lanes) used by the algorithm. Default: 2. - `update_mode`, integer. Defines how the database will be initialized/updated. 0 means automatically. 1 means manually using the initprovider sub-command. + - `skip_natural_keys_validation`, boolean. If `true` you can use any UTF-8 character for natural keys as username, admin name, folder name. These keys are used in URIs for REST API and Web admin. If `false` only unreserved URI characters are allowed: ALPHA / DIGIT / "-" / "." / "_" / "~". Default: `false`. - **"httpd"**, the configuration for the HTTP server used to serve REST API and to expose the built-in web interface - `bindings`, list of structs. Each struct has the following fields: - `port`, integer. The port used for serving HTTP requests. Default: 8080. diff --git a/httpd/httpd_test.go b/httpd/httpd_test.go index 79284747..16ada1f5 100644 --- a/httpd/httpd_test.go +++ b/httpd/httpd_test.go @@ -2228,6 +2228,65 @@ func TestCloseConnectionAfterUserUpdateDelete(t *testing.T) { assert.Len(t, common.Connections.GetStats(), 0) } +func TestSkipNaturalKeysValidation(t *testing.T) { + err := dataprovider.Close() + assert.NoError(t, err) + err = config.LoadConfig(configDir, "") + assert.NoError(t, err) + providerConf := config.GetProviderConf() + providerConf.SkipNaturalKeysValidation = true + err = dataprovider.Initialize(providerConf, configDir, true) + assert.NoError(t, err) + + u := getTestUser() + u.Username = "user@example.com" + user, _, err := httpdtest.AddUser(u, http.StatusCreated) + assert.NoError(t, err) + user.AdditionalInfo = "info" + user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) + user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK) + assert.NoError(t, err) + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) + + a := getTestAdmin() + a.Username = "admin@example.com" + admin, _, err := httpdtest.AddAdmin(a, http.StatusCreated) + assert.NoError(t, err) + admin.Email = admin.Username + admin, _, err = httpdtest.UpdateAdmin(admin, http.StatusOK) + assert.NoError(t, err) + admin, _, err = httpdtest.GetAdminByUsername(admin.Username, http.StatusOK) + assert.NoError(t, err) + _, err = httpdtest.RemoveAdmin(admin, http.StatusOK) + assert.NoError(t, err) + + f := vfs.BaseVirtualFolder{ + Name: "文件夹", + MappedPath: filepath.Clean(os.TempDir()), + } + folder, resp, err := httpdtest.AddFolder(f, http.StatusCreated) + assert.NoError(t, err, string(resp)) + folder, resp, err = httpdtest.UpdateFolder(folder, http.StatusOK) + assert.NoError(t, err, string(resp)) + folder, resp, err = httpdtest.GetFolderByName(folder.Name, http.StatusOK) + assert.NoError(t, err, string(resp)) + _, err = httpdtest.RemoveFolder(folder, http.StatusOK) + assert.NoError(t, err) + + err = dataprovider.Close() + assert.NoError(t, err) + err = config.LoadConfig(configDir, "") + assert.NoError(t, err) + providerConf = config.GetProviderConf() + providerConf.CredentialsPath = credentialsPath + err = os.RemoveAll(credentialsPath) + assert.NoError(t, err) + err = dataprovider.Initialize(providerConf, configDir, true) + assert.NoError(t, err) +} + func TestUserBaseDir(t *testing.T) { err := dataprovider.Close() assert.NoError(t, err) diff --git a/sftpgo.json b/sftpgo.json index 44b52b2d..0d041c5c 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -146,7 +146,8 @@ "parallelism": 2 } }, - "update_mode": 0 + "update_mode": 0, + "skip_natural_keys_validation": false }, "httpd": { "bindings": [ diff --git a/templates/admins.html b/templates/admins.html index bed10fa5..8d1b6a99 100644 --- a/templates/admins.html +++ b/templates/admins.html @@ -93,7 +93,7 @@ var table = $('#dataTable').DataTable(); table.button('delete:name').enable(false); var username = table.row({ selected: true }).data()[1]; - var path = '{{.AdminURL}}' + "/" + username; + var path = '{{.AdminURL}}' + "/" + fixedEncodeURIComponent(username); $('#deleteModal').modal('hide'); $.ajax({ url: path, @@ -137,8 +137,8 @@ name: 'edit', action: function (e, dt, node, config) { var username = dt.row({ selected: true }).data()[1]; - var path = '{{.AdminURL}}' + "/" + username; - window.location.href = encodeURI(path); + var path = '{{.AdminURL}}' + "/" + fixedEncodeURIComponent(username); + window.location.href = path; }, enabled: false }; diff --git a/templates/base.html b/templates/base.html index 5a0f038a..e9e8aff5 100644 --- a/templates/base.html +++ b/templates/base.html @@ -221,6 +221,14 @@ + + {{block "extra_js" .}}{{end}} diff --git a/templates/folders.html b/templates/folders.html index 677bab61..dcd815b1 100644 --- a/templates/folders.html +++ b/templates/folders.html @@ -91,10 +91,10 @@ function deleteAction() { var table = $('#dataTable').DataTable(); table.button('delete:name').enable(false); var folderName = table.row({ selected: true }).data()[0]; - var path = '{{.FolderURL}}' + "/" + folderName; + var path = '{{.FolderURL}}' + "/" + fixedEncodeURIComponent(folderName); $('#deleteModal').modal('hide'); $.ajax({ - url: encodeURI(path), + url: path, type: 'DELETE', dataType: 'json', headers: {'X-CSRF-TOKEN' : '{{.CSRFToken}}'}, @@ -135,8 +135,8 @@ function deleteAction() { name: 'edit', action: function (e, dt, node, config) { var folderName = table.row({ selected: true }).data()[0]; - var path = '{{.FolderURL}}' + "/" + folderName; - window.location.href = encodeURI(path); + var path = '{{.FolderURL}}' + "/" + fixedEncodeURIComponent(folderName); + window.location.href = path; }, enabled: false }; @@ -148,7 +148,7 @@ function deleteAction() { var selectedRows = table.rows({ selected: true }).count(); if (selectedRows == 1){ var folderName = table.row({ selected: true }).data()[0]; - var path = '{{.FolderTemplateURL}}' + "?from=" + encodeURIComponent(folderName); + var path = '{{.FolderTemplateURL}}' + "?from=" + fixedEncodeURIComponent(folderName); window.location.href = path; } else { window.location.href = '{{.FolderTemplateURL}}'; diff --git a/templates/users.html b/templates/users.html index d2298e8e..a6922916 100644 --- a/templates/users.html +++ b/templates/users.html @@ -99,10 +99,10 @@ var table = $('#dataTable').DataTable(); table.button('delete:name').enable(false); var username = table.row({ selected: true }).data()[1]; - var path = '{{.UserURL}}' + "/" + username; + var path = '{{.UserURL}}' + "/" + fixedEncodeURIComponent(username); $('#deleteModal').modal('hide'); $.ajax({ - url: encodeURI(path), + url: path, type: 'DELETE', dataType: 'json', headers: {'X-CSRF-TOKEN' : '{{.CSRFToken}}'}, @@ -143,8 +143,8 @@ name: 'edit', action: function (e, dt, node, config) { var username = dt.row({ selected: true }).data()[1]; - var path = '{{.UserURL}}' + "/" + username; - window.location.href = encodeURI(path); + var path = '{{.UserURL}}' + "/" + fixedEncodeURIComponent(username); + window.location.href = path; }, enabled: false }; @@ -154,7 +154,7 @@ name: 'clone', action: function (e, dt, node, config) { var username = dt.row({ selected: true }).data()[1]; - var path = '{{.UserURL}}' + "?clone-from=" + encodeURIComponent(username); + var path = '{{.UserURL}}' + "?clone-from=" + fixedEncodeURIComponent(username); window.location.href = path; }, enabled: false @@ -167,7 +167,7 @@ var selectedRows = table.rows({ selected: true }).count(); if (selectedRows == 1){ var username = dt.row({ selected: true }).data()[1]; - var path = '{{.UserTemplateURL}}' + "?from=" + encodeURIComponent(username); + var path = '{{.UserTemplateURL}}' + "?from=" + fixedEncodeURIComponent(username); window.location.href = path; } else { window.location.href = '{{.UserTemplateURL}}';