feat: CHK-3744 expose checkout authenticated ecommerce endpoint #2858

ciuffagianluca · 2025-03-04T22:52:17Z

Add ecommerce for checkout endpoints authorized via auth service token

List of changes

Add 4 endpoints:

post session
post transaction
get payment methods
get payment request

All these endpoints use auth token validate to authenticate api and discard recaptcha (where present)

Token validation is executed in the base policy.

The new apis, has the same schema for request and response, except for recaptcha in query param and the auth token in auth header. The new apis path, starts with auth/ prefix. This prefix is needed to avoid confusion with the twins endpoints of previous version for guest flux.

Motivation and context

This PR enable authenticated flux in checkout web portal

Type of changes

Add new resources
Update configuration to existing resources
Remove existing resources

Does this introduce a change to production resources with possible user impact?

Yes, users may be impacted applying this change
No

Does this introduce an unwanted change on infrastructure? Check terraform plan execution result

Yes
No

Other information

If PR is partially applied, why? (reserved to mantainers)

…erce-endpoint

…nt' of https://github.com/pagopa/pagopa-infra into CHK-3744-expose-checkout-authenticated-ecommerce-endpoint

src/domains/ecommerce-app/api/ecommerce-checkout/v3/_base_policy.xml.tpl

CianoDanilo · 2025-03-05T14:04:34Z

src/domains/ecommerce-app/api/ecommerce-checkout/v3/_base_policy.xml.tpl

+        </set-header>
+      </send-request>
+      <choose>
+        <when condition="@(((int)((IResponse)context.Variables["checkSessionResponse"]).StatusCode) != 200)">


Is there a reason we don't propagate downstream dependency status code here? If we get a 5xx we would give a 401, right?

In other authenticated flux, we reduce all errors in 401. I did the same here.
Yes, all code except 200 will return a 401.
Do you think it is better to return downstream dependency result code?

I was wondering if returning a 401 in case of a 5xx could be a problem for the frontend, we would show invalid token error page instead of generic error page, right?

Right, we will show an invalid token message in place of a generic error page.
It could be useful showing a generic error message, but it could be a little confusing for the user.
Is the payment api that results in 5xx or is the token validation?
Anyway I will add 4xx and 5xx status code but we should be able to manage this situation in a right way in checkout-fe

Done here 35b020a

…erce-endpoint

infantesimone · 2025-03-05T15:49:09Z

src/domains/ecommerce-app/api/ecommerce-checkout/v3/_methods_testing_policy.xml.tpl

+    <outbound>
+        <base />
+        <!-- APM test START -->
+        <set-variable name="allowedIps" value="PLACEHOLDER1,PLACEHOLDER2" />


maybe this (more or less :-) )feature flag by ip, is required for this version? We could add in a dedicated PR if needed

In order to manage dedicated payment method for restricted set of ips I paste this policy here. Anyway I'm going to remove it.

Done here cc468cd

infantesimone · 2025-03-05T15:50:45Z

src/domains/ecommerce-app/api/ecommerce-checkout/v3/_payment_request_policy.xml.tpl

+    <set-variable name="rptId" value="@{
+      return context.Request.MatchedParameters["rpt_id"];
+    }" />
+    <choose>


is it possible to move this logic in base policy? I think it is safe for all API because there is when condition

Should we have an rptId param in all request?

rptId is not present in all APIs, but with the when condition it is set only when present. This way, we avoid having a dedicated policy for a specific API, but we should consider the trade-off. It's an extra operation, although very light, but applied to all APIs.

So, it's a suggestion, we might not do it after all :-)

infantesimone · 2025-03-05T15:53:01Z

src/domains/ecommerce-app/api/ecommerce-checkout/v3/_post_sessions.xml.tpl

+<policies>
+    <inbound>
+    <rate-limit-by-key 
+    calls="150"


is it possible to move this logic in base policy? this rate limit is different considering base policy?

It was a misprint from copy/paste action. Removed here 8e7e6f7

infantesimone · 2025-03-05T15:55:14Z

src/domains/ecommerce-app/api/ecommerce-checkout/v3/_transaction_policy.xml.tpl

