Merge pull request #54 from scimmyjs/issue/44-binary-attribute-case-e…

…xact

Force `caseExact` and `uniqueness` config values to be `true` and `none` when attribute type is `binary`

src/lib/types/attribute.js

@@ -27,11 +27,45 @@ const BaseConfiguration = {
      * Proxied configuration validation trap handler
      * @alias ProxiedConfigHandler
      * @param {String} errorSuffix - the suffix to use in thrown type errors
-     * @returns {{set: (function(Object, String, *): boolean)}} the handler trap definition to use in the config proxy
+     * @param {String} type - the attribute type for which configuration is being proxied
+     * @returns {ProxyHandler<Object>} the handler trap definition to use in the config proxy
      * @private
      */
-    handler: (errorSuffix) => ({
+    handler: (errorSuffix, type) => ({
+        deleteProperty: (target, key) => {
+            // Prevent removal of known properties from attribute config
+            if (key in BaseConfiguration.target) throw new TypeError(`Cannot remove known property '${key}' from configuration of ${errorSuffix}`);
+            // Otherwise, delete the property from the target
+            else return Reflect.deleteProperty(target, key);
+        },
+        defineProperty: (target, key, descriptor) => {
+            // Only allow known properties to be defined on attribute config
+            if (key in BaseConfiguration.target) return Reflect.defineProperty(target, key, descriptor);
+            // Otherwise, throw an exception explaining the above
+            else throw new TypeError(`Cannot add unknown property '${key}' to configuration of ${errorSuffix}`);
+        },
+        get: (target, key) => {
+            // For binary attribute configuration...
+            if (type === "binary") {
+                // ...always return true for 'caseExact', and
+                if (key === "caseExact") return true;
+                // ...always return 'none' for 'uniqueness'
+                if (key === "uniqueness") return "none";
+            }
+
+            // Otherwise, return actual value
+            return Reflect.get(target, key);
+        },
         set: (target, key, value) => {
+            // Make sure the property is known before setting any value
+            if (!(key in BaseConfiguration.target))
+                throw new TypeError(`Cannot add unknown property '${key}' to configuration of ${errorSuffix}`);
+            // Make sure binary attributes only accept 'caseExact' values of true
+            if (type === "binary" && key === "caseExact" && value !== true)
+                throw new TypeError(`Attribute type 'binary' must specify 'caseExact' value as 'true' in ${errorSuffix}`);
+            // Make sure binary attributes only accept 'uniqueness' values of 'none'
+            if (type === "binary" && key === "uniqueness" && value !== "none")
+                throw new TypeError(`Attribute type 'binary' must specify 'uniqueness' value as 'none' in ${errorSuffix}`);
             // Make sure required, multiValued, and caseExact are booleans
             if (["required", "multiValued", "caseExact", "shadow"].includes(key) && (value !== undefined && typeof value !== "boolean"))
                 throw new TypeError(`Attribute '${key}' value must be either 'true' or 'false' in ${errorSuffix}`);
@@ -49,7 +83,7 @@ const BaseConfiguration = {
             }
 
             // Set the value!
-            return (target[key] = value) || true;
+            return Reflect.set(target, key, value);
         }
     })
 };
@@ -334,8 +368,7 @@ export class Attribute {
         this.name = name;
 
         // Prevent addition and removal of properties from config
-        this.config = Object.seal(Object
-            .assign(new Proxy({...BaseConfiguration.target}, BaseConfiguration.handler(errorSuffix)), config));
+        this.config = Object.assign(Object.seal(new Proxy({...BaseConfiguration.target}, BaseConfiguration.handler(errorSuffix, type))), config);
 
         // Store subAttributes, and make sure any additions are also attribute instances
         if (type === "complex") this.subAttributes = new Proxy([...subAttributes], {

test/lib/schemas/user.json

@@ -324,7 +324,7 @@
         "subAttributes": [
           {
             "name": "value", "type": "binary", "multiValued": false, "required": false,
-            "caseExact": false, "mutability": "readWrite", "returned": "default", "uniqueness": "none",
+            "caseExact": true, "mutability": "readWrite", "returned": "default", "uniqueness": "none",
             "description": "The value of an X.509 certificate."
           },
           {

test/lib/types/attribute.js

@@ -136,6 +136,28 @@ describe("SCIMMY.Types.Attribute", () => {
             });
         }
 
+        for (let attrib of ["required", "multiValued", "caseExact", "shadow"]) {
+            it(`should only accept boolean '${attrib}' configuration values`, () => {
+                for (let [type, value] of [["string", "a string"], ["number", 1], ["complex", {}], ["date", new Date()]]) {
+                    assert.throws(() => new Attribute("string", "test", {[attrib]: value}),
+                        {name: "TypeError", message: `Attribute '${attrib}' value must be either 'true' or 'false' in attribute definition 'test'`},
+                        `Attribute instantiated with invalid '${attrib}' configuration ${type} value${typeof value === "object" ? "" : ` '${value}'`}`);
+                }
+            });
+        }
+
+        it("should expect 'caseExact' value to be 'true' when attribute type is 'binary'", () => {
+            assert.throws(() => new Attribute("binary", "test", {caseExact: false}),
+                {name: "TypeError", message: "Attribute type 'binary' must specify 'caseExact' value as 'true' in attribute definition 'test'"},
+                "Attribute instance did not expect 'caseExact' configuration value to be 'true' when attribute type was 'binary'");
+        });
+
+        it("should expect 'uniqueness' value to be 'none' when attribute type is 'binary'", () => {
+            assert.throws(() => new Attribute("binary", "test", {uniqueness: false}),
+                {name: "TypeError", message: "Attribute type 'binary' must specify 'uniqueness' value as 'none' in attribute definition 'test'"},
+                "Attribute instance did not expect 'uniqueness' configuration value to be 'none' when attribute type was 'binary'");
+        });
+
         it("should be frozen after instantiation", () => {
             const attribute = new Attribute("string", "test");
 
@@ -151,6 +173,103 @@ describe("SCIMMY.Types.Attribute", () => {
         });
     });
 
+    describe("#config", () => {
+        it("should not allow setting values for unknown properties", () => {
+            const {config} = new Attribute("string", "test");
+
+            assert.throws(() => config.test = true,
+                {name: "TypeError", message: "Cannot add unknown property 'test' to configuration of attribute definition 'test'"},
+                "Instance member 'config' allowed unknown property addition after instantiation");
+        });
+
+        it("should not allow definition of unknown properties", () => {
+            const {config} = new Attribute("string", "test");
+
+            assert.throws(() => Object.defineProperty(config, "test", {value: 42, writable: false}),
+                {name: "TypeError", message: "Cannot add unknown property 'test' to configuration of attribute definition 'test'"},
+                "Instance member 'config' allowed unknown property definition after instantiation");
+        });
+
+        it("should not allow known properties to be removed", () => {
+            const {config} = new Attribute("string", "test");
+
+            for (let key of Object.keys(config)) {
+                assert.throws(() => delete config[key],
+                    {name: "TypeError", message: `Cannot remove known property '${key}' from configuration of attribute definition 'test'`},
+                    `Instance member 'config' allowed removal of known property '${key}' after instantiation`);
+            }
+        });
+
+        it("should ignore removal of unknown properties", () => {
+            const {config} = new Attribute("string", "test");
+
+            try {
+                delete config.test;
+            } catch (ex) {
+                assert.fail(`Instance member 'config' did not ignore removal of unknown property 'test'\r\n${ex.stack}`);
+            }
+        });
+
+        for (let attrib of ["canonicalValues", "referenceTypes"]) {
+            it(`should not accept invalid '${attrib}' values`, () => {
+                const {config} = new Attribute("string", "test");
+
+                for (let value of ["a string", true]) {
+                    assert.throws(() => config[attrib] = value,
+                        {name: "TypeError", message: `Attribute '${attrib}' value must be either a collection or 'false' in attribute definition 'test'`},
+                        `Instance member 'config' accepted invalid '${attrib}' value '${value}'`);
+                }
+            });
+        }
+
+        for (let [attrib, name = attrib] of [["mutable", "mutability"], ["returned"], ["uniqueness"]]) {
+            it(`should not accept invalid '${attrib}' values`, () => {
+                const {config} = new Attribute("string", "test");
+
+                assert.throws(() => config[attrib] = "a string",
+                    {name: "TypeError", message: `Attribute '${name}' value 'a string' not recognised in attribute definition 'test'`},
+                    `Instance member 'config' accepted invalid '${attrib}' value 'a string'`);
+                assert.throws(() => config[attrib] = 1,
+                    {name: "TypeError", message: `Attribute '${name}' value must be either string or boolean in attribute definition 'test'`},
+                    `Instance member 'config' accepted invalid '${attrib}' number value '1'`);
+                assert.throws(() => config[attrib] = {},
+                    {name: "TypeError", message: `Attribute '${name}' value must be either string or boolean in attribute definition 'test'`},
+                    `Instance member 'config' accepted invalid '${attrib}' complex value`);
+                assert.throws(() => config[attrib] = new Date(),
+                    {name: "TypeError", message: `Attribute '${name}' value must be either string or boolean in attribute definition 'test'`},
+                    `Instance member 'config' accepted invalid '${attrib}' date value`);
+            });
+        }
+
+        for (let attrib of ["required", "multiValued", "caseExact", "shadow"]) {
+            it(`should only accept boolean '${attrib}' values`, () => {
+                const {config} = new Attribute("string", "test");
+
+                for (let [type, value] of [["string", "a string"], ["number", 1], ["complex", {}], ["date", new Date()]]) {
+                    assert.throws(() => config[attrib] = value,
+                        {name: "TypeError", message: `Attribute '${attrib}' value must be either 'true' or 'false' in attribute definition 'test'`},
+                        `Instance member 'config' accepted invalid '${attrib}' configuration ${type} value${typeof value === "object" ? "" : ` '${value}'`}`);
+                }
+            });
+        }
+
+        it("should expect 'caseExact' value to be 'true' when attribute type is 'binary'", () => {
+            const {config} = new Attribute("binary", "test");
+
+            assert.throws(() => config.caseExact = false,
+                {name: "TypeError", message: "Attribute type 'binary' must specify 'caseExact' value as 'true' in attribute definition 'test'"},
+                "Instance member 'config' did not expect 'caseExact' value to be 'true' when attribute type was 'binary'");
+        });
+
+        it("should expect 'uniqueness' value to be 'none' when attribute type is 'binary'", () => {
+            const {config} = new Attribute("binary", "test");
+
+            assert.throws(() => config.uniqueness = true,
+                {name: "TypeError", message: "Attribute type 'binary' must specify 'uniqueness' value as 'none' in attribute definition 'test'"},
+                "Instance member 'config' did not expect 'uniqueness' value to be 'none' when attribute type was 'binary'");
+        });
+    });
+
     describe("#subAttributes", () => {
         it("should not be defined when type is not 'complex'", () => {
             assert.ok(!("subAttributes" in new Attribute("string", "test")),