Repository navigation
Conversation
The Authorization tab of the Access List modal stays mounted while the Details and Access Rules tabs are shown, so password managers fill the username and password inputs even though the user never opens that tab. Saving then silently enables basic auth on the access list with the admin's own NPM credentials. Browsers ignore autocomplete="off" on credential inputs. Use autocomplete="new-password" on both, as ChangePasswordModal already does, and give the inputs non-credential names so heuristic matching has nothing to latch onto. Fixes NginxProxyManager#5867
The autofill regression test only rendered the component once per case, so the name and autocomplete attributes were unpinned on rows created by the Add button and on rows renumbered after a remove. Split the multi-assert cases so each arm is shown to discriminate on its own, add the empty form-field-name boundary, and add a control asserting the submitted items payload is unchanged by the new DOM name attributes.
Share one render helper, table the attribute cases and use short behaviour names, keeping every input that was covered before.
handleRemove pushes a fresh blank item when the list would become empty, a second path onto the row template that the add and renumber cases do not reach.
Fold the duplicated name and autocomplete cases into two tables, drop the value and placeholder cases that do not depend on the changed attributes, and make the submission case fire a submit and assert the values the handler receives.
The row add, remove and submit cases covered behaviour this change does not touch.
|
Docker Image for build 2 is available on DockerHub: Note Ensure you backup your NPM instance before testing this image! Especially if there are database changes. Warning Changes and additions to DNS Providers require verification by at least 2 members of the community! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tab-panethat is always mounted (hidden by CSS, never unmounted), so its username/password inputs are live in the DOM while the user is on the Details or Access Rules tab.autoComplete="off", which Chrome and Firefox deliberately ignore for credential fields, and carried nonameattribute at all, so password-manager heuristics had nothing to exclude them by.handleChangewrites it into Formik, andAccessListModal'sonSubmitsends it aspayload.items, silently enabling HTTP Basic Auth on every proxy host using that access list.autoComplete="new-password"on both inputs (the value the repo's ownChangePasswordModalalready uses for exactly this purpose) plus per-row non-credentialnameattributes. One file, four lines changed.Tests
BasicAuthFields.test.tsxchecks the inputs'nameandautocompleteattributes. It fails ondevelopand passes with the fix (npx vitest run src/components/Form/BasicAuthFields.test.tsx).Why
A password manager fills the admin login into the hidden Authorization tab of the Access List modal, and saving the list then turns on HTTP Basic Auth for every proxy host using it (#5867). The inputs need an autocomplete value browsers honour for credential fields and non-credential names.
Type of Change
AI Usage
AI assistance: this bug was found and the fix and tests were drafted with AI tooling in my workflow; the tests and checks above were run as described. I'm responsible for the change and will handle review feedback.