Description
Goal
Robust authorization testing to prevent security holes created by developer error. Reduce chance developer forgets to call Authorize
, uses wrong rbac object/action, or a role is granted more privilege than it should have.
Current State
RBAC testing is broken up into 2 parts to achieve comprehensive tests across all the api routes.
- Assert various roles against given action+object combinations (link)
- ✔️
owner, orgMemberMe, orgAdmin, templateAdmin
- ❌
memberMe, otherOrgAdmin, otherOrgMember, userAdmin
- Assert a route checks a given action+object pair (link)
This makes testing a "route" with various user role combinations much simpler. Writing a unit test to check various roles against an endpoint is tedious and long. The above two unit tests give a high degree of confidence the correct rbac assertion will be made on the given route, and we know the user role combinations will return a true
or false
to that assertion.
Improve unit tests
Assert various roles
The current set of users and objects were created ad hoc. We should make sure all combinations of expected objects + actions that are hit in prod are covered.
Assert route checks
The current route check assertions are a bit weak.
Only asserts 1 rbac check per route
Firstly, only the last rbac assertion is checked. Some routes run 2 rbac checks (assuming the first one passed) (example). We need to have a way to assert all the checks that are run. Possibly requiring some design to allow the endpoint to run until the second Authorize
check (as all the auth calls return false atm).
Idea (From @spikecurtis): If we have a route that calls authorize
N times, we should run the assertion test on the route N times. Each call should have authorize
fail on the Nth call.
Example: Route postWorkspacesByOrganization
calls Authorize
on create.workspace
and read.template
.
- Test will run 2 times on this route.
- 1st test runs and fails on
create.workspace
. Assert checks run assuming only 1 authorize call - 2nd test runs and succeeds on
create.workspace
. Route continues and fails onread.template
. Assert checks run assuming 2 authorize calls.
This is to ensure the route always fails as Unauthorized
if any of the individual authorize calls returns false.
Asserts weakly
The object assertions do not check any fields not set. So for the workspace endpoint we do not check the Owner
, OrgID
, ACLUserList
or ACLGroupList
fields of the object. We only check the Type
. We should assert exact matches.
Does not assert the caller's roles
We should also asser the roles of the user calling in this test. A non-admin user should be used. This would catch things where the wrong user is being asserted as the actor in cases where more than 1 user is involved. (eg creating a workspace for another user)