openstack/ironic-python-agent
Code Issues Proposed changes

Fix: Remove legacy agent_token fallback code

Browse Source 
Removes the very old fallback code which would allow a
client to GET /v1/commands unauthenticated very early
in an IPA run.

This cleans up the checks entirely, making the code
suitable for a world that always has an agent token.

Assisted-by: Claude Code (Sonnet)
Closes-Bug: #2160196
Change-Id: Ia869cd1c796660cbd7e6656a5a2dd49d89be30b0
Signed-off-by: Jay Faulkner <jay@jvf.cc>
This commit is contained in:
Jay Faulkner
2026-07-15 16:19:47 +00:00
parent bd3c0cf1be
commit fa072d50b4
6 changed files with 49 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ironic_python_agent/api/app.py
+2 -12
View File
@@ -155,7 +155,6 @@ class Application(object):
routing.Rule('/commands/', endpoint='run_command',
methods=['POST']),
])
self.security_get_token_support = False
def __call__(self, environ, start_response):
"""WSGI entry point."""
@@ -278,13 +277,7 @@ class Application(object):
def require_agent_token_for_command(func):
def wrapper(self, request, *args, **kwargs):
token = request.args.get('agent_token', None)
if token:
# TODO(TheJulia): At some point down the road, remove the
# self.security_get_token_support flag and use the same
# decorator for the api_run_command endpoint.
self.security_get_token_support = True
if (self.security_get_token_support
and not self.agent.validate_agent_token(token)):
if not self.agent.validate_agent_token(token):
raise http_exc.Unauthorized('Token invalid.')
return func(self, request, *args, **kwargs)
return wrapper
@@ -306,15 +299,12 @@ class Application(object):
return jsonify(result)
@require_agent_token_for_command
def api_run_command(self, request):
body = request.get_json(force=True)
if ('name' not in body or 'params' not in body
or not isinstance(body['params'], dict)):
raise http_exc.BadRequest('Missing or invalid name or params')
token = request.args.get('agent_token', None)
if not self.agent.validate_agent_token(token):
raise http_exc.Unauthorized(
'Token invalid.')
with metrics_utils.get_metrics_logger(__name__).timer('run_command'):
result = self.agent.execute_command(body['name'], **body['params'])
wait = request.args.get('wait')
ironic_python_agent/tests/functional/base.py
+6 -2
View File
@@ -60,6 +60,7 @@ class FunctionalBase(test_base.BaseTestCase):
# Build a basic standalone agent using the config option defaults.
# 127.0.0.1:6835 is the fake Ironic client.
self.agent_token = '678123'
self.process = multiprocessing.Process(
target=_start_agent,
args=('http://127.0.0.1:6835',
@@ -72,7 +73,7 @@ class FunctionalBase(test_base.BaseTestCase):
300,
1,
True,
'678123'))
self.agent_token))
self.process.start()
self.addCleanup(self.process.terminate)
@@ -91,7 +92,7 @@ class FunctionalBase(test_base.BaseTestCase):
% (max_tries * sleep_time))
def request(self, method, path, expect_error=None, expect_json=True,
**kwargs):
with_token=True, **kwargs):
"""Send a request to the agent and verifies response.
:param method: type of request to send as a string
@@ -106,6 +107,9 @@ class FunctionalBase(test_base.BaseTestCase):
expect_error
:returns: the response object
"""
if with_token:
separator = '&' if '?' in path else '?'
path = f'{path}{separator}agent_token={self.agent_token}'
res = requests.request(method, 'http://localhost:%s/v1/%s' %
(self.test_port, path), **kwargs)
if expect_error is not None:
ironic_python_agent/tests/functional/test_commands.py
+7 -2
View File
@@ -26,6 +26,11 @@ class TestCommands(base.FunctionalBase):
node = {'uuid': '1', 'properties': {}, 'instance_info': {}}
def step_0_agent_token_required(self):
"""Validate that the agent token is required at startup."""
self.request(method='get', path='commands',
expect_error=401, with_token=False)
def step_1_get_empty_commands(self):
response = self.request('get', 'commands')
self.assertEqual({'commands': []}, response)
@@ -37,7 +42,7 @@ class TestCommands(base.FunctionalBase):
# success is required for steps 3 and 4 to succeed.
command = {'name': 'clean.get_clean_steps',
'params': {'node': self.node, 'ports': {}}}
response = self.request('post', 'commands/?agent_token=678123',
response = self.request('post', 'commands/',
json=command,
headers={'Content-Type': 'application/json'})
self.assertIsNone(response['command_error'])
@@ -64,7 +69,7 @@ class TestCommands(base.FunctionalBase):
def step_5_run_non_existent_command(self):
fake_command = {'name': 'bad_extension.fake_command', 'params': {}}
self.request('post', 'commands/?agent_token=678123',
self.request('post', 'commands/',
expect_error=404, json=fake_command)
def positive_get_post_command_steps(self):
ironic_python_agent/tests/functional/test_tls.py
+5 -1
View File
@@ -86,6 +86,7 @@ class TestTLSEnforcement(test_base.BaseTestCase):
'127.0.0.1'
)
self.agent_token = '678123'
# Start agent with TLS enabled
self.process = multiprocessing.Process(
target=_start_agent_with_tls,
@@ -99,7 +100,7 @@ class TestTLSEnforcement(test_base.BaseTestCase):
300,
1,
True,
'678123',
self.agent_token,
self.cert_file,
self.key_file))
self.process.start()
@@ -163,6 +164,9 @@ class TestTLSEnforcement(test_base.BaseTestCase):
session = requests.Session()
session.mount('https://', adapter)
separator = '&' if '?' in path else '?'
path = f'{path}{separator}agent_token={self.agent_token}'
# Disable cert verification at the requests level as well
res = session.request(
method,
ironic_python_agent/tests/unit/test_api.py
+17 -26
View File
@@ -331,46 +331,37 @@ class TestIronicAPI(ironic_agent_base.IronicAgentTest):
self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
self.assertEqual(0, self.mock_agent.get_command_result.call_count)
def test_get_command_locks_out_with_token(self):
"""Tests agent backwards compatibility and verifies upgrade lockout."""
def test_get_command_requires_token_without_one(self):
"""Tests LP#2160196: GET must not skip validation without a token.
GET endpoints must always call validate_agent_token, even when
the request carries no agent_token at all, instead of only
enforcing validation once some earlier GET happened to supply
one.
"""
cmd_result = base.SyncCommandResult('do_things',
{'key': 'value'},
True,
{'test': 'result'})
cmd_result.serialize()
self.mock_agent.get_command_result.return_value = cmd_result
agent_token = str('0123456789' * 10)
self.mock_agent.validate_agent_token.return_value = False
# Backwards compatible operation check.
response = self.get_json(
'/commands/abc123')
self.assertEqual(200, response.status_code)
self.assertFalse(self.app.security_get_token_support)
self.assertEqual(1, self.mock_agent.get_command_result.call_count)
self.mock_agent.reset_mock()
'/commands/abc123', expect_errors=True)
# Check with a newer ironic sending an agent_token upon the command.
# For context, in this case the token is wrong intentionally.
# It doesn't have to be right, but what we're testing is the
# submission of any value triggers the lockout
response = self.get_json(
'/commands/abc123?agent_token=%s' % agent_token,
expect_errors=True)
self.assertTrue(self.app.security_get_token_support)
self.assertEqual(401, response.status_code)
self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
self.assertEqual(0, self.mock_agent.get_command_result.call_count)
# Verifying the lockout is now being enforced and that agent token
# is now required by the agent.
response = self.get_json(
'/commands/abc123', expect_errors=True)
self.assertTrue(self.app.security_get_token_support)
def test_list_commands_requires_token_without_one(self):
"""Tests LP#2160196: GET must not skip validation without a token."""
self.mock_agent.validate_agent_token.return_value = False
response = self.get_json('/commands', expect_errors=True)
self.assertEqual(401, response.status_code)
self.assertEqual(0, self.mock_agent.get_command_result.call_count)
# Verify we still called validate_agent_token
self.assertEqual(2, self.mock_agent.validate_agent_token.call_count)
self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
self.assertEqual(0, self.mock_agent.list_command_results.call_count)
def test_execute_agent_command_with_token(self):
agent_token = str('0123456789' * 10)
releasenotes/notes/fix-late-binding-agent-token-gate-295e0a64e7c34090.yaml
+12
View File
@@ -0,0 +1,12 @@
---
security:
- |
The ``GET /v1/commands`` and ``GET /v1/commands/<uuid>`` API endpoints
now always validate the ``agent_token``, matching the ``POST``
endpoint. Previously, these ``GET`` endpoints only started enforcing
the token once a ``GET`` request happened to supply one, so if that
never occurred, e.g. in typical ``POST``/heartbeat-driven
deployments, the endpoints could remain unauthenticated indefinitely.
See `bug 2160196
<https://bugs.launchpad.net/ironic-python-agent/+bug/2160196>`_
for details.