Encrypt user password via CLI (#2080)

gradle/changelog/encrypt_cli_passwords.yaml

@@ -0,0 +1,2 @@
+- type: fixed
+  description: Encrypt passwords stored via cli commands ([#2080](https://github.com/scm-manager/scm-manager/pull/2080))

scm-webapp/src/main/java/sonia/scm/user/cli/UserConvertToInternalCommand.java

@@ -25,6 +25,7 @@
 package sonia.scm.user.cli;
 
 import com.google.common.annotations.VisibleForTesting;
+import org.apache.shiro.authc.credential.PasswordService;
 import picocli.CommandLine;
 import sonia.scm.cli.ParentCommand;
 import sonia.scm.user.User;
@@ -39,6 +40,7 @@ class UserConvertToInternalCommand implements Runnable {
   @CommandLine.Mixin
   private final UserTemplateRenderer templateRenderer;
   private final UserManager manager;
+  private final PasswordService passwordService;
 
   @CommandLine.Parameters(index = "0", paramLabel = "<username>", descriptionKey = "scm.user.username")
   private String username;
@@ -47,9 +49,10 @@ class UserConvertToInternalCommand implements Runnable {
   private String password;
 
   @Inject
-  UserConvertToInternalCommand(UserTemplateRenderer templateRenderer, UserManager manager) {
+  UserConvertToInternalCommand(UserTemplateRenderer templateRenderer, UserManager manager, PasswordService passwordService) {
     this.templateRenderer = templateRenderer;
     this.manager = manager;
+    this.passwordService = passwordService;
   }
 
   @Override
@@ -58,7 +61,7 @@ class UserConvertToInternalCommand implements Runnable {
 
     if (user != null) {
       user.setExternal(false);
-      user.setPassword(password);
+      user.setPassword(passwordService.encryptPassword(password));
       manager.modify(user);
       templateRenderer.render(user);
     } else {

scm-webapp/src/main/java/sonia/scm/user/cli/UserCreateCommand.java

@@ -25,6 +25,7 @@
 package sonia.scm.user.cli;
 
 import com.google.common.annotations.VisibleForTesting;
+import org.apache.shiro.authc.credential.PasswordService;
 import picocli.CommandLine;
 import sonia.scm.cli.CommandValidator;
 import sonia.scm.cli.ParentCommand;
@@ -43,6 +44,7 @@ class UserCreateCommand implements Runnable {
   @CommandLine.Mixin
   private final CommandValidator validator;
   private final UserManager manager;
+  private final PasswordService passwordService;
 
   @CommandLine.Parameters(index = "0", paramLabel = "<username>", descriptionKey = "scm.user.username")
   private String username;
@@ -66,10 +68,11 @@ class UserCreateCommand implements Runnable {
   @Inject
   public UserCreateCommand(UserTemplateRenderer templateRenderer,
                            CommandValidator validator,
-                           UserManager manager) {
+                           UserManager manager, PasswordService passwordService) {
     this.templateRenderer = templateRenderer;
     this.validator = validator;
     this.manager = manager;
+    this.passwordService = passwordService;
   }
 
   @Override
@@ -84,7 +87,7 @@ class UserCreateCommand implements Runnable {
       if (password == null) {
         templateRenderer.renderPasswordError();
       }
-      newUser.setPassword(password);
+      newUser.setPassword(passwordService.encryptPassword(password));
       newUser.setActive(!inactive);
     } else {
       if (inactive) {

scm-webapp/src/main/java/sonia/scm/user/cli/UserModifyCommand.java

@@ -25,6 +25,7 @@
 package sonia.scm.user.cli;
 
 import com.google.common.annotations.VisibleForTesting;
+import org.apache.shiro.authc.credential.PasswordService;
 import picocli.CommandLine;
 import sonia.scm.cli.CommandValidator;
 import sonia.scm.cli.ParentCommand;
@@ -43,6 +44,7 @@ class UserModifyCommand implements Runnable {
   @CommandLine.Mixin
   private final CommandValidator validator;
   private final UserManager manager;
+  private final PasswordService passwordService;
 
   @CommandLine.Parameters(index = "0", paramLabel = "<username>", descriptionKey = "scm.user.username")
   private String username;
@@ -58,10 +60,11 @@ class UserModifyCommand implements Runnable {
   private String password;
 
   @Inject
-  UserModifyCommand(UserTemplateRenderer templateRenderer, CommandValidator validator, UserManager manager) {
+  UserModifyCommand(UserTemplateRenderer templateRenderer, CommandValidator validator, UserManager manager, PasswordService passwordService) {
     this.templateRenderer = templateRenderer;
     this.validator = validator;
     this.manager = manager;
+    this.passwordService = passwordService;
   }
 
   @Override
@@ -78,7 +81,7 @@ class UserModifyCommand implements Runnable {
         user.setMail(email);
       }
       if (password != null) {
-        user.setPassword(password);
+        user.setPassword(passwordService.encryptPassword(password));
       }
       manager.modify(user);
       templateRenderer.render(user);

scm-webapp/src/test/java/sonia/scm/user/cli/UserConvertToInternalCommandTest.java

@@ -24,6 +24,7 @@
 
 package sonia.scm.user.cli;
 
+import org.apache.shiro.authc.credential.PasswordService;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -49,12 +50,14 @@ class UserConvertToInternalCommandTest {
 
   @Mock
   private UserManager manager;
+  @Mock
+  private PasswordService passwordService;
 
   private UserConvertToInternalCommand command;
 
   @BeforeEach
   void initCommand() {
-    command = new UserConvertToInternalCommand(testRenderer.getTemplateRenderer(), manager);
+    command = new UserConvertToInternalCommand(testRenderer.getTemplateRenderer(), manager, passwordService);
   }
 
   @Nested
@@ -73,7 +76,10 @@ class UserConvertToInternalCommandTest {
 
     @Test
     void shouldActivateInternalUser() {
-      command.setPassword("havelock123");
+      String password = "havelock123";
+      String enc_password = "enc_havelock123";
+      when(passwordService.encryptPassword(password)).thenReturn(enc_password);
+      command.setPassword(password);
 
       command.run();
 
@@ -81,7 +87,7 @@ class UserConvertToInternalCommandTest {
         assertThat(argument.getName()).isEqualTo("havelock");
         assertThat(argument.getDisplayName()).isEqualTo("Havelock Vetinari");
         assertThat(argument.isExternal()).isFalse();
-        assertThat(argument.getPassword()).isEqualTo("havelock123");
+        assertThat(argument.getPassword()).isEqualTo(enc_password);
         assertThat(argument.getMail()).isEqualTo("havelock.vetinari@discworld");
         assertThat(argument.isActive()).isTrue();
         return true;

scm-webapp/src/test/java/sonia/scm/user/cli/UserCreateCommandTest.java

@@ -24,6 +24,7 @@
 
 package sonia.scm.user.cli;
 
+import org.apache.shiro.authc.credential.PasswordService;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -40,6 +41,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
@@ -53,16 +55,20 @@ class UserCreateCommandTest {
   private CommandValidator validator;
   @Mock
   private UserManager manager;
+  @Mock
+  private PasswordService passwordService;
 
   private UserCreateCommand command;
 
   @BeforeEach
   void initCommand() {
-    command = new UserCreateCommand(testRenderer.getTemplateRenderer(), validator, manager);
+    command = new UserCreateCommand(testRenderer.getTemplateRenderer(), validator, manager, passwordService);
   }
 
   @Nested
   class ForSuccessfulCreationTest {
+    private static final String PASSWORD = "patrician";
+    private static final String ENC_PASSWORD = "enc_patrician";
 
     @BeforeEach
     void mockCreation() {
@@ -76,6 +82,7 @@ class UserCreateCommandTest {
       command.setUsername("havelock");
       command.setDisplayName("Havelock Vetinari");
       command.setEmail("havelock.vetinari@discworld");
+      lenient().when(passwordService.encryptPassword(PASSWORD)).thenReturn(ENC_PASSWORD);
     }
 
     @Test
@@ -88,7 +95,7 @@ class UserCreateCommandTest {
         assertThat(argument.getName()).isEqualTo("havelock");
         assertThat(argument.getDisplayName()).isEqualTo("Havelock Vetinari");
         assertThat(argument.isExternal()).isFalse();
-        assertThat(argument.getPassword()).isEqualTo("patrician");
+        assertThat(argument.getPassword()).isEqualTo(ENC_PASSWORD);
         assertThat(argument.getMail()).isEqualTo("havelock.vetinari@discworld");
         assertThat(argument.isActive()).isTrue();
         return true;
@@ -114,7 +121,7 @@ class UserCreateCommandTest {
 
     @Test
     void shouldCreateInactiveUser() {
-      command.setPassword("patrician");
+      command.setPassword(PASSWORD);
       command.setInactive(true);
 
       command.run();
@@ -123,7 +130,7 @@ class UserCreateCommandTest {
         assertThat(argument.getName()).isEqualTo("havelock");
         assertThat(argument.getDisplayName()).isEqualTo("Havelock Vetinari");
         assertThat(argument.isExternal()).isFalse();
-        assertThat(argument.getPassword()).isEqualTo("patrician");
+        assertThat(argument.getPassword()).isEqualTo(ENC_PASSWORD);
         assertThat(argument.getMail()).isEqualTo("havelock.vetinari@discworld");
         assertThat(argument.isActive()).isFalse();
         return true;
@@ -133,7 +140,7 @@ class UserCreateCommandTest {
     @Test
     void shouldPrintUserAfterCreationInEnglish() {
       testRenderer.setLocale("en");
-      command.setPassword("patrician");
+      command.setPassword(PASSWORD);
 
       command.run();
 
@@ -153,7 +160,7 @@ class UserCreateCommandTest {
     @Test
     void shouldPrintUserAfterCreationInGerman() {
       testRenderer.setLocale("de");
-      command.setPassword("patrician");
+      command.setPassword(PASSWORD);
 
       command.run();
 
@@ -169,6 +176,23 @@ class UserCreateCommandTest {
         );
       assertThat(testRenderer.getStdErr()).isEmpty();
     }
+
+    @Test
+    void shouldEncryptPassword() {
+      command.setPassword(PASSWORD);
+
+      command.run();
+
+      verify(manager).create(argThat(argument -> {
+        assertThat(argument.getName()).isEqualTo("havelock");
+        assertThat(argument.getDisplayName()).isEqualTo("Havelock Vetinari");
+        assertThat(argument.isExternal()).isFalse();
+        assertThat(argument.getPassword()).isEqualTo(ENC_PASSWORD);
+        assertThat(argument.getMail()).isEqualTo("havelock.vetinari@discworld");
+        assertThat(argument.isActive()).isTrue();
+        return true;
+      }));
+    }
   }
 
   @Nested

scm-webapp/src/test/java/sonia/scm/user/cli/UserModifyCommandTest.java

@@ -24,6 +24,7 @@
 
 package sonia.scm.user.cli;
 
+import org.apache.shiro.authc.credential.PasswordService;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -40,6 +41,7 @@ import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -53,16 +55,20 @@ class UserModifyCommandTest {
   private CommandValidator validator;
   @Mock
   private UserManager manager;
+  @Mock
+  private PasswordService passwordService;
 
   private UserModifyCommand command;
 
   @BeforeEach
   void initCommand() {
-    command = new UserModifyCommand(testRenderer.getTemplateRenderer(), validator, manager);
+    command = new UserModifyCommand(testRenderer.getTemplateRenderer(), validator, manager, passwordService);
   }
 
   @Nested
   class ForSuccessfulModificationTest {
+    private static final String NEW_PASSWORD = "havelock";
+    private static final String NEW_ENC_PASSWORD = "enc_havelock";
 
     @BeforeEach
     void mockGet() {
@@ -73,13 +79,14 @@ class UserModifyCommandTest {
       user.setCreationDate(1649262000000L);
       user.setLastModified(1649272000000L);
       when(manager.get(any())).thenReturn(user);
+      lenient().when(passwordService.encryptPassword(NEW_PASSWORD)).thenReturn(NEW_ENC_PASSWORD);
     }
 
     @Test
     void shouldModifyUser() {
       command.setDisplayName("Lord Vetinari");
       command.setEmail("patrician@discworld");
-      command.setPassword("havelock");
+      command.setPassword(NEW_PASSWORD);
 
       command.run();
 
@@ -87,7 +94,7 @@ class UserModifyCommandTest {
         assertThat(argument.getName()).isEqualTo("havelock");
         assertThat(argument.getDisplayName()).isEqualTo("Lord Vetinari");
         assertThat(argument.isExternal()).isFalse();
-        assertThat(argument.getPassword()).isEqualTo("havelock");
+        assertThat(argument.getPassword()).isEqualTo(NEW_ENC_PASSWORD);
         assertThat(argument.getMail()).isEqualTo("patrician@discworld");
         assertThat(argument.isActive()).isTrue();
         return true;
@@ -97,7 +104,7 @@ class UserModifyCommandTest {
     @Test
     void shouldNotModifyDisplayNameIfNotSet() {
       command.setEmail("patrician@discworld");
-      command.setPassword("havelock");
+      command.setPassword(NEW_PASSWORD);
 
       command.run();
 
@@ -105,7 +112,7 @@ class UserModifyCommandTest {
         assertThat(argument.getName()).isEqualTo("havelock");
         assertThat(argument.getDisplayName()).isEqualTo("Havelock Vetinari");
         assertThat(argument.isExternal()).isFalse();
-        assertThat(argument.getPassword()).isEqualTo("havelock");
+        assertThat(argument.getPassword()).isEqualTo(NEW_ENC_PASSWORD);
         assertThat(argument.getMail()).isEqualTo("patrician@discworld");
         assertThat(argument.isActive()).isTrue();
         return true;
@@ -115,7 +122,7 @@ class UserModifyCommandTest {
     @Test
     void shouldNotModifyEmailIfNotSet() {
       command.setDisplayName("Lord Vetinari");
-      command.setPassword("havelock");
+      command.setPassword(NEW_PASSWORD);
 
       command.run();
 
@@ -123,7 +130,7 @@ class UserModifyCommandTest {
         assertThat(argument.getName()).isEqualTo("havelock");
         assertThat(argument.getDisplayName()).isEqualTo("Lord Vetinari");
         assertThat(argument.isExternal()).isFalse();
-        assertThat(argument.getPassword()).isEqualTo("havelock");
+        assertThat(argument.getPassword()).isEqualTo(NEW_ENC_PASSWORD);
         assertThat(argument.getMail()).isEqualTo("havelock.vetinari@discworld");
         assertThat(argument.isActive()).isTrue();
         return true;