)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"a169b939b9342df8c08d96c1488e3088e858bfaf","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"extract policy from the controllers and move the"},{"line_number":10,"context_line":"logic to a stand alone file.  This is a step"},{"line_number":11,"context_line":"toward standarizing policy enforcement across Services"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I885bc439d69e7561fddbdedcc01ec591dc68597f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da86d52c_51b76cd8","line":11,"updated":"2015-02-02 19:34:30.000000000","message":"(nit) services","commit_id":"2a33e2c1468cac4f8e8ff43a34ef4953910a6fe7"}],"keystone/assignment/controllers.py":[{"author":{"_account_id":10765,"name":"Andre Aranha","email":"afaranha@lsd.ufcg.edu.br","username":"afaranha"},"change_message_id":"d44c6fecad24c7d94174cc88832c617930f6f3df","unresolved":false,"context_lines":[{"line_number":535,"context_line":"            ref[\u0027domain\u0027] \u003d self.assignment_api.get_domain(domain_id)"},{"line_number":536,"context_line":"        else:"},{"line_number":537,"context_line":"            ref[\u0027project\u0027] \u003d self.assignment_api.get_project(project_id)"},{"line_number":538,"context_line":"            ref[\u0027member\u0027] \u003d ref[\u0027project\u0027]"},{"line_number":539,"context_line":""},{"line_number":540,"context_line":"        self.check_protection(context, protection, ref)"},{"line_number":541,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_d308ea93","line":538,"updated":"2014-12-16 14:48:58.000000000","message":"Don\u0027t we need to put this ref[\u0027member\u0027] also for the others members?\nfor domain, user and group?","commit_id":"67bfa70515c674197a257e0796d651cf7c0129d1"}],"keystone/common/authorization.py":[{"author":{"_account_id":10765,"name":"Andre Aranha","email":"afaranha@lsd.ufcg.edu.br","username":"afaranha"},"change_message_id":"d44c6fecad24c7d94174cc88832c617930f6f3df","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        token_ref \u003d token_model.KeystoneToken("},{"line_number":168,"context_line":"            token_id\u003dcontext[\u0027subject_token_id\u0027],"},{"line_number":169,"context_line":"            token_data\u003dtoken_provider_api.validate_token("},{"line_number":170,"context_line":"                context[\u0027subject_token_id\u0027]))"},{"line_number":171,"context_line":"        policy_dict.setdefault(\u0027target\u0027, {})"},{"line_number":172,"context_line":"        policy_dict[\u0027target\u0027].setdefault(member_name, {})"},{"line_number":173,"context_line":"        policy_dict[\u0027target\u0027][member_name][\u0027user_id\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_50feb039","line":170,"updated":"2014-12-16 14:48:58.000000000","message":"Just a minor comment, as this patch is a refactoring one, in my opinion we should replace this second context[\u0027subject_token_id\u0027] for the variable token_id.","commit_id":"67bfa70515c674197a257e0796d651cf7c0129d1"},{"author":{"_account_id":4,"name":"Dolph Mathews","email":"dolph.mathews@gmail.com","username":"dolph"},"change_message_id":"0c3ee5e169ead2d4f10f69c19e72b50b0cdb0242","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from keystone.i18n import _, _LW"},{"line_number":21,"context_line":"from keystone.models import token_model"},{"line_number":22,"context_line":"from keystone.openstack.common import log"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"AUTH_CONTEXT_ENV \u003d \u0027KEYSTONE_AUTH_CONTEXT\u0027"},{"line_number":26,"context_line":"\"\"\"Environment variable used to convey the Keystone auth context."}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_7aa62255","side":"PARENT","line":23,"updated":"2014-12-16 18:11:55.000000000","message":"double space after imports is convention","commit_id":"20fc8152a2ba725484a63e96294b67ad1a89147d"},{"author":{"_account_id":10765,"name":"Andre Aranha","email":"afaranha@lsd.ufcg.edu.br","username":"afaranha"},"change_message_id":"b2d7d616dff3fa29dfe5217cca7a720ddece4037","unresolved":false,"context_lines":[{"line_number":155,"context_line":"            ref \u003d controller.get_member_from_driver(kwargs[key])"},{"line_number":156,"context_line":"            policy_dict[\u0027target\u0027] \u003d {"},{"line_number":157,"context_line":"                member_name: ref,"},{"line_number":158,"context_line":"                \u0027member\u0027: ref"},{"line_number":159,"context_line":"            }"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    # TODO(henry-nash): Move this entire code to a member"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_739712ec","line":158,"updated":"2014-12-30 19:41:53.000000000","message":"Since this is a refactoring patch, should we remove this \"\u0027member\u0027: ref\" here and put into another patch ( https://review.openstack.org/#/c/142162/ )?","commit_id":"0c1ede6a7b76e41c6a5e85b062a90f93f00eb942"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"a169b939b9342df8c08d96c1488e3088e858bfaf","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from keystone.i18n import _, _LW"},{"line_number":21,"context_line":"from keystone.models import token_model"},{"line_number":22,"context_line":"from keystone.openstack.common import log"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"AUTH_CONTEXT_ENV \u003d \u0027KEYSTONE_AUTH_CONTEXT\u0027"},{"line_number":26,"context_line":"\"\"\"Environment variable used to convey the Keystone auth context."}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_f1cce063","side":"PARENT","line":23,"updated":"2015-02-02 19:34:30.000000000","message":"This newline should stay","commit_id":"3a41a9b3490a5993f6eebec1666d72c2367e5009"}],"keystone/common/controller.py":[{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"a169b939b9342df8c08d96c1488e3088e858bfaf","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                    and self.get_member_from_driver is not None) else None"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"                member_name \u003d (self.member_name"},{"line_number":84,"context_line":"                               if hasattr(self, \u0027member_name\u0027) else None)"},{"line_number":85,"context_line":"                token_provider_api \u003d (self.token_provider_api"},{"line_number":86,"context_line":"                                      if hasattr(self, \u0027token_provider_api\u0027)"},{"line_number":87,"context_line":"                                      else None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_9442b20e","line":84,"updated":"2015-02-02 19:34:30.000000000","message":"What is the case for this hasattr? I don\u0027t see it in the existing code and I think it weakens the interface.","commit_id":"2a33e2c1468cac4f8e8ff43a34ef4953910a6fe7"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"a169b939b9342df8c08d96c1488e3088e858bfaf","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                member_name \u003d (self.member_name"},{"line_number":84,"context_line":"                               if hasattr(self, \u0027member_name\u0027) else None)"},{"line_number":85,"context_line":"                token_provider_api \u003d (self.token_provider_api"},{"line_number":86,"context_line":"                                      if hasattr(self, \u0027token_provider_api\u0027)"},{"line_number":87,"context_line":"                                      else None)"},{"line_number":88,"context_line":"                policy_api \u003d (self.policy_api"},{"line_number":89,"context_line":"                              if hasattr(self, \u0027policy_api\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_140402bf","line":86,"updated":"2015-02-02 19:34:30.000000000","message":"Same for this hasattr. This one will actually break if it turns out of be None because several attributes are accessed on the object.","commit_id":"2a33e2c1468cac4f8e8ff43a34ef4953910a6fe7"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"a169b939b9342df8c08d96c1488e3088e858bfaf","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                                      if hasattr(self, \u0027token_provider_api\u0027)"},{"line_number":87,"context_line":"                                      else None)"},{"line_number":88,"context_line":"                policy_api \u003d (self.policy_api"},{"line_number":89,"context_line":"                              if hasattr(self, \u0027policy_api\u0027)"},{"line_number":90,"context_line":"                              else None)"},{"line_number":91,"context_line":"                authorization.protect("},{"line_number":92,"context_line":"                    controller,"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_4054f020","line":89,"updated":"2015-02-02 19:34:30.000000000","message":"This will also break if the attribute doesn\u0027t exist because `None.enforce()` will be called.","commit_id":"2a33e2c1468cac4f8e8ff43a34ef4953910a6fe7"}],"keystone/openstack/common/policy.py":[{"author":{"_account_id":10765,"name":"Andre Aranha","email":"afaranha@lsd.ufcg.edu.br","username":"afaranha"},"change_message_id":"d44c6fecad24c7d94174cc88832c617930f6f3df","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        Requires that at least one rule accept in order to return True."},{"line_number":507,"context_line":"        \"\"\""},{"line_number":508,"context_line":"        for rule in self.rules:"},{"line_number":509,"context_line":"            try:"},{"line_number":510,"context_line":"                if rule(target, cred, enforcer):"},{"line_number":511,"context_line":"                    return True"},{"line_number":512,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_f38246c7","line":509,"updated":"2014-12-16 14:48:58.000000000","message":"Another minor comment. Should we use the same pattern as the code above? using the \"try\" or inside the \"for\" in both code or outside it?","commit_id":"67bfa70515c674197a257e0796d651cf7c0129d1"},{"author":{"_account_id":4,"name":"Dolph Mathews","email":"dolph.mathews@gmail.com","username":"dolph"},"change_message_id":"0c3ee5e169ead2d4f10f69c19e72b50b0cdb0242","unresolved":false,"context_lines":[{"line_number":464,"context_line":"            for rule in self.rules:"},{"line_number":465,"context_line":"                if not rule(target, cred, enforcer):"},{"line_number":466,"context_line":"                    return False"},{"line_number":467,"context_line":"        except Exception:"},{"line_number":468,"context_line":"            return False"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_da43966a","line":467,"updated":"2014-12-16 18:11:55.000000000","message":"need inline comments explaining why except Exception -- and then completely swallowing the failure -- is appropriate here, instead of catching something more specific.\n\ni imagine this should be reverted, though.","commit_id":"ba52915d4c1e37d53836beaa87fc45509684919c"},{"author":{"_account_id":2218,"name":"Adam Young","email":"adam@younglogic.com","username":"ayoung"},"change_message_id":"96c2b567f0974252643e84d1e9a59c019f9d92f8","unresolved":false,"context_lines":[{"line_number":464,"context_line":"            for rule in self.rules:"},{"line_number":465,"context_line":"                if not rule(target, cred, enforcer):"},{"line_number":466,"context_line":"                    return False"},{"line_number":467,"context_line":"        except Exception:"},{"line_number":468,"context_line":"            return False"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_f03f91d4","line":467,"in_reply_to":"3a961159_da43966a","updated":"2014-12-16 19:22:07.000000000","message":"So the logic here is that an Or Rule should still pass.  We can trip exceptions when policy rules reference portions of the request object that are optional, which is what I was seeing here.  I could probably  get this to a more specific exception for my particular case, but that will rule out future cases where something trips a different exception.\n\nI think I can move this try block to the Or rule and have the same effect, which does make more sense.","commit_id":"ba52915d4c1e37d53836beaa87fc45509684919c"}]}