fix shared mailbox subscription bug when one user is prefix of other

cassandane/Cassandane/Cyrus/Shared.pm

@@ -0,0 +1,249 @@
+#!/usr/bin/perl
+#
+#  Copyright (c) 2011-2024 Fastmail Pty Ltd. All rights reserved.
+#
+#  Redistribution and use in source and binary forms, with or without
+#  modification, are permitted provided that the following conditions
+#  are met:
+#
+#  1. Redistributions of source code must retain the above copyright
+#     notice, this list of conditions and the following disclaimer.
+#
+#  2. Redistributions in binary form must reproduce the above copyright
+#     notice, this list of conditions and the following disclaimer in
+#     the documentation and/or other materials provided with the
+#     distribution.
+#
+#  3. The name "Fastmail Pty Ltd" must not be used to
+#     endorse or promote products derived from this software without
+#     prior written permission. For permission or any legal
+#     details, please contact
+#      Fastmail Pty Ltd
+#      PO Box 234
+#      Collins St West 8007
+#      Victoria
+#      Australia
+#
+#  4. Redistributions of any form whatsoever must retain the following
+#     acknowledgment:
+#     "This product includes software developed by Fastmail Pty. Ltd."
+#
+#  FASTMAIL PTY LTD DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+#  INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY  AND FITNESS, IN NO
+#  EVENT SHALL OPERA SOFTWARE AUSTRALIA BE LIABLE FOR ANY SPECIAL, INDIRECT
+#  OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF
+#  USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+#  TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+#  OF THIS SOFTWARE.
+#
+
+package Cassandane::Cyrus::Shared;
+use strict;
+use warnings;
+use DateTime;
+use Data::Dumper;
+
+use lib '.';
+use base qw(Cassandane::Cyrus::TestCase);
+use Cassandane::Instance;
+use Cassandane::Mboxname;
+use Cassandane::Util::Log;
+use Cassandane::Util::Words;
+
+$Data::Dumper::Sortkeys = 1;
+
+sub new
+{
+    my ($class, @args) = @_;
+
+    my $self = $class->SUPER::new({ adminstore => 1 }, @args);
+
+    return $self;
+}
+
+sub set_up
+{
+    my ($self) = @_;
+
+    $self->SUPER::set_up();
+}
+
+sub tear_down
+{
+    my ($self) = @_;
+
+    $self->SUPER::tear_down();
+}
+
+sub shared_subscribe_common
+{
+    my ($self, $user1, $user2) = @_;
+
+    my $service = $self->{instance}->get_service('imap');
+    my $config = $self->{instance}->{config};
+    my $sep = $config->get_bool('unixhierarchysep', 'on') ? '/' : '.';
+
+    my $user1_inbox = Cassandane::Mboxname->new(config => $config);
+    $user1_inbox->from_username($user1);
+
+    my @user1_mailboxes = map {
+        $user1_inbox->make_child($_);
+    } random_words(3);
+    $self->{instance}->create_user($user1,
+                                   subdirs => \@user1_mailboxes);
+
+    my $user1_store = $service->create_store(username => $user1);
+    my $user1_talk = $user1_store->get_client();
+
+    foreach my $mb (@user1_mailboxes) {
+        $user1_talk->subscribe($mb->to_external('owner'));
+        $user1_talk->setacl($mb->to_external('owner'), $user2, 'lrs');
+    }
+
+    my $user2_inbox = Cassandane::Mboxname->new(config => $config);
+    $user2_inbox->from_username($user2);
+
+    my @user2_mailboxes = map {
+        $user2_inbox->make_child($_);
+    } random_words(3);
+    $self->{instance}->create_user($user2,
+                                   subdirs => \@user2_mailboxes);
+
+    my $user2_store = $service->create_store(username => $user2);
+    my $user2_talk = $user2_store->get_client();
+
+    foreach my $mb (@user2_mailboxes) {
+        $user2_talk->subscribe($mb->to_external('owner'));
+        $user2_talk->setacl($mb->to_external('owner'), $user1, 'lrs');
+    }
+
+    xlog("subscribe as $user1 to $user2\'s shared mb's");
+    foreach my $mb (@user2_mailboxes) {
+        $user1_talk->subscribe($mb->to_external('other'));
+        $self->assert_equals('ok', $user1_talk->get_last_completion_response());
+    }
+
+    xlog("but not their inbox");
+    $user1_talk->subscribe($user2_inbox->to_external('other'));
+    $self->assert_equals('no', $user1_talk->get_last_completion_response());
+
+    xlog("make sure $user1 has the right subscriptions");
+    my $user1_subs = $user1_talk->list([qw(SUBSCRIBED)],
+                                       '', '*',
+                                       'RETURN', [qw(CHILDREN)]);
+    $self->assert_mailbox_structure($user1_subs, $sep, {
+        (map {(
+            $_->to_external('owner') => [ '\\Subscribed', '\\HasNoChildren' ]
+        )} @user1_mailboxes),
+        (map {(
+            $_->to_external('other') => [
+                '\\Subscribed',
+                '\\HasNoChildren',
+            ]
+        )} @user2_mailboxes),
+    });
+
+    xlog("unsub as $user1 from $user2\'s folders");
+    foreach my $mb (@user2_mailboxes) {
+        $user1_talk->unsubscribe($mb->to_external('other'));
+        $self->assert_equals('ok', $user1_talk->get_last_completion_response());
+    }
+
+    xlog("make sure $user1 has the right subscriptions");
+    $user1_subs = $user1_talk->list([qw(SUBSCRIBED)],
+                                    '', '*',
+                                    'RETURN', [qw(CHILDREN)]);
+    $self->assert_mailbox_structure($user1_subs, $sep, {
+        (map {(
+            $_->to_external('owner') => [ '\\Subscribed', '\\HasNoChildren' ]
+        )} @user1_mailboxes),
+    });
+
+    xlog("subscribe as $user2 to $user1\'s shared mb's");
+    foreach my $mb (@user1_mailboxes) {
+        $user2_talk->subscribe($mb->to_external('other'));
+        $self->assert_equals('ok',
+                             $user2_talk->get_last_completion_response());
+    }
+
+    xlog("but not their inbox");
+    $user2_talk->subscribe($user1_inbox->to_external('other'));
+    $self->assert_equals('no', $user2_talk->get_last_completion_response());
+
+    xlog("make sure $user2 has the right subscriptions");
+    my $user2_subs = $user2_talk->list([qw(SUBSCRIBED)],
+                                       '', '*',
+                                       'RETURN', [qw(CHILDREN)]);
+    $self->assert_mailbox_structure($user2_subs, $sep, {
+        (map {(
+            $_->to_external('owner') => [ '\\Subscribed', '\\HasNoChildren' ]
+        )} @user2_mailboxes),
+        (map {(
+            $_->to_external('other') => [
+                '\\Subscribed',
+                '\\HasNoChildren',
+            ]
+        )} @user1_mailboxes),
+    });
+
+    xlog("unsub as $user2 from $user1\'s folders");
+    foreach my $mb (@user1_mailboxes) {
+        $user2_talk->unsubscribe($mb->to_external('other'));
+        $self->assert_equals('ok',
+                             $user2_talk->get_last_completion_response());
+    }
+
+    xlog("make sure $user2 has the right subscriptions");
+    $user2_subs = $user2_talk->list([qw(SUBSCRIBED)],
+                                    '', '*',
+                                    'RETURN', [qw(CHILDREN)]);
+    $self->assert_mailbox_structure($user2_subs, $sep, {
+        (map {(
+            $_->to_external('owner') => [ '\\Subscribed', '\\HasNoChildren' ]
+        )} @user2_mailboxes),
+    });
+}
+
+sub test_subscribe
+{
+    my ($self) = @_;
+
+    $self->shared_subscribe_common('firstuser', 'seconduser');
+}
+
+sub test_subscribe_prefix
+{
+    my ($self) = @_;
+
+    # one user is a prefix of the other!
+    $self->shared_subscribe_common('chris', 'christopher');
+}
+
+sub test_subscribe_vd
+    :VirtDomains :CrossDomains
+{
+    my ($self) = @_;
+
+    $self->shared_subscribe_common('firstuser@example.com',
+                                   'seconduser@example.com');
+}
+
+sub test_subscribe_vd_prefix
+    :VirtDomains :CrossDomains
+{
+    my ($self) = @_;
+
+    $self->shared_subscribe_common('matt@example.com',
+                                   'matthew@example.com');
+}
+
+sub test_subscribe_vd_prefix2
+    :VirtDomains :CrossDomains
+{
+    my ($self) = @_;
+
+    $self->shared_subscribe_common('jim@example.com',
+                                   'jim@example.coma.example.com');
+}
+
+1;

