Treat unencrypted local origins as an insecure context in Safari (#33700)

Closes #33557

Signed-off-by: Jon Koops <jonkoops@gmail.com>

common/src/main/java/org/keycloak/common/util/SecureContextResolver.java

@@ -1,49 +0,0 @@
-package org.keycloak.common.util;
-
-import java.net.URI;
-import java.util.regex.Pattern;
-
-public class SecureContextResolver {
-
-    private static final Pattern LOCALHOST_IPV4 = Pattern.compile("127.\\d{1,3}.\\d{1,3}.\\d{1,3}");
-
-    /**
-     * Determines if a URI is potentially trustworthy, meaning a user agent can generally trust it to deliver data securely.
-     *
-     * @see <a href="https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts">MDN Web Docs — Secure Contexts</a>
-     * @see <a href="https://w3c.github.io/webappsec-secure-contexts/#algorithms">W3C Secure Contexts specification — Is origin potentially trustworthy?</a>
-     * @param uri The URI to check.
-     * @return Whether the URI can be considered potentially trustworthy.
-     */
-    public static boolean isSecureContext(URI uri) {
-        if (uri.getScheme().equals("https")) {
-            return true;
-        }
-
-        String host = uri.getHost();
-        if (host == null) {
-            return false;
-        }
-
-        // The host matches a CIDR notation of ::1/128
-        if (host.equals("[::1]") || host.equals("[0000:0000:0000:0000:0000:0000:0000:0001]")) {
-            return true;
-        }
-
-        // The host matches a CIDR notation of 127.0.0.0/8
-        if (LOCALHOST_IPV4.matcher(host).matches()) {
-            return true;
-        }
-
-        if (host.equals("localhost") || host.equals("localhost.")) {
-            return true;
-        }
-
-        if (host.endsWith(".localhost") || host.endsWith(".localhost.")) {
-            return true;
-        }
-
-        return false;
-    }
-
-}

services/src/main/java/org/keycloak/cookie/DefaultCookieProvider.java

@@ -3,8 +3,9 @@ package org.keycloak.cookie;
 import jakarta.ws.rs.core.Cookie;
 import jakarta.ws.rs.core.NewCookie;
 import org.jboss.logging.Logger;
-import org.keycloak.common.util.SecureContextResolver;
 import org.keycloak.models.KeycloakContext;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.utils.SecureContextResolver;
 
 import java.util.Map;
 
@@ -12,7 +13,7 @@ public class DefaultCookieProvider implements CookieProvider {
 
     private static final Logger logger = Logger.getLogger(DefaultCookieProvider.class);
 
-    private final KeycloakContext context;
+    private final KeycloakSession session;
 
     private final CookiePathResolver pathResolver;
 
@@ -20,11 +21,13 @@ public class DefaultCookieProvider implements CookieProvider {
 
     private final Map<String, Cookie> cookies;
 
-    public DefaultCookieProvider(KeycloakContext context) {
+    public DefaultCookieProvider(KeycloakSession session) {
-        this.context = context;
+        KeycloakContext context = session.getContext();
+
+        this.session = session;
         this.cookies = context.getRequestHeaders().getCookies();
         this.pathResolver = new CookiePathResolver(context);
-        this.secure = SecureContextResolver.isSecureContext(context.getUri().getRequestUri());
+        this.secure = SecureContextResolver.isSecureContext(session);
 
         if (logger.isTraceEnabled()) {
             logger.tracef("Received cookies: %s, path: %s", String.join(", ", this.cookies.keySet()), context.getUri().getRequestUri().getRawPath());
@@ -67,7 +70,7 @@ public class DefaultCookieProvider implements CookieProvider {
                 .sameSite(sameSite)
                 .build();
 
-        context.getHttpResponse().setCookieIfAbsent(newCookie);
+        session.getContext().getHttpResponse().setCookieIfAbsent(newCookie);
 
         logger.tracef("Setting cookie: name: %s, path: %s, same-site: %s, secure: %s, http-only: %s, max-age: %d", name, path, sameSite, secure, httpOnly, maxAge);
     }
@@ -90,7 +93,7 @@ public class DefaultCookieProvider implements CookieProvider {
                     .maxAge(CookieMaxAge.EXPIRED)
                     .build();
 
-            context.getHttpResponse().setCookieIfAbsent(newCookie);
+            session.getContext().getHttpResponse().setCookieIfAbsent(newCookie);
 
             logger.tracef("Expiring cookie: name: %s, path: %s", cookie.getName(), path);
         }

services/src/main/java/org/keycloak/cookie/DefaultCookieProviderFactory.java

@@ -8,7 +8,7 @@ public class DefaultCookieProviderFactory implements CookieProviderFactory {
 
     @Override
     public CookieProvider create(KeycloakSession session) {
-        return new DefaultCookieProvider(session.getContext());
+        return new DefaultCookieProvider(session);
     }
 
     @Override

services/src/main/java/org/keycloak/device/DeviceRepresentationProviderImpl.java

@@ -16,6 +16,8 @@ public class DeviceRepresentationProviderImpl implements DeviceRepresentationPro
 
     private final KeycloakSession session;
 
+    private DeviceRepresentation deviceRepresentation;
+
     DeviceRepresentationProviderImpl(KeycloakSession session, Parser parser) {
         this.session = session;
         this.parser = parser;
@@ -23,6 +25,10 @@ public class DeviceRepresentationProviderImpl implements DeviceRepresentationPro
 
     @Override
     public DeviceRepresentation deviceRepresentation() {
+        if (deviceRepresentation != null) {
+            return deviceRepresentation;
+        }
+
         KeycloakContext context = session.getContext();
 
         if (context.getRequestHeaders() == null) {
@@ -81,6 +87,8 @@ public class DeviceRepresentationProviderImpl implements DeviceRepresentationPro
             current.setOsVersion(osVersion);
             current.setIpAddress(context.getConnection().getRemoteAddr());
             current.setMobile(userAgent.toLowerCase().contains("mobile"));
+
+            deviceRepresentation = current;
         } catch (Exception cause) {
             logger.error("Failed to create device info from user agent header", cause);
             return null;

services/src/main/java/org/keycloak/services/resources/account/AccountConsole.java

@@ -3,13 +3,12 @@ package org.keycloak.services.resources.account;
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.Path;
 import jakarta.ws.rs.core.Response;
-import jakarta.ws.rs.core.UriInfo;
 import org.jboss.resteasy.reactive.NoCache;
 import org.keycloak.authentication.requiredactions.DeleteAccount;
 import org.keycloak.common.Profile;
 import org.keycloak.common.Version;
 import org.keycloak.common.util.Environment;
-import org.keycloak.common.util.SecureContextResolver;
+import org.keycloak.utils.SecureContextResolver;
 import org.keycloak.models.AccountRoles;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
@@ -38,7 +37,6 @@ import org.keycloak.utils.MediaType;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.URI;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -110,7 +108,7 @@ public class AccountConsole implements AccountResourceProvider {
                 .path("/")
                 .build(realm);
 
-        final var isSecureContext = SecureContextResolver.isSecureContext(serverBaseUri);
+        final var isSecureContext = SecureContextResolver.isSecureContext(session);
 
         map.put("isSecureContext", isSecureContext);
         map.put("serverBaseUrl", serverBaseUrl);

services/src/main/java/org/keycloak/services/resources/admin/AdminConsole.java

@@ -33,7 +33,7 @@ import org.keycloak.common.ClientConnection;
 import org.keycloak.common.Profile;
 import org.keycloak.common.Version;
 import org.keycloak.common.util.Environment;
-import org.keycloak.common.util.SecureContextResolver;
+import org.keycloak.utils.SecureContextResolver;
 import org.keycloak.common.util.UriUtils;
 import org.keycloak.headers.SecurityHeadersProvider;
 import org.keycloak.http.HttpRequest;
@@ -54,14 +54,12 @@ import org.keycloak.services.managers.ClientManager;
 import org.keycloak.services.managers.RealmManager;
 import org.keycloak.services.util.ViteManifest;
 import org.keycloak.theme.FreeMarkerException;
-import org.keycloak.theme.Theme;
 import org.keycloak.theme.freemarker.FreeMarkerProvider;
 import org.keycloak.urls.UrlType;
 import org.keycloak.utils.MediaType;
 
 import java.io.IOException;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -348,7 +346,7 @@ public class AdminConsole {
 
             final var map = new HashMap<String, Object>();
             final var theme = AdminRoot.getTheme(session, realm);
-            final var isSecureContext = SecureContextResolver.isSecureContext(adminBaseUri);
+            final var isSecureContext = SecureContextResolver.isSecureContext(session);
 
             map.put("isSecureContext", isSecureContext);
             map.put("serverBaseUrl", serverBaseUrl);

services/src/main/java/org/keycloak/utils/SecureContextResolver.java

@@ -0,0 +1,76 @@
+package org.keycloak.utils;
+
+import org.keycloak.device.DeviceRepresentationProvider;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.representations.account.DeviceRepresentation;
+
+import java.net.URI;
+import java.util.function.Supplier;
+import java.util.regex.Pattern;
+
+public class SecureContextResolver {
+
+    private static final Pattern LOCALHOST_IPV4 = Pattern.compile("127.\\d{1,3}.\\d{1,3}.\\d{1,3}");
+
+    /**
+     * Determines if a session is within a 'secure context', meaning its origin is considered potentially trustworthy by user-agents.
+     *
+     * @see <a href="https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts">MDN Web Docs — Secure Contexts</a>
+     * @see <a href="https://w3c.github.io/webappsec-secure-contexts/#algorithms">W3C Secure Contexts specification — Is origin potentially trustworthy?</a>
+     * @param session The session to check for trustworthiness.
+     * @return Whether the session can be considered potentially trustworthy by user-agents.
+     */
+    public static boolean isSecureContext(KeycloakSession session) {
+        URI uri = session.getContext().getUri().getRequestUri();
+
+        // Use a Supplier so the user-agent is evaluated lazily, avoiding unnecessary parsing in production deployments.
+        Supplier<DeviceRepresentation> deviceRepresentationSupplier = () -> {
+            DeviceRepresentationProvider deviceRepresentationProvider = session.getProvider(DeviceRepresentationProvider.class);
+            return deviceRepresentationProvider.deviceRepresentation();
+        };
+
+        return isSecureContext(uri, deviceRepresentationSupplier);
+    }
+
+    static boolean isSecureContext(URI uri, Supplier<DeviceRepresentation> deviceRepresentationSupplier) {
+        if (uri.getScheme().equals("https")) {
+            return true;
+        }
+
+        DeviceRepresentation deviceRepresentation = deviceRepresentationSupplier.get();
+        String browser = deviceRepresentation.getBrowser();
+
+        // Safari has a bug where even a secure context is not able to set cookies with the 'Secure' directive.
+        // Hence, we need to assume the worst case scenario and downgrade to an insecure context.
+        // See:
+        // - https://github.com/keycloak/keycloak/issues/33557
+        // - https://webcompat.com/issues/142566
+        // - https://bugs.webkit.org/show_bug.cgi?id=232088
+        // - https://bugs.webkit.org/show_bug.cgi?id=276313
+        if (browser != null && browser.toLowerCase().contains("safari")) {
+            return false;
+        }
+
+        String host = uri.getHost();
+
+        if (host == null) {
+            return false;
+        }
+
+        // The host matches a CIDR notation of ::1/128
+        if (host.equals("[::1]") || host.equals("[0000:0000:0000:0000:0000:0000:0000:0001]")) {
+            return true;
+        }
+
+        // The host matches a CIDR notation of 127.0.0.0/8
+        if (LOCALHOST_IPV4.matcher(host).matches()) {
+            return true;
+        }
+
+        if (host.equals("localhost") || host.equals("localhost.")) {
+            return true;
+        }
+
+        return host.endsWith(".localhost") || host.endsWith(".localhost.");
+    }
+}

services/src/test/java/org/keycloak/utils/SecureContextResolverTest.java

@@ -1,13 +1,17 @@
-package org.keycloak.common.util;
+package org.keycloak.utils;
 
 import org.junit.Assert;
 import org.junit.Test;
+import org.keycloak.representations.account.DeviceRepresentation;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.function.Supplier;
 
 public class SecureContextResolverTest {
 
+    static final String BROWSER_SAFARI = "Safari/18.0.1";
+
     @Test
     public void testHttps() {
         assertSecureContext("https://127.0.0.1", true);
@@ -47,9 +51,32 @@ public class SecureContextResolverTest {
         assertSecureContext("http://test.localhost.not", false);
     }
 
+    @Test
+    public void testQuirksSafari() {
+        assertSecureContext("https://127.0.0.1", BROWSER_SAFARI, true);
+        assertSecureContext("https://something", BROWSER_SAFARI, true);
+        assertSecureContext("http://[::1]", BROWSER_SAFARI,false);
+        assertSecureContext("http://[0000:0000:0000:0000:0000:0000:0000:0001]", BROWSER_SAFARI, false);
+        assertSecureContext("http://localhost", BROWSER_SAFARI, false);
+        assertSecureContext("http://localhost.", BROWSER_SAFARI, false);
+        assertSecureContext("http://test.localhost", BROWSER_SAFARI, false);
+        assertSecureContext("http://test.localhost.", BROWSER_SAFARI, false);
+    }
+
     void assertSecureContext(String url, boolean expectedSecureContext) {
+        assertSecureContext(url, null, expectedSecureContext);
+    }
+
+    void assertSecureContext(String url, String browser, boolean expectedSecureContext) {
+        DeviceRepresentation deviceRepresentation = new DeviceRepresentation();
+        Supplier<DeviceRepresentation> deviceRepresentationSupplier = () -> deviceRepresentation;
+
+        if (browser != null) {
+            deviceRepresentation.setBrowser(browser);
+        }
+
         try {
-            Assert.assertEquals(expectedSecureContext, SecureContextResolver.isSecureContext(new URI(url)));
+            Assert.assertEquals(expectedSecureContext, SecureContextResolver.isSecureContext(new URI(url), deviceRepresentationSupplier));
         } catch (URISyntaxException e) {
             Assert.fail(e.getMessage());
         }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cookies/DefaultCookieProviderTest.java

@@ -2,6 +2,7 @@ package org.keycloak.testsuite.cookies;
 
 import jakarta.ws.rs.client.ClientRequestContext;
 import jakarta.ws.rs.client.ClientRequestFilter;
+import jakarta.ws.rs.core.HttpHeaders;
 import jakarta.ws.rs.core.NewCookie;
 import jakarta.ws.rs.core.Response;
 import org.jboss.resteasy.client.jaxrs.ResteasyClient;
@@ -171,6 +172,38 @@ public class DefaultCookieProviderTest extends AbstractKeycloakTest {
         assertCookie(response, "mycookie", "myvalue", "/auth/realms/master/testing/run-on-server", 1232, false, false, null, false);
     }
 
+    @Test
+    public void testSafariQuirks() {
+        Response response;
+
+        try (KeycloakTestingClient testingClient = createTestingClient("http://localhost:8180/auth")) {
+
+            filter.setHeader(HttpHeaders.USER_AGENT, "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.0.1 Safari/605.1.15");
+
+            response = testingClient.server("master").runWithResponse(session -> {
+                CookieProvider cookies = session.getProvider(CookieProvider.class);
+                cookies.set(CookieType.AUTH_SESSION_ID, "my-auth-session-id");
+                cookies.set(CookieType.AUTH_RESTART, "my-auth-restart");
+                cookies.set(CookieType.AUTH_DETACHED, "my-auth-detached", 222);
+                cookies.set(CookieType.IDENTITY, "my-identity", 333);
+                cookies.set(CookieType.LOCALE, "my-locale");
+                cookies.set(CookieType.LOGIN_HINT, "my-username");
+                cookies.set(CookieType.SESSION, "my-session", 444);
+                cookies.set(CookieType.WELCOME_CSRF, "my-welcome-csrf");
+            });
+        }
+
+        Assert.assertEquals(8, response.getCookies().size());
+        assertCookie(response, "AUTH_SESSION_ID", "my-auth-session-id", "/auth/realms/master/", -1, false, true, "Lax", true);
+        assertCookie(response, "KC_RESTART", "my-auth-restart", "/auth/realms/master/", -1, false, true, "Lax", false);
+        assertCookie(response, "KC_STATE_CHECKER", "my-auth-detached", "/auth/realms/master/", 222, false, true, "Strict", false);
+        assertCookie(response, "KEYCLOAK_IDENTITY", "my-identity", "/auth/realms/master/", 333, false, true, "Lax", true);
+        assertCookie(response, "KEYCLOAK_LOCALE", "my-locale", "/auth/realms/master/", -1, false, true, "Lax", false);
+        assertCookie(response, "KEYCLOAK_REMEMBER_ME", "my-username", "/auth/realms/master/", 31536000, false, true, "Lax", false);
+        assertCookie(response, "KEYCLOAK_SESSION", "my-session", "/auth/realms/master/", 444, false, false, "Lax", true);
+        assertCookie(response, "WELCOME_STATE_CHECKER", "my-welcome-csrf", "/auth/realms/master/testing/run-on-server", 300, false, true, "Strict", false);
+    }
+
     private void assertCookie(Response response, String name, String value, String path, int maxAge, boolean secure, boolean httpOnly, String sameSite, boolean verifyLegacyNotSent) {
         Map<String, NewCookie> cookies = response.getCookies();
         NewCookie cookie = cookies.get(name);