From fb8c9cf4e361841a8742b64967fe71fcb2a12f99 Mon Sep 17 00:00:00 2001 From: Edgar Espina Date: Sun, 25 Oct 2026 11:17:59 -0300 Subject: [PATCH] fix: pac4j: Add defense-in-depth escaping and startup warning to DevLoginForm fixes #4022 fixes #3996 --- .../io/jooby/internal/pac4j/DevLoginForm.java | 5 +- .../main/java/io/jooby/pac4j/Pac4jModule.java | 16 ++++-- .../internal/pac4j/DevLoginFormTest.java | 52 +++++++++++++++++++ .../java/io/jooby/pac4j/Pac4jModuleTest.java | 40 +++++++++++++- 4 files changed, 106 insertions(+), 7 deletions(-) diff --git a/modules/jooby-pac4j/src/main/java/io/jooby/internal/pac4j/DevLoginForm.java b/modules/jooby-pac4j/src/main/java/io/jooby/internal/pac4j/DevLoginForm.java index 6ca783fbc1..7989762f53 100644 --- a/modules/jooby-pac4j/src/main/java/io/jooby/internal/pac4j/DevLoginForm.java +++ b/modules/jooby-pac4j/src/main/java/io/jooby/internal/pac4j/DevLoginForm.java @@ -11,6 +11,7 @@ import io.jooby.Context; import io.jooby.MediaType; import io.jooby.Route; +import io.jooby.XSS; import io.jooby.pac4j.Pac4jContext; public class DevLoginForm implements Route.Handler { @@ -68,8 +69,8 @@ public DevLoginForm(Config pac4j, String callbackPath) { @Override public Object apply(Context ctx) throws Exception { - String error = ctx.query("error").value(""); - String username = ctx.query("username").value(""); + String error = XSS.html(ctx.query("error").value("")); + String username = XSS.html(ctx.query("username").value("")); ctx.setAttribute("username", username); ctx.setAttribute("error", error); diff --git a/modules/jooby-pac4j/src/main/java/io/jooby/pac4j/Pac4jModule.java b/modules/jooby-pac4j/src/main/java/io/jooby/pac4j/Pac4jModule.java index 7f500d69d4..9a623feb00 100644 --- a/modules/jooby-pac4j/src/main/java/io/jooby/pac4j/Pac4jModule.java +++ b/modules/jooby-pac4j/src/main/java/io/jooby/pac4j/Pac4jModule.java @@ -89,7 +89,7 @@ public void add(String authorizer, Object client) { private final Pac4jOptions options; - private Map clientMap; + private @Nullable Map clientMap; /** Creates a new pac4j module. */ public Pac4jModule() { @@ -353,9 +353,8 @@ public void install(Jooby app) throws Exception { Map> allClients = new LinkedHashMap<>(); /* Should add simple login form? */ - var devLogin = false; - if (clientMap.isEmpty()) { - devLogin = true; + var devLogin = app.getEnvironment().isActive("dev", "test"); + if (clientMap.isEmpty() && devLogin) { client( conf -> new FormClient( @@ -441,6 +440,15 @@ public void install(Jooby app) throws Exception { ofNullable(options.getProfileManagerFactory()).orElse(ProfileManagerFactory.DEFAULT)); if (devLogin) { + var devLoginFormWarning = + !conf.hasPath("pac4j.devLoginFormWarning") + || conf.getBoolean("pac4j.devLoginFormWarning"); + if (devLoginFormWarning) { + app.getLog() + .warn( + "Pac4j module is using the fallback DevLoginForm. This component is for development" + + " purposes only and should NOT be used in production."); + } app.get("/login", new DevLoginForm(options, contextPath + options.getCallbackPath())); } diff --git a/modules/jooby-pac4j/src/test/java/io/jooby/internal/pac4j/DevLoginFormTest.java b/modules/jooby-pac4j/src/test/java/io/jooby/internal/pac4j/DevLoginFormTest.java index d8f6ab7844..5b9c1ac9ab 100644 --- a/modules/jooby-pac4j/src/test/java/io/jooby/internal/pac4j/DevLoginFormTest.java +++ b/modules/jooby-pac4j/src/test/java/io/jooby/internal/pac4j/DevLoginFormTest.java @@ -5,12 +5,16 @@ */ package io.jooby.internal.pac4j; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.*; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.pac4j.core.client.Clients; import org.pac4j.core.config.Config; +import org.pac4j.core.context.WebContext; import org.pac4j.core.http.url.UrlResolver; import io.jooby.Context; @@ -98,4 +102,52 @@ void testApplyWithMissingQueryData() throws Exception { && html.contains("action=\"/callback?client_name=FormClient\"") && html.contains("value=\"\""))); } + + @Test + public void shouldEscapeXssPayloads() throws Exception { + Config config = mock(Config.class); + Clients clients = mock(Clients.class); + UrlResolver urlResolver = mock(UrlResolver.class); + + when(config.getClients()).thenReturn(clients); + when(clients.getUrlResolver()).thenReturn(urlResolver); + when(urlResolver.compute(anyString(), any(WebContext.class))).thenReturn("/callback"); + + Context ctx = mock(Context.class); + Value errorNode = mock(Value.class); + Value userNode = mock(Value.class); + + String xssError = ""; + String xssUser = "\">"; + + when(ctx.query("error")).thenReturn(errorNode); + when(errorNode.value("")).thenReturn(xssError); + + when(ctx.query("username")).thenReturn(userNode); + when(userNode.value("")).thenReturn(xssUser); + + // Mock response chain + when(ctx.setResponseType(MediaType.html)).thenReturn(ctx); + + // Capture the HTML argument passed to ctx.send() and return the ctx mock + ArgumentCaptor htmlCaptor = ArgumentCaptor.forClass(String.class); + when(ctx.send(htmlCaptor.capture())).thenReturn(ctx); + + DevLoginForm form = new DevLoginForm(config, "/callback"); + form.apply(ctx); + + // Retrieve the captured HTML + String html = htmlCaptor.getValue(); + + // Assert inputs were sanitized + assertFalse(html.contains(xssError), "Error payload was rendered unescaped!"); + assertFalse(html.contains(xssUser), "Username payload was rendered unescaped!"); + + // Verify HTML entity encoding occurred (checking < and >) + assertTrue(html.contains("<script>"), "Expected escaped