From d2dbab0cf3745c6850ba1fa1fad9b6afe0feac3c Mon Sep 17 00:00:00 2001
From: "Y.Tory" <5343692+kagemomiji@users.noreply.github.com>
Date: Sun, 28 Jan 2024 13:37:06 +0000
Subject: [PATCH] #355 Fix code scanning alert about Insecure Randomness

---
 .../player/service/SecurityService.java       |  7 +-
 .../service/SecurityServiceTestCase.java      | 73 +++++++++++++++++++
 2 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100644 airsonic-main/src/test/java/org/airsonic/player/service/SecurityServiceTestCase.java

diff --git a/airsonic-main/src/main/java/org/airsonic/player/service/SecurityService.java b/airsonic-main/src/main/java/org/airsonic/player/service/SecurityService.java
index 74729f297..77498ff14 100644
--- a/airsonic-main/src/main/java/org/airsonic/player/service/SecurityService.java
+++ b/airsonic-main/src/main/java/org/airsonic/player/service/SecurityService.java
@@ -34,8 +34,8 @@
 import org.airsonic.player.security.GlobalSecurityConfig;
 import org.airsonic.player.security.PasswordDecoder;
 import org.apache.commons.codec.digest.DigestUtils;
-import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.text.RandomStringGenerator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -571,7 +571,10 @@ public void createGuestUserIfNotExists() {
         if (!userRepository.existsById(User.USERNAME_GUEST)) {
             User user = new User(User.USERNAME_GUEST, null);
             user.setRoles(Set.of(Role.STREAM));
-            createUser(user, RandomStringUtils.randomAlphanumeric(30),
+            RandomStringGenerator generator = new RandomStringGenerator.Builder().withinRange('0', 'z')
+                .filteredBy(c -> Character.isLetterOrDigit(c))
+                .build();
+            createUser(user, generator.generate(30),
                     "Autogenerated for " + User.USERNAME_GUEST + " user");
         }
     }
diff --git a/airsonic-main/src/test/java/org/airsonic/player/service/SecurityServiceTestCase.java b/airsonic-main/src/test/java/org/airsonic/player/service/SecurityServiceTestCase.java
new file mode 100644
index 000000000..1dddad241
--- /dev/null
+++ b/airsonic-main/src/test/java/org/airsonic/player/service/SecurityServiceTestCase.java
@@ -0,0 +1,73 @@
+package org.airsonic.player.service;
+
+import org.airsonic.player.domain.MusicFolder;
+import org.airsonic.player.domain.User;
+import org.airsonic.player.domain.UserCredential;
+import org.airsonic.player.repository.UserCredentialRepository;
+import org.airsonic.player.repository.UserRepository;
+import org.airsonic.player.security.GlobalSecurityConfig;
+import org.airsonic.player.security.PasswordDecoder;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyCollection;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+public class SecurityServiceTestCase {
+
+    @Mock
+    private UserCredentialRepository userCredentialRepository;
+
+    @Mock
+    private UserRepository userRepository;
+
+    @Mock
+    private SettingsService settingsService;
+
+    @Mock
+    private MediaFolderService mediaFolderService;
+
+    @InjectMocks
+    private SecurityService securityService;
+
+    @RepeatedTest(10)
+    public void testCreateGuestUserIfNotExists() throws Exception {
+
+        ArgumentCaptor<UserCredential> argumentCaptor = ArgumentCaptor.forClass(UserCredential.class);
+
+        // given
+        when(userRepository.existsById(User.USERNAME_GUEST)).thenReturn(false);
+        // set encoder to hex
+        when(settingsService.getPreferNonDecodablePasswords()).thenReturn(false);
+        when(settingsService.getDecodablePasswordEncoder()).thenReturn("hex");
+        when(userRepository.saveAndFlush(any(User.class))).thenReturn(null);
+        when(userCredentialRepository.saveAndFlush(any(UserCredential.class))).thenReturn(null);
+        when(mediaFolderService.getAllMusicFolders()).thenReturn(List.of(new MusicFolder()));
+
+        // when
+        securityService.createGuestUserIfNotExists();
+
+        // then
+        verify(mediaFolderService).setMusicFoldersForUser(eq(User.USERNAME_GUEST), anyCollection());
+        verify(userCredentialRepository).saveAndFlush(argumentCaptor.capture());
+        UserCredential uc = argumentCaptor.getValue();
+        assertEquals(User.USERNAME_GUEST, uc.getUser().getUsername());
+        assertEquals("hex", uc.getEncoder());
+        PasswordDecoder decoder = (PasswordDecoder) GlobalSecurityConfig.ENCODERS.get("hex");
+        String credential = decoder.decode(uc.getCredential());
+        assertTrue(credential.matches("[0-9A-Za-z]{30}"));
+        assertEquals("Autogenerated for " + User.USERNAME_GUEST + " user", uc.getComment());
+    }
+}