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