You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by ar...@apache.org on 2026/03/31 13:29:30 UTC
(tooling-trusted-releases) branch session_validation created (now 97eab3a4)
This is an automated email from the ASF dual-hosted git repository.
arm pushed a change to branch session_validation
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
at 97eab3a4 Remove custom session age checking now we have it in asfquart, and add periodic revalidation of LDAP status
This branch includes the following new commits:
new 97eab3a4 Remove custom session age checking now we have it in asfquart, and add periodic revalidation of LDAP status
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org
(tooling-trusted-releases) 01/01: Remove custom session age checking now we have it in asfquart, and add periodic revalidation of LDAP status
Posted by ar...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
arm pushed a commit to branch session_validation
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
commit 97eab3a46eaee7224f9a58388e4120662bed921b
Author: Alastair McFarlane <ar...@apache.org>
AuthorDate: Tue Mar 31 14:29:10 2026 +0100
Remove custom session age checking now we have it in asfquart, and add periodic revalidation of LDAP status
---
atr/config.py | 3 ++-
atr/get/test.py | 48 +++++++++++++++++++++++++++++++++
atr/ldap.py | 2 ++
atr/server.py | 47 +++++++++++++++-----------------
playwright/test.py | 48 ++++++++++++++++++++++++++++-----
tests/unit/test_ldap.py | 72 +++++++++++++++++++++++++++++++++++++++++++++++--
6 files changed, 186 insertions(+), 34 deletions(-)
diff --git a/atr/config.py b/atr/config.py
index c144cb91..2bd57128 100644
--- a/atr/config.py
+++ b/atr/config.py
@@ -67,7 +67,7 @@ def _config_secrets_get(
class AppConfig:
- ABSOLUTE_SESSION_MAX_SECONDS = decouple.config("ABSOLUTE_SESSION_MAX_SECONDS", default=60 * 60 * 72, cast=int)
+ ACCOUNT_CHECK_INTERVAL = decouple.config("ACCOUNT_CHECK_INTERVAL", default=300, cast=int)
ALLOW_TESTS = decouple.config("ALLOW_TESTS", default=False, cast=bool)
ATR_STATUS = decouple.config("ATR_STATUS", default="ALPHA", cast=str)
DISABLE_CHECK_CACHE = decouple.config("DISABLE_CHECK_CACHE", default=False, cast=bool)
@@ -81,6 +81,7 @@ class AppConfig:
LOG_LEVEL = decouple.config("LOG_LEVEL", default="INFO", cast=lambda x: x.upper())
LOG_JSON = decouple.config("LOG_JSON", default=False, cast=bool)
LOG_PUBLIC_KEY = _config_secrets("LOG_PUBLIC_KEY", STATE_DIR, default=None, cast=str)
+ MAX_SESSION_AGE = decouple.config("MAX_SESSION_AGE", default=60 * 60 * 72, cast=int)
PUBSUB_URL = _config_secrets("PUBSUB_URL", STATE_DIR, default=None, cast=str)
PUBSUB_USER = _config_secrets("PUBSUB_USER", STATE_DIR, default=None, cast=str)
PUBSUB_PASSWORD = _config_secrets("PUBSUB_PASSWORD", STATE_DIR, default=None, cast=str)
diff --git a/atr/get/test.py b/atr/get/test.py
index f1ccba41..3c3a5346 100644
--- a/atr/get/test.py
+++ b/atr/get/test.py
@@ -86,6 +86,54 @@ async def test_login(_session: web.Public, _test_login: Literal["test/login"]) -
return await web.redirect(root.index)
+@get.typed
+async def test_login_banned(
+ _session: web.Public, _test_login_banned: Literal["test/login-banned"]
+) -> web.WerkzeugResponse:
+ """
+ URL: /test/login-banned
+ """
+ if not config.get().ALLOW_TESTS:
+ return quart.abort(404)
+
+ session_data = atr.models.session.CookieData(
+ uid="test-banned",
+ fullname="Banned Test User",
+ pmcs=[],
+ projects=[],
+ isMember=False,
+ isChair=False,
+ roleaccount=False,
+ metadata={},
+ )
+
+ util.write_quart_session_cookie(session_data)
+ return await web.redirect(root.index)
+
+
+@get.typed
+async def test_recheck_session(
+ _session: web.Public, _test_recheck_session: Literal["test/recheck-session"]
+) -> web.WerkzeugResponse:
+ """
+ URL: /test/recheck-session
+
+ Reset the last_account_check to epoch so the next request triggers a re-check.
+ """
+ if not config.get().ALLOW_TESTS:
+ return quart.abort(404)
+
+ import asfquart.session as asfquart_session
+
+ existing = await asfquart_session.read()
+ if existing is None:
+ raise base.ASFQuartException("No session to recheck", errorcode=400)
+
+ existing["metadata"]["last_account_check"] = 0
+ asfquart_session.write(existing)
+ return await web.redirect(root.index)
+
+
@get.typed
async def test_merge(
session: web.Committer,
diff --git a/atr/ldap.py b/atr/ldap.py
index e05fe99b..bbd4f21c 100644
--- a/atr/ldap.py
+++ b/atr/ldap.py
@@ -249,6 +249,8 @@ async def is_active(asf_uid: str) -> bool:
return True
if config.get().ALLOW_TESTS and (asf_uid == "test"):
return True
+ if config.get().ALLOW_TESTS and (asf_uid == "test-banned"):
+ return False
account = await account_lookup(asf_uid)
if account is None:
return False
diff --git a/atr/server.py b/atr/server.py
index 98c73b5b..1528a3f1 100644
--- a/atr/server.py
+++ b/atr/server.py
@@ -28,6 +28,7 @@ import pathlib
import queue
import stat
import sys
+import time
import urllib.parse
import uuid
from collections.abc import Iterable
@@ -54,6 +55,7 @@ import atr.db.interaction as interaction
import atr.filters as filters
import atr.form as form
import atr.jwtoken as jwtoken
+import atr.ldap as ldap
import atr.log as log
import atr.manager as manager
import atr.models.sql as sql
@@ -151,6 +153,7 @@ def _app_create_base(app_config: type[config.AppConfig]) -> base.QuartApp:
# Our AppConfig.SECRET_KEY is None since we no longer support that setting
asfquart_secret_key = app.secret_key
app.config.from_object(app_config)
+ app.cfg = app.config
app.secret_key = asfquart_secret_key
if not util.is_dev_environment():
@@ -451,40 +454,34 @@ def _app_setup_request_lifecycle(app: base.QuartApp) -> None:
await _reset_request_log_context()
@app.before_request
- async def validate_session_lifetime() -> None:
- """Enforce absolute maximum session lifetime per ASVS 7.3.2."""
+ async def validate_session() -> None:
+ """
+ Check account is still active and augment cookie with additional information
+ Note - absolute session max lifetime (MAX_SESSION_AGE) is handled by asfquart
+ """
session = await asfquart.session.read()
- if session is None:
+ if session is None or session.uid is None:
return
conf = config.get()
- max_lifetime_seconds = conf.ABSOLUTE_SESSION_MAX_SECONDS
- max_lifetime = datetime.timedelta(seconds=max_lifetime_seconds)
+ account_check_interval = conf.ACCOUNT_CHECK_INTERVAL
- # Check if session has a creation timestamp in metadata
- created_at_str = session.metadata.get("created_at")
+ # Check if session has a check timestamp in metadata
+ last_check = session.metadata.get("last_account_check")
+ current_time = time.time()
- if created_at_str is None:
- # First time seeing this session, record creation time
- session.metadata["created_at"] = datetime.datetime.now(datetime.UTC).isoformat()
+ if last_check is None or (current_time - last_check > account_check_interval):
+ # First time checking this session, record time
+ session.metadata["last_account_check"] = current_time
+ if not await ldap.is_active(str(session.uid)):
+ asfquart.session.clear()
+ raise base.ASFQuartException("Session expired", errorcode=401)
+
+ if last_check is None:
+ # If this was the first time we saw the session, make sure it contains the right data
pmcs = util.cookie_pmcs_or_session_pmcs(session)
session_data = util.session_cookie_data_from_client(session, pmcs)
util.write_quart_session_cookie(session_data)
- return
-
- # Parse the creation timestamp and check session age
- try:
- created_at = datetime.datetime.fromisoformat(created_at_str)
- except (ValueError, TypeError):
- # Invalid timestamp, treat as expired
- asfquart.session.clear()
- raise base.ASFQuartException("Session expired", errorcode=401)
-
- session_age = datetime.datetime.now(datetime.UTC) - created_at
-
- if session_age > max_lifetime:
- asfquart.session.clear()
- raise base.ASFQuartException("Session expired", errorcode=401)
@app.after_request
async def log_request(response: quart.Response) -> quart.Response:
diff --git a/playwright/test.py b/playwright/test.py
index 38bfd5eb..1ba110e6 100755
--- a/playwright/test.py
+++ b/playwright/test.py
@@ -142,8 +142,9 @@ def lifecycle_03_add_file(page: Page, credentials: Credentials, version_key: str
file_input_locator = page.locator('input[name="file_data"]')
expect(file_input_locator).to_be_visible()
- logging.info("Setting the input file to /run/tests/example.txt")
- file_input_locator.set_input_files("/run/tests/example.txt")
+ example_txt = os.path.join(os.getcwd(), "example.txt")
+ logging.info(f"Setting the input file to {example_txt}")
+ file_input_locator.set_input_files(example_txt)
logging.info("Locating and activating the add files button")
submit_button_locator = page.get_by_role("button", name="Add files")
@@ -557,6 +558,10 @@ def test_all(page: Page, credentials: Credentials, skip_slow: bool) -> None:
test_checks_06_targz,
test_checks_07_cache,
]
+ tests["session"] = [
+ test_session_01_banned_user_is_rejected,
+ test_session_02_recheck_allows_active_user,
+ ]
# Order between our tests must be preserved
# Insertion order is reliable since Python 3.6
@@ -758,8 +763,9 @@ def test_checks_07_cache(page: Page, credentials: Credentials) -> None:
file_input_locator = page.locator('input[name="file_data"]')
expect(file_input_locator).to_be_visible()
- logging.info("Setting the input file to /run/tests/example.txt")
- file_input_locator.set_input_files("/run/tests/example.txt")
+ example_txt = os.path.join(os.getcwd(), "example.txt")
+ logging.info(f"Setting the input file to {example_txt}")
+ file_input_locator.set_input_files(example_txt)
logging.info("Locating and activating the add files button")
submit_button_locator = page.get_by_role("button", name="Add files")
@@ -793,7 +799,7 @@ def test_checks_07_cache(page: Page, credentials: Credentials) -> None:
def test_openpgp_01_upload(page: Page, credentials: Credentials) -> None:
- for key_path in glob.glob("/run/tests/*.asc"):
+ for key_path in glob.glob(os.path.join(os.getcwd(), "*.asc")):
key_fingerprint_lower = os.path.basename(key_path).split(".")[0].lower()
key_fingerprint_upper = key_fingerprint_lower.upper()
break
@@ -1040,6 +1046,36 @@ def test_projects_03_add_project(page: Page, credentials: Credentials) -> None:
logging.info("Project title confirmed on view page")
+def test_session_01_banned_user_is_rejected(page: Page, credentials: Credentials) -> None:
+ logging.info("Navigating to test login as banned user")
+ go_to_path(page, "/test/login-banned", wait=False)
+ wait_for_path(page, "/")
+
+ logging.info("Verifying that the session expired error is shown")
+ error_locator = page.locator("h2.text-danger-emphasis:has-text('Session expired')")
+ expect(error_locator).to_be_visible()
+ logging.info("Session expired error confirmed for banned user")
+
+ logging.info("Clearing cookies to recover from banned session")
+ page.context.clear_cookies()
+
+ logging.info("Re-logging in as normal test user")
+ go_to_path(page, "/test/login", wait=False)
+ wait_for_path(page, "/")
+ logging.info("Banned user rejection test completed successfully")
+
+
+def test_session_02_recheck_allows_active_user(page: Page, credentials: Credentials) -> None:
+ logging.info("Navigating to recheck-session to reset last_account_check to epoch")
+ go_to_path(page, "/test/recheck-session", wait=False)
+ wait_for_path(page, "/")
+
+ logging.info("Verifying that active user passes the re-check (no error heading)")
+ error_locator = page.locator("h2.text-danger-emphasis")
+ expect(error_locator).not_to_be_visible()
+ logging.info("Active user recheck test completed successfully")
+
+
def test_ssh_01_add_key(page: Page, credentials: Credentials) -> None:
logging.info("Starting SSH key addition test")
go_to_path(page, "/keys")
@@ -1103,7 +1139,7 @@ def test_ssh_02_rsync_upload(page: Page, credentials: Credentials) -> None:
project_key = TEST_PROJECT
version_key = "0.2"
source_dir_rel = f"apache-{project_key}-{version_key}"
- source_dir_abs = f"/run/tests/{source_dir_rel}"
+ source_dir_abs = os.path.join(os.getcwd(), source_dir_rel)
file1 = f"apache-{project_key}-{version_key}.tar.gz"
file2 = f"{file1}.sha512"
diff --git a/tests/unit/test_ldap.py b/tests/unit/test_ldap.py
index 15f88441..a1260cee 100644
--- a/tests/unit/test_ldap.py
+++ b/tests/unit/test_ldap.py
@@ -16,6 +16,7 @@
# under the License.
import pathlib
+import unittest.mock as mock
from typing import TYPE_CHECKING
import pytest
@@ -33,10 +34,17 @@ class MockApp:
class MockConfig:
- def __init__(self, state_dir: pathlib.Path, ldap_bind_dn: str | None, ldap_bind_password: str | None):
- self.STATE_DIR = str(state_dir)
+ def __init__(
+ self,
+ state_dir: pathlib.Path | None = None,
+ ldap_bind_dn: str | None = None,
+ ldap_bind_password: str | None = None,
+ allow_tests: bool = False,
+ ):
+ self.STATE_DIR = str(state_dir) if state_dir else ""
self.LDAP_BIND_DN = ldap_bind_dn
self.LDAP_BIND_PASSWORD = ldap_bind_password
+ self.ALLOW_TESTS = allow_tests
@pytest.fixture
@@ -109,6 +117,66 @@ async def test_fetch_admin_users_returns_reasonable_count(ldap_configured: bool)
assert len(admins) < 100
+@pytest.mark.asyncio
+async def test_is_active_returns_true_when_ldap_not_configured(monkeypatch: "MonkeyPatch"):
+ monkeypatch.setattr("atr.ldap.get_bind_credentials", lambda: None)
+ assert await ldap.is_active("anyone") is True
+
+
+@pytest.mark.asyncio
+async def test_is_active_returns_true_for_test_user_when_tests_allowed(monkeypatch: "MonkeyPatch"):
+ monkeypatch.setattr("atr.ldap.get_bind_credentials", lambda: ("dn", "pw"))
+ monkeypatch.setattr("atr.config.get", lambda: MockConfig(allow_tests=True))
+ assert await ldap.is_active("test") is True
+
+
+@pytest.mark.asyncio
+async def test_is_active_returns_false_for_test_banned_user_when_tests_allowed(monkeypatch: "MonkeyPatch"):
+ monkeypatch.setattr("atr.ldap.get_bind_credentials", lambda: ("dn", "pw"))
+ monkeypatch.setattr("atr.config.get", lambda: MockConfig(allow_tests=True))
+ assert await ldap.is_active("test-banned") is False
+
+
+@pytest.mark.asyncio
+async def test_is_active_returns_false_when_account_not_found(monkeypatch: "MonkeyPatch"):
+ monkeypatch.setattr("atr.ldap.get_bind_credentials", lambda: ("dn", "pw"))
+ monkeypatch.setattr("atr.config.get", lambda: MockConfig(allow_tests=False))
+ monkeypatch.setattr("atr.ldap.account_lookup", mock.AsyncMock(return_value=None))
+ assert await ldap.is_active("ghost") is False
+
+
+@pytest.mark.asyncio
+async def test_is_active_returns_true_for_active_account(monkeypatch: "MonkeyPatch"):
+ account = ldap.Result(dn="uid=alice,ou=people,dc=apache,dc=org", uid=["alice"])
+ monkeypatch.setattr("atr.ldap.get_bind_credentials", lambda: ("dn", "pw"))
+ monkeypatch.setattr("atr.config.get", lambda: MockConfig(allow_tests=False))
+ monkeypatch.setattr("atr.ldap.account_lookup", mock.AsyncMock(return_value=account))
+ assert await ldap.is_active("alice") is True
+
+
+@pytest.mark.asyncio
+async def test_is_active_returns_false_for_banned_account(monkeypatch: "MonkeyPatch"):
+ account = ldap.Result.model_validate(
+ {"dn": "uid=bad,ou=people,dc=apache,dc=org", "uid": ["bad"], "asf-banned": ["yes"]}
+ )
+ monkeypatch.setattr("atr.ldap.get_bind_credentials", lambda: ("dn", "pw"))
+ monkeypatch.setattr("atr.config.get", lambda: MockConfig(allow_tests=False))
+ monkeypatch.setattr("atr.ldap.account_lookup", mock.AsyncMock(return_value=account))
+ assert await ldap.is_active("bad") is False
+
+
+def test_is_banned_returns_false_for_account_without_flag():
+ account = ldap.Result(dn="uid=alice,ou=people,dc=apache,dc=org", uid=["alice"])
+ assert ldap.is_banned(account) is False
+
+
+def test_is_banned_returns_true_for_account_with_flag():
+ account = ldap.Result.model_validate(
+ {"dn": "uid=bad,ou=people,dc=apache,dc=org", "uid": ["bad"], "asf-banned": ["yes"]}
+ )
+ assert ldap.is_banned(account) is True
+
+
def _skip_if_unavailable(ldap_configured: bool) -> None:
if not ldap_configured:
pytest.skip("LDAP not configured")
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org