Workload Identity#7850
Conversation
0703cd2 to
88ecdfd
Compare
|
|
mdellweg
left a comment
There was a problem hiding this comment.
I'm still not fully grasping this (well, it is a lot).
But I'm wondering why it does not tie into the Django authentication backend framework, specifically for functions like "permissions_for".
|
Hey @mdellweg, First thanks for the review :) To answer your question: two reasons. The permissions come from the token and change every request, they aren't tied to a stored user. A Django backend only sees the user object, never the token, so it would just read them back off the object we already built. No real gain. On top of that, this "user" isn't a real database user (it has no id). The default Django backend expects a real one and blows up when it tries to look things up for a user with no id. By answering the permission checks on the object itself, we sidestep that entirely. I'm happy to wrap it in a backend if you'd rather keep it consistent with the rest, but it would really just call the same code and still lean on this object, so it wouldn't change much under the hood. |
89386b9 to
35e29b2
Compare
|
Sorry, I probably didn't express that sufficiently (and then I got distracted), but i really want us to stick to the authentication class and backend mechanics provided by django. That should help us to keep the plug-ability and consistency is always desired. |
| def get_user_group_values(self, user): | ||
| """Let a stateless principal supply its groups directly instead of via the ORM.""" | ||
| from pulpcore.app.workload_identity.principal import WorkloadIdentityPrincipal | ||
|
|
||
| if isinstance(user, WorkloadIdentityPrincipal): | ||
| return list(user.group_names) | ||
| return super().get_user_group_values(user) | ||
|
|
There was a problem hiding this comment.
I'm rather conflicted here. Yes it's sad that access policies assumes the groups come from django.contrib.auth.
But as you say you don't intend to use gruops for now, what if we try to not touch this file at all, and add an empty list (of non existing groups) to the principle?
This MR, adds an optional way for a CI job to authenticate with a short lived OIDC token from a provider like GitHub Actions, instead of a stored password.
The token maps to roles and scopes for that request only, based on the WORKLOAD_IDENTITY setting. It is not active out of the box, existing deployments stay the same until the auth class is configured.
I have opened an issue that go in details into the changes in this MR : #7845