This is an automated email from the ASF dual-hosted git repository. akm pushed a commit to branch invalidate-pats-598 in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
commit 28e65be91c48ce919ad30eeb6fb1ae36ddfd9bf6 Author: Andrew K. Musselman <[email protected]> AuthorDate: Thu Feb 12 15:36:06 2026 -0800 Invalidate PATs; fixes #598 --- atr/admin/__init__.py | 75 ++++++++ atr/admin/templates/revoke-user-tokens.html | 48 +++++ atr/docs/authentication-security.md | 10 +- atr/docs/authorization-security.md | 11 ++ atr/server.py | 9 + atr/storage/__init__.py | 1 + atr/storage/writers/tokens.py | 30 +++ atr/templates/includes/topnav.html | 5 + atr/token_cleanup.py | 92 +++++++++ tests/e2e/admin/__init__.py | 16 ++ tests/e2e/admin/conftest.py | 53 ++++++ tests/e2e/admin/helpers.py | 41 ++++ tests/e2e/admin/test_revoke_tokens.py | 137 ++++++++++++++ tests/unit/test_token_cleanup.py | 283 ++++++++++++++++++++++++++++ 14 files changed, 808 insertions(+), 3 deletions(-) diff --git a/atr/admin/__init__.py b/atr/admin/__init__.py index 23bf9c52..d3e24c5a 100644 --- a/atr/admin/__init__.py +++ b/atr/admin/__init__.py @@ -88,6 +88,11 @@ class LdapLookupForm(form.Form): email: str = form.label("Email address (optional)", "Enter email address, e.g. [email protected]") +class RevokeUserTokensForm(form.Form): + asf_uid: str = form.label("ASF UID", "Enter the ASF UID whose tokens should be revoked.") + confirm_revoke: Literal["REVOKE"] = form.label("Confirmation", "Type REVOKE to confirm.") + + class SessionDataCommon(NamedTuple): uid: str fullname: str @@ -699,6 +704,76 @@ async def projects_update_post(session: web.Committer) -> str | web.WerkzeugResp }, 200 [email protected]("/revoke-user-tokens") +async def revoke_user_tokens_get(session: web.Committer) -> str: + # Show current PAT counts per user for context + token_counts: list[tuple[str, int]] = [] + async with db.session() as data: + stmt = ( + sqlmodel.select( + sql.PersonalAccessToken.asfuid, + sqlmodel.func.count(), + ) + .group_by(sql.PersonalAccessToken.asfuid) + .order_by(sql.PersonalAccessToken.asfuid) + ) + rows = await data.execute_query(stmt) + token_counts = [(row[0], row[1]) for row in rows] + + rendered_form = form.render( + model_cls=RevokeUserTokensForm, + submit_label="Revoke all tokens", + ) + return await template.render( + "revoke-user-tokens.html", + form=rendered_form, + token_counts=token_counts, + revocation_result=None, + ) + + [email protected]("/revoke-user-tokens") [email protected](RevokeUserTokensForm) +async def revoke_user_tokens_post( + session: web.Committer, revoke_form: RevokeUserTokensForm +) -> str | web.WerkzeugResponse: + target_uid = revoke_form.asf_uid.strip() + + async with storage.write(session) as write: + wafa = write.as_foundation_admin("infrastructure") + count = await wafa.tokens.revoke_all_user_tokens(target_uid) + + if count > 0: + await quart.flash(f"Revoked {count} token(s) for {target_uid}.", "success") + else: + await quart.flash(f"No tokens found for {target_uid}.", "info") + + # Re-render the page with updated counts + token_counts: list[tuple[str, int]] = [] + async with db.session() as data: + stmt = ( + sqlmodel.select( + sql.PersonalAccessToken.asfuid, + sqlmodel.func.count(), + ) + .group_by(sql.PersonalAccessToken.asfuid) + .order_by(sql.PersonalAccessToken.asfuid) + ) + rows = await data.execute_query(stmt) + token_counts = [(row[0], row[1]) for row in rows] + + rendered_form = form.render( + model_cls=RevokeUserTokensForm, + submit_label="Revoke all tokens", + ) + return await template.render( + "revoke-user-tokens.html", + form=rendered_form, + token_counts=token_counts, + revocation_result={"uid": target_uid, "count": count}, + ) + + @admin.get("/task-times/<project_name>/<version_name>/<revision_number>") async def task_times( session: web.Committer, project_name: str, version_name: str, revision_number: str diff --git a/atr/admin/templates/revoke-user-tokens.html b/atr/admin/templates/revoke-user-tokens.html new file mode 100644 index 00000000..441789f8 --- /dev/null +++ b/atr/admin/templates/revoke-user-tokens.html @@ -0,0 +1,48 @@ +{% extends "layouts/base-admin.html" %} + +{%- block title -%}Revoke user tokens ~ ATR{%- endblock title -%} + +{%- block description -%}Revoke all Personal Access Tokens for a user.{%- endblock description -%} + +{% block content %} + <h1>Revoke user tokens</h1> + <p>Revoke all Personal Access Tokens (PATs) for a user account. Use this when an account + is being disabled or when immediate token revocation is needed.</p> + + <div class="card mb-4"> + <div class="card-header"> + <h5 class="mb-0">Revoke tokens</h5> + </div> + <div class="card-body"> + {{ form }} + </div> + </div> + + {% if token_counts %} + <div class="card"> + <div class="card-header"> + <h5 class="mb-0">Users with active tokens</h5> + </div> + <div class="card-body"> + <table class="table table-sm table-striped table-bordered"> + <thead> + <tr> + <th>ASF UID</th> + <th>Token count</th> + </tr> + </thead> + <tbody> + {% for uid, count in token_counts %} + <tr> + <td><code>{{ uid }}</code></td> + <td>{{ count }}</td> + </tr> + {% endfor %} + </tbody> + </table> + </div> + </div> + {% else %} + <div class="alert alert-info" role="alert">No users currently have active tokens.</div> + {% endif %} +{% endblock content %} diff --git a/atr/docs/authentication-security.md b/atr/docs/authentication-security.md index 79827c95..6b249490 100644 --- a/atr/docs/authentication-security.md +++ b/atr/docs/authentication-security.md @@ -69,12 +69,13 @@ Committers can obtain PATs from the `/tokens` page on the ATR website. PATs have * **Validity**: 180 days from creation, while LDAP account is still active * **Storage**: ATR stores only SHA3-256 hashes, never the plaintext PAT -* **Revocation**: Users can revoke their own PATs at any time; admins can revoke any PAT +* **Revocation**: Users can revoke their own PATs at any time; admins can revoke all PATs for any user via the admin "Revoke user tokens" page +* **Automatic cleanup**: A background loop ([`token_cleanup`](/ref/atr/token_cleanup.py)) polls LDAP approximately every hour and automatically revokes all PATs belonging to banned or deleted accounts * **Purpose**: PATs are used solely to obtain JWTs; they cannot be used directly for API access Only authenticated committers (signed in via ASF OAuth) can create PATs. Each user can have multiple active PATs. -PATs are rejected if the user who created them has been removed from LDAP. +PATs are rejected if the user who created them has been removed from LDAP. This is enforced at three layers: the JWT exchange endpoint checks LDAP status before issuing a JWT (immediate), a background cleanup loop revokes PATs for banned or deleted accounts (within ~1 hour), and administrators can revoke PATs immediately through the admin interface. ### JSON Web Tokens (JWTs) @@ -139,7 +140,8 @@ For web users, authentication happens once via ASF OAuth, and the session persis ### Personal Access Tokens * Stored as SHA3-256 hashes -* Can be revoked immediately by the user +* Can be revoked immediately by the user or in bulk by administrators +* Automatically revoked when the owning account is banned or deleted in LDAP * Limited purpose (only for JWT issuance) reduces impact of compromise * Long validity (180 days) balanced by easy revocation @@ -162,3 +164,5 @@ Tokens must be protected by the user at all times: * [`principal.py`](/ref/atr/principal.py) - Session caching and authorization data * [`jwtoken.py`](/ref/atr/jwtoken.py) - JWT creation, verification, and decorators +* [`token_cleanup.py`](/ref/atr/token_cleanup.py) - Automated PAT revocation for banned/deleted accounts +* [`storage/writers/tokens.py`](/ref/atr/storage/writers/tokens.py) - Token creation, deletion, and admin revocation diff --git a/atr/docs/authorization-security.md b/atr/docs/authorization-security.md index 617ae3f4..6d58b2eb 100644 --- a/atr/docs/authorization-security.md +++ b/atr/docs/authorization-security.md @@ -122,10 +122,21 @@ Token operations apply to the authenticated user: * Allowed for: The token owner, or administrators * Constraint: Users can only revoke their own tokens (unless admin) +**Revoke all tokens for a user (admin)**: + +* Allowed for: ATR administrators only +* Interface: Admin "Revoke user tokens" page +* Constraint: Requires typing "REVOKE" as confirmation + +**Automated token revocation**: + +* The [`token_cleanup`](/ref/atr/token_cleanup.py) background loop runs approximately every hour, queries LDAP for all users who hold PATs, and revokes tokens belonging to accounts that are banned or deleted. This runs without human intervention and is audit logged. + **Exchange PAT for JWT**: * Allowed for: Anyone with a valid PAT * Note: This is an unauthenticated endpoint; the PAT serves as the credential +* Constraint: LDAP is checked at exchange time; banned or deleted accounts are rejected even if the PAT has not yet been cleaned up ## Access control for check ignores diff --git a/atr/server.py b/atr/server.py index c09357cf..cf178888 100644 --- a/atr/server.py +++ b/atr/server.py @@ -61,6 +61,7 @@ import atr.ssh as ssh import atr.svn.pubsub as pubsub import atr.tasks as tasks import atr.template as template +import atr.token_cleanup as token_cleanup import atr.user as user import atr.util as util @@ -269,6 +270,9 @@ def _app_setup_lifecycle(app: base.QuartApp, app_config: type[config.AppConfig]) admins_task = asyncio.create_task(cache.admins_refresh_loop()) app.extensions["admins_task"] = admins_task + pat_cleanup_task = asyncio.create_task(token_cleanup.cleanup_loop()) + app.extensions["pat_cleanup_task"] = pat_cleanup_task + worker_manager = manager.get_worker_manager() await worker_manager.start() @@ -312,6 +316,11 @@ def _app_setup_lifecycle(app: base.QuartApp, app_config: type[config.AppConfig]) with contextlib.suppress(asyncio.CancelledError): await task + if task := app.extensions.get("pat_cleanup_task"): + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + await db.shutdown_database() await _app_shutdown_log_listeners(app) diff --git a/atr/storage/__init__.py b/atr/storage/__init__.py index cf159412..7c758a94 100644 --- a/atr/storage/__init__.py +++ b/atr/storage/__init__.py @@ -247,6 +247,7 @@ class WriteAsFoundationAdmin(WriteAsCommitteeMember): self.__committee_name = committee_name self.keys = writers.keys.FoundationAdmin(write, self, data, committee_name) self.release = writers.release.FoundationAdmin(write, self, data, committee_name) + self.tokens = writers.tokens.FoundationAdmin(write, self, data, committee_name) @property def asf_uid(self) -> str: diff --git a/atr/storage/writers/tokens.py b/atr/storage/writers/tokens.py index d47cefa5..b29bd11f 100644 --- a/atr/storage/writers/tokens.py +++ b/atr/storage/writers/tokens.py @@ -169,3 +169,33 @@ class CommitteeMember(CommitteeParticipant): raise storage.AccessError("Not authorized") self.__asf_uid = asf_uid self.__committee_name = committee_name + + +class FoundationAdmin: + def __init__( + self, + write: storage.Write, + write_as: storage.WriteAsFoundationAdmin, + data: db.Session, + committee_name: str, + ): + self.__write = write + self.__write_as = write_as + self.__data = data + + async def revoke_all_user_tokens(self, target_asf_uid: str) -> int: + """Revoke all PATs for a specified user. Returns count of revoked tokens.""" + tokens = await self.__data.query_all( + sqlmodel.select(sql.PersonalAccessToken).where(sql.PersonalAccessToken.asfuid == target_asf_uid) + ) + count = len(tokens) + for token in tokens: + await self.__data.delete(token) + + if count > 0: + await self.__data.commit() + self.__write_as.append_to_audit_log( + target_asf_uid=target_asf_uid, + tokens_revoked=count, + ) + return count diff --git a/atr/templates/includes/topnav.html b/atr/templates/includes/topnav.html index d386dfd7..2a5b056c 100644 --- a/atr/templates/includes/topnav.html +++ b/atr/templates/includes/topnav.html @@ -216,6 +216,11 @@ href="{{ as_url(admin.ldap_get) }}" {% if request.endpoint == 'atr_admin_ldap_get' %}class="active"{% endif %}><i class="bi bi-person-plus"></i> LDAP search</a> </li> + <li> + <a class="dropdown-item" + href="{{ as_url(admin.revoke_user_tokens_get) }}" + {% if request.endpoint == 'atr_admin_revoke_user_tokens_get' %}class="active"{% endif %}><i class="bi bi-shield-x"></i> Revoke user tokens</a> + </li> <li> <a class="dropdown-item" href="{{ as_url(admin.toggle_view_get) }}" diff --git a/atr/token_cleanup.py b/atr/token_cleanup.py new file mode 100644 index 00000000..1159d8a4 --- /dev/null +++ b/atr/token_cleanup.py @@ -0,0 +1,92 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Periodic cleanup of Personal Access Tokens for banned or deleted accounts.""" + +import asyncio +from typing import Final + +import sqlalchemy +import sqlmodel + +import atr.db as db +import atr.ldap as ldap +import atr.log as log +import atr.models.sql as sql +import atr.storage as storage + +# ~1 hour, deliberately offset from the admin poll interval (3631s) +# to avoid simultaneous LDAP request spikes +POLL_INTERVAL_SECONDS: Final[int] = 3617 + + +async def cleanup_loop() -> None: + """Periodically revoke PATs belonging to banned or deleted LDAP accounts.""" + while True: + await asyncio.sleep(POLL_INTERVAL_SECONDS) + try: + await revoke_pats_for_banned_users() + except Exception as e: + log.warning(f"PAT banned-user cleanup failed: {e}") + + +async def revoke_pats_for_banned_users() -> int: + """Check all PAT-holding users against LDAP and revoke tokens for banned/deleted accounts. + + Returns the total number of tokens revoked. + """ + # Step 1: Get distinct UIDs that have PATs + async with db.session() as data: + stmt = sqlmodel.select(sql.PersonalAccessToken.asfuid).distinct() + rows = await data.execute_query(stmt) + uids_with_pats = [row[0] for row in rows] + + if not uids_with_pats: + return 0 + + # Step 2: Check each against LDAP, revoke if banned/deleted + revoked_total = 0 + for uid in uids_with_pats: + try: + account = await ldap.account_lookup(uid) + if (account is not None) and (not ldap.is_banned(account)): + continue # Account is active, skip + + # Account is gone or banned — delete all their tokens + async with db.session() as data: + delete_stmt = sqlalchemy.delete(sql.PersonalAccessToken).where(sql.PersonalAccessToken.asfuid == uid) + result = await data.execute_query(delete_stmt) + await data.commit() + count: int = getattr(result, "rowcount", 0) + + if count > 0: + storage.audit( + target_asf_uid=uid, + tokens_revoked=count, + reason="account_banned_or_deleted", + source="pat_cleanup_loop", + ) + log.info(f"Auto-revoked {count} PAT(s) for banned/deleted user {uid}") + revoked_total += count + + except Exception as e: + log.warning(f"PAT cleanup: failed to check/revoke for {uid}: {e}") + + if revoked_total > 0: + log.info(f"PAT cleanup cycle complete: revoked {revoked_total} total token(s)") + + return revoked_total diff --git a/tests/e2e/admin/__init__.py b/tests/e2e/admin/__init__.py new file mode 100644 index 00000000..13a83393 --- /dev/null +++ b/tests/e2e/admin/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/tests/e2e/admin/conftest.py b/tests/e2e/admin/conftest.py new file mode 100644 index 00000000..7eecb75d --- /dev/null +++ b/tests/e2e/admin/conftest.py @@ -0,0 +1,53 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import e2e.admin.helpers as admin_helpers +import e2e.helpers as helpers +import pytest + +if TYPE_CHECKING: + from collections.abc import Generator + + from playwright.sync_api import Page + + [email protected] +def page_revoke_tokens(page: Page) -> Generator[Page]: + helpers.log_in(page) + helpers.visit(page, admin_helpers.REVOKE_TOKENS_PATH) + yield page + + [email protected] +def page_revoke_tokens_with_token(page: Page) -> Generator[Page]: + """Log in, create a test token, then navigate to the revoke page.""" + helpers.log_in(page) + # Create a token first + helpers.visit(page, admin_helpers.TOKENS_PATH) + admin_helpers.create_token(page, admin_helpers.TOKEN_LABEL_FOR_TESTING) + # Navigate to admin revoke page + helpers.visit(page, admin_helpers.REVOKE_TOKENS_PATH) + yield page + # Cleanup: delete the test token if it still exists + helpers.visit(page, admin_helpers.TOKENS_PATH) + from e2e.tokens.helpers import delete_token_by_label + + delete_token_by_label(page, admin_helpers.TOKEN_LABEL_FOR_TESTING) diff --git a/tests/e2e/admin/helpers.py b/tests/e2e/admin/helpers.py new file mode 100644 index 00000000..13c17d25 --- /dev/null +++ b/tests/e2e/admin/helpers.py @@ -0,0 +1,41 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Final + +from playwright.sync_api import Page + +REVOKE_TOKENS_PATH: Final[str] = "/admin/revoke-user-tokens" +TOKENS_PATH: Final[str] = "/tokens" +TOKEN_LABEL_FOR_TESTING: Final[str] = "e2e-revoke-test-token" + + +def create_token(page: Page, label: str) -> None: + """Create a PAT via the tokens page.""" + label_input = page.locator('input[name="label"]') + label_input.fill(label) + page.get_by_role("button", name="Generate token").click() + page.wait_for_load_state() + + +def get_token_count_for_user(page: Page, uid: str) -> int: + """Read the token count for a user from the revoke tokens page table.""" + row = page.locator(f'tr:has(td code:text-is("{uid}"))') + if row.count() == 0: + return 0 + count_cell = row.locator("td").nth(1) + return int(count_cell.inner_text()) diff --git a/tests/e2e/admin/test_revoke_tokens.py b/tests/e2e/admin/test_revoke_tokens.py new file mode 100644 index 00000000..782c958e --- /dev/null +++ b/tests/e2e/admin/test_revoke_tokens.py @@ -0,0 +1,137 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import e2e.admin.helpers as admin_helpers +import e2e.helpers as helpers +from playwright.sync_api import Page, expect + + +def test_revoke_tokens_page_loads(page_revoke_tokens: Page) -> None: + expect(page_revoke_tokens).to_have_title("Revoke user tokens ~ ATR") + + +def test_revoke_tokens_page_has_heading(page_revoke_tokens: Page) -> None: + heading = page_revoke_tokens.get_by_role("heading", name="Revoke user tokens") + expect(heading).to_be_visible() + + +def test_revoke_tokens_page_has_uid_input(page_revoke_tokens: Page) -> None: + uid_input = page_revoke_tokens.locator('input[name="asf_uid"]') + expect(uid_input).to_be_visible() + + +def test_revoke_tokens_page_has_confirmation_input(page_revoke_tokens: Page) -> None: + confirm_input = page_revoke_tokens.locator('input[name="confirm_revoke"]') + expect(confirm_input).to_be_visible() + + +def test_revoke_tokens_page_has_submit_button(page_revoke_tokens: Page) -> None: + button = page_revoke_tokens.get_by_role("button", name="Revoke all tokens") + expect(button).to_be_visible() + + +def test_revoke_tokens_page_shows_token_counts_table( + page_revoke_tokens_with_token: Page, +) -> None: + page = page_revoke_tokens_with_token + table = page.locator("table") + expect(table).to_be_visible() + # The test user should appear in the table + test_user_row = page.locator('tr:has(td code:text-is("test"))') + expect(test_user_row).to_be_visible() + + +def test_revoke_tokens_page_shows_no_tokens_message_when_empty( + page_revoke_tokens: Page, +) -> None: + # This test assumes no PATs exist for any user + # If the table is not visible, the info alert should be + page = page_revoke_tokens + table = page.locator("table.table-striped") + info_alert = page.locator('.alert-info:has-text("No users currently have active tokens")') + # One of these should be visible + expect(table.or_(info_alert)).to_be_visible() + + +def test_revoke_shows_error_for_wrong_confirmation(page_revoke_tokens: Page) -> None: + page = page_revoke_tokens + page.locator('input[name="asf_uid"]').fill("test") + page.locator('input[name="confirm_revoke"]').fill("WRONG") + page.get_by_role("button", name="Revoke all tokens").click() + page.wait_for_load_state() + + error_message = page.locator(".flash-message.flash-error") + expect(error_message).to_be_visible() + + +def test_revoke_nonexistent_user_shows_info(page_revoke_tokens: Page) -> None: + page = page_revoke_tokens + page.locator('input[name="asf_uid"]').fill("nonexistent_user_abc123") + page.locator('input[name="confirm_revoke"]').fill("REVOKE") + page.get_by_role("button", name="Revoke all tokens").click() + page.wait_for_load_state() + + info_message = page.locator('.flash-message:has-text("No tokens found")') + expect(info_message).to_be_visible() + + +def test_revoke_deletes_tokens_and_shows_success( + page_revoke_tokens_with_token: Page, +) -> None: + page = page_revoke_tokens_with_token + + # Verify the test user has tokens before revocation + token_count = admin_helpers.get_token_count_for_user(page, "test") + assert token_count > 0 + + # Revoke all tokens for the test user + page.locator('input[name="asf_uid"]').fill("test") + page.locator('input[name="confirm_revoke"]').fill("REVOKE") + page.get_by_role("button", name="Revoke all tokens").click() + page.wait_for_load_state() + + # Should see success message + success_message = page.locator('.flash-message.flash-success:has-text("Revoked")') + expect(success_message).to_be_visible() + + # The test user should no longer appear in the table (or have 0 tokens) + test_user_row = page.locator('tr:has(td code:text-is("test"))') + expect(test_user_row).to_have_count(0) + + +def test_revoke_tokens_removes_tokens_from_user_view( + page_revoke_tokens_with_token: Page, +) -> None: + page = page_revoke_tokens_with_token + + # Revoke + page.locator('input[name="asf_uid"]').fill("test") + page.locator('input[name="confirm_revoke"]').fill("REVOKE") + page.get_by_role("button", name="Revoke all tokens").click() + page.wait_for_load_state() + + # Navigate to the user tokens page and verify tokens are gone + helpers.visit(page, admin_helpers.TOKENS_PATH) + token_row = page.locator(f'tr:has(td:text-is("{admin_helpers.TOKEN_LABEL_FOR_TESTING}"))') + expect(token_row).to_have_count(0) + + +def test_revoke_tokens_nav_link_exists(page_revoke_tokens: Page) -> None: + """The admin dropdown should contain a 'Revoke user tokens' link.""" + page = page_revoke_tokens + nav_link = page.locator('a.dropdown-item:has-text("Revoke user tokens")') + expect(nav_link).to_have_count(1) diff --git a/tests/unit/test_token_cleanup.py b/tests/unit/test_token_cleanup.py new file mode 100644 index 00000000..3ac45870 --- /dev/null +++ b/tests/unit/test_token_cleanup.py @@ -0,0 +1,283 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for periodic PAT cleanup of banned/deleted accounts.""" + +from typing import TYPE_CHECKING + +import pytest + +import atr.token_cleanup as token_cleanup + +if TYPE_CHECKING: + from pytest import MonkeyPatch + + +class MockDBSession: + """Minimal mock for db.Session used in PAT cleanup tests.""" + + def __init__(self, pat_uids: list[str] | None = None, delete_rowcount: int = 0): + self._pat_uids = pat_uids or [] + self._delete_rowcount = delete_rowcount + self._committed = False + self._executed_stmts: list[object] = [] + + async def execute_query(self, stmt: object) -> "MockResult": + self._executed_stmts.append(stmt) + # SELECT returns uid rows, DELETE returns rowcount + stmt_type = str(type(stmt)).lower() + if "select" in stmt_type: + return MockResult(rows=[(uid,) for uid in self._pat_uids]) + return MockResult(rowcount=self._delete_rowcount) + + async def commit(self) -> None: + self._committed = True + + async def __aenter__(self) -> "MockDBSession": + return self + + async def __aexit__(self, *args: object) -> None: + pass + + +class MockResult: + def __init__(self, rows: list[tuple[str]] | None = None, rowcount: int = 0): + self._rows = rows or [] + self.rowcount = rowcount + + def __iter__(self): + return iter(self._rows) + + [email protected] +async def test_cleanup_does_nothing_when_no_pats_exist(monkeypatch: "MonkeyPatch") -> None: + """When no users have PATs, the cleanup should not call LDAP at all.""" + lookup_called = False + + async def mock_account_lookup(uid: str) -> dict[str, str]: + nonlocal lookup_called + lookup_called = True + return {"uid": uid} + + monkeypatch.setattr("atr.token_cleanup.ldap.account_lookup", mock_account_lookup) + + mock_session = MockDBSession(pat_uids=[]) + monkeypatch.setattr("atr.token_cleanup.db.session", lambda: mock_session) + + revoked = await token_cleanup.revoke_pats_for_banned_users() + + assert revoked == 0 + assert not lookup_called + + [email protected] +async def test_cleanup_skips_active_accounts(monkeypatch: "MonkeyPatch") -> None: + """Active LDAP accounts should not have their PATs revoked.""" + lookup_calls: list[str] = [] + + async def mock_account_lookup(uid: str) -> dict[str, str]: + lookup_calls.append(uid) + return {"uid": uid, "asf-banned": "no"} + + monkeypatch.setattr("atr.token_cleanup.ldap.account_lookup", mock_account_lookup) + monkeypatch.setattr("atr.token_cleanup.ldap.is_banned", lambda account: False) + monkeypatch.setattr("atr.token_cleanup.storage.audit", lambda **kwargs: None) + + mock_session = MockDBSession(pat_uids=["active_user"]) + monkeypatch.setattr("atr.token_cleanup.db.session", lambda: mock_session) + + revoked = await token_cleanup.revoke_pats_for_banned_users() + + assert revoked == 0 + assert "active_user" in lookup_calls + # Only the SELECT should have been executed, no DELETE + assert len(mock_session._executed_stmts) == 1 + assert not mock_session._committed + + [email protected] +async def test_cleanup_revokes_for_banned_account(monkeypatch: "MonkeyPatch") -> None: + """Banned LDAP accounts should have all PATs deleted.""" + audit_calls: list[dict] = [] + + async def mock_account_lookup(uid: str) -> dict[str, str | list[str]]: + return {"uid": [uid], "asf-banned": "yes"} + + monkeypatch.setattr("atr.token_cleanup.ldap.account_lookup", mock_account_lookup) + monkeypatch.setattr("atr.token_cleanup.ldap.is_banned", lambda account: True) + monkeypatch.setattr("atr.token_cleanup.storage.audit", lambda **kwargs: audit_calls.append(kwargs)) + + # Two sessions: first returns UIDs, second handles the DELETE + call_count = 0 + select_session = MockDBSession(pat_uids=["banned_user"]) + delete_session = MockDBSession(delete_rowcount=3) + + def mock_db_session(): + nonlocal call_count + call_count += 1 + if call_count == 1: + return select_session + return delete_session + + monkeypatch.setattr("atr.token_cleanup.db.session", mock_db_session) + + revoked = await token_cleanup.revoke_pats_for_banned_users() + + assert revoked == 3 + assert delete_session._committed + assert len(audit_calls) == 1 + assert audit_calls[0]["target_asf_uid"] == "banned_user" + assert audit_calls[0]["tokens_revoked"] == 3 + assert audit_calls[0]["reason"] == "account_banned_or_deleted" + + [email protected] +async def test_cleanup_revokes_for_deleted_account(monkeypatch: "MonkeyPatch") -> None: + """Accounts not found in LDAP (returns None) should have PATs deleted.""" + audit_calls: list[dict] = [] + + async def mock_account_lookup(uid: str) -> None: + return None # Account doesn't exist + + monkeypatch.setattr("atr.token_cleanup.ldap.account_lookup", mock_account_lookup) + monkeypatch.setattr("atr.token_cleanup.storage.audit", lambda **kwargs: audit_calls.append(kwargs)) + + call_count = 0 + select_session = MockDBSession(pat_uids=["deleted_user"]) + delete_session = MockDBSession(delete_rowcount=2) + + def mock_db_session(): + nonlocal call_count + call_count += 1 + if call_count == 1: + return select_session + return delete_session + + monkeypatch.setattr("atr.token_cleanup.db.session", mock_db_session) + + revoked = await token_cleanup.revoke_pats_for_banned_users() + + assert revoked == 2 + assert delete_session._committed + assert len(audit_calls) == 1 + assert audit_calls[0]["target_asf_uid"] == "deleted_user" + assert audit_calls[0]["tokens_revoked"] == 2 + + [email protected] +async def test_cleanup_continues_after_single_ldap_failure(monkeypatch: "MonkeyPatch") -> None: + """One LDAP failure should not prevent cleanup of other users.""" + audit_calls: list[dict] = [] + lookup_calls: list[str] = [] + + async def mock_account_lookup(uid: str) -> dict[str, str | list[str]] | None: + lookup_calls.append(uid) + if uid == "failing_user": + raise ConnectionError("LDAP timeout") + return None # Other users are "deleted" + + monkeypatch.setattr("atr.token_cleanup.ldap.account_lookup", mock_account_lookup) + monkeypatch.setattr("atr.token_cleanup.storage.audit", lambda **kwargs: audit_calls.append(kwargs)) + + call_count = 0 + select_session = MockDBSession(pat_uids=["failing_user", "deleted_user"]) + delete_session = MockDBSession(delete_rowcount=1) + + def mock_db_session(): + nonlocal call_count + call_count += 1 + if call_count == 1: + return select_session + return delete_session + + monkeypatch.setattr("atr.token_cleanup.db.session", mock_db_session) + + revoked = await token_cleanup.revoke_pats_for_banned_users() + + assert revoked == 1 + # Both users should have been checked + assert "failing_user" in lookup_calls + assert "deleted_user" in lookup_calls + # Only the non-failing user should have been cleaned up + assert len(audit_calls) == 1 + assert audit_calls[0]["target_asf_uid"] == "deleted_user" + + [email protected] +async def test_cleanup_does_not_audit_when_zero_tokens_deleted(monkeypatch: "MonkeyPatch") -> None: + """If a banned user has 0 tokens (race condition), no audit entry should be written.""" + audit_calls: list[dict] = [] + + async def mock_account_lookup(uid: str) -> None: + return None + + monkeypatch.setattr("atr.token_cleanup.ldap.account_lookup", mock_account_lookup) + monkeypatch.setattr("atr.token_cleanup.storage.audit", lambda **kwargs: audit_calls.append(kwargs)) + + call_count = 0 + select_session = MockDBSession(pat_uids=["ghost_user"]) + delete_session = MockDBSession(delete_rowcount=0) # Already cleaned up + + def mock_db_session(): + nonlocal call_count + call_count += 1 + if call_count == 1: + return select_session + return delete_session + + monkeypatch.setattr("atr.token_cleanup.db.session", mock_db_session) + + revoked = await token_cleanup.revoke_pats_for_banned_users() + + assert revoked == 0 + assert len(audit_calls) == 0 + + [email protected] +async def test_cleanup_handles_multiple_banned_users(monkeypatch: "MonkeyPatch") -> None: + """Multiple banned users should each have their tokens revoked independently.""" + audit_calls: list[dict] = [] + + async def mock_account_lookup(uid: str) -> None: + return None # All accounts deleted + + monkeypatch.setattr("atr.token_cleanup.ldap.account_lookup", mock_account_lookup) + monkeypatch.setattr("atr.token_cleanup.storage.audit", lambda **kwargs: audit_calls.append(kwargs)) + + call_count = 0 + select_session = MockDBSession(pat_uids=["user_a", "user_b", "user_c"]) + delete_sessions = [ + MockDBSession(delete_rowcount=2), + MockDBSession(delete_rowcount=1), + MockDBSession(delete_rowcount=4), + ] + + def mock_db_session(): + nonlocal call_count + call_count += 1 + if call_count == 1: + return select_session + return delete_sessions[call_count - 2] + + monkeypatch.setattr("atr.token_cleanup.db.session", mock_db_session) + + revoked = await token_cleanup.revoke_pats_for_banned_users() + + assert revoked == 7 # 2 + 1 + 4 + assert len(audit_calls) == 3 + revoked_uids = {call["target_asf_uid"] for call in audit_calls} + assert revoked_uids == {"user_a", "user_b", "user_c"} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
