Merge pull request #208 from bcgov/bucket-check

Validate default bucket configuration on startup

.github/environments/values.dev.yaml

@@ -1,6 +1,7 @@
 ---
 features:
   basicAuth: true
+  defaultBucket: false
   oidcAuth: true
 
 autoscaling:
@@ -18,15 +19,12 @@ config:
       MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuy7zfh2ZgpDV5mH/aXyLDTddZK81rGakJcTy4KvCNOkDDxt1KAhW02lmbCo8YhHCOzjNZBp1+Vi6QiMRgBqAe2GTPZYEiV70aXfROGZe3Nvwcjbtki6HoyRte3SpqLJEIPL2F+hjJkw1UPGnjPTWZkEx9p74b9i3BjuE8RnjJ0Sza2MWw83zoQUZEJRGiopSL0yuVej6t2LO2btVdVf7QuZfPt9ehkcQYlPKpVvJA+pfeqPAdnNt7OjEIeYxinjurZr8Z04hz8UhkRefcWlSbFzFQYmL7O7iArjW0bsSvq8yNUd5r0KCOQkFduwZy26yTzTxj8OLFT91fEmbBBl4rQIDAQAB
     KC_REALM: standard
     KC_SERVERURL: "https://dev.loginproxy.gov.bc.ca/auth"
-    OBJECTSTORAGE_BUCKET: egejyy
-    OBJECTSTORAGE_TEMP_EXPIRESIN: "300"
-    OBJECTSTORAGE_ENDPOINT: "https://nrs.objectstore.gov.bc.ca"
-    # OBJECTSTORAGE_KEY: ~
     SERVER_BODYLIMIT: 30mb
     # SERVER_LOGFILE: ~
     SERVER_LOGLEVEL: http
     SERVER_PORT: "3000"
     SERVER_PRIVACY_MASK: "true"
+    SERVER_TEMP_EXPIRESIN: "300"
 
 patroni:
   enabled: true

.github/environments/values.pr.yaml

@@ -2,6 +2,7 @@
 features:
   basicAuth: true
   oidcAuth: true
+  defaultBucket: false
 
 patroni:
   enabled: true

.github/environments/values.prod.yaml

@@ -1,6 +1,7 @@
 ---
 features:
   basicAuth: true
+  defaultBucket: true
   oidcAuth: true
 
 autoscaling:
@@ -18,15 +19,12 @@ config:
       MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAmHiuPKOkpkq4GXN1ktr23rJtDl6Vdu/Y37ZAd3PnQ8/IDfAODvy1Y81aAUZicKe9egolv+OTRANN3yOg+TAbRhkeXLE5p/473EK0aQ0NazTCuWo6Am3oDQ7Yt8x0pw56/qcLtkTuXNyo5EnVV2Z2BzCnnaL31JOhyitolku0DNT6GDoRBmT4o2ItqEVHk5nM25cf1t2zbwI2790W6if1B2qVRkxxivS8tbH7nYC61Is3XCPockKptkH22cm2ZQJmtYd5sZKuXaGsvtyzHmn8/l0Kd1xnHmUu4JNuQ67YiNZGu3hOkrF0Js3BzAk1Qm4kvYRaxbJFCs/qokLZ4Z0W9wIDAQAB
     KC_REALM: standard
     KC_SERVERURL: "https://loginproxy.gov.bc.ca/auth"
-    OBJECTSTORAGE_BUCKET: egejyy
-    OBJECTSTORAGE_TEMP_EXPIRESIN: "300"
-    OBJECTSTORAGE_ENDPOINT: "https://nrs.objectstore.gov.bc.ca"
-    # OBJECTSTORAGE_KEY: ~
     SERVER_BODYLIMIT: 30mb
     # SERVER_LOGFILE: ~
     SERVER_LOGLEVEL: http
     SERVER_PORT: "3000"
     SERVER_PRIVACY_MASK: "true"
+    SERVER_TEMP_EXPIRESIN: "300"
 
 patroni:
   enabled: true

.github/environments/values.test.yaml

@@ -1,6 +1,7 @@
 ---
 features:
   basicAuth: true
+  defaultBucket: false
   oidcAuth: true
 
 autoscaling:
