KEYCLOAK-15259 Avoid using "null" Origin header as a valid value

This commit is contained in:
mhajas 2020-08-25 16:05:16 +02:00 committed by Stian Thorgersen
parent f7e0af438d
commit b75ad2fbd8
7 changed files with 42 additions and 19 deletions

View file

@ -89,7 +89,8 @@ public class AuthenticatedActionsHandler {
return true; return true;
} }
// Don't allow a CORS request if we're not validating CORS requests. // Don't allow a CORS request if we're not validating CORS requests.
if (!deployment.isCors() && facade.getRequest().getHeader(CorsHeaders.ORIGIN) != null) { String origin = facade.getRequest().getHeader(CorsHeaders.ORIGIN);
if (!deployment.isCors() && origin != null && !origin.equals("null")) {
facade.getResponse().setStatus(200); facade.getResponse().setStatus(200);
facade.getResponse().end(); facade.getResponse().end();
return true; return true;
@ -101,6 +102,7 @@ public class AuthenticatedActionsHandler {
if (!deployment.isCors()) return false; if (!deployment.isCors()) return false;
KeycloakSecurityContext securityContext = facade.getSecurityContext(); KeycloakSecurityContext securityContext = facade.getSecurityContext();
String origin = facade.getRequest().getHeader(CorsHeaders.ORIGIN); String origin = facade.getRequest().getHeader(CorsHeaders.ORIGIN);
origin = "null".equals(origin) ? null : origin;
String exposeHeaders = deployment.getCorsExposedHeaders(); String exposeHeaders = deployment.getCorsExposedHeaders();
if (deployment.getPolicyEnforcer() != null) { if (deployment.getPolicyEnforcer() != null) {

View file

@ -103,13 +103,13 @@ public class PreAuthActionsHandler {
if (!facade.getRequest().getMethod().equalsIgnoreCase("OPTIONS")) { if (!facade.getRequest().getMethod().equalsIgnoreCase("OPTIONS")) {
return false; return false;
} }
if (facade.getRequest().getHeader(CorsHeaders.ORIGIN) == null) { String origin = facade.getRequest().getHeader(CorsHeaders.ORIGIN);
if (origin == null || origin.equals("null")) {
log.debug("checkCorsPreflight: no origin header"); log.debug("checkCorsPreflight: no origin header");
return false; return false;
} }
log.debug("Preflight request returning"); log.debug("Preflight request returning");
facade.getResponse().setStatus(200); facade.getResponse().setStatus(200);
String origin = facade.getRequest().getHeader(CorsHeaders.ORIGIN);
facade.getResponse().setHeader(CorsHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, origin); facade.getResponse().setHeader(CorsHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, origin);
facade.getResponse().setHeader(CorsHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS, "true"); facade.getResponse().setHeader(CorsHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS, "true");
String requestMethods = facade.getRequest().getHeader(CorsHeaders.ACCESS_CONTROL_REQUEST_METHOD); String requestMethods = facade.getRequest().getHeader(CorsHeaders.ACCESS_CONTROL_REQUEST_METHOD);

View file

@ -137,7 +137,7 @@ public class Cors {
public Response build() { public Response build() {
String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN_HEADER); String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN_HEADER);
if (origin == null) { if (origin == null || origin.equals("null")) {
logger.trace("No origin header ignoring"); logger.trace("No origin header ignoring");
return builder.build(); return builder.build();
} }
@ -184,7 +184,7 @@ public class Cors {
public void build(HttpResponse response) { public void build(HttpResponse response) {
String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN_HEADER); String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN_HEADER);
if (origin == null) { if (origin == null || origin.equals("null")) {
logger.trace("No origin header ignoring"); logger.trace("No origin header ignoring");
return; return;
} }

View file

@ -157,7 +157,7 @@ public class AccountFormService extends AbstractSecuredLocalService {
String requestOrigin = UriUtils.getOrigin(session.getContext().getUri().getBaseUri()); String requestOrigin = UriUtils.getOrigin(session.getContext().getUri().getBaseUri());
String origin = headers.getRequestHeaders().getFirst("Origin"); String origin = headers.getRequestHeaders().getFirst("Origin");
if (origin != null && !requestOrigin.equals(origin)) { if (origin != null && !origin.equals("null") && !requestOrigin.equals(origin)) {
throw new ForbiddenException(); throw new ForbiddenException();
} }

View file

@ -80,9 +80,13 @@ import java.util.Map;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import org.junit.Assume; import org.junit.Assume;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
@ -1152,8 +1156,8 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest {
Assert.assertTrue(sessionsPage.isCurrent()); Assert.assertTrue(sessionsPage.isCurrent());
List<List<String>> sessions = sessionsPage.getSessions(); List<List<String>> sessions = sessionsPage.getSessions();
Assert.assertEquals(1, sessions.size()); assertThat(sessions, hasSize(1));
Assert.assertEquals("127.0.0.1", sessions.get(0).get(0)); assertThat(sessions.get(0).get(0), anyOf(equalTo("127.0.0.1"), equalTo("0:0:0:0:0:0:0:1")));
// Create second session // Create second session
try { try {

View file

@ -16,10 +16,13 @@
*/ */
package org.keycloak.testsuite.account; package org.keycloak.testsuite.account;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
import java.io.IOException; import java.io.IOException;
import java.util.List; import java.util.List;
@ -132,18 +135,20 @@ public class SessionRestServiceTest extends AbstractRestServiceTest {
for (SessionRepresentation session : sessions) { for (SessionRepresentation session : sessions) {
assertNotNull(session.getId()); assertNotNull(session.getId());
assertEquals("127.0.0.1", session.getIpAddress()); assertThat(session.getIpAddress(), anyOf(equalTo("127.0.0.1"), equalTo("0:0:0:0:0:0:0:1")));
assertTrue(session.getLastAccess() > 0); assertTrue(session.getLastAccess() > 0);
assertTrue(session.getExpires() > 0); assertTrue(session.getExpires() > 0);
assertTrue(session.getStarted() > 0); assertTrue(session.getStarted() > 0);
assertThat(session.getClients(), Matchers.hasItem(Matchers.hasProperty("clientId", assertThat(session.getClients(), Matchers.hasItem(Matchers.hasProperty("clientId",
Matchers.anyOf(Matchers.is("direct-grant"), Matchers.is("public-client-0"))))); anyOf(Matchers.is("direct-grant"), Matchers.is("public-client-0")))));
} }
} }
@Test @Test
@AuthServerContainerExclude(AuthServer.REMOTE) @AuthServerContainerExclude(AuthServer.REMOTE)
public void testGetDevicesResponse() throws Exception { public void testGetDevicesResponse() throws Exception {
assumeTrue("Browser must be htmlunit. Otherwise we are not able to set desired BrowserHeaders",
System.getProperty("browser").equals("htmlUnit"));
oauth.setBrowserHeader("User-Agent", "Mozilla/5.0 (Windows NT 10.0) Gecko/20100101 Firefox/15.0.1"); oauth.setBrowserHeader("User-Agent", "Mozilla/5.0 (Windows NT 10.0) Gecko/20100101 Firefox/15.0.1");
OAuthClient.AccessTokenResponse tokenResponse = codeGrant("public-client-0"); OAuthClient.AccessTokenResponse tokenResponse = codeGrant("public-client-0");
joinSsoSession("public-client-1"); joinSsoSession("public-client-1");
@ -168,14 +173,17 @@ public class SessionRestServiceTest extends AbstractRestServiceTest {
List<ClientRepresentation> clients = session.getClients(); List<ClientRepresentation> clients = session.getClients();
assertEquals(2, clients.size()); assertEquals(2, clients.size());
assertThat(session.getClients(), Matchers.hasItem(Matchers.hasProperty("clientId", assertThat(session.getClients(), Matchers.hasItem(Matchers.hasProperty("clientId",
Matchers.anyOf(Matchers.is("public-client-0"), Matchers.is("public-client-1"))))); anyOf(Matchers.is("public-client-0"), Matchers.is("public-client-1")))));
assertThat(session.getClients(), Matchers.hasItem(Matchers.hasProperty("clientName", assertThat(session.getClients(), Matchers.hasItem(Matchers.hasProperty("clientName",
Matchers.anyOf(Matchers.is("Public Client 0"), Matchers.is("Public Client 1"))))); anyOf(Matchers.is("Public Client 0"), Matchers.is("Public Client 1")))));
} }
@Test @Test
public void testGetDevicesSessions() throws Exception { public void testGetDevicesSessions() throws Exception {
ContainerAssume.assumeAuthServerUndertow(); ContainerAssume.assumeAuthServerUndertow();
assumeTrue("Browser must be htmlunit. Otherwise we are not able to set desired BrowserHeaders",
System.getProperty("browser").equals("htmlUnit"));
WebDriver firstBrowser = oauth.getDriver(); WebDriver firstBrowser = oauth.getDriver();
// first browser authenticates from Fedora // first browser authenticates from Fedora
@ -343,6 +351,9 @@ public class SessionRestServiceTest extends AbstractRestServiceTest {
@Test @Test
@AuthServerContainerExclude(AuthServer.REMOTE) @AuthServerContainerExclude(AuthServer.REMOTE)
public void testNullOrEmptyUserAgent() throws Exception { public void testNullOrEmptyUserAgent() throws Exception {
assumeTrue("Browser must be htmlunit. Otherwise we are not able to set desired BrowserHeaders",
System.getProperty("browser").equals("htmlUnit"));
oauth.setBrowserHeader("User-Agent", null); oauth.setBrowserHeader("User-Agent", null);
OAuthClient.AccessTokenResponse tokenResponse = codeGrant("public-client-0"); OAuthClient.AccessTokenResponse tokenResponse = codeGrant("public-client-0");
@ -367,6 +378,9 @@ public class SessionRestServiceTest extends AbstractRestServiceTest {
@Test @Test
public void testNonBrowserSession() throws Exception { public void testNonBrowserSession() throws Exception {
assumeTrue("Browser must be htmlunit. Otherwise we are not able to set desired BrowserHeaders",
System.getProperty("browser").equals("htmlUnit"));
// one device // one device
oauth.setBrowserHeader("User-Agent", "Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0.1"); oauth.setBrowserHeader("User-Agent", "Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0.1");
codeGrant("public-client-0"); codeGrant("public-client-0");
@ -382,7 +396,7 @@ public class SessionRestServiceTest extends AbstractRestServiceTest {
assertEquals(2, devices.size()); assertEquals(2, devices.size());
assertThat(devices, assertThat(devices,
Matchers.hasItems(Matchers.hasProperty("os", Matchers.anyOf(Matchers.is("Fedora"), Matchers.is("Other"))))); Matchers.hasItems(Matchers.hasProperty("os", anyOf(Matchers.is("Fedora"), Matchers.is("Other")))));
// three because tests use another client when booting tests // three because tests use another client when booting tests
assertEquals(3, devices.stream().filter(deviceRepresentation -> "Other".equals(deviceRepresentation.getOs())) assertEquals(3, devices.stream().filter(deviceRepresentation -> "Other".equals(deviceRepresentation.getOs()))

View file

@ -18,6 +18,7 @@
package org.keycloak.testsuite.forms; package org.keycloak.testsuite.forms;
import org.hamcrest.Matchers;
import org.jboss.arquillian.graphene.page.Page; import org.jboss.arquillian.graphene.page.Page;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
@ -52,13 +53,17 @@ import org.keycloak.testsuite.util.GreenMailRule;
import org.keycloak.testsuite.util.MailUtils; import org.keycloak.testsuite.util.MailUtils;
import org.keycloak.testsuite.util.URLUtils; import org.keycloak.testsuite.util.URLUtils;
import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.UserBuilder;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import javax.mail.internet.MimeMessage; import javax.mail.internet.MimeMessage;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import java.util.stream.Collectors;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.REMOTE; import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.REMOTE;
/** /**
@ -434,8 +439,8 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
pageSource = driver.getPageSource(); pageSource = driver.getPageSource();
// Check if OTP credential with empty label was created successfully // Check if OTP credential with empty label was created successfully
final String emptyOtpLabelPresentInAuthenticatorTable = "(?s)<td class=\"provider\"/>"; assertThat(driver.findElements(By.className("provider")).stream()
Assert.assertTrue(Pattern.compile(emptyOtpLabelPresentInAuthenticatorTable).matcher(pageSource).find()); .map(WebElement::getText).collect(Collectors.toList()), Matchers.hasItem(""));
accountTotpPage.removeTotp(); accountTotpPage.removeTotp();
// Logout // Logout
@ -473,11 +478,9 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
accountTotpPage.open(); accountTotpPage.open();
Assert.assertTrue(accountTotpPage.isCurrent()); Assert.assertTrue(accountTotpPage.isCurrent());
// Get the updated Account TOTP page source post OTP credential creation
pageSource = driver.getPageSource();
// Check if OTP credential with empty label was created successfully // Check if OTP credential with empty label was created successfully
Assert.assertTrue(Pattern.compile(emptyOtpLabelPresentInAuthenticatorTable).matcher(pageSource).find()); assertThat(driver.findElements(By.className("provider")).stream()
.map(WebElement::getText).collect(Collectors.toList()), Matchers.hasItem(""));;
// Logout // Logout
oauth.openLogout(); oauth.openLogout();