)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":13,"context_line":"policy format, to ensure backward compatibly."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Note that deployers need to specify a policy file which is created by"},{"line_number":16,"context_line":"themselves. There is a default policy file which is stored in"},{"line_number":17,"context_line":"/etc/swift/policy.json. The policy file can be a base of their cluster."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"How does it work:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1aca2d91_3d7daae7","line":16,"updated":"2015-05-07 14:21:31.000000000","message":"Should this file not be called /etc/swift/policy.json-sample? i.e. \"-sample\" to the name.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":13,"context_line":"policy format, to ensure backward compatibly."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Note that deployers need to specify a policy file which is created by"},{"line_number":16,"context_line":"themselves. There is a default policy file which is stored in"},{"line_number":17,"context_line":"/etc/swift/policy.json. The policy file can be a base of their cluster."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"How does it work:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1aca2d91_b3455471","line":16,"in_reply_to":"1aca2d91_3d7daae7","updated":"2015-05-08 06:16:02.000000000","message":"my current thought for this is \nthis is not a sample file, keystoneauth always works with this file.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"d5291295876b8a61436db0f39f8d7e67b933dfcc","unresolved":false,"context_lines":[{"line_number":13,"context_line":"policy format, to ensure backward compatibly."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Note that deployers need to specify a policy file which is created by"},{"line_number":16,"context_line":"themselves. There is a default policy file which is stored in"},{"line_number":17,"context_line":"/etc/swift/policy.json. The policy file can be a base of their cluster."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"How does it work:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1aca2d91_4d2894e2","line":16,"in_reply_to":"1aca2d91_b3455471","updated":"2015-05-08 11:09:40.000000000","message":"The intent of a policy.json file is that it can be changed by the user. For example, in a triplo setup, the operator_roles rule would be extended to add _member_ to the role list. Hence, this file is a *sample* -- not a fixed file.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"How does it work:"},{"line_number":20,"context_line":"* keystoneauth works with a policy file which is specified in"},{"line_number":21,"context_line":"  proxy-server.conf as policy_file."},{"line_number":22,"context_line":"* If \u0027reseller_admin_role\u0027, \u0027operator_roles\u0027 and \u0027service_roles\u0027 are"},{"line_number":23,"context_line":"  specified in proxy-server.conf, keystoneauth updates the policy"},{"line_number":24,"context_line":"  with the information and works with it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1aca2d91_5d618638","line":21,"updated":"2015-05-07 14:21:31.000000000","message":"As mentioned in detailed comments elsewhere, I think this is confusing. If no policy.json file exists, then use these options. Otherwise, the policy.json should be definitive.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"How does it work:"},{"line_number":20,"context_line":"* keystoneauth works with a policy file which is specified in"},{"line_number":21,"context_line":"  proxy-server.conf as policy_file."},{"line_number":22,"context_line":"* If \u0027reseller_admin_role\u0027, \u0027operator_roles\u0027 and \u0027service_roles\u0027 are"},{"line_number":23,"context_line":"  specified in proxy-server.conf, keystoneauth updates the policy"},{"line_number":24,"context_line":"  with the information and works with it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"1aca2d91_532280ba","line":21,"in_reply_to":"1aca2d91_5d618638","updated":"2015-05-08 06:16:02.000000000","message":"you are right. I will change this sentence.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"}],"etc/default_policy.json-sample":[{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"976b7dd4f453d9337d6fb8aaa16ae585cb4a3801","unresolved":false,"context_lines":[{"line_number":18,"context_line":"    \u0027allowed_user\u0027: (\u0027(rule:authenticated and \u0027"},{"line_number":19,"context_line":"                     \u0027rule:allowed_for_authenticated) or \u0027"},{"line_number":20,"context_line":"                     \u0027rule:allowed_for_anonymous\u0027),"},{"line_number":21,"context_line":"    \u0027get_account\u0027: \u0027rule:allowed_user\u0027,"},{"line_number":22,"context_line":"    \u0027post_account\u0027: \u0027rule:allowed_user\u0027,"},{"line_number":23,"context_line":"    \u0027head_account\u0027: \u0027rule:allowed_user\u0027,"},{"line_number":24,"context_line":"    \u0027delete_account\u0027: \u0027rule:reseller_admin_roles\u0027,"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"fa81d914_daee3122","line":21,"updated":"2015-01-28 12:21:26.000000000","message":"I think you are missing:\n\n    \u0027put_account\u0027: \u0027rule:allowed_for_authenticated\u0027,\n\nI\u0027m not sure the post_account should be rule: allowed_user, since that allows an anonymous user (in theory; since there is no account ACLs I guess an anonymous user won\u0027t get access anyway -- or do you anticipate account ACLs?","commit_id":"981d759e453e124a78e0c5803819df6ddceb16b7"}],"etc/policy.json":[{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":1,"context_line":"{"},{"line_number":2,"context_line":"    \"anonymous\": \"identity:None\","},{"line_number":3,"context_line":"    \"authenticated\": \"not rule:anonymous\","},{"line_number":4,"context_line":"    \"reseller_admin_roles\": \"role:ResellerAdmin\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_a492d09d","line":1,"updated":"2015-05-07 14:21:31.000000000","message":"I must say, I find this fiendishly complicated. I know json does not allow comments, but you need to devise some way (documentation) to tell us which bits of the file you expect users to change -- and which bits they must *not* change. For example, swift_owner is defined here. However, it is not apparently used in any subsequent rule/test. So can a user delete line 11? The answer is no, since it\u0027s used by line 759 of keystoneauth.py.\n\nnit: in many places you have used names such as \"reseller_admin_roles\". However, \"reseller_admin_roles\" is not a list of roles --- it\u0027s a boolean. It would be easier to understand if the name was \"is_reseller_admin\" or \"has_reseller_admin_role\".\n\nIt\u0027s interesting that you managed to get multiple reseller prefixes into one policy.json file (I had expected that you might need a separate policy file for each prefix). One file is better, so kudos for figuring this out. However, when you add a prefix, it seems you need to add and change too many places. For example, if prefix FOO is added, you woul add twp lines after line 8 and then also change line 9 and line 10. Why not simply put the required roles in line 9 and 10? It makes the lines a bit long, but I think it\u0027s clearer: For example, I would delete lines 5-8 and change lines 9-10 to:\n\n    \"operator_roles\": \"(reseller_prefix:AUTH and (role:admin or role:swiftoperator)) or (reseller_prefix:SERVICE (role:admin or role:swiftoperator))\",\n    \"service_roles\": \"(reseller_prefix:AUTH and \"@\") or (reseller_prefix:SERVICE and (srole:service))\",","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":1,"context_line":"{"},{"line_number":2,"context_line":"    \"anonymous\": \"identity:None\","},{"line_number":3,"context_line":"    \"authenticated\": \"not rule:anonymous\","},{"line_number":4,"context_line":"    \"reseller_admin_roles\": \"role:ResellerAdmin\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_198d477b","line":1,"in_reply_to":"1aca2d91_a492d09d","updated":"2015-05-08 06:16:02.000000000","message":"As for the complicated json file, yeah it is really complicated. I will split the policy file to an  operator part and logic part and hide the logic part so interface can be more simpler.\n\nAs for the nit, what do you think about splitting it to definition part (ie. reseller_admin_roles: role:XXX) and boolean part.\n\nAs for the one policy file, it\u0027s great. I will change it.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    \"SERVICE_operator_roles\": \"role:admin or role:swiftoperator\","},{"line_number":8,"context_line":"    \"SERVICE_service_roles\": \"srole:service\","},{"line_number":9,"context_line":"    \"operator_roles\": \"(reseller_prefix:AUTH and rule:AUTH_operator_roles) or (reseller_prefix:SERVICE and rule:SERVICE_operator_roles)\","},{"line_number":10,"context_line":"    \"service_roles\": \"(reseller_prefix:AUTH and rule:AUTH_service_roles) or (reseller_prefix:SERVICE and rule:SERVICE_service_roles)\","},{"line_number":11,"context_line":"    \"swift_owner\": \"rule:reseller_admin_roles or (rule:operator_roles and rule:service_roles) or is_admin:True\","},{"line_number":12,"context_line":"    \"reseller_request\": \"rule:reseller_admin_roles\","},{"line_number":13,"context_line":"    \"same_tenant\": \"tenant_id:%(account)s\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_047d046e","line":10,"updated":"2015-05-07 14:21:31.000000000","message":"Given that is_admin is \"depreciated\", I don\u0027t see a need to support this in a policy.json file. In anycase, I don\u0027t see how this can work in a Keystone context -- the account name is something like AUTH_\u003cprojectid\u003e -- that cannot match a username","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    \"SERVICE_operator_roles\": \"role:admin or role:swiftoperator\","},{"line_number":8,"context_line":"    \"SERVICE_service_roles\": \"srole:service\","},{"line_number":9,"context_line":"    \"operator_roles\": \"(reseller_prefix:AUTH and rule:AUTH_operator_roles) or (reseller_prefix:SERVICE and rule:SERVICE_operator_roles)\","},{"line_number":10,"context_line":"    \"service_roles\": \"(reseller_prefix:AUTH and rule:AUTH_service_roles) or (reseller_prefix:SERVICE and rule:SERVICE_service_roles)\","},{"line_number":11,"context_line":"    \"swift_owner\": \"rule:reseller_admin_roles or (rule:operator_roles and rule:service_roles) or is_admin:True\","},{"line_number":12,"context_line":"    \"reseller_request\": \"rule:reseller_admin_roles\","},{"line_number":13,"context_line":"    \"same_tenant\": \"tenant_id:%(account)s\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_a886fe62","line":10,"in_reply_to":"1aca2d91_047d046e","updated":"2015-05-08 06:16:02.000000000","message":"I thought that is_admin was necessary from backward compatibility point of view. Is there any problem to drop it with this patch?\n\nis_admin will be true with the following condition in keytoneauth.py:\n602         is_admin \u003d self._is_admin and (tenant_name \u003d\u003d user_name)","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"d5291295876b8a61436db0f39f8d7e67b933dfcc","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    \"SERVICE_operator_roles\": \"role:admin or role:swiftoperator\","},{"line_number":8,"context_line":"    \"SERVICE_service_roles\": \"srole:service\","},{"line_number":9,"context_line":"    \"operator_roles\": \"(reseller_prefix:AUTH and rule:AUTH_operator_roles) or (reseller_prefix:SERVICE and rule:SERVICE_operator_roles)\","},{"line_number":10,"context_line":"    \"service_roles\": \"(reseller_prefix:AUTH and rule:AUTH_service_roles) or (reseller_prefix:SERVICE and rule:SERVICE_service_roles)\","},{"line_number":11,"context_line":"    \"swift_owner\": \"rule:reseller_admin_roles or (rule:operator_roles and rule:service_roles) or is_admin:True\","},{"line_number":12,"context_line":"    \"reseller_request\": \"rule:reseller_admin_roles\","},{"line_number":13,"context_line":"    \"same_tenant\": \"tenant_id:%(account)s\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_0d5a9c4f","line":10,"in_reply_to":"1aca2d91_a886fe62","updated":"2015-05-08 11:09:40.000000000","message":"When this patch lands and an operator installs this software on their system, there are two cases:\n\n1/ Software installed, but the operator does not create /etc/policy.json\n\n2/ Software installed and operator also creates /etc/policy.json\n\nFor case 1, you must retain backward compatibility -- hence _get_policy_json_file() must include the is_admin rule\n\nFor case 2, we can decide whether to support is_admin or not. This is a good time to remove the is_admin feature. If someone *needs* the feature, they simply go to case 1 -- don\u0027t create a policy.json file. I suggest, that  we don\u0027t support it for case 2.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":10,"context_line":"    \"service_roles\": \"(reseller_prefix:AUTH and rule:AUTH_service_roles) or (reseller_prefix:SERVICE and rule:SERVICE_service_roles)\","},{"line_number":11,"context_line":"    \"swift_owner\": \"rule:reseller_admin_roles or (rule:operator_roles and rule:service_roles) or is_admin:True\","},{"line_number":12,"context_line":"    \"reseller_request\": \"rule:reseller_admin_roles\","},{"line_number":13,"context_line":"    \"same_tenant\": \"tenant_id:%(account)s\","},{"line_number":14,"context_line":"    \"allowed_for_authenticated\": \"rule:reseller_admin_roles or acl:check_public or acl:check_cross_tenant or (rule:same_tenant and ((rule:operator_roles and rule:service_roles) or is_admin:True or acl:check_roles))\","},{"line_number":15,"context_line":"    \"allowed_for_anonymous\": \"is_authoritative:True and acl:check_public\","},{"line_number":16,"context_line":"    \"allowed_user\": \"(rule:authenticated and rule:allowed_for_authenticated) or rule:allowed_for_anonymous\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_9d915e9c","line":13,"updated":"2015-05-07 14:21:31.000000000","message":"In allowed_for_authenticated, why not use swift_owner instead of checking/repating various roles?\n\nHowever, when I examine the differences between the swift_owner and allowed_for_authenticated, I can\u0027t see why swift_owner is missing rule:same_tenant -- is that a mistake or something subtle?\n\nThe Swift community is familiar with the swift_onwer concept, so it seems strange for it not to feature more. Once you are swift_owner you can do everything except delete_account, so you would expect the core rule to be something like:\n\n      go_ahead \u003d rule:swift_owner or ( something-from-an-acl-allows-it )\n\nThis would make the rules more readable for Swift developers.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":10,"context_line":"    \"service_roles\": \"(reseller_prefix:AUTH and rule:AUTH_service_roles) or (reseller_prefix:SERVICE and rule:SERVICE_service_roles)\","},{"line_number":11,"context_line":"    \"swift_owner\": \"rule:reseller_admin_roles or (rule:operator_roles and rule:service_roles) or is_admin:True\","},{"line_number":12,"context_line":"    \"reseller_request\": \"rule:reseller_admin_roles\","},{"line_number":13,"context_line":"    \"same_tenant\": \"tenant_id:%(account)s\","},{"line_number":14,"context_line":"    \"allowed_for_authenticated\": \"rule:reseller_admin_roles or acl:check_public or acl:check_cross_tenant or (rule:same_tenant and ((rule:operator_roles and rule:service_roles) or is_admin:True or acl:check_roles))\","},{"line_number":15,"context_line":"    \"allowed_for_anonymous\": \"is_authoritative:True and acl:check_public\","},{"line_number":16,"context_line":"    \"allowed_user\": \"(rule:authenticated and rule:allowed_for_authenticated) or rule:allowed_for_anonymous\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_e89bd670","line":13,"in_reply_to":"1aca2d91_9d915e9c","updated":"2015-05-08 06:16:02.000000000","message":"\u003e In allowed_for_authenticated, why not use swift_owner instead of checking/repating various roles?\n\nSo far I take care about the order of rules but I will check the necessity of it. \n\n\u003e However, when I examine the differences between the swift_owner and allowed_for_authenticated, I can\u0027t see why swift_owner is missing rule:same_tenant -- is that a mistake or something subtle?\n\nsame_tenant of allowed_for_authenticated  is matched to the following logic in original keystoneauth.py.\n\n465         if not self._account_matches_tenant(account, tenant_id): \n\nyeah, it\u0027s a bug. same_tenant should be used for the following logic too.\n\n487             req.environ[\u0027swift_owner\u0027] \u003d True\n489             req.environ[\u0027swift_owner\u0027] \u003d True\n\nAs for the swift_owner concept, I will check the necessity which is described above (if the result is \"there is no necessary, I will replace it)","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":18,"context_line":"    \"delete_account\": \"rule:reseller_admin_roles\","},{"line_number":19,"context_line":"    \"delete_container\": \"rule:allowed_user\","},{"line_number":20,"context_line":"    \"delete_object\": \"rule:allowed_user\","},{"line_number":21,"context_line":"    \"get_account\": \"rule:allowed_user\","},{"line_number":22,"context_line":"    \"get_container\": \"rule:allowed_user\","},{"line_number":23,"context_line":"    \"get_object\": \"rule:allowed_user\","},{"line_number":24,"context_line":"    \"head_account\": \"rule:allowed_user\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_7a7d68b0","line":21,"updated":"2015-05-07 14:21:31.000000000","message":"It seems wrong that get_account and get_object have the *same* rule. One compent of allowed_user is acl:check_roles. I have not managed to decode how acl:check_roles work, but since a referrer of .r:* and .rlistings are different, a single rule/flag seems wrong. A container with .r:* (only) should allow get_object, but not a get_container. Conversely, a container with .rlistings (only) would allow a get_container, but not get_object.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":18,"context_line":"    \"delete_account\": \"rule:reseller_admin_roles\","},{"line_number":19,"context_line":"    \"delete_container\": \"rule:allowed_user\","},{"line_number":20,"context_line":"    \"delete_object\": \"rule:allowed_user\","},{"line_number":21,"context_line":"    \"get_account\": \"rule:allowed_user\","},{"line_number":22,"context_line":"    \"get_container\": \"rule:allowed_user\","},{"line_number":23,"context_line":"    \"get_object\": \"rule:allowed_user\","},{"line_number":24,"context_line":"    \"head_account\": \"rule:allowed_user\","}],"source_content_type":"application/json","patch_set":14,"id":"1aca2d91_087972f6","line":21,"in_reply_to":"1aca2d91_7a7d68b0","updated":"2015-05-08 06:16:02.000000000","message":"I think that keystoneauth doesn\u0027t need to care about difference of aips such as get_account and get_object. keystoneauth only needs to define \"common\" authorization method for the apis.\n\nPlease see authorize_unconfirmed_identity method which is called from acl:check_public. I think the logic is same as original.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    \"options_container\": \"@\","},{"line_number":29,"context_line":"    \"options_object\": \"@\","},{"line_number":30,"context_line":"    \"post_account\": \"rule:allowed_user\","},{"line_number":31,"context_line":"    \"post_container\": \"rule:allowed_user\","},{"line_number":32,"context_line":"    \"post_object\": \"rule:allowed_user\","},{"line_number":33,"context_line":"    \"put_account\": \"rule:allowed_user\","},{"line_number":34,"context_line":"    \"put_container\": \"rule:allowed_user\","}],"source_content_type":"application/json","patch_set":15,"id":"fa32b979_141c6f68","line":31,"updated":"2015-06-19 04:50:30.000000000","message":"I feel like post_container and get_object *have* to have different rules in policy.json\n\nhaving your name in x-container-read-acl allows you get an object - but having your name in x-container-read-acl does *not* allow you to post to a container.\n\nThat sort of decision about what exactly authorizes this action on this target for this identity is *exactly* the kind of thing a policy engine should be making configurable.  The idea here is to make it so you don\u0027t have to write custom middleware every time you have a use-case that\u0027s slightly off the beaten path - or more likely - don\u0027t agree with all of the upstream decisions about what should or should not be a thing users can allow in their swift cluster.\n\nThe use cases I can think of that we should be thinking about solving are:\n\n * disallow public .r:* acls w/o having to re-write middleware\n * disallow referer based acl\u0027s of any kind to some accounts\n * restrict acls to tenant_id:user_id (no legacy acls)\n * restrict acls to roles only\n * restrict acls to w/i same tenant only\n * allow refer based acls to be used for put_object [1]\n\n1. hey, public ftp servers that allow anonymous uploads were a thing - it might make sense in some environments","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"}],"etc/proxy-server.conf-sample":[{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"976b7dd4f453d9337d6fb8aaa16ae585cb4a3801","unresolved":false,"context_lines":[{"line_number":308,"context_line":"# For a new installation, or an installation in which keystone projects may"},{"line_number":309,"context_line":"# move between domains, you should disable backwards compatible name matching"},{"line_number":310,"context_line":"# in ACLs by setting allow_names_in_acls to false:"},{"line_number":311,"context_line":"# allow_names_in_acls \u003d true"},{"line_number":312,"context_line":"# Neither \u0027reseller_admin_role\u0027 nor \u0027operator_roles\u0027 is not specified here"},{"line_number":313,"context_line":"# and \u0027policy_file\u0027 is specided, RBAC with oslo.policy will be enabled."},{"line_number":314,"context_line":"# policy_file \u003d /etc/swift/default_policy.json"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"fa81d914_4fe4410d","line":311,"updated":"2015-01-28 12:21:26.000000000","message":"Since policy_file is now, I think you can be more assertive i.e., if policy_file is present, then reseller_admin_role and operator_roles are *ignored*. Another potential option is to error out if policy_file is specified and the user has forgotten to remove reseller_admin_role and operator_roles","commit_id":"981d759e453e124a78e0c5803819df6ddceb16b7"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":323,"context_line":"# operator_roles \u003d admin, swiftoperator"},{"line_number":324,"context_line":"#"},{"line_number":325,"context_line":"# The reseller admin role has the ability to create and delete accounts"},{"line_number":326,"context_line":"# reseller_admin_role \u003d ResellerAdmin"},{"line_number":327,"context_line":"# "},{"line_number":328,"context_line":"# This allows deployers to use own policy file."},{"line_number":329,"context_line":"# policy_file \u003d /etc/swift/policy.json"}],"source_content_type":"application/octet-stream","patch_set":14,"id":"1aca2d91_fb3bc7e9","line":326,"updated":"2015-05-07 14:21:31.000000000","message":"whitespace","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":323,"context_line":"# operator_roles \u003d admin, swiftoperator"},{"line_number":324,"context_line":"#"},{"line_number":325,"context_line":"# The reseller admin role has the ability to create and delete accounts"},{"line_number":326,"context_line":"# reseller_admin_role \u003d ResellerAdmin"},{"line_number":327,"context_line":"# "},{"line_number":328,"context_line":"# This allows deployers to use own policy file."},{"line_number":329,"context_line":"# policy_file \u003d /etc/swift/policy.json"}],"source_content_type":"application/octet-stream","patch_set":14,"id":"1aca2d91_c8613a3b","line":326,"in_reply_to":"1aca2d91_fb3bc7e9","updated":"2015-05-08 06:16:02.000000000","message":"I will remove it.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"}],"swift/common/middleware/keystoneauth.py":[{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"976b7dd4f453d9337d6fb8aaa16ae585cb4a3801","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        roles \u003d []"},{"line_number":191,"context_line":"        if \u0027HTTP_X_ROLES\u0027 in environ:"},{"line_number":192,"context_line":"            roles \u003d environ[\u0027HTTP_X_ROLES\u0027].split(\u0027,\u0027)"},{"line_number":193,"context_line":"        env_identity \u003d {\u0027user\u0027: (environ.get(\u0027HTTP_X_USER_ID\u0027),"},{"line_number":194,"context_line":"                                 environ.get(\u0027HTTP_X_USER_NAME\u0027)),"},{"line_number":195,"context_line":"                        \u0027tenant\u0027: (environ.get(\u0027HTTP_X_TENANT_ID\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa81d914_efab3587","line":192,"updated":"2015-01-28 12:21:26.000000000","message":"Not sure of the value of renaming \"identity\" to \"env_identity\". It just creates more things to review.","commit_id":"981d759e453e124a78e0c5803819df6ddceb16b7"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":335,"context_line":"            self._policy_file \u003d self.conf_get(\u0027policy_file\u0027)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"            # After this patch is merged, this logic will be removed"},{"line_number":338,"context_line":"            # because it is only for backward compatibility of the gate."},{"line_number":339,"context_line":"            if not os.path.exists(self._policy_file):"},{"line_number":340,"context_line":"                self._policy_file \u003d _get_policy_json_file()"},{"line_number":341,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_7ce7b2e4","line":338,"updated":"2015-05-07 14:21:31.000000000","message":"Backward comparability is needed for more than just the gate. If you remove this code, you force existing deployments to add a policy file to their /etc/swift directory. At least, I think that\u0027s what you are implying.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"d5291295876b8a61436db0f39f8d7e67b933dfcc","unresolved":false,"context_lines":[{"line_number":335,"context_line":"            self._policy_file \u003d self.conf_get(\u0027policy_file\u0027)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"            # After this patch is merged, this logic will be removed"},{"line_number":338,"context_line":"            # because it is only for backward compatibility of the gate."},{"line_number":339,"context_line":"            if not os.path.exists(self._policy_file):"},{"line_number":340,"context_line":"                self._policy_file \u003d _get_policy_json_file()"},{"line_number":341,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_3115b23f","line":338,"in_reply_to":"1aca2d91_48564a5d","updated":"2015-05-08 11:09:40.000000000","message":"Updating devstack does not allow you to remove this logic. There are 100s of systems that have Swift installed on them. If you remove the logic, you force their operators to create an /etc/swift/policy.json file whenever they update the software. We try to avoid these situations. That\u0027s why you have been asked to continue to support operator_roles, etc.\n\nIf you attempt to remove this logic, I predict you will get several -2 from various cores.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":335,"context_line":"            self._policy_file \u003d self.conf_get(\u0027policy_file\u0027)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"            # After this patch is merged, this logic will be removed"},{"line_number":338,"context_line":"            # because it is only for backward compatibility of the gate."},{"line_number":339,"context_line":"            if not os.path.exists(self._policy_file):"},{"line_number":340,"context_line":"                self._policy_file \u003d _get_policy_json_file()"},{"line_number":341,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_48564a5d","line":338,"in_reply_to":"1aca2d91_7ce7b2e4","updated":"2015-05-08 06:16:02.000000000","message":"That\u0027s right. my plan is after this patch landed I will put a fix which deploy policy.json in /etc/swift to devstack then remove this logic.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        self._enforcer.load_rules()"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        rules \u003d {}"},{"line_number":391,"context_line":"        if self._reseller_admin_roles:"},{"line_number":392,"context_line":"            rules[\u0027reseller_admin_roles\u0027] \u003d \u0027 or \u0027.join("},{"line_number":393,"context_line":"                (\u0027role:%s\u0027 % role for role in self._reseller_admin_roles))"},{"line_number":394,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_5bf87b49","line":391,"updated":"2015-05-07 14:21:31.000000000","message":"This comment also applies to operator_roles (line 406-414).\n\nI think this substitution should only happen if os.path.exists(self._policy_file) i.e., if  a policy file exists, then the legacy reseller_admin_roles, service_roles, operator_roles should be ignored. Otherwise you have a situation where any change requires edits to both the policy.json and proxy-server.conf file. For example, if the proxy.-server.conf file has:\n\n    reseller_admin_role\u003dResellerAdmin\n    policy_file \u003d /etc/swift/policy.json\n\nI then edit ONLY the policy.json file to change ResellerAdmin to SuperAdmin. However, on restart, this code replaces this (silently) with ResellerAdmin.\n\nAs an administrator of an Openstack cloud, if I find a policy.json file, I would assume it\u0027s definitive and would be blind to /etc/proxy-server.conf options.\n\nAs a legacy Swift administrator, I would be familiar with the existing options and should be able to continue to use them. If I later decide to create a policy.json file, I would accept that it is definitive and overrides /etc/proxy-server.conf settings.\n\nIn summary, I think the correct logic should be:\n\n- No policy.json file -- use options in /etc/swift/\n\n- Have a policy.json file -- ignore legacy options in /e/tc/swift","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        self._enforcer.load_rules()"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        rules \u003d {}"},{"line_number":391,"context_line":"        if self._reseller_admin_roles:"},{"line_number":392,"context_line":"            rules[\u0027reseller_admin_roles\u0027] \u003d \u0027 or \u0027.join("},{"line_number":393,"context_line":"                (\u0027role:%s\u0027 % role for role in self._reseller_admin_roles))"},{"line_number":394,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_88e90283","line":391,"in_reply_to":"1aca2d91_5bf87b49","updated":"2015-05-08 06:16:02.000000000","message":"I understand. I will implement this like follows.\n\n* policy_file entry in proxy-server.conf - use specified policy file\n* no policy_file entry in proxy-server.conf - used default policy file (policy.json) with the info such as reseller_admin_role and XXXX_operator_role in proxy-server.conf.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        if (req.environ.get(\u0027swift_sync_key\u0027)"},{"line_number":727,"context_line":"                and (req.environ[\u0027swift_sync_key\u0027] \u003d\u003d"},{"line_number":728,"context_line":"                     req.headers.get(\u0027x-container-sync-key\u0027, None))"},{"line_number":729,"context_line":"                and \u0027x-timestamp\u0027 in req.headers):"},{"line_number":730,"context_line":"            # log_msg \u003d \u0027allowing proxy %s for container-sync\u0027"},{"line_number":731,"context_line":"            # self.logger.debug(log_msg, req.remote_addr)"},{"line_number":732,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_5dbd0667","line":729,"updated":"2015-05-07 14:21:31.000000000","message":"Since this is at debug level, why comment out?","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        if (req.environ.get(\u0027swift_sync_key\u0027)"},{"line_number":727,"context_line":"                and (req.environ[\u0027swift_sync_key\u0027] \u003d\u003d"},{"line_number":728,"context_line":"                     req.headers.get(\u0027x-container-sync-key\u0027, None))"},{"line_number":729,"context_line":"                and \u0027x-timestamp\u0027 in req.headers):"},{"line_number":730,"context_line":"            # log_msg \u003d \u0027allowing proxy %s for container-sync\u0027"},{"line_number":731,"context_line":"            # self.logger.debug(log_msg, req.remote_addr)"},{"line_number":732,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_68ec668f","line":729,"in_reply_to":"1aca2d91_5dbd0667","updated":"2015-05-08 06:16:02.000000000","message":"Sorry, forgot to recover them. I will recover them and also put some debug log specially before the enforcer.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"0b16c9de62fd41017fa23c579c64d22d89d17043","unresolved":false,"context_lines":[{"line_number":746,"context_line":""},{"line_number":747,"context_line":"        self._set_project_domain_id(req, identity)"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"        if self._check_action(action, req.environ, identity):"},{"line_number":750,"context_line":"            if self._check_action(\u0027swift_owner\u0027, req.environ, identity):"},{"line_number":751,"context_line":"                req.environ[\u0027swift_owner\u0027] \u003d True"},{"line_number":752,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_dd8a36bf","line":749,"updated":"2015-05-07 14:21:31.000000000","message":"What happens if the administrator accidentally deletes the swift_owner rule from the policy.json file? No error is logged, yet nobody can become a swift_owner.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ff4ce58e9bdeafc1f0f59ef6117e8b49e62d73ef","unresolved":false,"context_lines":[{"line_number":746,"context_line":""},{"line_number":747,"context_line":"        self._set_project_domain_id(req, identity)"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"        if self._check_action(action, req.environ, identity):"},{"line_number":750,"context_line":"            if self._check_action(\u0027swift_owner\u0027, req.environ, identity):"},{"line_number":751,"context_line":"                req.environ[\u0027swift_owner\u0027] \u003d True"},{"line_number":752,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":14,"id":"1aca2d91_48040a49","line":749,"in_reply_to":"1aca2d91_dd8a36bf","updated":"2015-05-08 06:16:02.000000000","message":"I will split the policy file to operator part and logic part. This is described before.","commit_id":"bc7813a2976233f78b0e0f5494762d504ca4d725"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"18714cb1e17e443895af78a4efad490be12eb293","unresolved":false,"context_lines":[{"line_number":37,"context_line":"# If \u0027project\u0027 is not found, that means no __call__() execution of"},{"line_number":38,"context_line":"# CONF so far. Therefore __call__() will be executed here."},{"line_number":39,"context_line":"if not hasattr(CONF, \u0027project\u0027):"},{"line_number":40,"context_line":"    CONF([], project\u003d\u0027swift\u0027)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"_KEYSTONEAUTH_GROUP \u003d \u0027keystoneauth\u0027"},{"line_number":43,"context_line":"_OPT_RESELLER_PREFIXES \u003d cfg.StrOpt(\u0027reseller_prefix\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_97fa0174","line":40,"updated":"2015-06-18 01:07:15.000000000","message":"So it\u0027s weird for me that this will parse the config file based on it\u0027s own opinionated path munging of project before we\u0027ve even finished registering all the options.\n\nCan we do this in the app factory when we need it - at the time the middleware instance is created during loading of the app - instead of anytime you import this module?","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"1cf95cd3dc1b7d1efeffcdf70cd481fd3403a334","unresolved":false,"context_lines":[{"line_number":37,"context_line":"# If \u0027project\u0027 is not found, that means no __call__() execution of"},{"line_number":38,"context_line":"# CONF so far. Therefore __call__() will be executed here."},{"line_number":39,"context_line":"if not hasattr(CONF, \u0027project\u0027):"},{"line_number":40,"context_line":"    CONF([], project\u003d\u0027swift\u0027)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"_KEYSTONEAUTH_GROUP \u003d \u0027keystoneauth\u0027"},{"line_number":43,"context_line":"_OPT_RESELLER_PREFIXES \u003d cfg.StrOpt(\u0027reseller_prefix\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_a00b2b23","line":40,"in_reply_to":"fa32b979_97fa0174","updated":"2015-06-18 09:19:08.000000000","message":"Thanks for the explanation. I understand your thought and I agree with it so I will move the logic to filter_factory.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"18714cb1e17e443895af78a4efad490be12eb293","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                        \u0027 access resources through services with\u0027"},{"line_number":90,"context_line":"                        \u0027 auth system %s.\u0027 % prefix))"},{"line_number":91,"context_line":"CONF.register_opts(_OPTS, group\u003d_KEYSTONEAUTH_GROUP)"},{"line_number":92,"context_line":"_OPTS.append(_OPT_RESELLER_PREFIXES)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"# After this patch is merged, this method will be removed because"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_c8c78696","line":92,"updated":"2015-06-18 01:07:15.000000000","message":"why do you append to the list after you register it?  is that important?","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                        \u0027 access resources through services with\u0027"},{"line_number":90,"context_line":"                        \u0027 auth system %s.\u0027 % prefix))"},{"line_number":91,"context_line":"CONF.register_opts(_OPTS, group\u003d_KEYSTONEAUTH_GROUP)"},{"line_number":92,"context_line":"_OPTS.append(_OPT_RESELLER_PREFIXES)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"# After this patch is merged, this method will be removed because"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_b6fca073","line":92,"in_reply_to":"fa32b979_a30d4ee9","updated":"2015-07-17 05:24:43.000000000","message":"yeah, _OPT didn\u0027t use later so it\u0027s not necessary to have but I merged the options to manage/see the registered options for comparing with CONF.  maybe it\u0027s too much.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                        \u0027 access resources through services with\u0027"},{"line_number":90,"context_line":"                        \u0027 auth system %s.\u0027 % prefix))"},{"line_number":91,"context_line":"CONF.register_opts(_OPTS, group\u003d_KEYSTONEAUTH_GROUP)"},{"line_number":92,"context_line":"_OPTS.append(_OPT_RESELLER_PREFIXES)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"# After this patch is merged, this method will be removed because"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_a30d4ee9","line":92,"in_reply_to":"fa32b979_c0298f8b","updated":"2015-06-19 04:50:30.000000000","message":"yeah, but isn\u0027t the only reason you put those cfg.Opt instances into the _OPTS list so you could pass them to register?  And that option is already registered - so why do you have to put it in the _OPTS list at all?   That list has already done it\u0027s job - doesn\u0027t anyone else reference it?  tests maybe?","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"1cf95cd3dc1b7d1efeffcdf70cd481fd3403a334","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                        \u0027 access resources through services with\u0027"},{"line_number":90,"context_line":"                        \u0027 auth system %s.\u0027 % prefix))"},{"line_number":91,"context_line":"CONF.register_opts(_OPTS, group\u003d_KEYSTONEAUTH_GROUP)"},{"line_number":92,"context_line":"_OPTS.append(_OPT_RESELLER_PREFIXES)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"# After this patch is merged, this method will be removed because"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_c0298f8b","line":92,"in_reply_to":"fa32b979_c8c78696","updated":"2015-06-18 09:19:08.000000000","message":"Yes, keys names\u0027 for operator_roles and service_roles such as AUTH_operator_roles, SERVICE_service_roles and so on can be constructed with reseller_prefix. So I need to register reseller_prefix and then get the values. After that I can construct full name for operator_roles and service_role to get their values.\nBut I will remove toransitioning some middleware configuration logic from this patch.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"18714cb1e17e443895af78a4efad490be12eb293","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    path \u003d \u0027/etc/swift/default-policy.json\u0027"},{"line_number":155,"context_line":"    with open(path, \u0027w\u0027) as f:"},{"line_number":156,"context_line":"        json_string \u003d json.dumps(obj, indent\u003d4)"},{"line_number":157,"context_line":"        f.write(json_string)"},{"line_number":158,"context_line":"    return path"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_8a8bb31e","line":157,"updated":"2015-06-18 01:07:15.000000000","message":"ummm... no - creating an instance of the fucking class can\u0027t go writing a file out to disk.  NO.  This probably wouldn\u0027t even work in most production settings - it wouldn\u0027t work on a SwiftStack\u0027s setup - the \u0027swift\u0027 user doesn\u0027t have write access to /etc/swift *on purpose* - that\u0027s root business - the wsgi server has already dropped priv by the time this would be executed.\n\nEnforcer has a rules kwarg that can take a dict - just pass the rules in that way - if there\u0027s a policy file you can always load_rules(force_reload)","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"1cf95cd3dc1b7d1efeffcdf70cd481fd3403a334","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    path \u003d \u0027/etc/swift/default-policy.json\u0027"},{"line_number":155,"context_line":"    with open(path, \u0027w\u0027) as f:"},{"line_number":156,"context_line":"        json_string \u003d json.dumps(obj, indent\u003d4)"},{"line_number":157,"context_line":"        f.write(json_string)"},{"line_number":158,"context_line":"    return path"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_202f1b89","line":157,"in_reply_to":"fa32b979_8a8bb31e","updated":"2015-06-18 09:19:08.000000000","message":"You are right. I will check Enforcer class to load the dic without creating a file. The logic L417-418 is a way to do it.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"18714cb1e17e443895af78a4efad490be12eb293","unresolved":false,"context_lines":[{"line_number":306,"context_line":"    :param app: The next WSGI app in the pipeline"},{"line_number":307,"context_line":"    :param conf: The dict of configuration values"},{"line_number":308,"context_line":"    \"\"\""},{"line_number":309,"context_line":"    _shared_state \u003d {}"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def __new__(cls, app\u003dNone, conf\u003dNone):"},{"line_number":312,"context_line":"        obj \u003d object.__new__(cls, app, conf)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_d77f2915","line":309,"updated":"2015-06-18 01:07:15.000000000","message":"What could possibly be the justification for this?  Please don\u0027t do this.  If you think you can\u0027t NOT do it - try harder.  If after hours of agony trying not to do this you feel you absolutely can not avoid it - ask for help in irc and write long prose describing why you think you can\u0027t avoid it so that people can explain to why you absolutely CAN do whatever it is you think you need to do without hanging a bunch of shared state off the class.  Then either don\u0027t do this - or give up.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"1cf95cd3dc1b7d1efeffcdf70cd481fd3403a334","unresolved":false,"context_lines":[{"line_number":306,"context_line":"    :param app: The next WSGI app in the pipeline"},{"line_number":307,"context_line":"    :param conf: The dict of configuration values"},{"line_number":308,"context_line":"    \"\"\""},{"line_number":309,"context_line":"    _shared_state \u003d {}"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def __new__(cls, app\u003dNone, conf\u003dNone):"},{"line_number":312,"context_line":"        obj \u003d object.__new__(cls, app, conf)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_a059eb36","line":309,"in_reply_to":"fa32b979_d77f2915","updated":"2015-06-18 09:19:08.000000000","message":"Thanks! I will check used attributes in the KeystoneAuth instance from AclCheck and try to remove the dependencies.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"18714cb1e17e443895af78a4efad490be12eb293","unresolved":false,"context_lines":[{"line_number":316,"context_line":"    def __init__(self, app\u003dNone, conf\u003dNone):"},{"line_number":317,"context_line":"        if app is not None:"},{"line_number":318,"context_line":"            self._app \u003d app"},{"line_number":319,"context_line":"        if conf is not None:"},{"line_number":320,"context_line":"            self._logger \u003d swift_utils.get_logger("},{"line_number":321,"context_line":"                conf, log_route\u003d\u0027keystoneauth\u0027)"},{"line_number":322,"context_line":"            self._conf \u003d conf"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_68b43a34","line":319,"updated":"2015-06-18 01:07:15.000000000","message":"and when *is* conf None?  how do these attributes get populated then?","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"1cf95cd3dc1b7d1efeffcdf70cd481fd3403a334","unresolved":false,"context_lines":[{"line_number":316,"context_line":"    def __init__(self, app\u003dNone, conf\u003dNone):"},{"line_number":317,"context_line":"        if app is not None:"},{"line_number":318,"context_line":"            self._app \u003d app"},{"line_number":319,"context_line":"        if conf is not None:"},{"line_number":320,"context_line":"            self._logger \u003d swift_utils.get_logger("},{"line_number":321,"context_line":"                conf, log_route\u003d\u0027keystoneauth\u0027)"},{"line_number":322,"context_line":"            self._conf \u003d conf"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_c0774f9e","line":319,"in_reply_to":"fa32b979_68b43a34","updated":"2015-06-18 09:19:08.000000000","message":"When KeystoneAuth calls from AclCheck (L788, L791 and L793), the conf is None.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":316,"context_line":"    def __init__(self, app\u003dNone, conf\u003dNone):"},{"line_number":317,"context_line":"        if app is not None:"},{"line_number":318,"context_line":"            self._app \u003d app"},{"line_number":319,"context_line":"        if conf is not None:"},{"line_number":320,"context_line":"            self._logger \u003d swift_utils.get_logger("},{"line_number":321,"context_line":"                conf, log_route\u003d\u0027keystoneauth\u0027)"},{"line_number":322,"context_line":"            self._conf \u003d conf"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_03a19af7","line":319,"in_reply_to":"fa32b979_c0774f9e","updated":"2015-06-19 04:50:30.000000000","message":"AH!  yes, thank you.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"18714cb1e17e443895af78a4efad490be12eb293","unresolved":false,"context_lines":[{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def _create_enforcer(self):"},{"line_number":388,"context_line":"        self._enforcer \u003d Enforcer(CONF, policy_file\u003dself._policy_file)"},{"line_number":389,"context_line":"        self._enforcer.load_rules()"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        rules \u003d {}"},{"line_number":392,"context_line":"        if self._reseller_admin_roles:"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_cab8db2c","line":389,"updated":"2015-06-18 01:07:15.000000000","message":"I think you\u0027ll find that the conf dict that comes into the middleware via the filter_factory has a __file__ key in it - that will either be the file or dir that created the application.\n\nYou should create a ConfigOpt instance like\n\n    conf \u003d cfg.ConfigOpts()\n    conf([\u0027--config-dir\u0027, conf[\u0027__file__\u0027]])\n    conf.register_opts(opts )\n\nin fact, you don\u0027t even *need* to register all those opts, just let them do what the always do.  The only thing you *need* to figure out is the policy.json - which oslo.policy is already going to register for you:\n\n    [oslo_policy]\n    policy_file \u003d /etc/swift/policy.json\n\nthen you can just check\n\n    if os.path.exists(enf.policy_file):\n        enf.load_rules(force_reload\u003dTrue)","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"1cf95cd3dc1b7d1efeffcdf70cd481fd3403a334","unresolved":false,"context_lines":[{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def _create_enforcer(self):"},{"line_number":388,"context_line":"        self._enforcer \u003d Enforcer(CONF, policy_file\u003dself._policy_file)"},{"line_number":389,"context_line":"        self._enforcer.load_rules()"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        rules \u003d {}"},{"line_number":392,"context_line":"        if self._reseller_admin_roles:"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_606763e9","line":389,"in_reply_to":"fa32b979_cab8db2c","updated":"2015-06-18 09:19:08.000000000","message":"Cool! I will read the code and test above behavior.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":624,"context_line":"    def _get_target(self, environ):"},{"line_number":625,"context_line":"        req \u003d Request(environ)"},{"line_number":626,"context_line":"        parts, version, account, container, obj \u003d self._split_path(req)"},{"line_number":627,"context_line":"        referrers, acls \u003d swift_acl.parse_acl(getattr(req, \u0027acl\u0027, None))"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":"        account_prefix \u003d None"},{"line_number":630,"context_line":"        if account:"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_1b8fc275","line":627,"updated":"2015-06-19 04:50:30.000000000","message":"in the old code, the result of parse_acl was (referres, roles) - roles was just a list of groups parsed out of the acl string.  That\u0027s fine, but after it got passed down through the enforcer to the AclCheck, then back into a fresh instance of the Keystone method and run through say... check_roles_by_acls - I\u0027d totally lost track of what it was.  So i\u0027m just calling attention to it for future reviewers.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":656,"context_line":"        elif container:"},{"line_number":657,"context_line":"            action +\u003d \u0027container\u0027"},{"line_number":658,"context_line":"        elif account:"},{"line_number":659,"context_line":"            action +\u003d \u0027account\u0027"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        return action"},{"line_number":662,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_4971030b","line":659,"updated":"2015-06-19 04:50:30.000000000","message":"I think this would be clearer with string interpolation:\n\n    action_template \u003d \u0027%(method)s_%(resource)s\u0027\n    values \u003d {\u0027method\u0027: req.method.lower()}\n    if object:\n       values[\u0027resource\u0027] \u003d \u0027object\u0027\n    ...\n    return action_template % values\n\nI guess you could get trixy with itertools if you want to avoid the if/elif case\n\n    resource_map \u003d {\n      1: \u0027account\u0027,\n      2: \u0027container\u0027,\n      3: \u0027object\u0027\n    }\n    values[\u0027resource\u0027] \u003d resource_map[len(list(itertools.takewhile(\n        lambda x: x is not None, (account, container, obj))))]\n\n\nIf a bug lets in a request without an account does the action \u0027get_\u0027 mean anything?","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":656,"context_line":"        elif container:"},{"line_number":657,"context_line":"            action +\u003d \u0027container\u0027"},{"line_number":658,"context_line":"        elif account:"},{"line_number":659,"context_line":"            action +\u003d \u0027account\u0027"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        return action"},{"line_number":662,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"ba3cc151_bf52fa11","line":659,"in_reply_to":"fa32b979_4971030b","updated":"2015-07-17 05:24:43.000000000","message":"I will update here with upper one. Lower one needs to read the logic for me but I like it. BTW last week I got python training so I think my python is better than before. :-)","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":756,"context_line":""},{"line_number":757,"context_line":""},{"line_number":758,"context_line":"@register(\u0027srole\u0027)"},{"line_number":759,"context_line":"class ServiceRoleCheck(Check):"},{"line_number":760,"context_line":"    \"\"\""},{"line_number":761,"context_line":"    Check that there is a matching service_role"},{"line_number":762,"context_line":"    in the ``creds`` dict."}],"source_content_type":"text/x-python","patch_set":15,"id":"ba3cc151_d5656545","line":759,"updated":"2015-07-17 05:24:43.000000000","message":"I will delete this class because oslo policy enhanced their I/F so this class is not necessary now.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":772,"context_line":"    \"\"\""},{"line_number":773,"context_line":"    Check cross-tenant ACL, a role in ACL and"},{"line_number":774,"context_line":"    access without confirmed identity."},{"line_number":775,"context_line":"    \"\"\""},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    def __call__(self, target, creds, enforcer):"},{"line_number":778,"context_line":"        identity \u003d creds.get(\u0027identity\u0027, None)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_7494db89","line":775,"updated":"2015-06-19 04:50:30.000000000","message":"This class is doing too much to carry all of the legacy behaviors without any additional flexibility - let\u0027s back up.\n\nSo, we want to authorize an action on a resource for the provided identity.  We have some information about the resource - an acl - which is a csv.  Currently the format supports either a list of valid referrers (public is special case) or strings that might be roles or might be user_id:tenant_id.\n\nReading over oslo.policy docs [1] starting at Special Checks and Policy Rule Expression - it seems like in a policy.json the left have side of the colon should be the *kind* of check and the right hand side would be the interpolated value you want to *match*\n\nSpecifically if you look at the GenericCheck and the example policy.json - we have a rule like:\n\n    tenant_id:%(account)s\n\nThis says the \"kind\" of check (the left hand side) is the tenant_id, the attribute \"tenant_id\" is what we\u0027re going to go looking for in creds (the provided identity).  The right hand side is what we\u0027re going to try and match - it\u0027s interpolated from the target.  So this says basically is the tenant_id of this request equal to the targets account.\n\nExtrapolating this, if we wanted to do a check that follows that convention - looks in the identity/request for the attribute on the LHS and then checks if that value is the same as the RHS/match interpolated with the values from the target/resource.\n\nFor referer you imagine someone might want to allow people to grant access for object_get if the request\u0027s referer header matches what\u0027s in the container\u0027s read_acl referrers list.  Something like...\n\nenv.http_referer:%(container.read_acl.referers)\n\n^ I think this would require us to register our own GenericCheck at None since the RHS would interpolate to a list not a string, and the current implementation only handles lists on the LHS.\n\nOTOH, an operator might also want to allow public access if the container.read_acl.referers containers \".r:*\" - but this isn\u0027t contingent on on the identity or the request - just the target - so it\u0027s probably a different kind of rule...\n\n    allow_public:%(container.read_acl.referers)\n\n... the implementation of allow_public would just check if any of the items in the match (container.read_acl.refers) were \u003d\u003d \u0027.r:*\u0027\n\nRoles based acls are probably an extension to the existing check for role - but we have to interpolate match first:\n\n    role:swiftoperator or role:%(container.read_acl.roles)\n\nAnd cross tenant would be:\n\n    tenant_id.user_id:%(container.read_acl.users)\n\nidk... I just feel like it\u0027s not just an \"acls\" check - there\u0027s three things that can get parsed out of *either* of properties (read or write acl) on the target/resource (container, or maybe some day account too) - and those three things should be able to be applied/restricted differently based on operator configuration.\n\n1. http://docs.openstack.org/developer/oslo.policy/api.html#special-checks","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":772,"context_line":"    \"\"\""},{"line_number":773,"context_line":"    Check cross-tenant ACL, a role in ACL and"},{"line_number":774,"context_line":"    access without confirmed identity."},{"line_number":775,"context_line":"    \"\"\""},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    def __call__(self, target, creds, enforcer):"},{"line_number":778,"context_line":"        identity \u003d creds.get(\u0027identity\u0027, None)"}],"source_content_type":"text/x-python","patch_set":15,"id":"ba3cc151_a42c314b","line":775,"in_reply_to":"fa32b979_7494db89","updated":"2015-07-17 05:24:43.000000000","message":"\u003e This class is doing too much to carry all of the legacy behaviors without any additional flexibility - let\u0027s back up.\n\nOK, I will try it. There is a hitorycal reason for this structure. There were three classes until patch #7, first one was for oslo policy, second one was for current behavior and last one was for common logic. So I kept common logic (same as \"legacy behaivors\"). But I decided to drop current hahaivor class in last hack-a-thon, you are right it\u0027s time for cleanup.\n\n\u003e So, we want to authorize an action on a resource for the provided identity. We have some information about the resource - an acl - which is a csv. Currently the format supports either a list of valid referrers (public is special case) or strings that might be roles or might be user_id:tenant_id.\n\u003e Reading over oslo.policy docs [1] starting at Special Checks and Policy Rule Expression - it seems like in a policy.json the left have side of the colon should be the *kind* of check and the right hand side would be the interpolated value you want to *match*\n\u003e Specifically if you look at the GenericCheck and the example policy.json - we have a rule like:\n    tenant_id:%(account)s\n\u003e This says the \"kind\" of check (the left hand side) is the tenant_id, the attribute \"tenant_id\" is what we\u0027re going to go looking for in creds (the provided identity). The right hand side is what we\u0027re going to try and match - it\u0027s interpolated from the target. So this says basically is the tenant_id of this request equal to the targets account.\n\u003e Extrapolating this, if we wanted to do a check that follows that convention - looks in the identity/request for the attribute on the LHS and then checks if that value is the same as the RHS/match interpolated with the values from the target/resource.\n\nI will check the contents in a creds and a target then try to replace AclCheck logic to oslo policy.\n\n\u003e For referer you imagine someone might want to allow people to grant access for object_get if the request\u0027s referer header matches what\u0027s in the container\u0027s read_acl referrers list. Something like...\nenv.http_referer:%(container.read_acl.referers)\n\u003e ^ I think this would require us to register our own GenericCheck at None since the RHS would interpolate to a list not a string, and the current implementation only handles lists on the LHS.\n\nGreat! I never thought like this. I will think it more deeply.\n\n\u003e OTOH, an operator might also want to allow public access if the container.read_acl.referers containers \".r:*\" - but this isn\u0027t contingent on on the identity or the request - just the target - so it\u0027s probably a different kind of rule...\n\u003e     allow_public:%(container.read_acl.referers)\n\u003e ... the implementation of allow_public would just check if any of the items in the match (container.read_acl.refers) were \u003d\u003d \u0027.r:*\u0027\n\n\n\n\u003e Roles based acls are probably an extension to the existing check for role - but we have to interpolate match first:\n\u003e     role:swiftoperator or role:%(container.read_acl.roles)\n\u003e And cross tenant would be:\n\u003e     tenant_id.user_id:%(container.read_acl.users)\n\nNow I have been developing tests. After it\u0027s landed I\u0027m not afraid to change it:-)\n\n\u003e idk... I just feel like it\u0027s not just an \"acls\" check - there\u0027s three things that can get parsed out of *either* of properties (read or write acl) on the target/resource (container, or maybe some day account too) - and those three things should be able to be applied/restricted differently based on operator configuration.\n\u003e 1. http://docs.openstack.org/developer/oslo.policy/api.html#special-checks\n\nYou mean exposing read/write acls on containers to proxy-server.conf?\nIf so, I think it might introduce operator complexity. I don\u0027t know I just feel it\u0027s better to handle separetely b/w static configuration (proxy-server.conf) and dynamic configuration (container acl).","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":774,"context_line":"    access without confirmed identity."},{"line_number":775,"context_line":"    \"\"\""},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    def __call__(self, target, creds, enforcer):"},{"line_number":778,"context_line":"        identity \u003d creds.get(\u0027identity\u0027, None)"},{"line_number":779,"context_line":"        roles \u003d creds.get(\u0027roles\u0027, None)"},{"line_number":780,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_43524236","line":777,"updated":"2015-06-19 04:50:30.000000000","message":"Can you add a doc string that includes what these arguments *are* (at least the types) - best information I have I pieced together from oslo docs [1] and the example policy.json\n\ncutting into the middle of a heavily abstracted external library doesn\u0027t play that well in general - but with out strong type documentation on the public interfaces (which is not mandatory in python) it sort of runs south.  Maybe I should check tests.\n\n1. https://github.com/openstack/oslo-specs/blob/master/specs/kilo/graduate-policy.rst","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":774,"context_line":"    access without confirmed identity."},{"line_number":775,"context_line":"    \"\"\""},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    def __call__(self, target, creds, enforcer):"},{"line_number":778,"context_line":"        identity \u003d creds.get(\u0027identity\u0027, None)"},{"line_number":779,"context_line":"        roles \u003d creds.get(\u0027roles\u0027, None)"},{"line_number":780,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"ba3cc151_c4a615c1","line":777,"in_reply_to":"fa32b979_43524236","updated":"2015-07-17 05:24:43.000000000","message":"I will add a doc-string if this class exists :)\n\nIf I will introduce classes using oslo policy related info, I will also add doc-strings on it.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":786,"context_line":"        match \u003d self.match.lower()"},{"line_number":787,"context_line":"        if match \u003d\u003d \u0027check_cross_tenant\u0027:"},{"line_number":788,"context_line":"            res \u003d KeystoneAuth().authorize_cross_tenant("},{"line_number":789,"context_line":"                req, identity, acls)"},{"line_number":790,"context_line":"        elif match \u003d\u003d \u0027check_roles\u0027:"},{"line_number":791,"context_line":"            res \u003d KeystoneAuth().check_role_by_acls(roles, acls)"},{"line_number":792,"context_line":"        elif match \u003d\u003d \u0027check_public\u0027:"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_a980cff7","line":789,"updated":"2015-06-19 04:50:30.000000000","message":"What... what prevents these from being refctored as pure functions?  Why do we have to create the instance.  Here we have the request, the identity and the acls - let\u0027s just move the logic here.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":786,"context_line":"        match \u003d self.match.lower()"},{"line_number":787,"context_line":"        if match \u003d\u003d \u0027check_cross_tenant\u0027:"},{"line_number":788,"context_line":"            res \u003d KeystoneAuth().authorize_cross_tenant("},{"line_number":789,"context_line":"                req, identity, acls)"},{"line_number":790,"context_line":"        elif match \u003d\u003d \u0027check_roles\u0027:"},{"line_number":791,"context_line":"            res \u003d KeystoneAuth().check_role_by_acls(roles, acls)"},{"line_number":792,"context_line":"        elif match \u003d\u003d \u0027check_public\u0027:"}],"source_content_type":"text/x-python","patch_set":15,"id":"ba3cc151_5fb40ef1","line":789,"in_reply_to":"fa32b979_a980cff7","updated":"2015-07-17 05:24:43.000000000","message":"I would like to share attributes of KeystoneAuth class such as self._XXX. These are used in  _is_name_allowed_in_acl so I used Borg pattern. I will try to remove this class.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":794,"context_line":"                req, obj, acls, referrers)"},{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            raise ValueError(\u0027{match} not allowed for rule \"acl\"\u0027."},{"line_number":797,"context_line":"                             format(match\u003dself.match))"},{"line_number":798,"context_line":"        return res"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_a9b20f04","line":797,"updated":"2015-06-19 04:50:30.000000000","message":"why do we have to wait till run time to raise this?  match comes from the policy.json right?  Is there not a prehook during load that we can validate the policy.json match values with?","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":794,"context_line":"                req, obj, acls, referrers)"},{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            raise ValueError(\u0027{match} not allowed for rule \"acl\"\u0027."},{"line_number":797,"context_line":"                             format(match\u003dself.match))"},{"line_number":798,"context_line":"        return res"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"ba3cc151_df76de3d","line":797,"in_reply_to":"fa32b979_a9b20f04","updated":"2015-07-17 05:24:43.000000000","message":"I think there is no prehook so this class is initiated with no-supported string. I will re-check it later.","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"42882d19093563dc76fc5044f6053f9daaf8e2a2","unresolved":false,"context_lines":[{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            raise ValueError(\u0027{match} not allowed for rule \"acl\"\u0027."},{"line_number":797,"context_line":"                             format(match\u003dself.match))"},{"line_number":798,"context_line":"        return res"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"def filter_factory(global_conf, **local_conf):"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_e3335612","line":798,"updated":"2015-06-19 04:50:30.000000000","message":"is res here just \"result\" - I read it as \"response\" but I think it\u0027s just a string or None","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d897dd2c4753f3ab082385dd17da853b50e6cf5b","unresolved":false,"context_lines":[{"line_number":795,"context_line":"        else:"},{"line_number":796,"context_line":"            raise ValueError(\u0027{match} not allowed for rule \"acl\"\u0027."},{"line_number":797,"context_line":"                             format(match\u003dself.match))"},{"line_number":798,"context_line":"        return res"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"def filter_factory(global_conf, **local_conf):"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_b66b40f1","line":798,"in_reply_to":"fa32b979_e3335612","updated":"2015-07-17 05:24:43.000000000","message":"right! I will change it to \"result\".","commit_id":"45d8ed93d70cba4a39dce7f2b9249b276705412d"}],"test-requirements.txt":[{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"976b7dd4f453d9337d6fb8aaa16ae585cb4a3801","unresolved":false,"context_lines":[{"line_number":12,"context_line":"oslosphinx"},{"line_number":13,"context_line":"sphinx\u003e\u003d1.1.2,\u003c1.2"},{"line_number":14,"context_line":"mock\u003e\u003d1.0"},{"line_number":15,"context_line":"python-swiftclient"},{"line_number":16,"context_line":"# oslo.policy"},{"line_number":17,"context_line":"-e git://github.com/rodrigods/oslo.policy.git#egg\u003doslo_policy"}],"source_content_type":"text/plain","patch_set":4,"id":"fa81d914_6f03c5b8","line":15,"updated":"2015-01-28 12:21:26.000000000","message":"There is no requirements.txt -- is that why devstack failed? Error: \"ImportError: No module named oslo_policy.policy\" in http://logs.openstack.org/30/149930/4/check/check-tempest-dsvm-full/f3f7cfb/logs/devstacklog.txt.gz","commit_id":"981d759e453e124a78e0c5803819df6ddceb16b7"}]}