From 0cce66b6b83fffa709aeeebb30e465728a8e01ff Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Tue, 17 Mar 2026 14:56:46 -0300 Subject: [PATCH 1/8] Set REMOTE_USER in WSGI environ and include it in uwsgi log format Set REMOTE_USER from the Basic auth username on the WSGI environ so uwsgi can log the patron identifier. Update the uwsgi log format to include %(var.REMOTE_USER) in place of the hardcoded dash. Only applies to Basic auth requests to avoid logging sensitive bearer tokens. --- docker/services/uwsgi/uwsgi.d/40_log.ini | 2 +- src/palace/manager/api/controller/base.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docker/services/uwsgi/uwsgi.d/40_log.ini b/docker/services/uwsgi/uwsgi.d/40_log.ini index f2a1b9cc1b..f2bfdf136e 100644 --- a/docker/services/uwsgi/uwsgi.d/40_log.ini +++ b/docker/services/uwsgi/uwsgi.d/40_log.ini @@ -1,4 +1,4 @@ [uwsgi] -log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) - - [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) +log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) %(var.REMOTE_USER) - [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) logger = stdio: logger = file:/var/log/uwsgi/uwsgi.log diff --git a/src/palace/manager/api/controller/base.py b/src/palace/manager/api/controller/base.py index c02c7fb976..ae8385e5a9 100644 --- a/src/palace/manager/api/controller/base.py +++ b/src/palace/manager/api/controller/base.py @@ -65,6 +65,10 @@ def authenticated_patron_from_request(self) -> ProblemDetail | Patron | Response # No credentials were provided. return self.authenticate() + # Set REMOTE_USER for uwsgi access log visibility + if auth.type.lower() == "basic" and auth.username: + flask.request.environ["REMOTE_USER"] = auth.username + patron = self.authenticated_patron(auth) if isinstance(patron, Patron): setattr(flask.request, "patron", patron) From a54534676b503b45b19a844194dbc91940b1b226 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Tue, 17 Mar 2026 15:18:41 -0300 Subject: [PATCH 2/8] Improve REMOTE_USER comments and set for non-basic auth flows --- src/palace/manager/api/controller/base.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/palace/manager/api/controller/base.py b/src/palace/manager/api/controller/base.py index ae8385e5a9..2244c81395 100644 --- a/src/palace/manager/api/controller/base.py +++ b/src/palace/manager/api/controller/base.py @@ -65,7 +65,8 @@ def authenticated_patron_from_request(self) -> ProblemDetail | Patron | Response # No credentials were provided. return self.authenticate() - # Set REMOTE_USER for uwsgi access log visibility + # Set REMOTE_USER for uwsgi access log visibility. We set this early, before + # authenticating the patron, so that we have the username on authentication failures. if auth.type.lower() == "basic" and auth.username: flask.request.environ["REMOTE_USER"] = auth.username @@ -73,6 +74,14 @@ def authenticated_patron_from_request(self) -> ProblemDetail | Patron | Response if isinstance(patron, Patron): setattr(flask.request, "patron", patron) + # Set REMOTE_USER again here for non-basic auth flows, using the patron's + # authorization identifier to provide detail in access logs. + if ( + flask.request.environ.get("REMOTE_USER") is None + and patron.authorization_identifier + ): + flask.request.environ["REMOTE_USER"] = patron.authorization_identifier + return patron def authenticated_patron( From 3e025e2979f6d7087a1a7c4b55b7ebf22c18b587 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Tue, 17 Mar 2026 15:29:03 -0300 Subject: [PATCH 3/8] Add tests for REMOTE_USER setting in authenticated_patron_from_request --- tests/manager/api/controller/test_base.py | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/manager/api/controller/test_base.py b/tests/manager/api/controller/test_base.py index b21d51a987..58f7d0ace3 100644 --- a/tests/manager/api/controller/test_base.py +++ b/tests/manager/api/controller/test_base.py @@ -138,6 +138,48 @@ def test_authenticated_patron_from_request( assert result.status_code == 401 assert getattr(flask.request, "patron") is None + def test_authenticated_patron_from_request_sets_remote_user_basic_auth( + self, circulation_fixture: CirculationControllerFixture + ): + """REMOTE_USER is set from basic auth credentials, even on auth failure.""" + # Successful basic auth sets REMOTE_USER to the username. + with circulation_fixture.request_context_with_library( + "/", headers=dict(Authorization=circulation_fixture.valid_auth) + ): + result = circulation_fixture.controller.authenticated_patron_from_request() + assert isinstance(result, Patron) + assert flask.request.environ["REMOTE_USER"] == "unittestuser" + + # Failed basic auth still sets REMOTE_USER to the submitted username. + with circulation_fixture.request_context_with_library( + "/", + headers=dict(Authorization=circulation_fixture.invalid_auth), + ): + result = circulation_fixture.controller.authenticated_patron_from_request() + assert not isinstance(result, Patron) + assert flask.request.environ["REMOTE_USER"] == "user1" + + def test_authenticated_patron_from_request_sets_remote_user_non_basic_auth( + self, circulation_fixture: CirculationControllerFixture + ): + """REMOTE_USER is set from patron.authorization_identifier for non-basic auth.""" + patron = circulation_fixture.default_patron + patron.authorization_identifier = "patron-auth-id" + + with ( + patch.object( + circulation_fixture.manager.auth, + "authenticated_patron", + return_value=patron, + ), + circulation_fixture.request_context_with_library( + "/", headers=dict(Authorization="Bearer some-token") + ), + ): + result = circulation_fixture.controller.authenticated_patron_from_request() + assert result == patron + assert flask.request.environ["REMOTE_USER"] == "patron-auth-id" + def test_authenticated_patron_invalid_credentials( self, circulation_fixture: CirculationControllerFixture ): From bf67eaf471c8091b6dccf5b3163ffbdc55ac298e Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Tue, 17 Mar 2026 15:55:23 -0300 Subject: [PATCH 4/8] Use %(user) in uwsgi log format to default to - when REMOTE_USER is unset --- docker/services/uwsgi/uwsgi.d/40_log.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/services/uwsgi/uwsgi.d/40_log.ini b/docker/services/uwsgi/uwsgi.d/40_log.ini index f2bfdf136e..89524372cc 100644 --- a/docker/services/uwsgi/uwsgi.d/40_log.ini +++ b/docker/services/uwsgi/uwsgi.d/40_log.ini @@ -1,4 +1,4 @@ [uwsgi] -log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) %(var.REMOTE_USER) - [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) +log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) %(user) - [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) logger = stdio: logger = file:/var/log/uwsgi/uwsgi.log From b9af9000997185b615d9d7b0b29da062d7005f9c Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Wed, 18 Mar 2026 16:41:18 -0300 Subject: [PATCH 5/8] Use uwsgi.set_logvar to set REMOTE_USER for uwsgi log format Setting flask.request.environ["REMOTE_USER"] does not propagate to uwsgi's log formatter, which reads from its own internal request variables captured before the WSGI app runs. Use uwsgi.set_logvar() instead, which sets log variables that uwsgi reads after the response, and reference it with %(REMOTE_USER) logvar syntax in the log format. --- docker/services/uwsgi/uwsgi.d/40_log.ini | 2 +- src/palace/manager/api/controller/base.py | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/docker/services/uwsgi/uwsgi.d/40_log.ini b/docker/services/uwsgi/uwsgi.d/40_log.ini index 89524372cc..c11d7e15b1 100644 --- a/docker/services/uwsgi/uwsgi.d/40_log.ini +++ b/docker/services/uwsgi/uwsgi.d/40_log.ini @@ -1,4 +1,4 @@ [uwsgi] -log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) %(user) - [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) +log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) %(REMOTE_USER) - [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) logger = stdio: logger = file:/var/log/uwsgi/uwsgi.log diff --git a/src/palace/manager/api/controller/base.py b/src/palace/manager/api/controller/base.py index 2244c81395..b37ec6ab76 100644 --- a/src/palace/manager/api/controller/base.py +++ b/src/palace/manager/api/controller/base.py @@ -10,6 +10,11 @@ from palace.manager.util.log import LoggerMixin from palace.manager.util.problem_detail import BaseProblemDetailException, ProblemDetail +try: + import uwsgi +except ImportError: + uwsgi = None + class BaseCirculationManagerController(LoggerMixin): """Define minimal standards for a circulation manager controller, @@ -68,19 +73,19 @@ def authenticated_patron_from_request(self) -> ProblemDetail | Patron | Response # Set REMOTE_USER for uwsgi access log visibility. We set this early, before # authenticating the patron, so that we have the username on authentication failures. if auth.type.lower() == "basic" and auth.username: - flask.request.environ["REMOTE_USER"] = auth.username + self._set_logvar("REMOTE_USER", auth.username) patron = self.authenticated_patron(auth) if isinstance(patron, Patron): setattr(flask.request, "patron", patron) - # Set REMOTE_USER again here for non-basic auth flows, using the patron's + # Set REMOTE_USER for non-basic auth flows, using the patron's # authorization identifier to provide detail in access logs. if ( - flask.request.environ.get("REMOTE_USER") is None + not flask.request.environ.get("REMOTE_USER") and patron.authorization_identifier ): - flask.request.environ["REMOTE_USER"] = patron.authorization_identifier + self._set_logvar("REMOTE_USER", patron.authorization_identifier) return patron @@ -115,6 +120,13 @@ def authenticate(self) -> Response: data = self.manager.authentication_for_opds_document return Response(data, 401, headers) + @staticmethod + def _set_logvar(key: str, value: str) -> None: + """Set a uwsgi log variable and the WSGI environ entry.""" + flask.request.environ[key] = value + if uwsgi: + uwsgi.set_logvar(key, value) + def library_for_request( self, library_short_name: str | None ) -> Library | ProblemDetail: From c0c67f10b08abf9bb27cf877983b0076aabe517c Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Wed, 18 Mar 2026 16:54:22 -0300 Subject: [PATCH 6/8] Sanitize logvar values to prevent log injection uwsgi does not escape logvar values, so control characters like newlines can split log lines and shift fields. Replace all control characters with underscores before passing values to set_logvar. --- src/palace/manager/api/controller/base.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/palace/manager/api/controller/base.py b/src/palace/manager/api/controller/base.py index b37ec6ab76..85edeabef1 100644 --- a/src/palace/manager/api/controller/base.py +++ b/src/palace/manager/api/controller/base.py @@ -1,3 +1,5 @@ +import re + import flask from flask import Response from flask_babel import lazy_gettext as _ @@ -122,10 +124,18 @@ def authenticate(self) -> Response: @staticmethod def _set_logvar(key: str, value: str) -> None: - """Set a uwsgi log variable and the WSGI environ entry.""" - flask.request.environ[key] = value + """Set a uwsgi log variable and the WSGI environ entry. + + The value is sanitized to prevent log injection — uwsgi does not + escape logvar values, so control characters and whitespace that + could split or corrupt log lines are replaced. + """ + # Replace control characters and whitespace with underscores to + # prevent log injection (newlines split lines, tabs/spaces shift fields). + sanitized = re.sub(r"[\x00-\x1f\x7f]", "_", value) + flask.request.environ[key] = sanitized if uwsgi: - uwsgi.set_logvar(key, value) + uwsgi.set_logvar(key, sanitized) def library_for_request( self, library_short_name: str | None From 4ccae066ad7da506e00959416f5a1c98d120c080 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Wed, 18 Mar 2026 16:55:13 -0300 Subject: [PATCH 7/8] Move REMOTE_USER after ident field in uwsgi log format Place REMOTE_USER in the standard position matching the Combined Log Format used by nginx, after the ident dash. --- docker/services/uwsgi/uwsgi.d/40_log.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/services/uwsgi/uwsgi.d/40_log.ini b/docker/services/uwsgi/uwsgi.d/40_log.ini index c11d7e15b1..34ff84a63e 100644 --- a/docker/services/uwsgi/uwsgi.d/40_log.ini +++ b/docker/services/uwsgi/uwsgi.d/40_log.ini @@ -1,4 +1,4 @@ [uwsgi] -log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) %(REMOTE_USER) - [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) +log-format = [uwsgi] %(var.HTTP_X_FORWARDED_FOR) (%(addr)) - %(REMOTE_USER) [%(ltime)] "%(method) %(uri) %(proto)" %(status) %(size) "%(referer)" "%(uagent)" host_hdr=%(host) req_time_elapsed=%(msecs) process=%(pid) worker=%(wid) logger = stdio: logger = file:/var/log/uwsgi/uwsgi.log From a37067b47f278d5f195856e43abdddc9e89a4f85 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Wed, 18 Mar 2026 16:58:50 -0300 Subject: [PATCH 8/8] Quote logvar values containing spaces to preserve field boundaries Wrap sanitized values in double quotes when they contain spaces, so log parsers can correctly delimit fields. Values without spaces are left unquoted to keep the common case unchanged. --- src/palace/manager/api/controller/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/palace/manager/api/controller/base.py b/src/palace/manager/api/controller/base.py index 85edeabef1..6bb6db171c 100644 --- a/src/palace/manager/api/controller/base.py +++ b/src/palace/manager/api/controller/base.py @@ -130,9 +130,12 @@ def _set_logvar(key: str, value: str) -> None: escape logvar values, so control characters and whitespace that could split or corrupt log lines are replaced. """ - # Replace control characters and whitespace with underscores to - # prevent log injection (newlines split lines, tabs/spaces shift fields). + # Replace control characters with underscores to prevent log + # injection (newlines split lines, tabs shift fields). sanitized = re.sub(r"[\x00-\x1f\x7f]", "_", value) + # Quote values containing spaces to preserve log field boundaries. + if " " in sanitized: + sanitized = f'"{sanitized}"' flask.request.environ[key] = sanitized if uwsgi: uwsgi.set_logvar(key, sanitized)