launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29417
[Merge] ~cjwatson/launchpad:personal-access-token-git into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:personal-access-token-git into launchpad:master.
Commit message:
Allow authenticating to git using access tokens
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1995535 in Launchpad itself: "Support personal access tokens for git HTTPS authentication"
https://bugs.launchpad.net/launchpad/+bug/1995535
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/433538
We added personal access tokens last year for some initial webservice API use cases, and with the intention of eventually extending them for git authentication as well, but never quite got round to the latter. This commit adds a new `repository:pull` scope to go with the previously-unimplemented `repository:push` scope, and implements both of those for git authentication over HTTPS.
The way that the git XML-RPC endpoint works means that we have to do this in a few different places: the initial entry point for HTTPS is `authenticateWithPassword`, which just verifies that the username and access token match and returns some information about them which turnip sends along with later calls. Those later calls (`translatePath`, `checkRefPermissions`, etc., which can only be called after successful authentication) then each need to handle more specific checks on the access token, such as whether it's for the correct repository and whether it permits pulling or pushing as appropriate. Fortunately, the same general idea was already in place for macaroon authentication, so for the most part we can just extend that.
This introduces the novel-to-Launchpad feature of a token that permits pushing but not pulling. While I don't have a specific use case for this, I didn't think it was wise to just have any token allow pulling (since the `repository:build_status` scope currently only allows using some specific webservice API endpoints), and it seemed clearer to add a specific scope than to have `repository:push` imply `repository:pull`. I've made sure that this works correctly with a real git client.
Once this is deployed to production, we can switch `snapcraft remote-build` over to use this instead of macaroons, and then begin the process of deprecating user macaroons for git.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:personal-access-token-git into launchpad:master.
diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
index 36a19eb..cbec524 100644
--- a/lib/lp/code/xmlrpc/git.py
+++ b/lib/lp/code/xmlrpc/git.py
@@ -55,8 +55,15 @@ from lp.registry.errors import InvalidName, NoSuchSourcePackageName
from lp.registry.interfaces.person import IPersonSet, NoSuchPerson
from lp.registry.interfaces.product import InvalidProductName, NoSuchProduct
from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
+from lp.services.auth.enums import AccessTokenScope
+from lp.services.auth.interfaces import IAccessTokenSet
from lp.services.features import getFeatureFlag
-from lp.services.macaroons.interfaces import NO_USER, IMacaroonIssuer
+from lp.services.identity.interfaces.account import AccountStatus
+from lp.services.macaroons.interfaces import (
+ NO_USER,
+ IMacaroonIssuer,
+ IMacaroonVerificationResult,
+)
from lp.services.webapp import LaunchpadXMLRPCView, canonical_url
from lp.services.webapp.authorization import check_permission
from lp.services.webapp.errorlog import ScriptRequest
@@ -107,6 +114,28 @@ class GitLoggerAdapter(logging.LoggerAdapter):
return msg, kwargs
+@implementer(IMacaroonVerificationResult)
+class AccessTokenVerificationResult:
+ def __init__(self, token):
+ self.token = token
+
+ @property
+ def issuer_name(self):
+ return None
+
+ @property
+ def user(self):
+ return self.token.owner
+
+ @property
+ def can_pull(self):
+ return AccessTokenScope.REPOSITORY_PULL in self.token.scopes
+
+ @property
+ def can_push(self):
+ return AccessTokenScope.REPOSITORY_PUSH in self.token.scopes
+
+
@implementer(IGitAPI)
class GitAPI(LaunchpadXMLRPCView):
"""See `IGitAPI`."""
@@ -141,27 +170,52 @@ class GitAPI(LaunchpadXMLRPCView):
raise faults.Unauthorized()
return verified
+ def _verifyAccessToken(
+ self, user, secret=None, token_id=None, repository=None
+ ):
+ access_token_set = removeSecurityProxy(getUtility(IAccessTokenSet))
+ if secret is not None:
+ assert token_id is None
+ access_token = access_token_set.getBySecret(secret)
+ else:
+ assert token_id is not None
+ access_token = access_token_set.getByID(token_id)
+ if access_token is None:
+ return None
+ if (
+ access_token.is_expired
+ or access_token.owner != user
+ or access_token.owner.account_status != AccountStatus.ACTIVE
+ ):
+ raise faults.Unauthorized()
+ if repository is not None and access_token.target != repository:
+ raise faults.Unauthorized()
+ access_token.updateLastUsed()
+ return AccessTokenVerificationResult(access_token)
+
def _verifyAuthParams(self, requester, repository, auth_params):
"""Verify authentication parameters in the context of a repository.
There are several possibilities:
- * Anonymous authentication with no macaroon. We do no additional
- checks here.
- * Anonymous authentication with a macaroon. This is forbidden.
- * User authentication with no macaroon. We can only get here if
- something else has already verified user authentication (SSH with
- a key checked against the authserver, or
+ * Anonymous authentication with no macaroon or access token. We do
+ no additional checks here.
+ * Anonymous authentication with a macaroon or access token. This
+ is forbidden.
+ * User authentication with no macaroon or access token. We can
+ only get here if something else has already verified user
+ authentication (SSH with a key checked against the authserver, or
`authenticateWithPassword`); we do no additional checks beyond
that.
- * User authentication with a macaroon. As above, we can only get
- here if something else has already verified user authentication.
- In this case, the macaroon is required to match the requester,
- and constrains their permissions.
+ * User authentication with a macaroon or access token. As above,
+ we can only get here if something else has already verified user
+ authentication. In this case, the macaroon or access token is
+ required to match the requester, and constrains their
+ permissions.
* Internal-services authentication with a macaroon. In this case,
we require that the macaroon does not identify a user.
- * Internal-services authentication with no macaroon. This is
- forbidden.
+ * Internal-services authentication with no macaroon or with an
+ access token. This is forbidden.
:param requester: The logged-in `IPerson`, `LAUNCHPAD_SERVICES`, or
None for anonymous access.
@@ -198,6 +252,34 @@ class GitAPI(LaunchpadXMLRPCView):
# Internal services must authenticate using a macaroon.
raise faults.Unauthorized()
+ access_token_id = auth_params.get("access-token")
+ if access_token_id is not None:
+ if requester is None:
+ raise faults.Unauthorized()
+ verified = self._verifyAccessToken(
+ requester, token_id=access_token_id, repository=repository
+ )
+ if verified is None:
+ # Access token authentication failed. Don't fall back to
+ # the requester's permission, since an access token is
+ # typically supposed to convey additional constraints.
+ # Instead, just return "authorisation required", thus
+ # preventing probing for the existence of repositories
+ # without presenting valid credentials.
+ raise faults.Unauthorized()
+ # _verifyAccessToken checks that the access token's owner
+ # matches the requester.
+ return verified
+
+ def _isReadable(self, requester, repository, verified):
+ # Most authentication methods allow readability.
+ readable = True
+ if isinstance(verified, AccessTokenVerificationResult):
+ # Access token authentication only allows readability with the
+ # "repository:pull" scope.
+ readable = verified.can_pull
+ return readable
+
def _isWritable(self, requester, repository, verified):
writable = False
naked_repository = removeSecurityProxy(repository)
@@ -207,13 +289,21 @@ class GitAPI(LaunchpadXMLRPCView):
# macaroon that specifically grants access to this repository.
# This is only permitted for macaroons not bound to a user.
writable = _can_internal_issuer_write(verified)
+ elif (
+ isinstance(verified, AccessTokenVerificationResult)
+ and not verified.can_push
+ ):
+ # The user authenticated with an access token without the
+ # "repository:push" scope, so pushing isn't allowed no matter
+ # what permissions they might ordinarily have.
+ writable = False
+ elif repository.repository_type != GitRepositoryType.HOSTED:
+ # Normal users can never push to non-hosted repositories.
+ writable = False
else:
# This isn't an authorised internal service, so perform normal
# user authorisation.
- writable = (
- repository.repository_type == GitRepositoryType.HOSTED
- and check_permission("launchpad.Edit", repository)
- )
+ writable = check_permission("launchpad.Edit", repository)
if not writable:
grants = naked_repository.findRuleGrantsByGrantee(requester)
if not grants.is_empty():
@@ -256,10 +346,12 @@ class GitAPI(LaunchpadXMLRPCView):
except Unauthorized:
return naked_repository, None
private = repository.private
+ readable = self._isReadable(requester, repository, verified)
writable = self._isWritable(requester, repository, verified)
return naked_repository, {
"path": hosting_path,
+ "readable": readable,
"writable": writable,
"trailing": extra_path,
"private": private,
@@ -445,6 +537,8 @@ class GitAPI(LaunchpadXMLRPCView):
}
if result is None:
raise faults.GitRepositoryNotFound(path)
+ if permission == "read" and not result["readable"]:
+ raise faults.PermissionDenied()
if permission != "read" and not result["writable"]:
raise faults.PermissionDenied()
return result
@@ -610,6 +704,9 @@ class GitAPI(LaunchpadXMLRPCView):
if username and username != LAUNCHPAD_SERVICES
else None
)
+ verified = self._verifyAccessToken(user, secret=password)
+ if verified is not None:
+ return {"access-token": verified.token.id, "uid": user.id}
verified = self._verifyMacaroon(password, user=user)
if verified:
auth_params = {"macaroon": password}
@@ -681,6 +778,16 @@ class GitAPI(LaunchpadXMLRPCView):
# as an anonymous repository owner. This is only permitted
# for selected macaroon issuers.
requester = GitGranteeType.REPOSITORY_OWNER
+ elif (
+ isinstance(verified, AccessTokenVerificationResult)
+ and not verified.can_push
+ ):
+ # The user authenticated with an access token without the
+ # "repository:push" scope, so pushing isn't allowed no
+ # matter what permissions they might ordinarily have. (This
+ # should already have been checked earlier, but it doesn't
+ # hurt to be careful about it here as well.)
+ raise faults.Unauthorized()
except faults.Unauthorized:
# XXX cjwatson 2019-05-09: It would be simpler to just raise
# this directly, but turnip won't handle it very gracefully at
@@ -761,12 +868,17 @@ class GitAPI(LaunchpadXMLRPCView):
verified = self._verifyAuthParams(requester, repository, auth_params)
if verified is not None and verified.user is NO_USER:
- # For internal-services authentication, we check if its using a
+ # For internal-services authentication, we check if it's using a
# suitable macaroon that specifically grants access to this
# repository. This is only permitted for macaroons not bound to
# a user.
if not _can_internal_issuer_write(verified):
raise faults.Unauthorized()
+ elif isinstance(verified, AccessTokenVerificationResult):
+ # Access tokens can currently only be issued for an existing
+ # repository, so it doesn't make sense to allow using one for
+ # creating a repository.
+ raise faults.Unauthorized()
else:
# This checks `requester` against `repo.registrant` because the
# requester should be the only user able to confirm/abort
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index 96610c3..c91d982 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -6,7 +6,7 @@
import hashlib
import uuid
import xmlrpc.client
-from datetime import datetime
+from datetime import datetime, timedelta
from urllib.parse import quote
import pytz
@@ -50,8 +50,10 @@ from lp.code.model.gitjob import GitRefScanJob
from lp.code.tests.helpers import GitHostingFixture
from lp.code.xmlrpc.git import GIT_ASYNC_CREATE_REPO
from lp.registry.enums import TeamMembershipPolicy
+from lp.services.auth.enums import AccessTokenScope
from lp.services.config import config
from lp.services.features.testing import FeatureFixture
+from lp.services.identity.interfaces.account import AccountStatus
from lp.services.job.runner import JobRunner
from lp.services.macaroons.interfaces import (
NO_USER,
@@ -77,7 +79,12 @@ from lp.testing.xmlrpc import XMLRPCTestTransport
from lp.xmlrpc import faults
-def _make_auth_params(requester, can_authenticate=False, macaroon_raw=None):
+def _make_auth_params(
+ requester,
+ can_authenticate=False,
+ macaroon_raw=None,
+ access_token_id=None,
+):
auth_params = {
"can-authenticate": can_authenticate,
"request-id": str(uuid.uuid4()),
@@ -88,6 +95,8 @@ def _make_auth_params(requester, can_authenticate=False, macaroon_raw=None):
auth_params["uid"] = requester.id
if macaroon_raw is not None:
auth_params["macaroon"] = macaroon_raw
+ if access_token_id is not None:
+ auth_params["access-token"] = access_token_id
return auth_params
@@ -216,13 +225,11 @@ class TestGitAPIMixin:
path,
permission="read",
can_authenticate=False,
- macaroon_raw=None,
+ **auth_kwargs
):
"""Assert that the given path cannot be translated."""
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
self.assertFault(
@@ -241,13 +248,11 @@ class TestGitAPIMixin:
message="Permission denied.",
permission="read",
can_authenticate=False,
- macaroon_raw=None,
+ **auth_kwargs
):
"""Assert that looking at the given path returns PermissionDenied."""
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
self.assertFault(
@@ -266,13 +271,11 @@ class TestGitAPIMixin:
message="Authorisation required.",
permission="read",
can_authenticate=False,
- macaroon_raw=None,
+ **auth_kwargs
):
"""Assert that looking at the given path returns Unauthorized."""
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
self.assertFault(
@@ -374,17 +377,16 @@ class TestGitAPIMixin:
requester,
path,
repository,
- writable,
permission="read",
can_authenticate=False,
- macaroon_raw=None,
+ readable=True,
+ writable=False,
trailing="",
private=False,
+ **auth_kwargs
):
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
translation = self.assertDoesNotFault(
@@ -394,6 +396,7 @@ class TestGitAPIMixin:
self.assertEqual(
{
"path": removeSecurityProxy(repository).getInternalPath(),
+ "readable": readable,
"writable": writable,
"trailing": trailing,
"private": private,
@@ -402,11 +405,7 @@ class TestGitAPIMixin:
)
def assertConfirmsRepoCreation(
- self,
- requester,
- git_repository,
- can_authenticate=True,
- macaroon_raw=None,
+ self, requester, git_repository, can_authenticate=True, **auth_kwargs
):
# Puts some refs in git hosting, to make sure we scanned them.
sha1 = lambda x: hashlib.sha1(x).hexdigest()
@@ -430,9 +429,7 @@ class TestGitAPIMixin:
)
translated_path = git_repository.getInternalPath()
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
result = self.assertDoesNotFault(
@@ -474,13 +471,11 @@ class TestGitAPIMixin:
requester,
git_repository,
can_authenticate=True,
- macaroon_raw=None,
+ **auth_kwargs
):
translated_path = git_repository.getInternalPath()
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
original_status = git_repository.status
@@ -497,29 +492,23 @@ class TestGitAPIMixin:
self.assertEqual(original_status, git_repository.status)
def assertConfirmRepoCreationUnauthorized(
- self,
- requester,
- git_repository,
- can_authenticate=True,
- macaroon_raw=None,
+ self, requester, git_repository, can_authenticate=True, **auth_kwargs
):
failure = faults.Unauthorized
self.assertConfirmRepoCreationFails(
- failure, requester, git_repository, can_authenticate, macaroon_raw
+ failure,
+ requester,
+ git_repository,
+ can_authenticate=can_authenticate,
+ **auth_kwargs,
)
def assertAbortsRepoCreation(
- self,
- requester,
- git_repository,
- can_authenticate=True,
- macaroon_raw=None,
+ self, requester, git_repository, can_authenticate=True, **auth_kwargs
):
translated_path = git_repository.getInternalPath()
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
result = self.assertDoesNotFault(
@@ -537,13 +526,11 @@ class TestGitAPIMixin:
requester,
git_repository,
can_authenticate=True,
- macaroon_raw=None,
+ **auth_kwargs
):
translated_path = git_repository.getInternalPath()
auth_params = _make_auth_params(
- requester,
- can_authenticate=can_authenticate,
- macaroon_raw=macaroon_raw,
+ requester, can_authenticate=can_authenticate, **auth_kwargs
)
request_id = auth_params["request-id"]
original_status = git_repository.status
@@ -565,15 +552,15 @@ class TestGitAPIMixin:
self.assertEqual(original_status, git_repository.status)
def assertAbortRepoCreationUnauthorized(
- self,
- requester,
- git_repository,
- can_authenticate=True,
- macaroon_raw=None,
+ self, requester, git_repository, can_authenticate=True, **auth_kwargs
):
failure = faults.Unauthorized
self.assertAbortRepoCreationFails(
- failure, requester, git_repository, can_authenticate, macaroon_raw
+ failure,
+ requester,
+ git_repository,
+ can_authenticate=can_authenticate,
+ **auth_kwargs,
)
def assertCreates(
@@ -601,6 +588,7 @@ class TestGitAPIMixin:
cloned_from = repository.getClonedFrom()
expected_translation = {
"path": repository.getInternalPath(),
+ "readable": True,
"writable": True,
"trailing": "",
"private": private,
@@ -654,9 +642,9 @@ class TestGitAPIMixin:
self.assertEqual(0, self.hosting_fixture.create.call_count)
def assertHasRefPermissions(
- self, requester, repository, ref_paths, permissions, macaroon_raw=None
+ self, requester, repository, ref_paths, permissions, **auth_kwargs
):
- auth_params = _make_auth_params(requester, macaroon_raw=macaroon_raw)
+ auth_params = _make_auth_params(requester, **auth_kwargs)
request_id = auth_params["request-id"]
translated_path = removeSecurityProxy(repository).getInternalPath()
ref_paths = [xmlrpc.client.Binary(ref_path) for ref_path in ref_paths]
@@ -706,7 +694,9 @@ class TestGitAPIMixin:
)
)
path = "/%s" % repository.unique_name
- self.assertTranslates(requester, path, repository, True, private=True)
+ self.assertTranslates(
+ requester, path, repository, writable=True, private=True
+ )
def test_translatePath_cannot_see_private_repository(self):
requester = self.factory.makePerson()
@@ -752,7 +742,7 @@ class TestGitAPIMixin:
team = self.factory.makeTeam(self.factory.makePerson())
repository = self.factory.makeGitRepository(owner=team)
path = "/%s" % repository.unique_name
- self.assertTranslates(requester, path, repository, False)
+ self.assertTranslates(requester, path, repository, writable=False)
self.assertPermissionDenied(requester, path, permission="write")
def test_translatePath_imported(self):
@@ -761,7 +751,7 @@ class TestGitAPIMixin:
owner=requester, repository_type=GitRepositoryType.IMPORTED
)
path = "/%s" % repository.unique_name
- self.assertTranslates(requester, path, repository, False)
+ self.assertTranslates(requester, path, repository, writable=False)
self.assertPermissionDenied(requester, path, permission="write")
def test_translatePath_create_personal_team_denied(self):
@@ -851,7 +841,7 @@ class TestGitAPIMixin:
)
path = "/%s" % repository.unique_name
self.assertTranslates(
- other_person, path, repository, True, private=False
+ other_person, path, repository, writable=True, private=False
)
def test_translatePath_grant_but_no_access(self):
@@ -867,7 +857,7 @@ class TestGitAPIMixin:
)
path = "/%s" % repository.unique_name
self.assertTranslates(
- other_person, path, repository, False, private=False
+ other_person, path, repository, writable=False, private=False
)
def test_translatePath_grant_to_other_private(self):
@@ -1388,6 +1378,24 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
requester, repository, macaroon_raw=macaroon.serialize()
)
+ def test_confirm_git_repository_access_token(self):
+ # An access token cannot be used to authorize confirming repository
+ # creation.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(
+ owner=requester, status=GitRepositoryStatus.CREATING
+ )
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
+ )
+ self.assertConfirmRepoCreationUnauthorized(
+ requester,
+ repository,
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
def test_abort_repo_creation(self):
requester = self.factory.makePerson()
repo = self.factory.makeGitRepository(owner=requester)
@@ -1599,6 +1607,24 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
requester, repository, macaroon_raw=macaroon.serialize()
)
+ def test_abort_git_repository_access_token(self):
+ # An access token cannot be used to authorize aborting repository
+ # creation.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(
+ owner=requester, status=GitRepositoryStatus.CREATING
+ )
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
+ )
+ self.assertAbortRepoCreationUnauthorized(
+ requester,
+ repository,
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
def test_abort_git_repository_creation_of_non_existing_repository(self):
owner = self.factory.makePerson()
repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner))
@@ -1619,27 +1645,25 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
requester = self.factory.makePerson()
repository = self.factory.makeGitRepository()
path = "/%s" % repository.unique_name
- self.assertTranslates(requester, path, repository, False)
+ self.assertTranslates(requester, path, repository)
def test_translatePath_repository_with_no_leading_slash(self):
requester = self.factory.makePerson()
repository = self.factory.makeGitRepository()
path = repository.unique_name
- self.assertTranslates(requester, path, repository, False)
+ self.assertTranslates(requester, path, repository)
def test_translatePath_repository_with_trailing_slash(self):
requester = self.factory.makePerson()
repository = self.factory.makeGitRepository()
path = "/%s/" % repository.unique_name
- self.assertTranslates(requester, path, repository, False)
+ self.assertTranslates(requester, path, repository)
def test_translatePath_repository_with_trailing_segments(self):
requester = self.factory.makePerson()
repository = self.factory.makeGitRepository()
path = "/%s/foo/bar" % repository.unique_name
- self.assertTranslates(
- requester, path, repository, False, trailing="foo/bar"
- )
+ self.assertTranslates(requester, path, repository, trailing="foo/bar")
def test_translatePath_no_such_repository(self):
requester = self.factory.makePerson()
@@ -1657,10 +1681,10 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
repository = self.factory.makeGitRepository()
path = "/%s" % repository.unique_name
self.assertTranslates(
- None, path, repository, False, can_authenticate=False
+ None, path, repository, can_authenticate=False, writable=False
)
self.assertTranslates(
- None, path, repository, False, can_authenticate=True
+ None, path, repository, can_authenticate=True, writable=False
)
def test_translatePath_owned(self):
@@ -1668,7 +1692,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
repository = self.factory.makeGitRepository(owner=requester)
path = "/%s" % repository.unique_name
self.assertTranslates(
- requester, path, repository, True, permission="write"
+ requester, path, repository, permission="write", writable=True
)
def test_translatePath_team_owned(self):
@@ -1677,7 +1701,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
repository = self.factory.makeGitRepository(owner=team)
path = "/%s" % repository.unique_name
self.assertTranslates(
- requester, path, repository, True, permission="write"
+ requester, path, repository, permission="write", writable=True
)
def test_translatePath_shortened_path(self):
@@ -1689,7 +1713,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
repository.target, repository
)
path = "/%s" % repository.target.name
- self.assertTranslates(requester, path, repository, False)
+ self.assertTranslates(requester, path, repository)
def test_translatePath_create_project_async(self):
# translatePath creates a project repository that doesn't exist, if
@@ -2198,9 +2222,9 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
LAUNCHPAD_SERVICES,
path,
code_imports[0].git_repository,
- True,
permission="write",
macaroon_raw=macaroons[0].serialize(),
+ writable=True,
)
self.assertUnauthorized(
LAUNCHPAD_SERVICES,
@@ -2270,10 +2294,10 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
LAUNCHPAD_SERVICES,
path,
code_imports[0].git_repository,
- True,
permission="write",
macaroon_raw=macaroons[0].serialize(),
private=True,
+ writable=True,
)
self.assertUnauthorized(
LAUNCHPAD_SERVICES,
@@ -2304,6 +2328,42 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
macaroon_raw=macaroons[0].serialize(),
)
+ def test_translatePath_private_code_import_access_token(self):
+ # An access token can only allow pulling from a code import
+ # repository, not pushing to it.
+ code_import = self.factory.makeCodeImport(
+ target_rcs_type=TargetRevisionControlSystems.GIT
+ )
+ repository = code_import.git_repository
+ owner = repository.owner
+ removeSecurityProxy(repository).transitionToInformationType(
+ InformationType.PRIVATESECURITY, owner
+ )
+ with person_logged_in(owner):
+ path = "/%s" % repository.unique_name
+ _, token = self.factory.makeAccessToken(
+ owner=owner,
+ target=repository,
+ scopes=[
+ AccessTokenScope.REPOSITORY_PULL,
+ AccessTokenScope.REPOSITORY_PUSH,
+ ],
+ )
+ self.assertTranslates(
+ owner,
+ path,
+ repository,
+ permission="read",
+ access_token_id=removeSecurityProxy(token).id,
+ private=True,
+ )
+ self.assertPermissionDenied(
+ owner,
+ path,
+ permission="write",
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
def test_translatePath_private_snap_build(self):
# A builder with a suitable macaroon can read from a repository
# associated with a running private snap build.
@@ -2342,10 +2402,10 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
LAUNCHPAD_SERVICES,
path,
repository,
- False,
permission="read",
macaroon_raw=macaroons[0].serialize(),
private=True,
+ writable=False,
)
self.assertUnauthorized(
LAUNCHPAD_SERVICES,
@@ -2420,10 +2480,10 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
LAUNCHPAD_SERVICES,
path,
repository,
- False,
permission="read",
macaroon_raw=macaroons[0].serialize(),
private=True,
+ writable=False,
)
self.assertUnauthorized(
LAUNCHPAD_SERVICES,
@@ -2487,9 +2547,9 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
requester,
paths[i],
repository,
- True,
permission="write",
macaroon_raw=macaroon.serialize(),
+ writable=True,
private=(i == 2),
)
else:
@@ -2548,8 +2608,8 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
requester,
path,
repository,
- False,
permission="read",
+ writable=False,
macaroon_raw=macaroon.serialize(),
)
for requester in unauthorized:
@@ -2561,6 +2621,150 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
macaroon_raw=macaroon.serialize(),
)
+ def test_translatePath_user_access_token_pull(self):
+ # A user with a suitable access token can pull from the
+ # corresponding repository, but not others, even if they own them.
+ requester = self.factory.makePerson()
+ repositories = [
+ self.factory.makeGitRepository(owner=requester) for _ in range(2)
+ ]
+ repositories.append(
+ self.factory.makeGitRepository(
+ owner=requester,
+ information_type=InformationType.PRIVATESECURITY,
+ )
+ )
+ tokens = []
+ with person_logged_in(requester):
+ for repository in repositories:
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PULL],
+ )
+ tokens.append(token)
+ paths = [
+ "/%s" % repository.unique_name for repository in repositories
+ ]
+ for i, repository in enumerate(repositories):
+ for j, token in enumerate(tokens):
+ login(ANONYMOUS)
+ if i == j:
+ self.assertTranslates(
+ requester,
+ paths[i],
+ repository,
+ permission="read",
+ access_token_id=removeSecurityProxy(token).id,
+ private=(i == 2),
+ )
+ else:
+ self.assertUnauthorized(
+ requester,
+ paths[i],
+ permission="read",
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
+ def test_translatePath_user_access_token_pull_wrong_scope(self):
+ # A user with an access token that does not have the repository:pull
+ # scope cannot pull from the corresponding repository.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS],
+ )
+ self.assertPermissionDenied(
+ requester,
+ "/%s" % repository.unique_name,
+ permission="read",
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
+ def test_translatePath_user_access_token_push(self):
+ # A user with a suitable access token can push to the corresponding
+ # repository, but not others, even if they own them.
+ requester = self.factory.makePerson()
+ repositories = [
+ self.factory.makeGitRepository(owner=requester) for _ in range(2)
+ ]
+ repositories.append(
+ self.factory.makeGitRepository(
+ owner=requester,
+ information_type=InformationType.PRIVATESECURITY,
+ )
+ )
+ tokens = []
+ with person_logged_in(requester):
+ for repository in repositories:
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
+ )
+ tokens.append(token)
+ paths = [
+ "/%s" % repository.unique_name for repository in repositories
+ ]
+ for i, repository in enumerate(repositories):
+ for j, token in enumerate(tokens):
+ login(ANONYMOUS)
+ if i == j:
+ self.assertTranslates(
+ requester,
+ paths[i],
+ repository,
+ permission="write",
+ access_token_id=removeSecurityProxy(token).id,
+ readable=False,
+ writable=True,
+ private=(i == 2),
+ )
+ else:
+ self.assertUnauthorized(
+ requester,
+ paths[i],
+ permission="write",
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
+ def test_translatePath_user_access_token_push_wrong_scope(self):
+ # A user with an access token that does not have the repository:push
+ # scope cannot push to the corresponding repository.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PULL],
+ )
+ self.assertPermissionDenied(
+ requester,
+ "/%s" % repository.unique_name,
+ permission="write",
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
+ def test_translatePath_user_access_token_nonexistent(self):
+ # Attempting to pass a nonexistent access token ID returns
+ # Unauthorized.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ self.assertUnauthorized(
+ requester,
+ "/%s" % repository.unique_name,
+ permission="read",
+ access_token_id=0,
+ )
+ self.assertUnauthorized(
+ requester,
+ "/%s" % repository.unique_name,
+ permission="write",
+ access_token_id=0,
+ )
+
def test_getMergeProposalURL_user(self):
# A merge proposal URL is returned by LP for a non-default branch
# pushed by a user that has their ordinary privileges on the
@@ -2675,6 +2879,50 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
auth_params,
)
+ def test_getMergeProposalURL_user_access_token(self):
+ # The merge proposal URL is returned by LP for a non-default branch
+ # pushed by a user with a suitable access token that has their
+ # ordinary privileges on the corresponding repository.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ self.factory.makeGitRefs(
+ repository=repository, paths=["refs/heads/main"]
+ )
+ removeSecurityProxy(repository).default_branch = "refs/heads/main"
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
+ )
+ auth_params = _make_auth_params(
+ requester, access_token_id=removeSecurityProxy(token).id
+ )
+ self.assertHasMergeProposalURL(repository, "branch", auth_params)
+
+ def test_getMergeProposalURL_user_access_token_wrong_repository(self):
+ # getMergeProposalURL refuses access tokens for a different
+ # repository.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ self.factory.makeGitRefs(
+ repository=repository, paths=["refs/heads/main"]
+ )
+ removeSecurityProxy(repository).default_branch = "refs/heads/main"
+ _, token = self.factory.makeAccessToken(
+ owner=requester, scopes=[AccessTokenScope.REPOSITORY_PUSH]
+ )
+ auth_params = _make_auth_params(
+ requester, access_token_id=removeSecurityProxy(token).id
+ )
+ self.assertFault(
+ faults.Unauthorized,
+ None,
+ "getMergeProposalURL",
+ repository.getInternalPath(),
+ "branch",
+ auth_params,
+ )
+
def test_getMergeProposalURL_code_import(self):
# A merge proposal URL from LP to Turnip is not returned for
# code import job as there is no User at the other end.
@@ -3005,6 +3253,32 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
auth_params,
)
+ def test_notify_set_repack_data_user_access_token(self):
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
+ )
+ auth_params = _make_auth_params(
+ requester, access_token_id=removeSecurityProxy(token).id
+ )
+ self.assertSetsRepackData(repository, auth_params)
+
+ def test_notify_set_repack_data_user_access_token_nonexistent(self):
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ auth_params = _make_auth_params(requester, access_token_id=0)
+ self.assertFault(
+ faults.Unauthorized,
+ None,
+ "notify",
+ repository.getInternalPath(),
+ {"loose_object_count": 5, "pack_count": 2},
+ auth_params,
+ )
+
def test_notify_set_repack_data_code_import(self):
# We set the repack data on a repository from a code import worker
# with a suitable macaroon.
@@ -3336,6 +3610,73 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
macaroon.serialize(),
)
+ def test_authenticateWithPassword_user_access_token(self):
+ # A user with a suitable access token can authenticate using it, in
+ # which case we return both the access token and the uid for use by
+ # later calls.
+ requester = self.factory.makePerson()
+ secret, token = self.factory.makeAccessToken(owner=requester)
+ self.assertIsNone(removeSecurityProxy(token).date_last_used)
+ self.assertEqual(
+ {
+ "access-token": removeSecurityProxy(token).id,
+ "uid": requester.id,
+ },
+ self.assertDoesNotFault(
+ None, "authenticateWithPassword", requester.name, secret
+ ),
+ )
+ self.assertIsNotNone(removeSecurityProxy(token).date_last_used)
+ for username in ("", "+launchpad-services", "nonexistent"):
+ self.assertFault(
+ faults.Unauthorized,
+ None,
+ "authenticateWithPassword",
+ username,
+ secret,
+ )
+
+ def test_authenticateWithPassword_user_access_token_expired(self):
+ # An expired access token is rejected.
+ requester = self.factory.makePerson()
+ secret, _ = self.factory.makeAccessToken(
+ owner=requester,
+ date_expires=datetime.now(pytz.UTC) - timedelta(days=1),
+ )
+ self.assertFault(
+ faults.Unauthorized,
+ None,
+ "authenticateWithPassword",
+ requester.name,
+ secret,
+ )
+
+ def test_authenticateWithPassword_user_access_token_wrong_user(self):
+ # An access token for a different user is rejected.
+ requester = self.factory.makePerson()
+ secret, _ = self.factory.makeAccessToken()
+ self.assertFault(
+ faults.Unauthorized,
+ None,
+ "authenticateWithPassword",
+ requester.name,
+ secret,
+ )
+
+ def test_authenticateWithPassword_user_access_token_inactive_account(self):
+ # An access token for an inactive user is rejected.
+ requester = self.factory.makePerson(
+ account_status=AccountStatus.SUSPENDED
+ )
+ secret, _ = self.factory.makeAccessToken(owner=requester)
+ self.assertFault(
+ faults.Unauthorized,
+ None,
+ "authenticateWithPassword",
+ requester.name,
+ secret,
+ )
+
def test_checkRefPermissions_code_import(self):
# A code import worker with a suitable macaroon has repository owner
# privileges on a repository associated with a running code import
@@ -3659,6 +4000,64 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
macaroon_raw=macaroon.serialize(),
)
+ def test_checkRefPermissions_user_access_token(self):
+ # A user with a suitable access token has their ordinary privileges
+ # on the corresponding repository, but not others, even if they own
+ # them.
+ requester = self.factory.makePerson()
+ repositories = [
+ self.factory.makeGitRepository(owner=requester) for _ in range(2)
+ ]
+ repositories.append(
+ self.factory.makeGitRepository(
+ owner=requester,
+ information_type=InformationType.PRIVATESECURITY,
+ )
+ )
+ ref_path = b"refs/heads/main"
+ tokens = []
+ with person_logged_in(requester):
+ for repository in repositories:
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
+ )
+ tokens.append(token)
+ for i, repository in enumerate(repositories):
+ for j, token in enumerate(tokens):
+ login(ANONYMOUS)
+ if i == j:
+ expected_permissions = ["create", "push", "force_push"]
+ else:
+ expected_permissions = []
+ self.assertHasRefPermissions(
+ requester,
+ repository,
+ [ref_path],
+ {ref_path: expected_permissions},
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
+ def test_checkRefPermissions_user_access_token_wrong_scope(self):
+ # A user with an access token that does not have the repository:push
+ # scope cannot push to any branch in the corresponding repository.
+ requester = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=requester)
+ _, token = self.factory.makeAccessToken(
+ owner=requester,
+ target=repository,
+ scopes=[AccessTokenScope.REPOSITORY_PULL],
+ )
+ ref_path = b"refs/heads/main"
+ self.assertHasRefPermissions(
+ requester,
+ repository,
+ [ref_path],
+ {ref_path: []},
+ access_token_id=removeSecurityProxy(token).id,
+ )
+
def assertUpdatesRepackStats(self, repo):
start_time = datetime.now(pytz.UTC)
self.assertIsNone(
diff --git a/lib/lp/services/auth/enums.py b/lib/lp/services/auth/enums.py
index 9ce4cbb..a2a6607 100644
--- a/lib/lp/services/auth/enums.py
+++ b/lib/lp/services/auth/enums.py
@@ -21,6 +21,14 @@ class AccessTokenScope(EnumeratedType):
"""
)
+ REPOSITORY_PULL = Item(
+ """
+ repository:pull
+
+ Can pull from a repository.
+ """
+ )
+
REPOSITORY_PUSH = Item(
"""
repository:push
diff --git a/lib/lp/services/auth/interfaces.py b/lib/lp/services/auth/interfaces.py
index 07e7c77..dd3315c 100644
--- a/lib/lp/services/auth/interfaces.py
+++ b/lib/lp/services/auth/interfaces.py
@@ -157,6 +157,12 @@ class IAccessTokenSet(Interface):
None.
"""
+ def getByID(token_id):
+ """Return the access token with this ID, or None.
+
+ :param token_id: An `AccessToken` ID.
+ """
+
def getBySecret(secret):
"""Return the access token with this secret, or None.
diff --git a/lib/lp/services/auth/model.py b/lib/lp/services/auth/model.py
index a629fcc..c436650 100644
--- a/lib/lp/services/auth/model.py
+++ b/lib/lp/services/auth/model.py
@@ -156,6 +156,10 @@ class AccessTokenSet:
store.add(token)
return token
+ def getByID(self, token_id):
+ """See `IAccessTokenSet`."""
+ return IStore(AccessToken).get(AccessToken, token_id)
+
def getBySecret(self, secret):
"""See `IAccessTokenSet`."""
return (
diff --git a/lib/lp/services/auth/tests/test_model.py b/lib/lp/services/auth/tests/test_model.py
index 14f32cc..0823994 100644
--- a/lib/lp/services/auth/tests/test_model.py
+++ b/lib/lp/services/auth/tests/test_model.py
@@ -198,6 +198,12 @@ class TestAccessTokenSet(TestCaseWithFactory):
),
)
+ def test_getByID(self):
+ secret, token = self.factory.makeAccessToken()
+ token_id = removeSecurityProxy(token).id
+ self.assertEqual(token, getUtility(IAccessTokenSet).getByID(token_id))
+ self.assertIsNone(getUtility(IAccessTokenSet).getByID(token_id + 1))
+
def test_getBySecret(self):
secret, token = self.factory.makeAccessToken()
self.assertEqual(
Follow ups
