)]}'
{"nova/policies/admin_password.py":[{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"049bec2897afdd4f2a3e00679363262b6fb4d053","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        # either a system administrator should be able to reset this with a"},{"line_number":37,"context_line":"        # system-scoped token (they should be able to do this for any"},{"line_number":38,"context_line":"        # instance). Or a user with a role on a project should be able to reset"},{"line_number":39,"context_line":"        # the password if they are a member of that project. This might require"},{"line_number":40,"context_line":"        # an addition check in nova to make sure the instance in question"},{"line_number":41,"context_line":"        # belongs to the project from the token scope (e.g. prevent a user from"},{"line_number":42,"context_line":"        # resetting the password on an instance they don\u0027t have access to)."}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_f3cdd95c","line":39,"range":{"start_line":39,"start_character":66,"end_line":39,"end_character":71},"updated":"2018-03-16 17:07:21.000000000","message":"not might, will. With appropriate caveats for backward compat controlled via config.\n\nAnd if we\u0027re not doing that in this commit, let\u0027s mark it as a TODO (or remove the sentence... are we going to say this everywhere?)","commit_id":"43c52f903cdfe98b9c3777ad2cd8b483bfd32308"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"049bec2897afdd4f2a3e00679363262b6fb4d053","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        # system-scoped token (they should be able to do this for any"},{"line_number":38,"context_line":"        # instance). Or a user with a role on a project should be able to reset"},{"line_number":39,"context_line":"        # the password if they are a member of that project. This might require"},{"line_number":40,"context_line":"        # an addition check in nova to make sure the instance in question"},{"line_number":41,"context_line":"        # belongs to the project from the token scope (e.g. prevent a user from"},{"line_number":42,"context_line":"        # resetting the password on an instance they don\u0027t have access to)."},{"line_number":43,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_73d8e951","line":40,"range":{"start_line":40,"start_character":13,"end_line":40,"end_character":21},"updated":"2018-03-16 17:07:21.000000000","message":"nit: additional","commit_id":"43c52f903cdfe98b9c3777ad2cd8b483bfd32308"}],"nova/tests/unit/test_policy.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"cfbadd0fb4dfb9de2f5e4dffa0ebb143206dede8","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        super(RealRolePolicyTestCase, self).setUp()"},{"line_number":269,"context_line":"        self.policy \u003d self.useFixture(policy_fixture.RealPolicyFixture())"},{"line_number":270,"context_line":"        self.non_admin_context \u003d context.RequestContext("},{"line_number":271,"context_line":"            user_id\u003d\u0027fake\u0027, project_id\u003d\u0027fake\u0027, roles\u003d[\u0027member\u0027]"},{"line_number":272,"context_line":"        )"},{"line_number":273,"context_line":"        self.admin_context \u003d context.RequestContext(\u0027fake\u0027, \u0027fake\u0027, True,"},{"line_number":274,"context_line":"                                                     roles\u003d[\u0027member\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_fb3e7089","line":271,"updated":"2018-04-20 21:52:39.000000000","message":"I\u0027m not sure what nova\u0027s convention is around this, but i was hacking on this locally and ended up seeing some issues with unrelated changes because these weren\u0027t being passed as kwargs, despite the method signature of the Context object [0].\n\nNot a big deal, I just noticed that if I attempted to swap the second \u0027fake\u0027 argument with something else (say for system scope), it would still set it as a project_id because it was process arguments based on position. I figured adding the keyword arguments would add some clarity to what we\u0027re passing to context and also prevent oddities if other arguments are added in different positions to the method signature (which should protect callers from breaking).\n\nI can revert this too, just wasn\u0027t sure if nova had a convention around this.\n\n[0] https://github.com/openstack/nova/blob/master/nova/context.py#L90-L93","commit_id":"fc1ca00b1f39d750f7fd7eb94b889eee94b007a6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d1f889021296cf3b4ab1bf02ab20a1e062f1b91","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        super(RealRolePolicyTestCase, self).setUp()"},{"line_number":269,"context_line":"        self.policy \u003d self.useFixture(policy_fixture.RealPolicyFixture())"},{"line_number":270,"context_line":"        self.non_admin_context \u003d context.RequestContext("},{"line_number":271,"context_line":"            user_id\u003d\u0027fake\u0027, project_id\u003d\u0027fake\u0027, roles\u003d[\u0027member\u0027]"},{"line_number":272,"context_line":"        )"},{"line_number":273,"context_line":"        self.admin_context \u003d context.RequestContext(\u0027fake\u0027, \u0027fake\u0027, True,"},{"line_number":274,"context_line":"                                                     roles\u003d[\u0027member\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_a1150f9e","line":271,"in_reply_to":"9f6a8fd7_fb3e7089","updated":"2018-04-23 17:01:16.000000000","message":"I\u0027m fine with this change, Lance. ++\n\nOthers may object to the closing paren on its own newline, though (even if I personally prefer this code style). Just a word of warning :)","commit_id":"fc1ca00b1f39d750f7fd7eb94b889eee94b007a6"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"cfbadd0fb4dfb9de2f5e4dffa0ebb143206dede8","unresolved":false,"context_lines":[{"line_number":514,"context_line":"        fixture.config(group\u003d\u0027oslo_policy\u0027, enforce_scope\u003dTrue)"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        # These roles are guaranteed to be provided in a deployment"},{"line_number":517,"context_line":"        self.default_roles \u003d [\u0027admin\u0027]"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        self.fake_system_scope \u003d {\u0027all\u0027: True}"},{"line_number":520,"context_line":"        self.fake_user_id \u003d \u0027fake-user-id\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_5b0c3cec","line":517,"range":{"start_line":517,"start_character":8,"end_line":517,"end_character":38},"updated":"2018-04-20 21:52:39.000000000","message":"This will likely get expanded if/when the openstack-spec merges [0], which should yield:\n\n  self.default_roles [\u0027admin\u0027, \u0027member\u0027, \u0027auditor\u0027]\n\nThen for each role + scope pair, we essentially have a persona to exercise (e.g. modeling a situation where Jane has role admin on the system, Alice has role member on the system, Bob has role auditor on the system, Steve has role admin on a project, etc.)\n\nIf anyone has ideas for how to make sure these tests stay DRY, I\u0027m all ears. I tinkered with expanding out those roles locally, and I essentially came up with what you see below, where each test case exercises a persona. Conversely, they could be structured to be:\n\n  def test_system_admin_can_execute_os_admin_password(self):\n      ...\n\nBut I feel that might result in a test explosion.\n\nThoughts?\n\n[0] https://review.openstack.org/#/c/523973/","commit_id":"fc1ca00b1f39d750f7fd7eb94b889eee94b007a6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d1f889021296cf3b4ab1bf02ab20a1e062f1b91","unresolved":false,"context_lines":[{"line_number":514,"context_line":"        fixture.config(group\u003d\u0027oslo_policy\u0027, enforce_scope\u003dTrue)"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        # These roles are guaranteed to be provided in a deployment"},{"line_number":517,"context_line":"        self.default_roles \u003d [\u0027admin\u0027]"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        self.fake_system_scope \u003d {\u0027all\u0027: True}"},{"line_number":520,"context_line":"        self.fake_user_id \u003d \u0027fake-user-id\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_e14c67c9","line":517,"range":{"start_line":517,"start_character":8,"end_line":517,"end_character":38},"in_reply_to":"9f6a8fd7_5b0c3cec","updated":"2018-04-23 17:01:16.000000000","message":"I\u0027m not a huge fan of DRY\u0027ing up tests. I feel being explicit (if wordy) about the fixtures being utilized in a test makes the test easier to read and understand, especially years after the test was originally written.","commit_id":"fc1ca00b1f39d750f7fd7eb94b889eee94b007a6"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1d53f471281721e47cb2ef66c1064e3e40707268","unresolved":false,"context_lines":[{"line_number":514,"context_line":"        fixture.config(group\u003d\u0027oslo_policy\u0027, enforce_scope\u003dTrue)"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        # These roles are guaranteed to be provided in a deployment"},{"line_number":517,"context_line":"        self.default_roles \u003d [\u0027admin\u0027]"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        self.fake_system_scope \u003d {\u0027all\u0027: True}"},{"line_number":520,"context_line":"        self.fake_user_id \u003d \u0027fake-user-id\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_944e076d","line":517,"range":{"start_line":517,"start_character":8,"end_line":517,"end_character":38},"in_reply_to":"9f6a8fd7_e14c67c9","updated":"2018-04-23 17:26:55.000000000","message":"That\u0027s totally understandable and I agree with that perspective. As I work through a few more of these, I keep them structured the way they are at the cost of modifying data structures in several tests when we add new policies.\n\nI suppose we can always change it later to reduce duplication.","commit_id":"fc1ca00b1f39d750f7fd7eb94b889eee94b007a6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d1f889021296cf3b4ab1bf02ab20a1e062f1b91","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        # These roles are guaranteed to be provided in a deployment"},{"line_number":517,"context_line":"        self.default_roles \u003d [\u0027admin\u0027]"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        self.fake_system_scope \u003d {\u0027all\u0027: True}"},{"line_number":520,"context_line":"        self.fake_user_id \u003d \u0027fake-user-id\u0027"},{"line_number":521,"context_line":"        self.fake_project_id \u003d \u0027fake-project-id\u0027"},{"line_number":522,"context_line":"        self.target \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_a1f18f6d","line":519,"range":{"start_line":519,"start_character":8,"end_line":519,"end_character":46},"updated":"2018-04-23 17:01:16.000000000","message":"Interesting. From looking at the oslo.context patch you linked in the Depends-On, I would have expected the system kwarg to oslo_context.RequestContext to be a boolean value, not a dict?","commit_id":"fc1ca00b1f39d750f7fd7eb94b889eee94b007a6"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1d53f471281721e47cb2ef66c1064e3e40707268","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        # These roles are guaranteed to be provided in a deployment"},{"line_number":517,"context_line":"        self.default_roles \u003d [\u0027admin\u0027]"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        self.fake_system_scope \u003d {\u0027all\u0027: True}"},{"line_number":520,"context_line":"        self.fake_user_id \u003d \u0027fake-user-id\u0027"},{"line_number":521,"context_line":"        self.fake_project_id \u003d \u0027fake-project-id\u0027"},{"line_number":522,"context_line":"        self.target \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_b40c6ba5","line":519,"range":{"start_line":519,"start_character":8,"end_line":519,"end_character":46},"in_reply_to":"9f6a8fd7_a1f18f6d","updated":"2018-04-23 17:26:55.000000000","message":"Good question. When we originally designed it, we envisioned it being a boolean. We changed it in later patches because we felt a dictionary was more appropriate and allowed for more flexibility in the future.\n\nContext can associate to project IDs and domain IDs, which are pretty specific. With system-scope we can make it a boolean, but that kind of prevents us from elaborating on system scope in the future. For example, today you can get a token scoped to \"system: {\u0027all\u0027: True}\", but in the future we could enhance keystone to expose the entries of the service catalog in such a way that allowed users to scope to parts of the system. Or allow administrators to grant authorization to a part of the overall system (e.g. you could give me a role on the identity portion of the system, only allowing me to do identity management).\n\nWe\u0027re pretty far from that today, but it seemed like it would be useful to leave the implementation open to moving in that direction if we wanted to in the future. We\u0027re hoping the current dict representation allows for that.","commit_id":"fc1ca00b1f39d750f7fd7eb94b889eee94b007a6"}]}
