diff --git a/e2e/kerberos/kerberos.spec.mjs b/e2e/kerberos/kerberos.spec.mjs index c612f01808..96e47e4af9 100644 --- a/e2e/kerberos/kerberos.spec.mjs +++ b/e2e/kerberos/kerberos.spec.mjs @@ -24,7 +24,7 @@ * pointed at .work/krb5.conf and .work/krb5cc by KRB5_CONFIG and KRB5CCNAME. * * Requires MIT Kerberos client tools (Debian/Ubuntu package krb5-user); skipped without them. - * Runs before HttpOnly mode is enabled: once #1170 is fixed, the curl case reads tokenId from the response body. + * Runs before HttpOnly mode is enabled: the curl case reads tokenId from the response body. * * /UI/Login is not covered: with the XUI enabled (the default) XUIFilter redirects it to the XUI. */ @@ -54,18 +54,6 @@ const BOOTSTRAP_ENV = { KRB_USER: process.env.KRB_USER ?? USERNAME, }; -// /json/authenticate answers 400 when the Negotiate header arrives with the request that starts the -// login: the module's script TextOutputCallback cannot be filled from the request, so -// RestAuthCallbackHandlerManager converts every callback to JSON and -// RestAuthHttpCallbackHandler.convertToJson rejects the HttpCallback. The browser resends the XUI -// request with the header after the 401 challenge, so the XUI hits the same error. -// The curl and XUI cases assert that 400. Once the endpoint is fixed they turn red: assert the session instead. -const REST_NEGOTIATE_BUG = "/json/authenticate rejects a Negotiate header with 400: " - + "https://github.com/OpenIdentityPlatform/OpenAM/issues/1170"; -// A request without the Authorization header gets the module's 401 first, so only one with the header -// reaches this message. -const HTTP_CALLBACK_REJECTED = "HttpCallback Authorization Header must be specified in the initial request"; - // The browser may answer Negotiate challenges from the OpenAM host only. test.use({ launchOptions: { @@ -173,8 +161,8 @@ test.afterEach(async ({}, testInfo) => { test.describe("OpenAM WindowsDesktopSSO - Kerberos SSO", () => { test("REST: the browser's Negotiate token completes a login started without it", async ({ page }) => { // The module reads the Negotiate header of whichever request continues the login, so a client - // that first obtains authId and then sends the header together with it is not affected by the - // one-step bug. This proves the KDC, the keytab, the module settings and the browser's Kerberos. + // that first obtains authId and then sends the header together with it completes the login too. + // This proves the KDC, the keytab, the module settings and the browser's Kerberos. const authorization = await negotiateHeaderFromBrowser(page); expect(authorization, "the browser should answer the Negotiate challenge").toBeTruthy(); @@ -194,7 +182,7 @@ test.describe("OpenAM WindowsDesktopSSO - Kerberos SSO", () => { expect(String(await idFromSession(page.request, tokenId)).toLowerCase()).toBe(krb.user.toLowerCase()); }); - test("REST: curl --negotiate is rejected with 400 until #1170 is fixed", async () => { + test("REST: curl --negotiate completes the login in one request", async ({ request }) => { const output = execFileSync("curl", [ "-sS", "--negotiate", "-u", ":", "-H", "Content-Type: application/json", @@ -208,12 +196,13 @@ test.describe("OpenAM WindowsDesktopSSO - Kerberos SSO", () => { const status = output.slice(separator + 1); console.log(`/json/authenticate answered ${status}: ${body}`); - // #1170: when this turns red, assert 200, tokenId and the session from idFromSession instead. - expect(status, REST_NEGOTIATE_BUG).toBe("400"); - expect(body, "the 400 should come from curl's Negotiate header").toContain(HTTP_CALLBACK_REJECTED); + expect(status, body).toBe("200"); + const { tokenId } = JSON.parse(body); + expect(tokenId, "the response should carry tokenId").toBeTruthy(); + expect(String(await idFromSession(request, tokenId)).toLowerCase()).toBe(krb.user.toLowerCase()); }); - test("XUI: the Negotiate login is rejected with 400 until #1170 is fixed", async ({ page }) => { + test("XUI: the browser's Negotiate token logs in without a form", async ({ page }) => { page.on("response", (resp) => { if (new URL(resp.url()).pathname.endsWith("/authenticate")) { console.log(`XUI ${resp.request().method()} ${resp.url()} answered ${resp.status()}`); @@ -225,10 +214,14 @@ test.describe("OpenAM WindowsDesktopSSO - Kerberos SSO", () => { await page.goto(`${OPENAM_BASE}/XUI/#login/&authIndexType=module` + `&authIndexValue=${encodeURIComponent(krb.module)}`); const resp = await login; - // #1170: when this turns red, poll idFromSession for the user and assert that no login form is shown instead. - expect(resp.status(), REST_NEGOTIATE_BUG).toBe(400); - expect(await resp.text(), "the 400 should come from the browser's Negotiate header") - .toContain(HTTP_CALLBACK_REJECTED); + // Read the body only on failure: after a successful login the XUI navigates away, and Chromium may drop + // the previous document's response bodies. + if (resp.status() !== 200) { + throw new Error(`/authenticate answered ${resp.status()}: ${await resp.text()}`); + } + await expect.poll(async () => String(await idFromSession(page.request)).toLowerCase(), { timeout: 30_000 }) + .toBe(krb.user.toLowerCase()); + await expect(page).not.toHaveURL(/#login/, { timeout: 30_000 }); }); test("fallback: without a ticket the chain continues to the DataStore form", async ({ request }) => { diff --git a/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManager.java b/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManager.java index f10c0d4f6e..14c51f4b63 100644 --- a/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManager.java +++ b/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManager.java @@ -12,19 +12,23 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2013-2015 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package org.forgerock.openam.core.rest.authn; import jakarta.inject.Inject; import javax.security.auth.callback.Callback; +import javax.security.auth.callback.TextOutputCallback; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import com.google.inject.Singleton; +import com.sun.identity.authentication.callbacks.NameValueOutputCallback; +import com.sun.identity.authentication.spi.HttpCallback; import com.sun.identity.shared.debug.Debug; import org.forgerock.json.JsonValue; import org.forgerock.json.resource.ResourceException; @@ -91,6 +95,10 @@ public JsonValue handleCallbacks(HttpServletRequest request, * headers and request or the headers and request do not contain the required information the method returns * false. * + * An HttpCallback has no JSON representation, so when one is present, output-only Callbacks (such as the script + * fallback of WindowsDesktopSSO) count as updated. Input Callbacks still have to be filled from the headers and + * request. The HttpCallback handler answers with a challenge if its header is missing. + * * @param request The HttpServletRequest from the request. * @param response The HttpServletResponse from the request. * @param callbacks The Callbacks to update with their required values from the headers and request. @@ -100,12 +108,15 @@ private boolean handleCallbacksInternally(HttpServletRequest request, HttpServletResponse response, Callback[] callbacks) throws RestAuthException { + boolean hasHttpCallback = Arrays.stream(callbacks).anyMatch(HttpCallback.class::isInstance); + for (Callback callback : callbacks) { RestAuthCallbackHandler restAuthCallbackHandler = restAuthCallbackHandlerFactory.getRestAuthCallbackHandler(callback.getClass()); - if (!restAuthCallbackHandler.updateCallbackFromRequest(request, response, callback)) { + if (!restAuthCallbackHandler.updateCallbackFromRequest(request, response, callback) + && !(hasHttpCallback && isOutputOnly(callback))) { return false; } } @@ -113,6 +124,10 @@ private boolean handleCallbacksInternally(HttpServletRequest request, return true; } + private static boolean isOutputOnly(Callback callback) { + return callback instanceof TextOutputCallback || callback instanceof NameValueOutputCallback; + } + /** * Handles the JSON representations of Callbacks, converting them back to Callbacks by setting the values from * the JSONArray to the original Callbacks passed in. diff --git a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManagerTest.java b/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManagerTest.java index ab8179e120..e990bb90f5 100644 --- a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManagerTest.java +++ b/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/authn/RestAuthCallbackHandlerManagerTest.java @@ -17,6 +17,9 @@ package org.forgerock.openam.core.rest.authn; +import com.sun.identity.authentication.callbacks.NameValueOutputCallback; +import com.sun.identity.authentication.callbacks.ScriptTextOutputCallback; +import com.sun.identity.authentication.spi.HttpCallback; import org.forgerock.json.JsonValue; import org.forgerock.openam.core.rest.authn.callbackhandlers.RestAuthCallbackHandler; import org.forgerock.openam.core.rest.authn.exceptions.RestAuthResponseException; @@ -26,6 +29,8 @@ import org.testng.annotations.Test; import javax.security.auth.callback.Callback; +import javax.security.auth.callback.NameCallback; +import javax.security.auth.callback.PasswordCallback; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.ws.rs.core.HttpHeaders; @@ -36,6 +41,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; import static org.testng.Assert.fail; public class RestAuthCallbackHandlerManagerTest { @@ -291,4 +297,119 @@ public void shouldFailToHandleJsonCallbacksWithMissingJSONCallback() throws Rest //Then fail(); } + + @Test + public void shouldSubmitCallbacksWhenHttpCallbackHeaderIsPresentAndScriptCannotBeFilled() + throws RestAuthException { + + //Given + RestAuthCallbackHandlerManager manager = + new RestAuthCallbackHandlerManager(RestAuthCallbackHandlerFactory.getInstance()); + HttpServletRequest request = postRequest(); + HttpServletResponse response = mock(HttpServletResponse.class); + HttpCallback httpCallback = new HttpCallback("Authorization", "WWW-Authenticate:Negotiate", "401"); + ScriptTextOutputCallback script = new ScriptTextOutputCallback("p.set('skipKerberos', 'true');"); + given(request.getHeader("Authorization")).willReturn("Negotiate YIIB"); + + //When + JsonValue jsonCallbacks = manager.handleCallbacks(request, response, new Callback[]{httpCallback, script}); + + //Then + assertEquals(jsonCallbacks.size(), 0); + assertEquals(httpCallback.getAuthorization(), "Negotiate YIIB"); + } + + @Test + public void shouldSubmitNameValueOutputCallbackWhenHttpCallbackHeaderIsPresent() throws RestAuthException { + + //Given + RestAuthCallbackHandlerManager manager = + new RestAuthCallbackHandlerManager(RestAuthCallbackHandlerFactory.getInstance()); + HttpServletRequest request = postRequest(); + HttpServletResponse response = mock(HttpServletResponse.class); + HttpCallback httpCallback = new HttpCallback("Authorization", "WWW-Authenticate:Negotiate", "401"); + NameValueOutputCallback nameValue = new NameValueOutputCallback("realm", "/"); + given(request.getHeader("Authorization")).willReturn("Negotiate YIIB"); + + //When + JsonValue jsonCallbacks = manager.handleCallbacks(request, response, + new Callback[]{httpCallback, nameValue}); + + //Then + assertEquals(jsonCallbacks.size(), 0); + } + + @Test + public void shouldChallengeWhenHttpCallbackFollowsOutputCallback() throws RestAuthException { + + //Given + RestAuthCallbackHandlerManager manager = + new RestAuthCallbackHandlerManager(RestAuthCallbackHandlerFactory.getInstance()); + HttpServletRequest request = postRequest(); + HttpServletResponse response = mock(HttpServletResponse.class); + HttpCallback httpCallback = new HttpCallback("Authorization", "WWW-Authenticate:Negotiate", "401"); + ScriptTextOutputCallback script = new ScriptTextOutputCallback("p.set('skipKerberos', 'true');"); + + //When + try { + manager.handleCallbacks(request, response, new Callback[]{script, httpCallback}); + fail("Expected a 401 challenge for the missing Authorization header"); + } catch (RestAuthResponseException e) { + //Then + assertEquals(e.getStatusCode(), 401); + assertEquals(e.getResponseHeaders().get("WWW-Authenticate"), "Negotiate"); + } + } + + @Test + public void shouldFillOtherCallbacksFromRequestWhenHttpCallbackHeaderIsPresent() throws RestAuthException { + + //Given + RestAuthCallbackHandlerManager manager = + new RestAuthCallbackHandlerManager(RestAuthCallbackHandlerFactory.getInstance()); + HttpServletRequest request = postRequest(); + HttpServletResponse response = mock(HttpServletResponse.class); + HttpCallback httpCallback = new HttpCallback("Authorization", "WWW-Authenticate:Negotiate", "401"); + NameCallback nameCallback = new NameCallback("User Name:"); + ScriptTextOutputCallback script = new ScriptTextOutputCallback("p.set('skipKerberos', 'true');"); + given(request.getHeader("Authorization")).willReturn("Negotiate YIIB"); + given(request.getHeader("X-OpenAM-Username")).willReturn("demo"); + + //When + JsonValue jsonCallbacks = manager.handleCallbacks(request, response, + new Callback[]{httpCallback, nameCallback, script}); + + //Then + assertEquals(jsonCallbacks.size(), 0); + assertEquals(nameCallback.getName(), "demo"); + } + + @Test + public void shouldRejectHttpCallbackNextToInputCallbackThatCannotBeFilled() throws RestAuthException { + + //Given + RestAuthCallbackHandlerManager manager = + new RestAuthCallbackHandlerManager(RestAuthCallbackHandlerFactory.getInstance()); + HttpServletRequest request = postRequest(); + HttpServletResponse response = mock(HttpServletResponse.class); + HttpCallback httpCallback = new HttpCallback("Authorization", "WWW-Authenticate:Negotiate", "401"); + PasswordCallback passwordCallback = new PasswordCallback("Password:", false); + given(request.getHeader("Authorization")).willReturn("Negotiate YIIB"); + + //When + try { + manager.handleCallbacks(request, response, new Callback[]{httpCallback, passwordCallback}); + fail("Expected a 400 for the PasswordCallback that cannot be filled from the request"); + } catch (RestAuthException e) { + //Then + assertEquals(e.getStatusCode(), 400); + assertNull(passwordCallback.getPassword()); + } + } + + private HttpServletRequest postRequest() { + HttpServletRequest request = mock(HttpServletRequest.class); + given(request.getMethod()).willReturn("POST"); + return request; + } }