SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.96k stars 1.24k forks source link

UI5 not fetching a csrf token - conflict with SuccessFactors theming scripts #3666

Closed piejanssens closed 1 year ago

piejanssens commented 1 year ago

OpenUI5 version: 1.96

What is the expected result? UI5 executes a HEAD/GET with "x-csrf-token": "Fetch"

What happens instead? The HEAD call to the OData endpoint is already containing an incorrect x-csrf-token and a valid x-csrf-token from the approuter it not requested. Consequently, the @sap/approuter package refuses any subsequent OData requests and returns 403 - forbidden as documented.

Any other information? A script from SAP SuccessFactors is extending XMLHttpRequest and might be the culprit: https://performancemanager.successfactors.eu/ui/extlib/XMLHttpRequest_1.0.5_sf.18/XMLHttpRequest.js`

As a workaround I have to disable CSRF protection on the @sap/approuter 👎

ThomasChadzelek commented 1 year ago

Are you talking about OData V2 or V4?

piejanssens commented 1 year ago

V2

pksinsik commented 1 year ago

Please provide more details, esp. a network trace (har) would be interesting to see the wrong HEAD and also the backend requests before this. Best regards, Patric

piejanssens commented 1 year ago
  1. Breakpoint in ODataModel-dbg.js on line containing mTokenRequest.request = requestToken("HEAD", handleHeadError);
  2. Activate an XHR Breakpoint set on path '/ci' (my OData service)
  3. Debugger stops in the SF source file for XMLHttpRequest.js... No trace of "x-csrf-token": "Fetch" (it must have been replaced inbetween these steps)
  4. In the same XMLHttpRequest.js file, I put a breakpoint on a.proptotype.setRequestHeader
  5. This is where the header value "Fetch" is being actively replaced by the window.ajaxSecKey / X-Ajax-Token value

From https://performancemanager.successfactors.eu/ui/extlib/XMLHttpRequest_1.0.5_sf.18/XMLHttpRequest.js:

    a.prototype.setRequestHeader = function(b, c) {
        function d(h) {
            return f("X-Ajax-Token", h)
        }
        this._headers || (this._headers = {});
        var f = function(h, g) {
            g = g || b;
            return h && g && g.toLowerCase() === h.toLowerCase()
        };
        if (!d(b) || !Object.keys(this._headers).some(d))
            if (window.ajaxSecKey && f(b, "x-csrf-token") && f(c, "fetch") && (c = String(window.ajaxSecKey)),
            this._headers[b] = c,
            "X-Allow-CORS" !== b)
                return this._object.setRequestHeader(b, c)
    }

This part is where it happens:

if (window.ajaxSecKey && f(b, "x-csrf-token") && f(c, "fetch") && (c = String(window.ajaxSecKey)),
            this._headers[b] = c,
            "X-Allow-CORS" !== b)

It basically reads as if ajaxSecKey exists and we are setting "x-csrf-token" with a value of "Fetch", then replace the value with the ajaxSecKey and set that instead.

It feels like a bad idea to put custom logic in XMLHttpRequest...

I know you can now close this swiftly by saying this is not a UI5 issue, but maybe you have a way to reach the SAP developers behind this file internally? We would save a lot of blood sweat and tears trying to get through the first 20 lines of SF support... :-)

pksinsik commented 1 year ago

I'll do my best to get this forwarded.