playlist-backend: migrate to support new auth services

Co-authored-by: Fredrik Adelöw <freben@gmail.com> Co-authored-by: Carl-Erik Bergström <cbergstrom@spotify.com> Co-authored-by: blam <ben@blam.sh> Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
2024-02-15 18:38:00 +01:00
parent 58b5e450dd
commit 68133666c6
5 changed files with 88 additions and 77 deletions
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-playlist-backend': patch
+---
+
+Migrated to support new auth services.
@@ -3,19 +3,21 @@
 > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).

 ```ts
+import { AuthService } from '@backstage/backend-plugin-api';
 import { BackendFeature } from '@backstage/backend-plugin-api';
 import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
 import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common';
 import { Conditions } from '@backstage/plugin-permission-node';
 import express from 'express';
+import { HttpAuthService } from '@backstage/backend-plugin-api';
 import { IdentityApi } from '@backstage/plugin-auth-node';
 import { Logger } from 'winston';
 import { Permission } from '@backstage/plugin-permission-common';
 import { PermissionCondition } from '@backstage/plugin-permission-common';
 import { PermissionCriteria } from '@backstage/plugin-permission-common';
-import { PermissionEvaluator } from '@backstage/plugin-permission-common';
 import { PermissionPolicy } from '@backstage/plugin-permission-node';
 import { PermissionRule } from '@backstage/plugin-permission-node';
+import { PermissionsService } from '@backstage/backend-plugin-api';
 import { PlaylistMetadata } from '@backstage/plugin-playlist-common';
 import { PluginDatabaseManager } from '@backstage/backend-common';
 import { PluginEndpointDiscovery } from '@backstage/backend-common';
@@ -87,15 +89,19 @@ export default playlistPlugin;

 // @public (undocumented)
 export interface RouterOptions {
+  // (undocumented)
+  auth?: AuthService;
  // (undocumented)
  database: PluginDatabaseManager;
  // (undocumented)
  discovery: PluginEndpointDiscovery;
  // (undocumented)
+  httpAuth?: HttpAuthService;
+  // (undocumented)
  identity: IdentityApi;
  // (undocumented)
  logger: Logger;
  // (undocumented)
-  permissions: PermissionEvaluator;
+  permissions: PermissionsService;
 }
 ```
@@ -15,7 +15,7 @@
 */

 import { resolvePackagePath } from '@backstage/backend-common';
-import { BackstageUserIdentity } from '@backstage/plugin-auth-node';
+import { BackstageUserPrincipal } from '@backstage/backend-plugin-api';
 import { Playlist, PlaylistMetadata } from '@backstage/plugin-playlist-common';
 import { Knex } from 'knex';
 import { v4 as uuid } from 'uuid';
