Merge ~cjwatson/launchpad:archive-subscriber-view-files into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: c146d8c67f571c7386e04e19b63dc70f6de4f7cd
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:archive-subscriber-view-files
Merge into: launchpad:master
Diff against target: 207 lines (+86/-38)
4 files modified
lib/lp/services/librarian/browser.py (+3/-1)
lib/lp/soyuz/browser/archive.py (+2/-1)
lib/lp/soyuz/interfaces/archive.py (+35/-35)
lib/lp/soyuz/stories/ppa/xx-ppa-files.rst (+46/-1)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+438337@code.launchpad.net

Commit message

Allow archive subscribers to use +sourcefiles and +files

Description of the change

This will be needed by the `debuginfod` service, but it seems reasonable anyway. It has the effect that subscribers will be able to see historical files in archives as well as current ones; but they can build much of that information up over time anyway, so this was never a robust boundary, and the in-progress archive snapshot service will soon allow equivalent access.

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

LGTM

I had to look up what a "subscriber" is in this context.
"subscriber: An `IPerson` who is allowed to access the repository for this archive."

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/librarian/browser.py b/lib/lp/services/librarian/browser.py
2index d364b68..334636f 100644
3--- a/lib/lp/services/librarian/browser.py
4+++ b/lib/lp/services/librarian/browser.py
5@@ -78,10 +78,12 @@ class FileNavigationMixin:
6 with the same filename (product files or bug attachments).
7 """
8
9+ file_navigation_permission = "launchpad.View"
10+
11 @stepthrough("+files")
12 def traverse_files(self, filename):
13 """Traverse on filename in the archive domain."""
14- if not check_permission("launchpad.View", self.context):
15+ if not check_permission(self.file_navigation_permission, self.context):
16 raise Unauthorized()
17 library_file = self.context.getFileByName(filename)
18
19diff --git a/lib/lp/soyuz/browser/archive.py b/lib/lp/soyuz/browser/archive.py
20index 63b9171..f785294 100644
21--- a/lib/lp/soyuz/browser/archive.py
22+++ b/lib/lp/soyuz/browser/archive.py
23@@ -222,6 +222,7 @@ class ArchiveNavigation(Navigation, FileNavigationMixin):
24 """Navigation methods for IArchive."""
25
26 usedfor = IArchive
27+ file_navigation_permission = "launchpad.SubscriberView"
28
29 @stepthrough("+build")
30 def traverse_build(self, name):
31@@ -463,7 +464,7 @@ class ArchiveNavigation(Navigation, FileNavigationMixin):
32 version = self.request.stepstogo.consume()
33 filename = self.request.stepstogo.consume()
34
35- if not check_permission("launchpad.View", self.context):
36+ if not check_permission("launchpad.SubscriberView", self.context):
37 raise Unauthorized()
38
39 library_file = self.context.getSourceFileByName(
40diff --git a/lib/lp/soyuz/interfaces/archive.py b/lib/lp/soyuz/interfaces/archive.py
41index f1e2c3f..f31793b 100644
42--- a/lib/lp/soyuz/interfaces/archive.py
43+++ b/lib/lp/soyuz/interfaces/archive.py
44@@ -794,6 +794,41 @@ class IArchiveSubscriberView(Interface):
45 :return: A collection containing `BinaryPackagePublishingHistory`.
46 """
47
48+ def getFileByName(filename):
49+ """Return the corresponding `ILibraryFileAlias` in this context.
50+
51+ The following file types (and extension) can be looked up in the
52+ archive context:
53+
54+ * Source files: '.orig.tar.gz', 'tar.gz', '.diff.gz' and '.dsc';
55+ * Binary files: '.deb' and '.udeb';
56+ * Source changesfile: '_source.changes';
57+ * Package diffs: '.diff.gz';
58+
59+ :param filename: exactly filename to be looked up.
60+
61+ :raises AssertionError if the given filename contains a unsupported
62+ filename and/or extension, see the list above.
63+ :raises NotFoundError if no file could not be found.
64+
65+ :return the corresponding `ILibraryFileAlias` is the file was found.
66+ """
67+
68+ def getSourceFileByName(name, version, filename):
69+ """Return the `ILibraryFileAlias` for a source name/version/filename.
70+
71+ This can be used to avoid ambiguities with `getFileByName` in
72+ imported archives, where the upstream archive software may not
73+ always have had robust historical filename uniqueness checks.
74+
75+ :param name: The name of the source package.
76+ :param version: The version of the source package.
77+ :param filename: The exact filename to look up.
78+
79+ :raises NotFoundError: if no matching file could be found.
80+ :return: the corresponding `ILibraryFileAlias`.
81+ """
82+
83 def getPoolFileByPath(
84 path: PurePath, live_at: typing.Optional[datetime] = None
85 ):
86@@ -1283,41 +1318,6 @@ class IArchiveView(IHasBuildRecords):
87 queue admin permissions for this archive.
88 """
89
90- def getFileByName(filename):
91- """Return the corresponding `ILibraryFileAlias` in this context.
92-
93- The following file types (and extension) can be looked up in the
94- archive context:
95-
96- * Source files: '.orig.tar.gz', 'tar.gz', '.diff.gz' and '.dsc';
97- * Binary files: '.deb' and '.udeb';
98- * Source changesfile: '_source.changes';
99- * Package diffs: '.diff.gz';
100-
101- :param filename: exactly filename to be looked up.
102-
103- :raises AssertionError if the given filename contains a unsupported
104- filename and/or extension, see the list above.
105- :raises NotFoundError if no file could not be found.
106-
107- :return the corresponding `ILibraryFileAlias` is the file was found.
108- """
109-
110- def getSourceFileByName(name, version, filename):
111- """Return the `ILibraryFileAlias` for a source name/version/filename.
112-
113- This can be used to avoid ambiguities with `getFileByName` in
114- imported archives, where the upstream archive software may not
115- always have had robust historical filename uniqueness checks.
116-
117- :param name: The name of the source package.
118- :param version: The version of the source package.
119- :param filename: The exact filename to look up.
120-
121- :raises NotFoundError: if no matching file could be found.
122- :return: the corresponding `ILibraryFileAlias`.
123- """
124-
125 def getBinaryPackageRelease(name, version, archtag):
126 """Find the specified `IBinaryPackageRelease` in the archive.
127
128diff --git a/lib/lp/soyuz/stories/ppa/xx-ppa-files.rst b/lib/lp/soyuz/stories/ppa/xx-ppa-files.rst
129index 7e0ce09..8eead97 100644
130--- a/lib/lp/soyuz/stories/ppa/xx-ppa-files.rst
131+++ b/lib/lp/soyuz/stories/ppa/xx-ppa-files.rst
132@@ -27,8 +27,19 @@ Create a private PPA for no-priv.
133 ... owner=no_priv, private=True, name="p3a", distribution=ubuntu
134 ... )
135
136+Add a subscriber to this private PPA.
137+
138+ >>> from zope.security.proxy import removeSecurityProxy
139+ >>> _ = login_person(no_priv)
140+ >>> subscriber = factory.makePerson()
141+ >>> subscriber_email = removeSecurityProxy(
142+ ... subscriber
143+ ... ).preferredemail.email
144+ >>> _ = no_priv_private_ppa.newSubscription(subscriber, no_priv)
145+
146 Initialize SoyuzTestPublisher.
147
148+ >>> login("foo.bar@canonical.com")
149 >>> test_publisher = SoyuzTestPublisher()
150 >>> test_publisher.prepareBreezyAutotest()
151 >>> test_publisher.addFakeChroots()
152@@ -317,6 +328,25 @@ The 'No Privileges' user, the PPA owner, can download the DSC file.
153 Location: https://...restricted.../test-pkg_1.0.dsc?token=...
154 ...
155
156+A subscriber can download the DSC file.
157+
158+ >>> print(
159+ ... http(
160+ ... r"""
161+ ... GET %s HTTP/1.1
162+ ... Authorization: Basic %s:test
163+ ... """
164+ ... % (
165+ ... dsc_file_lp_url.replace("http://launchpad.test", ""),
166+ ... subscriber_email,
167+ ... )
168+ ... )
169+ ... )
170+ HTTP/1.1 303 See Other
171+ ...
172+ Location: https://...restricted.../test-pkg_1.0.dsc?token=...
173+ ...
174+
175 Binary files are served via '+files' rather than '+sourcefiles'.
176
177 >>> login("foo.bar@canonical.com")
178@@ -342,6 +372,22 @@ Binary files are served via '+files' rather than '+sourcefiles'.
179 ...
180 Location: https://...restricted.../test-bin_1.0_all.deb?token=...
181 ...
182+ >>> print(
183+ ... http(
184+ ... r"""
185+ ... GET %s HTTP/1.1
186+ ... Authorization: Basic %s:test
187+ ... """
188+ ... % (
189+ ... deb_file_lp_url.replace("http://launchpad.test", ""),
190+ ... subscriber_email,
191+ ... )
192+ ... )
193+ ... )
194+ HTTP/1.1 303 See Other
195+ ...
196+ Location: https://...restricted.../test-bin_1.0_all.deb?token=...
197+ ...
198
199 If the associated PPA and the `LibraryFileAlias` are public, the +files/
200 proxy redirects to the public http url. We'll copy the test sources and
201@@ -465,7 +511,6 @@ immediately deleted in case of reported ToS violation.
202 >>> from lp.services.database.interfaces import IPrimaryStore
203 >>> login("foo.bar@canonical.com")
204 >>> IPrimaryStore(Archive).commit()
205- >>> from zope.security.proxy import removeSecurityProxy
206 >>> removeSecurityProxy(dsc_file).content = None
207 >>> transaction.commit()
208

Subscribers

People subscribed via source and target branches

to status/vote changes: