auth cleanups

diff --git a/src/authenticator.js b/src/authenticator.js
index a3a0638c6ad011403e03e1120fac1d1af3d12798..eeb6f201244e72f1f1bade2416031f1909608700 100644 (file)
--- a/src/authenticator.js
+++ b/src/authenticator.js
@@ -119,7 +119,7 @@ class Authenticator {
     await this.db.context(async (dbCtx) => {
       authData = await this.db.getAuthById(dbCtx, authenticationId);
     });
-    const secret = authData && authData.secret;
+    const secret = authData?.secret;
 
     if (!secret) {
       this.logger.debug(_scope, 'failed, invalid authenticationId', { ctx });
@@ -171,20 +171,26 @@ class Authenticator {
       return false;
     }
 
-    // Update pwhash
-    // authData.password = await argon2.hash(newPassword, { type: argon2.id });
     if (authData.password.startsWith('$argon2')) {
       if (await argon2.verify(authData.password, authenticationPass)) {
+        this.logger.debug(_scope, 'passed argon2 verify', { ctx });
+      } else {
         this.logger.debug(_scope, 'failed argon2 verify', { ctx });
         return false;
-      } else {
-        this.logger.debug(_scope, 'passed argon2 verify', { ctx });
       }
     } else {
-      if (authData.password !== authenticationPass) {
+      if (authData.password.length !== authenticationPass.length
+      ||  !crypto.timingSafeEqual(Buffer.from(authData.password), Buffer.from(authenticationPass))
+      ) {
         this.logger.debug(_scope, 'failed, password mismatch', { ctx });
         return false;
       }
+      // Update pwhash
+      const credential = await argon2.hash(authenticationPass, { type: argon2.argon2id });
+      await this.db.context(async (dbCtx) => {
+        await this.db.upsertAuth(dbCtx, authenticationId, authData.secret, credential);
+      });
+      this.logger.debug(_scope, 'migrated plain password', { ctx, authenticationId });
     }
 
     ctx.authenticationId = authenticationId;
@@ -319,7 +325,7 @@ class Authenticator {
       return this.requestBasic(res);
     }
 
-    const linkId = ctx.params && ctx.params.id;
+    const linkId = ctx?.params?.id;
     // If there is an id parameter, check for a valid token query parameter
     if (linkId) {
       authData = (ctx?.queryParams?.token) || (ctx?.parsedBody?.token);
@@ -367,9 +373,9 @@ class Authenticator {
     }
 
     // Allow a valid plain token.
-    const linkId = ctx.params && ctx.params.id;
+    const linkId = ctx?.params?.id;
     if (linkId) {
-      const token = (ctx.queryParams && ctx.queryParams.token) || (ctx.parsedBody && ctx.parsedBody.token);
+      const token = ctx?.queryParams?.token || ctx?.parsedBody?.token;
       if (token) {
         const validToken = await this.isValidToken(linkId, token);
         if (validToken) {

diff --git a/src/db/base.js b/src/db/base.js
index b109efd729323df837138114476035fe7abf4bf1..6bce83b800eadcaf1ea25e1b6b156c401c8dddb4 100644 (file)
--- a/src/db/base.js
+++ b/src/db/base.js
@@ -39,6 +39,10 @@ class BaseDatabase {
     this._notImplemented('getAuthById', { dbCtx, id });
   }
 
+  async upsertAuth(dbCtx, id, secert, credential) {
+    this._notImplemented('upsertAuthCredential', { dbCtx, id, credential });
+  }
+
   async insertLink(dbCtx, id, url, authToken) {
     this._notImplemented('insertLink', { dbCtx, id, url, authToken });
   }

diff --git a/src/db/postgres/index.js b/src/db/postgres/index.js
index e46a40113aee0db6d954ceb7852d8467a6500034..7549133a72604f24c58464c542836cbc65d4bdbd 100644 (file)
--- a/src/db/postgres/index.js
+++ b/src/db/postgres/index.js
@@ -198,6 +198,16 @@ class PostgresDatabase extends BaseDatabase {
   }
 
 
+  async upsertAuth(dbCtx, id, secret, credential) {
+    const _scope = _fileScope('upsertAuth');
+    this.logger.debug(_scope, 'called', { id });
+    dbCtx = dbCtx || this.db;
+
+    const result = await dbCtx.result(this.statement.authUpsert, { id, secret, credential });
+    this.logger.debug(_scope, 'result', PostgresDatabase._resultLog(result) );
+  }
+
+
   static _epochFix(epoch) {
     switch (epoch) {
       case Infinity:

diff --git a/src/db/postgres/sql/auth-upsert.sql b/src/db/postgres/sql/auth-upsert.sql
new file mode 100644 (file)
index 0000000..f90be73
--- /dev/null
+++ b/src/db/postgres/sql/auth-upsert.sql
@@ -0,0 +1,4 @@
+--
+INSERT INTO auth (id, secret, password) VALUES ($(id), $(secret), $(credential))
+ON CONFLICT (id) DO
+UPDATE SET password = $(credential), secret = $(secret)

diff --git a/src/db/sqlite/index.js b/src/db/sqlite/index.js
index 39b9b6b1250b6bb6d21c744c318c7ff34440820e..e1ce14dc58ddbb3536ef7bf68c015ceba7fb7c1d 100644 (file)
--- a/src/db/sqlite/index.js
+++ b/src/db/sqlite/index.js
@@ -93,6 +93,8 @@ class SQLiteDatabase extends BaseDatabase {
       _commit: this.db.prepare('COMMIT'),
       _rollback: this.db.prepare('ROLLBACK'),
       getAuthById: this.db.prepare('SELECT * FROM auth WHERE id = :id'),
+      insertAuth: this.db.prepare('INSERT INTO auth (id, secret, password) VALUES (:id, :secret, :credential)'),
+      updateAuth: this.db.prepare('UPDATE auth SET password = :credential, secret = :secret WHERE id = :id'),
       getLinkById: this.db.prepare('SELECT * FROM link WHERE id = :id'),
       getLinkByUrl: this.db.prepare('SELECT * FROM link WHERE url = :url'),
       insertLink: this.db.prepare('INSERT INTO link (id, url, auth_token) VALUES (:id, :url, :authToken)'),
@@ -168,6 +170,37 @@ class SQLiteDatabase extends BaseDatabase {
     return auth;
   }
 
+  async upsertAuth(dbCtx, id, secret, credential) {
+    const _scope = _fileScope('upsertAuthCredential');
+    this.logger.debug(_scope, 'called', { id });
+
+    let info;
+    try {
+      info = this.statement.insertAuth.run({ id, secret, credential });
+    } catch (e) {
+      switch (e.code) {
+        case 'SQLITE_CONSTRAINT_UNIQUE':
+        case 'SQLITE_CONSTRAINT_PRIMARYKEY': {
+          this.logger.debug(_scope, 'updating existing auth', { id });
+          info = this.statement.updateAuth.run({ id, secret, credential });
+          break;
+        }
+
+        default: {
+          this.logger.error(_scope, 'failed to upsert auth credential', { error: e, id });
+          throw e;
+        }
+      }
+    }
+    this.logger.debug(_scope, 'run', { info });
+    if (info.changes != 1) {
+      this.logger.error(_scope, 'failed to upsert auth credential', { id, info });
+      throw new DBErrors.UnexpectedResult();
+    }
+
+    return this._sqliteInfo(info);
+  }
+
   async insertLink(dbCtx, id, url, authToken) {
     const _scope = _fileScope('insertLink');
     this.logger.debug(_scope, 'called', { id, url });

diff --git a/test/src/authenticator.js b/test/src/authenticator.js
index 42629deb2b4a420a9475c1972255ebc8cb865385..23230981e3380b85bf60a1f64d5312c54b778aa4 100644 (file)
--- a/test/src/authenticator.js
+++ b/test/src/authenticator.js
@@ -19,6 +19,7 @@ describe('Authenticator', function () {
       context: async (fn) => fn({}),
       getAuthById: async () => {},
       getLinkById: async () => {},
+      upsertAuth: async () => {},
     };
     authenticator = new Authenticator(logger, db, options);
   });
@@ -182,29 +183,50 @@ describe('Authenticator', function () {
 
     beforeEach(function () {
       sinon.stub(authenticator, 'requestBasic');
+      sinon.stub(authenticator.db, 'getAuthById');
+      sinon.stub(authenticator.db, 'upsertAuth');
       credentials = 'id:password';
       ctx = {};
     });
 
-    it('accepts credentials', async function () {
-      sinon.stub(authenticator.db, 'getAuthById').resolves({ password: 'password' });
+    it('accepts plain credential and migrates to hash', async function () {
+      authenticator.db.getAuthById.resolves({ password: 'password' });
       const result = await authenticator.isValidBasic(credentials, ctx);
       assert.strictEqual(result, true);
       assert.strictEqual(ctx.authenticationId, 'id');
+      assert(authenticator.db.upsertAuth.called);
     });
 
-    it('rejects wrong password', async function () {
-      sinon.stub(authenticator.db, 'getAuthById').resolves({ password: 'wrong_password' });
+    it('rejects wrong plain credential', async function () {
+      authenticator.db.getAuthById.resolves({ password: 'wrong_password' });
       const result = await authenticator.isValidBasic(credentials, ctx);
       assert.strictEqual(result, false);
       assert(!('authenticationId' in ctx));
+      assert(authenticator.db.upsertAuth.notCalled);
+    });
+
+    it('accepts argon2 credential', async function () {
+      authenticator.db.getAuthById.resolves({ password: '$argon2id$v=19$m=65536,t=3,p=4$AQKIWU5puGDs3zKIPMo0Ew$Mzl/kzJE6/oRtJLHoGXaoUtlAiXs5HK2qLgHWF6euF8' });
+      const result = await authenticator.isValidBasic(credentials, ctx);
+      assert.strictEqual(result, true);
+      assert.strictEqual(ctx.authenticationId, 'id');
+      assert(authenticator.db.upsertAuth.notCalled);
+    });
+
+    it('rejects wrong argon2 credential', async function () {
+      authenticator.db.getAuthById.resolves({ password: '$argon2id$v=19$m=65536,t=3,p=4$BCPlf0NBgjyXOxdyUDs/FQ$wV4jERm50yByCpSr8lrD8Nu0uVPUsQcVghJQoix5ido' });
+      const result = await authenticator.isValidBasic(credentials, ctx);
+      assert.strictEqual(result, false);
+      assert(!('authenticationId' in ctx));
+      assert(authenticator.db.upsertAuth.notCalled);
     });
 
     it('rejects missing id', async function () {
-      sinon.stub(authenticator.db, 'getAuthById').resolves();
+      authenticator.db.getAuthById.resolves();
       const result = await authenticator.isValidBasic(credentials, ctx);
       assert.strictEqual(result, false);
       assert(!('authenticationId' in ctx));
+      assert(authenticator.db.upsertAuth.notCalled);
     });
   }); // isValidBasic
 

diff --git a/test/src/db/base.js b/test/src/db/base.js
index 72d96ac2fd9583ddddebb45d3f4b5e0d378a182e..9752b6afa55e8cf6610de2db951ab154fc8d73b8 100644 (file)
--- a/test/src/db/base.js
+++ b/test/src/db/base.js
@@ -42,6 +42,7 @@ describe('BaseDatabase', function () {
         'context',
         'transaction',
         'getAuthById',
+        'upsertAuth',
         'insertLink',
         'getLinkById',
         'getLinkByUrl',

diff --git a/test/src/db/postgres/index.js b/test/src/db/postgres/index.js
index d52fc9024b65c4e52411d832c5c443ccf0a0d8c1..61825ad2261546a5869a6abed2e9631f39d2a14b 100644 (file)
--- a/test/src/db/postgres/index.js
+++ b/test/src/db/postgres/index.js
@@ -44,6 +44,13 @@ describe('PostgresDatabase', function () {
     dbCtx = undefined;
   });
 
+  it('covers constructor options', function () {
+    options = {
+      queryLogLevel: 'debug',
+    };
+    db = new PostgresDatabase(logger, options, pgpStub);
+  });
+
   describe('context', function () {
     it('covers', async function () {
       const fn = sinon.stub();
@@ -86,6 +93,18 @@ describe('PostgresDatabase', function () {
     });
   }); // getAuthById
 
+  describe('upsertAuth', function () {
+    let id, secret, credential;
+    beforeEach(function () {
+      id = 'id';
+      secret = 'secret';
+      credential = 'credential';
+    });
+    it('stubbed success', async function () {
+      await db.upsertAuth(dbCtx, id, secret, credential);
+    });
+  }); // upsertAuth
+
   describe('_epochFix', function () {
     it('clamps infinity', function () {
       const epoch = Infinity;

diff --git a/test/src/db/sqlite/index.js b/test/src/db/sqlite/index.js
index b73b34f92bf36f11b5020f0882afef2e610c97d8..f437482c31cf37bb80e606070cab8ecbf2d8e986 100644 (file)
--- a/test/src/db/sqlite/index.js
+++ b/test/src/db/sqlite/index.js
@@ -35,6 +35,16 @@ describe('SQLiteDatabase', function () {
       await db.transaction(dbCtx, fn);
       assert(fn.called);
     });
+    it('covers rollback', async function () {
+      const fn = sinon.stub();
+      fn.throws(new Error('rollback'));
+      try {
+        await db.transaction(dbCtx, fn);
+        assert.fail(noExpectedException);
+      } catch (e) {
+        assert.strictEqual(e.message, 'rollback', noExpectedException);
+      }
+    });
   }); // transaction
 
   describe('getAuthById', function () {
@@ -67,6 +77,45 @@ describe('SQLiteDatabase', function () {
     });
   }); // getAuthById
 
+  describe('upsertAuth', function () {
+    let id, secret, credential;
+    beforeEach(function () {
+      sinon.stub(db.statement.insertAuth, 'run').returns({ changes: 1n, lastInsertRowid: 123n });
+      sinon.stub(db.statement.updateAuth, 'run').returns({ changes: 1n, lastInsertRowid: 123n });
+    });
+    it('stubbed insert success', async function () {
+      await db.upsertAuth(dbCtx, id, secret, credential);
+    });
+    it('stubbed update success', async function () {
+      db.statement.insertAuth.run.throws({ code: 'SQLITE_CONSTRAINT_UNIQUE' });
+      await db.upsertAuth(dbCtx, id, secret, credential);
+    });
+    it('covers error', async function () {
+      const expectedException = new Error('blah');
+      db.statement.insertAuth.run.throws(expectedException);
+      try {
+        await db.upsertAuth(dbCtx, id, secret, credential);
+        assert.fail(noExpectedException);
+      } catch (e) {
+        assert.deepStrictEqual(e, expectedException, noExpectedException);
+      }
+    });
+    it('covers unexpected error', async function () {
+      const expectedException = DBErrors.UnexpectedResult;
+      const returns = {
+        changes: 0n,
+        lastInsertRowid: undefined,
+      };
+      db.statement.insertAuth.run.returns(returns);
+      try {
+        await db.upsertAuth(dbCtx, id, secret, credential);
+        assert.fail(noExpectedException);
+      } catch (e) {
+        assert(e instanceof expectedException, noExpectedException);
+      }
+    });
+  }); // upsertAuth
+
   describe('insertLink', function () {
     let id, url, authToken;
     beforeEach(function () {