Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get/create user by login instead of mail address #449

Closed
blutorange opened this issue Mar 21, 2022 · 9 comments
Closed

Get/create user by login instead of mail address #449

blutorange opened this issue Mar 21, 2022 · 9 comments
Assignees
Labels
backport_1_6_x Backport to old bugfixing branch 1.6.x enhancement
Milestone

Comments

@blutorange
Copy link
Contributor

blutorange commented Mar 21, 2022

The default implementation of MessageDeliveryHandler attempts to find a user by the mail address.

For testing, it would be helpful if we could get the user by their login (AUTH) credentials instead. This way, as long as the SMTP connection with the username and password is configured correctly, all mails would be sent to the same inbox, irrespective of the recipient's mail address.

From a quick look at the code, I think the easiest way to enable this might be to add the login information to MovingMessage. Then we can set our own MessageDeliveryHandler to implement the mapping from the login to a user.

If you're open for PRs, I'd be happy to take a look at this.

@marcelmay
Copy link
Member

marcelmay commented Mar 22, 2022

If you write your own custom delivery handler, you could use the UserManager#getUser(String login) ?

Example:

UserManager userManager = greenMail.getUserManager()
final GreenMailUser user = userManager.getUser(login); // Forward always to user specified by login
MessageDeliveryHandler handler = new MessageDeliveryHandler(){
        public GreenMailUser handle(MovingMessage msg, MailAddress mailAddress) throws UserException {
            return user;
        }
}
userManager.setMessageDeliveryHandler(handler);

@blutorange
Copy link
Contributor Author

blutorange commented Mar 22, 2022

@marcelmay Thanks for the example. Maybe I misunderstand, but where does the login come from? There can be multiple users, each with their own login. What would be great is, if the login could be taken from the username that was used to authenticate against the SMTP server.

  • One green mail server, which is shared by multiple tests.
  • Each test should have its own separate inbox. It's hard for the application under test to guarantee that it always sends an email from or to a specific email address.
  • What we can guarantee, is the the application always uses the given username / password for connecting to the server.

=> One server, multiple users, and mails should be delivered to the user derived from the username that was used to authenticate.

This could also be solved by trying to ensure the application always uses the same sender address, or by starting a new mail server for each test. Although I'd prefer the above as it feels cleaner.

Before I found Greenmail I was trying subetha SMTP, and while great, it lacks POP3 / IMAP servers. For reference, it has a MessageHandlerFactory which receives a MessageContext containing not only the mail address but also the authentication information.

