NEUBIAS / bise

BIII development repository
http://biii.eu
GNU General Public License v2.0
5 stars 14 forks source link

Rest export for Comulis to be corrected #209

Open PerrineGilloteaux opened 1 year ago

PerrineGilloteaux commented 1 year ago

Following the drupal 9 update, the Rest export view of serach need to be checked since https://www.comulis.eu/correlation-software#search_result will return Failed retrieving data from biii! 0 -

See also #126

PerrineGilloteaux commented 1 year ago

It is actually the return of the CORS error #113 . ps://biii.eu/all-content-rest?type=software&field_has_topic_target_id_1%5B%5D=4774&_format=json&source=COMULIS' from origin 'https://www.comulis.eu' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

https://drupal.stackexchange.com/questions/312414/cors-policy-no-access-control-allow-origin

PerrineGilloteaux commented 1 year ago

Access-Control-Allow-Origin * in .htacess -> to solve the CORS problem (tested on prod side and revert back) (and clear cache)

RewriteEngine On
RewriteRule ^$ /web [L]
Header set Access-Control-Allow-Origin "*"

This will change the comulis website to a 500 error instead of 1 (no cors error anymore)

IMPORTANT This change can not be committed it will cause an internal error on the environment dev BUT in does work on the production site. Note that is it not the case right now)

The second part of the modification is the following (the REST below page still look the same as it was however) GET https://biii.eu/all-content-rest?type=software&field_has_topic_target_id_1%5B%5D=4774&_format=json&source=COMULIS 500 ajaxGet @ correlation-software:88 getBiseData @ correlation-software:194 onclick @ VM201 correlation-software:1 correlation-software:88

