web admin: allow both allowed and denied extensions/patterns for a dir

this fix a regression introduced in the previous commit
drakkan · Nov 16, 2020 · e3eca42 · e3eca42
1 parent a6355e2
commit e3eca42
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 49 deletions.
diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go
@@ -927,14 +927,14 @@ func validateFiltersPatternExtensions(user *User) error {
 		for _, pattern := range f.AllowedPatterns {
 			_, err := path.Match(pattern, "abc")
 			if err != nil {
-				return &ValidationError{err: fmt.Sprintf("invalid file pattern filter %v", pattern)}
+				return &ValidationError{err: fmt.Sprintf("invalid file pattern filter %#v", pattern)}
 			}
 			allowed = append(allowed, strings.ToLower(pattern))
 		}
 		for _, pattern := range f.DeniedPatterns {
 			_, err := path.Match(pattern, "abc")
 			if err != nil {
-				return &ValidationError{err: fmt.Sprintf("invalid file pattern filter %v", pattern)}
+				return &ValidationError{err: fmt.Sprintf("invalid file pattern filter %#v", pattern)}
 			}
 			denied = append(denied, strings.ToLower(pattern))
 		}

diff --git a/go.mod b/go.mod
@@ -26,7 +26,7 @@ require (
 	github.com/mitchellh/mapstructure v1.3.3 // indirect
 	github.com/otiai10/copy v1.2.0
 	github.com/pelletier/go-toml v1.8.1 // indirect
-	github.com/pires/go-proxyproto v0.3.1
+	github.com/pires/go-proxyproto v0.3.2
 	github.com/pkg/sftp v1.12.1-0.20201002132022-fcaa492add82
 	github.com/prometheus/client_golang v1.8.0
 	github.com/prometheus/common v0.15.0 // indirect
@@ -47,7 +47,7 @@ require (
 	golang.org/x/net v0.0.0-20201110031124-69a78807bb2b
 	golang.org/x/oauth2 v0.0.0-20201109201403-9fd604954f58 // indirect
 	golang.org/x/sys v0.0.0-20201113233024-12cec1faf1ba
-	golang.org/x/tools v0.0.0-20201113202037-1643af1435f3 // indirect
+	golang.org/x/tools v0.0.0-20201116002733-ac45abd4c88c // indirect
 	google.golang.org/api v0.35.0
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/genproto v0.0.0-20201113130914-ce600e9a6f9e // indirect

diff --git a/go.sum b/go.sum
@@ -381,8 +381,8 @@ github.com/pelletier/go-toml v1.8.1/go.mod h1:T2/BmBdy8dvIRq1a/8aqjN41wvWlN4lrap
 github.com/performancecopilot/speed v3.0.0+incompatible/go.mod h1:/CLtqpZ5gBg1M9iaPbIdPPGyKcA8hKdoy6hAWba7Yac=
 github.com/pierrec/lz4 v1.0.2-0.20190131084431-473cd7ce01a1/go.mod h1:3/3N9NVKO0jef7pBehbT1qWhCMrIgbYNnFAZCqQ5LRc=
 github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
-github.com/pires/go-proxyproto v0.3.1 h1:eWb52zeDUbSUDBV+8aVCfOy0pnEG6DrDW3cJ/WKdQsk=
-github.com/pires/go-proxyproto v0.3.1/go.mod h1:Odh9VFOZJCf9G8cLW5o435Xf1J95Jw9Gw5rnCjcwzAY=
+github.com/pires/go-proxyproto v0.3.2 h1:E5ig1h9SFGne7IWVY6yRu3UCzyAFkQIukXHMkdFUOCA=
+github.com/pires/go-proxyproto v0.3.2/go.mod h1:Odh9VFOZJCf9G8cLW5o435Xf1J95Jw9Gw5rnCjcwzAY=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
@@ -677,7 +677,7 @@ golang.org/x/tools v0.0.0-20200904185747-39188db58858/go.mod h1:Cj7w3i3Rnn0Xh82u
 golang.org/x/tools v0.0.0-20200915173823-2db8f0ff891c/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
 golang.org/x/tools v0.0.0-20200918232735-d647fc253266/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
 golang.org/x/tools v0.0.0-20201110124207-079ba7bd75cd/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
-golang.org/x/tools v0.0.0-20201113202037-1643af1435f3/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
+golang.org/x/tools v0.0.0-20201116002733-ac45abd4c88c/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

diff --git a/httpd/httpd_test.go b/httpd/httpd_test.go
@@ -2455,10 +2455,10 @@ func TestWebUserAddMock(t *testing.T) {
 	form.Set("permissions", "*")
 	form.Set("sub_dirs_permissions", " /subdir::list ,download ")
 	form.Set("virtual_folders", fmt.Sprintf(" /vdir:: %v :: 2 :: 1024", mappedDir))
-	form.Set("allowed_extensions", "/dir2::.jpg,.png\n/dir2::.ico")
-	form.Set("denied_extensions", "/dir1::.zip")
-	form.Set("allowed_patterns", "/dir2::*.jpg,*.png")
-	form.Set("denied_patterns", "/dir1::*.zip")
+	form.Set("allowed_extensions", "/dir2::.jpg,.png\n/dir2::.ico\n/dir1::.rar")
+	form.Set("denied_extensions", "/dir2::.webp,.webp\n/dir2::.tiff\n/dir1::.zip")
+	form.Set("allowed_patterns", "/dir2::*.jpg,*.png\n/dir1::*.png")
+	form.Set("denied_patterns", "/dir1::*.zip\n/dir3::*.rar\n/dir2::*.mkv")
 	b, contentType, _ := getMultipartFormData(form, "", "")
 	// test invalid url escape
 	req, _ := http.NewRequest(http.MethodPost, webUserPath+"?a=%2", &b)
@@ -2595,22 +2595,40 @@ func TestWebUserAddMock(t *testing.T) {
 	assert.Len(t, newUser.Filters.FileExtensions, 2)
 	for _, filter := range newUser.Filters.FileExtensions {
 		if filter.Path == "/dir1" {
+			assert.Len(t, filter.DeniedExtensions, 1)
+			assert.Len(t, filter.AllowedExtensions, 1)
 			assert.True(t, utils.IsStringInSlice(".zip", filter.DeniedExtensions))
+			assert.True(t, utils.IsStringInSlice(".rar", filter.AllowedExtensions))
 		}
 		if filter.Path == "/dir2" {
+			assert.Len(t, filter.DeniedExtensions, 2)
+			assert.Len(t, filter.AllowedExtensions, 3)
 			assert.True(t, utils.IsStringInSlice(".jpg", filter.AllowedExtensions))
 			assert.True(t, utils.IsStringInSlice(".png", filter.AllowedExtensions))
 			assert.True(t, utils.IsStringInSlice(".ico", filter.AllowedExtensions))
+			assert.True(t, utils.IsStringInSlice(".webp", filter.DeniedExtensions))
+			assert.True(t, utils.IsStringInSlice(".tiff", filter.DeniedExtensions))
 		}
 	}
-	assert.Len(t, newUser.Filters.FilePatterns, 2)
+	assert.Len(t, newUser.Filters.FilePatterns, 3)
 	for _, filter := range newUser.Filters.FilePatterns {
 		if filter.Path == "/dir1" {
+			assert.Len(t, filter.DeniedPatterns, 1)
+			assert.Len(t, filter.AllowedPatterns, 1)
+			assert.True(t, utils.IsStringInSlice("*.png", filter.AllowedPatterns))
 			assert.True(t, utils.IsStringInSlice("*.zip", filter.DeniedPatterns))
 		}
 		if filter.Path == "/dir2" {
+			assert.Len(t, filter.DeniedPatterns, 1)
+			assert.Len(t, filter.AllowedPatterns, 2)
 			assert.True(t, utils.IsStringInSlice("*.jpg", filter.AllowedPatterns))
 			assert.True(t, utils.IsStringInSlice("*.png", filter.AllowedPatterns))
+			assert.True(t, utils.IsStringInSlice("*.mkv", filter.DeniedPatterns))
+		}
+		if filter.Path == "/dir3" {
+			assert.Len(t, filter.DeniedPatterns, 1)
+			assert.Len(t, filter.AllowedPatterns, 0)
+			assert.True(t, utils.IsStringInSlice("*.rar", filter.DeniedPatterns))
 		}
 	}
 	req, _ = http.NewRequest(http.MethodDelete, userPath+"/"+strconv.FormatInt(newUser.ID, 10), nil)

diff --git a/httpd/web.go b/httpd/web.go
@@ -6,6 +6,7 @@ import (
 	"html/template"
 	"io/ioutil"
 	"net/http"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -314,7 +315,7 @@ func getListFromPostFields(value string) map[string][]string {
 			dirExts := strings.Split(cleaned, "::")
 			if len(dirExts) > 1 {
 				dir := dirExts[0]
-				dir = strings.TrimSpace(dir)
+				dir = path.Clean(strings.TrimSpace(dir))
 				exts := []string{}
 				for _, e := range strings.Split(dirExts[1], ",") {
 					cleanedExt := strings.TrimSpace(e)
@@ -328,46 +329,83 @@ func getListFromPostFields(value string) map[string][]string {
 					} else {
 						result[dir] = exts
 					}
+					result[dir] = utils.RemoveDuplicates(result[dir])
 				}
 			}
 		}
 	}
 	return result
 }
 
-func getFilePatternsFromPostField(value string, extesionsType int) []dataprovider.PatternsFilter {
+func getFilePatternsFromPostField(valueAllowed, valuesDenied string) []dataprovider.PatternsFilter {
 	var result []dataprovider.PatternsFilter
-	for dir, values := range getListFromPostFields(value) {
+	allowedPatterns := getListFromPostFields(valueAllowed)
+	deniedPatterns := getListFromPostFields(valuesDenied)
+
+	for dirAllowed, allowPatterns := range allowedPatterns {
 		filter := dataprovider.PatternsFilter{
-			Path: dir,
+			Path:            dirAllowed,
+			AllowedPatterns: allowPatterns,
 		}
-		if extesionsType == 1 {
-			filter.AllowedPatterns = values
-			filter.DeniedPatterns = []string{}
-		} else {
-			filter.DeniedPatterns = values
-			filter.AllowedPatterns = []string{}
+		for dirDenied, denPatterns := range deniedPatterns {
+			if dirAllowed == dirDenied {
+				filter.DeniedPatterns = denPatterns
+				break
+			}
 		}
 		result = append(result, filter)
 	}
+	for dirDenied, denPatterns := range deniedPatterns {
+		found := false
+		for _, res := range result {
+			if res.Path == dirDenied {
+				found = true
+				break
+			}
+		}
+		if !found {
+			result = append(result, dataprovider.PatternsFilter{
+				Path:           dirDenied,
+				DeniedPatterns: denPatterns,
+			})
+		}
+	}
 	return result
 }
 
-func getFileExtensionsFromPostField(value string, extesionsType int) []dataprovider.ExtensionsFilter {
+func getFileExtensionsFromPostField(valueAllowed, valuesDenied string) []dataprovider.ExtensionsFilter {
 	var result []dataprovider.ExtensionsFilter
-	for dir, values := range getListFromPostFields(value) {
+	allowedExtensions := getListFromPostFields(valueAllowed)
+	deniedExtensions := getListFromPostFields(valuesDenied)
+
+	for dirAllowed, allowedExts := range allowedExtensions {
 		filter := dataprovider.ExtensionsFilter{
-			Path: dir,
+			Path:              dirAllowed,
+			AllowedExtensions: allowedExts,
 		}
-		if extesionsType == 1 {
-			filter.AllowedExtensions = values
-			filter.DeniedExtensions = []string{}
-		} else {
-			filter.DeniedExtensions = values
-			filter.AllowedExtensions = []string{}
+		for dirDenied, deniedExts := range deniedExtensions {
+			if dirAllowed == dirDenied {
+				filter.DeniedExtensions = deniedExts
+				break
+			}
 		}
 		result = append(result, filter)
 	}
+	for dirDenied, deniedExts := range deniedExtensions {
+		found := false
+		for _, res := range result {
+			if res.Path == dirDenied {
+				found = true
+				break
+			}
+		}
+		if !found {
+			result = append(result, dataprovider.ExtensionsFilter{
+				Path:             dirDenied,
+				DeniedExtensions: deniedExts,
+			})
+		}
+	}
 	return result
 }
 
@@ -377,19 +415,8 @@ func getFiltersFromUserPostFields(r *http.Request) dataprovider.UserFilters {
 	filters.DeniedIP = getSliceFromDelimitedValues(r.Form.Get("denied_ip"), ",")
 	filters.DeniedLoginMethods = r.Form["ssh_login_methods"]
 	filters.DeniedProtocols = r.Form["denied_protocols"]
-	allowedExtensions := getFileExtensionsFromPostField(r.Form.Get("allowed_extensions"), 1)
-	deniedExtensions := getFileExtensionsFromPostField(r.Form.Get("denied_extensions"), 2)
-	extensions := []dataprovider.ExtensionsFilter{}
-	extensions = append(extensions, allowedExtensions...)
-	extensions = append(extensions, deniedExtensions...)
-	filters.FileExtensions = extensions
-	allowedPatterns := getFilePatternsFromPostField(r.Form.Get("allowed_patterns"), 1)
-	deniedPatterns := getFilePatternsFromPostField(r.Form.Get("denied_patterns"), 2)
-	patterns := []dataprovider.PatternsFilter{}
-	patterns = append(patterns, allowedPatterns...)
-	patterns = append(patterns, deniedPatterns...)
-	filters.FilePatterns = patterns
-
+	filters.FileExtensions = getFileExtensionsFromPostField(r.Form.Get("allowed_extensions"), r.Form.Get("denied_extensions"))
+	filters.FilePatterns = getFilePatternsFromPostField(r.Form.Get("allowed_patterns"), r.Form.Get("denied_patterns"))
 	return filters
 }
 

diff --git a/templates/user.html b/templates/user.html
@@ -119,7 +119,7 @@ <h1 class="h5 mb-4 text-gray-800">{{if .IsAdd}}Add a new user{{else}}Edit user{{
                 {{- end}}
                 {{- end}}</textarea>
             <small id="subDirsHelpBlock" class="form-text text-muted">
-                One exposed virtual directory path per line as dir::perms, for example /somedir::list,download
+                One exposed virtual directory path per line as /dir::perms, for example /somedir::list,download
             </small>
         </div>
     </div>
@@ -251,7 +251,7 @@ <h1 class="h5 mb-4 text-gray-800">{{if .IsAdd}}Add a new user{{else}}Edit user{{
                 {{- end}}
                 {{- end}}</textarea>
             <small id="deniedPatternsHelpBlock" class="form-text text-muted">
-                One exposed virtual directory per line as dir::pattern1,pattern2, for example /subdir::*.zip,*.rar
+                One exposed virtual directory per line as /dir::pattern1,pattern2, for example /subdir::*.zip,*.rar
             </small>
         </div>
     </div>
@@ -266,7 +266,7 @@ <h1 class="h5 mb-4 text-gray-800">{{if .IsAdd}}Add a new user{{else}}Edit user{{
                 {{- end}}
                 {{- end}}</textarea>
             <small id="allowedPatternsHelpBlock" class="form-text text-muted">
-                One exposed virtual directory per line as dir::pattern1,pattern2, for example /somedir::*.jpg,*.png
+                One exposed virtual directory per line as /dir::pattern1,pattern2, for example /somedir::*.jpg,*.png
             </small>
         </div>
     </div>
@@ -281,7 +281,7 @@ <h1 class="h5 mb-4 text-gray-800">{{if .IsAdd}}Add a new user{{else}}Edit user{{
                 {{- end}}
                 {{- end}}</textarea>
             <small id="deniedExtensionsHelpBlock" class="form-text text-muted">
-                One exposed virtual directory per line as dir::extension1,extension2, for example /subdir::.zip,.rar. Deprecated, use file patterns
+                One exposed virtual directory per line as /dir::extension1,extension2, for example /subdir::.zip,.rar. Deprecated, use file patterns
             </small>
         </div>
     </div>
@@ -296,7 +296,7 @@ <h1 class="h5 mb-4 text-gray-800">{{if .IsAdd}}Add a new user{{else}}Edit user{{
                 {{- end}}
                 {{- end}}</textarea>
             <small id="allowedExtensionsHelpBlock" class="form-text text-muted">
-                One exposed virtual directory per line as dir::extension1,extension2, for example /somedir::.jpg,.png. Deprecated, use file patterns
+                One exposed virtual directory per line as /dir::extension1,extension2, for example /somedir::.jpg,.png. Deprecated, use file patterns
             </small>
         </div>
     </div>

diff --git a/utils/utils.go b/utils/utils.go
@@ -53,6 +53,22 @@ func IsStringPrefixInSlice(obj string, list []string) bool {
 	return false
 }
 
+// RemoveDuplicates returns a new slice removing any duplicate element from the initial one
+func RemoveDuplicates(obj []string) []string {
+	if len(obj) == 0 {
+		return obj
+	}
+	result := make([]string, 0, len(obj))
+	seen := make(map[string]bool)
+	for _, item := range obj {
+		if _, ok := seen[item]; !ok {
+			result = append(result, item)
+		}
+		seen[item] = true
+	}
+	return result
+}
+
 // GetTimeAsMsSinceEpoch returns unix timestamp as milliseconds from a time struct
 func GetTimeAsMsSinceEpoch(t time.Time) int64 {
 	return t.UnixNano() / 1000000