Skip to content

Conversation

maryamkhidir
Copy link
Collaborator

Issue #, if available:

Description

This change adds a custom attribute configuration to the Cognito User Pool to support OIDC (OpenID Connect) and SAML authentication deployments. This enhancement improves the authentication flow by ensuring proper attribute mapping for federated identity providers.

Files Changed

  • lib/authentication/index.ts - Core authentication logic updated to include custom attribute configuration
  • Test snapshots updated to reflect the new Cognito configuration:
    • tests/snapshots/cdk-app.test.ts.snap
    • tests/authentication/snapshots/autentication-construct.test.ts.snap
    • tests/chatbot-api/snapshots/chatbot-api-construct.test.ts.snap

Impact

• Enables proper attribute handling for federated authentication scenarios
• Maintains backward compatibility with existing authentication flows
• Updates test snapshots to ensure CI/CD pipeline continues to pass

Testing

• All existing tests updated with new snapshots

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Collaborator

@charles-marion charles-marion left a comment

Choose a reason for hiding this comment

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

Approved but please review my comment before merging.

@@ -34,6 +34,9 @@ export class Authentication extends Construct {
signInAliases: {
email: true,
},
customAttributes: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this is only used when federation is used. I would conditionally set this.

To my knowledge the attribute was set without this cdk property. (but I only tested once a while back)

Also note this other change: https://github.com/aws-samples/aws-genai-llm-chatbot/pull/672/files

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