Description

Handle expiry for payment_token in make_pm_data

Description

Fix hard coded values in payment_methods/transformers.rs.

Fields

• merchant_id
• email address
• name on card
• nickname

Description

Refactor webhooks core to add retry logic in webhooks core.

Currently, we store information about whether we have sent a webhook or not, improving on this we can implement retry logic to make this more reliable

Payment Flows

• Authorize
  • Automatic #154
  • Manual
• Payment Sync
• Void
• Refund
• Refund Sync
• Redirect Flow
• Webhooks

Payment Methods

• Card

Authentication

• Add support for a way to get 3 values from keys.conf for http-signature support #69

Miscellaneous

• Tests
• Error Handling with error codes

I strongly advise to add this warning. #![warn(missing_docs)]

Model the KV storage flow as a series of common operations (For example, set in redis -> get sql query -> push query to redis stream) and define generic utilities that perform these operations for all KV-enabled storage interfaces (as opposed to rewriting code).

locker_key_identifier1 and locker_key_identifier2

1. Why this is not a Secret as in other places?
   https://github.com/juspay/orca/blob/dff8b22489e36058326bee36b2c91014cdb4c9f2/crates/router/src/connector/adyen/transformers.rs#L159
   https://github.com/juspay/orca/blob/dff8b22489e36058326bee36b2c91014cdb4c9f2/crates/router/src/connector/authorizedotnet/transformers.rs#L44-L46

https://github.com/juspay/orca/blob/dff8b22489e36058326bee36b2c91014cdb4c9f2/crates/router/src/types.rs#L240-L251

2. The secrets here (and in similar places below) are cloned into the unprotected memory, and, thus, not properly zeroized on Drop. Furthermore, .clone() produces a redundant allocation here. It shouldn't be used, as the format!() macro bellow may easily accept a reference.
   https://github.com/juspay/orca/blob/dff8b22489e36058326bee36b2c91014cdb4c9f2/crates/router/src/connector/authorizedotnet/transformers.rs#L137-L138

https://github.com/juspay/orca/blob/01cafe753bc4ea0cf33ec604dcbe0017abfebcad/crates/common_utils/src/pii.rs#L68

We should assume constructed value object is already validated. Email validation is quite a heavy operation. Doing it on each formatting is quite a subtle performance penalty, while being... unnecessary?
Another problem, that validation here is violation of the "Separation of concerns" design principle. Formatting is not a validation in any way. https://en.wikipedia.org/wiki/Separation_of_concerns Consider to provide a newtype for email strings. https://rust-unofficial.github.io/patterns/patterns/behavioural/newtype.html

This way you do the validation only once, when creating a value of the type, and then you may fearlessly reuse it as the type system protects you. https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate
Thus, as the result, you will be able to remove any validation code from the formatting, as the compiler will guarantee that you would have valid values here. The same is true for other formatting strategies in this module too, as they're effectively validators too.

https://github.com/juspay/orca/blob/56d153d8f7c2c75391799cf14280c448df97842f/crates/router/src/connector/adyen/transformers.rs#L309

CVC shouldn't be saved in our db Will need to implement tokenization that allows us to make payments without cvv
And this could be easily prevented if it was represented as a newtype for value object which doesn't implement ToSql.

possible race condition here. what if a concurrent user updates parent merchant between this operation and the following?
https://github.com/juspay/orca/blob/56d153d8f7c2c75391799cf14280c448df97842f/crates/router/src/core/admin.rs#L74-L79

possible race condition here. what if a concurrent user updates merchant account between this read and the last write?
https://github.com/juspay/orca/blob/56d153d8f7c2c75391799cf14280c448df97842f/crates/router/src/core/admin.rs#L148-L153

possible race condition here. what if a concurrent user updates parent merchant between this operation and the following?
https://github.com/juspay/orca/blob/56d153d8f7c2c75391799cf14280c448df97842f/crates/router/src/core/admin.rs#L205-L212

What if the application process would be killed after successful execution of delete_payment_method_by_merchant_id_payment_method_id(), but before executing delete_card()? Would be the card ever deleted? Seems like the system has weak consistency guarantees.

https://docs.diesel.rs/diesel/connection/trait.Connection.html#method.transaction

https://github.com/juspay/orca/blob/56d153d8f7c2c75391799cf14280c448df97842f/crates/router/src/core/payment_methods/cards.rs#L430-L434

