diff --git a/tests/unit/accounts/test_core.py b/tests/unit/accounts/test_core.py index 94d34ecb1267..24465b78c992 100644 --- a/tests/unit/accounts/test_core.py +++ b/tests/unit/accounts/test_core.py @@ -29,9 +29,10 @@ database_login_factory, ) from warehouse.accounts.tasks import compute_user_metrics +from warehouse.accounts.utils import UserTokenContext from warehouse.oidc.interfaces import SignedClaims from warehouse.oidc.models import OIDCPublisher -from warehouse.oidc.utils import OIDCContext +from warehouse.oidc.utils import PublisherTokenContext from warehouse.rate_limiting import IRateLimiter, RateLimit from ...common.db.accounts import UserFactory @@ -45,6 +46,13 @@ def test_with_user(self, db_request): assert accounts._user(request) is user + def test_with_user_token_context(self, db_request): + user = UserFactory.create() + user_ctx = UserTokenContext(user, pretend.stub()) + request = pretend.stub(identity=user_ctx) + + assert accounts._user(request) is user + def test_without_user_identity(self): nonuser = pretend.stub() request = pretend.stub(identity=nonuser) @@ -62,7 +70,7 @@ def test_with_oidc_publisher(self, db_request): assert isinstance(publisher, OIDCPublisher) claims = SignedClaims({"foo": "bar"}) - request = pretend.stub(identity=OIDCContext(publisher, claims)) + request = pretend.stub(identity=PublisherTokenContext(publisher, claims)) assert accounts._oidc_publisher(request) is publisher assert accounts._oidc_claims(request) is claims diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index 3b798cea6353..463e0cb9cddd 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -29,12 +29,13 @@ from trove_classifiers import classifiers from webob.multidict import MultiDict +from warehouse.accounts.utils import UserTokenContext from warehouse.admin.flags import AdminFlag, AdminFlagValue from warehouse.classifiers.models import Classifier from warehouse.forklift import legacy, metadata from warehouse.metrics import IMetricsService from warehouse.oidc.interfaces import SignedClaims -from warehouse.oidc.utils import OIDCContext +from warehouse.oidc.utils import PublisherTokenContext from warehouse.packaging.interfaces import IFileStorage, IProjectService from warehouse.packaging.models import ( Dependency, @@ -945,6 +946,7 @@ def test_upload_escapes_nul_characters(self, pyramid_config, db_request): assert "\x00" not in db_request.POST["summary"] + @pytest.mark.parametrize("token_context", [True, False]) @pytest.mark.parametrize( ("digests",), [ @@ -973,6 +975,7 @@ def test_successful_upload( pyramid_config, db_request, digests, + token_context, metrics, ): monkeypatch.setattr(tempfile, "tempdir", str(tmpdir)) @@ -987,8 +990,13 @@ def test_successful_upload( filename = f"{project.name}-{release.version}.tar.gz" - pyramid_config.testing_securitypolicy(identity=user) db_request.user = user + if token_context: + user_context = UserTokenContext(user, pretend.stub()) + pyramid_config.testing_securitypolicy(identity=user_context) + else: + pyramid_config.testing_securitypolicy(identity=user) + db_request.user_agent = "warehouse-tests/6.6.6" content = FieldStorage() @@ -2975,7 +2983,7 @@ def test_upload_succeeds_creates_release( else: publisher = GitHubPublisherFactory.create(projects=[project]) claims = {"sha": "somesha"} - identity = OIDCContext(publisher, SignedClaims(claims)) + identity = PublisherTokenContext(publisher, SignedClaims(claims)) db_request.oidc_publisher = identity.publisher db_request.oidc_claims = identity.claims diff --git a/tests/unit/macaroons/test_caveats.py b/tests/unit/macaroons/test_caveats.py index 35954ec5dec1..4d9828612ca6 100644 --- a/tests/unit/macaroons/test_caveats.py +++ b/tests/unit/macaroons/test_caveats.py @@ -20,6 +20,7 @@ from pymacaroons import Macaroon from warehouse.accounts import _oidc_publisher +from warehouse.accounts.utils import UserTokenContext from warehouse.macaroons import caveats from warehouse.macaroons.caveats import ( Caveat, @@ -36,7 +37,7 @@ verify, ) from warehouse.macaroons.caveats._core import _CaveatRegistry -from warehouse.oidc.utils import OIDCContext +from warehouse.oidc.utils import PublisherTokenContext from ...common.db.accounts import UserFactory from ...common.db.oidc import GitHubPublisherFactory @@ -280,10 +281,11 @@ def test_verify_invalid_identity(self): def test_verify_invalid_user_id(self, db_request): user = UserFactory.create() + user_token_context = UserTokenContext(user, pretend.stub()) caveat = RequestUser(user_id="invalid") result = caveat.verify( - pretend.stub(identity=user), pretend.stub(), pretend.stub() + pretend.stub(identity=user_token_context), pretend.stub(), pretend.stub() ) assert result == Failure( @@ -292,10 +294,11 @@ def test_verify_invalid_user_id(self, db_request): def test_verify_ok(self, db_request): user = UserFactory.create() + user_token_context = UserTokenContext(user, pretend.stub()) caveat = RequestUser(user_id=str(user.id)) result = caveat.verify( - pretend.stub(identity=user), pretend.stub(), pretend.stub() + pretend.stub(identity=user_token_context), pretend.stub(), pretend.stub() ) assert result == Success() @@ -315,7 +318,7 @@ def test_verify_no_identity(self): ) def test_verify_invalid_publisher_id(self, db_request): - identity = OIDCContext(GitHubPublisherFactory.create(), None) + identity = PublisherTokenContext(GitHubPublisherFactory.create(), None) request = pretend.stub(identity=identity) request.oidc_publisher = _oidc_publisher(request) @@ -327,7 +330,7 @@ def test_verify_invalid_publisher_id(self, db_request): ) def test_verify_invalid_context(self, db_request): - identity = OIDCContext(GitHubPublisherFactory.create(), None) + identity = PublisherTokenContext(GitHubPublisherFactory.create(), None) request = pretend.stub(identity=identity) request.oidc_publisher = _oidc_publisher(request) @@ -342,7 +345,9 @@ 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 = OIDCContext(GitHubPublisherFactory.create(projects=[foobar]), None) + identity = PublisherTokenContext( + 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)) @@ -356,7 +361,9 @@ 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 = OIDCContext(GitHubPublisherFactory.create(projects=[foobar]), None) + identity = PublisherTokenContext( + 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)) diff --git a/tests/unit/macaroons/test_security_policy.py b/tests/unit/macaroons/test_security_policy.py index 84397ac2bf47..4ce4a2e691b8 100644 --- a/tests/unit/macaroons/test_security_policy.py +++ b/tests/unit/macaroons/test_security_policy.py @@ -20,12 +20,13 @@ 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 OIDCContext +from warehouse.oidc.utils import PublisherTokenContext @pytest.mark.parametrize( @@ -214,7 +215,7 @@ def test_identity_user(self, monkeypatch): ), ) - assert policy.identity(request) is user + assert policy.identity(request) == UserTokenContext(user, macaroon) assert extract_http_macaroon.calls == [pretend.call(request)] assert request.find_service.calls == [ pretend.call(IMacaroonService, context=None), @@ -258,7 +259,7 @@ def test_identity_oidc_publisher(self, monkeypatch): identity = policy.identity(request) assert identity assert identity.publisher is oidc_publisher - assert identity == OIDCContext( + assert identity == PublisherTokenContext( oidc_publisher, SignedClaims(oidc_additional["oidc"]) ) diff --git a/tests/unit/macaroons/test_utils.py b/tests/unit/macaroons/test_utils.py new file mode 100644 index 000000000000..bbeab50c0667 --- /dev/null +++ b/tests/unit/macaroons/test_utils.py @@ -0,0 +1,24 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import pretend + +from tests.common.db.accounts import UserFactory +from warehouse.accounts.utils import UserTokenContext +from warehouse.utils.security_policy import principals_for + + +def test_usertoken_context_principals(db_request): + user = UserFactory.create() + assert principals_for( + UserTokenContext(user=user, macaroon=pretend.stub()) + ) == principals_for(user) diff --git a/tests/unit/oidc/test_utils.py b/tests/unit/oidc/test_utils.py index 3b51d18bd756..4522eb9b29c4 100644 --- a/tests/unit/oidc/test_utils.py +++ b/tests/unit/oidc/test_utils.py @@ -238,7 +238,7 @@ def test_find_publisher_by_issuer_activestate( def test_oidc_context_principals(): assert principals_for( - utils.OIDCContext(publisher=pretend.stub(id=17), claims=None) + utils.PublisherTokenContext(publisher=pretend.stub(id=17), claims=None) ) == [ Authenticated, "oidc:17", diff --git a/tests/unit/utils/test_security_policy.py b/tests/unit/utils/test_security_policy.py index 0c1c1b0683af..69324b8b8f84 100644 --- a/tests/unit/utils/test_security_policy.py +++ b/tests/unit/utils/test_security_policy.py @@ -15,6 +15,8 @@ from pyramid.interfaces import ISecurityPolicy from zope.interface.verify import verifyClass +from tests.common.db.accounts import UserFactory +from warehouse.accounts.utils import UserTokenContext from warehouse.utils import security_policy @@ -86,11 +88,24 @@ def test_authenticated_userid_nonuser_identity(self, db_request): assert policy.authenticated_userid(request) is None - def test_authenticated_userid(self, monkeypatch): - monkeypatch.setattr(security_policy, "User", pretend.stub) + def test_authenticated_userid_user_token_context(self, db_request): + user = UserFactory.create() + user_ctx = UserTokenContext(user, pretend.stub()) + + request = pretend.stub(add_finished_callback=lambda *a, **kw: None) + subpolicies = [pretend.stub(identity=lambda r: user_ctx)] + policy = security_policy.MultiSecurityPolicy(subpolicies) + + assert ( + policy.authenticated_userid(request) + == str(user.id) + == str(user_ctx.user.id) + ) + + def test_authenticated_userid(self, db_request): + user = UserFactory.create() request = pretend.stub(add_finished_callback=lambda *a, **kw: None) - user = pretend.stub(id="a fake user") subpolicies = [pretend.stub(identity=lambda r: user)] policy = security_policy.MultiSecurityPolicy(subpolicies) diff --git a/warehouse/accounts/__init__.py b/warehouse/accounts/__init__.py index c88cec823f2c..85837cfaa4c4 100644 --- a/warehouse/accounts/__init__.py +++ b/warehouse/accounts/__init__.py @@ -32,9 +32,10 @@ database_login_factory, ) from warehouse.accounts.tasks import compute_user_metrics +from warehouse.accounts.utils import UserTokenContext from warehouse.admin.flags import AdminFlagValue from warehouse.macaroons.security_policy import MacaroonSecurityPolicy -from warehouse.oidc.utils import OIDCContext +from warehouse.oidc.utils import PublisherTokenContext from warehouse.organizations.services import IOrganizationService from warehouse.rate_limiting import IRateLimiter, RateLimit from warehouse.utils.security_policy import MultiSecurityPolicy @@ -54,23 +55,29 @@ def _user(request): if request.identity is None: return None - if not isinstance(request.identity, User): + if isinstance(request.identity, UserTokenContext): + # A UserTokenContext signals a user-created API token; + # take the underlying user. + return request.identity.user + elif isinstance(request.identity, User): + return request.identity + else: return None - return request.identity - def _oidc_publisher(request): return ( request.identity.publisher - if isinstance(request.identity, OIDCContext) + if isinstance(request.identity, PublisherTokenContext) else None ) def _oidc_claims(request): return ( - request.identity.claims if isinstance(request.identity, OIDCContext) else None + request.identity.claims + if isinstance(request.identity, PublisherTokenContext) + else None ) diff --git a/warehouse/accounts/security_policy.py b/warehouse/accounts/security_policy.py index ab5e15932638..efcb99ec3fa8 100644 --- a/warehouse/accounts/security_policy.py +++ b/warehouse/accounts/security_policy.py @@ -171,7 +171,9 @@ def permits(self, request, context, permission): def _permits_for_user_policy(acl, request, context, permission): # It should only be possible for request.identity to be a User object - # at this point, and we only a User in these policies. + # at this point, and we only allow a User in these policies. + # Note that UserTokenContext is not allowed here, since a UserTokenContext + # can only appear in an API-token-authenticated request, not a session. assert isinstance(request.identity, User) # Dispatch to our ACL diff --git a/warehouse/accounts/utils.py b/warehouse/accounts/utils.py new file mode 100644 index 000000000000..d120945861e2 --- /dev/null +++ b/warehouse/accounts/utils.py @@ -0,0 +1,46 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from warehouse.accounts.models import User + from warehouse.macaroons.models import Macaroon + + +@dataclass +class UserTokenContext: + """ + This class supports `MacaroonSecurityPolicy` in + `warehouse.macaroons.security_policy`. + + It is a wrapper containing both the user associated with an API token + authenticated request and its corresponding Macaroon. We use it to smuggle + the Macaroon associated to the API token through to a session. `request.identity` + in an API token authenticated request should return this type. + """ + + user: User + """ + The associated user. + """ + + macaroon: Macaroon + """ + The Macaroon associated to the API token used to authenticate. + """ + + def __principals__(self) -> list[str]: + return self.user.__principals__() diff --git a/warehouse/macaroons/caveats/__init__.py b/warehouse/macaroons/caveats/__init__.py index 1b1bcdc06308..9bb112f57488 100644 --- a/warehouse/macaroons/caveats/__init__.py +++ b/warehouse/macaroons/caveats/__init__.py @@ -21,7 +21,7 @@ from pyramid.request import Request from pyramid.security import Allowed -from warehouse.accounts.models import User +from warehouse.accounts.utils import UserTokenContext from warehouse.errors import WarehouseDenied from warehouse.macaroons.caveats._core import ( Caveat, @@ -104,10 +104,10 @@ class RequestUser(Caveat): user_id: StrictStr def verify(self, request: Request, context: Any, permission: str) -> Result: - if not isinstance(request.identity, User): + if not isinstance(request.identity, UserTokenContext): return Failure("token with user restriction without a user") - if str(request.identity.id) != self.user_id: + if str(request.identity.user.id) != self.user_id: return Failure("current user does not match user restriction in token") return Success() diff --git a/warehouse/macaroons/security_policy.py b/warehouse/macaroons/security_policy.py index 5190d13cb3c2..0748c1025cc6 100644 --- a/warehouse/macaroons/security_policy.py +++ b/warehouse/macaroons/security_policy.py @@ -17,13 +17,14 @@ 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 OIDCContext +from warehouse.oidc.utils import PublisherTokenContext from warehouse.utils.security_policy import AuthenticationMethod, principals_for @@ -119,9 +120,9 @@ def identity(self, request): is_disabled, _ = login_service.is_disabled(dm.user.id) if is_disabled: return None - return dm.user + return UserTokenContext(dm.user, dm) - return OIDCContext(dm.oidc_publisher, oidc_claims) + return PublisherTokenContext(dm.oidc_publisher, oidc_claims) def remember(self, request, userid, **kw): # This is a NO-OP because our Macaroon header policy doesn't allow diff --git a/warehouse/oidc/utils.py b/warehouse/oidc/utils.py index 155275748077..3b38f861301e 100644 --- a/warehouse/oidc/utils.py +++ b/warehouse/oidc/utils.py @@ -94,7 +94,7 @@ def find_publisher_by_issuer( @dataclass -class OIDCContext: +class PublisherTokenContext: """ This class supports `MacaroonSecurityPolicy` in `warehouse.macaroons.security_policy`. diff --git a/warehouse/utils/security_policy.py b/warehouse/utils/security_policy.py index 25a81860a152..40db36d7aa51 100644 --- a/warehouse/utils/security_policy.py +++ b/warehouse/utils/security_policy.py @@ -18,6 +18,7 @@ from zope.interface import implementer from warehouse.accounts.models import User +from warehouse.accounts.utils import UserTokenContext # NOTE: Is there a better place for this to live? It may not even need to exist @@ -84,6 +85,8 @@ def authenticated_userid(self, request): # more correct pattern before fixing this. if isinstance(ident, User): return str(ident.id) + elif isinstance(ident, UserTokenContext): + return str(ident.user.id) return None def forget(self, request, **kw):