Moves the CORS header support to the library.
JIRA: FACTORY-4516 JIRA: FACTORY-4524 Signed-off-by: Lukas Holecek hluk@email.cz
Build d7cff59f400c69cf17b47aace50291d93bb4cea7 FAILED! Rebase or make new commits to rebuild.
rebased onto 5bd39e560aea206a7b853f7de90faf01acac4046
14:03:35 error: Failed build dependencies: 14:03:35 python3-flask-cors is needed by waiverdb-1.0.1-0.git.18.d7cff59.fc29.noarch
Failed because Jenkins slave didn't have the right dependencies installed. I've updated Jenkins slave with following command and repushed changes.
oc start-build --from-repo=. --commit=cors waiverdb-premerge-jenkins-slave
Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request?
CORS_SUPPORTS_CREDENTIALS
Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved.
:thumbsup:
looks good
rebased onto 994646673c0bd8b8419940eec33d590cd3b7b123
Had to remove the config CORS_METHODS = ['POST', 'OPTIONS']. Default is all HTTP methods. Otherwise GET about endpoint with authentication doesn't work.
CORS_METHODS = ['POST', 'OPTIONS']
Perhaps you need to add Authorization to the Access-Control-Request-Headers header returned.
Authorization
Access-Control-Request-Headers
Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request? Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved.
The documentation says "Credentials are cookies, authorization headers or TLS client certificates" [1]. So I think it maybe necessary.
1 - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials
Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request? Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved. The documentation says "Credentials are cookies, authorization headers or TLS client certificates" [1]. So I think it maybe necessary. 1 - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials
The documentation says "Credentials are cookies, authorization headers or TLS client certificates" [1]. So I think it maybe necessary. 1 - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials
From library docs:
supports_credentials (bool) – Allows users to make authenticated requests. If true, injects the Access-Control-Allow-Credentials header in responses. This allows cookies and credentials to be submitted across domains.
I don't think we want to send, for example, Bohdi credentials over to WaiverDB. Am I missing something?
Does CORS_SUPPORTS_CREDENTIALS need to be set as well to support the authenticated POST request? Not sure, works locally with mocked authentication using Firefox. I think this option is only required if cookies are involved. The documentation says "Credentials are cookies, authorization headers or TLS client certificates" [1]. So I think it maybe necessary. 1 - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials From library docs: supports_credentials (bool) – Allows users to make authenticated requests. If true, injects the Access-Control-Allow-Credentials header in responses. This allows cookies and credentials to be submitted across domains. I don't think we want to send, for example, Bohdi credentials over to WaiverDB. Am I missing something?
From library docs: supports_credentials (bool) – Allows users to make authenticated requests. If true, injects the Access-Control-Allow-Credentials header in responses. This allows cookies and credentials to be submitted across domains. I don't think we want to send, for example, Bohdi credentials over to WaiverDB. Am I missing something?
You're right. I misunderstood the documentation
Had to remove the config CORS_METHODS = ['POST', 'OPTIONS']. Default is all HTTP methods. Otherwise GET about endpoint with authentication doesn't work. Perhaps you need to add Authorization to the Access-Control-Request-Headers header returned.
Access-Control-Request-Headers header is set to * by flask-cors.
*
Adding 'GET' to CORS_METHODS works. GET requests in waiverdb doesn't require authentication but it's probably better not to block them when they contain Authorization header.
CORS_METHODS
Every comment seems to be addressed. Merging the PR.
Commit 093d9466 fixes this pull-request
Pull-Request has been merged by gnaponie
Moves the CORS header support to the library.
JIRA: FACTORY-4516
JIRA: FACTORY-4524
Signed-off-by: Lukas Holecek hluk@email.cz