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

Implements #449 Make authentication details available in MovingMessage #450

Merged

Conversation

blutorange
Copy link
Contributor

Implements #449: Stores the authentication details in the moving message so that it is available in e.g. MessageDeliveryHandler. See also the ExampleFindUserByAuthLoginTest.

Would be great if you could pick this into the 1.6.x branch, since we can't switch to the new Jakarta namespace quite yet. I tried cherry picking this commit into the release-1.6.7 tag, there don't seem to be any merge conflicts.

  • 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)
  • I was considering whether we should also make the authentication details available to Pop3State, but I didn't see any use for that where that info might be useful.

…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 marcelmay self-assigned this Mar 30, 2022
@marcelmay marcelmay merged commit 306165a into greenmail-mail-test:master Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants