Merge 1168c31e5c into c097c0d7b6

pom.xml

@@ -12,6 +12,8 @@
 
     <properties>
         <version.keycloak>26.0.5</version.keycloak>
+        <version.junit>6.1.0-M1</version.junit>
+        <version.mockito>5.21.0</version.mockito>
     </properties>
 
     <dependencies>
@@ -39,6 +41,30 @@
             <scope>provided</scope>
             <version>${version.keycloak}</version>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <scope>test</scope>
+            <version>${version.junit}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.platform</groupId>
+            <artifactId>junit-platform-commons</artifactId>
+            <scope>test</scope>
+            <version>${version.junit}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+            <version>${version.mockito}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-junit-jupiter</artifactId>
+            <scope>test</scope>
+            <version>${version.mockito}</version>
+        </dependency>
     </dependencies>
 
     <build>

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 <a href="mailto:wadahiro@gmail.com">Hiroyuki Wada</a>
@@ -45,9 +47,13 @@ public class DiscordIdentityProvider extends AbstractOAuth2IdentityProvider<Disc
     public static final String TOKEN_URL = "https://discord.com/api/oauth2/token";
     public static final String PROFILE_URL = "https://discord.com/api/users/@me";
     public static final String GROUP_URL = "https://discord.com/api/users/@me/guilds";
+    public static final String USER_PICTURE_URL = "https://cdn.discordapp.com/avatars/%s/%s.%s?size=%s";
     public static final String DEFAULT_SCOPE = "identify email";
     public static final String GUILDS_SCOPE = "guilds";
 
+    private static final Pattern AVATAR_HASH_PATTERN = Pattern.compile("^(a_)?[0-9a-f]{32}$");
+    private static final Pattern DISCORD_ID_PATTERN = Pattern.compile("^\\d+$");
+
     public DiscordIdentityProvider(KeycloakSession session, DiscordIdentityProviderConfig config) {
         super(session, config);
         config.setAuthorizationUrl(AUTH_URL);
@@ -78,6 +84,8 @@ public class DiscordIdentityProvider extends AbstractOAuth2IdentityProvider<Disc
 
         user.setUsername(username);
         user.setEmail(getJsonProperty(profile, "email"));
+
+        setUserPicture(user, profile);
         user.setIdp(this);
 
         AbstractJsonUserAttributeMapper.storeUserProfileForMapper(user, profile, getConfig().getAlias());
@@ -85,6 +93,35 @@ public class DiscordIdentityProvider extends AbstractOAuth2IdentityProvider<Disc
         return user;
     }
 
+    /**
+     * Constructs the OIDC 'picture' URL based on the Discord avatar hash and sets it
+     * in the user context to ensure OIDC compliance.
+     * <p>Discord returns an avatar hash (or null), but OIDC expects a direct URL
+     * to the image in the 'picture' claim.</p>
+     * @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";
+        }
+        // TODO Image size to be configured via provider config in the future
+        String finalURL = String.format(USER_PICTURE_URL, user.getId(), avatarHash, extension, "256");
+        user.setUserAttribute("picture", finalURL);
+        if (profile instanceof ObjectNode objectNodeProfile) {
+            objectNodeProfile.put("picture", finalURL);
+        }
+    }
+
     @Override
     protected BrokeredIdentityContext doGetFederatedIdentity(String accessToken) {
         log.debug("doGetFederatedIdentity()");
@@ -95,11 +132,10 @@ public class DiscordIdentityProvider extends AbstractOAuth2IdentityProvider<Disc
             throw new IdentityBrokerException("Could not obtain user profile from discord.", e);
         }
 
-        if (getConfig().hasAllowedGuilds()) {
-            if (!isAllowedGuild(accessToken)) {
-                throw new ErrorPageException(session, Response.Status.FORBIDDEN, Messages.INVALID_REQUESTER);
-            }
+        if (getConfig().hasAllowedGuilds() && !isAllowedGuild(accessToken)) {
+            throw new ErrorPageException(session, Response.Status.FORBIDDEN, Messages.INVALID_REQUESTER);
         }
+
         return extractIdentityFromProfile(null, profile);
     }
 

src/main/java/org/keycloak/social/discord/DiscordIdentityProviderConfig.java

@@ -30,6 +30,9 @@ import java.util.stream.Collectors;
  */
 public class DiscordIdentityProviderConfig extends OAuth2IdentityProviderConfig {
 
+    public static final String ALLOWED_GUILDS = "allowedGuilds";
+    public static final String PROMPT = "prompt";
+
     public DiscordIdentityProviderConfig(IdentityProviderModel model) {
         super(model);
     }
@@ -38,27 +41,35 @@ public class DiscordIdentityProviderConfig extends OAuth2IdentityProviderConfig
     }
 
     public String getAllowedGuilds() {
-        return getConfig().get("allowedGuilds");
+        return getConfig().get(ALLOWED_GUILDS);
     }
 
     public void setAllowedGuilds(String allowedGuilds) {
-        getConfig().put("allowedGuilds", 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() {
-        String guilds = getConfig().get("allowedGuilds");
+        String guilds = getConfig().get(ALLOWED_GUILDS);
         return guilds != null && !guilds.trim().isEmpty();
     }
 
     public Set<String> getAllowedGuildsAsSet() {
         if (hasAllowedGuilds()) {
-            String guilds = getConfig().get("allowedGuilds");
-            return Arrays.stream(guilds.split(",")).map(x -> x.trim()).collect(Collectors.toSet());
+            String guilds = getConfig().get(ALLOWED_GUILDS);
+            return Arrays.stream(guilds.split(",")).map(String::trim).collect(Collectors.toSet());
         }
         return Collections.emptySet();
     }
 
     public void setPrompt(String prompt) {
-        getConfig().put("prompt", prompt);
+        getConfig().put(PROMPT, prompt);
     }
 }

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<String> 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());
+    }
+}

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());
+    }
+
+}

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?size=256";
+        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?size=256";
+        assertEquals(expectedURL, user.getUserAttribute("picture"));
+
+    }
+}

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);
+    }
+
+}