[PAYOP-1116] Implement Larky function for rendering network token #326

fangpenlin · 2023-03-14T05:11:24Z

Fixes PAYOP-1116

Description of changes in release / Impact of release:

Implement Larky function for rendering network token

Documentation

Created a new network token module with service interface to be implemented later.

Risks of this release

The default module service either NoopNetworkTokenService or MockNetworkTokenService basically does nothing so shouldn't introduce any risk.

Is this a breaking change?

Yes
No

Is there a way to disable the change?

Use previous release
Use a feature flag
No

larky/pom.xml

Iapetus999 · 2023-03-14T14:31:01Z

larky/src/main/java/com/verygood/security/larky/modules/vgs/nts/LarkyNetworkToken.java

+   * @param thread Starlark thread object
+   * @return
+   */
+  Object render(


All this Object makes me cringe

I don't like passing Objects around, either. But cannot think of a better way to deal with it for now, open to suggestions 🙏

Iapetus999 · 2023-03-14T20:01:48Z

larky/src/test/resources/vgs_tests/nts/test_default_nts.star

+        pan="$.paymentMethod.number",
+        exp_month="$.paymentMethod.expiryMonth",
+        exp_year="$.paymentMethod.expiryYear",
+        cryptogram_value="$.mpiData.cavv",


seems like string here

mjallday

looks great! @mottersheadt to review the test please

mjallday · 2023-03-14T20:27:17Z

larky/src/main/java/com/verygood/security/larky/modules/NetworkTokenModule.java

+            named = true,
+            defaultValue = "None",
+            doc =
+                "JSONPath to insert the expire year from network token to the input payload and return",


Suggested change

"JSONPath to insert the expire year from network token to the input payload and return",

"JSONPath to insert the expiration year of the network token within the input payload",

mjallday · 2023-03-14T20:28:07Z

larky/src/test/resources/vgs_tests/nts/test_default_nts.star

+        "shopperReference": "YOUR_SHOPPER_REFERENCE",
+        "recurringProcessingModel": "CardOnFile",
+        "shopperInteraction": "Ecommerce",
+        # Deep JSONPath is not supported by the JSONPath lib, so that we need to


@liddellc need to ensure we document this

mjallday · 2023-03-14T20:30:34Z

larky/src/test/resources/vgs_tests/nts/test_default_nts.star

+load("@vgs//nts", "nts")
+
+
+def _test_render():


fantastic test. nice job @fangpenlin

mjallday · 2023-03-14T20:31:22Z

larky/src/test/resources/vgs_tests/nts/test_default_nts.star

+            lambda: nts.render(input, "$.pan", **{key: "$.not_exists"}),
+            (
+                    'JSONPath "\\$\\.not_exists" does not exist, if you want to insert value to a nested path, please ensure that the ' +
+                    "target value already exists at the given path in the input")


"please ensure that an empty element..."? may read better

mjallday · 2023-03-14T20:31:31Z

larky/src/test/resources/vgs_tests/nts/test_default_nts.star

+    input = {
+        "pan": "NOT_FOUND",
+    }
+    asserts.assert_fails(lambda: nts.render(input, "$.pan"), "network token does not found")


Suggested change

asserts.assert_fails(lambda: nts.render(input, "$.pan"), "network token does not found")

asserts.assert_fails(lambda: nts.render(input, "$.pan"), "network token is not found")

mjallday · 2023-03-14T20:34:29Z

larky/src/main/java/com/verygood/security/larky/modules/NetworkTokenModule.java

+            name = "pan",
+            named = true,
+            doc =
+                "JSONPath to the PAN alias in the input payload for looking up the corresponding network token to be used and replace the original value at given JSONPath",


Suggested change

"JSONPath to the PAN alias in the input payload for looking up the corresponding network token to be used and replace the original value at given JSONPath",

"JSONPath to the PAN alias in the input payload. Used to look up the corresponding network token to be rendered and injected into the payload",

mjallday · 2023-03-14T20:34:59Z

larky/src/main/java/com/verygood/security/larky/modules/NetworkTokenModule.java

+            named = true,
+            defaultValue = "None",
+            doc =
+                "JSONPath to insert the expire month from network token to the input payload and return",


Suggested change

"JSONPath to insert the expire month from network token to the input payload and return",

"JSONPath to insert the expiration month of the network token within the input payload",

mjallday · 2023-03-14T20:35:21Z

larky/src/main/java/com/verygood/security/larky/modules/NetworkTokenModule.java

+            named = true,
+            defaultValue = "None",
+            doc =
+                "JSONPath to insert the cryptogram value from network token to the input payload and return",


Suggested change

"JSONPath to insert the cryptogram value from network token to the input payload and return",

"JSONPath to insert the cryptogram value of the network token within the input payload",

mjallday · 2023-03-14T20:35:39Z

larky/src/main/java/com/verygood/security/larky/modules/NetworkTokenModule.java

+            named = true,
+            defaultValue = "None",
+            doc =
+                "JSONPath to insert the cryptogram eci from network token to the input payload and return",


Suggested change

"JSONPath to insert the cryptogram eci from network token to the input payload and return",

"JSONPath to insert the cryptogram ECI of the network token within the input payload",

mjallday · 2023-03-14T20:36:02Z

larky/src/main/java/com/verygood/security/larky/modules/NetworkTokenModule.java

+                  throw new RuntimeException(
+                      Starlark.errorf(
+                          "JSONPath \"%s\" does not exist, if you want to insert value to a nested path, please ensure "
+                              + "that the target value already exists at the given path in the input",


Suggested change

+ "that the target value already exists at the given path in the input",

+ "that an empty value already exists at the given path in the input",

fangpenlin · 2023-03-14T20:57:54Z

@mjallday FYI, I realized that Mahmoud tried pretty hard to reduce the dependencies of this project and keep the cpu and memory footprint low. I also realized there was a security bug in the json path before

json-path/JsonPath#687

There's a pure larky json path to be used as well:

https://github.com/verygoodsecurity/starlarky/blob/master/larky/src/main/resources/vendor/jsonpath_ng.star

I wonder maybe it would be way more secure to keep the java module as simple as possible and leave all the json path stuff in larky land. With that idea in mind I created an alt implemented here:

#327

the interface reminds the same

mottersheadt · 2023-03-16T16:33:49Z

larky/src/test/resources/vgs_tests/nts/test_default_nts.star

+        }
+    }
+
+    output = nts.render(


This is great, but I have some thoughts.

Can we return an object that the Larky script can use to input the values itself, rather than relying on the templatized rendering method? For example:

network_token_data = nts.render(input['paymentMethod']['number']) input['mpiData']['cavv'] = network_token_data.cryptogram input['mpiData']['eci'] = network_token_data.eci input['paymentMethod']['expiryYear'] = network_token_data.expiry_year input['paymentMethod']['expiryMonth'] = network_token_data.expiry_month input['paymentMethod']['number'] = network_token_data.DPAN

With the current approach, I'm worried about any complex integrations. I'm not sure how this approach would work with form data, XML, graphql, etc... By providing the data right to the integration, I think the integration level code will be easier to implement in each circumstance.

If we do stick with the templatized rendering approach, we need to ensure that the end-user doesn't need to send the "TO_BE_REPLACED" fields. If the field doesn't exist, I would expect it to be auto-created. Again need to account for how this would work with a graphql API (which I recently had to work with, hence being top of mind :) )

hey, here it is an alternative version #327

fangpenlin · 2023-03-20T16:35:03Z

close in favor of #327 instead

fangpenlin added 11 commits March 13, 2023 10:58

Add code provided by salava

2e43457

Interface

29cb317

Rename module

f71a231

Implement json path stuff

7a69632

Add token module stuff

c6c4e7a

Add tests

538dcb3

More test cases

909e2b7

Add NT not found test

bd1b26a

Remove unused stuff

dec1910

Handle all the corner cases

461e015

Make member private

10880b8

fangpenlin changed the title ~~WIP [PAYOP-1116] Implement Larky function for rendering network token~~ [PAYOP-1116] Implement Larky function for rendering network token Mar 14, 2023

viacheslav-fomin-main approved these changes Mar 14, 2023

View reviewed changes

Iapetus999 reviewed Mar 14, 2023

View reviewed changes

larky/pom.xml Outdated Show resolved Hide resolved

Iapetus999 reviewed Mar 14, 2023

View reviewed changes

Extract json path version

4928cf3

fangpenlin mentioned this pull request Mar 14, 2023

[PAYOP-1116] Implement Larky function for rendering network token (ALT implementation) #327

Merged

5 tasks

Iapetus999 reviewed Mar 14, 2023

View reviewed changes

mjallday reviewed Mar 14, 2023

View reviewed changes

mjallday requested a review from mottersheadt March 14, 2023 20:36

mottersheadt suggested changes Mar 16, 2023

View reviewed changes

fangpenlin closed this Mar 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PAYOP-1116] Implement Larky function for rendering network token #326

