Merge pull request #10740 from growilabs/fix/177549-block-user-page-searches

fix: Exclude user page data from search response when user pages are disabled

apps/app/src/features/search/utils/disable-user-pages.spec.ts

@@ -0,0 +1,150 @@
+import type { QueryTerms } from '~/server/interfaces/search';
+
+import { excludeUserPagesFromQuery } from './disable-user-pages';
+
+describe('excludeUserPagesFromQuery()', () => {
+  it('should exclude user page strings from query prefix', () => {
+    const userString = '/user';
+    const userStringSlash = '/user/';
+    const userStringSubPath = '/user/settings';
+    const userStringSubPathDeep = '/user/profile/edit';
+    const userStringSubPathQuery = '/user/settings?ref=top';
+
+    const query: QueryTerms = {
+      match: [],
+      not_match: [],
+      phrase: [],
+      not_phrase: [],
+      prefix: [
+        userString,
+        userStringSlash,
+        userStringSubPath,
+        userStringSubPathDeep,
+        userStringSubPathQuery,
+      ],
+      not_prefix: [],
+      tag: [],
+      not_tag: [],
+    };
+
+    excludeUserPagesFromQuery(query);
+
+    expect(query.prefix).not.toContain(userString);
+    // Should only contain '/user'
+    expect(query.not_prefix).toContain(userString);
+
+    expect(query.prefix).not.toContain(userStringSlash);
+    expect(query.not_prefix).not.toContain(userStringSlash);
+
+    expect(query.prefix).not.toContain(userStringSubPath);
+    expect(query.not_prefix).not.toContain(userStringSubPath);
+
+    expect(query.prefix).not.toContain(userStringSubPathDeep);
+    expect(query.not_prefix).not.toContain(userStringSubPathDeep);
+
+    expect(query.prefix).not.toContain(userStringSubPathQuery);
+    expect(query.not_prefix).not.toContain(userStringSubPathQuery);
+  });
+
+  it('should not exclude strings similar to /user from query prefix', () => {
+    const userRouter = '/userouter';
+    const userRoot = '/useroot';
+    const userSettings = '/user-settings';
+    const apiUser = '/api/user';
+    const emptyString = '';
+    const rootOnly = '/';
+    const upperCase = '/USER';
+    const doubleSlashStart = '//user/new';
+    const doubleSlashSub = '/user//new';
+
+    const query: QueryTerms = {
+      match: [],
+      not_match: [],
+      phrase: [],
+      not_phrase: [],
+      prefix: [
+        userRouter,
+        userRoot,
+        userSettings,
+        apiUser,
+        emptyString,
+        rootOnly,
+        upperCase,
+        doubleSlashStart,
+        doubleSlashSub,
+      ],
+      not_prefix: [],
+      tag: [],
+      not_tag: [],
+    };
+
+    excludeUserPagesFromQuery(query);
+
+    expect(query.prefix).toContain(userRouter);
+    expect(query.not_prefix).not.toContain(userRouter);
+
+    expect(query.prefix).toContain(userRoot);
+    expect(query.not_prefix).not.toContain(userRoot);
+
+    expect(query.prefix).toContain(userSettings);
+    expect(query.not_prefix).not.toContain(userSettings);
+
+    expect(query.prefix).toContain(apiUser);
+    expect(query.not_prefix).not.toContain(apiUser);
+
+    expect(query.prefix).toContain(emptyString);
+    expect(query.not_prefix).not.toContain(emptyString);
+
+    expect(query.prefix).toContain(rootOnly);
+    expect(query.not_prefix).not.toContain(rootOnly);
+
+    expect(query.prefix).toContain(upperCase);
+    expect(query.not_prefix).not.toContain(upperCase);
+
+    expect(query.prefix).toContain(doubleSlashStart);
+    expect(query.not_prefix).not.toContain(doubleSlashStart);
+
+    expect(query.prefix).toContain(doubleSlashSub);
+    expect(query.not_prefix).not.toContain(doubleSlashSub);
+  });
+
+  it('should add /user to not_prefix when it is empty', () => {
+    const query: QueryTerms = {
+      match: [],
+      not_match: [],
+      phrase: [],
+      not_phrase: [],
+      prefix: [],
+      not_prefix: [],
+      tag: [],
+      not_tag: [],
+    };
+
+    excludeUserPagesFromQuery(query);
+
+    expect(query.prefix).toHaveLength(0);
+    expect(query.not_prefix).toContain('/user');
+    expect(query.not_prefix).toHaveLength(1);
+  });
+
+  it('should remove existing /user strings and leave not_prefix with just one /user string', () => {
+    const userString = '/user';
+
+    const query: QueryTerms = {
+      match: [],
+      not_match: [],
+      phrase: [],
+      not_phrase: [],
+      prefix: [userString, userString],
+      not_prefix: [userString, userString],
+      tag: [],
+      not_tag: [],
+    };
+
+    excludeUserPagesFromQuery(query);
+
+    expect(query.prefix).toHaveLength(0);
+    expect(query.not_prefix).toContain('/user');
+    expect(query.not_prefix).toHaveLength(1);
+  });
+});

