)]}'
{"glance/api/v2/images.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7f78c75d2bd92d452cd94fe9e8418e8b16d1cf8c","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                                  self.policy).add_image()"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        image_factory \u003d self.gateway.get_image_factory(req.context)"},{"line_number":99,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":100,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":101,"context_line":"        try:"},{"line_number":102,"context_line":"            ks_quota.enforce_image_count_total(req.context, req.context.owner)"},{"line_number":103,"context_line":"            image \u003d image_factory.new_image(extra_properties\u003dextra_properties,"}],"source_content_type":"text/x-python","patch_set":1,"id":"276f9f4a_78021f69","line":100,"range":{"start_line":99,"start_character":8,"end_line":100,"end_character":69},"updated":"2021-08-17 07:36:53.000000000","message":"Proxy is causing something, even though we are removing policy layer, with this change we are having image object in location.py as glance.api.policy.ImageProxy object at 0x7f5a62b38190\n\nwhereas without this change same image object is;\nglance.location.ImageProxy object at 0x7efddeb75ee0\n\nglance.api.policy.ImageProxy has ImageLocationsProxy which causes enforcement with this change whereas without this change it does not enforce get_image_location policy in location add method.","commit_id":"7fdb10ac924156055867391d20fe99437af4aa20"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"6c33d6d2b59932e7c99440aa58fc3c88de9d1bf7","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                                  self.policy).add_image()"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        image_factory \u003d self.gateway.get_image_factory(req.context)"},{"line_number":99,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":100,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":101,"context_line":"        try:"},{"line_number":102,"context_line":"            ks_quota.enforce_image_count_total(req.context, req.context.owner)"},{"line_number":103,"context_line":"            image \u003d image_factory.new_image(extra_properties\u003dextra_properties,"}],"source_content_type":"text/x-python","patch_set":1,"id":"a5500c61_79b20405","line":100,"range":{"start_line":99,"start_character":8,"end_line":100,"end_character":69},"in_reply_to":"276f9f4a_78021f69","updated":"2021-08-17 08:25:22.000000000","message":"Further debugging I have found that this test requires admin role but currently it is running with member role and as now we are changing the flow which is causing location add to enforce get_image_policy at add method, the check is failing because we don\u0027t have admin role.\n\n\nThe reason behind test is passing on master branch without this change, even get_image_location is enforced twice is we are catching forbidden exception and attaching empty list to it (at line #1543)\n\nWe need to understand how this proxy thing works, (adding admin role to test will pass the test), can be reproduciable in local environment by changing get_image_location to rule:admin and running image-create command with member role.","commit_id":"7fdb10ac924156055867391d20fe99437af4aa20"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4a825f25743ef4d765a109d3d5c909b9a418800a","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                                  self.policy).add_image()"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        image_factory \u003d self.gateway.get_image_factory(req.context)"},{"line_number":99,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":100,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":101,"context_line":"        try:"},{"line_number":102,"context_line":"            ks_quota.enforce_image_count_total(req.context, req.context.owner)"},{"line_number":103,"context_line":"            image \u003d image_factory.new_image(extra_properties\u003dextra_properties,"}],"source_content_type":"text/x-python","patch_set":1,"id":"eeec8c95_0b612515","line":100,"range":{"start_line":99,"start_character":8,"end_line":100,"end_character":69},"in_reply_to":"a5500c61_79b20405","updated":"2021-08-17 09:22:21.000000000","message":"As I said earlier, if you build your factory at line #98 with authorization_layer\u003dFalse and remove policy and authorization factory form the layer, you will get image object at line 103 which will match with your Repo proxy layer.\n\n\nWith this change it will pass the failing functional test case without any change to it.","commit_id":"7fdb10ac924156055867391d20fe99437af4aa20"}],"glance/api/v2/policy.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"64d0f907bb985c959766e0743ddf7dd4a149f225","unresolved":true,"context_lines":[{"line_number":187,"context_line":"        properties \u003d properties or {}"},{"line_number":188,"context_line":"        if \u0027visibility\u0027 in properties:"},{"line_number":189,"context_line":"            self._enforce_visibility(properties[\u0027visibility\u0027])"},{"line_number":190,"context_line":"        if not CONF.enforce_secure_rbac:"},{"line_number":191,"context_line":"            check_admin_or_same_owner(self._context, properties)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def get_image(self):"},{"line_number":194,"context_line":"        self._enforce(\u0027get_image\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"24169aab_b667f6e4","line":191,"range":{"start_line":190,"start_character":8,"end_line":191,"end_character":64},"updated":"2021-08-19 10:08:19.000000000","message":"If RBAC is enabled then this (this mean repo.add() from controller) will go to db layer and raise forbidden from there with message;\n\nHTTP 403 Forbidden: Forbidding request, image 1d0435df-885e-4f62-803c-8b4f2b04b8fc not visible\n\nwhich will be confusing, so can we move this validation to API layer or give custom message in add method for forbidden exception?","commit_id":"640ed4a302b869c18b005068cdf6038f255c74fd"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"ccff70c1fa078640a33b2bdea3bf52f25b37d884","unresolved":true,"context_lines":[{"line_number":205,"context_line":"            # in the past."},{"line_number":206,"context_line":"            if self._target[\u0027owner\u0027] !\u003d self._context.project_id:"},{"line_number":207,"context_line":"                msg \u003d _(\"You are not permitted to create images \""},{"line_number":208,"context_line":"                        \"owned by \u0027%s\u0027\" % self._target[\u0027owner\u0027])"},{"line_number":209,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"dd54708e_9448c8a8","line":208,"range":{"start_line":208,"start_character":35,"end_line":208,"end_character":37},"updated":"2021-08-19 21:54:53.000000000","message":"I\u0027m muttering out load here - but this only ever comes from the request body, so if it\u0027s invalid it\u0027s because the user supplied garbage that doesn\u0027t match their project.\n\nThere should be anything in glance\u0027s pipeline that validates the owner/project_id of a new image against keystone, is there? In other words, I couldn\u0027t use this to find valid project IDs in keystone, could I?","commit_id":"de560b19e6bcc6e3eb7bbc94b42710c4ca8819ce"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"71ef0fd7be7ba36a276ae38515cd02eaa574b2d1","unresolved":true,"context_lines":[{"line_number":205,"context_line":"            # in the past."},{"line_number":206,"context_line":"            if self._target[\u0027owner\u0027] !\u003d self._context.project_id:"},{"line_number":207,"context_line":"                msg \u003d _(\"You are not permitted to create images \""},{"line_number":208,"context_line":"                        \"owned by \u0027%s\u0027\" % self._target[\u0027owner\u0027])"},{"line_number":209,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"9289e99f_04843f8f","line":208,"range":{"start_line":208,"start_character":35,"end_line":208,"end_character":37},"in_reply_to":"8acf8107_60fce46f","updated":"2021-08-20 16:11:56.000000000","message":"Yeah, not AFAIK. In fact, in our tests we create images with owner\u003d$garbage just because it\u0027s easy, and I\u0027m not aware of any part of the pipeline that we\u0027re skipping to make that happen (although I guess it\u0027s possible).","commit_id":"de560b19e6bcc6e3eb7bbc94b42710c4ca8819ce"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"2b1ae9fd938c87ecb4a25199fdd8e2ba2c9d53f8","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            # in the past."},{"line_number":206,"context_line":"            if self._target[\u0027owner\u0027] !\u003d self._context.project_id:"},{"line_number":207,"context_line":"                msg \u003d _(\"You are not permitted to create images \""},{"line_number":208,"context_line":"                        \"owned by \u0027%s\u0027\" % self._target[\u0027owner\u0027])"},{"line_number":209,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"edd211cd_9282ca10","line":208,"range":{"start_line":208,"start_character":35,"end_line":208,"end_character":37},"in_reply_to":"9289e99f_04843f8f","updated":"2021-08-20 16:14:24.000000000","message":"Ack","commit_id":"de560b19e6bcc6e3eb7bbc94b42710c4ca8819ce"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"00cfd10893a96c710432265185e445af6a67ae7a","unresolved":true,"context_lines":[{"line_number":205,"context_line":"            # in the past."},{"line_number":206,"context_line":"            if self._target[\u0027owner\u0027] !\u003d self._context.project_id:"},{"line_number":207,"context_line":"                msg \u003d _(\"You are not permitted to create images \""},{"line_number":208,"context_line":"                        \"owned by \u0027%s\u0027\" % self._target[\u0027owner\u0027])"},{"line_number":209,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"b2f921d2_7df274bc","line":208,"range":{"start_line":208,"start_character":35,"end_line":208,"end_character":37},"in_reply_to":"9850c25d_314b868c","updated":"2021-08-20 15:38:08.000000000","message":"\u003e Right, but what\u0027s the problem with that? You think that if I say \"create an image for $garbage\" that we shouldn\u0027t say \"you can\u0027t create images for $garbage\" ?\n\u003e \n\nI think that\u0027s the right behavior.\n\n\u003e It\u0027s the same (exact) message the existing code is returning, so I don\u0027t think it\u0027s worse than it was, unless I\u0027m missing something.\n\nNo, I\u0027m just being paranoid. But you\u0027re right, chances are if my concern were an issue (and it doesn\u0027t sound like it is), it would be something to fix in a separate patch.","commit_id":"de560b19e6bcc6e3eb7bbc94b42710c4ca8819ce"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5edca23dcdb5f3d23a246babfc9749cafbd5c110","unresolved":true,"context_lines":[{"line_number":205,"context_line":"            # in the past."},{"line_number":206,"context_line":"            if self._target[\u0027owner\u0027] !\u003d self._context.project_id:"},{"line_number":207,"context_line":"                msg \u003d _(\"You are not permitted to create images \""},{"line_number":208,"context_line":"                        \"owned by \u0027%s\u0027\" % self._target[\u0027owner\u0027])"},{"line_number":209,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"d61936d7_bc08f3f2","line":208,"range":{"start_line":208,"start_character":35,"end_line":208,"end_character":37},"in_reply_to":"b2f921d2_7df274bc","updated":"2021-08-20 15:45:45.000000000","message":"Okay, but still, I\u0027m not exactly sure what you\u0027re thinking. Are you thinking that if the $garbage isn\u0027t a valid id, then we should say \"$garbage isn\u0027t a valid owner\" and then after they correct the typo, say \"well, you\u0027re not allowed to do that even though it is correct\" ?\n\nI guess that\u0027s valid, but on the other hand, we can avoid the trip to validate with keystone by saying \"well, you can\u0027t do this anyway, so I need not even validate your garbage.\"","commit_id":"de560b19e6bcc6e3eb7bbc94b42710c4ca8819ce"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"76fcc7b457b8e038461a4d1fd8bb070990e685b4","unresolved":true,"context_lines":[{"line_number":205,"context_line":"            # in the past."},{"line_number":206,"context_line":"            if self._target[\u0027owner\u0027] !\u003d self._context.project_id:"},{"line_number":207,"context_line":"                msg \u003d _(\"You are not permitted to create images \""},{"line_number":208,"context_line":"                        \"owned by \u0027%s\u0027\" % self._target[\u0027owner\u0027])"},{"line_number":209,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"8acf8107_60fce46f","line":208,"range":{"start_line":208,"start_character":35,"end_line":208,"end_character":37},"in_reply_to":"d61936d7_bc08f3f2","updated":"2021-08-20 16:01:18.000000000","message":"The case I was thinking about is:\n\n  - a pepsi user starts fishing for valid project IDs using add_image\n  - glance validates the project ID in keystone\n  - the 403 error message confirms the users guess and they found coke\u0027s project ID\n\nBut that\u0027s not possible today because the second step doesn\u0027t exist?","commit_id":"de560b19e6bcc6e3eb7bbc94b42710c4ca8819ce"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"64bdf3430c19667eba5a47250e75a27db15550c5","unresolved":true,"context_lines":[{"line_number":205,"context_line":"            # in the past."},{"line_number":206,"context_line":"            if self._target[\u0027owner\u0027] !\u003d self._context.project_id:"},{"line_number":207,"context_line":"                msg \u003d _(\"You are not permitted to create images \""},{"line_number":208,"context_line":"                        \"owned by \u0027%s\u0027\" % self._target[\u0027owner\u0027])"},{"line_number":209,"context_line":"                raise webob.exc.HTTPForbidden(msg)"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"9850c25d_314b868c","line":208,"range":{"start_line":208,"start_character":35,"end_line":208,"end_character":37},"in_reply_to":"dd54708e_9448c8a8","updated":"2021-08-20 13:33:12.000000000","message":"Right, but what\u0027s the problem with that? You think that if I say \"create an image for $garbage\" that we shouldn\u0027t say \"you can\u0027t create images for $garbage\" ?\n\nIt\u0027s the same (exact) message the existing code is returning, so I don\u0027t think it\u0027s worse than it was, unless I\u0027m missing something.","commit_id":"de560b19e6bcc6e3eb7bbc94b42710c4ca8819ce"}],"glance/tests/functional/v2/test_images_api_policy.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"64d0f907bb985c959766e0743ddf7dd4a149f225","unresolved":true,"context_lines":[{"line_number":214,"context_line":"        # Now disable add_image and make sure we get 403"},{"line_number":215,"context_line":"        self.set_policy_rules({\u0027add_image\u0027: \u0027!\u0027})"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        self.assertEqual(403, self._create().status_code)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def test_image_delete(self):"},{"line_number":220,"context_line":"        self.start_server()"}],"source_content_type":"text/x-python","patch_set":2,"id":"283cd396_7b1ee487","line":217,"updated":"2021-08-19 10:08:19.000000000","message":"Better to add owner validation scenario here as well?","commit_id":"640ed4a302b869c18b005068cdf6038f255c74fd"}]}