chibenwa commented on a change in pull request #788:
URL: https://github.com/apache/james-project/pull/788#discussion_r767406310



##########
File path: 
server/apps/distributed-app/docs/modules/ROOT/pages/configure/imap.adoc
##########
@@ -44,7 +44,14 @@ Defaults to 10MB. Must be a positive integer, optionally 
with a unit: B, K, M, G
 Defaults to 0 (unlimited). Must be a positive integer, optionally with a unit: 
B, K, M, G.
 
 | plainAuthDisallowed
-| Whether to enable Authentication PLAIN if the connection is not encrypted 
via SSL or STARTTLS. Defaults to `true`.
+| Deprecated. Should use `auth.plainAuthEnabled`, `auth.requireSSL` instead.
+Whether to enable Authentication PLAIN if the connection is not encrypted via 
SSL or STARTTLS. Defaults to `true`.
+
+| auth.plainAuthEnabled
+| true or false. Defaults to `true`.

Review comment:
       Please expplain what this option does

##########
File path: src/site/xdoc/server/config-imap4.xml
##########
@@ -84,7 +84,11 @@
         <dd>This loads the core CommandHandlers. Only remove this if you 
really 
              know what you are doing</dd>
         <dt><strong>plainAuthDisallowed</strong></dt>
-        <dd>Whether to enable Authentication PLAIN if the connection is not 
encrypted via SSL or STARTTLS. Defaults to true.</dd>
+        <dd>Deprecated. Should use `auth.plainAuthEnabled`, `auth.requireSSL` 
instead. Whether to enable Authentication PLAIN if the connection is not 
encrypted via SSL or STARTTLS. Defaults to true.</dd>
+        <dt><strong>auth.plainAuthEnabled</strong></dt>
+        <dd>true or false. Defaults to true.</dd>
+        <dt><strong>auth.requireSSL</strong></dt>
+        <dd>true or false. Defaults to true.</dd>

Review comment:
       Maybe some explanations? 

##########
File path: 
server/apps/distributed-app/docs/modules/ROOT/pages/configure/imap.adoc
##########
@@ -44,7 +44,14 @@ Defaults to 10MB. Must be a positive integer, optionally 
with a unit: B, K, M, G
 Defaults to 0 (unlimited). Must be a positive integer, optionally with a unit: 
B, K, M, G.
 
 | plainAuthDisallowed
-| Whether to enable Authentication PLAIN if the connection is not encrypted 
via SSL or STARTTLS. Defaults to `true`.
+| Deprecated. Should use `auth.plainAuthEnabled`, `auth.requireSSL` instead.
+Whether to enable Authentication PLAIN if the connection is not encrypted via 
SSL or STARTTLS. Defaults to `true`.
+
+| auth.plainAuthEnabled
+| true or false. Defaults to `true`.
+
+| auth.requireSSL
+| true or false. Defaults to `true`.

Review comment:
       Please expplain what this option does

##########
File path: 
server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java
##########
@@ -75,15 +76,47 @@
     private boolean compress;
     private int maxLineLength;
     private int inMemorySizeLimit;
-    private boolean plainAuthDisallowed;
     private int timeout;
     private int literalSizeLimit;
+    private AuthenticationConfiguration authenticationConfiguration;
 
     public static final int DEFAULT_MAX_LINE_LENGTH = 65536; // Use a big 
default
     public static final Size DEFAULT_IN_MEMORY_SIZE_LIMIT = Size.of(10L, 
Size.Unit.M); // Use 10MB as default
     public static final int DEFAULT_TIMEOUT = 30 * 60; // default timeout is 
30 minutes
     public static final int DEFAULT_LITERAL_SIZE_LIMIT = 0;
 
+    public static class AuthenticationConfiguration {
+        private static final boolean REQUIRE_SSL_DEFAULT = true;
+        private static final boolean PLAIN_AUTH_ENABLED_DEFAULT = true;
+
+        public static AuthenticationConfiguration 
parse(HierarchicalConfiguration<ImmutableNode> configuration) {
+            boolean requireSSLConfig = 
configuration.getBoolean("auth.requireSSL", REQUIRE_SSL_DEFAULT);
+            boolean plainAuthEnabled = 
configuration.getBoolean("auth.plainAuthEnabled", 
fallbackPlainAuthEnabled(configuration));
+
+            boolean requireSSL = false;
+            if (requireSSLConfig && !plainAuthEnabled) {
+                requireSSL = true;

Review comment:
       Please avoid variable re-allocations you fools!




-- 
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]

Reply via email to