From cc591ebc66ab7684cf979c8084dfca7e7bccdacf Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 27 Jul 2025 13:15:21 -0400 Subject: [PATCH] Conversion of `User` to event system (#9741) * Conversion of User to event system * fix * warning * fixes * Skip lastActiveAt in changeset * fix: Skip count in view changeset * refactor: Remove userDestroyer * refactor: Remove userSuspender * refactor: Remove userUnsuspender * tests --- .../Settings/components/GroupDialogs.tsx | 1 - plugins/azure/server/auth/azure.ts | 11 +- plugins/discord/server/auth/discord.ts | 11 +- plugins/google/server/auth/google.ts | 11 +- plugins/oidc/server/auth/oidcRouter.ts | 11 +- plugins/slack/server/auth/slack.ts | 12 +- server/commands/accountProvisioner.test.ts | 87 ++++++------- server/commands/accountProvisioner.ts | 24 ++-- server/commands/subscriptionCreator.ts | 2 +- server/commands/teamCreator.ts | 12 +- server/commands/teamProvisioner.test.ts | 44 +++---- server/commands/teamProvisioner.ts | 18 +-- server/commands/teamUpdater.ts | 5 +- server/commands/userDestroyer.test.ts | 84 ------------ server/commands/userDestroyer.ts | 60 --------- server/commands/userInviter.test.ts | 123 +++++++++--------- server/commands/userInviter.ts | 65 +++++---- server/commands/userProvisioner.test.ts | 54 +++----- server/commands/userProvisioner.ts | 61 ++------- server/commands/userSuspender.test.ts | 67 ---------- server/commands/userSuspender.ts | 60 --------- server/commands/userUnsuspender.test.ts | 39 ------ server/commands/userUnsuspender.ts | 48 ------- server/context.ts | 7 +- server/models/Share.ts | 2 +- server/models/Team.ts | 4 + server/models/User.test.ts | 76 ++++++++++- server/models/User.ts | 54 +++++++- server/models/View.test.ts | 2 +- server/models/View.ts | 2 + server/models/base/Model.ts | 7 + server/models/decorators/Fix.ts | 17 ++- .../models/helpers/NotificationHelper.test.ts | 2 +- .../models/helpers/ProseMirrorHelper.test.ts | 2 +- server/policies/user.ts | 10 +- .../queues/processors/CollectionsProcessor.ts | 3 +- .../queues/processors/UserDemotedProcessor.ts | 2 +- server/routes/api/collections/collections.ts | 3 +- server/routes/api/developer/developer.ts | 7 +- server/routes/api/events/events.test.ts | 2 +- server/routes/api/groups/groups.test.ts | 2 +- .../routes/api/installation/installation.ts | 20 +-- server/routes/api/teams/teams.ts | 37 ++---- .../users/__snapshots__/users.test.ts.snap | 8 +- server/routes/api/users/users.test.ts | 6 +- server/routes/api/users/users.ts | 103 +++++---------- 46 files changed, 472 insertions(+), 816 deletions(-) delete mode 100644 server/commands/userDestroyer.test.ts delete mode 100644 server/commands/userDestroyer.ts delete mode 100644 server/commands/userSuspender.test.ts delete mode 100644 server/commands/userSuspender.ts delete mode 100644 server/commands/userUnsuspender.test.ts delete mode 100644 server/commands/userUnsuspender.ts diff --git a/app/scenes/Settings/components/GroupDialogs.tsx b/app/scenes/Settings/components/GroupDialogs.tsx index 474e551c46..32dd49cce7 100644 --- a/app/scenes/Settings/components/GroupDialogs.tsx +++ b/app/scenes/Settings/components/GroupDialogs.tsx @@ -4,7 +4,6 @@ import { PlusIcon } from "outline-icons"; import * as React from "react"; import { Trans, useTranslation } from "react-i18next"; import { toast } from "sonner"; -import { v4 as uuidv4 } from "uuid"; import Group from "~/models/Group"; import User from "~/models/User"; import Invite from "~/scenes/Invite"; diff --git a/plugins/azure/server/auth/azure.ts b/plugins/azure/server/auth/azure.ts index 5b45a3c7eb..e0059a7cb9 100644 --- a/plugins/azure/server/auth/azure.ts +++ b/plugins/azure/server/auth/azure.ts @@ -19,6 +19,7 @@ import { } from "@server/utils/passport"; import config from "../../plugin.json"; import env from "../env"; +import { createContext } from "@server/context"; const router = new Router(); const scopes: string[] = []; @@ -38,7 +39,7 @@ if (env.AZURE_CLIENT_ID && env.AZURE_CLIENT_SECRET) { scope: scopes, }, async function ( - ctx: Context, + context: Context, accessToken: string, refreshToken: string, params: { expires_in: number; id_token: string }, @@ -94,15 +95,15 @@ if (env.AZURE_CLIENT_ID && env.AZURE_CLIENT_SECRET) { ); } - const team = await getTeamFromContext(ctx); - const client = getClientFromContext(ctx); + const team = await getTeamFromContext(context); + const client = getClientFromContext(context); const domain = parseEmail(email).domain; const subdomain = slugifyDomain(domain); const teamName = organization.displayName; - const result = await accountProvisioner({ - ip: ctx.ip, + const ctx = createContext({ ip: context.ip }); + const result = await accountProvisioner(ctx, { team: { teamId: team?.id, name: teamName, diff --git a/plugins/discord/server/auth/discord.ts b/plugins/discord/server/auth/discord.ts index 868b1065bb..b81fb254e7 100644 --- a/plugins/discord/server/auth/discord.ts +++ b/plugins/discord/server/auth/discord.ts @@ -27,6 +27,7 @@ import { import config from "../../plugin.json"; import env from "../env"; import { DiscordGuildError, DiscordGuildRoleError } from "../errors"; +import { createContext } from "@server/context"; const router = new Router(); @@ -54,7 +55,7 @@ if (env.DISCORD_CLIENT_ID && env.DISCORD_CLIENT_SECRET) { pkce: false, }, async function ( - ctx: Context, + context: Context, accessToken: string, refreshToken: string, params: { expires_in: number }, @@ -66,8 +67,8 @@ if (env.DISCORD_CLIENT_ID && env.DISCORD_CLIENT_SECRET) { ) => void ) { try { - const team = await getTeamFromContext(ctx); - const client = getClientFromContext(ctx); + const team = await getTeamFromContext(context); + const client = getClientFromContext(context); /** Fetch the user's profile */ const profile: RESTGetAPICurrentUserResult = await request( "GET", @@ -180,8 +181,8 @@ if (env.DISCORD_CLIENT_ID && env.DISCORD_CLIENT_SECRET) { // if a team can be inferred, we assume the user is only interested in signing into // that team in particular; otherwise, we will do a best effort at finding their account // or provisioning a new one (within AccountProvisioner) - const result = await accountProvisioner({ - ip: ctx.ip, + const ctx = createContext({ ip: context.ip }); + const result = await accountProvisioner(ctx, { team: { teamId: team?.id, name: teamName, diff --git a/plugins/google/server/auth/google.ts b/plugins/google/server/auth/google.ts index 4b8bbb319b..3152dbfcdb 100644 --- a/plugins/google/server/auth/google.ts +++ b/plugins/google/server/auth/google.ts @@ -21,6 +21,7 @@ import { } from "@server/utils/passport"; import config from "../../plugin.json"; import env from "../env"; +import { createContext } from "@server/context"; const router = new Router(); @@ -51,7 +52,7 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) { scope: scopes, }, async function ( - ctx: Context, + context: Context, accessToken: string, refreshToken: string, params: { expires_in: number }, @@ -65,8 +66,8 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) { try { // "domain" is the Google Workspaces domain const domain = profile._json.hd; - const team = await getTeamFromContext(ctx); - const client = getClientFromContext(ctx); + const team = await getTeamFromContext(context); + const client = getClientFromContext(context); // No profile domain means personal gmail account // No team implies the request came from the apex domain @@ -108,8 +109,8 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) { // if a team can be inferred, we assume the user is only interested in signing into // that team in particular; otherwise, we will do a best effort at finding their account // or provisioning a new one (within AccountProvisioner) - const result = await accountProvisioner({ - ip: ctx.ip, + const ctx = createContext({ ip: context.ip }); + const result = await accountProvisioner(ctx, { team: { teamId: team?.id, name: teamName, diff --git a/plugins/oidc/server/auth/oidcRouter.ts b/plugins/oidc/server/auth/oidcRouter.ts index 3b53f0ae30..3f34dd1728 100644 --- a/plugins/oidc/server/auth/oidcRouter.ts +++ b/plugins/oidc/server/auth/oidcRouter.ts @@ -24,6 +24,7 @@ import { import config from "../../plugin.json"; import env from "../env"; import { OIDCStrategy } from "./OIDCStrategy"; +import { createContext } from "@server/context"; export interface OIDCEndpoints { authorizationURL: string; @@ -65,7 +66,7 @@ export function createOIDCRouter( // Any claim supplied in response to the userinfo request will be // available on the `profile` parameter async function ( - ctx: Context, + context: Context, accessToken: string, refreshToken: string, params: { expires_in: number; id_token: string }, @@ -118,8 +119,8 @@ export function createOIDCRouter( ); } - const team = await getTeamFromContext(ctx); - const client = getClientFromContext(ctx); + const team = await getTeamFromContext(context); + const client = getClientFromContext(context); const { domain } = parseEmail(email); // Only a single OIDC provider is supported – find the existing, if any. @@ -185,8 +186,8 @@ export function createOIDCRouter( avatarUrl = null; } - const result = await accountProvisioner({ - ip: ctx.ip, + const ctx = createContext({ ip: context.ip }); + const result = await accountProvisioner(ctx, { team: { teamId: team?.id, name: env.APP_NAME, diff --git a/plugins/slack/server/auth/slack.ts b/plugins/slack/server/auth/slack.ts index aed9b3c529..92d25cad43 100644 --- a/plugins/slack/server/auth/slack.ts +++ b/plugins/slack/server/auth/slack.ts @@ -29,6 +29,7 @@ import env from "../env"; import * as Slack from "../slack"; import * as T from "./schema"; import { SlackUtils } from "plugins/slack/shared/SlackUtils"; +import { createContext } from "@server/context"; type SlackProfile = Profile & { team: { @@ -68,7 +69,7 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) { scope: scopes, }, async function ( - ctx: Context, + context: Context, accessToken: string, refreshToken: string, params: { expires_in: number }, @@ -80,14 +81,13 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) { ) => void ) { try { - const team = await getTeamFromContext(ctx); - const client = getClientFromContext(ctx); + const team = await getTeamFromContext(context); + const client = getClientFromContext(context); - const { domain } = parseEmail(profile.user.email); - const result = await accountProvisioner({ - ip: ctx.ip, + const ctx = createContext({ ip: context.ip }); + const result = await accountProvisioner(ctx, { team: { teamId: team?.id, name: profile.team.name, diff --git a/server/commands/accountProvisioner.test.ts b/server/commands/accountProvisioner.test.ts index eac8bd16fa..01b1b9e5ab 100644 --- a/server/commands/accountProvisioner.test.ts +++ b/server/commands/accountProvisioner.test.ts @@ -7,36 +7,40 @@ import UserAuthentication from "@server/models/UserAuthentication"; import { buildUser, buildTeam, buildAdmin } from "@server/test/factories"; import { setSelfHosted } from "@server/test/support"; import accountProvisioner from "./accountProvisioner"; +import { createContext } from "@server/context"; describe("accountProvisioner", () => { - const ip = "127.0.0.1"; + const ip = faker.internet.ip(); + const ctx = createContext({ ip }); describe("hosted", () => { it("should create a new user and team", async () => { const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); const email = faker.internet.email().toLowerCase(); - const { user, team, isNewTeam, isNewUser } = await accountProvisioner({ - ip, - user: { - name: "Jenny Tester", - email, - avatarUrl: faker.internet.avatar(), - }, - team: { - name: "New workspace", - avatarUrl: faker.internet.avatar(), - subdomain: faker.internet.domainWord(), - }, - authenticationProvider: { - name: "google", - providerId: faker.internet.domainName(), - }, - authentication: { - providerId: uuidv4(), - accessToken: "123", - scopes: ["read"], - }, - }); + const { user, team, isNewTeam, isNewUser } = await accountProvisioner( + ctx, + { + user: { + name: "Jenny Tester", + email, + avatarUrl: faker.image.avatar(), + }, + team: { + name: "New workspace", + avatarUrl: faker.image.avatar(), + subdomain: faker.internet.domainWord(), + }, + authenticationProvider: { + name: "google", + providerId: faker.internet.domainName(), + }, + authentication: { + providerId: uuidv4(), + accessToken: "123", + scopes: ["read"], + }, + } + ); const authentications = await user.$get("authentications"); const auth = authentications[0]; expect(auth.accessToken).toEqual("123"); @@ -68,8 +72,7 @@ describe("accountProvisioner", () => { const authentications = await existing.$get("authentications"); const authentication = authentications[0]; const newEmail = faker.internet.email().toLowerCase(); - const { user, isNewUser, isNewTeam } = await accountProvisioner({ - ip, + const { user, isNewUser, isNewTeam } = await accountProvisioner(ctx, { user: { name: existing.name, email: newEmail, @@ -117,8 +120,7 @@ describe("accountProvisioner", () => { authentications: [], }); - const { user, isNewUser, isNewTeam } = await accountProvisioner({ - ip, + const { user, isNewUser, isNewTeam } = await accountProvisioner(ctx, { user: { name: userWithoutAuth.name, email, @@ -161,8 +163,7 @@ describe("accountProvisioner", () => { let error; try { - await accountProvisioner({ - ip, + await accountProvisioner(ctx, { user: { name: existing.name, email: existing.email!, @@ -210,8 +211,7 @@ describe("accountProvisioner", () => { }); const authentications = await existing.$get("authentications"); const authentication = authentications[0]; - const { isNewUser, isNewTeam } = await accountProvisioner({ - ip, + const { isNewUser, isNewTeam } = await accountProvisioner(ctx, { user: { name: existing.name, email: existing.email!, @@ -256,12 +256,11 @@ describe("accountProvisioner", () => { let error; try { - await accountProvisioner({ - ip, + await accountProvisioner(ctx, { user: { name: "Jenny Tester", email, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), }, team: { avatarUrl: existingTeam.avatarUrl, @@ -299,12 +298,11 @@ describe("accountProvisioner", () => { createdById: admin.id, }); const email = faker.internet.email({ provider: domain }); - const { user, isNewUser } = await accountProvisioner({ - ip, + const { user, isNewUser } = await accountProvisioner(ctx, { user: { name: "Jenny Tester", email, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), }, team: { avatarUrl: team.avatarUrl, @@ -347,12 +345,11 @@ describe("accountProvisioner", () => { ); const authenticationProvider = authenticationProviders[0]; const email = faker.internet.email().toLowerCase(); - const { user, isNewUser } = await accountProvisioner({ - ip, + const { user, isNewUser } = await accountProvisioner(ctx, { user: { name: "Jenny Tester", email, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), }, team: { name: team.name, @@ -397,12 +394,11 @@ describe("accountProvisioner", () => { const team = await buildTeam(); try { - await accountProvisioner({ - ip, + await accountProvisioner(ctx, { user: { name: "Jenny Tester", email: faker.internet.email(), - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), }, team: { teamId: team.id, @@ -430,12 +426,11 @@ describe("accountProvisioner", () => { it("should always use existing team if self-hosted", async () => { const team = await buildTeam(); const domain = faker.internet.domainName(); - const { user, isNewUser } = await accountProvisioner({ - ip, + const { user, isNewUser } = await accountProvisioner(ctx, { user: { name: "Jenny Tester", email: faker.internet.email(), - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), }, team: { teamId: team.id, diff --git a/server/commands/accountProvisioner.ts b/server/commands/accountProvisioner.ts index ebbb991f9f..b2acb4bdd8 100644 --- a/server/commands/accountProvisioner.ts +++ b/server/commands/accountProvisioner.ts @@ -20,9 +20,9 @@ import { DocumentHelper } from "@server/models/helpers/DocumentHelper"; import { sequelize } from "@server/storage/database"; import teamProvisioner from "./teamProvisioner"; import userProvisioner from "./userProvisioner"; +import { APIContext } from "@server/types"; type Props = { - ip: string; /** Details of the user logging in from SSO provider */ user: { /** The displayed name of the user */ @@ -79,19 +79,20 @@ export type AccountProvisionerResult = { isNewUser: boolean; }; -async function accountProvisioner({ - ip, - user: userParams, - team: teamParams, - authenticationProvider: authenticationProviderParams, - authentication: authenticationParams, -}: Props): Promise { +async function accountProvisioner( + ctx: APIContext, + { + user: userParams, + team: teamParams, + authenticationProvider: authenticationProviderParams, + authentication: authenticationParams, + }: Props +): Promise { let result; let emailMatchOnly; try { - result = await teamProvisioner({ - ip, + result = await teamProvisioner(ctx, { name: "Wiki", ...teamParams, authenticationProvider: authenticationProviderParams, @@ -141,14 +142,13 @@ async function accountProvisioner({ throw AuthenticationProviderDisabledError(); } - result = await userProvisioner({ + result = await userProvisioner(ctx, { name: userParams.name, email: userParams.email, language: userParams.language, role: isNewTeam ? UserRole.Admin : undefined, avatarUrl: userParams.avatarUrl, teamId: team.id, - ip, authentication: emailMatchOnly ? undefined : { diff --git a/server/commands/subscriptionCreator.ts b/server/commands/subscriptionCreator.ts index dffddd004a..913027f454 100644 --- a/server/commands/subscriptionCreator.ts +++ b/server/commands/subscriptionCreator.ts @@ -30,7 +30,7 @@ export default async function subscriptionCreator({ event, resubscribe = true, }: Props): Promise { - const { user } = ctx.context.auth; + const { user } = ctx.state.auth; const where: WhereOptions = { userId: user.id, diff --git a/server/commands/teamCreator.ts b/server/commands/teamCreator.ts index 9fc77dd0ef..7c861e869c 100644 --- a/server/commands/teamCreator.ts +++ b/server/commands/teamCreator.ts @@ -6,7 +6,6 @@ import { Team } from "@server/models"; import { APIContext } from "@server/types"; type Props = { - ctx: APIContext; /** The displayed name of the team */ name: string; /** The domain name from the email of the user logging in */ @@ -24,13 +23,10 @@ type Props = { }[]; }; -async function teamCreator({ - ctx, - name, - subdomain, - avatarUrl, - authenticationProviders, -}: Props): Promise { +async function teamCreator( + ctx: APIContext, + { name, subdomain, avatarUrl, authenticationProviders }: Props +): Promise { if (!avatarUrl?.startsWith("http")) { avatarUrl = null; } diff --git a/server/commands/teamProvisioner.test.ts b/server/commands/teamProvisioner.test.ts index 81ce331e95..bc4bc9316b 100644 --- a/server/commands/teamProvisioner.test.ts +++ b/server/commands/teamProvisioner.test.ts @@ -3,22 +3,23 @@ import TeamDomain from "@server/models/TeamDomain"; import { buildTeam, buildUser } from "@server/test/factories"; import { setSelfHosted } from "@server/test/support"; import teamProvisioner from "./teamProvisioner"; +import { createContext } from "@server/context"; describe("teamProvisioner", () => { - const ip = "127.0.0.1"; + const ip = faker.internet.ip(); + const ctx = createContext({ ip }); describe("hosted", () => { it("should create team and authentication provider", async () => { const subdomain = faker.internet.domainWord(); - const result = await teamProvisioner({ + const result = await teamProvisioner(ctx, { name: "Test team", subdomain, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), authenticationProvider: { name: "google", providerId: `${subdomain}.com`, }, - ip, }); const { team, authenticationProvider, isNewTeam } = result; expect(authenticationProvider.name).toEqual("google"); @@ -35,15 +36,14 @@ describe("teamProvisioner", () => { subdomain, }); - const result = await teamProvisioner({ + const result = await teamProvisioner(ctx, { name: "Test team", subdomain, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), authenticationProvider: { name: "google", providerId: `${subdomain}.com`, }, - ip, }); expect(result.isNewTeam).toEqual(true); @@ -58,15 +58,14 @@ describe("teamProvisioner", () => { await buildTeam({ subdomain: `${subdomain}1`, }); - const result = await teamProvisioner({ + const result = await teamProvisioner(ctx, { name: "Test team", subdomain, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), authenticationProvider: { name: "google", providerId: `${subdomain}.com`, }, - ip, }); expect(result.team.subdomain).toEqual(`${subdomain}2`); @@ -82,11 +81,10 @@ describe("teamProvisioner", () => { subdomain, authenticationProviders: [authenticationProvider], }); - const result = await teamProvisioner({ + const result = await teamProvisioner(ctx, { name: faker.company.name(), subdomain, authenticationProvider, - ip, }); const { team, isNewTeam } = result; expect(team.id).toEqual(existing.id); @@ -111,7 +109,7 @@ describe("teamProvisioner", () => { let error; try { const testSubdomain = faker.internet.domainWord(); - await teamProvisioner({ + await teamProvisioner(ctx, { teamId: exampleTeam.id, name: "name", subdomain: testSubdomain, @@ -119,7 +117,6 @@ describe("teamProvisioner", () => { name: "google", providerId: `${testSubdomain}.com`, }, - ip, }); } catch (e) { error = e; @@ -133,15 +130,14 @@ describe("teamProvisioner", () => { it("should allow creating first team", async () => { const subdomain = faker.internet.domainWord(); - const { team, isNewTeam } = await teamProvisioner({ + const { team, isNewTeam } = await teamProvisioner(ctx, { name: "Test team", subdomain, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), authenticationProvider: { name: "google", providerId: `${subdomain}.com`, }, - ip, }); expect(isNewTeam).toBeTruthy(); @@ -154,16 +150,15 @@ describe("teamProvisioner", () => { let error; try { - await teamProvisioner({ + await teamProvisioner(ctx, { name: "Test team", subdomain, - avatarUrl: faker.internet.avatar(), + avatarUrl: faker.image.avatar(), teamId: team.id, authenticationProvider: { name: "google", providerId: `${subdomain}.com`, }, - ip, }); } catch (err) { error = err; @@ -183,7 +178,7 @@ describe("teamProvisioner", () => { name: domain, createdById: user.id, }); - const result = await teamProvisioner({ + const result = await teamProvisioner(ctx, { name: "Updated name", subdomain: faker.internet.domainWord(), domain, @@ -192,7 +187,6 @@ describe("teamProvisioner", () => { name: "google", providerId: domain, }, - ip, }); const { team, authenticationProvider, isNewTeam } = result; expect(team.id).toEqual(existing.id); @@ -219,7 +213,7 @@ describe("teamProvisioner", () => { let error; try { - await teamProvisioner({ + await teamProvisioner(ctx, { name: "Updated name", subdomain: faker.internet.domainWord(), domain: otherDomain, @@ -228,7 +222,6 @@ describe("teamProvisioner", () => { name: "google", providerId: otherDomain, }, - ip, }); } catch (err) { error = err; @@ -247,11 +240,10 @@ describe("teamProvisioner", () => { subdomain, authenticationProviders: [authenticationProvider], }); - const result = await teamProvisioner({ + const result = await teamProvisioner(ctx, { name: "Updated name", subdomain, authenticationProvider, - ip, }); const { team, isNewTeam } = result; expect(team.id).toEqual(existing.id); diff --git a/server/commands/teamProvisioner.ts b/server/commands/teamProvisioner.ts index e8adc9b703..93599ab2cc 100644 --- a/server/commands/teamProvisioner.ts +++ b/server/commands/teamProvisioner.ts @@ -9,6 +9,7 @@ import { import { traceFunction } from "@server/logging/tracing"; import { Team, AuthenticationProvider } from "@server/models"; import { sequelize } from "@server/storage/database"; +import { APIContext } from "@server/types"; type TeamProvisionerResult = { team: Team; @@ -37,18 +38,12 @@ type Props = { /** External identifier of the authentication provider */ providerId: string; }; - ip?: string; }; -async function teamProvisioner({ - teamId, - name, - domain, - subdomain, - avatarUrl, - authenticationProvider, - ip, -}: Props): Promise { +async function teamProvisioner( + ctx: APIContext, + { teamId, name, domain, subdomain, avatarUrl, authenticationProvider }: Props +): Promise { let authP = await AuthenticationProvider.findOne({ where: teamId ? { ...authenticationProvider, teamId } @@ -109,8 +104,7 @@ async function teamProvisioner({ // We cannot find an existing team, so we create a new one const team = await sequelize.transaction((transaction) => - teamCreator({ - ctx: createContext({ ip, transaction }), + teamCreator(createContext({ transaction }), { name, domain, subdomain, diff --git a/server/commands/teamUpdater.ts b/server/commands/teamUpdater.ts index 7cc27d1345..26772eca20 100644 --- a/server/commands/teamUpdater.ts +++ b/server/commands/teamUpdater.ts @@ -5,13 +5,12 @@ import { Team, TeamDomain, User } from "@server/models"; import { APIContext } from "@server/types"; type Props = { - ctx: APIContext; params: Partial> & { allowedDomains?: string[] }; user: User; team: Team; }; -const teamUpdater = async ({ ctx, params, user, team }: Props) => { +const teamUpdater = async (ctx: APIContext, { params, user, team }: Props) => { const { allowedDomains, preferences, subdomain, ...attributes } = params; team.setAttributes(attributes); @@ -22,7 +21,7 @@ const teamUpdater = async ({ ctx, params, user, team }: Props) => { if (allowedDomains !== undefined) { const existingAllowedDomains = await TeamDomain.findAll({ where: { teamId: team.id }, - transaction: ctx.context.transaction, + transaction: ctx.state.transaction, }); // Only keep existing domains if they are still in the list of allowed domains diff --git a/server/commands/userDestroyer.test.ts b/server/commands/userDestroyer.test.ts deleted file mode 100644 index 5053982379..0000000000 --- a/server/commands/userDestroyer.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { buildUser, buildAdmin } from "@server/test/factories"; -import { withAPIContext } from "@server/test/support"; -import userDestroyer from "./userDestroyer"; - -describe("userDestroyer", () => { - it("should prevent last user from deleting account", async () => { - const user = await buildUser(); - let error; - - try { - await withAPIContext(user, async (ctx) => { - await userDestroyer(ctx, { - user, - }); - }); - } catch (err) { - error = err; - } - - expect(error && error.message).toContain("Cannot delete last user"); - }); - - it("should prevent last admin from deleting account", async () => { - const user = await buildAdmin(); - await buildUser({ - teamId: user.teamId, - }); - let error; - - try { - await withAPIContext(user, async (ctx) => { - await userDestroyer(ctx, { - user, - }); - }); - } catch (err) { - error = err; - } - - expect(error && error.message).toContain("Cannot delete account"); - }); - - it("should not prevent multiple admin from deleting account", async () => { - const actor = await buildAdmin(); - const user = await buildAdmin({ - teamId: actor.teamId, - }); - let error; - - try { - await withAPIContext(actor, async (ctx) => { - await userDestroyer(ctx, { - user, - }); - }); - } catch (err) { - error = err; - } - - expect(error).toBeFalsy(); - expect(user.deletedAt).toBeTruthy(); - }); - - it("should not prevent last non-admin from deleting account", async () => { - const user = await buildUser(); - await buildUser({ - teamId: user.teamId, - }); - let error; - - try { - await withAPIContext(user, async (ctx) => { - await userDestroyer(ctx, { - user, - }); - }); - } catch (err) { - error = err; - } - - expect(error).toBeFalsy(); - expect(user.deletedAt).toBeTruthy(); - }); -}); diff --git a/server/commands/userDestroyer.ts b/server/commands/userDestroyer.ts deleted file mode 100644 index ee0996ea3b..0000000000 --- a/server/commands/userDestroyer.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { Op } from "sequelize"; -import { UserRole } from "@shared/types"; -import { Event, User } from "@server/models"; -import { APIContext } from "@server/types"; -import { ValidationError } from "../errors"; - -export default async function userDestroyer( - ctx: APIContext, - { - user, - }: { - user: User; - } -) { - const { transaction } = ctx.state; - const { teamId } = user; - const usersCount = await User.count({ - where: { - teamId, - }, - transaction, - }); - - if (usersCount === 1) { - throw ValidationError( - "Cannot delete last user on the team, delete the workspace instead." - ); - } - - if (user.isAdmin) { - const otherAdminsCount = await User.count({ - where: { - role: UserRole.Admin, - teamId, - id: { - [Op.ne]: user.id, - }, - }, - transaction, - }); - - if (otherAdminsCount === 0) { - throw ValidationError( - "Cannot delete account as only admin. Please make another user admin and try again." - ); - } - } - - await Event.createFromContext(ctx, { - name: "users.delete", - userId: user.id, - data: { - name: user.name, - }, - }); - - return user.destroy({ - transaction, - }); -} diff --git a/server/commands/userInviter.test.ts b/server/commands/userInviter.test.ts index 6fd81e904a..358b394587 100644 --- a/server/commands/userInviter.test.ts +++ b/server/commands/userInviter.test.ts @@ -2,93 +2,92 @@ import { faker } from "@faker-js/faker"; import { UserRole } from "@shared/types"; import { buildUser } from "@server/test/factories"; import userInviter from "./userInviter"; +import { withAPIContext } from "@server/test/support"; describe("userInviter", () => { - const ip = "127.0.0.1"; - it("should return sent invites", async () => { const user = await buildUser(); - const response = await userInviter({ - invites: [ - { - role: UserRole.Member, - email: faker.internet.email(), - name: "Test", - }, - ], - user, - ip, - }); + const response = await withAPIContext(user, (ctx) => + userInviter(ctx, { + invites: [ + { + role: UserRole.Member, + email: faker.internet.email(), + name: "Test", + }, + ], + }) + ); expect(response.sent.length).toEqual(1); }); it("should filter empty invites", async () => { const user = await buildUser(); - const response = await userInviter({ - invites: [ - { - role: UserRole.Member, - email: " ", - name: "Test", - }, - ], - user, - ip, - }); + const response = await withAPIContext(user, (ctx) => + userInviter(ctx, { + invites: [ + { + role: UserRole.Member, + email: " ", + name: "Test", + }, + ], + }) + ); expect(response.sent.length).toEqual(0); }); it("should filter obviously bunk emails", async () => { const user = await buildUser(); - const response = await userInviter({ - invites: [ - { - role: UserRole.Member, - email: "notanemail", - name: "Test", - }, - ], - user, - ip, - }); + const response = await withAPIContext(user, (ctx) => + userInviter(ctx, { + invites: [ + { + role: UserRole.Member, + email: "notanemail", + name: "Test", + }, + ], + }) + ); expect(response.sent.length).toEqual(0); }); it("should not send duplicates", async () => { const user = await buildUser(); - const response = await userInviter({ - invites: [ - { - role: UserRole.Member, - email: "the@same.com", - name: "Test", - }, - { - role: UserRole.Member, - email: "the@SAME.COM", - name: "Test", - }, - ], - user, - ip, - }); + const response = await withAPIContext(user, (ctx) => + userInviter(ctx, { + invites: [ + { + role: UserRole.Member, + email: "the@same.com", + name: "Test", + }, + { + role: UserRole.Member, + email: "the@SAME.COM", + name: "Test", + }, + ], + }) + ); expect(response.sent.length).toEqual(1); }); it("should not send invites to existing team members", async () => { const email = faker.internet.email().toLowerCase(); const user = await buildUser({ email }); - const response = await userInviter({ - invites: [ - { - role: UserRole.Member, - email, - name: user.name, - }, - ], - user, - ip, - }); + const response = await withAPIContext(user, (ctx) => + userInviter(ctx, { + invites: [ + { + role: UserRole.Member, + email, + name: user.name, + }, + ], + }) + ); expect(response.sent.length).toEqual(0); }); }); diff --git a/server/commands/userInviter.ts b/server/commands/userInviter.ts index 879eb11c74..3c8150f65f 100644 --- a/server/commands/userInviter.ts +++ b/server/commands/userInviter.ts @@ -3,8 +3,9 @@ import { UserRole } from "@shared/types"; import InviteEmail from "@server/emails/templates/InviteEmail"; import env from "@server/env"; import Logger from "@server/logging/Logger"; -import { User, Event, Team } from "@server/models"; +import { User, Team } from "@server/models"; import { UserFlag } from "@server/models/User"; +import { APIContext } from "@server/types"; export type Invite = { name: string; @@ -12,18 +13,18 @@ export type Invite = { role: UserRole; }; -export default async function userInviter({ - user, - invites, - ip, -}: { - user: User; +type Props = { invites: Invite[]; - ip: string; -}): Promise<{ +}; + +export default async function userInviter( + ctx: APIContext, + { invites }: Props +): Promise<{ sent: Invite[]; users: User[]; }> { + const { user } = ctx.state.auth; const team = await Team.findByPk(user.teamId, { rejectOnEmpty: true }); // filter out empties and obvious non-emails @@ -56,34 +57,28 @@ export default async function userInviter({ // send and record remaining invites for (const invite of filteredInvites) { - const newUser = await User.create({ - teamId: user.teamId, - name: invite.name, - email: invite.email, - role: - user.isAdmin && invite.role === UserRole.Admin - ? UserRole.Admin - : user.isViewer || invite.role === UserRole.Viewer - ? UserRole.Viewer - : UserRole.Member, - invitedById: user.id, - flags: { - [UserFlag.InviteSent]: 1, - }, - }); - users.push(newUser); - await Event.create({ - name: "users.invite", - actorId: user.id, - teamId: user.teamId, - userId: newUser.id, - data: { - email: invite.email, + const newUser = await User.createWithCtx( + ctx, + { + teamId: user.teamId, name: invite.name, - role: invite.role, + email: invite.email, + role: + user.isAdmin && invite.role === UserRole.Admin + ? UserRole.Admin + : user.isViewer || invite.role === UserRole.Viewer + ? UserRole.Viewer + : UserRole.Member, + invitedById: user.id, + flags: { + [UserFlag.InviteSent]: 1, + }, }, - ip, - }); + { + name: "invite", + } + ); + users.push(newUser); await new InviteEmail({ to: invite.email, diff --git a/server/commands/userProvisioner.test.ts b/server/commands/userProvisioner.test.ts index 32a2e93b38..5c0c16ed8f 100644 --- a/server/commands/userProvisioner.test.ts +++ b/server/commands/userProvisioner.test.ts @@ -9,21 +9,22 @@ import { buildAdmin, } from "@server/test/factories"; import userProvisioner from "./userProvisioner"; +import { createContext } from "@server/context"; describe("userProvisioner", () => { - const ip = "127.0.0.1"; + const ip = faker.internet.ip(); + const ctx = createContext({ ip }); it("should update existing user and authentication", async () => { const existing = await buildUser(); const authentications = await existing.$get("authentications"); const existingAuth = authentications[0]; const newEmail = "test@example.com"; - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: existing.name, email: newEmail, avatarUrl: existing.avatarUrl, teamId: existing.teamId, - ip, authentication: { authenticationProviderId: existingAuth.authenticationProviderId, providerId: existingAuth.providerId, @@ -52,12 +53,11 @@ describe("userProvisioner", () => { authentications: [], }); - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: existing.name, email, avatarUrl: existing.avatarUrl, teamId: existing.teamId, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: uuidv4(), @@ -87,12 +87,11 @@ describe("userProvisioner", () => { teamId: team.id, }); - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: existing.name, email, avatarUrl: existing.avatarUrl, teamId: existing.teamId, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: uuidv4(), @@ -116,12 +115,11 @@ describe("userProvisioner", () => { const authentications = await existing.$get("authentications"); const existingAuth = authentications[0]; const newEmail = "test@example.com"; - await existing.destroy(); - const result = await userProvisioner({ + await existing.destroy({ hooks: false }); + const result = await userProvisioner(ctx, { name: "Test Name", email: "test@example.com", teamId: existing.teamId, - ip, authentication: { authenticationProviderId: existingAuth.authenticationProviderId, providerId: existingAuth.providerId, @@ -145,11 +143,10 @@ describe("userProvisioner", () => { let error; try { - await userProvisioner({ + await userProvisioner(ctx, { name: "Test Name", email: "test@example.com", teamId: existing.teamId, - ip, authentication: { authenticationProviderId: uuidv4(), providerId: existingAuth.providerId, @@ -168,11 +165,10 @@ describe("userProvisioner", () => { const team = await buildTeam(); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: "Test Name", email: "test@example.com", teamId: team.id, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", @@ -196,12 +192,11 @@ describe("userProvisioner", () => { }); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: "Test Name", email: "test@example.com", teamId: team.id, role: UserRole.Admin, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", @@ -219,11 +214,10 @@ describe("userProvisioner", () => { }); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: "Test Name", email: "test@example.com", teamId: team.id, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", @@ -233,11 +227,10 @@ describe("userProvisioner", () => { }); const { user: tname } = result; expect(tname.role).toEqual(UserRole.Viewer); - const tname2Result = await userProvisioner({ + const tname2Result = await userProvisioner(ctx, { name: "Test2 Name", email: "tes2@example.com", teamId: team.id, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", @@ -257,11 +250,10 @@ describe("userProvisioner", () => { }); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: invite.name, email: "invite@ExamPle.com", teamId: invite.teamId, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", @@ -289,11 +281,10 @@ describe("userProvisioner", () => { email: externalUser.email, }); - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: invite.name, email: "external@ExamPle.com", // ensure that email is case insensistive teamId: invite.teamId, - ip, }); const { user, authentication, isNewUser } = result; expect(authentication).toEqual(null); @@ -309,11 +300,10 @@ describe("userProvisioner", () => { let error; try { - await userProvisioner({ + await userProvisioner(ctx, { name: "Uninvited User", email: "invite@ExamPle.com", teamId: team.id, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", @@ -343,11 +333,10 @@ describe("userProvisioner", () => { const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; const email = faker.internet.email({ provider: domain }); - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: faker.person.fullName(), email, teamId: team.id, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", @@ -376,11 +365,10 @@ describe("userProvisioner", () => { createdById: admin.id, }); - const result = await userProvisioner({ + const result = await userProvisioner(ctx, { name: "Test Name", email, teamId: team.id, - ip, }); const { user, authentication, isNewUser } = result; expect(authentication).toBeUndefined(); @@ -393,11 +381,10 @@ describe("userProvisioner", () => { let error; try { - await userProvisioner({ + await userProvisioner(ctx, { name: "Test Name", email: faker.internet.email(), teamId: team.id, - ip, }); } catch (err) { error = err; @@ -420,11 +407,10 @@ describe("userProvisioner", () => { let error; try { - await userProvisioner({ + await userProvisioner(ctx, { name: "Bad Domain User", email: faker.internet.email(), teamId: team.id, - ip, authentication: { authenticationProviderId: authenticationProvider.id, providerId: "fake-service-id", diff --git a/server/commands/userProvisioner.ts b/server/commands/userProvisioner.ts index 59e11262eb..56225781ef 100644 --- a/server/commands/userProvisioner.ts +++ b/server/commands/userProvisioner.ts @@ -7,8 +7,9 @@ import { InviteRequiredError, } from "@server/errors"; import Logger from "@server/logging/Logger"; -import { Event, Team, User, UserAuthentication } from "@server/models"; +import { Team, User, UserAuthentication } from "@server/models"; import { sequelize } from "@server/storage/database"; +import { APIContext } from "@server/types"; type UserProvisionerResult = { user: User; @@ -32,8 +33,6 @@ type Props = { * subdomain that the request came from, if any. */ teamId: string; - /** The IP address of the incoming request */ - ip: string; /** Bundle of props related to the current external provider authentication */ authentication?: { authenticationProviderId: string; @@ -50,16 +49,10 @@ type Props = { }; }; -export default async function userProvisioner({ - name, - email, - role, - language, - avatarUrl, - teamId, - authentication, - ip, -}: Props): Promise { +export default async function userProvisioner( + ctx: APIContext, + { name, email, role, language, avatarUrl, teamId, authentication }: Props +): Promise { const auth = authentication ? await UserAuthentication.findOne({ where: { @@ -137,24 +130,6 @@ export default async function userProvisioner({ const isInvite = existingUser.isInvited; const userAuth = await sequelize.transaction(async (transaction) => { - if (isInvite) { - await Event.create( - { - name: "users.create", - actorId: existingUser.id, - userId: existingUser.id, - teamId: existingUser.teamId, - data: { - name, - }, - ip, - }, - { - transaction, - } - ); - } - // Regardless, create a new authentication record // against the existing user (user can auth with multiple SSO providers) // Update user's name and avatar based on the most recently added provider @@ -163,7 +138,7 @@ export default async function userProvisioner({ name, avatarUrl, lastActiveAt: new Date(), - lastActiveIp: ip, + lastActiveIp: ctx.ip, }, { transaction, @@ -230,7 +205,8 @@ export default async function userProvisioner({ throw DomainNotAllowedError(); } - const user = await User.create( + const user = await User.createWithCtx( + ctx, { name, email, @@ -239,25 +215,12 @@ export default async function userProvisioner({ teamId, avatarUrl, authentications: authentication ? [authentication] : [], + lastActiveAt: new Date(), + lastActiveIp: ctx.ip, } as Partial>, + undefined, { include: "authentications", - transaction, - } - ); - await Event.create( - { - name: "users.create", - actorId: user.id, - userId: user.id, - teamId: user.teamId, - data: { - name: user.name, - }, - ip, - }, - { - transaction, } ); await transaction.commit(); diff --git a/server/commands/userSuspender.test.ts b/server/commands/userSuspender.test.ts deleted file mode 100644 index cdcd5fc454..0000000000 --- a/server/commands/userSuspender.test.ts +++ /dev/null @@ -1,67 +0,0 @@ -import GroupUser from "@server/models/GroupUser"; -import { buildGroup, buildAdmin, buildUser } from "@server/test/factories"; -import userSuspender from "./userSuspender"; - -describe("userSuspender", () => { - const ip = "127.0.0.1"; - - it("should not suspend self", async () => { - const user = await buildUser(); - let error; - - try { - await userSuspender({ - actorId: user.id, - user, - ip, - }); - } catch (err) { - error = err; - } - - expect(error.message).toEqual("Unable to suspend the current user"); - }); - - it("should suspend the user", async () => { - const admin = await buildAdmin(); - const user = await buildUser({ - teamId: admin.teamId, - }); - await userSuspender({ - actorId: admin.id, - user, - ip, - }); - expect(user.suspendedAt).toBeTruthy(); - expect(user.suspendedById).toEqual(admin.id); - }); - - it("should remove group memberships", async () => { - const admin = await buildAdmin(); - const user = await buildUser({ - teamId: admin.teamId, - }); - const group = await buildGroup({ - teamId: user.teamId, - }); - await group.$add("user", user, { - through: { - createdById: user.id, - }, - }); - await userSuspender({ - actorId: admin.id, - user, - ip, - }); - expect(user.suspendedAt).toBeTruthy(); - expect(user.suspendedById).toEqual(admin.id); - expect( - await GroupUser.count({ - where: { - userId: user.id, - }, - }) - ).toEqual(0); - }); -}); diff --git a/server/commands/userSuspender.ts b/server/commands/userSuspender.ts deleted file mode 100644 index 5849b90f7b..0000000000 --- a/server/commands/userSuspender.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { Transaction } from "sequelize"; -import { User, Event, GroupUser } from "@server/models"; -import CleanupDemotedUserTask from "@server/queues/tasks/CleanupDemotedUserTask"; -import { ValidationError } from "../errors"; - -type Props = { - user: User; - actorId: string; - transaction?: Transaction; - ip: string; -}; - -/** - * This command suspends an active user, this will cause them to lose access to - * the team. - */ -export default async function userSuspender({ - user, - actorId, - transaction, - ip, -}: Props): Promise { - if (user.id === actorId) { - throw ValidationError("Unable to suspend the current user"); - } - - await user.update( - { - suspendedById: actorId, - suspendedAt: new Date(), - }, - { - transaction, - } - ); - await GroupUser.destroy({ - where: { - userId: user.id, - }, - transaction, - individualHooks: true, - }); - await Event.create( - { - name: "users.suspend", - actorId, - userId: user.id, - teamId: user.teamId, - data: { - name: user.name, - }, - ip, - }, - { - transaction, - } - ); - - await new CleanupDemotedUserTask().schedule({ userId: user.id }); -} diff --git a/server/commands/userUnsuspender.test.ts b/server/commands/userUnsuspender.test.ts deleted file mode 100644 index 182da44119..0000000000 --- a/server/commands/userUnsuspender.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { buildAdmin, buildUser } from "@server/test/factories"; -import userUnsuspender from "./userUnsuspender"; - -describe("userUnsuspender", () => { - const ip = "127.0.0.1"; - - it("should not allow unsuspending self", async () => { - const user = await buildUser(); - let error; - - try { - await userUnsuspender({ - actorId: user.id, - user, - ip, - }); - } catch (err) { - error = err; - } - - expect(error.message).toEqual("Unable to unsuspend the current user"); - }); - - it("should unsuspend the user", async () => { - const admin = await buildAdmin(); - const user = await buildUser({ - teamId: admin.teamId, - suspendedAt: new Date(), - suspendedById: admin.id, - }); - await userUnsuspender({ - actorId: admin.id, - user, - ip, - }); - expect(user.suspendedAt).toEqual(null); - expect(user.suspendedById).toEqual(null); - }); -}); diff --git a/server/commands/userUnsuspender.ts b/server/commands/userUnsuspender.ts deleted file mode 100644 index a3b14c3f95..0000000000 --- a/server/commands/userUnsuspender.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { Transaction } from "sequelize"; -import { User, Event } from "@server/models"; -import { ValidationError } from "../errors"; - -type Props = { - user: User; - actorId: string; - transaction?: Transaction; - ip: string; -}; - -/** - * This command unsuspends a previously suspended user, allowing access to the - * team again. - */ -export default async function userUnsuspender({ - user, - actorId, - transaction, - ip, -}: Props): Promise { - if (user.id === actorId) { - throw ValidationError("Unable to unsuspend the current user"); - } - - await user.update( - { - suspendedById: null, - suspendedAt: null, - }, - { transaction } - ); - await Event.create( - { - name: "users.activate", - actorId, - userId: user.id, - teamId: user.teamId, - data: { - name: user.name, - }, - ip, - }, - { - transaction, - } - ); -} diff --git a/server/context.ts b/server/context.ts index 1dcd3709cd..f6c54fec1d 100644 --- a/server/context.ts +++ b/server/context.ts @@ -13,9 +13,14 @@ export function createContext({ ip?: string | null; transaction?: Transaction; }) { + const auth = { user, type: authType }; return { + state: { + auth, + transaction, + }, context: { - auth: { user, type: authType }, + auth, ip: ip ?? user?.lastActiveIp, transaction, }, diff --git a/server/models/Share.ts b/server/models/Share.ts index 57db147872..5277f44bf1 100644 --- a/server/models/Share.ts +++ b/server/models/Share.ts @@ -194,7 +194,7 @@ class Share extends IdModel< documentId: string; revoke(ctx: APIContext) { - const { user } = ctx.context.auth; + const { user } = ctx.state.auth; this.revokedAt = new Date(); this.revokedById = user.id; return this.saveWithCtx(ctx, undefined, { name: "revoke" }); diff --git a/server/models/Team.ts b/server/models/Team.ts index f7d46a9b93..c616990cb2 100644 --- a/server/models/Team.ts +++ b/server/models/Team.ts @@ -50,6 +50,7 @@ import IsFQDN from "./validators/IsFQDN"; import IsUrlOrRelativePath from "./validators/IsUrlOrRelativePath"; import Length from "./validators/Length"; import NotContainsUrl from "./validators/NotContainsUrl"; +import { SkipChangeset } from "./decorators/Changeset"; @Scopes(() => ({ withDomains: { @@ -174,6 +175,7 @@ class Team extends ParanoidModel< /** Approximate size in bytes of all attachments in the team. */ @IsNumeric @Column(DataType.BIGINT) + @SkipChangeset approximateTotalAttachmentsSize: number; @AllowNull @@ -186,9 +188,11 @@ class Team extends ParanoidModel< @IsDate @Column + @SkipChangeset lastActiveAt: Date | null; @Column(DataType.ARRAY(DataType.STRING)) + @SkipChangeset previousSubdomains: string[] | null; // getters diff --git a/server/models/User.test.ts b/server/models/User.test.ts index 4c266b1ba4..ef8d4bde78 100644 --- a/server/models/User.test.ts +++ b/server/models/User.test.ts @@ -1,6 +1,12 @@ import { faker } from "@faker-js/faker"; import { CollectionPermission } from "@shared/types"; -import { buildUser, buildTeam, buildCollection } from "@server/test/factories"; +import { + buildUser, + buildTeam, + buildCollection, + buildAdmin, +} from "@server/test/factories"; +import { withAPIContext } from "@server/test/support"; import UserMembership from "./UserMembership"; beforeAll(() => { @@ -37,10 +43,78 @@ describe("user model", () => { describe("destroy", () => { it("should clear PII", async () => { const user = await buildUser(); + + await buildUser({ + teamId: user.teamId, + }); + await user.destroy(); expect(user.email).toBe(null); expect(user.name).toBe("Unknown"); }); + + it("should prevent last user from deleting account", async () => { + const user = await buildUser(); + let error; + + try { + await user.destroy(); + } catch (err) { + error = err; + } + + expect(error && error.message).toContain("Cannot delete last user"); + }); + + it("should prevent last admin from deleting account", async () => { + const user = await buildAdmin(); + await buildUser({ + teamId: user.teamId, + }); + let error; + + try { + await user.destroy(); + } catch (err) { + error = err; + } + + expect(error && error.message).toContain("Cannot delete account"); + }); + + it("should not prevent multiple admin from deleting account", async () => { + const actor = await buildAdmin(); + const user = await buildAdmin({ + teamId: actor.teamId, + }); + let error; + + try { + await user.destroy(); + } catch (err) { + error = err; + } + + expect(error).toBeFalsy(); + expect(user.deletedAt).toBeTruthy(); + }); + + it("should not prevent last non-admin from deleting account", async () => { + const user = await buildUser(); + await buildUser({ + teamId: user.teamId, + }); + let error; + + try { + await user.destroy(); + } catch (err) { + error = err; + } + + expect(error).toBeFalsy(); + expect(user.deletedAt).toBeTruthy(); + }); }); describe("getJwtToken", () => { diff --git a/server/models/User.ts b/server/models/User.ts index 4f844a461d..aa0ece4e42 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -66,6 +66,7 @@ import Fix from "./decorators/Fix"; import IsUrlOrRelativePath from "./validators/IsUrlOrRelativePath"; import Length from "./validators/Length"; import NotContainsUrl from "./validators/NotContainsUrl"; +import { SkipChangeset } from "./decorators/Changeset"; /** * Flags that are available for setting on the user. @@ -157,22 +158,27 @@ class User extends ParanoidModel< @IsDate @Column + @SkipChangeset lastActiveAt: Date | null; @IsIP @Column + @SkipChangeset lastActiveIp: string | null; @IsDate @Column + @SkipChangeset lastSignedInAt: Date | null; @IsIP @Column + @SkipChangeset lastSignedInIp: string | null; @IsDate @Column + @SkipChangeset lastSigninEmailSentAt: Date | null; @IsDate @@ -645,6 +651,52 @@ class User extends ParanoidModel< // hooks + @BeforeDestroy + static async checkLastUser( + model: User, + { transaction }: { transaction: Transaction } + ) { + const usersCount = await this.count({ + where: { + teamId: model.teamId, + }, + transaction, + }); + + if (usersCount === 1) { + throw ValidationError( + "Cannot delete last user on the team, delete the workspace instead." + ); + } + } + + @BeforeDestroy + static async checkLastAdmin( + model: User, + { transaction }: { transaction: Transaction } + ) { + if (model.role !== UserRole.Admin) { + return; + } + + const otherAdminsCount = await this.count({ + where: { + teamId: model.teamId, + role: UserRole.Admin, + id: { + [Op.ne]: model.id, + }, + }, + transaction, + }); + + if (otherAdminsCount === 0) { + throw ValidationError( + "Cannot delete account as only admin. Please make another user admin and try again." + ); + } + } + @BeforeDestroy static removeIdentifyingInfo = async ( model: User, @@ -754,7 +806,7 @@ class User extends ParanoidModel< static findByEmail = async function (ctx: APIContext, email: string) { return this.findOne({ where: { - teamId: ctx.context.auth.user.teamId, + teamId: ctx.state.auth.user.teamId, email: email.trim().toLowerCase(), }, ...ctx.context, diff --git a/server/models/View.test.ts b/server/models/View.test.ts index 5d3e6e0a1d..8f4c37e4a9 100644 --- a/server/models/View.test.ts +++ b/server/models/View.test.ts @@ -39,7 +39,7 @@ describe("View", () => { userId: user.id, }); - await user.destroy(); + await user.destroy({ hooks: false }); const views = await View.findByDocument(document.id, { includeSuspended: false, diff --git a/server/models/View.ts b/server/models/View.ts index 5076cbab5a..062ec19c59 100644 --- a/server/models/View.ts +++ b/server/models/View.ts @@ -18,6 +18,7 @@ import Document from "./Document"; import User from "./User"; import IdModel from "./base/IdModel"; import Fix from "./decorators/Fix"; +import { SkipChangeset } from "./decorators/Changeset"; @Scopes(() => ({ withUser: () => ({ @@ -41,6 +42,7 @@ class View extends IdModel< @Default(1) @Column(DataType.INTEGER) + @SkipChangeset count: number; // associations diff --git a/server/models/base/Model.ts b/server/models/base/Model.ts index 029aea2e5d..dac8a11b98 100644 --- a/server/models/base/Model.ts +++ b/server/models/base/Model.ts @@ -25,6 +25,7 @@ import { import Logger from "@server/logging/Logger"; import { Replace, APIContext } from "@server/types"; import { getChangsetSkipped } from "../decorators/Changeset"; +import { InternalError } from "@server/errors"; type EventOverrideOptions = { /** Override the default event name. */ @@ -241,6 +242,12 @@ class Model< }); } + if (context.event.name?.includes(".")) { + throw InternalError( + `Event name (${context.event.name}) should not include a period, the namespace is automatically prefixed` + ); + } + const attrs = { name: `${namespace}.${context.event.name ?? name}`, modelId: "modelId" in model ? model.modelId : model.id, diff --git a/server/models/decorators/Fix.ts b/server/models/decorators/Fix.ts index 796ce4ed6c..8a96529ce3 100644 --- a/server/models/decorators/Fix.ts +++ b/server/models/decorators/Fix.ts @@ -30,10 +30,23 @@ export default function Fix(target: any): void { Object.defineProperty(this, propertyKey, { get() { - return this.getDataValue(propertyKey); + // Safety check for Jest serialization - getDataValue may not be available + // during serialization for inter-process communication + if (typeof this.getDataValue === "function") { + return this.getDataValue(propertyKey); + } + // Fallback to direct dataValues access + return this.dataValues?.[propertyKey]; }, set(value) { - this.setDataValue(propertyKey, value); + // Safety check for Jest serialization - setDataValue may not be available + // during serialization for inter-process communication + if (typeof this.setDataValue === "function") { + this.setDataValue(propertyKey, value); + } else if (this.dataValues) { + // Fallback to direct dataValues assignment + this.dataValues[propertyKey] = value; + } }, }); }); diff --git a/server/models/helpers/NotificationHelper.test.ts b/server/models/helpers/NotificationHelper.test.ts index 6013601dc9..46cf3377d5 100644 --- a/server/models/helpers/NotificationHelper.test.ts +++ b/server/models/helpers/NotificationHelper.test.ts @@ -301,7 +301,7 @@ describe("NotificationHelper", () => { userId: deletedUser.id, documentId: document.id, }); - await deletedUser.destroy(); + await deletedUser.destroy({ hooks: false }); const recipients = await NotificationHelper.getDocumentNotificationRecipients({ diff --git a/server/models/helpers/ProseMirrorHelper.test.ts b/server/models/helpers/ProseMirrorHelper.test.ts index 90c019351e..db4f798d30 100644 --- a/server/models/helpers/ProseMirrorHelper.test.ts +++ b/server/models/helpers/ProseMirrorHelper.test.ts @@ -16,7 +16,7 @@ describe("ProsemirrorHelper", () => { modelId: user.id, }; - await user.destroy(); + await user.destroy({ hooks: false }); const mentionedParagraph: DeepPartial = { type: "paragraph", diff --git a/server/policies/user.ts b/server/policies/user.ts index af8b7d897f..9ca2e2c1f8 100644 --- a/server/policies/user.ts +++ b/server/policies/user.ts @@ -57,14 +57,17 @@ allow(User, "delete", User, (actor, user) => ) ); -allow(User, ["activate", "suspend"], User, isTeamAdmin); +allow(User, ["activate", "suspend"], User, (actor, user) => + and(isTeamAdmin(actor, user), user?.id !== actor.id) +); allow(User, "promote", User, (actor, user) => and( // isTeamAdmin(actor, user), !user?.isAdmin, - !user?.isSuspended + !user?.isSuspended, + user?.id !== actor.id ) ); @@ -72,7 +75,8 @@ allow(User, "demote", User, (actor, user) => and( // isTeamAdmin(actor, user), - !user?.isSuspended + !user?.isSuspended, + user?.id !== actor.id ) ); diff --git a/server/queues/processors/CollectionsProcessor.ts b/server/queues/processors/CollectionsProcessor.ts index 1623b43628..88381e7cc3 100644 --- a/server/queues/processors/CollectionsProcessor.ts +++ b/server/queues/processors/CollectionsProcessor.ts @@ -39,8 +39,7 @@ export default class CollectionsProcessor extends BaseProcessor { ip: event.ip, }); - await teamUpdater({ - ctx, + await teamUpdater(ctx, { params: { defaultCollectionId: null }, user, team, diff --git a/server/queues/processors/UserDemotedProcessor.ts b/server/queues/processors/UserDemotedProcessor.ts index a68945815c..be9efc0c5d 100644 --- a/server/queues/processors/UserDemotedProcessor.ts +++ b/server/queues/processors/UserDemotedProcessor.ts @@ -3,7 +3,7 @@ import CleanupDemotedUserTask from "../tasks/CleanupDemotedUserTask"; import BaseProcessor from "./BaseProcessor"; export default class UserDemotedProcessor extends BaseProcessor { - static applicableEvents: TEvent["name"][] = ["users.demote"]; + static applicableEvents: TEvent["name"][] = ["users.demote", "users.suspend"]; async perform(event: UserEvent) { await new CleanupDemotedUserTask().schedule({ userId: event.userId }); diff --git a/server/routes/api/collections/collections.ts b/server/routes/api/collections/collections.ts index 65289a2b09..8b5f285cab 100644 --- a/server/routes/api/collections/collections.ts +++ b/server/routes/api/collections/collections.ts @@ -659,8 +659,7 @@ router.post( collection.permission === null && team?.defaultCollectionId === collection.id ) { - await teamUpdater({ - ctx, + await teamUpdater(ctx, { params: { defaultCollectionId: null }, user, team, diff --git a/server/routes/api/developer/developer.ts b/server/routes/api/developer/developer.ts index fc6ef249e7..630b3ae56b 100644 --- a/server/routes/api/developer/developer.ts +++ b/server/routes/api/developer/developer.ts @@ -29,7 +29,6 @@ router.post( validate(T.CreateTestUsersSchema), async (ctx: APIContext) => { const { count = 10 } = ctx.input.body; - const { user } = ctx.state.auth; const invites = Array(Math.min(count, 100)) .fill(0) .map(() => { @@ -45,11 +44,7 @@ router.post( Logger.info("utils", `Creating ${count} test users`, invites); // Generate a bunch of invites - const response = await userInviter({ - user, - invites, - ip: ctx.request.ip, - }); + const response = await userInviter(ctx, { invites }); // Convert from invites to active users by marking as active await Promise.all( diff --git a/server/routes/api/events/events.test.ts b/server/routes/api/events/events.test.ts index d845e162d3..5264aa4354 100644 --- a/server/routes/api/events/events.test.ts +++ b/server/routes/api/events/events.test.ts @@ -287,7 +287,7 @@ describe("#events.list", () => { teamId: user.teamId, actorId: user.id, }); - await user.destroy(); + await user.destroy({ hooks: false }); const res = await server.post("/api/events.list", { body: { token: admin.getJwtToken(), diff --git a/server/routes/api/groups/groups.test.ts b/server/routes/api/groups/groups.test.ts index 0864d2049a..a6fa9c93de 100644 --- a/server/routes/api/groups/groups.test.ts +++ b/server/routes/api/groups/groups.test.ts @@ -183,7 +183,7 @@ describe("#groups.list", () => { createdById: me.id, }, }); - await user.destroy(); + await user.destroy({ hooks: false }); const res = await server.post("/api/groups.list", { body: { token: me.getJwtToken(), diff --git a/server/routes/api/installation/installation.ts b/server/routes/api/installation/installation.ts index e9c40fb273..932a27a780 100644 --- a/server/routes/api/installation/installation.ts +++ b/server/routes/api/installation/installation.ts @@ -29,24 +29,18 @@ router.post( throw ValidationError("Installation already has existing teams"); } - const team = await teamCreator({ - ctx, + const team = await teamCreator(ctx, { name: teamName, subdomain: slugify(teamName), authenticationProviders: [], }); - const user = await User.create( - { - name: userName, - email: userEmail, - teamId: team.id, - role: UserRole.Admin, - }, - { - transaction, - } - ); + const user = await User.createWithCtx(ctx, { + name: userName, + email: userEmail, + teamId: team.id, + role: UserRole.Admin, + }); await signIn(ctx, "email", { user, diff --git a/server/routes/api/teams/teams.ts b/server/routes/api/teams/teams.ts index 54b1d4d3f2..dae7edf0ca 100644 --- a/server/routes/api/teams/teams.ts +++ b/server/routes/api/teams/teams.ts @@ -9,7 +9,7 @@ import auth from "@server/middlewares/authentication"; import { rateLimiter } from "@server/middlewares/rateLimiter"; import { transaction } from "@server/middlewares/transaction"; import validate from "@server/middlewares/validate"; -import { Event, Team, TeamDomain, User } from "@server/models"; +import { Team, TeamDomain, User } from "@server/models"; import { authorize } from "@server/policies"; import { presentTeam, presentPolicies } from "@server/presenters"; import { APIContext } from "@server/types"; @@ -29,8 +29,7 @@ const handleTeamUpdate = async (ctx: APIContext) => { }); authorize(user, "update", team); - const updatedTeam = await teamUpdater({ - ctx, + const updatedTeam = await teamUpdater(ctx, { params: ctx.input.body, user, team, @@ -140,36 +139,18 @@ router.post( }) ); - const team = await teamCreator({ - ctx, + const team = await teamCreator(ctx, { name, subdomain: name, authenticationProviders, }); - const newUser = await User.create( - { - teamId: team.id, - name: user.name, - email: user.email, - role: UserRole.Admin, - }, - { transaction } - ); - - await Event.create( - { - name: "users.create", - actorId: user.id, - userId: newUser.id, - teamId: newUser.teamId, - data: { - name: newUser.name, - }, - ip: ctx.ip, - }, - { transaction } - ); + const newUser = await User.createWithCtx(ctx, { + teamId: team.id, + name: user.name, + email: user.email, + role: UserRole.Admin, + }); ctx.body = { success: true, diff --git a/server/routes/api/users/__snapshots__/users.test.ts.snap b/server/routes/api/users/__snapshots__/users.test.ts.snap index 2e7f70c5f0..a75571c330 100644 --- a/server/routes/api/users/__snapshots__/users.test.ts.snap +++ b/server/routes/api/users/__snapshots__/users.test.ts.snap @@ -45,12 +45,12 @@ exports[`#users.promote should require admin 1`] = ` } `; -exports[`#users.suspend should not allow suspending the user themselves 1`] = ` +exports[`#users.suspend should not allow suspending self 1`] = ` { - "error": "validation_error", - "message": "Unable to suspend the current user", + "error": "authorization_error", + "message": "Authorization error", "ok": false, - "status": 400, + "status": 403, } `; diff --git a/server/routes/api/users/users.test.ts b/server/routes/api/users/users.test.ts index 8387429619..88c5fc15ec 100644 --- a/server/routes/api/users/users.test.ts +++ b/server/routes/api/users/users.test.ts @@ -750,7 +750,7 @@ describe("#users.updateEmail", () => { await TeamDomain.create({ teamId: user.teamId, - name: "example.com", + name: "getoutline.com", createdById: user.id, }); @@ -971,7 +971,7 @@ describe("#users.suspend", () => { expect(res.status).toEqual(200); }); - it("should not allow suspending the user themselves", async () => { + it("should not allow suspending self", async () => { const admin = await buildAdmin(); const res = await server.post("/api/users.suspend", { body: { @@ -980,7 +980,7 @@ describe("#users.suspend", () => { }, }); const body = await res.json(); - expect(res.status).toEqual(400); + expect(res.status).toEqual(403); expect(body).toMatchSnapshot(); }); diff --git a/server/routes/api/users/users.ts b/server/routes/api/users/users.ts index ed00d484ed..5fafd32773 100644 --- a/server/routes/api/users/users.ts +++ b/server/routes/api/users/users.ts @@ -4,10 +4,7 @@ import { UserPreference, UserRole } from "@shared/types"; import { UserRoleHelper } from "@shared/utils/UserRoleHelper"; import { settingsPath } from "@shared/utils/routeHelpers"; import { UserValidation } from "@shared/validations"; -import userDestroyer from "@server/commands/userDestroyer"; import userInviter from "@server/commands/userInviter"; -import userSuspender from "@server/commands/userSuspender"; -import userUnsuspender from "@server/commands/userUnsuspender"; import ConfirmUpdateEmail from "@server/emails/templates/ConfirmUpdateEmail"; import ConfirmUserDeleteEmail from "@server/emails/templates/ConfirmUserDeleteEmail"; import InviteEmail from "@server/emails/templates/InviteEmail"; @@ -18,7 +15,7 @@ import auth from "@server/middlewares/authentication"; import { rateLimiter } from "@server/middlewares/rateLimiter"; import { transaction } from "@server/middlewares/transaction"; import validate from "@server/middlewares/validate"; -import { Event, User, Team } from "@server/models"; +import { User, Team } from "@server/models"; import { UserFlag } from "@server/models/User"; import { can, authorize } from "@server/policies"; import { presentUser, presentPolicies } from "@server/presenters"; @@ -291,13 +288,7 @@ router.get( throw ValidationError("User with email already exists"); } - user.email = email; - await Event.createFromContext(ctx, { - name: "users.update", - userId: user.id, - changes: user.changeset, - }); - await user.save({ transaction }); + await user.updateWithCtx(ctx, { email }); ctx.redirect(settingsPath()); } @@ -343,12 +334,7 @@ router.post( user.timezone = timezone; } - await Event.createFromContext(ctx, { - name: "users.update", - userId: user.id, - changes: user.changeset, - }); - await user.save({ transaction }); + await user.saveWithCtx(ctx); ctx.body = { data: presentUser(user, { @@ -440,25 +426,15 @@ async function updateRole(ctx: APIContext) { } if (UserRoleHelper.canDemote(user, role)) { - name = "users.demote"; + name = "demote"; authorize(actor, "demote", user); } if (UserRoleHelper.canPromote(user, role)) { - name = "users.promote"; + name = "promote"; authorize(actor, "promote", user); } - await user.update({ role }, { transaction }); - - await Event.createFromContext(ctx, { - name, - userId, - data: { - name: user.name, - role, - }, - }); - + await user.updateWithCtx(ctx, { role }, { name }); const includeDetails = !!can(actor, "readDetails", user); ctx.body = { @@ -485,12 +461,16 @@ router.post( }); authorize(actor, "suspend", user); - await userSuspender({ - user, - actorId: actor.id, - ip: ctx.request.ip, - transaction, - }); + await user.updateWithCtx( + ctx, + { + suspendedById: actor.id, + suspendedAt: new Date(), + }, + { + name: "suspend", + } + ); const includeDetails = !!can(actor, "readDetails", user); ctx.body = { @@ -518,12 +498,16 @@ router.post( }); authorize(actor, "activate", user); - await userUnsuspender({ - user, - actorId: actor.id, - transaction, - ip: ctx.request.ip, - }); + await user.updateWithCtx( + ctx, + { + suspendedById: null, + suspendedAt: null, + }, + { + name: "activate", + } + ); const includeDetails = !!can(actor, "readDetails", user); ctx.body = { @@ -542,29 +526,22 @@ router.post( validate(T.UsersInviteSchema), async (ctx: APIContext) => { const { invites } = ctx.input.body; - const actor = ctx.state.auth.user; + const { user } = ctx.state.auth; if (invites.length > UserValidation.maxInvitesPerRequest) { throw ValidationError( `You can only invite up to ${UserValidation.maxInvitesPerRequest} users at a time` ); } + authorize(user, "inviteUser", user.team); - const { user } = ctx.state.auth; - const team = await Team.findByPk(user.teamId); - authorize(user, "inviteUser", team); - - const response = await userInviter({ - user, - invites, - ip: ctx.request.ip, - }); + const response = await userInviter(ctx, { invites }); ctx.body = { data: { sent: response.sent, users: response.users.map((user) => - presentUser(user, { includeEmail: !!can(actor, "readEmail", user) }) + presentUser(user, { includeEmail: !!can(user, "readEmail", user) }) ), }, }; @@ -676,9 +653,7 @@ router.post( } } - await userDestroyer(ctx, { - user, - }); + await user.destroyWithCtx(ctx); ctx.body = { success: true, @@ -693,16 +668,10 @@ router.post( transaction(), async (ctx: APIContext) => { const { eventType } = ctx.input.body; - const { transaction } = ctx.state; const { user } = ctx.state.auth; user.setNotificationEventType(eventType, true); - await Event.createFromContext(ctx, { - name: "users.update", - userId: user.id, - changes: user.changeset, - }); - await user.save({ transaction }); + await user.saveWithCtx(ctx); ctx.body = { data: presentUser(user, { includeDetails: true }), @@ -717,16 +686,10 @@ router.post( transaction(), async (ctx: APIContext) => { const { eventType } = ctx.input.body; - const { transaction } = ctx.state; const { user } = ctx.state.auth; user.setNotificationEventType(eventType, false); - await Event.createFromContext(ctx, { - name: "users.update", - userId: user.id, - changes: user.changeset, - }); - await user.save({ transaction }); + await user.saveWithCtx(ctx); ctx.body = { data: presentUser(user, { includeDetails: true }),