Merge pull request #23055 from backstage/rugvip/pr/9e8e32

permissions: migrate to new auth system and accept credentials

.changeset/late-turkeys-remember.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-permission-common': patch
+---
+
+The `token` option of the `PermissionEvaluator` methods is now deprecated. The options that only apply to backend implementations have been moved to `PermissionsService` from `@backstage/backend-plugin-api` instead.

.changeset/neat-owls-pump.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-app-api': patch
+---
+
+Updated the `permissionsServiceFactory` to forward the `AuthService` to the implementation.

.changeset/polite-tips-begin.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-plugin-api': patch
+---
+
+Updated the `PermissionsService` methods to accept `BackstageCredentials` through options.

.changeset/soft-grapes-cough.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-permission-node': patch
+---
+
+The `ServerPermissionClient` has been migrated to implement the `PermissionsService` interface, now accepting the new `BackstageCredentials` object in addition to the `token` option, which is now deprecated. It now also optionally depends on the new `AuthService`.

packages/backend-app-api/src/services/implementations/permissions/permissionsServiceFactory.ts

@@ -24,12 +24,14 @@ import { ServerPermissionClient } from '@backstage/plugin-permission-node';
 export const permissionsServiceFactory = createServiceFactory({
   service: coreServices.permissions,
   deps: {
+    auth: coreServices.auth,
     config: coreServices.rootConfig,
     discovery: coreServices.discovery,
     tokenManager: coreServices.tokenManager,
   },
-  async factory({ config, discovery, tokenManager }) {
+  async factory({ auth, config, discovery, tokenManager }) {
     return ServerPermissionClient.fromConfig(config, {
+      auth,
       discovery,
       tokenManager,
     });

packages/backend-plugin-api/api-report.md

@@ -5,6 +5,8 @@
 ```ts
 /// <reference types="node" />
 
+import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common';
+import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common';
 import { Config } from '@backstage/config';
 import { Handler } from 'express';
 import { IdentityApi } from '@backstage/plugin-auth-node';
@@ -13,6 +15,8 @@ import { JsonValue } from '@backstage/types';
 import { Knex } from 'knex';
 import { PermissionEvaluator } from '@backstage/plugin-permission-common';
 import { PluginTaskScheduler } from '@backstage/backend-tasks';
+import { QueryPermissionRequest } from '@backstage/plugin-permission-common';
+import { QueryPermissionResponse } from '@backstage/plugin-permission-common';
 import { Readable } from 'stream';
 import { Request as Request_2 } from 'express';
 import { Response as Response_2 } from 'express';
@@ -364,7 +368,27 @@ export interface LoggerService {
 }
 
 // @public (undocumented)
-export interface PermissionsService extends PermissionEvaluator {}
+export interface PermissionsService extends PermissionEvaluator {
+  // (undocumented)
+  authorize(
+    requests: AuthorizePermissionRequest[],
+    options?: PermissionsServiceRequestOptions,
+  ): Promise<AuthorizePermissionResponse[]>;
+  // (undocumented)
+  authorizeConditional(
+    requests: QueryPermissionRequest[],
+    options?: PermissionsServiceRequestOptions,
+  ): Promise<QueryPermissionResponse[]>;
+}
+
+// @public
+export type PermissionsServiceRequestOptions =
+  | {
+      token?: string;
+    }
+  | {
+      credentials: BackstageCredentials;
+    };
 
 // @public (undocumented)
 export interface PluginMetadataService {

packages/backend-plugin-api/src/services/definitions/PermissionsService.ts

@@ -14,7 +14,38 @@
  * limitations under the License.
  */
 
-import { PermissionEvaluator } from '@backstage/plugin-permission-common';
+import {
+  AuthorizePermissionRequest,
+  AuthorizePermissionResponse,
+  PermissionEvaluator,
+  QueryPermissionRequest,
+  QueryPermissionResponse,
+} from '@backstage/plugin-permission-common';
+import { BackstageCredentials } from './AuthService';
+
+/**
+ * Options for {@link @backstage/plugin-permission-common#PermissionEvaluator} requests.
+ *
+ * @public
+ */
+export type PermissionsServiceRequestOptions =
+  | {
+      /** @deprecated use the `credentials` option instead. */
+      token?: string;
+    }
+  | {
+      credentials: BackstageCredentials;
+    };
 
 /** @public */
-export interface PermissionsService extends PermissionEvaluator {}
+export interface PermissionsService extends PermissionEvaluator {
+  authorize(
+    requests: AuthorizePermissionRequest[],
+    options?: PermissionsServiceRequestOptions,
+  ): Promise<AuthorizePermissionResponse[]>;
+
+  authorizeConditional(
+    requests: QueryPermissionRequest[],
+    options?: PermissionsServiceRequestOptions,
+  ): Promise<QueryPermissionResponse[]>;
+}

packages/backend-plugin-api/src/services/definitions/index.ts

@@ -44,7 +44,10 @@ export type {
   LifecycleServiceShutdownOptions,
 } from './LifecycleService';
 export type { LoggerService } from './LoggerService';
-export type { PermissionsService } from './PermissionsService';
+export type {
+  PermissionsService,
+  PermissionsServiceRequestOptions,
+} from './PermissionsService';
 export type { PluginMetadataService } from './PluginMetadataService';
 export type { RootHttpRouterService } from './RootHttpRouterService';
 export type { RootLifecycleService } from './RootLifecycleService';

plugins/permission-common/src/types/api.ts

@@ -263,9 +263,16 @@ export interface PermissionEvaluator {
 
 /**
  * Options for {@link PermissionEvaluator} requests.
- * The Backstage identity token should be defined if available.
+ *
  * @public
  */
 export type EvaluatorRequestOptions = {
+  /**
+   * @deprecated Backend plugins should no longer depend on the
+   * `PermissionEvaluator`, but instead use the `PermissionService` from
+   * `@backstage/backend-plugin-api`. Frontend plugins should not need to inject
+   * this token at all, but instead implicitly rely on underlying fetchApi to do
+   * it for them.
+   */
   token?: string;
 };

plugins/permission-node/api-report.md

@@ -7,20 +7,21 @@ import { AllOfCriteria } from '@backstage/plugin-permission-common';
 import { AnyOfCriteria } from '@backstage/plugin-permission-common';
 import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common';
 import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common';
+import { AuthService } from '@backstage/backend-plugin-api';
 import { BackstageIdentityResponse } from '@backstage/plugin-auth-node';
 import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common';
 import { Config } from '@backstage/config';
 import { DefinitivePolicyDecision } from '@backstage/plugin-permission-common';
-import { EvaluatorRequestOptions } from '@backstage/plugin-permission-common';
+import { DiscoveryService } from '@backstage/backend-plugin-api';
 import express from 'express';
 import { IdentifiedPermissionMessage } from '@backstage/plugin-permission-common';
 import { NotCriteria } from '@backstage/plugin-permission-common';
 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 { PermissionRuleParams } from '@backstage/plugin-permission-common';
-import { PluginEndpointDiscovery } from '@backstage/backend-common';
+import { PermissionsService } from '@backstage/backend-plugin-api';
+import { PermissionsServiceRequestOptions } from '@backstage/backend-plugin-api';
 import { PolicyDecision } from '@backstage/plugin-permission-common';
 import { QueryPermissionRequest } from '@backstage/plugin-permission-common';
 import { ResourcePermission } from '@backstage/plugin-permission-common';
@@ -272,23 +273,24 @@ export type PolicyQuery = {
 };
 
 // @public
-export class ServerPermissionClient implements PermissionEvaluator {
+export class ServerPermissionClient implements PermissionsService {
   // (undocumented)
   authorize(
     requests: AuthorizePermissionRequest[],
-    options?: EvaluatorRequestOptions,
+    options?: PermissionsServiceRequestOptions,
   ): Promise<AuthorizePermissionResponse[]>;
   // (undocumented)
   authorizeConditional(
     queries: QueryPermissionRequest[],
-    options?: EvaluatorRequestOptions,
+    options?: PermissionsServiceRequestOptions,
   ): Promise<PolicyDecision[]>;
   // (undocumented)
   static fromConfig(
     config: Config,
     options: {
-      discovery: PluginEndpointDiscovery;
+      discovery: DiscoveryService;
       tokenManager: TokenManager;
+      auth?: AuthService;
     },
   ): ServerPermissionClient;
 }

plugins/permission-node/src/ServerPermissionClient.test.ts

@@ -22,7 +22,11 @@ import {
   DefinitivePolicyDecision,
   ConditionalPolicyDecision,
 } from '@backstage/plugin-permission-common';
-import { setupRequestMockHandlers } from '@backstage/backend-test-utils';
+import {
+  mockCredentials,
+  mockServices,
+  setupRequestMockHandlers,
+} from '@backstage/backend-test-utils';
 import { ConfigReader } from '@backstage/config';
 import {
   getVoidLogger,
@@ -201,4 +205,162 @@ describe('ServerPermissionClient', () => {
       expect(mockAuthorizeHandler).toHaveBeenCalled();
     });
   });
+
+  describe('with credentials', () => {
+    describe('authorize', () => {
+      let mockAuthorizeHandler: jest.Mock;
+
+      beforeEach(() => {
+        mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => {
+          const responses = req.body.items.map(
+            (r: IdentifiedPermissionMessage<DefinitivePolicyDecision>) => ({
+              id: r.id,
+              result: AuthorizeResult.ALLOW,
+            }),
+          );
+
+          return res(json({ items: responses }));
+        });
+
+        server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler));
+      });
+
+      it('should bypass the permission backend if permissions are disabled', async () => {
+        const client = ServerPermissionClient.fromConfig(new ConfigReader({}), {
+          discovery,
+          tokenManager: ServerTokenManager.noop(),
+          auth: mockServices.auth(),
+        });
+
+        await client.authorize(
+          [
+            {
+              permission: testBasicPermission,
+            },
+          ],
+          { credentials: mockCredentials.none() },
+        );
+
+        expect(mockAuthorizeHandler).not.toHaveBeenCalled();
+      });
+
+      it('should bypass the permission backend if permissions are enabled and request has valid server token', async () => {
+        const tokenManager = ServerTokenManager.fromConfig(config, { logger });
+        const client = ServerPermissionClient.fromConfig(config, {
+          discovery,
+          tokenManager,
+          auth: mockServices.auth(),
+        });
+
+        await client.authorize([{ permission: testBasicPermission }], {
+          credentials: mockCredentials.service(),
+        });
+
+        expect(mockAuthorizeHandler).not.toHaveBeenCalled();
+      });
+
+      it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => {
+        const tokenManager = ServerTokenManager.fromConfig(config, { logger });
+        const client = ServerPermissionClient.fromConfig(config, {
+          discovery,
+          tokenManager,
+          auth: mockServices.auth(),
+        });
+
+        await client.authorize([{ permission: testBasicPermission }], {
+          credentials: mockCredentials.user(),
+        });
+
+        expect(mockAuthorizeHandler).toHaveBeenCalled();
+        expect(
+          mockAuthorizeHandler.mock.calls[0][0].headers.get('authorization'),
+        ).toBe(
+          mockCredentials.service.header({
+            onBehalfOf: mockCredentials.user(),
+            targetPluginId: 'permissions',
+          }),
+        );
+      });
+    });
+
+    describe('authorizeConditional', () => {
+      let mockAuthorizeHandler: jest.Mock;
+
+      beforeEach(() => {
+        mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => {
+          const responses = req.body.items.map(
+            (r: IdentifiedPermissionMessage<ConditionalPolicyDecision>) => ({
+              id: r.id,
+              result: AuthorizeResult.ALLOW,
+            }),
+          );
+
+          return res(json({ items: responses }));
+        });
+
+        server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler));
+      });
+
+      it('should bypass the permission backend if permissions are disabled', async () => {
+        const client = ServerPermissionClient.fromConfig(new ConfigReader({}), {
+          discovery,
+          tokenManager: ServerTokenManager.noop(),
+          auth: mockServices.auth(),
+        });
+
+        await client.authorizeConditional(
+          [{ permission: testResourcePermission }],
+          {
+            credentials: mockCredentials.none(),
+          },
+        );
+
+        expect(mockAuthorizeHandler).not.toHaveBeenCalled();
+      });
+
+      it('should bypass the permission backend if permissions are enabled and request has valid server token', async () => {
+        const tokenManager = ServerTokenManager.fromConfig(config, { logger });
+        const client = ServerPermissionClient.fromConfig(config, {
+          discovery,
+          tokenManager,
+          auth: mockServices.auth(),
+        });
+
+        await client.authorizeConditional(
+          [{ permission: testResourcePermission }],
+          {
+            credentials: mockCredentials.service(),
+          },
+        );
+
+        expect(mockAuthorizeHandler).not.toHaveBeenCalled();
+      });
+
+      it('should call the permission backend if permissions are enabled and request does not have valid server token', async () => {
+        const tokenManager = ServerTokenManager.fromConfig(config, { logger });
+        const client = ServerPermissionClient.fromConfig(config, {
+          discovery,
+          tokenManager,
+          auth: mockServices.auth(),
+        });
+
+        await client.authorizeConditional(
+          [{ permission: testResourcePermission }],
+          {
+            credentials: mockCredentials.user(),
+          },
+        );
+
+        expect(mockAuthorizeHandler).toHaveBeenCalled();
+        expect(
+          mockAuthorizeHandler.mock.calls[0][0].headers.get('authorization'),
+        ).toBe(
+          mockCredentials.service.header({
+            onBehalfOf: mockCredentials.user(),
+            targetPluginId: 'permissions',
+          }),
+        );
+      });
+    });
+  });
 });

plugins/permission-node/src/ServerPermissionClient.ts

@@ -16,15 +16,20 @@
 
 import {
   TokenManager,
-  PluginEndpointDiscovery,
+  createLegacyAuthAdapters,
 } from '@backstage/backend-common';
+import {
+  AuthService,
+  BackstageCredentials,
+  DiscoveryService,
+  PermissionsService,
+  PermissionsServiceRequestOptions,
+} from '@backstage/backend-plugin-api';
 import { Config } from '@backstage/config';
 import {
   AuthorizeResult,
   PermissionClient,
-  PermissionEvaluator,
   AuthorizePermissionRequest,
-  EvaluatorRequestOptions,
   AuthorizePermissionResponse,
   PolicyDecision,
   QueryPermissionRequest,
@@ -36,16 +41,17 @@ import {
  * service-to-service requests.
  * @public
  */
-export class ServerPermissionClient implements PermissionEvaluator {
-  private readonly permissionClient: PermissionClient;
-  private readonly tokenManager: TokenManager;
-  private readonly permissionEnabled: boolean;
+export class ServerPermissionClient implements PermissionsService {
+  readonly #auth: AuthService;
+  readonly #permissionClient: PermissionClient;
+  readonly #permissionEnabled: boolean;
 
   static fromConfig(
     config: Config,
     options: {
-      discovery: PluginEndpointDiscovery;
+      discovery: DiscoveryService;
       tokenManager: TokenManager;
+      auth?: AuthService;
     },
   ) {
     const { discovery, tokenManager } = options;
@@ -62,58 +68,92 @@ export class ServerPermissionClient implements PermissionEvaluator {
       );
     }
 
+    const { auth } = createLegacyAuthAdapters(options);
+
     return new ServerPermissionClient({
+      auth,
       permissionClient,
-      tokenManager,
       permissionEnabled,
     });
   }
 
   private constructor(options: {
+    auth: AuthService;
     permissionClient: PermissionClient;
-    tokenManager: TokenManager;
     permissionEnabled: boolean;
   }) {
-    this.permissionClient = options.permissionClient;
-    this.tokenManager = options.tokenManager;
-    this.permissionEnabled = options.permissionEnabled;
+    this.#auth = options.auth;
+    this.#permissionClient = options.permissionClient;
+    this.#permissionEnabled = options.permissionEnabled;
   }
 
   async authorizeConditional(
     queries: QueryPermissionRequest[],
-    options?: EvaluatorRequestOptions,
+    options?: PermissionsServiceRequestOptions,
   ): Promise<PolicyDecision[]> {
-    return (await this.isEnabled(options?.token))
-      ? this.permissionClient.authorizeConditional(queries, options)
-      : queries.map(_ => ({ result: AuthorizeResult.ALLOW }));
+    if (await this.#shouldPermissionsBeApplied(options)) {
+      return this.#permissionClient.authorizeConditional(
+        queries,
+        await this.#getRequestOptions(options),
+      );
+    }
+
+    return queries.map(_ => ({ result: AuthorizeResult.ALLOW }));
   }
 
   async authorize(
     requests: AuthorizePermissionRequest[],
-    options?: EvaluatorRequestOptions,
+    options?: PermissionsServiceRequestOptions,
   ): Promise<AuthorizePermissionResponse[]> {
-    return (await this.isEnabled(options?.token))
-      ? this.permissionClient.authorize(requests, options)
-      : requests.map(_ => ({ result: AuthorizeResult.ALLOW }));
+    if (await this.#shouldPermissionsBeApplied(options)) {
+      return this.#permissionClient.authorize(
+        requests,
+        await this.#getRequestOptions(options),
+      );
+    }
+
+    return requests.map(_ => ({ result: AuthorizeResult.ALLOW }));
   }
 
-  private async isValidServerToken(
-    token: string | undefined,
-  ): Promise<boolean> {
-    if (!token) {
+  async #getRequestOptions(options?: PermissionsServiceRequestOptions) {
+    if (options && 'credentials' in options) {
+      if (this.#auth.isPrincipal(options.credentials, 'none')) {
+        return {};
+      }
+
+      return this.#auth.getPluginRequestToken({
+        onBehalfOf: options.credentials,
+        targetPluginId: 'permissions',
+      });
+    }
+
+    return options;
+  }
+
+  async #shouldPermissionsBeApplied(
+    options?: PermissionsServiceRequestOptions,
+  ) {
+    if (!this.#permissionEnabled) {
       return false;
     }
-    return this.tokenManager
-      .authenticate(token)
-      .then(() => true)
-      .catch(() => false);
-  }
 
-  private async isEnabled(token?: string) {
-    // Check if permissions are enabled before validating the server token. That
-    // way when permissions are disabled, the noop token manager can be used
-    // without fouling up the logic inside the ServerPermissionClient, because
-    // the code path won't be reached.
-    return this.permissionEnabled && !(await this.isValidServerToken(token));
+    let credentials: BackstageCredentials;
+    if (options && 'credentials' in options) {
+      credentials = options.credentials;
+    } else {
+      if (!options?.token) {
+        return true;
+      }
+      try {
+        credentials = await this.#auth.authenticate(options.token);
+      } catch {
+        return true;
+      }
+    }
+
+    if (this.#auth.isPrincipal(credentials, 'service')) {
+      return false;
+    }
+    return true;
   }
 }