Files
gitmost/apps/server/src/core/share/share-alias.service.spec.ts
T
agent_coder 4109b2ef7f fix(security): share-alias reassign 409 — гейт раскрытия по view-праву (доведение #495 к.4)
Коммит 4 закрыл утечку currentPageId на анонимном /availability, но
эквивалентная (и более широкая) дыра оставалась на POST /aliases/set (reassign):
при занятом алиасе и confirmReassign=false 409 отдавал currentPageId И
currentPageTitle ЦЕЛЕВОЙ страницы, а контроллер гейтил только validateCanEdit на
ИСХОДНОЙ странице. Любой участник с одной редактируемой+расшаренной страницей мог
перебирать имена алиасов и мапить их на (id, title) чужих страниц без права
просмотра — тот же класс перечисления, плюс ещё и заголовок.

Фикс: setAlias теперь гейтит раскрытие. currentPageId НЕ отдаётся никогда
(клиент им не пользуется, это перечислимая идентичность). currentPageTitle —
только если validateCanView(целевая, user) проходит; иначе голый «занят» (клиент
и так показывает generic confirm-модалку без заголовка — UX не ломается).
Гейт живёт в сервисе, где строится раскрытие (PageAccessService — @Global, без
цикла); контроллер прокидывает user. Поправлен неточный коммент checkAvailability.

Тесты: viewer → 409 с title, БЕЗ id; не-viewer → 409 без title и без id.
Mutation-verify: вернул утечку (id + безусловный title) → оба теста краснеют.
Контроллер-спек и int-spec обновлены под новую сигнатуру; tsc чист.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 01:02:13 +03:00

608 lines
21 KiB
TypeScript

import {
BadRequestException,
ConflictException,
ForbiddenException,
} from '@nestjs/common';
import { NoResultError } from 'kysely';
import { ShareAliasService } from './share-alias.service';
/**
* Behaviour tests for the alias write/resolve semantics: create vs no-op vs the
* 409 reassign guard, uniqueness-race handling, availability probe, and the
* request-time readable-target resolution (which re-runs the share boundary).
*/
const USER = { id: 'u-1' } as any;
describe('ShareAliasService', () => {
// Sentinel handed to repo calls so tests can assert they ran inside the tx.
const trx = { __trx: true };
function makeService() {
const shareAliasRepo = {
findByAliasAndWorkspace: jest.fn(),
findByPageId: jest.fn(),
findById: jest.fn(),
insert: jest.fn(),
updateAlias: jest.fn(),
updatePageId: jest.fn(),
deleteOthersForPage: jest.fn(),
delete: jest.fn(),
};
const pageRepo = { findById: jest.fn() };
const shareService = {
resolveReadableSharePage: jest.fn(),
isSharingAllowed: jest.fn(),
};
// Default: the requester CAN view the target page (validateCanView resolves),
// so the reassign 409 may disclose its title. Tests override to reject to
// assert the no-leak path.
const pageAccessService = { validateCanView: jest.fn().mockResolvedValue(undefined) };
// Fake kysely db: only .transaction().execute(cb) is used by setAlias.
const db = {
transaction: jest.fn(() => ({
execute: jest.fn(async (cb: any) => cb(trx)),
})),
};
const service = new ShareAliasService(
shareAliasRepo as any,
pageRepo as any,
shareService as any,
pageAccessService as any,
db as any,
);
return { service, shareAliasRepo, pageRepo, shareService, pageAccessService, db };
}
describe('setAlias', () => {
it('rejects an invalid alias before touching the db', async () => {
const { service, shareAliasRepo } = makeService();
await expect(
service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'A', // too short + uppercase
}),
).rejects.toBeInstanceOf(BadRequestException);
expect(shareAliasRepo.findByAliasAndWorkspace).not.toHaveBeenCalled();
});
it('normalizes then inserts a brand-new alias (page has none yet)', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
shareAliasRepo.findByPageId.mockResolvedValue(undefined);
shareAliasRepo.insert.mockResolvedValue({ id: 'a-1', alias: 'my-page' });
const res = await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: ' My Page ',
});
expect(shareAliasRepo.findByAliasAndWorkspace).toHaveBeenCalledWith(
'my-page',
'ws-1',
trx,
);
expect(shareAliasRepo.insert).toHaveBeenCalledWith(
{
workspaceId: 'ws-1',
alias: 'my-page',
pageId: 'p-1',
creatorId: 'u-1',
},
trx,
);
expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled();
// self-heal still runs, keeping just the inserted row
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
'p-1',
'a-1',
'ws-1',
trx,
);
expect(res).toMatchObject({ id: 'a-1' });
});
it('renames the existing row in place when editing to a free name (te -> ted)', async () => {
const { service, shareAliasRepo } = makeService();
// The new slug is free...
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
// ...but the page already owns an alias named `te`.
shareAliasRepo.findByPageId.mockResolvedValue({
id: 'a-1',
alias: 'te',
pageId: 'p-1',
});
shareAliasRepo.updateAlias.mockResolvedValue({
id: 'a-1',
alias: 'ted',
pageId: 'p-1',
});
const res = await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'ted',
});
// RENAME, not INSERT a second row.
expect(shareAliasRepo.insert).not.toHaveBeenCalled();
expect(shareAliasRepo.updateAlias).toHaveBeenCalledWith(
'a-1',
'ted',
'ws-1',
trx,
);
// ...and any other row for the page is reaped, so `te` cannot survive.
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
'p-1',
'a-1',
'ws-1',
trx,
);
expect(res).toMatchObject({ id: 'a-1', alias: 'ted' });
});
it('is a no-op when the alias already points at the same page (and self-heals)', async () => {
const { service, shareAliasRepo } = makeService();
const existing = { id: 'a-1', alias: 'foo', pageId: 'p-1' };
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(existing);
const res = await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
});
expect(res).toBe(existing);
expect(shareAliasRepo.insert).not.toHaveBeenCalled();
expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled();
expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
// self-heal reaps any legacy duplicate rows for the page
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
'p-1',
'a-1',
'ws-1',
trx,
);
});
it('self-heals a page with pre-existing duplicate rows down to one', async () => {
const { service, shareAliasRepo } = makeService();
// Name free; the page already has a (legacy) alias row we rename.
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
shareAliasRepo.findByPageId.mockResolvedValue({
id: 'a-keep',
alias: 'old',
pageId: 'p-1',
});
shareAliasRepo.updateAlias.mockResolvedValue({
id: 'a-keep',
alias: 'new',
pageId: 'p-1',
});
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'new',
});
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
'p-1',
'a-keep',
'ws-1',
trx,
);
});
it('throws 409 with the target TITLE (never its id) when the requester CAN view it', async () => {
const { service, shareAliasRepo, pageRepo, pageAccessService } =
makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
alias: 'foo',
pageId: 'p-other',
});
pageRepo.findById.mockResolvedValue({ id: 'p-other', title: 'Other' });
pageAccessService.validateCanView.mockResolvedValue(undefined); // can view
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
});
fail('expected ConflictException');
} catch (err) {
expect(err).toBeInstanceOf(ConflictException);
const body = (err as ConflictException).getResponse();
expect(body).toMatchObject({
code: 'ALIAS_REASSIGN_REQUIRED',
currentPageTitle: 'Other',
});
// SECURITY (#495): the page id is NEVER disclosed, even to a viewer.
expect(body).not.toHaveProperty('currentPageId');
expect(pageAccessService.validateCanView).toHaveBeenCalledWith(
expect.objectContaining({ id: 'p-other' }),
USER,
);
}
expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
});
it('throws 409 WITHOUT the title or id when the requester CANNOT view the target (#495)', async () => {
const { service, shareAliasRepo, pageRepo, pageAccessService } =
makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
alias: 'foo',
pageId: 'p-secret',
});
pageRepo.findById.mockResolvedValue({ id: 'p-secret', title: 'Secret' });
// No view permission on the target page -> validateCanView throws.
pageAccessService.validateCanView.mockRejectedValue(
new ForbiddenException(),
);
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
});
fail('expected ConflictException');
} catch (err) {
expect(err).toBeInstanceOf(ConflictException);
const body = (err as ConflictException).getResponse() as Record<
string,
unknown
>;
expect(body).toMatchObject({ code: 'ALIAS_REASSIGN_REQUIRED' });
// The enumeration hole: neither the id nor the title of a page the
// requester cannot see may leak.
expect(body).not.toHaveProperty('currentPageId');
expect(body.currentPageTitle ?? null).toBeNull();
}
expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
});
it('retargets (UPDATE page_id) when confirmReassign is set', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
alias: 'foo',
pageId: 'p-other',
});
shareAliasRepo.updatePageId.mockResolvedValue({ id: 'a-1', pageId: 'p-1' });
const res = await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
confirmReassign: true,
});
expect(shareAliasRepo.updatePageId).toHaveBeenCalledWith(
'a-1',
'p-1',
'ws-1',
trx,
);
// ORDER MATTERS: the target page's existing alias row(s) are reaped BEFORE
// the retarget, so the non-deferrable (workspace_id, page_id) index never
// sees two rows for the page mid-statement. There is no trailing self-heal.
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith(
'p-1',
'a-1',
'ws-1',
trx,
);
expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledTimes(1);
const deleteOrder =
shareAliasRepo.deleteOthersForPage.mock.invocationCallOrder[0];
const updateOrder =
shareAliasRepo.updatePageId.mock.invocationCallOrder[0];
expect(deleteOrder).toBeLessThan(updateOrder);
expect(res).toMatchObject({ pageId: 'p-1' });
});
it('maps a unique-violation race (no constraint info) to 409 "Alias already taken"', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
shareAliasRepo.insert.mockRejectedValue({ code: '23505' });
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
});
fail('expected ConflictException');
} catch (err) {
expect(err).toBeInstanceOf(ConflictException);
expect((err as ConflictException).getResponse()).toMatchObject({
message: 'Alias already taken',
});
}
});
it('maps the (workspace_id, alias) index violation to "Alias already taken"', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
// postgres@3.x driver exposes the index name as `constraint_name`.
shareAliasRepo.insert.mockRejectedValue({
code: '23505',
constraint_name: 'share_aliases_workspace_id_alias_unique',
});
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
});
fail('expected ConflictException');
} catch (err) {
expect((err as ConflictException).getResponse()).toMatchObject({
message: 'Alias already taken',
});
}
});
it('maps the (workspace_id, page_id) index violation to a DISTINCT page-race outcome', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
shareAliasRepo.insert.mockRejectedValue({
code: '23505',
constraint_name: 'share_aliases_workspace_id_page_id_unique',
});
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
});
fail('expected ConflictException');
} catch (err) {
expect(err).toBeInstanceOf(ConflictException);
// NOT the misleading "Alias already taken" — a separate, page-scoped code.
expect((err as ConflictException).getResponse()).toMatchObject({
code: 'ALIAS_PAGE_RACE',
});
expect((err as ConflictException).getResponse()).not.toMatchObject({
message: 'Alias already taken',
});
}
});
it('reads the index name from `.constraint` when `.constraint_name` is absent', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
// Fallback path for non-postgres@3.x drivers.
shareAliasRepo.insert.mockRejectedValue({
code: '23505',
constraint: 'share_aliases_workspace_id_page_id_unique',
});
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
});
fail('expected ConflictException');
} catch (err) {
expect((err as ConflictException).getResponse()).toMatchObject({
code: 'ALIAS_PAGE_RACE',
});
}
});
it('maps a concurrent-delete race in the SWAP branch to a retryable 409 (not a 200-without-alias)', async () => {
const { service, shareAliasRepo } = makeService();
// Name points at another page; reassign confirmed -> swap branch.
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
alias: 'foo',
pageId: 'p-other',
});
// A concurrent removeAlias deleted the row between read and UPDATE, so the
// repo's executeTakeFirstOrThrow finds 0 rows and throws NoResultError.
shareAliasRepo.updatePageId.mockRejectedValue(
new NoResultError({} as any),
);
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
confirmReassign: true,
});
fail('expected ConflictException');
} catch (err) {
// Crucially NOT a resolved 200 carrying `undefined` as the alias.
expect(err).toBeInstanceOf(ConflictException);
expect((err as ConflictException).getResponse()).toMatchObject({
code: 'ALIAS_PAGE_RACE',
});
}
});
it('maps a concurrent-delete race in the RENAME branch to a retryable 409 (not a generic 400)', async () => {
const { service, shareAliasRepo } = makeService();
// New slug is free, but the page already owns an alias we rename in place.
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
shareAliasRepo.findByPageId.mockResolvedValue({
id: 'a-1',
alias: 'te',
pageId: 'p-1',
});
// The row vanished before the UPDATE; repo throws NoResultError rather
// than returning undefined (which would dereference undefined.id -> 400).
shareAliasRepo.updateAlias.mockRejectedValue(new NoResultError({} as any));
try {
await service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'ted',
});
fail('expected ConflictException');
} catch (err) {
expect(err).toBeInstanceOf(ConflictException);
expect(err).not.toBeInstanceOf(BadRequestException);
expect((err as ConflictException).getResponse()).toMatchObject({
code: 'ALIAS_PAGE_RACE',
});
}
});
it('maps a non-unique-violation db error to BadRequest (Failed to set alias)', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
shareAliasRepo.insert.mockRejectedValue({ code: '08006' }); // connection error
await expect(
service.setAlias({
workspaceId: 'ws-1',
pageId: 'p-1',
creatorId: 'u-1',
user: USER,
alias: 'foo',
}),
).rejects.toBeInstanceOf(BadRequestException);
});
});
describe('checkAvailability', () => {
it('reports invalid for a bad slug without a db hit', async () => {
const { service, shareAliasRepo } = makeService();
const res = await service.checkAvailability('Bad Slug!', 'ws-1');
expect(res).toMatchObject({ valid: false, available: false });
expect(shareAliasRepo.findByAliasAndWorkspace).not.toHaveBeenCalled();
});
it('reports available when no row exists', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined);
const res = await service.checkAvailability('free-name', 'ws-1');
expect(res).toMatchObject({
alias: 'free-name',
valid: true,
available: true,
});
// SECURITY (#495): the availability probe must NOT leak any page id.
expect(res).not.toHaveProperty('currentPageId');
});
it('reports taken WITHOUT leaking the current target page id (#495)', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
pageId: 'p-9',
});
const res = await service.checkAvailability('taken', 'ws-1');
expect(res).toMatchObject({ available: false });
// The row exists (available:false) but its pageId is never returned — an
// authenticated member cannot map an alias name to a page id it can't view.
expect(res).not.toHaveProperty('currentPageId');
});
});
describe('resolveReadableTarget', () => {
it('returns null for an invalid alias', async () => {
const { service } = makeService();
expect(await service.resolveReadableTarget('!!', 'ws-1')).toBeNull();
});
it('returns null for an unknown or dangling alias', async () => {
const { service, shareAliasRepo } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValueOnce(undefined);
expect(await service.resolveReadableTarget('foo', 'ws-1')).toBeNull();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValueOnce({
id: 'a-1',
pageId: null,
});
expect(await service.resolveReadableTarget('foo', 'ws-1')).toBeNull();
});
it('returns null when the page is no longer publicly readable', async () => {
const { service, shareAliasRepo, shareService } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
pageId: 'p-1',
});
shareService.resolveReadableSharePage.mockResolvedValue(null);
expect(await service.resolveReadableTarget('foo', 'ws-1')).toBeNull();
});
it('returns null when sharing is disabled for the space', async () => {
const { service, shareAliasRepo, shareService } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
pageId: 'p-1',
});
shareService.resolveReadableSharePage.mockResolvedValue({
share: { key: 'k', spaceId: 's-1' },
page: { slugId: 'sid', title: 'T' },
});
shareService.isSharingAllowed.mockResolvedValue(false);
expect(await service.resolveReadableTarget('foo', 'ws-1')).toBeNull();
});
it('returns the resolved share+page on success', async () => {
const { service, shareAliasRepo, shareService } = makeService();
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
id: 'a-1',
pageId: 'p-1',
});
const resolved = {
share: { key: 'k', spaceId: 's-1' },
page: { slugId: 'sid', title: 'T' },
};
shareService.resolveReadableSharePage.mockResolvedValue(resolved);
shareService.isSharingAllowed.mockResolvedValue(true);
const res = await service.resolveReadableTarget('FOO', 'ws-1');
expect(res).toBe(resolved);
// alias was normalized to lowercase before lookup
expect(shareAliasRepo.findByAliasAndWorkspace).toHaveBeenCalledWith(
'foo',
'ws-1',
);
});
});
});