fix(auth): User pool token, user sub should be returned for signedIn user with no identityPool config #632
Conversation
…user with no identityPool config
| @@ -222,4 +220,18 @@ extension AuthorizationProviderAdapter { | |||
| cognitoTokensResult: tokenResult) | |||
| completionHandler(.success(authSession)) | |||
| } | |||
|
|
|||
| /// Check if the error is related to improper configuration of Cognito Identity Pool | |||
| private func identityPoolConfigured(error: Error) -> Bool { | |||
There was a problem hiding this comment.
- See note above re method name
- I don't see this using any instance members, so make this
static
More importantly, it's hard to wade through the double negatives in this method logic: the method comment asserts that we're looking to see if the error is related to improper configuration of an identity pool, but we return false if we find that the error is in fact a cognitoIdentityPoolNotConfigured error. How is that error not related to improper configuration? Why are just these two error conditions sufficient to assert that the error isn't related to improper configuration?
| } else if let cognitoIdentityPoolError = error as NSError?, | ||
| cognitoIdentityPoolError.domain == AWSCognitoIdentityErrorDomain, | ||
| cognitoIdentityPoolError.code == AWSCognitoIdentityErrorType.notAuthorized.rawValue { | ||
| } else if !self.identityPoolConfigured(error: error!) { |
There was a problem hiding this comment.
More context would be handy here. Why is it safe to ignore what appears just be reading code to be a misconfigured identity pool? Changing the method name to self-document the intent of the check would be helpful, but honestly I can't tell what the method below is actually trying to do.
There was a problem hiding this comment.
Changed the method name
Description of changes:
Amplify can be used with only Cognito User Pool configured. With the configuration structure shown below. In this case while calling the api
fetchAuthSessionthe returned result doesnot provide the right values for usersub and cognito tokens. This PR fixes this behavior.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.