Fix dynamic registration to better comply w/RFC

oada/libs/lib-arangodb/package.json

@@ -58,7 +58,7 @@
     "deep-equal": "^2.0.5",
     "flat": "^5.0.2",
     "json-ptr": "^3.1.0",
-    "tslib": "^2.3.1"
+    "tslib": "^2.4.0"
   },
   "devDependencies": {
     "@ava/typescript": "^3.0.1",
@@ -70,7 +70,7 @@
     "@types/deep-equal": "^1.0.1",
     "@types/flat": "^5.0.2",
     "@types/json-pointer": "^1.0.31",
-    "@types/node": "^16.11.27",
+    "@types/node": "^16.11.30",
     "ava": "4.0.0-rc.1",
     "type-fest": "^2.12.2"
   }

oada/libs/lib-config/package.json

@@ -33,7 +33,7 @@
     "convict-format-with-validator": "^6.2.0",
     "dotenv": "^16.0.0",
     "json5": "^2.2.1",
-    "tslib": "^2.3.1",
+    "tslib": "^2.4.0",
     "yaml": "^2.0.1"
   },
   "devDependencies": {

oada/libs/lib-kafka/package.json

@@ -35,15 +35,15 @@
     "debug": "^4.3.4",
     "kafkajs": "^1.16.0",
     "ksuid": "^3.0.0",
-    "tslib": "^2.3.1",
+    "tslib": "^2.4.0",
     "uuid": "^8.3.2"
   },
   "devDependencies": {
     "@ava/typescript": "^3.0.1",
     "@types/bluebird": "^3.5.36",
     "@types/convict": "^6.1.1",
     "@types/debug": "^4.1.7",
-    "@types/node": "16.11.27",
+    "@types/node": "16.11.30",
     "@types/uuid": "^8.3.4",
     "ava": "4.0.0-rc.1"
   }

oada/libs/pino-debug/package.json

@@ -17,14 +17,14 @@
   "author": "Alex Layton <alex@layton.in>",
   "license": "ISC",
   "dependencies": {
-    "pino": "^7.10.0",
+    "pino": "^7.11.0",
     "pino-caller": "^3.2.0",
     "pino-debug": "^2.0.0",
-    "tslib": "^2.3.1"
+    "tslib": "^2.4.0"
   },
   "devDependencies": {
     "@types/debug": "^4.1.7",
-    "@types/node": "^16.11.27"
+    "@types/node": "^16.11.30"
   },
   "peerDependencies": {
     "debug": "*"

oada/package.json

@@ -14,12 +14,12 @@
   "devDependencies": {
     "@tsconfig/node16": "^1.0.2",
     "@types/eslint": "^8.4.1",
-    "@types/mocha": "^9.1.0",
-    "@types/node": "^16.11.27",
-    "@typescript-eslint/eslint-plugin": "^5.20.0",
-    "@typescript-eslint/parser": "^5.20.0",
+    "@types/mocha": "^9.1.1",
+    "@types/node": "^16.11.30",
+    "@typescript-eslint/eslint-plugin": "^5.21.0",
+    "@typescript-eslint/parser": "^5.21.0",
     "@yarnpkg/sdks": "^3.0.0-rc.2",
-    "eslint": "^8.13.0",
+    "eslint": "^8.14.0",
     "eslint-config-prettier": "^8.5.0",
     "eslint-config-xo": "^0.40.0",
     "eslint-config-xo-typescript": "^0.50.0",
@@ -39,7 +39,7 @@
     "eslint-plugin-optimize-regex": "^1.2.1",
     "eslint-plugin-prettier": "^4.0.0",
     "eslint-plugin-promise": "^6.0.0",
-    "eslint-plugin-regexp": "^1.6.0",
+    "eslint-plugin-regexp": "^1.7.0",
     "eslint-plugin-security": "^1.5.0",
     "eslint-plugin-sonarjs": "^0.13.0",
     "eslint-plugin-unicorn": "^42.0.0",

oada/services/auth/package.json

@@ -49,7 +49,7 @@
     "@oada/pino-debug": "^3.0.1",
     "@oada/well-known-json": "^2.0.1",
     "arangojs": "^7.7.0",
-    "axios": "^0.26.1",
+    "axios": "^0.27.1",
     "bcryptjs": "^2.4.3",
     "bluebird": "^3.7.2",
     "body-parser": "^1.20.0",
@@ -59,9 +59,9 @@
     "connect-ensure-login": "^0.1.1",
     "cors": "^2.8.5",
     "debug": "^4.3.4",
-    "ejs": "^3.1.6",
+    "ejs": "^3.1.7",
     "es-main": "^1.0.2",
-    "express": "^4.17.3",
+    "express": "^4.18.0",
     "express-session": "^1.17.2",
     "helmet": "^5.0.2",
     "jsonwebtoken": "^8.5.1",
@@ -75,7 +75,7 @@
     "passport-local": "^1.0.0",
     "passport-oauth2-client-password": "~0.1.2",
     "pem-jwk": "^2.0.0",
-    "tslib": "^2.3.1",
+    "tslib": "^2.4.0",
     "urijs": "^1.19.11",
     "uuid": "^8.3.2"
   },
@@ -102,7 +102,7 @@
     "@types/pem-jwk": "^2.0.0",
     "@types/urijs": "^1.19.19",
     "@types/uuid": "^8.3.4",
-    "c8": "^7.11.0",
+    "c8": "^7.11.2",
     "type-fest": "^2.12.2"
   }
 }

