Skip to content

Commit

Permalink
better comment, link to #3995
Browse files Browse the repository at this point in the history
  • Loading branch information
david-crespo committed Jun 17, 2024
1 parent 2d217ec commit e07a31f
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions nexus/src/app/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,16 @@ impl super::Nexus {
) -> LookupResult<(db::model::IpPool, db::model::IpPoolResource)> {
let (authz_pool, pool) = self
.ip_pool_lookup(opctx, pool)?
// TODO: This is a smell. This works because it is the permission
// for allocating IPs from a pool, which any authenticated user has.
// But what we really want to say is that any authenticated user has
// actual Read permission on any IP pool linked to their silo.
// TODO-robustness: https://github.com/oxidecomputer/omicron/issues/3995
// Checking CreateChild works because it is the permission for
// allocating IPs from a pool, which any authenticated user has.
// But what we really want to say is that any authenticated user
// has actual Read permission on any IP pool linked to their silo.
// Instead we are backing into this with the next line: never fail
// this auth check as long as you're authed, then 404 if unlinked.
// This is not a correctness issue per se because the logic as-is is
// correct. The main problem is that it is fiddly to get right and
// has to be done manually each time.
.fetch_for(authz::Action::CreateChild)
.await?;

Expand Down

0 comments on commit e07a31f

Please sign in to comment.