upon developing a rest frontend for roundup i came across 
the need for so called CORS preflight requests. 

see https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request

basically this is a "OPTIONS" request that gets sent by the browser when 
you want to do CORS.

in frontend development i have a roundup-server running on port 8080 and 
a node.js server running on port 8081 where the frontend gets 
developed (vue.js with hot module reload etc.)

the web frontend gets loaded from the node.js server at port 8081 (to 
have all the fancy hot reloading working) and communicates with the rest 
api of the roundup server at port 8080 thus forming a nice CORS setup 
where the browser issues CORS preflight requests before any real 
request to ask the server what he is allowed to do.

attached is a patch that i have applied to make this working.

as i am not really deep in the CORS thing i am not sure if my solution 
covers all aspects of this or has any security impacts. 

the patch most likely also needs to be utilized for the xmlrpc frontend 
if it gets accessed from within a webbrowser - can't imagine who wants 
this, but maybe we should support it - but maybe it should be implemented 
for all web requests ? - could make sense to also talk to the html 
frontend directly via CORS from within another webpage.

so the patch is meant as a starting point to discuss how this can be added 
to roundup at all - and what i needed to start hacking with the rest api.

note that when you finally run the web frontend served from the same url 
where the rest api is running, you dont need this at all !

regards,
marcus.

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?

Hello John,

the problem is that the preflight request comes without any
authorization (because it is asking the server what is allowed) and
therefore it always returns 403.

in client.py line 652 (with the patch applied) it reads:

if not self.db.security.hasPermission('Rest Access', self.userid):
     self.response_code = 403

user is "anonymous" here as we have no auth data in the preflight request.

thats what the patch in client.py (below "# handle preflight...")
addresses.

at this point the patch  allow everything that is supported down the way
(hopefully), assuming that at deeper levels somewhere things get
forbidden based on permissions and other rules.

the

	@Routing.route("/data/<:class_name>", 'OPTIONS')

section never gets hit at this point, because it only would if someone
has "Rest Access" permission. which anonymous dont has.

but maybe it should for that reason ? maybe that would be too open ?
maybe anonymous should just have enough permission to reach the code
above to send correct cors headers? - which probably would be better
than sending one answer for all ? ... but i am not that deep in the
permission thing if this is possible :(

acc. to max-age i found out that Access-Control-Max-Age indeed is a time
saver when you access same endpoint within the time range as the OPTIONS
request then gets not sent by the browser. maybe this should be
configurable for the user ?

> 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?

i also think that iframes dont use cors, but iframes are ugly anyway ;)

i dont have anything in mind, but i can imagine someone building a
frontend requesting html fragments (maybe running through tal for
i18n ;)) or something else ...

i am rather thinking about where to put the code to address all possible
use cases at once ...

when you say xmlrpc is working over post - hmmm, i dont think it would
be needed because most likely there will be no browser talking to it ...
but can be, if someone is wierd enough to build a webpage using xmlrpc
instead of rest ;)

in the end it's all just plain http requests ... so i would vote to have
the CORS thing for all http requests.

regards,
marcus.

Hi Marcus:

In message <52323af0-99c1-043d-4573-c954f914f52c@priesch.co.at>,
Marcus Priesch writes:
>the problem is that the preflight request comes without any
>authorization (because it is asking the server what is allowed) and
>therefore it always returns 403.

Hmm, have you tried making a credentialed request?

https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#requests_with_credentials
https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#sending_a_request_with_credentials_included

So rather than a simple fetch to /rest/issue/23 you do a credentialed
fetch there.

