From e82a2e518cdd018523dc5e08f328291d77854c21 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Mon, 23 Jan 2023 10:58:39 -0500 Subject: [PATCH] Add password confirmation to account delete dialog, v1/tiers test --- server/errors.go | 2 +- server/server.go | 4 +- server/server_account.go | 46 +++++++++++-- server/server_account_test.go | 12 +++- server/server_payments.go | 50 +++++++------- server/server_payments_test.go | 113 +++++++++++++++++++++++++++++++- server/types.go | 4 ++ server/util.go | 8 +-- user/manager.go | 2 - user/manager_test.go | 1 - user/types.go | 1 - web/public/static/langs/en.json | 6 +- web/src/app/AccountApi.js | 17 +++-- web/src/components/Account.js | 69 +++++++++---------- 14 files changed, 242 insertions(+), 93 deletions(-) diff --git a/server/errors.go b/server/errors.go index be63a54..359cedc 100644 --- a/server/errors.go +++ b/server/errors.go @@ -62,7 +62,7 @@ var ( errHTTPBadRequestNotAPaidUser = &errHTTP{40027, http.StatusBadRequest, "invalid request: not a paid user", ""} errHTTPBadRequestBillingRequestInvalid = &errHTTP{40028, http.StatusBadRequest, "invalid request: not a valid billing request", ""} errHTTPBadRequestBillingSubscriptionExists = &errHTTP{40029, http.StatusBadRequest, "invalid request: billing subscription already exists", ""} - errHTTPBadRequestCurrentPasswordWrong = &errHTTP{40030, http.StatusBadRequest, "invalid request: current password is not correct", ""} + errHTTPBadRequestIncorrectPasswordConfirmation = &errHTTP{40030, http.StatusBadRequest, "invalid request: password confirmation is not correct", ""} errHTTPNotFound = &errHTTP{40401, http.StatusNotFound, "page not found", ""} errHTTPUnauthorized = &errHTTP{40101, http.StatusUnauthorized, "unauthorized", "https://ntfy.sh/docs/publish/#authentication"} errHTTPForbidden = &errHTTP{40301, http.StatusForbidden, "forbidden", "https://ntfy.sh/docs/publish/#authentication"} diff --git a/server/server.go b/server/server.go index 05524fd..c6b94a2 100644 --- a/server/server.go +++ b/server/server.go @@ -40,9 +40,9 @@ TODO - Reservation: Kill existing subscribers when topic is reserved (deadcade) - Rate limiting: Sensitive endpoints (account/login/change-password/...) -- Stripe: Add metadata to customer - Reservation (UI): Show "This topic is reserved" error message when trying to reserve a reserved topic (Thorben) - Reservation (UI): Ask for confirmation when removing reservation (deadcade) +- Reservation icons (UI) races: - v.user --> see publishSyncEventAsync() test @@ -65,7 +65,6 @@ Make sure account endpoints make sense for admins UI: - -- reservation icons - reservation table delete button: dialog "keep or delete messages?" - flicker of upgrade banner - JS constants @@ -858,7 +857,6 @@ func (s *Server) handleBodyAsAttachment(r *http.Request, v *visitor, m *message, if m.Time > attachmentExpiry { return errHTTPBadRequestAttachmentsExpiryBeforeDelivery } - fmt.Printf("v = %#v\nlimits = %#v\nstats = %#v\n", v, vinfo.Limits, vinfo.Stats) contentLengthStr := r.Header.Get("Content-Length") if contentLengthStr != "" { // Early "do-not-trust" check, hard limit see below contentLength, err := strconv.ParseInt(contentLengthStr, 10, 64) diff --git a/server/server_account.go b/server/server_account.go index 72a7826..6fce9f1 100644 --- a/server/server_account.go +++ b/server/server_account.go @@ -7,6 +7,7 @@ import ( "heckel.io/ntfy/user" "heckel.io/ntfy/util" "net/http" + "strings" ) const ( @@ -118,17 +119,24 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, _ *http.Request, v *vis } func (s *Server) handleAccountDelete(w http.ResponseWriter, r *http.Request, v *visitor) error { + req, err := readJSONWithLimit[apiAccountDeleteRequest](r.Body, jsonBodyBytesLimit) + if err != nil { + return err + } else if req.Password == "" { + return errHTTPBadRequest + } + if _, err := s.userManager.Authenticate(v.user.Name, req.Password); err != nil { + return errHTTPBadRequestIncorrectPasswordConfirmation + } if v.user.Billing.StripeSubscriptionID != "" { log.Info("%s Canceling billing subscription %s", logHTTPPrefix(v, r), v.user.Billing.StripeSubscriptionID) - if v.user.Billing.StripeSubscriptionID != "" { - if _, err := s.stripe.CancelSubscription(v.user.Billing.StripeSubscriptionID); err != nil { - return err - } - } - if err := s.maybeRemoveExcessReservations(logHTTPPrefix(v, r), v.user, 0); err != nil { + if _, err := s.stripe.CancelSubscription(v.user.Billing.StripeSubscriptionID); err != nil { return err } } + if err := s.maybeRemoveMessagesAndExcessReservations(logHTTPPrefix(v, r), v.user, 0); err != nil { + return err + } log.Info("%s Marking user %s as deleted", logHTTPPrefix(v, r), v.user.Name) if err := s.userManager.MarkUserRemoved(v.user); err != nil { return err @@ -144,7 +152,7 @@ func (s *Server) handleAccountPasswordChange(w http.ResponseWriter, r *http.Requ return errHTTPBadRequest } if _, err := s.userManager.Authenticate(v.user.Name, req.Password); err != nil { - return errHTTPBadRequestCurrentPasswordWrong + return errHTTPBadRequestIncorrectPasswordConfirmation } if err := s.userManager.ChangePassword(v.user.Name, req.NewPassword); err != nil { return err @@ -365,6 +373,30 @@ func (s *Server) handleAccountReservationDelete(w http.ResponseWriter, r *http.R return s.writeJSON(w, newSuccessResponse()) } +// maybeRemoveMessagesAndExcessReservations deletes topic reservations for the given user (if too many for tier), +// and marks associated messages for the topics as deleted. This also eventually deletes attachments. +// The process relies on the manager to perform the actual deletions (see runManager). +func (s *Server) maybeRemoveMessagesAndExcessReservations(logPrefix string, u *user.User, reservationsLimit int64) error { + reservations, err := s.userManager.Reservations(u.Name) + if err != nil { + return err + } else if int64(len(reservations)) <= reservationsLimit { + return nil + } + topics := make([]string, 0) + for i := int64(len(reservations)) - 1; i >= reservationsLimit; i-- { + topics = append(topics, reservations[i].Topic) + } + log.Info("%s Removing excess reservations for topics %s", logPrefix, strings.Join(topics, ", ")) + if err := s.userManager.RemoveReservations(u.Name, topics...); err != nil { + return err + } + if err := s.messageCache.ExpireMessages(topics...); err != nil { + return err + } + return nil +} + func (s *Server) publishSyncEvent(v *visitor) error { if v.user == nil || v.user.SyncTopic == "" { return nil diff --git a/server/server_account_test.go b/server/server_account_test.go index dba1ad9..690dadd 100644 --- a/server/server_account_test.go +++ b/server/server_account_test.go @@ -319,7 +319,7 @@ func TestAccount_Delete_Success(t *testing.T) { }) require.Equal(t, 200, rr.Code) - rr = request(t, s, "DELETE", "/v1/account", "", map[string]string{ + rr = request(t, s, "DELETE", "/v1/account", `{"password":"mypass"}`, map[string]string{ "Authorization": util.BasicAuth("phil", "mypass"), }) require.Equal(t, 200, rr.Code) @@ -345,6 +345,15 @@ func TestAccount_Delete_Not_Allowed(t *testing.T) { rr = request(t, s, "DELETE", "/v1/account", "", nil) require.Equal(t, 401, rr.Code) + + rr = request(t, s, "DELETE", "/v1/account", `{"password":"mypass"}`, nil) + require.Equal(t, 401, rr.Code) + + rr = request(t, s, "DELETE", "/v1/account", `{"password":"INCORRECT"}`, map[string]string{ + "Authorization": util.BasicAuth("phil", "mypass"), + }) + require.Equal(t, 400, rr.Code) + require.Equal(t, 40030, toHTTPError(t, rr.Body.String()).Code) } func TestAccount_Reservation_AddWithoutTierFails(t *testing.T) { @@ -386,7 +395,6 @@ func TestAccount_Reservation_AddRemoveUserWithTierSuccess(t *testing.T) { // Create a tier require.Nil(t, s.userManager.CreateTier(&user.Tier{ Code: "pro", - Paid: false, MessagesLimit: 123, MessagesExpiryDuration: 86400 * time.Second, EmailsLimit: 32, diff --git a/server/server_payments.go b/server/server_payments.go index 3c29dae..2178d48 100644 --- a/server/server_payments.go +++ b/server/server_payments.go @@ -18,7 +18,6 @@ import ( "io" "net/http" "net/netip" - "strings" "time" ) @@ -66,6 +65,7 @@ func (s *Server) handleBillingTiersGet(w http.ResponseWriter, _ *http.Request, _ { // This is a bit of a hack: This is the "Free" tier. It has no tier code, name or price. Limits: &apiAccountLimits{ + Basis: string(visitorLimitBasisIP), Messages: freeTier.MessagesLimit, MessagesExpiryDuration: int64(freeTier.MessagesExpiryDuration.Seconds()), Emails: freeTier.EmailsLimit, @@ -90,6 +90,7 @@ func (s *Server) handleBillingTiersGet(w http.ResponseWriter, _ *http.Request, _ Name: tier.Name, Price: priceStr, Limits: &apiAccountLimits{ + Basis: string(visitorLimitBasisTier), Messages: tier.MessagesLimit, MessagesExpiryDuration: int64(tier.MessagesExpiryDuration.Seconds()), Emails: tier.EmailsLimit, @@ -143,6 +144,11 @@ func (s *Server) handleAccountBillingSubscriptionCreate(w http.ResponseWriter, r Quantity: stripe.Int64(1), }, }, + Params: stripe.Params{ + Metadata: map[string]string{ + "user_id": v.user.ID, + }, + }, } sess, err := s.stripe.NewCheckoutSession(params) if err != nil { @@ -185,6 +191,17 @@ func (s *Server) handleAccountBillingSubscriptionCreateSuccess(w http.ResponseWr return err } v.SetUser(u) + customerParams := &stripe.CustomerParams{ + Params: stripe.Params{ + Metadata: map[string]string{ + "user_id": u.ID, + "user_name": u.Name, + }, + }, + } + if _, err := s.stripe.UpdateCustomer(sess.Customer.ID, customerParams); err != nil { + return err + } if err := s.updateSubscriptionAndTier(logHTTPPrefix(v, r), u, tier, sess.Customer.ID, sub.ID, string(sub.Status), sub.CurrentPeriodEnd, sub.CancelAt); err != nil { return err } @@ -342,36 +359,12 @@ func (s *Server) handleAccountBillingWebhookSubscriptionDeleted(event json.RawMe return nil } -// maybeRemoveExcessReservations deletes topic reservations for the given user (if too many for tier), -// and marks associated messages for the topics as deleted. This also eventually deletes attachments. -// The process relies on the manager to perform the actual deletions (see runManager). -func (s *Server) maybeRemoveExcessReservations(logPrefix string, u *user.User, reservationsLimit int64) error { - reservations, err := s.userManager.Reservations(u.Name) - if err != nil { - return err - } else if int64(len(reservations)) <= reservationsLimit { - return nil - } - topics := make([]string, 0) - for i := int64(len(reservations)) - 1; i >= reservationsLimit; i-- { - topics = append(topics, reservations[i].Topic) - } - log.Info("%s Removing excess reservations for topics %s", logPrefix, strings.Join(topics, ", ")) - if err := s.userManager.RemoveReservations(u.Name, topics...); err != nil { - return err - } - if err := s.messageCache.ExpireMessages(topics...); err != nil { - return err - } - return nil -} - func (s *Server) updateSubscriptionAndTier(logPrefix string, u *user.User, tier *user.Tier, customerID, subscriptionID, status string, paidUntil, cancelAt int64) error { reservationsLimit := visitorDefaultReservationsLimit if tier != nil { reservationsLimit = tier.ReservationsLimit } - if err := s.maybeRemoveExcessReservations(logPrefix, u, reservationsLimit); err != nil { + if err := s.maybeRemoveMessagesAndExcessReservations(logPrefix, u, reservationsLimit); err != nil { return err } if tier == nil { @@ -426,6 +419,7 @@ type stripeAPI interface { GetCustomer(id string) (*stripe.Customer, error) GetSession(id string) (*stripe.CheckoutSession, error) GetSubscription(id string) (*stripe.Subscription, error) + UpdateCustomer(id string, params *stripe.CustomerParams) (*stripe.Customer, error) UpdateSubscription(id string, params *stripe.SubscriptionParams) (*stripe.Subscription, error) CancelSubscription(id string) (*stripe.Subscription, error) ConstructWebhookEvent(payload []byte, header string, secret string) (stripe.Event, error) @@ -472,6 +466,10 @@ func (s *realStripeAPI) GetSubscription(id string) (*stripe.Subscription, error) return subscription.Get(id, nil) } +func (s *realStripeAPI) UpdateCustomer(id string, params *stripe.CustomerParams) (*stripe.Customer, error) { + return customer.Update(id, params) +} + func (s *realStripeAPI) UpdateSubscription(id string, params *stripe.SubscriptionParams) (*stripe.Subscription, error) { return subscription.Update(id, params) } diff --git a/server/server_payments_test.go b/server/server_payments_test.go index d1008ef..82d4ca8 100644 --- a/server/server_payments_test.go +++ b/server/server_payments_test.go @@ -14,6 +14,108 @@ import ( "time" ) +func TestPayments_Tiers(t *testing.T) { + stripeMock := &testStripeAPI{} + defer stripeMock.AssertExpectations(t) + + c := newTestConfigWithAuthFile(t) + c.StripeSecretKey = "secret key" + c.StripeWebhookKey = "webhook key" + c.VisitorRequestLimitReplenish = 12 * time.Hour + c.CacheDuration = 13 * time.Hour + c.AttachmentFileSizeLimit = 111 + c.VisitorAttachmentTotalSizeLimit = 222 + c.AttachmentExpiryDuration = 123 * time.Second + s := newTestServer(t, c) + s.stripe = stripeMock + + // Define how the mock should react + stripeMock. + On("ListPrices", mock.Anything). + Return([]*stripe.Price{ + {ID: "price_123", UnitAmount: 500}, + {ID: "price_456", UnitAmount: 1000}, + {ID: "price_999", UnitAmount: 9999}, + }, nil) + + // Create tiers + require.Nil(t, s.userManager.CreateTier(&user.Tier{ + ID: "ti_1", + Code: "admin", + Name: "Admin", + })) + require.Nil(t, s.userManager.CreateTier(&user.Tier{ + ID: "ti_123", + Code: "pro", + Name: "Pro", + MessagesLimit: 1000, + MessagesExpiryDuration: time.Hour, + EmailsLimit: 123, + ReservationsLimit: 777, + AttachmentFileSizeLimit: 999, + AttachmentTotalSizeLimit: 888, + AttachmentExpiryDuration: time.Minute, + StripePriceID: "price_123", + })) + require.Nil(t, s.userManager.CreateTier(&user.Tier{ + ID: "ti_444", + Code: "business", + Name: "Business", + MessagesLimit: 2000, + MessagesExpiryDuration: 10 * time.Hour, + EmailsLimit: 123123, + ReservationsLimit: 777333, + AttachmentFileSizeLimit: 999111, + AttachmentTotalSizeLimit: 888111, + AttachmentExpiryDuration: time.Hour, + StripePriceID: "price_456", + })) + response := request(t, s, "GET", "/v1/tiers", "", nil) + require.Equal(t, 200, response.Code) + var tiers []apiAccountBillingTier + require.Nil(t, json.NewDecoder(response.Body).Decode(&tiers)) + require.Equal(t, 3, len(tiers)) + + // Free tier + tier := tiers[0] + require.Equal(t, "", tier.Code) + require.Equal(t, "", tier.Name) + require.Equal(t, "ip", tier.Limits.Basis) + require.Equal(t, int64(0), tier.Limits.Reservations) + require.Equal(t, int64(2), tier.Limits.Messages) // :-( + require.Equal(t, int64(13*3600), tier.Limits.MessagesExpiryDuration) + require.Equal(t, int64(24), tier.Limits.Emails) + require.Equal(t, int64(111), tier.Limits.AttachmentFileSize) + require.Equal(t, int64(222), tier.Limits.AttachmentTotalSize) + require.Equal(t, int64(123), tier.Limits.AttachmentExpiryDuration) + + // Admin tier is not included, because it is not paid! + + tier = tiers[1] + require.Equal(t, "pro", tier.Code) + require.Equal(t, "Pro", tier.Name) + require.Equal(t, "tier", tier.Limits.Basis) + require.Equal(t, int64(777), tier.Limits.Reservations) + require.Equal(t, int64(1000), tier.Limits.Messages) + require.Equal(t, int64(3600), tier.Limits.MessagesExpiryDuration) + require.Equal(t, int64(123), tier.Limits.Emails) + require.Equal(t, int64(999), tier.Limits.AttachmentFileSize) + require.Equal(t, int64(888), tier.Limits.AttachmentTotalSize) + require.Equal(t, int64(60), tier.Limits.AttachmentExpiryDuration) + + tier = tiers[2] + require.Equal(t, "business", tier.Code) + require.Equal(t, "Business", tier.Name) + require.Equal(t, "tier", tier.Limits.Basis) + require.Equal(t, int64(777333), tier.Limits.Reservations) + require.Equal(t, int64(2000), tier.Limits.Messages) + require.Equal(t, int64(36000), tier.Limits.MessagesExpiryDuration) + require.Equal(t, int64(123123), tier.Limits.Emails) + require.Equal(t, int64(999111), tier.Limits.AttachmentFileSize) + require.Equal(t, int64(888111), tier.Limits.AttachmentTotalSize) + require.Equal(t, int64(3600), tier.Limits.AttachmentExpiryDuration) +} + func TestPayments_SubscriptionCreate_NotAStripeCustomer_Success(t *testing.T) { stripeMock := &testStripeAPI{} defer stripeMock.AssertExpectations(t) @@ -122,7 +224,7 @@ func TestPayments_AccountDelete_Cancels_Subscription(t *testing.T) { require.Nil(t, s.userManager.ChangeBilling(u.Name, billing)) // Delete account - rr := request(t, s, "DELETE", "/v1/account", "", map[string]string{ + rr := request(t, s, "DELETE", "/v1/account", `{"password": "phil"}`, map[string]string{ "Authorization": util.BasicAuth("phil", "phil"), }) require.Equal(t, 200, rr.Code) @@ -258,6 +360,8 @@ type testStripeAPI struct { mock.Mock } +var _ stripeAPI = (*testStripeAPI)(nil) + func (s *testStripeAPI) NewCheckoutSession(params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) { args := s.Called(params) return args.Get(0).(*stripe.CheckoutSession), args.Error(1) @@ -288,6 +392,11 @@ func (s *testStripeAPI) GetSubscription(id string) (*stripe.Subscription, error) return args.Get(0).(*stripe.Subscription), args.Error(1) } +func (s *testStripeAPI) UpdateCustomer(id string, params *stripe.CustomerParams) (*stripe.Customer, error) { + args := s.Called(id) + return args.Get(0).(*stripe.Customer), args.Error(1) +} + func (s *testStripeAPI) UpdateSubscription(id string, params *stripe.SubscriptionParams) (*stripe.Subscription, error) { args := s.Called(id) return args.Get(0).(*stripe.Subscription), args.Error(1) @@ -303,8 +412,6 @@ func (s *testStripeAPI) ConstructWebhookEvent(payload []byte, header string, sec return args.Get(0).(stripe.Event), args.Error(1) } -var _ stripeAPI = (*testStripeAPI)(nil) - func jsonToStripeEvent(t *testing.T, v string) stripe.Event { var e stripe.Event if err := json.Unmarshal([]byte(v), &e); err != nil { diff --git a/server/types.go b/server/types.go index 38364e9..50e41d6 100644 --- a/server/types.go +++ b/server/types.go @@ -231,6 +231,10 @@ type apiAccountPasswordChangeRequest struct { NewPassword string `json:"new_password"` } +type apiAccountDeleteRequest struct { + Password string `json:"password"` +} + type apiAccountTokenResponse struct { Token string `json:"token"` Expires int64 `json:"expires"` diff --git a/server/util.go b/server/util.go index a9c4df5..2a7bfe8 100644 --- a/server/util.go +++ b/server/util.go @@ -57,18 +57,18 @@ func logHTTPPrefix(v *visitor, r *http.Request) string { if requestURI == "" { requestURI = r.URL.Path } - return fmt.Sprintf("%s HTTP %s %s", v.String(), r.Method, requestURI) + return fmt.Sprintf("HTTP %s %s %s", v.String(), r.Method, requestURI) } func logStripePrefix(customerID, subscriptionID string) string { if subscriptionID != "" { - return fmt.Sprintf("%s/%s STRIPE", customerID, subscriptionID) + return fmt.Sprintf("STRIPE %s/%s", customerID, subscriptionID) } - return fmt.Sprintf("%s STRIPE", customerID) + return fmt.Sprintf("STRIPE %s", customerID) } func logSMTPPrefix(state *smtp.ConnectionState) string { - return fmt.Sprintf("%s/%s SMTP", state.Hostname, state.RemoteAddr.String()) + return fmt.Sprintf("SMTP %s/%s", state.Hostname, state.RemoteAddr.String()) } func renderHTTPRequest(r *http.Request) string { diff --git a/user/manager.go b/user/manager.go index a46150e..e3c49c8 100644 --- a/user/manager.go +++ b/user/manager.go @@ -707,7 +707,6 @@ func (a *Manager) readUser(rows *sql.Rows) (*User, error) { user.Tier = &Tier{ Code: tierCode.String, Name: tierName.String, - Paid: stripePriceID.Valid, // If there is a price, it's a paid tier MessagesLimit: messagesLimit.Int64, MessagesExpiryDuration: time.Duration(messagesExpiryDuration.Int64) * time.Second, EmailsLimit: emailsLimit.Int64, @@ -1066,7 +1065,6 @@ func (a *Manager) readTier(rows *sql.Rows) (*Tier, error) { ID: id, Code: code, Name: name, - Paid: stripePriceID.Valid, // If there is a price, it's a paid tier MessagesLimit: messagesLimit.Int64, MessagesExpiryDuration: time.Duration(messagesExpiryDuration.Int64) * time.Second, EmailsLimit: emailsLimit.Int64, diff --git a/user/manager_test.go b/user/manager_test.go index 0fedc1c..9fa1819 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -350,7 +350,6 @@ func TestManager_ChangeRoleFromTierUserToAdmin(t *testing.T) { require.Nil(t, err) require.Equal(t, RoleUser, ben.Role) require.Equal(t, "pro", ben.Tier.Code) - require.Equal(t, true, ben.Tier.Paid) require.Equal(t, int64(5000), ben.Tier.MessagesLimit) require.Equal(t, 3*24*time.Hour, ben.Tier.MessagesExpiryDuration) require.Equal(t, int64(50), ben.Tier.EmailsLimit) diff --git a/user/types.go b/user/types.go index 8783479..14147c1 100644 --- a/user/types.go +++ b/user/types.go @@ -53,7 +53,6 @@ type Tier struct { ID string Code string Name string - Paid bool MessagesLimit int64 MessagesExpiryDuration time.Duration EmailsLimit int64 diff --git a/web/public/static/langs/en.json b/web/public/static/langs/en.json index 33442e4..cd8832d 100644 --- a/web/public/static/langs/en.json +++ b/web/public/static/langs/en.json @@ -175,7 +175,7 @@ "account_basics_password_dialog_confirm_password_label": "Confirm password", "account_basics_password_dialog_button_cancel": "Cancel", "account_basics_password_dialog_button_submit": "Change password", - "account_basics_password_dialog_current_password_incorrect": "Current password incorrect", + "account_basics_password_dialog_current_password_incorrect": "Password incorrect", "account_usage_title": "Usage", "account_usage_of_limit": "of {{limit}}", "account_usage_unlimited": "Unlimited", @@ -199,8 +199,8 @@ "account_usage_basis_ip_description": "Usage stats and limits for this account are based on your IP address, so they may be shared with other users. Limits shown above are approximates based on the existing rate limits.", "account_delete_title": "Delete account", "account_delete_description": "Permanently delete your account", - "account_delete_dialog_description": "This will permanently delete your account, including all data that is stored on the server. If you really want to proceed, please type '{{username}}' in the text box below.", - "account_delete_dialog_label": "Type '{{username}}' to delete account", + "account_delete_dialog_description": "This will permanently delete your account, including all data that is stored on the server. If you really want to proceed, please confirm with your password in the box below.", + "account_delete_dialog_label": "Password", "account_delete_dialog_button_cancel": "Cancel", "account_delete_dialog_button_submit": "Permanently delete account", "account_delete_dialog_billing_warning": "Deleting your account also cancels your billing subscription immediately. You will not have access to the billing dashboard anymore.", diff --git a/web/src/app/AccountApi.js b/web/src/app/AccountApi.js index 86670f0..581e4a3 100644 --- a/web/src/app/AccountApi.js +++ b/web/src/app/AccountApi.js @@ -106,14 +106,19 @@ class AccountApi { return account; } - async delete() { + async delete(password) { const url = accountUrl(config.base_url); console.log(`[AccountApi] Deleting user account ${url}`); const response = await fetch(url, { method: "DELETE", - headers: withBearerAuth({}, session.token()) + headers: withBearerAuth({}, session.token()), + body: JSON.stringify({ + password: password + }) }); - if (response.status === 401 || response.status === 403) { + if (response.status === 400) { + throw new IncorrectPasswordError(); + } else if (response.status === 401 || response.status === 403) { throw new UnauthorizedError(); } else if (response.status !== 200) { throw new Error(`Unexpected server response ${response.status}`); @@ -132,7 +137,7 @@ class AccountApi { }) }); if (response.status === 400) { - throw new CurrentPasswordWrongError(); + throw new IncorrectPasswordError(); } else if (response.status === 401 || response.status === 403) { throw new UnauthorizedError(); } else if (response.status !== 200) { @@ -397,9 +402,9 @@ export class AccountCreateLimitReachedError extends Error { } } -export class CurrentPasswordWrongError extends Error { +export class IncorrectPasswordError extends Error { constructor() { - super("Current password incorrect"); + super("Password incorrect"); } } diff --git a/web/src/components/Account.js b/web/src/components/Account.js index 67c2d4f..4d4aebf 100644 --- a/web/src/components/Account.js +++ b/web/src/components/Account.js @@ -19,7 +19,7 @@ import DialogActions from "@mui/material/DialogActions"; import routes from "./routes"; import IconButton from "@mui/material/IconButton"; import {formatBytes, formatShortDate, formatShortDateTime} from "../app/utils"; -import accountApi, {CurrentPasswordWrongError, UnauthorizedError} from "../app/AccountApi"; +import accountApi, {IncorrectPasswordError, UnauthorizedError} from "../app/AccountApi"; import InfoOutlinedIcon from '@mui/icons-material/InfoOutlined'; import {Pref, PrefGroup} from "./Pref"; import db from "../app/db"; @@ -128,7 +128,7 @@ const ChangePasswordDialog = (props) => { props.onClose(); } catch (e) { console.log(`[Account] Error changing password`, e); - if ((e instanceof CurrentPasswordWrongError)) { + if ((e instanceof IncorrectPasswordError)) { setErrorText(t("account_basics_password_dialog_current_password_incorrect")); } else if ((e instanceof UnauthorizedError)) { session.resetAndRedirect(routes.login); @@ -414,26 +414,10 @@ const DeleteAccount = () => { setDialogOpen(true); }; - const handleDialogCancel = () => { + const handleDialogClose = () => { setDialogOpen(false); }; - const handleDialogSubmit = async () => { - try { - await accountApi.delete(); - await db.delete(); - setDialogOpen(false); - console.debug(`[Account] Account deleted`); - session.resetAndRedirect(routes.app); - } catch (e) { - console.log(`[Account] Error deleting account`, e); - if ((e instanceof UnauthorizedError)) { - session.resetAndRedirect(routes.login); - } - // TODO show error - } - }; - return (
@@ -444,8 +428,7 @@ const DeleteAccount = () => { ) @@ -454,24 +437,42 @@ const DeleteAccount = () => { const DeleteAccountDialog = (props) => { const { t } = useTranslation(); const { account } = useContext(AccountContext); - const [username, setUsername] = useState(""); + const [password, setPassword] = useState(""); + const [errorText, setErrorText] = useState(""); const fullScreen = useMediaQuery(theme.breakpoints.down('sm')); - const buttonEnabled = username === session.username(); + + const handleSubmit = async () => { + try { + await accountApi.delete(password); + await db.delete(); + console.debug(`[Account] Account deleted`); + session.resetAndRedirect(routes.app); + } catch (e) { + console.log(`[Account] Error deleting account`, e); + if ((e instanceof IncorrectPasswordError)) { + setErrorText(t("account_basics_password_dialog_current_password_incorrect")); + } else if ((e instanceof UnauthorizedError)) { + session.resetAndRedirect(routes.login); + } + // TODO show error + } + }; + return ( - + {t("account_delete_title")} - {t("account_delete_dialog_description", { username: session.username()})} + {t("account_delete_dialog_description")} setUsername(ev.target.value)} + label={t("account_delete_dialog_label")} + aria-label={t("account_delete_dialog_label")} + type="password" + value={password} + onChange={ev => setPassword(ev.target.value)} fullWidth variant="standard" /> @@ -479,10 +480,10 @@ const DeleteAccountDialog = (props) => { {t("account_delete_dialog_billing_warning")} } - - - - + + + + ); };