apps/app/src/features/search/utils/disable-user-pages.ts

@@ -0,0 +1,10 @@
+import type { QueryTerms } from '~/server/interfaces/search';
+
+export function excludeUserPagesFromQuery(terms: QueryTerms): void {
+  const userRegex: RegExp = /^\/user($|\/(?!\/))/;
+
+  terms.prefix = terms.prefix.filter((p) => !userRegex.test(p));
+  terms.not_prefix = terms.not_prefix.filter((p) => !userRegex.test(p));
+
+  terms.not_prefix.push('/user');
+}

apps/app/src/server/service/search-query.spec.ts

@@ -0,0 +1,150 @@
+import { vi } from 'vitest';
+import { type MockProxy, mock } from 'vitest-mock-extended';
+
+import { SearchDelegatorName } from '~/interfaces/named-query';
+import type Crowi from '~/server/crowi';
+import { configManager } from '~/server/service/config-manager/config-manager';
+
+import type { SearchDelegator } from '../interfaces/search';
+import NamedQuery from '../models/named-query';
+import SearchService from './search';
+import type ElasticsearchDelegator from './search-delegator/elasticsearch';
+
+// Mock NamedQuery
+vi.mock('~/server/models/named-query', () => {
+  const mockModel = {
+    findOne: vi.fn(),
+  };
+  return {
+    NamedQuery: mockModel,
+    default: mockModel,
+  };
+});
+
+// Mock config manager
+vi.mock('~/server/service/config-manager/config-manager', () => {
+  return {
+    default: {
+      getConfig: vi.fn(),
+    },
+    configManager: {
+      getConfig: vi.fn(),
+    },
+  };
+});
+
+class TestSearchService extends SearchService {
+  override generateFullTextSearchDelegator(): ElasticsearchDelegator {
+    return mock<ElasticsearchDelegator>();
+  }
+
+  override generateNQDelegators(): {
+    [key in SearchDelegatorName]: SearchDelegator;
+  } {
+    return {
+      [SearchDelegatorName.DEFAULT]: mock<SearchDelegator>(),
+      [SearchDelegatorName.PRIVATE_LEGACY_PAGES]: mock<SearchDelegator>(),
+    };
+  }
+
+  override registerUpdateEvent(): void {}
+
+  override get isConfigured(): boolean {
+    return false;
+  }
+}
+
+describe('searchParseQuery()', () => {
+  let searchService: TestSearchService;
+  let mockCrowi: MockProxy<Crowi>;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    mockCrowi = mock<Crowi>();
+    mockCrowi.configManager = configManager;
+    searchService = new TestSearchService(mockCrowi);
+  });
+
+  it('should contain /user in the not_prefix query when user pages are disabled', async () => {
+    vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
+      if (key === 'security:disableUserPages') {
+        return true;
+      }
+
+      return false;
+    });
+
+    const result = await searchService.parseSearchQuery('/user/settings', null);
+
+    expect(configManager.getConfig).toHaveBeenCalledWith(
+      'security:disableUserPages',
+    );
+    expect(result.terms.not_prefix).toContain('/user');
+    expect(result.terms.prefix).toHaveLength(0);
+  });
+
+  it('should contain /user in the not_prefix even when search query is not a user page', async () => {
+    vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
+      if (key === 'security:disableUserPages') {
+        return true;
+      }
+
+      return false;
+    });
+
+    const result = await searchService.parseSearchQuery('/new-task', null);
+
+    expect(configManager.getConfig).toHaveBeenCalledWith(
+      'security:disableUserPages',
+    );
+    expect(result.terms.not_prefix).toContain('/user');
+    expect(result.terms.prefix).toHaveLength(0);
+  });
+
+  it('should add specific user prefixes in the query when user pages are enabled', async () => {
+    vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
+      if (key === 'security:disableUserPages') {
+        return false;
+      }
+
+      return true;
+    });
+
+    const result = await searchService.parseSearchQuery('/user/settings', null);
+
+    expect(configManager.getConfig).toHaveBeenCalledWith(
+      'security:disableUserPages',
+    );
+    expect(result.terms.not_prefix).not.toContain('/user');
+    expect(result.terms.not_prefix).not.toContain('/user/settings');
+    expect(result.terms.match).toContain('/user/settings');
+  });
+
+  it('should filter user pages even when resolved from a named query alias', async () => {
+    vi.mocked(configManager.getConfig).mockImplementation((key: string) => {
+      if (key === 'security:disableUserPages') {
+        return true;
+      }
+
+      return false;
+    });
+
+    const shortcutName = 'my-shortcut';
+    const aliasPath = '/user/my-private-page';
+
+    // Mock the DB response
+    vi.mocked(NamedQuery.findOne).mockResolvedValue({
+      name: shortcutName,
+      aliasOf: aliasPath,
+    });
+
+    const result = await searchService.parseSearchQuery('dummy', shortcutName);
+
+    expect(configManager.getConfig).toHaveBeenCalledWith(
+      'security:disableUserPages',
+    );
+    expect(result.terms.not_prefix).toContain('/user');
+    expect(result.terms.match).toContain('/user/my-private-page');
+  });
+});

