Skip to content

[WIP] initial tests for key backup #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
uhoreg wants to merge 1 commit into from
Closed

[WIP] initial tests for key backup #486

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
271 changes: 271 additions & 0 deletions tests/41end-to-end-keys/07-backup.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,271 @@
my $fixture = local_user_fixture();

my $current_version; # FIXME: is there a better way of passing the backup version between tests?
Copy link
Copy Markdown
Member

@ara4n ara4n Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine.


test "Can create backup version",
requires => [ $fixture ],

proves => [qw( can_create_backup_version )],

do => sub {
my ( $user ) = @_;

do_request_json_for( $user,
method => "POST",
uri => "/unstable/room_keys/version",
content => {
algorithm => "m.megolm_backup.v1",
Copy link
Copy Markdown
Member

@ara4n ara4n Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, shouldn't we use a real algorithm for this? (or is this a real one? it surprises me it doesn't include info about the ciphers being used, like the m.room.encryption algorithm does)

Copy link
Copy Markdown
Member Author

@uhoreg uhoreg Aug 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the algorithm given in the proposal, but I agree, it should probably include some algorithm info

auth_data => "anopaquestring",
}
)->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

assert_json_keys( $content, "version" );

$current_version = $content->{version};

do_request_json_for( $user,
method => "GET",
uri => "/unstable/room_keys/version",
);
})->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

assert_json_keys( $content, "algorithm" );

assert_json_keys( $content, "auth_data" );

$content->{algorithm} eq "m.megolm_backup.v1" or
Copy link
Copy Markdown
Member

@ara4n ara4n Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking, but i'd probably factor out these constants in the name of DRY

Copy link
Copy Markdown
Member

@ara4n ara4n Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, ignore this, it's more legible at it is.

die "Expected algorithm to match submitted data";

$content->{auth_data} eq "anopaquestring" or
die "Expected auth_data to match submitted data";

# FIXME: check that version matches the version returned above
Copy link
Copy Markdown
Member

@ara4n ara4n Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, isn't this just comparing $current_version with $content->{version} or something?

Copy link
Copy Markdown
Member Author

@uhoreg uhoreg Aug 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, except that I don't think the version is returned yet, so I need to fix synapse to do that first. ;)

Copy link
Copy Markdown
Member Author

@uhoreg uhoreg Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it turns out that Synapse does return the version, even though it wasn't in the proposal. 🤷‍♂️

Copy link
Copy Markdown
Member

@ara4n ara4n Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops. i can imagine that i fixed up some of the thinkos in the proposal whilst doing the implementation, but then given the implementation never got fully finished, the proposal wasn't updated - sorry.


Future->done(1);
});
};

test "Can backup keys",
requires => [ $fixture, qw( can_create_backup_version ) ],

proves => [qw( can_backup_e2e_keys )],

do => sub {
my ( $user ) = @_;

do_request_json_for( $user,
method => "PUT",
uri => "/unstable/room_keys/keys/!abcd/1234",
params => {
version => $current_version,
},
content => {
first_message_index => 3,
forwarded_count => 0,
is_verified => JSON::false,
session_data => "anopaquestring",
}
)->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

do_request_json_for( $user,
method => "GET",
uri => "/unstable/room_keys/keys/!abcd/1234",
params => {
version => $current_version,
}
);
})->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

assert_json_keys( $content, "first_message_index" );

assert_json_keys( $content, "forwarded_count" );

assert_json_keys( $content, "is_verified" );

assert_json_keys( $content, "session_data" );

$content->{first_message_index} == 3 or
die "Expected first message index to match submitted data";

$content->{forwarded_count} == 0 or
die "Expected forwarded count to match submitted data";

$content->{is_verified} == JSON::false or
die "Expected is_verified to match submitted data";

$content->{session_data} eq "anopaquestring" or
die "Expected session data to match submitted data";

Future->done(1);
});
};

test "Can update keys with better versions",
requires => [ $fixture, qw( can_backup_e2e_keys ) ],

proves => [qw( can_update_e2e_keys )],

do => sub {
my ( $user ) = @_;

do_request_json_for( $user,
method => "PUT",
uri => "/unstable/room_keys/keys/!abcd/1234",
params => {
version => $current_version,
},
content => {
first_message_index => 1,
forwarded_count => 0,
is_verified => JSON::false,
session_data => "anotheropaquestring",
}
)->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

do_request_json_for( $user,
method => "GET",
uri => "/unstable/room_keys/keys/!abcd/1234",
params => {
version => $current_version,
}
);
})->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

assert_json_keys( $content, "first_message_index" );

assert_json_keys( $content, "forwarded_count" );

assert_json_keys( $content, "is_verified" );

assert_json_keys( $content, "session_data" );

$content->{first_message_index} == 1 or
die "Expected first message index to match submitted data";

$content->{forwarded_count} == 0 or
die "Expected forwarded count to match submitted data";

$content->{is_verified} == JSON::false or
die "Expected is_verified to match submitted data";

$content->{session_data} eq "anotheropaquestring" or
die "Expected session data to match submitted data";

Future->done(1);
});
};

test "Will not update keys with worse versions",
requires => [ $fixture, qw( can_update_e2e_keys ) ],

proves => [qw( wont_update_e2e_keys )],

do => sub {
my ( $user ) = @_;

do_request_json_for( $user,
method => "PUT",
uri => "/unstable/room_keys/keys/!abcd/1234",
params => {
version => $current_version,
},
content => {
first_message_index => 5,
forwarded_count => 0,
is_verified => JSON::false,
session_data => "yetanotheropaquestring",
}
)->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

do_request_json_for( $user,
method => "GET",
uri => "/unstable/room_keys/keys/!abcd/1234",
params => {
version => $current_version,
}
);
})->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

assert_json_keys( $content, "first_message_index" );

assert_json_keys( $content, "forwarded_count" );

assert_json_keys( $content, "is_verified" );

assert_json_keys( $content, "session_data" );

# The data should not be overwritten, so should be the same as what
# was set by the previous test.
$content->{first_message_index} == 1 or
die "Expected first message index to match submitted data";

$content->{forwarded_count} == 0 or
Copy link
Copy Markdown
Member

@ara4n ara4n Aug 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably exercise forwarded_count here too - perhaps just put another update attempt in here to check that forwarded keys don't trump non-forwarded ones? (iirc that's how the logic went)

die "Expected forwarded count to match submitted data";

$content->{is_verified} == JSON::false or
die "Expected is_verified to match submitted data";

$content->{session_data} eq "anotheropaquestring" or
die "Expected session data to match submitted data";

Future->done(1);
});
};

test "Will not back up to an old backup version",
requires => [ $fixture, qw( can_create_backup_version ) ],

proves => [qw( wont_backup_to_old_version )],

do => sub {
my ( $user ) = @_;

do_request_json_for( $user,
method => "POST",
uri => "/unstable/room_keys/version",
content => {
algorithm => "m.megolm_backup.v1",
auth_data => "anopaquestring",
}
)->then( sub {
my ( $content ) = @_;
log_if_fail "Content", $content;

assert_json_keys( $content, "version" );

my $old_version = $current_version;

$current_version = $content->{version};

do_request_json_for( $user,
method => "PUT",
uri => "/unstable/room_keys/keys/!abcd/1234",
params => {
version => $old_version,
},
content => {
first_message_index => 3,
forwarded_count => 0,
is_verified => JSON::false,
session_data => "anopaquestring",
}
);
})->main::expect_http_4xx
->then_done(1);
};