@@ -18,15 +19,12 @@ config:
       MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAiFdv9GA83uHuy8Eu9yiZHGGF9j6J8t7FkbcpaN81GDjwbjsIJ0OJO9dKRAx6BAtTC4ubJTBJMPvQER5ikOhIeBi4o25fg61jpgsU6oRZHkCXc9gX6mrjMjbsPaf3/bjjYxP5jicBDJQeD1oRa24+tiGggoQ7k6gDEN+cRYqqNpzC/GQbkUPk8YsgroncEgu8ChMh/3ERsLV2zorchMANUq76max16mHrhtWIQxrb/STpSt4JuSlUzzBV/dcXjJe5gywZHe0jAutFhNqjHzHdgyaC4RAd3eYQo+Kl/JOgy2AZrnx+CiPmvOJKe9tAW4k4H087ng8aVE40v4HW/FEbnwIDAQAB
     KC_REALM: standard
     KC_SERVERURL: "https://test.loginproxy.gov.bc.ca/auth"
-    OBJECTSTORAGE_BUCKET: egejyy
-    OBJECTSTORAGE_TEMP_EXPIRESIN: "300"
-    OBJECTSTORAGE_ENDPOINT: "https://nrs.objectstore.gov.bc.ca"
-    # OBJECTSTORAGE_KEY: ~
     SERVER_BODYLIMIT: 30mb
     # SERVER_LOGFILE: ~
     SERVER_LOGLEVEL: http
     SERVER_PORT: "3000"
     SERVER_PRIVACY_MASK: "true"
+    SERVER_TEMP_EXPIRESIN: "300"
 
 patroni:
   enabled: true

app/README.md

@@ -82,9 +82,9 @@ The following variables enable and enforce the use of OIDC Bearer Authentication
 
 | Config Var | Env Var | Default | Notes |
 | --- | --- | --- | --- |
+| `enabled` | `OBJECTSTORAGE_ENABLED` | | Whether to run COMS with a default bucket |
 | `accessKeyId` | `OBJECTSTORAGE_ACCESSKEYID` | | The Access Key for your S3 compatible object storage account  |
 | `bucket` | `OBJECTSTORAGE_BUCKET` | | The object storage bucket name |
-| `defaultTempExpiresIn` | `OBJECTSTORAGE_TEMP_EXPIRESIN` | 300 | The expiry time for pre-signed URLs to objects in seconds  |
 | `endpoint` | `OBJECTSTORAGE_ENDPOINT` | | Object store URL. eg: `https://nrs.objectstore.gov.bc.ca` |
 | `key` | `OBJECTSTORAGE_KEY` | | The base path for storage location |
 | `secretAccessKey` | `OBJECTSTORAGE_SECRETACCESSKEY` | | The Secret Access Key for your S3 compatible object storage account |
@@ -96,6 +96,7 @@ The following variables alter the general Express application behavior. For most
 | Config Var | Env Var | Default | Notes |
 | --- | --- | --- | --- |
 | `bodyLimit` | `SERVER_BODYLIMIT` | 30mb | Maximum body size accepted for parsing to JSON body |
+| `defaultTempExpiresIn` | `SERVER_TEMP_EXPIRESIN` | 300 | The expiry time for pre-signed S3 URLs to objects in seconds  |
 | `logFile` | `SERVER_LOGFILE` | | Writes logs to the following file only if defined |
 | `logLevel` | `SERVER_LOGLEVEL` | http | The logging level of COMS |
 | `passphrase` | `SERVER_PASSPHRASE` | | A key to encrypt/decrypt bucket secretAccessKey's saved to the database |
@@ -122,6 +123,7 @@ Run COMS in **Unauthenticated mode** (replace environment values as necessary)
 
 ``` sh
 docker run -it --rm -p 3000:3000 \
+  -e OBJECTSTORAGE_ENABLED=true \
   -e OBJECTSTORAGE_ACCESSKEYID=<Access Key ID for your S3 account> \
   -e OBJECTSTORAGE_BUCKET=<Object storage bucket name> \
   -e OBJECTSTORAGE_ENDPOINT=<Object store URL. eg: https://nrs.objectstore.gov.bc.ca> \
@@ -134,6 +136,7 @@ Run COMS in **Basic Auth mode** (replace environment values as necessary)
 
 ``` sh
 docker run -it --rm -p 3000:3000 \
+  -e OBJECTSTORAGE_ENABLED=true \
   -e OBJECTSTORAGE_ACCESSKEYID=<Access Key ID for your S3 account> \
   -e OBJECTSTORAGE_BUCKET=<Object storage bucket name> \
   -e OBJECTSTORAGE_ENDPOINT=<Object store URL. eg: https://nrs.objectstore.gov.bc.ca> \
