Skip to content
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.

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

lookup API should use the correct Silo #850

Closed
davepacheco opened this issue Apr 1, 2022 · 0 comments · Fixed by #937
Closed

lookup API should use the correct Silo #850

davepacheco opened this issue Apr 1, 2022 · 0 comments · Fixed by #937

Comments

@davepacheco
Copy link
Collaborator

davepacheco commented Apr 1, 2022

As part of landing Silos in #814, we hardcoded all resource lookup to the sole Silo that currently exists. This issue covers the follow-up work to use the Silo from the OpContext that's doing the lookup.

#798 made this a bit harder because the code for Organization lookup is generated by the macro. I think the right answer here is to add Silos to the list of resources that the lookup API knows about, but have it be implicit from the OpContext. This is a bit more work than I realized:

  • Silos would need their own explicit authz resource, or else we do generic authz types could be more type-safe #848 first.
  • The definition of Organization in lookup.rs needs to say that Silo is an ancestor -- that's easy.
  • The top level organization_name() function needs to create an implicit Silo node in the path -- that should be easy too once we've done the above.
  • The macro-generated lookup-by-id functions go directly to a resource in the middle of the hierarchy, and we don't know if that's in your Silo! The good news is we already traverse back up to the Organization. We just need to check at that point whether the Silo associated with that Organization matches the one in the OpContext.

(edit: the "unauthorized" test may need to be updated, but I hope not if we land #873 first.)

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 a pull request may close this issue.

1 participant