Skip to content

Commit ea3a918

Browse files
committed
types: make EntityUID UnmarshalJSON more strict and add tests
Signed-Off-By: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
1 parent 770841a commit ea3a918

15 files changed

+161
-2
lines changed

types/entity_uid.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ func (e *EntityUID) UnmarshalCedar(data []byte) error {
8282
}
8383

8484
func (e *EntityUID) UnmarshalJSON(b []byte) error {
85-
// XXX: We might have to do something here?? For one thing, no extra keys should be allowed.
86-
// TODO: review after adding support for schemas
8785
var res entityValueJSON
8886
if err := json.Unmarshal(b, &res); err != nil {
8987
return err

types/entity_uid_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,53 @@ func TestEntity(t *testing.T) {
119119
})
120120
}
121121
})
122+
123+
t.Run("UnmarshalJSON", func(t *testing.T) {
124+
t.Parallel()
125+
tests := []struct {
126+
name string
127+
input string
128+
}{
129+
{"explicit", `{ "__entity": { "type": "Type", "id": "id" } }`},
130+
{"implicit", `{ "type": "Type", "id": "id" }`},
131+
{"explicit - extra keys in inner", `{ "__entity": { "type": "Type", "id": "id", "floob": "blah" } }`},
132+
{"implicit - extra keys", `{ "type": "Type", "id": "id", "floob": "blah" }`},
133+
}
134+
135+
for _, tt := range tests {
136+
t.Run(tt.name, func(t *testing.T) {
137+
t.Parallel()
138+
var got types.EntityUID
139+
err := got.UnmarshalJSON([]byte(tt.input))
140+
testutil.OK(t, err)
141+
testutil.Equals(t, got, types.NewEntityUID("Type", "id"))
142+
})
143+
}
144+
})
145+
146+
t.Run("UnmarshalJSON invalid", func(t *testing.T) {
147+
t.Parallel()
148+
tests := []struct {
149+
name string
150+
input string
151+
}{
152+
{"non-object", `"floob"`},
153+
{"explicit - wrong type", `{ "__entity": "wrong type"`},
154+
{"implicit - type wrong", `{ "type": 123, "id": "123" }`},
155+
{"implicit - id wrong type", `{ "type": "Type", "id": 123 }`},
156+
{"implicit - only type", `{ "type": "Type" }`},
157+
{"implicit - only id", `{ "id": 123 }`},
158+
}
159+
160+
for _, tt := range tests {
161+
t.Run(tt.name, func(t *testing.T) {
162+
t.Parallel()
163+
var got types.EntityUID
164+
err := got.UnmarshalJSON([]byte(tt.input))
165+
testutil.Error(t, err)
166+
})
167+
}
168+
})
122169
}
123170

124171
func TestEntityUIDSet(t *testing.T) {

types/json_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func TestJSON_Value(t *testing.T) {
3030
}{
3131
{"entity-likeRecord", `{ "type": "User", "id": "alice" }`, NewRecord(RecordMap{"type": String("User"), "id": String("alice")}), nil},
3232
{"explicitEntity", `{ "__entity": { "type": "User", "id": "alice" } }`, EntityUID{Type: "User", ID: "alice"}, nil},
33+
{"explicitEntityExtraField", `{ "__entity": { "type": "User", "id": "alice" }, "extra": "stuff" }`, EntityUID{Type: "User", ID: "alice"}, nil},
3334
{"explicitLongEntity", `{ "__entity": { "type": "User::External", "id": "alice" } }`, EntityUID{Type: "User::External", ID: "alice"}, nil},
3435
{"invalidJSON", `!@#$`, zeroValue(), errJSONDecode},
3536
{"numericOverflow", "12341234123412341234", zeroValue(), errJSONLongOutOfRange},
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
entity Manager;
2+
3+
entity User = {
4+
manager: Manager,
5+
};
6+
7+
action view appliesTo {
8+
principal: [User],
9+
resource: [User],
10+
};
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
[
2+
{
3+
"uid": {"type": "Manager", "id": "boss"},
4+
"attrs": {},
5+
"parents": []
6+
},
7+
{
8+
"uid": {"type": "User", "id": "alice"},
9+
"attrs": {
10+
"manager": {"__entity": {"type": "Manager", "id": "boss"}, "extra": "stuff"}
11+
},
12+
"parents": []
13+
}
14+
]
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"schema": "explicit-entity-extra-field.cedarschema",
3+
"entities": "explicit-entity-extra-field.entities.json"
4+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"allEntities": {
3+
"success": true
4+
},
5+
"perEntity": {
6+
"Manager::boss": {
7+
"success": true
8+
},
9+
"User::alice": {
10+
"success": true
11+
}
12+
}
13+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
entity Manager;
2+
3+
entity User = {
4+
manager: Manager,
5+
};
6+
7+
action view appliesTo {
8+
principal: [User],
9+
resource: [User],
10+
};
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
[
2+
{
3+
"uid": {"type": "Manager", "id": "boss"},
4+
"attrs": {},
5+
"parents": []
6+
},
7+
{
8+
"uid": {"type": "User", "id": "alice"},
9+
"attrs": {
10+
"manager": {"type": "Manager", "id": "boss", "extra": "stuff"}
11+
},
12+
"parents": []
13+
}
14+
]
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"schema": "implicit-entity-extra-field.cedarschema",
3+
"entities": "implicit-entity-extra-field.entities.json"
4+
}

0 commit comments

Comments
 (0)