{"action":"created","issue":{"url":"https://api.github.com/repos/sief/play-guard/issues/5","repository_url":"https://api.github.com/repos/sief/play-guard","labels_url":"https://api.github.com/repos/sief/play-guard/issues/5/labels{/name}","comments_url":"https://api.github.com/repos/sief/play-guard/issues/5/comments","events_url":"https://api.github.com/repos/sief/play-guard/issues/5/events","html_url":"https://github.com/sief/play-guard/pull/5","id":296264911,"number":5,"title":"Rely on play.http.forwarded.trustedProxies for X-Forwarded-For header parsing","user":{"login":"dominics","id":97427,"avatar_url":"https://avatars0.githubusercontent.com/u/97427?v=4","gravatar_id":"","url":"https://api.github.com/users/dominics","html_url":"https://github.com/dominics","followers_url":"https://api.github.com/users/dominics/followers","following_url":"https://api.github.com/users/dominics/following{/other_user}","gists_url":"https://api.github.com/users/dominics/gists{/gist_id}","starred_url":"https://api.github.com/users/dominics/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dominics/subscriptions","organizations_url":"https://api.github.com/users/dominics/orgs","repos_url":"https://api.github.com/users/dominics/repos","events_url":"https://api.github.com/users/dominics/events{/privacy}","received_events_url":"https://api.github.com/users/dominics/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-02-12T04:38:51Z","updated_at":"2018-02-12T18:18:39Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/sief/play-guard/pulls/5","html_url":"https://github.com/sief/play-guard/pull/5","diff_url":"https://github.com/sief/play-guard/pull/5.diff","patch_url":"https://github.com/sief/play-guard/pull/5.patch"},"body":"Pretty straightforward I think, and I've added a test that we're no longer trusting unvalidated any part of `X-Forwarded-For` when `playguard.clientipheader` isn't configured.\r\n\r\nNote that when `playguard.clientipheader` _is_ configured, the existing behaviour is to use the entire value of that header (package.scala, line 11), which is ... problematic for `X-Forwarded-For`. I think that'll only work in the case of e.g. the informal `X-Client-Ip` header (which doesn't usually have an appended \"path\" of proxies). \r\n\r\nI'd recommend fully deprecating that configuration option in any case (although I've left it alone, and it continues to work as it always did), and definitely never mentioning `clientipheader = \"X-Forwarded-For\"` specifically.\r\n\r\nREADME updated too. Fixes #4 "},"comment":{"url":"https://api.github.com/repos/sief/play-guard/issues/comments/365012749","html_url":"https://github.com/sief/play-guard/pull/5#issuecomment-365012749","issue_url":"https://api.github.com/repos/sief/play-guard/issues/5","id":365012749,"user":{"login":"sief","id":1277638,"avatar_url":"https://avatars3.githubusercontent.com/u/1277638?v=4","gravatar_id":"","url":"https://api.github.com/users/sief","html_url":"https://github.com/sief","followers_url":"https://api.github.com/users/sief/followers","following_url":"https://api.github.com/users/sief/following{/other_user}","gists_url":"https://api.github.com/users/sief/gists{/gist_id}","starred_url":"https://api.github.com/users/sief/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sief/subscriptions","organizations_url":"https://api.github.com/users/sief/orgs","repos_url":"https://api.github.com/users/sief/repos","events_url":"https://api.github.com/users/sief/events{/privacy}","received_events_url":"https://api.github.com/users/sief/received_events","type":"User","site_admin":false},"created_at":"2018-02-12T18:18:39Z","updated_at":"2018-02-12T18:18:39Z","author_association":"OWNER","body":"I ran into a problem with this approach: I need a solution that works in an environment, where I trust the immediate connection but don't know the IP address (e.g. Heroku)\r\n\r\nAlthough the the docs say \"taking the first untrusted IP address that it finds as the reported user remote address (or the the last IP address if all proxies are trusted.)\", trusting all proxies with: play.http.forwarded.trustedProxies=[\"0.0.0.0/0\", \"::/0\"] will result in the first address, not the last. \r\nI think this makes sense and it's obvious if you look at the source code:\r\n\r\nhttps://github.com/playframework/playframework/blob/145314cbab94d9ffb8abe14c9e4b65a10839b62f/framework/src/play-server/src/main/scala/play/core/server/common/ForwardedHeaderHandler.scala#L75\r\n\r\nhttps://github.com/playframework/playframework/blob/145314cbab94d9ffb8abe14c9e4b65a10839b62f/framework/src/play-server/src/test/scala/play/core/server/common/ForwardedHeaderHandlerSpec.scala#L426\r\n\r\n\r\nSetting no trusted proxies doesn't evaluate the forwarded header at all and gives you the immediate connection, definitely not useful for rate limiting ;)\r\n\r\n\r\n\r\n\r\nMaybe Play needs a config flag like `play.http.forwarded.trustImmediateConnection=true `\r\n\r\nAm I missing something here? Any ideas? \r\n"}}
{"actor":{"display_login":"sief"}}