Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
garemoko opened this issue Nov 28, 2017 · 3 comments

Comments

@garemoko
Copy link
Contributor

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.

@jybleau
Copy link

jybleau commented May 30, 2024

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?

@brianjmiller
Copy link
Member

@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.

@jybleau
Copy link

jybleau commented May 30, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants