diff --git a/docs/releases.md b/docs/releases.md
index f5fa37b4..dc4872ea 100644
--- a/docs/releases.md
+++ b/docs/releases.md
@@ -24,6 +24,11 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
* Portuguese (thanks to [@ssantos](https://hosted.weblate.org/user/ssantos/))
+**Special thanks:**
+
+A big Thank-you goes to everyone who tested the user account and payments work. I very much appreciate all the feedback,
+suggestions, and bug reports. Thank you, @nwithan8, @deadcade, and @xenrox.
+
## ntfy server v1.30.1
Released December 23, 2022 🎅
diff --git a/server/errors.go b/server/errors.go
index 20bf2e84..be63a54e 100644
--- a/server/errors.go
+++ b/server/errors.go
@@ -33,6 +33,7 @@ func wrapErrHTTP(err *errHTTP, message string, args ...any) *errHTTP {
}
var (
+ errHTTPBadRequest = &errHTTP{40000, http.StatusBadRequest, "invalid request", ""}
errHTTPBadRequestEmailDisabled = &errHTTP{40001, http.StatusBadRequest, "e-mail notifications are not enabled", "https://ntfy.sh/docs/config/#e-mail-notifications"}
errHTTPBadRequestDelayNoCache = &errHTTP{40002, http.StatusBadRequest, "cannot disable cache for delayed message", ""}
errHTTPBadRequestDelayNoEmail = &errHTTP{40003, http.StatusBadRequest, "delayed e-mail notifications are not supported", ""}
@@ -61,6 +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", ""}
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 42216531..12cff2b1 100644
--- a/server/server.go
+++ b/server/server.go
@@ -38,13 +38,12 @@ import (
TODO
--
-UAT results (round 1):
- Security: Account re-creation leads to terrible behavior. Use user ID instead of user name for (a) visitor map, (b) messages.user column, (c) Stripe checkout session
-- Account: Changing password should confirm the old password (Thorben)
- Reservation: Kill existing subscribers when topic is reserved (deadcade)
- 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)
- Logging: Add detailed logging with username/customerID for all Stripe events (phil)
+- Rate limiting: Sensitive endpoints (account/login/change-password/...)
races:
- v.user --> see publishSyncEventAsync() test
@@ -59,7 +58,6 @@ Limits & rate limiting:
rate limiting weirdness. wth is going on?
bandwidth limit must be in tier
users without tier: should the stats be persisted? are they meaningful? -> test that the visitor is based on the IP address!
- login/account endpoints
when ResetStats() is run, reset messagesLimiter (and others)?
Delete visitor when tier is changed to refresh rate limiters
diff --git a/server/server_account.go b/server/server_account.go
index 6bcb3233..755dcf75 100644
--- a/server/server_account.go
+++ b/server/server_account.go
@@ -136,11 +136,16 @@ func (s *Server) handleAccountDelete(w http.ResponseWriter, _ *http.Request, v *
}
func (s *Server) handleAccountPasswordChange(w http.ResponseWriter, r *http.Request, v *visitor) error {
- newPassword, err := readJSONWithLimit[apiAccountPasswordChangeRequest](r.Body, jsonBodyBytesLimit)
+ req, err := readJSONWithLimit[apiAccountPasswordChangeRequest](r.Body, jsonBodyBytesLimit)
if err != nil {
return err
+ } else if req.Password == "" || req.NewPassword == "" {
+ return errHTTPBadRequest
}
- if err := s.userManager.ChangePassword(v.user.Name, newPassword.Password); err != nil {
+ if _, err := s.userManager.Authenticate(v.user.Name, req.Password); err != nil {
+ return errHTTPBadRequestCurrentPasswordWrong
+ }
+ if err := s.userManager.ChangePassword(v.user.Name, req.NewPassword); err != nil {
return err
}
return s.writeJSON(w, newSuccessResponse())
diff --git a/server/types.go b/server/types.go
index 5d0f04b5..3d651d30 100644
--- a/server/types.go
+++ b/server/types.go
@@ -227,7 +227,8 @@ type apiAccountCreateRequest struct {
}
type apiAccountPasswordChangeRequest struct {
- Password string `json:"password"`
+ Password string `json:"password"`
+ NewPassword string `json:"new_password"`
}
type apiAccountTokenResponse struct {
diff --git a/web/public/static/langs/en.json b/web/public/static/langs/en.json
index cac86d87..33442e44 100644
--- a/web/public/static/langs/en.json
+++ b/web/public/static/langs/en.json
@@ -170,10 +170,12 @@
"account_basics_password_title": "Password",
"account_basics_password_description": "Change your account password",
"account_basics_password_dialog_title": "Change password",
+ "account_basics_password_dialog_current_password_label": "Current password",
"account_basics_password_dialog_new_password_label": "New password",
"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_usage_title": "Usage",
"account_usage_of_limit": "of {{limit}}",
"account_usage_unlimited": "Unlimited",
diff --git a/web/src/app/AccountApi.js b/web/src/app/AccountApi.js
index 05b3d6b6..86670f02 100644
--- a/web/src/app/AccountApi.js
+++ b/web/src/app/AccountApi.js
@@ -120,17 +120,20 @@ class AccountApi {
}
}
- async changePassword(newPassword) {
+ async changePassword(currentPassword, newPassword) {
const url = accountPasswordUrl(config.base_url);
console.log(`[AccountApi] Changing account password ${url}`);
const response = await fetch(url, {
method: "POST",
headers: withBearerAuth({}, session.token()),
body: JSON.stringify({
- password: newPassword
+ password: currentPassword,
+ new_password: newPassword
})
});
- if (response.status === 401 || response.status === 403) {
+ if (response.status === 400) {
+ throw new CurrentPasswordWrongError();
+ } else if (response.status === 401 || response.status === 403) {
throw new UnauthorizedError();
} else if (response.status !== 200) {
throw new Error(`Unexpected server response ${response.status}`);
@@ -394,6 +397,12 @@ export class AccountCreateLimitReachedError extends Error {
}
}
+export class CurrentPasswordWrongError extends Error {
+ constructor() {
+ super("Current password incorrect");
+ }
+}
+
export class UnauthorizedError extends Error {
constructor() {
super("Unauthorized");
diff --git a/web/src/components/Account.js b/web/src/components/Account.js
index 452868b7..67c2d4f0 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, {UnauthorizedError} from "../app/AccountApi";
+import accountApi, {CurrentPasswordWrongError, UnauthorizedError} from "../app/AccountApi";
import InfoOutlinedIcon from '@mui/icons-material/InfoOutlined';
import {Pref, PrefGroup} from "./Pref";
import db from "../app/db";
@@ -29,6 +29,7 @@ import UpgradeDialog from "./UpgradeDialog";
import CelebrationIcon from "@mui/icons-material/Celebration";
import {AccountContext} from "./App";
import {Warning, WarningAmber} from "@mui/icons-material";
+import DialogFooter from "./DialogFooter";
const Account = () => {
if (!session.exists()) {
@@ -90,24 +91,10 @@ const ChangePassword = () => {
setDialogOpen(true);
};
- const handleDialogCancel = () => {
+ const handleDialogClose = () => {
setDialogOpen(false);
};
- const handleDialogSubmit = async (newPassword) => {
- try {
- await accountApi.changePassword(newPassword);
- setDialogOpen(false);
- console.debug(`[Account] Password changed`);
- } catch (e) {
- console.log(`[Account] Error changing password`, e);
- if ((e instanceof UnauthorizedError)) {
- session.resetAndRedirect(routes.login);
- }
- // TODO show error
- }
- };
-
return (
<Pref labelId={labelId} title={t("account_basics_password_title")} description={t("account_basics_password_description")}>
<div aria-labelledby={labelId}>
@@ -119,8 +106,7 @@ const ChangePassword = () => {
<ChangePasswordDialog
key={`changePasswordDialog${dialogKey}`}
open={dialogOpen}
- onCancel={handleDialogCancel}
- onSubmit={handleDialogSubmit}
+ onClose={handleDialogClose}
/>
</Pref>
)
@@ -128,16 +114,44 @@ const ChangePassword = () => {
const ChangePasswordDialog = (props) => {
const { t } = useTranslation();
+ const [currentPassword, setCurrentPassword] = useState("");
const [newPassword, setNewPassword] = useState("");
const [confirmPassword, setConfirmPassword] = useState("");
+ const [errorText, setErrorText] = useState("");
+
const fullScreen = useMediaQuery(theme.breakpoints.down('sm'));
- const changeButtonEnabled = (() => {
- return newPassword.length > 0 && newPassword === confirmPassword;
- })();
+
+ const handleDialogSubmit = async () => {
+ try {
+ console.debug(`[Account] Changing password`);
+ await accountApi.changePassword(currentPassword, newPassword);
+ props.onClose();
+ } catch (e) {
+ console.log(`[Account] Error changing password`, e);
+ if ((e instanceof CurrentPasswordWrongError)) {
+ setErrorText(t("account_basics_password_dialog_current_password_incorrect"));
+ } else if ((e instanceof UnauthorizedError)) {
+ session.resetAndRedirect(routes.login);
+ }
+ // TODO show error
+ }
+ };
+
return (
<Dialog open={props.open} onClose={props.onCancel} fullScreen={fullScreen}>
<DialogTitle>{t("account_basics_password_dialog_title")}</DialogTitle>
<DialogContent>
+ <TextField
+ margin="dense"
+ id="current-password"
+ label={t("account_basics_password_dialog_current_password_label")}
+ aria-label={t("account_basics_password_dialog_current_password_label")}
+ type="password"
+ value={currentPassword}
+ onChange={ev => setCurrentPassword(ev.target.value)}
+ fullWidth
+ variant="standard"
+ />
<TextField
margin="dense"
id="new-password"
@@ -161,10 +175,15 @@ const ChangePasswordDialog = (props) => {
variant="standard"
/>
</DialogContent>
- <DialogActions>
- <Button onClick={props.onCancel}>{t("account_basics_password_dialog_button_cancel")}</Button>
- <Button onClick={() => props.onSubmit(newPassword)} disabled={!changeButtonEnabled}>{t("account_basics_password_dialog_button_submit")}</Button>
- </DialogActions>
+ <DialogFooter status={errorText}>
+ <Button onClick={props.onClose}>{t("account_basics_password_dialog_button_cancel")}</Button>
+ <Button
+ onClick={handleDialogSubmit}
+ disabled={newPassword.length === 0 || currentPassword.length === 0 || newPassword !== confirmPassword}
+ >
+ {t("account_basics_password_dialog_button_submit")}
+ </Button>
+ </DialogFooter>
</Dialog>
);
};
diff --git a/web/src/components/Preferences.js b/web/src/components/Preferences.js
index 88b97fc2..2ec59738 100644
--- a/web/src/components/Preferences.js
+++ b/web/src/components/Preferences.js
@@ -548,11 +548,9 @@ const ReservationsTable = (props) => {
const [dialogOpen, setDialogOpen] = useState(false);
const [dialogReservation, setDialogReservation] = useState(null);
const { subscriptions } = useOutletContext();
- const localSubscriptions = Object.assign(
- ...subscriptions
- .filter(s => s.baseUrl === config.base_url)
- .map(s => ({[s.topic]: s}))
- );
+ const localSubscriptions = (subscriptions?.length > 0)
+ ? Object.assign(...subscriptions.filter(s => s.baseUrl === config.base_url).map(s => ({[s.topic]: s})))
+ : [];
const handleEditClick = (reservation) => {
setDialogKey(prev => prev+1);