From 8eda0051eced29fe039e9fc1c50e4011b6c9545b Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 12 May 2023 11:17:31 +0200 Subject: [PATCH] [bugfix] Ensure account fields can be set by JSON (#1762) --- docs/api/swagger.yaml | 3 +- internal/api/client/accounts/accountupdate.go | 101 ++- .../api/client/accounts/accountupdate_test.go | 631 ++++++++---------- internal/api/model/account.go | 43 +- 4 files changed, 353 insertions(+), 425 deletions(-) diff --git a/docs/api/swagger.yaml b/docs/api/swagger.yaml index ddb31bc68..fb76bd6a2 100644 --- a/docs/api/swagger.yaml +++ b/docs/api/swagger.yaml @@ -2469,7 +2469,7 @@ definitions: x-go-name: Tag x-go-package: github.com/superseriousbusiness/gotosocial/internal/api/model updateField: - description: By default, max 4 fields and 255 characters per property/value. + description: By default, max 6 fields and 255 characters per property/value. properties: name: description: Name of the field @@ -3088,6 +3088,7 @@ paths: patch: consumes: - multipart/form-data + - application/json operationId: accountUpdate parameters: - description: Account should be made discoverable and shown in the profile directory (if enabled). diff --git a/internal/api/client/accounts/accountupdate.go b/internal/api/client/accounts/accountupdate.go index 6d0da95e3..26af29118 100644 --- a/internal/api/client/accounts/accountupdate.go +++ b/internal/api/client/accounts/accountupdate.go @@ -24,11 +24,13 @@ import ( "strconv" "github.com/gin-gonic/gin" + "github.com/gin-gonic/gin/binding" "github.com/go-playground/form/v4" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/oauth" + "golang.org/x/exp/slices" ) // AccountUpdateCredentialsPATCHHandler swagger:operation PATCH /api/v1/accounts/update_credentials accountUpdate @@ -41,6 +43,7 @@ import ( // // consumes: // - multipart/form-data +// - application/json // // produces: // - application/json @@ -169,26 +172,26 @@ func (m *Module) AccountUpdateCredentialsPATCHHandler(c *gin.Context) { c.JSON(http.StatusOK, acctSensitive) } -type fieldAttributesBinding struct{} +// fieldsAttributesFormBinding satisfies gin's binding.Binding interface. +// Should only be used specifically for multipart/form-data MIME type. +type fieldsAttributesFormBinding struct{} -func (fieldAttributesBinding) Name() string { - return "FieldAttributes" +func (fieldsAttributesFormBinding) Name() string { + return "FieldsAttributes" } -func (fieldAttributesBinding) Bind(req *http.Request, obj any) error { +func (fieldsAttributesFormBinding) Bind(req *http.Request, obj any) error { if err := req.ParseForm(); err != nil { return err } + // Change default namespace prefix and suffix to + // allow correct parsing of the field attributes. decoder := form.NewDecoder() - // change default namespace prefix and suffix to allow correct parsing of the field attributes decoder.SetNamespacePrefix("[") decoder.SetNamespaceSuffix("]") - if err := decoder.Decode(obj, req.Form); err != nil { - return err - } - return nil + return decoder.Decode(obj, req.Form) } func parseUpdateAccountForm(c *gin.Context) (*apimodel.UpdateCredentialsRequest, error) { @@ -196,36 +199,34 @@ func parseUpdateAccountForm(c *gin.Context) (*apimodel.UpdateCredentialsRequest, Source: &apimodel.UpdateSource{}, } - if err := c.ShouldBind(&form); err != nil { - return nil, fmt.Errorf("could not parse form from request: %s", err) - } - - // use custom form binding to support field attributes in the form data - if err := c.ShouldBindWith(&form, fieldAttributesBinding{}); err != nil { - return nil, fmt.Errorf("could not parse form from request: %s", err) - } - - // parse source field-by-field - sourceMap := c.PostFormMap("source") - - if privacy, ok := sourceMap["privacy"]; ok { - form.Source.Privacy = &privacy - } - - if sensitive, ok := sourceMap["sensitive"]; ok { - sensitiveBool, err := strconv.ParseBool(sensitive) - if err != nil { - return nil, fmt.Errorf("error parsing form source[sensitive]: %s", err) + switch ct := c.ContentType(); ct { + case binding.MIMEJSON: + // Bind with default json binding first. + if err := c.ShouldBindWith(form, binding.JSON); err != nil { + return nil, err } - form.Source.Sensitive = &sensitiveBool - } - if language, ok := sourceMap["language"]; ok { - form.Source.Language = &language - } + // Now use custom form binding for + // field attributes in the json data. + var err error + form.FieldsAttributes, err = parseFieldsAttributesFromJSON(form.JSONFieldsAttributes) + if err != nil { + return nil, fmt.Errorf("custom json binding failed: %w", err) + } + case binding.MIMEMultipartPOSTForm: + // Bind with default form binding first. + if err := c.ShouldBindWith(form, binding.FormMultipart); err != nil { + return nil, err + } - if statusContentType, ok := sourceMap["status_content_type"]; ok { - form.Source.StatusContentType = &statusContentType + // 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) + } + default: + err := fmt.Errorf("content-type %s not supported for this endpoint; supported content-types are %s, %s", ct, binding.MIMEJSON, binding.MIMEMultipartPOSTForm) + return nil, err } if form == nil || @@ -248,3 +249,31 @@ func parseUpdateAccountForm(c *gin.Context) (*apimodel.UpdateCredentialsRequest, return form, nil } + +func parseFieldsAttributesFromJSON(jsonFieldsAttributes *map[string]apimodel.UpdateField) (*[]apimodel.UpdateField, error) { + if jsonFieldsAttributes == nil { + // Nothing set, nothing to do. + return nil, nil + } + + fieldsAttributes := make([]apimodel.UpdateField, 0, len(*jsonFieldsAttributes)) + for keyStr, updateField := range *jsonFieldsAttributes { + key, err := strconv.Atoi(keyStr) + if err != nil { + return nil, fmt.Errorf("couldn't parse fieldAttributes key %s to int: %w", keyStr, err) + } + + fieldsAttributes = append(fieldsAttributes, apimodel.UpdateField{ + Key: key, + Name: updateField.Name, + Value: updateField.Value, + }) + } + + // Sort slice by the key each field was submitted with. + slices.SortFunc(fieldsAttributes, func(a, b apimodel.UpdateField) bool { + return a.Key < b.Key + }) + + return &fieldsAttributes, nil +} diff --git a/internal/api/client/accounts/accountupdate_test.go b/internal/api/client/accounts/accountupdate_test.go index f073bac95..7b2ab0713 100644 --- a/internal/api/client/accounts/accountupdate_test.go +++ b/internal/api/client/accounts/accountupdate_test.go @@ -20,7 +20,8 @@ package accounts_test import ( "context" "encoding/json" - "io/ioutil" + "fmt" + "io" "net/http" "net/http/httptest" "testing" @@ -28,6 +29,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/api/client/accounts" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -35,460 +37,351 @@ type AccountUpdateTestSuite struct { AccountStandardTestSuite } -func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandler() { - // set up the request - // we're updating the note and profile fields of zork - newBio := "this is my new bio read it and weep" - requestBody, w, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "note": newBio, - "fields_attributes[0][name]": "pronouns", - "fields_attributes[0][value]": "they/them", - }) +func (suite *AccountUpdateTestSuite) updateAccountFromFormData(data map[string]string, expectedHTTPStatus int, expectedBody string) (*apimodel.Account, error) { + requestBody, w, err := testrig.CreateMultipartFormData("", "", data) if err != nil { - panic(err) + suite.FailNow(err.Error()) } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler + return suite.updateAccount(requestBody.Bytes(), w.FormDataContentType(), expectedHTTPStatus, expectedBody) +} + +func (suite *AccountUpdateTestSuite) updateAccountFromFormDataWithFile(fieldName string, fileName string, data map[string]string, expectedHTTPStatus int, expectedBody string) (*apimodel.Account, error) { + requestBody, w, err := testrig.CreateMultipartFormData(fieldName, fileName, data) + if err != nil { + suite.FailNow(err.Error()) + } + + return suite.updateAccount(requestBody.Bytes(), w.FormDataContentType(), expectedHTTPStatus, expectedBody) +} + +func (suite *AccountUpdateTestSuite) updateAccountFromJSON(data string, expectedHTTPStatus int, expectedBody string) (*apimodel.Account, error) { + return suite.updateAccount([]byte(data), "application/json", expectedHTTPStatus, expectedBody) +} + +func (suite *AccountUpdateTestSuite) updateAccount( + bodyBytes []byte, + contentType string, + expectedHTTPStatus int, + expectedBody string, +) (*apimodel.Account, error) { + // Initialize http test context. + recorder := httptest.NewRecorder() + ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, contentType) + + // Trigger the handler. suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder.Code) - - // 2. we should have no error message in the result body + // Read the result. result := recorder.Result() defer result.Body.Close() - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) + b, err := io.ReadAll(result.Body) + if err != nil { + return nil, err + } - // unmarshal the returned account - apimodelAccount := &apimodel.Account{} - err = json.Unmarshal(b, apimodelAccount) - suite.NoError(err) + errs := gtserror.MultiError{} + + // Check expected code + body. + if resultCode := recorder.Code; expectedHTTPStatus != resultCode { + errs = append(errs, fmt.Sprintf("expected %d got %d", expectedHTTPStatus, resultCode)) + } + + // If we got an expected body, return early. + if expectedBody != "" && string(b) != expectedBody { + errs = append(errs, fmt.Sprintf("expected %s got %s", expectedBody, string(b))) + } + + if err := errs.Combine(); err != nil { + return nil, fmt.Errorf("%v (body %s)", err, string(b)) + } + + // Return account response. + resp := &apimodel.Account{} + if err := json.Unmarshal(b, resp); err != nil { + return nil, err + } + + return resp, nil +} + +func (suite *AccountUpdateTestSuite) TestUpdateAccountBasicFormData() { + 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.updateAccountFromFormData(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } - // check the returned api model account - // fields should be updated suite.Equal("

this is my new bio read it and weep

", apimodelAccount.Note) - suite.Equal("they/them", apimodelAccount.Fields[0].Value) - suite.Equal(newBio, apimodelAccount.Source.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) TestAccountUpdateCredentialsPATCHHandlerUnlockLock() { - // set up the first request - requestBody1, w1, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "locked": "false", - }) - if err != nil { - panic(err) - } - bodyBytes1 := requestBody1.Bytes() - recorder1 := httptest.NewRecorder() - ctx1 := suite.newContext(recorder1, http.MethodPatch, bodyBytes1, accounts.UpdateCredentialsPath, w1.FormDataContentType()) - - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx1) - - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder1.Code) - - // 2. we should have no error message in the result body - result1 := recorder1.Result() - defer result1.Body.Close() - - // check the response - b1, err := ioutil.ReadAll(result1.Body) - suite.NoError(err) - - // unmarshal the returned account - apimodelAccount1 := &apimodel.Account{} - err = json.Unmarshal(b1, apimodelAccount1) - suite.NoError(err) - - // check the returned api model account - // fields should be updated - suite.False(apimodelAccount1.Locked) - - // set up the first request - requestBody2, w2, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "locked": "true", - }) - if err != nil { - panic(err) - } - bodyBytes2 := requestBody2.Bytes() - recorder2 := httptest.NewRecorder() - ctx2 := suite.newContext(recorder2, http.MethodPatch, bodyBytes2, accounts.UpdateCredentialsPath, w2.FormDataContentType()) - - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx2) - - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder1.Code) - - // 2. we should have no error message in the result body - result2 := recorder2.Result() - defer result2.Body.Close() - - // check the response - b2, err := ioutil.ReadAll(result2.Body) - suite.NoError(err) - - // unmarshal the returned account - apimodelAccount2 := &apimodel.Account{} - err = json.Unmarshal(b2, apimodelAccount2) - suite.NoError(err) - - // check the returned api model account - // fields should be updated - suite.True(apimodelAccount2.Locked) +func (suite *AccountUpdateTestSuite) TestUpdateAccountBasicJSON() { + data := ` +{ + "note": "this is my new bio read it and weep", + "fields_attributes": { + "0": { + "name": "pronouns", + "value": "they/them" + }, + "1": { + "name": "Website", + "value": "https://example.com" + } + } } +` -func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerGetAccountFirst() { - // get the account first to make sure it's in the database cache -- when the account is updated via - // the PATCH handler, it should invalidate the cache and not return the old version - _, err := suite.db.GetAccountByID(context.Background(), suite.testAccounts["local_account_1"].ID) - suite.NoError(err) - - // set up the request - // we're updating the note of zork - newBio := "this is my new bio read it and weep" - requestBody, w, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "note": newBio, - }) + apimodelAccount, err := suite.updateAccountFromJSON(data, http.StatusOK, "") if err != nil { - panic(err) + suite.FailNow(err.Error()) } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) - - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder.Code) - - // 2. we should have no error message in the result body - result := recorder.Result() - defer result.Body.Close() - - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) - - // unmarshal the returned account - apimodelAccount := &apimodel.Account{} - err = json.Unmarshal(b, apimodelAccount) - suite.NoError(err) - - // check the returned api model account - // fields should be updated suite.Equal("

this is my new bio read it and weep

", apimodelAccount.Note) - suite.Equal(newBio, apimodelAccount.Source.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) TestAccountUpdateCredentialsPATCHHandlerTwoFields() { - // set up the request - // we're updating the note of zork, and setting locked to true - newBio := "this is my new bio read it and weep :rainbow:" - requestBody, w, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "note": newBio, - "locked": "true", - }) - if err != nil { - panic(err) +func (suite *AccountUpdateTestSuite) TestUpdateAccountLockFormData() { + data := map[string]string{ + "locked": "true", } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) + apimodelAccount, err := suite.updateAccountFromFormData(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder.Code) - - // 2. we should have no error message in the result body - result := recorder.Result() - defer result.Body.Close() - - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) - - // unmarshal the returned account - apimodelAccount := &apimodel.Account{} - err = json.Unmarshal(b, apimodelAccount) - suite.NoError(err) - - // check the returned api model account - // fields should be updated - suite.Equal("

this is my new bio read it and weep :rainbow:

", apimodelAccount.Note) - suite.Equal(newBio, apimodelAccount.Source.Note) suite.True(apimodelAccount.Locked) - suite.NotEmpty(apimodelAccount.Emojis) - suite.Equal(apimodelAccount.Emojis[0].Shortcode, "rainbow") - - // check the account in the database - dbZork, err := suite.db.GetAccountByID(context.Background(), apimodelAccount.ID) - suite.NoError(err) - suite.Equal(newBio, dbZork.NoteRaw) - suite.Equal("

this is my new bio read it and weep :rainbow:

", dbZork.Note) - suite.True(*dbZork.Locked) - suite.NotEmpty(dbZork.EmojiIDs) } -func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerDiscoverable() { - requestBody, w, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "discoverable": "false", - }) +func (suite *AccountUpdateTestSuite) TestUpdateAccountLockJSON() { + data := ` +{ + "locked": true +}` + + apimodelAccount, err := suite.updateAccountFromJSON(data, http.StatusOK, "") if err != nil { - panic(err) + suite.FailNow(err.Error()) } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) + suite.True(apimodelAccount.Locked) +} - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder.Code) +func (suite *AccountUpdateTestSuite) TestUpdateAccountUnlockFormData() { + data := map[string]string{ + "locked": "false", + } - // 2. we should have no error message in the result body - result := recorder.Result() - defer result.Body.Close() + apimodelAccount, err := suite.updateAccountFromFormData(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) + suite.False(apimodelAccount.Locked) +} - // unmarshal the returned account - apimodelAccount := &apimodel.Account{} - err = json.Unmarshal(b, apimodelAccount) - suite.NoError(err) +func (suite *AccountUpdateTestSuite) TestUpdateAccountUnlockJSON() { + data := ` +{ + "locked": false +}` + + apimodelAccount, err := suite.updateAccountFromJSON(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } + + suite.False(apimodelAccount.Locked) +} + +func (suite *AccountUpdateTestSuite) TestUpdateAccountCache() { + // Get the account first to make sure it's in the database + // cache. When the account is updated via the PATCH handler, + // it should invalidate the cache and return the new version. + if _, err := suite.db.GetAccountByID(context.Background(), suite.testAccounts["local_account_1"].ID); err != nil { + suite.FailNow(err.Error()) + } + + data := map[string]string{ + "note": "this is my new bio read it and weep", + } + + apimodelAccount, err := suite.updateAccountFromFormData(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Equal("

this is my new bio read it and weep

", apimodelAccount.Note) +} + +func (suite *AccountUpdateTestSuite) TestUpdateAccountDiscoverableFormData() { + data := map[string]string{ + "discoverable": "false", + } + + apimodelAccount, err := suite.updateAccountFromFormData(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } - // check the returned api model account - // fields should be updated suite.False(apimodelAccount.Discoverable) - // check the account in the database + // 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) TestAccountUpdateCredentialsPATCHHandlerWithMedia() { - // set up the request - // we're updating the header image, the display name, and the locked status of zork - // we're removing the note/bio - requestBody, w, err := testrig.CreateMultipartFormData( - "header", "../../../../testrig/media/test-jpeg.jpg", - map[string]string{ - "display_name": "updated zork display name!!!", - "note": "", - "locked": "true", - }) +func (suite *AccountUpdateTestSuite) TestUpdateAccountDiscoverableJSON() { + data := ` +{ + "discoverable": false +}` + + apimodelAccount, err := suite.updateAccountFromJSON(data, http.StatusOK, "") if err != nil { - panic(err) + suite.FailNow(err.Error()) } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) + suite.False(apimodelAccount.Discoverable) - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder.Code) - - // 2. we should have no error message in the result body - result := recorder.Result() - defer result.Body.Close() - - // check the response - b, err := ioutil.ReadAll(result.Body) + // Check the account in the database too. + dbZork, err := suite.db.GetAccountByID(context.Background(), apimodelAccount.ID) suite.NoError(err) + suite.False(*dbZork.Discoverable) +} - // unmarshal the returned account - apimodelAccount := &apimodel.Account{} - err = json.Unmarshal(b, apimodelAccount) - suite.NoError(err) +func (suite *AccountUpdateTestSuite) TestUpdateAccountWithImageFormData() { + data := map[string]string{ + "display_name": "updated zork display name!!!", + "note": "", + "locked": "true", + } - // check the returned api model account - // fields should be updated - suite.Equal("updated zork display name!!!", apimodelAccount.DisplayName) + apimodelAccount, err := suite.updateAccountFromFormDataWithFile("header", "../../../../testrig/media/test-jpeg.jpg", data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Equal(data["display_name"], apimodelAccount.DisplayName) suite.True(apimodelAccount.Locked) suite.Empty(apimodelAccount.Note) suite.Empty(apimodelAccount.Source.Note) - - // header values... - // should be set suite.NotEmpty(apimodelAccount.Header) suite.NotEmpty(apimodelAccount.HeaderStatic) - // should be different from the values set before + // Can't predict IDs generated for new media + // so just ensure it's different than before. suite.NotEqual("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/original/01PFPMWK2FF0D9WMHEJHR07C3Q.jpg", apimodelAccount.Header) suite.NotEqual("http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.jpg", apimodelAccount.HeaderStatic) } -func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerEmptyForm() { - // set up the request - bodyBytes := []byte{} - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, "") +func (suite *AccountUpdateTestSuite) TestUpdateAccountEmptyFormData() { + data := make(map[string]string) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) - - // 1. we should have OK because our request was valid - suite.Equal(http.StatusBadRequest, recorder.Code) - - // 2. we should have no error message in the result body - result := recorder.Result() - defer result.Body.Close() - - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) - suite.Equal(`{"error":"Bad Request: empty form submitted"}`, string(b)) + _, err := suite.updateAccountFromFormData(data, http.StatusBadRequest, `{"error":"Bad Request: empty form submitted"}`) + if err != nil { + suite.FailNow(err.Error()) + } } -func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerUpdateSource() { - // set up the request - // we're updating the language of zork - newLanguage := "de" - requestBody, w, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "source[privacy]": string(apimodel.VisibilityPrivate), - "source[language]": "de", - "source[sensitive]": "true", - "locked": "true", - }) - if err != nil { - panic(err) +func (suite *AccountUpdateTestSuite) TestUpdateAccountSourceFormData() { + data := map[string]string{ + "source[privacy]": string(apimodel.VisibilityPrivate), + "source[language]": "de", + "source[sensitive]": "true", + "locked": "true", } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) + apimodelAccount, err := suite.updateAccountFromFormData(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder.Code) - - // 2. we should have no error message in the result body - result := recorder.Result() - defer result.Body.Close() - - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) - - // unmarshal the returned account - apimodelAccount := &apimodel.Account{} - err = json.Unmarshal(b, apimodelAccount) - suite.NoError(err) - - // check the returned api model account - // fields should be updated - suite.Equal(newLanguage, apimodelAccount.Source.Language) + 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) TestAccountUpdateCredentialsPATCHHandlerUpdateStatusContentTypeOK() { - // set up the request - // we're updating the language of zork - requestBody, w, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "source[status_content_type]": "text/markdown", - }) +func (suite *AccountUpdateTestSuite) TestUpdateAccountSourceJSON() { + data := ` +{ + "source": { + "privacy": "private", + "language": "de", + "sensitive": true + }, + "locked": true +} +` + + apimodelAccount, err := suite.updateAccountFromJSON(data, http.StatusOK, "") if err != nil { - panic(err) + suite.FailNow(err.Error()) } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) + suite.Equal("de", apimodelAccount.Source.Language) + suite.EqualValues(apimodel.VisibilityPrivate, apimodelAccount.Source.Privacy) + suite.True(apimodelAccount.Source.Sensitive) + suite.True(apimodelAccount.Locked) +} - // 1. we should have OK because our request was valid - suite.Equal(http.StatusOK, recorder.Code) +func (suite *AccountUpdateTestSuite) TestUpdateAccountSourceBadContentTypeFormData() { + data := map[string]string{ + "source[status_content_type]": "text/markdown", + } - // 2. we should have no error message in the result body - result := recorder.Result() - defer result.Body.Close() + apimodelAccount, err := suite.updateAccountFromFormData(data, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) - - // unmarshal the returned account - apimodelAccount := &apimodel.Account{} - err = json.Unmarshal(b, apimodelAccount) - suite.NoError(err) - - // check the returned api model account - // fields should be updated - suite.Equal("text/markdown", apimodelAccount.Source.StatusContentType) + suite.Equal(data["source[status_content_type]"], apimodelAccount.Source.StatusContentType) + // Check the account in the database too. dbAccount, err := suite.db.GetAccountByID(context.Background(), suite.testAccounts["local_account_1"].ID) if err != nil { suite.FailNow(err.Error()) } - suite.Equal(dbAccount.StatusContentType, "text/markdown") + suite.Equal(data["source[status_content_type]"], dbAccount.StatusContentType) } func (suite *AccountUpdateTestSuite) TestAccountUpdateCredentialsPATCHHandlerUpdateStatusContentTypeBad() { - // set up the request - // we're updating the language of zork - requestBody, w, err := testrig.CreateMultipartFormData( - "", "", - map[string]string{ - "source[status_content_type]": "peepeepoopoo", - }) - if err != nil { - panic(err) + data := map[string]string{ + "source[status_content_type]": "peepeepoopoo", } - bodyBytes := requestBody.Bytes() - recorder := httptest.NewRecorder() - ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, accounts.UpdateCredentialsPath, w.FormDataContentType()) - // call the handler - suite.accountsModule.AccountUpdateCredentialsPATCHHandler(ctx) - - suite.Equal(http.StatusBadRequest, recorder.Code) - - result := recorder.Result() - defer result.Body.Close() - - // check the response - b, err := ioutil.ReadAll(result.Body) - suite.NoError(err) - - suite.Equal(`{"error":"Bad Request: status content type 'peepeepoopoo' was not recognized, valid options are 'text/plain', 'text/markdown'"}`, string(b)) + _, err := suite.updateAccountFromFormData(data, http.StatusBadRequest, `{"error":"Bad Request: status content type 'peepeepoopoo' was not recognized, valid options are 'text/plain', 'text/markdown'"}`) + if err != nil { + suite.FailNow(err.Error()) + } } func TestAccountUpdateTestSuite(t *testing.T) { diff --git a/internal/api/model/account.go b/internal/api/model/account.go index 36138a618..31615d26b 100644 --- a/internal/api/model/account.go +++ b/internal/api/model/account.go @@ -140,27 +140,29 @@ type AccountCreateRequest struct { // swagger:ignore type UpdateCredentialsRequest struct { // Account should be made discoverable and shown in the profile directory (if enabled). - Discoverable *bool `form:"discoverable" json:"discoverable" xml:"discoverable"` + Discoverable *bool `form:"discoverable" json:"discoverable"` // Account is flagged as a bot. - Bot *bool `form:"bot" json:"bot" xml:"bot"` + Bot *bool `form:"bot" json:"bot"` // The display name to use for the account. - DisplayName *string `form:"display_name" json:"display_name" xml:"display_name"` + DisplayName *string `form:"display_name" json:"display_name"` // Bio/description of this account. - Note *string `form:"note" json:"note" xml:"note"` + Note *string `form:"note" json:"note"` // Avatar image encoded using multipart/form-data. - Avatar *multipart.FileHeader `form:"avatar" json:"avatar" xml:"avatar"` + Avatar *multipart.FileHeader `form:"avatar" json:"-"` // Header image encoded using multipart/form-data - Header *multipart.FileHeader `form:"header" json:"header" xml:"header"` + Header *multipart.FileHeader `form:"header" json:"-"` // Require manual approval of follow requests. - Locked *bool `form:"locked" json:"locked" xml:"locked"` + Locked *bool `form:"locked" json:"locked"` // New Source values for this account. - Source *UpdateSource `form:"source" json:"source" xml:"source"` - // Profile metadata name and value - FieldsAttributes *[]UpdateField `form:"fields_attributes" json:"fields_attributes" xml:"fields_attributes"` + Source *UpdateSource `form:"source" json:"source"` + // Profile metadata names and values. + FieldsAttributes *[]UpdateField `form:"fields_attributes" json:"-"` + // Profile metadata names and values, parsed from JSON. + JSONFieldsAttributes *map[string]UpdateField `form:"-" json:"fields_attributes"` // Custom CSS to be included when rendering this account's profile or statuses. - CustomCSS *string `form:"custom_css" json:"custom_css" xml:"custom_css"` + CustomCSS *string `form:"custom_css" json:"custom_css"` // Enable RSS feed of public toots for this account at /@[username]/feed.rss - EnableRSS *bool `form:"enable_rss" json:"enable_rss" xml:"enable_rss"` + EnableRSS *bool `form:"enable_rss" json:"enable_rss"` } // UpdateSource is to be used specifically in an UpdateCredentialsRequest. @@ -168,24 +170,27 @@ type UpdateCredentialsRequest struct { // swagger:model updateSource type UpdateSource struct { // Default post privacy for authored statuses. - Privacy *string `form:"privacy" json:"privacy" xml:"privacy"` + Privacy *string `form:"privacy" json:"privacy"` // Mark authored statuses as sensitive by default. - Sensitive *bool `form:"sensitive" json:"sensitive" xml:"sensitive"` + Sensitive *bool `form:"sensitive" json:"sensitive"` // Default language to use for authored statuses. (ISO 6391) - Language *string `form:"language" json:"language" xml:"language"` + Language *string `form:"language" json:"language"` // Default format for authored statuses (text/plain or text/markdown). - StatusContentType *string `form:"status_content_type" json:"status_content_type" xml:"status_content_type"` + StatusContentType *string `form:"status_content_type" json:"status_content_type"` } // UpdateField is to be used specifically in an UpdateCredentialsRequest. -// By default, max 4 fields and 255 characters per property/value. +// By default, max 6 fields and 255 characters per property/value. // // swagger:model updateField type UpdateField struct { + // Key this form field was submitted with; + // only set if it was submitted as JSON. + Key int `form:"-" json:"-"` // Name of the field - Name *string `form:"name" json:"name" xml:"name"` + Name *string `form:"name" json:"name"` // Value of the field - Value *string `form:"value" json:"value" xml:"value"` + Value *string `form:"value" json:"value"` } // AccountFollowRequest models a request to follow an account.