Skip to content

luci-base: fix setPassword handling#8696

Open
ckorber wants to merge 4 commits into
openwrt:masterfrom
ckorber:pr/fix_pw
Open

luci-base: fix setPassword handling#8696
ckorber wants to merge 4 commits into
openwrt:masterfrom
ckorber:pr/fix_pw

Conversation

@ckorber

@ckorber ckorber commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Pull request details

Description

The earlier commits didn't work for creating new users in rpcd.
Also, creating new users resulted in duplicated rpcd logins, because save and apply also created the uci section. So therefore, setPassword returns the generated hash and let luci-mod-system and luci-app-acl set the new hash.

Screenshot or video of changes (if applicable)

Maintainer (preferred)

@systemcrash @AndyChiang888


Tested on

OpenWrt version: openwrt-25.12
LuCI version: luci-openwrt-25.12
Web browser(s): firefox


Checklist

  • This PR is not from my main or master branch 💩, but a separate branch. ✅
  • Each commit has a valid ✒️ Signed-off-by: <my@email.address> row (via git commit --signoff).
  • Each commit and PR title has a valid 📝 <package name>: title first line subject for packages.
  • Incremented 🆙 any PKG_VERSION in the Makefile.

Remove old password check in JS when creating a new user,
as username verification is handled by the backend.

Signed-off-by: Andy Chiang <AndyChiang_git@outlook.com>
@ckorber ckorber changed the title Pr/fix pw luci-base: fix setPassword handling Jun 8, 2026
@ckorber ckorber force-pushed the pr/fix_pw branch 3 times, most recently from 60d2e3e to 0688112 Compare June 8, 2026 11:23
@AndyChiang888

Copy link
Copy Markdown
Contributor

Thanks, I'll test it later.

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 new commits. Two related issues stand out: the backend's known_user failure branch now writes the new password before returning result: 0 (bypassing the old-password check), and both frontends still treat the resolved promise as a scalar after expect: { result: 1 } was dropped, so the failure return { result: 0 } is read as success. Details inline.


Generated by Claude Code

Comment thread modules/luci-base/root/usr/share/rpcd/ucode/luci
Comment thread modules/luci-mod-system/htdocs/luci-static/resources/view/system/password.js Outdated
Comment thread applications/luci-app-acl/htdocs/luci-static/resources/view/system/acl.js Outdated
ckorber added 3 commits June 10, 2026 11:17
The earlier commit to unify password setting had multiple uci contexts
and didn't distinguish correctly between newly created or existing rpcd
users. Also a break command was misplaced.

With this commit the created hash is also propagated in order to set it
in `luci-app-acl` and `luci-mod-system`.

Signed-off-by: Christian Korber <ckorber@tdt.de>
This commit sets the generated hash after changing password for rpc
users.

Signed-off-by: Christian Korber <ckorber@tdt.de>
This commit sets the hash for changed password of rpc user.

Signed-off-by: Christian Korber <ckorber@tdt.de>

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 new commits since my last review. All three earlier findings (the failure-path write in the backend, and the result/result.result truthiness checks in both frontends) are addressed correctly. One remaining question about new-user creation in the system password view is inline. Commit messages match their changes.


Generated by Claude Code

@ckorber

ckorber commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Hi @systemcrash , do you approve?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants