refactor: form binding validation (#38832)

Make "form-fetch-action" highlight the invalid field as "error"

routers/web/admin/auths.go

@@ -212,11 +212,11 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source {
 func parseSSPIConfig(ctx *context.Context, form forms.AuthenticationForm) (*sspi.Source, error) {
 	if util.IsEmptyString(form.SSPISeparatorReplacement) {
 		ctx.Data["Err_SSPISeparatorReplacement"] = true
-		return nil, errors.New(ctx.Locale.TrString("form.SSPISeparatorReplacement") + ctx.Locale.TrString("form.require_error"))
+		return nil, errors.New(ctx.Locale.TrString("form.require_error", ctx.Locale.TrString("form.SSPISeparatorReplacement")))
 	}
 	if separatorAntiPattern.MatchString(form.SSPISeparatorReplacement) {
 		ctx.Data["Err_SSPISeparatorReplacement"] = true
-		return nil, errors.New(ctx.Locale.TrString("form.SSPISeparatorReplacement") + ctx.Locale.TrString("form.alpha_dash_dot_error"))
+		return nil, errors.New(ctx.Locale.TrString("form.alpha_dash_dot_error", ctx.Locale.TrString("form.SSPISeparatorReplacement")))
 	}
 
 	if form.SSPIDefaultLanguage != "" && !langCodePattern.MatchString(form.SSPIDefaultLanguage) {

routers/web/repo/setting/deploy_key.go

@@ -4,13 +4,12 @@
 package setting
 
 import (
+	"errors"
 	"net/http"
 
 	asymkey_model "gitea.dev/models/asymkey"
 	"gitea.dev/models/db"
-	"gitea.dev/modules/log"
 	"gitea.dev/modules/setting"
-	"gitea.dev/modules/web"
 	asymkey_service "gitea.dev/services/asymkey"
 	"gitea.dev/services/context"
 	"gitea.dev/services/forms"
@@ -32,69 +31,45 @@ func DeployKeys(ctx *context.Context) {
 	ctx.HTML(http.StatusOK, tplDeployKeys)
 }
 
-// DeployKeysPost response for adding a deploy key of a repository
+// DeployKeysPost response for adding a deploy-key of a repository
 func DeployKeysPost(ctx *context.Context) {
-	form := web.GetForm[*forms.AddKeyForm](ctx)
-	ctx.Data["Title"] = ctx.Tr("repo.settings.deploy_keys")
-	ctx.Data["PageIsSettingsKeys"] = true
-	ctx.Data["DisableSSH"] = setting.SSH.Disabled
-
-	keys, err := db.Find[asymkey_model.DeployKey](ctx, asymkey_model.ListDeployKeysOptions{RepoID: ctx.Repo.Repository.ID})
-	if err != nil {
-		ctx.ServerError("ListDeployKeys", err)
+	form := context.GetFetchActionForm[*forms.AddKeyForm](ctx)
+	if form == nil {
 		return
 	}
-	ctx.Data["Deploykeys"] = keys
-
-	if ctx.HasError() {
-		ctx.HTML(http.StatusOK, tplDeployKeys)
-		return
-	}
-
 	content, err := asymkey_model.CheckPublicKeyString(form.Content)
 	if err != nil {
 		if db.IsErrSSHDisabled(err) {
-			ctx.Flash.Info(ctx.Tr("settings.ssh_disabled"))
+			ctx.JSONError(ctx.Tr("settings.ssh_disabled"))
 		} else if asymkey_model.IsErrKeyUnableVerify(err) {
-			ctx.Flash.Info(ctx.Tr("form.unable_verify_ssh_key"))
-		} else if err == asymkey_model.ErrKeyIsPrivate {
-			ctx.Data["HasError"] = true
-			ctx.Data["Err_Content"] = true
-			ctx.Flash.Error(ctx.Tr("form.must_use_public_key"))
+			ctx.JSONErrorWithField(ctx.Tr("form.unable_verify_ssh_key"), "content")
+		} else if errors.Is(err, asymkey_model.ErrKeyIsPrivate) {
+			ctx.JSONErrorWithField(ctx.Tr("form.must_use_public_key"), "content")
 		} else {
-			ctx.Data["HasError"] = true
-			ctx.Data["Err_Content"] = true
-			ctx.Flash.Error(ctx.Tr("form.invalid_ssh_key", err.Error()))
+			ctx.JSONErrorWithField(ctx.Tr("form.invalid_ssh_key", err.Error()), "content")
 		}
-		ctx.Redirect(ctx.Repo.RepoLink + "/settings/keys")
 		return
 	}
 
 	key, err := asymkey_model.AddDeployKey(ctx, ctx.Repo.Repository.ID, form.Title, content, !form.IsWritable)
 	if err != nil {
-		ctx.Data["HasError"] = true
 		switch {
 		case asymkey_model.IsErrDeployKeyAlreadyExist(err):
-			ctx.Data["Err_Content"] = true
-			ctx.RenderWithErrDeprecated(ctx.Tr("repo.settings.key_been_used"), tplDeployKeys, &form)
+			ctx.JSONErrorWithField(ctx.Tr("repo.settings.key_been_used"), "content")
 		case asymkey_model.IsErrKeyAlreadyExist(err):
-			ctx.Data["Err_Content"] = true
-			ctx.RenderWithErrDeprecated(ctx.Tr("settings.ssh_key_been_used"), tplDeployKeys, &form)
+			ctx.JSONErrorWithField(ctx.Tr("settings.ssh_key_been_used"), "content")
 		case asymkey_model.IsErrKeyNameAlreadyUsed(err):
-			ctx.Data["Err_Title"] = true
-			ctx.RenderWithErrDeprecated(ctx.Tr("repo.settings.key_name_used"), tplDeployKeys, &form)
+			ctx.JSONErrorWithField(ctx.Tr("repo.settings.key_name_used"), "title")
 		case asymkey_model.IsErrDeployKeyNameAlreadyUsed(err):
-			ctx.Data["Err_Title"] = true
-			ctx.RenderWithErrDeprecated(ctx.Tr("repo.settings.key_name_used"), tplDeployKeys, &form)
+			ctx.JSONErrorWithField(ctx.Tr("repo.settings.key_name_used"), "title")
 		default:
 			ctx.ServerError("AddDeployKey", err)
 		}
 		return
 	}
 
-	log.Trace("Deploy key added: %d", ctx.Repo.Repository.ID)
 	ctx.Flash.Success(ctx.Tr("repo.settings.add_key_success", key.Name))
-	ctx.Redirect(ctx.Repo.RepoLink + "/settings/keys")
+	ctx.JSONRedirect(ctx.Repo.RepoLink + "/settings/keys")
 }
 
 // DeleteDeployKey response for deleting a deploy key

routers/web/repo/setting/settings_test.go

@@ -5,6 +5,7 @@ package setting
 
 import (
 	"net/http"
+	"net/url"
 	"testing"
 
 	asymkey_model "gitea.dev/models/asymkey"
@@ -27,53 +28,25 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestAddReadOnlyDeployKey(t *testing.T) {
-	defer test.MockVariableValue(&setting.SSH.RootPath, t.TempDir())()
+func TestAddDeployKey(t *testing.T) {
 	unittest.PrepareTestEnv(t)
-
-	ctx, _ := contexttest.MockContext(t, "user2/repo1/settings/keys")
-
-	contexttest.LoadUser(t, ctx, 2)
-	contexttest.LoadRepo(t, ctx, 2)
-
-	addKeyForm := forms.AddKeyForm{
-		Title:   "read-only",
-		Content: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC4cn+iXnA4KvcQYSV88vGn0Yi91vG47t1P7okprVmhNTkipNRIHWr6WdCO4VDr/cvsRkuVJAsLO2enwjGWWueOO6BodiBgyAOZ/5t5nJNMCNuLGT5UIo/RI1b0WRQwxEZTRjt6mFNw6lH14wRd8ulsr9toSWBPMOGWoYs1PDeDL0JuTjL+tr1SZi/EyxCngpYszKdXllJEHyI79KQgeD0Vt3pTrkbNVTOEcCNqZePSVmUH8X8Vhugz3bnE0/iE9Pb5fkWO9c4AnM1FgI/8Bvp27Fw2ShryIXuR6kKvUqhVMTuOSDHwu6A8jLE5Owt3GAYugDpDYuwTVNGrHLXKpPzrGGPE/jPmaLCMZcsdkec95dYeU3zKODEm8UQZFhmJmDeWVJ36nGrGZHL4J5aTTaeFUJmmXDaJYiJ+K2/ioKgXqnXvltu0A9R8/LGy4nrTJRr4JMLuJFoUXvGm1gXQ70w2LSpk6yl71RNC0hCtsBe8BP8IhYCM0EP5jh7eCMQZNvM= nocomment\n",
-	}
-	web.SetForm(ctx, &addKeyForm)
-	DeployKeysPost(ctx)
-	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
-
-	unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{
-		Name:    addKeyForm.Title,
-		Content: addKeyForm.Content,
-		Mode:    perm.AccessModeRead,
+	t.Run("ReadOnly", func(t *testing.T) {
+		const testKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf\n"
+		ctx, _ := contexttest.MockContext(t, "POST /user2/repo1/settings/keys")
+		contexttest.MockRequestPostForm(ctx.Req, url.Values{"title": {"read-only"}, "content": {testKey}})
+		contexttest.LoadRepo(t, ctx, 2)
+		DeployKeysPost(ctx)
+		assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus())
+		unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{Name: "read-only", Content: testKey, Mode: perm.AccessModeRead})
 	})
-}
-
-func TestAddReadWriteOnlyDeployKey(t *testing.T) {
-	defer test.MockVariableValue(&setting.SSH.RootPath, t.TempDir())()
-
-	unittest.PrepareTestEnv(t)
-
-	ctx, _ := contexttest.MockContext(t, "user2/repo1/settings/keys")
-
-	contexttest.LoadUser(t, ctx, 2)
-	contexttest.LoadRepo(t, ctx, 2)
-
-	addKeyForm := forms.AddKeyForm{
-		Title:      "read-write",
-		Content:    "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC4cn+iXnA4KvcQYSV88vGn0Yi91vG47t1P7okprVmhNTkipNRIHWr6WdCO4VDr/cvsRkuVJAsLO2enwjGWWueOO6BodiBgyAOZ/5t5nJNMCNuLGT5UIo/RI1b0WRQwxEZTRjt6mFNw6lH14wRd8ulsr9toSWBPMOGWoYs1PDeDL0JuTjL+tr1SZi/EyxCngpYszKdXllJEHyI79KQgeD0Vt3pTrkbNVTOEcCNqZePSVmUH8X8Vhugz3bnE0/iE9Pb5fkWO9c4AnM1FgI/8Bvp27Fw2ShryIXuR6kKvUqhVMTuOSDHwu6A8jLE5Owt3GAYugDpDYuwTVNGrHLXKpPzrGGPE/jPmaLCMZcsdkec95dYeU3zKODEm8UQZFhmJmDeWVJ36nGrGZHL4J5aTTaeFUJmmXDaJYiJ+K2/ioKgXqnXvltu0A9R8/LGy4nrTJRr4JMLuJFoUXvGm1gXQ70w2LSpk6yl71RNC0hCtsBe8BP8IhYCM0EP5jh7eCMQZNvM= nocomment\n",
-		IsWritable: true,
-	}
-	web.SetForm(ctx, &addKeyForm)
-	DeployKeysPost(ctx)
-	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
-
-	unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{
-		Name:    addKeyForm.Title,
-		Content: addKeyForm.Content,
-		Mode:    perm.AccessModeWrite,
+	t.Run("ReadWrite", func(t *testing.T) {
+		const testKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIEHjnNEfE88W1pvBLdV3otv28x760gdmPao3lVD5uAt9\n"
+		ctx, _ := contexttest.MockContext(t, "POST /user2/repo1/settings/keys")
+		contexttest.MockRequestPostForm(ctx.Req, url.Values{"title": {"read-write"}, "content": {testKey}, "is_writable": {"on"}})
+		contexttest.LoadRepo(t, ctx, 2)
+		DeployKeysPost(ctx)
+		assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus())
+		unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{Name: "read-write", Content: testKey, Mode: perm.AccessModeWrite})
 	})
 }
 

routers/web/web.go

@@ -1225,7 +1225,7 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) {
 
 		m.Group("/keys", func() {
 			m.Combo("").Get(repo_setting.DeployKeys).
-				Post(web.Bind(forms.AddKeyForm{}), repo_setting.DeployKeysPost)
+				Post(repo_setting.DeployKeysPost)
 			m.Post("/delete", repo_setting.DeleteDeployKey)
 		})