)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad4aab4cff55f87ede78943b1ced7881e6ea489e","unresolved":true,"context_lines":[{"line_number":16,"context_line":"to include the other states, like get_image does. I think this was"},{"line_number":17,"context_line":"just an oversight in the original RBAC patches, which didn\u0027t matter"},{"line_number":18,"context_line":"because they weren\u0027t really being honored strictly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I70100cd7f01da803e9740cea1f7ce7ae18ad6919"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"3d6543cc_d3af4b43","line":19,"updated":"2021-07-27 07:52:17.000000000","message":"Add partially-implements tag","commit_id":"cf3995cebf4f89e6bff68380adfc7cd225f9285f"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"2e47d88982086573e4205405140835cde7c6d14f","unresolved":true,"context_lines":[{"line_number":12,"context_line":"since the DB was performing its own checks and would never raise"},{"line_number":13,"context_line":"Forbidden."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This also includes a change of the default policy for get_images"},{"line_number":16,"context_line":"to include the other states, like get_image does. I think this was"},{"line_number":17,"context_line":"just an oversight in the original RBAC patches, which didn\u0027t matter"},{"line_number":18,"context_line":"because they weren\u0027t really being honored strictly."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"41595eee_c5a8974d","line":15,"range":{"start_line":15,"start_character":54,"end_line":15,"end_character":64},"updated":"2021-08-04 19:41:21.000000000","message":"nit: get_image?\n\nI think the changes made here only affect get_image and not get_images.\n\nIf so - this doesn\u0027t need to be respun to fix this, just wanted to double check to make sure I was understanding the commit message.","commit_id":"ba37ea322777bb1efb210f4cbb72cae2f2c541d2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9dbd11122946df8fb0464e68dbe1d12ba5852f12","unresolved":true,"context_lines":[{"line_number":12,"context_line":"since the DB was performing its own checks and would never raise"},{"line_number":13,"context_line":"Forbidden."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This also includes a change of the default policy for get_images"},{"line_number":16,"context_line":"to include the other states, like get_image does. I think this was"},{"line_number":17,"context_line":"just an oversight in the original RBAC patches, which didn\u0027t matter"},{"line_number":18,"context_line":"because they weren\u0027t really being honored strictly."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"93c106a7_060353ec","line":15,"range":{"start_line":15,"start_character":54,"end_line":15,"end_character":64},"in_reply_to":"41595eee_c5a8974d","updated":"2021-08-04 20:44:47.000000000","message":"Yeah, originally I was changing both before I realized we didn\u0027t need to be that specific in get_images. Probably a remnant of that. If I have to respin, I\u0027ll fix this.","commit_id":"ba37ea322777bb1efb210f4cbb72cae2f2c541d2"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7981b8e673e0ef35f141de248f9a0b86a9de26e8","unresolved":true,"context_lines":[{"line_number":15,"context_line":"This also includes a change of the default policy for get_images"},{"line_number":16,"context_line":"to include the other states, like get_image does. I think this was"},{"line_number":17,"context_line":"just an oversight in the original RBAC patches, which didn\u0027t matter"},{"line_number":18,"context_line":"because they weren\u0027t really being honored strictly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Partially implements: blueprint policy-refactor"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"f4552ff6_1126a98f","line":18,"updated":"2021-08-04 19:35:49.000000000","message":"++\n\nThanks for covering the \u0027shared\u0027 case.","commit_id":"ba37ea322777bb1efb210f4cbb72cae2f2c541d2"}],"glance/api/v2/images.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"210cf85bb2d302ce87aeac0164c922cd049c7cd0","unresolved":true,"context_lines":[{"line_number":523,"context_line":"            target \u003d {\u0027project_id\u0027: req.context.project_id}"},{"line_number":524,"context_line":"            self.policy.enforce(req.context, \u0027get_images\u0027, target)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"            images \u003d image_repo.list(marker\u003dmarker, limit\u003dlimit,"},{"line_number":527,"context_line":"                                     sort_key\u003dsort_key,"},{"line_number":528,"context_line":"                                     sort_dir\u003dsort_dir,"},{"line_number":529,"context_line":"                                     filters\u003dfilters,"},{"line_number":530,"context_line":"                                     member_status\u003dmember_status)"},{"line_number":531,"context_line":"            db_image_count \u003d len(images)"},{"line_number":532,"context_line":"            images \u003d [image for image in images"},{"line_number":533,"context_line":"                      if api_policy.ImageAPIPolicy(req.context, image,"}],"source_content_type":"text/x-python","patch_set":18,"id":"eb3fb461_f1e78c01","line":530,"range":{"start_line":526,"start_character":12,"end_line":530,"end_character":65},"updated":"2021-07-23 16:55:32.000000000","message":"I am sure that our db query is returning us only those records which are visible to project member/reader, shared with user etc (similar to what our default rbac policy for get_image is). Which means then below policy will only be enforced on those images which are visible to me. In short length of db_image_count and len(images) after get_image enforce check will always be same.","commit_id":"46d9f27cadf07bee52c35a082b6e4b256a4c04b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"406394c408d4c0ad008ec57b3ff2a143432414a1","unresolved":true,"context_lines":[{"line_number":523,"context_line":"            target \u003d {\u0027project_id\u0027: req.context.project_id}"},{"line_number":524,"context_line":"            self.policy.enforce(req.context, \u0027get_images\u0027, target)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"            images \u003d image_repo.list(marker\u003dmarker, limit\u003dlimit,"},{"line_number":527,"context_line":"                                     sort_key\u003dsort_key,"},{"line_number":528,"context_line":"                                     sort_dir\u003dsort_dir,"},{"line_number":529,"context_line":"                                     filters\u003dfilters,"},{"line_number":530,"context_line":"                                     member_status\u003dmember_status)"},{"line_number":531,"context_line":"            db_image_count \u003d len(images)"},{"line_number":532,"context_line":"            images \u003d [image for image in images"},{"line_number":533,"context_line":"                      if api_policy.ImageAPIPolicy(req.context, image,"}],"source_content_type":"text/x-python","patch_set":18,"id":"b79dec7f_0b4cd096","line":530,"range":{"start_line":526,"start_character":12,"end_line":530,"end_character":65},"in_reply_to":"eb3fb461_f1e78c01","updated":"2021-07-26 13:39:15.000000000","message":"Ack, but I think this change belongs here to protect against this breaking when that happens, right? I guess I should write a test for this here though.","commit_id":"46d9f27cadf07bee52c35a082b6e4b256a4c04b7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"210cf85bb2d302ce87aeac0164c922cd049c7cd0","unresolved":true,"context_lines":[{"line_number":557,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":558,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":559,"context_line":"        try:"},{"line_number":560,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":561,"context_line":"            api_policy.ImageAPIPolicy(req.context, image,"},{"line_number":562,"context_line":"                                      self.policy).get_image()"},{"line_number":563,"context_line":"            return image"}],"source_content_type":"text/x-python","patch_set":18,"id":"50171401_803c8f7c","line":560,"range":{"start_line":560,"start_character":12,"end_line":560,"end_character":44},"updated":"2021-07-23 16:55:32.000000000","message":"IF RBAC is enabled then at this line db layer readonly check returns forbidden which will be suppressed to 404 in db layer itself and returned. Which means default policy check will never be enforced.\n\nSame goes with update call also.","commit_id":"46d9f27cadf07bee52c35a082b6e4b256a4c04b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8d8ad2fa44f04a74e9adb4e317beddb857cf841e","unresolved":true,"context_lines":[{"line_number":557,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":558,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":559,"context_line":"        try:"},{"line_number":560,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":561,"context_line":"            api_policy.ImageAPIPolicy(req.context, image,"},{"line_number":562,"context_line":"                                      self.policy).get_image()"},{"line_number":563,"context_line":"            return image"}],"source_content_type":"text/x-python","patch_set":18,"id":"2a15c306_29e37e4f","line":560,"range":{"start_line":560,"start_character":12,"end_line":560,"end_character":44},"in_reply_to":"15b53e67_6b051a51","updated":"2021-07-26 14:03:38.000000000","message":"Right, so when we remove that (or disable it for secure rbac) we\u0027ll get the image back and get a proper Forbdiden from ImageAPIPolicy, which means we\u0027ll start running the exception handler at L564, which I expect we are *not* ever hitting today.","commit_id":"46d9f27cadf07bee52c35a082b6e4b256a4c04b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"406394c408d4c0ad008ec57b3ff2a143432414a1","unresolved":true,"context_lines":[{"line_number":557,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":558,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":559,"context_line":"        try:"},{"line_number":560,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":561,"context_line":"            api_policy.ImageAPIPolicy(req.context, image,"},{"line_number":562,"context_line":"                                      self.policy).get_image()"},{"line_number":563,"context_line":"            return image"}],"source_content_type":"text/x-python","patch_set":18,"id":"aa5dae3b_5e25ea30","line":560,"range":{"start_line":560,"start_character":12,"end_line":560,"end_character":44},"in_reply_to":"50171401_803c8f7c","updated":"2021-07-26 13:39:15.000000000","message":"Right, so I\u0027m guessing that we never actually hit the raise on L567 currently. I wonder if I should go ahead and change it to 404 now so that it\u0027s right for when the DB bit changes?","commit_id":"46d9f27cadf07bee52c35a082b6e4b256a4c04b7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"99c54833472889561fe2892b9e0124bf04f743fc","unresolved":true,"context_lines":[{"line_number":557,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context,"},{"line_number":558,"context_line":"                                           authorization_layer\u003dFalse)"},{"line_number":559,"context_line":"        try:"},{"line_number":560,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":561,"context_line":"            api_policy.ImageAPIPolicy(req.context, image,"},{"line_number":562,"context_line":"                                      self.policy).get_image()"},{"line_number":563,"context_line":"            return image"}],"source_content_type":"text/x-python","patch_set":18,"id":"15b53e67_6b051a51","line":560,"range":{"start_line":560,"start_character":12,"end_line":560,"end_character":44},"in_reply_to":"aa5dae3b_5e25ea30","updated":"2021-07-26 14:01:03.000000000","message":"I think in case of secured_rbac we don\u0027t need db read-only check to be executed.","commit_id":"46d9f27cadf07bee52c35a082b6e4b256a4c04b7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"cf6af2bc011c968f45427b70e5fc2797f96bdd04","unresolved":true,"context_lines":[{"line_number":561,"context_line":"            api_policy.ImageAPIPolicy(req.context, image,"},{"line_number":562,"context_line":"                                      self.policy).get_image()"},{"line_number":563,"context_line":"            return image"},{"line_number":564,"context_line":"        except webob.exc.HTTPForbidden:"},{"line_number":565,"context_line":"            LOG.debug(\"User not permitted to show image \u0027%s\u0027\", image_id)"},{"line_number":566,"context_line":"            raise webob.exc.HTTPNotFound()"},{"line_number":567,"context_line":"        except exception.NotFound as e:"},{"line_number":568,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.msg)"},{"line_number":569,"context_line":"        except exception.NotAuthenticated as e:"}],"source_content_type":"text/x-python","patch_set":22,"id":"80c6fec5_0df1002a","line":566,"range":{"start_line":564,"start_character":8,"end_line":566,"end_character":42},"updated":"2021-08-04 06:10:00.000000000","message":"I think you are already converting Forbidden to NotFound here,\nhttps://review.opendev.org/c/openstack/glance/+/796067/22/glance/api/v2/policy.py#72\n\nSo this is not required now.","commit_id":"1c3e1003e7b3b84a699ad6550b6fc52a07d8974c"}],"glance/api/v2/policy.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad4aab4cff55f87ede78943b1ced7881e6ea489e","unresolved":true,"context_lines":[{"line_number":68,"context_line":"        except exception.Forbidden as e:"},{"line_number":69,"context_line":"            raise webob.exc.HTTPForbidden(explanation\u003dstr(e))"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def check(self, name, *args):"},{"line_number":72,"context_line":"        \"\"\"Perform a soft check of a named policy."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        This is used when you need to check if a policy is allowed for the"},{"line_number":75,"context_line":"        given image, without needing to catch an exception. If the policy"},{"line_number":76,"context_line":"        check requires args, those are accepted here as well."},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        :param name: Policy name to check"},{"line_number":79,"context_line":"        :returns: bool indicating if the policy is allowed."},{"line_number":80,"context_line":"        \"\"\""},{"line_number":81,"context_line":"        try:"},{"line_number":82,"context_line":"            getattr(self, name)(*args)"},{"line_number":83,"context_line":"            return True"},{"line_number":84,"context_line":"        except webob.exc.HTTPForbidden:"},{"line_number":85,"context_line":"            return False"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def update_property(self, name, value):"},{"line_number":88,"context_line":"        if name \u003d\u003d \u0027visibility\u0027:"}],"source_content_type":"text/x-python","patch_set":19,"id":"a1949493_792fcdcd","line":85,"range":{"start_line":71,"start_character":4,"end_line":85,"end_character":24},"updated":"2021-07-27 07:52:17.000000000","message":"I think we have same implementation in base class (At line #37), so this is not needed.","commit_id":"cf3995cebf4f89e6bff68380adfc7cd225f9285f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8af1fb908e5c877cbbc08cc1efa4c6b1e220f8d","unresolved":true,"context_lines":[{"line_number":68,"context_line":"        except exception.Forbidden as e:"},{"line_number":69,"context_line":"            raise webob.exc.HTTPForbidden(explanation\u003dstr(e))"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def check(self, name, *args):"},{"line_number":72,"context_line":"        \"\"\"Perform a soft check of a named policy."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        This is used when you need to check if a policy is allowed for the"},{"line_number":75,"context_line":"        given image, without needing to catch an exception. If the policy"},{"line_number":76,"context_line":"        check requires args, those are accepted here as well."},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        :param name: Policy name to check"},{"line_number":79,"context_line":"        :returns: bool indicating if the policy is allowed."},{"line_number":80,"context_line":"        \"\"\""},{"line_number":81,"context_line":"        try:"},{"line_number":82,"context_line":"            getattr(self, name)(*args)"},{"line_number":83,"context_line":"            return True"},{"line_number":84,"context_line":"        except webob.exc.HTTPForbidden:"},{"line_number":85,"context_line":"            return False"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def update_property(self, name, value):"},{"line_number":88,"context_line":"        if name \u003d\u003d \u0027visibility\u0027:"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa7c3154_1f8867ea","line":85,"range":{"start_line":71,"start_character":4,"end_line":85,"end_character":24},"in_reply_to":"a1949493_792fcdcd","updated":"2021-07-27 13:37:50.000000000","message":"Yeah, git just merged this in on rebase and I didn\u0027t notice.","commit_id":"cf3995cebf4f89e6bff68380adfc7cd225f9285f"}],"glance/tests/etc/policy.yaml":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1cdb71240699e27b0ac14633e9e293faa53c701f","unresolved":true,"context_lines":[{"line_number":28,"context_line":"                                  project_id:%(member_id)s or"},{"line_number":29,"context_line":"                                  \"community\":%(visibility)s or"},{"line_number":30,"context_line":"                                  \"shared\":%(visibility)s or"},{"line_number":31,"context_line":"                                  \"public\":%(visibility)s))"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# modify_image"},{"line_number":34,"context_line":"\"modify_image\": \"role:admin or (role:member and project_id:%(project_id)s)\""}],"source_content_type":"text/x-yaml","patch_set":1,"id":"e9501039_87e8654f","line":31,"updated":"2021-06-11 21:41:29.000000000","message":"So, I think our new defaults are actually wrong. I wanted to make this identical to what we have, but those don\u0027t include member images or the other states here. So I think we need to fix those.","commit_id":"3019662be8fc2aed6994ccb358266053d29c7f67"}]}