From e69935be9982a98dfb43983e742b9a44dc05f438 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Fri, 17 Mar 2023 11:23:32 -0400 Subject: [PATCH] Remove username column (#5052) --- plugins/oidc/server/auth/oidc.ts | 1 - server/commands/accountProvisioner.test.ts | 13 ------------- server/commands/accountProvisioner.ts | 3 --- server/commands/userProvisioner.test.ts | 12 ------------ server/commands/userProvisioner.ts | 5 ----- .../20230317144617-remove-user-username.js | 14 ++++++++++++++ server/models/User.ts | 6 ------ server/presenters/user.test.ts | 2 -- server/test/factories.ts | 1 - server/test/support.ts | 1 - 10 files changed, 14 insertions(+), 44 deletions(-) create mode 100644 server/migrations/20230317144617-remove-user-username.js diff --git a/plugins/oidc/server/auth/oidc.ts b/plugins/oidc/server/auth/oidc.ts index 073a8cf7c..39cb6e9dd 100644 --- a/plugins/oidc/server/auth/oidc.ts +++ b/plugins/oidc/server/auth/oidc.ts @@ -115,7 +115,6 @@ if ( name: profile.name || username || profile.username, email: profile.email, avatarUrl: profile.picture, - username, }, authenticationProvider: { name: providerName, diff --git a/server/commands/accountProvisioner.test.ts b/server/commands/accountProvisioner.test.ts index a70b34704..caef7c3ae 100644 --- a/server/commands/accountProvisioner.test.ts +++ b/server/commands/accountProvisioner.test.ts @@ -25,7 +25,6 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", - username: "jtester", }, team: { name: "New workspace", @@ -49,7 +48,6 @@ describe("accountProvisioner", () => { expect(auth.scopes[0]).toEqual("read"); expect(team.name).toEqual("New workspace"); expect(user.email).toEqual("jenny@example-company.com"); - expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(isNewTeam).toEqual(true); expect(spy).toHaveBeenCalled(); @@ -70,14 +68,12 @@ describe("accountProvisioner", () => { const authentications = await existing.$get("authentications"); const authentication = authentications[0]; const newEmail = "test@example-company.com"; - const newUsername = "tname"; const { user, isNewUser, isNewTeam } = await accountProvisioner({ ip, user: { name: existing.name, email: newEmail, avatarUrl: existing.avatarUrl, - username: newUsername, }, team: { name: existingTeam.name, @@ -99,7 +95,6 @@ describe("accountProvisioner", () => { expect(auth?.scopes.length).toEqual(1); expect(auth?.scopes[0]).toEqual("read"); expect(user.email).toEqual(newEmail); - expect(user.username).toEqual(newUsername); expect(isNewTeam).toEqual(false); expect(isNewUser).toEqual(false); expect(spy).not.toHaveBeenCalled(); @@ -211,7 +206,6 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", - username: "jtester", }, team: { name: existingTeam.name, @@ -254,7 +248,6 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", - username: "jtester", }, team: { name: team.name, @@ -277,7 +270,6 @@ describe("accountProvisioner", () => { expect(auth.scopes.length).toEqual(1); expect(auth.scopes[0]).toEqual("read"); expect(user.email).toEqual("jenny@example-company.com"); - expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(spy).toHaveBeenCalled(); // should provision welcome collection @@ -300,7 +292,6 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", - username: "jtester", }, team: { name: team.name, @@ -323,7 +314,6 @@ describe("accountProvisioner", () => { expect(auth.scopes.length).toEqual(1); expect(auth.scopes[0]).toEqual("read"); expect(user.email).toEqual("jenny@example-company.com"); - expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(spy).toHaveBeenCalled(); // should provision welcome collection @@ -350,7 +340,6 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", - username: "jtester", }, team: { teamId: team.id, @@ -385,7 +374,6 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example-company.com", avatarUrl: "https://example.com/avatar.png", - username: "jtester", }, team: { teamId: team.id, @@ -406,7 +394,6 @@ describe("accountProvisioner", () => { }); expect(user.teamId).toEqual(team.id); - expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); const providers = await team.$get("authenticationProviders"); diff --git a/server/commands/accountProvisioner.ts b/server/commands/accountProvisioner.ts index 118fe34c4..6704da8c8 100644 --- a/server/commands/accountProvisioner.ts +++ b/server/commands/accountProvisioner.ts @@ -21,8 +21,6 @@ type Props = { email: string; /** The public url of an image representing the user */ avatarUrl?: string | null; - /** The username of the user */ - username?: string; }; /** Details of the team the user is logging into */ team: { @@ -129,7 +127,6 @@ async function accountProvisioner({ const result = await userProvisioner({ name: userParams.name, email: userParams.email, - username: userParams.username, isAdmin: isNewTeam || undefined, avatarUrl: userParams.avatarUrl, teamId: team.id, diff --git a/server/commands/userProvisioner.test.ts b/server/commands/userProvisioner.test.ts index f7434e7b5..16702c46a 100644 --- a/server/commands/userProvisioner.test.ts +++ b/server/commands/userProvisioner.test.ts @@ -14,11 +14,9 @@ describe("userProvisioner", () => { const authentications = await existing.$get("authentications"); const existingAuth = authentications[0]; const newEmail = "test@example.com"; - const newUsername = "tname"; const result = await userProvisioner({ name: existing.name, email: newEmail, - username: newUsername, avatarUrl: existing.avatarUrl, teamId: existing.teamId, ip, @@ -35,7 +33,6 @@ describe("userProvisioner", () => { expect(authentication?.scopes.length).toEqual(1); expect(authentication?.scopes[0]).toEqual("read"); expect(user.email).toEqual(newEmail); - expect(user.username).toEqual(newUsername); expect(isNewUser).toEqual(false); }); @@ -54,7 +51,6 @@ describe("userProvisioner", () => { const result = await userProvisioner({ name: existing.name, email, - username: "new-username", avatarUrl: existing.avatarUrl, teamId: existing.teamId, ip, @@ -90,7 +86,6 @@ describe("userProvisioner", () => { const result = await userProvisioner({ name: existing.name, email, - username: "new-username", avatarUrl: existing.avatarUrl, teamId: existing.teamId, ip, @@ -172,7 +167,6 @@ describe("userProvisioner", () => { const result = await userProvisioner({ name: "Test Name", email: "test@example.com", - username: "tname", teamId: team.id, ip, authentication: { @@ -188,7 +182,6 @@ describe("userProvisioner", () => { expect(authentication?.scopes.length).toEqual(1); expect(authentication?.scopes[0]).toEqual("read"); expect(user.email).toEqual("test@example.com"); - expect(user.username).toEqual("tname"); expect(user.isAdmin).toEqual(false); expect(user.isViewer).toEqual(false); expect(isNewUser).toEqual(true); @@ -203,7 +196,6 @@ describe("userProvisioner", () => { const result = await userProvisioner({ name: "Test Name", email: "test@example.com", - username: "tname", teamId: team.id, isAdmin: true, ip, @@ -227,7 +219,6 @@ describe("userProvisioner", () => { const result = await userProvisioner({ name: "Test Name", email: "test@example.com", - username: "tname", teamId: team.id, ip, authentication: { @@ -238,13 +229,11 @@ describe("userProvisioner", () => { }, }); const { user: tname } = result; - expect(tname.username).toEqual("tname"); expect(tname.isAdmin).toEqual(false); expect(tname.isViewer).toEqual(true); const tname2Result = await userProvisioner({ name: "Test2 Name", email: "tes2@example.com", - username: "tname2", teamId: team.id, isAdmin: false, ip, @@ -256,7 +245,6 @@ describe("userProvisioner", () => { }, }); const { user: tname2 } = tname2Result; - expect(tname2.username).toEqual("tname2"); expect(tname2.isAdmin).toEqual(false); expect(tname2.isViewer).toEqual(true); }); diff --git a/server/commands/userProvisioner.ts b/server/commands/userProvisioner.ts index cb3382eca..79fa92d49 100644 --- a/server/commands/userProvisioner.ts +++ b/server/commands/userProvisioner.ts @@ -18,8 +18,6 @@ type Props = { name: string; /** The email address of the user */ email: string; - /** The username of the user */ - username?: string; /** Provision the new user as an administrator */ isAdmin?: boolean; /** The public url of an image representing the user */ @@ -50,7 +48,6 @@ type Props = { export default async function userProvisioner({ name, email, - username, isAdmin, avatarUrl, teamId, @@ -92,7 +89,6 @@ export default async function userProvisioner({ if (user) { await user.update({ email, - username, }); await auth.update(rest); return { @@ -231,7 +227,6 @@ export default async function userProvisioner({ { name, email, - username, isAdmin: typeof isAdmin === "boolean" && isAdmin, isViewer: isAdmin === true ? false : defaultUserRole === "viewer", teamId, diff --git a/server/migrations/20230317144617-remove-user-username.js b/server/migrations/20230317144617-remove-user-username.js new file mode 100644 index 000000000..241c13680 --- /dev/null +++ b/server/migrations/20230317144617-remove-user-username.js @@ -0,0 +1,14 @@ +"use strict"; + +module.exports = { + async up(queryInterface) { + return queryInterface.removeColumn("users", "username"); + }, + + async down(queryInterface, Sequelize) { + return queryInterface.addColumn("users", "username", { + type: Sequelize.STRING, + allowNull: true, + }); + }, +}; diff --git a/server/models/User.ts b/server/models/User.ts index 654856b82..19ca60f2f 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -110,11 +110,6 @@ class User extends ParanoidModel { @Column email: string | null; - @NotContainsUrl - @Length({ max: 255, msg: "User username must be 255 characters or less" }) - @Column - username: string | null; - @NotContainsUrl @Length({ max: 255, msg: "User name must be 255 characters or less" }) @Column @@ -562,7 +557,6 @@ class User extends ParanoidModel { model.email = null; model.name = "Unknown"; model.avatarUrl = null; - model.username = null; model.lastActiveIp = null; model.lastSignedInIp = null; diff --git a/server/presenters/user.test.ts b/server/presenters/user.test.ts index 6ab827070..3290a7efe 100644 --- a/server/presenters/user.test.ts +++ b/server/presenters/user.test.ts @@ -6,7 +6,6 @@ it("presents a user", async () => { User.build({ id: "123", name: "Test User", - username: "testuser", }) ); expect(user).toMatchSnapshot(); @@ -17,7 +16,6 @@ it("presents a user without slack data", async () => { User.build({ id: "123", name: "Test User", - username: "testuser", }) ); expect(user).toMatchSnapshot(); diff --git a/server/test/factories.ts b/server/test/factories.ts index 8f9061a82..f1779bed4 100644 --- a/server/test/factories.ts +++ b/server/test/factories.ts @@ -182,7 +182,6 @@ export async function buildUser(overrides: Partial = {}) { { email: `user${count}@example.com`, name: `User ${count}`, - username: `user${count}`, createdAt: new Date("2018-01-01T00:00:00.000Z"), updatedAt: new Date("2018-01-02T00:00:00.000Z"), lastActiveAt: new Date("2018-01-03T00:00:00.000Z"), diff --git a/server/test/support.ts b/server/test/support.ts index 17ad9c7b1..ff43e6831 100644 --- a/server/test/support.ts +++ b/server/test/support.ts @@ -28,7 +28,6 @@ export const seed = async () => { const admin = await User.create( { email: "admin@example.com", - username: "admin", name: "Admin User", teamId: team.id, isAdmin: true,