From 42d0b1f53e5b1fad20439176bd12def9456e3a82 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Tue, 8 Nov 2022 10:49:56 +0800 Subject: [PATCH] fix: regression of password hasing & fine tune default logging --- package.json | 2 +- packages/internal/package.json | 6 +- packages/internal/src/handler/data/handler.ts | 8 ++- .../internal/src/handler/data/policy-utils.ts | 2 +- packages/runtime/package.json | 2 +- packages/schema/package.json | 8 ++- packages/schema/src/res/stdlib.zmodel | 10 +-- pnpm-lock.yaml | 4 ++ samples/todo/package.json | 2 +- samples/todo/zenstack.config.json | 3 + tests/integration/tests/logging.test.ts | 62 +++++++++---------- tests/integration/tests/password.test.ts | 8 ++- 12 files changed, 68 insertions(+), 49 deletions(-) create mode 100644 samples/todo/zenstack.config.json diff --git a/package.json b/package.json index a7df610c9..b640da001 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "zenstack-monorepo", - "version": "0.2.12", + "version": "0.2.15", "description": "", "scripts": { "build": "pnpm -r build", diff --git a/packages/internal/package.json b/packages/internal/package.json index 0014dbddf..4ff9a625d 100644 --- a/packages/internal/package.json +++ b/packages/internal/package.json @@ -1,6 +1,6 @@ { "name": "@zenstackhq/internal", - "version": "0.2.12", + "version": "0.2.15", "displayName": "ZenStack Internal Library", "description": "ZenStack internal runtime library. This package is for supporting runtime functionality of ZenStack and not supposed to be used directly.", "repository": { @@ -10,7 +10,8 @@ "main": "lib/index.js", "types": "lib/index.d.ts", "scripts": { - "build": "tsc", + "clean": "rimraf lib", + "build": "npm run clean && tsc", "watch": "tsc --watch", "lint": "eslint src --ext ts", "prepublishOnly": "pnpm build" @@ -45,6 +46,7 @@ "@types/uuid": "^8.3.4", "eslint": "^8.27.0", "jest": "^29.0.3", + "rimraf": "^3.0.2", "ts-jest": "^29.0.1", "ts-node": "^10.9.1", "tsc-alias": "^1.7.0", diff --git a/packages/internal/src/handler/data/handler.ts b/packages/internal/src/handler/data/handler.ts index ee664e292..09dbc85c2 100644 --- a/packages/internal/src/handler/data/handler.ts +++ b/packages/internal/src/handler/data/handler.ts @@ -78,9 +78,9 @@ export default class DataHandler break; } } catch (err: unknown) { - this.service.error(`${method} ${model}: ${err}`); - if (err instanceof RequestHandlerError) { + this.service.warn(`${method} ${model}: ${err}`); + // in case of errors thrown directly by ZenStack switch (err.code) { case ServerErrorCode.DENIED_BY_POLICY: @@ -105,6 +105,8 @@ export default class DataHandler }); } } else if (this.isPrismaClientKnownRequestError(err)) { + this.service.warn(`${method} ${model}: ${err}`); + // errors thrown by Prisma, try mapping to a known error if (PRISMA_ERROR_MAPPING[err.code]) { res.status(400).send({ @@ -120,6 +122,8 @@ export default class DataHandler }); } } else if (this.isPrismaClientValidationError(err)) { + this.service.warn(`${method} ${model}: ${err}`); + // prisma validation error res.status(400).send({ code: ServerErrorCode.INVALID_REQUEST_PARAMS, diff --git a/packages/internal/src/handler/data/policy-utils.ts b/packages/internal/src/handler/data/policy-utils.ts index bcb015185..317270195 100644 --- a/packages/internal/src/handler/data/policy-utils.ts +++ b/packages/internal/src/handler/data/policy-utils.ts @@ -634,7 +634,7 @@ export async function preprocessWritePayload( const pwdAttr = fieldInfo.attributes?.find( (attr) => attr.name === '@password' ); - if (pwdAttr && fieldInfo.type !== 'String') { + if (pwdAttr && fieldInfo.type === 'String') { // hash password value let salt: string | number | undefined = pwdAttr.args.find( (arg) => arg.name === 'salt' diff --git a/packages/runtime/package.json b/packages/runtime/package.json index bec0d68b1..b0779d542 100644 --- a/packages/runtime/package.json +++ b/packages/runtime/package.json @@ -1,7 +1,7 @@ { "name": "@zenstackhq/runtime", "displayName": "ZenStack Runtime Library", - "version": "0.2.12", + "version": "0.2.15", "description": "This package contains runtime library for consuming client and server side code generated by ZenStack.", "repository": { "type": "git", diff --git a/packages/schema/package.json b/packages/schema/package.json index 1af0273c5..f618436de 100644 --- a/packages/schema/package.json +++ b/packages/schema/package.json @@ -3,7 +3,7 @@ "publisher": "zenstack", "displayName": "ZenStack Language Tools", "description": "ZenStack is a toolkit that simplifies full-stack development", - "version": "0.2.12", + "version": "0.2.15", "author": { "name": "ZenStack Team" }, @@ -67,8 +67,9 @@ "vscode:publish": "vsce publish --no-dependencies", "vscode:prepublish": "cp ../../README.md ./ && pnpm lint && pnpm build", "vscode:package": "vsce package --no-dependencies", + "clean": "rimraf bundle", "build": "pnpm langium:generate && tsc --noEmit && pnpm bundle && cp -r src/res/* bundle/res/", - "bundle": "node build/bundle.js --minify", + "bundle": "npm run clean && node build/bundle.js --minify", "bundle-watch": "node build/bundle.js --watch", "ts:watch": "tsc --watch --noEmit", "tsc-alias:watch": "tsc-alias --watch", @@ -77,7 +78,7 @@ "langium:watch": "langium generate --watch", "watch": "concurrently --kill-others \"npm:langium:watch\" \"npm:bundle-watch\"", "test": "jest", - "prepublishOnly": "cp ../../README.md ./ && pnpm build && pnpm bundle" + "prepublishOnly": "cp ../../README.md ./ && pnpm build" }, "dependencies": { "@zenstackhq/internal": "workspace:*", @@ -112,6 +113,7 @@ "eslint": "^8.27.0", "jest": "^29.2.1", "langium-cli": "^0.5.0", + "rimraf": "^3.0.2", "tmp": "^0.2.1", "ts-jest": "^29.0.3", "ts-node": "^10.9.1", diff --git a/packages/schema/src/res/stdlib.zmodel b/packages/schema/src/res/stdlib.zmodel index c08141ba2..f4679ba3b 100644 --- a/packages/schema/src/res/stdlib.zmodel +++ b/packages/schema/src/res/stdlib.zmodel @@ -104,11 +104,13 @@ attribute @@deny(_ operation: String, _ condition: Boolean) * Indicates that the field is a password field and needs to be hashed before persistence. * * ZenStack uses `bcryptjs` library to hash password. You can use the `saltLength` parameter - * to configure length of salt, or use parameter to provide an explicit salt. By default, 12-byte - * long salt is used. + * to configure the cost of hashing, or use `salt` parameter to provide an explicit salt. + * By default, salt length of 12 is used. * - * @saltLength: length of salt to use - * @salt: salt to use + * @see https://www.npmjs.com/package/bcryptjs for details + * + * @saltLength: length of salt to use (cost factor for the hash function) + * @salt: salt to use (a pregenerated valid salt) */ attribute @password(saltLength: Int?, salt: String?) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 65c3a4d7d..5bfa7c4e0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -22,6 +22,7 @@ importers: next: ^12.3.1 react: ^17.0.2 || ^18 react-dom: ^17.0.2 || ^18 + rimraf: ^3.0.2 swr: ^1.3.0 ts-jest: ^29.0.1 ts-node: ^10.9.1 @@ -46,6 +47,7 @@ importers: '@types/uuid': 8.3.4 eslint: 8.27.0 jest: 29.0.3_johvxhudwcpndp4mle25vwrlq4 + rimraf: 3.0.2 ts-jest: 29.0.1_poggjixajg6vd6yquly7s7dsj4 ts-node: 10.9.1_ck2axrxkiif44rdbzjywaqjysa tsc-alias: 1.7.0 @@ -87,6 +89,7 @@ importers: pluralize: ^8.0.0 prisma: ^4.5.0 promisify: ^0.0.3 + rimraf: ^3.0.2 tmp: ^0.2.1 ts-jest: ^29.0.3 ts-morph: ^16.0.0 @@ -133,6 +136,7 @@ importers: eslint: 8.27.0 jest: 29.2.1_4f2ldd7um3b3u4eyvetyqsphze langium-cli: 0.5.0 + rimraf: 3.0.2 tmp: 0.2.1 ts-jest: 29.0.3_nvckv3qbfhmmsla6emqlkyje4a ts-node: 10.9.1_jcmx33t3olsvcxopqdljsohpme diff --git a/samples/todo/package.json b/samples/todo/package.json index 9efdf340d..99c23dc06 100644 --- a/samples/todo/package.json +++ b/samples/todo/package.json @@ -1,6 +1,6 @@ { "name": "todo", - "version": "0.2.12", + "version": "0.2.15", "private": true, "scripts": { "dev": "next dev", diff --git a/samples/todo/zenstack.config.json b/samples/todo/zenstack.config.json new file mode 100644 index 000000000..b1b6cafaf --- /dev/null +++ b/samples/todo/zenstack.config.json @@ -0,0 +1,3 @@ +{ + "log": ["info", "warn", "error"] +} diff --git a/tests/integration/tests/logging.test.ts b/tests/integration/tests/logging.test.ts index 6d0f9eb2b..c76d85c42 100644 --- a/tests/integration/tests/logging.test.ts +++ b/tests/integration/tests/logging.test.ts @@ -26,12 +26,12 @@ describe('Logging tests', () => { let gotInfoEmit = false; let gotQueryEmit = false; let gotVerboseEmit = false; - let gotErrorEmit = false; + let gotWarnEmit = false; let gotInfoStd = false; let gotQueryStd = false; let gotVerboseStd = false; - let gotErrorStd = false; + let gotWarnStd = false; console.log = jest.fn((...args) => { const msg = args?.[0] as string; @@ -46,10 +46,10 @@ describe('Logging tests', () => { } }); - console.error = jest.fn((...args) => { + console.warn = jest.fn((...args) => { const msg = args?.[0] as string; - if (msg.includes(':error')) { - gotErrorStd = true; + if (msg.includes(':warn')) { + gotWarnStd = true; } }); @@ -68,9 +68,9 @@ describe('Logging tests', () => { gotVerboseEmit = true; }); - service.$on('error', (event) => { - console.log('Got error', event); - gotErrorEmit = true; + service.$on('warn', (event) => { + console.log('Got warn', event); + gotWarnEmit = true; }); await makeClient('/api/data/User').post('/').send({ @@ -80,12 +80,12 @@ describe('Logging tests', () => { expect(gotQueryStd).toBeFalsy(); expect(gotVerboseStd).toBeFalsy(); expect(gotInfoStd).toBeFalsy(); - expect(gotErrorStd).toBeTruthy(); + expect(gotWarnStd).toBeTruthy(); expect(gotInfoEmit).toBeFalsy(); expect(gotQueryEmit).toBeFalsy(); expect(gotVerboseEmit).toBeFalsy(); - expect(gotErrorEmit).toBeFalsy(); + expect(gotWarnEmit).toBeFalsy(); }); it('logging with stdout', async () => { @@ -93,7 +93,7 @@ describe('Logging tests', () => { './zenstack.config.json', ` { - "log": ["query", "verbose", "info", "error"] + "log": ["query", "verbose", "info", "warn"] } ` ); @@ -104,12 +104,12 @@ describe('Logging tests', () => { let gotInfoEmit = false; let gotQueryEmit = false; let gotVerboseEmit = false; - let gotErrorEmit = false; + let gotWarnEmit = false; let gotInfoStd = false; let gotQueryStd = false; let gotVerboseStd = false; - let gotErrorStd = false; + let gotWarnStd = false; console.log = jest.fn((...args) => { const msg = args?.[0] as string; @@ -124,10 +124,10 @@ describe('Logging tests', () => { } }); - console.error = jest.fn((...args) => { + console.warn = jest.fn((...args) => { const msg = args?.[0] as string; - if (msg.includes(':error')) { - gotErrorStd = true; + if (msg.includes(':warn')) { + gotWarnStd = true; } }); @@ -146,9 +146,9 @@ describe('Logging tests', () => { gotVerboseEmit = true; }); - service.$on('error', (event) => { - console.log('Got error', event); - gotErrorEmit = true; + service.$on('warn', (event) => { + console.log('Got warn', event); + gotWarnEmit = true; }); await makeClient('/api/data/User').post('/').send({ @@ -158,12 +158,12 @@ describe('Logging tests', () => { expect(gotQueryStd).toBeTruthy(); expect(gotVerboseStd).toBeTruthy(); expect(gotInfoStd).toBeTruthy(); - expect(gotErrorStd).toBeTruthy(); + expect(gotWarnStd).toBeTruthy(); expect(gotInfoEmit).toBeFalsy(); expect(gotQueryEmit).toBeFalsy(); expect(gotVerboseEmit).toBeFalsy(); - expect(gotErrorEmit).toBeFalsy(); + expect(gotWarnEmit).toBeFalsy(); }); it('logging with event', async () => { @@ -175,7 +175,7 @@ describe('Logging tests', () => { { "level": "query", "emit": "event" }, { "level": "verbose", "emit": "event" }, { "level": "info", "emit": "event" }, - { "level": "error", "emit": "event" } + { "level": "warn", "emit": "event" } ] } ` @@ -187,12 +187,12 @@ describe('Logging tests', () => { let gotInfoEmit = false; let gotQueryEmit = false; let gotVerboseEmit = false; - let gotErrorEmit = false; + let gotWarnEmit = false; let gotInfoStd = false; let gotQueryStd = false; let gotVerboseStd = false; - let gotErrorStd = false; + let gotWarnStd = false; console.log = jest.fn((...args) => { const msg = args?.[0] as string; @@ -207,10 +207,10 @@ describe('Logging tests', () => { } }); - console.error = jest.fn((...args) => { + console.warn = jest.fn((...args) => { const msg = args?.[0] as string; - if (msg.includes('zenstack:error')) { - gotErrorStd = true; + if (msg.includes('zenstack:warn')) { + gotWarnStd = true; } }); @@ -233,10 +233,10 @@ describe('Logging tests', () => { gotVerboseEmit = true; }); - service.$on('error', (event) => { + service.$on('warn', (event) => { expect(event.timestamp).not.toBeUndefined(); expect(event.message).not.toBeUndefined(); - gotErrorEmit = true; + gotWarnEmit = true; }); await makeClient('/api/data/User').post('/').send({ @@ -246,11 +246,11 @@ describe('Logging tests', () => { expect(gotInfoEmit).toBeTruthy(); expect(gotQueryEmit).toBeTruthy(); expect(gotVerboseEmit).toBeTruthy(); - expect(gotErrorEmit).toBeTruthy(); + expect(gotWarnEmit).toBeTruthy(); expect(gotInfoStd).toBeFalsy(); expect(gotQueryStd).toBeFalsy(); expect(gotVerboseStd).toBeFalsy(); - expect(gotErrorStd).toBeFalsy(); + expect(gotWarnStd).toBeFalsy(); }); }); diff --git a/tests/integration/tests/password.test.ts b/tests/integration/tests/password.test.ts index 241b7d746..2bba24d78 100644 --- a/tests/integration/tests/password.test.ts +++ b/tests/integration/tests/password.test.ts @@ -28,7 +28,7 @@ describe('Password attribute tests', () => { }, }) .expect(async (resp) => - expect(compareSync('abc123', resp.body.password)) + expect(compareSync('abc123', resp.body.password)).toBeTruthy() ); await makeClient('/api/data/User/1') @@ -39,7 +39,7 @@ describe('Password attribute tests', () => { }, }) .expect(async (resp) => - expect(compareSync('abc456', resp.body.password)) + expect(compareSync('abc456', resp.body.password)).toBeTruthy() ); }); @@ -71,7 +71,9 @@ describe('Password attribute tests', () => { }, }) .expect(async (resp) => - expect(compareSync('abc456', resp.body.user.password)) + expect( + compareSync('abc456', resp.body.user.password) + ).toBeTruthy() ); }); });