Merge lp:~edwin-grubbs/launchpad/bug-451208-subscribing-reveals-email into lp:launchpad
| Status: | Merged |
|---|---|
| Merged at revision: | not available |
| Proposed branch: | lp:~edwin-grubbs/launchpad/bug-451208-subscribing-reveals-email |
| Merge into: | lp:launchpad |
| Diff against target: |
1808 lines (+788/-258) 16 files modified
lib/canonical/launchpad/browser/vocabulary.py (+5/-1) lib/canonical/launchpad/doc/vocabularies.txt (+8/-5) lib/canonical/launchpad/doc/vocabulary-json.txt (+20/-0) lib/canonical/launchpad/vocabularies/configure.zcml (+297/-86) lib/canonical/launchpad/vocabularies/dbobjects.py (+5/-3) lib/canonical/launchpad/webapp/configure.zcml (+1/-0) lib/canonical/launchpad/webapp/metazcml.py (+7/-2) lib/canonical/launchpad/zcml/binaryandsourcepackagename.zcml (+13/-2) lib/lp/answers/configure.zcml (+9/-2) lib/lp/answers/doc/faq-vocabulary.txt (+2/-1) lib/lp/registry/doc/vocabularies.txt (+57/-56) lib/lp/registry/vocabularies.zcml (+304/-95) lib/lp/services/tests/test_vocabularies.py (+37/-0) lib/lp/services/worlddata/vocabularies.zcml (+5/-2) lib/lp/soyuz/configure.zcml (+16/-2) lib/lp/soyuz/interfaces/binarypackagename.py (+2/-1) |
| To merge this branch: | bzr merge lp:~edwin-grubbs/launchpad/bug-451208-subscribing-reveals-email |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Guilherme Salgado (community) | code | 2010-01-12 | Approve on 2010-01-18 |
| Canonical Launchpad Engineering | code | 2010-01-12 | Pending |
|
Review via email:
|
|||
| Edwin Grubbs (edwin-grubbs) wrote : | # |
| Guilherme Salgado (salgado) wrote : | # |
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -809,6 +809,7 @@
>
> <class class="
> <allow interface=
> + <allow attributes=
Wouldn't it make sense to add __getslice__ to CountableIterator?
> </class>
>
> <class class="
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -20,7 +20,9 @@
> The active mailing lists vocabulary matches and returns only those mailing
> lists which are active.
>
> - >>> list_vocabulary = vocabulary_
> + >>> from zope.security.proxy import removeSecurityProxy
> + >>> list_vocabulary = removeSecurityP
> + ... vocabulary_
> >>> from canonical.
> >>> from canonical.
> >>> verifyObject(
> @@ -203,6 +205,7 @@
> u'The Hoary Hedgehog Release'
>
> >>> def getTerms(vocab, search_text):
> + ... vocab = removeSecurityP
> ... [vocab.toTerm(item) for item in vocab.search(
>
> >>> getTerms(
> @@ -508,7 +511,8 @@
>
> The list of selectable projects. The results are ordered by displayname.
>
> - >>> project_vocabulary = vocabulary_
> + >>> project_vocabulary = removeSecurityP
> + ... vocabulary_
I think we wought to have a wrapper around vocabulary_
the security proxy of the vocab before returning. Something like
def get_naked_
return removeSecurityP
That way we don't have to repeat the removeSecurityP
places we call vocabulary_
> >>> project_
> 'Select a project group'
>
> @@ -542,7 +546,8 @@
>
> The list of selectable products. Results are ordered by displayname.
>
> - >>> product_vocabulary = vocabulary_
> + >>> product_vocabulary = removeSecurityP
> + ... vocabulary_
> >>> product_
> 'Select a project'
>
> @@ -583,8 +588,8 @@
>
> The list of selectable products releases.
>
> - >>> productrelease_
> - ... "ProductRelease")
> + >>> productrelease_
> + ... vocabulary_
> >>> productrelease_
| Guilherme Salgado (salgado) wrote : | # |
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -809,6 +809,7 @@
>
> <class class="
> <allow interface=
> + <allow attributes=
Wouldn't it make sense to add __getslice__ to CountableIterator?
> </class>
>
> <class class="
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -20,7 +20,9 @@
> The active mailing lists vocabulary matches and returns only those mailing
> lists which are active.
>
> - >>> list_vocabulary = vocabulary_
> + >>> from zope.security.proxy import removeSecurityProxy
> + >>> list_vocabulary = removeSecurityP
> + ... vocabulary_
> >>> from canonical.
> >>> from canonical.
> >>> verifyObject(
> @@ -203,6 +205,7 @@
> u'The Hoary Hedgehog Release'
>
> >>> def getTerms(vocab, search_text):
> + ... vocab = removeSecurityP
> ... [vocab.toTerm(item) for item in vocab.search(
>
> >>> getTerms(
> @@ -508,7 +511,8 @@
>
> The list of selectable projects. The results are ordered by displayname.
>
> - >>> project_vocabulary = vocabulary_
> + >>> project_vocabulary = removeSecurityP
> + ... vocabulary_
I think we wought to have a wrapper around vocabulary_
the security proxy of the vocab before returning. Something like
def get_naked_
return removeSecurityP
That way we don't have to repeat the removeSecurityP
places we call vocabulary_
> >>> project_
> 'Select a project group'
>
> @@ -542,7 +546,8 @@
>
> The list of selectable products. Results are ordered by displayname.
>
> - >>> product_vocabulary = vocabulary_
> + >>> product_vocabulary = removeSecurityP
> + ... vocabulary_
> >>> product_
> 'Select a project'
>
> @@ -583,8 +588,8 @@
>
> The list of selectable products releases.
>
> - >>> productrelease_
> - ... "ProductRelease")
> + >>> productrelease_
> + ... vocabulary_
> >>> productrelease_
| Edwin Grubbs (edwin-grubbs) wrote : | # |
> > === modified file 'lib/canonical/
> > --- lib/canonical/
> +0000
> > +++ lib/canonical/
> +0000
> > @@ -809,6 +809,7 @@
> >
> > <class class="
> > <allow
> interface=
> > + <allow attributes=
>
> Wouldn't it make sense to add __getslice__ to CountableIterator?
Gary had recommended that I add __getslice__ to the zcml instead of the interface
since this is a security concern and not a method that implementors of ICountableIterator
need to create.
> > </class>
> >
> > <class
> class="
> >
> > === modified file 'lib/lp/
> > --- lib/lp/
> > +++ lib/lp/
> > @@ -20,7 +20,9 @@
> > The active mailing lists vocabulary matches and returns only those mailing
> > lists which are active.
> >
> > - >>> list_vocabulary = vocabulary_
> 'ActiveMailingL
> > + >>> from zope.security.proxy import removeSecurityProxy
> > + >>> list_vocabulary = removeSecurityP
> > + ... vocabulary_
> > >>> from canonical.
> > >>> from canonical.
> > >>> verifyObject(
> > @@ -203,6 +205,7 @@
> > u'The Hoary Hedgehog Release'
> >
> > >>> def getTerms(vocab, search_text):
> > + ... vocab = removeSecurityP
> > ... [vocab.toTerm(item) for item in vocab.search(
> >
> > >>> getTerms(
> > @@ -508,7 +511,8 @@
> >
> > The list of selectable projects. The results are ordered by displayname.
> >
> > - >>> project_vocabulary = vocabulary_
> > + >>> project_vocabulary = removeSecurityP
> > + ... vocabulary_
>
> I think we wought to have a wrapper around vocabulary_
> the security proxy of the vocab before returning. Something like
>
> def get_naked_
> return removeSecurityP
>
> That way we don't have to repeat the removeSecurityP
> places we call vocabulary_
Done.
> > >>> project_
> > 'Select a project group'
> >
> > @@ -542,7 +546,8 @@
> >
> > The list of selectable products. Results are ordered by displayname.
> >
> > - >>> product_vocabulary = vocabulary_
> > + >>> product_vocabulary = removeSecurityP
> > + ... vocabulary_
> > >>> product_
> > 'Select a project'
> >
> > @@ -583,8 +588,8 @@
> >
> > The list of sel...
| Guilherme Salgado (salgado) wrote : | # |
Just add a comment explaining why we need the "vocab.

Summary
-------
Registered all the Launchpad vocabularies with <securedutility>
instead of <utility> so that it will not be possible to get
a db object without a security proxy.
This required an <allow> block on each utility to allow tory).
access to the __call__ method on the vocabulary class (IVocabularyFac
The vocabulary objects required a <class> directive with IHugeVocabulary >.
<allow interface=
Some vocabulary factories were actually functions, such as eriesVocabulary , which would actually return either roductSeriesVoc abulary or BugNominatableD istroSeriesVoca bulary
BugNominatableS
BugNominatableP
objects.
Implementation details ------- ------- -
-------
New tests: lp/services/ tests/test_ vocabularies. py canonical/ launchpad/ doc/vocabulary- json.txt
lib/
lib/
Indicate in the picker widget when email address is hidden. canonical/ launchpad/ browser/ vocabulary. py
lib/
Some vocabularies need to get the IBinaryAndSourc ePackageName. id lp/soyuz/ interfaces/ binarypackagena me.py
attribute from objects that were passed in from widgets, so they
are already security proxied.
lib/
Fixed tests that broke since they were accessing methods that canonical/ launchpad/ doc/vocabularie s.txt lp/answers/ doc/faq- vocabulary. txt lp/registry/ doc/vocabularie s.txt
should not be exposed through the security proxy.
lib/
lib/
lib/
Since vocabularies must be registered with a name parameter, canonical/ launchpad/ webapp/ metazcml. py
the <securedutility> directive needed to be updated to pass
this along to the underlying <utility>.
lib/
Added BugNominatableD istroSeriesVoca bulary and roductSeriesVoc abulary to __all__. canonical/ launchpad/ vocabularies/ dbobjects. py
BugNominatableP
lib/
ICountableIterator defines a __getitem__ method that provides canonical/ launchpad/ webapp/ configure. zcml
the __getslice__ functionality.
lib/
Somewhat interesting zcml changes: canonical/ launchpad/ zcml/binaryands ourcepackagenam e.zcml lp/soyuz/ configure. zcml
lib/
lib/
The changes for SimpleVocabulary, SimpleTerm, and SeriesVocabular y are the only parts of note among the canonical/ launchpad/ vocabularies/ configure. zcml
BugNominatable*
many changes in this file.
lib/
TimezoneNameVoc abulary is just a function that returns a lp/services/ worlddata/ vocabularies. zcml
SimpleVocabulary, so it doesn't need any <class> directives.
lib/
Uninteresting bulk changes to use <securedutility>: lp/answers/ configure. zcml lp/registry/ vocabularies. zcml
lib/
lib/
Tests
-----
./bin/test -vv -t 'test_vocabular ies|vocabulary- json.txt'
Demo and Q/A
------------
* Open https:/ /launchpad. dev/people/ +requestmerge
* Click on "Choose" to bring up the picker.
* Search for "name12".
* The email address should be hidden.