Skip to content

Revert "Return Macaroon alongside User in `MacaroonSecurityPolicy… #15588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 13, 2024
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
4 changes: 2 additions & 2 deletions tests/unit/accounts/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from warehouse.accounts.tasks import compute_user_metrics
from warehouse.oidc.interfaces import SignedClaims
from warehouse.oidc.models import OIDCPublisher
from warehouse.oidc.utils import PublisherTokenContext
from warehouse.oidc.utils import OIDCContext
from warehouse.rate_limiting import IRateLimiter, RateLimit

from ...common.db.accounts import UserFactory
Expand Down Expand Up @@ -62,7 +62,7 @@ def test_with_oidc_publisher(self, db_request):
assert isinstance(publisher, OIDCPublisher)
claims = SignedClaims({"foo": "bar"})

request = pretend.stub(identity=PublisherTokenContext(publisher, claims))
request = pretend.stub(identity=OIDCContext(publisher, claims))

assert accounts._oidc_publisher(request) is publisher
assert accounts._oidc_claims(request) is claims
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/forklift/test_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from warehouse.forklift import legacy
from warehouse.metrics import IMetricsService
from warehouse.oidc.interfaces import SignedClaims
from warehouse.oidc.utils import PublisherTokenContext
from warehouse.oidc.utils import OIDCContext
from warehouse.packaging.interfaces import IFileStorage, IProjectService
from warehouse.packaging.models import (
Dependency,
Expand Down Expand Up @@ -3354,7 +3354,7 @@ def test_upload_succeeds_creates_release(
else:
publisher = GitHubPublisherFactory.create(projects=[project])
claims = {"sha": "somesha"}
identity = PublisherTokenContext(publisher, SignedClaims(claims))
identity = OIDCContext(publisher, SignedClaims(claims))
db_request.oidc_publisher = identity.publisher
db_request.oidc_claims = identity.claims

Expand Down
14 changes: 5 additions & 9 deletions tests/unit/macaroons/test_caveats.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
verify,
)
from warehouse.macaroons.caveats._core import _CaveatRegistry
from warehouse.oidc.utils import PublisherTokenContext
from warehouse.oidc.utils import OIDCContext

from ...common.db.accounts import UserFactory
from ...common.db.oidc import GitHubPublisherFactory
Expand Down Expand Up @@ -295,7 +295,7 @@ def test_verify_no_identity(self):
)

def test_verify_invalid_publisher_id(self, db_request):
identity = PublisherTokenContext(GitHubPublisherFactory.create(), None)
identity = OIDCContext(GitHubPublisherFactory.create(), None)
request = pretend.stub(identity=identity)
request.oidc_publisher = _oidc_publisher(request)

Expand All @@ -307,7 +307,7 @@ def test_verify_invalid_publisher_id(self, db_request):
)

def test_verify_invalid_context(self, db_request):
identity = PublisherTokenContext(GitHubPublisherFactory.create(), None)
identity = OIDCContext(GitHubPublisherFactory.create(), None)
request = pretend.stub(identity=identity)
request.oidc_publisher = _oidc_publisher(request)

Expand All @@ -322,9 +322,7 @@ def test_verify_invalid_project(self, db_request):

# This OIDC publisher is only registered to "foobar", so it should
# not verify a caveat presented for "foobaz".
identity = PublisherTokenContext(
GitHubPublisherFactory.create(projects=[foobar]), None
)
identity = OIDCContext(GitHubPublisherFactory.create(projects=[foobar]), None)
request = pretend.stub(identity=identity)
request.oidc_publisher = _oidc_publisher(request)
caveat = OIDCPublisher(oidc_publisher_id=str(request.oidc_publisher.id))
Expand All @@ -338,9 +336,7 @@ def test_verify_ok(self, db_request):

# This OIDC publisher is only registered to "foobar", so it should
# not verify a caveat presented for "foobaz".
identity = PublisherTokenContext(
GitHubPublisherFactory.create(projects=[foobar]), None
)
identity = OIDCContext(GitHubPublisherFactory.create(projects=[foobar]), None)
request = pretend.stub(identity=identity)
request.oidc_publisher = _oidc_publisher(request)
caveat = OIDCPublisher(oidc_publisher_id=str(request.oidc_publisher.id))
Expand Down
7 changes: 3 additions & 4 deletions tests/unit/macaroons/test_security_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@
from zope.interface.verify import verifyClass

