Merge ~cjwatson/launchpad:access-token-target-container into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 290b69a059aa77f5a37127fcf44738d18d1be97d
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:access-token-target-container
Merge into: launchpad:master
Diff against target: 61 lines (+27/-4)
2 files modified
lib/lp/services/webapp/tests/test_servers.py (+8/-0)
lib/lp/services/webservice/configuration.py (+19/-4)
Reviewer Review Type Date Requested Status
Ioana Lasc (community) Approve
Review via email: mp+412524@code.launchpad.net

Commit message

Allow access token targets to be containers

Description of the change

A test failure in https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/410373 revealed that it's cumbersome to require access token targets to match the context object exactly: that requires all the interesting methods to be implemented directly on the access token target, and in the case of (for example) updating a revision status report on a commit in a Git repository, that's quite cumbersome.

A better approach would be to work the same way as OAuth token scopes: in those cases, an exact match is fine, but so is a match for the nearest `LaunchpadContainer` in its URL chain, or one of its parent containers. This allows access tokens for Git repositories to be valid for other objects that are subordinate to those repositories.

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) wrote :

This approach makes sense to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/services/webapp/tests/test_servers.py b/lib/lp/services/webapp/tests/test_servers.py
2index a5ffe0a..7b288fe 100644
3--- a/lib/lp/services/webapp/tests/test_servers.py
4+++ b/lib/lp/services/webapp/tests/test_servers.py
5@@ -897,6 +897,14 @@ class TestWebServiceAccessTokens(TestCaseWithFactory):
6 repository,
7 ["repository:build_status", "repository:another_scope"])
8
9+ def test_checkRequest_contains_context(self):
10+ [ref] = self.factory.makeGitRefs()
11+ self._makeAccessTokenVerifiedRequest(
12+ target=ref.repository,
13+ scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS])
14+ getUtility(IWebServiceConfiguration).checkRequest(
15+ ref, ["repository:build_status", "repository:another_scope"])
16+
17 def test_checkRequest_bad_context(self):
18 repository = self.factory.makeGitRepository()
19 self._makeAccessTokenVerifiedRequest(
20diff --git a/lib/lp/services/webservice/configuration.py b/lib/lp/services/webservice/configuration.py
21index 8b56842..665c64e 100644
22--- a/lib/lp/services/webservice/configuration.py
23+++ b/lib/lp/services/webservice/configuration.py
24@@ -15,8 +15,13 @@ from zope.security.interfaces import Unauthorized
25 from lp.app import versioninfo
26 from lp.services.config import config
27 from lp.services.database.sqlbase import block_implicit_flushes
28+from lp.services.webapp.canonicalurl import nearest_adapter
29 from lp.services.webapp.interaction import get_interaction_extras
30-from lp.services.webapp.interfaces import ILaunchBag
31+from lp.services.webapp.interfaces import (
32+ ILaunchBag,
33+ ILaunchpadContainer,
34+ )
35+from lp.services.webapp.publisher import canonical_url
36 from lp.services.webapp.servers import (
37 WebServiceClientRequest,
38 WebServicePublication,
39@@ -102,9 +107,19 @@ class LaunchpadWebServiceConfiguration(BaseWebServiceConfiguration):
40 access_token = get_interaction_extras().access_token
41 if access_token is None:
42 return
43- if access_token.target != context:
44- raise Unauthorized(
45- "Current authentication does not allow access to this object.")
46+
47+ # The access token must be for a target that either exactly matches
48+ # or contains the context object.
49+ if access_token.target == context:
50+ pass
51+ else:
52+ container = nearest_adapter(context, ILaunchpadContainer)
53+ if not container.isWithin(
54+ canonical_url(access_token.target, force_local_path=True)):
55+ raise Unauthorized(
56+ "Current authentication does not allow access to this "
57+ "object.")
58+
59 if not required_scopes:
60 raise Unauthorized(
61 "Current authentication only allows calling scoped methods.")

Subscribers

People subscribed via source and target branches

to status/vote changes: