Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tests/unit/oidc/models/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ def test_check_claim_invariant():


class TestOIDCPublisher:
def test_lookup_by_claims_default_none(self):
assert (
_core.OIDCPublisher.lookup_by_claims(pretend.stub(), pretend.stub()) is None
)

def test_oidc_publisher_not_default_verifiable(self):
publisher = _core.OIDCPublisher(projects=[])

Expand Down
15 changes: 15 additions & 0 deletions tests/unit/oidc/models/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,22 @@ def test_check_sub(claim):
assert github._check_sub(pretend.stub(), claim, pretend.stub()) is False


def test_lookup_strategies():
assert (
len(github.GitHubPublisher.__lookup_strategies__)
== len(github.PendingGitHubPublisher.__lookup_strategies__)
== 2
)


class TestGitHubPublisher:
def test_lookup_strategies(self):
assert (
len(github.GitHubPublisher.__lookup_strategies__)
== len(github.PendingGitHubPublisher.__lookup_strategies__)
== 2
)

def test_github_publisher_all_known_claims(self):
assert github.GitHubPublisher.all_known_claims() == {
# verifiable claims
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/oidc/models/test_google.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@
from warehouse.oidc.models import _core, google


def test_lookup_strategies():
assert (
len(google.GooglePublisher.__lookup_strategies__)
== len(google.PendingGooglePublisher.__lookup_strategies__)
== 2
)


class TestGooglePublisher:
def test_stringifies_as_email(self):
publisher = google.GooglePublisher(email="[email protected]")
Expand Down
38 changes: 36 additions & 2 deletions tests/unit/oidc/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from pyramid.authorization import Authenticated

from tests.common.db.oidc import GitHubPublisherFactory
from tests.common.db.oidc import GitHubPublisherFactory, GooglePublisherFactory
from warehouse.oidc import utils
from warehouse.utils.security_policy import principals_for

Expand Down Expand Up @@ -69,7 +69,41 @@ def test_find_publisher_by_issuer_github(db_request, environment, expected_id):
assert (
utils.find_publisher_by_issuer(
db_request.db,
"https://token.actions.githubusercontent.com",
utils.GITHUB_OIDC_ISSUER_URL,
signed_claims,
).id
== expected_id
)


@pytest.mark.parametrize(
"sub, expected_id",
[
("some-other-subject", uuid.UUID("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa")),
("some-subject", uuid.UUID("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb")),
],
)
def test_find_publisher_by_issuer_google(db_request, sub, expected_id):
GooglePublisherFactory(
id="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
email="[email protected]",
sub=None, # No subject
)
GooglePublisherFactory(
id="bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
email="[email protected]",
sub="some-subject", # Subject set
)

signed_claims = {
"email": "[email protected]",
"sub": sub,
}

assert (
utils.find_publisher_by_issuer(
db_request.db,
utils.GOOGLE_OIDC_ISSUER_URL,
signed_claims,
).id
== expected_id
Expand Down
11 changes: 10 additions & 1 deletion warehouse/oidc/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from warehouse.oidc.interfaces import IOIDCPublisherService
from warehouse.oidc.services import OIDCPublisherServiceFactory
from warehouse.oidc.utils import GITHUB_OIDC_ISSUER_URL
from warehouse.oidc.utils import GITHUB_OIDC_ISSUER_URL, GOOGLE_OIDC_ISSUER_URL


def includeme(config):
Expand All @@ -29,6 +29,15 @@ def includeme(config):
IOIDCPublisherService,
name="github",
)
config.register_service_factory(
OIDCPublisherServiceFactory(
publisher="google",
issuer_url=GOOGLE_OIDC_ISSUER_URL,
service_class=oidc_publisher_service_class,
),
IOIDCPublisherService,
name="google",
)

# During deployments, we separate auth routes into their own subdomain
# to simplify caching exclusion.
Expand Down
33 changes: 33 additions & 0 deletions warehouse/oidc/models/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

from collections.abc import Callable
from typing import Any, TypeVar

Expand Down Expand Up @@ -104,6 +106,37 @@ class OIDCPublisherMixin:
# not checked as part of verifying the JWT.
__unchecked_claims__: set[str] = set()

# Individual publishers can have complex unique constraints on their
# required and optional attributes, and thus can't be naively looked
# up from a raw claim set.
#
# Each subclass should explicitly override this list to contain
# class methods that take a `SignedClaims` and return a SQLAlchemy
# expression that, when queried, should produce exactly one or no result.
# This list should be ordered by specificity, e.g. selecting for the
# expression with the most optional constraints first, and ending with
# the expression with only required constraints.
#
# TODO(ww): In principle this list is computable directly from
# `__required_verifiable_claims__` and `__optional_verifiable_claims__`,
# but there are a few problems: those claim sets don't map to their
# "equivalent" column (only to an instantiated property), and may not
# even have an "equivalent" column.
__lookup_strategies__: list = []

@classmethod
def lookup_by_claims(cls, session, signed_claims: SignedClaims):
for lookup in cls.__lookup_strategies__:
query = lookup(cls, signed_claims)
if not query:
# We might not build a query if we know the claim set can't
# satisfy it. If that's the case, then we skip.
continue

if publisher := query.with_session(session).one_or_none():
return publisher
return None

@classmethod
def all_known_claims(cls) -> set[str]:
"""
Expand Down
53 changes: 53 additions & 0 deletions warehouse/oidc/models/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@

from sqlalchemy import Column, ForeignKey, String, UniqueConstraint
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.orm import Query
from sqlalchemy.sql.expression import func, literal

from warehouse.oidc.interfaces import SignedClaims
from warehouse.oidc.models._core import (
OIDCPublisher,
PendingOIDCPublisher,
Expand Down Expand Up @@ -125,6 +128,56 @@ class GitHubPublisherMixin:
"enterprise",
}

@staticmethod
def __lookup_all__(klass, signed_claims: SignedClaims) -> Query | None:
# This lookup requires the environment claim to be present;
# if it isn't, bail out early.
if not (environment := signed_claims.get("environment")):
return None

repository = signed_claims["repository"]
repository_owner, repository_name = repository.split("/", 1)
workflow_prefix = f"{repository}/.github/workflows/"
workflow_ref = signed_claims["job_workflow_ref"].removeprefix(workflow_prefix)

return (
Query(klass)
.filter_by(
repository_name=repository_name,
repository_owner=repository_owner,
repository_owner_id=signed_claims["repository_owner_id"],
environment=environment.lower(),
)
.filter(
literal(workflow_ref).like(func.concat(klass.workflow_filename, "%"))
)
)

@staticmethod
def __lookup_no_environment__(klass, signed_claims: SignedClaims) -> Query | None:
repository = signed_claims["repository"]
repository_owner, repository_name = repository.split("/", 1)
workflow_prefix = f"{repository}/.github/workflows/"
workflow_ref = signed_claims["job_workflow_ref"].removeprefix(workflow_prefix)

return (
Query(klass)
.filter_by(
repository_name=repository_name,
repository_owner=repository_owner,
repository_owner_id=signed_claims["repository_owner_id"],
environment=None,
)
.filter(
literal(workflow_ref).like(func.concat(klass.workflow_filename, "%"))
)
)

__lookup_strategies__ = [
__lookup_all__,
__lookup_no_environment__,
]

@property
def _workflow_slug(self):
return f".github/workflows/{self.workflow_filename}"
Expand Down
16 changes: 16 additions & 0 deletions warehouse/oidc/models/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from sqlalchemy import Column, ForeignKey, String, UniqueConstraint
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.orm import Query

from warehouse.oidc.interfaces import SignedClaims
from warehouse.oidc.models._core import (
Expand Down Expand Up @@ -61,6 +62,21 @@ class GooglePublisherMixin:

__unchecked_claims__ = {"azp", "google"}

@staticmethod
def __lookup_all__(klass, signed_claims: SignedClaims) -> Query | None:
return Query(klass).filter_by(
email=signed_claims["email"], sub=signed_claims["sub"]
)

@staticmethod
def __lookup_no_sub__(klass, signed_claims: SignedClaims) -> Query | None:
return Query(klass).filter_by(email=signed_claims["email"], sub=None)

__lookup_strategies__ = [
__lookup_all__,
__lookup_no_sub__,
]

@property
def email_verified(self):
# We don't consider a claim set valid unless `email_verified` is true;
Expand Down
78 changes: 19 additions & 59 deletions warehouse/oidc/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,26 @@
from dataclasses import dataclass

from pyramid.authorization import Authenticated
from sqlalchemy.sql.expression import func, literal

from warehouse.oidc.interfaces import SignedClaims
from warehouse.oidc.models import GitHubPublisher, OIDCPublisher, PendingGitHubPublisher
from warehouse.oidc.models import (
GitHubPublisher,
GooglePublisher,
OIDCPublisher,
PendingGitHubPublisher,
PendingGooglePublisher,
)
from warehouse.oidc.models._core import OIDCPublisherMixin

GITHUB_OIDC_ISSUER_URL = "https://token.actions.githubusercontent.com"
GOOGLE_OIDC_ISSUER_URL = "https://accounts.google.com"

OIDC_ISSUER_URLS = {GITHUB_OIDC_ISSUER_URL}
OIDC_ISSUER_URLS = {GITHUB_OIDC_ISSUER_URL, GOOGLE_OIDC_ISSUER_URL}

OIDC_PUBLISHER_CLASSES: dict[str, dict[bool, type[OIDCPublisherMixin]]] = {
GITHUB_OIDC_ISSUER_URL: {False: GitHubPublisher, True: PendingGitHubPublisher},
GOOGLE_OIDC_ISSUER_URL: {False: GooglePublisher, True: PendingGooglePublisher},
}


def find_publisher_by_issuer(session, issuer_url, signed_claims, *, pending=False):
Expand All @@ -35,66 +47,14 @@ def find_publisher_by_issuer(session, issuer_url, signed_claims, *, pending=Fals
Returns `None` if no publisher can be found.
"""

if issuer_url not in OIDC_ISSUER_URLS:
try:
publisher_cls = OIDC_PUBLISHER_CLASSES[issuer_url][pending]
except KeyError:
# This indicates a logic error, since we shouldn't have verified
# claims for an issuer that we don't recognize and support.
return None

# This is the ugly part: OIDCPublisher and PendingOIDCPublisher are both
# polymorphic, and retrieving the correct publisher requires us to query
# based on publisher-specific claims.
if issuer_url == GITHUB_OIDC_ISSUER_URL:
repository = signed_claims["repository"]
repository_owner, repository_name = repository.split("/", 1)
workflow_prefix = f"{repository}/.github/workflows/"
workflow_ref = signed_claims["job_workflow_ref"].removeprefix(workflow_prefix)

publisher_cls = GitHubPublisher if not pending else PendingGitHubPublisher

publisher = None
# If an environment exists in the claim set, try finding a publisher
# that matches the provided environment first.
if environment := signed_claims.get("environment"):
publisher = (
session.query(publisher_cls)
.filter_by(
repository_name=repository_name,
repository_owner=repository_owner,
repository_owner_id=signed_claims["repository_owner_id"],
environment=environment.lower(),
)
.filter(
literal(workflow_ref).like(
func.concat(publisher_cls.workflow_filename, "%")
)
)
.one_or_none()
)

# There are no publishers for that specific environment, try finding a
# publisher without a restriction on the environment
if not publisher:
publisher = (
session.query(publisher_cls)
.filter_by(
repository_name=repository_name,
repository_owner=repository_owner,
repository_owner_id=signed_claims["repository_owner_id"],
environment=None,
)
.filter(
literal(workflow_ref).like(
func.concat(publisher_cls.workflow_filename, "%")
)
)
.one_or_none()
)

return publisher

else:
# Unreachable; same logic error as above.
return None # pragma: no cover
return publisher_cls.lookup_by_claims(session, signed_claims)


@dataclass
Expand Down