apps/app/src/server/service/search.ts

@@ -8,6 +8,7 @@ import {
   isIncludeAiMenthion,
   removeAiMenthion,
 } from '~/features/search/utils/ai';
+import { excludeUserPagesFromQuery } from '~/features/search/utils/disable-user-pages';
 import { SearchDelegatorName } from '~/interfaces/named-query';
 import type {
   IFormattedSearchResult,
@@ -328,34 +329,37 @@ class SearchService implements SearchQueryParser, SearchResolver {
     _queryString: string,
     nqName: string | null,
   ): Promise<ParsedQuery> {
+    const disableUserPages = configManager.getConfig(
+      'security:disableUserPages',
+    );
     const queryString = normalizeQueryString(_queryString);
-
     const terms = this.parseQueryString(queryString);
 
-    if (nqName == null) {
-      return { queryString, terms };
-    }
+    let parsedQuery: ParsedQuery = { queryString, terms };
 
-    const nq = await NamedQuery.findOne({ name: normalizeNQName(nqName) });
+    if (nqName != null) {
+      const nq = await NamedQuery.findOne({ name: normalizeNQName(nqName) });
 
-    // will delegate to full-text search
-    if (nq == null) {
-      logger.debug(
-        `Delegated to full-text search since a named query document did not found. (nqName="${nqName}")`,
-      );
-      return { queryString, terms };
-    }
+      if (nq != null) {
+        const { aliasOf, delegatorName } = nq;
 
-    const { aliasOf, delegatorName } = nq;
+        if (aliasOf != null) {
+          parsedQuery = {
+            queryString: normalizeQueryString(aliasOf),
+            terms: this.parseQueryString(aliasOf),
+          };
+        } else {
+          parsedQuery = { queryString, terms, delegatorName };
+        }
+      } else {
+        logger.debug(
+          `Delegated to full-text search since a named query document did not found. (nqName="${nqName}")`,
+        );
+      }
+    }
 
-    let parsedQuery: ParsedQuery;
-    if (aliasOf != null) {
-      parsedQuery = {
-        queryString: normalizeQueryString(aliasOf),
-        terms: this.parseQueryString(aliasOf),
-      };
-    } else {
-      parsedQuery = { queryString, terms, delegatorName };
+    if (disableUserPages) {
+      excludeUserPagesFromQuery(parsedQuery.terms);
     }
 
     return parsedQuery;