diff --git a/enterprise/integrations/github/github_manager.py b/enterprise/integrations/github/github_manager.py index 0d6d9d655f..1451595357 100644 --- a/enterprise/integrations/github/github_manager.py +++ b/enterprise/integrations/github/github_manager.py @@ -21,6 +21,7 @@ from integrations.utils import ( CONVERSATION_URL, HOST_URL, OPENHANDS_RESOLVER_TEMPLATES_DIR, + get_session_expired_message, ) from integrations.v1_utils import get_saas_user_auth from jinja2 import Environment, FileSystemLoader @@ -31,7 +32,11 @@ from server.utils.conversation_callback_utils import register_callback_processor from openhands.core.logger import openhands_logger as logger from openhands.integrations.provider import ProviderToken, ProviderType -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) from openhands.storage.data_models.secrets import Secrets from openhands.utils.async_utils import call_sync_from_async @@ -342,6 +347,13 @@ class GithubManager(Manager): msg_info = f'@{user_info.username} please set a valid LLM API key in [OpenHands Cloud]({HOST_URL}) before starting a job.' + except SessionExpiredError as e: + logger.warning( + f'[GitHub] Session expired for user {user_info.username}: {str(e)}' + ) + + msg_info = get_session_expired_message(user_info.username) + msg = self.create_outgoing_message(msg_info) await self.send_message(msg, github_view) diff --git a/enterprise/integrations/gitlab/gitlab_manager.py b/enterprise/integrations/gitlab/gitlab_manager.py index 4ab3644250..ccc6da3621 100644 --- a/enterprise/integrations/gitlab/gitlab_manager.py +++ b/enterprise/integrations/gitlab/gitlab_manager.py @@ -15,6 +15,7 @@ from integrations.utils import ( CONVERSATION_URL, HOST_URL, OPENHANDS_RESOLVER_TEMPLATES_DIR, + get_session_expired_message, ) from jinja2 import Environment, FileSystemLoader from pydantic import SecretStr @@ -24,7 +25,11 @@ from server.utils.conversation_callback_utils import register_callback_processor from openhands.core.logger import openhands_logger as logger from openhands.integrations.gitlab.gitlab_service import GitLabServiceImpl from openhands.integrations.provider import ProviderToken, ProviderType -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) from openhands.storage.data_models.secrets import Secrets @@ -249,6 +254,13 @@ class GitlabManager(Manager): msg_info = f'@{user_info.username} please set a valid LLM API key in [OpenHands Cloud]({HOST_URL}) before starting a job.' + except SessionExpiredError as e: + logger.warning( + f'[GitLab] Session expired for user {user_info.username}: {str(e)}' + ) + + msg_info = get_session_expired_message(user_info.username) + # Send the acknowledgment message msg = self.create_outgoing_message(msg_info) await self.send_message(msg, gitlab_view) diff --git a/enterprise/integrations/jira/jira_manager.py b/enterprise/integrations/jira/jira_manager.py index b8c7fecfc9..1c49b417aa 100644 --- a/enterprise/integrations/jira/jira_manager.py +++ b/enterprise/integrations/jira/jira_manager.py @@ -17,6 +17,7 @@ from integrations.utils import ( HOST_URL, OPENHANDS_RESOLVER_TEMPLATES_DIR, filter_potential_repos_by_user_msg, + get_session_expired_message, ) from jinja2 import Environment, FileSystemLoader from server.auth.saas_user_auth import get_user_auth_from_keycloak_id @@ -30,7 +31,11 @@ from openhands.core.logger import openhands_logger as logger from openhands.integrations.provider import ProviderHandler from openhands.integrations.service_types import Repository from openhands.server.shared import server_config -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) from openhands.server.user_auth.user_auth import UserAuth from openhands.utils.http_session import httpx_verify_option @@ -380,6 +385,10 @@ class JiraManager(Manager): logger.warning(f'[Jira] LLM authentication error: {str(e)}') msg_info = f'Please set a valid LLM API key in [OpenHands Cloud]({HOST_URL}) before starting a job.' + except SessionExpiredError as e: + logger.warning(f'[Jira] Session expired: {str(e)}') + msg_info = get_session_expired_message() + except Exception as e: logger.error( f'[Jira] Unexpected error starting job: {str(e)}', exc_info=True diff --git a/enterprise/integrations/jira_dc/jira_dc_manager.py b/enterprise/integrations/jira_dc/jira_dc_manager.py index 700267511b..a658a791b3 100644 --- a/enterprise/integrations/jira_dc/jira_dc_manager.py +++ b/enterprise/integrations/jira_dc/jira_dc_manager.py @@ -19,6 +19,7 @@ from integrations.utils import ( HOST_URL, OPENHANDS_RESOLVER_TEMPLATES_DIR, filter_potential_repos_by_user_msg, + get_session_expired_message, ) from jinja2 import Environment, FileSystemLoader from server.auth.saas_user_auth import get_user_auth_from_keycloak_id @@ -32,7 +33,11 @@ from openhands.core.logger import openhands_logger as logger from openhands.integrations.provider import ProviderHandler from openhands.integrations.service_types import Repository from openhands.server.shared import server_config -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) from openhands.server.user_auth.user_auth import UserAuth from openhands.utils.http_session import httpx_verify_option @@ -397,6 +402,10 @@ class JiraDcManager(Manager): logger.warning(f'[Jira DC] LLM authentication error: {str(e)}') msg_info = f'Please set a valid LLM API key in [OpenHands Cloud]({HOST_URL}) before starting a job.' + except SessionExpiredError as e: + logger.warning(f'[Jira DC] Session expired: {str(e)}') + msg_info = get_session_expired_message() + except Exception as e: logger.error( f'[Jira DC] Unexpected error starting job: {str(e)}', exc_info=True diff --git a/enterprise/integrations/linear/linear_manager.py b/enterprise/integrations/linear/linear_manager.py index 5eed24d674..9ac7d8ed49 100644 --- a/enterprise/integrations/linear/linear_manager.py +++ b/enterprise/integrations/linear/linear_manager.py @@ -16,6 +16,7 @@ from integrations.utils import ( HOST_URL, OPENHANDS_RESOLVER_TEMPLATES_DIR, filter_potential_repos_by_user_msg, + get_session_expired_message, ) from jinja2 import Environment, FileSystemLoader from server.auth.saas_user_auth import get_user_auth_from_keycloak_id @@ -29,7 +30,11 @@ from openhands.core.logger import openhands_logger as logger from openhands.integrations.provider import ProviderHandler from openhands.integrations.service_types import Repository from openhands.server.shared import server_config -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) from openhands.server.user_auth.user_auth import UserAuth from openhands.utils.http_session import httpx_verify_option @@ -387,6 +392,10 @@ class LinearManager(Manager): logger.warning(f'[Linear] LLM authentication error: {str(e)}') msg_info = f'Please set a valid LLM API key in [OpenHands Cloud]({HOST_URL}) before starting a job.' + except SessionExpiredError as e: + logger.warning(f'[Linear] Session expired: {str(e)}') + msg_info = get_session_expired_message() + except Exception as e: logger.error( f'[Linear] Unexpected error starting job: {str(e)}', exc_info=True diff --git a/enterprise/integrations/slack/slack_manager.py b/enterprise/integrations/slack/slack_manager.py index 1fd4e20759..858c7c98f0 100644 --- a/enterprise/integrations/slack/slack_manager.py +++ b/enterprise/integrations/slack/slack_manager.py @@ -14,6 +14,7 @@ from integrations.slack.slack_view import ( from integrations.utils import ( HOST_URL, OPENHANDS_RESOLVER_TEMPLATES_DIR, + get_session_expired_message, ) from jinja2 import Environment, FileSystemLoader from pydantic import SecretStr @@ -29,7 +30,11 @@ from openhands.core.logger import openhands_logger as logger from openhands.integrations.provider import ProviderHandler from openhands.integrations.service_types import Repository from openhands.server.shared import config, server_config -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) from openhands.server.user_auth.user_auth import UserAuth authorize_url_generator = AuthorizeUrlGenerator( @@ -352,6 +357,13 @@ class SlackManager(Manager): msg_info = f'@{user_info.slack_display_name} please set a valid LLM API key in [OpenHands Cloud]({HOST_URL}) before starting a job.' + except SessionExpiredError as e: + logger.warning( + f'[Slack] Session expired for user {user_info.slack_display_name}: {str(e)}' + ) + + msg_info = get_session_expired_message(user_info.slack_display_name) + except StartingConvoException as e: msg_info = str(e) diff --git a/enterprise/integrations/utils.py b/enterprise/integrations/utils.py index 5cdf38666d..49cc4087c2 100644 --- a/enterprise/integrations/utils.py +++ b/enterprise/integrations/utils.py @@ -47,6 +47,27 @@ ENABLE_PROACTIVE_CONVERSATION_STARTERS = ( os.getenv('ENABLE_PROACTIVE_CONVERSATION_STARTERS', 'false').lower() == 'true' ) + +def get_session_expired_message(username: str | None = None) -> str: + """Get a user-friendly session expired message. + + Used by integrations to notify users when their Keycloak offline session + has expired. + + Args: + username: Optional username to mention in the message. If provided, + the message will include @username prefix (used by Git providers + like GitHub, GitLab, Slack). If None, returns a generic message + (used by Jira, Jira DC, Linear). + + Returns: + A formatted session expired message + """ + if username: + return f'@{username} your session has expired. Please login again at [OpenHands Cloud]({HOST_URL}) and try again.' + return f'Your session has expired. Please login again at [OpenHands Cloud]({HOST_URL}) and try again.' + + # Toggle for solvability report feature ENABLE_SOLVABILITY_ANALYSIS = ( os.getenv('ENABLE_SOLVABILITY_ANALYSIS', 'false').lower() == 'true' diff --git a/enterprise/server/auth/token_manager.py b/enterprise/server/auth/token_manager.py index 6061518cb4..1c641a094c 100644 --- a/enterprise/server/auth/token_manager.py +++ b/enterprise/server/auth/token_manager.py @@ -14,6 +14,7 @@ from keycloak.exceptions import ( KeycloakAuthenticationError, KeycloakConnectionError, KeycloakError, + KeycloakPostError, ) from server.auth.constants import ( BITBUCKET_APP_CLIENT_ID, @@ -43,6 +44,7 @@ from storage.offline_token_store import OfflineTokenStore from tenacity import RetryCallState, retry, retry_if_exception_type, stop_after_attempt from openhands.integrations.service_types import ProviderType +from openhands.server.types import SessionExpiredError from openhands.utils.http_session import httpx_verify_option @@ -465,6 +467,14 @@ class TokenManager: except KeycloakConnectionError: logger.exception('KeycloakConnectionError when refreshing token') raise + except KeycloakPostError as e: + error_message = str(e) + if 'invalid_grant' in error_message or 'session not found' in error_message: + logger.warning(f'User session expired or invalid: {error_message}') + raise SessionExpiredError( + 'Your session has expired. Please login again.' + ) from e + raise @retry( stop=stop_after_attempt(2), diff --git a/enterprise/tests/unit/integrations/jira/test_jira_manager.py b/enterprise/tests/unit/integrations/jira/test_jira_manager.py index e1420c0f0e..9240af3728 100644 --- a/enterprise/tests/unit/integrations/jira/test_jira_manager.py +++ b/enterprise/tests/unit/integrations/jira/test_jira_manager.py @@ -18,7 +18,11 @@ from integrations.jira.jira_view import ( from integrations.models import Message, SourceType from openhands.integrations.service_types import ProviderType, Repository -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) class TestJiraManagerInit: @@ -732,6 +736,32 @@ class TestStartJob: call_args = jira_manager.send_message.call_args[0] assert 'valid LLM API key' in call_args[0].message + @pytest.mark.asyncio + async def test_start_job_session_expired_error( + self, jira_manager, sample_jira_workspace + ): + """Test job start with session expired error.""" + mock_view = MagicMock(spec=JiraNewConversationView) + mock_view.jira_user = MagicMock() + mock_view.jira_user.keycloak_user_id = 'test_user' + mock_view.job_context = MagicMock() + mock_view.job_context.issue_key = 'PROJ-123' + mock_view.jira_workspace = sample_jira_workspace + mock_view.create_or_update_conversation = AsyncMock( + side_effect=SessionExpiredError('Session expired') + ) + + jira_manager.send_message = AsyncMock() + jira_manager.token_manager.decrypt_text.return_value = 'decrypted_key' + + await jira_manager.start_job(mock_view) + + # Should send error message about session expired + jira_manager.send_message.assert_called_once() + call_args = jira_manager.send_message.call_args[0] + assert 'session has expired' in call_args[0].message + assert 'login again' in call_args[0].message + @pytest.mark.asyncio async def test_start_job_unexpected_error( self, jira_manager, sample_jira_workspace diff --git a/enterprise/tests/unit/integrations/jira_dc/test_jira_dc_manager.py b/enterprise/tests/unit/integrations/jira_dc/test_jira_dc_manager.py index e26994bfeb..9efbc55c04 100644 --- a/enterprise/tests/unit/integrations/jira_dc/test_jira_dc_manager.py +++ b/enterprise/tests/unit/integrations/jira_dc/test_jira_dc_manager.py @@ -18,7 +18,11 @@ from integrations.jira_dc.jira_dc_view import ( from integrations.models import Message, SourceType from openhands.integrations.service_types import ProviderType, Repository -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) class TestJiraDcManagerInit: @@ -761,6 +765,32 @@ class TestStartJob: call_args = jira_dc_manager.send_message.call_args[0] assert 'valid LLM API key' in call_args[0].message + @pytest.mark.asyncio + async def test_start_job_session_expired_error( + self, jira_dc_manager, sample_jira_dc_workspace + ): + """Test job start with session expired error.""" + mock_view = MagicMock(spec=JiraDcNewConversationView) + mock_view.jira_dc_user = MagicMock() + mock_view.jira_dc_user.keycloak_user_id = 'test_user' + mock_view.job_context = MagicMock() + mock_view.job_context.issue_key = 'PROJ-123' + mock_view.jira_dc_workspace = sample_jira_dc_workspace + mock_view.create_or_update_conversation = AsyncMock( + side_effect=SessionExpiredError('Session expired') + ) + + jira_dc_manager.send_message = AsyncMock() + jira_dc_manager.token_manager.decrypt_text.return_value = 'decrypted_key' + + await jira_dc_manager.start_job(mock_view) + + # Should send error message about session expired + jira_dc_manager.send_message.assert_called_once() + call_args = jira_dc_manager.send_message.call_args[0] + assert 'session has expired' in call_args[0].message + assert 'login again' in call_args[0].message + @pytest.mark.asyncio async def test_start_job_unexpected_error( self, jira_dc_manager, sample_jira_dc_workspace diff --git a/enterprise/tests/unit/integrations/linear/test_linear_manager.py b/enterprise/tests/unit/integrations/linear/test_linear_manager.py index 22f0294e06..ac48fe8014 100644 --- a/enterprise/tests/unit/integrations/linear/test_linear_manager.py +++ b/enterprise/tests/unit/integrations/linear/test_linear_manager.py @@ -18,7 +18,11 @@ from integrations.linear.linear_view import ( from integrations.models import Message, SourceType from openhands.integrations.service_types import ProviderType, Repository -from openhands.server.types import LLMAuthenticationError, MissingSettingsError +from openhands.server.types import ( + LLMAuthenticationError, + MissingSettingsError, + SessionExpiredError, +) class TestLinearManagerInit: @@ -826,6 +830,33 @@ class TestStartJob: call_args = linear_manager.send_message.call_args[0] assert 'valid LLM API key' in call_args[0].message + @pytest.mark.asyncio + async def test_start_job_session_expired_error( + self, linear_manager, sample_linear_workspace + ): + """Test job start with session expired error.""" + mock_view = MagicMock(spec=LinearNewConversationView) + mock_view.linear_user = MagicMock() + mock_view.linear_user.keycloak_user_id = 'test_user' + mock_view.job_context = MagicMock() + mock_view.job_context.issue_key = 'TEST-123' + mock_view.job_context.issue_id = 'issue_id' + mock_view.linear_workspace = sample_linear_workspace + mock_view.create_or_update_conversation = AsyncMock( + side_effect=SessionExpiredError('Session expired') + ) + + linear_manager.send_message = AsyncMock() + linear_manager.token_manager.decrypt_text.return_value = 'decrypted_key' + + await linear_manager.start_job(mock_view) + + # Should send error message about session expired + linear_manager.send_message.assert_called_once() + call_args = linear_manager.send_message.call_args[0] + assert 'session has expired' in call_args[0].message + assert 'login again' in call_args[0].message + @pytest.mark.asyncio async def test_start_job_unexpected_error( self, linear_manager, sample_linear_workspace diff --git a/enterprise/tests/unit/integrations/test_utils.py b/enterprise/tests/unit/integrations/test_utils.py index 17105e1b57..38710af083 100644 --- a/enterprise/tests/unit/integrations/test_utils.py +++ b/enterprise/tests/unit/integrations/test_utils.py @@ -4,7 +4,9 @@ from unittest.mock import patch import pytest from integrations.utils import ( + HOST_URL, append_conversation_footer, + get_session_expired_message, get_summary_for_agent_state, ) @@ -164,6 +166,68 @@ class TestGetSummaryForAgentState: assert self.conversation_link not in result +class TestGetSessionExpiredMessage: + """Test cases for get_session_expired_message function.""" + + def test_message_with_username_contains_at_prefix(self): + """Test that the message contains the username with @ prefix.""" + result = get_session_expired_message('testuser') + assert '@testuser' in result + + def test_message_with_username_contains_session_expired_text(self): + """Test that the message contains session expired text.""" + result = get_session_expired_message('testuser') + assert 'session has expired' in result + + def test_message_with_username_contains_login_instruction(self): + """Test that the message contains login instruction.""" + result = get_session_expired_message('testuser') + assert 'login again' in result + + def test_message_with_username_contains_host_url(self): + """Test that the message contains the OpenHands Cloud URL.""" + result = get_session_expired_message('testuser') + assert HOST_URL in result + assert 'OpenHands Cloud' in result + + def test_different_usernames(self): + """Test that different usernames produce different messages.""" + result1 = get_session_expired_message('user1') + result2 = get_session_expired_message('user2') + assert '@user1' in result1 + assert '@user2' in result2 + assert '@user1' not in result2 + assert '@user2' not in result1 + + def test_message_without_username_contains_session_expired_text(self): + """Test that the message without username contains session expired text.""" + result = get_session_expired_message() + assert 'session has expired' in result + + def test_message_without_username_contains_login_instruction(self): + """Test that the message without username contains login instruction.""" + result = get_session_expired_message() + assert 'login again' in result + + def test_message_without_username_contains_host_url(self): + """Test that the message without username contains the OpenHands Cloud URL.""" + result = get_session_expired_message() + assert HOST_URL in result + assert 'OpenHands Cloud' in result + + def test_message_without_username_does_not_contain_at_prefix(self): + """Test that the message without username does not contain @ prefix.""" + result = get_session_expired_message() + assert not result.startswith('@') + assert 'Your session' in result + + def test_message_with_none_username(self): + """Test that passing None explicitly works the same as no argument.""" + result = get_session_expired_message(None) + assert not result.startswith('@') + assert 'Your session' in result + + class TestAppendConversationFooter: """Test cases for append_conversation_footer function.""" diff --git a/openhands/server/types.py b/openhands/server/types.py index 6809f8256a..5e6d2a9368 100644 --- a/openhands/server/types.py +++ b/openhands/server/types.py @@ -50,3 +50,9 @@ class LLMAuthenticationError(ValueError): """Raised when there is an issue with LLM authentication.""" pass + + +class SessionExpiredError(ValueError): + """Raised when the user's authentication session has expired.""" + + pass