store role assignments in the database #520
Conversation
nexus/src/authz/api_resources.rs
Outdated
| } | ||
|
|
||
| fn parent(&self) -> Option<Box<dyn AuthzResource>> { | ||
| Some(Box::new(Fleet {})) |
There was a problem hiding this comment.
Same thing here. Any equality check would of course still pass, but that won't be true if we change the Fleet type to include state. It might also be good documentation to identify this as FLEET. Again, assuming that's actually what we want, I'm still getting my bearings in this code.
There was a problem hiding this comment.
Fixed in 00ac778.
| //! "quidditch". It looks like this: | ||
| //! | ||
| //! ```text | ||
| //! +---> table: "project" |
There was a problem hiding this comment.
Nice diagram. Did you use a tool to generate this?
There was a problem hiding this comment.
Thanks! Nope! Maybe should have.
| authenticated: bool, | ||
| actor_id: Option<Uuid>, | ||
| roles: RoleSet, | ||
| } |
There was a problem hiding this comment.
Why is this called AnyActor instead of Actor?
There was a problem hiding this comment.
Just to better distinguish it from AuthenticatedActor.
(That predates this change -- this code just moved.)
| if let Some(parent) = self.parent() { | ||
| parent | ||
| .fetch_all_related_roles_for_user( | ||
| opctx, datastore, authn, roleset, |
There was a problem hiding this comment.
Doesn't need to be addressed in this PR, but since we have the whole chain of (ID, resource type) in memory here, we should be able to pull them out into a list and have role_asgn_builtin_list_for query for them all at once in a single query.
There was a problem hiding this comment.
Yeah, it might be a nice improvement if this function, rather than making the query and updating RoleSet, actually just stored the relevant (resource_type, id) tuples into a BTreeSet, and then we used that at the end to build a sort of monstrous query in role_asgn_builtin_list_for.
This depends on #512. This change moves role assignments to the database. This should remove almost all the hardcoding from the policy (aside from the policy itself). (The remaining bit of hardcoded behavior is that the special "db-init" user is hardcoded to have the "init" role on the database. This is necessary because this user uses this role to load the rest of the built-in users and roles.)
Everything here is still "built-in users" with "built-in roles". I'm reluctant to generalize further without knowing what IdP-based users or custom roles will look like. I think we may want these to be in separate tables, which is why these tables all have "builtin" in the name. That said, I'm leaning towards replacing "built-in users" with service accounts, which we already know we need -- that way there are fewer distinct kinds of actor.
There are a few side effects of this change:
authorize()functions are now async. I think this is unavoidable. (It's also not a big deal.) In order to authorize you, we have to look up roles in the database. We prefetch the roles, but we can't do that until we know what resource you're authorized on, which we don't know untilauthorize().Status: I want to take a serious edit pass on the documentation in the authz crate, but otherwise I think this is ready for a look.