mirror of
https://github.com/NginxProxyManager/nginx-proxy-manager.git
synced 2026-09-24 19:00:20 +01:00
Stamp the password change from the app clock, not the database one
Your CI caught this: the check passed on SQLite and never fired on the stack where the database container runs on a different timezone from the app, so a stale token stayed valid. The comparison was between a token's `iat`, which is UTC seconds from Node, and `auth.modified_on`, which the driver hands back interpreted in the app's timezone. With the app on Australia/Brisbane and the database on UTC, that column comes back ten hours in the past and the token always looks newer than the change. Record the moment in `auth.meta.password_changed_at` instead, written by `setPassword` with the same `Date.now()` clock that mints `iat`. Same unit on both sides, one clock, and no timestamp parsing: the Date and local-string branch is gone, and so is the whole-second flooring that Postgres microseconds made necessary. Rows written before this have no marker and revoke nothing until their next password change, which is the safe direction to be wrong in.
This commit is contained in:
@@ -388,11 +388,21 @@ const internalUser = {
|
||||
.andWhere("type", data.type)
|
||||
.first()
|
||||
.then((existing_auth) => {
|
||||
// Stamped here rather than read off modified_on, because it is compared against a
|
||||
// token's `iat` and the two only line up when the same clock writes both. The
|
||||
// database clock is a different one: with the app on one timezone and the database
|
||||
// on another, its timestamps come back hours away from where Node thinks it is.
|
||||
const password_changed_at = Math.floor(Date.now() / 1000);
|
||||
|
||||
if (existing_auth) {
|
||||
// patch
|
||||
const meta = existing_auth.meta || {};
|
||||
meta.password_changed_at = password_changed_at;
|
||||
|
||||
return authModel.query().where("user_id", user.id).andWhere("type", data.type).patch({
|
||||
type: data.type, // This is required for the model to encrypt on save
|
||||
secret: data.secret,
|
||||
meta,
|
||||
});
|
||||
}
|
||||
// insert
|
||||
@@ -400,7 +410,7 @@ const internalUser = {
|
||||
user_id: user.id,
|
||||
type: data.type,
|
||||
secret: data.secret,
|
||||
meta: {},
|
||||
meta: { password_changed_at },
|
||||
});
|
||||
})
|
||||
.then(() => {
|
||||
|
||||
+5
-12
@@ -90,18 +90,11 @@ export default function (tokenString) {
|
||||
.where("type", "=", "password")
|
||||
.first();
|
||||
|
||||
if (auth && typeof tokenData.iat === "number") {
|
||||
// SQLite gives this back as a local time string, the other drivers as a Date.
|
||||
const changedAt =
|
||||
auth.modified_on instanceof Date
|
||||
? auth.modified_on.getTime()
|
||||
: Date.parse(String(auth.modified_on).replace(" ", "T"));
|
||||
|
||||
// Whole seconds on both sides, which is all `iat` carries, so a token issued in the
|
||||
// same second as the change is kept. Postgres stores this column to the microsecond.
|
||||
if (!Number.isNaN(changedAt) && tokenData.iat < Math.floor(changedAt / 1000)) {
|
||||
throw new errs.TokenRevokedError("Token was issued before the password was changed");
|
||||
}
|
||||
// Both sides come from the same clock and in the same unit, whole seconds since
|
||||
// the epoch: `setPassword` stamps the marker and `jsonwebtoken` stamps `iat`.
|
||||
const changedAt = auth?.meta?.password_changed_at;
|
||||
if (changedAt && typeof tokenData.iat === "number" && tokenData.iat < changedAt) {
|
||||
throw new errs.TokenRevokedError("Token was issued before the password was changed");
|
||||
}
|
||||
|
||||
initialised = true;
|
||||
|
||||
Reference in New Issue
Block a user