convert disk, project, and organization lookups to new lookup API#844
convert disk, project, and organization lookups to new lookup API#844davepacheco wants to merge 7 commits into
Conversation
| /// authz checks are required. As a result, it should not be made | ||
| /// accessible outside the DataStore. It should always be wrapped by | ||
| /// something that does the appropriate authz check. | ||
| // TODO-security We should refactor things so that it's harder to |
There was a problem hiding this comment.
\o/ We finally did this!
| .lookup_for(authz::Action::CreateChild) | ||
| .await?; | ||
|
|
||
| // TODO-security This may need to be revisited once we implement authz |
There was a problem hiding this comment.
This TODO is no longer applicable. We did implement authz for saga actions. The pattern is: we do the authz check before creating the saga for a better user experience, and we also serialize the current authn context, rehydrate it inside the saga, and use that to check again when we run the saga action. So this TODO is resolved.
I suspect we're not actually authorizing the operations that this saga is doing, but that'll be caught when I sweep through DataStore looking for unprotected functions (which is tracked by #419).
The authorize check isn't removed -- it got moved into the lookup now.
| .lookup_for(authz::Action::Delete) | ||
| .await?; | ||
|
|
||
| // TODO: We need to sort out the authorization checks. |
|
Thanks for the review! |
|
GitHub actions looks stuck on the Ubuntu and Mac builds after 10+ hours: I will try to push a noop commit to try again. |
|
I'm going to close and open a new PR to try to kick the GitHub Actions. |
Pull request was closed
Crucible changes are: Nexus notifications have different importance (#1621) Propolis changes are: lib: don't send CPUID settings in vCPU device state payload (#848) server/lib: add generic hypervisor enlightenment interface (#846) server: read host CPUID values if the spec has none (#844) Added a new field to the Board struct that propolis requires.
Crucible changes are: Nexus notifications have different importance (#1621) Propolis changes are: lib: don't send CPUID settings in vCPU device state payload (#848) server/lib: add generic hypervisor enlightenment interface (#846) server: read host CPUID values if the spec has none (#844) Added a new field to the Board struct that propolis requires. Co-authored-by: Alan Hanson <alan@oxide.computer>
See #798. This change:
authz::Organization-- expecting the caller to have done the lookup -- rather than aname.)There are other resources not covered here but they're all harder to deal with. See #845.