Rename the struct PaymentsRequestSyncData to PaymentsSyncData.
Use this pattern <Feature>Data.

And rename PaymentsRouterSyncData struct to PaymentsSyncRouterData.
Pattern - <Feature>RouterData

And rename the field name called request to data in RouterData struct.

Originally posted by @jarnura in #7 (comment)

https://github.com/juspay/orca/blob/56d153d8f7c2c75391799cf14280c448df97842f/crates/router/src/connector/authorizedotnet/transformers.rs#L219-L227

Instead of such renaming, it would be more properly to use #[repr(u8)] here:

#[derive(Serialize_repr, Deserialize_repr)]
#[repr(u8)]
pub enum AuthorizedotnetPaymentStatus {
   Approved = 1,
   Declined = 2,
   Error = 3,
   #[default]
   HeldForReview = 4,
}

https://serde.rs/enum-number.html

The Open API specification needs to be updated to sync with:

• newly implemented APIs such as /session and /verify
• the addition/ changes in parameters of existing APIs shall also be modified in the specifications
• payment status fields, description and significance
• documentation of all possible API errors, explanation and significance
• documentation of all webhook events with explanation
• include the health check endpoint (/health) in the spec

Description

This issue's primary focus lies in implementing the pending interface functionality for MockDb. The purpose of this change is to have a simple and effective mechanism for testing without connecting to any outside entity for the storage need. The MockDb will come in play for unit tests, integration tests, etc.

Interfaces to implement

• #954
• #996
• #1189
• #997
• Implement ConnectorResponseInterface for MockDb (partially complete)
• Implement CustomerInterface for MockDb (partially complete)
• #1190
• #1191
• #1116
• #1257
• #1256
• #2438
• #1123
• #1329
• Implement PaymentIntentInterface for MockDb (partially complete)
• #1491
• #4150
• Implement ProcessTrackerInterface for MockDb (partially complete)
• #1490
• #1276
• #1115
• #1697
• #2039
• Implement BatchSampleDataInterface for MockDb
• #3044

If you're interested in picking up any item from the list, please let us know. We'll spin out an issue and assign it to you.

Allow ConnectorIntegration::get_headers() to accept connectors parameter so that we can set Host headers.

requirement came for Cybersource #154

Payment Flows

• Authorize
  • Automatic #30
  • Manual
• Payment Sync #30 #73
• Void #73
• Refund #73
  • Partial Refund
• Refund Sync
• Redirect Flow
• Webhooks

Payment Methods

• Card #30
• Wallet

Miscellaneous

• Tests
• Error Handling with error codes
  • Reverse engineer response and find Error Code
  • Map Braintree error codes and send appropriate messages

PR #142 included support for INSERT and UPDATE SQL queries, but support for DELETE queries is pending. Implement it for PaymentAttempt, PaymentIntent and Refund storage types.

GlobalPayments and Paypal are to connectors that require tokens to make any call to their Api's.
These tokens have an expiry as well.

Need feature to get the token for any of the connector flows to proceed with the payment.

Description

Implement capture for payments that are captured manually

Connectors to implement

• ACI
• Authorizedotnet
• Adyen #261
• Braintree
• Checkout #259

Set up clippy to warn on clippy::use_self and address the lints thrown.

