cancel
Showing results for 
Search instead for 
Did you mean: 

Issues with autobatched requests and x-csrf-tokens

carlonnheim
Participant
0 Kudos

Hi,

I am trying to upgrade our project from CDS 5.9.5 to 6.1.3. I am getting issues with the new handling of csrf tokens and autobatched requests.

It is "libx/_runtim/remote/utils/client.js" which used to have this

  if (PPPD[requestConfig.method] && cds.env.features.fetch_csrf) {
    requestOptions = { fetchCsrfToken: true }
  }

and now has this

  if (PPPD[requestConfig.method]) {
    // For GET requests, one doesn't need to fetch CSRF tokens.
    // Once we support batch requests (other than autoBatched GET requests),
    // we must check the respective subrequests.
    const csrfRequired = requestConfig._autoBatch ? false : cds.env.features.fetch_csrf === true
    requestOptions = { fetchCsrfToken: csrfRequired }
  }

The endpoint we are accessing is SAP Business ByDesign which does not accept a $batch POST without x-csrf-token even though the actual contained request is a single GET.

Workaround for now is to set the "max_get_url_length" parameter of the service large enough to avoid the autobatched requests. This works so far, but is not a proper solution either.

Can we get an improved way to cope with this? If I may suggest, I think the csrf-related options should sit at service level, not as a global feature flag, and allow for the different flavors other systems adhere to. Otherwise we get into issues whenever we mix remote services with different policies.

Thanks in advance!

//Carl

gregorw
Active Contributor

You mean @sap/cds 6.1.3 or?

carlonnheim
Participant
0 Kudos

Yes! Thanks!

Accepted Solutions (1)

Accepted Solutions (1)

OlenaT
Advisor
Advisor
0 Kudos

Hi onnheimc ,

We are introducing two new configuration options: csrf: true/false and csrfInBatch: true/false which will allow you to configure csrf token handling pro service. Please track our release notes as David mentioned. Global env variable cds.env.features.fetch_csrf will be deprecated.

Regarding your problem with an action call: We have created a task for this, but there is no timeline on it yet. One of our colleagues will get back to you after the task is completed.

Best regards,

Olena

Answers (1)

Answers (1)

david_kunz2
Advisor
Advisor
0 Kudos

Hi onnheimc ,

We're currently working on an option to enable the propagation of the csrf header also for auto-batched requests.
Please refer to https://cap.cloud.sap/docs/releases/ for updates on this topic.

Thanks and best regards,
David

carlonnheim
Participant

Sounds great david.kunz2! Any thoughts on enabling per-service configuration of csrf handling? This would be really helpful. In a current project we have a mix of remote systems with characteristics:

  • Demanding csrf-tokens for all POST requests (including $batch with only GET calls - the one prompting this question)
  • Not requiring csrf's, but also not complaining when we try to get them (does waste roundtrip time though)
  • Not requiring csrf's and failing if we try to get them. That app (non-SAP) does not implement "HEAD" requests and stalls if we send them

In this scenario it would be really helpful to control the csrf-behavior per service. We now work around that by flipping the global flag before and after service calls (which is actually non-trivial when different apps asynchronously interleave their service calls).

Another issue I have noticed (maybe worthy of a separate thread) is that action calls between two CAP applications with csrf activated is causing issues. Calling an action is a POST request, so the caller is trying to get a csrf token by making a "HEAD" to "someservice/SomeEntity(someId)/SomeService.someAction", which does not work well on the other side (it eventually gets through, but there are "bad request" errors along the way which fill up logs with lengthy axios request dumps - and includes secrets to the logs too). Hence - I think it would be good if the url to which csrf fetching is directed could be configurable (e.g. by querying the service root instead of the resource itself).

Thanks for your support and continuous work on this great framework!

//Carl

david_kunz2
Advisor
Advisor

Thanks onnheimc , I've forwarded these requirements!

carlonnheim
Participant

Great, thanks david.kunz2 ,

Meanwhile we are applying a patch in our project. Not sure if it is of help, we are probably not considering all dependencies, but here it is:

diff --git a/node_modules/@sap/cds/libx/_runtime/remote/utils/client.js b/node_modules/@sap/cds/libx/_runtime/remote/utils/client.js
index deeb86c..46cb18d 100644
--- a/node_modules/@sap/cds/libx/_runtime/remote/utils/client.js
+++ b/node_modules/@sap/cds/libx/_runtime/remote/utils/client.js
@@ -47,7 +47,12 @@ const _executeHttpRequest = async ({ requestConfig, destination, destinationOpti
     // For GET requests, one doesn't need to fetch CSRF tokens.
     // Once we support batch requests (other than autoBatched GET requests),
     // we must check the respective subrequests.
-    const csrfRequired = requestConfig._autoBatch ? false : cds.env.features.fetch_csrf === true
+    let csrfRequired = requestConfig._autoBatch ? false : cds.env.features.fetch_csrf === true;
+    switch (requestConfig._csrfMode) {
+      case 'none': csrfRequired = false; break;
+      case 'standard': csrfRequired = !requestConfig._autoBatch; break;
+      case 'excessive': csrfRequired = true; break;
+    }
     requestOptions = { fetchCsrfToken: csrfRequired }
   }
 
@@ -454,6 +459,7 @@ const getReqOptions = (req, query, service) => {
 
   reqOptions.headers = { accept: 'application/json,text/plain' }
   reqOptions.timeout = service.requestTimeout
+  reqOptions._csrfMode = service.options?.csrf_mode
 
   if (!_hasHeader(req.headers, 'accept-language')) {
     // Forward the locale properties from the original request (including region variants or weight factors),

Regards

//Carl