Skip to content

Commit

Permalink
🔒️ Require the current password for changing it
Browse files Browse the repository at this point in the history
Admins may still change existing user account passwords without
having to enter the current one, however the regular user oriented
password change dialog has been adjusted to require entry of the
current password. The API has been locked down accordingly and
the password change endpoint has seen a small change due to that,
please refer to the updated documentation.
  • Loading branch information
foosel committed Aug 22, 2022
1 parent 9836850 commit 1453076
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 30 deletions.
14 changes: 10 additions & 4 deletions docs/api/access.rst
Original file line number Diff line number Diff line change
Expand Up @@ -238,16 +238,22 @@ Change a user's password
Changes the password of a user.

Expects a JSON object with a single property ``password`` as request body.
Expects a JSON object with a property ``password`` containing the new password as
request body. Without the ``SETTINGS`` permission, an additional property ``current``
is also required to be set on the request body, containing the user's current password.

Requires the ``SETTINGS`` permission or to be logged in as the user.
Requires the ``SETTINGS`` permission or to be logged in as the user. Note that ``current``
will be evaluated even in presence of the ``SETTINGS`` permission, if set.

:param username: Name of the user to change the password for
:json password: The new password to set
:json current: The current password
:status 200: No error
:status 400: If the request doesn't contain a ``password`` property or the request
:status 400: If the request doesn't contain a ``password`` property, doesn't
contain a ``current`` property even though required, or the request
is otherwise invalid
:status 403: No admin rights and not logged in as the user
:status 403: No admin rights, not logged in as the user or a current password
mismatch
:status 404: The user is unknown

.. _sec-api-access-users-settings-get:
Expand Down
14 changes: 12 additions & 2 deletions src/octoprint/server/api/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@ def change_password_for_user(username):
if (
current_user is not None
and not current_user.is_anonymous
and (current_user.get_name() == username or current_user.is_admin)
and (
current_user.get_name() == username
or current_user.has_permission(Permissions.SETTINGS)
)
):
if "application/json" not in request.headers["Content-Type"]:
abort(400, description="Expected content-type JSON")
Expand All @@ -252,7 +255,14 @@ def change_password_for_user(username):
abort(400, description="Malformed JSON body in request")

if "password" not in data or not data["password"]:
abort(400, description="password is missing")
abort(400, description="new password is missing")

if not current_user.has_permission(Permissions.SETTINGS) or "current" in data:
if "current" not in data or not data["current"]:
abort(400, description="current password is missing")

if not userManager.check_password(username, data["current"]):
abort(403, description="Invalid current password")

try:
userManager.change_user_password(username, data["password"])
Expand Down
11 changes: 10 additions & 1 deletion src/octoprint/static/js/app/client/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,26 @@
OctoPrintAccessUsersClient.prototype.changePassword = function (
name,
password,
oldpw,
opts
) {
if (_.isObject(oldpw)) {
opts = oldpw;
oldpw = undefined;
}

if (!name || !password) {
throw new OctoPrintClient.InvalidArgumentError(
"user name and password must be set"
"user name and new password must be set"
);
}

var data = {
password: password
};
if (oldpw) {
data["current"] = oldpw;
}
return this.base.putJson(this.url(name, "password"), data, opts);
};

