diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsage.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsage.java index 9783c407..ac8a5b99 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsage.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsage.java @@ -33,6 +33,7 @@ import java.util.Set; import static fiftyone.pipeline.core.Constants.*; +import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS; /** * This filter is used by the {@link ShareUsageElement} to decide which of the @@ -44,12 +45,13 @@ * 2) A cookie that starts with {@link Constants#FIFTYONE_COOKIE_PREFIX} or is * the session cookie (if configured in the constructor). * 3) A session value that starts with {@link Constants#FIFTYONE_COOKIE_PREFIX}. - * 4) A query string parameter that starts with + * 4) A query string parameter, when all query string parameters are shared, + * which is the default. Otherwise one that starts with * {@link Constants#FIFTYONE_COOKIE_PREFIX} or has been configured to be * shared using the constructor parameter. * 5) Not a header, cookie, session value or query string parameter. - * When all Evidence is shared, everything is included. When all query string - * parameters are shared, every query string parameter is included. + * When all Evidence is shared, everything is included but cookies, which + * follow rule 2 whatever the configuration. * The prefix of a key is everything before its first '.', and the field * everything after it. An email address, the salt for hashed email * identifiers, the query string, the path and the Cookie header are never @@ -112,12 +114,14 @@ public class EvidenceKeyFilterShareUsage implements EvidenceKeyFilter { }; /** - * Constructor. + * Constructor. All Evidence is not shared, and all query string + * parameters are, as are the defaults. * @param blockedHttpHeaders a list of the names of the HTTP headers that * share usage should not send to 51Degrees * @param includedQueryStringParams a list of the names of query string * parameters that share usage should send - * to 51Degrees + * to 51Degrees when not all of them are + * shared * @param includeSession if true then the session cookie will be included in * the filter * @param sessionCookieName the name of the cookie that contains the session @@ -134,7 +138,7 @@ public EvidenceKeyFilterShareUsage( includeSession, sessionCookieName, false, - false); + SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS); } /** @@ -143,13 +147,15 @@ public EvidenceKeyFilterShareUsage( * share usage should not send to 51Degrees * @param includedQueryStringParams a list of the names of query string * parameters that share usage should send - * to 51Degrees + * to 51Degrees when not all of them are + * shared * @param includeSession if true then the session cookie will be included in * the filter * @param sessionCookieName the name of the cookie that contains the session * id * @param shareAllEvidence if true then all Evidence is included, apart from - * the keys that are never shared + * the keys that are never shared and the cookies + * other than the 51Degrees ones * @param shareAllQueryStringParameters if true then all query string * parameters are included */ @@ -197,12 +203,22 @@ public boolean include(String key) { } } + int separator = lowerKey.indexOf(EVIDENCE_SEPERATOR); + + // Only the 51Degrees cookies are shared, even when all Evidence is. + // The session cookie is included only for the tracker to compare. + if (separator > 0 && + EVIDENCE_COOKIE_PREFIX.equals(lowerKey.substring(0, separator))) { + String field = lowerKey.substring(separator + 1); + return field.startsWith(Constants.FIFTYONE_COOKIE_PREFIX) || + (includeSession && field.equals(sessionCookieName)); + } + if (shareAllEvidence) { return true; } boolean result; - int separator = lowerKey.indexOf(EVIDENCE_SEPERATOR); if (separator > 0) { String prefix = lowerKey.substring(0, separator); String field = lowerKey.substring(separator + 1); @@ -212,12 +228,6 @@ public boolean include(String key) { // appear in the list of blocked headers. result = blockedHttpHeaders.contains(field) == false; break; - case EVIDENCE_COOKIE_PREFIX: - // Only add cookies that start with the 51Degrees cookie - // prefix. - result = field.startsWith(Constants.FIFTYONE_COOKIE_PREFIX) || - (includeSession && field.equals(sessionCookieName)); - break; case EVIDENCE_SESSION_PREFIX: // Only add session values that start with the 51Degrees cookie // prefix. diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/Constants.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/Constants.java index 0641be77..d68f5f0e 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/Constants.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/Constants.java @@ -112,6 +112,13 @@ public class Constants { */ static final String SHARE_USAGE_DEFAULT_URL = "https://devices-v4.51degrees.com/new.ashx"; + /** + * Every query string and HTTP form parameter is shared by default, not + * only the 51Degrees ones and those named. + */ + public static final boolean + SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS = true; + /** * Constant Added to blocked HTTP Headers so that the cookie header is * blocked by default. diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java index 1fd5bf08..c985340c 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java @@ -434,7 +434,7 @@ protected ShareUsageBase( Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT, Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT, false, - false); + Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS); } /** diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBuilderBase.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBuilderBase.java index 1ab56e3f..a19f28f7 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBuilderBase.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBuilderBase.java @@ -65,7 +65,8 @@ public abstract class ShareUsageBuilderBase { protected double maxRecoveryDelaySeconds = Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT; protected double recoveryMultiplier = Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT; protected boolean shareAllEvidence; - protected boolean shareAllQueryStringParameters; + protected boolean shareAllQueryStringParameters = + Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS; /** * Constructor @@ -154,13 +155,14 @@ public ShareUsageBuilderBase setIncludedQueryStringParameter( * If set to true, every query string and HTTP form parameter is shared, * rather than only those that start with 51D_ or are named in the * included parameters. The whole query string and the parameters that are - * never shared are still not shared. + * never shared are still not shared. Set it to false to share only the + * 51D_ parameters and those named. *