--> I do not see any difference between config for rest services (get is well activated config/services/rest/resource/entity%3Anode/edit

--> I do not see any difference in the https://biii.eu/all-content-rest?type=software&field_has_topic_target_id_1%5B%5D=4774&_format=json&source=COMULIS link result

@delestro , @joakimlindblad could you have a look please? Maybe I have changed something that you do not support from the comulis website code? (oidgnore the cors error, look at the second part mentionned above)

miura commented 1 year ago

Looks ok now? https://biii.eu/all-content-rest?type=software&field_has_topic_target_id_1%5B%5D=4774&_format=json&source=COMULIS

PerrineGilloteaux commented 1 year ago

Hello, to test you need to go to the comulis software https://www.comulis.eu/correlation-software#search_result but first edit the .htaccess from the server directly ( and add it to .git ignore if it works) to remise the cors error. i think i had no server error this morning ( on prod but i had on gitpod). I will try tomorrow early morning otherwise. We also need to test the fiji search bar which may have the same problem.

miura commented 1 year ago

@PerrineGilloteaux

You probably mean this one in 2019 right?

https://github.com/NEUBIAS/bise/pull/114/files#diff-270939b4fba4be968ab78e23dc0207eb893744491980a25c99a27c809a82ddab

In fact, I have never applied this change as it crashed the whole biii.eu (production). See:

https://github.com/NEUBIAS/bise/pull/114

I guess it's better to set the proper headers for COR as it is explained in the link you found.

https://drupal.stackexchange.com/questions/312414/cors-policy-no-access-control-allow-origin

In biii, service.yml is bise.service.yml. Here:

https://github.com/NEUBIAS/bise/blob/dev/web/sites/default/bise.services.yml

In the allowdHeader array, the first item is missing a single colon. Maybe try fixing this? It's possible that with Drupal9, the parsing has become more strict. Please also check the counterpart service.yml file.

image

miura commented 1 year ago

... so I fixed the typo. Please check the comulis side.

https://github.com/NEUBIAS/bise/commit/1d781dba38ae9b5449f9a05c0fcc17655c861c20

joakimlindblad commented 1 year ago

Currently the COMULIS search does not work due to the current CORS policy of the Biii-site, which does not allow external sites to ask for BIII resources.

@miura , as commented by @PerrineGilloteaux above, to check if it works you just need to go to https://www.comulis.eu/correlation-software#search_result and click any of the three buttons.

When failing, you may also see the following in the browser console (Ctrl-J in Chrome): correlation-software#search_result:1 Access to XMLHttpRequest at 'https://biii.eu/all-content-rest?type=software&field_has_topic_target_id_1%5B%5D=4774&_format=json&source=COMULIS' from origin 'https://www.comulis.eu' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

miura commented 1 year ago

Hi @joakimlindblad

Yes, I do understand the problem as the symptom is similar.

[edit] .htaccess is added temporally with the suggested line Header set Access-Control-Allow-Origin "*"

But the reason probably is not the same (I checked all the external interfaces and should be OK) but more internal.

I now found this error that comes up in relation to the use of php8:

image

This actually happens also in some occasions with clicking internal links. I will try adding the patch below later.

https://www.drupal.org/project/field_group/issues/3311905

joakimlindblad commented 1 year ago

Yes, in deed now the error looks completely different; presumably after you added the mentioned .htaccess line. I will check back once the relevant looking patch is applied.

miura commented 1 year ago

Hi @PerrineGilloteaux , @joakimlindblad

I hacked in a very bad way to stream the search results into Comulis, but this should be soon solved in a different way.

image

DETAILS

To not forget the current temporal solution, more details are below (also for myself).

There are two options for cors.config that are causing the access issue: Access-Control-Allow-Origin and Access-Control-Expose-Headers

Access-Control-Allow-Origin

.htaccess was temporally added with a line Header set Access-Control-Allow-Origin "*", but this became interfering with cors.config in web/sites/default/bise.service.yml and the access from Comunlis becomes blocked because the origins become parsed as multiple sites as an array [comulis.eu, "*"] and this seems to be not allowed. For this reason, I restored the original .htaccess without the Header set

Access-Control-Expose-Headers

There seems to be a problem with the type of the second argument within one of the modules for CORS in Drupal. This version is apparently an old one (v1.1), even though there is a much newer version v2.1.1. The error looks like this and also causes similar issues on some other occasions like when changing some configuration. Here is an example of the error when accessing from comulis.eu:

https://biii.eu/admin/reports/dblog/event/14667917

TypeError: implode(): Argument #2 ($array) must be of type ?array, bool given in implode() (line 94 of /home/biiieudinm/production/vendor/asm89/stack-cors/src/Asm89/Stack/CorsService.php)
#0 /home/biiieudinm/production/vendor/asm89/stack-cors/src/Asm89/Stack/CorsService.php(94): implode(', ', true)
#1 /home/biiieudinm/production/vendor/asm89/stack-cors/src/Asm89/Stack/Cors.php(62): Asm89\Stack\CorsService->addActualRequestHeaders(Object(Drupal\Core\Cache\CacheableResponse), Object(Symfony\Component\HttpFoundation\Request))
#2 /home/biiieudinm/production/web/core/modules/ban/src/BanMiddleware.php(50): Asm89\Stack\Cors->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#3 /home/biiieudinm/production/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\ban\BanMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#4 /home/biiieudinm/production/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#5 /home/biiieudinm/production/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#6 /home/biiieudinm/production/web/core/lib/Drupal/Core/DrupalKernel.php(718): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#7 /home/biiieudinm/production/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#8 {main}

As this is saying, the second argument passed to the function implode is causing type error. To avoid this, I probably have to look for the code of "implode" and think about solving this from the bottom up, but as I am not a PHP programmer I do not have the confidence to fix them all. I also compared the corresponding code in the latest version of asm89/stack-cors, but this did not really help. In the end, I made a brute force reparation by simply replacing implode(', ', $this->options['exposedHeaders']) with 'true'. Apparently, this is ignoring cors.config and hard-coding the option directly in the source file - which should be avoided and replaced with a better solution soon. The repair was done with the file vendor/asm89/stack-cors/src/Asm89/Stack/CorsService.php, commented out line 94, and added line 95.

image

vendor/asm89/stack-cors/src/Asm89/Stack/CorsService.php

//$response->headers->set('Access-Control-Expose-Headers', implode(', ', $this->options['exposedHeaders']));
 $response->headers->set('Access-Control-Expose-Headers', 'true');

Even though this will be very temporal, I will try to create a patch with this change to make it a bit easier for restoring the original code later. Currently, I really hard coded and this is not under the control of the composer package manager nor Git.

I list the related links in below, which lead us to this temporal solution for further fix.

... so please let this issue be opened for a while...

PerrineGilloteaux commented 1 year ago

You are my hero @miura

miura commented 8 months ago

@PerrineGilloteaux @albangaignard So my best bet is that this is now properly fixed with this commit. https://github.com/NEUBIAS/bise/commit/a00d8e3e01b27a065bcc4d182d6af602c1b67aa0

This fix is in the production site already.