chibenwa commented on a change in pull request #753:
URL: https://github.com/apache/james-project/pull/753#discussion_r752264506
##########
File path:
server/apps/distributed-app/docs/modules/ROOT/pages/configure/usersrepository.adoc
##########
@@ -39,6 +39,15 @@ acting on the behalf of any user.
| verifyFailureDelay
| Delay after a failed authentication attempt with an invalid user name or
password. Duration string defaulting to seconds, e.g. `2`, `2s`, `2000ms`.
Default `0s` (disabled).
+| algorithm
+| use a specific hash algorithm to compute passwords, e.g. `SHA-512` (default)
+
+| hashingMode
+| change the way password hashes are computed: `plain` (default), `salted`,
`legacy`
Review comment:
HmmmHmm imo salted sould be the default for newer installs to promote
good security practices.
We should then warn about such a change in admin upgrade instructions.
##########
File path:
server/apps/distributed-app/docs/modules/ROOT/pages/configure/usersrepository.adoc
##########
@@ -39,6 +39,15 @@ acting on the behalf of any user.
| verifyFailureDelay
| Delay after a failed authentication attempt with an invalid user name or
password. Duration string defaulting to seconds, e.g. `2`, `2s`, `2000ms`.
Default `0s` (disabled).
+| algorithm
+| use a specific hash algorithm to compute passwords, e.g. `SHA-512` (default)
+
+| hashingMode
+| change the way password hashes are computed: `plain` (default), `salted`,
`legacy`
+
+| hashingModeFallback
Review comment:
I do not get why we need this fallback and when it differ from
hashingMode. Can you develop?
##########
File path:
server/data/data-cassandra/src/main/java/org/apache/james/user/cassandra/CassandraUsersRepository.java
##########
@@ -0,0 +1,42 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.user.cassandra;
+
+import javax.inject.Inject;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.james.domainlist.api.DomainList;
+import org.apache.james.user.lib.UsersRepositoryImpl;
+
+
+public class CassandraUsersRepository extends
UsersRepositoryImpl<CassandraUsersDAO> {
+ @Inject
+ public CassandraUsersRepository(DomainList domainList, CassandraUsersDAO
usersDAO) {
+ super(domainList, usersDAO);
+ }
+
+ @Override
+ public void configure(HierarchicalConfiguration<ImmutableNode> config)
throws ConfigurationException {
+ super.configure(config);
+ usersDAO.configure(config);
Review comment:
👎
1. We tend to associate a pojo to all new confs being written and inject
that pojo. It makes things easier (conf details centralisation, beautiful
builders for the conf to discover options from within an ide, easier injects,
static congiguration)
2. Inheritence should imo be reserved for last resort needs. This do not
sound like the case here.
Why don t we write a pojo summurizing this conf and pass it to
CassandraUserDao constructor?
Then the guice module would be free to parse it from usersrepository.xml
file if it wants, without inheritence.
##########
File path:
server/data/data-library/src/test/java/org/apache/james/user/lib/model/DefaultUserTest.java
##########
@@ -0,0 +1,43 @@
+package org.apache.james.user.lib.model;
+
Review comment:
Top apachev2 license missing. That's compulsory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
