greenmail-mail-test / greenmail

Official master for the Greenmail project
http://greenmail-mail-test.github.io/greenmail/
Apache License 2.0
633 stars 181 forks source link

Support authentication for SMTP #127

Open marcelmay opened 8 years ago

marcelmay commented 8 years ago

Note: SMTP-AUTH is rarely used

buildscientist commented 8 years ago

@marcelmay in hindsight we probably shouldn't have enabled this by default. Now users will need to enable auth for SMTP but it's enabled for IMAP/POP3 by default. There's some cognitive dissonance here from a usability standpoint that makes things confusing.

I suggest we force users to auth against ALL services and then give them the option to disable auth all services.

camann9 commented 8 years ago

As long as we have a flag that controls this and we have the current default as the default for the new flag I'm good with that.

On Tue, Apr 12, 2016, 10:59 AM Youssuf ElKalay notifications@github.com wrote:

@marcelmay https://github.com/marcelmay in hindsight we probably shouldn't have enabled this by default. Now users will need to enable auth for SMTP but it's enabled for IMAP/POP3 by default. There's some cognitive dissonance here from a usability standpoint that makes things confusing.

I suggest we force users to auth against ALL services and then give them the option to disable auth all services.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/greenmail-mail-test/greenmail/issues/127#issuecomment-209030650

buildscientist commented 8 years ago

@camann9 so what we're saying is that out of box users will NOT have to authenticate against SMTP/IMAP/POP3 - and that a flag or configuration param will be required to activate authentication.

camann9 commented 8 years ago

I think the default state for anything should be to have behavior that's compatible with the current situation

On Tue, Apr 12, 2016, 4:07 PM Youssuf ElKalay notifications@github.com wrote:

@camann9 https://github.com/camann9 so what we're saying is that out of box users will NOT have to authenticate against SMTP/IMAP/POP3 - and that a flag or configuration param will be required to activate authentication.

— You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub https://github.com/greenmail-mail-test/greenmail/issues/127#issuecomment-209143181

buildscientist commented 8 years ago

Right so therein lies my point - today as it stands authentication is NOT required for SMTP. It is required for IMAP/POP3.

If we go with a default of auth not required for all services - users shouldn't have to update tests provided they are not testing for failed authentication.

If we go with a default where auth IS required for all services - users will need to update all tests when connecting to our SMTP service.

Example code being:

    Session smtpSession = greenMail.getSmtp().createSession();
    Message msg = new MimeMessage(smtpSession);
    msg.setFrom(new InternetAddress("foo@example.com"));
    msg.addRecipient(Message.RecipientType.TO,
            new InternetAddress("bar@example.com"));
    msg.setSubject("Email sent to GreenMail via plain JavaMail");
    msg.setText("Fetch me via IMAP");
    Transport.send(msg); // Will fail if SMTP auth is required by default
camann9 commented 8 years ago

Yeah, that was my point. We will have to have a third mode for the flag which makes the behaviour exactly the same as it is now in addition to the two sane modes (autocreate in all protocols or do not autocreate anything). And I'm arguing that this mode should be the default.

On Tue, Apr 12, 2016, 4:24 PM Youssuf ElKalay notifications@github.com wrote:

Right so therein lies my point - today as it stands authentication is NOT required for SMTP. It is required for IMAP/POP3.

If we go with a default of auth not required for all services - users shouldn't have to update tests provided they are not testing for failed authentication.

If we go with a default where auth IS required for all services - users will need to update all tests when connecting to our SMTP service.

Example code being:

Session smtpSession = greenMail.getSmtp().createSession();
Message msg = new MimeMessage(smtpSession);
msg.setFrom(new InternetAddress("foo@example.com"));
msg.addRecipient(Message.RecipientType.TO,
        new InternetAddress("bar@example.com"));
msg.setSubject("Email sent to GreenMail via plain JavaMail");
msg.setText("Fetch me via IMAP");
Transport.send(msg); // Will fail if SMTP auth is required by default

— You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub https://github.com/greenmail-mail-test/greenmail/issues/127#issuecomment-209147639

EarthCitizen commented 8 years ago

The lack of authentication for SMTP is creating a huge problem for me. In my unit tests I am using:

@Rule
  public final GreenMailRule mailRule =
      new GreenMailRule(ServerSetupTest.SMTPS)
        .withConfiguration(GreenMailConfiguration.aConfig().withUser('someone', 'somepassword'))

I have tried doing this in the tests:

mailRule.managers.userManager.setAuthRequired(true)

but it is still not verifying the password. I need the password to be verified because the production code will be connecting to an authenticated SMTPS server. So, GreenMail authenticating my password is the only way I have to make sure my code is correctly sending the password to the server.

Am I doing something wrong? Is there another way to enable SMTP authentication in the tests?

buildscientist commented 8 years ago

@EarthCitizen Until this issue is resolved authentication against SMTP services is currently not required.

The setAuthRequired() call on UserManager only applies to IMAP and POP3.

buildscientist commented 8 years ago

Additionally I'm not really sure how your test suite would benefit from having assertion on a password.

Yes I agree that not having SMTP authentication is an issue but you can write your test such that it is implied that authentication always passes. I encourage you to review your test case/method and ask yourself this question:

What am I testing? Authentication or a valid socket/connection to my SMTP peer. I would argue the latter.

EarthCitizen commented 8 years ago

The test is actually that the correct password is send to the server. Which SMTP authentication would have verified.

buildscientist commented 8 years ago

@EarthCitizen

Ok so there seems to be some confusion here - when you open a connection to the GreenMail SMTP server it's still going to authenticate - but it will accept any password. Hence my comment about asserting on a valid/connected Transport session instead of a password.

buildscientist commented 8 years ago

@EarthCitizen It looks like I was mistaken - the GreenMail SMTP service does not support the AUTH command as shown here in the SMTPCommandRegistry class

We'll need to implement AUTH command.

@marcelmay is this something we'd grab from Apache James given that IMAP and POP3 use code from that project?

hazemkmammu commented 3 years ago

Any update on this?

marcelmay commented 3 years ago

@hazemkmammu : What type of SMTP AUTH do you require exactly?

Currently PLAIN and LOGIN are supported via AuthCommand