launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

On 11.10.2012 10:40, William Grant wrote:
> AIUI the main thing launchpad.View on IProduct is used for today is to prevent traversal to deactivated products. If you add an extra guard to prevent such traversal even if launchpad.View is held, then the launchpad.View adapter can have an emergency feature flag which grants it to everyone.
> 

A feature flag alone would not help much. I think the main issue is
that the same checker is used for IPillar and IProductLimitedView
(should be renamed to IProductView, but this is different issue).

Let me try to explain the problem with this branch:
lp:~adeuring/launchpad/authentication-for-private-products-test

It has my core changes: most of attributes of IProduct are defined in
IProductLimitedView and require the permission launchpad.View.

The relevant security adapter is ViewProduct.

First, the variant r16120:

The section for Product in registry/configure.zcml does not contain
any directive for IPillar.

Now do "make run", log in as foo.bar@xxxxxxxxxxxxx and try to view any
product related page. You'll get "LocationError:
(<Product at 0xba970d0>, 'active')<br />"

Or run iharness:

from lp.registry.interfaces.product import IProductSet
p = getUtility(IProductSet).getByName('firefox')
p.active

You'll get a ForbiddenAttribute exception.

So we need a directive for IPillar in registry/configure.zcml.

r16121 has

        <allow
            interface="lp.registry.interfaces.product.IPillar"/>

The iharness test now works.

Now "make run", log in as an admin and make thunderbird private, as
suggested by Curtis.

Visit https://launchpad.dev/thunderbird as an admin, as an ordinary user
and without being logged in. This works as expected: The admin sees
the regular page with the notice "this project is currently inactive."

Anonymous and test@xxxxxxxxxxxxx see a 404 page.

Now visit https://bugs.launchpad.dev/redfish/+bug/15 (this bug has a
task for thunderbird.)

Anonymous and the admin see the page just fine, but test@xxxxxxxxxxxxx
gets a "Not allowed here" error:
Unauthorized: (<Product at 0x2b0588b8d9d0>, 'project',
'launchpad.View')<br />

as noticed by Curtis.

Now let us try to remove the check for inactive products in the
security adapter, as suggested by William (out of lazyness, without a
feature flag, i just removed the check for inactive products): r16122

The bug page works fine for all users, but anonymous and
test@canoncial,com no longer get the 404 error for
https://launchpad.dev/thunderbird . Instead, they see the same page as
the admin: "This project is currently inactive."

So, no luck either.

Let us now require the permission lp.View for IPillar: r16123

Both pages, https://bugs.launchpad.dev/redfish/+bug/15 (good) and
https://launchpad.dev/thunderbird (bad) are still visible for anonymous
and test@xxxxxxxxxxxxx .

Let's reenable the check for inactive products in ViewProduct: r16124

Now anonymous and test@xxxxxxxxxxxxx get again the 404 error for
https://launchpad.dev/thunderbird -- but test@xxxxxxxxxxxxx gets
a 403 for https://bugs.launchpad.dev/redfish/+bug/15. So we are back
at the situation that required a rollback of r16090 of trunk.

I think the main problem is that the interfaces IPillar and IProductView
use the same security adapter because they use the same permission.

Regular users need to have access to several attributes of inactive
products (name, displayname, and some more -- I see no better way than
to wait for OOPSes or to check every location where some set of
products is quite late filtered to keep only active prodic...), but
that they must not be able to access IPillar.active if the flag is
False.

I see these options to proceed:

1. move more attributes from IProduct(Limited)View to IProductPublic
2. keep the permission lp.View for IPillar and for IProduct(Limited)View
   (I am not sure if this is possible -- at least I don't know how to
   configure this.)
3. use different permissions for IPillar and IProduct(Limited)View,
   at least temporarily. Once private projects are enabled,
   we will get similar errors as those we got now for inactive products,
   but hopefully with less drastic consequences.

   Then we can fix the queries that wrongly return private projects so
   that inactive projects are filtered out too, and once we are
   confident that we fixed most of the problems, we can use the same
   permission and checker for IPillar and IProduct(Limited)View


-- 
https://code.launchpad.net/~adeuring/launchpad/authentication-for-private-products/+merge/129014
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.