[Jira]: separate signature verification from conversation orchestration (#12478)

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
Rohit Malhotra
2026-01-20 13:21:23 -08:00
committed by GitHub
parent 87a5762bf2
commit 3bc56740b9
5 changed files with 394 additions and 176 deletions

View File

@@ -1,10 +1,7 @@
import hashlib
import hmac
from typing import Dict, Optional, Tuple
from typing import Tuple
from urllib.parse import urlparse
import httpx
from fastapi import Request
from integrations.jira.jira_types import JiraViewInterface
from integrations.jira.jira_view import (
JiraFactory,
@@ -87,53 +84,20 @@ class JiraManager(Manager):
)
return repos
async def validate_request(
self, request: Request
) -> Tuple[bool, Optional[str], Optional[Dict]]:
"""Verify Jira webhook signature."""
signature_header = request.headers.get('x-hub-signature')
signature = signature_header.split('=')[1] if signature_header else None
body = await request.body()
payload = await request.json()
workspace_name = ''
def get_workspace_name_from_payload(self, payload: dict) -> str | None:
"""Extract workspace name from Jira webhook payload."""
if payload.get('webhookEvent') == 'comment_created':
selfUrl = payload.get('comment', {}).get('author', {}).get('self')
elif payload.get('webhookEvent') == 'jira:issue_updated':
selfUrl = payload.get('user', {}).get('self')
else:
workspace_name = ''
return None
if not selfUrl:
return None
parsedUrl = urlparse(selfUrl)
if parsedUrl.hostname:
workspace_name = parsedUrl.hostname
if not workspace_name:
logger.warning('[Jira] No workspace name found in webhook payload')
return False, None, None
if not signature:
logger.warning('[Jira] No signature found in webhook headers')
return False, None, None
workspace = await self.integration_store.get_workspace_by_name(workspace_name)
if not workspace:
logger.warning('[Jira] Could not identify workspace for webhook')
return False, None, None
if workspace.status != 'active':
logger.warning(f'[Jira] Workspace {workspace.id} is not active')
return False, None, None
webhook_secret = self.token_manager.decrypt_text(workspace.webhook_secret)
digest = hmac.new(webhook_secret.encode(), body, hashlib.sha256).hexdigest()
if hmac.compare_digest(signature, digest):
logger.info('[Jira] Webhook signature verified successfully')
return True, signature, payload
return False, None, None
return parsedUrl.hostname or None
def parse_webhook(self, message: Message) -> JobContext | None:
payload = message.message.get('payload', {})

View File

@@ -1,3 +1,5 @@
import hashlib
import hmac
import json
import os
import re
@@ -5,7 +7,7 @@ import uuid
from urllib.parse import urlparse
import requests
from fastapi import APIRouter, BackgroundTasks, HTTPException, Request, status
from fastapi import APIRouter, BackgroundTasks, Header, HTTPException, Request, status
from fastapi.responses import JSONResponse, RedirectResponse
from integrations.jira.jira_manager import JiraManager
from integrations.models import Message, SourceType
@@ -14,6 +16,7 @@ from pydantic import BaseModel, Field, field_validator
from server.auth.constants import JIRA_CLIENT_ID, JIRA_CLIENT_SECRET
from server.auth.saas_user_auth import SaasUserAuth
from server.auth.token_manager import TokenManager
from storage.jira_workspace import JiraWorkspace
from storage.redis import create_redis_client
from openhands.core.logger import openhands_logger as logger
@@ -122,6 +125,63 @@ jira_manager = JiraManager(token_manager)
redis_client = create_redis_client()
async def verify_jira_signature(body: bytes, signature: str, payload: dict):
"""
Verify Jira webhook signature.
Args:
body: Raw request body bytes
signature: Signature from x-hub-signature header (format: "sha256=<hash>")
payload: Parsed JSON payload from webhook
Raises:
HTTPException: 403 if signature verification fails or workspace is invalid
Returns:
None (raises exception on failure)
"""
if not signature:
raise HTTPException(
status_code=403, detail='x-hub-signature header is missing!'
)
workspace_name = jira_manager.get_workspace_name_from_payload(payload)
if workspace_name is None:
logger.warning('[Jira] No workspace name found in webhook payload')
raise HTTPException(
status_code=403, detail='Workspace name not found in payload'
)
workspace: (
JiraWorkspace | None
) = await jira_manager.integration_store.get_workspace_by_name(workspace_name)
if workspace is None:
logger.warning(f'[Jira] Could not identify workspace {workspace_name}')
raise HTTPException(status_code=403, detail='Unidentified workspace')
if workspace.status != 'active':
logger.warning(
'[Jira] Workspace is inactive',
extra={
'jira_workspace_id': workspace.id,
'parsed_workspace_name': workspace.name,
'status': workspace.status,
},
)
raise HTTPException(status_code=403, detail='Workspace is inactive')
webhook_secret = token_manager.decrypt_text(workspace.webhook_secret)
expected_signature = hmac.new(
webhook_secret.encode(), body, hashlib.sha256
).hexdigest()
if not hmac.compare_digest(expected_signature, signature):
raise HTTPException(status_code=403, detail="Request signatures didn't match!")
async def _handle_workspace_link_creation(
user_id: str, jira_user_id: str, target_workspace: str
):
@@ -216,6 +276,7 @@ async def _validate_workspace_update_permissions(user_id: str, target_workspace:
async def jira_events(
request: Request,
background_tasks: BackgroundTasks,
x_hub_signature: str = Header(None),
):
"""Handle Jira webhook events."""
# Check if Jira webhooks are enabled
@@ -227,13 +288,15 @@ async def jira_events(
)
try:
signature_valid, signature, payload = await jira_manager.validate_request(
request
)
parts = x_hub_signature.split('=', 1)
if not (len(parts) == 2 and parts[1]):
raise HTTPException(status_code=403, detail='Malformed x-hub-signature!')
if not signature_valid:
logger.warning('[Jira] Invalid webhook signature')
raise HTTPException(status_code=403, detail='Invalid webhook signature!')
signature = parts[1]
body = await request.body()
payload = await request.json()
await verify_jira_signature(body, signature, payload)
# Check for duplicate requests using Redis
key = f'jira:{signature}'

View File

@@ -118,9 +118,7 @@ class JiraIntegrationStore:
.first()
)
async def get_workspace_by_name(
self, workspace_name: str
) -> Optional[JiraWorkspace]:
async def get_workspace_by_name(self, workspace_name: str) -> JiraWorkspace | None:
"""Retrieve workspace by name."""
with session_maker() as session:
return (

View File

@@ -2,13 +2,9 @@
Unit tests for JiraManager.
"""
import hashlib
import hmac
import json
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from fastapi import Request
from integrations.jira.jira_manager import JiraManager
from integrations.jira.jira_types import JiraViewInterface
from integrations.jira.jira_view import (
@@ -124,124 +120,83 @@ class TestGetRepositories:
mock_client.get_repositories.assert_called_once()
class TestValidateRequest:
"""Test webhook request validation."""
class TestGetWorkspaceNameFromPayload:
"""Test workspace name extraction from webhook payload."""
@pytest.mark.asyncio
async def test_validate_request_success(
def test_get_workspace_name_from_comment_created_payload(
self,
jira_manager,
mock_token_manager,
sample_jira_workspace,
sample_comment_webhook_payload,
):
"""Test successful webhook validation."""
# Setup mocks
mock_token_manager.decrypt_text.return_value = 'test_secret'
jira_manager.integration_store.get_workspace_by_name.return_value = (
sample_jira_workspace
"""Test extracting workspace name from comment_created webhook."""
workspace_name = jira_manager.get_workspace_name_from_payload(
sample_comment_webhook_payload
)
# Create mock request
body = json.dumps(sample_comment_webhook_payload).encode()
signature = hmac.new('test_secret'.encode(), body, hashlib.sha256).hexdigest()
assert workspace_name == 'test.atlassian.net'
mock_request = MagicMock(spec=Request)
mock_request.headers = {'x-hub-signature': f'sha256={signature}'}
mock_request.body = AsyncMock(return_value=body)
mock_request.json = AsyncMock(return_value=sample_comment_webhook_payload)
is_valid, returned_signature, payload = await jira_manager.validate_request(
mock_request
)
assert is_valid is True
assert returned_signature == signature
assert payload == sample_comment_webhook_payload
@pytest.mark.asyncio
async def test_validate_request_missing_signature(
self, jira_manager, sample_comment_webhook_payload
):
"""Test webhook validation with missing signature."""
mock_request = MagicMock(spec=Request)
mock_request.headers = {}
mock_request.body = AsyncMock(return_value=b'{}')
mock_request.json = AsyncMock(return_value=sample_comment_webhook_payload)
is_valid, signature, payload = await jira_manager.validate_request(mock_request)
assert is_valid is False
assert signature is None
assert payload is None
@pytest.mark.asyncio
async def test_validate_request_workspace_not_found(
self, jira_manager, sample_comment_webhook_payload
):
"""Test webhook validation when workspace is not found."""
jira_manager.integration_store.get_workspace_by_name.return_value = None
mock_request = MagicMock(spec=Request)
mock_request.headers = {'x-hub-signature': 'sha256=test_signature'}
mock_request.body = AsyncMock(return_value=b'{}')
mock_request.json = AsyncMock(return_value=sample_comment_webhook_payload)
is_valid, signature, payload = await jira_manager.validate_request(mock_request)
assert is_valid is False
assert signature is None
assert payload is None
@pytest.mark.asyncio
async def test_validate_request_workspace_inactive(
def test_get_workspace_name_from_issue_updated_payload(
self,
jira_manager,
mock_token_manager,
sample_jira_workspace,
sample_comment_webhook_payload,
sample_issue_update_webhook_payload,
):
"""Test webhook validation when workspace is inactive."""
sample_jira_workspace.status = 'inactive'
jira_manager.integration_store.get_workspace_by_name.return_value = (
sample_jira_workspace
"""Test extracting workspace name from jira:issue_updated webhook."""
workspace_name = jira_manager.get_workspace_name_from_payload(
sample_issue_update_webhook_payload
)
mock_request = MagicMock(spec=Request)
mock_request.headers = {'x-hub-signature': 'sha256=test_signature'}
mock_request.body = AsyncMock(return_value=b'{}')
mock_request.json = AsyncMock(return_value=sample_comment_webhook_payload)
assert workspace_name == 'jira.company.com'
is_valid, signature, payload = await jira_manager.validate_request(mock_request)
assert is_valid is False
assert signature is None
assert payload is None
@pytest.mark.asyncio
async def test_validate_request_invalid_signature(
def test_get_workspace_name_from_unknown_event(
self,
jira_manager,
mock_token_manager,
sample_jira_workspace,
sample_comment_webhook_payload,
):
"""Test webhook validation with invalid signature."""
mock_token_manager.decrypt_text.return_value = 'test_secret'
jira_manager.integration_store.get_workspace_by_name.return_value = (
sample_jira_workspace
)
"""Test extracting workspace name from unknown webhook event."""
payload = {
'webhookEvent': 'unknown_event',
'some_data': {'self': 'https://example.atlassian.net/rest/api/2/something'},
}
mock_request = MagicMock(spec=Request)
mock_request.headers = {'x-hub-signature': 'sha256=invalid_signature'}
mock_request.body = AsyncMock(return_value=b'{}')
mock_request.json = AsyncMock(return_value=sample_comment_webhook_payload)
workspace_name = jira_manager.get_workspace_name_from_payload(payload)
is_valid, signature, payload = await jira_manager.validate_request(mock_request)
assert workspace_name is None
assert is_valid is False
assert signature is None
assert payload is None
def test_get_workspace_name_with_missing_author_self(
self,
jira_manager,
):
"""Test extracting workspace name when author self URL is missing."""
payload = {
'webhookEvent': 'comment_created',
'comment': {
'body': 'Test comment',
'author': {
'emailAddress': 'user@test.com',
'displayName': 'Test User',
},
},
}
workspace_name = jira_manager.get_workspace_name_from_payload(payload)
assert workspace_name is None
def test_get_workspace_name_with_missing_user_self(
self,
jira_manager,
):
"""Test extracting workspace name when user self URL is missing."""
payload = {
'webhookEvent': 'jira:issue_updated',
'user': {
'emailAddress': 'user@test.com',
'displayName': 'Test User',
},
}
workspace_name = jira_manager.get_workspace_name_from_payload(payload)
assert workspace_name is None
class TestParseWebhook:

View File

@@ -1,3 +1,5 @@
import hashlib
import hmac
import json
from datetime import datetime
from unittest.mock import AsyncMock, MagicMock, patch
@@ -19,6 +21,7 @@ from server.routes.integration.jira import (
jira_events,
unlink_workspace,
validate_workspace_integration,
verify_jira_signature,
)
@@ -61,25 +64,35 @@ def mock_user_auth():
@pytest.mark.asyncio
@patch('server.routes.integration.jira.jira_manager', new_callable=AsyncMock)
@patch('server.routes.integration.jira.verify_jira_signature', new_callable=AsyncMock)
@patch('server.routes.integration.jira.redis_client', new_callable=MagicMock)
async def test_jira_events_invalid_signature(mock_redis, mock_manager, mock_request):
async def test_jira_events_invalid_signature(mock_redis, mock_verify, mock_request):
with patch('server.routes.integration.jira.JIRA_WEBHOOKS_ENABLED', True):
mock_manager.validate_request.return_value = (False, None, None)
mock_request.body = AsyncMock(return_value=b'{}')
mock_request.json = AsyncMock(return_value={})
mock_verify.side_effect = HTTPException(
status_code=403, detail="Request signatures didn't match!"
)
with pytest.raises(HTTPException) as exc_info:
await jira_events(mock_request, MagicMock())
await jira_events(
mock_request, MagicMock(), x_hub_signature='sha256=invalid'
)
assert exc_info.value.status_code == 403
assert exc_info.value.detail == 'Invalid webhook signature!'
assert exc_info.value.detail == "Request signatures didn't match!"
@pytest.mark.asyncio
@patch('server.routes.integration.jira.jira_manager', new_callable=AsyncMock)
@patch('server.routes.integration.jira.verify_jira_signature', new_callable=AsyncMock)
@patch('server.routes.integration.jira.redis_client')
async def test_jira_events_duplicate_request(mock_redis, mock_manager, mock_request):
async def test_jira_events_duplicate_request(mock_redis, mock_verify, mock_request):
with patch('server.routes.integration.jira.JIRA_WEBHOOKS_ENABLED', True):
mock_manager.validate_request.return_value = (True, 'sig123', 'payload')
mock_request.body = AsyncMock(return_value=b'{}')
mock_request.json = AsyncMock(return_value={})
mock_verify.return_value = None
mock_redis.exists.return_value = True
response = await jira_events(mock_request, MagicMock())
response = await jira_events(
mock_request, MagicMock(), x_hub_signature='sha256=sig123'
)
assert response.status_code == 200
body = json.loads(response.body)
assert body['success'] is True
@@ -348,18 +361,21 @@ class TestJiraLinkCreateValidation:
# Test jira_events error scenarios
@pytest.mark.asyncio
@patch('server.routes.integration.jira.jira_manager', new_callable=AsyncMock)
@patch('server.routes.integration.jira.verify_jira_signature', new_callable=AsyncMock)
@patch('server.routes.integration.jira.redis_client', new_callable=MagicMock)
async def test_jira_events_processing_success(mock_redis, mock_manager, mock_request):
async def test_jira_events_processing_success(
mock_redis, mock_verify, mock_manager, mock_request
):
with patch('server.routes.integration.jira.JIRA_WEBHOOKS_ENABLED', True):
mock_manager.validate_request.return_value = (
True,
'sig123',
{'test': 'payload'},
)
mock_request.body = AsyncMock(return_value=b'{"test": "payload"}')
mock_request.json = AsyncMock(return_value={'test': 'payload'})
mock_verify.return_value = None
mock_redis.exists.return_value = False
background_tasks = MagicMock()
response = await jira_events(mock_request, background_tasks)
response = await jira_events(
mock_request, background_tasks, x_hub_signature='sha256=sig123'
)
assert response.status_code == 200
body = json.loads(response.body)
@@ -369,19 +385,241 @@ async def test_jira_events_processing_success(mock_redis, mock_manager, mock_req
@pytest.mark.asyncio
@patch('server.routes.integration.jira.jira_manager', new_callable=AsyncMock)
@patch('server.routes.integration.jira.verify_jira_signature', new_callable=AsyncMock)
@patch('server.routes.integration.jira.redis_client', new_callable=MagicMock)
async def test_jira_events_general_exception(mock_redis, mock_manager, mock_request):
async def test_jira_events_general_exception(mock_redis, mock_verify, mock_request):
with patch('server.routes.integration.jira.JIRA_WEBHOOKS_ENABLED', True):
mock_manager.validate_request.side_effect = Exception('Unexpected error')
mock_request.body = AsyncMock(side_effect=Exception('Unexpected error'))
mock_request.json = AsyncMock(return_value={})
response = await jira_events(mock_request, MagicMock())
response = await jira_events(
mock_request, MagicMock(), x_hub_signature='sha256=sig123'
)
assert response.status_code == 500
body = json.loads(response.body)
assert 'Internal server error processing webhook' in body['error']
# Test verify_jira_signature
class TestVerifyJiraSignature:
"""Test Jira webhook signature verification."""
@pytest.fixture
def sample_payload(self):
"""Sample webhook payload with comment_created event."""
return {
'webhookEvent': 'comment_created',
'comment': {
'body': 'Test comment @openhands',
'author': {
'emailAddress': 'user@test.com',
'displayName': 'Test User',
'self': 'https://test.atlassian.net/rest/api/2/user?accountId=123',
},
},
'issue': {
'id': '12345',
'key': 'TEST-123',
'self': 'https://test.atlassian.net/rest/api/2/issue/12345',
},
}
@pytest.fixture
def mock_workspace(self):
"""Create a mock workspace."""
workspace = MagicMock()
workspace.id = 1
workspace.name = 'test.atlassian.net'
workspace.status = 'active'
workspace.webhook_secret = 'encrypted_secret'
return workspace
@pytest.mark.asyncio
@pytest.mark.parametrize(
'signature,expected_detail',
[
(None, 'x-hub-signature header is missing!'),
('', 'x-hub-signature header is missing!'),
],
ids=['signature_none', 'signature_empty'],
)
async def test_missing_signature(self, signature, expected_detail, sample_payload):
"""Test that missing or empty signature raises HTTPException."""
body = json.dumps(sample_payload).encode()
with pytest.raises(HTTPException) as exc_info:
await verify_jira_signature(body, signature, sample_payload)
assert exc_info.value.status_code == 403
assert exc_info.value.detail == expected_detail
@pytest.mark.asyncio
@pytest.mark.parametrize(
'payload',
[
{'webhookEvent': 'unknown_event'},
{'webhookEvent': 'comment_created', 'comment': {}},
{'webhookEvent': 'comment_created', 'comment': {'author': {}}},
{'webhookEvent': 'jira:issue_updated', 'user': {}},
{},
],
ids=[
'unknown_event',
'missing_author',
'missing_self_url',
'issue_updated_missing_self',
'empty_payload',
],
)
@patch('server.routes.integration.jira.jira_manager')
async def test_workspace_name_not_found(self, mock_manager, payload):
"""Test that missing workspace name in payload raises HTTPException."""
mock_manager.get_workspace_name_from_payload.return_value = None
body = json.dumps(payload).encode()
with pytest.raises(HTTPException) as exc_info:
await verify_jira_signature(body, 'valid_signature', payload)
assert exc_info.value.status_code == 403
assert exc_info.value.detail == 'Workspace name not found in payload'
@pytest.mark.asyncio
@patch('server.routes.integration.jira.jira_manager')
async def test_workspace_not_found_in_database(self, mock_manager, sample_payload):
"""Test that workspace not found in database raises HTTPException."""
mock_manager.get_workspace_name_from_payload.return_value = 'test.atlassian.net'
mock_manager.integration_store.get_workspace_by_name = AsyncMock(
return_value=None
)
body = json.dumps(sample_payload).encode()
with pytest.raises(HTTPException) as exc_info:
await verify_jira_signature(body, 'valid_signature', sample_payload)
assert exc_info.value.status_code == 403
assert exc_info.value.detail == 'Unidentified workspace'
@pytest.mark.asyncio
@pytest.mark.parametrize(
'workspace_status',
['inactive', 'disabled', 'pending'],
ids=['inactive', 'disabled', 'pending'],
)
@patch('server.routes.integration.jira.jira_manager')
async def test_workspace_not_active(
self, mock_manager, workspace_status, sample_payload, mock_workspace
):
"""Test that inactive workspace raises HTTPException."""
mock_workspace.status = workspace_status
mock_manager.get_workspace_name_from_payload.return_value = 'test.atlassian.net'
mock_manager.integration_store.get_workspace_by_name = AsyncMock(
return_value=mock_workspace
)
body = json.dumps(sample_payload).encode()
with pytest.raises(HTTPException) as exc_info:
await verify_jira_signature(body, 'valid_signature', sample_payload)
assert exc_info.value.status_code == 403
assert exc_info.value.detail == 'Workspace is inactive'
@pytest.mark.asyncio
@patch('server.routes.integration.jira.token_manager')
@patch('server.routes.integration.jira.jira_manager')
async def test_signature_mismatch(
self, mock_manager, mock_token_mgr, sample_payload, mock_workspace
):
"""Test that signature mismatch raises HTTPException."""
mock_manager.get_workspace_name_from_payload.return_value = 'test.atlassian.net'
mock_manager.integration_store.get_workspace_by_name = AsyncMock(
return_value=mock_workspace
)
mock_token_mgr.decrypt_text.return_value = 'webhook_secret'
body = json.dumps(sample_payload).encode()
with pytest.raises(HTTPException) as exc_info:
await verify_jira_signature(body, 'invalid_signature', sample_payload)
assert exc_info.value.status_code == 403
assert exc_info.value.detail == "Request signatures didn't match!"
@pytest.mark.asyncio
@patch('server.routes.integration.jira.token_manager')
@patch('server.routes.integration.jira.jira_manager')
async def test_valid_signature(
self, mock_manager, mock_token_mgr, sample_payload, mock_workspace
):
"""Test that valid signature passes verification."""
webhook_secret = 'webhook_secret'
mock_manager.get_workspace_name_from_payload.return_value = 'test.atlassian.net'
mock_manager.integration_store.get_workspace_by_name = AsyncMock(
return_value=mock_workspace
)
mock_token_mgr.decrypt_text.return_value = webhook_secret
body = json.dumps(sample_payload).encode()
valid_signature = hmac.new(
webhook_secret.encode(), body, hashlib.sha256
).hexdigest()
# Should not raise any exception
result = await verify_jira_signature(body, valid_signature, sample_payload)
assert result is None
@pytest.mark.asyncio
@pytest.mark.parametrize(
'event_type,payload_key,author_key',
[
('comment_created', 'comment', 'author'),
('jira:issue_updated', 'user', None),
],
ids=['comment_created', 'issue_updated'],
)
@patch('server.routes.integration.jira.token_manager')
@patch('server.routes.integration.jira.jira_manager')
async def test_valid_signature_different_events(
self,
mock_manager,
mock_token_mgr,
event_type,
payload_key,
author_key,
mock_workspace,
):
"""Test valid signature verification for different webhook events."""
webhook_secret = 'webhook_secret'
mock_manager.get_workspace_name_from_payload.return_value = 'test.atlassian.net'
mock_manager.integration_store.get_workspace_by_name = AsyncMock(
return_value=mock_workspace
)
mock_token_mgr.decrypt_text.return_value = webhook_secret
if event_type == 'comment_created':
payload = {
'webhookEvent': event_type,
'comment': {
'body': 'Test',
'author': {
'self': 'https://test.atlassian.net/rest/api/2/user?id=1'
},
},
}
else:
payload = {
'webhookEvent': event_type,
'user': {'self': 'https://test.atlassian.net/rest/api/2/user?id=1'},
}
body = json.dumps(payload).encode()
valid_signature = hmac.new(
webhook_secret.encode(), body, hashlib.sha256
).hexdigest()
result = await verify_jira_signature(body, valid_signature, payload)
assert result is None
# Test create_jira_workspace error scenarios
@pytest.mark.asyncio
@patch('server.routes.integration.jira.get_user_auth')