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.

[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?

[miketheman]

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

I believe the correct resolution for that would be to add an entry to:

warehouse/tests/conftest.py

Lines 307 to 342 in f59df18

	def get_app_config(database, nondefaults=None):
	settings = {
	"warehouse.prevent_esi": True,
	"warehouse.token": "insecure token",
	"warehouse.ip_salt": "insecure salt",
	"camo.url": "http://localhost:9000/",
	"camo.key": "insecure key",
	"celery.broker_redis_url": "redis://localhost:0/",
	"celery.result_url": "redis://localhost:0/",
	"celery.scheduler_url": "redis://localhost:0/",
	"database.url": database,
	"docs.url": "http://docs.example.com/",
	"ratelimit.url": "memory://",
	"db_results_cache.url": "redis://redis:0/",
	"opensearch.url": "https://localhost/warehouse",
	"files.backend": "warehouse.packaging.services.LocalFileStorage",
	"archive_files.backend": "warehouse.packaging.services.LocalArchiveFileStorage",
	"archive_files.path": "/tmp",
	"simple.backend": "warehouse.packaging.services.LocalSimpleStorage",
	"docs.backend": "warehouse.packaging.services.LocalDocsStorage",
	"sponsorlogos.backend": "warehouse.admin.services.LocalSponsorLogoStorage",
	"billing.backend": "warehouse.subscriptions.services.MockStripeBillingService",
	"integrity.backend": "warehouse.attestations.services.NullIntegrityService",
	"billing.api_base": "http://stripe:12111",
	"billing.api_version": "2020-08-27",
	"mail.backend": "warehouse.email.services.SMTPEmailSender",
	"helpdesk.backend": "warehouse.helpdesk.services.ConsoleHelpDeskService",
	"helpdesk.notification_backend": "warehouse.helpdesk.services.ConsoleAdminNotificationService", # noqa: E501
	"files.url": "http://localhost:7000/",
	"archive_files.url": "http://localhost:7000/archive",
	"sessions.secret": "123456",
	"sessions.url": "redis://localhost:0/",
	"statuspage.url": "https://2p66nmmycsj3.statuspage.io",
	"warehouse.xmlrpc.cache.url": "redis://localhost:0/",
	"terms.revision": "initial",
	}

Similar to "celery.scheduler_url": "redis://localhost:0/", this likely wants a "real" redis instance.

The get_app_config fixture is used in the webtest fixture creation

[facutuesca]

Nice! That worked. I've added two functional tests with a GitHub trusted publisher (while also modifying the test config to use NullOIDCPublisherService, since we won't be able to verify JWTs in the tests due to their expiration).

Last question: I tried to add tests for the other providers, but OIDC is disabled for those providers in the admin flags of the test db:

E           webtest.app.AppError: Bad response: 422 Unprocessable Entity (not 200)
E           b'{"errors":[{"code":"not-enabled","description":"gitlab trusted publishing functionality not enabled"}],"message":"Token request failed"}\n'

these are the flags:

warehouse/warehouse/admin/flags.py

Lines 18 to 22 in 79691b4

	DISALLOW_OIDC = "disallow-oidc"
	DISALLOW_GITHUB_OIDC = "disallow-github-oidc"
	DISALLOW_GITLAB_OIDC = "disallow-gitlab-oidc"
	DISALLOW_GOOGLE_OIDC = "disallow-google-oidc"
	DISALLOW_ACTIVESTATE_OIDC = "disallow-activestate-oidc"

What would be the correct way to enable them?

[miketheman]

Last question: I tried to add tests for the other providers, but OIDC is disabled for those providers in the admin flags of the test db:
...
What would be the correct way to enable them?

I haven't fully tested this, but this should be a good start.

In the context of webtest, we can dig through and get the preconfigured database session and use it:

from warehouse.admin.flags import AdminFlag, AdminFlagValue

db_sess = webtest.extra_environ['warehouse.db_session']

flag = db_sess.get(AdminFlag, AdminFlagValue.DISALLOW_GITLAB_OIDC.value)
flag.enabled = False

webtest.post(....)

Or something close to it.