@@ -108,7 +108,7 @@ export class DatabaseHandler {
  private playlistColumns = ['id', 'name', 'description', 'owner', 'public'];

  async listPlaylists(
-    user: BackstageUserIdentity,
+    user: BackstageUserPrincipal,
    filter?: ListPlaylistsFilter,
  ): Promise<Playlist[]> {
    let playlistQuery = this.database<Omit<Playlist, 'isFollowing'>>(
@@ -177,7 +177,7 @@ export class DatabaseHandler {

  async getPlaylist(
    id: string,
-    user?: BackstageUserIdentity,
+    user?: BackstageUserPrincipal,
  ): Promise<Playlist | undefined> {
    const playlist = await this.database<Omit<Playlist, 'isFollowing'>>(
      'playlists',
@@ -263,14 +263,14 @@ export class DatabaseHandler {
      .del();
  }

-  async followPlaylist(playlistId: string, user: BackstageUserIdentity) {
+  async followPlaylist(playlistId: string, user: BackstageUserPrincipal) {
    await this.database('followers')
      .insert({ playlist_id: playlistId, user_ref: user.userEntityRef })
      .onConflict(['playlist_id', 'user_ref'])
      .ignore();
  }

-  async unfollowPlaylist(playlistId: string, user: BackstageUserIdentity) {
+  async unfollowPlaylist(playlistId: string, user: BackstageUserPrincipal) {
    await this.database('followers')
      .where({ playlist_id: playlistId, user_ref: user.userEntityRef })
      .del();
@@ -20,13 +20,13 @@ import {
  PluginEndpointDiscovery,
 } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
-import { IdentityApi } from '@backstage/plugin-auth-node';
 import { AuthorizeResult } from '@backstage/plugin-permission-common';
 import { permissions } from '@backstage/plugin-playlist-common';
 import express from 'express';
 import request from 'supertest';

 import { createRouter } from './router';
+import { mockCredentials, mockServices } from '@backstage/backend-test-utils';

 const sampleEntities = [
  {
@@ -60,10 +60,6 @@ jest.mock('@backstage/catalog-client', () => ({
    .mockImplementation(() => ({ getEntities: mockGetEntties })),
 }));

-jest.mock('@backstage/plugin-auth-node', () => ({
-  getBearerTokenFromAuthorizationHeader: () => 'token',
-}));
-
 const mockConditionFilter = { key: 'test', values: ['test-val'] };
 jest.mock('../permissions', () => ({
  ...jest.requireActual('../permissions'),
@@ -128,17 +124,6 @@ describe('createRouter', () => {
    authorizeConditional: mockedAuthorizeConditional,
  };

-  const mockUser = {
-    type: 'user',
-    ownershipEntityRefs: ['user:default/me', 'group:default/owner'],
-    userEntityRef: 'user:default/me',
-  };
-  const mockIdentityClient = {
-    getIdentity: jest
-      .fn()
-      .mockImplementation(async () => ({ identity: mockUser })),
-  } as unknown as IdentityApi;
-
  const discovery: jest.Mocked<PluginEndpointDiscovery> = {
    getBaseUrl: jest.fn(),
    getExternalBaseUrl: jest.fn(),
@@ -148,9 +133,11 @@ describe('createRouter', () => {
    const router = await createRouter({
      database: createDatabase(),
      discovery,
-      identity: mockIdentityClient,
+      identity: mockServices.identity(),
      logger: getVoidLogger(),
      permissions: mockPermissionEvaluator,
+      auth: mockServices.auth(),
+      httpAuth: mockServices.httpAuth(),
    });

    app = express().use(router);
@@ -173,7 +160,7 @@ describe('createRouter', () => {

      expect(mockedAuthorizeConditional).toHaveBeenCalledWith(
        [{ permission: permissions.playlistListRead }],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.listPlaylists).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -182,7 +169,7 @@ describe('createRouter', () => {
    it('should get playlists correctly', async () => {
      let response = await request(app).get('/').send();
      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
-        mockUser,
+        mockCredentials.user().principal,
        undefined,
      );
      expect(response.status).toEqual(200);
@@ -193,7 +180,7 @@ describe('createRouter', () => {
      ]);
      response = await request(app).get('/').send();
      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
-        mockUser,
+        mockCredentials.user().principal,
        mockConditionFilter,
      );
      expect(response.status).toEqual(200);
@@ -205,7 +192,7 @@ describe('createRouter', () => {
        .get('/?filter=mock=test&filter=foo=bar')
        .send();
      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
-        mockUser,
+        mockCredentials.user().principal,
        mockRequestFilter,
      );
      expect(response.status).toEqual(200);
@@ -217,9 +204,12 @@ describe('createRouter', () => {
      response = await request(app)
        .get('/?filter=mock=test&filter=foo=bar')
        .send();
-      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(mockUser, {
-        allOf: [mockRequestFilter, mockConditionFilter],
-      });
+      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
+        mockCredentials.user().principal,
+        {
+          allOf: [mockRequestFilter, mockConditionFilter],
+        },
+      );
      expect(response.status).toEqual(200);
      expect(response.body).toEqual([mockPlaylist]);
    });
@@ -228,10 +218,10 @@ describe('createRouter', () => {
      let response = await request(app).get('/?editable=true').send();
      expect(mockedAuthorizeConditional).toHaveBeenCalledWith(
        [{ permission: permissions.playlistListUpdate }],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
-        mockUser,
+        mockCredentials.user().principal,
        undefined,
      );
      expect(response.status).toEqual(200);
@@ -241,7 +231,7 @@ describe('createRouter', () => {
        .get('/?editable=true&filter=mock=test&filter=foo=bar')
        .send();
      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
-        mockUser,
+        mockCredentials.user().principal,
        mockRequestFilter,
      );
      expect(response.status).toEqual(200);
@@ -251,9 +241,10 @@ describe('createRouter', () => {
        { result: AuthorizeResult.CONDITIONAL },
      ]);
      response = await request(app).get('/?editable=true').send();
-      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(mockUser, {
-        allOf: [mockConditionFilter, mockConditionFilter],
-      });
+      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
+        mockCredentials.user().principal,
+        { allOf: [mockConditionFilter, mockConditionFilter] },
+      );
      expect(response.status).toEqual(200);
      expect(response.body).toEqual([mockPlaylist]);

@@ -263,12 +254,15 @@ describe('createRouter', () => {
      response = await request(app)
        .get('/?editable=true&filter=mock=test&filter=foo=bar')
        .send();
-      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(mockUser, {
-        allOf: [
-          { allOf: [mockRequestFilter, mockConditionFilter] },
-          mockConditionFilter,
-        ],
-      });
+      expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(
+        mockCredentials.user().principal,
+        {
+          allOf: [
+            { allOf: [mockRequestFilter, mockConditionFilter] },
+            mockConditionFilter,
+          ],
+        },
+      );
      expect(response.status).toEqual(200);
      expect(response.body).toEqual([mockPlaylist]);
    });
@@ -285,7 +279,7 @@ describe('createRouter', () => {

      expect(mockedAuthorize).toHaveBeenCalledWith(
        [{ permission: permissions.playlistListCreate }],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.createPlaylist).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -313,7 +307,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.getPlaylist).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -323,7 +317,7 @@ describe('createRouter', () => {
      const response = await request(app).get('/playlist-id').send();
      expect(mockDbHandler.getPlaylist).toHaveBeenCalledWith(
        'playlist-id',
-        mockUser,
+        mockCredentials.user().principal,
      );
      expect(response.status).toEqual(200);
      expect(response.body).toEqual(mockPlaylist);
@@ -346,7 +340,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.updatePlaylist).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -375,7 +369,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.deletePlaylist).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -404,7 +398,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.addPlaylistEntities).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -436,7 +430,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.getPlaylistEntities).not.toHaveBeenCalled();
      expect(mockGetEntties).not.toHaveBeenCalled();
@@ -463,7 +457,12 @@ describe('createRouter', () => {
            },
          ],
        },
-        { token: 'token' },
+        {
+          token: mockCredentials.service.token({
+            onBehalfOf: mockCredentials.user(),
+            targetPluginId: 'catalog',
+          }),
+        },
      );
      expect(response.status).toEqual(200);
      expect(response.body).toEqual(sampleEntities);
@@ -486,7 +485,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.removePlaylistEntities).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -518,7 +517,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.followPlaylist).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -528,7 +527,7 @@ describe('createRouter', () => {
      const response = await request(app).post('/playlist-id/followers').send();
      expect(mockDbHandler.followPlaylist).toHaveBeenCalledWith(
        'playlist-id',
-        mockUser,
+        mockCredentials.user().principal,
      );
      expect(response.status).toEqual(200);
    });
@@ -550,7 +549,7 @@ describe('createRouter', () => {
            resourceRef: 'playlist-id',
          },
        ],
-        { token: 'token' },
+        { credentials: mockCredentials.user() },
      );
      expect(mockDbHandler.unfollowPlaylist).not.toHaveBeenCalled();
      expect(response.status).toEqual(403);
@@ -562,7 +561,7 @@ describe('createRouter', () => {
        .send();
      expect(mockDbHandler.unfollowPlaylist).toHaveBeenCalledWith(
        'playlist-id',
-        mockUser,
+        mockCredentials.user().principal,
      );
      expect(response.status).toEqual(200);
    });
@@ -15,6 +15,7 @@
 */

 import {
+  createLegacyAuthAdapters,
  errorHandler,
  PluginDatabaseManager,
  PluginEndpointDiscovery,
@@ -22,14 +23,10 @@ import {
 import { CatalogClient } from '@backstage/catalog-client';
 import { parseEntityRef } from '@backstage/catalog-model';
 import { NotAllowedError } from '@backstage/errors';
-import {
-  getBearerTokenFromAuthorizationHeader,
-  IdentityApi,
-} from '@backstage/plugin-auth-node';
+import { IdentityApi } from '@backstage/plugin-auth-node';
 import {
  AuthorizePermissionRequest,
  AuthorizeResult,
-  PermissionEvaluator,
  QueryPermissionRequest,
 } from '@backstage/plugin-permission-common';
 import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node';
@@ -44,6 +41,11 @@ import { Logger } from 'winston';
 import { rules, transformConditions } from '../permissions';
 import { DatabaseHandler } from './DatabaseHandler';
 import { parseListPlaylistsFilterParams } from './ListPlaylistsFilter';
+import {
+  AuthService,
+  HttpAuthService,
+  PermissionsService,
+} from '@backstage/backend-plugin-api';

 /**
 * @public
@@ -53,7 +55,9 @@ export interface RouterOptions {
  discovery: PluginEndpointDiscovery;
  identity: IdentityApi;
  logger: Logger;
-  permissions: PermissionEvaluator;
+  permissions: PermissionsService;
+  auth?: AuthService;
+  httpAuth?: HttpAuthService;
 }

 /**
@@ -65,11 +69,12 @@ export async function createRouter(
  const {
    database,
    discovery,
-    identity,
    logger,
    permissions: permissionEvaluator,
  } = options;

+  const { auth, httpAuth } = createLegacyAuthAdapters(options);
+
  logger.info('Initializing Playlist backend');

  const catalogClient = new CatalogClient({ discoveryApi: discovery });
@@ -81,26 +86,21 @@ export async function createRouter(
    permission: AuthorizePermissionRequest | QueryPermissionRequest,
    conditional: boolean = false,
  ) => {
-    const token = getBearerTokenFromAuthorizationHeader(
-      request.header('authorization'),
-    );
-
-    const user = await identity.getIdentity({ request });
-    if (!user) {
-      throw new NotAllowedError('Unauthorized');
-    }
+    const credentials = await httpAuth.credentials(request, {
+      allow: ['user'],
+    });

    const decision = conditional
      ? (
          await permissionEvaluator.authorizeConditional(
            [permission as QueryPermissionRequest],
-            { token },
+            { credentials },
          )
        )[0]
      : (
          await permissionEvaluator.authorize(
            [permission as AuthorizePermissionRequest],
-            { token },
+            { credentials },
          )
        )[0];

@@ -108,7 +108,7 @@ export async function createRouter(
      throw new NotAllowedError('Unauthorized');
    }

-    return { decision, user: user.identity };
+    return { decision, user: credentials.principal };
  };

  const permissionIntegrationRouter = createPermissionIntegrationRouter({
@@ -227,9 +227,10 @@ export async function createRouter(
      };
    });

-    const token = getBearerTokenFromAuthorizationHeader(
-      req.header('authorization'),
-    );
+    const { token } = await auth.getPluginRequestToken({
+      onBehalfOf: await httpAuth.credentials(req),
+      targetPluginId: 'catalog',
+    });

    // TODO(kuanpg): entities in this playlist that no longer exist in the catalog will be
    // excluded from this response, we need a way to clean up these orphaned refs potentially