From 0fa06c0cde19ba5a4b77aa8372509cf9116fc546 Mon Sep 17 00:00:00 2001 From: Umar Getagazov Date: Fri, 16 Jun 2023 12:16:04 +0300 Subject: [PATCH] [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 --- docs/api/swagger.yaml | 8 ++ internal/api/client/accounts/accountupdate.go | 14 ++- .../api/client/accounts/accountupdate_test.go | 107 ++++++++++++++++++ 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/docs/api/swagger.yaml b/docs/api/swagger.yaml index 34a3dd7d1..839793f82 100644 --- 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: diff --git a/internal/api/client/accounts/accountupdate.go b/internal/api/client/accounts/accountupdate.go index 26af29118..9c51f5924 100644 --- 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 } diff --git a/internal/api/client/accounts/accountupdate_test.go b/internal/api/client/accounts/accountupdate_test.go index 7b2ab0713..f6bff4825 100644 --- 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("

this is my new bio read it and weep

", 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(`https://example.com`, 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(`https://example.com`, 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("

this is my new bio read it and weep

", 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),