You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/03/05 04:38:01 UTC
[GitHub] [james-project] chibenwa opened a new pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
chibenwa opened a new pull request #323:
URL: https://github.com/apache/james-project/pull/323
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r588315677
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -22,36 +22,52 @@
import java.util.Objects;
public class Algorithm {
+ private static final boolean LEGACY = true;
+ public static final String LEGACY_SUFFIX = "-legacy";
+
public static Algorithm of(String rawValue) {
- return new Algorithm(rawValue);
+ if (rawValue.endsWith(LEGACY_SUFFIX)) {
+ return new Algorithm(rawValue, rawValue.substring(0, rawValue.length() - LEGACY_SUFFIX.length()), LEGACY);
Review comment:
Well I do not want to build an abstraction limiting the options offered by Java security implementations.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r588117912
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -22,36 +22,52 @@
import java.util.Objects;
public class Algorithm {
+ private static final boolean LEGACY = true;
+ public static final String LEGACY_SUFFIX = "-legacy";
+
public static Algorithm of(String rawValue) {
- return new Algorithm(rawValue);
+ if (rawValue.endsWith(LEGACY_SUFFIX)) {
+ return new Algorithm(rawValue, rawValue.substring(0, rawValue.length() - LEGACY_SUFFIX.length()), LEGACY);
Review comment:
Because it would be a breaking change?
Because it is not related to the ISSUE addressed?
Because as a user I might want to use something stronger than SHA-512 ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #323:
URL: https://github.com/apache/james-project/pull/323#issuecomment-793590870
@mbaechler Just out of curiosity what did you have in mind?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] mbaechler commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
mbaechler commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r588111096
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * 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.lib.model;
+
+import java.util.Objects;
+
+public class Algorithm {
+ public static Algorithm of(String rawValue) {
+ return new Algorithm(rawValue);
+ }
+
+ private final String rawValue;
+
+ public Algorithm(String rawValue) {
+ this.rawValue = rawValue;
+ }
+
+ public String algorithmName() {
+ return rawValue;
+ }
+
+ public String rawValue() {
Review comment:
`asString`?
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -22,36 +22,52 @@
import java.util.Objects;
public class Algorithm {
+ private static final boolean LEGACY = true;
+ public static final String LEGACY_SUFFIX = "-legacy";
+
public static Algorithm of(String rawValue) {
- return new Algorithm(rawValue);
+ if (rawValue.endsWith(LEGACY_SUFFIX)) {
+ return new Algorithm(rawValue, rawValue.substring(0, rawValue.length() - LEGACY_SUFFIX.length()), LEGACY);
Review comment:
why don't we define a closed list of support Algorithm in an enum instead?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] mbaechler commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
mbaechler commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r588320203
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -22,36 +22,52 @@
import java.util.Objects;
public class Algorithm {
+ private static final boolean LEGACY = true;
+ public static final String LEGACY_SUFFIX = "-legacy";
+
public static Algorithm of(String rawValue) {
- return new Algorithm(rawValue);
+ if (rawValue.endsWith(LEGACY_SUFFIX)) {
+ return new Algorithm(rawValue, rawValue.substring(0, rawValue.length() - LEGACY_SUFFIX.length()), LEGACY);
Review comment:
can we at least avoid to expose the `-legacy` string and use two different factory methods?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r589164917
##########
File path: upgrade-instructions.md
##########
@@ -41,14 +41,15 @@ string being omitted, causing inter-operability issues when synchronizing passwo
New digest algorithm closes the base64 encoding stream resulting in a breaking change forcing user to reset their
password.
-If you wish to avoid this breaking change you can add the `-legacy` suffix to the hash algorithm configured, which will
-default to previous behaviour.
+If you wish to avoid this breaking change you can add the `hashingMode` configuration option in `usersrepository.xml`
+configuration file, with the value `legacy`, which will ensure previous behaviour.
Example `usersrepository.xml`:
```
<usersrepository>
<algorithm>SHA-512-legacy</algorithm>
+ <hashingMode>legacy</hashingMode>
Review comment:
This is more or less what I understood from your remark @mbaechler .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa closed pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
chibenwa closed pull request #323:
URL: https://github.com/apache/james-project/pull/323
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #323:
URL: https://github.com/apache/james-project/pull/323#issuecomment-796457077
Merged
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] mbaechler commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
mbaechler commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r588274728
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -22,36 +22,52 @@
import java.util.Objects;
public class Algorithm {
+ private static final boolean LEGACY = true;
+ public static final String LEGACY_SUFFIX = "-legacy";
+
public static Algorithm of(String rawValue) {
- return new Algorithm(rawValue);
+ if (rawValue.endsWith(LEGACY_SUFFIX)) {
+ return new Algorithm(rawValue, rawValue.substring(0, rawValue.length() - LEGACY_SUFFIX.length()), LEGACY);
Review comment:
`blabla-legacy` is not a breaking change?
I commented on some code that didn't exist before this PR, so debating the design is part of the issue AFAIC.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r588331321
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -22,36 +22,52 @@
import java.util.Objects;
public class Algorithm {
+ private static final boolean LEGACY = true;
+ public static final String LEGACY_SUFFIX = "-legacy";
+
public static Algorithm of(String rawValue) {
- return new Algorithm(rawValue);
+ if (rawValue.endsWith(LEGACY_SUFFIX)) {
+ return new Algorithm(rawValue, rawValue.substring(0, rawValue.length() - LEGACY_SUFFIX.length()), LEGACY);
Review comment:
> can we at least avoid to expose the -legacy string
How would you do to achieve that?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a change in pull request #323: JAMES-3512 DigestUtil: close base64 encoding stream
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #323:
URL: https://github.com/apache/james-project/pull/323#discussion_r589171364
##########
File path: server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java
##########
@@ -19,19 +19,57 @@
package org.apache.james.user.lib.model;
+import java.util.Arrays;
import java.util.Objects;
+import com.google.common.collect.ImmutableList;
+
public class Algorithm {
- private static final boolean LEGACY = true;
- public static final String LEGACY_SUFFIX = "-legacy";
- public static Algorithm of(String rawValue) {
- if (rawValue.endsWith(LEGACY_SUFFIX)) {
- return new Algorithm(rawValue, rawValue.substring(0, rawValue.length() - LEGACY_SUFFIX.length()), LEGACY);
+ public enum HashingMode {
+ LEGACY(LEGACY_FACTORY),
+ DEFAULT(DEFAULT_FACTORY);
+
+ public static HashingMode parse(String value) {
+ return Arrays.stream(values())
+ .filter(aValue -> value.equalsIgnoreCase(aValue.toString()))
+ .findFirst()
+ .orElseThrow(() -> new IllegalArgumentException("Unsuported value for HashingMode: " + value + ". Should be one of " + ImmutableList.copyOf(values())));
+ }
+
+ private final Factory factory;
+
+ HashingMode(Factory factory) {
+ this.factory = factory;
+ }
+
+ public Factory getFactory() {
+ return factory;
+ }
+ }
+
+ public interface Factory {
+ Algorithm of(String rawValue);
+ }
+
+ public static class LegacyFactory implements Factory {
+ @Override
+ public Algorithm of(String rawValue) {
+ return new Algorithm(rawValue, rawValue, LEGACY);
Review comment:
If the two arguments for Algorithm `rawValue` and `algorithmName` are the same now, it's probably better to drop one?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org