Skip to content

Commit 8302194

Browse files
authored
Add Tests for HTTP Authentication and SSL (#4)
* add testing for api key authentication * test for https * adding tests for http authentication and SSL * some python formatting configurations to ruff * pre-commit
1 parent 2eac874 commit 8302194

File tree

14 files changed

+934
-491
lines changed

14 files changed

+934
-491
lines changed

.config/ruff.toml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Ruff configuration for robot_mcp
2+
# https://docs.astral.sh/ruff/configuration/
3+
4+
# Use 2 spaces for indentation (ROS2 Python convention)
5+
indent-width = 2
6+
7+
# Line length (default is 88, can adjust if needed)
8+
line-length = 100
9+
10+
[format]
11+
# Use spaces for indentation
12+
indent-style = "space"
13+
14+
# Use double quotes for strings
15+
quote-style = "double"
16+
17+
[lint]
18+
# Enable pycodestyle (E), Pyflakes (F), and isort (I) rules
19+
select = ["E", "F", "I"]
20+
21+
# Ignore specific rules if needed
22+
ignore = []

.pre-commit-config.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ repos:
3232
rev: v0.11.5
3333
hooks:
3434
- id: ruff-format
35+
args: [--config, .config/ruff.toml]
3536
- id: ruff
36-
args: [--fix]
37+
args: [--fix, --config, .config/ruff.toml]
3738

3839
# C++ formatting
3940
- repo: https://github.com/pre-commit/mirrors-clang-format

robot_mcp_server/CMakeLists.txt

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -91,18 +91,19 @@ install(DIRECTORY include/
9191
DESTINATION include
9292
)
9393

94-
# Install test configuration files
95-
install(DIRECTORY test/launch_tests/
96-
DESTINATION share/${PROJECT_NAME}/test/launch_tests
97-
FILES_MATCHING
98-
PATTERN "*.yaml"
99-
)
100-
10194
if(BUILD_TESTING)
10295
# Find robot_mcp_test package
10396
find_package(robot_mcp_test REQUIRED)
10497
find_package(launch_testing_ament_cmake REQUIRED)
10598

99+
# Install test configuration files and test utilities
100+
install(DIRECTORY test/launch_tests/
101+
DESTINATION share/${PROJECT_NAME}/test/launch_tests
102+
FILES_MATCHING
103+
PATTERN "*.yaml"
104+
PATTERN "test_utils.py"
105+
)
106+
106107
# Include the custom test function
107108
include(${robot_mcp_test_DIR}/add_robot_mcp_test.cmake)
108109

@@ -120,18 +121,12 @@ if(BUILD_TESTING)
120121
${PROJECT_NAME}
121122
)
122123

123-
# Add launch tests
124-
add_launch_test(
125-
test/launch_tests/test_complete_config.py
126-
)
127-
128-
add_launch_test(
129-
test/launch_tests/test_minimal_config.py
130-
)
131-
132-
add_launch_test(
133-
test/launch_tests/test_http_integration.py
134-
)
124+
# Add launch tests with test_utils support
125+
add_robot_mcp_launch_test(test/launch_tests/test_complete_config.py)
126+
add_robot_mcp_launch_test(test/launch_tests/test_minimal_config.py)
127+
add_robot_mcp_launch_test(test/launch_tests/test_http_integration.py)
128+
add_robot_mcp_launch_test(test/launch_tests/test_authentication.py)
129+
add_robot_mcp_launch_test(test/launch_tests/test_https.py)
135130
endif()
136131

137132
ament_export_targets(${PROJECT_NAME}Targets HAS_LIBRARY_TARGET)

