-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: remove dependency on amplifyconfiguration.json #58
base: main
Are you sure you want to change the base?
Conversation
self.cognito = CognitoConfiguration( | ||
usernameAttributes: configuration.usernameAttributes, | ||
signupAttributes: configuration.signUpAttributes, | ||
passwordProtectionSettings: configuration.passwordProtectionSettings ?? .init(minLength: 0, characterPolicy: []), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❗ This is a change in existing behaviour. Currently, the Authenticator requires these password settings to be explicitly set, showing an error if that is not the case.
But now we will just default to not having password validations.
I don't feel strongly against this, but I think Android also requires these values so we should coordinate to make the change there as well. And also call it out in the release notes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great callout, will follow up on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: We need to update the Package.swift
file to use the new Amplify version that contains this change, otherwise it will not compile.
Issue #, if available:
Description of changes:
This PR includes the changes that no longer depend on
jsonConfiguration
to get the auth configuration values. This PR needs to be updated with a version upgrade on the Amplify dependency to get theauthConfiguration
changes at the same time. It depends on this PR to be merged and released first: aws-amplify/amplify-swift#3566By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.