diff --git a/src/github_app.py b/src/github_app.py index c18b6a9..fe7236e 100644 --- a/src/github_app.py +++ b/src/github_app.py @@ -4,12 +4,16 @@ from __future__ import annotations import contextlib +import logging import time from typing import Generator import jwt import requests +logger = logging.getLogger(__name__) +TOKEN_REVOKE_TIMEOUT_SECONDS = 10 + class GithubAppToken: def __init__(self, private_key, app_id) -> None: @@ -29,10 +33,17 @@ def get_token(self, installation_id: int) -> Generator[str, None, None]: # This token expires in an hour yield resp["token"] finally: - requests.delete( - "https://api.github.com/installation/token", - headers={"Authorization": f"token {resp['token']}"}, - ) + try: + requests.delete( + "https://api.github.com/installation/token", + headers={"Authorization": f"token {resp['token']}"}, + timeout=TOKEN_REVOKE_TIMEOUT_SECONDS, + ) + except requests.RequestException: + logger.warning( + "Failed to revoke GitHub App installation token.", + exc_info=True, + ) def get_jwt_token(self, private_key, app_id): payload = { diff --git a/tests/test_github_app.py b/tests/test_github_app.py new file mode 100644 index 0000000..3c782a8 --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,77 @@ +from __future__ import annotations + +from unittest.mock import Mock +from unittest.mock import patch + +import pytest +import requests + +from src.github_app import GithubAppToken +from src.github_app import TOKEN_REVOKE_TIMEOUT_SECONDS + + +def _token_response(token: str = "temp-token") -> Mock: + response = Mock() + response.raise_for_status.return_value = None + response.json.return_value = {"token": token} + return response + + +@patch.object( + GithubAppToken, + "get_authentication_header", + return_value={"Accept": "application/vnd.github.v3+json", "Authorization": "Bearer test"}, +) +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_revokes_token_with_timeout( + mock_post, mock_delete, _mock_get_authentication_header +): + mock_post.return_value = _token_response() + app_token = GithubAppToken(private_key="unused", app_id="123") + + with app_token.get_token(installation_id=1) as token: + assert token == "temp-token" + + mock_delete.assert_called_once_with( + "https://api.github.com/installation/token", + headers={"Authorization": "token temp-token"}, + timeout=TOKEN_REVOKE_TIMEOUT_SECONDS, + ) + + +@patch.object( + GithubAppToken, + "get_authentication_header", + return_value={"Accept": "application/vnd.github.v3+json", "Authorization": "Bearer test"}, +) +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_ignores_revoke_timeout( + mock_post, mock_delete, _mock_get_authentication_header +): + mock_post.return_value = _token_response() + mock_delete.side_effect = requests.ConnectTimeout("timed out") + app_token = GithubAppToken(private_key="unused", app_id="123") + + with app_token.get_token(installation_id=1) as token: + assert token == "temp-token" + + +@patch.object( + GithubAppToken, + "get_authentication_header", + return_value={"Accept": "application/vnd.github.v3+json", "Authorization": "Bearer test"}, +) +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_does_not_mask_inner_exception_when_revoke_fails( + mock_post, mock_delete, _mock_get_authentication_header +): + mock_post.return_value = _token_response() + mock_delete.side_effect = requests.ConnectTimeout("timed out") + app_token = GithubAppToken(private_key="unused", app_id="123") + + with pytest.raises(RuntimeError, match="original failure"): + with app_token.get_token(installation_id=1): + raise RuntimeError("original failure")