Skip to content

Commit f93c8aa

Browse files
AAP-37017 Fix how we build AuthExceptions (#664)
The `AuthException` class from `social_core.exceptions` takes a first parameter of a backend and a second of a message. Since we only had one parameter when we constructed our AuthException, printing it may be black: ``` In [6]: from social_core.exceptions import AuthException In [7]: try: ...: raise AuthException(None, 'Testing') ...: except AuthException as e: ...: print(f'AuthException: {e}') ...: AuthException: Testing In [8]: try: ...: raise AuthException('Testing') ...: except AuthException as e: ...: print(f'AuthException: {e}') ...: AuthException: ``` This PR fixes this by passing in `None` as the first backend parameter to the `AuthException`.
1 parent 2585c59 commit f93c8aa

File tree

2 files changed

+32
-10
lines changed

2 files changed

+32
-10
lines changed

ansible_base/authentication/utils/authentication.py

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ def setting(self, *args, **kwargs):
2121
return ["username", "email"]
2222

2323

24+
def raise_auth_exception(message: str, backend: Optional[Authenticator] = None):
25+
raise AuthException(backend, message)
26+
27+
2428
def migrate_from_existing_authenticator(
2529
uid: str, alt_uid: Optional[str], authenticator: Authenticator, preferred_username: Optional[str] = None
2630
) -> Optional[str]:
@@ -84,14 +88,14 @@ def get_local_username(user_details: dict) -> str:
8488
return username['username']
8589

8690

87-
def check_system_username(uid: str) -> None:
91+
def check_system_username(uid: str, authenticator: Optional[Authenticator]) -> None:
8892
"""
8993
Determine if a username is identical with SYSTEM_USERNAME
9094
Raise AuthException if system user attempts to login via an external authentication source
9195
"""
9296
if uid.casefold() == settings.SYSTEM_USERNAME.casefold():
9397
logger.warning(f'{settings.SYSTEM_USERNAME} cannot log in from an authenticator!')
94-
raise AuthException(_('System user is not allowed to log in from external authentication sources.'))
98+
raise_auth_exception(_('System user is not allowed to log in from external authentication sources.'), backend=authenticator)
9599

96100

97101
def determine_username_from_uid_social(**kwargs) -> dict:
@@ -108,17 +112,18 @@ def determine_username_from_uid_social(**kwargs) -> dict:
108112
# 'last_name': <user last name if any>
109113
# }
110114
# If this data structure does not return the username, this method will fail
115+
authenticator = kwargs.get('backend')
116+
if not authenticator:
117+
raise_auth_exception(_('Unable to get backend from kwargs'))
118+
111119
selected_username = kwargs.get('details', {}).get('username', None)
112120
if not selected_username:
113-
raise AuthException(
121+
raise_auth_exception(
114122
_('Unable to get associated username from details, expected entry "username". Full user details: %(details)s')
115-
% {'details': kwargs.get("details", None)}
123+
% {'details': kwargs.get("details", None)},
124+
backend=authenticator,
116125
)
117126

118-
authenticator = kwargs.get('backend')
119-
if not authenticator:
120-
raise AuthException(_('Unable to get backend from kwargs'))
121-
122127
alt_uid = authenticator.get_alternative_uid(**kwargs)
123128

124129
if migrated_username := migrate_from_existing_authenticator(
@@ -146,7 +151,7 @@ def determine_username_from_uid(uid: str = None, authenticator: Authenticator =
146151
a username of timmy<hash>. This literally does not make sense for the local authenticator because the DB is its own source of truth.
147152
"""
148153
try:
149-
check_system_username(uid)
154+
check_system_username(uid, authenticator=authenticator)
150155
except AuthException as e:
151156
logger.warning(f"AuthException: {e}")
152157
raise
@@ -190,7 +195,7 @@ def get_or_create_authenticator_user(
190195
For example, LDAP might return sn, location, phone_number, etc
191196
"""
192197
try:
193-
check_system_username(uid)
198+
check_system_username(uid, authenticator=authenticator)
194199
except AuthException as e:
195200
logger.warning(f"AuthException: {e}")
196201
raise

test_app/tests/authentication/utils/test_authentication.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,20 @@ def test_determine_username_from_uid_social_happy_path(self, ldap_authenticator)
197197
uid="Bob",
198198
)
199199
assert response == {'username': 'Bob'}
200+
201+
def test_raise_auth_exception(self):
202+
try:
203+
authentication.raise_auth_exception('testing')
204+
except AuthException as e:
205+
assert str(e) == 'testing'
206+
207+
def test_raise_auth_exception_in_logs(self, local_authenticator, expected_log):
208+
with expected_log(
209+
'ansible_base.authentication.utils.authentication.logger',
210+
'warning',
211+
'AuthException: System user is not allowed to log in from external authentication sources.',
212+
):
213+
try:
214+
authentication.get_or_create_authenticator_user(settings.SYSTEM_USERNAME, local_authenticator, user_details={}, extra_data={})
215+
except AuthException:
216+
pass

0 commit comments

Comments
 (0)