From 72b48bbe812c6bfab99391608076b7679071a8d0 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Thu, 25 Aug 2022 08:39:28 +0200 Subject: [PATCH 1/9] fix: enforce non-nullability for createdBy/modifiedBy arguments (#1959) --- src/lib/services/project-service.ts | 12 +- .../e2e/services/project-service.e2e.test.ts | 105 +++++++++++++++--- 2 files changed, 93 insertions(+), 24 deletions(-) diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 35fd9e29e9..5c5d2732ee 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -294,12 +294,11 @@ export default class ProjectService { }; } - // TODO: Remove the optional nature of createdBy - this in place to make sure enterprise is compatible async addUser( projectId: string, roleId: number, userId: number, - createdBy?: string, + createdBy: string, ): Promise { const [roles, users] = await this.accessService.getProjectRoleAccess( projectId, @@ -323,7 +322,7 @@ export default class ProjectService { await this.eventStore.store( new ProjectUserAddedEvent({ project: projectId, - createdBy, + createdBy: createdBy || 'system-user', data: { roleId, userId, @@ -334,12 +333,11 @@ export default class ProjectService { ); } - // TODO: Remove the optional nature of createdBy - this in place to make sure enterprise is compatible async removeUser( projectId: string, roleId: number, userId: number, - createdBy?: string, + createdBy: string, ): Promise { const role = await this.findProjectRole(projectId, roleId); @@ -367,7 +365,7 @@ export default class ProjectService { projectId: string, roleId: number, groupId: number, - modifiedBy?: string, + modifiedBy: string, ): Promise { const role = await this.accessService.getRole(roleId); const group = await this.groupService.getGroup(groupId); @@ -397,7 +395,7 @@ export default class ProjectService { projectId: string, roleId: number, groupId: number, - modifiedBy?: string, + modifiedBy: string, ): Promise { const group = await this.groupService.getGroup(groupId); const role = await this.accessService.getRole(roleId); diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index df43e62698..57bc0fc355 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -254,8 +254,18 @@ test('should add a member user to the project', async () => { const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); - await projectService.addUser(project.id, memberRole.id, projectMember1.id); - await projectService.addUser(project.id, memberRole.id, projectMember2.id); + await projectService.addUser( + project.id, + memberRole.id, + projectMember1.id, + 'test', + ); + await projectService.addUser( + project.id, + memberRole.id, + projectMember2.id, + 'test', + ); const { users } = await projectService.getAccessToProject(project.id); const memberUsers = users.filter((u) => u.roleId === memberRole.id); @@ -286,8 +296,18 @@ test('should add admin users to the project', async () => { const ownerRole = await stores.roleStore.getRoleByName(RoleName.OWNER); - await projectService.addUser(project.id, ownerRole.id, projectAdmin1.id); - await projectService.addUser(project.id, ownerRole.id, projectAdmin2.id); + await projectService.addUser( + project.id, + ownerRole.id, + projectAdmin1.id, + 'test', + ); + await projectService.addUser( + project.id, + ownerRole.id, + projectAdmin2.id, + 'test', + ); const { users } = await projectService.getAccessToProject(project.id); @@ -315,10 +335,20 @@ test('add user should fail if user already have access', async () => { const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); - await projectService.addUser(project.id, memberRole.id, projectMember1.id); + await projectService.addUser( + project.id, + memberRole.id, + projectMember1.id, + 'test', + ); await expect(async () => - projectService.addUser(project.id, memberRole.id, projectMember1.id), + projectService.addUser( + project.id, + memberRole.id, + projectMember1.id, + 'test', + ), ).rejects.toThrow( new Error('User already has access to project=add-users-twice'), ); @@ -339,11 +369,17 @@ test('should remove user from the project', async () => { const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); - await projectService.addUser(project.id, memberRole.id, projectMember1.id); + await projectService.addUser( + project.id, + memberRole.id, + projectMember1.id, + 'test', + ); await projectService.removeUser( project.id, memberRole.id, projectMember1.id, + 'test', ); const { users } = await projectService.getAccessToProject(project.id); @@ -364,7 +400,12 @@ test('should not remove user from the project', async () => { const ownerRole = roles.find((r) => r.name === RoleName.OWNER); await expect(async () => { - await projectService.removeUser(project.id, ownerRole.id, user.id); + await projectService.removeUser( + project.id, + ownerRole.id, + user.id, + 'test', + ); }).rejects.toThrowError( new Error('A project must have at least one owner'), ); @@ -617,7 +658,12 @@ test('should add a user to the project with a custom role', async () => { ], }); - await projectService.addUser(project.id, customRole.id, projectMember1.id); + await projectService.addUser( + project.id, + customRole.id, + projectMember1.id, + 'test', + ); const { users } = await projectService.getAccessToProject(project.id); @@ -668,8 +714,8 @@ test('should delete role entries when deleting project', async () => { ], }); - await projectService.addUser(project.id, customRole.id, user1.id); - await projectService.addUser(project.id, customRole.id, user2.id); + await projectService.addUser(project.id, customRole.id, user1.id, 'test'); + await projectService.addUser(project.id, customRole.id, user2.id, 'test'); let usersForRole = await accessService.getUsersForRole(customRole.id); expect(usersForRole.length).toBe(2); @@ -715,15 +761,25 @@ test('should change a users role in the project', async () => { }); const member = await stores.roleStore.getRoleByName(RoleName.MEMBER); - await projectService.addUser(project.id, member.id, projectUser.id); + await projectService.addUser(project.id, member.id, projectUser.id, 'test'); const { users } = await projectService.getAccessToProject(project.id); let memberUser = users.filter((u) => u.roleId === member.id); expect(memberUser).toHaveLength(1); expect(memberUser[0].id).toBe(projectUser.id); expect(memberUser[0].name).toBe(projectUser.name); - await projectService.removeUser(project.id, member.id, projectUser.id); - await projectService.addUser(project.id, customRole.id, projectUser.id); + await projectService.removeUser( + project.id, + member.id, + projectUser.id, + 'test', + ); + await projectService.addUser( + project.id, + customRole.id, + projectUser.id, + 'test', + ); let { users: updatedUsers } = await projectService.getAccessToProject( project.id, @@ -751,7 +807,12 @@ test('should update role for user on project', async () => { const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); const ownerRole = await stores.roleStore.getRoleByName(RoleName.OWNER); - await projectService.addUser(project.id, memberRole.id, projectMember1.id); + await projectService.addUser( + project.id, + memberRole.id, + projectMember1.id, + 'test', + ); await projectService.changeRole( project.id, ownerRole.id, @@ -788,7 +849,12 @@ test('should able to assign role without existing members', async () => { const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); - await projectService.addUser(project.id, memberRole.id, projectMember1.id); + await projectService.addUser( + project.id, + memberRole.id, + projectMember1.id, + 'test', + ); await projectService.changeRole( project.id, testRole.id, @@ -819,7 +885,12 @@ test('should not update role for user on project when she is the owner', async ( const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); - await projectService.addUser(project.id, memberRole.id, projectMember1.id); + await projectService.addUser( + project.id, + memberRole.id, + projectMember1.id, + 'test', + ); await expect(async () => { await projectService.changeRole( From 46ef3282d9a1f835c0e103239b9714b06a4046fb Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 25 Aug 2022 19:41:52 +0200 Subject: [PATCH 2/9] chore(deps): update dpage/pgadmin4 docker tag to v6.13 (#1966) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- scripts/docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/docker-compose.yml b/scripts/docker-compose.yml index 00a2211486..6789e18047 100644 --- a/scripts/docker-compose.yml +++ b/scripts/docker-compose.yml @@ -11,7 +11,7 @@ services: - 5432:5432 pgadmin: - image: dpage/pgadmin4:6.12 + image: dpage/pgadmin4:6.13 environment: PGADMIN_DEFAULT_EMAIL: 'admin@admin.com' PGADMIN_DEFAULT_PASSWORD: 'admin' From b2663878f95ee5cb6f0dd84b8a21f4d4d5b2c27a Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 25 Aug 2022 21:12:36 +0000 Subject: [PATCH 3/9] chore(deps): update typescript-eslint monorepo to v5.35.1 (#1963) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package.json | 4 +-- yarn.lock | 94 ++++++++++++++++++++++++++-------------------------- 2 files changed, 49 insertions(+), 49 deletions(-) diff --git a/package.json b/package.json index 9b952aee14..4ee7b78d84 100644 --- a/package.json +++ b/package.json @@ -149,8 +149,8 @@ "@types/supertest": "2.0.12", "@types/type-is": "1.6.3", "@types/uuid": "8.3.4", - "@typescript-eslint/eslint-plugin": "5.34.0", - "@typescript-eslint/parser": "5.34.0", + "@typescript-eslint/eslint-plugin": "5.35.1", + "@typescript-eslint/parser": "5.35.1", "copyfiles": "2.4.1", "coveralls": "3.1.1", "del-cli": "5.0.0", diff --git a/yarn.lock b/yarn.lock index 99ba25b95a..b2c01fbf08 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1352,14 +1352,14 @@ dependencies: "@types/yargs-parser" "*" -"@typescript-eslint/eslint-plugin@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-5.34.0.tgz#d690f60e335596f38b01792e8f4b361d9bd0cb35" - integrity sha512-eRfPPcasO39iwjlUAMtjeueRGuIrW3TQ9WseIDl7i5UWuFbf83yYaU7YPs4j8+4CxUMIsj1k+4kV+E+G+6ypDQ== +"@typescript-eslint/eslint-plugin@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-5.35.1.tgz#0d822bfea7469904dfc1bb8f13cabd362b967c93" + integrity sha512-RBZZXZlI4XCY4Wzgy64vB+0slT9+yAPQRjj/HSaRwUot33xbDjF1oN9BLwOLTewoOI0jothIltZRe9uJCHf8gg== dependencies: - "@typescript-eslint/scope-manager" "5.34.0" - "@typescript-eslint/type-utils" "5.34.0" - "@typescript-eslint/utils" "5.34.0" + "@typescript-eslint/scope-manager" "5.35.1" + "@typescript-eslint/type-utils" "5.35.1" + "@typescript-eslint/utils" "5.35.1" debug "^4.3.4" functional-red-black-tree "^1.0.1" ignore "^5.2.0" @@ -1367,69 +1367,69 @@ semver "^7.3.7" tsutils "^3.21.0" -"@typescript-eslint/parser@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/parser/-/parser-5.34.0.tgz#ca710858ea85dbfd30c9b416a335dc49e82dbc07" - integrity sha512-SZ3NEnK4usd2CXkoV3jPa/vo1mWX1fqRyIVUQZR4As1vyp4fneknBNJj+OFtV8WAVgGf+rOHMSqQbs2Qn3nFZQ== +"@typescript-eslint/parser@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/parser/-/parser-5.35.1.tgz#bf2ee2ebeaa0a0567213748243fb4eec2857f04f" + integrity sha512-XL2TBTSrh3yWAsMYpKseBYTVpvudNf69rPOWXWVBI08My2JVT5jR66eTt4IgQFHA/giiKJW5dUD4x/ZviCKyGg== dependencies: - "@typescript-eslint/scope-manager" "5.34.0" - "@typescript-eslint/types" "5.34.0" - "@typescript-eslint/typescript-estree" "5.34.0" + "@typescript-eslint/scope-manager" "5.35.1" + "@typescript-eslint/types" "5.35.1" + "@typescript-eslint/typescript-estree" "5.35.1" debug "^4.3.4" -"@typescript-eslint/scope-manager@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/scope-manager/-/scope-manager-5.34.0.tgz#14efd13dc57602937e25f188fd911f118781e527" - integrity sha512-HNvASMQlah5RsBW6L6c7IJ0vsm+8Sope/wu5sEAf7joJYWNb1LDbJipzmdhdUOnfrDFE6LR1j57x1EYVxrY4ow== +"@typescript-eslint/scope-manager@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/scope-manager/-/scope-manager-5.35.1.tgz#ccb69d54b7fd0f2d0226a11a75a8f311f525ff9e" + integrity sha512-kCYRSAzIW9ByEIzmzGHE50NGAvAP3wFTaZevgWva7GpquDyFPFcmvVkFJGWJJktg/hLwmys/FZwqM9EKr2u24Q== dependencies: - "@typescript-eslint/types" "5.34.0" - "@typescript-eslint/visitor-keys" "5.34.0" + "@typescript-eslint/types" "5.35.1" + "@typescript-eslint/visitor-keys" "5.35.1" -"@typescript-eslint/type-utils@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/type-utils/-/type-utils-5.34.0.tgz#7a324ab9ddd102cd5e1beefc94eea6f3eb32d32d" - integrity sha512-Pxlno9bjsQ7hs1pdWRUv9aJijGYPYsHpwMeCQ/Inavhym3/XaKt1ZKAA8FIw4odTBfowBdZJDMxf2aavyMDkLg== +"@typescript-eslint/type-utils@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/type-utils/-/type-utils-5.35.1.tgz#d50903b56758c5c8fc3be52b3be40569f27f9c4a" + integrity sha512-8xT8ljvo43Mp7BiTn1vxLXkjpw8wS4oAc00hMSB4L1/jIiYbjjnc3Qp2GAUOG/v8zsNCd1qwcqfCQ0BuishHkw== dependencies: - "@typescript-eslint/utils" "5.34.0" + "@typescript-eslint/utils" "5.35.1" debug "^4.3.4" tsutils "^3.21.0" -"@typescript-eslint/types@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-5.34.0.tgz#217bf08049e9e7b86694d982e88a2c1566330c78" - integrity sha512-49fm3xbbUPuzBIOcy2CDpYWqy/X7VBkxVN+DC21e0zIm3+61Z0NZi6J9mqPmSW1BDVk9FIOvuCFyUPjXz93sjA== +"@typescript-eslint/types@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-5.35.1.tgz#af355fe52a0cc88301e889bc4ada72f279b63d61" + integrity sha512-FDaujtsH07VHzG0gQ6NDkVVhi1+rhq0qEvzHdJAQjysN+LHDCKDKCBRlZFFE0ec0jKxiv0hN63SNfExy0KrbQQ== -"@typescript-eslint/typescript-estree@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-5.34.0.tgz#ba7b83f4bf8ccbabf074bbf1baca7a58de3ccb9a" - integrity sha512-mXHAqapJJDVzxauEkfJI96j3D10sd567LlqroyCeJaHnu42sDbjxotGb3XFtGPYKPD9IyLjhsoULML1oI3M86A== +"@typescript-eslint/typescript-estree@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-5.35.1.tgz#db878a39a0dbdc9bb133f11cdad451770bfba211" + integrity sha512-JUqE1+VRTGyoXlDWWjm6MdfpBYVq+hixytrv1oyjYIBEOZhBCwtpp5ZSvBt4wIA1MKWlnaC2UXl2XmYGC3BoQA== dependencies: - "@typescript-eslint/types" "5.34.0" - "@typescript-eslint/visitor-keys" "5.34.0" + "@typescript-eslint/types" "5.35.1" + "@typescript-eslint/visitor-keys" "5.35.1" debug "^4.3.4" globby "^11.1.0" is-glob "^4.0.3" semver "^7.3.7" tsutils "^3.21.0" -"@typescript-eslint/utils@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/utils/-/utils-5.34.0.tgz#0cae98f48d8f9e292e5caa9343611b6faf49e743" - integrity sha512-kWRYybU4Rn++7lm9yu8pbuydRyQsHRoBDIo11k7eqBWTldN4xUdVUMCsHBiE7aoEkFzrUEaZy3iH477vr4xHAQ== +"@typescript-eslint/utils@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/utils/-/utils-5.35.1.tgz#ae1399afbfd6aa7d0ed1b7d941e9758d950250eb" + integrity sha512-v6F8JNXgeBWI4pzZn36hT2HXXzoBBBJuOYvoQiaQaEEjdi5STzux3Yj8v7ODIpx36i/5s8TdzuQ54TPc5AITQQ== dependencies: "@types/json-schema" "^7.0.9" - "@typescript-eslint/scope-manager" "5.34.0" - "@typescript-eslint/types" "5.34.0" - "@typescript-eslint/typescript-estree" "5.34.0" + "@typescript-eslint/scope-manager" "5.35.1" + "@typescript-eslint/types" "5.35.1" + "@typescript-eslint/typescript-estree" "5.35.1" eslint-scope "^5.1.1" eslint-utils "^3.0.0" -"@typescript-eslint/visitor-keys@5.34.0": - version "5.34.0" - resolved "https://registry.yarnpkg.com/@typescript-eslint/visitor-keys/-/visitor-keys-5.34.0.tgz#d0fb3e31033e82ddd5de048371ad39eb342b2d40" - integrity sha512-O1moYjOSrab0a2fUvFpsJe0QHtvTC+cR+ovYpgKrAVXzqQyc74mv76TgY6z+aEtjQE2vgZux3CQVtGryqdcOAw== +"@typescript-eslint/visitor-keys@5.35.1": + version "5.35.1" + resolved "https://registry.yarnpkg.com/@typescript-eslint/visitor-keys/-/visitor-keys-5.35.1.tgz#285e9e34aed7c876f16ff646a3984010035898e6" + integrity sha512-cEB1DvBVo1bxbW/S5axbGPE6b7FIMAbo3w+AGq6zNDA7+NYJOIkKj/sInfTv4edxd4PxJSgdN4t6/pbvgA+n5g== dependencies: - "@typescript-eslint/types" "5.34.0" + "@typescript-eslint/types" "5.35.1" eslint-visitor-keys "^3.3.0" "@unleash/express-openapi@^0.2.0": From b3949dc9f50b4f333cc920b5882d8319856974c0 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 26 Aug 2022 03:45:26 +0000 Subject: [PATCH 4/9] chore(deps): update dependency typescript to v4.8.2 (#1967) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package.json | 2 +- website/package.json | 2 +- website/yarn.lock | 8 ++++---- yarn.lock | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 4ee7b78d84..3cbcb51a78 100644 --- a/package.json +++ b/package.json @@ -175,7 +175,7 @@ "ts-jest": "27.1.5", "ts-node": "10.9.1", "tsc-watch": "5.0.3", - "typescript": "4.7.4" + "typescript": "4.8.2" }, "resolutions": { "async": "^3.2.4", diff --git a/website/package.json b/website/package.json index 312290d7f1..d57b1b2da3 100644 --- a/website/package.json +++ b/website/package.json @@ -70,6 +70,6 @@ "enhanced-resolve": "5.10.0", "react-router": "6.3.0", "storybook-addon-root-attribute": "1.0.2", - "typescript": "4.7.4" + "typescript": "4.8.2" } } diff --git a/website/yarn.lock b/website/yarn.lock index d7fc72c404..1723cb1610 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -13651,10 +13651,10 @@ typedarray@^0.0.6: resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777" integrity sha512-/aCDEGatGvZ2BIk+HmLf4ifCJFwvKFNb9/JeZPMulfgFracn9QFcAf5GO8B/mweUjSoblS5In0cWhqpfs/5PQA== -typescript@4.7.4: - version "4.7.4" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.7.4.tgz#1a88596d1cf47d59507a1bcdfb5b9dfe4d488235" - integrity sha512-C0WQT0gezHuw6AdY1M2jxUO83Rjf0HP7Sk1DtXj6j1EwkQNZrHAg2XPWlq62oqEhYvONq5pkC2Y9oPljWToLmQ== +typescript@4.8.2: + version "4.8.2" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.8.2.tgz#e3b33d5ccfb5914e4eeab6699cf208adee3fd790" + integrity sha512-C0I1UsrrDHo2fYI5oaCGbSejwX4ch+9Y5jTQELvovfmFkK3HHSZJB8MSJcWLmCUBzQBchCrZ9rMRV6GuNrvGtw== ua-parser-js@^0.7.30: version "0.7.31" diff --git a/yarn.lock b/yarn.lock index b2c01fbf08..58f41979aa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7187,10 +7187,10 @@ typedarray@^0.0.6: resolved "https://registry.npmjs.org/typedarray/-/typedarray-0.0.6.tgz" integrity sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c= -typescript@4.7.4: - version "4.7.4" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.7.4.tgz#1a88596d1cf47d59507a1bcdfb5b9dfe4d488235" - integrity sha512-C0WQT0gezHuw6AdY1M2jxUO83Rjf0HP7Sk1DtXj6j1EwkQNZrHAg2XPWlq62oqEhYvONq5pkC2Y9oPljWToLmQ== +typescript@4.8.2: + version "4.8.2" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.8.2.tgz#e3b33d5ccfb5914e4eeab6699cf208adee3fd790" + integrity sha512-C0I1UsrrDHo2fYI5oaCGbSejwX4ch+9Y5jTQELvovfmFkK3HHSZJB8MSJcWLmCUBzQBchCrZ9rMRV6GuNrvGtw== uid-safe@~2.1.5: version "2.1.5" From f3e8f723a242dd330fddee0628c25cc10836b37e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 26 Aug 2022 08:22:42 +0200 Subject: [PATCH 5/9] Feat/exp flag loader (#1961) * fix: remove unused exp flag * fix: remove unused flag * fix: add support for external flag resolver * fix: rename flagsresolver to flagresolver * fix: disable external flag resolver * fix: refactor a bit * fix: stop using unleash in server-dev * fix: remove userGroups flag * fix: revert bumping frontend --- .../__snapshots__/create-config.test.ts.snap | 23 +++- src/lib/app.ts | 2 +- src/lib/create-config.ts | 22 ++-- src/lib/db/access-store.ts | 46 +++----- src/lib/db/index.ts | 7 +- src/lib/experimental.ts | 12 --- src/lib/middleware/api-token-middleware.ts | 3 +- src/lib/routes/admin-api/config.ts | 13 ++- src/lib/routes/admin-api/event.ts | 7 +- src/lib/routes/admin-api/events.test.ts | 2 +- src/lib/routes/admin-api/user-admin.ts | 7 +- src/lib/routes/client-api/metrics.test.ts | 4 +- src/lib/routes/index.ts | 2 +- .../client-metrics/metrics-service-v2.ts | 11 +- src/lib/types/experimental.ts | 43 ++++++++ src/lib/types/option.ts | 7 +- src/lib/util/flag-resolver.test.ts | 101 ++++++++++++++++++ src/lib/util/flag-resolver.ts | 38 +++++++ src/server-dev.ts | 11 +- src/test/config/test-config.ts | 7 +- .../e2e/api/admin/client-metrics.e2e.test.ts | 4 +- src/test/e2e/api/client/metricsV2.e2e.test.ts | 4 +- 22 files changed, 278 insertions(+), 98 deletions(-) delete mode 100644 src/lib/experimental.ts create mode 100644 src/lib/types/experimental.ts create mode 100644 src/lib/util/flag-resolver.test.ts create mode 100644 src/lib/util/flag-resolver.ts diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index c2cf006a39..e9783fec3a 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -62,8 +62,26 @@ Object { }, "eventHook": undefined, "experimental": Object { - "batchMetrics": false, - "embedProxy": false, + "externalResolver": Object { + "isEnabled": [Function], + }, + "flags": Object { + "ENABLE_DARK_MODE_SUPPORT": false, + "anonymiseEventLog": false, + "batchMetrics": false, + "embedProxy": false, + }, + }, + "flagResolver": FlagResolver { + "experiments": Object { + "ENABLE_DARK_MODE_SUPPORT": false, + "anonymiseEventLog": false, + "batchMetrics": false, + "embedProxy": false, + }, + "externalResolver": Object { + "isEnabled": [Function], + }, }, "frontendApiOrigins": Array [], "getLogger": [Function], @@ -106,6 +124,7 @@ Object { "ui": Object { "flags": Object { "E": true, + "ENABLE_DARK_MODE_SUPPORT": false, }, }, "versionCheck": Object { diff --git a/src/lib/app.ts b/src/lib/app.ts index 645d83fc09..e85b21cb81 100644 --- a/src/lib/app.ts +++ b/src/lib/app.ts @@ -70,7 +70,7 @@ export default async function getApp( } if ( - config.experimental.embedProxy && + config.experimental.flags.embedProxy && config.frontendApiOrigins.length > 0 ) { // Support CORS preflight requests for the frontend endpoints. diff --git a/src/lib/create-config.ts b/src/lib/create-config.ts index fd739350b6..cc3a5cdb88 100644 --- a/src/lib/create-config.ts +++ b/src/lib/create-config.ts @@ -34,11 +34,15 @@ import { parseEnvVarNumber, parseEnvVarStrings, } from './util/parseEnvVar'; -import { IExperimentalOptions } from './experimental'; +import { + defaultExperimentalOptions, + IExperimentalOptions, +} from './types/experimental'; import { DEFAULT_SEGMENT_VALUES_LIMIT, DEFAULT_STRATEGY_SEGMENTS_LIMIT, } from './util/segments'; +import FlagResolver from './util/flag-resolver'; const safeToUpper = (s: string) => (s ? s.toUpperCase() : s); @@ -55,15 +59,12 @@ function mergeAll(objects: Partial[]): T { function loadExperimental(options: IUnleashOptions): IExperimentalOptions { return { + ...defaultExperimentalOptions, ...options.experimental, - embedProxy: parseEnvVarBoolean( - process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY, - Boolean(options.experimental?.embedProxy), - ), - batchMetrics: parseEnvVarBoolean( - process.env.UNLEASH_EXPERIMENTAL_BATCH_METRICS, - Boolean(options.experimental?.batchMetrics), - ), + flags: { + ...defaultExperimentalOptions.flags, + ...options.experimental?.flags, + }, }; } @@ -102,6 +103,7 @@ function loadUI(options: IUnleashOptions): IUIConfig { ui.flags = { E: true, + ENABLE_DARK_MODE_SUPPORT: false, }; return mergeAll([uiO, ui]); } @@ -375,6 +377,7 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig { ]); const experimental = loadExperimental(options); + const flagResolver = new FlagResolver(experimental); const ui = loadUI(options); @@ -434,6 +437,7 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig { ui, import: importSetting, experimental, + flagResolver, email, secureHeaders, enableOAS, diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts index b1eb409ed3..69b4e06f21 100644 --- a/src/lib/db/access-store.ts +++ b/src/lib/db/access-store.ts @@ -45,16 +45,8 @@ export class AccessStore implements IAccessStore { private db: Knex; - private enableUserGroupPermissions: boolean; - - constructor( - db: Knex, - eventBus: EventEmitter, - getLogger: Function, - enableUserGroupPermissions: boolean, - ) { + constructor(db: Knex, eventBus: EventEmitter, getLogger: Function) { this.db = db; - this.enableUserGroupPermissions = enableUserGroupPermissions; this.logger = getLogger('access-store.ts'); this.timer = (action: string) => metricsHelper.wrapTimer(eventBus, DB_TIME, { @@ -133,27 +125,21 @@ export class AccessStore implements IAccessStore { .join(`${T.PERMISSIONS} AS p`, 'p.id', 'rp.permission_id') .where('ur.user_id', '=', userId); - if (this.enableUserGroupPermissions) { - userPermissionQuery = userPermissionQuery.union((db) => { - db.select( - 'project', - 'permission', - 'environment', - 'p.type', - 'gr.role_id', - ) - .from(`${T.GROUP_USER} AS gu`) - .join(`${T.GROUPS} AS g`, 'g.id', 'gu.group_id') - .join(`${T.GROUP_ROLE} AS gr`, 'gu.group_id', 'gr.group_id') - .join( - `${T.ROLE_PERMISSION} AS rp`, - 'rp.role_id', - 'gr.role_id', - ) - .join(`${T.PERMISSIONS} AS p`, 'p.id', 'rp.permission_id') - .where('gu.user_id', '=', userId); - }); - } + userPermissionQuery = userPermissionQuery.union((db) => { + db.select( + 'project', + 'permission', + 'environment', + 'p.type', + 'gr.role_id', + ) + .from(`${T.GROUP_USER} AS gu`) + .join(`${T.GROUPS} AS g`, 'g.id', 'gu.group_id') + .join(`${T.GROUP_ROLE} AS gr`, 'gu.group_id', 'gr.group_id') + .join(`${T.ROLE_PERMISSION} AS rp`, 'rp.role_id', 'gr.role_id') + .join(`${T.PERMISSIONS} AS p`, 'p.id', 'rp.permission_id') + .where('gu.user_id', '=', userId); + }); const rows = await userPermissionQuery; stopTimer(); return rows.map(this.mapUserPermission); diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 100d914a3a..b9282b2735 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -57,12 +57,7 @@ export const createStores = ( tagStore: new TagStore(db, eventBus, getLogger), tagTypeStore: new TagTypeStore(db, eventBus, getLogger), addonStore: new AddonStore(db, eventBus, getLogger), - accessStore: new AccessStore( - db, - eventBus, - getLogger, - config?.experimental?.userGroups, - ), + accessStore: new AccessStore(db, eventBus, getLogger), apiTokenStore: new ApiTokenStore(db, eventBus, getLogger), resetTokenStore: new ResetTokenStore(db, eventBus, getLogger), sessionStore: new SessionStore(db, eventBus, getLogger), diff --git a/src/lib/experimental.ts b/src/lib/experimental.ts deleted file mode 100644 index 4476646c53..0000000000 --- a/src/lib/experimental.ts +++ /dev/null @@ -1,12 +0,0 @@ -export interface IExperimentalOptions { - metricsV2?: IExperimentalToggle; - clientFeatureMemoize?: IExperimentalToggle; - userGroups?: boolean; - anonymiseEventLog?: boolean; - embedProxy?: boolean; - batchMetrics?: boolean; -} - -export interface IExperimentalToggle { - enabled: boolean; -} diff --git a/src/lib/middleware/api-token-middleware.ts b/src/lib/middleware/api-token-middleware.ts index 32cb948476..0323b241d3 100644 --- a/src/lib/middleware/api-token-middleware.ts +++ b/src/lib/middleware/api-token-middleware.ts @@ -42,7 +42,8 @@ const apiAccessMiddleware = ( if ( (apiUser.type === CLIENT && !isClientApi(req)) || (apiUser.type === FRONTEND && !isProxyApi(req)) || - (apiUser.type === FRONTEND && !experimental.embedProxy) + (apiUser.type === FRONTEND && + !experimental.flags.embedProxy) ) { res.status(403).send({ message: TOKEN_TYPE_ERROR_MESSAGE }); return; diff --git a/src/lib/routes/admin-api/config.ts b/src/lib/routes/admin-api/config.ts index 24338ac160..ada7724331 100644 --- a/src/lib/routes/admin-api/config.ts +++ b/src/lib/routes/admin-api/config.ts @@ -1,4 +1,5 @@ -import { Request, Response } from 'express'; +import { Response } from 'express'; +import { AuthedRequest } from '../../types/core'; import { IUnleashServices } from '../../types/services'; import { IAuthType, IUnleashConfig } from '../../types/option'; import version from '../../util/version'; @@ -66,7 +67,7 @@ class ConfigController extends Controller { } async getUIConfig( - req: Request, + req: AuthedRequest, res: Response, ): Promise { const simpleAuthSettings = @@ -76,8 +77,14 @@ class ConfigController extends Controller { simpleAuthSettings?.disabled || this.config.authentication.type == IAuthType.NONE; + const expFlags = this.config.flagResolver.getAll({ + email: req.user.email, + }); + const flags = { ...this.config.ui.flags, ...expFlags }; + const response: UiConfigSchema = { ...this.config.ui, + flags, version, emailEnabled: this.emailService.isEnabled(), unleashUrl: this.config.server.unleashUrl, @@ -87,7 +94,7 @@ class ConfigController extends Controller { strategySegmentsLimit: this.config.strategySegmentsLimit, versionInfo: this.versionService.getVersionInfo(), disablePasswordAuth, - embedProxy: this.config.experimental.embedProxy, + embedProxy: this.config.experimental.flags.embedProxy, }; this.openApiService.respondWithValidation( diff --git a/src/lib/routes/admin-api/event.ts b/src/lib/routes/admin-api/event.ts index 6886281e0c..3f0fd8dec7 100644 --- a/src/lib/routes/admin-api/event.ts +++ b/src/lib/routes/admin-api/event.ts @@ -21,12 +21,13 @@ import { import { getStandardResponses } from '../../../lib/openapi/util/standard-responses'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; import { SearchEventsSchema } from '../../openapi/spec/search-events-schema'; +import { IFlagResolver } from '../../types/experimental'; const version = 1; export default class EventController extends Controller { private eventService: EventService; - private anonymise: boolean = false; + private flagResolver: IFlagResolver; private openApiService: OpenApiService; @@ -39,7 +40,7 @@ export default class EventController extends Controller { ) { super(config); this.eventService = eventService; - this.anonymise = config.experimental?.anonymiseEventLog; + this.flagResolver = config.flagResolver; this.openApiService = openApiService; this.route({ @@ -106,7 +107,7 @@ export default class EventController extends Controller { } maybeAnonymiseEvents(events: IEvent[]): IEvent[] { - if (this.anonymise) { + if (this.flagResolver.isEnabled('anonymiseEventLog')) { return events.map((e: IEvent) => ({ ...e, createdBy: anonymise(e.createdBy), diff --git a/src/lib/routes/admin-api/events.test.ts b/src/lib/routes/admin-api/events.test.ts index 333300229b..526cda2589 100644 --- a/src/lib/routes/admin-api/events.test.ts +++ b/src/lib/routes/admin-api/events.test.ts @@ -12,7 +12,7 @@ async function getSetup(anonymise: boolean = false) { const stores = createStores(); const config = createTestConfig({ server: { baseUriPath: base }, - experimental: { anonymiseEventLog: anonymise }, + experimental: { flags: { anonymiseEventLog: anonymise } }, }); const services = createServices(stores, config); const app = await getApp(config, stores, services); diff --git a/src/lib/routes/admin-api/user-admin.ts b/src/lib/routes/admin-api/user-admin.ts index acf1c458cb..b7a21cc68c 100644 --- a/src/lib/routes/admin-api/user-admin.ts +++ b/src/lib/routes/admin-api/user-admin.ts @@ -37,9 +37,10 @@ import { usersGroupsBaseSchema, } from '../../openapi/spec/users-groups-base-schema'; import { IGroup } from '../../types/group'; +import { IFlagResolver } from '../../types/experimental'; export default class UserAdminController extends Controller { - private anonymise: boolean = false; + private flagResolver: IFlagResolver; private userService: UserService; @@ -90,7 +91,7 @@ export default class UserAdminController extends Controller { this.groupService = groupService; this.logger = config.getLogger('routes/user-controller.ts'); this.unleashUrl = config.server.unleashUrl; - this.anonymise = config.experimental?.anonymiseEventLog; + this.flagResolver = config.flagResolver; this.route({ method: 'post', @@ -294,7 +295,7 @@ export default class UserAdminController extends Controller { typeof q === 'string' && q.length > 1 ? await this.userService.search(q) : []; - if (this.anonymise) { + if (this.flagResolver.isEnabled('anonymiseEventLog')) { users = this.anonymiseUsers(users); } this.openApiService.respondWithValidation( diff --git a/src/lib/routes/client-api/metrics.test.ts b/src/lib/routes/client-api/metrics.test.ts index 98a7414803..9a579de5e4 100644 --- a/src/lib/routes/client-api/metrics.test.ts +++ b/src/lib/routes/client-api/metrics.test.ts @@ -82,9 +82,7 @@ test('should accept client metrics with yes/no', () => { }); test('should accept client metrics with yes/no with metricsV2', async () => { - const testRunner = await getSetup({ - experimental: { metricsV2: { enabled: true } }, - }); + const testRunner = await getSetup(); await testRunner.request .post('/api/client/metrics') .send({ diff --git a/src/lib/routes/index.ts b/src/lib/routes/index.ts index 410590c49a..27e842dfb4 100644 --- a/src/lib/routes/index.ts +++ b/src/lib/routes/index.ts @@ -28,7 +28,7 @@ class IndexRouter extends Controller { this.use('/api/admin', new AdminApi(config, services).router); this.use('/api/client', new ClientApi(config, services).router); - if (config.experimental.embedProxy) { + if (config.experimental.flags.embedProxy) { this.use( '/api/frontend', new ProxyController(config, services).router, diff --git a/src/lib/services/client-metrics/metrics-service-v2.ts b/src/lib/services/client-metrics/metrics-service-v2.ts index 9a47faad76..3ffece6f23 100644 --- a/src/lib/services/client-metrics/metrics-service-v2.ts +++ b/src/lib/services/client-metrics/metrics-service-v2.ts @@ -16,7 +16,6 @@ import ApiUser from '../../types/api-user'; import { ALL } from '../../types/models/api-token'; import User from '../../types/user'; import { collapseHourlyMetrics } from '../../util/collapseHourlyMetrics'; -import { IExperimentalOptions } from '../../experimental'; export default class ClientMetricsServiceV2 { private timers: NodeJS.Timeout[] = []; @@ -27,7 +26,7 @@ export default class ClientMetricsServiceV2 { private featureToggleStore: IFeatureToggleStore; - private experimental: IExperimentalOptions; + private batchMetricsEnabled: boolean; private eventBus: EventEmitter; @@ -47,13 +46,13 @@ export default class ClientMetricsServiceV2 { ) { this.featureToggleStore = featureToggleStore; this.clientMetricsStoreV2 = clientMetricsStoreV2; - this.experimental = experimental; + this.batchMetricsEnabled = experimental.flags.batchMetrics; this.eventBus = eventBus; this.logger = getLogger( '/services/client-metrics/client-metrics-service-v2.ts', ); - if (this.experimental.batchMetrics) { + if (this.batchMetricsEnabled) { this.timers.push( setInterval(() => { this.bulkAdd().catch(console.error); @@ -91,7 +90,7 @@ export default class ClientMetricsServiceV2 { })) .filter((item) => !(item.yes === 0 && item.no === 0)); - if (this.experimental.batchMetrics) { + if (this.batchMetricsEnabled) { this.unsavedMetrics = collapseHourlyMetrics([ ...this.unsavedMetrics, ...clientMetrics, @@ -104,7 +103,7 @@ export default class ClientMetricsServiceV2 { } async bulkAdd(): Promise { - if (this.experimental.batchMetrics && this.unsavedMetrics.length > 0) { + if (this.batchMetricsEnabled && this.unsavedMetrics.length > 0) { // Make a copy of `unsavedMetrics` in case new metrics // arrive while awaiting `batchInsertMetrics`. const copy = [...this.unsavedMetrics]; diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts new file mode 100644 index 0000000000..902442ff8d --- /dev/null +++ b/src/lib/types/experimental.ts @@ -0,0 +1,43 @@ +import { parseEnvVarBoolean } from '../util/parseEnvVar'; + +export type IFlags = Partial>; + +export const defaultExperimentalOptions = { + flags: { + ENABLE_DARK_MODE_SUPPORT: false, + anonymiseEventLog: false, + embedProxy: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY, + false, + ), + batchMetrics: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_BATCH_METRICS, + false, + ), + }, + externalResolver: { isEnabled: (): boolean => false }, +}; + +export interface IExperimentalOptions { + flags: { + [key: string]: boolean; + ENABLE_DARK_MODE_SUPPORT?: boolean; + embedProxy?: boolean; + batchMetrics?: boolean; + anonymiseEventLog?: boolean; + }; + externalResolver: IExternalFlagResolver; +} + +export interface IFlagContext { + [key: string]: string; +} + +export interface IFlagResolver { + getAll: (context?: IFlagContext) => IFlags; + isEnabled: (expName: string, context?: IFlagContext) => boolean; +} + +export interface IExternalFlagResolver { + isEnabled: (flagName: string, context?: IFlagContext) => boolean; +} diff --git a/src/lib/types/option.ts b/src/lib/types/option.ts index f3cd0cbfff..da8013afeb 100644 --- a/src/lib/types/option.ts +++ b/src/lib/types/option.ts @@ -1,7 +1,7 @@ import EventEmitter from 'events'; import { LogLevel, LogProvider } from '../logger'; import { ILegacyApiTokenCreate } from './models/api-token'; -import { IExperimentalOptions } from '../experimental'; +import { IFlagResolver, IExperimentalOptions, IFlags } from './experimental'; import SMTPTransport from 'nodemailer/lib/smtp-transport'; export type EventHook = (eventName: string, data: object) => void; @@ -102,7 +102,7 @@ export interface IUnleashOptions { authentication?: Partial; ui?: object; import?: Partial; - experimental?: IExperimentalOptions; + experimental?: Partial; email?: Partial; secureHeaders?: boolean; additionalCspAllowedDomains?: ICspDomainOptions; @@ -139,7 +139,6 @@ export interface IListeningHost { export interface IUIConfig { slogan?: string; name?: string; - flags?: { [key: string]: boolean }; links?: [ { value: string; @@ -148,6 +147,7 @@ export interface IUIConfig { title: string; }, ]; + flags?: IFlags; } export interface ICspDomainOptions { @@ -177,6 +177,7 @@ export interface IUnleashConfig { ui: IUIConfig; import: IImportOption; experimental?: IExperimentalOptions; + flagResolver: IFlagResolver; email: IEmailOption; secureHeaders: boolean; additionalCspAllowedDomains: ICspDomainConfig; diff --git a/src/lib/util/flag-resolver.test.ts b/src/lib/util/flag-resolver.test.ts new file mode 100644 index 0000000000..65cd148f2e --- /dev/null +++ b/src/lib/util/flag-resolver.test.ts @@ -0,0 +1,101 @@ +import { defaultExperimentalOptions } from '../types/experimental'; +import FlagResolver from './flag-resolver'; + +test('should produce empty exposed flags', () => { + const resolver = new FlagResolver(defaultExperimentalOptions); + + const result = resolver.getAll(); + + expect(result.ENABLE_DARK_MODE_SUPPORT).toBe(false); +}); + +test('should produce UI flags with extra dynamic flags', () => { + const config = { + ...defaultExperimentalOptions, + flags: { extraFlag: false }, + }; + const resolver = new FlagResolver(config); + const result = resolver.getAll(); + + expect(result.extraFlag).toBe(false); +}); + +test('should use external resolver for dynamic flags', () => { + const externalResolver = { + isEnabled: (name: string) => { + if (name === 'extraFlag') { + return true; + } + }, + }; + + const resolver = new FlagResolver({ + flags: { + extraFlag: false, + }, + externalResolver, + }); + + const result = resolver.getAll(); + + expect(result.extraFlag).toBe(true); +}); + +test('should not use external resolver for enabled experiments', () => { + const externalResolver = { + isEnabled: () => { + return false; + }, + }; + + const resolver = new FlagResolver({ + flags: { + should_be_enabled: true, + extraFlag: false, + }, + externalResolver, + }); + + const result = resolver.getAll(); + + expect(result.should_be_enabled).toBe(true); +}); + +test('should load experimental flags', () => { + const externalResolver = { + isEnabled: () => { + return false; + }, + }; + const resolver = new FlagResolver({ + flags: { + extraFlag: false, + someFlag: true, + }, + externalResolver, + }); + + expect(resolver.isEnabled('someFlag')).toBe(true); + expect(resolver.isEnabled('extraFlag')).toBe(false); +}); + +test('should load experimental flags from external provider', () => { + const externalResolver = { + isEnabled: (name: string) => { + if (name === 'extraFlag') { + return true; + } + }, + }; + + const resolver = new FlagResolver({ + flags: { + extraFlag: false, + someFlag: true, + }, + externalResolver, + }); + + expect(resolver.isEnabled('someFlag')).toBe(true); + expect(resolver.isEnabled('extraFlag')).toBe(true); +}); diff --git a/src/lib/util/flag-resolver.ts b/src/lib/util/flag-resolver.ts new file mode 100644 index 0000000000..4d3206f0a7 --- /dev/null +++ b/src/lib/util/flag-resolver.ts @@ -0,0 +1,38 @@ +import { + IExperimentalOptions, + IExternalFlagResolver, + IFlagContext, + IFlags, + IFlagResolver, +} from '../types/experimental'; +export default class FlagResolver implements IFlagResolver { + private experiments: IFlags; + + private externalResolver: IExternalFlagResolver; + + constructor(expOpt: IExperimentalOptions) { + this.experiments = expOpt.flags; + this.externalResolver = expOpt.externalResolver; + } + + getAll(context?: IFlagContext): IFlags { + const flags: IFlags = { ...this.experiments }; + + Object.keys(flags).forEach((flagName) => { + if (!this.experiments[flagName]) + flags[flagName] = this.externalResolver.isEnabled( + flagName, + context, + ); + }); + + return flags; + } + + isEnabled(expName: string, context?: IFlagContext): boolean { + if (this.experiments[expName]) { + return true; + } + return this.externalResolver.isEnabled(expName, context); + } +} diff --git a/src/server-dev.ts b/src/server-dev.ts index 1c5a095d2a..20975cc69f 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -31,11 +31,12 @@ process.nextTick(async () => { enable: false, }, experimental: { - metricsV2: { enabled: true }, - anonymiseEventLog: false, - userGroups: true, - embedProxy: true, - batchMetrics: true, + // externalResolver: unleash, + flags: { + embedProxy: true, + batchMetrics: true, + anonymiseEventLog: false, + }, }, authentication: { initApiTokens: [ diff --git a/src/test/config/test-config.ts b/src/test/config/test-config.ts index 45e81aac29..76033fbb79 100644 --- a/src/test/config/test-config.ts +++ b/src/test/config/test-config.ts @@ -23,9 +23,10 @@ export function createTestConfig(config?: IUnleashOptions): IUnleashConfig { enabled: false, }, experimental: { - userGroups: true, - embedProxy: true, - batchMetrics: true, + flags: { + embedProxy: true, + batchMetrics: true, + }, }, }; const options = mergeAll([testConfig, config]); diff --git a/src/test/e2e/api/admin/client-metrics.e2e.test.ts b/src/test/e2e/api/admin/client-metrics.e2e.test.ts index f961bde0c2..2294cc5c9f 100644 --- a/src/test/e2e/api/admin/client-metrics.e2e.test.ts +++ b/src/test/e2e/api/admin/client-metrics.e2e.test.ts @@ -9,9 +9,7 @@ let db: ITestDb; beforeAll(async () => { db = await dbInit('client_metrics_serial', getLogger); - app = await setupAppWithCustomConfig(db.stores, { - experimental: { metricsV2: { enabled: true } }, - }); + app = await setupAppWithCustomConfig(db.stores, {}); }); afterAll(async () => { diff --git a/src/test/e2e/api/client/metricsV2.e2e.test.ts b/src/test/e2e/api/client/metricsV2.e2e.test.ts index 7986df5dad..a2afc9673d 100644 --- a/src/test/e2e/api/client/metricsV2.e2e.test.ts +++ b/src/test/e2e/api/client/metricsV2.e2e.test.ts @@ -11,9 +11,7 @@ let defaultToken; beforeAll(async () => { db = await dbInit('metrics_two_api_client', getLogger); - app = await setupAppWithAuth(db.stores, { - experimental: { metricsV2: { enabled: true } }, - }); + app = await setupAppWithAuth(db.stores, {}); defaultToken = await app.services.apiTokenService.createApiToken({ type: ApiTokenType.CLIENT, project: 'default', From 42d64c8803f1718be5bb9be971f513f54719670e Mon Sep 17 00:00:00 2001 From: olav Date: Fri, 26 Aug 2022 09:09:48 +0200 Subject: [PATCH 6/9] feat: add CORS instance settings (#1957) * feat: add CORS instance settings * refactor: disallow arbitrary asterisks in CORS origins --- .../__snapshots__/create-config.test.ts.snap | 4 +- src/lib/app.ts | 5 +- src/lib/create-config.test.ts | 25 +++++++ src/lib/create-config.ts | 21 ++++-- .../middleware/cors-origin-middleware.test.ts | 68 +++++++++++++++++++ src/lib/middleware/cors-origin-middleware.ts | 28 +++++--- src/lib/openapi/index.ts | 2 + src/lib/openapi/spec/set-ui-config-schema.ts | 24 +++++++ src/lib/openapi/spec/ui-config-schema.ts | 6 ++ src/lib/routes/admin-api/config.ts | 55 +++++++++++++-- src/lib/routes/admin-api/user-admin.ts | 6 +- src/lib/routes/proxy-api/index.ts | 36 ++++------ src/lib/services/setting-service.ts | 48 +++++++++++-- src/lib/services/user-service.ts | 4 +- src/lib/types/settings/frontend-settings.ts | 5 ++ .../types/settings/simple-auth-settings.ts | 3 +- src/lib/util/parseEnvVar.test.ts | 4 +- src/lib/util/parseEnvVar.ts | 4 +- src/lib/util/validateOrigin.test.ts | 24 +++++++ src/lib/util/validateOrigin.ts | 24 +++++++ src/test/e2e/api/admin/config.e2e.test.ts | 62 +++++++++++++++-- .../__snapshots__/openapi.e2e.test.ts.snap | 52 +++++++++++++- .../e2e/services/user-service.e2e.test.ts | 12 +++- 23 files changed, 451 insertions(+), 71 deletions(-) create mode 100644 src/lib/openapi/spec/set-ui-config-schema.ts create mode 100644 src/lib/types/settings/frontend-settings.ts create mode 100644 src/lib/util/validateOrigin.test.ts create mode 100644 src/lib/util/validateOrigin.ts diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index e9783fec3a..c376183173 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -83,7 +83,9 @@ Object { "isEnabled": [Function], }, }, - "frontendApiOrigins": Array [], + "frontendApiOrigins": Array [ + "*", + ], "getLogger": [Function], "import": Object { "dropBeforeImport": false, diff --git a/src/lib/app.ts b/src/lib/app.ts index e85b21cb81..bc67591ebb 100644 --- a/src/lib/app.ts +++ b/src/lib/app.ts @@ -76,10 +76,7 @@ export default async function getApp( // Support CORS preflight requests for the frontend endpoints. // Preflight requests should not have Authorization headers, // so this must be handled before the API token middleware. - app.options( - '/api/frontend*', - corsOriginMiddleware(config.frontendApiOrigins), - ); + app.options('/api/frontend*', corsOriginMiddleware(services)); } switch (config.authentication.type) { diff --git a/src/lib/create-config.test.ts b/src/lib/create-config.test.ts index 67a7b196d1..89cf169e2e 100644 --- a/src/lib/create-config.test.ts +++ b/src/lib/create-config.test.ts @@ -403,3 +403,28 @@ test('Environment variables for client features caching takes priority over opti expect(config.clientFeatureCaching.enabled).toBe(true); expect(config.clientFeatureCaching.maxAge).toBe(120); }); + +test('Environment variables for frontend CORS origins takes priority over options', async () => { + const create = (frontendApiOrigins?): string[] => { + return createConfig({ + frontendApiOrigins, + }).frontendApiOrigins; + }; + + expect(create()).toEqual(['*']); + expect(create([])).toEqual([]); + expect(create(['*'])).toEqual(['*']); + expect(create(['https://example.com'])).toEqual(['https://example.com']); + expect(() => create(['a'])).toThrow('Invalid origin: a'); + + process.env.UNLEASH_FRONTEND_API_ORIGINS = ''; + expect(create()).toEqual([]); + process.env.UNLEASH_FRONTEND_API_ORIGINS = '*'; + expect(create()).toEqual(['*']); + process.env.UNLEASH_FRONTEND_API_ORIGINS = 'https://example.com, *'; + expect(create()).toEqual(['https://example.com', '*']); + process.env.UNLEASH_FRONTEND_API_ORIGINS = 'b'; + expect(() => create(['a'])).toThrow('Invalid origin: b'); + delete process.env.UNLEASH_FRONTEND_API_ORIGINS; + expect(create()).toEqual(['*']); +}); diff --git a/src/lib/create-config.ts b/src/lib/create-config.ts index cc3a5cdb88..f5abb14047 100644 --- a/src/lib/create-config.ts +++ b/src/lib/create-config.ts @@ -43,6 +43,7 @@ import { DEFAULT_STRATEGY_SEGMENTS_LIMIT, } from './util/segments'; import FlagResolver from './util/flag-resolver'; +import { validateOrigins } from './util/validateOrigin'; const safeToUpper = (s: string) => (s ? s.toUpperCase() : s); @@ -311,6 +312,20 @@ const parseCspEnvironmentVariables = (): ICspDomainConfig => { }; }; +const parseFrontendApiOrigins = (options: IUnleashOptions): string[] => { + const frontendApiOrigins = parseEnvVarStrings( + process.env.UNLEASH_FRONTEND_API_ORIGINS, + options.frontendApiOrigins || ['*'], + ); + + const error = validateOrigins(frontendApiOrigins); + if (error) { + throw new Error(error); + } + + return frontendApiOrigins; +}; + export function createConfig(options: IUnleashOptions): IUnleashConfig { let extraDbOptions = {}; @@ -420,10 +435,6 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig { DEFAULT_STRATEGY_SEGMENTS_LIMIT, ); - const frontendApiOrigins = - options.frontendApiOrigins || - parseEnvVarStrings(process.env.UNLEASH_FRONTEND_API_ORIGINS, []); - const clientFeatureCaching = loadClientCachingOptions(options); return { @@ -449,7 +460,7 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig { eventBus: new EventEmitter(), environmentEnableOverrides, additionalCspAllowedDomains, - frontendApiOrigins, + frontendApiOrigins: parseFrontendApiOrigins(options), inlineSegmentConstraints, segmentValuesLimit, strategySegmentsLimit, diff --git a/src/lib/middleware/cors-origin-middleware.test.ts b/src/lib/middleware/cors-origin-middleware.test.ts index 82ffc43df4..2696c01bc8 100644 --- a/src/lib/middleware/cors-origin-middleware.test.ts +++ b/src/lib/middleware/cors-origin-middleware.test.ts @@ -1,4 +1,21 @@ import { allowRequestOrigin } from './cors-origin-middleware'; +import FakeSettingStore from '../../test/fixtures/fake-setting-store'; +import SettingService from '../services/setting-service'; +import { createTestConfig } from '../../test/config/test-config'; +import FakeEventStore from '../../test/fixtures/fake-event-store'; +import { randomId } from '../util/random-id'; +import { frontendSettingsKey } from '../types/settings/frontend-settings'; + +const createSettingService = (frontendApiOrigins: string[]): SettingService => { + const config = createTestConfig({ frontendApiOrigins }); + + const stores = { + settingStore: new FakeSettingStore(), + eventStore: new FakeEventStore(), + }; + + return new SettingService(stores, config); +}; test('allowRequestOrigin', () => { const dotCom = 'https://example.com'; @@ -16,3 +33,54 @@ test('allowRequestOrigin', () => { expect(allowRequestOrigin(dotCom, [dotOrg, '*'])).toEqual(true); expect(allowRequestOrigin(dotCom, [dotCom, dotOrg, '*'])).toEqual(true); }); + +test('corsOriginMiddleware origin validation', async () => { + const service = createSettingService([]); + const userName = randomId(); + await expect(() => + service.setFrontendSettings({ frontendApiOrigins: ['a'] }, userName), + ).rejects.toThrow('Invalid origin: a'); +}); + +test('corsOriginMiddleware without config', async () => { + const service = createSettingService([]); + const userName = randomId(); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: [], + }); + await service.setFrontendSettings({ frontendApiOrigins: [] }, userName); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: [], + }); + await service.setFrontendSettings({ frontendApiOrigins: ['*'] }, userName); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: ['*'], + }); + await service.delete(frontendSettingsKey, userName); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: [], + }); +}); + +test('corsOriginMiddleware with config', async () => { + const service = createSettingService(['*']); + const userName = randomId(); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: ['*'], + }); + await service.setFrontendSettings({ frontendApiOrigins: [] }, userName); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: [], + }); + await service.setFrontendSettings( + { frontendApiOrigins: ['https://example.com', 'https://example.org'] }, + userName, + ); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: ['https://example.com', 'https://example.org'], + }); + await service.delete(frontendSettingsKey, userName); + expect(await service.getFrontendSettings()).toEqual({ + frontendApiOrigins: ['*'], + }); +}); diff --git a/src/lib/middleware/cors-origin-middleware.ts b/src/lib/middleware/cors-origin-middleware.ts index 5e04abb98f..d9b952b823 100644 --- a/src/lib/middleware/cors-origin-middleware.ts +++ b/src/lib/middleware/cors-origin-middleware.ts @@ -1,25 +1,33 @@ import { RequestHandler } from 'express'; import cors from 'cors'; - -const ANY_ORIGIN = '*'; +import { IUnleashServices } from '../types'; export const allowRequestOrigin = ( requestOrigin: string, allowedOrigins: string[], ): boolean => { return allowedOrigins.some((allowedOrigin) => { - return allowedOrigin === requestOrigin || allowedOrigin === ANY_ORIGIN; + return allowedOrigin === requestOrigin || allowedOrigin === '*'; }); }; // Check the request's Origin header against a list of allowed origins. // The list may include '*', which `cors` does not support natively. -export const corsOriginMiddleware = ( - allowedOrigins: string[], -): RequestHandler => { - return cors((req, callback) => { - callback(null, { - origin: allowRequestOrigin(req.header('Origin'), allowedOrigins), - }); +export const corsOriginMiddleware = ({ + settingService, +}: Pick): RequestHandler => { + return cors(async (req, callback) => { + try { + const { frontendApiOrigins = [] } = + await settingService.getFrontendSettings(); + callback(null, { + origin: allowRequestOrigin( + req.header('Origin'), + frontendApiOrigins, + ), + }); + } catch (error) { + callback(error); + } }); }; diff --git a/src/lib/openapi/index.ts b/src/lib/openapi/index.ts index 22a7d2eb59..86d04fca28 100644 --- a/src/lib/openapi/index.ts +++ b/src/lib/openapi/index.ts @@ -110,6 +110,7 @@ import { proxyFeaturesSchema } from './spec/proxy-features-schema'; import { proxyFeatureSchema } from './spec/proxy-feature-schema'; import { proxyClientSchema } from './spec/proxy-client-schema'; import { proxyMetricsSchema } from './spec/proxy-metrics-schema'; +import { setUiConfigSchema } from './spec/set-ui-config-schema'; // All schemas in `openapi/spec` should be listed here. export const schemas = { @@ -187,6 +188,7 @@ export const schemas = { searchEventsSchema, segmentSchema, setStrategySortOrderSchema, + setUiConfigSchema, sortOrderSchema, splashSchema, stateSchema, diff --git a/src/lib/openapi/spec/set-ui-config-schema.ts b/src/lib/openapi/spec/set-ui-config-schema.ts new file mode 100644 index 0000000000..27f7153a2d --- /dev/null +++ b/src/lib/openapi/spec/set-ui-config-schema.ts @@ -0,0 +1,24 @@ +import { FromSchema } from 'json-schema-to-ts'; + +export const setUiConfigSchema = { + $id: '#/components/schemas/setUiConfigSchema', + type: 'object', + additionalProperties: false, + properties: { + frontendSettings: { + type: 'object', + additionalProperties: false, + required: ['frontendApiOrigins'], + properties: { + frontendApiOrigins: { + type: 'array', + additionalProperties: false, + items: { type: 'string' }, + }, + }, + }, + }, + components: {}, +} as const; + +export type SetUiConfigSchema = FromSchema; diff --git a/src/lib/openapi/spec/ui-config-schema.ts b/src/lib/openapi/spec/ui-config-schema.ts index dde89ef689..1c82027bab 100644 --- a/src/lib/openapi/spec/ui-config-schema.ts +++ b/src/lib/openapi/spec/ui-config-schema.ts @@ -37,6 +37,12 @@ export const uiConfigSchema = { strategySegmentsLimit: { type: 'number', }, + frontendApiOrigins: { + type: 'array', + items: { + type: 'string', + }, + }, flags: { type: 'object', additionalProperties: { diff --git a/src/lib/routes/admin-api/config.ts b/src/lib/routes/admin-api/config.ts index ada7724331..e0037c9a8a 100644 --- a/src/lib/routes/admin-api/config.ts +++ b/src/lib/routes/admin-api/config.ts @@ -7,10 +7,10 @@ import Controller from '../controller'; import VersionService from '../../services/version-service'; import SettingService from '../../services/setting-service'; import { - simpleAuthKey, + simpleAuthSettingsKey, SimpleAuthSettings, } from '../../types/settings/simple-auth-settings'; -import { NONE } from '../../types/permissions'; +import { ADMIN, NONE } from '../../types/permissions'; import { createResponseSchema } from '../../openapi/util/create-response-schema'; import { uiConfigSchema, @@ -18,6 +18,12 @@ import { } from '../../openapi/spec/ui-config-schema'; import { OpenApiService } from '../../services/openapi-service'; import { EmailService } from '../../services/email-service'; +import { emptyResponse } from '../../openapi/util/standard-responses'; +import { IAuthRequest } from '../unleash-types'; +import { extractUsername } from '../../util/extract-user'; +import NotFoundError from '../../error/notfound-error'; +import { SetUiConfigSchema } from '../../openapi/spec/set-ui-config-schema'; +import { createRequestSchema } from '../../openapi/util/create-request-schema'; class ConfigController extends Controller { private versionService: VersionService; @@ -52,26 +58,43 @@ class ConfigController extends Controller { this.route({ method: 'get', path: '', - handler: this.getUIConfig, + handler: this.getUiConfig, permission: NONE, middleware: [ openApiService.validPath({ tags: ['Admin UI'], - operationId: 'getUIConfig', + operationId: 'getUiConfig', responses: { 200: createResponseSchema('uiConfigSchema'), }, }), ], }); + + this.route({ + method: 'post', + path: '', + handler: this.setUiConfig, + permission: ADMIN, + middleware: [ + openApiService.validPath({ + tags: ['Admin UI'], + operationId: 'setUiConfig', + requestBody: createRequestSchema('setUiConfigSchema'), + responses: { 200: emptyResponse }, + }), + ], + }); } - async getUIConfig( + async getUiConfig( req: AuthedRequest, res: Response, ): Promise { - const simpleAuthSettings = - await this.settingService.get(simpleAuthKey); + const [frontendSettings, simpleAuthSettings] = await Promise.all([ + this.settingService.getFrontendSettings(), + this.settingService.get(simpleAuthSettingsKey), + ]); const disablePasswordAuth = simpleAuthSettings?.disabled || @@ -92,6 +115,7 @@ class ConfigController extends Controller { authenticationType: this.config.authentication?.type, segmentValuesLimit: this.config.segmentValuesLimit, strategySegmentsLimit: this.config.strategySegmentsLimit, + frontendApiOrigins: frontendSettings.frontendApiOrigins, versionInfo: this.versionService.getVersionInfo(), disablePasswordAuth, embedProxy: this.config.experimental.flags.embedProxy, @@ -104,5 +128,22 @@ class ConfigController extends Controller { response, ); } + + async setUiConfig( + req: IAuthRequest, + res: Response, + ): Promise { + if (req.body.frontendSettings) { + await this.settingService.setFrontendSettings( + req.body.frontendSettings, + extractUsername(req), + ); + res.sendStatus(204); + return; + } + + throw new NotFoundError(); + } } + export default ConfigController; diff --git a/src/lib/routes/admin-api/user-admin.ts b/src/lib/routes/admin-api/user-admin.ts index b7a21cc68c..acef4f0b29 100644 --- a/src/lib/routes/admin-api/user-admin.ts +++ b/src/lib/routes/admin-api/user-admin.ts @@ -10,7 +10,7 @@ import ResetTokenService from '../../services/reset-token-service'; import { IAuthRequest } from '../unleash-types'; import SettingService from '../../services/setting-service'; import { IUser, SimpleAuthSettings } from '../../server-impl'; -import { simpleAuthKey } from '../../types/settings/simple-auth-settings'; +import { simpleAuthSettingsKey } from '../../types/settings/simple-auth-settings'; import { anonymise } from '../../util/anonymise'; import { OpenApiService } from '../../services/openapi-service'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; @@ -369,7 +369,9 @@ export default class UserAdminController extends Controller { ); const passwordAuthSettings = - await this.settingService.get(simpleAuthKey); + await this.settingService.get( + simpleAuthSettingsKey, + ); let inviteLink: string; if (!passwordAuthSettings?.disabled) { diff --git a/src/lib/routes/proxy-api/index.ts b/src/lib/routes/proxy-api/index.ts index d0fc73429b..2443a47957 100644 --- a/src/lib/routes/proxy-api/index.ts +++ b/src/lib/routes/proxy-api/index.ts @@ -2,9 +2,7 @@ import { Response, Request } from 'express'; import Controller from '../controller'; import { IUnleashConfig, IUnleashServices } from '../../types'; import { Logger } from '../../logger'; -import { OpenApiService } from '../../services/openapi-service'; import { NONE } from '../../types/permissions'; -import { ProxyService } from '../../services/proxy-service'; import ApiUser from '../../types/api-user'; import { proxyFeaturesSchema, @@ -28,29 +26,25 @@ interface ApiUserRequest< user: ApiUser; } +type Services = Pick< + IUnleashServices, + 'settingService' | 'proxyService' | 'openApiService' +>; + export default class ProxyController extends Controller { private readonly logger: Logger; - private proxyService: ProxyService; + private services: Services; - private openApiService: OpenApiService; - - constructor( - config: IUnleashConfig, - { - proxyService, - openApiService, - }: Pick, - ) { + constructor(config: IUnleashConfig, services: Services) { super(config); this.logger = config.getLogger('client-api/feature.js'); - this.proxyService = proxyService; - this.openApiService = openApiService; + this.services = services; if (config.frontendApiOrigins.length > 0) { // Support CORS requests for the frontend endpoints. // Preflight requests are handled in `app.ts`. - this.app.use(corsOriginMiddleware(config.frontendApiOrigins)); + this.app.use(corsOriginMiddleware(services)); } this.route({ @@ -59,7 +53,7 @@ export default class ProxyController extends Controller { handler: this.getProxyFeatures, permission: NONE, middleware: [ - this.openApiService.validPath({ + this.services.openApiService.validPath({ tags: ['Unstable'], operationId: 'getFrontendFeatures', responses: { @@ -89,7 +83,7 @@ export default class ProxyController extends Controller { handler: this.registerProxyMetrics, permission: NONE, middleware: [ - this.openApiService.validPath({ + this.services.openApiService.validPath({ tags: ['Unstable'], operationId: 'registerFrontendMetrics', requestBody: createRequestSchema('proxyMetricsSchema'), @@ -104,7 +98,7 @@ export default class ProxyController extends Controller { handler: ProxyController.registerProxyClient, permission: NONE, middleware: [ - this.openApiService.validPath({ + this.services.openApiService.validPath({ tags: ['Unstable'], operationId: 'registerFrontendClient', requestBody: createRequestSchema('proxyClientSchema'), @@ -141,11 +135,11 @@ export default class ProxyController extends Controller { req: ApiUserRequest, res: Response, ) { - const toggles = await this.proxyService.getProxyFeatures( + const toggles = await this.services.proxyService.getProxyFeatures( req.user, ProxyController.createContext(req), ); - this.openApiService.respondWithValidation( + this.services.openApiService.respondWithValidation( 200, res, proxyFeaturesSchema.$id, @@ -157,7 +151,7 @@ export default class ProxyController extends Controller { req: ApiUserRequest, res: Response, ) { - await this.proxyService.registerProxyMetrics( + await this.services.proxyService.registerProxyMetrics( req.user, req.body, req.ip, diff --git a/src/lib/services/setting-service.ts b/src/lib/services/setting-service.ts index 0b2cf3c030..77ad7c54a2 100644 --- a/src/lib/services/setting-service.ts +++ b/src/lib/services/setting-service.ts @@ -8,31 +8,48 @@ import { SettingDeletedEvent, SettingUpdatedEvent, } from '../types/events'; +import { validateOrigins } from '../util/validateOrigin'; +import { + FrontendSettings, + frontendSettingsKey, +} from '../types/settings/frontend-settings'; +import BadDataError from '../error/bad-data-error'; export default class SettingService { + private config: IUnleashConfig; + private logger: Logger; private settingStore: ISettingStore; private eventStore: IEventStore; + // SettingService.getFrontendSettings is called on every request to the + // frontend API. Keep fetched settings in a cache for fewer DB queries. + private cache = new Map(); + constructor( { settingStore, eventStore, }: Pick, - { getLogger }: Pick, + config: IUnleashConfig, ) { - this.logger = getLogger('services/setting-service.ts'); + this.config = config; + this.logger = config.getLogger('services/setting-service.ts'); this.settingStore = settingStore; this.eventStore = eventStore; } - async get(id: string): Promise { - return this.settingStore.get(id); + async get(id: string, defaultValue?: T): Promise { + if (!this.cache.has(id)) { + this.cache.set(id, await this.settingStore.get(id)); + } + return (this.cache.get(id) as T) || defaultValue; } async insert(id: string, value: object, createdBy: string): Promise { + this.cache.delete(id); const exists = await this.settingStore.exists(id); if (exists) { await this.settingStore.updateRow(id, value); @@ -54,6 +71,7 @@ export default class SettingService { } async delete(id: string, createdBy: string): Promise { + this.cache.delete(id); await this.settingStore.delete(id); await this.eventStore.store( new SettingDeletedEvent({ @@ -64,6 +82,28 @@ export default class SettingService { }), ); } + + async deleteAll(): Promise { + this.cache.clear(); + await this.settingStore.deleteAll(); + } + + async setFrontendSettings( + value: FrontendSettings, + createdBy: string, + ): Promise { + const error = validateOrigins(value.frontendApiOrigins); + if (error) { + throw new BadDataError(error); + } + await this.insert(frontendSettingsKey, value, createdBy); + } + + async getFrontendSettings(): Promise { + return this.get(frontendSettingsKey, { + frontendApiOrigins: this.config.frontendApiOrigins, + }); + } } module.exports = SettingService; diff --git a/src/lib/services/user-service.ts b/src/lib/services/user-service.ts index a983abb546..8534099d4b 100644 --- a/src/lib/services/user-service.ts +++ b/src/lib/services/user-service.ts @@ -22,7 +22,7 @@ import { IUserStore } from '../types/stores/user-store'; import { RoleName } from '../types/model'; import SettingService from './setting-service'; import { SimpleAuthSettings } from '../server-impl'; -import { simpleAuthKey } from '../types/settings/simple-auth-settings'; +import { simpleAuthSettingsKey } from '../types/settings/simple-auth-settings'; import DisabledError from '../error/disabled-error'; import PasswordMismatch from '../error/password-mismatch'; import BadDataError from '../error/bad-data-error'; @@ -276,7 +276,7 @@ class UserService { async loginUser(usernameOrEmail: string, password: string): Promise { const settings = await this.settingService.get( - simpleAuthKey, + simpleAuthSettingsKey, ); if (settings?.disabled) { diff --git a/src/lib/types/settings/frontend-settings.ts b/src/lib/types/settings/frontend-settings.ts new file mode 100644 index 0000000000..909797fac0 --- /dev/null +++ b/src/lib/types/settings/frontend-settings.ts @@ -0,0 +1,5 @@ +import { IUnleashConfig } from '../option'; + +export const frontendSettingsKey = 'unleash.frontend'; + +export type FrontendSettings = Pick; diff --git a/src/lib/types/settings/simple-auth-settings.ts b/src/lib/types/settings/simple-auth-settings.ts index eb63658910..b343e7288b 100644 --- a/src/lib/types/settings/simple-auth-settings.ts +++ b/src/lib/types/settings/simple-auth-settings.ts @@ -1,4 +1,5 @@ -export const simpleAuthKey = 'unleash.auth.simple'; +export const simpleAuthSettingsKey = 'unleash.auth.simple'; + export interface SimpleAuthSettings { disabled: boolean; } diff --git a/src/lib/util/parseEnvVar.test.ts b/src/lib/util/parseEnvVar.test.ts index a147e6cca9..c70df3551e 100644 --- a/src/lib/util/parseEnvVar.test.ts +++ b/src/lib/util/parseEnvVar.test.ts @@ -29,9 +29,11 @@ test('parseEnvVarBoolean', () => { }); test('parseEnvVarStringList', () => { + expect(parseEnvVarStrings(undefined, [])).toEqual([]); + expect(parseEnvVarStrings(undefined, ['a'])).toEqual(['a']); + expect(parseEnvVarStrings('', ['a'])).toEqual([]); expect(parseEnvVarStrings('', [])).toEqual([]); expect(parseEnvVarStrings(' ', [])).toEqual([]); - expect(parseEnvVarStrings('', ['*'])).toEqual(['*']); expect(parseEnvVarStrings('a', ['*'])).toEqual(['a']); expect(parseEnvVarStrings('a,b,c', [])).toEqual(['a', 'b', 'c']); expect(parseEnvVarStrings('a,b,c', [])).toEqual(['a', 'b', 'c']); diff --git a/src/lib/util/parseEnvVar.ts b/src/lib/util/parseEnvVar.ts index a52ada078e..7bc5d1b50e 100644 --- a/src/lib/util/parseEnvVar.ts +++ b/src/lib/util/parseEnvVar.ts @@ -20,10 +20,10 @@ export function parseEnvVarBoolean( } export function parseEnvVarStrings( - envVar: string, + envVar: string | undefined, defaultVal: string[], ): string[] { - if (envVar) { + if (typeof envVar === 'string') { return envVar .split(',') .map((item) => item.trim()) diff --git a/src/lib/util/validateOrigin.test.ts b/src/lib/util/validateOrigin.test.ts new file mode 100644 index 0000000000..fefb4eba60 --- /dev/null +++ b/src/lib/util/validateOrigin.test.ts @@ -0,0 +1,24 @@ +import { validateOrigin } from './validateOrigin'; + +test('validateOrigin', () => { + expect(validateOrigin(undefined)).toEqual(false); + expect(validateOrigin('')).toEqual(false); + expect(validateOrigin(' ')).toEqual(false); + expect(validateOrigin('a')).toEqual(false); + expect(validateOrigin('**')).toEqual(false); + expect(validateOrigin('localhost')).toEqual(false); + expect(validateOrigin('localhost:8080')).toEqual(false); + expect(validateOrigin('//localhost:8080')).toEqual(false); + expect(validateOrigin('http://localhost/')).toEqual(false); + expect(validateOrigin('http://localhost/a')).toEqual(false); + expect(validateOrigin('https://example.com/a')).toEqual(false); + expect(validateOrigin('https://example.com ')).toEqual(false); + expect(validateOrigin('https://*.example.com ')).toEqual(false); + expect(validateOrigin('*.example.com')).toEqual(false); + + expect(validateOrigin('*')).toEqual(true); + expect(validateOrigin('http://localhost')).toEqual(true); + expect(validateOrigin('http://localhost:8080')).toEqual(true); + expect(validateOrigin('https://example.com')).toEqual(true); + expect(validateOrigin('https://example.com:8080')).toEqual(true); +}); diff --git a/src/lib/util/validateOrigin.ts b/src/lib/util/validateOrigin.ts new file mode 100644 index 0000000000..e9ad69705f --- /dev/null +++ b/src/lib/util/validateOrigin.ts @@ -0,0 +1,24 @@ +export const validateOrigin = (origin: string): boolean => { + if (origin === '*') { + return true; + } + + if (origin?.includes('*')) { + return false; + } + + try { + const parsed = new URL(origin); + return parsed.origin && parsed.origin === origin; + } catch { + return false; + } +}; + +export const validateOrigins = (origins: string[]): string | undefined => { + for (const origin of origins) { + if (!validateOrigin(origin)) { + return `Invalid origin: ${origin}`; + } + } +}; diff --git a/src/test/e2e/api/admin/config.e2e.test.ts b/src/test/e2e/api/admin/config.e2e.test.ts index b4812fa2d5..4bf36e8812 100644 --- a/src/test/e2e/api/admin/config.e2e.test.ts +++ b/src/test/e2e/api/admin/config.e2e.test.ts @@ -1,10 +1,11 @@ import dbInit, { ITestDb } from '../../helpers/database-init'; -import { setupApp } from '../../helpers/test-helper'; +import { IUnleashTest, setupApp } from '../../helpers/test-helper'; import getLogger from '../../../fixtures/no-logger'; -import { simpleAuthKey } from '../../../../lib/types/settings/simple-auth-settings'; +import { simpleAuthSettingsKey } from '../../../../lib/types/settings/simple-auth-settings'; +import { randomId } from '../../../../lib/util/random-id'; let db: ITestDb; -let app; +let app: IUnleashTest; beforeAll(async () => { db = await dbInit('config_api_serial', getLogger); @@ -16,24 +17,71 @@ afterAll(async () => { await db.destroy(); }); +beforeEach(async () => { + await app.services.settingService.deleteAll(); +}); + test('gets ui config fields', async () => { const { body } = await app.request .get('/api/admin/ui-config') .expect('Content-Type', /json/) .expect(200); - expect(body.unleashUrl).toBe('http://localhost:4242'); expect(body.version).toBeDefined(); expect(body.emailEnabled).toBe(false); }); test('gets ui config with disablePasswordAuth', async () => { - await db.stores.settingStore.insert(simpleAuthKey, { disabled: true }); - + await db.stores.settingStore.insert(simpleAuthSettingsKey, { + disabled: true, + }); const { body } = await app.request .get('/api/admin/ui-config') .expect('Content-Type', /json/) .expect(200); - expect(body.disablePasswordAuth).toBe(true); }); + +test('gets ui config with frontendSettings', async () => { + const frontendApiOrigins = ['https://example.net']; + await app.services.settingService.setFrontendSettings( + { frontendApiOrigins }, + randomId(), + ); + await app.request + .get('/api/admin/ui-config') + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => + expect(res.body.frontendApiOrigins).toEqual(frontendApiOrigins), + ); +}); + +test('sets ui config with frontendSettings', async () => { + const frontendApiOrigins = ['https://example.org']; + await app.request + .get('/api/admin/ui-config') + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => expect(res.body.frontendApiOrigins).toEqual(['*'])); + await app.request + .post('/api/admin/ui-config') + .send({ frontendSettings: { frontendApiOrigins: [] } }) + .expect(204); + await app.request + .get('/api/admin/ui-config') + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => expect(res.body.frontendApiOrigins).toEqual([])); + await app.request + .post('/api/admin/ui-config') + .send({ frontendSettings: { frontendApiOrigins } }) + .expect(204); + await app.request + .get('/api/admin/ui-config') + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => + expect(res.body.frontendApiOrigins).toEqual(frontendApiOrigins), + ); +}); diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index ae38379ffc..0f250958cb 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -2498,6 +2498,28 @@ Object { }, "type": "array", }, + "setUiConfigSchema": Object { + "additionalProperties": false, + "properties": Object { + "frontendSettings": Object { + "additionalProperties": false, + "properties": Object { + "frontendApiOrigins": Object { + "additionalProperties": false, + "items": Object { + "type": "string", + }, + "type": "array", + }, + }, + "required": Array [ + "frontendApiOrigins", + ], + "type": "object", + }, + }, + "type": "object", + }, "sortOrderSchema": Object { "additionalProperties": Object { "type": "number", @@ -2829,6 +2851,12 @@ Object { }, "type": "object", }, + "frontendApiOrigins": Object { + "items": Object { + "type": "string", + }, + "type": "array", + }, "links": Object { "items": Object { "type": "object", @@ -6172,7 +6200,7 @@ If the provided project does not exist, the list of events will be empty.", }, "/api/admin/ui-config": Object { "get": Object { - "operationId": "getUIConfig", + "operationId": "getUiConfig", "responses": Object { "200": Object { "content": Object { @@ -6189,6 +6217,28 @@ If the provided project does not exist, the list of events will be empty.", "Admin UI", ], }, + "post": Object { + "operationId": "setUiConfig", + "requestBody": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/setUiConfigSchema", + }, + }, + }, + "description": "setUiConfigSchema", + "required": true, + }, + "responses": Object { + "200": Object { + "description": "This response has no body.", + }, + }, + "tags": Array [ + "Admin UI", + ], + }, }, "/api/admin/user": Object { "get": Object { diff --git a/src/test/e2e/services/user-service.e2e.test.ts b/src/test/e2e/services/user-service.e2e.test.ts index 04f56966b9..62ae96b4e9 100644 --- a/src/test/e2e/services/user-service.e2e.test.ts +++ b/src/test/e2e/services/user-service.e2e.test.ts @@ -11,9 +11,10 @@ import NotFoundError from '../../../lib/error/notfound-error'; import { IRole } from '../../../lib/types/stores/access-store'; import { RoleName } from '../../../lib/types/model'; import SettingService from '../../../lib/services/setting-service'; -import { simpleAuthKey } from '../../../lib/types/settings/simple-auth-settings'; +import { simpleAuthSettingsKey } from '../../../lib/types/settings/simple-auth-settings'; import { addDays, minutesToMilliseconds } from 'date-fns'; import { GroupService } from '../../../lib/services/group-service'; +import { randomId } from '../../../lib/util/random-id'; let db; let stores; @@ -22,6 +23,7 @@ let userStore: UserStore; let adminRole: IRole; let viewerRole: IRole; let sessionService: SessionService; +let settingService: SettingService; beforeAll(async () => { db = await dbInit('user_service_serial', getLogger); @@ -32,7 +34,7 @@ beforeAll(async () => { const resetTokenService = new ResetTokenService(stores, config); const emailService = new EmailService(undefined, config.getLogger); sessionService = new SessionService(stores, config); - const settingService = new SettingService(stores, config); + settingService = new SettingService(stores, config); userService = new UserService(stores, config, { accessService, @@ -101,7 +103,11 @@ test('should create user with password', async () => { }); test('should not login user if simple auth is disabled', async () => { - await db.stores.settingStore.insert(simpleAuthKey, { disabled: true }); + await settingService.insert( + simpleAuthSettingsKey, + { disabled: true }, + randomId(), + ); await userService.createUser({ username: 'test_no_pass', From 42ee35a2b8df378d94515d73a99bb847a9d582cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 26 Aug 2022 10:21:05 +0200 Subject: [PATCH 7/9] fix: upgrade unleash-frontend to v4.14.8 --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 3cbcb51a78..92935245fa 100644 --- a/package.json +++ b/package.json @@ -125,7 +125,7 @@ "ts-toolbelt": "^9.6.0", "type-is": "^1.6.18", "unleash-client": "3.15.0", - "unleash-frontend": "4.15.0-beta.1", + "unleash-frontend": "4.14.8", "uuid": "^8.3.2" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index 58f41979aa..2c72e4e1bb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7243,10 +7243,10 @@ unleash-client@3.15.0: murmurhash3js "^3.0.1" semver "^7.3.5" -unleash-frontend@4.15.0-beta.1: - version "4.15.0-beta.1" - resolved "https://registry.yarnpkg.com/unleash-frontend/-/unleash-frontend-4.15.0-beta.1.tgz#c98255af5408c7cce3aa5f3a38fe2c634813a93b" - integrity sha512-6kHYetlytLVibTfi+QAweve7YWFmXdWzl6aDcYg2XMYj2Ago0gMHwTqtkG7JfZonG1vU5dkfk/KwBKTFKLJn/g== +unleash-frontend@4.14.8: + version "4.14.8" + resolved "https://registry.yarnpkg.com/unleash-frontend/-/unleash-frontend-4.14.8.tgz#c31ded48d8f6de859bde39833fc78ad8bd74c770" + integrity sha512-CcqyFhIZyb8qCHe6iX3saHTdIfN0TzAES2PaSWDCRPt17L9KcV1t+fns1nFvCIzH/XmM416uQC4HgKyoLtR9tw== unpipe@1.0.0, unpipe@~1.0.0: version "1.0.0" From 9e23c4e969bbf35ac21d752121996c6481e7f285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 26 Aug 2022 10:27:30 +0200 Subject: [PATCH 8/9] fix: website dependencies --- website/package.json | 5 ++++- website/yarn.lock | 36 ++++++++++-------------------------- 2 files changed, 14 insertions(+), 27 deletions(-) diff --git a/website/package.json b/website/package.json index d57b1b2da3..a24471b526 100644 --- a/website/package.json +++ b/website/package.json @@ -36,13 +36,16 @@ "url-loader": "4.1.1" }, "resolutions": { + "async": "^3.2.4", "trim": "^1.0.0", "glob-parent": "^6.0.0", "browserslist": "^4.16.5", "set-value": "^4.0.1", "immer": "^9.0.6", "ansi-regex": "^5.0.1", - "nth-check": "^2.0.1" + "nth-check": "^2.0.1", + "shelljs": "^0.8.5", + "trim-newlines": "^3.0.1" }, "browserslist": { "production": [ diff --git a/website/yarn.lock b/website/yarn.lock index 1723cb1610..099ce47367 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -4583,17 +4583,10 @@ async-each@^1.0.1: resolved "https://registry.yarnpkg.com/async-each/-/async-each-1.0.3.tgz#b727dbf87d7651602f06f4d4ac387f47d91b0cbf" integrity sha512-z/WhQ5FPySLdvREByI2vZiTWwCnF0moMJ1hK9YQwDTHKh6I7/uSckMetoRGb5UBZPC1z0jlw+n/XCgjeH7y1AQ== -async@2.6.3: - version "2.6.3" - resolved "https://registry.yarnpkg.com/async/-/async-2.6.3.tgz#d72625e2344a3656e3a3ad4fa749fa83299d82ff" - integrity sha512-zflvls11DCy+dQWzTW2dzuilv8Z5X/pjfmZOWba6TNIVDm+2UDaJmXSOXlasHKfNBs8oo3M0aT50fDEWfKZjXg== - dependencies: - lodash "^4.17.14" - -async@3.2.1: - version "3.2.1" - resolved "https://registry.yarnpkg.com/async/-/async-3.2.1.tgz#d3274ec66d107a47476a4c49136aacdb00665fc8" - integrity sha512-XdD5lRO/87udXCMC9meWdYiR+Nq6ZjUfXidViUZGu2F1MO4T3XwZ1et0hb2++BgLfhyJwy44BGB/yx80ABx8hg== +async@2.6.3, async@3.2.1, async@^3.2.4: + version "3.2.4" + resolved "https://registry.yarnpkg.com/async/-/async-3.2.4.tgz#2d22e00f8cddeb5fde5dd33522b56d1cf569a81c" + integrity sha512-iAB+JbDEGXhyIUavoDl9WP/Jj106Kz9DEn1DPgYw5ruDn0e3Wgi3sKFm55sASdGBNOQB8F59d9qQ7deqrHA8wQ== asynckit@^0.4.0: version "0.4.0" @@ -9200,7 +9193,7 @@ lodash.uniq@4.5.0, lodash.uniq@^4.5.0: resolved "https://registry.yarnpkg.com/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773" integrity sha512-xfBaXQd9ryd9dlSDvnvI0lvxfLJlYAZzXomUYzLKtUeOQvOP5piqAWuGtrhWeqaXK9hhoM/iyJc5AV+XfsX3HQ== -lodash@4.17.21, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.4: +lodash@4.17.21, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.4: version "4.17.21" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c" integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg== @@ -12649,16 +12642,7 @@ shell-quote@^1.7.3: resolved "https://registry.yarnpkg.com/shell-quote/-/shell-quote-1.7.3.tgz#aa40edac170445b9a431e17bb62c0b881b9c4123" integrity sha512-Vpfqwm4EnqGdlsBFNmHhxhElJYrdfcxPThu+ryKS5J8L/fhAwLazFZtq+S+TWZ9ANj2piSQLGj6NQg+lKPmxrw== -shelljs@0.8.4: - version "0.8.4" - resolved "https://registry.yarnpkg.com/shelljs/-/shelljs-0.8.4.tgz#de7684feeb767f8716b326078a8a00875890e3c2" - integrity sha512-7gk3UZ9kOfPLIAbslLzyWeGiEqx9e3rxwZM0KE6EL8GlGwjym9Mrlx5/p33bWTu9YG6vcS4MBxYZDHYr5lr8BQ== - dependencies: - glob "^7.0.0" - interpret "^1.0.0" - rechoir "^0.6.2" - -shelljs@^0.8.5: +shelljs@0.8.4, shelljs@^0.8.5: version "0.8.5" resolved "https://registry.yarnpkg.com/shelljs/-/shelljs-0.8.5.tgz#de055408d8361bed66c669d2f000538ced8ee20c" integrity sha512-TiwcRcrkhHvbrZbnRcFYMLl30Dfov3HKqzp5tO5b4pt6G/SezKcYhmDg15zXVBswHmctSAQKznqNW2LO5tTDow== @@ -13549,10 +13533,10 @@ trim-lines@^3.0.0: resolved "https://registry.yarnpkg.com/trim-lines/-/trim-lines-3.0.1.tgz#d802e332a07df861c48802c04321017b1bd87338" integrity sha512-kRj8B+YHZCc9kQYdWfJB2/oUl9rA99qbowYYBtr4ui4mZyAQ2JpvVBd/6U2YloATfqBhBTSMhTpgBHtU0Mf3Rg== -trim-newlines@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/trim-newlines/-/trim-newlines-1.0.0.tgz#5887966bb582a4503a41eb524f7d35011815a613" - integrity sha512-Nm4cF79FhSTzrLKGDMi3I4utBtFv8qKy4sq1enftf2gMdpqI8oVQTAfySkTz5r49giVzDj88SVZXP4CeYQwjaw== +trim-newlines@^1.0.0, trim-newlines@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/trim-newlines/-/trim-newlines-3.0.1.tgz#260a5d962d8b752425b32f3a7db0dcacd176c144" + integrity sha512-c1PTsA3tYrIsLGkJkzHF+w9F2EyxfXGo4UyJc4pFL++FMjnq0HJS69T3M7d//gKrFKwy429bouPescbjecU+Zw== trim-trailing-lines@^1.0.0: version "1.1.4" From 2a2ed3754188dcc80f8a0b188f6b2da5657bc679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 26 Aug 2022 10:43:47 +0200 Subject: [PATCH 9/9] 4.15.0-beta.4 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 92935245fa..68e06c631b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "unleash-server", "description": "Unleash is an enterprise ready feature toggles service. It provides different strategies for handling feature toggles.", - "version": "4.15.0-beta.2", + "version": "4.15.0-beta.4", "keywords": [ "unleash", "feature toggle",