Make exposed CORS headers configurable (#1322)

* Prevent losing headers (like CORS related ones) when resetting the response headers due to artifact stream.

* add comment

* Extend CORS tests to verify provided header.
This commit is contained in:
Michael Herdt
2023-02-23 14:16:42 +01:00
committed by GitHub
parent 06dca2d5d2
commit 06fc4fb6d2
5 changed files with 55 additions and 18 deletions

View File

@@ -591,6 +591,7 @@ public class SecurityManagedConfiguration {
corsConfiguration.setAllowCredentials(true); corsConfiguration.setAllowCredentials(true);
corsConfiguration.setAllowedHeaders(securityProperties.getCors().getAllowedHeaders()); corsConfiguration.setAllowedHeaders(securityProperties.getCors().getAllowedHeaders());
corsConfiguration.setAllowedMethods(securityProperties.getCors().getAllowedMethods()); corsConfiguration.setAllowedMethods(securityProperties.getCors().getAllowedMethods());
corsConfiguration.setExposedHeaders(securityProperties.getCors().getExposedHeaders());
return corsConfiguration; return corsConfiguration;
} }

View File

@@ -40,14 +40,12 @@ import org.eclipse.hawkbit.repository.builder.TargetUpdate;
import org.eclipse.hawkbit.repository.event.remote.TargetAttributesRequestedEvent; import org.eclipse.hawkbit.repository.event.remote.TargetAttributesRequestedEvent;
import org.eclipse.hawkbit.repository.event.remote.TargetDeletedEvent; import org.eclipse.hawkbit.repository.event.remote.TargetDeletedEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.TargetUpdatedEvent; import org.eclipse.hawkbit.repository.event.remote.entity.TargetUpdatedEvent;
import org.eclipse.hawkbit.repository.exception.AutoConfirmationAlreadyActiveException;
import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException; import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException;
import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException;
import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetCreate; import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetCreate;
import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetUpdate; import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetUpdate;
import org.eclipse.hawkbit.repository.jpa.configuration.Constants; import org.eclipse.hawkbit.repository.jpa.configuration.Constants;
import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor; import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor;
import org.eclipse.hawkbit.repository.jpa.model.JpaAutoConfirmationStatus;
import org.eclipse.hawkbit.repository.jpa.model.JpaTarget; import org.eclipse.hawkbit.repository.jpa.model.JpaTarget;
import org.eclipse.hawkbit.repository.jpa.model.JpaTargetMetadata; import org.eclipse.hawkbit.repository.jpa.model.JpaTargetMetadata;
import org.eclipse.hawkbit.repository.jpa.model.JpaTargetMetadata_; import org.eclipse.hawkbit.repository.jpa.model.JpaTargetMetadata_;
@@ -59,7 +57,6 @@ import org.eclipse.hawkbit.repository.jpa.rsql.RSQLUtility;
import org.eclipse.hawkbit.repository.jpa.specifications.SpecificationsBuilder; import org.eclipse.hawkbit.repository.jpa.specifications.SpecificationsBuilder;
import org.eclipse.hawkbit.repository.jpa.specifications.TargetSpecifications; import org.eclipse.hawkbit.repository.jpa.specifications.TargetSpecifications;
import org.eclipse.hawkbit.repository.jpa.utils.QuotaHelper; import org.eclipse.hawkbit.repository.jpa.utils.QuotaHelper;
import org.eclipse.hawkbit.repository.model.AutoConfirmationStatus;
import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.DistributionSet;
import org.eclipse.hawkbit.repository.model.DistributionSetType; import org.eclipse.hawkbit.repository.model.DistributionSetType;
import org.eclipse.hawkbit.repository.model.MetaData; import org.eclipse.hawkbit.repository.model.MetaData;

View File