I would think you would have to do that anyway as /rest/issue/23 may
not be readable to anonymous (you can control permission on a class
(e.g. can't access any user item), an item (you can access only your
user item), or property (you can't access the Role property of your
user item). All three of these are exposed as endpoints in the REST
interface.

>in client.py line 652 (with the patch applied) it reads:
>
>if not self.db.security.hasPermission('Rest Access', self.userid):
>     self.response_code = 403
>
>user is "anonymous" here as we have no auth data in the preflight request.

Does it work with a non-credentialed preflight if anonymous has 'Rest
Access' permission? (See below for how to enable.)

>thats what the patch in client.py (below "# handle preflight...")
>addresses.
>
>at this point the patch allow everything that is supported down the way
>(hopefully), assuming that at deeper levels somewhere things get
>forbidden based on permissions and other rules.

You are correct that a delete against /rest/issue would be rejected
regardless of what CORS reports. However if somebody codes an
interface that trusts what CORS reports it could get ugly by showing
users things that will not work.

>	@Routing.route("/data/<:class_name>", 'OPTIONS')
>
>section never gets hit at this point, because it only would if someone
>has "Rest Access" permission. which anonymous dont has.

We could add a "CORS Access" right and check to see if it's an OPTIONS
with the proper header subset (Allow-*). So your patch would not
perform the response but would permit the request to bypass the 403. (1)

However if anonymous is not allowed Rest Access, you will have to pass
credentials when performing the actual POST/DELETE/PUT/GET call. So
the credentials for the rest end point should be passed for CORS as
well.

>but maybe it should for that reason ? maybe that would be too open ?

Currently the Anonymous user has "Web Access" rights in almost all
cases otherwise you can't log in. They may still be unable to see any
data (due to the permissions model). For anybody implementing a REST
based front end I think anonymous should have REST access.

Consider the roundup-tracker. Anonymous is allowed read only access to
issues. Implementing the same via rest would have to allow anonymous
"Rest Access" permissions.

Simply add:

  db.security.addPermissionToRole('Anonymous', 'Rest Access')

at the end of schema.py and restart the server.

>maybe anonymous should just have enough permission to reach the code
>above to send correct cors headers? - which probably would be better
>than sending one answer for all ? ... but i am not that deep in the
>permission thing if this is possible :(

We would need to add a patch as in (1).

>acc. to max-age i found out that Access-Control-Max-Age indeed is a time
>saver when you access same endpoint within the time range as the OPTIONS
>request then gets not sent by the browser. maybe this should be
>configurable for the user ?

For right now, hard code the lifetime to say a week or so. I think
this is ok since:

  1. the access data is static. The only way the permissions can
     change is by changing the schema and restarting the server.

  2. if the cached access settings are invalid (become stale) the client
     will make a request that returns a 403 or other error. This isn't
     great but if it only occurs after permissions are changed it should
     not occur often.

So there is no security or data integrity issues with caching the
Access results too long. If we need to make that configurable in
config.ini or extract it into something that can be overridden using
interfaces.py we can discuss it when we see the need.

>[...] What use cases do you have in mind?
>> Would Roundup supply HTML partials/fragments to be composed on the
>> client?
> [...]
>i dont have anything in mind, but i can imagine someone building a
>frontend requesting html fragments (maybe running through tal for
>i18n ;)) or something else ...

Ok, so my scenario isn't totally crazy.

>i am rather thinking about where to put the code to address all possible
>use cases at once ...

I am not sure that is possible to do accurately.

  1. for rest we need to parse the route to determine if it's valid
     endpoint (rest/isue for example should fail)
  2. for rest we should to return only supported methods (e.g. not
     include DELETE for collection endpoints)
  3. for xmlrpc it only supports POST
  4. for http, the verb I think is only GET or POST. The actual verb
     is defined in the @action component of the query string.
     It's not even determined by a path component in the
     URL. So create is @action=new and retire/delete is @action=retire.

>when you say xmlrpc is working over post - hmmm, i dont think it would
>be needed because most likely there will be no browser talking to it ...
>but can be, if someone is wierd enough to build a webpage using xmlrpc
>instead of rest ;)

True, today you would probably use the rest endpoint and not XMLRPC,
but in AJAX, the X is XML. (FYI the rest endpoint can return XML by
adding a python module and setting the client content headers.)

>in the end it's all just plain http requests ... so i would vote to have
>the CORS thing for all http requests.

