NERC-CEH / dunes-app

📱Dynamic Dunescapes mobile application
Other
0 stars 0 forks source link

Fix CORS for warehouse uploaded media images #37

Closed kazlauskis closed 4 years ago

kazlauskis commented 4 years ago

Something has changed lately so that the app has started getting CORS issues when downloading images from the warehouse directly. For example, try in the browser console:

fetch('https://warehouse1.indicia.org.uk/upload/o_1eekka9rhhmbl9d1aub1s55f0ob.jpg')

I think the Access-Control-Allow-Origin: * is missing from the response.

@BirenRathod @johnvanbreda

johnvanbreda commented 4 years ago

Presumably this relates to the fact that Biren reconfigured CORS on the warehouse due to the API being enabled and this header ending up being duplicated? This link is direct file access so any headers have to be controlled by IIS not PHP.

@kazlauskis do you know if it is normal practice for the code in a REST API to add CORS headers? Or should this be handled in the webserver config?

kazlauskis commented 4 years ago

To be honest, I am not sure what's the best practice. For some projects where we did Node API services, code was the only place we could have this functionality. For some others we had like an API gateway so some stuff went there.

BirenRathod commented 4 years ago

Yes, you are right @johnvanbreda. That has caused this. The CORS setting has to be done at server level. The setting will help to use with other services as well for e.g. to access Geoserver map layer. I have enabled that again. My understanding is that it should be always at server level and it should not be request from client separately unless server has got no access to do that.

johnvanbreda commented 4 years ago

Ok, so I think the answer here is to make adding a CORS header an option in the REST API config, so we can disable it when CORS is handled by IIS. @BirenRathod are you able to confirm exactly what CORS headers are added by IIS when this is enabled?

kazlauskis commented 4 years ago

At the moment, we are back to double OPTIONS header issue. @BirenRathod I wonder if you could only add the headers for certain endpoints, in this case it would enabling it only for the /upload path which falls under IIS domain?

johnvanbreda commented 4 years ago

@BirenRathod @kazlauskis I've realised I'd already coded an option 'allow_cors' in the REST API config, under the JwtUser config section. I've just turned it off for both warehouse1 and dev warehouse which means that CORS should be added via IIS as it was before.

BirenRathod commented 4 years ago

@johnvanbreda at present I have only enabled this "Access-Control-Allow-Origin". Jut to let you know that It is not actually IIS is enabling. I am adding configuration in the web.config file at website level. There is no button or icon in IIs to do that.

johnvanbreda commented 4 years ago

@BirenRathod from my perspective, web.config is an IIS thing ;)

In the code, if allow_cors is enabled, then the following headers get added:

Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, POST, DELETE, PUT, OPTIONS
Access-Control-Allow-Headers: Content-Type, Authorization

@kazlauskis do you think that Biren should add these extra headers?

kazlauskis commented 4 years ago

