Address various comments round 1

Signed-off-by: Joon Park <joonp@spotify.com>

packages/app-defaults/package.json

@@ -32,6 +32,7 @@
     "@backstage/core-components": "^0.8.0",
     "@backstage/core-app-api": "^0.2.0",
     "@backstage/core-plugin-api": "^0.3.0",
+    "@backstage/plugin-permission-react": "^0.1.1",
     "@backstage/theme": "^0.2.14",
     "@material-ui/core": "^4.12.2",
     "@material-ui/icons": "^4.9.1",

packages/app-defaults/src/defaults/apis.ts

@@ -61,7 +61,12 @@ import {
   oidcAuthApiRef,
   bitbucketAuthApiRef,
   atlassianAuthApiRef,
+  identityApiRef,
 } from '@backstage/core-plugin-api';
+import {
+  permissionApiRef,
+  IdentityPermissionApi,
+} from '@backstage/plugin-permission-react';
 
 export const apis = [
   createApiFactory({
@@ -296,4 +301,14 @@ export const apis = [
       });
     },
   }),
+  createApiFactory({
+    api: permissionApiRef,
+    deps: {
+      discoveryApi: discoveryApiRef,
+      identityApi: identityApiRef,
+      configApi: configApiRef,
+    },
+    factory: ({ configApi, discoveryApi, identityApi }) =>
+      IdentityPermissionApi.create({ configApi, discoveryApi, identityApi }),
+  }),
 ];

packages/app/package.json

@@ -35,7 +35,6 @@
     "@backstage/plugin-newrelic": "^0.3.10",
     "@backstage/plugin-org": "^0.3.31",
     "@backstage/plugin-pagerduty": "0.3.19",
-    "@backstage/plugin-permission-react": "^0.1.1",
     "@backstage/plugin-rollbar": "^0.3.20",
     "@backstage/plugin-scaffolder": "^0.11.14",
     "@backstage/plugin-search": "^0.5.1",

packages/app/src/apis.ts

@@ -31,15 +31,9 @@ import {
   AnyApiFactory,
   configApiRef,
   createApiFactory,
-  discoveryApiRef,
   errorApiRef,
   githubAuthApiRef,
-  identityApiRef,
 } from '@backstage/core-plugin-api';
-import {
-  permissionApiRef,
-  IdentityPermissionApi,
-} from '@backstage/plugin-permission-react';
 
 export const apis: AnyApiFactory[] = [
   createApiFactory({
@@ -70,15 +64,4 @@ export const apis: AnyApiFactory[] = [
   }),
 
   createApiFactory(costInsightsApiRef, new ExampleCostInsightsClient()),
-
-  createApiFactory({
-    api: permissionApiRef,
-    deps: {
-      discoveryApi: discoveryApiRef,
-      identityApi: identityApiRef,
-      configApi: configApiRef,
-    },
-    factory: ({ configApi, discoveryApi, identityApi }) =>
-      IdentityPermissionApi.create({ configApi, discoveryApi, identityApi }),
-  }),
 ];

packages/backend-common/src/tokens/ServerTokenManager.ts

@@ -45,35 +45,25 @@ export class ServerTokenManager implements TokenManager {
   static fromConfig(config: Config, options: { logger: Logger }) {
     const { logger } = options;
 
-    if (process.env.NODE_ENV === 'development') {
-      let secrets: string[] = [];
-      try {
-        secrets = this.getSecrets(config);
-      } catch {
-        // For development, if a secret has not been configured, we auto generate a secret instead of throwing.
-      }
-
-      if (!secrets.length) {
-        const generatedDevOnlyKey = JWK.generateSync('oct', 24 * 8);
-        if (generatedDevOnlyKey.k === undefined) {
-          throw new Error('No key generated');
-        }
-        logger.warn(
-          'Generated a secret for backend-to-backend authentication: DEVELOPMENT USE ONLY. You must configure a secret before deploying to production.',
-        );
-        return new ServerTokenManager([generatedDevOnlyKey.k]);
-      }
-      return new ServerTokenManager(secrets);
+    const keys = config.getOptionalConfigArray('backend.auth.keys');
+    if (keys?.length) {
+      return new ServerTokenManager(keys.map(key => key.getString('secret')));
+    }
+    if (process.env.NODE_ENV !== 'development') {
+      throw new Error(
+        'You must configure at least one key in backend.auth.keys for production.',
+      );
     }
 
-    const secrets = this.getSecrets(config);
-    return new ServerTokenManager(secrets);
-  }
-
-  private static getSecrets(config: Config) {
-    return config
-      .getConfigArray('backend.auth.keys')
-      .map(key => key.getString('secret'));
+    // For development, if a secret has not been configured, we auto generate a secret instead of throwing.
+    const generatedDevOnlyKey = JWK.generateSync('oct', 24 * 8);
+    if (generatedDevOnlyKey.k === undefined) {
+      throw new Error('No key generated');
+    }
+    logger.warn(
+      'Generated a secret for backend-to-backend authentication: DEVELOPMENT USE ONLY.',
+    );
+    return new ServerTokenManager([generatedDevOnlyKey.k]);
   }
 
   private constructor(secrets: string[]) {

packages/backend/src/index.ts

@@ -63,7 +63,7 @@ function makeCreateEnv(config: Config) {
   const root = getRootLogger();
   const reader = UrlReaders.default({ logger: root, config });
   const discovery = SingleHostDiscovery.fromConfig(config);
-  const tokenManager = ServerTokenManager.noop();
+  const tokenManager = ServerTokenManager.fromConfig(config, { logger: root });
   const permissions = new ServerPermissionClient({
     discoveryApi: discovery,
     configApi: config,

plugins/permission-node/src/ServerPermissionClient.ts

@@ -38,7 +38,12 @@ export class ServerPermissionClient extends PermissionClient {
     const { discoveryApi, configApi, serverTokenManager } = options;
     super({ discoveryApi, configApi });
 
-    if (this.enabled && !(serverTokenManager instanceof ServerTokenManager)) {
+    if (
+      this.enabled &&
+      // TODO: Find a cleaner way of ensuring usage of SERVER token manager when
+      // permissions are enabled.
+      serverTokenManager instanceof ServerTokenManager.noop().constructor
+    ) {
       throw new Error(
         'You must configure at least one key in backend.auth.keys if permissions are enabled.',
       );