mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-01-08 22:38:05 -05:00
Handle expired Keycloak session with user-friendly error message (#12168)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user