Skip to content

Workload Identity#7850

Open
BaptisteCentreon wants to merge 10 commits into
pulp:mainfrom
BaptisteCentreon:oidc-authentication-core
Open

Workload Identity#7850
BaptisteCentreon wants to merge 10 commits into
pulp:mainfrom
BaptisteCentreon:oidc-authentication-core

Conversation

@BaptisteCentreon

Copy link
Copy Markdown

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

@BaptisteCentreon BaptisteCentreon changed the title OIDC authentication core Workload Identity Jul 8, 2026
@BaptisteCentreon
BaptisteCentreon force-pushed the oidc-authentication-core branch from 0703cd2 to 88ecdfd Compare July 8, 2026 12:19
@ggainey

ggainey commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

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

Comment thread docs/admin/guides/auth/workload_identity.md Outdated
Comment thread pulpcore/app/workload_identity/authentication.py Outdated

@mdellweg mdellweg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

BaptisteCentreon commented Jul 13, 2026

Copy link
Copy Markdown
Author

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.

Comment thread pulpcore/app/workload_identity/principal.py Outdated
Comment thread pulpcore/app/access_policy.py Outdated
@BaptisteCentreon
BaptisteCentreon force-pushed the oidc-authentication-core branch from 89386b9 to 35e29b2 Compare July 13, 2026 11:02
@mdellweg

Copy link
Copy Markdown
Member

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

Comment thread pulpcore/app/access_policy.py
@BaptisteCentreon

BaptisteCentreon commented Jul 23, 2026

Copy link
Copy Markdown
Author

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

BaptisteCentreon commented Jul 24, 2026

Copy link
Copy Markdown
Author

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 mdellweg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pyproject.toml Outdated
Comment on lines +541 to +542
@pytest.fixture(scope="module")
def rsa_keys():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we use trustme in other places. Would that work here?

But also, Yay for generating on the fly. We don't want to ship key material here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked it, trustme generates ECDSA keys, so signing a JWT with RS256 fails outright (InvalidKeyError: Invalid Key type for RSAAlgorithm).

We'd have to hand it a separate RSA key anyway, so generating an RSA keypair with cryptography in the fixture is the direct fit, still on the fly with no key material shipped.

Comment thread pulpcore/app/checks.py

if collides:
messages.append(
CheckWarning(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only trigger at startup. So we are not protected against someone creating such a user later.

The "workload_identity.basic_username" is thought to be something like "token" for cases where clients don't really know anything but basic auth?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A check only runs at deploy time, so it won't catch a user created later, true. But that case is harmless: the auth falls through, so a real user with that name still logs in with their own password (a password can't be a valid signed JWT).

The warning is just there to avoid confusion, not as a security guarantee.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(a password can't be a valid signed JWT)

Cannot is probably not quite true. But if you manage to take the full blown token and set it as your usual password, you almost deserve it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well you got the point 😅

Comment thread pulpcore/app/workload_identity/principal.py Outdated
@BaptisteCentreon

Copy link
Copy Markdown
Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants