summaryrefslogtreecommitdiffstats
path: root/modules
diff options
context:
space:
mode:
authorRowan Bohde <rowan.bohde@gmail.com>2024-11-28 03:50:27 +0100
committerEarl Warren <contact@earl-warren.org>2024-12-03 10:19:22 +0100
commit2e00ae4cddff6ba04fb52adc44b21293857f4267 (patch)
treeba85bf1d629e101311d2122b535e3b4cc951dd90 /modules
parentAdd github compatible tarball download API endpoints (#32572) (diff)
downloadforgejo-2e00ae4cddff6ba04fb52adc44b21293857f4267.tar.xz
forgejo-2e00ae4cddff6ba04fb52adc44b21293857f4267.zip
Validate OAuth Redirect URIs (#32643)
This fixes a TODO in the code to validate the RedirectURIs when adding or editing an OAuth application in user settings. This also includes a refactor of the user settings tests to only create the DB once per top-level test to avoid reloading fixtures. (cherry picked from commit 16a7d343d78807e39df124756e5d43a69a2203a3) Conflicts: services/forms/user_form.go tests/integration/user_settings_test.go simple conflicts
Diffstat (limited to 'modules')
-rw-r--r--modules/util/truncate.go2
-rw-r--r--modules/validation/binding.go37
-rw-r--r--modules/validation/binding_test.go1
-rw-r--r--modules/validation/validurllist_test.go157
4 files changed, 193 insertions, 4 deletions
diff --git a/modules/util/truncate.go b/modules/util/truncate.go
index 77b116eeff..f2edbdc673 100644
--- a/modules/util/truncate.go
+++ b/modules/util/truncate.go
@@ -41,6 +41,8 @@ func SplitStringAtByteN(input string, n int) (left, right string) {
// SplitTrimSpace splits the string at given separator and trims leading and trailing space
func SplitTrimSpace(input, sep string) []string {
+ // Trim initial leading & trailing space
+ input = strings.TrimSpace(input)
// replace CRLF with LF
input = strings.ReplaceAll(input, "\r\n", "\n")
diff --git a/modules/validation/binding.go b/modules/validation/binding.go
index 1c81423ab3..006fbfafc1 100644
--- a/modules/validation/binding.go
+++ b/modules/validation/binding.go
@@ -10,6 +10,7 @@ import (
"code.gitea.io/gitea/modules/auth"
"code.gitea.io/gitea/modules/git"
+ "code.gitea.io/gitea/modules/util"
"code.forgejo.org/go-chi/binding"
"github.com/gobwas/glob"
@@ -33,6 +34,7 @@ const (
// AddBindingRules adds additional binding rules
func AddBindingRules() {
addGitRefNameBindingRule()
+ addValidURLListBindingRule()
addValidURLBindingRule()
addValidSiteURLBindingRule()
addGlobPatternRule()
@@ -47,7 +49,7 @@ func addGitRefNameBindingRule() {
// Git refname validation rule
binding.AddRule(&binding.Rule{
IsMatch: func(rule string) bool {
- return strings.HasPrefix(rule, "GitRefName")
+ return rule == "GitRefName"
},
IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
str := fmt.Sprintf("%v", val)
@@ -61,11 +63,38 @@ func addGitRefNameBindingRule() {
})
}
+func addValidURLListBindingRule() {
+ // URL validation rule
+ binding.AddRule(&binding.Rule{
+ IsMatch: func(rule string) bool {
+ return rule == "ValidUrlList"
+ },
+ IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
+ str := fmt.Sprintf("%v", val)
+ if len(str) == 0 {
+ errs.Add([]string{name}, binding.ERR_URL, "Url")
+ return false, errs
+ }
+
+ ok := true
+ urls := util.SplitTrimSpace(str, "\n")
+ for _, u := range urls {
+ if !IsValidURL(u) {
+ ok = false
+ errs.Add([]string{name}, binding.ERR_URL, u)
+ }
+ }
+
+ return ok, errs
+ },
+ })
+}
+
func addValidURLBindingRule() {
// URL validation rule
binding.AddRule(&binding.Rule{
IsMatch: func(rule string) bool {
- return strings.HasPrefix(rule, "ValidUrl")
+ return rule == "ValidUrl"
},
IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
str := fmt.Sprintf("%v", val)
@@ -83,7 +112,7 @@ func addValidSiteURLBindingRule() {
// URL validation rule
binding.AddRule(&binding.Rule{
IsMatch: func(rule string) bool {
- return strings.HasPrefix(rule, "ValidSiteUrl")
+ return rule == "ValidSiteUrl"
},
IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
str := fmt.Sprintf("%v", val)
@@ -174,7 +203,7 @@ func addUsernamePatternRule() {
func addValidGroupTeamMapRule() {
binding.AddRule(&binding.Rule{
IsMatch: func(rule string) bool {
- return strings.HasPrefix(rule, "ValidGroupTeamMap")
+ return rule == "ValidGroupTeamMap"
},
IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
_, err := auth.UnmarshalGroupTeamMapping(fmt.Sprintf("%v", val))
diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go
index 4f1031abf3..5adcdf0289 100644
--- a/modules/validation/binding_test.go
+++ b/modules/validation/binding_test.go
@@ -27,6 +27,7 @@ type (
TestForm struct {
BranchName string `form:"BranchName" binding:"GitRefName"`
URL string `form:"ValidUrl" binding:"ValidUrl"`
+ URLs string `form:"ValidUrls" binding:"ValidUrlList"`
GlobPattern string `form:"GlobPattern" binding:"GlobPattern"`
RegexPattern string `form:"RegexPattern" binding:"RegexPattern"`
}
diff --git a/modules/validation/validurllist_test.go b/modules/validation/validurllist_test.go
new file mode 100644
index 0000000000..506f96da69
--- /dev/null
+++ b/modules/validation/validurllist_test.go
@@ -0,0 +1,157 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package validation
+
+import (
+ "testing"
+
+ "code.forgejo.org/go-chi/binding"
+)
+
+// This is a copy of all the URL tests cases, plus additional ones to
+// account for multiple URLs
+var urlListValidationTestCases = []validationTestCase{
+ {
+ description: "Empty URL",
+ data: TestForm{
+ URLs: "",
+ },
+ expectedErrors: binding.Errors{},
+ },
+ {
+ description: "URL without port",
+ data: TestForm{
+ URLs: "http://test.lan/",
+ },
+ expectedErrors: binding.Errors{},
+ },
+ {
+ description: "URL with port",
+ data: TestForm{
+ URLs: "http://test.lan:3000/",
+ },
+ expectedErrors: binding.Errors{},
+ },
+ {
+ description: "URL with IPv6 address without port",
+ data: TestForm{
+ URLs: "http://[::1]/",
+ },
+ expectedErrors: binding.Errors{},
+ },
+ {
+ description: "URL with IPv6 address with port",
+ data: TestForm{
+ URLs: "http://[::1]:3000/",
+ },
+ expectedErrors: binding.Errors{},
+ },
+ {
+ description: "Invalid URL",
+ data: TestForm{
+ URLs: "http//test.lan/",
+ },
+ expectedErrors: binding.Errors{
+ binding.Error{
+ FieldNames: []string{"URLs"},
+ Classification: binding.ERR_URL,
+ Message: "http//test.lan/",
+ },
+ },
+ },
+ {
+ description: "Invalid schema",
+ data: TestForm{
+ URLs: "ftp://test.lan/",
+ },
+ expectedErrors: binding.Errors{
+ binding.Error{
+ FieldNames: []string{"URLs"},
+ Classification: binding.ERR_URL,
+ Message: "ftp://test.lan/",
+ },
+ },
+ },
+ {
+ description: "Invalid port",
+ data: TestForm{
+ URLs: "http://test.lan:3x4/",
+ },
+ expectedErrors: binding.Errors{
+ binding.Error{
+ FieldNames: []string{"URLs"},
+ Classification: binding.ERR_URL,
+ Message: "http://test.lan:3x4/",
+ },
+ },
+ },
+ {
+ description: "Invalid port with IPv6 address",
+ data: TestForm{
+ URLs: "http://[::1]:3x4/",
+ },
+ expectedErrors: binding.Errors{
+ binding.Error{
+ FieldNames: []string{"URLs"},
+ Classification: binding.ERR_URL,
+ Message: "http://[::1]:3x4/",
+ },
+ },
+ },
+ {
+ description: "Multi URLs",
+ data: TestForm{
+ URLs: "http://test.lan:3000/\nhttp://test.local/",
+ },
+ expectedErrors: binding.Errors{},
+ },
+ {
+ description: "Multi URLs with newline",
+ data: TestForm{
+ URLs: "http://test.lan:3000/\nhttp://test.local/\n",
+ },
+ expectedErrors: binding.Errors{},
+ },
+ {
+ description: "List with invalid entry",
+ data: TestForm{
+ URLs: "http://test.lan:3000/\nhttp://[::1]:3x4/",
+ },
+ expectedErrors: binding.Errors{
+ binding.Error{
+ FieldNames: []string{"URLs"},
+ Classification: binding.ERR_URL,
+ Message: "http://[::1]:3x4/",
+ },
+ },
+ },
+ {
+ description: "List with two invalid entries",
+ data: TestForm{
+ URLs: "ftp://test.lan:3000/\nhttp://[::1]:3x4/\n",
+ },
+ expectedErrors: binding.Errors{
+ binding.Error{
+ FieldNames: []string{"URLs"},
+ Classification: binding.ERR_URL,
+ Message: "ftp://test.lan:3000/",
+ },
+ binding.Error{
+ FieldNames: []string{"URLs"},
+ Classification: binding.ERR_URL,
+ Message: "http://[::1]:3x4/",
+ },
+ },
+ },
+}
+
+func Test_ValidURLListValidation(t *testing.T) {
+ AddBindingRules()
+
+ for _, testCase := range urlListValidationTestCases {
+ t.Run(testCase.description, func(t *testing.T) {
+ performValidationTest(t, testCase)
+ })
+ }
+}