Repository: scimmyjs/scimmy (js)

Issue:
Error "Binaries have to be case-exact by definition" when connecting Keycloak (SCIM-Client) to scimmy
When trying to define scimmy as an SCIM-Server in Keycloak, I'm getting the following error when loading the provider configuration.

`errorMessage: The attribute with the name 'urn:ietf:params:scim:schemas:core:2.0:User:x509Certificates.value' has an invalid declaration. Binaries have to be case-exact by definition.`



Pull request:
Force `caseExact` and `uniqueness` config values to be `true` and `none` when attribute type is `binary`
Currently, attributes of type `binary` can be defined with a `caseExact` configuration value of `false`, and a `uniqueness` configuration value of anything other than `none`. According to [RFC7643§2.3.6](https://datatracker.ietf.org/doc/html/rfc7643#section-2.3.6), binary attributes are always case-exact and have no uniqueness.

This change ensures attributes with type `binary` will, by default, have `caseExact` and `uniqueness` configuration values of `false` and `none` respectively. It also adds checks to any custom configuration of `binary` attributes to ensure these values cannot be changed (fixes #44). Test fixtures have also been updated to reflect this change, and to more thoroughly check behaviour when invalid attribute configuration is attempted either at instantiation, or afterward.

Patch:
diff --git a/src/lib/schemas/user.js b/src/lib/schemas/user.js
index 0252cee..c16ac5c 100644
--- a/src/lib/schemas/user.js
+++ b/src/lib/schemas/user.js
@@ -86,7 +86,7 @@ export class User extends Types.Schema {
             new Types.Attribute("boolean", "primary", {description: "A Boolean value indicating the 'primary' or preferred attribute value for this attribute. The primary attribute value 'true' MUST appear no more than once."})
         ]),
         new Types.Attribute("complex", "x509Certificates", {multiValued: true, uniqueness: false, description: "A list of certificates issued to the User."}, [
-            new Types.Attribute("binary", "value", {description: "The value of an X.509 certificate."}),
+            new Types.Attribute("binary", "value", {description: "The value of an X.509 certificate.", caseExact: true}),
             new Types.Attribute("string", "display", {description: "A human-readable name, primarily used for display purposes. READ-ONLY."}),
             new Types.Attribute("string", "type", {canonicalValues: [], description: "A label indicating the attribute's function."}),
             new Types.Attribute("boolean", "primary", {description: "A Boolean value indicating the 'primary' or preferred attribute value for this attribute. The primary attribute value 'true' MUST appear no more than once."})
diff --git a/src/lib/types/attribute.js b/src/lib/types/attribute.js
index cfabbff..4d258b4 100644
--- a/src/lib/types/attribute.js
+++ b/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 patch:
diff --git a/test/lib/schemas/user.json b/test/lib/schemas/user.json
index 5fe42c4..4ec02b3 100644
--- a/test/lib/schemas/user.json
+++ b/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."
           },
           {
diff --git a/test/lib/types/attribute.js b/test/lib/types/attribute.js
index c219cc6..d0dff8b 100644
--- a/test/lib/types/attribute.js
+++ b/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")),