aws-amplify / aws-sdk-ios

AWS SDK for iOS. For more information, see our web site:
https://aws-amplify.github.io/docs
Other
1.67k stars 877 forks source link

Custom challenge OTP: session expires after 1 retry #4964

Closed juchampagne closed 9 months ago

juchampagne commented 9 months ago

I have implemented the OTP sign in using AWSCognitoIdentityProvider a while ago, and have just now noticed that I can only attempt to verify the OTP twice, instead of 3 times, with the latest versions (currently on 2.33.4).

I initiate the sign in process by calling getSession, which will send the OTP to the user, and getCustomChallengeDetails is called one time on success with the USERNAME set. Then the user will enter the OTP and i set the ANSWER on the completion source, which will on success call the continuation block of getSession with a session as the result, or getCustomChallengeDetails on failure without a USERNAME set. If it fails the first time, i can retry only one more time, and if it fails the second time, getCustomChallengeDetails is not called, the error returned in the continuation block of getSession is notAuthorized, and the task is now marked as completed so i cannot retry a third time.

Unless i am doing anything wrong, it seems that the sdk is invalidating the session after just one retry, on Android with a similar implementation, it allows 2 retries before throwing the notAuthorized error, so the user can try 3 times overall which is expected from the lambdas on the server.

Here is my implementation:

class AWSV2Signer: NSObject, AWSCognitoIdentityInteractiveAuthenticationDelegate, AWSCognitoIdentityCustomAuthentication {

    private var _userPool: AWSCognitoIdentityUserPool!

    private var _customAuthChallengeTaskCompletionSource: AWSTaskCompletionSource<AWSCognitoIdentityCustomChallengeDetails>?
    private var _customUsername = ""

    func startCustomAuthentication() -> AWSCognitoIdentityCustomAuthentication {
        return self
    }

    func signIn(phoneNumber: String) {
        if let user = self._userPool.getUser(alias: phoneNumber) {

            user.getSession(phoneNumber, password: UUID().uuidString, validationData: nil, clientMetaData: nil, isInitialCustomChallenge: true).continueWith(block: {
                task in
                if let error = task.error as NSError? {
                    // error signin
                }
                else if let session = task.result as? AWSCognitoIdentityUserSession {
                    // successfull signin
                }
                return nil
            })
        }
    }

    func confirmSignIn(phoneNumber: String, code: String) {
        let customAuthDetails = AWSCognitoIdentityCustomChallengeDetails(challengeResponses: ["USERNAME": _customUsername, "ANSWER": code])
        _customAuthChallengeTaskCompletionSource?.set(result: customAuthDetails)
    }

    func getCustomChallengeDetails(_ authenticationInput: AWSCognitoIdentityCustomAuthenticationInput, customAuthCompletionSource: AWSTaskCompletionSource<AWSCognitoIdentityCustomChallengeDetails>) {
        _customAuthChallengeTaskCompletionSource = customAuthCompletionSource

        if authenticationInput.challengeParameters["USERNAME"] != nil {
            _customUsername = authenticationInput.challengeParameters["USERNAME"]!
            // OTP successfully sent, ask user to enter the OTP
        }
        else {
            // wrong OTP entered, ask user to enter again
        }
    }

    func didCompleteStepWithError(_ error: Error?) {
    }

}
harsh62 commented 9 months ago

@juchampagne Thanks for opening the issue. Since you using a Custom Challenge to sign in, the logic of expiring OTP's should exist in the Lamda Triggers that were added with the custom Auth challenge. I would suggest to either explore the lambdas to find the business logic that is causing this issue.

If you are still not able to figure out, kindly share all the lambdas associated with the Custom Auth and we can try to see if something is wrong there or not.

Hope this helps.

juchampagne commented 9 months ago

@harsh62 thanks for the reply, please find the lambdas below, on Android with these same lambdas, the OTP expires after 3 tries, but only 2 tries on iOS.

create:

exports.handler = createHandler(async (event, context) => {
  let secretLoginCode
  console.log(event.request.session)
  if (!event.request.session
    || !event.request.session.length
    || event.request.session.length === 0
    || (event.request.session && event.request.session.length && event.request.session.slice(-1)[0].challengeName === 'SRP_A')) {

    const loginConfig = await cognitoService.getLoginConfig({ sub: event.request.userAttributes.sub }, context.redis)
    secretLoginCode = loginConfig.code

    ${secretLoginCode}`)
    await sendSms(event.request.userAttributes.phone_number, secretLoginCode)
  } else {
    // If there is an existing session then it's a retrial, don't generate new code
    const previousChallenge = event.request.session.slice(-1)[0]
    secretLoginCode = previousChallenge.challengeMetadata.match(/CODE-(\d*)/)[1]
  }

  event.response.publicChallengeParameters = {
    email: event.request.userAttributes.email
  }

  event.response.privateChallengeParameters = { secretLoginCode }

  event.response.challengeMetadata = `CODE-${secretLoginCode}`

  return event
})
  .use(cacheManager())

const sendSms = async (phoneNumber, secretLoginCode) => {
  await notifier.send({
    sms: {
      from: 'test',
      to: phoneNumber,
      text: `Your temporary login code is : ${secretLoginCode}`
    }
  })
}

define:

exports.handler = async event => {
  if (event.request.session && event.request.session.length && event.request.session.slice(-1)[0].challengeName === 'SRP_A') {
    event.request.session = []
    event.response.issueTokens = false
    event.response.failAuthentication = false
    event.response.challengeName = 'CUSTOM_CHALLENGE'
  } else if (
    event.request.session &&
    event.request.session.length >= 3 &&
    event.request.session.slice(-1)[0].challengeResult === false
  ) {
    event.response.issueTokens = false
    event.response.failAuthentication = true
  } else if (
    event.request.session &&
    event.request.session.length &&
    event.request.session.slice(-1)[0].challengeResult === true
  ) {
    event.response.issueTokens = true
    event.response.failAuthentication = false
  } else {
    event.response.issueTokens = false
    event.response.failAuthentication = false
    event.response.challengeName = 'CUSTOM_CHALLENGE'
  }
  return event
}

and verify:

exports.handler = async event => {
  const expectedAnswer = event.request.privateChallengeParameters.secretLoginCode
  if (event.request.challengeAnswer === expectedAnswer) {
    event.response.answerCorrect = true
  } else {
    event.response.answerCorrect = false
  }
  return event
}
harsh62 commented 9 months ago

@juchampagne

So the problem is that iOS code always start with an SRP_A challenge by default during sign in and there is no way to modify it. Because SRP_A is considered as the first challenge (which is also handled in your lamda as follows), your session.length already has a value of 1 here. So, your temporary code expires in the next 2 attempts because of your lambda logic, event.request.session.length >= 3.

  if (event.request.session && event.request.session.length && event.request.session.slice(-1)[0].challengeName === 'SRP_A') {
    event.request.session = []
    event.response.issueTokens = false
    event.response.failAuthentication = false
    event.response.challengeName = 'CUSTOM_CHALLENGE'
  } 

I believe Android works because it might not be going through the SRP_A flow. That might be possibly because of the else condition in your lambda.

  } else {
    event.response.issueTokens = false
    event.response.failAuthentication = false
    event.response.challengeName = 'CUSTOM_CHALLENGE'
  }
juchampagne commented 9 months ago

@harsh62 ah ok make sense, that can be solved in the lambda definitely👍

Thanks a lot for your help!