Expand Down
27 changes: 21 additions & 6 deletions src/octoprint/static/js/app/viewmodels/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ $(function () {
groups: ko.observableArray([]),
permissions: ko.observableArray([]),
password: ko.observable(undefined),
currentPassword: ko.observable(undefined),
repeatedPassword: ko.observable(undefined),
passwordMismatch: ko.pureComputed(function () {
return self.editor.password() !== self.editor.repeatedPassword();
}),
currentPasswordMismatch: ko.observable(false),
apikey: ko.observable(undefined),
active: ko.observable(undefined),
permissionSelectable: function (permission) {
Expand Down Expand Up @@ -128,6 +130,11 @@ $(function () {
}
self.editor.password(undefined);
self.editor.repeatedPassword(undefined);
self.editor.currentPassword(undefined);
self.editor.currentPasswordMismatch(false);
});
self.editor.currentPassword.subscribe(function () {
self.editor.currentPasswordMismatch(false);
});

self.requestData = function () {
Expand Down Expand Up @@ -244,13 +251,21 @@ $(function () {
self.confirmChangePassword = function () {
if (!CONFIG_ACCESS_CONTROL) return;

self.updatePassword(self.currentUser().name, self.editor.password()).done(
function () {
self.updatePassword(
self.currentUser().name,
self.editor.password(),
self.editor.currentPassword()
)
.done(function () {
// close dialog
self.currentUser(undefined);
self.changePasswordDialog.modal("hide");
}
);
})
.fail(function (xhr) {
if (xhr.status === 403) {
self.currentPasswordMismatch(true);
}
});
};

self.confirmGenerateApikey = function () {
Expand Down Expand Up @@ -349,8 +364,8 @@ $(function () {
.done(self.fromResponse);
};

self.updatePassword = function (username, password) {
return OctoPrint.access.users.changePassword(username, password);
self.updatePassword = function (username, password, current) {
return OctoPrint.access.users.changePassword(username, password, current);
};

self.generateApikey = function (username) {
Expand Down
54 changes: 37 additions & 17 deletions src/octoprint/static/js/app/viewmodels/usersettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,17 @@ $(function () {

self.access_password = ko.observable(undefined);
self.access_repeatedPassword = ko.observable(undefined);
self.access_currentPassword = ko.observable(undefined);
self.access_currentPasswordMismatch = ko.observable(false);
self.access_apikey = ko.observable(undefined);
self.interface_language = ko.observable(undefined);

self.currentUser = ko.observable(undefined);
self.currentUser.subscribe(function (newUser) {
self.access_password(undefined);
self.access_repeatedPassword(undefined);
self.access_currentPassword(undefined);
self.access_currentPasswordMismatch(false);
self.access_apikey(undefined);
self.interface_language("_default");

Expand All @@ -45,6 +49,9 @@ $(function () {
}
}
});
self.access_currentPassword.subscribe(function () {
self.access_currentPasswordMismatch(false);
});

self.passwordMismatch = ko.pureComputed(function () {
return self.access_password() !== self.access_repeatedPassword();
Expand Down Expand Up @@ -81,25 +88,38 @@ $(function () {

self.userSettingsDialog.trigger("beforeSave");

if (self.access_password() && !self.passwordMismatch()) {
self.users.updatePassword(
self.currentUser().name,
self.access_password(),
function () {}
);
function process() {
var settings = {
interface: {
language: self.interface_language()
}
};
self.updateSettings(self.currentUser().name, settings).done(function () {
// close dialog
self.currentUser(undefined);
self.userSettingsDialog.modal("hide");
self.loginState.reloadUser();
});
}

var settings = {
interface: {
language: self.interface_language()
}
};
self.updateSettings(self.currentUser().name, settings).done(function () {
// close dialog
self.currentUser(undefined);
self.userSettingsDialog.modal("hide");
self.loginState.reloadUser();
});
if (self.access_password() && !self.passwordMismatch()) {
self.users
.updatePassword(
self.currentUser().name,
self.access_password(),
self.access_currentPassword()
)
.done(function () {
process();
})
.fail(function (xhr) {
if (xhr.status === 403) {
self.access_currentPasswordMismatch(true);
}
});
} else {
process();
}
};

self.copyApikey = function () {
Expand Down
7 changes: 7 additions & 0 deletions src/octoprint/templates/dialogs/usersettings/access.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@
<p>
{{ _('If you do not wish to change your password, just leave the following fields empty.') }}
</p>
<div class="control-group" data-bind="css: {error: access_currentPasswordMismatch()}">
<label class="control-label" for="userSettings-access_currentPassword">{{ _('Current Password') }}</label>
<div class="controls">
<input type="password" class="input-block-level" id="userSettings-access_currentPassword" data-bind="value: access_currentPassword, valueUpdate: 'afterkeydown'" required>
<span class="help-inline" data-bind="visible: access_currentPasswordMismatch()">{{ _('Passwords do not match') }}</span>
</div>
</div>
<div class="control-group">
<label class="control-label" for="userSettings-access_password">{{ _('New Password') }}</label>
<div class="controls">
Expand Down

0 comments on commit 1453076

Please sign in to comment.