Well CORS is used to determine what verbs are valid for an endpoint.
XMLRPC and HTTP/html use a very restricted set of verbs in general.

Hi Marcus:

A followup. It looks like preflight requests will never send
credentials regardless of the credential settings 8-(.

From:

 https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#sending_a_request_with_credentials_included

    CORS-preflight requests must never include credentials. The
    response to a preflight request must specify
    Access-Control-Allow-Credentials: true to indicate that the actual
    request can be made with credentials.

So for now I think allowing anonymous to have "Rest Access"
permissions and establishing a new "CORS Access" permission that
permits OPTIONS with CORS headers is the way to go.

However this opens a can of worms if CORS is applied to all three
services:

  1 rest
  2 xmlrpc
  3 html

Do we need "CORS-rest Access", "CORS-xmlrpc Access" .... permissions
to go along with the "Rest Access", "Web Access", "XMLRPC Access"?

Also should "Rest Access" imply "CORS-Rest Access" so we don't have to
add multiple permissions to a role.

At the moment, I am leaning toward just a "CORS Access" that will work
for any service (rest, xmlrpc, html). It is checked only if the user
doesn't have access to the service (e.g via "Rest Access", "Web
Access"). (This is similar to how roundup already handles search. It
checks for view access to the searched class/property and if not found
looks for search access permissions to the class/property.)

While it is possible to probe for endpoints using a CORS request, I
don't think this a major problem. At the moment the http endpoint
treats an OPTIONS request like a GET request and returns an html page.
The XMLRPC endpoint just hangs waiting for a document/body which will
never come since there is no body/payload in options requests.

Thoughts?

Hi John,

> A followup. It looks like preflight requests will never send 
> credentials regardless of the credential settings 8-(.
[...]
> Thoughts?

cors-preflight is a different story than the actual cors thing.

i am not really sure if we have common sense here, therefore let me
briefly sum it up:

1) when you issue request to a different url from where the page
actually was loaded in javascript (== CORS), then

2) the browser automatically sends a cors-preflight request right before
your actual request to query the server what he is allowed to do.

this request has no auth info as it's just there to query the server how
to continue, if cors actually is allowed, what headers are allowed, is
sending credentials allowed or not, etc.

3) the answer includes allowed headers, if sending credentials is
allowed, what methods are allowed, etc.

it is therefore an anonymous, public and general information.

this is covered by the first part of the patch.

of course we could limit access to certain "Origin"'s to limit who
acutally is allowed to do cors at all.

4) with this information the browser then crafts the actual cors request
and sends it to the server (including auth cookies - if allowed, uses
only allowed headers, etc).

5) the answer then again needs to have a couple of headers set to inform
the browser that it is still allowed to continue (that covers the second
part of the patch)

therefore you actually have two requests in this case.

so, the question is, where to put the logic and what actually gets sent
in 3)?

letting this all come down through the permission system to the actual
OPTIONS method you mentioned in an earlier post, maybe is not that
useful (and a bit of an overkill) as you basically would have the
same answer for almost any endpoint.

except the allowed methods for that endpoint.

but i think that e.g. when 3) returns that PATCH is allowed and the
browser then issue a PATCH on an item where PATCH actually is *not*
allowed, the browser gets back the error anyway ...

i think we can live with this as it would make things easier.

so for me it would be enough to have a config option "ALLOW_CORS" which
enables/disables the answer to the preflight request (first part of the
patch - 3) form above)

optionally maybe also configurable / default:

   Access-Control-Max-Age / 86400
   Access-Control-Allow-Methods / OPTIONS, GET, POST, PUT, DELETE, PATCH
   Access-Control-Allow-Credentials / true

and maybe some sort of configuring what Origins are allowed.

"*" meaning that the requests Origin header just gets copied (as in the
patch) and some kind of check if it's not "*", maybe regex on the Origin
header ?

the second part of the patch needs to be applied anyway as the browser
dont accept Access-Control-Allow-Credentials: true without
Access-Control-Allow-Origin being set to the Origin of the request (as
you already found out).

