)]}'
{"etc/policy.json":[{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"947d1ecbbf2862bd049ccbd4997226e019ae9c57","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    \"event_triggers:delete\": \"rule:admin_or_owner\","},{"line_number":61,"context_line":"    \"event_triggers:get\": \"rule:admin_or_owner\","},{"line_number":62,"context_line":"    \"event_triggers:list\": \"rule:admin_or_owner\","},{"line_number":63,"context_line":"    \"event_triggers:update\": \"rule:admin_or_owner\","},{"line_number":64,"context_line":"}"}],"source_content_type":"application/json","patch_set":18,"id":"dada55a8_425ef412","line":63,"updated":"2016-07-25 16:30:48.000000000","message":"I believe that it is unsafe for non-admins to have access to this API. At least as specified in http://specs.openstack.org/openstack/mistral-specs/specs/newton/approved/event-notification-trigger.html (I haven\u0027t read the whole implementation) it violates the security invariants of the oslo.messaging notification bus, which all projects assume is not accessible by end-users. See my comments on the spec at https://review.openstack.org/#/c/308664/\n\nAt the *very minimum* the default policy must be locked down to rule:admin_only.\n\nHowever, the current definition of admin_only is also inadequate, due to bug 968696. A definition we use in Heat is:\n\n  \"role:admin and auth_token_info.token.is_admin_project:True\"\n\nto check that the user has the admin role on the admin project.","commit_id":"c1ce50b8b5b19dc2985cdfe4ca696fe71dae6c93"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"04633ddd182c41805676f6b5e63bdcbaa9f78a1a","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    \"event_triggers:delete\": \"rule:admin_or_owner\","},{"line_number":61,"context_line":"    \"event_triggers:get\": \"rule:admin_or_owner\","},{"line_number":62,"context_line":"    \"event_triggers:list\": \"rule:admin_or_owner\","},{"line_number":63,"context_line":"    \"event_triggers:update\": \"rule:admin_or_owner\","},{"line_number":64,"context_line":"}"}],"source_content_type":"application/json","patch_set":18,"id":"bacf61ea_6b54cb20","line":63,"in_reply_to":"bacf61ea_3a3a5437","updated":"2016-07-28 20:18:30.000000000","message":"\u003e It\u0027s up to the deployer to decide how to modify the file to accommodate their own use case.\n\nAgreed, and I hope there will be an appropriate degree of warning that their cloud will no longer be secure for multiple tenants (or, indeed, multiple users) if they change the default.\n\n\u003e BTW, I believe the notification is really useful to end users of cloud, just like what AWS did to allow S3 notification trigger Lambda function for developers.\n\nI couldn\u0027t agree more, I think it\u0027s *incredibly* useful - possibly *the* most important missing feature in OpenStack - and that\u0027s why I think it\u0027s unfortunate that no end users will ever get to use it because it\u0027s insecure by design.","commit_id":"c1ce50b8b5b19dc2985cdfe4ca696fe71dae6c93"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"1ec7f228975e992653c5c4999f29a2dc2a2caef2","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    \"event_triggers:delete\": \"rule:admin_or_owner\","},{"line_number":61,"context_line":"    \"event_triggers:get\": \"rule:admin_or_owner\","},{"line_number":62,"context_line":"    \"event_triggers:list\": \"rule:admin_or_owner\","},{"line_number":63,"context_line":"    \"event_triggers:update\": \"rule:admin_or_owner\","},{"line_number":64,"context_line":"}"}],"source_content_type":"application/json","patch_set":18,"id":"bacf61ea_3a3a5437","line":63,"in_reply_to":"dada55a8_425ef412","updated":"2016-07-27 23:57:49.000000000","message":"Hi, Zane, thanks for the comments. I am fine with the change in policy.json to only allow admin role in admin project to use the event trigger function. It\u0027s up to the deployer to decide how to modify the file to accommodate their own use case.\n\nBTW, I believe the notification is really useful to end users of cloud, just like what AWS did to allow S3 notification trigger Lambda function for developers.","commit_id":"c1ce50b8b5b19dc2985cdfe4ca696fe71dae6c93"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"bf40d9579fa304466bfd70975f796220f1cf016e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"{"},{"line_number":2,"context_line":"    \"admin_only\": \"is_admin:True\","},{"line_number":3,"context_line":"    \"admin_or_owner\":  \"is_admin:True or project_id:%(project_id)s\","},{"line_number":4,"context_line":"    \"default\": \"rule:admin_or_owner\","},{"line_number":5,"context_line":""}],"source_content_type":"application/json","patch_set":19,"id":"bacf61ea_d60c7a3d","line":2,"updated":"2016-07-28 20:18:46.000000000","message":"As I mentioned in the previous patchset, this rule is insufficient because of bug 968696. A better definition would be:\n\n    \"admin_only\": \"role:admin and auth_token_info.token.is_admin_project:True\",","commit_id":"60fb14926ed86b4be294081ee1e17ecf86aa702b"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"cd9e2593a95178caa6495dfa88807250d01a3515","unresolved":false,"context_lines":[{"line_number":1,"context_line":"{"},{"line_number":2,"context_line":"    \"admin_only\": \"is_admin:True\","},{"line_number":3,"context_line":"    \"admin_or_owner\":  \"is_admin:True or project_id:%(project_id)s\","},{"line_number":4,"context_line":"    \"default\": \"rule:admin_or_owner\","},{"line_number":5,"context_line":""}],"source_content_type":"application/json","patch_set":19,"id":"bacf61ea_22bd46d8","line":2,"in_reply_to":"bacf61ea_9d9133f4","updated":"2016-08-01 21:59:15.000000000","message":"That doesn\u0027t matter, because the oslo.messaging notifications contain data that is private to the operator even if it is about a resource owned by the user, and this features provides a way for the user to extract that private data.\n\nMany clouds are deployed in such a way that the user has the \"admin\" role in their own tenant due to bug 968696; therefore they will not be protected by this rule. The rule I suggested ensures that the user has the \"admin\" role on the \"admin\" tenant, which means they really are an operator admin. If that doesn\u0027t work for your design then you need to find some other way to work around bug 968696 and ensure that only the operator has access to private data.","commit_id":"60fb14926ed86b4be294081ee1e17ecf86aa702b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"79e969a0cd1ec9de664daef533302f12735acc08","unresolved":false,"context_lines":[{"line_number":1,"context_line":"{"},{"line_number":2,"context_line":"    \"admin_only\": \"is_admin:True\","},{"line_number":3,"context_line":"    \"admin_or_owner\":  \"is_admin:True or project_id:%(project_id)s\","},{"line_number":4,"context_line":"    \"default\": \"rule:admin_or_owner\","},{"line_number":5,"context_line":""}],"source_content_type":"application/json","patch_set":19,"id":"bacf61ea_9d9133f4","line":2,"in_reply_to":"bacf61ea_d60c7a3d","updated":"2016-07-31 02:13:30.000000000","message":"Actually, according to the design, whichever role the user has, he can only get notifications relevant to his own resources. Please see https://github.com/openstack/mistral/blob/master/mistral/services/event_engine.py#L185.","commit_id":"60fb14926ed86b4be294081ee1e17ecf86aa702b"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"14c9006507231453ca5cfcaeef80a77ae7140450","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    \"event_triggers:delete\": \"rule:admin_or_owner\","},{"line_number":61,"context_line":"    \"event_triggers:get\": \"rule:admin_or_owner\","},{"line_number":62,"context_line":"    \"event_triggers:list\": \"rule:admin_or_owner\","},{"line_number":63,"context_line":"    \"event_triggers:update\": \"rule:admin_or_owner\","},{"line_number":64,"context_line":"}"}],"source_content_type":"application/json","patch_set":20,"id":"bacf61ea_a242d6ab","line":63,"updated":"2016-08-01 22:06:12.000000000","message":"Please see comments on previous patchset.\n\nhttps://review.openstack.org/#/c/320509/19/etc/policy.json\n\nThis breaks the contract with *every* OpenStack service that oslo.messaging notifications will never be handed over to the user.","commit_id":"a580c0d31340f951d92ba2d25d5183ef12a4e6c3"}],"mistral/api/controllers/v2/event_trigger.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"10754511ac5a18f20eadccc6e127493a27bc97da","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        LOG.info(\u0027Create event trigger: %s\u0027, values)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        db_model \u003d triggers.create_event_trigger("},{"line_number":67,"context_line":"            values[\u0027name\u0027],"},{"line_number":68,"context_line":"            values.get(\u0027exchange\u0027),"},{"line_number":69,"context_line":"            values.get(\u0027topic\u0027),"},{"line_number":70,"context_line":"            values.get(\u0027event\u0027),"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_cd497e90","line":67,"range":{"start_line":67,"start_character":12,"end_line":67,"end_character":26},"updated":"2016-07-25 14:39:56.000000000","message":"\u0027name\u0027 is not in CREATE_MANDATORY. How do we make sure that there is such key in \u0027values\u0027?","commit_id":"c1ce50b8b5b19dc2985cdfe4ca696fe71dae6c93"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"1ec7f228975e992653c5c4999f29a2dc2a2caef2","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        LOG.info(\u0027Create event trigger: %s\u0027, values)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        db_model \u003d triggers.create_event_trigger("},{"line_number":67,"context_line":"            values[\u0027name\u0027],"},{"line_number":68,"context_line":"            values.get(\u0027exchange\u0027),"},{"line_number":69,"context_line":"            values.get(\u0027topic\u0027),"},{"line_number":70,"context_line":"            values.get(\u0027event\u0027),"}],"source_content_type":"text/x-python","patch_set":18,"id":"bacf61ea_7786a5ba","line":67,"range":{"start_line":67,"start_character":12,"end_line":67,"end_character":26},"in_reply_to":"dada55a8_cd497e90","updated":"2016-07-27 23:57:49.000000000","message":"Nice catch, Dawid!","commit_id":"c1ce50b8b5b19dc2985cdfe4ca696fe71dae6c93"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"86318f839f3796cb15556a3f0e193788d018c51c","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        db_model \u003d db_api.get_event_trigger(id)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        return resources.EventTrigger.from_dict(db_model.to_dict())"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    @rest_utils.wrap_wsme_controller_exception"},{"line_number":49,"context_line":"    @wsme_pecan.wsexpose(resources.EventTrigger, body\u003dresources.EventTrigger,"}],"source_content_type":"text/x-python","patch_set":19,"id":"bacf61ea_21ddfd14","line":46,"range":{"start_line":46,"start_character":0,"end_line":46,"end_character":67},"updated":"2016-07-28 08:59:46.000000000","message":"Soft: We are converting the db_model to dictionary just to create EventTrigger based on it. Would it be possible to use method like \u0027from_db_model\u0027, so there is no need for extra calls?","commit_id":"60fb14926ed86b4be294081ee1e17ecf86aa702b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"79e969a0cd1ec9de664daef533302f12735acc08","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        db_model \u003d db_api.get_event_trigger(id)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        return resources.EventTrigger.from_dict(db_model.to_dict())"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    @rest_utils.wrap_wsme_controller_exception"},{"line_number":49,"context_line":"    @wsme_pecan.wsexpose(resources.EventTrigger, body\u003dresources.EventTrigger,"}],"source_content_type":"text/x-python","patch_set":19,"id":"bacf61ea_fd5caf7e","line":46,"range":{"start_line":46,"start_character":0,"end_line":46,"end_character":67},"in_reply_to":"bacf61ea_21ddfd14","updated":"2016-07-31 02:13:30.000000000","message":"hmm...I don\u0027t think mix operation of api and db layer is good idea. We can improve it in future if you have a good idea about that.","commit_id":"60fb14926ed86b4be294081ee1e17ecf86aa702b"}],"mistral/engine/rpc_backend/rpc.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"10754511ac5a18f20eadccc6e127493a27bc97da","unresolved":false,"context_lines":[{"line_number":609,"context_line":"            auth_ctx.JsonPayloadSerializer()"},{"line_number":610,"context_line":"        )"},{"line_number":611,"context_line":""},{"line_number":612,"context_line":"        self._client \u003d messaging.RPCClient("},{"line_number":613,"context_line":"            transport,"},{"line_number":614,"context_line":"            messaging.Target(topic\u003dcfg.CONF.event_engine.topic),"},{"line_number":615,"context_line":"            serializer\u003dserializer"},{"line_number":616,"context_line":"        )"},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def create_event_trigger(self, trigger, events):"},{"line_number":619,"context_line":"        return self._client.call("}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_4d4aadc6","line":616,"range":{"start_line":612,"start_character":0,"end_line":616,"end_character":9},"updated":"2016-07-25 14:39:56.000000000","message":"It should use \u0027get_rpc_client_driver\u0027 rather then calling oslo directly. Please take a look on EngineClient or ExecutorClient in this file.","commit_id":"c1ce50b8b5b19dc2985cdfe4ca696fe71dae6c93"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"1ec7f228975e992653c5c4999f29a2dc2a2caef2","unresolved":false,"context_lines":[{"line_number":609,"context_line":"            auth_ctx.JsonPayloadSerializer()"},{"line_number":610,"context_line":"        )"},{"line_number":611,"context_line":""},{"line_number":612,"context_line":"        self._client \u003d messaging.RPCClient("},{"line_number":613,"context_line":"            transport,"},{"line_number":614,"context_line":"            messaging.Target(topic\u003dcfg.CONF.event_engine.topic),"},{"line_number":615,"context_line":"            serializer\u003dserializer"},{"line_number":616,"context_line":"        )"},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def create_event_trigger(self, trigger, events):"},{"line_number":619,"context_line":"        return self._client.call("}],"source_content_type":"text/x-python","patch_set":18,"id":"bacf61ea_9a20a04b","line":616,"range":{"start_line":612,"start_character":0,"end_line":616,"end_character":9},"in_reply_to":"dada55a8_4d4aadc6","updated":"2016-07-27 23:57:49.000000000","message":"Done","commit_id":"c1ce50b8b5b19dc2985cdfe4ca696fe71dae6c93"}],"mistral/services/triggers.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"86318f839f3796cb15556a3f0e193788d018c51c","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                                          topic\u003dtopic)"},{"line_number":145,"context_line":"        events \u003d [t.event for t in trigs]"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        # NOTE(kong): Send RPC message within the db transaction, rollback if"},{"line_number":148,"context_line":"        # any error occurs."},{"line_number":149,"context_line":"        rpc.get_event_engine_client().create_event_trigger("},{"line_number":150,"context_line":"            trig.to_dict(),"},{"line_number":151,"context_line":"            events"}],"source_content_type":"text/x-python","patch_set":19,"id":"bacf61ea_3f2b54ed","line":148,"range":{"start_line":147,"start_character":0,"end_line":148,"end_character":27},"updated":"2016-07-28 08:59:46.000000000","message":"Not sure if this is the best approach. It calls sync_call, which make take up to 60 seconds with default config. Maybe the event_engine should create entries in the DB?","commit_id":"60fb14926ed86b4be294081ee1e17ecf86aa702b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"79e969a0cd1ec9de664daef533302f12735acc08","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                                          topic\u003dtopic)"},{"line_number":145,"context_line":"        events \u003d [t.event for t in trigs]"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        # NOTE(kong): Send RPC message within the db transaction, rollback if"},{"line_number":148,"context_line":"        # any error occurs."},{"line_number":149,"context_line":"        rpc.get_event_engine_client().create_event_trigger("},{"line_number":150,"context_line":"            trig.to_dict(),"},{"line_number":151,"context_line":"            events"}],"source_content_type":"text/x-python","patch_set":19,"id":"bacf61ea_ddef2b85","line":148,"range":{"start_line":147,"start_character":0,"end_line":148,"end_character":27},"in_reply_to":"bacf61ea_3f2b54ed","updated":"2016-07-31 02:13:30.000000000","message":"what\u0027s your meaning by \u0027Maybe the event_engine should create entries in the DB\u0027?","commit_id":"60fb14926ed86b4be294081ee1e17ecf86aa702b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"ee8864d88fef24918b232807b05cbdf494e33aa2","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        eng_utils.validate_input("},{"line_number":124,"context_line":"            wf_def,"},{"line_number":125,"context_line":"            workflow_input or {},"},{"line_number":126,"context_line":"            parser.get_workflow_spec(wf_def.spec)"},{"line_number":127,"context_line":"        )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        values \u003d {"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a89bdaa_a2292ee8","line":126,"range":{"start_line":126,"start_character":19,"end_line":126,"end_character":36},"updated":"2016-09-08 07:57:17.000000000","message":"Please use method \u0027get_workflow_spec_by_definition_id\u0027 that was added a month ago. This method uses specification cache to avoid unnecessary parsing.","commit_id":"2fb766308ce879065ab197386c66563889b6c79e"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"5e26d45728b368d1e82cd028ceab9429eb4d8b49","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        eng_utils.validate_input("},{"line_number":124,"context_line":"            wf_def,"},{"line_number":125,"context_line":"            workflow_input or {},"},{"line_number":126,"context_line":"            parser.get_workflow_spec(wf_def.spec)"},{"line_number":127,"context_line":"        )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        values \u003d {"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a89bdaa_47c227ea","line":126,"range":{"start_line":126,"start_character":19,"end_line":126,"end_character":36},"in_reply_to":"9a89bdaa_a2292ee8","updated":"2016-09-08 11:24:44.000000000","message":"Done","commit_id":"2fb766308ce879065ab197386c66563889b6c79e"}]}