Skip to content

Conversation

zkayyali812
Copy link
Contributor

@zkayyali812 zkayyali812 commented Aug 21, 2025

Putting this PR on hold for now as it will be part of another JIRA eventually. JIRA# TBD.

Description

  • What is being changed? Updates the default for JWT_ALGORITHMS. If not specified, we will use the default from .well-known/configuration endpoint
  • Why is this change needed? To set sane defaults with an OIDC authenticator is configured
  • How does this change address the issue? If no authenticators are specified, we will save the authenticator using the defaults from the .well-known endpoint of the OIDC authenticator.

This PR is an optional alternative to #820

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. Create a Generic OIDC authenticator without specifying any algorithms
  2. Ensure well known algorithms are populated upon save.

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

@zkayyali812 zkayyali812 marked this pull request as ready for review August 21, 2025 23:04
@zkayyali812 zkayyali812 force-pushed the zk/AAP-50420-well-known branch from ba959b1 to 6728570 Compare August 22, 2025 14:25
@zkayyali812 zkayyali812 force-pushed the zk/AAP-50420-well-known branch from 6728570 to 75adf22 Compare August 22, 2025 14:47
Copy link

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

Copy link

@john-westcott-iv john-westcott-iv changed the title Add Default JWT Algorithms from .well-known endpoint [HOLD] Add Default JWT Algorithms from .well-known endpoint Aug 25, 2025
on_delete=SET_NULL,
)

def save_default_jwt_algorithms(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should move this out of the model. It shouldn't know about specifics for requirements of any of the plugins.

@john-westcott-iv john-westcott-iv changed the title [HOLD] Add Default JWT Algorithms from .well-known endpoint [HOLD] [AAP-52033] Add Default JWT Algorithms from .well-known endpoint Aug 25, 2025
john-westcott-iv added a commit that referenced this pull request Aug 25, 2025
## Description
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
- What is being changed? Updates the default for `JWT_ALGORITHMS`. If
not specified, we will use the default from
`OpenIdConnectAuth.JWT_ALGORITHMS`
- Why is this change needed? This change is needed to ensure a sane
default set of JWT_ALGORITHMS is applied, if none are specified.
- How does this change address the issue? This change addresses the
issue, by ensuring we apply a default, instead of specifying None.
- 
This PR is an optional alternative to #821 

## Type of Change
<!-- Mandatory: Check one or more boxes that apply -->
- [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
<!-- These items help ensure quality - they complement our automated CI
checks -->
- [x] I have performed a self-review of my code
- [x] I have added relevant comments to complex code sections
- [x] I have updated documentation where needed
- [x] I have considered the security impact of these changes
- [x] I have considered performance implications
- [x] I have thought about error handling and edge cases
- [x] I have tested the changes in my local environment

## Testing Instructions
<!-- Optional for test-only changes. Mandatory for all other changes -->
<!-- Must be detailed enough for reviewers to reproduce -->
### Prerequisites
<!-- List any specific setup required -->

### Steps to Test
1. Create a Generic OIDC authenticator without specifying any algorithms
2. Ensure default algorithm is populated upon save.
3. 

### Expected Results
<!-- Describe what should happen after following the steps -->

## Additional Context
<!-- Optional but helpful information -->

### Required Actions
<!-- Check if changes require work in other areas -->
<!-- Remove section if no external actions needed -->
- [ ] Requires documentation updates
  <!-- API docs, feature docs, deployment guides -->
- [ ] Requires downstream repository changes
  <!-- Specify repos: django-ansible-base, eda-server, etc. -->
- [ ] Requires infrastructure/deployment changes
  <!-- CI/CD, installer updates, new services -->
- [ ] Requires coordination with other teams
  <!-- UI team, platform services, infrastructure -->
- [ ] Blocked by PR/MR: #XXX
  <!-- Reference blocking PRs/MRs with brief context -->

### Screenshots/Logs
<!-- Add if relevant to demonstrate the changes -->

---------

Co-authored-by: John Westcott IV <[email protected]>
fosterseth pushed a commit to fosterseth/django-ansible-base that referenced this pull request Sep 8, 2025
## Description
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
- What is being changed? Updates the default for `JWT_ALGORITHMS`. If
not specified, we will use the default from
`OpenIdConnectAuth.JWT_ALGORITHMS`
- Why is this change needed? This change is needed to ensure a sane
default set of JWT_ALGORITHMS is applied, if none are specified.
- How does this change address the issue? This change addresses the
issue, by ensuring we apply a default, instead of specifying None.
- 
This PR is an optional alternative to ansible#821 

## Type of Change
<!-- Mandatory: Check one or more boxes that apply -->
- [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
<!-- These items help ensure quality - they complement our automated CI
checks -->
- [x] I have performed a self-review of my code
- [x] I have added relevant comments to complex code sections
- [x] I have updated documentation where needed
- [x] I have considered the security impact of these changes
- [x] I have considered performance implications
- [x] I have thought about error handling and edge cases
- [x] I have tested the changes in my local environment

## Testing Instructions
<!-- Optional for test-only changes. Mandatory for all other changes -->
<!-- Must be detailed enough for reviewers to reproduce -->
### Prerequisites
<!-- List any specific setup required -->

### Steps to Test
1. Create a Generic OIDC authenticator without specifying any algorithms
2. Ensure default algorithm is populated upon save.
3. 

### Expected Results
<!-- Describe what should happen after following the steps -->

## Additional Context
<!-- Optional but helpful information -->

### Required Actions
<!-- Check if changes require work in other areas -->
<!-- Remove section if no external actions needed -->
- [ ] Requires documentation updates
  <!-- API docs, feature docs, deployment guides -->
- [ ] Requires downstream repository changes
  <!-- Specify repos: django-ansible-base, eda-server, etc. -->
- [ ] Requires infrastructure/deployment changes
  <!-- CI/CD, installer updates, new services -->
- [ ] Requires coordination with other teams
  <!-- UI team, platform services, infrastructure -->
- [ ] Blocked by PR/MR: #XXX
  <!-- Reference blocking PRs/MRs with brief context -->

### Screenshots/Logs
<!-- Add if relevant to demonstrate the changes -->

---------

Co-authored-by: John Westcott IV <[email protected]>
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.

2 participants