And you actually need credentials when you want to do something user
specific with the rest api (as in my case).

... just my thoughts ...

regards,
marcus.

Hi Marcus:

In message <42c5af33-fdde-e0c4-96f7-b4908a4bd174@priesch.co.at>,
Marcus Priesch writes:
>> A followup. It looks like preflight requests will never send 
>> credentials regardless of the credential settings 8-(.
>[...]
>> Thoughts?
>
>cors-preflight is a different story than the actual cors thing.
>
>i am not really sure if we have common sense here,

I agree. Thanks for the summary.

>briefly sum it up:

>1) when you issue request to a different url from where the page
>actually was loaded in javascript (== CORS), then

Just to clarify it's not a different url it's a different origin right?
So serving up data from:

   https://example.com/roundup

would be ok from:

   https://example.com/myapp

but would not be ok from

   https://example.com:8080/myapp

since that is a different origin than example.com. (Also I think
changing to http from https might also change the origin??)

>2) the browser automatically sends a cors-preflight request right before
>your actual request to query the server what he is allowed to do.

For anything that is not a simple request. So a GET doesn't get
preflighted, right?

>this request has no auth info as it's just there to query the server how
>to continue, if cors actually is allowed, what headers are allowed, is
>sending credentials allowed or not, etc.
>
>3) the answer includes allowed headers, if sending credentials is
>allowed, what methods are allowed, etc.
>
>it is therefore an anonymous, public and general information.

Got it.

>this is covered by the first part of the patch.
>
>of course we could limit access to certain "Origin"'s to limit who
>acutally is allowed to do cors at all.

The REST interface can be programmed (see
https://www.roundup-tracker.org/docs/rest.html#programming-the-rest-api).
Using interfaces.py filtering the origin can be added on a per tracker
basis by overriding/replacing the existing handler in rest.py then
calling it from the override. It's a bit tricky and more fragile than
I would like but I think it can be done.

I agree it would be nice to have a way to set an Origin_Filter in
rest.py similar to how Client.Cache_Control can be set.

>4) with this information the browser then crafts the actual cors request
>and sends it to the server (including auth cookies - if allowed, uses
>only allowed headers, etc).
>
>5) the answer then again needs to have a couple of headers set to inform
>the browser that it is still allowed to continue (that covers the second
>part of the patch)
>
>therefore you actually have two requests in this case.
>
>so, the question is, where to put the logic and what actually gets sent
>in 3)?
>
>letting this all come down through the permission system to the actual
>OPTIONS method you mentioned in an earlier post, maybe is not that
>useful (and a bit of an overkill) as you basically would have the
>same answer for almost any endpoint.
>
>except the allowed methods for that endpoint.

Possibly. The question is what if my endpoint is invalid?  Do I return
a valid 204 preflight for:

  https:/example.com/roundup/rest/data/iss

where there is no iss class? I am not sure what the proper failure
mode is here, but it seems that we should return a 404 during
preflight.

By pushing it down into the rest library, you get sanity checking on
the URL. Plus as I mentioned above, a roundup-admin can add additional
checks on the preflight (e.g. origin, maybe the client IP) using an
existing documented mechanism (even if it is less streamlined than I
would like). Plus we supply the client/user with the correct methods.

>i think we can live with this as it would make things easier.

But easier for who? The roundup maintainers or the people using the
rest interface? I would prefer to make it easier for the users. At the
moment you and one other person are the only users I know of.  The
openapi_doc decorators were requested by him/her.

>so for me it would be enough to have a config option "ALLOW_CORS" which
>enables/disables the answer to the preflight request (first part of the
>patch - 3) form above)

Actually we don't really need it. REST is unusable from a third party
without anonymous CORS-preflight access. After your explanation I
can't see a reason to implement this. If a developer wants to
implement it they can by programming the OPTIONS endpoints in the REST
interface.

