Skip to content

Commit 95e8ccf

Browse files
bhavenstBryan Havenstein
andauthored
AAP-39645 Allow non-admin admin (#716)
## Description Fixes issues blocking a user not named "admin" from being the admin user. The issue in this case was creation of the db authenticator via "aap-gateway-manage authenticators --initialize" was hard coded to the user "admin" but this PR allows it to be created by the system user or by no user if there is no "admin" user or system user defined. ## 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 ### Steps to Test 1. Start gateway (with PR for AAP-39645 + this PR) after modifying gateway_admin_username in container-startup.yml to something other than admin. 2. See that everything works OK (there were errors in the log and didn't start before). Log in as the specified user and see they have system administrator role. 3. It's not really possible to unset SYSTEM_USER for gateway or for test_app, both are not happy without one. This situation could be tested with awx (since apparently it does not use a system user) but that is a challenge to set up. Unit tests cover this scenario. ### Expected Results See above. ## Additional Context N/A ### Required Actions - [ ] Requires documentation updates - [ ] Requires downstream repository changes - [ ] Requires infrastructure/deployment changes - [ ] Requires coordination with other teams - [ ] Blocked by PR/MR: #XXX ### Screenshots/Logs N/A --------- Co-authored-by: Bryan Havenstein <[email protected]>
1 parent a74944a commit 95e8ccf

File tree

2 files changed

+82
-42
lines changed

2 files changed

+82
-42
lines changed

ansible_base/authentication/management/commands/authenticators.py

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,18 @@
99
from django.core.management.base import BaseCommand, CommandError
1010
from django.utils.translation import gettext_lazy as _
1111

12-
from ansible_base.authentication.models import Authenticator, AuthenticatorUser
12+
from ansible_base.authentication.models import Authenticator
13+
from ansible_base.lib.utils.models import get_system_user
1314

1415

1516
class Command(BaseCommand):
1617
help = "Initialize service configuration with an admin user and a local authenticator"
1718

1819
def add_arguments(self, parser):
19-
parser.add_argument("--list", action="store_true", help="list the authenticators", required=False)
20+
parser.add_argument("--list", action="store_true", help="List the authenticators", required=False)
2021
parser.add_argument("--initialize", action="store_true", help="Initialize an admin user and local db authenticator", required=False)
21-
parser.add_argument("--enable", type=int, help="Initialize an admin user and local db authenticator", required=False)
22-
parser.add_argument("--disable", type=int, help="Initialize an admin user and local db authenticator", required=False)
22+
parser.add_argument("--enable", type=int, help="Enable the authenticator with provided ID", required=False)
23+
parser.add_argument("--disable", type=int, help="Disable the authenticator with provided ID", required=False)
2324

2425
def handle(self, *args, **options):
2526
took_action = False
@@ -30,17 +31,20 @@ def handle(self, *args, **options):
3031
for id, state in [(options['enable'], True), (options['disable'], False)]:
3132
if not id:
3233
continue
33-
try:
34-
authenticator = Authenticator.objects.get(id=id)
35-
except Authenticator.DoesNotExist:
36-
raise CommandError(_("Authenticator %(id)s does not exist") % {"id": id})
37-
if authenticator.enabled is not state:
38-
authenticator.enabled = state
39-
authenticator.save()
34+
self._update_authenticator(id, state)
4035
took_action = True
4136
if options["list"] or not took_action:
4237
self.list_authenticators()
4338

39+
def _update_authenticator(self, id: int, state: bool):
40+
try:
41+
authenticator = Authenticator.objects.get(id=id)
42+
except Authenticator.DoesNotExist:
43+
raise CommandError(_("Authenticator %(id)s does not exist") % {"id": id})
44+
if authenticator.enabled is not state:
45+
authenticator.enabled = state
46+
authenticator.save()
47+
4448
def list_authenticators(self):
4549
authenticators = []
4650
headers = ["ID", "Enabled", "Name", "Order"]
@@ -58,26 +62,34 @@ def list_authenticators(self):
5862
self.stdout.write('')
5963

6064
def initialize_authenticators(self):
61-
admin_user = get_user_model().objects.filter(username="admin").first()
62-
if not admin_user:
63-
raise CommandError("Admin user with username 'admin' not defined.")
65+
if Authenticator.objects.filter(type="ansible_base.authentication.authenticator_plugins.local").exists():
66+
self.stdout.write("Local authenticator already exists, skipping")
67+
return
6468

65-
existing_authenticator = Authenticator.objects.filter(type="ansible_base.authentication.authenticator_plugins.local").first()
66-
if not existing_authenticator:
67-
existing_authenticator = Authenticator.objects.create(
68-
name='Local Database Authenticator',
69-
enabled=True,
70-
create_objects=True,
71-
configuration={},
72-
created_by=admin_user,
73-
modified_by=admin_user,
74-
remove_users=False,
75-
type='ansible_base.authentication.authenticator_plugins.local',
76-
)
77-
self.stdout.write("Created default local authenticator")
69+
# First try to get the system user
70+
system_user = get_system_user()
71+
admin_user = None
72+
try:
73+
admin_user = get_user_model().objects.filter(username="admin").first()
74+
except get_user_model().DoesNotExist:
75+
pass
76+
creator = None
77+
if system_user is not None:
78+
creator = system_user
79+
elif admin_user is not None:
80+
creator = admin_user
81+
else:
82+
creator = None
83+
self.stderr.write("Neither system user nor admin user were defined, local authenticator will be created without created_by set")
7884

79-
AuthenticatorUser.objects.get_or_create(
80-
uid=admin_user.username,
81-
user=admin_user,
82-
provider=existing_authenticator,
83-
)
85+
Authenticator.objects.create(
86+
name='Local Database Authenticator',
87+
enabled=True,
88+
create_objects=True,
89+
configuration={},
90+
created_by=creator,
91+
modified_by=creator,
92+
remove_users=False,
93+
type='ansible_base.authentication.authenticator_plugins.local',
94+
)
95+
self.stdout.write("Created default local authenticator")

test_app/tests/authentication/management/test_authenticators.py

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import pytest
55
from django.core.management import CommandError, call_command
6+
from django.test.utils import override_settings
67

78
from ansible_base.authentication.models import Authenticator, AuthenticatorUser
89

@@ -77,27 +78,49 @@ def test_authenticators_cli_list_without_tabulate(command_args, local_authentica
7778
assert order.strip() == str(authenticator.order)
7879

7980

80-
def test_authenticators_cli_initialize(django_user_model):
81+
@pytest.mark.parametrize(
82+
"system_user_exists,admin_user_exists,log_location,expected_log_entry,expected_authenticator_creator",
83+
[
84+
(True, True, "stdout", "Created default local authenticator", "_system"),
85+
(True, False, "stdout", "Created default local authenticator", "_system"),
86+
(False, True, "stdout", "Created default local authenticator", "admin"),
87+
(False, False, "stderr", "Neither system user nor admin user were defined", None),
88+
],
89+
)
90+
def test_authenticators_cli_initialize(
91+
django_user_model, system_user_exists, admin_user_exists, log_location, expected_log_entry, expected_authenticator_creator
92+
):
8193
"""
82-
Calling with --initialize will create:
83-
- An authenticator if there is an admin user
94+
Tests the different options for --initialize on authenticators.
8495
"""
8596
out = StringIO()
8697
err = StringIO()
8798

8899
# Sanity check:
89100
assert django_user_model.objects.count() == 0
90101

91-
with pytest.raises(CommandError) as e:
102+
# Optionally create admin user
103+
if admin_user_exists:
104+
django_user_model.objects.create(username="admin")
105+
106+
# Set system user
107+
system_username = "_system" if system_user_exists else None
108+
with override_settings(SYSTEM_USERNAME=system_username):
92109
call_command('authenticators', "--initialize", stdout=out, stderr=err)
93-
assert "Admin user with username 'admin' not defined." in str(e.value)
94110

95-
django_user_model.objects.create(username="admin")
96-
call_command('authenticators', "--initialize", stdout=out, stderr=err)
97-
assert "Created default local authenticator" in out.getvalue()
111+
if log_location == "stdout":
112+
assert expected_log_entry in out.getvalue()
113+
else:
114+
assert expected_log_entry in err.getvalue()
98115

116+
assert Authenticator.objects.count() == 1
117+
if admin_user_exists or system_user_exists:
118+
assert Authenticator.objects.first().created_by.username == expected_authenticator_creator
119+
else:
120+
assert Authenticator.objects.first().created_by is None
99121

100-
def test_authenticators_cli_initialize_pre_existing(django_user_model, local_authenticator, admin_user):
122+
123+
def test_authenticators_cli_initialize_pre_existing(django_user_model, local_authenticator, admin_user, unauthenticated_api_client):
101124
"""
102125
What if we already have an admin user?
103126
@@ -121,12 +144,17 @@ def test_authenticators_cli_initialize_pre_existing(django_user_model, local_aut
121144
# Nothing should have changed
122145
assert existing_user == new_user
123146
assert existing_user.date_joined == new_user.date_joined
124-
assert out.getvalue() == ""
147+
assert "Local authenticator already exists, skipping" in out.getvalue()
125148
assert err.getvalue() == ""
126149

127150
# No AuthenticatorUser should get created in this case
128151
assert AuthenticatorUser.objects.count() == 0
129152

153+
# Log in to auto-create AuthenticatorUser
154+
unauthenticated_api_client.login(username="admin", password="password")
155+
assert AuthenticatorUser.objects.count() == 1
156+
assert AuthenticatorUser.objects.first().user == admin_user
157+
130158

131159
@pytest.mark.parametrize(
132160
"start_state, flag, end_state, exp_out, exp_err",

0 commit comments

Comments
 (0)