Changeset 851

Timestamp:

02/11/09 21:57:06 (10 months ago)

Author:

ykerherve

Message:

Finished resource conflit handling per RFC 3920
this introduced a medium change in the way DJabberd
do JID registration. Now, register_jid takes a bare_jid and
a resource instead of a fulljid so that we can generate a
resource in case of conflict as RECOMMENDED. XEP 0078 behaviour
is kept unchanged though.

Location:

trunk/DJabberd

Files:

• lib/DJabberd/Bot.pm (modified) (1 diff)
• lib/DJabberd/IQ.pm (modified) (8 diffs)
• lib/DJabberd/JID.pm (modified) (2 diffs)
• lib/DJabberd/VHost.pm (modified) (2 diffs)
• t/sasl-login.t (modified) (1 diff)

trunk/DJabberd/lib/DJabberd/Bot.pm

@@ -52 +52 @@
 sub register {
     my ($self, $vhost) = @_;
-    $self->{jid} = DJabberd::JID->new("$self->{nodename}\@" . $vhost->server_name . "/$self->{resource}");
+    my $barejid = DJabberd::JID->new("$self->{nodename}\@" . $vhost->server_name );
+    my $resource = $self->{resource};
 
     $self->{vhost} = $vhost;
     Scalar::Util::weaken($self->{vhost});
 
+    my $reg_jid;
     my $regcb = DJabberd::Callback->new({
         registered => sub {
-            $logger->debug("Bot $self->{jid} is now registered");
+            (undef, my $reg_jid) = @_;
+            $logger->debug("Bot $reg_jid is now registered");
         },
         error => sub {
-            $logger->error("Bot $self->{jid} failed to register");
+            $logger->error("Bot $barejid/$resource failed to register");
             },
     });
 
-    $vhost->register_jid($self->{jid}, $self , $regcb);
-    DJabberd::Presence->set_local_presence(
-        $self->{jid}, 
-        DJabberd::Presence->available( from => $self->{jid} )
-    );
+    $vhost->register_jid($barejid, $resource, $self, $regcb);
+    if ($reg_jid) {
+        $self->{jid} = $reg_jid;
+        DJabberd::Presence->set_local_presence(
+            $self->{jid},
+            DJabberd::Presence->available( from => $self->{jid} )
+        );
+    }
 }
 

trunk/DJabberd/lib/DJabberd/IQ.pm

@@ -496 +496 @@
     my $digest   = $get->("digest");
 
-    return unless $username =~ /^\w+$/;
+    # "Both the username and the resource are REQUIRED for client
+    # authentication" Section 3.1 of XEP 0078
+    return unless $username && $username =~ /^\w+$/;
+    return unless $resource;
 
     my $vhost = $conn->vhost;
@@ -518 +521 @@
 
         # register
-        my $jid = DJabberd::JID->new("$authjid/$resource");
+        my $jid = DJabberd::JID->new("$authjid");
 
         unless ($jid) {
@@ -527 +530 @@
         my $regcb = DJabberd::Callback->new({
             registered => sub {
-                $conn->set_bound_jid($jid);
+                (undef, my $fulljid) = @_;
+                $conn->set_bound_jid($fulljid);
                 $DJabberd::Stats::counter{'auth_success'}++;
                 $iq->send_result;
@@ -540 +544 @@
         });
 
-        $vhost->register_jid($jid, $conn, $regcb);
+        $vhost->register_jid($jid, $resource, $conn, $regcb);
     };
 
@@ -627 +631 @@
     };
 
-    my $resource = $get->("resource")
-                 || Digest::SHA1::sha1_hex(rand() . rand() . rand());
+    my $resource = $get->("resource") || DJabberd::JID->rand_resource;
 
     my $vhost = $conn->vhost;
@@ -669 +672 @@
 
     # register
