[AAP-49757] Add fallback authentication support to local authenticator #868

john-westcott-iv · 2025-10-20T07:27:57Z

Description

This PR adds pluggable fallback authentication support to the local authenticator and includes several code quality improvements and utility refactorings discovered during implementation.

Primary Feature: Fallback Authentication System

What is being changed?

Added fallback_authentication configuration field (array of module paths) to LocalConfiguration
Implemented dynamic loading of FallbackAuthenticator classes with fixed naming convention
Implemented _try_fallback_authenticators() to orchestrate fallback authentication attempts
Fallback authenticators are simple classes with no constructor parameters, just an authenticate() method
Module path validation using compiled regex patterns for performance and security
Comprehensive test coverage including edge cases, error handling, and parallel execution safety

Why is this change needed?

Enables separation of implementation-specific authentication logic from the shared DAB library
Provides extensible plugin pattern for custom authentication fallbacks
Required for Gateway Controller migration functionality (AAP-49757)

How does this change address the issue?

When primary authentication fails, fallbacks are attempted in order
Plugin pattern with fixed FallbackAuthenticator class name makes configuration simple
Robust error handling ensures one failing fallback doesn't prevent others from attempting
Each fallback authenticator is isolated and self-contained

Code Quality Improvements

1. Duration Utility Refactoring

Moved _convert_to_seconds from local authenticator to reusable ansible_base/lib/utils/duration.py
Added type hints and comprehensive docstring
Improved parsing logic with pre-compiled regex and dictionary-based unit conversion
Fixed edge cases: negative durations, simple integers, None handling
Added runtime validation for default parameter (type checking with warnings)
Achieved 100% test coverage with 79 parameterized test cases
Enhanced error logging to include exception details for debugging

2. Import Utility Consolidation

Created ansible_base/lib/utils/imports.py with unified import_object() function
Eliminated code duplication across 4+ modules (local.py, backend.py, settings.py, utils.py)
Improved error handling with specific exception catching (ValueError, ImportError, AttributeError)
Added regex validation for import paths with clear error messages
Deprecated get_from_import() gracefully with DeprecationWarning
Added 22 parameterized tests covering all edge cases and error conditions
Benefits: Single source of truth for dynamic imports, consistent error handling, improved maintainability

3. Regex Pattern Optimization

Replaced [a-zA-Z0-9_] with concise \w character class syntax
Moved regex compilation to module level for performance (compile once, use many times)
Satisfies SonarCloud code quality requirements
Maintains identical validation behavior with cleaner, more readable code

Configuration Example

{
  "fallback_authentication": [
    "aap_gateway_api.authentication.fallbacks.controller",
    "custom_app.authentication.fallbacks.ldap"
  ]
}

Plugin Contract

Each fallback module must export a FallbackAuthenticator class with:

__init__() constructor (no parameters required)
authenticate(request, username, password, **kwargs) method returning User or None

Security Considerations

Module path validation using strict regex patterns ensures only valid Python module paths are accepted
Import/AttributeError exceptions are caught and logged without breaking authentication flow
Fallback authenticators are responsible for their own security checks and validation
No sensitive data is passed through configuration - fallbacks access what they need directly

Type of Change

✅ New feature (non-breaking change which adds functionality)
✅ Code refactoring (non-breaking internal improvements)
✅ Test updates and improvements
✅ Code quality improvements (SonarCloud compliance)

Testing

Fallback Authentication Tests (44 tests)

Plugin loading mechanisms and error handling
Fallback orchestration with multiple authenticators
Integration with main authenticate() method
Edge cases, error handling, and negative tests
Parallel execution safety and state isolation

Duration Utility Tests (79 test cases)

Valid duration formats (15s, 1h, 1d, etc.)
Invalid formats and error handling
Edge cases (None, empty string, negative values, zero)
Default parameter behavior and type validation
Consistent parsing across all unit types

Import Utility Tests (22 test cases)

Valid imports (full path and split module/attribute)
Invalid imports (non-existent modules/attributes)
Standard library imports
Error handling and validation
Consistency and callable results

Test Results

✅ All 2711 tests pass in django-ansible-base test suite
✅ 100% code coverage on new utilities and fallback authentication code
✅ All pre-commit hooks pass (black, flake8, isort)
✅ SonarCloud quality gate requirements met

