Merge pull request #312 from Permify/validation-command-bugs

Validation command bugs
Permify · Mar 14, 2023 · b71ca2a · b71ca2a
2 parents c3464ec + d6f8a57
commit b71ca2a
Show file tree

Hide file tree

Showing 9 changed files with 208 additions and 24 deletions.
diff --git a/docs/v1/apidocs.swagger.json b/docs/v1/apidocs.swagger.json
@@ -3,7 +3,7 @@
   "info": {
     "title": "Permify API",
     "description": "Permify is an open-source authorization service for creating and maintaining fine-grained authorizations across your individual applications and services. Permify converts authorization data as relational tuples into a database you point at. We called that database a Write Database (WriteDB) and it behaves as a centralized data source for your authorization system. You can model of your authorization with Permify's DSL - Permify Schema - and perform access checks with a single API call anywhere on your stack. Access decisions made according to stored relational tuples.",
-    "version": "v0.3.3",
+    "version": "v0.3.4",
     "contact": {
       "name": "API Support",
       "url": "https://github.com/Permify/permify/issues",

diff --git a/internal/info.go b/internal/info.go
@@ -2,7 +2,7 @@ package internal
 
 const (
 	// Version is the last release of the Permify (e.g. v0.1.0)
-	Version = "v0.3.3"
+	Version = "v0.3.4"
 
 	// Banner is the view for terminal.
 	Banner = `

diff --git a/internal/servers/relationshipServer.go b/internal/servers/relationshipServer.go
@@ -1,6 +1,8 @@
 package servers
 
 import (
+	"errors"
+
 	"google.golang.org/grpc/status"
 
 	otelCodes "go.opentelemetry.io/otel/codes"
@@ -63,9 +65,10 @@ func (r *RelationshipServer) Write(ctx context.Context, request *v1.Relationship
 	}
 
 	for _, tup := range request.GetTuples() {
-		v = tuple.ValidateSubject(tup.GetSubject())
-		if v != nil {
-			return nil, v
+		if tuple.IsSubjectUser(tup.GetSubject()) {
+			if tup.GetSubject().GetRelation() != "" {
+				return nil, errors.New(v1.ErrorCode_ERROR_CODE_SUBJECT_RELATION_MUST_BE_EMPTY.String())
+			}
 		}
 	}
 

diff --git a/internal/services/relationshipService.go b/internal/services/relationshipService.go
@@ -3,6 +3,8 @@ package services
 import (
 	"context"
 	"errors"
+	"fmt"
+
 	otelCodes "go.opentelemetry.io/otel/codes"
 
 	"github.com/Permify/permify/internal/repositories"
@@ -62,7 +64,14 @@ func (service *RelationshipService) WriteRelationships(ctx context.Context, tena
 		version = v
 	}
 
+	relationships := make([]*base.Tuple, 0, len(tuples))
+
 	for _, tup := range tuples {
+		subject := tup.GetSubject()
+		if !tuple.IsSubjectUser(subject) {
+			subject.Relation = tuple.ELLIPSIS
+		}
+
 		var entity *base.EntityDefinition
 		entity, _, err = service.sr.ReadSchemaDefinition(ctx, tenantID, tup.GetEntity().GetType(), version)
 		if err != nil {
@@ -84,18 +93,28 @@ func (service *RelationshipService) WriteRelationships(ctx context.Context, tena
 			return token, err
 		}
 		for _, t := range rel.GetRelationReferences() {
-			vt = append(vt, t.GetType())
+			if t.GetRelation() != "" {
+				vt = append(vt, fmt.Sprintf("%s#%s", t.GetType(), t.GetRelation()))
+			} else {
+				vt = append(vt, t.GetType())
+			}
 		}
 
-		err = tuple.ValidateSubjectType(tup.GetSubject(), vt)
+		err = tuple.ValidateSubjectType(subject, vt)
 		if err != nil {
 			span.RecordError(err)
 			span.SetStatus(otelCodes.Error, err.Error())
 			return token, err
 		}
+
+		relationships = append(relationships, &base.Tuple{
+			Entity:   tup.GetEntity(),
+			Relation: tup.GetRelation(),
+			Subject:  subject,
+		})
 	}
 
-	return service.rw.WriteRelationships(ctx, tenantID, database.NewTupleCollection(tuples...))
+	return service.rw.WriteRelationships(ctx, tenantID, database.NewTupleCollection(relationships...))
 }
 
 // DeleteRelationships -

diff --git a/pkg/dsl/lexer/lexer_test.go b/pkg/dsl/lexer/lexer_test.go
@@ -450,5 +450,105 @@ entity organization {
 				Expect(index + lexeme.Literal).Should(Equal(index + tt.expectedLiteral))
 			}
 		})
+
+		It("Case 6", func() {
+			str := `
+	entity user {}
+
+	entity organization {
+		relation member @user
+	}
+
+	// This is a role for an entity
+	entity maintainer {
+		relation org @organization#member
+
+		action enabled = org
+	}`
+
+			tests := []struct {
+				expectedType    token.Type
+				expectedLiteral string
+			}{
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.ENTITY, "entity"},
+				{token.SPACE, " "},
+				{token.IDENT, "user"},
+				{token.SPACE, " "},
+				{token.LBRACE, "{"},
+				{token.RBRACE, "}"},
+				{token.NEWLINE, "\n"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.ENTITY, "entity"},
+				{token.SPACE, " "},
+
+				{token.IDENT, "organization"},
+				{token.SPACE, " "},
+				{token.LBRACE, "{"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.TAB, "\t"},
+				{token.RELATION, "relation"},
+				{token.SPACE, " "},
+				{token.IDENT, "member"},
+				{token.SPACE, " "},
+				{token.SIGN, "@"},
+
+				{token.IDENT, "user"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.RBRACE, "}"},
+				{token.NEWLINE, "\n"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.SINGLE_LINE_COMMENT, " This is a role for an entity"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.ENTITY, "entity"},
+				{token.SPACE, " "},
+				{token.IDENT, "maintainer"},
+
+				{token.SPACE, " "},
+				{token.LBRACE, "{"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.TAB, "\t"},
+				{token.RELATION, "relation"},
+				{token.SPACE, " "},
+				{token.IDENT, "org"},
+				{token.SPACE, " "},
+				{token.SIGN, "@"},
+				{token.IDENT, "organization"},
+				{token.HASH, "#"},
+				{token.IDENT, "member"},
+				{token.NEWLINE, "\n"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.TAB, "\t"},
+
+				{token.ACTION, "action"},
+				{token.SPACE, " "},
+				{token.IDENT, "enabled"},
+				{token.SPACE, " "},
+				{token.ASSIGN, "="},
+				{token.SPACE, " "},
+				{token.IDENT, "org"},
+				{token.NEWLINE, "\n"},
+				{token.TAB, "\t"},
+				{token.RBRACE, "}"},
+				{token.EOF, ""},
+			}
+
+			l := NewLexer(str)
+
+			for i, tt := range tests {
+				lexeme := l.NextToken()
+				index := strconv.Itoa(i) + ": "
+				Expect(index + lexeme.Type.String()).Should(Equal(index + tt.expectedType.String()))
+				Expect(index + lexeme.Literal).Should(Equal(index + tt.expectedLiteral))
+			}
+		})
 	})
 })
diff --git a/pkg/pb/base/v1/openapi.pb.go b/pkg/pb/base/v1/openapi.pb.go
diff --git a/pkg/tuple/tuple.go b/pkg/tuple/tuple.go
@@ -32,20 +32,6 @@ func IsSubjectUser(subject *base.Subject) bool {
 	return subject.Type == USER
 }
 
-// ValidateSubject -
-func ValidateSubject(subject *base.Subject) error {
-	if subject.Type == USER {
-		if subject.GetRelation() != "" {
-			return errors.New(base.ErrorCode_ERROR_CODE_SUBJECT_RELATION_MUST_BE_EMPTY.String())
-		}
-	} else {
-		if subject.GetRelation() == "" {
-			return errors.New(base.ErrorCode_ERROR_CODE_SUBJECT_RELATION_CANNOT_BE_EMPTY.String())
-		}
-	}
-	return nil
-}
-
 // AreSubjectsEqual -
 func AreSubjectsEqual(s1, s2 *base.Subject) bool {
 	return s1.GetRelation() == s2.GetRelation() && s1.GetId() == s2.GetId() && s1.GetType() == s2.GetType()

diff --git a/pkg/tuple/tuple_test.go b/pkg/tuple/tuple_test.go
@@ -1,6 +1,7 @@
 package tuple
 
 import (
+	`errors`
 	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -335,5 +336,80 @@ var _ = Describe("tuple", func() {
 				Expect(IsSubjectValid(tt.target)).Should(Equal(tt.expected))
 			}
 		})
+
+		It("ValidateSubjectType", func() {
+			tests := []struct {
+				target        *base.Subject
+				relationTypes []string
+				expected      error
+			}{
+				{
+					target: &base.Subject{
+						Type:     "organization",
+						Id:       "1",
+						Relation: "member",
+					},
+					relationTypes: []string{
+						"organization#member",
+						"user",
+					},
+					expected: nil,
+				},
+				{
+					target: &base.Subject{
+						Type:     "organization",
+						Id:       "1",
+						Relation: "",
+					},
+					relationTypes: []string{
+						"organization",
+					},
+					expected: nil,
+				},
+				{
+					target: &base.Subject{
+						Type:     "user",
+						Id:       "u82",
+						Relation: "",
+					},
+					relationTypes: []string{
+						"user",
+					},
+					expected: nil,
+				},
+				{
+					target: &base.Subject{
+						Type:     "testrel",
+						Id:       "u82",
+						Relation: "",
+					},
+					relationTypes: []string{
+						"test",
+						"user",
+					},
+					expected: errors.New(base.ErrorCode_ERROR_CODE_SUBJECT_TYPE_NOT_FOUND.String()),
+				},
+				{
+					target: &base.Subject{
+						Type:     "test",
+						Id:       "u3",
+						Relation: "mem",
+					},
+					relationTypes: []string{
+						"test#member",
+						"user",
+					},
+					expected: errors.New(base.ErrorCode_ERROR_CODE_SUBJECT_TYPE_NOT_FOUND.String()),
+				},
+			}
+
+			for _, tt := range tests {
+				if tt.expected == nil {
+					Expect(ValidateSubjectType(tt.target, tt.relationTypes)).Should(BeNil())
+				} else {
+					Expect(ValidateSubjectType(tt.target, tt.relationTypes)).Should(Equal(tt.expected))
+				}
+			}
+		})
 	})
 })
diff --git a/proto/base/v1/openapi.proto b/proto/base/v1/openapi.proto
@@ -9,7 +9,7 @@ option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_swagger) = {
   info: {
     title: "Permify API";
     description: "Permify is an open-source authorization service for creating and maintaining fine-grained authorizations across your individual applications and services. Permify converts authorization data as relational tuples into a database you point at. We called that database a Write Database (WriteDB) and it behaves as a centralized data source for your authorization system. You can model of your authorization with Permify's DSL - Permify Schema - and perform access checks with a single API call anywhere on your stack. Access decisions made according to stored relational tuples.";
-    version: "v0.3.3";
+    version: "v0.3.4";
     contact: {
       name: "API Support";
       url: "https://github.com/Permify/permify/issues";