20:00:34 <ying_zuo> #startmeeting horizon
20:00:35 <openstack> Meeting started Wed Aug 30 20:00:34 2017 UTC and is due to finish in 60 minutes.  The chair is ying_zuo. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:00:36 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
20:00:39 <openstack> The meeting name has been set to 'horizon'
20:00:44 <ying_zuo> Hi everyone o/
20:00:50 <e0ne> hi
20:01:04 <david-lyle> o/
20:01:09 <robcresswell> o/
20:01:21 <rdopiera> o/
20:02:18 <ying_zuo> #topic Announcements
20:02:24 <ying_zuo> #info OpenStack Pike is available now!
20:02:29 <ying_zuo> #link https://www.openstack.org/software/pike/
20:02:37 <ying_zuo> Thanks everyone for your contribution in making this release successful!
20:02:53 <robcresswell> #o/
20:02:57 <e0ne> my congratulations!
20:03:00 <robcresswell> \o/ *
20:03:16 <ying_zuo> \o/
20:03:27 <ying_zuo> #info Schedule for PTG
20:03:32 <ying_zuo> #link https://etherpad.openstack.org/p/horizon-ptg-queens
20:03:43 <ying_zuo> Feel free to comment or add more topics in the open discussion time slots.
20:04:09 <ying_zuo> #info Bug report guidelines
20:04:15 <ying_zuo> #link https://bugs.launchpad.net/horizon/+filebug
20:04:21 <e0ne> ying_zuo: is it final schedule?
20:04:31 <ying_zuo> not really
20:04:51 <ying_zuo> are you planning to add changes?
20:05:35 <ying_zuo> you can add more topics in the open discussion time slots
20:06:04 <e0ne> ying_zuo: not yet. I'm trying to manage my schedule to resolve conflicts and participate in pagination discussion
20:06:05 <ying_zuo> there's a time slot for it each day
20:07:15 <ying_zuo> what time will work for you?
20:08:48 <ying_zuo> can you let me know in #openstack-horizon?
20:09:16 <e0ne> ying_zuo: before the noon on Monday or Tuesday will be great for me
20:09:34 <e0ne> ying_zuo: but I don't really want to change schedule if only me is interested in it
20:10:12 <e0ne> ying_zuo: I mean all day Tuesday
20:10:16 <ying_zuo> I will move it to Tuesday then
20:10:40 <ying_zuo> unless there's other objection...
20:11:30 <e0ne> ying_zuo: thank you. I apprciate it
20:11:40 <ying_zuo> np
20:12:01 <ying_zuo> going back to the bug report guidelines topic...
20:12:13 <ying_zuo> You will notice the report guidelines when you try to open a bug report now. Thanks robcresswell
20:12:26 <ying_zuo> This is to make sure the bug report has the important information and it will help both developers and reviewers.
20:12:56 <ying_zuo> #topic Open Discussion
20:14:07 <e0ne> I hope, my question will be fast enough
20:14:36 <e0ne> myself with rdopiera found that some admin pages could be open by non-admin users: http://paste.openstack.org/show/619772/
20:15:38 <e0ne> what is expected behaviour for /admin/ paged? should render some data for non-admin users or raise NotAuthorized error?
20:15:44 <ying_zuo> do they show admin information?
20:16:02 <e0ne> ying_zuo: no
20:16:03 <david-lyle> identity pages are designed to open based on roles and policy, the test is not admin or not
20:16:05 <rdopiera> they just show messages that they failed to get the information
20:16:20 <david-lyle> is this via going to the URL directly?
20:16:26 <rdopiera> yeah
20:16:34 <david-lyle> shrug
20:16:38 <david-lyle> :)
20:16:38 <rdopiera> I was trying to test the new "not authorized" page
20:16:40 <e0ne> ying_zuo: but sometimes (e.g. hypervisors) it shows almost empty page with error messages like 'can't retrieve data'
20:16:43 <rdopiera> and stumbled upon that
20:17:01 <e0ne> for note: it's reproducible on a current master
20:17:02 <ying_zuo> but that's because the users try to hack the pages
20:17:27 <rdopiera> ying_zuo: or they just changed the user to one that doesn't have the permission
20:17:43 <e0ne> I prefer to show some error and do not try to render such pages
20:17:53 <rdopiera> ying_zuo: which is the reason for working on the not authorized page in the first place
20:18:06 <ying_zuo> rdopiera: what do you mean?
20:18:37 <e0ne> ying_zuo: we're talking about this patch https://review.openstack.org/491479
20:18:37 <ying_zuo> how can they change the user?
20:18:40 <rdopiera> ying_zuo: you can switch domains or other things, and suddenly not have the permission to see the page on which you are currently
20:18:41 <david-lyle> so I'm guessing old policy files
20:19:15 <david-lyle> https://github.com/openstack/horizon/blob/master/openstack_dashboard/dashboards/admin/hypervisors/panel.py#L24
20:19:26 <david-lyle> that policy rule is relatively new
20:19:31 <ying_zuo> I see. so they can do actions that they are not supposed to
20:19:32 <rdopiera> david-lyle: that seems to work fine, as the page is not displayed in the menu
20:19:55 <david-lyle> rdopiera, ok
20:19:57 <david-lyle> hmm
20:19:58 <rdopiera> ying_zuo: no, because the services will check the permissions on their side and not allow that
20:20:10 <rdopiera> ying_zuo: you just get an empty page with a lot of errors
20:20:29 <e0ne> ^^ and it's not a user-friendly at all
20:20:30 <rdopiera> which is not critically bad, in fact could be acceptable
20:20:33 <rdopiera> but a bit messy
20:20:39 <ying_zuo> it's cleaner to just disable the panel
20:20:42 <e0ne> rdopiera: +1
20:20:56 <rdopiera> ying_zuo: the panel is disabled correctly
20:21:19 <rdopiera> ying_zuo: but if you were on that page while you switched domains or users, you will still visit that page
20:21:23 <ying_zuo> I meant setting the policy rule to disalbe it as david-lyle mentioned
20:21:52 <ying_zuo> got it
20:22:19 <rdopiera> we are working on a patch that displays a "not authorized" page then
20:22:34 <rdopiera> and it works on most of the pages, which raise the correct exception
20:22:43 <rdopiera> but those pages for some reason don't
20:23:28 <e0ne> ying_zuo: here is a screenshot of hypervisors page: https://cl.ly/3M423D1Z2b34
20:24:02 <ying_zuo> thanks e0ne
20:24:11 <rdopiera> I think the problem is that they have a catch-all try-except and don't delegate the exception handling to horizon
20:24:22 <rdopiera> but I didn't verify that
20:24:29 <e0ne> ying_zuo: other pages in the list look similar
20:24:57 <e0ne> rdopiera: let's decide what do we want to have first
20:25:29 <e0ne> rdopiera: IMO, we'll be able to fix it really quick if we decide what is excepted behaviour
20:26:11 * e0ne wants quick meetings at 11pm
20:28:06 <robcresswell> I feel like I missed the "solutions" part of the discussion
20:28:35 <ying_zuo> I think just raise an exception. doesn't need to show a "not authorized" page.
20:28:38 <e0ne> robcresswell: +1 :(
20:28:45 <david-lyle> where is unauthorized escaping in admin.instance ?
20:28:57 <robcresswell> So, the Not Authorised page from before is no longer an option?
20:29:07 <rdopiera> robcresswell: it works on most pages
20:29:16 <e0ne> ying_zuo: but we're talking about '/admin/some-page' and non-admin users
20:29:30 <david-lyle> rdopiera, by luck?
20:29:42 <rdopiera> another possibility is that those pages raise some other exception that doesn't get caught by our "not authorized page" mechanism
20:30:02 <rdopiera> david-lyle: by courage
20:30:06 <robcresswell> hahaha
20:30:07 <e0ne> david-lyle, rdopiera: it depends on policies and who raises NotAuthorized exception
20:30:15 <e0ne> rdopiera: :)
20:30:47 <david-lyle> I'm looking at admin.instance get_data and not seeing any non-blanket "except Exception"
20:30:51 <rdopiera> I say let's get the page to work on most pages, and then we can start cleaning up the mess
20:30:56 <david-lyle> so I'm wondering what is working there
20:31:06 <david-lyle> most likely we're leaking exceptions somehow
20:31:08 <robcresswell> Alright so... whats the question here, exactly?
20:31:18 <robcresswell> ah my internet skipped there, once sec
20:31:18 <e0ne> the question is:
20:31:20 <robcresswell> one*
20:31:21 <rdopiera> "what should we do" :)
20:31:37 <e0ne> what is expected result for such cases?
20:32:08 <robcresswell> e0ne: Expected result? Isn't this what we discussed before though? A Not Authorised page with a login box or something
20:32:27 <robcresswell> or a "Go to Login" and "Go to home"
20:32:31 <e0ne> if non-admin user somehow opens e.g. /admin/hypersvisors/ page. what should be?
20:32:39 <david-lyle> robcresswell, so we catch all exceptions other than not authorized?
20:33:00 <robcresswell> david-lyle: Sorry?
20:33:19 <e0ne> robcresswell: it's a bit different case, but I'm OK to show Not Authorized page
20:33:24 <david-lyle> the policy before was not let exception bubble up to the user uncaught
20:33:32 <robcresswell> e0ne: I'm having deja vu, I thought we discussed this a few weeks ago :p
20:33:50 <robcresswell> david-lyle: Ah, I see your train of thought now
20:34:01 <e0ne> robcresswell: not really this issue, but very similar
20:34:04 <david-lyle> which the unauthorized page would cover but at a different level, it seems weird to only bubble that one
20:34:28 <e0ne> robcresswell: now we have pages that partially work for non-admin
20:34:56 <david-lyle> partially work is maybe an overstatement :)
20:35:01 <robcresswell> Not weird, not really. In web stuff I write separately I usually have redirects for specific HTTP codes. (401, 403, off the top of my head)
20:35:03 <david-lyle> they don't load data
20:35:26 <david-lyle> robcresswell, sure, ideally we should be explicit about our exceptions
20:35:46 <e0ne> robcresswell: e.g.: http://10.13.0.43/dashboard/admin/volume_types/ can be both opened by admin and non-admin users
20:35:53 <david-lyle> unfortunately things have gotten lax and we attempt to catch all generally
20:36:01 <e0ne> robcresswell: but non-admins won't see private volume types
20:36:15 <robcresswell> urgh, this "admin" concept
20:36:24 <david-lyle> admin is not a concept
20:36:29 <david-lyle> it's a legacy
20:36:47 <e0ne> IMO, we should not allow for non-admin to open any of /admin/* pages
20:36:51 <robcresswell> I meant specifically the Horizon interpretation of admin
20:36:55 <david-lyle> hiding pages from the nav should be sufficient
20:37:04 <rdopiera> e0ne: there are no admins
20:37:31 <rdopiera> e0ne: permissions defpend on the policies
20:37:48 <e0ne> OK, replace, please, 'admin' to 'users w/o permissions to open this page or view all data on the page'
20:38:18 <david-lyle> if a user wants to go directly to a page what is the harm of being told you're not authorized to get the data on the page?
20:38:22 <robcresswell> So, the issue as described before was something like, if you can see admin/instances on one project, but then you change to one where you can't, Horizon gives you a double login in some circumstances
20:38:53 <rdopiera> david-lyle: no harm, but it's messy, it would be nicer to show a nice "not authorized" message and a link to the login page
20:39:17 <david-lyle> on that login page I've lost all context
20:39:26 <david-lyle> perhaps I'm just scoped to the wrong project
20:39:47 <robcresswell> david-lyle: Horizon passes through that little "next" link though
20:40:13 <robcresswell> Which is problematic if you try a different login, and thats not got permissions either, I *think* is the issue
20:40:17 <david-lyle> with the project you were scoped to visable to the user?
20:40:54 <robcresswell> No, but isn't the idea that you would then login under a different user (magic admin account) to do some action?
20:41:03 <david-lyle> the user could have been correct
20:41:11 <david-lyle> the project scope could have been wrong
20:41:33 <robcresswell> True
20:42:04 * robcresswell is thinking
20:42:12 <david-lyle> I think it's by strange luck that some pages would filter out and other won't at this point. I worry that the behavior will not be reliable
20:42:38 <robcresswell> What's the alternative, show a blank page?
20:43:08 <david-lyle> just show the page with the error message that you don't have proper privilege to view the contents of this page
20:43:20 <david-lyle> that's what the identity pages do
20:43:36 <david-lyle> but they're intended for all users to be able to hit
20:43:55 <robcresswell> I wonder if people would complain about losing the redirect functionality
20:44:25 <lucasxu> i will ;(
20:44:43 <e0ne> robcresswell: we can redirect to login page with a correct '?next=' param
20:45:00 <rdopiera> we discussed that already
20:45:08 <rdopiera> and agreed on it
20:45:12 <e0ne> rdopiera: +1
20:45:20 <robcresswell> e0ne: Right, but I'm saying that if we just displayed a blank page with an error, we lose that
20:45:23 <rdopiera> the problem now is whether all pages should behave consistently or not
20:45:38 <david-lyle> +1 to consistency
20:45:41 <e0ne> robcresswell: page with error and login link
20:45:42 <rdopiera> and how urgent it is
20:45:54 <rdopiera> personally I think it would be nice to be consistent but it's not high priority
20:45:57 <e0ne> rdopiera: +1, I absolutely agree
20:46:00 <robcresswell> e0ne: Didn't we just say we can't reliably do that?
20:46:35 <robcresswell> I *think* what david-lyle was suggesting is that if you go to admin/instances without correct permissions you just show a blank page with "You dont have permissions to do X / Y"
20:47:04 <e0ne> what about page like https://review.openstack.org/#/c/491479/9/horizon/templates/not_authorized.html
20:47:05 <e0ne> ?
20:47:40 <robcresswell> e0ne: You're not reading my questions. Didn't we say earlie that we could not reliably determine that?
20:47:42 <robcresswell> earlier*
20:47:48 <david-lyle> I think with policy that becomes a very gray line
20:48:21 <rdopiera> robcresswell: we can determine very reliably that we got an exception from the api
20:48:36 <e0ne> robcresswell: we can catch needed API from APIs
20:48:41 <robcresswell> "Something went wrong. Would you like to log out?"
20:48:46 <e0ne> *can catch needed exception
20:49:00 <david-lyle> rdopiera, so are you suggest we rewrite all the excepts in the get_data calls?
20:49:22 <robcresswell> e0ne: I thought you and rdopiera earlier said we cannot get the exception reliably
20:49:27 <e0ne> robcresswell: I hope, all APIs can return not authorized. we can handle them correctly
20:49:54 <e0ne> robcresswell: it's only in a *current* implementation of some pages
20:50:02 <rdopiera> david-lyle: something like that, perhaps fix it in the handling of the exceptions, less invasive
20:50:11 <david-lyle> so so except Exception as e, if e is not UnAuthorized?
20:50:39 <rdopiera> david-lyle: we have an exception handler function already that handles most of those exceptions
20:50:46 <rdopiera> david-lyle: but for some reason not all of them
20:51:31 <rdopiera> david-lyle: and it does see those exceptions, because it displays the messages
20:52:07 <e0ne> 10mins reminder
20:52:17 <david-lyle> so https://github.com/openstack/horizon/blob/master/openstack_dashboard/exceptions.py#L29 isn't broad enough?
20:52:33 <rdopiera> apparently
20:52:58 <david-lyle> hypervisors doesn't use novaclient.Unauthorized?
20:53:00 <rdopiera> or some other special case is happening
20:53:15 <rdopiera> I didn't analyze the code enough to be sure
20:53:18 <e0ne> david-lyle: the issue is, sometimes we catch Exception and return error message to user instead of re-raising NonAuthorized
20:54:22 * robcresswell reminds everyone we have crippling performance issues on other pages, even if users sometimes have to log in twice when they try and view a page they dont have permissions for.
20:54:43 <rdopiera> e0ne: that coould make sense when the page contains multiple things, and some of them the user has permission to see
20:55:01 <e0ne> rdopiera: good point
20:55:12 <rdopiera> so maybe let's leave it as it is for now
20:55:18 <robcresswell> e0ne: If it is possible to correctly identify that a NotAuthorised exception is there in all cases, then we can consider doing something special
20:55:18 <david-lyle> e0ne, I understand. My two points are I think a blanket unauthorized for a page that attempts to make several API calls may not be correct and two, it seems you'll have to reverse engineer all client authorization failures to handle them properly
20:55:43 <e0ne> david-lyle: I agree
20:55:51 <e0ne> looks like we have a decision:
20:56:17 <david-lyle> to me the unauthorized page is a carry over from the admin/non-admin days when things were binary
20:56:34 <rdopiera> it's better than the redirect
20:56:41 <e0ne> try to handle NotAuthorised if we can or leave it as is for now
20:58:19 <e0ne> 2mins reminder
20:58:29 <ying_zuo> cool. I think that's it for today. Thanks everyone for attending.
20:58:32 <rdopiera> tick tock
20:58:41 <rdopiera> goodnight
20:58:54 <e0ne> see you next week
20:58:54 <ying_zuo> #endmeeting