Skip to content
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

Update CognitoSecretHash.java #593

Merged
merged 2 commits into from
Dec 5, 2018
Merged

Conversation

NischalManandhar
Copy link
Contributor

While creating CognitoUserPool instance using the awsConfiguration.json file with following description.
"CognitoUserPool": {
"Default": {
"PoolId": "xxxxxxxxxx",
"AppClientId": "xxxxxxxx",
"AppClientSecret": "",
"Region": "xxxx"
}
}

It will return appClientSecret as "" which is not checked in this file and cause crash in android application.

While creating CognitoUserPool instance using the awsConfiguration.json file with following description.
"CognitoUserPool": {
    "Default": {
      "PoolId": "xxxxxxxxxx",
      "AppClientId": "xxxxxxxx",
      "AppClientSecret": "",
      "Region": "xxxx"
    }
  }

It will return appClientSecret as "" which is not checked in this file and cause crash in android application.
@palpatim palpatim added cognito Issues with the AWS Android SDK for Cognito pull request labels Nov 29, 2018
@palpatim palpatim requested a review from minbi November 29, 2018 15:14
Copy link
Member

@palpatim palpatim left a comment

Choose a reason for hiding this comment

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

This change generally looks good to me, with one small typo in the comment.

I'll wait for @minbi to give the deciding review, though.

Closed the last quote in comment
Copy link
Contributor

@minbi minbi left a comment

Choose a reason for hiding this comment

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

Thanks for tracking this down.

@minbi
Copy link
Contributor

minbi commented Dec 3, 2018

Fixes #587

@LithiumSheep
Copy link

Just throwing in my two cents, but would it make sense to use the already existing com.amazonaws.util.StringUtils instead of importing android TextUtils?

There is already a method public static boolean isBlank(CharSequence cs) that exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cognito Issues with the AWS Android SDK for Cognito
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants