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
commit 18d73aab4c36a64b2808168019f256cc5654e122 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]