>optionally maybe also configurable / default:
>
>   Access-Control-Max-Age / 86400
>   Access-Control-Allow-Methods / OPTIONS, GET, POST, PUT, DELETE, PATCH
>   Access-Control-Allow-Credentials / true
>
>and maybe some sort of configuring what Origins are allowed.

All of these can be done by overriding the options endpoint as
documented above. If enough people clamor for this capability we can
expose it via a configuration item or via interfaces.py.

>the second part of the patch needs to be applied anyway as the browser
>dont accept Access-Control-Allow-Credentials: true without
>Access-Control-Allow-Origin being set to the Origin of the request (as
>you already found out).

Got it.

>... just my thoughts ...

Good thoughts.

I have attached my current patch that I claim should work. Can you
apply it and see if it works.

Also do you have a test case that I can use to test this? I think a
single HTML file with a couple of inputs (for username/password) along
with an input for a URL that has javascript that goes and pulls data
from the URL. Then I can load it in my browser and test things. I am
currently using curl and visually inspecting what is returned.

Thanks.

Hi Marcus:

I added documentation on CORS preflight and CORS to doc/rest.txt in
changeset:   6674:ff8845ca305e. Can you take  look at it and see where it needs 
correction/improvement.

Thanks.

-- rouilj

I finally got a coffee and read through: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS

my original patch was broken. I tried to be to clever and not send headers that are needed
only by preflight but not CORS. I removed too many headers. The Fixed1 file should
be the one you try to use.

Two other notes:

I just added a Vary: Origin header to the rest responses since the existing OPTIONS
handlers can be wrapped to implement origin filtering and return different info based
on origin. 

CSRF Origin checks currently have to be turned off for CORS REST calls.
This check either needs to be:

  driven off a prefix table that can be set in config.ini or by interfaces.py.
  If the origin sent by the client is matched by a prefix:
      ['https://foo.bar/', 'https://example.com', 'http://example.com']
  in the table then the origin check passes. Right now the prefix is the tracker url.

  is re-implemented as a method on the client object that can be wrapped/overridden in
  interfaces.py.

I am leaning toward the prefix table from config.ini myself as I think it can be used to 
implement origin filtering for preflight requests in the core code.

An open question is what to return if an origin check fails in preflight or in cors.
* return some 400 value
* remove Access-Control-Allow-Origin header with no other change. This will fail a
  prefetch and prevent the data returned from being accessible to JavaScript IIUC.

Hi John,

sorry, but i was off the last week ...

> I added documentation on CORS preflight and CORS to doc/rest.txt in 
> changeset:   6674:ff8845ca305e. Can you take  look at it and see
> where it needs correction/improvement.

looks good, maybe i would add some information that this is done by the
browser automatically and transparently for the user, if the preflight
request fails your ajax request also fails.

> An open question is what to return if an origin check fails in preflight or in cors.
> * return some 400 value
> * remove Access-Control-Allow-Origin header with no other change. This will fail a
>    prefetch and prevent the data returned from being accessible to JavaScript IIUC.

i would vote for 403 (maybe 404 to not reveal too much)

and btw: how is the status now with the support for preflight requests ? 
what can i do ? - the patch you provided in an earlier post was rejected 
by you ... is there anything i can test or help ?

regards,
marcus.

Hi Marcus:

No worries about time off. It's a good idea in general.  I'll update
the docs.

In message <71471f9c-d38e-35a2-0739-835fc127475f@priesch.co.at>,
Marcus Priesch writes:
>> An open question is what to return if an origin check fails in
>> preflight or in cors.
>> * return some 400 value
>> * remove Access-Control-Allow-Origin header with no other change.
>>   This will fail a prefetch and prevent the data returned from being
>>   accessible to JavaScript IIUC.
>
>i would vote for 403 (maybe 404 to not reveal too much)

404 indicates the URL endpoint is bad. The URL is fine it's just the
request that is bad. Some proxy's/caches invalidate data on a 404 (GET
method). I am not sure if a 404 with an OPTIONS method could be cached
or cause side effects.

Per

https://stackoverflow.com/questions/33739107/how-to-handle-an-invalid-cors-preflight-request

