ivmartel / dwv

DICOM Web Viewer: open source zero footprint medical image library.
https://ivmartel.github.io/dwv/
GNU General Public License v3.0
1.64k stars 591 forks source link

Request not sending Credentials/Cookies with it #707

Closed fschirinzi closed 4 years ago

fschirinzi commented 4 years ago

Problem: Images that were requested from within the dvw.js were rejected, because of authorization, from my backend. I use HttpOnly Cookies to send the auth token. After analyzing the network traffic I saw, that the credentials weren't sent with my requests. I also couldn't find a way to set withCredentials = true

Temporary Fix/Solution:

Adding this snippet globally in my app.

var xp = XMLHttpRequest.prototype;
XMLHttpRequest = function (args) {
  var obj = new xp.constructor(args);
  obj.withCredentials = true;
  return obj;
};
XMLHttpRequest.prototype = xp;
fschirinzi commented 4 years ago

I'm not an expert in JS. But if you need help or if I should make a pull request (with a professional solution), just tell me.

ivmartel commented 4 years ago

From what I get, the withCredentials flag is not needed when you manually set an Authorization request header (ref). The app.loadURLs accepts the headers as a second optional argument. Would that be enough?

fschirinzi commented 4 years ago

I saw that possibility. Unfortunately, since I use a secure & httpOnly cookie (MDN Web docs - Secure & HttpOnly Cookies), I can't access the token from javascript and pass it as an argument. That's why I need the possibility to set the withCredentials.

Thanks for your help!

ivmartel commented 4 years ago

I was not sure there was a case where withCredentials was needed, now you're giving me one! Adding it as an option for the app.loadURLs does not look complicated, I can do it for this release.

fschirinzi commented 4 years ago

Thanks! :)

ivmartel commented 4 years ago

As ref, here is a link to the documentation about XMLHttpRequest/withCredentials.

ivmartel commented 4 years ago

I'm still thinking about the api, it would maybe be simpler if the app.loadURLs just took one option object containing headers and the withCredentials flag...

fschirinzi commented 4 years ago

For backward compatibility and to reduce changes to this code, you could change the signature of loadURLS the following:

function loadURLs(urls, requestHeaders, overwriteRequestOptions = {});

Then you can iterate over the keys in overwriteRequestOptions, kinda like for requestHeaders, and set the options to the request variable.

for (let key in overwriteRequestOptions) {
    if (overwriteRequestOptions.hasOwnProperty(key)) {
      request[key] = overwriteRequestOptions[key];
    }
  }

OR shorter

request = { ...request, ...overwriteRequestOptions}

In this case, if overwriteRequestOptions is null or undefined, the request object is not changed.

If one wants to set other or custom options, there is no need to change the code again.

What do you think?