launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

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. 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)
        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.