Always encrypt password (#2085)

First, we make "encryptPassword" in the PasswordService
idempotent, so that the method will not change the password
when the method is called with an already encrypted string.
Then, in the user manager, we will always call this method
to encrypt the password, if this is not already the case.

Co-authored-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
This commit is contained in:
René Pfeuffer
2022-07-15 15:02:52 +02:00
committed by GitHub
parent 8c41fab30d
commit f61d0c113f
6 changed files with 149 additions and 36 deletions

View File

@@ -148,9 +148,8 @@ public class ScmSecurityModule extends ShiroWebModule
*/
private PasswordService createPasswordService()
{
DefaultPasswordService passwordService = new DefaultPasswordService();
DefaultPasswordService passwordService = new IdempotentPasswordService();
DefaultHashService hashService = new DefaultHashService();
hashService.setHashIterations(ITERATIONS);
passwordService.setHashService(hashService);
@@ -161,4 +160,19 @@ public class ScmSecurityModule extends ShiroWebModule
/** Field description */
private final ExtensionProcessor extensionProcessor;
static class IdempotentPasswordService extends DefaultPasswordService {
private boolean isEncrypted(Object password) {
return password instanceof String && ((String) password).startsWith("$shiro1$SHA-512$");
}
@Override
public String encryptPassword(Object plaintext) {
if (isEncrypted(plaintext)) {
return plaintext.toString();
}
return super.encryptPassword(plaintext);
}
}
}

View File

@@ -28,6 +28,7 @@ import com.github.sdorra.ssp.PermissionActionCheck;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.authc.credential.PasswordService;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.ContextEntry;
@@ -36,11 +37,9 @@ import sonia.scm.HandlerEventType;
import sonia.scm.ManagerDaoAdapter;
import sonia.scm.NotFoundException;
import sonia.scm.SCMContextProvider;
import sonia.scm.TransformFilter;
import sonia.scm.search.SearchRequest;
import sonia.scm.search.SearchUtil;
import sonia.scm.security.Authentications;
import sonia.scm.util.CollectionAppender;
import sonia.scm.util.Util;
import java.io.IOException;
@@ -68,14 +67,18 @@ public class DefaultUserManager extends AbstractUserManager
//~--- constructors ---------------------------------------------------------
private final PasswordService passwordService;
/**
* Constructs ...
*
* @param passwordService
* @param userDAO
*/
@Inject
public DefaultUserManager(UserDAO userDAO)
public DefaultUserManager(PasswordService passwordService, UserDAO userDAO)
{
this.passwordService = passwordService;
this.userDAO = userDAO;
this.managerDaoAdapter = new ManagerDaoAdapter<>(userDAO);
}
@@ -125,6 +128,7 @@ public class DefaultUserManager extends AbstractUserManager
}
logger.info("create user {} of type {}", user.getName(), user.getType());
ensurePasswordEncrypted(user);
return managerDaoAdapter.create(
user,
@@ -167,6 +171,7 @@ public class DefaultUserManager extends AbstractUserManager
@Override
public void modify(User user) {
logger.info("modify user {} of type {}", user.getName(), user.getType());
ensurePasswordEncrypted(user);
managerDaoAdapter.modify(
user,
UserPermissions::modify,
@@ -174,6 +179,10 @@ public class DefaultUserManager extends AbstractUserManager
notModified -> fireEvent(HandlerEventType.MODIFY, user, notModified));
}
private void ensurePasswordEncrypted(User user) {
user.setPassword(passwordService.encryptPassword(user.getPassword()));
}
/**
* Method description
*
@@ -364,7 +373,7 @@ public class DefaultUserManager extends AbstractUserManager
throw new InvalidPasswordException(ContextEntry.ContextBuilder.entity("PasswordChange", "-").in(User.class, user.getName()));
}
user.setPassword(newPassword);
user.setPassword(passwordService.encryptPassword(newPassword));
managerDaoAdapter.modify(
user,