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

[WFCORE-5602] security enable-http-auth-http-server command fails for the security domain "other" #4783

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

rmartinc
Copy link
Contributor

@rmartinc rmartinc commented Oct 5, 2021

Issue: https://issues.redhat.com/browse/WFCORE-5602

When executing a security command over an existing undertow app domain it can happen that it was defined using referenced domain and the command is trying to add an http factory (or vice-versa). In one case the command returns the weird error detected in the JIRA and in the other the domain is not modified at all. This PR just detects that situation and throws an error: Can't mix mechanism and referenced security domain (the same error that was already used when the command passed both parameters).

The tests seem to be in wildfly SecurityAuthCommandsTestCase.java class. But we need to wait for that test to be re-enabled in WFLY-15057 (PR wildfly/wildfly#14670).

@jfdenise Please review this when you have some time.

Thanks!

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Oct 5, 2021
@jfdenise
Copy link
Contributor

jfdenise commented Oct 5, 2021

@rmartinc , thank-you. The change looks good to me. Did you ran the currently ignored tests with this change? It would be nice to add a testcase to cover it.

@rmartinc
Copy link
Contributor Author

rmartinc commented Oct 5, 2021

Thanks @jfdenise!

Did you ran the currently ignored tests with this change? It would be nice to add a testcase to cover it.

Yes, I have worked on top of your PR wildfly/wildfly#14670, tests pass OK and I have tentatively added something like this:

@@ -371,6 +373,16 @@ public class SecurityAuthCommandsTestCase {
     public void testOOBHTTP() throws Exception {
         ctx.handle("security enable-http-auth-http-server --no-reload --security-domain=" + TEST_UNDERTOW_DOMAIN);
         Assert.assertEquals(ElytronUtil.OOTB_APPLICATION_DOMAIN, getReferencedSecurityDomain(ctx, TEST_UNDERTOW_DOMAIN));
+
+        try {
+           // enabling the same domain using mechanism should fail now
+            ctx.handle("security enable-http-auth-http-server --no-reload --mechanism=BASIC "
+                    + "--file-system-realm-name=" + TEST_FS_REALM + " --security-domain=" + TEST_UNDERTOW_DOMAIN);
+            Assert.fail("Enabling using mechanism should have failed");
+        } catch (Exception e) {
+            MatcherAssert.assertThat(e.getMessage(), CoreMatchers.containsString("Can't mix mechanism and referenced security domain"));
+        }
+
         ctx.handle("security disable-http-auth-http-server --no-reload --security-domain=" + TEST_UNDERTOW_DOMAIN);
         Assert.assertFalse(domainExists(TEST_UNDERTOW_DOMAIN));
     }

The same in the testOOBHTTP2 to test the other path. I have created WFLY-15410 to modify the tests once the activation PR is merged.

@jfdenise
Copy link
Contributor

jfdenise commented Oct 5, 2021

@rmartinc , perfect, thank-you!

@wildfly wildfly deleted a comment from wildfly-ci Oct 12, 2021
@jmesnil jmesnil merged commit fced231 into wildfly:main Oct 12, 2021
@jmesnil
Copy link
Member

jmesnil commented Oct 12, 2021

@rmartinc thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants