Merge lp:~lifeless/launchpad/private-librarian into lp:launchpad/db-devel

Proposed by Robert Collins on 2010-07-27
Status: Merged
Approved by: Stuart Bishop on 2010-09-06
Approved revision: no longer in the source branch.
Merged at revision: 9757
Proposed branch: lp:~lifeless/launchpad/private-librarian
Merge into: lp:launchpad/db-devel
Diff against target: 1731 lines (+804/-148)
29 files modified
database/schema/launchpad_session.sql (+16/-0)
lib/canonical/database/sqlbase.py (+10/-2)
lib/canonical/launchpad/browser/librarian.py (+88/-12)
lib/canonical/launchpad/database/ftests/test_timelimitedtoken.py (+29/-0)
lib/canonical/launchpad/database/librarian.py (+69/-1)
lib/canonical/launchpad/doc/librarian.txt (+175/-24)
lib/canonical/launchpad/ftests/test_libraryfilealias.py (+1/-6)
lib/canonical/launchpad/interfaces/librarian.py (+5/-1)
lib/canonical/launchpad/interfaces/lpstorm.py (+0/-1)
lib/canonical/launchpad/scripts/garbo.py (+47/-5)
lib/canonical/launchpad/scripts/tests/test_garbo.py (+23/-2)
lib/canonical/launchpad/utilities/looptuner.py (+4/-0)
lib/canonical/launchpad/webapp/session.py (+2/-1)
lib/canonical/launchpad/zcml/librarian.zcml (+5/-0)
lib/canonical/librarian/client.py (+37/-9)
lib/canonical/librarian/db.py (+33/-5)
lib/canonical/librarian/ftests/test_db.py (+14/-12)
lib/canonical/librarian/ftests/test_storage.py (+3/-2)
lib/canonical/librarian/ftests/test_web.py (+157/-25)
lib/canonical/librarian/interfaces.py (+13/-2)
lib/canonical/librarian/storage.py (+2/-2)
lib/canonical/librarian/web.py (+37/-9)
lib/canonical/testing/layers.py (+6/-4)
lib/lp/bugs/browser/bugattachment.py (+1/-16)
lib/lp/bugs/browser/configure.zcml (+0/-4)
lib/lp/bugs/browser/tests/test_bugattachment_file_access.py (+17/-2)
lib/lp/scripts/utilities/importfascist.py (+1/-0)
lib/lp/services/features/webapp.py (+8/-0)
lib/lp/testing/fakelibrarian.py (+1/-1)
To merge this branch: bzr merge lp:~lifeless/launchpad/private-librarian
Reviewer Review Type Date Requested Status
Stuart Bishop 2010-07-27 Approve on 2010-09-06
Graham Binns (community) release-critical 2010-09-06 Approve on 2010-09-06
Launchpad code reviewers 2010-07-27 Pending
Review via email: mp+31020@code.launchpad.net

Commit message

Introduce a public restricted librarian uses a time limited token approach.

Description of the change

The basic idea is to have an https librarian that uses an access token for a time limited period, rather than proxying on the appservers which is terrible in several ways that aren't all that relevant except to say its hard to improve and incompatible with our peformance goals.

So in this model, we hand out a token when someone (including wget) accesses a private attachment on launchpad, and issue a temporary redirect (over ssl) to https://iLFAID.launchpadlibrarian.net/...file?token=xxxxx

The token goes in the session DB, the garbo cleans that up, and we all are happy happy happy.

Oh, and the librarian rejects requests without a token for private files.

We can't use OAuth because then the OAuth token would be attackable by content in the private librarian.

RT 41202 contains the request for wildcard DNS keys.

The remaining work to make this fully reviewable is to:
 - profit

Folk wanting to test or collaborate on this branch should:
 - dropdb session_dev
 - dropdb session_ftest
 - pull the branch
 - run 'make schema' which will create the timelimitedtoken table in your session dbs.

To post a comment you must log in.
Stuart Bishop (stub) wrote :

The idea seems good.

I don't see why the table should exist in the session database. It should exist in the main database. Putting it in the session database just doubles the number of database connections the Librarian and garbage collector need to maintain, and makes things a maintenance pain as that database has to have schema changes and database permissions applied manually to the primary and fail over databases. This table will get comparatively little traffic, so scalability issues are a non-issue.

I believe the blessed import location for the Storm class is from storm.locals.

Should we be storing the expiry timestamp? I'm wondering if the length of time a token is valid for should be a function of the file size, or if it will be different for different parts of Launchpad.

The index on (url, token) is redundant, as it is created implicitly by the primary key constraint.

Do we actually need to store url and token separately, or just a single url column that includes the query string?

Do we want to store the url, or just the path segment? Storing just the path segment would avoid ambiguities between host and host:port, and might future proof things a bit if our SSL front ends get distributed around the world on different domain names.

Are their any magic headers the Librarian can emit so if a launchpadlibrarian.net URL is bookmarked, the original launchpad.net URL is bookmarked instead? Not terribly important, but if we can do this we will need to store the source URL too as the Librarian has no way of reconstructing it.

review: Needs Information
Stuart Bishop (stub) wrote :

So one thing we will need to do is raise an exception if we try to generate a secure url as an anonymous user. This is a safety net, as I don't think we are doing this at the moment. It will stop accidentally ending up with stale URLs in the Squid caches.

I think I understand why the session store was (ab)used here - it runs in autocommit mode and we rollback the database connections on GET requests to ensure they are idempotent. I still don't like it but can't really offer a better solution with our existing infrastructure except perhaps using memcached. Certainly session.sql should not be modified though, with all the new stuff going into launchpad_session.sql (session.sql is what should have been sent upstream a few years ago).

Robert Collins (lifeless) wrote :

I used the session DB because this stuff is transient : memcached might be an answer too but the token *system* has to be available or private file downloads will fail, and memcached is currently (AIUI) a 'when its there use it' system - which is fine. Replicating the tokens isn't terribly useful given we're just going to delete them again, but all of that is a bit beside the point : yes, we do need to commit changes in get requests :).

Will move the definitions to launchpad_session.sql.

We could store the expiry time, but I figured we'd make the lifetime a config option and thus work from the creation point rather than expiry.

I figured we might want reporting on stuff in the table - thats why I used url, token, rather than a single column. Easy to change if you have a strong opinion any which way - I don't.

I can't answer the url/path segment question yet : once its fully hooked up it should be obvious what is easiest. The current schema is what was easiest based on the attributes of the objects I am using (LibraryFileAlias etc).

We could make the librarian offer a redirect *back* to launchpad for expired tokens, rather than a go-away message - adding the source canonical_url to the table would permit that trivially.

Re: exceptions for anonymous users calling TimeLimitedToken.allocate - I can add that in, is there some existing similar safety need I can reference?

-Rob

Stuart Bishop (stub) wrote :

On Wed, Jul 28, 2010 at 5:08 AM, Robert Collins
<email address hidden> wrote:

> I used the session DB because this stuff is transient : memcached might be an answer too but the token *system* has to be available or private file downloads will fail, and memcached is currently (AIUI) a 'when its there use it' system - which is fine. Replicating the tokens isn't terribly useful given we're just going to delete them again, but all of that is a bit beside the point : yes, we do need to commit changes in get requests :).

Right. We should consider promoting memcached to an always-on service
as we have tripped over a few cases where this would be desirable.
That is a separate task though. Stuffing this in the session database
seems the best fit for now.

> Will move the definitions to launchpad_session.sql.

Ta.

> We could store the expiry time, but I figured we'd make the lifetime a config option and thus work from the creation point rather than expiry.
>
> I figured we might want reporting on stuff in the table - thats why I used url, token, rather than a single column. Easy to change if you have a strong opinion any which way - I don't.
>
> I can't answer the url/path segment question yet : once its fully hooked up it should be obvious what is easiest. The current schema is what was easiest based on the attributes of the objects I am using (LibraryFileAlias etc).

I suspect the host and port is just noise and we only want the path.
We have a complex environment, and relying on the URLs seems fragile
so I personally would feel more secure if we just stored the path.

> We could make the librarian offer a redirect *back* to launchpad for expired tokens, rather than a go-away message - adding the source canonical_url to the table would permit that trivially.

That sounds like a good idea.

> Re: exceptions for anonymous users calling TimeLimitedToken.allocate - I can add that in, is there some existing similar safety need I can reference?

If we offer the redirect back on expired tokens, there is no need for
this. My concern was expired tokens getting stuck in caches and Squid
caching Anonymous connections was the first I thought off.

After going over bug lists with Gary last night, this will solve a lot
of outstanding Librarian issues.

--
Stuart Bishop <email address hidden>
http://www.stuartbishop.net/

Stuart Bishop (stub) wrote :

So files served from the same domain can steal content from files served from the same domain. We don't care for public information, but this isn't cool for private.

One way around this is to serve every file from a unique domain. Changing the URL structure from https://launchpadlibrarian.net/123/file.txt to https://i123.launchpadlibrarian.net/file.txt will do this, and an Apache rewrite rule can be used to redirect old URLs to the new URL.

Graham Binns (gmb) :
review: Approve (release-critical)
Stuart Bishop (stub) wrote :

