Migrate ssl package to new test framework by michalvavrik · Pull Request #48407 · keycloak/keycloak

michalvavrik · 2026-04-23T12:17:42Z

Closes: Migrate ssl package to new test framework #47812
As far as the email goes, I thought it would be better to avoid adding extra dependency (SubEthaSmtp) to the new test framework for the SMTPS feature that only tests added in this PR are using. So I turned around the paradigm (change truststore on the server side, not keystore on the mail server side). I believe all the original scenarios are tested, you can compare it yourself:

Old test method	New test
`test01VerifyEmailWithSslWrongCertificate`	`EmailUntrustedCertTest#testVerifyEmailWithUntrustedCert`
`test02VerifyEmailWithSslWrongCertificateAndAnyHostnamePolicy`	`EmailUntrustedCertAnyHostnameTest#testEmailSucceedsWithSslAndAnyHostnamePolicyDespiteEmptyTruststore` due to #48407 (comment)
`test03erifyEmailWithSslWrongHostname`	`EmailTest#testVerifyEmailWithSslWrongHostname`
`test04VerifyEmailWithSslEnabled`	`EmailTest#testVerifyEmailWithSslEnabled`
`test05VerifyEmailWithSslWrongHostnameButAnyHostnamePolicy`	`EmailAnyHostnameTest#testVerifyEmailWithSslWrongHostnameSucceeds`
`test06VerifyEmailOpportunisticEncryptionWithAnyHostnamePolicy`	`EmailPlainAuthTest#testVerifyEmailWithPlainSmtpAuth`

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-04-23T13:51:59Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-04-23T18:32:47Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

vmuzikar · 2026-05-06T16:23:13Z

@shawkins Could you please review and merge this?

vmuzikar · 2026-05-06T16:23:59Z

@michalvavrik Can you please rebase?

michalvavrik · 2026-05-06T18:59:26Z

@michalvavrik Can you please rebase?

done

shawkins · 2026-05-07T14:54:29Z

Will this need to align to #48785

michalvavrik · 2026-05-07T15:29:21Z

Will this need to align to #48785

This PR has been opened for 2 weeks +, while #48785 has been opened for 5 hours and is still draft. Why don't they align with this PR?

michalvavrik · 2026-05-07T15:33:31Z

AFAICT it will still work, there will just be some merge conflicts.

Copilot

Pull request overview

Migrates the legacy Arquillian-based ssl tests to the new test framework, moving coverage for SMTP(S) truststore/hostname verification and ssl-required behavior into tests/base and wiring the new package into the JUnit suite.

Changes:

Removed legacy Arquillian SSL tests and suite entries (testsuite/.../ssl, base-suite, fips-suite).
Added new org.keycloak.tests.ssl test package (email over SMTPS + hostname policy scenarios, and ssl-required HTTP/TLS behavior).
Added SSL test resources (SMTP server cert PEM) and registered the new test package in Base2TestSuite.

Reviewed changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
testsuite/integration-arquillian/tests/base/testsuites/fips-suite	Removes legacy `TrustStoreEmailTest` from FIPS suite list.
testsuite/integration-arquillian/tests/base/testsuites/base-suite	Removes legacy `ssl` group from Arquillian base suite.
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/ssl/TrustStoreEmailTest.java	Deletes legacy Arquillian truststore email tests.
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/ssl/TLSTest.java	Deletes legacy Arquillian TLS/ssl-required test.
tests/base/src/test/resources/org/keycloak/tests/ssl/smtp-server.pem	Adds SMTP server certificate intended for truststore-paths PEM loading.
tests/base/src/test/java/org/keycloak/tests/suites/Base2TestSuite.java	Adds `org.keycloak.tests.ssl` to the new test suite package list.
tests/base/src/test/java/org/keycloak/tests/ssl/AbstractSslEmailTest.java	Introduces shared SMTPS+verify-email test base using GreenMail + shared keystore config.
tests/base/src/test/java/org/keycloak/tests/ssl/TrustStoreEmailTest.java	New SMTPS verify-email success + wrong-hostname failure tests.
tests/base/src/test/java/org/keycloak/tests/ssl/TrustStoreEmailAnyHostnameTest.java	New hostname-policy-ANY test where hostname mismatch should succeed.
tests/base/src/test/java/org/keycloak/tests/ssl/TrustStoreEmailUntrustedCertTest.java	New untrusted-certificate (empty truststore) failure test.
tests/base/src/test/java/org/keycloak/tests/ssl/TrustStoreEmailUntrustedCertAnyHostnameTest.java	New “untrusted cert + ANY hostname policy” failure test (currently mixes STARTTLS/SMTPS).
tests/base/src/test/java/org/keycloak/tests/ssl/TrustStoreEmailPlainAuthTest.java	New plain SMTP auth verify-email flow test.
tests/base/src/test/java/org/keycloak/tests/ssl/TlsSslRequiredTest.java	New `ssl-required` behavior test for HTTP allowed/rejected depending on realm setting.

michalvavrik · 2026-05-07T17:42:54Z

