TinCanJS icon indicating copy to clipboard operation
TinCanJS copied to clipboard

Catch error when neither auth or username and password LRS properties are set

Open garemoko opened this issue 8 years ago • 3 comments

Currently, if you don't specify either auth or username and password config properties (maybe you specify key and secret by mistake) and then try to make a request (e.g. send a statement) the error you get in node is one that comes from the xhr2 library complaining that your trying ton convert a null property (the auth header value) toString.

It would be 'nice to have' for TinCanJS to catch the error further up the chain with a message that you tried to make a request when the LRS auth is null and to check that either auth or username and password are set.

garemoko avatar Nov 28 '17 10:11 garemoko

I've had to deal with a LMS that does not provide any of the auth, username, pw query param. It relies on session cookie already set by the LMS, and the endpoint is on the same domain.

I think the Authorization header should just be ommited if no auth params are set.

@brianjmiller what do you think? Should I create a PR?

jybleau avatar May 30 '24 16:05 jybleau

@jybleau though I don't love the reason, in general the LMS should not do that, and I don't think we'd accept a patch to allow that, ultimately maybe they should accept something in Authorization and just ignore it, or leverage the header instead. But, the reason, several years ago there was a patent troll that came out of the woodworks and started firing off legal notices to extract money from anyone leveraging an LRS, and what we eventually determined was that the patent didn't apply whenever an Authorization header was included in the requests. So to avoid a patent claim an Authorization header should always be sent, even if it gets ignored.

brianjmiller avatar May 30 '24 17:05 brianjmiller

@brianjmiller that makes sense. I doubt i will make them change, but I'll ask.

Possible options I see. A: Could a solution be that the library should just set an empty Authorization header when the auth property is still null?

https://github.com/RusticiSoftware/TinCanJS/blob/master/src/LRS.js#L287 Could be something like: headers.Authorization = this.auth || '';

Then it will not crash when calling xhr.setRequestHeader(prop, headers[prop]); later.

Or B: On my side of the code, I could check the query params and set an empty auth when nothing is provided. No need to modify TinCanJS then.

Thanks

jybleau avatar May 30 '24 18:05 jybleau