)]}'
{"doc/source/framework/requirements_authority.rst":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"2dcec5623405c6808c0d1e0843eefbf9c76fb2ee","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        - \u003callowed_role_3\u003e"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"Must be read as ``\u003callowed_role_1\u003e and \u003callowed_role_2\u003e or \u003callowed_role_3\u003e``."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":".. code-block:: yaml"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    \u003cservice_foo\u003e:"}],"source_content_type":"text/x-rst","patch_set":5,"id":"3f79a3b5_1fc92f79","line":123,"updated":"2018-11-02 02:01:44.000000000","message":"Should add a sentence about how this is an example with negative roles.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"8e0997eecc87271618a779a48e56ee8c51649563","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        - \u003callowed_role_3\u003e"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"Must be read as ``\u003callowed_role_1\u003e and \u003callowed_role_2\u003e or \u003callowed_role_3\u003e``."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":".. code-block:: yaml"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    \u003cservice_foo\u003e:"}],"source_content_type":"text/x-rst","patch_set":5,"id":"3f79a3b5_c2e51e3f","line":123,"in_reply_to":"3f79a3b5_1fc92f79","updated":"2018-11-02 03:12:13.000000000","message":"Done","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"}],"patrole_tempest_plugin/requirements_authority.py":[{"author":{"_account_id":27078,"name":"Manik Bindlish","email":"manikbindlish19@gmail.com","username":"manik"},"change_message_id":"bc0ea34e55df20deb21805de11641b2c62812274","unresolved":false,"context_lines":[{"line_number":52,"context_line":"              \u003capi_action_a\u003e:"},{"line_number":53,"context_line":"                - \u003callowed_role_1\u003e"},{"line_number":54,"context_line":"                - \u003callowed_role_2\u003e,\u003callowed_role_3\u003e"},{"line_number":55,"context_line":"                - \u003callowed_role_3\u003e"},{"line_number":56,"context_line":"              \u003capi_action_b\u003e:"},{"line_number":57,"context_line":"                - \u003callowed_role_2\u003e"},{"line_number":58,"context_line":"                - \u003callowed_role_4\u003e"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_dae8d3e1","line":55,"range":{"start_line":55,"start_character":16,"end_line":55,"end_character":34},"updated":"2018-10-31 04:43:58.000000000","message":"I am not sure but I think \"\u003callowed_role_3\u003e\" should be remove as already used as \"and\" with \u003callowed_role_2\u003e as a new change","commit_id":"3524924fc10e95cc76d8726fd55fcb2122938b28"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"0d3dc9301c4d6827efad30838847f8c89d8316af","unresolved":false,"context_lines":[{"line_number":52,"context_line":"              \u003capi_action_a\u003e:"},{"line_number":53,"context_line":"                - \u003callowed_role_1\u003e"},{"line_number":54,"context_line":"                - \u003callowed_role_2\u003e,\u003callowed_role_3\u003e"},{"line_number":55,"context_line":"                - \u003callowed_role_3\u003e"},{"line_number":56,"context_line":"              \u003capi_action_b\u003e:"},{"line_number":57,"context_line":"                - \u003callowed_role_2\u003e"},{"line_number":58,"context_line":"                - \u003callowed_role_4\u003e"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_8dc9b553","line":55,"range":{"start_line":55,"start_character":16,"end_line":55,"end_character":34},"in_reply_to":"3f79a3b5_dae8d3e1","updated":"2018-10-31 20:22:34.000000000","message":"What we have in this patchset:\n\n``role1 or role2 and role3 or role3``\n\nif I drop it, I\u0027ll get\n\n``role1 or role2 and role3``\n\nSo user with role role3 (rbac_test_role\u003drole3) will have access to API based on mine change and not on yours.\n\nOnly pointing this out to explain that those are 2 different scenarios.","commit_id":"3524924fc10e95cc76d8726fd55fcb2122938b28"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"65cde3d771c2891d4b981388f82103174734b317","unresolved":false,"context_lines":[{"line_number":71,"context_line":"                    component_rules \u003d copy.copy(section[component])"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"                    for rule in component_rules:"},{"line_number":74,"context_line":"                        # Remove spaces to prevent roles with spaces in names"},{"line_number":75,"context_line":"                        component_rules[rule] \u003d ["},{"line_number":76,"context_line":"                            role.replace(\" \", \"\")"},{"line_number":77,"context_line":"                            for role in component_rules[rule]]"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"                        # Split comma separated roles"},{"line_number":80,"context_line":"                        component_rules[rule] \u003d ["}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_9c454d75","line":77,"range":{"start_line":74,"start_character":0,"end_line":77,"end_character":62},"updated":"2018-11-02 02:01:12.000000000","message":"What is the need for this change? If a role contains a space that should be OK.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"8e0997eecc87271618a779a48e56ee8c51649563","unresolved":false,"context_lines":[{"line_number":71,"context_line":"                    component_rules \u003d copy.copy(section[component])"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"                    for rule in component_rules:"},{"line_number":74,"context_line":"                        # Remove spaces to prevent roles with spaces in names"},{"line_number":75,"context_line":"                        component_rules[rule] \u003d ["},{"line_number":76,"context_line":"                            role.replace(\" \", \"\")"},{"line_number":77,"context_line":"                            for role in component_rules[rule]]"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"                        # Split comma separated roles"},{"line_number":80,"context_line":"                        component_rules[rule] \u003d ["}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_3f1c2bc2","line":77,"range":{"start_line":74,"start_character":0,"end_line":77,"end_character":62},"in_reply_to":"3f79a3b5_9c454d75","updated":"2018-11-02 03:12:13.000000000","message":"Done","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"65cde3d771c2891d4b981388f82103174734b317","unresolved":false,"context_lines":[{"line_number":77,"context_line":"                            for role in component_rules[rule]]"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"                        # Split comma separated roles"},{"line_number":80,"context_line":"                        component_rules[rule] \u003d ["},{"line_number":81,"context_line":"                            roles.split(\u0027,\u0027)"},{"line_number":82,"context_line":"                            for roles in component_rules[rule]]"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_dc52e536","line":80,"updated":"2018-11-02 02:01:12.000000000","message":"Need to remove whitespace between commas, to avoid roles having extra whitespace that isn\u0027t valid. For example:\n\n    component_rules[rule] \u003d [\n        r.lsplit(\u0027 \u0027) for r in roles.split(\u0027,\u0027)\n        for roles in component_rules[rule]]\n\nAlthough maybe strip() is better as it\u0027ll handle whitespace on both sides.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"8e0997eecc87271618a779a48e56ee8c51649563","unresolved":false,"context_lines":[{"line_number":77,"context_line":"                            for role in component_rules[rule]]"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"                        # Split comma separated roles"},{"line_number":80,"context_line":"                        component_rules[rule] \u003d ["},{"line_number":81,"context_line":"                            roles.split(\u0027,\u0027)"},{"line_number":82,"context_line":"                            for roles in component_rules[rule]]"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_df1ad7c5","line":80,"in_reply_to":"3f79a3b5_dc52e536","updated":"2018-11-02 03:12:13.000000000","message":"Done","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"65cde3d771c2891d4b981388f82103174734b317","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                        # Split comma separated roles"},{"line_number":80,"context_line":"                        component_rules[rule] \u003d ["},{"line_number":81,"context_line":"                            roles.split(\u0027,\u0027)"},{"line_number":82,"context_line":"                            for roles in component_rules[rule]]"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"                    return component_rules"},{"line_number":85,"context_line":"        except yaml.parser.ParserError:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_bfe59b3f","line":82,"updated":"2018-11-02 02:01:12.000000000","message":"Also this logic will have to be more intricate because right now it doesn\u0027t distinguish between logical or/logical and.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"8e0997eecc87271618a779a48e56ee8c51649563","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                        # Split comma separated roles"},{"line_number":80,"context_line":"                        component_rules[rule] \u003d ["},{"line_number":81,"context_line":"                            roles.split(\u0027,\u0027)"},{"line_number":82,"context_line":"                            for roles in component_rules[rule]]"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"                    return component_rules"},{"line_number":85,"context_line":"        except yaml.parser.ParserError:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_bf759b01","line":82,"in_reply_to":"3f79a3b5_bfe59b3f","updated":"2018-11-02 03:12:13.000000000","message":"It\u0027s not as easy as oslo.policy to read, but I think it is:\n\n``\nneutron:\n  create_port:\n    - role1,role2\n    - role3\n    - role1,!role2\n``\n\nEvery line under ``create_port`` joined with OR, and every comma-separated item within each line joined with AND.\n\nIn other words, we will get:\n\ncomponent_rules[rule] \u003d [[role1, role2], [role3], [role1, !role2]]\n\nin oslo.policy you would write it as:\n\n``role1 and role2 or role3 or role1 and not role2``","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"65cde3d771c2891d4b981388f82103174734b317","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                \"empty. Ensure the requirements YAML file is correctly \""},{"line_number":130,"context_line":"                \"formatted.\")"},{"line_number":131,"context_line":"        try:"},{"line_number":132,"context_line":"            requirement_roles \u003d self.roles_dict[rule_name]"},{"line_number":133,"context_line":"            for required_roles in requirement_roles:"},{"line_number":134,"context_line":"                must_have \u003d [required_role"},{"line_number":135,"context_line":"                             for required_role in required_roles"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_7f3dc38b","line":132,"updated":"2018-11-02 02:01:12.000000000","message":"Something like:\n\n    result \u003d True\n    for role_reqs in component_rules[rule]:\n        if \u0027,\u0027 in role_reqs:\n            role_reqs \u003d [r.strip() for in role_reqs.split(\u0027,\u0027)]\n            forbidden_roles \u003d [r for r in role_reqs if r.startswith(\"!\")]\n            if any(r in forbidden_roles for r in roles):\n                result \u003d result and False\n            result \u003d result and all(r for r in req_roles for r in roles)\n        else:\n            result \u003d result or any(r \u003d\u003d role_reqs for r in roles)\n    return result","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"fefd838f0edc50ae11938bbec76ebb4837aa1ab9","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                \"empty. Ensure the requirements YAML file is correctly \""},{"line_number":130,"context_line":"                \"formatted.\")"},{"line_number":131,"context_line":"        try:"},{"line_number":132,"context_line":"            requirement_roles \u003d self.roles_dict[rule_name]"},{"line_number":133,"context_line":"            for required_roles in requirement_roles:"},{"line_number":134,"context_line":"                must_have \u003d [required_role"},{"line_number":135,"context_line":"                             for required_role in required_roles"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_c85c545a","line":132,"in_reply_to":"3f79a3b5_5f0d673e","updated":"2018-11-02 14:17:18.000000000","message":"Then it would just become: `all(r for r in roles for r in req_roles)`. If current implementation works that is fine, but I think we are missing some test cases.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"8e0997eecc87271618a779a48e56ee8c51649563","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                \"empty. Ensure the requirements YAML file is correctly \""},{"line_number":130,"context_line":"                \"formatted.\")"},{"line_number":131,"context_line":"        try:"},{"line_number":132,"context_line":"            requirement_roles \u003d self.roles_dict[rule_name]"},{"line_number":133,"context_line":"            for required_roles in requirement_roles:"},{"line_number":134,"context_line":"                must_have \u003d [required_role"},{"line_number":135,"context_line":"                             for required_role in required_roles"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_5f0d673e","line":132,"in_reply_to":"3f79a3b5_7f3dc38b","updated":"2018-11-02 03:12:13.000000000","message":"``all(r for r in req_roles for r in roles)``\n\nLet\u0027s say user have rbac_test_role\u003dmember\n\nOur req_file:\n\nservice:\n  update_port:\n    - member,support\n\nIt will result in incorrect behavior.\n\nAt every iteration here we got the whole pack of roles joined with AND.\n\nReformated code a bit to make it more clear.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"c10b2d0c8b8c8e9d05d51be306e2e7375f7dfc3b","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                \"empty. Ensure the requirements YAML file is correctly \""},{"line_number":130,"context_line":"                \"formatted.\")"},{"line_number":131,"context_line":"        try:"},{"line_number":132,"context_line":"            requirement_roles \u003d self.roles_dict[rule_name]"},{"line_number":133,"context_line":"            for required_roles in requirement_roles:"},{"line_number":134,"context_line":"                must_have \u003d [required_role"},{"line_number":135,"context_line":"                             for required_role in required_roles"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_74463659","line":132,"in_reply_to":"3f79a3b5_c85c545a","updated":"2018-11-02 16:57:02.000000000","message":"That is exactly what we have. ``any(negative_roles) and all(positive_roles)``  should cover all ``and``+``not`` syntax.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"c10b2d0c8b8c8e9d05d51be306e2e7375f7dfc3b","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        try:"},{"line_number":128,"context_line":"            requirement_roles \u003d self.roles_dict[rule_name]"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"            for role_reqs in requirement_roles:"},{"line_number":131,"context_line":"                allowed_roles \u003d ["},{"line_number":132,"context_line":"                    role for role in role_reqs if not role.startswith(\"!\")]"},{"line_number":133,"context_line":"                forbidden_roles \u003d ["},{"line_number":134,"context_line":"                    role[1:] for role in role_reqs if role.startswith(\"!\")]"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"                # If user possess at least single forbidden role - skip to"},{"line_number":137,"context_line":"                # next pack of roles."},{"line_number":138,"context_line":"                if any([r in roles for r in forbidden_roles]):"},{"line_number":139,"context_line":"                    continue"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"                # If user possess all required roles - exit method since user"},{"line_number":142,"context_line":"                # is allowed to perform operation."},{"line_number":143,"context_line":"                if all([r in roles for r in allowed_roles]):"},{"line_number":144,"context_line":"                    return True"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            return False"},{"line_number":147,"context_line":"        except KeyError:"},{"line_number":148,"context_line":"            raise KeyError(\"\u0027%s\u0027 API is not defined in the requirements YAML \""},{"line_number":149,"context_line":"                           \"file\" % rule_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_6883b907","line":146,"range":{"start_line":130,"start_character":0,"end_line":146,"end_character":24},"updated":"2018-11-02 16:57:02.000000000","message":"# Every loop iteration we receive bunch of roles that should\n# be considered as logical and.\n# Lets assume we have:\n#     \"member and support and !admin and !manager\"\n# required_roles \u003d [member, support]\n# forbidden_roles \u003d [admin, manager]\n#\n# User roles: [member, support, admin]\n# required_passed \u003d\u003d True\n# forbidden_passed \u003d\u003d True\n# Result required_passed and forbidden_passed \u003d\u003d True\n#\n# User roles: [member, admin]\n# required_passed \u003d\u003d True\n# forbidden_passed \u003d\u003d True\n# Result required_passed and forbidden_passed \u003d\u003d True\n#\n# User roles: [member, support, admin, manager]\n# required_passed \u003d\u003d True\n# forbidden_passed \u003d\u003d False\n# Result required_passed and forbidden_passed \u003d\u003d False","commit_id":"e1b1fd49f19df2461d5ebd73a235eaf3a23be0b5"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"ef7317c858ed58825ef0b88a06de8d6013cd276f","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                                        for r in roles])"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"                if required_passed and forbidden_passed:"},{"line_number":143,"context_line":"                    print(roles, role_reqs)"},{"line_number":144,"context_line":"                    return True"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_84b159e6","line":143,"range":{"start_line":143,"start_character":20,"end_line":143,"end_character":43},"updated":"2018-11-02 21:25:36.000000000","message":"nit: Remove this.","commit_id":"39402e596ae3c5f53b890b18b73beac14548cfb4"}],"patrole_tempest_plugin/tests/unit/test_requirements_authority.py":[{"author":{"_account_id":27078,"name":"Manik Bindlish","email":"manikbindlish19@gmail.com","username":"manik"},"change_message_id":"bc0ea34e55df20deb21805de11641b2c62812274","unresolved":false,"context_lines":[{"line_number":86,"context_line":"            req_auth.RequirementsParser.parse(\"Failure\")"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        self.assertIsNone(self.rbac_auth.roles_dict)"},{"line_number":89,"context_line":"        self.assertRaises(exceptions.InvalidConfiguration,"},{"line_number":90,"context_line":"                          self.rbac_auth.allowed, \"\", [\"\"])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_5adfe30a","line":90,"range":{"start_line":89,"start_character":8,"end_line":90,"end_character":59},"updated":"2018-10-31 04:43:58.000000000","message":"pep8 fail: no newline at end of file","commit_id":"3524924fc10e95cc76d8726fd55fcb2122938b28"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"65cde3d771c2891d4b981388f82103174734b317","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        for roles in roles_for_user:"},{"line_number":69,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!_member_\u0027]]}"},{"line_number":70,"context_line":"            self.assertFalse(self.rbac_auth.allowed(\"api\", roles))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_auth_allowed_roles_with_not_invoked_not_statement(self):"},{"line_number":73,"context_line":"        \"\"\"If user have no ``viewer`` role assigned - call should be allowed."},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_9fb53ff3","line":71,"updated":"2018-11-02 02:01:12.000000000","message":"This should result in True:\n        \n         roles_for_user \u003d [\n            [\"_member_\"],\n            [\"test_member\", \"!_member_\"]\n\nIf the role is \"_member_\" as it should evaluate to True or False.\n\nThis is the equivalent of \"(role:test_member and not role:_member_) or role:_member_\".\n\nLooking at this, though, it becomes increasingly meaningless to use this requirements-driven approach as there\u0027s hardly any reduction in complexity between oslo.policy syntax and this...\n\nBut that is an aside, I think the requirements-driven approach is fraught with issues in general.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"7ec2950205b46ad313d5c2e7b186c69ceda35684","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        for roles in roles_for_user:"},{"line_number":69,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!_member_\u0027]]}"},{"line_number":70,"context_line":"            self.assertFalse(self.rbac_auth.allowed(\"api\", roles))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_auth_allowed_roles_with_not_invoked_not_statement(self):"},{"line_number":73,"context_line":"        \"\"\"If user have no ``viewer`` role assigned - call should be allowed."},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_6e6f49c7","line":71,"in_reply_to":"3f79a3b5_0849ecb6","updated":"2018-11-02 14:22:52.000000000","message":"Actually, scratch that. I would expect:\n\nself.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!viewer\u0027\u0027]]}, and:\n\n       roles_for_user \u003d [\n            [\"viewer\"],\n            [\"test_member\", \"__member__\"],\n            [\"test_member\", \"viewer\"]\n        ]\n\nImplies:\n\n       roles_for_user \u003d [\n            [\"viewer\"], \u003d\u003e False\n            [\"test_member\", \"__member__\"], \u003d\u003e False\n            [\"test_member\", \"viewer\"] \u003d\u003e False\n        ]\n\nBut self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!viewer\u0027\u0027], [\u0027viewer\u0027]}, and:\n\n       roles_for_user \u003d [\n            [\"viewer\"],\n            [\"test_member\", \"__member__\"],\n            [\"test_member\", \"viewer\"]\n        ]\n\nImplies:\n\n       roles_for_user \u003d [\n            [\"viewer\"], \u003d\u003e True\n            [\"test_member\", \"__member__\"], \u003d\u003e False\n            [\"test_member\", \"viewer\"] \u003d\u003e False\n        ]","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"c10b2d0c8b8c8e9d05d51be306e2e7375f7dfc3b","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        for roles in roles_for_user:"},{"line_number":69,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!_member_\u0027]]}"},{"line_number":70,"context_line":"            self.assertFalse(self.rbac_auth.allowed(\"api\", roles))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_auth_allowed_roles_with_not_invoked_not_statement(self):"},{"line_number":73,"context_line":"        \"\"\"If user have no ``viewer`` role assigned - call should be allowed."},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_e2c92852","line":71,"in_reply_to":"3f79a3b5_6e6f49c7","updated":"2018-11-02 16:57:02.000000000","message":"Done-ish","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"8e0997eecc87271618a779a48e56ee8c51649563","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        for roles in roles_for_user:"},{"line_number":69,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!_member_\u0027]]}"},{"line_number":70,"context_line":"            self.assertFalse(self.rbac_auth.allowed(\"api\", roles))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_auth_allowed_roles_with_not_invoked_not_statement(self):"},{"line_number":73,"context_line":"        \"\"\"If user have no ``viewer`` role assigned - call should be allowed."},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_c2975e9b","line":71,"in_reply_to":"3f79a3b5_9fb53ff3","updated":"2018-11-02 03:12:13.000000000","message":"``roles_for_user`` is not rquirements_auth, it\u0027s list of roles to be assigned in loop to user to see how method .allowed() will act in different combinations of roles.\n\nI think the confusion here comes from line 80. I have ``!viewer`` there on purpose to ensure that parser will not forget to drop exclamation mark.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"fefd838f0edc50ae11938bbec76ebb4837aa1ab9","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        for roles in roles_for_user:"},{"line_number":69,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!_member_\u0027]]}"},{"line_number":70,"context_line":"            self.assertFalse(self.rbac_auth.allowed(\"api\", roles))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_auth_allowed_roles_with_not_invoked_not_statement(self):"},{"line_number":73,"context_line":"        \"\"\"If user have no ``viewer`` role assigned - call should be allowed."},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_0849ecb6","line":71,"in_reply_to":"3f79a3b5_c2975e9b","updated":"2018-11-02 14:17:18.000000000","message":"I meant it might be good to add another test where the same role is denied in an AND check and allowed in an OR check to make sure it is evaluating the combination of all the checks, like:\n\n        roles_for_user \u003d [\n            [\"viewer\"],\n            [\"test_member\", \"__member__\"],\n            [\"test_member\", \"!viewer\"]\n        ]\n\nShould say that when user has \"viewer\" then is allowed.\n\nBut it should say that when user has \"test_member\" and \"viewer\" then that is not allowed.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"fefd838f0edc50ae11938bbec76ebb4837aa1ab9","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        roles_for_user \u003d ["},{"line_number":78,"context_line":"            [\"test_member\"],"},{"line_number":79,"context_line":"            [\"test_member\", \"__member__\"],"},{"line_number":80,"context_line":"            [\"test_member\", \"!viewer\"]"},{"line_number":81,"context_line":"        ]"},{"line_number":82,"context_line":"        for roles in roles_for_user:"},{"line_number":83,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!viewer\u0027]]}"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_c88e94d6","line":80,"range":{"start_line":80,"start_character":28,"end_line":80,"end_character":37},"updated":"2018-11-02 14:17:18.000000000","message":"Does this correspond to a value in CONF.patrole.rbac_test_roles? We should only allow roles that exist in the system.\n\nThis would simply be [\"test_member\", \"viewer\"]. Then it is up to RequirementsAuthority to reject viewer role if the check in the roles_dict is \"!viewer\" which I agree is OK to put there.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"c10b2d0c8b8c8e9d05d51be306e2e7375f7dfc3b","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        roles_for_user \u003d ["},{"line_number":78,"context_line":"            [\"test_member\"],"},{"line_number":79,"context_line":"            [\"test_member\", \"__member__\"],"},{"line_number":80,"context_line":"            [\"test_member\", \"!viewer\"]"},{"line_number":81,"context_line":"        ]"},{"line_number":82,"context_line":"        for roles in roles_for_user:"},{"line_number":83,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!viewer\u0027]]}"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_343afe96","line":80,"range":{"start_line":80,"start_character":28,"end_line":80,"end_character":37},"in_reply_to":"3f79a3b5_c88e94d6","updated":"2018-11-02 16:57:02.000000000","message":"`Does this correspond to a value in CONF.patrole.rbac_test_roles`\n\nYes. It\u0027s done here this way so at line 83, where we have \u0027!viewer\u0027 in role_dict, during parsing of \u0027!viewer\u0027 ``!`` should be dropped so \"!viewer\" from rbac_test_roles !\u003d \"viewer\" from roles_dict post-parsing.\n\nBut since it so confusing let me just drop it here and create separate patchset.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"7ec2950205b46ad313d5c2e7b186c69ceda35684","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            [\"test_member\", \"!viewer\"]"},{"line_number":81,"context_line":"        ]"},{"line_number":82,"context_line":"        for roles in roles_for_user:"},{"line_number":83,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!viewer\u0027]]}"},{"line_number":84,"context_line":"            self.assertTrue(self.rbac_auth.allowed(\"api\", roles))"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def test_parser_get_allowed_except_keyerror(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_0e5ed513","line":83,"range":{"start_line":83,"start_character":40,"end_line":83,"end_character":77},"updated":"2018-11-02 14:22:52.000000000","message":"I think we need some checks that combine AND with OR, like:\n\n{\u0027api\u0027: [[\u0027test_member\u0027, \u0027!viewer\u0027], [\u0027foo\u0027], [\u0027bar\u0027], [\u0027!baz\u0027]]}\n\nAs well.","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"},{"author":{"_account_id":16274,"name":"Mykola Yakovliev","email":"VegasQ@gmail.com","username":"vegasq"},"change_message_id":"c10b2d0c8b8c8e9d05d51be306e2e7375f7dfc3b","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            [\"test_member\", \"!viewer\"]"},{"line_number":81,"context_line":"        ]"},{"line_number":82,"context_line":"        for roles in roles_for_user:"},{"line_number":83,"context_line":"            self.rbac_auth.roles_dict \u003d {\u0027api\u0027: [[\u0027test_member\u0027, \u0027!viewer\u0027]]}"},{"line_number":84,"context_line":"            self.assertTrue(self.rbac_auth.allowed(\"api\", roles))"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def test_parser_get_allowed_except_keyerror(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_74839607","line":83,"range":{"start_line":83,"start_character":40,"end_line":83,"end_character":77},"in_reply_to":"3f79a3b5_0e5ed513","updated":"2018-11-02 16:57:02.000000000","message":"Done","commit_id":"4c49a8eccbd61d6ae5d105e49013688dadb79555"}]}