test: Unit test to assert role capabilities #1781

Emyrk · 2022-05-26T15:19:32Z

This unit test allows for asserting which roles can perform
actions on various objects. This is much easier than making
unit tests to hit the api.

This unit test allows for asserting which roles can perform actions on various objects. This is much easier than making unit tests to hit the api.

johnstcn

Love me some more tests!

coderd/rbac/builtin_test.go

johnstcn · 2022-05-27T08:05:33Z

coderd/rbac/builtin_test.go

+			Name:     "AUser",
+			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
+			Resource: rbac.ResourceUser,
+			Assertions: map[bool][]authSubject{
+				true:  {admin},
+				false: {member, orgMember, orgAdmin, otherOrgMember, otherOrgAdmin},
+			},


suggestion: Could we rename these fields for clarity? When I initially read the test I got a bit confused, as it appears to me that this test was asserting that any random user resource should have the admin role. Now after reading it, it appears that the Resource in question is the object, and the subjects are all of those identified in assertions.

Correct. I can rename. I changed this struct up a few times, probably why it ended as it did.

Added comments too.

johnstcn · 2022-05-27T08:08:15Z

coderd/rbac/builtin_test.go

+		Resource rbac.Object
+		Actions  []rbac.Action
+		// Assertions must cover all subjects in 'requiredSubjects'
+		Assertions map[bool][]authSubject


nit: map[authSubject]bool? Kind of easier to reason about. Optional though, this is really a nit pick.

I felt it was easier to just list out subjects in a 1 line list.

johnstcn · 2022-05-27T08:09:10Z

coderd/rbac/builtin_test.go

+			},
+		},
+		{
+			Name:     "APIKey",


We should probably clarify that the APIKey is owned by the user referred to by orgMember

It is. I changed member and orgMember to memberMe and orgMemberMe respectively.

- Add comments, rename fields

* test: Unit test to assert role permissions This unit test allows for asserting which roles can perform actions on various objects. This is much easier than making unit tests to hit the api.

Emyrk added 3 commits May 26, 2022 10:18

test: Unit test to assert role permissions

3688b95

This unit test allows for asserting which roles can perform actions on various objects. This is much easier than making unit tests to hit the api.

Import order

9129ef6

Add some comments

67a7fee

Emyrk changed the title ~~test: Unit test to assert role permissions~~ test: Unit test to assert role capabilities May 26, 2022

Emyrk requested review from spikecurtis and johnstcn May 26, 2022 15:23

johnstcn approved these changes May 27, 2022

View reviewed changes

PR cleanup from comments

f60365b

- Add comments, rename fields

Emyrk merged commit ebaae75 into main May 27, 2022

Emyrk deleted the stevenmasley/rbac_more_testing branch May 27, 2022 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: Unit test to assert role capabilities #1781

test: Unit test to assert role capabilities #1781

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

test: Unit test to assert role capabilities #1781

test: Unit test to assert role capabilities #1781

Uh oh!

Conversation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!