-
Notifications
You must be signed in to change notification settings - Fork 249
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
chore!: remove deprecated auth types #4764
Conversation
8d66d80
to
e975914
Compare
/// The default flow used to sign in. | ||
late final AuthFlowType defaultAuthFlowType = () { | ||
// Get the flow from the plugin config | ||
final pluginFlowType = | ||
expect<CognitoPluginConfig>().auth?.default$?.authenticationFlowType ?? | ||
AuthenticationFlowType.userSrpAuth; | ||
return pluginFlowType.sdkValue; | ||
}(); | ||
|
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.
I acknowledge that this field is not present in CLI Gen2. However, with these modifications, we are bypassing the configuration value for users who are still using CLI Gen1.
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.
That is correct. Customers can specify this via SignInOptions though.
The current behavior is to use the value from SignInOptions if it exists. If not, use the value from the config if it exists. If neither exist, AuthenticationFlowType.userSrpAuth
is the default.
The new behavior is to use the value from SignInOptions if it exists. If not, AuthenticationFlowType.userSrpAuth
is the default.
It is a breaking change that will be called out in the change log for v2.
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.
I think we may need to add a note in our Gen1/V2 documentation indicating the value from the configuration file is not used by the library. Instead, users should use the SignInOptions.
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.
Agreed. I meant to say this will be called out in the migration guide
c67ed08
to
c99eb94
Compare
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.