-    my $jid = DJabberd::JID->new("$authjid/$resource");
+    my $jid = DJabberd::JID->new($authjid);
 
     unless ($jid) {
@@ -678 +681 @@
     my $regcb = DJabberd::Callback->new({
         registered => sub {
-            $conn->set_bound_jid($jid);
+            (undef, my $fulljid) = @_;
+            $conn->set_bound_jid($fulljid);
             $DJabberd::Stats::counter{'auth_success'}++;
             my $xml = <<EOX;
-<iq id='$id' type='result'>
+<iq id='$fulljid' type='result'>
     <bind xmlns='urn:ietf:params:xml:ns:xmpp-bind'>
-        <jid>$jid</jid>
+        <jid>$fulljid</jid>
     </bind>
 </iq>
@@ -699 +703 @@
     });
 
-    $vhost->register_jid($jid, $conn, $regcb);
+    $vhost->register_jid($jid, $resource, $conn, $regcb);
     return 1;
 }

trunk/DJabberd/lib/DJabberd/JID.pm

@@ -2 +2 @@
 use strict;
 use DJabberd::Util qw(exml);
+use Digest::SHA1;
 
 use overload
@@ -76 +77 @@
 }
 
+sub rand_resource {
+    Digest::SHA1::sha1_hex(rand() . rand() . rand());
+}
+
 1;
 

trunk/DJabberd/lib/DJabberd/VHost.pm

@@ -5 +5 @@
 use DJabberd::Util qw(tsub as_bool);
 use DJabberd::Log;
+use DJabberd::JID;
 use DJabberd::Roster;
 
@@ -364 +365 @@
 
 sub register_jid {
-    my ($self, $jid, $conn, $cb) = @_;
+    my ($self, $jid, $resource, $conn, $cb) = @_;
+
+    my $barestr = $jid->as_bare_string; ## $jid should be bare anyway
+    my $fullstr = "$barestr/$resource";
+
     # $cb can ->registered, ->error
-    $logger->info("Registering '$jid' to connection '$conn->{id}'");
-
-    my $barestr = $jid->as_bare_string;
-    my $fullstr = $jid->as_string;
-
+    $logger->info("Registering '$fullstr' to connection '$conn->{id}'");
+
+    ## deprecated 0078 appears a bit conflicting with RFC 3920
+    ## the recommended behaviour in the latter is to generate a resource for
+    ## the dupe. Don't ask me if one resource uses RFC 3920 and the other
+    ## XEP 0078 :D. If we detect a sasl connection, we go with the RFC way.
     if (my $econn = $self->{jid2sock}{$fullstr}) {
-        $econn->stream_error("conflict");
-    }
+        if ($conn->sasl) {
+            my $resource = DJabberd::JID->rand_resource;
+            $fullstr = "$barestr/$resource";
+        }
+        else {
+            $econn->stream_error("conflict");
+        }
+    }
+    my $fulljid = DJabberd::JID->new($fullstr);
 
     $self->{jid2sock}{$fullstr} = $conn;
     ($self->{bare2fulls}{$barestr} ||= {})->{$fullstr} = 1;  # TODO: this should be the connection, not a 1, saves work in unregister JID?
 
-    $cb->registered;
+    $cb->registered($fulljid);
 }
 

trunk/DJabberd/t/sasl-login.t

@@ -66 +66 @@
 {
     two_parties(sub {
-        my ($pa, $pb) = @_;
+        my ($pa1, $pb) = @_;
+        my $conflicted_res = "yann";
 
+        my $pa2 = Test::DJabberd::Client->new(server   => $pa1->server,
+                                              name     => $pa1->username,
+                                              resource => $conflicted_res,
+                                            );
         my $sasl = Authen::SASL->new(
             mechanism => "DIGEST-MD5",
             callback  => {
-                pass => sub { $pa->password },
-                user => sub { $pa->{name}   },
+                pass => sub { $pa1->password },
+                user => sub { $pa1->{name}   },
             },
         );
-        my $pa_res = $pa->sasl_login($sasl, "yann")->resource;
-        my $pb_res = $pb->sasl_login($sasl, "yann")->resource;
-        cmp_ok $pa_res, 'ne', $pb_res, "resources are different";
-        is   $pa_res, "yann", "first got what it wanted";
-        isnt $pb_res, "yann", "second didn't";
+        my $pa1_res = $pa1->sasl_login($sasl, $conflicted_res)->resource;
+
+        my $pa2_res = $pa2->sasl_login($sasl, $conflicted_res)->resource;
+        cmp_ok $pa1_res, 'ne', $pa2_res, "resources are different";
+        is   $pa1_res, $conflicted_res, "first got what it wanted";
+        isnt $pa2_res, $conflicted_res, "second didn't";
     });
 }