From 40e907e8ee61615fa246302d726dbdc12697a1f0 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Sat, 29 Jun 2019 16:49:54 +0200 Subject: [PATCH] Clean-up & refactor --- client_test.go | 8 ++-- session.go | 7 ++-- stanza/{auth_sasl.go => sasl_auth.go} | 59 +++++++++++++++++++-------- stanza/sasl_auth_test.go | 29 +++++++++++++ stanza/stream.go | 5 ++- 5 files changed, 81 insertions(+), 27 deletions(-) rename stanza/{auth_sasl.go => sasl_auth.go} (51%) create mode 100644 stanza/sasl_auth_test.go diff --git a/client_test.go b/client_test.go index a599d5b..d55a1d6 100644 --- a/client_test.go +++ b/client_test.go @@ -161,7 +161,7 @@ func readAuth(t *testing.T, decoder *xml.Decoder) string { } var nv interface{} - nv = &stanza.Auth{} + nv = &stanza.SASLAuth{} // Decode element into pointer storage if err = decoder.DecodeElement(nv, &se); err != nil { t.Errorf("cannot decode auth: %s", err) @@ -169,14 +169,14 @@ func readAuth(t *testing.T, decoder *xml.Decoder) string { } switch v := nv.(type) { - case *stanza.Auth: + case *stanza.SASLAuth: return v.Value } return "" } func sendBindFeature(t *testing.T, c net.Conn, _ *xml.Decoder) { - // This is a basic server, supporting only 1 stream feature: SASL Plain Auth + // This is a basic server, supporting only 1 stream feature after auth: session binding features := ` ` @@ -201,7 +201,7 @@ func bind(t *testing.T, c net.Conn, decoder *xml.Decoder) { // TODO Check all elements switch iq.Payload.(type) { - case *stanza.BindBind: + case *stanza.Bind: result := ` %s diff --git a/session.go b/session.go index 7e7a60a..84dd040 100644 --- a/session.go +++ b/session.go @@ -167,7 +167,7 @@ func (s *Session) bind(o Config) { // TODO Check all elements switch payload := iq.Payload.(type) { - case *stanza.BindBind: + case *stanza.Bind: s.BindJid = payload.Jid // our local id (with possibly randomly generated resource default: s.err = errors.New("iq bind result missing") @@ -176,15 +176,14 @@ func (s *Session) bind(o Config) { return } -// TODO: remove when ejabberd is fixed: https://github.com/processone/ejabberd/issues/869 -// After the bind, if the session is required (as per old RFC 3921), we send the session open iq +// After the bind, if the session is not optional (as per old RFC 3921), we send the session open iq. func (s *Session) rfc3921Session(o Config) { if s.err != nil { return } var iq stanza.IQ - if s.Features.Session.Optional.Local != "" { + if s.Features.Session.XMLName.Local == "session" && !s.Features.Session.Optional { fmt.Fprintf(s.streamLogger, "", s.PacketId(), stanza.NSSession) if s.err = s.decoder.Decode(&iq); s.err != nil { s.err = errors.New("expecting iq result after session open: " + s.err.Error()) diff --git a/stanza/auth_sasl.go b/stanza/sasl_auth.go similarity index 51% rename from stanza/auth_sasl.go rename to stanza/sasl_auth.go index 5961d9f..b38570c 100644 --- a/stanza/auth_sasl.go +++ b/stanza/sasl_auth.go @@ -3,8 +3,20 @@ package stanza import "encoding/xml" // ============================================================================ -// SASLSuccess +// SASLAuth implements SASL Authentication initiation. +// Reference: https://tools.ietf.org/html/rfc6120#section-6.4.2 +type SASLAuth struct { + XMLName xml.Name `xml:"urn:ietf:params:xml:ns:xmpp-sasl auth"` + Mechanism string `xml:"mechanism,attr"` + Value string `xml:",innerxml"` +} + +// ============================================================================ + +// SASLSuccess implements SASL Success nonza, sent by server as a result of the +// SASL auth negotiation. +// Reference: https://tools.ietf.org/html/rfc6120#section-6.4.6 type SASLSuccess struct { XMLName xml.Name `xml:"urn:ietf:params:xml:ns:xmpp-sasl success"` } @@ -13,6 +25,7 @@ func (SASLSuccess) Name() string { return "sasl:success" } +// SASLSuccess decoding type saslSuccessDecoder struct{} var saslSuccess saslSuccessDecoder @@ -24,8 +37,8 @@ func (saslSuccessDecoder) decode(p *xml.Decoder, se xml.StartElement) (SASLSucce } // ============================================================================ -// SASLFailure +// SASLFailure type SASLFailure struct { XMLName xml.Name `xml:"urn:ietf:params:xml:ns:xmpp-sasl failure"` Any xml.Name // error reason is a subelement @@ -35,6 +48,7 @@ func (SASLFailure) Name() string { return "sasl:failure" } +// SASLFailure decoding type saslFailureDecoder struct{} var saslFailure saslFailureDecoder @@ -45,35 +59,46 @@ func (saslFailureDecoder) decode(p *xml.Decoder, se xml.StartElement) (SASLFailu return packet, err } -// ============================================================================ +// =========================================================================== +// Resource binding -type Auth struct { - XMLName xml.Name `xml:"urn:ietf:params:xml:ns:xmpp-sasl auth"` - Mechanism string `xml:"mecanism,attr"` - Value string `xml:",innerxml"` -} - -type BindBind struct { +// Bind is an IQ payload used during session negotiation to bind user resource +// to the current XMPP stream. +// Reference: https://tools.ietf.org/html/rfc6120#section-7 +type Bind struct { XMLName xml.Name `xml:"urn:ietf:params:xml:ns:xmpp-bind bind"` Resource string `xml:"resource,omitempty"` Jid string `xml:"jid,omitempty"` } -func (b *BindBind) Namespace() string { +func (b *Bind) Namespace() string { return b.XMLName.Space } -// Session is obsolete in RFC 6121. -// Added for compliance with RFC 3121. -// Remove when ejabberd purely conforms to RFC 6121. -type sessionSession struct { +// ============================================================================ +// Session (Obsolete) + +// Session is both a stream feature and an obsolete IQ Payload, used to bind a +// resource to the current XMPP stream on RFC 3121 only XMPP servers. +// Session is obsolete in RFC 6121. It is added to Fluux XMPP for compliance +// with RFC 3121. +// Reference: https://xmpp.org/rfcs/rfc3921.html#session +// +// This is the draft defining how to handle the transition: +// https://tools.ietf.org/html/draft-cridland-xmpp-session-01 +type StreamSession struct { XMLName xml.Name `xml:"urn:ietf:params:xml:ns:xmpp-session session"` - Optional xml.Name // If it does exist, it mean we are not required to open session + Optional bool // If element does exist, it mean we are not required to open session +} + +func (s *StreamSession) Namespace() string { + return s.XMLName.Space } // ============================================================================ // Registry init func init() { - TypeRegistry.MapExtension(PKTIQ, xml.Name{"urn:ietf:params:xml:ns:xmpp-bind", "bind"}, BindBind{}) + TypeRegistry.MapExtension(PKTIQ, xml.Name{"urn:ietf:params:xml:ns:xmpp-bind", "bind"}, Bind{}) + TypeRegistry.MapExtension(PKTIQ, xml.Name{"urn:ietf:params:xml:ns:xmpp-session", "bind"}, StreamSession{}) } diff --git a/stanza/sasl_auth_test.go b/stanza/sasl_auth_test.go new file mode 100644 index 0000000..df2ee9a --- /dev/null +++ b/stanza/sasl_auth_test.go @@ -0,0 +1,29 @@ +package stanza_test + +import ( + "encoding/xml" + "testing" + + "gosrc.io/xmpp/stanza" +) + +// Check that we can detect optional session from advertised stream features +func TestSession(t *testing.T) { + streamFeatures := stanza.StreamFeatures{Session: stanza.StreamSession{Optional: true}} + + data, err := xml.Marshal(streamFeatures) + if err != nil { + t.Errorf("cannot marshal xml structure: %s", err) + } + + parsedStream := stanza.StreamFeatures{} + if err = xml.Unmarshal(data, &parsedStream); err != nil { + t.Errorf("Unmarshal(%s) returned error", data) + } + + if !parsedStream.Session.Optional { + t.Error("Session should be optional") + } +} + +// TODO Test Sasl mechanism diff --git a/stanza/stream.go b/stanza/stream.go index 221e7fb..11cd96b 100644 --- a/stanza/stream.go +++ b/stanza/stream.go @@ -17,9 +17,10 @@ type StreamFeatures struct { // Stream features StartTLS tlsStartTLS Mechanisms saslMechanisms - Bind BindBind - Session sessionSession + Bind Bind StreamManagement streamManagement + // Obsolete + Session StreamSession // ProcessOne Stream Features P1Push p1Push P1Rebind p1Rebind