Clean-up & refactor

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 := `<stream:features>
   <bind xmlns='urn:ietf:params:xml:ns:xmpp-bind'/>
 </stream: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 := `<iq id='%s' type='result'>
   <bind xmlns='urn:ietf:params:xml:ns:xmpp-bind'>
   	<jid>%s</jid>

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, "<iq type='set' id='%s'><session xmlns='%s'/></iq>", 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())

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{})
 }

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

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