)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"7cf3f481e482a0f01f9ac2f55b99deed797b6e5f","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Co-Authored-By: Sean Perry \u003csean.perry@hpe.com\u003e"},{"line_number":14,"context_line":"Change-Id: Id1f5ccac3c639a44b33780b001e401bab195d8b3"},{"line_number":15,"context_line":"Fixes-Bug: #1593813"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a8ec9b2_8b64ae49","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":9},"updated":"2016-09-15 11:47:23.000000000","message":"Closes-Bug","commit_id":"fdc9a935d17a926e28ad026bfae2d8af655a1e00"}],"etc/policy.v3cloudsample.json":[{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"7cf3f481e482a0f01f9ac2f55b99deed797b6e5f","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    \"identity:get_implied_role\": \"rule:cloud_admin or rule:admin_and_matching_prior_role_domain_id\","},{"line_number":99,"context_line":"    \"identity:list_implied_roles\": \"rule:cloud_admin\","},{"line_number":100,"context_line":"    \"identity:create_implied_role\": \"rule:cloud_admin or rule:admin_and_matching_prior_role_domain_id\","},{"line_number":101,"context_line":"    \"identity:delete_implied_role\": \"rule:cloud_admin\","},{"line_number":102,"context_line":"    \"identity:list_role_inference_rules\": \"rule:cloud_admin\","},{"line_number":103,"context_line":"    \"identity:check_implied_role\": \"rule:cloud_admin\","},{"line_number":104,"context_line":""}],"source_content_type":"application/json","patch_set":2,"id":"7a8ec9b2_0b797e2f","line":101,"updated":"2016-09-15 11:47:23.000000000","message":"domain_admin should also be able to delete, right?","commit_id":"fdc9a935d17a926e28ad026bfae2d8af655a1e00"},{"author":{"_account_id":17026,"name":"Sean Perry","email":"sean.perry@hpe.com","username":"sean_perry"},"change_message_id":"eb6d8494dcb084ed1698771798435108b235798c","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    \"identity:get_implied_role\": \"rule:cloud_admin or rule:admin_and_matching_prior_role_domain_id\","},{"line_number":99,"context_line":"    \"identity:list_implied_roles\": \"rule:cloud_admin\","},{"line_number":100,"context_line":"    \"identity:create_implied_role\": \"rule:cloud_admin or rule:admin_and_matching_prior_role_domain_id\","},{"line_number":101,"context_line":"    \"identity:delete_implied_role\": \"rule:cloud_admin\","},{"line_number":102,"context_line":"    \"identity:list_role_inference_rules\": \"rule:cloud_admin\","},{"line_number":103,"context_line":"    \"identity:check_implied_role\": \"rule:cloud_admin\","},{"line_number":104,"context_line":""}],"source_content_type":"application/json","patch_set":2,"id":"7a8ec9b2_a22896e3","line":101,"in_reply_to":"7a8ec9b2_0b797e2f","updated":"2016-09-15 17:47:53.000000000","message":"thanks Rodgrigo. I added support in the code but forgot to update the policy. Same for the check below.","commit_id":"fdc9a935d17a926e28ad026bfae2d8af655a1e00"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"7cf3f481e482a0f01f9ac2f55b99deed797b6e5f","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    \"identity:create_implied_role\": \"rule:cloud_admin or rule:admin_and_matching_prior_role_domain_id\","},{"line_number":101,"context_line":"    \"identity:delete_implied_role\": \"rule:cloud_admin\","},{"line_number":102,"context_line":"    \"identity:list_role_inference_rules\": \"rule:cloud_admin\","},{"line_number":103,"context_line":"    \"identity:check_implied_role\": \"rule:cloud_admin\","},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    \"identity:check_grant\": \"rule:cloud_admin or rule:domain_admin_for_grants or rule:project_admin_for_grants\","},{"line_number":106,"context_line":"    \"identity:list_grants\": \"rule:cloud_admin or rule:domain_admin_for_list_grants or rule:project_admin_for_list_grants\","}],"source_content_type":"application/json","patch_set":2,"id":"7a8ec9b2_2baf1a82","line":103,"updated":"2016-09-15 11:47:23.000000000","message":"think check should also be doable by the domain_admin","commit_id":"fdc9a935d17a926e28ad026bfae2d8af655a1e00"}],"keystone/assignment/controllers.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"fae7bc54c84a184fd929c968e5caffc916ece27b","unresolved":false,"context_lines":[{"line_number":443,"context_line":"class ImpliedRolesV3(controller.V3Controller):"},{"line_number":444,"context_line":"    \"\"\"The V3 ImpliedRoles CRD APIs.  There is no Update.\"\"\""},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def _check_implies_role(self, request, prep_info,"},{"line_number":447,"context_line":"                            prior_role_id, implied_role_id\u003dNone):"},{"line_number":448,"context_line":"        ref \u003d {}"},{"line_number":449,"context_line":"        ref[\u0027prior_role\u0027] \u003d self.role_api.get_role(prior_role_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_e193eafe","line":446,"updated":"2016-09-19 19:44:32.000000000","message":"implied*","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":17026,"name":"Sean Perry","email":"sean.perry@hpe.com","username":"sean_perry"},"change_message_id":"cc9f20a85a0e7478783bdd227d5b02f00c7a70e5","unresolved":false,"context_lines":[{"line_number":443,"context_line":"class ImpliedRolesV3(controller.V3Controller):"},{"line_number":444,"context_line":"    \"\"\"The V3 ImpliedRoles CRD APIs.  There is no Update.\"\"\""},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def _check_implies_role(self, request, prep_info,"},{"line_number":447,"context_line":"                            prior_role_id, implied_role_id\u003dNone):"},{"line_number":448,"context_line":"        ref \u003d {}"},{"line_number":449,"context_line":"        ref[\u0027prior_role\u0027] \u003d self.role_api.get_role(prior_role_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_e75137da","line":446,"in_reply_to":"7a8ec9b2_e193eafe","updated":"2016-09-19 19:47:16.000000000","message":"I named it \u0027implies\u0027 on purpose to match the REST call. In reality it is the prior_role that is most often checked....","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"}],"keystone/tests/unit/test_v3_protection.py":[{"author":{"_account_id":6460,"name":"Brad Topol","email":"btopol@us.ibm.com","username":"btopol"},"change_message_id":"20e0706904690220d8f46e5056e87294b0be10c1","unresolved":false,"context_lines":[{"line_number":1865,"context_line":"            expected\u003dexception.ForbiddenAction.code)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"    @utils.wip(\u0027waiting on bug #1593813\u0027)"},{"line_number":1868,"context_line":"    def test_implied_domain_specific_roles_through_rest_api(self):"},{"line_number":1869,"context_line":"        domain_admin_auth \u003d self.build_authentication_request("},{"line_number":1870,"context_line":"            user_id\u003dself.domain_admin_user[\u0027id\u0027],"},{"line_number":1871,"context_line":"            password\u003dself.domain_admin_user[\u0027password\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_78b9432e","line":1868,"updated":"2016-07-10 02:52:59.000000000","message":"So all the other test_domain_XXX tests do a really nice job of providing comments that describe what is being tested by the domain test.  This one does not have it and it makes it harder to understand and inconsistent from the rest of the domain tests. It really should have explanatory comments like the other domain tests.","commit_id":"c17a5f999fa3f387165f859bc8f3120e52272cb7"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"7cf3f481e482a0f01f9ac2f55b99deed797b6e5f","unresolved":false,"context_lines":[{"line_number":1872,"context_line":"            self.domainA[\u0027id\u0027], read_status_OK\u003dTrue,"},{"line_number":1873,"context_line":"            expected\u003dexception.ForbiddenAction.code)"},{"line_number":1874,"context_line":""},{"line_number":1875,"context_line":"    def test_implied_domain_specific_roles_through_rest_api(self):"},{"line_number":1876,"context_line":"        # A domain admin should be able to create an implied role on the"},{"line_number":1877,"context_line":"        # domain for which they are the admin."},{"line_number":1878,"context_line":"        domain_admin_auth \u003d self.build_authentication_request("}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_8b082e93","line":1875,"updated":"2016-09-15 11:47:23.000000000","message":"additional tests for \"delete\", \"get\" and \"check\" are needed.","commit_id":"fdc9a935d17a926e28ad026bfae2d8af655a1e00"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"259a07ff501f2bacd0f4389b07150b393526a743","unresolved":false,"context_lines":[{"line_number":1898,"context_line":"    def test_get(self):"},{"line_number":1899,"context_line":"        # A domain admin should be able to get an existing implied role"},{"line_number":1900,"context_line":"        # on the domain for which they are the admin."},{"line_number":1901,"context_line":"        self._create_implied_role()"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"        self.get(\u0027/roles/%s/implies/%s\u0027"},{"line_number":1904,"context_line":"                 % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_b39f794f","line":1901,"updated":"2016-09-19 18:20:43.000000000","message":"I actually like it here. We have fallen into this trap of \"less code\" so we do crazy things in our test setup. Many of my test improvements (for test speed and coverage) over the last year has revolved around removing setup.","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"6d54c3948084bf9a48cd6decf8a3b68ed59486d9","unresolved":false,"context_lines":[{"line_number":1898,"context_line":"    def test_get(self):"},{"line_number":1899,"context_line":"        # A domain admin should be able to get an existing implied role"},{"line_number":1900,"context_line":"        # on the domain for which they are the admin."},{"line_number":1901,"context_line":"        self._create_implied_role()"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"        self.get(\u0027/roles/%s/implies/%s\u0027"},{"line_number":1904,"context_line":"                 % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_7f061d1e","line":1901,"range":{"start_line":1901,"start_character":8,"end_line":1901,"end_character":35},"updated":"2016-09-15 21:25:06.000000000","message":"you can put this in the setUp() so you won\u0027t repeat in the beginning of each test.","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"3e5902a90ce842c0888aabbfb098aa2b5aa103f1","unresolved":false,"context_lines":[{"line_number":1898,"context_line":"    def test_get(self):"},{"line_number":1899,"context_line":"        # A domain admin should be able to get an existing implied role"},{"line_number":1900,"context_line":"        # on the domain for which they are the admin."},{"line_number":1901,"context_line":"        self._create_implied_role()"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"        self.get(\u0027/roles/%s/implies/%s\u0027"},{"line_number":1904,"context_line":"                 % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_e11f4307","line":1901,"in_reply_to":"7a8ec9b2_45cf40b3","updated":"2016-09-20 11:02:20.000000000","message":"example:\n\n  def _assert_create_implied_role(self):\n     ref \u003d self.put(\u0027/roles/%s/implies/%s\u0027,\n                    % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),\n                    token\u003dself.admin_token,\n                    expected_status\u003dhttp_client.CREATED)\n\n     implied_role \u003d self.assertValidImpliedRoleResponse(resp) # you would need to implement it in test_v3.py\n     return implied_role\n\nhaving this method, you avoid the need of having \"test_put()\" below and, at the same time, you can add it to setUp() to not repeat in each test.","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":17026,"name":"Sean Perry","email":"sean.perry@hpe.com","username":"sean_perry"},"change_message_id":"3cd7470b0766d225bc9d4be1dd24282b107bc00b","unresolved":false,"context_lines":[{"line_number":1898,"context_line":"    def test_get(self):"},{"line_number":1899,"context_line":"        # A domain admin should be able to get an existing implied role"},{"line_number":1900,"context_line":"        # on the domain for which they are the admin."},{"line_number":1901,"context_line":"        self._create_implied_role()"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"        self.get(\u0027/roles/%s/implies/%s\u0027"},{"line_number":1904,"context_line":"                 % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_c04a6cac","line":1901,"range":{"start_line":1901,"start_character":8,"end_line":1901,"end_character":35},"in_reply_to":"7a8ec9b2_7f061d1e","updated":"2016-09-19 17:16:02.000000000","message":"Bah. I like the clarity the tests have now. Why is this a bikeshed?","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"db4dd99c03526d428343857f4f0ddee8db793390","unresolved":false,"context_lines":[{"line_number":1898,"context_line":"    def test_get(self):"},{"line_number":1899,"context_line":"        # A domain admin should be able to get an existing implied role"},{"line_number":1900,"context_line":"        # on the domain for which they are the admin."},{"line_number":1901,"context_line":"        self._create_implied_role()"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"        self.get(\u0027/roles/%s/implies/%s\u0027"},{"line_number":1904,"context_line":"                 % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_45cf40b3","line":1901,"in_reply_to":"7a8ec9b2_ac0f162d","updated":"2016-09-19 22:09:17.000000000","message":"I mean, adding it in the setUp() makes more sense to me than repeating in each test since the only one that doesn\u0027t use is \"test_put()\".","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"5ce657e9f27f3636d6a80176afca7cc23700e5e4","unresolved":false,"context_lines":[{"line_number":1898,"context_line":"    def test_get(self):"},{"line_number":1899,"context_line":"        # A domain admin should be able to get an existing implied role"},{"line_number":1900,"context_line":"        # on the domain for which they are the admin."},{"line_number":1901,"context_line":"        self._create_implied_role()"},{"line_number":1902,"context_line":""},{"line_number":1903,"context_line":"        self.get(\u0027/roles/%s/implies/%s\u0027"},{"line_number":1904,"context_line":"                 % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_ac0f162d","line":1901,"in_reply_to":"7a8ec9b2_b39f794f","updated":"2016-09-19 19:19:05.000000000","message":"it is used in all tests but one, think the \"exception\" is test_put(), so that test should be different one.","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"259a07ff501f2bacd0f4389b07150b393526a743","unresolved":false,"context_lines":[{"line_number":1921,"context_line":"                  % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"},{"line_number":1922,"context_line":"                  token\u003dself.admin_token)"},{"line_number":1923,"context_line":""},{"line_number":1924,"context_line":"    def test_put(self):"},{"line_number":1925,"context_line":"        # A domain admin should be able to create an implied role on the"},{"line_number":1926,"context_line":"        # domain for which they are the admin."},{"line_number":1927,"context_line":"        self.put(\u0027/roles/%s/implies/%s\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_d3ac058f","line":1924,"updated":"2016-09-19 18:20:43.000000000","message":"Is this change just to consolidate setup or is there other value?","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"6d54c3948084bf9a48cd6decf8a3b68ed59486d9","unresolved":false,"context_lines":[{"line_number":1921,"context_line":"                  % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"},{"line_number":1922,"context_line":"                  token\u003dself.admin_token)"},{"line_number":1923,"context_line":""},{"line_number":1924,"context_line":"    def test_put(self):"},{"line_number":1925,"context_line":"        # A domain admin should be able to create an implied role on the"},{"line_number":1926,"context_line":"        # domain for which they are the admin."},{"line_number":1927,"context_line":"        self.put(\u0027/roles/%s/implies/%s\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_9f55a9fb","line":1924,"range":{"start_line":1924,"start_character":8,"end_line":1924,"end_character":16},"updated":"2016-09-15 21:25:06.000000000","message":"for this test you can create two new roles and then create the implies (so you will be able to remove the _create_implied_role() of the other test methods and put in the setUp())","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"5ce657e9f27f3636d6a80176afca7cc23700e5e4","unresolved":false,"context_lines":[{"line_number":1921,"context_line":"                  % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"},{"line_number":1922,"context_line":"                  token\u003dself.admin_token)"},{"line_number":1923,"context_line":""},{"line_number":1924,"context_line":"    def test_put(self):"},{"line_number":1925,"context_line":"        # A domain admin should be able to create an implied role on the"},{"line_number":1926,"context_line":"        # domain for which they are the admin."},{"line_number":1927,"context_line":"        self.put(\u0027/roles/%s/implies/%s\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_cce62244","line":1924,"in_reply_to":"7a8ec9b2_735ed177","updated":"2016-09-19 19:19:05.000000000","message":"actually, this is exactly what \"_create_implied_role()\" does, the difference is that it calls the manager layer directly. Usually, tests that \"replicate\" the setUp() are done not using the setUp().\n\nI would be ok removing this test and using the controller layer in \"_create_implied_role()\".","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"db4dd99c03526d428343857f4f0ddee8db793390","unresolved":false,"context_lines":[{"line_number":1921,"context_line":"                  % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"},{"line_number":1922,"context_line":"                  token\u003dself.admin_token)"},{"line_number":1923,"context_line":""},{"line_number":1924,"context_line":"    def test_put(self):"},{"line_number":1925,"context_line":"        # A domain admin should be able to create an implied role on the"},{"line_number":1926,"context_line":"        # domain for which they are the admin."},{"line_number":1927,"context_line":"        self.put(\u0027/roles/%s/implies/%s\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_c5f8b0c7","line":1924,"in_reply_to":"7a8ec9b2_cce62244","updated":"2016-09-19 22:09:17.000000000","message":"_create_implied_role() is a method that does the same stuff as this test. Above I\u0027ve suggested to change _create_implied_role() to call the controller layer as it is being done and this test (than removing the test).","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"},{"author":{"_account_id":17026,"name":"Sean Perry","email":"sean.perry@hpe.com","username":"sean_perry"},"change_message_id":"08a8252b0ec5ffdfadaa891fbcbcb2d9ccfc0a2d","unresolved":false,"context_lines":[{"line_number":1921,"context_line":"                  % (self.appadmin_role[\u0027id\u0027], self.appdev_role[\u0027id\u0027]),"},{"line_number":1922,"context_line":"                  token\u003dself.admin_token)"},{"line_number":1923,"context_line":""},{"line_number":1924,"context_line":"    def test_put(self):"},{"line_number":1925,"context_line":"        # A domain admin should be able to create an implied role on the"},{"line_number":1926,"context_line":"        # domain for which they are the admin."},{"line_number":1927,"context_line":"        self.put(\u0027/roles/%s/implies/%s\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_735ed177","line":1924,"in_reply_to":"7a8ec9b2_d3ac058f","updated":"2016-09-19 18:23:16.000000000","message":"Zero testing value. I would need to make a new role here to ignore the one created by setUp in order to test.","commit_id":"47d4d08ecb6804841d6de640afed8ca743f254b3"}]}