Skip to content

Commit

Permalink
KEYCLOAK_SESSION not working for some user federation setups when use…
Browse files Browse the repository at this point in the history
…r ID has special chars (keycloak#14560)

closes keycloak#14354
  • Loading branch information
mposolda authored Oct 5, 2022
1 parent 7ae1fa4 commit c59660c
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 3 deletions.
21 changes: 21 additions & 0 deletions common/src/main/java/org/keycloak/common/util/Encode.java
Original file line number Diff line number Diff line change
Expand Up @@ -570,4 +570,25 @@ public static String decode(String string)
}
}


/**
* @param string
* @return URL encoded input
*/
public static String urlEncode(String string) {
try {
return URLEncoder.encode(string, UTF_8);
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
}

/**
* @param string
* @return URL decoded input
*/
public static String urlDecode(String string) {
return decode(string);
}

}
26 changes: 26 additions & 0 deletions common/src/test/java/org/keycloak/common/util/URLEncodingTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.keycloak.common.util;

import org.junit.Assert;
import org.junit.Test;

/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class URLEncodingTest {

@Test
public void testUrlEncoding() {
Assert.assertEquals("some", Encode.urlEncode("some"));
Assert.assertEquals("330cbb52-c3eb-4c4a-9f23-77a8094cd969", Encode.urlEncode("330cbb52-c3eb-4c4a-9f23-77a8094cd969"));
Assert.assertEquals("sp%C3%A9cial.char", Encode.urlEncode("spécial.char"));
Assert.assertEquals("sp%C3%A9cial.ch%2Far.%C5%BE%C3%BD%C3%A1%C3%A1", Encode.urlEncode("spécial.ch/ar.žýáá"));
}

@Test
public void testUrlDecoding() {
Assert.assertEquals("some", Encode.urlDecode("some"));
Assert.assertEquals("330cbb52-c3eb-4c4a-9f23-77a8094cd969", Encode.urlDecode("330cbb52-c3eb-4c4a-9f23-77a8094cd969"));
Assert.assertEquals("spécial.char", Encode.urlDecode("sp%C3%A9cial.char"));
Assert.assertEquals("spécial.ch/ar.žýáá", Encode.urlDecode("sp%C3%A9cial.ch%2Far.%C5%BE%C3%BD%C3%A1%C3%A1"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.keycloak.common.Profile;
import org.keycloak.common.VerificationException;
import org.keycloak.common.util.Base64Url;
import org.keycloak.common.util.Encode;
import org.keycloak.common.util.SecretGenerator;
import org.keycloak.common.util.Time;
import org.keycloak.crypto.SignatureProvider;
Expand Down Expand Up @@ -784,7 +785,8 @@ public static void createLoginCookie(KeycloakSession keycloakSession, RealmModel
CookieHelper.addCookie(KEYCLOAK_IDENTITY_COOKIE, encoded, cookiePath, null, null, maxAge, secureOnly, true, SameSiteAttributeValue.NONE);
//builder.cookie(new NewCookie(cookieName, encoded, cookiePath, null, null, maxAge, secureOnly));// todo httponly , true);

String sessionCookieValue = realm.getName() + "/" + user.getId();
// With user-storage providers, user ID can contain special characters, which need to be encoded
String sessionCookieValue = realm.getName() + "/" + Encode.urlEncode(user.getId());
if (session != null) {
sessionCookieValue += "/" + session.getId();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.keycloak.testsuite.updaters.RealmAttributeUpdater;
import org.keycloak.testsuite.util.GreenMailRule;
import org.keycloak.testsuite.util.TestCleanup;
import org.openqa.selenium.Cookie;

import javax.mail.internet.MimeMessage;
import javax.ws.rs.NotFoundException;
Expand All @@ -72,14 +73,18 @@
import static java.util.Calendar.DAY_OF_WEEK;
import static java.util.Calendar.HOUR_OF_DAY;
import static java.util.Calendar.MINUTE;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.keycloak.models.UserModel.RequiredAction.UPDATE_PROFILE;
import static org.keycloak.services.managers.AuthenticationManager.KEYCLOAK_SESSION_COOKIE;
import static org.keycloak.services.util.CookieHelper.LEGACY_COOKIE;
import static org.keycloak.storage.UserStorageProviderModel.CACHE_POLICY;
import static org.keycloak.storage.UserStorageProviderModel.EVICTION_DAY;
import static org.keycloak.storage.UserStorageProviderModel.EVICTION_HOUR;
Expand Down Expand Up @@ -242,6 +247,26 @@ public void testLoginSuccess() {
loginBadPassword("tbrady");
}

@Test
public void testLoginSuccessWithSpecialCharacter() {
testRealmAccountPage.navigateTo();
testRealmLoginPage.form().login("spécial", "pw");
assertCurrentUrlStartsWith(testRealmAccountPage);

Cookie sameSiteSessionCookie = driver.manage().getCookieNamed(KEYCLOAK_SESSION_COOKIE);
Cookie legacySessionCookie = driver.manage().getCookieNamed(KEYCLOAK_SESSION_COOKIE + LEGACY_COOKIE);

String cookieValue = sameSiteSessionCookie.getValue();
Assert.assertThat(cookieValue.contains("spécial"), is(false));
Assert.assertThat(cookieValue.contains("sp%C3%A9cial"), is(true));

String legacyCookieValue = legacySessionCookie.getValue();
Assert.assertThat(legacyCookieValue.contains("spécial"), is(false));
Assert.assertThat(legacyCookieValue.contains("sp%C3%A9cial"), is(true));

testRealmAccountPage.logOut();
}

@Test
public void testUpdate() {
UserRepresentation thor = ApiUtil.findUserByUsername(testRealmResource(), "thor");
Expand Down Expand Up @@ -419,11 +444,12 @@ public void testQuery() {
usernames.add(user.getUsername());
log.info(user.getUsername());
}
Assert.assertEquals(8, queried.size());
Assert.assertEquals(9, queried.size());
Assert.assertTrue(usernames.contains("thor"));
Assert.assertTrue(usernames.contains("zeus"));
Assert.assertTrue(usernames.contains("apollo"));
Assert.assertTrue(usernames.contains("perseus"));
Assert.assertTrue(usernames.contains("spécial"));
Assert.assertTrue(usernames.contains("tbrady"));
Assert.assertTrue(usernames.contains("rob"));
Assert.assertTrue(usernames.contains("jules"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
thor=hammer
zeus=pw
apollo=pw
perseus=pw
perseus=pw
spécial=pw

0 comments on commit c59660c

Please sign in to comment.