[01:27] <StevenK> wgrant: Unauthorized: (<zope.browserpage.metaconfigure.SimpleViewClass from /home/steven/launchpad/lp-branches/archive-limitedview/lib/lp/soyuz/browser/../templates/archive-packages.pt object at 0x2b1e9815e290>, 'browserDefault', 'launchpad.View')<br /> I guess from traversal, which create_initialized_view doesn't call into?
[01:43] <RoelV> Hi all
[01:43] <RoelV> bzr: ERROR: [Errno 1] _ssl.c:504: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
[01:43] <RoelV> how do I fix this?
[02:28] <wgrant> StevenK: That usually just means you don't hold launchpad.View on the object, and the view requires launchpad.View
[02:32] <StevenK> wgrant: Right. Which is expected. But the test uses create_initialized_view, which shows me the page content
[02:33] <wgrant> Right, that might skip it. Normally the page would 403 anyway because it accesses launchpad.View attributes
[03:18] <StevenK> wgrant: So this change looks okay to me, except for the +packages test for subscribers failing
[03:19] <wgrant> StevenK: What if you call the view?
[03:19] <StevenK> view.render() returns the page content
[03:19] <wgrant> Are you sure you set up the interaction correctly?
[03:19] <wgrant> Surely it needs to access some attributes that need launchpad.View
[03:20] <StevenK> The test calls login_person(self.subscriber) and c_i_v is called with self.subscriber as the principal
[03:25] <StevenK> wgrant: The P3A is empty, so maybe I need to publish a source into it first
[03:28] <wgrant> StevenK: Doesn't getPublishedSources need launchpad.View?
[03:28] <wgrant> Oh, I guess not
[03:28] <wgrant> Since it's necessary for +index
[03:28] <wgrant> So you may be right
[03:28] <StevenK> Right
[03:28] <wgrant> But stuff like the build counters might be protected
[03:29] <StevenK> I've moved description, is_active, is_copy, num_pkgs_building, publish, series_with_sources, signing_key and getPublishedSources()
[03:34] <StevenK> wgrant: So I can destroy the test, but I'd like to keep it
[03:35] <StevenK> Maybe I should switch it to the browser test
[03:35] <wgrant> StevenK: You probably want to adjust it so it triggers the view permission failure
[03:35] <StevenK> wgrant: Right, and it seems neither c_i_v() or c_v() will do so for me
[03:35] <StevenK> Whereas +packages requires View in the ZCML
[03:44] <StevenK> +        browser = setupBrowserForUser(self.subscriber)
[03:44] <StevenK> +        self.assertRaises(Unauthorized, browser.open, url)
[03:45] <StevenK> That works fine, since it uses the traversal rules
[03:45] <wgrant> Right
[03:45] <StevenK> So maybe I'll push this crap up and you can sob
[03:45] <StevenK> lib/lp/security.py:120: 'IAbstractMilestone' imported but unused
[03:45] <wgrant> Oops
[03:45] <wgrant> My fault
[03:45] <wgrant> A few weeks ago
[03:46] <StevenK> But I got burned last time I removed useless imports :-)
[03:46] <StevenK> I shall pull it out
[03:46] <wgrant> lp.security is not webservice definitions :)
[04:08] <StevenK> wgrant: https://code.launchpad.net/~stevenk/launchpad/new-perm-for-archive-subscribers/+merge/148974
[04:13] <wgrant> StevenK: Doesn't SubscriberView want to short-circuit if View is held?
[04:14] <StevenK> Probably
[04:14] <wgrant> Also, SubscriberView might want to imply LimitedView
[04:14] <wgrant> s/might/does/
[04:15] <StevenK> wgrant: How do I imply LimitedView?
[04:15] <wgrant> StevenK: The current default LimitedView adapter delegates to View, I believe
[04:15] <wgrant> It probably wants to fall back to SubscriberView
[04:16] <wgrant> StevenK: You also seem to be seriously allergic to VWS
[04:16] <wgrant> There's no gap before getPulbishedSources in the interface
[04:17] <wgrant> And you'll want tests for the permissions
[04:17] <StevenK> There is one before the API params
[04:17] <wgrant> eg. check directly that a subscriber has SubscriberView but not View
[04:17] <wgrant> 242	+ )
[04:17] <wgrant> 243	+ # Really returns ISourcePackagePublishingHistory, see below for
[04:17] <wgrant> 244	+ # patch to avoid circular import.
[04:17] <wgrant> 245	+ @call_with(eager_load=True)
[04:17] <wgrant> No space
[04:17] <wgrant> Oh
[04:17] <wgrant> So there is
[04:18] <wgrant> That's just a really huge operation_parameters
[04:18] <StevenK> Yes
[04:22] <StevenK> wgrant: Why do I want to imply LimitedView ?
[04:22] <wgrant> StevenK: Some things (eg. lazr.restful) won't acknowledge an object's existence without LimitedView
[04:22] <wgrant> eg. objects will not appear in collections unless LimitedView is held
[04:22] <StevenK> wgrant: http://pastebin.ubuntu.com/1675575/ is the permission test
[04:26] <StevenK> wgrant: Oh, ViewArchive backs onto get_enabled_archive_filter anyway, so it should be fine, no?
[04:26] <wgrant> StevenK: What do you mean?
[04:27] <StevenK> get_enabled_archive_filter is used for checking for launchpad.View, and we use the same method for SubscriberView, so we don't need a short-cut?
[04:27] <wgrant> StevenK: ViewArchive has several shortcuts
[04:28] <wgrant> I'm not sure if get_enabled_archive_filter also has all of them, but it's almost certainly slower too
[04:28] <StevenK> Right, I want to return True if ViewArchive does?
[04:29] <wgrant> Right
[04:30] <StevenK> wgrant: http://pastebin.ubuntu.com/1675592/
[04:31] <wgrant> if check_permission(...):
[04:31] <wgrant>    return True
[04:31] <wgrant> if check_permission(...):
[04:31] <wgrant>    return True
[04:31] <wgrant> return False
[04:31] <wgrant> Or or, I guess
[04:33] <StevenK> wgrant: In which bit of that diff?
[04:33] <StevenK> Oh, right
[04:33] <StevenK> Fixing
[04:34] <wgrant> All of it
[04:35] <StevenK> wgrant: http://pastebin.ubuntu.com/1675600/
[04:38] <wgrant> StevenK: Also, what happens when there's no SubscriberView adapter?
[04:38] <wgrant> I assume it just evaluates to False, but you should confirm
[04:38] <wgrant>      def checkAuthenticated(self, user):
[04:38] <wgrant> +        view_permission = ViewArchive.checkAuthenticated(self, user)
[04:38] <wgrant> +        if view_permission:
[04:38] <wgrant> +            return view_permission
[04:38] <wgrant> That's a bit silly
[04:40] <StevenK> So I considered calling if check_permission or self.store.find, but that means we need to do the work to calculate the filter if the user has View anyway
[04:41] <wgrant> foo = bar
[04:41] <wgrant> if foo:
[04:41] <wgrant>    return foo
[04:41] <wgrant> Where foo is boolean
[04:41] <wgrant> Is equivalent to:
[04:41] <wgrant> if bar:
[04:41] <wgrant>     return true
[04:41] <wgrant> s/t/T/
[04:42] <StevenK> wgrant: No SubscriberView adapter for IArchive, or something like IProduct?
[04:43] <wgrant> StevenK: Anything
[04:43] <wgrant> Because if something asks for LimitedView and View fails, it'll defer to SubscriberView
[04:50] <StevenK> wgrant: Hmmm, still trying to toss up how to test that
[04:51] <StevenK> And grumbling at the construction crew outside, who sound like they're grinding up cars
[04:54] <wgrant> StevenK: check_permission('launchpad.SubscriberView', someproduct)
[04:54] <wgrant> done
[04:54] <wgrant> No need for an actual test, just do it in a harness
[04:57] <StevenK> Yeah
[04:58] <StevenK> ------> print(check_permission('launchpad.SubscriberView', factory.makeProduct()))
[04:58] <StevenK> True
[04:58] <wgrant> Erm
[05:01] <wgrant> So, work out why :)
[05:08] <StevenK> Sorry, dealing with the fun of car ownership
[05:11] <StevenK> wgrant: Due to PermissiveSecurityPolicy
[05:11] <StevenK> Which returns True for anything
[05:12] <StevenK> And interaction being none results in <lp.services.webapp.authorization.LaunchpadPermissiveSecurityPolicy object at 0x8072d90>
[05:13] <wgrant> StevenK: Ah, right.
[05:13] <wgrant> Might have to hack the harness to use_web_security=True
[05:16] <StevenK> In [2]: print(check_permission('launchpad.SubscriberView', factory.makeProduct()))
[05:16] <StevenK> False
[05:16] <wgrant> Right
[05:16] <StevenK> With the following diff:
[05:16] <wgrant> That is less terrifyingly incorrect
[05:16] <StevenK> -    execute_zcml_for_scripts()
[05:16] <StevenK> +    execute_zcml_for_scripts(use_web_security=True)
[05:18] <StevenK> Tempted to hack iharness to respect LP_HARNESS_WEB_SECURITY=1
[05:18] <StevenK> (Or so)
[05:26] <StevenK> wgrant: Diff updated
[05:37] <wgrant> StevenK: Done
[05:39] <StevenK> wgrant: Does http://pastebin.ubuntu.com/1675739/ address your concerns?
[05:42] <wgrant> StevenK: Indeed.
[06:00] <StevenK> wgrant: Where's your GC branch + Archive:+delete timeout fix up to?
[06:03] <wgrant> StevenK: Doing more archive deletion rework
[06:03] <StevenK> Oh, so it won't suck. That would be nice.
[06:16] <wgrant> Actually, might propose the first stage now
[06:33] <wgrant> StevenK: https://code.launchpad.net/~wgrant/launchpad/archive-defer-deletion/+merge/148988
[06:34] <wgrant> I may not be around to answer grillings
[06:34]  * StevenK gets the chair, desk lamp and dripping tap ready
[06:40] <StevenK> wgrant: Did you consider only calling getUtility(IPublishingSet).requestDeletion() once in the publisher?
[06:42] <wgrant> StevenK: That'd delete most binaries twice
[06:42]  * wgrant goes
[06:45] <StevenK> wgrant: So, you did consider it. r=me
[08:56] <adeuring> good morning
[08:57] <StevenK> Unauthorized: (<Archive at 0xef0f490>, 'dependencies', 'launchpad.View')
[08:57] <StevenK> Bah! This was the *whole* point of the exercise