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

fix: add isHashed flag to createSrpSession to allow users to defer password hashing to signSrpSession #29

Conversation

simonmcallister0210
Copy link
Owner

@simonmcallister0210 simonmcallister0210 commented Mar 9, 2024

Context

Email and Phone logins don't work because hashing requires the user ID. See this issue for details. To fix this we can allow users to defer hashing until the signing operation. At this point we'll have access to user ID, which will allow users to login with their Email and Phone number

Changes

  • add isHashed parameter to createSrpSession
  • hash password inside signSrpSession if isHashed=fasle
  • update types
  • update docs
  • update tests
  • add integration tests for email and phone number logins

Testing

With password hashing - users can only authenticate with Username (aka. user ID)

const {
  createSrpSession,
  signSrpSession,
  wrapAuthChallenge,
  wrapInitiateAuth,
  createPasswordHash,
} = require("../cognito-srp-helper");
const {
  CognitoIdentityProviderClient,
  InitiateAuthCommand,
  RespondToAuthChallengeCommand,
} = require("@aws-sdk/client-cognito-identity-provider");

(async () => {
  const username = "a2c2e290-6d0f-4a08-a5ca-f0162935f3a6"; // this works
  // const username = "[email protected]"; // this won't work

  const password = "Qwerty1!";
  const poolId = "eu-west-2_eYpv1mFHB";
  const clientId = "18u8119jgbpr464n28s1itk2mq";
  const cognitoIdentityProviderClient = new CognitoIdentityProviderClient({
    region: "eu-west-2",
  });

  const passwordHash = createPasswordHash(username, password, poolId);
  const srpSession = createSrpSession(username, passwordHash, poolId);

  const initiateAuthRes = await cognitoIdentityProviderClient
    .send(
      new InitiateAuthCommand(
        wrapInitiateAuth(srpSession, {
          ClientId: clientId,
          AuthFlow: "USER_SRP_AUTH",
          AuthParameters: {
            CHALLENGE_NAME: "SRP_A",
            USERNAME: username,
          },
        })
      )
    )
    .catch((err) => {
      console.error(err);
      throw err;
    });

  console.log("initiateAuthRes:");
  console.log(initiateAuthRes);

  const signedSrpSession = signSrpSession(srpSession, initiateAuthRes);

  console.log("signedSrpSession:");
  console.log(signedSrpSession);

  const respondToAuthChallengeRes = await cognitoIdentityProviderClient
    .send(
      new RespondToAuthChallengeCommand(
        wrapAuthChallenge(signedSrpSession, {
          ClientId: clientId,
          ChallengeName: "PASSWORD_VERIFIER",
          ChallengeResponses: {
            USERNAME: username,
          },
        })
      )
    )
    .catch((err) => {
      console.error(err);
      throw err;
    });

  console.log("respondToAuthChallengeRes:");
  console.log(respondToAuthChallengeRes);
})();

Without password hashing - users can authenticate with Username (aka. user ID), and email

const {
  createSrpSession,
  signSrpSession,
  wrapAuthChallenge,
  wrapInitiateAuth,
} = require("../cognito-srp-helper");
const {
  CognitoIdentityProviderClient,
  InitiateAuthCommand,
  RespondToAuthChallengeCommand,
} = require("@aws-sdk/client-cognito-identity-provider");

(async () => {
  // const username = "a2c2e290-6d0f-4a08-a5ca-f0162935f3a6"; // this works
  const username = "[email protected]"; // and this works

  const password = "Qwerty1!";
  const poolId = "eu-west-2_eYpv1mFHB";
  const clientId = "18u8119jgbpr464n28s1itk2mq";
  const cognitoIdentityProviderClient = new CognitoIdentityProviderClient({
    region: "eu-west-2",
  });

  const srpSession = createSrpSession(username, password, poolId, false);

  const initiateAuthRes = await cognitoIdentityProviderClient
    .send(
      new InitiateAuthCommand(
        wrapInitiateAuth(srpSession, {
          ClientId: clientId,
          AuthFlow: "USER_SRP_AUTH",
          AuthParameters: {
            CHALLENGE_NAME: "SRP_A",
            USERNAME: username,
          },
        })
      )
    )
    .catch((err) => {
      console.error(err);
      throw err;
    });

  console.log("initiateAuthRes:");
  console.log(initiateAuthRes);

  const signedSrpSession = signSrpSession(srpSession, initiateAuthRes);

  console.log("signedSrpSession:");
  console.log(signedSrpSession);

  const respondToAuthChallengeRes = await cognitoIdentityProviderClient
    .send(
      new RespondToAuthChallengeCommand(
        wrapAuthChallenge(signedSrpSession, {
          ClientId: clientId,
          ChallengeName: "PASSWORD_VERIFIER",
          ChallengeResponses: {
            USERNAME: username,
          },
        })
      )
    )
    .catch((err) => {
      console.error(err);
      throw err;
    });

  console.log("respondToAuthChallengeRes:");
  console.log(respondToAuthChallengeRes);
})();

@simonmcallister0210 simonmcallister0210 marked this pull request as ready for review March 26, 2024 01:29
@simonmcallister0210 simonmcallister0210 merged commit 8c1d279 into main Mar 26, 2024
2 checks passed
simonmcallister0210 added a commit that referenced this pull request Mar 26, 2024
…in `createSrpSession` (#29)

* add isHashed flag to createSrpSession to allow users to defer password hashing to signSrpSession

* fix integration test

* update tests

* add node v21 to test since it's in active use

* fix incorrectly referenced env var

* ban 0 for short public key test cases to stop AbortOnZeroSrpError causing flaking tests

* update integration tests

* update test workflow with new github action secrets

* update github actions node version

* update github actions node version

* drop support for node v16

* only run integration tests after unit tests

* drop tests for node v21 for now

* only run integration tests if unit tests pass

* update API docs in README. add section about password hashing

* rm node v21 for release workflow

* rm comment

* update README examples

* update CONTRIBUTING

* typo

---------

Co-authored-by: Simon McAllister <[email protected]>
Co-authored-by: Simon McAllister <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect password hash when using account alias or regular username
1 participant