Merge lp:~cprov/launchpad/bug-409208-ppa-search-permissions into lp:launchpad

[Celso Providelo]

[Celso Providelo (cprov)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

= Summary =

This branch fixes https://bugs.edge.launchpad.net/bugs/409208, when disabled PPAs were leaking in the PPA search results.

Since https://bugs.edge.launchpad.net/bugs/367796 (devel r9015) only owners can view disabled PPAs.

== Proposed fix ==

The fix it trivial, simply filters out disabled PPAs from the search results

== Tests ==

./bin/test -vv -t package-cache.txt

== Demo and Q/A ==

1. As foo.bar disable Celso PPA (~cprov/+archive/+ppa)

2. Yet as 'foo.bar' search for PPAs matching 'cprov' in ubuntu/+ppas, you will be able to see it.

3. As anonymous or 'no-priv' repeat the search and you will get nothing.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/doc/package-cache.txt
  lib/lp/registry/model/distribution.py
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkqDdNAACgkQ7KBXuXyZSjDNlACdFMQBI9HNQIiM2YOR/o86o2EW
G48AoIjaXSKjgC5djvyXpL+kRt9dciSo
=oBIT
-----END PGP SIGNATURE-----

[Michael Nelson (michael.nelson)]

Great work Celso! The doc test changes document the nuance very well.

My only thought was whether it is worth a comment above the switchDbUser just letting the reader know why it is necessary (for permission on TeamParticipation?).

Thanks!

review: Approve

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2009-08-03 13:00:37 +0000
+++ lib/lp/registry/model/distribution.py	2009-08-13 01:58:22 +0000
@@ -1228,7 +1228,7 @@
         if user is not None:
             if not user.inTeam(getUtility(ILaunchpadCelebrities).admin):
                 clauses.append("""
-                (Archive.private = FALSE OR
+                ((Archive.private = FALSE AND Archive.enabled = TRUE) OR
                  Archive.owner = %s OR
                  %s IN (SELECT TeamParticipation.person
                         FROM TeamParticipation
@@ -1237,7 +1237,8 @@
                 )
                 """ % sqlvalues(user, user, user))
         else:
-            clauses.append("Archive.private = FALSE")
+            clauses.append(
+                "Archive.private = FALSE AND Archive.enabled = TRUE")
 
 
         query = ' AND '.join(clauses)

=== modified file 'lib/lp/soyuz/doc/package-cache.txt'
--- lib/lp/soyuz/doc/package-cache.txt	2009-08-03 13:00:37 +0000
+++ lib/lp/soyuz/doc/package-cache.txt	2009-08-13 01:58:22 +0000
@@ -715,6 +715,9 @@
     ...         archive.sources_cached, source_caches.count())
     ...     print '%d binaries cached [%d]' % (
     ...         archive.binaries_cached, binary_caches.count())
+    >>> def print_search_results(text, user=None):
+    ...     for ppa in ubuntu.searchPPAs(text, user=user):
+    ...         print ppa.displayname
 
     >>> rebuild_caches(cprov.archive)
 
@@ -722,19 +725,55 @@
     3 sources cached [3]
     2 binaries cached [2]
 
-When Celso's PPA gets disabled and the indexes rebuilt, the cache
-records are removed.
+    >>> print_search_results('pmount')
+    PPA for Celso Providelo
+
+When Celso's PPA gets disabled, the indexes remain in the DB.
 
     >>> cprov.archive.enabled = False
 
+    >>> print_caches(cprov.archive)
+    3 sources cached [3]
+    2 binaries cached [2]
+
+However the disabled PPA is not included in search results for
+anonymous requests or requests from users with no view permission to
+Celso's PPA.
+
+    >>> commit()
+    >>> LaunchpadZopelessLayer.switchDbUser('launchpad')
+
+    >>> print_search_results('pmount')
+
+    >>> no_priv = getUtility(IPersonSet).getByName('no-priv')
+    >>> print_search_results('pmount', user=no_priv)
+
+Only the owner of the PPA can still find it until the changes are
+removed.
+
+    >>> print_search_results('pmount', user=cprov)
+    PPA for Celso Providelo
+
+When indexes rebuilt the cache records are removed and not even the
+owner is able to find the disabled PPA.
+
+    >>> LaunchpadZopelessLayer.switchDbUser(test_dbuser)
     >>> rebuild_caches(cprov.archive)
 
     >>> print_caches(cprov.archive)
     0 sources cached [0]
     0 binaries cached [0]
 
+    >>> commit()
+    >>> LaunchpadZopelessLayer.switchDbUser('launchpad')
+
+    >>> print_search_results('pmount', user=cprov)
+
+    >>> LaunchpadZopelessLayer.switchDbUser(test_dbuser)
+
 If by any chance, the disabled PPA gets re-enabled, the cache records
-will be re-created when the indexes are rebuilt.
+will be re-created when the indexes are rebuilt and the ppa becomes
+publicly searchable again.
 
     >>> cprov.archive.enabled = True
 
@@ -744,3 +783,5 @@
     3 sources cached [3]
     2 binaries cached [2]
 
+    >>> print_search_results('cprov')
+    PPA for Celso Providelo