| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef namespaces https://review.opendev.org/c/openstack/glance-tempest-plugin/+/800902 | 09:43 |
|---|---|---|
| jokke_ | abhishekk: responded to your comment on test. Feel free to add any of your negative cases not covered as I have no idea what else you want to cover as such | 10:35 |
| abhishekk | jokke_, ack | 10:36 |
| jokke_ | abhishekk: hope that helps understanding what's going on in there ;) | 10:43 |
| jokke_ | let me know if not | 10:43 |
| abhishekk | didn't have detail look yet, in middle of something, will ping you if something is needed | 10:44 |
| opendevreview | Merged openstack/glance stable/wallaby: Fix the policy deprecation message https://review.opendev.org/c/openstack/glance/+/800100 | 14:30 |
| * abhishekk working on members API | 14:34 | |
| * dansmith is doing death-by-delete-policy | 14:41 | |
| abhishekk | same is for member delete as well :P | 14:44 |
| dansmith | I'm actually not sure that delete has enough protection in it to run without the auth layer and rule:default | 14:56 |
| abhishekk | :/ | 15:04 |
| croelandt | dansmith: did we get through all your patches under review or am I hallucinating? | 15:15 |
| dansmith | croelandt: all the ones we can merge yeah | 15:16 |
| dansmith | trying to decide between suicide and fixing the delete patch | 15:16 |
| croelandt | I'd fix the delete patch if I were you | 15:17 |
| dansmith | we'll see. | 15:17 |
| dansmith | basically I think the location layer in the onion does no checking and will nuke the locations on the image if it's called, | 15:18 |
| dansmith | while the other layers delete operation just mutate the image object in memory and wait for the image_repo to save those changes, where the actual permissions checks are don | 15:18 |
| dansmith | *done | 15:18 |
| dansmith | with the auth layer this all exploded earlier so we never got this far | 15:19 |
| dansmith | it's really a disaster. | 15:19 |
| abhishekk | ohh, sounds frightening | 15:21 |
| abhishekk | lbragstad, Policies ['get_member', 'get_members'] reference a rule that is not defined. | 15:21 |
| abhishekk | does this mean something is wrong with f string defined in policies ? | 15:22 |
| dansmith | the db just enforces "is admin or owner" if the policy check doesn't do anything at the top layers, so I think I'm just going to have to add that to the API if secure_rbac=False | 15:23 |
| abhishekk | I think I have done that in member | 15:24 |
| dansmith | it sucks pretty bad. | 15:25 |
| abhishekk | https://review.opendev.org/c/openstack/glance/+/802996/1/glance/api/v2/image_members.py#363 | 15:25 |
| dansmith | ugh | 15:25 |
| dansmith | okay, let's at least put that in a handler we can test and both use | 15:25 |
| dansmith | I'll do it in mine | 15:25 |
| abhishekk | I am opting another method as well, lets see both and decide | 15:26 |
| dansmith | abhishekk: this is what I was thinking: https://termbin.com/ccub | 15:37 |
| abhishekk | looking | 15:37 |
| dansmith | fixes my immediate issue, but need to make sure it doesn't break anything else | 15:37 |
| dansmith | the delete operation in API does several things that all need to not run if we're not going to actually let the user do it, so checking it there when we're doing our new policy check properly keeps us from doing any of those things | 15:38 |
| abhishekk | looks better | 15:39 |
| abhishekk | This could be required everywhere (for each delete call I guess) | 15:40 |
| dansmith | meaning member delete as well? | 15:41 |
| abhishekk | i think so | 15:41 |
| abhishekk | dansmith, quick help | 15:41 |
| dansmith | if that falls back to admin-or-owner(-of-image) too then yeah | 15:41 |
| abhishekk | could you please tell me what is wrong with these f strings ? | 15:42 |
| abhishekk | ADMIN_OR_SHARED_MEMBER = f'role:admin or {IMAGE_MEMBER_CHECK}' | 15:42 |
| abhishekk | ADMIN_OR_PROJECT_READER_OR_SHARED_MEMBER = ( | 15:42 |
| abhishekk | f'rule:admin or ' | 15:42 |
| abhishekk | f'role:reader and (project_id:%(project_id)s or {IMAGE_MEMBER_CHECK})' | 15:42 |
| abhishekk | ) | 15:42 |
| dansmith | what's wrong with them is that they're f**k-strings | 15:42 |
| abhishekk | haha | 15:43 |
| dansmith | you have "rule:admin" but meant "role:admin" | 15:43 |
| dansmith | that's why policy is claiming missing rule :) | 15:43 |
| abhishekk | ohhh | 15:43 |
| abhishekk | thank you | 15:44 |
| dansmith | np :) | 15:44 |
| abhishekk | I was struggling for half an hour with this :/ | 15:44 |
| dansmith | I know the feeling.. fresh eyes. | 15:44 |
| abhishekk | now I will have my dinner in peace :D | 15:46 |
| abhishekk | will be back in short | 15:46 |
| dansmith | hah | 15:49 |
| opendevreview | Dan Smith proposed openstack/glance master: Check delete_image policy in the API https://review.opendev.org/c/openstack/glance/+/798073 | 15:53 |
| opendevreview | Dan Smith proposed openstack/glance master: POC: Check deactivate, reactivate policy in the API https://review.opendev.org/c/openstack/glance/+/798266 | 15:53 |
| opendevreview | Dan Smith proposed openstack/glance master: Add check_is_image_mutable() legacy helper https://review.opendev.org/c/openstack/glance/+/803776 | 15:53 |
| dansmith | ooh, missed a test case actually | 15:55 |
| opendevreview | Dan Smith proposed openstack/glance master: Check delete_image policy in the API https://review.opendev.org/c/openstack/glance/+/798073 | 16:03 |
| opendevreview | Dan Smith proposed openstack/glance master: POC: Check deactivate, reactivate policy in the API https://review.opendev.org/c/openstack/glance/+/798266 | 16:03 |
| * lbragstad trying to catch up | 16:08 | |
| lbragstad | ok - sounds like you all figured out the undefined policy thing? | 16:12 |
| dansmith | if you mean "reference a rule that is not defined." yeah | 16:13 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Refactor gateway auth layer for member APIs https://review.opendev.org/c/openstack/glance/+/802525 | 16:47 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: PoC Move member policy checks to API layer https://review.opendev.org/c/openstack/glance/+/802996 | 16:47 |
| abhishekk | lbragstad, need your help here as one test will fail in glance-tempest-plugin with this change | 16:47 |
| abhishekk | dansmith, this is how I managed to do it | 16:48 |
| dansmith | abhishekk: ack on both TODOs if I respin, good call | 17:24 |
| abhishekk | great | 17:25 |
| dansmith | I mentioned "transitional" in the docstring of the helper, at least | 17:25 |
| abhishekk | yeah | 17:25 |
| opendevreview | Abhishek Kekane proposed openstack/glance-tempest-plugin master: Fix modify_member policy functional test case https://review.opendev.org/c/openstack/glance-tempest-plugin/+/803801 | 18:49 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: PoC Move member policy checks to API layer https://review.opendev.org/c/openstack/glance/+/802996 | 18:50 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: PoC Move member policy checks to API layer https://review.opendev.org/c/openstack/glance/+/802996 | 18:52 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: PoC Move member policy checks to API layer https://review.opendev.org/c/openstack/glance/+/802996 | 18:54 |
| * abhishekk signing out for the day | 19:14 | |
| opendevreview | Ade Lee proposed openstack/glance master: DNM: Add fips check jobs https://review.opendev.org/c/openstack/glance/+/790536 | 20:39 |
| opendevreview | Lance Bragstad proposed openstack/glance master: DNM: Fix role check in image membership policy https://review.opendev.org/c/openstack/glance/+/803808 | 21:55 |
| opendevreview | Ade Lee proposed openstack/glance master: DNM: Add fips check jobs https://review.opendev.org/c/openstack/glance/+/790536 | 22:04 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!