Skip to content

Commit

Permalink
Add a little logic to CORS config processing and update the CORS doc
Browse files Browse the repository at this point in the history
Signed-off-by: Tim Quinn <tim.quinn@oracle.com>
  • Loading branch information
tjquinno committed Jan 8, 2024
1 parent db10837 commit b5f215a
Show file tree
Hide file tree
Showing 9 changed files with 206 additions and 158 deletions.
23 changes: 18 additions & 5 deletions cors/src/main/java/io/helidon/cors/Aggregator.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2023 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -118,10 +118,23 @@ public Aggregator build() {

Builder config(Config config) {
if (config.exists()) {
ConfigValue<CrossOriginConfig.Builder> configValue = config.map(CrossOriginConfig::builder);
if (configValue.isPresent()) {
CrossOriginConfig crossOriginConfig = configValue.get().build();
addPathlessCrossOrigin(crossOriginConfig);
// The config node might be a simple config block for a single cross-origin config (containing allow-origins,
// allow-methods, etc.). Or it might be a mapped cross-origin config introduced by "paths" and
// containing a list, each element of which has a path-pattern plus the "plain" cross-origin config settings
// (allow-origins, etc.)
Config pathsNode = config.get(CrossOriginConfig.CORS_PATHS_CONFIG_KEY);
if (pathsNode.exists()) {
ConfigValue<MappedCrossOriginConfig.Builder> configValue = config.map(MappedCrossOriginConfig::builder);
if (configValue.isPresent()) {
MappedCrossOriginConfig mappedCrossOriginConfig = configValue.get().build();
mappedCrossOriginConfig.forEach(this::addCrossOrigin);
}
} else {
ConfigValue<CrossOriginConfig.Builder> configValue = config.map(CrossOriginConfig::builder);
if (configValue.isPresent()) {
CrossOriginConfig crossOriginConfig = configValue.get().build();
addPathlessCrossOrigin(crossOriginConfig);
}
}
}
return this;
Expand Down
4 changes: 2 additions & 2 deletions cors/src/main/java/io/helidon/cors/CorsSupportHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ Optional<R> processCorsRequest(

CrossOriginConfig crossOriginConfig = crossOriginOpt.get();

// If enabled but not whitelisted, deny request
// If enabled but not allow-listed, deny request
List<String> allowedOrigins = Arrays.asList(crossOriginConfig.allowOrigins());
Optional<String> originOpt = requestAdapter.firstHeader(HeaderNames.ORIGIN);
if (!allowedOrigins.contains("*") && !contains(originOpt, allowedOrigins, CorsSupportHelper::compareOrigins)) {
Expand Down Expand Up @@ -555,7 +555,7 @@ R processCorsPreFlightRequest(CorsRequestAdapter<Q> requestAdapter, CorsResponse
}
CrossOriginConfig crossOrigin = crossOriginOpt.get();

// If enabled but not whitelisted, deny request
// If enabled but not allow-listed, deny request
List<String> allowedOrigins = Arrays.asList(crossOrigin.allowOrigins());
if (!allowedOrigins.contains("*") && !contains(originOpt, allowedOrigins, CorsSupportHelper::compareOrigins)) {
return forbid(requestAdapter,
Expand Down
13 changes: 8 additions & 5 deletions cors/src/main/java/io/helidon/cors/LogHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,23 @@ static <T> void logIsRequestTypeNormal(boolean result, boolean silent, CorsReque
}

if (host.isEmpty()) {
reasonsWhyNormal.add("header " + HeaderNames.HOST + " is absent");
reasonsWhyNormal.add("header " + HeaderNames.HOST + "/requested URI is absent");
} else {
factorsWhyCrossHost.add(String.format("header %s is present (%s)", HeaderNames.HOST, host));
factorsWhyCrossHost.add(String.format("header %s/requested URI is present (%s)", HeaderNames.HOST, host));
}

if (originOpt.isPresent()) {
String partOfOriginMatchingHost = "://" + host;
if (originOpt.get()
.contains(partOfOriginMatchingHost)) {
reasonsWhyNormal.add(String.format("header %s '%s' matches header %s '%s'", HeaderNames.ORIGIN,
reasonsWhyNormal.add(String.format("header %s '%s' matches header/requested URI %s '%s'", HeaderNames.ORIGIN,
effectiveOrigin, HeaderNames.HOST, host));
} else {
factorsWhyCrossHost.add(String.format("header %s '%s' does not match header %s '%s'", HeaderNames.ORIGIN,
effectiveOrigin, HeaderNames.HOST, host));
factorsWhyCrossHost.add(String.format("header %s '%s' does not match header/requested URI %s '%s'",
HeaderNames.ORIGIN,
effectiveOrigin,
HeaderNames.HOST,
host));
}
}

Expand Down
61 changes: 60 additions & 1 deletion cors/src/test/java/io/helidon/cors/CrossOriginConfigTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2022 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,9 +19,14 @@
import java.util.List;
import java.util.Optional;

import io.helidon.common.testing.junit5.OptionalMatcher;
import io.helidon.common.uri.UriInfo;
import io.helidon.config.Config;
import io.helidon.config.ConfigSources;
import io.helidon.config.MissingValueException;
import io.helidon.http.HeaderNames;
import io.helidon.http.Status;
import io.helidon.http.WritableHeaders;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
Expand Down Expand Up @@ -175,4 +180,58 @@ void testOrdering() {
crossOriginConfigOpt.get().pathPattern(),
is(crossOriginConfigs.get(1).pathPattern()));
}

@Test
void testBuiltInServiceConfig() {
Config node = testConfig.get("built-in-service");
assertThat(node, is(notNullValue()));
assertThat(node.exists(), is(true));
CorsSupportHelper<TestCorsServerRequestAdapter, TestCorsServerResponseAdapter> helper =
CorsSupportHelper.<TestCorsServerRequestAdapter, TestCorsServerResponseAdapter>builder()
.config(node)
.build();

// In the first test, use an unacceptable origin and make sure we get a 403 response.
WritableHeaders<?> headers = WritableHeaders.create();
headers.add(HeaderNames.ORIGIN, "http://bad.com")
.add(HeaderNames.HOST, "someProxy.com")
.add(HeaderNames.ACCESS_CONTROL_REQUEST_METHOD, "GET");
UriInfo uriInfo = UriInfo.builder()
.scheme("http")
.host("localhost")
.path("/")
.port(80)
.build();
CorsRequestAdapter<TestCorsServerRequestAdapter> req = new TestCorsServerRequestAdapter("/observe/health",
uriInfo,
"GET",
headers);
CorsResponseAdapter<TestCorsServerResponseAdapter> resp = new TestCorsServerResponseAdapter();
Optional<TestCorsServerResponseAdapter> resultOpt = helper.processRequest(req, resp);

assertThat("Response from GET to built-in service path",
resultOpt,
OptionalMatcher.optionalPresent());
assertThat("Response status from secure PUT to unsecured path",
resultOpt.get().status(),
is(Status.FORBIDDEN_403.code()));

// In the next test, use an acceptable origin in the request. There should be no response because that's how
// the CORS support helper lets the caller know that it's an acceptable CORS request.
headers = WritableHeaders.create();
headers.add(HeaderNames.ORIGIN, "http://roothere.com")
.add(HeaderNames.HOST, "someProxy.com")
.add(HeaderNames.ACCESS_CONTROL_REQUEST_METHOD, "GET");
req = new TestCorsServerRequestAdapter("/observe/health",
uriInfo,
"GET",
headers);
resp = new TestCorsServerResponseAdapter();

resultOpt = helper.processRequest(req, resp);

assertThat("Response from GET to built-in service path",
resultOpt,
OptionalMatcher.optionalEmpty());
}
}
7 changes: 7 additions & 0 deletions cors/src/test/resources/configMapperTest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,10 @@ same-host-diff-ports:
allow-origins: [ "http://here.com:8080"]
- path-pattern: "/greet"
allow-origins: [ "http://here.com"]


built-in-service:
paths:
- path-pattern: "/observe/health"
allow-origins: ["http://roothere.com"]
allow-methods: ["GET", "HEAD"]
3 changes: 2 additions & 1 deletion docs/src/main/asciidoc/includes/attributes.adoc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
///////////////////////////////////////////////////////////////////////////////

Copyright (c) 2022, 2023 Oracle and/or its affiliates.
Copyright (c) 2022, 2024 Oracle and/or its affiliates.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -197,6 +197,7 @@ endif::[]
:config-git-javadoc-base-url: {javadoc-base-url}/io.helidon.config.git
:config-mapping-javadoc-base-url: {javadoc-base-url}/io.helidon.config.objectmapping
:configurable-javadoc-base-url: {javadoc-base-url}/io.helidon.common.configurable
:cors-javadoc-base-url: {javadoc-base-url}/io.helidon.cors
:faulttolerance-javadoc-base-url: {javadoc-base-url}/io.helidon.faulttolerance
:grpc-server-javadoc-base-url: {javadoc-base-url}/io.helidon.webserver.grpc
:health-javadoc-base-url: {javadoc-base-url}/io.helidon.health.checks
Expand Down
Loading

0 comments on commit b5f215a

Please sign in to comment.