diff --git a/dev/environment b/dev/environment index e047ef16ccd3..fabc0cd1bd6a 100644 --- a/dev/environment +++ b/dev/environment @@ -37,6 +37,3 @@ TOKEN_PASSWORD_SECRET="an insecure password reset secret key" TOKEN_EMAIL_SECRET="an insecure email verification secret key" WAREHOUSE_LEGACY_DOMAIN=pypi.python.org - -ACCOUNT_TOKEN_SECRET="an insecure token api secret" -ACCOUNT_TOKEN_PUBLIC_ID="a public token api id" diff --git a/tests/common/db/accounts.py b/tests/common/db/accounts.py index ec239f7c6100..905c2c47b714 100644 --- a/tests/common/db/accounts.py +++ b/tests/common/db/accounts.py @@ -53,6 +53,7 @@ class Meta: model = AccountToken id = factory.Sequence(lambda n: uuid.uuid4()) + secret = factory.fuzzy.FuzzyText(length=100) username = factory.fuzzy.FuzzyText() description = factory.fuzzy.FuzzyText(length=100) is_active = True diff --git a/tests/unit/accounts/test_auth_policy.py b/tests/unit/accounts/test_auth_policy.py index 1050355a2ec9..b667f01abd26 100644 --- a/tests/unit/accounts/test_auth_policy.py +++ b/tests/unit/accounts/test_auth_policy.py @@ -14,12 +14,12 @@ import uuid from pymacaroons import Macaroon -from pyramid import authentication -from pyramid.interfaces import IAuthenticationPolicy +from pyramid import authentication, security +from pyramid.interfaces import IAuthenticationPolicy, IAuthorizationPolicy from zope.interface.verify import verifyClass from warehouse.accounts import auth_policy -from warehouse.accounts.interfaces import IUserService +from warehouse.accounts.interfaces import IUserService, IAccountTokenService from ...common.db.accounts import AccountTokenFactory, UserFactory @@ -112,174 +112,223 @@ def test_unauthenticated_userid(self, monkeypatch): class TestAccountTokenAuthenticationPolicy: def test_verify(self): - assert isinstance( - auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()), - authentication.CallbackAuthenticationPolicy, + assert verifyClass( + IAuthenticationPolicy, auth_policy.AccountTokenAuthenticationPolicy ) - def test_account_token_routes_allowed(self): - request = pretend.stub(matched_route=pretend.stub(name="not_a_real_route")) + def test_routes_not_allowed(self): + request = pretend.stub(matched_route=pretend.stub(name="not_allowed_route")) - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) - assert policy.unauthenticated_userid(request) is None + authn_policy = auth_policy.AccountTokenAuthenticationPolicy( + pretend.stub(), ["allowed_route"] + ) + + assert authn_policy.unauthenticated_userid(request) is None - def test_account_token_required_parameter(self): + def test_require_known(self): + # Ensure we don't accept just any macaroon request = pretend.stub( - matched_route=pretend.stub(name="forklift.legacy.file_upload"), params={} + find_service=lambda iface, **kw: { + IAccountTokenService: pretend.stub( + get_unverified_macaroon=(lambda: (None, None)) + ) + }[iface], + matched_route=pretend.stub(name="allowed_route"), + add_response_callback=lambda x: None, ) - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) - assert policy.unauthenticated_userid(request) is None + authn_policy = auth_policy.AccountTokenAuthenticationPolicy( + pretend.stub(), ["allowed_route"] + ) + + assert authn_policy.unauthenticated_userid(request) is None + + def test_macaroon_verifier(self, db_request): + user = UserFactory.create(username="test_user") + + account_token = AccountTokenFactory.create( + secret="some_secret", username=user.username + ) + + macaroon = Macaroon( + location="pypi.org", identifier="some_id", key="wrong_secret" + ) - def test_account_token_malformed(self): request = pretend.stub( - matched_route=pretend.stub(name="forklift.legacy.file_upload"), - params={"account_token": "DEADBEEF"}, + find_service=lambda iface, **kw: { + IAccountTokenService: pretend.stub( + get_unverified_macaroon=(lambda: (macaroon, account_token)) + ) + }[iface], + matched_route=pretend.stub(name="allowed_route"), + add_response_callback=lambda x: None, ) - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) - assert policy.unauthenticated_userid(request) is None + authn_policy = auth_policy.AccountTokenAuthenticationPolicy( + pretend.stub(), ["allowed_route"] + ) + + assert authn_policy.unauthenticated_userid(request) is None + + def test_account_token_auth(self, db_request): + # Test basic happy path + user = UserFactory.create(username="test_user") + account_token = AccountTokenFactory.create( + secret="some_secret", username=user.username + ) - def test_account_token_bad_settings(self): - # Test bad location macaroon = Macaroon( - location="notpypi.org", identifier="example_id", key="example_secret" + location="pypi.org", identifier=str(account_token.id), key="some_secret" ) request = pretend.stub( - matched_route=pretend.stub(name="forklift.legacy.file_upload"), - params={"account_token": macaroon.serialize()}, - registry=pretend.stub( - settings={ - "account_token.id": "example_id", - "account_token.secret": "example_secret", - } - ), + find_service=lambda iface, **kw: { + IUserService: pretend.stub( + find_userid=(lambda x: user.id if x == "test_user" else None) + ), + IAccountTokenService: pretend.stub( + get_unverified_macaroon=(lambda: (macaroon, account_token)), + update_last_used=(lambda x: None), + ), + }[iface], + matched_route=pretend.stub(name="allowed_route"), + add_response_callback=lambda x: None, + session={}, ) - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) - assert policy.unauthenticated_userid(request) is None + authn_policy = auth_policy.AccountTokenAuthenticationPolicy( + pretend.stub(), ["allowed_route"] + ) - # Test bad identifier - macaroon = Macaroon( - location="pypi.org", identifier="bad_id", key="example_secret" + assert authn_policy.unauthenticated_userid(request) == user.id + + # Make sure we allow first-party and third-party caveats + macaroon.add_first_party_caveat("first party caveat") + + macaroon.add_third_party_caveat( + location="mysite.com", key="anykey", key_id="anykeyid" ) - request.params["account_token"] = macaroon.serialize() - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) - assert policy.unauthenticated_userid(request) is None + assert authn_policy.unauthenticated_userid(request) == user.id - # Tamper with macaroon - macaroon = Macaroon( - location="pypi.org", identifier="example_id", key="example_secret" + def test_account_token_interface(self): + def _authenticate(a, b): + return a, b + + policy = auth_policy.AccountTokenAuthenticationPolicy(_authenticate, ["route"]) + + assert policy.remember("", "") == [] + assert policy.forget("") == [] + assert policy._auth_callback(1, 2) == (1, 2) + assert policy._routes_allowed == ["route"] + + +class TestAccountTokenAuthorizationPolicy: + def test_verify(self): + assert verifyClass( + IAuthorizationPolicy, auth_policy.AccountTokenAuthorizationPolicy ) - serialized = macaroon.serialize() + def test_have_request(self, monkeypatch): + monkeypatch.setattr(auth_policy, "get_current_request", lambda: None) + authz_policy = auth_policy.AccountTokenAuthorizationPolicy(pretend.stub()) - request.params["account_token"] = "".join( - (serialized[:-8], "AAAAAAA", serialized[-1:]) + assert isinstance( + authz_policy.permits(pretend.stub(), pretend.stub(), pretend.stub()), + security.Denied, + ) + + def test_macaroon_verifier(self, db_request, monkeypatch): + user = UserFactory.create(username="test_user") + + account_token = AccountTokenFactory.create( + secret="some_secret", username=user.username ) - assert policy.unauthenticated_userid(request) is None - def test_account_token_with_no_user(self, db_request): macaroon = Macaroon( - location="pypi.org", identifier="example_id", key="example_secret" + location="pypi.org", identifier="some_id", key="wrong_secret" ) + monkeypatch.setattr(auth_policy, "get_current_request", lambda: request) request = pretend.stub( find_service=lambda iface, **kw: { - IUserService: pretend.stub(find_userid_by_account_token=pretend.stub()) - }[iface], - params={"account_token": macaroon.serialize()}, - registry=pretend.stub( - settings={ - "account_token.id": "example_id", - "account_token.secret": "example_secret", - } - ), - matched_route=pretend.stub(name="forklift.legacy.file_upload"), - db=db_request, - session={}, + IAccountTokenService: pretend.stub( + get_unverified_macaroon=(lambda: (macaroon, account_token)) + ) + }[iface] ) - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) - assert policy.unauthenticated_userid(request) is None + authz_policy = auth_policy.AccountTokenAuthorizationPolicy(pretend.stub()) - def test_account_token_auth(self, db_request): - # Test basic happy path + assert isinstance( + authz_policy.permits(pretend.stub(), pretend.stub(), pretend.stub()), + security.Denied, + ) + + def test_account_token_authz(self, db_request, monkeypatch): user = UserFactory.create(username="test_user") - account_token = AccountTokenFactory.create(username=user.username) - account_token_id = str(account_token.id) - macaroon = Macaroon( - location="pypi.org", identifier="example_id", key="example_secret" + account_token = AccountTokenFactory.create( + secret="some_secret", username=user.username ) - macaroon.add_first_party_caveat(f"id: {account_token_id}") + macaroon = Macaroon( + location="pypi.org", identifier="some_id", key="some_secret" + ) + monkeypatch.setattr(auth_policy, "get_current_request", lambda: request) request = pretend.stub( find_service=lambda iface, **kw: { - IUserService: pretend.stub( - find_userid_by_account_token=( - lambda x: user.id if x == account_token_id else None - ) + IAccountTokenService: pretend.stub( + get_unverified_macaroon=(lambda: (macaroon, account_token)) ) - }[iface], - params={"account_token": macaroon.serialize()}, - registry=pretend.stub( - settings={ - "account_token.id": "example_id", - "account_token.secret": "example_secret", - } - ), - matched_route=pretend.stub(name="forklift.legacy.file_upload"), - db=db_request, - session={}, + }[iface] ) - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) - assert policy.unauthenticated_userid(request) == user.id + authz_policy = auth_policy.AccountTokenAuthorizationPolicy( + pretend.stub(permits=(lambda *args, **kwargs: "allow")) + ) - # Test package_list caveats - macaroon.add_first_party_caveat("package_list: pyexample1 pyexample2") - macaroon.add_third_party_caveat( - location="mysite.com", key="anykey", key_id="anykeyid" + assert ( + authz_policy.permits(pretend.stub(), pretend.stub(), pretend.stub()) + == "allow" ) - request.params["account_token"] = macaroon.serialize() - request.session["account_token_package_list"] = None - assert policy.unauthenticated_userid(request) == user.id - assert request.session["account_token_package_list"] == "pyexample1 pyexample2" + # Make sure we allow first-party and third-party caveats + macaroon.add_first_party_caveat("first party caveat") - # Ensure you can't overwrite previous caveats - takeover_user = UserFactory.create(username="takeover_user") - takeover_account_token = AccountTokenFactory.create( - username=takeover_user.username + macaroon.add_third_party_caveat( + location="mysite.com", key="anykey", key_id="anykeyid" ) - takeover_account_token_id = str(takeover_account_token.id) - - macaroon.add_first_party_caveat(f"id: {takeover_account_token_id}") - macaroon.add_first_party_caveat("package_list: additionalpackage") - request.params["account_token"] = macaroon.serialize() - request.session["account_token_package_list"] = None + assert ( + authz_policy.permits(pretend.stub(), pretend.stub(), pretend.stub()) + == "allow" + ) - assert policy.unauthenticated_userid(request) == user.id - assert request.session["account_token_package_list"] == "pyexample1 pyexample2" + def test_missing_macaroon(self, monkeypatch): + monkeypatch.setattr(auth_policy, "get_current_request", lambda: request) - def test_first_party_caveat_validation(self): - policy = auth_policy.AccountTokenAuthenticationPolicy(pretend.stub()) + request = pretend.stub( + find_service=lambda iface, **kw: { + IAccountTokenService: pretend.stub( + get_unverified_macaroon=(lambda: (None, None)) + ) + }[iface] + ) - assert policy._validate_first_party_caveat("id") - assert policy._validate_first_party_caveat("package_list") - assert not policy._validate_first_party_caveat("not_valid") + authz_policy = auth_policy.AccountTokenAuthorizationPolicy( + pretend.stub(permits=(lambda *args, **kwargs: "allow")) + ) - def test_account_token_interface(self): - def _authenticate(a, b): - return a, b + assert ( + authz_policy.permits(pretend.stub(), pretend.stub(), pretend.stub()) + == "allow" + ) - policy = auth_policy.AccountTokenAuthenticationPolicy(_authenticate) + def test_principals_allowed_by_permission(self): + authz_policy = auth_policy.AccountTokenAuthorizationPolicy( + pretend.stub(principals_allowed_by_permission=(lambda a, b: (a, b))) + ) - assert policy.remember("", "") == [] - assert policy.forget("") == [] - assert policy._auth_callback(1, 2) == (1, 2) + assert authz_policy.principals_allowed_by_permission(1, 2) == (1, 2) diff --git a/tests/unit/accounts/test_core.py b/tests/unit/accounts/test_core.py index e0de90d4682f..030cb9694082 100644 --- a/tests/unit/accounts/test_core.py +++ b/tests/unit/accounts/test_core.py @@ -19,6 +19,7 @@ from warehouse import accounts from warehouse.accounts.interfaces import ( IUserService, + IAccountTokenService, ITokenService, IPasswordBreachedService, ) @@ -26,6 +27,7 @@ TokenServiceFactory, HaveIBeenPwnedPasswordBreachedService, database_login_factory, + database_account_token_factory, ) from warehouse.accounts.models import DisableReason from warehouse.errors import BasicAuthBreachedPassword @@ -254,7 +256,9 @@ def test_without_userid(self): def test_includeme(monkeypatch): account_token_obj = pretend.stub() - account_token_cls = pretend.call_recorder(lambda authenticate: account_token_obj) + account_token_cls = pretend.call_recorder( + lambda authenticate, routes_allowed: account_token_obj + ) basic_authn_obj = pretend.stub() basic_authn_cls = pretend.call_recorder(lambda check: basic_authn_obj) session_authn_obj = pretend.stub() @@ -262,12 +266,15 @@ def test_includeme(monkeypatch): authn_obj = pretend.stub() authn_cls = pretend.call_recorder(lambda *a: authn_obj) authz_obj = pretend.stub() - authz_cls = pretend.call_recorder(lambda: authz_obj) + authz_cls = pretend.call_recorder(lambda policy: authz_obj) + acl_authz_obj = pretend.stub() + acl_authz_cls = pretend.call_recorder(lambda *args: acl_authz_obj) monkeypatch.setattr(accounts, "AccountTokenAuthenticationPolicy", account_token_cls) monkeypatch.setattr(accounts, "BasicAuthAuthenticationPolicy", basic_authn_cls) monkeypatch.setattr(accounts, "SessionAuthenticationPolicy", session_authn_cls) monkeypatch.setattr(accounts, "MultiAuthenticationPolicy", authn_cls) - monkeypatch.setattr(accounts, "ACLAuthorizationPolicy", authz_cls) + monkeypatch.setattr(accounts, "AccountTokenAuthorizationPolicy", authz_cls) + monkeypatch.setattr(accounts, "ACLAuthorizationPolicy", acl_authz_cls) config = pretend.stub( registry=pretend.stub(settings={}), @@ -284,6 +291,7 @@ def test_includeme(monkeypatch): assert config.register_service_factory.calls == [ pretend.call(database_login_factory, IUserService), + pretend.call(database_account_token_factory, IAccountTokenService), pretend.call( TokenServiceFactory(name="password"), ITokenService, name="password" ), @@ -303,11 +311,14 @@ def test_includeme(monkeypatch): assert config.set_authentication_policy.calls == [pretend.call(authn_obj)] assert config.set_authorization_policy.calls == [pretend.call(authz_obj)] assert account_token_cls.calls == [ - pretend.call(authenticate=accounts._authenticate) + pretend.call( + authenticate=accounts._authenticate, + routes_allowed=["forklift.legacy.file_upload"], + ) ] assert basic_authn_cls.calls == [pretend.call(check=accounts._basic_auth_login)] assert session_authn_cls.calls == [pretend.call(callback=accounts._authenticate)] assert authn_cls.calls == [ pretend.call([account_token_obj, session_authn_obj, basic_authn_obj]) ] - assert authz_cls.calls == [pretend.call()] + assert authz_cls.calls == [pretend.call(policy=acl_authz_obj)] diff --git a/tests/unit/accounts/test_services.py b/tests/unit/accounts/test_services.py index ae4a3adbb2be..b88a1666f253 100644 --- a/tests/unit/accounts/test_services.py +++ b/tests/unit/accounts/test_services.py @@ -24,6 +24,7 @@ from warehouse.accounts import services from warehouse.accounts.interfaces import ( IUserService, + IAccountTokenService, ITokenService, IPasswordBreachedService, TokenExpired, @@ -340,23 +341,91 @@ def test_updating_password_undisables(self, user_service): user_service.update_user(user.id, password="foo") assert user_service.is_disabled(user.id) == (False, None) - def test_find_userid_by_account_token(self, user_service): - user = UserFactory.create() - account_token = AccountTokenFactory.create(username=user.username) - user_id = user_service.find_userid_by_account_token(account_token.id) - assert user_id == user.id +class TestAccountTokenService: + def test_verify_service(self): + assert verifyClass(IAccountTokenService, services.AccountTokenService) - def test_find_userid_by_account_token_failure(self, user_service): - user_id = user_service.find_userid_by_account_token(uuid.uuid4()) - assert user_id is None + def test_get_nonexistant_account_token(self, db_session): + service = services.AccountTokenService(headers={}, session=db_session) + assert service.get_account_token(str(uuid.uuid4())) is None - def test_get_tokens_by_username(self, user_service): - user = UserFactory.create() - account_token = AccountTokenFactory.create(username=user.username) + def test_get_account_token(self, db_session): + user = UserFactory.create(username="test_user") + account_token = AccountTokenFactory.create( + secret="some_secret", username=user.username + ) + + service = services.AccountTokenService(headers={}, session=db_session) + assert service.get_account_token(account_token.id).id == account_token.id + + def test_get_unverified_macaroon(self, db_session): + user = UserFactory.create(username="test_user") + service = services.AccountTokenService(headers={}, session=db_session) + + unverified_macaroon, account_token = service.get_unverified_macaroon() + assert unverified_macaroon is None and account_token is None + + service.headers["Authorization"] = "Basic" + unverified_macaroon, account_token = service.get_unverified_macaroon() + assert unverified_macaroon is None and account_token is None + + service.headers["Authorization"] = "notmacaroon 123" + unverified_macaroon, account_token = service.get_unverified_macaroon() + assert unverified_macaroon is None and account_token is None + + service.headers["Authorization"] = "macaroon AAAAAA" + unverified_macaroon, account_token = service.get_unverified_macaroon() + assert unverified_macaroon is None and account_token is None + + macaroon = service.create_token(user.username, "testing tokens") + service.headers["Authorization"] = "macaroon {}".format(macaroon) + unverified_macaroon, account_token = service.get_unverified_macaroon() + assert unverified_macaroon.identifier == str(account_token.id) + + service.delete_token(account_token.id, user.username) + unverified_macaroon, account_token = service.get_unverified_macaroon() + assert unverified_macaroon is None and account_token is None + + def test_update_last_used(self, db_session): + user = UserFactory.create(username="test_user") + account_token = AccountTokenFactory.create( + secret="some_secret", username=user.username + ) + + service = services.AccountTokenService(headers={}, session=db_session) + service.update_last_used(account_token.id) + + account_token = service.get_account_token(account_token.id) + assert account_token.last_used is not None + + def test_get_tokens_by_username(self, db_session): + user = UserFactory.create(username="test_user") + service = services.AccountTokenService(headers={}, session=db_session) + + service.create_token(user.username, "test token 1") + service.create_token(user.username, "test token 2") + + assert len(service.get_tokens_by_username("test_user")) == 2 + + def test_delete_token(self, db_session): + user = UserFactory.create(username="test_user") + account_token = AccountTokenFactory.create( + secret="some_secret", username=user.username + ) + + service = services.AccountTokenService(headers={}, session=db_session) + assert service.delete_token(account_token.id, user.username) + assert not service.delete_token(account_token.id, user.username) + + +def test_database_account_token_factory(): + account_token_service = services.database_account_token_factory( + pretend.stub(), pretend.stub(headers=1, db=2) + ) - found_account_token = user_service.get_tokens_by_username(user.username) - assert account_token in found_account_token + assert account_token_service.headers == 1 + assert account_token_service.db == 2 class TestTokenService: diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index a0ebf5c8bc53..c3f81054cad4 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -21,11 +21,15 @@ from webob.multidict import MultiDict from warehouse.manage import views -from warehouse.accounts.interfaces import IUserService, IPasswordBreachedService +from warehouse.accounts.interfaces import ( + IUserService, + IPasswordBreachedService, + IAccountTokenService, +) from warehouse.packaging.models import JournalEntry, Project, Role, User from warehouse.utils.project import remove_documentation -from ...common.db.accounts import AccountTokenFactory, EmailFactory +from ...common.db.accounts import EmailFactory from ...common.db.packaging import ( JournalEntryFactory, ProjectFactory, @@ -38,12 +42,14 @@ class TestManageAccount: def test_default_response(self, monkeypatch): breach_service = pretend.stub() - user_service = pretend.stub(get_tokens_by_username=lambda x: []) + user_service = pretend.stub() + account_token_service = pretend.stub(get_tokens_by_username=lambda x: []) name = pretend.stub() request = pretend.stub( find_service=lambda iface, **kw: { IPasswordBreachedService: breach_service, IUserService: user_service, + IAccountTokenService: account_token_service, }[iface], user=pretend.stub(name=name, username=pretend.stub()), ) @@ -492,21 +498,20 @@ def test_reverify_email_already_verified(self, monkeypatch): def test_add_new_account_token(self, db_request): user = UserFactory.create() breach_service = pretend.stub() - user_service = pretend.stub(get_tokens_by_username=lambda x: []) + user_service = pretend.stub() + account_token_service = pretend.stub( + create_token=lambda username, description: "macaroon", + get_tokens_by_username=lambda x: [], + ) request = pretend.stub( db=db_request.db, find_service=lambda iface, **kw: { IPasswordBreachedService: breach_service, IUserService: user_service, + IAccountTokenService: account_token_service, }[iface], POST={"description": "account token description"}, - registry=pretend.stub( - settings={ - "account_token.id": "example_id", - "account_token.secret": "example_secret", - } - ), session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), user=user, ) @@ -520,13 +525,15 @@ def test_add_new_account_token(self, db_request): def test_invalid_add_new_account_token(self, db_request): user = UserFactory.create() breach_service = pretend.stub() - user_service = pretend.stub(get_tokens_by_username=lambda x: []) + user_service = pretend.stub() + account_token_service = pretend.stub(get_tokens_by_username=lambda x: []) request = pretend.stub( db=db_request.db, find_service=lambda iface, **kw: { IPasswordBreachedService: breach_service, IUserService: user_service, + IAccountTokenService: account_token_service, }[iface], POST={}, session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), @@ -540,18 +547,21 @@ def test_invalid_add_new_account_token(self, db_request): def test_delete_account_token(self, db_request): user = UserFactory.create() - account_token = AccountTokenFactory.create(username=user.username) breach_service = pretend.stub() - user_service = pretend.stub(get_tokens_by_username=lambda x: []) + user_service = pretend.stub() + account_token_service = pretend.stub( + get_tokens_by_username=lambda x: [], delete_token=lambda x, y: True + ) request = pretend.stub( db=db_request.db, find_service=lambda iface, **kw: { IPasswordBreachedService: breach_service, IUserService: user_service, + IAccountTokenService: account_token_service, }[iface], - params={"account_token_id": account_token.id}, + params={"account_token_id": uuid.uuid4()}, session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), user=user, ) @@ -566,13 +576,17 @@ def test_delete_account_token(self, db_request): def test_delete_nonexistent_account_token(self, db_request): user = UserFactory.create() breach_service = pretend.stub() - user_service = pretend.stub(get_tokens_by_username=lambda x: []) + user_service = pretend.stub() + account_token_service = pretend.stub( + get_tokens_by_username=lambda x: [], delete_token=lambda x, y: False + ) request = pretend.stub( db=db_request.db, find_service=lambda iface, **kw: { IPasswordBreachedService: breach_service, IUserService: user_service, + IAccountTokenService: account_token_service, }[iface], params={"account_token_id": uuid.uuid4()}, session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), diff --git a/warehouse/accounts/__init__.py b/warehouse/accounts/__init__.py index 419dadd0a708..e468b9facbf9 100644 --- a/warehouse/accounts/__init__.py +++ b/warehouse/accounts/__init__.py @@ -17,6 +17,7 @@ from warehouse.accounts.interfaces import ( IUserService, + IAccountTokenService, ITokenService, IPasswordBreachedService, ) @@ -25,10 +26,12 @@ NullPasswordBreachedService, TokenServiceFactory, database_login_factory, + database_account_token_factory, ) from warehouse.accounts.models import DisableReason from warehouse.accounts.auth_policy import ( AccountTokenAuthenticationPolicy, + AccountTokenAuthorizationPolicy, BasicAuthAuthenticationPolicy, SessionAuthenticationPolicy, ) @@ -117,6 +120,11 @@ def includeme(config): # Register our login service config.register_service_factory(database_login_factory, IUserService) + # Register our account token service + config.register_service_factory( + database_account_token_factory, IAccountTokenService + ) + # Register our token services config.register_service_factory( TokenServiceFactory(name="password"), ITokenService, name="password" @@ -139,13 +147,18 @@ def includeme(config): config.set_authentication_policy( MultiAuthenticationPolicy( [ - AccountTokenAuthenticationPolicy(authenticate=_authenticate), + AccountTokenAuthenticationPolicy( + authenticate=_authenticate, + routes_allowed=["forklift.legacy.file_upload"], + ), SessionAuthenticationPolicy(callback=_authenticate), BasicAuthAuthenticationPolicy(check=_basic_auth_login), ] ) ) - config.set_authorization_policy(ACLAuthorizationPolicy()) + config.set_authorization_policy( + AccountTokenAuthorizationPolicy(policy=ACLAuthorizationPolicy()) + ) # Add a request method which will allow people to access the user object. config.add_request_method(_user, name="user", reify=True) diff --git a/warehouse/accounts/auth_policy.py b/warehouse/accounts/auth_policy.py index 03f33db4913b..6dc064041957 100644 --- a/warehouse/accounts/auth_policy.py +++ b/warehouse/accounts/auth_policy.py @@ -10,9 +10,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import struct - -from pymacaroons import Macaroon, Verifier +from pymacaroons import Verifier from pymacaroons.caveat_delegates import ThirdPartyCaveatVerifierDelegate from pymacaroons.exceptions import MacaroonException @@ -22,98 +20,85 @@ SessionAuthenticationPolicy as _SessionAuthenticationPolicy, ) -from warehouse.accounts.interfaces import IUserService +from pyramid.interfaces import IAuthenticationPolicy, IAuthorizationPolicy +from pyramid.security import Denied +from pyramid.threadlocal import get_current_request + +from zope.interface import implementer + +from warehouse.accounts.interfaces import IAccountTokenService, IUserService from warehouse.cache.http import add_vary_callback +class PassThroughThirdPartyCaveats(ThirdPartyCaveatVerifierDelegate): + def verify_third_party_caveat(self, *args, **kwargs): + return True + + +@implementer(IAuthenticationPolicy) class AccountTokenAuthenticationPolicy(_CallbackAuthenticationPolicy): - def __init__(self, authenticate): + def __init__(self, authenticate, routes_allowed): self._authenticate = authenticate self.callback = self._auth_callback - self._routes_allowed = ["forklift.legacy.file_upload"] + self._routes_allowed = routes_allowed def unauthenticated_userid(self, request): if request.matched_route.name not in self._routes_allowed: return None - account_token = request.params.get("account_token") + # If we're calling into this API on a request, then we want to register + # a callback which will ensure that the response varies based on the + # Authorization header. + request.add_response_callback(add_vary_callback("Authorization")) + + account_token_service = request.find_service(IAccountTokenService, context=None) + + ( + unverified_macaroon, + account_token, + ) = account_token_service.get_unverified_macaroon() if account_token is None: return None + # Validate authentication properties of the macaroon try: - macaroon = Macaroon.deserialize(account_token) - - # First, check identifier and location - if macaroon.identifier != request.registry.settings["account_token.id"]: - return None - - if macaroon.location != "pypi.org": - return None - - # Check the macaroon against our configuration verifier = Verifier() + verifier.third_party_caveat_verifier_delegate = ( + PassThroughThirdPartyCaveats() + ) - verifier.third_party_caveat_verifier_delegate = IgnoreThirdPartyCaveats() - - verifier.satisfy_general(self._validate_first_party_caveat) - - verifier.verify(macaroon, request.registry.settings["account_token.secret"]) - - # Get id from token - account_token_id = None - package_list = None - - for each in macaroon.first_party_caveats(): - caveat = each.to_dict() - caveat_parts = caveat["cid"].split(": ") - caveat_key = caveat_parts[0] - caveat_value = ": ".join(caveat_parts[1:]) - - # If caveats are specified multiple times, only trust the - # first value we encounter. - if caveat_key == "id" and account_token_id is None: - account_token_id = caveat_value - - elif caveat_key == "package_list" and package_list is None: - package_list = caveat_value - - if package_list is not None: - request.session["account_token_package_list"] = package_list + verifier.satisfy_general(self._validate_first_party_caveat_authentication) - if account_token_id is not None: - login_service = request.find_service(IUserService, context=None) + verifier.verify(unverified_macaroon, account_token.secret) - return login_service.find_userid_by_account_token(account_token_id) - - except (struct.error, MacaroonException): + except MacaroonException: return None + # Macaroon verified, so update last used and return user + account_token_service.update_last_used(account_token.id) + login_service = request.find_service(IUserService, context=None) + return login_service.find_userid(account_token.username) + def remember(self, request, userid, **kw): """A no-op. Let other authenticators handle this.""" return [] def forget(self, request): - """ A no-op. Let other authenticators handle this.""" + """A no-op. Let other authenticators handle this.""" return [] - def _validate_first_party_caveat(self, caveat): - # Only support 'id' and 'package_list' caveat for now - if caveat.split(": ")[0] not in ["id", "package_list"]: - return False - + def _validate_first_party_caveat_authentication(self, caveat): + """Validate caveats relating to authentication.""" + # TODO: Decide on caveat language and implement authentication caveats + # here (things like not_before, not_after). return True def _auth_callback(self, userid, request): return self._authenticate(userid, request) -class IgnoreThirdPartyCaveats(ThirdPartyCaveatVerifierDelegate): - def verify_third_party_caveat(self, *args, **kwargs): - return True - - class BasicAuthAuthenticationPolicy(_BasicAuthAuthenticationPolicy): def unauthenticated_userid(self, request): # If we're calling into this API on a request, then we want to register @@ -140,3 +125,54 @@ def unauthenticated_userid(self, request): # Dispatch to the real SessionAuthenticationPolicy return super().unauthenticated_userid(request) + + +@implementer(IAuthorizationPolicy) +class AccountTokenAuthorizationPolicy: + def __init__(self, policy): + self.policy = policy + + def permits(self, context, principals, permission): + # The Pyramid API doesn't let us access the request here, so we have to + # pull it out of the thread local instead. + request = get_current_request() + + if request is None: + return Denied("No active request.") + + # See if we have a macaroon + account_token_service = request.find_service(IAccountTokenService, context=None) + + unverified_macaroon, account_token = ( + account_token_service.get_unverified_macaroon() + ) + + if unverified_macaroon is not None and account_token is not None: + # Validate authorization properties of the macaroon + try: + verifier = Verifier() + verifier.third_party_caveat_verifier_delegate = ( + PassThroughThirdPartyCaveats() + ) + + verifier.satisfy_general( + self._validate_first_party_caveat_authorization + ) + + verifier.verify(unverified_macaroon, account_token.secret) + + except MacaroonException: + return Denied("Invalid credentials") + + # The macaroon validated, so pass this on to the underlying + # Authorization policies. + return self.policy.permits(context, principals, permission) + + def principals_allowed_by_permission(self, context, permission): + return self.policy.principals_allowed_by_permission(context, permission) + + def _validate_first_party_caveat_authorization(self, caveat): + """Validate caveats relating to authorization.""" + # TODO: Decide on caveat language and implement authorization caveats + # here (things like project name, project version, filename, hash, ... + return True diff --git a/warehouse/accounts/interfaces.py b/warehouse/accounts/interfaces.py index bf6a4253d08a..b424257c13da 100644 --- a/warehouse/accounts/interfaces.py +++ b/warehouse/accounts/interfaces.py @@ -97,10 +97,21 @@ def is_disabled(user_id): (IsDisabled: bool, Reason: Optional[DisableReason]) """ - def find_userid_by_account_token(account_token_id): + +class IAccountTokenService(Interface): + def get_unverified_macaroon(): + """ + Extract macaroon from request. Return macaroon with matching account token. + """ + + def get_account_token(account_token_id): + """ + Get the account token object associated with an id. + """ + + def update_last_used(account_token_id): """ - Find the unique user identifier for the given token_id or None if there - is no matching user. + Update when account token was last used to now. """ def get_tokens_by_username(username): @@ -108,6 +119,16 @@ def get_tokens_by_username(username): Returns tokens for the specified username. """ + def create_token(username, description): + """ + Creates a token for the specified username. + """ + + def delete_token(account_token_id, username): + """ + Deletes a token with the given id (if the specified username matches). + """ + class ITokenService(Interface): def dumps(data): diff --git a/warehouse/accounts/models.py b/warehouse/accounts/models.py index abca6489cbe2..305b44d331b8 100644 --- a/warehouse/accounts/models.py +++ b/warehouse/accounts/models.py @@ -108,6 +108,8 @@ class AccountToken(db.Model): # and will be used to distinguish this token from others which # may otherwise be equivalent. + secret = Column(String(length=100)) + username = Column( CIText, ForeignKey("accounts_user.username", onupdate="CASCADE", ondelete="CASCADE"), diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index 110ae91c2646..0920811e17c4 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -10,12 +10,14 @@ # See the License for the specific language governing permissions and # limitations under the License. +import base64 import collections import datetime import functools import hashlib import hmac import logging +import os import posixpath import urllib.parse import uuid @@ -23,11 +25,14 @@ import requests from passlib.context import CryptContext +from pymacaroons import Macaroon, Verifier +from pymacaroons.exceptions import MacaroonDeserializationException from sqlalchemy.orm.exc import NoResultFound from zope.interface import implementer from warehouse.accounts.interfaces import ( IUserService, + IAccountTokenService, ITokenService, IPasswordBreachedService, TokenExpired, @@ -237,32 +242,106 @@ def is_disabled(self, user_id): else: return (True, user.disabled_for) - def find_userid_by_account_token(self, account_token_id): - # Look up user from token_id + +@implementer(IAccountTokenService) +class AccountTokenService(object): + def __init__(self, headers, session): + self.headers = headers + self.db = session + + def get_unverified_macaroon(self): + authorization_header = self.headers.get("Authorization") + + if not authorization_header or " " not in authorization_header: + return (None, None) + + auth_type, credential = authorization_header.split(" ") + + if auth_type.lower() != "macaroon": + return (None, None) + + try: + unverified_macaroon = Macaroon.deserialize(credential) + except MacaroonDeserializationException: + return (None, None) + + account_token = self.get_account_token(unverified_macaroon.identifier) + + if account_token is None: + return (None, None) + + return (unverified_macaroon, account_token) + + def get_account_token(self, account_token_id): try: account_token = ( self.db.query(AccountToken) .filter(AccountToken.id == account_token_id) + .filter(AccountToken.is_active == True) .one() ) except NoResultFound: return None - # Update that token was used + return account_token + + def update_last_used(self, account_token_id): self.db.query(AccountToken).filter(AccountToken.id == account_token_id).update( values={"last_used": datetime.datetime.utcnow()} ) self.db.flush() - return self.find_userid(account_token.username) - def get_tokens_by_username(self, username): return ( - self.db.query(AccountToken).filter(AccountToken.username == username).all() + self.db.query(AccountToken) + .filter(AccountToken.username == username) + .filter(AccountToken.is_active == True) + .all() + ) + + def create_token(self, username, description): + secret = os.urandom(72) + + account_token = AccountToken( + secret=base64.b64encode(secret).decode("utf8"), + username=username, + description=description, ) + self.db.add(account_token) + self.db.flush() + + macaroon = Macaroon( + location="pypi.org", + identifier=str(account_token.id), + key=account_token.secret, + ) + + return macaroon.serialize() + + def delete_token(self, account_token_id, username): + try: + account_token = ( + self.db.query(AccountToken) + .filter( + AccountToken.id == account_token_id, + AccountToken.username == username, + ) + .one() + ) + + self.db.delete(account_token) + return True + + except NoResultFound: + return False + + +def database_account_token_factory(context, request): + return AccountTokenService(request.headers, request.db) + @implementer(ITokenService) class TokenService: diff --git a/warehouse/config.py b/warehouse/config.py index 6c707b85e743..870b1ceaf8ae 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -19,7 +19,7 @@ from pyramid import renderers from pyramid.config import Configurator as _Configurator from pyramid.response import Response -from pyramid.security import Allow, Authenticated +from pyramid.security import Allow, Authenticated, Everyone from pyramid.tweens import EXCVIEW from pyramid_rpc.xmlrpc import XMLRPCRenderer @@ -55,7 +55,11 @@ class RootFactory: __parent__ = None __name__ = None - __acl__ = [(Allow, "group:admins", "admin"), (Allow, Authenticated, "manage:user")] + __acl__ = [ + (Allow, "group:admins", "admin"), + (Allow, Authenticated, "manage:user"), + (Allow, Everyone, "check_macaroon_authorization"), + ] def __init__(self, request): pass @@ -189,8 +193,6 @@ def configure(settings=None): maybe_set_compound(settings, "mail", "backend", "MAIL_BACKEND") maybe_set_compound(settings, "metrics", "backend", "METRICS_BACKEND") maybe_set_compound(settings, "breached_passwords", "backend", "BREACHED_PASSWORDS") - maybe_set(settings, "account_token.secret", "ACCOUNT_TOKEN_SECRET") - maybe_set(settings, "account_token.id", "ACCOUNT_TOKEN_PUBLIC_ID") # Add the settings we use when the environment is set to development. if settings["warehouse.env"] == Environment.development: diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index f83f279bcf57..68181a1da169 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -699,6 +699,7 @@ def validate_no_deprecated_classifiers(form, field): uses_session=True, require_csrf=False, require_methods=["POST"], + permission="check_macaroon_authorization", ) def file_upload(request): # If we're in read-only mode, let upload clients know diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 0dedae0b8d10..47170702fecc 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -12,14 +12,17 @@ from collections import defaultdict -from pymacaroons import Macaroon from pyramid.httpexceptions import HTTPSeeOther from pyramid.view import view_config, view_defaults from sqlalchemy import func from sqlalchemy.orm.exc import NoResultFound -from warehouse.accounts.interfaces import IUserService, IPasswordBreachedService -from warehouse.accounts.models import AccountToken, Email, User +from warehouse.accounts.interfaces import ( + IUserService, + IAccountTokenService, + IPasswordBreachedService, +) +from warehouse.accounts.models import Email, User from warehouse.accounts.views import logout from warehouse.email import ( send_account_deletion_email, @@ -79,6 +82,9 @@ class ManageAccountViews: def __init__(self, request): self.request = request self.user_service = request.find_service(IUserService, context=None) + self.account_token_service = request.find_service( + IAccountTokenService, context=None + ) self.breach_service = request.find_service( IPasswordBreachedService, context=None ) @@ -90,7 +96,9 @@ def active_projects(self): @property def active_tokens(self): """ Return all active tokens """ - return self.user_service.get_tokens_by_username(self.request.user.username) + return self.account_token_service.get_tokens_by_username( + self.request.user.username + ) @property def default_response(self): @@ -227,23 +235,13 @@ def add_new_account_token(self): form = AccountTokenForm(**self.request.POST) if form.validate(): - account_token = AccountToken( - username=self.request.user.username, description=form.description.data - ) - self.request.db.add(account_token) - self.request.db.flush() - - macaroon = Macaroon( - location="pypi.org", - identifier=self.request.registry.settings["account_token.id"], - key=self.request.registry.settings["account_token.secret"], + serialized_macaroon = self.account_token_service.create_token( + self.request.user.username, form.description.data ) - macaroon.add_first_party_caveat(f"id: {account_token.id}") - self.request.session.flash( "Here is your account token, save it in a safe place: " - f"{macaroon.serialize()}", + f"{serialized_macaroon}", queue="success", ) @@ -253,21 +251,13 @@ def add_new_account_token(self): def delete_account_token(self): account_token_id = self.request.params.get("account_token_id") - try: - account_token = ( - self.request.db.query(AccountToken) - .filter( - AccountToken.id == account_token_id, - AccountToken.username == self.request.user.username, - ) - .one() - ) - - self.request.db.delete(account_token) + success = self.account_token_service.delete_token( + account_token_id, self.request.user.username + ) + if success: self.request.session.flash("Account token deleted", queue="success") - - except NoResultFound: + else: self.request.session.flash("Account token not found", queue="error") return self.default_response diff --git a/warehouse/migrations/versions/ac7073f2f9f2_add_accounts_token_table_to_store_api_.py b/warehouse/migrations/versions/6230118da311_adding_accounts_token_table.py similarity index 90% rename from warehouse/migrations/versions/ac7073f2f9f2_add_accounts_token_table_to_store_api_.py rename to warehouse/migrations/versions/6230118da311_adding_accounts_token_table.py index c7bbef5cbcff..155e348dc289 100644 --- a/warehouse/migrations/versions/ac7073f2f9f2_add_accounts_token_table_to_store_api_.py +++ b/warehouse/migrations/versions/6230118da311_adding_accounts_token_table.py @@ -10,11 +10,11 @@ # See the License for the specific language governing permissions and # limitations under the License. """ -add accounts_token table to store API key state +adding accounts_token table -Revision ID: ac7073f2f9f2 -Revises: e82c3a017d60 -Create Date: 2018-08-22 12:48:48.526328 +Revision ID: 6230118da311 +Revises: eeb23d9b4d00 +Create Date: 2018-11-09 21:47:21.174973 """ from alembic import op @@ -22,8 +22,8 @@ import sqlalchemy as sa from sqlalchemy.dialects import postgresql -revision = "ac7073f2f9f2" -down_revision = "e82c3a017d60" +revision = "6230118da311" +down_revision = "eeb23d9b4d00" # Note: It is VERY important to ensure that a migration does not lock for a # long period of time and to ensure that each individual migration does @@ -45,6 +45,7 @@ def upgrade(): server_default=sa.text("gen_random_uuid()"), nullable=False, ), + sa.Column("secret", sa.String(length=100), nullable=True), sa.Column("username", citext.CIText(), nullable=True), sa.Column("description", sa.String(length=100), nullable=True), sa.Column("is_active", sa.Boolean(), server_default="TRUE", nullable=True),