[PAYOP-1116] Implement Larky function for rendering network token #326

fangpenlin commented Mar 14, 2023 •

edited by jira bot

Loading

Iapetus999 Mar 14, 2023

fangpenlin Mar 14, 2023

Iapetus999 Mar 14, 2023

Iapetus999 Mar 14, 2023

mjallday left a comment

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

mjallday Mar 14, 2023

fangpenlin commented Mar 14, 2023

mottersheadt Mar 16, 2023

viacheslav-fomin-main Mar 16, 2023

fangpenlin commented Mar 20, 2023

	"JSONPath to insert the expire year from network token to the input payload and return",
	"JSONPath to insert the expiration year of the network token within the input payload",

	asserts.assert_fails(lambda: nts.render(input, "$.pan"), "network token does not found")
	asserts.assert_fails(lambda: nts.render(input, "$.pan"), "network token is not found")

	"JSONPath to the PAN alias in the input payload for looking up the corresponding network token to be used and replace the original value at given JSONPath",
	"JSONPath to the PAN alias in the input payload. Used to look up the corresponding network token to be rendered and injected into the payload",

	"JSONPath to insert the expire month from network token to the input payload and return",
	"JSONPath to insert the expiration month of the network token within the input payload",

	"JSONPath to insert the cryptogram value from network token to the input payload and return",
	"JSONPath to insert the cryptogram value of the network token within the input payload",

	"JSONPath to insert the cryptogram eci from network token to the input payload and return",
	"JSONPath to insert the cryptogram ECI of the network token within the input payload",

	+ "that the target value already exists at the given path in the input",
	+ "that an empty value already exists at the given path in the input",

		load("@vgs//nts", "nts")


		def _test_render():

[PAYOP-1116] Implement Larky function for rendering network token #326

[PAYOP-1116] Implement Larky function for rendering network token #326

Conversation

fangpenlin commented Mar 14, 2023 • edited by jira bot Loading

Fixes PAYOP-1116

Description of changes in release / Impact of release:

Documentation

Risks of this release

Is this a breaking change?

Is there a way to disable the change?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjallday left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fangpenlin commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fangpenlin commented Mar 20, 2023

fangpenlin commented Mar 14, 2023 •

edited by jira bot

Loading