)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e6089a8115866803199338a8433b39ea1fa03ba3","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enforce scope check always when rule has scope_types set"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Previously it was checked only for rules which are subclasses of the"},{"line_number":10,"context_line":"BaseCheck class."},{"line_number":11,"context_line":"Now it\u0027s checked for all rules which have scope_types attribute."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-bug: #1923503"},{"line_number":14,"context_line":"Change-Id: I55258c1f999c84220518d1fbbf5e1e514361cebe"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"866da65f_79abdf79","line":11,"range":{"start_line":9,"start_character":0,"end_line":11,"end_character":0},"updated":"2021-08-19 19:57:53.000000000","message":"this is opposite right? previously it is only checked for non-BaseCheck-subclass.","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"06bafd0c3fd01bd7c937947c2b35769d97bd22ed","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enforce scope check always when rule has scope_types set"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Previously it was checked only for rules which are subclasses of the"},{"line_number":10,"context_line":"BaseCheck class."},{"line_number":11,"context_line":"Now it\u0027s checked for all rules which have scope_types attribute."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-bug: #1923503"},{"line_number":14,"context_line":"Change-Id: I55258c1f999c84220518d1fbbf5e1e514361cebe"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"f900b173_50653eeb","line":11,"range":{"start_line":9,"start_character":0,"end_line":11,"end_character":0},"in_reply_to":"866da65f_79abdf79","updated":"2021-09-06 10:18:18.000000000","message":"Done","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c160d4af6dadee593f700cfd636a05c2fe10b7a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"517e04d9_b8633f5e","updated":"2021-10-12 14:04:42.000000000","message":"can we add a reno also here for https://review.opendev.org/c/openstack/oslo.policy/+/812468/1 too to notify that BaseCheck has ability to add scope_type now.","commit_id":"8a544be515342953360bbfb56f9ac71e134bc4e5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"def8d317ec4874ce29bd6ee15a5a2a06d92b4d4e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c0a5c014_e4fecdec","in_reply_to":"517e04d9_b8633f5e","updated":"2021-10-25 14:51:04.000000000","message":"I added reno.","commit_id":"8a544be515342953360bbfb56f9ac71e134bc4e5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9b0b8c5397a94bcca5ee3593dfb7ee1dde176d54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"63150de2_33270c33","updated":"2021-11-19 14:30:44.000000000","message":"I need to investigate that timeout in Neutron UT job","commit_id":"02cad7203822acb405b4fe2d57b594be332c81ae"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e94247e69ccd9b71629e83b11b15997027163575","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ce87f8d8_08ab4469","updated":"2021-11-18 00:19:03.000000000","message":"it seems neutron tox py38 job is timeout consistently \n\n- https://zuul.opendev.org/t/openstack/build/a32356dd7ad5448badda26e02469fbaa/log/job-output.txt#24762\n\nis it failing on neutron side too ?","commit_id":"02cad7203822acb405b4fe2d57b594be332c81ae"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4534503cf57f7e327cde9ba6399c54fd060449f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"558216a4_5bfc208f","updated":"2021-11-18 00:19:07.000000000","message":"recheck","commit_id":"02cad7203822acb405b4fe2d57b594be332c81ae"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"3e8b4c2a4f642ff9de0c6423b9642ec5e1ad52ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d440a85c_a9235644","updated":"2021-11-17 09:56:25.000000000","message":"recheck","commit_id":"02cad7203822acb405b4fe2d57b594be332c81ae"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"52832c13d3561be90067e79fc7fe56539a0fce53","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d8762332_220f4ad1","updated":"2021-10-28 22:55:57.000000000","message":"thanks. lgtm","commit_id":"02cad7203822acb405b4fe2d57b594be332c81ae"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7fc15ce370a8c077fc519bbab35a2a10ae418f18","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a51c8d86_fed01e24","in_reply_to":"63150de2_33270c33","updated":"2021-11-19 15:17:13.000000000","message":"i am just wondering why it is happening in neutron gate","commit_id":"02cad7203822acb405b4fe2d57b594be332c81ae"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"35c7e1653ae3542217583b08a152aa04ce472cdc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"12a6e252_0eeab64b","updated":"2021-11-24 21:46:05.000000000","message":"The change looks reasonable, but I\u0027m curious about the time out. We hit another performance issues in earlier versions of oslo.policy that were only applicable to neutron.","commit_id":"919c3280aa79762df8475f131a65d12b78ac436e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0e87af7279288bdbc3d4ea92595be2ddba981ded","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e33179f9_44fd4785","updated":"2021-11-24 22:40:21.000000000","message":"lgtm","commit_id":"919c3280aa79762df8475f131a65d12b78ac436e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b7bcc6db0bbe8e3283e104f450dfce13288a3a5b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"909ddaa4_d8c2b16a","updated":"2021-11-26 18:05:05.000000000","message":"recheck","commit_id":"919c3280aa79762df8475f131a65d12b78ac436e"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"49429e6ede379403ae2acbbdc58e6fe4fe882028","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"877438f5_b88c7972","updated":"2021-12-13 09:18:02.000000000","message":"recheck\nWhy it was not merged?","commit_id":"919c3280aa79762df8475f131a65d12b78ac436e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0e87af7279288bdbc3d4ea92595be2ddba981ded","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"253f19ba_85016c22","in_reply_to":"12a6e252_0eeab64b","updated":"2021-11-24 22:40:21.000000000","message":"may be we did not increase the timeout here but in neutron it is 3600 for all unit test job https://github.com/openstack/neutron/blob/770b64b90e0ada90a40538830002fc447a8dee9c/zuul.d/project.yaml#L46","commit_id":"919c3280aa79762df8475f131a65d12b78ac436e"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"be66cc119c70d358368dcfc76d2c77074e2c4fe8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"78b87819_2d778a33","in_reply_to":"49e8c65a_e3ad4bb9","updated":"2021-12-13 10:04:49.000000000","message":"thanks, I see now","commit_id":"919c3280aa79762df8475f131a65d12b78ac436e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"47529b4503aab5d264de821b0223c4e88ea88999","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"49e8c65a_e3ad4bb9","in_reply_to":"877438f5_b88c7972","updated":"2021-12-13 09:33:48.000000000","message":"Because one of the Depends-On patches isn\u0027t merged yet - https://review.opendev.org/c/openstack/neutron/+/818725","commit_id":"919c3280aa79762df8475f131a65d12b78ac436e"}],"oslo_policy/policy.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"062539bcf51d6abdc71d0bc2a03c1e65cb7d9fb4","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"65e258f7_5ba73d3a","line":1007,"updated":"2021-08-18 19:40:24.000000000","message":"We should make sure we add a test to ensure this actually raise an exception for subclassed BaseChecks","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"06bafd0c3fd01bd7c937947c2b35769d97bd22ed","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2a77db4a_425ef2b5","line":1007,"in_reply_to":"13d8c1f8_4e2b7843","updated":"2021-09-06 10:18:18.000000000","message":"Yes, that\u0027s right. But also the thing is that Neutron wasn\u0027t setting scope_types for such BaseCheck rules thus this enforcer couldn\u0027t be done here too. Now I proposed also https://review.opendev.org/c/openstack/neutron/+/807559 in Neutron to set scope_types on that newly created Check objects, based on the Registered rules for the action.\n@Lance please check that Neutron patch too and tell me if that makes sense for You.","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"60dec7f2b880a157c54994b10c7514b4a01cc65f","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"365b33d4_dbb93d33","line":1007,"in_reply_to":"2a77db4a_425ef2b5","updated":"2021-09-10 20:59:15.000000000","message":"so in that case, we basically associating the scope_type in Check but from seeing the \u0027Check\u0027 object definition or its purpose it seems confusing and allow special case runtime attribute setting (which pass because of L1054). should we add scope_type in BaseCheck object itself so that we know services can set it in the case where it is treated as Rule like neutron use case?","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"914e0b5414e7eb0b6a76c1942ae386c9657f1b40","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f5c6befa_67714859","line":1007,"in_reply_to":"3fe76608_6e72e856","updated":"2021-08-19 17:57:15.000000000","message":"humm, but in this case (\u0027rule\u0027 param as a Check tree) how enforcing scope work/needed then? I am not sure what is neutron use case but interesting to know how we will define the scope type in Check based enforcement ?","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7dfd9a83f5f228edb20c4cc48de852cef423b272","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8a839be0_d9cc77e3","line":1007,"in_reply_to":"5ff5ac57_a41afdfd","updated":"2021-08-18 23:06:38.000000000","message":"+1 on test. in your example you did not add the scope_type in NotCheck rule which is one condition at L1054.\n\nI think neutron case is having scope_type of subclassed BaseChecks ?","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"bc20bbd7bae667f459d6579b061e8adb42d02a8c","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff5ac57_a41afdfd","line":1007,"in_reply_to":"65e258f7_5ba73d3a","updated":"2021-08-18 20:23:34.000000000","message":"Maybe something like this [0] will help.\n\nI tried using the NotCheck to see if that would be simple enough, but that didn\u0027t quite work as I was expecting. We might need to try something closer to what neutron is actually doing.\n\n[0] https://paste.opendev.org/show/808187/","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7fa0edca0d1db4e42bdec90c513d80e3d4c27948","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fe76608_6e72e856","line":1007,"in_reply_to":"8a839be0_d9cc77e3","updated":"2021-08-19 16:50:25.000000000","message":"I don\u0027t think you can associate scope types to a check, can you? It looks like the scope types are associated to the rule.","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"722a505b224f4b301fa72ce8d241a55e79091e8c","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"13d8c1f8_4e2b7843","line":1007,"in_reply_to":"ec7fe322_f98462a9","updated":"2021-08-19 20:33:39.000000000","message":"I was having a hard time following the implementation yesterday.\n\nThe use-case is a little easier to explain. To experience the problem, all you have to do is configure neutron to use `neutron.conf [oslo_policy] enforce_scope \u003d True` and start using project-admin tokens for system-admin APIs. You\u0027ll see that project-admins are allowed to do things they shouldn\u0027t be allowed to do because the scope_types check is short-circuited.\n\nThis is because neutron subclasses the Check object, and ultimately the BaseCheck object, allowing checks in neutron to hit this code path.\n\nSo the use case is to continue letting neutron implement their custom checks [0][1], while maintaining scope enforcement using scope_types on the rules. This check might fall into that category since it\u0027s using rule:network_owner, which leverages the FieldCheck class in it\u0027s check string [2].\n\nSlawek will have to keep me honest here about what\u0027s actually going on. I could be standing in left field by my self 😊\n\n[0] https://github.com/openstack/neutron/blob/master/neutron/policy.py#L249\n[1] https://github.com/openstack/neutron/blob/master/neutron/policy.py#L369\n[2] https://github.com/openstack/neutron/blob/master/neutron/conf/policies/port.py#L103-L122","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e6089a8115866803199338a8433b39ea1fa03ba3","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ec7fe322_f98462a9","line":1007,"in_reply_to":"f11f7fef_c6d1b8e6","updated":"2021-08-19 19:57:53.000000000","message":"and that object has scope_check? that is why we need to add the scope enforcement here?. so basically it is associating the scope_type in checks. May be I am little confused with the use case.","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"fced2405f762fa4e55bdc228f2a6caf3abb3419b","unresolved":true,"context_lines":[{"line_number":1004,"context_line":"                      else \u0027\"%s\"\u0027 % rule, creds_msg, target_msg)"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":"        # Allow the rule to be a Check tree"},{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f11f7fef_c6d1b8e6","line":1007,"in_reply_to":"f5c6befa_67714859","updated":"2021-08-19 19:05:47.000000000","message":"IIUC neutron subclasses the BaseCheck object, which makes it an instance of that object and it executes in this if statement, which doesn\u0027t do anything which scope types.\n\nTraditionally, the rule is a string which hits the else check below and that\u0027s where we originally implemented scope type checks.","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"60dec7f2b880a157c54994b10c7514b4a01cc65f","unresolved":true,"context_lines":[{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"},{"line_number":1011,"context_line":"            result \u003d _checks._check("},{"line_number":1012,"context_line":"                rule\u003drule,"},{"line_number":1013,"context_line":"                target\u003dtarget,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5de049f0_3f1614ce","line":1010,"range":{"start_line":1010,"start_character":0,"end_line":1010,"end_character":44},"updated":"2021-09-10 20:59:15.000000000","message":"instead of L1054, we can check here if rule.scope_type is set or not. and let _enforce_scope() method make sure it check the scope type if it is called. Otherwise L1054 can give silent pass when registered rule itself forget to set scope_type.","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e1e5aefae5a7f056e8230bf660decec6fce4f7e8","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"        if isinstance(rule, _checks.BaseCheck):"},{"line_number":1008,"context_line":"            # If the thing we\u0027re given is a Check, we don\u0027t know the"},{"line_number":1009,"context_line":"            # name of the rule, so pass None for current_rule."},{"line_number":1010,"context_line":"            self._enforce_scope(creds, rule)"},{"line_number":1011,"context_line":"            result \u003d _checks._check("},{"line_number":1012,"context_line":"                rule\u003drule,"},{"line_number":1013,"context_line":"                target\u003dtarget,"}],"source_content_type":"text/x-python","patch_set":1,"id":"810554fe_c7d8c8cd","line":1010,"range":{"start_line":1010,"start_character":0,"end_line":1010,"end_character":44},"in_reply_to":"5de049f0_3f1614ce","updated":"2021-10-05 09:21:16.000000000","message":"Done","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"60dec7f2b880a157c54994b10c7514b4a01cc65f","unresolved":true,"context_lines":[{"line_number":1051,"context_line":"        return result"},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"    def _enforce_scope(self, creds, rule):"},{"line_number":1054,"context_line":"        if not hasattr(rule, \u0027scope_types\u0027):"},{"line_number":1055,"context_line":"            return"},{"line_number":1056,"context_line":"        # Check the scope of the operation against the possible scope"},{"line_number":1057,"context_line":"        # attributes provided in `creds`."},{"line_number":1058,"context_line":"        if creds.get(\u0027system\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"61f6ab2c_7f2eecd8","line":1055,"range":{"start_line":1054,"start_character":0,"end_line":1055,"end_character":18},"updated":"2021-09-10 20:59:15.000000000","message":"I think we should not have this check which can give false/silent pass if by mistake we forget the scope_type in registered rule itself. and for BaseChecks type rule we check this condition at L1010 itself.","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e1e5aefae5a7f056e8230bf660decec6fce4f7e8","unresolved":false,"context_lines":[{"line_number":1051,"context_line":"        return result"},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"    def _enforce_scope(self, creds, rule):"},{"line_number":1054,"context_line":"        if not hasattr(rule, \u0027scope_types\u0027):"},{"line_number":1055,"context_line":"            return"},{"line_number":1056,"context_line":"        # Check the scope of the operation against the possible scope"},{"line_number":1057,"context_line":"        # attributes provided in `creds`."},{"line_number":1058,"context_line":"        if creds.get(\u0027system\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d647560b_4fb8d995","line":1055,"range":{"start_line":1054,"start_character":0,"end_line":1055,"end_character":18},"in_reply_to":"61f6ab2c_7f2eecd8","updated":"2021-10-05 09:21:16.000000000","message":"Done","commit_id":"8f7944b4308e6f5bf38fccbc921b3091fb3a155b"},{"author":{"_account_id":2218,"name":"Adam Young","email":"adam@younglogic.com","username":"ayoung"},"change_message_id":"fa0076521089d94b2c0427a57980213b8b8cf504","unresolved":true,"context_lines":[{"line_number":1051,"context_line":"        return result"},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"    def _enforce_scope(self, creds, rule):"},{"line_number":1054,"context_line":"        if not hasattr(rule, \u0027scope_types\u0027):"},{"line_number":1055,"context_line":"            return"},{"line_number":1056,"context_line":"        # Check the scope of the operation against the possible scope"},{"line_number":1057,"context_line":"        # attributes provided in `creds`."}],"source_content_type":"text/x-python","patch_set":2,"id":"bb629dff_b7d6dd34","line":1054,"updated":"2021-09-27 16:35:27.000000000","message":"Is this line the only behavioral change in the commit?  If so, please simplify the commit to make it clearer.","commit_id":"6ed0db26d20d01188256907d3bfefb202b35d303"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e1e5aefae5a7f056e8230bf660decec6fce4f7e8","unresolved":true,"context_lines":[{"line_number":1051,"context_line":"        return result"},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"    def _enforce_scope(self, creds, rule):"},{"line_number":1054,"context_line":"        if not hasattr(rule, \u0027scope_types\u0027):"},{"line_number":1055,"context_line":"            return"},{"line_number":1056,"context_line":"        # Check the scope of the operation against the possible scope"},{"line_number":1057,"context_line":"        # attributes provided in `creds`."}],"source_content_type":"text/x-python","patch_set":2,"id":"192f72c4_6c83ecaa","line":1054,"in_reply_to":"bb629dff_b7d6dd34","updated":"2021-10-05 09:21:16.000000000","message":"I refactored it and proposed 3 patches:\n\nhttps://review.opendev.org/c/openstack/oslo.policy/+/812468\nhttps://review.opendev.org/c/openstack/oslo.policy/+/812469\nhttps://review.opendev.org/c/openstack/oslo.policy/+/804980\n\nI hope this will be better now.","commit_id":"6ed0db26d20d01188256907d3bfefb202b35d303"}],"oslo_policy/tests/test_policy.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c160d4af6dadee593f700cfd636a05c2fe10b7a0","unresolved":true,"context_lines":[{"line_number":1010,"context_line":"        ctx \u003d context.RequestContext(system_scope\u003d\u0027all\u0027, roles\u003d[\u0027admin\u0027])"},{"line_number":1011,"context_line":"        self.assertRaises("},{"line_number":1012,"context_line":"            policy.InvalidScope,"},{"line_number":1013,"context_line":"            self.enforcer.enforce, rule, {}, ctx)"},{"line_number":1014,"context_line":""},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"class EnforcerNoPolicyFileTest(base.PolicyBaseTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"27a7911f_a1a681c5","line":1013,"range":{"start_line":1013,"start_character":48,"end_line":1013,"end_character":49},"updated":"2021-10-12 14:04:42.000000000","message":"along with that, can we also add a test with no scope_types and see no scope enforcement happening?","commit_id":"8a544be515342953360bbfb56f9ac71e134bc4e5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"def8d317ec4874ce29bd6ee15a5a2a06d92b4d4e","unresolved":false,"context_lines":[{"line_number":1010,"context_line":"        ctx \u003d context.RequestContext(system_scope\u003d\u0027all\u0027, roles\u003d[\u0027admin\u0027])"},{"line_number":1011,"context_line":"        self.assertRaises("},{"line_number":1012,"context_line":"            policy.InvalidScope,"},{"line_number":1013,"context_line":"            self.enforcer.enforce, rule, {}, ctx)"},{"line_number":1014,"context_line":""},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"class EnforcerNoPolicyFileTest(base.PolicyBaseTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"eed5a47f_d38fadbf","line":1013,"range":{"start_line":1013,"start_character":48,"end_line":1013,"end_character":49},"in_reply_to":"27a7911f_a1a681c5","updated":"2021-10-25 14:51:04.000000000","message":"Done","commit_id":"8a544be515342953360bbfb56f9ac71e134bc4e5"}]}