Roundup Tracker - Issues

Message7504

Author rouilj
Recipients marcus.priesch, rouilj, schlatterbeck
Date 2022-05-04.14:40:28
Message-id <1651675228.52.0.0191992563803.issue2551203@roundup.psfhosted.org>
In-reply-to
Hi Marcus:

Hmm, I coded the rest interface with CORS headers. They return the
actual methods allowed for the end point. Any idea why it's not
working?

For example look in rest.py's dispatch() method for:

        # add access-control-allow-* to support CORS
        self.client.setHeader("Access-Control-Allow-Origin", "*")
        self.client.setHeader(
            "Access-Control-Allow-Headers",
            "Content-Type, Authorization, X-Requested-With, X-HTTP-Method-Override"
        )
        self.client.setHeader(
            "Allow",
            "OPTIONS, GET, POST, PUT, DELETE, PATCH"
        )
        self.client.setHeader(
            "Access-Control-Allow-Methods",
            "HEAD, OPTIONS, GET, POST, PUT, DELETE, PATCH"
        )

The Access-Control-Allow-Methods is modified for each route. For
example in rest.py find:

    @Routing.route("/data/<:class_name>", 'OPTIONS')
    @_data_decorator
    def options_collection(self, class_name, input):

which handles an OPTIONS request to e.g. /data/issue. At the end of
that method is an override:

        self.client.setHeader(
            "Access-Control-Allow-Methods",
            "OPTIONS, GET, POST"
        )
        return 204, ""

since PUT, PATCH and DELETE aren't valid for /data/issue.

I return these headers blind - I don't check the client's
Access-Control-Request-Method or Access-Control-Request-Headers.  But
that shouldn't matter if the headers are valid/allowed.

I did not implement Access-Control-Max-Age and
Access-Control-Allow-Credentials.  Max-Age should be optional. Reading
up on Access-Control-Allow-Credentials, it looks like I should be
returning that header set to true if credentials are supplied for the
options request. Supplying credentials is the normal use case.
 
Also I wildcard (*) Access-Control-Allow-Origin, so any origin is
accepted. Your code just mirrors the origin supplied by the client
which (should have) the same result.  Ah, I just reread the doc on it.
'*' only works when no credentials are supplied.

Can you try patching rest.py::dispatch() to set
Access-Control-Allow-Origin using the client header (as in your patch)
and add an:

  Access-Control-Allow-Credentials: true

header and see if CORS works without the patch you attached here.

We should not need "Vary: Origin" the response, since we aren't
filtering on the client's Origin header.

Also I am not sure if XMLRPC needs CORS. I am not even sure if xmlrpc
will respond to an OPTIONS query. IIRC the xmlrpc endpoint uses only
POST requests. If we implement this, it might be enough to add support
for OPTIONS on that end point, reply with the same headers as rest.py::dispatch()
but hard code the allow methods to POST.
 
> could make sense to also talk to the html frontend directly via CORS
> from within another webpage.

My initial thought is that iframe would be used for this which IIRC
doesn't use CORS. It has other restrictions in CSP etc. However IIRC
download/upload of files runs through the web endpoint so we might
need something there as well. What use cases do you have in mind?
Would Roundup supply HTML partials/fragments to be composed on the
client?
History
Date User Action Args
2022-05-04 14:40:28rouiljsetmessageid: <1651675228.52.0.0191992563803.issue2551203@roundup.psfhosted.org>
2022-05-04 14:40:28rouiljsetrecipients: + rouilj, schlatterbeck, marcus.priesch
2022-05-04 14:40:28rouiljlinkissue2551203 messages
2022-05-04 14:40:28rouiljcreate