Skip to content

Commit 5da2594

Browse files
bhavenstBryan Havenstein
andauthored
AAP-43587: Fix 50x error in ldap legacy auth (#721)
## Description Bug description: * Resource (LDAP legacy migrated user, though it doesn't really matter the type) exists in controller and gateway with ansible_id A * User logs in via "I have a controller account" legacy auth on gateway * Gateway (for LDAP users only) tests a login via the opposite component as well (hub if controller login, controller if hub login) to see if the user can log in there (see this code) * In our pipeline 2.4 -> 2.5 upgrade scenarios w/ LDAP, we have the same LDAP instance connected to hub and controller, so even if a user is created only on 2.4 controller, it's perfectly fine for that user to login as well on hub at any point even though the pipeline hasn't done that and doesn't intend to the hub test login succeeds and creates a new user with the same username, but with ansible_id B * Hub immediately syncs to gateway, who updates the existing matching user from ansible_id A to B * User clicks the finalize button to confirm user migration, but it fails with a 50x error because gateway is using the new ansible_id B while controller still has ansible_id A for that user Fix: DAB is updated such that it will not attempt a local authentication upon request if it does not already have a user matching the requested username. This will prevent the issue without making any general behavioral changes in how resources are managed. ## Type of Change - [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 - [x] I have performed a self-review of my code - [x] I have added relevant comments to complex code sections - [ ] I have updated documentation where needed - [ ] I have considered the security impact of these changes - [ ] I have considered performance implications - [ ] I have thought about error handling and edge cases - [x] I have tested the changes in my local environment ## Testing Instructions ### Prerequisites Use a 2.4 -> 2.5 upgrade pipeline from https://jenkins-csb-aap-main.dno.corp.redhat.com/job/AAPQA/job/AAPQA%20Provisioner/job/AAPQA-ATF-Upgrade-On-Demand-Pipeline/ WITH LDAP enabled ### Steps to Test 1. The pipeline will create some users that are defined only in controller (see https://gitlab.cee.redhat.com/ansible/testing/test-suite/-/blob/main/src/aap_test_suite/resources/freeipa.py?ref_type=heads users starting with ldap_ui_ctlr_, such as tower_all, tower_1, saml_user). Pick one of these users. 2. From gateway login screen, select "I have a controller account" and log in with the user. 3. The migration screen will be shown. Click through and finish (finalize) the user migration. ### Expected Results No 50x error encountered and migration completed successfully. ## Additional Context Before the change, any of these users would trigger a 50x error upon finalization. Co-authored-by: Bryan Havenstein <[email protected]>
1 parent 9291277 commit 5da2594

File tree

2 files changed

+14
-0
lines changed

2 files changed

+14
-0
lines changed

ansible_base/resource_registry/views.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from collections import OrderedDict
33

44
from django.conf import settings
5+
from django.contrib.auth import get_user_model
56
from django.db.models import Q
67
from django.http import HttpResponseNotFound
78
from django.shortcuts import get_object_or_404
@@ -198,6 +199,11 @@ def post(self, request, **kwargs):
198199
serializer = UserAuthenticationSerializer(data=request.data)
199200
serializer.is_valid(raise_exception=True)
200201

202+
# Ensure the users exists before authenticating
203+
if not get_user_model().objects.filter(username=serializer.validated_data["username"]).exists():
204+
logger.debug(f"User {serializer.validated_data['username']} does not exist, not validating authentication")
205+
return Response(status=401)
206+
201207
api_config = get_registry().api_config
202208
user = api_config.authenticate_local_user(serializer.validated_data["username"], serializer.validated_data["password"])
203209

test_app/tests/resource_registry/test_views.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ def test_validate_local_user(unauthenticated_api_client, admin_user, local_authe
2323
assert 'ansible_id' in response.data
2424
assert response.data['auth_code'] is None
2525

26+
# Should return 401 for non-existent user
27+
data = {
28+
"username": "fakeuser",
29+
"password": "doesnotexist",
30+
}
31+
response = unauthenticated_api_client.post(url, data=data)
32+
assert response.status_code == 401
33+
2634

2735
def get_users_manifest(client, data=None, expect=200):
2836
if data is None:

0 commit comments

Comments
 (0)