[bugfix] Accept non-multipart forms for account updates (#1896)

* [bugfix] Update Swagger schema per max_profile_fields addition * [bugfix] Accept non-multipart forms for account updates
2023-06-16 12:16:04 +03:00 · 2023-06-16 12:16:04 +03:00 · 0fa06c0cde
commit 0fa06c0cde
parent 827cc4df56
3 changed files with 128 additions and 1 deletions
--- a/docs/api/swagger.yaml
+++ b/docs/api/swagger.yaml
@ -1147,6 +1147,13 @@ definitions:
                format: int64
                type: integer
                x-go-name: MaxFeaturedTags
+            max_profile_fields:
+                description: |-
+                    The maximum number of profile fields allowed for each account.
+                    Currently not configurable, so this is hardcoded to 6. (https://github.com/superseriousbusiness/gotosocial/issues/1876)
+                format: int64
+                type: integer
+                x-go-name: MaxProfileFields
        title: InstanceConfigurationAccounts models instance account config parameters.
        type: object
        x-go-name: InstanceConfigurationAccounts
@ -3144,6 +3151,7 @@ paths:
        patch:
            consumes:
                - multipart/form-data
+                - application/x-www-form-urlencoded
                - application/json
            operationId: accountUpdate
            parameters:
--- a/internal/api/client/accounts/accountupdate.go
+++ b/internal/api/client/accounts/accountupdate.go
@ -43,6 +43,7 @@ import (
 //
 //	consumes:
 //	- multipart/form-data
+//	- application/x-www-form-urlencoded
 //	- application/json
 //
 //	produces:
@ -213,6 +214,17 @@ func parseUpdateAccountForm(c *gin.Context) (*apimodel.UpdateCredentialsRequest,
 		if err != nil {
 			return nil, fmt.Errorf("custom json binding failed: %w", err)
 		}
+	case binding.MIMEPOSTForm:
+		// Bind with default form binding first.
+		if err := c.ShouldBindWith(form, binding.FormPost); err != nil {
+			return nil, err
+		}
+
+		// Now use custom form binding for
+		// field attributes in the form data.
+		if err := c.ShouldBindWith(form, fieldsAttributesFormBinding{}); err != nil {
+			return nil, fmt.Errorf("custom form binding failed: %w", err)
+		}
 	case binding.MIMEMultipartPOSTForm:
 		// Bind with default form binding first.
 		if err := c.ShouldBindWith(form, binding.FormMultipart); err != nil {
@ -225,7 +237,7 @@ func parseUpdateAccountForm(c *gin.Context) (*apimodel.UpdateCredentialsRequest,
 			return nil, fmt.Errorf("custom form binding failed: %w", err)
 		}
 	default:
-		err := fmt.Errorf("content-type %s not supported for this endpoint; supported content-types are %s, %s", ct, binding.MIMEJSON, binding.MIMEMultipartPOSTForm)
+		err := fmt.Errorf("content-type %s not supported for this endpoint; supported content-types are %s, %s, %s", ct, binding.MIMEJSON, binding.MIMEPOSTForm, binding.MIMEMultipartPOSTForm)
 		return nil, err
 	}

--- a/internal/api/client/accounts/accountupdate_test.go
+++ b/internal/api/client/accounts/accountupdate_test.go
@ -24,6 +24,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"

 	"github.com/stretchr/testify/suite"
@ -37,6 +38,14 @@ type AccountUpdateTestSuite struct {
 	AccountStandardTestSuite
 }

+func (suite *AccountUpdateTestSuite) updateAccountFromForm(data map[string]string, expectedHTTPStatus int, expectedBody string) (*apimodel.Account, error) {
+	form := url.Values{}
+	for key, val := range data {
+		form[key] = []string{val}
+	}
+	return suite.updateAccount([]byte(form.Encode()), "application/x-www-form-urlencoded", expectedHTTPStatus, expectedBody)
+}
+
 func (suite *AccountUpdateTestSuite) updateAccountFromFormData(data map[string]string, expectedHTTPStatus int, expectedBody string) (*apimodel.Account, error) {
 	requestBody, w, err := testrig.CreateMultipartFormData("", "", data)
 	if err != nil {
@ -106,6 +115,32 @@ func (suite *AccountUpdateTestSuite) updateAccount(
 	return resp, nil
 }

+func (suite *AccountUpdateTestSuite) TestUpdateAccountBasicForm() {
+	data := map[string]string{
+		"note":                        "this is my new bio read it and weep",
+		"fields_attributes[0][name]":  "pronouns",
+		"fields_attributes[0][value]": "they/them",
+		"fields_attributes[1][name]":  "Website",
+		"fields_attributes[1][value]": "https://example.com",
+	}
+
+	apimodelAccount, err := suite.updateAccountFromForm(data, http.StatusOK, "")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.Equal("<p>this is my new bio read it and weep</p>", apimodelAccount.Note)
+	suite.Equal("this is my new bio read it and weep", apimodelAccount.Source.Note)
+
+	if l := len(apimodelAccount.Fields); l != 2 {
+		suite.FailNow("", "expected %d fields, got %d", 2, l)
+	}
+	suite.Equal(`pronouns`, apimodelAccount.Fields[0].Name)
+	suite.Equal(`they/them`, apimodelAccount.Fields[0].Value)
+	suite.Equal(`Website`, apimodelAccount.Fields[1].Name)
+	suite.Equal(`<a href="https://example.com" rel="nofollow noreferrer noopener" target="_blank">https://example.com</a>`, apimodelAccount.Fields[1].Value)
+}
+
 func (suite *AccountUpdateTestSuite) TestUpdateAccountBasicFormData() {
 	data := map[string]string{
 		"note":                        "this is my new bio read it and weep",
@ -166,6 +201,19 @@ func (suite *AccountUpdateTestSuite) TestUpdateAccountBasicJSON() {
 	suite.Equal(`<a href="https://example.com" rel="nofollow noreferrer noopener" target="_blank">https://example.com</a>`, apimodelAccount.Fields[1].Value)
 }

+func (suite *AccountUpdateTestSuite) TestUpdateAccountLockForm() {
+	data := map[string]string{
+		"locked": "true",
+	}
+
+	apimodelAccount, err := suite.updateAccountFromForm(data, http.StatusOK, "")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.True(apimodelAccount.Locked)
+}
+
 func (suite *AccountUpdateTestSuite) TestUpdateAccountLockFormData() {
 	data := map[string]string{
 		"locked": "true",
@ -193,6 +241,19 @@ func (suite *AccountUpdateTestSuite) TestUpdateAccountLockJSON() {
 	suite.True(apimodelAccount.Locked)
 }

+func (suite *AccountUpdateTestSuite) TestUpdateAccountUnlockForm() {
+	data := map[string]string{
+		"locked": "false",
+	}
+
+	apimodelAccount, err := suite.updateAccountFromForm(data, http.StatusOK, "")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.False(apimodelAccount.Locked)
+}
+
 func (suite *AccountUpdateTestSuite) TestUpdateAccountUnlockFormData() {
 	data := map[string]string{
 		"locked": "false",
@ -240,6 +301,24 @@ func (suite *AccountUpdateTestSuite) TestUpdateAccountCache() {
 	suite.Equal("<p>this is my new bio read it and weep</p>", apimodelAccount.Note)
 }

+func (suite *AccountUpdateTestSuite) TestUpdateAccountDiscoverableForm() {
+	data := map[string]string{
+		"discoverable": "false",
+	}
+
+	apimodelAccount, err := suite.updateAccountFromForm(data, http.StatusOK, "")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.False(apimodelAccount.Discoverable)
+
+	// Check the account in the database too.
+	dbZork, err := suite.db.GetAccountByID(context.Background(), apimodelAccount.ID)
+	suite.NoError(err)
+	suite.False(*dbZork.Discoverable)
+}
+
 func (suite *AccountUpdateTestSuite) TestUpdateAccountDiscoverableFormData() {
 	data := map[string]string{
 		"discoverable": "false",
@ -302,6 +381,15 @@ func (suite *AccountUpdateTestSuite) TestUpdateAccountWithImageFormData() {
 	suite.NotEqual("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.jpg", apimodelAccount.HeaderStatic)
 }

+func (suite *AccountUpdateTestSuite) TestUpdateAccountEmptyForm() {
+	data := make(map[string]string)
+
+	_, err := suite.updateAccountFromForm(data, http.StatusBadRequest, `{"error":"Bad Request: empty form submitted"}`)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+}
+
 func (suite *AccountUpdateTestSuite) TestUpdateAccountEmptyFormData() {
 	data := make(map[string]string)

@ -311,6 +399,25 @@ func (suite *AccountUpdateTestSuite) TestUpdateAccountEmptyFormData() {
 	}
 }

+func (suite *AccountUpdateTestSuite) TestUpdateAccountSourceForm() {
+	data := map[string]string{
+		"source[privacy]":   string(apimodel.VisibilityPrivate),
+		"source[language]":  "de",
+		"source[sensitive]": "true",
+		"locked":            "true",
+	}
+
+	apimodelAccount, err := suite.updateAccountFromForm(data, http.StatusOK, "")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.Equal(data["source[language]"], apimodelAccount.Source.Language)
+	suite.EqualValues(apimodel.VisibilityPrivate, apimodelAccount.Source.Privacy)
+	suite.True(apimodelAccount.Source.Sensitive)
+	suite.True(apimodelAccount.Locked)
+}
+
 func (suite *AccountUpdateTestSuite) TestUpdateAccountSourceFormData() {
 	data := map[string]string{
 		"source[privacy]":   string(apimodel.VisibilityPrivate),