Skip to content

Commit

Permalink
address various review comments
Browse files Browse the repository at this point in the history
This commit will address various review comments on #77:

- Move TestEntityTagMarshalJSON into types/entity_test.go
- Delete entity_tag_test.go
- Remove "omitempty" JSON tag from Entity.Tag field
- Rename "UnspecifiedEntity" -> "ZeroEntity" in eval/tests
- Rename "nil-entity-map" -> "nil-entity-getter" and explicitly
  set Entities: nil in TestIsAuthorized
- Move a tag authorization test from corpus_test.go
  to authorize_test.go
- Uncomment a test directive which was commented by mistake

Signed-off-by: Ali Dowair <[email protected]>
  • Loading branch information
adowair committed Dec 12, 2024
1 parent 4f433e3 commit 9074d5b
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 67 deletions.
22 changes: 20 additions & 2 deletions authorize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,26 @@ func TestIsAuthorized(t *testing.T) {
DiagErr: 0,
},
{
Name: "nil-entity-map",
Name: "permit-when-tags",
Policy: `permit(principal,action,resource) when { principal.hasTag("foo") };`,
Entities: types.EntityMap{
cuzco: types.Entity{
Tags: types.NewRecord(cedar.RecordMap{
"foo": types.String("bar"),
}),
},
},
Principal: cuzco,
Action: dropTable,
Resource: cedar.NewEntityUID("table", "whatever"),
Context: cedar.Record{},
Want: true,
DiagErr: 0,
},
{
Name: "nil-entity-getter",
Policy: `permit(principal,action,resource);`,
Entities: nil,
Principal: cuzco,
Action: dropTable,
Resource: cedar.NewEntityUID("table", "whatever"),
Expand Down Expand Up @@ -795,7 +813,7 @@ func TestIsAuthorized(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.Name, func(t *testing.T) {
// t.Parallel()
t.Parallel()
ps, err := cedar.NewPolicySetFromBytes("policy.cedar", []byte(tt.Policy))
testutil.Equals(t, err != nil, tt.ParseErr)
ok, diag := ps.IsAuthorized(tt.Entities, cedar.Request{
Expand Down
15 changes: 0 additions & 15 deletions corpus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,21 +251,6 @@ func TestCorpusRelated(t *testing.T) {
reasons []cedar.PolicyID
errors []cedar.PolicyID
}{
{
"a9fe7e4b20024dc7818a168c67ce312d6e076b93",
`forbid(
principal,
action in [Action::"action",Action::"action"],
resource
) when {
true && (resource.hasTag("A"))
};`,
types.EntityMap{cedar.NewEntityUID("a", ""): cedar.Entity{Attributes: cedar.NewRecord(cedar.RecordMap{"A": types.False})}},
cedar.Request{Principal: cedar.NewEntityUID("a", ""), Action: cedar.NewEntityUID("Action", "action"), Resource: cedar.NewEntityUID("a", "'")},
cedar.Deny,
nil,
nil,
},
{
"0cb1ad7042508e708f1999284b634ed0f334bc00",
`forbid(
Expand Down
4 changes: 2 additions & 2 deletions internal/eval/evalers.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,8 +820,8 @@ func (n *getTagEval) Eval(env Env) (types.Value, error) {
return zeroValue(), err
}

var unspecified types.EntityUID
if eid == unspecified {
var zero types.EntityUID
if eid == zero {
return zeroValue(), fmt.Errorf("cannot access tag `%s` of %w", n.rhs, errUnspecifiedEntity)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/eval/evalers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1505,7 +1505,7 @@ func TestGetTagNode(t *testing.T) {
newLiteralEval(knownTag),
zeroValue(),
errEntityNotExist},
{"UnspecifiedEntity",
{"ZeroEntity",
newLiteralEval(types.NewEntityUID("", "")),
newLiteralEval(knownTag),
zeroValue(),
Expand Down
2 changes: 1 addition & 1 deletion types/entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type Entity struct {
UID EntityUID `json:"uid"`
Parents EntityUIDSet `json:"parents"`
Attributes Record `json:"attrs"`
Tags Record `json:"tags,omitempty"`
Tags Record `json:"tags"`
}

// MarshalJSON serializes Entity as a JSON object, using the implicit form of EntityUID encoding to match the Rust
Expand Down
46 changes: 0 additions & 46 deletions types/entity_tag_test.go

This file was deleted.

37 changes: 37 additions & 0 deletions types/entity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types_test

import (
"testing"
"time"

"github.com/cedar-policy/cedar-go/internal/testutil"
"github.com/cedar-policy/cedar-go/types"
Expand Down Expand Up @@ -57,3 +58,39 @@ func TestEntityMarshalJSON(t *testing.T) {
"tags":{}
}`)
}

func TestEntityTagMarshalJSON(t *testing.T) {
t.Parallel()
e := types.Entity{
UID: types.NewEntityUID("FooType", "1"),
Parents: types.NewEntityUIDSet(),
Attributes: types.Record{},
Tags: types.NewRecord(types.RecordMap{
"key": types.String("value"),
"entity": types.NewEntityUID("FootType", "1"),
"datetime": types.NewDatetime(time.Unix(0, 0)),
}),
}

testutil.JSONMarshalsTo(t, e,
`{
"uid": {"type":"FooType","id":"1"},
"parents": [],
"attrs":{},
"tags": {
"datetime": {
"__extn": {
"fn": "datetime",
"arg": "1970-01-01T00:00:00.000Z"
}
},
"entity": {
"__entity": {
"type": "FootType",
"id": "1"
}
},
"key": "value"
}
}`)
}

0 comments on commit 9074d5b

Please sign in to comment.