From 2ac59b772f5417694962e4c1c21e4469c456e4e8 Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Fri, 14 Aug 2020 17:16:10 +0200
Subject: [PATCH] refactor: only allow objects as `claims` configuration
 parameter

BREAKING CHANGE: The `claims` configuration property can no longer be
a `Map` instance, only plain objects are allowed.
---
 lib/helpers/claims.js                    |  2 +-
 lib/helpers/configuration.js             | 42 ++++++------------------
 test/configuration/constructor.test.js   | 16 ---------
 test/configuration/scopes_claims.test.js | 18 +++++-----
 4 files changed, 19 insertions(+), 59 deletions(-)

diff --git a/lib/helpers/claims.js b/lib/helpers/claims.js
index 244a766df..c44ba61b5 100644
--- a/lib/helpers/claims.js
+++ b/lib/helpers/claims.js
@@ -26,7 +26,7 @@ module.exports = function getClaims(provider) {
     scope(value = '') {
       assert(!Object.keys(this.filter).length, 'scope cannot be assigned after mask has been set');
       value.split(' ').forEach((scope) => {
-        this.mask(claimConfig.get(scope));
+        this.mask(claimConfig[scope]);
       });
       return this;
     }
diff --git a/lib/helpers/configuration.js b/lib/helpers/configuration.js
index fa7b3015f..4d28b20f9 100644
--- a/lib/helpers/configuration.js
+++ b/lib/helpers/configuration.js
@@ -68,11 +68,8 @@ class Configuration {
     featuresTypeErrorCheck(this);
     clientAuthDefaults(this.clientDefaults);
 
-    this.fixMapClaims(); // TODO: remove the Map option in 7.x
-
     this.logDraftNotice();
 
-    this.ensureMaps();
     this.ensureSets();
 
     this.fixResponseTypes();
@@ -110,25 +107,6 @@ class Configuration {
     }
   }
 
-  fixMapClaims() { // TODO: remove the Map option in 7.x
-    if (this.claims instanceof Map) {
-      Object.entries(this.defaults.claims).forEach(([key, value]) => {
-        if (!this.claims.has(key)) {
-          this.claims.set(key, value);
-        }
-      });
-    }
-  }
-
-  ensureMaps() {
-    if (!(this.claims instanceof Map)) {
-      if (!isPlainObject(this.claims)) {
-        throw new TypeError('claims must be a plain javascript object or Map');
-      }
-      this.claims = new Map(Object.entries(this.claims));
-    }
-  }
-
   ensureSets() {
     [
       'scopes', 'subjectTypes', 'extraParams', 'acrValues',
@@ -191,7 +169,7 @@ class Configuration {
 
   collectScopes() {
     const claimDefinedScopes = [];
-    this.claims.forEach((value, key) => {
+    Object.entries(this.claims).forEach(([key, value]) => {
       if (isPlainObject(value) || Array.isArray(value)) {
         claimDefinedScopes.push(key);
       }
@@ -213,38 +191,38 @@ class Configuration {
   }
 
   unpackArrayClaims() {
-    this.claims.forEach((value, key) => {
+    Object.entries(this.claims).forEach(([key, value]) => {
       if (Array.isArray(value)) {
-        this.claims.set(key, value.reduce((accumulator, claim) => {
+        this.claims[key] = value.reduce((accumulator, claim) => {
           const scope = accumulator;
           scope[claim] = null;
           return scope;
-        }, {}));
+        }, {});
       }
     });
   }
 
   ensureOpenIdSub() {
-    if (!Object.keys(this.claims.get('openid')).includes('sub')) {
-      this.claims.get('openid').sub = null;
+    if (!Object.keys(this.claims.openid).includes('sub')) {
+      this.claims.openid.sub = null;
     }
   }
 
   removeAcrIfEmpty() {
     if (!this.acrValues.size) {
-      this.claims.delete('acr');
+      delete this.claims.acr;
     }
   }
 
   collectClaims() {
     const claims = new Set();
     this.scopes.forEach((scope) => {
-      if (this.claims.has(scope)) {
-        Object.keys(this.claims.get(scope)).forEach(Set.prototype.add.bind(claims));
+      if (scope in this.claims) {
+        Object.keys(this.claims[scope]).forEach(Set.prototype.add.bind(claims));
       }
     });
 
-    this.claims.forEach((value, key) => {
+    Object.entries(this.claims).forEach(([key, value]) => {
       if (value === null) claims.add(key);
     });
 
diff --git a/test/configuration/constructor.test.js b/test/configuration/constructor.test.js
index 57fa5a05d..a73de7495 100644
--- a/test/configuration/constructor.test.js
+++ b/test/configuration/constructor.test.js
@@ -72,22 +72,6 @@ describe('Provider configuration', () => {
     });
   });
 
-  describe('claims', () => {
-    it('only accepts maps and objects', () => {
-      new Provider('http://localhost:3000', { claims: { foo: null } });
-      new Provider('http://localhost:3000', { claims: new Map(Object.entries({ foo: null })) });
-      expect(() => {
-        const claims = new class {
-          constructor() {
-            this.foo = null;
-          }
-        }();
-
-        new Provider('http://localhost:3000', { claims });
-      }).to.throw('claims must be a plain javascript object or Map');
-    });
-  });
-
   describe('ttl', () => {
     it('checks the values are positive safe integers or functions', () => {
       let throws = [
diff --git a/test/configuration/scopes_claims.test.js b/test/configuration/scopes_claims.test.js
index 010061464..16d2c4218 100644
--- a/test/configuration/scopes_claims.test.js
+++ b/test/configuration/scopes_claims.test.js
@@ -10,7 +10,7 @@ describe('custom claims', () => {
       },
     });
 
-    expect(i(provider).configuration('claims').get('openid')).to.eql({
+    expect(i(provider).configuration('claims').openid).to.eql({
       sub: null,
       foo: null,
     });
@@ -23,22 +23,20 @@ describe('custom claims', () => {
       },
     });
 
-    expect(i(provider).configuration('claims').get('openid')).to.eql({
+    expect(i(provider).configuration('claims').openid).to.eql({
       sub: null,
       foo: null,
     });
   });
 
   it('detects new scopes from claims definition', () => {
-    const claims = new Map(Object.entries({
-      insurance: ['company_name', 'coverage'],
-      payment: {
-        preferred_method: null,
-      },
-    }));
-
     const provider = new Provider('https://op.example.com', {
-      claims,
+      claims: {
+        insurance: ['company_name', 'coverage'],
+        payment: {
+          preferred_method: null,
+        },
+      },
     });
 
     expect(i(provider).configuration('scopes')).to.contain('insurance', 'payment');