*** marcoemorais has quit IRC | 00:01 | |
*** marcoemorais has joined #openstack-keystone | 00:02 | |
*** alex_xu has quit IRC | 00:03 | |
*** gokrokve has joined #openstack-keystone | 00:14 | |
openstackgerrit | Jamie Lennox proposed a change to openstack/keystonemiddleware: Convert true string parsing to use strutils https://review.openstack.org/115438 | 00:16 |
---|---|---|
*** dims has joined #openstack-keystone | 00:18 | |
*** henrynash has quit IRC | 00:19 | |
jamielennox | is there a global state issue if i take the conf values passed to auth_token and use them to override CONF and kill off _conf_get? | 00:20 |
*** gokrokve has quit IRC | 00:21 | |
*** gokrokve has joined #openstack-keystone | 00:21 | |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Update AuthContextMiddleware to not use token_api https://review.openstack.org/113429 | 00:21 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Add __repr__ to KeystoneToken model https://review.openstack.org/113430 | 00:22 |
jamielennox | gyee: ^^ | 00:22 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Add extra guarding to revoke_by_audit_id methods https://review.openstack.org/115147 | 00:22 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove trust dependency on token_api https://review.openstack.org/109462 | 00:22 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove SAML2 plugin dependency on token_api https://review.openstack.org/115012 | 00:22 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove identity_api dependency on token_api https://review.openstack.org/115045 | 00:23 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove wsgi and base controller dependency on token_api https://review.openstack.org/115205 | 00:23 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Notification Constant Cleanup and internal notify type https://review.openstack.org/115337 | 00:23 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove assignment_api dependency on token_api https://review.openstack.org/115338 | 00:23 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove oauth controller dependency on token_api https://review.openstack.org/115343 | 00:23 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Mark methods on token_api deprecated https://review.openstack.org/115347 | 00:23 |
*** bknudson has joined #openstack-keystone | 00:24 | |
gyee | jamielennox, global state? not sure if I understand the question | 00:27 |
jamielennox | gyee: so i have my cool little session.load_from_conf_options function | 00:27 |
jamielennox | which was pretty much designed to mirror what we needed in middleware | 00:27 |
jamielennox | and then i now realize that CONF isn't enough because we also accept variables from paste.ini which comes through as parameters | 00:28 |
jamielennox | so if i load_from_conf_options i will miss anything coming via parameter | 00:28 |
jamielennox | i was looking at doing a CONF.set_override(k, v, group='keystone_authtoken') for every element in the paste params | 00:29 |
gyee | ahh, I see what you mean | 00:29 |
jamielennox | but i go from a local state to a global state | 00:29 |
jamielennox | i just can't tell if that matters | 00:29 |
jamielennox | i don't know if i have any other choice, i can do away with worrying about session.load_ but when it gets down to auth plugins that's a lot more difficult to emulate | 00:32 |
*** ncoghlan_afk is now known as ncoghlan | 00:32 | |
openstackgerrit | Steve Martinelli proposed a change to openstack/keystone: Add CADF notifications for role assignment create and delete https://review.openstack.org/112204 | 00:33 |
gyee | jamielennox, overriding global conf seem like a bad idea | 00:33 |
gyee | but plugins have access to global conf right? | 00:34 |
gyee | so I am guessing you will have to do that _conf_get dance there | 00:34 |
jamielennox | right, i can construct the session pretty simply | 00:35 |
jamielennox | but its a fair bit to do to construct a plugin | 00:36 |
jamielennox | stupid CONF should never have been global | 00:37 |
gyee | jamielennox, I hear ya, the group registration is inconsistent | 00:40 |
gyee | nova api-paste.ini have it as 'authtoken' | 00:40 |
gyee | https://github.com/openstack/nova/blob/master/etc/nova/api-paste.ini#L117 | 00:40 |
gyee | but we are register the opts in the "keystone_authtoken" group | 00:41 |
gyee | so essentially, the same opt can come from multiple places | 00:42 |
*** ncoghlan is now known as ncoghlan_afk | 00:42 | |
jamielennox | yea, it doesn't matter because paste just takes whatever else is in that ini section and passes it through to the object creation | 00:44 |
jamielennox | so authtoken is ignored in that sense | 00:44 |
jamielennox | everything about oslo.config is private as well, there's no where i can override anything | 00:45 |
gyee | ha | 00:45 |
openstackgerrit | Steve Martinelli proposed a change to openstack/keystone: Add CADF notifications for role assignment create and delete https://review.openstack.org/112204 | 00:46 |
*** marcoemorais has quit IRC | 00:47 | |
*** cjellick has quit IRC | 00:47 | |
*** cjellick has joined #openstack-keystone | 00:48 | |
*** cjellick has quit IRC | 00:52 | |
*** xianghui has quit IRC | 00:54 | |
*** gokrokve_ has joined #openstack-keystone | 00:56 | |
*** xianghui has joined #openstack-keystone | 00:58 | |
*** gokrokve has quit IRC | 00:59 | |
*** xianghui has quit IRC | 01:07 | |
openstackgerrit | Jamie Lennox proposed a change to openstack/keystonemiddleware: Wrap a CONF object and intercept authtoken options https://review.openstack.org/115451 | 01:07 |
*** xianghui has joined #openstack-keystone | 01:07 | |
*** jorge_munoz has joined #openstack-keystone | 01:08 | |
jamielennox | gyee: ^^ yuk | 01:08 |
openstackgerrit | Steve Martinelli proposed a change to openstack/keystone: Create additional docs for role assignment events https://review.openstack.org/114813 | 01:10 |
*** gokrokve_ has quit IRC | 01:13 | |
morganfainberg | jamielennox, there is a bug that fix will also close. | 01:13 |
morganfainberg | jamielennox, fyi | 01:13 |
jamielennox | morganfainberg: oh? cause at the moment i want to drown it | 01:14 |
morganfainberg | well if you can turn the values from paste into something that oslo can consume | 01:14 |
morganfainberg | it would be much better | 01:14 |
jamielennox | morganfainberg: so i was talking about that eariler in that it would be nice if i could just CONF.set_override on anything that comes in via paste | 01:16 |
jamielennox | but it's kind of a local state vs global state problem | 01:16 |
*** ayoung has quit IRC | 01:17 | |
morganfainberg | right | 01:18 |
jamielennox | what bug are you referring to? | 01:19 |
*** gokrokve has joined #openstack-keystone | 01:23 | |
*** dims has quit IRC | 01:26 | |
*** ncoghlan_afk is now known as ncoghlan | 01:26 | |
morganfainberg | jamielennox, can't find it right now, but i know it's around | 01:27 |
jamielennox | morganfainberg: you remember what the issue was? | 01:27 |
jamielennox | mostly that it was central to auth_token and not somebody else trying to read config options from us and missing paste options | 01:28 |
*** gokrokve_ has joined #openstack-keystone | 01:34 | |
morganfainberg | jamielennox, nope, i'll need to dig around to try and figure out what the issue really was | 01:37 |
jamielennox | morganfainberg: it's ok, i would just feel better about such a hack if there was another reason | 01:37 |
*** gokrokve has quit IRC | 01:37 | |
morganfainberg | maybe we should start telling people to put the configs in their normal conf files and long-term deprecate the paste-ini configs for middleware? | 01:38 |
*** gokrokve_ has quit IRC | 01:39 | |
*** gordc has joined #openstack-keystone | 01:44 | |
*** richm has quit IRC | 01:47 | |
bknudson | morganfainberg: I think the goal was to deprecate the options in paste.ini a long time ago | 01:49 |
bknudson | we could not support it for new options. | 01:50 |
openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Enhance GET /v3 to handle Accept header https://review.openstack.org/115462 | 01:54 |
openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Enhance GET /v3 to handle Accept header https://review.openstack.org/115462 | 01:54 |
bknudson | jamielennox: morganfainberg: the problem was that there was an integer config option but from paste.ini it's a string. | 01:58 |
morganfainberg | bknudson, thats the one! | 01:58 |
bknudson | https://review.openstack.org/#/c/113191/ | 01:58 |
bknudson | there's already a fix for it! | 01:59 |
jamielennox | oh, yea and it messes with delay_auth_decision because otherwise the BoolOpt would handle it's type | 01:59 |
jamielennox | i only saw that the other day, should have realized | 01:59 |
bknudson | jamielennox: morganfainberg: the fix doesn't look too bad. | 02:00 |
*** shakamunyi has quit IRC | 02:01 | |
jamielennox | bknudson: it's kind of insignificant i'm just not a fan of the loop | 02:02 |
jamielennox | the way auth_token is written we will only read conf options once anyway | 02:02 |
bknudson | loops are evil! | 02:02 |
jamielennox | i'd prefer he just put it in _conf_get | 02:02 |
bknudson | jamielennox: then it would have to find the option every time _conf_get is called... gross. | 02:03 |
jamielennox | bknudson: hmm, move opts to a dict | 02:03 |
bknudson | the change also seems to make assumptions about CONF options that I'd rather weren't made | 02:04 |
bknudson | are they really all going to process the value using type() ? | 02:04 |
bknudson | does that handle config options with max and min values correctly? | 02:04 |
bknudson | seems like conf should work that way. | 02:05 |
bknudson | dhellmann had a different suggestion | 02:05 |
*** xianghui has quit IRC | 02:07 | |
*** xianghui has joined #openstack-keystone | 02:08 | |
jamielennox | yea, types are callable and do that stuff | 02:08 |
dhellmann | bknudson, jamielennox : at the very least the option definitions could be saved to a dict to avoid the O(n^2) looping, but I think you should look at set_default(), too, because that should do some of what this is trying to do | 02:13 |
jamielennox | dhellmann: the problem is similar to what we've been talking about in channel recently | 02:14 |
jamielennox | dhellmann: we have two sources of config, CONF and the params from paste.ini | 02:14 |
jamielennox | if we use set_default or set_override on CONF then we are taking state local to auth_token and setting it globally | 02:14 |
*** alex_xu has joined #openstack-keystone | 02:14 | |
dhellmann | if set_default() barfs when you pass a string as the default for an int option, we should look at whether we can make it try the type conversion for you | 02:14 |
dhellmann | jamielennox: have you seen the config filter stuff markmc added this cycle? | 02:15 |
jamielennox | dhellmann: i remember being referred to it for something else but i haven't looked closely | 02:15 |
dhellmann | jamielennox: http://docs.openstack.org/developer/oslo.config/cfgfilter.html | 02:15 |
jamielennox | my understanding that was more protection though | 02:15 |
jamielennox | so that nobody outside of your module could modify your state | 02:15 |
dhellmann | jamielennox: right, that avoids the global issue I thought you were talking about | 02:16 |
jamielennox | dhellmann: i guess the (mostly theoretical) problem is that people could start multipe auth_tokens from different paste.ini pipelines with different configs | 02:17 |
dhellmann | when you say "local to auth_token" do you mean an individual user token? | 02:17 |
jamielennox | no its provided at __init__ to auth_token | 02:17 |
dhellmann | jamielennox: ok, different pipelines makes sense | 02:17 |
dhellmann | they have names or something, right? | 02:17 |
dhellmann | you can register the options more than once in different groups, as long as you can come up with unique names, but maybe you're right and this shouldn't be in the global config object | 02:18 |
jamielennox | i don't think names bubble down into code | 02:18 |
jamielennox | dhellmann: looking at http://docs.openstack.org/developer/oslo.config/cfgfilter.html#private-configuration-options | 02:18 |
dhellmann | ok, yeah, maybe we need to think about how to expose some of the config stuff for reuse then instead of trying to push the data into the config object in this case | 02:19 |
jamielennox | actually that might work | 02:19 |
jamielennox | self._private_conf = ConfigFilter(self.conf) | 02:20 |
jamielennox | self._private_conf.register_opt(StrOpt('foo')) | 02:20 |
dhellmann | there's nothing fundamentally wrong with what's being done in https://review.openstack.org/#/c/113191/2/keystonemiddleware/auth_token.py except the O(n^2) loop, assuming the type attribute is meant to be exposed | 02:20 |
jamielennox | if the option is registered on _private_conf only then it's fine, if the option is registered on CONF but only accessible by _private_conf we have the same problem | 02:20 |
dhellmann | jamielennox: you would still need different groups, though, because underneath I think the filter is using the same storage for option values | 02:20 |
dhellmann | ah, true, you don't have options in that storage, so using set_default is going to update the option definition | 02:21 |
dhellmann | so maybe the filter would do what you want | 02:21 |
jamielennox | dhellmann: at the moment i'm looking at something like: https://review.openstack.org/#/c/115451/ | 02:21 |
*** ncoghlan has quit IRC | 02:21 | |
jamielennox | for a slightly different reason, but to make the values from paste look like they came via conf | 02:22 |
jamielennox | it's pretty horrible | 02:22 |
*** jimbaker has quit IRC | 02:22 | |
dhellmann | I suppose we can't change the things that requrie the conf object to use some other object that can be constructed from different data sources? | 02:23 |
*** jimbaker has joined #openstack-keystone | 02:23 | |
jamielennox | not really | 02:23 |
*** jimbaker has quit IRC | 02:23 | |
*** jimbaker has joined #openstack-keystone | 02:23 | |
dhellmann | yeah | 02:23 |
dhellmann | at least they take the object as an argument, so you should be able to pass the filter instead | 02:23 |
*** gordc has quit IRC | 02:24 | |
jamielennox | how do those options then get exposed to a config generator? | 02:24 |
*** diegows has quit IRC | 02:25 | |
dhellmann | these options can be set in the global config as well? | 02:25 |
dhellmann | oh, yeah, you said two sources, didn't you | 02:25 |
jamielennox | because i would need to move nearly the entire OPT list into private_configs | 02:25 |
jamielennox | yes, it needs to work as a regular CONF as well | 02:26 |
dhellmann | I'm curious, what's the use case for this? | 02:27 |
dhellmann | I'm not sure what would happen if you have a global option set and then try to use the filter for overrides. That may not work. But I could see that as a useful feature, and a class similar to the filter but that did that could be added to oslo.config | 02:28 |
jamielennox | all of the session and auth loading work is written to be used by everyone by session.load_from_conf_object auth.load_from_conf_object | 02:28 |
jamielennox | this was fine until realizing that it's not just a CONF object we have, we have to include the paste.ini attributes as well | 02:29 |
dhellmann | so you could have a ConfigOverride that takes a ConfigParser and a bunch of overrides and uses the overrides or the underlying values, but doesn't change the values inside the parser so a different set of overrides could be used elsewhere | 02:29 |
dhellmann | why are there auth settings in the paste file? | 02:29 |
jamielennox | dhellmann: i'm not sure, apparently we've tried to kill them in the past but people keep putting them there | 02:30 |
dhellmann | jamielennox: ok, I won't dredge that history up then :-) | 02:30 |
dhellmann | jamielennox: let's see if we can find a way to build something like the ConfigOverrides I described ^^ and put it in oslo.config instead of you guys hacking around it just in keystone, because it might be of use to the swift guys, too | 02:30 |
bknudson | I think we can live without supporting new options in api-paste.ini | 02:30 |
jamielennox | ok so a ConfigOverride object is pretty much what i'm looking for, a 'local view' of the CONF | 02:30 |
dhellmann | exactly | 02:31 |
bknudson | support the ones that are already there but don't bother with new ones | 02:31 |
dhellmann | and the filter is one type of view like that, but it's sort of backwards for what you need in this case | 02:31 |
dhellmann | bknudson: that might turn out to be more confusing, in the end, than just allowing all of the options to be overridden | 02:31 |
bknudson | I agree it will be confusing and hopefully that will convince people to stop putting stuff in api-paste.ini. | 02:32 |
dhellmann | jamielennox: I could see ConfigOverrides having an invariant that the option in question has to already be registered, for instance, where the filter expects that it is not | 02:32 |
dhellmann | bknudson: :-) | 02:32 |
jamielennox | bknudson: i was thinking that anything new from an auth plugin perspective could ignore paste.ini | 02:32 |
jamielennox | bknudson: i think he meant confusing for us :) | 02:33 |
dhellmann | jamielennox: no, I did actually mean deployers ("why can I set foo in paste.ini but not bar?") | 02:34 |
*** gordc has joined #openstack-keystone | 02:34 | |
jamielennox | ok, well both then | 02:34 |
*** gordc has quit IRC | 02:34 | |
dhellmann | but I think that only goes as far as this "class" of options (the auth_token stuff) and not *all* options | 02:34 |
*** shakamunyi has joined #openstack-keystone | 02:34 | |
jamielennox | would a view have the option to register? | 02:34 |
jamielennox | register new options on it | 02:35 |
dhellmann | jamielennox: no, I wouldn't expect it to, but you could always combine it with a filter in a stack | 02:35 |
dhellmann | filter(view(parser())) | 02:35 |
*** harlowja is now known as harlowja_away | 02:35 | |
dhellmann | in principle it should be possible to nest views and filters arbitrarily | 02:35 |
dhellmann | jamielennox: although I've never tried that, so I don't know if it actually works :-) | 02:36 |
jamielennox | maybe it does need to be called an override, a view sounds like it's restricting it or modifying it somehow | 02:38 |
*** vkmc has quit IRC | 02:38 | |
dhellmann | yeah | 02:38 |
*** shakamunyi has quit IRC | 02:41 | |
*** shakamunyi has joined #openstack-keystone | 02:42 | |
jamielennox | dhellmann: alright, i'll look into that. It'll require a bit more figuring out of oslo.config, but it's a better way of doing it that the hacks i'm looking at | 02:44 |
jamielennox | dhellmann: thanks | 02:44 |
dhellmann | jamielennox: np, thanks, and ping me in openstack-oslo when you're ready for a review so I definitely see it. I'm at a conference this week, but I'll be checking in regularly | 02:45 |
jamielennox | dhellmann: will do | 02:45 |
*** KanagarajM has joined #openstack-keystone | 03:00 | |
*** xianghui has quit IRC | 03:09 | |
*** xianghui has joined #openstack-keystone | 03:10 | |
*** rushiagr_away has quit IRC | 03:12 | |
*** rushiagr_away has joined #openstack-keystone | 03:14 | |
*** dims has joined #openstack-keystone | 03:14 | |
*** xianghui has quit IRC | 03:16 | |
*** alex_xu has quit IRC | 03:17 | |
*** xianghui has joined #openstack-keystone | 03:17 | |
*** dims has quit IRC | 03:23 | |
*** alex_xu has joined #openstack-keystone | 03:23 | |
openstackgerrit | Kanagaraj Manickam proposed a change to openstack/keystone: endpoint table is missing reference to region table https://review.openstack.org/113183 | 03:49 |
*** gokrokve has joined #openstack-keystone | 03:57 | |
*** rushiagr_away is now known as rushiagr | 03:59 | |
*** chandankumar has joined #openstack-keystone | 03:59 | |
*** gokrokve has quit IRC | 04:00 | |
*** gokrokve has joined #openstack-keystone | 04:00 | |
*** oomichi has joined #openstack-keystone | 04:01 | |
*** gokrokve has quit IRC | 04:14 | |
*** gokrokve has joined #openstack-keystone | 04:15 | |
*** xianghui has quit IRC | 04:17 | |
*** xianghui has joined #openstack-keystone | 04:17 | |
*** amirosh has joined #openstack-keystone | 04:18 | |
*** gokrokve has quit IRC | 04:40 | |
*** gokrokve has joined #openstack-keystone | 04:41 | |
*** gokrokve has quit IRC | 04:45 | |
*** chandankumar has quit IRC | 04:52 | |
*** stevemar has quit IRC | 04:54 | |
*** chandankumar has joined #openstack-keystone | 04:59 | |
*** chandankumar has quit IRC | 05:02 | |
*** dims has joined #openstack-keystone | 05:07 | |
*** dims has quit IRC | 05:11 | |
*** ajayaa has joined #openstack-keystone | 05:19 | |
*** ajayaa has quit IRC | 05:34 | |
*** chandankumar has joined #openstack-keystone | 05:40 | |
*** jimbaker has quit IRC | 05:42 | |
*** jimbaker has joined #openstack-keystone | 05:46 | |
*** jimbaker has quit IRC | 05:46 | |
*** jimbaker has joined #openstack-keystone | 05:46 | |
*** jorge_munoz has quit IRC | 05:47 | |
*** jorge_munoz has joined #openstack-keystone | 05:47 | |
*** jorge_munoz has quit IRC | 05:51 | |
*** tomoiaga has joined #openstack-keystone | 05:55 | |
*** ajayaa has joined #openstack-keystone | 06:03 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/keystone: Imported Translations from Transifex https://review.openstack.org/111920 | 06:06 |
*** afazekas has quit IRC | 06:11 | |
*** afazekas_ has quit IRC | 06:16 | |
*** marekd|away is now known as marekd | 06:38 | |
*** shakamunyi has quit IRC | 06:43 | |
*** shakamunyi has joined #openstack-keystone | 06:44 | |
*** shakamunyi has quit IRC | 06:47 | |
*** shakamunyi has joined #openstack-keystone | 06:48 | |
*** afazekas_ has joined #openstack-keystone | 06:57 | |
openstackgerrit | wanghong proposed a change to openstack/keystonemiddleware: convert the conf value into correct type https://review.openstack.org/113191 | 07:08 |
*** afazekas_ is now known as afazekas | 07:32 | |
*** henrynash has joined #openstack-keystone | 07:41 | |
*** tomoiaga has quit IRC | 07:56 | |
*** alex_xu has quit IRC | 07:59 | |
*** oomichi has quit IRC | 08:07 | |
openstackgerrit | Marek Denis proposed a change to openstack/keystone: IdP SAML Metadata generator https://review.openstack.org/114850 | 08:10 |
*** alex_xu has joined #openstack-keystone | 08:11 | |
mhu | marekd, I don't know if you answered me yesterday, my irc client crashed :/ | 08:20 |
*** jimbaker has quit IRC | 08:21 | |
openstackgerrit | Marek Denis proposed a change to openstack/keystone: IdP SAML Metadata generator https://review.openstack.org/114850 | 08:21 |
marekd | mhu: yes, I did | 08:21 |
marekd | mhu: let me re-paste the link :-) | 08:21 |
marekd | mhu: how about this piece od code: https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/contrib/auth/v3/saml2.py#L416 ? | 08:22 |
marekd | (already in the master) | 08:22 |
marekd | mhu: https://review.openstack.org/#/c/110770/ this is unmerged, but it's easy to change locally :-) | 08:23 |
marekd | mhu: and since osc is not ready yet I made a super-simple wrapper for that: https://gist.github.com/zaccone/509136cfa1c4efca6926 | 08:23 |
*** jimbaker has joined #openstack-keystone | 08:25 | |
*** jimbaker has quit IRC | 08:25 | |
*** jimbaker has joined #openstack-keystone | 08:25 | |
mhu | marekd, thanks, I see - I guess the version of python-keystoneclient in the requirements is too old to reflect this change, this plugin isn't loaded in stevedore | 08:25 |
*** shakamunyi has quit IRC | 08:25 | |
marekd | mhu: it's in the master but it was not released in the keystoneclient. | 08:25 |
marekd | mhu: mainly due to we wanted to push bunch of plugins so ksc can be used for federated authN | 08:26 |
marekd | and it took me some time to push it through reviews ;/ | 08:26 |
marekd | mhu: so I advise checking master, not pip | 08:26 |
mhu | marekd, I'll see with stevemar if I can get a minor release, otherwise I'll get the same problems with pbr as before ... | 08:27 |
marekd | mhu: dolphm is responsible for cutting the ksc. | 08:27 |
marekd | mhu: unless you need osc version. | 08:27 |
mhu | marekd, so he's the one I'll bug then :) | 08:28 |
marekd | mhu: than stevemar is a gatekeeper :-) | 08:28 |
*** alex_xu has quit IRC | 08:28 | |
marekd | probably not before tomorrow as everybody is rushing due to PFP | 08:28 |
mhu | marekd, it's alright, it'll give me time to patch something in ksc so that I can allow the use of admin tokens in osc | 08:29 |
marekd | mhu: you opinions and reviews on this: https://review.openstack.org/#/c/110770/ and https://review.openstack.org/#/c/106751/ are highly appreciated. Also how you see it from OSC point of view. | 08:30 |
mhu | (they don't work with the regular v*token plugin) | 08:30 |
mhu | marekd, I'll give them a look, the last one definitely had me scratching my head on the OSC side :) so if you're dealing with it in there, all the better | 08:31 |
marekd | the wrapper? | 08:32 |
marekd | mhu: i think this should be resolved on the lib level, not OSC | 08:32 |
mhu | marekd, yes, therefore my question about v3samlscoped | 08:32 |
*** Dafna has quit IRC | 08:33 | |
marekd | mhu: uhm | 08:33 |
*** Dafna has joined #openstack-keystone | 08:34 | |
mhu | marekd, I am going to give it a proper look | 08:34 |
marekd | mhu: thanks. | 08:39 |
*** alex_xu has joined #openstack-keystone | 08:42 | |
openstackgerrit | Marek Denis proposed a change to openstack/keystone: Create SAML generation route and controller https://review.openstack.org/114138 | 08:43 |
*** shakamunyi has joined #openstack-keystone | 08:52 | |
*** shakamunyi has quit IRC | 08:56 | |
*** tomoiaga has joined #openstack-keystone | 08:57 | |
*** alex_xu has quit IRC | 09:11 | |
openstackgerrit | henry-nash proposed a change to openstack/keystone: Implements backend for policy endpoint extension https://review.openstack.org/115362 | 09:17 |
*** henrynash has quit IRC | 09:30 | |
*** ajayaa has quit IRC | 09:37 | |
*** jeffrey4l has joined #openstack-keystone | 09:47 | |
jeffrey4l | Any core viewer can review my patch https://review.openstack.org/#/c/51610/ | 09:50 |
jeffrey4l | Any core reviewer can review my patch https://review.openstack.org/#/c/51610/ | 09:50 |
*** ajayaa has joined #openstack-keystone | 09:52 | |
openstackgerrit | Marek Denis proposed a change to openstack/keystone: Transform a Keystone token to a SAML assertion https://review.openstack.org/110542 | 09:52 |
*** shakamunyi has joined #openstack-keystone | 09:53 | |
openstackgerrit | Marek Denis proposed a change to openstack/keystone: IdP SAML Metadata generator https://review.openstack.org/114850 | 09:54 |
*** shakamunyi has quit IRC | 09:57 | |
openstackgerrit | Marcos Fermín Lobo proposed a change to openstack/python-keystoneclient: Attributes required using token for auth https://review.openstack.org/115228 | 09:59 |
*** openstackgerrit has quit IRC | 10:10 | |
*** aix has joined #openstack-keystone | 10:19 | |
*** ajayaa has quit IRC | 10:42 | |
*** Kui has quit IRC | 10:45 | |
*** amerine_ has joined #openstack-keystone | 10:59 | |
*** amerine has quit IRC | 11:01 | |
*** ajayaa has joined #openstack-keystone | 11:03 | |
*** ajayaa has quit IRC | 11:20 | |
*** ajayaa has joined #openstack-keystone | 11:21 | |
*** ajayaa has quit IRC | 11:28 | |
*** ajayaa has joined #openstack-keystone | 11:34 | |
*** henrynash has joined #openstack-keystone | 11:42 | |
*** RicoLin has quit IRC | 11:45 | |
*** RicoLin has joined #openstack-keystone | 11:46 | |
*** diegows has joined #openstack-keystone | 11:50 | |
*** dims has joined #openstack-keystone | 11:50 | |
*** KanagarajM has quit IRC | 12:06 | |
*** alex_xu has joined #openstack-keystone | 12:17 | |
*** ajayaa has quit IRC | 12:24 | |
*** ajayaa has joined #openstack-keystone | 12:35 | |
*** hrybacki has joined #openstack-keystone | 12:38 | |
*** gordc has joined #openstack-keystone | 12:42 | |
*** miqui has joined #openstack-keystone | 12:43 | |
*** henrynash has quit IRC | 12:44 | |
*** andreaf has joined #openstack-keystone | 12:51 | |
*** andreaf has quit IRC | 12:51 | |
*** andreaf has joined #openstack-keystone | 12:51 | |
*** andreaf has quit IRC | 12:51 | |
*** hrybacki has quit IRC | 12:52 | |
*** jimbaker has quit IRC | 12:52 | |
*** bknudson has quit IRC | 12:53 | |
*** jimbaker has joined #openstack-keystone | 12:54 | |
*** jimbaker has quit IRC | 12:54 | |
*** jimbaker has joined #openstack-keystone | 12:54 | |
*** aix has quit IRC | 13:00 | |
*** stevemar has joined #openstack-keystone | 13:01 | |
*** aix has joined #openstack-keystone | 13:03 | |
*** henrynash has joined #openstack-keystone | 13:08 | |
*** nkinder has quit IRC | 13:11 | |
*** richm has joined #openstack-keystone | 13:12 | |
*** radez_g0n3 is now known as radez | 13:16 | |
*** henrynash has quit IRC | 13:31 | |
*** bknudson has joined #openstack-keystone | 13:32 | |
*** amcrn has joined #openstack-keystone | 13:35 | |
*** ajayaa has quit IRC | 13:37 | |
*** amcrn has quit IRC | 13:40 | |
*** amcrn has joined #openstack-keystone | 13:57 | |
*** samuelmz_ has joined #openstack-keystone | 14:08 | |
*** aix has quit IRC | 14:08 | |
*** nkinder has joined #openstack-keystone | 14:10 | |
*** gokrokve has joined #openstack-keystone | 14:11 | |
*** amcrn has quit IRC | 14:14 | |
*** jorge_munoz has joined #openstack-keystone | 14:14 | |
*** aix has joined #openstack-keystone | 14:20 | |
*** samuelmz_ has quit IRC | 14:22 | |
*** rwsu has quit IRC | 14:24 | |
samuelmz | Hi, I'm part of the team that is in charge of implement the hierarchical projects concept. We have to add support on the role_assignments API. Looking at the current implementation, we saw that it's quite inefficient, then we just reported a bug to improve its performance. It's better to have this refactoring in order to implement the hierarchical projects concept on top of it. | 14:27 |
samuelmz | Could you take a look at the bug and confirm it? (https://bugs.launchpad.net/keystone/+bug/1359231) | 14:28 |
uvirtbot | Launchpad bug 1359231 in keystone "List role assignments filters performance " [Undecided,New] | 14:28 |
*** amirosh has quit IRC | 14:29 | |
*** david-lyle has joined #openstack-keystone | 14:30 | |
*** amirosh has joined #openstack-keystone | 14:30 | |
*** david-lyle has quit IRC | 14:31 | |
*** david-lyle has joined #openstack-keystone | 14:31 | |
*** amirosh has quit IRC | 14:34 | |
*** rwsu has joined #openstack-keystone | 14:36 | |
*** amcrn has joined #openstack-keystone | 14:38 | |
*** marekd is now known as marekd|away | 14:43 | |
*** hrybacki has joined #openstack-keystone | 14:43 | |
*** tomoiaga has quit IRC | 14:43 | |
dolphm | dstanek: is there a way in jsonschema to say property X xor property Y is required? | 14:45 |
dolphm | dstanek: mutliple object definitions with oneOf? | 14:46 |
dstanek | dolphm: yes, that's the only way i've seen it done; i don't think there is any direct support for it | 14:47 |
dolphm | alrighty | 14:47 |
*** hrybacki has quit IRC | 14:47 | |
dstanek | dolphm: you're basically left to duplication | 14:47 |
*** openstackgerrit has joined #openstack-keystone | 14:59 | |
*** shakamunyi has joined #openstack-keystone | 15:00 | |
*** ayoung has joined #openstack-keystone | 15:04 | |
*** zzzeek has joined #openstack-keystone | 15:05 | |
*** amirosh has joined #openstack-keystone | 15:08 | |
*** openstackgerrit has joined #openstack-keystone | 15:12 | |
openstackgerrit | David Stanek proposed a change to openstack/keystone: Sync Py2 and Py3 requirements files https://review.openstack.org/115678 | 15:14 |
*** henrynash has joined #openstack-keystone | 15:17 | |
*** gokrokve_ has joined #openstack-keystone | 15:20 | |
*** mrmoje has joined #openstack-keystone | 15:21 | |
*** topol has joined #openstack-keystone | 15:21 | |
*** gokrokve has quit IRC | 15:23 | |
*** jeffrey4l has quit IRC | 15:23 | |
*** gokrokve_ has quit IRC | 15:24 | |
*** cjellick has joined #openstack-keystone | 15:25 | |
*** BAKfr has joined #openstack-keystone | 15:26 | |
*** amcrn has quit IRC | 15:28 | |
openstackgerrit | Steve Martinelli proposed a change to openstack/keystone: Create SAML generation route and controller https://review.openstack.org/114138 | 15:32 |
*** jeffrey4l has joined #openstack-keystone | 15:36 | |
dstanek | henrynash: in https://review.openstack.org/#/c/112292/ you can't have more than one policy associated with an endpoint can you? | 15:36 |
dstanek | i assume not because you call out that no merging of policies occurs | 15:36 |
*** amirosh has quit IRC | 15:41 | |
*** amirosh has joined #openstack-keystone | 15:42 | |
*** rushiagr is now known as rushiagr_away | 15:44 | |
*** rushiagr_away is now known as rushiagr | 15:44 | |
*** amirosh has quit IRC | 15:46 | |
henrynash | dstanek: explicitely to a given endpoint, no | 15:47 |
raildo | henrynash: ping | 16:09 |
henrynash | raildo: hi | 16:09 |
dstanek | morganfainberg: ping | 16:11 |
*** henrique_ has quit IRC | 16:13 | |
*** raildo has quit IRC | 16:13 | |
*** samuelmz has quit IRC | 16:14 | |
*** rodrigods has quit IRC | 16:14 | |
*** BAKfr has quit IRC | 16:14 | |
*** raildo has joined #openstack-keystone | 16:15 | |
*** htruta has joined #openstack-keystone | 16:15 | |
raildo | henrynash: I want to know if there's anything I can do to approve the spec about hierarchical projects. | 16:17 |
henrynash | raildo: it’s done! | 16:20 |
henrynash | raildo: now the eral fun begins... | 16:20 |
ayoung | nkinder, I think this change is pretty important to us. Please review when you get a chance: https://review.openstack.org/#/c/105597/13 as it will open up the mapping plugin for Kerberos and mod_id_lookup | 16:20 |
henrynash | real even | 16:20 |
raildo | henrynash: :-D:-D:-D:-D | 16:21 |
raildo | henrynash: thank you! | 16:21 |
openstackgerrit | A change was merged to openstack/keystone-specs: Hierarchical Multitenacy https://review.openstack.org/101017 | 16:23 |
*** k4n0 has quit IRC | 16:25 | |
*** gyee_ has joined #openstack-keystone | 16:27 | |
*** gyee_ has left #openstack-keystone | 16:28 | |
*** afazekas has quit IRC | 16:29 | |
*** gyee_ has joined #openstack-keystone | 16:30 | |
*** rushiagr is now known as rushiagr_away | 16:32 | |
*** samuelmz has joined #openstack-keystone | 16:32 | |
ayoung | morganfainberg, in https://review.openstack.org/#/c/114864/8/keystone/token/provider.py,cm did you intend to leave this code: | 16:34 |
ayoung | if revoke_by_expires: | 16:34 |
ayoung | self.revoke_api.revoke_by_expiration(user_id, expires_at, | 16:34 |
ayoung | project_id=project_id, | 16:34 |
ayoung | domain_id=domain_id) | 16:34 |
*** chandankumar has quit IRC | 16:37 | |
*** hrybacki has joined #openstack-keystone | 16:37 | |
*** rushiagr_away is now known as rushiagr | 16:37 | |
*** marcoemorais has joined #openstack-keystone | 16:38 | |
*** gokrokve has joined #openstack-keystone | 16:38 | |
*** hrybacki has quit IRC | 16:39 | |
ayoung | henrynash, stevemar one thing topol and I discussed over dinner was consolidating all of our notifications in CADF. Is there any reason not to do only CADF for notifications? | 16:42 |
topol | o/ | 16:43 |
henrynash | ayoung: the only thing would be if there are some notifactios that don’t fit CADF…not sure if there are any | 16:43 |
morganfainberg | dstanek, pong | 16:44 |
morganfainberg | ayoung, yes that is intended | 16:44 |
morganfainberg | ayoung, tokens without audit ids need to still be revokeable (upgrade path) | 16:45 |
topol | probably are some that require us to enhance pycadf. but I think that is a point in time statement as opposed to an architectural show stopper | 16:45 |
ayoung | morganfainberg, its hard coded to always be False, though | 16:45 |
morganfainberg | look at line 439 and 435 | 16:45 |
ayoung | Ah...ok, hidden in there. I'll revise my review | 16:46 |
morganfainberg | ayoung, if we could just reject all non-audit tokens it would be easier | 16:46 |
ayoung | morganfainberg, we can | 16:46 |
ayoung | tokens don't live long. Saying that an upgrade like this revokes all tokens is acceptable | 16:47 |
morganfainberg | ayoung, tokens lifespan is configurable | 16:47 |
ayoung | it will cause a hiccup, but it just means that users need to re-authenticate | 16:47 |
ayoung | we've made that decision before, like on migrations on the token table | 16:47 |
morganfainberg | ayoung, i erred on the side of compatibility. but if we make that same decision again thats fine, i just didn't want to assume | 16:48 |
ayoung | morganfainberg, get dolphm 's opinion. I'd hate to have to carry code like that forever to cover the 12 hour time period when the upgrade happens | 16:51 |
morganfainberg | ayoung, yeah, worst case, we can yank that code in K. | 16:52 |
morganfainberg | ayoung, but yeah i'll go with dolphm's opinion on it | 16:52 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Fix Python3 issue with token data helper test https://review.openstack.org/115707 | 16:53 |
morganfainberg | dstanek, ^ | 16:53 |
morganfainberg | dstanek, addressing the python3 issue in the tests | 16:53 |
openstackgerrit | henry-nash proposed a change to openstack/keystone: Implements backend for policy endpoint extension https://review.openstack.org/115362 | 16:54 |
dstanek | morganfainberg: thanks | 16:54 |
dstanek | morganfainberg: after that initial review i think i have only found small nits | 16:55 |
*** mrmoje has quit IRC | 16:55 | |
morganfainberg | dstanek, thats good to hear :) | 16:56 |
morganfainberg | ayoung, i'm willing to pull out the compatibility code, but i think i'd like to do it as a separate commit (even if dolphm says to break it) in the case we need to revert we only revert the compatibility removal | 16:57 |
morganfainberg | ayoung, rather than re-engineering how to make things compatible | 16:57 |
morganfainberg | dstanek, https://review.openstack.org/#/c/113430/12/keystone/models/token_model.py were you saying just put a space in? | 17:00 |
morganfainberg | so <type> (values) | 17:01 |
morganfainberg | vs <type>(values) | 17:01 |
morganfainberg | or were you saying put all of the values of the token in? | 17:01 |
morganfainberg | vs just audit ids | 17:01 |
*** richm has quit IRC | 17:01 | |
*** browne has joined #openstack-keystone | 17:02 | |
dstanek | morganfainberg: i was thinking if would look like: <KeystoneToken (audit_id=?, audit_chain_id=?) at 0xDEADBEEF> | 17:03 |
morganfainberg | dstanek, cool thats how i read it | 17:03 |
morganfainberg | dstanek, should i put "object" in? | 17:03 |
morganfainberg | dstanek, /me doesn't really care, but wanted something more than KeystoneToken object | 17:03 |
dstanek | morganfainberg: i don't think it would benefit us so i say no | 17:03 |
morganfainberg | k | 17:03 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Add __repr__ to KeystoneToken model https://review.openstack.org/113430 | 17:05 |
morganfainberg | done. | 17:06 |
morganfainberg | dstanek, also addressed ayoung's comments (if you saw the convo here as well) | 17:08 |
morganfainberg | dstanek, on https://review.openstack.org/#/c/114864 | 17:08 |
ayoung | morganfainberg, I'm totally fine with that. Yanking the "revoke by expiration" code altogether in Kilo is fine | 17:10 |
morganfainberg | ayoung, ++ | 17:10 |
ayoung | morganfainberg, since you already wrote it | 17:11 |
ayoung | I would have advised against it, up front, but since the price is paid... | 17:11 |
morganfainberg | ayoung, lol it was actually an afterthought because i ran into an issue with using a stale token on a rejoin-stack | 17:11 |
morganfainberg | ayoung, stale = pre-update to audit_ids | 17:12 |
ayoung | morganfainberg, right. I think we've needed audit_ids for a long time. Good to finally have them | 17:12 |
morganfainberg | i think this https://review.openstack.org/#/c/114590/ and https://review.openstack.org/#/c/114857/ need to go in before we merge the code. | 17:13 |
morganfainberg | since it's the identity-api stuff | 17:13 |
*** RicoLin has quit IRC | 17:13 | |
*** richm has joined #openstack-keystone | 17:17 | |
*** marcoemorais has quit IRC | 17:19 | |
*** harlowja_away is now known as harlowja | 17:19 | |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Notification Constant Cleanup and internal notify type https://review.openstack.org/115337 | 17:20 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove assignment_api dependency on token_api https://review.openstack.org/115338 | 17:20 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove oauth controller dependency on token_api https://review.openstack.org/115343 | 17:20 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Mark methods on token_api deprecated https://review.openstack.org/115347 | 17:20 |
*** marcoemorais has joined #openstack-keystone | 17:22 | |
*** marcoemorais has quit IRC | 17:23 | |
*** marcoemorais has joined #openstack-keystone | 17:25 | |
*** afazekas has joined #openstack-keystone | 17:30 | |
openstackgerrit | A change was merged to openstack/keystonemiddleware: Hash for PKIZ https://review.openstack.org/114646 | 17:31 |
*** browne has quit IRC | 17:31 | |
ayoung | morganfainberg, are you using the URL safe Base64 encoding for audit ids? | 17:32 |
*** amirosh has joined #openstack-keystone | 17:32 | |
morganfainberg | ayoung, negative. | 17:32 |
morganfainberg | ayoung, there are no rest apis that include the audit id as part of the uri | 17:32 |
ayoung | morganfainberg, I realize that we are not planning on having them in URLs, but I think you might be overoptimizing here, and the base64edness of it might keep us from using them in an URL at some point | 17:33 |
ayoung | I'd either go with the same ID approach we are using elsewhere, or at least make them url safe | 17:33 |
ayoung | the API spec can just say "url safe strings" and I would not limit the m to 22 chars, even if that is what we are doing. | 17:34 |
ayoung | maybe "url safe strings, no longer than 64 chars" | 17:34 |
*** nkinder has quit IRC | 17:35 | |
morganfainberg | initially i used .hex and was asked if we could make it shorter and not 'hex' limited | 17:35 |
morganfainberg | hex = 32 characters from uuid4 | 17:36 |
*** gyee_ has left #openstack-keystone | 17:36 | |
*** amcrn has joined #openstack-keystone | 17:37 | |
*** zzzeek has quit IRC | 17:37 | |
ayoung | morganfainberg, yeah, I see that the goal is to keep them short. since only Keystone needs to generate them, the Spec does not need to indicate the algorithm | 17:38 |
*** zzzeek has joined #openstack-keystone | 17:38 | |
ayoung | but to do url safe base64 is pretty easy...its a slightly different call, and we already do that for PKIZ tokens | 17:38 |
ayoung | base64 non-url safe is kindof nasty | 17:39 |
morganfainberg | codecs doesn't support it so i'd need to move to using raw base64 lib | 17:39 |
morganfainberg | codecs only support b64 not b64 urlsafe | 17:39 |
*** bambam1 has joined #openstack-keystone | 17:39 | |
morganfainberg | not a big deal. but honestly, we could also just urlencode the b64 id if we want to use it in rest somewhere | 17:40 |
bambam1 | Hello guys I was wondering if there is a way once I have the auth token from the keystone rest api to know my email address or contact information? i'm using the v2 version | 17:41 |
*** browne has joined #openstack-keystone | 17:41 | |
morganfainberg | bambam1, not in the token data itself. | 17:42 |
ayoung | bambam1, PKI or UUID? | 17:42 |
ayoung | morganfainberg, codecs? | 17:43 |
morganfainberg | ayoung, doesn't matter, we don't put 'extra' fields in the token, or at least as far as i know we dont | 17:43 |
bambam1 | UUID | 17:43 |
morganfainberg | ayoung, codecs is the library (built-in) I was using for encoding the uuid | 17:43 |
morganfainberg | ayoung, it's a base-python lib | 17:43 |
ayoung | bambam1, a user cannot figure out what data is in a UUID token. They don;t have permissions to validate it | 17:43 |
ayoung | morganfainberg, http://git.openstack.org/cgit/openstack/python-keystoneclient/tree/keystoneclient/common/cms.py#n201 | 17:44 |
morganfainberg | like i said, i'd just need to move away from using codecs | 17:44 |
bambam1 | I see I was trying to query the user's list with my username /v2.0/users/my_user | 17:45 |
bambam1 | but it seems like I have to be an admin for that | 17:45 |
*** raildo has quit IRC | 17:47 | |
*** nkinder has joined #openstack-keystone | 17:48 | |
*** samuelmz has quit IRC | 17:48 | |
*** htruta has quit IRC | 17:49 | |
ayoung | bambam1, yep | 17:50 |
*** htruta has joined #openstack-keystone | 17:50 | |
*** samuelmz has joined #openstack-keystone | 17:52 | |
openstackgerrit | Steve Martinelli proposed a change to openstack/keystone: Create SAML generation route and controller https://review.openstack.org/114138 | 17:52 |
*** rodrigods has joined #openstack-keystone | 17:53 | |
*** rodrigods has joined #openstack-keystone | 17:53 | |
morganfainberg | ayoung, i'm going to say no way to the arbitrary length of data in the audit_ids field. | 17:53 |
*** rushiagr is now known as rushiagr_away | 17:53 | |
morganfainberg | ayoung, we absolutely should change the spec if we're cramming more ids in there. | 17:53 |
ayoung | morganfainberg, give a max length | 17:53 |
morganfainberg | not just limit in practice. | 17:53 |
morganfainberg | 2 | 17:53 |
morganfainberg | it's already defined as having a max length :) | 17:54 |
ayoung | morganfainberg, why? | 17:54 |
morganfainberg | ayoung, to avoid token size creep and potential dos scenarios | 17:54 |
morganfainberg | ayoung, what use case do you want in there that isn't an unbounded dataset | 17:54 |
morganfainberg | ayoung, and if we move to unscoped -> scoped (not scoped -> scoped) that chain will always be 2 anyway | 17:55 |
ayoung | morganfainberg, why do it as an array, then? | 17:55 |
morganfainberg | ayoung, json doesn't support tuples. | 17:55 |
ayoung | audit_id and parent_audit_id | 17:55 |
ayoung | with parent being optional | 17:55 |
ayoung | microoptimization? | 17:56 |
morganfainberg | ayoung, because the array saves us bytes (yep another one i was asked for when pitching this) | 17:56 |
ayoung | morganfainberg, what about K2K? We could possibly have length 4 there | 17:56 |
ayoung | unscoped orig, scoped orig, unscoped local, scoped local? | 17:56 |
morganfainberg | ayoung, i'd argue that you're not really in the same token chain there as you've authed on the remote end (it happens to be a SAML2 (from a token) -> token auth, but still a first order auth) | 17:58 |
*** dims has quit IRC | 17:59 | |
*** dims has joined #openstack-keystone | 17:59 | |
ayoung | morganfainberg, the fact is, there are easily exceptions to a length 2 array that we can concieve of, and limiting it to length 2 buys us nothing. THe revoaciton events code is going to have to be a for-each loop anyway | 18:01 |
morganfainberg | ayoung, then propose an explicit set of data that will be in there, not "some number" | 18:01 |
morganfainberg | ayoung, not "up to XXX number" | 18:02 |
morganfainberg | what is each element | 18:02 |
ayoung | morganfainberg, the token is issued by keystone. A user cannot issue an arbitrary token. I wouldn't worry about DOS by array length here. If that were the case, it would be the service catalog that was the problem anyway | 18:02 |
*** marcoemorais has quit IRC | 18:03 | |
*** marcoemorais has joined #openstack-keystone | 18:03 | |
ayoung | its not hard to revise the spec if we need to, but it does mean that people that coded to the spec need to revise their code. | 18:03 |
morganfainberg | ayoung, unless you're willing to give me an explicit list of what each element in the list is and it isn't an unbounded data set (think of someone who sits rescoping the token all day to be malicious), i am not budging on this one | 18:03 |
ayoung | morganfainberg, the session token proposal I gave would require arbitrary length | 18:04 |
morganfainberg | ayoung, why | 18:04 |
*** hrybacki has joined #openstack-keystone | 18:04 | |
ayoung | the current horizon code requires arbitrary length | 18:04 |
morganfainberg | no it doesnt | 18:05 |
ayoung | I can use one token to get another token, and do that forever | 18:05 |
morganfainberg | *today* we revoke by expiration time | 18:05 |
morganfainberg | which all the tokens are tied to | 18:05 |
ayoung | morganfainberg, not really, as that code is not in use | 18:05 |
ayoung | horizon does it like this: | 18:05 |
morganfainberg | ayoung, today we revoke by token id | 18:05 |
ayoung | log in, get token (maybe scoped, depending) | 18:05 |
ayoung | use that to get another tpoken scoped to project. revoke first | 18:05 |
ayoung | continue | 18:06 |
morganfainberg | so you revoke by audit id there. | 18:06 |
morganfainberg | the local audit id | 18:06 |
morganfainberg | if you want to revoke the whole chain, you'd still use (what today would be expires at) the audit_chain_id | 18:06 |
ayoung | forget it, that use case will not work anyway | 18:06 |
ayoung | they will need to revoke explicitly the token they are using, while leaving the child token still valid | 18:06 |
ayoung | but today there is no rule unscoped->scoped only | 18:07 |
ayoung | so there is no way to revoke a whole chain | 18:07 |
ayoung | my approach was overkill | 18:07 |
ayoung | but you can never have too much overkill | 18:07 |
morganfainberg | ayoung, there is no rest api to revoke a whole chain. | 18:07 |
morganfainberg | but .. it would be easy to add a query param to do it (which this code now supports) | 18:08 |
ayoung | morganfainberg, and there is no rule that limits the length of a chain | 18:08 |
ayoung | or to say "revoke fromthis point on forward" | 18:08 |
morganfainberg | ayoung, except that is an unbounded data set and is open for abuse that *can* and probably will affect keystone. | 18:08 |
morganfainberg | ayoung, we can't allow unbounded datasets in the tokens (really it is the problem with catalog) | 18:09 |
ayoung | its not a huge deal, I just think you are defining it too narrowly, but it is not something I'd throw over the review for | 18:09 |
morganfainberg | i'm adding the b64url safe stuff in | 18:09 |
morganfainberg | i'll add the change to b64urlsafe as part of the python3 fix i did for dstanek | 18:10 |
*** browne has quit IRC | 18:10 | |
morganfainberg | ayoung, so it doesn't require massive rebase of the whole chain. | 18:10 |
ayoung | if wecome across then need, we'll revise the spec, but I would argue that the spec should read "expected to be no more than length 2" | 18:10 |
morganfainberg | sure, i'll add that verbiage in specifically. just that the max is 2 (actually we have a test for that as well) | 18:10 |
ayoung | but leave it such that code should expect a length >2 in the future and should be written to process that | 18:11 |
ayoung | your call, though. I think you;ve thought this through | 18:11 |
morganfainberg | ayoung, "attribute is a list that contains either one or two elements." -> a list of no more than 2 | 18:12 |
morganfainberg | ? | 18:12 |
ayoung | how about "no more than three" | 18:13 |
ayoung | the two we've thought of and wiggle room for one more | 18:13 |
ayoung | plus with 3 you know that its going to be a foreach | 18:13 |
ayoung | :) | 18:13 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/identity-api: Add information about audit_id in token docs https://review.openstack.org/114590 | 18:13 |
*** PsionTheory has joined #openstack-keystone | 18:14 | |
openstackgerrit | Morgan Fainberg proposed a change to openstack/identity-api: Update revoke-ext https://review.openstack.org/114857 | 18:17 |
dstanek | morganfainberg: any reason to even call out the length of the audit ids in the spec? | 18:19 |
morganfainberg | dstanek, i just went with less than 32 characters so we know they are meant to be small | 18:19 |
morganfainberg | dstanek, i don't disagree with ayoung there. | 18:19 |
morganfainberg | or.. crap should be 32 characters or less. | 18:20 |
morganfainberg | blah | 18:20 |
morganfainberg | or would "short" be sufficient? | 18:20 |
*** harlowja has quit IRC | 18:20 | |
*** harlowja_ has joined #openstack-keystone | 18:20 | |
openstackgerrit | Morgan Fainberg proposed a change to openstack/identity-api: Add information about audit_id in token docs https://review.openstack.org/114590 | 18:21 |
morganfainberg | dstanek, ayoung, ^ there "short urlsafe string" | 18:21 |
ayoung | morganfainberg, we had the issue with userid when we went to sha256 that they had code column lengths to 32 chars. | 18:22 |
openstackgerrit | Morgan Fainberg proposed a change to openstack/identity-api: Update revoke-ext https://review.openstack.org/114857 | 18:22 |
morganfainberg | ayoung, yeah i know :( had to fix them. | 18:23 |
ayoung | I was thinking we want to give an upper end. What did we say in the base spec for identifiers? | 18:23 |
*** rushiagr_away is now known as rushiagr | 18:23 | |
morganfainberg | ayoung, 64 for user_ids (and it's not in the spec afaik) | 18:23 |
ayoung | looking... | 18:23 |
morganfainberg | it's just that our schema was 64 | 18:23 |
dstanek | morganfainberg: love it! | 18:23 |
ayoung | "Each resource contains a canonically unique identifier (ID) defined by the Identity service implementation and is provided as the id attribute; Resource ID's are strings of non-zero length." | 18:23 |
morganfainberg | yeah | 18:23 |
morganfainberg | we should update that to put an upper length. | 18:24 |
morganfainberg | but they all coded to uuid, - should we say 32 characters or less? or is "short" suffcient here | 18:24 |
ayoung | I'm tempted to use 64chars across the board, here as well as the rest of id | 18:24 |
ayoung | Databases are typically OK with anything under 256 | 18:25 |
dstanek | short is good because if you specify something that doesn't need to be specified someone will code to it | 18:25 |
dstanek | then we break then when we need to change | 18:25 |
morganfainberg | dstanek, ++ | 18:25 |
ayoung | here is the trade off | 18:25 |
ayoung | if we do it a s a string, it is open ended, and a database olumn has to be "text" | 18:25 |
ayoung | if we say "length <= 64" then the db column will be 64 chards long, and it will waste space | 18:26 |
ayoung | but...that is a minimal amount of sapce, so I'm not too worried | 18:26 |
morganfainberg | ayoung, varchar is pretty efficient | 18:26 |
ayoung | it won't change what goes across the wire | 18:26 |
ayoung | morganfainberg, yep and varchar(64) is, I think, a good middleground | 18:26 |
ayoung | ids can always be shorter | 18:27 |
morganfainberg | ayoung, anywhere i see "short string" i'd code it as a varchar(255) worst case. | 18:27 |
ayoung | why not formalize that? | 18:27 |
morganfainberg | ayoung, sure, probably should across all of keystone | 18:27 |
ayoung | anyway, your spec looks good | 18:27 |
morganfainberg | ayoung, i'm happy to add in an upper bound here if we want :) | 18:28 |
ayoung | your call. We can do it across all ID in one swoop instead of piecemealing it | 18:28 |
*** ukalifon1 has quit IRC | 18:28 | |
openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Convert to urlsafe base64 audit ids https://review.openstack.org/115707 | 18:29 |
morganfainberg | ayoung, yeah lets do it across the board if we're going to do that | 18:29 |
morganfainberg | dstanek, ^ minor change + urlsafe fix for the py3 patch | 18:30 |
morganfainberg | dstanek, instead of using codecs | 18:30 |
*** miqui has quit IRC | 18:31 | |
ayoung | morganfainberg, I'll +A the spec if no objections? | 18:31 |
dstanek | i lost dolphm's list of reviews from yesterday - anyone have the link handy? | 18:35 |
*** mrmoje has joined #openstack-keystone | 18:36 | |
dstanek | ayoung: no objections from me | 18:39 |
dolphm | dstanek: https://gist.github.com/dolph/651c6a1748f69637abd0 | 18:39 |
dstanek | dolphm: thanks | 18:39 |
*** aix has quit IRC | 18:40 | |
htruta | hello, everyone! I noticed that these calls (http://docs.openstack.org/api/openstack-identity-service/3/content/api-1.html) aren't on keystone client. I was thinking about proposing (and implement) a BP to include these calls on keystone-client | 18:45 |
htruta | are you okay with that? | 18:45 |
htruta | after that, we can also implement the same calls to hierarchical projects | 18:46 |
*** amcrn has quit IRC | 18:48 | |
*** amirosh has quit IRC | 18:50 | |
*** raildo has joined #openstack-keystone | 18:50 | |
*** amirosh has joined #openstack-keystone | 18:50 | |
openstackgerrit | A change was merged to openstack/identity-api: Add information about audit_id in token docs https://review.openstack.org/114590 | 18:51 |
*** mrmoje has quit IRC | 18:52 | |
*** amirosh has quit IRC | 18:54 | |
dolphm | dstanek: i'm doing something dumb send help | 18:55 |
dolphm | dstanek: i've convinced myself that i need a @classproperty or something. i want to be able to call the parent class' property and extend it, basically. | 18:56 |
dolphm | dstanek: this is for pycadf. `@property def schema(self):` worked fine until i wanted to be able to access the schema without instantiating an instance of the object. so, `@property def schema(cls):` ... which suddenly makes no sense | 18:57 |
*** miqui has joined #openstack-keystone | 18:58 | |
dolphm | dstanek: is there are not-callable class equivalent to @property? do i just need to fall back on static callable methods or something? | 19:00 |
*** chandankumar has joined #openstack-keystone | 19:02 | |
openstackgerrit | ayoung proposed a change to openstack/python-keystoneclient: Hash for PKIZ https://review.openstack.org/114654 | 19:03 |
ayoung | bknudson, ^^ incorporates your test changes | 19:03 |
bknudson | ayoung: thanks. I forgot about keystoneclient. | 19:04 |
ayoung | bknudson, most people do... | 19:04 |
*** bknudson has left #openstack-keystone | 19:04 | |
ayoung | https://review.openstack.org/#/c/106838/ | 19:04 |
*** bknudson has joined #openstack-keystone | 19:04 | |
ayoung | that is why Jamie has such a long review queue | 19:05 |
*** chandankumar has quit IRC | 19:09 | |
openstackgerrit | ayoung proposed a change to openstack/python-keystoneclient: Add v3scopedsaml entry to the setup.cfg. https://review.openstack.org/110770 | 19:09 |
morganfainberg | dstanek, there isn't a @classproperty type thing, there is only @classmethod. there are ways to get class properties but they start looking crazy. | 19:09 |
morganfainberg | dolphm, ^ not dstanek | 19:09 |
dolphm | morganfainberg: i'm playing with staticmethod now -- it's not ideal but solves the use case i think | 19:10 |
*** rushiagr is now known as rushiagr_away | 19:10 | |
dolphm | this means i can't define schemas dynamically based on the values in the notification though :P which was probably an illconceived idea on my part and will make topol sad | 19:11 |
dolphm | topol: you're online! | 19:11 |
morganfainberg | dolphm, this is where the descriptor stuff might work better | 19:11 |
topol | dolphm, sorry yes I am | 19:11 |
morganfainberg | topol, classic mistake, responding to someone on IRC :P now you've been roped in! | 19:12 |
topol | K | 19:12 |
topol | I deserve it. I have been evil lately | 19:12 |
dolphm | topol: i'm been meaning to ask you this for a couple days | 19:12 |
dolphm | topol: but the pycadf Credential type | 19:13 |
dstanek | dolphm: catching up | 19:13 |
topol | dolphm you have my cell phone | 19:13 |
dolphm | dstanek: i think morganfainberg got me sorted, but happy to have a miracle | 19:13 |
topol | dolphm yes | 19:13 |
topol | pycadf credential | 19:13 |
dolphm | topol: you asked for additional attributes on Credential if Credential.type is 'saml2', one of which was protocol | 19:13 |
dolphm | topol: won't Credential.type == Credential.protocol in that case, always? | 19:14 |
topol | dolphm wanted to be able to add extra key value pairs | 19:14 |
dolphm | topol: yes, but specific ones | 19:14 |
topol | yes | 19:15 |
topol | the ones you put in the schema validation | 19:15 |
dolphm | topol: so won't Credential.type == Credential.protocol in that case, always? | 19:15 |
dolphm | 'saml2' and 'saml2' ? | 19:15 |
topol | agreed. I think so | 19:15 |
openstackgerrit | henry-nash proposed a change to openstack/keystone: Implements the controller for the endpoint policy extension https://review.openstack.org/115746 | 19:15 |
topol | but there were the other two. | 19:16 |
dolphm | topol: that's a red flag that we're doing something wrong to me :) | 19:16 |
dstanek | dolphm: did you decide to go with a classmethod then? | 19:16 |
dolphm | topol: are there any other values of 'type' that you're aware of? | 19:16 |
topol | how so | 19:16 |
topol | thats a fed identiy mapping question. need stevebot for that | 19:16 |
dolphm | topol: i think the right solution is to have a new Credential type in pycadf, like FederatedCredential or something. that's what i'd like to figure out here | 19:16 |
dolphm | stevemar: ^ | 19:16 |
stevemar | o/ | 19:17 |
dolphm | topol: saml2 is just one type of federated credential... should type='federated'? | 19:17 |
stevemar | catching up... | 19:17 |
dolphm | topol: and protocol='saml2'? | 19:17 |
topol | from the credential point of view it should let you add extra key value pairs. saml2 may be a special case one | 19:17 |
openstackgerrit | henry-nash proposed a change to openstack/keystone: Implements the controller for the endpoint policy extension https://review.openstack.org/115746 | 19:17 |
topol | eventually they would have other protocols | 19:17 |
dolphm | topol: prior to this work, i'm not clear on what values 'type' could have | 19:17 |
topol | let me go look at the code | 19:18 |
stevemar | dolphm, wouldn't 'type' have been password or token? | 19:18 |
dolphm | stevemar: that's what i'd like to know as well | 19:18 |
stevemar | shouldn't we have a type='federated' for federated auth | 19:19 |
stevemar | and then protocol = saml/oidc | 19:19 |
stevemar | as an additional field | 19:19 |
openstackgerrit | henry-nash proposed a change to openstack/keystone: Implements the controller for the endpoint policy extension https://review.openstack.org/115746 | 19:19 |
stevemar | dstanek, i'm going to bug you to review the notifications for role_assignments again | 19:19 |
dstanek | stevemar: link? | 19:19 |
stevemar | dstanek, https://review.openstack.org/#/c/112204/ | 19:20 |
topol | stevemar +++ | 19:20 |
stevemar | dstanek; gordc likes it, and i was hoping to sneak this in so topol could leverage some of refactoring i did | 19:20 |
dolphm | topol: stevemar: in one of the examples, 'type' is a URL: "type": "https://mycloud.com/v2/token" | 19:20 |
stevemar | dolphm, that means sausage to me | 19:21 |
dstanek | i think gerrit punishes you for doing too many reviews with an onslaught of email | 19:21 |
dolphm | stevemar: me too | 19:21 |
stevemar | dstanek, that sounds familiar / appropriate | 19:21 |
henrynash | dstanek: know your busy, but I responded to your comments on the endpoint policy API | 19:21 |
stevemar | i need to review some more, get my karma up and things moving, so other can review my stuff :D | 19:22 |
dstanek | henrynash: thanks. i'll take a look at that next. i'm interested to see in anyone else feels the same | 19:22 |
henrynash | dtsanek: so I’m not opposed to a change…i was just basing it on endpoint_filter and role assignments that are kind of done this way | 19:22 |
dstanek | stevemar: yes! no rebase! i can actually see the diff between 25..current | 19:23 |
topol | dolphm, stevemar, so looking at the code the interesting values are group_ids, identity_provider, and protocol | 19:23 |
dolphm | topol: agree. so what's a useful 'type'? | 19:23 |
topol | and if I dont add them as extra key value pairs we dont capture this info in the audit record | 19:23 |
henrynash | dstanek: I believe i the past we used to to role assignments with a body…but moved to an “all url” solution for sepcifying the components of that association | 19:23 |
topol | dolphm what do you mean by type? | 19:24 |
dolphm | topol: Credentials have an existing 'type' attribute | 19:24 |
dolphm | topol: i think you wanted to set that to 'saml2', which is also the protocol value. i don't think that's useful (in fact, i think it might be wrong) | 19:24 |
topol | dolphm so why isnt that saml2? | 19:24 |
dolphm | topol: but i don't know, because i don't know what 'type' is supposed to represent, beyond the apparently conflicting examples | 19:25 |
dstanek | henrynash: it feels unnatural to me that putting /x/y/a would effectively delete (and conceptually tied to) /x/y/z | 19:25 |
dolphm | topol: because 'saml2' is the federation protocol | 19:25 |
topol | dolphm oh | 19:25 |
dolphm | topol: at least, it is in keystone land | 19:25 |
henrynash | dstanek: delete? | 19:26 |
dolphm | topol: the statement "the type of federated credential is saml2" makes sense to me as well, though. so, i don't know what pycadf expects and how we should be using it | 19:26 |
henrynash | dstanek: oh, you mean the overwriting? | 19:26 |
dstanek | henrynash: yeah, multiple resources are sort of tied together | 19:26 |
henrynash | dstanek: not quite sure I see that…. | 19:27 |
henrynash | dstanek: when you say “tied together”, you mean the act of putting /x/y/z is tying those together? | 19:28 |
dstanek | henrynash: in my mind when modeling a endpoint's policy as a REST resource i see it as a single resource; i can get it to see its value and put/delete to change | 19:29 |
dstanek | henrynash: putting to one URL effectively deletes the other resource | 19:29 |
topol | so dolphm, from the spec: | 19:29 |
topol | Type of credential. (e.g., auth. token, identity token, etc.) | 19:29 |
dolphm | topol: and the two examples i can find, one doesn't have a type at all, and one has a URL | 19:30 |
topol | Note: Profiles of this specification MAY define URIs for their credential types. | 19:30 |
topol | my guess is the URL one is actually a URI that is more formally defining the type. but thats optional | 19:31 |
topol | and accosing to the spec type is not required | 19:31 |
topol | so both naswers are ok according to the spec, dolphm | 19:31 |
topol | the key is for it to be easy to set that attribute as a string | 19:32 |
henrynash | dstanek: overrites any previous association…right… | 19:32 |
topol | dolphm, make sense? | 19:32 |
dolphm | topol: so a URL is a string, so that's all we need to validate it as... if it exists. | 19:32 |
topol | yes, just that it is a string | 19:33 |
dolphm | topol: so wtf is the intended difference between "auth", "token", and "identity token" ?? | 19:33 |
topol | dolphm he was just coming up with some potential examples to illustrate there are lots of types of credentials | 19:34 |
topol | I believe he was trying to match keystone like things but if it confused you its porbably a poor lis of examples | 19:34 |
dstanek | henrynash: i'm fine if the general opinion is that we are modeling after an existing API; i wanted to bring it up now for discussion before we have to support it forever | 19:34 |
topol | dolphm, the key thing is that should just be a String and ideally we end up with a clean list of defined URIs to show what credentials are being supported | 19:35 |
*** amcrn has joined #openstack-keystone | 19:36 | |
topol | dolphm perhaps his doc should say something like: A named credential can be a user name/password, a public key-private key pair, or an X509v3 certificate. | 19:37 |
topol | and maybe some of those dont apply to keystone but you get the gist of it | 19:37 |
henrynash | dstanek:…and I fine thingto bring up sire | 19:38 |
dolphm | topol: i don't understand how you reconsile 'just be a string' and 'defined URIs' - either you allow any sort of garbage, you allow any URL, or you allow a specific set of predefined URI's..? | 19:38 |
henrynash | (and a fine thing…) | 19:38 |
topol | for keystone, I think we just used token | 19:38 |
richm | dstanek: ping - re: https://review.openstack.org/74897 - how would I put test_deleteTree in its own test class? | 19:39 |
topol | as the type | 19:39 |
dolphm | topol: so, does type='federated' or something make sense? | 19:39 |
henrynash | dolphm: when you have a moment, could you perhaps have a look at the discussion comments regarding the API for endpoint policy: https://review.openstack.org/#/c/112292/ | 19:39 |
dolphm | henrynash: i'll try; i'm trying to solve the pycadf use case first so topol's bp makes fpf | 19:40 |
henrynash | dolphm: ok, np…yep, get that done first... | 19:40 |
dolphm | henrynash: undefined API is probably my second highest priority though :) | 19:40 |
topol | dolphm, I think federated makes sense | 19:41 |
topol | it really captures the typo of token we are dealing with here | 19:41 |
topol | we need to get Matt Rutkowski to verify that | 19:42 |
topol | The spec says it should be xs:anyURI | 19:43 |
*** gyee_ has joined #openstack-keystone | 19:44 | |
*** dims has quit IRC | 19:45 | |
dstanek | stevemar: dumb question on https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py - is there some metadata somewhere that says how to interpret target and actor? like actor is a user vs. group | 19:45 |
*** dims has joined #openstack-keystone | 19:45 | |
topol | dolphm, so as I interpret the spec the value should be a URI. Any URI at all | 19:46 |
stevemar | dstanek, hmm... no i guess not | 19:46 |
stevemar | it would just be a uuid | 19:46 |
stevemar | so the person reading it wouldn't know if it's a user or group :( | 19:47 |
topol | and we would come up with a name like http://http://openstack.org/v3/federated-token | 19:47 |
topol | dolphm does that make sense | 19:47 |
topol | ? | 19:47 |
topol | dolphm oops , I meant http://openstack.org/v3/federated-token | 19:47 |
dstanek | stevemar: i don't know much about the CADF stuff, but that sounds like it could be an issue | 19:49 |
stevemar | dstanek, yeah, hmmm | 19:49 |
topol | dstanek, stevemar we should get matt rutkowski for these questions | 19:50 |
stevemar | dstanek, add actor_type? rename actor to user (blah) | 19:50 |
raildo | henrynash: I need to create a blueprint for changes in keystone-client or can I just send the patch referencing the hierarchical projects blueprint? | 19:50 |
dstanek | richm: i'll take a look at how to make that test more of a unit test in a little bit and get back to you | 19:50 |
*** dims has quit IRC | 19:51 | |
henrynash | raildo: so I don’t think you need another spec…but you should add a blueprint aimed at tehcleint | 19:51 |
*** dims has joined #openstack-keystone | 19:51 | |
dstanek | topol: how do we do that? | 19:51 |
dstanek | topol: can you invite him in here? | 19:51 |
raildo | henrynash: ok, thanks | 19:52 |
topol | dstanek, yes, Im getting him to join now. he was rebooting his machine | 19:53 |
*** zzzeek has quit IRC | 19:53 | |
*** mrutkows has joined #openstack-keystone | 19:54 | |
mrutkows | hello | 19:54 |
topol | dstanek, dolphm, FIRE AWAY!!! | 19:54 |
mrutkows | Hey all, here to clarify any CADF format issues | 19:54 |
openstackgerrit | henry-nash proposed a change to openstack/keystone: Add delete notifications for policy, region and service. https://review.openstack.org/115763 | 19:55 |
topol | mrutkows, so what value should credential type be? The spec says xs:anyuri | 19:56 |
mrutkows | credential type was intended to identify the "type" of credential that was being described in the structure. This could be a URI if it references a well known format or could be a string that identifies it locally | 19:56 |
topol | dolphm: | 19:57 |
mrutkows | its for when (or if) it is federated that others that receive the data know what type of credential was used | 19:57 |
topol | dstanek what is your questionm | 19:57 |
dstanek | topol,mrutkows, stevemar: my question was more operational than format based - we are storing a UUID as the actor. but that could either be a user_id or group_id and right now where is no way to tell | 19:58 |
dstanek | feels like it will eventually cause an auditor pain at some point | 19:58 |
*** harlowja_ is now known as harlowja_away | 19:58 | |
morganfainberg | dstanek, topol, stevemar, mrutkows, and keep in mind that may not be a uuid it might be a sha256().hex [just make sure you're not assuming 32 characters) | 19:59 |
topol | mrutkows, so for validating the type value dolph dstanek, need more context. which part of the audit structure? | 19:59 |
*** zzzeek has joined #openstack-keystone | 19:59 | |
topol | dstanek, need more context | 19:59 |
topol | show the example CADF record | 19:59 |
topol | or field | 19:59 |
dolphm | topol: dstanek: mrutkows: of course, i went AFK earlier, but catching up now... | 20:00 |
mrutkows | if we have multiple UUIDs each with different identity context, we can add them where needed | 20:00 |
dstanek | topol: all i know right now is line 250 of https://review.openstack.org/#/c/112204/28/keystone/notifications.py | 20:00 |
dolphm | mrutkows: so if it just boils down to an aribtrary string, what are the well known values of "type", if any? | 20:01 |
dstanek | topol: i noticed it because of line 507 on https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py | 20:02 |
mrutkows | dstanek: looking at notifications.py, it appears that we grabbed whatever was placed in the environment variable and our developer assumed it was a user (which was true for when it was only used for APIs) | 20:03 |
dstanek | topol, mrutkows: so i was not sure if there was a way to mark the actor as user or group | 20:04 |
dstanek | mrutkows: yeah, this came up in a review of some code we are trying to get in | 20:04 |
henrynash | stevemar: docs…ooh..where do we document which events get notifcations | 20:05 |
henrynash | ? | 20:05 |
topol | https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py | 20:05 |
*** dims has quit IRC | 20:05 | |
mrutkows | dstanek: if we have the ability to distinguish userid from groupid, etc. then we can clearly put them in a correct keyname | 20:05 |
*** dims has joined #openstack-keystone | 20:06 | |
topol | dstanek, so it looks like stevemar wrote this but the values look incorrect | 20:07 |
*** dims_ has joined #openstack-keystone | 20:07 | |
*** dims has quit IRC | 20:07 | |
topol | folks, let's try and close out dolphm's question first | 20:07 |
topol | dolphm you still there? | 20:08 |
*** marcoemorais has quit IRC | 20:08 | |
dstanek | mrutkows: are the fields on Resource arbitrary? or does actor mean something special? | 20:08 |
*** stevemar has quit IRC | 20:08 | |
*** marcoemorais has joined #openstack-keystone | 20:08 | |
mrutkows | actor is a non-normative field someone added | 20:08 |
mrutkows | not part of the spec. | 20:08 |
mrutkows | Steve seems to have added this as some attachment | 20:09 |
henrynash | stevemar: sorry, found it | 20:09 |
mrutkows | dstanek: The spec allows you to add attachments or additional key-values, but they are not normative. What we have done in the past is decided on a very few (3 or less) fields added key-values for Openstack that we have documented in an published openstack profile | 20:10 |
mrutkows | dstanak: the idea is that everyone agrees what fields are needed to audit and add them | 20:11 |
topol | dstanek | 20:11 |
*** marcoemorais has quit IRC | 20:11 | |
mrutkows | dstanak: then we can publish them back to the OpenStack profile so anyone receiving CADF events know what they are | 20:12 |
topol | dstanek | 20:12 |
*** marcoemorais has joined #openstack-keystone | 20:12 | |
mrutkows | sorry dstanek | 20:12 |
*** marcoemorais has quit IRC | 20:12 | |
dolphm | topol: yep | 20:12 |
*** marcoemorais has joined #openstack-keystone | 20:12 | |
openstackgerrit | Thiago Paiva Brito proposed a change to openstack/python-keystoneclient: Implementing hierarchical calls on keystoneclient v3 (python only) https://review.openstack.org/115770 | 20:12 |
dstanek | mrutkows: so right now we are using target for either the domain or project that is being acted on - should i change that to just be project or domain? | 20:12 |
dstanek | mrutkows: same goes for actor that could represent either a user or a group | 20:13 |
mrutkows | dstanek: keystone devs should decide what fields are important to add, add them (validate them) and we can add them to the CADF spec./OpenStack profile | 20:13 |
dstanek | mrutkows: ok, and it's not a problem to have an xor situation where a record with have either domain or project, but not both? | 20:14 |
*** dims_ has quit IRC | 20:14 | |
*** dims has joined #openstack-keystone | 20:15 | |
mrutkows | dstanek: the target should just represent the resource an action (API) is targeting, and whatever fields are needed to describe it (keys) beyond the resource ID and typeURI string that are CADF nromative | 20:15 |
*** radez is now known as radez_g0n3 | 20:15 | |
mrutkows | dstanek: if there is additional context, such as group, project etc then these can be added as new key-value pairs | 20:15 |
dstanek | mrutkows: in your opinion what is better having a (domain or project) field or having a (target and target_type) when target_type would be either 'domain' or 'project'? | 20:17 |
mrutkows | dstanek: the target should be what you would identify as the primary resource (logical) that is being acted upon, and the rest add/treat as additional context (add them as new keys) | 20:18 |
mrutkows | dstanek: and let me know so I can publish into the OpenStack spec | 20:18 |
mrutkows | dstanek: hehe | 20:18 |
*** dims has quit IRC | 20:19 | |
mrutkows | dstanek: to better answer (project or domain), I would need to understand the activity/action/api we were talking about | 20:19 |
topol | mrutkows, dstanek, did you need a key value pair to say wheter it is a group or user uuid? isnt this how the whole conversation started? | 20:20 |
mrutkows | dstanek: typically we want to name the target's typeURI (if its data held be a service) as relative to that service | 20:20 |
dstanek | topol, mrutkows: yes, that's exactly it - my assumption is that a uuid isn't good enough and there needs to be something that says what the uuid refers to | 20:21 |
dolphm | mrutkows: topol: alright, i put this up as (i think) the smallest possible patch to solve topol's use case for auditing federation: https://review.openstack.org/#/c/115771/1/pycadf/credential.py | 20:21 |
dstanek | mrutkows: so the API in question is creating and deleting grants - https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py | 20:21 |
dolphm | dstanek: i've only been skimming the rest of the above... but where is an entire group being supplied as the actor? | 20:22 |
dolphm | that sort of seems to defeat the purpose of 'actor' :-/ | 20:22 |
dstanek | dolphm: lines 350 and 351 of https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py are what i was concern with | 20:23 |
dolphm | oh for assignment notifications, gotcha | 20:23 |
dstanek | dolphm: wrong link...correct one is https://review.openstack.org/#/c/112204/28/keystone/notifications.py | 20:23 |
dolphm | actor should be the API user, not the subject of the assignment | 20:23 |
dstanek | so target is what is being granted (either the project or domain) and actor is who it's granted to (user or group) | 20:25 |
topol | dstanek, we need to code review stevemars code | 20:25 |
topol | he is new to cadf and I dont think its the way it needs to be | 20:26 |
*** dims has joined #openstack-keystone | 20:26 | |
mrutkows | dstanek: well, being put "on the spot", without a lot of context... the target's primary type should include the words "user" or "group" if a role is being granted/added, if we have metadata about the user/group we can add it to Target, but if we have metadata about the role itself it can be added as an attachment, but more likely on the event itself. It looks like we need a code review... | 20:27 |
mrutkows | ...with Steve | 20:27 |
dolphm | dstanek: and where is the API user going? | 20:27 |
dstanek | dolphm: i think that's the same as it was in the host object | 20:28 |
dolphm | dstanek: but the host is just an address? | 20:28 |
dolphm | dstanek: multiple users could be coming from the same host | 20:28 |
dstanek | dolphm: oh, sorry. the resource name http://git.openstack.org/cgit/openstack/keystone/tree/keystone/notifications.py#n254 | 20:29 |
dstanek | mrutkows: here is what i was thinking as potential options http://paste.openstack.org/show/97912/ | 20:29 |
ayoung | richm, have you worked through the steps necessary to get cinder to authenticate tokens with Keystone over https? | 20:30 |
*** amerine has joined #openstack-keystone | 20:32 | |
dolphm | dstanek: is there any justification against the specialized key approach? | 20:32 |
dstanek | dolphm: not that i know of - that's probably the one i would opt for | 20:33 |
dolphm | 1 | 20:33 |
dolphm | dstanek: then ++! | 20:33 |
*** jamielennox is now known as jamielennox|away | 20:33 | |
mrutkows | dstanek: anything that helps describe the "user" (or source of the action) and their context should indeed be added to the initiator field; however, we also have a target field... it just looks odd that fields are named target are being placed in the initiator structure | 20:34 |
mrutkows | dstanek: also the initiator already has a typeURI which describes the initiator resource, if an additional "type" is needed that is meaningful to openstack then its fine to have a different field | 20:35 |
*** PsionTheory has quit IRC | 20:37 | |
mrutkows | dstanek: be aware i am not a keystone dev. so i am now sure what all the semantics/values you have are, as you educate me more on what these fields are for/or hold the better answer i can provide | 20:37 |
mrutkows | dstanek: stevemar is offline or else i would grab a phone and ask him about all these fields | 20:38 |
dstanek | mrutkows: he had to take off so i told him i would make any change we decide on | 20:38 |
topol | dolphm, your code and test case look spot on to me | 20:38 |
dolphm | topol: note that it's not exactly what you asked for | 20:40 |
dolphm | topol: but i think it solves the use case | 20:40 |
dstanek | mrutkows: i'm not sure what the typeURI actually means | 20:40 |
*** henrynash has quit IRC | 20:40 | |
dolphm | dstanek: i think that's "service type" | 20:40 |
dolphm | dstanek: it's not actually a URI | 20:40 |
topol | dolphm, how is it different??? | 20:41 |
dolphm | topol: i'm not letting you set Credential.type! | 20:42 |
dstanek | mrutkows: basically someone is granting a particular role (say 'manager') to a user or group for either a project or a domain - he is using then name in the initiator for saying who made the API call, target for project or domain and actor for the user or group | 20:42 |
richm | ayoung: no | 20:42 |
dstanek | mrutkows: where is the current openstack spec so i can take a quick look at it? | 20:42 |
dolphm | topol: mrutkows: i left an inline comment on https://review.openstack.org/#/c/115771/1/pycadf/credential.py since that's probably a better place to have this conversation | 20:43 |
topol | dolphm, OK, I see that now. | 20:44 |
topol | dolphm so talking to matt he feels type should describe the format of the credential | 20:46 |
*** gyee_ has quit IRC | 20:46 | |
topol | dolphm most accurately saml2 more accurately describes the credential | 20:47 |
dolphm | topol: describe the pycadf credential object, or the actual credential being audited | 20:47 |
dolphm | ? | 20:47 |
mrutkows | dolphm: the credential.type should describe the type of credential being audited (its specification if there is one or format) so people if the token is attached (optionally) it can be parsed | 20:49 |
*** gyee_ has joined #openstack-keystone | 20:50 | |
*** mrmoje has joined #openstack-keystone | 20:50 | |
mrutkows | dolphm: or even if the token is not attached, where the fields came from | 20:50 |
mrutkows | dolphm: is the token (or data described in the credential object) from SAML, from Oauth, from etc. | 20:50 |
dolphm | topol: what are you putting into the "token" attribute? | 20:51 |
dolphm | topol: the entire saml doc? | 20:51 |
topol | dolphm, if one day we add saml3 as a type I cant change this. Im stuck | 20:51 |
topol | adolphm and when I say saml2 and saml3 I really mean more globally context names | 20:51 |
mrutkows | dolphm: either use the OASIS assigned URI for SAML 2 or create an openstack value for SAML 2 that we can publish | 20:52 |
dolphm | topol: can you answer my question above? | 20:53 |
*** marcoemorais has quit IRC | 20:53 | |
topol | dolphn, letting mrutkows try and answer it | 20:53 |
*** marcoemorais has joined #openstack-keystone | 20:53 | |
topol | mrutkow type should reflect this is saml2 "stuff" | 20:53 |
dolphm | topol: i'm asking you the question because i'm asking about your keystone patch | 20:53 |
topol | err dolphm type should reflect this is saml2 stuff not just that its federated | 20:53 |
topol | dolphm trying to answer | 20:54 |
mrutkows | dolphm: SAML defines a "format" value for each spec. | 20:55 |
topol | dolphm thats why type needs to convey saml2 as opposed to just federated. Now where I am lying to you is that the name needs to have a more official global name than saml2 | 20:55 |
mrutkows | dolphm: the core spec is http://docs.oasis-open.org/security/saml/v2.0 | 20:56 |
mrutkows | dolphm: i would use this to identify the SAML 2 standard generically which should be sufficient, we wont get into protocol varianbces | 20:58 |
mrutkows | variances | 20:58 |
topol | dolphm so the value for type should be http://docs.oasis-open.org/security/saml/v2.0 | 20:58 |
topol | dolphm when someone sees that they know its federated identity using saml v2 assertions | 20:59 |
*** henrynash has joined #openstack-keystone | 20:59 | |
dolphm | topol: mrutkows: so now that we've come full circle without answering my original question: what should the value of protocol be, if type is "http://docs.oasis-open.org/security/saml/v2.0" ? | 21:00 |
topol | and dolphm I should be able to set that, dolphm why do you want to hard code it and lock me in? | 21:00 |
dolphm | topol: that's the question i'm trying to answer. we have two poorly defined attributes in the proposed notification at this point | 21:01 |
mrutkows | dolphm: protocol was an extra field someone added, steve added this perhaps? | 21:01 |
*** amcrn has quit IRC | 21:01 | |
dolphm | holy fucking shit read the damned review | 21:02 |
topol | dolphm so are you trying to convice me protocol should really be http://docs.oasis-open.org/security/saml/v2.0 | 21:02 |
mrutkows | dolphm: if we want to just describe the whole thing generically as SAML 2 then use the strin i provided, if additionally you want to describe the assertion format you use: urn:oasis:names:tc:SAML:2.0:assertion | 21:03 |
dolphm | i added it in the patchset above at topol's request to solve topol's use case and am here questioning it's value when compared to another poorly defined attribute | 21:03 |
topol | dolphm , I have read it | 21:03 |
topol | dolphm yes, I know that is where you were taking me | 21:03 |
mrutkows | dolphm: if for some reason you want to say or capture (optional) that the saml 2 protocol binding was used (overkill perhaps) | 21:03 |
*** vhoward has left #openstack-keystone | 21:04 | |
mrutkows | dolphm: you could add a key urn:oasis:names:tc:SAML:2.0:protocol... not sure what steve intended | 21:04 |
*** jasondotstar has joined #openstack-keystone | 21:05 | |
topol | dolphm, I need to to see why stevemar used saml2 as his protocol value. and yes either thats just a poorly chosen attribute that should be changed | 21:05 |
*** raildo has left #openstack-keystone | 21:05 | |
dolphm | topol: because that's what we defined to be the conventional protocol ID for saml2 use cases with shibboleth | 21:05 |
topol | or its just an openstack keystone code convention thats just harmless | 21:05 |
dolphm | it's the use case we documented over and over in https://github.com/openstack/identity-api/blob/master/v3/src/markdown/identity-api-v3-os-federation-ext.md | 21:06 |
mrutkows | dolphm: we can simply use SAML2 and document that is the value openstack/keystone uses for the SAML 2.0 spec. | 21:07 |
topol | dolphm I agree, but I think is we have two versions of the same thing. one was an internal one that had a simple name and works fine and the other gets an official name that works for external federated auditing | 21:08 |
topol | mrutkows is wrong | 21:08 |
topol | dolphm when you send the auditing record we want to be more precise cause its goes out the world in an audit record | 21:09 |
*** gokrokve has quit IRC | 21:09 | |
dolphm | topol: so you *don't* want to include the protocol ID in the pycadf credential notification then? so "protocol" goes away here https://review.openstack.org/#/c/115771/1/pycadf/credential.py and type becomes user-defined in the case of FederatedCredential? | 21:10 |
*** gokrokve has joined #openstack-keystone | 21:10 | |
dolphm | topol: my impression from this conversation, all the documentation, and all the examples, is that we don't care what sort of garbage we emit in auditing notifications | 21:10 |
topol | dolphm, it helpos to have it because when you look at the mapping stuff that is what is used | 21:10 |
dolphm | tests too | 21:10 |
mrutkows | dolphm: i care | 21:11 |
topol | dolphm you have a label for the mapping stuff and then you have a spec that desires a more formal name | 21:11 |
dolphm | mrutkows: there's a lot of unsubstantiated "should"s flying around | 21:12 |
topol | dolphm so Im frustrating you because we have something that has two different meanings although simillar | 21:12 |
topol | dolphm if someone was looking at the audit record it would help them to see saml2 cause that is what is used in the mapping code | 21:12 |
dolphm | topol: i'm frustrated because i'm asking the same basic questions over and over, and i have yet to get a straight answer | 21:13 |
mrutkows | dolphm: the value that goes out to the world SHOULD be the oasis name, this has meaning in a federated context, if we have a local name (openstack name, like protocol) that should be added for local context (but it would not have meaning outside openstack) | 21:13 |
dolphm | topol: most recently.... topol: so you *don't* want to include the protocol ID in the pycadf credential notification then? so "protocol" goes away here https://review.openstack.org/#/c/115771/1/pycadf/credential.py and type becomes user-defined in the case of FederatedCredential? | 21:13 |
topol | dolphm I misundersood your question because to the two different contexts | 21:13 |
topol | dolphm can I call you? | 21:13 |
topol | will be much easier | 21:13 |
dolphm | topol: i'd rather keep this in public / on record | 21:14 |
dolphm | topol: i put up the code review because that's obviously the best place to iterate https://review.openstack.org/ | 21:14 |
dolphm | whoops: https://review.openstack.org/#/c/115771/ | 21:14 |
topol | Ok. so on the record. I did a poor job of distinguishing between something from our mapping syntax and the desire for CADF to have a formal label for essentially the same thing | 21:15 |
topol | when you see saml2 in keystone it means use the saml2 mapping stuff. agre or disagree? | 21:15 |
*** marcoemorais has quit IRC | 21:16 | |
*** marcoemorais has joined #openstack-keystone | 21:16 | |
*** henrynash has quit IRC | 21:17 | |
*** harlowja_away is now known as harlowja_ | 21:17 | |
dolphm | topol: that's about half the definition i would use for icehouse, i suppose | 21:17 |
topol | dolphm, when I see saml2 its like the following from the markdown: | 21:17 |
topol | protocol": { | 21:17 |
topol | "id": "saml2", | 21:17 |
topol | "mapping_id": "xyz234", | 21:17 |
topol | "links": { | 21:17 |
topol | "identity_provider": "http://identity:35357/v3/OS-FEDERATION/identity_providers/ACME", | 21:17 |
topol | "self": "http://identity:35357/v3/OS-FEDERATION/identity_providers/ACME/protocols/saml2" | 21:18 |
topol | } | 21:18 |
dolphm | topol: the other half is that it's part of a contract between shib's configuration and keystone, and there's also the authentication method that's going away in juno | 21:18 |
topol | dolphm agree or disagree? | 21:18 |
dolphm | topol: agree that's half of what a protocol "is" to keystone | 21:18 |
topol | dolphm whats the other half? | 21:18 |
dolphm | topol: mrutkows: p.s. i put up a second patchset here, assuming a straightforward "yes" to the question i tried ask above: https://review.openstack.org/#/c/115771/ | 21:19 |
topol | and dolphm would you agree that saml2 was a name someone just picked cause the knew they were doing saml2? | 21:19 |
dolphm | topol: topol: the other half is that it's part of a contract between shib's configuration and keystone, and there's also the authentication method that's going away in juno | 21:19 |
dolphm | topol: yes | 21:19 |
topol | and dolphm and would you agree that in shib they add saml2 in the config as well? | 21:20 |
dolphm | topol: it was the first protocol we approach with federation, so use chose that as a conventional URL-safe identifier and authentication method identifier, both of which are deployer-configurable | 21:20 |
dolphm | topol: yes | 21:20 |
topol | so dolphm, then we really dont know that its saml2 being used. its just the contract between the mapping and shib in this case is using saml2 | 21:21 |
topol | dolphm maybe in shib we explicitly say hey its saml2 | 21:22 |
mrutkows | dolphm: the patch for the IDP, user and groups does belong as you have coded it into the credential | 21:22 |
mrutkows | dolphm: thanks | 21:22 |
dolphm | mrutkows: that qualifies as a straightforward answer - THANK YOU | 21:23 |
dolphm | topol: so we're dropping the defined-in-keystone federated protocol-ID from the audit notification, but keeping the defined-in-keystone federated identity-provider-ID, correct? | 21:24 |
topol | so dolphm where I struggle is I was hoping since we all knew it was saml2 that we could add the official type to be the http://docs.oasis-open.org/security/saml/v2.0 | 21:24 |
topol | but dolphm, I CHEATED | 21:24 |
dolphm | topol: like so? https://review.openstack.org/#/c/115771/2/pycadf/tests/test_cadf_spec.py | 21:24 |
topol | dolphm how can I know that saml2 really meant http://docs.oasis-open.org/security/saml/v2.0 | 21:25 |
topol | stay with me dolphm | 21:25 |
topol | dolphm, I cant | 21:25 |
dolphm | topol: in the context of keystone? | 21:25 |
topol | dolphm, that in kesytone when we see saml2 it means http://docs.oasis-open.org/security/saml/v2.0 | 21:26 |
topol | dolphm how can I know that??? | 21:26 |
dolphm | topol: correct | 21:27 |
dolphm | topol: it's a federated audit notification, not a saml2 audit notification | 21:27 |
topol | dolphm I cant . so really all i can do is say the protocol is saml2 | 21:27 |
topol | all I know is the id the user picked | 21:27 |
dolphm | topol: so, shall i let you and mrutkows work out how we rectify the impossible with the wrong? | 21:28 |
topol | dolphm, one more step and Im done | 21:28 |
dolphm | topol: oh, i assumed "impossible problem" must be the the last step :) | 21:28 |
topol | so it should be simply saml2 but the spec would like to prefix it and make it http://openstack.org/identity_provider/saml2 | 21:29 |
topol | because the spec wants and any:URI | 21:29 |
topol | cause when federated its now clear this value came from openstack keystone federation identity provider | 21:30 |
topol | that way I satisfy both masters | 21:30 |
topol | saml2 is accurate cause that was defined in keystone | 21:30 |
dolphm | topol: you might be able to get something useful out of the environment variables from shibboleth (like AuthnContextClassRef) | 21:31 |
topol | and http://openstack.org/identity_provider/saml2 is the CADF spec compliant version of that | 21:31 |
dolphm | topol: https://wiki.shibboleth.net/confluence/display/SHIB2/NativeSPAttributeAccess#NativeSPAttributeAccess-EnvironmentVariables | 21:31 |
*** zzzeek has quit IRC | 21:31 | |
dolphm | topol: where did http://openstack.org/identity_provider/saml2 come from? | 21:32 |
openstackgerrit | David Stanek proposed a change to openstack/keystone: Add CADF notifications for role assignment create and delete https://review.openstack.org/112204 | 21:32 |
topol | dolphm , perhaps, but I would think http://openstack.org/identity_provider/saml2 enables someone audting to go check and determine which user chosen idp was sued | 21:32 |
dolphm | topol: but that's what the identity_provider attribute of the audit notification provides | 21:33 |
dolphm | topol: unless you, again, want to go back to shibboleth and pull Shib-Identity-Provider for that | 21:33 |
topol | so in the cadf record the normative one (type) is the full name and the extra one could just be the local name (ie just saml2) | 21:34 |
dolphm | topol: and, again, drop the keystone defined bits on the floor | 21:34 |
*** dims has quit IRC | 21:34 | |
topol | just to have all bases covered | 21:34 |
openstackgerrit | A change was merged to openstack/keystone-specs: Add deprecation tasks to auth-specific-data https://review.openstack.org/115424 | 21:34 |
topol | which bits did I drop? | 21:34 |
dolphm | topol: the "extra" one? | 21:34 |
*** dims has joined #openstack-keystone | 21:34 | |
topol | extra is the provider that youjust added | 21:34 |
topol | dolphm you feel its stupid to have both? | 21:35 |
dolphm | topol: well, this is reiterating my notion of shoveling a bunch of crap onto the message bus and calling it auditing | 21:35 |
topol | dolphm the auditng allows extra stuff to be added. most of what is in the record is normative | 21:36 |
*** henrynash has joined #openstack-keystone | 21:36 | |
morganfainberg | dolphm, when you're done chatting w/ topol have an audit_id question for you | 21:36 |
topol | dolphm if you feel strongly we can drop provider and just keep the long name in type | 21:36 |
*** zzzeek has joined #openstack-keystone | 21:36 | |
topol | dolphm I am happy to drop adding provider as an extra and just use the concatenated one I suggested and I would argue that looks more like audting | 21:37 |
dolphm | topol: a couple weeks ago, i might have agreed. after attempting to rewrite pycadf, i'm under the (perhaps false) impression that the advantage of pycadf is that it's intended to be strict about what auditors can expect from audit notifications. but we don't seem to be delivering on that intention, anywhere | 21:37 |
dolphm | morganfainberg: ack | 21:38 |
mrutkows | dolphm: \o/ YES YOU GET THE INTENT | 21:38 |
morganfainberg | mrutkows, dolphm, we're not delivering on that? that was my understanding of pycadg | 21:38 |
morganfainberg | pycadf | 21:38 |
dolphm | mrutkows: \o/ YAY | 21:38 |
morganfainberg | or is it we're just bad at consuming pycadf | 21:38 |
dstanek | topol, mrutkows: my best guess as to what we should report on for grants - https://review.openstack.org/#/c/112204/28..29/keystone/notifications.py | 21:38 |
topol | dolphm, I promise to never to be kind and add extra info out of kindness again | 21:38 |
*** dims has quit IRC | 21:39 | |
dolphm | morganfainberg: not when the answer to the question of "if we're going to be strict, what value belongs here?" is "let's put all the metadata and cross our fingers that's doubly helpful" | 21:39 |
topol | so dolphm some extras are valuable because of the context | 21:39 |
topol | dolphm so should we not add the groupids??? | 21:40 |
mrutkows | dolphm: clearly add things that are of value to auditors, especially security auditors, like groups and things that establish identity (or access) | 21:40 |
mrutkows | dolphm: but no more | 21:40 |
dstanek | does the spec for this call our which extra metadata must be included under which contexts? | 21:41 |
dstanek | s/our/out/ | 21:41 |
dolphm | topol: i'm not referring to group_ids specifically here | 21:41 |
topol | dolphm, dstanek, there are only a few spots where the cadf allows some extensions. credential was one of them | 21:42 |
dolphm | mrutkows: do we have a strict list of questions that pycadf is intended to answer somewhere? i could probably produce one if not. it'd be a valuable litmus test for "should this value appear in an audit notification?" | 21:42 |
topol | I did someting bad and added one extra one that would have been already covered had I thought about it more clearly. I sincerely apologize for that oversight | 21:43 |
nkinder | morganfainberg: you around? | 21:43 |
morganfainberg | nkinder, sure am | 21:43 |
nkinder | morganfainberg: The gate job is failing for merging one of my patches, and I'm not sure what to file the bug against | 21:44 |
nkinder | morganfainberg: http://logs.openstack.org/08/104408/5/gate/gate-tempest-dsvm-full/f3081d0/console.html | 21:44 |
mrutkows | dstanek: best practice states which fields are normative and required, all other fields can be ignored (or stripped even) since they are considered local context and have no federated value, | 21:44 |
nkinder | morganfainberg: a bunch of the volume tests fail with timeouts | 21:44 |
nkinder | morganfainberg: cinder has some errors too - http://logs.openstack.org/08/104408/5/gate/gate-tempest-dsvm-full/f3081d0/logs/screen-c-vol.txt.gz?level=ERROR | 21:44 |
mrutkows | dstanek: however, i am relying on the great minds in keystone to decide what other fields have value for openstack | 21:44 |
mrutkows | dstanek: and we can publish these fields in the profile | 21:45 |
morganfainberg | nkinder, thats a new one | 21:45 |
morganfainberg | i've not seen that one before | 21:45 |
nkinder | morganfainberg: I didn't see any recheck bugs that seem to match, so I was looking to file a new one | 21:45 |
nkinder | is cinder the right place to start, or should I file this against something else? | 21:45 |
morganfainberg | nkinder, this looks like cinder related. | 21:46 |
nkinder | morganfainberg: cinder it is then | 21:46 |
morganfainberg | nkinder, also recheck <bug####> isn't the norm anymore, file a bug, file a review to add to elasticrecheck, just use "recheck" iirc | 21:46 |
morganfainberg | nkinder, at least thats what i kindof gleaned from the -infra chatter | 21:47 |
dolphm | mrutkows: i figure i'd capture that https://review.openstack.org/#/c/115796/1/README.rst | 21:47 |
nkinder | morganfainberg: this has received all reviews, so it's reverify, right? | 21:47 |
morganfainberg | nkinder, not that it wont work if you recheck bug ### | 21:47 |
morganfainberg | nkinder, recheck/reverify does the same thing, it'll re-run the check | 21:47 |
nkinder | morganfainberg: ok, the wiki disagrees, but I'll try it | 21:47 |
morganfainberg | nkinder, ah maybe they haven't announced that stuff yet | 21:48 |
morganfainberg | sometimes i mix up IRC chatter/capabilities merged and official stance | 21:48 |
morganfainberg | / announcements | 21:48 |
*** dolphm changes topic to "{"feature_freeze": "september 4"}" | 21:49 | |
morganfainberg | nkinder, it's hard to keep it all 100% straight when you lurk in ~15 channels | 21:49 |
nkinder | morganfainberg: tell me about it... :) | 21:49 |
morganfainberg | nkinder, and i don't even have the internal RH IRC to contend with | 21:49 |
nkinder | my xchat tabs scroll off the right side of my screen, and I use small fonts | 21:50 |
morganfainberg | nkinder, lol | 21:53 |
dolphm | morganfainberg: how about that audit question, or can i help with nkinder's recheckage | 21:54 |
morganfainberg | dolphm, do we want compatibility code for old token pre-audit ids? | 21:55 |
morganfainberg | dolphm, i have that code in now. but if we want to enforce "no audit id no auth" i can pull it out | 21:55 |
dolphm | morganfainberg: uhh, where? in keystone? | 21:56 |
morganfainberg | dolphm, yeah in keystone | 21:56 |
dolphm | morganfainberg: is it on gerrit? | 21:56 |
morganfainberg | dolphm, it would affect keystone rest apis and validated tokens. | 21:56 |
morganfainberg | dolphm, https://review.openstack.org/#/c/114864/8/keystone/token/provider.py the comments by ayoung and there are some special case bits of code that verify the audit_ids exist or set them to an empty value | 21:57 |
morganfainberg | dolphm, earlier in the chain | 21:57 |
morganfainberg | dolphm, let me dig up specifics. | 21:57 |
morganfainberg | dolphm, example https://review.openstack.org/#/c/114306/9/keystone/auth/plugins/token.py the top change in that file | 21:58 |
morganfainberg | and https://review.openstack.org/#/c/114306/9/keystone/token/controllers.py line 260ish | 21:58 |
morganfainberg | dolphm, we can slate that compat code to be removed in K as well if we want to maintain live tokens across the upgrade | 21:59 |
*** harlowja_ has quit IRC | 21:59 | |
dolphm | morganfainberg: so, basically your compatibility is a fail safe? | 22:00 |
topol | dolphm, so https://review.openstack.org/#/c/115771/1..2/pycadf/credential.py looks good to me. | 22:00 |
openstackgerrit | A change was merged to openstack/python-keystoneclient: Remove lxml as a forced depend https://review.openstack.org/114701 | 22:00 |
morganfainberg | dolphm, yeah, basically we allow tokens that were issued prior to the upgrade with audit ids vs. rejecting them | 22:00 |
openstackgerrit | A change was merged to openstack/python-keystoneclient: Allow passing user_id to v2Password plugin https://review.openstack.org/113712 | 22:01 |
openstackgerrit | A change was merged to openstack/python-keystoneclient: Fix handling of deprecated opts in CLI https://review.openstack.org/113859 | 22:01 |
morganfainberg | dolphm, but all new tokens get audit_ids and use them | 22:01 |
*** harlowja has joined #openstack-keystone | 22:02 | |
*** jorge_munoz has left #openstack-keystone | 22:02 | |
dolphm | morganfainberg: that sounds reasonable... if a deployer wanted to force audit IDs everywhere immediately, how would they do that? flush the token table, and? | 22:05 |
dolphm | truncate* | 22:05 |
morganfainberg | dolphm, for UUID truncate, for PKI, issue a revocation for all tokens and/or a revocation event for issued_before = now | 22:06 |
morganfainberg | dolphm, (don't think we have that ability on the rev. events) | 22:06 |
*** dims has joined #openstack-keystone | 22:06 | |
morganfainberg | but most tokens will naturally shake out and expire. so if there is some overlap between audit_id and non-audit id i don't think it'll hurt anyone | 22:07 |
openstackgerrit | Dolph Mathews proposed a change to openstack/python-keystoneclient: Remove cruft from setup.cfg https://review.openstack.org/114708 | 22:07 |
openstackgerrit | Dolph Mathews proposed a change to openstack/python-keystoneclient: Unsort pbr and hacking in requirements files https://review.openstack.org/114707 | 22:07 |
nkinder | morganfainberg, dolphm: jenkins/gerrit doesn't like my recheckage... - https://review.openstack.org/#/c/104408/ | 22:08 |
nkinder | ...and now I appear desperate in my constant "recheck" comments :) | 22:08 |
*** mrutkows has quit IRC | 22:09 | |
openstackgerrit | Dolph Mathews proposed a change to openstack/python-keystoneclient: Remove cruft from setup.cfg https://review.openstack.org/114708 | 22:09 |
openstackgerrit | Dolph Mathews proposed a change to openstack/python-keystoneclient: Unsort pbr and hacking in requirements files https://review.openstack.org/114707 | 22:09 |
morganfainberg | nkinder, it looks like it's already in the recheck queue | 22:09 |
nkinder | huh, guess it doesn't add a comment in gerrit | 22:10 |
morganfainberg | nkinder, about 37 minutes ago | 22:10 |
dolphm | jamielennox|away: bknudson: trivially rebased this, if one of you wants to +A again https://review.openstack.org/#/c/114707/ | 22:10 |
nkinder | I expected a "Starting gate jobs" comment | 22:10 |
*** mrmoje has quit IRC | 22:11 | |
morganfainberg | nkinder, you may need to either rebase/change commit msg or wait for it to fall out of the check queue | 22:11 |
dolphm | nkinder: those have been suppressed due to the noise they were generating | 22:11 |
morganfainberg | dolphm, ah | 22:11 |
bknudson | I was wondering if they had gotten rid of those messages. | 22:11 |
dolphm | jenkins comments aren't published in the UI anymore | 22:11 |
dolphm | nor any CI, i don't think | 22:11 |
nkinder | dolphm: ok (even though it's hidden in the toggle CI stuff) | 22:11 |
morganfainberg | honestly i kindof liked the -2 going to a score of 0 | 22:11 |
morganfainberg | when you recheck/reverify | 22:12 |
morganfainberg | but i can see why the extra noise is bad | 22:12 |
dolphm | morganfainberg: that still happens, there's just not record of it in the UI :-/ | 22:12 |
morganfainberg | dolphm, ah it's not happening on this review | 22:12 |
dolphm | morganfainberg: the vote should be reset when you recheck - it's not? | 22:12 |
morganfainberg | dolphm, i think the review got into check queue before it was -2 scored, so now it's running check again | 22:12 |
dolphm | morganfainberg: https://review.openstack.org/#/c/104408/ ? | 22:12 |
morganfainberg | dolphm, not on nkinder's | 22:12 |
bknudson | did they finally get rid of reverify? | 22:12 |
morganfainberg | dolphm, yeah | 22:13 |
dolphm | bknudson: kinda sorta yes | 22:13 |
morganfainberg | dolphm, that is *in* the check queue | 22:13 |
morganfainberg | dolphm, but it didn't clear the score (it landed in check queue about 37 mins ago) | 22:13 |
morganfainberg | which is why it didn't respond to nkinder's or my 'recheck' comments | 22:13 |
morganfainberg | s/37/25/ | 22:14 |
dolphm | morganfainberg: nkinder: well that's weird. hopefully it doesn't vanish when it's done with the check | 22:14 |
morganfainberg | dolphm, it should get a +1 or move to gate (depending on things) | 22:14 |
morganfainberg | if it stops at +1 it's easy to rekick, just bug me if i don't catch it | 22:15 |
dolphm | morganfainberg: ++ same here nkinder | 22:15 |
dolphm | nkinder: i actually have a big orange alarm on my desk when we have approved reviews that are stuck, so i'll certainly follow up :) | 22:16 |
morganfainberg | dstanek, woth upgrading https://review.openstack.org/#/c/114306/ to +2 since the py3k + urlsafe stuff passed check | 22:16 |
morganfainberg | dstanek, ? | 22:16 |
dstanek | morganfainberg: sure - any objections to a +A at this point? | 22:19 |
morganfainberg | dstanek, not from me | 22:19 |
openstackgerrit | Dolph Mathews proposed a change to openstack/keystone: move GET /v3/catalog to GET /v3/auth/catalog https://review.openstack.org/108043 | 22:21 |
*** jamielennox|away is now known as jamielennox | 22:22 | |
dstanek | morganfainberg: looking back at the history bknudson had quite a few comments - might be wise to have him give it a quick review | 22:23 |
morganfainberg | dstanek, works for me | 22:23 |
bknudson | which? | 22:24 |
morganfainberg | bknudson, https://review.openstack.org/#/c/114306/ audit_id adding | 22:24 |
*** jasondotstar has quit IRC | 22:24 | |
bknudson | morganfainberg: I thought you were going to do another rev | 22:24 |
bknudson | trailing whitespace in the commit message! | 22:25 |
morganfainberg | bknudson, ... | 22:25 |
jamielennox | dolphm: i didn't use your move /catalog patch. just moved it here: https://review.openstack.org/#/c/114903/ | 22:25 |
morganfainberg | stupid gerrit web interface | 22:25 |
dolphm | jamielennox: i'm just noticing that | 22:25 |
dolphm | jamielennox: why not? | 22:25 |
*** nkinder has quit IRC | 22:26 | |
morganfainberg | bknudson, let me know if you see anything else needed before i just fix that (i guess) | 22:26 |
morganfainberg | bknudson, :) | 22:26 |
bknudson | morganfainberg: I still need to be convinced as to what's supposed to happen if the original token doesn't have audit_id | 22:26 |
bknudson | I don't want another CVE | 22:27 |
jamielennox | dolphm: no reason, i should have - but i started working on the other stuff without finding that patch and just moved it myself | 22:27 |
jamielennox | dolphm: can rebase ontop if you like, it'll be more or less exactly the same | 22:27 |
bknudson | morganfainberg: is the revocation event going to have the expires_at instead? | 22:27 |
morganfainberg | bknudson, it's strictly compatibility, in the patch that does the actual revocation events, it will revoke_by_expires if the audit_id doesn't exist | 22:27 |
morganfainberg | bknudson, https://review.openstack.org/#/c/114864/8/keystone/token/provider.py | 22:27 |
dolphm | jamielennox: you must not have tried very hard lol | 22:28 |
jamielennox | dolphm: nope, didn't even think about it at the time | 22:28 |
*** david-lyle has quit IRC | 22:29 | |
bknudson | morganfainberg: what if it's a child token of a token that didn't have an audit_id? | 22:29 |
*** david-lyle has joined #openstack-keystone | 22:29 | |
dolphm | jamielennox: why is it "def list_domains_for_user" if it's for a user and a list of groups and the sql implementation will not returning anything if i provide both? | 22:29 |
bknudson | morganfainberg: the child would only have audit_id and not the audit_chain_id | 22:29 |
*** jasondotstar has joined #openstack-keystone | 22:29 | |
morganfainberg | bknudson, correct | 22:30 |
morganfainberg | bknudson, it would be treated as the start of a chain. | 22:30 |
morganfainberg | bknudson, i was actually just checking that before answering :) | 22:30 |
jamielennox | dolphm: it doesn't return for both? it did... | 22:31 |
bknudson | morganfainberg: so you revoke the parent token and the child would be revoked because of expires_at? | 22:31 |
morganfainberg | bknudson, yes. | 22:31 |
*** marcoemorais has quit IRC | 22:31 | |
dolphm | jamielennox: it won't return anything, but based on the method signature alone, i have no idea what the method is supposed to do | 22:32 |
bknudson | morganfainberg: ok... I can't think of any problems. | 22:32 |
*** marcoemorais has joined #openstack-keystone | 22:32 | |
morganfainberg | bknudson, if we need to smash the whitespace in the commit msg (pep8 obviously doesn't care) i'll do that now. | 22:32 |
morganfainberg | bknudson, aparently it's easy to get that in there from the gerrit web interface :P | 22:32 |
*** marcoemorais has quit IRC | 22:33 | |
*** marcoemorais has joined #openstack-keystone | 22:33 | |
jamielennox | method signautre is exactly the same as list_projects_for_user | 22:34 |
bknudson | morganfainberg: I still think it would be easier if the child of a token without audit_id didn't have an audit_id. | 22:34 |
jamielennox | dolphm: there is a test there that should test exactly this case: test_list_domains_for_user_with_grants | 22:35 |
morganfainberg | bknudson, i think it will be a very minimal window where we'd be dealing with it (if at all). | 22:35 |
jamielennox | dolphm: it'll be called with the user, and then the groups that the user is a part of | 22:35 |
bknudson | morganfainberg: it could be a rolling update. | 22:36 |
bknudson | I suppose we don't support that since the database wouldn't work. | 22:37 |
morganfainberg | bknudson, yeah. | 22:37 |
bknudson | someday maybe | 22:37 |
dolphm | jamielennox: so instead of overloading the method where the name makes no sense, why not have two methods? | 22:37 |
morganfainberg | bknudson, i was thinking of how i'd do a rolling update | 22:37 |
jamielennox | seperate list_domains_for_user and list_domains_for_groups? can do, i was going for consistency with the existing list_projects_for_user which works in that way | 22:38 |
*** jasondotstar has quit IRC | 22:39 | |
jamielennox | which is why i copied the big comment from henrynash into the new method as well | 22:40 |
*** hrybacki has quit IRC | 22:40 | |
dolphm | jamielennox: do you ever call it with groups? | 22:41 |
jamielennox | dolphm: it's the users groups, not just groups | 22:41 |
jamielennox | so no | 22:41 |
*** gordc has quit IRC | 22:41 | |
jamielennox | https://review.openstack.org/#/c/114903/3/keystone/assignment/core.py | 22:42 |
jamielennox | the api function is list_domains_for_user(self, user_id, hints=None) | 22:42 |
*** henrynash has quit IRC | 22:42 | |
dolphm | oh i see what you did | 22:42 |
jamielennox | which finds the groups associated with the user_id and passes them to backend | 22:42 |
dolphm | jamielennox: so the manager method name is fine, but the driver method name is overloaded | 22:42 |
dolphm | jamielennox: yeah, can you break the driver calls in two? | 22:43 |
jamielennox | i wouldn't say i'm a fan of the approach and i can change it, but i went that way for precedent | 22:43 |
dolphm | jamielennox: the overloaded methods in assignment are a bad precedent :) no need to contribute to it! | 22:43 |
jamielennox | https://github.com/openstack/keystone/blob/master/keystone/assignment/backends/sql.py#L236 | 22:43 |
jamielennox | dolphm: alright, i'll fix that | 22:44 |
jamielennox | i think there is already a domains_for_groups function that the federation work needed | 22:44 |
dolphm | jamielennox: actually, list_domains_for_groups is already implemented | 22:44 |
*** topol has quit IRC | 22:44 | |
jamielennox | yep, there is a list_projects_for_groups as well... | 22:45 |
jamielennox | was thinking when i wrote this it'd be nice to do a mass cleanup of the calls to that backend | 22:45 |
*** amerine_ has quit IRC | 22:46 | |
dolphm | jamielennox: oh, you were following: return self.driver.list_projects_for_user(user_id, group_ids, hints or driver_hints.Hints()) # yuck! | 22:47 |
jamielennox | yep | 22:47 |
jamielennox | dolphm: so, still split? | 22:51 |
dolphm | jamielennox: yes definitely, but i'm trying to decide if i want to +A as-is and then see a refactor for all this at once | 22:51 |
jamielennox | refactor including the old list_projects_for_users you mean? | 22:52 |
*** dims has quit IRC | 22:52 | |
dolphm | jamielennox: yeah | 22:53 |
dolphm | jamielennox: i'd rather not hold up your patch, and i'd rather see all this fixed in lockstep | 22:53 |
dolphm | jamielennox: holding up your patch to introduce more incosistency, even if it's more logical = meh | 22:54 |
dolphm | jamielennox: almost done reviewing tests... | 22:54 |
*** amerine has quit IRC | 22:55 | |
dolphm | jamielennox: +A | 22:55 |
dolphm | and my laptop battery is at 1% so /me waves goodbye | 22:55 |
*** bknudson has quit IRC | 22:55 | |
jamielennox | dolphm: let me know what you want me to do regarding a cleanup | 22:56 |
jamielennox | dolphm: my thought as well, that way isn't great but if i was implementing my own backend i prefer weird and consistent than all over the shop | 22:57 |
*** hrybacki has joined #openstack-keystone | 22:58 | |
*** hrybacki has quit IRC | 23:03 | |
*** shakamunyi has quit IRC | 23:13 | |
*** alex_xu has quit IRC | 23:25 | |
*** topol has joined #openstack-keystone | 23:25 | |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Allow unauthenticated discovery https://review.openstack.org/107570 | 23:29 |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Make keystoneclient use an adapter https://review.openstack.org/97681 | 23:29 |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Allow providing a default value to CLI loading https://review.openstack.org/113742 | 23:29 |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Version independent plugins https://review.openstack.org/81147 | 23:29 |
*** zzzeek has quit IRC | 23:30 | |
*** zzzeek has joined #openstack-keystone | 23:30 | |
*** david-lyle has quit IRC | 23:34 | |
*** hrybacki has joined #openstack-keystone | 23:40 | |
*** portante has quit IRC | 23:40 | |
*** portante has joined #openstack-keystone | 23:40 | |
*** ncoghlan has joined #openstack-keystone | 23:43 | |
*** nkinder has joined #openstack-keystone | 23:46 | |
*** gothicmindfood has quit IRC | 23:46 | |
*** gothicmindfood has joined #openstack-keystone | 23:46 | |
*** dhellmann has quit IRC | 23:49 | |
*** dhellmann has joined #openstack-keystone | 23:50 | |
*** RicoLin has joined #openstack-keystone | 23:52 | |
*** openstackgerrit has quit IRC | 23:54 | |
*** gokrokve has quit IRC | 23:57 | |
*** openstackgerrit has joined #openstack-keystone | 23:57 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!