Skip to content
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.

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

feat(connector): implement authentication flow for netcetera #4334

Merged
merged 31 commits into from
Apr 23, 2024

Conversation

hrithikesh026
Copy link
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Implemented Authentication flow for netcetera authentication connector.

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@hrithikesh026 hrithikesh026 added A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Apr 10, 2024
@hrithikesh026 hrithikesh026 added this to the April 2024 milestone Apr 10, 2024
@hrithikesh026 hrithikesh026 self-assigned this Apr 10, 2024
@hrithikesh026 hrithikesh026 requested review from a team as code owners April 10, 2024 09:22
Base automatically changed from integrate-netcetera to main April 16, 2024 10:38
@hrithikesh026 hrithikesh026 requested a review from a team as a code owner April 16, 2024 10:38
crates/router/src/connector/netcetera.rs Show resolved Hide resolved
},
))
}
_ => types::authentication::AuthNFlowType::Frictionless,
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this default so that in future type safe errors can be detected at compile time

three_ds_requestor_authentication_info: Some(
netcetera_types::SingleOrListElement::new_single(
netcetera_types::ThreeDSRequestorAuthenticationInformation {
three_ds_req_auth_method: "01".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to change this field into an enum?

),
)?,
),
recurring_expiry: Some("20240401".to_string()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this hardcoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a temporary solution. Core is not ending these values currently but netcetera requires these fields to be sent. Core changes to send these info in router_data.request will be done in near future.

Comment on lines +1989 to +1991
_ => Err(errors::ConnectorError::NotSupported {
message: SELECTED_PAYMENT_METHOD.to_string(),
connector: connector_name,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a scope in future to add other payment methods into the current implemented flows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. external 3ds is only available for cards.

SamraatBansal
SamraatBansal previously approved these changes Apr 18, 2024
Copy link
Contributor

@SamraatBansal SamraatBansal left a comment

Choose a reason for hiding this comment

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

Approving from a code structure and practices view, @sai-harsha-vardhan please verify the logical sanity.

@ArjunKarthik
Copy link
Contributor

@hrithikesh026 I have reviewed the final commit 2174ec2. Looks good to me

@@ -85,6 +85,7 @@ zen.base_url = "https://api.zen.com/"
zen.secondary_base_url = "https://secure.zen.com/"
zsl.base_url = "https://api.sitoffalb.net/"
threedsecureio.base_url = "https://service.3dsecure.io"
netcetera.base_url = "https://{{merchant_endpoint_prefix}}.3ds-server.prev.netcetera-cloud-payment.ch"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we raise a query, if this would be the same URL prev for PROD too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If netcetera.base_url is missing in production.toml, it would cause production deployment to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will just be a temporary fix to avoid production deployment failure.

@likhinbopanna likhinbopanna added this pull request to the merge queue Apr 23, 2024
Merged via the queue into main with commit 5ce0535 Apr 23, 2024
10 of 12 checks passed
@likhinbopanna likhinbopanna deleted the implement-authentication-flow-netcetera branch April 23, 2024 10:43
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants