scality / Arsenal

Common utilities for the open-source Scality S3 project components
Apache License 2.0
14 stars 19 forks source link

ARSN-387: check for forwarded proto header #2210

Open KazToozs opened 8 months ago

KazToozs commented 8 months ago

This fix is for the aws:secureTransport condition, related to the TSKB.

With load balancers in front, the check for SSL in the request must be done on the x-forwarded-proto header.

Tests have been updated accordingly.

The other necessary change for this condition to work correctly is for the nginx config to properly pass this header:

proxy_set_header X-Forwarded-Proto $scheme;

Edit: Green CS and Vault builds: https://github.com/scality/Vault/pull/2151 https://github.com/scality/cloudserver/pull/5546

bert-e commented 8 months ago

Hello kaztoozs,

My role is to assist you with the merge of this pull request. Please type @bert-e help to get information on this process, or consult the user documentation.

Status report is not available.

bert-e commented 6 months ago

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically create the integration branches.

KazToozs commented 6 months ago

/create_integration_branches

bert-e commented 6 months ago

Integration data created

I have created the integration data for the additional destination branches.

The following branches will NOT be impacted:

You can set option create_pull_requests if you need me to create integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: create_integration_branches

bert-e commented 6 months ago

Waiting for approval

The following approvals are needed before I can proceed with the merge:

The following options are set: create_integration_branches

fredmnl commented 6 months ago

I may be wrong but I'm seeing that the getSslEnabled function has become dead code, it's not called anywhere anymore. We should either look into removing that if it's not needed, or perhaps the checking of the forwarded header should actually be put in that function. I'm not sure which one it is because I'm not sure what I'm looking at with regards to the requestcontext object.