s3proxy icon indicating copy to clipboard operation
s3proxy copied to clipboard
verified publisher icon gaul

add Access-Control-Allow-Credentials header

Open st-h opened this issue 3 years ago • 2 comments

fixes #415

st-h avatar Apr 18 '22 12:04 st-h

@reimannf could you review this?

gaul avatar Apr 18 '22 12:04 gaul

I think CORS does not include cookies/credentials on cross-origin requests by default. Both, client and server has to opt-in if they want to do this. So instead of always sending that header, which might open some attacking space, I suggest to introduce a config property PROPERTY_CORS_ALLOW_CREDENTIAL here: https://github.com/gaul/s3proxy/blob/217308abd735c5f6e31ef4cf8351d5b287a5dc03/src/main/java/org/gaul/s3proxy/S3ProxyConstants.java#L41-L42

You might want to send the header always with the value configured in PROPERTY_CORS_ALLOW_CREDENTIAL unless you default to something different than true and make this an active decision.

reimannf avatar Apr 20 '22 10:04 reimannf

@st-h Could you address the review comment so I can merge this?

gaul avatar Sep 26 '22 07:09 gaul

@gaul thanks for the reminder. I totally forgot this PR was still open and I was chugging along with my local docker image. Would have been up for a surprise when switching to a different computer. I just added the requested property.

st-h avatar Sep 27 '22 07:09 st-h

@gaul Any chance we can still merge and release this?

st-h avatar Jun 23 '23 12:06 st-h

Thank you for your contribution and patience @st-h!

gaul avatar Sep 26 '23 11:09 gaul