Dependencies

This PR is required by:

AAP Gateway PR #1049: [AAP-49757] Implement Controller fallback authentication
- Uses the new fallback_authentication configuration
- Implements aap_gateway_api.authentication.fallbacks.controller.FallbackAuthenticator

Additional Context

Architecture Benefits

Separation of Concerns: Implementation-specific auth logic stays in implementations, not DAB
Extensibility: New fallback authenticators can be added without modifying DAB code
Reusability: Duration and import utilities benefit the entire codebase
Maintainability: Consolidated import logic reduces duplication and improves consistency
Code Quality: SonarCloud compliance and comprehensive test coverage

Backward Compatibility

✅ No breaking changes to existing authentication behavior
✅ Fallback authentication is opt-in via configuration
✅ get_from_import() deprecated but still functional for external code
✅ All existing tests continue to pass

Co-authored-by: Claude claude@anthropic.com

ansible_base/lib/utils/duration.py

ansible_base/authentication/authenticator_plugins/local.py

ansible_base/lib/utils/imports.py

bhavenst

Should this maybe get a blurb in docs/apps/authentication?

ansible_base/authentication/authenticator_plugins/local.py

- Move _convert_to_seconds from local.py authenticator to reusable ansible_base/lib/utils/duration.py module - Add type hints: duration_string: Optional[str], default: int - Update function to support negative durations (validation left to callers) - Fix parsing logic to correctly handle simple integer strings like '0' - Relocate tests to test_app/tests/lib/utils/test_duration.py - Consolidate tests into parameterized test suite (62 test cases) - Achieve 100% line coverage with comprehensive edge case testing - Update local.py authenticator to import and use new utility function Co-authored-by: Claude <claude@anthropic.com>

- Add fallback_authentication configuration field (ListField of module paths) - Implement _load_fallback_plugin() to dynamically load FallbackAuthenticator classes - Implement _try_fallback_authenticators() to orchestrate fallback attempts - Fallback authentication only attempted when primary auth fails - Each fallback receives database_instance and configuration parameters - Comprehensive test coverage for plugin loading, orchestration, and integration - Tests include edge cases, error handling, and parallel execution safety - Moved duration conversion tests (test_duration.py removed as functionality moved to lib) Co-authored-by: Claude <claude@anthropic.com>

- Replace multiple if-elif conditions with single regex pattern - Pattern validates proper Python module path format - Each segment must start with letter or underscore - Must have at least one dot separating segments - Consolidate error messages for clearer user feedback Co-authored-by: Claude <claude@anthropic.com>

- Create test_app/tests/lib/utils/test_duration.py with 79 tests - Parameterized tests for valid durations (all units: s/m/h/d/w) - Parameterized tests for invalid inputs with custom defaults - Test positive and negative values - Test case-insensitive unit support - Test edge cases (zero values, lone dash, None input) - Test consistency across multiple calls - Achieve 82% code coverage with branch coverage Co-authored-by: Claude <claude@anthropic.com>

- Move MODULE_PATH_PATTERN to module level as a constant - Compile regex pattern once at module load instead of every validate() call - Improves performance by avoiding repeated regex compilation - All tests pass, no functional changes Co-authored-by: Claude <claude@anthropic.com>

- Add DURATION_CHAR_TO_SECONDS dictionary for unit-to-seconds mapping - Pre-compile DURATION_RE regex pattern at module level for performance - Support negative durations in regex pattern (^(-?\d+)([smhdw]?)$) - Add explicit None handling before regex match - Use walrus operator (:=) for cleaner match assignment - Default to 's' unit when no unit specified (or 's') - Add comprehensive try-except to catch all edge cases - All 79 tests pass with 88% code coverage Co-authored-by: Claude <claude@anthropic.com>

…ation - Consolidated test_convert_to_seconds_default_parameter, test_convert_to_seconds_zero_default, and test_convert_to_seconds_negative_default into a single parameterized test - Parameterized test_convert_to_seconds_edge_case_just_minus with multiple default values - Parameterized test_convert_to_seconds_edge_case_zero_values to test all zero unit types - Reduced code duplication while maintaining comprehensive test coverage - Test count increased from 79 to 89 due to additional parameterized cases

