feat(oauth2): strict string matching for redirect_uri
#82294
Conversation
github-actions
bot
added
the
Scope: Backend
| @@ -13,15 +13,15 @@ def test_is_valid_redirect_uri(self): | |||
| assert app.is_valid_redirect_uri("http://example.com/") | |||
| assert app.is_valid_redirect_uri("http://example.com") | |||
| assert app.is_valid_redirect_uri("http://example.com/.") | |||
| assert app.is_valid_redirect_uri("http://example.com//") | |||
| assert app.is_valid_redirect_uri("http://example.com/biz/baz") | |||
| assert not app.is_valid_redirect_uri("http://example.com//") | |||
There was a problem hiding this comment.
First three examples are normalized to / by user agents. However, the last one is //, considered a different URL:
$ curl -v http://example.com/ 2>&1 | grep GET
> GET / HTTP/1.1
$ curl -v http://example.com 2>&1 | grep GET
> GET / HTTP/1.1
$ curl -v http://example.com/. 2>&1 | grep GET
> GET / HTTP/1.1
$ curl -v http://example.com// 2>&1 | grep GET
> GET // HTTP/1.1
| assert not app.is_valid_redirect_uri("https://example.com/") | ||
| assert not app.is_valid_redirect_uri("http://foo.com") | ||
| assert not app.is_valid_redirect_uri("http://example.com.foo.com") | ||
|
|
||
| assert app.is_valid_redirect_uri("http://sub.example.com/path") | ||
| assert app.is_valid_redirect_uri("http://sub.example.com/path/") | ||
| assert app.is_valid_redirect_uri("http://sub.example.com/path/bar") | ||
| assert not app.is_valid_redirect_uri("http://sub.example.com/path/") |
There was a problem hiding this comment.
When it's not root of the domain, then the path with and without slash are considered different:
$ curl -v http://example.com/path 2>&1 | grep GET
> GET /path HTTP/1.1
$ curl -v http://example.com/path/ 2>&1 | grep GET
> GET /path/ HTTP/1.1
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #82294 +/- ##
==========================================
+ Coverage 87.60% 87.65% +0.04%
==========================================
Files 9647 9502 -145
Lines 545335 543030 -2305
Branches 21428 21009 -419
==========================================
- Hits 477732 475983 -1749
+ Misses 67260 66641 -619
- Partials 343 406 +63 |
Enforcing strict string matching for
redirect_urifor more secure OAuth2 flows according to best practices: