Merge ~ines-almeida/launchpad:project-tokens/update-models into launchpad:master

Proposed by Ines Almeida
Status: Merged
Approved by: Ines Almeida
Approved revision: bd5a112e360f894e0e7d0bd03615ac0eb01f3d8f
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ines-almeida/launchpad:project-tokens/update-models
Merge into: launchpad:master
Prerequisite: ~ines-almeida/launchpad:project-tokens/refactor-access-token-tests
Diff against target: 915 lines (+542/-95)
7 files modified
lib/lp/code/xmlrpc/git.py (+4/-1)
lib/lp/code/xmlrpc/tests/test_git.py (+365/-61)
lib/lp/registry/model/person.py (+2/-5)
lib/lp/registry/model/product.py (+8/-3)
lib/lp/services/auth/interfaces.py (+9/-1)
lib/lp/services/auth/model.py (+52/-21)
lib/lp/services/auth/tests/test_model.py (+102/-3)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+451542@code.launchpad.net

Commit message

Update Access Token model and logic to enable Project scoped Access Tokens

This change introduces Projects as possible targets for Access Tokens similarly to what already existed for Git Repository, with the difference that a Project Access Token can be used to authenticate requests against any Git Repository within that Project.

Description of the change

Changes in this MP:
 - Added `project` and a field within the `AccessToken` model, enabling Access Tokens to be targeted at Projects (before only Git Repositories could be targets for Access Tokens)
 - Enabled using Project tokens to authenticate against requests similarly as Git Repository Access Tokens worked. Project Access Tokens can be used to authenticate against any of the Project's Git Repositories

Changes not in this MP:
 - No interfaces (UI and API) to add Project Access Tokens are exposed

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

You might want to rebase this onto master so that the CI lint check can pass.

This can't land until https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/451470 (or its successor) has landed on db-devel, has been deployed to production, and has been merged onto master.

review: Approve
9bae4b0... by Ines Almeida

Address MP comments

bd5a112... by Ines Almeida

Refactor AccessToken.findByTarget query and unit tests

Revision history for this message
Ines Almeida (ines-almeida) wrote :