- Renamed all test functions to use clearer names: - test_convert_to_seconds_valid -> test_convert_to_seconds_valid_inputs - test_convert_to_seconds_invalid -> test_convert_to_seconds_invalid_inputs - Renamed all parameter names to be more descriptive: - duration_string -> duration_input, invalid_input - expected -> expected_seconds, expected_result - default -> default_value - Added pytest.param() with explicit id for every test case - IDs now clearly describe what is being tested (e.g., 'positive_15_seconds', 'invalid_string_with_default_10') - Merged edge case tests into main parameterized tests with descriptive IDs - Improved consistency test to validate expected values, not just consistency - All 83 tests pass with clear, readable test output

- Added test_convert_to_seconds_valid_inputs_ignore_default test function - Tests verify that when parsing succeeds, the default parameter is properly ignored - Covers 7 scenarios: positive/negative/zero values across different units - Addresses gap in test coverage where custom defaults were only tested with invalid inputs - All 76 tests pass

- Validate that default is an integer (excluding booleans which are int subclass) - Log warning with stack trace when invalid default is provided - Automatically fall back to default value of 10 - Add comprehensive tests for boolean default edge cases - Consolidate default behavior tests into single parameterized function The stack_info=True in logger.warning() provides developers with a full stack trace showing exactly where convert_to_seconds() was called with an invalid default, making debugging much easier while still being defensive by allowing the function to continue. Note: typeguard already prevents most invalid types (str, float, list, dict, None) from reaching the function, so we only need to explicitly handle booleans which pass isinstance(x, int) checks.

This commit introduces a new centralized import utility and refactors existing code to use it, eliminating duplication across multiple modules. New Utility Function: - Created ansible_base.lib.utils.imports.import_object() - Unified interface for dynamically importing objects from modules - Supports two formats: 'module.path.Class' or ('module.path', 'Class') - Uses regex validation (FULL_IMPORT_PATTERN) to ensure valid Python paths - Raises ValueError for invalid paths, ImportError for missing modules, and AttributeError for missing attributes Refactored Modules: - local.py: Removed _load_fallback_plugin(), now uses import_object directly - settings.py: get_function_from_setting() uses import_object - authenticator_plugins/utils.py: get_authenticator_class() uses import_object - django_app_api.py: Updated to use import_object directly Exception Handling: - Updated exception handlers to catch specific exceptions (ValueError, ImportError, AttributeError) instead of broad Exception catches - Improved error specificity in settings.py and utils.py Deprecation: - Marked get_from_import() as deprecated with DeprecationWarning - Added Sphinx-style deprecation notice in docstring - Updated internal usage and tests to use import_object directly - Function remains for backward compatibility with external code Testing: - Added comprehensive test suite in test_app/tests/lib/utils/test_imports.py - 22 parameterized tests covering valid/invalid imports, edge cases, standard library modules, and error conditions - Updated test_views.py mock to target import_object Benefits: - Eliminates code duplication across 4+ modules - Single source of truth for dynamic imports - Better error messages with regex validation - Consistent error handling across the codebase - Improved maintainability and testability

…ack_plugin Updated all tests that were mocking _load_fallback_plugin (which was removed during refactoring) to now mock import_object from ansible_base.lib.utils.imports. Changes: - TestLoadFallbackPlugin: Updated to test import_object integration instead of the removed _load_fallback_plugin method - TestTryFallbackAuthenticators: All 10 tests now mock import_object with proper side_effect functions that accept (path, attr) parameters - TestAuthenticateIntegration: Updated test_successful_primary_auth_skips_fallback - TestParallelExecutionSafety: Updated test_fallback_state_not_shared - TestEdgeCases: Updated 3 tests (kwargs, long list, special characters) All mock functions updated to match import_object signature (path, attr) instead of the old _load_fallback_plugin signature (path). This fixes 16 failing tests in the CI.

The previous commit (113c4b0) incorrectly added database_instance and configuration parameters back to fallback authenticator instantiation. This was wrong - our design doesn't pass these parameters. Changes: 1. local.py: Remove database_instance and configuration params from fallback_class() call - fallback authenticators don't need them 2. test_local.py: Replace test_fallback_passes_correct_parameters with test_fallback_instantiation that just verifies instantiation works, not that parameters are passed This fixes the test failure and aligns with our actual architecture where fallback authenticators (like Controller fallback) are simple classes with just an authenticate() method.