(Originated from #190 (comment))

Consider to mark any new code/items, generated by procedural macros, with the #[automatically_derived] attribute. This makes code style linters to omit the generated code and doesn't report redundant warnings.

Example: https://github.com/cucumber-rs/cucumber/blob/v0.15.1/codegen/src/world.rs

Description

Currently in the stripe compatibility layer all the error codes defined are limited and are the one's that we directly need to map with stripe. Though moving forward as the compatibility layer continues to develop we might need to keep adding new error codes from the stripe documentation.

• stripe error codes: https://stripe.com/docs/error-codes
• This change is intended for the errors.rs file in the compatibility module.

This is to educate PR authors to look for config files at different places if their changes affect them.

https://github.com/juspay/orca/blob/dddc9eaf1430adcd1293933e726c2f4f296c805b/crates/router/src/types/transformers.rs#L5-L14

Why not just use frunk?

Here's an example:

use frunk::LabelledGeneric;

#[derive(LabelledGeneric)]
enum ApiRoutingAlgorithm {
    RoundRobin,
    MaxConversion,
    MinCost,
    Custom,
}

#[derive(LabelledGeneric, PartialEq, Debug)]
enum RoutingAlgorithm {
    RoundRobin,
    MaxConversion,
    MinCost,
    Custom,
}

fn main() {
    let api_algorithm = ApiRoutingAlgorithm::MinCost;
    let algorithm: RoutingAlgorithm = frunk::labelled_convert_from(api_algorithm);

    assert_eq!(algorithm, RoutingAlgorithm::MinCost);
}

Usual comparison is not safe in cryptography as is lazy (fail-fast) and makes the code potentially vulnerable to timing attacks: https://www.chosenplaintext.ca/articles/beginners-guide-constant-time-cryptography.html.
Use crates like subtle for constant-time comparison of secret values: https://docs.rs/subtle.

https://github.com/juspay/orca/blob/dff8b22489e36058326bee36b2c91014cdb4c9f2/crates/masking/src/strong_secret.rs#L54-L62

Description

Implement mandate data fetching functionality for connectors.

Connectors to implement for

• Adyen
• Checkout
• Braintree
• Authorizedotnet
• ACI

Proc macro should not panic. When proc macro panics, it will be the only error message returned by the rust compiler. Instead, in case of errors, proc macro should emit compile_error!() to communicate error to the user. This will allow other errors to be displayed.

https://github.com/juspay/orca/blob/01cafe753bc4ea0cf33ec604dcbe0017abfebcad/crates/router_derive/src/macros/operation.rs#L92

Description

Propogate Some hard coded fields in PaymentMethodResponse

Fields

• recurring_enabled
• installment_payment_enabled
• payment_experience

Nix support for orca:

• Dev shell to include all the required tools, setup
• Build with nix flakes
• CI/CD with nix flakes

Description

Implement Verify flow for the connectors. Verify flow makes sure if the card is valid or not for future payments like mandate etc.

Connectors to implement

• Authorizedotnet
• Checkout
• ACI
• #1068
• #1045

Currently we support max 2 values from keys.conf / db

pub enum ConnectorAuthType {
    HeaderKey { api_key: String },
    BodyKey { api_key: String, key1: String },
}

I'm trying to integrate Cybersource, it requires http signatures.

I will need 3 values

• Merchant Id
• Api Key
• Api Secret

With these I'm able to generate the http signature headers, like

digest: "SHA-256=cwjLNSMNo0IFp7hbUtTNu+7KxaF9O67ydqKWMnQ7J5g="
signature: 'keyid="5476633e-eff2-4e65-9834-58081207dd61", algorithm="HmacSHA256", headers="host (request-target) digest v-c-merchant-id", signature="djnWLdaLRh8xtWLCXxGIlavyRG4jBvB7gIzUWTKzPoQ="'

So I'm proposing

pub enum ConnectorAuthType {
    HeaderKey { api_key: String },
    BodyKey { api_key: String, key1: String },
    SignatureKey { api_key: String, key1: String, api_secret: String },
}

@SanchithHegde @Narayanbhat166 @jarnura

I can add the PR for it.

referring #58

This issue serves an extension of #9; the following pieces of information are yet to be added to the contributing guidelines:

• Tests
• Versioning policy
• Release process
• Links to Discord/Slack communities #310
  • Also add them to issue template config.yml
• Process to bump MSRV (Optional)

I'll briefly jot down what I feel are necessary with respect to each of the items.

Tests

• Specify what type of changes require tests. Example: new features, bug fixes, connector integrations, etc.
• Specify where to place unit/integration/documentation/API tests. (Of course documentation tests would be alongside documentation.)
• Specify how to run tests, especially when external tools are involved (anything other than cargo).

Versioning Policy

• It would be better if we can provide certain guarantees to the users of our APIs, that our APIs won't change behavior between certain types of releases.
• Specify what changes can be expected between major, minor and patch releases.
• If possible, try to follow SemVer strictly.

Release Process

• This is targeted primarily at maintainers, but aims to serve as a checklist to refer to before publishing releases.
• List out each of the steps in detail. These could include:
  • Bumping up versions
  • Updating Cargo metadata
  • Updating links on documentation pages
  • Updating changelog files
  • Review changes
  • Open PR
  • Release

Now that I think of the release process, I'm wondering if we should also document the steps to take when bumping up MSRV.

Links to Discord/Slack communities

• As of now, users can use the GitHub Discussions space on our repository to interact with the project maintainers. But a more organized space is necessary; two of the potential candidates are a Discord server and a Slack workspace.
• Both of these spaces exist as of creating this issue, but require a bit of "initial setup" before people can be invited in there.

Discovered while running payment-confirm loadtesting script.

Log:

{
  ...
  "flow":"PaymentsCreate",
  "extra":{
    "api_authentication":"ApiKey",
    "error":"{\"error\":{\"type\":\"server_not_available\",\"code\":\"RE_00\",\"message\":\"Something went wrong.\"}}\n├╴crates/router/src/core/payments.rs:96:10\n│\n├─▶ DatabaseError: A unique constraint violation occurred\n│   ├╴crates/router/src/types/storage/query/generics.rs:168:22\n│   ╰╴Error while inserting CustomerNew { customer_id: \"StripeCustomer\", merchant_id: \"merchant_1669119076606\", name: Some(\"John Doe\"), email: Some(*****@example.com), phone: Some(*** alloc::string::String ***), description: None, phone_country_code: Some(\"+65\"), address: None, metadata: None }\n│\n╰─▶ Failed to issue a query: duplicate key value violates unique constraint \"customers_pkey\"\n    ╰╴crates/router/src/types/storage/query/generics.rs:167:27",
    "http.client_ip":"192.168.48.10",
    "http.flavor":"1.1",
    "http.host":"router-server:8080",
    "http.method":"POST",
    "http.route":"/payments"
    ...
  }
}

Steps to reproduce:
check out rnd-loadtest-failing-requests branch
I made a minor change which gives us access to logs of the router container.
once you checked out the branch:

cd loadtest
docker compose build # will take some time
bash loadtest.sh -s payment-confirm

the last command will give you something like http_req_failed................: 0.18% ✓ 7 ✗ 3718 in the end. The numbers could be different. Those 7 requests are failed.

You can access the logs stored at loadtest/logs.tmp/logs. Search for "server_not_available" string in it.

Notes:

1. Those customer queries need to be in a single transaction but currently we are not using pgdb as a relational storage so that would not be a good solution.

Possible solutions:

1. We implement api level locking in future, that will handle parallel request errors. (cc @jarnura)

Visible issue:

• There are a few cases on response construction and using fields from router_data.response while building request, which is logically invalid.

Changes required:

• Check inconsistencies and make the connector request construction purely dependent on the router_data and router_data.request
• Constructing the router_data.response only after calling the connector

Here and in any other crate roots it's better to declare #![forbid(unsafe_code)] as we do in masking crate. This will ease a life of readers and auditors a lot, and will require quite a reasoning for those who will intend to contribute any unsafe code.

Description

Implement token flow for other payment methods currently it's only supported for cards

Use here, below, and in similar places, theformat_args!() macro instead, to omit redundant allocations produced by the format!() macro (as it returns a new String).

https://github.com/juspay/orca/blob/01cafe753bc4ea0cf33ec604dcbe0017abfebcad/crates/router_env/src/logger/formatter.rs#L207
https://github.com/juspay/orca/blob/01cafe753bc4ea0cf33ec604dcbe0017abfebcad/crates/router_env/src/logger/formatter.rs#L213

Redundant allocation on heap. This code produces the redundant String allocation as the result of format!() macro call. Omit it by writing into the formatter directly with write!() macro:

::core::write!(f, "{:?}", self)

https://github.com/juspay/orca/blob/01cafe753bc4ea0cf33ec604dcbe0017abfebcad/crates/router_derive/src/macros.rs#L23

This code contains redundant heap allocation .to_string() allocations, because we can write directly into the formatter:

write!(f, "{}", serde_json::json!({"error": &self.err }))

https://github.com/juspay/orca/blob/01cafe753bc4ea0cf33ec604dcbe0017abfebcad/crates/router/src/utils.rs#L37-L42

Checklist

• #1293
• #1294

Since we will be integrating different payment methods, they might have varying requirements with respect to the api calls, request and response structs. For calling connectors to support these, the response struct must also need to be generic and dependent on the flow. Currently RouterData supports only PaymentsResponseData.

@jarnura @NishantJoshi00

Once the session is complete we need delete the entry from basilisk db