robot_mcp_server/src/mcp_config/config_parser.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,10 @@ ServerConfig ConfigParser::parseServerConfig(rclcpp_lifecycle::LifecycleNode::Sh
9191
config.bond_timeout = node->declare_parameter("server.bond_timeout", config.bond_timeout);
9292
config.bond_heartbeat_period = node->declare_parameter("server.bond_heartbeat_period", config.bond_heartbeat_period);
9393

94-
// Optional API key
95-
if (node->has_parameter("server.api_key")) {
96-
config.api_key = node->get_parameter("server.api_key").as_string();
94+
// Optional API key - declare with empty string default
95+
std::string api_key_str = node->declare_parameter("server.api_key", std::string(""));
96+
if (!api_key_str.empty()) {
97+
config.api_key = api_key_str;
9798
}
9899

99100
return config;

robot_mcp_server/src/robot_mcp_server_node.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,12 @@ CallbackReturn MCPServerNode::on_deactivate(const rclcpp_lifecycle::State & /*st
149149

150150
// Stop bond heartbeat
151151
if (bond_) {
152-
bond_->breakBond();
152+
try {
153+
bond_->breakBond();
154+
} catch (const std::exception & e) {
155+
// Bond breaking may fail if RCL context is already shut down during SIGINT
156+
RCLCPP_DEBUG(get_logger(), "Bond break failed (context may be shut down): %s", e.what());
157+
}
153158
bond_.reset();
154159
RCLCPP_INFO(get_logger(), "Bond heartbeat stopped");
155160
}
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2025-present WATonomous. All rights reserved.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
"""Integration tests for MCP server authentication."""
17+
18+
import time
19+
import unittest
20+
21+
import launch
22+
import launch.actions
23+
import launch_ros.actions
24+
import launch_testing.actions
25+
import pytest
26+
import requests
27+
28+
29+
@pytest.mark.launch_test
30+
def generate_test_description():
31+
"""Launch the MCP server with authentication enabled."""
32+
# Test API key
33+
test_api_key = "test_secret_key_12345"
34+
35+
mcp_server_node = launch_ros.actions.LifecycleNode(
36+
package="robot_mcp_server",
37+
executable="robot_mcp_server_node",
38+
name="mcp_http_server",
39+
namespace="",
40+
parameters=[
41+
{
42+
"server.host": "127.0.0.2",
43+
"server.port": 18081,
44+
"server.api_key": test_api_key,
45+
"server.enable_https": False,
46+
}
47+
],
48+
output="screen",
49+
)
50+
51+
# Use nav2_lifecycle_manager to automatically activate the node
52+
lifecycle_manager = launch_ros.actions.Node(
53+
package="nav2_lifecycle_manager",
54+
executable="lifecycle_manager",
55+
name="test_lifecycle_manager",
56+
parameters=[{"autostart": True, "node_names": ["mcp_http_server"], "bond_timeout": 4.0}],
57+
output="screen",
58+
)
59+
60+
return launch.LaunchDescription(
61+
[
62+
mcp_server_node,
63+
lifecycle_manager,
64+
launch_testing.actions.ReadyToTest(),
65+
]
66+
)
67+
68+
69+
class TestAuthentication(unittest.TestCase):
70+
"""Active tests for authentication."""
71+
72+
BASE_URL = "http://127.0.0.2:18081"
73+
VALID_API_KEY = "test_secret_key_12345"
74+
75+
@classmethod
76+
def setUpClass(cls):
77+
"""Wait for server to be ready."""
78+
time.sleep(5)
79+
80+
def test_valid_api_key(self):
81+
"""Test request with valid API key succeeds."""
82+
payload = {"jsonrpc": "2.0", "method": "test", "id": 1}
83+
headers = {"Authorization": f"Bearer {self.VALID_API_KEY}"}
84+
85+
response = requests.post(f"{self.BASE_URL}/mcp", json=payload, headers=headers)
86+
87+
assert response.status_code == 200
88+
data = response.json()
89+
assert "result" in data or "message" in data
90+
91+
def test_invalid_api_key(self):
92+
"""Test request with invalid API key is rejected."""
93+
payload = {"jsonrpc": "2.0", "method": "test", "id": 1}
94+
headers = {"Authorization": "Bearer wrong_key"}
95+
96+
response = requests.post(f"{self.BASE_URL}/mcp", json=payload, headers=headers)
97+
98+
assert response.status_code == 401
99+
data = response.json()
100+
assert "error" in data
101+
102+
def test_missing_authorization_header(self):
103+
"""Test request without Authorization header is rejected."""
104+
payload = {"jsonrpc": "2.0", "method": "test", "id": 1}
105+
106+
response = requests.post(f"{self.BASE_URL}/mcp", json=payload)
107+
108+
assert response.status_code == 401
109+
data = response.json()
110+
assert "error" in data
111+
112+
def test_malformed_authorization_header(self):
113+
"""Test request with malformed Authorization header is rejected."""
114+
payload = {"jsonrpc": "2.0", "method": "test", "id": 1}
115+
headers = {"Authorization": "InvalidFormat"}
116+
117+
response = requests.post(f"{self.BASE_URL}/mcp", json=payload, headers=headers)
118+
119+
assert response.status_code == 401
120+
data = response.json()
121+
assert "error" in data
122+
123+
def test_authorization_header_without_bearer(self):
124+
"""Test request with Authorization header but no Bearer prefix is rejected."""
125+
payload = {"jsonrpc": "2.0", "method": "test", "id": 1}
126+
headers = {"Authorization": self.VALID_API_KEY}
127+
128+
response = requests.post(f"{self.BASE_URL}/mcp", json=payload, headers=headers)
129+
130+
assert response.status_code == 401
131+
data = response.json()
132+
assert "error" in data
133+
134+
135+
@launch_testing.post_shutdown_test()
136+
class TestAuthenticationShutdown(unittest.TestCase):
137+
"""Post-shutdown tests."""
138+
139+
def test_exit_codes(self, proc_info):
140+
"""Check that all processes exited cleanly."""
141+
launch_testing.asserts.assertExitCodes(proc_info)

0 commit comments

Comments
 (0)