fine tune DetectInvalidOAuth2ApplicationRedirectURI

Author: wxiaoguang

services/forms/user_form.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
+	"code.gitea.io/gitea/modules/validation"
 	"code.gitea.io/gitea/modules/web/middleware"
 	"code.gitea.io/gitea/services/context"
 
@@ -367,9 +368,7 @@ type EditOAuth2ApplicationForm struct {
 func DetectInvalidOAuth2ApplicationRedirectURI(uris []string) (invalidURL string) {
 	for _, u := range uris {
 		scheme, _, ok := strings.Cut(u, ":")
-		valid := ok && (strings.EqualFold(scheme, "http") ||
-			strings.EqualFold(scheme, "https") ||
-			util.SliceContainsString(setting.OAuth2.CustomSchemes, scheme, true))
+		valid := ok && (validation.IsValidURL(u) || util.SliceContainsString(setting.OAuth2.CustomSchemes, scheme))
 		if !valid {
 			return u
 		}

services/forms/user_form_test.go

@@ -14,15 +14,9 @@ import (
 )
 
 func TestRegisterForm_IsDomainAllowed_Empty(t *testing.T) {
-	oldService := setting.Service
-	defer func() {
-		setting.Service = oldService
-	}()
-
+	defer test.MockVariableValue(&setting.Service)()
 	setting.Service.EmailDomainAllowList = nil
-
 	form := RegisterForm{}
-
 	assert.True(t, form.IsEmailDomainAllowed())
 }
 
@@ -87,3 +81,30 @@ func TestRegisterForm_IsDomainAllowed_BlockedEmail(t *testing.T) {
 		assert.Equal(t, v.valid, form.IsEmailDomainAllowed())
 	}
 }
+
+func TestDetectInvalidOAuth2ApplicationRedirectURI(t *testing.T) {
+	defer test.MockVariableValue(&setting.OAuth2.CustomSchemes)()
+	setting.OAuth2.CustomSchemes = []string{"my-app"}
+	assertValid := func(t *testing.T, s string, valid bool) {
+		ret := DetectInvalidOAuth2ApplicationRedirectURI([]string{s})
+		if valid {
+			assert.Empty(t, ret)
+		} else {
+			assert.Equal(t, s, ret)
+		}
+	}
+	assertValid(t, "my-app:", true)
+	assertValid(t, "my-app:/foo", true)
+	assertValid(t, "http://foo", true)
+	assertValid(t, "https://foo", true)
+
+	assertValid(t, "my-app", false)
+	assertValid(t, "ftp:", false)
+	assertValid(t, "ftp://foo", false)
+	assertValid(t, "https://[invalid", false)
+
+	ret := DetectInvalidOAuth2ApplicationRedirectURI([]string{"my-app:", "http://foo", "https://foo"})
+	assert.Empty(t, ret)
+	ret = DetectInvalidOAuth2ApplicationRedirectURI([]string{"my-app:", "http://foo", "invalid", "https://foo"})
+	assert.Equal(t, "invalid", ret)
+}