feat: allow users to defer password hasing with isHashed parameter …

…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 <simonmcallister@simons-mbp.home> Co-authored-by: Simon McAllister <simonmcallister@Simons-MBP.broadband>
simonmcallister0210 · Mar 26, 2024 · 9d56963 · 9d56963
1 parent 605d5ea
commit 9d56963
Show file tree

Hide file tree

Showing 18 changed files with 565 additions and 233 deletions.
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -10,14 +10,14 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        # https://what-version-of-node.js.org/
-        node-version: [16.x, 18.x, 20.x] # remember to update this when support is added/dropped
+        # https://nodejs.org/en/about/previous-releases#release-schedule
+        node-version: [18, 20] # remember to update this when support is added/dropped
     steps:
       - uses: actions/checkout@v3
       - name: Use Node.js ${{ matrix.node-version }}
         uses: actions/setup-node@v3
         with:
-          node-version: 18 # remember to update this when support is added/dropped
+          node-version: ${{ matrix.node-version }}
       - run: npm ci
       - run: npm run lint
       - run: npm run build

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -11,24 +11,42 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        # https://what-version-of-node.js.org/
-        node-version: [16.x, 18.x, 20.x] # remember to update this when support is added/dropped
+        # https://nodejs.org/en/about/previous-releases#release-schedule
+        node-version: [18, 20] # remember to update this when support is added/dropped
     steps:
       - uses: actions/checkout@v3
       - name: Use Node.js ${{ matrix.node-version }}
         uses: actions/setup-node@v3
         with:
-          node-version: 18 # remember to update this when support is added/dropped
+          node-version: ${{ matrix.node-version }}
       - run: npm ci
       - run: npm run lint
       - run: npm run build
-      - run: npm run test
+      - run: npm run test:unit
+      - run: npm run test:integration
         env:
