asf-tooling commented on issue #873:
URL:
https://github.com/apache/tooling-trusted-releases/issues/873#issuecomment-4407542460
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@751c2146`
**Type:** `discussion` • **Classification:** `no_action` •
**Confidence:** `high`
**Application domain(s):** `authentication_authorization`, `infrastructure`
### Summary
Issue #873 is a tracking/parent issue for the broader session handling
refactor and principal cache rework to follow LDAP changes via pubsub. @sbp
noted they need to discuss with @Humbedooh about how much should be in ASFQuart
vs ATR. Significant work has already been completed — a database-backed session
store (migration 0065, created 2026-04-09, after issue filing) is fully
implemented, PubSub integration for LDAP updates already revokes credentials
for banned users, and comprehensive session tests exist. The remaining
architectural question is the ASFQuart/ATR boundary.
### Where this lives in the code today
#### `atr/sessions.py` — `Store` (lines 136-152)
_currently does this_
The database-backed session store is already implemented as part of this
refactor effort.
```python
class Store:
async def create(self, hsid: str, session_data: dict[str, Any]) -> None:
prepared = _prepare_session_data(session_data)
now = time.time()
user_session = sql.UserSession(sid_hash=hsid, cts=now, uts=now,
**prepared)
async with db.session() as data:
data.add(user_session)
await data.commit()
async def destroy(self, hsid: str) -> None:
async with db.session() as data:
statement =
sqlmodel.select(sql.UserSession).where(sql.UserSession.sid_hash == hsid)
result = await data.execute(statement)
user_session = result.scalar_one_or_none()
if user_session is not None:
await data.delete(user_session)
await data.commit()
```
#### `atr/ldap.py` — `handle_update` (lines 261-284)
_currently does this_
LDAP pubsub handling already revokes sessions and credentials when a user is
banned — part of the 'follow ldap changes via pubsub' goal.
```python
async def handle_update(payload: dict) -> None:
import atr.log as log
try:
parsed = PubSubPayload.model_validate(payload)
except schema.pydantic.ValidationError:
log.warning(f"Failed to parse LDAP pubsub payload with DN:
{payload.get('dn', '<missing>')}")
return
uid = _extract_uid_from_pubsub(parsed)
if uid is None:
log.debug(f"Ignoring LDAP pubsub event with no uid: {parsed.dn}")
return
was_banned = bool(parsed.old_attributes.asf_banned)
now_banned = bool(parsed.new_attributes.asf_banned)
if now_banned and (not was_banned):
log.info(f"LDAP pubsub: user {uid} has been deactivated")
log.auth_event("account_deactivated", uid)
await _revoke_all_credentials(uid, log)
elif was_banned and (not now_banned):
log.info(f"LDAP pubsub: user {uid} has been reactivated")
log.auth_event("account_reactivated", uid)
```
#### `atr/pubsub.py` — `PubSubListener` (lines 144-173)
_currently does this_
PubSub listener routes LDAP change events to ldap.handle_update,
implementing the pubsub-based LDAP change tracking described in the issue.
```python
class PubSubListener:
def __init__(
self,
svn_working_copy_root: os.PathLike | str,
url: str,
username: str,
password: str,
topics: str = "commit/svn,ldap",
) -> None:
self.svn_working_copy_root = pathlib.Path(svn_working_copy_root)
self.url = url
self.username = username
self.password = password
self.topics = topics
async def start(self) -> None:
...
try:
async for payload in listen(
full_url,
username=self.username,
password=self.password,
):
if (payload is None) or ("stillalive" in payload):
continue
if is_commit_payload(payload):
await commits.handle(payload, self.svn_working_copy_root)
elif is_ldap_payload(payload):
await ldap.handle_update(payload)
```
#### `atr/principal.py` — `Cache` (lines 198-203)
_needs modification_
The principal cache uses time-based polling TTL (600s). The issue implies
this should be refreshed reactively via pubsub LDAP change events rather than
just by polling.
```python
class Cache:
def __init__(self, cache_for_at_most_seconds: int = 600):
self.cache_for_at_most_seconds = cache_for_at_most_seconds
self.last_refreshed: dict[str, int | None] = {}
self.member_of: dict[str, frozenset[str]] = {}
self.participant_of: dict[str, frozenset[str]] = {}
```
#### `atr/cache.py` — `admins_refresh_loop` (lines 62-71)
_needs modification_
Admin cache currently polls every 3631 seconds. Could be enhanced to also
refresh reactively from pubsub LDAP group membership changes.
```python
async def admins_refresh_loop() -> None:
while True:
await asyncio.sleep(ADMINS_POLL_INTERVAL_SECONDS)
try:
users = await ldap.fetch_admin_users()
await admins_save_to_file(users)
_admins_update_app_extensions(users)
log.info(f"Admin users cache refreshed: {len(users)} users")
except Exception as e:
log.warning(f"Admin refresh failed: {e}")
```
#### `migrations/versions/0065_2026.04.09_56a8188f.py` — `upgrade` (lines
20-41)
_currently does this_
Database migration for the usersession table, created after this issue was
filed, demonstrating work already completed as part of this refactor.
```python
def upgrade() -> None:
op.create_table(
"usersession",
sa.Column("sid_hash", sa.String(), nullable=False),
sa.Column("uid", sa.String(), nullable=False),
sa.Column("dn", sa.String(), nullable=True),
sa.Column("fullname", sa.String(), nullable=True),
sa.Column("email", sa.String(), nullable=True),
sa.Column("is_member", sa.Boolean(), nullable=False),
sa.Column("is_chair", sa.Boolean(), nullable=False),
sa.Column("is_root", sa.Boolean(), nullable=False),
sa.Column("committees", sa.JSON(), nullable=False),
sa.Column("projects", sa.JSON(), nullable=False),
sa.Column("mfa", sa.Boolean(), nullable=False),
sa.Column("is_role", sa.Boolean(), nullable=False),
sa.Column("admin_uid", sa.String(), nullable=True),
sa.Column("last_account_check", sa.Float(), nullable=True),
sa.Column("downgrade_admin_to_user", sa.Boolean(), nullable=False),
sa.Column("cts", sa.Float(), nullable=False),
sa.Column("uts", sa.Float(), nullable=False),
sa.PrimaryKeyConstraint("sid_hash", name=op.f("pk_usersession")),
)
```
### Proposed approach
This is a tracking/parent issue for an ongoing refactoring effort.
Significant work has already been completed: database-backed sessions (Store
class + migration 0065), PubSub integration for LDAP ban/unban events, and
comprehensive session lifecycle tests. The remaining open work likely involves:
(1) resolving the ASFQuart vs ATR boundary question raised by @sbp, (2)
enhancing the principal Cache to invalidate/refresh reactively via pubsub when
LDAP group memberships change (not just when users are banned), and (3)
potentially moving the admin cache to also react to pubsub events for group
membership changes rather than polling every ~3600 seconds. No single diff is
appropriate since this is a discussion/tracking issue awaiting architectural
decisions.
### Open questions
- What is the agreed boundary between ASFQuart and ATR for session handling?
(@sbp planned to discuss with @Humbedooh)
- Should the principal Cache (atr/principal.py:Cache) be invalidated
reactively when pubsub delivers LDAP group membership changes (not just bans)?
- Should the admin cache (atr/cache.py) switch from polling to reactive
pubsub-based refresh for group membership changes?
- Are there child issues already filed for the remaining work items under
this parent?
_The agent reviewed this issue and is not proposing patches in this run.
Review the existing-code citations and open questions above before deciding
next steps._
### Files examined
- `atr/cache.py`
- `atr/ldap.py`
- `atr/pubsub.py`
- `tests/unit/test_sessions.py`
- `atr/principal.py`
- `atr/sessions.py`
- `atr/docs/authorization-security.md`
- `migrations/versions/0065_2026.04.09_56a8188f.py`
### Related issues
This issue appears related to: #882.
_Both involve LDAP caching and session handling improvements, with #882
depending on #873's pubsub notification mechanism_
---
*Draft from a triage agent. A human reviewer should validate before merging
any change. The agent did not run tests or verify diffs apply.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