My feeling is that it is probably the quickest to add it at the IIS-level and tailor the headers to the endpoints there. @BirenRathod could you please add the above headers to all methods requesting /upload/* and /index.php/services/rest/* endpoints?

BirenRathod commented 4 years ago

@johnvanbreda & @kazlauskis I have added the above headers on dev warehouse. See if that is ok. @kazlauskis This is website level setting, so this setting will apply to whole website regardless of individual folder and when I mentioned website level means I can do settings for each website but at the same time I can do that for whole server for all websites.

kazlauskis commented 4 years ago

Thanks, I am a bit surprised that the header rules can't target specific url paths with a regex or something.

BirenRathod commented 4 years ago

@kazlauskis That is how it works. here is doc to help you understand more. https://docs.microsoft.com/en-us/iis/extensions/cors-module/cors-module-configuration-reference#cors-configuration

kazlauskis commented 4 years ago

In the dev site, OPTIONS return the headers but not the other methods, could you add it to the others too?

BirenRathod commented 4 years ago

@kazlauskis I can see all methods. I have already added. see below.

image

kazlauskis commented 4 years ago

For which method and endpoint is this?

BirenRathod commented 4 years ago

@kazlauskis As I mentioned above there no endpoint setup , so works at website level. I got this by requesting OPTIONS method.

kazlauskis commented 4 years ago

Thanks, OPTIONS return the headers but not the other methods, could you add it to the others too? - Do GET, POST, PUT, DELETE as well.

BirenRathod commented 4 years ago

@kazlauskis It added already. All methods are added on live and dev warehouses.

kazlauskis commented 4 years ago

Try this command, it prints out the headers (I'll send the token separately):

curl -L -v -s -o --location --request GET 'https://devwarehouse.indicia.org.uk/index.php/services/rest/reports/library/months/filterable_species_counts.xml' \
--header 'Authorization: Bearer TOKEN'
BirenRathod commented 4 years ago

@kazlauskis I have attached the response.

response.zip

kazlauskis commented 4 years ago

Sorry, but the contents of the zipped file are: [object Object]

BirenRathod commented 4 years ago

Sorry @kazlauskis. That was the different file. Here it is. response.zip

kazlauskis commented 4 years ago

Yes, I am also getting this response body but the response headers don't include the CORS related ones.

BirenRathod commented 4 years ago

that is how server responses.

kazlauskis commented 4 years ago

Yes, we are talking about the headers here. The server message (aka response) consists of a body and headers. In our case here, the headers are missing the CORS related information. The body that you have attached it here is fine but the browser cannot process the body unless the CORS headers are also present. I suggest to read this introduction on the HTTP messages.

The curl request I have posted earlier prints out only the headers and at the moment it gets:

< HTTP/1.1 200 OK
< Content-Type: application/json
< Server: Microsoft-IIS/8.5
< Date: Tue, 22 Sep 2020 14:40:01 GMT
< Content-Length: 697

Could you please run the curl command in your terminal and see what what headers you're getting. I can see you are using the Postman, in your screenshot previously it also listed the headers so you can use that as well.

BirenRathod commented 4 years ago

@kazlauskis My understanding is because you are getting this from xml file, browser will not send you headers.

Also, on the other hand why do you need headers? if you get results with 200 status, it should be fine.

kazlauskis commented 4 years ago

Yes, the headers matter. Browsers can reject responses from the server if the CORS headers are not present when doing cross-domain requests.

The xml file extension is irrelevant to this, GET, POST, PUT, DELETE for any endpoints don't get the headers. At the moment, it looks like only the OPTIONS does.

Can you please enable it for GET, POST, PUT, DELETE methods as well?

BirenRathod commented 4 years ago

@kazlauskis Thanks for info. As mentioned above it enabled already. So what more can I do?

kazlauskis commented 4 years ago

Thanks, unfortunately, it is not working for all the methods that we require. I don't have the access to the IIS to help you but maybe some of your colleagues (@burkmarr?) would be able to help you with this?

BirenRathod commented 4 years ago

@kazlauskis If it is not working something is missing either your end or my end. Why do you thing @burkmar can able to help me on this?

kazlauskis commented 4 years ago

I have tried the requests multiple times now to see if I am missing something but it is clear the warehouse is not returning the required headers. Would it help if I called you? It might be more efficient than the chat.

BirenRathod commented 4 years ago

@kazlauskis you mentioned only OPTIONS does then that should be same for all, so it should return with all methods.

kazlauskis commented 4 years ago

Can you confirm it returns the headers for other methods your end?

BirenRathod commented 4 years ago

Hello @johnvanbreda @kazlauskis has made below request. curl -L -v -s -o --location --request GET 'https://devwarehouse.indicia.org.uk/index.php/services/rest/reports/library/months/filterable_species_counts.xml' \ --header 'Authorization: Bearer TOKEN'

and he got status code 200 and results in JSON. I have attached the result here. response.zip

Is it correct behavior or not?

kazlauskis commented 4 years ago

This is the body you have attached not the headers. Maybe this command works differently on Windows? Try to find a command that gets you the headers only.

BirenRathod commented 4 years ago

@kazlauskis I made another request and I got same results. This time report file is in different folder. curl -L -v -s -o --location --request GET 'https://devwarehouse.indicia.org.uk/index.php/services/rest/reports/projects/dunescapes/samples_list.xml'

My understanding is, this is how XML reports response.

kazlauskis commented 4 years ago

Can you confirm you get the CORS headers?

BirenRathod commented 4 years ago

No. same like other one.

BirenRathod commented 4 years ago

I think CORS headers is trivial if server deal with XML file.

kazlauskis commented 4 years ago

OK, then it means it is not working yet. Try to play around with the IIS config until you find the headers appearing correct. If you needed help with it then please send me a zoom link and I will try to help you direct.

BirenRathod commented 4 years ago

There is nothing to config in IIS. As per this doc https://docs.microsoft.com/en-us/iis/extensions/cors-module/cors-module-configuration-reference#cors-configuration, I already done it.

kazlauskis commented 4 years ago

Can I call you somehow?

johnvanbreda commented 4 years ago

@BirenRathod yes the response you posted above is the correct expected body from the request.

BirenRathod commented 4 years ago

Thanks @johnvanbreda.

@kazlauskis, any thoughts?

@johnvanbreda Do you get from above @kazlauskis's query?

BirenRathod commented 4 years ago

@kazlauskis can you see now?

BirenRathod commented 4 years ago

@kazlauskis you can call me now, if you want.

kazlauskis commented 4 years ago

OK, how can I reach you?

BirenRathod commented 4 years ago

@kazlauskis I sent you zoom invitation on your email address.

BirenRathod commented 4 years ago

@johnvanbreda Is it possible to add below settings in php of rest api? Or if you can tell me, I can add it. header("Access-Control-Allow-Origin: *"); header('Access-Control-Allow-Methods: GET, POST, DELETE, PUT, OPTIONS'); header("Access-Control-Allow-Headers: Content-Type, Authorization");

BirenRathod commented 4 years ago

@johnvanbreda ignore my above request.

@kazlauskis I fixed this issue now. Check this on devwarehouse first. If you happy with that let me know and I will implement same for the live warehouse.