[full-ci] adapt cors headers #8518
Conversation
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
|
@saw-jan || @phil-davis || @individual-it can you take a look at the failing cors tests. I have adapted the default config to match #8514 but I am unsure about the expectations for the suite. |
Yeah, we will have a look on it. Thanks for mentioning 👍 |
dragonchaser
force-pushed
the
cors-issues
branch
from
06f3820 to
a040189
Compare
| if cfg.HTTP.CORS.AllowedOrigins == nil && cfg.Commons != nil && cfg.Commons.OcisURL != "" || | ||
| len(cfg.HTTP.CORS.AllowedOrigins) == 1 && cfg.HTTP.CORS.AllowedOrigins[0] == "*" { | ||
| cfg.HTTP.CORS.AllowedOrigins = []string{cfg.Commons.OcisURL} | ||
| } |
There was a problem hiding this comment.
@dragonchaser another question. if we run ocis with OCIS_URL="https://ocis:9200", should cfg.HTTP.CORS.AllowedOrigins have ["https://ocis:9200"]
There was a problem hiding this comment.
That the default config for the suite contains the address of the ocis-server step from the ci.
There was a problem hiding this comment.
What I observed:
- ocis with
OCIS_URL=https://localhost:9200curl "https://:localhost9200/ocs/v1.php/config" -H"Origin: https://localhost:9200" -vk
< Access-Control-Allow-Origin: https://localhost:9200 < Access-Control-Expose-Headers: Location < Content-Length: 266
- ocis with
OCIS_URL=https://host.docker.internal:9200(no cors headers ❓)curl "https://host.docker.internal:9200/ocs/v1.php/config" -H"Origin: https://host.docker.internal:9200" -vk
< Content-Length: 266
There was a problem hiding this comment.
Remaining failing API test is affected by this behavior: https://drone.owncloud.com/owncloud/ocis/32398/14/5
Other webUI tests might be the same issue
There was a problem hiding this comment.
@saw-jan what is needed to do to fix them?
There was a problem hiding this comment.
what is needed to do to fix them?
For the API test, I am expecting that when using OCIS_URL=https://host.docker.internal:9200 the following request should give the cors headers
curl "https://host.docker.internal:9200/ocs/v1.php/config" -H"Origin: https://host.docker.internal:9200" -vk< Access-Control-Allow-Origin: https://localhost:9200
< Access-Control-Expose-Headers: Location
| @@ -31,7 +31,7 @@ func DefaultConfig() *config.Config { | |||
| Protocol: "tcp", | |||
| Prefix: "", | |||
| CORS: config.CORS{ | |||
| AllowedOrigins: []string{"*"}, | |||
| AllowedOrigins: []string{"https://localhost:9200"}, | |||
There was a problem hiding this comment.
why not use the shared config value for the ocis url as a default?
There was a problem hiding this comment.
Because it was also hardcoded in line 82
dragonchaser
force-pushed
the
cors-issues
branch
from
f3c126b to
0d52f89
Compare
butonic
force-pushed
the
cors-issues
branch
2 times, most recently
from
5e444c0 to
dc64e2c
Compare
dragonchaser
force-pushed
the
cors-issues
branch
2 times, most recently
from
bf17602 to
cae0613
Compare
Signed-off-by: Christian Richter <[email protected]> Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Christian Richter <[email protected]>
dragonchaser
force-pushed
the
cors-issues
branch
from
cae0613 to
cf5558d
Compare
Signed-off-by: Christian Richter <[email protected]>
Signed-off-by: Christian Richter <[email protected]>
dragonchaser
force-pushed
the
cors-issues
branch
from
2411c3b to
8a19239
Compare
Signed-off-by: Christian Richter <[email protected]>
|
Most of the tests are green. The failing tests are the ones that require adjustment. I can make a push with the test changes |
dragonchaser
force-pushed
the
cors-issues
branch
3 times, most recently
from
22e232e to
419ff64
Compare
Signed-off-by: Christian Richter <[email protected]>
dragonchaser
force-pushed
the
cors-issues
branch
from
419ff64 to
6c0156c
Compare
|
[full-ci] adapt cors headers
Enhencement: Change Cors default settings
We have changed the default CORS settings to set
Access-Control-Allow-Originto theOCIS_URLif not explicitely setand
Access-Control-Allow-Credentialstofalseif not explicitely set.refs #8514