Replace [a-zA-Z0-9_] with \w for better readability and to satisfy SonarCloud code quality requirements. Changes: - local.py: MODULE_PATH_PATTERN now uses \w for word characters - imports.py: FULL_IMPORT_PATTERN now uses \w for word characters Both patterns still correctly validate: - First character must be letter or underscore [a-zA-Z_] - Subsequent characters can be word characters (\w = [a-zA-Z0-9_]) All 44 tests in test_local.py and 22 tests in test_imports.py pass.

- Move MODULE_PATH_PATTERN from local.py to imports.py - Factor out [a-zA-Z_] pattern into _IDENTIFIER_START constant - Use _IDENTIFIER_START in _MODULE_SEGMENT pattern - Reuse _MODULE_SEGMENT in both MODULE_PATH_PATTERN and FULL_IMPORT_PATTERN - Eliminates all duplication of [a-zA-Z_] to satisfy SonarCloud This improves maintainability and ensures consistency across all Python identifier validation patterns. Co-authored-by: Claude <claude@anthropic.com>

This reverts commit e4be818.

github-actions · 2025-10-23T17:05:45Z

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

sonarqubecloud · 2025-10-23T17:19:24Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

john-westcott-iv force-pushed the AAP-49757 branch 5 times, most recently from 07155e6 to 7c0c75c Compare October 20, 2025 10:55

jay-steurer requested review from AlanCoding and bhavenst October 20, 2025 12:17

jay-steurer added the Ready for review This PR is ready for review either initially or comments have been address label Oct 20, 2025

john-westcott-iv requested review from BrennanPaciorek and zkayyali812 October 20, 2025 13:45

tznamena reviewed Oct 20, 2025

View reviewed changes

ansible_base/lib/utils/duration.py Show resolved Hide resolved

ansible_base/authentication/authenticator_plugins/local.py Outdated Show resolved Hide resolved

AlanCoding reviewed Oct 20, 2025

View reviewed changes

ansible_base/lib/utils/imports.py Show resolved Hide resolved

bhavenst reviewed Oct 20, 2025

View reviewed changes

ansible_base/authentication/authenticator_plugins/local.py Show resolved Hide resolved

ansible_base/authentication/authenticator_plugins/local.py Show resolved Hide resolved

bhavenst approved these changes Oct 23, 2025

View reviewed changes

john-westcott-iv and others added 16 commits October 23, 2025 13:05

Fixing minor issues

3002fd8

[AAP-49757] Add exception logging to convert_to_seconds

2f1779d

john-westcott-iv and others added 3 commits October 23, 2025 13:05

test

156d9c4

Revert "test"

e8da3f4

This reverts commit e4be818.

BrennanPaciorek force-pushed the AAP-49757 branch from 16c9cec to e8da3f4 Compare October 23, 2025 17:05

BrennanPaciorek approved these changes Oct 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AAP-49757] Add fallback authentication support to local authenticator #868

[AAP-49757] Add fallback authentication support to local authenticator #868

Uh oh!

john-westcott-iv commented Oct 20, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bhavenst left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 23, 2025

Uh oh!

sonarqubecloud bot commented Oct 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[AAP-49757] Add fallback authentication support to local authenticator #868

Are you sure you want to change the base?

[AAP-49757] Add fallback authentication support to local authenticator #868

Uh oh!

Conversation

john-westcott-iv commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Primary Feature: Fallback Authentication System

Code Quality Improvements

1. Duration Utility Refactoring

2. Import Utility Consolidation

3. Regex Pattern Optimization

Configuration Example

Plugin Contract

Security Considerations

Type of Change

Testing

Fallback Authentication Tests (44 tests)

Duration Utility Tests (79 test cases)

Import Utility Tests (22 test cases)

Test Results

Dependencies

Additional Context

Architecture Benefits

Backward Compatibility

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bhavenst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 23, 2025

Uh oh!

sonarqubecloud bot commented Oct 23, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

john-westcott-iv commented Oct 20, 2025 •

edited

Loading