Skip to content

test write access by unprivileged users to read-only resources#1399

Merged
davepacheco merged 4 commits into
mainfrom
followup-1341
Jul 13, 2022
Merged

test write access by unprivileged users to read-only resources#1399
davepacheco merged 4 commits into
mainfrom
followup-1341

Conversation

@davepacheco

Copy link
Copy Markdown
Collaborator

In #1341 I suggested that the "unauthorized.rs" test could test the behavior of unprivileged users attempting to POST/PUT/DELETE things they're only supposed to be able to read. I'm trying that here.

@davepacheco

Copy link
Copy Markdown
Collaborator Author

On commit cbf1b8d, here's the output of the unauthorized test:

SUMMARY OF REQUESTS MADE

KEY, USING HEADER AND EXAMPLE ROW:

          +----------------------------> privileged GET (expects 200 or 500)
          |                              (digit = last digit of status code)
          |
          |                          +-> privileged GET (expects same as above)
          |                          |   (digit = last digit of status code)
          |                          |   ('-' => skipped (N/A))
          ^                          ^
HEADER:   G GET  PUT  POST DEL  TRCE G  URL
EXAMPLE:  0 3111 5555 3111 5555 5555 0  /organizations
    ROW     ^^^^
            ||||                      TEST CASES FOR EACH HTTP METHOD:
            +|||----------------------< authenticated, unauthorized request
             +||----------------------< unauthenticated request
              +|----------------------< bad authentication: no such user
               +----------------------< bad authentication: invalid syntax

            \__/ \__/ \__/ \__/ \__/
            GET  PUT  etc.  The test cases are repeated for each HTTP method.

            The number in each cell is the last digit of the 400-level response
            that was expected for this test case.

    In this case, an unauthenticated request to "GET /organizations" returned
    401.  All requests to "PUT /organizations" returned 405.