+    static class StarttlsEmailRealmConfig implements RealmConfig {
+        @Override
+        public RealmBuilder configure(RealmBuilder realm) {
+            realm.verifyEmail(true);
+            realm.build().setSmtpServer(starttlsSmtpConfig());
+            return realm;
+        }
+
+        private static Map<String, String> starttlsSmtpConfig() {
+            return Map.of("from", FROM, "host", HOST, "port", PORT_SSL, "starttls", "true");
+        }


StarttlsEmailRealmConfig sets starttls=true while also using PORT_SSL (SMTPS port) and the test starts an SMTPS server via AbstractSslEmailTest.createSmtpsServer(). This mixes STARTTLS and SMTPS and will cause the email send to fail for the wrong reason (protocol mismatch), so the test does not actually validate the “untrusted cert even with ANY hostname policy” scenario.
Align the realm SMTP configuration with the server you start (e.g., use ssl=true + PORT_SSL for SMTPS, or start an SMTP server configured for STARTTLS on the non-SSL port).

The original test also used starttls=true, it started with SMTP, then upgraded. The DefaultEmailSenderProvider.setupTruststore() sets mail.smtp.ssl.trust=* when hostname policy is ANY, which tells JavaMail to trust all server certificates regardless of the truststore. So yes, with STARTTLS capable mail server (like SubEthaSMTP used by the original test) it failed. But it is not very useful because it just verifies Jakarta Mail implementation that behaves differently for STARTTLS and differently for SSL=TRUE. What use is it if untrusted certificate is rejected for STARTTLS=TRUE but allowed for SSL=TRUE? IMO that is useless assertion. Either it is a bug in Keycloak, or we should assert success with the SSL=TRUE, so I reworked this test after Copilet comment to show "pass scenario" instead.

keycloak/services/src/main/java/org/keycloak/email/DefaultEmailSenderProvider.java

Line 325 in a9d523b

props.setProperty("mail.smtp.ssl.trust", "*");

Copilot

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

shawkins

I think what you did with the mail sever makes sense. As long as the cert ends up untrusted, we get the same assertion.

test06VerifyEmailOpportunisticEncryptionWithAnyHostnamePolicy

That seems like an odd name for the legacy test as it implies encryption was still being used. I agree with your new naming testVerifyEmailWithPlainSmtpAuth.

On the naming of the test classes in general, it would probably read better to drop the TrustStore prefix - in particular testVerifyEmailWithPlainSmtpAuth would pass without a truststore set. But this isn't a blocker for me.

* Closes: keycloak#47812 Signed-off-by: Michal Vavřík <michal.vavrik@aol.com>

michalvavrik · 2026-05-07T20:22:15Z

On the naming of the test classes in general, it would probably read better to drop the TrustStore prefix - in particular testVerifyEmailWithPlainSmtpAuth would pass without a truststore set. But this isn't a blocker for me.

I've dropped the TrustStore prefix, I agree it reads better.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-05-07T21:19:41Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostSigEmailTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testReloginWithInvalidAuthSessionCookie

Keycloak CI - Adapter IT Strict Cookies

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/demo/login-action ; actual URL: https://localhost:8643/sales-post-sig-email/ ==> expected: <true> but was: <false> within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1160)
...

Report flaky test

shawkins

LGTM

michalvavrik · 2026-05-11T08:07:19Z

maybe let's merge it to avoid conflicts with the other PR from Stian @vmuzikar ?

vmuzikar · 2026-05-11T08:16:25Z

Merging based on @shawkins' review.

michalvavrik requested review from a team as code owners April 23, 2026 12:17

keycloak-github-bot Bot added team/cloud-native team/core-clients team/core-iam labels Apr 23, 2026

michalvavrik force-pushed the chore/issues/47812 branch from 23d6ff7 to 5ec8a92 Compare April 23, 2026 12:34

michalvavrik requested a review from vmuzikar April 23, 2026 12:37

michalvavrik force-pushed the chore/issues/47812 branch from 5ec8a92 to 7af881a Compare April 23, 2026 12:44

keycloak-github-bot Bot added the flaky-test label Apr 23, 2026

keycloak-github-bot Bot mentioned this pull request Apr 23, 2026

Flaky test: org.keycloak.testsuite.forms.AuthenticatorSubflowsTest2#testSubflow2 #47477

Closed

keycloak-github-bot Bot reviewed Apr 23, 2026

View reviewed changes

michalvavrik marked this pull request as draft April 23, 2026 14:52

michalvavrik force-pushed the chore/issues/47812 branch from 7af881a to 84b352b Compare April 23, 2026 16:58

michalvavrik marked this pull request as ready for review April 23, 2026 16:59

michalvavrik force-pushed the chore/issues/47812 branch from 84b352b to 6158ce0 Compare April 23, 2026 17:15

keycloak-github-bot Bot reviewed Apr 23, 2026

View reviewed changes

vmuzikar requested a review from shawkins May 6, 2026 16:23

vmuzikar assigned shawkins May 6, 2026

vmuzikar removed their request for review May 6, 2026 16:23

michalvavrik force-pushed the chore/issues/47812 branch from 6158ce0 to 9c94801 Compare May 6, 2026 18:59

shawkins requested a review from Copilot May 7, 2026 15:48

Copilot started reviewing on behalf of shawkins May 7, 2026 15:57 View session