cassandane/Cassandane/Cyrus/Lsub.pm → cassandane/Cassandane/Cyrus/Subscriptions.pm

@@ -37,14 +37,18 @@
 #  OF THIS SOFTWARE.
 #
 
-package Cassandane::Cyrus::Lsub;
+package Cassandane::Cyrus::Subscriptions;
 use strict;
 use warnings;
 use DateTime;
 use Data::Dumper;
+use File::Basename;
+use File::Path qw(mkpath);
 
 use lib '.';
 use base qw(Cassandane::Cyrus::TestCase);
+use Cassandane::Util::Log;
+use Cassandane::Util::Words;
 
 sub new
 {
@@ -221,4 +225,87 @@ sub test_lsub_extrachild
     ], "LSUB extrachild mismatch: " . Dumper($subdata));
 }
 
+sub v2
+{
+    my ($first, @rest) = @_;
+
+    my $v2 = 'N' . $first;
+    $v2 .= "\x1f" . $_ for @rest;
+    return $v2;
+}
+
+sub test_upgrade_from_2
+{
+    my ($self) = @_;
+
+    my $userid = 'matt';
+    my @subdirs = random_words(3);
+    $self->{instance}->create_user($userid,
+                                   subdirs => \@subdirs);
+    $self->{instance}->create_user('matthew',
+                                   subdirs => [ 'foo' ]);
+
+    my $mbpath = $self->{instance}->run_mbpath('-u', $userid);
+    my $subdb = $mbpath->{user}->{sub};
+    my $engine = $self->{instance}->{config}->get('subscription_db') || 'flat';
+
+    # generate subscriptions in the v2 format
+    my @items = (
+        # version key
+        [ 'SET', "\x1fVER\x1f", '2' ],
+        # subscribe to own inbox (user.matt)
+        [ 'SET', v2('INBOX') ],
+        # subscribe to own subdirs (user.matt.foo)
+        (map { [ 'SET', v2('INBOX', $_) ] } @subdirs),
+        # some bad subscriptions as if we had hit bug #5146
+        [ 'SET', v2('INBOXhew') ],          # user.matthew
+        [ 'SET', v2('INBOXhew', 'foo') ],   # user.matthew.foo
+    );
+
+    # XXX run_dbcommand needs the file to already exist
+    mkpath(dirname($subdb));
+    open my $fh, '>', $subdb or die "open $subdb: $!";
+    close $fh;
+
+    # create v2 subscriptions file
+    $self->{instance}->run_dbcommand($subdb, $engine, @items);
+
+    # discard syslog accumulated so far
+    $self->{instance}->getsyslog();
+
+    # list user's subscriptions normally (db should be upgraded on open)
+    my $service = $self->{instance}->get_service('imap');
+    my $store = $service->create_store(username => $userid);
+    my $talk = $store->get_client();
+
+    my $subdata = $talk->lsub("", "*");
+    $self->assert_mailbox_structure($subdata, '.', {
+        'INBOX' => [],
+        (map { $_ => [] } @subdirs),
+        'Other Users.matthew' => [ '\\HasChildren' ],
+        'Other Users.matthew.foo' => [],
+    });
+    $talk->logout();
+
+    # check that we upgraded
+    $self->assert_syslog_matches($self->{instance},
+                                 qr{upgrading user subscriptions});
+
+    # list user's subscriptions again
+    $talk = $store->get_client();
+
+    $subdata = $talk->lsub("", "*");
+    $self->assert_mailbox_structure($subdata, '.', {
+        'INBOX' => [],
+        (map { $_ => [] } @subdirs),
+        'Other Users.matthew' => [ '\\HasChildren' ],
+        'Other Users.matthew.foo' => [],
+    });
+    $talk->logout();
+
+    # better not have upgraded this time
+    $self->assert_syslog_does_not_match($self->{instance},
+                                        qr{upgrading user subscriptions});
+}
+
 1;

cassandane/Cassandane/Instance.pm

@@ -1073,8 +1073,15 @@ sub create_user
     my $adminclient = $adminstore->get_client();
 
     my @mboxes = ( $mb->to_external() );
-    map { push(@mboxes, $mb->make_child($_)->to_external()); } @{$params{subdirs}}
-        if ($params{subdirs});
+    foreach my $subdir (defined $params{subdirs} ? @{$params{subdirs}} : ())
+    {
+        if (ref $subdir eq 'Cassandane::Mboxname') {
+            push(@mboxes, $subdir->to_external());
+        }
+        else {
+            push(@mboxes, $mb->make_child($subdir)->to_external());
+        }
+    }
 
     foreach my $mb (@mboxes)
     {