From 305de71e8b203135be82d4835c54ac13955fae68 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Tue, 21 Jun 2022 11:29:43 +0300 Subject: [PATCH] chore: Block all email providers from being added as team domains (#3678) --- app/scenes/Settings/Security.tsx | 4 +-- package.json | 1 + server/commands/accountProvisioner.test.ts | 26 +++++++++---------- server/commands/userCreator.test.ts | 6 ++--- server/models/TeamDomain.ts | 6 +++++ server/routes/api/team.test.ts | 30 +++++++++++++++------- server/typings/index.d.ts | 5 ++++ yarn.lock | 5 ++++ 8 files changed, 56 insertions(+), 27 deletions(-) diff --git a/app/scenes/Settings/Security.tsx b/app/scenes/Settings/Security.tsx index 2415f7247..f978340d5 100644 --- a/app/scenes/Settings/Security.tsx +++ b/app/scenes/Settings/Security.tsx @@ -59,12 +59,12 @@ function Security() { setData(newData); await auth.updateTeam(newData); showSuccessMessage(); + setDomainsChanged(false); } catch (err) { + setDomainsChanged(true); showToast(err.message, { type: "error", }); - } finally { - setDomainsChanged(false); } }, [auth, showSuccessMessage, showToast] diff --git a/package.json b/package.json index 80a712699..8472c543e 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "datadog-metrics": "^0.9.3", "date-fns": "^2.25.0", "dotenv": "^4.0.0", + "email-providers": "^1.13.1", "emoji-regex": "^10.0.0", "es6-error": "^4.1.1", "exports-loader": "^0.6.4", diff --git a/server/commands/accountProvisioner.test.ts b/server/commands/accountProvisioner.test.ts index afd8f6aed..7a65280de 100644 --- a/server/commands/accountProvisioner.test.ts +++ b/server/commands/accountProvisioner.test.ts @@ -22,7 +22,7 @@ describe("accountProvisioner", () => { ip, user: { name: "Jenny Tester", - email: "jenny@example.com", + email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", username: "jtester", }, @@ -33,7 +33,7 @@ describe("accountProvisioner", () => { }, authenticationProvider: { name: "google", - providerId: "example.com", + providerId: "example-company.com", }, authentication: { providerId: "123456789", @@ -47,7 +47,7 @@ describe("accountProvisioner", () => { expect(auth.scopes.length).toEqual(1); expect(auth.scopes[0]).toEqual("read"); expect(team.name).toEqual("New team"); - expect(user.email).toEqual("jenny@example.com"); + expect(user.email).toEqual("jenny@example-company.com"); expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(isNewTeam).toEqual(true); @@ -68,7 +68,7 @@ describe("accountProvisioner", () => { }); const authentications = await existing.$get("authentications"); const authentication = authentications[0]; - const newEmail = "test@example.com"; + const newEmail = "test@example-company.com"; const newUsername = "tname"; const { user, isNewUser, isNewTeam } = await accountProvisioner({ ip, @@ -170,7 +170,7 @@ describe("accountProvisioner", () => { ip, user: { name: "Jenny Tester", - email: "jenny@example.com", + email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", username: "jtester", }, @@ -203,7 +203,7 @@ describe("accountProvisioner", () => { const authenticationProvider = authenticationProviders[0]; await TeamDomain.create({ teamId: team.id, - name: "example.com", + name: "example-company.com", createdById: admin.id, }); @@ -211,7 +211,7 @@ describe("accountProvisioner", () => { ip, user: { name: "Jenny Tester", - email: "jenny@example.com", + email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", username: "jtester", }, @@ -235,7 +235,7 @@ describe("accountProvisioner", () => { expect(auth.accessToken).toEqual("123"); expect(auth.scopes.length).toEqual(1); expect(auth.scopes[0]).toEqual("read"); - expect(user.email).toEqual("jenny@example.com"); + expect(user.email).toEqual("jenny@example-company.com"); expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(spy).toHaveBeenCalled(); @@ -255,7 +255,7 @@ describe("accountProvisioner", () => { ip, user: { name: "Jenny Tester", - email: "jenny@example.com", + email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", username: "jtester", }, @@ -279,7 +279,7 @@ describe("accountProvisioner", () => { expect(auth.accessToken).toEqual("123"); expect(auth.scopes.length).toEqual(1); expect(auth.scopes[0]).toEqual("read"); - expect(user.email).toEqual("jenny@example.com"); + expect(user.email).toEqual("jenny@example-company.com"); expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(spy).toHaveBeenCalled(); @@ -301,7 +301,7 @@ describe("accountProvisioner", () => { ip, user: { name: "Jenny Tester", - email: "jenny@example.com", + email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", username: "jtester", }, @@ -312,7 +312,7 @@ describe("accountProvisioner", () => { }, authenticationProvider: { name: "google", - providerId: "example.com", + providerId: "example-company.com", }, authentication: { providerId: "123456789", @@ -337,7 +337,7 @@ describe("accountProvisioner", () => { ip, user: { name: "Jenny Tester", - email: "jenny@example.com", + email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", username: "jtester", }, diff --git a/server/commands/userCreator.test.ts b/server/commands/userCreator.test.ts index 5555ae7e6..d22d7bf43 100644 --- a/server/commands/userCreator.test.ts +++ b/server/commands/userCreator.test.ts @@ -245,7 +245,7 @@ describe("userCreator", () => { const { admin, team } = await seed(); await TeamDomain.create({ teamId: team.id, - name: "example.com", + name: "example-company.com", createdById: admin.id, }); @@ -253,7 +253,7 @@ describe("userCreator", () => { const authenticationProvider = authenticationProviders[0]; const result = await userCreator({ name: "Test Name", - email: "user@example.com", + email: "user@example-company.com", teamId: team.id, ip, authentication: { @@ -267,7 +267,7 @@ describe("userCreator", () => { expect(authentication.accessToken).toEqual("123"); expect(authentication.scopes.length).toEqual(1); expect(authentication.scopes[0]).toEqual("read"); - expect(user.email).toEqual("user@example.com"); + expect(user.email).toEqual("user@example-company.com"); expect(isNewUser).toEqual(true); }); diff --git a/server/models/TeamDomain.ts b/server/models/TeamDomain.ts index f316ca7b8..ed4358fc5 100644 --- a/server/models/TeamDomain.ts +++ b/server/models/TeamDomain.ts @@ -1,9 +1,11 @@ +import emailProviders from "email-providers"; import { Column, Table, BelongsTo, ForeignKey, NotEmpty, + NotIn, } from "sequelize-typescript"; import Team from "./Team"; import User from "./User"; @@ -13,6 +15,10 @@ import Fix from "./decorators/Fix"; @Table({ tableName: "team_domains", modelName: "team_domain" }) @Fix class TeamDomain extends IdModel { + @NotIn({ + args: [emailProviders], + msg: "You chose a restricted domain, please try another.", + }) @NotEmpty @Column name: string; diff --git a/server/routes/api/team.test.ts b/server/routes/api/team.test.ts index f34b58c07..44db902dc 100644 --- a/server/routes/api/team.test.ts +++ b/server/routes/api/team.test.ts @@ -28,19 +28,28 @@ describe("#team.update", () => { const res = await server.post("/api/team.update", { body: { token: admin.getJwtToken(), - allowedDomains: ["example.com", "", "example.org", "", ""], + allowedDomains: [ + "example-company.com", + "", + "example-company.org", + "", + "", + ], }, }); const body = await res.json(); expect(res.status).toEqual(200); - expect(body.data.allowedDomains).toEqual(["example.com", "example.org"]); + expect(body.data.allowedDomains).toEqual([ + "example-company.com", + "example-company.org", + ]); const teamDomains: TeamDomain[] = await TeamDomain.findAll({ where: { teamId: team.id }, }); expect(teamDomains.map((d) => d.name)).toEqual([ - "example.com", - "example.org", + "example-company.com", + "example-company.org", ]); }); @@ -48,7 +57,7 @@ describe("#team.update", () => { const { admin, team } = await seed(); const existingTeamDomain = await TeamDomain.create({ teamId: team.id, - name: "example.com", + name: "example-company.com", createdById: admin.id, }); @@ -74,25 +83,28 @@ describe("#team.update", () => { const { admin, team } = await seed(); const existingTeamDomain = await TeamDomain.create({ teamId: team.id, - name: "example.com", + name: "example-company.com", createdById: admin.id, }); const res = await server.post("/api/team.update", { body: { token: admin.getJwtToken(), - allowedDomains: ["example.org", "example.net"], + allowedDomains: ["example-company.org", "example-company.net"], }, }); const body = await res.json(); expect(res.status).toEqual(200); - expect(body.data.allowedDomains).toEqual(["example.org", "example.net"]); + expect(body.data.allowedDomains).toEqual([ + "example-company.org", + "example-company.net", + ]); const teamDomains: TeamDomain[] = await TeamDomain.findAll({ where: { teamId: team.id }, }); expect(teamDomains.map((d) => d.name).sort()).toEqual( - ["example.org", "example.net"].sort() + ["example-company.org", "example-company.net"].sort() ); expect(await TeamDomain.findByPk(existingTeamDomain.id)).toBeNull(); diff --git a/server/typings/index.d.ts b/server/typings/index.d.ts index d966c5a67..fcc219e3d 100644 --- a/server/typings/index.d.ts +++ b/server/typings/index.d.ts @@ -12,6 +12,11 @@ declare module "oy-vey"; declare module "fetch-test-server"; +declare module "email-providers" { + const list: string[]; + export default list; +} + declare module "@joplin/turndown-plugin-gfm" { import { Plugin } from "turndown"; diff --git a/yarn.lock b/yarn.lock index 8f749400e..c8aa958e0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6128,6 +6128,11 @@ elliptic@^6.5.3: minimalistic-assert "^1.0.1" minimalistic-crypto-utils "^1.0.1" +email-providers@^1.13.1: + version "1.13.1" + resolved "https://registry.yarnpkg.com/email-providers/-/email-providers-1.13.1.tgz#dfaea33a7744035510f0f64ed44098e7077f68c9" + integrity sha512-+BPUngcWMy9piqS33yeOcqJXYhIxet94UbK1B/uDOGfjLav4YlDAf9/RhplRypSDBSKx92STNH0PcwgCJnNATw== + emittery@^0.8.1: version "0.8.1" resolved "https://registry.yarnpkg.com/emittery/-/emittery-0.8.1.tgz#bb23cc86d03b30aa75a7f734819dee2e1ba70860"