Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rest export for Comulis to be corrected #209

Open
PerrineGilloteaux opened this issue May 17, 2023 · 12 comments
Open

Rest export for Comulis to be corrected #209

PerrineGilloteaux opened this issue May 17, 2023 · 12 comments
Assignees
Labels
bug Something isn't working website dev To be done on css, js, configuration etc.. and git tracked

Comments

@PerrineGilloteaux
Copy link
Member

PerrineGilloteaux commented May 17, 2023

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 PerrineGilloteaux added bug Something isn't working website dev To be done on css, js, configuration etc.. and git tracked labels May 17, 2023
@PerrineGilloteaux PerrineGilloteaux self-assigned this May 17, 2023
@PerrineGilloteaux
Copy link
Member Author

PerrineGilloteaux commented May 17, 2023

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
Copy link
Member Author

PerrineGilloteaux commented May 17, 2023

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
Copy link
Member

miura commented May 17, 2023

@PerrineGilloteaux
Copy link
Member Author

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
Copy link
Member

miura commented May 17, 2023

@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:

#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
Copy link
Member

miura commented May 17, 2023

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

1d781db

@joakimlindblad
Copy link

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
Copy link
Member

miura commented May 23, 2023

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
Copy link

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
Copy link
Member

miura commented May 24, 2023

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
Copy link
Member Author

You are my hero @miura

miura added a commit to miura/bise that referenced this issue Dec 22, 2023
@miura
Copy link
Member

miura commented Dec 25, 2023

@PerrineGilloteaux @albangaignard
So my best bet is that this is now properly fixed with this commit.
a00d8e3

This fix is in the production site already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working website dev To be done on css, js, configuration etc.. and git tracked
Projects
Status: No status
Development

No branches or pull requests

4 participants