Skip to content

Commit

Permalink
OIDC: Configurable audience (#13140)
Browse files Browse the repository at this point in the history 
* warehouse, dev: configurable audience

Adds `OIDC_AUDIENCE` + `warehouse.oidc.audience` to allow
a Warehouse deployment to vary its expected OIDC audience.

Signed-off-by: William Woodruff <william@trailofbits.com>

* contest: fix `oidc_service` fixture

Signed-off-by: William Woodruff <william@trailofbits.com>

* tests: fixup OIDC service tests

Signed-off-by: William Woodruff <william@trailofbits.com>

---------

Signed-off-by: William Woodruff <william@trailofbits.com>
  • Loading branch information
@woodruffw
woodruffw authored Mar 6, 2023
1 parent ff64c6b commit 61d5c58
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 7 deletions.
1 change: 1 addition & 0 deletions dev/environment
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,4 @@ TWOFACTORREQUIREMENT_ENABLED=true
TWOFACTORMANDATE_AVAILABLE=true
TWOFACTORMANDATE_ENABLED=true
OIDC_ENABLED=true
OIDC_AUDIENCE=pypi
1 change: 1 addition & 0 deletions tests/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ def oidc_service(db_session):
GITHUB_OIDC_ISSUER_URL,
pretend.stub(),
pretend.stub(),
pretend.stub(),
)


Expand Down
32 changes: 30 additions & 2 deletions tests/unit/oidc/test_services.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ def test_oidc_publisher_service_factory():
request = pretend.stub(
db=pretend.stub(),
registry=pretend.stub(
settings={"oidc.jwk_cache_url": "rediss://another.example.com"}
settings={
"oidc.jwk_cache_url": "rediss://another.example.com",
"warehouse.oidc.audience": "fakeaudience",
}
),
find_service=lambda *a, **kw: metrics,
)
Expand All @@ -43,6 +46,7 @@ def test_oidc_publisher_service_factory():
assert service.db == request.db
assert service.publisher == factory.publisher
assert service.issuer_url == factory.issuer_url
assert service.audience == "fakeaudience"
assert service.cache_url == "rediss://another.example.com"
assert service.metrics == metrics

