From 6f78067e05ec4085570ec72c3072571eb314c106 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Dec 2021 16:49:05 -0500 Subject: [PATCH 1/2] Ensure wildcard subject object IDs are not used with non-empty relations --- .../api/v1/00_handwritten_validation.go | 30 +++++++++++++++++++ .../api/validation_test/tuples_test.go | 23 ++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/proto/authzed/api/v1/00_handwritten_validation.go b/proto/authzed/api/v1/00_handwritten_validation.go index 73429fc..211a53b 100644 --- a/proto/authzed/api/v1/00_handwritten_validation.go +++ b/proto/authzed/api/v1/00_handwritten_validation.go @@ -9,6 +9,9 @@ func (m *CheckPermissionRequest) HandwrittenValidate() error { reason: "alphanumeric value is required", } } + if m.GetSubject() != nil { + return m.GetSubject().HandwrittenValidate() + } return nil } @@ -39,6 +42,19 @@ func (m *RelationshipFilter) HandwrittenValidate() error { reason: "alphanumeric value is required", } } + if m.GetOptionalSubjectFilter() != nil { + return m.GetOptionalSubjectFilter().HandwrittenValidate() + } + return nil +} + +func (m *SubjectFilter) HandwrittenValidate() error { + if m.GetOptionalSubjectId() == "*" && m.GetOptionalRelation() != nil && m.GetOptionalRelation().GetRelation() != "" { + return SubjectFilterValidationError{ + field: "OptionalRelation", + reason: "optionalrelation must be empty on subject if object ID is a wildcard", + } + } return nil } @@ -49,6 +65,16 @@ func (m *RelationshipUpdate) HandwrittenValidate() error { return nil } +func (m *SubjectReference) HandwrittenValidate() error { + if m.GetObject() != nil && m.GetObject().GetObjectId() == "*" && m.GetOptionalRelation() != "" { + return SubjectReferenceValidationError{ + field: "OptionalRelation", + reason: "optionalrelation must be empty on subject if object ID is a wildcard", + } + } + return nil +} + func (m *Relationship) HandwrittenValidate() error { if m.GetResource() != nil && m.GetResource().GetObjectId() == "*" { return ObjectReferenceValidationError{ @@ -57,6 +83,10 @@ func (m *Relationship) HandwrittenValidate() error { } } + if m.GetSubject() != nil { + return m.GetSubject().HandwrittenValidate() + } + return nil } diff --git a/proto/authzed/api/validation_test/tuples_test.go b/proto/authzed/api/validation_test/tuples_test.go index 5b49dce..295d8bf 100644 --- a/proto/authzed/api/validation_test/tuples_test.go +++ b/proto/authzed/api/validation_test/tuples_test.go @@ -389,3 +389,26 @@ func TestV1CoreObjectValidity(t *testing.T) { } } } + +func TestWildcardSubjectRelation(t *testing.T) { + subjObjRef := &v1.ObjectReference{ + ObjectType: "somenamespace", + ObjectId: "*", + } + subRef := &v1.SubjectReference{ + Object: subjObjRef, + OptionalRelation: "somerelation", + } + require.Error(t, subRef.HandwrittenValidate()) +} + +func TestWildcardSubjectRelationEmpty(t *testing.T) { + subjObjRef := &v1.ObjectReference{ + ObjectType: "somenamespace", + ObjectId: "*", + } + subRef := &v1.SubjectReference{ + Object: subjObjRef, + } + require.NoError(t, subRef.HandwrittenValidate()) +} From 3dedc2afc4215575d0b1d6abbb2778c7ad1edcaa Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Dec 2021 17:00:14 -0500 Subject: [PATCH 2/2] Add nil check on root message to each handwritten validation --- .../api/v1/00_handwritten_validation.go | 62 +++++++++++-------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/proto/authzed/api/v1/00_handwritten_validation.go b/proto/authzed/api/v1/00_handwritten_validation.go index 211a53b..720e601 100644 --- a/proto/authzed/api/v1/00_handwritten_validation.go +++ b/proto/authzed/api/v1/00_handwritten_validation.go @@ -3,20 +3,25 @@ package v1 func (m *CheckPermissionRequest) HandwrittenValidate() error { + if m == nil { + return nil + } + if m.GetResource() != nil && m.GetResource().GetObjectId() == "*" { return ObjectReferenceValidationError{ field: "ObjectId", reason: "alphanumeric value is required", } } - if m.GetSubject() != nil { - return m.GetSubject().HandwrittenValidate() - } - return nil + return m.GetSubject().HandwrittenValidate() } func (m *ExpandPermissionTreeRequest) HandwrittenValidate() error { + if m == nil { + return nil + } + if m.GetResource() != nil && m.GetResource().GetObjectId() == "*" { return ObjectReferenceValidationError{ field: "ObjectId", @@ -28,27 +33,33 @@ func (m *ExpandPermissionTreeRequest) HandwrittenValidate() error { } func (m *Precondition) HandwrittenValidate() error { - if m.GetFilter() != nil { - return m.GetFilter().HandwrittenValidate() + if m == nil { + return nil } - return nil + return m.GetFilter().HandwrittenValidate() } func (m *RelationshipFilter) HandwrittenValidate() error { + if m == nil { + return nil + } + if m.GetOptionalResourceId() == "*" { return RelationshipFilterValidationError{ field: "OptionalResourceId", reason: "alphanumeric value is required", } } - if m.GetOptionalSubjectFilter() != nil { - return m.GetOptionalSubjectFilter().HandwrittenValidate() - } - return nil + + return m.GetOptionalSubjectFilter().HandwrittenValidate() } func (m *SubjectFilter) HandwrittenValidate() error { + if m == nil { + return nil + } + if m.GetOptionalSubjectId() == "*" && m.GetOptionalRelation() != nil && m.GetOptionalRelation().GetRelation() != "" { return SubjectFilterValidationError{ field: "OptionalRelation", @@ -59,10 +70,7 @@ func (m *SubjectFilter) HandwrittenValidate() error { } func (m *RelationshipUpdate) HandwrittenValidate() error { - if m.GetRelationship() != nil { - return m.GetRelationship().HandwrittenValidate() - } - return nil + return m.GetRelationship().HandwrittenValidate() } func (m *SubjectReference) HandwrittenValidate() error { @@ -76,6 +84,10 @@ func (m *SubjectReference) HandwrittenValidate() error { } func (m *Relationship) HandwrittenValidate() error { + if m == nil { + return nil + } + if m.GetResource() != nil && m.GetResource().GetObjectId() == "*" { return ObjectReferenceValidationError{ field: "ObjectId", @@ -83,14 +95,14 @@ func (m *Relationship) HandwrittenValidate() error { } } - if m.GetSubject() != nil { - return m.GetSubject().HandwrittenValidate() - } - - return nil + return m.GetSubject().HandwrittenValidate() } func (m *DeleteRelationshipsRequest) HandwrittenValidate() error { + if m == nil { + return nil + } + if m.GetOptionalPreconditions() != nil { for _, precondition := range m.GetOptionalPreconditions() { err := precondition.HandwrittenValidate() @@ -100,14 +112,14 @@ func (m *DeleteRelationshipsRequest) HandwrittenValidate() error { } } - if m.GetRelationshipFilter() != nil { - return m.GetRelationshipFilter().HandwrittenValidate() - } - - return nil + return m.GetRelationshipFilter().HandwrittenValidate() } func (m *WriteRelationshipsRequest) HandwrittenValidate() error { + if m == nil { + return nil + } + if m.GetOptionalPreconditions() != nil { for _, precondition := range m.GetOptionalPreconditions() { err := precondition.HandwrittenValidate()