- * Default value is false + * Default value is true * @param shareAll true to share all query string parameters * @return this builder */ - @DefaultValue(booleanValue = false) + @DefaultValue(booleanValue = true) public ShareUsageBuilderBase setShareAllQueryStringParameters( boolean shareAll) { this.shareAllQueryStringParameters = shareAll; @@ -177,10 +179,11 @@ public boolean isShareAllQueryStringParameters() { /** * If set to true, all Evidence is shared, which overrides the blocked - * HTTP headers and the rules for cookies, session values and query string + * HTTP headers and the rules for session values and query string * parameters. The keys that are never shared (an email address, the salt * for hashed email identifiers, the whole query string, the path and the - * Cookie header) are still not shared. + * Cookie header) are still not shared, and nor is any cookie whose name + * does not start with 51D_. *

* Default value is false * @param shareAll true to share all Evidence diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElement.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElement.java index 58109cb2..e7b00bac 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElement.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElement.java @@ -43,6 +43,7 @@ import java.util.zip.GZIPOutputStream; import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_DEFAULT_HTTP_POST_TIMEOUT; +import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS; import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_INITIAL_DELAY_SECONDS_DEFAULT; import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT; import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT; @@ -332,7 +333,7 @@ public class ShareUsageElement extends ShareUsageBase { SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT, SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT, false, - false); + SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS); } /** diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsageTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsageTests.java index 8aeb6e7f..34ca22e2 100644 --- a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsageTests.java +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsageTests.java @@ -56,6 +56,35 @@ private static EvidenceKeyFilterShareUsage defaults() { false); } + private static EvidenceKeyFilterShareUsage filter( + boolean includeSession, + boolean shareAllEvidence, + boolean shareAllQueryStringParameters) { + return new EvidenceKeyFilterShareUsage( + Collections.emptyList(), + Collections.emptyList(), + includeSession, + DEFAULT_SESSION_COOKIE_NAME, + shareAllEvidence, + shareAllQueryStringParameters); + } + + /** + * Shares only the 51D_ query string parameters and those named, as when + * share all query string parameters is turned off. + */ + private static EvidenceKeyFilterShareUsage namedParametersOnly( + List blockedHttpHeaders, + List includedQueryStringParams) { + return new EvidenceKeyFilterShareUsage( + blockedHttpHeaders, + includedQueryStringParams, + false, + DEFAULT_SESSION_COOKIE_NAME, + false, + false); + } + /** * By default headers other than Cookie are shared, cookies, session * values and query string parameters only when their names start with @@ -73,6 +102,11 @@ public void webRequestIsNeverIncluded() { assertFalse(filter.include("Server.Http-Request")); } + /** + * By default every header but Cookie, every query string parameter but + * those never shared, and any other evidence are shared, and cookies and + * session values only when their names start with 51D_. + */ @Test public void defaultRules() { EvidenceKeyFilterShareUsage filter = defaults(); @@ -87,22 +121,93 @@ public void defaultRules() { assertFalse(filter.include("session.org.example.attribute")); assertTrue(filter.include("query.51d_gethighentropyvalues")); assertTrue(filter.include("query.51D_Test")); - assertFalse(filter.include("query.password")); + assertTrue(filter.include("query.password")); + assertTrue(filter.include("Query.Utm_Source")); assertFalse(filter.include("query.id.email")); + assertFalse(filter.include("server.request-query")); assertTrue(filter.include("server.client-ip")); assertTrue(filter.include("nodot")); } + /** + * With share all query string parameters off, only the 51D_ parameters + * and those named are shared. + */ + @Test + public void shareAllQueryStringParametersOff_OnlyNamedAnd51D() { + EvidenceKeyFilterShareUsage filter = namedParametersOnly( + Collections.emptyList(), + Collections.singletonList("campaign")); + + assertFalse(filter.include("query.password")); + assertTrue(filter.include("query.campaign")); + assertTrue(filter.include("query.51D_Test")); + assertTrue(filter.include("header.user-agent")); + } + + /** + * Even when all Evidence is shared, a cookie is shared only when its + * name starts with 51D_, whatever its case. The session cookie is kept + * only by a filter that includes it for the tracker to compare. + */ + @Test + public void shareAllEvidence_NoCookieBut51D() { + EvidenceKeyFilterShareUsage sent = filter(false, true, true); + EvidenceKeyFilterShareUsage kept = filter(true, true, true); + + for (EvidenceKeyFilterShareUsage filter : Arrays.asList(sent, kept)) { + assertTrue(filter.include("cookie.51D_Profile")); + assertTrue(filter.include("cookie.51d_screenpixelswidth")); + assertFalse(filter.include("cookie._ga")); + assertFalse(filter.include("Cookie.Other")); + assertFalse(filter.include("cookie.a.b")); + assertTrue(filter.include("session.org.example.attribute")); + assertTrue(filter.include("header.x-anything")); + assertTrue(filter.include("query.password")); + } + assertFalse(sent.include("cookie." + DEFAULT_SESSION_COOKIE_NAME)); + assertTrue(kept.include("cookie." + DEFAULT_SESSION_COOKIE_NAME)); + } + + /** + * With all Evidence shared, the tracker compares the session cookie only + * when track session is on, and no other cookie but the 51D_ ones. + */ + @Test + public void trackerWithShareAllEvidence_SessionCookieOnlyWhenTracked() { + EvidenceKeyFilterShareUsageTracker tracked = + new EvidenceKeyFilterShareUsageTracker( + Collections.emptyList(), + Collections.emptyList(), + true, + DEFAULT_SESSION_COOKIE_NAME, + true, + true); + EvidenceKeyFilterShareUsageTracker untracked = + new EvidenceKeyFilterShareUsageTracker( + Collections.emptyList(), + Collections.emptyList(), + false, + DEFAULT_SESSION_COOKIE_NAME, + true, + true); + + assertTrue(tracked.include("cookie." + DEFAULT_SESSION_COOKIE_NAME)); + assertFalse(untracked.include("cookie." + DEFAULT_SESSION_COOKIE_NAME)); + assertFalse(tracked.include("cookie._ga")); + assertFalse(untracked.include("cookie._ga")); + assertTrue(untracked.include("cookie.51D_Profile")); + } + /** * A name containing a dot is matched as a whole against the parameters to * share. */ @Test public void fieldIsEverythingAfterTheFirstDot() { - EvidenceKeyFilterShareUsage filter = filter( + EvidenceKeyFilterShareUsage filter = namedParametersOnly( Collections.emptyList(), - Collections.singletonList("valid.email"), - false); + Collections.singletonList("valid.email")); assertTrue(filter.include("query.valid.email")); assertFalse(filter.include("query.valid")); @@ -115,10 +220,9 @@ public void fieldIsEverythingAfterTheFirstDot() { */ @Test public void listsIgnoreCase() { - EvidenceKeyFilterShareUsage filter = filter( + EvidenceKeyFilterShareUsage filter = namedParametersOnly( Collections.singletonList("X-Secret"), - Collections.singletonList("Campaign"), - false); + Collections.singletonList("Campaign")); assertFalse(filter.include("header.x-secret")); assertTrue(filter.include("query.campaign")); diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java index eb719098..21e8a184 100644 --- a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java @@ -514,7 +514,7 @@ public void ShareUsageElement_WebRequest_SharesWhatNoOtherElementAccepts() request.put("header.X-Blocked", "blocked"); request.put("cookie.51D_Profile", "123456"); request.put("cookie.other", "not-shared"); - request.put("query.secret", "not-shared"); + request.put("query.campaign", "shared-query"); request.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); request.put("server.request-method", "GET"); request.put("server.request-path", "/a%2Fb/c"); @@ -554,9 +554,13 @@ public void ShareUsageElement_WebRequest_SharesWhatNoOtherElementAccepts() "//cookie[@Name='51D_Profile']", doc), "A 51D_ cookie from the web request was not shared."); assertEquals(0, ((NodeList) xpath.evaluate( - "//cookie[@Name='other'] | //query[@Name='secret']", doc, + "//cookie[@Name='other']", doc, XPathConstants.NODESET)).getLength(), - "A cookie or query string parameter the rules leave out was shared."); + "A cookie the rules leave out was shared."); + assertEquals("shared-query", xpath.evaluate( + "//query[@Name='campaign']", doc), + "A query string parameter from the web request was not " + + "shared by default."); assertEquals("1.2.3.4", xpath.evaluate("//ClientIP", doc), "The client IP from the web request was not shared."); assertEquals("GET", xpath.evaluate( @@ -1260,10 +1264,10 @@ public void CheckForContinueAndResumeWhenQueueFull() throws Exception { } /** - * Default settings share headers other than Cookie, the 51Degrees cookies - * and query string parameters, and other evidence, and leave out ordinary - * cookies, other parameters, session values and the evidence that is - * never shared. + * Default settings share headers other than Cookie, the 51Degrees cookies, + * every query string parameter but those never shared, and other + * evidence, and leave out ordinary cookies, session values and the + * evidence that is never shared. */ @Test public void ShareUsageElement_DefaultSharesOnlyAcceptedEvidence() throws Exception { @@ -1304,11 +1308,12 @@ public void ShareUsageElement_DefaultSharesOnlyAcceptedEvidence() throws Excepti String output = new String( connector.getBaos().toByteArray(), StandardCharsets.UTF_8); for (String shared : Arrays.asList( - "some user agent", "screen width", "fiftyone parameter")) { + "some user agent", "screen width", "fiftyone parameter", + "password parameter")) { assertTrue(output.contains(shared), shared); } for (String withheld : Arrays.asList( - "sessionid cookie", "cookie header", "password parameter", + "sessionid cookie", "cookie header", "email parameter", "email header", "salt value", "whole query string", "request path", "session attribute")) { assertFalse(output.contains(withheld), withheld); diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageOptionsTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageOptionsTests.java index 3bc53524..fe71cf40 100644 --- a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageOptionsTests.java +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageOptionsTests.java @@ -127,6 +127,7 @@ private static Map evidence() { evidence.put("header.x-blocked", "blocked"); evidence.put("header.cookie", "a=b"); evidence.put("cookie.plain", "cookie"); + evidence.put("cookie.51D_plain", "51d-cookie"); evidence.put("session.plain", "session"); evidence.put("query.plain", "query"); evidence.put("query.id.email", "a@example.com"); @@ -178,8 +179,9 @@ public void defaults_ShareOnlyWhatTheRulesAllow() throws Exception { assertTrue(shared(sent, "header", "user-agent")); assertFalse(shared(sent, "header", "x-blocked")); assertFalse(shared(sent, "cookie", "plain")); + assertTrue(shared(sent, "cookie", "51D_plain")); assertFalse(shared(sent, "session", "plain")); - assertFalse(shared(sent, "query", "plain")); + assertTrue(shared(sent, "query", "plain")); } @Test @@ -189,7 +191,8 @@ public void shareAllEvidence_FromConfiguration_SharesEverythingButNeverShared() assertTrue(shared(sent, "header", "user-agent")); assertTrue(shared(sent, "header", "x-blocked")); - assertTrue(shared(sent, "cookie", "plain")); + assertFalse(shared(sent, "cookie", "plain")); + assertTrue(shared(sent, "cookie", "51D_plain")); assertTrue(shared(sent, "session", "plain")); assertTrue(shared(sent, "query", "plain")); assertFalse(shared(sent, "query", "id.email")); @@ -207,7 +210,7 @@ public void shareAllEvidence_False_FromConfiguration_KeepsTheRules() assertFalse(shared(sent, "header", "x-blocked")); assertFalse(shared(sent, "cookie", "plain")); - assertFalse(shared(sent, "query", "plain")); + assertTrue(shared(sent, "query", "plain")); } @Test @@ -237,13 +240,17 @@ public void shareAllQueryStringParameters_False_FromConfiguration_KeepsTheRules( assertFalse(shared(sent, "query", "plain")); } + /** + * Share all Evidence is off by default and share all query string + * parameters is on. + */ @Test - public void builder_ShareOptionsDefaultToFalse() { + public void builder_ShareOptionDefaults() { ShareUsageBuilder builder = new ShareUsageBuilder( LoggerFactory.getILoggerFactory()); assertFalse(builder.isShareAllEvidence()); - assertFalse(builder.isShareAllQueryStringParameters()); + assertTrue(builder.isShareAllQueryStringParameters()); } @Test