Hence my idea that perhaps we could pass on the data from the authentication to the message delivery handler (which I hope shouldn't be too hard).

@marcelmay
Copy link
Member

If I understand you correctly, this is the default behaviour by GreenMail.

The use case for the delivery handler is if you want to do something non standard:

  • Do delivery status notification Support for Delivery Status Notification #307
  • forward mail to another GreenMail account/inbox
  • have a single account receiving all messages regardless of TO header
    The MDH runs server-side as part of the mail server, delivering by default mails to the user accounts/INBOX.

The GreenMailUser (identified by the received message header TO email) holds also login etc.:

private MessageDeliveryHandler messageDeliveryHandler = new MessageDeliveryHandler(){

When the server receives an email, the default delivery handler tries to create a user unless it already exists:

...
    private MessageDeliveryHandler messageDeliveryHandler = new MessageDeliveryHandler(){
        public GreenMailUser handle(MovingMessage msg, MailAddress mailAddress) throws UserException {
            GreenMailUser user = getUserByEmail(mailAddress.getEmail());   // TO
            if(null==user) {
                String login = mailAddress.getEmail();
                String email = mailAddress.getEmail();
                String password = mailAddress.getEmail();
                user = createUser(email, login, password);
                ...
            }
            return user;
        }
    };
...

Could you provide an example how you would change the default / use a login?

@blutorange
Copy link
Contributor Author

blutorange commented Mar 24, 2022

Yes, by default it tries to find a user based on the address to which the mail is sent. What I'd like to do, is to use a user based on the AUTH PLAIN credentials. But perhaps an example can explain this better:

First, we create new user Greenmail account

userManager.createUser("whatever@example.com", "john", "secretPassword")

Now we enter the SMTP settings into our application that's being tested

host: localhost:9999
username: john
password: secretPassword

And finally, we start a test and our application sents an email to randomAddress@foobar.org

What happens now is:

  • The default implementation calls getUserByEmail(mailAddress.getEmail())
  • It finds none and creates a new user account with login=randomAddress@foobar.org and the same username.

What I'd like to happen is

  • My own implementation read the login from the PLAIN AUTH authentication
  • And creates a new user account for john.

This would ensure that all mails are directed to the same user account, as long as the SMTP connection has the correct username/password. This would make (1) easier to find mails and (2) would allow a single mail server to be used for different tests.


The message delivery handler could look something like this

    private MessageDeliveryHandler messageDeliveryHandler = new MessageDeliveryHandler(){
        public GreenMailUser handle(MovingMessage msg, MailAddress mailAddress) throws UserException {

            // msg.getAuth() returns the username/password from the PLAIN AUTH, if available
            GreenMailUser user = getUserByLogin(msg.getAuth().getUsername());

            if(null==user) {
                String login = msg.getAuth().getUsername();
                String email = msg.getAuth().getUsername() + "@example.com"; // or some other host
                String password = msg.getAuth().getUsername();
                user = createUser(email, login, password);
            }

            return user;
        }
    };

@marcelmay
Copy link
Member

I still hope that your use case could be handled by matching eg the msg FROM/TO/SUBJECT .
For keeping the tests isolated, you'd have test specific emails?

Regarding (1) and (2), you can also fetch all messages via greenMail.getReceivedMessages() and filter out the ones for your current test case.

Users login and email should be uniqe, too.

If you really need the extension, this would require adding the user identity (or GreenMailUser?) to SmtpState and MovingMessage, see the SmtpConnection.setAuthenticated uses.

@blutorange
Copy link
Contributor Author

blutorange commented Mar 27, 2022

First of all, I appreciate the quick responses and that you're taking your time to read all of this. It's not something everybody does for other open source projects.

I still hope that your use case could be handled by matching eg the msg FROM/TO/SUBJECT .

That's pretty much what I'm doing for now, I try to make sure that every tests use the correct FROM address.

This kinda works, but it's not ideal, since the application being tested lets users of the application configure "email actions" and lets them change all of TO/FROM/SUBJECT/... for the email to send (in principle). So making sure all mails for a particular test case are redirected to a particular user, regardless of the mail's contents, would simplify things quite a bit.

Regarding (1) and (2), you can also fetch all messages via greenMail.getReceivedMessages() and filter out the ones for your current test case.

That might work, but we really don't have any particular identifier we could use for that.

If you really need the extension, this

If possible, I think this would be a cleaner approach. Also, tests are written by different persons, and it'd help if we didn't have to require them to use specific values for TO/FROM/SUBJECT.

would require adding the user identity (or GreenMailUser?) to SmtpState and MovingMessage, see the SmtpConnection.setAuthenticated uses.

If you think it doesn't fit Greenmail from an architectural point of view, we could probably make do somehow with the current test setup. Otherwise, I'd like to take a shot at this, from what I've seen this shouldn't be too hard to implement. And it shouldn't change any of the current behavior.

blutorange added a commit to blutorange/greenmail that referenced this issue Mar 29, 2022
…ble in MovingMessage

* Implements greenmail-mail-test#449: Stores the authentication details in the moving
message so that it is available in e.g. MessageDeliveryHandler
* Since different authentication methods have different details, there's
a AuthenticationState base interface with PlainAuthenticationState (with
an additional authorizationId) and LoginAuthenticationState POJOs for
the currently supported AUTH mechanisms LOGIN and AUTH. Other AUTH
mechanisms may or may not have a username and/or password.
* What I don't fully understand is why the SMTPState needs to be cleared
so often. It's also cleared by the MailCommand, which removes the
authentication details from the MovingMessage. I modfied the MailCommand
so that it clears the SMTPState, but transfers the authentication
details from the old moving message to the new one. We could also store
the authentication details in the SMTPState as well, but that feels a
bit redundant.
* When I run the tests locally, the
DummySSLServerSocketFactoryTest#testLoadKeyStoreViaSystemProperty fails,
but that's because I don't have a certificate with an `amazonrootca1` in
my system root key store (there is, e.g. a `debian:amazon_root_ca_1.pem`
certificate and when I change the test to use that, it passes)
marcelmay pushed a commit that referenced this issue Mar 30, 2022
* Implements #449: Stores the authentication details in the moving
message so that it is available in e.g. MessageDeliveryHandler
* Since different authentication methods have different details, there's
a AuthenticationState base interface with PlainAuthenticationState (with
an additional authorizationId) and LoginAuthenticationState POJOs for
the currently supported AUTH mechanisms LOGIN and AUTH. Other AUTH
mechanisms may or may not have a username and/or password.
* What I don't fully understand is why the SMTPState needs to be cleared
so often. It's also cleared by the MailCommand, which removes the
authentication details from the MovingMessage. I modfied the MailCommand
so that it clears the SMTPState, but transfers the authentication
details from the old moving message to the new one. We could also store
the authentication details in the SMTPState as well, but that feels a
bit redundant.
* When I run the tests locally, the
DummySSLServerSocketFactoryTest#testLoadKeyStoreViaSystemProperty fails,
but that's because I don't have a certificate with an `amazonrootca1` in
my system root key store (there is, e.g. a `debian:amazon_root_ca_1.pem`
certificate and when I change the test to use that, it passes)
@marcelmay marcelmay self-assigned this Mar 30, 2022
@marcelmay marcelmay added enhancement backport_1_6_x Backport to old bugfixing branch 1.6.x labels Mar 30, 2022
@marcelmay marcelmay added this to the 2.0.0-alpha-3 milestone Mar 30, 2022
marcelmay pushed a commit that referenced this issue Mar 30, 2022
#451)

* Implements #449: Stores the authentication details in the moving
message so that it is available in e.g. MessageDeliveryHandler
* Since different authentication methods have different details, there's
a AuthenticationState base interface with PlainAuthenticationState (with
an additional authorizationId) and LoginAuthenticationState POJOs for
the currently supported AUTH mechanisms LOGIN and AUTH. Other AUTH
mechanisms may or may not have a username and/or password.
* What I don't fully understand is why the SMTPState needs to be cleared
so often. It's also cleared by the MailCommand, which removes the
authentication details from the MovingMessage. I modfied the MailCommand
so that it clears the SMTPState, but transfers the authentication
details from the old moving message to the new one. We could also store
the authentication details in the SMTPState as well, but that feels a
bit redundant.
* When I run the tests locally, the
DummySSLServerSocketFactoryTest#testLoadKeyStoreViaSystemProperty fails,
but that's because I don't have a certificate with an `amazonrootca1` in
my system root key store (there is, e.g. a `debian:amazon_root_ca_1.pem`
certificate and when I change the test to use that, it passes)
@marcelmay
Copy link
Member

Thanks alot, @blutorange !

I plan a 1.6.x release this weekend

@blutorange
Copy link
Contributor Author

You're welcome, and thanks for merging, Yes, a release this week is more than enough for us.

@blutorange
Copy link
Contributor Author

Just tried with the release and it works great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport_1_6_x Backport to old bugfixing branch 1.6.x enhancement
Projects
None yet
Development

No branches or pull requests

2 participants