This is an automated email from the ASF dual-hosted git repository.
arm pushed a commit to branch arm
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
The following commit(s) were added to refs/heads/arm by this push:
new 99c893ef Add failed authentication logging to identified areas. Closes
#725
99c893ef is described below
commit 99c893efe7a2c0e0b4822d5abc46c78579e2b9be
Author: Alastair McFarlane <[email protected]>
AuthorDate: Mon Mar 2 14:30:31 2026 +0000
Add failed authentication logging to identified areas. Closes #725
---
atr/api/__init__.py | 2 +-
atr/jwtoken.py | 11 ++++++++++-
atr/log.py | 11 +++++++++++
atr/ssh.py | 15 +++++++++++++--
atr/storage/writers/tokens.py | 7 +------
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/atr/api/__init__.py b/atr/api/__init__.py
index 251e268e..cf2d9ebf 100644
--- a/atr/api/__init__.py
+++ b/atr/api/__init__.py
@@ -438,7 +438,7 @@ async def jwt_create(data: models.api.JwtCreateArgs) ->
DictResponse:
# Expects {"asfuid": "uid", "pat": "pat-token"}
# Returns {"asfuid": "uid", "jwt": "jwt-token"}
asf_uid = data.asfuid
- log.add_context(user_id=asf_uid)
+ log.set_asf_uid(asf_uid)
async with storage.write(asf_uid) as write:
wafc = write.as_foundation_committer()
jwt = await wafc.tokens.issue_jwt(data.pat)
diff --git a/atr/jwtoken.py b/atr/jwtoken.py
index 634780fe..3ed8edef 100644
--- a/atr/jwtoken.py
+++ b/atr/jwtoken.py
@@ -72,10 +72,13 @@ def require[**P, R](func: Callable[P, Coroutine[Any, Any,
R]]) -> Callable[P, Aw
try:
claims = await verify(token)
except jwt.ExpiredSignatureError as exc:
+ log.failed_authentication("jwt_token_expired")
raise base.ASFQuartException("Token has expired", errorcode=401)
from exc
except jwt.InvalidTokenError as exc:
+ log.failed_authentication("jwt_token_invalid")
raise base.ASFQuartException("Invalid Bearer JWT format",
errorcode=401) from exc
except jwt.PyJWTError as exc:
+ log.failed_authentication("jwt_token_invalid_2")
raise base.ASFQuartException(f"Invalid Bearer JWT: {exc}",
errorcode=401) from exc
quart.g.jwt_claims = claims
@@ -85,6 +88,10 @@ def require[**P, R](func: Callable[P, Coroutine[Any, Any,
R]]) -> Callable[P, Aw
async def verify(token: str) -> dict[str, Any]:
+ # Grab the "supposed" asf UID from the token presented, to make sure we
know who failed to authenticate on failure.
+ claims_unsafe = jwt.decode(token, verify=False)
+ asf_uid = claims_unsafe.get("sub")
+ log.set_asf_uid(asf_uid)
claims = jwt.decode(
token,
_JWT_SECRET_KEY,
@@ -93,10 +100,12 @@ async def verify(token: str) -> dict[str, Any]:
audience=_ATR_JWT_AUDIENCE,
options={"require": ["sub", "iss", "aud", "iat", "exp", "jti"]},
)
- asf_uid = claims.get("sub")
+ log.debug(f"JWT claims: {claims}")
if not isinstance(asf_uid, str):
+ log.failed_authentication("jwt_subject_invalid")
raise jwt.InvalidTokenError("Invalid Bearer JWT subject")
if not await ldap.is_active(asf_uid):
+ log.failed_authentication("account_deleted_or_banned")
raise base.ASFQuartException("Account is disabled", errorcode=401)
return claims
diff --git a/atr/log.py b/atr/log.py
index 480f5159..75506713 100644
--- a/atr/log.py
+++ b/atr/log.py
@@ -119,6 +119,13 @@ def exception(msg: str, **kwargs) -> None:
_event(logging.ERROR, msg, exc_info=True, **kwargs)
+def failed_authentication(reason: str) -> None:
+ warning(
+ "Authentication failed",
+ reason=reason,
+ )
+
+
def get_context(arg) -> Any | None:
"""Get context from the request log"""
return structlog.contextvars.get_contextvars().get(arg, None)
@@ -158,6 +165,10 @@ def python_repr(object_name: str) -> str:
return f"<{object_name}>"
+def set_asf_uid(asfuid: str | None) -> None:
+ add_context(asfuid=asfuid)
+
+
def warning(msg: str, **kwargs) -> None:
_event(logging.WARNING, msg, **kwargs)
diff --git a/atr/ssh.py b/atr/ssh.py
index 567a4085..80bad1d4 100644
--- a/atr/ssh.py
+++ b/atr/ssh.py
@@ -131,10 +131,12 @@ class SSHServer(asyncssh.SSHServer):
return True
async def validate_public_key(self, username: str, key: asyncssh.SSHKey)
-> bool:
- # This method is not called when username is not "github"
+ # This method is not called when username is not "github" - as long as
the key was valid
# Also, this SSHServer.validate_public_key method does not perform
signature verification
# The SSHServerConnection.validate_public_key method performs
signature verification
if username != "github":
+ # We log an authentication failure in here because if we're here
with a different username the key failed
+ log.failed_authentication("public_key_invalid")
return False
fingerprint = key.get_fingerprint()
@@ -144,11 +146,15 @@ class SSHServer(asyncssh.SSHServer):
if workflow_key is None:
return False
+ # In some cases this will be a service account
+ self._github_asf_uid = workflow_key.asf_uid
+ log.set_asf_uid(self._github_asf_uid)
+
now = int(time.time())
if workflow_key.expires < now:
+ log.failed_authentication("public_key_expired")
return False
- self._github_asf_uid = workflow_key.asf_uid
self._github_payload = workflow_key.github_payload
return True
@@ -188,10 +194,14 @@ async def server_start() -> asyncssh.SSHAcceptor:
server_instance = connection.get_owner()
return asyncio.create_task(_step_01_handle_client(process,
server_instance))
+ def error_handler(*args, **kwargs) -> None:
+ pass
+
server = await asyncssh.create_server(
SSHServer,
server_host_keys=[key_path],
process_factory=process_factory,
+ error_handler=error_handler,
host=_CONFIG.SSH_HOST,
port=_CONFIG.SSH_PORT,
encoding=None,
@@ -243,6 +253,7 @@ async def _step_01_handle_client(process:
asyncssh.SSHServerProcess, server: SSH
async def _step_02_handle_safely(process: asyncssh.SSHServerProcess, server:
SSHServer) -> None:
asf_uid = server._get_asf_uid(process)
+ log.set_asf_uid(asf_uid)
log.info(f"Handling command for authenticated user: {asf_uid}")
if not process.command:
diff --git a/atr/storage/writers/tokens.py b/atr/storage/writers/tokens.py
index d700dee5..620e9d3b 100644
--- a/atr/storage/writers/tokens.py
+++ b/atr/storage/writers/tokens.py
@@ -128,12 +128,7 @@ class FoundationCommitter(GeneralPublic):
# Verify account still exists in LDAP
account_details = await ldap.account_lookup(self.__asf_uid)
if (account_details is None) or ldap.is_banned(account_details):
- log.warning(
- "Authentication failed",
- extra={
- "reason": "account_deleted_or_banned",
- },
- )
+ log.failed_authentication("account_deleted_or_banned")
raise storage.AccessError("Authentication failed")
issued_jwt = jwtoken.issue(self.__asf_uid)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
