fix: fix validation of object IDs (#348)

openfga · Sep 25, 2024 · dc0d9f1 · dc0d9f1
2 parents 230df3f + 1dfc019
commit dc0d9f1
Show file tree

Hide file tree

Showing 7 changed files with 155 additions and 4 deletions.
diff --git a/pkg/go/validation/validation-rules.go b/pkg/go/validation/validation-rules.go
@@ -11,7 +11,7 @@ const (
 	RuleType      Rule = "[^:#@\\*\\s]{1,254}"
 	RuleRelation  Rule = "[^:#@\\*\\s]{1,50}"
 	RuleCondition Rule = "[^\\*\\s]{1,50}"
-	RuleID        Rule = "[^#:\\*\\s]+"
+	RuleID        Rule = "[^#:\\s*][a-zA-Z0-9_|*@.+]*"
 	RuleObject    Rule = "[^\\s]{2,256}"
 )
 
@@ -22,6 +22,12 @@ func ValidateObject(object string) bool {
 	return typeMatch && objectMatch
 }
 
+func ValidateObjectID(relation string) bool {
+	match, _ := regexp.MatchString(fmt.Sprintf("^%s$", RuleID), relation)
+
+	return match
+}
+
 func ValidateRelation(relation string) bool {
 	match, _ := regexp.MatchString(fmt.Sprintf("^%s$", RuleRelation), relation)
 

diff --git a/pkg/go/validation/validation-rules_test.go b/pkg/go/validation/validation-rules_test.go
@@ -24,6 +24,43 @@ func validateBadStructure(t *testing.T, validator func(string) bool) {
 	assert.False(t, validator("item:**"))
 }
 
+func TestValidateObjectID(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		value    string
+		expected bool
+	}{
+		{"document1", true},                   // Should pass valid ID
+		{"doc_123", true},                     // Should pass valid ID with underscore
+		{"[email protected]", true},             // Should pass valid email-like ID
+		{"file.name", true},                   // Should pass valid ID with dot
+		{"data+set", true},                    // Should pass valid ID with plus
+		{"pipe|char", true},                   // Should pass valid ID with pipe
+		{"star*char", true},                   // Should pass valid ID with star
+		{"underscore_", true},                 // Should pass valid ID with underscore
+		{"pipe|[email protected]", true}, // Should pass valid complex ID
+		{"#document1", false},                 // Should fail if starts with #
+		{":doc123", false},                    // Should fail if starts with :
+		{" doc123", false},                    // Should fail if starts with space
+		{"doc*123", true},                     // Should pass valid ID with star
+		{"doc:123", false},                    // Should fail if contains :
+		{"doc#123", false},                    // Should fail if contains #
+		{"doc 123", false},                    // Should fail if contains space
+		{"doc*", true},                        // Should pass valid ID with star
+		{"doc:", false},                       // Should fail if ends with :
+		{"    doc", false},                    // Should fail if starts with space
+	}
+
+	for _, test := range tests {
+		t.Run(test.value, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, test.expected, ValidateObjectID(test.value))
+		})
+	}
+
+}
+
 func TestValidateObject(t *testing.T) {
 	t.Parallel()
 

diff --git a/pkg/java/src/main/java/dev/openfga/language/validation/Validator.java b/pkg/java/src/main/java/dev/openfga/language/validation/Validator.java
@@ -6,14 +6,16 @@ public class Rules {
         public static final String TYPE = "[^:#@\\*\\s]{1,254}";
         public static final String RELATION = "[^:#@\\*\\s]{1,50}";
         public static final String CONDITION = "[^\\*\\s]{1,50}";
-        public static final String ID = "[^#:\\*\\s]+";
+        public static final String ID = "[^#:\\s*][a-zA-Z0-9_|*@.+]*";
         public static final String OBJECT = "[^\\s]{2,256}";
     }
 
     public static class Regexes {
         public static final ValidationRegex object =
                 ValidationRegex.build("object", String.format("^%s$", Rules.OBJECT));
 
+        public static final ValidationRegex objectId = ValidationRegex.build("object", String.format("^%s$", Rules.ID));
+
         public static final ValidationRegex typeId =
                 ValidationRegex.build("object", String.format("^%s:%s$", Rules.TYPE, Rules.ID));
 
@@ -40,6 +42,10 @@ public static boolean validateObject(String object) {
         return Regexes.typeId.matches(object) && Regexes.object.matches(object);
     }
 
+    public static boolean validateObjectId(String objectId) {
+        return Regexes.objectId.matches(objectId);
+    }
+
     public static boolean validateRelation(String relation) {
         return Regexes.relation.matches(relation);
     }

diff --git a/pkg/java/src/test/java/dev/openfga/language/validation/ValidationRules.java b/pkg/java/src/test/java/dev/openfga/language/validation/ValidationRules.java
@@ -43,6 +43,30 @@ public void ruleObjectTest() {
         validatedBadStructure(Validator::validateObject);
     }
 
+    @Test
+    public void testValidateObjectId() {
+        // Valid cases
+        assertTrue(Validator.Regexes.objectId.matches("document1"));
+        assertTrue(Validator.Regexes.objectId.matches("doc_123"));
+        assertTrue(Validator.Regexes.objectId.matches("[email protected]"));
+        assertTrue(Validator.Regexes.objectId.matches("file.name"));
+        assertTrue(Validator.Regexes.objectId.matches("data+set"));
+        assertTrue(Validator.Regexes.objectId.matches("pipe|char"));
+        assertTrue(Validator.Regexes.objectId.matches("star*char"));
+        assertTrue(Validator.Regexes.objectId.matches("underscore_"));
+        assertTrue(Validator.Regexes.objectId.matches("pipe|[email protected]"));
+
+        // Invalid cases
+        assertFalse(Validator.Regexes.objectId.matches("#document1"));
+        assertFalse(Validator.Regexes.objectId.matches(":doc123"));
+        assertFalse(Validator.Regexes.objectId.matches(" doc123"));
+        assertFalse(Validator.Regexes.objectId.matches("doc:123"));
+        assertFalse(Validator.Regexes.objectId.matches("doc#123"));
+        assertFalse(Validator.Regexes.objectId.matches("doc 123"));
+        assertFalse(Validator.Regexes.objectId.matches("doc:"));
+        assertFalse(Validator.Regexes.objectId.matches("    doc"));
+    }
+
     @Test
     public void ruleUserTest() {
 

diff --git a/pkg/js/tests/validate-rules.test.ts b/pkg/js/tests/validate-rules.test.ts
@@ -172,4 +172,78 @@ describe("Validation Rules", () => {
 
     validatedBadStructure(Validator.type);
   });
+
+  describe("Rule 'id'", () => {
+    it("should pass 'document1'", () => {
+      expect(Validator.objectId("document1")).toBeTruthy();
+    });
+
+    it("should pass 'doc_123'", () => {
+      expect(Validator.objectId("doc_123")).toBeTruthy();
+    });
+
+    it("should pass '[email protected]'", () => {
+      expect(Validator.objectId("[email protected]")).toBeTruthy();
+    });
+
+    it("should pass 'file.name'", () => {
+      expect(Validator.objectId("file.name")).toBeTruthy();
+    });
+
+    it("should pass 'data+set'", () => {
+      expect(Validator.objectId("data+set")).toBeTruthy();
+    });
+
+    it("should pass 'pipe|char'", () => {
+      expect(Validator.objectId("pipe|char")).toBeTruthy();
+    });
+
+    it("should pass 'star*char'", () => {
+      expect(Validator.objectId("star*char")).toBeTruthy();
+    });
+
+    it("should pass 'underscore_'", () => {
+      expect(Validator.objectId("underscore_")).toBeTruthy();
+    });
+
+    it("should pass 'pipe|[email protected]'", () => {
+      expect(Validator.objectId("pipe|[email protected]")).toBeTruthy();
+    });
+
+    it("should fail '#document1'", () => {
+      expect(Validator.objectId("#document1")).toBeFalsy();
+    });
+
+    it("should fail ':doc123'", () => {
+      expect(Validator.objectId(":doc123")).toBeFalsy();
+    });
+
+    it("should fail ' doc123'", () => {
+      expect(Validator.objectId(" doc123")).toBeFalsy();
+    });
+
+    it("should fail 'doc*123'", () => {
+      expect(Validator.objectId("doc*123")).toBeTruthy();
+    });
+
+    it("should fail 'doc:123'", () => {
+      expect(Validator.objectId("doc:123")).toBeFalsy();
+    });
+
+    it("should fail 'doc#123'", () => {
+      expect(Validator.objectId("doc#123")).toBeFalsy();
+    });
+
+    it("should fail 'doc 123'", () => {
+      expect(Validator.objectId("doc 123")).toBeFalsy();
+    });
+
+    it("should fail 'doc*'", () => {
+      expect(Validator.objectId("doc*")).toBeTruthy();
+    });
+
+    it("should fail 'doc:'", () => {
+      expect(Validator.objectId("doc:")).toBeFalsy();
+    });
+  });
 });
diff --git a/pkg/js/tests/valdiate-store.test.ts → pkg/js/tests/validate-store.test.ts b/pkg/js/tests/valdiate-store.test.ts → pkg/js/tests/validate-store.test.ts
@@ -13,7 +13,7 @@ describe("validate valid store file", () => {
   testCases.forEach((testCase) => {
     const testFn = testCase.skip ? it.skip : it;
 
-    testFn(`should valdiate ${testCase.name} `, () => {
+    testFn(`should validate ${testCase.name} `, () => {
       const schemaValidator: ValidateFunction = YamlStoreValidator();
 
       const yaml = YAML.parseDocument(fs.readFileSync(testCase.store).toString());

diff --git a/pkg/js/validator/validate-rules.ts b/pkg/js/validator/validate-rules.ts
@@ -2,7 +2,7 @@ export const Rules = {
   type: "[^:#@\\*\\s]{1,254}",
   relation: "[^:#@\\*\\s]{1,50}",
   condition: "[^\\*\\s]{1,50}",
-  id: "[^#:\\*\\s]+",
+  id: "[^#:\\s*][a-zA-Z0-9_|*@.+]*",
   object: "[^\\s]{2,256}",
 };
 
@@ -42,6 +42,10 @@ export const Validator = {
   type: (type: string): boolean => {
     return validateFieldValue(`^${Rules.type}$`, type);
   },
+  // ObjectId name
+  objectId: (id: string): boolean => {
+    return validateFieldValue(`^${Rules.id}$`, id);
+  },
 };
 
 const validateFieldValue = (rule: string, value: string): boolean => {