@@ -14,7 +14,9 @@ import java.io.OutputStream;
import java.math.RoundingMode; import java.math.RoundingMode;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map;
import javax.servlet.ServletOutputStream; import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
@@ -127,7 +129,8 @@ public final class FileStreamingUtil {
final String etag = artifact.getHashes().getSha1(); final String etag = artifact.getHashes().getSha1();
final long length = artifact.getSize(); final long length = artifact.getSize();
response.reset(); resetResponseExceptHeaders(response);
response.setHeader(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=" + filename); response.setHeader(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=" + filename);
response.setHeader(HttpHeaders.ETAG, etag); response.setHeader(HttpHeaders.ETAG, etag);
response.setHeader(HttpHeaders.ACCEPT_RANGES, "bytes"); response.setHeader(HttpHeaders.ACCEPT_RANGES, "bytes");
@@ -189,6 +192,19 @@ public final class FileStreamingUtil {
return result; return result;
} }
private static void resetResponseExceptHeaders(final HttpServletResponse response){
// do backup the current headers (like CORS related)
final Map<String, String> storedHeaders = new HashMap<>();
for (final String header : response.getHeaderNames()) {
storedHeaders.put(header, response.getHeader(header));
}
// resetting the response is needed only partially. Headers set before e.b. by
// the CORS security config needs to be persisted.
response.reset();
// restore headers again
storedHeaders.forEach(response::addHeader);
}
private static ResponseEntity<InputStream> handleFullFileRequest(final DbArtifact artifact, final String filename, private static ResponseEntity<InputStream> handleFullFileRequest(final DbArtifact artifact, final String filename,
final HttpServletResponse response, final FileStreamingProgressListener progressListener, final HttpServletResponse response, final FileStreamingProgressListener progressListener,
final ByteRange full) { final ByteRange full) {
@@ -196,7 +212,7 @@ public final class FileStreamingUtil {
response.setHeader(HttpHeaders.CONTENT_RANGE, "bytes " + r.getStart() + "-" + r.getEnd() + "/" + r.getTotal()); response.setHeader(HttpHeaders.CONTENT_RANGE, "bytes " + r.getStart() + "-" + r.getEnd() + "/" + r.getTotal());
response.setContentLengthLong(r.getLength()); response.setContentLengthLong(r.getLength());
try (InputStream from = artifact.getFileInputStream()) { try (final InputStream from = artifact.getFileInputStream()) {
final ServletOutputStream to = response.getOutputStream(); final ServletOutputStream to = response.getOutputStream();
copyStreams(from, to, progressListener, r.getStart(), r.getLength(), filename); copyStreams(from, to, progressListener, r.getStart(), r.getLength(), filename);
} catch (final IOException e) { } catch (final IOException e) {
@@ -268,7 +284,7 @@ public final class FileStreamingUtil {
final ServletOutputStream to = response.getOutputStream(); final ServletOutputStream to = response.getOutputStream();
for (final ByteRange r : ranges) { for (final ByteRange r : ranges) {
try (InputStream from = artifact.getFileInputStream()) { try (final InputStream from = artifact.getFileInputStream()) {
// Add multipart boundary and header fields for every range. // Add multipart boundary and header fields for every range.
to.println(); to.println();
@@ -299,7 +315,7 @@ public final class FileStreamingUtil {
response.setContentLengthLong(r.getLength()); response.setContentLengthLong(r.getLength());
response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT); response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT);
try (InputStream from = artifact.getFileInputStream()) { try (final InputStream from = artifact.getFileInputStream()) {
final ServletOutputStream to = response.getOutputStream(); final ServletOutputStream to = response.getOutputStream();
copyStreams(from, to, progressListener, r.getStart(), r.getLength(), filename); copyStreams(from, to, progressListener, r.getStart(), r.getLength(), filename);
} catch (final IOException e) { } catch (final IOException e) {

View File

@@ -10,29 +10,30 @@ package org.eclipse.hawkbit.app;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.options; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.options;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants; import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants;
import org.eclipse.hawkbit.repository.test.util.MsSqlTestDatabase; import org.eclipse.hawkbit.repository.test.util.MsSqlTestDatabase;
import org.eclipse.hawkbit.repository.test.util.MySqlTestDatabase; import org.eclipse.hawkbit.repository.test.util.MySqlTestDatabase;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.eclipse.hawkbit.repository.test.util.PostgreSqlTestDatabase;
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.http.HttpHeaders;
import org.springframework.security.test.context.support.WithUserDetails; import org.springframework.security.test.context.support.WithUserDetails;
import org.springframework.test.context.TestExecutionListeners; import org.springframework.test.context.TestExecutionListeners;
import org.springframework.test.context.TestExecutionListeners.MergeMode; import org.springframework.test.context.TestExecutionListeners.MergeMode;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.ResultActions;
import io.qameta.allure.Description; import io.qameta.allure.Description;
import io.qameta.allure.Feature; import io.qameta.allure.Feature;
import io.qameta.allure.Story; import io.qameta.allure.Story;
@SpringBootTest(properties = {"hawkbit.dmf.rabbitmq.enabled=false", "hawkbit.server.security.cors.enabled=true", @SpringBootTest(properties = { "hawkbit.dmf.rabbitmq.enabled=false", "hawkbit.server.security.cors.enabled=true",
"hawkbit.server.security.cors.allowedOrigins=" + CorsTest.ALLOWED_ORIGIN_FIRST + "," + CorsTest.ALLOWED_ORIGIN_SECOND}) "hawkbit.server.security.cors.allowedOrigins=" + CorsTest.ALLOWED_ORIGIN_FIRST + ","
@TestExecutionListeners(listeners = { MySqlTestDatabase.class, MsSqlTestDatabase.class }, + CorsTest.ALLOWED_ORIGIN_SECOND,
mergeMode = MergeMode.MERGE_WITH_DEFAULTS) "hawkbit.server.security.cors.exposedHeaders=Access-Control-Allow-Origin" })
@TestExecutionListeners(listeners = { MySqlTestDatabase.class,
MsSqlTestDatabase.class }, mergeMode = MergeMode.MERGE_WITH_DEFAULTS)
@Feature("Integration Test - Security") @Feature("Integration Test - Security")
@Story("CORS") @Story("CORS")
public class CorsTest extends AbstractSecurityTest { public class CorsTest extends AbstractSecurityTest {
@@ -47,16 +48,21 @@ public class CorsTest extends AbstractSecurityTest {
@Test @Test
@Description("Ensures that Cors is working.") @Description("Ensures that Cors is working.")
public void validateCorsRequest() throws Exception { public void validateCorsRequest() throws Exception {
performOptionsRequestToRestWithOrigin(ALLOWED_ORIGIN_FIRST).andExpect(status().isOk()); performOptionsRequestToRestWithOrigin(ALLOWED_ORIGIN_FIRST).andExpect(status().isOk())
performOptionsRequestToRestWithOrigin(ALLOWED_ORIGIN_SECOND).andExpect(status().isOk()); .andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, ALLOWED_ORIGIN_FIRST));
performOptionsRequestToRestWithOrigin(ALLOWED_ORIGIN_SECOND).andExpect(status().isOk())
.andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, ALLOWED_ORIGIN_SECOND));
final String invalidOriginResponseBody = performOptionsRequestToRestWithOrigin(INVALID_ORIGIN) final String invalidOriginResponseBody = performOptionsRequestToRestWithOrigin(INVALID_ORIGIN)
.andExpect(status().isForbidden()).andReturn().getResponse().getContentAsString(); .andExpect(status().isForbidden())
.andExpect(header().doesNotExist(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).andReturn().getResponse()
.getContentAsString();
assertThat(invalidOriginResponseBody).isEqualTo(INVALID_CORS_REQUEST); assertThat(invalidOriginResponseBody).isEqualTo(INVALID_CORS_REQUEST);
final String invalidCorsUrlResponseBody = performOptionsRequestToUrlWithOrigin( final String invalidCorsUrlResponseBody = performOptionsRequestToUrlWithOrigin(
MgmtRestConstants.BASE_SYSTEM_MAPPING, ALLOWED_ORIGIN_FIRST).andExpect(status().isForbidden()) MgmtRestConstants.BASE_SYSTEM_MAPPING, ALLOWED_ORIGIN_FIRST).andExpect(status().isForbidden())
.andReturn().getResponse().getContentAsString(); .andExpect(header().doesNotExist(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).andReturn()
.getResponse().getContentAsString();
assertThat(invalidCorsUrlResponseBody).isEqualTo(INVALID_CORS_REQUEST); assertThat(invalidCorsUrlResponseBody).isEqualTo(INVALID_CORS_REQUEST);
} }

View File

@@ -130,6 +130,11 @@ public class HawkbitSecurityProperties {
*/ */
private List<String> allowedMethods = Arrays.asList("DELETE", "GET", "POST", "PATCH", "PUT"); private List<String> allowedMethods = Arrays.asList("DELETE", "GET", "POST", "PATCH", "PUT");
/**
* Exposed headers for CORS.
*/
private List<String> exposedHeaders = Collections.emptyList();
public boolean isEnabled() { public boolean isEnabled() {
return enabled; return enabled;
} }
@@ -161,6 +166,14 @@ public class HawkbitSecurityProperties {
public void setAllowedMethods(final List<String> allowedMethods) { public void setAllowedMethods(final List<String> allowedMethods) {
this.allowedMethods = allowedMethods; this.allowedMethods = allowedMethods;
} }
public List<String> getExposedHeaders() {
return exposedHeaders;
}
public void setExposedHeaders(final List<String> exposedHeaders) {
this.exposedHeaders = exposedHeaders;
}
} }
/** /**
@@ -448,6 +461,10 @@ public class HawkbitSecurityProperties {
return maxDistributionSetTypesPerTargetType; return maxDistributionSetTypesPerTargetType;
} }
public void setMaxDistributionSetTypesPerTargetType(final int maxDistributionSetTypesPerTargetType) {
this.maxDistributionSetTypesPerTargetType = maxDistributionSetTypesPerTargetType;
}
/** /**
* Configuration for hawkBits DOS prevention filter. This is usually an * Configuration for hawkBits DOS prevention filter. This is usually an
* infrastructure topic (e.g. Web Application Firewall (WAF)) but might * infrastructure topic (e.g. Web Application Firewall (WAF)) but might