Wednesday, 2014-08-20

*** marcoemorais has quit IRC00:01
*** marcoemorais has joined #openstack-keystone00:02
*** alex_xu has quit IRC00:03
*** gokrokve has joined #openstack-keystone00:14
openstackgerritJamie Lennox proposed a change to openstack/keystonemiddleware: Convert true string parsing to use strutils  https://review.openstack.org/11543800:16
*** dims has joined #openstack-keystone00:18
*** henrynash has quit IRC00:19
jamielennoxis 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 IRC00:21
*** gokrokve has joined #openstack-keystone00:21
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Update AuthContextMiddleware to not use token_api  https://review.openstack.org/11342900:21
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Add __repr__ to KeystoneToken model  https://review.openstack.org/11343000:22
jamielennoxgyee: ^^00:22
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Add extra guarding to revoke_by_audit_id methods  https://review.openstack.org/11514700:22
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove trust dependency on token_api  https://review.openstack.org/10946200:22
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove SAML2 plugin dependency on token_api  https://review.openstack.org/11501200:22
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove identity_api dependency on token_api  https://review.openstack.org/11504500:23
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove wsgi and base controller dependency on token_api  https://review.openstack.org/11520500:23
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Notification Constant Cleanup and internal notify type  https://review.openstack.org/11533700:23
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove assignment_api dependency on token_api  https://review.openstack.org/11533800:23
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove oauth controller dependency on token_api  https://review.openstack.org/11534300:23
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Mark methods on token_api deprecated  https://review.openstack.org/11534700:23
*** bknudson has joined #openstack-keystone00:24
gyeejamielennox, global state? not sure if I understand the question00:27
jamielennoxgyee: so i have my cool little session.load_from_conf_options function00:27
jamielennoxwhich was pretty much designed to mirror what we needed in middleware00:27
jamielennoxand then i now realize that CONF isn't enough because we also accept variables from paste.ini which comes through as parameters00:28
jamielennoxso if i load_from_conf_options i will miss anything coming via parameter00:28
jamielennoxi was looking at doing a CONF.set_override(k, v, group='keystone_authtoken') for every element in the paste params00:29
gyeeahh, I see what you mean00:29
jamielennoxbut i go from a local state to a global state00:29
jamielennoxi just can't tell if that matters00:29
jamielennoxi 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 emulate00:32
*** ncoghlan_afk is now known as ncoghlan00:32
openstackgerritSteve Martinelli proposed a change to openstack/keystone: Add CADF notifications for role assignment create and delete  https://review.openstack.org/11220400:33
gyeejamielennox, overriding global conf seem like a bad idea00:33
gyeebut plugins have access to global conf right?00:34
gyeeso I am guessing you will have to do that _conf_get dance there00:34
jamielennoxright, i can construct the session pretty simply00:35
jamielennoxbut its a fair bit to do to construct a plugin00:36
jamielennoxstupid CONF should never have been global00:37
gyeejamielennox, I hear ya, the group registration is inconsistent00:40
gyeenova api-paste.ini have it as 'authtoken'00:40
gyeehttps://github.com/openstack/nova/blob/master/etc/nova/api-paste.ini#L11700:40
gyeebut we are register the opts in the "keystone_authtoken" group00:41
gyeeso essentially, the same opt can come from multiple places00:42
*** ncoghlan is now known as ncoghlan_afk00:42
jamielennoxyea, it doesn't matter because paste just takes whatever else is in that ini section and passes it through to the object creation00:44
jamielennoxso authtoken is ignored in that sense00:44
jamielennoxeverything about oslo.config is private as well, there's no where i can override anything00:45
gyeeha00:45
openstackgerritSteve Martinelli proposed a change to openstack/keystone: Add CADF notifications for role assignment create and delete  https://review.openstack.org/11220400:46
*** marcoemorais has quit IRC00:47
*** cjellick has quit IRC00:47
*** cjellick has joined #openstack-keystone00:48
*** cjellick has quit IRC00:52
*** xianghui has quit IRC00:54
*** gokrokve_ has joined #openstack-keystone00:56
*** xianghui has joined #openstack-keystone00:58
*** gokrokve has quit IRC00:59
*** xianghui has quit IRC01:07
openstackgerritJamie Lennox proposed a change to openstack/keystonemiddleware: Wrap a CONF object and intercept authtoken options  https://review.openstack.org/11545101:07
*** xianghui has joined #openstack-keystone01:07
*** jorge_munoz has joined #openstack-keystone01:08
jamielennoxgyee: ^^ yuk01:08
openstackgerritSteve Martinelli proposed a change to openstack/keystone: Create additional docs for role assignment events  https://review.openstack.org/11481301:10
*** gokrokve_ has quit IRC01:13
morganfainbergjamielennox, there is a bug that fix will also close.01:13
morganfainbergjamielennox, fyi01:13
jamielennoxmorganfainberg: oh? cause at the moment i want to drown it01:14
morganfainbergwell if you can turn the values from paste into something that oslo can consume01:14
morganfainbergit would be much better01:14
jamielennoxmorganfainberg: 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 paste01:16
jamielennoxbut it's kind of a local state vs global state problem01:16
*** ayoung has quit IRC01:17
morganfainbergright01:18
jamielennoxwhat bug are you referring to?01:19
*** gokrokve has joined #openstack-keystone01:23
*** dims has quit IRC01:26
*** ncoghlan_afk is now known as ncoghlan01:26
morganfainbergjamielennox, can't find it right now, but i know it's around01:27
jamielennoxmorganfainberg: you remember what the issue was?01:27
jamielennoxmostly that it was central to auth_token and not somebody else trying to read config options from us and missing paste options01:28
*** gokrokve_ has joined #openstack-keystone01:34
morganfainbergjamielennox, nope, i'll need to dig around to try and figure out what the issue really was01:37
jamielennoxmorganfainberg: it's ok, i would just feel better about such a hack if there was another reason01:37
*** gokrokve has quit IRC01:37
morganfainbergmaybe 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 IRC01:39
*** gordc has joined #openstack-keystone01:44
*** richm has quit IRC01:47
bknudsonmorganfainberg: I think the goal was to deprecate the options in paste.ini a long time ago01:49
bknudsonwe could not support it for new options.01:50
openstackgerritBrant Knudson proposed a change to openstack/keystone: Enhance GET /v3 to handle Accept header  https://review.openstack.org/11546201:54
openstackgerritBrant Knudson proposed a change to openstack/keystone: Enhance GET /v3 to handle Accept header  https://review.openstack.org/11546201:54
bknudsonjamielennox: morganfainberg: the problem was that there was an integer config option but from paste.ini it's a string.01:58
morganfainbergbknudson, thats the one!01:58
bknudsonhttps://review.openstack.org/#/c/113191/01:58
bknudsonthere's already a fix for it!01:59
jamielennoxoh, yea and it messes with delay_auth_decision because otherwise the BoolOpt would handle it's type01:59
jamielennoxi only saw that the other day, should have realized01:59
bknudsonjamielennox: morganfainberg: the fix doesn't look too bad.02:00
*** shakamunyi has quit IRC02:01
jamielennoxbknudson: it's kind of insignificant i'm just not a fan of the loop02:02
jamielennoxthe way auth_token is written we will only read conf options once anyway02:02
bknudsonloops are evil!02:02
jamielennoxi'd prefer he just put it in _conf_get02:02
bknudsonjamielennox: then it would have to find the option every time _conf_get is called... gross.02:03
jamielennoxbknudson: hmm, move opts to a dict02:03
bknudsonthe change also seems to make assumptions about CONF options that I'd rather weren't made02:04
bknudsonare they really all going to process the value using type() ?02:04
bknudsondoes that handle config options with max and min values correctly?02:04
bknudsonseems like conf should work that way.02:05
bknudsondhellmann had a different suggestion02:05
*** xianghui has quit IRC02:07
*** xianghui has joined #openstack-keystone02:08
jamielennoxyea, types are callable and do that stuff02:08
dhellmannbknudson, 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 do02:13
jamielennoxdhellmann: the problem is similar to what we've been talking about in channel recently02:14
jamielennoxdhellmann: we have two sources of config, CONF and the params from paste.ini02:14
jamielennoxif we use set_default or set_override on CONF then we are taking state local to auth_token and setting it globally02:14
*** alex_xu has joined #openstack-keystone02:14
dhellmannif 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 you02:14
dhellmannjamielennox: have you seen the config filter stuff markmc added this cycle?02:15
jamielennoxdhellmann: i remember being referred to it for something else but i haven't looked closely02:15
dhellmannjamielennox: http://docs.openstack.org/developer/oslo.config/cfgfilter.html02:15
jamielennoxmy understanding that was more protection though02:15
jamielennoxso that nobody outside of your module could modify your state02:15
dhellmannjamielennox: right, that avoids the global issue I thought you were talking about02:16
jamielennoxdhellmann: i guess the (mostly theoretical) problem is that people could start multipe auth_tokens from different paste.ini pipelines with different configs02:17
dhellmannwhen you say "local to auth_token" do you mean an individual user token?02:17
jamielennoxno its provided at __init__ to auth_token02:17
dhellmannjamielennox: ok, different pipelines makes sense02:17
dhellmannthey have names or something, right?02:17
dhellmannyou 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 object02:18
jamielennoxi don't think names bubble down into code02:18
jamielennoxdhellmann: looking at http://docs.openstack.org/developer/oslo.config/cfgfilter.html#private-configuration-options02:18
dhellmannok, 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 case02:19
jamielennoxactually that might work02:19
jamielennox        self._private_conf = ConfigFilter(self.conf)02:20
jamielennox        self._private_conf.register_opt(StrOpt('foo'))02:20
dhellmannthere'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 exposed02:20
jamielennoxif 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 problem02:20
dhellmannjamielennox: you would still need different groups, though, because underneath I think the filter is using the same storage for option values02:20
dhellmannah, true, you don't have options in that storage, so using set_default is going to update the option definition02:21
dhellmannso maybe the filter would do what you want02:21
jamielennoxdhellmann: at the moment i'm looking at something like: https://review.openstack.org/#/c/115451/02:21
*** ncoghlan has quit IRC02:21
jamielennoxfor a slightly different reason, but to make the values from paste look like they came via conf02:22
jamielennoxit's pretty horrible02:22
*** jimbaker has quit IRC02:22
dhellmannI 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-keystone02:23
jamielennoxnot really02:23
*** jimbaker has quit IRC02:23
*** jimbaker has joined #openstack-keystone02:23
dhellmannyeah02:23
dhellmannat least they take the object as an argument, so you should be able to pass the filter instead02:23
*** gordc has quit IRC02:24
jamielennoxhow do those options then get exposed to a config generator?02:24
*** diegows has quit IRC02:25
dhellmannthese options can be set in the global config as well?02:25
dhellmannoh, yeah, you said two sources, didn't you02:25
jamielennoxbecause i would need to move nearly the entire OPT list into private_configs02:25
jamielennoxyes, it needs to work as a regular CONF as well02:26
dhellmannI'm curious, what's the use case for this?02:27
dhellmannI'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.config02:28
jamielennoxall of the session and auth loading work is written to be used by everyone by session.load_from_conf_object auth.load_from_conf_object02:28
jamielennoxthis was fine until realizing that it's not just a CONF object we have, we have to include the paste.ini attributes as well02:29
dhellmannso 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 elsewhere02:29
dhellmannwhy are there auth settings in the paste file?02:29
jamielennoxdhellmann: i'm not sure, apparently we've tried to kill them in the past but people keep putting them there02:30
dhellmannjamielennox: ok, I won't dredge that history up then :-)02:30
dhellmannjamielennox: 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, too02:30
bknudsonI think we can live without supporting new options in api-paste.ini02:30
jamielennoxok so a ConfigOverride object is pretty much what i'm looking for, a 'local view' of the CONF02:30
dhellmannexactly02:31
bknudsonsupport the ones that are already there but don't bother with new ones02:31
dhellmannand the filter is one type of view like that, but it's sort of backwards for what you need in this case02:31
dhellmannbknudson: that might turn out to be more confusing, in the end, than just allowing all of the options to be overridden02:31
bknudsonI agree it will be confusing and hopefully that will convince people to stop putting stuff in api-paste.ini.02:32
dhellmannjamielennox: 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 not02:32
dhellmannbknudson: :-)02:32
jamielennoxbknudson: i was thinking that anything new from an auth plugin perspective could ignore paste.ini02:32
jamielennoxbknudson: i think he meant confusing for us :)02:33
dhellmannjamielennox: no, I did actually mean deployers ("why can I set foo in paste.ini but not bar?")02:34
*** gordc has joined #openstack-keystone02:34
jamielennoxok, well both then02:34
*** gordc has quit IRC02:34
dhellmannbut I think that only goes as far as this "class" of options (the auth_token stuff) and not *all* options02:34
*** shakamunyi has joined #openstack-keystone02:34
jamielennoxwould a view have the option to register?02:34
jamielennoxregister new options on it02:35
dhellmannjamielennox: no, I wouldn't expect it to, but you could always combine it with a filter in a stack02:35
dhellmannfilter(view(parser()))02:35
*** harlowja is now known as harlowja_away02:35
dhellmannin principle it should be possible to nest views and filters arbitrarily02:35
dhellmannjamielennox: although I've never tried that, so I don't know if it actually works :-)02:36
jamielennoxmaybe it does need to be called an override, a view sounds like it's restricting it or modifying it somehow02:38
*** vkmc has quit IRC02:38
dhellmannyeah02:38
*** shakamunyi has quit IRC02:41
*** shakamunyi has joined #openstack-keystone02:42
jamielennoxdhellmann: 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 at02:44
jamielennoxdhellmann: thanks02:44
dhellmannjamielennox: 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 regularly02:45
jamielennoxdhellmann: will do02:45
*** KanagarajM has joined #openstack-keystone03:00
*** xianghui has quit IRC03:09
*** xianghui has joined #openstack-keystone03:10
*** rushiagr_away has quit IRC03:12
*** rushiagr_away has joined #openstack-keystone03:14
*** dims has joined #openstack-keystone03:14
*** xianghui has quit IRC03:16
*** alex_xu has quit IRC03:17
*** xianghui has joined #openstack-keystone03:17
*** dims has quit IRC03:23
*** alex_xu has joined #openstack-keystone03:23
openstackgerritKanagaraj Manickam proposed a change to openstack/keystone: endpoint table is missing reference to region table  https://review.openstack.org/11318303:49
*** gokrokve has joined #openstack-keystone03:57
*** rushiagr_away is now known as rushiagr03:59
*** chandankumar has joined #openstack-keystone03:59
*** gokrokve has quit IRC04:00
*** gokrokve has joined #openstack-keystone04:00
*** oomichi has joined #openstack-keystone04:01
*** gokrokve has quit IRC04:14
*** gokrokve has joined #openstack-keystone04:15
*** xianghui has quit IRC04:17
*** xianghui has joined #openstack-keystone04:17
*** amirosh has joined #openstack-keystone04:18
*** gokrokve has quit IRC04:40
*** gokrokve has joined #openstack-keystone04:41
*** gokrokve has quit IRC04:45
*** chandankumar has quit IRC04:52
*** stevemar has quit IRC04:54
*** chandankumar has joined #openstack-keystone04:59
*** chandankumar has quit IRC05:02
*** dims has joined #openstack-keystone05:07
*** dims has quit IRC05:11
*** ajayaa has joined #openstack-keystone05:19
*** ajayaa has quit IRC05:34
*** chandankumar has joined #openstack-keystone05:40
*** jimbaker has quit IRC05:42
*** jimbaker has joined #openstack-keystone05:46
*** jimbaker has quit IRC05:46
*** jimbaker has joined #openstack-keystone05:46
*** jorge_munoz has quit IRC05:47
*** jorge_munoz has joined #openstack-keystone05:47
*** jorge_munoz has quit IRC05:51
*** tomoiaga has joined #openstack-keystone05:55
*** ajayaa has joined #openstack-keystone06:03
openstackgerritOpenStack Proposal Bot proposed a change to openstack/keystone: Imported Translations from Transifex  https://review.openstack.org/11192006:06
*** afazekas has quit IRC06:11
*** afazekas_ has quit IRC06:16
*** marekd|away is now known as marekd06:38
*** shakamunyi has quit IRC06:43
*** shakamunyi has joined #openstack-keystone06:44
*** shakamunyi has quit IRC06:47
*** shakamunyi has joined #openstack-keystone06:48
*** afazekas_ has joined #openstack-keystone06:57
openstackgerritwanghong proposed a change to openstack/keystonemiddleware: convert the conf value into correct type  https://review.openstack.org/11319107:08
*** afazekas_ is now known as afazekas07:32
*** henrynash has joined #openstack-keystone07:41
*** tomoiaga has quit IRC07:56
*** alex_xu has quit IRC07:59
*** oomichi has quit IRC08:07
openstackgerritMarek Denis proposed a change to openstack/keystone: IdP SAML Metadata generator  https://review.openstack.org/11485008:10
*** alex_xu has joined #openstack-keystone08:11
mhumarekd, I don't know if you answered me yesterday, my irc client crashed :/08:20
*** jimbaker has quit IRC08:21
openstackgerritMarek Denis proposed a change to openstack/keystone: IdP SAML Metadata generator  https://review.openstack.org/11485008:21
marekdmhu: yes, I did08:21
marekdmhu: let me re-paste the link :-)08:21
marekdmhu: 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
marekdmhu:  https://review.openstack.org/#/c/110770/ this is unmerged, but it's easy to change locally :-)08:23
marekdmhu: and since osc is not ready yet I made a super-simple wrapper for that: https://gist.github.com/zaccone/509136cfa1c4efca692608:23
*** jimbaker has joined #openstack-keystone08:25
*** jimbaker has quit IRC08:25
*** jimbaker has joined #openstack-keystone08:25
mhumarekd, 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 stevedore08:25
*** shakamunyi has quit IRC08:25
marekdmhu: it's in the master but it was not released in the keystoneclient.08:25
marekdmhu: mainly due to we wanted to push bunch of plugins so ksc can be used for federated authN08:26
marekdand it took me some time to push it through reviews ;/08:26
marekdmhu: so I advise checking master, not pip08:26
mhumarekd, 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
marekdmhu: dolphm is responsible for cutting the ksc.08:27
marekdmhu: unless you need osc version.08:27
mhumarekd, so he's the one I'll bug then :)08:28
marekdmhu: than stevemar is a gatekeeper :-)08:28
*** alex_xu has quit IRC08:28
marekdprobably not before tomorrow as everybody is rushing due to PFP08:28
mhumarekd, it's alright, it'll give me time to patch something in ksc so that I can allow the use of admin tokens in osc08:29
marekdmhu: 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
mhumarekd, 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 better08:31
marekdthe wrapper?08:32
marekdmhu: i think this should be resolved on the lib level, not OSC08:32
mhumarekd, yes, therefore my question about v3samlscoped08:32
*** Dafna has quit IRC08:33
marekdmhu: uhm08:33
*** Dafna has joined #openstack-keystone08:34
mhumarekd, I am going to give it a proper look08:34
marekdmhu: thanks.08:39
*** alex_xu has joined #openstack-keystone08:42
openstackgerritMarek Denis proposed a change to openstack/keystone: Create SAML generation route and controller  https://review.openstack.org/11413808:43
*** shakamunyi has joined #openstack-keystone08:52
*** shakamunyi has quit IRC08:56
*** tomoiaga has joined #openstack-keystone08:57
*** alex_xu has quit IRC09:11
openstackgerrithenry-nash proposed a change to openstack/keystone: Implements backend for policy endpoint extension  https://review.openstack.org/11536209:17
*** henrynash has quit IRC09:30
*** ajayaa has quit IRC09:37
*** jeffrey4l has joined #openstack-keystone09:47
jeffrey4lAny core viewer can review my patch https://review.openstack.org/#/c/51610/09:50
jeffrey4lAny core reviewer can review my patch https://review.openstack.org/#/c/51610/09:50
*** ajayaa has joined #openstack-keystone09:52
openstackgerritMarek Denis proposed a change to openstack/keystone: Transform a Keystone token to a SAML assertion  https://review.openstack.org/11054209:52
*** shakamunyi has joined #openstack-keystone09:53
openstackgerritMarek Denis proposed a change to openstack/keystone: IdP SAML Metadata generator  https://review.openstack.org/11485009:54
*** shakamunyi has quit IRC09:57
openstackgerritMarcos Fermín Lobo proposed a change to openstack/python-keystoneclient: Attributes required using token for auth  https://review.openstack.org/11522809:59
*** openstackgerrit has quit IRC10:10
*** aix has joined #openstack-keystone10:19
*** ajayaa has quit IRC10:42
*** Kui has quit IRC10:45
*** amerine_ has joined #openstack-keystone10:59
*** amerine has quit IRC11:01
*** ajayaa has joined #openstack-keystone11:03
*** ajayaa has quit IRC11:20
*** ajayaa has joined #openstack-keystone11:21
*** ajayaa has quit IRC11:28
*** ajayaa has joined #openstack-keystone11:34
*** henrynash has joined #openstack-keystone11:42
*** RicoLin has quit IRC11:45
*** RicoLin has joined #openstack-keystone11:46
*** diegows has joined #openstack-keystone11:50
*** dims has joined #openstack-keystone11:50
*** KanagarajM has quit IRC12:06
*** alex_xu has joined #openstack-keystone12:17
*** ajayaa has quit IRC12:24
*** ajayaa has joined #openstack-keystone12:35
*** hrybacki has joined #openstack-keystone12:38
*** gordc has joined #openstack-keystone12:42
*** miqui has joined #openstack-keystone12:43
*** henrynash has quit IRC12:44
*** andreaf has joined #openstack-keystone12:51
*** andreaf has quit IRC12:51
*** andreaf has joined #openstack-keystone12:51
*** andreaf has quit IRC12:51
*** hrybacki has quit IRC12:52
*** jimbaker has quit IRC12:52
*** bknudson has quit IRC12:53
*** jimbaker has joined #openstack-keystone12:54
*** jimbaker has quit IRC12:54
*** jimbaker has joined #openstack-keystone12:54
*** aix has quit IRC13:00
*** stevemar has joined #openstack-keystone13:01
*** aix has joined #openstack-keystone13:03
*** henrynash has joined #openstack-keystone13:08
*** nkinder has quit IRC13:11
*** richm has joined #openstack-keystone13:12
*** radez_g0n3 is now known as radez13:16
*** henrynash has quit IRC13:31
*** bknudson has joined #openstack-keystone13:32
*** amcrn has joined #openstack-keystone13:35
*** ajayaa has quit IRC13:37
*** amcrn has quit IRC13:40
*** amcrn has joined #openstack-keystone13:57
*** samuelmz_ has joined #openstack-keystone14:08
*** aix has quit IRC14:08
*** nkinder has joined #openstack-keystone14:10
*** gokrokve has joined #openstack-keystone14:11
*** amcrn has quit IRC14:14
*** jorge_munoz has joined #openstack-keystone14:14
*** aix has joined #openstack-keystone14:20
*** samuelmz_ has quit IRC14:22
*** rwsu has quit IRC14:24
samuelmzHi, 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
samuelmzCould you take a look at the bug and confirm it? (https://bugs.launchpad.net/keystone/+bug/1359231)14:28
uvirtbotLaunchpad bug 1359231 in keystone "List role assignments filters performance " [Undecided,New]14:28
*** amirosh has quit IRC14:29
*** david-lyle has joined #openstack-keystone14:30
*** amirosh has joined #openstack-keystone14:30
*** david-lyle has quit IRC14:31
*** david-lyle has joined #openstack-keystone14:31
*** amirosh has quit IRC14:34
*** rwsu has joined #openstack-keystone14:36
*** amcrn has joined #openstack-keystone14:38
*** marekd is now known as marekd|away14:43
*** hrybacki has joined #openstack-keystone14:43
*** tomoiaga has quit IRC14:43
dolphmdstanek: is there a way in jsonschema to say property X xor property Y is required?14:45
dolphmdstanek: mutliple object definitions with oneOf?14:46
dstanekdolphm: yes, that's the only way i've seen it done; i don't think there is any direct support for it14:47
dolphmalrighty14:47
*** hrybacki has quit IRC14:47
dstanekdolphm: you're basically left to duplication14:47
*** openstackgerrit has joined #openstack-keystone14:59
*** shakamunyi has joined #openstack-keystone15:00
*** ayoung has joined #openstack-keystone15:04
*** zzzeek has joined #openstack-keystone15:05
*** amirosh has joined #openstack-keystone15:08
*** openstackgerrit has joined #openstack-keystone15:12
openstackgerritDavid Stanek proposed a change to openstack/keystone: Sync Py2 and Py3 requirements files  https://review.openstack.org/11567815:14
*** henrynash has joined #openstack-keystone15:17
*** gokrokve_ has joined #openstack-keystone15:20
*** mrmoje has joined #openstack-keystone15:21
*** topol has joined #openstack-keystone15:21
*** gokrokve has quit IRC15:23
*** jeffrey4l has quit IRC15:23
*** gokrokve_ has quit IRC15:24
*** cjellick has joined #openstack-keystone15:25
*** BAKfr has joined #openstack-keystone15:26
*** amcrn has quit IRC15:28
openstackgerritSteve Martinelli proposed a change to openstack/keystone: Create SAML generation route and controller  https://review.openstack.org/11413815:32
*** jeffrey4l has joined #openstack-keystone15:36
dstanekhenrynash: in https://review.openstack.org/#/c/112292/ you can't have more than one policy associated with an endpoint can you?15:36
dstaneki assume not because you call out that no merging of policies occurs15:36
*** amirosh has quit IRC15:41
*** amirosh has joined #openstack-keystone15:42
*** rushiagr is now known as rushiagr_away15:44
*** rushiagr_away is now known as rushiagr15:44
*** amirosh has quit IRC15:46
henrynashdstanek: explicitely to a given endpoint, no15:47
raildohenrynash: ping16:09
henrynashraildo: hi16:09
dstanekmorganfainberg: ping16:11
*** henrique_ has quit IRC16:13
*** raildo has quit IRC16:13
*** samuelmz has quit IRC16:14
*** rodrigods has quit IRC16:14
*** BAKfr has quit IRC16:14
*** raildo has joined #openstack-keystone16:15
*** htruta has joined #openstack-keystone16:15
raildohenrynash: I want to know if there's anything I can do to approve the spec about hierarchical projects.16:17
henrynashraildo: it’s done!16:20
henrynashraildo: now the eral fun begins...16:20
ayoungnkinder, 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_lookup16:20
henrynashreal even16:20
raildohenrynash: :-D:-D:-D:-D16:21
raildohenrynash: thank you!16:21
openstackgerritA change was merged to openstack/keystone-specs: Hierarchical Multitenacy  https://review.openstack.org/10101716:23
*** k4n0 has quit IRC16:25
*** gyee_ has joined #openstack-keystone16:27
*** gyee_ has left #openstack-keystone16:28
*** afazekas has quit IRC16:29
*** gyee_ has joined #openstack-keystone16:30
*** rushiagr is now known as rushiagr_away16:32
*** samuelmz has joined #openstack-keystone16:32
ayoungmorganfainberg, 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 IRC16:37
*** hrybacki has joined #openstack-keystone16:37
*** rushiagr_away is now known as rushiagr16:37
*** marcoemorais has joined #openstack-keystone16:38
*** gokrokve has joined #openstack-keystone16:38
*** hrybacki has quit IRC16:39
ayounghenrynash, 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
topolo/16:43
henrynashayoung: the only thing would be if there are some notifactios that don’t fit CADF…not sure if there are any16:43
morganfainbergdstanek, pong16:44
morganfainbergayoung, yes that is intended16:44
morganfainbergayoung, tokens without audit ids need to still be revokeable (upgrade path)16:45
topolprobably are some that require us to enhance pycadf. but I think that is a point in time statement as opposed to an architectural show stopper16:45
ayoungmorganfainberg, its hard coded to always be False, though16:45
morganfainberglook at line 439 and 43516:45
ayoungAh...ok,  hidden in there.  I'll revise my review16:46
morganfainbergayoung, if we could just reject all non-audit tokens it would be easier16:46
ayoungmorganfainberg, we can16:46
ayoungtokens don't live long.  Saying that an upgrade like this revokes all tokens is acceptable16:47
morganfainbergayoung, tokens lifespan is configurable16:47
ayoungit will cause a hiccup, but it just means that users need to re-authenticate16:47
ayoungwe've made that decision before, like on migrations on the token table16:47
morganfainbergayoung, i erred on the side of compatibility. but if we make that same decision again thats fine, i just didn't want to assume16:48
ayoungmorganfainberg, 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 happens16:51
morganfainbergayoung, yeah, worst case, we can yank that code in K.16:52
morganfainbergayoung, but yeah i'll go with dolphm's opinion on it16:52
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Fix Python3 issue with token data helper test  https://review.openstack.org/11570716:53
morganfainbergdstanek, ^16:53
morganfainbergdstanek, addressing the python3 issue in the tests16:53
openstackgerrithenry-nash proposed a change to openstack/keystone: Implements backend for policy endpoint extension  https://review.openstack.org/11536216:54
dstanekmorganfainberg: thanks16:54
dstanekmorganfainberg: after that initial review i think i have only found small nits16:55
*** mrmoje has quit IRC16:55
morganfainbergdstanek, thats good to hear :)16:56
morganfainbergayoung, 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 removal16:57
morganfainbergayoung, rather than re-engineering how to make things compatible16:57
morganfainbergdstanek, https://review.openstack.org/#/c/113430/12/keystone/models/token_model.py were you saying just put a space in?17:00
morganfainbergso <type> (values)17:01
morganfainbergvs <type>(values)17:01
morganfainbergor were you saying put all of the values of the token in?17:01
morganfainbergvs just audit ids17:01
*** richm has quit IRC17:01
*** browne has joined #openstack-keystone17:02
dstanekmorganfainberg: i was thinking if would look like: <KeystoneToken (audit_id=?, audit_chain_id=?) at 0xDEADBEEF>17:03
morganfainbergdstanek, cool thats how i read it17:03
morganfainbergdstanek, should i put "object" in?17:03
morganfainbergdstanek, /me doesn't really care, but wanted something more than KeystoneToken object17:03
dstanekmorganfainberg: i don't think it would benefit us so i say no17:03
morganfainbergk17:03
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Add __repr__ to KeystoneToken model  https://review.openstack.org/11343017:05
morganfainbergdone.17:06
morganfainbergdstanek, also addressed ayoung's comments (if you saw the convo here as well)17:08
morganfainbergdstanek, on https://review.openstack.org/#/c/11486417:08
ayoungmorganfainberg, I'm totally fine with that.  Yanking the "revoke by expiration" code altogether in Kilo is fine17:10
morganfainbergayoung, ++17:10
ayoungmorganfainberg, since you already wrote it17:11
ayoungI would have advised against it, up front, but since the price is paid...17:11
morganfainbergayoung, lol it was actually an afterthought because i ran into an issue with using a stale token on a rejoin-stack17:11
morganfainbergayoung, stale = pre-update to audit_ids17:12
ayoungmorganfainberg, right.  I think we've needed audit_ids for a long time.  Good to finally have them17:12
morganfainbergi 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
morganfainbergsince it's the identity-api stuff17:13
*** RicoLin has quit IRC17:13
*** richm has joined #openstack-keystone17:17
*** marcoemorais has quit IRC17:19
*** harlowja_away is now known as harlowja17:19
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Notification Constant Cleanup and internal notify type  https://review.openstack.org/11533717:20
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove assignment_api dependency on token_api  https://review.openstack.org/11533817:20
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Remove oauth controller dependency on token_api  https://review.openstack.org/11534317:20
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Mark methods on token_api deprecated  https://review.openstack.org/11534717:20
*** marcoemorais has joined #openstack-keystone17:22
*** marcoemorais has quit IRC17:23
*** marcoemorais has joined #openstack-keystone17:25
*** afazekas has joined #openstack-keystone17:30
openstackgerritA change was merged to openstack/keystonemiddleware: Hash for PKIZ  https://review.openstack.org/11464617:31
*** browne has quit IRC17:31
ayoungmorganfainberg, are you using the URL safe Base64 encoding for audit ids?17:32
*** amirosh has joined #openstack-keystone17:32
morganfainbergayoung, negative.17:32
morganfainbergayoung, there are no rest apis that include the audit id as part of the uri17:32
ayoungmorganfainberg, 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 point17:33
ayoungI'd either go with the same ID approach we are using elsewhere, or at least make them url safe17:33
ayoungthe 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
ayoungmaybe "url safe strings, no longer than 64 chars"17:34
*** nkinder has quit IRC17:35
morganfainberginitially i used .hex and was asked if we could make it shorter and not 'hex' limited17:35
morganfainberghex = 32 characters from uuid417:36
*** gyee_ has left #openstack-keystone17:36
*** amcrn has joined #openstack-keystone17:37
*** zzzeek has quit IRC17:37
ayoungmorganfainberg, 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  algorithm17:38
*** zzzeek has joined #openstack-keystone17:38
ayoungbut to do url safe base64 is pretty easy...its a slightly different call, and we already do that for PKIZ tokens17:38
ayoungbase64 non-url safe is kindof nasty17:39
morganfainbergcodecs doesn't support it so i'd need to move to using raw base64 lib17:39
morganfainbergcodecs only support b64 not b64 urlsafe17:39
*** bambam1 has joined #openstack-keystone17:39
morganfainbergnot a big deal. but honestly, we could also just urlencode the b64 id if we want to use it in rest somewhere17:40
bambam1Hello 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 version17:41
*** browne has joined #openstack-keystone17:41
morganfainbergbambam1, not in the token data itself.17:42
ayoungbambam1, PKI or UUID?17:42
ayoungmorganfainberg, codecs?17:43
morganfainbergayoung, doesn't matter, we don't put 'extra' fields in the token, or at least as far as i know we dont17:43
bambam1UUID17:43
morganfainbergayoung, codecs is the library (built-in) I was using for encoding the uuid17:43
morganfainbergayoung, it's a base-python lib17:43
ayoungbambam1, a user cannot figure out what data is in a UUID token.  They don;t have permissions to validate it17:43
ayoungmorganfainberg,  http://git.openstack.org/cgit/openstack/python-keystoneclient/tree/keystoneclient/common/cms.py#n20117:44
morganfainberglike i said, i'd just need to move away from using codecs17:44
bambam1I see I was trying to query the user's list with my username /v2.0/users/my_user17:45
bambam1but it seems like I have to be an admin for that17:45
*** raildo has quit IRC17:47
*** nkinder has joined #openstack-keystone17:48
*** samuelmz has quit IRC17:48
*** htruta has quit IRC17:49
ayoungbambam1, yep17:50
*** htruta has joined #openstack-keystone17:50
*** samuelmz has joined #openstack-keystone17:52
openstackgerritSteve Martinelli proposed a change to openstack/keystone: Create SAML generation route and controller  https://review.openstack.org/11413817:52
*** rodrigods has joined #openstack-keystone17:53
*** rodrigods has joined #openstack-keystone17:53
morganfainbergayoung, 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_away17:53
morganfainbergayoung, we absolutely should change the spec if we're cramming more ids in there.17:53
ayoungmorganfainberg, give a max length17:53
morganfainbergnot just limit in practice.17:53
morganfainberg217:53
morganfainbergit's already defined as having a max length :)17:54
ayoungmorganfainberg, why?17:54
morganfainbergayoung, to avoid token size creep and potential dos scenarios17:54
morganfainbergayoung, what use case do you want in there that isn't an unbounded dataset17:54
morganfainbergayoung, and if we move to unscoped -> scoped (not scoped -> scoped) that chain will always be 2 anyway17:55
ayoungmorganfainberg, why do it as an array, then?17:55
morganfainbergayoung, json doesn't support tuples.17:55
ayoungaudit_id  and parent_audit_id17:55
ayoungwith parent being optional17:55
ayoungmicrooptimization?17:56
morganfainbergayoung, because the array saves us bytes (yep another one i was asked for when pitching this)17:56
ayoungmorganfainberg, what about K2K?  We could possibly have length 4 there17:56
ayoungunscoped orig, scoped orig,  unscoped local, scoped local?17:56
morganfainbergayoung, 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 IRC17:59
*** dims has joined #openstack-keystone17:59
ayoungmorganfainberg, 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 anyway18:01
morganfainbergayoung, then propose an explicit set of data that will be in there, not "some number"18:01
morganfainbergayoung, not "up to XXX number"18:02
morganfainbergwhat is each element18:02
ayoungmorganfainberg, 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 anyway18:02
*** marcoemorais has quit IRC18:03
*** marcoemorais has joined #openstack-keystone18:03
ayoungits 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
morganfainbergayoung, 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 one18:03
ayoungmorganfainberg, the session token proposal I gave would require arbitrary length18:04
morganfainbergayoung, why18:04
*** hrybacki has joined #openstack-keystone18:04
ayoungthe current horizon code requires arbitrary length18:04
morganfainbergno it doesnt18:05
ayoungI can use one token to get another token, and do that forever18:05
morganfainberg*today* we revoke by expiration time18:05
morganfainbergwhich all the tokens are tied to18:05
ayoungmorganfainberg, not really,  as that code is not in use18:05
ayounghorizon does it like this:18:05
morganfainbergayoung, today we revoke by token id18:05
ayounglog in, get token (maybe scoped, depending)18:05
ayounguse that to get another tpoken scoped to project.  revoke first18:05
ayoungcontinue18:06
morganfainbergso you revoke by audit id there.18:06
morganfainbergthe local audit id18:06
morganfainbergif you want to revoke the whole chain, you'd still use (what today would be expires at) the audit_chain_id18:06
ayoungforget it, that use case will not work anyway18:06
ayoungthey will need to revoke explicitly the token they are using, while leaving the child token still valid18:06
ayoungbut  today there is no rule unscoped->scoped only18:07
ayoungso there is no way to revoke a whole chain18:07
ayoungmy approach was overkill18:07
ayoungbut you can never have too much overkill18:07
morganfainbergayoung, there is no rest api to revoke a whole chain.18:07
morganfainbergbut .. it would be easy to add a query param to do it (which this code now supports)18:08
ayoungmorganfainberg, and there is no rule that limits the length of a chain18:08
ayoungor to say "revoke fromthis point on forward"18:08
morganfainbergayoung, except that is an unbounded data set and is open for abuse that *can* and probably will affect keystone.18:08
morganfainbergayoung, we can't allow unbounded datasets in the tokens (really it is the problem with catalog)18:09
ayoungits not a huge deal,  I just think you are defining it too narrowly, but it is not something I'd throw over the review for18:09
morganfainbergi'm adding the b64url safe stuff in18:09
morganfainbergi'll add the change to b64urlsafe as part of the python3 fix i did for dstanek18:10
*** browne has quit IRC18:10
morganfainbergayoung, so it doesn't require massive rebase of the whole chain.18:10
ayoungif 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
morganfainbergsure, i'll add that verbiage in specifically. just that the max is 2 (actually we have a test for that as well)18:10
ayoungbut leave it such that code should expect a length >2 in the future and should be written to process that18:11
ayoungyour call, though.  I think you;ve thought this through18:11
morganfainbergayoung, "attribute is a list that contains either one or two elements." -> a list of no more than 218:12
morganfainberg?18:12
ayounghow about "no more than three"18:13
ayoungthe two we've thought of and wiggle room for one more18:13
ayoungplus with 3  you know that its going to be a foreach18:13
ayoung:)18:13
openstackgerritMorgan Fainberg proposed a change to openstack/identity-api: Add information about audit_id in token docs  https://review.openstack.org/11459018:13
*** PsionTheory has joined #openstack-keystone18:14
openstackgerritMorgan Fainberg proposed a change to openstack/identity-api: Update revoke-ext  https://review.openstack.org/11485718:17
dstanekmorganfainberg: any reason to even call out the length of the audit ids in the spec?18:19
morganfainbergdstanek, i just went with less than 32 characters so we know they are meant to be small18:19
morganfainbergdstanek, i don't disagree with ayoung there.18:19
morganfainbergor.. crap should be 32 characters or less.18:20
morganfainbergblah18:20
morganfainbergor would "short" be sufficient?18:20
*** harlowja has quit IRC18:20
*** harlowja_ has joined #openstack-keystone18:20
openstackgerritMorgan Fainberg proposed a change to openstack/identity-api: Add information about audit_id in token docs  https://review.openstack.org/11459018:21
morganfainbergdstanek, ayoung, ^ there "short urlsafe string"18:21
ayoungmorganfainberg, we had the issue with userid when we went to sha256 that they had code column lengths to 32 chars.18:22
openstackgerritMorgan Fainberg proposed a change to openstack/identity-api: Update revoke-ext  https://review.openstack.org/11485718:22
morganfainbergayoung, yeah i know :( had to fix them.18:23
ayoungI 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 rushiagr18:23
morganfainbergayoung, 64 for user_ids (and it's not in the spec afaik)18:23
ayounglooking...18:23
morganfainbergit's just that our schema was 6418:23
dstanekmorganfainberg: 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
morganfainbergyeah18:23
morganfainbergwe should update that to put an upper length.18:24
morganfainbergbut they all coded to uuid, - should we say 32 characters or less? or is "short" suffcient here18:24
ayoungI'm tempted to use 64chars across the board,  here as well as the rest of id18:24
ayoungDatabases are typically OK with anything under 25618:25
dstanekshort is good because if you specify something that doesn't need to be specified someone will code to it18:25
dstanekthen we break then when we need to change18:25
morganfainbergdstanek, ++18:25
ayounghere is the trade off18:25
ayoungif we do it a s a string, it is open ended, and a database olumn has to be "text"18:25
ayoungif we say "length <= 64"  then the db column will be 64 chards long, and it will waste space18:26
ayoungbut...that is a minimal amount of sapce, so I'm not too worried18:26
morganfainbergayoung, varchar is pretty efficient18:26
ayoungit won't change what goes across the wire18:26
ayoungmorganfainberg, yep  and varchar(64) is, I think, a good middleground18:26
ayoungids can always be shorter18:27
morganfainbergayoung, anywhere i see "short string" i'd code it as a varchar(255) worst case.18:27
ayoungwhy not formalize that?18:27
morganfainbergayoung, sure, probably should across all of keystone18:27
ayounganyway,  your spec looks good18:27
morganfainbergayoung, i'm happy to add in an upper bound here if we want :)18:28
ayoungyour call.  We can do it across all ID in one swoop instead of piecemealing it18:28
*** ukalifon1 has quit IRC18:28
openstackgerritMorgan Fainberg proposed a change to openstack/keystone: Convert to urlsafe base64 audit ids  https://review.openstack.org/11570718:29
morganfainbergayoung, yeah lets do it across the board if we're going to do that18:29
morganfainbergdstanek, ^ minor change + urlsafe fix for the py3 patch18:30
morganfainbergdstanek, instead of using codecs18:30
*** miqui has quit IRC18:31
ayoungmorganfainberg, I'll +A the spec if no objections?18:31
dstaneki lost dolphm's list of reviews from yesterday - anyone have the link handy?18:35
*** mrmoje has joined #openstack-keystone18:36
dstanekayoung: no objections from me18:39
dolphmdstanek: https://gist.github.com/dolph/651c6a1748f69637abd018:39
dstanekdolphm: thanks18:39
*** aix has quit IRC18:40
htrutahello, 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-client18:45
htrutaare you okay with that?18:45
htrutaafter that, we can also implement the same calls to hierarchical projects18:46
*** amcrn has quit IRC18:48
*** amirosh has quit IRC18:50
*** raildo has joined #openstack-keystone18:50
*** amirosh has joined #openstack-keystone18:50
openstackgerritA change was merged to openstack/identity-api: Add information about audit_id in token docs  https://review.openstack.org/11459018:51
*** mrmoje has quit IRC18:52
*** amirosh has quit IRC18:54
dolphmdstanek: i'm doing something dumb send help18:55
dolphmdstanek: 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
dolphmdstanek: 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 sense18:57
*** miqui has joined #openstack-keystone18:58
dolphmdstanek: 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-keystone19:02
openstackgerritayoung proposed a change to openstack/python-keystoneclient: Hash for PKIZ  https://review.openstack.org/11465419:03
ayoungbknudson, ^^ incorporates your test changes19:03
bknudsonayoung: thanks. I forgot about keystoneclient.19:04
ayoungbknudson, most people do...19:04
*** bknudson has left #openstack-keystone19:04
ayounghttps://review.openstack.org/#/c/106838/19:04
*** bknudson has joined #openstack-keystone19:04
ayoungthat is why Jamie has such a  long review queue19:05
*** chandankumar has quit IRC19:09
openstackgerritayoung proposed a change to openstack/python-keystoneclient: Add v3scopedsaml entry to the setup.cfg.  https://review.openstack.org/11077019:09
morganfainbergdstanek, 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
morganfainbergdolphm, ^ not dstanek19:09
dolphmmorganfainberg: i'm playing with staticmethod now -- it's not ideal but solves the use case i think19:10
*** rushiagr is now known as rushiagr_away19:10
dolphmthis 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 sad19:11
dolphmtopol: you're online!19:11
morganfainbergdolphm, this is where the descriptor stuff might work better19:11
topoldolphm, sorry yes I am19:11
morganfainbergtopol, classic mistake, responding to someone on IRC :P now you've been roped in!19:12
topolK19:12
topolI deserve it. I have been evil lately19:12
dolphmtopol: i'm been meaning to ask you this for a couple days19:12
dolphmtopol: but the pycadf Credential type19:13
dstanekdolphm: catching up19:13
topoldolphm you have my cell phone19:13
dolphmdstanek: i think morganfainberg got me sorted, but happy to have a miracle19:13
topoldolphm yes19:13
topolpycadf credential19:13
dolphmtopol: you asked for additional attributes on Credential if Credential.type is 'saml2', one of which was protocol19:13
dolphmtopol: won't Credential.type == Credential.protocol in that case, always?19:14
topoldolphm  wanted to be able to add extra key value pairs19:14
dolphmtopol: yes, but specific ones19:14
topolyes19:15
topolthe ones you put in the schema validation19:15
dolphmtopol: so won't Credential.type == Credential.protocol in that case, always?19:15
dolphm'saml2' and 'saml2' ?19:15
topolagreed. I think so19:15
openstackgerrithenry-nash proposed a change to openstack/keystone: Implements the controller for the endpoint policy extension  https://review.openstack.org/11574619:15
topolbut there were the other two.19:16
dolphmtopol: that's a red flag that we're doing something wrong to me :)19:16
dstanekdolphm: did you decide to go with a classmethod then?19:16
dolphmtopol: are there any other values of 'type' that you're aware of?19:16
topolhow so19:16
topolthats a fed identiy mapping question. need stevebot for that19:16
dolphmtopol: 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 here19:16
dolphmstevemar: ^19:16
stevemaro/19:17
dolphmtopol: saml2 is just one type of federated credential... should type='federated'?19:17
stevemarcatching up...19:17
dolphmtopol: and protocol='saml2'?19:17
topolfrom the credential point of view it should let you add extra key value pairs. saml2 may be a special case one19:17
openstackgerrithenry-nash proposed a change to openstack/keystone: Implements the controller for the endpoint policy extension  https://review.openstack.org/11574619:17
topoleventually they would have other protocols19:17
dolphmtopol: prior to this work, i'm not clear on what values 'type' could have19:17
topollet me go look at the code19:18
stevemardolphm, wouldn't 'type' have been password or token?19:18
dolphmstevemar: that's what i'd like to know as well19:18
stevemarshouldn't we have a type='federated' for federated auth19:19
stevemarand then protocol = saml/oidc19:19
stevemaras an additional field19:19
openstackgerrithenry-nash proposed a change to openstack/keystone: Implements the controller for the endpoint policy extension  https://review.openstack.org/11574619:19
stevemardstanek, i'm going to bug you to review the notifications for role_assignments again19:19
dstanekstevemar: link?19:19
stevemardstanek, https://review.openstack.org/#/c/112204/19:20
topolstevemar +++19:20
stevemardstanek; gordc likes it, and i was hoping to sneak this in so topol could leverage some of refactoring i did19:20
dolphmtopol: stevemar: in one of the examples, 'type' is a URL: "type": "https://mycloud.com/v2/token"19:20
stevemardolphm, that means sausage to me19:21
dstaneki think gerrit punishes you for doing too many reviews with an onslaught of email19:21
dolphmstevemar: me too19:21
stevemardstanek, that sounds familiar / appropriate19:21
henrynashdstanek: know your busy, but I responded to your comments on the endpoint policy API19:21
stevemari need to review some more, get my karma up and things moving, so other can review my stuff :D19:22
dstanekhenrynash: thanks. i'll take a look at that next. i'm interested to see in anyone else feels the same19:22
henrynashdtsanek: 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 way19:22
dstanekstevemar: yes! no rebase! i can actually see the diff between 25..current19:23
topoldolphm, stevemar, so looking at the code the interesting values are group_ids, identity_provider, and protocol19:23
dolphmtopol: agree. so what's a useful 'type'?19:23
topoland if I dont add them as extra key value pairs we dont capture this info in the audit record19:23
henrynashdstanek: 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 association19:23
topoldolphm what do you mean by type?19:24
dolphmtopol: Credentials have an existing 'type' attribute19:24
dolphmtopol: 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
topoldolphm so why isnt that saml2?19:24
dolphmtopol: but i don't know, because i don't know what 'type' is supposed to represent, beyond the apparently conflicting examples19:25
dstanekhenrynash: it feels unnatural to me that putting /x/y/a would effectively delete (and conceptually tied to) /x/y/z19:25
dolphmtopol: because 'saml2' is the federation protocol19:25
topoldolphm oh19:25
dolphmtopol: at least, it is in keystone land19:25
henrynashdstanek: delete?19:26
dolphmtopol: 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 it19:26
henrynashdstanek: oh, you mean the overwriting?19:26
dstanekhenrynash: yeah, multiple resources are sort of tied together19:26
henrynashdstanek: not quite sure I see that….19:27
henrynashdstanek: when you say “tied together”, you mean the act of putting /x/y/z is tying those together?19:28
dstanekhenrynash: 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 change19:29
dstanekhenrynash: putting to one URL effectively deletes the other resource19:29
topolso dolphm, from the spec:19:29
topolType of credential. (e.g., auth. token, identity token, etc.)19:29
dolphmtopol: and the two examples i can find, one doesn't have a type at all, and one has a URL19:30
topolNote: Profiles of this specification MAY define URIs for their credential types.19:30
topolmy guess is the URL one is actually a URI that is more formally defining the type. but thats optional19:31
topoland accosing to the spec type is not required19:31
topolso both naswers are ok according to the spec, dolphm19:31
topolthe key is for it to be easy to set that attribute as a string19:32
henrynashdstanek: overrites any previous association…right…19:32
topoldolphm, make sense?19:32
dolphmtopol: so a URL is a string, so that's all we need to validate it as... if it exists.19:32
topolyes, just that it is a string19:33
dolphmtopol: so wtf is the intended difference between "auth", "token", and "identity token" ??19:33
topoldolphm he was just coming up with some potential examples to illustrate there are lots of types of credentials19:34
topolI believe he was trying to match keystone like things but if it confused you its porbably a poor lis of examples19:34
dstanekhenrynash: 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 forever19:34
topoldolphm, 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 supported19:35
*** amcrn has joined #openstack-keystone19:36
topoldolphm 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
topoland maybe some of those dont apply to keystone but you get the gist of it19:37
henrynashdstanek:…and I fine thingto bring up sire19:38
dolphmtopol: 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
topolfor keystone, I think we just used token19:38
richmdstanek: ping - re: https://review.openstack.org/74897 - how would I put test_deleteTree in its own test class?19:39
topolas the type19:39
dolphmtopol: so, does type='federated' or something make sense?19:39
henrynashdolphm: 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
dolphmhenrynash: i'll try; i'm trying to solve the pycadf use case first so topol's bp makes fpf19:40
henrynashdolphm: ok, np…yep, get that done first...19:40
dolphmhenrynash: undefined API is probably my second highest priority though :)19:40
topoldolphm, I think federated makes sense19:41
topolit really captures the typo of token we are dealing with here19:41
topolwe need to get Matt Rutkowski to verify that19:42
topolThe spec says it should be xs:anyURI19:43
*** gyee_ has joined #openstack-keystone19:44
*** dims has quit IRC19:45
dstanekstevemar: 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. group19:45
*** dims has joined #openstack-keystone19:45
topoldolphm, so as I interpret the spec the value should be a URI. Any URI at all19:46
stevemardstanek, hmm... no i guess not19:46
stevemarit would just be a uuid19:46
stevemarso the person reading it wouldn't know if it's a user or group :(19:47
topoland we would come up with a name like http://http://openstack.org/v3/federated-token19:47
topoldolphm does that make sense19:47
topol?19:47
topoldolphm oops , I meant http://openstack.org/v3/federated-token19:47
dstanekstevemar: i don't know much about the CADF stuff, but that sounds like it could be an issue19:49
stevemardstanek, yeah, hmmm19:49
topoldstanek, stevemar we should get matt rutkowski for these questions19:50
stevemardstanek, add actor_type? rename actor to user (blah)19:50
raildohenrynash: 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
dstanekrichm: i'll take a look at how to make that test more of a unit test in a little bit and get back to you19:50
*** dims has quit IRC19:51
henrynashraildo: so I don’t think you need another spec…but you should add a blueprint aimed at tehcleint19:51
*** dims has joined #openstack-keystone19:51
dstanektopol: how do we do that?19:51
dstanektopol: can you invite him in here?19:51
raildohenrynash: ok, thanks19:52
topoldstanek, yes, Im getting him to join now. he was rebooting his machine19:53
*** zzzeek has quit IRC19:53
*** mrutkows has joined #openstack-keystone19:54
mrutkowshello19:54
topoldstanek, dolphm, FIRE AWAY!!!19:54
mrutkowsHey all, here to clarify any CADF format issues19:54
openstackgerrithenry-nash proposed a change to openstack/keystone: Add delete notifications for policy, region and service.  https://review.openstack.org/11576319:55
topolmrutkows, so what value should credential type be? The spec says xs:anyuri19:56
mrutkowscredential 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 locally19:56
topoldolphm:19:57
mrutkowsits for when (or if) it is federated that others that receive the data know what type of credential was used19:57
topoldstanek what is your questionm19:57
dstanektopol,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 tell19:58
dstanekfeels like it will eventually cause an auditor pain at some point19:58
*** harlowja_ is now known as harlowja_away19:58
morganfainbergdstanek, 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
topolmrutkows, so for validating the type value dolph dstanek, need more context. which part of the audit structure?19:59
*** zzzeek has joined #openstack-keystone19:59
topoldstanek, need more context19:59
topolshow the example CADF record19:59
topolor field19:59
dolphmtopol: dstanek: mrutkows: of course, i went AFK earlier, but catching up now...20:00
mrutkowsif we have multiple UUIDs each with different identity context, we can add them where needed20:00
dstanektopol: all i know right now is line 250 of https://review.openstack.org/#/c/112204/28/keystone/notifications.py20:00
dolphmmrutkows: so if it just boils down to an aribtrary string, what are the well known values of "type", if any?20:01
dstanektopol: i noticed it because of line 507 on https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py20:02
mrutkowsdstanek: 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
dstanektopol, mrutkows: so i was not sure if there was a way to mark the actor as user or group20:04
dstanekmrutkows: yeah, this came up in a review of some code we are trying to get in20:04
henrynashstevemar: docs…ooh..where do we document which events get notifcations20:05
henrynash?20:05
topolhttps://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py20:05
*** dims has quit IRC20:05
mrutkowsdstanek: if we have the ability to distinguish userid from groupid, etc.  then we can clearly put them in a correct keyname20:05
*** dims has joined #openstack-keystone20:06
topoldstanek, so it looks like stevemar wrote this but the values look incorrect20:07
*** dims_ has joined #openstack-keystone20:07
*** dims has quit IRC20:07
topolfolks, let's try and close out dolphm's question first20:07
topoldolphm you still there?20:08
*** marcoemorais has quit IRC20:08
dstanekmrutkows: are the fields on Resource arbitrary? or does actor mean something special?20:08
*** stevemar has quit IRC20:08
*** marcoemorais has joined #openstack-keystone20:08
mrutkowsactor is a non-normative field someone added20:08
mrutkowsnot part of the spec.20:08
mrutkowsSteve seems to have added this as some attachment20:09
henrynashstevemar: sorry, found it20:09
mrutkowsdstanek: 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 profile20:10
mrutkowsdstanak: the idea is that everyone agrees what fields are needed to audit and add them20:11
topoldstanek20:11
*** marcoemorais has quit IRC20:11
mrutkowsdstanak: then we can publish them back to the OpenStack profile so anyone receiving CADF events know what they are20:12
topoldstanek20:12
*** marcoemorais has joined #openstack-keystone20:12
mrutkowssorry dstanek20:12
*** marcoemorais has quit IRC20:12
dolphmtopol: yep20:12
*** marcoemorais has joined #openstack-keystone20:12
openstackgerritThiago Paiva Brito proposed a change to openstack/python-keystoneclient: Implementing hierarchical calls on keystoneclient v3 (python only)  https://review.openstack.org/11577020:12
dstanekmrutkows: 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
dstanekmrutkows: same goes for actor that could represent either a user or a group20:13
mrutkowsdstanek: keystone devs should decide what fields are important to add, add them (validate them) and we can add them to the CADF spec./OpenStack profile20:13
dstanekmrutkows: 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 IRC20:14
*** dims has joined #openstack-keystone20:15
mrutkowsdstanek: 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 nromative20:15
*** radez is now known as radez_g0n320:15
mrutkowsdstanek: if there is additional context, such as group, project etc then these can be added as new key-value pairs20:15
dstanekmrutkows: 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
mrutkowsdstanek: 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
mrutkowsdstanek: and let me know so I can publish into the OpenStack spec20:18
mrutkowsdstanek: hehe20:18
*** dims has quit IRC20:19
mrutkowsdstanek: to better answer (project or domain), I would need to understand the activity/action/api we were talking about20:19
topolmrutkows, 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
mrutkowsdstanek: typically we want to name the target's typeURI (if its data held be a service) as relative to that service20:20
dstanektopol, 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 to20:21
dolphmmrutkows: 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.py20:21
dstanekmrutkows: so the API in question is creating and deleting grants - https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py20:21
dolphmdstanek: i've only been skimming the rest of the above... but where is an entire group being supplied as the actor?20:22
dolphmthat sort of seems to defeat the purpose of 'actor' :-/20:22
dstanekdolphm: lines 350 and 351 of https://review.openstack.org/#/c/112204/28/keystone/tests/test_notifications.py are what i was concern with20:23
dolphmoh for assignment notifications, gotcha20:23
dstanekdolphm: wrong link...correct one is https://review.openstack.org/#/c/112204/28/keystone/notifications.py20:23
dolphmactor should be the API user, not the subject of the assignment20:23
dstanekso target is what is being granted (either the project or domain) and actor is who it's granted to (user or group)20:25
topoldstanek, we need to code review stevemars code20:25
topolhe is new to cadf and I dont think its the way it needs to be20:26
*** dims has joined #openstack-keystone20:26
mrutkowsdstanek: 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 Steve20:27
dolphmdstanek: and where is the API user going?20:27
dstanekdolphm: i think that's the same as it was in the host object20:28
dolphmdstanek: but the host is just an address?20:28
dolphmdstanek: multiple users could be coming from the same host20:28
dstanekdolphm: oh, sorry. the resource name http://git.openstack.org/cgit/openstack/keystone/tree/keystone/notifications.py#n25420:29
dstanekmrutkows: here is what i was thinking as potential options http://paste.openstack.org/show/97912/20:29
ayoungrichm, have you worked through the steps necessary to get cinder  to authenticate tokens with Keystone over https?20:30
*** amerine has joined #openstack-keystone20:32
dolphmdstanek: is there any justification against the specialized key approach?20:32
dstanekdolphm: not that i know of - that's probably the one i would opt for20:33
dolphm120:33
dolphmdstanek: then ++!20:33
*** jamielennox is now known as jamielennox|away20:33
mrutkowsdstanek: 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 structure20:34
mrutkowsdstanek: 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 field20:35
*** PsionTheory has quit IRC20:37
mrutkowsdstanek: 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 provide20:37
mrutkowsdstanek: stevemar is offline or else i would grab a phone and ask him about all these fields20:38
dstanekmrutkows: he had to take off so i told him i would make any change we decide on20:38
topoldolphm, your code and test case look spot on to me20:38
dolphmtopol: note that it's not exactly what you asked for20:40
dolphmtopol: but i think it solves the use case20:40
dstanekmrutkows: i'm not sure what the typeURI actually means20:40
*** henrynash has quit IRC20:40
dolphmdstanek: i think that's "service type"20:40
dolphmdstanek: it's not actually a URI20:40
topoldolphm, how is it different???20:41
dolphmtopol: i'm not letting you set Credential.type!20:42
dstanekmrutkows: 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 group20:42
richmayoung: no20:42
dstanekmrutkows: where is the current openstack spec so i can take a quick look at it?20:42
dolphmtopol: 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 conversation20:43
topoldolphm, OK, I see that now.20:44
topoldolphm so talking to matt he feels type should describe the format of the credential20:46
*** gyee_ has quit IRC20:46
topoldolphm most accurately saml2  more accurately describes the credential20:47
dolphmtopol: describe the pycadf credential object, or the actual credential being audited20:47
dolphm?20:47
mrutkowsdolphm: 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 parsed20:49
*** gyee_ has joined #openstack-keystone20:50
*** mrmoje has joined #openstack-keystone20:50
mrutkowsdolphm: or even if the token is not attached, where the fields came from20:50
mrutkowsdolphm: is the token (or data described in the credential object) from SAML, from Oauth, from etc.20:50
dolphmtopol: what are you putting into the "token" attribute?20:51
dolphmtopol: the entire saml doc?20:51
topoldolphm, if one day we add saml3 as a type I cant change this. Im stuck20:51
topoladolphm and when I say saml2 and saml3 I really mean more globally context names20:51
mrutkowsdolphm: either use the OASIS assigned URI for SAML 2 or create an openstack value for SAML 2 that we can publish20:52
dolphmtopol: can you answer my question above?20:53
*** marcoemorais has quit IRC20:53
topoldolphn, letting mrutkows try and answer it20:53
*** marcoemorais has joined #openstack-keystone20:53
topolmrutkow type should reflect this is saml2 "stuff"20:53
dolphmtopol: i'm asking you the question because i'm asking about your keystone patch20:53
topolerr dolphm type should reflect this is saml2 stuff not just that its federated20:53
topoldolphm trying to answer20:54
mrutkowsdolphm: SAML defines a "format" value for each spec.20:55
topoldolphm 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 saml220:55
mrutkowsdolphm: the core spec is http://docs.oasis-open.org/security/saml/v2.020:56
mrutkowsdolphm: i would use this to identify the SAML 2 standard generically which should be sufficient, we wont get into protocol varianbces20:58
mrutkowsvariances20:58
topoldolphm so the value for type should be http://docs.oasis-open.org/security/saml/v2.020:58
topoldolphm when someone sees that they know its federated identity using saml v2 assertions20:59
*** henrynash has joined #openstack-keystone20:59
dolphmtopol: 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
topoland dolphm I should be able to set that, dolphm why do you want to hard code it and lock me in?21:00
dolphmtopol: that's the question i'm trying to answer. we have two poorly defined attributes in the proposed notification at this point21:01
mrutkowsdolphm: protocol was an extra field someone added, steve added this perhaps?21:01
*** amcrn has quit IRC21:01
dolphmholy fucking shit read the damned review21:02
topoldolphm so are you trying to convice me protocol should really be http://docs.oasis-open.org/security/saml/v2.021:02
mrutkowsdolphm: 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:assertion21:03
dolphmi 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 attribute21:03
topoldolphm , I have read it21:03
topoldolphm yes, I know that is where you were taking me21:03
mrutkowsdolphm: 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-keystone21:04
mrutkowsdolphm: you could add a key urn:oasis:names:tc:SAML:2.0:protocol... not sure what steve intended21:04
*** jasondotstar has joined #openstack-keystone21:05
topoldolphm, 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 changed21:05
*** raildo has left #openstack-keystone21:05
dolphmtopol: because that's what we defined to be the conventional protocol ID for saml2 use cases with shibboleth21:05
topolor its just an openstack keystone code convention thats just harmless21:05
dolphmit'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.md21:06
mrutkowsdolphm: we can simply use SAML2 and document that is the value openstack/keystone uses for the SAML 2.0 spec.21:07
topoldolphm 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 auditing21:08
topolmrutkows is wrong21:08
topoldolphm when you send the auditing record we want to be more precise cause its goes out the world in an audit record21:09
*** gokrokve has quit IRC21:09
dolphmtopol: 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-keystone21:10
dolphmtopol: 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 notifications21:10
topoldolphm, it helpos to have it because when you look at the mapping stuff that is what is used21:10
dolphmtests too21:10
mrutkowsdolphm: i care21:11
topoldolphm you have a label for the mapping stuff and then you have a spec that desires a more formal name21:11
dolphmmrutkows: there's a lot of unsubstantiated "should"s flying around21:12
topoldolphm so Im frustrating you because we have something that has two different meanings although simillar21:12
topoldolphm if someone was looking at the audit record it would help them to see saml2 cause that is what is used in the mapping code21:12
dolphmtopol: i'm frustrated because i'm asking the same basic questions over and over, and i have yet to get a straight answer21:13
mrutkowsdolphm: 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
dolphmtopol: 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
topoldolphm I misundersood your question because to the two different contexts21:13
topoldolphm can I call you?21:13
topolwill be much easier21:13
dolphmtopol: i'd rather keep this in public / on record21:14
dolphmtopol: i put up the code review because that's obviously the best place to iterate https://review.openstack.org/21:14
dolphmwhoops: https://review.openstack.org/#/c/115771/21:14
topolOk. 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 thing21:15
topolwhen you see saml2 in keystone it means use the saml2 mapping stuff. agre or disagree?21:15
*** marcoemorais has quit IRC21:16
*** marcoemorais has joined #openstack-keystone21:16
*** henrynash has quit IRC21:17
*** harlowja_away is now known as harlowja_21:17
dolphmtopol: that's about half the definition i would use for icehouse, i suppose21:17
topoldolphm, when I see saml2 its like the following from the markdown:21:17
topolprotocol": {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
dolphmtopol: 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 juno21:18
topoldolphm agree or disagree?21:18
dolphmtopol: agree that's half of what a protocol "is" to keystone21:18
topoldolphm whats the other half?21:18
dolphmtopol: 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
topoland dolphm would you agree that saml2 was a name someone just picked cause the knew they were doing saml2?21:19
dolphmtopol: 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 juno21:19
dolphmtopol: yes21:19
topoland dolphm and would you agree that in shib they add saml2 in the config as well?21:20
dolphmtopol: 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-configurable21:20
dolphmtopol: yes21:20
topolso 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 saml221:21
topoldolphm maybe in shib we explicitly say hey its saml221:22
mrutkowsdolphm: the patch for the IDP, user and groups does belong as you have coded it into the credential21:22
mrutkowsdolphm: thanks21:22
dolphmmrutkows: that qualifies as a straightforward answer - THANK YOU21:23
dolphmtopol: 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
topolso 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.021:24
topolbut dolphm, I CHEATED21:24
dolphmtopol: like so? https://review.openstack.org/#/c/115771/2/pycadf/tests/test_cadf_spec.py21:24
topoldolphm how can I know that saml2 really meant  http://docs.oasis-open.org/security/saml/v2.021:25
topolstay with me dolphm21:25
topoldolphm, I cant21:25
dolphmtopol: in the context of keystone?21:25
topoldolphm, that in kesytone when we see saml2 it means  http://docs.oasis-open.org/security/saml/v2.021:26
topoldolphm how can I know that???21:26
dolphmtopol: correct21:27
dolphmtopol: it's a federated audit notification, not a saml2 audit notification21:27
topoldolphm I cant .  so really all i can do is say the protocol is saml221:27
topolall I know is the id the user picked21:27
dolphmtopol: so, shall i let you and mrutkows work out how we rectify the impossible with the wrong?21:28
topoldolphm, one more step and Im done21:28
dolphmtopol: oh, i assumed "impossible problem" must be the the last step :)21:28
topolso it should be simply saml2 but the spec would like to prefix it and make it  http://openstack.org/identity_provider/saml221:29
topolbecause the spec wants and any:URI21:29
topolcause when federated its now clear this value came from openstack keystone federation identity provider21:30
topolthat way I satisfy both masters21:30
topolsaml2 is accurate cause that was defined in keystone21:30
dolphmtopol: you might be able to get something useful out of the environment variables from shibboleth (like AuthnContextClassRef)21:31
topoland http://openstack.org/identity_provider/saml2 is the CADF spec compliant version of that21:31
dolphmtopol: https://wiki.shibboleth.net/confluence/display/SHIB2/NativeSPAttributeAccess#NativeSPAttributeAccess-EnvironmentVariables21:31
*** zzzeek has quit IRC21:31
dolphmtopol: where did http://openstack.org/identity_provider/saml2 come from?21:32
openstackgerritDavid Stanek proposed a change to openstack/keystone: Add CADF notifications for role assignment create and delete  https://review.openstack.org/11220421:32
topoldolphm , perhaps, but I would think  http://openstack.org/identity_provider/saml2 enables someone audting to go check and determine which user chosen idp was sued21:32
dolphmtopol: but that's what the identity_provider attribute of the audit notification provides21:33
dolphmtopol: unless you, again, want to go back to shibboleth and pull Shib-Identity-Provider for that21:33
topolso 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
dolphmtopol: and, again, drop the keystone defined bits on the floor21:34
*** dims has quit IRC21:34
topoljust to have all bases covered21:34
openstackgerritA change was merged to openstack/keystone-specs: Add deprecation tasks to auth-specific-data  https://review.openstack.org/11542421:34
topolwhich bits did I drop?21:34
dolphmtopol: the "extra" one?21:34
*** dims has joined #openstack-keystone21:34
topolextra is the provider that youjust added21:34
topoldolphm you feel its stupid to have both?21:35
dolphmtopol: well, this is reiterating my notion of shoveling a bunch of crap onto the message bus and calling it auditing21:35
topoldolphm the auditng allows extra stuff to be added. most of what is in the record is normative21:36
*** henrynash has joined #openstack-keystone21:36
morganfainbergdolphm, when you're done chatting w/ topol have an audit_id question for you21:36
topoldolphm if you feel strongly we can drop provider and just keep the long name in type21:36
*** zzzeek has joined #openstack-keystone21:36
topoldolphm 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 audting21:37
dolphmtopol: 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, anywhere21:37
dolphmmorganfainberg: ack21:38
mrutkowsdolphm: \o/ YES YOU GET THE INTENT21:38
morganfainbergmrutkows, dolphm, we're not delivering on that? that was my understanding of pycadg21:38
morganfainbergpycadf21:38
dolphmmrutkows: \o/ YAY21:38
morganfainbergor is it we're just bad at consuming pycadf21:38
dstanektopol, mrutkows: my best guess as to what we should report on for grants - https://review.openstack.org/#/c/112204/28..29/keystone/notifications.py21:38
topoldolphm, I promise to never to be kind and add extra info out of kindness again21:38
*** dims has quit IRC21:39
dolphmmorganfainberg: 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
topolso dolphm some extras are valuable because of the context21:39
topoldolphm so should we not add the groupids???21:40
mrutkowsdolphm: clearly add things that are of value to auditors, especially security auditors, like groups and things that establish identity (or access)21:40
mrutkowsdolphm: but no more21:40
dstanekdoes the spec for this call our which extra metadata must be included under which contexts?21:41
dstaneks/our/out/21:41
dolphmtopol: i'm not referring to group_ids specifically here21:41
topoldolphm, dstanek, there are only a few spots where the cadf allows some extensions. credential was one of them21:42
dolphmmrutkows: 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
topolI 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 oversight21:43
nkindermorganfainberg: you around?21:43
morganfainbergnkinder, sure am21:43
nkindermorganfainberg: The gate job is failing for merging one of my patches, and I'm not sure what to file the bug against21:44
nkindermorganfainberg: http://logs.openstack.org/08/104408/5/gate/gate-tempest-dsvm-full/f3081d0/console.html21:44
mrutkowsdstanek: 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
nkindermorganfainberg: a bunch of the volume tests fail with timeouts21:44
nkindermorganfainberg: 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=ERROR21:44
mrutkowsdstanek: however, i am relying on the great minds in keystone to decide what other fields have value for openstack21:44
mrutkowsdstanek: and we can publish these fields in the profile21:45
morganfainbergnkinder, thats a new one21:45
morganfainbergi've not seen that one before21:45
nkindermorganfainberg: I didn't see any recheck bugs that seem to match, so I was looking to file a new one21:45
nkinderis cinder the right place to start, or should I file this against something else?21:45
morganfainbergnkinder, this looks like cinder related.21:46
nkindermorganfainberg: cinder it is then21:46
morganfainbergnkinder, also recheck <bug####> isn't the norm anymore, file a bug, file a review to add to elasticrecheck, just use "recheck" iirc21:46
morganfainbergnkinder, at least thats what i kindof gleaned from the -infra chatter21:47
dolphmmrutkows: i figure i'd capture that https://review.openstack.org/#/c/115796/1/README.rst21:47
nkindermorganfainberg: this has received all reviews, so it's reverify, right?21:47
morganfainbergnkinder, not that it wont work if you recheck bug ###21:47
morganfainbergnkinder, recheck/reverify does the same thing, it'll re-run the check21:47
nkindermorganfainberg: ok, the wiki disagrees, but I'll try it21:47
morganfainbergnkinder, ah maybe they haven't announced that stuff yet21:48
morganfainbergsometimes i mix up IRC chatter/capabilities merged and official stance21:48
morganfainberg / announcements21:48
*** dolphm changes topic to "{"feature_freeze": "september 4"}"21:49
morganfainbergnkinder, it's hard to keep it all 100% straight when you lurk in ~15 channels21:49
nkindermorganfainberg: tell me about it... :)21:49
morganfainbergnkinder, and i don't even have the internal RH IRC to contend with21:49
nkindermy xchat tabs scroll off the right side of my screen, and I use small fonts21:50
morganfainbergnkinder, lol21:53
dolphmmorganfainberg: how about that audit question, or can i help with nkinder's recheckage21:54
morganfainbergdolphm, do we want compatibility code for old token pre-audit ids?21:55
morganfainbergdolphm, i have that code in now. but if we want to enforce "no audit id no auth" i can pull it out21:55
dolphmmorganfainberg: uhh, where? in keystone?21:56
morganfainbergdolphm, yeah in keystone21:56
dolphmmorganfainberg: is it on gerrit?21:56
morganfainbergdolphm, it would affect keystone rest apis and validated tokens.21:56
morganfainbergdolphm, 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 value21:57
morganfainbergdolphm, earlier in the chain21:57
morganfainbergdolphm, let me dig up specifics.21:57
morganfainbergdolphm, example https://review.openstack.org/#/c/114306/9/keystone/auth/plugins/token.py the top change in that file21:58
morganfainbergand https://review.openstack.org/#/c/114306/9/keystone/token/controllers.py line 260ish21:58
morganfainbergdolphm, we can slate that compat code to be removed in K as well if we want to maintain live tokens across the upgrade21:59
*** harlowja_ has quit IRC21:59
dolphmmorganfainberg: so, basically your compatibility is a fail safe?22:00
topoldolphm, so https://review.openstack.org/#/c/115771/1..2/pycadf/credential.py looks good to me.22:00
openstackgerritA change was merged to openstack/python-keystoneclient: Remove lxml as a forced depend  https://review.openstack.org/11470122:00
morganfainbergdolphm, yeah, basically we allow tokens that were issued prior to the upgrade with audit ids vs. rejecting them22:00
openstackgerritA change was merged to openstack/python-keystoneclient: Allow passing user_id to v2Password plugin  https://review.openstack.org/11371222:01
openstackgerritA change was merged to openstack/python-keystoneclient: Fix handling of deprecated opts in CLI  https://review.openstack.org/11385922:01
morganfainbergdolphm, but all new tokens get audit_ids and use them22:01
*** harlowja has joined #openstack-keystone22:02
*** jorge_munoz has left #openstack-keystone22:02
dolphmmorganfainberg: 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
dolphmtruncate*22:05
morganfainbergdolphm, for UUID truncate, for PKI, issue a revocation for all tokens and/or a revocation event for issued_before = now22:06
morganfainbergdolphm, (don't think we have that ability on the rev. events)22:06
*** dims has joined #openstack-keystone22:06
morganfainbergbut 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 anyone22:07
openstackgerritDolph Mathews proposed a change to openstack/python-keystoneclient: Remove cruft from setup.cfg  https://review.openstack.org/11470822:07
openstackgerritDolph Mathews proposed a change to openstack/python-keystoneclient: Unsort pbr and hacking in requirements files  https://review.openstack.org/11470722:07
nkindermorganfainberg, 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 IRC22:09
openstackgerritDolph Mathews proposed a change to openstack/python-keystoneclient: Remove cruft from setup.cfg  https://review.openstack.org/11470822:09
openstackgerritDolph Mathews proposed a change to openstack/python-keystoneclient: Unsort pbr and hacking in requirements files  https://review.openstack.org/11470722:09
morganfainbergnkinder, it looks like it's already in the recheck queue22:09
nkinderhuh, guess it doesn't add a comment in gerrit22:10
morganfainbergnkinder, about 37 minutes ago22:10
dolphmjamielennox|away: bknudson: trivially rebased this, if one of you wants to +A again https://review.openstack.org/#/c/114707/22:10
nkinderI expected a "Starting gate jobs" comment22:10
*** mrmoje has quit IRC22:11
morganfainbergnkinder, you may need to either rebase/change commit msg or wait for it to fall out of the check queue22:11
dolphmnkinder: those have been suppressed due to the noise they were generating22:11
morganfainbergdolphm, ah22:11
bknudsonI was wondering if they had gotten rid of those messages.22:11
dolphmjenkins comments aren't published in the UI anymore22:11
dolphmnor any CI, i don't think22:11
nkinderdolphm: ok (even though it's hidden in the toggle CI stuff)22:11
morganfainberghonestly i kindof liked the -2 going to a score of 022:11
morganfainbergwhen you recheck/reverify22:12
morganfainbergbut i can see why the extra noise is bad22:12
dolphmmorganfainberg: that still happens, there's just not record of it in the UI :-/22:12
morganfainbergdolphm, ah it's not happening on this review22:12
dolphmmorganfainberg: the vote should be reset when you recheck - it's not?22:12
morganfainbergdolphm, i think the review got into check queue before it was -2 scored, so now it's running check again22:12
dolphmmorganfainberg: https://review.openstack.org/#/c/104408/ ?22:12
morganfainbergdolphm, not on nkinder's22:12
bknudsondid they finally get rid of reverify?22:12
morganfainbergdolphm, yeah22:13
dolphmbknudson: kinda sorta yes22:13
morganfainbergdolphm, that is *in* the check queue22:13
morganfainbergdolphm, but it didn't clear the score (it landed in check queue about 37 mins ago)22:13
morganfainbergwhich is why it didn't respond to nkinder's or my 'recheck' comments22:13
morganfainbergs/37/25/22:14
dolphmmorganfainberg: nkinder: well that's weird. hopefully it doesn't vanish when it's done with the check22:14
morganfainbergdolphm, it should get a +1 or move to gate (depending on things)22:14
morganfainbergif it stops at +1 it's easy to rekick, just bug me if i don't catch it22:15
dolphmmorganfainberg: ++ same here nkinder22:15
dolphmnkinder: 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
morganfainbergdstanek, woth upgrading https://review.openstack.org/#/c/114306/ to +2 since the py3k + urlsafe stuff passed check22:16
morganfainbergdstanek, ?22:16
dstanekmorganfainberg: sure - any objections to a +A at this point?22:19
morganfainbergdstanek, not from me22:19
openstackgerritDolph Mathews proposed a change to openstack/keystone: move GET /v3/catalog to GET /v3/auth/catalog  https://review.openstack.org/10804322:21
*** jamielennox|away is now known as jamielennox22:22
dstanekmorganfainberg: looking back at the history bknudson had  quite a few comments - might be wise to have him give it a quick review22:23
morganfainbergdstanek, works for me22:23
bknudsonwhich?22:24
morganfainbergbknudson, https://review.openstack.org/#/c/114306/ audit_id adding22:24
*** jasondotstar has quit IRC22:24
bknudsonmorganfainberg: I thought you were going to do another rev22:24
bknudsontrailing whitespace in the commit message!22:25
morganfainbergbknudson, ...22:25
jamielennoxdolphm: i didn't use your move /catalog patch. just moved it here: https://review.openstack.org/#/c/114903/22:25
morganfainbergstupid gerrit web interface22:25
dolphmjamielennox: i'm just noticing that22:25
dolphmjamielennox: why not?22:25
*** nkinder has quit IRC22:26
morganfainbergbknudson, let me know if you see anything else needed before i just fix that (i guess)22:26
morganfainbergbknudson, :)22:26
bknudsonmorganfainberg: I still need to be convinced as to what's supposed to happen if the original token doesn't have audit_id22:26
bknudsonI don't want another CVE22:27
jamielennoxdolphm: no reason, i should have - but i started working on the other stuff without finding that patch and just moved it myself22:27
jamielennoxdolphm: can rebase ontop if you like, it'll be more or less exactly the same22:27
bknudsonmorganfainberg: is the revocation event going to have the expires_at instead?22:27
morganfainbergbknudson, it's strictly compatibility, in the patch that does the actual revocation events, it will revoke_by_expires if the audit_id doesn't exist22:27
morganfainbergbknudson, https://review.openstack.org/#/c/114864/8/keystone/token/provider.py22:27
dolphmjamielennox: you must not have tried very hard lol22:28
jamielennoxdolphm: nope, didn't even think about it at the time22:28
*** david-lyle has quit IRC22:29
bknudsonmorganfainberg: what if it's a child token of a token that didn't have an audit_id?22:29
*** david-lyle has joined #openstack-keystone22:29
dolphmjamielennox: 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
bknudsonmorganfainberg: the child would only have audit_id and not the audit_chain_id22:29
*** jasondotstar has joined #openstack-keystone22:29
morganfainbergbknudson, correct22:30
morganfainbergbknudson, it would be treated as the start of a chain.22:30
morganfainbergbknudson, i was actually just checking that before answering :)22:30
jamielennoxdolphm: it doesn't return for  both? it did...22:31
bknudsonmorganfainberg: so you revoke the parent token and the child would be revoked because of expires_at?22:31
morganfainbergbknudson, yes.22:31
*** marcoemorais has quit IRC22:31
dolphmjamielennox: it won't return anything, but based on the method signature alone, i have no idea what the method is supposed to do22:32
bknudsonmorganfainberg: ok... I can't think of any problems.22:32
*** marcoemorais has joined #openstack-keystone22:32
morganfainbergbknudson, if we need to smash the whitespace in the commit msg (pep8 obviously doesn't care) i'll do that now.22:32
morganfainbergbknudson, aparently it's easy to get that in there from the gerrit web interface :P22:32
*** marcoemorais has quit IRC22:33
*** marcoemorais has joined #openstack-keystone22:33
jamielennoxmethod signautre is exactly the same as list_projects_for_user22:34
bknudsonmorganfainberg: I still think it would be easier if the child of a token without audit_id didn't have an audit_id.22:34
jamielennoxdolphm: there is a test there that should test exactly this case: test_list_domains_for_user_with_grants22:35
morganfainbergbknudson, i think it will be a very minimal window where we'd be dealing with it (if at all).22:35
jamielennoxdolphm: it'll be called with the user, and then the groups that the user is a part of22:35
bknudsonmorganfainberg: it could be a rolling update.22:36
bknudsonI suppose we don't support that since the database wouldn't work.22:37
morganfainbergbknudson, yeah.22:37
bknudsonsomeday maybe22:37
dolphmjamielennox: so instead of overloading the method where the name makes no sense, why not have two methods?22:37
morganfainbergbknudson, i was thinking of how i'd do a rolling update22:37
jamielennoxseperate 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 way22:38
*** jasondotstar has quit IRC22:39
jamielennoxwhich is why i copied the big comment from henrynash into the new method as well22:40
*** hrybacki has quit IRC22:40
dolphmjamielennox: do you ever call it with groups?22:41
jamielennoxdolphm: it's the users groups, not just groups22:41
jamielennoxso no22:41
*** gordc has quit IRC22:41
jamielennoxhttps://review.openstack.org/#/c/114903/3/keystone/assignment/core.py22:42
jamielennoxthe api function is list_domains_for_user(self, user_id, hints=None)22:42
*** henrynash has quit IRC22:42
dolphmoh i see what you did22:42
jamielennoxwhich finds the groups associated with the user_id and passes them to backend22:42
dolphmjamielennox: so the manager method name is fine, but the driver method name is overloaded22:42
dolphmjamielennox: yeah, can you break the driver calls in two?22:43
jamielennoxi wouldn't say i'm a fan of the approach and i can change it, but i went that way for precedent22:43
dolphmjamielennox: the overloaded methods in assignment are a bad precedent :) no need to contribute to it!22:43
jamielennoxhttps://github.com/openstack/keystone/blob/master/keystone/assignment/backends/sql.py#L23622:43
jamielennoxdolphm: alright, i'll fix that22:44
jamielennoxi think there is already a domains_for_groups function that the federation work needed22:44
dolphmjamielennox: actually, list_domains_for_groups is already implemented22:44
*** topol has quit IRC22:44
jamielennoxyep, there is a list_projects_for_groups as well...22:45
jamielennoxwas thinking when i wrote this it'd be nice to do a mass cleanup of the calls to that backend22:45
*** amerine_ has quit IRC22:46
dolphmjamielennox: oh, you were following: return self.driver.list_projects_for_user(user_id, group_ids, hints or driver_hints.Hints()) # yuck!22:47
jamielennoxyep22:47
jamielennoxdolphm: so,  still split?22:51
dolphmjamielennox: yes definitely, but i'm trying to decide if i want to +A as-is and then see a refactor for all this at once22:51
jamielennoxrefactor including the old list_projects_for_users you mean?22:52
*** dims has quit IRC22:52
dolphmjamielennox: yeah22:53
dolphmjamielennox: i'd rather not hold up your patch, and i'd rather see all this fixed in lockstep22:53
dolphmjamielennox: holding up your patch to introduce more incosistency, even if it's more logical = meh22:54
dolphmjamielennox: almost done reviewing tests...22:54
*** amerine has quit IRC22:55
dolphmjamielennox: +A22:55
dolphmand my laptop battery is at 1% so /me waves goodbye22:55
*** bknudson has quit IRC22:55
jamielennoxdolphm: let me know what you want me to do regarding a cleanup22:56
jamielennoxdolphm: 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 shop22:57
*** hrybacki has joined #openstack-keystone22:58
*** hrybacki has quit IRC23:03
*** shakamunyi has quit IRC23:13
*** alex_xu has quit IRC23:25
*** topol has joined #openstack-keystone23:25
openstackgerritJamie Lennox proposed a change to openstack/python-keystoneclient: Allow unauthenticated discovery  https://review.openstack.org/10757023:29
openstackgerritJamie Lennox proposed a change to openstack/python-keystoneclient: Make keystoneclient use an adapter  https://review.openstack.org/9768123:29
openstackgerritJamie Lennox proposed a change to openstack/python-keystoneclient: Allow providing a default value to CLI loading  https://review.openstack.org/11374223:29
openstackgerritJamie Lennox proposed a change to openstack/python-keystoneclient: Version independent plugins  https://review.openstack.org/8114723:29
*** zzzeek has quit IRC23:30
*** zzzeek has joined #openstack-keystone23:30
*** david-lyle has quit IRC23:34
*** hrybacki has joined #openstack-keystone23:40
*** portante has quit IRC23:40
*** portante has joined #openstack-keystone23:40
*** ncoghlan has joined #openstack-keystone23:43
*** nkinder has joined #openstack-keystone23:46
*** gothicmindfood has quit IRC23:46
*** gothicmindfood has joined #openstack-keystone23:46
*** dhellmann has quit IRC23:49
*** dhellmann has joined #openstack-keystone23:50
*** RicoLin has joined #openstack-keystone23:52
*** openstackgerrit has quit IRC23:54
*** gokrokve has quit IRC23:57
*** openstackgerrit has joined #openstack-keystone23:57

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!