Skip to content

Commit 6232741

Browse files
Invalidate PATs; fixes #598
1 parent d71388d commit 6232741

File tree

14 files changed

+786
-3
lines changed

14 files changed

+786
-3
lines changed

atr/admin/__init__.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ class LdapLookupForm(form.Form):
8888
email: str = form.label("Email address (optional)", "Enter email address, e.g. user@example.org")
8989

9090

91+
class RevokeUserTokensForm(form.Form):
92+
asf_uid: str = form.label("ASF UID", "Enter the ASF UID whose tokens should be revoked.")
93+
confirm_revoke: Literal["REVOKE"] = form.label("Confirmation", "Type REVOKE to confirm.")
94+
95+
9196
class SessionDataCommon(NamedTuple):
9297
uid: str
9398
fullname: str
@@ -699,6 +704,53 @@ async def projects_update_post(session: web.Committer) -> str | web.WerkzeugResp
699704
}, 200
700705

701706

707+
@admin.get("/revoke-user-tokens")
708+
async def revoke_user_tokens_get(session: web.Committer) -> str:
709+
"""Revoke all Personal Access Tokens for a specified user."""
710+
token_counts: list[tuple[str, int]] = []
711+
async with db.session() as data:
712+
stmt = (
713+
sqlmodel.select(
714+
sql.PersonalAccessToken.asfuid,
715+
sqlmodel.func.count(),
716+
)
717+
.group_by(sql.PersonalAccessToken.asfuid)
718+
.order_by(sql.PersonalAccessToken.asfuid)
719+
)
720+
rows = await data.execute_query(stmt)
721+
token_counts = [(row[0], row[1]) for row in rows]
722+
723+
rendered_form = form.render(
724+
model_cls=RevokeUserTokensForm,
725+
submit_label="Revoke all tokens",
726+
)
727+
return await template.render(
728+
"revoke-user-tokens.html",
729+
form=rendered_form,
730+
token_counts=token_counts,
731+
)
732+
733+
734+
@admin.post("/revoke-user-tokens")
735+
@admin.form(RevokeUserTokensForm)
736+
async def revoke_user_tokens_post(
737+
session: web.Committer, revoke_form: RevokeUserTokensForm
738+
) -> str | web.WerkzeugResponse:
739+
"""Revoke all Personal Access Tokens for a specified user."""
740+
target_uid = revoke_form.asf_uid.strip()
741+
742+
async with storage.write(session) as write:
743+
wafa = write.as_foundation_admin("infrastructure")
744+
count = await wafa.tokens.revoke_all_user_tokens(target_uid)
745+
746+
if count > 0:
747+
await quart.flash(f"Revoked {count} token(s) for {target_uid}.", "success")
748+
else:
749+
await quart.flash(f"No tokens found for {target_uid}.", "info")
750+
751+
return await session.redirect(revoke_user_tokens_get)
752+
753+
702754
@admin.get("/task-times/<project_name>/<version_name>/<revision_number>")
703755
async def task_times(
704756
session: web.Committer, project_name: str, version_name: str, revision_number: str
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
{% extends "layouts/base-admin.html" %}
2+
3+
{%- block title -%}Revoke user tokens ~ ATR{%- endblock title -%}
4+
5+
{%- block description -%}Revoke all Personal Access Tokens for a user.{%- endblock description -%}
6+
7+
{% block content %}
8+
<h1>Revoke user tokens</h1>
9+
<p>Revoke all Personal Access Tokens (PATs) for a user account. Use this when an account
10+
is being disabled or when immediate token revocation is needed.</p>
11+
12+
<div class="card mb-4">
13+
<div class="card-header">
14+
<h5 class="mb-0">Revoke tokens</h5>
15+
</div>
16+
<div class="card-body">
17+
{{ form }}
18+
</div>
19+
</div>
20+
21+
{% if token_counts %}
22+
<div class="card">
23+
<div class="card-header">
24+
<h5 class="mb-0">Users with active tokens</h5>
25+
</div>
26+
<div class="card-body">
27+
<table class="table table-sm table-striped table-bordered">
28+
<thead>
29+
<tr>
30+
<th>ASF UID</th>
31+
<th>Token count</th>
32+
</tr>
33+
</thead>
34+
<tbody>
35+
{% for uid, count in token_counts %}
36+
<tr>
37+
<td><code>{{ uid }}</code></td>
38+
<td>{{ count }}</td>
39+
</tr>
40+
{% endfor %}
41+
</tbody>
42+
</table>
43+
</div>
44+
</div>
45+
{% else %}
46+
<div class="alert alert-info" role="alert">No users currently have active tokens.</div>
47+
{% endif %}
48+
{% endblock content %}

atr/docs/authentication-security.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,13 @@ Committers can obtain PATs from the `/tokens` page on the ATR website. PATs have
6969

7070
* **Validity**: 180 days from creation, while LDAP account is still active
7171
* **Storage**: ATR stores only SHA3-256 hashes, never the plaintext PAT
72-
* **Revocation**: Users can revoke their own PATs at any time; admins can revoke any PAT
72+
* **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
73+
* **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
7374
* **Purpose**: PATs are used solely to obtain JWTs; they cannot be used directly for API access
7475

7576
Only authenticated committers (signed in via ASF OAuth) can create PATs. Each user can have multiple active PATs.
7677

77-
PATs are rejected if the user who created them has been removed from LDAP.
78+
PATs are rejected if the user who created them has been banned in or 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.
7879

7980
### JSON Web Tokens (JWTs)
8081

@@ -139,7 +140,8 @@ For web users, authentication happens once via ASF OAuth, and the session persis
139140
### Personal Access Tokens
140141

141142
* Stored as SHA3-256 hashes
142-
* Can be revoked immediately by the user
143+
* Can be revoked immediately by the user or in bulk by administrators
144+
* Automatically revoked when the owning account is banned or deleted in LDAP
143145
* Limited purpose (only for JWT issuance) reduces impact of compromise
144146
* Long validity (180 days) balanced by easy revocation
145147

@@ -162,3 +164,5 @@ Tokens must be protected by the user at all times:
162164

163165
* [`principal.py`](/ref/atr/principal.py) - Session caching and authorization data
164166
* [`jwtoken.py`](/ref/atr/jwtoken.py) - JWT creation, verification, and decorators
167+
* [`token_cleanup.py`](/ref/atr/token_cleanup.py) - Automated PAT revocation for banned/deleted accounts
168+
* [`storage/writers/tokens.py`](/ref/atr/storage/writers/tokens.py) - Token creation, deletion, and admin revocation

atr/docs/authorization-security.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,21 @@ Token operations apply to the authenticated user:
122122
* Allowed for: The token owner, or administrators
123123
* Constraint: Users can only revoke their own tokens (unless admin)
124124

125+
**Revoke all tokens for a user (admin)**:
126+
127+
* Allowed for: ATR administrators only
128+
* Interface: Admin "Revoke user tokens" page
129+
* Constraint: Requires typing "REVOKE" as confirmation
130+
131+
**Automated token revocation**:
132+
133+
* 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.
134+
125135
**Exchange PAT for JWT**:
126136

127137
* Allowed for: Anyone with a valid PAT
128138
* Note: This is an unauthenticated endpoint; the PAT serves as the credential
139+
* Constraint: LDAP is checked at exchange time; banned or deleted accounts are rejected even if the PAT has not yet been cleaned up
129140

130141
## Access control for check ignores
131142

atr/server.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import atr.svn.pubsub as pubsub
6262
import atr.tasks as tasks
6363
import atr.template as template
64+
import atr.token_cleanup as token_cleanup
6465
import atr.user as user
6566
import atr.util as util
6667

@@ -269,6 +270,9 @@ async def startup() -> None:
269270
admins_task = asyncio.create_task(cache.admins_refresh_loop())
270271
app.extensions["admins_task"] = admins_task
271272

273+
pat_cleanup_task = asyncio.create_task(token_cleanup.cleanup_loop())
274+
app.extensions["pat_cleanup_task"] = pat_cleanup_task
275+
272276
worker_manager = manager.get_worker_manager()
273277
await worker_manager.start()
274278

@@ -312,6 +316,11 @@ async def shutdown() -> None:
312316
with contextlib.suppress(asyncio.CancelledError):
313317
await task
314318

319+
if task := app.extensions.get("pat_cleanup_task"):
320+
task.cancel()
321+
with contextlib.suppress(asyncio.CancelledError):
322+
await task
323+
315324
await db.shutdown_database()
316325

317326
await _app_shutdown_log_listeners(app)

atr/storage/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ def __init__(self, write: Write, data: db.Session, committee_name: str):
247247
self.__committee_name = committee_name
248248
self.keys = writers.keys.FoundationAdmin(write, self, data, committee_name)
249249
self.release = writers.release.FoundationAdmin(write, self, data, committee_name)
250+
self.tokens = writers.tokens.FoundationAdmin(write, self, data, committee_name)
250251

251252
@property
252253
def asf_uid(self) -> str:

atr/storage/writers/tokens.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,34 @@ def __init__(
169169
raise storage.AccessError("Not authorized")
170170
self.__asf_uid = asf_uid
171171
self.__committee_name = committee_name
172+
173+
174+
class FoundationAdmin(CommitteeMember):
175+
def __init__(
176+
self,
177+
write: storage.Write,
178+
write_as: storage.WriteAsFoundationAdmin,
179+
data: db.Session,
180+
committee_name: str,
181+
):
182+
super().__init__(write, write_as, data, committee_name)
183+
self.__write = write
184+
self.__write_as = write_as
185+
self.__data = data
186+
187+
async def revoke_all_user_tokens(self, target_asf_uid: str) -> int:
188+
"""Revoke all PATs for a specified user. Returns count of revoked tokens."""
189+
tokens = await self.__data.query_all(
190+
sqlmodel.select(sql.PersonalAccessToken).where(sql.PersonalAccessToken.asfuid == target_asf_uid)
191+
)
192+
count = len(tokens)
193+
for token in tokens:
194+
await self.__data.delete(token)
195+
196+
if count > 0:
197+
await self.__data.commit()
198+
self.__write_as.append_to_audit_log(
199+
target_asf_uid=target_asf_uid,
200+
tokens_revoked=count,
201+
)
202+
return count

atr/templates/includes/topnav.html

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,11 @@
216216
href="{{ as_url(admin.ldap_get) }}"
217217
{% if request.endpoint == 'atr_admin_ldap_get' %}class="active"{% endif %}><i class="bi bi-person-plus"></i> LDAP search</a>
218218
</li>
219+
<li>
220+
<a class="dropdown-item"
221+
href="{{ as_url(admin.revoke_user_tokens_get) }}"
222+
{% if request.endpoint == 'atr_admin_revoke_user_tokens_get' %}class="active"{% endif %}><i class="bi bi-shield-x"></i> Revoke user tokens</a>
223+
</li>
219224
<li>
220225
<a class="dropdown-item"
221226
href="{{ as_url(admin.toggle_view_get) }}"

atr/token_cleanup.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
"""Periodic cleanup of Personal Access Tokens for banned or deleted accounts."""
19+
20+
import asyncio
21+
from typing import Final
22+
23+
import sqlalchemy
24+
import sqlmodel
25+
26+
import atr.db as db
27+
import atr.ldap as ldap
28+
import atr.log as log
29+
import atr.models.sql as sql
30+
import atr.storage as storage
31+
32+
# ~1 hour, deliberately offset from the admin poll interval (3631s)
33+
# to avoid simultaneous LDAP request spikes
34+
POLL_INTERVAL_SECONDS: Final[int] = 3617
35+
36+
37+
async def cleanup_loop() -> None:
38+
"""Periodically revoke PATs belonging to banned or deleted LDAP accounts."""
39+
while True:
40+
await asyncio.sleep(POLL_INTERVAL_SECONDS)
41+
try:
42+
await revoke_pats_for_banned_users()
43+
except Exception as e:
44+
log.warning(f"PAT banned-user cleanup failed: {e}")
45+
46+
47+
async def revoke_pats_for_banned_users() -> int:
48+
"""Check all PAT-holding users against LDAP and revoke tokens for banned/deleted accounts.
49+
50+
Returns the total number of tokens revoked.
51+
"""
52+
# Step 1: Get distinct UIDs that have PATs
53+
async with db.session() as data:
54+
stmt = sqlmodel.select(sql.PersonalAccessToken.asfuid).distinct()
55+
rows = await data.execute_query(stmt)
56+
uids_with_pats = [row[0] for row in rows]
57+
58+
if not uids_with_pats:
59+
return 0
60+
61+
# Step 2: Check each against LDAP, revoke if banned/deleted
62+
revoked_total = 0
63+
for uid in uids_with_pats:
64+
try:
65+
account = await ldap.account_lookup(uid)
66+
if (account is not None) and (not ldap.is_banned(account)):
67+
continue
68+
69+
# Account is gone or banned — delete all their tokens
70+
async with db.session() as data:
71+
delete_stmt = sqlalchemy.delete(sql.PersonalAccessToken).where(sql.PersonalAccessToken.asfuid == uid)
72+
result = await data.execute_query(delete_stmt)
73+
await data.commit()
74+
count: int = getattr(result, "rowcount", 0)
75+
76+
if count > 0:
77+
storage.audit(
78+
target_asf_uid=uid,
79+
tokens_revoked=count,
80+
reason="account_banned_or_deleted",
81+
source="pat_cleanup_loop",
82+
)
83+
log.info(f"Auto-revoked {count} PAT(s) for banned/deleted user {uid}")
84+
revoked_total += count
85+
86+
except Exception as e:
87+
log.warning(f"PAT cleanup: failed to check/revoke for {uid}: {e}")
88+
89+
if revoked_total > 0:
90+
log.info(f"PAT cleanup cycle complete: revoked {revoked_total} total token(s)")
91+
92+
return revoked_total

tests/e2e/admin/__init__.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.

0 commit comments

Comments
 (0)