From 02c99564b2bb419df8a8d47d66f3c3d8c000184f Mon Sep 17 00:00:00 2001 From: Toporkov Igor Date: Wed, 26 Aug 2020 12:23:31 +0300 Subject: [PATCH] FF-134: Validate identity providers on withdrawal creation (#283) * Validate providers on start of withrawal * Test provider_mismatch error * Draft provider_mismath thrift error * Add error drafts to CreateQuote * Upgrade fistful_proto * Throw thrift error * Test identity proviers mismatch HTTP API error * Specify that we are talking about identity providers, fix typo * Add new error spec * Apply suggestions from review * Reword provider_mismatch error description * Fix lines being too long * Reword error description --- apps/ff_server/src/ff_codec.erl | 3 ++ apps/ff_server/src/ff_withdrawal_handler.erl | 10 +++++ apps/ff_transfer/src/ff_withdrawal.erl | 23 ++++++++++- apps/ff_transfer/test/ff_withdrawal_SUITE.erl | 22 ++++++++++ apps/wapi/src/wapi_wallet_ff_backend.erl | 2 + apps/wapi/src/wapi_wallet_handler.erl | 10 +++++ apps/wapi/test/wapi_SUITE.erl | 40 ++++++++++++++++++- rebar.lock | 2 +- 8 files changed, 109 insertions(+), 3 deletions(-) diff --git a/apps/ff_server/src/ff_codec.erl b/apps/ff_server/src/ff_codec.erl index 80423a8..400f254 100644 --- a/apps/ff_server/src/ff_codec.erl +++ b/apps/ff_server/src/ff_codec.erl @@ -74,6 +74,9 @@ marshal(blocking, blocked) -> marshal(blocking, unblocked) -> unblocked; +marshal(identity_provider, Provider) when is_binary(Provider) -> + Provider; + marshal(transaction_info, TransactionInfo = #{ id := TransactionID, extra := Extra diff --git a/apps/ff_server/src/ff_withdrawal_handler.erl b/apps/ff_server/src/ff_withdrawal_handler.erl index 93ab3c7..72053a3 100644 --- a/apps/ff_server/src/ff_withdrawal_handler.erl +++ b/apps/ff_server/src/ff_withdrawal_handler.erl @@ -52,6 +52,11 @@ handle_function_('GetQuote', [MarshaledParams], _Opts) -> destination_currency = ff_codec:marshal(currency_ref, Destination), wallet_currency = ff_codec:marshal(currency_ref, Wallet) }); + {error, {identity_providers_mismatch, {WalletProvider, DestinationProvider}}} -> + woody_error:raise(business, #wthd_IdentityProvidersMismatch{ + wallet_provider = ff_codec:marshal(identity_provider, WalletProvider), + destination_provider = ff_codec:marshal(identity_provider, DestinationProvider) + }); {error, {destination_resource, {bin_data, _}}} -> woody_error:raise(business, #wthd_NoDestinationResourceInfo{}) end; @@ -88,6 +93,11 @@ handle_function_('Create', [MarshaledParams, MarshaledContext], Opts) -> destination_currency = ff_codec:marshal(currency_ref, Destination), wallet_currency = ff_codec:marshal(currency_ref, Wallet) }); + {error, {identity_providers_mismatch, {WalletProvider, DestinationProvider}}} -> + woody_error:raise(business, #wthd_IdentityProvidersMismatch{ + wallet_provider = ff_codec:marshal(identity_provider, WalletProvider), + destination_provider = ff_codec:marshal(identity_provider, DestinationProvider) + }); {error, {destination_resource, {bin_data, not_found}}} -> woody_error:raise(business, #wthd_NoDestinationResourceInfo{}) end; diff --git a/apps/ff_transfer/src/ff_withdrawal.erl b/apps/ff_transfer/src/ff_withdrawal.erl index d859bed..75df0cc 100644 --- a/apps/ff_transfer/src/ff_withdrawal.erl +++ b/apps/ff_transfer/src/ff_withdrawal.erl @@ -74,6 +74,7 @@ {destination, notfound | unauthorized} | {inconsistent_currency, {Withdrawal :: currency_id(), Wallet :: currency_id(), Destination :: currency_id()}} | {terms, ff_party:validate_withdrawal_creation_error()} | + {identity_providers_mismatch, {ff_provider:id(), ff_provider:id()}} | {destination_resource, {bin_data, ff_bin_data:bin_data_error()}}. -type route() :: ff_withdrawal_routing:route(). @@ -1003,6 +1004,15 @@ get_destination(DestinationID) -> identity(). get_wallet_identity(Wallet) -> IdentityID = ff_wallet:identity(Wallet), + get_identity(IdentityID). + +-spec get_destination_identity(destination()) -> + identity(). +get_destination_identity(Destination) -> + IdentityID = ff_destination:identity(Destination), + get_identity(IdentityID). + +get_identity(IdentityID) -> {ok, IdentityMachine} = ff_identity_machine:get(IdentityID), ff_identity_machine:identity(IdentityMachine). @@ -1245,9 +1255,20 @@ validate_withdrawal_creation(Terms, Body, Wallet, Destination) -> do(fun() -> valid = unwrap(terms, validate_withdrawal_creation_terms(Terms, Body)), valid = unwrap(validate_withdrawal_currency(Body, Wallet, Destination)), - valid = unwrap(validate_destination_status(Destination)) + valid = unwrap(validate_destination_status(Destination)), + valid = unwrap(validate_withdrawal_providers(Wallet, Destination)) end). +validate_withdrawal_providers(Wallet, Destination) -> + WalletIdentity = get_wallet_identity(Wallet), + DestinationIdentity = get_destination_identity(Destination), + WalletProvider = ff_identity:provider(WalletIdentity), + DestinationProvider = ff_identity:provider(DestinationIdentity), + case WalletProvider =:= DestinationProvider of + true -> {ok, valid}; + false -> {error, {identity_providers_mismatch, {WalletProvider, DestinationProvider}}} + end. + -spec validate_withdrawal_creation_terms(terms(), body()) -> {ok, valid} | {error, ff_party:validate_withdrawal_creation_error()}. diff --git a/apps/ff_transfer/test/ff_withdrawal_SUITE.erl b/apps/ff_transfer/test/ff_withdrawal_SUITE.erl index 89c7ea3..690d655 100644 --- a/apps/ff_transfer/test/ff_withdrawal_SUITE.erl +++ b/apps/ff_transfer/test/ff_withdrawal_SUITE.erl @@ -24,6 +24,7 @@ -export([limit_check_fail_test/1]). -export([create_cashlimit_validation_error_test/1]). -export([create_wallet_currency_validation_error_test/1]). +-export([create_identity_providers_mismatch_error_test/1]). -export([create_destination_currency_validation_error_test/1]). -export([create_currency_validation_error_test/1]). -export([create_destination_resource_notfound_test/1]). @@ -78,6 +79,7 @@ groups() -> create_wallet_currency_validation_error_test, create_destination_currency_validation_error_test, create_currency_validation_error_test, + create_identity_providers_mismatch_error_test, create_destination_resource_notfound_test, create_destination_notfound_test, create_wallet_notfound_test, @@ -341,6 +343,26 @@ create_currency_validation_error_test(C) -> }, ?assertMatch({error, {terms, {terms_violation, {not_allowed_currency, Details}}}}, Result). +-spec create_identity_providers_mismatch_error_test(config()) -> test_return(). + +create_identity_providers_mismatch_error_test(C) -> + Cash = {100, <<"RUB">>}, + #{ + destination_id := DestinationID + } = prepare_standard_environment(Cash, C), + Party = create_party(C), + IdentityID = create_identity(Party, <<"good-two">>, <<"person">>, C), + WalletID = create_wallet(IdentityID, <<"My wallet">>, <<"RUB">>, C), + WithdrawalID = generate_id(), + WithdrawalParams = #{ + id => WithdrawalID, + destination_id => DestinationID, + wallet_id => WalletID, + body => {100, <<"RUB">>} + }, + Result = ff_withdrawal_machine:create(WithdrawalParams, ff_entity_context:new()), + ?assertMatch({error, {identity_providers_mismatch, {<<"good-two">>, <<"good-one">>}}}, Result). + -spec create_destination_resource_notfound_test(config()) -> test_return(). create_destination_resource_notfound_test(C) -> Cash = {100, <<"RUB">>}, diff --git a/apps/wapi/src/wapi_wallet_ff_backend.erl b/apps/wapi/src/wapi_wallet_ff_backend.erl index d74e0fc..110d225 100644 --- a/apps/wapi/src/wapi_wallet_ff_backend.erl +++ b/apps/wapi/src/wapi_wallet_ff_backend.erl @@ -389,6 +389,7 @@ create_destination(Params = #{<<"identity">> := IdenityId}, Context) -> {quote, {invalid_body, _}} | {quote, {invalid_destination, _}} | {terms, {terms_violation, _}} | + {identity_providers_mismatch, {ff_provider:id(), ff_provider:id()}} | {destination_resource, {bin_data, ff_bin_data:bin_data_error()}} | {Resource, {unauthorized, _}} ) when Resource :: wallet | destination. @@ -468,6 +469,7 @@ list_withdrawals(Params, Context) -> {destination, notfound} | {destination, unauthorized} | {route, _Reason} | + {identity_providers_mismatch, {ff_provider:id(), ff_provider:id()}} | {wallet, notfound} ). create_quote(#{'WithdrawalQuoteParams' := Params}, Context) -> diff --git a/apps/wapi/src/wapi_wallet_handler.erl b/apps/wapi/src/wapi_wallet_handler.erl index 74a2eb4..c995cbf 100644 --- a/apps/wapi/src/wapi_wallet_handler.erl +++ b/apps/wapi/src/wapi_wallet_handler.erl @@ -373,6 +373,10 @@ process_request('CreateWithdrawal', #{'WithdrawalParameters' := Params}, Context {error, {destination_resource, {bin_data, {unknown_residence, _Residence}}}} -> wapi_handler_utils:reply_ok(422, wapi_handler_utils:get_error_msg(<<"Unknown card issuer residence">>) + ); + {error, {identity_providers_mismatch, _}} -> + wapi_handler_utils:reply_ok(422, + wapi_handler_utils:get_error_msg(<<"This wallet and destination cannot be used together">>) ) end; process_request('GetWithdrawal', #{'withdrawalID' := WithdrawalId}, Context, _Opts) -> @@ -441,6 +445,12 @@ process_request('CreateQuote', Params, Context, _Opts) -> {error, {wallet, notfound}} -> wapi_handler_utils:reply_ok(422, wapi_handler_utils:get_error_msg(<<"Wallet not found">>) + ); + {error, {identity_providers_mismatch, _}} -> + wapi_handler_utils:reply_ok(422, + wapi_handler_utils:get_error_msg( + <<"This wallet and destination cannot be used together">> + ) ) end; diff --git a/apps/wapi/test/wapi_SUITE.erl b/apps/wapi/test/wapi_SUITE.erl index 4223c92..054a277 100644 --- a/apps/wapi/test/wapi_SUITE.erl +++ b/apps/wapi/test/wapi_SUITE.erl @@ -29,6 +29,7 @@ -export([get_wallet_by_external_id/1]). -export([check_withdrawal_limit_test/1]). -export([check_withdrawal_limit_exceeded_test/1]). +-export([identity_providers_mismatch_test/1]). -export([consume_eventsinks/1]). @@ -70,7 +71,8 @@ groups() -> {errors, [], [ not_allowed_currency_test, check_withdrawal_limit_test, - check_withdrawal_limit_exceeded_test + check_withdrawal_limit_exceeded_test, + identity_providers_mismatch_test ]}, {eventsink, [], [ consume_eventsinks @@ -346,6 +348,42 @@ check_withdrawal_limit_exceeded_test(C) -> } }}, get_withdrawal(WithdrawalID, C)). +-spec identity_providers_mismatch_test(config()) -> test_return(). + +identity_providers_mismatch_test(C) -> + Name = <<"Tony Dacota">>, + WalletProvider = ?ID_PROVIDER, + Class = ?ID_CLASS, + WalletIdentityID = create_identity(Name, WalletProvider, Class, C), + ok = check_identity(Name, WalletIdentityID, WalletProvider, Class, C), + WalletID = create_wallet(WalletIdentityID, C), + ok = check_wallet(WalletID, C), + CardToken = store_bank_card(C), + {ok, _Card} = get_bank_card(CardToken, C), + Resource = make_bank_card_resource(CardToken), + DestinationProvider = ?ID_PROVIDER2, + DestinationIdentityID = create_identity(Name, DestinationProvider, Class, C), + {ok, Dest} = create_destination(DestinationIdentityID, Resource, C), + DestID = destination_id(Dest), + ok = check_destination(DestinationIdentityID, DestID, Resource, C), + {ok, _Grants} = issue_destination_grants(DestID, C), + % ожидаем выполнения асинхронного вызова выдачи прав на вывод + await_destination(DestID), + + {error, {422, #{<<"message">> := <<"This wallet and destination cannot be used together">>}}} = call_api( + fun swag_client_wallet_withdrawals_api:create_withdrawal/3, + #{body => genlib_map:compact(#{ + <<"wallet">> => WalletID, + <<"destination">> => DestID, + <<"body">> => #{ + <<"amount">> => 100000, + <<"currency">> => <<"RUB">> + }, + <<"quoteToken">> => undefined + })}, + cfg(context, C) + ). + -spec unknown_withdrawal_test(config()) -> test_return(). unknown_withdrawal_test(C) -> diff --git a/rebar.lock b/rebar.lock index 1ff25d4..b75d47e 100644 --- a/rebar.lock +++ b/rebar.lock @@ -67,7 +67,7 @@ 0}, {<<"fistful_proto">>, {git,"git@github.com:rbkmoney/fistful-proto.git", - {ref,"ba8f0f2799d609b40d188a6fbaaac5e03143110a"}}, + {ref,"c93609c858f988e67016ac77a2947884a3cdae3f"}}, 0}, {<<"fistful_reporter_proto">>, {git,"git@github.com:rbkmoney/fistful-reporter-proto.git",