API: Add HTTP 200 Return in case we have an OPTIONS request #2594
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2594 +/- ##
==========================================
+ Coverage 27.42% 27.43% +0.01%
==========================================
Files 85 85
Lines 11774 11783 +9
==========================================
+ Hits 3229 3233 +4
- Misses 8545 8550 +5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
After skipping through CORS and xmlrpc.server the PR seems fine. My only question left is, why you separated the header requests and did not put them all together in the do_OPTIONS() method like this:
class RequestHandler(SimpleXMLRPCRequestHandler):
def do_OPTIONS(self):
self.send_response(200)
self.send_header("Access-Control-Allow-Headers",
"Origin, X-Requested-With, Content-Type, Accept")
self.send_header("Access-Control-Allow-Origin", "*")
SimpleXMLRPCRequestHandler.end_headers(self)The methond itself implements the CORS pre-flighted access for resources and I thought only this is what we need. Furthermore I am not sure if we acutally need all of the allowed Access-Control-Allow-Headers but I don't know how to test this with the new Angular web interface. @SchoolGuy Could you provide some steps how to reproduce the issue you ran into?
As long as it works, it's fine. But we should definitly check those settings regarding security before shipping the new web interface.
|
@nodeg I will merge this PR then and I would like that you create an issue with all things we should consider before releasing the next version. Would that be okay with you? |
|
Of course. The comment was only about things I wondered about 😄 |
While working on the new Angular Frontend for Cobbler I noticed that browsers send an OPTIONS request for pre CORS checks. Since our application didn't implement those yet, we were never successfully able to being set up in an environment where this is of concern. We now add this with this commit.