Store multiple named HTTP Basic credentials per install - #3433
Merged
Conversation
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
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
http_basicnow stores an account-keyed credential map instead of a single credentialget_account_names()anddelete_account()Problem
The provider is documented as generic, and its config field reads "Local auth ref account name, for example logstash" — implying many. It stored exactly one:
save_config()replaced the whole blob, so configuring a second service silently evicted the first. On an install already usinghttp_basic:logstash, addinghttp_basic:matticspacewould have broken Logstash with no error at write time and a confusingauth_ref_unresolvedlater.This is the "which credential" axis. It is orthogonal to the principal scoping the base class already provides, which is the "whose credential" axis — both now apply, and a user-scoped config holds its own account set.
Encryption
encrypt_fields()only walks top-level string keys, so nesting credentials underaccountswould have written every password towp_sitemetain plaintext. Each account entry is therefore encrypted explicitly by the provider on write and decrypted on read. A regression test asserts the plaintext password appears nowhere in the stored option, not merely that the expected key is enveloped.Compatibility
Installs holding the pre-change flat shape are read as a single-entry map, so existing auth refs resolve untouched and the next save rewrites them into the map. Covered by tests, including adding a new account alongside a legacy one.
Verification
tests/http-basic-auth-provider-smoke.php— rewritten: coexistence, rotation isolation, deletion, unconfigured lookups, legacy shape, legacy+new coexistence, and no-plaintext-at-resthttp_basic:logstashcredential still resolves against production Logstash after the storage changeAI assistance
OpenAI gpt-5.6-sol via OpenCode found the single-credential limitation while wiring a second Matticspace-backed service, implemented the account map and per-entry encryption, and wrote the regression coverage. Chris Huber directed the work and is responsible for every line.