Skip to content

Commit 6fe5a6f

Browse files
committed
feat: Only give auth errors if the server does
Branch: InitialCLI Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
1 parent 5efcf46 commit 6fe5a6f

2 files changed

Lines changed: 43 additions & 12 deletions

File tree

cforge/common.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,10 @@ def make_authenticated_request(
161161
) -> Dict[str, Any]:
162162
"""Make an authenticated HTTP request to the gateway API.
163163
164+
Supports both authenticated and unauthenticated servers. Will attempt
165+
the request without authentication if no token is configured, and only
166+
fail if the server requires authentication.
167+
164168
Args:
165169
method: HTTP method (GET, POST, etc.)
166170
url: URL path for the request
@@ -171,25 +175,29 @@ def make_authenticated_request(
171175
JSON response from the API
172176
173177
Raises:
174-
AuthenticationError: If no authentication is configured
178+
AuthenticationError: If the server requires authentication but none is configured
175179
CLIError: If the API request fails
176180
"""
177181
token = get_auth_token()
178-
if not token:
179-
raise AuthenticationError("No authentication configured. Set MCPGATEWAY_BEARER_TOKEN environment variable or run cforge login.")
180182

181183
headers = {"Content-Type": "application/json"}
182-
if token.startswith("Basic "):
183-
headers["Authorization"] = token
184-
else:
185-
headers["Authorization"] = f"Bearer {token}"
184+
# Only add Authorization header if a token is available
185+
if token:
186+
if token.startswith("Basic "):
187+
headers["Authorization"] = token
188+
else:
189+
headers["Authorization"] = f"Bearer {token}"
186190

187191
gateway_url = f"http://{get_settings().host}:{get_settings().port}"
188192
full_url = f"{gateway_url}{url}"
189193

190194
try:
191195
response = requests.request(method=method, url=full_url, json=json_data, params=params, headers=headers)
192196

197+
# Handle authentication errors specifically
198+
if response.status_code in (401, 403):
199+
raise AuthenticationError("Authentication required but not configured. " "Set MCPGATEWAY_BEARER_TOKEN environment variable or run 'cforge login'.")
200+
193201
if response.status_code >= 400:
194202
raise CLIError(f"API request failed ({response.status_code}): {response.text}")
195203

tests/test_common.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,37 @@ def test_authentication_error(self) -> None:
137137
class TestMakeAuthenticatedRequest:
138138
"""Tests for make_authenticated_request function using a server mock."""
139139

140-
def test_request_no_auth_raises_error(self, mock_client) -> None:
141-
"""Test that request without auth raises AuthenticationError."""
140+
def test_request_no_auth_raises_error_when_server_requires_it(self, mock_settings) -> None:
141+
"""Test that request without auth raises AuthenticationError when server requires it."""
142142
# Ensure no token is available
143143
with patch("cforge.common.load_token", return_value=None):
144-
with pytest.raises(AuthenticationError) as exc_info:
145-
make_authenticated_request("GET", "/test")
144+
# Mock a 401 response from server (authentication required)
145+
mock_response = Mock()
146+
mock_response.status_code = 401
147+
mock_response.text = "Unauthorized"
146148

147-
assert "No authentication configured" in str(exc_info.value)
149+
with patch("cforge.common.requests.request", return_value=mock_response):
150+
with pytest.raises(AuthenticationError) as exc_info:
151+
make_authenticated_request("GET", "/test")
152+
153+
assert "Authentication required but not configured" in str(exc_info.value)
154+
155+
def test_request_without_auth_succeeds_on_unauthenticated_server(self, mock_settings) -> None:
156+
"""Test that request without auth succeeds when server doesn't require it."""
157+
# Ensure no token is available
158+
with patch("cforge.common.load_token", return_value=None):
159+
# Mock a successful response from server (no auth required)
160+
mock_response = Mock()
161+
mock_response.status_code = 200
162+
mock_response.json.return_value = {"result": "success"}
163+
164+
with patch("cforge.common.requests.request", return_value=mock_response) as mock_req:
165+
result = make_authenticated_request("GET", "/test")
166+
167+
# Verify the request was made without Authorization header
168+
call_args = mock_req.call_args
169+
assert "Authorization" not in call_args[1]["headers"]
170+
assert result == {"result": "success"}
148171

149172
def test_request_with_bearer_token(self, mock_client) -> None:
150173
"""Test successful request with Bearer token."""

0 commit comments

Comments
 (0)