+        </choose>
+
+        <set-variable name="pdvToken" value="@(((IResponse)context.Variables["pdv-token"]).Body.As<JObject>())" />
+        <set-variable name="emailToken" value="@((string)((JObject)context.Variables["pdvToken"])["token"])" />


at this level, we add tokenization of fiscalCode and pass it to backend (as header x-user-id)?

Yes, should I do this in this pr or in a dedicated one?

Ok, it's a dedicated PR :-)

infantesimone · 2025-03-05T15:56:37Z

src/domains/ecommerce-app/04_apim_ecommerce_checkout.tf

+  api_name            = "${local.project}-ecommerce-checkout-api-v3"
+  api_management_name = local.pagopa_apim_name
+  resource_group_name = local.pagopa_apim_rg
+  operation_id        = "newTransaction"


Suggested change

operation_id = "newTransaction"

operation_id = "newTransactionV3"

in order to avoid conflict with operationId related v1, v2

Done here 8e7e6f7

infantesimone · 2025-03-05T15:56:44Z

src/domains/ecommerce-app/04_apim_ecommerce_checkout.tf

+  api_name            = "${local.project}-ecommerce-checkout-api-v3"
+  resource_group_name = local.pagopa_apim_rg
+  api_management_name = local.pagopa_apim_name
+  operation_id        = "createSession"


Done here 8e7e6f7

infantesimone · 2025-03-05T15:56:49Z

src/domains/ecommerce-app/04_apim_ecommerce_checkout.tf

+  api_name            = "${local.project}-ecommerce-checkout-api-v3"
+  resource_group_name = local.pagopa_apim_rg
+  api_management_name = local.pagopa_apim_name
+  operation_id        = "getAllPaymentMethods"


Done here 8e7e6f7

…and with counter key referring to the auth token

…ionId in openapi definition and in base policy clustering. Update readme

…erce-endpoint

github-actions · 2025-03-06T13:36:42Z

🎉 This PR is included in version 1.426.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

ciuffagianluca added 3 commits March 4, 2025 23:03

add api endpoint

37bd64a

add checkout ingress hostname

751f1e1

handle validate response in base policy

d0a244e

github-actions bot assigned ciuffagianluca Mar 4, 2025

ciuffagianluca and others added 8 commits March 4, 2025 23:53

fix comment

c19b18b

add auth prefix to all endpoint

f07b163

remove when condition for operationId service mapping

4d3b7f2

fix checking authorization start comment

7bcce68

remove recaptcha. fix validate url

b4aede0

fix header authorization to validate

02c27f7

fix base policy

20cb1af

Merge branch 'main' into CHK-3744-expose-checkout-authenticated-ecomm…

91d7c78

…erce-endpoint

ciuffagianluca changed the title ~~[CHK-3744] expose checkout authenticated ecommerce endpoint~~ (chore) CHK-3744: expose checkout authenticated ecommerce endpoint Mar 5, 2025

ciuffagianluca marked this pull request as ready for review March 5, 2025 11:22

ciuffagianluca requested review from a team as code owners March 5, 2025 11:22

ciuffagianluca changed the title ~~(chore) CHK-3744: expose checkout authenticated ecommerce endpoint~~ feat: CHK-3744: expose checkout authenticated ecommerce endpoint Mar 5, 2025

ciuffagianluca changed the title ~~feat: CHK-3744: expose checkout authenticated ecommerce endpoint~~ feat: CHK-3744 expose checkout authenticated ecommerce endpoint Mar 5, 2025

Merge branch 'main' into CHK-3744-expose-checkout-authenticated-ecomm…

36e89ac

…erce-endpoint

github-actions bot assigned infantesimone Mar 5, 2025

ciuffagianluca added 2 commits March 5, 2025 14:41

set backend service for post transactions

e1f2ad2

Merge branch 'CHK-3744-expose-checkout-authenticated-ecommerce-endpoi…

160f643

…nt' of https://github.com/pagopa/pagopa-infra into CHK-3744-expose-checkout-authenticated-ecommerce-endpoint

CianoDanilo reviewed Mar 5, 2025

View reviewed changes

src/domains/ecommerce-app/api/ecommerce-checkout/v3/_base_policy.xml.tpl Outdated Show resolved Hide resolved

