Skip to content

Commit

Permalink
Merge pull request #356 from kagemomiji/issue355-fix-insecure-randomness
Browse files Browse the repository at this point in the history
#355 Fix code scanning alert about Insecure Randomness
  • Loading branch information
kagemomiji authored Jan 28, 2024
2 parents 93b40f9 + d2dbab0 commit 2b76c54
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}

0 comments on commit 2b76c54

Please sign in to comment.