This MP is ready to merge once its DB dependency is deployed to production

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
2index 3452fcd..7b450c0 100644
3--- a/lib/lp/code/xmlrpc/git.py
4+++ b/lib/lp/code/xmlrpc/git.py
5@@ -195,7 +195,10 @@ class GitAPI(LaunchpadXMLRPCView):
6 or access_token.owner.account_status != AccountStatus.ACTIVE
7 ):
8 raise faults.Unauthorized()
9- if repository is not None and access_token.target != repository:
10+ if repository is not None and (
11+ access_token.target != repository
12+ and access_token.target != repository.target
13+ ):
14 raise faults.Unauthorized()
15 access_token.updateLastUsed()
16 return AccessTokenVerificationResult(access_token)
17diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
18index 15e4f09..add6348 100644
19--- a/lib/lp/code/xmlrpc/tests/test_git.py
20+++ b/lib/lp/code/xmlrpc/tests/test_git.py
21@@ -1407,6 +1407,27 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
22 access_token_id=removeSecurityProxy(token).id,
23 )
24
25+ def test_confirm_git_repository_with_project_access_token(self):
26+ # Similarly to git repository tokens, a project access token cannot be
27+ # used to authorize confirming repository creation.
28+ requester = self.factory.makePerson()
29+ project = self.factory.makeProduct(owner=requester)
30+ repository = self.factory.makeGitRepository(
31+ target=project,
32+ owner=requester,
33+ status=GitRepositoryStatus.CREATING,
34+ )
35+ _, token = self.factory.makeAccessToken(
36+ owner=requester,
37+ target=project,
38+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
39+ )
40+ self.assertConfirmRepoCreationUnauthorized(
41+ requester,
42+ repository,
43+ access_token_id=removeSecurityProxy(token).id,
44+ )
45+
46 def test_abort_repo_creation(self):
47 requester = self.factory.makePerson()
48 repo = self.factory.makeGitRepository(owner=requester)
49@@ -1636,6 +1657,27 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
50 access_token_id=removeSecurityProxy(token).id,
51 )
52
53+ def test_abort_git_repository_with_project_access_token(self):
54+ # Similarly to git repository tokens, a project access token cannot be
55+ # used to authorize aborting repository creation.
56+ requester = self.factory.makePerson()
57+ project = self.factory.makeProduct(owner=requester)
58+ repository = self.factory.makeGitRepository(
59+ target=project,
60+ owner=requester,
61+ status=GitRepositoryStatus.CREATING,
62+ )
63+ _, token = self.factory.makeAccessToken(
64+ owner=requester,
65+ target=project,
66+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
67+ )
68+ self.assertAbortRepoCreationUnauthorized(
69+ requester,
70+ repository,
71+ access_token_id=removeSecurityProxy(token).id,
72+ )
73+
74 def test_abort_git_repository_creation_of_non_existing_repository(self):
75 owner = self.factory.makePerson()
76 repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner))
77@@ -2634,6 +2676,110 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
78 macaroon_raw=macaroon.serialize(),
79 )
80
81+ def _assert_translatePath_expected(
82+ self, requester, token, repository, expected_success=True, scope="read"
83+ ):
84+ with person_logged_in(requester):
85+ path = "/%s" % repository.unique_name
86+ private = (
87+ repository.information_type == InformationType.PRIVATESECURITY
88+ )
89+
90+ login(ANONYMOUS)
91+ if expected_success:
92+ self.assertTranslates(
93+ requester,
94+ path,
95+ repository,
96+ permission=scope,
97+ readable=(scope == "read"),
98+ writable=(scope == "write"),
99+ private=private,
100+ access_token_id=removeSecurityProxy(token).id,
101+ )
102+ else:
103+ self.assertUnauthorized(
104+ requester,
105+ path,
106+ permission=scope,
107+ access_token_id=removeSecurityProxy(token).id,
108+ )
109+
110+ def test_translatePath_user_project_access_token_pull(self):
111+ # A user with a suitable project access token can pull from a
112+ # repository that belongs to that project, but not others, even if they
113+ # own them.
114+ requester = self.factory.makePerson()
115+ project = self.factory.makeProduct(owner=requester)
116+ with person_logged_in(requester):
117+ _, token = self.factory.makeAccessToken(
118+ owner=requester,
119+ target=project,
120+ scopes=[AccessTokenScope.REPOSITORY_PULL],
121+ )
122+
123+ repositories_access = [
124+ (self.factory.makeGitRepository(), False),
125+ (self.factory.makeGitRepository(owner=requester), False),
126+ (self.factory.makeGitRepository(target=project), True),
127+ (
128+ self.factory.makeGitRepository(
129+ owner=requester, target=project
130+ ),
131+ True,
132+ ),
133+ (
134+ self.factory.makeGitRepository(
135+ owner=requester,
136+ target=project,
137+ information_type=InformationType.PRIVATESECURITY,
138+ ),
139+ True,
140+ ),
141+ ]
142+
143+ for repository, expected_success in repositories_access:
144+ self._assert_translatePath_expected(
145+ requester, token, repository, expected_success
146+ )
147+
148+ def test_translatePath_user_project_access_token_push(self):
149+ # A user with a suitable project access token can push from a
150+ # repository that belongs to that project, but not others, even if they
151+ # own them.
152+ requester = self.factory.makePerson()
153+ project = self.factory.makeProduct(owner=requester)
154+ with person_logged_in(requester):
155+ _, token = self.factory.makeAccessToken(
156+ owner=requester,
157+ target=project,
158+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
159+ )
160+
161+ repositories_access = [
162+ (self.factory.makeGitRepository(), False),
163+ (self.factory.makeGitRepository(owner=requester), False),
164+ (
165+ self.factory.makeGitRepository(
166+ owner=requester, target=project
167+ ),
168+ True,
169+ ),
170+ (
171+ self.factory.makeGitRepository(
172+ owner=requester,
173+ target=project,
174+ information_type=InformationType.PRIVATESECURITY,
175+ ),
176+ True,
177+ ),
178+ ]
179+
180+ for repository, expected_success in repositories_access:
181+ self._assert_translatePath_expected(
182+ requester, token, repository, expected_success, "write"
183+ )
184+
185 def test_translatePath_user_access_token_pull(self):
186 # A user with a suitable access token can pull from the
187 # corresponding repository, but not others, even if they own them.
188@@ -2679,23 +2825,6 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
189 access_token_id=removeSecurityProxy(token).id,
190 )
191
192- def test_translatePath_user_access_token_pull_wrong_scope(self):
193- # A user with an access token that does not have the repository:pull
194- # scope cannot pull from the corresponding repository.
195- requester = self.factory.makePerson()
196- repository = self.factory.makeGitRepository(owner=requester)
197- _, token = self.factory.makeAccessToken(
198- owner=requester,
199- target=repository,
200- scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS],
201- )
202- self.assertPermissionDenied(
203- requester,
204- "/%s" % repository.unique_name,
205- permission="read",
206- access_token_id=removeSecurityProxy(token).id,
207- )
208-
209 def test_translatePath_user_access_token_push(self):
210 # A user with a suitable access token can push to the corresponding
211 # repository, but not others, even if they own them.
212@@ -2743,23 +2872,66 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
213 access_token_id=removeSecurityProxy(token).id,
214 )
215
216- def test_translatePath_user_access_token_push_wrong_scope(self):
217- # A user with an access token that does not have the repository:push
218- # scope cannot push to the corresponding repository.
219- requester = self.factory.makePerson()
220- repository = self.factory.makeGitRepository(owner=requester)
221+ def _assert_translatePath_permission_denied_wrong_scope(
222+ self, requester, repository, token_target, scope
223+ ):
224+ wrong_scope = (
225+ AccessTokenScope.REPOSITORY_PUSH
226+ if scope == "read"
227+ else AccessTokenScope.REPOSITORY_PULL
228+ )
229 _, token = self.factory.makeAccessToken(
230 owner=requester,
231- target=repository,
232- scopes=[AccessTokenScope.REPOSITORY_PULL],
233+ target=token_target,
234+ scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS, wrong_scope],
235 )
236 self.assertPermissionDenied(
237 requester,
238 "/%s" % repository.unique_name,
239- permission="write",
240+ permission=scope,
241 access_token_id=removeSecurityProxy(token).id,
242 )
243
244+ def test_translatePath_user_git_access_token_pull_wrong_scope(self):
245+ # A user with a git repository access token that does not have the
246+ # repository:pull scope cannot pull from the corresponding repository.
247+ requester = self.factory.makePerson()
248+ repository = self.factory.makeGitRepository(owner=requester)
249+ self._assert_translatePath_permission_denied_wrong_scope(
250+ requester, repository, token_target=repository, scope="read"
251+ )
252+
253+ def test_translatePath_user_project_access_token_pull_wrong_scope(self):
254+ # A user with a project access token that does not have the
255+ # repository:pull scope cannot pull from the corresponding repository.
256+ requester = self.factory.makePerson()
257+ project = self.factory.makeProduct(owner=requester)
258+ repository = self.factory.makeGitRepository(
259+ owner=requester, target=project
260+ )
261+ self._assert_translatePath_permission_denied_wrong_scope(
262+ requester, repository, token_target=project, scope="read"
263+ )
264+
265+ def test_translatePath_user_git_access_token_push_wrong_scope(self):
266+ # A user with a git repository access token that does not have the
267+ # repository:push scope cannot push to the corresponding repository.
268+ requester = self.factory.makePerson()
269+ repository = self.factory.makeGitRepository(owner=requester)
270+ self._assert_translatePath_permission_denied_wrong_scope(
271+ requester, repository, token_target=repository, scope="write"
272+ )
273+
274+ def test_translatePath_user_project_access_token_push_wrong_scope(self):
275+ # A user with a project access token that does not have the
276+ # repository:push scope cannot push to the corresponding repository.
277+ requester = self.factory.makePerson()
278+ project = self.factory.makeProduct(owner=requester)
279+ repository = self.factory.makeGitRepository(target=project)
280+ self._assert_translatePath_permission_denied_wrong_scope(
281+ requester, repository, token_target=project, scope="write"
282+ )
283+
284 def test_translatePath_user_access_token_nonexistent(self):
285 # Attempting to pass a nonexistent access token ID returns
286 # Unauthorized.
287@@ -2927,15 +3099,12 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
288 auth_params,
289 )
290
291- def test_getMergeProposalURL_user_access_token(self):
292- # The merge proposal URL is returned by LP for a non-default branch
293- # pushed by a user with a suitable access token that has their
294- # ordinary privileges on the corresponding repository.
295- requester = self.factory.makePerson()
296- repository = self._makeGitRepositoryWithRefs(owner=requester)
297+ def _assert_getMergeProposalURL_user_access_token(
298+ self, requester, repository, token_target
299+ ):
300 _, token = self.factory.makeAccessToken(
301 owner=requester,
302- target=repository,
303+ target=token_target,
304 scopes=[AccessTokenScope.REPOSITORY_PUSH],
305 )
306 auth_params = _make_auth_params(
307@@ -2943,13 +3112,35 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
308 )
309 self.assertHasMergeProposalURL(repository, "branch", auth_params)
310
311- def test_getMergeProposalURL_user_access_token_wrong_repository(self):
312- # getMergeProposalURL refuses access tokens for a different
313- # repository.
314+ def test_getMergeProposalURL_user_git_access_token(self):
315+ # The merge proposal URL is returned by LP for a non-default branch
316+ # pushed by a user with a suitable git repository access token that has
317+ # their ordinary privileges on a repository in that project.
318 requester = self.factory.makePerson()
319 repository = self._makeGitRepositoryWithRefs(owner=requester)
320+ self._assert_getMergeProposalURL_user_access_token(
321+ requester, repository, token_target=repository
322+ )
323+
324+ def test_getMergeProposalURL_user_project_access_token(self):
325+ # The merge proposal URL is returned by LP for a non-default branch
326+ # pushed by a user with a suitable project access token that has
327+ # their ordinary privileges on a repository in that project.
328+ requester = self.factory.makePerson()
329+ project = self.factory.makeProduct(owner=requester)
330+ repository = self._makeGitRepositoryWithRefs(target=project)
331+ self._assert_getMergeProposalURL_user_access_token(
332+ requester, repository, token_target=repository
333+ )
334+
335+ def _assert_getMergeProposalURL_user_access_token_wrong_repository(
336+ self, requester, token_target
337+ ):
338+ repository = self._makeGitRepositoryWithRefs(owner=requester)
339 _, token = self.factory.makeAccessToken(
340- owner=requester, scopes=[AccessTokenScope.REPOSITORY_PUSH]
341+ target=token_target,
342+ owner=requester,
343+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
344 )
345 auth_params = _make_auth_params(
346 requester, access_token_id=removeSecurityProxy(token).id
347@@ -2963,6 +3154,26 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
348 auth_params,
349 )
350
351+ def test_getMergeProposalURL_user_git_access_token_wrong_repository(self):
352+ # getMergeProposalURL refuses access tokens for a different
353+ # repository.
354+ requester = self.factory.makePerson()
355+ repository = self.factory.makeGitRepository(owner=requester)
356+ self._assert_getMergeProposalURL_user_access_token_wrong_repository(
357+ requester, token_target=repository
358+ )
359+
360+ def test_getMergeProposalURL_user_project_access_token_wrong_repository(
361+ self,
362+ ):
363+ # getMergeProposalURL refuses access tokens for repository from a
364+ # different project.
365+ requester = self.factory.makePerson()
366+ project = self.factory.makeProduct(owner=requester)
367+ self._assert_getMergeProposalURL_user_access_token_wrong_repository(
368+ requester, token_target=project
369+ )
370+
371 def test_getMergeProposalURL_code_import(self):
372 # A merge proposal URL from LP to Turnip is not returned for
373 # code import job as there is no User at the other end.
374@@ -3464,12 +3675,15 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
375 macaroon.serialize(),
376 )
377
378- def test_authenticateWithPassword_user_access_token(self):
379+ def _assert_authenticateWithPassword_user_access_token(
380+ self,
381+ requester,
382+ secret,
383+ token,
384+ ):
385 # A user with a suitable access token can authenticate using it, in
386 # which case we return both the access token and the uid for use by
387 # later calls.
388- requester = self.factory.makePerson()
389- secret, token = self.factory.makeAccessToken(owner=requester)
390 self.assertIsNone(removeSecurityProxy(token).date_last_used)
391 self.assertEqual(
392 {
393@@ -3490,6 +3704,30 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
394 secret,
395 )
396
397+ def test_authenticateWithPassword_user_git_access_token(self):
398+ # A user with a suitable git repository access token can authenticate
399+ # using it
400+ requester = self.factory.makePerson()
401+ secret, token = self.factory.makeAccessToken(owner=requester)
402+ self._assert_authenticateWithPassword_user_access_token(
403+ requester,
404+ secret,
405+ token,
406+ )
407+
408+ def test_authenticateWithPassword_user_project_access_token(self):
409+ # A user with a suitable project access token can authenticate using it
410+ requester = self.factory.makePerson()
411+ project = self.factory.makeProduct(owner=requester)
412+ secret, token = self.factory.makeAccessToken(
413+ owner=requester, target=project
414+ )
415+ self._assert_authenticateWithPassword_user_access_token(
416+ requester,
417+ secret,
418+ token,
419+ )
420+
421 def test_authenticateWithPassword_user_access_token_expired(self):
422 # An expired access token is rejected.
423 requester = self.factory.makePerson()
424@@ -3855,21 +4093,76 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
425 macaroon_raw=macaroon.serialize(),
426 )
427
428- def test_checkRefPermissions_user_access_token(self):
429+ def _assert_checkRefPermissions_permissions(
430+ self, requester, token, repository, expected_success
431+ ):
432+ ref_path = b"refs/heads/main"
433+ login(ANONYMOUS)
434+ if expected_success:
435+ # This expects that since we don't add any permission rules and the
436+ # requester is the owner of the repository, then the call returns
437+ # all 3 permissions
438+ expected_permissions = ["create", "push", "force_push"]
439+ else:
440+ expected_permissions = []
441+ self.assertHasRefPermissions(
442+ requester,
443+ repository,
444+ [ref_path],
445+ {ref_path: expected_permissions},
446+ access_token_id=removeSecurityProxy(token).id,
447+ )
448+
449+ def test_checkRefPermissions_user_project_access_token(self):
450+ # A user with a suitable access token targetted at a project, has their
451+ # ordinary privileges on repositories from the same project, but not
452+ # others, even if they own them.
453+ requester = self.factory.makePerson()
454+ project = self.factory.makeProduct(owner=requester)
455+ repositories = [
456+ (self.factory.makeGitRepository(), False),
457+ (self.factory.makeGitRepository(owner=requester), False),
458+ (self.factory.makeGitRepository(target=project), False),
459+ (
460+ self.factory.makeGitRepository(
461+ owner=requester, target=project
462+ ),
463+ True,
464+ ),
465+ (
466+ self.factory.makeGitRepository(
467+ target=project,
468+ owner=requester,
469+ information_type=InformationType.PRIVATESECURITY,
470+ ),
471+ True,
472+ ),
473+ ]
474+ with person_logged_in(requester):
475+ _, token = self.factory.makeAccessToken(
476+ owner=requester,
477+ target=project,
478+ scopes=[AccessTokenScope.REPOSITORY_PUSH],
479+ )
480+
481+ for repository, expected_success in repositories:
482+ self._assert_checkRefPermissions_permissions(
483+ requester, token, repository, expected_success
484+ )
485+
486+ def test_checkRefPermissions_user_git_access_token(self):
487 # A user with a suitable access token has their ordinary privileges
488 # on the corresponding repository, but not others, even if they own
489 # them.
490 requester = self.factory.makePerson()
491 repositories = [
492- self.factory.makeGitRepository(owner=requester) for _ in range(2)
493- ]
494- repositories.append(
495+ self.factory.makeGitRepository(owner=requester),
496+ self.factory.makeGitRepository(owner=requester),
497 self.factory.makeGitRepository(
498 owner=requester,
499 information_type=InformationType.PRIVATESECURITY,
500- )
501- )
502- ref_path = b"refs/heads/main"
503+ ),
504+ ]
505 tokens = []
506 with person_logged_in(requester):
507 for repository in repositories:
508@@ -3879,29 +4172,21 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
509 scopes=[AccessTokenScope.REPOSITORY_PUSH],
510 )
511 tokens.append(token)
512+
513 for i, repository in enumerate(repositories):
514 for j, token in enumerate(tokens):
515- login(ANONYMOUS)
516- if i == j:
517- expected_permissions = ["create", "push", "force_push"]
518- else:
519- expected_permissions = []
520- self.assertHasRefPermissions(
521- requester,
522- repository,
523- [ref_path],
524- {ref_path: expected_permissions},
525- access_token_id=removeSecurityProxy(token).id,
526+ self._assert_checkRefPermissions_permissions(
527+ requester, token, repository, expected_success=(i == j)
528 )
529
530- def test_checkRefPermissions_user_access_token_wrong_scope(self):
531+ def _assert_checkRefPermissions_user_access_token_wrong_scope(
532+ self, requester, repository, token_target
533+ ):
534 # A user with an access token that does not have the repository:push
535 # scope cannot push to any branch in the corresponding repository.
536- requester = self.factory.makePerson()
537- repository = self.factory.makeGitRepository(owner=requester)
538 _, token = self.factory.makeAccessToken(
539 owner=requester,
540- target=repository,
541+ target=token_target,
542 scopes=[AccessTokenScope.REPOSITORY_PULL],
543 )
544 ref_path = b"refs/heads/main"
545@@ -3913,6 +4198,25 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
546 access_token_id=removeSecurityProxy(token).id,
547 )
548
549+ def test_checkRefPermissions_user_git_access_token_wrong_scope(self):
550+ # A user with an access token that does not have the repository:push
551+ # scope cannot push to any branch in the corresponding repository.
552+ requester = self.factory.makePerson()
553+ repository = self.factory.makeGitRepository(owner=requester)
554+ self._assert_checkRefPermissions_user_access_token_wrong_scope(
555+ requester, repository, token_target=repository
556+ )
557+
558+ def test_checkRefPermissions_user_project_access_token_wrong_scope(self):
559+ # A user with an access token that does not have the repository:push
560+ # scope cannot push to any branch in the corresponding repository.
561+ requester = self.factory.makePerson()
562+ project = self.factory.makeProduct(owner=requester)
563+ repository = self.factory.makeGitRepository(target=project)
564+ self._assert_checkRefPermissions_user_access_token_wrong_scope(
565+ requester, repository, token_target=project
566+ )
567+
568 def assertUpdatesRepackStats(self, repo):
569 start_time = datetime.now(timezone.utc)
570 self.assertIsNone(
571diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
572index 788333b..1bc9a03 100644
573--- a/lib/lp/registry/model/person.py
574+++ b/lib/lp/registry/model/person.py
575@@ -1206,13 +1206,10 @@ class Person(
576 ]
577
578 if transitive:
579- # getProductPrivacyFilter may also use TeamParticipation, so
580- # ensure we use a different one.
581- ownership_participation = ClassAlias(TeamParticipation)
582 clauses.extend(
583 [
584- Product._owner_id == ownership_participation.team_id,
585- ownership_participation.person_id == self.id,
586+ Product._owner_id == TeamParticipation.team_id,
587+ TeamParticipation.person_id == self.id,
588 ]
589 )
590 else:
591diff --git a/lib/lp/registry/model/product.py b/lib/lp/registry/model/product.py
592index f3bb669..4319feb 100644
593--- a/lib/lp/registry/model/product.py
594+++ b/lib/lp/registry/model/product.py
595@@ -31,6 +31,7 @@ from storm.expr import (
596 Or,
597 Select,
598 )
599+from storm.info import ClassAlias
600 from storm.locals import (
601 Bool,
602 DateTime,
603@@ -1891,6 +1892,10 @@ class ProductSet:
604 if roles.in_admin or roles.in_commercial_admin:
605 return True
606
607+ # In places where this method is used, they might want to use
608+ # TeamParticipation. This ensures that we use a different one.
609+ ownership_participation = ClassAlias(TeamParticipation)
610+
611 # Normal users can see any project for which they can see either
612 # an entire policy or an artifact.
613 # XXX wgrant 2015-06-26: This is slower than ideal for people in
614@@ -1905,12 +1910,12 @@ class ProductSet:
615 tables=(
616 AccessPolicyGrantFlat,
617 Join(
618- TeamParticipation,
619- TeamParticipation.team_id
620+ ownership_participation,
621+ ownership_participation.team_id
622 == AccessPolicyGrantFlat.grantee_id,
623 ),
624 ),
625- where=(TeamParticipation.person == user),
626+ where=(ownership_participation.person_id == user.id),
627 ),
628 ),
629 False,
630diff --git a/lib/lp/services/auth/interfaces.py b/lib/lp/services/auth/interfaces.py
631index 6081830..10c3c58 100644
632--- a/lib/lp/services/auth/interfaces.py
633+++ b/lib/lp/services/auth/interfaces.py
634@@ -73,7 +73,15 @@ class IAccessToken(Interface):
635 description=_("The Git repository for which the token was issued."),
636 # Really IGitRepository, patched in lp.services.auth.webservice.
637 schema=Interface,
638- required=True,
639+ required=False,
640+ readonly=True,
641+ )
642+
643+ project = Reference(
644+ title=_("Project"),
645+ description=_("The project for which the token was issued."),
646+ schema=Interface,
647+ required=False,
648 readonly=True,
649 )
650
651diff --git a/lib/lp/services/auth/model.py b/lib/lp/services/auth/model.py
652index f595637..2b4fe2b 100644
653--- a/lib/lp/services/auth/model.py
654+++ b/lib/lp/services/auth/model.py
655@@ -21,6 +21,7 @@ from zope.security.proxy import removeSecurityProxy
656
657 from lp.code.interfaces.gitcollection import IAllGitRepositories
658 from lp.code.interfaces.gitrepository import IGitRepository
659+from lp.registry.interfaces.product import IProduct
660 from lp.registry.model.teammembership import TeamParticipation
661 from lp.services.auth.enums import AccessTokenScope
662 from lp.services.auth.interfaces import IAccessToken, IAccessTokenSet
663@@ -50,9 +51,12 @@ class AccessToken(StormBase):
664
665 description = Unicode(name="description", allow_none=False)
666
667- git_repository_id = Int(name="git_repository", allow_none=False)
668+ git_repository_id = Int(name="git_repository", allow_none=True)
669 git_repository = Reference(git_repository_id, "GitRepository.id")
670
671+ project_id = Int(name="project", allow_none=True)
672+ project = Reference(project_id, "Product.id")
673+
674 _scopes = JSON(name="scopes", allow_none=False)
675
676 date_last_used = DateTime(
677@@ -76,6 +80,8 @@ class AccessToken(StormBase):
678 self.description = description
679 if IGitRepository.providedBy(target):
680 self.git_repository = target
681+ elif IProduct.providedBy(target):
682+ self.project = target
683 else:
684 raise TypeError("Unsupported target: {!r}".format(target))
685 self.scopes = scopes
686@@ -85,7 +91,7 @@ class AccessToken(StormBase):
687 @property
688 def target(self):
689 """See `IAccessToken`."""
690- return self.git_repository
691+ return self.git_repository or self.project
692
693 @property
694 def scopes(self):
695@@ -181,32 +187,57 @@ class AccessTokenSet:
696 self, target, visible_by_user=None, include_expired=False
697 ):
698 """See `IAccessTokenSet`."""
699+
700 clauses = []
701 if IGitRepository.providedBy(target):
702 clauses.append(AccessToken.git_repository == target)
703- if visible_by_user is not None:
704- collection = (
705- getUtility(IAllGitRepositories)
706- .visibleByUser(visible_by_user)
707- .ownedByTeamMember(visible_by_user)
708+ elif IProduct.providedBy(target):
709+ clauses.append(AccessToken.project == target)
710+ else:
711+ raise TypeError("Unsupported target: {!r}".format(target))
712+
713+ if visible_by_user is not None:
714+ # Evaluate if user owns the target (directly or indirectly).
715+ # The queries below might look more complex than they should to
716+ # evaluate ownership, but they are more performant as this will run
717+ # in one single query in the end.
718+ if IGitRepository.providedBy(target):
719+ user_owns_target = AccessToken.git_repository_id.is_in(
720+ removeSecurityProxy(
721+ (
722+ getUtility(IAllGitRepositories)
723+ .visibleByUser(visible_by_user)
724+ .ownedByTeamMember(visible_by_user)
725+ ).getRepositoryIds()
726+ )._get_select()
727 )
728- ids = collection.getRepositoryIds()
729- clauses.append(
730- Or(
731- AccessToken.owner_id.is_in(
732- Select(
733- TeamParticipation.team_id,
734- where=TeamParticipation.person
735- == visible_by_user,
736- )
737- ),
738- AccessToken.git_repository_id.is_in(
739- removeSecurityProxy(ids)._get_select()
740+ else:
741+ # Avoid circular import
742+ from lp.registry.model.product import Product, ProductSet
743+
744+ clauses.extend([Product.active, Product.id == target.id])
745+ user_owns_target = AccessToken.project_id.is_in(
746+ Select(
747+ Product.id,
748+ where=And(
749+ Product._owner_id == TeamParticipation.team_id,
750+ TeamParticipation.person_id == visible_by_user.id,
751+ ProductSet.getProductPrivacyFilter(
752+ visible_by_user
753+ ),
754 ),
755 )
756 )
757- else:
758- raise TypeError("Unsupported target: {!r}".format(target))
759+
760+ user_owns_token = AccessToken.owner_id.is_in(
761+ Select(
762+ TeamParticipation.team_id,
763+ where=TeamParticipation.person_id == visible_by_user.id,
764+ )
765+ )
766+
767+ clauses.append(Or(user_owns_target, user_owns_token))
768+
769 if not include_expired:
770 clauses.append(
771 Or(
772diff --git a/lib/lp/services/auth/tests/test_model.py b/lib/lp/services/auth/tests/test_model.py
773index ca33eec..a02904a 100644
774--- a/lib/lp/services/auth/tests/test_model.py
775+++ b/lib/lp/services/auth/tests/test_model.py
776@@ -22,6 +22,7 @@ from zope.component import getUtility
777 from zope.security.proxy import removeSecurityProxy
778
779 from lp.code.interfaces.gitrepository import IGitRepository
780+from lp.registry.interfaces.person import TeamMembershipPolicy
781 from lp.services.auth.enums import AccessTokenScope
782 from lp.services.auth.interfaces import IAccessTokenSet
783 from lp.services.auth.utils import create_access_token_secret
784@@ -197,6 +198,11 @@ class TestAccessTokenGitRepository(TestAccessTokenBase, TestCaseWithFactory):
785 return self.factory.makeGitRepository(owner=owner)
786
787
788+class TestAccessTokenProduct(TestAccessTokenBase, TestCaseWithFactory):
789+ def makeTarget(self, owner=None):
790+ return self.factory.makeProduct(owner=owner)
791+
792+
793 class TestAccessTokenSetBase:
794 layer = DatabaseFunctionalLayer
795
796@@ -226,7 +232,7 @@ class TestAccessTokenSetBase:
797 )
798
799 def test_getByID(self):
800- secret, token = self.factory.makeAccessToken(target=self.makeTarget())
801+ _, token = self.factory.makeAccessToken(target=self.makeTarget())
802 token_id = removeSecurityProxy(token).id
803 self.assertEqual(token, getUtility(IAccessTokenSet).getByID(token_id))
804 self.assertIsNone(getUtility(IAccessTokenSet).getByID(token_id + 1))
805@@ -315,6 +321,94 @@ class TestAccessTokenSetBase:
806 ),
807 )
808
809+ def test_findByTarget_visible_by_owner(self):
810+ # If a user owns a target, they can see all the tokens for that target
811+ # regadless if they own the tokens.
812+ owner = self.factory.makePerson()
813+ owned_target = self.makeTarget(owner=owner)
814+ owned_target_tokens = [
815+ self.factory.makeAccessToken(target=owned_target)[1],
816+ self.factory.makeAccessToken(target=owned_target)[1],
817+ ]
818+ self.factory.makeAccessToken(target=self.makeTarget())[1]
819+ self.factory.makeAccessToken(target=self.makeTarget())[1]
820+
821+ self.assertContentEqual(
822+ owned_target_tokens,
823+ getUtility(IAccessTokenSet).findByTarget(
824+ owned_target, visible_by_user=owner
825+ ),
826+ )
827+
828+ def test_findByTarget_visible_by_team_member(self):
829+ # If a user belongs to a team that owns a target, they can see all the
830+ # tokens for that target regardless if they own the tokens.
831+
832+ # Only restricted or moderated teams can own certain targets (Product)
833+ team = self.factory.makeTeam(
834+ membership_policy=TeamMembershipPolicy.RESTRICTED
835+ )
836+ team_member = self.factory.makePerson()
837+ with person_logged_in(team.teamowner):
838+ team.addMember(team_member, team)
839+
840+ owned_target = self.makeTarget(owner=team)
841+ owned_target_tokens = [
842+ self.factory.makeAccessToken(target=owned_target)[1],
843+ self.factory.makeAccessToken(target=owned_target)[1],
844+ ]
845+
846+ self.factory.makeAccessToken(target=self.makeTarget())[1]
847+ self.factory.makeAccessToken(target=self.makeTarget())[1]
848+
849+ self.assertContentEqual(
850+ owned_target_tokens,
851+ getUtility(IAccessTokenSet).findByTarget(
852+ owned_target, visible_by_user=team_member
853+ ),
854+ )
855+
856+ def test_findByTarget_visible_by_user_inactive_target(self):
857+ # User cannot see tokens for inactive targets.
858+
859+ owner = self.factory.makePerson()
860+ target = self.makeTarget(owner=owner)
861+
862+ if IGitRepository.providedBy(target):
863+ self.skipTest(
864+ "Currently, Git repositories allow requests without scopes."
865+ )
866+
867+ removeSecurityProxy(target).active = False
868+ self.factory.makeAccessToken(target=target, owner=owner)[1]
869+
870+ self.assertContentEqual(
871+ [],
872+ getUtility(IAccessTokenSet).findByTarget(
873+ target, visible_by_user=owner
874+ ),
875+ )
876+
877+ def test_findByTarget_visible_by_user_from_a_team_that_owns_token(self):
878+ # Method with a visible_by_user parameter, returns only tokens that
879+ # are owned by the `visible_by_user` Person or by a team that the
880+ # `visible_by_user` Person belongs to.
881+ target = self.makeTarget()
882+ team = self.factory.makeTeam()
883+ team_member = self.factory.makePerson()
884+ with person_logged_in(team.teamowner):
885+ team.addMember(team_member, team)
886+ _, token = self.factory.makeAccessToken(owner=team, target=target)
887+ self.factory.makeAccessToken(target=target)
888+
889+ for user in [team.teamowner, team_member]:
890+ self.assertContentEqual(
891+ [token],
892+ getUtility(IAccessTokenSet).findByTarget(
893+ target, visible_by_user=user
894+ ),
895+ )
896+
897 def test_findByTarget_excludes_expired(self):
898 target = self.makeTarget()
899 _, current_token = self.factory.makeAccessToken(target=target)
900@@ -432,8 +526,13 @@ class TestAccessTokenSetBase:
901 class TestGitRepositoryAccessTokenSet(
902 TestAccessTokenSetBase, TestCaseWithFactory
903 ):
904- def makeTarget(self):
905- return self.factory.makeGitRepository()
906+ def makeTarget(self, **kwargs):
907+ return self.factory.makeGitRepository(**kwargs)
908+
909+
910+class TestProjectAccessTokenSet(TestAccessTokenSetBase, TestCaseWithFactory):
911+ def makeTarget(self, **kwargs):
912+ return self.factory.makeProduct(**kwargs)
913
914
915 class TestAccessTokenTargetBase:

Subscribers

People subscribed via source and target branches

to status/vote changes: