From d10340f287bfdd152c84d27389bb530d457f6f19 Mon Sep 17 00:00:00 2001 From: Bartosz Majewski <30874844+majewskibartosz@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:08:22 +0200 Subject: [PATCH 1/5] fix(platform): health page no longer reports two different app versions at once (#14819) Co-authored-by: louai --- .../engineering/ci-pr-review-hygiene.md | 1 + .../engineering/web-feature-anatomy.md | 1 + .../health/components/system-health-tab.tsx | 9 +- .../components/system-health-tab.test.tsx | 118 ++++++++++++++++++ 4 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 packages/web/test/app/routes/platform/infra/health/components/system-health-tab.test.tsx diff --git a/brain/knowledge/engineering/ci-pr-review-hygiene.md b/brain/knowledge/engineering/ci-pr-review-hygiene.md index cf32f8738963..91cf99bfaeba 100644 --- a/brain/knowledge/engineering/ci-pr-review-hygiene.md +++ b/brain/knowledge/engineering/ci-pr-review-hygiene.md @@ -28,3 +28,4 @@ Enforcement is the **`Codeowners review` repository ruleset** (active on the def - **Retargeting a stacked PR to `main` does not drop its base branch — it merges the whole thing.** A PR opened against a long-lived feature branch shows a small diff *relative to that base*, but `gh pr edit --base main` only moves the target; the branch still contains every commit of its old base. [#14593](https://github.com/activepieces/activepieces/pull/14593) read as 2 docs files against `feat/autumn-billing-integration` and as 198 commits / 211 files / +12k lines against `main`. Check with `git diff --stat origin/main...` **before** retargeting, and if it disagrees with the PR page, cherry-pick that PR's own commits onto `main` and force-push instead. A "conflict" on such a PR is often against the feature base only — those same commits can apply to `main` cleanly. - **A decision authored on a long-lived branch will collide on its number.** `brain/decisions/` numbers are assigned once and never reused, but the next free number is only knowable against `main` — two branches in flight both grab it. #14593 carried a `000024` that `main` had since filled, and `000025` too, so it landed as `000026`. Renumber against `main` at merge time and update every referring link; nothing in CI catches a duplicate number or a dead decision link. - **`tools/scripts/` is outside the lint and test wiring.** ESLint ignores it, and `npm run test-unit` only covers engine/shared/web. A script there with real policy logic must run its own tests from its own workflow — `pr-size.yml` runs `bun test tools/scripts/pr-size-check.test.ts` as a step before the check itself. +- **A branch that predates the `brain/` → `brain/knowledge/` move cannot edit a brain page in place — GitHub will call the PR conflicting even when `git merge` is clean locally.** Git follows the rename and merges the modification into the new path; GitHub's mergeability check does not, so it reports `modify/delete` on the old path and the PR goes `dirty`. Local `git merge-tree --write-tree` exits 0 and hides the problem; reproduce what GitHub sees with `git merge -X no-renames origin/main`. Fix: merge `origin/main` into the branch first, which lands the edit at the new path, then push. diff --git a/brain/knowledge/engineering/web-feature-anatomy.md b/brain/knowledge/engineering/web-feature-anatomy.md index b1769884fc23..ea4c738aa85c 100644 --- a/brain/knowledge/engineering/web-feature-anatomy.md +++ b/brain/knowledge/engineering/web-feature-anatomy.md @@ -71,3 +71,4 @@ Verify with `npx turbo run lint --filter=web`, or `npm run lint-dev` for the who - **`Alert`'s `warning` and `destructive` variants ship without a background tint, so a tinted banner has to add one at the call site.** `components/ui/alert.tsx` gives `primary` and `success` a `bg-*-100/10` wash but leaves `warning` and `destructive` transparent (`destructive` sets `bg-card`, which reads as a plain panel on a page background, and unlike `warning` it sets no border colour either). A banner that needs to look like a banner rather than a bordered paragraph passes `bg-warning-100/10` / `bg-destructive-100/10 border-destructive/50` itself — that is what the credits usage alert does. Don't "fix" it in the variant without looking: eight-plus existing warning alerts sit inside dialogs on card backgrounds and were designed against the untinted look. Note also that `--warning-100` and `--destructive-100` are *not* redefined in the `.dark` block of `styles.css` (unlike `--primary-100`), so in dark mode both tints are a very pale hue at 10% over near-black — subtle by accident, not by design. - **`npx turbo run serve --filter=web -- --mode=cloud` cannot do OAuth2 connections.** The provider redirects to `cloud.activepieces.com` after sign-in instead of your local frontend. Use API-key or basic-auth connections, or run a fully local backend. - **`--mode=cloud` also floods the terminal with `[vite] http proxy error: /ingest/... ETIMEDOUT 127.0.0.1:3000`.** The mode only redirects the API (`API_BASE_URL` → `https://cloud.activepieces.com` in `lib/api.ts`); PostHog still posts to the *relative* `api_host: '/ingest'` (a same-origin reverse proxy so ad blockers don't drop ingestion — `providers/telemetry-provider.tsx`, mirrored in prod by the `fastifyHttpProxy` in `server.ts`). Vite proxies `/ingest` to `127.0.0.1:3000`, which isn't running. Cloud flags also turn telemetry *on* (`TELEMETRY_ENABLED` + `EDITION=cloud`), unlike a local CE backend — so posthog-js keeps polling `/ingest/flags` and flushing `/ingest/e` every few seconds. Harmless, but note the same setup sends real dev clicks to production PostHog whenever `/ingest` does resolve; the clean fix is skipping `posthog.init` under `import.meta.env.DEV`. +- **`packages/web`'s lint script only globs `src/**`, so nothing under `packages/web/test/` is ever linted** — not by CI's `lint` job, not by `npm run lint-dev`. Running `npx eslint 'test/**/*.{ts,tsx}'` from `packages/web` today reports 21 errors nobody has seen, so a new web test needs a manual eslint pass or it ships with errors. Most common trap: `testing-library/render-result-naming-convention` fires on any local helper whose name merely *starts with* `render` even when testing-library is not involved — renaming `render` to `renderTabText` does not silence it, only a name that doesn't begin with `render` does. diff --git a/packages/web/src/app/routes/platform/infra/health/components/system-health-tab.tsx b/packages/web/src/app/routes/platform/infra/health/components/system-health-tab.tsx index 5a93dd2486ba..d40a6dee83ba 100644 --- a/packages/web/src/app/routes/platform/infra/health/components/system-health-tab.tsx +++ b/packages/web/src/app/routes/platform/infra/health/components/system-health-tab.tsx @@ -1,4 +1,4 @@ -import { ApFlagId, isNil } from '@activepieces/shared'; +import { isNil } from '@activepieces/shared'; import { t } from 'i18next'; import { Boxes, @@ -18,7 +18,6 @@ import { LoadingSpinner } from '@/components/custom/spinner'; import { Alert, AlertDescription } from '@/components/ui/alert'; import { Card, CardContent } from '@/components/ui/card'; import { healthQueries } from '@/features/platform-admin'; -import { flagsHooks } from '@/hooks/flags-hooks'; import { cn } from '@/lib/utils'; import { DailyHealthStrip } from './daily-health-strip'; @@ -38,18 +37,16 @@ type SystemHealthTabProps = { }; export function SystemHealthTab({ onSeeRuns }: SystemHealthTabProps) { - const { data: currentVersion } = flagsHooks.useFlag( - ApFlagId.CURRENT_VERSION, - ); const { data: systemHealth, isPending } = healthQueries.useSystemHealth(); const latestVersion = systemHealth?.latestVersion; + const release = systemHealth?.release; + const currentVersion = release?.current; const isVersionUpToDate = React.useMemo(() => { if (!currentVersion || !latestVersion) return false; return semver.gte(currentVersion, latestVersion); }, [currentVersion, latestVersion]); - const release = systemHealth?.release; const releaseIntegrityOk = !!release && release.current !== UNREADABLE_RELEASE_VERSION && diff --git a/packages/web/test/app/routes/platform/infra/health/components/system-health-tab.test.tsx b/packages/web/test/app/routes/platform/infra/health/components/system-health-tab.test.tsx new file mode 100644 index 000000000000..ba1a02ead3c7 --- /dev/null +++ b/packages/web/test/app/routes/platform/infra/health/components/system-health-tab.test.tsx @@ -0,0 +1,118 @@ +/** + * @vitest-environment jsdom + */ +/* eslint-disable testing-library/no-unnecessary-act */ +import { GetSystemHealthChecksResponse } from '@activepieces/shared'; +import * as React from 'react'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const versions = vi.hoisted(() => ({ + running: '0.88.1', + staleFlag: '0.88.0', +})); + +const systemHealthMock = vi.hoisted(() => ({ + data: undefined as GetSystemHealthChecksResponse | undefined, +})); + +vi.mock('i18next', () => ({ t: (key: string) => key })); + +vi.mock('lucide-react', () => ({ + Boxes: () => null, + Cpu: () => null, + ExternalLink: () => null, + GitCompareArrows: () => null, + HardDrive: () => null, + Info: () => null, + MemoryStick: () => null, + Package: () => null, + Server: () => null, +})); + +vi.mock( + '@/app/routes/platform/infra/health/components/daily-health-strip', + () => ({ DailyHealthStrip: () => null }), +); + +vi.mock('@/features/platform-admin', () => ({ + healthQueries: { + useSystemHealth: () => ({ data: systemHealthMock.data, isPending: false }), + }, +})); + +vi.mock('@/hooks/flags-hooks', () => ({ + flagsHooks: { + useFlag: () => ({ data: versions.staleFlag }), + }, +})); + +// eslint-disable-next-line import/first +import { SystemHealthTab } from '@/app/routes/platform/infra/health/components/system-health-tab'; + +declare global { + // eslint-disable-next-line no-var + var IS_REACT_ACT_ENVIRONMENT: boolean; +} + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +describe('SystemHealthTab version row', () => { + let container: HTMLDivElement | null = null; + let root: Root | null = null; + + const readTabText = (current: string) => { + systemHealthMock.data = buildHealth(current); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + root!.render( {}} />); + }); + return container.textContent ?? ''; + }; + + afterEach(() => { + act(() => root?.unmount()); + container?.remove(); + container = null; + root = null; + }); + + it('renders the release from the health payload, never the cached flag', () => { + const text = readTabText(versions.running); + + expect(text).toContain(`Current ${versions.running}`); + expect(text).not.toContain(`Current ${versions.staleFlag}`); + }); + + it('passes when the payload release matches the latest release', () => { + const text = readTabText(versions.running); + + expect(text).not.toContain('Needs attention'); + }); + + it('needs attention when the payload release is behind the latest release', () => { + const text = readTabText(versions.staleFlag); + + expect(text).toContain(`Current ${versions.staleFlag}`); + expect(text).toContain('Needs attention'); + }); +}); + +function buildHealth(current: string): GetSystemHealthChecksResponse { + return { + latestVersion: versions.running, + appCpu: true, + appRam: true, + disk: true, + workerCpu: true, + workerRam: true, + database: true, + release: { + current, + workers: { total: 1, versionMismatched: 0, mismatchedVersions: [] }, + }, + }; +} From 542a31a804d02df99442155d58aa49fc1a1acaf8 Mon Sep 17 00:00:00 2001 From: Bartosz Majewski <30874844+majewskibartosz@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:27:23 +0200 Subject: [PATCH 2/5] fix(users): allow re-inviting a deleted user with the same email (#14399) Co-authored-by: louai --- .../platform-editions-ee/user-invitations.md | 2 + brain/knowledge/platform-editions-ee/users.md | 5 + .../user-invitation.module.ts | 4 +- .../user-invitation.service.ts | 9 +- .../server/api/src/app/user/user-service.ts | 20 +++- .../accept-invitation.test.ts | 88 +++++++++++++++ .../ce/user/platform-user-community.test.ts | 104 ++++++++++++++++++ 7 files changed, 225 insertions(+), 7 deletions(-) create mode 100644 packages/server/api/test/integration/ce/user-invitations/accept-invitation.test.ts diff --git a/brain/knowledge/platform-editions-ee/user-invitations.md b/brain/knowledge/platform-editions-ee/user-invitations.md index 23f5c4ebf959..6301ffb8b201 100644 --- a/brain/knowledge/platform-editions-ee/user-invitations.md +++ b/brain/knowledge/platform-editions-ee/user-invitations.md @@ -21,6 +21,8 @@ Lets platform owners (and project members with `WRITE_INVITATION`) invite users - If SMTP unconfigured, the `link` field is included in the response for the caller to surface manually; if configured, `link` is omitted and email is sent. Auto-accept + SMTP sends a "project member added" notification instead. - Seat enforcement only bites when billing is enforced for the platform (`isBillingEnforced`, OBSERVE otherwise) and is skipped in CE; the seat limit applied is `min(usersLimit, scheduledUsersLimit)`. Only invites whose email is not yet a platform member reserve a seat, and a repeat invite to the same reserved email needs no extra seat. - **The "does this invite reserve a seat" rule has three implementations — change them together.** `countReservedInvites` (`platform-plan.service.ts`) expresses it as raw SQL (`status IN (PENDING, ACCEPTED)` + expiry cutoff + `NOT EXISTS` on an existing platform user); `wouldAddNewUser` re-checks the same membership condition in TypeScript; `countAdditionalSeatsNeeded` restates the same predicate in a third query builder. Editing one alone makes the counter and the guard disagree, which silently over- or under-counts seats. +- **Re-inviting a previously deleted user** works only because the CE/EE user delete also removes the now-orphaned `UserIdentity` (see [Users](./users.md)). The invitation itself always succeeds; when that cleanup has not run, the recipient's sign-up is what fails, with `EXISTING_USER`. +- **`POST /accept` answers with `registered`** — `true` when an identity already claims the invited email. The web accept page routes on exactly that field: falsy sends the recipient to `/sign-up`, `true` to `/sign-in`. The route has to spread it onto the invitation explicitly; while it was missing, every invitee landed on `/sign-up`, where an already-registered email dead-ends with `EXISTING_USER`. ### Key files Entry point: `userInvitationsService`, defined in `user-invitation.service.ts` and called from the routes in `user-invitation.module.ts` (which exports `invitationModule`). diff --git a/brain/knowledge/platform-editions-ee/users.md b/brain/knowledge/platform-editions-ee/users.md index 55b0aa9feabe..ac459886562f 100644 --- a/brain/knowledge/platform-editions-ee/users.md +++ b/brain/knowledge/platform-editions-ee/users.md @@ -24,6 +24,11 @@ Manages user identity, platform membership, roles, and session security. A `User - `GET /v1/users/me`, `POST /v1/users/me` (update firstName/lastName/profilePicture) — CE. - Platform admin CRUD (list, update role/status, delete) via `platform-user-controller.ts` — EE/Cloud. +### Gotchas +- **Deleting a user also deletes its `UserIdentity`** on self-hosted (CE/EE), but only when no `User` row on any platform still references that identity. Skip that cleanup and the orphaned identity keeps the email claimed: re-inviting the same person dead-ends with `EXISTING_USER` / `INVITATION_ONLY_SIGN_UP` on sign-up and `INVALID_CREDENTIALS` on sign-in, and CE has no reset-password path to recover from it. `otp` rows cascade away with the identity. +- **Cloud takes the other branch.** `platform-user-controller.ts` routes Cloud to `removeFromPlatform`, which nulls `platformId` and keeps the identity, since the same person may belong to other platforms. Only the CE/EE `delete` path removes identities. +- **`userIdentityService.create` matches email globally**, ignoring platform, so any identity left behind with no `User` row blocks sign-up for that email on every platform. Installs that deleted users before this cleanup existed still carry those orphans; clearing them needs `DELETE FROM user_identity ui WHERE NOT EXISTS (SELECT 1 FROM "user" u WHERE u."identityId" = ui.id)`. + ### Key files Entry point: `userService`, a log-scoped factory in `user/user-service.ts` that most callers across the API import directly. diff --git a/packages/server/api/src/app/user-invitations/user-invitation.module.ts b/packages/server/api/src/app/user-invitations/user-invitation.module.ts index b210b5bb2860..05648a7956a7 100644 --- a/packages/server/api/src/app/user-invitations/user-invitation.module.ts +++ b/packages/server/api/src/app/user-invitations/user-invitation.module.ts @@ -85,11 +85,11 @@ const invitationController: FastifyPluginAsyncZod = async (app) => { app.post('/accept', AcceptUserInvitationRequestParams, async (request, reply) => { const invitation = await userInvitationsService(request.log).getOneByInvitationTokenOrThrow(request.body.invitationToken) - await userInvitationsService(request.log).accept({ + const { registered } = await userInvitationsService(request.log).accept({ invitationId: invitation.id, platformId: invitation.platformId, }) - await reply.status(StatusCodes.OK).send(invitation) + await reply.status(StatusCodes.OK).send({ ...invitation, registered }) }) app.delete('/:id', DeleteInvitationRequestParams, async (request, reply) => { diff --git a/packages/server/api/src/app/user-invitations/user-invitation.service.ts b/packages/server/api/src/app/user-invitations/user-invitation.service.ts index cf74262658c6..d73dec5bfa4c 100644 --- a/packages/server/api/src/app/user-invitations/user-invitation.service.ts +++ b/packages/server/api/src/app/user-invitations/user-invitation.service.ts @@ -233,18 +233,19 @@ export const userInvitationsService = (log: FastifyBaseLogger) => ({ } return invitation }, - async accept({ invitationId, platformId }: AcceptParams): Promise { + async accept({ invitationId, platformId }: AcceptParams): Promise { const invitation = await this.getOneOrThrow({ id: invitationId, platformId }) await repo().update(invitation.id, { status: InvitationStatus.ACCEPTED, }) const identity = await userIdentityService(log).getIdentityByEmail(invitation.email) if (isNil(identity)) { - return + return { registered: false } } await this.provisionUserInvitation({ email: invitation.email, }) + return { registered: true } }, async hasAnyAcceptedInvitationsForEmail({ email }: { email: string }): Promise { const count = await repo().createQueryBuilder('user_invitation') @@ -358,6 +359,10 @@ type AcceptParams = { platformId: string } +type AcceptResult = { + registered: boolean +} + export type CreateInvitationRecordParams = { email: string platformId: string diff --git a/packages/server/api/src/app/user/user-service.ts b/packages/server/api/src/app/user/user-service.ts index 66b070c6184f..ef982469436a 100644 --- a/packages/server/api/src/app/user/user-service.ts +++ b/packages/server/api/src/app/user/user-service.ts @@ -166,13 +166,20 @@ export const userService = (log: FastifyBaseLogger) => ({ }, async delete({ id, platformId }: DeleteParams): Promise { await assertNotPlatformOwner({ id, platformId, log }) + const user = await userRepo().findOneBy({ id, platformId }) + if (isNil(user)) { + return + } await platformProjectService(log).deletePersonalProjectForUser({ userId: id, platformId, }) - await userRepo().delete({ - id, - platformId, + await transaction(async (entityManager) => { + await userRepo(entityManager).delete({ + id, + platformId, + }) + await deleteIdentityIfOrphaned({ identityId: user.identityId, entityManager }) }) }, async removeFromPlatform({ id, platformId }: DeleteParams): Promise { @@ -264,6 +271,13 @@ async function assertNotPlatformOwner({ id, platformId, log }: DeleteParams & { } } +async function deleteIdentityIfOrphaned({ identityId, entityManager }: { identityId: string, entityManager: EntityManager }): Promise { + const identityStillReferenced = await userRepo(entityManager).existsBy({ identityId }) + if (!identityStillReferenced) { + await userIdentityRepository(entityManager).delete({ id: identityId }) + } +} + async function getUsersForProject(platformId: PlatformId, projectId: string): Promise { const platformAdmins = await userRepo().find({ where: { platformId, platformRole: PlatformRole.ADMIN } }).then((users) => users.map((user) => user.id)) const edition = system.getEdition() diff --git a/packages/server/api/test/integration/ce/user-invitations/accept-invitation.test.ts b/packages/server/api/test/integration/ce/user-invitations/accept-invitation.test.ts new file mode 100644 index 000000000000..3fc026aad072 --- /dev/null +++ b/packages/server/api/test/integration/ce/user-invitations/accept-invitation.test.ts @@ -0,0 +1,88 @@ +import { apId } from '@activepieces/core-utils' +import { InvitationStatus, InvitationType, PlatformRole } from '@activepieces/shared' +import { FastifyInstance } from 'fastify' +import { StatusCodes } from 'http-status-codes' +import { JwtAudience, jwtUtils } from '../../../../src/app/helper/jwt-utils' +import { db } from '../../../helpers/db' +import { + createMockUserInvitation, + mockAndSaveBasicSetup, + mockBasicUser, +} from '../../../helpers/mocks' +import { setupTestEnvironment, teardownTestEnvironment } from '../../../helpers/test-setup' + +let app: FastifyInstance | null = null + +beforeAll(async () => { + app = await setupTestEnvironment() +}) + +afterAll(async () => { + await teardownTestEnvironment() +}) + +describe('Accept User Invitation API', () => { + it('Reports registered false when no identity claims the email yet', async () => { + // arrange + const { mockPlatform } = await mockAndSaveBasicSetup() + const invitationToken = await saveInvitationAndSignToken({ + email: `${apId().toLowerCase()}@example.com`, + platformId: mockPlatform.id, + }) + + // act + const response = await app?.inject({ + method: 'POST', + url: '/api/v1/user-invitations/accept', + body: { invitationToken }, + }) + + // assert + expect(response?.statusCode).toBe(StatusCodes.OK) + expect(response?.json()?.registered).toBe(false) + }) + + it('Reports registered true when the email already has an identity', async () => { + // arrange + const { mockPlatform } = await mockAndSaveBasicSetup() + const email = `${apId().toLowerCase()}@example.com` + await mockBasicUser({ + userIdentity: { email }, + user: { + platformId: mockPlatform.id, + platformRole: PlatformRole.MEMBER, + }, + }) + const invitationToken = await saveInvitationAndSignToken({ + email, + platformId: mockPlatform.id, + }) + + // act + const response = await app?.inject({ + method: 'POST', + url: '/api/v1/user-invitations/accept', + body: { invitationToken }, + }) + + // assert + expect(response?.statusCode).toBe(StatusCodes.OK) + expect(response?.json()?.registered).toBe(true) + }) +}) + +async function saveInvitationAndSignToken({ email, platformId }: { email: string, platformId: string }): Promise { + const invitation = createMockUserInvitation({ + email, + platformId, + type: InvitationType.PLATFORM, + platformRole: PlatformRole.MEMBER, + status: InvitationStatus.PENDING, + }) + await db.save('user_invitation', invitation) + return jwtUtils.sign({ + payload: { id: invitation.id }, + key: await jwtUtils.getJwtSecret(), + audience: JwtAudience.USER_INVITATION, + }) +} diff --git a/packages/server/api/test/integration/ce/user/platform-user-community.test.ts b/packages/server/api/test/integration/ce/user/platform-user-community.test.ts index 8d049e56cd43..906bc0e5af6a 100644 --- a/packages/server/api/test/integration/ce/user/platform-user-community.test.ts +++ b/packages/server/api/test/integration/ce/user/platform-user-community.test.ts @@ -6,6 +6,7 @@ import { databaseConnection } from '../../../../src/app/database/database-connec import { generateMockToken } from '../../../helpers/auth' import { createMockProject, + createMockUser, mockAndSaveBasicSetup, mockBasicUser, } from '../../../helpers/mocks' @@ -276,6 +277,109 @@ describe('User API', () => { expect(response?.statusCode).toBe(StatusCodes.NO_CONTENT) }) + it('Returns 204 when deleting a non-existent user', async () => { + // arrange + const { mockOwner, mockPlatform } = await mockAndSaveBasicSetup() + const mockOwnerToken = await generateMockToken({ + id: mockOwner.id, + type: PrincipalType.USER, + platform: { + id: mockPlatform.id, + }, + }) + + // act + const response = await app?.inject({ + method: 'DELETE', + url: `/api/v1/users/${apId()}`, + headers: { + authorization: `Bearer ${mockOwnerToken}`, + }, + }) + + // assert + expect(response?.statusCode).toBe(StatusCodes.NO_CONTENT) + }) + + it('Deletes the orphaned identity when the deleted user was its only reference', async () => { + // arrange + const { mockOwner, mockPlatform } = await mockAndSaveBasicSetup() + const { mockUser: mockMember, mockUserIdentity } = await mockBasicUser({ + user: { + platformId: mockPlatform.id, + platformRole: PlatformRole.MEMBER, + }, + }) + + const mockOwnerToken = await generateMockToken({ + id: mockOwner.id, + type: PrincipalType.USER, + platform: { + id: mockPlatform.id, + }, + }) + + // act + const response = await app?.inject({ + method: 'DELETE', + url: `/api/v1/users/${mockMember.id}`, + headers: { + authorization: `Bearer ${mockOwnerToken}`, + }, + }) + + // assert + expect(response?.statusCode).toBe(StatusCodes.NO_CONTENT) + const identity = await databaseConnection() + .getRepository('user_identity') + .findOneBy({ id: mockUserIdentity.id }) + expect(identity).toBeNull() + }) + + it('Keeps the identity when another user still references it', async () => { + // arrange + const { mockOwner, mockPlatform } = await mockAndSaveBasicSetup() + const { mockUser: mockMember, mockUserIdentity } = await mockBasicUser({ + user: { + platformId: mockPlatform.id, + platformRole: PlatformRole.MEMBER, + }, + }) + const { mockPlatform: otherPlatform } = await mockAndSaveBasicSetup() + const sharedUserOnOtherPlatform = createMockUser({ + identityId: mockUserIdentity.id, + platformId: otherPlatform.id, + platformRole: PlatformRole.MEMBER, + }) + await databaseConnection() + .getRepository('user') + .save(sharedUserOnOtherPlatform) + + const mockOwnerToken = await generateMockToken({ + id: mockOwner.id, + type: PrincipalType.USER, + platform: { + id: mockPlatform.id, + }, + }) + + // act + const response = await app?.inject({ + method: 'DELETE', + url: `/api/v1/users/${mockMember.id}`, + headers: { + authorization: `Bearer ${mockOwnerToken}`, + }, + }) + + // assert + expect(response?.statusCode).toBe(StatusCodes.NO_CONTENT) + const identity = await databaseConnection() + .getRepository('user_identity') + .findOneBy({ id: mockUserIdentity.id }) + expect(identity).not.toBeNull() + }) + it('Fails if user is not platform owner', async () => { // arrange const { mockPlatform } = await mockAndSaveBasicSetup() From ce30ed52a34cae85cc1838f622ef8554d3c0ecb4 Mon Sep 17 00:00:00 2001 From: Bartosz Majewski <30874844+majewskibartosz@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:36:40 +0200 Subject: [PATCH 3/5] fix(management): custom roles can now grant Variables and Knowledge Base access (#14814) Co-authored-by: louai --- .../platform-editions-ee/ee-projects-rbac.md | 6 ++++-- .../project-role/project-role-dialog.tsx | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/brain/knowledge/platform-editions-ee/ee-projects-rbac.md b/brain/knowledge/platform-editions-ee/ee-projects-rbac.md index 2c1c7f13daab..08f3eaf5f2b0 100644 --- a/brain/knowledge/platform-editions-ee/ee-projects-rbac.md +++ b/brain/knowledge/platform-editions-ee/ee-projects-rbac.md @@ -8,8 +8,8 @@ The EE Projects module adds team collaboration, role-based access control, git-b ### Members & roles - **ProjectMember** entity: `(projectId, userId, projectRoleId, platformId)`, unique on (projectId, userId, platformId). Service: `upsert`, `list`, `getRole` (returns ADMIN if owner/platform admin), `update`, `delete`, `getIdsOfProjects`. -- **ProjectRole**: named permission set, platform-scoped, `type` DEFAULT/CUSTOM. Built-in: **ADMIN** (all 26 permissions), **EDITOR** (read + write flows/folders/tables, update flow status), **VIEWER** (read-only). Custom roles behind `customRolesEnabled`. -- **Permission**: one of 26 granular capabilities (`READ_FLOW`, `WRITE_CONNECTION`, etc.). +- **ProjectRole**: named permission set, platform-scoped, `type` DEFAULT/CUSTOM. Built-in: **ADMIN** (every permission), **EDITOR** (read + write flows/folders/tables, update flow status), **VIEWER** (read-only). Custom roles behind `customRolesEnabled`. +- **Permission**: one granular capability (`READ_FLOW`, `WRITE_CONNECTION`, etc.), almost all of them READ/WRITE pairs per feature area. ### RBAC enforcement Yes, RBAC is a middleware layer. `rbacMiddleware` is registered once as a Fastify `preHandler` in `app.ts`, so every route passes through it. It resolves the route's project + permission and delegates to `rbacService`, which routes by principal type: **USER** goes to the member's role permission check; **ENGINE** checks `principal.projectId === requestedProjectId`; **SERVICE** checks `project.platformId === principal.platform.id`. UNKNOWN, WORKER and ONBOARDING are rejected outright. @@ -23,6 +23,7 @@ Note it lives under `ee/authentication/`, not `ee/projects/`, which is where mos - **Git Sync**: SSH repo URL + branch + folder path; push exports published flows/tables, pull imports as a release source; individual-item push supported. ### Gotchas +- **A new `Permission` needs a row in the role dialog, or custom roles can never grant it.** The toggle list is a hardcoded array, `initialPermissions` in `packages/web/src/app/routes/platform/security/project-role/project-role-dialog.tsx`, and the dialog is a plain `.map()` over it. Default-role grants are hardcoded separately in `access-control-list.ts`, so a permission added there but not here is invisible: ADMIN/EDITOR/VIEWER have it, custom roles cannot be given it, and the feature's tab just never appears for those members. This has already shipped twice — Variables + Knowledge Base (GIT-1751), then Agents. Nothing catches the drift: CI neither typechecks nor unit-tests `web`, so add the row in the same PR as the enum entry. - **Piece filtering** now via **piece sets** — `project.pieceSetId` (nullable FK, SET NULL). When `managePiecesEnabled`, new EE projects get the Default set on create; unassigned resolves to Default at filter time. This supersedes the legacy project-plan allow/block list. - **Worker routing**: `workerGroupId` (bare label, `^[a-z0-9_-]+$`) gated by `workerGroupsEnabled`. When set, the project's `EXECUTE_FLOW`/`EXECUTE_WEBHOOK` jobs route to `project-