Skip to content

Commit f3549ff

Browse files
john-westcott-ivfostersethClaude
authored
[AAP-55305] Short circuiting claims processing if user is a super user (ansible#863)
## Description <!-- Mandatory: Provide a clear, concise description of the changes and their purpose --> - What is being changed? When running migrate service data gateway is put into a state that prevents querying the gateway for user/object info. During JWT authentication if the claims hash in the token doesn't match the claims hash in the service a call from the service to the gateway to get the claims (which then fails). With this patch, if a user has is_super_user we will just completely skip processing the claims data which will prevent this scenario and optimize authenticator for superusers in general. - Why is this change needed? See above. - How does this change address the issue? Short circuits the claims processing if user is superuser. ## Type of Change <!-- Mandatory: Check one or more boxes that apply --> - [X] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Test update - [ ] Refactoring (no functional changes) - [ ] Development environment change - [ ] Configuration change ## Self-Review Checklist <!-- These items help ensure quality - they complement our automated CI checks --> - [X] I have performed a self-review of my code - [X] I have added relevant comments to complex code sections - [X] I have updated documentation where needed - [X] I have considered the security impact of these changes - [X] I have considered performance implications - [X] I have thought about error handling and edge cases - [X] I have tested the changes in my local environment ## Testing Instructions <!-- Optional for test-only changes. Mandatory for all other changes --> <!-- Must be detailed enough for reviewers to reproduce --> ### Prerequisites <!-- List any specific setup required --> ### Steps to Test 1. 2. 3. ### Expected Results <!-- Describe what should happen after following the steps --> ## Additional Context <!-- Optional but helpful information --> ### Required Actions <!-- Check if changes require work in other areas --> <!-- Remove section if no external actions needed --> - [ ] Requires documentation updates <!-- API docs, feature docs, deployment guides --> - [ ] Requires downstream repository changes <!-- Specify repos: django-ansible-base, eda-server, etc. --> - [ ] Requires infrastructure/deployment changes <!-- CI/CD, installer updates, new services --> - [ ] Requires coordination with other teams <!-- UI team, platform services, infrastructure --> - [ ] Blocked by PR/MR: #XXX <!-- Reference blocking PRs/MRs with brief context --> ### Screenshots/Logs <!-- Add if relevant to demonstrate the changes --> --------- Co-authored-by: Seth Foster <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 92ea086 commit f3549ff

File tree

2 files changed

+120
-27
lines changed

2 files changed

+120
-27
lines changed

ansible_base/jwt_consumer/common/auth.py

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,23 @@
2323
logger = logging.getLogger("ansible_base.jwt_consumer.common.auth")
2424

2525

26+
class GatewayLockedException(Exception):
27+
"""
28+
Exception raised when the gateway is locked (returns 423 status).
29+
This typically happens when migrations are in progress.
30+
"""
31+
32+
pass
33+
34+
35+
class InvalidGatewayResponseException(Exception):
36+
"""
37+
Exception raised when the gateway response is not 200 or 423
38+
"""
39+
40+
pass
41+
42+
2643
# These fields are used to both map the user as well as to validate the JWT token
2744
default_mapped_user_fields = [
2845
"username",
@@ -272,9 +289,8 @@ def process_rbac_permissions(self):
272289

273290
# Claims hash mismatch - fetch from gateway
274291
logger.info(f"Claims hash mismatch for user {user_ansible_id}. JWT: {jwt_claims_hash}, Local: {local_claims_hash}. Fetching from gateway.")
275-
gateway_claims = self._fetch_jwt_claims_from_gateway(user_ansible_id)
276-
277-
if gateway_claims:
292+
try:
293+
gateway_claims = self._fetch_jwt_claims_from_gateway(user_ansible_id)
278294
# Extract claims structure from gateway response
279295
objects = gateway_claims.get('objects', {})
280296
object_roles = gateway_claims.get('object_roles', {})
@@ -285,32 +301,34 @@ def process_rbac_permissions(self):
285301

286302
# Update cache with the new hash
287303
self.cache.cache_claims_hash(user_ansible_id, jwt_claims_hash)
288-
else:
304+
except GatewayLockedException:
305+
if self.token.get('user_data', {}).get("is_superuser", False) is False:
306+
self.log_and_raise(
307+
_("User %(user_ansible_id)s is not a superuser and gateway is locked, denying access!"), {"user_ansible_id": user_ansible_id}
308+
)
309+
except Exception as e:
289310
self.log_and_raise(
290-
_("Unable to validate user permissions - gateway claims fetch failed for user %(user_ansible_id)s"), {"user_ansible_id": user_ansible_id}
311+
_("Unable to validate user permissions - gateway claims fetch or processing failed for user %(user_ansible_id)s: %(e)s"),
312+
{"user_ansible_id": user_ansible_id, "e": e},
291313
)
292314

293315
def _fetch_jwt_claims_from_gateway(self, user_ansible_id: str) -> Optional[dict]:
294316
"""
295317
Fetch JWT claims from the gateway endpoint using resource server client
296318
"""
297-
try:
298-
# Use the resource server client to make the request
299-
client = get_resource_server_client(service_path="api/gateway/v1")
300-
301-
logger.debug(f"Fetching claims from gateway for user {user_ansible_id}")
302-
response = client._make_request("GET", f"jwt_claims/{user_ansible_id}/")
319+
# Use the resource server client to make the request
320+
client = get_resource_server_client(service_path="api/gateway/v1")
303321

304-
if response.status_code == 200:
305-
claims_data = response.json()
306-
return claims_data
307-
else:
308-
logger.error(f"Gateway request failed with status {response.status_code}")
309-
return None
322+
logger.debug(f"Fetching claims from gateway for user {user_ansible_id}")
323+
response = client._make_request("GET", f"jwt_claims/{user_ansible_id}/")
310324

311-
except Exception as e:
312-
logger.error(f"Error fetching claims from gateway: {e}")
313-
return None
325+
if response.status_code == 200:
326+
claims_data = response.json()
327+
return claims_data
328+
elif response.status_code == 423:
329+
raise GatewayLockedException("Gateway is locked")
330+
else:
331+
raise InvalidGatewayResponseException(f"Gateway request failed with status {response.status_code}")
314332

315333

316334
class JWTAuthentication(BaseAuthentication):

test_app/tests/jwt_consumer/common/test_auth.py

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,60 @@ def test_process_rbac_permissions_cache_scenarios(
537537
else:
538538
mock_apply.assert_not_called()
539539

540+
@pytest.mark.django_db
541+
@pytest.mark.parametrize(
542+
"is_superuser_value,should_allow",
543+
[
544+
(True, True), # Superuser should be allowed when gateway is locked
545+
(False, False), # Non-superuser should be denied when gateway is locked
546+
],
547+
)
548+
def test_process_rbac_permissions_gateway_locked(self, admin_user, is_superuser_value, should_allow):
549+
"""Test process_rbac_permissions handles GatewayLockedException based on superuser status"""
550+
from ansible_base.jwt_consumer.common.auth import GatewayLockedException
551+
552+
authentication = JWTCommonAuth()
553+
authentication.user = admin_user
554+
authentication.token = {"sub": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "claims_hash": "a1b2c3d4", "user_data": {"is_superuser": is_superuser_value}}
555+
556+
with (
557+
mock.patch.object(authentication.cache, 'get_cached_claims_hash') as mock_get_cache,
558+
mock.patch.object(authentication, '_fetch_jwt_claims_from_gateway') as mock_gateway,
559+
):
560+
# Setup mocks - cache miss to trigger gateway call
561+
mock_get_cache.return_value = None
562+
mock_gateway.side_effect = GatewayLockedException("Gateway is locked")
563+
564+
if should_allow:
565+
# Superuser should be allowed - no exception raised
566+
authentication.process_rbac_permissions()
567+
else:
568+
# Non-superuser should be denied
569+
with pytest.raises(Exception) as exc_info:
570+
authentication.process_rbac_permissions()
571+
assert "not a superuser and gateway is locked" in str(exc_info.value)
572+
573+
@pytest.mark.django_db
574+
def test_process_rbac_permissions_gateway_fetch_exception(self, admin_user):
575+
"""Test process_rbac_permissions handles generic exceptions from gateway fetch"""
576+
authentication = JWTCommonAuth()
577+
authentication.user = admin_user
578+
authentication.token = {"sub": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "claims_hash": "a1b2c3d4", "user_data": {"is_superuser": False}}
579+
580+
with (
581+
mock.patch.object(authentication.cache, 'get_cached_claims_hash') as mock_get_cache,
582+
mock.patch.object(authentication, '_fetch_jwt_claims_from_gateway') as mock_gateway,
583+
):
584+
# Setup mocks - cache miss to trigger gateway call
585+
mock_get_cache.return_value = None
586+
mock_gateway.side_effect = Exception("Network error")
587+
588+
# Should raise an exception with the error details
589+
with pytest.raises(Exception) as exc_info:
590+
authentication.process_rbac_permissions()
591+
assert "unable to validate user permissions" in str(exc_info.value).lower()
592+
assert "network error" in str(exc_info.value).lower()
593+
540594
@pytest.mark.django_db
541595
def test_process_rbac_permissions_missing_token_data(self):
542596
"""Test process_rbac_permissions with missing required token data"""
@@ -758,6 +812,7 @@ def test__fetch_jwt_claims_from_gateway_success(self):
758812
def test__fetch_jwt_claims_from_gateway_non_200(self):
759813
authentication = JWTCommonAuth()
760814
user_ansible_id = '12345678-1234-5678-9abc-123456789012'
815+
from ansible_base.jwt_consumer.common.auth import InvalidGatewayResponseException
761816

762817
with mock.patch('ansible_base.jwt_consumer.common.auth.get_resource_server_client') as mock_get_client:
763818
mock_client = mock.Mock()
@@ -766,18 +821,38 @@ def test__fetch_jwt_claims_from_gateway_non_200(self):
766821
mock_client._make_request.return_value = mock_response
767822
mock_get_client.return_value = mock_client
768823

769-
result = authentication._fetch_jwt_claims_from_gateway(user_ansible_id)
770-
771-
assert result is None
824+
# Should raise InvalidGatewayResponseException for non-200/423 status codes
825+
with pytest.raises(InvalidGatewayResponseException) as exc_info:
826+
authentication._fetch_jwt_claims_from_gateway(user_ansible_id)
827+
assert "gateway request failed with status 404" in str(exc_info.value).lower()
772828
mock_get_client.assert_called_once_with(service_path='api/gateway/v1')
773829
mock_client._make_request.assert_called_once_with('GET', f'jwt_claims/{user_ansible_id}/')
774830

775-
def test__fetch_jwt_claims_from_gateway_exception(self):
831+
def test__fetch_jwt_claims_from_gateway_423_locked(self):
776832
authentication = JWTCommonAuth()
777833
user_ansible_id = '12345678-1234-5678-9abc-123456789012'
834+
from ansible_base.jwt_consumer.common.auth import GatewayLockedException
778835

779-
with mock.patch('ansible_base.jwt_consumer.common.auth.get_resource_server_client', side_effect=Exception('boom')) as mock_get_client:
780-
result = authentication._fetch_jwt_claims_from_gateway(user_ansible_id)
836+
with mock.patch('ansible_base.jwt_consumer.common.auth.get_resource_server_client') as mock_get_client:
837+
mock_client = mock.Mock()
838+
mock_response = mock.Mock()
839+
mock_response.status_code = 423
840+
mock_client._make_request.return_value = mock_response
841+
mock_get_client.return_value = mock_client
781842

782-
assert result is None
843+
# Should raise GatewayLockedException for 423 status
844+
with pytest.raises(GatewayLockedException) as exc_info:
845+
authentication._fetch_jwt_claims_from_gateway(user_ansible_id)
846+
assert "gateway is locked" in str(exc_info.value).lower()
783847
mock_get_client.assert_called_once_with(service_path='api/gateway/v1')
848+
mock_client._make_request.assert_called_once_with('GET', f'jwt_claims/{user_ansible_id}/')
849+
850+
def test__fetch_jwt_claims_from_gateway_exception(self):
851+
authentication = JWTCommonAuth()
852+
user_ansible_id = '12345678-1234-5678-9abc-123456789012'
853+
854+
with mock.patch('ansible_base.jwt_consumer.common.auth.get_resource_server_client', side_effect=Exception('boom')):
855+
# Should re-raise the exception
856+
with pytest.raises(Exception) as exc_info:
857+
authentication._fetch_jwt_claims_from_gateway(user_ansible_id)
858+
assert "boom" in str(exc_info.value)

0 commit comments

Comments
 (0)