)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"23d83bcbf68111b9ec50f903808a59ae0e9e5df4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e564b58e_7d902e04","updated":"2024-08-02 20:34:18.000000000","message":"My apologies, but these kinds of changes take me some time to understand and think through, as I am not super familiar with the keystone enforcer.  So far I\u0027ve looked at the list_roles policies and I think they still need some work.  Some of this stuff is a little tricky to understand, so let me know if you have questions.","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ad61bfb7f363deb9a5fa9fa0a317f8e2b476e098","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3cbe9df1_adcbf70a","updated":"2024-08-05 15:13:14.000000000","message":"Thank you very much for the code review Douglas! I implemented some changes and responded to your inline comments.","commit_id":"e9732e20df16a45c141ac1e56fb63143ea4ae662"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"ecaeb9ab_0e9137b3","updated":"2024-08-28 05:00:40.000000000","message":"I reviewed the rest of the policy changes I had not previously had a chance to look at.  Things are looking good, except for a few policies that should be reverted since they already allowed domain-manager.\n\nThe \"manager\" role is part of the implied role hierarchy [1].  This means that a domain-manager is implied to also be a domain-member, and a domain-reader.\n\nAll policies which currently allow domain-member also allow domain-manager and domain-admin without having to be explicitly added to the policy.\n\nLikewise, all policies that currently allow domain-reader also allow domain-member, domain-manager, and domain-admin without having to name any of those roles in the policy.\n\nThis works because keystone adds all implied roles to the user\u0027s token when it is issued.\n\n[1] https://opendev.org/openstack/keystone/src/commit/b95b2b80021360524c314f4c45d3a1aa6f407da2/keystone/cmd/bootstrap.py#L195\n\ne.g.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"aa4148ad_3ce72a45","updated":"2024-08-28 13:33:57.000000000","message":"Thank you very much for the thorough review Douglas!\n\nI addressed all the spots you commented on except for one, please see the corresponding comment.","commit_id":"3907a856fb2232c3c99848b4b9362e013d07f86f"}],"keystone/api/roles.py":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"23d83bcbf68111b9ec50f903808a59ae0e9e5df4","unresolved":true,"context_lines":[{"line_number":87,"context_line":"            ENFORCER.enforce_call("},{"line_number":88,"context_line":"                action\u003d\u0027identity:list_domain_roles\u0027,"},{"line_number":89,"context_line":"                filters\u003dfilters,"},{"line_number":90,"context_line":"                target_attr\u003d{\"domain_id\": domain_filter},"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            ENFORCER.enforce_call("}],"source_content_type":"text/x-python","patch_set":7,"id":"c6a5515c_98528646","line":90,"range":{"start_line":90,"start_character":16,"end_line":90,"end_character":57},"updated":"2024-08-02 20:34:18.000000000","message":"Is this change necessary?  \n\nThis patch does not currently use the resulting `target.domain_id` value in the `identity:list_domain_roles` policy.\n\nIf we need to check the domain_id in the policy, then the filter parameter already ensures that the domain specified in a query string is part of the `policy_dict` [1] and can already be used to enforce policy.\n\n[1] https://opendev.org/openstack/keystone/src/branch/master/keystone/common/rbac_enforcer/enforcer.py#L372-L378","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ad61bfb7f363deb9a5fa9fa0a317f8e2b476e098","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            ENFORCER.enforce_call("},{"line_number":88,"context_line":"                action\u003d\u0027identity:list_domain_roles\u0027,"},{"line_number":89,"context_line":"                filters\u003dfilters,"},{"line_number":90,"context_line":"                target_attr\u003d{\"domain_id\": domain_filter},"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            ENFORCER.enforce_call("}],"source_content_type":"text/x-python","patch_set":7,"id":"a51ec176_2ddc6119","line":90,"range":{"start_line":90,"start_character":16,"end_line":90,"end_character":57},"in_reply_to":"c6a5515c_98528646","updated":"2024-08-05 15:13:14.000000000","message":"No, this is not necessary anymore. It was a remnant of my first patchset that still intended to give domain managers access to domain roles, which turned out to be pointless in the end. See the comment link in my other comment.\n\nThanks for spotting this! I removed this change.","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"23d83bcbf68111b9ec50f903808a59ae0e9e5df4","unresolved":true,"context_lines":[{"line_number":90,"context_line":"                target_attr\u003d{\"domain_id\": domain_filter},"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            ENFORCER.enforce_call("},{"line_number":94,"context_line":"                action\u003d\u0027identity:list_roles\u0027, filters\u003dfilters"},{"line_number":95,"context_line":"            )"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        hints \u003d self.build_driver_hints(filters)"},{"line_number":98,"context_line":"        if not domain_filter:"}],"source_content_type":"text/x-python","patch_set":7,"id":"2dbf1895_0a64f18d","line":95,"range":{"start_line":93,"start_character":12,"end_line":95,"end_character":13},"updated":"2024-08-02 20:34:18.000000000","message":"As I mentioned in `keystone/common/policies/role.py`, we need to reflect the domain_id here so that we can evaluate domain-scoped tokens correctly.\n\nProviding a `target_attr` would make more sense here, since we know that `domain_id` is not in the query string for this branch, then it will also not be available in the policy_dict for use by the enforcer.  However, it looks like we don\u0027t currently access the context here, deferring to the enforcer to get the context during `enforce_call`.\n\nI am not sure if we want to start accessing the context here, to pull out the incoming domain_id and reflect it in the policy_dict by passing in target_attr, or if we should add a new flag to the enforcer to reflect domain_id for domain-scoped tokens to keep context access contained within the enforcer.","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ad61bfb7f363deb9a5fa9fa0a317f8e2b476e098","unresolved":true,"context_lines":[{"line_number":90,"context_line":"                target_attr\u003d{\"domain_id\": domain_filter},"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            ENFORCER.enforce_call("},{"line_number":94,"context_line":"                action\u003d\u0027identity:list_roles\u0027, filters\u003dfilters"},{"line_number":95,"context_line":"            )"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        hints \u003d self.build_driver_hints(filters)"},{"line_number":98,"context_line":"        if not domain_filter:"}],"source_content_type":"text/x-python","patch_set":7,"id":"e04cb375_febe97b1","line":95,"range":{"start_line":93,"start_character":12,"end_line":95,"end_character":13},"in_reply_to":"2dbf1895_0a64f18d","updated":"2024-08-05 15:13:14.000000000","message":"I tried to solve this purely via policy definition by checking for a domain_id that is not None in `keystone/common/policies/role.py`, which (based on my testing so far) seems to work.\n\nDo you still see the need to change anything here?","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                target_attr\u003d{\"domain_id\": domain_filter},"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            ENFORCER.enforce_call("},{"line_number":94,"context_line":"                action\u003d\u0027identity:list_roles\u0027, filters\u003dfilters"},{"line_number":95,"context_line":"            )"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        hints \u003d self.build_driver_hints(filters)"},{"line_number":98,"context_line":"        if not domain_filter:"}],"source_content_type":"text/x-python","patch_set":7,"id":"c23472eb_acbd4199","line":95,"range":{"start_line":93,"start_character":12,"end_line":95,"end_character":13},"in_reply_to":"da48802a_910a3b84","updated":"2024-08-28 13:33:57.000000000","message":"Acknowledged","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":90,"context_line":"                target_attr\u003d{\"domain_id\": domain_filter},"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            ENFORCER.enforce_call("},{"line_number":94,"context_line":"                action\u003d\u0027identity:list_roles\u0027, filters\u003dfilters"},{"line_number":95,"context_line":"            )"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        hints \u003d self.build_driver_hints(filters)"},{"line_number":98,"context_line":"        if not domain_filter:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7085ca9b_8163fc2b","line":95,"range":{"start_line":93,"start_character":12,"end_line":95,"end_character":13},"in_reply_to":"e04cb375_febe97b1","updated":"2024-08-28 05:00:40.000000000","message":"I tested","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":90,"context_line":"                target_attr\u003d{\"domain_id\": domain_filter},"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            ENFORCER.enforce_call("},{"line_number":94,"context_line":"                action\u003d\u0027identity:list_roles\u0027, filters\u003dfilters"},{"line_number":95,"context_line":"            )"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        hints \u003d self.build_driver_hints(filters)"},{"line_number":98,"context_line":"        if not domain_filter:"}],"source_content_type":"text/x-python","patch_set":7,"id":"da48802a_910a3b84","line":95,"range":{"start_line":93,"start_character":12,"end_line":95,"end_character":13},"in_reply_to":"e04cb375_febe97b1","updated":"2024-08-28 05:00:40.000000000","message":"I tested the updated policy with (role:manager and not domain_id:None) and it works for me in devstack.  I also tested a project-manager and the policy denied access as expected.","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"}],"keystone/common/policies/domain.py":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":57,"context_line":")"},{"line_number":58,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":59,"context_line":"    base.RULE_ADMIN_OR_SYSTEM_READER + \u0027 or \u0027"},{"line_number":60,"context_line":"    \u0027(role:manager and domain_id:%(target.domain.id)s) or \u0027"},{"line_number":61,"context_line":"    \u0027(role:reader and domain_id:%(target.domain.id)s)\u0027"},{"line_number":62,"context_line":")"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"e3155383_2d51574b","line":60,"range":{"start_line":60,"start_character":4,"end_line":60,"end_character":59},"updated":"2024-08-28 05:00:40.000000000","message":"This is not needed.  The policy already allows domain-manager without this change because the manager role is part of the implied roles hierarchy.\n\nThe context object contains the assigned role (in this case \"manager\") and also includes all implied roles (in this case, \"member\" and \"reader\").\n\nSince the context object for a domain-manager already includes the \"reader\" role, the clause below will allow access.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":57,"context_line":")"},{"line_number":58,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":59,"context_line":"    base.RULE_ADMIN_OR_SYSTEM_READER + \u0027 or \u0027"},{"line_number":60,"context_line":"    \u0027(role:manager and domain_id:%(target.domain.id)s) or \u0027"},{"line_number":61,"context_line":"    \u0027(role:reader and domain_id:%(target.domain.id)s)\u0027"},{"line_number":62,"context_line":")"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"e2ea2b82_20060b73","line":60,"range":{"start_line":60,"start_character":4,"end_line":60,"end_character":59},"in_reply_to":"e3155383_2d51574b","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"}],"keystone/common/policies/grant.py":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":72,"context_line":"    \u0027 \u0027 + DOMAIN_MATCHES_TARGET_DOMAIN + \u0027)\u0027"},{"line_number":73,"context_line":")"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":76,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"},{"line_number":77,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_MANAGER + \u0027) or \u0027"},{"line_number":78,"context_line":"    \u0027(\u0027 + SYSTEM_READER_OR_DOMAIN_READER + \u0027)\u0027"},{"line_number":79,"context_line":")"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER_LIST \u003d ("},{"line_number":82,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"},{"line_number":83,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_MANAGER + \u0027) or \u0027"},{"line_number":84,"context_line":"    \u0027(\u0027 + SYSTEM_READER_OR_DOMAIN_READER_LIST + \u0027)\u0027"},{"line_number":85,"context_line":")"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"ADMIN_OR_DOMAIN_ADMIN_OR_DOMAIN_MANAGER \u003d ("},{"line_number":88,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"975fbffa_0b363e02","line":85,"range":{"start_line":75,"start_character":0,"end_line":85,"end_character":1},"updated":"2024-08-28 05:00:40.000000000","message":"These changes are not needed. The context for a domain-manager includes all roles in the implied roles hierarchy, which includes \"reader\".\n\nA domain-manager would already be allowed because GRANTS_DOMAIN_READER evaluates to true for domain-manager as well as domain-member.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":72,"context_line":"    \u0027 \u0027 + DOMAIN_MATCHES_TARGET_DOMAIN + \u0027)\u0027"},{"line_number":73,"context_line":")"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":76,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"},{"line_number":77,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_MANAGER + \u0027) or \u0027"},{"line_number":78,"context_line":"    \u0027(\u0027 + SYSTEM_READER_OR_DOMAIN_READER + \u0027)\u0027"},{"line_number":79,"context_line":")"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER_LIST \u003d ("},{"line_number":82,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"},{"line_number":83,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_MANAGER + \u0027) or \u0027"},{"line_number":84,"context_line":"    \u0027(\u0027 + SYSTEM_READER_OR_DOMAIN_READER_LIST + \u0027)\u0027"},{"line_number":85,"context_line":")"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"ADMIN_OR_DOMAIN_ADMIN_OR_DOMAIN_MANAGER \u003d ("},{"line_number":88,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"5b890d60_2e46e1c5","line":85,"range":{"start_line":75,"start_character":0,"end_line":85,"end_character":1},"in_reply_to":"975fbffa_0b363e02","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"ADMIN_OR_DOMAIN_ADMIN_OR_DOMAIN_MANAGER \u003d ("},{"line_number":88,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"},{"line_number":89,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_ADMIN + \u0027) and \u0027"},{"line_number":90,"context_line":"    \u0027(\u0027 + DOMAIN_MATCHES_ROLE + \u0027) or \u0027"},{"line_number":91,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_MANAGER + \u0027) and \u0027"},{"line_number":92,"context_line":"    \u0027rule:domain_managed_target_role\u0027"},{"line_number":93,"context_line":")"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"DEPRECATED_REASON \u003d ("},{"line_number":96,"context_line":"    \"The assignment API is now aware of system scope and default roles.\""}],"source_content_type":"text/x-python","patch_set":9,"id":"a26a92bb_b550404e","line":93,"range":{"start_line":89,"start_character":4,"end_line":93,"end_character":1},"updated":"2024-08-28 05:00:40.000000000","message":"This policy can be simplified by removing GRANTS_DOMAIN_ADMIN and replacing with GRANTS_DOMAIN_MANAGER.\n\nEven though the explicit check for \"admin\" is removed, the context object for a domain-admin will also include the \"manager\" role because it is implied in the role hierarchy.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"88822e7710288ea69cf11dab7ff146014eb5786d","unresolved":true,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"ADMIN_OR_DOMAIN_ADMIN_OR_DOMAIN_MANAGER \u003d ("},{"line_number":88,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"},{"line_number":89,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_ADMIN + \u0027) and \u0027"},{"line_number":90,"context_line":"    \u0027(\u0027 + DOMAIN_MATCHES_ROLE + \u0027) or \u0027"},{"line_number":91,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_MANAGER + \u0027) and \u0027"},{"line_number":92,"context_line":"    \u0027rule:domain_managed_target_role\u0027"},{"line_number":93,"context_line":")"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"DEPRECATED_REASON \u003d ("},{"line_number":96,"context_line":"    \"The assignment API is now aware of system scope and default roles.\""}],"source_content_type":"text/x-python","patch_set":9,"id":"dcfc8b4b_121bb72b","line":93,"range":{"start_line":89,"start_character":4,"end_line":93,"end_character":1},"in_reply_to":"6b6f2b85_2088c6dc","updated":"2024-08-28 14:49:39.000000000","message":"You are correct, I missed that there are different checks after the AND.  This lgtm.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":true,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"ADMIN_OR_DOMAIN_ADMIN_OR_DOMAIN_MANAGER \u003d ("},{"line_number":88,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_REQUIRED + \u0027) or \u0027"},{"line_number":89,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_ADMIN + \u0027) and \u0027"},{"line_number":90,"context_line":"    \u0027(\u0027 + DOMAIN_MATCHES_ROLE + \u0027) or \u0027"},{"line_number":91,"context_line":"    \u0027(\u0027 + GRANTS_DOMAIN_MANAGER + \u0027) and \u0027"},{"line_number":92,"context_line":"    \u0027rule:domain_managed_target_role\u0027"},{"line_number":93,"context_line":")"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"DEPRECATED_REASON \u003d ("},{"line_number":96,"context_line":"    \"The assignment API is now aware of system scope and default roles.\""}],"source_content_type":"text/x-python","patch_set":9,"id":"6b6f2b85_2088c6dc","line":93,"range":{"start_line":89,"start_character":4,"end_line":93,"end_character":1},"in_reply_to":"a26a92bb_b550404e","updated":"2024-08-28 13:33:57.000000000","message":"The permission sets aren\u0027t equal though.\n\n( GRANTS_DOMAIN_ADMIN and DOMAIN_MATCHES_ROLE ) allows:\n\n- assign any global role\n- assigning any domain role of the same domain\n\nThis is intended for the domain admin persona and equals the current implementation.\n\n( GRANTS_DOMAIN_MANAGER and rule:domain_managed_target_role ) allows:\n\n- assigning only a specific set of global roles defined by the domain_managed_target_role rule\n- none of the domain roles\n\nThis is intended solely for domain managers.\n\nHence, simplifying the rule as you suggest would limit a domain admin more than the current implementation does and effectively introduce a change/regression in permission set of the domain admin persona, limiting it to the same level as domain managers.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"}],"keystone/common/policies/group.py":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":41,"context_line":"    \u0027domain_id:%(target.group.domain_id)s and \u0027"},{"line_number":42,"context_line":"    \u0027domain_id:%(target.user.domain_id)s\u0027"},{"line_number":43,"context_line":")"},{"line_number":44,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER_FOR_TARGET_GROUP \u003d ("},{"line_number":45,"context_line":"    \u0027(\u0027"},{"line_number":46,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":47,"context_line":"    + \u0027) or (\u0027"},{"line_number":48,"context_line":"    + DOMAIN_MANAGER_FOR_TARGET_GROUP_USER"},{"line_number":49,"context_line":"    + \u0027) or \u0027"},{"line_number":50,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_FOR_TARGET_GROUP_USER"},{"line_number":51,"context_line":")"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":54,"context_line":"    \u0027(role:reader and system_scope:all) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"4cd31ba0_2cefe7af","line":51,"range":{"start_line":44,"start_character":0,"end_line":51,"end_character":1},"updated":"2024-08-28 05:00:40.000000000","message":"This change is not necessary because domain-manager is already allowed by SYSTEM_READER_OR_DOMAIN_READER_FOR_TARGET_GROUP_USER due to implied roles.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    \u0027domain_id:%(target.group.domain_id)s and \u0027"},{"line_number":42,"context_line":"    \u0027domain_id:%(target.user.domain_id)s\u0027"},{"line_number":43,"context_line":")"},{"line_number":44,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER_FOR_TARGET_GROUP \u003d ("},{"line_number":45,"context_line":"    \u0027(\u0027"},{"line_number":46,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":47,"context_line":"    + \u0027) or (\u0027"},{"line_number":48,"context_line":"    + DOMAIN_MANAGER_FOR_TARGET_GROUP_USER"},{"line_number":49,"context_line":"    + \u0027) or \u0027"},{"line_number":50,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_FOR_TARGET_GROUP_USER"},{"line_number":51,"context_line":")"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":54,"context_line":"    \u0027(role:reader and system_scope:all) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"1314a434_0b6f5509","line":51,"range":{"start_line":44,"start_character":0,"end_line":51,"end_character":1},"in_reply_to":"4cd31ba0_2cefe7af","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":54,"context_line":"    \u0027(role:reader and system_scope:all) or \u0027"},{"line_number":55,"context_line":"    \u0027(role:reader and domain_id:%(target.group.domain_id)s)\u0027"},{"line_number":56,"context_line":")"},{"line_number":57,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":58,"context_line":"    \u0027(\u0027"},{"line_number":59,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":60,"context_line":"    + \u0027) or (\u0027"},{"line_number":61,"context_line":"    + DOMAIN_MANAGER_FOR_TARGET_GROUP"},{"line_number":62,"context_line":"    + \u0027) or \u0027"},{"line_number":63,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER"},{"line_number":64,"context_line":")"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"SYSTEM_ADMIN_OR_DOMAIN_ADMIN \u003d ("},{"line_number":67,"context_line":"    \u0027(role:admin and system_scope:all) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"b2dddc17_31a5003e","line":64,"range":{"start_line":57,"start_character":0,"end_line":64,"end_character":1},"updated":"2024-08-28 05:00:40.000000000","message":"This change is not necessary. domain-manager is already allowed by SYSTEM_READER_OR_DOMAIN_READER because of implied roles.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    \u0027(role:reader and system_scope:all) or \u0027"},{"line_number":55,"context_line":"    \u0027(role:reader and domain_id:%(target.group.domain_id)s)\u0027"},{"line_number":56,"context_line":")"},{"line_number":57,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER \u003d ("},{"line_number":58,"context_line":"    \u0027(\u0027"},{"line_number":59,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":60,"context_line":"    + \u0027) or (\u0027"},{"line_number":61,"context_line":"    + DOMAIN_MANAGER_FOR_TARGET_GROUP"},{"line_number":62,"context_line":"    + \u0027) or \u0027"},{"line_number":63,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER"},{"line_number":64,"context_line":")"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"SYSTEM_ADMIN_OR_DOMAIN_ADMIN \u003d ("},{"line_number":67,"context_line":"    \u0027(role:admin and system_scope:all) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"dc4f9b15_18fed019","line":64,"range":{"start_line":57,"start_character":0,"end_line":64,"end_character":1},"in_reply_to":"b2dddc17_31a5003e","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"}],"keystone/common/policies/project.py":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"1014181d4199979c8e48ec8b6e49eb63cce27a6a","unresolved":true,"context_lines":[{"line_number":24,"context_line":"    \u0027(\u0027"},{"line_number":25,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":26,"context_line":"    + \u0027) or \u0027"},{"line_number":27,"context_line":"    + \u0027(role:admin and domain_id:%(target.project.domain_id)s) or \u0027"},{"line_number":28,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER"},{"line_number":29,"context_line":")"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"03c146a7_7eb36e8a","line":27,"updated":"2024-07-26 14:25:37.000000000","message":"shouldn\u0027t that be manager?","commit_id":"d000ca210b4f6f0c52ef26de087f210deb034578"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"f52bb6d1327355d281210752e610ebcbd5f53275","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \u0027(\u0027"},{"line_number":25,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":26,"context_line":"    + \u0027) or \u0027"},{"line_number":27,"context_line":"    + \u0027(role:admin and domain_id:%(target.project.domain_id)s) or \u0027"},{"line_number":28,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER"},{"line_number":29,"context_line":")"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"0ca7ab16_aeabb61c","line":27,"in_reply_to":"03c146a7_7eb36e8a","updated":"2024-07-26 14:46:47.000000000","message":"Correct. Thanks for spotting! I didn\u0027t notice it during testing since \u0027manager\u0027 also implies \u0027reader\u0027 which also satisfies this check.\n\nFixed.","commit_id":"d000ca210b4f6f0c52ef26de087f210deb034578"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":20,"context_line":"    \u0027(role:reader and domain_id:%(target.project.domain_id)s) or \u0027"},{"line_number":21,"context_line":"    \u0027project_id:%(target.project.id)s\u0027"},{"line_number":22,"context_line":")"},{"line_number":23,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER \u003d ("},{"line_number":24,"context_line":"    \u0027(\u0027"},{"line_number":25,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":26,"context_line":"    + \u0027) or \u0027"},{"line_number":27,"context_line":"    + \u0027(role:manager and domain_id:%(target.project.domain_id)s) or \u0027"},{"line_number":28,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER"},{"line_number":29,"context_line":")"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"SYSTEM_ADMIN_OR_DOMAIN_ADMIN_OR_PROJECT_ADMIN \u003d ("},{"line_number":32,"context_line":"    \u0027(\u0027 + base.SYSTEM_ADMIN + \u0027) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"38422677_b385c510","line":29,"range":{"start_line":23,"start_character":0,"end_line":29,"end_character":1},"updated":"2024-08-28 05:00:40.000000000","message":"This change is not necessary.  domain-manager is already allowed by SYSTEM_READER_OR_DOMAIN_READER_FOR_TARGET_GROUP_USER because of implied roles.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":20,"context_line":"    \u0027(role:reader and domain_id:%(target.project.domain_id)s) or \u0027"},{"line_number":21,"context_line":"    \u0027project_id:%(target.project.id)s\u0027"},{"line_number":22,"context_line":")"},{"line_number":23,"context_line":"ADMIN_OR_DOMAIN_MANAGER_OR_SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER \u003d ("},{"line_number":24,"context_line":"    \u0027(\u0027"},{"line_number":25,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":26,"context_line":"    + \u0027) or \u0027"},{"line_number":27,"context_line":"    + \u0027(role:manager and domain_id:%(target.project.domain_id)s) or \u0027"},{"line_number":28,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER"},{"line_number":29,"context_line":")"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"SYSTEM_ADMIN_OR_DOMAIN_ADMIN_OR_PROJECT_ADMIN \u003d ("},{"line_number":32,"context_line":"    \u0027(\u0027 + base.SYSTEM_ADMIN + \u0027) or \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"69b7f831_5be5fdba","line":29,"range":{"start_line":23,"start_character":0,"end_line":29,"end_character":1},"in_reply_to":"38422677_b385c510","updated":"2024-08-28 13:33:57.000000000","message":"I think you meant SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER but yes, you are right. It seems already covered by that. I changed it.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":57,"context_line":"    \u0027(\u0027"},{"line_number":58,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":59,"context_line":"    + \u0027) or \u0027"},{"line_number":60,"context_line":"    + \u0027(role:manager and domain_id:%(target.user.domain_id)s) or \u0027"},{"line_number":61,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_OWNER"},{"line_number":62,"context_line":")"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"SYSTEM_READER_OR_DOMAIN_READER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"cc2a7586_c0b00635","line":61,"range":{"start_line":60,"start_character":6,"end_line":61,"end_character":45},"updated":"2024-08-28 05:00:40.000000000","message":"This change is not necessary. SYSTEM_READER_OR_DOMAIN_READER_OR_OWNER already allows domain-manager because of implied roles.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    \u0027(\u0027"},{"line_number":58,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":59,"context_line":"    + \u0027) or \u0027"},{"line_number":60,"context_line":"    + \u0027(role:manager and domain_id:%(target.user.domain_id)s) or \u0027"},{"line_number":61,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_OWNER"},{"line_number":62,"context_line":")"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"SYSTEM_READER_OR_DOMAIN_READER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"a644d3f3_dd7b4016","line":61,"range":{"start_line":60,"start_character":6,"end_line":61,"end_character":45},"in_reply_to":"cc2a7586_c0b00635","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":74,"context_line":"    \u0027(\u0027"},{"line_number":75,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":76,"context_line":"    + \u0027) or \u0027"},{"line_number":77,"context_line":"    + \u0027(role:manager and domain_id:%(target.domain_id)s) or \u0027"},{"line_number":78,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER"},{"line_number":79,"context_line":")"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"ADMIN_OR_DOMAIN_MANAGER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"2f89340b_cfd987f3","line":78,"range":{"start_line":77,"start_character":6,"end_line":78,"end_character":36},"updated":"2024-08-28 05:00:40.000000000","message":"This change is not necessary.  SYSTEM_READER_OR_DOMAIN_READER already allows domain-manager because of implied roles.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":74,"context_line":"    \u0027(\u0027"},{"line_number":75,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":76,"context_line":"    + \u0027) or \u0027"},{"line_number":77,"context_line":"    + \u0027(role:manager and domain_id:%(target.domain_id)s) or \u0027"},{"line_number":78,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER"},{"line_number":79,"context_line":")"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"ADMIN_OR_DOMAIN_MANAGER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"7ab94999_5804a4bb","line":78,"range":{"start_line":77,"start_character":6,"end_line":78,"end_character":36},"in_reply_to":"2f89340b_cfd987f3","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"}],"keystone/common/policies/role.py":[{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3908064748b6857434bfe6294cd6943ae396a76e","unresolved":true,"context_lines":[{"line_number":175,"context_line":"        name\u003dbase.IDENTITY % \u0027create_domain_role\u0027,"},{"line_number":176,"context_line":"        check_str\u003dADMIN_OR_DOMAIN_MANAGER_FOR_DOMAIN_ROLE,"},{"line_number":177,"context_line":"        description\u003d\u0027Create domain role.\u0027,"},{"line_number":178,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027domain\u0027, \u0027project\u0027],"},{"line_number":179,"context_line":"        operations\u003d[{\u0027path\u0027: \u0027/v3/roles\u0027,"},{"line_number":180,"context_line":"                     \u0027method\u0027: \u0027POST\u0027}],"},{"line_number":181,"context_line":"        deprecated_rule\u003ddeprecated_create_domain_role),"}],"source_content_type":"text/x-python","patch_set":1,"id":"d13d5bc4_540fd19a","line":178,"updated":"2024-07-16 12:10:37.000000000","message":"Note: adding the domain scope for the domain role operations here was necessary for the Domain Manager Persona to be able to work with domain roles. This introduces a change in behavior: a domain-scoped admin can now also use these endpoints, which was previously forbidden.\nSince a domain admin has implicitly a higher privilege level than a domain manager, I don\u0027t deem this problematic.","commit_id":"a955976aff04c9f8bd253871f8356d16a59397f0"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"a54457e06251d74562112218b8dffd1179386e93","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        name\u003dbase.IDENTITY % \u0027create_domain_role\u0027,"},{"line_number":176,"context_line":"        check_str\u003dADMIN_OR_DOMAIN_MANAGER_FOR_DOMAIN_ROLE,"},{"line_number":177,"context_line":"        description\u003d\u0027Create domain role.\u0027,"},{"line_number":178,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027domain\u0027, \u0027project\u0027],"},{"line_number":179,"context_line":"        operations\u003d[{\u0027path\u0027: \u0027/v3/roles\u0027,"},{"line_number":180,"context_line":"                     \u0027method\u0027: \u0027POST\u0027}],"},{"line_number":181,"context_line":"        deprecated_rule\u003ddeprecated_create_domain_role),"}],"source_content_type":"text/x-python","patch_set":1,"id":"72b159fa_d2e0f9cd","line":178,"in_reply_to":"d13d5bc4_540fd19a","updated":"2024-07-24 14:52:42.000000000","message":"Upon further investigation and testing I realized that domain role functions do note make much sense for a Domain Manager. I admit that such statement might sound contradictory at first but here is the reasoning:\n\nA Domain Manager is primarily meant to be used on customer side so that a customer can manage users, groups and role assignments within their domain as self-service functionality.\nTo prevent privelege escalation scenarios, in the original spec we agreed on limiting the roles that a Domain Manager may assign/revoke to a specific set via dedicated policy rule (\"rule:domain_managed_target_role\"). In the default in-code policies this set is defined as \"reader\", \"member\" and \"manager\". As such, a Domain Manager cannot assign the \"admin\" role for example.\nLet\u0027s assume a Domain Manager is able to manage (CRUD) domain roles. They could not choose any name of the global roles (e.g. \"member\") as those already exist as global roles. They would need to choose a different name. This name would not be part of the domain_managed_target_role list and as a result a Domain Manager would unable to assign a domain role they created.\nFurthermore, for any newly created domain role with non-standard name to have any effect, it needs to be considered in the policy.yaml of services in most cases. A customer Domain Manager is unable to access and edit this file since it is part of the cloud configuratin and cannot be influenced via the API.\n\nIn short:\n\n1. A customer Domain Manager cannot adjust the domain_managed_target_role list (part of Keystone\u0027s policy definitions) and as a result would not be able to assign any domain role they create.\n2. A customer Domain Manager cannot adjust the policy.yaml of OpenStack services and as such a newly created domain role would serve little to no purpose (its permission set cannot be specified by the Domain Manager).\n\nHence, I decided to omit the Domain Manager from the domain roles functionality and removed domain role management for the Domain Manager Persona from the patchset.","commit_id":"a955976aff04c9f8bd253871f8356d16a59397f0"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"1014181d4199979c8e48ec8b6e49eb63cce27a6a","unresolved":true,"context_lines":[{"line_number":21,"context_line":")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"ADMIN_OR_SYSTEM_READER_OR_DOMAIN_MANAGER \u003d ("},{"line_number":24,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_OR_SYSTEM_READER + \u0027) or \u0027 \u0027role:manager\u0027"},{"line_number":25,"context_line":")"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"DEPRECATED_REASON \u003d ("}],"source_content_type":"text/x-python","patch_set":5,"id":"2d954fb3_5b8f61b4","line":24,"updated":"2024-07-26 14:25:37.000000000","message":"could you please concatenate sting properly \") or role:manager\" (drop a \") or \u0027 \u0027\")","commit_id":"d000ca210b4f6f0c52ef26de087f210deb034578"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"f52bb6d1327355d281210752e610ebcbd5f53275","unresolved":false,"context_lines":[{"line_number":21,"context_line":")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"ADMIN_OR_SYSTEM_READER_OR_DOMAIN_MANAGER \u003d ("},{"line_number":24,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_OR_SYSTEM_READER + \u0027) or \u0027 \u0027role:manager\u0027"},{"line_number":25,"context_line":")"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"DEPRECATED_REASON \u003d ("}],"source_content_type":"text/x-python","patch_set":5,"id":"61100479_7742323c","line":24,"in_reply_to":"2d954fb3_5b8f61b4","updated":"2024-07-26 14:46:47.000000000","message":"Good catch. This seems to have gotten garbled up during the blackify rebase I did yesterday.\n\nFixed.","commit_id":"d000ca210b4f6f0c52ef26de087f210deb034578"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"23d83bcbf68111b9ec50f903808a59ae0e9e5df4","unresolved":true,"context_lines":[{"line_number":21,"context_line":")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"ADMIN_OR_SYSTEM_READER_OR_DOMAIN_MANAGER \u003d ("},{"line_number":24,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_OR_SYSTEM_READER + \u0027) or role:manager\u0027"},{"line_number":25,"context_line":")"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"DEPRECATED_REASON \u003d ("}],"source_content_type":"text/x-python","patch_set":7,"id":"16fa0d5c_668dbe68","line":24,"range":{"start_line":24,"start_character":51,"end_line":24,"end_character":63},"updated":"2024-08-02 20:34:18.000000000","message":"`role:manager` here is only a partial check for a domain-manager, since it only checks that the token has the \"manager\" role, but does not ensure the token is domain-scoped.","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ad61bfb7f363deb9a5fa9fa0a317f8e2b476e098","unresolved":false,"context_lines":[{"line_number":21,"context_line":")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"ADMIN_OR_SYSTEM_READER_OR_DOMAIN_MANAGER \u003d ("},{"line_number":24,"context_line":"    \u0027(\u0027 + base.RULE_ADMIN_OR_SYSTEM_READER + \u0027) or role:manager\u0027"},{"line_number":25,"context_line":")"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"DEPRECATED_REASON \u003d ("}],"source_content_type":"text/x-python","patch_set":7,"id":"e4dbb4b0_1def9058","line":24,"range":{"start_line":24,"start_character":51,"end_line":24,"end_character":63},"in_reply_to":"16fa0d5c_668dbe68","updated":"2024-08-05 15:13:14.000000000","message":"I changed it to check for a domain_id that is not None. Based on my testing this works for differentiating between domain manager and project manager.","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"23d83bcbf68111b9ec50f903808a59ae0e9e5df4","unresolved":true,"context_lines":[{"line_number":104,"context_line":"    ),"},{"line_number":105,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":106,"context_line":"        name\u003dbase.IDENTITY % \u0027list_roles\u0027,"},{"line_number":107,"context_line":"        check_str\u003dADMIN_OR_SYSTEM_READER_OR_DOMAIN_MANAGER,"},{"line_number":108,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027domain\u0027, \u0027project\u0027],"},{"line_number":109,"context_line":"        description\u003d\u0027List roles.\u0027,"},{"line_number":110,"context_line":"        operations\u003d["},{"line_number":111,"context_line":"            {\u0027path\u0027: \u0027/v3/roles\u0027, \u0027method\u0027: \u0027GET\u0027},"}],"source_content_type":"text/x-python","patch_set":7,"id":"e0ba06ad_59db9ceb","line":108,"range":{"start_line":107,"start_character":8,"end_line":108,"end_character":52},"updated":"2024-08-02 20:34:18.000000000","message":"As I mentioned above, the additional `role:manager` is only a partial check for DOMAIN_MANAGER since it does not verify the scope.\n\nI think it is reasonable to expect that a project-manager persona may be added in the future either in our policies or some other project\u0027s policies.  There is nothing currently preventing a role assignment of (Role\u003d\"manager\", Resrouce\u003d$PROJECT_ID, Identity\u003d$USER_ID), nor do I think it should be prevented.\n\nFor this policy, `scope_types` is going to allow all tokens except for unscoped tokens.  After the scope_types check, the policy enforcer could be processing a token of any of those scopes, so we need to check for scope again in the policy itself to narrow access to specific scopes.  If we examine the policy in parts by looking at the clauses that are separated by `\u0027 or \u0027` we get:\n\n\n`ADMIN` which resolves to `\"role:admin or is_admin:1\"` - here we don\u0027t care about scope, only that the user has been assigned the \"admin\" role on _some_ scope.  Could be system-admin, or domain-admin or project-admin.  All 3 personas will get access since scope is not checked.\n\n\n`SYSTEM_READER` resolves to `\"role:reader and system_scope:all\"` - here we do care about scope because we want to exclude domain-reader and project-reader.  This works because the context object for system-scoped tokens includes the string \"all\" as the value for \"system_scope\".  For all other scopes, this value is null which makes this part of the policy fail.\n\n\n`DOMAN_MANAGER` -  Here we want to exclude a potential project-manager.  Note that it\u0027s impossible to exclude system-manager because system-manager implies system-reader and they are allowed because of the previous clause.\n\nTo exclude project-scoped tokens we need to ensure that the token being presented with the request is domain-scoped. For this purpose we can check the context object for the top level key of `domain_id` which is only set when the token is domain-scoped.\n\nThe tricky part about this particular API is that it\u0027s an API to list all domains, so we don\u0027t have any information about a possible target we could use to compare the domain_id to.  Filtering does address this somewhat, e.g.\n\n    GET /v3/roles?domain_id\u003d$REQUESTED_DOMAIN_ID\n\nResults in `{\u0027domain_id\u0027: \"$REQUESTED_DOMAIN_ID\"}` in the policy_dict, so we could do something like `\"role:manager and domain_id:%(domain_id)s\"`, however, the enforcer currently uses a different rule for requests that include domain_id in the query string.\n\nOne way to get around this is to just reflect the domain_id from the context object into the policy object, and updating this rule to use that reflected value. See my comment in `keystone/api/roles.py`","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ad61bfb7f363deb9a5fa9fa0a317f8e2b476e098","unresolved":true,"context_lines":[{"line_number":104,"context_line":"    ),"},{"line_number":105,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":106,"context_line":"        name\u003dbase.IDENTITY % \u0027list_roles\u0027,"},{"line_number":107,"context_line":"        check_str\u003dADMIN_OR_SYSTEM_READER_OR_DOMAIN_MANAGER,"},{"line_number":108,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027domain\u0027, \u0027project\u0027],"},{"line_number":109,"context_line":"        description\u003d\u0027List roles.\u0027,"},{"line_number":110,"context_line":"        operations\u003d["},{"line_number":111,"context_line":"            {\u0027path\u0027: \u0027/v3/roles\u0027, \u0027method\u0027: \u0027GET\u0027},"}],"source_content_type":"text/x-python","patch_set":7,"id":"192cef7b_6cb057ac","line":108,"range":{"start_line":107,"start_character":8,"end_line":108,"end_character":52},"in_reply_to":"e0ba06ad_59db9ceb","updated":"2024-08-05 15:13:14.000000000","message":"Thank you for the detailed writeup! It really helps to understand the whole situation.\n\nA project-scoped token does not seem to present a domain_id in the token context, so I implemented a check in the policy rule above to only accept the manager role when the domain_id is also present. This circumvents the whole tricky part about not having a domain-scoped target to check the id against. Would this suffice from your point of view?","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"23d83bcbf68111b9ec50f903808a59ae0e9e5df4","unresolved":true,"context_lines":[{"line_number":150,"context_line":"    ),"},{"line_number":151,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":152,"context_line":"        name\u003dbase.IDENTITY % \u0027list_domain_roles\u0027,"},{"line_number":153,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_SYSTEM_READER,"},{"line_number":154,"context_line":"        description\u003d\u0027List domain roles.\u0027,"},{"line_number":155,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":156,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":7,"id":"e5afbe7c_5b8b2f0d","line":153,"range":{"start_line":153,"start_character":23,"end_line":153,"end_character":50},"updated":"2024-08-02 20:34:18.000000000","message":"Domain-manager may also want to list roles filtered by its own domain.  I think maybe that was the intention of adding the domain being filtered to the policy_dict target in nkeystone/api/roles.py?","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    ),"},{"line_number":151,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":152,"context_line":"        name\u003dbase.IDENTITY % \u0027list_domain_roles\u0027,"},{"line_number":153,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_SYSTEM_READER,"},{"line_number":154,"context_line":"        description\u003d\u0027List domain roles.\u0027,"},{"line_number":155,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":156,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":7,"id":"3e209c78_48525274","line":153,"range":{"start_line":153,"start_character":23,"end_line":153,"end_character":50},"in_reply_to":"76e267f7_ba806630","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ad61bfb7f363deb9a5fa9fa0a317f8e2b476e098","unresolved":true,"context_lines":[{"line_number":150,"context_line":"    ),"},{"line_number":151,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":152,"context_line":"        name\u003dbase.IDENTITY % \u0027list_domain_roles\u0027,"},{"line_number":153,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_SYSTEM_READER,"},{"line_number":154,"context_line":"        description\u003d\u0027List domain roles.\u0027,"},{"line_number":155,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":156,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":7,"id":"76e267f7_ba806630","line":153,"range":{"start_line":153,"start_character":23,"end_line":153,"end_character":50},"in_reply_to":"e5afbe7c_5b8b2f0d","updated":"2024-08-05 15:13:14.000000000","message":"I think this would be confusing for such users because they cannot use domain roles at all, see my second comment here: https://review.opendev.org/c/openstack/keystone/+/924132/1..7/keystone/common/policies/role.py#b178\n\nIn the early versions of the patchset, access to the domain role endpoints was still included for domain managers but I removed it. I don\u0027t think domain roles should be visible to domain managers if they cannot assign/revoke them or change their permission set.","commit_id":"28fd7979dcab61f2e8c4962e3d1c251bfd28936d"}],"keystone/common/policies/user.py":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":24,"context_line":"    \u0027(\u0027"},{"line_number":25,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":26,"context_line":"    + \u0027) or \u0027"},{"line_number":27,"context_line":"    + \u0027(role:manager and token.domain.id:%(target.user.domain_id)s) or \u0027"},{"line_number":28,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_USER"},{"line_number":29,"context_line":")"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"SYSTEM_READER_OR_DOMAIN_READER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"e7acfabd_537e81fe","line":28,"range":{"start_line":27,"start_character":4,"end_line":28,"end_character":44},"updated":"2024-08-28 05:00:40.000000000","message":"This change is not necessary.  SYSTEM_READER_OR_DOMAIN_READER_OR_USER already allows domain-manager because of implied roles.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \u0027(\u0027"},{"line_number":25,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":26,"context_line":"    + \u0027) or \u0027"},{"line_number":27,"context_line":"    + \u0027(role:manager and token.domain.id:%(target.user.domain_id)s) or \u0027"},{"line_number":28,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER_OR_USER"},{"line_number":29,"context_line":")"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"SYSTEM_READER_OR_DOMAIN_READER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"608a0260_33a6e362","line":28,"range":{"start_line":27,"start_character":4,"end_line":28,"end_character":44},"in_reply_to":"e7acfabd_537e81fe","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e860f2f1d8d0750ca069153e9c2430f08265f428","unresolved":true,"context_lines":[{"line_number":40,"context_line":"    \u0027(\u0027"},{"line_number":41,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":42,"context_line":"    + \u0027) or \u0027"},{"line_number":43,"context_line":"    + \u0027(role:manager and token.domain.id:%(target.domain_id)s) or \u0027"},{"line_number":44,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER"},{"line_number":45,"context_line":")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"ADMIN_OR_DOMAIN_MANAGER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"f77f4090_2e9d47c5","line":44,"range":{"start_line":43,"start_character":4,"end_line":44,"end_character":36},"updated":"2024-08-28 05:00:40.000000000","message":"This change is not necessary. SYSTEM_READER_OR_DOMAIN_READER already allows domain-manager because of implied roles.","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ce05eafe46d7104e9c0339e0e751c995f0fe66f3","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    \u0027(\u0027"},{"line_number":41,"context_line":"    + base.RULE_ADMIN_REQUIRED"},{"line_number":42,"context_line":"    + \u0027) or \u0027"},{"line_number":43,"context_line":"    + \u0027(role:manager and token.domain.id:%(target.domain_id)s) or \u0027"},{"line_number":44,"context_line":"    + SYSTEM_READER_OR_DOMAIN_READER"},{"line_number":45,"context_line":")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"ADMIN_OR_DOMAIN_MANAGER \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"53f57286_94eecf43","line":44,"range":{"start_line":43,"start_character":4,"end_line":44,"end_character":36},"in_reply_to":"f77f4090_2e9d47c5","updated":"2024-08-28 13:33:57.000000000","message":"Done","commit_id":"c2b011b9646bf99cddf45fef6bb1b8efe075b99b"}]}