Repository: rickbergfalk/sqlpad (js)

Issue:
Persist user record for noauth
It might be problematic to have a user id reference to a user that doesn't exist now that `createdBy`/`updatedBy` act as FKs to `user.id`

Pull request description:
Add `noauth` user record and `disableAuth` tests
* Adds `noauth` user record when `disableAuth` is enabled
* Updates `disableAuth` implementation to use custom passport strategy
* Adds `disableAuth` test cases

closes #751 

Patch:
diff --git a/server/app.js b/server/app.js
index 07926658..3b8eaae7 100644
--- a/server/app.js
+++ b/server/app.js
@@ -29,7 +29,7 @@ BigInt.prototype.toJSON = function () {
  * Create an express app using config
  * @param {object} config
  */
-function makeApp(config, models) {
+async function makeApp(config, models) {
   if (typeof config.get !== 'function') {
     throw new Error('config is required to create app');
   }
@@ -111,7 +111,7 @@ function makeApp(config, models) {
 
   /*  Passport setup
   ============================================================================= */
-  authStrategies(config);
+  await authStrategies(config, models);
   app.use(passport.initialize());
   app.use(passport.session());
 
diff --git a/server/auth-strategies/disable-auth.js b/server/auth-strategies/disable-auth.js
new file mode 100644
index 00000000..3ca94d7e
--- /dev/null
+++ b/server/auth-strategies/disable-auth.js
@@ -0,0 +1,66 @@
+require('../typedefs');
+const passport = require('passport');
+const passportCustom = require('passport-custom');
+const CustomStrategy = passportCustom.Strategy;
+const appLog = require('../lib/app-log');
+
+function getNoAuthUser(config) {
+  const role =
+    config.get('disableAuthDefaultRole') === 'admin' ? 'admin' : 'editor';
+
+  return {
+    id: 'noauth',
+    name: 'noauth',
+    role,
+    email: 'noauth@example.com',
+  };
+}
+
+/**
+ * A noauth / disableAuth custom strategy
+ * When enabled, it always authenticates the request to whatever noauth user is configured
+ * Using passport here probably isn't necessary, but it keeps the user/session objects consistent
+ * @param {Req} req
+ * @param {function} done
+ */
+async function disableAuthStrategy(req, done) {
+  try {
+    const { config } = req;
+    const noAuthUser = getNoAuthUser(config);
+    return done(null, noAuthUser);
+  } catch (error) {
+    done(error);
+  }
+}
+
+/**
+ * Adds auth proxy strategy if configured
+ * Adds a noauth user to users table if it does not yet exist
+ * @param {object} config
+ * @param {import('../models')} models
+ */
+async function enableDisableAuth(config, models) {
+  if (config.get('disableAuth')) {
+    appLog.info('Enabling disableAuth "authentication" strategy.');
+
+    const noAuthUser = getNoAuthUser(config);
+
+    // Try to find existing user in SQLPad's backing db
+    // This tries to find a match on both id and email, with id taking priority
+    let existingUser = await models.users.findOneById(noAuthUser.id);
+    if (!existingUser) {
+      await models.users.create(noAuthUser);
+    } else if (
+      existingUser.name !== noAuthUser.name ||
+      existingUser.role !== noAuthUser.role ||
+      existingUser.email !== noAuthUser.email
+    ) {
+      const { id, ...updates } = noAuthUser;
+      await models.users.update(id, updates);
+    }
+
+    passport.use('disable-auth', new CustomStrategy(disableAuthStrategy));
+  }
+}
+
+module.exports = enableDisableAuth;
diff --git a/server/auth-strategies/index.js b/server/auth-strategies/index.js
index 486c9907..3faac895 100644
--- a/server/auth-strategies/index.js
+++ b/server/auth-strategies/index.js
@@ -1,6 +1,7 @@
 const passport = require('passport');
 const authProxy = require('./auth-proxy');
 const basic = require('./basic');
+const disableAuth = require('./disable-auth');
 const google = require('./google');
 const jwtServiceToken = require('./jwt-service-token');
 const ldap = require('./ldap');
@@ -35,10 +36,12 @@ passport.deserializeUser(async function (req, id, done) {
 /**
  * Register auth strategies (if configured)
  * @param {object} config
+ * @param {object} models
  */
-function authStrategies(config) {
+async function authStrategies(config, models) {
   authProxy(config);
   basic(config);
+  await disableAuth(config, models);
   google(config);
   jwtServiceToken(config);
   ldap(config);
diff --git a/server/middleware/sessionless-auth.js b/server/middleware/sessionless-auth.js
index 4e9f641f..84dea1a0 100644
--- a/server/middleware/sessionless-auth.js
+++ b/server/middleware/sessionless-auth.js
@@ -42,15 +42,10 @@ function sessionlessAuth(req, res, next) {
     return next();
   }
 
-  // If auth is disabled, hardcode a user and continue on
+  // If auth is disabled, "authenticate" with the custom disable-auth strategy
+  // This will stub in a noauth user into the users table, and associate the session accordingly
   if (config.get('disableAuth')) {
-    req.user = {
-      id: 'noauth',
-      role:
-        config.get('disableAuthDefaultRole') === 'admin' ? 'admin' : 'editor',
-      email: 'test@example.com',
-    };
-    return next();
+    return passport.authenticate('disable-auth', handleAuth)(req, res, next);
   }
 
   // If authorization header is present, attempt to authenticate based on the type of auth header
diff --git a/server/server.js b/server/server.js
index a420024d..5034081e 100755
--- a/server/server.js
+++ b/server/server.js
@@ -106,7 +106,7 @@ async function startServer() {
   }, CLEANING_MS);
 
   // Create expressjs app
-  const app = makeApp(config, models);
+  const app = await makeApp(config, models);
 
   // determine if key pair exists for certs
   if (keyPath && certPath) {


Test patch:
diff --git a/server/test/auth/disable-auth.js b/server/test/auth/disable-auth.js
new file mode 100644
index 00000000..6b1cee97
--- /dev/null
+++ b/server/test/auth/disable-auth.js
@@ -0,0 +1,55 @@
+const assert = require('assert');
+const request = require('supertest');
+const TestUtil = require('../utils');
+
+describe('auth/disable-auth', function () {
+  it('signs in with noauth user', async function () {
+    const utils = new TestUtil({
+      disableAuth: true,
+    });
+    await utils.init();
+
+    const { body } = await request(utils.app).get('/api/users').expect(200);
+    let user = body[0];
+    assert.equal(user.id, 'noauth');
+    assert.equal(user.role, 'editor');
+    assert.equal(user.name, 'noauth');
+    assert.equal(user.email, 'noauth@example.com');
+
+    user = await utils.models.users.findOneById('noauth');
+    assert.equal(user.id, 'noauth');
+    assert.equal(user.role, 'editor');
+    assert.equal(user.name, 'noauth');
+    assert.equal(user.email, 'noauth@example.com');
+  });
+
+  it('disableAuthDefaultRole editor', async function () {
+    const utils = new TestUtil({
+      disableAuth: true,
+      disableAuthDefaultRole: 'editor',
+    });
+    await utils.init();
+
+    const { body } = await request(utils.app).get('/api/users').expect(200);
+    let user = body[0];
+    assert.equal(user.role, 'editor');
+
+    user = await utils.models.users.findOneById('noauth');
+    assert.equal(user.role, 'editor');
+  });
+
+  it('disableAuthDefaultRole admin', async function () {
+    const utils = new TestUtil({
+      disableAuth: true,
+      disableAuthDefaultRole: 'admin',
+    });
+    await utils.init();
+
+    const { body } = await request(utils.app).get('/api/users').expect(200);
+    let user = body[0];
+    assert.equal(user.role, 'admin');
+
+    user = await utils.models.users.findOneById('noauth');
+    assert.equal(user.role, 'admin');
+  });
+});
diff --git a/server/test/middleware/passport-auth-proxy.js b/server/test/auth/passport-auth-proxy.js
similarity index 99%
rename from server/test/middleware/passport-auth-proxy.js
rename to server/test/auth/passport-auth-proxy.js
index 1d46577d..4c856312 100644
--- a/server/test/middleware/passport-auth-proxy.js
+++ b/server/test/auth/passport-auth-proxy.js
@@ -2,7 +2,7 @@ const assert = require('assert');
 const request = require('supertest');
 const TestUtil = require('../utils');
 
-describe('passport-proxy-auth', function () {
+describe('auth/passport-proxy-auth', function () {
   it('auto sign up creates user w/default role', async function () {
     const utils = new TestUtil({
       authProxyEnabled: true,
diff --git a/server/test/utils.js b/server/test/utils.js
index c23a71c3..0d362029 100644
--- a/server/test/utils.js
+++ b/server/test/utils.js
@@ -158,7 +158,7 @@ class TestUtils {
     await this.loadSeedData();
     await ensureConnectionAccess(this.sequelizeDb, this.config);
 
-    this.app = makeApp(this.config, this.models);
+    this.app = await makeApp(this.config, this.models);
 
     assert.throws(() => {
       db.makeDb(this.config, this.instanceAlias);