From 5e4e84558ca8fc0876b9240f7a3452cf64adfa1e Mon Sep 17 00:00:00 2001 From: Kevin Paul Date: Mon, 22 Dec 2025 13:37:51 +0100 Subject: [PATCH 1/4] feat: Add OIDC compliant 'picture' attribute mapping --- .../discord/DiscordIdentityProvider.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java b/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java index b14fc3e..e346ab4 100755 --- a/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java +++ b/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java @@ -18,6 +18,7 @@ package org.keycloak.social.discord; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; import jakarta.ws.rs.core.Response; import org.jboss.logging.Logger; import org.keycloak.broker.oidc.AbstractOAuth2IdentityProvider; @@ -32,6 +33,7 @@ import org.keycloak.services.ErrorPageException; import org.keycloak.services.messages.Messages; import java.util.Set; +import java.util.regex.Pattern; /** * @author Hiroyuki Wada @@ -45,9 +47,13 @@ public class DiscordIdentityProvider extends AbstractOAuth2IdentityProviderDiscord returns an avatar hash (or null), but OIDC expects a direct URL + * to the image in the 'picture' claim.

+ * @param user The context where the 'picture' attribute will be set + * @param profile The raw Discord user profile JSON containing the 'avatar' hash + */ + private void setUserPicture(BrokeredIdentityContext user, JsonNode profile) { + if (user.getId() == null || !DISCORD_ID_PATTERN.matcher(user.getId()).matches()) { + return; + } + + String avatarHash = getJsonProperty(profile, "avatar"); + if (avatarHash == null || avatarHash.isEmpty() || !AVATAR_HASH_PATTERN.matcher(avatarHash).matches()) { + return; + } + String extension = "png"; + if (avatarHash.startsWith("a_")) { + extension = "gif"; + } + String finalURL = String.format(USER_PICTURE_URL, user.getId(), avatarHash, extension); + user.setUserAttribute("picture", finalURL); + if (profile instanceof ObjectNode objectNodeProfile) { + objectNodeProfile.put("picture", finalURL); + } + } + @Override protected BrokeredIdentityContext doGetFederatedIdentity(String accessToken) { log.debug("doGetFederatedIdentity()"); From 585a7ed5777d2b8b4e5999ce6fc1b058d2545300 Mon Sep 17 00:00:00 2001 From: Kevin Paul Date: Mon, 22 Dec 2025 14:57:26 +0100 Subject: [PATCH 2/4] refactor: use constants and simplify conditional logic --- .../social/discord/DiscordIdentityProvider.java | 7 +++---- .../discord/DiscordIdentityProviderConfig.java | 13 ++++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java b/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java index e346ab4..bc03d7e 100755 --- a/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java +++ b/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java @@ -131,11 +131,10 @@ public class DiscordIdentityProvider extends AbstractOAuth2IdentityProvider getAllowedGuildsAsSet() { if (hasAllowedGuilds()) { - String guilds = getConfig().get("allowedGuilds"); + String guilds = getConfig().get(ALLOWED_GUILDS); return Arrays.stream(guilds.split(",")).map(x -> x.trim()).collect(Collectors.toSet()); } return Collections.emptySet(); } public void setPrompt(String prompt) { - getConfig().put("prompt", prompt); + getConfig().put(PROMPT, prompt); } } From 88297e42a7a0c9de1c1fc42704c8af8303641f32 Mon Sep 17 00:00:00 2001 From: Kevin Paul Date: Mon, 22 Dec 2025 16:50:25 +0100 Subject: [PATCH 3/4] fix: handle malformed guild config strings & add unit tests --- pom.xml | 26 ++++ .../DiscordIdentityProviderConfig.java | 12 +- .../DiscordIdentityProviderConfigTest.java | 68 +++++++++ .../DiscordIdentityProviderFactoryTest.java | 18 +++ .../discord/DiscordIdentityProviderTest.java | 133 ++++++++++++++++++ .../DiscordUserAttributeMapperTest.java | 16 +++ 6 files changed, 271 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/keycloak/social/discord/DiscordIdentityProviderConfigTest.java create mode 100644 src/test/java/org/keycloak/social/discord/DiscordIdentityProviderFactoryTest.java create mode 100644 src/test/java/org/keycloak/social/discord/DiscordIdentityProviderTest.java create mode 100644 src/test/java/org/keycloak/social/discord/DiscordUserAttributeMapperTest.java diff --git a/pom.xml b/pom.xml index 6aed171..06a1616 100755 --- a/pom.xml +++ b/pom.xml @@ -12,6 +12,8 @@ 26.0.5 + 6.1.0-M1 + 5.21.0 @@ -39,6 +41,30 @@ provided ${version.keycloak} + + org.junit.jupiter + junit-jupiter + test + ${version.junit} + + + org.junit.platform + junit-platform-commons + test + ${version.junit} + + + org.mockito + mockito-core + test + ${version.mockito} + + + org.mockito + mockito-junit-jupiter + test + ${version.mockito} + diff --git a/src/main/java/org/keycloak/social/discord/DiscordIdentityProviderConfig.java b/src/main/java/org/keycloak/social/discord/DiscordIdentityProviderConfig.java index 5ad9ead..b402c1b 100755 --- a/src/main/java/org/keycloak/social/discord/DiscordIdentityProviderConfig.java +++ b/src/main/java/org/keycloak/social/discord/DiscordIdentityProviderConfig.java @@ -45,7 +45,15 @@ public class DiscordIdentityProviderConfig extends OAuth2IdentityProviderConfig } public void setAllowedGuilds(String allowedGuilds) { - getConfig().put(ALLOWED_GUILDS, allowedGuilds); + if (allowedGuilds != null) { + String cleanGuilds = Arrays.stream(allowedGuilds.split(",")) + .map(String::trim) + .filter(s -> !s.isBlank()) + .collect(Collectors.joining(",")); + getConfig().put(ALLOWED_GUILDS, cleanGuilds); + } else { + getConfig().put(ALLOWED_GUILDS, null); + } } public boolean hasAllowedGuilds() { @@ -56,7 +64,7 @@ public class DiscordIdentityProviderConfig extends OAuth2IdentityProviderConfig public Set getAllowedGuildsAsSet() { if (hasAllowedGuilds()) { String guilds = getConfig().get(ALLOWED_GUILDS); - return Arrays.stream(guilds.split(",")).map(x -> x.trim()).collect(Collectors.toSet()); + return Arrays.stream(guilds.split(",")).map(String::trim).collect(Collectors.toSet()); } return Collections.emptySet(); } diff --git a/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderConfigTest.java b/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderConfigTest.java new file mode 100644 index 0000000..b154bba --- /dev/null +++ b/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderConfigTest.java @@ -0,0 +1,68 @@ +package org.keycloak.social.discord; + +import org.junit.jupiter.api.Test; +import org.keycloak.models.IdentityProviderModel; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +class DiscordIdentityProviderConfigTest { + + @Test + void given_noAllowedGuilds_expect_emptyResult() { + IdentityProviderModel model = new IdentityProviderModel(); + DiscordIdentityProviderConfig config = new DiscordIdentityProviderConfig(model); + + assertFalse(config.hasAllowedGuilds()); + + config.setAllowedGuilds(""); + assertFalse(config.hasAllowedGuilds()); + assertTrue(config.getAllowedGuildsAsSet().isEmpty()); + + config.setAllowedGuilds(" "); + assertFalse(config.hasAllowedGuilds()); + assertTrue(config.getAllowedGuildsAsSet().isEmpty()); + + config.setAllowedGuilds(",,,"); + assertFalse(config.hasAllowedGuilds()); + assertTrue(config.getAllowedGuildsAsSet().isEmpty()); + + config.setAllowedGuilds(", ,, , ,"); + assertFalse(config.hasAllowedGuilds()); + assertTrue(config.getAllowedGuildsAsSet().isEmpty()); + } + + @Test + void given_allowedGuildsPresent_expect_results() { + List guilds = List.of("0123456789123456789", "9876543210987654321"); + String guildsAsString = "0123456789123456789,9876543210987654321"; + + IdentityProviderModel model = new IdentityProviderModel(); + DiscordIdentityProviderConfig config = new DiscordIdentityProviderConfig(model); + + String expectedGuild = guilds.get(0); + config.setAllowedGuilds(expectedGuild); + assertTrue(config.hasAllowedGuilds()); + assertEquals(1, config.getAllowedGuildsAsSet().size()); + assertTrue(config.getAllowedGuildsAsSet().contains(expectedGuild)); + assertEquals(expectedGuild, config.getAllowedGuilds()); + + config.setAllowedGuilds("," + expectedGuild + ",,"); + assertTrue(config.hasAllowedGuilds()); + assertEquals(1, config.getAllowedGuildsAsSet().size()); + assertTrue(config.getAllowedGuildsAsSet().contains(expectedGuild)); + + config.setAllowedGuilds(String.join(",", guilds)); + assertTrue(config.hasAllowedGuilds()); + assertEquals(guilds.size(), config.getAllowedGuildsAsSet().size()); + assertTrue(config.getAllowedGuildsAsSet().containsAll(guilds)); + assertEquals(guildsAsString, config.getAllowedGuilds()); + + config.setAllowedGuilds(String.join(", ,, , ,", guilds)); + assertTrue(config.hasAllowedGuilds()); + assertEquals(guilds.size(), config.getAllowedGuildsAsSet().size()); + assertTrue(config.getAllowedGuildsAsSet().containsAll(guilds)); + assertEquals(guildsAsString, config.getAllowedGuilds()); + } +} \ No newline at end of file diff --git a/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderFactoryTest.java b/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderFactoryTest.java new file mode 100644 index 0000000..120543a --- /dev/null +++ b/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderFactoryTest.java @@ -0,0 +1,18 @@ +package org.keycloak.social.discord; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class DiscordIdentityProviderFactoryTest { + + @Test + void getName() { + DiscordIdentityProviderFactory factory = new DiscordIdentityProviderFactory(); + + String name = factory.getName(); + assertNotNull(name); + assertNotEquals(0, name.length()); + } + +} \ No newline at end of file diff --git a/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderTest.java b/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderTest.java new file mode 100644 index 0000000..3f3b3aa --- /dev/null +++ b/src/test/java/org/keycloak/social/discord/DiscordIdentityProviderTest.java @@ -0,0 +1,133 @@ +package org.keycloak.social.discord; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.keycloak.broker.provider.BrokeredIdentityContext; +import org.keycloak.models.KeycloakSession; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DiscordIdentityProviderTest { + + DiscordIdentityProvider provider; + + ObjectMapper mapper; + + @Mock + KeycloakSession session; + + @Mock + DiscordIdentityProviderConfig config; + + @BeforeEach + void setUp() { + when(config.getAlias()).thenReturn("discord"); + + provider = new DiscordIdentityProvider(session, config); + + mapper = new ObjectMapper(); + } + + @Test + void testNameDiscriminatorHandling() throws Exception { + String jsonProfileLegacy = """ +{ + "id": "80351110224678912", + "username": "Nelly", + "discriminator": "1337", + "email": "nelly@discord.com" +} + """; + JsonNode profile = mapper.readTree(jsonProfileLegacy); + BrokeredIdentityContext user = provider.extractIdentityFromProfile(null, profile); + + assertEquals("80351110224678912", user.getId()); + assertEquals("nelly#1337", user.getUsername()); + + } + + @Test + void testNameDiscriminatorHandling_NoDiscriminator() throws Exception { + String jsonProfileNew = """ +{ + "id": "80351110224678912", + "username": "nelly", + "discriminator": "0", + "email": "nelly@discord.com" +} + """; + JsonNode profile = mapper.readTree(jsonProfileNew); + BrokeredIdentityContext user = provider.extractIdentityFromProfile(null, profile); + + assertEquals("80351110224678912", user.getId()); + assertEquals("nelly", user.getUsername()); + assertEquals("nelly@discord.com", user.getEmail()); + + } + + @Test + void testExtractIdentity_NoAvatar() throws Exception { + String jsonProfile = """ +{ + "id": "80351110224678912", + "username": "nelly", + "discriminator": "0", + "email": "nelly@discord.com" +} + """; + JsonNode profile = mapper.readTree(jsonProfile); + + BrokeredIdentityContext user = provider.extractIdentityFromProfile(null, profile); + + assertNull(user.getUserAttribute("picture")); + + } + + @Test + void testExtractIdentity_WithStaticAvatar() throws Exception { + String jsonProfile = """ +{ + "id": "80351110224678912", + "username": "nelly", + "discriminator": "0", + "avatar": "8342729096ea3675442027381ff50dfe", + "email": "nelly@discord.com" +} + """; + JsonNode profile = mapper.readTree(jsonProfile); + + BrokeredIdentityContext user = provider.extractIdentityFromProfile(null, profile); + + String expectedURL = "https://cdn.discordapp.com/avatars/80351110224678912/8342729096ea3675442027381ff50dfe.png"; + assertEquals(expectedURL, user.getUserAttribute("picture")); + + } + + @Test + void testExtractIdentity_WithAnimatedAvatar() throws Exception { + String jsonProfile = """ +{ + "id": "80351110224678912", + "username": "nelly", + "discriminator": "0", + "avatar": "a_8342729096ea3675442027381ff50dfe", + "email": "nelly@discord.com" +} + """; + JsonNode profile = mapper.readTree(jsonProfile); + + BrokeredIdentityContext user = provider.extractIdentityFromProfile(null, profile); + + String expectedURL = "https://cdn.discordapp.com/avatars/80351110224678912/a_8342729096ea3675442027381ff50dfe.gif"; + assertEquals(expectedURL, user.getUserAttribute("picture")); + + } +} \ No newline at end of file diff --git a/src/test/java/org/keycloak/social/discord/DiscordUserAttributeMapperTest.java b/src/test/java/org/keycloak/social/discord/DiscordUserAttributeMapperTest.java new file mode 100644 index 0000000..b97dac3 --- /dev/null +++ b/src/test/java/org/keycloak/social/discord/DiscordUserAttributeMapperTest.java @@ -0,0 +1,16 @@ +package org.keycloak.social.discord; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class DiscordUserAttributeMapperTest { + @Test + void getCompatibleProviders() { + DiscordUserAttributeMapper mapper = new DiscordUserAttributeMapper(); + String[] providers = mapper.getCompatibleProviders(); + assertNotNull(providers); + assertNotEquals(0, providers.length); + } + +} \ No newline at end of file From 1168c31e5c4a3534d31adfb49875373eee08ab71 Mon Sep 17 00:00:00 2001 From: Kevin Paul Date: Mon, 22 Dec 2025 17:15:47 +0100 Subject: [PATCH 4/4] refactor: hardcode avatar size to 256px and add TODO --- .../org/keycloak/social/discord/DiscordIdentityProvider.java | 5 +++-- .../keycloak/social/discord/DiscordIdentityProviderTest.java | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java b/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java index bc03d7e..115370c 100755 --- a/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java +++ b/src/main/java/org/keycloak/social/discord/DiscordIdentityProvider.java @@ -47,7 +47,7 @@ public class DiscordIdentityProvider extends AbstractOAuth2IdentityProvider