chibenwa commented on a change in pull request #791:
URL: https://github.com/apache/james-project/pull/791#discussion_r766446160
##########
File path:
server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java
##########
@@ -210,9 +212,9 @@ protected ChannelUpstreamHandler createCoreHandler() {
ImapChannelUpstreamHandler coreHandler;
Encryption secure = getEncryption();
if (secure != null && secure.isStartTLS()) {
- coreHandler = new ImapChannelUpstreamHandler(hello, processor,
encoder, compress, plainAuthDisallowed, secure, imapMetrics);
+ coreHandler = new ImapChannelUpstreamHandler(hello, processor,
encoder, compress, plainAuthDisallowed, secure, plainAuthEnabled, imapMetrics);
Review comment:
As said for Tung PR I would enjoy that we pass a configuration POJO
around rather than individual parameters.
##########
File path:
protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java
##########
@@ -49,20 +49,24 @@ public LoginProcessor(ImapProcessor next, MailboxManager
mailboxManager, StatusR
@Override
protected void processRequest(LoginRequest request, ImapSession session,
Responder responder) {
- // check if the login is allowed with LOGIN command. See IMAP-304
- if (session.isPlainAuthDisallowed() && session.isTLSActive() ==
false) {
- no(request, responder, HumanReadableText.DISABLED_LOGIN);
- } else {
- doAuth(noDelegation(request.getUserid(),
request.getPassword()),
- session, request, responder,
HumanReadableText.INVALID_LOGIN);
- }
+ // check if the login is allowed with LOGIN command. See IMAP-304
+ boolean isPlainAuthDisabled = !session.isPlainAuthEnabled();
+ boolean isPlainAuthDisabledWhenNotEncrypted =
session.isPlainAuthDisallowed() && !session.isTLSActive();
Review comment:
The boolean expression looks like the same than for AuthProcessor.
Maybe we could move it in the IMAP session for reuse (eg using a default
method?)
##########
File path:
protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java
##########
@@ -140,7 +142,7 @@ private AuthenticationAttempt parseDelegationAttempt(String
initialClientRespons
List<Capability> caps = new ArrayList<>();
// Only ounce AUTH=PLAIN if the session does allow plain auth or TLS
is active.
// See IMAP-304
- if (session.isPlainAuthDisallowed() == false ||
session.isTLSActive()) {
+ if (session.isPlainAuthEnabled() && (!session.isPlainAuthDisallowed()
|| session.isTLSActive())) {
Review comment:
Is this connected to the boolean expression line 63 - 66 above?
Any way we could reuse 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.
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]