from warehouse.accounts.interfaces import IUserService
from warehouse.accounts.utils import UserTokenContext
from warehouse.authnz import Permissions
from warehouse.macaroons import security_policy
from warehouse.macaroons.interfaces import IMacaroonService
from warehouse.macaroons.services import InvalidMacaroonError
from warehouse.oidc.interfaces import SignedClaims
from warehouse.oidc.utils import PublisherTokenContext
from warehouse.oidc.utils import OIDCContext


@pytest.mark.parametrize(
Expand Down Expand Up @@ -215,7 +214,7 @@ def test_identity_user(self, monkeypatch):
),
)

assert policy.identity(request) == UserTokenContext(user, macaroon)
assert policy.identity(request) is user
assert extract_http_macaroon.calls == [pretend.call(request)]
assert request.find_service.calls == [
pretend.call(IMacaroonService, context=None),
Expand Down Expand Up @@ -259,7 +258,7 @@ def test_identity_oidc_publisher(self, monkeypatch):
identity = policy.identity(request)
assert identity
assert identity.publisher is oidc_publisher
assert identity == PublisherTokenContext(
assert identity == OIDCContext(
oidc_publisher, SignedClaims(oidc_additional["oidc"])
)

Expand Down
24 changes: 0 additions & 24 deletions tests/unit/macaroons/test_utils.py

This file was deleted.

2 changes: 1 addition & 1 deletion tests/unit/oidc/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def test_find_publisher_by_issuer_activestate(

def test_oidc_context_principals():
assert principals_for(
utils.PublisherTokenContext(publisher=pretend.stub(id=17), claims=None)
utils.OIDCContext(publisher=pretend.stub(id=17), claims=None)
) == [
Authenticated,
"oidc:17",
Expand Down
8 changes: 3 additions & 5 deletions warehouse/accounts/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from warehouse.accounts.tasks import compute_user_metrics
from warehouse.admin.flags import AdminFlagValue
from warehouse.macaroons.security_policy import MacaroonSecurityPolicy
from warehouse.oidc.utils import PublisherTokenContext
from warehouse.oidc.utils import OIDCContext
from warehouse.organizations.services import IOrganizationService
from warehouse.rate_limiting import IRateLimiter, RateLimit
from warehouse.utils.security_policy import MultiSecurityPolicy
Expand Down Expand Up @@ -63,16 +63,14 @@ def _user(request):
def _oidc_publisher(request):
return (
request.identity.publisher
if isinstance(request.identity, PublisherTokenContext)
if isinstance(request.identity, OIDCContext)
else None
)


def _oidc_claims(request):
return (
request.identity.claims
if isinstance(request.identity, PublisherTokenContext)
else None
request.identity.claims if isinstance(request.identity, OIDCContext) else None
)


Expand Down
44 changes: 0 additions & 44 deletions warehouse/accounts/utils.py

This file was deleted.

7 changes: 3 additions & 4 deletions warehouse/macaroons/security_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@
from zope.interface import implementer

from warehouse.accounts.interfaces import IUserService
from warehouse.accounts.utils import UserTokenContext
from warehouse.authnz import Permissions
from warehouse.cache.http import add_vary_callback
from warehouse.errors import WarehouseDenied
from warehouse.macaroons import InvalidMacaroonError
from warehouse.macaroons.interfaces import IMacaroonService
from warehouse.metrics.interfaces import IMetricsService
from warehouse.oidc.utils import PublisherTokenContext
from warehouse.oidc.utils import OIDCContext
from warehouse.utils.security_policy import AuthenticationMethod, principals_for


Expand Down Expand Up @@ -120,9 +119,9 @@ def identity(self, request):
is_disabled, _ = login_service.is_disabled(dm.user.id)
if is_disabled:
return None
return UserTokenContext(dm.user, dm)
return dm.user

return PublisherTokenContext(dm.oidc_publisher, oidc_claims)
return OIDCContext(dm.oidc_publisher, oidc_claims)

def remember(self, request, userid, **kw):
# This is a NO-OP because our Macaroon header policy doesn't allow
Expand Down
2 changes: 1 addition & 1 deletion warehouse/oidc/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def find_publisher_by_issuer(


@dataclass
class PublisherTokenContext:
class OIDCContext:
"""
This class supports `MacaroonSecurityPolicy` in
`warehouse.macaroons.security_policy`.
Expand Down