Roundup Tracker - Issues

Message7513

Author rouilj
Recipients marcus.priesch, rouilj, schlatterbeck
Date 2022-05-08.23:55:57
Message-id <20220508235552.90A636A0289@pe15.cs.umb.edu>
In-reply-to <42c5af33-fdde-e0c4-96f7-b4908a4bd174@priesch.co.at>
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.
Files
File name Uploaded
Allow_CORS_Preflight_through_rest_handlers.patch rouilj, 2022-05-08.23:55:57
History
Date User Action Args
2022-05-08 23:55:58rouiljsetrecipients: + rouilj, schlatterbeck, marcus.priesch
2022-05-08 23:55:58rouiljlinkissue2551203 messages
2022-05-08 23:55:57rouiljcreate