one token manager per plugin

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
2024-05-20 11:27:46 +02:00
parent b155d854bc
commit 17be3e6962
9 changed files with 138 additions and 85 deletions
@@ -23,11 +23,7 @@ import {
  BackstageServicePrincipal,
  BackstageUserPrincipal,
 } from '@backstage/backend-plugin-api';
-import {
-  AuthenticationError,
-  ForwardedError,
-  NotAllowedError,
-} from '@backstage/errors';
+import { AuthenticationError, ForwardedError } from '@backstage/errors';
 import { JsonObject } from '@backstage/types';
 import { decodeJwt } from 'jose';
 import { ExternalTokenHandler } from './external/ExternalTokenHandler';
@@ -86,20 +82,10 @@ export class DefaultAuthService implements AuthService {

    const externalResult = await this.externalTokenHandler.verifyToken(token);
    if (externalResult) {
-      const restrictions = externalResult.accessRestrictions;
-      if (restrictions) {
-        if (!restrictions.has(this.pluginId)) {
-          const valid = [...restrictions.keys()].map(k => `'${k}'`).join(', ');
-          throw new NotAllowedError(
-            `This token's access is restricted to plugin(s) ${valid}`,
-          );
-        }
-      }
-
      return createCredentialsWithServicePrincipal(
        externalResult.subject,
        undefined,
-        restrictions?.get(this.pluginId),
+        externalResult.accessRestrictions,
      );
    }

@@ -39,19 +39,7 @@ export const authServiceFactory = createServiceFactory({
    // new auth services in the new backend system.
    tokenManager: coreServices.tokenManager,
  },
-  async createRootContext({ config, logger }) {
-    const externalTokens = ExternalTokenHandler.create({
-      config,
-      logger,
-    });
-    return {
-      externalTokens,
-    };
-  },
-  async factory(
-    { config, discovery, plugin, tokenManager, logger, database },
-    { externalTokens },
-  ) {
+  async factory({ config, discovery, plugin, tokenManager, logger, database }) {
    const disableDefaultAuthPolicy = Boolean(
      config.getOptionalBoolean(
        'backend.auth.dangerouslyDisableDefaultAuthPolicy',
@@ -73,6 +61,11 @@ export const authServiceFactory = createServiceFactory({
      publicKeyStore,
      discovery,
    });
+    const externalTokens = ExternalTokenHandler.create({
+      ownPluginId: plugin.getId(),
+      config,
+      logger,
+    });

    return new DefaultAuthService(
      userTokens,
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { BackstagePrincipalAccessRestrictions } from '@backstage/backend-plugin-api';
+import { ExternalTokenHandler } from './ExternalTokenHandler';
+import { TokenHandler } from './types';
+
+describe('ExternalTokenHandler', () => {
+  it('skips over inner handlers that do not match, and applies plugin restrictions', async () => {
+    const handler1: TokenHandler = {
+      add: jest.fn(),
+      verifyToken: jest.fn().mockResolvedValue(undefined),
+    };
+
+    const handler2: TokenHandler = {
+      add: jest.fn(),
+      verifyToken: jest.fn().mockResolvedValue({
+        subject: 'sub',
+        allAccessRestrictions: new Map(
+          Object.entries({
+            plugin1: {
+              permissionNames: ['do.it'],
+            } satisfies BackstagePrincipalAccessRestrictions,
+          }),
+        ),
+      }),
+    };
+
+    const plugin1 = new ExternalTokenHandler('plugin1', [handler1, handler2]);
+    const plugin2 = new ExternalTokenHandler('plugin2', [handler1, handler2]);
+
+    await expect(plugin1.verifyToken('token')).resolves.toEqual({
+      subject: 'sub',
+      accessRestrictions: { permissionNames: ['do.it'] },
+    });
+    await expect(
+      plugin2.verifyToken('token'),
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"This token's access is restricted to plugin(s) 'plugin1'"`,
+    );
+  });
+});
@@ -15,16 +15,19 @@
 */

 import {
+  BackstagePrincipalAccessRestrictions,
  LoggerService,
  RootConfigService,
 } from '@backstage/backend-plugin-api';
+import { NotAllowedError } from '@backstage/errors';
 import { LegacyTokenHandler } from './legacy';
 import { StaticTokenHandler } from './static';
 import { JWKSHandler } from './jwks';
-import { AccessRestriptionsMap, TokenHandler } from './types';
+import { TokenHandler } from './types';

 const NEW_CONFIG_KEY = 'backend.auth.externalAccess';
 const OLD_CONFIG_KEY = 'backend.auth.keys';
+let loggedDeprecationWarning = false;

 /**
 * Handles all types of external caller token types (i.e. not Backstage user
@@ -34,10 +37,11 @@ const OLD_CONFIG_KEY = 'backend.auth.keys';
 */
 export class ExternalTokenHandler {
  static create(options: {
+    ownPluginId: string;
    config: RootConfigService;
    logger: LoggerService;
  }): ExternalTokenHandler {
-    const { config, logger } = options;
+    const { ownPluginId, config, logger } = options;

    const staticHandler = new StaticTokenHandler();
    const legacyHandler = new LegacyTokenHandler();
@@ -66,7 +70,8 @@ export class ExternalTokenHandler {

    // Load the old keys too
    const legacyConfigs = config.getOptionalConfigArray(OLD_CONFIG_KEY) ?? [];
-    if (legacyConfigs.length) {
+    if (legacyConfigs.length && !loggedDeprecationWarning) {
+      loggedDeprecationWarning = true;
      logger.warn(
        `DEPRECATION WARNING: The ${OLD_CONFIG_KEY} config has been replaced by ${NEW_CONFIG_KEY}, see https://backstage.io/docs/auth/service-to-service-auth`,
      );
@@ -75,24 +80,48 @@ export class ExternalTokenHandler {
      legacyHandler.addOld(handlerConfig);
    }

-    return new ExternalTokenHandler(Object.values(handlers));
+    return new ExternalTokenHandler(ownPluginId, Object.values(handlers));
  }

-  constructor(private readonly handlers: TokenHandler[]) {}
+  constructor(
+    private readonly ownPluginId: string,
+    private readonly handlers: TokenHandler[],
+  ) {}

  async verifyToken(token: string): Promise<
    | {
        subject: string;
-        accessRestrictions?: AccessRestriptionsMap;
+        accessRestrictions?: BackstagePrincipalAccessRestrictions;
      }
    | undefined
  > {
    for (const handler of this.handlers) {
      const result = await handler.verifyToken(token);
      if (result) {
-        return result;
+        const { allAccessRestrictions, ...rest } = result;
+        if (allAccessRestrictions) {
+          const accessRestrictions = allAccessRestrictions.get(
+            this.ownPluginId,
+          );
+          if (!accessRestrictions) {
+            const valid = [...allAccessRestrictions.keys()]
+              .map(k => `'${k}'`)
+              .join(', ');
+            throw new NotAllowedError(
+              `This token's access is restricted to plugin(s) ${valid}`,
+            );
+          }
+
+          return {
+            ...rest,
+            accessRestrictions,
+          };
+        }
+
+        return rest;
      }
    }
+
    return undefined;
  }
 }
@@ -72,7 +72,7 @@ describe('LegacyTokenHandler', () => {

    await expect(tokenHandler.verifyToken(token1)).resolves.toEqual({
      subject: 'key1',
-      accessRestrictions: accessRestrictions1,
+      allAccessRestrictions: accessRestrictions1,
    });

    const token2 = await new SignJWT({
@@ -84,7 +84,7 @@ describe('LegacyTokenHandler', () => {

    await expect(tokenHandler.verifyToken(token2)).resolves.toEqual({
      subject: 'key2',
-      accessRestrictions: accessRestrictions2,
+      allAccessRestrictions: accessRestrictions2,
    });

    const token3 = await new SignJWT({
@@ -27,16 +27,18 @@ import { AccessRestriptionsMap, TokenHandler } from './types';
 export class LegacyTokenHandler implements TokenHandler {
  #entries = new Array<{
    key: Uint8Array;
-    subject: string;
-    accessRestrictions?: AccessRestriptionsMap;
+    result: {
+      subject: string;
+      allAccessRestrictions?: AccessRestriptionsMap;
+    };
  }>();

  add(config: Config) {
-    const accessRestrictions = readAccessRestrictionsFromConfig(config);
+    const allAccessRestrictions = readAccessRestrictionsFromConfig(config);
    this.#doAdd(
      config.getString('options.secret'),
      config.getString('options.subject'),
-      accessRestrictions,
+      allAccessRestrictions,
    );
  }

@@ -49,10 +51,12 @@ export class LegacyTokenHandler implements TokenHandler {
  #doAdd(
    secret: string,
    subject: string,
-    accessRestrictions?: AccessRestriptionsMap,
+    allAccessRestrictions?: AccessRestriptionsMap,
  ) {
    if (!secret.match(/^\S+$/)) {
      throw new Error('Illegal secret, must be a valid base64 string');
+    } else if (!subject.match(/^\S+$/)) {
+      throw new Error('Illegal subject, must be a set of non-space characters');
    }

    let key: Uint8Array;
@@ -62,14 +66,12 @@ export class LegacyTokenHandler implements TokenHandler {
      throw new Error('Illegal secret, must be a valid base64 string');
    }

-    if (!subject.match(/^\S+$/)) {
-      throw new Error('Illegal subject, must be a set of non-space characters');
-    }
-
    this.#entries.push({
      key,
-      subject,
-      accessRestrictions,
+      result: {
+        subject,
+        allAccessRestrictions,
+      },
    });
  }

@@ -94,13 +96,10 @@ export class LegacyTokenHandler implements TokenHandler {
      return undefined;
    }

-    for (const entry of this.#entries) {
+    for (const { key, result } of this.#entries) {
      try {
-        await jwtVerify(token, entry.key);
-        return {
-          subject: entry.subject,
-          accessRestrictions: entry.accessRestrictions,
-        };
+        await jwtVerify(token, key);
+        return result;
      } catch (e) {
        if (e.code !== 'ERR_JWS_SIGNATURE_VERIFICATION_FAILED') {
          throw e;
@@ -45,11 +45,11 @@ describe('StaticTokenHandler', () => {

    await expect(handler.verifyToken('abcabcabc')).resolves.toEqual({
      subject: 'one',
-      accessRestrictions: accessRestrictionsOne,
+      allAccessRestrictions: accessRestrictionsOne,
    });
    await expect(handler.verifyToken('defdefdef')).resolves.toEqual({
      subject: 'two',
-      accessRestrictions: accessRestrictionsTwo,
+      allAccessRestrictions: accessRestrictionsTwo,
    });
    await expect(handler.verifyToken('ghighighi')).resolves.toBeUndefined();
  });
@@ -26,46 +26,37 @@ const MIN_TOKEN_LENGTH = 8;
 * @internal
 */
 export class StaticTokenHandler implements TokenHandler {
-  #entries = new Array<{
-    token: string;
-    subject: string;
-    accessRestrictions?: AccessRestriptionsMap;
-  }>();
+  #entries = new Map<
+    string,
+    {
+      subject: string;
+      allAccessRestrictions?: AccessRestriptionsMap;
+    }
+  >();

  add(config: Config) {
    const token = config.getString('options.token');
+    const subject = config.getString('options.subject');
+    const allAccessRestrictions = readAccessRestrictionsFromConfig(config);
+
    if (!token.match(/^\S+$/)) {
      throw new Error('Illegal token, must be a set of non-space characters');
-    }
-    if (token.length < MIN_TOKEN_LENGTH) {
+    } else if (token.length < MIN_TOKEN_LENGTH) {
      throw new Error(
        `Illegal token, must be at least ${MIN_TOKEN_LENGTH} characters length`,
      );
-    }
-
-    const subject = config.getString('options.subject');
-    if (!subject.match(/^\S+$/)) {
+    } else if (!subject.match(/^\S+$/)) {
      throw new Error('Illegal subject, must be a set of non-space characters');
+    } else if (this.#entries.has(token)) {
+      throw new Error(
+        'Static externalAccess token was declared more than once',
+      );
    }

-    const accessRestrictions = readAccessRestrictionsFromConfig(config);
-
-    this.#entries.push({
-      token,
-      subject,
-      accessRestrictions,
-    });
+    this.#entries.set(token, { subject, allAccessRestrictions });
  }

  async verifyToken(token: string) {
-    const entry = this.#entries.find(e => e.token === token);
-    if (!entry) {
-      return undefined;
-    }
-
-    return {
-      subject: entry.subject,
-      accessRestrictions: entry.accessRestrictions,
-    };
+    return this.#entries.get(token);
  }
 }
@@ -27,7 +27,7 @@ export interface TokenHandler {
  verifyToken(token: string): Promise<
    | {
        subject: string;
-        accessRestrictions?: AccessRestriptionsMap;
+        allAccessRestrictions?: AccessRestriptionsMap;
      }
    | undefined
  >;