From d9bbcc60a6cd32282de907a2090c674c4616219e Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 31 Mar 2023 15:01:29 +0200 Subject: [PATCH] [bugfix] Fix report serialization errors caused by user delete (#1659) * [bugfix] Fix report serialization errors caused by user delete * fix tests --- internal/api/client/admin/reportsget_test.go | 12 +- internal/processing/account/delete.go | 70 +++++- internal/processing/account/delete_test.go | 102 ++++++++ internal/typeutils/converter_test.go | 23 +- internal/typeutils/internaltofrontend.go | 11 +- internal/typeutils/internaltofrontend_test.go | 232 +++++++++++++++++- 6 files changed, 430 insertions(+), 20 deletions(-) create mode 100644 internal/processing/account/delete_test.go diff --git a/internal/api/client/admin/reportsget_test.go b/internal/api/client/admin/reportsget_test.go index 8135ab6ca..67e6103ae 100644 --- a/internal/api/client/admin/reportsget_test.go +++ b/internal/api/client/admin/reportsget_test.go @@ -195,7 +195,7 @@ func (suite *ReportsGetTestSuite) TestReportsGetAll() { "ip": "118.44.18.196", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "user" }, @@ -240,7 +240,7 @@ func (suite *ReportsGetTestSuite) TestReportsGetAll() { "ip": "89.122.255.1", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "admin" }, @@ -286,7 +286,7 @@ func (suite *ReportsGetTestSuite) TestReportsGetAll() { "ip": "89.122.255.1", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "admin" }, @@ -345,7 +345,7 @@ func (suite *ReportsGetTestSuite) TestReportsGetAll() { "ip": "118.44.18.196", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "user" }, @@ -546,7 +546,7 @@ func (suite *ReportsGetTestSuite) TestReportsGetCreatedByAccount() { "ip": "118.44.18.196", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "user" }, @@ -747,7 +747,7 @@ func (suite *ReportsGetTestSuite) TestReportsGetTargetAccount() { "ip": "118.44.18.196", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "user" }, diff --git a/internal/processing/account/delete.go b/internal/processing/account/delete.go index f3dfecc7b..2a20ec96e 100644 --- a/internal/processing/account/delete.go +++ b/internal/processing/account/delete.go @@ -21,15 +21,18 @@ "context" "errors" "fmt" + "net" "time" "codeberg.org/gruf/go-kv" + "github.com/google/uuid" "github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/messages" + "golang.org/x/crypto/bcrypt" ) const deleteSelectLimit = 50 @@ -136,8 +139,13 @@ func (p *Processor) deleteUserAndTokensForAccount(ctx context.Context, account * } } - if err := p.state.DB.DeleteUserByID(ctx, user.ID); err != nil { - return fmt.Errorf("deleteUserAndTokensForAccount: db error deleting user: %w", err) + columns, err := stubbifyUser(user) + if err != nil { + return fmt.Errorf("deleteUserAndTokensForAccount: error stubbifying user: %w", err) + } + + if err := p.state.DB.UpdateUser(ctx, user, columns...); err != nil { + return fmt.Errorf("deleteUserAndTokensForAccount: db error updating user: %w", err) } return nil @@ -473,3 +481,61 @@ func stubbifyAccount(account *gtsmodel.Account, origin string) []string { "enable_rss", } } + +// stubbifyUser renders the given user as a stub, +// removing sensitive information like IP addresses +// and sign-in times, but keeping email addresses to +// prevent the same email address from creating another +// account on this instance. +// +// `encrypted_password` is set to the bcrypt hash of a +// random uuid, so if the action is reversed, the user +// will have to reset their password via email. +// +// For caller's convenience, this function returns the db +// names of all columns that are updated by it. +func stubbifyUser(user *gtsmodel.User) ([]string, error) { + uuid, err := uuid.New().MarshalBinary() + if err != nil { + return nil, err + } + + dummyPassword, err := bcrypt.GenerateFromPassword(uuid, bcrypt.DefaultCost) + if err != nil { + return nil, err + } + + var never = time.Time{} + + user.EncryptedPassword = string(dummyPassword) + user.SignUpIP = net.IPv4zero + user.CurrentSignInAt = never + user.CurrentSignInIP = net.IPv4zero + user.LastSignInAt = never + user.LastSignInIP = net.IPv4zero + user.SignInCount = 1 + user.Locale = "" + user.CreatedByApplicationID = "" + user.LastEmailedAt = never + user.ConfirmationToken = "" + user.ConfirmationSentAt = never + user.ResetPasswordToken = "" + user.ResetPasswordSentAt = never + + return []string{ + "encrypted_password", + "sign_up_ip", + "current_sign_in_at", + "current_sign_in_ip", + "last_sign_in_at", + "last_sign_in_ip", + "sign_in_count", + "locale", + "created_by_application_id", + "last_emailed_at", + "confirmation_token", + "confirmation_sent_at", + "reset_password_token", + "reset_password_sent_at", + }, nil +} diff --git a/internal/processing/account/delete_test.go b/internal/processing/account/delete_test.go new file mode 100644 index 000000000..5a68eda0c --- /dev/null +++ b/internal/processing/account/delete_test.go @@ -0,0 +1,102 @@ +// GoToSocial +// Copyright (C) GoToSocial Authors admin@gotosocial.org +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package account_test + +import ( + "context" + "net" + "testing" + "time" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" +) + +type AccountDeleteTestSuite struct { + AccountStandardTestSuite +} + +func (suite *AccountDeleteTestSuite) TestAccountDeleteLocal() { + ctx := context.Background() + + // Keep a reference around to the original account + // and user, before the delete was processed. + ogAccount := suite.testAccounts["local_account_1"] + ogUser := suite.testUsers["local_account_1"] + + testAccount := >smodel.Account{} + *testAccount = *ogAccount + + suspensionOrigin := "01GWVP2A8J38Q2J2FDZ6TS8AQG" + if err := suite.accountProcessor.Delete(ctx, testAccount, suspensionOrigin); err != nil { + suite.FailNow(err.Error()) + } + + updatedAccount, err := suite.db.GetAccountByID(ctx, ogAccount.ID) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.WithinDuration(time.Now(), updatedAccount.UpdatedAt, 1*time.Minute) + suite.Zero(updatedAccount.FetchedAt) + suite.Zero(updatedAccount.AvatarMediaAttachmentID) + suite.Zero(updatedAccount.AvatarRemoteURL) + suite.Zero(updatedAccount.HeaderMediaAttachmentID) + suite.Zero(updatedAccount.HeaderRemoteURL) + suite.Zero(updatedAccount.DisplayName) + suite.Nil(updatedAccount.EmojiIDs) + suite.Nil(updatedAccount.Emojis) + suite.Nil(updatedAccount.Fields) + suite.Zero(updatedAccount.Note) + suite.Zero(updatedAccount.NoteRaw) + suite.False(*updatedAccount.Memorial) + suite.Zero(updatedAccount.AlsoKnownAs) + suite.Zero(updatedAccount.Reason) + suite.False(*updatedAccount.Discoverable) + suite.Zero(updatedAccount.StatusContentType) + suite.Zero(updatedAccount.CustomCSS) + suite.WithinDuration(time.Now(), updatedAccount.SuspendedAt, 1*time.Minute) + suite.Equal(suspensionOrigin, updatedAccount.SuspensionOrigin) + suite.True(*updatedAccount.HideCollections) + suite.False(*updatedAccount.EnableRSS) + + updatedUser, err := suite.db.GetUserByAccountID(ctx, testAccount.ID) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.WithinDuration(time.Now(), updatedUser.UpdatedAt, 1*time.Minute) + suite.NotEqual(updatedUser.EncryptedPassword, ogUser.EncryptedPassword) + suite.Equal(net.IPv4zero, updatedUser.SignUpIP) + suite.Zero(updatedUser.CurrentSignInAt) + suite.Equal(net.IPv4zero, updatedUser.CurrentSignInIP) + suite.Zero(updatedUser.LastSignInAt) + suite.Equal(net.IPv4zero, updatedUser.LastSignInIP) + suite.Equal(1, updatedUser.SignInCount) + suite.Zero(updatedUser.Locale) + suite.Zero(updatedUser.CreatedByApplicationID) + suite.Zero(updatedUser.LastEmailedAt) + suite.Zero(updatedUser.ConfirmationToken) + suite.Zero(updatedUser.ConfirmationSentAt) + suite.Zero(updatedUser.ResetPasswordToken) + suite.Zero(updatedUser.ResetPasswordSentAt) +} + +func TestAccountDeleteTestSuite(t *testing.T) { + suite.Run(t, new(AccountDeleteTestSuite)) +} diff --git a/internal/typeutils/converter_test.go b/internal/typeutils/converter_test.go index 88c0256c8..d92a30c13 100644 --- a/internal/typeutils/converter_test.go +++ b/internal/typeutils/converter_test.go @@ -22,6 +22,7 @@ "github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/processing" "github.com/superseriousbusiness/gotosocial/internal/state" "github.com/superseriousbusiness/gotosocial/internal/typeutils" "github.com/superseriousbusiness/gotosocial/testrig" @@ -482,13 +483,17 @@ type TypeUtilsTestSuite struct { typeconverter typeutils.TypeConverter } -func (suite *TypeUtilsTestSuite) SetupSuite() { +func (suite *TypeUtilsTestSuite) SetupTest() { suite.state.Caches.Init() testrig.InitTestConfig() testrig.InitTestLog() suite.db = testrig.NewTestDB(&suite.state) + suite.state.DB = suite.db + storage := testrig.NewInMemoryStorage() + suite.state.Storage = storage + suite.testAccounts = testrig.NewTestAccounts() suite.testStatuses = testrig.NewTestStatuses() suite.testAttachments = testrig.NewTestAttachments() @@ -497,13 +502,23 @@ func (suite *TypeUtilsTestSuite) SetupSuite() { suite.testReports = testrig.NewTestReports() suite.testMentions = testrig.NewTestMentions() suite.typeconverter = typeutils.NewConverter(suite.db) -} -func (suite *TypeUtilsTestSuite) SetupTest() { - suite.state.Caches.Init() // reset testrig.StandardDBSetup(suite.db, nil) } func (suite *TypeUtilsTestSuite) TearDownTest() { testrig.StandardDBTeardown(suite.db) + testrig.StopWorkers(&suite.state) +} + +// GetProcessor is a utility function that instantiates a processor. +// Useful when a test in the test suite needs to change some state. +func (suite *TypeUtilsTestSuite) GetProcessor() *processing.Processor { + testrig.StartWorkers(&suite.state) + httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") + transportController := testrig.NewTestTransportController(&suite.state, httpClient) + mediaManager := testrig.NewTestMediaManager(&suite.state) + federator := testrig.NewTestFederator(&suite.state, transportController, mediaManager) + emailSender := testrig.NewEmailSender("../../web/template/", nil) + return testrig.NewTestProcessor(&suite.state, federator, emailSender, mediaManager) } diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index 198bed099..74b061fb0 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -270,7 +270,7 @@ func (c *converter) AccountToAdminAPIAccount(ctx context.Context, a *gtsmodel.Ac ) // take user-level information if possible - if a.Domain != "" { + if a.IsRemote() { domain = &a.Domain } else { user, err := c.db.GetUserByAccountID(ctx, a.ID) @@ -289,7 +289,9 @@ func (c *converter) AccountToAdminAPIAccount(ctx context.Context, a *gtsmodel.Ac } locale = user.Locale - inviteRequest = &user.Account.Reason + if user.Account.Reason != "" { + inviteRequest = &user.Account.Reason + } if *user.Admin { role.Name = apimodel.AccountRoleAdmin } else if *user.Moderator { @@ -298,11 +300,12 @@ func (c *converter) AccountToAdminAPIAccount(ctx context.Context, a *gtsmodel.Ac confirmed = !user.ConfirmedAt.IsZero() approved = *user.Approved disabled = *user.Disabled - silenced = !user.Account.SilencedAt.IsZero() - suspended = !user.Account.SuspendedAt.IsZero() createdByApplicationID = user.CreatedByApplicationID } + silenced = !a.SilencedAt.IsZero() + suspended = !a.SuspendedAt.IsZero() + apiAccount, err := c.AccountToAPIAccountPublic(ctx, a) if err != nil { return nil, fmt.Errorf("AccountToAdminAPIAccount: error converting account to api account for account id %s: %w", a.ID, err) diff --git a/internal/typeutils/internaltofrontend_test.go b/internal/typeutils/internaltofrontend_test.go index 40e074fe3..993dc920b 100644 --- a/internal/typeutils/internaltofrontend_test.go +++ b/internal/typeutils/internaltofrontend_test.go @@ -26,6 +26,7 @@ "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/testrig" ) type InternalToFrontendTestSuite struct { @@ -908,7 +909,7 @@ func (suite *InternalToFrontendTestSuite) TestAdminReportToFrontend1() { "ip": "118.44.18.196", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "user" }, @@ -953,7 +954,7 @@ func (suite *InternalToFrontendTestSuite) TestAdminReportToFrontend1() { "ip": "89.122.255.1", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "admin" }, @@ -999,7 +1000,7 @@ func (suite *InternalToFrontendTestSuite) TestAdminReportToFrontend1() { "ip": "89.122.255.1", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "admin" }, @@ -1068,7 +1069,7 @@ func (suite *InternalToFrontendTestSuite) TestAdminReportToFrontend2() { "ip": "118.44.18.196", "ips": [], "locale": "en", - "invite_request": "", + "invite_request": null, "role": { "name": "user" }, @@ -1234,6 +1235,229 @@ func (suite *InternalToFrontendTestSuite) TestAdminReportToFrontend2() { }`, string(b)) } +func (suite *InternalToFrontendTestSuite) TestAdminReportToFrontendSuspendedLocalAccount() { + ctx := context.Background() + requestingAccount := suite.testAccounts["admin_account"] + reportedAccount := >smodel.Account{} + *reportedAccount = *suite.testAccounts["local_account_2"] + + // Suspend/delete the reported account. + if err := suite.GetProcessor().Account().Delete(ctx, reportedAccount, requestingAccount.ID); err != nil { + suite.FailNow(err.Error()) + } + + // Wait for the delete to process. Stubbifying + // the account is the last part of the delete, + // so once it's stubbified we know we're done. + if !testrig.WaitFor(func() bool { + dbAccount, err := suite.db.GetAccountByID(ctx, reportedAccount.ID) + if err != nil { + suite.FailNow(err.Error()) + } + return dbAccount.DisplayName == "" + }) { + suite.FailNow("timed out waiting for account delete") + } + + adminReport, err := suite.typeconverter.ReportToAdminAPIReport(context.Background(), suite.testReports["remote_account_1_report_local_account_2"], requestingAccount) + suite.NoError(err) + + b, err := json.MarshalIndent(adminReport, "", " ") + suite.NoError(err) + + suite.Equal(`{ + "id": "01GP3DFY9XQ1TJMZT5BGAZPXX7", + "action_taken": true, + "action_taken_at": "2022-05-15T15:01:56.000Z", + "category": "other", + "comment": "this is a turtle, not a person, therefore should not be a poster", + "forwarded": true, + "created_at": "2022-05-15T14:20:12.000Z", + "updated_at": "2022-05-15T14:20:12.000Z", + "account": { + "id": "01F8MH5ZK5VRH73AKHQM6Y9VNX", + "username": "foss_satan", + "domain": "fossbros-anonymous.io", + "created_at": "2021-09-26T10:52:36.000Z", + "email": "", + "ip": null, + "ips": [], + "locale": "", + "invite_request": null, + "role": { + "name": "user" + }, + "confirmed": false, + "approved": false, + "disabled": false, + "silenced": false, + "suspended": false, + "account": { + "id": "01F8MH5ZK5VRH73AKHQM6Y9VNX", + "username": "foss_satan", + "acct": "foss_satan@fossbros-anonymous.io", + "display_name": "big gerald", + "locked": false, + "discoverable": true, + "bot": false, + "created_at": "2021-09-26T10:52:36.000Z", + "note": "i post about like, i dunno, stuff, or whatever!!!!", + "url": "http://fossbros-anonymous.io/@foss_satan", + "avatar": "", + "avatar_static": "", + "header": "http://localhost:8080/assets/default_header.png", + "header_static": "http://localhost:8080/assets/default_header.png", + "followers_count": 0, + "following_count": 0, + "statuses_count": 1, + "last_status_at": "2021-09-20T10:40:37.000Z", + "emojis": [], + "fields": [] + } + }, + "target_account": { + "id": "01F8MH5NBDF2MV7CTC4Q5128HF", + "username": "1happyturtle", + "domain": null, + "created_at": "2022-06-04T13:12:00.000Z", + "email": "tortle.dude@example.org", + "ip": "0.0.0.0", + "ips": [], + "locale": "", + "invite_request": null, + "role": { + "name": "user" + }, + "confirmed": true, + "approved": true, + "disabled": false, + "silenced": false, + "suspended": true, + "account": { + "id": "01F8MH5NBDF2MV7CTC4Q5128HF", + "username": "1happyturtle", + "acct": "1happyturtle", + "display_name": "", + "locked": true, + "discoverable": false, + "bot": false, + "created_at": "2022-06-04T13:12:00.000Z", + "note": "", + "url": "http://localhost:8080/@1happyturtle", + "avatar": "", + "avatar_static": "", + "header": "http://localhost:8080/assets/default_header.png", + "header_static": "http://localhost:8080/assets/default_header.png", + "followers_count": 0, + "following_count": 0, + "statuses_count": 0, + "last_status_at": null, + "emojis": [], + "fields": [], + "suspended": true, + "role": { + "name": "user" + } + } + }, + "assigned_account": { + "id": "01F8MH17FWEB39HZJ76B6VXSKF", + "username": "admin", + "domain": null, + "created_at": "2022-05-17T13:10:59.000Z", + "email": "admin@example.org", + "ip": "89.122.255.1", + "ips": [], + "locale": "en", + "invite_request": null, + "role": { + "name": "admin" + }, + "confirmed": true, + "approved": true, + "disabled": false, + "silenced": false, + "suspended": false, + "account": { + "id": "01F8MH17FWEB39HZJ76B6VXSKF", + "username": "admin", + "acct": "admin", + "display_name": "", + "locked": false, + "discoverable": true, + "bot": false, + "created_at": "2022-05-17T13:10:59.000Z", + "note": "", + "url": "http://localhost:8080/@admin", + "avatar": "", + "avatar_static": "", + "header": "http://localhost:8080/assets/default_header.png", + "header_static": "http://localhost:8080/assets/default_header.png", + "followers_count": 1, + "following_count": 1, + "statuses_count": 4, + "last_status_at": "2021-10-20T10:41:37.000Z", + "emojis": [], + "fields": [], + "enable_rss": true, + "role": { + "name": "admin" + } + }, + "created_by_application_id": "01F8MGXQRHYF5QPMTMXP78QC2F" + }, + "action_taken_by_account": { + "id": "01F8MH17FWEB39HZJ76B6VXSKF", + "username": "admin", + "domain": null, + "created_at": "2022-05-17T13:10:59.000Z", + "email": "admin@example.org", + "ip": "89.122.255.1", + "ips": [], + "locale": "en", + "invite_request": null, + "role": { + "name": "admin" + }, + "confirmed": true, + "approved": true, + "disabled": false, + "silenced": false, + "suspended": false, + "account": { + "id": "01F8MH17FWEB39HZJ76B6VXSKF", + "username": "admin", + "acct": "admin", + "display_name": "", + "locked": false, + "discoverable": true, + "bot": false, + "created_at": "2022-05-17T13:10:59.000Z", + "note": "", + "url": "http://localhost:8080/@admin", + "avatar": "", + "avatar_static": "", + "header": "http://localhost:8080/assets/default_header.png", + "header_static": "http://localhost:8080/assets/default_header.png", + "followers_count": 1, + "following_count": 1, + "statuses_count": 4, + "last_status_at": "2021-10-20T10:41:37.000Z", + "emojis": [], + "fields": [], + "enable_rss": true, + "role": { + "name": "admin" + } + }, + "created_by_application_id": "01F8MGXQRHYF5QPMTMXP78QC2F" + }, + "statuses": [], + "rule_ids": [], + "action_taken_comment": "user was warned not to be a turtle anymore" +}`, string(b)) +} + func TestInternalToFrontendTestSuite(t *testing.T) { suite.Run(t, new(InternalToFrontendTestSuite)) }