CianoDanilo reviewed Mar 5, 2025

View reviewed changes

Merge branch 'main' into CHK-3744-expose-checkout-authenticated-ecomm…

5161945

…erce-endpoint

ciuffagianluca requested a review from CianoDanilo March 5, 2025 15:08

infantesimone reviewed Mar 5, 2025

View reviewed changes

ciuffagianluca added 5 commits March 5, 2025 17:07

add 401 error code result for all apis. Set rate limit counter lower …

09b2fbb

…and with counter key referring to the auth token

add validate error propagation

35b020a

fix counter key

148244d

remove post session rate limit

8e7e6f7

remove post session and get payment methods policy. Update all operat…

cc468cd

…ionId in openapi definition and in base policy clustering. Update readme

ciuffagianluca requested a review from infantesimone March 6, 2025 08:38

Merge branch 'main' into CHK-3744-expose-checkout-authenticated-ecomm…

851b793

…erce-endpoint

infantesimone previously approved these changes Mar 6, 2025

View reviewed changes

CianoDanilo previously approved these changes Mar 6, 2025

View reviewed changes

EmanueleBVtech previously approved these changes Mar 6, 2025

View reviewed changes

ciuffagianluca added 2 commits March 6, 2025 11:17

Merge branch 'main' into CHK-3744-expose-checkout-authenticated-ecomm…

8400ca6

…erce-endpoint

Merge branch 'main' into CHK-3744-expose-checkout-authenticated-ecomm…

96d856e

…erce-endpoint

ciuffagianluca dismissed stale reviews from infantesimone, CianoDanilo, and EmanueleBVtech via aeb2a57 March 6, 2025 13:28

remove domain name from api name

aeb2a57

pietro-tota approved these changes Mar 6, 2025

View reviewed changes

sfarralorenzo approved these changes Mar 6, 2025

View reviewed changes

ciuffagianluca merged commit 54f3bb5 into main Mar 6, 2025
5 of 6 checks passed

ciuffagianluca deleted the CHK-3744-expose-checkout-authenticated-ecommerce-endpoint branch March 6, 2025 13:36

github-actions bot added the released label Mar 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: CHK-3744 expose checkout authenticated ecommerce endpoint #2858

feat: CHK-3744 expose checkout authenticated ecommerce endpoint #2858

ciuffagianluca commented Mar 4, 2025 •

edited

Loading

CianoDanilo Mar 5, 2025

ciuffagianluca Mar 5, 2025

CianoDanilo Mar 5, 2025

ciuffagianluca Mar 5, 2025

ciuffagianluca Mar 5, 2025

infantesimone Mar 5, 2025

ciuffagianluca Mar 6, 2025 •

edited

Loading

ciuffagianluca Mar 6, 2025

infantesimone Mar 5, 2025

ciuffagianluca Mar 6, 2025

infantesimone Mar 6, 2025

infantesimone Mar 6, 2025

infantesimone Mar 5, 2025

ciuffagianluca Mar 6, 2025

infantesimone Mar 5, 2025

ciuffagianluca Mar 6, 2025

infantesimone Mar 6, 2025

infantesimone Mar 5, 2025

ciuffagianluca Mar 6, 2025

infantesimone Mar 5, 2025

ciuffagianluca Mar 6, 2025

infantesimone Mar 5, 2025

ciuffagianluca Mar 6, 2025

github-actions bot commented Mar 6, 2025

	operation_id = "newTransaction"
	operation_id = "newTransactionV3"

feat: CHK-3744 expose checkout authenticated ecommerce endpoint #2858

feat: CHK-3744 expose checkout authenticated ecommerce endpoint #2858

Conversation

ciuffagianluca commented Mar 4, 2025 • edited Loading

List of changes

Motivation and context

Type of changes

Does this introduce a change to production resources with possible user impact?

Does this introduce an unwanted change on infrastructure? Check terraform plan execution result

Other information

If PR is partially applied, why? (reserved to mantainers)

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

ciuffagianluca Mar 6, 2025 • edited Loading

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

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

github-actions bot commented Mar 6, 2025

ciuffagianluca commented Mar 4, 2025 •

edited

Loading

ciuffagianluca Mar 6, 2025 •

edited

Loading