From 4e8c484618f5bfd5b7349cc2afd102cd5feb3861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFck=20Bonniot?= Date: Fri, 2 Oct 2020 18:18:20 +0200 Subject: [PATCH] Implement account deactivation (#1455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implement account deactivation See #610 Signed-off-by: Loïck Bonniot * Rename 'is_active' to 'is_deactivated' Signed-off-by: Loïck Bonniot Co-authored-by: Kegsay --- clientapi/routing/deactivate.go | 55 +++++++++++++++++++ clientapi/routing/routing.go | 9 +++ sytest-whitelist | 3 + userapi/api/api.go | 11 ++++ userapi/internal/api.go | 7 +++ userapi/inthttp/client.go | 19 +++++-- userapi/inthttp/server.go | 13 +++++ userapi/storage/accounts/interface.go | 1 + .../accounts/postgres/accounts_table.go | 20 ++++++- .../deltas/20200929203058_is_active.sql | 9 +++ userapi/storage/accounts/postgres/storage.go | 5 ++ .../accounts/sqlite3/accounts_table.go | 20 ++++++- .../deltas/20200929203058_is_active.sql | 38 +++++++++++++ userapi/storage/accounts/sqlite3/storage.go | 5 ++ 14 files changed, 206 insertions(+), 9 deletions(-) create mode 100644 clientapi/routing/deactivate.go create mode 100644 userapi/storage/accounts/postgres/deltas/20200929203058_is_active.sql create mode 100644 userapi/storage/accounts/sqlite3/deltas/20200929203058_is_active.sql diff --git a/clientapi/routing/deactivate.go b/clientapi/routing/deactivate.go new file mode 100644 index 000000000..effe3769d --- /dev/null +++ b/clientapi/routing/deactivate.go @@ -0,0 +1,55 @@ +package routing + +import ( + "io/ioutil" + "net/http" + + "github.com/matrix-org/dendrite/clientapi/auth" + "github.com/matrix-org/dendrite/clientapi/jsonerror" + "github.com/matrix-org/dendrite/userapi/api" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" +) + +// Deactivate handles POST requests to /account/deactivate +func Deactivate( + req *http.Request, + userInteractiveAuth *auth.UserInteractive, + userAPI api.UserInternalAPI, + deviceAPI *api.Device, +) util.JSONResponse { + ctx := req.Context() + defer req.Body.Close() // nolint:errcheck + bodyBytes, err := ioutil.ReadAll(req.Body) + if err != nil { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON("The request body could not be read: " + err.Error()), + } + } + + login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes, deviceAPI) + if errRes != nil { + return *errRes + } + + localpart, _, err := gomatrixserverlib.SplitID('@', login.User) + if err != nil { + util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed") + return jsonerror.InternalServerError() + } + + var res api.PerformAccountDeactivationResponse + err = userAPI.PerformAccountDeactivation(ctx, &api.PerformAccountDeactivationRequest{ + Localpart: localpart, + }, &res) + if err != nil { + util.GetLogger(ctx).WithError(err).Error("userAPI.PerformAccountDeactivation failed") + return jsonerror.InternalServerError() + } + + return util.JSONResponse{ + Code: http.StatusOK, + JSON: struct{}{}, + } +} diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index ab56c5f4d..8606f69c3 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -435,6 +435,15 @@ func Setup( }), ).Methods(http.MethodPost, http.MethodOptions) + r0mux.Handle("/account/deactivate", + httputil.MakeAuthAPI("deactivate", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + if r := rateLimits.rateLimit(req); r != nil { + return *r + } + return Deactivate(req, userInteractiveAuth, userAPI, device) + }), + ).Methods(http.MethodPost, http.MethodOptions) + // Stub endpoints required by Riot r0mux.Handle("/login", diff --git a/sytest-whitelist b/sytest-whitelist index eaaeea00f..a811259fb 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -219,6 +219,9 @@ Regular users cannot create room aliases within the AS namespace Deleting a non-existent alias should return a 404 Users can't delete other's aliases Outbound federation can query room alias directory +Can deactivate account +Can't deactivate account with wrong password +After deactivating account, can't log in with password After deactivating account, can't log in with an email Remote room alias queries can handle Unicode Newly joined room is included in an incremental sync after invite diff --git a/userapi/api/api.go b/userapi/api/api.go index 3baaa1002..d384c5b18 100644 --- a/userapi/api/api.go +++ b/userapi/api/api.go @@ -30,6 +30,7 @@ type UserInternalAPI interface { PerformDeviceCreation(ctx context.Context, req *PerformDeviceCreationRequest, res *PerformDeviceCreationResponse) error PerformDeviceDeletion(ctx context.Context, req *PerformDeviceDeletionRequest, res *PerformDeviceDeletionResponse) error PerformDeviceUpdate(ctx context.Context, req *PerformDeviceUpdateRequest, res *PerformDeviceUpdateResponse) error + PerformAccountDeactivation(ctx context.Context, req *PerformAccountDeactivationRequest, res *PerformAccountDeactivationResponse) error QueryProfile(ctx context.Context, req *QueryProfileRequest, res *QueryProfileResponse) error QueryAccessToken(ctx context.Context, req *QueryAccessTokenRequest, res *QueryAccessTokenResponse) error QueryDevices(ctx context.Context, req *QueryDevicesRequest, res *QueryDevicesResponse) error @@ -199,6 +200,16 @@ type PerformDeviceCreationResponse struct { Device *Device } +// PerformAccountDeactivationRequest is the request for PerformAccountDeactivation +type PerformAccountDeactivationRequest struct { + Localpart string +} + +// PerformAccountDeactivationResponse is the response for PerformAccountDeactivation +type PerformAccountDeactivationResponse struct { + AccountDeactivated bool +} + // Device represents a client's device (mobile, web, etc) type Device struct { ID string diff --git a/userapi/internal/api.go b/userapi/internal/api.go index 461c548cc..ec8284397 100644 --- a/userapi/internal/api.go +++ b/userapi/internal/api.go @@ -388,3 +388,10 @@ func (a *UserInternalAPI) queryAppServiceToken(ctx context.Context, token, appSe dev.UserID = appService.SenderLocalpart return &dev, nil } + +// PerformAccountDeactivation deactivates the user's account, removing all ability for the user to login again. +func (a *UserInternalAPI) PerformAccountDeactivation(ctx context.Context, req *api.PerformAccountDeactivationRequest, res *api.PerformAccountDeactivationResponse) error { + err := a.AccountDB.DeactivateAccount(ctx, req.Localpart) + res.AccountDeactivated = err == nil + return err +} diff --git a/userapi/inthttp/client.go b/userapi/inthttp/client.go index 6dcaf7568..4d9dcc416 100644 --- a/userapi/inthttp/client.go +++ b/userapi/inthttp/client.go @@ -28,11 +28,12 @@ import ( const ( InputAccountDataPath = "/userapi/inputAccountData" - PerformDeviceCreationPath = "/userapi/performDeviceCreation" - PerformAccountCreationPath = "/userapi/performAccountCreation" - PerformPasswordUpdatePath = "/userapi/performPasswordUpdate" - PerformDeviceDeletionPath = "/userapi/performDeviceDeletion" - PerformDeviceUpdatePath = "/userapi/performDeviceUpdate" + PerformDeviceCreationPath = "/userapi/performDeviceCreation" + PerformAccountCreationPath = "/userapi/performAccountCreation" + PerformPasswordUpdatePath = "/userapi/performPasswordUpdate" + PerformDeviceDeletionPath = "/userapi/performDeviceDeletion" + PerformDeviceUpdatePath = "/userapi/performDeviceUpdate" + PerformAccountDeactivationPath = "/userapi/performAccountDeactivation" QueryProfilePath = "/userapi/queryProfile" QueryAccessTokenPath = "/userapi/queryAccessToken" @@ -126,6 +127,14 @@ func (h *httpUserInternalAPI) PerformDeviceUpdate(ctx context.Context, req *api. return httputil.PostJSON(ctx, span, h.httpClient, apiURL, req, res) } +func (h *httpUserInternalAPI) PerformAccountDeactivation(ctx context.Context, req *api.PerformAccountDeactivationRequest, res *api.PerformAccountDeactivationResponse) error { + span, ctx := opentracing.StartSpanFromContext(ctx, "PerformAccountDeactivation") + defer span.Finish() + + apiURL := h.apiURL + PerformAccountDeactivationPath + return httputil.PostJSON(ctx, span, h.httpClient, apiURL, req, res) +} + func (h *httpUserInternalAPI) QueryProfile( ctx context.Context, request *api.QueryProfileRequest, diff --git a/userapi/inthttp/server.go b/userapi/inthttp/server.go index d26746788..e24aad3a9 100644 --- a/userapi/inthttp/server.go +++ b/userapi/inthttp/server.go @@ -91,6 +91,19 @@ func AddRoutes(internalAPIMux *mux.Router, s api.UserInternalAPI) { return util.JSONResponse{Code: http.StatusOK, JSON: &response} }), ) + internalAPIMux.Handle(PerformAccountDeactivationPath, + httputil.MakeInternalAPI("performAccountDeactivation", func(req *http.Request) util.JSONResponse { + request := api.PerformAccountDeactivationRequest{} + response := api.PerformAccountDeactivationResponse{} + if err := json.NewDecoder(req.Body).Decode(&request); err != nil { + return util.MessageResponse(http.StatusBadRequest, err.Error()) + } + if err := s.PerformAccountDeactivation(req.Context(), &request, &response); err != nil { + return util.ErrorResponse(err) + } + return util.JSONResponse{Code: http.StatusOK, JSON: &response} + }), + ) internalAPIMux.Handle(QueryProfilePath, httputil.MakeInternalAPI("queryProfile", func(req *http.Request) util.JSONResponse { request := api.QueryProfileRequest{} diff --git a/userapi/storage/accounts/interface.go b/userapi/storage/accounts/interface.go index 49446f11f..c86b2c391 100644 --- a/userapi/storage/accounts/interface.go +++ b/userapi/storage/accounts/interface.go @@ -51,6 +51,7 @@ type Database interface { CheckAccountAvailability(ctx context.Context, localpart string) (bool, error) GetAccountByLocalpart(ctx context.Context, localpart string) (*api.Account, error) SearchProfiles(ctx context.Context, searchString string, limit int) ([]authtypes.Profile, error) + DeactivateAccount(ctx context.Context, localpart string) (err error) } // Err3PIDInUse is the error returned when trying to save an association involving diff --git a/userapi/storage/accounts/postgres/accounts_table.go b/userapi/storage/accounts/postgres/accounts_table.go index 7500e1e82..254da84c3 100644 --- a/userapi/storage/accounts/postgres/accounts_table.go +++ b/userapi/storage/accounts/postgres/accounts_table.go @@ -37,7 +37,9 @@ CREATE TABLE IF NOT EXISTS account_accounts ( -- The password hash for this account. Can be NULL if this is a passwordless account. password_hash TEXT, -- Identifies which application service this account belongs to, if any. - appservice_id TEXT + appservice_id TEXT, + -- If the account is currently active + is_deactivated BOOLEAN DEFAULT FALSE -- TODO: -- is_guest, is_admin, upgraded_ts, devices, any email reset stuff? ); @@ -51,11 +53,14 @@ const insertAccountSQL = "" + const updatePasswordSQL = "" + "UPDATE account_accounts SET password_hash = $1 WHERE localpart = $2" +const deactivateAccountSQL = "" + + "UPDATE account_accounts SET is_deactivated = TRUE WHERE localpart = $1" + const selectAccountByLocalpartSQL = "" + "SELECT localpart, appservice_id FROM account_accounts WHERE localpart = $1" const selectPasswordHashSQL = "" + - "SELECT password_hash FROM account_accounts WHERE localpart = $1" + "SELECT password_hash FROM account_accounts WHERE localpart = $1 AND is_deactivated = FALSE" const selectNewNumericLocalpartSQL = "" + "SELECT nextval('numeric_username_seq')" @@ -63,6 +68,7 @@ const selectNewNumericLocalpartSQL = "" + type accountsStatements struct { insertAccountStmt *sql.Stmt updatePasswordStmt *sql.Stmt + deactivateAccountStmt *sql.Stmt selectAccountByLocalpartStmt *sql.Stmt selectPasswordHashStmt *sql.Stmt selectNewNumericLocalpartStmt *sql.Stmt @@ -80,6 +86,9 @@ func (s *accountsStatements) prepare(db *sql.DB, server gomatrixserverlib.Server if s.updatePasswordStmt, err = db.Prepare(updatePasswordSQL); err != nil { return } + if s.deactivateAccountStmt, err = db.Prepare(deactivateAccountSQL); err != nil { + return + } if s.selectAccountByLocalpartStmt, err = db.Prepare(selectAccountByLocalpartSQL); err != nil { return } @@ -127,6 +136,13 @@ func (s *accountsStatements) updatePassword( return } +func (s *accountsStatements) deactivateAccount( + ctx context.Context, localpart string, +) (err error) { + _, err = s.deactivateAccountStmt.ExecContext(ctx, localpart) + return +} + func (s *accountsStatements) selectPasswordHash( ctx context.Context, localpart string, ) (hash string, err error) { diff --git a/userapi/storage/accounts/postgres/deltas/20200929203058_is_active.sql b/userapi/storage/accounts/postgres/deltas/20200929203058_is_active.sql new file mode 100644 index 000000000..32e6e1664 --- /dev/null +++ b/userapi/storage/accounts/postgres/deltas/20200929203058_is_active.sql @@ -0,0 +1,9 @@ +-- +goose Up +-- +goose StatementBegin +ALTER TABLE account_accounts ADD COLUMN IF NOT EXISTS is_deactivated BOOLEAN DEFAULT FALSE; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +ALTER TABLE account_accounts DROP COLUMN is_deactivated; +-- +goose StatementEnd diff --git a/userapi/storage/accounts/postgres/storage.go b/userapi/storage/accounts/postgres/storage.go index 8b9ebef80..2230f7e79 100644 --- a/userapi/storage/accounts/postgres/storage.go +++ b/userapi/storage/accounts/postgres/storage.go @@ -317,3 +317,8 @@ func (d *Database) SearchProfiles(ctx context.Context, searchString string, limi ) ([]authtypes.Profile, error) { return d.profiles.selectProfilesBySearch(ctx, searchString, limit) } + +// DeactivateAccount deactivates the user's account, removing all ability for the user to login again. +func (d *Database) DeactivateAccount(ctx context.Context, localpart string) (err error) { + return d.accounts.deactivateAccount(ctx, localpart) +} diff --git a/userapi/storage/accounts/sqlite3/accounts_table.go b/userapi/storage/accounts/sqlite3/accounts_table.go index 2d935fb63..d0ea8a8bc 100644 --- a/userapi/storage/accounts/sqlite3/accounts_table.go +++ b/userapi/storage/accounts/sqlite3/accounts_table.go @@ -37,7 +37,9 @@ CREATE TABLE IF NOT EXISTS account_accounts ( -- The password hash for this account. Can be NULL if this is a passwordless account. password_hash TEXT, -- Identifies which application service this account belongs to, if any. - appservice_id TEXT + appservice_id TEXT, + -- If the account is currently active + is_deactivated BOOLEAN DEFAULT 0 -- TODO: -- is_guest, is_admin, upgraded_ts, devices, any email reset stuff? ); @@ -49,11 +51,14 @@ const insertAccountSQL = "" + const updatePasswordSQL = "" + "UPDATE account_accounts SET password_hash = $1 WHERE localpart = $2" +const deactivateAccountSQL = "" + + "UPDATE account_accounts SET is_deactivated = 1 WHERE localpart = $1" + const selectAccountByLocalpartSQL = "" + "SELECT localpart, appservice_id FROM account_accounts WHERE localpart = $1" const selectPasswordHashSQL = "" + - "SELECT password_hash FROM account_accounts WHERE localpart = $1" + "SELECT password_hash FROM account_accounts WHERE localpart = $1 AND is_deactivated = 0" const selectNewNumericLocalpartSQL = "" + "SELECT COUNT(localpart) FROM account_accounts" @@ -62,6 +67,7 @@ type accountsStatements struct { db *sql.DB insertAccountStmt *sql.Stmt updatePasswordStmt *sql.Stmt + deactivateAccountStmt *sql.Stmt selectAccountByLocalpartStmt *sql.Stmt selectPasswordHashStmt *sql.Stmt selectNewNumericLocalpartStmt *sql.Stmt @@ -81,6 +87,9 @@ func (s *accountsStatements) prepare(db *sql.DB, server gomatrixserverlib.Server if s.updatePasswordStmt, err = db.Prepare(updatePasswordSQL); err != nil { return } + if s.deactivateAccountStmt, err = db.Prepare(deactivateAccountSQL); err != nil { + return + } if s.selectAccountByLocalpartStmt, err = db.Prepare(selectAccountByLocalpartSQL); err != nil { return } @@ -128,6 +137,13 @@ func (s *accountsStatements) updatePassword( return } +func (s *accountsStatements) deactivateAccount( + ctx context.Context, localpart string, +) (err error) { + _, err = s.deactivateAccountStmt.ExecContext(ctx, localpart) + return +} + func (s *accountsStatements) selectPasswordHash( ctx context.Context, localpart string, ) (hash string, err error) { diff --git a/userapi/storage/accounts/sqlite3/deltas/20200929203058_is_active.sql b/userapi/storage/accounts/sqlite3/deltas/20200929203058_is_active.sql new file mode 100644 index 000000000..51e9bae3c --- /dev/null +++ b/userapi/storage/accounts/sqlite3/deltas/20200929203058_is_active.sql @@ -0,0 +1,38 @@ +-- +goose Up +-- +goose StatementBegin +ALTER TABLE account_accounts RENAME TO account_accounts_tmp; +CREATE TABLE account_accounts ( + localpart TEXT NOT NULL PRIMARY KEY, + created_ts BIGINT NOT NULL, + password_hash TEXT, + appservice_id TEXT, + is_deactivated BOOLEAN DEFAULT 0 +); +INSERT + INTO account_accounts ( + localpart, created_ts, password_hash, appservice_id + ) SELECT + localpart, created_ts, password_hash, appservice_id + FROM account_accounts_tmp +; +DROP TABLE account_accounts_tmp; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +ALTER TABLE account_accounts RENAME TO account_accounts_tmp; +CREATE TABLE account_accounts ( + localpart TEXT NOT NULL PRIMARY KEY, + created_ts BIGINT NOT NULL, + password_hash TEXT, + appservice_id TEXT +); +INSERT + INTO account_accounts ( + localpart, created_ts, password_hash, appservice_id + ) SELECT + localpart, created_ts, password_hash, appservice_id + FROM account_accounts_tmp +; +DROP TABLE account_accounts_tmp; +-- +goose StatementEnd diff --git a/userapi/storage/accounts/sqlite3/storage.go b/userapi/storage/accounts/sqlite3/storage.go index 4b66304c2..7a2830a93 100644 --- a/userapi/storage/accounts/sqlite3/storage.go +++ b/userapi/storage/accounts/sqlite3/storage.go @@ -359,3 +359,8 @@ func (d *Database) SearchProfiles(ctx context.Context, searchString string, limi ) ([]authtypes.Profile, error) { return d.profiles.selectProfilesBySearch(ctx, searchString, limit) } + +// DeactivateAccount deactivates the user's account, removing all ability for the user to login again. +func (d *Database) DeactivateAccount(ctx context.Context, localpart string) (err error) { + return d.accounts.deactivateAccount(ctx, localpart) +}