Skip to content

authz: make it easier to test authn/authz protection for new endpoints #652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 31, 2022
Merged

authz: make it easier to test authn/authz protection for new endpoints #652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4086d58
authz: make it easier to test authn/authz protection for new endpoints
davepacheco Jan 28, 2022
2c28623
rip out now-redundant tests
davepacheco Jan 29, 2022
3eec4e3
clean up and document new test
davepacheco Jan 29, 2022
af6b8d7
self-review: clean up
davepacheco Jan 31, 2022
13e0c74
Merge branch 'main' into authz-tests
davepacheco Jan 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 0 additions & 102 deletions nexus/tests/integration_tests/authz.rs
View file
Open in desktop

This file was deleted.

109 changes: 2 additions & 107 deletions nexus/tests/integration_tests/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,27 +177,6 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
create_organization(&client, &org_name).await;
let projects_url = "/organizations/test-org/projects";

/* Unauthenticated and unauthorized users cannot list projects. */
NexusRequest::expect_failure(
client,
http::StatusCode::NOT_FOUND,
http::Method::GET,
projects_url,
)
.execute()
.await
.expect("failed to make request");
NexusRequest::expect_failure(
client,
http::StatusCode::NOT_FOUND,
http::Method::GET,
projects_url,
)
.authn_as(AuthnMode::UnprivilegedUser)
.execute()
.await
.expect("failed to make request");

/*
* Verify that there are no projects to begin with.
*/
Expand Down Expand Up @@ -240,30 +219,6 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
project_ids
};

/*
* Unauthenticated and unauthorized users cannot fetch the Project.
*/
let simproject1_url = "/organizations/test-org/projects/simproject1";
NexusRequest::expect_failure(
client,
http::StatusCode::NOT_FOUND,
http::Method::GET,
simproject1_url,
)
.execute()
.await
.expect("failed to make request");
NexusRequest::expect_failure(
client,
http::StatusCode::NOT_FOUND,
http::Method::GET,
simproject1_url,
)
.authn_as(AuthnMode::UnprivilegedUser)
.execute()
.await
.expect("failed to make request");

/*
* Error case: GET /organizations/test-org/projects/simproject1/nonexistent
* (a path that does not exist beneath a resource that does exist)
Expand Down Expand Up @@ -384,41 +339,6 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
expected_projects[1].identity.description
);

/*
* Unprivileged users should not be able to update a Project.
*/
let project_update = params::ProjectUpdate {
identity: IdentityMetadataUpdateParams {
name: None,
description: None,
},
};
NexusRequest::new(
RequestBuilder::new(
client,
Method::PUT,
"/organizations/test-org/projects/simproject3",
)
.body(Some(&project_update))
.expect_status(Some(StatusCode::NOT_FOUND)),
)
.execute()
.await
.expect("failed to make request");
NexusRequest::new(
RequestBuilder::new(
client,
Method::PUT,
"/organizations/test-org/projects/simproject3",
)
.body(Some(&project_update))
.expect_status(Some(StatusCode::NOT_FOUND)),
)
.authn_as(AuthnMode::UnprivilegedUser)
.execute()
.await
.expect("failed to make request");

/*
* Update "simproject3". We'll make sure that's reflected in the other
* requests.
Expand Down Expand Up @@ -511,6 +431,8 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
.unwrap();
assert_eq!("already exists: project \"simproject1\"", error.message);

// TODO-coverage try to rename it to a name that conflicts

/*
* Try to create a project with an unsupported name.
* TODO-polish why doesn't serde include the field name in this error?
Expand Down Expand Up @@ -559,33 +481,6 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
assert_eq!(project.identity.name, "honor-roller");
assert_eq!(project.identity.description, "a soapbox racer");

/*
* Attempt to create a project without authenticating or without privileges.
* TODO-security TODO-correctness One thing that's a little strange here: we
* currently return a 404 if you attempt to create a Project inside an
* Organization and you're not authorized to do that. In an ideal world,
* we'd return a 403 if you can _see_ the Organization and a 404 if not.
* But we don't really know if you should be able to see the Organization.
* Right now, the only real way to tell that is if you have permissions on
* anything _inside_ the Organization, which is incredibly expensive to
* determine in general.
*/
RequestBuilder::new(client, Method::POST, &projects_url)
.expect_status(Some(StatusCode::NOT_FOUND))
.body(Some(&project_create))
.execute()
.await
.expect("expected request to fail");
NexusRequest::new(
RequestBuilder::new(client, Method::POST, &projects_url)
.body(Some(&project_create))
.expect_status(Some(StatusCode::NOT_FOUND)),
)
.authn_as(AuthnMode::UnprivilegedUser)
.execute()
.await
.expect("expected request to fail");

/*
* List projects again and verify all of our changes. We should have:
*
Expand Down
2 changes: 1 addition & 1 deletion nexus/tests/integration_tests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
//! the way it is.

mod authn_http;
mod authz;
mod basic;
mod commands;
mod console_api;
Expand All @@ -18,6 +17,7 @@ mod roles_builtin;
mod router_routes;
mod subnet_allocation;
mod timeseries;
mod unauthorized;
mod users_builtin;
mod vpc_firewall;
mod vpc_routers;
Expand Down
46 changes: 0 additions & 46 deletions nexus/tests/integration_tests/organizations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,6 @@ async fn test_organizations(cptestctx: &ControlPlaneTestContext) {
.unwrap();
assert_eq!(organization.identity.name, o1_name);

// You should get a 404 if not authenticated.
NexusRequest::expect_failure(
&client,
StatusCode::NOT_FOUND,
Method::GET,
&o1_url,
)
.execute()
.await
.expect("failed to make request");

// Same if you're authenticated but not authorized to see it.
NexusRequest::expect_failure(
&client,
StatusCode::NOT_FOUND,
Method::GET,
&o1_url,
)
.authn_as(AuthnMode::UnprivilegedUser)
.execute()
.await
.expect("failed to make request");

let o2_url = format!("/organizations/{}", o2_name);
let organization: Organization = NexusRequest::object_get(&client, &o2_url)
.authn_as(AuthnMode::PrivilegedUser)
Expand Down Expand Up @@ -86,29 +63,6 @@ async fn test_organizations(cptestctx: &ControlPlaneTestContext) {
assert_eq!(organizations[0].identity.name, o2_name);
assert_eq!(organizations[1].identity.name, o1_name);

// You should get a 404 if you attempt to delete an organization if you are
// unauthenticated or unauthorized.
NexusRequest::expect_failure(
&client,
StatusCode::NOT_FOUND,
Method::DELETE,
&o1_url,
)
.execute()
.await
.expect("failed to make request");

NexusRequest::expect_failure(
&client,
StatusCode::NOT_FOUND,
Method::DELETE,
&o1_url,
)
.authn_as(AuthnMode::UnprivilegedUser)
.execute()
.await
.expect("failed to make request");

// Verify DELETE /organization/{org} works
let o1_old_id = organizations[1].identity.id;
NexusRequest::object_delete(&client, &o1_url)
Expand Down
Loading