search

aws / aws-sdk-net-extensions-cognito

An extension library to assist in the Amazon Cognito User Pools authentication process
Apache License 2.0
102 stars 49 forks source link

Device SRP Authentication #46

Closed dcolclazier closed 3 years ago

dcolclazier commented 4 years ago

Issue #, if available:

Issue 44, Issue 28, Issue 10

Description of changes: Fixes "Key already in dictionary" exception in StartWithSrpAuthAsync. Key is already being added earlier in authentication process, so to maintain existing logic, we simply use an indexer to 'add' the device key to the challenge auth response (Deeper fix could look at why it is being added twice)

Adds necessary logic to handle Device SRP authentication after a successful User SRP_AUTH and PASSWORD_VERIFIER. Within StartWithSrpAuthAsync, if the verifierResponse from the SrpPasswordVerifierAuthRequest is null, we attempt to respond to a "DEVICE_SRP_AUTH" challenge, as well as a "DEVICE_PASSWORD_VERIFIER" challenge.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

dcolclazier commented 4 years ago

I noticed a bug in this pull request where either the generated seed or password verifier could be negative, causing an error to be thrown back from AWS. Have a lot of other things on the plate, but will fix as soon as I can.

dcolclazier commented 3 years ago

Have there been any updates on this? Looks like we're still waiting for a review.

SrutiG commented 3 years ago

@dcolclazier sorry for the delay! We are currently investigating this issue and will review the pull request once we have a better understanding of it.

SrutiG commented 3 years ago

@dcolclazier Update: We have been able to reproduce this issue. We are currently working on reviewing your PR and checking how we can fix the seed/password verifier negative issue.

gertjangaillet commented 3 years ago

Hi, would you have any update about the timelines of the fix, please?

SrutiG commented 3 years ago

@gertjangaillet We have figured out a solution for the negative seed/password verifier issue. This is pending a future release.

gertjangaillet commented 3 years ago

Thanks @SrutiG - when is a release planned? Are we talking about days, weeks, ...?

SrutiG commented 3 years ago

We fixed the negative salt/password verifier issue and did some minor cleanup, this PR is now merged into master. Thank you for your contribution @dcolclazier!