G GET  PUT  POST DEL  TRCE G  URL
0 3111 3111 5555 5555 5555 0  /policy
0 3111 5555 3111 5555 5555 0  /ip-pools
0 4111 4111 5555 4111 5555 0  /ip-pools/pool0
0 4111 5555 5555 5555 5555 0  /ip-pools/pool0/ranges
- 5555 5555 4111 5555 5555 -  /ip-pools/pool0/ranges/add
- 5555 5555 4111 5555 5555 -  /ip-pools/pool0/ranges/remove
0 3111 5555 3111 5555 5555 0  /silos
0 4111 5555 5555 4111 5555 0  /silos/demo-silo
0 4111 4111 5555 5555 5555 0  /silos/demo-silo/policy
0 -111 5555 5555 5555 5555 0  /users
0 3111 5555 3111 5555 5555 0  /organizations
0 4111 5555 5555 5555 5555 0  /by-id/organizations/{id}
0 4111 4111 5555 4111 5555 0  /organizations/demo-org
0 4111 4111 5555 5555 5555 0  /organizations/demo-org/policy
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects
0 4111 5555 5555 5555 5555 0  /by-id/projects/{id}
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project
0 4111 4111 5555 5555 5555 0  /organizations/demo-org/projects/demo-project/policy
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs
0 4111 5555 5555 5555 5555 0  /by-id/vpcs/{id}
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc
0 4111 4111 5555 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/firewall/rules
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/subnets
0 4111 5555 5555 5555 5555 0  /by-id/vpc-subnets/{id}
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/subnets/demo-vpc-subnet
0 4111 5555 5555 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/subnets/demo-vpc-subnet/network-interfaces
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers
0 4111 5555 5555 5555 5555 0  /by-id/vpc-routers/{id}
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers/demo-vpc-router
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers/demo-vpc-router/routes
0 4111 5555 5555 5555 5555 0  /by-id/vpc-router-routes/{id}
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/vpcs/demo-vpc/routers/demo-vpc-router/routes/demo-router-route
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/disks
0 4111 5555 5555 5555 5555 0  /by-id/disks/{id}
0 4111 5555 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/disks/demo-disk
0 4111 5555 5555 5555 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance/disks
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/disks/attach
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/disks/detach
0 4111 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/images
0 4111 5555 5555 5555 5555 -  /by-id/images/{id}
0 4111 5555 5555 4111 5555 -  /organizations/demo-org/projects/demo-project/images/demo-image
0 4111 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/snapshots
0 4111 5555 5555 5555 5555 -  /by-id/snapshots/{id}
0 4111 5555 5555 4111 5555 -  /organizations/demo-org/projects/demo-project/snapshots/demo-snapshot
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/instances
0 4111 5555 5555 5555 5555 0  /by-id/instances/{id}
0 4111 5555 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/start
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/stop
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/reboot
- 5555 5555 4111 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/migrate
- 4111 5555 5555 5555 5555 -  /organizations/demo-org/projects/demo-project/instances/demo-instance/serial-console
0 4111 5555 4111 5555 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance/network-interfaces
0 4111 5555 5555 5555 5555 0  /by-id/network-interfaces/{id}
0 4111 4111 5555 4111 5555 0  /organizations/demo-org/projects/demo-project/instances/demo-instance/network-interfaces/net0
0 3111 5555 5555 5555 5555 0  /roles
0 4111 5555 5555 5555 5555 0  /roles/fleet.admin
0 3111 5555 5555 5555 5555 0  /system/user
0 4111 5555 5555 5555 5555 0  /system/user/db-init
0 3111 5555 5555 5555 5555 0  /hardware/racks
0 4111 5555 5555 5555 5555 0  /hardware/racks/c19a698f-c6f9-4a17-ae30-20d711b8f7dc
0 3111 5555 5555 5555 5555 0  /hardware/sleds
0 4111 5555 5555 5555 5555 0  /hardware/sleds/b6d65341-167c-41df-9b5c-41cded99c229
0 3111 5555 5555 5555 5555 0  /sagas
- 3111 5555 5555 5555 5555 -  /sagas/48a1b8c8-fc1c-6fea-9de9-fdeb8dda7823
0 3111 5555 5555 5555 5555 0  /timeseries/schema
- 5555 5555 3111 5555 5555 -  /updates/refresh
0 -111 5555 3111 5555 5555 0  /images
0 -111 5555 5555 5555 5555 0  /by-id/global-images/{id}
0 -111 5555 5555 4111 5555 0  /images/alpine-edge
0 -111 5555 5555 5555 5555 0  /silos/default-silo/identity_providers
- 5555 5555 3111 5555 5555 -  /silos/default-silo/saml_identity_providers
0 4111 5555 5555 5555 5555 0  /silos/default-silo/saml_identity_providers/demo-saml-provider
0 -111 5555 5555 5555 5555 0  /session/me
0 -111 -555 -111 -555 -555 0  /session/me/sshkeys
0 -111 -555 -555 -111 -555 0  /session/me/sshkeys/aaaaa-ssh-key

Note that we're skipping testing the authenticated, unauthorized user for all methods for endpoints like /session/me/sshkeys. We're doing this test for POST /images, but we're skipping it for GET /images (since the user can access that). I think that should probably be a 403 and not a 404, but that's not changed by this test.

@davepacheco davepacheco changed the title test read-only access by unprivileged users test write access by unprivileged users to read-only resources Jul 13, 2022
#
# It's unclear what else would break if users couldn't see their own Silo.
has_permission(actor: AuthenticatedActor, "read", silo: Silo)
# TODO actor.silo is *not* a list, so `in` is incorrect here, but if you

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this comment and the one at L171 because this behavior is documented:
https://docs.osohq.com/rust/reference/polar/classes.html#options

What wasn't obvious to me from the documentation was whether x in y returns true if y was any Some value or only if it was Some(x). I've tested it, and it's only if y is Some(x) (which of course makes much more sense).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, was not aware of that language feature. Makes sense, though still a bit non-obvious (to me, at least).

@davepacheco davepacheco requested a review from plotnick July 13, 2022 00:20
@davepacheco davepacheco marked this pull request as ready for review July 13, 2022 00:21

@plotnick plotnick left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks as always for helping to ensure that authorization is working correctly and as intended.

@davepacheco davepacheco merged commit ede7ad2 into main Jul 13, 2022
@davepacheco davepacheco deleted the followup-1341 branch July 13, 2022 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants