From 56e035bb5077545fccdf300d714dcee7c03d26f3 Mon Sep 17 00:00:00 2001 From: Vincent Michel Date: Wed, 11 Dec 2024 16:33:33 +0100 Subject: [PATCH] Address @touilleMan's comments about invite_shamir_recovery_reveal --- libparsec/crates/client/src/invite/claimer.rs | 5 ++++- .../invite_shamir_recovery_reveal.json5 | 5 ++++- .../v4/invite_shamir_recovery_reveal.rs | 20 +++++++++++++++---- misc/gen_protocol_typings.py | 1 - .../v4/invite_shamir_recovery_reveal.pyi | 7 ++++++- server/parsec/components/invite.py | 9 ++++++--- server/parsec/components/memory/invite.py | 15 +++++++++----- .../test_invite_shamir_recovery_reveal.py | 16 +++++++++++++-- 8 files changed, 60 insertions(+), 18 deletions(-) diff --git a/libparsec/crates/client/src/invite/claimer.rs b/libparsec/crates/client/src/invite/claimer.rs index d49d5f22236..712c5552fe1 100644 --- a/libparsec/crates/client/src/invite/claimer.rs +++ b/libparsec/crates/client/src/invite/claimer.rs @@ -495,9 +495,12 @@ impl ShamirRecoveryClaimRecoverDeviceCtx { return Ok(ShamirRecoveryClaimMaybeFinalizeCtx::Offline(self)) } // Errors - Ok(Rep::NotFound) => { + Ok(Rep::BadRevealToken) => { Err(ShamirRecoveryClaimRecoverDeviceError::CipheredDataNotFound) } + Ok(Rep::BadInvitationType) => { + Err(anyhow::anyhow!("Unexpected bad invitation type response").into()) + } Ok(Rep::UnknownStatus { .. }) => { Err(anyhow::anyhow!("Unexpected server response: {:?}", rep).into()) } diff --git a/libparsec/crates/protocol/schema/invited_cmds/invite_shamir_recovery_reveal.json5 b/libparsec/crates/protocol/schema/invited_cmds/invite_shamir_recovery_reveal.json5 index 3b698abd732..ce03cbcbdbe 100644 --- a/libparsec/crates/protocol/schema/invited_cmds/invite_shamir_recovery_reveal.json5 +++ b/libparsec/crates/protocol/schema/invited_cmds/invite_shamir_recovery_reveal.json5 @@ -23,7 +23,10 @@ ] }, { - "status": "not_found" + "status": "bad_invitation_type" + }, + { + "status": "bad_reveal_token" } ] } diff --git a/libparsec/crates/protocol/tests/invited_cmds/v4/invite_shamir_recovery_reveal.rs b/libparsec/crates/protocol/tests/invited_cmds/v4/invite_shamir_recovery_reveal.rs index 23782e1ee82..e04133f21aa 100644 --- a/libparsec/crates/protocol/tests/invited_cmds/v4/invite_shamir_recovery_reveal.rs +++ b/libparsec/crates/protocol/tests/invited_cmds/v4/invite_shamir_recovery_reveal.rs @@ -5,13 +5,25 @@ use super::invited_cmds; use libparsec_tests_lite::{hex, p_assert_eq}; use libparsec_types::InvitationToken; -pub fn rep_not_found() { +pub fn rep_bad_invitation_type() { // Generated from Parsec 3.2.1-a.0+dev // Content: - // status: 'not_found' - let raw: &[u8] = hex!("81a6737461747573a96e6f745f666f756e64").as_ref(); + // status: 'bad_invitation_type' + let raw: &[u8] = hex!("81a6737461747573b36261645f696e7669746174696f6e5f74797065").as_ref(); - let expected = invited_cmds::invite_shamir_recovery_reveal::Rep::NotFound; + let expected = invited_cmds::invite_shamir_recovery_reveal::Rep::BadInvitationType; + println!("***expected: {:?}", expected.dump().unwrap()); + let data = invited_cmds::invite_shamir_recovery_reveal::Rep::load(raw).unwrap(); + p_assert_eq!(data, expected); +} + +pub fn rep_bad_reveal_token() { + // Generated from Parsec 3.2.1-a.0+dev + // Content: + // status: 'bad_reveal_token' + let raw: &[u8] = hex!("81a6737461747573b06261645f72657665616c5f746f6b656e").as_ref(); + + let expected = invited_cmds::invite_shamir_recovery_reveal::Rep::BadRevealToken; println!("***expected: {:?}", expected.dump().unwrap()); let data = invited_cmds::invite_shamir_recovery_reveal::Rep::load(raw).unwrap(); p_assert_eq!(data, expected); diff --git a/misc/gen_protocol_typings.py b/misc/gen_protocol_typings.py index 6ae1eede722..a2faa28bfe3 100755 --- a/misc/gen_protocol_typings.py +++ b/misc/gen_protocol_typings.py @@ -99,7 +99,6 @@ def cook_field_type( ("Index", "int"), ("NonZeroInteger", "int"), ("NonZeroU8", "int"), - ("IntegerBetween1And100", "int"), ]: if raw_type == candidate: return py_type_name diff --git a/server/parsec/_parsec_pyi/protocol/invited_cmds/v4/invite_shamir_recovery_reveal.pyi b/server/parsec/_parsec_pyi/protocol/invited_cmds/v4/invite_shamir_recovery_reveal.pyi index 08a0dfb048c..728ac973da4 100644 --- a/server/parsec/_parsec_pyi/protocol/invited_cmds/v4/invite_shamir_recovery_reveal.pyi +++ b/server/parsec/_parsec_pyi/protocol/invited_cmds/v4/invite_shamir_recovery_reveal.pyi @@ -29,7 +29,12 @@ class RepOk(Rep): @property def ciphered_data(self) -> bytes: ... -class RepNotFound(Rep): +class RepBadInvitationType(Rep): + def __init__( + self, + ) -> None: ... + +class RepBadRevealToken(Rep): def __init__( self, ) -> None: ... diff --git a/server/parsec/components/invite.py b/server/parsec/components/invite.py index 05dff4af4db..01026564cac 100644 --- a/server/parsec/components/invite.py +++ b/server/parsec/components/invite.py @@ -281,7 +281,8 @@ class InviteShamirRecoveryRevealBadOutcome(BadOutcomeEnum): ORGANIZATION_EXPIRED = auto() INVITATION_NOT_FOUND = auto() INVITATION_DELETED = auto() - DATA_NOT_FOUND = auto() + BAD_INVITATION_TYPE = auto() + BAD_REVEAL_TOKEN = auto() # New transport definitions @@ -1379,8 +1380,10 @@ async def api_invite_shamir_recovery_reveal( return invited_cmds.latest.invite_shamir_recovery_reveal.RepOk( ciphered_data=bytes(ciphered_data) ) - case InviteShamirRecoveryRevealBadOutcome.DATA_NOT_FOUND: - return invited_cmds.latest.invite_shamir_recovery_reveal.RepNotFound() + case InviteShamirRecoveryRevealBadOutcome.BAD_INVITATION_TYPE: + return invited_cmds.latest.invite_shamir_recovery_reveal.RepBadInvitationType() + case InviteShamirRecoveryRevealBadOutcome.BAD_REVEAL_TOKEN: + return invited_cmds.latest.invite_shamir_recovery_reveal.RepBadRevealToken() case InviteShamirRecoveryRevealBadOutcome.ORGANIZATION_NOT_FOUND: client_ctx.organization_not_found_abort() case InviteShamirRecoveryRevealBadOutcome.ORGANIZATION_EXPIRED: diff --git a/server/parsec/components/memory/invite.py b/server/parsec/components/memory/invite.py index d45d09e3a6c..5db21b20d02 100644 --- a/server/parsec/components/memory/invite.py +++ b/server/parsec/components/memory/invite.py @@ -589,16 +589,21 @@ async def shamir_recovery_reveal( if invitation.is_deleted: return InviteShamirRecoveryRevealBadOutcome.INVITATION_DELETED - if invitation.claimer_user_id is None: - return InviteShamirRecoveryRevealBadOutcome.DATA_NOT_FOUND + if invitation.type != InvitationType.SHAMIR_RECOVERY: + return InviteShamirRecoveryRevealBadOutcome.BAD_INVITATION_TYPE + assert invitation.claimer_user_id is not None - shamir_recoveries = org.shamir_recoveries.get(invitation.claimer_user_id, []) + # Failing this assert means that some data has been corrupted, + # since there needs to be a valid shamir setup in order to create + # a shamir recovery invitation + assert invitation.claimer_user_id in org.shamir_recoveries + shamir_recoveries = org.shamir_recoveries[invitation.claimer_user_id] if not shamir_recoveries: - return InviteShamirRecoveryRevealBadOutcome.DATA_NOT_FOUND + return InviteShamirRecoveryRevealBadOutcome.BAD_REVEAL_TOKEN *_, shamir_recovery = shamir_recoveries if shamir_recovery.reveal_token != reveal_token: - return InviteShamirRecoveryRevealBadOutcome.DATA_NOT_FOUND + return InviteShamirRecoveryRevealBadOutcome.BAD_REVEAL_TOKEN return shamir_recovery.ciphered_data diff --git a/server/tests/api_v4/invited/test_invite_shamir_recovery_reveal.py b/server/tests/api_v4/invited/test_invite_shamir_recovery_reveal.py index 5e50e123f7c..99e002f4ba9 100644 --- a/server/tests/api_v4/invited/test_invite_shamir_recovery_reveal.py +++ b/server/tests/api_v4/invited/test_invite_shamir_recovery_reveal.py @@ -12,12 +12,24 @@ async def test_invited_invite_shamir_recovery_reveal_ok(shamirorg: ShamirOrgRpcC assert rep == invited_cmds.v4.invite_shamir_recovery_reveal.RepOk(ciphered_data=ciphered_data) -async def test_invited_invite_shamir_recovery_reveal_not_found( +async def test_invited_invite_shamir_recovery_reveal_bad_reveal_token( shamirorg: ShamirOrgRpcClients, ) -> None: token = InvitationToken.new() rep = await shamirorg.shamir_invited_alice.invite_shamir_recovery_reveal(token) - assert rep == invited_cmds.v4.invite_shamir_recovery_reveal.RepNotFound() + assert rep == invited_cmds.v4.invite_shamir_recovery_reveal.RepBadRevealToken() + + +async def test_invited_invite_shamir_recovery_reveal_bad_invitation_type( + coolorg: CoolorgRpcClients, +) -> None: + token = InvitationToken.new() + + rep = await coolorg.invited_zack.invite_shamir_recovery_reveal(token) + assert rep == invited_cmds.v4.invite_shamir_recovery_reveal.RepBadInvitationType() + + rep = await coolorg.invited_alice_dev3.invite_shamir_recovery_reveal(token) + assert rep == invited_cmds.v4.invite_shamir_recovery_reveal.RepBadInvitationType() async def test_invited_invite_shamir_recovery_reveal_http_common_errors(