tomcat returns a 403. But the MDN docs say this about 403:

  ...Unlike 401 Unauthorized, the client's identity is known to the server. 

We aren't sending credentials/identity for a preflight, so I don't
think 403 is right. 403 is correct if a CORS /rest request occurred
for a user without "Rest Access" permissions. 401 might be valid even
though CORS isn't an authentication/authorization mechanism.

https://stackoverflow.com/questions/14015118/what-is-the-expected-response-to-an-invalid-cors-request

notes the result code is not specified and recommends 403.

https://stackoverflow.com/questions/69427953/cors-cross-origin-resource-sharing-origin-validation-failure

indicates returning 200 without the Access-Control-Allow-Origin header.

>and btw: how is the status now with the support for preflight requests ? 
>what can i do ? - the patch you provided in an earlier post was rejected 
>by you ... is there anything i can test or help ?

When I rejected my previous patch, I added a new patch. Try:

  https://issues.roundup-tracker.org/file1784/Fixed1_Allow_CORS_Preflight_through_rest_handlers.patch

and see if that works for you.

This patch does not add "Origin" to the Vary header. For your
testing purposes it doesn't matter. In real life, the
Access-Control-Allow-Origin could be different depending on the
origin.

Also can you confirm that your are using your patch with
csrf_enforce_header_origin turned off (not yes or required). The
preflight request skips the CSRF check, but with origin checks turned
on it should reject the actual CORS request.

Looks like I will need to allow the user to provide a list of valid
origins and add the tracker origin to the list by default. If * is in
the origin list, I pass any origin. I should do this for xmlrpc and
rest. For the html interface, only the tracker will be valid.

Also can you post the headers from one of your CORS requests? I need
to verify that the checks for Host/X-Forwarded-Host and Referer will
pass. (I have a feeling Referer won't.)

Have a great week.

Marcus, any feedback on my last patch?

I have done a bit more work, and am working on tests. I can upload a new patch
if you want, but my changes don't affect the core functionality in the latest existing patch.

I plan on another release of Roundup on 13 July, so I will be making a beta in a
week or two. Ideally these changes would be in the beta so we can release them
as part of 2.2.0.

Updating to newest patch with test case changes. Marcus if you haven't tested already with the 
first patch, use this one. If you have tested with the first patch let me know if
you need any fixes.

The one change here is a 400 return status if the valid origin check fails. Since 401/403 are
authentication/authorization issues they didn't seem right for an origin check failure.
(Arguably, the origin check failure is an authorization issue for the request not the user
making the request but 403 seems user based. But this may be my misunderstanding.)

Interesting that Marcus' email doesn't show up in the web interface.
I did receive it. Ralf did you get it as well?

Forwarding the body to see if this causes an error of some sort.

Also Marcus sorry you were out of sick leave. Hope you are feeling
better. Thanks for the confirmation that this works. Did you use my
second patch (Fixed2) or the original(Fixed1)?

   ====
Content-Transfer-Encoding: quoted-printable
Subject: [issue2551203] Add support for CORS preflight request
From: Marcus Priesch <issues@roundup-tracker.org>
Date: Tue, 07 Jun 2022 09:56:18 +0000

Marcus Priesch added the comment:

Hi John,

sorry for not answering for that long, but i was off for sick-leave :(

Thanks for the patch, i have tested it and it works !

unsetting allowed_api_origins leads to a 400, setting it to * or
http://localhost:8081 (where the frontend server is running) works :)

so i think for this we can apply this patch and go ahead ;)

btw: as per your request, here are the headers from the preflight=20
request as seen from devtools:

request:

OPTIONS=20
/t/rest/data/time_activity?@sort=3Dname&@verbose=3D0&@fields=3Dname,id,desc=
ription,travel&is_valid=3D1=20
HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=3D0.9,de;q=3D0.8
Access-Control-Request-Headers: content-type,x-requested-with
Access-Control-Request-Method: GET
Cache-Control: no-cache
Connection: keep-alive
Host: localhost:8080
Origin: http://localhost:8081
Pragma: no-cache
Referer: http://localhost:8081/
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-site
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML,=20
like Gecko) Chrome/101.0.4951.64 Safari/537.36
=EF=BF=BC
answer:

