| 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:27 |
|---|---|---|
| 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? | 01:43 |
| wgrant | StevenK: That usually just means you don't hold launchpad.View on the object, and the view requires launchpad.View | 02:28 |
| StevenK | wgrant: Right. Which is expected. But the test uses create_initialized_view, which shows me the page content | 02:32 |
| wgrant | Right, that might skip it. Normally the page would 403 anyway because it accesses launchpad.View attributes | 02:33 |
| StevenK | wgrant: So this change looks okay to me, except for the +packages test for subscribers failing | 03:18 |
| 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:19 |
| StevenK | The test calls login_person(self.subscriber) and c_i_v is called with self.subscriber as the principal | 03:20 |
| StevenK | wgrant: The P3A is empty, so maybe I need to publish a source into it first | 03:25 |
| 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:28 |
| StevenK | I've moved description, is_active, is_copy, num_pkgs_building, publish, series_with_sources, signing_key and getPublishedSources() | 03:29 |
| StevenK | wgrant: So I can destroy the test, but I'd like to keep it | 03:34 |
| 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:35 |
| StevenK | + browser = setupBrowserForUser(self.subscriber) | 03:44 |
| StevenK | + self.assertRaises(Unauthorized, browser.open, url) | 03:44 |
| 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:45 |
| 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 :) | 03:46 |
| StevenK | wgrant: https://code.launchpad.net/~stevenk/launchpad/new-perm-for-archive-subscribers/+merge/148974 | 04:08 |
| wgrant | StevenK: Doesn't SubscriberView want to short-circuit if View is held? | 04:13 |
| StevenK | Probably | 04:14 |
| wgrant | Also, SubscriberView might want to imply LimitedView | 04:14 |
| wgrant | s/might/does/ | 04:14 |
| 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:15 |
| wgrant | StevenK: You also seem to be seriously allergic to VWS | 04:16 |
| wgrant | There's no gap before getPulbishedSources in the interface | 04:16 |
| 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:17 |
| wgrant | That's just a really huge operation_parameters | 04:18 |
| StevenK | Yes | 04:18 |
| 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:22 |
| 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:26 |
| 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:27 |
| 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:28 |
| wgrant | Right | 04:29 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1675592/ | 04:30 |
| 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:31 |
| StevenK | wgrant: In which bit of that diff? | 04:33 |
| StevenK | Oh, right | 04:33 |
| StevenK | Fixing | 04:33 |
| wgrant | All of it | 04:34 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1675600/ | 04:35 |
| 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:38 |
| 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:40 |
| 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:41 |
| StevenK | wgrant: No SubscriberView adapter for IArchive, or something like IProduct? | 04:42 |
| wgrant | StevenK: Anything | 04:43 |
| wgrant | Because if something asks for LimitedView and View fails, it'll defer to SubscriberView | 04:43 |
| StevenK | wgrant: Hmmm, still trying to toss up how to test that | 04:50 |
| StevenK | And grumbling at the construction crew outside, who sound like they're grinding up cars | 04:51 |
| 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:54 |
| StevenK | Yeah | 04:57 |
| StevenK | ------> print(check_permission('launchpad.SubscriberView', factory.makeProduct())) | 04:58 |
| StevenK | True | 04:58 |
| wgrant | Erm | 04:58 |
| wgrant | So, work out why :) | 05:01 |
| StevenK | Sorry, dealing with the fun of car ownership | 05:08 |
| StevenK | wgrant: Due to PermissiveSecurityPolicy | 05:11 |
| StevenK | Which returns True for anything | 05:11 |
| StevenK | And interaction being none results in <lp.services.webapp.authorization.LaunchpadPermissiveSecurityPolicy object at 0x8072d90> | 05:12 |
| wgrant | StevenK: Ah, right. | 05:13 |
| wgrant | Might have to hack the harness to use_web_security=True | 05:13 |
| 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:16 |
| StevenK | Tempted to hack iharness to respect LP_HARNESS_WEB_SECURITY=1 | 05:18 |
| StevenK | (Or so) | 05:18 |
| StevenK | wgrant: Diff updated | 05:26 |
| wgrant | StevenK: Done | 05:37 |
| StevenK | wgrant: Does http://pastebin.ubuntu.com/1675739/ address your concerns? | 05:39 |
| wgrant | StevenK: Indeed. | 05:42 |
| StevenK | wgrant: Where's your GC branch + Archive:+delete timeout fix up to? | 06:00 |
| wgrant | StevenK: Doing more archive deletion rework | 06:03 |
| StevenK | Oh, so it won't suck. That would be nice. | 06:03 |
| wgrant | Actually, might propose the first stage now | 06:16 |
| wgrant | StevenK: https://code.launchpad.net/~wgrant/launchpad/archive-defer-deletion/+merge/148988 | 06:33 |
| wgrant | I may not be around to answer grillings | 06:34 |
| * StevenK gets the chair, desk lamp and dripping tap ready | 06:34 | |
| StevenK | wgrant: Did you consider only calling getUtility(IPublishingSet).requestDeletion() once in the publisher? | 06:40 |
| wgrant | StevenK: That'd delete most binaries twice | 06:42 |
| * wgrant goes | 06:42 | |
| StevenK | wgrant: So, you did consider it. r=me | 06:45 |
| === almaisan-away is now known as al-maisan | ||
| adeuring | good morning | 08:56 |
| StevenK | Unauthorized: (<Archive at 0xef0f490>, 'dependencies', 'launchpad.View') | 08:57 |
| StevenK | Bah! This was the *whole* point of the exercise | 08:57 |
| === 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 | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!