)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"647e7334c44ed85a042b88ca9da27244a92c9d03","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"NOTE: This has changes to make existing tests pass, but likely needs"},{"line_number":10,"context_line":"additional coverage for full verification"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"NOTE: This includes a default non-RBAC policy change, which is"},{"line_number":13,"context_line":"required to make the de/reactivate policy check actually useful."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ia6e51228aee1ddf09734f922c5f6f3eb4ece60c1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"591e85c3_b73c3c66","line":13,"range":{"start_line":11,"start_character":0,"end_line":13,"end_character":64},"updated":"2021-07-07 05:46:59.000000000","message":"I think this should be reported as a bug and fixed separately.\n\nThis is why I said in the spec that some of our policies are confusing and this is the right time to get those fixed.","commit_id":"8a3db67da1332634602d79f4efe600d5ae9591e0"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5dbb7347d7a15dbc2291650fa92b2c19c423b4db","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"NOTE: This has changes to make existing tests pass, but likely needs"},{"line_number":10,"context_line":"additional coverage for full verification"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"NOTE: This includes a default non-RBAC policy change, which is"},{"line_number":13,"context_line":"required to make the de/reactivate policy check actually useful."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ia6e51228aee1ddf09734f922c5f6f3eb4ece60c1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5b7c3a46_a4a3c4c2","line":13,"range":{"start_line":11,"start_character":0,"end_line":13,"end_character":64},"in_reply_to":"2ffad117_653b9c34","updated":"2021-07-07 13:59:26.000000000","message":"Lets see, I have added this topic for tomorrow\u0027s discussion.","commit_id":"8a3db67da1332634602d79f4efe600d5ae9591e0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"57cfc045f24281115f6182ae2f7ddd275ec76a1b","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"NOTE: This has changes to make existing tests pass, but likely needs"},{"line_number":10,"context_line":"additional coverage for full verification"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"NOTE: This includes a default non-RBAC policy change, which is"},{"line_number":13,"context_line":"required to make the de/reactivate policy check actually useful."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ia6e51228aee1ddf09734f922c5f6f3eb4ece60c1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"2ffad117_653b9c34","line":13,"range":{"start_line":11,"start_character":0,"end_line":13,"end_character":64},"in_reply_to":"591e85c3_b73c3c66","updated":"2021-07-07 13:28:05.000000000","message":"By confusing do you mean \"completely wrong\" ? ;)\n\nI agree this should be a separate fix, but I also think there are more than just this one. Not sure if you want to do an audit of those and try to get them all done or fix each one as we come to it.","commit_id":"8a3db67da1332634602d79f4efe600d5ae9591e0"}],"glance/api/v2/policy.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"60b4a1f037afc6f32e52a95f11eab62d95c1ae8b","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        self._enforce(\u0027delete_image\u0027)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def deactivate_image(self):"},{"line_number":98,"context_line":"        self._enforce(\u0027deactivate\u0027)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def reactivate_image(self):"},{"line_number":101,"context_line":"        self._enforce(\u0027reactivate\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"c3460453_228b2319","line":98,"updated":"2021-07-06 16:44:55.000000000","message":"Unless I\u0027m missing something, the default policy for deactivate and reactivate is \"rule:default\" which resolves to \"\". That means anyone can do it according to the policy we have and are shipping today. I think we\u0027ve buried some bespoke enforcement in the onion layers (potentially just the \"only owner can modify\") which makes this not actually a thing, but I think the test failure on this patch is exposing that fact at the moment. The test is depending on the fact that TENANT2 can\u0027t de/reactivate the image, but that\u0027s actually just because they don\u0027t own it, not because they fail the policy check.\n\nThis is another good reason not to have hard-coded checks for these things, and not to have one policy check imply the other. Operators could have totally broken policy that doesn\u0027t expose something because there is some implied additional check, but if something changes in the order or method of enforcement, suddenly they\u0027re allowing people to do things they don\u0027t intend. So in this case, someone taking the default policy is literally saying \"anyone can de/reactivate anyone else\u0027s image\" according to the rules, and we\u0027re only preventing it because of the hard-coded \"only owner can do anything\" check, which needs to change anyway.","commit_id":"739c0d46d64927b0ae9c7b6c0ab4dc1b325d92ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8bd8a9e0c70a48a40002808098a9c9ba0c75709","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        self._enforce(\u0027delete_image\u0027)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def deactivate_image(self):"},{"line_number":98,"context_line":"        self._enforce(\u0027deactivate\u0027)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def reactivate_image(self):"},{"line_number":101,"context_line":"        self._enforce(\u0027reactivate\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"8d62375a_363fc3d1","line":98,"in_reply_to":"c3460453_228b2319","updated":"2021-07-06 16:49:20.000000000","message":"Adding this:\n\nhttps://termbin.com/rjen\n\nMakes this test pass for me, and is probably what we need to do. TBH, it looks like maybe the defaults for a lot of operations are defaulting to \"anyone can do anything\" and the only thing that is preventing that today is the hard-coded \"only owner\" layer :/","commit_id":"739c0d46d64927b0ae9c7b6c0ab4dc1b325d92ab"}],"glance/tests/functional/v2/test_images_api_policy.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"123420b5724dfff86ccc49283c9c8a6832256b84","unresolved":true,"context_lines":[{"line_number":405,"context_line":"        resp \u003d self.api_post(\u0027/v2/images/%s/actions/reactivate\u0027 % image_id)"},{"line_number":406,"context_line":"        self.assertEqual(204, resp.status_code)"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"        # Make sure it is really deactivated"},{"line_number":409,"context_line":"        resp \u003d self.api_get(\u0027/v2/images/%s\u0027 % image_id)"},{"line_number":410,"context_line":"        self.assertEqual(\u0027active\u0027, resp.json[\u0027status\u0027])"},{"line_number":411,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"71ee2ef4_f020040f","line":408,"range":{"start_line":408,"start_character":33,"end_line":408,"end_character":44},"updated":"2021-08-19 21:09:19.000000000","message":"active* reactivated*?","commit_id":"662d7700f48b37f1ed32c7bb254ff90b6a63b382"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"71a906784193c36368a0edb637e3b2b0da9bfa06","unresolved":true,"context_lines":[{"line_number":452,"context_line":"        resp \u003d self.api_post(\u0027/v2/images/%s/actions/reactivate\u0027 % image_id)"},{"line_number":453,"context_line":"        self.assertEqual(204, resp.status_code)"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"        # Make sure it is really deactivated"},{"line_number":456,"context_line":"        resp \u003d self.api_get(\u0027/v2/images/%s\u0027 % image_id)"},{"line_number":457,"context_line":"        self.assertEqual(\u0027active\u0027, resp.json[\u0027status\u0027])"},{"line_number":458,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"9da632a4_9bc8ce34","line":455,"range":{"start_line":455,"start_character":33,"end_line":455,"end_character":44},"updated":"2021-08-20 16:12:09.000000000","message":"I think this is copy/pasted?","commit_id":"e1808c008e4843b05b491033344b957efe9bef8b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2d4ff1d8bae1f72d8aa67b89db7f625ef5355565","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        resp \u003d self.api_post(\u0027/v2/images/%s/actions/reactivate\u0027 % image_id)"},{"line_number":453,"context_line":"        self.assertEqual(204, resp.status_code)"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"        # Make sure it is really deactivated"},{"line_number":456,"context_line":"        resp \u003d self.api_get(\u0027/v2/images/%s\u0027 % image_id)"},{"line_number":457,"context_line":"        self.assertEqual(\u0027active\u0027, resp.json[\u0027status\u0027])"},{"line_number":458,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"778195c1_4efaabd7","line":455,"range":{"start_line":455,"start_character":33,"end_line":455,"end_character":44},"in_reply_to":"9da632a4_9bc8ce34","updated":"2021-08-23 05:59:44.000000000","message":"Ack","commit_id":"e1808c008e4843b05b491033344b957efe9bef8b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4de3f1da13ddb7350d14ec49ed7e46ee36fdc821","unresolved":true,"context_lines":[{"line_number":514,"context_line":"                self.assertEqual(404, resp.status_code)"},{"line_number":515,"context_line":"            else:"},{"line_number":516,"context_line":"                # public and community visibility will return 403"},{"line_number":517,"context_line":"                self.assertEqual(403, resp.status_code)"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"    def test_image_reactivate(self):"},{"line_number":520,"context_line":"        self.start_server()"}],"source_content_type":"text/x-python","patch_set":30,"id":"c8e77fe6_554483c9","line":517,"updated":"2021-08-23 13:42:47.000000000","message":"I think I would put everything from L472 to here in a separate test. It\u0027s pretty long at this point add this last bit is self-contained.","commit_id":"82111150d603d8a445876afbfce67b47aee39bf7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4de3f1da13ddb7350d14ec49ed7e46ee36fdc821","unresolved":true,"context_lines":[{"line_number":624,"context_line":"                self.assertEqual(404, resp.status_code)"},{"line_number":625,"context_line":"            else:"},{"line_number":626,"context_line":"                # public and community visibility will return 403"},{"line_number":627,"context_line":"                self.assertEqual(403, resp.status_code)"}],"source_content_type":"text/x-python","patch_set":30,"id":"8b22c9b0_5fe7c9ea","line":627,"updated":"2021-08-23 13:42:47.000000000","message":"Same here, and it looks like maybe combining this with the split-out test above might be good too?","commit_id":"82111150d603d8a445876afbfce67b47aee39bf7"}]}