From: Justin Wind <justin.wind+git@gmail.com>
Date: Sat, 11 May 2024 18:39:27 +0000 (-0700)
Subject: minor internal refactors
X-Git-Tag: v1.4.1~2
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=8e6eeb4ff8fa853f200d3ed8a61ac31932371851;hp=9c8e775e5ab96a1788f535760bfa72205c430d15;p=squeep-authentication-module

minor internal refactors
---

diff --git a/lib/authenticator.js b/lib/authenticator.js
index eab1a1a..3df0b9d 100644
--- a/lib/authenticator.js
+++ b/lib/authenticator.js
@@ -351,14 +351,15 @@ class Authenticator {
 
   /**
    * Check for valid Basic auth, updates ctx with identifier if valid.
-   * @param {string} credentials basic auth field (decoded)
+   * @param {string} authValue basic auth value (base64)
    * @param {object} ctx context
    * @returns {Promise<boolean>} is valid
    */
-  async isValidBasic(credentials, ctx) {
+  async isValidBasic(authValue, ctx) {
     const _scope = _fileScope('isValidBasic');
     this.logger.debug(_scope, 'called', { ctx });
 
+    const credentials = Buffer.from(authValue, 'base64').toString('utf-8');
     const [identifier, credential] = common.splitFirst(credentials, ':', '');
 
     return this.isValidIdentifierCredential(identifier, credential, ctx);
@@ -379,8 +380,7 @@ class Authenticator {
     // eslint-disable-next-line sonarjs/no-small-switch
     switch (authMethod.toLowerCase()) {
       case 'basic': {
-        const credentials = Buffer.from(authString, 'base64').toString('utf-8'); // FIXME: move into isValidBasic, why is it here?
-        return this.isValidBasic(credentials, ctx);
+        return this.isValidBasic(authString, ctx);
       }
 
       default:
@@ -567,17 +567,22 @@ class Authenticator {
     const _scope = _fileScope('apiRequiredLocal');
     this.logger.debug(_scope, 'called', { ctx, sessionAlsoValid });
 
-    // If a Authorization header was provided, never consider session as a fallback.
-    const authorizationHeader = req.getHeader(Enum.Header.Authorization);
-    if (authorizationHeader) {
-      if (await this.isValidAuthorization(authorizationHeader, ctx)) {
-        this.logger.debug(_scope, 'valid authorization', { ctx, sessionAlsoValid });
+    try {
+      // If a Authorization header was provided, never consider session as a fallback.
+      const authorizationHeader = req.getHeader(Enum.Header.Authorization);
+      if (authorizationHeader) {
+        if (await this.isValidAuthorization(authorizationHeader, ctx)) {
+          this.logger.debug(_scope, 'valid authorization', { ctx, sessionAlsoValid });
+          return true;
+        }
+      } else if (sessionAlsoValid
+      &&         await this.sessionCheck(req, res, ctx, undefined, false, false)) {
+        this.logger.debug(_scope, 'valid session', { ctx, sessionAlsoValid });
         return true;
       }
-    } else if (sessionAlsoValid
-    &&         await this.sessionCheck(req, res, ctx, undefined, false, false)) {
-      this.logger.debug(_scope, 'valid session', { ctx, sessionAlsoValid });
-      return true;
+    } catch (e) {
+      this.logger.error(_scope, 'failed', { error: e });
+      throw e;
     }
 
     this.logger.debug(_scope, 'invalid authorization', { ctx, sessionAlsoValid });
diff --git a/test/lib/authenticator.js b/test/lib/authenticator.js
index 7142081..84ee1c5 100644
--- a/test/lib/authenticator.js
+++ b/test/lib/authenticator.js
@@ -140,13 +140,14 @@ describe('Authenticator', function () {
   }); // _validateAuthDataCredential
 
   describe('isValidBasic', function () {
+    const b64 = (x) => Buffer.from(x).toString('base64');
     it('succeeds', async function () {
       _authMechanismRequired(authenticator, 'argon2');
       authenticator.db.authenticationGet.resolves({
         identifier,
         credential,
       });
-      const authString = `${identifier}:${password}`;
+      const authString = b64(`${identifier}:${password}`);
       const result = await authenticator.isValidBasic(authString, ctx);
       assert.strictEqual(result, true);
       assert.strictEqual(ctx.authenticationId, identifier);
@@ -157,14 +158,14 @@ describe('Authenticator', function () {
         identifier,
         credential,
       });
-      const authString = `${identifier}:wrongPassword}`;
+      const authString = b64(`${identifier}:wrongPassword}`);
       const result = await authenticator.isValidBasic(authString, ctx);
       assert.strictEqual(result, false);
       assert.strictEqual(ctx.authenticationId, undefined);
     });
     it('covers no entry', async function() {
       authenticator.db.authenticationGet.resolves();
-      const authString = `${identifier}:wrongPassword}`;
+      const authString = b64(`${identifier}:wrongPassword}`);
       const result = await authenticator.isValidBasic(authString, ctx);
       assert.strictEqual(result, false);
       assert.strictEqual(ctx.authenticationId, undefined);
@@ -174,7 +175,7 @@ describe('Authenticator', function () {
         identifier,
         credential: '$other$kind_of_credential',
       });
-      const authString = `${identifier}:wrongPassword}`;
+      const authString = b64(`${identifier}:wrongPassword}`);
       const result = await authenticator.isValidBasic(authString, ctx);
       assert.strictEqual(result, false);
       assert.strictEqual(ctx.authenticationId, undefined);
@@ -577,7 +578,7 @@ describe('Authenticator', function () {
     it('covers missing basic auth, ignores session', async function () {
       req.getHeader.returns();
       sinon.stub(authenticator, 'isValidAuthorization').resolves(true);
-      assert.rejects(authenticator.apiRequiredLocal(req, res, ctx, false), {
+      assert.rejects(() => authenticator.apiRequiredLocal(req, res, ctx, false), {
         name: 'ResponseError',
         statusCode: 401,
       });
@@ -585,6 +586,11 @@ describe('Authenticator', function () {
       assert(!authenticator.isValidAuthorization.called);
       assert(res.setHeader.called);
     });
+    it('covers errors', async function () {
+      sinon.stub(authenticator, 'isValidAuthorization').rejects();
+      req.getHeader.returns('Basic Zm9vOmJhcg==');
+      assert.rejects(() => authenticator.apiRequiredLocal(req, res, ctx));
+    });
   }); // apiRequiredLocal
 
 }); // Authenticator