From 443209a4379590b5a0135f081b85fd5dea4084dd Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Thu, 3 Sep 2026 23:41:50 +0200 Subject: [PATCH 1/6] OF-2534: Refactor XEP-0198 stream resumption request handling Extracts the parsing of a element into a ResumeRequest value object, and the validation of a resumption attempt into a separate method that returns its outcome rather than writing a response. Neither is used by more than the existing, traditional resume flow yet; this is groundwork for inline (SASL2) resumption. Behaviour is preserved, with one exception: a element that carries no usable 'previd' attribute now results in a stream error rather than a element. Such a client cannot resume in any case, and will reconnect without stream management. --- .../MalformedResumeRequestException.java | 28 ++ .../streammanagement/ResumeRequest.java | 167 +++++++++++ .../ResumeRequestValidationResult.java | 95 ++++++ .../streammanagement/StreamManager.java | 147 +++++---- .../streammanagement/ResumeRequestTest.java | 283 ++++++++++++++++++ .../ResumeRequestValidationResultTest.java | 62 ++++ 6 files changed, 723 insertions(+), 59 deletions(-) create mode 100644 xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/MalformedResumeRequestException.java create mode 100644 xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequest.java create mode 100644 xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResult.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestTest.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResultTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/MalformedResumeRequestException.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/MalformedResumeRequestException.java new file mode 100644 index 0000000000..917a69b5e8 --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/MalformedResumeRequestException.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.streammanagement; + +/** + * Thrown by {@link ResumeRequest} when a XEP-0198 {@code } element cannot be parsed, because it is + * missing a required attribute, or one of its attributes has an illegal value. + */ +public class MalformedResumeRequestException extends Exception +{ + public MalformedResumeRequestException(final String message) + { + super(message); + } +} diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequest.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequest.java new file mode 100644 index 0000000000..c21815073f --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequest.java @@ -0,0 +1,167 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.streammanagement; + +import org.dom4j.Element; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +/** + * Represents a XEP-0198 stream resumption request: the data carried by a {@code } element. + * + * This is used by both variants of stream resumption defined by XEP-0198: + *
    + *
  • the traditional flow, in which the client sends a top-level {@code } element after having + * established a stream (see § 5, "Resumption"); and
  • + *
  • the inline flow, in which the {@code } element is instead nested inside a SASL2 (XEP-0388) + * {@code } element (see § 9.2, "Inline Stream Resumption").
  • + *
+ * + * Either way, the element takes the same shape: + *
{@code
+ * 
+ * }
+ */ +public final class ResumeRequest +{ + private static final String ELEMENT_NAME = "resume"; + + private final String previd; + private final long h; + private final String namespace; + + /** + * Constructs a request. + * + * @param previd the SM-ID of the former stream that the client wishes to resume (cannot be null). + * @param h the sequence number of the last stanza that the client received from the server over the former stream. + * @param namespace the Stream Management namespace that the client used for this request (cannot be null). + */ + private ResumeRequest(@Nonnull final String previd, final long h, @Nonnull final String namespace) + { + this.previd = previd; + this.h = h; + this.namespace = namespace; + } + + /** + * Parses a (traditional, top-level) XEP-0198 {@code } element. + * + * @param resumeElement the {@code } element (cannot be null). + * @return the parsed request (never null). + * @throws MalformedResumeRequestException if the element is malformed. + */ + @Nonnull + public static ResumeRequest from(@Nonnull final Element resumeElement) throws MalformedResumeRequestException + { + return parse(resumeElement, resumeElement.getNamespaceURI()); + } + + /** + * Parses the inline XEP-0198 {@code } content of a SASL2 {@code } element. + * + * Returns {@code null} when the element carries no {@code } child (in a namespace recognized as a + * Stream Management namespace), in which case there is nothing for the caller to act on. + * + * @param authenticateElement the {@code } element (cannot be null). + * @return the parsed request, or {@code null} if the element carries no inline resume request. + * @throws MalformedResumeRequestException if a {@code } element is present but is malformed. + */ + @Nullable + public static ResumeRequest fromSasl2Authenticate(@Nonnull final Element authenticateElement) throws MalformedResumeRequestException + { + final Element resumeElement = authenticateElement.element(ELEMENT_NAME); + if (resumeElement == null) { + return null; + } + + final String namespace = resumeElement.getNamespaceURI(); + if (!StreamManager.NAMESPACE_V3.equals(namespace) && !StreamManager.NAMESPACE_V2.equals(namespace)) { + // Not a Stream Management resume request (could be some other, unrelated, element named 'resume'). + return null; + } + + return parse(resumeElement, namespace); + } + + @Nonnull + private static ResumeRequest parse(@Nonnull final Element resumeElement, @Nonnull final String namespace) throws MalformedResumeRequestException + { + final String previd = resumeElement.attributeValue("previd"); + if (previd == null || previd.isEmpty()) { + throw new MalformedResumeRequestException("Stream resumption requires a 'previd' attribute."); + } + + final String hValue = resumeElement.attributeValue("h"); + if (hValue == null || hValue.isEmpty()) { + throw new MalformedResumeRequestException("Stream resumption requires an 'h' attribute."); + } + + final long h; + try { + h = Long.parseLong(hValue); + } catch (final NumberFormatException e) { + throw new MalformedResumeRequestException("Stream resumption 'h' attribute must be a number, but was: " + hValue); + } + if (h < 0 || h > StreamManager.MASK) { + // XEP-0198 § 4: 'h' is an unsigned 32-bit integer. Out-of-range values are rejected here rather than + // later by StreamManager#validateClientAcknowledgement(long), which throws for them. + throw new MalformedResumeRequestException("Stream resumption 'h' attribute must be an unsigned 32-bit integer, but was: " + h); + } + + return new ResumeRequest(previd, h, namespace); + } + + /** + * Returns the SM-ID of the former stream that the client wishes to resume. + * + * @return the (still Base64-encoded) SM-ID. + */ + @Nonnull + public String getPrevId() + { + return previd; + } + + /** + * Returns the sequence number of the last stanza that the client received from the server over the former + * stream. + * + * @return a non-negative sequence number. + */ + public long getH() + { + return h; + } + + /** + * Returns the Stream Management namespace that the client used for this request. + * + * @return a Stream Management namespace. + */ + @Nonnull + public String getNamespace() + { + return namespace; + } + + @Override + public String toString() + { + return "ResumeRequest{previd='" + previd + "', h=" + h + ", namespace='" + namespace + "'}"; + } +} diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResult.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResult.java new file mode 100644 index 0000000000..2faf5c87aa --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResult.java @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.streammanagement; + +import org.jivesoftware.openfire.session.LocalClientSession; +import org.xmpp.packet.PacketError; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +/** + * Outcome of StreamManager#validateResumeRequest(ResumeRequest): either the pre-existing session that + * is to be resumed, or the condition to report as to why the resumption request cannot be honored. + */ +final class ResumeRequestValidationResult +{ + @Nullable private final LocalClientSession target; + @Nullable private final PacketError.Condition failureCondition; + + private ResumeRequestValidationResult(@Nullable final LocalClientSession target, @Nullable final PacketError.Condition failureCondition) + { + this.target = target; + this.failureCondition = failureCondition; + } + + /** + * Creates a result representing a successfully validated resume request. + * + * @param target the pre-existing session that is to be resumed (cannot be null). + * @return a success result. + */ + static ResumeRequestValidationResult success(@Nonnull final LocalClientSession target) + { + return new ResumeRequestValidationResult(target, null); + } + + /** + * Creates a result representing a resume request that failed validation. + * + * @param condition the condition to report as to why the resumption request cannot be honored (cannot be null). + * @return a failure result. + */ + static ResumeRequestValidationResult failure(@Nonnull final PacketError.Condition condition) + { + return new ResumeRequestValidationResult(null, condition); + } + + /** + * Returns whether the resume request was successfully validated. + * + * @return {@code true} if the request is valid and a target session was found. + */ + boolean isSuccess() + { + return target != null; + } + + /** + * Returns the pre-existing session that is to be resumed. Only set when {@link #isSuccess()} returns {@code true}. + * Guaranteed to be non-null when {@link #isSuccess()} returns {@code true}. + * + * @return the target session, or {@code null} on failure. + */ + @Nullable + LocalClientSession getTarget() + { + return target; + } + + /** + * Returns the condition to report as to why the resumption request cannot be honored. Only set when + * {@link #isSuccess()} returns {@code false}. Guaranteed to be non-null when {@link #isSuccess()} + * returns {@code false}. + * + * @return the failure condition, or {@code null} on success. + */ + @Nullable + PacketError.Condition getFailureCondition() + { + return failureCondition; + } +} diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java index 2f365f2e78..64573968b1 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java @@ -120,9 +120,9 @@ public static boolean isStreamManagementActive() { private AtomicLong clientProcessedStanzas = new AtomicLong( 0 ); /** - * The value (2^32)-1, used to emulate roll-over + * The value (2^32)-1, used to emulate roll-over and the largest legal value for the 'h' attribute (XEP-0198 § 4). */ - private static final long MASK = new BigInteger( "2" ).pow( 32 ).longValue() - 1; + static final long MASK = new BigInteger( "2" ).pow( 32 ).longValue() - 1; /** * Collection of stanzas/packets sent to client that haven't been acknowledged. @@ -183,26 +183,17 @@ public void process( Element element ) enable( element.getNamespace().getStringValue(), resume ); break; case "resume": - final String hValue = element.attributeValue("h"); - final long h; + final ResumeRequest resumeRequest; try { - h = Long.parseLong(hValue); - } catch (NumberFormatException e) { - Log.warn( "Closing client session. Client sends non-numeric value for SM 'h': {}, affected session: {}", hValue, session ); - final StreamError error = new StreamError( StreamError.Condition.undefined_condition, "You acknowledged stanzas using a 'h' value that is not a number (which is illegal). Your Ack h: " + hValue + ", our last unacknowledged stanza: " + (unacknowledgedServerStanzas.isEmpty() ? "(none)" : unacknowledgedServerStanzas.getLast().x) ); + resumeRequest = ResumeRequest.from(element); + } catch (MalformedResumeRequestException e) { + Log.info( "Closing client session that sent a malformed 'resume' request. Error message: {}. Affected session: {}", e.getMessage(), session ); + final StreamError error = new StreamError( StreamError.Condition.undefined_condition, e.getMessage() + " Our last unacknowledged stanza: " + (unacknowledgedServerStanzas.isEmpty() ? "(none)" : unacknowledgedServerStanzas.getLast().x) ); session.deliverRawText( error.toXML() ); session.close(); return; } - if (h < 0) { - Log.warn( "Closing client session. Client sends negative value for SM 'h': {}, affected session: {}", h, session ); - final StreamError error = new StreamError( StreamError.Condition.undefined_condition, "You acknowledged stanzas using a negative value (which is illegal). Your Ack h: " + h + ", our last unacknowledged stanza: " + (unacknowledgedServerStanzas.isEmpty() ? "(none)" : unacknowledgedServerStanzas.getLast().x) ); - session.deliverRawText( error.toXML() ); - session.close(); - return; - } - String previd = element.attributeValue("previd"); - startResume( element.getNamespaceURI(), previd, h); + processResume( resumeRequest ); break; case "r": @@ -313,30 +304,87 @@ public Element enableAndBuildElement( String namespace, boolean resume ) throws return enabled; } - private void startResume(String namespace, String previd, long h) { + /** + * Attempts to process (validate and perform) a {@code } request, as defined by XEP-0198. + * + * This writes its response (either a stream error, or the effects of + * {@link LocalSession#reattach(LocalSession, long)}) directly to the connection. + * + * @param request the parsed resume request (cannot be null). + */ + private void processResume(@Nonnull final ResumeRequest request) + { + this.namespace = request.getNamespace(); + + final ResumeRequestValidationResult validation = validateResumeRequest(request); + if (!validation.isSuccess()) { + assert validation.getFailureCondition() != null; // Per definition of the method contract. + sendError(new PacketError(validation.getFailureCondition())); + return; + } + + final LocalClientSession otherSession = validation.getTarget(); + assert otherSession != null; // Per definition of the method contract. + detachIfNeeded(otherSession); + + // If we're all happy, re-attach the connection from the pre-existing session to the new session, discarding the old session. + Log.debug("Attaching to other session '{}'.", otherSession.getStreamID()); + otherSession.reattach(session, request.getH()); + Log.debug("Perform resumption of session {}, using connection from session {}", otherSession.getStreamID(), session.getStreamID()); + } + + /** + * Detaches the connection of a to-be-resumed session, unless it is already detached. + * + * @param otherSession the pre-existing session that is about to be resumed. + */ + private void detachIfNeeded(@Nonnull final LocalClientSession otherSession) + { + if (!otherSession.isDetached()) { + Log.debug("Existing session {} is not detached; detaching.", otherSession.getStreamID()); + final Connection oldConnection = otherSession.getConnection(); + otherSession.setDetached(); + assert oldConnection != null; // If the other session is not detached, the connection can't be null. + oldConnection.close(new StreamError(StreamError.Condition.conflict, "The stream previously served over this connection is resumed on a new connection.")); + } + } + + /** + * Validates a stream resumption request, without performing any of the state changes + * (detaching/reattaching) that are needed to actually resume the session. + * + * @param request the parsed resume request (cannot be null). + * @return the outcome of the validation. + */ + @Nonnull + private ResumeRequestValidationResult validateResumeRequest(@Nonnull final ResumeRequest request) + { + final String namespace = request.getNamespace(); + final String previd = request.getPrevId(); + final long h = request.getH(); + Log.debug("Attempting resumption for {}, h={}", previd, h); - this.namespace = namespace; + // Ensure that resource binding has NOT occurred. - if (!allowResume() ) { + if (!allowResume()) { Log.debug("Unable to process session resumption attempt, as session {} is in a state where session resumption is not allowed.", session); - sendUnexpectedError(); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } + if (session.isAuthenticated()) { - Log.debug("Unable to process session resumption attempt, as session {} is not authenticated.", session); - sendUnexpectedError(); - return; + Log.debug("Unable to process session resumption attempt, as session {} is already authenticated.", session); + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } + AuthToken authToken = null; - // Ensure that resource binding has occurred. if (session instanceof ClientSession) { authToken = ((LocalClientSession) session).getAuthToken(); } if (authToken == null) { Log.debug("Unable to process session resumption attempt, as session {} does not provide any auth context.", session); - sendUnexpectedError(); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } + // Decode previd. String resource; String streamId; @@ -346,8 +394,7 @@ private void startResume(String namespace, String previd, long h) { streamId = toks.nextToken(); } catch (Exception e) { Log.debug("Exception from previd decode:", e); - sendUnexpectedError(); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } final JID fullJid; if ( authToken.isAnonymous() ){ @@ -365,66 +412,48 @@ private void startResume(String namespace, String previd, long h) { // When the client tries to resume a connection on this host, it is unlikely to try other hosts. Remove any detached sessions living elsewhere in the cluster. (OF-2753) CacheFactory.doClusterTask(new ClientSessionTask(fullJid, RemoteSessionTask.Operation.removeDetached)); } - sendError(new PacketError(PacketError.Condition.item_not_found)); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.item_not_found); } - if (!(route instanceof LocalClientSession)) { + if (!(route instanceof LocalClientSession otherSession)) { Log.debug("Not allowing a client of '{}' to resume a session on this cluster node. The session can only be resumed on the Openfire cluster node where the original session was connected.", fullJid); if (LOCATION_TERMINATE_OTHERS_ENABLED.getValue()) { // When the client tries to resume a connection on this host, it is unlikely to try other hosts. Remove any detached sessions living elsewhere in the cluster. (OF-2753) CacheFactory.doClusterTask(new ClientSessionTask(fullJid, RemoteSessionTask.Operation.removeDetached)); } - sendError(new PacketError(PacketError.Condition.unexpected_request)); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } - final LocalClientSession otherSession = (LocalClientSession) route; if (!otherSession.getStreamID().getID().equals(streamId)) { - sendError(new PacketError(PacketError.Condition.item_not_found)); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.item_not_found); } Log.debug("Found existing session for '{}', checking status", fullJid); // OF-2811: Cannot resume a session that's already closed. That session is likely busy firing its 'closeListeners'. if (route.isClosed()) { Log.debug("Not allowing a client of '{}' to resume a session, as the preexisting session is already in process of being closed.", fullJid); - sendError(new PacketError(PacketError.Condition.unexpected_request)); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } // Previd identifies proper session. Now check SM status if (!otherSession.getStreamManager().resume) { Log.debug("Not allowing a client of '{}' to resume a session, the session to be resumed does not have the stream management resumption feature enabled.", fullJid); - sendError(new PacketError(PacketError.Condition.unexpected_request)); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } if (otherSession.getStreamManager().namespace == null) { Log.debug("Not allowing a client of '{}' to resume a session, the session to be resumed disabled SM functionality as a response to an earlier error.", fullJid); - sendError(new PacketError(PacketError.Condition.unexpected_request)); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } if (!otherSession.getStreamManager().namespace.equals(namespace)) { Log.debug("Not allowing a client of '{}' to resume a session, the session to be resumed used a different version ({}) of the session management resumption feature as compared to the version that's requested now: {}.", fullJid, otherSession.getStreamManager().namespace, namespace); - sendError(new PacketError(PacketError.Condition.unexpected_request)); - return; + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } if (!otherSession.getStreamManager().validateClientAcknowledgement(h)) { Log.debug("Not allowing a client of '{}' to resume a session, as it reports it received more stanzas from us than that we've send it.", fullJid); - sendError(new PacketError(PacketError.Condition.unexpected_request)); - return; - } - if (!otherSession.isDetached()) { - Log.debug("Existing session {} of '{}' is not detached; detaching.", otherSession.getStreamID(), fullJid); - Connection oldConnection = otherSession.getConnection(); - otherSession.setDetached(); - assert oldConnection != null; // If the other session is not detached, the connection can't be null. - oldConnection.close(new StreamError(StreamError.Condition.conflict, "The stream previously served over this connection is resumed on a new connection.")); + return ResumeRequestValidationResult.failure(PacketError.Condition.unexpected_request); } - Log.debug("Attaching to other session '{}' of '{}'.", otherSession.getStreamID(), fullJid); - // If we're all happy, re-attach the connection from the pre-existing session to the new session, discarding the old session. - otherSession.reattach(session, h); - Log.debug("Perform resumption of session {} for '{}', using connection from session {}", otherSession.getStreamID(), fullJid, session.getStreamID()); + + return ResumeRequestValidationResult.success(otherSession); } /** diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestTest.java new file mode 100644 index 0000000000..69e2625ed4 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestTest.java @@ -0,0 +1,283 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.streammanagement; + +import org.dom4j.DocumentHelper; +import org.dom4j.Element; +import org.dom4j.Namespace; +import org.dom4j.QName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Verifies {@link ResumeRequest}, which parses a XEP-0198 {@code } element, be it a traditional top-level + * one, or one nested inline inside a SASL2 {@code } element. + */ +public class ResumeRequestTest +{ + /** + * Verifies that a valid, traditional, top-level {@code } element is parsed correctly. + */ + @Test + public void testFromTraditionalResumeElement() throws Exception + { + // Setup test fixture. + final Element resume = DocumentHelper.createElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "5"); + + // Execute system under test. + final ResumeRequest result = ResumeRequest.from(resume); + + // Verify result. + assertEquals("cHJldmlk", result.getPrevId(), "Expected the parsed previd value to match the value in the 'previd' attribute."); + assertEquals(5L, result.getH(), "Expected the parsed h value to match the value in the 'h' attribute."); + assertEquals(StreamManager.NAMESPACE_V3, result.getNamespace(), "Expected the parsed namespace to match the namespace used by the 'resume' element."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when a traditional, top-level + * {@code } element is missing its required {@code h} attribute. + */ + @Test + public void testFromTraditionalResumeElementMissingH() throws Exception + { + // Setup test fixture. + final Element resume = DocumentHelper.createElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.from(resume), "Expected a MalformedResumeRequestException to be thrown, as the 'resume' element is missing its required 'h' attribute."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when a traditional, top-level + * {@code } element is missing its required {@code previd} attribute. + */ + @Test + public void testFromTraditionalResumeElementMissingPrevid() + { + // Setup test fixture. + final Element resume = DocumentHelper.createElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("h", "5"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.from(resume), "Expected a MalformedResumeRequestException to be thrown, as the 'resume' element is missing its required 'previd' attribute."); + } + + /** + * Verifies that {@code null} is returned when the {@code } element does not contain a + * {@code } child element at all. + */ + @Test + public void testFromElementWithoutResumeElement() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + + // Execute system under test. + final ResumeRequest result = ResumeRequest.fromSasl2Authenticate(authenticate); + + // Verify result. + assertNull(result, "Expected no ResumeRequest to be returned, as the input has no 'resume' child element."); + } + + /** + * Verifies that {@code null} is returned when the {@code } child element uses a namespace other than + * one of the recognized XEP-0198 namespaces. + */ + @Test + public void testFromElementWithWrongNamespace() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(QName.get("resume", "wrong:namespace")); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "5"); + + // Execute system under test. + final ResumeRequest result = ResumeRequest.fromSasl2Authenticate(authenticate); + + // Verify result. + assertNull(result, "Expected no ResumeRequest to be returned, as the 'resume' element uses an unrecognized namespace."); + } + + /** + * Verifies that a valid {@code } element using the XEP-0198 v3 namespace is parsed correctly. + */ + @Test + public void testFromValidResumeElementV3() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "5"); + + // Execute system under test. + final ResumeRequest result = ResumeRequest.fromSasl2Authenticate(authenticate); + + // Verify result. + assertNotNull(result, "Expected a ResumeRequest to be returned, as the input contains a valid 'resume' element."); + assertEquals("cHJldmlk", result.getPrevId(), "Expected the parsed previd value to match the value in the 'previd' attribute."); + assertEquals(5L, result.getH(), "Expected the parsed h value to match the value in the 'h' attribute."); + assertEquals(StreamManager.NAMESPACE_V3, result.getNamespace(), "Expected the parsed namespace to match the namespace used by the 'resume' element."); + } + + /** + * Verifies that a valid {@code } element using the XEP-0198 v2 namespace is parsed correctly. + */ + @Test + public void testFromValidResumeElementV2() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V2))); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "0"); + + // Execute system under test. + final ResumeRequest result = ResumeRequest.fromSasl2Authenticate(authenticate); + + // Verify result. + assertNotNull(result, "Expected a ResumeRequest to be returned, as the input contains a valid 'resume' element."); + assertEquals("cHJldmlk", result.getPrevId(), "Expected the parsed previd value to match the value in the 'previd' attribute."); + assertEquals(0L, result.getH(), "Expected the parsed h value to match the value in the 'h' attribute."); + assertEquals(StreamManager.NAMESPACE_V2, result.getNamespace(), "Expected the parsed namespace to match the namespace used by the 'resume' element."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when the + * {@code } element is missing its required {@code previd} attribute. + */ + @Test + public void testFromElementMissingPrevid() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("h", "5"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.fromSasl2Authenticate(authenticate), "Expected a MalformedResumeRequestException to be thrown, as the 'resume' element is missing its required 'previd' attribute."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when the + * {@code } element is missing its required {@code h} attribute. + */ + @Test + public void testFromElementMissingH() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.fromSasl2Authenticate(authenticate), "Expected a MalformedResumeRequestException to be thrown, as the 'resume' element is missing its required 'h' attribute."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when the + * {@code h} attribute of the {@code } element does not contain a valid number. + */ + @Test + public void testFromElementMalformedH() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "not-a-number"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.fromSasl2Authenticate(authenticate), "Expected a MalformedResumeRequestException to be thrown, as the 'h' attribute is not a valid number."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when the + * {@code h} attribute of the {@code } element is a negative number. + */ + @Test + public void testFromElementNegativeH() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "-1"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.fromSasl2Authenticate(authenticate), "Expected a MalformedResumeRequestException to be thrown, as the 'h' attribute is negative."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when the + * {@code previd} attribute of the {@code } element is empty. + */ + @Test + public void testFromElementEmptyPrevid() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", ""); + resume.addAttribute("h", "5"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.fromSasl2Authenticate(authenticate), "Expected a MalformedResumeRequestException to be thrown, as the 'previd' attribute is empty."); + } + + /** + * Verifies that the largest legal value for the {@code h} attribute (XEP-0198 § 4 defines it as an unsigned + * 32-bit integer) is accepted. + */ + @Test + public void testFromElementMaximumH() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "4294967295"); + + // Execute system under test. + final ResumeRequest result = ResumeRequest.fromSasl2Authenticate(authenticate); + + // Verify result. + assertNotNull(result, "Expected a ResumeRequest to be returned, as the input contains a valid 'resume' element."); + assertEquals(4294967295L, result.getH(), "Expected the parsed h value to match the value in the 'h' attribute."); + } + + /** + * Verifies that a {@link MalformedResumeRequestException} is thrown when the {@code h} attribute of the + * {@code } element exceeds the largest legal value for an unsigned 32-bit integer. + */ + @Test + public void testFromElementExcessiveH() throws Exception + { + // Setup test fixture. + final Element authenticate = DocumentHelper.createElement("authenticate"); + final Element resume = authenticate.addElement(new QName("resume", new Namespace("", StreamManager.NAMESPACE_V3))); + resume.addAttribute("previd", "cHJldmlk"); + resume.addAttribute("h", "4294967296"); + + // Execute system under test & verify result. + assertThrows(MalformedResumeRequestException.class, () -> ResumeRequest.fromSasl2Authenticate(authenticate), "Expected a MalformedResumeRequestException to be thrown, as the 'h' attribute exceeds the largest legal value."); + } +} diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResultTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResultTest.java new file mode 100644 index 0000000000..37d7735122 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/ResumeRequestValidationResultTest.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.streammanagement; + +import org.jivesoftware.openfire.session.LocalClientSession; +import org.junit.jupiter.api.Test; +import org.xmpp.packet.PacketError; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; + +/** + * Verifies {@link ResumeRequestValidationResult}. + */ +public class ResumeRequestValidationResultTest +{ + /** + * Verifies that a successful result reports success, exposes the target session, and has no failure condition. + */ + @Test + public void testSuccess() throws Exception + { + // Setup test fixture. + final LocalClientSession target = mock(LocalClientSession.class); + + // Execute system under test. + final ResumeRequestValidationResult result = ResumeRequestValidationResult.success(target); + + // Verify result. + assertTrue(result.isSuccess(), "Expected a success result to report success."); + assertEquals(target, result.getTarget(), "Expected a success result to expose the provided target session."); + assertNull(result.getFailureCondition(), "Expected a success result to have no failure condition."); + } + + /** + * Verifies that a failure result reports no success, exposes the failure condition, and has no target session. + */ + @Test + public void testFailure() throws Exception + { + // Execute system under test. + final ResumeRequestValidationResult result = ResumeRequestValidationResult.failure(PacketError.Condition.item_not_found); + + // Verify result. + assertFalse(result.isSuccess(), "Expected a failure result to not report success."); + assertNull(result.getTarget(), "Expected a failure result to expose no target session."); + assertEquals(PacketError.Condition.item_not_found, result.getFailureCondition(), "Expected a failure result to expose the provided failure condition."); + } +} From e6036c81e58e2a4ef63b20aeb59859541e9859e7 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Thu, 3 Sep 2026 23:42:50 +0200 Subject: [PATCH 2/6] OF-2534: Defer post-resumption state restore until the client has been told the stream resumed LocalClientSession restores Client State Indication to 'active' when a stream is resumed (XEP-0352). CsiManager#activate() is not a flag flip: it drains the delay queue and pushes those stanzas to the connection synchronously. That must not happen until the resumption has been confirmed to the client and unacknowledged stanzas have been retransmitted, or those stanzas precede on the wire and are retransmitted a second time. The hook is therefore invoked at the tail of each resumption flow rather than from the shared connection-transfer helper. This also introduces the SASL2 variants, which are not used until inline stream resumption is added, and splits StreamManager#onResume() so that building the element and retransmitting unacknowledged stanzas can be sequenced separately. --- .../openfire/session/LocalClientSession.java | 5 +- .../openfire/session/LocalSession.java | 63 ++++++++++++++++++- .../streammanagement/StreamManager.java | 39 +++++++++++- 3 files changed, 99 insertions(+), 8 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java index a0ba40ba3e..acad4f595b 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java @@ -755,9 +755,10 @@ public void setOfflineFloodStopped(boolean offlineFloodStopped) { } } - public void reattach(LocalSession connectionProvider, long h) + @Override + protected void onReattached() { - super.reattach(connectionProvider, h); + super.onReattached(); // XEP-0352: "After a previous stream was resumed using mechanisms like Stream Management (XEP-0198), the CSI // state is not restored. That is, stream resumption does not affect the current CSI state, which always diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java index 9e0cbdcc90..324fb7c1e0 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java @@ -187,6 +187,53 @@ Connection releaseConnection() * @param h the sequence number of the last handled stanza sent over the former stream */ public void reattach(LocalSession connectionProvider, long h) { + reattachConnection(connectionProvider); + this.streamManager.onResume(new JID(null, this.serverName, null, true), h); + this.sessionManager.removeSession((LocalClientSession) connectionProvider); + onReattached(); + } + + /** + * Reattach the (existing) session to the connection provided by a new session, for an inline XEP-0198 resume + * request that is embedded in a SASL2 (XEP-0388) authentication exchange (see XEP-0198 § 9.2). + * + * This transfers the connection exactly as {@link #reattach(LocalSession, long)} does, but, unlike that method, + * does not complete the resumption: a SASL2 caller cannot have the stream manager write a + * {@code } directly to the connection, since that element needs to be embedded in the SASL2 + * {@code } response that the caller is still constructing. Callers must invoke + * {@link #completeSasl2Resume(long)} after they have written that response. + * + * @param connectionProvider Session from which to obtain the connection from. + */ + public void reattachForSasl2(LocalSession connectionProvider) { + reattachConnection(connectionProvider); + this.sessionManager.removeSession((LocalClientSession) connectionProvider); + } + + /** + * Completes an inline SASL2 resumption, after the caller has delivered the SASL2 {@code } response + * carrying the {@code } element built by StreamManager#buildResumedElement(). + * + * This performs the second half of what {@link StreamManager#onResume(JID, long)} does for the traditional flow: + * it processes the client's acknowledgement, retransmits anything still unacknowledged, and then invokes + * {@link #onReattached()}. It must not be invoked before the {@code } has been written: everything it + * delivers would otherwise precede the resumption confirmation on the wire. + * + * @param h the sequence number of the last handled stanza, as reported by the resuming client. + */ + public void completeSasl2Resume(final long h) { + this.streamManager.redeliverUnackedStanzas(new JID(null, this.serverName, null, true), h); + onReattached(); + } + + /** + * Transfers the connection of connectionProvider to this session, closing any (stale) connection that this session + * might still have. Note that this does not invoke onReattached(): that is deferred until the resumption has been + * confirmed to the client (see reattach(LocalSession, long) and completeSasl2Resume(long)). + * + * @param connectionProvider Session from which to obtain the connection from. + */ + private void reattachConnection(LocalSession connectionProvider) { lock.lock(); try { Log.debug("Reattaching session with address {} and streamID {} using connection from session with address {} and streamID {}.", this.address, this.streamID, connectionProvider.getAddress(), connectionProvider.getStreamID()); @@ -196,13 +243,23 @@ public void reattach(LocalSession connectionProvider, long h) { } this.conn = connectionProvider.releaseConnection(); this.conn.reinit(this); - }finally { + } finally { lock.unlock(); } this.status = Session.Status.AUTHENTICATED; this.sessionManager.removeDetached(this); - this.streamManager.onResume(new JID(null, this.serverName, null, true), h); - this.sessionManager.removeSession((LocalClientSession)connectionProvider); + } + + /** + * Hook invoked after this session's connection has been transferred from another session, either through + * {@link #reattach(LocalSession, long)} or {@link #completeSasl2Resume(long)}. + * + * This is invoked only after the resumption has been confirmed and unacknowledged stanzas retransmitted. + * + * The default implementation does nothing; subclasses can override this to restore state that a resumed stream is + * expected to reset. + */ + protected void onReattached() { } /** diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java index 64573968b1..f832f8d1db 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java @@ -698,15 +698,48 @@ public void onClose(PacketRouter router, JID serverAddress) { public void onResume(JID serverAddress, long h) { Log.debug("Agreeing to resume"); - Element resumed = new DOMElement(QName.get("resumed", namespace)); - resumed.addAttribute("previd", Base64.getEncoder().encodeToString((session.getAddress().getResource() + "\0" + session.getStreamID().getID()).getBytes(StandardCharsets.UTF_8))); - resumed.addAttribute("h", Long.toString(serverProcessedStanzas.get())); + final Element resumed = buildResumedElement(); final Connection connection = session.getConnection(); assert connection != null; // While the client is resuming a session, the connection on which the session is resumed can't be null. connection.deliverRawText(resumed.asXML()); + redeliverUnackedStanzas(serverAddress, h); + } + + /** + * Constructs the XEP-0198 {@code } element for this session's stream manager, without delivering it. + * + * This is used by the traditional resume flow (through {@link #onResume(JID, long)}, which sends it directly), + * as well as by the inline SASL2 resume flow, which instead needs to embed the element in a SASL2 + * {@code } response, rather than write it to the connection itself. + * + * @return the {@code } element. + */ + @Nonnull + Element buildResumedElement() { + final Element resumed = new DOMElement(QName.get("resumed", namespace)); + resumed.addAttribute("previd", Base64.getEncoder().encodeToString((session.getAddress().getResource() + "\0" + session.getStreamID().getID()).getBytes(StandardCharsets.UTF_8))); + resumed.addAttribute("h", Long.toString(serverProcessedStanzas.get())); + return resumed; + } + + /** + * Processes the client's acknowledgement of 'h' as reported in its (traditional or inline SASL2) resume request, + * and retransmits any stanzas that remain unacknowledged after that. + * + * This is the second half of what {@link #onResume(JID, long)} does for the traditional resume flow. It is + * split out so that the inline SASL2 resume flow can defer this until after it has delivered its own response + * (typically, the SASL2 {@code } that embeds the {@code } element built by + * {@link #buildResumedElement()}), handled by {@link LocalSession#completeSasl2Resume(long)}. + * + * @param serverAddress this server's bare-domain address, used to stamp delay information on redelivered stanzas. + * @param h the sequence number of the last handled stanza, as reported by the client that is resuming. + */ + public void redeliverUnackedStanzas(@Nonnull final JID serverAddress, final long h) { Log.debug("Resuming session: Ack for {}", h); processClientAcknowledgement(h); Log.debug("Processing remaining unacked stanzas"); + final Connection connection = session.getConnection(); + assert connection != null; // While the client is resuming a session, the connection on which the session is resumed can't be null. // Re-deliver unacknowledged stanzas from broken stream (XEP-0198) synchronized (this) { if(isEnabled()) { From 7c434cc1c3b1f6e0afb7512eaa80c213330cf726 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Thu, 3 Sep 2026 23:43:40 +0200 Subject: [PATCH 3/6] OF-2534: Separate SASL2 post-authentication handling from feature delivery Splits StanzaHandler#sasl2Successful() into the post-authentication step and the transport-specific delivery of the stream features that it emits. Only the latter is overridden by transports with different framing, which leaves the former free to gain behaviour that applies to every transport. No functional change: sasl2Successful() delivers features exactly as before. --- .../org/jivesoftware/openfire/net/StanzaHandler.java | 10 ++++++++-- .../websocket/WebSocketClientStanzaHandler.java | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java index e6a9beb163..0ff8d3caa7 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -581,10 +581,16 @@ protected void saslSuccessful() { /** * Emits post-authentication stream features for SASL2 (XEP-0388), which does NOT restart the stream. - * On TCP the features element is sent inline in the existing stream. Transports with different framing - * (e.g. RFC 7395 WebSocket) override this. */ protected void sasl2Successful() { + deliverSasl2Features(); + } + + /** + * Delivers post-authentication stream features for SASL2 (XEP-0388). On TCP the features element is sent + * inline in the existing stream. Transports with different framing (e.g. RFC 7395 WebSocket) override this. + */ + protected void deliverSasl2Features() { final Element features = generateFeatures(); connection.deliverRawText(features.asXML()); } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java index 12f4911dc7..a2feca77fc 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java @@ -207,7 +207,7 @@ protected void saslSuccessful() { * updated features, as their own RFC 7395 frame. */ @Override - protected void sasl2Successful() { + protected void deliverSasl2Features() { sendStreamFeatures(); } From fb71b37b89bacdaa946b1b9106574e1b66f4cfa5 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Thu, 3 Sep 2026 23:44:25 +0200 Subject: [PATCH 4/6] =?UTF-8?q?OF-2534:=20Add=20inline=20stream=20resumpti?= =?UTF-8?q?on=20to=20the=20SASL2=20authentication=20flow=20(XEP-0198=20?= =?UTF-8?q?=C2=A7=209.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allows a client to nest a XEP-0198 element in its SASL2 element, resuming a pre-existing session in the same round trip as authentication. Support for this is advertised in the element of the SASL2 stream feature, as the XEP requires; without that advertisement no client will attempt it. A resume request is processed before any Bind2 request. When it succeeds, the carries , is delivered over the resumed session, no resource is bound, and no stream features follow. When it fails, the carries and the client proceeds to (Bind2) resource binding as usual. --- .../openfire/net/SASLAuthentication.java | 289 ++++++++++++------ .../openfire/net/SaslStreamFeatures.java | 7 +- .../openfire/net/StanzaHandler.java | 66 +++- .../streammanagement/Sasl2ResumeResult.java | 106 +++++++ .../streammanagement/StreamManager.java | 73 ++++- .../openfire/net/SaslStreamFeaturesTest.java | 84 +++++ .../net/StanzaHandlerSasl2ResumeTest.java | 180 +++++++++++ .../Sasl2ResumeResultTest.java | 67 ++++ 8 files changed, 778 insertions(+), 94 deletions(-) create mode 100644 xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResult.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerSasl2ResumeTest.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResultTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java index 3883f3efe0..0aec5d24ef 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java @@ -38,6 +38,9 @@ import org.jivesoftware.openfire.sasl.SaslMechanismEligibility; import org.jivesoftware.openfire.sasl.ScramSaslServer; import org.jivesoftware.openfire.session.*; +import org.jivesoftware.openfire.streammanagement.MalformedResumeRequestException; +import org.jivesoftware.openfire.streammanagement.ResumeRequest; +import org.jivesoftware.openfire.streammanagement.Sasl2ResumeResult; import org.jivesoftware.util.CertificateManager; import org.jivesoftware.util.SystemProperty; import org.slf4j.Logger; @@ -46,6 +49,7 @@ import org.xmpp.packet.StreamError; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.security.sasl.Sasl; import javax.security.sasl.SaslException; import javax.security.sasl.SaslServer; @@ -216,6 +220,23 @@ public class SASLAuthentication { */ public static final String AVAILABLE_CHANNEL_BINDING_TYPES_FOR_SESSION = "ChannelBindingTypesOfferedByServer"; + /** + * Session Data property name used, on the temporary session that is negotiating a SASL2 authentication, to hand + * off the pre-existing session that was resumed inline (XEP-0198 § 9.2) to the caller of {@link #handle(LocalSession, Element, boolean)}. + * + * The resumed session itself cannot be represented in {@link Status}, so this session data, together with + * {@link Status#authenticatedResumed}, is used instead to communicate the outcome. A caller (typically + * {@code StanzaHandler}) is expected to switch to using the referenced session, and to remove this session data. + */ + public static final String SASL2_RESUMED_SESSION = "Sasl2.resumed-session"; + + /** + * Session Data property name used to store a parsed inline XEP-0198 resume request (see + * {@link ResumeRequest}), between the moment it is parsed from the SASL2 {@code } element + * and the moment SASL authentication succeeds and the request can be acted on. + */ + private static final String SASL2_RESUME_REQUEST = "Sasl2.resume-request"; + /** * Controls whether the SCRAM mechanisms that are advertised to a client are tailored to the user that is expected * to authenticate. @@ -299,7 +320,15 @@ public enum Status * delivered asynchronously (e.g. when Bind2 resource binding completes). The caller must not * send stream features itself. */ - authenticatedAwaitingFeatures + authenticatedAwaitingFeatures, + + /** + * SASL2 negotiation has been successful by inline-resuming a pre-existing session (XEP-0198 § 9.2). The + * {@code } response (including the {@code } element) has already been delivered, over + * the resumed session, by {@link SASLAuthentication}. The caller must adopt the session referenced by + * {@link #SASL2_RESUMED_SESSION} session data, and must NOT send stream features, per XEP-0198 § 9.2. + */ + authenticatedResumed } /** @@ -554,6 +583,7 @@ public static Status handle(LocalSession session, Element doc, boolean usingSASL // Clear any unexecuted bind2-request session.removeSessionData("bind2-request"); session.removeSessionData("user-agent-info"); + session.removeSessionData(SASL2_RESUME_REQUEST); FastSessionState.clearRequest(session); if (usingSASL2 && session instanceof LocalClientSession clientSession) { UserAgentInfo userAgentInfo = null; @@ -565,15 +595,29 @@ public static Status handle(LocalSession session, Element doc, boolean usingSASL session.setSessionData("user-agent-info", userAgentInfo); } } - Bind2Request bind2Request = Bind2Request.from(doc); - if (bind2Request != null) { - session.setSessionData("bind2-request", bind2Request); - } - final FastRequest fastRequest = FastRequest.from(doc, mechanismName, - userAgentInfo == null ? null : userAgentInfo.getId(), clientSession); + + // XEP-0484 § 3.1 & § 3.2: one of several requests related to Fast Authentication Streamlining Tokens. + final FastRequest fastRequest = FastRequest.from(doc, mechanismName, userAgentInfo == null ? null : userAgentInfo.getId(), clientSession); if (fastRequest != null) { fastRequest.applyTo(session); } + + // XEP-0198 § 9.2: an inline stream resumption request. + final ResumeRequest resumeRequest; + try { + resumeRequest = ResumeRequest.fromSasl2Authenticate(doc); + } catch (final MalformedResumeRequestException e) { + throw new SaslFailureException(Failure.MALFORMED_REQUEST, e.getMessage()); + } + if (resumeRequest != null) { + session.setSessionData(SASL2_RESUME_REQUEST, resumeRequest); + } + + // XEP-0386 § 3.2: a resource binding request. + final Bind2Request bind2Request = Bind2Request.from(doc); + if (bind2Request != null) { + session.setSessionData("bind2-request", bind2Request); + } } // intended fall-through @@ -625,6 +669,12 @@ else if ( decoded.length == 0 ) if (MechanismName.requiresChannelBinding(saslServer.getMechanismName())) { session.setSessionData("ChannelBindingType", saslServer.getNegotiatedProperty(ScramSaslServer.PROPNAME_CHANNELBINDINGTYPE)); } + if (usingSASL2 && session.getSessionData(SASL2_RESUMED_SESSION) != null) { + // XEP-0198 § 9.2: the session was resumed inline. The (with ) has + // already been delivered, over the resumed session, by authenticationSuccessful(). The + // caller must adopt that session and must not send stream features. + return Status.authenticatedResumed; + } return hasBind2Request ? Status.authenticatedAwaitingFeatures : Status.authenticated; default: @@ -774,93 +824,153 @@ else if (session instanceof LocalIncomingServerSession serverSession) { authorizationIdentity = username; } - if (usingSASL2) { - if (session instanceof LocalClientSession clientSession) { - // XEP-0484: determine if a FAST token should be issued. - // A token is issued when: - // (a) the client included with a valid mechanism, OR - // (b) this was a FAST authentication and invalidate was NOT requested (token rotation). - // If invalidate=true was requested, delete the existing token and do not rotate. - final boolean fastInvalidate = FastSessionState.isInvalidateRequested(session); - final String fastRequestedMechanism = FastSessionState.getRequestedMechanism(session); - final boolean isFastAuth = MechanismName.isFast(mechanismName); - final String authenticatedClientId = FastSessionState.getAuthenticatedClientId(session); - final String requestingClientId = FastSessionState.getClientId(session); - - FastToken fastToken = null; - if (fastInvalidate) { - // Client requested token invalidation: delete the token used for this auth, do not rotate. - if (username != null) { - if (isFastAuth && authenticatedClientId != null) { - FastTokenManager.invalidateToken(username, mechanismName, authenticatedClientId); - Log.debug("FAST token invalidated for user '{}' per client request.", username); - } - } - // Still issue a new token if the client also sent . - if (fastRequestedMechanism != null && username != null) { - fastToken = issueFastToken(username, requestingClientId, fastRequestedMechanism); - Log.debug("FAST token (re-)issued for user '{}' mechanism '{}' after invalidation+request.", username, fastRequestedMechanism); + if (!usingSASL2) { + Log.debug("Sending SASL success response for user '{}'.", username); + SaslOutcome.sendSuccess(session, successData); + return; + } + + // The remainder of this method is specific to SASL2. + Log.debug("Processing SASL2 request for user '{}'.", username); + if (session instanceof LocalClientSession clientSession) { + // XEP-0484: determine if a FAST token should be issued. + // A token is issued when: + // (a) the client included with a valid mechanism, OR + // (b) this was a FAST authentication and invalidate was NOT requested (token rotation). + // If invalidate=true was requested, delete the existing token and do not rotate. + final boolean fastInvalidate = FastSessionState.isInvalidateRequested(session); + final String fastRequestedMechanism = FastSessionState.getRequestedMechanism(session); + final boolean isFastAuth = MechanismName.isFast(mechanismName); + final String authenticatedClientId = FastSessionState.getAuthenticatedClientId(session); + final String requestingClientId = FastSessionState.getClientId(session); + + FastToken fastToken = null; + if (fastInvalidate) { + // Client requested token invalidation: delete the token used for this auth, do not rotate. + if (username != null) { + if (isFastAuth && authenticatedClientId != null) { + FastTokenManager.invalidateToken(username, mechanismName, authenticatedClientId); + Log.debug("FAST token invalidated for user '{}' per client request.", username); } - } else if (fastRequestedMechanism != null && username != null) { - // Client requested a new FAST token (e.g. during initial password auth). + } + // Still issue a new token if the client also sent . + if (fastRequestedMechanism != null && username != null) { fastToken = issueFastToken(username, requestingClientId, fastRequestedMechanism); - Log.debug("FAST token issued for user '{}' mechanism '{}'.", username, fastRequestedMechanism); - } else if (isFastAuth && username != null) { - // FAST authentication: the SaslServer already rotated the token internally; - // retrieve the new token from the SaslServer's rotatedToken field if accessible, - // or issue a fresh token here for inclusion in the . - // The rotated token is stored by HtSaslServer/Ht2SaslServer via AbstractHtSaslServer. - // We expose it via the "RotatedToken" session data key set by AbstractHtSaslServer. - fastToken = FastSessionState.getRotatedToken(session); + Log.debug("FAST token (re-)issued for user '{}' mechanism '{}' after invalidation+request.", username, fastRequestedMechanism); } - FastSessionState.clearAuthenticationAttempt(session); - - final FastToken finalFastToken = fastToken; - final Bind2Request bind2Request = (Bind2Request) session.getSessionData("bind2-request"); - if (bind2Request != null && clientSession.getStatus() != Session.Status.AUTHENTICATED) { - clientSession.removeSessionData("bind2-request"); - final UserAgentInfo userAgentInfo = (UserAgentInfo) session.getSessionData("user-agent-info"); - final String resource = bind2Request.generateResourceString(userAgentInfo); - final JID preBindAddress = clientSession.getAddress(); - - if (clientAuthToken.isAnonymous()) { - // An anonymous session needs no conflict resolution: its node-part and resource are both the session's - // own generated identifier, so no other session can hold the same full JID. SessionManager#bindResource - // documents this and dereferences the (null) username, so it must not be used here. Note that this - // discards the resource that Bind2 generated; XEP-0386 leaves the assigned resource to the server. - clientSession.setAnonymousAuth(); - final JID bound = clientSession.getAddress(); - completeSasl2Bind2(clientSession, bind2Request, successData, finalFastToken, bound.toBareJID(), bound.getResource(), preBindAddress); - } else { - // A non-anonymous session performs regular resource binding. - final String bareJid = new JID(clientAuthToken.getUsername(), XMPPServer.getInstance().getServerInfo().getXMPPDomain(), null, true).toString(); - SessionManager.getInstance().bindResource(clientSession, clientAuthToken, resource) - .whenComplete((result, throwable) -> { - if (throwable != null) { - Log.warn("An exception occurred while binding resource '{}' for session '{}' during SASL2+Bind2 authentication.", resource, clientSession, throwable); - } - if (throwable != null || result != SessionManager.BindResult.BOUND) { - Log.warn("Unable to bind resource '{}' for session '{}' during SASL2+Bind2 authentication. Bind result: {}", resource, clientSession, result); - abortSasl2(clientSession, Failure.TEMPORARY_AUTH_FAILURE); - return; - } - clientSession.setAuthToken(clientAuthToken); - completeSasl2Bind2(clientSession, bind2Request, successData, finalFastToken, bareJid, resource, preBindAddress); - }); + } else if (fastRequestedMechanism != null && username != null) { + // Client requested a new FAST token (e.g. during initial password auth). + fastToken = issueFastToken(username, requestingClientId, fastRequestedMechanism); + Log.debug("FAST token issued for user '{}' mechanism '{}'.", username, fastRequestedMechanism); + } else if (isFastAuth && username != null) { + // FAST authentication: the SaslServer already rotated the token internally; + // retrieve the new token from the SaslServer's rotatedToken field if accessible, + // or issue a fresh token here for inclusion in the . + // The rotated token is stored by HtSaslServer/Ht2SaslServer via AbstractHtSaslServer. + // We expose it via the "RotatedToken" session data key set by AbstractHtSaslServer. + fastToken = FastSessionState.getRotatedToken(session); + Log.debug("FAST token rotated for user '{}'.", username); + } + FastSessionState.clearAuthenticationAttempt(session); + final FastToken finalFastToken = fastToken; + + // SASL authentication has completed, but resource binding has not (yet). This one-argument form + // records the identity without transitioning the session to AUTHENTICATED, which is exactly the state + // that inline XEP-0198 resumption (below) and StreamManager#allowResume() require. + clientSession.setAuthToken(clientAuthToken); + + // XEP-0198 § 9.2: an inline resume request, if present, must be processed before any Bind2 request. + Element resumeFailedElement = null; + final ResumeRequest resumeRequest = (ResumeRequest) session.removeSessionData(SASL2_RESUME_REQUEST); + if (resumeRequest != null) { + Log.debug("Processing inline resume request for user '{}'.", username); + final Sasl2ResumeResult resumeResult = clientSession.getStreamManager().processSasl2Resume(resumeRequest); + if (resumeResult.isSuccess()) { + final LocalClientSession resumedSession = resumeResult.getResumedSession(); + assert resumedSession != null; // Per contract of Sasl2ResumeResult + final JID resumedAddress = resumedSession.getAddress(); + final Element success = SaslOutcome.buildSasl2SuccessElement(successData, resumedAddress.toBareJID(), resumedAddress.getResource(), finalFastToken); + success.add(resumeResult.getResultElement()); + + // Signal to the caller (typically StanzaHandler) that it must adopt the resumed session, and + // must not deliver a fresh set of post-authentication stream features (XEP-0198 § 9.2). This is + // recorded before the response is delivered: the connection now belongs to the resumed session, + // so the caller must adopt it even when delivering that response fails. + session.setSessionData(SASL2_RESUMED_SESSION, resumedSession); + + // The connection has already been transferred to the resumed session (by processSasl2Resume(), + // through StreamManager and LocalSession#reattachForSasl2()). It must be delivered to, and only + // to, that session; the temporary session is being discarded. + try { + resumedSession.deliverRawText(success.asXML()); + resumedSession.completeSasl2Resume(resumeRequest.getH()); + } catch (final Exception e) { + // The connection is no longer the temporary session's to fail on: a SASL failure cannot be + // reported over it, and the resumed session cannot be left half-resumed. Close it instead. + Log.warn("An exception occurred while completing an inline stream resumption for user '{}'. Closing the resumed session.", username, e); + resumedSession.close(new StreamError(StreamError.Condition.internal_server_error, "An error occurred while resuming the stream.")); } + + // If resumption succeeds, resource binding (and any inlined Bind2 request) is skipped entirely: a + // resumed session already has a resource bound. + Log.debug("Inline resume request for user '{}' processed successfully.", username); + return; + } + // Resumption failed: fall through to the normal Bind2 (or plain) success flow below, embedding + // the element in the response, as required by XEP-0198 § 9.2.1. + Log.debug("Inline resume request for user '{}' failed.", username); + resumeFailedElement = resumeResult.getResultElement(); + } + final Element finalResumeFailedElement = resumeFailedElement; + + // Resumption was not requested or has failed: fall through to the normal Bind2 (or plain) success flow. + final Bind2Request bind2Request = (Bind2Request) session.getSessionData("bind2-request"); + if (bind2Request != null && clientSession.getStatus() != Session.Status.AUTHENTICATED) { + Log.debug("Processing bind2 request for user '{}'.", username); + clientSession.removeSessionData("bind2-request"); + final UserAgentInfo userAgentInfo = (UserAgentInfo) session.getSessionData("user-agent-info"); + final String resource = bind2Request.generateResourceString(userAgentInfo); + final JID preBindAddress = clientSession.getAddress(); + + if (clientAuthToken.isAnonymous()) { + // An anonymous session needs no conflict resolution: its node-part and resource are both the session's + // own generated identifier, so no other session can hold the same full JID. SessionManager#bindResource + // documents this and dereferences the (null) username, so it must not be used here. Note that this + // discards the resource that Bind2 generated; XEP-0386 leaves the assigned resource to the server. + clientSession.setAnonymousAuth(); + final JID bound = clientSession.getAddress(); + completeSasl2Bind2(clientSession, bind2Request, successData, finalFastToken, bound.toBareJID(), bound.getResource(), preBindAddress, finalResumeFailedElement); + Log.debug("Bind2 request for anonymous user '{}' processed successfully.", username); } else { - // No Bind2 request, or session already authenticated: send synchronously without . - clientSession.setAuthToken(clientAuthToken); - final Element success = SaslOutcome.buildSasl2SuccessElement(successData, authorizationIdentity, null, finalFastToken); - session.deliverRawText(success.asXML()); + // A non-anonymous session performs regular resource binding. + final String bareJid = new JID(clientAuthToken.getUsername(), XMPPServer.getInstance().getServerInfo().getXMPPDomain(), null, true).toString(); + SessionManager.getInstance().bindResource(clientSession, clientAuthToken, resource) + .whenComplete((result, throwable) -> { + if (throwable != null) { + Log.warn("An exception occurred while binding resource '{}' for session '{}' during SASL2+Bind2 authentication.", resource, clientSession, throwable); + } + if (throwable != null || result != SessionManager.BindResult.BOUND) { + Log.warn("Unable to bind resource '{}' for session '{}' during SASL2+Bind2 authentication. Bind result: {}", resource, clientSession, result); + abortSasl2(clientSession, Failure.TEMPORARY_AUTH_FAILURE); + return; + } + // bindResource() already installs the auth token (two-argument form, which also transitions the session to AUTHENTICATED); no need to set it again here. + completeSasl2Bind2(clientSession, bind2Request, successData, finalFastToken, bareJid, resource, preBindAddress, finalResumeFailedElement); + Log.debug("Bind2 request for user '{}' processed successfully.", username); + }); } } else { - // Non-client session (e.g. server): send synchronously. - final Element success = SaslOutcome.buildSasl2SuccessElement(successData, authorizationIdentity, null, null); + Log.debug("No bind2 request, or session already authenticated for user '{}'; sending synchronously without .", username); + final Element success = SaslOutcome.buildSasl2SuccessElement(successData, authorizationIdentity, null, finalFastToken); + if (finalResumeFailedElement != null) { + success.add(finalResumeFailedElement); + } session.deliverRawText(success.asXML()); } } else { - SaslOutcome.sendSuccess(session, successData); + Log.debug("Non-client session (e.g. server) for user '{}'; sending synchronously.", username); + final Element success = SaslOutcome.buildSasl2SuccessElement(successData, authorizationIdentity, null, null); + session.deliverRawText(success.asXML()); } } @@ -883,6 +993,8 @@ private static void abortSasl2(@Nonnull final LocalSession session, @Nonnull fin } session.removeSessionData("bind2-request"); session.removeSessionData("user-agent-info"); + session.removeSessionData(SASL2_RESUME_REQUEST); + session.removeSessionData(SASL2_RESUMED_SESSION); session.removeSessionData("SaslServer"); FastSessionState.clearAuthenticationAttempt(session); SaslOutcome.authenticationFailed(session, failure, true); @@ -903,6 +1015,8 @@ private static void abortSasl2(@Nonnull final LocalSession session, @Nonnull fin * @param authorizationIdentity the bare JID authorization identity (e.g. user@domain or uuid@domain for anonymous). * @param resource the bound resource, or null if no resource was bound. * @param preBindAddress The session's address prior to the binding attempt. Must not be null. + * @param resumeFailedElement the {@code } element from a failed inline XEP-0198 resume attempt that + * preceded this bind, or {@code null} if no resume was attempted. */ private static void completeSasl2Bind2(@Nonnull final LocalClientSession clientSession, @Nonnull final Bind2Request bind2Request, @@ -910,12 +1024,17 @@ private static void completeSasl2Bind2(@Nonnull final LocalClientSession clientS final FastToken fastToken, final String authorizationIdentity, final String resource, - @Nonnull final JID preBindAddress) + @Nonnull final JID preBindAddress, + @Nullable final Element resumeFailedElement) { boolean successDelivered = false; try { final Element success = SaslOutcome.buildSasl2SuccessElement(successData, authorizationIdentity, resource, fastToken); + if (resumeFailedElement != null) { + // XEP-0198 § 9.2.1: a failed inline resume is reported alongside (and before) . + success.add(resumeFailedElement); + } bind2Request.processFeatureRequests(clientSession, success); clientSession.deliverRawText(success.asXML()); successDelivered = true; diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java index 4bdc61d8e6..da97b59d79 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java @@ -27,6 +27,7 @@ import org.jivesoftware.openfire.session.ClientSession; import org.jivesoftware.openfire.session.LocalIncomingServerSession; import org.jivesoftware.openfire.session.LocalSession; +import org.jivesoftware.openfire.streammanagement.StreamManager; import org.jivesoftware.util.JiveGlobals; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -173,8 +174,12 @@ static Element asSASLMechanismsElementForClientSessions(@Nonnull final Set fastMechanisms = advertisableMechanismNames.stream() .filter(MechanismName::isFast).collect(Collectors.toSet()); diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java index 0ff8d3caa7..32f03a616a 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -92,6 +92,13 @@ public abstract class StanzaHandler { */ protected boolean usingSASL2 = false; + /** + * Flag that indicates that SASL2 authentication succeeded by inline-resuming a pre-existing session (XEP-0198 + * § 9.2), rather than by binding a (new or Bind2) resource. When set, {@link #sasl2Successful()} must not + * (re)send post-authentication stream features, per XEP-0198 § 9.2. + */ + protected boolean sasl2SessionResumed = false; + /** * SASL status based on the last SASL interaction */ @@ -243,7 +250,11 @@ else if ("auth".equals(tag)) { // User is trying to authenticate using SASL2. startedSASL = true; usingSASL2 = true; - saslStatus = SASLAuthentication.handle(session, doc, usingSASL2); + // An inline XEP-0198 resumption transfers the connection to the resumed session, which (through + // Connection#reinit) replaces this handler's 'session' field before handle() returns. Retain the session + // that is negotiating the authentication, as that is where the outcome of the negotiation is recorded. + final LocalSession authenticatingSession = session; + saslStatus = SASLAuthentication.handle(authenticatingSession, doc, usingSASL2); if (saslStatus == SASLAuthentication.Status.authenticated && usingSASL2) { // No Bind2: send features synchronously now. startedSASL = false; // Without a multi-step SASL mechanism, this can be reset here immediately, rather than in initiateSession (as SASL1 does). @@ -251,12 +262,19 @@ else if ("auth".equals(tag)) { } else if (saslStatus == SASLAuthentication.Status.authenticatedAwaitingFeatures) { // Bind2: and features are delivered asynchronously by SASLAuthentication. startedSASL = false; + } else if (saslStatus == SASLAuthentication.Status.authenticatedResumed) { + // Inline XEP-0198 resume: (with ) was already delivered, over the resumed + // session, by SASLAuthentication. Adopt that session and suppress stream features (XEP-0198 § 9.2). + startedSASL = false; + adoptSasl2ResumedSession(authenticatingSession); } // If authenticatedAwaitingFeatures, and features are delivered asynchronously // by SASLAuthentication once Bind2 resource binding completes. } else if (startedSASL && ("response".equals(tag) || "abort".equals(tag))) { // User is responding to SASL challenge. Process response - saslStatus = SASLAuthentication.handle(session, doc, usingSASL2); + // See the 'authenticate' branch: an inline XEP-0198 resumption can replace this handler's session. + final LocalSession authenticatingSession = session; + saslStatus = SASLAuthentication.handle(authenticatingSession, doc, usingSASL2); if (saslStatus == SASLAuthentication.Status.failed) { startedSASL = false; usingSASL2 = false; @@ -267,6 +285,11 @@ else if ("auth".equals(tag)) { } else if (saslStatus == SASLAuthentication.Status.authenticatedAwaitingFeatures) { // Bind2: and features are delivered asynchronously by SASLAuthentication. startedSASL = false; + } else if (saslStatus == SASLAuthentication.Status.authenticatedResumed) { + // Inline XEP-0198 resume: (with ) was already delivered, over the resumed + // session, by SASLAuthentication. Adopt that session and suppress stream features (XEP-0198 § 9.2). + startedSASL = false; + adoptSasl2ResumedSession(authenticatingSession); } // If authenticatedAwaitingFeatures, and features are delivered asynchronously // by SASLAuthentication once Bind2 resource binding completes. @@ -581,9 +604,15 @@ protected void saslSuccessful() { /** * Emits post-authentication stream features for SASL2 (XEP-0388), which does NOT restart the stream. + * + * When the SASL2 authentication succeeded by inline-resuming a pre-existing session (XEP-0198 § 9.2), features + * are deliberately not (re)sent: the resumed stream is considered re-established immediately after the + * {@code } element, and XEP-0198 § 9.2 mandates that stream features MUST NOT be sent in this case. */ protected void sasl2Successful() { - deliverSasl2Features(); + if (!sasl2SessionResumed) { + deliverSasl2Features(); + } } /** @@ -595,6 +624,37 @@ protected void deliverSasl2Features() { connection.deliverRawText(features.asXML()); } + /** + * Adopts the pre-existing session that a SASL2 authentication resumed inline (XEP-0198 § 9.2), replacing the + * temporary session that was negotiating the SASL2 authentication. + * + * The {@code } response (including the {@code } element) has already been delivered, over + * the resumed session, by {@link SASLAuthentication}, and XEP-0198 § 9.2 forbids sending stream features after + * it. This method therefore only switches this handler over to the resumed session; no features are sent. The + * {@link #sasl2SessionResumed} flag it sets guards {@link #sasl2Successful()} against a future caller that + * would. + * + * Note that transferring the connection re-initializes it for its new owner, which on some transports already + * replaces this handler's session. The switch is performed here regardless, so that this does not depend on the + * transport. For the same reason, the session that negotiated the authentication (which holds the outcome of + * that negotiation) must be provided by the caller, rather than read from {@link #session}. + * + * @param authenticatingSession the session that negotiated the SASL2 authentication (cannot be null). + */ + protected void adoptSasl2ResumedSession(final LocalSession authenticatingSession) { + final Object data = authenticatingSession.removeSessionData(SASLAuthentication.SASL2_RESUMED_SESSION); + if (!(data instanceof LocalSession resumedSession)) { + // Unreachable in practice: SASLAuthentication only reports 'authenticatedResumed' after having stored the + // resumed session under this key. If it does happen, the client has already been told that its stream was + // resumed, over a connection that this handler can no longer serve. There is nothing to do but disconnect. + Log.error("Expected a resumed session to be available in session data under key '{}', but found: {}. Closing the connection.", SASLAuthentication.SASL2_RESUMED_SESSION, data); + connection.close(new StreamError(StreamError.Condition.internal_server_error, "Unable to complete inline stream resumption.")); + return; + } + this.session = resumedSession; + sasl2SessionResumed = true; + } + /** * Helper to generate stream:features, populated simply from the session., * diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResult.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResult.java new file mode 100644 index 0000000000..af77f4867b --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResult.java @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.streammanagement; + +import org.dom4j.Element; +import org.jivesoftware.openfire.session.LocalClientSession; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +/** + * Outcome of {@link StreamManager#processSasl2Resume(ResumeRequest)}: either the {@code } element + * (and the now-resumed session) to embed in a SASL2 {@code } response, or the {@code } element to + * embed instead. + */ +public final class Sasl2ResumeResult +{ + private final boolean success; + private final Element resultElement; + private final LocalClientSession resumedSession; + + /** + * Constructs a result. + * + * @param success whether the resume request was honored. + * @param resultElement the element to embed in the SASL2 {@code } response (cannot be null). + * @param resumedSession the resumed session, or {@code null} on failure. + */ + private Sasl2ResumeResult(final boolean success, @Nonnull final Element resultElement, @Nullable final LocalClientSession resumedSession) + { + this.success = success; + this.resultElement = resultElement; + this.resumedSession = resumedSession; + } + + /** + * Creates a result representing a successfully resumed session. + * + * @param resumedElement the {@code } element to embed in the SASL2 {@code } response (cannot be null). + * @param resumedSession the session that was resumed (cannot be null). + * @return a success result. + */ + static Sasl2ResumeResult success(@Nonnull final Element resumedElement, @Nonnull final LocalClientSession resumedSession) + { + return new Sasl2ResumeResult(true, resumedElement, resumedSession); + } + + /** + * Creates a result representing a resume request that could not be honored. + * + * @param failedElement the {@code } element to embed instead (cannot be null). + * @return a failure result. + */ + static Sasl2ResumeResult failure(@Nonnull final Element failedElement) + { + return new Sasl2ResumeResult(false, failedElement, null); + } + + /** + * Returns whether the resume request was honored. + * + * @return {@code true} if the session was successfully resumed. + */ + public boolean isSuccess() + { + return success; + } + + /** + * Returns the element ({@code } on success, {@code } otherwise) to embed in the SASL2 + * {@code } response. + * + * @return the result element (never null). + */ + @Nonnull + public Element getResultElement() + { + return resultElement; + } + + /** + * Returns the session that was resumed. Only set when {@link #isSuccess()} returns {@code true}. + * + * Guaranteed to be non-null when {@link #isSuccess()} returns {@code true}. + * + * @return the resumed session, or {@code null} on failure. + */ + @Nullable + public LocalClientSession getResumedSession() + { + return resumedSession; + } +} diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java index f832f8d1db..6859ba4a60 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java @@ -25,6 +25,7 @@ import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.openfire.auth.AuthToken; import org.jivesoftware.openfire.auth.UnauthorizedException; +import org.jivesoftware.openfire.net.Bind2Request; import org.jivesoftware.openfire.session.*; import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.SystemProperty; @@ -167,6 +168,8 @@ public boolean getResume() { /** * Processes a stream management element. * + * Inline resume requests, nested in a SASL2 element, are not processed here; see processSasl2Resume(ResumeRequest) + * * @param element The stream management element to be processed. */ public void process( Element element ) @@ -305,10 +308,11 @@ public Element enableAndBuildElement( String namespace, boolean resume ) throws } /** - * Attempts to process (validate and perform) a {@code } request, as defined by XEP-0198. + * Attempts to process (validate and perform) a traditional {@code } request, as defined by XEP-0198. * - * This writes its response (either a stream error, or the effects of - * {@link LocalSession#reattach(LocalSession, long)}) directly to the connection. + * Unlike {@link #processSasl2Resume(ResumeRequest)}, this writes its response (either a stream error, or the + * effects of {@link LocalSession#reattach(LocalSession, long)}) directly to the connection, rather than + * returning a result to the caller. * * @param request the parsed resume request (cannot be null). */ @@ -333,6 +337,49 @@ private void processResume(@Nonnull final ResumeRequest request) Log.debug("Perform resumption of session {}, using connection from session {}", otherSession.getStreamID(), session.getStreamID()); } + /** + * Attempts to process (validate and perform) an inline SASL2 (XEP-0388) resume request, as defined by XEP-0198 + * § 9.2 ("Inline Stream Resumption"). + * + * Unlike {@link #processResume(ResumeRequest)}, this does not write its response to the connection. Instead, the + * outcome is returned as a {@link Sasl2ResumeResult}, for the caller to embed in the SASL2 {@code } + * response that it is constructing. + * + * This method is invoked on the stream manager of the temporary session that is negotiating the SASL2 + * authentication. On success, the connection has been transferred to the resumed session, which is the session + * that the {@code } must be delivered to. Having delivered it, the caller completes the resumption by + * invoking {@link LocalSession#completeSasl2Resume(long)} on that session: everything it delivers must follow the + * resumption confirmation on the wire, which is why it cannot be done here. + * + * On failure, no state is changed: the temporary session remains usable, and the caller is expected to proceed + * with resource binding, reporting the returned {@code } element alongside the outcome of that bind. + * + * @param request the parsed inline resume request (cannot be null). + * @return the outcome of the resume attempt. + */ + @Nonnull + public Sasl2ResumeResult processSasl2Resume(@Nonnull final ResumeRequest request) + { + final ResumeRequestValidationResult validation = validateResumeRequest(request); + if (!validation.isSuccess()) { + assert validation.getFailureCondition() != null; // Per definition of the method contract. + // Note: deliberately no side effects on this (temporary) session's stream management state. Unlike the + // traditional flow, a failed inline resume does not abandon the stream: the client proceeds to Bind2, + // possibly inlining an of its own (XEP-0198 § 9.2.1). + return Sasl2ResumeResult.failure(buildFailedElement(request.getNamespace(), validation.getFailureCondition())); + } + + final LocalClientSession otherSession = validation.getTarget(); + assert otherSession != null; // Per definition of the method contract. + detachIfNeeded(otherSession); + + Log.debug("Attaching to other session '{}' via inline SASL2 resume.", otherSession.getStreamID()); + otherSession.reattachForSasl2(session); + + final Element resumed = otherSession.getStreamManager().buildResumedElement(); + return Sasl2ResumeResult.success(resumed, otherSession); + } + /** * Detaches the connection of a to-be-resumed session, unless it is already detached. * @@ -350,8 +397,10 @@ private void detachIfNeeded(@Nonnull final LocalClientSession otherSession) } /** - * Validates a stream resumption request, without performing any of the state changes - * (detaching/reattaching) that are needed to actually resume the session. + * Validates a stream resumption request (traditional or inline SASL2), without performing any of the state + * changes (detaching/reattaching) needed to actually resume the session. This is shared by {@link #processResume(ResumeRequest)} + * and {@link #processSasl2Resume(ResumeRequest)}, so that the two flows agree on what is (and is not) an + * acceptable resumption attempt. * * @param request the parsed resume request (cannot be null). * @return the outcome of the validation. @@ -838,4 +887,18 @@ public void removeTerminationDelegate(@Nonnull final TerminationDelegate delegat { terminationDelegates.remove(delegate); } + + /** + * Returns the element that advertises support for inline stream resumption in the {@code } element of + * the SASL2 stream feature, as defined in XEP-0198 § 9.2. + * + * Note that this is distinct from the Bind2 inline feature (XEP-0198 § 9.1) that allows a client to enable + * stream management as part of a resource bind: that one is advertised through {@link Bind2Request#featureElement()}. + * + * @return the {@code } feature element. + */ + @Nonnull + public static Element sasl2InlineFeatureElement() { + return DocumentHelper.createElement(QName.get("sm", NAMESPACE_V3)); + } } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java index 5c84c22bb3..6ebe9be439 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java @@ -31,6 +31,7 @@ import org.jivesoftware.openfire.session.LocalIncomingServerSession; import org.jivesoftware.openfire.session.LocalSession; import org.jivesoftware.openfire.spi.BasicStreamIDFactory; +import org.jivesoftware.openfire.streammanagement.StreamManager; import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.junit.jupiter.api.AfterAll; @@ -80,6 +81,7 @@ public static void setupClass() throws Exception @AfterAll public static void tearDownClass() { + StreamManager.ACTIVE.setValue(StreamManager.ACTIVE.getDefaultValue()); Fixtures.clearExistingProperties(); } @@ -559,6 +561,88 @@ public void appendFeaturesDoesNotRecordFastMechanismsWhenSasl2IsNotOffered() { assertEquals(Set.of(), FastSessionState.getAdvertisedMechanisms(session).orElseThrow()); } + // ------------------------------------------------------------------------- + // Stream management / XEP-0198 inline feature + // ------------------------------------------------------------------------- + + /** + * Verifies that the SASL2 inline feature list advertises support for inline stream resumption, as required by + * XEP-0198 § 9.2, when stream management is active. + */ + @Test + public void testSasl2InlineAdvertisesStreamResumption() throws Exception + { + // Setup test fixture. + StreamManager.ACTIVE.setValue(true); + + // Execute system under test. + final Element result = SaslStreamFeatures.asSASLMechanismsElementForClientSessions(Set.of("SCRAM-SHA-1"), true); + + // Verify result. + assertNotNull(result, "Expected a SASL2 'authentication' element to be generated."); + final Element inline = result.element("inline"); + assertNotNull(inline, "Expected the SASL2 'authentication' element to contain an 'inline' element."); + assertNotNull(inline.element(QName.get("sm", StreamManager.NAMESPACE_V3)), "Expected the SASL2 inline feature list to advertise inline stream resumption."); + } + + /** + * Verifies that the SASL2 inline feature list does not advertise inline stream resumption when stream management + * is not active, while continuing to advertise Bind2. + */ + @Test + public void testSasl2InlineDoesNotAdvertiseStreamResumptionWhenInactive() throws Exception + { + // Setup test fixture. + StreamManager.ACTIVE.setValue(false); + + // Execute system under test. + final Element result = SaslStreamFeatures.asSASLMechanismsElementForClientSessions(Set.of("SCRAM-SHA-1"), true); + + // Verify result. + assertNotNull(result, "Expected a SASL2 'authentication' element to be generated."); + final Element inline = result.element("inline"); + assertNotNull(inline, "Expected the SASL2 'authentication' element to contain an 'inline' element."); + assertNull(inline.element(QName.get("sm", StreamManager.NAMESPACE_V3)), "Expected the SASL2 inline feature list to not advertise inline stream resumption while stream management is inactive."); + assertNotNull(inline.element("bind"), "Expected the SASL2 inline feature list to advertise Bind2 regardless of the stream management configuration."); + } + + /** + * Verifies that the advertisement of inline stream resumption (XEP-0198 § 9.2) is a direct child of the SASL2 + * {@code } element, and is not confused with the distinct Bind2 inline feature (XEP-0198 § 9.1) that + * allows stream management to be enabled as part of a resource bind. + */ + @Test + public void testSasl2InlineStreamResumptionIsNotNestedInBind2() throws Exception + { + // Setup test fixture. + StreamManager.ACTIVE.setValue(true); + + // Execute system under test. + final Element result = SaslStreamFeatures.asSASLMechanismsElementForClientSessions(Set.of("SCRAM-SHA-1"), true); + + // Verify result. + final Element bind = result.element("inline").element("bind"); + assertNotNull(bind, "Expected the SASL2 inline feature list to advertise Bind2."); + assertNull(bind.element("sm"), "Expected the Bind2 inline feature list to advertise stream management as a 'feature' element, rather than as an 'sm' element."); + } + + /** + * Verifies that no {@code } element is generated for SASL1 (RFC 6120), for which it is not defined. + */ + @Test + public void testSasl1HasNoInlineElement() throws Exception + { + // Setup test fixture. + StreamManager.ACTIVE.setValue(true); + + // Execute system under test. + final Element result = SaslStreamFeatures.asSASLMechanismsElementForClientSessions(Set.of("SCRAM-SHA-1"), false); + + // Verify result. + assertNotNull(result, "Expected a SASL1 'mechanisms' element to be generated."); + assertNull(result.element("inline"), "Expected no 'inline' element to be generated for SASL1."); + } + // ------------------------------------------------------------------------- // Helpers // ------------------------------------------------------------------------- diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerSasl2ResumeTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerSasl2ResumeTest.java new file mode 100644 index 0000000000..37fa11d4fa --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerSasl2ResumeTest.java @@ -0,0 +1,180 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.net; + +import org.dom4j.Element; +import org.dom4j.Namespace; +import org.jivesoftware.openfire.Connection; +import org.jivesoftware.openfire.PacketRouter; +import org.jivesoftware.openfire.session.LocalClientSession; +import org.jivesoftware.openfire.session.LocalSession; +import org.junit.jupiter.api.Test; +import org.xmlpull.v1.XmlPullParser; +import org.xmpp.packet.StreamError; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +/** + * Verifies how {@link StanzaHandler} adopts the pre-existing session that a SASL2 authentication resumed inline + * (XEP-0198 § 9.2). + */ +public class StanzaHandlerSasl2ResumeTest +{ + /** + * Verifies that the session referenced by the session data that {@link SASLAuthentication} provides is adopted, + * replacing the temporary session that negotiated the SASL2 authentication. + */ + @Test + public void testAdoptsResumedSession() throws Exception + { + // Setup test fixture. + final LocalClientSession resumedSession = mock(LocalClientSession.class); + final LocalSession temporarySession = mock(LocalSession.class); + when(temporarySession.removeSessionData(SASLAuthentication.SASL2_RESUMED_SESSION)).thenReturn(resumedSession); + final Connection connection = mock(Connection.class); + final TestStanzaHandler handler = new TestStanzaHandler(mock(PacketRouter.class), connection); + handler.setSession(temporarySession); + + // Execute system under test. + handler.adoptSasl2ResumedSession(temporarySession); + + // Verify result. + assertEquals(resumedSession, handler.session, "Expected the handler to have adopted the resumed session."); + assertTrue(handler.sasl2SessionResumed, "Expected the handler to have recorded that the session was resumed."); + verify(temporarySession).removeSessionData(SASLAuthentication.SASL2_RESUMED_SESSION); + verify(connection, never()).close(any(StreamError.class)); + } + + /** + * Verifies that no stream features are delivered after a session was resumed inline, as XEP-0198 § 9.2 requires. + */ + @Test + public void testDoesNotDeliverFeaturesAfterResume() throws Exception + { + // Setup test fixture. + final LocalSession temporarySession = mock(LocalSession.class); + when(temporarySession.removeSessionData(SASLAuthentication.SASL2_RESUMED_SESSION)).thenReturn(mock(LocalClientSession.class)); + final TestStanzaHandler handler = new TestStanzaHandler(mock(PacketRouter.class), mock(Connection.class)); + handler.setSession(temporarySession); + handler.adoptSasl2ResumedSession(temporarySession); + + // Execute system under test. + handler.sasl2Successful(); + + // Verify result. + assertFalse(handler.deliveredFeatures, "Expected no post-authentication stream features to be delivered after a session was resumed inline."); + } + + /** + * Verifies that the connection is closed, rather than served by a session that can no longer be used, when the + * expected resumed session is absent from the session data. + */ + @Test + public void testClosesConnectionWhenResumedSessionIsAbsent() throws Exception + { + // Setup test fixture. + final LocalSession temporarySession = mock(LocalSession.class); + when(temporarySession.removeSessionData(SASLAuthentication.SASL2_RESUMED_SESSION)).thenReturn("not-a-session"); + final Connection connection = mock(Connection.class); + final TestStanzaHandler handler = new TestStanzaHandler(mock(PacketRouter.class), connection); + handler.setSession(temporarySession); + + // Execute system under test. + handler.adoptSasl2ResumedSession(temporarySession); + + // Verify result. + verify(connection).close(any(StreamError.class)); + assertEquals(temporarySession, handler.session, "Expected the handler to not have adopted a value that is not a session."); + } + + /** + * Verifies that the resumed session is adopted even when the connection transfer that preceded this already + * replaced this handler's session, which is what {@link org.jivesoftware.openfire.Connection#reinit} does. + */ + @Test + public void testAdoptsResumedSessionAfterConnectionReinit() throws Exception + { + // Setup test fixture. + final LocalClientSession resumedSession = mock(LocalClientSession.class); + final LocalSession temporarySession = mock(LocalSession.class); + when(temporarySession.removeSessionData(SASLAuthentication.SASL2_RESUMED_SESSION)).thenReturn(resumedSession); + final Connection connection = mock(Connection.class); + final TestStanzaHandler handler = new TestStanzaHandler(mock(PacketRouter.class), connection); + handler.setSession(resumedSession); // As Connection#reinit will have done. + + // Execute system under test. + handler.adoptSasl2ResumedSession(temporarySession); + + // Verify result. + assertEquals(resumedSession, handler.session, "Expected the handler to have adopted the resumed session."); + verify(connection, never()).close(any(StreamError.class)); + } + + /** + * A minimal concrete {@link StanzaHandler}, which records whether stream features were delivered. + */ + private static class TestStanzaHandler extends StanzaHandler + { + private boolean deliveredFeatures = false; + + TestStanzaHandler(final PacketRouter router, final Connection connection) + { + super(router, connection); + } + + @Override + protected void deliverSasl2Features() + { + deliveredFeatures = true; + } + + @Override + boolean processUnknowPacket(final Element doc) + { + return false; + } + + @Override + void startTLS() + { + } + + @Override + Namespace getNamespace() + { + return Namespace.get("jabber:client"); + } + + @Override + boolean validateHost() + { + return false; + } + + @Override + boolean validateJIDs() + { + return false; + } + + @Override + void createSession(final String serverName, final XmlPullParser xpp, final Connection connection) + { + } + } +} diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResultTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResultTest.java new file mode 100644 index 0000000000..06c3f08d6e --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/Sasl2ResumeResultTest.java @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.streammanagement; + +import org.dom4j.DocumentHelper; +import org.dom4j.Element; +import org.jivesoftware.openfire.session.LocalClientSession; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; + +/** + * Verifies {@link Sasl2ResumeResult}. + */ +public class Sasl2ResumeResultTest +{ + /** + * Verifies that a successful result reports success, and exposes the resumed session and result element. + */ + @Test + public void testSuccess() throws Exception + { + // Setup test fixture. + final Element resumedElement = DocumentHelper.createElement("resumed"); + final LocalClientSession resumedSession = mock(LocalClientSession.class); + + // Execute system under test. + final Sasl2ResumeResult result = Sasl2ResumeResult.success(resumedElement, resumedSession); + + // Verify result. + assertTrue(result.isSuccess(), "Expected a success result to report success."); + assertEquals(resumedElement, result.getResultElement(), "Expected a success result to expose the provided result element."); + assertEquals(resumedSession, result.getResumedSession(), "Expected a success result to expose the provided resumed session."); + } + + /** + * Verifies that a failure result reports no success, exposes the failure element, and has no resumed session. + */ + @Test + public void testFailure() throws Exception + { + // Setup test fixture. + final Element failedElement = DocumentHelper.createElement("failed"); + + // Execute system under test. + final Sasl2ResumeResult result = Sasl2ResumeResult.failure(failedElement); + + // Verify result. + assertFalse(result.isSuccess(), "Expected a failure result to not report success."); + assertEquals(failedElement, result.getResultElement(), "Expected a failure result to expose the provided result element."); + assertNull(result.getResumedSession(), "Expected a failure result to expose no resumed session."); + } +} From e20497e7d3087697d318f4eac8a9867b7a697aed Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Sat, 5 Sep 2026 12:03:12 +0200 Subject: [PATCH 5/6] OF-2534: fix: inline resumption handoff could disable resume and tear down a live session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing the connection being superseded during a resumption hand-off (traditional or inline SASL2) previously disabled stream resumption (OF-2751) and, via an async close-listener race, could send unavailable presence to every joined MUC and remove the routing entry for a session that had just been successfully resumed. NettyConnection#close(StreamError) now checks whether the session is already detached before disabling resumption: detachIfNeeded() always detaches before closing, so a detached session at close() time means this is a benign hand-off, not a genuine stream failure. The connectionis still closed with a 'conflict' StreamError, per XEP-0198 §5. The session-close listener also now refuses to tear down a session that already has a different, live connection, as a backstop independent of the isDetached()/resume flags. Also adds debug logging around formalClose(), reattachment, and the close-listener's decision state, to make this flow assertable from logs going forward. --- .../jivesoftware/openfire/SessionManager.java | 13 +++ .../openfire/net/SASLAuthentication.java | 3 + .../openfire/nio/NettyConnection.java | 11 ++- .../openfire/session/LocalSession.java | 1 + .../streammanagement/StreamManager.java | 5 +- .../streammanagement/StreamManagerTest.java | 86 ++++++++++++++++++- 6 files changed, 114 insertions(+), 5 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/SessionManager.java b/xmppserver/src/main/java/org/jivesoftware/openfire/SessionManager.java index 91349d8ed1..97e35de850 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/SessionManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/SessionManager.java @@ -1653,6 +1653,19 @@ private class ClientSessionListener implements ConnectionCloseListener public CompletableFuture onConnectionClosing(Object handback) { final LocalClientSession session = (LocalClientSession) handback; + Log.trace("onConnectionClosing invoked for session with address {} and streamID {}: isDetached={}, resume={}, currentConnection={}.", session.getAddress(), session.getStreamID(), session.isDetached(), session.getStreamManager().getResume(), session.getConnection()); + + // A close notification can arrive for a connection that has since been superseded by a resumed session + // (XEP-0198, traditional or inline SASL2): the old connection is closed deliberately as part of the + // handoff, but this listener callback is asynchronous and may run after the session has already been + // reattached to a different, live connection. Treat that as a no-op rather than tearing down a session + // that is connected right now. + final Connection currentConnection = session.getConnection(); + if (currentConnection != null && !currentConnection.isClosed()) { + Log.debug("Ignoring stale close notification for session with address {} and streamID {}: it already has a different, live connection.", session.getAddress(), session.getStreamID()); + return CompletableFuture.completedFuture(null); + } + if (session.isDetached()) { Log.debug("Closing client session with address {} and streamID {} is detached already; this is a no-op.", session.getAddress(), session.getStreamID()); return CompletableFuture.completedFuture(null); diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java index 0aec5d24ef..5de8850a03 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java @@ -914,6 +914,9 @@ else if (session instanceof LocalIncomingServerSession serverSession) { // If resumption succeeds, resource binding (and any inlined Bind2 request) is skipped entirely: a // resumed session already has a resource bound. Log.debug("Inline resume request for user '{}' processed successfully.", username); + if (session.getSessionData("bind2-request") != null) { + Log.debug("Inline resume for user '{}' succeeded; ignoring the inlined bind2 request per XEP-0198 §9.2.", username); + } return; } // Resumption failed: fall through to the normal Bind2 (or plain) success flow below, embedding diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java index e8fd092792..370ca6ff09 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java @@ -226,8 +226,15 @@ public void close(@Nullable final StreamError error) { ChannelFuture f; if (session != null) { - // If the stream was ended because of an error, it should not be possible to resume it (OF-2751). - if (error != null) { + // If the stream ended because of an error, it should not be possible to resume it (OF-2751). + // Exception: if the session is already detached by the time this runs, the error isn't reporting a + // genuine failure of this stream. It's StreamManager#detachIfNeeded() closing a connection that has + // already been handed off to a newly resumed session, choosing to still send a 'conflict' StreamError + // only because XEP-0198 §5 recommends that for a superseded former stream that is still open. + // detachIfNeeded() always detaches the session before closing its connection, so a detached session + // at this point reliably signals that hand-off, not a failure - and that hand-off must not disable + // resumption of the very session it's transferring the connection to. + if (error != null && !session.isDetached()) { session.getStreamManager().formalClose(); } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java index 324fb7c1e0..96b796e41c 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalSession.java @@ -248,6 +248,7 @@ private void reattachConnection(LocalSession connectionProvider) { } this.status = Session.Status.AUTHENTICATED; this.sessionManager.removeDetached(this); + Log.debug("Reattach complete for session with address {} and streamID {}: status={}, resumable={}, detached={}.", this.address, this.streamID, this.status, this.streamManager.getResume(), this.isDetached()); } /** diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java index 6859ba4a60..ee2a9377c3 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/streammanagement/StreamManager.java @@ -385,13 +385,15 @@ public Sasl2ResumeResult processSasl2Resume(@Nonnull final ResumeRequest request * * @param otherSession the pre-existing session that is about to be resumed. */ - private void detachIfNeeded(@Nonnull final LocalClientSession otherSession) + @VisibleForTesting + void detachIfNeeded(@Nonnull final LocalClientSession otherSession) { if (!otherSession.isDetached()) { Log.debug("Existing session {} is not detached; detaching.", otherSession.getStreamID()); final Connection oldConnection = otherSession.getConnection(); otherSession.setDetached(); assert oldConnection != null; // If the other session is not detached, the connection can't be null. + Log.debug("Closing superseded connection {} for session {}", oldConnection, otherSession.getStreamID()); oldConnection.close(new StreamError(StreamError.Condition.conflict, "The stream previously served over this connection is resumed on a new connection.")); } } @@ -510,6 +512,7 @@ private ResumeRequestValidationResult validateResumeRequest(@Nonnull final Resum * session from being detached. */ public void formalClose() { + Log.debug("formalClose() invoked for session with address {} and streamID {}; resumption is now permanently disabled for this stream.", session.getAddress(), session.getStreamID()); this.resume = false; } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/StreamManagerTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/StreamManagerTest.java index 73a0e5fc6c..347cb94fa9 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/StreamManagerTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/streammanagement/StreamManagerTest.java @@ -15,12 +15,18 @@ */ package org.jivesoftware.openfire.streammanagement; +import org.jivesoftware.openfire.Connection; +import org.jivesoftware.openfire.session.LocalClientSession; +import org.jivesoftware.openfire.session.LocalSession; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; +import org.xmpp.packet.StreamError; import java.math.BigInteger; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; /** * Unit tests that verify the implementation of {@link StreamManager}. @@ -326,4 +332,80 @@ public void testValidateClientAcknowledgement_rollover_edgecase5_unsent() throws // Verify results. assertFalse(result); } + + /** + * Verifies that {@link StreamManager#detachIfNeeded(LocalClientSession)} closes the pre-existing session's + * connection with a 'conflict' {@link StreamError}, as recommended by XEP-0198 §5 for a former stream that is still + * open when it is superseded by a resumed session. + */ + @Test + void closesOldConnectionWithAConflictStreamError() + { + // Setup test fixture. + final LocalClientSession otherSession = mock(LocalClientSession.class); + final Connection oldConnection = mock(Connection.class); + when(otherSession.isDetached()).thenReturn(false); + when(otherSession.getConnection()).thenReturn(oldConnection); + final LocalSession tempSession = mock(LocalSession.class); // owns this StreamManager + final StreamManager sm = new StreamManager(tempSession); + + // Execute system under test. + sm.detachIfNeeded(otherSession); + + // Verify results. + verify(otherSession, times(1).description("The pre-existing session must be marked detached before its connection is handed off to the resumed session.")).setDetached(); + + final ArgumentCaptor errorCaptor = ArgumentCaptor.forClass(StreamError.class); + verify(oldConnection, times(1).description("The pre-existing connection must be closed exactly once as part of the hand-off.")).close(errorCaptor.capture()); + final StreamError closeError = errorCaptor.getValue(); + assertNotNull(closeError, "The pre-existing connection must be closed with a StreamError, as recommended by XEP-0198 §5 for a former stream that is still open when superseded by a resumed session."); + assertEquals(StreamError.Condition.conflict, closeError.getCondition(), "The StreamError used to close a superseded connection must use the 'conflict' condition, per the example in XEP-0198 §5."); + } + + /** + * Verifies that {@link StreamManager#detachIfNeeded(LocalClientSession)} is a no-op for a session + * that is already detached, leaving its (already absent) connection untouched. + */ + @Test + void isANoOpWhenAlreadyDetached() + { + // Setup test fixture. + final LocalClientSession otherSession = mock(LocalClientSession.class); + when(otherSession.isDetached()).thenReturn(true); + final StreamManager sm = new StreamManager(mock(LocalSession.class)); + + // Execute system under test. + sm.detachIfNeeded(otherSession); + + // Verify results. + verify(otherSession, never().description("An already-detached session has no connection to obtain; detachIfNeeded() must not attempt to read it.")).getConnection(); + verify(otherSession, never().description("An already-detached session must not be detached again.")).setDetached(); + } + + /** + * Verifies that {@link StreamManager#detachIfNeeded(LocalClientSession)} marks the pre-existing session as detached + * strictly before closing its connection. This ordering is not incidental: it is what lets + * NettyConnection#close(StreamError) tell a benign resumption hand-off (session already detached when close() runs) + * apart from a genuine stream failure (session not yet detached), and so decide whether to disable resumption + * (OF-2751). If this ordering were ever reversed, that distinction (and with it, the safety of resuming a session + * across a hand-off) would silently break. + */ + @Test + void detachesSessionBeforeClosingItsConnection() + { + // Setup test fixture. + final LocalClientSession otherSession = mock(LocalClientSession.class); + final Connection oldConnection = mock(Connection.class); + when(otherSession.isDetached()).thenReturn(false); + when(otherSession.getConnection()).thenReturn(oldConnection); + final StreamManager sm = new StreamManager(mock(LocalSession.class)); + + // Execute system under test. + sm.detachIfNeeded(otherSession); + + // Verify results. + final InOrder inOrder = inOrder(otherSession, oldConnection); + inOrder.verify(otherSession).setDetached(); + inOrder.verify(oldConnection).close(any(StreamError.class)); + } } From 568b443ea73d442325fa7c1a0b05dd30aeb4dfa2 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Sat, 5 Sep 2026 13:03:17 +0200 Subject: [PATCH 6/6] OF-2534: Add integration tests for inline SASL2 stream resumption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inline-resume branch of SASLAuthentication#authenticationSuccessful() had no coverage that actually drove it against a real target session. Adds four tests, each constructing a real, authenticated, SM-resumable LocalClientSession and resuming it via a genuine ResumeRequest: - successful resume delivers + on the new connection, not the superseded one - successful resume suppresses an inlined Bind2 request entirely, per XEP-0198 § 9.2 - is delivered before any retransmission of stanzas left unacknowledged on the former stream - a failed resume (streamID mismatch) falls through to Bind2, reporting alongside a completed , per XEP-0198 § 9.2.1 --- .../openfire/net/SASLAuthenticationTest.java | 174 +++++++++++++++++- 1 file changed, 170 insertions(+), 4 deletions(-) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/SASLAuthenticationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/SASLAuthenticationTest.java index a3fc82dc06..5998a11577 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/SASLAuthenticationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/SASLAuthenticationTest.java @@ -15,6 +15,7 @@ */ package org.jivesoftware.openfire.net; +import org.jivesoftware.openfire.*; import org.jivesoftware.openfire.fast.FastSessionState; import org.jivesoftware.openfire.fast.FastTokenManager; import org.jivesoftware.openfire.fast.FastToken; @@ -24,8 +25,6 @@ import org.dom4j.Namespace; import org.dom4j.QName; import org.jivesoftware.Fixtures; -import org.jivesoftware.openfire.Connection; -import org.jivesoftware.openfire.SessionManager; import org.jivesoftware.openfire.entitycaps.EntityCapabilitiesManager; import org.jivesoftware.openfire.sasl.Failure; import org.jivesoftware.openfire.sasl.SaslFailureException; @@ -33,23 +32,27 @@ import org.jivesoftware.openfire.sasl.TestSaslMechanism; import org.jivesoftware.openfire.session.Session; import org.jivesoftware.openfire.spi.ConnectionConfiguration; -import org.jivesoftware.openfire.StreamID; -import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.openfire.auth.AuthToken; import org.jivesoftware.openfire.session.LocalClientSession; import org.jivesoftware.openfire.session.LocalIncomingServerSession; import org.jivesoftware.openfire.session.ServerSession; import org.jivesoftware.openfire.spi.BasicStreamIDFactory; +import org.jivesoftware.openfire.streammanagement.ResumeRequest; +import org.jivesoftware.openfire.streammanagement.StreamManager; import org.jivesoftware.util.JiveGlobals; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.MockedStatic; +import org.xmpp.packet.IQ; import org.xmpp.packet.JID; +import org.xmpp.packet.Packet; import javax.security.sasl.SaslServer; +import java.nio.charset.StandardCharsets; import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; @@ -1774,6 +1777,134 @@ public void sasl2FailsWhenAcceptedFastTokenRequestCannotBePersisted() } } + /** + * Verifies that a successful inline XEP-0198 resume (XEP-0198 § 9.2) delivers {@code }, embedding + * {@code }, on the connection of the session that is negotiating the authentication - not on the + * pre-existing session's old connection, which by this point has already been superseded. + */ + @Test + public void inlineResumeSuccessDeliversSuccessOnNewConnection() throws Exception + { + // Setup test fixture. + final Connection oldConnection = mock(Connection.class); + final LocalClientSession otherSession = new LocalClientSession(Fixtures.XMPP_DOMAIN, oldConnection, new BasicStreamIDFactory().createStreamID(), Locale.ENGLISH); + final String previd = makeResumableSession(otherSession, "testuser", "test-resource"); + + final Connection newConnection = mock(Connection.class); + final LocalClientSession session = new LocalClientSession(Fixtures.XMPP_DOMAIN, newConnection, new BasicStreamIDFactory().createStreamID(), Locale.ENGLISH); + session.setSessionData("Sasl2.resume-request", resumeRequest(previd, 0)); + + // Execute system under test. + SASLAuthentication.authenticationSuccessful(session, "testuser", "PLAIN", new byte[0], true); + + // Verify result. + // Note: completeSasl2Resume() also sends a trailing ack-request after retransmission, even with nothing + // to retransmit, so more than one deliverRawText call on newConnection is expected; only the first matters here. + final ArgumentCaptor delivered = ArgumentCaptor.forClass(String.class); + verify(newConnection, atLeastOnce()).deliverRawText(delivered.capture()); + final Element success = DocumentHelper.parseText(delivered.getAllValues().get(0)).getRootElement(); + assertEquals("success", success.getName(), "Expected a successful resume to deliver ."); + assertNotNull(success.element(QName.get("resumed", StreamManager.NAMESPACE_V3)), "Expected to embed for an inline resume, per XEP-0198 § 9.2."); + verify(oldConnection, never()).deliverRawText(any()); + } + + /** + * Verifies that when a client hedges by inlining both {@code } and a Bind2 {@code } request in + * the same {@code } element, a successful resume causes the Bind2 request to be entirely ignored, + * as XEP-0198 § 9.2 requires: "the server MUST skip resource binding ... and MUST entirely ignore the + * {@code } request". + */ + @Test + public void inlineResumeSuccessSuppressesInlinedBind2() throws Exception + { + // Setup test fixture. + final LocalClientSession otherSession = new LocalClientSession(Fixtures.XMPP_DOMAIN, mock(Connection.class), new BasicStreamIDFactory().createStreamID(), Locale.ENGLISH); + final String previd = makeResumableSession(otherSession, "testuser", "test-resource"); + + final LocalClientSession session = new LocalClientSession(Fixtures.XMPP_DOMAIN, mock(Connection.class), new BasicStreamIDFactory().createStreamID(), Locale.ENGLISH); + session.setSessionData("Sasl2.resume-request", resumeRequest(previd, 0)); + final Bind2Request bind2Request = mock(Bind2Request.class); + session.setSessionData("bind2-request", bind2Request); // client hedged: sent and together + + // Execute system under test. + SASLAuthentication.authenticationSuccessful(session, "testuser", "PLAIN", new byte[0], true); + + // Verify result. + verify(bind2Request, never()).generateResourceString(any()); + verify(bind2Request, never()).processFeatureRequests(any(), any()); + verify(XMPPServer.getInstance().getSessionManager(), never()).bindResource(any(), any(), any()); + } + + /** + * Verifies that the resumption confirmation reaches the wire before any retransmission of stanzas that went + * unacknowledged on the former stream. Reordering these would mean a client sees retransmitted stanzas before + * {@code }, out of the sequence XEP-0198/XEP-0388 assume, and (per the analogous CSI-queue fix in + * OF-2534) risks the client processing them a second time after the confirmation. + */ + @Test + public void inlineResumeDeliversSuccessBeforeRetransmittingUnackedStanzas() throws Exception + { + // Setup test fixture. + final LocalClientSession otherSession = new LocalClientSession(Fixtures.XMPP_DOMAIN, mock(Connection.class), new BasicStreamIDFactory().createStreamID(), Locale.ENGLISH); + final String previd = makeResumableSession(otherSession, "testuser", "test-resource"); + otherSession.getStreamManager().sentStanza(new IQ()); // x=1 + otherSession.getStreamManager().sentStanza(new IQ()); // x=2, left unacknowledged below + + final Connection newConnection = mock(Connection.class); + final LocalClientSession session = new LocalClientSession(Fixtures.XMPP_DOMAIN, newConnection, new BasicStreamIDFactory().createStreamID(), Locale.ENGLISH); + session.setSessionData("Sasl2.resume-request", resumeRequest(previd, 1)); // acks only x=1 + + // Execute system under test. + SASLAuthentication.authenticationSuccessful(session, "testuser", "PLAIN", new byte[0], true); + + // Verify result. + final InOrder inOrder = inOrder(newConnection); + inOrder.verify(newConnection).deliverRawText(contains(" element directly, per its real contract. + when(bind2Request.processFeatureRequests(any(), any())).thenAnswer(invocation -> { + final Element successElement = invocation.getArgument(1); + return successElement.addElement(QName.get("bound", "urn:xmpp:bind:0")); + }); + session.setSessionData("bind2-request", bind2Request); + stubSuccessfulBind(XMPPServer.getInstance().getSessionManager()); + + // Execute system under test. + SASLAuthentication.authenticationSuccessful(session, "testuser", "PLAIN", new byte[0], true); + + // Verify result. + verify(bind2Request).processFeatureRequests(any(), any()); + // Note: a completed Bind2 always delivers followed by post-bind stream features as a second, + // separate call (see bind2FailureAfterSuccessClosesTheStream); only the first delivery matters here. + final ArgumentCaptor delivered = ArgumentCaptor.forClass(String.class); + verify(connection, atLeastOnce()).deliverRawText(delivered.capture()); + final Element success = DocumentHelper.parseText(delivered.getAllValues().get(0)).getRootElement(); + assertEquals("success", success.getName(), "A failed resume must not abandon the stream; Bind2 must still complete it."); + assertNotNull(success.element(QName.get("failed", StreamManager.NAMESPACE_V3)), "Expected the failed resume's to be reported alongside , per XEP-0198 § 9.2.1."); + assertNotNull(success.element(QName.get("bound", "urn:xmpp:bind:0")), "Expected the fallback bind to still complete."); + } + /** * Stubs {@link SessionManager#bindResource(LocalClientSession, AuthToken, String)} to emulate a successful bind, * rather than merely returning {@code BOUND}. @@ -1802,4 +1933,39 @@ private static void stubSuccessfulBind(final SessionManager sessionManager) return CompletableFuture.completedFuture(SessionManager.BindResult.BOUND); }); } + + /** + * Builds an authenticated, SM-resumable session and registers it in the (mocked) routing table, so that + * {@link org.jivesoftware.openfire.streammanagement.StreamManager#processSasl2Resume} can locate it as an + * existing session to resume. + * + * @return the previd (SM-ID) a client would use to resume the returned session. + */ + private static String makeResumableSession(final LocalClientSession otherSession, final String username, final String resource) throws Exception + { + final AuthToken authToken = AuthToken.generateUserToken(username); + otherSession.setAddress(new JID(authToken.getUsername(), otherSession.getServerName(), resource, true)); + otherSession.setAuthToken(authToken); // one-arg overload: avoids PrivacyListManager/sessionManager.addSession side effects + otherSession.setStatus(Session.Status.AUTHENTICATED); + + final Element enabled = otherSession.getStreamManager().enableAndBuildElement(StreamManager.NAMESPACE_V3, true); + final String previd = enabled.attributeValue("id"); + + final RoutingTable routingTable = Fixtures.mockRoutingTable(); + when(routingTable.getClientRoute(otherSession.getAddress())).thenReturn(otherSession); + when(XMPPServer.getInstance().getRoutingTable()).thenReturn(routingTable); + return previd; + } + + /** + * Builds a parsed {@link ResumeRequest}, as if a client had sent {@code } inline + * in its SASL2 {@code } element. + */ + private static ResumeRequest resumeRequest(final String previd, final long h) throws Exception + { + final Element resumeElement = DocumentHelper.createElement(QName.get("resume", StreamManager.NAMESPACE_V3)); + resumeElement.addAttribute("h", Long.toString(h)); + resumeElement.addAttribute("previd", previd); + return ResumeRequest.from(resumeElement); + } }