search-backend: update to treat requests without credentials as unauthenticated

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/tiny-pianos-work.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-search-backend': patch
+---
+
+The `AuthorizedSearchEngine` will now ignore the deprecated `token` option, and treat it as an unauthorized request. This will not have any effect in practice, since credentials are always provided by the router.

plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts

@@ -32,6 +32,7 @@ import {
   decodePageCursor,
   AuthorizedSearchEngine,
 } from './AuthorizedSearchEngine';
+import { mockCredentials, mockServices } from '@backstage/backend-test-utils';
 
 describe('AuthorizedSearchEngine', () => {
   const typeUsers = 'users';
@@ -117,10 +118,11 @@ describe('AuthorizedSearchEngine', () => {
     searchEngine,
     defaultTypes,
     permissionEvaluator,
+    mockServices.auth(),
     new ConfigReader({}),
   );
 
-  const options = { token: 'token' };
+  const options = { credentials: mockCredentials.user() };
 
   const allowAll: PermissionEvaluator['authorize'] &
     PermissionEvaluator['authorizeConditional'] = async queries => {
@@ -149,7 +151,7 @@ describe('AuthorizedSearchEngine', () => {
         types: ['one', 'two'],
         filters,
       },
-      { token: 'token' },
+      options,
     );
   });
 
@@ -160,7 +162,18 @@ describe('AuthorizedSearchEngine', () => {
     await authorizedSearchEngine.query({ term: '' }, options);
     expect(mockedQuery).toHaveBeenCalledWith(
       { term: '', types: ['users', 'templates', 'services', 'groups'] },
-      { token: 'token' },
+      options,
+    );
+  });
+
+  it('should fall back to none credentials if a token is provided', async () => {
+    mockedQuery.mockImplementation(async () => ({ results }));
+    mockedPermissionQuery.mockImplementation(allowAll);
+
+    await authorizedSearchEngine.query({ term: '' }, { token: 'token' });
+    expect(mockedQuery).toHaveBeenCalledWith(
+      { term: '', types: ['users', 'templates', 'services', 'groups'] },
+      { credentials: mockCredentials.none() },
     );
   });
 
@@ -184,7 +197,7 @@ describe('AuthorizedSearchEngine', () => {
     );
     expect(mockedQuery).toHaveBeenCalledWith(
       { term: '', types: ['users', 'templates'] },
-      { token: 'token' },
+      options,
     );
     expect(mockedPermissionQuery).toHaveBeenCalledTimes(1);
     expect(mockedPermissionQuery).toHaveBeenLastCalledWith(
@@ -192,7 +205,7 @@ describe('AuthorizedSearchEngine', () => {
         { permission: defaultTypes[typeUsers].visibilityPermission },
         { permission: defaultTypes[typeTemplates].visibilityPermission },
       ],
-      { token: 'token' },
+      options,
     );
   });
 
@@ -218,7 +231,7 @@ describe('AuthorizedSearchEngine', () => {
 
     expect(mockedQuery).toHaveBeenCalledWith(
       { term: '', types: ['templates', 'services', 'groups'] },
-      { token: 'token' },
+      options,
     );
 
     expect(mockedPermissionQuery).toHaveBeenCalledTimes(1);
@@ -270,7 +283,7 @@ describe('AuthorizedSearchEngine', () => {
 
     expect(mockedQuery).toHaveBeenCalledWith(
       { term: '', types: ['users'] },
-      { token: 'token' },
+      options,
     );
   });
 
@@ -326,7 +339,7 @@ describe('AuthorizedSearchEngine', () => {
           }),
         },
       ],
-      { token: 'token' },
+      options,
     );
     expect(mockedAuthorize).toHaveBeenCalledTimes(1);
     expect(mockedAuthorize).toHaveBeenCalledWith(
@@ -338,7 +351,7 @@ describe('AuthorizedSearchEngine', () => {
           resourceRef: 'template_doc_0',
         },
       ],
-      { token: 'token' },
+      options,
     );
   });
 