oada/services/auth/src/config.ts

@@ -173,6 +173,25 @@ export const { config, schema } = await libConfig({
       },
     },
     dynamicRegistration: {
+      softwareStatement: {
+        require: {
+          description:
+            'Whether to require all clients send a software_statement to register',
+          format: Boolean,
+          default: false,
+        },
+        mustTrust: {
+          description:
+            'Whether to outright reject clients with untrusted software_statement',
+          format: Boolean,
+          default: false,
+        },
+        mustInclude: {
+          description: 'List of field that any software_statement must include',
+          format: Array,
+          default: ['software_id'],
+        },
+      },
       trustedListLookupTimeout: {
         format: 'duration',
         default: 5000,

oada/services/auth/src/dynReg.ts

@@ -20,123 +20,153 @@ import { config } from './config.js';
 import type { RequestHandler } from 'express';
 import debug from 'debug';
 
+import Metadata, {
+  assert as assertMetadata,
+} from '@oada/types/oauth-dyn-reg/metadata.js';
 import { validate } from '@oada/certs';
 
 import { IClient, save } from './db/models/client.js';
 
+/**
+ * @see {@link https://datatracker.ietf.org/doc/html/rfc7591#section-3.2.2}
+ */
+const enum RegistrationErrorCode {
+  InvalidRedirectURI = 'invalid_redirect_uri',
+  InvalidClientMetadata = 'invalid_client_metadata',
+  InvalidSoftwareStatement = 'invalid_software_statement',
+  UnapprovedSoftwareStatement = 'unapproved_software_statement',
+}
+
+/**
+ * @see {@link https://datatracker.ietf.org/doc/html/rfc7591#section-3.2.2}
+ */
+class RegistrationError extends Error {
+  readonly code;
+
+  constructor(code: RegistrationErrorCode, message?: string) {
+    super(message);
+    this.code = code;
+  }
+
+  toJSON() {
+    return {
+      error: this.code,
+      error_description: this.message,
+    };
+  }
+}
+
 const error = debug('oada-ref-auth:dynReg:error');
 const info = debug('oada-ref-auth:dynReg:info');
 const trace = debug('oada-ref-auth:dynReg:trace');
 
+const {
+  require: requireSS,
+  mustInclude,
+  mustTrust,
+} = config.get('auth.dynamicRegistration.softwareStatement');
+const timeout = config.get('auth.dynamicRegistration.trustedListLookupTimeout');
+
+async function getSoftwareStatement({ software_statement }: Metadata) {
+  if (!software_statement) {
+    return undefined;
+  }
+
+  const { payload, trusted, valid, details } = await validate.validate(
+    software_statement,
+    { timeout }
+  );
+  if (!valid) {
+    throw new RegistrationError(
+      RegistrationErrorCode.InvalidSoftwareStatement,
+      `Software statement was not a valid JWT. Details = "${details}"`
+    );
+  }
+
+  const statements: Metadata =
+    typeof payload === 'string' ? JSON.parse(payload) : payload;
+  return {
+    ...statements,
+    // Set the "trusted" status based on JWS library return value
+    trusted,
+  };
+}
+
 const dynReg: RequestHandler = async (request, response) => {
   try {
-    if (!request.body?.software_statement) {
+    const metadata = request.body;
+    // TODO: More thorough checking of sent metadata
+    assertMetadata(metadata);
+
+    const softwareStatement = await getSoftwareStatement(metadata);
+
+    if (requireSS && !softwareStatement) {
       info(
-        request.body,
+        metadata,
         'Request body does not have software_statement key. Did you remember content-type=application/json?'
       );
-      response.status(400).json({
-        error: 'invalid_client_registration_body',
-        error_description:
-          'POST to Client registration must include a software_statement in the body',
-      });
-      return;
+      throw new RegistrationError(
+        // FIXME: What is the correct code here??
+        RegistrationErrorCode.InvalidSoftwareStatement,
+        'Client registration MUST include a software_statement for this server'
+      );
     }
 
-    const { payload, trusted, valid, details } = await validate.validate(
-      request.body.software_statement,
-      {
-        timeout: config.get(
-          'auth.dynamicRegistration.trustedListLookupTimeout'
-        ),
-      }
-    );
-    const clientcert = {
-      ...((typeof payload === 'string'
-        ? JSON.parse(payload)
-        : payload) as IClient),
-      // Set the "trusted" status based on JWS library return value
-      valid,
-      trusted,
-    };
+    if (mustTrust && !softwareStatement?.trusted) {
+      info(metadata, 'Request body does not have a trusted software_statement');
+      throw new RegistrationError(
+        RegistrationErrorCode.UnapprovedSoftwareStatement,
+        'Client registration MUST include a software_statement for this server'
+      );
+    }
 
-    // Must have contacts, client_name, and redirect_uris or we won't save it
     if (
-      !clientcert.contacts ||
-      !clientcert.client_name ||
-      !clientcert.redirect_uris
+      softwareStatement &&
+      !mustInclude.every((statement) => statement in softwareStatement)
     ) {
-      response.status(400).json({
-        error: 'invalid_software_statement',
-        error_description:
-          'Software statement must include at least client_name, redirect_uris, and contacts',
-      });
-      return;
-    }
-
-    if (!valid) {
-      response.status(400).json({
-        error: 'invalid_software_statement',
-        error_description: `Software statement was not a valid JWT. Details on rejection = ${JSON.stringify(
-          details,
-          null,
-          '  '
-        )}`,
-      });
-      return;
-    }
-
-    // If scopes is listed in the body, check them to make sure they are in the software_statement, then
-    // replace the signed ones with the subset given in the body
-    const { scope } = (request.body ?? {}) as { scope?: string };
-    if (typeof scope === 'string') {
-      const possiblescopes = (clientcert.scope ?? '').split(' ');
-      const subsetscopes = scope.split(' ');
-      const finalscopes = subsetscopes.filter((s) =>
-        possiblescopes.includes(s)
+      throw new RegistrationError(
+        RegistrationErrorCode.InvalidSoftwareStatement,
+        `Software statement must include at least ${mustInclude}`
       );
-      clientcert.scope = finalscopes.join(' ');
     }
 
+    // Fields in software statement MUST take precedence
+    const registrationData = { ...metadata, ...softwareStatement };
+
     // ------------------------------------------
     // Save client to database, return client_id for their future OAuth2 requests
     trace(
       'Saving client %s registration, trusted = %s',
-      clientcert.client_name,
-      trusted
+      registrationData.client_name,
+      registrationData.trusted
     );
-    try {
-      const client = await save(clientcert);
-      const result = {
-        ...clientcert,
-        clientId: undefined,
-        client_id: client.clientId,
-      };
-      info(
-        'Saved new client ID %s to DB, client_name = %s',
-        result.client_id,
-        result.client_name
-      );
-      response.status(201).json(result);
-    } catch (cError: unknown) {
-      error(cError, 'Failed to save new dynReg client');
-      response.status(400).json({
-        error: 'invalid_client_registration',
-        error_description: `Unexpected error - Client registration could not be stored. Err = ${cError}`,
-      });
-    }
-
-    // If @oada/certs fails
-  } catch (cError: unknown) {
-    error(
-      'Failed to validate client registration, oada-certs threw error: %O',
-      cError
+    const client = await save(registrationData as IClient);
+    const result = {
+      ...registrationData,
+      client_id: client.clientId,
+    };
+    info(
+      'Saved new client ID %s to DB, client_name = %s',
+      result.client_id,
+      result.client_name
     );
-    response.status(400).json({
-      error: 'invalid_client_registration',
-      error_description:
-        'Client registration failed decoding or had invalid signature.',
-    });
+    response.status(201).json(result);
+  } catch (cError: unknown) {
+    error(cError, 'Failed to validate client registration');
+    if (cError instanceof RegistrationError) {
+      response.status(400).json(cError);
+    } else {
+      response
+        .status(400)
+        .json(
+          new RegistrationError(
+            RegistrationErrorCode.InvalidClientMetadata,
+            `Client registration failed: ${
+              (cError as Error)?.message ?? cError
+            }`
+          )
+        );
+    }
   }
 };