@@ -158,6 +161,7 @@ Run COMS in **OIDC Auth Mode** (replace environment values as necessary)
 
 ``` sh
 docker run -it --rm -p 3000:3000 \
+  -e OBJECTSTORAGE_ENABLED=true \
   -e OBJECTSTORAGE_ACCESSKEYID=<Access Key ID for your S3 account> \
   -e OBJECTSTORAGE_BUCKET=<Object storage bucket name> \
   -e OBJECTSTORAGE_ENDPOINT=<Object store URL. eg: https://nrs.objectstore.gov.bc.ca> \
@@ -178,6 +182,7 @@ Run COMS in **Full Auth Mode** (replace environment values as necessary)
 
 ``` sh
 docker run -it --rm -p 3000:3000 \
+  -e OBJECTSTORAGE_ENABLED=true \
   -e OBJECTSTORAGE_ACCESSKEYID=<Access Key ID for your S3 account> \
   -e OBJECTSTORAGE_BUCKET=<Object storage bucket name> \
   -e OBJECTSTORAGE_ENDPOINT=<Object store URL. eg: https://nrs.objectstore.gov.bc.ca> \
@@ -233,6 +238,7 @@ To run COMS in Full Auth mode you will want your `local.json` to have the follow
     "serverUrl": "<OIDC server auth URL>"
   },
   "objectStorage": {
+    "enabled": true,
     "secretAccessKey": "<Secret Access Key for your S3 compatible object storage account>",
     "key": "<base path for storage location>",
     "accessKeyId": "<Access Key ID for your S3 account>",

app/app.js

@@ -13,6 +13,7 @@ const QueueManager = require('./src/components/queueManager');
 const { getAppAuthMode, getGitRevision } = require('./src/components/utils');
 const DataConnection = require('./src/db/dataConnection');
 const v1Router = require('./src/routes/v1');
+const { readUnique } = require('./src/services/bucket');
 
 const dataConnection = new DataConnection();
 const queueManager = new QueueManager();
@@ -215,6 +216,12 @@ function initializeConnections() {
       if (state.connections.data) {
         log.info('DataConnection Reachable', { function: 'initializeConnections' });
       }
+      if (config.has('objectStorage.enabled')) {
+        readUnique(config.get('objectStorage')).then(() => {
+          log.error('Default bucket cannot also exist in database', { function: 'initializeConnections' });
+          fatalErrorHandler();
+        }).catch(() => { });
+      }
     })
     .catch(error => {
       log.error(`Initialization failed: Database OK = ${state.connections.data}`, { function: 'initializeConnections' });

app/config/custom-environment-variables.json

@@ -25,13 +25,14 @@
   "objectStorage": {
     "accessKeyId": "OBJECTSTORAGE_ACCESSKEYID",
     "bucket": "OBJECTSTORAGE_BUCKET",
-    "defaultTempExpiresIn": "OBJECTSTORAGE_TEMP_EXPIRESIN",
+    "enabled": "OBJECTSTORAGE_ENABLED",
     "endpoint": "OBJECTSTORAGE_ENDPOINT",
     "key": "OBJECTSTORAGE_KEY",
     "secretAccessKey": "OBJECTSTORAGE_SECRETACCESSKEY"
   },
   "server": {
     "bodyLimit": "SERVER_BODYLIMIT",
+    "defaultTempExpiresIn": "SERVER_TEMP_EXPIRESIN",
     "hardReset": "SERVER_HARDRESET",
     "logFile": "SERVER_LOGFILE",
     "logLevel": "SERVER_LOGLEVEL",

app/config/default.json

@@ -7,11 +7,9 @@
     "poolMax": "10",
     "username": "app"
   },
-  "objectStorage": {
-    "defaultTempExpiresIn": "300"
-  },
   "server": {
     "bodyLimit": "30mb",
+    "defaultTempExpiresIn": "300",
     "logLevel": "http",
     "maxRetries": "3",
     "port": "3000"

app/src/components/utils.js

@@ -61,42 +61,44 @@ const utils = {
 
   /**
    * @function getBucket
-   * Acquire core S3 bucket credential information with graceful default fallback
-   * @param {string} [bucketId=undefined] An optional bucketId to lookup
-   * @param {boolean} [throwable=false] Throws an error if no `bucketId` is found
+   * Acquire core S3 bucket credential information from database or configuration
+   * @param {string} [bucketId=undefined] An optional bucket ID to query database for bucket
    * @returns {object} An object containing accessKeyId, bucket, endpoint, key,
    * region and secretAccessKey attributes
-   * @throws If there are no records found with `bucketId` and `throwable` is true
+   * @throws If there are no records found with `bucketId` or, if `bucketId` is undefined,
+   * no bucket details exist in the configuration
    */
-  async getBucket(bucketId = undefined, throwable = false) {
-    const data = {
-      accessKeyId: config.get('objectStorage.accessKeyId'),
-      bucket: config.get('objectStorage.bucket'),
-      endpoint: config.get('objectStorage.endpoint'),
-      key: config.get('objectStorage.key'),
-      region: DEFAULTREGION,
-      secretAccessKey: config.get('objectStorage.secretAccessKey')
-    };
-
-    if (bucketId) {
-      // Function scoped import to avoid circular dependencies
-      const { bucketService } = require('../services');
-
-      try {
-        const bucketData = await bucketService.read(bucketId);
+  async getBucket(bucketId = undefined) {
+    try {
+      const data = { region: DEFAULTREGION };
+      if (bucketId) {
+        // Function scoped import to avoid circular dependencies
+        const { read } = require('../services/bucket');
+        const bucketData = await read(bucketId);
+
         data.accessKeyId = bucketData.accessKeyId;
         data.bucket = bucketData.bucket;
         data.endpoint = bucketData.endpoint;
         data.key = bucketData.key;
         data.secretAccessKey = bucketData.secretAccessKey;
         if (bucketData.region) data.region = bucketData.region;
-      } catch (err) {
-        log.warn(err.message, { function: 'getBucket' });
-        if (throwable) throw new Problem(404, { details: err.message });
+      } else if (config.has('objectStorage') && config.has('objectStorage.enabled')) {
+        data.accessKeyId = config.get('objectStorage.accessKeyId');
+        data.bucket = config.get('objectStorage.bucket');
+        data.endpoint = config.get('objectStorage.endpoint');
+        data.key = config.get('objectStorage.key');
+        data.secretAccessKey = config.get('objectStorage.secretAccessKey');
+        if (config.has('objectStorage.region')) {
+          data.region = config.get('objectStorage.region');
+        }
+      } else {
+        throw new Error('Unable to get bucket');
       }
+      return data;
+    } catch (err) {
+      log.error(err.message, { function: 'getBucket' });
+      throw new Problem(404, { details: err.message });
     }
-
-    return data;
   },
 
   /**

app/src/controllers/object.js

@@ -239,7 +239,7 @@ const controller = {
       }
 
       // Preflight existence check for bucketId
-      const { key: bucketKey } = await getBucket(bucketId, true);
+      const { key: bucketKey } = await getBucket(bucketId);
 
       const objId = uuidv4();
       const data = {
@@ -373,7 +373,7 @@ const controller = {
       }
 
       // Preflight existence check for bucketId
-      const { key: bucketKey } = await getBucket(bucketId, true);
+      const { key: bucketKey } = await getBucket(bucketId);
 
       bb.on('file', async (name, stream, info) => {
         try {
@@ -1059,7 +1059,7 @@ const controller = {
 
       // Preflight existence check for bucketId
       const bucketId = req.currentObject?.bucketId;
-      const { key: bucketKey } = await getBucket(bucketId, true);
+      const { key: bucketKey } = await getBucket(bucketId);
 
       const filename = req.currentObject?.path.match(/(?!.*\/)(.*)$/)[0];
       const objId = addDashesToUuid(req.params.objectId);
@@ -1196,7 +1196,7 @@ const controller = {
 
       // Preflight existence check for bucketId
       const bucketId = req.currentObject?.bucketId;
-      const { key: bucketKey } = await getBucket(bucketId, true);
+      const { key: bucketKey } = await getBucket(bucketId);
 
       bb.on('file', async (name, stream, info) => {
         try {

app/src/services/storage.js

@@ -27,7 +27,7 @@ const utils = require('../components/utils');
 const DELIMITER = '/';
 
 // Get app configuration
-const defaultTempExpiresIn = parseInt(config.get('objectStorage.defaultTempExpiresIn'), 10);
+const defaultTempExpiresIn = parseInt(config.get('server.defaultTempExpiresIn'), 10);
 
 /**
  * The Core S3 Object Storage Service
@@ -207,7 +207,7 @@ const objectStorageService = {
    */
   async headBucket(options = {}) {
     const data = options.bucketId
-      ? await utils.getBucket(options.bucketId, true)
+      ? await utils.getBucket(options.bucketId)
       : options;
     const params = {
       Bucket: data.bucket,

app/tests/unit/components/utils.spec.js

@@ -160,16 +160,15 @@ describe('getBucket', () => {
   };
   const readBucketSpy = jest.spyOn(bucketService, 'read');
 
-  beforeEach(() => {
+  it('should return config data when it exists, given no bucketId', async () => {
+    config.has.mockReturnValue(true);
     config.get
       .mockReturnValueOnce(cdata.accessKeyId) // objectStorage.accessKeyId
       .mockReturnValueOnce(cdata.bucket) // objectStorage.bucket
       .mockReturnValueOnce(cdata.endpoint) // objectStorage.endpoint
       .mockReturnValueOnce(cdata.key) // objectStorage.key
-      .mockReturnValueOnce(cdata.secretAccessKey); // objectStorage.secretAccessKey
-  });
-
-  it('should return config data given no bucketId and not throwable', async () => {
+      .mockReturnValueOnce(cdata.secretAccessKey) // objectStorage.secretAccessKey
+      .mockReturnValueOnce(cdata.region); // objectStorage.region
 
     const result = await utils.getBucket();
 
@@ -184,22 +183,16 @@ describe('getBucket', () => {
     expect(readBucketSpy).toHaveBeenCalledTimes(0);
   });
 
-  it('should return config data given no bucketId and is throwable', async () => {
+  it('should throw when no config data exists, given no bucketId', async () => {
+    config.has.mockReturnValue(false);
 
-    const result = await utils.getBucket(undefined, true);
+    const result = (() => utils.getBucket(undefined))();
 
-    expect(result).toBeTruthy();
-    expect(result).toEqual(cdata);
-    expect(result).toHaveProperty('accessKeyId', cdata.accessKeyId);
-    expect(result).toHaveProperty('bucket', cdata.bucket);
-    expect(result).toHaveProperty('endpoint', cdata.endpoint);
-    expect(result).toHaveProperty('key', cdata.key);
-    expect(result).toHaveProperty('region', cdata.region);
-    expect(result).toHaveProperty('secretAccessKey', cdata.secretAccessKey);
+    expect(result).rejects.toThrow();
     expect(readBucketSpy).toHaveBeenCalledTimes(0);
   });
 
-  it('should return database data given a good bucketId and not throwable', async () => {
+  it('should return database data given a good bucketId', async () => {
     readBucketSpy.mockResolvedValue(ddata);
 
     const result = await utils.getBucket('bucketId');
@@ -215,31 +208,14 @@ describe('getBucket', () => {
     expect(readBucketSpy).toHaveBeenCalledWith('bucketId');
   });
 
-  it('should return config data given a bad bucketId and not throwable', async () => {
-    readBucketSpy.mockImplementation(() => { throw new Problem(422); });
-
-    const result = await (() => utils.getBucket('bucketId'))();
-
-    expect(result).toBeTruthy();
-    expect(result).toEqual(cdata);
-    expect(result).toHaveProperty('accessKeyId', cdata.accessKeyId);
-    expect(result).toHaveProperty('bucket', cdata.bucket);
-    expect(result).toHaveProperty('endpoint', cdata.endpoint);
-    expect(result).toHaveProperty('key', cdata.key);
-    expect(result).toHaveProperty('region', cdata.region);
-    expect(result).toHaveProperty('secretAccessKey', cdata.secretAccessKey);
-    expect(readBucketSpy).toHaveBeenCalledTimes(1);
-    expect(readBucketSpy).toHaveBeenCalledWith('bucketId');
-  });
-
-  it('should throw given a bucketId and is throwable', () => {
+  it('should throw given a bad bucketId', () => {
     readBucketSpy.mockImplementation(() => { throw new Problem(422); });
 
-    const result = (() => utils.getBucket('bucketId', true))();
+    const result = (() => utils.getBucket('bad bucketId'))();
 
     expect(result).rejects.toThrow();
     expect(readBucketSpy).toHaveBeenCalledTimes(1);
-    expect(readBucketSpy).toHaveBeenCalledWith('bucketId');
+    expect(readBucketSpy).toHaveBeenCalledWith('bad bucketId');
   });
 });