Skip to content

Conversation

john-westcott-iv
Copy link
Member

@john-westcott-iv john-westcott-iv commented Aug 21, 2025

Description

  • What is being changed?
    Controller 2.4 defaulted to using the social_auth library defaults of [RS256].
    Additionally, as an enhancement, we will first attempt to pull the keys from the providers .well-know configuration. If that fails for any reason we will go back to the library defaults.

  • Why is this change needed?
    In gateway we did not default to values from the library, the admin has to enter the values. This is painful and not inline with the controller defaults.

  • How does this change address the issue?
    See what is being changed.

Type of Change

  • 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

  • I have performed a self-review of my code
  • 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
  • I have tested the changes in my local environment

Testing Instructions

Prerequisites

Steps to Test

  1. Have ATF create you an authenticator. In test-suite create a test method like:
    def test_plumb_oidc(
        self,
        api_registry: APIRegistry,
        app: AnsibleAutomationPlatformTestingApplication,
        factories: Factories,
        keycloak_service_saml_oidc: HttpApiService,
    ) -> None:
        """When expected attributes for mapping are missing from the identity provider,
        an error is logged but authentication is handled gracefully.
        """
        oidc_authenticator = (
            api_registry.gateway.authenticator.list_resource(
                type_icontains=TypeEnum.ANSIBLE_BASE_AUTHENTICATION_AUTHENTICATOR_PLUGINS_OIDC
            )
            .assert_and_get()
            .results.pop()
        )
        breakpoint()
  1. Run the test. When you hit the breakpoint whatever environment you pointed this to will now have a "Generic OIDC" authenticator in it.
  2. Manipulate the keycloak and authenticator however you want and test logins.
  3. You can also use breakpoints in the gateway code to enable debugging.

Expected Results

Additional Context

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

@john-westcott-iv john-westcott-iv changed the title Altering how JWT_ALGORITHMS work in oidc authenticator [AAP-50420] Altering how JWT_ALGORITHMS work in oidc authenticator Aug 21, 2025
…ovements

- Enhanced _get_jwt_algorithms method with robust fallback logic:
  * Admin-specified algorithms take priority
  * Fallback to OIDC provider configuration
  * Graceful fallback to Django settings or library defaults on errors
  * Added comprehensive debug and error logging

- Improved user_data method with JWT 'none' algorithm support:
  * Support for unencrypted tokens when algorithms=['none']
  * Enhanced error handling and logging for JWT decode failures
  * Better public key validation with detailed error messages

- Comprehensive test suite improvements:
  * Parameterized 8 individual tests into 2 efficient test classes
  * TestGetJwtAlgorithms: 5 scenarios covering all JWT algorithm flows
  * TestUserDataWithNoneAlgorithm: 3 scenarios for JWT processing + edge cases
  * Integrated expected_log fixture for proper logging assertions
  * Hybrid approach: expected_log for single calls, mock_logger for multiple calls
  * All tests maintain cognitive complexity < 15 and pass linting

- Code quality enhancements:
  * Fixed f-string with internationalization to legacy % formatting
  * Updated exception handling for better Django settings integration
  * Maintained backward compatibility with controller 2.4
  * All functions have cognitive complexity between 1-4 (well below 15 limit)

Co-Authored-By: Andrew Potozniak <[email protected]>
Co-Authored-By: Claude AI <[email protected]>
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

@john-westcott-iv
Copy link
Member Author

Closing in favor of #821

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant