Hi Folks,
I've reviewed the private project functionality currently committed to
trunk. This is some exciting work that I'm looking forward to leveraging
but unfortunately I do not feel the code is ready to live in trunk and
thus intend to move the code into a feature branch to incubate (once
this occurs and we begin discussion on my points below I'll be able to
provide more details on what to expect regarding a timeline for the code
actually making it into trunk). Here are my findings, when viewed as a
whole, I feel is sufficiently compelling:
N.B. I intend to file bugs for some of the specific bugs I mention below
but didn't want to hold this e-mail up any longer. Apologies - I'll file
bugs and annotate bug numbers in a reply tomorrow morning.
1. All models that are potentially private have been updated to declare
two model managers in the same fashion. The first manager declared is
'all_objects' which is the AccessControlledObjectsManager manager (a
subclass of django-group-access's AccessManager manager) and it does not
restrict the queryset but does provide additional methods related to
access control. The second manager declared is 'objects' (which is the
same name as the default manager for models that do not declare their
own managers) which is the PublicOnlyObjectsManager manager and it only
allows public objects to be returned. Comments in the source code
explicitly explain that this approach is to "ensure that any oversights
when updating existing uses of (<Model>).objects won't leak private
(objects)". Although true, this design decision also makes it too easy
to make such oversights and result in breakage of existing functionality
(especially considering the current state of Offspring's test suite)
which is indeed the case. ex. It is no longer possible to cancel queued
builds for a private project and the 'needs_build' property on the
Project model will not be accurate for private projects. Both are due to
oversight in updating use of objects model manager.
2. Unfortunately, the model managers alone are not sufficient to
necessarily protect the privacy of private objects. For example, the
very commonly used django.shortcuts.get_{object,list}_or_404 functions
and other pieces of internal Django machinery use the 'default' manager
for a model, which is the first manager declared, which provides
unrestricted access objects. Some views have been updated to use a
wrapper function called 'get_possibly_private_object' or methods such as
'accessible_by_user' on the all_objects manager but failure to do so
causes odd behaviour, breakage of existing functionality, and/or
disclose of (or permit operations on) private objects. Ex. It is
possible for a user who has the necessary system wide permission to
queue builds to queue builds for a private project by directly accessing
URL due to oversight in updating use of get_object_or_404.
3. Additionally, I suspect that this approach will cause problems (and
confusion - calling the public manager something other than objects, ex.
'public_objects', would probably be better to make it clear the model
needs special 'privacy' considerations - e.g. failing explicitly is
preferable) down the road when third-party applications or code attempt
to interact with our models. As a result, accepting this approach into
trunk will require us to adapt existing and future code to be
privacy-aware and thus be a commitment that could be extremely costly to
back out, due to the invasive nature, if ever needed.
4. It is not currently possible to update the owner or the 'is_private'
flag on a project via the django admin UI. It is, however, possible to
do so via the 'update project' form in the front-end which is accessible
to anyone who has the system wide permission to edit projects. Thus, it
is quite easy to make a project inaccessible to (for all practical
purposes) everyone including admins and the only remedy is to manually
modify the database (as we wouldn't even be able to see who the owner
was mistakenly set to).
5. Admins can not see or interact with private objects in the front-end
unless they've been granted explicit access to the private objects by
the private object owner.
6. Idle builders erroneouslylist the current job as 'private build'.
7. Not much consideration given to UI- specifically, the columns on the
project details page are broken (due to using buggy css/html that I
borrowed from Launchpad). Additionally, the label for the 'is_private'
field on the project forms is simply 'is private' which looks weird. On
the same thread, not all deployment would want users to be able to
select owners and make projects private as they see fit.
Cheers,
--
Cody A.W. Somerville
Release Engineer
Commercial Engineering
Canonical Canada Ltd.
Phone: +1 781 850 2087
Cell: +1 613 401 5141
Fax: +1 613 687 7368
Email: cody.somervi...@canonical.com
--
Mailing list: https://launchpad.net/~offspring-hackers
Post to : offspring-hackers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~offspring-hackers
More help : https://help.launchpad.net/ListHelp
