search

Brightspace / D2L.Security.OAuth2

Brightspace OAuth 2.0 for C#
Apache License 2.0
7 stars 16 forks source link

Changing scope authorizor to be case sensitive. #66

Closed neverendingqs closed 7 years ago

neverendingqs commented 7 years ago

The value of the scope parameter is expressed as a list of space- delimited, case-sensitive strings. ~https://tools.ietf.org/html/rfc6749#section-3.3

I can add tests - just want to make sure I'm reading this right / this wasn't an active decision I'm not aware about.

j3parker commented 7 years ago

Presumably we need to go on an updating tour? We're you planning to do that? (Ok if not) @omsmith how do the JS libraries behave?

omsmith commented 7 years ago

https://github.com/Brightspace/node-auth-token/blob/master/src/index.js#L80

Key equality

Key equality is based on the "SameValueZero" algorithm: NaN is considered the same as NaN (even though NaN !== NaN) and all other values are considered equal according to the semantics of the === operator. In earlier versions of the ECMAScript 2015 draft -0 and +0 were considered distinct (even though -0 === +0), this has been changed in later versions and has been adapted in Gecko 29 (Firefox 29 / Thunderbird 29 / SeaMonkey 2.26) (bug 952870) and a recent nightly Chrome. https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Map

Value equality

Because each value in the Set has to be unique, the value equality will be checked. In an earlier version of ECMAScript specification this was not based on the same algorithm as the one used in the === operator. Specifically, for Sets, +0 (which is strictly equal to -0) and -0 were different values. However, this was changed in the ECMAScript 2015 specification. See "Value equality for -0 and 0" in the browser compatability table for details.

Also, NaN and undefined can also be stored in a Set. NaN is considered the same as NaN (even though NaN !== NaN). https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Set

neverendingqs commented 7 years ago

Presumably we need to go on an updating tour?

I didn't think about it. but I can do it. It sounds like the JS library is okay. Is there a list of all libraries that check scope somewhere?

j3parker commented 7 years ago

I don't think there is much danger in this change in practice, but theoretically it could break things.

I'll likely be making some changes to this library in the near future and will roll this into the next update.