)]}'
{"cinder/policies/attachments.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"e6a8a72052c1df013e86310787dba0cfb8b74204","unresolved":false,"context_lines":[{"line_number":50,"context_line":"attachments_policies \u003d ["},{"line_number":51,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":52,"context_line":"        name\u003dCREATE_POLICY,"},{"line_number":53,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":54,"context_line":"        scope_types\u003d[\u0027project\u0027],"},{"line_number":55,"context_line":"        description\u003d\"Create attachment.\","},{"line_number":56,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"3f65232a_77a898da","line":53,"range":{"start_line":53,"start_character":23,"end_line":53,"end_character":38},"updated":"2020-10-27 20:39:26.000000000","message":"Do we only expect project users to call this API? Or do we expect system administrators to be able to create attachments (using system-scoped tokens that aren\u0027t associated to a project)?","commit_id":"b05998e1adc0582a31908eef4f4bc9ef11fffafd"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"e6a8a72052c1df013e86310787dba0cfb8b74204","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    ),"},{"line_number":66,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":67,"context_line":"        name\u003dUPDATE_POLICY,"},{"line_number":68,"context_line":"        check_str\u003dbase.SYSTEM_OR_PROJECT_MEMBER,"},{"line_number":69,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":70,"context_line":"        description\u003d\"Update attachment.\","},{"line_number":71,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"3f65232a_17b9642d","line":68,"range":{"start_line":68,"start_character":23,"end_line":68,"end_character":47},"updated":"2020-10-27 20:39:26.000000000","message":"Should this be system-admin or system-member? I used system-member since it\u0027s more includsive and still allows system-administrators to call the API.","commit_id":"b05998e1adc0582a31908eef4f4bc9ef11fffafd"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"c04d3f8bf1ae7e184658ccfe3e72474efff44086","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        deprecated_reason\u003dDEPRECATED_REASON,"},{"line_number":94,"context_line":"        deprecated_since\u003dversionutils.deprecated.WALLABY"},{"line_number":95,"context_line":"    )"},{"line_number":96,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":97,"context_line":"        name\u003dCOMPLETE_POLICY,"},{"line_number":98,"context_line":"        check_str\u003dbase.SYSTEM_OR_PROJECT_MEMBER,"},{"line_number":99,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_9d08f52d","line":96,"updated":"2020-10-28 01:43:27.000000000","message":"pep8: E999 SyntaxError: invalid syntax","commit_id":"153102f3cfe7279d4d674f9adacadbb444d3bb00"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"525945131179aa0985b202515c68023a0dc94fee","unresolved":true,"context_lines":[{"line_number":30,"context_line":"\"\"\""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"deprecated_create_policy \u003d policy.DeprecatedRule("},{"line_number":33,"context_line":"    name\u003dCREATE_POLICY, check_str\u003d\"\""},{"line_number":34,"context_line":")"},{"line_number":35,"context_line":"deprecated_update_policy \u003d policy.DeprecatedRule("},{"line_number":36,"context_line":"    name\u003dUPDATE_POLICY, check_str\u003dbase.RULE_ADMIN_OR_OWNER"}],"source_content_type":"text/x-python","patch_set":7,"id":"2b3c034a_07818657","line":33,"range":{"start_line":33,"start_character":35,"end_line":33,"end_character":36},"updated":"2021-08-03 20:14:16.000000000","message":"you can add \n\n deprecated_reason\n deprecated_since \n\nto this DeprecatedRule instead of to the DocumentedRuleDefault below: https://docs.openstack.org/oslo.policy/wallaby/reference/api/oslo_policy.html#oslo_policy.policy.DeprecatedRule","commit_id":"f855f210979fc3ebac048ab3e416cdad0b123d65"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a480a051d519d89eced112494e74f7f15efa49e0","unresolved":true,"context_lines":[{"line_number":64,"context_line":"    base.CinderDocumentedRuleDefault("},{"line_number":65,"context_line":"        name\u003dUPDATE_POLICY,"},{"line_number":66,"context_line":"        check_str\u003dbase.SYSTEM_ADMIN_OR_PROJECT_MEMBER,"},{"line_number":67,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":68,"context_line":"        description\u003d\"Update attachment.\","},{"line_number":69,"context_line":"        operations\u003d["},{"line_number":70,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa2192c2_553a391a","line":67,"range":{"start_line":67,"start_character":22,"end_line":67,"end_character":28},"updated":"2021-08-09 16:34:10.000000000","message":"For now, this won\u0027t work with system-scope because we have to relax cinder API code to allow for that, right?\n\nIn addition to some python-cinderclient code to pass fake project IDs for system-scoped requests?","commit_id":"6662f6fb675574db6bd43af61733f4f9a9109d65"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c15b952991d3673abcc7515fdc97e7ef0c2bbd80","unresolved":true,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"attachments_policies \u003d ["},{"line_number":51,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":52,"context_line":"        name\u003dCREATE_POLICY,"},{"line_number":53,"context_line":"        check_str\u003dbase.SYSTEM_ADMIN_OR_PROJECT_MEMBER,"},{"line_number":54,"context_line":"        scope_types\u003d[\u0027project\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"e9731b2b_3341963a","line":51,"range":{"start_line":51,"start_character":4,"end_line":51,"end_character":32},"updated":"2021-08-20 20:43:13.000000000","message":"Should this be base.CinderDocumentedRuleDefault ?","commit_id":"08ebd9584d2f1aac978901928a14112df3fa269b"}],"cinder/tests/unit/api/v3/test_attachments.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"5499489f80dbef9e5a61e3ca2f4195cea01f2205","unresolved":true,"context_lines":[{"line_number":144,"context_line":"                          self.controller.update, req,"},{"line_number":145,"context_line":"                          self.attachment1.id, body\u003dbody)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @mock.patch(\u0027cinder.coordination.synchronized\u0027)"},{"line_number":148,"context_line":"    @mock.patch.object(objects.VolumeAttachment, \u0027get_by_id\u0027)"},{"line_number":149,"context_line":"    def test_attachment_operations_not_authorized(self, mock_get, mock_synch):"},{"line_number":150,"context_line":"        mock_get.return_value \u003d self.attachment1"}],"source_content_type":"text/x-python","patch_set":18,"id":"4eee0fb9_34673ae9","side":"PARENT","line":147,"updated":"2021-09-03 18:37:49.000000000","message":"So long as these pass, we can always clean them up later. Just a thought to make the refactor a little less invasive.","commit_id":"81e0da35dc04ebd1409b401f02f2d3455ac98b27"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"ad04354fe0f02044af20db59389367b6269f15b0","unresolved":true,"context_lines":[{"line_number":144,"context_line":"                          self.controller.update, req,"},{"line_number":145,"context_line":"                          self.attachment1.id, body\u003dbody)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @mock.patch(\u0027cinder.coordination.synchronized\u0027)"},{"line_number":148,"context_line":"    @mock.patch.object(objects.VolumeAttachment, \u0027get_by_id\u0027)"},{"line_number":149,"context_line":"    def test_attachment_operations_not_authorized(self, mock_get, mock_synch):"},{"line_number":150,"context_line":"        mock_get.return_value \u003d self.attachment1"}],"source_content_type":"text/x-python","patch_set":18,"id":"358b61b8_7a1d7ceb","side":"PARENT","line":147,"in_reply_to":"4eee0fb9_34673ae9","updated":"2021-09-03 18:41:00.000000000","message":"I guess, but feared they would start to fail as we move to Y and beyond, and the tests are superseded (I hope!) by the new ones in this patch.","commit_id":"81e0da35dc04ebd1409b401f02f2d3455ac98b27"}],"cinder/tests/unit/policies/test_attachments.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"0f4e70294c4190236052fb9cb4246a43adcc76b0","unresolved":true,"context_lines":[{"line_number":98,"context_line":"            }"},{"line_number":99,"context_line":"        }"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # Some context return HTTP 404 (rather than 403)."},{"line_number":102,"context_line":"        unauthorized_exceptions \u003d ["},{"line_number":103,"context_line":"            exception.VolumeNotFound,"},{"line_number":104,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f500fb9_bc8470de","line":101,"updated":"2021-08-24 22:11:17.000000000","message":"I imagine an 404 is returned when the user isn\u0027t allowed to view the attachment (e.g., an attachment for project foo when the user is only authorized to work on project bar). And the 403 is when a user is allowed to see the attachment, but doesn\u0027t have sufficient permission to do the operation, right?","commit_id":"00315df2ebb99b55cc02278ce925f8a0cb398b61"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"dcf9bda551b4477ba8a1e95d7e35273c910c6c03","unresolved":true,"context_lines":[{"line_number":98,"context_line":"            }"},{"line_number":99,"context_line":"        }"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # Some context return HTTP 404 (rather than 403)."},{"line_number":102,"context_line":"        unauthorized_exceptions \u003d ["},{"line_number":103,"context_line":"            exception.VolumeNotFound,"},{"line_number":104,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":14,"id":"c33c76b9_28d7f3da","line":101,"in_reply_to":"1f500fb9_bc8470de","updated":"2021-08-24 22:39:04.000000000","message":"Not quite. The 404 situation arises when the code attempts to fetch the volume for which the attachment would be created, and cinder\u0027s DB layer throws VolumeNotFound. You see [1] occurs before the CREATE_POLCY is even checked, and ultimately the DB access fails due to [2].\n\n[1] https://opendev.org/openstack/cinder/src/branch/master/cinder/api/v3/attachments.py#L178\n[2] https://opendev.org/openstack/cinder/src/branch/master/cinder/db/sqlalchemy/api.py#L186\n\nThis is due to cinder having access checks at the DB layer that predate the policy-in-code.\n\nThe 403 will occur if the user has access to see the volume, but fails the CHECK_POLICY. I think you\u0027ll actually only see that in the AttachmentsPolicySecureRbacTest suite, where enforce_new_defaults is True and you\u0027re just a project_reader.","commit_id":"00315df2ebb99b55cc02278ce925f8a0cb398b61"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a858a01b456221fc674c89ac294b63c301dca2f0","unresolved":true,"context_lines":[{"line_number":98,"context_line":"            }"},{"line_number":99,"context_line":"        }"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # Some context return HTTP 404 (rather than 403)."},{"line_number":102,"context_line":"        unauthorized_exceptions \u003d ["},{"line_number":103,"context_line":"            exception.VolumeNotFound,"},{"line_number":104,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":14,"id":"dd9cdca0_837cbdc4","line":101,"in_reply_to":"c33c76b9_28d7f3da","updated":"2021-08-25 13:28:10.000000000","message":"\u003e Not quite. The 404 situation arises when the code attempts to fetch the volume for which the attachment would be created, and cinder\u0027s DB layer throws VolumeNotFound. You see [1] occurs before the CREATE_POLCY is even checked, and ultimately the DB access fails due to [2].\n\nOk - so if a user has a role assignment on foo but they try to create an attachment for a volume in bar, it will fail with a 404 because the database API utility method filters the volume.project_id to use the context.project_id [0], right? That throws a 404 because the user isn\u0027t authorized to view that volume since it\u0027s outside the project they have authorization on.\n\nJust making sure I understand the different cases that account for a 404 versus a 403.\n\n[0] https://opendev.org/openstack/cinder/src/branch/master/cinder/db/sqlalchemy/api.py#L7176-L7177\n\n\u003e \n\u003e [1] https://opendev.org/openstack/cinder/src/branch/master/cinder/api/v3/attachments.py#L178\n\u003e [2] https://opendev.org/openstack/cinder/src/branch/master/cinder/db/sqlalchemy/api.py#L186\n\u003e \n\u003e This is due to cinder having access checks at the DB layer that predate the policy-in-code.\n\u003e \n\u003e The 403 will occur if the user has access to see the volume, but fails the CHECK_POLICY. I think you\u0027ll actually only see that in the AttachmentsPolicySecureRbacTest suite, where enforce_new_defaults is True and you\u0027re just a project_reader.","commit_id":"00315df2ebb99b55cc02278ce925f8a0cb398b61"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"61163d9c516cb0d88e7a462321e3aea6cc3a6e1c","unresolved":true,"context_lines":[{"line_number":98,"context_line":"            }"},{"line_number":99,"context_line":"        }"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # Some context return HTTP 404 (rather than 403)."},{"line_number":102,"context_line":"        unauthorized_exceptions \u003d ["},{"line_number":103,"context_line":"            exception.VolumeNotFound,"},{"line_number":104,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":14,"id":"93ed6929_2365799e","line":101,"in_reply_to":"dd9cdca0_837cbdc4","updated":"2021-08-25 14:47:34.000000000","message":"Yes, your understanding is correct. User in project foo cannot access a volume in project bar, and DB layer throws VolumeNotFound before any policy checks take place.","commit_id":"00315df2ebb99b55cc02278ce925f8a0cb398b61"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c427f26c2bfa0c0ab4b500f384a7ea3aafc97ba5","unresolved":true,"context_lines":[{"line_number":121,"context_line":"        rule_name \u003d attachments_policies.UPDATE_POLICY"},{"line_number":122,"context_line":"        attachment_id \u003d self._create_attachment()"},{"line_number":123,"context_line":"        url \u003d \u0027%s/%s\u0027 % (self.api_path, attachment_id)"},{"line_number":124,"context_line":"        req \u003d fake_api.HTTPRequest.blank(url, version\u003dself.api_version)"},{"line_number":125,"context_line":"        body \u003d {"},{"line_number":126,"context_line":"            \"attachment\": {"},{"line_number":127,"context_line":"                \"connector\": {"}],"source_content_type":"text/x-python","patch_set":18,"id":"20e6182f_9214a676","line":124,"updated":"2021-09-13 17:05:19.000000000","message":"I think we\u0027re missing the req.method \u003d \u0027PUT\u0027 ?","commit_id":"3cedf7d81cb7e40383502bd89d380c9a3879062a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f89ddf0192e1e6f571ca2ba0654b5a66364a8c28","unresolved":true,"context_lines":[{"line_number":121,"context_line":"        rule_name \u003d attachments_policies.UPDATE_POLICY"},{"line_number":122,"context_line":"        attachment_id \u003d self._create_attachment()"},{"line_number":123,"context_line":"        url \u003d \u0027%s/%s\u0027 % (self.api_path, attachment_id)"},{"line_number":124,"context_line":"        req \u003d fake_api.HTTPRequest.blank(url, version\u003dself.api_version)"},{"line_number":125,"context_line":"        body \u003d {"},{"line_number":126,"context_line":"            \"attachment\": {"},{"line_number":127,"context_line":"                \"connector\": {"}],"source_content_type":"text/x-python","patch_set":18,"id":"3972c854_b1910081","line":124,"in_reply_to":"20e6182f_9214a676","updated":"2021-09-13 18:49:20.000000000","message":"Yes, though we don\u0027t really need it because instead of using the mapper, we are directly passing the request to the controller method.  On the other hand, the method is stored in the request and is available to the controller to inspect (though our code isn\u0027t currently doing that here).  Would it be ok if i add it (and the DELETE below) in a follow-up patch?","commit_id":"3cedf7d81cb7e40383502bd89d380c9a3879062a"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c2925b77acd55fd69b4799fee5d037e7eae540ca","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        rule_name \u003d attachments_policies.UPDATE_POLICY"},{"line_number":122,"context_line":"        attachment_id \u003d self._create_attachment()"},{"line_number":123,"context_line":"        url \u003d \u0027%s/%s\u0027 % (self.api_path, attachment_id)"},{"line_number":124,"context_line":"        req \u003d fake_api.HTTPRequest.blank(url, version\u003dself.api_version)"},{"line_number":125,"context_line":"        body \u003d {"},{"line_number":126,"context_line":"            \"attachment\": {"},{"line_number":127,"context_line":"                \"connector\": {"}],"source_content_type":"text/x-python","patch_set":18,"id":"02fd0834_b8029d4a","line":124,"in_reply_to":"3972c854_b1910081","updated":"2021-09-13 18:52:28.000000000","message":"sure","commit_id":"3cedf7d81cb7e40383502bd89d380c9a3879062a"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c427f26c2bfa0c0ab4b500f384a7ea3aafc97ba5","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        rule_name \u003d attachments_policies.DELETE_POLICY"},{"line_number":158,"context_line":"        attachment_id \u003d self._create_attachment()"},{"line_number":159,"context_line":"        url \u003d \u0027%s/%s\u0027 % (self.api_path, attachment_id)"},{"line_number":160,"context_line":"        req \u003d fake_api.HTTPRequest.blank(url, version\u003dself.api_version)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        unauthorized_exceptions \u003d []"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"b3483e92_2c99ec16","line":160,"updated":"2021-09-13 17:05:19.000000000","message":"same here, are we missing req.method \u003d \u0027DELETE\u0027?","commit_id":"3cedf7d81cb7e40383502bd89d380c9a3879062a"}]}