Skip to content

Commit

Permalink
Fix relation validation (#118)
Browse files Browse the repository at this point in the history
* add test checking relationships of different types

Signed-off-by: Mike Mason <[email protected]>

* correct relationship check direction

Previously we were checking if the destination resource had a relation
rather than the targeted resource having a relation to the destination.

This resulted in the following not working:

```yaml
resourcetypes:
  - name: folder
    relationships:
      - relation: parent
        targettypenames:
          - folder
  - name: file
    relationships:
      - relation: parent
        targettypenames:
          - folder
```

When creating a relationship previously, relating a folder to a parent
folder would work since they happen to have the same resource type.
However relating a file to a parent folder would fail due to us checking
if folder has a parent relationship type name for file.

Signed-off-by: Mike Mason <[email protected]>

---------

Signed-off-by: Mike Mason <[email protected]>
  • Loading branch information
mikemrm authored Jun 20, 2023
1 parent 91b97b0 commit f8be5d1
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 23 deletions.
11 changes: 8 additions & 3 deletions internal/iapl/default.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package iapl

// DefaultPolicy generates the default policy for permissions-api.
func DefaultPolicy() Policy {
policyDocument := PolicyDocument{
// DefaultPolicyDocument returns the default policy document for permissions-api.
func DefaultPolicyDocument() PolicyDocument {
return PolicyDocument{
ResourceTypes: []ResourceType{
{
Name: "role",
Expand Down Expand Up @@ -204,6 +204,11 @@ func DefaultPolicy() Policy {
},
},
}
}

// DefaultPolicy generates the default policy for permissions-api.
func DefaultPolicy() Policy {
policyDocument := DefaultPolicyDocument()

policy := NewPolicy(policyDocument)
if err := policy.Validate(); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/query/relations.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ func (e *engine) validateRelationship(rel types.Relationship) error {
return err
}

for _, typeRel := range subjType.Relationships {
for _, typeRel := range resType.Relationships {
// If we find a relation with a name and type that matches our relationship,
// return
if rel.Relation == typeRel.Relation {
for _, typeName := range typeRel.Types {
if resType.Name == typeName {
if subjType.Name == typeName {
return nil
}
}
Expand Down
88 changes: 70 additions & 18 deletions internal/query/relations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.infratographer.com/permissions-api/internal/iapl"
"go.infratographer.com/permissions-api/internal/spicedbx"
"go.infratographer.com/permissions-api/internal/testingx"
"go.infratographer.com/permissions-api/internal/types"
Expand All @@ -25,19 +26,50 @@ func testEngine(ctx context.Context, t *testing.T, namespace string) Engine {
client, err := spicedbx.NewClient(config, false)
require.NoError(t, err)

request := &pb.WriteSchemaRequest{Schema: spicedbx.GeneratedSchema(namespace)}
policy := testPolicy()

schema, err := spicedbx.GenerateSchema(namespace, policy.Schema())
require.NoError(t, err)

request := &pb.WriteSchemaRequest{Schema: schema}
_, err = client.WriteSchema(ctx, request)
require.NoError(t, err)

t.Cleanup(func() {
cleanDB(ctx, t, client, namespace)
})

out := NewEngine(namespace, client)
out := NewEngine(namespace, client, WithPolicy(policy))

return out
}

func testPolicy() iapl.Policy {
policyDocument := iapl.DefaultPolicyDocument()

policyDocument.ResourceTypes = append(policyDocument.ResourceTypes,
iapl.ResourceType{
Name: "child",
IDPrefix: "chldten",
Relationships: []iapl.Relationship{
{
Relation: "parent",
TargetTypeNames: []string{
"tenant",
},
},
},
},
)

policy := iapl.NewPolicy(policyDocument)
if err := policy.Validate(); err != nil {
panic(err)
}

return policy
}

func cleanDB(ctx context.Context, t *testing.T, client *authzed.Client, namespace string) {
for _, dbType := range []string{"user", "client", "role", "tenant"} {
namespacedType := namespace + "/" + dbType
Expand Down Expand Up @@ -178,29 +210,29 @@ func TestRelationships(t *testing.T) {
require.NoError(t, err)
childRes, err := e.NewResourceFromID(childID)
require.NoError(t, err)
child2ID, err := gidx.NewID("chldten")
require.NoError(t, err)
child2Res, err := e.NewResourceFromID(child2ID)
require.NoError(t, err)

testCases := []testingx.TestCase[[]types.Relationship, []types.Relationship]{
testCases := []testingx.TestCase[types.Relationship, []types.Relationship]{
{
Name: "InvalidRelationship",
Input: []types.Relationship{
{
Resource: childRes,
Relation: "foo",
Subject: parentRes,
},
Input: types.Relationship{
Resource: childRes,
Relation: "foo",
Subject: parentRes,
},
CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Relationship]) {
assert.ErrorIs(t, res.Err, errorInvalidRelationship)
},
},
{
Name: "Success",
Input: []types.Relationship{
{
Resource: childRes,
Relation: "parent",
Subject: parentRes,
},
Input: types.Relationship{
Resource: childRes,
Relation: "parent",
Subject: parentRes,
},
CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Relationship]) {
expRels := []types.Relationship{
Expand All @@ -211,21 +243,41 @@ func TestRelationships(t *testing.T) {
},
}

require.NoError(t, res.Err)
assert.Equal(t, expRels, res.Success)
},
},
{
Name: "Different Success",
Input: types.Relationship{
Resource: child2Res,
Relation: "parent",
Subject: parentRes,
},
CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Relationship]) {
expRels := []types.Relationship{
{
Resource: child2Res,
Relation: "parent",
Subject: parentRes,
},
}

require.NoError(t, res.Err)
assert.Equal(t, expRels, res.Success)
},
},
}

testFn := func(ctx context.Context, input []types.Relationship) testingx.TestResult[[]types.Relationship] {
queryToken, err := e.CreateRelationships(ctx, input)
testFn := func(ctx context.Context, input types.Relationship) testingx.TestResult[[]types.Relationship] {
queryToken, err := e.CreateRelationships(ctx, []types.Relationship{input})
if err != nil {
return testingx.TestResult[[]types.Relationship]{
Err: err,
}
}

rels, err := e.ListRelationships(ctx, childRes, queryToken)
rels, err := e.ListRelationships(ctx, input.Resource, queryToken)

return testingx.TestResult[[]types.Relationship]{
Success: rels,
Expand Down

0 comments on commit f8be5d1

Please sign in to comment.