per IRC discussion

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/launchpad_session.sql'
2--- database/schema/launchpad_session.sql 2009-06-24 21:17:33 +0000
3+++ database/schema/launchpad_session.sql 2010-09-07 04:07:54 +0000
4@@ -14,3 +14,19 @@
5 GRANT EXECUTE ON FUNCTION
6 set_session_pkg_data(text, text, text, bytea) TO session;
7
8+CREATE TABLE TimeLimitedToken (
9+ path text NOT NULL,
10+ token text NOT NULL,
11+ created timestamp without time zone NOT NULL DEFAULT CURRENT_TIMESTAMP,
12+ constraint timelimitedtoken_pky primary key (path, token)
13+ ) WITHOUT OIDS;
14+COMMENT ON TABLE TimeLimitedToken IS 'stores tokens for granting access to a single path in the librarian for a short while. The garbo takes care of cleanups, and we should only have a few thousand at a time. Tokens are handed out just-in-time on the appserver, when a client attempts to dereference a private thing which we do not want to deliver in-line. OAuth tokens cannot be used for the launchpadlibrarian content because they would then be attackable. See lib.canonical.database.librarian for the python class.';
15+-- Give the garbo an efficient selection to cleanup
16+CREATE INDEX timelimitedtoken_created ON TimeLimitedToken(created);
17+-- Give the librarian an efficient lookup
18+CREATE INDEX timelimitedtoken_path_token ON TimeLimitedToken(path, token);
19+
20+-- Let the session user access file access tokens.
21+GRANT SELECT, INSERT, UPDATE, DELETE ON TimeLimitedToken TO session;
22+-- And the garbo needs to run on it too.
23+GRANT SELECT, DELETE ON TimeLimitedToken TO session;
24
25=== modified file 'lib/canonical/database/sqlbase.py'
26--- lib/canonical/database/sqlbase.py 2010-08-24 10:45:57 +0000
27+++ lib/canonical/database/sqlbase.py 2010-09-07 04:07:54 +0000
28@@ -27,11 +27,12 @@
29 'RandomiseOrderDescriptor',
30 'reset_store',
31 'rollback',
32+ 'session_store',
33 'SQLBase',
34 'sqlvalues',
35 'StupidCache',
36 'ZopelessTransactionManager',
37-]
38+ ]
39
40
41 from datetime import datetime
42@@ -578,7 +579,7 @@
43
44 """
45 if not isinstance(x, basestring):
46- raise TypeError, 'Not a string (%s)' % type(x)
47+ raise TypeError('Not a string (%s)' % type(x))
48 return quote(x).replace('%', r'\\%').replace('_', r'\\_')
49
50
51@@ -693,6 +694,7 @@
52 clause = clause.replace('?', '%s') % sqlvalues(*parameters)
53 return clause
54
55+
56 def flush_database_updates():
57 """Flushes all pending database updates.
58
59@@ -833,6 +835,7 @@
60 DEPRECATED - use of this class is deprecated in favour of using
61 Store.execute().
62 """
63+
64 def __init__(self):
65 self._connection = _get_sqlobject_store()._connection
66 self._result = None
67@@ -861,3 +864,8 @@
68 if self._result is not None:
69 self._result.close()
70 self._result = None
71+
72+
73+def session_store():
74+ """Return a store connected to the session DB."""
75+ return getUtility(IZStorm).get('session', 'launchpad-session:')
76
77=== modified file 'lib/canonical/launchpad/browser/librarian.py'
78--- lib/canonical/launchpad/browser/librarian.py 2010-09-03 07:53:11 +0000
79+++ lib/canonical/launchpad/browser/librarian.py 2010-09-07 04:07:54 +0000
80@@ -11,7 +11,9 @@
81 'LibraryFileAliasMD5View',
82 'LibraryFileAliasView',
83 'ProxiedLibraryFileAlias',
84+ 'SafeStreamOrRedirectLibraryFileAliasView',
85 'StreamOrRedirectLibraryFileAliasView',
86+ 'RedirectPerhapsWithTokenLibraryFileAliasView',
87 ]
88
89 import os
90@@ -44,6 +46,7 @@
91 filechunks,
92 guess_librarian_encoding,
93 )
94+from lp.services.features import getFeatureFlag
95
96
97 class LibraryFileAliasView(LaunchpadView):
98@@ -77,11 +80,29 @@
99 return '%s %s' % (self.context.content.md5, self.context.filename)
100
101
102-class StreamOrRedirectLibraryFileAliasView(LaunchpadView):
103+class MixedFileAliasView(LaunchpadView):
104 """Stream or redirects to `ILibraryFileAlias`.
105
106- It streams the contents of restricted library files or redirects
107- to public ones.
108+ If the file is public, it will redirect to the files http url.
109+
110+ Otherwise if the feature flag publicrestrictedlibrarian is set to 'on' this
111+ will allocate a token and redirect to the aliases private url.
112+
113+ Otherwise it will proxy the file in the appserver.
114+
115+ Once we no longer have any proxy code at all it should be possible to
116+ consolidate this with LibraryFileAliasView.
117+
118+ Note that streaming restricted files is a security concern - they show up
119+ in the launchpad.net domain rather than launchpadlibrarian.net and thus
120+ we have to take special care about their origin.
121+ SafeStreamOrRedirectLibraryFileAliasView is used when we do not trust the
122+ content, otherwise StreamOrRedirectLibraryFileAliasView. We are working
123+ to remove both of these views entirely, but some transition will be
124+ needed.
125+
126+ The context provides a file-like interface - it can be opened and closed
127+ and read from.
128 """
129 implements(IBrowserPublisher)
130
131@@ -91,6 +112,8 @@
132 # the shell environment changes. Download the library file
133 # content into a local temporary file. Finally, restore original
134 # proxy-settings and refresh the urllib2 opener.
135+ # XXX: This is not threadsafe, so two calls at once will collide and
136+ # can then corrupt the variable. bug=395960
137 original_proxy = os.getenv('http_proxy')
138 try:
139 if original_proxy is not None:
140@@ -137,25 +160,65 @@
141 return tmp_file
142
143 def browserDefault(self, request):
144- """Decides whether to redirect or stream the file content.
145+ """Decides how to deliver the file.
146+
147+ The options are:
148+ - redirect to the contexts http url
149+ - redirect to a time limited secure url
150+ - stream the file content.
151
152 Only restricted file contents are streamed, finishing the traversal
153 chain with this view. If the context file is public return the
154 appropriate `RedirectionView` for its HTTP url.
155 """
156+ # Perhaps we should give a 404 at this point rather than asserting?
157+ # If someone has a page open with an attachment link, then someone
158+ # else deletes the attachment, this is a normal situation, not an
159+ # error. -- RBC 20100726.
160 assert not self.context.deleted, (
161- "StreamOrRedirectLibraryFileAliasView can not operate on "
162- "deleted librarian files, since their URL is undefined.")
163-
164- if self.context.restricted:
165+ "MixedFileAliasView can not operate on deleted librarian files, "
166+ "since their URL is undefined.")
167+ if not self.context.restricted:
168+ # Public file, just point the client at the right place.
169+ return RedirectionView(self.context.http_url, self.request), ()
170+ if getFeatureFlag(u'publicrestrictedlibrarian') != 'on':
171+ # Restricted file and we have not enabled the public
172+ # restricted librarian yet :- deliver inline.
173+ self._when_streaming()
174 return self, ()
175-
176- return RedirectionView(self.context.http_url, self.request), ()
177+ # Avoids a circular import seen in
178+ # scripts/ftests/librarianformatter.txt
179+ from canonical.launchpad.database.librarian import TimeLimitedToken
180+ # Allocate a token for the file to be accessed for a limited time and
181+ # redirect the client to the file.
182+ token = TimeLimitedToken.allocate(self.context.private_url)
183+ final_url = self.context.private_url + '?token=%s' % token
184+ return RedirectionView(final_url, self.request), ()
185
186 def publishTraverse(self, request, name):
187- """See `IBrowserPublisher`."""
188+ """See `IBrowserPublisher` - can't traverse below a file."""
189 raise NotFound(name, self.context)
190
191+ def _when_streaming(self):
192+ """Hook for SafeStreamOrRedirectLibraryFileAliasView."""
193+
194+
195+
196+class SafeStreamOrRedirectLibraryFileAliasView(MixedFileAliasView):
197+ """A view for Librarian files that sets the content disposition header."""
198+
199+ def _when_streaming(self):
200+ super(SafeStreamOrRedirectLibraryFileAliasView, self)._when_streaming()
201+ self.request.response.setHeader(
202+ 'Content-Disposition', 'attachment')
203+
204+
205+# The eventual name of MixedFileAliasView once the proxy code
206+# is ripped out.
207+RedirectPerhapsWithTokenLibraryFileAliasView = MixedFileAliasView
208+# The name for the old behaviour being removed:
209+StreamOrRedirectLibraryFileAliasView = MixedFileAliasView
210+
211
212 class DeletedProxiedLibraryFileAlias(NotFound):
213 """Raised when a deleted `ProxiedLibraryFileAlias` is accessed."""
214@@ -192,7 +255,20 @@
215
216
217 class ProxiedLibraryFileAlias:
218- """A `LibraryFileAlias` proxied via webapp.
219+ """A `LibraryFileAlias` decorator for use in URL generation.
220+
221+ The URL's output by this decorator will always point at the webapp. This is
222+ useful when:
223+ - we are proxying files via the webapp (as we do at the moment)
224+ - when the webapp has to be contacted to get access to a file (the case
225+ for restricted files in the future)
226+ - files might change from public to private and thus not work even if the
227+ user has access to the once its private, unless they go via the webapp.
228+
229+ This should be used anywhere we are outputting URL's to LibraryFileAliases
230+ other than directly in rendered pages. For rendered pages, using a
231+ LibraryFileAlias directly is OK as at that point the status of the file
232+ is know.
233
234 Overrides `ILibraryFileAlias.http_url` to always point to the webapp URL,
235 even when called from the webservice domain.
236
237=== added file 'lib/canonical/launchpad/database/ftests/test_timelimitedtoken.py'
238--- lib/canonical/launchpad/database/ftests/test_timelimitedtoken.py 1970-01-01 00:00:00 +0000
239+++ lib/canonical/launchpad/database/ftests/test_timelimitedtoken.py 2010-09-07 04:07:54 +0000
240@@ -0,0 +1,29 @@
241+# Copyright 2010 Canonical Ltd. This software is licensed under the
242+# GNU Affero General Public License version 3 (see the file LICENSE).
243+
244+"""Tests for TimeLimitedToken."""
245+
246+__metaclass__ = type
247+
248+import testtools
249+import transaction
250+
251+from canonical.database.sqlbase import session_store
252+from canonical.launchpad.database.librarian import TimeLimitedToken
253+from canonical.testing import LaunchpadFunctionalLayer
254+
255+
256+class TestLibraryFileAlias(testtools.TestCase):
257+
258+ layer = LaunchpadFunctionalLayer
259+
260+ def test_allocate(self):
261+ store = session_store()
262+ store.find(TimeLimitedToken).remove()
263+ token1 = TimeLimitedToken.allocate('foo://')
264+ token2 = TimeLimitedToken.allocate('foo://')
265+ # We must get unique tokens
266+ self.assertNotEqual(token1, token2)
267+ # They must be bytestrings (as a surrogate for valid url fragment')
268+ self.assertIsInstance(token1, str)
269+ self.assertIsInstance(token2, str)
270
271=== modified file 'lib/canonical/launchpad/database/librarian.py'
272--- lib/canonical/launchpad/database/librarian.py 2010-08-20 20:31:18 +0000
273+++ lib/canonical/launchpad/database/librarian.py 2010-09-07 04:07:54 +0000
274@@ -10,12 +10,16 @@
275 'LibraryFileAliasSet',
276 'LibraryFileContent',
277 'LibraryFileDownloadCount',
278+ 'TimeLimitedToken',
279 ]
280
281 from datetime import (
282 datetime,
283 timedelta,
284 )
285+from hashlib import md5
286+import random
287+from urlparse import urlparse
288
289 from lazr.delegates import delegates
290 import pytz
291@@ -26,6 +30,7 @@
292 SQLRelatedJoin,
293 StringCol,
294 )
295+import storm.base
296 from storm.locals import (
297 Date,
298 Desc,
299@@ -48,7 +53,10 @@
300 UTC_NOW,
301 )
302 from canonical.database.datetimecol import UtcDateTimeCol
303-from canonical.database.sqlbase import SQLBase
304+from canonical.database.sqlbase import (
305+ session_store,
306+ SQLBase,
307+ )
308 from canonical.launchpad.interfaces import (
309 ILibraryFileAlias,
310 ILibraryFileAliasSet,
311@@ -127,8 +135,15 @@
312 return url
313 return url.replace('http', 'https', 1)
314
315+ @property
316+ def private_url(self):
317+ """See ILibraryFileAlias.https_url"""
318+ return self.client.getURLForAlias(self.id, secure=True)
319+
320 def getURL(self):
321 """See ILibraryFileAlias.getURL"""
322+ if self.restricted:
323+ return self.private_url
324 if config.librarian.use_https:
325 return self.https_url
326 else:
327@@ -285,3 +300,56 @@
328 count = Int(allow_none=False)
329 country_id = Int(name='country', allow_none=True)
330 country = Reference(country_id, 'Country.id')
331+
332+
333+class TimeLimitedToken(storm.base.Storm):
334+ """A time limited access token for accessing a private file."""
335+
336+ __storm_table__ = 'TimeLimitedToken'
337+
338+ created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
339+ path = StringCol(notNull=True)
340+ token = StringCol(notNull=True)
341+ __storm_primary__ = ("path", "token")
342+
343+ def __init__(self, path, token, created=None):
344+ """Create a TimeLimitedToken."""
345+ if created is not None:
346+ self.created = created
347+ self.path = path
348+ self.token = token
349+
350+ @staticmethod
351+ def allocate(url):
352+ """Allocate a token for url path in the librarian.
353+
354+ :param url: A url bytestring. e.g.
355+ https://i123.restricted.launchpad-librarian.net/123/foo.txt
356+ Note that the token is generated for 123/foo.txt
357+ :return: A url fragment token ready to be attached to the url.
358+ e.g. 'a%20token'
359+ """
360+ # We use random.random to get a string which varies reasonably, and we
361+ # hash it to distribute it widely and get a easily copy and pastable
362+ # single string (nice for debugging). The randomness is not a key
363+ # factor here: as long as tokens are not guessable, they are hidden by
364+ # https, not exposed directly in the API (tokens will be allocated by
365+ # the appropriate objects), not by direct access to the
366+ # TimeLimitedToken class.
367+ baseline = str(random.random())
368+ hashed = md5(baseline).hexdigest()
369+ token = hashed
370+ store = session_store()
371+ path = TimeLimitedToken.url_to_token_path(url)
372+ store.add(TimeLimitedToken(path, token))
373+ # The session isn't part of the main transaction model, and in fact it
374+ # has autocommit on. The commit here is belts and bracers: after
375+ # allocation the external librarian must be able to serve the file
376+ # immediately.
377+ store.commit()
378+ return token
379+
380+ @staticmethod
381+ def url_to_token_path(url):
382+ """Return the token path used for authorising access to url."""
383+ return urlparse(url)[2]
384
385=== modified file 'lib/canonical/launchpad/doc/librarian.txt'
386--- lib/canonical/launchpad/doc/librarian.txt 2010-09-04 05:01:17 +0000
387+++ lib/canonical/launchpad/doc/librarian.txt 2010-09-07 04:07:54 +0000
388@@ -1,5 +1,31 @@
389 = Librarian Access =
390
391+The librarian is a file storage service for launchpad. Conceptually similar to
392+other file storage API's like S3, it is used to store binary or large content -
393+bug attachments, package builds, images and so on.
394+
395+Content in the librarian can be exposed at different urls. To expose some
396+content use a LibraryFileAlias. Private content is supported as well - for that
397+tokens are added to permit access for a limited time by a client - each time a
398+client attempts to dereference a private LibraryFileAlias a token is emitted.
399+
400+= Deployment notes =
401+
402+(These may seem a bit out of place - they are, but they need to be written down
403+somewhere, and the deployment choices inform the implementation choices).
404+
405+The basics are simple: The librarian talks to clients. However restricted file
406+access makes things a little more complex. As the librarian itself doesn't do
407+SSL processing, and we want restricted files to be kept confidential the
408+librarian will need a hint from the SSL front end that SSL was in fact used.
409+The semi standard header Front-End-Https can be used for this if we filter it
410+in incoming requests from clients.
411+
412+== setUp ==
413+
414+ >>> from canonical.database.sqlbase import session_store
415+ >>> from canonical.launchpad.database.librarian import TimeLimitedToken
416+
417 == High Level ==
418
419 >>> from canonical.launchpad.interfaces import ILibraryFileAliasSet
420@@ -58,7 +84,9 @@
421 ... ) is not None
422 True
423
424-Librarian also serves the same file through https.
425+Librarian also serves the same file through https, we use this for branding
426+and similar inline-presented objects which would trigger security warnings on
427+https pages otherwise.
428
429 >>> re.search(
430 ... r'^https://localhost:58000/\d+/text.txt$', alias.https_url
431@@ -83,7 +111,7 @@
432 ... """)
433 >>> config.push('test', test_data)
434 >>> re.search(
435- ... r'^https://localhost:58000/\d+/text.txt$', alias.getURL()
436+ ... r'^https://localhost:58000/\d+/text.txt$', alias.https_url
437 ... ) is not None
438 True
439
440@@ -183,6 +211,15 @@
441 >>> re.search(r'^http://localhost:58000/\d+/text.txt$', url) is not None
442 True
443
444+When secure=True, the returned url has the id as part of the domain name and
445+the protocol is https:
446+
447+ >>> expected = 'https://i%d.restricted.localhost:58000/%d/text.txt' % (
448+ ... aid, aid)
449+ >>> expected == client.getURLForAlias(aid, secure=True)
450+ True
451+
452+
453 Librarian reads are logged in the request timeline.
454
455 >>> from canonical.lazr.utils import get_current_browser_request
456@@ -465,10 +502,9 @@
457 True
458
459
460-== StreamOrRedirectLibraryFileAliasView ==
461+== File views setup ==
462
463-This special view allows call sites to stream restricted `LibraryFileAlias`
464-contents directly from launchpad instances.
465+We need some files to test different ways of accessing them.
466
467 >>> filename = 'public.txt'
468 >>> content = 'PUBLIC'
469@@ -482,11 +518,141 @@
470 ... filename, len(content), StringIO(content), 'text/plain',
471 ... NEVER_EXPIRES, restricted=True)
472
473+ # Create a new LibraryFileAlias not referencing any LibraryFileContent
474+ # record. Such records are considered as being deleted.
475+ >>> from canonical.launchpad.database import LibraryFileAlias
476+ >>> from canonical.launchpad.webapp.interfaces import (
477+ ... IStoreSelector, MAIN_STORE, MASTER_FLAVOR)
478+
479+ >>> store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
480+ >>> deleted_file = LibraryFileAlias(
481+ ... content=None, filename='deleted.txt',
482+ ... mimetype='text/plain')
483+ >>> ignore = store.add(deleted_file)
484+
485 Commit the just-created files.
486
487 >>> from canonical.database.sqlbase import commit
488 >>> commit()
489
490+ >>> deleted_file = getUtility(ILibraryFileAliasSet)[deleted_file.id]
491+ >>> print deleted_file.deleted
492+ True
493+
494+Clear out existing tokens.
495+
496+ >>> _ = session_store().find(TimeLimitedToken).remove()
497+
498+== RedirectPerhapsWithTokenLibraryFileAliasView ==
499+
500+This is the eventual name of StreamOrRedirectLibraryFileAliasView once we have
501+migrated over to the token based approach. For now the code for the two things
502+is interleaved: zope's registration system is too static to permit doing it
503+cleanly with separate classes.
504+
505+To enable the behaviour we want to test while its controlled by a feature flag,
506+we must enable a feature flag for it:
507+
508+ >>> from lp.services.features.model import FeatureFlag, getFeatureStore
509+ >>> ignore = getFeatureStore().add(FeatureFlag(
510+ ... scope=u'default', flag=u'publicrestrictedlibrarian', value=u'on',
511+ ... priority=1))
512+ >>> transaction.commit()
513+
514+This special view allows granting access to restricted `LibraryFileAlias`
515+objects on the launchpadlibrarian. All requests get redirected, restricted
516+files have an access token allocated.
517+
518+This view implements `IBrowserPublisher` which allows us to use it in
519+traversals.
520+
521+ >>> from canonical.launchpad.browser.librarian import (
522+ ... RedirectPerhapsWithTokenLibraryFileAliasView)
523+
524+ >>> empty_request = LaunchpadTestRequest()
525+
526+XXX bug=631884 we have to establish the flags object manually for testing.
527+
528+ >>> from lp.services.features.webapp import ScopesFromRequest
529+ >>> from lp.services.features.flags import FeatureController
530+ >>> from lp.services.features import per_thread
531+ >>> per_thread.features = FeatureController(
532+ ... ScopesFromRequest(empty_request).lookup)
533+
534+ >>> restricted_view = RedirectPerhapsWithTokenLibraryFileAliasView(
535+ ... restricted_file, empty_request)
536+
537+ >>> from zope.publisher.interfaces.browser import IBrowserPublisher
538+ >>> verifyObject(IBrowserPublisher, restricted_view)
539+ True
540+
541+When the context file is a restricted `LibraryFileAlias`, traversal causes an
542+access token to be allocated and a redirection to https on a unique domain to
543+be issued.
544+
545+ >>> view, extra = restricted_view.browserDefault(empty_request)
546+ >>> print view
547+ <...RedirectionView...>
548+ >>> view.target
549+ 'https://...restricted.txt?token=...'
550+ >>> private_path = TimeLimitedToken.url_to_token_path(
551+ ... restricted_file.private_url)
552+ >>> view.target.endswith(session_store().find(TimeLimitedToken,
553+ ... path=private_path).any().token)
554+ True
555+
556+The domain for the target starts with the id of the LibraryFileAlias.
557+
558+ >>> view.target.startswith('https://i%d.restricted.' % restricted_file.id)
559+ True
560+
561+Otherwise, no token is allocated and a redirection is issued to the http url.
562+
563+ >>> public_view = RedirectPerhapsWithTokenLibraryFileAliasView(
564+ ... public_file, empty_request)
565+ >>> verifyObject(IBrowserPublisher, public_view)
566+ True
567+ >>> view, extra = public_view.browserDefault(empty_request)
568+ >>> print view
569+ <...RedirectionView ...>
570+ >>> view.target == public_file.http_url
571+ True
572+ >>> view.target
573+ 'http://.../public.txt'
574+
575+Deleted files cannot be handled by the view and it raises an appropriate
576+AssertionError when it gets traversed.
577+
578+ >>> deleted_view = RedirectPerhapsWithTokenLibraryFileAliasView(
579+ ... deleted_file, empty_request)
580+
581+ >>> view, extra = deleted_view.browserDefault(empty_request)
582+ Traceback (most recent call last):
583+ ...
584+ AssertionError: MixedFileAliasView can not operate on deleted librarian
585+ files, since their URL is undefined.
586+
587+
588+As RedirectPerhapsWithTokenLibraryFileAliasView is
589+StreamOrRedirectLibraryFileAliasView influence by a feature flag, until the
590+proxy code is removed, we must remove the feature to test the old behaviour.
591+
592+ >>> ignore = getFeatureStore().find(FeatureFlag,
593+ ... FeatureFlag.flag==u'publicrestrictedlibrarian').remove()
594+ >>> transaction.commit()
595+
596+Because the flags code aggressively caches, we have to do a small dance to
597+convince it a new request has started too.
598+
599+ >>> empty_request = LaunchpadTestRequest()
600+ >>> per_thread.features = FeatureController(
601+ ... ScopesFromRequest(empty_request).lookup)
602+
603+== StreamOrRedirectLibraryFileAliasView ==
604+
605+This special view allows call sites to stream restricted `LibraryFileAlias`
606+contents directly from launchpad instances.
607+
608 This view implements `IBrowserPublisher` which allows us to use it in
609 traversals.
610
611@@ -505,6 +671,8 @@
612 traversal chain is finished with the view object itself.
613
614 >>> view, extra = restricted_view.browserDefault(empty_request)
615+ >>> if view != restricted_view:
616+ ... print view
617 >>> view == restricted_view
618 True
619
620@@ -586,31 +754,14 @@
621 Files in this condition cannot be handled by the view and it raises an
622 appropriate AssertionError when it gets traversed.
623
624- # Create a new LibraryFileAlias not referencing any LibraryFileContent
625- # record. Such records are considered as being deleted.
626- >>> from canonical.launchpad.database import LibraryFileAlias
627- >>> from canonical.launchpad.webapp.interfaces import (
628- ... IStoreSelector, MAIN_STORE, MASTER_FLAVOR)
629-
630- >>> store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
631- >>> deleted_file = LibraryFileAlias(
632- ... content=None, filename='deleted.txt',
633- ... mimetype='text/plain')
634- >>> ignore = store.add(deleted_file)
635- >>> store.commit()
636-
637- >>> deleted_file = getUtility(ILibraryFileAliasSet)[deleted_file.id]
638- >>> print deleted_file.deleted
639- True
640-
641 >>> deleted_view = StreamOrRedirectLibraryFileAliasView(
642 ... deleted_file, empty_request)
643
644 >>> view, extra = deleted_view.browserDefault(empty_request)
645 Traceback (most recent call last):
646 ...
647- AssertionError: StreamOrRedirectLibraryFileAliasView can not
648- operate on deleted librarian files, since their URL is undefined.
649+ AssertionError: MixedFileAliasView can not operate on deleted librarian
650+ files, since their URL is undefined.
651
652
653 == LibraryFileAliasMD5View ==
654
655=== modified file 'lib/canonical/launchpad/ftests/test_libraryfilealias.py'
656--- lib/canonical/launchpad/ftests/test_libraryfilealias.py 2010-08-20 20:31:18 +0000
657+++ lib/canonical/launchpad/ftests/test_libraryfilealias.py 2010-09-07 04:07:54 +0000
658@@ -6,8 +6,8 @@
659 __metaclass__ = type
660
661 from cStringIO import StringIO
662+
663 import unittest
664-
665 import transaction
666 from zope.component import getUtility
667
668@@ -45,8 +45,3 @@
669 # the remaining content. If it's reset, the file will be auto-opened
670 # and its whole content will be returned.
671 self.assertEquals(self.text_content, self.file_alias.read())
672-
673-
674-def test_suite():
675- return unittest.TestLoader().loadTestsFromName(__name__)
676-
677
678=== modified file 'lib/canonical/launchpad/interfaces/librarian.py'
679--- lib/canonical/launchpad/interfaces/librarian.py 2010-08-20 20:31:18 +0000
680+++ lib/canonical/launchpad/interfaces/librarian.py 2010-09-07 04:07:54 +0000
681@@ -80,13 +80,17 @@
682 # byte strings.
683 http_url = Attribute(_("The http URL to this file"))
684 https_url = Attribute(_("The https URL to this file"))
685+ private_url = Attribute(_("The secure URL to this file (private files)"))
686
687 def getURL():
688 """Return this file's http or https URL.
689
690+ If the file is a restricted file, the private_url will be returned,
691+ which is on https and uses unique domains per file alias.
692+
693 The generated URL will be https if the use_https config variable is
694 set, in order to prevent warnings about insecure objects from
695- happening in some browsers.
696+ happening in some browsers (this is used for, e.g., branding).
697
698 If config.launchpad.virtual_host.use_https is set, then return the
699 https URL. Otherwise return the http URL.
700
701=== modified file 'lib/canonical/launchpad/interfaces/lpstorm.py'
702--- lib/canonical/launchpad/interfaces/lpstorm.py 2009-06-25 05:30:52 +0000
703+++ lib/canonical/launchpad/interfaces/lpstorm.py 2010-09-07 04:07:54 +0000
704@@ -32,4 +32,3 @@
705
706 class IMasterObject(IDBObject):
707 """A Storm database object associated with its master Store."""
708-
709
710=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
711--- lib/canonical/launchpad/scripts/garbo.py 2010-08-20 20:31:18 +0000
712+++ lib/canonical/launchpad/scripts/garbo.py 2010-09-07 04:07:54 +0000
713@@ -29,10 +29,14 @@
714 from canonical.database.constants import THIRTY_DAYS_AGO
715 from canonical.database.sqlbase import (
716 cursor,
717+ session_store,
718 sqlvalues,
719 )
720 from canonical.launchpad.database.emailaddress import EmailAddress
721-from canonical.launchpad.database.librarian import LibraryFileAlias
722+from canonical.launchpad.database.librarian import (
723+ LibraryFileAlias,
724+ TimeLimitedToken,
725+ )
726 from canonical.launchpad.database.oauth import OAuthNonce
727 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
728 from canonical.launchpad.interfaces import IMasterStore
729@@ -685,6 +689,43 @@
730 transaction.commit()
731
732
733+class OldTimeLimitedTokenDeleter(TunableLoop):
734+ """Delete expired url access tokens from the session DB."""
735+
736+ maximum_chunk_size = 24*60*60 # 24 hours in seconds.
737+
738+ def __init__(self, log, abort_time=None):
739+ super(OldTimeLimitedTokenDeleter, self).__init__(log, abort_time)
740+ self.store = session_store()
741+ self._update_oldest()
742+
743+ def _update_oldest(self):
744+ self.oldest_age = self.store.execute("""
745+ SELECT COALESCE(EXTRACT(EPOCH FROM
746+ CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
747+ - MIN(created)), 0)
748+ FROM TimeLimitedToken
749+ """).get_one()[0]
750+
751+ def isDone(self):
752+ return self.oldest_age <= ONE_DAY_IN_SECONDS
753+
754+ def __call__(self, chunk_size):
755+ self.oldest_age = max(
756+ ONE_DAY_IN_SECONDS, self.oldest_age - chunk_size)
757+
758+ self.log.debug(
759+ "Removed TimeLimitedToken rows older than %d seconds"
760+ % self.oldest_age)
761+ self.store.find(
762+ TimeLimitedToken,
763+ TimeLimitedToken.created < SQL(
764+ "CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - interval '%d seconds'"
765+ % ONE_DAY_IN_SECONDS)).remove()
766+ transaction.commit()
767+ self._update_oldest()
768+
769+
770 class SuggestiveTemplatesCacheUpdater(TunableLoop):
771 """Refresh the SuggestivePOTemplate cache.
772
773@@ -806,13 +847,14 @@
774 class DailyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
775 script_name = 'garbo-daily'
776 tunable_loops = [
777+ BranchJobPruner,
778+ BugNotificationPruner,
779+ BugWatchActivityPruner,
780 CodeImportResultPruner,
781- RevisionAuthorEmailLinker,
782 HWSubmissionEmailLinker,
783- BugNotificationPruner,
784- BranchJobPruner,
785- BugWatchActivityPruner,
786 ObsoleteBugAttachmentDeleter,
787+ OldTimeLimitedTokenDeleter,
788+ RevisionAuthorEmailLinker,
789 SuggestiveTemplatesCacheUpdater,
790 ]
791 experimental_tunable_loops = [
792
793=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
794--- lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-08-20 20:31:18 +0000
795+++ lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-09-07 04:07:54 +0000
796@@ -23,11 +23,12 @@
797 from zope.security.proxy import removeSecurityProxy
798
799 from canonical.config import config
800+from canonical.database import sqlbase
801 from canonical.database.constants import (
802 THIRTY_DAYS_AGO,
803 UTC_NOW,
804 )
805-from canonical.database.sqlbase import quote
806+from canonical.launchpad.database.librarian import TimeLimitedToken
807 from canonical.launchpad.database.message import Message
808 from canonical.launchpad.database.oauth import OAuthNonce
809 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
810@@ -561,6 +562,26 @@
811 LaunchpadZopelessLayer.switchDbUser('testadmin')
812 self.assertEqual(bug.attachments.count(), 0)
813
814+ def test_TimeLimitedTokenPruner(self):
815+ # Ensure there are no tokens
816+ store = sqlbase.session_store()
817+ map(store.remove, store.find(TimeLimitedToken))
818+ store.flush()
819+ self.assertEqual(0, len(list(store.find(TimeLimitedToken,
820+ path="sample path"))))
821+ # One to clean and one to keep
822+ store.add(TimeLimitedToken(path="sample path", token="foo",
823+ created=datetime(2008, 01, 01, tzinfo=UTC)))
824+ store.add(TimeLimitedToken(path="sample path", token="bar")),
825+ store.commit()
826+ self.assertEqual(2, len(list(store.find(TimeLimitedToken,
827+ path="sample path"))))
828+ self.runDaily()
829+ self.assertEqual(0, len(list(store.find(TimeLimitedToken,
830+ path="sample path", token="foo"))))
831+ self.assertEqual(1, len(list(store.find(TimeLimitedToken,
832+ path="sample path", token="bar"))))
833+
834 def test_CacheSuggestivePOTemplates(self):
835 LaunchpadZopelessLayer.switchDbUser('testadmin')
836 template = self.factory.makePOTemplate()
837@@ -571,6 +592,6 @@
838 SELECT count(*)
839 FROM SuggestivePOTemplate
840 WHERE potemplate = %s
841- """ % quote(template.id)).get_one()
842+ """ % sqlbase.quote(template.id)).get_one()
843
844 self.assertEqual(1, count)
845
846=== modified file 'lib/canonical/launchpad/utilities/looptuner.py'
847--- lib/canonical/launchpad/utilities/looptuner.py 2010-08-20 20:31:18 +0000
848+++ lib/canonical/launchpad/utilities/looptuner.py 2010-09-07 04:07:54 +0000
849@@ -314,6 +314,10 @@
850 self.log = log
851 self.abort_time = abort_time
852
853+ def isDone(self):
854+ """Return True when the TunableLoop is complete."""
855+ raise NotImplementedError(self.isDone)
856+
857 def run(self):
858 assert self.maximum_chunk_size is not None, (
859 "Did not override maximum_chunk_size.")
860
861=== modified file 'lib/canonical/launchpad/webapp/session.py'
862--- lib/canonical/launchpad/webapp/session.py 2010-08-20 20:31:18 +0000
863+++ lib/canonical/launchpad/webapp/session.py 2010-09-07 04:07:54 +0000
864@@ -13,6 +13,7 @@
865 from zope.session.http import CookieClientIdManager
866
867 from canonical.config import config
868+from canonical.database.sqlbase import session_store
869
870
871 SECONDS = 1
872@@ -77,7 +78,7 @@
873 # Secret is looked up here rather than in __init__, because
874 # we can't be sure the database connections are setup at that point.
875 if self._secret is None:
876- store = getUtility(IZStorm).get('session', 'launchpad-session:')
877+ store = session_store()
878 result = store.execute("SELECT secret FROM secret")
879 self._secret = result.get_one()[0]
880 return self._secret
881
882=== modified file 'lib/canonical/launchpad/zcml/librarian.zcml'
883--- lib/canonical/launchpad/zcml/librarian.zcml 2010-07-06 16:12:46 +0000
884+++ lib/canonical/launchpad/zcml/librarian.zcml 2010-09-07 04:07:54 +0000
885@@ -55,6 +55,11 @@
886 <allow interface="zope.publisher.interfaces.browser.IBrowserPublisher" />
887 </class>
888
889+ <class class="canonical.launchpad.browser.librarian.SafeStreamOrRedirectLibraryFileAliasView">
890+ <allow attributes="__call__" />
891+ <allow interface="zope.publisher.interfaces.browser.IBrowserPublisher" />
892+ </class>
893+
894 <adapter
895 factory="canonical.launchpad.database.librarian.LibraryFileAliasWithParent"
896 provides="canonical.launchpad.interfaces.librarian.ILibraryFileAliasWithParent"
897
898=== modified file 'lib/canonical/librarian/client.py'
899--- lib/canonical/librarian/client.py 2010-09-04 05:01:17 +0000
900+++ lib/canonical/librarian/client.py 2010-09-07 04:07:54 +0000
901@@ -23,7 +23,11 @@
902
903 from select import select
904 from socket import SOCK_STREAM, AF_INET
905-from urlparse import urljoin
906+from urlparse import (
907+ urlparse,
908+ urljoin,
909+ urlunparse,
910+ )
911
912 from storm.store import Store
913 from zope.interface import implements
914@@ -291,16 +295,17 @@
915 # raise DownloadFailed, 'Incomplete response'
916 # return paths
917
918- def _getPathForAlias(self, aliasID):
919+ def _getPathForAlias(self, aliasID, secure=False):
920 """Returns the path inside the librarian to talk about the given
921 alias.
922
923 :param aliasID: A unique ID for the alias
924-
925+ :param secure: Controls the behaviour when looking up restricted
926+ files. If False restricted files are only permitted when
927+ self.restricted is True. See getURLForAlias.
928 :returns: String path, url-escaped. Unicode is UTF-8 encoded before
929 url-escaping, as described in section 2.2.5 of RFC 2718.
930 None if the file has been deleted.
931-
932 :raises: DownloadFailed if the alias is invalid
933 """
934 from canonical.launchpad.database import LibraryFileAlias
935@@ -311,7 +316,7 @@
936 lfa = LibraryFileAlias.get(aliasID)
937 except SQLObjectNotFound:
938 raise DownloadFailed('Alias %d not found' % aliasID)
939- if self.restricted != lfa.restricted:
940+ if not secure and self.restricted != lfa.restricted:
941 raise DownloadFailed(
942 'Alias %d cannot be downloaded from this client.' % aliasID)
943 if lfa.deleted:
944@@ -319,18 +324,41 @@
945 return get_libraryfilealias_download_path(
946 aliasID, lfa.filename.encode('utf-8'))
947
948- def getURLForAlias(self, aliasID):
949+ def getURLForAlias(self, aliasID, secure=False):
950 """Returns the url for talking to the librarian about the given
951 alias.
952
953 :param aliasID: A unique ID for the alias
954-
955+ :param secure: If true generate https urls on unique domains for
956+ security.
957 :returns: String URL, or None if the file has expired and been deleted.
958 """
959- path = self._getPathForAlias(aliasID)
960+ # Note that the path is the same for both secure and insecure URLs -
961+ # this is deliberate: the server doesn't need to know about the original
962+ # Host the client provides, testing is easier as we don't need wildcard
963+ # https environments on dev machines.
964+ path = self._getPathForAlias(aliasID, secure=secure)
965 if path is None:
966 return None
967- base = self.download_url
968+ if not secure:
969+ base = self.download_url
970+ else:
971+ # Secure url generation is the same for both restricted and
972+ # unrestricted files aliases : it is used to give web clients (not
973+ # appservers) a url to use to access a file which is either
974+ # restricted (and so they will also need a TimeLimitedToken) or
975+ # is suspected hostile (and so it should be isolated on its own
976+ # domain). Note that only the former is currently used in LP.
977+ # The algorithm is:
978+ # parse the url
979+ download_url = config.librarian.download_url
980+ parsed = list(urlparse(download_url))
981+ # Force the scheme to https
982+ parsed[0] = 'https'
983+ # Insert the alias id (which is a unique key, thus unique) in the
984+ # netloc
985+ parsed[1] = ('i%d.restricted.' % aliasID) + parsed[1]
986+ base = urlunparse(parsed)
987 return urljoin(base, path)
988
989 def _getURLForDownload(self, aliasID):
990
991=== modified file 'lib/canonical/librarian/db.py'
992--- lib/canonical/librarian/db.py 2009-11-24 15:36:44 +0000
993+++ lib/canonical/librarian/db.py 2010-09-07 04:07:54 +0000
994@@ -13,12 +13,19 @@
995 from psycopg2.extensions import TransactionRollbackError
996 from sqlobject.sqlbuilder import AND
997 from storm.exceptions import DisconnectionError, IntegrityError
998+from storm.expr import SQL
999 import transaction
1000 from twisted.python.util import mergeFunctionMetadata
1001
1002-from canonical.database.sqlbase import reset_store
1003+from canonical.database.sqlbase import (
1004+ reset_store,
1005+ session_store,
1006+ )
1007 from canonical.launchpad.database.librarian import (
1008- LibraryFileContent, LibraryFileAlias)
1009+ LibraryFileAlias,
1010+ LibraryFileContent,
1011+ TimeLimitedToken,
1012+ )
1013
1014
1015 RETRY_ATTEMPTS = 3
1016@@ -98,19 +105,40 @@
1017 def lookupBySHA1(self, digest):
1018 return [fc.id for fc in LibraryFileContent.selectBy(sha1=digest)]
1019
1020- def getAlias(self, aliasid):
1021+ def getAlias(self, aliasid, token, path):
1022 """Returns a LibraryFileAlias, or raises LookupError.
1023
1024 A LookupError is raised if no record with the given ID exists
1025 or if not related LibraryFileContent exists.
1026+
1027+ :param token: The token for the file. If None no token is present.
1028+ When a token is supplied, it is looked up with path.
1029+ :param path: The path the request is for, unused unless a token
1030+ is supplied; when supplied it must match the token. The
1031+ value of path is expected to be that from a twisted request.args
1032+ e.g. /foo/bar.
1033 """
1034+ restricted = self.restricted
1035+ if token and path:
1036+ # with a token and a path we may be able to serve restricted files
1037+ # on the public port.
1038+ store = session_store()
1039+ token_found = store.find(TimeLimitedToken,
1040+ SQL("age(created) < interval '1 day'"),
1041+ TimeLimitedToken.token==token,
1042+ TimeLimitedToken.path==path).is_empty()
1043+ store.reset()
1044+ if token_found:
1045+ raise LookupError("Token stale/pruned/path mismatch")
1046+ else:
1047+ restricted = True
1048 alias = LibraryFileAlias.selectOne(AND(
1049 LibraryFileAlias.q.id==aliasid,
1050 LibraryFileAlias.q.contentID==LibraryFileContent.q.id,
1051- LibraryFileAlias.q.restricted==self.restricted,
1052+ LibraryFileAlias.q.restricted==restricted,
1053 ))
1054 if alias is None:
1055- raise LookupError
1056+ raise LookupError("No file alias with LibraryFileContent")
1057 return alias
1058
1059 def getAliases(self, fileid):
1060
1061=== modified file 'lib/canonical/librarian/ftests/test_db.py'
1062--- lib/canonical/librarian/ftests/test_db.py 2009-11-24 16:54:45 +0000
1063+++ lib/canonical/librarian/ftests/test_db.py 2010-09-07 04:07:54 +0000
1064@@ -39,7 +39,7 @@
1065 sorted(library.lookupBySHA1('deadbeef')))
1066
1067 aliasID = library.addAlias(fileID, 'file1', 'text/unknown')
1068- alias = library.getAlias(aliasID)
1069+ alias = library.getAlias(aliasID, None, '/')
1070 self.assertEqual('file1', alias.filename)
1071 self.assertEqual('text/unknown', alias.mimetype)
1072
1073@@ -65,42 +65,44 @@
1074 # Library.getAlias() returns the LibrarayFileAlias for a given
1075 # LibrarayFileAlias ID.
1076 library = db.Library(restricted=False)
1077- alias = library.getAlias(1)
1078+ alias = library.getAlias(1, None, '/')
1079 self.assertEqual(1, alias.id)
1080
1081 def test_getAlias_no_such_record(self):
1082 # Library.getAlias() raises a LookupError, if no record with
1083 # the given ID exists.
1084 library = db.Library(restricted=False)
1085- self.assertRaises(LookupError, library.getAlias, -1)
1086+ self.assertRaises(LookupError, library.getAlias, -1, None, '/')
1087
1088 def test_getAlias_content_is_null(self):
1089 # Library.getAlias() raises a LookupError, if no content
1090 # record for the given alias exists.
1091 library = db.Library(restricted=False)
1092- alias = library.getAlias(1)
1093+ alias = library.getAlias(1, None, '/')
1094 alias.content = None
1095- self.assertRaises(LookupError, library.getAlias, 1)
1096+ self.assertRaises(LookupError, library.getAlias, 1, None, '/')
1097
1098 def test_getAlias_content_is_none(self):
1099 # Library.getAlias() raises a LookupError, if the matching
1100 # record does not reference any LibraryFileContent record.
1101 library = db.Library(restricted=False)
1102- alias = library.getAlias(1)
1103+ alias = library.getAlias(1, None, '/')
1104 alias.content = None
1105- self.assertRaises(LookupError, library.getAlias, 1)
1106+ self.assertRaises(LookupError, library.getAlias, 1, None, '/')
1107
1108 def test_getAlias_content_wrong_library(self):
1109 # Library.getAlias() raises a LookupError, if a restricted
1110 # library looks up a unrestricted LibraryFileAlias and
1111 # vice versa.
1112 restricted_library = db.Library(restricted=True)
1113- self.assertRaises(LookupError, restricted_library.getAlias, 1)
1114+ self.assertRaises(
1115+ LookupError, restricted_library.getAlias, 1, None, '/')
1116
1117 unrestricted_library = db.Library(restricted=False)
1118- alias = unrestricted_library.getAlias(1)
1119+ alias = unrestricted_library.getAlias(1, None, '/')
1120 alias.restricted = True
1121- self.assertRaises(LookupError, unrestricted_library.getAlias, 1)
1122+ self.assertRaises(
1123+ LookupError, unrestricted_library.getAlias, 1, None, '/')
1124
1125 def test_getAliases(self):
1126 # Library.getAliases() returns a sequence
1127@@ -119,7 +121,7 @@
1128 # Library.getAliases() does not return records which do not
1129 # reference any LibraryFileContent record.
1130 library = db.Library(restricted=False)
1131- alias = library.getAlias(1)
1132+ alias = library.getAlias(1, None, '/')
1133 alias.content = None
1134 aliases = library.getAliases(1)
1135 expected_aliases = [
1136@@ -132,7 +134,7 @@
1137 # LibrarayFileAlias records when called from a unrestricted
1138 # library and vice versa.
1139 unrestricted_library = db.Library(restricted=False)
1140- alias = unrestricted_library.getAlias(1)
1141+ alias = unrestricted_library.getAlias(1, None, '/')
1142 alias.restricted = True
1143
1144 aliases = unrestricted_library.getAliases(1)
1145
1146=== modified file 'lib/canonical/librarian/ftests/test_storage.py'
1147--- lib/canonical/librarian/ftests/test_storage.py 2010-02-09 00:17:40 +0000
1148+++ lib/canonical/librarian/ftests/test_storage.py 2010-09-07 04:07:54 +0000
1149@@ -71,7 +71,7 @@
1150 fileid, aliasid = newfile.store()
1151
1152 # Check that its alias has the right mimetype
1153- fa = self.storage.getFileAlias(aliasid)
1154+ fa = self.storage.getFileAlias(aliasid, None, '/')
1155 self.assertEqual('text/unknown', fa.mimetype)
1156
1157 # Re-add the same file, with the same name and mimetype...
1158@@ -81,7 +81,8 @@
1159 fileid2, aliasid2 = newfile2.store()
1160
1161 # Verify that we didn't get back the same alias ID
1162- self.assertNotEqual(fa.id, self.storage.getFileAlias(aliasid2).id)
1163+ self.assertNotEqual(fa.id,
1164+ self.storage.getFileAlias(aliasid2, None, '/').id)
1165
1166 def test_clientProvidedDuplicateIDs(self):
1167 # This test checks the new behaviour specified by LibrarianTransactions
1168
1169=== modified file 'lib/canonical/librarian/ftests/test_web.py'
1170--- lib/canonical/librarian/ftests/test_web.py 2009-11-24 15:36:44 +0000
1171+++ lib/canonical/librarian/ftests/test_web.py 2010-09-07 04:07:54 +0000
1172@@ -3,17 +3,28 @@
1173
1174 from cStringIO import StringIO
1175 from datetime import datetime
1176+import httplib
1177 import unittest
1178 from urllib2 import urlopen, HTTPError
1179+from urlparse import urlparse
1180
1181 import pytz
1182-
1183+from storm.expr import SQL
1184 import transaction
1185 from zope.component import getUtility
1186
1187 from canonical.config import config
1188-from canonical.database.sqlbase import flush_database_updates, cursor
1189-from canonical.librarian.client import LibrarianClient
1190+from canonical.database.sqlbase import (
1191+ cursor,
1192+ flush_database_updates,
1193+ session_store,
1194+ )
1195+from canonical.launchpad.database.librarian import TimeLimitedToken
1196+from canonical.librarian.client import (
1197+ get_libraryfilealias_download_path,
1198+ LibrarianClient,
1199+ RestrictedLibrarianClient,
1200+ )
1201 from canonical.librarian.interfaces import DownloadFailed
1202 from canonical.launchpad.database import LibraryFileAlias
1203 from canonical.launchpad.interfaces import IMasterStore
1204@@ -57,12 +68,7 @@
1205 # librarian allow access to files that don't exist in the DB
1206 # and spitting them out with an 'unknown' mime-type
1207 # -- StuartBishop)
1208- try:
1209- urlopen(url)
1210- self.fail('Should have raised a 404')
1211- except HTTPError, x:
1212- self.failUnlessEqual(x.code, 404)
1213-
1214+ self.require404(url)
1215 self.commit()
1216
1217 # Make sure we can download it using the API
1218@@ -148,11 +154,7 @@
1219 config.librarian.download_port,
1220 aid, filename
1221 )
1222- try:
1223- urlopen(self._makeURL(aid, 'different.txt'))
1224- self.fail('404 not raised')
1225- except HTTPError, x:
1226- self.failUnlessEqual(x.code, 404)
1227+ self.require404(self._makeURL(aid, 'different.txt'))
1228
1229 def _makeURL(self, aliasID, filename):
1230 host = config.librarian.download_host
1231@@ -172,18 +174,9 @@
1232 self.failUnlessEqual(url, self._makeURL(aid, filename))
1233
1234 # Change the aliasid and assert we get a 404
1235- try:
1236- urlopen(self._makeURL(aid+1, filename))
1237- self.fail('404 not raised')
1238- except HTTPError, x:
1239- self.failUnlessEqual(x.code, 404)
1240-
1241+ self.require404(self._makeURL(aid+1, filename))
1242 # Change the filename and assert we get a 404
1243- try:
1244- urlopen(self._makeURL(aid, 'different.txt'))
1245- self.fail('404 not raised')
1246- except HTTPError, x:
1247- self.failUnlessEqual(x.code, 404)
1248+ self.require404(self._makeURL(aid, 'different.txt'))
1249
1250 def test_duplicateuploads(self):
1251 client = LibrarianClient()
1252@@ -237,6 +230,145 @@
1253 self.failUnlessEqual(
1254 last_modified_header, 'Tue, 30 Jan 2001 13:45:59 GMT')
1255
1256+ def get_restricted_file_and_public_url(self):
1257+ # Use a regular LibrarianClient to ensure we speak to the nonrestricted
1258+ # port on the librarian which is where secured restricted files are
1259+ # served from.
1260+ client = LibrarianClient()
1261+ fileAlias = client.addFile('sample', 12, StringIO('a'*12),
1262+ contentType='text/plain')
1263+ # Note: We're deliberately using the wrong url here: we should be
1264+ # passing secure=True to getURLForAlias, but to use the returned URL we
1265+ # would need a wildcard DNS facility patched into urlopen; instead
1266+ # we use the *deliberate* choice of having the path of secure and insecure
1267+ # urls be the same, so that we can test it: the server code doesn't need
1268+ # to know about the fancy wildcard domains.
1269+ url = client.getURLForAlias(fileAlias)
1270+ # Now that we have a url which talks to the public librarian, make the
1271+ # file restricted.
1272+ IMasterStore(LibraryFileAlias).find(LibraryFileAlias,
1273+ LibraryFileAlias.id==fileAlias).set(
1274+ LibraryFileAlias.restricted==True)
1275+ self.commit()
1276+ return fileAlias, url
1277+
1278+ def test_restricted_subdomain_must_match_file_alias(self):
1279+ # IFF there is a .restricted. in the host, then the library file alias
1280+ # in the subdomain must match that in the path.
1281+ client = LibrarianClient()
1282+ fileAlias = client.addFile('sample', 12, StringIO('a'*12),
1283+ contentType='text/plain')
1284+ fileAlias2 = client.addFile('sample', 12, StringIO('b'*12),
1285+ contentType='text/plain')
1286+ self.commit()
1287+ url = client.getURLForAlias(fileAlias)
1288+ download_host = urlparse(config.librarian.download_url)[1]
1289+ if ':' in download_host:
1290+ download_host = download_host[:download_host.find(':')]
1291+ template_host = 'i%%d.restricted.%s' % download_host
1292+ path = get_libraryfilealias_download_path(fileAlias, 'sample')
1293+ # The basic URL must work.
1294+ urlopen(url)
1295+ # Use the network level protocol because DNS resolution won't work
1296+ # here (no wildcard support)
1297+ connection = httplib.HTTPConnection(
1298+ config.librarian.download_host,
1299+ config.librarian.download_port)
1300+ # A valid subdomain based URL must work.
1301+ good_host = template_host % fileAlias
1302+ connection.request("GET", path, headers={'Host': good_host})
1303+ response = connection.getresponse()
1304+ response.read()
1305+ self.assertEqual(200, response.status)
1306+ # A subdomain based URL trying to put fileAlias into the restricted
1307+ # domain of fileAlias2 must not work.
1308+ hostile_host = template_host % fileAlias2
1309+ connection.request("GET", path, headers={'Host': hostile_host})
1310+ response = connection.getresponse()
1311+ response.read()
1312+ self.assertEqual(404, response.status)
1313+ # A subdomain which matches the LFA but is nested under one that
1314+ # doesn't is also treated as hostile.
1315+ nested_host = 'i%d.restricted.i%d.restricted.%s' % (
1316+ fileAlias, fileAlias2, download_host)
1317+ connection.request("GET", path, headers={'Host': nested_host})
1318+ response = connection.getresponse()
1319+ response.read()
1320+ self.assertEqual(404, response.status)
1321+
1322+ def test_restricted_no_token(self):
1323+ fileAlias, url = self.get_restricted_file_and_public_url()
1324+ # The file should not be able to be opened - we haven't allocated a
1325+ # token. When the token is wrong or stale a 404 is given (to avoid
1326+ # disclosure about what content we hold. Alternatively a 401 could be
1327+ # given (as long as we give a 401 when the file is missing as well -
1328+ # but that requires some more complex changes in the deployment
1329+ # infrastructure to permit more backend knowledge of the frontend
1330+ # request.
1331+ self.require404(url)
1332+
1333+ def test_restricted_made_up_token(self):
1334+ fileAlias, url = self.get_restricted_file_and_public_url()
1335+ # The file should not be able to be opened - the token supplied
1336+ # is not one we issued.
1337+ self.require404(url + '?token=haxx0r')
1338+
1339+ def test_restricted_with_token(self):
1340+ fileAlias, url = self.get_restricted_file_and_public_url()
1341+ # We have the base url for a restricted file; grant access to it
1342+ # for a short time.
1343+ token = TimeLimitedToken.allocate(url)
1344+ url = url + "?token=%s" % token
1345+ # Now we should be able to access the file.
1346+ fileObj = urlopen(url)
1347+ try:
1348+ self.assertEqual("a"*12, fileObj.read())
1349+ finally:
1350+ fileObj.close()
1351+
1352+ def test_restricted_with_expired_token(self):
1353+ fileAlias, url = self.get_restricted_file_and_public_url()
1354+ # We have the base url for a restricted file; grant access to it
1355+ # for a short time.
1356+ token = TimeLimitedToken.allocate(url)
1357+ # But time has passed
1358+ store = session_store()
1359+ tokens = store.find(TimeLimitedToken, TimeLimitedToken.token==token)
1360+ tokens.set(
1361+ TimeLimitedToken.created==SQL("created - interval '1 week'"))
1362+ url = url + "?token=%s" % token
1363+ # Now, as per test_restricted_no_token we should get a 404.
1364+ self.require404(url)
1365+
1366+ def test_restricted_file_headers(self):
1367+ fileAlias, url = self.get_restricted_file_and_public_url()
1368+ token = TimeLimitedToken.allocate(url)
1369+ url = url + "?token=%s" % token
1370+ # Change the date_created to a known value for testing.
1371+ file_alias = IMasterStore(LibraryFileAlias).get(
1372+ LibraryFileAlias, fileAlias)
1373+ file_alias.date_created = datetime(
1374+ 2001, 01, 30, 13, 45, 59, tzinfo=pytz.utc)
1375+ # Commit the update.
1376+ self.commit()
1377+ # Fetch the file via HTTP, recording the interesting headers
1378+ result = urlopen(url)
1379+ last_modified_header = result.info()['Last-Modified']
1380+ cache_control_header = result.info()['Cache-Control']
1381+ # No caching for restricted files.
1382+ self.failUnlessEqual(cache_control_header, 'max-age=0, private')
1383+ # And we should have a correct Last-Modified header too.
1384+ self.failUnlessEqual(
1385+ last_modified_header, 'Tue, 30 Jan 2001 13:45:59 GMT')
1386+ # Perhaps we should also set Expires to the Last-Modified.
1387+
1388+ def require404(self, url):
1389+ try:
1390+ urlopen(url)
1391+ self.fail('404 not raised')
1392+ except HTTPError, e:
1393+ self.failUnlessEqual(e.code, 404)
1394+
1395
1396 class LibrarianZopelessWebTestCase(LibrarianWebTestCase):
1397 layer = LaunchpadZopelessLayer
1398
1399=== modified file 'lib/canonical/librarian/interfaces.py'
1400--- lib/canonical/librarian/interfaces.py 2010-08-17 21:05:47 +0000
1401+++ lib/canonical/librarian/interfaces.py 2010-09-07 04:07:54 +0000
1402@@ -89,8 +89,19 @@
1403 class IFileDownloadClient(Interface):
1404 """Download API for the Librarian client."""
1405
1406- def getURLForAlias(aliasID):
1407- """Returns the URL to the given file"""
1408+ def getURLForAlias(aliasID, secure=False):
1409+ """Returns the URL to the given file.
1410+
1411+ :param aliasID: The LibraryFileAlias for the file to get. A DB lookup
1412+ will be done for this - if many are to be calculated, eagar loading
1413+ is recommended.
1414+ :param secure: If False, generate an http url on the main librarian
1415+ domain.
1416+ If True, generate an https url on a subdomain
1417+ i$aliasID.restricted.$main_librarian_domain.
1418+ Note that when a secure URL is generated, a TimeLimitedToken must
1419+ separately be obtained and combined with the URL to use it.
1420+ """
1421
1422 def getFileByAlias(aliasID, timeout=LIBRARIAN_SERVER_DEFAULT_TIMEOUT):
1423 """Returns a file-like object to read the file contents from.
1424
1425=== modified file 'lib/canonical/librarian/storage.py'
1426--- lib/canonical/librarian/storage.py 2010-08-16 18:50:40 +0000
1427+++ lib/canonical/librarian/storage.py 2010-09-07 04:07:54 +0000
1428@@ -72,8 +72,8 @@
1429 def startAddFile(self, filename, size):
1430 return LibraryFileUpload(self, filename, size)
1431
1432- def getFileAlias(self, aliasid):
1433- return self.library.getAlias(aliasid)
1434+ def getFileAlias(self, aliasid, token, path):
1435+ return self.library.getAlias(aliasid, token, path)
1436
1437
1438 class LibraryFileUpload(object):
1439
1440=== modified file 'lib/canonical/librarian/web.py'
1441--- lib/canonical/librarian/web.py 2010-08-17 16:33:33 +0000
1442+++ lib/canonical/librarian/web.py 2010-09-07 04:07:54 +0000
1443@@ -5,10 +5,12 @@
1444
1445 from datetime import datetime
1446 import time
1447+from urlparse import urlparse
1448
1449 from twisted.web import resource, static, util, server, proxy
1450 from twisted.internet.threads import deferToThread
1451
1452+from canonical.config import config
1453 from canonical.librarian.client import url_path_quote
1454 from canonical.librarian.db import read_transaction, write_transaction
1455 from canonical.librarian.utils import guess_librarian_encoding
1456@@ -62,11 +64,11 @@
1457 self.upstreamPort = upstreamPort
1458
1459 def getChild(self, filename, request):
1460-
1461 # If we still have another component of the path, then we have
1462 # an old URL that encodes the content ID. We want to keep supporting
1463 # these, so we just ignore the content id that is currently in
1464- # self.aliasID and extract the real one from the URL.
1465+ # self.aliasID and extract the real one from the URL. Note that
1466+ # tokens do not work with the old URL style: they are URL specific.
1467 if len(request.postpath) == 1:
1468 try:
1469 self.aliasID = int(filename)
1470@@ -74,7 +76,27 @@
1471 return fourOhFour
1472 filename = request.postpath[0]
1473
1474- deferred = deferToThread(self._getFileAlias, self.aliasID)
1475+ # IFF the request has a .restricted. subdomain, ensure there is a
1476+ # alias id in the right most subdomain, and that it matches
1477+ # self.aliasIDd, And that the host precisely matches what we generate
1478+ # (specifically to stop people putting a good prefix to the left of an
1479+ # attacking one).
1480+ hostname = request.getRequestHostname()
1481+ if '.restricted.' in hostname:
1482+ # Configs can change without warning: evaluate every time.
1483+ download_url = config.librarian.download_url
1484+ parsed = list(urlparse(download_url))
1485+ netloc = parsed[1]
1486+ # Strip port if present
1487+ if netloc.find(':') > -1:
1488+ netloc = netloc[:netloc.find(':')]
1489+ expected_hostname = 'i%d.restricted.%s' % (self.aliasID, netloc)
1490+ if expected_hostname != hostname:
1491+ return fourOhFour
1492+
1493+ token = request.args.get('token', [None])[0]
1494+ path = request.path
1495+ deferred = deferToThread(self._getFileAlias, self.aliasID, token, path)
1496 deferred.addCallback(
1497 self._cb_getFileAlias, filename, request
1498 )
1499@@ -82,12 +104,12 @@
1500 return util.DeferredResource(deferred)
1501
1502 @write_transaction
1503- def _getFileAlias(self, aliasID):
1504+ def _getFileAlias(self, aliasID, token, path):
1505 try:
1506- alias = self.storage.getFileAlias(aliasID)
1507+ alias = self.storage.getFileAlias(aliasID, token, path)
1508 alias.updateLastAccessed()
1509 return (alias.contentID, alias.filename,
1510- alias.mimetype, alias.date_created)
1511+ alias.mimetype, alias.date_created, alias.restricted)
1512 except LookupError:
1513 raise NotFound
1514
1515@@ -96,7 +118,7 @@
1516 return fourOhFour
1517
1518 def _cb_getFileAlias(
1519- self, (dbcontentID, dbfilename, mimetype, date_created),
1520+ self, (dbcontentID, dbfilename, mimetype, date_created, restricted),
1521 filename, request
1522 ):
1523 # Return a 404 if the filename in the URL is incorrect. This offers
1524@@ -105,8 +127,14 @@
1525 if dbfilename.encode('utf-8') != filename:
1526 return fourOhFour
1527
1528- # Set our caching headers. Librarian files can be cached forever.
1529- request.setHeader('Cache-Control', 'max-age=31536000, public')
1530+ if not restricted:
1531+ # Set our caching headers. Librarian files can be cached forever.
1532+ request.setHeader('Cache-Control', 'max-age=31536000, public')
1533+ else:
1534+ # Restricted files require revalidation every time. For now,
1535+ # until the deployment details are completely reviewed, the
1536+ # simplest, most cautious approach is taken: no caching permited.
1537+ request.setHeader('Cache-Control', 'max-age=0, private')
1538
1539 if self.storage.hasFile(dbcontentID) or self.upstreamHost is None:
1540 # XXX: Brad Crittenden 2007-12-05 bug=174204: When encodings are
1541
1542=== modified file 'lib/canonical/testing/layers.py'
1543--- lib/canonical/testing/layers.py 2010-05-06 15:51:34 +0000
1544+++ lib/canonical/testing/layers.py 2010-09-07 04:07:54 +0000
1545@@ -96,7 +96,11 @@
1546 from canonical.config import CanonicalConfig, config, dbconfig
1547 from canonical.database.revision import (
1548 confirm_dbrevision, confirm_dbrevision_on_startup)
1549-from canonical.database.sqlbase import cursor, ZopelessTransactionManager
1550+from canonical.database.sqlbase import (
1551+ cursor,
1552+ session_store,
1553+ ZopelessTransactionManager,
1554+ )
1555 from canonical.launchpad.interfaces import IMailBox, IOpenLaunchBag
1556 from lp.testing import ANONYMOUS, login, logout, is_logged_in
1557 import lp.services.mail.stub
1558@@ -193,9 +197,7 @@
1559 # as soon as SQLBase._connection is accessed
1560 r = main_store.execute('SELECT count(*) FROM LaunchpadDatabaseRevision')
1561 assert r.get_one()[0] > 0, 'Storm is not talking to the database'
1562-
1563- session_store = getUtility(IZStorm).get('session', 'launchpad-session:')
1564- assert session_store is not None, 'Failed to reconnect'
1565+ assert session_store() is not None, 'Failed to reconnect'
1566
1567
1568 def wait_children(seconds=120):
1569
1570=== modified file 'lib/lp/bugs/browser/bugattachment.py'
1571--- lib/lp/bugs/browser/bugattachment.py 2010-08-20 20:31:18 +0000
1572+++ lib/lp/bugs/browser/bugattachment.py 2010-09-07 04:07:54 +0000
1573@@ -10,7 +10,6 @@
1574 'BugAttachmentSetNavigation',
1575 'BugAttachmentEditView',
1576 'BugAttachmentURL',
1577- 'SafeStreamOrRedirectLibraryFileAliasView',
1578 ]
1579
1580 from cStringIO import StringIO
1581@@ -23,6 +22,7 @@
1582 FileNavigationMixin,
1583 ProxiedLibraryFileAlias,
1584 StreamOrRedirectLibraryFileAliasView,
1585+ SafeStreamOrRedirectLibraryFileAliasView,
1586 )
1587 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
1588 from canonical.launchpad.webapp import (
1589@@ -242,21 +242,6 @@
1590 return self.context.type == BugAttachmentType.PATCH
1591
1592
1593-class SafeStreamOrRedirectLibraryFileAliasView(
1594- StreamOrRedirectLibraryFileAliasView):
1595- """A view for Librarian files that sets the content disposion header."""
1596-
1597- def __call__(self):
1598- """Stream the content of the context `ILibraryFileAlias`.
1599-
1600- Set the content disposition header to the safe value "attachment".
1601- """
1602- self.request.response.setHeader(
1603- 'Content-Disposition', 'attachment')
1604- return super(
1605- SafeStreamOrRedirectLibraryFileAliasView, self).__call__()
1606-
1607-
1608 class BugAttachmentFileNavigation(Navigation, FileNavigationMixin):
1609 """Traversal to +files/${filename}."""
1610
1611
1612=== modified file 'lib/lp/bugs/browser/configure.zcml'
1613--- lib/lp/bugs/browser/configure.zcml 2010-08-03 13:37:55 +0000
1614+++ lib/lp/bugs/browser/configure.zcml 2010-09-07 04:07:54 +0000
1615@@ -1159,9 +1159,5 @@
1616 classes="
1617 BugWatchSetNavigation"/>
1618 </facet>
1619- <class class="lp.bugs.browser.bugattachment.SafeStreamOrRedirectLibraryFileAliasView">
1620- <allow attributes="__call__" />
1621- <allow interface="zope.publisher.interfaces.browser.IBrowserPublisher" />
1622- </class>
1623
1624 </configure>
1625
1626=== modified file 'lib/lp/bugs/browser/tests/test_bugattachment_file_access.py'
1627--- lib/lp/bugs/browser/tests/test_bugattachment_file_access.py 2010-08-20 20:31:18 +0000
1628+++ lib/lp/bugs/browser/tests/test_bugattachment_file_access.py 2010-09-07 04:07:54 +0000
1629@@ -15,6 +15,7 @@
1630
1631 from canonical.launchpad.browser.librarian import (
1632 StreamOrRedirectLibraryFileAliasView,
1633+ SafeStreamOrRedirectLibraryFileAliasView,
1634 )
1635 from canonical.launchpad.interfaces import ILaunchBag
1636 from canonical.launchpad.interfaces.librarian import (
1637@@ -25,12 +26,13 @@
1638 from canonical.testing import LaunchpadFunctionalLayer
1639 from lp.bugs.browser.bugattachment import (
1640 BugAttachmentFileNavigation,
1641- SafeStreamOrRedirectLibraryFileAliasView,
1642 )
1643 from lp.testing import (
1644 login_person,
1645 TestCaseWithFactory,
1646 )
1647+import lp.services.features
1648+from lp.services.features.flags import NullFeatureController
1649
1650
1651 class TestAccessToBugAttachmentFiles(TestCaseWithFactory):
1652@@ -80,7 +82,10 @@
1653 self.assertIsNot(None, mo)
1654
1655 def test_access_to_restricted_file(self):
1656- # Requests of restricted files are handled by ProxiedLibraryFileAlias.
1657+ # Requests of restricted files are handled by ProxiedLibraryFileAlias
1658+ # until we enable the publicrestrictedlibrarian (at which point
1659+ # this test should check the view like
1660+ # test_access_to_unrestricted_file.
1661 lfa_with_parent = getMultiAdapter(
1662 (self.bugattachment.libraryfile, self.bugattachment),
1663 ILibraryFileAliasWithParent)
1664@@ -91,6 +96,11 @@
1665 request.setTraversalStack(['foo.txt'])
1666 navigation = BugAttachmentFileNavigation(self.bugattachment, request)
1667 view = navigation.publishTraverse(request, '+files')
1668+ # XXX Ensure the feature will be off - everything is off with
1669+ # NullFeatureController. bug=631884
1670+ lp.services.features.per_thread.features = NullFeatureController()
1671+ self.addCleanup(
1672+ setattr, lp.services.features.per_thread, 'features', None)
1673 next_view, traversal_path = view.browserDefault(request)
1674 self.assertEqual(view, next_view)
1675 file_ = next_view()
1676@@ -130,6 +140,11 @@
1677 request.setTraversalStack(['foo.txt'])
1678 navigation = BugAttachmentFileNavigation(self.bugattachment, request)
1679 view = navigation.publishTraverse(request, '+files')
1680+ # XXX Ensure the feature will be off - everything is off with
1681+ # NullFeatureController. bug=631884
1682+ lp.services.features.per_thread.features = NullFeatureController()
1683+ self.addCleanup(
1684+ setattr, lp.services.features.per_thread, 'features', None)
1685 next_view, traversal_path = view.browserDefault(request)
1686 self.assertIsInstance(
1687 next_view, SafeStreamOrRedirectLibraryFileAliasView)
1688
1689=== modified file 'lib/lp/scripts/utilities/importfascist.py'
1690--- lib/lp/scripts/utilities/importfascist.py 2010-08-20 20:31:18 +0000
1691+++ lib/lp/scripts/utilities/importfascist.py 2010-09-07 04:07:54 +0000
1692@@ -31,6 +31,7 @@
1693 lp.codehosting.inmemory
1694 canonical.launchpad.browser.branchlisting
1695 lp.code.browser.branchlisting
1696+ canonical.launchpad.browser.librarian
1697 canonical.launchpad.feed.branch
1698 lp.code.feed.branch
1699 canonical.launchpad.interfaces.person
1700
1701=== modified file 'lib/lp/services/features/webapp.py'
1702--- lib/lp/services/features/webapp.py 2010-08-20 20:31:18 +0000
1703+++ lib/lp/services/features/webapp.py 2010-09-07 04:07:54 +0000
1704@@ -19,6 +19,14 @@
1705 self._request = request
1706
1707 def lookup(self, scope_name):
1708+ """Determine if scope_name applies to this request.
1709+
1710+ Currently supports the following scopes:
1711+ - default
1712+ - is_edge/is_lpnet etc (thunks through to the config)
1713+ """
1714+ if scope_name == 'default':
1715+ return True
1716 parts = scope_name.split('.')
1717 if len(parts) == 2:
1718 if parts[0] == 'server':
1719
1720=== modified file 'lib/lp/testing/fakelibrarian.py'
1721--- lib/lp/testing/fakelibrarian.py 2010-09-03 13:08:42 +0000
1722+++ lib/lp/testing/fakelibrarian.py 2010-09-07 04:07:54 +0000
1723@@ -150,7 +150,7 @@
1724 """See `IFileUploadClient`."""
1725 return NotImplementedError()
1726
1727- def getURLForAlias(self, aliasID):
1728+ def getURLForAlias(self, aliasID, secure=False):
1729 """See `IFileDownloadClient`."""
1730 alias = self.aliases.get(aliasID)
1731 path = get_libraryfilealias_download_path(aliasID, alias.filename)

Subscribers

People subscribed via source and target branches

to status/vote changes: