Repository navigation
mod_authn_dbd, mod_authn_socache: Preserve extra query columns on a c… - #982
Open
arturobernalg wants to merge 1 commit into
Open
arturobernalg wants to merge 1 commit into
arturobernalg wants to merge 1 commit into
Conversation
…ache hit AuthDBDUserPWQuery may select columns beyond the password; each extra column is exported as AUTHENTICATE_<NAME> in r->subprocess_env by authn_dbd_password()/authn_dbd_realm(), the documented way to pass along per-user data from the database. With AuthBasicProvider "socache dbd" (the combination the docs recommend for caching), only the first, uncached login for a user goes through that code; once mod_authn_socache has a cache entry, its own check_password()/ get_realm_hash() answer directly from the cached password hash and never call back into mod_authn_dbd, so AUTHENTICATE_* silently disappears on every following login until the entry expires. Have mod_authn_dbd append the already-exported extra columns to the string it hands to ap_authn_cache_store(), as one NAME=VALUE line per column after the password hash (a column whose value contains a newline is left out of the cache, though still set in the current request's own environment, since it would be misread as an extra line). mod_authn_socache splits the hash back off on a cache hit and restores those entries into r->subprocess_env before validating, so a cache hit behaves like a real lookup from the caller's point of view. PR 65172.
Collaborator
|
Can you put the test under |
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.
AuthDBDUserPWQuery may select columns beyond the password; each extra column is exported as AUTHENTICATE_ in r->subprocess_env by authn_dbd_password()/authn_dbd_realm(), the documented way to pass along per-user data from the database. With AuthBasicProvider "socache dbd" (the combination the docs recommend for caching), only the first, uncached login for a user goes through that code; once mod_authn_socache has a cache entry, its own check_password()/get_realm_hash() answer directly from the cached password hash and never call back into mod_authn_dbd, so AUTHENTICATE_* silently disappears on every following login until the entry expires.
Have mod_authn_dbd append the already-exported extra columns to the string it hands to ap_authn_cache_store(), as one NAME=VALUE line per column after the password hash (a column whose value contains a newline is left out of the cache, though still set in the current request's own environment, since it would be misread as an extra line). mod_authn_socache splits the hash back off on a cache hit and restores those entries into r->subprocess_env before validating, so a cache hit behaves like a real lookup from the caller's point of view. This does not change the signature of ap_authn_cache_store(), which already takes a plain string.
PR 65172.
Verified with a new pytest case (test/modules/authn_dbd_cache/) against a real built server (SQLite + mod_dbd + a CGI script reading the environment): the first login shows the extra column correctly; a second login with the same credentials, served from the cache, loses it before this change and keeps it after. The existing aaa test suite (84 tests) passes unchanged.
Checklist:
[x] This PR does not report or address a security vulnerability (see SECURITY.md and https://www.apache.org/security/#reporting-a-vulnerability).
[x] Code follows the httpd style guide (https://httpd.apache.org/dev/styleguide.html).
[x] New log messages: none added by this change.
[x] changes-entries/65172.txt is included, following the README.CHANGES template.
[x] Test cases are included (test/modules/authn_dbd_cache/) and pass locally against a real build.