mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 19:11:06 +01:00
test(auth): stop proxy provisioning tests depending on run order (#1381)
I hit this while building the Debrid-Link client in #1380. The new tests there changed the test count, that reshuffled the xdist workers, and these two went red. They were related to this branch, but were tests related to another PR I did which didn't have proper tests.. `test_sets_session_from_header` and `test_reads_remote_user_wsgi_fallback` both assert that a proxy-authenticated user comes back with `is_admin is True`. Since #1356 that only holds for the bootstrap account, because `_proxy_default_is_admin` returns True only while `has_admin()` is False. Both tests assume they're provisioning the first account, and only one of them can be. Run with `-n 0` so nothing is sharded: | | | |---|---| | either test alone | passes | | both, file order | the second fails | | both, reversed | the second fails | | both, across 2 workers | both pass | Reversing the order moving which one breaks is what makes it an ordering problem rather than a real one. It stays green on CI because the suite runs with `-n auto` and `tests/conftest.py` calls `mkdtemp` at module level, which runs once per worker process. Each worker gets its own `CONFIG_DIR` and its own `users.db`, the two tests land on different workers, and each one is genuinely first in its own database. That passed but it's luck rather than design, and any change to the test count can put them back together. So this gives every test in the file an empty user table and stops the question of who ran first from mattering. While I was fixed that I added coverage for the rule itself, which I had failed to test for: - the bootstrap account is an admin and the next one isn't - `PROXY_AUTH_DEFAULT_ROLE=admin` promotes later accounts - a user already in the database keeps its stored role instead of picking up the default Tests only, no source changes. The full suite passes serially now, where it had those two failures before, and it's still green under `-n auto`.
This commit is contained in:
@@ -36,6 +36,25 @@ def main_module():
|
||||
return main
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _empty_user_table(main_module):
|
||||
"""Give every test an instance with no accounts yet.
|
||||
|
||||
Proxy provisioning keys off whether the instance already has an admin, so
|
||||
two tests sharing a user table are really asserting the order they happened
|
||||
to run in. The suite runs with xdist, where each worker gets its own
|
||||
CONFIG_DIR, so that order changes whenever the test count does.
|
||||
"""
|
||||
|
||||
def _clear() -> None:
|
||||
for user in main_module.user_db.list_users():
|
||||
main_module.user_db.delete_user(user["id"])
|
||||
|
||||
_clear()
|
||||
yield
|
||||
_clear()
|
||||
|
||||
|
||||
class TestProxyAuthMiddleware:
|
||||
def test_skips_for_non_proxy_mode(self, main_module):
|
||||
with (
|
||||
@@ -231,6 +250,92 @@ class TestProxyAuthMiddleware:
|
||||
assert db_user is not None
|
||||
assert db_user["username"] == username
|
||||
|
||||
def test_first_account_is_admin_and_later_ones_are_not(self, main_module):
|
||||
"""The bootstrap account is an admin; after that the default role decides."""
|
||||
with (
|
||||
patch.object(main_module, "get_auth_mode", return_value="proxy"),
|
||||
patch.object(
|
||||
main_module.app_config,
|
||||
"get",
|
||||
side_effect=_config_getter({"PROXY_AUTH_USER_HEADER": "X-Auth-User"}),
|
||||
),
|
||||
):
|
||||
with main_module.app.test_request_context(
|
||||
"/api/releases",
|
||||
headers={"X-Auth-User": "first_proxy_user"},
|
||||
):
|
||||
assert main_module.proxy_auth_middleware() is None
|
||||
assert main_module.session.get("is_admin") is True
|
||||
|
||||
# The instance now has an admin, so nobody is at risk of being locked
|
||||
# out and the next account follows PROXY_AUTH_DEFAULT_ROLE.
|
||||
with main_module.app.test_request_context(
|
||||
"/api/releases",
|
||||
headers={"X-Auth-User": "second_proxy_user"},
|
||||
):
|
||||
assert main_module.proxy_auth_middleware() is None
|
||||
assert main_module.session.get("is_admin") is False
|
||||
|
||||
def test_default_role_admin_promotes_later_accounts(self, main_module):
|
||||
main_module.user_db.create_user(
|
||||
username="existing_admin",
|
||||
role="admin",
|
||||
auth_source="proxy",
|
||||
)
|
||||
|
||||
with (
|
||||
patch.object(main_module, "get_auth_mode", return_value="proxy"),
|
||||
patch.object(
|
||||
main_module.app_config,
|
||||
"get",
|
||||
side_effect=_config_getter(
|
||||
{
|
||||
"PROXY_AUTH_USER_HEADER": "X-Auth-User",
|
||||
"PROXY_AUTH_DEFAULT_ROLE": "admin",
|
||||
}
|
||||
),
|
||||
),
|
||||
main_module.app.test_request_context(
|
||||
"/api/releases",
|
||||
headers={"X-Auth-User": "later_proxy_admin"},
|
||||
),
|
||||
):
|
||||
assert main_module.proxy_auth_middleware() is None
|
||||
assert main_module.session.get("is_admin") is True
|
||||
|
||||
def test_existing_user_keeps_its_stored_role(self, main_module):
|
||||
main_module.user_db.create_user(
|
||||
username="existing_admin",
|
||||
role="admin",
|
||||
auth_source="proxy",
|
||||
)
|
||||
main_module.user_db.create_user(
|
||||
username="known_plain_user",
|
||||
role="user",
|
||||
auth_source="proxy",
|
||||
)
|
||||
|
||||
with (
|
||||
patch.object(main_module, "get_auth_mode", return_value="proxy"),
|
||||
patch.object(
|
||||
main_module.app_config,
|
||||
"get",
|
||||
side_effect=_config_getter(
|
||||
{
|
||||
"PROXY_AUTH_USER_HEADER": "X-Auth-User",
|
||||
"PROXY_AUTH_DEFAULT_ROLE": "admin",
|
||||
}
|
||||
),
|
||||
),
|
||||
main_module.app.test_request_context(
|
||||
"/api/releases",
|
||||
headers={"X-Auth-User": "known_plain_user"},
|
||||
),
|
||||
):
|
||||
assert main_module.proxy_auth_middleware() is None
|
||||
# The stored role wins; the default only applies to new accounts.
|
||||
assert main_module.session.get("is_admin") is False
|
||||
|
||||
def test_returns_401_when_header_missing_on_protected_path(self, main_module):
|
||||
with (
|
||||
patch.object(main_module, "get_auth_mode", return_value="proxy"),
|
||||
|
||||
Reference in New Issue
Block a user