Skip to content

Conversation

zkayyali812
Copy link
Contributor

@zkayyali812 zkayyali812 commented Jul 15, 2025

Description

  • What is being changed? We are planning to merge our feature branch into devel to leverage quicker testing and feedback from the nightlies.
  • Why is this change needed? To continuously deliver on 1459, and to get more iterative feedback from the nightlies.
  • How does this change address the issue? This change addresses the issue by merging our in progress feature branch into devel to incorporate our already validated changes.

@AlanCoding attempted summary:

This includes 2 merges to the feature branch of 1459

The change descriptions are there.

  • Allowing the authenticator_map (field @john-westcott-iv ) serializer to store {% for_attr_value(something) %}
    • This will allow the authenticator maps to have templates in them. (so basically, adding templating to the authenticator_maps field?)
  • email field is added
    • This will make it easier to debug issues regarding how users authenticate into the platform, since we will be able to tell directly which email address their authenticatoruser is configured with.
    • the PoC for this was [PoC] [AAP-48066] Make users join by emai #751
    • the PoC used the email from the User model, which is likely improved by moving the emtil to the AuthenticatorUser

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Test update

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

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 and others added 2 commits July 15, 2025 09:38
## Description
- What is being changed?
The AuthenticatorUser model is extended with the `email` field. This PR
is only to add the field, which will be populated by a later ticket-
https://issues.redhat.com/browse/AAP-48066.
- Why is this change needed?
This change is needed to associate AuthenticatorUsers directly with
their email address. This will make it easier to debug issues regarding
how users authenticate into the platform, since we will be able to tell
directly which email address their authenticatoruser is configured with.
When authenticating via multiple different authenticators, we will sign
the user into the AAP account associated with their email.
- How does this change address the issue?
This change addresses the issue by adding the email field to the
AuthenticatorUser table, which will be later populated by AAP-48066.

## Type of Change
<!-- Mandatory: Check one or more boxes that apply -->
- [x] New feature (non-breaking change which adds functionality)

## Self-Review Checklist
<!-- These items help ensure quality - they complement our automated CI
checks -->
- [ ] 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
<!-- 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. 
2. 
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 -->
## Description
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
- What is being changed?
Allowing the authenticator_map serializer to store `{%
for_attr_value(something) %}`

- Why is this change needed?
This stages authenticator_maps to support a missing SAML option.

- How does this change address the issue?
This will allow the authenticator maps to have templates in them.

## Type of Change
<!-- Mandatory: Check one or more boxes that apply -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] 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. Log into your instance and create an authenticator_map with different
formats of {% in it.
2. 
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 -->
@zkayyali812 zkayyali812 force-pushed the Feature-ANSTRAT-1459 branch from 1e6b964 to 2552fc9 Compare July 15, 2025 13:38
Copy link

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

@zkayyali812 zkayyali812 marked this pull request as ready for review July 15, 2025 13:40
Copy link

@zkayyali812 zkayyali812 merged commit 316ebce into devel Jul 15, 2025
24 checks passed
@zkayyali812 zkayyali812 deleted the Feature-ANSTRAT-1459 branch July 15, 2025 17:32
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.

3 participants