From 40183959dea383659f29d67194ce7433e0de5375 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 26 Oct 2023 15:03:11 -0700 Subject: [PATCH 01/24] Switch to node:crypto randomBytes generator for secret generator --- packages/cli/src/commands/generate/secret/secret.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/commands/generate/secret/secret.js b/packages/cli/src/commands/generate/secret/secret.js index d8cf9514e229..ffd6e04a5773 100644 --- a/packages/cli/src/commands/generate/secret/secret.js +++ b/packages/cli/src/commands/generate/secret/secret.js @@ -1,4 +1,5 @@ -import password from 'secure-random-password' +import crypto from 'node:crypto' + import terminalLink from 'terminal-link' import { recordTelemetryAttributes } from '@redwoodjs/cli-helpers' @@ -6,10 +7,7 @@ import { recordTelemetryAttributes } from '@redwoodjs/cli-helpers' const DEFAULT_LENGTH = 64 export const generateSecret = (length = DEFAULT_LENGTH) => { - return password.randomPassword({ - length, - characters: [password.lower, password.upper, password.digits], - }) + return crypto.randomBytes(length).toString('base64') } export const command = 'secret' From 55988e6c244bcc0c35428f6725f0dae6c0c8b262 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 26 Oct 2023 15:50:46 -0700 Subject: [PATCH 02/24] Update dbAuth setup to generate key from node:crypto --- packages/auth-providers/dbAuth/setup/src/setupData.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/auth-providers/dbAuth/setup/src/setupData.ts b/packages/auth-providers/dbAuth/setup/src/setupData.ts index 284ee1548927..eb2623b6acff 100644 --- a/packages/auth-providers/dbAuth/setup/src/setupData.ts +++ b/packages/auth-providers/dbAuth/setup/src/setupData.ts @@ -1,7 +1,6 @@ +import crypto from 'node:crypto' import path from 'path' -import password from 'secure-random-password' - import { getPaths, colors, addEnvVarTask } from '@redwoodjs/cli-helpers' export const libPath = getPaths().api.lib.replace(getPaths().base, '') @@ -10,10 +9,7 @@ export const functionsPath = getPaths().api.functions.replace( '' ) -const secret = password.randomPassword({ - length: 64, - characters: [password.lower, password.upper, password.digits], -}) +const secret = crypto.randomBytes(64).toString('base64') export const extraTask = addEnvVarTask( 'SESSION_SECRET', From 478655cf1538f259030dfe80d8aa9614affe1678 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 26 Oct 2023 15:52:08 -0700 Subject: [PATCH 03/24] Remove secure-random-password as a dependency --- .../auth-providers/dbAuth/setup/package.json | 2 -- packages/cli/package.json | 2 -- yarn.lock | 27 ------------------- 3 files changed, 31 deletions(-) diff --git a/packages/auth-providers/dbAuth/setup/package.json b/packages/auth-providers/dbAuth/setup/package.json index 980069bb5957..d2bb2f1a7480 100644 --- a/packages/auth-providers/dbAuth/setup/package.json +++ b/packages/auth-providers/dbAuth/setup/package.json @@ -27,14 +27,12 @@ "@simplewebauthn/browser": "7.2.0", "core-js": "3.32.2", "prompts": "2.4.2", - "secure-random-password": "0.2.3", "terminal-link": "2.1.1" }, "devDependencies": { "@babel/cli": "7.23.0", "@babel/core": "^7.22.20", "@simplewebauthn/typescript-types": "7.0.0", - "@types/secure-random-password": "0.2.1", "@types/yargs": "17.0.24", "jest": "29.7.0", "typescript": "5.2.2" diff --git a/packages/cli/package.json b/packages/cli/package.json index ee0f9ae82bf9..a7523c3cd0e3 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -45,7 +45,6 @@ "@redwoodjs/project-config": "6.0.7", "@redwoodjs/structure": "6.0.7", "@redwoodjs/telemetry": "6.0.7", - "@types/secure-random-password": "0.2.1", "boxen": "5.1.2", "camelcase": "6.3.0", "chalk": "4.1.2", @@ -74,7 +73,6 @@ "prisma": "5.4.2", "prompts": "2.4.2", "rimraf": "5.0.1", - "secure-random-password": "0.2.3", "semver": "7.5.3", "string-env-interpolation": "1.0.1", "systeminformation": "5.21.7", diff --git a/yarn.lock b/yarn.lock index d6e31dd041b2..a6fd4a16b651 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8076,12 +8076,10 @@ __metadata: "@redwoodjs/cli-helpers": 6.0.7 "@simplewebauthn/browser": 7.2.0 "@simplewebauthn/typescript-types": 7.0.0 - "@types/secure-random-password": 0.2.1 "@types/yargs": 17.0.24 core-js: 3.32.2 jest: 29.7.0 prompts: 2.4.2 - secure-random-password: 0.2.3 terminal-link: 2.1.1 typescript: 5.2.2 languageName: unknown @@ -8462,7 +8460,6 @@ __metadata: "@redwoodjs/structure": 6.0.7 "@redwoodjs/telemetry": 6.0.7 "@types/crypto-js": 4.1.1 - "@types/secure-random-password": 0.2.1 boxen: 5.1.2 camelcase: 6.3.0 chalk: 4.1.2 @@ -8492,7 +8489,6 @@ __metadata: prisma: 5.4.2 prompts: 2.4.2 rimraf: 5.0.1 - secure-random-password: 0.2.3 semver: 7.5.3 string-env-interpolation: 1.0.1 systeminformation: 5.21.7 @@ -12169,13 +12165,6 @@ __metadata: languageName: node linkType: hard -"@types/secure-random-password@npm:0.2.1": - version: 0.2.1 - resolution: "@types/secure-random-password@npm:0.2.1" - checksum: 87f0528b7ccb907706b0cc77160c6771279508de3852213f2c4f28a83af482b016cf3e0b414f62d2e8b3713f1733ad787e3bd40c201463ac822c117856a7886a - languageName: node - linkType: hard - "@types/semver@npm:^7.3.12, @types/semver@npm:^7.3.4": version: 7.5.0 resolution: "@types/semver@npm:7.5.0" @@ -31692,22 +31681,6 @@ __metadata: languageName: node linkType: hard -"secure-random-password@npm:0.2.3": - version: 0.2.3 - resolution: "secure-random-password@npm:0.2.3" - dependencies: - secure-random: ^1.1.2 - checksum: ced04529b96724b921b82b7527e1ba2f1299c3f327f941076e9edaea66445118b67a6b7f6edfe3b705a240e595de49144b2b796116c994cb52eb52efdd3d51b5 - languageName: node - linkType: hard - -"secure-random@npm:^1.1.2": - version: 1.1.2 - resolution: "secure-random@npm:1.1.2" - checksum: 612934cd5b1ea217d5e248a16ff2752411474997ede1f460ff37fe3214eedfd66ef6a5936ff76b3a5df3d057a8d2d4ed48298f5500bf837beb911522caac7f5c - languageName: node - linkType: hard - "selderee@npm:^0.10.0": version: 0.10.0 resolution: "selderee@npm:0.10.0" From fd00b281fdea89f9d2be72c6d6962b8c3ea53631 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 26 Oct 2023 16:25:32 -0700 Subject: [PATCH 04/24] Update hashPassword to use new algorithm, fallback to old algorithm and update in database when logging in a user --- .../dbAuth/api/src/DbAuthHandler.ts | 35 +++- .../api/src/__tests__/DbAuthHandler.test.js | 37 +++- .../dbAuth/api/src/__tests__/shared.test.ts | 176 ++++++++++-------- .../auth-providers/dbAuth/api/src/shared.ts | 17 +- 4 files changed, 179 insertions(+), 86 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts index c48ecc8586d1..6bdf4346085c 100644 --- a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts +++ b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts @@ -27,6 +27,7 @@ import { extractCookie, getSession, hashPassword, + legacyHashPassword, hashToken, webAuthnSession, } from './shared' @@ -637,11 +638,13 @@ export class DbAuthHandler< let user = await this._findUserByToken(resetToken as string) const [hashedPassword] = hashPassword(password, user.salt) + const [legacyHashedPassword] = legacyHashPassword(password, user.salt) if ( - !(this.options.resetPassword as ResetPasswordFlowOptions) + (!(this.options.resetPassword as ResetPasswordFlowOptions) .allowReusedPassword && - user.hashedPassword === hashedPassword + user.hashedPassword === hashedPassword) || + user.hashedPassword === legacyHashedPassword ) { throw new DbAuthError.ReusedPasswordError( ( @@ -1279,16 +1282,36 @@ export class DbAuthHandler< ) } - // is password correct? - const [hashedPassword, _salt] = hashPassword( + await this._verifyPassword(user, password) + return user + } + + async _verifyPassword(user: Record, password: string) { + const [hashedPassword] = hashPassword( password, - user[this.options.authFields.salt] + user[this.options.authFields.salt] as string ) + if (hashedPassword === user[this.options.authFields.hashedPassword]) { return user + } + + // fallback to old CryptoJS hashing + const [legacyHashedPassword] = legacyHashPassword( + password, + user[this.options.authFields.salt] as string + ) + + if (legacyHashedPassword === user[this.options.authFields.hashedPassword]) { + // update user's hash to the new algorithm + await this.dbAccessor.update({ + where: { id: user.id }, + data: { [this.options.authFields.hashedPassword]: hashedPassword }, + }) + return user } else { throw new DbAuthError.IncorrectPasswordError( - username, + user[this.options.authFields.username] as string, (this.options.login as LoginFlowOptions)?.errors?.incorrectPassword ) } diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js index 5abdf3aa731c..538c2ca3e883 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js +++ b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js @@ -102,9 +102,10 @@ const createDbUser = async (attributes = {}) => { return await db.user.create({ data: { email: 'rob@redwoodjs.com', + // default hashedPassword is from `node:crypto` hashedPassword: - '0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba', - salt: '2ef27f4073c603ba8b7807c6de6d6a89', + '230847bea5154b6c7d281d09593ad1be26fa03a93c04a73bcc2b608c073a8213', + salt: 'ba8b7807c6de6d6a892ef27f4073c603', ...attributes, }, }) @@ -2335,6 +2336,38 @@ describe('dbAuth', () => { expect(user.id).toEqual(dbUser.id) }) + + it('returns the user if password is hashed with legacy algorithm', async () => { + const dbUser = await createDbUser({ + // CryptoJS hashed password + hashedPassword: + '0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba', + salt: '2ef27f4073c603ba8b7807c6de6d6a89', + }) + const dbAuth = new DbAuthHandler(event, context, options) + const user = await dbAuth._verifyUser(dbUser.email, 'password') + + expect(user.id).toEqual(dbUser.id) + }) + + it('updates the user hashPassword to the new algorithm', async () => { + const dbUser = await createDbUser({ + // CryptoJS hashed password + hashedPassword: + '0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba', + salt: '2ef27f4073c603ba8b7807c6de6d6a89', + }) + const dbAuth = new DbAuthHandler(event, context, options) + await dbAuth._verifyUser(dbUser.email, 'password') + const user = await db.user.findFirst({ where: { id: dbUser.id } }) + + // password now hashed by node:crypto + expect(user.hashedPassword).toEqual( + 'f20d69d478fa1afc85057384e21bd457a76b23b23e2a94f5bd982976f700a552' + ) + // salt should remain the same + expect(user.salt).toEqual('2ef27f4073c603ba8b7807c6de6d6a89') + }) }) describe('_getCurrentUser()', () => { diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts index c96c7bc85892..e08c11542910 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts +++ b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts @@ -9,6 +9,7 @@ import { getSession, cookieName, hashPassword, + legacyHashPassword, decryptSession, dbAuthSession, webAuthnSession, @@ -137,6 +138,29 @@ describe('webAuthnSession', () => { describe('hashPassword', () => { it('hashes a password with a given salt and returns both', () => { const [hash, salt] = hashPassword( + 'password', + 'ba8b7807c6de6d6a892ef27f4073c603' + ) + + expect(hash).toEqual( + '230847bea5154b6c7d281d09593ad1be26fa03a93c04a73bcc2b608c073a8213' + ) + expect(salt).toEqual('ba8b7807c6de6d6a892ef27f4073c603') + }) + + it('hashes a password with a generated salt if none provided', () => { + const [hash, salt] = hashPassword('password') + + expect(hash).toMatch(/^[a-f0-9]+$/) + expect(hash.length).toEqual(64) + expect(salt).toMatch(/^[a-f0-9]+$/) + expect(salt.length).toEqual(64) + }) +}) + +describe('legacyHashPassword', () => { + it('hashes a password with CryptoJS given a salt and returns both', () => { + const [hash, salt] = legacyHashPassword( 'password', '2ef27f4073c603ba8b7807c6de6d6a89' ) @@ -148,41 +172,62 @@ describe('hashPassword', () => { }) it('hashes a password with a generated salt if none provided', () => { - const [hash, salt] = hashPassword('password') + const [hash, salt] = legacyHashPassword('password') expect(hash).toMatch(/^[a-f0-9]+$/) expect(hash.length).toEqual(64) expect(salt).toMatch(/^[a-f0-9]+$/) - expect(salt.length).toEqual(32) + expect(salt.length).toEqual(64) }) +}) - describe('session cookie extraction', () => { - let event +describe('session cookie extraction', () => { + let event - const encryptToCookie = (data) => { - return `session=${CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET)}` + const encryptToCookie = (data) => { + return `session=${CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET)}` + } + + beforeEach(() => { + event = { + queryStringParameters: {}, + path: '/.redwood/functions/auth', + headers: {}, } + }) - beforeEach(() => { - event = { - queryStringParameters: {}, - path: '/.redwood/functions/auth', - headers: {}, - } - }) + it('extracts from the event', () => { + const cookie = encryptToCookie( + JSON.stringify({ id: 9999999999 }) + ';' + 'token' + ) - it('extracts from the event', () => { - const cookie = encryptToCookie( - JSON.stringify({ id: 9999999999 }) + ';' + 'token' - ) + event = { + headers: { + cookie, + }, + } - event = { - headers: { - cookie, - }, - } + expect(extractCookie(event)).toEqual(cookie) + }) - expect(extractCookie(event)).toEqual(cookie) + it('extract cookie handles non-JSON event body', () => { + event.body = '' + + expect(extractCookie(event)).toBeUndefined() + }) + + describe('when in development', () => { + const curNodeEnv = process.env.NODE_ENV + + beforeAll(() => { + // Session cookie from graphiQLHeaders only extracted in dev + process.env.NODE_ENV = 'development' + }) + + afterAll(() => { + process.env.NODE_ENV = curNodeEnv + event = {} + expect(process.env.NODE_ENV).toBe('test') }) it('extract cookie handles non-JSON event body', () => { @@ -191,69 +236,48 @@ describe('hashPassword', () => { expect(extractCookie(event)).toBeUndefined() }) - describe('when in development', () => { - const curNodeEnv = process.env.NODE_ENV + it('extracts GraphiQL cookie from the header extensions', () => { + const dbUserId = 42 - beforeAll(() => { - // Session cookie from graphiQLHeaders only extracted in dev - process.env.NODE_ENV = 'development' - }) - - afterAll(() => { - process.env.NODE_ENV = curNodeEnv - event = {} - expect(process.env.NODE_ENV).toBe('test') + const cookie = encryptToCookie(JSON.stringify({ id: dbUserId })) + event.body = JSON.stringify({ + extensions: { + headers: { + 'auth-provider': 'dbAuth', + cookie, + authorization: 'Bearer ' + dbUserId, + }, + }, }) - it('extract cookie handles non-JSON event body', () => { - event.body = '' - - expect(extractCookie(event)).toBeUndefined() - }) + expect(extractCookie(event)).toEqual(cookie) + }) - it('extracts GraphiQL cookie from the header extensions', () => { - const dbUserId = 42 - - const cookie = encryptToCookie(JSON.stringify({ id: dbUserId })) - event.body = JSON.stringify({ - extensions: { - headers: { - 'auth-provider': 'dbAuth', - cookie, - authorization: 'Bearer ' + dbUserId, - }, - }, - }) + it('overwrites cookie with event header GraphiQL when in dev', () => { + const sessionCookie = encryptToCookie( + JSON.stringify({ id: 9999999999 }) + ';' + 'token' + ) - expect(extractCookie(event)).toEqual(cookie) - }) + event = { + headers: { + cookie: sessionCookie, + }, + } - it('overwrites cookie with event header GraphiQL when in dev', () => { - const sessionCookie = encryptToCookie( - JSON.stringify({ id: 9999999999 }) + ';' + 'token' - ) + const dbUserId = 42 - event = { + const cookie = encryptToCookie(JSON.stringify({ id: dbUserId })) + event.body = JSON.stringify({ + extensions: { headers: { - cookie: sessionCookie, + 'auth-provider': 'dbAuth', + cookie, + authorization: 'Bearer ' + dbUserId, }, - } - - const dbUserId = 42 - - const cookie = encryptToCookie(JSON.stringify({ id: dbUserId })) - event.body = JSON.stringify({ - extensions: { - headers: { - 'auth-provider': 'dbAuth', - cookie, - authorization: 'Bearer ' + dbUserId, - }, - }, - }) - - expect(extractCookie(event)).toEqual(cookie) + }, }) + + expect(extractCookie(event)).toEqual(cookie) }) }) }) diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index 552833411a36..f2c3fd981efe 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -1,3 +1,5 @@ +import crypto from 'node:crypto' + import type { APIGatewayProxyEvent } from 'aws-lambda' import CryptoJS from 'crypto-js' @@ -116,10 +118,21 @@ export const hashToken = (token: string) => { // hashes a password using either the given `salt` argument, or creates a new // salt and hashes using that. Either way, returns an array with [hash, salt] export const hashPassword = (text: string, salt?: string) => { - const useSalt = salt || CryptoJS.lib.WordArray.random(128 / 8).toString() + const useSalt = salt || crypto.randomBytes(32).toString('hex') + return [ + crypto + .scryptSync(text, useSalt, 32, { cost: 2 ** 14, blockSize: 8 }) + .toString('hex'), + useSalt, + ] +} +// uses the old algorithm from CryptoJS: +// CryptoJS.PBKDF2(password, salt, { keySize: 8 }).toString() +export const legacyHashPassword = (text: string, salt?: string) => { + const useSalt = salt || crypto.randomBytes(32).toString('hex') return [ - CryptoJS.PBKDF2(text, useSalt, { keySize: 256 / 32 }).toString(), + crypto.pbkdf2Sync(text, useSalt, 1, 32, 'SHA1').toString('hex'), useSalt, ] } From 3bd1f5473a4005f9bad7a50bd64bfa85ad6e8a21 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 26 Oct 2023 18:10:49 -0700 Subject: [PATCH 05/24] Converts cookie session management to use node:crypto algorithm --- .../dbAuth/api/src/DbAuthHandler.ts | 13 ++--- .../api/src/__tests__/DbAuthHandler.test.js | 38 ++++++++----- .../auth-providers/dbAuth/api/src/shared.ts | 54 +++++++++++++++++-- 3 files changed, 78 insertions(+), 27 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts index 6bdf4346085c..c6c15087a496 100644 --- a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts +++ b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts @@ -1,3 +1,5 @@ +import crypto from 'node:crypto' + import type { PrismaClient } from '@prisma/client' import type { GenerateAuthenticationOptionsOpts, @@ -13,7 +15,6 @@ import type { } from '@simplewebauthn/typescript-types' import type { APIGatewayProxyEvent, Context as LambdaContext } from 'aws-lambda' import base64url from 'base64url' -import CryptoJS from 'crypto-js' import md5 from 'md5' import { v4 as uuidv4 } from 'uuid' @@ -24,6 +25,7 @@ import * as DbAuthError from './errors' import { cookieName, decryptSession, + encryptSession, extractCookie, getSession, hashPassword, @@ -1157,11 +1159,6 @@ export class DbAuthHandler< return meta } - // encrypts a string with the SESSION_SECRET - _encrypt(data: string) { - return CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET as string) - } - // returns the set-cookie header to be returned in the request (effectively // creates the session) _createSessionHeader( @@ -1169,9 +1166,9 @@ export class DbAuthHandler< csrfToken: string ): SetCookieHeader { const session = JSON.stringify(data) + ';' + csrfToken - const encrypted = this._encrypt(session) + const encrypted = encryptSession(session) const cookie = [ - `${cookieName(this.options.cookie?.name)}=${encrypted.toString()}`, + `${cookieName(this.options.cookie?.name)}=${encrypted}`, ...this._cookieAttributes({ expires: this.sessionExpiresDate }), ].join(';') diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js index 538c2ca3e883..a47232ada744 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js +++ b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js @@ -1,6 +1,6 @@ import path from 'node:path' -import CryptoJS from 'crypto-js' +import crypto from 'node:crypto' import { DbAuthHandler } from '../DbAuthHandler' import * as dbAuthError from '../errors' @@ -81,10 +81,10 @@ const db = new DbMock(['user', 'userCredential']) const UUID_REGEX = /\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b/ -const SET_SESSION_REGEX = /^session=[a-zA-Z0-9+=/]+;/ +const SET_SESSION_REGEX = /^session=[a-zA-Z0-9+=/|]+;/ const UTC_DATE_REGEX = /\w{3}, \d{2} \w{3} \d{4} [\d:]{8} GMT/ const LOGOUT_COOKIE = 'session=;Expires=Thu, 01 Jan 1970 00:00:00 GMT' - +const SESSION_SECRET = '540d03ebb00b441f8f7442cbc39958ad' const FIXTURE_PATH = path.resolve( __dirname, '../../../../../../__fixtures__/example-todo-main' @@ -120,8 +120,17 @@ const expectLoggedInResponse = (response) => { } const encryptToCookie = (data) => { - return `session=${CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET)}` -} + const iv = crypto.randomBytes(16) + const cipher = crypto.createCipheriv( + 'aes-256-cbc', + (process.env.SESSION_SECRET as string).substring(0, 32), + iv + ) + let encryptedSession = cipher.update(data, 'utf-8', 'base64') + encryptedSession += cipher.final('base64') + + return `session=${encryptedSession}|${iv.toString('base64')}` + } let event, context, options @@ -130,7 +139,8 @@ describe('dbAuth', () => { // hide deprecation warnings during test jest.spyOn(console, 'warn').mockImplementation(() => {}) // encryption key so results are consistent regardless of settings in .env - process.env.SESSION_SECRET = 'nREjs1HPS7cFia6tQHK70EWGtfhOgbqJQKsHQz3S' + // CryptoJS secret: process.env.SESSION_SECRET = 'nREjs1HPS7cFia6tQHK70EWGtfhOgbqJQKsHQz3S' + process.env.SESSION_SECRET = SESSION_SECRET delete process.env.DBAUTH_COOKIE_DOMAIN event = { @@ -582,7 +592,7 @@ describe('dbAuth', () => { event = { headers: { cookie: - 'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx', + 'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w==', }, } const dbAuth = new DbAuthHandler(event, context, options) @@ -615,7 +625,7 @@ describe('dbAuth', () => { event.body = JSON.stringify({ method: 'logout' }) event.httpMethod = 'GET' event.headers.cookie = - 'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx' + 'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w==' const dbAuth = new DbAuthHandler(event, context, options) const response = await dbAuth.invoke() @@ -626,7 +636,7 @@ describe('dbAuth', () => { event.body = JSON.stringify({ method: 'foobar' }) event.httpMethod = 'POST' event.headers.cookie = - 'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx' + 'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w==' const dbAuth = new DbAuthHandler(event, context, options) const response = await dbAuth.invoke() @@ -637,7 +647,7 @@ describe('dbAuth', () => { event.body = JSON.stringify({ method: 'logout' }) event.httpMethod = 'POST' event.headers.cookie = - 'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx' + 'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w==' const dbAuth = new DbAuthHandler(event, context, options) dbAuth.logout = jest.fn(() => { throw Error('Logout error') @@ -675,7 +685,7 @@ describe('dbAuth', () => { event.body = JSON.stringify({ method: 'logout' }) event.httpMethod = 'POST' event.headers.cookie = - 'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx' + 'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w==' const dbAuth = new DbAuthHandler(event, context, options) dbAuth.logout = jest.fn(() => ['body', { foo: 'bar' }]) const response = await dbAuth.invoke() @@ -2169,11 +2179,11 @@ describe('dbAuth', () => { `Expires=${dbAuth.sessionExpiresDate}` ) // can't really match on the session value since it will change on every render, - // due to CSRF token generation but we can check that it contains a only the - // characters that would be returned by the hash function + // due to CSRF token generation but we can check that it contains only the + // characters that would be returned by the encrypt function expect(headers['set-cookie']).toMatch(SET_SESSION_REGEX) // and we can check that it's a certain number of characters - expect(headers['set-cookie'].split(';')[0].length).toEqual(72) + expect(headers['set-cookie'].split(';')[0].length).toEqual(77) }) }) diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index f2c3fd981efe..d421901fc244 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -43,11 +43,42 @@ export const decryptSession = (text: string | null) => { return [] } + // if cookie contains a pipe then it was encrypted using the `node:crypto` + // algorithm (first element is the ecrypted data, second is the initialization vector) + // otherwise fall back to using the CryptoJS algorithm + + let decoded + const [encryptedText, iv] = text.split('|') + + // console.info(text) + // console.info(encryptedText, iv) + try { - const decoded = CryptoJS.AES.decrypt( - text, - process.env.SESSION_SECRET as string - ).toString(CryptoJS.enc.Utf8) + if (iv) { + // decrypt using the `node:crypto` algorithm + // console.info('\n\n') + // console.info('Using node:crypto algorithm to decrypt session cookie') + // console.info('\n\n') + const decipher = crypto.createDecipheriv( + 'aes-256-cbc', + process.env.SESSION_SECRET as string, + Buffer.from(iv, 'base64') + ) + decoded = decipher.update(encryptedText, 'base64', 'utf-8') + decoded += decipher.final('utf-8') + } else { + // console.info('\n\n') + // console.info('Using CryptoJS algorithm to decrypt session cookie') + // console.info('\n\n') + // decrypt using the CryptoJS algorithm + decoded = CryptoJS.AES.decrypt( + encryptedText, + process.env.SESSION_SECRET as string + ).toString(CryptoJS.enc.Utf8) + } + + // console.info('decoded', decoded) + const [data, csrf] = decoded.split(';') const json = JSON.parse(data) @@ -57,6 +88,19 @@ export const decryptSession = (text: string | null) => { } } +export const encryptSession = (data: string) => { + const iv = crypto.randomBytes(16) + const cipher = crypto.createCipheriv( + 'aes-256-cbc', + (process.env.SESSION_SECRET as string).substring(0, 32), + iv + ) + let encryptedSession = cipher.update(data, 'utf-8', 'base64') + encryptedSession += cipher.final('base64') + + return `${encryptedSession}|${iv.toString('base64')}` +} + // returns the actual value of the session cookie export const getSession = ( text: string | undefined, @@ -75,7 +119,7 @@ export const getSession = ( return null } - return sessionCookie.split('=')[1].trim() + return sessionCookie.replace(`${cookieName(cookieNameOption)}=`, '') } // Convenience function to get session, decrypt, and return session data all From 973f690ef608e0e2681ff534f33cd4082dbcee00 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 26 Oct 2023 20:14:17 -0700 Subject: [PATCH 06/24] Create 32 byte long secrets --- packages/auth-providers/dbAuth/setup/src/setupData.ts | 2 +- packages/cli/src/commands/generate/secret/secret.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/auth-providers/dbAuth/setup/src/setupData.ts b/packages/auth-providers/dbAuth/setup/src/setupData.ts index eb2623b6acff..82bdf2d88dab 100644 --- a/packages/auth-providers/dbAuth/setup/src/setupData.ts +++ b/packages/auth-providers/dbAuth/setup/src/setupData.ts @@ -9,7 +9,7 @@ export const functionsPath = getPaths().api.functions.replace( '' ) -const secret = crypto.randomBytes(64).toString('base64') +const secret = crypto.randomBytes(32).toString('base64') export const extraTask = addEnvVarTask( 'SESSION_SECRET', diff --git a/packages/cli/src/commands/generate/secret/secret.js b/packages/cli/src/commands/generate/secret/secret.js index ffd6e04a5773..3fcfa074793a 100644 --- a/packages/cli/src/commands/generate/secret/secret.js +++ b/packages/cli/src/commands/generate/secret/secret.js @@ -4,7 +4,7 @@ import terminalLink from 'terminal-link' import { recordTelemetryAttributes } from '@redwoodjs/cli-helpers' -const DEFAULT_LENGTH = 64 +const DEFAULT_LENGTH = 32 export const generateSecret = (length = DEFAULT_LENGTH) => { return crypto.randomBytes(length).toString('base64') From 853ca31fbc3d2dd1bf4e8bb680f500c658c8a1b9 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 27 Oct 2023 14:59:26 -0700 Subject: [PATCH 07/24] Ports CryptoJS session decryption to node:crypto --- .../api/src/__tests__/DbAuthHandler.test.js | 1 - .../dbAuth/api/src/__tests__/shared.test.ts | 35 +++++++++-- .../auth-providers/dbAuth/api/src/shared.ts | 60 ++++++++++--------- 3 files changed, 64 insertions(+), 32 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js index a47232ada744..24c1d2e5e6d9 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js +++ b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js @@ -139,7 +139,6 @@ describe('dbAuth', () => { // hide deprecation warnings during test jest.spyOn(console, 'warn').mockImplementation(() => {}) // encryption key so results are consistent regardless of settings in .env - // CryptoJS secret: process.env.SESSION_SECRET = 'nREjs1HPS7cFia6tQHK70EWGtfhOgbqJQKsHQz3S' process.env.SESSION_SECRET = SESSION_SECRET delete process.env.DBAUTH_COOKIE_DOMAIN diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts index e08c11542910..869f4de991b4 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts +++ b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts @@ -1,7 +1,7 @@ +import crypto from 'node:crypto' import path from 'node:path' import type { APIGatewayProxyEvent } from 'aws-lambda' -import CryptoJS from 'crypto-js' import * as error from '../errors' import { @@ -19,10 +19,19 @@ const FIXTURE_PATH = path.resolve( __dirname, '../../../../../../__fixtures__/example-todo-main' ) -process.env.SESSION_SECRET = 'nREjs1HPS7cFia6tQHK70EWGtfhOgbqJQKsHQz3S' +const SESSION_SECRET = '540d03ebb00b441f8f7442cbc39958ad' const encrypt = (data) => { - return CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET).toString() + const iv = crypto.randomBytes(16) + const cipher = crypto.createCipheriv( + 'aes-256-cbc', + SESSION_SECRET.substring(0, 32), + iv + ) + let encryptedSession = cipher.update(data, 'utf-8', 'base64') + encryptedSession += cipher.final('base64') + + return `${encryptedSession}|${iv.toString('base64')}` } function dummyEvent(cookie?: string) { @@ -82,6 +91,10 @@ describe('cookieName()', () => { }) describe('decryptSession()', () => { + beforeEach(() => { + process.env.SESSION_SECRET = SESSION_SECRET + }) + it('returns an empty array if no session', () => { expect(decryptSession(null)).toEqual([]) }) @@ -104,9 +117,23 @@ describe('decryptSession()', () => { expect(decryptSession(text)).toEqual([first, second]) }) + + it.only('decrypts a session cookie that was created with the legacy CryptoJS algorithm', () => { + process.env.SESSION_SECRET = + 'QKxN2vFSHAf94XYynK8LUALfDuDSdFowG6evfkFX8uszh4YZqhTiqEdshrhWbwbw' + const [json] = decryptSession( + 'U2FsdGVkX1+s7seQJnVgGgInxuXm13l8VvzA3Mg2fYg=' + ) + + expect(json).toEqual({ id: 7 }) + }) }) describe('dbAuthSession()', () => { + beforeEach(() => { + process.env.SESSION_SECRET = SESSION_SECRET + }) + it('returns null if no cookies', () => { expect(dbAuthSession(dummyEvent(), 'session_%port%')).toEqual(null) }) @@ -185,7 +212,7 @@ describe('session cookie extraction', () => { let event const encryptToCookie = (data) => { - return `session=${CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET)}` + return `session=${encrypt(data)}` } beforeEach(() => { diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index d421901fc244..01a20e9d7ba6 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -1,7 +1,6 @@ import crypto from 'node:crypto' import type { APIGatewayProxyEvent } from 'aws-lambda' -import CryptoJS from 'crypto-js' import { getConfig } from '@redwoodjs/project-config' @@ -31,6 +30,28 @@ const eventGraphiQLHeadersCookie = (event: APIGatewayProxyEvent) => { return } +// decrypts session text using old CryptoJS algorithm (using node:crypto library) +const legacyDecryptSession = (encryptedText) => { + const cypher = Buffer.from(encryptedText, 'base64') + const salt = cypher.slice(8, 16) + const password = Buffer.concat([ + Buffer.from(process.env.SESSION_SECRET as string, 'binary'), + salt, + ]) + const md5Hashes = [] + let digest = password + for (let i = 0; i < 3; i++) { + md5Hashes[i] = crypto.createHash('md5').update(digest).digest() + digest = Buffer.concat([md5Hashes[i], password]) + } + const key = Buffer.concat([md5Hashes[0], md5Hashes[1]]) + const iv = md5Hashes[2] + const contents = cypher.slice(16) + const decipher = crypto.createDecipheriv('aes-256-cbc', key, iv) + + return decipher.update(contents) + decipher.final('utf-8') +} + // Extracts the session cookie from an event, handling both // development environment GraphiQL headers and production environment headers. export const extractCookie = (event: APIGatewayProxyEvent) => { @@ -43,42 +64,27 @@ export const decryptSession = (text: string | null) => { return [] } + let decoded // if cookie contains a pipe then it was encrypted using the `node:crypto` // algorithm (first element is the ecrypted data, second is the initialization vector) - // otherwise fall back to using the CryptoJS algorithm - - let decoded + // otherwise fall back to using the older CryptoJS algorithm const [encryptedText, iv] = text.split('|') - // console.info(text) - // console.info(encryptedText, iv) - try { if (iv) { // decrypt using the `node:crypto` algorithm - // console.info('\n\n') - // console.info('Using node:crypto algorithm to decrypt session cookie') - // console.info('\n\n') const decipher = crypto.createDecipheriv( 'aes-256-cbc', process.env.SESSION_SECRET as string, Buffer.from(iv, 'base64') ) - decoded = decipher.update(encryptedText, 'base64', 'utf-8') - decoded += decipher.final('utf-8') + decoded = + decipher.update(encryptedText, 'base64', 'utf-8') + + decipher.final('utf-8') } else { - // console.info('\n\n') - // console.info('Using CryptoJS algorithm to decrypt session cookie') - // console.info('\n\n') - // decrypt using the CryptoJS algorithm - decoded = CryptoJS.AES.decrypt( - encryptedText, - process.env.SESSION_SECRET as string - ).toString(CryptoJS.enc.Utf8) + decoded = legacyDecryptSession(text) } - // console.info('decoded', decoded) - const [data, csrf] = decoded.split(';') const json = JSON.parse(data) @@ -95,10 +101,10 @@ export const encryptSession = (data: string) => { (process.env.SESSION_SECRET as string).substring(0, 32), iv ) - let encryptedSession = cipher.update(data, 'utf-8', 'base64') - encryptedSession += cipher.final('base64') + let encryptedData = cipher.update(data, 'utf-8', 'base64') + encryptedData += cipher.final('base64') - return `${encryptedSession}|${iv.toString('base64')}` + return `${encryptedData}|${iv.toString('base64')}` } // returns the actual value of the session cookie @@ -119,7 +125,7 @@ export const getSession = ( return null } - return sessionCookie.replace(`${cookieName(cookieNameOption)}=`, '') + return sessionCookie.replace(`${cookieName(cookieNameOption)}=`, '').trim() } // Convenience function to get session, decrypt, and return session data all @@ -156,7 +162,7 @@ export const webAuthnSession = (event: APIGatewayProxyEvent) => { } export const hashToken = (token: string) => { - return CryptoJS.SHA256(token).toString(CryptoJS.enc.Hex) + return crypto.createHash('sha256').update(token).digest('hex') } // hashes a password using either the given `salt` argument, or creates a new From 373e82e9a166ea19b9769b48315d513a6a64012a Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 27 Oct 2023 15:25:27 -0700 Subject: [PATCH 08/24] Removes CryptoJS import --- packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts index c6c15087a496..fca64de228ef 100644 --- a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts +++ b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts @@ -1,5 +1,3 @@ -import crypto from 'node:crypto' - import type { PrismaClient } from '@prisma/client' import type { GenerateAuthenticationOptionsOpts, From bfd61ad24374a7d91cb876484791281be1badfc5 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 27 Oct 2023 15:25:31 -0700 Subject: [PATCH 09/24] Adds type --- packages/auth-providers/dbAuth/api/src/shared.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index 01a20e9d7ba6..efebb8d01835 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -31,7 +31,7 @@ const eventGraphiQLHeadersCookie = (event: APIGatewayProxyEvent) => { } // decrypts session text using old CryptoJS algorithm (using node:crypto library) -const legacyDecryptSession = (encryptedText) => { +const legacyDecryptSession = (encryptedText: string) => { const cypher = Buffer.from(encryptedText, 'base64') const salt = cypher.slice(8, 16) const password = Buffer.concat([ From 5dda72e9507cb4642b2153bb571215e03cec4779 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 27 Oct 2023 15:25:53 -0700 Subject: [PATCH 10/24] Update graphiql and studio to use encryptSession function --- .../graphiql/__tests__/graphiqlHandler.test.js | 14 +++++++++----- .../src/commands/setup/graphiql/graphiqlHandler.js | 4 ++-- .../commands/setup/graphiql/supportedProviders.js | 11 +++++------ .../api/lib/authProviderEncoders/dbAuthEncoder.ts | 8 +++----- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js b/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js index 4354c56aadd5..20a859ce65fd 100644 --- a/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js +++ b/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js @@ -53,18 +53,18 @@ describe('Graphiql generator tests', () => { expect(processExitSpy).toHaveBeenCalledWith(1) }) - it('throws an error if auth provider is dbAuth and no user id is provided', () => { + it('throws an error if auth provider is dbAuth and no user id is provided', async () => { try { - graphiqlHelpers.generatePayload('dbAuth') + await graphiqlHelpers.generatePayload('dbAuth') } catch (e) { expect(e.message).toBe('Require an unique id to generate session cookie') } }) - it('throws an error if auth provider is dbAuth and no supabase env is set', () => { + it('throws an error if auth provider is dbAuth and no supabase env is set', async () => { process.env.SESSION_SECRET = null try { - graphiqlHelpers.generatePayload('dbAuth', 'user-id-123') + await graphiqlHelpers.generatePayload('dbAuth', 'user-id-123') } catch (e) { expect(e.message).toBe( 'dbAuth requires a SESSION_SECRET environment variable that is used to encrypt session cookies. Use `yarn rw g secret` to create one, then add to your `.env` file. DO NOT check this variable in your version control system!!' @@ -75,7 +75,11 @@ describe('Graphiql generator tests', () => { it('returns a payload if a token is provided', async () => { const provider = 'supabase' const token = 'mock-token' - const response = graphiqlHelpers.generatePayload(provider, null, token) + const response = await graphiqlHelpers.generatePayload( + provider, + null, + token + ) expect(response).toEqual({ 'auth-provider': provider, authorization: `Bearer ${token}`, diff --git a/packages/cli/src/commands/setup/graphiql/graphiqlHandler.js b/packages/cli/src/commands/setup/graphiql/graphiqlHandler.js index 6b1aaeca11a1..c2f0c148f71c 100644 --- a/packages/cli/src/commands/setup/graphiql/graphiqlHandler.js +++ b/packages/cli/src/commands/setup/graphiql/graphiqlHandler.js @@ -68,8 +68,8 @@ export const handler = async ({ provider, id, token, expiry, view }) => { [ { title: 'Generating graphiql header...', - task: () => { - payload = generatePayload(provider, id, token, expiry) + task: async () => { + payload = await generatePayload(provider, id, token, expiry) }, }, { diff --git a/packages/cli/src/commands/setup/graphiql/supportedProviders.js b/packages/cli/src/commands/setup/graphiql/supportedProviders.js index b02c48f42498..fdb50cafa9b5 100644 --- a/packages/cli/src/commands/setup/graphiql/supportedProviders.js +++ b/packages/cli/src/commands/setup/graphiql/supportedProviders.js @@ -1,4 +1,3 @@ -import CryptoJS from 'crypto-js' import { v4 as uuidv4 } from 'uuid' // tests if id, which is always a string from cli, is actually a number or uuid @@ -10,7 +9,7 @@ const getExpiryTime = (expiry) => { return expiry ? Date.now() + expiry * 60 * 1000 : Date.now() + 3600 * 1000 } -const getDBAuthHeader = (userId) => { +const getDBAuthHeader = async (userId) => { if (!userId) { throw new Error('Require an unique id to generate session cookie') } @@ -20,11 +19,11 @@ const getDBAuthHeader = (userId) => { 'dbAuth requires a SESSION_SECRET environment variable that is used to encrypt session cookies. Use `yarn rw g secret` to create one, then add to your `.env` file. DO NOT check this variable in your version control system!!' ) } + + const { encryptSession } = await import('@redwoodjs/auth-dbauth-api') + const id = isNumeric(userId) ? parseInt(userId) : userId - const cookie = CryptoJS.AES.encrypt( - JSON.stringify({ id }) + ';' + uuidv4(), - process.env.SESSION_SECRET - ).toString() + const cookie = encryptSession(JSON.stringify({ id }) + ';' + uuidv4()) return { 'auth-provider': 'dbAuth', diff --git a/packages/studio/api/lib/authProviderEncoders/dbAuthEncoder.ts b/packages/studio/api/lib/authProviderEncoders/dbAuthEncoder.ts index bbd89198ed3e..771fefca29a1 100644 --- a/packages/studio/api/lib/authProviderEncoders/dbAuthEncoder.ts +++ b/packages/studio/api/lib/authProviderEncoders/dbAuthEncoder.ts @@ -1,4 +1,3 @@ -import CryptoJS from 'crypto-js' import { v4 as uuidv4 } from 'uuid' import { SESSION_SECRET } from '../envars' @@ -18,11 +17,10 @@ export const getDBAuthHeader = async (userId?: string) => { ) } + const { encryptSession } = await import('@redwoodjs/auth-dbauth-api') + const id = isNumeric(userId) ? parseInt(userId) : userId - const cookie = CryptoJS.AES.encrypt( - JSON.stringify({ id }) + ';' + uuidv4(), - SESSION_SECRET - ).toString() + const cookie = encryptSession(JSON.stringify({ id }) + ';' + uuidv4()) return { authProvider: 'dbAuth', From 0dd276239552682aceb5badd9f7d99e2d7d62d9d Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 27 Oct 2023 15:39:48 -0700 Subject: [PATCH 11/24] Update secret generation tests --- .../generate/secret/__tests__/secret.test.js | 21 +++++++++++++------ .../src/commands/generate/secret/secret.js | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/commands/generate/secret/__tests__/secret.test.js b/packages/cli/src/commands/generate/secret/__tests__/secret.test.js index be2680dbede0..417f52f9cadd 100644 --- a/packages/cli/src/commands/generate/secret/__tests__/secret.test.js +++ b/packages/cli/src/commands/generate/secret/__tests__/secret.test.js @@ -1,18 +1,27 @@ import yargs from 'yargs' -import { generateSecret, handler, builder } from './../secret.js' +import { + DEFAULT_LENGTH, + generateSecret, + handler, + builder, +} from './../secret.js' describe('generateSecret', () => { - it('contains only uppercase letters, lowercase letters, and digits', () => { + it('contains base64-encoded string', () => { const secret = generateSecret() + const buffer = Buffer.alloc(DEFAULT_LENGTH) + const stringLength = buffer.toString('base64').length - expect(secret).toMatch(/^[A-Za-z0-9]{64}$/) + expect(secret).toMatch(new RegExp(`^[A-Za-z0-9+/=]{${stringLength}}$`)) }) it('can optionally accept a length', () => { const secret = generateSecret(16) + const buffer = Buffer.alloc(16) - expect(secret.length).toEqual(16) + // however long a 16-byte buffer is when base64-encoded (24 characters) + expect(secret.length).toEqual(buffer.toString('base64').length) }) it('prints nothing but the secret when setting the --raw flag', () => { @@ -26,7 +35,7 @@ describe('generateSecret', () => { console.info = (...args) => (output += args.join(' ') + '\n') process.stdout.write = (str) => (output += str) - const { raw, length } = yargs + const { raw } = yargs .command('secret', false, builder, handler) .parse('secret --raw') @@ -35,6 +44,6 @@ describe('generateSecret', () => { process.stdout.write = realWrite expect(raw).toBeTruthy() - expect(output).toMatch(new RegExp(`^[A-Za-z0-9]{${length}}\n$`)) + expect(output).toMatch(new RegExp(`^[A-Za-z0-9+/=]+\n$`)) }) }) diff --git a/packages/cli/src/commands/generate/secret/secret.js b/packages/cli/src/commands/generate/secret/secret.js index 3fcfa074793a..a012efe5e227 100644 --- a/packages/cli/src/commands/generate/secret/secret.js +++ b/packages/cli/src/commands/generate/secret/secret.js @@ -4,7 +4,7 @@ import terminalLink from 'terminal-link' import { recordTelemetryAttributes } from '@redwoodjs/cli-helpers' -const DEFAULT_LENGTH = 32 +export const DEFAULT_LENGTH = 32 export const generateSecret = (length = DEFAULT_LENGTH) => { return crypto.randomBytes(length).toString('base64') From a4b03c4c750464b9a9fa219957bf6aabfd6f3a26 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 27 Oct 2023 15:40:05 -0700 Subject: [PATCH 12/24] Delete SESSION_SECRET completely instead of nullifying --- .../commands/setup/graphiql/__tests__/graphiqlHandler.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js b/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js index 20a859ce65fd..49f8250fa3d7 100644 --- a/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js +++ b/packages/cli/src/commands/setup/graphiql/__tests__/graphiqlHandler.test.js @@ -62,7 +62,7 @@ describe('Graphiql generator tests', () => { }) it('throws an error if auth provider is dbAuth and no supabase env is set', async () => { - process.env.SESSION_SECRET = null + delete process.env.SESSION_SECRET try { await graphiqlHelpers.generatePayload('dbAuth', 'user-id-123') } catch (e) { From e2a8a32fb2ed93677ec08ab2ac1e61877cc792fa Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 27 Oct 2023 16:01:13 -0700 Subject: [PATCH 13/24] Add function to determine if legacy session cookie --- .../dbAuth/api/src/__tests__/shared.test.ts | 19 ++++++++++++++++++- .../auth-providers/dbAuth/api/src/shared.ts | 6 ++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts index 869f4de991b4..592d34854bd0 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts +++ b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts @@ -9,6 +9,7 @@ import { getSession, cookieName, hashPassword, + isLegacySession, legacyHashPassword, decryptSession, dbAuthSession, @@ -90,6 +91,22 @@ describe('cookieName()', () => { }) }) +describe('isLegacySession()', () => { + it('returns `true` if the session cookie appears to be encrypted with CryptoJS', () => { + expect( + isLegacySession('U2FsdGVkX1+s7seQJnVgGgInxuXm13l8VvzA3Mg2fYg=') + ).toEqual(true) + }) + + it('returns `false` if the session cookie appears to be encrypted with node:crypto', () => { + expect( + isLegacySession( + 'ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w==' + ) + ).toEqual(false) + }) +}) + describe('decryptSession()', () => { beforeEach(() => { process.env.SESSION_SECRET = SESSION_SECRET @@ -118,7 +135,7 @@ describe('decryptSession()', () => { expect(decryptSession(text)).toEqual([first, second]) }) - it.only('decrypts a session cookie that was created with the legacy CryptoJS algorithm', () => { + it('decrypts a session cookie that was created with the legacy CryptoJS algorithm', () => { process.env.SESSION_SECRET = 'QKxN2vFSHAf94XYynK8LUALfDuDSdFowG6evfkFX8uszh4YZqhTiqEdshrhWbwbw' const [json] = decryptSession( diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index efebb8d01835..5064cfb7689b 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -58,6 +58,12 @@ export const extractCookie = (event: APIGatewayProxyEvent) => { return eventGraphiQLHeadersCookie(event) || eventHeadersCookie(event) } +// whether this encrypted session was made with the old CryptoJS algorithm +export const isLegacySession = (text) => { + const [_encryptedText, iv] = text.split('|') + return !iv +} + // decrypts the session cookie and returns an array: [data, csrf] export const decryptSession = (text: string | null) => { if (!text || text.trim() === '') { From 8415d57d12bb7d96073cd25c84cf12524c5b85dc Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Mon, 30 Oct 2023 11:21:09 -0700 Subject: [PATCH 14/24] Re-encrypt session cookie if getToken() is called with a legacy one --- .../dbAuth/api/src/DbAuthHandler.ts | 17 +++++---- .../api/src/__tests__/DbAuthHandler.test.js | 38 ++++++++++++++----- .../auth-providers/dbAuth/api/src/shared.ts | 4 +- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts index fca64de228ef..42281493f8e1 100644 --- a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts +++ b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts @@ -28,6 +28,7 @@ import { getSession, hashPassword, legacyHashPassword, + isLegacySession, hashToken, webAuthnSession, } from './shared' @@ -570,11 +571,15 @@ export class DbAuthHandler< async getToken() { try { const user = await this._getCurrentUser() + let headers = {} - // need to return *something* for our existing Authorization header stuff - // to work, so return the user's ID in case we can use it for something - // in the future - return [user[this.options.authFields.id]] + // if the session was encrypted with the old algorithm, re-encrypt it + // with the new one + if (isLegacySession(this.cookie)) { + headers = this._loginResponse(user)[1] + } + + return [user[this.options.authFields.id], headers] } catch (e: any) { if (e instanceof DbAuthError.NotLoggedInError) { return this._logoutResponse() @@ -1424,9 +1429,7 @@ export class DbAuthHandler< ] { const sessionData = { id: user[this.options.authFields.id] } - // TODO: this needs to go into graphql somewhere so that each request makes - // a new CSRF token and sets it in both the encrypted session and the - // csrf-token header + // TODO: this needs to go into graphql somewhere so that each request makes a new CSRF token and sets it in both the encrypted session and the csrf-token header const csrfToken = DbAuthHandler.CSRF_TOKEN return [ diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js index 24c1d2e5e6d9..6a5ca9e9ac4e 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js +++ b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js @@ -81,7 +81,7 @@ const db = new DbMock(['user', 'userCredential']) const UUID_REGEX = /\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b/ -const SET_SESSION_REGEX = /^session=[a-zA-Z0-9+=/|]+;/ +const SET_SESSION_REGEX = /^session=[a-zA-Z0-9+=/]|[a-zA-Z0-9+=/]+;/ const UTC_DATE_REGEX = /\w{3}, \d{2} \w{3} \d{4} [\d:]{8} GMT/ const LOGOUT_COOKIE = 'session=;Expires=Thu, 01 Jan 1970 00:00:00 GMT' const SESSION_SECRET = '540d03ebb00b441f8f7442cbc39958ad' @@ -123,7 +123,7 @@ const encryptToCookie = (data) => { const iv = crypto.randomBytes(16) const cipher = crypto.createCipheriv( 'aes-256-cbc', - (process.env.SESSION_SECRET as string).substring(0, 32), + (SESSION_SECRET as string).substring(0, 32), iv ) let encryptedSession = cipher.update(data, 'utf-8', 'base64') @@ -1568,8 +1568,8 @@ describe('dbAuth', () => { }) }) - describe('getToken', () => { - it('returns the ID of the logged in user', async () => { + describe('getToken',() => { + it('returns the ID of the logged in user',async () => { const user = await createDbUser() event = { headers: { @@ -1578,20 +1578,20 @@ describe('dbAuth', () => { ), }, } - const dbAuth = new DbAuthHandler(event, context, options) + const dbAuth = new DbAuthHandler(event,context,options) const response = await dbAuth.getToken() expect(response[0]).toEqual(user.id) }) - it('returns nothing if user is not logged in', async () => { - const dbAuth = new DbAuthHandler(event, context, options) + it('returns nothing if user is not logged in',async () => { + const dbAuth = new DbAuthHandler(event,context,options) const response = await dbAuth.getToken() expect(response[0]).toEqual('') }) - it('returns any other error', async () => { + it('returns any other error',async () => { event = { headers: { cookie: encryptToCookie( @@ -1600,11 +1600,31 @@ describe('dbAuth', () => { }, } - const dbAuth = new DbAuthHandler(event, context, options) + const dbAuth = new DbAuthHandler(event,context,options) const response = await dbAuth.getToken() expect(response[0]).toEqual('{"error":"User not found"}') }) + + it('re-encrypts the session cookie if using the legacy algorithm',async () => { + const user = await createDbUser({ id: 7 }) + event = { + headers: { + // legacy session with { id: 7 } for userID + cookie: 'session=U2FsdGVkX1+s7seQJnVgGgInxuXm13l8VvzA3Mg2fYg=' + }, + } + process.env.SESSION_SECRET = 'QKxN2vFSHAf94XYynK8LUALfDuDSdFowG6evfkFX8uszh4YZqhTiqEdshrhWbwbw' + + const dbAuth = new DbAuthHandler(event, context, options) + const [userId, headers] = await dbAuth.getToken() + + expect(userId).toEqual(7) + expect(headers['set-cookie']).toMatch(SET_SESSION_REGEX) + + // set session back to default + process.env.SESSION_SECRET = SESSION_SECRET + }) }) describe('When a developer has set GraphiQL headers to mock a session cookie', () => { diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index 5064cfb7689b..dc2317b27560 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -100,14 +100,14 @@ export const decryptSession = (text: string | null) => { } } -export const encryptSession = (data: string) => { +export const encryptSession = (dataString: string) => { const iv = crypto.randomBytes(16) const cipher = crypto.createCipheriv( 'aes-256-cbc', (process.env.SESSION_SECRET as string).substring(0, 32), iv ) - let encryptedData = cipher.update(data, 'utf-8', 'base64') + let encryptedData = cipher.update(dataString, 'utf-8', 'base64') encryptedData += cipher.final('base64') return `${encryptedData}|${iv.toString('base64')}` From 893f8f940881943ed728a160a2eef34b261beb53 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Mon, 30 Oct 2023 11:21:43 -0700 Subject: [PATCH 15/24] String.normalize() passwords in case of weird unicode characters --- .../dbAuth/api/src/__tests__/shared.test.ts | 11 +++++++++++ packages/auth-providers/dbAuth/api/src/shared.ts | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts index 592d34854bd0..b1079b274841 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts +++ b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts @@ -200,6 +200,17 @@ describe('hashPassword', () => { expect(salt).toMatch(/^[a-f0-9]+$/) expect(salt.length).toEqual(64) }) + + it('normalizes strings so utf-8 variants hash to the same output', () => { + const salt = crypto.randomBytes(32).toString('hex') + const [hash1] = hashPassword('\u0041\u006d\u00e9\u006c\u0069\u0065', salt) // Amélie + const [hash2] = hashPassword( + '\u0041\u006d\u0065\u0301\u006c\u0069\u0065', + salt + ) // Amélie but separate e and accent characters + + expect(hash1).toEqual(hash2) + }) }) describe('legacyHashPassword', () => { diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index dc2317b27560..fc2e20353dc3 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -173,11 +173,15 @@ export const hashToken = (token: string) => { // hashes a password using either the given `salt` argument, or creates a new // salt and hashes using that. Either way, returns an array with [hash, salt] +// normalizes the string in case it contains unicode characters: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/normalize export const hashPassword = (text: string, salt?: string) => { const useSalt = salt || crypto.randomBytes(32).toString('hex') return [ crypto - .scryptSync(text, useSalt, 32, { cost: 2 ** 14, blockSize: 8 }) + .scryptSync(text.normalize('NFC'), useSalt, 32, { + cost: 2 ** 14, + blockSize: 8, + }) .toString('hex'), useSalt, ] From 8152eab43285666b0cd97eb4d3434e7682ea9c15 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Mon, 30 Oct 2023 12:42:30 -0700 Subject: [PATCH 16/24] Encodes scrypt difficulty settings into hashed password --- .../dbAuth/api/src/DbAuthHandler.ts | 69 ++++++++++++------- .../api/src/__tests__/DbAuthHandler.test.js | 6 +- .../dbAuth/api/src/__tests__/shared.test.ts | 69 +++++++++++++++---- .../auth-providers/dbAuth/api/src/shared.ts | 56 ++++++++++++--- 4 files changed, 150 insertions(+), 50 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts index 42281493f8e1..7608844813e3 100644 --- a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts +++ b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts @@ -31,6 +31,7 @@ import { isLegacySession, hashToken, webAuthnSession, + extractHashingOptions, } from './shared' type SetCookieHeader = { 'set-cookie': string } @@ -642,7 +643,9 @@ export class DbAuthHandler< } let user = await this._findUserByToken(resetToken as string) - const [hashedPassword] = hashPassword(password, user.salt) + const [hashedPassword] = hashPassword(password, { + salt: user.salt, + }) const [legacyHashedPassword] = legacyHashPassword(password, user.salt) if ( @@ -1286,35 +1289,52 @@ export class DbAuthHandler< return user } + // extracts scrypt strength options from hashed password (if present) and + // compares the hashed plain text password just submitted using those options + // with the one in the database. Falls back to the legacy CryptoJS algorihtm + // if no options are present. async _verifyPassword(user: Record, password: string) { - const [hashedPassword] = hashPassword( - password, - user[this.options.authFields.salt] as string - ) - - if (hashedPassword === user[this.options.authFields.hashedPassword]) { - return user - } - - // fallback to old CryptoJS hashing - const [legacyHashedPassword] = legacyHashPassword( - password, - user[this.options.authFields.salt] as string + const options = extractHashingOptions( + user[this.options.authFields.hashedPassword] as string ) - if (legacyHashedPassword === user[this.options.authFields.hashedPassword]) { - // update user's hash to the new algorithm - await this.dbAccessor.update({ - where: { id: user.id }, - data: { [this.options.authFields.hashedPassword]: hashedPassword }, + if (Object.keys(options).length) { + // hashed using the node:crypto algorithm + const [hashedPassword] = hashPassword(password, { + salt: user[this.options.authFields.salt] as string, + options, }) - return user + + if (hashedPassword === user[this.options.authFields.hashedPassword]) { + return user + } } else { - throw new DbAuthError.IncorrectPasswordError( - user[this.options.authFields.username] as string, - (this.options.login as LoginFlowOptions)?.errors?.incorrectPassword + // fallback to old CryptoJS hashing + const [legacyHashedPassword] = legacyHashPassword( + password, + user[this.options.authFields.salt] as string ) + + if ( + legacyHashedPassword === user[this.options.authFields.hashedPassword] + ) { + const [newHashedPassword] = hashPassword(password, { + salt: user[this.options.authFields.salt] as string, + }) + + // update user's hash to the new algorithm + await this.dbAccessor.update({ + where: { id: user.id }, + data: { [this.options.authFields.hashedPassword]: newHashedPassword }, + }) + return user + } } + + throw new DbAuthError.IncorrectPasswordError( + user[this.options.authFields.username] as string, + (this.options.login as LoginFlowOptions)?.errors?.incorrectPassword + ) } // gets the user from the database and returns only its ID @@ -1374,8 +1394,7 @@ export class DbAuthHandler< ) } - // if we get here everything is good, call the app's signup handler and let - // them worry about scrubbing data and saving to the DB + // if we get here everything is good, call the app's signup handler const [hashedPassword, salt] = hashPassword(password) const newUser = await (this.options.signup as SignupFlowOptions).handler({ username, diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js index 6a5ca9e9ac4e..c43fbb7f6d11 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js +++ b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js @@ -104,7 +104,7 @@ const createDbUser = async (attributes = {}) => { email: 'rob@redwoodjs.com', // default hashedPassword is from `node:crypto` hashedPassword: - '230847bea5154b6c7d281d09593ad1be26fa03a93c04a73bcc2b608c073a8213', + '230847bea5154b6c7d281d09593ad1be26fa03a93c04a73bcc2b608c073a8213|16384|8|1', salt: 'ba8b7807c6de6d6a892ef27f4073c603', ...attributes, }, @@ -969,7 +969,7 @@ describe('dbAuth', () => { expect(user).toEqual(user) return user } - const dbAuth = new DbAuthHandler(event, context, options) + const dbAuth = new DbAuthHandler(event,context,options) await dbAuth.login() }) @@ -2392,7 +2392,7 @@ describe('dbAuth', () => { // password now hashed by node:crypto expect(user.hashedPassword).toEqual( - 'f20d69d478fa1afc85057384e21bd457a76b23b23e2a94f5bd982976f700a552' + 'f20d69d478fa1afc85057384e21bd457a76b23b23e2a94f5bd982976f700a552|16384|8|1' ) // salt should remain the same expect(user.salt).toEqual('2ef27f4073c603ba8b7807c6de6d6a89') diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts index b1079b274841..67b1becccc60 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts +++ b/packages/auth-providers/dbAuth/api/src/__tests__/shared.test.ts @@ -14,6 +14,7 @@ import { decryptSession, dbAuthSession, webAuthnSession, + extractHashingOptions, } from '../shared' const FIXTURE_PATH = path.resolve( @@ -181,13 +182,12 @@ describe('webAuthnSession', () => { describe('hashPassword', () => { it('hashes a password with a given salt and returns both', () => { - const [hash, salt] = hashPassword( - 'password', - 'ba8b7807c6de6d6a892ef27f4073c603' - ) + const [hash, salt] = hashPassword('password', { + salt: 'ba8b7807c6de6d6a892ef27f4073c603', + }) expect(hash).toEqual( - '230847bea5154b6c7d281d09593ad1be26fa03a93c04a73bcc2b608c073a8213' + '230847bea5154b6c7d281d09593ad1be26fa03a93c04a73bcc2b608c073a8213|16384|8|1' ) expect(salt).toEqual('ba8b7807c6de6d6a892ef27f4073c603') }) @@ -195,22 +195,34 @@ describe('hashPassword', () => { it('hashes a password with a generated salt if none provided', () => { const [hash, salt] = hashPassword('password') - expect(hash).toMatch(/^[a-f0-9]+$/) - expect(hash.length).toEqual(64) + expect(hash).toMatch(/^[a-f0-9]+|16384|8|1$/) + expect(hash.length).toEqual(74) expect(salt).toMatch(/^[a-f0-9]+$/) expect(salt.length).toEqual(64) }) it('normalizes strings so utf-8 variants hash to the same output', () => { const salt = crypto.randomBytes(32).toString('hex') - const [hash1] = hashPassword('\u0041\u006d\u00e9\u006c\u0069\u0065', salt) // Amélie - const [hash2] = hashPassword( - '\u0041\u006d\u0065\u0301\u006c\u0069\u0065', - salt - ) // Amélie but separate e and accent characters + const [hash1] = hashPassword('\u0041\u006d\u00e9\u006c\u0069\u0065', { + salt, + }) // Amélie + const [hash2] = hashPassword('\u0041\u006d\u0065\u0301\u006c\u0069\u0065', { + salt, + }) // Amélie but separate e and accent codepoints expect(hash1).toEqual(hash2) }) + + it('encodes the scrypt difficulty options into the hash', () => { + const [hash] = hashPassword('password', { + options: { cost: 8192, blockSize: 16, parallelization: 2 }, + }) + const [_hash, cost, blockSize, parallelization] = hash.split('|') + + expect(cost).toEqual('8192') + expect(blockSize).toEqual('16') + expect(parallelization).toEqual('2') + }) }) describe('legacyHashPassword', () => { @@ -336,3 +348,36 @@ describe('session cookie extraction', () => { }) }) }) + +describe('extractHashingOptions()', () => { + it('returns an empty object if no options', () => { + expect(extractHashingOptions('')).toEqual({}) + expect( + extractHashingOptions( + '0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba' + ) + ).toEqual({}) + expect( + extractHashingOptions( + '0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba|1' + ) + ).toEqual({}) + expect( + extractHashingOptions( + '0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba|1|2' + ) + ).toEqual({}) + }) + + it('returns an object with scrypt options', () => { + expect( + extractHashingOptions( + '0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba|16384|8|1' + ) + ).toEqual({ + cost: 16384, + blockSize: 8, + parallelization: 1, + }) + }) +}) diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index fc2e20353dc3..f6155b396579 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -6,6 +6,22 @@ import { getConfig } from '@redwoodjs/project-config' import * as DbAuthError from './errors' +type ScryptOptions = { + cost?: number + blockSize?: number + parallelization?: number + N?: number + r?: number + p?: number + maxmem?: number +} + +const DEFAULT_SCRYPT_OPTIONS: ScryptOptions = { + cost: 2 ** 14, + blockSize: 8, + parallelization: 1, +} + // Extracts the cookie from an event, handling lower and upper case header names. const eventHeadersCookie = (event: APIGatewayProxyEvent) => { return event.headers.cookie || event.headers.Cookie @@ -174,17 +190,23 @@ export const hashToken = (token: string) => { // hashes a password using either the given `salt` argument, or creates a new // salt and hashes using that. Either way, returns an array with [hash, salt] // normalizes the string in case it contains unicode characters: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/normalize -export const hashPassword = (text: string, salt?: string) => { - const useSalt = salt || crypto.randomBytes(32).toString('hex') - return [ - crypto - .scryptSync(text.normalize('NFC'), useSalt, 32, { - cost: 2 ** 14, - blockSize: 8, - }) - .toString('hex'), - useSalt, +// TODO: Add validation that the options are valid values for the scrypt algorithm +export const hashPassword = ( + text: string, + { + salt = crypto.randomBytes(32).toString('hex'), + options = DEFAULT_SCRYPT_OPTIONS, + }: { salt?: string; options?: ScryptOptions } = {} +) => { + const encryptedString = crypto + .scryptSync(text.normalize('NFC'), salt, 32, options) + .toString('hex') + const optionsToString = [ + options.cost, + options.blockSize, + options.parallelization, ] + return [`${encryptedString}|${optionsToString.join('|')}`, salt] } // uses the old algorithm from CryptoJS: @@ -203,3 +225,17 @@ export const cookieName = (name: string | undefined) => { return cookieName } + +export const extractHashingOptions = (text: string): ScryptOptions => { + const [_hash, ...options] = text.split('|') + + if (options.length === 3) { + return { + cost: parseInt(options[0]), + blockSize: parseInt(options[1]), + parallelization: parseInt(options[2]), + } + } else { + return {} + } +} From 164079f391ae46aa4306fe6234b7947180471e46 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 3 Nov 2023 14:04:15 -0700 Subject: [PATCH 17/24] Added type --- packages/auth-providers/dbAuth/api/src/shared.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index f6155b396579..e6f0854219dd 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -75,7 +75,7 @@ export const extractCookie = (event: APIGatewayProxyEvent) => { } // whether this encrypted session was made with the old CryptoJS algorithm -export const isLegacySession = (text) => { +export const isLegacySession = (text: string) => { const [_encryptedText, iv] = text.split('|') return !iv } From dde912605b87776dbfc6f71b1f586c6168b8d6ed Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 3 Nov 2023 14:06:53 -0700 Subject: [PATCH 18/24] text could be undefined --- packages/auth-providers/dbAuth/api/src/shared.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index e6f0854219dd..ace55dd03955 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -75,7 +75,11 @@ export const extractCookie = (event: APIGatewayProxyEvent) => { } // whether this encrypted session was made with the old CryptoJS algorithm -export const isLegacySession = (text: string) => { +export const isLegacySession = (text: string | undefined) => { + if (!text) { + return false + } + const [_encryptedText, iv] = text.split('|') return !iv } From 622d7922b4f3c6ae5fd0c8da8f270f8beb8a66f1 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 3 Nov 2023 14:31:54 -0700 Subject: [PATCH 19/24] Need only 32 characters for session secret --- packages/auth-providers/dbAuth/api/src/shared.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth-providers/dbAuth/api/src/shared.ts b/packages/auth-providers/dbAuth/api/src/shared.ts index ace55dd03955..0d3e4b278db4 100644 --- a/packages/auth-providers/dbAuth/api/src/shared.ts +++ b/packages/auth-providers/dbAuth/api/src/shared.ts @@ -101,7 +101,7 @@ export const decryptSession = (text: string | null) => { // decrypt using the `node:crypto` algorithm const decipher = crypto.createDecipheriv( 'aes-256-cbc', - process.env.SESSION_SECRET as string, + (process.env.SESSION_SECRET as string).substring(0, 32), Buffer.from(iv, 'base64') ) decoded = From 537c5da9d7b154b5b39e20e90486ea749c0760f0 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 3 Nov 2023 14:32:13 -0700 Subject: [PATCH 20/24] resetTokenExpiresAt not being properly set as DateTime, need to convert to string first? --- packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts index 7608844813e3..4b77d294ab44 100644 --- a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts +++ b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts @@ -530,7 +530,8 @@ export class DbAuthHandler< }, data: { [this.options.authFields.resetToken]: tokenHash, - [this.options.authFields.resetTokenExpiresAt]: tokenExpires, + [this.options.authFields.resetTokenExpiresAt]: + tokenExpires.toISOString(), }, }) } catch (e) { From 7aed7bf3a7caa64cd597c1a485891130347f09de Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Fri, 3 Nov 2023 14:34:20 -0700 Subject: [PATCH 21/24] Revert "resetTokenExpiresAt not being properly set as DateTime, need to convert to string first?" This reverts commit 537c5da9d7b154b5b39e20e90486ea749c0760f0. --- packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts index 4b77d294ab44..7608844813e3 100644 --- a/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts +++ b/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts @@ -530,8 +530,7 @@ export class DbAuthHandler< }, data: { [this.options.authFields.resetToken]: tokenHash, - [this.options.authFields.resetTokenExpiresAt]: - tokenExpires.toISOString(), + [this.options.authFields.resetTokenExpiresAt]: tokenExpires, }, }) } catch (e) { From 0e8e37d5071a0313dc79166f34ae5d329b13a799 Mon Sep 17 00:00:00 2001 From: Dominic Saadi Date: Fri, 3 Nov 2023 15:11:55 -0700 Subject: [PATCH 22/24] chore: lint fix; remove TS from JS file to fix parsing --- .../api/src/__tests__/DbAuthHandler.test.js | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js index c43fbb7f6d11..4d19f4847055 100644 --- a/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js +++ b/packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js @@ -1,6 +1,5 @@ -import path from 'node:path' - import crypto from 'node:crypto' +import path from 'node:path' import { DbAuthHandler } from '../DbAuthHandler' import * as dbAuthError from '../errors' @@ -121,16 +120,16 @@ const expectLoggedInResponse = (response) => { const encryptToCookie = (data) => { const iv = crypto.randomBytes(16) - const cipher = crypto.createCipheriv( - 'aes-256-cbc', - (SESSION_SECRET as string).substring(0, 32), - iv - ) - let encryptedSession = cipher.update(data, 'utf-8', 'base64') - encryptedSession += cipher.final('base64') + const cipher = crypto.createCipheriv( + 'aes-256-cbc', + SESSION_SECRET.substring(0, 32), + iv + ) + let encryptedSession = cipher.update(data, 'utf-8', 'base64') + encryptedSession += cipher.final('base64') return `session=${encryptedSession}|${iv.toString('base64')}` - } +} let event, context, options @@ -969,7 +968,7 @@ describe('dbAuth', () => { expect(user).toEqual(user) return user } - const dbAuth = new DbAuthHandler(event,context,options) + const dbAuth = new DbAuthHandler(event, context, options) await dbAuth.login() }) @@ -1568,8 +1567,8 @@ describe('dbAuth', () => { }) }) - describe('getToken',() => { - it('returns the ID of the logged in user',async () => { + describe('getToken', () => { + it('returns the ID of the logged in user', async () => { const user = await createDbUser() event = { headers: { @@ -1578,20 +1577,20 @@ describe('dbAuth', () => { ), }, } - const dbAuth = new DbAuthHandler(event,context,options) + const dbAuth = new DbAuthHandler(event, context, options) const response = await dbAuth.getToken() expect(response[0]).toEqual(user.id) }) - it('returns nothing if user is not logged in',async () => { - const dbAuth = new DbAuthHandler(event,context,options) + it('returns nothing if user is not logged in', async () => { + const dbAuth = new DbAuthHandler(event, context, options) const response = await dbAuth.getToken() expect(response[0]).toEqual('') }) - it('returns any other error',async () => { + it('returns any other error', async () => { event = { headers: { cookie: encryptToCookie( @@ -1600,21 +1599,22 @@ describe('dbAuth', () => { }, } - const dbAuth = new DbAuthHandler(event,context,options) + const dbAuth = new DbAuthHandler(event, context, options) const response = await dbAuth.getToken() expect(response[0]).toEqual('{"error":"User not found"}') }) - it('re-encrypts the session cookie if using the legacy algorithm',async () => { - const user = await createDbUser({ id: 7 }) + it('re-encrypts the session cookie if using the legacy algorithm', async () => { + await createDbUser({ id: 7 }) event = { headers: { // legacy session with { id: 7 } for userID - cookie: 'session=U2FsdGVkX1+s7seQJnVgGgInxuXm13l8VvzA3Mg2fYg=' + cookie: 'session=U2FsdGVkX1+s7seQJnVgGgInxuXm13l8VvzA3Mg2fYg=', }, } - process.env.SESSION_SECRET = 'QKxN2vFSHAf94XYynK8LUALfDuDSdFowG6evfkFX8uszh4YZqhTiqEdshrhWbwbw' + process.env.SESSION_SECRET = + 'QKxN2vFSHAf94XYynK8LUALfDuDSdFowG6evfkFX8uszh4YZqhTiqEdshrhWbwbw' const dbAuth = new DbAuthHandler(event, context, options) const [userId, headers] = await dbAuth.getToken() From 7226ae9f4d20df81f13f1e16134eb57369c89c92 Mon Sep 17 00:00:00 2001 From: Dominic Saadi Date: Fri, 3 Nov 2023 15:59:06 -0700 Subject: [PATCH 23/24] remove crypto-js from deps --- .../auth-providers/dbAuth/api/package.json | 2 -- packages/cli/package.json | 2 -- packages/studio/package.json | 2 -- yarn.lock | 20 ------------------- 4 files changed, 26 deletions(-) diff --git a/packages/auth-providers/dbAuth/api/package.json b/packages/auth-providers/dbAuth/api/package.json index 0e0033b5a22d..2d4ec6095086 100644 --- a/packages/auth-providers/dbAuth/api/package.json +++ b/packages/auth-providers/dbAuth/api/package.json @@ -26,7 +26,6 @@ "@redwoodjs/project-config": "6.0.7", "base64url": "3.0.1", "core-js": "3.32.2", - "crypto-js": "4.1.1", "md5": "2.3.0", "uuid": "9.0.0" }, @@ -35,7 +34,6 @@ "@babel/core": "^7.22.20", "@redwoodjs/api": "6.0.7", "@simplewebauthn/server": "7.3.1", - "@types/crypto-js": "4.1.1", "@types/md5": "2.3.2", "@types/uuid": "9.0.2", "jest": "29.7.0", diff --git a/packages/cli/package.json b/packages/cli/package.json index d149aada5bfc..046a41ed0082 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -53,7 +53,6 @@ "configstore": "3.1.5", "core-js": "3.32.2", "cross-env": "7.0.3", - "crypto-js": "4.1.1", "decamelize": "5.0.1", "dotenv-defaults": "5.0.2", "enquirer": "2.4.1", @@ -84,7 +83,6 @@ "devDependencies": { "@babel/cli": "7.23.0", "@babel/core": "^7.22.20", - "@types/crypto-js": "4.1.1", "jest": "29.7.0", "typescript": "5.2.2" }, diff --git a/packages/studio/package.json b/packages/studio/package.json index a76ac633deef..676cc9f9798b 100644 --- a/packages/studio/package.json +++ b/packages/studio/package.json @@ -32,7 +32,6 @@ "ansi-colors": "4.1.3", "chokidar": "3.5.3", "core-js": "3.32.2", - "crypto-js": "4.1.1", "dotenv": "16.3.1", "fast-json-parse": "1.0.3", "fastify": "4.23.2", @@ -68,7 +67,6 @@ "@tailwindcss/forms": "0.5.3", "@tremor/react": "3.4.1", "@types/aws-lambda": "8.10.119", - "@types/crypto-js": "4.1.1", "@types/jsonwebtoken": "9.0.2", "@types/lodash": "4.14.195", "@types/mailparser": "^3", diff --git a/yarn.lock b/yarn.lock index cc8e116059bc..fca460bb418c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8072,12 +8072,10 @@ __metadata: "@redwoodjs/api": 6.0.7 "@redwoodjs/project-config": 6.0.7 "@simplewebauthn/server": 7.3.1 - "@types/crypto-js": 4.1.1 "@types/md5": 2.3.2 "@types/uuid": 9.0.2 base64url: 3.0.1 core-js: 3.32.2 - crypto-js: 4.1.1 jest: 29.7.0 md5: 2.3.0 typescript: 5.2.2 @@ -8478,7 +8476,6 @@ __metadata: "@redwoodjs/project-config": 6.0.7 "@redwoodjs/structure": 6.0.7 "@redwoodjs/telemetry": 6.0.7 - "@types/crypto-js": 4.1.1 boxen: 5.1.2 camelcase: 6.3.0 chalk: 4.1.2 @@ -8487,7 +8484,6 @@ __metadata: configstore: 3.1.5 core-js: 3.32.2 cross-env: 7.0.3 - crypto-js: 4.1.1 decamelize: 5.0.1 dotenv-defaults: 5.0.2 enquirer: 2.4.1 @@ -9078,7 +9074,6 @@ __metadata: "@tailwindcss/forms": 0.5.3 "@tremor/react": 3.4.1 "@types/aws-lambda": 8.10.119 - "@types/crypto-js": 4.1.1 "@types/jsonwebtoken": 9.0.2 "@types/lodash": 4.14.195 "@types/mailparser": ^3 @@ -9097,7 +9092,6 @@ __metadata: buffer: 6.0.3 chokidar: 3.5.3 core-js: 3.32.2 - crypto-js: 4.1.1 dotenv: 16.3.1 fast-json-parse: 1.0.3 fastify: 4.23.2 @@ -11326,13 +11320,6 @@ __metadata: languageName: node linkType: hard -"@types/crypto-js@npm:4.1.1": - version: 4.1.1 - resolution: "@types/crypto-js@npm:4.1.1" - checksum: e53b712c5d3b72d19c67a06c8bbaaafd989d78f71a2168f1376c8fb84d5744e5166b58d79528a124645e13f13fe4d2c97ee8f03d649ef913e93ca6b8cee41370 - languageName: node - linkType: hard - "@types/d3-array@npm:^3.0.3": version: 3.0.5 resolution: "@types/d3-array@npm:3.0.5" @@ -16824,13 +16811,6 @@ __metadata: languageName: node linkType: hard -"crypto-js@npm:4.1.1": - version: 4.1.1 - resolution: "crypto-js@npm:4.1.1" - checksum: 50cc66a35f2738171d9a6d80c85ba7d00cb6440b756db035ba9ccd03032c0a803029a62969ecd4c844106c980af87687c64b204dd967989379c4f354fb482d37 - languageName: node - linkType: hard - "crypto-random-string@npm:^1.0.0": version: 1.0.0 resolution: "crypto-random-string@npm:1.0.0" From 2990e9391b08b86bb63de4a0d122fecb8c63b8aa Mon Sep 17 00:00:00 2001 From: Dominic Saadi Date: Fri, 3 Nov 2023 16:26:23 -0700 Subject: [PATCH 24/24] try refactoring the authChecks tests these tests (and others) were using an older more verbose playwright api. playwright now recommends using aria-backed selectors (`page.getByRole` instead of `page.locator`). i've also added some logic to the authChecks graphql test so that you can run it more than one time in succession locally. before you'd have to reset your database between runs. lastly, i don't think the `Promise.race()` logic was correct. fixed here. --- .../smoke-tests/auth/tests/authChecks.spec.ts | 79 +++++++++---------- tasks/smoke-tests/shared/common.ts | 58 +++++--------- 2 files changed, 58 insertions(+), 79 deletions(-) diff --git a/tasks/smoke-tests/auth/tests/authChecks.spec.ts b/tasks/smoke-tests/auth/tests/authChecks.spec.ts index b20171bbe277..86b4c711eb88 100644 --- a/tasks/smoke-tests/auth/tests/authChecks.spec.ts +++ b/tasks/smoke-tests/auth/tests/authChecks.spec.ts @@ -2,12 +2,16 @@ import { test, expect } from '@playwright/test' import { loginAsTestUser, signUpTestUser } from '../../shared/common' -// Signs up a user before these tests +const testUser = { + email: 'testuser@bazinga.com', + password: 'test123', + fullName: 'Test User', +} test.beforeAll(async ({ browser }) => { const page = await browser.newPage() - await signUpTestUser({ page }) + await signUpTestUser({ page, ...testUser }) await page.close() }) @@ -20,7 +24,7 @@ test('useAuth hook, auth redirects checks', async ({ page }) => { `http://localhost:8910/login?redirectTo=/profile` ) - await loginAsTestUser({ page }) + await loginAsTestUser({ page, ...testUser }) await page.goto('/profile') @@ -41,14 +45,19 @@ test('useAuth hook, auth redirects checks', async ({ page }) => { 'Is Adminfalse' ) - // Log Out await page.goto('/') - await page.click('text=Log Out') - await expect(await page.locator('text=Login')).toBeTruthy() + await page.getByText('Log Out').click() + await expect(page.getByText('Log In')).toBeVisible() }) +const post = { + title: 'Hello world! Soft kittens are the best.', + body: 'Bazinga, bazinga, bazinga', + authorId: '2', +} + test('requireAuth graphql checks', async ({ page }) => { - // Create posts + // Try to create a post as an anonymous user. await createNewPost({ page }) await expect( @@ -59,48 +68,36 @@ test('requireAuth graphql checks', async ({ page }) => { await page.goto('/') - await expect( - await page - .locator('article:has-text("Hello world! Soft kittens are the best.")') - .count() - ).toBe(0) + await expect(page.getByText(post.title)).not.toBeVisible() - await loginAsTestUser({ - page, - }) + // Now log in and try again. + await loginAsTestUser({ page, ...testUser }) await createNewPost({ page }) await page.goto('/') - await expect( - await page - .locator('article:has-text("Hello world! Soft kittens are the best.")') - .first() - ).not.toBeEmpty() + + await expect(page.getByText(post.title)).toBeVisible() + + // Delete the post to keep this test idempotent. + // Clicking "Delete" opens a confirmation dialog that we havee to accept. + await page.goto('/posts') + + page.once('dialog', (dialog) => dialog.accept()) + + await page + .getByRole('row') + .filter({ has: page.getByText(post.title) }) + .getByRole('button', { name: 'Delete' }) + .click() }) async function createNewPost({ page }) { await page.goto('/posts/new') - await page.locator('input[name="title"]').click() - await page - .locator('input[name="title"]') - .fill('Hello world! Soft kittens are the best.') - await page.locator('input[name="title"]').press('Tab') - await page.locator('input[name="body"]').fill('Bazinga, bazinga, bazinga') - await page.locator('input[name="authorId"]').fill('2') - - const permissionError = page - .locator('.rw-form-error-title') - .locator(`text=You don't have permission to do that`) - - // Either wait for success and redirect - // Or get the error - await Promise.all([ - Promise.race([ - page.waitForURL('**/'), - permissionError.waitFor({ timeout: 5000 }), - ]), - await page.click('text=SAVE'), - ]) + await page.getByLabel('Title').fill(post.title) + await page.getByLabel('Body').fill(post.body) + await page.getByLabel('Author id').fill(post.authorId) + + await page.getByRole('button', { name: 'Save' }).click() } diff --git a/tasks/smoke-tests/shared/common.ts b/tasks/smoke-tests/shared/common.ts index d2ab977dddba..829f68854f26 100644 --- a/tasks/smoke-tests/shared/common.ts +++ b/tasks/smoke-tests/shared/common.ts @@ -59,31 +59,21 @@ export const signUpTestUser = async ({ }: AuthUtilsParams) => { await page.goto('/signup') - await page.locator('input[name="username"]').click() - // Fill input[name="username"] - await page.locator('input[name="username"]').fill(email) - // Press Tab - await page.locator('input[name="username"]').press('Tab') - // Fill input[name="password"] - await page.locator('input[name="password"]').fill(password) - await page.locator('input[name="full-name"]').click() - await page.locator('input[name="full-name"]').fill(fullName) - - const alreadyRegisteredErr = page.locator( - `text=Username \`${email}\` already in use` - ) - - // Either wait for signup to succeed and redirect - // Or get the username already registered error, either way is fine! - await Promise.all([ - Promise.race([ - page.waitForURL('**/'), - alreadyRegisteredErr.waitFor({ timeout: 5000 }), - ]), - page.locator('text=Sign Up').click(), + await page.getByLabel('Username').fill(email) + await page.getByLabel('Password').fill(password) + await page.getByLabel('Full Name').fill(fullName) + + await page.getByRole('button', { name: 'Sign Up' }).click() + + // Wait for either... + // - signup to succeed and redirect to the home page + // - an error message to appear in a toast + await Promise.race([ + page.waitForURL('/'), + expect( + page.getByText(`Username \`${email}\` already in use`) + ).toBeVisible(), ]) - - console.log(`Signup successful for ${email}!`) } export const loginAsTestUser = async ({ @@ -93,18 +83,10 @@ export const loginAsTestUser = async ({ }: AuthUtilsParams) => { await page.goto('/login') - // Click input[name="username"] - await page.locator('input[name="username"]').click() - // Fill input[name="username"] - await page.locator('input[name="username"]').fill(email) - // Click input[name="password"] - await page.locator('input[name="password"]').click() - // Fill input[name="password"] - await page.locator('input[name="password"]').fill(password) - - // Click button:has-text("Login") - await Promise.all([ - page.waitForURL('**/'), - page.locator('button:has-text("Login")').click(), - ]) + await page.getByLabel('Username').fill(email) + await page.getByLabel('Password').fill(password) + + await page.getByRole('button', { name: 'Login' }).click() + + await page.waitForURL('/') }