Mercurial > p > roundup > code
comparison roundup/cgi/client.py @ 8247:6747051fef79
feat: issue2551372 - REST-API CSRF protection should document mandatory Origin header
Logging is more useful I hope.
Logs the name of the user making the request.
Logs the value of the origin header if the value is not authorized
to use the rest interface.
Added a comment about difficulty include originating IP address
in log.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Tue, 31 Dec 2024 17:11:17 -0500 |
| parents | 741ea8a86012 |
| children | cae1bbf2536b |
comparison
equal
deleted
inserted
replaced
| 8246:3812c0fb1137 | 8247:6747051fef79 |
|---|---|
| 724 # verify Origin is allowed on all requests including GET. | 724 # verify Origin is allowed on all requests including GET. |
| 725 # If a GET, missing origin is allowed (i.e. same site GET request) | 725 # If a GET, missing origin is allowed (i.e. same site GET request) |
| 726 if not self.is_origin_header_ok(api=True): | 726 if not self.is_origin_header_ok(api=True): |
| 727 if 'HTTP_ORIGIN' not in self.env: | 727 if 'HTTP_ORIGIN' not in self.env: |
| 728 msg = self._("Required Header Missing") | 728 msg = self._("Required Header Missing") |
| 729 err = 'Origin header missing' | 729 err = "REST request missing 'Origin' header by user %(user)s." |
| 730 else: | 730 else: |
| 731 msg = self._("Client is not allowed to use Rest Interface.") | 731 msg = self._("Client is not allowed to use Rest Interface.") |
| 732 err = 'Unauthorized for REST request' | 732 err = "REST request 'Origin' (%(origin)s) unauthorized by user %(user)s." |
| 733 | 733 |
| 734 # Use code 400. Codes 401 and 403 imply that authentication | 734 # Use code 400. Codes 401 and 403 imply that authentication |
| 735 # is needed or authenticated person is not authorized. | 735 # is needed or authenticated person is not authorized. |
| 736 # Preflight doesn't do authentication. | 736 # Preflight doesn't do authentication. |
| 737 output = s2b( | 737 output = s2b( |
| 738 '{ "error": { "status": 400, "msg": "%s" } }' % msg) | 738 '{ "error": { "status": 400, "msg": "%s" } }' % msg) |
| 739 self.reject_request(output, | 739 self.reject_request(output, |
| 740 message_type="application/json", | 740 message_type="application/json", |
| 741 status=400) | 741 status=400) |
| 742 logger.error(err) | 742 # Would be nice to log the original source address here to |
| 743 # allow firewalling in case of abuse/attack. Especially if | |
| 744 # anonymous is allowed REST access. However, | |
| 745 # self.request.connection.getpeername() | |
| 746 # only gets us 127.0.0.1 when a proxy is used. I think the | |
| 747 # same is true of wsgi mode (but it might be a UNIX domain | |
| 748 # socket address). The upstream server needs to supply the | |
| 749 # real IP as it sees it and we need to consume it. There | |
| 750 # is no method for this that handles all the ways roundup | |
| 751 # can be run AFAIK. So no IP address, just user. | |
| 752 logger.error(err, {"user": self.user, | |
| 753 "origin": self.env.get('HTTP_ORIGIN', None)}) | |
| 743 return | 754 return |
| 744 | 755 |
| 745 # Handle CORS preflight request. We know rest is enabled | 756 # Handle CORS preflight request. We know rest is enabled |
| 746 # because handle_rest is called. Preflight requests | 757 # because handle_rest is called. Preflight requests |
| 747 # are unauthenticated, so no need to check permissions. | 758 # are unauthenticated, so no need to check permissions. |