From 2e3472d88fd660ddd07596a4781984417eb071ec Mon Sep 17 00:00:00 2001 From: "Alexander \"PapaTutuWawa" Date: Sat, 19 Nov 2022 21:48:28 +0100 Subject: [PATCH] feat: Rework how the negotiator system works We can now return what exactly made a connection attempt fail. --- packages/moxxmpp/lib/src/connection.dart | 151 +++++++----------- packages/moxxmpp/lib/src/errors.dart | 20 +++ .../lib/src/negotiators/negotiator.dart | 10 +- .../lib/src/negotiators/resource_binding.dart | 11 +- .../lib/src/negotiators/sasl/errors.dart | 3 + .../lib/src/negotiators/sasl/plain.dart | 12 +- .../lib/src/negotiators/sasl/scram.dart | 25 ++- .../moxxmpp/lib/src/negotiators/starttls.dart | 16 +- packages/moxxmpp/lib/src/roster.dart | 5 +- .../lib/src/xeps/xep_0198/negotiator.dart | 18 +-- packages/moxxmpp/lib/src/xeps/xep_0352.dart | 5 +- packages/moxxmpp/test/negotiator_test.dart | 20 +-- packages/moxxmpp/test/sasl/scram_test.dart | 28 ++-- 13 files changed, 154 insertions(+), 170 deletions(-) create mode 100644 packages/moxxmpp/lib/src/errors.dart create mode 100644 packages/moxxmpp/lib/src/negotiators/sasl/errors.dart diff --git a/packages/moxxmpp/lib/src/connection.dart b/packages/moxxmpp/lib/src/connection.dart index 1c97b28..464ca58 100644 --- a/packages/moxxmpp/lib/src/connection.dart +++ b/packages/moxxmpp/lib/src/connection.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'package:logging/logging.dart'; import 'package:meta/meta.dart'; import 'package:moxxmpp/src/buffer.dart'; +import 'package:moxxmpp/src/errors.dart'; import 'package:moxxmpp/src/events.dart'; import 'package:moxxmpp/src/iq.dart'; import 'package:moxxmpp/src/managers/attributes.dart'; @@ -61,14 +62,14 @@ class XmppConnectionResult { const XmppConnectionResult( this.success, { - this.reason, + this.error, } ); final bool success; - // NOTE: [reason] is not human-readable, but the type of SASL error. - // See sasl/errors.dart - final String? reason; + // If a connection attempt fails, i.e. success is false, then this indicates the + // reason the connection failed. + final XmppError? error; } class XmppConnection { @@ -345,12 +346,8 @@ class XmppConnection { } /// Called when a stream ending error has occurred - Future handleError(Object? error) async { - if (error != null) { - _log.severe('handleError: $error'); - } else { - _log.severe('handleError: Called with null'); - } + Future handleError(XmppError error) async { + _log.severe('handleError called with ${error.toString()}'); // Whenever we encounter an error that would trigger a reconnection attempt while // the connection result is being awaited, don't attempt a reconnection but instead @@ -358,7 +355,12 @@ class XmppConnection { if (_connectionCompleter != null) { _log.info('Not triggering reconnection since connection result is being awaited'); await _disconnect(triggeredByUser: false, state: XmppConnectionState.error); - _connectionCompleter?.complete(const XmppConnectionResult(false)); + _connectionCompleter?.complete( + XmppConnectionResult( + false, + error: error, + ), + ); _connectionCompleter = null; return; } @@ -370,7 +372,7 @@ class XmppConnection { /// Called whenever the socket creates an event Future _handleSocketEvent(XmppSocketEvent event) async { if (event is XmppSocketErrorEvent) { - await handleError(event.error); + await handleError(SocketError(event)); } else if (event is XmppSocketClosureEvent) { if (_socketClosureTriggersReconnect) { _log.fine('Received XmppSocketClosureEvent. Reconnecting...'); @@ -525,7 +527,7 @@ class XmppConnection { /// Called when we timeout during connecting Future _onConnectingTimeout() async { _log.severe('Connection stuck in "connecting". Causing a reconnection...'); - await handleError('Connecting timeout'); + await handleError(TimeoutError()); } void _destroyConnectingTimer() { @@ -749,20 +751,34 @@ class XmppConnection { // Send out initial presence await getPresenceManager().sendInitialPresence(); } - - /// To be called after _currentNegotiator!.negotiate(..) has been called. Checks the - /// state of the negotiator and picks the next negotiatior, ends negotiation or - /// waits, depending on what the negotiator did. - Future _checkCurrentNegotiator() async { - if (_currentNegotiator!.state == NegotiatorState.done) { - _log.finest('Negotiator ${_currentNegotiator!.id} done'); + Future _executeCurrentNegotiator(XMLNode nonza) async { + // If we don't have a negotiator get one + _currentNegotiator ??= getNextNegotiator(_streamFeatures); + if (_currentNegotiator == null && _isMandatoryNegotiationDone(_streamFeatures) && !_isNegotiationPossible(_streamFeatures)) { + _log.finest('Negotiations done!'); + _updateRoutingState(RoutingState.handleStanzas); + await _onNegotiationsDone(); + return; + } + + final result = await _currentNegotiator!.negotiate(nonza); + if (result.isType()) { + _log.severe('Negotiator returned an error'); + await handleError(result.get()); + return; + } + + final state = result.get(); + _currentNegotiator!.state = state; + switch (state) { + case NegotiatorState.ready: return; + case NegotiatorState.done: if (_currentNegotiator!.sendStreamHeaderWhenDone) { _currentNegotiator = null; _streamFeatures.clear(); _sendStreamHeader(); } else { - // Track what features we still have _streamFeatures .removeWhere((node) { return node.attributes['xmlns'] == _currentNegotiator!.negotiatingXmlns; @@ -772,7 +788,6 @@ class XmppConnection { if (_isMandatoryNegotiationDone(_streamFeatures) && !_isNegotiationPossible(_streamFeatures)) { _log.finest('Negotiations done!'); _updateRoutingState(RoutingState.handleStanzas); - await _onNegotiationsDone(); } else { _currentNegotiator = getNextNegotiator(_streamFeatures); @@ -782,15 +797,16 @@ class XmppConnection { tag: 'stream:features', children: _streamFeatures, ); - await _currentNegotiator!.negotiate(fakeStanza); - await _checkCurrentNegotiator(); + + await _executeCurrentNegotiator(fakeStanza); } } - } else if (_currentNegotiator!.state == NegotiatorState.retryLater) { + break; + case NegotiatorState.retryLater: _log.finest('Negotiator wants to continue later. Picking new one...'); - _currentNegotiator!.state = NegotiatorState.ready; + if (_isMandatoryNegotiationDone(_streamFeatures) && !_isNegotiationPossible(_streamFeatures)) { _log.finest('Negotiations done!'); @@ -804,24 +820,18 @@ class XmppConnection { tag: 'stream:features', children: _streamFeatures, ); - await _currentNegotiator!.negotiate(fakeStanza); - await _checkCurrentNegotiator(); + await _executeCurrentNegotiator(fakeStanza); } - } else if (_currentNegotiator!.state == NegotiatorState.skipRest) { + break; + case NegotiatorState.skipRest: _log.finest('Negotiator wants to skip the remaining negotiation... Negotiations (assumed) done!'); _updateRoutingState(RoutingState.handleStanzas); await _onNegotiationsDone(); - } else if (_currentNegotiator!.state == NegotiatorState.error) { - _log.severe('Negotiator returned an error'); - await handleError(null); + break; } } - void _closeSocket() { - _socket.close(); - } - /// Called whenever we receive data that has been parsed as XML. Future handleXmlStream(XMLNode node) async { // Check if we received a stream error @@ -829,7 +839,7 @@ class XmppConnection { _log ..finest('<== ${node.toXml()}') ..severe('Received a stream error! Attempting reconnection'); - await handleError('Stream error'); + await handleError(StreamError()); return; } @@ -849,53 +859,14 @@ class XmppConnection { return; } - if (_currentNegotiator != null) { - // If we already have a negotiator, just let it do its thing - _log.finest('Negotiator currently active...'); - - await _currentNegotiator!.negotiate(node); - await _checkCurrentNegotiator(); - } else { + if (node.tag == 'stream:features') { + // Store the received stream features _streamFeatures - ..clear() - ..addAll(node.children); - - // We need to pick a new one - if (_isMandatoryNegotiationDone(node.children)) { - // Mandatory features are done but can we still negotiate more? - if (_isNegotiationPossible(node.children)) {// We can still negotiate features, so do that. - _log.finest('All required stream features done! Continuing negotiation'); - _currentNegotiator = getNextNegotiator(node.children); - _log.finest('Chose $_currentNegotiator as next negotiator'); - await _currentNegotiator!.negotiate(node); - await _checkCurrentNegotiator(); - } else { - _updateRoutingState(RoutingState.handleStanzas); - } - } else { - // There still are mandatory features - if (!_isNegotiationPossible(node.children)) { - _log.severe('Mandatory negotiations not done but continuation not possible'); - _updateRoutingState(RoutingState.error); - await _setConnectionState(XmppConnectionState.error); - - // Resolve the connection completion future - _connectionCompleter?.complete( - const XmppConnectionResult( - false, - reason: 'Could not complete connection negotiations', - ), - ); - _connectionCompleter = null; - return; - } - - _currentNegotiator = getNextNegotiator(node.children); - _log.finest('Chose $_currentNegotiator as next negotiator'); - await _currentNegotiator!.negotiate(node); - await _checkCurrentNegotiator(); - } + ..clear() + ..addAll(node.children); } + + await _executeCurrentNegotiator(node); }); break; case RoutingState.handleStanzas: @@ -927,20 +898,6 @@ class XmppConnection { } else if (event is AuthenticationSuccessEvent) { _log.finest('Received AuthenticationSuccessEvent. Setting _isAuthenticated to true'); _isAuthenticated = true; - } else if (event is AuthenticationFailedEvent) { - _log.finest('Failed authentication'); - _updateRoutingState(RoutingState.error); - await _setConnectionState(XmppConnectionState.error); - - // Resolve the connection completion future - _connectionCompleter?.complete( - XmppConnectionResult( - false, - reason: 'Authentication failed: ${event.saslError}', - ), - ); - _connectionCompleter = null; - _closeSocket(); } for (final manager in _xmppManagers.values) { @@ -1053,7 +1010,7 @@ class XmppConnection { port: port, ); if (!result) { - await handleError(null); + await handleError(NoConnectionError()); } else { await _reconnectionPolicy.onSuccess(); _log.fine('Preparing the internal state for a connection attempt'); diff --git a/packages/moxxmpp/lib/src/errors.dart b/packages/moxxmpp/lib/src/errors.dart new file mode 100644 index 0000000..15dbcb8 --- /dev/null +++ b/packages/moxxmpp/lib/src/errors.dart @@ -0,0 +1,20 @@ +import 'package:moxxmpp/src/socket.dart'; + +/// An internal error class +abstract class XmppError {} + +/// Returned if we could not establish a TCP connection +/// to the server. +class NoConnectionError extends XmppError {} + +/// Returned if a socket error occured +class SocketError extends XmppError { + SocketError(this.event); + final XmppSocketErrorEvent event; +} + +/// Returned if we time out +class TimeoutError extends XmppError {} + +/// Returned if we received a stream error +class StreamError extends XmppError {} diff --git a/packages/moxxmpp/lib/src/negotiators/negotiator.dart b/packages/moxxmpp/lib/src/negotiators/negotiator.dart index c91a945..57ba230 100644 --- a/packages/moxxmpp/lib/src/negotiators/negotiator.dart +++ b/packages/moxxmpp/lib/src/negotiators/negotiator.dart @@ -1,10 +1,12 @@ import 'package:moxlib/moxlib.dart'; +import 'package:moxxmpp/src/errors.dart'; import 'package:moxxmpp/src/events.dart'; import 'package:moxxmpp/src/jid.dart'; import 'package:moxxmpp/src/managers/base.dart'; import 'package:moxxmpp/src/settings.dart'; import 'package:moxxmpp/src/socket.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/types/result.dart'; /// The state a negotiator is currently in enum NegotiatorState { @@ -14,15 +16,15 @@ enum NegotiatorState { done, // Cancel the current attempt but we are not done retryLater, - // The negotiator is in an error state - error, // Skip the rest of the negotiation and assume the stream ready. Only use this when // using stream restoration XEPs, like Stream Management. skipRest, } -class NegotiatorAttributes { +/// A base class for all errors that may occur during feature negotiation +abstract class NegotiatorError extends XmppError {} +class NegotiatorAttributes { const NegotiatorAttributes( this.sendNonza, this.getConnectionSettings, @@ -97,7 +99,7 @@ abstract class XmppFeatureNegotiatorBase { /// must switch some internal state to prevent getting matched immediately again. /// If ready is returned, then the negotiator indicates that it is not done with /// negotiation. - Future negotiate(XMLNode nonza); + Future> negotiate(XMLNode nonza); /// Reset the negotiator to a state that negotation can happen again. void reset() { diff --git a/packages/moxxmpp/lib/src/negotiators/resource_binding.dart b/packages/moxxmpp/lib/src/negotiators/resource_binding.dart index 9be1fbf..ec46984 100644 --- a/packages/moxxmpp/lib/src/negotiators/resource_binding.dart +++ b/packages/moxxmpp/lib/src/negotiators/resource_binding.dart @@ -4,9 +4,12 @@ import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/types/result.dart'; import 'package:moxxmpp/src/xeps/xep_0198/xep_0198.dart'; import 'package:uuid/uuid.dart'; +class ResourceBindingFailedError extends NegotiatorError {} + class ResourceBindingNegotiator extends XmppFeatureNegotiatorBase { ResourceBindingNegotiator() : _requestSent = false, super(0, false, bindXmlns, resourceBindingNegotiator); @@ -23,7 +26,7 @@ class ResourceBindingNegotiator extends XmppFeatureNegotiatorBase { } @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { if (!_requestSent) { final stanza = XMLNode.xmlns( tag: 'iq', @@ -42,10 +45,10 @@ class ResourceBindingNegotiator extends XmppFeatureNegotiatorBase { _requestSent = true; attributes.sendNonza(stanza); + return const Result(NegotiatorState.ready); } else { if (nonza.tag != 'iq' || nonza.attributes['type'] != 'result') { - state = NegotiatorState.error; - return; + return Result(ResourceBindingFailedError()); } final bind = nonza.firstTag('bind')!; @@ -53,7 +56,7 @@ class ResourceBindingNegotiator extends XmppFeatureNegotiatorBase { final resource = jid.innerText().split('/')[1]; await attributes.sendEvent(ResourceBindingSuccessEvent(resource: resource)); - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } } diff --git a/packages/moxxmpp/lib/src/negotiators/sasl/errors.dart b/packages/moxxmpp/lib/src/negotiators/sasl/errors.dart new file mode 100644 index 0000000..32348fa --- /dev/null +++ b/packages/moxxmpp/lib/src/negotiators/sasl/errors.dart @@ -0,0 +1,3 @@ +import 'package:moxxmpp/src/negotiators/negotiator.dart'; + +class SaslFailedError extends NegotiatorError {} diff --git a/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart b/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart index d6d5a81..da40ea6 100644 --- a/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart +++ b/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart @@ -1,12 +1,13 @@ import 'dart:convert'; - import 'package:logging/logging.dart'; import 'package:moxxmpp/src/events.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; +import 'package:moxxmpp/src/negotiators/sasl/errors.dart'; import 'package:moxxmpp/src/negotiators/sasl/negotiator.dart'; import 'package:moxxmpp/src/negotiators/sasl/nonza.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/types/result.dart'; class SaslPlainAuthNonza extends SaslAuthNonza { SaslPlainAuthNonza(String username, String password) : super( @@ -15,7 +16,6 @@ class SaslPlainAuthNonza extends SaslAuthNonza { } class SaslPlainNegotiator extends SaslNegotiator { - SaslPlainNegotiator() : _authSent = false, _log = Logger('SaslPlainNegotiator'), @@ -41,7 +41,7 @@ class SaslPlainNegotiator extends SaslNegotiator { } @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { if (!_authSent) { final settings = attributes.getConnectionSettings(); attributes.sendNonza( @@ -49,17 +49,17 @@ class SaslPlainNegotiator extends SaslNegotiator { redact: SaslPlainAuthNonza('******', '******').toXml(), ); _authSent = true; + return const Result(NegotiatorState.ready); } else { final tag = nonza.tag; if (tag == 'success') { await attributes.sendEvent(AuthenticationSuccessEvent()); - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } else { // We assume it's a final error = nonza.children.first.tag; await attributes.sendEvent(AuthenticationFailedEvent(error)); - - state = NegotiatorState.error; + return Result(SaslFailedError()); } } } diff --git a/packages/moxxmpp/lib/src/negotiators/sasl/scram.dart b/packages/moxxmpp/lib/src/negotiators/sasl/scram.dart index c4157f1..b8f5b7d 100644 --- a/packages/moxxmpp/lib/src/negotiators/sasl/scram.dart +++ b/packages/moxxmpp/lib/src/negotiators/sasl/scram.dart @@ -1,16 +1,17 @@ import 'dart:convert'; import 'dart:math' show Random; - import 'package:cryptography/cryptography.dart'; import 'package:logging/logging.dart'; import 'package:moxxmpp/src/events.dart'; import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; +import 'package:moxxmpp/src/negotiators/sasl/errors.dart'; import 'package:moxxmpp/src/negotiators/sasl/kv.dart'; import 'package:moxxmpp/src/negotiators/sasl/negotiator.dart'; import 'package:moxxmpp/src/negotiators/sasl/nonza.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/types/result.dart'; import 'package:random_string/random_string.dart'; import 'package:saslprep/saslprep.dart'; @@ -89,7 +90,6 @@ enum ScramState { const gs2Header = 'n,,'; class SaslScramNegotiator extends SaslNegotiator { - // NOTE: NEVER, and I mean, NEVER set clientNonce or initalMessageNoGS2. They are just there for testing SaslScramNegotiator( int priority, @@ -197,7 +197,7 @@ class SaslScramNegotiator extends SaslNegotiator { } @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { switch (_scramState) { case ScramState.preSent: if (clientNonce == null || clientNonce == '') { @@ -211,15 +211,14 @@ class SaslScramNegotiator extends SaslNegotiator { SaslScramAuthNonza(body: base64.encode(utf8.encode(gs2Header + initialMessageNoGS2)), type: hashType), redact: SaslScramAuthNonza(body: '******', type: hashType).toXml(), ); - break; + return const Result(NegotiatorState.ready); case ScramState.initialMessageSent: if (nonza.tag != 'challenge') { final error = nonza.children.first.tag; await attributes.sendEvent(AuthenticationFailedEvent(error)); - state = NegotiatorState.error; _scramState = ScramState.error; - return; + return Result(SaslFailedError()); } final challengeBase64 = nonza.innerText(); @@ -230,15 +229,14 @@ class SaslScramNegotiator extends SaslNegotiator { SaslScramResponseNonza(body: responseBase64), redact: SaslScramResponseNonza(body: '******').toXml(), ); - return; + return const Result(NegotiatorState.ready); case ScramState.challengeResponseSent: if (nonza.tag != 'success') { // We assume it's a final error = nonza.children.first.tag; await attributes.sendEvent(AuthenticationFailedEvent(error)); _scramState = ScramState.error; - state = NegotiatorState.error; - return; + return Result(SaslFailedError()); } // NOTE: This assumes that the string is always "v=..." and contains no other parameters @@ -248,16 +246,13 @@ class SaslScramNegotiator extends SaslNegotiator { //final error = nonza.children.first.tag; //attributes.sendEvent(AuthenticationFailedEvent(error)); _scramState = ScramState.error; - state = NegotiatorState.error; - return; + return Result(SaslFailedError()); } await attributes.sendEvent(AuthenticationSuccessEvent()); - state = NegotiatorState.done; - return; + return const Result(NegotiatorState.done); case ScramState.error: - state = NegotiatorState.error; - return; + return Result(SaslFailedError()); } } diff --git a/packages/moxxmpp/lib/src/negotiators/starttls.dart b/packages/moxxmpp/lib/src/negotiators/starttls.dart index e30111b..0d2df94 100644 --- a/packages/moxxmpp/lib/src/negotiators/starttls.dart +++ b/packages/moxxmpp/lib/src/negotiators/starttls.dart @@ -3,12 +3,15 @@ import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/types/result.dart'; enum _StartTlsState { ready, requested } +class StartTLSFailedError extends NegotiatorError {} + class StartTLSNonza extends XMLNode { StartTLSNonza() : super.xmlns( tag: 'starttls', @@ -27,18 +30,17 @@ class StartTlsNegotiator extends XmppFeatureNegotiatorBase { final Logger _log; @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { switch (_state) { case _StartTlsState.ready: _log.fine('StartTLS is available. Performing StartTLS upgrade...'); _state = _StartTlsState.requested; attributes.sendNonza(StartTLSNonza()); - break; + return const Result(NegotiatorState.ready); case _StartTlsState.requested: if (nonza.tag != 'proceed' || nonza.attributes['xmlns'] != startTlsXmlns) { _log.severe('Failed to perform StartTLS negotiation'); - state = NegotiatorState.error; - return; + return Result(StartTLSFailedError()); } _log.fine('Securing socket'); @@ -46,13 +48,11 @@ class StartTlsNegotiator extends XmppFeatureNegotiatorBase { .secure(attributes.getConnectionSettings().jid.domain); if (!result) { _log.severe('Failed to secure stream'); - state = NegotiatorState.error; - return; + return Result(StartTLSFailedError()); } _log.fine('Stream is now TLS secured'); - state = NegotiatorState.done; - break; + return const Result(NegotiatorState.done); } } diff --git a/packages/moxxmpp/lib/src/roster.dart b/packages/moxxmpp/lib/src/roster.dart index ffe9736..4417402 100644 --- a/packages/moxxmpp/lib/src/roster.dart +++ b/packages/moxxmpp/lib/src/roster.dart @@ -10,6 +10,7 @@ import 'package:moxxmpp/src/negotiators/negotiator.dart'; import 'package:moxxmpp/src/stanza.dart'; import 'package:moxxmpp/src/stringxml.dart'; import 'package:moxxmpp/src/types/error.dart'; +import 'package:moxxmpp/src/types/result.dart'; const rosterErrorNoQuery = 1; const rosterErrorNonResult = 2; @@ -53,11 +54,11 @@ class RosterFeatureNegotiator extends XmppFeatureNegotiatorBase { bool get isSupported => _supported; @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { // negotiate is only called when the negotiator matched, meaning the server // advertises roster versioning. _supported = true; - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } @override diff --git a/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart b/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart index 606428f..9f96b7d 100644 --- a/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart +++ b/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart @@ -5,6 +5,7 @@ import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/types/result.dart'; import 'package:moxxmpp/src/xeps/xep_0198/nonzas.dart'; import 'package:moxxmpp/src/xeps/xep_0198/state.dart'; import 'package:moxxmpp/src/xeps/xep_0198/xep_0198.dart'; @@ -23,7 +24,6 @@ enum _StreamManagementNegotiatorState { /// StreamManagementManager at least once before connecting, if stream resumption /// is wanted. class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { - StreamManagementNegotiator() : _state = _StreamManagementNegotiatorState.ready, _supported = false, @@ -59,7 +59,7 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { } @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { // negotiate is only called when we matched the stream feature, so we know // that the server advertises it. _supported = true; @@ -80,7 +80,8 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { _state = _StreamManagementNegotiatorState.enableRequested; attributes.sendNonza(StreamManagementEnableNonza()); } - break; + + return const Result(NegotiatorState.ready); case _StreamManagementNegotiatorState.resumeRequested: if (nonza.tag == 'resumed') { _log.finest('Stream Management resumption successful'); @@ -97,7 +98,7 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { _resumeFailed = false; _isResumed = true; - state = NegotiatorState.skipRest; + return const Result(NegotiatorState.skipRest); } else { // We assume it is _log.info('Stream resumption failed. Expected , got ${nonza.tag}, Proceeding with new stream...'); @@ -113,9 +114,8 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { _resumeFailed = true; _isResumed = false; _state = _StreamManagementNegotiatorState.ready; - state = NegotiatorState.retryLater; + return const Result(NegotiatorState.retryLater); } - break; case _StreamManagementNegotiatorState.enableRequested: if (nonza.tag == 'enabled') { _log.finest('Stream Management enabled'); @@ -133,14 +133,12 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { ), ); - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } else { // We assume a _log.warning('Stream Management enablement failed'); - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } - - break; } } diff --git a/packages/moxxmpp/lib/src/xeps/xep_0352.dart b/packages/moxxmpp/lib/src/xeps/xep_0352.dart index 92d4ba7..da81c28 100644 --- a/packages/moxxmpp/lib/src/xeps/xep_0352.dart +++ b/packages/moxxmpp/lib/src/xeps/xep_0352.dart @@ -4,6 +4,7 @@ import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/types/result.dart'; class CSIActiveNonza extends XMLNode { CSIActiveNonza() : super( @@ -32,11 +33,11 @@ class CSINegotiator extends XmppFeatureNegotiatorBase { bool get isSupported => _supported; @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { // negotiate is only called when the negotiator matched, meaning the server // advertises CSI. _supported = true; - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } @override diff --git a/packages/moxxmpp/test/negotiator_test.dart b/packages/moxxmpp/test/negotiator_test.dart index 5b934a0..e2589ef 100644 --- a/packages/moxxmpp/test/negotiator_test.dart +++ b/packages/moxxmpp/test/negotiator_test.dart @@ -3,10 +3,10 @@ import 'package:test/test.dart'; import 'helpers/logging.dart'; import 'helpers/xmpp.dart'; -const exampleXmlns1 = 'im:moxxy:example1'; -const exampleNamespace1 = 'im.moxxy.test.example1'; -const exampleXmlns2 = 'im:moxxy:example2'; -const exampleNamespace2 = 'im.moxxy.test.example2'; +const exampleXmlns1 = 'im:moxxmpp:example1'; +const exampleNamespace1 = 'im.moxxmpp.test.example1'; +const exampleXmlns2 = 'im:moxxmpp:example2'; +const exampleNamespace2 = 'im.moxxmpp.test.example2'; class StubNegotiator1 extends XmppFeatureNegotiatorBase { StubNegotiator1() : called = false, super(1, false, exampleXmlns1, exampleNamespace1); @@ -14,9 +14,9 @@ class StubNegotiator1 extends XmppFeatureNegotiatorBase { bool called; @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { called = true; - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } } @@ -26,9 +26,9 @@ class StubNegotiator2 extends XmppFeatureNegotiatorBase { bool called; @override - Future negotiate(XMLNode nonza) async { + Future> negotiate(XMLNode nonza) async { called = true; - state = NegotiatorState.done; + return const Result(NegotiatorState.done); } } @@ -47,8 +47,8 @@ void main() { from="test.server" xml:lang="en"> - - + + ''', ), ], diff --git a/packages/moxxmpp/test/sasl/scram_test.dart b/packages/moxxmpp/test/sasl/scram_test.dart index 973cb43..4e15309 100644 --- a/packages/moxxmpp/test/sasl/scram_test.dart +++ b/packages/moxxmpp/test/sasl/scram_test.dart @@ -128,9 +128,9 @@ void main() { 'c=biws,r=rOprNGfwEbeRWgbNEkqO%hvYDpWUa2RaTCAfuxFIlj)hNlF\$k0,p=dHzbZapWIk4jUhN+Ute9ytag9zjfMHgsqmmiz7AndVQ=', ); - await negotiator.negotiate(XMLNode.fromString("dj02cnJpVFJCaTIzV3BSUi93dHVwK21NaFVaVW4vZEI1bkxUSlJzamw5NUc0PQ==")); + final result = await negotiator.negotiate(XMLNode.fromString("dj02cnJpVFJCaTIzV3BSUi93dHVwK21NaFVaVW4vZEI1bkxUSlJzamw5NUc0PQ==")); - expect(negotiator.state, NegotiatorState.done); + expect(result.get(), NegotiatorState.done); }); test('Test a positive server signature check', () async { @@ -150,9 +150,9 @@ void main() { await negotiator.negotiate(scramSha1StreamFeatures); await negotiator.negotiate(XMLNode.fromString("cj1meWtvK2QybGJiRmdPTlJ2OXFreGRhd0wzcmZjTkhZSlkxWlZ2V1ZzN2oscz1RU1hDUitRNnNlazhiZjkyLGk9NDA5Ng==")); - await negotiator.negotiate(XMLNode.fromString("dj1ybUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); + final result = await negotiator.negotiate(XMLNode.fromString("dj1ybUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); - expect(negotiator.state, NegotiatorState.done); + expect(result.get(), NegotiatorState.done); }); test('Test a negative server signature check', () async { @@ -170,11 +170,15 @@ void main() { ), ); - await negotiator.negotiate(scramSha1StreamFeatures); - await negotiator.negotiate(XMLNode.fromString("cj1meWtvK2QybGJiRmdPTlJ2OXFreGRhd0wzcmZjTkhZSlkxWlZ2V1ZzN2oscz1RU1hDUitRNnNlazhiZjkyLGk9NDA5Ng==")); - await negotiator.negotiate(XMLNode.fromString("dj1zbUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); + var result; + result = await negotiator.negotiate(scramSha1StreamFeatures); + expect(result.isType(), true); - expect(negotiator.state, NegotiatorState.error); + result = await negotiator.negotiate(XMLNode.fromString("cj1meWtvK2QybGJiRmdPTlJ2OXFreGRhd0wzcmZjTkhZSlkxWlZ2V1ZzN2oscz1RU1hDUitRNnNlazhiZjkyLGk9NDA5Ng==")); + expect(result.isType(), true); + + result = await negotiator.negotiate(XMLNode.fromString("dj1zbUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); + expect(result.isType(), true); }); test('Test a resetting the SCRAM negotiator', () async { @@ -194,14 +198,14 @@ void main() { await negotiator.negotiate(scramSha1StreamFeatures); await negotiator.negotiate(XMLNode.fromString("cj1meWtvK2QybGJiRmdPTlJ2OXFreGRhd0wzcmZjTkhZSlkxWlZ2V1ZzN2oscz1RU1hDUitRNnNlazhiZjkyLGk9NDA5Ng==")); - await negotiator.negotiate(XMLNode.fromString("dj1ybUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); - expect(negotiator.state, NegotiatorState.done); + final result1 = await negotiator.negotiate(XMLNode.fromString("dj1ybUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); + expect(result1.get(), NegotiatorState.done); // Reset and try again negotiator.reset(); await negotiator.negotiate(scramSha1StreamFeatures); await negotiator.negotiate(XMLNode.fromString("cj1meWtvK2QybGJiRmdPTlJ2OXFreGRhd0wzcmZjTkhZSlkxWlZ2V1ZzN2oscz1RU1hDUitRNnNlazhiZjkyLGk9NDA5Ng==")); - await negotiator.negotiate(XMLNode.fromString("dj1ybUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); - expect(negotiator.state, NegotiatorState.done); + final result2 = await negotiator.negotiate(XMLNode.fromString("dj1ybUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9")); + expect(result2.get(), NegotiatorState.done); }); }