oidc: Refactor lookup strategies into single functions

[facutuesca]

This PR refactors the Trusted Publishing "lookup strategies" pattern into a single lookup_by_claims() method for each of the publishers.

Context

A strategy is a way of, given a set of OIDC claims, query the database for a matching Trusted Publisher. Concretely, a strategy is a function that takes a set of claims and returns a Query object. Each publisher has a list of these strategies, ordered from specific to general. When trying to find a Trusted Publisher, the more specific strategies are tried first, and if they fail the more general ones are tried.

For example, for the given set of claims for a GitHub OIDC token:

{
    "repository": "foo/bar",
    "job_workflow_ref": "foo/bar/.github/workflows/release.yml@refs/heads/main",
    "environment": "my_environment",
}

first we ran a strategy that tried to find publishers with exactly those values (in particular, environment==my_environment). If that strategy failed, we tried the second strategy, where we tried to find publishers with environment==None (that is, allowing any environment).
The "specific to general" order in this case meant going from "Publishers that only allow my_environment as an environment" to "Publishers that allow any environment".

New implementation

This PR changes the above approach to a single function per provider called lookup_by_claims() which takes a set of claims and returns a Publisher.

The multiple strategies are collapsed into a single query: we query for all publishers where the non-optional fields match the claims. In the example above, this means our single query looks for all publishers that match repository and job_workflow_ref, ignoring the environment value.

We then look at the resulting Python Publisher objects, and select the most specific one.

Rationale

The reasons for this change are:

• Removing the multiple lookup strategies, which are unnecessary since a single (general) query is enough
• Preparing for adding support for GitHub reusable workflows (as sketched in Trusted publishing: Support for GitHub reusable workflows #11096 (comment)).

cc @woodruffw @miketheman @di

[miketheman]

I like the move from using a list-in-subclass to a function-that-contains-the-complexity, as that simplifies the overall design and allows individual publishers to decide their own lookup method.

I'm also a little wary of how well-tested these publishers are. There's one functional test for provenance that creates a GitHub publisher, but everything else in theOIDC publishers stack seems heavily unit/mocked tests, so would it make sense to add some more functional tests around these interactions? The amount of mixins and abstractions make me a little cautions, being less familiar with the code behaviors.

[miketheman]

I think this comment is now invalid and probably should be written to express the new design of "implement the checks in a subclass' lookup_by_claims()

[facutuesca]

good catch, fixed

[miketheman]

Optimize: This might be cleaner with more_itertools.first_true, like so:

Suggested change

	specific_publishers = [p for p in publishers if p.sub == sub]
	if specific_publishers:
	return specific_publishers[0]
	if specific_publisher := first_true(publishers, pred=lambda p: p.sub == sub):
	return specific_publisher

And in other places that are using this iteration pattern. Definitely worth thinking about other selector patterns from more-itertools, since we have it available for use.

Not a sticking point, but wanted to call out the pattern while I noticed it.

[facutuesca]

fixed

[facutuesca]

@miketheman

so would it make sense to add some more functional tests around these interactions

I have a question that came up while trying to write the functional tests: I started the tests by POSTing to /_/oidc/mint-token, since the token exchange is where the trusted publisher lookup happens. However, since I'm not mocking the request object (like the unit tests do), I get a failure here:

warehouse/warehouse/oidc/services.py

Lines 343 to 346 in 79691b4

	def __call__(self, _context, request):
	cache_url = request.registry.settings["oidc.jwk_cache_url"]
	audience = request.registry.settings["warehouse.oidc.audience"]
	metrics = request.find_service(IMetricsService, context=None)

since the oidc.jwk_cache_url settings key does not exist:

>       cache_url = request.registry.settings["oidc.jwk_cache_url"]
E       KeyError: 'oidc.jwk_cache_url'

Do we have a way to set it from the tests, without mocking the whole request object?