Expand All @@ -63,6 +67,7 @@ def test_verify_jwt_signature(self, monkeypatch):
session=pretend.stub(),
publisher=pretend.stub(),
issuer_url=pretend.stub(),
audience="fakeaudience",
cache_url=pretend.stub(),
metrics=pretend.stub(),
)
Expand Down Expand Up @@ -91,7 +96,7 @@ def test_verify_jwt_signature(self, monkeypatch):
verify_aud=True,
),
issuer=service.issuer_url,
audience="pypi",
audience="fakeaudience",
leeway=30,
)
]
Expand All @@ -102,6 +107,7 @@ def test_verify_jwt_signature_fails(self, monkeypatch, exc):
session=pretend.stub(),
publisher="fakepublisher",
issuer_url=pretend.stub(),
audience="fakeaudience",
cache_url=pretend.stub(),
metrics=pretend.stub(
increment=pretend.call_recorder(lambda *a, **kw: None)
Expand All @@ -128,6 +134,7 @@ def test_find_publisher(self, monkeypatch):
session=pretend.stub(),
publisher="fakepublisher",
issuer_url=pretend.stub(),
audience="fakeaudience",
cache_url=pretend.stub(),
metrics=pretend.stub(
increment=pretend.call_recorder(lambda *a, **kw: None)
Expand Down Expand Up @@ -159,6 +166,7 @@ def test_find_publisher_issuer_lookup_fails(self, monkeypatch):
session=pretend.stub(),
publisher="fakepublisher",
issuer_url=pretend.stub(),
audience="fakeaudience",
cache_url=pretend.stub(),
metrics=pretend.stub(
increment=pretend.call_recorder(lambda *a, **kw: None)
Expand Down Expand Up @@ -188,6 +196,7 @@ def test_find_publisher_verify_claims_fails(self, monkeypatch):
session=pretend.stub(),
publisher="fakepublisher",
issuer_url=pretend.stub(),
audience="fakeaudience",
cache_url=pretend.stub(),
metrics=pretend.stub(
increment=pretend.call_recorder(lambda *a, **kw: None)
Expand Down Expand Up @@ -219,6 +228,7 @@ def test_get_keyset_not_cached(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url=pretend.stub(),
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand All @@ -235,6 +245,7 @@ def test_get_keyset_cached(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url=pretend.stub(),
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand All @@ -254,6 +265,7 @@ def test_refresh_keyset_timeout(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand All @@ -277,6 +289,7 @@ def test_refresh_keyset_oidc_config_fails(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand Down Expand Up @@ -312,6 +325,7 @@ def test_refresh_keyset_oidc_config_no_jwks_uri(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand Down Expand Up @@ -349,6 +363,7 @@ def test_refresh_keyset_oidc_config_no_jwks_json(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand Down Expand Up @@ -397,6 +412,7 @@ def test_refresh_keyset_oidc_config_no_jwks_keys(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand Down Expand Up @@ -442,6 +458,7 @@ def test_refresh_keyset_successful(self, monkeypatch, mockredis):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand Down Expand Up @@ -492,6 +509,7 @@ def test_get_key_cached(self, monkeypatch):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand Down Expand Up @@ -522,6 +540,7 @@ def test_get_key_uncached(self, monkeypatch):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand Down Expand Up @@ -553,6 +572,7 @@ def test_get_key_refresh_fails(self, monkeypatch):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=metrics,
)
Expand All @@ -578,6 +598,7 @@ def test_get_key_for_token(self, monkeypatch):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand All @@ -598,6 +619,7 @@ def test_reify_publisher(self, monkeypatch):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand Down Expand Up @@ -629,6 +651,7 @@ def test_warns_on_init(self, monkeypatch):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand All @@ -648,6 +671,7 @@ def test_verify_jwt_signature_malformed_jwt(self):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand Down Expand Up @@ -675,6 +699,7 @@ def test_verify_jwt_signature_missing_aud(self):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand Down Expand Up @@ -704,6 +729,7 @@ def test_verify_jwt_signature_wrong_aud(self):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand All @@ -723,6 +749,7 @@ def test_find_publisher(self, monkeypatch):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="pypi",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand Down Expand Up @@ -819,6 +846,7 @@ def test_reify_publisher(self):
session=pretend.stub(),
publisher="example",
issuer_url="https://example.com",
audience="fakeaudience",
cache_url="rediss://fake.example.com",
metrics=pretend.stub(),
)
Expand Down
3 changes: 2 additions & 1 deletion warehouse/config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ def configure(settings=None):
default=0,
)

# OIDC feature flags
# OIDC feature flags and settings
maybe_set(settings, "warehouse.oidc.audience", "OIDC_AUDIENCE")
maybe_set(
settings,
"warehouse.oidc.enabled",
Expand Down
15 changes: 11 additions & 4 deletions warehouse/oidc/services.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class InsecureOIDCPublisherWarning(UserWarning):

@implementer(IOIDCPublisherService)
class NullOIDCPublisherService:
def __init__(self, session, publisher, issuer_url, cache_url, metrics):
def __init__(self, session, publisher, issuer_url, audience, cache_url, metrics):
warnings.warn(
"NullOIDCPublisherService is intended only for use in development, "
"you should not use it in production due to the lack of actual "
Expand Down Expand Up @@ -82,10 +82,11 @@ def reify_pending_publisher(self, pending_publisher, project):

@implementer(IOIDCPublisherService)
class OIDCPublisherService:
def __init__(self, session, publisher, issuer_url, cache_url, metrics):
def __init__(self, session, publisher, issuer_url, audience, cache_url, metrics):
self.db = session
self.publisher = publisher
self.issuer_url = issuer_url
self.audience = audience
self.cache_url = cache_url
self.metrics = metrics

Expand Down Expand Up @@ -241,7 +242,7 @@ def verify_jwt_signature(self, unverified_token: str) -> SignedClaims | None:
verify_aud=True,
),
issuer=self.issuer_url,
audience="pypi",
audience=self.audience,
leeway=30,
)
return SignedClaims(signed_payload)
Expand Down Expand Up @@ -304,10 +305,16 @@ def __init__(self, publisher, issuer_url, service_class=OIDCPublisherService):

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)

return self.service_class(
request.db, self.publisher, self.issuer_url, cache_url, metrics
request.db,
self.publisher,
self.issuer_url,
audience,
cache_url,
metrics,
)

def __eq__(self, other):
Expand Down

0 comments on commit 61d5c58

Please sign in to comment.