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

StartWithSrpAuthAsync fails with a parse exception #48

Closed
efess opened this issue Mar 10, 2020 · 12 comments
Closed

StartWithSrpAuthAsync fails with a parse exception #48

efess opened this issue Mar 10, 2020 · 12 comments
Labels
bug This issue is a bug. closed-for-staleness module/cognito-ext response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@efess
Copy link

efess commented Mar 10, 2020

I have a cognito user pool built using the email as a sign-in alias. When I use user_1@someemail.com as a username using StartWithSrpAuthAsync, the library throws an exception "The value could not be parsed."

I narrowed this down to the fact the salt string is a negative hex value, which the lib is assuming is always unsigned. This error is occuring in AuthenticationHelper.cs:

var salt = BigIntegerExtensions.FromLittleEndianHex(saltString);

Is it possible to support signed salt values here? I tried this in nodejs Amplify and they seem to handle it w/o an issue.

@klaytaybai klaytaybai added the needs-triage This issue or PR still needs to be triaged. label Mar 10, 2020
@efess
Copy link
Author

efess commented Mar 10, 2020

OK I guess this happens when trying to initiateAuth using an email which hasn't been verified yet.

The fix for me is to specify email_verified = true on user creation, then the SRP authentication works as intended.

Not sure if any action needs to be taken to handle that particular case, otherwise I can close this

@klaytaybai
Copy link

Thanks for the feedback. I'll look into whether we can use the signed salt values. I don't want to recommend setting email_verified = true unless it has been verified or isn't critical for you.

@timcromarty
Copy link

Getting the same issue. However, do not want to set email_verified as true as I need to log in to obtain a session from which I can call ResponseToNewPasswordRequiredAsync in order to change the temporary password (and confirm the email)

@dtarczynski
Copy link

Having the same issue where I use Email as UserName for not activated accounts. This way I can't response to challenge and require to validate Email by providing validation code in next step.

@vazquezbonilla
Copy link

I have email_verified as true and I'm getting the same issue

@DevonHansen
Copy link

DevonHansen commented Jun 22, 2020

Getting this issue as well on a user that has been verified and has been able to log in prior.
Edit: This is when a user doesn't exist on the database. So what I think may be happening is that a user has an email such as "bob@gmail.com", and is also using "bob+1@gmail.com". This is the only thing that separates between this format exception and an unknown user error, and the format issue is because the salt hex is negative.
Stack trace below

at System.Numerics.BigNumber.ParseBigInteger (System.ReadOnlySpan'1[T] value, System.Globalization.NumberStyles style, System.Globalization.NumberFormatInfo info) [0x0001e] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/external/corefx/src/System.Runtime.Numerics/src/System/Numerics/BigNumber.cs:386 
  at System.Numerics.BigNumber.ParseBigInteger (System.String value, System.Globalization.NumberStyles style, System.Globalization.NumberFormatInfo info) [0x0000e] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/external/corefx/src/System.Runtime.Numerics/src/System/Numerics/BigNumber.cs:374 
  at System.Numerics.BigInteger.Parse (System.String value, System.Globalization.NumberStyles style, System.IFormatProvider provider) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/external/corefx/src/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs:675 
  at System.Numerics.BigInteger.Parse (System.String value, System.Globalization.NumberStyles style) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/external/corefx/src/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs:665 
  at Amazon.Extensions.CognitoAuthentication.Util.BigIntegerExtensions.FromUnsignedLittleEndianHex (System.String hex) [0x0000b] in <bfb077a8c7a341ef900f115d002a1c51>:0 
  at Amazon.Extensions.CognitoAuthentication.Util.AuthenticationHelper.AuthenticateUser (System.String username, System.String password, System.String poolName, System.Tuple`2[T1,T2] tupleAa, System.String saltString, System.String srpbString, System.String secretBlockBase64, System.String formattedTimestamp) [0x0003b] in <bfb077a8c7a341ef900f115d002a1c51>:0 
  at Amazon.Extensions.CognitoAuthentication.CognitoUser.CreateSrpPasswordVerifierAuthRequest (Amazon.CognitoIdentityProvider.Model.InitiateAuthResponse challenge, System.String password, System.Tuple`2[T1,T2] tupleAa) [0x000a8] in <bfb077a8c7a341ef900f115d002a1c51>:0 
  at Amazon.Extensions.CognitoAuthentication.CognitoUser.StartWithSrpAuthAsync (Amazon.Extensions.CognitoAuthentication.InitiateSrpAuthRequest srpRequest) [0x000f2] in <bfb077a8c7a341ef900f115d002a1c51>:0 
  <ommited project specific stack>

@ashishdhingra
Copy link
Contributor

ashishdhingra commented Oct 13, 2020

Hi @efess,

Good afternoon.

I was going through the issue backlog and came across this issue. I tested the functionality and it appears that StartWithSrpAuthAsync() works when a verified email address (e.g. test_email@somedomain.com) having special character _, is used in place of user name. Also, for un-verified email address, I didn't got any parse exception, it didn't allowed me to login with an invalid username/password exception.

AWSSDK.Extensions.CognitoAuthentication Version: 0.9.4

Please confirm if we could close this issue.

Thanks,
Ashish

@ashishdhingra ashishdhingra added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 13, 2020
@efess
Copy link
Author

efess commented Oct 13, 2020

@ashishdhingra the issue is when you try to initiate auth using an email address which has not been verified yet. This will throw an exception within the library rather than any useful information telling the user that the email address needs to be verified first.

Edit: If it's not reproducible, please close it. I'm off the project which I originally experienced this issue so I cannot test myself.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 14, 2020
@ashishdhingra
Copy link
Contributor

Hi @timcromarty / @dtarczynski / @vazquezbonilla / @DevonHansen,

Good afternoon.

Please review the above comments and let me know if this is still an issue. Else, since the issue is not reproducible, we could close this issue.

Thanks,
Ashish

@ashishdhingra ashishdhingra added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 5, 2020
@mattmapadmi
Copy link

This seems to have fixed itself for me overnight. I don't manage our Cognito instance so can't say for certain if something has changed there.

I was previously testing with test@test.com and password and getting the FormatException, but now getting NotAuthorizedException as you'd expect.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 7, 2020
@DevonHansen
Copy link

I'll look to see if I can replicate sometime today. We have a workaround where we assume that FormatException is a reason to not log the person in for invalid details.

@ashishdhingra ashishdhingra added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 10, 2020
@github-actions
Copy link

This issue has not recieved a response in 2 weeks. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Nov 24, 2020
@github-actions github-actions bot added closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Dec 1, 2020
@github-actions github-actions bot closed this as completed Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closed-for-staleness module/cognito-ext response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

8 participants