@@ -403,7 +416,7 @@ describe('AuthorizedSearchEngine', () => {
     expect(mockedQuery).toHaveBeenNthCalledWith(
       1,
       { term: '', types: ['users', 'templates', 'services'] },
-      { token: 'token' },
+      options,
     );
     expect(mockedQuery).toHaveBeenNthCalledWith(
       2,
@@ -412,7 +425,7 @@ describe('AuthorizedSearchEngine', () => {
         types: ['users', 'templates', 'services'],
         pageCursor: 'MQ==',
       },
-      { token: 'token' },
+      options,
     );
     expect(mockedQuery).toHaveBeenNthCalledWith(
       3,
@@ -421,7 +434,7 @@ describe('AuthorizedSearchEngine', () => {
         types: ['users', 'templates', 'services'],
         pageCursor: 'Mg==',
       },
-      { token: 'token' },
+      options,
     );
 
     const expectedResult = allDocuments
@@ -494,7 +507,7 @@ describe('AuthorizedSearchEngine', () => {
     expect(mockedQuery).toHaveBeenNthCalledWith(
       1,
       { term: '', types: ['users', 'templates', 'services', 'groups'] },
-      { token: 'token' },
+      options,
     );
     expect(mockedQuery).toHaveBeenNthCalledWith(
       2,
@@ -503,7 +516,7 @@ describe('AuthorizedSearchEngine', () => {
         types: ['users', 'templates', 'services', 'groups'],
         pageCursor: 'MQ==',
       },
-      { token: 'token' },
+      options,
     );
     expect(mockedQuery).toHaveBeenNthCalledWith(
       3,
@@ -512,7 +525,7 @@ describe('AuthorizedSearchEngine', () => {
         types: ['users', 'templates', 'services', 'groups'],
         pageCursor: 'Mg==',
       },
-      { token: 'token' },
+      options,
     );
 
     const expectedResult = allDocuments
@@ -574,7 +587,7 @@ describe('AuthorizedSearchEngine', () => {
     expect(mockedQuery).toHaveBeenNthCalledWith(
       1,
       { term: '', types: ['users', 'templates', 'services'] },
-      { token: 'token' },
+      options,
     );
     expect(mockedQuery).toHaveBeenNthCalledWith(
       2,
@@ -583,7 +596,7 @@ describe('AuthorizedSearchEngine', () => {
         types: ['users', 'templates', 'services'],
         pageCursor: 'MQ==',
       },
-      { token: 'token' },
+      options,
     );
     expect(mockedQuery).toHaveBeenNthCalledWith(
       3,
@@ -592,7 +605,7 @@ describe('AuthorizedSearchEngine', () => {
         types: ['users', 'templates', 'services'],
         pageCursor: 'Mg==',
       },
-      { token: 'token' },
+      options,
     );
 
     const expectedResults = servicesWithAuth

plugins/search-backend/src/service/AuthorizedSearchEngine.ts

@@ -39,7 +39,7 @@ import {
 import { Config } from '@backstage/config';
 import { InputError } from '@backstage/errors';
 import { Writable } from 'stream';
-import { PermissionsService } from '@backstage/backend-plugin-api';
+import { AuthService, PermissionsService } from '@backstage/backend-plugin-api';
 
 export function decodePageCursor(pageCursor?: string): { page: number } {
   if (!pageCursor) {
@@ -71,6 +71,7 @@ export class AuthorizedSearchEngine implements SearchEngine {
     private readonly searchEngine: SearchEngine,
     private readonly types: Record<string, DocumentTypeInfo>,
     private readonly permissions: PermissionsService,
+    private readonly auth: AuthService,
     config: Config,
   ) {
     this.queryLatencyBudgetMs =
@@ -92,9 +93,14 @@ export class AuthorizedSearchEngine implements SearchEngine {
   ): Promise<IndexableResultSet> {
     const queryStartTime = Date.now();
 
+    const compatOptions =
+      'credentials' in options
+        ? options
+        : { credentials: await this.auth.getNoneCredentials() };
+
     const conditionFetcher = new DataLoader(
       (requests: readonly QueryPermissionRequest[]) =>
-        this.permissions.authorizeConditional(requests.slice(), options),
+        this.permissions.authorizeConditional(requests.slice(), compatOptions),
       {
         cacheKeyFn: ({ permission: { name } }) => name,
       },
@@ -102,7 +108,7 @@ export class AuthorizedSearchEngine implements SearchEngine {
 
     const authorizer = new DataLoader(
       (requests: readonly AuthorizePermissionRequest[]) =>
-        this.permissions.authorize(requests.slice(), options),
+        this.permissions.authorize(requests.slice(), compatOptions),
       {
         // Serialize the permission name and resourceRef as
         // a query string to avoid collisions from overlapping
@@ -159,7 +165,7 @@ export class AuthorizedSearchEngine implements SearchEngine {
     if (!resultByResultFilteringRequired) {
       return this.searchEngine.query(
         { ...query, types: authorizedTypes },
-        options,
+        compatOptions,
       );
     }
 
@@ -174,7 +180,7 @@ export class AuthorizedSearchEngine implements SearchEngine {
     do {
       const nextPage = await this.searchEngine.query(
         { ...query, types: authorizedTypes, pageCursor: nextPageCursor },
-        options,
+        compatOptions,
       );
 
       filteredResults = filteredResults.concat(

plugins/search-backend/src/service/router.ts

@@ -145,6 +145,7 @@ export async function createRouter(
         inputEngine,
         types,
         permissionEvaluator,
+        auth,
         config,
       )
     : inputEngine;