-          AWS_ACCESS_KEY_ID: ${{secrets.AWS_ACCESS_KEY_ID}}
-          AWS_SECRET_ACCESS_KEY: ${{secrets.AWS_SECRET_ACCESS_KEY}}
-          INT_TEST_USERNAME: ${{secrets.INT_TEST_USERNAME}}
-          INT_TEST_PASSWORD: ${{secrets.INT_TEST_PASSWORD}}
-          INT_TEST_POOL_ID: ${{secrets.INT_TEST_POOL_ID}}
-          INT_TEST_CLIENT_ID: ${{secrets.INT_TEST_CLIENT_ID}}
-          INT_TEST_SECRET_ID: ${{secrets.INT_TEST_SECRET_ID}}
-          INT_TEST_AWS_REGION: ${{secrets.INT_TEST_AWS_REGION}}
+          # Credentials
+          AWS_REGION: ${{ secrets.AWS_REGION }}
+          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
+          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
+
+          # Username
+          INT_TEST__USERNAME__USERNAME: ${{ secrets.INT_TEST__USERNAME__USERNAME }}
+          INT_TEST__USERNAME__PASSWORD: ${{ secrets.INT_TEST__USERNAME__PASSWORD }}
+          INT_TEST__USERNAME__POOL_ID: ${{ secrets.INT_TEST__USERNAME__POOL_ID }}
+          INT_TEST__USERNAME__CLIENT_ID: ${{ secrets.INT_TEST__USERNAME__CLIENT_ID }}
+          INT_TEST__USERNAME__SECRET_ID: ${{ secrets.INT_TEST__USERNAME__SECRET_ID }}
+
+          # Email
+          INT_TEST__EMAIL__USERNAME: ${{ secrets.INT_TEST__EMAIL__USERNAME }}
+          INT_TEST__EMAIL__PASSWORD: ${{ secrets.INT_TEST__EMAIL__PASSWORD }}
+          INT_TEST__EMAIL__POOL_ID: ${{ secrets.INT_TEST__EMAIL__POOL_ID }}
+          INT_TEST__EMAIL__CLIENT_ID: ${{ secrets.INT_TEST__EMAIL__CLIENT_ID }}
+          INT_TEST__EMAIL__SECRET_ID: ${{ secrets.INT_TEST__EMAIL__SECRET_ID }}
+
+          # Phone
+          INT_TEST__PHONE__USERNAME: ${{ secrets.INT_TEST__PHONE__USERNAME }}
+          INT_TEST__PHONE__PASSWORD: ${{ secrets.INT_TEST__PHONE__PASSWORD }}
+          INT_TEST__PHONE__POOL_ID: ${{ secrets.INT_TEST__PHONE__POOL_ID }}
+          INT_TEST__PHONE__CLIENT_ID: ${{ secrets.INT_TEST__PHONE__CLIENT_ID }}
+          INT_TEST__PHONE__SECRET_ID: ${{ secrets.INT_TEST__PHONE__SECRET_ID }}
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -5,28 +5,26 @@ Table of Contents:
 1. [Steps for Implementing a Fix or Feature](#steps-for-implementing-a-fix-or-feature)
 2. [Conventional Commit](#conventional-commit)
 3. [Releases](#releases)
-4. [VSCode setup](#vscode-setup)
-5. [Running integration tests locally](#running-integration-tests-locally)
-6. [Supporting old major releases](#supporting-old-major-releases)
+4. [Running integration tests locally](#running-integration-tests-locally)
 
 ## Steps for Implementing a Fix or Feature
 
-There's no strict requirements for adding a fix or feature, just open an issue and a PR
+There's no strict requirements for adding a fix or feature, just open an issue and a PR. Code owners will make any necessary amendments
 
 ## Conventional Commit
 
 This projects follows the Conventional Commit specification (at least for commits on main):
 
 https://www.conventionalcommits.org/en/v1.0.0/
 
-This allows us to categorize changes and make automated updates to our project version and change log based on the categories being merged. For example, if you commit a feature, you could set the commit message to be:
+This allows us to categorize changes and make automated updates to our project version and change log based on the categories being merged
+
+When merging your PR the you should use squash-merge, and use the default PR commit message with a conventional commit prefix, e.g.
 
 ```sh
-git commit -m 'feat: my new feature'
+feat: add support for AWS SDK v3 (#23)
 ```
 
-Once you merge your PR the you should use squash-merge, and the commit message should contain all these [commit messages in the footer](https://github.com/googleapis/release-please#what-if-my-pr-contains-multiple-fixes-or-features), so release-please can track all of the change
-
 Then once the release-please draft PR is merged into the main branch, the version is updated from x.1.x to x.2.x and the change log with be updated with the commit message. The project will also be tagged with the updated version
 
 For more information on that this works see [release-please](https://github.com/googleapis/release-please)
@@ -49,48 +47,4 @@ ReferenceError: Integration test could not run because USERNAME is undefined or
 
 You don't need to run integration tests locally, they'll be triggered in Github when you push your changes to your branch
 
-However, there may be times when you need to fix an integration bug, or make changes to the integration test. In this case you want will want to run integration tests locally. To do this you can ask an code owner for a `.env` file so you can run the tests, or you can setup a Cognito user pool on your own AWS account (or at least have access to one). To create a new user pool follow these steps...
-
-### _0. Setup an AWS account if you don't have one already_
-
-You will need to setup a Cognito user pool, which requires an AWS account. If you don't have an AWS account you can find a guide on how to set one up [here](https://docs.aws.amazon.com/accounts/latest/reference/manage-acct-creating.html). You will also need to run some AWS CLI commands later on, so [setup AWS CLI](https://docs.aws.amazon.com/cli/latest/userguide/cli-chap-configure.html) if you haven't already
-
-### _1. Setup your own user pool_
-
-You need a Cognito user pool to store user credentials in. To do that follow [this](https://docs.aws.amazon.com/cognito/latest/developerguide/tutorial-create-user-pool.html) guide
-
-### _2. Create an app in said user pool_
-
-Inside this new user pool you need to create an app. When creating the new app make sure you add a client secret and have the ALLOW_USER_SRP_AUTH authentication flow enabled. Follow [this](https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-client-apps.html) guide
-
-### _3. Create a new user_
-
-Next you need to [create a new user](https://docs.aws.amazon.com/cognito/latest/developerguide/how-to-create-user-accounts.html). There are no special requirements for the user, as long as you have a username and password that should be enough, but for your own convenience you shouldn't send the confirmation email. You will notice the user has a confirmation status of 'Force change password'. To get around this you need to run this CLI command to permanently set the user's password:
-
-```sh
-aws cognito-idp admin-set-user-password \
-  --user-pool-id <your-user-pool-id> \
-  --username <username> \
-  --password <password> \
-  --permanent
-```
-
-### _4. Create a `.env` file with the relevant credentials_
-
-Finally in your local repo, create a `.env` file in the root of the project. This file should contain the following:
-
-```sh
-# Credentials used in integration test
-
-AWS_ACCESS_KEY_ID=<aws_access_key_id>
-AWS_SECRET_ACCESS_KEY=<aws_secret_access_key>
-
-INT_TEST_USERNAME=<username_of_the_new_user>
-INT_TEST_PASSWORD=<password_of_the_new_user>
-INT_TEST_POOL_ID=<pool_id_of_the_new_userpool>
-INT_TEST_CLIENT_ID=<client_id_of_the_new_app>
-INT_TEST_SECRET_ID=<secret_id_of_the_new_app>
-INT_TEST_AWS_REGION=<aws_region_of_userpool>
-```
-
-After all these steps have been completed, you should be able to run integration tests locally. If you have any problems following these steps feel free to open an issue
+However, there may be times when you need to fix an integration bug, or make changes to the integration test. In this case you want will want to run integration tests locally. To do this you can ask an code owner for a `.env` file so you can run the tests
diff --git a/README.md b/README.md
@@ -23,31 +23,21 @@ const CognitoSrpHelper = require("cognito-srp-helper");
 Here is an example of how you would use the helper to implement SRP authentication with Cognito using the AWS JavaScript SDK v3:
 
 ```ts
-import {
-  createSecretHash,
-  createPasswordHash,
-  createSrpSession,
-  signSrpSession,
-  wrapAuthChallenge,
-  wrapInitiateAuth,
-} from "cognito-srp-helper";
-
 // . . . obtain user credentials, IDs, and setup Cognito client
 
 const secretHash = createSecretHash(username, clientId, secretId);
-const passwordHash = createPasswordHash(username, password, poolId);
-const srpSession = createSrpSession(username, passwordHash, poolId);
+const srpSession = createSrpSession(username, password, poolId, false);
 
 const initiateAuthRes = await cognitoIdentityProviderClient
   .send(
     new InitiateAuthCommand(
       wrapInitiateAuth(srpSession, {
-        ClientId: CLIENT_ID,
+        ClientId: clientId,
         AuthFlow: "USER_SRP_AUTH",
         AuthParameters: {
           CHALLENGE_NAME: "SRP_A",
           SECRET_HASH: secretHash,
-          USERNAME,
+          USERNAME: username,
         },
       }),
     ),
@@ -62,11 +52,11 @@ const respondToAuthChallengeRes = await cognitoIdentityProviderClient
   .send(
     new RespondToAuthChallengeCommand(
       wrapAuthChallenge(signedSrpSession, {
-        ClientId: CLIENT_ID,
+        ClientId: clientId,
         ChallengeName: "PASSWORD_VERIFIER",
         ChallengeResponses: {
           SECRET_HASH: secretHash,
-          USERNAME,
+          USERNAME: username,
         },
       }),
     ),
@@ -78,18 +68,9 @@ const respondToAuthChallengeRes = await cognitoIdentityProviderClient
 // . . . return login tokens from respondToAuthChallengeResponse
 ```
 
-Here is an example of how you would use the helper to implement SRP authentication with Cognito using the AWS JavaScript SDK v2 (deprecated):
+Here is an example of how you would use the helper to implement SRP authentication with Cognito using the AWS JavaScript SDK v2 (deprecated) using a pre-hashed password:
 
 ```ts
-import {
-  createSecretHash,
-  createPasswordHash,
-  createSrpSession,
-  signSrpSession,
-  wrapAuthChallenge,
-  wrapInitiateAuth,
-} from "cognito-srp-helper";
-
 // . . . obtain user credentials, IDs, and setup Cognito client
 
 const secretHash = createSecretHash(username, clientId, secretId);
@@ -104,7 +85,7 @@ const initiateAuthRes = await cognitoIdentityServiceProvider
       AuthParameters: {
         CHALLENGE_NAME: "SRP_A",
         SECRET_HASH: secretHash,
-        USERNAME,
+        USERNAME: username,
       },
     }),
   )
@@ -122,7 +103,7 @@ const respondToAuthChallengeRes = await cognitoIdentityServiceProvider
       ChallengeName: "PASSWORD_VERIFIER",
       ChallengeResponses: {
         SECRET_HASH: secretHash,
-        USERNAME,
+        USERNAME: username,
       },
     }),
   )
@@ -134,19 +115,6 @@ const respondToAuthChallengeRes = await cognitoIdentityServiceProvider
 // . . . return login tokens from respondToAuthChallengeResponse
 ```
 
-## Zero values in SRP
-
-Should you worry about 0 being used during the SRP calculations?
-
-Short answer: no!
-
-Long answer: according to the [safeguards of SRP](https://en.wikipedia.org/wiki/Secure_Remote_Password_protocol#Protocol), if a 0 value is given for A, B, or u then the protocol must abort to avoid compromising the security of the exchange. The possible scenarios in which a 0 value is used are:
-
-1. A value of 0 is randomly generated via SHA256 which is _extremely_ unlikely to occur, ~1/10^77
-2. A SRP_B value of 0 is received from the Cogntio initiateAuth call, which won't happen unless someone is purposefully trying to compromise security by intercepting the response from Cognito
-
-If any of these scenarios occur this package will throw a `AbortOnZeroSrpError`, so you don't need to worry about the security of the exchange being compromised
-
 ## API
 
 The types _InitiateAuthRequest_, _InitiateAuthResponse_, _RespondToAuthChallengeRequest_ refer to both the SDK v2 and v3 versions of these types, and their admin variants. For example _InitiateAuthRequest_ can be _AdminInitiateAuthRequest_, _InitiateAuthCommandInput_, etc.
@@ -173,7 +141,7 @@ _string_ - A hash of the secret. This is passed to the SECRET_HASH field
 
 Generates the required password hash from the user's credentials and user pool ID
 
-_TIP: If you are authenticating from the backend, you can call this function from the frontend and pass the hash value to the backend. While the user's password is secure being transmitted over HTTPS, this step can add an extra layer of security_
+> NOTE: pre-hashing the password only works when you're sign-in attribute is Username. If you're using Email or Phone Number you need to use an unhashed password
 
 **Parameters**:
 
@@ -193,12 +161,16 @@ _string_ - A hash of the user's password. Used to create an SRP session
 
 Creates an SRP session using the user's credentials and a Cognito user pool ID. This session contains the public/private SRP key for the client, and a timestamp in the unique format required by Cognito. With this session we can add to our public key (SRP_A) to the initiateAuth request
 
+> NOTE: pre-hashing the password only works when you're sign-in attribute is Username. If you're using Email or Phone Number you should set `isHashed` as `false`
+
 `username` - _string_ - The user's username
 
-`passwordHash` - _string_ - A hash of the user's password
+`password` - _string_ - The user's password
 
 `poolId` - _string_ - The ID of the user pool the user's credentials are stored in
 
+`isHashed` - _boolean_ - A flag indicating whether the password has already been hashed. The default value is `true`
+
 **Returns**:
 
 _SrpSession_ - Client SRP session object containing user credentials and session keys
@@ -251,6 +223,25 @@ Wraps a _RespondToAuthChallengeRequest_ and attaches the PASSWORD_CLAIM_SECRET_B
 
 _RespondToAuthChallengeRequest_ - The same request but with the additional PASSWORD_CLAIM_SECRET_BLOCK, PASSWORD_CLAIM_SIGNATURE, and TIMESTAMP fields
 
+## Password hashing
+
+It's possible to hash the user's password before you create the SRP session. This might be useful if you're calling InitiateAuth from the backend. This step can add an extra layer of security by obfuscating the user's password. To be clear though, the user's password is perfectly secure being transmitted using a secure protocol like HTTPS, this step is entirely optional
+
+Be aware that password hashing will only work if the user's sign-in attribute is Username. If you're using Email or Phone Number the hashing function `createPasswordHash` will not generate a valid hash
+
+## Zero values in SRP
+
+Should you worry about 0 being used during the SRP calculations?
+
+Short answer: no!
+
+Long answer: according to the [safeguards of SRP](https://en.wikipedia.org/wiki/Secure_Remote_Password_protocol#Protocol), if a 0 value is given for A, B, or u then the protocol must abort to avoid compromising the security of the exchange. The possible scenarios in which a 0 value is used are:
+
+1. A value of 0 is randomly generated via SHA256 which is _extremely_ unlikely to occur, ~1/10^77
+2. A SRP_B value of 0 is received from the Cogntio initiateAuth call, which won't happen unless someone is purposefully trying to compromise security by intercepting the response from Cognito
+
+If any of these scenarios occur this package will throw a `AbortOnZeroSrpError`, so you don't need to worry about the security of the exchange being compromised
+
 ## See Also
 
 - [amazon-cognito-identity-js](https://www.npmjs.com/package/amazon-cognito-identity-js) - NPM package for the Amplify cognito implementation