launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

On 10.10.2012 22:30, Curtis Hovey wrote:
> This is the checker used in production:
> 
> class ViewPillar(AuthorizationBase):
>     usedfor = IPillar
>     permission = 'launchpad.View'
> 
>     def checkUnauthenticated(self):
>         return self.obj.active
> 
>     def checkAuthenticated(self, user):
>         """The Admins & Commercial Admins can see inactive pillars."""
>         if self.obj.active:
>             return True
>         else:
>             return (user.in_commercial_admin or
>                     user.in_admin or
>                     user.in_registry_experts)
> 
> You introduced a new checker that is specific to IProduct, but is does not ever consider .active.

The one in r16090 did.

> As is said in the hangout, IPillar is not properly implemented. IDistribution.active cannot ever be false, so we know the .active rule is mostly for IProduct. I think this phrasing of rules always defers to ViewPillar for the current case that all projects are public. We only do new rule checking for private types. Unauthenticated is always false, and authenticated has to exempt A and CA from the data drive rules in userCanView()
> 
> class ViewProduct(ViewPillar):
>     permission = 'launchpad.View'
>     usedfor = IProduct
> 
>     def checkAuthenticated(self, user):
>         if self.obj.information_type in PUBLIC_INFORMATION_TYPES:
>             return super(ViewProduct, self).checkAuthenticated(user)

...this would deny access to properties like name, displayname etc which
we need for deactivated products, so the same problem we had with r16090.

>         return (user.in_commercial_admin
>                 or user.in_admin 
>                 or self.obj.userCanView(user))
> 
>     def checkUnauthenticated(self):
>         if self.obj.information_type in PUBLIC_INFORMATION_TYPES:
>             return super(ViewProduct, self).checkUnauthenticated()
>         return False
> 


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