mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-06-22 10:02:15 +00:00
feat: in-browser validation of website URLs for user, repository, and organization profiles (#12991)
This PR pertains to the client-side validation of the Website input on user, repo, and org profiles. #12962 extends `[service].VALID_SITE_URL_SCHEMES` to cover Website fields on repo and org profiles, where before that config key only applied to the one on user profiles. If that change merges, it will then be possible to construct an HTML [`pattern`](https://developer.mozilla.org/docs/Web/HTML/Reference/Elements/input#pattern) attribute for general use on any Website form input that the server validates this way, thus enabling browsers to catch errors early relating to URL scheme confusion. This PR (1) introduces such a `pattern` attribute, and (2) adds a new UI note to make clear to users which URL schemes are permitted. This change helps explain the browser's otherwise cryptic error messages regarding pattern mismatch, while also letting users know what URI schemes the Forgejo instance supports as Website links (e.g. gemini:// URLs).  This MUST NOT merge before #12962. To do so would introduce a regression wherein the UI may suggest and validate a different set of allowed URL schemes than the server actually permits. See also #5519 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12991 Reviewed-by: 0ko <0ko@noreply.codeberg.org>
This commit is contained in:
parent
0e283c5485
commit
408572dd35
8 changed files with 98 additions and 4 deletions
|
|
@ -288,6 +288,10 @@ func loadServiceFrom(rootCfg ConfigProvider) {
|
|||
}
|
||||
}
|
||||
Service.ValidSiteURLSchemes = schemes
|
||||
|
||||
// A pattern from ValidSiteURLSchemes must be valid for use in HTML <input pattern=""> validation
|
||||
_ = regexp.MustCompile(`^(?:` + ValidSiteURLPattern() + `)$`)
|
||||
|
||||
Service.UsernameCooldownPeriod = sec.Key("USERNAME_COOLDOWN_PERIOD").MustInt64(0)
|
||||
|
||||
// Only set a default if USERNAME_COOLDOWN_PERIOD's feature is active.
|
||||
|
|
@ -302,6 +306,26 @@ func loadServiceFrom(rootCfg ConfigProvider) {
|
|||
loadOpenIDSetting(rootCfg)
|
||||
}
|
||||
|
||||
// Returns a regex pattern string based on the current value of
|
||||
// `Service.ValidSiteURLSchemes`.
|
||||
//
|
||||
// This pattern string is meant to be used as the value of an <input> element's
|
||||
// `pattern` attribute. As such, this function assumes that the pattern will be
|
||||
// implicitly wrapped with `^(?:` and `)$`, such that the match is required
|
||||
// against the entire input value, i.e., `^(?:<pattern>)$`.
|
||||
//
|
||||
// See [MDN] for more details.
|
||||
//
|
||||
// [MDN]: https://developer.mozilla.org/docs/Web/HTML/Reference/Elements/input#pattern
|
||||
func ValidSiteURLPattern() string {
|
||||
// While technically possible, returning a compiled Regexp from this seems heavy-handed.
|
||||
// We could store a compiled Regexp instead of generating on the fly, but that complicates testing!
|
||||
// Much easier to simply edit `Service.ValidSiteURLSchemes for a given test and proceed.
|
||||
// We run the compiler against the initial config value anyway, so this string always works in production.
|
||||
schemes := strings.Join(Service.ValidSiteURLSchemes, "|")
|
||||
return `(` + schemes + `)://.+`
|
||||
}
|
||||
|
||||
func loadOpenIDSetting(rootCfg ConfigProvider) {
|
||||
sec := rootCfg.Section("openid")
|
||||
Service.EnableOpenIDSignIn = sec.Key("ENABLE_OPENID_SIGNIN").MustBool(!InstallLock)
|
||||
|
|
|
|||
|
|
@ -937,6 +937,7 @@
|
|||
"markup.filepreview.lines": "Lines %[1]d to %[2]d in %[3]s",
|
||||
"markup.filepreview.truncated": "Preview has been truncated",
|
||||
"form.RunnerName": "Name",
|
||||
"form.website.valid_url_schemes": "Allowed URL schemes include: %s",
|
||||
"graphs.recent_commits.title": "Number of commits in the past year",
|
||||
"graphs.code_frequency.title": "Code frequency over the history of {0}",
|
||||
"meta.last_line": "Thank you for translating Forgejo! This line isn't seen by the users but it serves other purposes in the translation management. You can place a fun fact in the translation instead of translating it."
|
||||
|
|
|
|||
|
|
@ -146,6 +146,8 @@ func Contexter() func(next http.Handler) http.Handler {
|
|||
ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this
|
||||
ctx.Data["CurrentURL"] = setting.AppSubURL + req.URL.RequestURI()
|
||||
ctx.Data["Link"] = ctx.Link
|
||||
ctx.Data["ValidSiteURLSchemes"] = setting.Service.ValidSiteURLSchemes
|
||||
ctx.Data["ValidSiteURLPattern"] = setting.ValidSiteURLPattern()
|
||||
|
||||
// PageData is passed by reference, and it will be rendered to `window.config.pageData` in `head.tmpl` for JavaScript modules
|
||||
ctx.PageData = map[string]any{}
|
||||
|
|
|
|||
|
|
@ -97,7 +97,8 @@
|
|||
|
||||
<div class="field {{if .Err_Website}}error{{end}}">
|
||||
<label for="website">{{ctx.Locale.Tr "settings.website"}}</label>
|
||||
<input id="website" name="website" type="url" value="{{.User.Website}}" placeholder="http://mydomain.com or https://mydomain.com" maxlength="255">
|
||||
<input id="website" name="website" type="url" value="{{.User.Website}}" maxlength="255" pattern="{{.ValidSiteURLPattern}}">
|
||||
<span class="help">{{ctx.Locale.Tr "form.website.valid_url_schemes" (StringUtils.Join .ValidSiteURLSchemes ", ")}}</span>
|
||||
</div>
|
||||
<div class="field {{if .Err_Location}}error{{end}}">
|
||||
<label for="location">{{ctx.Locale.Tr "settings.location"}}</label>
|
||||
|
|
|
|||
|
|
@ -31,7 +31,8 @@
|
|||
</div>
|
||||
<div class="field {{if .Err_Website}}error{{end}}">
|
||||
<label for="website">{{ctx.Locale.Tr "org.settings.website"}}</label>
|
||||
<input id="website" name="website" type="url" value="{{.Org.Website}}" maxlength="255">
|
||||
<input id="website" name="website" type="url" value="{{.Org.Website}}" maxlength="255" pattern="{{.ValidSiteURLPattern}}">
|
||||
<span class="help">{{ctx.Locale.Tr "form.website.valid_url_schemes" (StringUtils.Join .ValidSiteURLSchemes ", ")}}</span>
|
||||
</div>
|
||||
<div class="field">
|
||||
<label for="location">{{ctx.Locale.Tr "org.settings.location"}}</label>
|
||||
|
|
|
|||
|
|
@ -42,7 +42,8 @@
|
|||
</div>
|
||||
<div class="field {{if .Err_Website}}error{{end}}">
|
||||
<label for="website">{{ctx.Locale.Tr "repo.settings.site"}}</label>
|
||||
<input id="website" name="website" type="url" maxlength="1024" value="{{.Repository.Website}}">
|
||||
<input id="website" name="website" type="url" maxlength="1024" pattern="{{.ValidSiteURLPattern}}" value="{{.Repository.Website}}">
|
||||
<span class="help">{{ctx.Locale.Tr "form.website.valid_url_schemes" (StringUtils.Join .ValidSiteURLSchemes ", ")}}</span>
|
||||
</div>
|
||||
<div class="field">
|
||||
<button class="ui primary button">{{ctx.Locale.Tr "repo.settings.update_settings"}}</button>
|
||||
|
|
|
|||
|
|
@ -46,7 +46,8 @@
|
|||
|
||||
<label {{if .Err_Website}}class="field error"{{end}}>
|
||||
{{ctx.Locale.Tr "settings.website"}}
|
||||
<input name="website" type="url" value="{{.SignedUser.Website}}" maxlength="255">
|
||||
<input name="website" type="url" value="{{.SignedUser.Website}}" maxlength="255" pattern="{{.ValidSiteURLPattern}}">
|
||||
<span class="help">{{ctx.Locale.Tr "form.website.valid_url_schemes" (StringUtils.Join .ValidSiteURLSchemes ", ")}}</span>
|
||||
</label>
|
||||
|
||||
<label>
|
||||
|
|
|
|||
63
tests/integration/form_validation_test.go
Normal file
63
tests/integration/form_validation_test.go
Normal file
|
|
@ -0,0 +1,63 @@
|
|||
// Copyright 2026 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package integration
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"forgejo.org/modules/setting"
|
||||
"forgejo.org/modules/test"
|
||||
"forgejo.org/tests"
|
||||
|
||||
"github.com/PuerkitoBio/goquery"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestWebsitePattern(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
|
||||
cases := [][2]string{
|
||||
// these routes have a "Website" form input whose validation changes based on `[service].VALID_SITE_URL_SCHEMES`:
|
||||
{"admin user edit", "/admin/users/2/edit"},
|
||||
{"org settings", "/org/org3/settings"},
|
||||
{"repo settings", "/user2/repo1/settings"},
|
||||
{"user own settings", "/user/settings"},
|
||||
}
|
||||
for _, testCase := range cases {
|
||||
title := testCase[0]
|
||||
urlStr := testCase[1]
|
||||
|
||||
t.Run(title+" form under default schemes", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
// Website pattern and note should contain only http(s)
|
||||
req := NewRequest(t, "GET", urlStr)
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
doc := NewHTMLParser(t, resp.Body)
|
||||
doc.AssertAttrEqual(t, "input[type=url][name=website]", "pattern", `(http|https)://.+`)
|
||||
doc.AssertElementPredicate(t, "input[type=url][name=website] + .help", func(element *goquery.Selection) {
|
||||
assert.Equal(t, "Allowed URL schemes include: http, https", strings.TrimSpace(element.Text()))
|
||||
})
|
||||
})
|
||||
|
||||
t.Run(title+" form under custom schemes", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
defer test.MockProtect(&setting.Service.ValidSiteURLSchemes)()
|
||||
setting.Service.ValidSiteURLSchemes = append(setting.Service.ValidSiteURLSchemes, "h3")
|
||||
|
||||
// Website pattern and note should contain only http(s) and h3
|
||||
req := NewRequest(t, "GET", urlStr)
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
doc := NewHTMLParser(t, resp.Body)
|
||||
doc.AssertAttrEqual(t, "input[type=url][name=website]", "pattern", `(http|https|h3)://.+`)
|
||||
doc.AssertElementPredicate(t, "input[type=url][name=website] + .help", func(element *goquery.Selection) {
|
||||
assert.Equal(t, "Allowed URL schemes include: http, https, h3", strings.TrimSpace(element.Text()))
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue