Workload Identity

[BaptisteCentreon]

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

[ggainey]

ruff format needs to run clean before CI executes - could you do a quick format-pass and resubmit?

[mdellweg]

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".

[BaptisteCentreon]

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.

[mdellweg]

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.
As far as I can see, your "Principal" is what is later presented as the "User" on all requests. And the question of having permissions is to be answered by the backends (Django will query all configured ones I believe). "Not changing anything under the hood" sounds good, not against that...

[BaptisteCentreon]

Hey,
I was wondering if you need any additionnal changes ? Anything you'd like me to fix or we are good ?

Edit : Nevermind I had missed the notification

[BaptisteCentreon]

Reworked both to lean on Django's backend mechanics as you asked :

Permissions are now answered by a dedicated WorkloadIdentityBackend in AUTHENTICATION_BACKENDS, so has_perm and get_all_permissions go through the normal backend loop instead of the principal answering them itself.

The principal just carries the grants and exposes empty relations so ModelBackend and the role backend run without a database id.

For groups, get_user_group_values now branches on the user type: real database users take the normal ORM path, and any other principal (this one, or a future SAML2 one) supplies its groups via group_names.

No reference to the workload-identity class in access_policy.py anymore, so it stays pluggable.

[mdellweg]

Not a full review, but i want to keep the ball rolling.

Are all the lazy imports necessary? And if so by means of circular imports, we should think about how the dependencies flow.

[BaptisteCentreon]

I think fixed all of the points, please let me know if you need any more changes. I'm not a python or django expert, I appreciate the time spent to review :)

[mdellweg]

Please don't merge changes from main. You should rebase your branch instead.

[BaptisteCentreon]

Please don't merge changes from main. You should rebase your branch instead.

No problem, done :)

It might be useful to add this to the contributing docs as this is very much a per project preference thing. I had a look and could not see anything, if I missed it, my bad.

[mdellweg]

We do document that:
https://pulpproject.org/pulpcore/docs/dev/guides/git/#rebasing-and-squashing

Also I will eventually ask you to squash the commits so that they are logically and not temporally sorted.

[mdellweg]

https://docs.djangoproject.com/en/6.1/ref/contrib/auth/#django.contrib.auth.models.User.get_all_permissions

This is not Pulp...

[BaptisteCentreon]

Hmm I'm not sure I understand what you mean on this one 😅

I did it this way to mirror what ObjectRolePermissionBackend already does. It returns bare codenames when there's an obj, and my_permissions counts on that since it prepends the app_label itself.

Did I misunderstand what you're after?

[BaptisteCentreon]

Ok i took me a minute but I think I get what you meant now.

What do you prefer : keep it aligned with the rest of the code (ObjectRolePermissionBackend + my_permissions) or follow Django's format? Either works for me, just let me know.

[mdellweg]

What i was saying is that it's not a pulp decision, but a django.contrib.auth interface we are implementing. I was referring to the comment.

Now that you made me look, I believe this might actually be a misstep made in pulpcore.backends. "If obj is passed in, only returns the permissions for this specific object." does not mean the format should change, just that the list is filtered.

I need to think about this.

[mdellweg]

I completely forgot about that:

#1767

I don't really see evidence of the claims in the code (not even the 3.2 django branch), so it would be up to experiment.

[BaptisteCentreon]

I didn't think to look in howtos, my bad, I will squash everything and add the changes entry once your are satisfied with the code :)

[mdellweg]

I meant you want to remove this comment. It's redundant (and i happened to spot it).