)]}'
{"swift/common/middleware/keystoneauth.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"db1c15b3404f216d4758c2e55a3ac0040f4a4e7b","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        # In next release, we would add user id in env[\u0027keystone.identity\u0027] by"},{"line_number":242,"context_line":"        # using _integral_keystone_identity to replace current"},{"line_number":243,"context_line":"        # _keystone_identity. The purpose of keeping it in this release it for"},{"line_number":244,"context_line":"        # back compatibility."},{"line_number":245,"context_line":"        if environ.get(\u0027HTTP_X_IDENTITY_STATUS\u0027) !\u003d \u0027Confirmed\u0027:"},{"line_number":246,"context_line":"            return"},{"line_number":247,"context_line":"        roles \u003d []"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_769bd017","line":244,"updated":"2015-03-10 10:12:34.000000000","message":"should this be where we add check on \u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027?\n\n  if environ.get(\u0027HTTP_X_IDENTITY_STATUS\u0027) !\u003d \u0027Confirmed\u0027\n          or environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027) not in (None, \u0027Confirmed\u0027):\n      return","commit_id":"02901dc88d0eaebdc20e8b30361ebb7f19faaf9f"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"4af9c8642616c43d8fdcb1b5812b9c2cfccf9ab6","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        are valid (this is, we know who the user is)."},{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        srv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027,"},{"line_number":578,"context_line":"                                           \u0027Confirmed\u0027)"},{"line_number":579,"context_line":"        if req.remote_user and srv_token_status \u003d\u003d \u0027Confirmed\u0027:"},{"line_number":580,"context_line":"            return HTTPForbidden(request\u003dreq)"},{"line_number":581,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_4903f641","line":578,"updated":"2015-03-03 02:27:13.000000000","message":"If HTTP_X_SERVICE_IDENTITY_STATUS is not specified in this environ, this means a service token is not presented so I think we need to deal with \u0027None\u0027 in addition to \u0027Confirmed\u0027 and \u0027Invalid\u0027.\n\nsrv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027)","commit_id":"02901dc88d0eaebdc20e8b30361ebb7f19faaf9f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"db1c15b3404f216d4758c2e55a3ac0040f4a4e7b","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        are valid (this is, we know who the user is)."},{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        srv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027,"},{"line_number":578,"context_line":"                                           \u0027Confirmed\u0027)"},{"line_number":579,"context_line":"        if req.remote_user and srv_token_status \u003d\u003d \u0027Confirmed\u0027:"},{"line_number":580,"context_line":"            return HTTPForbidden(request\u003dreq)"},{"line_number":581,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_33ec0abb","line":578,"in_reply_to":"ba7be1f8_4903f641","updated":"2015-03-10 10:12:34.000000000","message":"I think Donagh\u0027s intent here is that if HTTP_X_SERVICE_IDENTITY_STATUS is not specified in this environ, then the behavior should be the same as if HTTP_X_SERVICE_IDENTITY_STATUS\u003d\u003d\u0027Confirmed\u0027, so I think this code is fine.\n\nHowever, I think it could be clearer to write:\n\n  srv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027)\n  if req.remote_user and srv_token_status in(None, \u0027Confirmed\u0027)","commit_id":"02901dc88d0eaebdc20e8b30361ebb7f19faaf9f"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"4af9c8642616c43d8fdcb1b5812b9c2cfccf9ab6","unresolved":false,"context_lines":[{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        srv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027,"},{"line_number":578,"context_line":"                                           \u0027Confirmed\u0027)"},{"line_number":579,"context_line":"        if req.remote_user and srv_token_status \u003d\u003d \u0027Confirmed\u0027:"},{"line_number":580,"context_line":"            return HTTPForbidden(request\u003dreq)"},{"line_number":581,"context_line":"        else:"},{"line_number":582,"context_line":"            return HTTPUnauthorized(request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_897ecead","line":579,"updated":"2015-03-03 02:27:13.000000000","message":"REMOTE_USER is set if HTTP_X__IDENTITY_STATUS is \u0027Confirmed\u0027 (means NOT anonymous). It might be good if we use HTTP_X__IDENTITY_STATUS instead of REMOTE_USER. when we introduce HTTP_X_SERVICE_IDENTITY_STATUS.","commit_id":"02901dc88d0eaebdc20e8b30361ebb7f19faaf9f"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"48352fe55a369a3142739e2b80d75b3d8f23bf0a","unresolved":false,"context_lines":[{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        srv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027,"},{"line_number":578,"context_line":"                                           \u0027Confirmed\u0027)"},{"line_number":579,"context_line":"        if req.remote_user and srv_token_status \u003d\u003d \u0027Confirmed\u0027:"},{"line_number":580,"context_line":"            return HTTPForbidden(request\u003dreq)"},{"line_number":581,"context_line":"        else:"},{"line_number":582,"context_line":"            return HTTPUnauthorized(request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_aac5cf3e","line":579,"in_reply_to":"9a80dd14_42e0c6cc","updated":"2015-03-10 14:25:32.000000000","message":"Having thought about it a bit more, I\u0027m happy to keep it simple: invalid token: remote_user not set. The edge case I mentioned above would not even get through auth_token, so I\u0027m happy to out the change as suggested at line 245. Alternates seem to involve too much plumbing (specifically, it\u0027s not enough to pass something to authorize() because denied_response() is only passed the request.","commit_id":"02901dc88d0eaebdc20e8b30361ebb7f19faaf9f"},{"author":{"_account_id":860,"name":"Donagh McCabe","email":"donagh.mccabe@gmail.com","username":"donagh-mccabe"},"change_message_id":"eeaa62d3b3047ae61791879d746db075fb64a38e","unresolved":false,"context_lines":[{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        srv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027,"},{"line_number":578,"context_line":"                                           \u0027Confirmed\u0027)"},{"line_number":579,"context_line":"        if req.remote_user and srv_token_status \u003d\u003d \u0027Confirmed\u0027:"},{"line_number":580,"context_line":"            return HTTPForbidden(request\u003dreq)"},{"line_number":581,"context_line":"        else:"},{"line_number":582,"context_line":"            return HTTPUnauthorized(request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_42e0c6cc","line":579,"in_reply_to":"9a80dd14_a17ee467","updated":"2015-03-10 11:17:55.000000000","message":"Moving the check to line 245 has the effect of turning a request with a valid X-Auth-Token but invalid X-Service-Token into an anonymous request.\n\nConsider this situation:\n\nUser can perform an action using X-Auth-Token ONLY (i.e, the token gives enough roles, etc, by itself)\n\n- User omits X-Service-Token -- request works\n- User supplies invalid X-Service-Token -- request denied\n\n(I\u0027m assuming we don\u0027t have an ACL such as .r:* (referrer ACL) that would allow an anonymous request)\n\nI guess if the user chooses to present an invalid X-Service-Token, they deserve the denial. However, I\u0027d like to think about it a bit more.","commit_id":"02901dc88d0eaebdc20e8b30361ebb7f19faaf9f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"db1c15b3404f216d4758c2e55a3ac0040f4a4e7b","unresolved":false,"context_lines":[{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        srv_token_status \u003d req.environ.get(\u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027,"},{"line_number":578,"context_line":"                                           \u0027Confirmed\u0027)"},{"line_number":579,"context_line":"        if req.remote_user and srv_token_status \u003d\u003d \u0027Confirmed\u0027:"},{"line_number":580,"context_line":"            return HTTPForbidden(request\u003dreq)"},{"line_number":581,"context_line":"        else:"},{"line_number":582,"context_line":"            return HTTPUnauthorized(request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_a17ee467","line":579,"in_reply_to":"ba7be1f8_897ecead","updated":"2015-03-10 10:12:34.000000000","message":"I went digging into use of remote_user a bit:\n\nsee https://github.com/openstack/swift/blob/master/swift/common/wsgi.py#L636\n\nwhen subrequests are made, remote_user is copied to subrequest but HTTP_[SERVICE_]IDENTITY_* headers are not (because they are authtoken specific and therefore unknown to wsgi.py). \u0027swift.authorize\u0027 is also copied and in case of keystoneauth this is a partial with reference to the identity dict.\n\nSo if a subrequest ends up passed to denied_response we will not have any HTTP_[SERVICE_]IDENTITY_* headers from original request, which I *think* means testing for \u0027HTTP_X_SERVICE_IDENTITY_STATUS\u0027 here is broken. (And also explains why we *should* test remote_user and not HTTP_X_IDENTITY_STATUS)\n\n@Donagh - what do you think about making the check at line 245?","commit_id":"02901dc88d0eaebdc20e8b30361ebb7f19faaf9f"}]}