)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8d2096d1b2dc3e6c647f3cb1970263b45aad8a1c","unresolved":true,"context_lines":[{"line_number":10,"context_line":"returns 404 if the user can\u0027t delete the image via policy, but leaves"},{"line_number":11,"context_line":"the existing internal forbidden-\u003e403 handler because of how image"},{"line_number":12,"context_line":"property protections work."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I5f2a406e31b706906b71ea5ecb4f1a53674c97fa"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"e27023f2_02b2fffb","line":13,"updated":"2021-08-04 06:15:13.000000000","message":"Need partially-implements tag","commit_id":"164f9c1051e20e7b5c566493ec70b92f2d4e15c1"}],"glance/api/v2/images.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"842331223066e6ad2b402891ce4c405eba27c756","unresolved":true,"context_lines":[{"line_number":809,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":810,"context_line":"        try:"},{"line_number":811,"context_line":"            # FIXME(danms): This will still enforce the get_image policy"},{"line_number":812,"context_line":"            # which we don\u0027t want"},{"line_number":813,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":814,"context_line":""},{"line_number":815,"context_line":"            # NOTE(abhishekk): This is the right place to check whether user"}],"source_content_type":"text/x-python","patch_set":1,"id":"b95019c6_98786729","line":812,"updated":"2021-06-28 14:03:52.000000000","message":"You can remove this if you add authorization_layer\u003dFalse on L809 above. Since I started with update_image (per our plan) I have that TODO in the first patch, but after the second one to further refactor get image, we can avoid the unnecessary get-image check in subsequent patches. Doing so may uncover tests that continue to pass after your changes below, but only because we fail due to the existing layers that are still included with the authorization layer being in your repo.","commit_id":"7f91286e3a1d563c8b4e8f5005201f6b4363ab15"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0aeb1738cf69a0ea347468785819a94fbf8a61ca","unresolved":true,"context_lines":[{"line_number":809,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":810,"context_line":"        try:"},{"line_number":811,"context_line":"            # FIXME(danms): This will still enforce the get_image policy"},{"line_number":812,"context_line":"            # which we don\u0027t want"},{"line_number":813,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":814,"context_line":""},{"line_number":815,"context_line":"            # NOTE(abhishekk): This is the right place to check whether user"}],"source_content_type":"text/x-python","patch_set":1,"id":"326eba31_bb8d0d62","line":812,"in_reply_to":"b95019c6_98786729","updated":"2021-06-28 14:10:09.000000000","message":"Ack, the authorization layer is mocked in distributed image import tests, so I need to look out to mock other layers get method in order to return desired parmeters.","commit_id":"7f91286e3a1d563c8b4e8f5005201f6b4363ab15"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"20a1adec37c597b8d03fa36e0aea41c9e8549e2d","unresolved":true,"context_lines":[{"line_number":707,"context_line":"            LOG.warn(msg)"},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"    @utils.mutating"},{"line_number":710,"context_line":"    def delete_from_store(self, req, store_id, image_id):"},{"line_number":711,"context_line":"        if not CONF.enabled_backends:"},{"line_number":712,"context_line":"            raise webob.exc.HTTPNotFound()"},{"line_number":713,"context_line":"        if store_id not in CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":26,"id":"66bb6b6a_b45e0627","line":710,"range":{"start_line":710,"start_character":4,"end_line":710,"end_character":57},"updated":"2021-08-05 15:06:36.000000000","message":"should we cover this call in same patch ?\nI guess it is enofrcing get_image and delete_image_location policy.","commit_id":"1eedb2bcd53608021721a748ac6b8fb237128ce9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dcb0fd1e1e356063fe2b6dfd92e776307b0405bf","unresolved":true,"context_lines":[{"line_number":707,"context_line":"            LOG.warn(msg)"},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"    @utils.mutating"},{"line_number":710,"context_line":"    def delete_from_store(self, req, store_id, image_id):"},{"line_number":711,"context_line":"        if not CONF.enabled_backends:"},{"line_number":712,"context_line":"            raise webob.exc.HTTPNotFound()"},{"line_number":713,"context_line":"        if store_id not in CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":26,"id":"62482f17_230de984","line":710,"range":{"start_line":710,"start_character":4,"end_line":710,"end_character":57},"in_reply_to":"66bb6b6a_b45e0627","updated":"2021-08-05 20:10:07.000000000","message":"I think I\u0027d rather not cram that into this patch, but will take note that it needs it also.","commit_id":"1eedb2bcd53608021721a748ac6b8fb237128ce9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dcb0fd1e1e356063fe2b6dfd92e776307b0405bf","unresolved":true,"context_lines":[{"line_number":814,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":815,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":816,"context_line":"        try:"},{"line_number":817,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":818,"context_line":""},{"line_number":819,"context_line":"            # NOTE(abhishekk): This is the right place to check whether user"},{"line_number":820,"context_line":"            # have permission to delete the image and remove the policy check"}],"source_content_type":"text/x-python","patch_set":26,"id":"5e357ee3_6c3cfcc9","line":817,"range":{"start_line":817,"start_character":20,"end_line":817,"end_character":34},"updated":"2021-08-05 20:10:07.000000000","message":"Note that with auth layer, this passes, but returns an immutableimage, which causes us to 403 if it\u0027s not ours. Without the auth layer, we get NotFound here, which ends up as a 404. The tests expect the 403, which is also how things will behave with new policy since we can see the image (in that test) and thus a disallowed delete should return 403.\n\nSo I need to modify this to give the same behavior on both cases.","commit_id":"1eedb2bcd53608021721a748ac6b8fb237128ce9"}],"glance/api/v2/policy.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9d1e41b806231b60d52ba14b4aa4ea7490c38536","unresolved":true,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def delete_image(self):"},{"line_number":144,"context_line":"        self._enforce(\u0027delete_image\u0027)"},{"line_number":145,"context_line":"        if not CONF.enforce_secure_rbac:"},{"line_number":146,"context_line":"            check_is_image_mutable(self._context, self._image)"}],"source_content_type":"text/x-python","patch_set":28,"id":"c10d8143_a2054fab","line":146,"range":{"start_line":145,"start_character":8,"end_line":146,"end_character":62},"updated":"2021-08-06 17:01:58.000000000","message":"Or either here ToDo comment will be good to have.\n\nToDo(dansmith): Remove this once RBAC is fully supported.","commit_id":"c710ee855bc3688fdb2612f72f1c859ebea34bcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f6350ec36af95867992918bfd66903177f68ad76","unresolved":true,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def delete_image(self):"},{"line_number":144,"context_line":"        self._enforce(\u0027delete_image\u0027)"},{"line_number":145,"context_line":"        if not CONF.enforce_secure_rbac:"},{"line_number":146,"context_line":"            check_is_image_mutable(self._context, self._image)"}],"source_content_type":"text/x-python","patch_set":28,"id":"a48b37c6_158f49bf","line":146,"range":{"start_line":145,"start_character":8,"end_line":146,"end_character":62},"in_reply_to":"917da284_a34fcab4","updated":"2021-08-10 15:10:00.000000000","message":"\u003e I kind of agree, but I also think that once all of this works, we will just \"git grep enforce_secure_rbac\" and remove most occurrences, right?\n\nYeah, I think anything that looks at the CONF will be easy to spot. Comments for review clarity perhaps, but ... yeah.","commit_id":"c710ee855bc3688fdb2612f72f1c859ebea34bcb"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"50edfff091f549c00ccb395b7f1e03859847bfd0","unresolved":true,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def delete_image(self):"},{"line_number":144,"context_line":"        self._enforce(\u0027delete_image\u0027)"},{"line_number":145,"context_line":"        if not CONF.enforce_secure_rbac:"},{"line_number":146,"context_line":"            check_is_image_mutable(self._context, self._image)"}],"source_content_type":"text/x-python","patch_set":28,"id":"917da284_a34fcab4","line":146,"range":{"start_line":145,"start_character":8,"end_line":146,"end_character":62},"in_reply_to":"c10d8143_a2054fab","updated":"2021-08-07 00:40:21.000000000","message":"I kind of agree, but I also think that once all of this works, we will just \"git grep enforce_secure_rbac\" and remove most occurrences, right?","commit_id":"c710ee855bc3688fdb2612f72f1c859ebea34bcb"}],"glance/tests/unit/v2/test_images_resource.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"50edfff091f549c00ccb395b7f1e03859847bfd0","unresolved":true,"context_lines":[{"line_number":3071,"context_line":"            exc \u003d self.assertRaises(webob.exc.HTTPNotFound,"},{"line_number":3072,"context_line":"                                    self.controller.delete, request, UUID1)"},{"line_number":3073,"context_line":"            self.assertTrue(mock_enf.called)"},{"line_number":3074,"context_line":"        # Make sure we did not leak detauils of the original Forbidden"},{"line_number":3075,"context_line":"        # error into the NotFound returned to the client."},{"line_number":3076,"context_line":"        self.assertEqual(\u0027The resource could not be found.\u0027, str(exc))"},{"line_number":3077,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"67b2237e_f4ee29a0","line":3074,"range":{"start_line":3074,"start_character":36,"end_line":3074,"end_character":44},"updated":"2021-08-07 00:40:21.000000000","message":"Typo, but don\u0027t bother fixing it unless another patchset is needed :)","commit_id":"c710ee855bc3688fdb2612f72f1c859ebea34bcb"}]}