HTTP/1.1 204 No Content
Server: BaseHTTP/0.3 Python/2.7.18
Date: Tue, 07 Jun 2022 09:49:24 GMT
Access-Control-Allow-Methods: OPTIONS, GET, POST
Access-Control-Max-Age: 86400
Vary: Origin, Origin
Allow: OPTIONS, GET, POST
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: http://localhost:8081
Access-Control-Allow-Headers: Content-Type, Authorization,=20
X-Requested-With, X-HTTP-Method-Override

thanks for the nice work and good support !

regards,
marcus.

------- End of Forwarded Message

Hmm my last message made it onto the ticket as msg7559, but msg7558
doesn't exist. msg7557 does exist. I'll try reaching out to some of
the PSF folks (ezio M). Never seen a failure like this before.

-- rouilj

> Interesting that Marcus' email doesn't show up in the web interface.
> I did receive it. Ralf did you get it as well?

i got a submission failure email:

There was a problem with the message you sent:
    'ascii' codec can't decode byte 0xef in position 1451: ordinal not 
in range(128)

there were some non-ascii characters in the body of the mail left over 
from copying the text from the browsers devtools page.

interesting that there was an failure submission, nosy mail, but the 
message was not stored in the db ... looks like roundup bug ;)

i have the original mail and the failure submission still at hand - if 
you need if for some tests ...

> Also Marcus sorry you were out of sick leave. Hope you are feeling
> better. Thanks for the confirmation that this works. Did you use my
> second patch (Fixed2) or the original(Fixed1)?

i used the Fixed2 !

regards,
marcus.

Hi Marcus:

In message <d80c754a-77da-9827-66f5-7e337eb6af1e@priesch.co.at>,
Marcus Priesch writes:
>Marcus Priesch added the comment:
>
>> Interesting that Marcus' email doesn't show up in the web interface.
>> I did receive it. Ralf did you get it as well?
>
>i got a submission failure email:
>
>There was a problem with the message you sent:
>    'ascii' codec can't decode byte 0xef in position 1451: ordinal not 
>in range(128)
>
>there were some non-ascii characters in the body of the mail

Probably smart quotes. A message is committed in 2 phases. The first
phase creates a msgXXXX.tmp file. Then renames it to msgXXXX.
The .tmp file for your message was still around.

Also we are running the 1.6.1 version of roundup for the tracker.  It
is unlikely to get upgraded unless we put together our own server and
handle mail ourselves etc.

>interesting that there was an failure submission, nosy mail, but the 
>message was not stored in the db ... looks like roundup bug ;)

IIRC that's an old bug:

 https://issues.roundup-tracker.org/issue2550740 
 https://issues.roundup-tracker.org/issue2550540

are probably related. They are from before I had taken over roundup
maintenance. I have no access to the original reporters anymore and
the patches don't make any sense to me. Also they are python 2
patches.  The changes to make Roundup handle python 2 and 3 mean the
patches have to reverse engineered. Internationalization, character
sets etc. are not my strong suit.

>i have the original mail and the failure submission still at hand - if 
>you need if for some tests ...

Can you open a new issue addnig the original mail and failure message
as attachments. I'll try running it through the gateway on my test
system to see if I can get it to break.

It may be too late to get it in 2.2.0 but.....

>> [...] Did you use my second patch (Fixed2) or the original(Fixed1)?
>
>i used the Fixed2 !

Good.

I committed it:

  changeset:   6693:9a1f5e496e6c

it passed CI for all pythons except 3.11-dev. However it's failing
ther with an disk i/o error in the sqlite tests. So I claim it's a
platform and not a roundup issue.

I think roundup needs to get off travis-ci.

codecov is being a pain but it looks like I have 100% coverage for
changed lines so there's that.