)]}'
{"climate/api/context.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1b101fe3b02f1aebbc3209400deec4ca4e97c5bb","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":"    target \u003d {\u0027tenant_id\u0027: ctx.tenant_id, \u0027user_id\u0027: ctx.user_id}"},{"line_number":41,"context_line":"    if policy.enforce(ctx, \"admin\", target, do_raise\u003dFalse):"},{"line_number":42,"context_line":"        ctx.is_admin \u003d True"},{"line_number":43,"context_line":"    return ctx"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUX%2F%2FrcI%3D","side":"PARENT","line":42,"updated":"2014-02-11 15:09:29.000000000","message":"We need to make sure is_admin matching with \u0027admin\u0027 context, because it\u0027s verified afterwards. Please keep this dependency.","commit_id":"fe9c660f541bbaa8e3b0b8cb76feaf8c21278d05"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d3a3531bcc8dbcfc60e90e2dda476d5d96d1cf18","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":"    target \u003d {\u0027tenant_id\u0027: ctx.tenant_id, \u0027user_id\u0027: ctx.user_id}"},{"line_number":41,"context_line":"    if policy.enforce(ctx, \"admin\", target, do_raise\u003dFalse):"},{"line_number":42,"context_line":"        ctx.is_admin \u003d True"},{"line_number":43,"context_line":"    return ctx"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUX%2F%2Fp10%3D","side":"PARENT","line":42,"in_reply_to":"AAAAUX%2F%2FqpM%3D","updated":"2014-02-11 18:18:03.000000000","message":"Ok. So that logic should be fixed first. I\u0027ll propose another change shortly.","commit_id":"fe9c660f541bbaa8e3b0b8cb76feaf8c21278d05"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"98b4c8eadb5f6f0371bc30ce3f2ec2316ad8ce79","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":"    target \u003d {\u0027tenant_id\u0027: ctx.tenant_id, \u0027user_id\u0027: ctx.user_id}"},{"line_number":41,"context_line":"    if policy.enforce(ctx, \"admin\", target, do_raise\u003dFalse):"},{"line_number":42,"context_line":"        ctx.is_admin \u003d True"},{"line_number":43,"context_line":"    return ctx"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUX%2F%2FqpM%3D","side":"PARENT","line":42,"in_reply_to":"AAAAUX%2F%2Fqsw%3D","updated":"2014-02-11 16:36:47.000000000","message":"Nope, you missed the call here :\nhttps://github.com/stackforge/climate/blob/fe9c660f541bbaa8e3b0b8cb76feaf8c21278d05/climate/db/sqlalchemy/api.py#L65\n\nwhich itself is looking at the is_admin flag, which is set at context creation time. If you remove the set at this place, is_admin will still be set to False and consequently the DB request will be filtered while it shouldn\u0027t.","commit_id":"fe9c660f541bbaa8e3b0b8cb76feaf8c21278d05"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"199727aed82422d8f090061d11e32ec61aed0578","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":"    target \u003d {\u0027tenant_id\u0027: ctx.tenant_id, \u0027user_id\u0027: ctx.user_id}"},{"line_number":41,"context_line":"    if policy.enforce(ctx, \"admin\", target, do_raise\u003dFalse):"},{"line_number":42,"context_line":"        ctx.is_admin \u003d True"},{"line_number":43,"context_line":"    return ctx"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUX%2F%2Fqsw%3D","side":"PARENT","line":42,"in_reply_to":"AAAAUX%2F%2FrcI%3D","updated":"2014-02-11 16:29:22.000000000","message":"I din\u0027t find any references to \u0027is_admin\u0027 in the code. All \u0027climate:oshosts:*\u0027 rules refer to \u0027admin_api\u0027 rule that verifies \u0027admin\u0027 rule by itself. No need to verify \u0027admin\u0027 rule separately.","commit_id":"fe9c660f541bbaa8e3b0b8cb76feaf8c21278d05"},{"author":{"_account_id":3012,"name":"Dina Belova","email":"dbelova@mirantis.com","username":"dbelova"},"change_message_id":"e700281d6a8eff38940eec6a8621acb1efedafdb","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":"    target \u003d {\u0027tenant_id\u0027: ctx.tenant_id, \u0027user_id\u0027: ctx.user_id}"},{"line_number":41,"context_line":"    if policy.enforce(ctx, \"admin\", target, do_raise\u003dFalse):"},{"line_number":42,"context_line":"        ctx.is_admin \u003d True"},{"line_number":43,"context_line":"    return ctx"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUX%2F%2FrZA%3D","side":"PARENT","line":42,"in_reply_to":"AAAAUX%2F%2FrcI%3D","updated":"2014-02-11 15:16:08.000000000","message":"if so, there are three variants:\n\n1) add is_admin to values and create ctx with ...is_admin\u003d\u0027admin\u0027 in roles\n2) use roles in place where is_admin is used\n3) keep as is - but as I said before, that seems useless because if there is request to Climate and there is \u0027admin\u0027 in roles, is_admin will be True. If there is no \u0027admin\u0027 in roles here - there is no chance is_admin will be true\n\nPersonally I prefer 2","commit_id":"fe9c660f541bbaa8e3b0b8cb76feaf8c21278d05"}]}